feat(sdk): let a credential provider own the OAuth refresh grant

A provider that serves an indirection instead of a raw value can protect an
access token: that token's only use is to be sent to a bound host, and the
reply is not itself a credential. The refresh grant breaks that. The exchange
needs the real refresh token, and the authorization server's reply carries a
brand-new real access token, so serving an indirection here moves the exposure
one step later while appearing to remove it. Providers backed by a sealed store
have to refuse the refresh item outright today, which costs them refresh
entirely.

Add an optional `refreshGrant` to CredentialProvider so such a provider can own
the exchange instead: it spends the refresh token, seals the new access token
(and a rotated refresh token) under the same item ids, and returns only
`{ expiresAt, scope }`. The caller then reads the access token back through
`get`, the same hop every other credential already takes.

Absence is not a downgrade: when the method is missing the existing host-side
exchange runs unchanged. client_credentials is excluded deliberately - it has
no refresh token to spend. Secrets are named by item id, never passed as
values, since passing them would reintroduce the exposure this removes.

The test pins the custody property directly - that the host never resolves the
refresh token through the provider - rather than asserting the refresh
succeeded, because a provider that quietly served the token would also go
green.

Author: GeiserX

packages/core/sdk/src/executor.ts

@@ -1867,6 +1867,62 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
           ? String(row.oauth_token_url)
           : String(clientRow.token_url);
 
+        // OAuth is always single-input: the access token lives in the `token`
+        // item. Fall back to a deterministic id if the map is somehow empty.
+        const tokenItemId = ProviderItemId.make(
+          connectionItemIds(row)[PRIMARY_INPUT_VARIABLE] ??
+            `connection:${row.owner}:${row.integration}:${row.name}:${PRIMARY_INPUT_VARIABLE}`,
+        );
+
+        // Shared by both grant paths so their bookkeeping cannot drift apart.
+        // `scope` is written only when the authorization server reported one —
+        // an unreported scope must leave the recorded scope alone rather than
+        // clearing it.
+        const recordRefreshOutcome = (expiresAt: number | null, scope: string | undefined) =>
+          Effect.gen(function* () {
+            const set: Record<string, unknown> = { expires_at: expiresAt, updated_at: new Date() };
+            if (scope !== undefined) set.oauth_scope = scope;
+            yield* core.updateMany("connection", {
+              where: (b: AnyCb) =>
+                b.and(
+                  byOwner(owner)(b),
+                  b("integration", "=", String(row.integration)),
+                  b("name", "=", String(row.name)),
+                ),
+              set,
+            });
+          });
+
+        // A provider that can perform the grant itself owns the whole exchange:
+        // it spends the refresh token, seals the newly minted tokens under the
+        // same item ids, and tells us only when they expire and what scope was
+        // granted. We then read the access token back through `get`, which is
+        // the same hop every other credential already takes — so the refresh
+        // path stops being the one place that hands a plaintext token upward.
+        //
+        // client_credentials is excluded deliberately: it has no refresh token
+        // to spend (the token is re-minted from the client id/secret), so it is
+        // a different exchange and is left on the path below.
+        if (provider.refreshGrant && String(clientRow.grant) !== "client_credentials") {
+          if (!row.refresh_item_id) {
+            return yield* reauth("No refresh token is stored for this connection.");
+          }
+          const granted = yield* provider.refreshGrant({
+            refreshItemId: ProviderItemId.make(String(row.refresh_item_id)),
+            accessItemId: tokenItemId,
+            clientSecretItemId: clientRow.client_secret_item_id
+              ? ProviderItemId.make(String(clientRow.client_secret_item_id))
+              : undefined,
+            tokenUrl,
+            clientId: String(clientRow.client_id),
+            scopes: grantedScopes,
+            // RFC 8707: keep the re-minted token bound to the same resource.
+            resource: clientRow.resource ? String(clientRow.resource) : undefined,
+          });
+          yield* recordRefreshOutcome(granted.expiresAt, granted.scope ?? undefined);
+          return yield* provider.get(tokenItemId);
+        }
+
         // client_credentials (machine-to-machine) has NO refresh token — the
         // token is RE-MINTED from the client id/secret. The authorization_code
         // path below needs a stored refresh token. Branching on grant here is
@@ -1959,33 +2015,15 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
               });
 
         if (provider.set) {
-          // OAuth is always single-input: the access token lives in the `token`
-          // item. Fall back to a deterministic id if the map is somehow empty.
-          const tokenItemId =
-            connectionItemIds(row)[PRIMARY_INPUT_VARIABLE] ??
-            `connection:${row.owner}:${row.integration}:${row.name}:${PRIMARY_INPUT_VARIABLE}`;
-          yield* provider.set(ProviderItemId.make(tokenItemId), token.access_token);
+          yield* provider.set(tokenItemId, token.access_token);
           if (token.refresh_token && row.refresh_item_id) {
             yield* provider.set(ProviderItemId.make(row.refresh_item_id), token.refresh_token);
           }
         }
 
         const nextExpiresAt =
           typeof token.expires_in === "number" ? Date.now() + token.expires_in * 1000 : null;
-        const set: Record<string, unknown> = {
-          expires_at: nextExpiresAt,
-          updated_at: new Date(),
-        };
-        if (token.scope !== undefined) set.oauth_scope = token.scope;
-        yield* core.updateMany("connection", {
-          where: (b: AnyCb) =>
-            b.and(
-              byOwner(owner)(b),
-              b("integration", "=", String(row.integration)),
-              b("name", "=", String(row.name)),
-            ),
-          set,
-        });
+        yield* recordRefreshOutcome(nextExpiresAt, token.scope);
 
         return token.access_token;
       }).pipe(

packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts

@@ -0,0 +1,185 @@
+import { describe, expect, it } from "@effect/vitest";
+import { Effect } from "effect";
+
+import {
+  AuthTemplateSlug,
+  ConnectionName,
+  IntegrationSlug,
+  OAuthClientSlug,
+  ProviderItemId,
+  ProviderKey,
+  ToolAddress,
+  ToolName,
+} from "./ids";
+import { definePlugin } from "./plugin";
+import type { CredentialProvider, RefreshGrantInput } from "./provider";
+import { makeTestWorkspaceHarness } from "./test-config";
+import { serveOAuthTestServer } from "./testing/oauth-test-server";
+
+// A provider that OWNS the refresh grant never hands the refresh token out. These tests pin that
+// property directly rather than asserting "the refresh succeeded" — success is not the claim. The
+// claim is that the host never resolved the secret, and only a test watching `get` can tell a
+// provider that protected the token from one that quietly served it. Both would go green.
+
+const INTEG = IntegrationSlug.make("acme");
+const TEMPLATE = AuthTemplateSlug.make("oauth");
+const CLIENT = OAuthClientSlug.make("acme-app");
+
+const oauthPlugin = definePlugin(() => ({
+  id: "acme" as const,
+  storage: () => ({}),
+  resolveTools: () =>
+    Effect.succeed({ tools: [{ name: ToolName.make("whoami"), description: "whoami" }] }),
+  describeAuthMethods: (record) => {
+    const config = record.config as { readonly scopes?: readonly string[] } | null;
+    return [
+      {
+        id: "oauth",
+        label: "OAuth2",
+        kind: "oauth" as const,
+        template: String(TEMPLATE),
+        oauth: { scopes: config?.scopes ?? [] },
+      },
+    ];
+  },
+  invokeTool: ({ credential }) => Effect.succeed({ token: credential.value }),
+  extension: (ctx) => ({
+    seed: (scopes: readonly string[] = []) =>
+      ctx.core.integrations.register({ slug: INTEG, description: "Acme", config: { scopes } }),
+  }),
+}))();
+
+/** Records what the host asked the provider for, so a test can assert what it did NOT ask for. */
+interface Recorder {
+  readonly reads: string[];
+  readonly grants: RefreshGrantInput[];
+}
+
+/** A memory provider that can also perform the refresh grant itself.
+ *
+ * `refreshGrant` seals a new access token under `accessItemId`, exactly as a sealed-store provider
+ * would, and returns only expiry and scope. It never calls `get`. */
+const delegatingCredentialsPlugin = (recorder: Recorder, withGrant: boolean) =>
+  definePlugin(() => {
+    const store = new Map<string, string>();
+
+    const base = {
+      key: ProviderKey.make("memory"),
+      writable: true as const,
+      get: (id: ProviderItemId) =>
+        Effect.sync(() => {
+          recorder.reads.push(String(id));
+          return store.get(String(id)) ?? null;
+        }),
+      set: (id: ProviderItemId, value: string) =>
+        Effect.sync(() => {
+          store.set(String(id), value);
+        }),
+      delete: (id: ProviderItemId) =>
+        Effect.sync(() => {
+          store.delete(String(id));
+        }),
+    };
+
+    const provider: CredentialProvider = withGrant
+      ? {
+          ...base,
+          refreshGrant: (input: RefreshGrantInput) =>
+            Effect.sync(() => {
+              recorder.grants.push(input);
+              store.set(String(input.accessItemId), "delegated-access-token");
+              return { expiresAt: Date.now() + 3_600_000, scope: "read" };
+            }),
+        }
+      : base;
+
+    return {
+      id: "memory-credentials" as const,
+      storage: () => ({}),
+      credentialProviders: [provider],
+    };
+  })();
+
+describe("provider-owned OAuth refresh grant", () => {
+  const scenario = (withGrant: boolean) =>
+    Effect.gen(function* () {
+      const recorder: Recorder = { reads: [], grants: [] };
+      const server = yield* serveOAuthTestServer({ scopes: ["read"] });
+      const plugins = [delegatingCredentialsPlugin(recorder, withGrant), oauthPlugin] as const;
+      const { executor, config } = yield* makeTestWorkspaceHarness({ plugins });
+      yield* executor.acme.seed();
+
+      yield* executor.oauth.createClient({
+        owner: "org",
+        slug: CLIENT,
+        authorizationUrl: server.authorizationEndpoint,
+        tokenUrl: server.tokenEndpoint,
+        grant: "authorization_code",
+        clientId: "test-client",
+        clientSecret: "test-secret",
+      });
+
+      const started = yield* executor.oauth.start({
+        owner: "org",
+        client: CLIENT,
+        clientOwner: "org",
+        name: ConnectionName.make("main"),
+        integration: INTEG,
+        template: TEMPLATE,
+      });
+      // Assert-then-return rather than throwing: this is Effect domain code, and the repo's lint
+      // forbids constructing or throwing built-in Errors here. A failed expectation already fails
+      // the test, so the early return only satisfies the type.
+      expect(started.status).toBe("redirect");
+      if (started.status !== "redirect") return { recorder, server };
+      const callback = yield* server.completeAuthorizationCodeFlow({
+        authorizationUrl: started.authorizationUrl,
+      });
+      yield* executor.oauth.complete({ state: started.state, code: callback.code });
+
+      // Force the next resolve down the refresh path.
+      yield* Effect.promise(() =>
+        config.db.updateMany("connection", {
+          where: (b) => b("name", "=", "main"),
+          set: { expires_at: Date.now() - 60_000 },
+        }),
+      );
+
+      recorder.reads.length = 0;
+      recorder.grants.length = 0;
+      yield* executor.execute(ToolAddress.make("tools.acme.org.main.whoami"), {});
+      return { recorder, server };
+    });
+
+  it.effect("delegates the grant and never resolves the refresh token through the host", () =>
+    Effect.scoped(
+      Effect.gen(function* () {
+        const { recorder, server } = yield* scenario(true);
+
+        // The grant was delegated, and named by id rather than handed a value.
+        expect(recorder.grants).toHaveLength(1);
+        const grant = recorder.grants[0]!;
+        expect(String(grant.refreshItemId)).toContain(":refresh");
+        expect(grant.tokenUrl).toBe(server.tokenEndpoint);
+
+        // THE CUSTODY CLAIM. If this ever fails, the host is asking for the secret again and the
+        // guarantee is gone — while the refresh itself still appears to work.
+        expect(recorder.reads.some((id) => id.endsWith(":refresh"))).toBe(false);
+      }),
+    ),
+  );
+
+  it.effect("falls back to the host-side exchange when the provider cannot do the grant", () =>
+    Effect.scoped(
+      Effect.gen(function* () {
+        const { recorder } = yield* scenario(false);
+
+        // Absence of `refreshGrant` changes nothing: the host performs the exchange, so it DOES
+        // resolve the refresh token. Pinning that here is what makes the test above meaningful —
+        // it shows the difference is the provider capability, not the harness.
+        expect(recorder.grants).toHaveLength(0);
+        expect(recorder.reads.some((id) => id.endsWith(":refresh"))).toBe(true);
+      }),
+    ),
+  );
+});

packages/core/sdk/src/provider.ts

@@ -32,4 +32,63 @@ export interface CredentialProvider {
   /** Browse entries for discovery (pick a 1Password item). Optional — some
    *  backends can't enumerate. */
   readonly list?: () => Effect.Effect<readonly ProviderEntry[], StorageFailure>;
+  /** Perform the OAuth refresh grant inside the provider, instead of handing the
+   *  refresh token out to be exchanged here.
+   *
+   *  WHY THIS EXISTS. A provider that hides values behind an indirection can
+   *  protect an access token, because that token's only use is to be sent to a
+   *  bound host and the reply is not itself a credential. The refresh grant
+   *  breaks that: the exchange needs the real refresh token AND the authorization
+   *  server's reply carries a brand-new real access token, so a provider that
+   *  serves indirection here only moves the exposure one step later while
+   *  appearing to have removed it. Providers backed by a sealed store therefore
+   *  have to refuse the refresh item outright — the honest option, but it costs
+   *  them refresh entirely.
+   *
+   *  Implementing this gives them the other option: own the exchange, seal the
+   *  new tokens under the same item ids, and return only what the caller needs to
+   *  update its bookkeeping. The caller then resolves the access token through
+   *  `get`, exactly as it resolves every other credential.
+   *
+   *  OPTIONAL, and absence is not a downgrade: when it is missing the caller
+   *  performs the exchange itself, unchanged. Implement it only if the exchange
+   *  genuinely happens somewhere the host cannot read — returning success without
+   *  performing the grant is worse than not implementing it. */
+  readonly refreshGrant?: (
+    input: RefreshGrantInput,
+  ) => Effect.Effect<RefreshGrantResult, StorageFailure>;
+}
+
+/** What the provider needs to perform the grant on the caller's behalf.
+ *
+ *  Secrets are named by ITEM ID, never passed as values — passing the refresh
+ *  token or the client secret here would reintroduce exactly the exposure this
+ *  interface exists to remove. */
+export interface RefreshGrantInput {
+  /** The stored refresh token to spend. */
+  readonly refreshItemId: ProviderItemId;
+  /** Where to seal the newly minted access token. The caller reads it back from
+   *  here through `get`. */
+  readonly accessItemId: ProviderItemId;
+  /** The OAuth app's client secret, by id. Absent for a public client. */
+  readonly clientSecretItemId?: ProviderItemId;
+  readonly tokenUrl: string;
+  readonly clientId: string;
+  readonly scopes: readonly string[];
+  /** RFC 8707 — keeps the re-minted token bound to the same resource. */
+  readonly resource?: string;
+}
+
+/** Deliberately carries NO token material.
+ *
+ *  These two fields are the whole of what the caller needs to update a
+ *  connection row after a refresh; anything more would put the host back in the
+ *  data path. A rotated refresh token is sealed by the provider under the same
+ *  `refreshItemId` and is never reported here. */
+export interface RefreshGrantResult {
+  /** Epoch millis, or null when the authorization server did not say. */
+  readonly expiresAt: number | null;
+  /** The granted scope as reported by the authorization server, or null when it
+   *  did not report one (distinct from an empty scope). */
+  readonly scope: string | null;
 }