Repository navigation
security: never follow a redirect on a token-bearing request (v0.11.1) - #14
Merged
Merged
Conversation
…uest Finding: F-01 (audit, Medium) / CR-03 (CodeRabbit, #13) from the operator-panel-2026-09-p2-queue-read-api audit. fetch follows redirects by default and Undici keeps X-Task-Queue-Token across the hop, so a redirect from the configured base would deliver the token to the Location's origin. send() now passes redirect: 'manual' and refuses any 3xx or opaque-redirect response as a 502. A two-server loopback test fails on v0.11.0 (the token reaches the redirect target) and passes here. v0.11.1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fetchfollows redirects by default, and Undici keeps custom headers across the hop. A redirect from the configuredTASK_QUEUE_APIwould therefore have deliveredX-Task-Queue-Token(read + operator-write) to whatever origin theLocationnamed.insecureApiBase()only vets the configured base, not where a response points next.send()now passesredirect: 'manual', and any 3xx oropaqueredirectresponse is refused as a 502 without reading its body. task-queue-mcp never redirects, so there is no behaviour change in normal use.Deliberately not done
src/gates/*.ts) are unchanged. They fetch public raw files with no credential, and already carry a reviewedSECURITY[accepted]for following redirects.Look hardest at
src/control-api.tssend(): the redirect check has to cover both the spec'sopaqueredirect(status 0) and Undici's real 3xx.src/tests/control-api.test.ts, "a real redirect to another origin never delivers the token there": two real loopback servers. It fails against v0.11.0.Test plan
npm run build,npm test(204 pass)redirect: manualtest both fail with the fix reverted🤖 Generated with Claude Code