diff --git a/apps/local/src/server/config-sync.boot.test.ts b/apps/local/src/server/config-sync.boot.test.ts new file mode 100644 index 000000000..8b5b2062a --- /dev/null +++ b/apps/local/src/server/config-sync.boot.test.ts @@ -0,0 +1,225 @@ +// Boot-time replay of MCP auth from `executor.jsonc`. The plugin already +// resolves `secret-public-ref:` strings at connect time via +// `ctx.secrets.get` — the regression class here is auth being silently +// dropped at the config boundary. `mcp.addSource` persists the source +// row even when the remote is unreachable, so unreachable endpoints are +// enough to assert on the stored auth shape without running an MCP +// server. The new credential-binding flow does require referenced +// secrets/connections to exist at the target scope, hence the seeds +// before each test. + +import { afterEach, beforeEach, describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { SECRET_REF_PREFIX, type ExecutorFileConfig } from "@executor-js/config"; +import { + ConnectionId, + CreateConnectionInput, + OAUTH2_PROVIDER_KEY, + ScopeId, + SecretId, + TokenMaterial, + createExecutor, + definePlugin, + makeTestConfig, + type SecretProvider, +} from "@executor-js/sdk"; +import { mcpPlugin } from "@executor-js/plugin-mcp"; + +import { syncFromConfig } from "./config-sync"; +import type { LocalExecutor } from "./executor"; + +const UNREACHABLE = "http://127.0.0.1:1/mcp"; +const TEST_SCOPE = ScopeId.make("test-scope"); + +const makeMemorySecretsPlugin = () => { + const store = new Map(); + const provider: SecretProvider = { + key: "memory", + writable: true, + get: (id, scope) => Effect.sync(() => store.get(`${scope} ${id}`) ?? null), + set: (id, value, scope) => + Effect.sync(() => { + store.set(`${scope} ${id}`, value); + }), + delete: (id, scope) => Effect.sync(() => store.delete(`${scope} ${id}`)), + list: () => + Effect.sync(() => + Array.from(store.keys()).map((k) => { + const id = k.split(" ", 2)[1] ?? k; + return { id, name: id }; + }), + ), + }; + return definePlugin(() => ({ + id: "memory-secrets" as const, + storage: () => ({}), + secretProviders: [provider], + })); +}; + +let workDir: string; + +beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), "exec-config-sync-")); +}); + +afterEach(() => { + rmSync(workDir, { recursive: true, force: true }); +}); + +const writeConfig = (config: ExecutorFileConfig): string => { + const path = join(workDir, "executor.jsonc"); + writeFileSync(path, JSON.stringify(config, null, 2)); + return path; +}; + +const makeExecutor = () => + createExecutor(makeTestConfig({ plugins: [makeMemorySecretsPlugin()(), mcpPlugin()] as const })); + +describe("syncFromConfig — MCP auth replay", () => { + it.effect("strips secret-public-ref prefix from header auth", () => + Effect.gen(function* () { + const configPath = writeConfig({ + sources: [ + { + kind: "mcp", + transport: "remote", + name: "PostHog", + endpoint: UNREACHABLE, + namespace: "posthog", + auth: { + kind: "header", + headerName: "Authorization", + secret: `${SECRET_REF_PREFIX}posthog-api-key`, + prefix: "Bearer ", + }, + }, + ], + }); + const executor = yield* makeExecutor(); + yield* executor.secrets.set({ + id: SecretId.make("posthog-api-key"), + scope: TEST_SCOPE, + name: "PostHog API Key", + value: "phx_test_token", + provider: "memory", + }); + + yield* syncFromConfig({ + // The test executor uses a narrower plugin tuple than LocalExecutor (no + // openapi/graphql), but Match in addSourceFromConfig only dispatches to + // the mcp branch for these fixtures, so the missing methods are never + // touched at runtime. + // oxlint-disable-next-line executor/no-double-cast + executor: executor as unknown as LocalExecutor, + configPath, + targetScope: TEST_SCOPE, + }); + + const stored = yield* executor.mcp.getSource("posthog", TEST_SCOPE); + expect(stored).not.toBeNull(); + expect(stored!.config).toMatchObject({ + transport: "remote", + endpoint: UNREACHABLE, + auth: { + kind: "header", + headerName: "Authorization", + secretSlot: "auth:header", + prefix: "Bearer ", + }, + }); + }), + ); + + it.effect("passes oauth2 auth through unchanged", () => + Effect.gen(function* () { + const configPath = writeConfig({ + sources: [ + { + kind: "mcp", + transport: "remote", + name: "Linear", + endpoint: UNREACHABLE, + namespace: "linear", + auth: { kind: "oauth2", connectionId: "mcp-oauth2-linear" }, + }, + ], + }); + const executor = yield* makeExecutor(); + const connectionId = ConnectionId.make("mcp-oauth2-linear"); + yield* executor.connections.create( + new CreateConnectionInput({ + id: connectionId, + scope: TEST_SCOPE, + provider: OAUTH2_PROVIDER_KEY, + identityLabel: "user@example.com", + accessToken: new TokenMaterial({ + secretId: SecretId.make(`${connectionId}.access_token`), + name: "MCP Access Token", + value: "access-token-value", + }), + refreshToken: null, + expiresAt: null, + oauthScope: null, + providerState: { + endpoint: UNREACHABLE, + tokenType: "Bearer", + clientInformation: { client_id: "fake" }, + authorizationServerUrl: null, + authorizationServerMetadata: null, + resourceMetadataUrl: null, + resourceMetadata: null, + }, + }), + ); + + yield* syncFromConfig({ + // oxlint-disable-next-line executor/no-double-cast + executor: executor as unknown as LocalExecutor, + configPath, + targetScope: TEST_SCOPE, + }); + + const stored = yield* executor.mcp.getSource("linear", TEST_SCOPE); + expect(stored!.config).toMatchObject({ + transport: "remote", + auth: { kind: "oauth2", connectionSlot: "auth:oauth2:connection" }, + }); + }), + ); + + it.effect("preserves kind:none auth on replay", () => + Effect.gen(function* () { + const configPath = writeConfig({ + sources: [ + { + kind: "mcp", + transport: "remote", + name: "DeepWiki", + endpoint: UNREACHABLE, + namespace: "devin", + auth: { kind: "none" }, + }, + ], + }); + const executor = yield* makeExecutor(); + + yield* syncFromConfig({ + // oxlint-disable-next-line executor/no-double-cast + executor: executor as unknown as LocalExecutor, + configPath, + targetScope: TEST_SCOPE, + }); + + const stored = yield* executor.mcp.getSource("devin", TEST_SCOPE); + expect(stored!.config).toMatchObject({ + transport: "remote", + auth: { kind: "none" }, + }); + }), + ); +}); diff --git a/apps/local/src/server/config-sync.ts b/apps/local/src/server/config-sync.ts index d68c495ea..b3ac30b79 100644 --- a/apps/local/src/server/config-sync.ts +++ b/apps/local/src/server/config-sync.ts @@ -118,6 +118,7 @@ const addSourceFromConfig = ( baseUrl: s.baseUrl, namespace: s.namespace, headers: translateHeaders(s.headers), + credentialTargetScope: targetScope, }) .pipe(Effect.asVoid), ), @@ -128,6 +129,7 @@ const addSourceFromConfig = ( scope: targetScope, namespace: s.namespace, headers: translateHeaders(s.headers) as Record | undefined, + credentialTargetScope: targetScope, }) .pipe(Effect.asVoid), ), @@ -157,6 +159,7 @@ const addSourceFromConfig = ( headers: s.headers, namespace: s.namespace, auth: translateMcpAuth(s.auth), + credentialTargetScope: targetScope, }) .pipe(Effect.asVoid); }), diff --git a/packages/plugins/mcp/src/sdk/plugin.test.ts b/packages/plugins/mcp/src/sdk/plugin.test.ts index 10586683d..52e643781 100644 --- a/packages/plugins/mcp/src/sdk/plugin.test.ts +++ b/packages/plugins/mcp/src/sdk/plugin.test.ts @@ -454,6 +454,88 @@ describe("mcpPlugin", () => { }), ); + // ------------------------------------------------------------------------- + // updateSource must persist auth changes to the config file too — + // otherwise the next boot replays the file's stale auth and silently + // overwrites the DB. Symmetric with addSource/removeSource which + // already write through. + // ------------------------------------------------------------------------- + + it.effect("updateSource writes auth changes back to the config file", () => + Effect.gen(function* () { + const calls: Array<{ op: "upsert" | "remove"; payload: unknown }> = []; + const stubSink = { + upsertSource: (source: unknown) => + Effect.sync(() => { + calls.push({ op: "upsert", payload: source }); + }), + removeSource: (namespace: string) => + Effect.sync(() => { + calls.push({ op: "remove", payload: namespace }); + }), + }; + + const executor = yield* createExecutor( + makeTestConfig({ + plugins: [makeMemorySecretsPlugin()(), mcpPlugin({ configFile: stubSink })] as const, + }), + ); + + for (const id of ["sentry-token-old", "sentry-token-new"]) { + yield* executor.secrets.set({ + id: SecretId.make(id), + scope: ScopeId.make("test-scope"), + name: id, + value: `value-${id}`, + provider: "memory", + }); + } + + yield* executor.mcp + .addSource({ + transport: "remote", + scope: "test-scope", + name: "Sentry", + endpoint: "http://127.0.0.1:1/sentry-mcp", + namespace: "sentry", + credentialTargetScope: "test-scope", + auth: { + kind: "header", + headerName: "Authorization", + secretId: "sentry-token-old", + prefix: "Bearer ", + }, + }) + .pipe(Effect.result); + + calls.length = 0; // ignore the addSource upsert; we're asserting on update + + yield* executor.mcp.updateSource("sentry", "test-scope", { + credentialTargetScope: ScopeId.make("test-scope"), + auth: { + kind: "header", + headerName: "Authorization", + secretId: "sentry-token-new", + prefix: "Bearer ", + }, + }); + + const upserts = calls.filter((c) => c.op === "upsert"); + expect(upserts).toHaveLength(1); + expect(upserts[0]!.payload).toMatchObject({ + kind: "mcp", + transport: "remote", + namespace: "sentry", + auth: { + kind: "header", + headerName: "Authorization", + secret: "secret-public-ref:sentry-token-new", + prefix: "Bearer ", + }, + }); + }), + ); + // ------------------------------------------------------------------------- // Deferred OAuth — admin saves a source with `{kind: "oauth2", // connectionId}` before any user has signed in, so the row lands in diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index ab3991642..03eb9c590 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -18,6 +18,7 @@ import { ConfiguredCredentialBinding, ConnectionId, type CredentialBindingRef, + type CredentialBindingValue, ScopeId, SecretId, SourceDetectionResult, @@ -946,6 +947,102 @@ const authToConfig = (auth: McpConnectionAuthInput | undefined): McpAuthConfig | }; }; +// --------------------------------------------------------------------------- +// Storage-form → input-form reconstruction +// +// `toMcpConfigEntry` consumes the `McpSourceConfig` *input* shape — the +// legacy form with `secretId` / `connectionId`, which `authToConfig` and +// `plainStringMap` know how to render into the file. Stored remote data +// is in slot form (`secretSlot`, `{kind: "binding", slot}`), so writing +// the file from a stored row needs the slot → secret/connection lookups +// realized first. Walk the source's `credential_binding` rows and rebuild +// the input shape; any slot whose binding is missing is dropped. +// --------------------------------------------------------------------------- + +const toCredentialInput = ( + bySlot: Map, + configured: ConfiguredMcpCredentialValue, +): McpCredentialInput | undefined => { + if (typeof configured === "string") return configured; + const value = bySlot.get(configured.slot); + if (!value) return undefined; + if (value.kind === "secret") { + return { + secretId: value.secretId, + ...(configured.prefix ? { prefix: configured.prefix } : {}), + }; + } + if (value.kind === "text") return value.text; + // headers / queryParams cannot reference connections — only auth can. + return undefined; +}; + +const toCredentialInputMap = ( + bySlot: Map, + values: Record | undefined, +): Record | undefined => { + if (!values) return undefined; + const out: Record = {}; + for (const [name, configured] of Object.entries(values)) { + const input = toCredentialInput(bySlot, configured); + if (input !== undefined) out[name] = input; + } + return Object.keys(out).length > 0 ? out : undefined; +}; + +const toAuthInput = ( + bySlot: Map, + auth: McpConnectionAuth, +): McpConnectionAuthInput | undefined => { + if (auth.kind === "none") return { kind: "none" }; + if (auth.kind === "header") { + const value = bySlot.get(auth.secretSlot); + if (value?.kind !== "secret") return undefined; + return { + kind: "header", + headerName: auth.headerName, + secretId: value.secretId, + prefix: auth.prefix, + }; + } + const connection = bySlot.get(auth.connectionSlot); + if (connection?.kind !== "connection") return undefined; + return { kind: "oauth2", connectionId: connection.connectionId }; +}; + +const inputFormFromStored = ( + bindings: ReadonlyArray, + stored: McpStoredSourceData, + scope: string, + sourceName: string, + namespace: string, +): McpSourceConfig => { + if (stored.transport === "stdio") { + return { + transport: "stdio", + scope, + name: sourceName, + namespace, + command: stored.command, + args: stored.args ? [...stored.args] : undefined, + env: stored.env, + cwd: stored.cwd, + }; + } + const bySlot = new Map(bindings.map((b) => [b.slotKey, b.value] as const)); + return { + transport: "remote", + scope, + name: sourceName, + namespace, + endpoint: stored.endpoint, + remoteTransport: stored.remoteTransport, + headers: toCredentialInputMap(bySlot, stored.headers), + queryParams: toCredentialInputMap(bySlot, stored.queryParams), + auth: toAuthInput(bySlot, stored.auth), + }; +}; + const toMcpConfigEntry = ( namespace: string, sourceName: string, @@ -1489,6 +1586,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { ...(canonicalAuth ? { auth: canonicalAuth.auth } : {}), ...(canonicalQueryParams ? { queryParams: canonicalQueryParams.values } : {}), }; + const sourceName = input.name?.trim() || existing.name; const affectedPrefixes = [ ...(input.headers !== undefined ? ["header:"] : []), @@ -1501,7 +1599,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { yield* ctx.storage.putSource({ namespace, scope, - name: input.name?.trim() || existing.name, + name: sourceName, config: updatedConfig, }); if (affectedPrefixes.length > 0 || directBindings.length > 0) { @@ -1519,6 +1617,24 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { } }), ); + + if (configFile) { + const bindings = yield* ctx.credentialBindings.listForSource({ + pluginId: MCP_PLUGIN_ID, + sourceId: namespace, + sourceScope: ScopeId.make(scope), + }); + const inputForm = inputFormFromStored( + bindings, + updatedConfig, + scope, + sourceName, + namespace, + ); + yield* configFile + .upsertSource(toMcpConfigEntry(namespace, sourceName, inputForm)) + .pipe(Effect.withSpan("mcp.plugin.config_file.upsert")); + } }).pipe( Effect.withSpan("mcp.plugin.update_source", { attributes: { "mcp.source.namespace": namespace },