mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-08-30 17:14:40 +08:00
fix(config): expand MCP header env refs without wiping MCP set (#12554)
* fix(config): expand MCP header env refs without wiping MCP set
Project kilo.jsonc with {env:…} in MCP headers caused the whole config
load to fail since 7.4.5, silently removing every MCP. Leave {env:} literal
during untrusted text substitution, expand only remote MCP headers after
parse, and drop individual MCPs with blocked env refs instead of the full set.
Fixes #12319
* fix(config): ban {file:} during MCP header env expansion
Post-parse header expand used trusted substitute without fileScope, so a
decoy in-repo {file:} plus any {env:} could read arbitrary paths into
remote MCP headers. Expand only {env:} and reject residual {file:}.
* fix(config): ban env secret expansion in project MCP headers
Untrusted project MCP headers must not read process.env or authEnv.
Reject {env:} and {file:} per entry with a warning so sibling MCPs still load.
* fix(config): drop global MCP headers on project URL retarget
When a project config overrides a same-named global remote MCP with a
new URL and does not restate headers, do not inherit expanded secret
headers from the global entry. Also fix MCP union narrowing in tests
and stop saying "header env expansion failed" when only validating.
* fix(config): do not mutate caller's patch in mergeConfig
Shallow-copy patch before delete p.mcp so probe-then-write
updateProjectConfig paths keep mcp on the original object.
* fix(config): secure project MCP header loading
* Update packages/opencode/src/config/config.ts
Co-authored-by: kilo-code-bot[bot] <240665456+kilo-code-bot[bot]@users.noreply.github.com>
* fix(config): secure layered MCP transitions
---------
Co-authored-by: arimu1 <19286898+arimu1@users.noreply.github.com>
Co-authored-by: Johnny Eric Amancio <johnnyeric@gmail.com>
Co-authored-by: kilo-code-bot[bot] <240665456+kilo-code-bot[bot]@users.noreply.github.com>
This commit is contained in:
@@ -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.
|
||||
@@ -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<string, string>,
|
||||
// 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<string, string>,
|
||||
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)
|
||||
|
||||
@@ -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<string, unknown>
|
||||
const p = patch as Record<string, unknown>
|
||||
// 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<string, unknown>
|
||||
|
||||
// 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<string, unknown>) 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<string, unknown>) : mergeDeep(e, p)) as Config.Info
|
||||
}
|
||||
|
||||
delete e.mcp
|
||||
delete p.mcp
|
||||
const merged = (clean ? stripNulls(mergeDeep(e, p) as Record<string, unknown>) : mergeDeep(e, p)) as Config.Info
|
||||
const baseMcp = isRecord(existingMcp) ? (existingMcp as NonNullable<Config.Info["mcp"]>) : undefined
|
||||
const srcMcp = isRecord(patchMcp) ? (patchMcp as NonNullable<Config.Info["mcp"]>) : undefined
|
||||
if (!srcMcp) {
|
||||
if (baseMcp) merged.mcp = baseMcp
|
||||
return merged
|
||||
}
|
||||
if (!baseMcp) {
|
||||
merged.mcp = srcMcp
|
||||
return merged
|
||||
}
|
||||
|
||||
const out: NonNullable<Config.Info["mcp"]> = { ...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<string, unknown>
|
||||
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 ───────────────────────────────────────────
|
||||
|
||||
@@ -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<T>(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 }
|
||||
}
|
||||
@@ -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<Config.Info> {
|
||||
// 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
|
||||
}
|
||||
|
||||
|
||||
@@ -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 })
|
||||
|
||||
@@ -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<Config.Info["mcp"]>[string] | undefined,
|
||||
): m is Extract<NonNullable<Config.Info["mcp"]>[string], { type: "remote" }> {
|
||||
return !!m && typeof m === "object" && "type" in m && m.type === "remote"
|
||||
}
|
||||
|
||||
function remote(
|
||||
url: string,
|
||||
headers?: Record<string, string>,
|
||||
): Extract<NonNullable<Config.Info["mcp"]>[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<Config.Info["mcp"]>[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")
|
||||
})
|
||||
Reference in New Issue
Block a user