Skip to content

[WRONG BRANCH] fix(responses): strip admission bearer from forwarded headers for routed providers - #324

Closed
luvs01 wants to merge 1 commit into
mainfrom
codex/propose-fix-for-bearer-token-vulnerability
Closed

[WRONG BRANCH] fix(responses): strip admission bearer from forwarded headers for routed providers#324
luvs01 wants to merge 1 commit into
mainfrom
codex/propose-fix-for-bearer-token-vulnerability

Conversation

@luvs01

@luvs01 luvs01 commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Prevent OpenCodex admission bearer tokens (Authorization: Bearer ocx_data_*) from being forwarded to routed, non-Codex adapters that may treat that header as their own upstream credential.
  • Preserve existing behavior for pool/main substitution and for dedicated-header passthrough to native Codex routes while removing the leak vector introduced by preserving the original Authorization header for routed adapters.

Description

  • Added an explicit stripAuthorization option to materializeCodexUpstreamAuth and respected it when building selectedForwardHeaders, so admission bearers can be removed before adapter dispatch (src/codex/auth-context.ts).
  • When resolving Responses auth, compute stripAuthorization for caller-admitted bearer tokens that are not substituted with a stored Codex credential and pass it into materializeCodexUpstreamAuth (src/server/responses/core.ts).
  • Kept existing substitution semantics: pool/main-pool overwrite, main substitution when requested, and dedicated-header passthrough for native Codex forward paths remain unchanged.
  • Added a unit regression asserting that a routed-provider admission bearer is removed while unrelated forwarded headers continue to be forwarded (tests/codex-auth-context.test.ts).
  • Files changed: src/server/responses/core.ts, src/codex/auth-context.ts, and tests/codex-auth-context.test.ts.

Testing

  • Ran the focused unit tests: npx -y bun@1.3.14 test tests/codex-auth-context.test.ts, and the updated codex-auth-context tests passed (including the new case).
  • Ran typecheck with npx -y bun@1.3.14 run typecheck which completed successfully.
  • Ran privacy checks with npx -y bun@1.3.14 run privacy:scan which passed.
  • Ran the original integration test for bearer admission (npx -y bun@1.3.14 test tests/bearer-admission-routed-provider.test.ts) in this environment and observed environment-dependent failures (loopback/private-network / 403 and other harness differences) unrelated to the change; the changes are focused and the new unit coverage verifies the intended behavior.
  • A full npx -y bun@1.3.14 run test was attempted; many unrelated, environment-sensitive tests failed in this runner (infrastructure / Docker / platform-specific guards), so full-suite failures are considered unrelated to this targeted fix. The focused regression and static checks are green.

Codex Task

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c7ee983b-a7f7-4098-95e8-73f0380ce9bf


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Aug 20, 2026
@github-actions

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot changed the title fix(responses): strip admission bearer from forwarded headers for routed providers [WRONG BRANCH] fix(responses): strip admission bearer from forwarded headers for routed providers Aug 20, 2026
@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

⏳ DRAFT

  • wrong target branch (main); retarget to dev.

What to do

  • Retarget this PR to dev — all contributions go to dev.

Its title has been prefixed with [WRONG BRANCH].
This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.

@github-actions
github-actions Bot marked this pull request as draft August 20, 2026 15:05

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e163c89d0e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// consume a stored Codex credential still need the caller's admission Authorization removed:
// some adapters intentionally use forwarded Authorization as their provider credential.
// Dedicated-header admission remains unchanged because its Authorization belongs upstream.
const stripAuthorization = options.admission?.source === "bearer" && !substituteMainCredential;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Inspect Authorization before preserving it

When a request supplies both a valid x-opencodex-api-key and Authorization: Bearer <another proxy key>, resolveResponsesApiAuth deliberately records the admission source as dedicated, so this condition is false and the proxy bearer remains in the forwarded headers. A routed Cursor provider without its own apiKey, for example, consumes that forwarded value as its upstream token in src/adapters/cursor/live-transport.ts:159-164, exposing an OpenCodex secret despite this fix. Determine whether the actual Authorization value is a proxy admission secret (for example via isProxyAdmissionSecret) instead of relying solely on the winning admission source, while preserving genuinely foreign upstream bearers.

AGENTS.md reference: AGENTS.md:L266-L272

Useful? React with 👍 / 👎.

@luvs01 luvs01 closed this Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark bug Something isn't working codex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant