fix(claude): contain main credentials to canonical forward routes - #1358
Conversation
📝 WalkthroughWalkthroughClaude Messages replays now enable credential stripping for noncanonical OpenAI Responses forwards. Responses handling clears native authentication and Codex routing metadata. Tests cover direct custom forwarding and shadow-call rerouting. ChangesClaude forward authentication
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ClaudeMessagesReplay
participant HandleResponses
participant AuthContext
participant CustomOpenAIResponsesRoute
ClaudeMessagesReplay->>HandleResponses: enable credential stripping
HandleResponses->>AuthContext: reset auth and remove native headers and routing metadata
HandleResponses->>CustomOpenAIResponsesRoute: forward request without main credentials
CustomOpenAIResponsesRoute-->>ClaudeMessagesReplay: return streamed response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/claude-messages-endpoint.test.ts`:
- Line 693: Remove the duplicate captured declarations within the test()
callbacks so each callback has only one declaration. Apply this at
tests/claude-messages-endpoint.test.ts lines 693-693 and 745-745, retaining the
single declaration needed by each test.
🪄 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: 018ae3a4-151b-40e8-a848-2e65bd069496
📒 Files selected for processing (3)
src/server/claude-messages.tssrc/server/responses/core.tstests/claude-messages-endpoint.test.ts
|
Please rebase this PR onto the latest |
cb1cb39 to
9eaae1b
Compare
Rate Limit Exceeded
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="lidge-jun/opencodex"
pr_number="1358"
required_dev_sha="e8ce2b93dd3ef8074d96dee407704c5dc06e2699"
echo "== Pull request metadata =="
gh pr view "$pr_number" --repo "$repo" \
--json number,state,isDraft,headRefName,headRefOid,baseRefName,baseRefOid,mergeStateStatus,statusCheckRollup \
--jq '{
number,
state,
isDraft,
headRefName,
headRefOid,
baseRefName,
baseRefOid,
mergeStateStatus,
checks: [.statusCheckRollup[]? | {
name,
status,
conclusion,
workflowName,
startedAt,
completedAt,
detailsUrl
}]
}'
echo
echo "== Required dev commit =="
gh api "repos/$repo/commits/$required_dev_sha" \
--jq '{sha, commit: {message: .commit.message, committer: .commit.committer}}'
echo
echo "== Is the required dev commit an ancestor of the exact PR head? =="
head_sha="$(gh pr view "$pr_number" --repo "$repo" --json headRefOid --jq '.headRefOid')"
comparison="$(gh api "repos/$repo/compare/$required_dev_sha...$head_sha")"
printf '%s\n' "$comparison" | jq '{
status,
ahead_by,
behind_by,
merge_base_commit: .merge_base_commit.sha
}'
echo
echo "== Check runs attached to the exact PR head =="
gh api "repos/$repo/commits/$head_sha/check-runs?per_page=100" \
--jq '[.check_runs[] | {
name,
status,
conclusion,
started_at,
completed_at,
details_url
}]'Length of output: 19838 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="lidge-jun/opencodex"
pr_number="1358"
head_sha="$(gh pr view "$pr_number" --repo "$repo" --json headRefOid --jq '.headRefOid')"
current_dev_sha="$(gh api "repos/$repo/git/ref/heads/dev" --jq '.object.sha')"
echo "== Current dev versus PR head =="
echo "current_dev_sha=$current_dev_sha"
echo "pr_head_sha=$head_sha"
gh api "repos/$repo/compare/$current_dev_sha...$head_sha" \
--jq '{
status,
ahead_by,
behind_by,
merge_base_commit: .merge_base_commit.sha
}'Length of output: 907
The PR meets the stated rebase and exact-head CI requirements. You are interacting with an AI system. |
|
Thanks @luvs01 — really appreciate the careful credential-boundary hardening here, especially covering final-route overrides and shadow rerouting with regressions. Merged! 🙏 |
2 participants
Summary
openai-responsesforward destination.This keeps existing key/OAuth adapters and sidecar behavior intact while preventing an internal main-account credential from following a final custom forward route.
Verification
bun test tests/claude-messages-endpoint.test.ts -t 'custom forward|shadow-call'— 2 passed.bun run typecheckbun run privacy:scangit diff --checkEBUSYreproduced on an unchangeddevcontrol. Exact-head GitHub CI remains required.Checklist
Review readiness checklist
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.