diff --git a/apps/cloud/src/engine/execution-stack.ts b/apps/cloud/src/engine/execution-stack.ts index cc9afb38d..a7ff4de70 100644 --- a/apps/cloud/src/engine/execution-stack.ts +++ b/apps/cloud/src/engine/execution-stack.ts @@ -135,7 +135,7 @@ const GOOGLE_FIRST_PARTY_ALLOWED_SCOPES: readonly string[] = [ // `_TOKEN_URL` overrides exist so tests and dev instances can point the app at // an emulated provider (`@executor-js/emulate`) and run the complete flow. // Production leaves them unset. -const cloudFirstPartyOAuthClients = (): readonly FirstPartyOAuthClientConfig[] => [ +export const cloudFirstPartyOAuthClients = (): readonly FirstPartyOAuthClientConfig[] => [ ...(env.FIRST_PARTY_GITHUB_CLIENT_ID && env.FIRST_PARTY_GITHUB_CLIENT_SECRET ? [ { @@ -162,6 +162,12 @@ const cloudFirstPartyOAuthClients = (): readonly FirstPartyOAuthClientConfig[] = clientId: env.FIRST_PARTY_GOOGLE_CLIENT_ID, clientSecret: env.FIRST_PARTY_GOOGLE_CLIENT_SECRET, allowedScopes: GOOGLE_FIRST_PARTY_ALLOWED_SCOPES, + // Withdrawn from the connect picker: no new connection is offered the + // Executor-owned Google app. The entry stays declared on purpose — + // every connection already minted against it keeps refreshing and + // reconnecting through it. Deleting this block, or unsetting the env + // vars, would strand those connections instead. + unlisted: true, }, ] : []), diff --git a/apps/cloud/src/engine/first-party-oauth-clients.test.ts b/apps/cloud/src/engine/first-party-oauth-clients.test.ts new file mode 100644 index 000000000..9780a1c62 --- /dev/null +++ b/apps/cloud/src/engine/first-party-oauth-clients.test.ts @@ -0,0 +1,87 @@ +import { env } from "cloudflare:workers"; +import { beforeAll, describe, expect, it } from "@effect/vitest"; + +import { cloudFirstPartyOAuthClients } from "./execution-stack"; + +// The reviewed consumer scope boundary of the Executor-owned Google app. +// +// These assertions used to live in `e2e/scenarios/first-party-oauth.test.ts`, +// read off `listClients`. The app is now `unlisted`, so it has no read surface +// to introspect — the bundle is only observable on the config it is built from, +// which is here. The e2e still owns the BEHAVIOUR the boundary produces (which +// scopes an `oauth.start` requests, and that admin scopes are refused). +const GOOGLE_SCOPE = (suffix: string) => `https://www.googleapis.com/auth/${suffix}`; + +describe("cloud first-party oauth clients", () => { + beforeAll(() => { + env.FIRST_PARTY_GOOGLE_CLIENT_ID = "test-google-client"; + env.FIRST_PARTY_GOOGLE_CLIENT_SECRET = "test-google-secret"; + }); + + const google = () => cloudFirstPartyOAuthClients().find((client) => client.name === "google"); + + it("declares the Google app but withholds it from every listing", () => { + const client = google(); + expect(client, "the env-declared first-party Google app is configured").toBeDefined(); + // The entry MUST stay declared: `loadClient` resolves it by slug for every + // existing connection's refresh and reconnect. `unlisted` is what stops it + // being offered for new connections. + expect(client?.unlisted).toBe(true); + }); + + it("covers the reviewed consumer bundle", () => { + const allowed = google()?.allowedScopes; + expect(allowed).toBeDefined(); + for (const scope of [ + "calendar", + "meetings.space.readonly", + "spreadsheets", + "drive.file", + "drive", + "documents", + "presentations", + "forms.body", + "forms.responses.readonly", + "tasks", + "contacts", + "contacts.other.readonly", + "directory.readonly", + "user.addresses.read", + "user.birthday.read", + "user.emails.read", + "user.gender.read", + "user.organization.read", + "user.phonenumbers.read", + "photoslibrary.appendonly", + "photoslibrary.edit.appcreateddata", + "photospicker.mediaitems.readonly", + "webmasters", + "gmail.settings.basic", + ]) { + expect(allowed).toContain(GOOGLE_SCOPE(scope)); + } + // `gmail.modify` stays in the host-enforced allowlist on purpose: a + // connection created before the full-Gmail review still declares it, and + // `resolveFirstPartyScopes` filters discovered scopes through this list, so + // dropping it would break those reconnects — as the legacy-spec case in the + // e2e asserts. The invariant that new Gmail presets request + // `mail.google.com` instead lives in the preset unit tests + // (packages/plugins/openapi/.../presets.test.ts), which is where the + // request-side scope choice is actually decided. + expect(allowed).toContain("https://mail.google.com/"); + expect(allowed).toContain(GOOGLE_SCOPE("gmail.modify")); + }); + + it("excludes the scopes held back from consumer review", () => { + const allowed = google()?.allowedScopes; + expect(allowed).toBeDefined(); + for (const scope of [ + "gmail.settings.sharing", + "admin.directory.user", + "youtube", + "cloud-platform", + ]) { + expect(allowed).not.toContain(GOOGLE_SCOPE(scope)); + } + }); +}); diff --git a/e2e/scenarios/first-party-oauth.test.ts b/e2e/scenarios/first-party-oauth.test.ts index f162163a1..e11984aa3 100644 --- a/e2e/scenarios/first-party-oauth.test.ts +++ b/e2e/scenarios/first-party-oauth.test.ts @@ -169,7 +169,7 @@ scenario( ); scenario( - "First-party OAuth · Google offers the reviewed consumer bundle and refuses admin scopes", + "First-party OAuth · unlisted Google still authorizes its bundle and refuses admin scopes", {}, Effect.scoped( Effect.gen(function* () { @@ -212,76 +212,18 @@ scenario( } }); + // The Executor-owned Google app is withheld from every listing: it is no + // longer offered for new connections, so connecting Google means bringing + // your own OAuth app. It stays fully resolvable by slug, which the + // `oauth.start` cases below exercise — that is the guarantee for everyone + // already connected through it. Its reviewed consumer scope bundle, no + // longer introspectable from here, is asserted on the config it is built + // from, in apps/cloud/src/engine/first-party-oauth-clients.test.ts. const clients = yield* client.oauth.listClients(); - const google = clients.find((candidate) => String(candidate.slug) === "first-party:google"); - expect(google, "the env-declared first-party Google app is listed").toBeDefined(); - expect(google?.origin.kind).toBe("first_party"); - if (google?.origin.kind !== "first_party") return; - expect(google.origin.allowedScopes).toContain("https://www.googleapis.com/auth/calendar"); - expect(google.origin.allowedScopes).toContain( - "https://www.googleapis.com/auth/meetings.space.readonly", - ); - // `gmail.modify` stays in the host-enforced allowlist on purpose: a - // connection created before the full-Gmail review still declares it, and - // `resolveFirstPartyScopes` filters discovered scopes through this list, - // so dropping it would break those reconnects — as the legacy-spec case - // further down this file asserts. The invariant that new Gmail presets - // request `mail.google.com` instead lives in the preset unit tests - // (packages/plugins/openapi/.../presets.test.ts), which is where the - // request-side scope choice is actually decided. - expect(google.origin.allowedScopes).toContain("https://mail.google.com/"); - expect(google.origin.allowedScopes).toContain( - "https://www.googleapis.com/auth/gmail.settings.basic", - ); - expect(google.origin.allowedScopes).not.toContain( - "https://www.googleapis.com/auth/gmail.settings.sharing", - ); - expect(google.origin.allowedScopes).toContain("https://www.googleapis.com/auth/spreadsheets"); - expect(google.origin.allowedScopes).toContain("https://www.googleapis.com/auth/drive.file"); - expect(google.origin.allowedScopes).toContain("https://www.googleapis.com/auth/drive"); - expect(google.origin.allowedScopes).toContain("https://www.googleapis.com/auth/documents"); - expect(google.origin.allowedScopes).toContain( - "https://www.googleapis.com/auth/presentations", - ); - expect(google.origin.allowedScopes).toContain("https://www.googleapis.com/auth/forms.body"); - expect(google.origin.allowedScopes).toContain( - "https://www.googleapis.com/auth/forms.responses.readonly", - ); - expect(google.origin.allowedScopes).toContain("https://www.googleapis.com/auth/tasks"); - expect(google.origin.allowedScopes).toContain("https://www.googleapis.com/auth/contacts"); - expect(google.origin.allowedScopes).toContain( - "https://www.googleapis.com/auth/contacts.other.readonly", - ); - expect(google.origin.allowedScopes).toContain( - "https://www.googleapis.com/auth/directory.readonly", - ); - for (const scope of [ - "user.addresses.read", - "user.birthday.read", - "user.emails.read", - "user.gender.read", - "user.organization.read", - "user.phonenumbers.read", - ]) { - expect(google.origin.allowedScopes).toContain(`https://www.googleapis.com/auth/${scope}`); - } - expect(google.origin.allowedScopes).toContain( - "https://www.googleapis.com/auth/photoslibrary.appendonly", - ); - expect(google.origin.allowedScopes).toContain( - "https://www.googleapis.com/auth/photoslibrary.edit.appcreateddata", - ); - expect(google.origin.allowedScopes).toContain( - "https://www.googleapis.com/auth/photospicker.mediaitems.readonly", - ); - expect(google.origin.allowedScopes).toContain("https://www.googleapis.com/auth/webmasters"); - expect(google.origin.allowedScopes).not.toContain( - "https://www.googleapis.com/auth/admin.directory.user", - ); - expect(google.origin.allowedScopes).not.toContain("https://www.googleapis.com/auth/youtube"); - expect(google.origin.allowedScopes).not.toContain( - "https://www.googleapis.com/auth/cloud-platform", - ); + expect( + clients.find((candidate) => String(candidate.slug) === "first-party:google"), + "the first-party Google app is not offered in listings", + ).toBeUndefined(); const calendar = IntegrationSlug.make(unique("google_calendar")); yield* client.openapi.addSpec({ diff --git a/packages/core/sdk/src/oauth-client.ts b/packages/core/sdk/src/oauth-client.ts index 2cbbbd911..843f80da3 100644 --- a/packages/core/sdk/src/oauth-client.ts +++ b/packages/core/sdk/src/oauth-client.ts @@ -187,6 +187,17 @@ export interface FirstPartyOAuthClientConfig { * GitHub Apps, whose capabilities are configured on the app and whose OAuth * user-token flow does not use scopes. Omit for normal OAuth clients. */ readonly authorizationScopes?: readonly string[]; + /** Withdraw the app from every listing surface without retiring it. It stops + * appearing in `listClients` — so connect pickers and the agent-facing + * client list never offer it — while remaining fully resolvable by slug. + * Load, start, completion, and refresh all go through `loadClient`, which + * reads config directly, so connections already minted against the app keep + * renewing and reconnecting exactly as before. + * + * This is the safe way to stop offering a shared app. Dropping its env vars + * instead removes the config entry itself, which strands every existing + * connection on a client the host can no longer resolve. */ + readonly unlisted?: boolean; /** OAuth scopes this deployment permits the app to request. Omit to allow * every scope declared by a matching integration. For declared scopes, * start and completion fail unless every requested scope belongs to this diff --git a/packages/core/sdk/src/oauth-first-party.test.ts b/packages/core/sdk/src/oauth-first-party.test.ts index d9070bcea..848c2fd53 100644 --- a/packages/core/sdk/src/oauth-first-party.test.ts +++ b/packages/core/sdk/src/oauth-first-party.test.ts @@ -288,6 +288,71 @@ describe("first-party oauth clients", () => { ), ); + it.effect("an unlisted first-party app is withheld from listings but still refreshes", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const harness = yield* makeTestWorkspaceHarness({ + plugins, + firstPartyOAuthClients: [{ ...firstPartyClientFor(server), unlisted: true }], + }); + const { executor, config } = harness; + yield* executor.acme.seed(); + + yield* executor.oauth.createClient({ + owner: "org", + slug: OAuthClientSlug.make("byo-app"), + authorizationUrl: server.authorizationEndpoint, + tokenUrl: server.tokenEndpoint, + grant: "authorization_code", + clientId: "byo-client", + clientSecret: "byo-secret", + }); + + // Withheld from the surface that OFFERS an app for a new connection. + const clients = yield* executor.oauth.listClients(); + expect(clients.map((c) => String(c.slug))).toEqual(["byo-app"]); + + // …yet the app itself is untouched: a connection resolves, mints, and + // renews through it exactly as a listed one would. + const started = yield* executor.oauth.start({ + owner: "org", + client: FIRST_PARTY, + clientOwner: "org", + name: ConnectionName.make("main"), + integration: INTEG, + template: TEMPLATE, + }); + expect(started.status).toBe("redirect"); + if (started.status !== "redirect") return; + const callback = yield* server.completeAuthorizationCodeFlow({ + authorizationUrl: started.authorizationUrl, + }); + yield* executor.oauth.complete({ state: started.state, code: callback.code }); + + const firstToken = (yield* executor.execute( + ToolAddress.make("tools.acme.org.main.whoami"), + {}, + )) as { token: string }; + + // Force expiry so the next resolve must refresh through the config client. + yield* Effect.promise(() => + config.db.updateMany("connection", { + where: (b) => b("name", "=", "main"), + set: { expires_at: Date.now() - 60_000 }, + }), + ); + + const refreshedToken = (yield* executor.execute( + ToolAddress.make("tools.acme.org.main.whoami"), + {}, + )) as { token: string }; + expect(refreshedToken.token).not.toBe(firstToken.token); + expect(yield* server.acceptsAccessToken(refreshedToken.token)).toBe(true); + }), + ), + ); + it.effect("a scope-limited first-party app rejects an integration outside its policy", () => Effect.scoped( Effect.gen(function* () { diff --git a/packages/core/sdk/src/oauth-service.ts b/packages/core/sdk/src/oauth-service.ts index 751579979..ab964eb5d 100644 --- a/packages/core/sdk/src/oauth-service.ts +++ b/packages/core/sdk/src/oauth-service.ts @@ -1181,8 +1181,14 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { // and projected exactly like stored rows — clientId only, never the secret. // Owner is reported as "org" (the widest visibility the summary shape can // express); the flow itself ignores owner for first-party slugs. - const firstPartySummaries: readonly OAuthClientSummary[] = [...firstPartyBySlug.values()].map( - (config) => ({ + // + // `unlisted` apps are withheld here and ONLY here: listing is what offers an + // app for a NEW connection, so this is the whole of "stop offering it". + // `loadClient` still resolves them, keeping every existing connection's + // refresh and reconnect intact. + const firstPartySummaries: readonly OAuthClientSummary[] = [...firstPartyBySlug.values()] + .filter((config) => config.unlisted !== true) + .map((config) => ({ owner: "org", slug: firstPartyOAuthClientSlug(config.name), grant: "authorization_code", @@ -1195,8 +1201,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { ...(config.integrations !== undefined ? { integrations: config.integrations } : {}), ...(config.allowedScopes !== undefined ? { allowedScopes: config.allowedScopes } : {}), }, - }), - ); + })); return deps.fuma .use("oauth_client.findMany", (db) => looseDb(db).findMany("oauth_client", {})) .pipe(