fix(auth): throttle a failed exchange, never one that would succeed (#802) - #806
Merged
Merged
Conversation
davidfarah2003
force-pushed
the
fix/802-throttle-after-credential
branch
from
August 24, 2026 00:55
4ceea93 to
18e5548
Compare
The refused-exchange budget was enforced before readJsonBody, so a full bucket refused every request from that peer key - valid credentials included. On the public face the default peer key is the socket address, and run-a-mesh.md tells the operator to front the listener with a reverse proxy while framing --exchange-trusted-proxy as an optional hazard. Follow that guidance without the flag and every client collapses into one bucket, so thirty unauthenticated garbage POSTs deny the public mint path for a rolling minute, holders of valid IdP JWTs and actor tokens included. Loopback is unaffected, so local health looks fine while the published face is down. Throttling exists to slow probing, and a valid credential is not probing. The gate is still evaluated up front, but enforced only where a credential actually failed: a throttled peer with a good credential mints, and a throttled peer with a bad one is answered 429 instead of the specific reason, because the reason is what makes probing cheap. Both halves are proven red-first against a built defect: enforcing before the body turns the valid-token cell red (49/1), and stripping the 429 short-circuit turns the reason-withholding cell red (47/3). Fixed: 50/0.
davidfarah2003
force-pushed
the
fix/802-throttle-after-credential
branch
from
August 24, 2026 01:25
18e5548 to
fd8aaa5
Compare
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.
What
The public exchange face no longer refuses a request that would have succeeded.
The defect
implementations/auth/src/service.tsenforced the refused-exchange throttle before reading the body, so it fired before any credential existed to evaluate:The default peer key on the public face is the socket address, and
docs/run-a-mesh.mdtells the operator to front the listener with a reverse proxy while framing--exchange-trusted-proxyas an optional hazard. Follow that guidance without the flag and every client collapses into one bucket, so 30 unauthenticated garbage POSTs deny the public mint path for a rolling minute — holders of valid IdP JWTs and actor tokens included. Loopback is unaffected, so local health looks fine while the published face is down.Not a mint bypass: no token is issued that should not be. This is availability of the public identity plane, in the topology the docs recommend.
The fix
Throttling exists to slow probing, and a valid credential is not probing. The gate is still evaluated up front (cheap, prunes the window) but enforced only where a credential actually failed:
Nothing that would have succeeded is refused, and the budget still does its real job.
Proof, red-first against a built defect
Both halves fail independently, so both were proven independently:
readJsonBody(the shipped defect)That second mutation matters: without that assertion, "fixing" the DoS would silently gut the throttle, since a throttled peer would leak the refusal reason and probing becomes cheap again.
Note the proof required rebuilding between mutation and run — this suite imports the built
@cotal-ai/auth, so a source-only mutation is invisible and grades everything green. My first attempt made exactly that mistake and reported false passes.Coverage this closes
Smoke G previously ran only with
--exchange-trusted-proxy, so it asked whether a throttled peer is refused and whether other peers are isolated — never whether a legitimate caller behind that same key still works. That is why 47 green cells sat on top of an unauthenticated DoS. Two new cells cover it.Partially addresses #799 (the suite's missing observed-failure evidence) for these cells specifically.
Closes #802.
Base retargeted to
main. This PR was opened againstfeat/remote-exchange(old base tipb91796c8, previous head4ceea934). That branch is a strict ancestor ofmainwith zero unique commits and sat 202 commits behind it, so grading this change against it measured a snapshot that no longer exists. The single commit was rebased ontomainunchanged: the diff is still the same 3 files (+67/-3), and every added/removed line is byte-for-byte identical to the pre-rebase diff — only three hunk headers inservice.tsshifted by 8 lines.Also fixes the tail count this PR should have carried: it adds 3 cells to section G, so
EXPECTEDgoes 47 → 50.pnpm smoke:remote-exchange:livereported50 passed, 0 failed.