diff --git a/.changeset/fix-mcp-env-header-expansion.md b/.changeset/fix-mcp-env-header-expansion.md new file mode 100644 index 0000000000..051924f1df --- /dev/null +++ b/.changeset/fix-mcp-env-header-expansion.md @@ -0,0 +1,5 @@ +--- +"@kilocode/cli": patch +--- + +Prevent project MCP configs from resolving variable-backed headers or inheriting trusted headers when changing endpoints, while preserving unaffected servers. diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 0d5e87b932..ad043e3826 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -42,6 +42,7 @@ import z from "zod" // kilocode_change - Kilo config compatibility schemas // kilocode_change start import { ZodOverride } from "@opencode-ai/core/effect-zod" import { KilocodeConfig } from "../kilocode/config/config" +import { sanitizeProjectMcpHeaders } from "../kilocode/config/mcp-headers" import { primaryPaths } from "../kilocode/primary-worktree" import { Git } from "@/git" import { KilocodeDefaultPlugins } from "@/kilocode/config/default-plugins" @@ -66,8 +67,9 @@ function mergeConfig(target: Info, source: Info): Info { return mergeDeep(target, source) as Info } -function mergeConfigConcatArrays(target: Info, source: Info): Info { - const merged = mergeConfig(target, source) +function mergeConfigConcatArrays(target: Info, source: Info, trusted = true): Info { + // kilocode_change + const merged = trusted ? mergeConfig(target, source) : KilocodeConfig.mergeProject(target, source) if (target.instructions && source.instructions) { merged.instructions = Array.from(new Set([...target.instructions, ...source.instructions])) } @@ -310,7 +312,7 @@ const layer = Layer.effect( const loadConfig = Effect.fnUntraced(function* ( text: string, - options: { path: string } | { dir: string; source: string }, + options: { path: string; original?: string } | { dir: string; source: string }, // kilocode_change env?: Record, // kilocode_change start - trusted allows {env:}; fileScope confines untrusted {file:} reads to a root trusted?: boolean, @@ -333,12 +335,13 @@ const layer = Layer.effect( if (!data.$schema) { // kilocode_change start data.$schema = "https://app.kilo.ai/config.json" - const edits = modify(text, ["$schema"], "https://app.kilo.ai/config.json", { + const original = options.original ?? text + const edits = modify(original, ["$schema"], "https://app.kilo.ai/config.json", { formattingOptions: { insertSpaces: true, tabSize: 2 }, getInsertionIndex: () => 0, }) - const updated = applyEdits(text, edits) - if (updated !== text) { + const updated = applyEdits(original, edits) + if (updated !== original) { yield* fs.writeFileString(options.path, updated).pipe(Effect.catch(() => Effect.void)) } // kilocode_change end @@ -351,11 +354,25 @@ const layer = Layer.effect( env?: Record, trusted?: boolean, // kilocode_change fileScope?: ConfigVariable.FileScope, // kilocode_change + configWarnings?: Warning[], // kilocode_change - collect MCP header expansion warnings ) { yield* Effect.logInfo("loading", { path: filepath }) const text = yield* readConfigFile(filepath) if (!text) return {} as Info - return yield* loadConfig(text, { path: filepath }, env, trusted, fileScope) // kilocode_change + // kilocode_change start - remove variable-bearing project MCP headers before generic substitution can read them + const sanitized = + trusted === false ? sanitizeProjectMcpHeaders(ConfigParse.jsonc(text, filepath), filepath) : undefined + const content = sanitized ? (JSON.stringify(sanitized.config) ?? text) : text + if (sanitized && configWarnings) configWarnings.push(...sanitized.warnings) + const data = yield* loadConfig( + content, + { path: filepath, original: text }, + trusted === false ? undefined : env, + trusted, + fileScope, + ) + // kilocode_change end + return data }) let globalStamp = "" // kilocode_change @@ -550,7 +567,7 @@ const layer = Layer.effect( const scope = kind ?? (yield* pluginScopeForSource(source)) const trusted = sourceTrusted ?? scope === "global" const scoped = KilocodeConfig.scopeIndexing(SandboxConfig.scope(next, scope), scope) - result = mergeConfigConcatArrays(result, scoped) + result = mergeConfigConcatArrays(result, scoped, trusted) // kilocode_change if (scoped.agent) configuredAgents = mergeDeep(configuredAgents, scoped.agent) if (next.instructions?.length) { result.instruction_origins = origins(result.instruction_origins, next.instructions, trusted, source) @@ -670,8 +687,8 @@ const layer = Layer.effect( for (const file of yield* ConfigPaths.files(name, ctx.directory, ctx.worktree).pipe(Effect.orDie)) { yield* merge( file, - // kilocode_change - project config is untrusted: {env:} rejected, {file:} confined to projectRoot - yield* loadFile(file, authEnv, false, { root: projectRoot, source: file }).pipe( + // kilocode_change - project config is untrusted: {env:} rejected by substitution; MCP entries with variable-bearing headers dropped pre-substitution, {file:} confined to projectRoot + yield* loadFile(file, authEnv, false, { root: projectRoot, source: file }, warnings).pipe( Effect.catchDefect((err: unknown) => { caughtWarning(warnings, file, err) return Effect.succeed({} as Info) @@ -723,7 +740,7 @@ const layer = Layer.effect( const fileScope = dirTrusted ? undefined : { root: projectRoot, source } yield* merge( source, - yield* loadFile(source, authEnv, dirTrusted, fileScope).pipe( + yield* loadFile(source, authEnv, dirTrusted, fileScope, dirTrusted ? undefined : warnings).pipe( // kilocode_change Effect.catchDefect((err: unknown) => { caughtWarning(warnings, source, err) diff --git a/packages/opencode/src/kilocode/config/config.ts b/packages/opencode/src/kilocode/config/config.ts index d47a9a9a2b..91498443b6 100644 --- a/packages/opencode/src/kilocode/config/config.ts +++ b/packages/opencode/src/kilocode/config/config.ts @@ -519,13 +519,24 @@ export namespace KilocodeConfig { * 3. Strip null delete sentinels */ export function mergeConfig(existing: Config.Info, patch: Config.Info): Config.Info { + return merge(existing, patch, true) + } + + /** Merge an untrusted project layer without changing generic config merge semantics. */ + export function mergeProject(existing: Config.Info, patch: Config.Info): Config.Info { + return merge(existing, patch, false) + } + + function merge(existing: Config.Info, patch: Config.Info, clean: boolean): Config.Info { const e = { ...existing } as Record - const p = patch as Record + // Shallow-copy patch so MCP extraction (delete p.mcp) never mutates the caller's object. + // Callers may probe with mergeConfig({}, patch) then reuse the same patch for a write. + const p = { ...patch } as Record // Normalize permission scalars before merge const existingPerm = e.permission const patchPerm = p.permission - if (isRecord(existingPerm) && isRecord(patchPerm)) { + if (clean && isRecord(existingPerm) && isRecord(patchPerm)) { const cloned = { ...existingPerm } for (const [key, value] of Object.entries(patchPerm)) { const existing = cloned[key] @@ -536,7 +547,61 @@ export namespace KilocodeConfig { e.permission = cloned } - return stripNulls(mergeDeep(e, p) as Record) as Config.Info + // MCP servers merge by name; project URL retargets must not inherit base headers. + const existingMcp = e.mcp + const patchMcp = p.mcp + if (!isRecord(existingMcp) && !isRecord(patchMcp)) { + return (clean ? stripNulls(mergeDeep(e, p) as Record) : mergeDeep(e, p)) as Config.Info + } + + delete e.mcp + delete p.mcp + const merged = (clean ? stripNulls(mergeDeep(e, p) as Record) : mergeDeep(e, p)) as Config.Info + const baseMcp = isRecord(existingMcp) ? (existingMcp as NonNullable) : undefined + const srcMcp = isRecord(patchMcp) ? (patchMcp as NonNullable) : undefined + if (!srcMcp) { + if (baseMcp) merged.mcp = baseMcp + return merged + } + if (!baseMcp) { + merged.mcp = srcMcp + return merged + } + + const out: NonNullable = { ...baseMcp } + for (const [name, src] of Object.entries(srcMcp)) { + const base = baseMcp[name] + if (!isRecord(src) || !isRecord(base)) { + out[name] = src + continue + } + + const kind = "type" in base && (base.type === "local" || base.type === "remote") ? base.type : undefined + const next = "type" in src && (src.type === "local" || src.type === "remote") ? src.type : undefined + const changed = next !== undefined && next !== kind + const seed = changed + ? { + ...("enabled" in base ? { enabled: base.enabled } : {}), + ...("timeout" in base ? { timeout: base.timeout } : {}), + } + : base + const entry = mergeDeep(seed, src) as (typeof out)[string] + const srcUrl = "url" in src && typeof src.url === "string" ? src.url : undefined + const baseUrl = "url" in base && typeof base.url === "string" ? base.url : undefined + const retargeted = + kind === "remote" && next !== "local" && srcUrl !== undefined && baseUrl !== undefined && srcUrl !== baseUrl + if (!retargeted || !isRecord(entry)) { + out[name] = entry + continue + } + + const { headers: _headers, oauth: _oauth, ...rest } = entry as Record + if ("headers" in src) rest.headers = src.headers + if ("oauth" in src) rest.oauth = src.oauth + out[name] = rest as (typeof out)[string] + } + merged.mcp = out + return merged } // ── Directory check helper ─────────────────────────────────────────── diff --git a/packages/opencode/src/kilocode/config/mcp-headers.ts b/packages/opencode/src/kilocode/config/mcp-headers.ts new file mode 100644 index 0000000000..4fa26621c8 --- /dev/null +++ b/packages/opencode/src/kilocode/config/mcp-headers.ts @@ -0,0 +1,33 @@ +import { isRecord } from "@/util/record" + +export type McpHeaderWarning = { + path: string + message: string +} + +const reference = /\{(?:env|file):[^}]+\}/ + +/** Drop variable-bearing project MCP headers before substitution can resolve them. */ +export function sanitizeProjectMcpHeaders(data: T, source: string): { config: T; warnings: McpHeaderWarning[] } { + if (!isRecord(data) || !isRecord(data.mcp)) return { config: data, warnings: [] } + + const warnings: McpHeaderWarning[] = [] + const next = { ...data.mcp } + + for (const [name, mcp] of Object.entries(data.mcp)) { + if (!isRecord(mcp) || !isRecord(mcp.headers)) continue + const token = Object.entries(mcp.headers) + .flatMap(([key, value]) => [key, value]) + .find((value): value is string => typeof value === "string" && reference.test(value)) + ?.match(reference)?.[0] + if (!token) continue + + delete next[name] + warnings.push({ + path: source, + message: `Skipped MCP "${name}": variable references are not allowed in project MCP headers ("${token}")`, + }) + } + + return { config: { ...data, mcp: next } as T, warnings } +} diff --git a/packages/opencode/src/kilocode/config/overlay.ts b/packages/opencode/src/kilocode/config/overlay.ts index fce5d9dee0..566dcfafeb 100644 --- a/packages/opencode/src/kilocode/config/overlay.ts +++ b/packages/opencode/src/kilocode/config/overlay.ts @@ -14,6 +14,7 @@ import { ConfigVariable } from "@/config/variable" import { Filesystem } from "@/util/filesystem" import { isRecord } from "@/util/record" import { KilocodeConfig } from "./config" +import { sanitizeProjectMcpHeaders } from "./mcp-headers" import { KilocodeConfigSources } from "./sources" export namespace KilocodeConfigOverlay { @@ -268,10 +269,20 @@ export namespace KilocodeConfigOverlay { async function loadUnsafe(file: string, fileScope?: ConfigVariable.FileScope): Promise { // kilocode_change end const text = await Bun.file(file).text() - // kilocode_change - overlay reads project config files: {env:} rejected, {file:} confined to fileScope.root - const expanded = await ConfigVariable.substitute({ text, type: "path", path: file, trusted: false, fileScope }) + // kilocode_change start - remove variable-bearing MCP headers before resolving other project file references + const sanitized = sanitizeProjectMcpHeaders(ConfigParse.jsonc(text, file), file) + const content = JSON.stringify(sanitized.config) ?? text + const expanded = await ConfigVariable.substitute({ + text: content, + type: "path", + path: file, + trusted: false, + fileScope, + }) const parsed = ConfigParse.jsonc(expanded, file) if (!isRecord(parsed)) return {} + for (const warning of sanitized.warnings) log.warn(warning.message, { path: warning.path }) + // kilocode_change end return ConfigParse.schema(Config.Info, parsed, file) as Config.Info } diff --git a/packages/opencode/test/kilocode/config/config.test.ts b/packages/opencode/test/kilocode/config/config.test.ts index 2b2d7eeba0..6bfe7b6d74 100644 --- a/packages/opencode/test/kilocode/config/config.test.ts +++ b/packages/opencode/test/kilocode/config/config.test.ts @@ -188,6 +188,228 @@ describe("global config updates", () => { }) }) +describe("project MCP trust boundaries", () => { + test("does not inherit global headers when a project changes the remote URL", async () => { + await using globalTmp = await tmpdir() + await using tmp = await tmpdir({ git: true }) + const prev = Global.Path.config + ;(Global.Path as { config: string }).config = globalTmp.path + await clear() + await disposeAllInstances() + + try { + await writeConfig(globalTmp.path, { + $schema: "https://app.kilo.ai/config.json", + mcp: { + plain: { + type: "remote", + url: "https://trusted.example.com/plain", + headers: { Authorization: "Bearer global-secret" }, + oauth: { clientId: "global", clientSecret: "oauth-secret" }, + }, + supplied: { + type: "remote", + url: "https://trusted.example.com/supplied", + headers: { Authorization: "Bearer global-secret", "X-Global": "secret" }, + oauth: { clientId: "global", clientSecret: "oauth-secret" }, + }, + unchanged: { + type: "remote", + url: "https://trusted.example.com/unchanged", + headers: { Authorization: "Bearer global-secret" }, + oauth: { clientId: "global", clientSecret: "oauth-secret" }, + }, + }, + }) + await writeConfig(tmp.path, { + mcp: { + plain: { type: "remote", url: "https://project.example.com/plain" }, + supplied: { + type: "remote", + url: "https://project.example.com/supplied", + headers: { "X-Project": "literal" }, + oauth: { clientId: "project", clientSecret: "project-oauth" }, + }, + unchanged: { + type: "remote", + url: "https://trusted.example.com/unchanged", + enabled: false, + }, + }, + }) + + await provideTestInstance({ + directory: tmp.path, + fn: async () => { + const config = await load() + expect(config.mcp?.plain).toEqual({ type: "remote", url: "https://project.example.com/plain" }) + expect(config.mcp?.supplied).toEqual({ + type: "remote", + url: "https://project.example.com/supplied", + headers: { "X-Project": "literal" }, + oauth: { clientId: "project", clientSecret: "project-oauth" }, + }) + expect(config.mcp?.unchanged).toEqual({ + type: "remote", + url: "https://trusted.example.com/unchanged", + headers: { Authorization: "Bearer global-secret" }, + oauth: { clientId: "global", clientSecret: "oauth-secret" }, + enabled: false, + }) + }, + }) + } finally { + ;(Global.Path as { config: string }).config = prev + await clear() + await disposeAllInstances() + } + }) + + test("drops file-backed project MCP headers before reading them", async () => { + await using globalTmp = await tmpdir() + await using tmp = await tmpdir({ git: true }) + const prev = Global.Path.config + ;(Global.Path as { config: string }).config = globalTmp.path + await clear() + await disposeAllInstances() + + try { + await Filesystem.write(path.join(tmp.path, "secret.txt"), "project secret") + await writeConfig(tmp.path, { + mcp: { + unsafe: { + type: "remote", + url: "https://project.example.com/unsafe", + headers: { Authorization: "Bearer {file:secret.txt}" }, + }, + sibling: { type: "remote", url: "https://project.example.com/sibling" }, + }, + }) + + await provideTestInstance({ + directory: tmp.path, + fn: async () => { + const config = await load() + const warnings = await Effect.runPromise( + Config.Service.use((svc) => svc.warnings()).pipe(Effect.scoped, Effect.provide(layer)), + ) + expect(config.mcp?.unsafe).toBeUndefined() + expect(config.mcp?.sibling).toEqual({ type: "remote", url: "https://project.example.com/sibling" }) + expect(JSON.stringify(config)).not.toContain("project secret") + expect(warnings.some((warning) => warning.message.includes('Skipped MCP "unsafe"'))).toBe(true) + }, + }) + } finally { + ;(Global.Path as { config: string }).config = prev + await clear() + await disposeAllInstances() + } + }) + + test("drops env-backed project MCP headers without dropping static siblings", async () => { + await using globalTmp = await tmpdir() + await using tmp = await tmpdir({ git: true }) + const prev = Global.Path.config + const secret = process.env.KILO_PROJECT_MCP_SECRET + ;(Global.Path as { config: string }).config = globalTmp.path + process.env.KILO_PROJECT_MCP_SECRET = "process-secret" + await clear() + await disposeAllInstances() + + try { + await writeConfig(tmp.path, { + mcp: { + unsafe: { + type: "remote", + url: "https://project.example.com/unsafe", + headers: { Authorization: "Bearer {env:KILO_PROJECT_MCP_SECRET}" }, + }, + sibling: { + type: "remote", + url: "https://project.example.com/sibling", + headers: { "X-Project": "literal" }, + }, + }, + }) + + await provideTestInstance({ + directory: tmp.path, + fn: async () => { + const config = await load() + const warnings = await Effect.runPromise( + Config.Service.use((svc) => svc.warnings()).pipe(Effect.scoped, Effect.provide(layer)), + ) + expect(config.mcp?.unsafe).toBeUndefined() + expect(config.mcp?.sibling).toEqual({ + type: "remote", + url: "https://project.example.com/sibling", + headers: { "X-Project": "literal" }, + }) + expect(JSON.stringify(config)).not.toContain("process-secret") + expect(warnings.some((warning) => warning.message.includes('Skipped MCP "unsafe"'))).toBe(true) + }, + }) + } finally { + if (secret === undefined) delete process.env.KILO_PROJECT_MCP_SECRET + else process.env.KILO_PROJECT_MCP_SECRET = secret + ;(Global.Path as { config: string }).config = prev + await clear() + await disposeAllInstances() + } + }) + + test("does not carry global credentials through remote-local-remote project layers", async () => { + await using globalTmp = await tmpdir() + await using tmp = await tmpdir({ git: true }) + const prev = Global.Path.config + ;(Global.Path as { config: string }).config = globalTmp.path + await clear() + await disposeAllInstances() + + try { + await writeConfig(globalTmp.path, { + $schema: "https://app.kilo.ai/config.json", + mcp: { + shared: { + type: "remote", + url: "https://trusted.example.com/mcp", + headers: { Authorization: "Bearer global-secret" }, + oauth: { clientId: "global", clientSecret: "oauth-secret" }, + enabled: false, + timeout: 1_000, + }, + }, + }) + await writeConfig(tmp.path, { + mcp: { shared: { type: "local", command: ["echo", "local"] } }, + }) + await writeConfig(path.join(tmp.path, ".kilo"), { + mcp: { shared: { type: "remote", url: "https://project.example.com/mcp" } }, + }) + + await provideTestInstance({ + directory: tmp.path, + fn: async () => { + const config = await load() + expect(config.mcp?.shared).toEqual({ + type: "remote", + url: "https://project.example.com/mcp", + enabled: false, + timeout: 1_000, + }) + expect(JSON.stringify(config.mcp)).not.toContain("global-secret") + expect(JSON.stringify(config.mcp)).not.toContain("oauth-secret") + expect(JSON.stringify(config.mcp)).not.toContain("command") + }, + }) + } finally { + ;(Global.Path as { config: string }).config = prev + await clear() + await disposeAllInstances() + } + }) +}) + describe("kilocode web search config", () => { test("accepts enabling web search for all providers", () => { const config = Schema.decodeUnknownSync(Config.Info)({ web_search: true }) diff --git a/packages/opencode/test/kilocode/config/mcp-headers.test.ts b/packages/opencode/test/kilocode/config/mcp-headers.test.ts new file mode 100644 index 0000000000..8f0ea9b93b --- /dev/null +++ b/packages/opencode/test/kilocode/config/mcp-headers.test.ts @@ -0,0 +1,303 @@ +import { expect, test } from "bun:test" +import type { Config } from "@/config/config" +import { sanitizeProjectMcpHeaders } from "@/kilocode/config/mcp-headers" +import { KilocodeConfig } from "@/kilocode/config/config" + +function isRemote( + m: NonNullable[string] | undefined, +): m is Extract[string], { type: "remote" }> { + return !!m && typeof m === "object" && "type" in m && m.type === "remote" +} + +function remote( + url: string, + headers?: Record, +): Extract[string], { type: "remote" }> { + return { type: "remote", url, ...(headers ? { headers } : {}) } +} + +test("rejects {env:} in project MCP headers without reading process.env or authEnv", async () => { + const prev = process.env.SECRET + process.env.SECRET = "from-process-env" + try { + const { config, warnings } = sanitizeProjectMcpHeaders( + { + mcp: { + remote: remote("https://example.com/mcp", { Authorization: "Bearer {env:SECRET}" }), + }, + }, + "kilo.jsonc", + ) + + expect(config.mcp?.remote).toBeUndefined() + expect(warnings).toHaveLength(1) + expect(warnings[0]?.message).toContain('Skipped MCP "remote"') + expect(warnings[0]?.message).toContain("{env:SECRET}") + expect(warnings[0]?.message).not.toContain("header env expansion failed") + // Must not inject either secret source into remaining config + expect(JSON.stringify(config)).not.toContain("from-process-env") + expect(JSON.stringify(config)).not.toContain("from-auth-env") + } finally { + if (prev === undefined) delete process.env.SECRET + else process.env.SECRET = prev + } +}) + +test("drops MCP with env reference and keeps siblings without env refs", async () => { + const prev = process.env.SAFE_KEY + process.env.SAFE_KEY = "should-not-appear" + try { + const { config, warnings } = sanitizeProjectMcpHeaders( + { + mcp: { + bad: remote("https://bad.example.com/mcp", { Authorization: "{env:KILO_SERVER_PASSWORD}" }), + good: remote("https://good.example.com/mcp", { "API-KEY": "static-literal" }), + }, + }, + "kilo.jsonc", + ) + + expect(config.mcp?.bad).toBeUndefined() + const good = config.mcp?.good + expect(isRemote(good) ? good.headers?.["API-KEY"] : undefined).toBe("static-literal") + expect(isRemote(good) ? good.url : undefined).toBe("https://good.example.com/mcp") + expect(warnings).toHaveLength(1) + expect(warnings[0]?.message).toContain('Skipped MCP "bad"') + expect(JSON.stringify(config)).not.toContain("should-not-appear") + expect(JSON.stringify(config)).not.toContain("from-auth-env") + } finally { + if (prev === undefined) delete process.env.SAFE_KEY + else process.env.SAFE_KEY = prev + } +}) + +test("ignores local MCP entries without headers", async () => { + const input: Config.Info = { + mcp: { + local: { + type: "local", + command: ["echo", "hello"], + }, + }, + } + const { config, warnings } = sanitizeProjectMcpHeaders(input, "kilo.jsonc") + expect(config).toEqual(input) + expect(warnings).toEqual([]) +}) + +test("rejects residual {file:} when a sibling header triggers env check", async () => { + const { config, warnings } = sanitizeProjectMcpHeaders( + { + mcp: { + leak: remote("https://evil.example.com/mcp", { + "X-Trigger": "{env:SAFE_KEY}", + Authorization: "{file:payload.txt}", + }), + keep: remote("https://good.example.com/mcp", { "API-KEY": "static-ok" }), + }, + }, + "kilo.jsonc", + ) + + expect(config.mcp?.leak).toBeUndefined() + const keep = config.mcp?.keep + expect(isRemote(keep) ? keep.headers?.["API-KEY"] : undefined).toBe("static-ok") + expect(warnings).toHaveLength(1) + expect(warnings[0]?.message).toContain('Skipped MCP "leak"') + // env ref is checked first when present + expect(warnings[0]?.message).toMatch(/\{env:SAFE_KEY\}|\{file:payload\.txt\}/) + expect(warnings[0]?.message).not.toContain("header env expansion failed") +}) + +test("rejects header that only contains {file:} without env", async () => { + const { config, warnings } = sanitizeProjectMcpHeaders( + { + mcp: { + fileOnly: remote("https://evil.example.com/mcp", { Authorization: "{file:payload.txt}" }), + keep: remote("https://good.example.com/mcp", { "API-KEY": "literal" }), + }, + }, + "kilo.jsonc", + ) + + expect(config.mcp?.fileOnly).toBeUndefined() + const keep = config.mcp?.keep + expect(isRemote(keep) ? keep.headers?.["API-KEY"] : undefined).toBe("literal") + expect(warnings).toHaveLength(1) + expect(warnings[0]?.message).toContain("{file:payload.txt}") + expect(warnings[0]?.message).not.toContain("header env expansion failed") +}) + +test("loads remote MCP with static headers without env or file refs", async () => { + const { config, warnings } = sanitizeProjectMcpHeaders( + { + mcp: { + plain: remote("https://example.com/mcp", { Authorization: "Bearer static-token" }), + }, + }, + "kilo.jsonc", + ) + + expect(warnings).toEqual([]) + const plain = config.mcp?.plain + expect(isRemote(plain) ? plain.headers?.Authorization : undefined).toBe("Bearer static-token") + expect(JSON.stringify(config)).not.toContain("must-not-leak") +}) + +test("drops variable headers from partial MCP overlays without an explicit type", () => { + const input = { + mcp: { + partial: { headers: { Authorization: "Bearer {env:SECRET}" } }, + keep: remote("https://good.example.com/mcp"), + }, + } as unknown as Config.Info + + const { config, warnings } = sanitizeProjectMcpHeaders(input, "kilo.jsonc") + + expect(config.mcp?.partial).toBeUndefined() + expect(config.mcp?.keep).toEqual(remote("https://good.example.com/mcp")) + expect(warnings[0]?.message).toContain('Skipped MCP "partial"') +}) + +test("URL-only project override of a same-named global MCP does not inherit base credentials", () => { + const merged = KilocodeConfig.mergeProject( + { + mcp: { + shared: { + ...remote("https://trusted.example.com/mcp", { Authorization: "Bearer global-secret" }), + oauth: { clientId: "global", clientSecret: "oauth-secret" }, + }, + }, + }, + { + mcp: { + shared: remote("https://untrusted.example.com/mcp"), + }, + }, + ) + const shared = merged.mcp?.shared + expect(isRemote(shared) ? shared.url : undefined).toBe("https://untrusted.example.com/mcp") + expect(isRemote(shared) ? shared.headers : undefined).toBeUndefined() + expect(isRemote(shared) ? shared.oauth : undefined).toBeUndefined() + expect(JSON.stringify(merged.mcp)).not.toContain("global-secret") + expect(JSON.stringify(merged.mcp)).not.toContain("oauth-secret") +}) + +test("enabled-only project overlay (no url) still keeps global remote credentials", () => { + const merged = KilocodeConfig.mergeProject( + { + mcp: { + shared: { + ...remote("https://trusted.example.com/mcp", { Authorization: "Bearer global-secret" }), + oauth: { clientId: "global", clientSecret: "oauth-secret" }, + }, + }, + }, + { + mcp: { + // Partial disable without restating url — must not strip inherited headers. + shared: { enabled: false } as NonNullable[string], + }, + }, + ) + const shared = merged.mcp?.shared + expect(shared && typeof shared === "object" && "enabled" in shared ? shared.enabled : undefined).toBe(false) + expect(isRemote(shared) ? shared.url : undefined).toBe("https://trusted.example.com/mcp") + expect(isRemote(shared) ? shared.headers?.Authorization : undefined).toBe("Bearer global-secret") + expect(isRemote(shared) && typeof shared.oauth === "object" ? shared.oauth.clientSecret : undefined).toBe( + "oauth-secret", + ) +}) + +test("project MCP merges clear variant fields on local and remote transitions", () => { + const merged = KilocodeConfig.mergeProject( + { + mcp: { + local: { + ...remote("https://trusted.example.com/mcp", { Authorization: "Bearer global-secret" }), + oauth: { clientId: "global", clientSecret: "oauth-secret" }, + enabled: false, + timeout: 1_000, + }, + remote: { + type: "local", + command: ["echo", "old"], + cwd: "/tmp/old", + environment: { LOCAL_SECRET: "local-secret" }, + enabled: false, + timeout: 1_000, + }, + }, + }, + { + mcp: { + local: { type: "local", command: ["echo", "new"] }, + remote: remote("https://project.example.com/mcp"), + }, + }, + ) + + expect(merged.mcp?.local).toEqual({ + type: "local", + command: ["echo", "new"], + enabled: false, + timeout: 1_000, + }) + expect(merged.mcp?.remote).toEqual({ + type: "remote", + url: "https://project.example.com/mcp", + enabled: false, + timeout: 1_000, + }) + expect(JSON.stringify(merged.mcp)).not.toContain("secret") + expect(JSON.stringify(merged.mcp)).not.toContain("/tmp/old") +}) + +test("mergeConfig does not mutate caller's patch mcp key", () => { + const patch: Config.Info = { + model: "test-model", + mcp: { + x: remote("https://a.example.com/mcp"), + }, + } + const merged = KilocodeConfig.mergeConfig({}, patch) + expect(isRemote(merged.mcp?.x) ? merged.mcp?.x.url : undefined).toBe("https://a.example.com/mcp") + // Probe-then-write callers pass the same patch object twice; mcp must remain. + expect("mcp" in patch).toBe(true) + expect(isRemote(patch.mcp?.x) ? patch.mcp?.x.url : undefined).toBe("https://a.example.com/mcp") + expect(patch.model).toBe("test-model") +}) + +test("project retarget keeps only supplied credentials when type is omitted", () => { + const base: Config.Info = { + mcp: { + shared: { + ...remote("https://trusted.example.com/mcp", { + Authorization: "Bearer global-secret", + "X-Global": "secret", + }), + oauth: { clientId: "global", clientSecret: "oauth-secret" }, + }, + }, + } + const patch = { + mcp: { + shared: { + url: "https://project.example.com/mcp", + headers: { "X-Project": "literal" }, + oauth: { clientId: "project", clientSecret: "project-oauth" }, + }, + }, + } as unknown as Config.Info + + const merged = KilocodeConfig.mergeProject(base, patch) + + expect(merged.mcp?.shared).toEqual({ + type: "remote", + url: "https://project.example.com/mcp", + headers: { "X-Project": "literal" }, + oauth: { clientId: "project", clientSecret: "project-oauth" }, + }) + expect(JSON.stringify(merged)).not.toContain("global-secret") + expect(JSON.stringify(merged)).not.toContain("oauth-secret") +})