History

Surface rejected OAuth token refreshes as auth failures (#1422)

* Surface AS-rejected token refreshes as actionable auth failures A refresh-token grant the authorization server rejects with an RFC 6749 error code other than invalid_grant was classified as a StorageError, which the sandbox boundary scrubs to "Internal tool error [id]" - the caller had no signal that the connection needed re-authentication. Any refresh rejection carrying an RFC 6749 error code now maps to CredentialResolutionError, so it reaches the sandbox as an oauth_refresh_failed auth failure with the server's error code and description (invalid_grant still maps to oauth_reauth_required). Code-less failures (transport blips, non-OAuth-shaped responses) stay StorageError so the next invoke retries. * Assert the rejected-refresh failure over the MCP surface The scenario now invokes the tool through a real MCP session (the channel the regression actually hid the failure on) instead of the executions API, so it covers dispatch, classification, and MCP result rendering end to end.

Rhys Sullivan committed 3 months ago

Commit e2712db1 parent 13732fe5 files · +415 −13Browse files

Unified · Split

5 files
  1. .changeset/oauth-refresh-rejection-surface.md +6 −0
  2. e2e/scenarios/oauth-refresh-rejected.test.ts +286 −0
  3. packages/core/execution/src/tool-invoker.test.ts +89 −0
  4. packages/core/sdk/src/executor.ts +28 −12
  5. packages/core/sdk/src/testing/oauth-test-server.ts +6 −1
.changeset/oauth-refresh-rejection-surface.md+6−0
@@ -0,0 +1,6 @@
1+---
2+"@executor-js/sdk": patch
3+"@executor-js/execution": patch
4+---
5+
6+A token refresh the authorization server definitively rejects (any RFC 6749 error code, not just `invalid_grant`) now surfaces to the sandbox as an `oauth_refresh_failed` auth failure carrying the server's error code and description, instead of being scrubbed to "Internal tool error". `invalid_grant` still classifies as `oauth_reauth_required`. Code-less failures (transport blips) keep retrying as before.
e2e/scenarios/oauth-refresh-rejected.test.ts+286−0
@@ -0,0 +1,286 @@
1+// Cross-target: when the authorization server definitively rejects a
2+// refresh-token grant, the sandbox must see an auth failure it can act on —
3+// never the scrubbed "Internal tool error [hex]" defect.
4+//
5+// Prod regression (2026-07-16): an AS answered refresh grants with a 400
6+// whose RFC 6749 error code was NOT `invalid_grant`. Executor classified
7+// that as a storage failure, and the opaque-defect boundary scrubbed it to
8+// "Internal tool error [id]" — so agents (and the humans driving them) had
9+// no signal that the connection simply needed to be re-authenticated.
10+//
11+// The journey: an OpenAPI integration completes a real authorization-code
12+// flow against a live test AS that mints instantly-expiring access tokens
13+// and rejects every refresh grant with `invalid_request`. The first tool
14+// call must therefore refresh, the AS says a definitive no, and the failure
15+// the sandbox sees carries code `oauth_refresh_failed`, the AS's own error
16+// code and description, and connection-recovery guidance.
17+import { randomBytes } from "node:crypto";
18+import { createServer } from "node:http";
19+
20+import { expect } from "@effect/vitest";
21+import { Effect } from "effect";
22+import { composePluginApi } from "@executor-js/api/server";
23+import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";
24+import {
25+ AuthTemplateSlug,
26+ ConnectionName,
27+ IntegrationSlug,
28+ OAuthClientSlug,
29+} from "@executor-js/sdk/shared";
30+import { serveOAuthTestServer } from "@executor-js/sdk/testing";
31+
32+import { scenario } from "../src/scenario";
33+import { Api, Mcp, Target } from "../src/services";
34+
35+const api = composePluginApi([openApiHttpPlugin()] as const);
36+
37+const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`;
38+
39+/** Upstream on 127.0.0.1: `GET /issues` is 200 for any bearer. The refresh is
40+ * rejected before any upstream call, so this only proves the failure came
41+ * from the token endpoint, not from here. */
42+const serveUpstream = () =>
43+ Effect.acquireRelease(
44+ Effect.callback<{ readonly url: string; readonly close: () => void }>((resume) => {
45+ const server = createServer((request, response) => {
46+ if (request.method === "GET" && (request.url ?? "").startsWith("/issues")) {
47+ response.writeHead(200, { "content-type": "application/json" });
48+ response.end(JSON.stringify({ issues: [] }));
49+ return;
50+ }
51+ response.writeHead(404, { "content-type": "application/json" });
52+ response.end(JSON.stringify({ error: "not_found" }));
53+ });
54+ server.listen(0, "127.0.0.1", () => {
55+ const address = server.address();
56+ const port = typeof address === "object" && address ? address.port : 0;
57+ resume(
58+ Effect.succeed({
59+ url: `http://127.0.0.1:${port}`,
60+ close: () => {
61+ server.close();
62+ server.closeAllConnections();
63+ },
64+ }),
65+ );
66+ });
67+ }),
68+ (server) => Effect.sync(server.close),
69+ );
70+
71+const spec = (
72+ baseUrl: string,
73+ oauth: { readonly authorizationEndpoint: string; readonly tokenEndpoint: string },
74+): string =>
75+ JSON.stringify({
76+ openapi: "3.0.3",
77+ info: { title: "Issues API", version: "1.0.0" },
78+ servers: [{ url: baseUrl }],
79+ paths: {
80+ "/issues": {
81+ get: {
82+ operationId: "listIssues",
83+ summary: "List issues",
84+ security: [{ oauth: ["issues.read"] }],
85+ responses: { "200": { description: "issues" } },
86+ },
87+ },
88+ },
89+ components: {
90+ securitySchemes: {
91+ oauth: {
92+ type: "oauth2",
93+ flows: {
94+ authorizationCode: {
95+ authorizationUrl: oauth.authorizationEndpoint,
96+ tokenUrl: oauth.tokenEndpoint,
97+ scopes: { "issues.read": "Read issues" },
98+ },
99+ },
100+ },
101+ },
102+ },
103+ });
104+
105+const invokeByAddressCode = (address: string, args: unknown) => `
106+const segments = ${JSON.stringify(address)}.split(".").slice(1);
107+let node = tools;
108+for (const segment of segments) node = node[segment];
109+const result = await node(${JSON.stringify(args)});
110+return JSON.stringify(result);
111+`;
112+
113+type ToolEnvelope = {
114+ readonly ok: boolean;
115+ readonly error?: {
116+ readonly code?: string;
117+ readonly message?: string;
118+ readonly details?: {
119+ readonly category?: string;
120+ readonly recovery?: Record<string, string>;
121+ };
122+ };
123+};
124+
125+scenario(
126+ "Auth failures · a refresh rejected by the authorization server surfaces as an actionable auth failure, not an internal error",
127+ {},
128+ Effect.scoped(
129+ Effect.gen(function* () {
130+ const target = yield* Target;
131+ const { client: makeClient } = yield* Api;
132+ const mcp = yield* Mcp;
133+ const identity = yield* target.newIdentity();
134+ const client = yield* makeClient(api, identity);
135+ const upstream = yield* serveUpstream();
136+ // Instantly-expiring access tokens force the first tool call to
137+ // refresh; the AS rejects every refresh grant with a non-invalid_grant
138+ // 400, mirroring the prod AS that triggered the regression.
139+ const oauth = yield* serveOAuthTestServer({
140+ scopes: ["issues.read"],
141+ tokenExpiresInSeconds: 0,
142+ supportRefresh: false,
143+ invalidRefreshTokenErrorCode: "invalid_request",
144+ invalidRefreshTokenDescription: "Refresh token expired",
145+ });
146+ const slug = unique("refreshrej");
147+ const clientSlug = OAuthClientSlug.make(unique("refreshrejc"));
148+
149+ yield* Effect.ensuring(
150+ Effect.gen(function* () {
151+ yield* client.openapi.addSpec({
152+ payload: {
153+ spec: { kind: "blob", value: spec(upstream.url, oauth) },
154+ slug,
155+ baseUrl: upstream.url,
156+ authenticationTemplate: [
157+ {
158+ slug: "oauth",
159+ kind: "oauth2",
160+ authorizationUrl: oauth.authorizationEndpoint,
161+ tokenUrl: oauth.tokenEndpoint,
162+ scopes: ["issues.read"],
163+ },
164+ ],
165+ },
166+ });
167+ yield* client.oauth.createClient({
168+ payload: {
169+ owner: "org",
170+ slug: clientSlug,
171+ grant: "authorization_code",
172+ authorizationUrl: oauth.authorizationEndpoint,
173+ tokenUrl: oauth.tokenEndpoint,
174+ clientId: "test-client",
175+ clientSecret: "test-secret",
176+ originIntegration: IntegrationSlug.make(slug),
177+ },
178+ });
179+
180+ const started = yield* client.oauth.start({
181+ payload: {
182+ client: clientSlug,
183+ clientOwner: "org",
184+ owner: "org",
185+ name: ConnectionName.make("main"),
186+ integration: IntegrationSlug.make(slug),
187+ template: AuthTemplateSlug.make("oauth"),
188+ },
189+ });
190+ expect(started.status, "oauth.start redirects to the authorization server").toBe(
191+ "redirect",
192+ );
193+ if (started.status !== "redirect") return yield* Effect.die("no redirect");
194+
195+ // Drive the test IdP's consent by hand (authorize → login → code).
196+ const code = yield* Effect.promise(async () => {
197+ const authorize = await fetch(started.authorizationUrl, { redirect: "manual" });
198+ const loginUrl = authorize.headers.get("location");
199+ if (!loginUrl) throw new Error(`authorize did not redirect: ${authorize.status}`);
200+ const login = await fetch(loginUrl, {
201+ method: "POST",
202+ headers: {
203+ authorization: `Basic ${Buffer.from("alice:password").toString("base64")}`,
204+ },
205+ redirect: "manual",
206+ });
207+ const callbackUrl = login.headers.get("location");
208+ if (!callbackUrl) throw new Error(`login did not redirect: ${login.status}`);
209+ const minted = new URL(callbackUrl).searchParams.get("code");
210+ if (!minted) throw new Error("callback carried no authorization code");
211+ return minted;
212+ });
213+ yield* client.oauth.complete({ payload: { state: started.state, code } });
214+
215+ const tools = yield* client.tools.list({ query: {} });
216+ const address = tools
217+ .filter((tool) => String(tool.integration) === slug)
218+ .map((tool) => String(tool.address))
219+ .find((addr) => addr.endsWith("listIssues"));
220+ expect(address, "the listIssues tool is in the catalog").toBeDefined();
221+
222+ // Call through the real MCP surface — the exact channel the prod
223+ // regression hid the failure on — so the assertion covers the whole
224+ // path an MCP client sees: dispatch, classification, rendering.
225+ const session = mcp.session(identity);
226+ let called = yield* session.call("execute", {
227+ code: invokeByAddressCode(address!, {}),
228+ });
229+ // Approval-gated tools pause the execution once per gated call.
230+ let guard = 0;
231+ while (called.text.includes("executionId:") && guard < 10) {
232+ called = yield* session.approvePaused(called.text);
233+ guard += 1;
234+ }
235+ expect(
236+ called.ok,
237+ `the MCP execute call itself completed (got: ${called.text.slice(0, 400)})`,
238+ ).toBe(true);
239+ const failure = JSON.parse(called.text) as ToolEnvelope;
240+
241+ // THE guarantee: the AS's definitive rejection reaches the agent
242+ // as an auth failure with the AS's own verdict in the message.
243+ expect(failure.ok, "the tool call failed").toBe(false);
244+ expect(
245+ failure.error?.code,
246+ "the failure is classified as a rejected refresh, not an internal defect",
247+ ).toBe("oauth_refresh_failed");
248+ expect(
249+ failure.error?.message ?? "",
250+ "the message carries the AS's RFC 6749 error code",
251+ ).toContain("invalid_request");
252+ expect(
253+ failure.error?.message ?? "",
254+ "the message carries the AS's error description",
255+ ).toContain("Refresh token expired");
256+ expect(
257+ failure.error?.message ?? "",
258+ "the opaque defect message never reaches the sandbox",
259+ ).not.toContain("Internal tool error");
260+ expect(failure.error?.details?.category, "the failure is auth-flavored").toBe(
261+ "authentication",
262+ );
263+ expect(
264+ failure.error?.details?.recovery?.oauthInstructions ?? "",
265+ "the recovery tells the agent how to re-connect via OAuth",
266+ ).toContain("startOAuthTool");
267+ }),
268+ Effect.gen(function* () {
269+ yield* client.connections
270+ .remove({
271+ params: {
272+ owner: "org",
273+ integration: IntegrationSlug.make(slug),
274+ name: ConnectionName.make("main"),
275+ },
276+ })
277+ .pipe(Effect.ignore);
278+ yield* client.oauth
279+ .removeClient({ params: { slug: clientSlug }, payload: { owner: "org" } })
280+ .pipe(Effect.ignore);
281+ yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore);
282+ }),
283+ );
284+ }),
285+ ),
286+);
packages/core/execution/src/tool-invoker.test.ts+89−0
@@ -1285,6 +1285,95 @@ describe("tool discovery", () => { · 1284 unmodified lines
12851285 ),
12861286 );
12871287
1288+ // Prod regression (Pylon, 2026-07-16): the AS rejected refresh grants with a
1289+ // 400 whose error code was NOT invalid_grant. That is a definitive "no" from
1290+ // the AS, but it surfaced to the sandbox as an opaque "Internal tool error
1291+ // [hex]" — the caller had no way to know the connection needed re-auth.
1292+ it.effect("returns AS-rejected token refreshes (non-invalid_grant) as ToolResult.fail", () =>
1293+ Effect.scoped(
1294+ Effect.gen(function* () {
1295+ const server = yield* serveOAuthTestServer({
1296+ scopes: ["read"],
1297+ supportRefresh: false,
1298+ invalidRefreshTokenErrorCode: "invalid_request",
1299+ invalidRefreshTokenDescription: "Refresh token expired",
1300+ });
1301+ const config = makeTestConfig({
1302+ plugins: [memoryCredentialsPlugin(), oauthErrorPlugin] as const,
1303+ });
1304+ const executor = yield* createExecutor(config);
1305+ yield* executor["oauth-error-test"].seed();
1306+
1307+ yield* executor.oauth.createClient({
1308+ owner: "org",
1309+ slug: OAUTH_CLIENT,
1310+ authorizationUrl: server.authorizationEndpoint,
1311+ tokenUrl: server.tokenEndpoint,
1312+ grant: "authorization_code",
1313+ clientId: "test-client",
1314+ clientSecret: "test-secret",
1315+ });
1316+
1317+ const started = yield* executor.oauth.start({
1318+ owner: "org",
1319+ client: OAUTH_CLIENT,
1320+ clientOwner: "org",
1321+ name: CONN,
1322+ integration: IntegrationSlug.make("oauth_records"),
1323+ template: OAUTH_TEMPLATE,
1324+ });
1325+ expect(started.status).toBe("redirect");
1326+ if (started.status !== "redirect") return;
1327+ const callback = yield* server.completeAuthorizationCodeFlow({
1328+ authorizationUrl: started.authorizationUrl,
1329+ });
1330+ yield* executor.oauth.complete({ state: started.state, code: callback.code });
1331+
1332+ // Expire the access token so the next resolve fires a real
1333+ // refresh-token grant against the live test AS, which rejects it.
1334+ yield* Effect.promise(() =>
1335+ config.db.updateMany("connection", {
1336+ where: (b) =>
1337+ b.and(b("integration", "=", "oauth_records"), b("name", "=", String(CONN))),
1338+ set: { expires_at: Date.now() - 60_000 },
1339+ }),
1340+ );
1341+
1342+ const invoker = makeExecutorToolInvoker(executor, {
1343+ invokeOptions: { onElicitation: acceptAll },
1344+ });
1345+ const result = yield* invoker.invoke({
1346+ path: "oauth_records.org.main.queryRows",
1347+ args: {},
1348+ });
1349+
1350+ expect(result).toMatchObject({
1351+ ok: false,
1352+ error: {
1353+ code: "oauth_refresh_failed",
1354+ details: {
1355+ category: "authentication",
1356+ credential: {
1357+ kind: "oauth",
1358+ label: "oauth_records.org.main",
1359+ },
1360+ },
1361+ retryable: false,
1362+ },
1363+ });
1364+ const failure = result as {
1365+ readonly ok: false;
1366+ readonly error: { readonly message: string };
1367+ };
1368+ // The AS's verdict (code + description) reaches the caller verbatim —
1369+ // never the scrubbed "Internal tool error [hex]".
1370+ expect(failure.error.message).toContain("invalid_request");
1371+ expect(failure.error.message).toContain("Refresh token expired");
1372+ expect(failure.error.message).not.toContain("Internal tool error");
1373+ }),
1374+ ),
1375+ );
1376+
12881377 it.effect("returns missing tool dispatches as ToolResult.fail", () =>
12891378 Effect.gen(function* () {
12901379 const executor = yield* makeExecutorWith([] as const);
packages/core/sdk/src/executor.ts+28−12
@@ -1592,18 +1592,34 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 1591 unmodified lines
15921592 endpointUrlPolicy: config.oauthEndpointUrlPolicy,
15931593 fetch: config.fetch,
15941594 }).pipe(
1595− Effect.mapError((cause) =>
1596− cause.error === "invalid_grant"
1597− ? reauth(
1598− // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message`
1599− `OAuth token refresh was rejected (invalid_grant): ${cause.message}`,
1600− )
1601− : new StorageError({
1602− // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message`
1603− message: `OAuth token refresh failed: ${cause.message}`,
1604− cause,
1605− }),
1606− ),
1595+ Effect.mapError((cause) => {
1596+ // An RFC 6749 §5.2 error code is the AS's definitive
1597+ // verdict on this grant — retrying cannot change it.
1598+ // invalid_grant means the refresh token itself is dead
1599+ // (re-auth required); every other code must still reach
1600+ // the caller as an auth failure, because a StorageError
1601+ // is scrubbed to "Internal tool error [id]" at the
1602+ // sandbox boundary (the Pylon prod regression: the AS
1603+ // rejected refreshes with a non-invalid_grant 400 and
1604+ // callers saw only the opaque defect). Code-less
1605+ // failures (transport blips, non-OAuth-shaped responses)
1606+ // stay StorageError so the next invoke retries.
1607+ if (cause.error !== undefined) {
1608+ return new CredentialResolutionError({
1609+ owner,
1610+ integration: IntegrationSlug.make(row.integration),
1611+ name: ConnectionName.make(row.name),
1612+ // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message`
1613+ message: `OAuth token refresh was rejected (${cause.error}): ${cause.message}`,
1614+ reauthRequired: cause.error === "invalid_grant",
1615+ });
1616+ }
1617+ return new StorageError({
1618+ // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message`
1619+ message: `OAuth token refresh failed: ${cause.message}`,
1620+ cause,
1621+ });
1622+ }),
16071623 );
16081624 });
16091625
packages/core/sdk/src/testing/oauth-test-server.ts+6−1
@@ -55,6 +55,10 @@ export interface OAuthTestServerOptions { · 54 unmodified lines
5555 readonly supportRefresh?: boolean;
5656 readonly tokenExpiresInSeconds?: number;
5757 readonly invalidRefreshTokenDescription?: string;
58+ /** RFC 6749 error code returned when a refresh-token grant is rejected.
59+ * Defaults to `invalid_grant`; set to e.g. `invalid_request` to mirror
60+ * authorization servers that reject dead refresh tokens with other codes. */
61+ readonly invalidRefreshTokenErrorCode?: string;
5862 readonly idTokenClaims?: Readonly<Record<string, unknown>>;
5963 readonly refreshIdTokenClaims?: Readonly<Record<string, unknown>>;
6064 /** Gate Dynamic Client Registration on the requested redirect URIs. When set,
@@ -426,6 +430,7 @@ export const serveOAuthTestServer = ( · 365 unmodified lines
426430 const tokenExpiresInSeconds = options.tokenExpiresInSeconds ?? 3600;
427431 const invalidRefreshTokenDescription =
428432 options.invalidRefreshTokenDescription ?? "Unknown refresh token";
433+ const invalidRefreshTokenErrorCode = options.invalidRefreshTokenErrorCode ?? "invalid_grant";
429434 const scopes = options.scopes ?? defaultScopes;
430435 const omittedTokenResponseScopes = new Set(options.omitTokenResponseScopes ?? []);
431436 const tokenResponseScope = (scope: string | null): string | undefined => {
@@ -671,7 +676,7 @@ export const serveOAuthTestServer = ( · 239 unmodified lines
671676 const refreshToken = params.get("refresh_token");
672677 const record = refreshToken ? refreshTokens.get(refreshToken) : undefined;
673678 if (!supportRefresh || !refreshToken || !record || record.clientId !== clientId) {
674− return oauthError(400, "invalid_grant", invalidRefreshTokenDescription);
679+ return oauthError(400, invalidRefreshTokenErrorCode, invalidRefreshTokenDescription);
675680 }
676681 const nextAccessToken = `at_${randomUUID()}`;
677682 const nextRefreshToken = `rt_${randomUUID()}`;