Skip to content

fix(proxy): reject a private x-headroom-base-url on chat and responses with 400 - #4857

Open
chopratejas wants to merge 5 commits into
mainfrom
fix/openai-base-url-reject
Open

chopratejas wants to merge 5 commits into
mainfrom
fix/openai-base-url-reject

Conversation

@chopratejas

@chopratejas chopratejas commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

Description

When the upstream guard rejected an x-headroom-base-url override (an address on a private, loopback, link-local or otherwise non-public network, or a host outside HEADROOM_ALLOWED_BASE_URLS in allowlist mode), _resolve_openai_upstream_base logged a warning and returned None. /v1/chat/completions and /v1/responses then sent the request — the prompt and the client's Authorization, meant for the gateway the client named — to the configured OPENAI_API_URL instead. Every other route that reads the header (headroom/providers/proxy_routes.py: Anthropic messages, Gemini, models, embeddings, audio, passthrough) refuses with 400 Rejected unsafe upstream base URL.

The OpenAI chat and responses handlers now refuse the same way.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Changes Made

  • headroom/proxy/handlers/openai.py: _resolve_openai_upstream_base raises HTTPException(400, "Rejected unsafe upstream base URL") instead of returning None when the guard rejects the override (malformed / non-http(s) values are unchanged: still ignored).
  • docs/content/docs/security-model.mdx: one behaviour for all routes (the page previously documented the two different outcomes).
  • tests/test_upstream_guard.py::test_openai_routes_refuse_a_private_override_instead_of_using_the_default[chat|responses]: real app, configured OpenAI target is a local recording server; asserts 400 and that the configured target is never contacted.

