From b2fe257b5bc3afaea9e6689f34bc4e1ae6de1834 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Wed, 12 Aug 2026 01:38:43 +0200 Subject: [PATCH 1/5] 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. --- packages/core/sdk/src/executor.ts | 78 ++++++-- .../oauth-refresh-grant-delegation.test.ts | 185 ++++++++++++++++++ packages/core/sdk/src/provider.ts | 59 ++++++ 3 files changed, 302 insertions(+), 20 deletions(-) create mode 100644 packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 56cb2e2997..f236c017f1 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -1867,6 +1867,62 @@ export const createExecutor = + Effect.gen(function* () { + const set: Record = { 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,12 +2015,7 @@ export const createExecutor = = { - 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( diff --git a/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts b/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts new file mode 100644 index 0000000000..e616b91fde --- /dev/null +++ b/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(); + + 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); + }), + ), + ); +}); diff --git a/packages/core/sdk/src/provider.ts b/packages/core/sdk/src/provider.ts index 42a3defa4a..31b2df8748 100644 --- a/packages/core/sdk/src/provider.ts +++ b/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; + /** 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; +} + +/** 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; } From a767adc1e3a0e78f8627f2a7c1dadcdadb910ff7 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Wed, 12 Aug 2026 10:21:04 +0200 Subject: [PATCH 2/5] Harden the provider-owned OAuth refresh grant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review of the delegated refresh path: the fast path returned before the machinery that classifies an authorization-server refusal, so a provider that owned the grant lost re-auth entirely. - Report a refusal with a typed `RefreshGrantRejected` carrying the RFC 6749 §5.2 code. Both grant paths now share one classifier and one known-dead gate, so a delegated refresh surfaces `invalid_grant` to the caller and stops re-sending a doomed grant on every proactive cycle. - Resolve the client secret BELOW the delegated branch. It was read in plaintext and then never used, which both defeated the point of passing `clientSecretItemId` and meant a store that seals that item failed the refresh before `refreshGrant` was ever reached. - Read the new access token back before recording success, and fail when it cannot be resolved, instead of stamping a fresh expiry over a token nobody can read. - Enforce the host's OAuth endpoint URL policy on the delegated path too. - Report `expiresInSeconds` rather than an absolute instant, so the caller converts on the clock that later decides whether the token is due. - Carry `clientAuth` so a provider never has to guess the client authentication method. - Export `RefreshGrantInput`, `RefreshGrantResult` and `RefreshGrantRejected` from the package entry point; an external provider could not name them. - Add a changeset, and cover the refusal, read-back, scope, expiry and client_credentials paths. --- .../provider-owned-oauth-refresh-grant.md | 11 + packages/core/sdk/src/executor.ts | 160 +++++++----- packages/core/sdk/src/index.ts | 8 +- .../oauth-refresh-grant-delegation.test.ts | 234 +++++++++++++++--- packages/core/sdk/src/provider.ts | 77 ++++-- 5 files changed, 376 insertions(+), 114 deletions(-) create mode 100644 .changeset/provider-owned-oauth-refresh-grant.md diff --git a/.changeset/provider-owned-oauth-refresh-grant.md b/.changeset/provider-owned-oauth-refresh-grant.md new file mode 100644 index 0000000000..e5ed2ea501 --- /dev/null +++ b/.changeset/provider-owned-oauth-refresh-grant.md @@ -0,0 +1,11 @@ +--- +"executor": minor +--- + +**Credential providers can now own the OAuth refresh grant** + +`CredentialProvider` gains an optional `refreshGrant`. When a provider implements it, the host asks it to _perform_ the refresh exchange rather than to hand over the refresh token: the provider spends the token, seals the newly minted access token (and a rotated refresh token, if the authorization server sent one) under the same item ids, and returns only the granted lifetime and scope. The host then resolves the access token through `get`, the same hop every other credential takes. + +This closes the one gap where a backend that keeps secrets sealed had no honest option but to refuse to refresh at all — the refresh grant is the only exchange where a long-lived stored secret must be spent and the reply is itself a fresh credential. Providers that do not implement `refreshGrant` are unaffected: the existing host-side exchange runs unchanged. + +A refused grant is reported with the new `RefreshGrantRejected` error carrying the RFC 6749 §5.2 code, so a delegated refresh classifies re-authentication, surfaces `invalid_grant` to the caller, and arms the known-dead gate exactly as the host-side path does. diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index f236c017f1..c17f3d6915 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -163,6 +163,7 @@ import { collectReferencedDefinitions } from "./schema-refs"; import { refreshAccessToken, exchangeClientCredentials, + isSupportedOAuthEndpointUrl, shouldRefreshToken, type OAuthEndpointUrlPolicy, } from "./oauth-helpers"; @@ -1848,11 +1849,6 @@ export const createExecutor = + cause.error !== undefined + ? new CredentialResolutionError({ + owner, + integration: IntegrationSlug.make(row.integration), + name: ConnectionName.make(row.name), + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: the refusal carries a typed `message` + message: `OAuth token refresh was rejected (${cause.error}): ${cause.message}`, + reauthRequired: cause.error === "invalid_grant", + oauthErrorCode: cause.error, + }) + : new StorageError({ + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: the refusal carries a typed `message` + message: `OAuth token refresh failed: ${cause.message}`, + cause, + }); + + // Persist the definitive verdict so the NEXT refresh skips the doomed + // grant (see the known-dead gate above) and the connection shows + // `expired` without waiting for a probe. Shared for the same reason as + // the classifier: a delegating provider that armed no gate would re-send + // a dead grant on every proactive cycle, forever. + const armKnownDeadGate = ( + error: CredentialResolutionError | StorageFailure, + ): Effect.Effect => + Predicate.isTagged(error, "CredentialResolutionError") && error.reauthRequired === true + ? // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: CredentialResolutionError carries a typed `message` field + markRefreshGrantDead(row, error.message) + : Effect.void; + // 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 @@ -1907,22 +1945,64 @@ export const createExecutor = + Effect.fail(classifyGrantRefusal(cause)), + ), + Effect.tapError(armKnownDeadGate), + ); + // Read the token back BEFORE recording success. A provider that + // reported a grant it did not actually seal would otherwise leave the + // row stamped with a fresh expiry over a stale or absent token, and + // the connection would read healthy for a whole token lifetime while + // every call using it failed. + const access = yield* provider.get(tokenItemId); + if (!access) { + return yield* reauth("Refreshed access token could not be resolved."); + } + // Convert on OUR clock, never the provider's — `shouldRefreshToken` + // compares the stored instant against this same clock, so an absolute + // instant computed on a remote machine would import its skew. + yield* recordRefreshOutcome( + granted.expiresInSeconds === null ? null : Date.now() + granted.expiresInSeconds * 1000, + granted.scope ?? undefined, + ); + return access; } + // The secret is stored in the provider (a vault item id), not inline. + // Resolved BELOW the delegated branch: a store that seals this item + // would fail the whole refresh here, before the provider that can do the + // grant without ever revealing it is even consulted. + const clientSecret = clientRow.client_secret_item_id + ? ((yield* provider.get(ProviderItemId.make(String(clientRow.client_secret_item_id)))) ?? + "") + : ""; + // 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 @@ -1971,47 +2051,7 @@ export const createExecutor = { - // An RFC 6749 §5.2 error code is the AS's definitive - // verdict on this grant — retrying cannot change it. - // invalid_grant means the refresh token itself is dead - // (re-auth required); every other code must still reach - // the caller as an auth failure, because a StorageError - // is scrubbed to "Internal tool error [id]" at the - // sandbox boundary (the Pylon prod regression: the AS - // rejected refreshes with a non-invalid_grant 400 and - // callers saw only the opaque defect). Code-less - // failures (transport blips, non-OAuth-shaped responses) - // stay StorageError so the next invoke retries. - if (cause.error !== undefined) { - return new CredentialResolutionError({ - owner, - integration: IntegrationSlug.make(row.integration), - name: ConnectionName.make(row.name), - // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message` - message: `OAuth token refresh was rejected (${cause.error}): ${cause.message}`, - reauthRequired: cause.error === "invalid_grant", - oauthErrorCode: cause.error, - }); - } - return new StorageError({ - // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message` - message: `OAuth token refresh failed: ${cause.message}`, - cause, - }); - }), - // Persist the definitive verdict so the NEXT refresh skips - // the doomed grant (see the known-dead gate above) and the - // connection shows `expired` without waiting for a probe. - Effect.tapError((error) => - Predicate.isTagged(error, "CredentialResolutionError") && - error.reauthRequired === true - ? // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: CredentialResolutionError carries a typed `message` field - markRefreshGrantDead(row, error.message) - : Effect.void, - ), - ); + }).pipe(Effect.mapError(classifyGrantRefusal), Effect.tapError(armKnownDeadGate)); }); if (provider.set) { diff --git a/packages/core/sdk/src/index.ts b/packages/core/sdk/src/index.ts index 9d3cc9f587..e91a2c00d3 100644 --- a/packages/core/sdk/src/index.ts +++ b/packages/core/sdk/src/index.ts @@ -102,7 +102,13 @@ export type { export type { Tool, ToolDef, ToolListFilter, ToolAnnotations } from "./tool"; // Credential providers. -export type { CredentialProvider, ProviderEntry } from "./provider"; +export { RefreshGrantRejected } from "./provider"; +export type { + CredentialProvider, + ProviderEntry, + RefreshGrantInput, + RefreshGrantResult, +} from "./provider"; // Public projections / detection. export { ToolSchemaView, IntegrationDetectionResult } from "./types"; diff --git a/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts b/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts index e616b91fde..095ab815de 100644 --- a/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts +++ b/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts @@ -12,7 +12,7 @@ import { ToolName, } from "./ids"; import { definePlugin } from "./plugin"; -import type { CredentialProvider, RefreshGrantInput } from "./provider"; +import { RefreshGrantRejected, type CredentialProvider, type RefreshGrantInput } from "./provider"; import { makeTestWorkspaceHarness } from "./test-config"; import { serveOAuthTestServer } from "./testing/oauth-test-server"; @@ -24,6 +24,7 @@ import { serveOAuthTestServer } from "./testing/oauth-test-server"; const INTEG = IntegrationSlug.make("acme"); const TEMPLATE = AuthTemplateSlug.make("oauth"); const CLIENT = OAuthClientSlug.make("acme-app"); +const TOOL = ToolAddress.make("tools.acme.org.main.whoami"); const oauthPlugin = definePlugin(() => ({ id: "acme" as const, @@ -49,6 +50,21 @@ const oauthPlugin = definePlugin(() => ({ }), }))(); +/** What the delegating provider does when the host asks it to perform the grant. */ +type GrantBehaviour = + /** The honest implementation: seal a new access token, report expiry and scope. */ + | { + readonly kind: "seals"; + readonly scope: string | null; + readonly expiresInSeconds: number | null; + } + /** Reports success but leaves nothing resolvable under `accessItemId`. */ + | { readonly kind: "sealsNothing" } + /** The authorization server refused the grant (RFC 6749 §5.2). */ + | { readonly kind: "rejected"; readonly error?: string }; + +const SEALS: GrantBehaviour = { kind: "seals", scope: "read", expiresInSeconds: 3_600 }; + /** Records what the host asked the provider for, so a test can assert what it did NOT ask for. */ interface Recorder { readonly reads: string[]; @@ -57,9 +73,10 @@ interface Recorder { /** 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) => + * `refreshGrant` seals under `accessItemId` exactly as a sealed-store provider would, and returns + * only expiry and scope. It never calls `get`. `behaviour: null` omits the capability entirely, + * which is how the fallback test shows the difference is the capability and not the harness. */ +const delegatingCredentialsPlugin = (recorder: Recorder, behaviour: GrantBehaviour | null) => definePlugin(() => { const store = new Map(); @@ -81,17 +98,36 @@ const delegatingCredentialsPlugin = (recorder: Recorder, withGrant: boolean) => }), }; - 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; + const provider: CredentialProvider = + behaviour === null + ? base + : { + ...base, + refreshGrant: (input: RefreshGrantInput) => + Effect.suspend(() => { + recorder.grants.push(input); + if (behaviour.kind === "rejected") { + return Effect.fail( + new RefreshGrantRejected({ + message: "the authorization server refused the grant", + error: behaviour.error, + }), + ); + } + if (behaviour.kind === "sealsNothing") { + // A provider reporting a grant it did not perform is out of contract. What the + // host CAN do is refuse to stamp the row healthy over a token it cannot read + // back, which is what this drives. + store.delete(String(input.accessItemId)); + return Effect.succeed({ expiresInSeconds: 3_600, scope: "read" }); + } + store.set(String(input.accessItemId), "delegated-access-token"); + return Effect.succeed({ + expiresInSeconds: behaviour.expiresInSeconds, + scope: behaviour.scope, + }); + }), + }; return { id: "memory-credentials" as const, @@ -101,20 +137,29 @@ const delegatingCredentialsPlugin = (recorder: Recorder, withGrant: boolean) => })(); describe("provider-owned OAuth refresh grant", () => { - const scenario = (withGrant: boolean) => + /** Connect, force the connection past expiry, and hand back the pieces a test asserts on. The + * tool is NOT invoked here — each test drives the refresh itself so it can assert on failure. */ + const scenario = (options: { + readonly behaviour: GrantBehaviour | null; + readonly grant?: "authorization_code" | "client_credentials"; + }) => Effect.gen(function* () { const recorder: Recorder = { reads: [], grants: [] }; const server = yield* serveOAuthTestServer({ scopes: ["read"] }); - const plugins = [delegatingCredentialsPlugin(recorder, withGrant), oauthPlugin] as const; + const plugins = [ + delegatingCredentialsPlugin(recorder, options.behaviour), + oauthPlugin, + ] as const; const { executor, config } = yield* makeTestWorkspaceHarness({ plugins }); yield* executor.acme.seed(); + const grant = options.grant ?? "authorization_code"; yield* executor.oauth.createClient({ owner: "org", slug: CLIENT, authorizationUrl: server.authorizationEndpoint, tokenUrl: server.tokenEndpoint, - grant: "authorization_code", + grant, clientId: "test-client", clientSecret: "test-secret", }); @@ -127,15 +172,20 @@ describe("provider-owned OAuth refresh grant", () => { 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 }); + + // `die` rather than `expect` — this is shared setup, not the assertion under test, and an + // expect inside a branch is what the repo's no-conditional-tests rule exists to stop. + if (grant === "authorization_code") { + if (started.status !== "redirect") { + return yield* Effect.die("expected a redirect-status OAuth start"); + } + const callback = yield* server.completeAuthorizationCodeFlow({ + authorizationUrl: started.authorizationUrl, + }); + yield* executor.oauth.complete({ state: started.state, code: callback.code }); + } else if (started.status !== "connected") { + return yield* Effect.die("expected client_credentials to connect without a redirect"); + } // Force the next resolve down the refresh path. yield* Effect.promise(() => @@ -147,24 +197,34 @@ describe("provider-owned OAuth refresh grant", () => { recorder.reads.length = 0; recorder.grants.length = 0; - yield* executor.execute(ToolAddress.make("tools.acme.org.main.whoami"), {}); - return { recorder, server }; + return { recorder, server, config, executor }; }); 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); + const { recorder, server, executor } = yield* scenario({ behaviour: SEALS }); + const out = yield* executor.execute(TOOL, {}); // 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); + expect(grant.clientAuth).toBe("body"); // 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); + + // The client secret is a long-lived credential too, and the provider was given its ID + // precisely so it need never be revealed. Resolving it anyway would leave a sealed store + // failing the refresh before `refreshGrant` was ever reached. + expect(recorder.reads.some((id) => id.includes("secret"))).toBe(false); + expect(grant.clientSecretItemId).toBeDefined(); + + // The token the tool ran with is the one the provider sealed. + expect(out).toEqual({ token: "delegated-access-token" }); }), ), ); @@ -172,7 +232,8 @@ describe("provider-owned OAuth refresh grant", () => { 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); + const { recorder, executor } = yield* scenario({ behaviour: null }); + yield* executor.execute(TOOL, {}); // 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 — @@ -182,4 +243,115 @@ describe("provider-owned OAuth refresh grant", () => { }), ), ); + + it.effect("records the expiry and scope the provider reported", () => + Effect.scoped( + Effect.gen(function* () { + const before = Date.now(); + const { config, executor } = yield* scenario({ behaviour: SEALS }); + yield* executor.execute(TOOL, {}); + + const row = yield* Effect.promise(() => + config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), + ); + // Converted against the HOST clock, so the stored instant is comparable with the one + // `shouldRefreshToken` later reads. A provider-computed absolute instant would import that + // machine's skew and either serve expired tokens or churn. + expect(Number(row?.expires_at)).toBeGreaterThanOrEqual(before + 3_600_000); + expect(Number(row?.expires_at)).toBeLessThanOrEqual(Date.now() + 3_600_000); + expect(row?.oauth_scope).toBe("read"); + }), + ), + ); + + it.effect("leaves the recorded scope alone when the provider reports none", () => + Effect.scoped( + Effect.gen(function* () { + const { config, executor } = yield* scenario({ + behaviour: { kind: "seals", scope: null, expiresInSeconds: null }, + }); + // Give the row a scope to preserve. Without a known prior value the assertion below cannot + // tell "left alone" from "cleared" — both would read null. + yield* Effect.promise(() => + config.db.updateMany("connection", { + where: (b) => b("name", "=", "main"), + set: { oauth_scope: "read" }, + }), + ); + yield* executor.execute(TOOL, {}); + + const row = yield* Effect.promise(() => + config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), + ); + // `null` scope means "the AS did not report one", which must not clear what was granted at + // connect time — distinct from an empty scope, which would. + expect(row?.oauth_scope).toBe("read"); + expect(row?.expires_at).toBeNull(); + }), + ), + ); + + it.effect("surfaces a refused grant as re-auth and arms the known-dead gate", () => + Effect.scoped( + Effect.gen(function* () { + const { config, executor, recorder } = yield* scenario({ + behaviour: { kind: "rejected", error: "invalid_grant" }, + }); + + const failure = yield* Effect.flip(executor.execute(TOOL, {})); + // Not a StorageError: that is scrubbed to "Internal tool error [id]" at the sandbox + // boundary, so the user would never be told to reconnect. + expect(JSON.stringify(failure)).toContain("invalid_grant"); + + const row = yield* Effect.promise(() => + config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), + ); + expect( + (row?.provider_state as { oauthReauthRequiredAt?: number } | null)?.oauthReauthRequiredAt, + ).toEqual(expect.any(Number)); + expect(row?.last_health).toMatchObject({ status: "expired" }); + + // The gate is armed, so the doomed grant is not re-sent on the next resolve. Without this + // a dead connection re-sends its dead grant on every proactive cycle, indefinitely. + recorder.grants.length = 0; + yield* Effect.flip(executor.execute(TOOL, {})); + expect(recorder.grants).toHaveLength(0); + }), + ), + ); + + it.effect("fails rather than reporting success when the new token cannot be read back", () => + Effect.scoped( + Effect.gen(function* () { + const { config, executor } = yield* scenario({ behaviour: { kind: "sealsNothing" } }); + + const failure = yield* Effect.flip(executor.execute(TOOL, {})); + expect(JSON.stringify(failure)).toContain("could not be resolved"); + + // The row must NOT have been stamped with a fresh expiry — doing that over a token nobody + // can resolve leaves the connection reading healthy for a full lifetime while every call + // using it fails. + const row = yield* Effect.promise(() => + config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), + ); + expect(Number(row?.expires_at)).toBeLessThan(Date.now()); + }), + ), + ); + + it.effect("leaves client_credentials on the host-side exchange", () => + Effect.scoped( + Effect.gen(function* () { + const { recorder, executor } = yield* scenario({ + behaviour: SEALS, + grant: "client_credentials", + }); + yield* executor.execute(TOOL, {}); + + // client_credentials has no refresh token to spend — the token is re-minted from the + // client id/secret — so it is a different exchange and must not be delegated. + expect(recorder.grants).toHaveLength(0); + }), + ), + ); }); diff --git a/packages/core/sdk/src/provider.ts b/packages/core/sdk/src/provider.ts index 31b2df8748..9a881c9f2e 100644 --- a/packages/core/sdk/src/provider.ts +++ b/packages/core/sdk/src/provider.ts @@ -1,4 +1,4 @@ -import type { Effect } from "effect"; +import { Data, type Effect } from "effect"; import type { StorageFailure } from "./fuma-runtime"; import type { ProviderItemId, ProviderKey } from "./ids"; @@ -35,28 +35,21 @@ export interface CredentialProvider { /** 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. + * A provider that serves an indirection can protect an access token: it is + * spent against 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 reply carries a brand-new one — so a store the host genuinely cannot + * read has to refuse the refresh item, losing refresh entirely. Implementing + * this gives it the other option: own the exchange, seal the new tokens under + * the same item ids, and report only what the caller's bookkeeping needs. * - * 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. */ + * OPTIONAL — when absent 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; + ) => Effect.Effect; } /** What the provider needs to perform the grant on the caller's behalf. @@ -72,8 +65,21 @@ export interface RefreshGrantInput { readonly accessItemId: ProviderItemId; /** The OAuth app's client secret, by id. Absent for a public client. */ readonly clientSecretItemId?: ProviderItemId; + /** The token endpoint to post to. + * + * SECURITY: this is the CALLER's view of the endpoint, and a caller whose + * process is part of your threat model is not a trustworthy source for it — + * a rewritten value turns the grant into an exfiltration of the very token + * this interface exists to seal. A provider whose whole purpose is to + * withhold the refresh token from the caller MUST pin the endpoint against a + * value it recorded when the item was sealed, and reject a mismatch. */ readonly tokenUrl: string; readonly clientId: string; + /** How to present the client secret: `"body"` is `client_secret_post`, + * `"basic"` is `client_secret_basic`. Passed explicitly so a provider never + * has to guess — RFC 6749 §2.3.1 prefers Basic, while this caller's default + * is post, so a guess would be wrong as often as right. */ + readonly clientAuth: "body" | "basic"; readonly scopes: readonly string[]; /** RFC 8707 — keeps the re-minted token bound to the same resource. */ readonly resource?: string; @@ -86,9 +92,36 @@ export interface RefreshGrantInput { * 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; + /** Lifetime in seconds (RFC 6749 §5.1 `expires_in`), or null when the + * authorization server did not say. + * + * RELATIVE, not an absolute instant, precisely because the provider may run + * where the caller cannot read — which usually means a different machine and + * therefore a different clock. The caller converts against its OWN clock, the + * same one that later decides whether the token is due for refresh. */ + readonly expiresInSeconds: 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; } + +/** The authorization server refused the grant. + * + * Distinct from `StorageFailure` because the two demand opposite responses: a + * storage failure is transient and worth retrying, whereas an RFC 6749 §5.2 + * refusal is the AS's standing verdict — `invalid_grant` in particular means + * the refresh token is dead and only re-authentication recovers it. Without + * this the caller cannot tell "the vault is down" from "this connection is + * finished", so it can neither prompt for re-auth nor stop re-sending a grant + * that will never succeed. + * + * The §5.2 error response carries no token material, so reporting the code + * costs nothing in custody. */ +export class RefreshGrantRejected extends Data.TaggedError("RefreshGrantRejected")<{ + readonly message: string; + /** The RFC 6749 §5.2 code (`invalid_grant`, `invalid_client`, …) when the + * token endpoint returned one. Omit it for a failure that carried no code — + * the caller then treats the failure as transient. */ + readonly error?: string; + readonly cause?: unknown; +}> {} From d358caafbbae56143f46383613c8e3ffd1b6ba1b Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Wed, 12 Aug 2026 10:32:37 +0200 Subject: [PATCH 3/5] Cover the endpoint-policy guard on the delegated refresh path --- .../oauth-refresh-grant-delegation.test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts b/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts index 095ab815de..1587b4db53 100644 --- a/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts +++ b/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts @@ -339,6 +339,28 @@ describe("provider-owned OAuth refresh grant", () => { ), ); + it.effect("refuses to delegate a grant to an endpoint the host's policy rejects", () => + Effect.scoped( + Effect.gen(function* () { + const { recorder, executor, config } = yield* scenario({ behaviour: SEALS }); + // The token URL is read from the connection row, so it is the caller's view of where the + // grant goes. Delegating the exchange must not delegate the guard: a provider holding a + // sealed refresh token would otherwise post it wherever this column pointed. + yield* Effect.promise(() => + config.db.updateMany("connection", { + where: (b) => b("name", "=", "main"), + set: { oauth_token_url: "http://evil.example/token" }, + }), + ); + + const failure = yield* Effect.flip(executor.execute(TOOL, {})); + expect(JSON.stringify(failure)).toContain("https:"); + // The point of the guard: the provider is never asked, so the sealed token never moves. + expect(recorder.grants).toHaveLength(0); + }), + ), + ); + it.effect("leaves client_credentials on the host-side exchange", () => Effect.scoped( Effect.gen(function* () { From c69cc529b7e51deb51ecc40ea6559a732687cd38 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:21:25 +0200 Subject: [PATCH 4/5] Contain the provider boundary on the delegated refresh grant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A credential provider is an external plugin boundary, so nothing it authors may reach host error channels, telemetry, or persisted connection state — those values can carry token responses and other secret material. - Close the rejection classification to the standards-defined set (RFC 6749 §5.2 plus RFC 8707 `invalid_target`) and validate it at runtime, so an unrecognised value cannot reach `oauthErrorCode`, span attributes or persisted health. Drop `message`/`cause` from `RefreshGrantRejected` entirely; Executor now emits fixed host-facing text carrying only the validated code. - Contain provider storage failures, synchronous throws, Effect defects, and throwing or stateful property getters — including on the capability itself, on the success object's fields, and on the post-grant read-back. Cancellation is still propagated as cancellation; only the provider-authored reasons are dropped. - Rebuild the persisted scope from the host's own recorded grant set rather than the provider's string. `oauth_scope` is replayed to the authorization server on the next refresh, so accepting it verbatim was a persisted provider-controlled channel. A scope outside the granted set fails the refresh (RFC 6749 §6: a refresh may narrow scope, never widen it). - Bound the reported lifetime to finite, non-negative and at most ten years, instead of stamping NaN/Infinity/negative straight into `expires_at`. - Treat an unresolvable read-back as a retryable provider-invariant failure rather than demanding re-authentication: the authorization server ACCEPTED the grant, so re-auth is the one remedy that cannot be required, and a rotated refresh token may already be sealed. - Re-export the contract from the promise and shared surfaces too, and generalise the security note from `tokenUrl` to the whole caller-authored input tuple. --- .../provider-owned-oauth-refresh-grant.md | 4 +- packages/core/sdk/src/executor.ts | 161 ++++- packages/core/sdk/src/index.ts | 7 +- .../oauth-refresh-grant-delegation.test.ts | 565 +++++++++++++++++- packages/core/sdk/src/promise.ts | 13 +- packages/core/sdk/src/provider.ts | 74 ++- packages/core/sdk/src/shared.ts | 13 +- 7 files changed, 763 insertions(+), 74 deletions(-) diff --git a/.changeset/provider-owned-oauth-refresh-grant.md b/.changeset/provider-owned-oauth-refresh-grant.md index e5ed2ea501..ea04529923 100644 --- a/.changeset/provider-owned-oauth-refresh-grant.md +++ b/.changeset/provider-owned-oauth-refresh-grant.md @@ -6,6 +6,6 @@ `CredentialProvider` gains an optional `refreshGrant`. When a provider implements it, the host asks it to _perform_ the refresh exchange rather than to hand over the refresh token: the provider spends the token, seals the newly minted access token (and a rotated refresh token, if the authorization server sent one) under the same item ids, and returns only the granted lifetime and scope. The host then resolves the access token through `get`, the same hop every other credential takes. -This closes the one gap where a backend that keeps secrets sealed had no honest option but to refuse to refresh at all — the refresh grant is the only exchange where a long-lived stored secret must be spent and the reply is itself a fresh credential. Providers that do not implement `refreshGrant` are unaffected: the existing host-side exchange runs unchanged. +This gives a backend that keeps secrets sealed a way to close the refresh gap instead of refusing refresh entirely — the refresh grant is the exchange where a long-lived stored secret must be spent and the reply is itself a fresh credential. The provider remains responsible for authenticating the complete caller-supplied grant tuple against independently trusted enrollment metadata before opening a secret. Providers that do not implement `refreshGrant` are unaffected: the existing host-side exchange runs unchanged. -A refused grant is reported with the new `RefreshGrantRejected` error carrying the RFC 6749 §5.2 code, so a delegated refresh classifies re-authentication, surfaces `invalid_grant` to the caller, and arms the known-dead gate exactly as the host-side path does. +A refused grant is reported with the new `RefreshGrantRejected` error carrying a closed standards-defined token-endpoint code (RFC 6749 §5.2 plus RFC 8707 `invalid_target`), so a delegated refresh classifies re-authentication, surfaces `invalid_grant` to the caller, and arms the known-dead gate exactly as the host-side path does. Free-form provider messages, causes, defects, and malformed result metadata stay inside the provider boundary; Executor generates fixed host-facing text and only persists validated lifetime/scope metadata. diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index c17f3d6915..5f27a67423 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -1,4 +1,4 @@ -import { Effect, Inspectable, Layer, Option, Predicate, Schema } from "effect"; +import { Cause, Effect, Inspectable, Layer, Option, Predicate, Schema } from "effect"; import { FetchHttpClient, type HttpClient } from "effect/unstable/http"; import { fumadb } from "@executor-js/fumadb"; import { memoryAdapter } from "@executor-js/fumadb/adapters/memory"; @@ -122,7 +122,13 @@ import { type ToolPolicy, type UpdateToolPolicyInput, } from "./policies"; -import type { CredentialProvider, ProviderEntry } from "./provider"; +import { + MAX_REFRESH_GRANT_EXPIRES_IN_SECONDS, + isRefreshGrantRejectionCode, + type CredentialProvider, + type ProviderEntry, + type RefreshGrantRejected, +} from "./provider"; import { touchSubject } from "./subject-registry"; import type { AnyPlugin, @@ -1898,7 +1904,7 @@ export const createExecutor = @@ -1918,6 +1924,31 @@ export const createExecutor = { + const reportedError = cause.error; + const error = isRefreshGrantRejectionCode(reportedError) ? reportedError : undefined; + return error !== undefined + ? new CredentialResolutionError({ + owner, + integration: IntegrationSlug.make(row.integration), + name: ConnectionName.make(row.name), + message: `OAuth token refresh was rejected (${error}).`, + reauthRequired: error === "invalid_grant", + oauthErrorCode: error, + }) + : new StorageError({ + message: "Credential provider could not complete OAuth token refresh.", + cause: undefined, + }); + }; + // Persist the definitive verdict so the NEXT refresh skips the doomed // grant (see the known-dead gate above) and the connection shows // `expired` without waiting for a probe. Shared for the same reason as @@ -1941,7 +1972,26 @@ export const createExecutor = + new StorageError({ + message: "Credential provider could not complete OAuth token refresh.", + cause: undefined, + }); + const preserveProviderInterruption = ( + cause: Cause.Cause, + ): Effect.Effect => + Effect.failCause(Cause.fromReasons(cause.reasons.filter(Cause.isInterruptReason))); + const delegatedRefreshGrant = + String(clientRow.grant) === "client_credentials" + ? undefined + : yield* Effect.suspend(() => Effect.succeed(provider.refreshGrant)).pipe( + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) + ? preserveProviderInterruption(cause) + : Effect.fail(providerRefreshFailure()), + ), + ); + if (delegatedRefreshGrant && String(clientRow.grant) !== "client_credentials") { if (!row.refresh_item_id) { return yield* reauth("No refresh token is stored for this connection."); } @@ -1953,8 +2003,8 @@ export const createExecutor = + delegatedRefreshGrant.call(provider, { refreshItemId: ProviderItemId.make(String(row.refresh_item_id)), accessItemId: tokenItemId, clientSecretItemId: clientRow.client_secret_item_id @@ -1968,21 +2018,95 @@ export const createExecutor = - Effect.fail(classifyGrantRefusal(cause)), - ), - Effect.tapError(armKnownDeadGate), - ); + }), + ).pipe( + // Project the success value while it is still inside the guarded provider boundary. + // Accessors on a remote/plugin object can throw, and an arbitrary scope string would + // otherwise be a direct channel into persisted host state. Rebuild scope exclusively + // from the host's already-trusted grant set. + Effect.flatMap((result) => + Effect.suspend(() => { + const expiresInSeconds = result.expiresInSeconds; + const reportedScope = result.scope; + if ( + expiresInSeconds !== null && + (typeof expiresInSeconds !== "number" || + !Number.isFinite(expiresInSeconds) || + expiresInSeconds < 0 || + expiresInSeconds > MAX_REFRESH_GRANT_EXPIRES_IN_SECONDS) + ) { + return Effect.fail(providerRefreshFailure()); + } + if (reportedScope !== null && typeof reportedScope !== "string") { + return Effect.fail(providerRefreshFailure()); + } + const trustedScopes = new Map(grantedScopes.map((scope) => [scope, scope])); + const reportedScopes = + reportedScope === null + ? null + : [...new Set(reportedScope.split(/\s+/).filter(Boolean))]; + if ( + reportedScopes !== null && + reportedScopes.some((scope) => !trustedScopes.has(scope)) + ) { + return Effect.fail(providerRefreshFailure()); + } + return Effect.succeed({ + expiresInSeconds, + scope: + reportedScopes === null + ? null + : reportedScopes.map((scope) => trustedScopes.get(scope)!).join(" "), + }); + }), + ), + // This is an external plugin boundary. Preserve cancellation, but discard every + // provider-authored failure/defect before it can reach Cause.pretty, traces, or logs. + Effect.catchCause((cause) => { + if (Cause.hasInterrupts(cause)) return preserveProviderInterruption(cause); + const reason = cause.reasons.length === 1 ? cause.reasons[0] : undefined; + return Effect.suspend(() => + Effect.succeed( + reason !== undefined && + Cause.isFailReason(reason) && + Predicate.isTagged(reason.error, "RefreshGrantRejected") + ? classifyProviderGrantRefusal(reason.error) + : providerRefreshFailure(), + ), + ).pipe( + // Even a malformed tagged object may throw from `_tag`/`error` accessors. + Effect.catchCause((classificationCause) => + Cause.hasInterrupts(classificationCause) + ? preserveProviderInterruption(classificationCause) + : Effect.succeed(providerRefreshFailure()), + ), + Effect.flatMap((error) => Effect.fail(error)), + ); + }), + Effect.tapError(armKnownDeadGate), + ); // Read the token back BEFORE recording success. A provider that // reported a grant it did not actually seal would otherwise leave the // row stamped with a fresh expiry over a stale or absent token, and // the connection would read healthy for a whole token lifetime while // every call using it failed. - const access = yield* provider.get(tokenItemId); - if (!access) { - return yield* reauth("Refreshed access token could not be resolved."); + const access = yield* Effect.suspend(() => provider.get(tokenItemId)).pipe( + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) + ? preserveProviderInterruption(cause) + : Effect.fail( + new StorageError({ + message: "Credential provider could not resolve the refreshed access token.", + cause: undefined, + }), + ), + ), + ); + if (typeof access !== "string" || access.length === 0) { + return yield* new StorageError({ + message: "Credential provider did not make the refreshed access token resolvable.", + cause: undefined, + }); } // Convert on OUR clock, never the provider's — `shouldRefreshToken` // compares the stored instant against this same clock, so an absolute @@ -2051,7 +2175,10 @@ export const createExecutor = ({ id: "acme" as const, @@ -61,7 +69,33 @@ type GrantBehaviour = /** Reports success but leaves nothing resolvable under `accessItemId`. */ | { readonly kind: "sealsNothing" } /** The authorization server refused the grant (RFC 6749 §5.2). */ - | { readonly kind: "rejected"; readonly error?: string }; + | { + readonly kind: "rejected"; + readonly error?: RefreshGrantRejectionCode; + /** Test-only hostile fields a JavaScript/remote provider could attach despite the type. */ + readonly unsafeDetails?: string; + readonly unsafeError?: string; + } + /** A provider-side storage failure whose diagnostic details must remain provider-side. */ + | { readonly kind: "storageFailure" } + /** A provider implementation that throws before it can return an Effect. */ + | { readonly kind: "syncThrow" } + /** A provider implementation that dies inside its Effect. */ + | { readonly kind: "defect" } + /** A concurrent provider failure that contains cancellation plus a secret-bearing defect. */ + | { readonly kind: "interruptedDefect" } + /** A malformed remote provider object whose classification getter throws. */ + | { readonly kind: "throwingRejectionGetter" } + /** A stateful rejection getter that changes after returning one valid code. */ + | { readonly kind: "changingRejectionGetter" } + /** Reading the optional capability itself throws before a grant can start. */ + | { readonly kind: "throwingCapabilityGetter" } + /** A success object whose property access throws after the provider Effect succeeds. */ + | { readonly kind: "throwingResultGetter"; readonly field: "expiry" | "scope" } + /** A method-shaped provider that relies on its receiver. */ + | { readonly kind: "requiresReceiver" } + /** A successful grant followed by a failure while resolving the new access token. */ + | { readonly kind: "readFailure"; readonly failure: "storage" | "defect" }; const SEALS: GrantBehaviour = { kind: "seals", scope: "read", expiresInSeconds: 3_600 }; @@ -69,6 +103,7 @@ const SEALS: GrantBehaviour = { kind: "seals", scope: "read", expiresInSeconds: interface Recorder { readonly reads: string[]; readonly grants: RefreshGrantInput[]; + rejectionErrorReads: number; } /** A memory provider that can also perform the refresh grant itself. @@ -84,9 +119,26 @@ const delegatingCredentialsPlugin = (recorder: Recorder, behaviour: GrantBehavio key: ProviderKey.make("memory"), writable: true as const, get: (id: ProviderItemId) => - Effect.sync(() => { + Effect.suspend(() => { recorder.reads.push(String(id)); - return store.get(String(id)) ?? null; + if ( + behaviour?.kind === "readFailure" && + recorder.grants.length > 0 && + recorder.grants.some((grant) => String(grant.accessItemId) === String(id)) + ) { + return behaviour.failure === "storage" + ? Effect.fail( + new StorageError({ + message: TOKEN_CANARY, + cause: { tokenResponse: TOKEN_CANARY }, + }), + ) + : Effect.die( + // oxlint-disable-next-line executor/no-error-constructor -- boundary: leak test deliberately injects a raw provider defect + new Error(TOKEN_CANARY), + ); + } + return Effect.succeed(store.get(String(id)) ?? null); }), set: (id: ProviderItemId, value: string) => Effect.sync(() => { @@ -101,33 +153,119 @@ const delegatingCredentialsPlugin = (recorder: Recorder, behaviour: GrantBehavio const provider: CredentialProvider = behaviour === null ? base - : { - ...base, - refreshGrant: (input: RefreshGrantInput) => - Effect.suspend(() => { + : behaviour.kind === "throwingCapabilityGetter" + ? (Object.defineProperty({ ...base }, "refreshGrant", { + get: () => { + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: leak test simulates an untyped plugin capability getter + throw TOKEN_CANARY; + }, + }) as CredentialProvider) + : { + ...base, + refreshGrant(input: RefreshGrantInput) { recorder.grants.push(input); - if (behaviour.kind === "rejected") { + if (behaviour.kind === "requiresReceiver" && this.key !== base.key) { + return Effect.die(TOKEN_CANARY); + } + // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: leak test simulates an untyped plugin throwing before it returns an Effect + if (behaviour.kind === "syncThrow") throw new Error(TOKEN_CANARY); + if (behaviour.kind === "storageFailure") { return Effect.fail( - new RefreshGrantRejected({ - message: "the authorization server refused the grant", - error: behaviour.error, + new StorageError({ + message: TOKEN_CANARY, + cause: { tokenResponse: TOKEN_CANARY }, }), ); } - if (behaviour.kind === "sealsNothing") { - // A provider reporting a grant it did not perform is out of contract. What the - // host CAN do is refuse to stamp the row healthy over a token it cannot read - // back, which is what this drives. - store.delete(String(input.accessItemId)); - return Effect.succeed({ expiresInSeconds: 3_600, scope: "read" }); + if (behaviour.kind === "defect") { + // oxlint-disable-next-line executor/no-error-constructor -- boundary: leak test deliberately injects a raw provider defect + return Effect.die(new Error(TOKEN_CANARY)); + } + if (behaviour.kind === "interruptedDefect") { + return Effect.failCause( + Cause.combine(Cause.die(TOKEN_CANARY), Cause.interrupt(123)), + ); + } + if (behaviour.kind === "throwingRejectionGetter") { + const malformed = Object.defineProperty( + { _tag: "RefreshGrantRejected" }, + "error", + { + get: () => { + // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: leak test simulates a malformed remote object with a throwing getter + throw new Error(TOKEN_CANARY); + }, + }, + ); + return Effect.fail(malformed as RefreshGrantRejected); + } + if (behaviour.kind === "changingRejectionGetter") { + const malformed = Object.defineProperty( + { _tag: "RefreshGrantRejected" }, + "error", + { + get: () => { + recorder.rejectionErrorReads += 1; + return recorder.rejectionErrorReads === 1 ? "invalid_grant" : TOKEN_CANARY; + }, + }, + ); + return Effect.fail(malformed as RefreshGrantRejected); + } + if (behaviour.kind === "throwingResultGetter") { + const result = { expiresInSeconds: 3_600, scope: "read" }; + Object.defineProperty( + result, + behaviour.field === "expiry" ? "expiresInSeconds" : "scope", + { + get: () => { + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: leak test simulates a malformed remote success object + throw TOKEN_CANARY; + }, + }, + ); + return Effect.succeed(result); } - store.set(String(input.accessItemId), "delegated-access-token"); - return Effect.succeed({ - expiresInSeconds: behaviour.expiresInSeconds, - scope: behaviour.scope, + return Effect.suspend(() => { + if (behaviour.kind === "rejected") { + const rejection = new RefreshGrantRejected({ error: behaviour.error }); + if ( + behaviour.unsafeDetails === undefined && + behaviour.unsafeError === undefined + ) { + return Effect.fail(rejection); + } + // Simulate an untyped JavaScript or remote provider. Executor must project only + // the validated RFC classification, even when extra secret-bearing fields exist. + return Effect.fail( + Object.assign(rejection, { + error: behaviour.unsafeError ?? rejection.error, + message: behaviour.unsafeDetails, + cause: { message: behaviour.unsafeDetails }, + }), + ); + } + if (behaviour.kind === "sealsNothing") { + // A provider reporting a grant it did not perform is out of contract. What the + // host CAN do is refuse to stamp the row healthy over a token it cannot read + // back, which is what this drives. + store.delete(String(input.accessItemId)); + return Effect.succeed({ expiresInSeconds: 3_600, scope: "read" }); + } + store.set(String(input.accessItemId), "delegated-access-token"); + return Effect.succeed({ + expiresInSeconds: + behaviour.kind === "readFailure" || behaviour.kind === "requiresReceiver" + ? 3_600 + : behaviour.expiresInSeconds, + scope: + behaviour.kind === "readFailure" || behaviour.kind === "requiresReceiver" + ? "read" + : behaviour.scope, + }); }); - }), - }; + }, + }; return { id: "memory-credentials" as const, @@ -137,6 +275,19 @@ const delegatingCredentialsPlugin = (recorder: Recorder, behaviour: GrantBehavio })(); describe("provider-owned OAuth refresh grant", () => { + const expectOpaqueProviderFailure = (exit: Exit.Exit, message: string) => { + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) return; + expect(JSON.stringify(exit)).not.toContain(TOKEN_CANARY); + expect(Cause.pretty(exit.cause)).not.toContain(TOKEN_CANARY); + const reason = exit.cause.reasons.find(Cause.isFailReason); + expect(reason).toBeDefined(); + if (reason === undefined) return; + expect(reason.error).toBeInstanceOf(StorageError); + expect((reason.error as StorageError).message).toBe(message); + expect((reason.error as StorageError).cause).toBeUndefined(); + }; + /** Connect, force the connection past expiry, and hand back the pieces a test asserts on. The * tool is NOT invoked here — each test drives the refresh itself so it can assert on failure. */ const scenario = (options: { @@ -144,14 +295,14 @@ describe("provider-owned OAuth refresh grant", () => { readonly grant?: "authorization_code" | "client_credentials"; }) => Effect.gen(function* () { - const recorder: Recorder = { reads: [], grants: [] }; + const recorder: Recorder = { reads: [], grants: [], rejectionErrorReads: 0 }; const server = yield* serveOAuthTestServer({ scopes: ["read"] }); const plugins = [ delegatingCredentialsPlugin(recorder, options.behaviour), oauthPlugin, ] as const; const { executor, config } = yield* makeTestWorkspaceHarness({ plugins }); - yield* executor.acme.seed(); + yield* executor.acme.seed(["read"]); const grant = options.grant ?? "authorization_code"; yield* executor.oauth.createClient({ @@ -264,6 +415,47 @@ describe("provider-owned OAuth refresh grant", () => { ), ); + it.effect("accepts the documented maximum delegated token lifetime", () => + Effect.scoped( + Effect.gen(function* () { + const before = Date.now(); + const { config, executor } = yield* scenario({ + behaviour: { + kind: "seals", + scope: "read", + expiresInSeconds: MAX_REFRESH_GRANT_EXPIRES_IN_SECONDS, + }, + }); + yield* executor.execute(TOOL, {}); + const row = yield* Effect.promise(() => + config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), + ); + expect(Number(row?.expires_at)).toBeGreaterThanOrEqual( + before + MAX_REFRESH_GRANT_EXPIRES_IN_SECONDS * 1_000, + ); + }), + ), + ); + + it.effect("rejects a delegated token lifetime above the documented maximum", () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* scenario({ + behaviour: { + kind: "seals", + scope: "read", + expiresInSeconds: MAX_REFRESH_GRANT_EXPIRES_IN_SECONDS + 1, + }, + }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + exit, + "Credential provider could not complete OAuth token refresh.", + ); + }), + ), + ); + it.effect("leaves the recorded scope alone when the provider reports none", () => Effect.scoped( Effect.gen(function* () { @@ -295,13 +487,19 @@ describe("provider-owned OAuth refresh grant", () => { Effect.scoped( Effect.gen(function* () { const { config, executor, recorder } = yield* scenario({ - behaviour: { kind: "rejected", error: "invalid_grant" }, + behaviour: { + kind: "rejected", + error: "invalid_grant", + unsafeDetails: TOKEN_CANARY, + }, }); const failure = yield* Effect.flip(executor.execute(TOOL, {})); // Not a StorageError: that is scrubbed to "Internal tool error [id]" at the sandbox // boundary, so the user would never be told to reconnect. - expect(JSON.stringify(failure)).toContain("invalid_grant"); + const serializedFailure = JSON.stringify(failure); + expect(serializedFailure).toContain("invalid_grant"); + expect(serializedFailure).not.toContain(TOKEN_CANARY); const row = yield* Effect.promise(() => config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), @@ -310,6 +508,9 @@ describe("provider-owned OAuth refresh grant", () => { (row?.provider_state as { oauthReauthRequiredAt?: number } | null)?.oauthReauthRequiredAt, ).toEqual(expect.any(Number)); expect(row?.last_health).toMatchObject({ status: "expired" }); + expect( + JSON.stringify({ providerState: row?.provider_state, lastHealth: row?.last_health }), + ).not.toContain(TOKEN_CANARY); // The gate is armed, so the doomed grant is not re-sent on the next resolve. Without this // a dead connection re-sends its dead grant on every proactive cycle, indefinitely. @@ -320,13 +521,306 @@ describe("provider-owned OAuth refresh grant", () => { ), ); + it.effect("preserves RFC 8707 invalid_target as a safe actionable classification", () => + Effect.scoped( + Effect.gen(function* () { + const { config, executor, recorder } = yield* scenario({ + behaviour: { kind: "rejected", error: "invalid_target" }, + }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) return; + expect(Cause.pretty(exit.cause)).toContain("invalid_target"); + expect(Cause.pretty(exit.cause)).not.toContain(TOKEN_CANARY); + const row = yield* Effect.promise(() => + config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), + ); + expect( + (row?.provider_state as { oauthReauthRequiredAt?: number } | null)?.oauthReauthRequiredAt, + ).toBeUndefined(); + + // Unlike invalid_grant, invalid_target does not prove the refresh token is dead. + recorder.grants.length = 0; + yield* Effect.exit(executor.execute(TOOL, {})); + expect(recorder.grants).toHaveLength(1); + }), + ), + ); + + it.effect("drops free-form rejection details and malformed classifications", () => + Effect.scoped( + Effect.gen(function* () { + const { config, executor, recorder } = yield* scenario({ + behaviour: { + kind: "rejected", + unsafeDetails: TOKEN_CANARY, + unsafeError: TOKEN_CANARY, + }, + }); + + const failure = yield* Effect.flip(executor.execute(TOOL, {})); + // This is also the failure object an outer boundary may log. A fixed message plus an + // undefined cause proves the hostile provider payload cannot flow through that log path. + expect(failure).toMatchObject({ + _tag: "StorageError", + message: "Credential provider could not complete OAuth token refresh.", + cause: undefined, + }); + expect(JSON.stringify(failure)).not.toContain(TOKEN_CANARY); + + const row = yield* Effect.promise(() => + config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), + ); + expect( + (row?.provider_state as { oauthReauthRequiredAt?: number } | null)?.oauthReauthRequiredAt, + ).toBeUndefined(); + expect( + JSON.stringify({ providerState: row?.provider_state, lastHealth: row?.last_health }), + ).not.toContain(TOKEN_CANARY); + + // An unknown classification is retryable and must not arm the known-dead gate. + recorder.grants.length = 0; + yield* Effect.flip(executor.execute(TOOL, {})); + expect(recorder.grants).toHaveLength(1); + }), + ), + ); + + it.effect("scrubs provider storage failures before they reach host error channels", () => + Effect.scoped( + Effect.gen(function* () { + const { config, executor, recorder } = yield* scenario({ + behaviour: { kind: "storageFailure" }, + }); + + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + exit, + "Credential provider could not complete OAuth token refresh.", + ); + + const row = yield* Effect.promise(() => + config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), + ); + expect( + JSON.stringify({ providerState: row?.provider_state, lastHealth: row?.last_health }), + ).not.toContain(TOKEN_CANARY); + expect( + (row?.provider_state as { oauthReauthRequiredAt?: number } | null)?.oauthReauthRequiredAt, + ).toBeUndefined(); + + // A storage failure is retryable and must not arm the known-dead grant gate. + recorder.grants.length = 0; + const retry = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + retry, + "Credential provider could not complete OAuth token refresh.", + ); + expect(recorder.grants).toHaveLength(1); + }), + ), + ); + + it.effect("scrubs a synchronous provider throw", () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* scenario({ behaviour: { kind: "syncThrow" } }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + exit, + "Credential provider could not complete OAuth token refresh.", + ); + }), + ), + ); + + it.effect("scrubs a provider defect", () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* scenario({ behaviour: { kind: "defect" } }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + exit, + "Credential provider could not complete OAuth token refresh.", + ); + }), + ), + ); + + it.effect("scrubs malformed rejection getters", () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* scenario({ + behaviour: { kind: "throwingRejectionGetter" }, + }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + exit, + "Credential provider could not complete OAuth token refresh.", + ); + }), + ), + ); + + it.effect("snapshots a stateful rejection classification exactly once", () => + Effect.scoped( + Effect.gen(function* () { + const { config, executor, recorder } = yield* scenario({ + behaviour: { kind: "changingRejectionGetter" }, + }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) return; + expect(recorder.rejectionErrorReads).toBe(1); + expect(Cause.pretty(exit.cause)).toContain("invalid_grant"); + expect(Cause.pretty(exit.cause)).not.toContain(TOKEN_CANARY); + + const row = yield* Effect.promise(() => + config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), + ); + expect( + JSON.stringify({ providerState: row?.provider_state, lastHealth: row?.last_health }), + ).not.toContain(TOKEN_CANARY); + }), + ), + ); + + it.effect("scrubs a throwing refresh capability getter", () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* scenario({ + behaviour: { kind: "throwingCapabilityGetter" }, + }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + exit, + "Credential provider could not complete OAuth token refresh.", + ); + }), + ), + ); + + it.effect("scrubs a throwing expiry getter on a successful provider result", () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* scenario({ + behaviour: { kind: "throwingResultGetter", field: "expiry" }, + }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + exit, + "Credential provider could not complete OAuth token refresh.", + ); + }), + ), + ); + + it.effect("scrubs a throwing scope getter on a successful provider result", () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* scenario({ + behaviour: { kind: "throwingResultGetter", field: "scope" }, + }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + exit, + "Credential provider could not complete OAuth token refresh.", + ); + }), + ), + ); + + it.effect("refuses to persist a provider scope outside the host-trusted grant set", () => + Effect.scoped( + Effect.gen(function* () { + const { config, executor } = yield* scenario({ + behaviour: { + kind: "seals", + expiresInSeconds: 3_600, + scope: TOKEN_CANARY, + }, + }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + exit, + "Credential provider could not complete OAuth token refresh.", + ); + const row = yield* Effect.promise(() => + config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), + ); + expect( + JSON.stringify({ scope: row?.oauth_scope, lastHealth: row?.last_health }), + ).not.toContain(TOKEN_CANARY); + }), + ), + ); + + it.effect("preserves a provider method's receiver", () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* scenario({ behaviour: { kind: "requiresReceiver" } }); + expect(yield* executor.execute(TOOL, {})).toEqual({ token: "delegated-access-token" }); + }), + ), + ); + + it.effect("preserves cancellation while dropping a concurrent secret-bearing defect", () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* scenario({ behaviour: { kind: "interruptedDefect" } }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) return; + expect(Cause.hasInterrupts(exit.cause)).toBe(true); + expect(exit.cause.reasons.every(Cause.isInterruptReason)).toBe(true); + expect(Cause.pretty(exit.cause)).not.toContain(TOKEN_CANARY); + }), + ), + ); + + it.effect("scrubs storage failures while resolving the refreshed access token", () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* scenario({ + behaviour: { kind: "readFailure", failure: "storage" }, + }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + exit, + "Credential provider could not resolve the refreshed access token.", + ); + }), + ), + ); + + it.effect("scrubs defects while resolving the refreshed access token", () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* scenario({ + behaviour: { kind: "readFailure", failure: "defect" }, + }); + const exit = yield* Effect.exit(executor.execute(TOOL, {})); + expectOpaqueProviderFailure( + exit, + "Credential provider could not resolve the refreshed access token.", + ); + }), + ), + ); + it.effect("fails rather than reporting success when the new token cannot be read back", () => Effect.scoped( Effect.gen(function* () { - const { config, executor } = yield* scenario({ behaviour: { kind: "sealsNothing" } }); + const { config, executor, recorder } = yield* scenario({ + behaviour: { kind: "sealsNothing" }, + }); const failure = yield* Effect.flip(executor.execute(TOOL, {})); - expect(JSON.stringify(failure)).toContain("could not be resolved"); + expect(failure).toBeInstanceOf(StorageError); + expect(failure.message).toBe( + "Credential provider did not make the refreshed access token resolvable.", + ); + expect(failure.cause).toBeUndefined(); // The row must NOT have been stamped with a fresh expiry — doing that over a token nobody // can resolve leaves the connection reading healthy for a full lifetime while every call @@ -335,6 +829,15 @@ describe("provider-owned OAuth refresh grant", () => { config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), ); expect(Number(row?.expires_at)).toBeLessThan(Date.now()); + expect( + (row?.provider_state as { oauthReauthRequiredAt?: number } | null)?.oauthReauthRequiredAt, + ).toBeUndefined(); + expect((row?.last_health as { status?: string } | null)?.status).not.toBe("expired"); + + // This is a provider invariant/storage failure, not a dead OAuth grant: retry it. + recorder.grants.length = 0; + yield* Effect.flip(executor.execute(TOOL, {})); + expect(recorder.grants).toHaveLength(1); }), ), ); diff --git a/packages/core/sdk/src/promise.ts b/packages/core/sdk/src/promise.ts index d48d106071..775abd8d86 100644 --- a/packages/core/sdk/src/promise.ts +++ b/packages/core/sdk/src/promise.ts @@ -38,7 +38,18 @@ export type { // Credential providers are Effect-native (their `get`/`set` return `Effect`s), // but Promise consumers still author them to register an inline writable store // via `createExecutor({ providers })`. -export type { CredentialProvider, ProviderEntry } from "./provider"; +export { + MAX_REFRESH_GRANT_EXPIRES_IN_SECONDS, + RefreshGrantRejected, + isRefreshGrantRejectionCode, +} from "./provider"; +export type { + CredentialProvider, + ProviderEntry, + RefreshGrantInput, + RefreshGrantRejectionCode, + RefreshGrantResult, +} from "./provider"; export type { CreateToolPolicyInput, RemoveToolPolicyInput, diff --git a/packages/core/sdk/src/provider.ts b/packages/core/sdk/src/provider.ts index 9a881c9f2e..799e2303cc 100644 --- a/packages/core/sdk/src/provider.ts +++ b/packages/core/sdk/src/provider.ts @@ -56,7 +56,17 @@ export interface CredentialProvider { * * 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. */ + * interface exists to remove. + * + * SECURITY: this ENTIRE input tuple is the CALLER's view. A caller whose + * process is part of the threat model can rewrite not only `tokenUrl`, but + * also every item id, the client id/auth method, scopes, and resource. A + * provider that withholds credentials from that caller MUST authenticate the + * complete tuple against independently trusted enrollment metadata and reject + * mismatches before resolving or spending any secret. This warning documents + * the current contract; it does not solve the structural limitation that the + * API still transports caller-authored grant parameters rather than one + * provider-owned sealed descriptor. */ export interface RefreshGrantInput { /** The stored refresh token to spend. */ readonly refreshItemId: ProviderItemId; @@ -65,14 +75,8 @@ export interface RefreshGrantInput { readonly accessItemId: ProviderItemId; /** The OAuth app's client secret, by id. Absent for a public client. */ readonly clientSecretItemId?: ProviderItemId; - /** The token endpoint to post to. - * - * SECURITY: this is the CALLER's view of the endpoint, and a caller whose - * process is part of your threat model is not a trustworthy source for it — - * a rewritten value turns the grant into an exfiltration of the very token - * this interface exists to seal. A provider whose whole purpose is to - * withhold the refresh token from the caller MUST pin the endpoint against a - * value it recorded when the item was sealed, and reject a mismatch. */ + /** The token endpoint to post to. A mismatch against the provider's enrolled + * endpoint can exfiltrate the sealed refresh token. */ readonly tokenUrl: string; readonly clientId: string; /** How to present the client secret: `"body"` is `client_secret_post`, @@ -98,30 +102,58 @@ export interface RefreshGrantResult { * RELATIVE, not an absolute instant, precisely because the provider may run * where the caller cannot read — which usually means a different machine and * therefore a different clock. The caller converts against its OWN clock, the - * same one that later decides whether the token is due for refresh. */ + * same one that later decides whether the token is due for refresh. Executor + * accepts only a finite, non-negative value no greater than + * `MAX_REFRESH_GRANT_EXPIRES_IN_SECONDS`. */ readonly expiresInSeconds: number | null; /** The granted scope as reported by the authorization server, or null when it - * did not report one (distinct from an empty scope). */ + * did not report one (distinct from an empty scope). Executor accepts only a + * canonical subset of the connection's already-recorded granted scopes. */ readonly scope: string | null; } +/** Largest delegated access-token lifetime Executor accepts: ten 365-day years. */ +export const MAX_REFRESH_GRANT_EXPIRES_IN_SECONDS = 10 * 365 * 24 * 60 * 60; + +/** The closed standards-defined token-endpoint error set. + * + * Keeping this classification closed is a custody boundary: a provider error + * reaches host telemetry and, for `invalid_grant`, persisted connection health. + * Free-form values would therefore be another channel for token material. The + * first six values are RFC 6749 section 5.2; `invalid_target` is RFC 8707 + * section 4 for this API's optional `resource` parameter. */ +const REFRESH_GRANT_REJECTION_CODES = [ + "invalid_request", + "invalid_client", + "invalid_grant", + "unauthorized_client", + "unsupported_grant_type", + "invalid_scope", + "invalid_target", +] as const; + +export type RefreshGrantRejectionCode = (typeof REFRESH_GRANT_REJECTION_CODES)[number]; + +export const isRefreshGrantRejectionCode = (value: unknown): value is RefreshGrantRejectionCode => + typeof value === "string" && (REFRESH_GRANT_REJECTION_CODES as readonly string[]).includes(value); + /** The authorization server refused the grant. * * Distinct from `StorageFailure` because the two demand opposite responses: a - * storage failure is transient and worth retrying, whereas an RFC 6749 §5.2 - * refusal is the AS's standing verdict — `invalid_grant` in particular means + * storage failure is transient and worth retrying, whereas a standards-defined + * token-endpoint refusal is the AS's standing verdict — `invalid_grant` in particular means * the refresh token is dead and only re-authentication recovers it. Without * this the caller cannot tell "the vault is down" from "this connection is * finished", so it can neither prompt for re-auth nor stop re-sending a grant * that will never succeed. * - * The §5.2 error response carries no token material, so reporting the code - * costs nothing in custody. */ + * Only the closed standards-defined classification crosses this boundary. In particular + * there is deliberately no provider-controlled message or cause: those fields + * can contain response bodies, URLs, or secret-bearing errors and would be + * surfaced to callers, persistence, or logs by the host. */ export class RefreshGrantRejected extends Data.TaggedError("RefreshGrantRejected")<{ - readonly message: string; - /** The RFC 6749 §5.2 code (`invalid_grant`, `invalid_client`, …) when the - * token endpoint returned one. Omit it for a failure that carried no code — - * the caller then treats the failure as transient. */ - readonly error?: string; - readonly cause?: unknown; + /** The validated token-endpoint code (`invalid_grant`, `invalid_client`, + * `invalid_target`, …) when the endpoint returned one. Omit it for a failure + * that carried no code — the caller then treats the failure as transient. */ + readonly error?: RefreshGrantRejectionCode; }> {} diff --git a/packages/core/sdk/src/shared.ts b/packages/core/sdk/src/shared.ts index 615c7c6c57..17fe5ec6e8 100644 --- a/packages/core/sdk/src/shared.ts +++ b/packages/core/sdk/src/shared.ts @@ -46,7 +46,18 @@ export type { UpdateConnectionInput, ValidateConnectionInput, } from "./connection"; -export type { CredentialProvider, ProviderEntry } from "./provider"; +export { + MAX_REFRESH_GRANT_EXPIRES_IN_SECONDS, + RefreshGrantRejected, + isRefreshGrantRejectionCode, +} from "./provider"; +export type { + CredentialProvider, + ProviderEntry, + RefreshGrantInput, + RefreshGrantRejectionCode, + RefreshGrantResult, +} from "./provider"; export type { Tool, ToolDef, ToolListFilter, ToolAnnotations } from "./tool"; // Tagged errors (Schema-based — browser-safe). From 014285f5965df4e18ed2f1b49318d7610eddb7dc Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:47:34 +0200 Subject: [PATCH 5/5] Keep refreshing a connection that has no recorded scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scope validation compared the provider's reported scope against the connection's recorded grant set. When that set is empty the comparison had no subset to accept, so every reported scope failed the whole refresh — and the failure is a retryable StorageError, so the connection would retry a grant that could never succeed, indefinitely. An empty grant set is a legitimate state, not a corrupt one: RFC 6749 §5.1 lets an authorization server omit the scope it granted, and the refresh request then omits the scope parameter entirely. With nothing recorded there is also nothing to widen away from, so the safe action is to keep the refresh and record no scope. The provider's string is still never persisted, which is the property the validation exists to hold. --- packages/core/sdk/src/executor.ts | 9 +++++++ .../oauth-refresh-grant-delegation.test.ts | 27 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 5f27a67423..f1153d1312 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -2045,6 +2045,15 @@ export const createExecutor = !trustedScopes.has(scope)) diff --git a/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts b/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts index 42b901aa34..8dd50c50b6 100644 --- a/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts +++ b/packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts @@ -879,4 +879,31 @@ describe("provider-owned OAuth refresh grant", () => { }), ), ); + + it.effect("keeps refreshing a connection that has no recorded scope", () => + Effect.scoped( + Effect.gen(function* () { + const { config, executor } = yield* scenario({ behaviour: SEALS }); + // RFC 6749 §5.1 lets an authorization server omit the granted scope, so a live connection + // can legitimately carry none. The scope validation must not turn that into a permanent + // failure: with nothing recorded there is nothing to widen from. + yield* Effect.promise(() => + config.db.updateMany("connection", { + where: (b) => b("name", "=", "main"), + set: { oauth_scope: null }, + }), + ); + + const out = yield* executor.execute(TOOL, {}); + expect(out).toEqual({ token: "delegated-access-token" }); + + // The provider's scope string is still never persisted — that is the property the + // validation exists to hold, and it holds here by recording nothing at all. + const row = yield* Effect.promise(() => + config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }), + ); + expect(row?.oauth_scope).toBeNull(); + }), + ), + ); });