From 7a17493b2234f19d369c5fb946b98e0c274327c3 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Wed, 8 Jul 2026 19:22:04 -0700 Subject: [PATCH 1/4] Add e2e repro for vault write failure during token refresh --- e2e/cloud/vault-write-outage.test.ts | 313 +++++++++++++++++++++++++++ 1 file changed, 313 insertions(+) create mode 100644 e2e/cloud/vault-write-outage.test.ts diff --git a/e2e/cloud/vault-write-outage.test.ts b/e2e/cloud/vault-write-outage.test.ts new file mode 100644 index 0000000000..4c4c85809f --- /dev/null +++ b/e2e/cloud/vault-write-outage.test.ts @@ -0,0 +1,313 @@ +// Cloud: reproduce the production "StorageError: WorkOS Vault secret write +// failed" Sentry events (NODE-CLOUDFLARE-WORKERS-52/53/4T) at the real +// upstream — faults armed on the WorkOS emulator's Vault KV PUT, the same +// emulator the product's real WorkOS SDK talks to. No product code touched. +// +// The production chain, staged here end-to-end: +// 1. An OAuth connection's access token expires (per-client TTL of 1s via +// the emulator's DCR `access_token_ttl_seconds` extension, well inside +// the 60s refresh skew). +// 2. A health check resolves the connection → triggers the refresh-token +// grant. The grant SUCCEEDS and the authorization server ROTATES the +// refresh token (single use — the emulator mirrors AuthKit). +// 3. Persisting the new tokens does read → `PUT /vault/v1/kv/:id`. The PUT +// fails (in prod: 409 version conflict, an OAuth-shaped 400, or an HTML +// error page; here: the armed 400 with `error`/`error_description`, the +// exact shape the WorkOS SDK maps to OauthException — Sentry issue -53). +// 4. The write failure surfaces as `StorageError: WorkOS Vault secret write +// failed` → the health endpoint answers a typed InternalError (the 500 +// Sentry records). +// +// The second scenario pins the DAMAGE, not just the surface error: the vault +// write failed AFTER the refresh token was consumed and rotated, so the new +// refresh token is lost and the stored one is revoked. Once the vault +// recovers, the connection must recover too — today it cannot (the next +// refresh gets invalid_grant → the connection reads "expired" and demands a +// re-auth). Red until the refresh/persist ordering is made crash-safe. +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import type { HttpApiClient } from "effect/unstable/httpapi"; +import { composePluginApi } from "@executor-js/api/server"; +import { connectEmulator, type EmulatorClient } from "@executor-js/emulate"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { + AuthTemplateSlug, + ConnectionName, + IntegrationSlug, + OAuthClientSlug, +} from "@executor-js/sdk/shared"; + +import { scenario } from "../src/scenario"; +import { Api, Target } from "../src/services"; +import type { Target as TargetShape } from "../src/target"; +import { WORKOS_EMULATOR_PORT } from "../targets/cloud"; + +const api = composePluginApi([openApiHttpPlugin()] as const); +type Client = HttpApiClient.ForApi; + +const TEMPLATE = AuthTemplateSlug.make("oauth2"); +const CONNECTION = ConnectionName.make("main"); +const WORKOS_EMULATOR_URL = `http://127.0.0.1:${WORKOS_EMULATOR_PORT}`; + +const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`; + +/** Minimal OpenAPI spec with a single GET /ping — never contacted (the + * integration declares no health-check spec, so checkHealth takes the + * credential-resolution path, which is where the vault write lives). */ +const pingSpec = JSON.stringify({ + openapi: "3.0.3", + info: { title: "Ping API", version: "1.0.0" }, + paths: { + "/ping": { + get: { operationId: "ping", summary: "Ping", responses: { "200": { description: "pong" } } }, + }, + }, +}); + +// The vault-write outage: `PUT /vault/v1/kv/:id` (the WorkOS SDK's +// updateObject) starts answering with the OAuth-shaped 400 the production +// events carry ("OauthException: Error: Invalid request parameters" — the +// SDK maps any non-{401,404,409,422,429} status whose body has +// `error`/`error_description` to OauthException). Reads (GET) and creates +// (POST) stay healthy: only the update leg fails, as in production. +// `times` bounds the blast radius; the finalizer clears whatever remains. +const VAULT_UPDATE_FAULT = { + match: { method: "PUT", pathPattern: "/vault/v1/kv/*" }, + response: { + status: 400, + body: { + code: "invalid_request", + message: "Invalid request parameters", + error: "Invalid request parameters", + error_description: "Invalid request parameters", + }, + }, + times: 8, +} as const; + +/** DCR-register an OAuth client directly on the WorkOS emulator with a 1s + * access-token TTL (per-client, so the emulator's default — which the + * product's own AuthKit sessions depend on — is untouched). 1s is inside + * executor's 60s refresh skew: the first resolve after connect refreshes. */ +const registerShortLivedOAuthClient = (redirectUri: string) => + Effect.promise(async () => { + const response = await fetch(`${WORKOS_EMULATOR_URL}/oauth2/register`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + client_name: "vault-write-outage-e2e", + redirect_uris: [redirectUri], + access_token_ttl_seconds: 1, + }), + }); + if (response.status !== 201) { + throw new Error(`WorkOS emulator DCR failed: ${response.status}`); + } + const body = (await response.json()) as { readonly client_id: string }; + return body.client_id; + }); + +/** Complete the emulator's authorize hop headlessly: `login_hint` makes the + * authorize endpoint 302 straight back with a code (no consent page). */ +const completeConsent = (authorizationUrl: string, email: string) => + Effect.promise(async () => { + const url = new URL(authorizationUrl); + url.searchParams.set("login_hint", email); + const callback = await fetch(url, { redirect: "manual" }); + const location = callback.headers.get("location"); + if (callback.status !== 302 || !location) { + throw new Error(`WorkOS emulator authorize did not redirect: ${callback.status}`); + } + const code = new URL(location).searchParams.get("code"); + if (!code) throw new Error("WorkOS emulator callback did not include a code"); + return code; + }); + +/** Register a fresh integration + OAuth app against the emulator's generic + * authorize/token endpoints and connect it. `offline_access` mints a + * refresh token, so the expired-token path refreshes instead of re-authing. */ +const connectExpiringOAuthConnection = (input: { + readonly client: Client; + readonly target: TargetShape; + readonly integration: IntegrationSlug; + readonly oauthClient: OAuthClientSlug; +}) => + Effect.gen(function* () { + const redirectUri = new URL("/api/oauth/callback", input.target.baseUrl).toString(); + + yield* input.client.openapi.addSpec({ + payload: { + spec: { kind: "blob", value: pingSpec }, + slug: input.integration, + baseUrl: "http://127.0.0.1:59999", // never contacted + authenticationTemplate: [ + { + slug: "oauth2", + kind: "oauth2", + authorizationUrl: `${WORKOS_EMULATOR_URL}/oauth2/authorize`, + tokenUrl: `${WORKOS_EMULATOR_URL}/oauth2/token`, + scopes: ["offline_access"], + }, + ], + }, + }); + + const clientId = yield* registerShortLivedOAuthClient(redirectUri); + yield* input.client.oauth.createClient({ + payload: { + owner: "org", + slug: input.oauthClient, + grant: "authorization_code", + authorizationUrl: `${WORKOS_EMULATOR_URL}/oauth2/authorize`, + tokenUrl: `${WORKOS_EMULATOR_URL}/oauth2/token`, + clientId, + // The emulator's DCR clients are public (auth method "none"); the + // secret is carried but never validated. + clientSecret: "unused", + originIntegration: input.integration, + }, + }); + + const started = yield* input.client.oauth.start({ + payload: { + client: input.oauthClient, + clientOwner: "org", + owner: "org", + name: CONNECTION, + integration: input.integration, + template: TEMPLATE, + redirectUri, + }, + }); + expect(started.status, "OAuth starts with an emulator redirect").toBe("redirect"); + if (started.status !== "redirect") return yield* Effect.die("OAuth unexpectedly connected"); + + const code = yield* completeConsent(started.authorizationUrl, "vault-outage@example.com"); + const completed = yield* input.client.oauth.complete({ + payload: { state: started.state, code }, + }); + expect(completed.integration, "OAuth completion creates the connection").toBe( + input.integration, + ); + }); + +const removeEverything = (input: { + readonly client: Client; + readonly integration: IntegrationSlug; + readonly oauthClient: OAuthClientSlug; +}) => + Effect.gen(function* () { + yield* input.client.connections + .remove({ params: { owner: "org", integration: input.integration, name: CONNECTION } }) + .pipe(Effect.ignore); + yield* input.client.oauth + .removeClient({ params: { slug: input.oauthClient }, payload: { owner: "org" } }) + .pipe(Effect.ignore); + yield* input.client.openapi + .removeSpec({ params: { slug: input.integration } }) + .pipe(Effect.ignore); + }); + +const checkHealth = (client: Client, integration: IntegrationSlug) => + client.connections.checkHealth({ + params: { owner: "org", integration, name: CONNECTION }, + query: { ifStaleMs: 0 }, + }); + +scenario( + "Vault · a vault write failure during token refresh surfaces as the internal storage error", + {}, + Effect.gen(function* () { + const target = yield* Target; + const { client: makeClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeClient(api, identity); + const integration = IntegrationSlug.make(unique("vault_outage")); + const oauthClient = OAuthClientSlug.make(unique("vault_outage_app")); + const workos = yield* Effect.promise(() => connectEmulator({ baseUrl: WORKOS_EMULATOR_URL })); + + yield* Effect.ensuring( + Effect.gen(function* () { + yield* connectExpiringOAuthConnection({ client, target, integration, oauthClient }); + + // The 1s token is already inside the refresh skew. Break the vault's + // update leg, then run the health check that resolves the credential. + yield* Effect.promise(() => workos.faults.arm(VAULT_UPDATE_FAULT)); + + const error = yield* checkHealth(client, integration).pipe(Effect.flip); + expect( + (error as { _tag?: string })._tag, + "the failed vault write surfaces as the typed internal error (the prod 500)", + ).toBe("InternalError"); + + // The proof this is the production mechanism and not an incidental + // failure: the product performed the refresh grant and then hit the + // injected fault on the vault update. + const ledger = yield* Effect.promise(() => workos.ledger.list(200)); + const faultedPut = ledger.find((entry) => entry.faulted === true && entry.method === "PUT"); + expect(faultedPut?.path, "the failure came from the injected vault-update fault").toMatch( + /\/vault\/v1\/kv\//, + ); + const refreshGrant = ledger.find( + (entry) => + entry.path.endsWith("/oauth2/token") && + JSON.stringify(entry.request.body ?? "").includes("refresh_token"), + ); + expect( + refreshGrant, + "the vault write that failed was persisting a completed refresh grant", + ).toBeDefined(); + }), + Effect.gen(function* () { + yield* Effect.promise(() => workos.faults.clear()); + yield* removeEverything({ client, integration, oauthClient }); + }), + ); + }), +); + +scenario( + "Vault · a transient vault write outage does not invalidate the connection once the vault recovers", + {}, + Effect.gen(function* () { + const target = yield* Target; + const { client: makeClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeClient(api, identity); + const integration = IntegrationSlug.make(unique("vault_recover")); + const oauthClient = OAuthClientSlug.make(unique("vault_recover_app")); + const workos = yield* Effect.promise(() => connectEmulator({ baseUrl: WORKOS_EMULATOR_URL })); + + yield* Effect.ensuring( + Effect.gen(function* () { + yield* connectExpiringOAuthConnection({ client, target, integration, oauthClient }); + + // One health check during the outage: the refresh grant consumes and + // ROTATES the single-use refresh token, then the vault write of the + // rotated pair fails. + yield* Effect.promise(() => workos.faults.arm(VAULT_UPDATE_FAULT)); + yield* checkHealth(client, integration).pipe(Effect.flip); + yield* Effect.promise(() => workos.faults.clear()); + + // The vault has recovered. A transient storage blip must not cost the + // user their grant: the connection has a live authorization at the AS + // and must come back healthy without a re-auth. Today it cannot — the + // rotated refresh token was never persisted, so the stored (revoked) + // one is replayed, the AS answers invalid_grant, and the connection + // reads "expired" until a human reconnects. This is the real damage + // behind the prod Sentry events. + const health = yield* checkHealth(client, integration); + expect( + health.status, + `a transient vault outage must not permanently invalidate the connection: ${JSON.stringify(health)}`, + ).toBe("healthy"); + }), + Effect.gen(function* () { + yield* Effect.promise(() => workos.faults.clear()); + yield* removeEverything({ client, integration, oauthClient }); + }), + ); + }), +); From 8a89dd561bcc18b0699aeedf84f9024ebeb4b9ac Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Wed, 8 Jul 2026 19:54:43 -0700 Subject: [PATCH 2/4] Gate OAuth token refresh on credential-store writability A vault write failure while persisting a refreshed OAuth token lost the rotated refresh token: the grant had already consumed the single-use stored token at the authorization server, so once the vault recovered the next refresh replayed the revoked token, got invalid_grant, and the connection demanded a re-auth over what was only a storage blip. Before consuming the refresh token, rewrite the stored value in place as a writability probe. A store outage now fails the resolve before the grant, leaving the stored token valid so the connection recovers with the store. --- e2e/cloud/vault-write-outage.test.ts | 43 +++++++++++++++------------- packages/core/sdk/src/executor.ts | 11 +++++++ 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/e2e/cloud/vault-write-outage.test.ts b/e2e/cloud/vault-write-outage.test.ts index 4c4c85809f..49e3d39e0c 100644 --- a/e2e/cloud/vault-write-outage.test.ts +++ b/e2e/cloud/vault-write-outage.test.ts @@ -7,30 +7,31 @@ // 1. An OAuth connection's access token expires (per-client TTL of 1s via // the emulator's DCR `access_token_ttl_seconds` extension, well inside // the 60s refresh skew). -// 2. A health check resolves the connection → triggers the refresh-token -// grant. The grant SUCCEEDS and the authorization server ROTATES the -// refresh token (single use — the emulator mirrors AuthKit). -// 3. Persisting the new tokens does read → `PUT /vault/v1/kv/:id`. The PUT -// fails (in prod: 409 version conflict, an OAuth-shaped 400, or an HTML -// error page; here: the armed 400 with `error`/`error_description`, the -// exact shape the WorkOS SDK maps to OauthException — Sentry issue -53). -// 4. The write failure surfaces as `StorageError: WorkOS Vault secret write +// 2. A health check resolves the connection → the refresh path runs. Vault +// writes go through read → `PUT /vault/v1/kv/:id`; the PUT fails (in +// prod: 409 version conflict, an OAuth-shaped 400, or an HTML error +// page; here: the armed 400 with `error`/`error_description`, the exact +// shape the WorkOS SDK maps to OauthException — Sentry issue -53). +// 3. The write failure surfaces as `StorageError: WorkOS Vault secret write // failed` → the health endpoint answers a typed InternalError (the 500 -// Sentry records). +// Sentry records). Crucially it must fail BEFORE the refresh-token grant +// (the AS rotates the single-use token; consuming it with an unwritable +// store loses the rotated copy forever). // -// The second scenario pins the DAMAGE, not just the surface error: the vault -// write failed AFTER the refresh token was consumed and rotated, so the new -// refresh token is lost and the stored one is revoked. Once the vault -// recovers, the connection must recover too — today it cannot (the next -// refresh gets invalid_grant → the connection reads "expired" and demands a -// re-auth). Red until the refresh/persist ordering is made crash-safe. +// The second scenario pins the DAMAGE the surface error used to hide: when +// the vault write failed AFTER the refresh token was consumed and rotated, +// the rotated token was lost and the stored one already revoked — the next +// refresh got invalid_grant and the connection demanded a re-auth over a +// storage blip. The fix gates the grant on a proof-of-writability rewrite of +// the stored refresh token, so a vault outage fails BEFORE the single-use +// token is spent and the connection recovers with the vault. import { randomBytes } from "node:crypto"; import { expect } from "@effect/vitest"; import { Effect } from "effect"; import type { HttpApiClient } from "effect/unstable/httpapi"; import { composePluginApi } from "@executor-js/api/server"; -import { connectEmulator, type EmulatorClient } from "@executor-js/emulate"; +import { connectEmulator } from "@executor-js/emulate"; import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; import { AuthTemplateSlug, @@ -243,13 +244,15 @@ scenario( ).toBe("InternalError"); // The proof this is the production mechanism and not an incidental - // failure: the product performed the refresh grant and then hit the - // injected fault on the vault update. + // failure: the product hit the injected fault on the vault update leg. const ledger = yield* Effect.promise(() => workos.ledger.list(200)); const faultedPut = ledger.find((entry) => entry.faulted === true && entry.method === "PUT"); expect(faultedPut?.path, "the failure came from the injected vault-update fault").toMatch( /\/vault\/v1\/kv\//, ); + // The crash-safety contract: the write-gate fails BEFORE the refresh + // grant, so the single-use refresh token is never consumed while the + // store cannot persist its rotated successor. const refreshGrant = ledger.find( (entry) => entry.path.endsWith("/oauth2/token") && @@ -257,8 +260,8 @@ scenario( ); expect( refreshGrant, - "the vault write that failed was persisting a completed refresh grant", - ).toBeDefined(); + "no refresh grant is issued while the vault cannot persist the rotated token", + ).toBeUndefined(); }), Effect.gen(function* () { yield* Effect.promise(() => workos.faults.clear()); diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 5f470a70e9..15030d0337 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -1570,6 +1570,17 @@ export const createExecutor = Date: Thu, 27 Aug 2026 22:50:55 -0700 Subject: [PATCH 3/4] Fold the store-writability gate into the credential-persistence suite Main now persists the rotated refresh token before the access token, so the ordering half of this failure is fixed. The writability half is not: when the store refuses writes outright, the grant has already spent the stored token and there is nowhere to put its successor. Keep the pre-flight rewrite that proves the store is writable before the grant, move its scenario into the existing credential-write-durability suite rather than standing up a second file, and drop the prose that still claimed the persist ordering was wrong. Skip the persist's own refresh-token write when the authorization server hands back an unrotated token, so the probe removes a vault write on the common path instead of adding one. --- .../oauth-refresh-store-writability-gate.md | 11 + e2e/cloud/credential-write-durability.test.ts | 76 ++++- e2e/cloud/vault-write-outage.test.ts | 316 ------------------ packages/core/sdk/src/executor.ts | 51 ++- 4 files changed, 127 insertions(+), 327 deletions(-) create mode 100644 .changeset/oauth-refresh-store-writability-gate.md delete mode 100644 e2e/cloud/vault-write-outage.test.ts diff --git a/.changeset/oauth-refresh-store-writability-gate.md b/.changeset/oauth-refresh-store-writability-gate.md new file mode 100644 index 0000000000..4290036d9b --- /dev/null +++ b/.changeset/oauth-refresh-store-writability-gate.md @@ -0,0 +1,11 @@ +--- +"executor": patch +--- + +**A credential-store outage no longer costs an OAuth connection its grant** + +Refreshing an OAuth token spends the stored refresh token: the authorization server rotates it, so the copy we sent stops working the moment the grant succeeds and the rotated one is the only thing that can mint again. Persisting the rotated token first bounds what a partial write can lose, but it cannot help when the store is refusing writes outright — the grant has already run, there is nowhere to put the successor, and every later refresh replays a token the server has revoked. The connection then reports `invalid_grant` and demands a re-auth over what was only a storage blip. + +The refresh is now gated on a store that is proven writable. Before the grant runs, the refresh token just read is written back to its own item; a store that cannot take that write fails the resolve while the stored token is still valid, so the connection recovers on its own once the store does. + +The probe also removes a write rather than adding one in the common case. Authorization servers that do not rotate hand back the same refresh token, and that value is no longer re-persisted when it has not changed — a rotated token never matches, so the write that matters still happens. diff --git a/e2e/cloud/credential-write-durability.test.ts b/e2e/cloud/credential-write-durability.test.ts index 589b050db4..eb339f28c2 100644 --- a/e2e/cloud/credential-write-durability.test.ts +++ b/e2e/cloud/credential-write-durability.test.ts @@ -1,6 +1,6 @@ // Cloud: a refreshed OAuth credential has to be PERSISTED, and persisting it is // a pair of version-checked writes into WorkOS Vault — the rotated refresh -// token and the new access token, one after the other, not atomically. Two +// token and the new access token, one after the other, not atomically. Three // production failures live in that gap. // // 1. Contention. Two concurrent probes of one connection each run a refresh and @@ -14,6 +14,11 @@ // token we sent, so nothing can mint again and every later use of the // connection comes back `invalid_grant`. The access token, by contrast, is // disposable — one more grant re-mints it. +// 3. Writability. Ordering bounds the damage but cannot remove it: when the +// store refuses writes outright, a grant that has already run has spent the +// stored refresh token and there is nowhere to put its successor. The +// refresh must therefore be gated on a store that is proven writable BEFORE +// the grant, so a storage outage costs the user nothing but the wait. // // Both are pinned here at the product surface, black box. Failures are armed on // the WorkOS emulator that the product's own WorkOS client talks to; no product @@ -568,3 +573,72 @@ scenario( }), ), ); + +scenario( + "Credential persistence · a store that cannot accept writes fails the refresh before the grant is spent", + {}, + Effect.scoped( + Effect.gen(function* () { + const { attempt, call, oauth, slug, upstream, workos } = yield* connectIntegration; + + yield* call("baseline"); + + // Break the REFRESH TOKEN's object, and only that one, with a status the + // write policy cannot treat as contention — 503 is the store being down, + // not a peer holding the row, so no amount of retrying can land it. This + // is a store that will not accept a write at all. + const objects = yield* vaultObjectsFor(workos, slug); + const refreshObject = objects.find((object) => object.name.endsWith("refresh")); + expect(refreshObject, "the connection stored a refresh token in the vault").toBeDefined(); + + const grantsBeforeOutage = (yield* oauth.requests).filter(isRefreshGrant).length; + + const duringOutage = yield* Effect.scoped( + Effect.gen(function* () { + const armed = yield* armFault(workos, { + match: { method: "PUT", pathPattern: `/vault/v1/kv/${refreshObject!.id}` }, + response: { status: 503, body: { code: "unavailable", message: "vault unavailable" } }, + times: 3, + }); + + upstream.revokeSeenBearers(); + const result = yield* attempt(); + expect( + yield* faultsServed(workos, armed), + "the refresh token's write is the one that broke", + ).toBeGreaterThanOrEqual(1); + return result; + }), + ); + expect( + duringOutage.ok, + `the call reports the failure while the store is down (got: ${duringOutage.text.slice(0, 200)})`, + ).toBe(false); + + // The whole point: the failure landed on the writability check, not on + // the persist that follows a grant. No grant ran, so the stored refresh + // token was never spent and is still the one the authorization server + // will honour. + expect( + (yield* oauth.requests).filter(isRefreshGrant).length, + "no refresh grant is spent while the store cannot persist its rotated successor", + ).toBe(grantsBeforeOutage); + + // The store is back. A transient outage must cost nothing but the wait: + // the connection still holds a live grant and refreshes itself, with no + // human reconnecting anything. + const recovered = yield* attempt(); + expect( + recovered.ok, + `the connection refreshes itself once the store recovers (got: ${recovered.text.slice(0, 400)})`, + ).toBe(true); + expect((JSON.parse(recovered.text) as ToolEnvelope).ok, "the recovered call succeeded").toBe( + true, + ); + expect( + (yield* oauth.requests).filter(isRefreshGrant).length, + "the recovery was a real refresh grant on the token the outage preserved", + ).toBe(grantsBeforeOutage + 1); + }), + ), +); diff --git a/e2e/cloud/vault-write-outage.test.ts b/e2e/cloud/vault-write-outage.test.ts deleted file mode 100644 index 49e3d39e0c..0000000000 --- a/e2e/cloud/vault-write-outage.test.ts +++ /dev/null @@ -1,316 +0,0 @@ -// Cloud: reproduce the production "StorageError: WorkOS Vault secret write -// failed" Sentry events (NODE-CLOUDFLARE-WORKERS-52/53/4T) at the real -// upstream — faults armed on the WorkOS emulator's Vault KV PUT, the same -// emulator the product's real WorkOS SDK talks to. No product code touched. -// -// The production chain, staged here end-to-end: -// 1. An OAuth connection's access token expires (per-client TTL of 1s via -// the emulator's DCR `access_token_ttl_seconds` extension, well inside -// the 60s refresh skew). -// 2. A health check resolves the connection → the refresh path runs. Vault -// writes go through read → `PUT /vault/v1/kv/:id`; the PUT fails (in -// prod: 409 version conflict, an OAuth-shaped 400, or an HTML error -// page; here: the armed 400 with `error`/`error_description`, the exact -// shape the WorkOS SDK maps to OauthException — Sentry issue -53). -// 3. The write failure surfaces as `StorageError: WorkOS Vault secret write -// failed` → the health endpoint answers a typed InternalError (the 500 -// Sentry records). Crucially it must fail BEFORE the refresh-token grant -// (the AS rotates the single-use token; consuming it with an unwritable -// store loses the rotated copy forever). -// -// The second scenario pins the DAMAGE the surface error used to hide: when -// the vault write failed AFTER the refresh token was consumed and rotated, -// the rotated token was lost and the stored one already revoked — the next -// refresh got invalid_grant and the connection demanded a re-auth over a -// storage blip. The fix gates the grant on a proof-of-writability rewrite of -// the stored refresh token, so a vault outage fails BEFORE the single-use -// token is spent and the connection recovers with the vault. -import { randomBytes } from "node:crypto"; - -import { expect } from "@effect/vitest"; -import { Effect } from "effect"; -import type { HttpApiClient } from "effect/unstable/httpapi"; -import { composePluginApi } from "@executor-js/api/server"; -import { connectEmulator } from "@executor-js/emulate"; -import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; -import { - AuthTemplateSlug, - ConnectionName, - IntegrationSlug, - OAuthClientSlug, -} from "@executor-js/sdk/shared"; - -import { scenario } from "../src/scenario"; -import { Api, Target } from "../src/services"; -import type { Target as TargetShape } from "../src/target"; -import { WORKOS_EMULATOR_PORT } from "../targets/cloud"; - -const api = composePluginApi([openApiHttpPlugin()] as const); -type Client = HttpApiClient.ForApi; - -const TEMPLATE = AuthTemplateSlug.make("oauth2"); -const CONNECTION = ConnectionName.make("main"); -const WORKOS_EMULATOR_URL = `http://127.0.0.1:${WORKOS_EMULATOR_PORT}`; - -const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`; - -/** Minimal OpenAPI spec with a single GET /ping — never contacted (the - * integration declares no health-check spec, so checkHealth takes the - * credential-resolution path, which is where the vault write lives). */ -const pingSpec = JSON.stringify({ - openapi: "3.0.3", - info: { title: "Ping API", version: "1.0.0" }, - paths: { - "/ping": { - get: { operationId: "ping", summary: "Ping", responses: { "200": { description: "pong" } } }, - }, - }, -}); - -// The vault-write outage: `PUT /vault/v1/kv/:id` (the WorkOS SDK's -// updateObject) starts answering with the OAuth-shaped 400 the production -// events carry ("OauthException: Error: Invalid request parameters" — the -// SDK maps any non-{401,404,409,422,429} status whose body has -// `error`/`error_description` to OauthException). Reads (GET) and creates -// (POST) stay healthy: only the update leg fails, as in production. -// `times` bounds the blast radius; the finalizer clears whatever remains. -const VAULT_UPDATE_FAULT = { - match: { method: "PUT", pathPattern: "/vault/v1/kv/*" }, - response: { - status: 400, - body: { - code: "invalid_request", - message: "Invalid request parameters", - error: "Invalid request parameters", - error_description: "Invalid request parameters", - }, - }, - times: 8, -} as const; - -/** DCR-register an OAuth client directly on the WorkOS emulator with a 1s - * access-token TTL (per-client, so the emulator's default — which the - * product's own AuthKit sessions depend on — is untouched). 1s is inside - * executor's 60s refresh skew: the first resolve after connect refreshes. */ -const registerShortLivedOAuthClient = (redirectUri: string) => - Effect.promise(async () => { - const response = await fetch(`${WORKOS_EMULATOR_URL}/oauth2/register`, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - client_name: "vault-write-outage-e2e", - redirect_uris: [redirectUri], - access_token_ttl_seconds: 1, - }), - }); - if (response.status !== 201) { - throw new Error(`WorkOS emulator DCR failed: ${response.status}`); - } - const body = (await response.json()) as { readonly client_id: string }; - return body.client_id; - }); - -/** Complete the emulator's authorize hop headlessly: `login_hint` makes the - * authorize endpoint 302 straight back with a code (no consent page). */ -const completeConsent = (authorizationUrl: string, email: string) => - Effect.promise(async () => { - const url = new URL(authorizationUrl); - url.searchParams.set("login_hint", email); - const callback = await fetch(url, { redirect: "manual" }); - const location = callback.headers.get("location"); - if (callback.status !== 302 || !location) { - throw new Error(`WorkOS emulator authorize did not redirect: ${callback.status}`); - } - const code = new URL(location).searchParams.get("code"); - if (!code) throw new Error("WorkOS emulator callback did not include a code"); - return code; - }); - -/** Register a fresh integration + OAuth app against the emulator's generic - * authorize/token endpoints and connect it. `offline_access` mints a - * refresh token, so the expired-token path refreshes instead of re-authing. */ -const connectExpiringOAuthConnection = (input: { - readonly client: Client; - readonly target: TargetShape; - readonly integration: IntegrationSlug; - readonly oauthClient: OAuthClientSlug; -}) => - Effect.gen(function* () { - const redirectUri = new URL("/api/oauth/callback", input.target.baseUrl).toString(); - - yield* input.client.openapi.addSpec({ - payload: { - spec: { kind: "blob", value: pingSpec }, - slug: input.integration, - baseUrl: "http://127.0.0.1:59999", // never contacted - authenticationTemplate: [ - { - slug: "oauth2", - kind: "oauth2", - authorizationUrl: `${WORKOS_EMULATOR_URL}/oauth2/authorize`, - tokenUrl: `${WORKOS_EMULATOR_URL}/oauth2/token`, - scopes: ["offline_access"], - }, - ], - }, - }); - - const clientId = yield* registerShortLivedOAuthClient(redirectUri); - yield* input.client.oauth.createClient({ - payload: { - owner: "org", - slug: input.oauthClient, - grant: "authorization_code", - authorizationUrl: `${WORKOS_EMULATOR_URL}/oauth2/authorize`, - tokenUrl: `${WORKOS_EMULATOR_URL}/oauth2/token`, - clientId, - // The emulator's DCR clients are public (auth method "none"); the - // secret is carried but never validated. - clientSecret: "unused", - originIntegration: input.integration, - }, - }); - - const started = yield* input.client.oauth.start({ - payload: { - client: input.oauthClient, - clientOwner: "org", - owner: "org", - name: CONNECTION, - integration: input.integration, - template: TEMPLATE, - redirectUri, - }, - }); - expect(started.status, "OAuth starts with an emulator redirect").toBe("redirect"); - if (started.status !== "redirect") return yield* Effect.die("OAuth unexpectedly connected"); - - const code = yield* completeConsent(started.authorizationUrl, "vault-outage@example.com"); - const completed = yield* input.client.oauth.complete({ - payload: { state: started.state, code }, - }); - expect(completed.integration, "OAuth completion creates the connection").toBe( - input.integration, - ); - }); - -const removeEverything = (input: { - readonly client: Client; - readonly integration: IntegrationSlug; - readonly oauthClient: OAuthClientSlug; -}) => - Effect.gen(function* () { - yield* input.client.connections - .remove({ params: { owner: "org", integration: input.integration, name: CONNECTION } }) - .pipe(Effect.ignore); - yield* input.client.oauth - .removeClient({ params: { slug: input.oauthClient }, payload: { owner: "org" } }) - .pipe(Effect.ignore); - yield* input.client.openapi - .removeSpec({ params: { slug: input.integration } }) - .pipe(Effect.ignore); - }); - -const checkHealth = (client: Client, integration: IntegrationSlug) => - client.connections.checkHealth({ - params: { owner: "org", integration, name: CONNECTION }, - query: { ifStaleMs: 0 }, - }); - -scenario( - "Vault · a vault write failure during token refresh surfaces as the internal storage error", - {}, - Effect.gen(function* () { - const target = yield* Target; - const { client: makeClient } = yield* Api; - const identity = yield* target.newIdentity(); - const client = yield* makeClient(api, identity); - const integration = IntegrationSlug.make(unique("vault_outage")); - const oauthClient = OAuthClientSlug.make(unique("vault_outage_app")); - const workos = yield* Effect.promise(() => connectEmulator({ baseUrl: WORKOS_EMULATOR_URL })); - - yield* Effect.ensuring( - Effect.gen(function* () { - yield* connectExpiringOAuthConnection({ client, target, integration, oauthClient }); - - // The 1s token is already inside the refresh skew. Break the vault's - // update leg, then run the health check that resolves the credential. - yield* Effect.promise(() => workos.faults.arm(VAULT_UPDATE_FAULT)); - - const error = yield* checkHealth(client, integration).pipe(Effect.flip); - expect( - (error as { _tag?: string })._tag, - "the failed vault write surfaces as the typed internal error (the prod 500)", - ).toBe("InternalError"); - - // The proof this is the production mechanism and not an incidental - // failure: the product hit the injected fault on the vault update leg. - const ledger = yield* Effect.promise(() => workos.ledger.list(200)); - const faultedPut = ledger.find((entry) => entry.faulted === true && entry.method === "PUT"); - expect(faultedPut?.path, "the failure came from the injected vault-update fault").toMatch( - /\/vault\/v1\/kv\//, - ); - // The crash-safety contract: the write-gate fails BEFORE the refresh - // grant, so the single-use refresh token is never consumed while the - // store cannot persist its rotated successor. - const refreshGrant = ledger.find( - (entry) => - entry.path.endsWith("/oauth2/token") && - JSON.stringify(entry.request.body ?? "").includes("refresh_token"), - ); - expect( - refreshGrant, - "no refresh grant is issued while the vault cannot persist the rotated token", - ).toBeUndefined(); - }), - Effect.gen(function* () { - yield* Effect.promise(() => workos.faults.clear()); - yield* removeEverything({ client, integration, oauthClient }); - }), - ); - }), -); - -scenario( - "Vault · a transient vault write outage does not invalidate the connection once the vault recovers", - {}, - Effect.gen(function* () { - const target = yield* Target; - const { client: makeClient } = yield* Api; - const identity = yield* target.newIdentity(); - const client = yield* makeClient(api, identity); - const integration = IntegrationSlug.make(unique("vault_recover")); - const oauthClient = OAuthClientSlug.make(unique("vault_recover_app")); - const workos = yield* Effect.promise(() => connectEmulator({ baseUrl: WORKOS_EMULATOR_URL })); - - yield* Effect.ensuring( - Effect.gen(function* () { - yield* connectExpiringOAuthConnection({ client, target, integration, oauthClient }); - - // One health check during the outage: the refresh grant consumes and - // ROTATES the single-use refresh token, then the vault write of the - // rotated pair fails. - yield* Effect.promise(() => workos.faults.arm(VAULT_UPDATE_FAULT)); - yield* checkHealth(client, integration).pipe(Effect.flip); - yield* Effect.promise(() => workos.faults.clear()); - - // The vault has recovered. A transient storage blip must not cost the - // user their grant: the connection has a live authorization at the AS - // and must come back healthy without a re-auth. Today it cannot — the - // rotated refresh token was never persisted, so the stored (revoked) - // one is replayed, the AS answers invalid_grant, and the connection - // reads "expired" until a human reconnects. This is the real damage - // behind the prod Sentry events. - const health = yield* checkHealth(client, integration); - expect( - health.status, - `a transient vault outage must not permanently invalidate the connection: ${JSON.stringify(health)}`, - ).toBe("healthy"); - }), - Effect.gen(function* () { - yield* Effect.promise(() => workos.faults.clear()); - yield* removeEverything({ client, integration, oauthClient }); - }), - ); - }), -); diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 07463d1e3a..b18db3019b 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -1956,11 +1956,21 @@ export const createExecutor = => Effect.gen(function* () { if (provider.set) { @@ -1969,7 +1979,11 @@ export const createExecutor = Date: Thu, 27 Aug 2026 23:26:04 -0700 Subject: [PATCH 4/4] Probe store writability on a dedicated item, not the refresh token The writability gate proved the store by rewriting the refresh token with the value it had just read. That is a read-then-write with no compare-and-set, so two instances refreshing one connection lose the newer token: one reads the stored token, the other spends it and stores the rotated replacement, and the first then writes the spent one back over it. The in-flight refresh gate is per instance and cannot see the peer. The gate now writes a fixed value to an item of its own, derived from the refresh item's id so it lands in the same store partition and proves the same thing. It holds no credential, so nothing is at risk when two refreshers race on it. --- .../oauth-refresh-store-writability-gate.md | 4 +- e2e/cloud/credential-write-durability.test.ts | 30 +++- packages/core/sdk/src/executor.ts | 44 +++-- packages/core/sdk/src/oauth-flow.test.ts | 166 +++++++++++++++++- packages/core/sdk/src/oauth-service.ts | 28 +++ packages/core/sdk/src/provider-item-owner.ts | 7 +- 6 files changed, 251 insertions(+), 28 deletions(-) diff --git a/.changeset/oauth-refresh-store-writability-gate.md b/.changeset/oauth-refresh-store-writability-gate.md index 4290036d9b..23d6994d30 100644 --- a/.changeset/oauth-refresh-store-writability-gate.md +++ b/.changeset/oauth-refresh-store-writability-gate.md @@ -6,6 +6,8 @@ Refreshing an OAuth token spends the stored refresh token: the authorization server rotates it, so the copy we sent stops working the moment the grant succeeds and the rotated one is the only thing that can mint again. Persisting the rotated token first bounds what a partial write can lose, but it cannot help when the store is refusing writes outright — the grant has already run, there is nowhere to put the successor, and every later refresh replays a token the server has revoked. The connection then reports `invalid_grant` and demands a re-auth over what was only a storage blip. -The refresh is now gated on a store that is proven writable. Before the grant runs, the refresh token just read is written back to its own item; a store that cannot take that write fails the resolve while the stored token is still valid, so the connection recovers on its own once the store does. +The refresh is now gated on a store that is proven writable. Before the grant runs, it writes a fixed value to an item of its own that holds no credential and sits in the same partition as the connection's tokens. A store that cannot take that write fails the resolve while the stored refresh token is still valid, so the connection recovers on its own once the store does. + +The probe deliberately does not test the store by rewriting the refresh token with the value it just read. That is a read-then-write with no compare-and-set, and two instances refreshing one connection would lose the newer token to it: one reads the stored token, the other spends that same token and stores its rotated replacement, and the first then writes the spent one back over the replacement. The connection would die exactly the way the gate is meant to prevent. The probe also removes a write rather than adding one in the common case. Authorization servers that do not rotate hand back the same refresh token, and that value is no longer re-persisted when it has not changed — a rotated token never matches, so the write that matters still happens. diff --git a/e2e/cloud/credential-write-durability.test.ts b/e2e/cloud/credential-write-durability.test.ts index eb339f28c2..3353513cdc 100644 --- a/e2e/cloud/credential-write-durability.test.ts +++ b/e2e/cloud/credential-write-durability.test.ts @@ -18,7 +18,10 @@ // store refuses writes outright, a grant that has already run has spent the // stored refresh token and there is nowhere to put its successor. The // refresh must therefore be gated on a store that is proven writable BEFORE -// the grant, so a storage outage costs the user nothing but the wait. +// the grant, so a storage outage costs the user nothing but the wait. The +// gate writes an object of its own that holds no credential — proving the +// store on the refresh token's own object would mean writing a value read +// moments earlier, which is how a peer's rotated token gets overwritten. // // Both are pinned here at the product surface, black box. Failures are armed on // the WorkOS emulator that the product's own WorkOS client talks to; no product @@ -583,20 +586,33 @@ scenario( yield* call("baseline"); - // Break the REFRESH TOKEN's object, and only that one, with a status the - // write policy cannot treat as contention — 503 is the store being down, - // not a peer holding the row, so no amount of retrying can land it. This - // is a store that will not accept a write at all. + // One healthy refresh first, so the object the writability gate uses + // exists and can be named. That object is the point of this scenario: + // the gate proves the store on an item of its OWN, never by rewriting + // the refresh token it is about to spend. + upstream.revokeSeenBearers(); + yield* call("pre-outage-refresh"); + const objects = yield* vaultObjectsFor(workos, slug); const refreshObject = objects.find((object) => object.name.endsWith("refresh")); expect(refreshObject, "the connection stored a refresh token in the vault").toBeDefined(); + const probeObject = objects.find((object) => object.name.endsWith("store-probe")); + expect( + probeObject, + "the writability gate wrote an object of its own, not the refresh token's", + ).toBeDefined(); + expect(probeObject!.id, "and it is a distinct object").not.toBe(refreshObject!.id); const grantsBeforeOutage = (yield* oauth.requests).filter(isRefreshGrant).length; + // Break that object, and only that one, with a status the write policy + // cannot treat as contention — 503 is the store being down, not a peer + // holding the row, so no amount of retrying can land it. This is a store + // that will not accept a write at all. const duringOutage = yield* Effect.scoped( Effect.gen(function* () { const armed = yield* armFault(workos, { - match: { method: "PUT", pathPattern: `/vault/v1/kv/${refreshObject!.id}` }, + match: { method: "PUT", pathPattern: `/vault/v1/kv/${probeObject!.id}` }, response: { status: 503, body: { code: "unavailable", message: "vault unavailable" } }, times: 3, }); @@ -605,7 +621,7 @@ scenario( const result = yield* attempt(); expect( yield* faultsServed(workos, armed), - "the refresh token's write is the one that broke", + "the write the gate makes is the one that broke", ).toBeGreaterThanOrEqual(1); return result; }), diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index b18db3019b..675ded62d0 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -105,6 +105,8 @@ import type { } from "./integration"; import { makeOAuthService, + STORE_WRITABILITY_PROBE_VALUE, + storeWritabilityProbeItemIdFor, type MintOAuthConnectionInput, type OAuthScopePolicy, } from "./oauth-service"; @@ -1958,14 +1960,14 @@ export const createExecutor = { ), ); + // Two product instances, one connection, one credential store. The in-flight + // refresh gate serialises refreshes WITHIN an instance and cannot see across + // them, so nothing but the store itself stands between two refreshers and + // the same rotated token. The provider below opens a seam exactly where the + // danger is — between the read of the stored refresh token and whatever the + // reader writes next — because a probe that "tests" the store by rewriting + // the value it just read would put the spent token back over the peer's + // rotated one, and kill the connection it was added to protect. + it.effect( + "a refresher paused after reading the stored token never writes it back over a peer's rotated one", + () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + + const store = new Map(); + // Every item id written, in order — the tape that says WHICH item a + // refresher touched, which is the whole question here. + const writes: string[] = []; + const pausedAtRead = yield* Deferred.make(); + const resumeFromRead = yield* Deferred.make(); + // One-shot: the FIRST read of a refresh token stops there; the + // peer's read, moments later, runs straight through. + let pauseNextRefreshRead = false; + + const sharedStore: CredentialProvider = { + key: ProviderKey.make("shared-memory"), + writable: true, + get: (id) => + Effect.gen(function* () { + const value = store.get(String(id)) ?? null; + if (pauseNextRefreshRead && String(id).endsWith(":refresh")) { + pauseNextRefreshRead = false; + yield* Deferred.succeed(pausedAtRead, undefined); + yield* Deferred.await(resumeFromRead); + } + return value; + }), + set: (id, value) => + Effect.sync(() => { + writes.push(String(id)); + store.set(String(id), value); + }), + delete: (id) => Effect.sync(() => void store.delete(String(id))), + }; + + // One database and one credential store, two executors over them — + // the deployment this race needs and the one a single harness + // cannot express. + const config = { + ...makeTestConfig({ plugins: [oauthPlugin] as const }), + providers: [sharedStore], + }; + const instanceA = yield* createExecutor(config); + const instanceB = yield* createExecutor(config); + yield* Effect.addFinalizer(() => + Effect.promise(() => config.testDb.close()).pipe(Effect.ignore), + ); + yield* Effect.addFinalizer(() => instanceA.close().pipe(Effect.ignore)); + yield* Effect.addFinalizer(() => instanceB.close().pipe(Effect.ignore)); + + yield* instanceA.acme.seed(); + yield* instanceA.oauth.createClient({ + owner: "org", + slug: CLIENT, + authorizationUrl: server.authorizationEndpoint, + tokenUrl: server.tokenEndpoint, + grant: "authorization_code", + clientId: "test-client", + clientSecret: "test-secret", + }); + const started = yield* instanceA.oauth.start({ + owner: "org", + client: CLIENT, + clientOwner: "org", + name: ConnectionName.make("main"), + integration: INTEG, + template: TEMPLATE, + }); + expect(started.status).toBe("redirect"); + if (started.status !== "redirect") return; + const callback = yield* server.completeAuthorizationCodeFlow({ + authorizationUrl: started.authorizationUrl, + }); + yield* instanceA.oauth.complete({ state: started.state, code: callback.code }); + + const refreshItemId = [...store.keys()].find((key) => key.endsWith(":refresh")); + expect(refreshItemId, "the completed connection stored a refresh token").toBeDefined(); + const spentRefreshToken = store.get(refreshItemId!); + + // Expire the access token so both instances must refresh. + yield* Effect.promise(() => + config.db.updateMany("connection", { + where: (b) => b("name", "=", "main"), + set: { expires_at: Date.now() - 60_000 }, + }), + ); + + // A begins a refresh and stops the instant it has the stored refresh + // token in hand. This is the window. + pauseNextRefreshRead = true; + const refresherA = yield* Effect.forkChild( + Effect.exit(instanceA.execute(ToolAddress.make("tools.acme.org.main.whoami"), {})), + ); + yield* Deferred.await(pausedAtRead); + + // B refreshes on that same token, to completion. The authorization + // server rotates it, so what the store holds afterwards is the only + // value left that can ever mint again. + yield* instanceB.execute(ToolAddress.make("tools.acme.org.main.whoami"), {}); + const rotatedByB = store.get(refreshItemId!); + expect(rotatedByB, "the peer's refresh rotated the stored token").not.toBe( + spentRefreshToken, + ); + const writesBeforeAResumes = writes.length; + + // A resumes into a world where the token it is holding is already + // spent — and still has to prove the store is writable before it + // tries to spend it. + yield* Deferred.succeed(resumeFromRead, undefined); + yield* Fiber.join(refresherA); + + expect( + store.get(refreshItemId!), + "the peer's rotated refresh token is still what the store holds", + ).toBe(rotatedByB); + expect( + [...store.entries()] + .filter(([, value]) => value === spentRefreshToken) + .map(([key]) => key), + "the spent refresh token was not written back anywhere", + ).toEqual([]); + + // The gate did still run for A — on an item of its own, holding no + // credential. Its grant then failed on the spent token, so it + // persisted nothing: this one write is everything A wrote. + const writtenByA = writes.slice(writesBeforeAResumes); + expect(writtenByA, "the resumed refresher wrote exactly one item").toHaveLength(1); + expect(writtenByA[0], "and it was not the refresh token's own item").not.toBe( + refreshItemId, + ); + expect( + [spentRefreshToken, rotatedByB], + "the item it wrote carries no credential", + ).not.toContain(store.get(writtenByA[0]!)); + + // Both instances really did reach the authorization server, so the + // interleaving under test happened rather than being short-circuited. + expect( + (yield* server.requests).filter( + (request) => + request.path === "/token" && request.body.includes("grant_type=refresh_token"), + ), + "both instances sent a refresh grant", + ).toHaveLength(2); + }), + ), + ); + it.effect( "refreshes a Personal (user) connection minted through a Workspace (org) app — own→shared client resolution", () => diff --git a/packages/core/sdk/src/oauth-service.ts b/packages/core/sdk/src/oauth-service.ts index ad35a07aa1..b853a8eac0 100644 --- a/packages/core/sdk/src/oauth-service.ts +++ b/packages/core/sdk/src/oauth-service.ts @@ -270,6 +270,34 @@ const accessItemId = (owner: Owner, integration: IntegrationSlug, name: Connecti `oauth:${owner}:${integration}:${name}`; const refreshItemIdFor = (accessId: string): string => `${accessId}:refresh`; +/** The item a refresh writes to prove the credential store will ACCEPT a write, + * before the grant spends the single-use refresh token. It holds no credential + * and never has. + * + * It has to be its own item. The cheaper-looking probe — rewriting the refresh + * token with the value just read — is a read-then-write with no + * compare-and-set, and two refreshers of one connection on different instances + * lose the newer token to it: A reads R0, B consumes R0 and stores the rotated + * R1, then A's probe puts R0 back over R1 and the connection is dead the next + * time anything needs it. The in-memory single-flight gate spans one instance + * only, and a backing store whose own write path is read-latest-then-write + * cannot catch it either. + * + * The id is the refresh item's id plus a fixed suffix, rather than one + * rebuilt from the connection's parts, so it carries the same prefix and + * therefore the same embedded owner — the same store partition, the same + * encryption context, the same object-name head. A store that would refuse + * the refresh token's write refuses this one. Per connection rather than one + * per partition, so the only writers that can contend on it are the + * concurrent refreshers of a single connection, and they all write the same + * constant. */ +export const storeWritabilityProbeItemIdFor = (refreshItemId: string): string => + `${refreshItemId}:store-probe`; + +/** What the writability probe stores. A constant, because the item exists to + * prove a write lands and carries no information of its own. */ +export const STORE_WRITABILITY_PROBE_VALUE = "writable"; + /** Order-preserving de-duplication of a scope list. */ const dedupeScopes = (scopes: readonly string[]): readonly string[] => [...new Set(scopes)]; diff --git a/packages/core/sdk/src/provider-item-owner.ts b/packages/core/sdk/src/provider-item-owner.ts index 74e857fdb1..f4513faef5 100644 --- a/packages/core/sdk/src/provider-item-owner.ts +++ b/packages/core/sdk/src/provider-item-owner.ts @@ -4,9 +4,14 @@ // ids in executor.ts, the oauth-client secret ids): // // connection:::: -// oauth:::[:refresh] +// oauth:::[:refresh[:store-probe]] // oauth-client:::secret // +// The `:store-probe` tail is the one id here that holds no credential: a +// refresh writes it to prove the store accepts writes before a grant spends +// the refresh token. It hangs off the refresh id so it inherits the same +// owner, and therefore the same partition, as the credential it stands in for. +// // Credential providers file plugin-storage rows by THIS owner, not the acting // caller's binding — an org connection whose OAuth consent completes in one // member's browser session must produce rows every member can resolve