Sibling PRs that touch the same code (merge order note): fix/upstream-guard-off-loop adds _resolve_openai_upstream_base_async next to this function; whichever merges second must make the async resolver raise the same 400 (that PR's rejection branch is the one place to change). fix/upstream-idna-pin-once touches the call sites only. Suggested order: idna-pin-once, off-loop, then this PR rebased.

Testing

  • Unit tests pass (pytest)
  • Linting passes (ruff check .)
  • Type checking passes (mypy headroom)
  • New tests added for new functionality
  • Manual testing performed

Test Output

# New test on origin/main (c21b0813)
E   AssertionError: {"secret":"internal-only"}
E   assert 200 == 400                (chat: answered by the configured target)
E   assert 200 == 400                (responses: answered by the configured target)
======================= 2 failed, 61 deselected in 5.64s =======================

# This branch
======================= 2 passed, 61 deselected in 5.81s =======================

# Area suites: test_upstream_guard, test_grok_shared_proxy_call_sites, test_proxy/test_openai_upstream_header,
# test_proxy_openai_cache_key_integration, test_upstream_credential_scoping, test_header_isolation,
# test_proxy_hardening, test_openai_codex_routing, test_proxy_loopback_gating
================= 429 passed, 10 warnings in 87.64s (0:01:27) ==================

$ ruff check / ruff format --check (0.17.3) on changed files
All checks passed!
2 files already formatted

Real Behavior Proof

  • Environment: macOS, Python 3.12, python -m headroom.cli proxy --no-optimize from this branch and from origin/main, OPENAI_TARGET_API_URL pointing at a local server that records every request it receives.
  • Exact command / steps:
    curl http://127.0.0.1:$PORT/v1/chat/completions -H 'authorization: Bearer client-gateway-key' -H 'x-headroom-base-url: http://192.168.1.50:8000/v1' -d '{"model":"gpt-4o","messages":[...]}', and the same for /v1/responses.
  • Observed result:
    == origin/main
    POST /v1/chat/completions -> HTTP 200 {"...":"from-configured-target"}
    POST /v1/responses        -> HTTP 200 {"...":"from-configured-target"}
    configured target received:
      /v1/chat/completions auth= Bearer client-gateway-key
      /v1/responses        auth= Bearer client-gateway-key
    == this branch
    POST /v1/chat/completions -> HTTP 400 {"detail":"Rejected unsafe upstream base URL"}
    POST /v1/responses        -> HTTP 400 {"detail":"Rejected unsafe upstream base URL"}
    configured target received: (nothing)
    
  • Not tested: Codex WebSocket responses path (it does not read this header).

Runtime Rollout Safety

  • Rollout-managed feature(s): none
  • Minimum rollout channel: n/a
  • Stable/default behavior changed: yes. A client that sends x-headroom-base-url pointing at a private/LAN/loopback address (or, in allowlist mode, an unlisted host) to /v1/chat/completions or /v1/responses now gets a 400 instead of having its request answered by the configured OpenAI target. Such a request never reached the host the client named, so the only working setups affected are ones that relied on the silent fallback. A DNS lookup that fails or times out for a public gateway is also a rejection, so it now surfaces as a 400 instead of quietly sending the request to the default target.
  • Kill switch / disable path: operators who route to an internal gateway add it to HEADROOM_ALLOWED_BASE_URLS (existing setting); clients can drop the header to use the configured target.
  • Unsafe override required: no
  • Qualification impact: none
  • Rollback path: revert

Review Readiness

  • I have performed a self-review
  • This PR is ready for human review

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I did not edit CHANGELOG.md — it is generated by release-please from my Conventional Commit PR title (a CI guard enforces this)

Additional Notes

Fix adapted from a proposal by HetCreep.

Follow-up, not in this PR: the OpenCode plugin (plugins/opencode/src/transport.ts, isRoutableUpstream) only keeps loopback upstreams out of the proxy, so an OpenCode provider on a LAN address is still routed through Headroom, which now answers 400 for it (previously the request went to the default OpenAI target). Sending private-network upstreams direct needs a source change plus regenerating the committed bundles (headroom/providers/opencode/_dist/index.js, hook-shim/handler.js), and hostnames that resolve to LAN addresses cannot be recognised without DNS, so it is left for a separate PR.

🤖 Generated with Claude Code


Devin Review

Reported by @JoaoMarcos44. Thank you for your contribution! 🩵

…s with 400

When the upstream guard rejected the x-headroom-base-url override, the
OpenAI chat-completions and responses handlers dropped the header and sent
the request, with the client's Authorization, to the configured OpenAI
target. Every other route that reads the header refuses with 400.

The two handlers now refuse the same way, so a prompt and key meant for the
client's own gateway are not sent to a different host. The security model
page is updated to describe one behaviour for all routes.

Assay-Kind: bugfix
Assay-Ask: A rejected private x-headroom-base-url on chat/responses is silently dropped and the request with the caller's key goes to the default OpenAI target.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chopratejas chopratejas added bug Something isn't working size: small At most 100 added/deleted lines. risk: medium Scope estimate: bounded implementation or tooling change; verification still required. area: proxy Touches proxy files; triage routing, not a CODEOWNERS assignment. labels Nov 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR governance

This PR does not yet satisfy the required template fields:

  • Fill in Real Behavior Proof → Exact command / steps.
  • Fill in Real Behavior Proof → Observed result.

Please update the PR body, or move the PR back to draft while it is still in progress.

@github-actions github-actions Bot added status: needs author action Pull request body or readiness checklist still needs author updates status: ci failing Required or reported CI checks are failing labels Nov 13, 2026

@devin-ai-integration devin-ai-integration Bot 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review completed; approval remains pending the items below.

Reviewed head b2bccf4f6f8309d642bb6c1253800decee71ee04 against main 0368a872cdd5064b39888e13847200ff562ac192.

Qualification still required:

  • Hosted check evidence changed
  • Passing test evidence is missing
  • Exact-source provenance review is incomplete
  • Check/run 132188737092 is not terminal
  • Check/run 132188736926 is not terminal
  • Check/run 132186956047 is not successful or baseline-qualified
  • Check/run 132186955946 is not successful or baseline-qualified
  • Check/run 43512780771 is not successful or baseline-qualified
  • Check/run 43512780689 is not terminal

Assay-Kind: deps
Assay-Ask: The dependency audit checks failed on this PR because the branch predates the dependency updates on main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot removed the status: ci failing Required or reported CI checks are failing label Nov 13, 2026

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review completed; approval remains pending the items below.

Reviewed head 3ad01b49193d8dc592a3a48b4d4ba14f7d2c5158 against main 0368a872cdd5064b39888e13847200ff562ac192.

Qualification still required:

  • Passing test evidence is missing
  • Exact-source provenance review is incomplete

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 3ad01b4 against 0368a87. No blocking defects found. Independently ran the upstream-guard suite: 63 passed in the supported Python-only runtime. Both new chat/responses regressions return 400 without contacting the configured recording target; on recorded main the same tests fail because the target returns 200. Hosted checks also report success. Native Rust runtime and external provider behavior were not tested. Approval uses the verified review-only baseline; inherited workflow provenance remains held and no workflow execution is requested.

JerrettDavis
JerrettDavis previously approved these changes Nov 14, 2026

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed bd3ef99 against b324efb. No blocking defects found. Independently ran tests/test_upstream_guard.py: 63 passed in Python-only mode. Both new chat/responses cases return 400 without contacting the configured recording target; on recorded main both fail with HTTP 200 from that target. Current-head hosted checks report success. Native Rust runtime and external providers were not tested. Approval uses the verified review-only baseline; inherited workflow execution provenance remains held and no workflow execution is requested.

chopratejas and others added 2 commits November 13, 2026 21:23
Kept both sides of the test_upstream_guard.py conflict (this branch's 400
test and main's non-ASCII pin test). Main's resolve-once test now expects
the 400 this branch returns for a rejected override, instead of a fallback
to the configured target.

Assay-Kind: config
Assay-Ask: The branch conflicts with main after the sibling upstream fix was merged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… fix/openai-base-url-reject

Brings in the earlier merge of main already on the branch; no content change.

Assay-Kind: config
Assay-Ask: The branch conflicts with main after the sibling upstream fix was merged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chopratejas

Copy link
Copy Markdown
Collaborator Author

Jerrett: conflict-only merge with main (c2804aa, plus a5f9f46 to take in your earlier merge commit; no content change from that one). In tests/test_upstream_guard.py I kept both this PR's 400 test and main's non-ASCII pin test. Main's resolve-once test now expects the 400 this PR returns for a rejected override, instead of a fallback to the configured target. No production code changed in the merge. Area tests, ruff and mypy pass locally.

@github-actions github-actions Bot added the status: ci failing Required or reported CI checks are failing label Nov 14, 2026
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@chopratejas

Copy link
Copy Markdown
Collaborator Author

Jerrett: the approval was dismissed by the merge push after all, so I re-requested review. The one red check, test (3), is tests/test_memory_tools_only_when_injected.py::test_memory_call_the_proxy_did_not_inject_reaches_the_client[not_injected-anthropic*], which fails the same way on main (fda75de) and passes locally on this branch.

This branch was successfully deployed

1 active deployment
Preview — a5f9f464 Deployed Nov 14, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: proxy Touches proxy files; triage routing, not a CODEOWNERS assignment. bug Something isn't working risk: medium Scope estimate: bounded implementation or tooling change; verification still required. size: small At most 100 added/deleted lines. status: ci failing Required or reported CI checks are failing status: needs author action Pull request body or readiness checklist still needs author updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants