fix(claude): isolate native passthrough credentials - #1536
Conversation
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
📝 WalkthroughWalkthroughNative Anthropic passthrough now requires dedicated proxy admission on non-loopback listeners. It validates credentials, removes proxy secrets, rejects ambiguous headers, propagates listener policy, and documents the behavior across supported languages. ChangesNative Anthropic passthrough
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ClaudeHandlers
participant RequestPolicyView
participant AnthropicUpstream
Client->>ClaudeHandlers: Send Messages or count_tokens request
ClaudeHandlers->>RequestPolicyView: Validate listener admission
RequestPolicyView-->>ClaudeHandlers: Return admission decision
ClaudeHandlers->>ClaudeHandlers: Validate and filter credentials
ClaudeHandlers->>AnthropicUpstream: Forward filtered request
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/ko/guides/claude-code.md`:
- Around line 84-86: Clarify the passthrough conditions in
docs-site/src/content/docs/ko/guides/claude-code.md lines 84-86,
docs-site/src/content/docs/ru/guides/claude-code.md lines 64-66, and
docs-site/src/content/docs/zh-cn/guides/claude-code.md lines 58-60 so each
explicitly states that the token following “Bearer” starts with “sk-ant-”,
rather than implying the complete Authorization header does. Preserve the
distinction between native Anthropic OAuth/API-key authentication and
passthrough behavior in all three localized guides.
In `@docs-site/src/content/docs/reference/configuration/server.md`:
- Around line 63-67: Update the admission-policy paragraphs to document both
`/v1/messages` and `/v1/messages/count_tokens`, preserving the existing policy
details. Apply this change in
docs-site/src/content/docs/reference/configuration/server.md lines 63-67,
docs-site/src/content/docs/ko/reference/configuration/server.md lines 57-60, and
docs-site/src/content/docs/ru/reference/configuration/server.md lines 65-69.
In `@src/server/index.ts`:
- Around line 1243-1244: Update the withCors call wrapping handleClaudeMessages
in the /v1/messages branch to use the request’s policy instead of the shared
config, matching the count_tokens branch and ensuring CORS decisions use the
correct listener view.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c4bf0697-c89c-4077-8c7a-138a3b427ef4
📒 Files selected for processing (21)
docs-site/src/content/docs/guides/claude-code.mddocs-site/src/content/docs/ja/guides/claude-code.mddocs-site/src/content/docs/ja/reference/configuration/server.mddocs-site/src/content/docs/ja/reference/proxy-formats.mddocs-site/src/content/docs/ko/guides/claude-code.mddocs-site/src/content/docs/ko/reference/configuration/server.mddocs-site/src/content/docs/ko/reference/proxy-formats.mddocs-site/src/content/docs/reference/configuration/server.mddocs-site/src/content/docs/reference/proxy-formats.mddocs-site/src/content/docs/ru/guides/claude-code.mddocs-site/src/content/docs/ru/reference/configuration/server.mddocs-site/src/content/docs/ru/reference/proxy-formats.mddocs-site/src/content/docs/zh-cn/guides/claude-code.mddocs-site/src/content/docs/zh-cn/reference/configuration/server.mddocs-site/src/content/docs/zh-cn/reference/proxy-formats.mddocs-site/src/content/docs/zh-tw/guides/claude-code.mddocs-site/src/content/docs/zh-tw/reference/configuration/server.mddocs-site/src/content/docs/zh-tw/reference/proxy-formats.mdsrc/server/claude-messages.tssrc/server/index.tstests/claude-native-passthrough.test.ts
|
Tip For best results, initiate chat on the files or code changes.
I am replying to the latest comment only. The earlier comment chain is not available. This status does not request a review. Post 🧠 Learnings usedYou are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
No review was requested. Post 🧠 Learnings usedYou are interacting with an AI system. |
1768e78 to
884e7fe
Compare
|
Tip For best results, initiate chat on the files or code changes.
No CodeRabbit review was requested. Post 🧠 Learnings usedYou are interacting with an AI system. |
|
🧠 Learnings used
|
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedYou are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/ja/reference/configuration/server.md`:
- Line 56: Update the Japanese sentence in the configuration documentation so it
states that candidate values are compared in constant time, using wording
equivalent to「候補値は定数時間で比較されます」. Keep the surrounding explanation unchanged and
align it with the English source’s constant-time comparison guarantee.
In `@tests/loopback-listener-admission.test.ts`:
- Around line 70-75: Extend the loopback listener assertions for the two source
branches covering handleClaudeCountTokens and handleClaudeMessages, verifying
each exact handler call passes policy rather than config. Keep the existing
withCors assertions so both handler policy propagation and CORS policy
propagation remain covered.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a6ebff89-c874-4ddb-81c5-bd5f4f1737ed
📒 Files selected for processing (14)
docs-site/src/content/docs/guides/claude-code.mddocs-site/src/content/docs/ja/guides/claude-code.mddocs-site/src/content/docs/ja/reference/configuration/server.mddocs-site/src/content/docs/ko/guides/claude-code.mddocs-site/src/content/docs/ko/reference/configuration/server.mddocs-site/src/content/docs/reference/configuration/server.mddocs-site/src/content/docs/ru/guides/claude-code.mddocs-site/src/content/docs/ru/reference/configuration/server.mddocs-site/src/content/docs/zh-cn/guides/claude-code.mddocs-site/src/content/docs/zh-cn/reference/configuration/server.mddocs-site/src/content/docs/zh-tw/guides/claude-code.mddocs-site/src/content/docs/zh-tw/reference/configuration/server.mdsrc/server/index.tstests/loopback-listener-admission.test.ts
| | `/v1/messages/count_tokens` |受け入れられました |受け入れられました |受け入れられました | | ||
| | `/v1/models` |受け入れられました |受け入れられました |受け入れられました | | ||
|
|
||
| 応答とチャット完了では、Codex Direct パススルーの可能性のために `Authorization` を予約しているため、そこでは専用のアドミッション ヘッダーのみが受け入れられます。ダッシュボードで生成された `apiKeys` は、起動後に環境トークンを置き換える可能性があります。候補は一定時間内に比較されます。 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Preserve the constant-time comparison guarantee.
Line 56 says 候補は一定時間内に比較されます. This describes a time window, not constant-time comparison. The English source states that candidate credentials are compared in constant time. Use Japanese wording such as 候補値は定数時間で比較されます.
Proposed wording
-候補は一定時間内に比較されます。
+候補値は定数時間で比較されます。Based on the English source in docs-site/src/content/docs/reference/configuration/server.md Lines 60-62, the intended property is constant-time comparison. As per path instructions: “Check that user-facing docs stay in sync with actual CLI/API behavior and that translated locale pages (ja, ko, ru, zh-cn) are not left contradicting the English source.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 応答とチャット完了では、Codex Direct パススルーの可能性のために `Authorization` を予約しているため、そこでは専用のアドミッション ヘッダーのみが受け入れられます。ダッシュボードで生成された `apiKeys` は、起動後に環境トークンを置き換える可能性があります。候補は一定時間内に比較されます。 | |
| 応答とチャット完了では、Codex Direct パススルーの可能性のために `Authorization` を予約しているため、そこでは専用のアドミッション ヘッダーのみが受け入れられます。ダッシュボードで生成された `apiKeys` は、起動後に環境トークンを置き換える可能性があります。候補値は定数時間で比較されます。 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs-site/src/content/docs/ja/reference/configuration/server.md` at line 56,
Update the Japanese sentence in the configuration documentation so it states
that candidate values are compared in constant time, using wording equivalent
to「候補値は定数時間で比較されます」. Keep the surrounding explanation unchanged and align it
with the English source’s constant-time comparison guarantee.
Source: Path instructions
| for (const branch of [ | ||
| source.slice(countTokensStart, messagesStart), | ||
| source.slice(messagesStart, chatStart), | ||
| ]) { | ||
| expect(branch).toContain("req,\n policy,\n ));"); | ||
| expect(branch).not.toContain("req,\n config,\n ));"); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
Assert handler policy propagation as well as CORS policy propagation.
Lines 70-75 check only the final withCors argument. The test will pass if either handler regresses from policy to config, even though native passthrough then receives the wrong listener policy. Add assertions for the exact handleClaudeCountTokens(..., policy) and handleClaudeMessages(..., policy) calls.
🛡️ Proposed assertions
+ expect(source.slice(countTokensStart, messagesStart)).toContain(
+ "await handleClaudeCountTokens(req, config, policy)",
+ );
+ expect(source.slice(messagesStart, chatStart)).toContain(
+ "await handleClaudeMessages(req, config, logCtx, { requestId, start, turnAdmissionLease }, policy)",
+ );As per path instructions, tests for changed server behavior must provide focused regression coverage. The supplied src/server/claude-messages.ts contract shows that requestPolicy is a separate native-passthrough input.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const branch of [ | |
| source.slice(countTokensStart, messagesStart), | |
| source.slice(messagesStart, chatStart), | |
| ]) { | |
| expect(branch).toContain("req,\n policy,\n ));"); | |
| expect(branch).not.toContain("req,\n config,\n ));"); | |
| expect(source.slice(countTokensStart, messagesStart)).toContain( | |
| "await handleClaudeCountTokens(req, config, policy)", | |
| ); | |
| expect(source.slice(messagesStart, chatStart)).toContain( | |
| "await handleClaudeMessages(req, config, logCtx, { requestId, start, turnAdmissionLease }, policy)", | |
| ); | |
| for (const branch of [ | |
| source.slice(countTokensStart, messagesStart), | |
| source.slice(messagesStart, chatStart), | |
| ]) { | |
| expect(branch).toContain("req,\n policy,\n ));"); | |
| expect(branch).not.toContain("req,\n config,\n ));"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/loopback-listener-admission.test.ts` around lines 70 - 75, Extend the
loopback listener assertions for the two source branches covering
handleClaudeCountTokens and handleClaudeMessages, verifying each exact handler
call passes policy rather than config. Keep the existing withCors assertions so
both handler policy propagation and CORS policy propagation remain covered.
Source: Path instructions
|
Tip For best results, initiate chat on the files or code changes.
A CodeRabbit review was already triggered for this head SHA. No additional review request is needed. 🧠 Learnings usedYou are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
A CodeRabbit review was already triggered for this head SHA. No additional review request is required. 🧠 Learnings usedYou are interacting with an AI system. |
1 similar comment
|
Tip For best results, initiate chat on the files or code changes.
A CodeRabbit review was already triggered for this head SHA. No additional review request is required. 🧠 Learnings usedYou are interacting with an AI system. |
|
Security review complete — LAND. Independent threat-model review plus my own verification of the listener boundary. The admission decision comes from which Verified against the three questions:
Verification on a Linux runner (Bun 1.3.14): red-before 26 pass / 2 fail — the exposed listener previously accepted native passthrough without the dedicated header; at head 70 pass / 0 fail across the passthrough, loopback-admission, and messages-endpoint suites; |
2 participants
Summary
x-opencodex-api-keyadmission header before native Anthropic passthrough on a non-loopback listenerAuthorizationandx-api-keyas upstream credential domains on that branch and strip any exact OpenCodex admission secret before forwardingcount_tokens, preserving loopback behavior and routed Messages compatibilityCoverage includes both safe credential layouts: provider credentials in
Authorizationwith proxy admission elsewhere, and exact proxy admission inAuthorizationwith a real Anthropic credential inx-api-key.Verification
tsc --noEmitpassed under Bun 1.3.14 and Bun 1.4.0-canary.1bun run privacy:scanpassed under both Bun runtimesdocs-site:bun install --frozen-lockfileandbun run buildpassed (265 pages)git diff --checkpassed884e7fe, answered, and resolveddevrebase preserved both validated patch ids; the intervening provider-management commit had no changed-path overlapFocused dual-runtime tests, typecheck, privacy, docs build, diff validation, and independent review were used instead of duplicating the full GitHub CI workload locally.
Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
Bug Fixes
x-opencodex-api-keyheader when applicable.host,content-length, andaccept-encoding.Documentation