[cliffhall]

Closes #2734

Problem

#2731 carries a client credential from a superseded namespace only where the state file still holds the client the ledger recorded as its owner. But a value reaches the keychain without the ledger seeing it. The file-to-keychain hand-off keeps an existing keychain value and copies a file's value otherwise, so per-location ledger records can't say which value is there. All three cases on the issue come from that gap:

• Repeated hand-off (issue body): A is absorbed, the file re-registers B under the same namespace, and the next hand-off keeps A's value. The file record says B, so both the pending read and the carry pair A's secret with B.
• Cleanup index read as a keychain write (first comment): the keychain record lists the server with no owner, foldOwners discards the file's valid owner, and the only secret is purged without being carried.
• Several secrets.json files (first comment): effectiveOwners merges file: records in insertion order and picks the wrong file's owner.

Fix

The hand-off now carries each value's owner along with the value, as the issue comment proposes. Instead of teaching the hand-off about state files and ledgers (it knows neither), the owner is stored next to the value in the secret store:

• Each DCR client_secret / registration token gets an owner stamp in a separate …#owner field. The stamp holds the client_id it was written for and an HMAC-SHA256 over that client_id keyed by the value itself, so the secret is never the hashed input (CodeQL js/insufficient-password-hash) (clientCredentialStampField, clientCredentialStamp, stampedOwner in oauth-secrets.ts). splitServerOAuthState emits it, and serverSecretFields lists it, so the save path's set, delete and rollback handling covers stamps unchanged. Pre-registered clients get no stamp, for the reason clientCredentialOwners already gives.
• The hand-off copies or keeps the stamp exactly as it does the value, so the stamp names the client of the value actually there. The MAC rejects a stamp that got separated from its value, or whose client_id was swapped: a kept value next to a copied stamp, or a value an older Inspector rewrote without its stamp. absorbFileSecretsIntoKeyring itself is unchanged.
• Carries trust only the stamp. That covers both the pending read in readEntrySecrets and carryOwed. A field is carried only when its stamp vouches for the value and names the client the state file holds now, and the stamp is copied with it. The ledger's owners now only say which fields to look in (candidateFields, a union over the covered records). foldOwners/effectiveOwners are gone.
• Adoption restamps (restampClientCredentials) the values it moves. Legacy-id values predate stamps, and a fallback's stamp may belong to a value read from elsewhere. A stamp that vouches for its value is kept; otherwise the file's own pairing is stamped, the same assumption fix(auth): carry a superseded namespace's client secret across a backend switch #2731 made at adoption. This keeps adopted legacy secrets carriable.

The ledger format is unchanged. #2731's owner records are in no release yet, so nothing needs migrating.

A value with no stamp that vouches for it is purged as before, not guessed at: either it was written before this change or it no longer sits beside its own stamp. Such a value had no carry before #2731 either, and re-authorizing recovers.

Tests

• oauth-namespace-ledger.test.ts: the three cases from the issue at ledger level, plus stamp rejection (no stamp, malformed, not an object, no client, another value's stamp). The A repeated secrets.json hand-off can pair a carried client secret with the wrong client #2734 cases fail on the old owner logic.
• oauth-secrets.test.ts: stamps from the split (DCR yes, pre-registered no, none without a client_id), stampedOwner rejections, restampClientCredentials.
• oauth-secrets-namespace.test.ts (integration): the issue's repro end to end through two real absorbFileSecretsIntoKeyring calls (fails on v2/main, passes here), the matching positive case with no re-registration, and a legacy secret adopted, stripped and re-adopted elsewhere that is still carried.

docs/secret-storage.md now describes the stamp in place of the ledger-recorded owner.

npm run format and npm run local:gate pass. No UI change, so no screenshots.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

A failed stamp write leaves a partial carry that retries skip, risking credential loss during later re-adoption.

1 open issue

• Prevent unstamped destination values when stamp writes fail · New

What changed in this PR

Fixes OAuth credential ownership across secret-store hand-offs by storing value-bound owner stamps beside credentials.

Changes:

• Validates owner stamps before carrying credentials between namespaces.
• Restamps adopted credentials and replaces ledger-owner inference with candidate-field lookup.
• Adds regression coverage and updates storage documentation.

File	Description
docs/​secret-storage.md	Documents stamp-based ownership and carry behavior.
core/​auth/​node/​oauth-secrets.ts	Adds stamp creation, validation, and restamping.
core/​auth/​node/​oauth-persist-file.ts	Applies stamps during adoption and pending reads.
core/​auth/​node/​oauth-namespace-ledger.ts	Validates and copies stamps during carries.
clients/​web/​src/​test/​integration/​auth/​node/​oauth-secrets-namespace.test.ts	Covers repeated hand-offs and legacy adoption.
clients/​web/​src/​test/​core/​auth/​oauth-secrets.test.ts	Tests stamp helpers and credential splitting.
clients/​web/​src/​test/​core/​auth/​oauth-namespace-ledger.test.ts	Tests stamp-based carry decisions.

_{Review effort: Balanced}

[cliffhall]

Copilot round 1 (1 comment, no suppressed comments)

• Prevent unstamped destination values when stamp writes fail — fixed in 9285a96: the carry writes the stamp before the value, so either failure leaves the field vacant and retryable; regression added for each write failing in turn. npm run local:gate green.

[Copilot]

🟢 Approval recommended

No blocking issues remain, and targeted regressions cover ownership conflicts and the corrected write-order failure.

No open issues 🎉

1 resolved since last review

• Prevent unstamped destination values when stamp writes fail

_{Review effort: Balanced}

[cliffhall]

Copilot review loop closed. Round 2 was clean — no inline comments, no suppressed comments, and the round-1 finding marked resolved — so the loop stops on its first clean round.

[cliffhall]

CodeQL js/insufficient-password-hash (#82 → #83): HMAC kept; #83 to be dismissed as a false positive

#82 flagged the first version of the owner stamp, a plain SHA-256 of the client secret. 15de439 replaced it with an HMAC-SHA256 over the client_id, keyed by the credential. #82 is now fixed, and CodeQL raised #83 under the same rule. The rule treats any fast derivation from the secret as a password hash, the HMAC key included. Only a slow KDF (scrypt/pbkdf2/bcrypt/argon2) or no derivation at all would clear it.

Why the rule's threat doesn't apply here:

• The stamp is stored exactly where the secret is. It's a …#owner account in the same secret store (the OS keychain, or the same secrets.json, under the same encryption) as the credential it vouches for. An offline guessing attack needs the stamp. Anyone who can read the stamp can read the credential itself, so the stamp gives away nothing the store doesn't already.
• The values aren't human-chosen passwords. They are DCR-issued client_secrets and RFC 7592 registration tokens: high-entropy and server-generated. Pre-registered, user-entered secrets are never stamped (stamped: false in PREREG_CLIENT_INFO_FIELDS).
• A slow KDF would only satisfy the scanner. It would add a key derivation to every save and every carry, for each stored client credential, and buy no protection. The alternative with no derivation at all would store a second copy of each secret.

So the HMAC stays, and #83 should be dismissed as false positive with a pointer to this comment.

[Copilot]

🔵 Needs a closer look

Cross-backend credential migration combined with destructive namespace cleanup warrants final human validation.

No open issues 🎉

_{Review effort: Balanced}

[cliffhall]

Copilot review loop closed (again). Round 3 reviewed the HMAC change (15de439) and was clean: no inline comments and no suppressed comments. Its headline ("warrants final human validation") names no specific defect, so it is a pointer for the maintainer review rather than a finding. CodeQL #82 is fixed and #83 was dismissed as a false positive (see the comment above).