test(claude): pin listener policy propagation - #1548
Conversation
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe documentation now specifies constant-time authentication comparison. Tests now verify that both Anthropic route handlers receive the effective listener policy. ChangesListener policy behavior
Estimated code review effort: 1 (Trivial) | ~5 minutes 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 |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
|
Tip For best results, initiate chat on the files or code changes.
I am replying to the latest comment only. The complete earlier comment chain was not available. 🧠 Learnings usedYou are interacting with an AI system. |
54776fc to
ad79520
Compare
|
🧠 Learnings used
|
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Reviewed. The Japanese wording now correctly states constant-time comparison, and the added assertions pin the listener-effective policy argument passed to both Anthropic handlers within the existing listener-policy source contract. No code findings.
|
🧠 Learnings used
|
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== HEAD and changed files =='
git rev-parse HEAD
git diff --name-status HEAD^ HEAD
echo '== Changed diff =='
git diff --check HEAD^ HEAD
git diff --unified=80 HEAD^ HEAD -- \
docs-site/src/content/docs/ja/reference/configuration/server.md \
tests/loopback-listener-admission.test.ts
echo '== Test file outline =='
ast-grep outline tests/loopback-listener-admission.test.ts --items all
echo '== Relevant policy and Anthropic handler references =='
rg -n -C 5 --glob '!node_modules/**' \
'Anthropic|listener[-A-Za-z_ ]*policy|effective[-A-Za-z_ ]*policy|constant[-A-Za-z_ ]*time|timingSafeEqual' \
. --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.mjs' --glob '*.md'
echo '== Japanese documentation context =='
nl -ba docs-site/src/content/docs/ja/reference/configuration/server.md | sed -n '1,260p'Length of output: 191694 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Checked-out commit =='
git rev-parse HEAD
echo '== Parent and changed files =='
git rev-parse HEAD^
git diff --name-status HEAD^ HEAD
echo '== Whitespace validation =='
git diff --check HEAD^ HEAD
echo '== Exact PR diff =='
git diff --unified=100 HEAD^ HEAD -- \
docs-site/src/content/docs/ja/reference/configuration/server.md \
tests/loopback-listener-admission.test.ts
echo '== Changed test file with line numbers =='
sed -n '=' tests/loopback-listener-admission.test.ts | sed 'N;s/\n/\t/'
echo '== Credential comparison implementation =='
rg -n -C 12 --glob 'auth-cors.ts' \
'timingSafeEqual|secretEquals|isDataPlaneAdmissionSecret|apiKeys' src
echo '== Anthropic handler definitions and listener policy call sites =='
rg -n -C 16 --glob '*.ts' \
'handleAnthropic|Anthropic.*Handler|anthropic.*handler|listenerEffective|effectiveListener|listener.*policy' \
src/server tests/loopback-listener-admission.test.ts
echo '== Japanese documentation with line numbers =='
sed -n '=' docs-site/src/content/docs/ja/reference/configuration/server.md | sed 'N;s/\n/\t/'Length of output: 39626 🏁 Script executed: #!/bin/bash
set -euo pipefail
file="src/server/index.ts"
echo '== Route locations =='
grep -nF \
-e 'url.pathname === "/v1/messages/count_tokens"' \
-e 'url.pathname === "/v1/messages"' \
-e 'url.pathname === "/v1/chat/completions"' \
-e 'function handleClaudeCountTokens' \
-e 'function handleClaudeMessages' \
"$file"
echo '== Count-tokens route branch =='
line=$(grep -nF 'url.pathname === "/v1/messages/count_tokens"' "$file" | head -n1 | cut -d: -f1)
sed -n "$((line-20)),$((line+65))p" "$file"
echo '== Messages route branch =='
line=$(grep -nF 'url.pathname === "/v1/messages"' "$file" | head -n1 | cut -d: -f1)
sed -n "$((line-20)),$((line+85))p" "$file"
echo '== Count-tokens handler =='
line=$(grep -nF 'function handleClaudeCountTokens' "$file" | head -n1 | cut -d: -f1)
sed -n "$((line-10)),$((line+180))p" "$file"
echo '== Messages handler =='
line=$(grep -nF 'function handleClaudeMessages' "$file" | head -n1 | cut -d: -f1)
sed -n "$((line-10)),$((line+260))p" "$file"
echo '== CORS-related calls in the handler ranges =='
grep -n -E 'handleClaude(CountTokens|Messages)|withCors|corsHeaders|RequestPolicyView' "$file" | head -n 160Length of output: 10798 🏁 Script executed: #!/bin/bash
set -euo pipefail
file="src/server/index.ts"
echo '== All handler references =='
grep -n -E 'handleClaudeCountTokens|handleClaudeMessages' "$file"
echo '== Declaration contexts =='
for symbol in handleClaudeCountTokens handleClaudeMessages; do
echo "-- ${symbol} --"
grep -n -E "(const|let|var|async)[[:space:]]+${symbol}[[:space:]]*=|${symbol}[[:space:]]*\\(" "$file" \
| while IFS=: read -r line rest; do
start=$((line - 8))
if [ "$start" -lt 1 ]; then start=1; fi
end=$((line + 120))
sed -n "${start},${end}p" "$file"
echo '----'
done
done
echo '== Handler policy parameter and CORS uses =='
grep -n -E 'RequestPolicyView|withCors\(' "$file" | sed -n '1,220p'Length of output: 25136
I found no actionable findings. Verified:
🧠 Learnings used
|
|
🧠 Learnings used
|
|
Tip For best results, initiate chat on the files or code changes.
The current head SHA was already reviewed. No actionable findings were identified. 🧠 Learnings usedYou are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
CodeRabbit already reviewed this head SHA. The review found no actionable findings. 🧠 Learnings usedYou are interacting with an AI system. |
|
Thanks @luvs01, merged. 🙌 This is a useful little guardrail: the new assertions make sure the listener-effective policy is actually propagated into both Anthropic handlers, not just applied later at the CORS layer. That gives us a focused regression test for the exact boundary that could otherwise drift during future server refactors. The Japanese wording fix is also worthwhile because "constant time" is a security property, not merely "within a fixed time". Small diff, but it pins an important auth/policy invariant in a place where regressions would be easy to miss. |
Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
2 participants
Summary
Why
The Japanese wording described a bounded time interval instead of the constant-time comparison guarantee. The existing listener-policy regression also covered final CORS handling but did not independently pin the policy argument passed into the two Anthropic handlers.
Impact
This is a documentation and regression-coverage follow-up only. Runtime behavior is unchanged.
Test plan
bun test --isolate tests/loopback-listener-admission.test.ts(17 pass)bun test --isolate tests/loopback-listener-admission.test.ts(17 pass)git diff --checkReview readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met: