Skip to content

FIX: Include endpoint and method in HTTPXAPITarget identifier - #3374

Merged
Roman Lutz (romanlutz) merged 10 commits into
microsoft:mainfrom
ManoharPaturi:fix/httpx-api-target-identifier
Oct 25, 2026
Merged

Roman Lutz (romanlutz) merged 10 commits into
microsoft:mainfrom
ManoharPaturi:fix/httpx-api-target-identifier

Conversation

@ManoharPaturi

Copy link
Copy Markdown
Contributor

Description

Fixes #3373

HTTPXAPITarget previously delegated raw request setup to HTTPTarget with an empty string, causing the target endpoint to remain an empty string. It also inherited HTTPTarget._build_identifier without overriding it, leaving prompt_regex_string and use_tls while omitting method and http2. This change sets the endpoint and destination origin to the configured http_url and implements _build_identifier to capture method, follow_redirects, callback_function, and http2 so that distinct API targets produce distinct identifiers.

Tests and Documentation

Added unit tests in test_http_api_target.py verifying that HTTPXAPITarget populates its endpoint and method parameters properly, and that distinct endpoints and methods produce distinct identifier hashes.

No documentation change needed, so I did not run JupyText.

@romanlutz Roman Lutz (romanlutz) 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.

Please address the credential exposure and the remaining identifier collision described below before merging.

Comment thread pyrit/prompt_target/http_target/httpx_api_target.py Outdated
Comment thread pyrit/prompt_target/http_target/httpx_api_target.py Outdated
@ManoharPaturi

Copy link
Copy Markdown
Contributor Author

Roman Lutz (@romanlutz) thanks for checking this PR I will update it as soon as possible

@ManoharPaturi

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review Roman Lutz (@romanlutz).

I have updated the implementation to address both points. The target now resolves http_url against any configured base_url via httpx.URL(base_url).join(http_url) before building the identifier and origin. This ensures relative paths pointing to different hosts or base paths resolve their destination origin and avoid hash collisions.

For credential handling, the identifier endpoint now strips userinfo (username and password) and filters out sensitive query parameters like keys, tokens, secrets, signatures, and auth parameters. The full raw URL is preserved on the target for actual outbound requests.

I also added unit tests covering credential redaction from the identifier dump, base_url path resolution across distinct hosts, and credential stripping when credentials exist in base_url.

… with httpx URL semantics

Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
@ManoharPaturi

Manohar Paturi (ManoharPaturi) commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor Author

Roman Lutz (@romanlutz) both follow-ups are addressed in f7312f8.

P1 — remaining credential exposure:

  • The fragment is now dropped from the identifier entirely (copy_with(fragment=None)) since it is never part of the HTTP request target, so #access_token=dummy-token no longer reaches the persisted identifier.
  • jwt, assertion (covers client_assertion), and bearer were added to the sensitive query-name filter, so ?jwt=dummy-token and ?client_assertion=dummy-assertion are stripped like the other credential names.
  • New test test_httpx_api_target_identifier_strips_fragment_and_extra_credential_queries covers both cases and asserts the identifier dump contains neither dummy value. The full raw URL is still preserved for the outbound request.

P2 — URL resolution semantics:

  • Replaced URL(base_url).join(http_url) with _merge_base_and_url, which replicates httpx.AsyncClient._merge_url exactly: the base URL is given a trailing slash on its raw path and the request path is appended with any leading slash stripped. So invoke and /invoke against base_url="https://example.test/alpha" both resolve to /alpha/invoke, and alpha vs beta bases produce distinct endpoints and hashes instead of colliding on https://example.test/invoke.
  • New test test_httpx_api_target_identifier_matches_captured_request_url sends through the real send path with a MockTransport that captures the actual request URLs, and asserts the identifier endpoints equal the captured URLs for the alpha/beta base paths plus the leading-slash spelling, so the test locks the identifier to the exact URLs httpx requests rather than to our own merge logic.

The full test file passes (22 tests) and Ruff check/format are clean. Requesting re-review — thanks again for the thorough validation.

Comment thread pyrit/prompt_target/http_target/httpx_api_target.py
Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
@ManoharPaturi

Copy link
Copy Markdown
Contributor Author

Roman Lutz (@romanlutz) fixed in 6925784.

P2 — substring false positives:

  • _is_sensitive_query_param now splits the parameter name into words on non-alphanumeric separators and matches those tokens against the credential-name set (_CREDENTIAL_QUERY_NAME_TOKENS), instead of substring matching. So design (contains "sig"), author (contains "auth"), and assigned_model keep their values and distinct URLs keep distinct identifiers, while compound credential names (api_key, client_secret, api-token, access_token, jwt, client_assertion) are still redacted because they contain the word key, secret, token, jwt, or assertion as a whole word. Standalone names like code and pass are still matched as exact full names.
  • New regression test test_httpx_api_target_identifier_keeps_non_credential_names_containing_credential_substrings covers your exact case: ?design=alpha vs ?design=beta keep their values and get distinct hashes, plus author/assigned_model kept and the compound credential names still stripped.

Full test file passes (23 tests), Ruff check/format clean. Requesting re-review.

@romanlutz

Copy link
Copy Markdown
Contributor

Summary of the review findings and the discussions that are still open:

  1. P1: Credentials reaching stored metadata. URL fragments and credential query parameters such as jwt and client_assertion were still appearing in persisted identifiers, registry metadata, and REST responses. Discussion and reproduction.
  2. P2: Identifier URL differing from the actual request URL. Base URLs without trailing slashes and request paths with leading slashes were resolved differently from HTTPX, so different destinations could still share an identifier. Discussion and reproduction.
  3. P2: Harmless query parameters being removed. Substring matching treated names such as design, author, and assigned_model as credentials, creating new identifier collisions. Discussion and reproduction.

These findings were reproduced against 81c3481. The author has since pushed f7312f8 and 6925784 and reports fixes for all three. I have not re-reviewed those newer commits yet, so the open threads are a verification checklist, not a claim that the issues still reproduce on the latest head.

@romanlutz

Copy link
Copy Markdown
Contributor

Re-reviewed 86ad4d9. The URL-resolution and harmless-query-parameter collision findings are fixed, and I have resolved both threads.

One P1 finding is still open: camelCase credential names such as accessToken, clientSecret, and authToken bypass the new matcher, so their values still appear in persisted identifiers, registry metadata, and REST responses. The reproduction and requested change are in the remaining open discussion.

Please address that remaining credential-exposure issue before merging.

@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 25, 2026
Merged via the queue into microsoft:main with commit b2b9087 Oct 25, 2026
65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: HTTPXAPITarget identifier leaves endpoint empty and drops request parameters

2 participants