Skip to content

Arm the served-identity fence, and stop treating an absent claim as a mismatch - #157

Merged
ualtinok merged 10 commits into
cortexkit:mainfrom
iceteaSA:feat/revendor-identity-fence
Sep 18, 2026
Merged

ualtinok merged 10 commits into
cortexkit:mainfrom
iceteaSA:feat/revendor-identity-fence

Conversation

@iceteaSA

@iceteaSA iceteaSA commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

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

verifyServedFallbackIdentity has 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 was it.skipped because our vendored ServedCredential did not decode accountId, though the wire had been sending it for months. The client widened; ServedCredential now carries accountId, email, orgName, projectId, credentialId, all optional.

Vendored from merge commit 1828f35, byte-compared file by file. email and orgName are 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_id is best-effort, not guaranteed, including for chatgpt:openai. It is derived from chatgpt_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:

  • resolveFallbackAccess collapsed every non-ok verdict into CUSTODY_REFUSE. The function already returned nullClaim and identityMismatch as distinct reasons — the consumer flattened them one frame up, which is why the function reads as correct.
  • the runtime's vault-state gate returned identity_mismatch when no id was served.

Both matter because the vault derives its account_id from 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 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 with the credential id.

What deliberately did not change

reconcileFallbackCustody calls the same verify function and still refuses nullClaim. 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:

(fail) refuses a served vault credential whose asserted account differs from its bound identity
(fail) serves a vault credential whose account claim is absent
(fail) keeps a corrupt fallback marker when the served credential asserts a different account
(fail) installs an unasserted corrupt fallback credential and logs only that absence
(fail) does not tombstone an enrolling account when the served credential omits its identity claim

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 !== expectedAccountId without 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 through cacheConnector instead of the override, and reddens against that original condition.

Gates from the repo root: build, format:check, lint, types, test all pass.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Re-vendors @cortexkit/claustrum-client at merge 1828f35, arms the served-identity fence that was it.skipped waiting for the wire to carry accountId, and makes the two serving-path gates serve when the vault omits an identity claim instead of refusing. Old behavior refused on absent accountId; new behavior refuses only on a positively disagreeing claim and warns otherwise.

Vendoring

  • ServedCredential now decodes optional accountId, email, orgName, projectId, and credentialId; a new vendored handles.ts ships with the snapshot.
  • Logger redaction covers the newly surfaced email and orgName/organizationName fields so they never reach a log file.

Serving-path behavior

  • resolveFallbackAccess no longer flattens a nullClaim into CUSTODY_REFUSE; it serves and logs a warning with the credential id.
  • The runtime vault-state gate returns serves when no id is served, instead of identity_mismatch.
  • reconcileFallbackCustody still refuses nullClaim: 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.

Review in cubic

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.
@ualtinok
ualtinok merged commit f0e03c9 into cortexkit:main Sep 18, 2026
5 checks passed
@ualtinok

Copy link
Copy Markdown
Contributor

Merged as f0e03c9. Gate on the merge: core 147, opencode 1359, pi 14, typecheck and biome clean.

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: ServedCredential.accountId is ?: on the vendored wire type, and the doc comment describes it as the identity the token executes under rather than anything guaranteed. Absence is a wire state, not a signal. Refusing on it converts the vault's silence into an outage that only we can see, which is a bad trade against a check that proves nothing in that branch.

Mutation-verified both halves myself:

  • removing the mismatch fence reddens refuses a served vault credential whose asserted account differs from its bound identity — the security half still bites
  • restoring the refusal on absence reddens three, including serves a vault credential whose account claim is absent

One thing your proof 5 does not establish

You cite does not tombstone an enrolling account when the served credential omits its identity claim as pinning the verify/bind asymmetry. It pins the property, and the property holds — but it does not pin the guard you attribute it to.

reconcileFallbackCustody refuses absence twice, independently. The identity.reason !== 'ok' branch at custody.ts:869 is the one your comment at :353-355 points at. Below it at :888, after re-parsing the JWT, there is a second if (!servedAccountId) returning the same nullClaim outcome.

Mutating either alone leaves the test green:

# first guard narrowed to identityMismatch only
(pass) does not tombstone an enrolling account when the served credential omits its identity claim

# second guard disabled
(pass) does not tombstone an enrolling account when the served credential omits its identity claim

# both
(fail) does not tombstone an enrolling account when the served credential omits its identity claim

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 rest

The self-caught bug is the one worth keeping in the record: your first runtime fix compared servedAccountId !== expectedAccountId without checking a served id exists, which preserved the exact defect it sat under a comment describing. It passed all five gates because every existing runtime test injects resolveFallbackVaultState and returns before that line, so nothing in the repo had ever executed it. "The reason the code was wrong and the reason nothing caught it were the same fact" is the right diagnosis, and reaching the gate through cacheConnector instead is the right fix — that is the third time on this feature that the missing coverage was a test entering through the real door.

Redaction landing in the same commit as the snapshot is correct and I checked it holds: email and orgname are in the scrub list in 96bbd22, alongside the wire change that introduced them. A scrub-list logger makes every upstream field addition an obligation here, so coupling them in one commit is the only shape where the window cannot open.

I did briefly think this branch reverted the RPC and empty-bearer work, because git diff main pr157 shows their lines as deletions. That is an artifact of comparing tips across a branch based on 8ed3ef2 rather than a real revert — the branch's own diff touches eleven files, all vendor and custody, and git merge-tree reports no conflicts. Mentioning it because your rr-cache incident makes the opposite mistake expensive, and I would rather say I checked and it was fine than leave you guessing whether I did.

@ualtinok

Copy link
Copy Markdown
Contributor

Followed my own note up rather than leaving it with you — the branch is merged, so it is mine now. 919fd2f.

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 — verifyServedFallbackIdentity does parseJwtClaims then extractAccountIdFromClaims on served.payload.access and returns nullClaim when the claim is missing, and :888 recomputed exactly that and checked it again. The first returns first on precisely the input that would trigger the second, so no input can enter it.

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 :869 beside the comment explaining why a bind path refuses what a verify path tolerates, and mutating that one site reddens your test. Which makes proof 5 pin the guard after all — it just needed the decoy gone rather than another test.

Gate: core 147, opencode 1359, pi 14, typecheck and biome clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants