From f5f4b2830ffc44cbcf1e07de949b9e7c37800a87 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Sun, 8 Nov 2026 21:35:13 +0000 Subject: [PATCH 1/8] Record a failed refresh on the linked identity row so another process does not re-present the token A failed or timed-out refresh attempt now writes refresh_failed_at under the row lock; a process that takes the lock inside a 60 s backoff answers managed_credential_refresh_failed without calling the provider. Rotation, revocation and re-link clear the marker. Closes the cross-process residual that refresh-once-per-token left to a follow-up, and pins it on the two-pool rig. Co-Authored-By: Claude Sonnet 5.5 --- docs/service-contract.md | 12 ++-- src/managed-credential.ts | 25 ++++++-- src/store/in-memory.ts | 6 ++ src/store/postgres.ts | 15 ++++- src/store/schema.ts | 5 ++ src/store/types.ts | 10 ++- tests/managed-credential.test.ts | 104 ++++++++++++++++++++++++++++--- 7 files changed, 156 insertions(+), 21 deletions(-) diff --git a/docs/service-contract.md b/docs/service-contract.md index e6ad6c1..f27e5e1 100644 --- a/docs/service-contract.md +++ b/docs/service-contract.md @@ -567,11 +567,13 @@ Important behavior: identity (`SELECT ... FOR UPDATE`), so a given refresh token is presented to the provider once across concurrent requests and across two Heimdall processes sharing one database. Within one process, concurrent refreshes of - one identity share a single attempt and its outcome (success or failure), so - nobody re-presents a token behind a failed holder in that burst. Residual - risk, tracked as follow-up `joystick:follow_up:cross-process-failed-refresh-marker`: - the guard is one presentation per process per burst, so N - Heimdall processes can present one token N times after a transient failure. + one identity share a single attempt and its outcome (success or failure). A + failed or timed-out attempt is recorded on the identity row + (`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, + a revocation or a re-link clears the marker. The provider call is bounded at 10 seconds - a provider whose refresh tokens rotate (Joystick) fails closed: a refresh response without a new refresh token revokes the credential instead of diff --git a/src/managed-credential.ts b/src/managed-credential.ts index 32e9f51..063b733 100644 --- a/src/managed-credential.ts +++ b/src/managed-credential.ts @@ -16,6 +16,14 @@ export const MANAGED_CREDENTIAL_REFRESH_SKEW_MS = 5 * 60 * 1000; */ export const MANAGED_CREDENTIAL_REFRESH_TIMEOUT_MS = 10_000; +/** + * After a refresh attempt fails or times out, no process presents the row's + * refresh token again until this long has passed (the failure is recorded on + * the row, read under the lock). A provider that rotates may have consumed the + * token before the attempt was aborted, and presenting it again reads as theft. + */ +export const MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS = 60 * 1000; + export interface ManagedCredentialContext { config: HeimdallConfig; store: HeimdallStore; @@ -68,10 +76,8 @@ function withoutTokens(identity: StoredLinkedIdentity): StoredLinkedIdentity { * attempt is in flight join it and receive its outcome, success or failure. * The entry lives exactly as long as the attempt: a later request retries. * - * Known residual risk (docs/service-contract.md): one presentation per - * process per burst, so N processes may present one token N times. The - * cross-process failed-refresh marker that would close it is - * joystick:follow_up:cross-process-failed-refresh-marker. + * Across processes the row owns it: a failed attempt leaves a marker on the + * row, and a process that takes the lock inside the backoff presents nothing. */ const refreshesInFlight = new WeakMap>>(); @@ -120,10 +126,15 @@ async function refreshUnderLock( const runtime = ctx.oauthRuntimes[provider]; let revocation: ManagedCredentialRevocationReason | undefined; + let failed = false; const decide = async (locked: StoredLinkedIdentity): Promise => { if (!locked.accessTokenEncrypted || !dueForRefresh(locked, runtime, clock())) { return { kind: "keep" }; } + if (locked.refreshFailedAt && clock() - Date.parse(locked.refreshFailedAt) < MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS) { + failed = true; + return { kind: "keep" }; + } const updatedAt = new Date(clock()).toISOString(); const refreshToken = ctx.tokenCustody.decrypt(locked.refreshTokenEncrypted!); @@ -139,7 +150,8 @@ async function refreshUnderLock( revocation = "provider_revoked"; return { kind: "revoke", updatedAt }; } - throw new ManagedCredentialRefreshFailed(provider); + failed = true; + return { kind: "failed", failedAt: updatedAt }; } let refreshTokenEncrypted: string; @@ -174,6 +186,9 @@ async function refreshUnderLock( createdAt: new Date(clock()).toISOString(), }); } + if (failed) { + throw new ManagedCredentialRefreshFailed(provider); + } if (!committed) { return { kind: "revoked", identity: withoutTokens(snapshot) }; } diff --git a/src/store/in-memory.ts b/src/store/in-memory.ts index e829900..e86490d 100644 --- a/src/store/in-memory.ts +++ b/src/store/in-memory.ts @@ -203,7 +203,13 @@ export class InMemoryStore implements HeimdallStore { if (decision.kind === "keep") { return clone(current); } + if (decision.kind === "failed") { + const marked: StoredLinkedIdentity = { ...current, refreshFailedAt: decision.failedAt }; + this.linkedIdentities.set(id, clone(marked)); + return clone(marked); + } const next: StoredLinkedIdentity = { ...current, updatedAt: decision.updatedAt }; + delete next.refreshFailedAt; delete next.accessTokenEncrypted; delete next.refreshTokenEncrypted; delete next.tokenExpiresAt; diff --git a/src/store/postgres.ts b/src/store/postgres.ts index 3d34047..0c4fc05 100644 --- a/src/store/postgres.ts +++ b/src/store/postgres.ts @@ -47,6 +47,7 @@ interface LinkedIdentityRow extends QueryResultRow { access_token_encrypted: string | null; refresh_token_encrypted: string | null; token_expires_at: string | null; + refresh_failed_at: string | null; scopes: string; profile_json: Record; created_at: string; @@ -207,6 +208,10 @@ function mapLinkedIdentityRow(row: LinkedIdentityRow): StoredLinkedIdentity { identity.tokenExpiresAt = new Date(row.token_expires_at).toISOString(); } + if (row.refresh_failed_at) { + identity.refreshFailedAt = new Date(row.refresh_failed_at).toISOString(); + } + return identity; } @@ -777,6 +782,7 @@ export class PostgresStore implements HeimdallStore { access_token_encrypted = EXCLUDED.access_token_encrypted, refresh_token_encrypted = EXCLUDED.refresh_token_encrypted, token_expires_at = EXCLUDED.token_expires_at, + refresh_failed_at = NULL, scopes = EXCLUDED.scopes, profile_json = EXCLUDED.profile_json, updated_at = EXCLUDED.updated_at @@ -822,7 +828,13 @@ export class PostgresStore implements HeimdallStore { const row = mapLinkedIdentityRow(expectRow(locked.rows[0], "updateLinkedIdentityCredential")); const decision = await decide(row); let committed = row; - if (decision.kind !== "keep") { + if (decision.kind === "failed") { + const marked = await client.query( + "UPDATE linked_identities SET refresh_failed_at = $2 WHERE id = $1 RETURNING *", + [id, decision.failedAt] + ); + committed = mapLinkedIdentityRow(expectRow(marked.rows[0], "updateLinkedIdentityCredential")); + } else if (decision.kind !== "keep") { const rotated = decision.kind === "rotate" ? decision : undefined; const written = await client.query( ` @@ -830,6 +842,7 @@ export class PostgresStore implements HeimdallStore { SET access_token_encrypted = $2, refresh_token_encrypted = $3, token_expires_at = $4, + refresh_failed_at = NULL, scopes = $5, updated_at = $6 WHERE id = $1 diff --git a/src/store/schema.ts b/src/store/schema.ts index 294ee2d..7aa6823 100644 --- a/src/store/schema.ts +++ b/src/store/schema.ts @@ -123,6 +123,11 @@ ALTER TABLE auth_completions ADD COLUMN IF NOT EXISTS attempt_id TEXT; -- compares it against the target the redeeming app states. ALTER TABLE auth_completions ADD COLUMN IF NOT EXISTS target_id TEXT; +-- When a refresh attempt on this row last failed or timed out, read under the row +-- lock so a second Heimdall process does not present the same refresh token +-- right behind it. Cleared by rotation, revocation and re-link. +ALTER TABLE linked_identities ADD COLUMN IF NOT EXISTS refresh_failed_at TIMESTAMPTZ; + -- One attempt handle may bind at most one unconsumed completion (R21.1): a -- second completion for the same handle would let the by-attempt UPDATE in -- consumeAuthCompletionByAttempt match more than one row, silently consuming diff --git a/src/store/types.ts b/src/store/types.ts index bb2e080..c795247 100644 --- a/src/store/types.ts +++ b/src/store/types.ts @@ -33,6 +33,8 @@ export interface StoredLinkedIdentity { accessTokenEncrypted?: string; refreshTokenEncrypted?: string; tokenExpiresAt?: string; + /** When a refresh attempt last failed; cleared by the next rotation, revocation or re-link. */ + refreshFailedAt?: string; scopes: string[]; profileJson: Record; createdAt: string; @@ -58,8 +60,9 @@ export interface UpsertLinkedIdentityInput { /** * What the owner of a refresh decides after reading the row under its lock. - * `rotate` and `revoke` write token columns, scopes and updated_at only; they - * never touch account, profile or identity fields. + * `rotate` and `revoke` write token columns, scopes, updated_at and clear the + * failed-refresh marker; `failed` writes the marker alone. None touches + * account, profile or identity fields. */ export type CredentialDecision = | { kind: "keep" } @@ -71,7 +74,8 @@ export type CredentialDecision = scopes: string[]; updatedAt: string; } - | { kind: "revoke"; updatedAt: string }; + | { kind: "revoke"; updatedAt: string } + | { kind: "failed"; failedAt: string }; export interface StoredSession { id: string; diff --git a/tests/managed-credential.test.ts b/tests/managed-credential.test.ts index ffe1472..03bcc1a 100644 --- a/tests/managed-credential.test.ts +++ b/tests/managed-credential.test.ts @@ -6,6 +6,7 @@ import { type HeimdallConfig } from "../src/config.js"; import { type Provider } from "../src/contracts.js"; import { createTokenCustody } from "../src/custody.js"; import { + MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS, MANAGED_CREDENTIAL_REFRESH_SKEW_MS, ManagedCredentialRefreshFailed, refreshManagedCredential, @@ -419,17 +420,18 @@ for (const { kind, live } of rigKinds) describe.skipIf(live && !adminUrl)(`manag expect(Date.now() - started).toBeLessThan(5_000); endpoint.mode = "normal"; - const outcome = await refreshManagedCredential(ctx, await rig.row()); + const afterBackoff = { ...ctx, clock: () => Date.now() + MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS }; + const outcome = await refreshManagedCredential(afterBackoff, await rig.row()); expect(outcome.kind).toBe("current"); expect(rig.custody.decrypt(outcome.identity.accessTokenEncrypted!)).toBe("access-1"); }); - it("single flight: concurrent refreshes against a never-answering endpoint make one provider call per process, and every caller gets the typed failure", timeout, async () => { + it("single flight: concurrent refreshes against a never-answering endpoint make one provider call across all processes, and every caller gets the typed failure", timeout, async () => { const rig = await makeRig(provider); const endpoint = installTokenEndpoint(provider, "refresh-0"); endpoint.mode = "hang"; const snapshot = await rig.row(); - const ctxFor = (store: HeimdallStore) => ({ config: rig.config, store, tokenCustody: rig.custody, oauthRuntimes: createOAuthRuntimeRegistry(), timeoutMs: 150 }); + const ctxFor = (store: HeimdallStore, clock = Date.now) => ({ config: rig.config, store, tokenCustody: rig.custody, oauthRuntimes: createOAuthRuntimeRegistry(), timeoutMs: 150, clock }); const results = await Promise.allSettled( Array.from({ length: 12 }, (_, i) => refreshManagedCredential(ctxFor(rig.stores[i % rig.stores.length]!), snapshot)) @@ -439,14 +441,102 @@ for (const { kind, live } of rigKinds) describe.skipIf(live && !adminUrl)(`manag for (const result of results) { expect((result as PromiseRejectedResult).reason).toBeInstanceOf(ManagedCredentialRefreshFailed); } - expect(endpoint.calls).toBe(rig.stores.length); + // The process that queued behind the failed holder reads the holder's marker under the lock and presents nothing. + expect(endpoint.calls).toBe(1); expect(rig.custody.decrypt((await rig.row()).refreshTokenEncrypted!)).toBe("refresh-0"); - // The burst is over: a later independent request tries again. + // The burst is over and so is the backoff the failure left on the row: a later independent request tries again. endpoint.mode = "normal"; - const later = await refreshManagedCredential(ctxFor(rig.stores[0]!), await rig.row()); + const later = await refreshManagedCredential(ctxFor(rig.stores[0]!, () => Date.now() + MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS), await rig.row()); expect(later.kind).toBe("current"); - expect(endpoint.calls).toBe(rig.stores.length + 1); + expect(endpoint.calls).toBe(2); + }); + + it.each(["server_error", "hang"] as const)( + "cross-process failed-refresh marker (%s): the failed attempt is recorded on the row, and another process does not present the token inside the backoff", + timeout, + async (mode) => { + const rig = await makeRig(provider); + const endpoint = installTokenEndpoint(provider, "refresh-0"); + endpoint.mode = mode; + const t0 = Date.now(); + const ctxFor = (store: HeimdallStore, at: number) => ({ + config: rig.config, store, tokenCustody: rig.custody, oauthRuntimes: createOAuthRuntimeRegistry(), timeoutMs: 100, clock: () => at, + }); + const snapshot = await rig.row(); + const first = rig.stores[0]!; + const other = rig.stores[rig.stores.length - 1]!; + + await expect(refreshManagedCredential(ctxFor(first, t0), snapshot)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(endpoint.calls).toBe(1); + const marked = await rig.row(); + expect(marked.refreshFailedAt).toBe(new Date(t0).toISOString()); + expect(marked.accessTokenEncrypted).toBe(snapshot.accessTokenEncrypted); + expect(marked.refreshTokenEncrypted).toBe(snapshot.refreshTokenEncrypted); + expect(marked.tokenExpiresAt).toBe(snapshot.tokenExpiresAt); + + // Another process, holding the stale snapshot, one tick before the backoff ends: the locked row decides, and no token call is made. + const backedOff = ctxFor(other, t0 + MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS - 1); + await expect(refreshManagedCredential(backedOff, snapshot)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(endpoint.calls).toBe(1); + // Being turned away does not extend the backoff. + expect((await rig.row()).refreshFailedAt).toBe(marked.refreshFailedAt); + expect(await rig.audit()).toEqual([]); + } + ); + + it("cross-process failed-refresh marker: the backoff ends exactly at its length, a success clears the marker, and a failure after it marks anew", timeout, async () => { + const rig = await makeRig(provider); + const endpoint = installTokenEndpoint(provider, "refresh-0"); + endpoint.mode = "server_error"; + const t0 = Date.now(); + const ctxAt = (at: number) => ({ config: rig.config, store: rig.stores[0]!, tokenCustody: rig.custody, oauthRuntimes: createOAuthRuntimeRegistry(), clock: () => at }); + await expect(refreshManagedCredential(ctxAt(t0), await rig.row())).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + + const retryAt = t0 + MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS; + await expect(refreshManagedCredential(ctxAt(retryAt), await rig.row())).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(endpoint.calls).toBe(2); + expect((await rig.row()).refreshFailedAt).toBe(new Date(retryAt).toISOString()); + + endpoint.mode = "normal"; + const laterAt = retryAt + MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS; + const outcome = await refreshManagedCredential(ctxAt(laterAt), await rig.row()); + expect(outcome.kind).toBe("current"); + expect(endpoint.calls).toBe(3); + expect(rig.custody.decrypt(outcome.identity.accessTokenEncrypted!)).toBe("access-1"); + expect((await rig.row()).refreshFailedAt).toBeUndefined(); + }); + + it("cross-process failed-refresh marker: a revocation and a re-link both clear it", timeout, async () => { + const rig = await makeRig(provider); + const endpoint = installTokenEndpoint(provider, "refresh-0"); + endpoint.mode = "server_error"; + const t0 = Date.now(); + const ctxAt = (at: number) => ({ config: rig.config, store: rig.stores[0]!, tokenCustody: rig.custody, oauthRuntimes: createOAuthRuntimeRegistry(), clock: () => at }); + await expect(refreshManagedCredential(ctxAt(t0), await rig.row())).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect((await rig.row()).refreshFailedAt).toBeDefined(); + + endpoint.mode = "invalid_grant"; + const revoked = await refreshManagedCredential(ctxAt(t0 + MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS), await rig.row()); + expect(revoked.kind).toBe("revoked"); + expect((await rig.row()).refreshFailedAt).toBeUndefined(); + + // A fresh consent after a failure replaces the whole credential, marker included. + endpoint.mode = "server_error"; + const relinkSeed = await rig.row(); + await rig.stores[0]!.upsertLinkedIdentity({ + accountId: rig.accountId, provider, providerUserId: relinkSeed.providerUserId, scopes: ["identity:read"], profileJson: {}, + accessTokenEncrypted: rig.custody.encrypt("access-r"), refreshTokenEncrypted: rig.custody.encrypt("refresh-r"), + tokenExpiresAt: new Date(t0 + 1000).toISOString(), createdAt: relinkSeed.createdAt, updatedAt: new Date(t0).toISOString(), + }); + await expect(refreshManagedCredential(ctxAt(t0 + 2000), await rig.row())).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect((await rig.row()).refreshFailedAt).toBeDefined(); + await rig.stores[0]!.upsertLinkedIdentity({ + accountId: rig.accountId, provider, providerUserId: relinkSeed.providerUserId, scopes: ["identity:read"], profileJson: {}, + accessTokenEncrypted: rig.custody.encrypt("access-s"), refreshTokenEncrypted: rig.custody.encrypt("refresh-s"), + tokenExpiresAt: new Date(t0 + 3000).toISOString(), createdAt: relinkSeed.createdAt, updatedAt: new Date(t0 + 2500).toISOString(), + }); + expect((await rig.row()).refreshFailedAt).toBeUndefined(); }); it("a snapshot taken before another caller's refresh does not decide: the locked row does", timeout, async () => { -- 2.47.3 From a0bd9836b130935872884cb3b72c3314937cf733 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Sun, 8 Nov 2026 21:40:39 +0000 Subject: [PATCH 2/8] Count the new linked_identities column in the schema inventory test Co-Authored-By: Claude Sonnet 5.5 --- tests/store.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/store.test.ts b/tests/store.test.ts index 7599ff6..75ee256 100644 --- a/tests/store.test.ts +++ b/tests/store.test.ts @@ -333,7 +333,7 @@ describe("PostgresStore.checkSchema", () => { "consent_receipts", "audit_events", ]); - expect(REQUIRED_COLUMNS).toHaveLength(112); + expect(REQUIRED_COLUMNS).toHaveLength(113); expect(REQUIRED_COLUMNS).toEqual( expect.arrayContaining([ { table: "auth_completions", column: "code", type: "TEXT", nullable: false }, -- 2.47.3 From 9da4b12bc27d011cd0a1405d38e10da6cfc5f17e Mon Sep 17 00:00:00 2001 From: Heimdall Date: Sun, 8 Nov 2026 21:48:13 +0000 Subject: [PATCH 3/8] Make the failed-refresh marker durable and stamp it with the failure time The marker carried the time the attempt began, so a slow failure ate the backoff; it now carries the time the provider's answer (or failure) was known. A throw after the provider answered (custody encrypt) records the marker instead of rolling it back, and a rotation Postgres refuses to write leaves the marker in a transaction of its own. A stamp from a writer's clock ahead of the reader's by the backoff or more is treated as skew, not a fresh failure. Co-Authored-By: Claude Sonnet 5.5 --- src/managed-credential.ts | 78 ++++++++++++++++++----------- src/store/postgres.ts | 11 ++++- tests/managed-credential.test.ts | 85 ++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 29 deletions(-) diff --git a/src/managed-credential.ts b/src/managed-credential.ts index 063b733..b11f3f4 100644 --- a/src/managed-credential.ts +++ b/src/managed-credential.ts @@ -1,7 +1,7 @@ import { type HeimdallConfig } from "./config.js"; import { type Provider } from "./contracts.js"; import { type TokenCustody } from "./custody.js"; -import { type OAuthProviderRuntime, type OAuthRuntimeRegistry, ProviderResponseError } from "./oauth.js"; +import { type ExpiringTokenSet, type OAuthProviderRuntime, type OAuthRuntimeRegistry, ProviderResponseError } from "./oauth.js"; import { providerCatalog } from "./providers.js"; import { type HeimdallStore } from "./store/index.js"; import { type CredentialDecision, type StoredLinkedIdentity } from "./store/types.js"; @@ -63,6 +63,17 @@ function dueForRefresh(identity: StoredLinkedIdentity, runtime: OAuthProviderRun return Date.parse(identity.tokenExpiresAt) - now <= MANAGED_CREDENTIAL_REFRESH_SKEW_MS; } +/** + * Whether a failure stamped `failedAt` still holds this clock off. A stamp from + * a writer whose clock runs ahead reads as the future; one this far ahead or + * more is skew, not a fresh failure, so the wait stays bounded at twice the + * backoff. + */ +function withinBackoff(failedAt: string, now: number): boolean { + const elapsed = now - Date.parse(failedAt); + return elapsed > -MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS && elapsed < MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS; +} + function withoutTokens(identity: StoredLinkedIdentity): StoredLinkedIdentity { const { accessTokenEncrypted: _a, refreshTokenEncrypted: _r, tokenExpiresAt: _e, ...rest } = identity; return rest; @@ -127,33 +138,7 @@ async function refreshUnderLock( let revocation: ManagedCredentialRevocationReason | undefined; let failed = false; - const decide = async (locked: StoredLinkedIdentity): Promise => { - if (!locked.accessTokenEncrypted || !dueForRefresh(locked, runtime, clock())) { - return { kind: "keep" }; - } - if (locked.refreshFailedAt && clock() - Date.parse(locked.refreshFailedAt) < MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS) { - failed = true; - return { kind: "keep" }; - } - const updatedAt = new Date(clock()).toISOString(); - const refreshToken = ctx.tokenCustody.decrypt(locked.refreshTokenEncrypted!); - - let refreshed; - try { - refreshed = await runtime.refreshAccessToken!({ - config: ctx.config, - refreshToken, - signal: AbortSignal.timeout(ctx.timeoutMs ?? MANAGED_CREDENTIAL_REFRESH_TIMEOUT_MS), - }); - } catch (error) { - if (error instanceof ProviderResponseError && error.oauthError === "invalid_grant") { - revocation = "provider_revoked"; - return { kind: "revoke", updatedAt }; - } - failed = true; - return { kind: "failed", failedAt: updatedAt }; - } - + const rotation = (locked: StoredLinkedIdentity, refreshed: ExpiringTokenSet, updatedAt: string): CredentialDecision => { let refreshTokenEncrypted: string; if (refreshed.refreshToken) { refreshTokenEncrypted = ctx.tokenCustody.encrypt(refreshed.refreshToken); @@ -177,6 +162,43 @@ async function refreshUnderLock( }; }; + const decide = async (locked: StoredLinkedIdentity): Promise => { + if (!locked.accessTokenEncrypted || !dueForRefresh(locked, runtime, clock())) { + return { kind: "keep" }; + } + if (locked.refreshFailedAt && withinBackoff(locked.refreshFailedAt, clock())) { + failed = true; + return { kind: "keep" }; + } + const refreshToken = ctx.tokenCustody.decrypt(locked.refreshTokenEncrypted!); + + let refreshed; + try { + refreshed = await runtime.refreshAccessToken!({ + config: ctx.config, + refreshToken, + signal: AbortSignal.timeout(ctx.timeoutMs ?? MANAGED_CREDENTIAL_REFRESH_TIMEOUT_MS), + }); + } catch (error) { + if (error instanceof ProviderResponseError && error.oauthError === "invalid_grant") { + revocation = "provider_revoked"; + return { kind: "revoke", updatedAt: new Date(clock()).toISOString() }; + } + failed = true; + return { kind: "failed", failedAt: new Date(clock()).toISOString() }; + } + + // The provider has answered, and may have consumed the token: from here a throw must still leave the marker, which + // a rolled-back transaction would not (the store writes it on its own for a rotation it could not commit). + const answeredAt = new Date(clock()).toISOString(); + try { + return rotation(locked, refreshed, answeredAt); + } catch { + failed = true; + return { kind: "failed", failedAt: new Date(clock()).toISOString() }; + } + }; + const committed = await ctx.store.updateLinkedIdentityCredential(snapshot.id, decide); if (revocation) { await ctx.store.createAuditEvent({ diff --git a/src/store/postgres.ts b/src/store/postgres.ts index 0c4fc05..d2fc9db 100644 --- a/src/store/postgres.ts +++ b/src/store/postgres.ts @@ -816,6 +816,7 @@ export class PostgresStore implements HeimdallStore { const client = await this.pool.connect(); try { await client.query("BEGIN"); + let decision: CredentialDecision | undefined; try { const locked = await client.query( "SELECT * FROM linked_identities WHERE id = $1 FOR UPDATE", @@ -826,7 +827,7 @@ export class PostgresStore implements HeimdallStore { return null; } const row = mapLinkedIdentityRow(expectRow(locked.rows[0], "updateLinkedIdentityCredential")); - const decision = await decide(row); + decision = await decide(row); let committed = row; if (decision.kind === "failed") { const marked = await client.query( @@ -863,6 +864,14 @@ export class PostgresStore implements HeimdallStore { return committed; } catch (error) { await client.query("ROLLBACK"); + if (decision?.kind === "rotate") { + // The provider answered and may have consumed the refresh token, and the rotation 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. + await client + .query("UPDATE linked_identities SET refresh_failed_at = $2 WHERE id = $1", [id, decision.updatedAt]) + .catch(() => undefined); + } throw error; } } finally { diff --git a/tests/managed-credential.test.ts b/tests/managed-credential.test.ts index 03bcc1a..9744abc 100644 --- a/tests/managed-credential.test.ts +++ b/tests/managed-credential.test.ts @@ -539,6 +539,91 @@ for (const { kind, live } of rigKinds) describe.skipIf(live && !adminUrl)(`manag expect((await rig.row()).refreshFailedAt).toBeUndefined(); }); + it("cross-process failed-refresh marker: stamped with the time of the failure, so a slow failure does not eat the backoff", timeout, async () => { + const rig = await makeRig(provider); + const endpoint = installTokenEndpoint(provider, "refresh-0"); + endpoint.mode = "server_error"; + const t0 = Date.now(); + // The provider call takes 9 s of the clock's time: the failure is known at t0 + 9 s, not at t0. + const slowClock = () => t0 + (endpoint.calls > 0 ? 9_000 : 0); + const ctx = (store: HeimdallStore, clock: () => number) => ({ config: rig.config, store, tokenCustody: rig.custody, oauthRuntimes: createOAuthRuntimeRegistry(), clock }); + const snapshot = await rig.row(); + + await expect(refreshManagedCredential(ctx(rig.stores[0]!, slowClock), snapshot)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect((await rig.row()).refreshFailedAt).toBe(new Date(t0 + 9_000).toISOString()); + + // 51 s after the failure, 60 s after the attempt began: still inside the backoff. + const other = rig.stores[rig.stores.length - 1]!; + await expect(refreshManagedCredential(ctx(other, () => t0 + MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS), snapshot)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(endpoint.calls).toBe(1); + }); + + it("cross-process failed-refresh marker: a failure after the provider answered still leaves the marker (the provider may have consumed the token)", timeout, async () => { + const rig = await makeRig(provider); + const endpoint = installTokenEndpoint(provider, "refresh-0"); + const snapshot = await rig.row(); + const throwingCustody = { + ...rig.custody, + encrypt: (plaintext: string) => { + if (plaintext === "access-1") throw new Error("CANARY custody encrypt failed"); + return rig.custody.encrypt(plaintext); + }, + }; + const ctx = (store: HeimdallStore, tokenCustody: typeof rig.custody) => ({ config: rig.config, store, tokenCustody, oauthRuntimes: createOAuthRuntimeRegistry() }); + + const failure = await refreshManagedCredential(ctx(rig.stores[0]!, throwingCustody), snapshot).then(() => undefined, (error: unknown) => error); + expect(failure).toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(String(failure) + JSON.stringify(failure)).not.toContain("CANARY"); + expect(endpoint.calls).toBe(1); + const marked = await rig.row(); + expect(marked.refreshFailedAt).toBeDefined(); + expect(marked.refreshTokenEncrypted).toBe(snapshot.refreshTokenEncrypted); + + // Another process inside the backoff does not present the old token again. + await expect(refreshManagedCredential(ctx(rig.stores[rig.stores.length - 1]!, rig.custody), snapshot)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(endpoint.calls).toBe(1); + }); + + it.skipIf(!live)("cross-process failed-refresh marker: a rotation the database refuses to write still leaves the marker", timeout, async () => { + const rig = await makeRig(provider); + const endpoint = installTokenEndpoint(provider, "refresh-0"); + const snapshot = await rig.row(); + const db = new Pool({ connectionString: rig.databaseUrl!, max: 1 }); + db.on("error", () => undefined); + try { + await db.query(`CREATE FUNCTION refuse_rotation() RETURNS trigger AS $$ BEGIN RAISE EXCEPTION 'CANARY rotation write refused'; END $$ LANGUAGE plpgsql`); + await db.query(`CREATE TRIGGER refuse_rotation BEFORE UPDATE ON linked_identities FOR EACH ROW WHEN (NEW.access_token_encrypted IS DISTINCT FROM OLD.access_token_encrypted) EXECUTE FUNCTION refuse_rotation()`); + const ctx = (store: HeimdallStore) => ({ config: rig.config, store, tokenCustody: rig.custody, oauthRuntimes: createOAuthRuntimeRegistry() }); + + await expect(refreshManagedCredential(ctx(rig.stores[0]!), snapshot)).rejects.toThrow(); + expect(endpoint.calls).toBe(1); + const marked = await rig.row(); + expect(marked.refreshFailedAt).toBeDefined(); + expect(marked.refreshTokenEncrypted).toBe(snapshot.refreshTokenEncrypted); + + await expect(refreshManagedCredential(ctx(rig.stores[rig.stores.length - 1]!), snapshot)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(endpoint.calls).toBe(1); + } finally { + await db.end(); + } + }); + + it("cross-process failed-refresh marker: a writer's clock skewed by under the backoff still holds the reader off, and by the backoff or more does not", timeout, async () => { + const rig = await makeRig(provider); + const endpoint = installTokenEndpoint(provider, "refresh-0"); + endpoint.mode = "server_error"; + const t0 = Date.now(); + const ctxAt = (at: number) => ({ config: rig.config, store: rig.stores[0]!, tokenCustody: rig.custody, oauthRuntimes: createOAuthRuntimeRegistry(), clock: () => at }); + await expect(refreshManagedCredential(ctxAt(t0), await rig.row())).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(endpoint.calls).toBe(1); + + // The marker is in this reader's future: a writer ahead of it. A bounded wait, not a refusal for as long as the skew. + await expect(refreshManagedCredential(ctxAt(t0 - MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS + 1), await rig.row())).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(endpoint.calls).toBe(1); + await expect(refreshManagedCredential(ctxAt(t0 - MANAGED_CREDENTIAL_REFRESH_BACKOFF_MS), await rig.row())).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(endpoint.calls).toBe(2); + }); + it("a snapshot taken before another caller's refresh does not decide: the locked row does", timeout, async () => { const rig = await makeRig(provider); const endpoint = installTokenEndpoint(provider, "refresh-0"); -- 2.47.3 From 1118f235b02b937fadb7569f40cbba61651c4353 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Sun, 8 Nov 2026 21:53:12 +0000 Subject: [PATCH 4/8] Docs: name the process-death residual in the refresh contract Co-Authored-By: Claude Sonnet 5.5 --- docs/service-contract.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/service-contract.md b/docs/service-contract.md index f27e5e1..f0be8ed 100644 --- a/docs/service-contract.md +++ b/docs/service-contract.md @@ -574,6 +574,14 @@ Important behavior: that takes the lock inside the backoff answers the same `managed_credential_refresh_failed` without calling the provider. A rotation, a revocation or a re-link clears the marker. + Residual risk: if a Heimdall process dies between the provider's token + answer and the commit of the marker or the rotation, 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. Closing it + needs a write-ahead record of the attempt, written before the provider call on + a connection that does not wait on the row lock (a separate table). That is + deferred by steward ruling and tracked as + `joystick:follow_up:write-ahead-refresh-attempt-record` The provider call is bounded at 10 seconds - a provider whose refresh tokens rotate (Joystick) fails closed: a refresh response without a new refresh token revokes the credential instead of -- 2.47.3 From 9c7d980165c867bde3ae499b6312498ae707feb1 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Sun, 8 Nov 2026 21:56:40 +0000 Subject: [PATCH 5/8] Leave the failed-refresh marker when the revoke a rotating provider's answer called for cannot be written A rotating provider that answered without a new refresh token has consumed the old one; the revoke that follows is rolled back if its UPDATE fails, and no marker remained. The fallback marker now covers revoke as well as rotate. The residual-risk paragraph says the window opens when the request is sent, cites the steward ruling, and ends its sentences. Co-Authored-By: Claude Sonnet 5.5 --- docs/service-contract.md | 18 +++++++++--------- src/store/postgres.ts | 7 ++++--- tests/managed-credential.test.ts | 29 +++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/docs/service-contract.md b/docs/service-contract.md index f0be8ed..5c0fa5c 100644 --- a/docs/service-contract.md +++ b/docs/service-contract.md @@ -574,15 +574,15 @@ Important behavior: that takes the lock inside the backoff answers the same `managed_credential_refresh_failed` without calling the provider. A rotation, a revocation or a re-link clears the marker. - Residual risk: if a Heimdall process dies between the provider's token - answer and the commit of the marker or the rotation, 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. Closing it - needs a write-ahead record of the attempt, written before the provider call on - a connection that does not wait on the row lock (a separate table). That is - deferred by steward ruling and tracked as - `joystick:follow_up:write-ahead-refresh-attempt-record` - The provider call is bounded at 10 seconds + 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 + 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 + connection before the provider call, replaces this marker; it is tracked in + `joystick:follow_up:write-ahead-refresh-attempt-record`. + The provider call is bounded at 10 seconds. - a provider whose refresh tokens rotate (Joystick) fails closed: a refresh response without a new refresh token revokes the credential instead of keeping the old token diff --git a/src/store/postgres.ts b/src/store/postgres.ts index d2fc9db..eb7f068 100644 --- a/src/store/postgres.ts +++ b/src/store/postgres.ts @@ -864,9 +864,10 @@ export class PostgresStore implements HeimdallStore { return committed; } catch (error) { await client.query("ROLLBACK"); - if (decision?.kind === "rotate") { - // The provider answered and may have consumed the refresh token, and the rotation 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 + 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. await client .query("UPDATE linked_identities SET refresh_failed_at = $2 WHERE id = $1", [id, decision.updatedAt]) diff --git a/tests/managed-credential.test.ts b/tests/managed-credential.test.ts index 9744abc..a2aa2ef 100644 --- a/tests/managed-credential.test.ts +++ b/tests/managed-credential.test.ts @@ -662,6 +662,35 @@ for (const { kind, live } of rigKinds) describe.skipIf(live && !adminUrl)(`manag { eventType: "managed_credential_revoked", payload: { provider, reason: "rotation_missing" } }, ]); }); + + it.skipIf(!live).each(["no_refresh_token", "empty_refresh_token"] as const)( + "rotation-fails-closed: when the revoke the provider's answer called for cannot be written (%s), the marker is still left", + timeout, + async (mode) => { + const rig = await makeRig(provider); + const endpoint = installTokenEndpoint(provider, "refresh-0"); + endpoint.mode = mode; + const snapshot = await rig.row(); + const db = new Pool({ connectionString: rig.databaseUrl!, max: 1 }); + db.on("error", () => undefined); + try { + await db.query(`CREATE FUNCTION refuse_revoke() RETURNS trigger AS $$ BEGIN RAISE EXCEPTION 'CANARY revoke write refused'; END $$ LANGUAGE plpgsql`); + await db.query(`CREATE TRIGGER refuse_revoke BEFORE UPDATE ON linked_identities FOR EACH ROW WHEN (NEW.access_token_encrypted IS DISTINCT FROM OLD.access_token_encrypted) EXECUTE FUNCTION refuse_revoke()`); + const ctx = (store: HeimdallStore) => ({ config: rig.config, store, tokenCustody: rig.custody, oauthRuntimes: createOAuthRuntimeRegistry() }); + + await expect(refreshManagedCredential(ctx(rig.stores[0]!), snapshot)).rejects.toThrow(); + expect(endpoint.calls).toBe(1); + const marked = await rig.row(); + expect(marked.refreshFailedAt).toBeDefined(); + expect(marked.refreshTokenEncrypted).toBe(snapshot.refreshTokenEncrypted); + + await expect(refreshManagedCredential(ctx(rig.stores[rig.stores.length - 1]!), snapshot)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(endpoint.calls).toBe(1); + } finally { + await db.end(); + } + } + ); } else { it.each(["no_refresh_token", "empty_refresh_token"] as const)("a response with %s keeps the old one for a non-rotating provider", timeout, async (mode) => { const rig = await makeRig(provider); -- 2.47.3 From f2f1dabc06f905ebfc3f7023bb335a245058704d Mon Sep 17 00:00:00 2001 From: Heimdall Date: Mon, 9 Nov 2026 10:07:24 +0000 Subject: [PATCH 6/8] 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(); -- 2.47.3 From d4cd1121939db162dbf53959b7afc4213721af1e Mon Sep 17 00:00:00 2001 From: Heimdall Date: Mon, 9 Nov 2026 10:22:14 +0000 Subject: [PATCH 7/8] Blank the refresh token in the fallback write only when it is spent A provider that keeps its refresh token valid (Twitch) keeps it when the rotation cannot be committed: the marker is left, the token stays, and the next caller after the backoff presents it. Blanking stays for a revoke and for a provider that rotates (the rotate decision now carries consumedRefreshToken). Live-Postgres test for Twitch written first and seen failing; the contract states the rule inline. Co-Authored-By: Claude Sonnet 5.5 --- docs/service-contract.md | 11 ++++++----- src/managed-credential.ts | 1 + src/store/postgres.ts | 17 +++++++++++------ src/store/types.ts | 2 ++ tests/managed-credential.test.ts | 27 +++++++++++++++++++++------ 5 files changed, 41 insertions(+), 17 deletions(-) diff --git a/docs/service-contract.md b/docs/service-contract.md index d68f66f..bf8d810 100644 --- a/docs/service-contract.md +++ b/docs/service-contract.md @@ -579,11 +579,12 @@ Important behavior: 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). + committed, the fallback write leaves the marker and blanks the refresh token + only when that token is spent: on a revoke (a rotating provider answered + without a refresh token, or the grant was rejected), or when the provider + rotates refresh tokens (Joystick) and so consumed it. A provider that keeps + its refresh token valid (Twitch) keeps it, so a transient database failure + does not strand the link: the next caller after the backoff presents it again. 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, or if the blanking fallback diff --git a/src/managed-credential.ts b/src/managed-credential.ts index b11f3f4..fe60fc6 100644 --- a/src/managed-credential.ts +++ b/src/managed-credential.ts @@ -159,6 +159,7 @@ async function refreshUnderLock( tokenExpiresAt: refreshed.expiresAt, scopes: refreshed.scope.length ? refreshed.scope : locked.scopes, updatedAt, + consumedRefreshToken: providerCatalog[provider].rotatesRefreshTokens, }; }; diff --git a/src/store/postgres.ts b/src/store/postgres.ts index cd323cc..6d96598 100644 --- a/src/store/postgres.ts +++ b/src/store/postgres.ts @@ -865,13 +865,18 @@ export class PostgresStore implements HeimdallStore { } catch (error) { await client.query("ROLLBACK"); 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. 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. + // The provider answered, and what its answer called for could not be committed: the rollback took the answer + // with it. In a transaction of its own, leave the marker, so no caller presents the token within the backoff. + // Blank the token only when it is spent: a revoke, or a provider that rotates (Joystick) and consumed it. + // A provider that keeps its refresh token valid (Twitch) keeps it, so a transient write failure does not + // strand the link. 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. + const spent = decision.kind === "revoke" || decision.consumedRefreshToken; await client - .query("UPDATE linked_identities SET refresh_token_encrypted = NULL, refresh_failed_at = $2 WHERE id = $1", [id, decision.updatedAt]) + .query( + `UPDATE linked_identities SET ${spent ? "refresh_token_encrypted = NULL, " : ""}refresh_failed_at = $2 WHERE id = $1`, + [id, decision.updatedAt] + ) .catch(() => undefined); } throw error; diff --git a/src/store/types.ts b/src/store/types.ts index c795247..9eb805b 100644 --- a/src/store/types.ts +++ b/src/store/types.ts @@ -73,6 +73,8 @@ export type CredentialDecision = tokenExpiresAt: string; scopes: string[]; updatedAt: string; + /** The provider rotates refresh tokens, so the one presented is spent whether or not this decision commits. */ + consumedRefreshToken: boolean; } | { kind: "revoke"; updatedAt: string } | { kind: "failed"; failedAt: string }; diff --git a/tests/managed-credential.test.ts b/tests/managed-credential.test.ts index 6572c15..6ddfa62 100644 --- a/tests/managed-credential.test.ts +++ b/tests/managed-credential.test.ts @@ -599,13 +599,28 @@ 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(); - // The provider consumed the token: nothing replayable is left on the row, so the backoff ending presents nothing. - expect(marked.refreshTokenEncrypted).toBeUndefined(); + if (providerCatalog[provider].rotatesRefreshTokens) { + // 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)).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); + 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); + } else { + // A non-rotating provider keeps the token valid: a transient write failure must not strand the link. + expect(marked.refreshTokenEncrypted).toBe(snapshot.refreshTokenEncrypted); + + await expect(refreshManagedCredential(ctx(rig.stores[rig.stores.length - 1]!), snapshot)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(endpoint.calls).toBe(1); + await db.query("DROP TRIGGER refuse_rotation ON linked_identities"); + // The fake endpoint rotates on every refresh; Twitch does not, so its answer left refresh-0 valid. + endpoint.live = "refresh-0"; + const afterBackoff = { ...ctx(rig.stores[0]!), clock: () => Date.now() + 5 * 60_000 }; + const outcome = await refreshManagedCredential(afterBackoff, snapshot); + expect(outcome.kind).toBe("current"); + expect(endpoint.calls).toBe(2); + } } finally { await db.end(); } -- 2.47.3 From 246f2d4f714f6b9e44ec330981772f4039dd221e Mon Sep 17 00:00:00 2001 From: Heimdall Date: Mon, 9 Nov 2026 10:27:35 +0000 Subject: [PATCH 8/8] Tests: rotate fixtures carry consumedRefreshToken Co-Authored-By: Claude Sonnet 5.5 --- tests/managed-credential.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/managed-credential.test.ts b/tests/managed-credential.test.ts index 6ddfa62..9b226b7 100644 --- a/tests/managed-credential.test.ts +++ b/tests/managed-credential.test.ts @@ -757,6 +757,7 @@ for (const { kind, live } of rigKinds) describe.skipIf(live && !adminUrl)(`manag tokenExpiresAt: past, scopes: locked.scopes, updatedAt: past, + consumedRefreshToken: false, })); expect((await rig.resolve()).statusCode).toBe(200); expect(endpoint.calls).toBe(2); @@ -936,6 +937,7 @@ for (const { kind, live } of rigKinds) describe.skipIf(live && !adminUrl)(`manag tokenExpiresAt: "2999-01-01T00:00:00.000Z", scopes: ["a", "b"], updatedAt: "2999-01-02T00:00:00.000Z", + consumedRefreshToken: false, })); expect(rotated).toEqual({ ...before, @@ -1019,6 +1021,7 @@ for (const { kind, live } of rigKinds) describe.skipIf(live && !adminUrl)(`manag tokenExpiresAt: new Date(Date.now() + 3600_000).toISOString(), scopes: locked.scopes, updatedAt: new Date().toISOString(), + consumedRefreshToken: false, }; }) ) @@ -1297,6 +1300,7 @@ describe("refresh answer classification", () => { tokenExpiresAt: new Date().toISOString(), scopes: locked.scopes, updatedAt: new Date().toISOString(), + consumedRefreshToken: false, })))!; const calls = answer(JSON.stringify({ access_token: "access-1", refresh_token: "refresh-1", ...extra }), 200); let skew = 0; @@ -1329,6 +1333,7 @@ describe("refresh answer classification", () => { tokenExpiresAt: dueAt, scopes: ["s"], updatedAt: dueAt, + consumedRefreshToken: false, })); const calls = answer(JSON.stringify({ access_token: "access-2", expires_in: 3600 }), 200); await refreshManagedCredential(ctxFor(store), snapshot); -- 2.47.3