fix(pair): specific error detail for expired vs invalid pairing code#164
Merged
Conversation
The /public/pair/terminal handler collapsed InvalidPairingCode and
PairingCodeExpired into a bare HTTPException(401), so FastAPI emitted a
generic {"detail": "Unauthorized"} and the web terminal could only render
"AxiosError: Received HTTP status 401". The service layer already
distinguishes the two cases; that information was discarded at the API
boundary.
Split the handler and attach a fixed, hand-written detail per case. The
detail strings are hard-coded rather than derived from str(e): the
service-layer messages embed the valid pairing code, and this is an
unauthenticated public endpoint, so forwarding them would hand the valid
code to any caller submitting a wrong one. log.info(e) keeps the verbose
form in the server log.
Both cases stay 401. InvalidPairingCode deliberately covers both "no code
issued yet" and "code does not match" -- telling them apart would be an
oracle for whether a code is currently outstanding.
Drop KeyError from the except tuple: redeem_pairing_code catches KeyError
from database.get_value internally and re-raises it as InvalidPairingCode,
so it was unreachable.
Fix test_pairing_expired_code, which passed the whole pairing-code dict
instead of its "code" field. The dict stringified into the query, so the
test exercised the invalid-code path and never covered expiry at all.
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
2 participants
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.
Closes #156 (backend half — the frontend half is FreeshardBase/web-terminal#38).
Problem
POST /public/pair/terminalcollapsed every pairing failure into a bareHTTPException(401)with nodetail, so FastAPI emitted a generic{"detail": "Unauthorized"}and the web terminal could only renderAxiosError: Received HTTP status 401. The service layer already distinguishesInvalidPairingCodefromPairingCodeExpired; that information was thrown away at the API boundary.Change
Split the handler and attach a fixed
detailper case, both still 401:PairingCodeExpired→ "This pairing code has expired. Generate a new code on your shard and try again."InvalidPairingCode→ "This pairing code is not valid."InvalidPairingCodedeliberately covers both "no code issued yet" and "code does not match". Distinguishing them would be an oracle for whether a code is currently outstanding.The detail strings are hard-coded on purpose
The service-layer exception messages embed the valid pairing code (
f"code ({incoming_code}) does not match existing code ({existing_pairing_code.code})"). This is an unauthenticated public endpoint, so passingstr(e)through would hand the valid code to any caller who submits a wrong one.log.info(e)keeps the verbose form in the server log, where it belongs.Also dropped
KeyErrorfrom theexcepttuple — it was unreachable, sinceredeem_pairing_codecatches it fromdatabase.get_valueand re-raises asInvalidPairingCode.Tests
test_pairing_expired_codepassed the whole pairing-code dict instead of itscodefield. The dict stringified into the query, so the test exercised the invalid-code path and never covered expiry at all. Fixed, and it now asserts the expiry-specific detail.Added
test_pairing_wrong_code_does_not_leak_valid_code: submits a wrong code while a valid one is outstanding and asserts the valid code appears nowhere in the raw response body. This is the check that catches the leak — a correct-code test cannot fail this way.Verified the tests have teeth by reverting the handler and re-running: 3 of 4 fail without the fix.
Server log during the run confirms the verbose message survives:
INFO shard_core.web.public.pair:pair.py:29 issued code (110799) is expiredRecommended reading order
shard_core/web/public/pair.py— the fixtests/test_terminals.py— detail assertions + the leak guardNo
agents.mdupdate: no architecture, config, or convention changed.🤖 Generated with Claude Code