Skip to content

DEV-15466: Replaced cURL command with PUT Request in es_configure.py - #4708

Merged
gregrholden merged 4 commits into
qatfrom
fix/DEV-15466-remove-es-creds-from-curl-cmd
Jul 29, 2026
Merged

DEV-15466: Replaced cURL command with PUT Request in es_configure.py#4708
gregrholden merged 4 commits into
qatfrom
fix/DEV-15466-remove-es-creds-from-curl-cmd

Conversation

@gregrholden

Copy link
Copy Markdown
Contributor

Description:

In vulnerability finding f222, it was reported that ES_HOSTNAME (with embedded basic-auth) passed through the shell curl command line. DEV-15466 notes:

  • ES_USER and ES_PASSWORD values can potentially be added to the cURL request made in es_configure.py, but that it only happens when someone is using environment variables to interact with the API and so would not happen in production.
  • Initial solution was to remove the ability to use env vars for passing this user/pass combo to secure it.
  • Another proposed solution was to remove the cURL command entirely in favor of a PUT Request.
    • This option was implemented.

Technical Details:

Solution implemented:

  1. Removed subprocess dependency in exchange for the use of the Requests library.
  2. Added secure HTTP handling with urllib.parse.
  3. Refactored run_curl_cmd method to handle the PUT request using the Request library instead of a cURL command.
  4. Small code refactoring to fix lint findings (e.g., missing data types and return types).

NOTE: There is an assumption made here that this will continue working in production with this change, but this needs to be tested in an environment (not a local) to be certain.

Requirements for PR Merge:

  1. Unit & integration tests updated
  2. API documentation updated (examples listed below)
    1. API Contracts
    2. API UI
    3. Comments
  3. Data validation completed (examples listed below)
    1. Does this work well with the current frontend? Or is the frontend aware of a needed change?
    2. Is performance impacted in the changes (e.g., API, pipeline, downloads, etc.)?
    3. Is the expected data returned with the expected format?
  4. Appropriate Operations ticket(s) created
  5. Jira Ticket(s)
    1. DEV-0

Explain N/A in above checklist:

@gregrholden gregrholden self-assigned this Jul 27, 2026
@gregrholden gregrholden added the ready for review [PR] ready to be reviewed label Jul 27, 2026

@james-at-kc james-at-kc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. cURL construction was fully replaced by PUT request. Ready for merge.

@gregrholden
gregrholden merged commit bc145f3 into qat Jul 29, 2026
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review [PR] ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants