diff --git a/e2e/scenarios/oauth-scope-insufficient.test.ts b/e2e/scenarios/oauth-scope-insufficient.test.ts new file mode 100644 index 000000000..c9a6b7d1f --- /dev/null +++ b/e2e/scenarios/oauth-scope-insufficient.test.ts @@ -0,0 +1,324 @@ +// Cross-target: a scope-insufficient upstream 403 is a distinct, actionable +// failure — not a re-authenticate loop. When a connection's OAuth grant does +// not cover the scope an operation requires, the upstream rejects the call +// with a scope signal (Google's ACCESS_TOKEN_SCOPE_INSUFFICIENT, RFC 6750's +// insufficient_scope). Re-running the same grant returns the identical 403, +// so the tool failure must say so: code `oauth_scope_insufficient`, guidance +// to reconnect with broader access, and NO `oauth.start` recovery hint (an +// agent following one would loop through identical consent forever). +// +// The journey: an OpenAPI integration with two operations under one OAuth +// method; the connection completes a real authorization-code flow granting +// only `mail.read`. Calling the `files.read` operation dispatches upstream +// (catalog projection by scope is #1384, tracked separately), which answers +// with a Google-shaped scope-insufficient 403 — and the failure the sandbox +// sees carries the new code, while the ordinary revoked-token 403 stays +// `connection_rejected`. +import { randomBytes } from "node:crypto"; +import { createServer } from "node:http"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { + AuthTemplateSlug, + ConnectionName, + IntegrationSlug, + OAuthClientSlug, +} from "@executor-js/sdk/shared"; +import { serveOAuthTestServer } from "@executor-js/sdk/testing"; + +import { scenario } from "../src/scenario"; +import { Api, Target } from "../src/services"; + +const api = composePluginApi([openApiHttpPlugin()] as const); + +const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`; + +/** Upstream on 127.0.0.1: `GET /inbox` is 200 for any bearer; `GET /files` + * answers a Google-shaped scope-insufficient 403; `GET /admin` answers an + * ordinary 403 with no scope signal. */ +const serveScopedUpstream = () => + Effect.acquireRelease( + Effect.callback<{ readonly url: string; readonly close: () => void }>((resume) => { + const server = createServer((request, response) => { + const path = request.url ?? ""; + if (request.method === "GET" && path.startsWith("/inbox")) { + response.writeHead(200, { "content-type": "application/json" }); + response.end(JSON.stringify({ messages: [] })); + return; + } + if (request.method === "GET" && path.startsWith("/files")) { + response.writeHead(403, { "content-type": "application/json" }); + response.end( + JSON.stringify({ + error: { + code: 403, + message: "Request had insufficient authentication scopes.", + status: "PERMISSION_DENIED", + details: [ + { + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + reason: "ACCESS_TOKEN_SCOPE_INSUFFICIENT", + domain: "googleapis.com", + metadata: { service: "drive.googleapis.com" }, + }, + ], + }, + }), + ); + return; + } + if (request.method === "GET" && path.startsWith("/admin")) { + response.writeHead(403, { "content-type": "application/json" }); + response.end( + JSON.stringify({ error: { status: "PERMISSION_DENIED", message: "not allowed" } }), + ); + return; + } + response.writeHead(404, { "content-type": "application/json" }); + response.end(JSON.stringify({ error: "not_found" })); + }); + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + const port = typeof address === "object" && address ? address.port : 0; + resume( + Effect.succeed({ + url: `http://127.0.0.1:${port}`, + close: () => { + server.close(); + server.closeAllConnections(); + }, + }), + ); + }); + }), + (server) => Effect.sync(server.close), + ); + +const spec = ( + baseUrl: string, + oauth: { readonly authorizationEndpoint: string; readonly tokenEndpoint: string }, +): string => + JSON.stringify({ + openapi: "3.0.3", + info: { title: "Scoped API", version: "1.0.0" }, + servers: [{ url: baseUrl }], + paths: { + "/inbox": { + get: { + operationId: "readInbox", + summary: "List inbox messages", + security: [{ oauth: ["mail.read"] }], + responses: { "200": { description: "messages" } }, + }, + }, + "/files": { + get: { + operationId: "listFiles", + summary: "List drive files", + security: [{ oauth: ["files.read"] }], + responses: { "200": { description: "files" } }, + }, + }, + "/admin": { + get: { + operationId: "adminAction", + summary: "An admin-only action", + security: [{ oauth: ["mail.read"] }], + responses: { "200": { description: "ok" } }, + }, + }, + }, + components: { + securitySchemes: { + oauth: { + type: "oauth2", + flows: { + authorizationCode: { + authorizationUrl: oauth.authorizationEndpoint, + tokenUrl: oauth.tokenEndpoint, + scopes: { + "mail.read": "Read mail", + "files.read": "Read files", + }, + }, + }, + }, + }, + }, + }); + +const invokeByAddressCode = (address: string, args: unknown) => ` +const segments = ${JSON.stringify(address)}.split(".").slice(1); +let node = tools; +for (const segment of segments) node = node[segment]; +const result = await node(${JSON.stringify(args)}); +return JSON.stringify(result); +`; + +type ToolEnvelope = { + readonly ok: boolean; + readonly error?: { + readonly code?: string; + readonly message?: string; + readonly details?: { readonly recovery?: Record }; + }; +}; + +scenario( + "Auth failures · a scope-insufficient 403 tells the agent to reconnect with broader access, not to re-authenticate", + {}, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const { client: makeClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeClient(api, identity); + const upstream = yield* serveScopedUpstream(); + const oauth = yield* serveOAuthTestServer({ scopes: ["mail.read", "files.read"] }); + const slug = unique("scopeins"); + const clientSlug = OAuthClientSlug.make(unique("scopeinsc")); + + yield* Effect.ensuring( + Effect.gen(function* () { + // The integration's OAuth method requests only mail.read — the + // whole grant the connection will hold. + yield* client.openapi.addSpec({ + payload: { + spec: { kind: "blob", value: spec(upstream.url, oauth) }, + slug, + baseUrl: upstream.url, + authenticationTemplate: [ + { + slug: "oauth", + kind: "oauth2", + authorizationUrl: oauth.authorizationEndpoint, + tokenUrl: oauth.tokenEndpoint, + scopes: ["mail.read"], + }, + ], + }, + }); + yield* client.oauth.createClient({ + payload: { + owner: "org", + slug: clientSlug, + grant: "authorization_code", + authorizationUrl: oauth.authorizationEndpoint, + tokenUrl: oauth.tokenEndpoint, + clientId: "test-client", + clientSecret: "test-secret", + originIntegration: IntegrationSlug.make(slug), + }, + }); + + const started = yield* client.oauth.start({ + payload: { + client: clientSlug, + clientOwner: "org", + owner: "org", + name: ConnectionName.make("main"), + integration: IntegrationSlug.make(slug), + template: AuthTemplateSlug.make("oauth"), + }, + }); + expect(started.status, "oauth.start redirects to the authorization server").toBe( + "redirect", + ); + if (started.status !== "redirect") return yield* Effect.die("no redirect"); + + // Drive the test IdP's consent by hand (authorize → login → code). + const code = yield* Effect.promise(async () => { + const authorize = await fetch(started.authorizationUrl, { redirect: "manual" }); + const loginUrl = authorize.headers.get("location"); + if (!loginUrl) throw new Error(`authorize did not redirect: ${authorize.status}`); + const login = await fetch(loginUrl, { + method: "POST", + headers: { + authorization: `Basic ${Buffer.from("alice:password").toString("base64")}`, + }, + redirect: "manual", + }); + const callbackUrl = login.headers.get("location"); + if (!callbackUrl) throw new Error(`login did not redirect: ${login.status}`); + const minted = new URL(callbackUrl).searchParams.get("code"); + if (!minted) throw new Error("callback carried no authorization code"); + return minted; + }); + yield* client.oauth.complete({ payload: { state: started.state, code } }); + + const tools = yield* client.tools.list({ query: {} }); + const addresses = tools + .filter((tool) => String(tool.integration) === slug) + .map((tool) => String(tool.address)); + const addressOf = (suffix: string) => { + const found = addresses.find((addr) => addr.endsWith(suffix)); + expect(found, `the ${suffix} tool is in the catalog`).toBeDefined(); + return found!; + }; + + const invoke = (address: string) => + Effect.gen(function* () { + const executed = yield* client.executions.execute({ + payload: { code: invokeByAddressCode(address, {}), autoApprove: true }, + }); + expect(executed.status, "the sandbox execution completed").toBe("completed"); + return JSON.parse(executed.text) as ToolEnvelope; + }); + + // THE guarantee: the scope-insufficient 403 carries its own code + // and reconnect guidance — and no oauth.start hint, because + // re-running the identical grant cannot satisfy the scope. + const scopeFailure = yield* invoke(addressOf("listFiles")); + expect(scopeFailure.ok, "the out-of-scope call failed").toBe(false); + expect( + scopeFailure.error?.code, + "the failure names the scope shortfall, not a rejected credential", + ).toBe("oauth_scope_insufficient"); + expect( + scopeFailure.error?.message ?? "", + "the message says re-authenticating will not help", + ).toContain("Re-authenticating with the same grant"); + expect( + scopeFailure.error?.details?.recovery?.startOAuthTool, + "no oauth.start recovery hint", + ).toBeUndefined(); + expect( + scopeFailure.error?.details?.recovery?.scopeInstructions ?? "", + "the recovery tells the agent to reconnect with broader access", + ).toContain("broader access"); + + // An ordinary 403 with no scope signal keeps the existing + // classification — the new code is additive, not a re-label. + const plainForbidden = yield* invoke(addressOf("adminAction")); + expect(plainForbidden.ok, "the plain-403 call failed").toBe(false); + expect( + plainForbidden.error?.code, + "a 403 without a scope signal stays connection_rejected", + ).toBe("connection_rejected"); + + // And the in-scope operation works, proving the connection is fine. + const inScope = yield* invoke(addressOf("readInbox")); + expect(inScope.ok, "the in-scope call succeeds with the same token").toBe(true); + }), + Effect.gen(function* () { + yield* client.connections + .remove({ + params: { + owner: "org", + integration: IntegrationSlug.make(slug), + name: ConnectionName.make("main"), + }, + }) + .pipe(Effect.ignore); + yield* client.oauth + .removeClient({ params: { slug: clientSlug }, payload: { owner: "org" } }) + .pipe(Effect.ignore); + yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore); + }), + ); + }), + ), +); diff --git a/packages/core/sdk/src/auth-tool-failure.ts b/packages/core/sdk/src/auth-tool-failure.ts index 807d18456..3216fd9a8 100644 --- a/packages/core/sdk/src/auth-tool-failure.ts +++ b/packages/core/sdk/src/auth-tool-failure.ts @@ -5,7 +5,8 @@ export type AuthToolFailureCode = | "connection_rejected" | "oauth_connection_missing" | "oauth_refresh_failed" - | "oauth_reauth_required"; + | "oauth_reauth_required" + | "oauth_scope_insufficient"; export type AuthToolFailureInput = { readonly code: AuthToolFailureCode; @@ -37,18 +38,35 @@ export type AuthToolFailureInput = { // creates the bound connection in one step); OAuth credentials are minted by // the OAuth start flow. These strings are read by the agent resolving the // failure, so they must name tools that actually exist on the executor. -const authRecovery = (input?: AuthToolFailureInput["recovery"]) => ({ - createConnectionTool: "executor.coreTools.connections.createHandoff", - startOAuthTool: "executor.coreTools.oauth.start", - listConnectionsTool: "executor.coreTools.connections.list", - ...(input?.configureIntegrationTool - ? { configureIntegrationTool: input.configureIntegrationTool } - : {}), - connectionInstructions: - "For API keys and tokens, call createConnectionTool for the integration to get a browser URL; the user enters the credential there, which creates the bound connection. Do not ask the user to paste secrets into chat. Then call listConnectionsTool to confirm the connection exists before retrying this tool.", - oauthInstructions: - "For OAuth credentials, call startOAuthTool and give the returned authorizationUrl to the user. The completed connection binds automatically, then retry the tool.", -}); +const authRecovery = (code: AuthToolFailureCode, input?: AuthToolFailureInput["recovery"]) => { + // A scope-insufficient rejection cannot be fixed by re-running the same + // grant, so this branch deliberately omits startOAuthTool/oauthInstructions: + // an agent following the hints would loop through an identical consent and + // land on the identical 403. The connection must be reconnected with a + // broader scope, which is a user decision, not a retryable tool call. + if (code === "oauth_scope_insufficient") { + return { + listConnectionsTool: "executor.coreTools.connections.list", + ...(input?.configureIntegrationTool + ? { configureIntegrationTool: input.configureIntegrationTool } + : {}), + scopeInstructions: + "The connection's OAuth grant does not cover the scope this operation requires; re-authenticating with the same grant will return the same error. Tell the user which operation was denied and ask them to reconnect the integration with broader access (or use a connection that already has it). Call listConnectionsTool to see the available connections and their scopes.", + }; + } + return { + createConnectionTool: "executor.coreTools.connections.createHandoff", + startOAuthTool: "executor.coreTools.oauth.start", + listConnectionsTool: "executor.coreTools.connections.list", + ...(input?.configureIntegrationTool + ? { configureIntegrationTool: input.configureIntegrationTool } + : {}), + connectionInstructions: + "For API keys and tokens, call createConnectionTool for the integration to get a browser URL; the user enters the credential there, which creates the bound connection. Do not ask the user to paste secrets into chat. Then call listConnectionsTool to confirm the connection exists before retrying this tool.", + oauthInstructions: + "For OAuth credentials, call startOAuthTool and give the returned authorizationUrl to the user. The completed connection binds automatically, then retry the tool.", + }; +}; export const authToolFailure = (input: AuthToolFailureInput): ToolResult => { const error: ToolError = { @@ -61,7 +79,7 @@ export const authToolFailure = (input: AuthToolFailureInput): ToolRes ...(input.integration ? { integration: input.integration } : {}), ...(input.credential ? { credential: input.credential } : {}), ...(input.upstream ? { upstream: input.upstream } : {}), - recovery: authRecovery(input.recovery), + recovery: authRecovery(input.code, input.recovery), }, }; return ToolResult.fail(error); diff --git a/packages/core/sdk/src/index.ts b/packages/core/sdk/src/index.ts index 99e34baed..73a8e9f6c 100644 --- a/packages/core/sdk/src/index.ts +++ b/packages/core/sdk/src/index.ts @@ -407,3 +407,8 @@ export { type AuthToolFailureCode, type AuthToolFailureInput, } from "./auth-tool-failure"; +export { + detectInsufficientScope, + insufficientScopeFromEmbeddedJson, + type InsufficientScopeDetection, +} from "./insufficient-scope"; diff --git a/packages/core/sdk/src/insufficient-scope.test.ts b/packages/core/sdk/src/insufficient-scope.test.ts new file mode 100644 index 000000000..f67908adb --- /dev/null +++ b/packages/core/sdk/src/insufficient-scope.test.ts @@ -0,0 +1,314 @@ +import { describe, expect, it } from "@effect/vitest"; + +import { authToolFailure } from "./auth-tool-failure"; +import { detectInsufficientScope } from "./insufficient-scope"; + +describe("detectInsufficientScope", () => { + it("detects Google's ErrorInfo reason nested in an error body", () => { + const body = { + error: { + code: 403, + message: "Request had insufficient authentication scopes.", + status: "PERMISSION_DENIED", + details: [ + { + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + reason: "ACCESS_TOKEN_SCOPE_INSUFFICIENT", + domain: "googleapis.com", + metadata: { service: "drive.googleapis.com" }, + }, + ], + }, + }; + expect(detectInsufficientScope({ body })).toEqual({ requiredScopes: [] }); + }); + + it("detects the RFC 6750 error body", () => { + expect(detectInsufficientScope({ body: { error: "insufficient_scope" } })).toEqual({ + requiredScopes: [], + }); + }); + + it("reads required scopes from a WWW-Authenticate challenge", () => { + expect( + detectInsufficientScope({ + body: null, + headers: { + "www-authenticate": + 'Bearer realm="example", error="insufficient_scope", scope="files.read files.meta"', + }, + }), + ).toEqual({ requiredScopes: ["files.read", "files.meta"] }); + }); + + it("detects the signal inside a JSON text body", () => { + expect(detectInsufficientScope({ body: '{"error":"insufficient_scope"}' })).toEqual({ + requiredScopes: [], + }); + }); + + it("never classifies non-JSON text, even when it embeds example JSON", () => { + expect( + detectInsufficientScope({ + body: "Proxy note: error=insufficient_scope was returned upstream", + }), + ).toBeNull(); + expect( + detectInsufficientScope({ + body: 'The docs show {"error":"insufficient_scope"} as an example response', + }), + ).toBeNull(); + }); + + it("ignores quoted challenge parameter values that embed the error token", () => { + expect( + detectInsufficientScope({ + headers: { + "www-authenticate": + 'Bearer error_description="Example: error=insufficient_scope for missing grants"', + }, + }), + ).toBeNull(); + }); + + it("ignores prose that merely mentions the tokens", () => { + expect( + detectInsufficientScope({ + body: { + error: { + message: + "If the token lacks access you may see insufficient_scope or ACCESS_TOKEN_SCOPE_INSUFFICIENT in provider docs", + }, + }, + }), + ).toBeNull(); + expect( + detectInsufficientScope({ + body: "Consult the OAuth guide about insufficient_scope errors", + }), + ).toBeNull(); + }); + + it("ignores the tokens under other field names and inside data lists", () => { + expect( + detectInsufficientScope({ + body: { supportedErrors: ["invalid_token", "insufficient_scope"] }, + }), + ).toBeNull(); + expect(detectInsufficientScope({ body: { code: "insufficient_scope" } })).toBeNull(); + }); + + it("ignores look-alike challenge parameters and values", () => { + expect( + detectInsufficientScope({ + headers: { "www-authenticate": 'Bearer x-error="insufficient_scope"' }, + }), + ).toBeNull(); + expect( + detectInsufficientScope({ + headers: { "www-authenticate": 'Bearer error="insufficient_scope_extra"' }, + }), + ).toBeNull(); + }); + + it("consumes quoted-pairs, so an escaped quote cannot fabricate a parameter", () => { + expect( + detectInsufficientScope({ + headers: { + "www-authenticate": 'Bearer error_description="Example: \\"error=insufficient_scope"', + }, + }), + ).toBeNull(); + }); + + it("never discovers a scheme inside another challenge's quoted value", () => { + expect( + detectInsufficientScope({ + headers: { + "www-authenticate": + 'Basic error_description="Proxy saw Bearer error=insufficient_scope upstream"', + }, + }), + ).toBeNull(); + expect( + detectInsufficientScope({ + headers: { + "www-authenticate": + 'Digest realm="x", qop="auth Bearer error=insufficient_scope", nonce="n"', + }, + }), + ).toBeNull(); + // A Basic challenge followed by a harmless real Bearer challenge: the + // Bearer params are read, and they do not include the Basic params. + expect( + detectInsufficientScope({ + headers: { + "www-authenticate": 'Basic error=insufficient_scope, Bearer realm="api"', + }, + }), + ).toBeNull(); + }); + + it("only reads the Bearer scheme's parameters", () => { + expect( + detectInsufficientScope({ + headers: { "www-authenticate": 'Basic realm="example", error=insufficient_scope' }, + }), + ).toBeNull(); + // Multi-challenge: params after the NEXT scheme are not Bearer's. + expect( + detectInsufficientScope({ + headers: { + "www-authenticate": 'Bearer realm="api", Basic error=insufficient_scope', + }, + }), + ).toBeNull(); + }); + + it("evaluates repeated Bearer challenges independently", () => { + // The second Bearer challenge carries the signal; the first must not + // shadow it (params are per challenge, not first-wins across the header). + expect( + detectInsufficientScope({ + headers: { + "www-authenticate": + "Bearer error=invalid_token, Basic realm=x, Bearer error=insufficient_scope", + }, + }), + ).toEqual({ requiredScopes: [] }); + // And a scope attr from an unrelated Bearer challenge does not leak into + // the one that matched. + expect( + detectInsufficientScope({ + headers: { + "www-authenticate": 'Bearer scope="other.scope", Bearer error=insufficient_scope', + }, + }), + ).toEqual({ requiredScopes: [] }); + }); + + it("recognizes a new scheme only at a comma boundary", () => { + // Comma-less run-ons are malformed and must not fabricate a Bearer + // challenge out of a bare word. + expect( + detectInsufficientScope({ + headers: { "www-authenticate": "Basic realm=x Bearer error=insufficient_scope" }, + }), + ).toBeNull(); + expect( + detectInsufficientScope({ + headers: { + "www-authenticate": "Basic error_description=Proxy Bearer error=insufficient_scope", + }, + }), + ).toBeNull(); + }); + + it("steps over a token68 blob to a later legitimate Bearer challenge", () => { + expect( + detectInsufficientScope({ + headers: { + "www-authenticate": "Negotiate abc/def==, Bearer error=insufficient_scope", + }, + }), + ).toEqual({ requiredScopes: [] }); + }); + + it("never classifies malformed headers", () => { + // Unterminated quoted-string. + expect( + detectInsufficientScope({ + headers: { "www-authenticate": 'Bearer error="insufficient_scope' }, + }), + ).toBeNull(); + // Quoted value run into the next token with no separator. + expect( + detectInsufficientScope({ + headers: { "www-authenticate": 'Basic realm="x"Bearer error=insufficient_scope' }, + }), + ).toBeNull(); + }); + + it("accepts BWS around the auth-param equals sign", () => { + for (const header of [ + "Bearer error =insufficient_scope", + "Bearer error= insufficient_scope", + 'Bearer error = "insufficient_scope"', + ]) { + expect(detectInsufficientScope({ headers: { "www-authenticate": header } })).toEqual({ + requiredScopes: [], + }); + } + }); + + it("rejects params attached to token68 or scheme-only challenges", () => { + for (const header of [ + "Bearer a=, error=insufficient_scope", + "Bearer abc, error=insufficient_scope", + "Bearer, error=insufficient_scope", + 'Bearer realm="x" error=insufficient_scope', + ]) { + expect( + detectInsufficientScope({ headers: { "www-authenticate": header } }), + header, + ).toBeNull(); + } + }); + + it("accepts comma-separated params within one Bearer challenge", () => { + expect( + detectInsufficientScope({ + headers: { + "www-authenticate": 'Bearer realm="api", error="insufficient_scope", scope="a.b"', + }, + }), + ).toEqual({ requiredScopes: ["a.b"] }); + }); + + it("accepts an unquoted RFC 6750 challenge value", () => { + expect( + detectInsufficientScope({ + headers: { "www-authenticate": "Bearer error=insufficient_scope" }, + }), + ).toEqual({ requiredScopes: [] }); + }); + + it("returns null for an ordinary 403 body", () => { + expect( + detectInsufficientScope({ + body: { error: { status: "PERMISSION_DENIED", message: "Caller lacks permission" } }, + headers: { "www-authenticate": 'Bearer realm="example", error="invalid_token"' }, + }), + ).toBeNull(); + }); + + it("returns null for empty input", () => { + expect(detectInsufficientScope({})).toBeNull(); + }); +}); + +describe("authToolFailure recovery for oauth_scope_insufficient", () => { + const recoveryOf = (result: ReturnType) => { + const details = (result as { error: { details: { recovery: Record } } }).error + .details; + return details.recovery; + }; + + it("omits the oauth.start hint, which would re-run the identical grant", () => { + const recovery = recoveryOf( + authToolFailure({ code: "oauth_scope_insufficient", message: "scope shortfall" }), + ); + expect(recovery.startOAuthTool).toBeUndefined(); + expect(recovery.oauthInstructions).toBeUndefined(); + expect(recovery.scopeInstructions).toContain("does not cover the scope"); + expect(recovery.listConnectionsTool).toBe("executor.coreTools.connections.list"); + }); + + it("keeps the full recovery block for connection_rejected", () => { + const recovery = recoveryOf( + authToolFailure({ code: "connection_rejected", message: "rejected" }), + ); + expect(recovery.startOAuthTool).toBe("executor.coreTools.oauth.start"); + expect(recovery.oauthInstructions).toBeDefined(); + }); +}); diff --git a/packages/core/sdk/src/insufficient-scope.ts b/packages/core/sdk/src/insufficient-scope.ts new file mode 100644 index 000000000..7d45ce3bd --- /dev/null +++ b/packages/core/sdk/src/insufficient-scope.ts @@ -0,0 +1,242 @@ +// Detecting a scope-insufficient upstream rejection. A 403 that means "this +// grant does not cover that operation" is unfixable by re-running the same +// OAuth flow, so it must not be labelled connection_rejected (whose recovery +// tells the agent to re-authenticate). Providers signal it three ways: +// +// - RFC 6750: a `WWW-Authenticate: Bearer error="insufficient_scope" +// scope="..."` challenge header (the `scope` attribute names what the +// request needed). +// - Google (google.rpc.ErrorInfo): a JSON body whose error details carry +// `reason: "ACCESS_TOKEN_SCOPE_INSUFFICIENT"`. +// - Generic OAuth JSON: `{ "error": "insufficient_scope" }` (RFC 6750 §3.1 +// as a body, which some providers emit instead of the header). +// +// Detection is deliberately strict — a false positive strips the +// re-authenticate recovery from a 403 that re-auth WOULD fix: +// +// - Challenge headers are PARSED into parameters (quoted values consumed +// whole), so `error=insufficient_scope` inside another parameter's +// quoted value never counts; only the `error` parameter's own exact +// value does. +// - Structured bodies match only exact `error` / `reason` field values. +// - Text bodies are JSON-parsed first and then inspected structurally; +// non-JSON text never classifies (prose mentioning the tokens misses). +// +// A miss is benign: the failure stays on the existing classification. + +export type InsufficientScopeDetection = { + /** Scopes the upstream named as required, when it named any (RFC 6750's + * `scope` attribute). Empty when the provider only signalled the class of + * failure (Google's ErrorInfo does not carry the missing scope). */ + readonly requiredScopes: readonly string[]; +}; + +const MAX_DEPTH = 8; + +const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null && !Array.isArray(value); + +/** Parser for the whole WWW-Authenticate header per RFC 7235 §2.1: a + * comma-separated #list of challenges, each + * `scheme [ 1*SP ( token68 / #auth-param ) ]`. Implemented as an explicit + * per-challenge state machine so params can never attach across challenge + * boundaries or to a token68 credential: + * + * - "scheme": just read a scheme; accepts a token68 OR a first auth-param + * (space-separated, no comma). + * - "params": accepts further auth-params ONLY after a comma. + * - "token68": accepts nothing; any trailing param is malformed. + * + * Auth-params allow BWS around `=` (RFC 7230). Quoted-strings consume + * quoted-pairs whole and must end at a separator. ANY malformed shape — + * scheme-less params, space-separated param runs, params after token68, + * stray quotes/bytes — returns null and never classifies: a miss is benign, + * a false positive strips a valid recovery path. */ +type Challenge = { readonly scheme: string; readonly params: Map }; + +// token68 characters (RFC 7235) minus `=` (handled as padding); also covers +// auth-param names and schemes (subsets of HTTP token). +const WORD_RE = /[A-Za-z0-9._~+/-]/; + +const parseChallenges = (header: string): readonly Challenge[] | null => { + const len = header.length; + const challenges: Challenge[] = []; + let current: Challenge | null = null; + let state: "boundary" | "scheme" | "token68" | "params" = "boundary"; + let sawComma = true; // header start counts as a list boundary + let i = 0; + + const readWord = (): string => { + const start = i; + while (i < len && WORD_RE.test(header[i]!)) i += 1; + return header.slice(start, i); + }; + // Returns null on an unterminated quote or a quote run into the next token. + const readQuoted = (): string | null => { + let value = ""; + i += 1; // opening quote + while (i < len) { + const ch = header[i]!; + if (ch === '"') { + i += 1; + return i >= len || /[\s,]/.test(header[i]!) ? value : null; + } + if (ch === "\\" && i + 1 < len) { + value += header[i + 1]; + i += 2; + continue; + } + value += ch; + i += 1; + } + return null; // unterminated + }; + + while (i < len) { + while (i < len && /\s/.test(header[i]!)) i += 1; + if (i >= len) break; + if (header[i] === ",") { + sawComma = true; + i += 1; + continue; + } + if (!WORD_RE.test(header[i]!)) return null; // stray quote/byte: malformed + const word = readWord(); + // Look ahead through BWS for `=` to classify the word. + let j = i; + while (j < len && /[ \t]/.test(header[j]!)) j += 1; + const isPaddingRun = (() => { + // An `=`-run directly on the word (no BWS) that is followed (after + // optional whitespace) by a comma or the end of input is token68 + // padding. An `=` followed by a value — even across BWS — is an + // auth-param (RFC 7230 allows BWS around `=`). + if (header[i] !== "=") return false; + let k = i; + while (k < len && header[k] === "=") k += 1; + while (k < len && /[ \t]/.test(header[k]!)) k += 1; + return k >= len || header[k] === ","; + })(); + + if (isPaddingRun) { + // token68 with padding — only legal directly after a scheme. + if (state !== "scheme" || sawComma) return null; + while (i < len && header[i] === "=") i += 1; + state = "token68"; + sawComma = false; + continue; + } + + if (header[j] === "=") { + // auth-param: `word BWS = BWS value`. + if (current === null) return null; // scheme-less param + if (state === "token68") return null; // params after token68 + if (state === "scheme" && sawComma) return null; // "Bearer, a=b" + if (state === "params" && !sawComma) return null; // space-separated run + i = j + 1; + while (i < len && /[ \t]/.test(header[i]!)) i += 1; + let value: string; + if (header[i] === '"') { + const quoted = readQuoted(); + if (quoted === null) return null; + value = quoted; + } else { + const start = i; + while (i < len && !/[\s,]/.test(header[i]!)) i += 1; + value = header.slice(start, i); + } + if (!current.params.has(word.toLowerCase())) { + current.params.set(word.toLowerCase(), value); + } + state = "params"; + sawComma = false; + continue; + } + + // Bare word: a new challenge's scheme at a list boundary, a token68 + // directly after a scheme, malformed anywhere else. + if (sawComma) { + current = { scheme: word.toLowerCase(), params: new Map() }; + challenges.push(current); + state = "scheme"; + sawComma = false; + continue; + } + if (state === "scheme") { + state = "token68"; + continue; + } + return null; + } + + return challenges; +}; + +const detectFromChallenge = (header: string): InsufficientScopeDetection | null => { + const challenges = parseChallenges(header); + if (challenges === null) return null; + for (const challenge of challenges) { + if (challenge.scheme !== "bearer") continue; + if (challenge.params.get("error") !== "insufficient_scope") continue; + const scope = challenge.params.get("scope"); + return { requiredScopes: scope ? scope.split(/\s+/).filter(Boolean) : [] }; + } + return null; +}; + +const detectFromStructured = (body: unknown, depth: number): boolean => { + if (depth > MAX_DEPTH) return false; + if (Array.isArray(body)) { + return body.some((item) => detectFromStructured(item, depth + 1)); + } + if (!isRecord(body)) return false; + if (body.error === "insufficient_scope") return true; + if (body.reason === "ACCESS_TOKEN_SCOPE_INSUFFICIENT") return true; + // Recurse into records/arrays only — nested strings are data (scope lists, + // docs prose), never the error envelope itself. + return Object.values(body).some( + (value) => (isRecord(value) || Array.isArray(value)) && detectFromStructured(value, depth + 1), + ); +}; + +const parseJsonSafe = (text: string): unknown => { + // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-json-parse -- boundary: classifying an untrusted upstream error body; a parse failure just means "not a JSON body", never an error path + try { + return JSON.parse(text) as unknown; + } catch { + return undefined; + } +}; + +const detectFromBody = (body: unknown): boolean => { + if (typeof body === "string") { + const parsed = parseJsonSafe(body); + return parsed !== undefined && detectFromStructured(parsed, 0); + } + return detectFromStructured(body, 0); +}; + +/** Strict detection over a text fragment whose JSON body survives only as a + * suffix of a transport error message (the MCP SDK embeds the failed POST's + * body after a fixed prefix). The JSON is isolated from the first `{` or `[` + * and parsed; prose without a parseable JSON envelope never classifies. */ +export const insufficientScopeFromEmbeddedJson = (text: string): boolean => { + const start = text.search(/[{[]/); + if (start < 0) return false; + const parsed = parseJsonSafe(text.slice(start)); + return parsed !== undefined && detectFromStructured(parsed, 0); +}; + +/** Inspect an upstream 401/403's body and headers for a scope-insufficiency + * signal. Returns `null` when nothing matches, so callers fall through to + * their existing classification. */ +export const detectInsufficientScope = (input: { + readonly body?: unknown; + readonly headers?: Record; +}): InsufficientScopeDetection | null => { + for (const [name, value] of Object.entries(input.headers ?? {})) { + if (name.toLowerCase() !== "www-authenticate") continue; + const detected = detectFromChallenge(value); + if (detected) return detected; + } + return detectFromBody(input.body) ? { requiredScopes: [] } : null; +}; diff --git a/packages/plugins/graphql/src/sdk/invoke.ts b/packages/plugins/graphql/src/sdk/invoke.ts index b99dcc381..819acefb0 100644 --- a/packages/plugins/graphql/src/sdk/invoke.ts +++ b/packages/plugins/graphql/src/sdk/invoke.ts @@ -139,6 +139,8 @@ export const invoke = Effect.fn("GraphQL.invoke")(function* ( status, data: gqlBody?.data ?? null, errors: hasErrors ? gqlBody!.errors : null, + body, + headers: { ...response.headers }, }); }); diff --git a/packages/plugins/graphql/src/sdk/plugin.test.ts b/packages/plugins/graphql/src/sdk/plugin.test.ts index 0ef97fedf..505f27879 100644 --- a/packages/plugins/graphql/src/sdk/plugin.test.ts +++ b/packages/plugins/graphql/src/sdk/plugin.test.ts @@ -480,6 +480,140 @@ describe("graphqlPlugin real protocol server", () => { }), ); + it.effect("classifies a 401 as connection_rejected even when the body is GraphQL-shaped", () => + Effect.gen(function* () { + // An auth gateway may answer with a GraphQL-style errors array AND a + // 401 status. The transport status is authoritative: the failure must + // reach the agent as a credential problem, not as graphql_errors. + const server = yield* serveTestHttpApp((request) => + Effect.gen(function* () { + const webRequest = yield* HttpServerRequest.toWeb(request); + const body = yield* Effect.promise(() => webRequest.text()); + if (body.includes("__schema")) { + return HttpServerResponse.jsonUnsafe({ data: introspectionResult }); + } + return HttpServerResponse.jsonUnsafe( + { errors: [{ message: "Not authenticated" }] }, + { status: 401 }, + ); + }), + ); + const executor = yield* makeExecutor(); + + yield* executor.graphql.addIntegration({ + endpoint: server.url("/graphql"), + slug: "auth_wall_graph", + }); + yield* createOrgConnection(executor, { + integration: "auth_wall_graph", + name: "main", + template: "none", + value: "unused", + }); + + const result = yield* executor.execute(toolAddr("auth_wall_graph", "main", "query.hello"), { + name: "Ada", + }); + + expect(result).toMatchObject({ + ok: false, + error: { code: "connection_rejected", status: 401 }, + }); + }), + ); + + it.effect( + "classifies a scope-insufficient 403 (OAuth error body) as oauth_scope_insufficient", + () => + Effect.gen(function* () { + // The 403 body is an OAuth error object, not GraphQL-shaped at all — + // exactly what an auth gateway in front of a GraphQL API returns. + const server = yield* serveTestHttpApp((request) => + Effect.gen(function* () { + const webRequest = yield* HttpServerRequest.toWeb(request); + const body = yield* Effect.promise(() => webRequest.text()); + if (body.includes("__schema")) { + return HttpServerResponse.jsonUnsafe({ data: introspectionResult }); + } + return HttpServerResponse.jsonUnsafe( + { error: "insufficient_scope", error_description: "needs repo scope" }, + { status: 403 }, + ); + }), + ); + const executor = yield* makeExecutor(); + + yield* executor.graphql.addIntegration({ + endpoint: server.url("/graphql"), + slug: "scope_graph", + }); + yield* createOrgConnection(executor, { + integration: "scope_graph", + name: "main", + template: "none", + value: "unused", + }); + + const result = yield* executor.execute(toolAddr("scope_graph", "main", "query.hello"), { + name: "Ada", + }); + + expect(result).toMatchObject({ + ok: false, + error: { code: "oauth_scope_insufficient", status: 403 }, + }); + const recovery = (result as { error: { details: { recovery: Record } } }) + .error.details.recovery; + expect( + recovery.startOAuthTool, + "no oauth.start hint: re-running the identical grant cannot satisfy the scope", + ).toBeUndefined(); + }), + ); + + it.effect( + "classifies a scope-insufficient 403 challenge header as oauth_scope_insufficient", + () => + Effect.gen(function* () { + const server = yield* serveTestHttpApp((request) => + Effect.gen(function* () { + const webRequest = yield* HttpServerRequest.toWeb(request); + const body = yield* Effect.promise(() => webRequest.text()); + if (body.includes("__schema")) { + return HttpServerResponse.jsonUnsafe({ data: introspectionResult }); + } + return HttpServerResponse.text("forbidden", { + status: 403, + headers: { + "www-authenticate": 'Bearer realm="api", error="insufficient_scope"', + }, + }); + }), + ); + const executor = yield* makeExecutor(); + + yield* executor.graphql.addIntegration({ + endpoint: server.url("/graphql"), + slug: "scope_hdr_graph", + }); + yield* createOrgConnection(executor, { + integration: "scope_hdr_graph", + name: "main", + template: "none", + value: "unused", + }); + + const result = yield* executor.execute(toolAddr("scope_hdr_graph", "main", "query.hello"), { + name: "Ada", + }); + + expect(result).toMatchObject({ + ok: false, + error: { code: "oauth_scope_insufficient", status: 403 }, + }); + }), + ); + it.effect("invokes OAuth-backed integrations with a rendered bearer token", () => Effect.gen(function* () { const server = yield* serveGraphqlTestServer({ diff --git a/packages/plugins/graphql/src/sdk/plugin.ts b/packages/plugins/graphql/src/sdk/plugin.ts index 9135687eb..1363b3a07 100644 --- a/packages/plugins/graphql/src/sdk/plugin.ts +++ b/packages/plugins/graphql/src/sdk/plugin.ts @@ -4,6 +4,7 @@ import { HttpClient } from "effect/unstable/http"; import { authToolFailure, + detectInsufficientScope, AuthTemplateSlug, definePlugin, IntegrationAlreadyExistsError, @@ -1134,6 +1135,46 @@ export const graphqlPlugin = definePlugin((options?: GraphqlPluginOptions) => { httpClientLayer, ); + // An HTTP auth wall is classified BEFORE the GraphQL-errors branch: a + // 401/403 body is usually not GraphQL-shaped at all (an auth + // gateway's OAuth error object), and even when it is, the transport + // status is the authoritative signal — labelling it graphql_errors + // would hide the credential problem from the agent entirely. + if (result.status === 401 || result.status === 403) { + // A scope-insufficient 403 is not fixable by re-authenticating + // the same grant; give it its own code so the agent stops looping + // through identical consent flows. Detection reads the RAW body and + // response headers (RFC 6750 WWW-Authenticate challenge), not the + // GraphQL projection. + const insufficientScope = + result.status === 403 + ? detectInsufficientScope({ body: result.body, headers: result.headers }) + : null; + if (insufficientScope) { + return authToolFailure({ + code: "oauth_scope_insufficient", + status: result.status, + message: `The connection for GraphQL integration "${integration}" is authorized, but its grant does not cover the scope this operation requires. Re-authenticating with the same grant will return the same error; reconnect with broader access.`, + integration: { id: integration, scope: credential.owner }, + credential: { kind: "oauth", label: "Upstream authorization" }, + upstream: { + status: result.status, + details: { body: result.body }, + }, + }); + } + return authToolFailure({ + code: "connection_rejected", + status: result.status, + message: `Upstream rejected credentials for GraphQL integration "${integration}" with HTTP ${result.status}. Re-authenticate or update the connection before retrying this tool.`, + integration: { id: integration, scope: credential.owner }, + credential: { kind: "upstream", label: "Upstream authorization" }, + upstream: { + status: result.status, + details: { body: result.body }, + }, + }); + } const errors = decodeGraphqlErrors(result.errors); if (errors !== undefined && errors.length > 0) { const firstMessage = extractGraphqlErrorMessage(errors); @@ -1144,22 +1185,6 @@ export const graphqlPlugin = definePlugin((options?: GraphqlPluginOptions) => { }); } if (result.status < 200 || result.status >= 300) { - if (result.status === 401 || result.status === 403) { - return authToolFailure({ - code: "connection_rejected", - status: result.status, - message: `Upstream rejected credentials for GraphQL integration "${integration}" with HTTP ${result.status}. Re-authenticate or update the connection before retrying this tool.`, - integration: { id: integration, scope: credential.owner }, - credential: { kind: "upstream", label: "Upstream authorization" }, - upstream: { - status: result.status, - details: { - data: result.data, - errors: result.errors, - }, - }, - }); - } return ToolResult.fail({ code: "graphql_http_error", status: result.status, diff --git a/packages/plugins/graphql/src/sdk/types.ts b/packages/plugins/graphql/src/sdk/types.ts index f9cb00899..6189d192b 100644 --- a/packages/plugins/graphql/src/sdk/types.ts +++ b/packages/plugins/graphql/src/sdk/types.ts @@ -210,5 +210,11 @@ export const InvocationResult = Schema.Struct({ status: Schema.Number, data: Schema.NullOr(Schema.Unknown), errors: Schema.NullOr(Schema.Unknown), + /** The raw response body (parsed JSON or text) and response headers: a + * non-2xx response is usually NOT GraphQL-shaped (an auth gateway's OAuth + * error object, an HTML page), so 401/403 classification needs what + * `data`/`errors` cannot carry. */ + body: Schema.optional(Schema.Unknown), + headers: Schema.optional(Schema.Record(Schema.String, Schema.String)), }); export type InvocationResult = typeof InvocationResult.Type; diff --git a/packages/plugins/mcp/src/sdk/connection.ts b/packages/plugins/mcp/src/sdk/connection.ts index 4edf6b5d1..74e031d0f 100644 --- a/packages/plugins/mcp/src/sdk/connection.ts +++ b/packages/plugins/mcp/src/sdk/connection.ts @@ -16,8 +16,13 @@ import { HttpClient, HttpClientRequest } from "effect/unstable/http"; // stdio branch of `createMcpConnector`. import type { McpRemoteIntegrationConfig, McpStdioIntegrationConfig } from "./types"; -import { McpConnectionError, McpOAuthReauthorizationRequired } from "./errors"; +import { + McpConnectionError, + McpInsufficientScopeError, + McpOAuthReauthorizationRequired, +} from "./errors"; import { httpStatusFromCause } from "./http-status"; +import { detectInsufficientScope } from "@executor-js/sdk/core"; // --------------------------------------------------------------------------- // Connection type @@ -134,6 +139,25 @@ const fetchFromHttpClientLayer = ( for (const [key, value] of Object.entries(response.headers)) { if (value !== undefined) responseHeaders.set(key, value); } + // A 403 carrying an RFC 6750 insufficient_scope challenge is + // intercepted HERE, below the SDK: with an authProvider the SDK would + // consume the challenge and re-run auth ("upscoping"), which our + // static-token provider can only answer by demanding reauthorization — + // misclassifying an unfixable scope shortfall as oauth_reauth_required. + if (response.status === 403) { + const challenge = response.headers["www-authenticate"]; + if ( + challenge !== undefined && + detectInsufficientScope({ headers: { "www-authenticate": challenge } }) !== null + ) { + return yield* Effect.die( + new McpInsufficientScopeError({ + message: + "MCP server rejected the call: the OAuth grant does not cover the required scope", + }), + ); + } + } const body = response.status === 204 || response.status === 205 || response.status === 304 ? null @@ -186,6 +210,16 @@ const connectionFailure = ( if (Predicate.isTagged(cause, "McpOAuthReauthorizationRequired")) { return new McpOAuthReauthorizationRequired({ message: "MCP OAuth re-authorization required" }); } + if (Predicate.isTagged(cause, "McpInsufficientScopeError")) { + // Surfaced as a connection error with the 403 status; the invoke/connect + // catch sites detect the tag and classify as oauth_scope_insufficient. + return new McpConnectionError({ + transport, + message: `${message} (HTTP 403: insufficient scope)`, + httpStatus: 403, + insufficientScope: true, + }); + } // Carry the handshake HTTP status structurally (and in the message for // humans) so the liveness health check can classify a rejected credential // as expired rather than a generic connection failure. diff --git a/packages/plugins/mcp/src/sdk/errors.ts b/packages/plugins/mcp/src/sdk/errors.ts index b5cbc9471..7ea487d14 100644 --- a/packages/plugins/mcp/src/sdk/errors.ts +++ b/packages/plugins/mcp/src/sdk/errors.ts @@ -12,6 +12,10 @@ export class McpConnectionError extends Schema.TaggedErrorClass {} export class McpOAuthReauthorizationRequired extends Data.TaggedError( @@ -45,6 +54,16 @@ export class McpOAuthReauthorizationRequired extends Data.TaggedError( readonly message: string; }> {} +/** Thrown by the fetch adapter when a 403 carries an RFC 6750 + * `error="insufficient_scope"` challenge. Raised BELOW the MCP SDK on + * purpose: with an authProvider present the SDK would otherwise consume the + * challenge and re-run auth ("upscoping"), which our static-token provider + * can only answer by demanding reauthorization — the exact loop the + * oauth_scope_insufficient classification exists to break. */ +export class McpInsufficientScopeError extends Data.TaggedError("McpInsufficientScopeError")<{ + readonly message: string; +}> {} + export class McpOAuthError extends Schema.TaggedErrorClass()( "McpOAuthError", { diff --git a/packages/plugins/mcp/src/sdk/http-status.test.ts b/packages/plugins/mcp/src/sdk/http-status.test.ts new file mode 100644 index 000000000..dd546720c --- /dev/null +++ b/packages/plugins/mcp/src/sdk/http-status.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from "@effect/vitest"; + +import { insufficientScopeFromCause } from "./http-status"; + +// The MCP SDK surfaces a 403 two ways, and both must classify: +// - no authProvider: the transport throws with the response body embedded +// in the message ("Error POSTing to endpoint: "); +// - with an authProvider (the production OAuth path): the StreamableHTTP +// transport consumes the insufficient_scope challenge itself, retries +// with the broader scope, and only when THAT fails throws the fixed +// "Server returned 403 after trying upscoping" message. +describe("insufficientScopeFromCause", () => { + it("detects the OAuth error body embedded in a transport message", () => { + expect( + insufficientScopeFromCause( + new Error( + 'Error POSTing to endpoint: {"error":"insufficient_scope","error_description":"needs files.read"}', + ), + ), + ).toBe(true); + }); + + it("detects a Google ErrorInfo body embedded in a transport message", () => { + expect( + insufficientScopeFromCause( + new Error( + 'Error POSTing to endpoint: {"error":{"details":[{"reason":"ACCESS_TOKEN_SCOPE_INSUFFICIENT"}]}}', + ), + ), + ).toBe(true); + }); + + it("detects the SDK's exhausted-upscoping failure (the authProvider path)", () => { + expect( + insufficientScopeFromCause(new Error("Server returned 403 after trying upscoping")), + ).toBe(true); + }); + + it("ignores prose that merely mentions the tokens", () => { + expect( + insufficientScopeFromCause( + new Error( + "Error POSTing to endpoint: see the OAuth docs about insufficient_scope handling", + ), + ), + ).toBe(false); + }); + + it("ignores non-error causes", () => { + expect(insufficientScopeFromCause(undefined)).toBe(false); + expect(insufficientScopeFromCause("insufficient_scope")).toBe(false); + }); +}); diff --git a/packages/plugins/mcp/src/sdk/http-status.ts b/packages/plugins/mcp/src/sdk/http-status.ts index 366549993..a4442f8d2 100644 --- a/packages/plugins/mcp/src/sdk/http-status.ts +++ b/packages/plugins/mcp/src/sdk/http-status.ts @@ -8,6 +8,7 @@ import { Option, Schema } from "effect"; +import { insufficientScopeFromEmbeddedJson } from "@executor-js/sdk/core"; import { StreamableHTTPError } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; const SsePostErrorCause = Schema.Struct({ message: Schema.String }); @@ -34,3 +35,23 @@ const statusFromStreamableHttpError = (cause: unknown): number | undefined => { export const httpStatusFromCause = (cause: unknown): number | undefined => statusFromStreamableHttpError(cause) ?? statusFromSsePostError(cause); + +// The SDK embeds the upstream response text in the transport error message +// ("Error POSTing to endpoint: "), which is the only place a 403's body +// survives for connections without an authProvider. For OAuth connections the +// StreamableHTTP transport consumes the insufficient_scope challenge ITSELF: +// it re-runs auth requesting the broader scope, and only when that upscoped +// retry still 403s does it throw — with the fixed message matched below +// (verified against @modelcontextprotocol/sdk streamableHttp.js; re-verify on +// SDK bumps). Both paths mean the same thing: the grant does not cover the +// operation, and re-running the identical flow cannot help. Strict matching +// (exact serialized field forms via the shared core detector, or the SDK's +// exact upscoping message) — a miss stays on the generic auth path. +const SDK_UPSCOPING_EXHAUSTED_RE = /Server returned 403 after trying upscoping/; + +export const insufficientScopeFromCause = (cause: unknown): boolean => + Option.match(decodeSsePostErrorCause(cause), { + onNone: () => false, + onSome: ({ message }) => + insufficientScopeFromEmbeddedJson(message) || SDK_UPSCOPING_EXHAUSTED_RE.test(message), + }); diff --git a/packages/plugins/mcp/src/sdk/invoke.ts b/packages/plugins/mcp/src/sdk/invoke.ts index be68fbde7..e14998fab 100644 --- a/packages/plugins/mcp/src/sdk/invoke.ts +++ b/packages/plugins/mcp/src/sdk/invoke.ts @@ -30,7 +30,7 @@ import { import { McpConnectionError, McpInvocationError, McpOAuthReauthorizationRequired } from "./errors"; import type { McpConnection, McpConnector } from "./connection"; -import { httpStatusFromCause } from "./http-status"; +import { httpStatusFromCause, insufficientScopeFromCause } from "./http-status"; // --------------------------------------------------------------------------- // Helpers @@ -172,12 +172,26 @@ const useConnection = ( message: "MCP OAuth re-authorization required", }); } + // Raised by the fetch adapter when the 403 carried an RFC 6750 + // insufficient_scope challenge (the authProvider path, where the SDK + // would otherwise consume the challenge before we could see it). + if (Predicate.isTagged(cause, "McpInsufficientScopeError")) { + return new McpInvocationError({ + toolName, + message: `MCP tool call failed for ${toolName}`, + status: 403, + insufficientScope: true, + }); + } const status = httpStatusFromCause(cause); return new McpInvocationError({ toolName, message: `MCP tool call failed for ${toolName}`, ...(status === undefined ? {} : { status }), ...(isUnknownToolCause(cause, toolName) ? { unknownTool: true } : {}), + ...(status === 403 && insufficientScopeFromCause(cause) + ? { insufficientScope: true } + : {}), }); }, }).pipe( diff --git a/packages/plugins/mcp/src/sdk/plugin.test.ts b/packages/plugins/mcp/src/sdk/plugin.test.ts index 48a223383..0b8338684 100644 --- a/packages/plugins/mcp/src/sdk/plugin.test.ts +++ b/packages/plugins/mcp/src/sdk/plugin.test.ts @@ -119,7 +119,14 @@ const jsonRpcErrorCallTool = error: { code, message: "application-level do-not-leak" }, }); -const seedCallToolExecutor = (input: { slug: string; callTool: CallToolResponder }) => +const seedCallToolExecutor = (input: { + slug: string; + callTool: CallToolResponder; + /** Seed an oauth2-templated connection with a static access token, so the + * transport gets a real authProvider — the production OAuth path where the + * SDK's own challenge handling would otherwise swallow scope signals. */ + oauth?: boolean; +}) => Effect.acquireRelease( Effect.gen(function* () { const server = yield* serveCallToolServer(input.callTool); @@ -133,13 +140,14 @@ const seedCallToolExecutor = (input: { slug: string; callTool: CallToolResponder endpoint: server.url("/mcp"), slug: input.slug, remoteTransport: "streamable-http", + ...(input.oauth ? { auth: { kind: "oauth2" as const } } : {}), }); yield* executor.connections.create({ owner: "org", name: ConnectionName.make("main"), integration: IntegrationSlug.make(input.slug), - template: TEMPLATE, - value: "", + template: input.oauth ? AuthTemplateSlug.make("oauth2") : TEMPLATE, + value: input.oauth ? "static-access-token" : "", }); return { @@ -643,6 +651,100 @@ describe("mcpPlugin", () => { ); } + it.effect( + "classifies a scope-insufficient 403 as oauth_scope_insufficient, not connection_rejected", + () => + Effect.scoped( + Effect.gen(function* () { + const slug = "call_status_scope"; + const { executor, toolAddress } = yield* seedCallToolExecutor({ + slug, + // RFC 6750 insufficient_scope in the response body: re-running the + // same grant cannot fix this, so the failure must not carry the + // re-authenticate recovery (whose oauth.start hint would loop). + callTool: () => + HttpServerResponse.text( + '{"error":"insufficient_scope","error_description":"do-not-leak: needs files.read"}', + { status: 403 }, + ), + }); + + const result = yield* executor.execute(toolAddress, {}, { onElicitation: "accept-all" }); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "oauth_scope_insufficient", + status: 403, + retryable: false, + details: { + category: "authentication", + integration: { id: slug }, + credential: { kind: "oauth", label: "main" }, + upstream: { status: 403 }, + }, + }, + }); + + const failure = result as { + readonly ok: false; + readonly error: { + readonly message: string; + readonly details: { readonly recovery: Record }; + }; + }; + expect(failure.error.message).not.toContain("do-not-leak"); + expect( + failure.error.details.recovery.startOAuthTool, + "no oauth.start hint: re-running the identical grant cannot satisfy the scope", + ).toBeUndefined(); + expect(failure.error.details.recovery.scopeInstructions).toBeDefined(); + }), + ), + ); + + it.effect( + "classifies a challenge-header scope 403 on an OAuth connection as oauth_scope_insufficient", + () => + Effect.scoped( + Effect.gen(function* () { + // The production path: an oauth2-templated connection gives the + // transport an authProvider, and the MCP SDK reacts to an RFC 6750 + // insufficient_scope challenge by re-running auth itself + // ("upscoping") — which our static-token provider can only answer by + // demanding reauthorization. The fetch adapter intercepts the + // challenge below the SDK, so the failure classifies as the scope + // shortfall it is instead of oauth_reauth_required. + const slug = "call_status_oauth_scope"; + const { executor, toolAddress } = yield* seedCallToolExecutor({ + slug, + oauth: true, + callTool: () => + HttpServerResponse.text("do-not-leak: forbidden", { + status: 403, + headers: { + "www-authenticate": + 'Bearer realm="mcp", error="insufficient_scope", scope="files.read"', + }, + }), + }); + + const result = yield* executor.execute(toolAddress, {}, { onElicitation: "accept-all" }); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "oauth_scope_insufficient", + status: 403, + details: { category: "authentication" }, + }, + }); + const failure = result as { readonly error: { readonly message: string } }; + expect(failure.error.message).not.toContain("do-not-leak"); + }), + ), + ); + it.effect("does not classify non-auth tools/call HTTP failures as auth failures", () => Effect.scoped( Effect.gen(function* () { diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index 9b84e4d81..4459f2bc1 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -292,8 +292,22 @@ const mcpInvocationAuthFailure = (input: { readonly status: 401 | 403; readonly integration: string; readonly connection: string; -}) => - authToolFailure({ + readonly insufficientScope?: boolean; +}) => { + // A 403 that named a scope shortfall is unfixable by re-running the same + // grant, so it carries its own code (and recovery without the oauth.start + // hint) instead of the re-authenticate loop. + if (input.status === 403 && input.insufficientScope === true) { + return authToolFailure({ + code: "oauth_scope_insufficient", + message: `MCP server rejected connection "${input.connection}" with HTTP 403: the connection's OAuth grant does not cover the scope this tool requires. Re-authenticating with the same grant will return the same error; reconnect with broader access.`, + integration: { id: input.integration }, + credential: { kind: "oauth", label: input.connection }, + status: input.status, + upstream: { status: input.status }, + }); + } + return authToolFailure({ code: "connection_rejected", message: input.status === 403 @@ -304,6 +318,7 @@ const mcpInvocationAuthFailure = (input: { status: input.status, upstream: { status: input.status }, }); +}; const mcpInvocationOAuthReauthFailure = (input: { readonly integration: string; @@ -1328,6 +1343,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { status: error.httpStatus, integration: String(credential.integration), connection: String(credential.connection), + ...(error.insufficientScope === true ? { insufficientScope: true } : {}), }), ); } @@ -1347,6 +1363,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { status: error.status, integration: String(credential.integration), connection: String(credential.connection), + ...(error.insufficientScope === true ? { insufficientScope: true } : {}), }), ); } diff --git a/packages/plugins/openapi/src/sdk/backing.ts b/packages/plugins/openapi/src/sdk/backing.ts index 753874ad6..234cefe09 100644 --- a/packages/plugins/openapi/src/sdk/backing.ts +++ b/packages/plugins/openapi/src/sdk/backing.ts @@ -8,6 +8,7 @@ import { ToolResult, authToolFailure, classifyHttpStatus, + detectInsufficientScope, sortHealthCheckCandidatesByIdentity, extractIdentity, extractResponseFields, @@ -721,6 +722,28 @@ export const invokeOpenApiBackedTool = (input: { const ok = result.status >= 200 && result.status < 300; if (!ok) { if (result.status === 401 || result.status === 403) { + // A 403 naming a scope shortfall (RFC 6750 insufficient_scope, + // Google's ACCESS_TOKEN_SCOPE_INSUFFICIENT) cannot be fixed by + // re-running the same grant, so it gets its own code and recovery + // guidance instead of the re-authenticate loop. + const insufficientScope = + result.status === 403 + ? detectInsufficientScope({ body: result.error, headers: result.headers }) + : null; + if (insufficientScope) { + const required = insufficientScope.requiredScopes; + return openApiAuthToolFailure({ + code: "oauth_scope_insufficient", + status: result.status, + message: `The connection "${input.credential.connection}" for "${integration}" is authorized, but its grant does not cover the scope this operation requires${required.length > 0 ? ` (${required.join(" ")})` : ""}. Re-authenticating with the same grant will return the same error; reconnect with broader access.`, + owner: input.credential.owner, + integration, + connection: String(input.credential.connection), + credentialKind: "oauth", + credentialLabel: "Upstream authorization", + details: result.error, + }); + } return openApiAuthToolFailure({ code: "connection_rejected", status: result.status, diff --git a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts index 961fd14aa..146876be4 100644 --- a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts +++ b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts @@ -219,6 +219,109 @@ describe("OpenAPI upstream failure modes", () => { }), ); + // A 403 that names a scope shortfall is unfixable by re-running the same + // grant: it must NOT be connection_rejected (whose recovery tells the agent + // to oauth.start the identical grant and loop on the identical 403). + it.effect( + "scope-insufficient 403 (Google ErrorInfo) is classified as oauth_scope_insufficient", + () => + Effect.gen(function* () { + const server = yield* startScriptedServer(() => ({ + status: 403, + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + error: { + code: 403, + message: "Request had insufficient authentication scopes.", + status: "PERMISSION_DENIED", + details: [ + { + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + reason: "ACCESS_TOKEN_SCOPE_INSUFFICIENT", + domain: "googleapis.com", + metadata: { service: "drive.googleapis.com" }, + }, + ], + }, + }), + })); + const { executor, address } = yield* buildExecutorForOpenApiServer(server); + + const result = yield* executor.execute(address, {}); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "oauth_scope_insufficient", + status: 403, + message: expect.stringContaining("does not cover the scope"), + details: { + category: "authentication", + upstream: { status: 403 }, + }, + }, + }); + const recovery = ( + result as { + error: { details: { recovery: Record } }; + } + ).error.details.recovery; + expect( + recovery.startOAuthTool, + "no oauth.start hint: re-running the identical grant cannot satisfy the scope", + ).toBeUndefined(); + expect(recovery.scopeInstructions).toBeDefined(); + }), + ); + + it.effect( + "scope-insufficient 403 (RFC 6750 WWW-Authenticate challenge) is classified as oauth_scope_insufficient", + () => + Effect.gen(function* () { + const server = yield* startScriptedServer(() => ({ + status: 403, + headers: { + "content-type": "application/json", + "www-authenticate": + 'Bearer realm="api", error="insufficient_scope", scope="files.read"', + }, + body: '{"message":"forbidden"}', + })); + const { executor, address } = yield* buildExecutorForOpenApiServer(server); + + const result = yield* executor.execute(address, {}); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "oauth_scope_insufficient", + status: 403, + // The challenge names the missing scope; the message carries it so + // the agent can tell the user exactly what to grant. + message: expect.stringContaining("files.read"), + }, + }); + }), + ); + + it.effect("ordinary 403 without a scope signal stays connection_rejected", () => + Effect.gen(function* () { + const server = yield* startScriptedServer(() => ({ + status: 403, + headers: { "content-type": "application/json" }, + body: '{"error":{"status":"PERMISSION_DENIED","message":"Caller lacks permission"}}', + })); + const { executor, address } = yield* buildExecutorForOpenApiServer(server); + + const result = yield* executor.execute(address, {}); + + expect(result).toMatchObject({ + ok: false, + error: { code: "connection_rejected", status: 403 }, + }); + }), + ); + it.effect("upstream returns malformed JSON despite Content-Type: application/json", () => Effect.gen(function* () { const server = yield* startScriptedServer(() => ({