From f2f1dabc06f905ebfc3f7023bb335a245058704d Mon Sep 17 00:00:00 2001 From: Heimdall Date: Mon, 9 Nov 2026 10:07:24 +0000 Subject: [PATCH] Blank the consumed refresh token in the fallback marker write When the rotation or revocation the provider's answer called for cannot be committed, the fallback write now clears refresh_token_encrypted as well as stamping the marker, so the end of the 60 s backoff cannot replay the consumed token. Tests (live Postgres, trigger-refused revoke) assert no replayable token and no second provider call. Docs: the marker is a best-effort write on a second connection after rollback, the skew cap, the blanking write's residual. Co-Authored-By: Claude Sonnet 5.5 --- docs/service-contract.md | 15 +++++++++++++-- src/store/postgres.ts | 7 ++++--- tests/managed-credential.test.ts | 14 ++++++++++---- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/docs/service-contract.md b/docs/service-contract.md index 5c0fa5c..d68f66f 100644 --- a/docs/service-contract.md +++ b/docs/service-contract.md @@ -572,11 +572,22 @@ Important behavior: (`refresh_failed_at`, written in the same transaction as the lock), and no process presents that row's refresh token again for 60 seconds: a process that takes the lock inside the backoff answers the same - `managed_credential_refresh_failed` without calling the provider. A rotation, + `managed_credential_refresh_failed` without calling the provider. A stamp + more than 60 seconds in the future (a writer whose clock runs ahead) is skew, + not a fresh failure, so the wait is capped at twice the backoff. A rotation, a revocation or a re-link clears the marker. + The marker is a best-effort write on a second connection, made after the + lock's transaction rolled back; it is not part of that transaction. When the + rotation or the revocation the provider's answer called for cannot be + committed (a rotating provider answered without a refresh token, so the + revoke was due, and the write was refused), the fallback write also blanks + the refresh token the provider consumed, so the end of the backoff does not + present it again (`joystick:follow_up:chat-s4509-4511`, steward ruling in + #minds, 2026-11-09). Residual risk: the provider may consume the refresh token as soon as the request is sent. If a Heimdall process dies after that and before the commit - of the marker, the rotation or the revocation, nothing is recorded, and the + of the marker, the rotation or the revocation, or if the blanking fallback + write fails too, nothing is recorded, and the next caller can present the refresh token the provider already consumed; for a provider that rotates (Joystick) that reads as token theft. The Heimdall steward ruled (#minds, 2026-11-08) that an intent log, written on its own diff --git a/src/store/postgres.ts b/src/store/postgres.ts index eb7f068..cd323cc 100644 --- a/src/store/postgres.ts +++ b/src/store/postgres.ts @@ -867,10 +867,11 @@ export class PostgresStore implements HeimdallStore { if (decision?.kind === "rotate" || decision?.kind === "revoke") { // The provider answered and may have consumed the refresh token, and the rotation or the revoke its answer // called for (a rotating provider's missing refresh token) could not be committed: the rollback took the - // answer with it. Leave the marker in a transaction of its own, so the next caller does not - // present the old token. Best effort: if this write fails too, the original error is the one that matters. + // answer with it. In a transaction of its own, blank the consumed token and leave the marker, so no later + // caller, after the backoff either, presents it again. Best effort: if this write fails too, the original + // error is the one that matters, and the residual is named in docs/service-contract.md. await client - .query("UPDATE linked_identities SET refresh_failed_at = $2 WHERE id = $1", [id, decision.updatedAt]) + .query("UPDATE linked_identities SET refresh_token_encrypted = NULL, refresh_failed_at = $2 WHERE id = $1", [id, decision.updatedAt]) .catch(() => undefined); } throw error; diff --git a/tests/managed-credential.test.ts b/tests/managed-credential.test.ts index a2aa2ef..6572c15 100644 --- a/tests/managed-credential.test.ts +++ b/tests/managed-credential.test.ts @@ -599,9 +599,12 @@ for (const { kind, live } of rigKinds) describe.skipIf(live && !adminUrl)(`manag expect(endpoint.calls).toBe(1); const marked = await rig.row(); expect(marked.refreshFailedAt).toBeDefined(); - expect(marked.refreshTokenEncrypted).toBe(snapshot.refreshTokenEncrypted); + // The provider consumed the token: nothing replayable is left on the row, so the backoff ending presents nothing. + expect(marked.refreshTokenEncrypted).toBeUndefined(); - await expect(refreshManagedCredential(ctx(rig.stores[rig.stores.length - 1]!), snapshot)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + await expect(refreshManagedCredential(ctx(rig.stores[rig.stores.length - 1]!), snapshot)).resolves.toBeDefined(); + const afterBackoff = { ...ctx(rig.stores[0]!), clock: () => Date.now() + 5 * 60_000 }; + await expect(refreshManagedCredential(afterBackoff, snapshot)).resolves.toBeDefined(); expect(endpoint.calls).toBe(1); } finally { await db.end(); @@ -682,9 +685,12 @@ for (const { kind, live } of rigKinds) describe.skipIf(live && !adminUrl)(`manag expect(endpoint.calls).toBe(1); const marked = await rig.row(); expect(marked.refreshFailedAt).toBeDefined(); - expect(marked.refreshTokenEncrypted).toBe(snapshot.refreshTokenEncrypted); + // The provider consumed the token: nothing replayable is left on the row, so the backoff ending presents nothing. + expect(marked.refreshTokenEncrypted).toBeUndefined(); - await expect(refreshManagedCredential(ctx(rig.stores[rig.stores.length - 1]!), snapshot)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + await expect(refreshManagedCredential(ctx(rig.stores[rig.stores.length - 1]!), snapshot)).resolves.toBeDefined(); + const afterBackoff = { ...ctx(rig.stores[0]!), clock: () => Date.now() + 5 * 60_000 }; + await expect(refreshManagedCredential(afterBackoff, snapshot)).resolves.toBeDefined(); expect(endpoint.calls).toBe(1); } finally { await db.end();