Skip to content

fix(privacy-shield): preserve HTTP query strings - #2972

Open
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/privacy-shield-query-passthrough
Open

fix(privacy-shield): preserve HTTP query strings#2972
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/privacy-shield-query-passthrough

Conversation

@tang-vu

@tang-vu tang-vu commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Why this matters

Privacy Shield's authenticated HTTP catch-all rebuilt the upstream URL from only the route path. Query parameters were silently discarded, so pagination, filters, cursors, feature flags, and provider-specific options could change meaning between the client request and the model/API upstream.

Root cause: target_url omitted the ASGI query_string even though the WebSocket lane already preserves query parameters.

Behavioral invariant: the exact raw query bytes received at the HTTP boundary are attached to the same upstream path without decoding/re-encoding them.

Overlap check

Searched open and closed upstream PRs for Privacy Shield query-string/query-parameter/proxy-URL/passthrough terms and inspected PRs changing ods/extensions/services/privacy-shield/proxy.py. Current work addresses an await bug and error-information leakage; no PR preserves HTTP query strings. The WebSocket query handling is existing behavior and is not modified.

What changed

  • Build the upstream target as an httpx.URL.
  • Copy the raw ASGI query bytes onto that URL before opening the stream.
  • Add a public authenticated GET regression with ordered parameters, an encoded slash, and a plus sign.

Validation

  • Red before fix: upstream observed only /v1/models; ?limit=5&cursor=a%2Fb&tag=x+y was missing.
  • pytest tests/test_streaming_proxy.py::TestQueryPassthrough -q — 1 passed.
  • pytest tests -q — 53 passed.
  • python -m py_compile proxy.py tests/test_streaming_proxy.py — passed.
  • git diff --check — passed.

Platform and rollback

This is shared Python behavior across Linux, macOS, and Windows/WSL containers; no platform-specific path changes. The request body, PII scrubbing, headers, and response streaming are untouched. Rollback is one commit with no state migration.

Batch compatibility

Validated as an independent ten-PR batch from upstream main 6ff9b4fc5190099705043acaab7e9b6ad9c8b8f1. The final PR heads merged without conflicts in this order: #2964 -> #2965 -> #2967 -> #2969 -> #2970 -> #2971 -> #2972 -> #2973 -> #2974 -> #2975. The resulting local synthetic merge head is 4ae60eadad9696a9accb735aad71af87d2be802d.

Combined validation on that exact tree:

  • Dashboard API boundary suites: 323 passed, 4 skipped.
  • Token Spy suite: 36 passed, 1 skipped.
  • Privacy Shield suite: 55 passed.
  • APE suite: 47 passed.
  • Python compile checks and git diff --check: passed.

The scopes are behaviorally independent. The stated order is the tested rollback/merge sequence for shared-file changes; each PR remains individually useful and revertible.

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.

1 participant