You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
A repeated secrets.json hand-off can pair a carried client secret with the wrong client #2734
Follow-up to #2672 (PR #2731, raised in its fourth Copilot review round and declined there as out of scope).
#2731 carries a client credential from a superseded namespace on another backend only where the state file still holds the client the ledger recorded as the credential's owner. In a keychain run, for a server the keychain record was never written for, the owner is taken from a covered secrets.json record (effectiveOwners / foldOwners in core/auth/node/oauth-namespace-ledger.ts). The assumption is that a keychain value there can only have come from that file through the file-to-keychain hand-off.
A repeated hand-off breaks that assumption:
A file-backed save stores client A's secret under namespace ns1.
absorbFileSecretsIntoKeyring copies it into the keychain, and no keychain ledger record is created.
A later file-backed save registers client B under ns1, so the file record's owner becomes B.
The next hand-off keeps A's existing keychain value (secret-store-selection.ts, which keeps an existing keychain field).
The stamp is stripped and the file is re-adopted on a memory backend. The keychain run then labels A's secret as B's, so both the pending read and the carry pair it with client B.
Expected
Ownership should follow the value the hand-off actually copied or kept. For example, the hand-off could record the owner of each value it copies into the ledger. Alternatively, a carry with no trustworthy owner could be skipped, but that also drops the recovery of absorbed copies that #2731 added.
Narrower than #2672: it needs a repeated hand-off with a re-registration between them, then a strip and a cross-backend re-adoption.
Severity 2 — a wrong client/secret pairing is rejected by the AS and re-authorizing recovers; it needs a repeated hand-off with a re-registration between them, then a strip and a cross-backend re-adoption
Urgency 1 — nothing waits on it
Bonuses: +1 bug label, +1 milestoned (filed through the create flow)
Two more cases of the same class, from #2731's fifth Copilot round. Both come from the hand-off not recording which value it copied or kept:
The keychain's cleanup index treated as proof of a keychain write. A save records every merged server in the ledger but owners only for the entries it writes. After a file-backed secret is absorbed into an empty keychain, an unrelated keychain save lists the server in the keychain record with no owner. foldOwners then discards the file's valid owner, so a later strip, a memory re-adoption, and a keychain run purge the only secret without carrying it.
Several secrets.json files. Client A is saved to F1 and B to F2 under one namespace. A keychain run configured for F1 absorbs A's secret, but effectiveOwners merges the file: records in insertion order and picks F2's owner B.
The fix proposed above covers both: the hand-off records the owner of each value it actually copies or keeps, and any owner it can't establish is treated as ambiguous and never carried.
Activity
cliffhall commentedon Oct 20, 2026
Triage: Priority Low (total 5)
buglabel, +1 milestoned (filed through the create flow)Board: #28, Status
Todo.cliffhall commentedon Oct 20, 2026
Two more cases of the same class, from #2731's fifth Copilot round. Both come from the hand-off not recording which value it copied or kept:
foldOwnersthen discards the file's valid owner, so a later strip, a memory re-adoption, and a keychain run purge the only secret without carrying it.secrets.jsonfiles. Client A is saved to F1 and B to F2 under one namespace. A keychain run configured for F1 absorbs A's secret, buteffectiveOwnersmerges thefile:records in insertion order and picks F2's owner B.The fix proposed above covers both: the hand-off records the owner of each value it actually copies or keeps, and any owner it can't establish is treated as ambiguous and never carried.
fix(auth): write a carried credential's stamp before its value
cliffhall commentedon Oct 20, 2026
Fixed by #2739 (merged into
v2/mainas 1b4d4ec).