Gate OAuth token refresh on credential store writability

.changeset/oauth-refresh-store-writability-gate.md

@@ -0,0 +1,13 @@
+---
+"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, 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.

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,14 @@
 //     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. 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
@@ -568,3 +576,85 @@ 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");
+
+      // 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/${probeObject!.id}` },
+            response: { status: 503, body: { code: "unavailable", message: "vault unavailable" } },
+            times: 3,
+          });
+
+          upstream.revokeSeenBearers();
+          const result = yield* attempt();
+          expect(
+            yield* faultsServed(workos, armed),
+            "the write the gate makes 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);
+    }),
+  ),
+);

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";
@@ -1956,11 +1958,21 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
      *  re-mints it. Persisting the access token first means a failure in
      *  between drops a single-use credential the authorization server has
      *  already consumed, and every later refresh comes back `invalid_grant` —
-     *  a connection that silently disconnects itself. */
+     *  a connection that silently disconnects itself.
+     *
+     *  `storedRefreshToken` is the value the store already held when the
+     *  caller read it on the way in. Many authorization servers do NOT rotate
+     *  on refresh and hand back the very same refresh token, so writing it
+     *  again is a round trip that can only re-persist what is already there —
+     *  and every write bumps the stored object's version, which is the
+     *  contention this path spends retries fighting. Skip it when the value
+     *  has not changed; a rotated token never matches, so the write that
+     *  actually matters is never skipped. */
     const persistRefreshedToken = (
       row: ConnectionRow,
       provider: CredentialProvider,
       token: OAuth2TokenResponse,
+      storedRefreshToken?: string | undefined,
     ): Effect.Effect<void, StorageFailure> =>
       Effect.gen(function* () {
         if (provider.set) {
@@ -1969,7 +1981,11 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
           const tokenItemId =
             connectionItemIds(row)[PRIMARY_INPUT_VARIABLE] ??
             `connection:${row.owner}:${row.integration}:${row.name}:${PRIMARY_INPUT_VARIABLE}`;
-          if (token.refresh_token && row.refresh_item_id) {
+          if (
+            token.refresh_token &&
+            row.refresh_item_id &&
+            token.refresh_token !== storedRefreshToken
+          ) {
             yield* provider.set(ProviderItemId.make(row.refresh_item_id), token.refresh_token);
           }
           yield* provider.set(ProviderItemId.make(tokenItemId), token.access_token);
@@ -2243,6 +2259,12 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
         // path below needs a stored refresh token. Branching on grant here is
         // what keeps a client_credentials connection (e.g. DealCloud) from
         // demanding a re-auth on a credential that has no human to re-auth.
+        // What the credential store held for this connection when the grant
+        // below was prepared, so the persist can tell a ROTATED refresh token
+        // from one the authorization server simply handed back unchanged.
+        // Only the authorization_code path has one; client_credentials and
+        // id_jag carry no refresh token at all.
+        let storedRefreshToken: string | undefined;
         const token =
           clientRow.grant === "client_credentials"
             ? yield* exchangeClientCredentials({
@@ -2275,6 +2297,36 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
                 if (!refreshToken) {
                   return yield* reauth("Stored refresh token could not be resolved.");
                 }
+                // Prove the credential store is WRITABLE before consuming the
+                // single-use refresh token. Ordering the persist correctly
+                // (refresh token first) bounds the damage once a grant has
+                // run, but it cannot help when the store is refusing writes
+                // outright: the grant spends the stored token at the
+                // authorization server, so a store that cannot accept the
+                // rotated successor leaves the connection holding a token the
+                // server has already revoked. Every later refresh then replays
+                // it, gets invalid_grant, and a storage outage that healed in
+                // minutes has cost the user a re-auth. Failing here instead
+                // leaves the stored token valid, so the connection recovers on
+                // its own when the store does.
+                //
+                // The probe writes its OWN item and never the refresh token's.
+                // Rewriting the value just read would be one round trip
+                // cheaper and is the trap: it is a read-then-write with no
+                // compare-and-set, so a peer refresher on another instance
+                // that spent this same token and stored its rotated successor
+                // in between would have that successor overwritten by the
+                // stale value — the exact dead connection this gate exists to
+                // prevent, now caused by the gate. The probe item sits in the
+                // same partition and holds a constant, so it proves what the
+                // store will accept while no credential is ever at risk.
+                if (provider.set) {
+                  yield* provider.set(
+                    ProviderItemId.make(storeWritabilityProbeItemIdFor(row.refresh_item_id)),
+                    STORE_WRITABILITY_PROBE_VALUE,
+                  );
+                }
+                storedRefreshToken = refreshToken;
                 return yield* refreshAccessToken({
                   tokenUrl,
                   clientId: clientRow.clientId,
@@ -2354,7 +2406,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
                 );
               });
 
-        yield* persistRefreshedToken(row, provider, token);
+        yield* persistRefreshedToken(row, provider, token, storedRefreshToken);
         return token.access_token;
       }).pipe(
         // The refresh path was previously invisible to telemetry: no span, no