fix: stop sending the API key across redirects (sable-2s6p)

[Rome-1]

Closes sable-2s6p.

The bug

requests' SessionRedirectMixin.rebuild_auth strips Authorization on a host change and leaves arbitrary custom headers intact. axios/follow-redirects does the same. So x-api-key rides a 302 to whatever host it points at.

This is pre-existing and was not introduced by #220. The retry loop added there raises exposure from one transmission to as many as five, which is why it matters more today than it did last week — but it is not the cause. Worth being explicit about, so nobody reading the diff concludes the retry fix created a security bug.

The composite action was never affected. Verified rather than assumed: none of the eight curl invocations in github-action/action.yml carries -L or --location, so curl never followed a redirect there. That bounds the blast radius to the two CLI runtimes.

The fix

Every authenticated call site, not just the poll path I was looking at:

• Node — 14 sites now go through a shared apiClient (axios.create({ maxRedirects: 0 })).
• Python — 13 sites go through api_get/api_post, which force allow_redirects=False. Forced, not defaulted, so no call site can opt back in; there's a test for exactly that.

Deliberately left on plain axios/urllib: update-checker.ts's npm registry call, and the Slack/Discord webhook posts in notify.ts/notify.py. None carries the Rafter key. Each was checked individually rather than by filename.

What the security review caught — this would have shipped a broken CLI

API/API_BASE end in /, and 11 call sites concatenated /static/..., building https://rafter.so/api//static/scan. Production answers that with a 308 to the single-slash form:

GET https://rafter.so/api//static/usage  ->  HTTP 308, location: /api/static/usage
GET https://rafter.so/api/static/usage   ->  HTTP 400  (endpoint reached)

It worked only because the client followed the redirect. Refusing redirects turned rafter run, get, usage, notify --scan-id and issues from-scan into hard failures in both runtimes. Every existing test mocks the transport, so nothing caught it.

Both runtimes now build URLs through apiUrl() / api_url(), and a test in each fails on any ${API}/ or {API_BASE}/ construction. The latent bug here is worth naming: the CLI has been depending on a redirect to reach its own API.

Also from that review:

• The error message named a flag that doesn't exist. It told users to point --rafter-url at the final URL; that's a GitHub Action input, not a CLI flag. Removed rather than shipping advice nobody can follow.
• A redirect Location is attacker-controlled if the endpoint is. Header values can't carry CR/LF but ESC is legal, so the raw value could emit ANSI sequences that rewrite the user's terminal. Both runtimes strip non-printables and cap at 200 chars; the fixtures assert it with a real escape sequence.
• The source-scanning guards only caught the most literal bypass. They now also match the .request() form and flag any second axios.create() / requests.Session() built outside the api utils.
• The Node test shim made axios and apiClient the same mock, so a regression to bare axios would still have passed. create() now returns a distinct object and the tests watch that instance.

Verification

• Guards mutation-tested in both runtimes: flipping maxRedirects to 5, defaulting allow_redirects back to True, and reverting one call site to bare axios each produce failures.
• Affected suites pass: 100 Node tests across the 6 files that mock the HTTP layer, 168 Python tests across the 8 affected files, plus the new 6 + 8 redirect tests.
• pnpm run build clean.
• The double-slash 308 was confirmed against the live API before and after.

Note on test churn

Five Node test files and five Python test files change their mocking seam, because the code now calls apiClient/api_get rather than axios.get/requests.get. That is the point — the tests now mock what the code actually calls, so bypassing the safe client shows up as a failure.

[Rome-1]

Worth recording what just happened on this PR, because it is #221's argument making itself.

This change rewrites the HTTP layer of both clients — 27 call sites across node/src and python/rafter_cli, plus the mocking seam in ten test files. On open, the checks were:

skipping - test-node
skipping - test-python
skipping - cross-platform
skipping - backend-api
skipping - e2e-node
skipping - package-integrity
skipping - sarif-validation
skipping - secret-detection-accuracy
pass - gate

Nothing substantive ran. The only green was the job that decides whether to run the others.

I dispatched test-comprehensive.yml by hand against the branch to get a real result — all 14 jobs green, including test-node, test-python, and the full 6-way cross-platform matrix. That manual dispatch is the workaround, and needing it is the finding: the gate's premise is that internal work is verified before it is pushed, but the verification it is standing in for is something a human has to remember to trigger.

With #221 merged, test-node and test-python would have run here automatically and this comment would not exist.

Not arguing for reverting the gate wholesale — #219's cost case is still sound and the expensive cross-platform grid should stay gated. Just noting that the second PR in a row to touch both clients had zero client-test coverage until someone intervened by hand.