Arm the served-identity fence, and stop treating an absent claim as a mismatch - #157
Conversation
Ship served-identity redaction with the snapshot because the widening introduces the fields it scrubs.
Two serving-path gates turned an absent `chatgpt_account_id` claim into a refusal. The vault derives its own `account_id` from that same claim, so a token without it fails both gates at once, taking the account dark with no local credential to fall back on — and a served credential.get writes no audit row, so the plugin is the only witness to the resulting outage. Absence proves neither the right identity nor the wrong one. Only a claim that positively disagrees now refuses; absence serves and warns. The bind path in reconcileFallbackCustody keeps refusing nullClaim. There absence means there is nothing to record, and binding an identity nobody asserted is worse than refusing to bind.
The previous commit's condition read `servedAccountId !== expectedAccountId` without first checking that a served id exists. An absent id compares unequal to any bound id, so absence still returned identity_mismatch and never reached the serve-and-warn branch below it — the exact defect that commit set out to remove, preserved in the arm above the fix. It passed every gate because all existing runtime tests inject resolveFallbackVaultState, which returns before this gate executes. The reason the code was wrong and the reason nothing caught it are the same fact.
|
Merged as The loosening is the part that needed independent checking, since "serve when we cannot verify" is the kind of change that reads as a weakened fence. I checked the claim rather than the reasoning: Mutation-verified both halves myself:
One thing your proof 5 does not establishYou cite
Mutating either alone leaves the test green: Not a defect — the property is defended twice, which is stronger than claimed. But it is worth knowing, because the failure mode is specific: someone reads your comment, sees the first guard, tries the obvious simplification of "the serving path tolerates absence, so this arm is inconsistent", finds the suite green, and removes it. What remains is the deeper guard, which is correct but says nothing about why it exists, and the asymmetry loses the explanation that makes it survivable. A test asserting the first guard specifically — or a note that the second one is the backstop rather than the mechanism — would close that. Your call whether it is worth a follow-up; I did not want to add it under your authorship without asking. The restThe self-caught bug is the one worth keeping in the record: your first runtime fix compared Redaction landing in the same commit as the snapshot is correct and I checked it holds: I did briefly think this branch reverted the RPC and empty-bearer work, because |
|
Followed my own note up rather than leaving it with you — the branch is merged, so it is mine now. It turned out stronger than I wrote. I said the second refusal was a backstop; it is not reachable at all. Both derive the same value from the same token — I proved it rather than reading it: made the branch throw, ran the whole suite, zero hits across 1359 tests. That is why mutating either guard alone left your test green, and it is worse than the redundancy I described. An unreachable guard reads as a safety net, so the real one looks removable — narrow the first and everything stays green, because the decoy appears to catch what you just stopped catching. The verify/bind asymmetry is subtle enough without something that looks like it is also defending it. The refusal is now singular, at Gate: core 147, opencode 1359, pi 14, typecheck and biome clean. |
Re-vendors the Claustrum client at its widening merge, arms the identity fence that has been skipped waiting for it, and fixes two serving-path gates that turned a missing identity claim into a refusal.
Why the fence was skipped
verifyServedFallbackIdentityhas always had a branch comparing the vault's asserted account against the local binding — the check that catches a handle bound to the wrong ChatGPT account. It wasit.skipped because our vendoredServedCredentialdid not decodeaccountId, though the wire had been sending it for months. The client widened;ServedCredentialnow carriesaccountId,email,orgName,projectId,credentialId, all optional.Vendored from merge commit
1828f35, byte-compared file by file.emailandorgNameare added to logger redaction in the same commit as the snapshot — a widening that lands without its redaction opens a window where the field exists and nothing scrubs it. Our logger is a scrub-list rather than an allowlist, so every upstream field addition is a new obligation here; I checked for any call logging a whole credential object wholesale and found none.The defect this turned up
Arming the fence reddened four fixtures whose served credentials carry no account id. The tempting read is fixture debt. It was not:
account_idis best-effort, not guaranteed, including forchatgpt:openai. It is derived fromchatgpt_account_id_from_jwt(access_token)and falls back to a stored identity that is itself optional — four reachable routes to absent, three pinned by the vault's own tests. The fixtures encoded the wire's real shape correctly.Two serving-path gates treated that absence as a mismatch:
resolveFallbackAccesscollapsed every non-ok verdict intoCUSTODY_REFUSE. The function already returnednullClaimandidentityMismatchas distinct reasons — the consumer flattened them one frame up, which is why the function reads as correct.identity_mismatchwhen no id was served.Both matter because the vault derives its
account_idfrom the same JWT claim, so a token without it fails both gates at the same instant — one trigger, two detonators. Under custody there is no local credential to fall back on, and a servedcredential.getwrites no audit row, so the plugin is the only witness to the resulting outage. Absence proves neither the right identity nor the wrong one. Only a claim that positively disagrees now refuses; absence serves and warns with the credential id.What deliberately did not change
reconcileFallbackCustodycalls the same verify function and still refusesnullClaim. It is a bind path: it decides what identity to record, and absence there means there is nothing to bind. Recording an identity nobody asserted is worse than refusing.On a verify path absence is tolerable; on a bind path absence is disqualifying. Applying one rule uniformly would have taught enrollment to mint bindings from nothing.
That asymmetry is pinned by a test (proof 5 below) rather than a comment. Without it, the next reader sees two sites tolerating absence and one refusing, reads inconsistency, and fixes the correct one into a defect.
Proofs
Five mutations, each reverting one change alone:
One thing worth flagging, because it says something about the test coverage here. My first version of the runtime fix was wrong: the condition read
servedAccountId !== expectedAccountIdwithout first checking a served id exists, so absence compared unequal to any bound id and still refused — the defect preserved in the arm directly above the fix, under a comment saying only a positive disagreement should refuse.It passed all five gates. Every existing runtime test injects
resolveFallbackVaultState, which returns before that gate executes, so nothing in the repo had ever run that line. The reason the code was wrong and the reason nothing caught it were the same fact. Proof 4 reaches the gate throughcacheConnectorinstead of the override, and reddens against that original condition.Gates from the repo root:
build,format:check,lint,types,testall pass.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Re-vendors
@cortexkit/claustrum-clientat merge1828f35, arms the served-identity fence that wasit.skipped waiting for the wire to carryaccountId, and makes the two serving-path gates serve when the vault omits an identity claim instead of refusing. Old behavior refused on absentaccountId; new behavior refuses only on a positively disagreeing claim and warns otherwise.Vendoring
ServedCredentialnow decodes optionalaccountId,email,orgName,projectId, andcredentialId; a new vendoredhandles.tsships with the snapshot.emailandorgName/organizationNamefields so they never reach a log file.Serving-path behavior
resolveFallbackAccessno longer flattens anullClaimintoCUSTODY_REFUSE; it serves and logs a warning with the credential id.serveswhen no id is served, instead ofidentity_mismatch.reconcileFallbackCustodystill refusesnullClaim: on its bind path absence means there is nothing to record, so serving would mint an unasserted binding.Written for commit 15fdf0b. Summary will update on new commits.