diff --git a/.changeset/first-party-oauth-integration-policy.md b/.changeset/first-party-oauth-integration-policy.md new file mode 100644 index 0000000000..fbc8992d46 --- /dev/null +++ b/.changeset/first-party-oauth-integration-policy.md @@ -0,0 +1,6 @@ +--- +"@executor-js/sdk": patch +"@executor-js/react": patch +--- + +Enforce optional first-party OAuth integration allow-lists in client selection, authorization start, and callback completion. Restrict the cloud GitHub App to its configured GitHub REST integration so shared OAuth endpoints cannot silently select it for GitHub MCP. Existing credentials continue to refresh; other integrations can use their own OAuth app. diff --git a/apps/cloud/src/engine/first-party-oauth-clients.test.ts b/apps/cloud/src/engine/first-party-oauth-clients.test.ts index 1ca3962d6b..eeee58faa9 100644 --- a/apps/cloud/src/engine/first-party-oauth-clients.test.ts +++ b/apps/cloud/src/engine/first-party-oauth-clients.test.ts @@ -77,6 +77,10 @@ describe("cloud first-party OAuth clients", () => { tokenEndpointAuthMethod: "basic", allowedScopes: expect.arrayContaining(["folder_metadata:read", "folders:read"]), }); + expect(byName.get("github")).toMatchObject({ + allowedIntegrations: ["github_rest"], + authorizationScopes: [], + }); expect(byName.get("hubspot")).toMatchObject({ tokenUrl: "https://api.hubapi.com/oauth/v3/token", authorizationExtraParams: { diff --git a/apps/cloud/src/engine/first-party-oauth-clients.ts b/apps/cloud/src/engine/first-party-oauth-clients.ts index 12b0748627..b96ef5ff70 100644 --- a/apps/cloud/src/engine/first-party-oauth-clients.ts +++ b/apps/cloud/src/engine/first-party-oauth-clients.ts @@ -270,6 +270,9 @@ export const firstPartyOAuthClientsFor = ( env.FIRST_PARTY_GITHUB_AUTHORIZE_URL ?? "https://github.com/login/oauth/authorize", tokenUrl: env.FIRST_PARTY_GITHUB_TOKEN_URL ?? "https://github.com/login/oauth/access_token", integrations: [IntegrationSlug.make("github_rest")], + // Sharing github.com OAuth endpoints must not offer this app to GitHub + // MCP or custom integrations whose capabilities have not been configured. + allowedIntegrations: [IntegrationSlug.make("github_rest")], // GitHub App user access tokens do not use classic OAuth scopes; their // capabilities come from the app's registered permissions. authorizationScopes: [], diff --git a/e2e/scenarios/first-party-oauth.test.ts b/e2e/scenarios/first-party-oauth.test.ts index bee0c78daa..ba2c27957d 100644 --- a/e2e/scenarios/first-party-oauth.test.ts +++ b/e2e/scenarios/first-party-oauth.test.ts @@ -26,7 +26,7 @@ import { import { scenario } from "../src/scenario"; import { Api, Browser, Target } from "../src/services"; -import { visit } from "../src/surfaces/browser"; +import { hydrated, visit } from "../src/surfaces/browser"; const api = composePluginApi([openApiHttpPlugin()] as const); @@ -115,7 +115,10 @@ scenario( // 2. A start through the first-party slug builds GitHub's authorize URL // from the config identity and this platform's served callback. - const integration = IntegrationSlug.make(unique("fpgh")); + const integration = IntegrationSlug.make("github_rest"); + yield* Effect.addFinalizer(() => + client.openapi.removeSpec({ params: { slug: integration } }).pipe(Effect.ignore), + ); yield* client.openapi.addSpec({ payload: { ...githubShapedIntegrationSpec, slug: integration }, }); @@ -168,6 +171,64 @@ scenario( ), ); +scenario( + "First-party OAuth · a shared GitHub endpoint cannot bypass the integration policy", + {}, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + if (target.name !== "cloud") return; + const browser = yield* Browser; + const { client: makeApiClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeApiClient(api, identity); + const integration = IntegrationSlug.make(unique("github_com")); + yield* Effect.addFinalizer(() => + client.openapi.removeSpec({ params: { slug: integration } }).pipe(Effect.ignore), + ); + yield* client.openapi.addSpec({ + payload: { ...githubShapedIntegrationSpec, slug: integration }, + }); + + const clients = yield* client.oauth.listClients(); + const firstParty = clients.find( + (candidate) => String(candidate.slug) === "first-party:github", + ); + expect(firstParty?.origin).toMatchObject({ + kind: "first_party", + allowedIntegrations: ["github_rest"], + }); + const blocked = yield* client.oauth + .start({ + payload: { + client: OAuthClientSlug.make("first-party:github"), + clientOwner: "org", + owner: "org", + name: ConnectionName.make("blocked"), + integration, + template: AuthTemplateSlug.make("oauth"), + }, + }) + .pipe(Effect.flip); + expect(blocked).toMatchObject({ + message: `The built-in OAuth app is not enabled for integration ${integration}. Choose another OAuth app.`, + }); + expect(yield* client.connections.list({ query: { integration } })).toEqual([]); + + yield* browser.session(identity, async ({ page, step }) => { + await step("Open another integration sharing GitHub's OAuth endpoints", async () => { + await visit(page, `/integrations/${integration}?addAccount=1`); + await hydrated(page); + await page.getByRole("button", { name: "Register app", exact: true }).waitFor(); + expect( + await page.getByRole("button", { name: "Connect with OAuth", exact: true }).isEnabled(), + ).toBe(false); + }); + }); + }), + ), +); + scenario( "First-party OAuth · unlisted Google still authorizes its bundle and refuses admin scopes", {}, diff --git a/packages/core/api/src/oauth/api.ts b/packages/core/api/src/oauth/api.ts index 5d0eee3d41..39c9419b54 100644 --- a/packages/core/api/src/oauth/api.ts +++ b/packages/core/api/src/oauth/api.ts @@ -128,11 +128,12 @@ const OAuthClientSummaryResponse = Schema.Struct({ }), /** Host-operated app declared in executor config — every org connects * through it; nothing to paste. `integrations` ranks it as the default - * for those integrations; `allowedScopes` is the host-enforced scope - * boundary the picker mirrors before offering it. */ + * for those integrations; `allowedIntegrations` and `allowedScopes` are + * host-enforced boundaries the picker mirrors before offering it. */ Schema.Struct({ kind: Schema.Literal("first_party"), integrations: Schema.optional(Schema.Array(IntegrationSlug)), + allowedIntegrations: Schema.optional(Schema.Array(IntegrationSlug)), allowedScopes: Schema.optional(Schema.Array(Schema.String)), }), ]), diff --git a/packages/core/sdk/src/oauth-client.ts b/packages/core/sdk/src/oauth-client.ts index 8778205014..61180b5a83 100644 --- a/packages/core/sdk/src/oauth-client.ts +++ b/packages/core/sdk/src/oauth-client.ts @@ -165,6 +165,9 @@ export type OAuthClientOrigin = * one Google app deliberately backs gmail, calendar, drive, …. */ readonly kind: "first_party"; readonly integrations?: readonly IntegrationSlug[]; + /** Host-enforced integration allow-list, independent of picker ranking. + * Omitted permits any integration; an empty list permits none. */ + readonly allowedIntegrations?: readonly IntegrationSlug[]; /** OAuth scopes this deployment permits the app to request. Omitted means * the provider app is unrestricted; present means every requested scope * must be in this set. This is public policy metadata, not a secret. */ @@ -206,6 +209,12 @@ export interface FirstPartyOAuthClientConfig { * exact-match default for those integrations. Endpoint-host matching still * applies when omitted. */ readonly integrations?: readonly IntegrationSlug[]; + /** Integrations permitted to start or complete authorization through this + * app. Unlike `integrations`, this is an authorization boundary, not a + * ranking hint. Omit for provider-wide clients; an empty list denies all + * new authorizations. Existing credentials remain usable and refreshable. + * Slugs are exact: custom or renamed integrations must be listed too. */ + readonly allowedIntegrations?: readonly IntegrationSlug[]; /** Scopes sent on the provider authorization request instead of the * integration-declared set. Use an empty array for providers such as * GitHub Apps, whose capabilities are configured on the app and whose OAuth @@ -272,6 +281,15 @@ export const firstPartyOAuthClientAllowsScopes = ( return requestedScopes.every((scope) => allowed.has(scope)); }; +/** An explicit integration policy fails closed when the picker has not yet + * resolved the integration. Omitted policies retain provider-wide matching. */ +export const firstPartyOAuthClientAllowsIntegration = ( + config: Pick, + integration: IntegrationSlug | undefined, +): boolean => + config.allowedIntegrations === undefined || + (integration !== undefined && config.allowedIntegrations.includes(integration)); + export type CreateOAuthClientInput = OAuthClient & { /** Stored-row origins only — `first_party` is config-declared, never created * through this surface (the service also rejects the slug namespace). */ diff --git a/packages/core/sdk/src/oauth-first-party.test.ts b/packages/core/sdk/src/oauth-first-party.test.ts index 8a2be1cc3d..cc2b2d236e 100644 --- a/packages/core/sdk/src/oauth-first-party.test.ts +++ b/packages/core/sdk/src/oauth-first-party.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "@effect/vitest"; import { Effect, Predicate } from "effect"; import type * as Tracer from "effect/Tracer"; +import { createExecutor } from "./executor"; import { AuthTemplateSlug, @@ -14,6 +15,7 @@ import { firstPartyOAuthClientSlug, type FirstPartyOAuthClientConfig, type OAuthStartError, + type OAuthCompleteError, } from "./oauth-client"; import { definePlugin } from "./plugin"; import { makeTestWorkspaceHarness, memoryCredentialsPlugin } from "./test-config"; @@ -111,6 +113,113 @@ const firstPartyClientFor = (server: { }); describe("first-party oauth clients", () => { + // These guards run before contacting a provider; no upstream server is needed. + const policyClient = firstPartyClientFor({ + authorizationEndpoint: "https://oauth.example.invalid/authorize", + tokenEndpoint: "https://oauth.example.invalid/token", + }); + + for (const allowedIntegrations of [[], [IntegrationSlug.make("another_api")]]) { + it.effect(`rejects an integration outside policy ${JSON.stringify(allowedIntegrations)}`, () => + Effect.scoped( + Effect.gen(function* () { + const { executor, config } = yield* makeTestWorkspaceHarness({ + plugins, + firstPartyOAuthClients: [{ ...policyClient, allowedIntegrations }], + }); + yield* executor.acme.seed(); + const error = yield* executor.oauth + .start({ + owner: "org", + clientOwner: "org", + client: FIRST_PARTY, + integration: INTEG, + template: TEMPLATE, + name: ConnectionName.make("blocked"), + }) + .pipe(Effect.flip); + expect(Predicate.isTagged("OAuthStartError")(error)).toBe(true); + const startError = error as OAuthStartError; + expect(startError.message).toContain("Choose another OAuth app"); + expect(yield* Effect.promise(() => config.db.findMany("oauth_session", {}))).toEqual([]); + expect(yield* executor.connections.list()).toEqual([]); + }), + ), + ); + } + + for (const allowedIntegrations of [undefined, [INTEG]]) { + it.effect( + `allows authorization with integration policy ${JSON.stringify(allowedIntegrations)}`, + () => + Effect.scoped( + Effect.gen(function* () { + const { executor } = yield* makeTestWorkspaceHarness({ + plugins, + firstPartyOAuthClients: [{ ...policyClient, integrations: [], allowedIntegrations }], + }); + yield* executor.acme.seed(); + const started = yield* executor.oauth.start({ + owner: "org", + clientOwner: "org", + client: FIRST_PARTY, + integration: INTEG, + template: TEMPLATE, + name: ConnectionName.make("allowed"), + }); + expect(started.status).toBe("redirect"); + const listed = yield* executor.oauth.listClients(); + expect(listed[0]?.origin).toEqual({ + kind: "first_party", + integrations: [], + ...(allowedIntegrations === undefined ? {} : { allowedIntegrations }), + }); + }), + ), + ); + } + + it.effect("rejects an in-flight callback after the host restricts the integration", () => + Effect.scoped( + Effect.gen(function* () { + const { executor, config } = yield* makeTestWorkspaceHarness({ + plugins, + firstPartyOAuthClients: [policyClient], + }); + yield* executor.acme.seed(); + const started = yield* executor.oauth.start({ + owner: "org", + clientOwner: "org", + client: FIRST_PARTY, + integration: INTEG, + template: TEMPLATE, + name: ConnectionName.make("in-flight"), + }); + expect(started.status).toBe("redirect"); + if (started.status !== "redirect") return; + + const restricted = yield* Effect.acquireRelease( + createExecutor({ + ...config, + firstPartyOAuthClients: [{ ...policyClient, allowedIntegrations: [] }], + }), + (instance) => instance.close().pipe(Effect.ignore), + ); + const error = yield* restricted.oauth + .complete({ + state: started.state, + code: "must-not-be-redeemed", + }) + .pipe(Effect.flip); + expect(Predicate.isTagged("OAuthCompleteError")(error)).toBe(true); + const completeError = error as OAuthCompleteError; + expect(completeError.restartRequired).toBe(true); + expect(completeError.message).toContain("no longer enabled for integration acme"); + expect(yield* restricted.connections.list()).toEqual([]); + }), + ), + ); + it.effect( "start → complete through a config-declared client mints an executable connection", () => { diff --git a/packages/core/sdk/src/oauth-service.ts b/packages/core/sdk/src/oauth-service.ts index 7d8d7d46ea..3dd6a87dcf 100644 --- a/packages/core/sdk/src/oauth-service.ts +++ b/packages/core/sdk/src/oauth-service.ts @@ -50,6 +50,7 @@ import { OAuthSessionNotFoundError, OAuthStartError, firstPartyOAuthClientAllowsScopes, + firstPartyOAuthClientAllowsIntegration, firstPartyOAuthClientSlug, isFirstPartyOAuthClientSlug, parseStoredTokenEndpointAuthMethod, @@ -1559,6 +1560,9 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { origin: { kind: "first_party", ...(config.integrations !== undefined ? { integrations: config.integrations } : {}), + ...(config.allowedIntegrations !== undefined + ? { allowedIntegrations: config.allowedIntegrations } + : {}), ...(config.allowedScopes !== undefined ? { allowedScopes: config.allowedScopes } : {}), }, })); @@ -1742,6 +1746,17 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { message: `OAuth client not found: ${input.client}`, }); } + const firstParty = firstPartyFlow ? firstPartyBySlug.get(String(input.client)) : undefined; + // Check before scope discovery, any provider request, or session creation. + // A shared endpoint does not imply the host app supports this integration. + if ( + firstParty !== undefined && + !firstPartyOAuthClientAllowsIntegration(firstParty, input.integration) + ) { + return yield* new OAuthStartError({ + message: `The built-in OAuth app is not enabled for integration ${input.integration}. Choose another OAuth app.`, + }); + } // Normalize the name the same way the mint stores it, so the free-name // guard below compares against the exact stored form. @@ -1786,7 +1801,6 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { }), ), ); - const firstParty = firstPartyFlow ? firstPartyBySlug.get(String(input.client)) : undefined; const requestedScopes = scopePolicy.kind === "discover" ? yield* (() => { @@ -2219,9 +2233,10 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { const firstParty = firstPartyBySlug.get(String(session.clientSlug)); if ( firstParty !== undefined && - firstParty.allowedScopes !== undefined && - (session.requestedScopes === null || - !firstPartyOAuthClientAllowsScopes(firstParty, session.requestedScopes)) + (!firstPartyOAuthClientAllowsIntegration(firstParty, session.integration) || + (firstParty.allowedScopes !== undefined && + (session.requestedScopes === null || + !firstPartyOAuthClientAllowsScopes(firstParty, session.requestedScopes)))) ) { return yield* new OAuthCompleteError({ message: `The built-in OAuth app is no longer enabled for integration ${session.integration}; restart the flow.`, diff --git a/packages/core/sdk/src/shared.ts b/packages/core/sdk/src/shared.ts index 391c12e3fa..6cbbd088b8 100644 --- a/packages/core/sdk/src/shared.ts +++ b/packages/core/sdk/src/shared.ts @@ -151,6 +151,7 @@ export { export { FIRST_PARTY_OAUTH_CLIENT_PREFIX, firstPartyOAuthClientSlug, + firstPartyOAuthClientAllowsIntegration, isFirstPartyOAuthClientSlug, SubjectTokenTypeSchema, DEFAULT_SUBJECT_TOKEN_TYPE, diff --git a/packages/react/src/plugins/use-effective-oauth-client.test.ts b/packages/react/src/plugins/use-effective-oauth-client.test.ts index a204fb2ae3..0d5a1627de 100644 --- a/packages/react/src/plugins/use-effective-oauth-client.test.ts +++ b/packages/react/src/plugins/use-effective-oauth-client.test.ts @@ -38,6 +38,81 @@ const spotify = app("spotify-app", { }); describe("selectClientsForEndpoints", () => { + const restrictedGitHub = app("first-party:github", { + owner: "org", + authorizationUrl: "https://github.com/login/oauth/authorize", + tokenUrl: "https://github.com/login/oauth/access_token", + origin: { + kind: "first_party", + integrations: [IntegrationSlug.make("github_rest")], + allowedIntegrations: [IntegrationSlug.make("github_rest")], + }, + }); + + for (const endpoints of [ + { tokenUrl: restrictedGitHub.tokenUrl, integration: IntegrationSlug.make("github_com") }, + { tokenUrl: "https://api.github.com/token", integration: IntegrationSlug.make("github_com") }, + { integration: IntegrationSlug.make("github_com") }, + { tokenUrl: restrictedGitHub.tokenUrl }, + ]) { + it(`hides a restricted app from every picker tier for ${JSON.stringify(endpoints)}`, () => { + const result = selectClientsForEndpoints([restrictedGitHub], endpoints); + expect(result.matched).toEqual([]); + expect(result.nearMatches).toEqual([]); + expect(result.unmatched).toEqual([]); + }); + } + + it("keeps supported first-party and manual apps available", () => { + const byo = app("byo-github", { + authorizationUrl: restrictedGitHub.authorizationUrl, + tokenUrl: restrictedGitHub.tokenUrl, + }); + const endpoints = { tokenUrl: restrictedGitHub.tokenUrl }; + expect( + selectClientsForEndpoints([restrictedGitHub, byo], { + ...endpoints, + integration: IntegrationSlug.make("github_rest"), + }).matched, + ).toEqual([restrictedGitHub, byo]); + expect( + selectClientsForEndpoints([restrictedGitHub, byo], { + ...endpoints, + integration: IntegrationSlug.make("github_com"), + }).matched, + ).toEqual([byo]); + }); + + it("preserves host matching when only a ranking hint is configured", () => { + const unrestricted = { + ...restrictedGitHub, + origin: { kind: "first_party" as const, integrations: [IntegrationSlug.make("github_rest")] }, + }; + expect( + selectClientsForEndpoints([unrestricted], { + tokenUrl: unrestricted.tokenUrl, + integration: IntegrationSlug.make("custom_github"), + }).matched, + ).toEqual([unrestricted]); + }); + + it("an empty integration policy overrides recorded intent", () => { + expect( + selectClientsForEndpoints( + [ + { + ...restrictedGitHub, + origin: { ...restrictedGitHub.origin, kind: "first_party", allowedIntegrations: [] }, + }, + ], + { + tokenUrl: restrictedGitHub.tokenUrl, + integration: IntegrationSlug.make("github_rest"), + }, + ).matched, + ).toEqual([]); + }); + it("excludes unrelated providers and reports no match (drives the register CTA)", () => { // Integration declares Google's split authorize/token roots; only the // Spotify app is registered → nothing matches. diff --git a/packages/react/src/plugins/use-effective-oauth-client.tsx b/packages/react/src/plugins/use-effective-oauth-client.tsx index 9f898e9c61..e27a27f682 100644 --- a/packages/react/src/plugins/use-effective-oauth-client.tsx +++ b/packages/react/src/plugins/use-effective-oauth-client.tsx @@ -3,6 +3,7 @@ import { useAtomValue } from "@effect/atom-react"; import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; import { OAuthClientSlug, + firstPartyOAuthClientAllowsIntegration, type IntegrationSlug, type OAuthClientOrigin, type Owner, @@ -201,6 +202,8 @@ export function selectClientsForEndpoints( const manual = all.filter( (app) => !isDcrClient(app) && + (app.origin.kind !== "first_party" || + firstPartyOAuthClientAllowsIntegration(app.origin, endpoints.integration)) && firstPartyClientAllowsScopes(app, endpoints.scopes, endpoints.discoversScopes === true), );