From 3096cc85d8c60201187f840ec583257b5ebef8e8 Mon Sep 17 00:00:00 2001 From: Saatvik Arya Date: Fri, 8 May 2026 11:40:29 +0530 Subject: [PATCH 1/2] fix(local): replay MCP auth during config sync Boot-time addSourceFromConfig was forwarding endpoint/headers/queryParams from executor.jsonc into executor.mcp.addSource(...) but silently dropping the auth block. Keychain-backed remote MCP sources started unauthenticated on every boot, the SSE handshake fired with no Authorization header, and upstreams rejected with the "Failed connecting via sse" error class. Add an inline mcpAuthFromConfig converter that strips the secret-public-ref: prefix from file-shape auth into runtime McpConnectionAuth.secretId, and thread it through the MCP remote addSource call. none and oauth2 are identity passthroughs (oauth2's runtime extras are optional, so the file shape is structurally valid as runtime auth). New config-sync.test.ts covers all three auth kinds against an unreachable endpoint, which still persists the source row so the stored auth shape can be asserted without seeding secrets or running an MCP server. --- .../local/src/server/config-sync.boot.test.ts | 225 ++++++++++++++++++ apps/local/src/server/config-sync.ts | 3 + 2 files changed, 228 insertions(+) create mode 100644 apps/local/src/server/config-sync.boot.test.ts 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); }), From b925017c5bebb1a990a7a862e724bc83f32ba75c Mon Sep 17 00:00:00 2001 From: Saatvik Arya Date: Fri, 8 May 2026 11:40:45 +0530 Subject: [PATCH 2/2] fix(mcp): persist source auth updates to config file updateSource wrote new auth to the DB but never to executor.jsonc, so the next boot replayed the file's stale auth (or none) and silently overwrote the DB. The smoking gun: after signing into Sentry via the UI, the mcp-oauth2-sentry connection survived in keychain with valid tokens, but the next reboot cleared the source row's auth_connection_id because the file never reflected the link. Mirror the existing addSource/removeSource pattern by calling configFile.upsertSource(toMcpConfigEntry(...)) after putSource. The existing toMcpConfigEntry/authToConfig path handles the runtime->file conversion (writes secret-public-ref: back), so the file stays authoritative for boot replay. Test exercises updateSource against a stub ConfigFileSink and asserts the upsert payload uses secret-public-ref: in file shape. --- packages/plugins/mcp/src/sdk/plugin.test.ts | 82 ++++++++++++++ packages/plugins/mcp/src/sdk/plugin.ts | 118 +++++++++++++++++++- 2 files changed, 199 insertions(+), 1 deletion(-) 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 },