fix(cli): block project markdown secret exfiltration (#12168)

* fix(cli): guard markdown substitutions by config trust

* chore(cli): annotate markdown trust test changes

* fix(cli): preserve trusted global instruction patterns
This commit is contained in:
Marius
2026-07-13 12:17:59 +00:00
committed by GitHub
parent 6639022345
commit 032f3bb55f
19 changed files with 693 additions and 116 deletions
@@ -91,7 +91,7 @@ describe("ConfigMarkdown: normal template", () => {
})
describe("ConfigMarkdown: frontmatter parsing", async () => {
const parsed = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/frontmatter.md")
const parsed = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/frontmatter.md", { trusted: true }) // kilocode_change
test("should parse without throwing", () => {
expect(parsed).toBeDefined()
@@ -172,7 +172,7 @@ describe("ConfigMarkdown: frontmatter parsing", async () => {
})
describe("ConfigMarkdown: frontmatter parsing w/ empty frontmatter", async () => {
const result = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/empty-frontmatter.md")
const result = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/empty-frontmatter.md", { trusted: true }) // kilocode_change
test("should parse without throwing", () => {
expect(result).toBeDefined()
@@ -182,7 +182,7 @@ describe("ConfigMarkdown: frontmatter parsing w/ empty frontmatter", async () =>
})
describe("ConfigMarkdown: frontmatter parsing w/ no frontmatter", async () => {
const result = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/no-frontmatter.md")
const result = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/no-frontmatter.md", { trusted: true }) // kilocode_change
test("should parse without throwing", () => {
expect(result).toBeDefined()
@@ -192,7 +192,7 @@ describe("ConfigMarkdown: frontmatter parsing w/ no frontmatter", async () => {
})
describe("ConfigMarkdown: frontmatter parsing w/ Markdown header", async () => {
const result = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/markdown-header.md")
const result = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/markdown-header.md", { trusted: true }) // kilocode_change
test("should parse and match", () => {
expect(result).toBeDefined()
@@ -212,7 +212,7 @@ Always structure your responses using clear markdown formatting:
})
describe("ConfigMarkdown: frontmatter has weird model id", async () => {
const result = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/weird-model-id.md")
const result = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/weird-model-id.md", { trusted: true }) // kilocode_change
test("should parse and match", () => {
expect(result).toBeDefined()
@@ -1,4 +1,5 @@
import { afterEach, describe, expect, test } from "bun:test"
import fs from "node:fs/promises"
import path from "path"
import { Config } from "../../src/config/config"
import { AppRuntime } from "../../src/effect/app-runtime"
@@ -15,6 +16,96 @@ afterEach(async () => {
})
describe("config resilience", () => {
test("retains untrusted provenance for external markdown paths selected by project config", async () => {
await using tmp = await tmpdir({
init: async (dir) => {
const project = path.join(dir, "project")
const instruction = path.join(dir, "external.md")
await Filesystem.write(
path.join(project, "kilo.json"),
JSON.stringify({ instructions: [instruction], skills: { paths: ["../external-skills"] } }),
)
await Filesystem.write(instruction, "external")
await Filesystem.write(path.join(dir, "external-skills", "SKILL.md"), "external")
return { project, instruction }
},
})
await provideTestInstance({
directory: tmp.extra.project,
fn: async () => {
const cfg = await load()
expect(cfg.instruction_origins?.[tmp.extra.instruction]).toMatchObject({
trusted: false,
root: tmp.extra.project,
})
expect(cfg.skill_path_origins?.["../external-skills"]).toMatchObject({
trusted: false,
root: tmp.extra.project,
})
},
})
})
test("skips project markdown that references environment or out-of-project files", async () => {
const name = "KILO_CONFIG_MARKDOWN_PROJECT_SECRET"
const prior = process.env[name]
process.env[name] = "environment secret"
try {
await using tmp = await tmpdir({
init: async (dir) => {
const project = path.join(dir, "project")
const secret = path.join(dir, "secret.txt")
const prompt = [`{file:${secret}}`, `{env:${name}}`].join("\n")
await Filesystem.write(path.join(project, ".kilo", "agent", "unsafe.md"), prompt)
await Filesystem.write(path.join(project, ".kilo", "command", "unsafe.md"), prompt)
await Filesystem.write(secret, "file secret")
return project
},
})
await provideTestInstance({
directory: tmp.extra,
fn: async () => {
const cfg = await load()
const warns = await warnings()
expect(cfg.agent?.unsafe).toBeUndefined()
expect(cfg.command?.unsafe).toBeUndefined()
expect(warns.filter((warning) => warning.path.endsWith("unsafe.md"))).toHaveLength(2)
},
})
} finally {
if (prior === undefined) delete process.env[name]
else process.env[name] = prior
}
})
test("skips project markdown symlinks that escape the project root", async () => {
await using tmp = await tmpdir({
init: async (dir) => {
const project = path.join(dir, "project")
const item = path.join(project, ".kilo", "agent", "unsafe.md")
const secret = path.join(dir, "secret.md")
await Filesystem.write(secret, "file secret")
await fs.mkdir(path.dirname(item), { recursive: true })
await fs.symlink(secret, item)
return project
},
})
await provideTestInstance({
directory: tmp.extra,
fn: async () => {
const cfg = await load()
const warns = await warnings()
expect(cfg.agent?.unsafe).toBeUndefined()
expect(warns.some((warning) => warning.path.endsWith("unsafe.md"))).toBe(true)
},
})
})
test("skips invalid agent markdown configs", async () => {
await using tmp = await tmpdir({
init: async (dir) => {
@@ -105,7 +105,7 @@ describe("markdown substitutions", () => {
},
})
const md = await ConfigMarkdown.parse(path.join(tmp.path, "SKILL.md"))
const md = await ConfigMarkdown.parse(path.join(tmp.path, "SKILL.md"), { trusted: true })
expect(md.content).toContain("file content")
expect(md.content).toContain("env content")
@@ -0,0 +1,59 @@
import path from "node:path"
import { expect, test } from "bun:test"
import { KilocodeMarkdown } from "@/kilocode/config/markdown"
import { tmpdir } from "../../fixture/fixture"
test("confines project markdown substitutions while preserving trusted substitutions", async () => {
const name = "KILO_MARKDOWN_SUBSTITUTE_TEST_SECRET"
const prior = process.env[name]
process.env[name] = "environment secret"
try {
await using tmp = await tmpdir({
init: async (dir) => {
const project = path.join(dir, "project")
const item = path.join(project, ".kilo", "agents", "unsafe.md")
const global = path.join(dir, "global", "agents", "trusted.md")
const secret = path.join(dir, "secret.txt")
const file = `{file:${secret}}`
const env = `{env:${name}}`
const text = [file, env].join("\n")
await Bun.write(item, text)
await Bun.write(global, text)
await Bun.write(secret, "file secret")
await Bun.write(path.join(project, "allowed.txt"), "project content")
return { project, item, global, file, env, text }
},
})
const file = await KilocodeMarkdown.substitute(tmp.extra.file, tmp.extra.item, {
trusted: false,
fileScope: { root: tmp.extra.project, source: tmp.extra.item },
}).then(
() => false,
() => true,
)
expect(file).toBe(true)
const env = await KilocodeMarkdown.substitute(tmp.extra.env, tmp.extra.item, {
trusted: false,
fileScope: { root: tmp.extra.project, source: tmp.extra.item },
}).then(
() => false,
() => true,
)
expect(env).toBe(true)
expect(
await KilocodeMarkdown.substitute("{file:../../allowed.txt}", tmp.extra.item, {
trusted: false,
fileScope: { root: tmp.extra.project, source: tmp.extra.item },
}),
).toBe("project content")
const trusted = await KilocodeMarkdown.substitute(tmp.extra.text, tmp.extra.global, { trusted: true })
expect(trusted).toContain("file secret")
expect(trusted).toContain("environment secret")
} finally {
if (prior === undefined) delete process.env[name]
else process.env[name] = prior
}
})
@@ -10,7 +10,7 @@ import { Reference } from "../../../src/reference/reference"
import { Instruction } from "../../../src/session/instruction"
import { MessageID } from "../../../src/session/schema"
import { Global } from "@opencode-ai/core/global"
import { provideTmpdirInstance } from "../../fixture/fixture"
import { provideInstance, provideTmpdirInstance, tmpdirScoped } from "../../fixture/fixture"
import { testEffect } from "../../lib/effect"
import { TestConfig } from "../../fixture/config"
@@ -27,9 +27,9 @@ const it = testEffect(
const configLayer = TestConfig.layer()
const layer = (dir: string) =>
const layer = (dir: string, config = configLayer) =>
Instruction.layer.pipe(
Layer.provide(configLayer),
Layer.provide(config),
Layer.provide(AppFileSystem.defaultLayer),
Layer.provide(FetchHttpClient.layer),
Layer.provide(Global.layerWith({ home: dir, config: dir })),
@@ -43,15 +43,146 @@ const write = (filepath: string, content: string) =>
})
describe("instruction markdown substitutions", () => {
it.live("applies file and env substitutions to nearby AGENTS.md", () =>
it.live("preserves trusted relative instructions when project config is disabled", () =>
Effect.acquireUseRelease(
Effect.sync(() => {
const prior = {
flag: process.env.KILO_DISABLE_PROJECT_CONFIG,
secret: process.env.KILO_INSTRUCTION_GLOBAL_PATTERN_SECRET,
}
process.env.KILO_DISABLE_PROJECT_CONFIG = "1"
process.env.KILO_INSTRUCTION_GLOBAL_PATTERN_SECRET = "environment secret"
return prior
}),
() =>
Effect.gen(function* () {
const dir = yield* tmpdirScoped()
const project = path.join(dir, "project")
const home = path.join(dir, "global")
yield* write(path.join(project, "README.md"), "project")
yield* write(
path.join(home, "rules", "trusted.md"),
"{env:KILO_INSTRUCTION_GLOBAL_PATTERN_SECRET}",
)
const config = TestConfig.layer({
get: () =>
Effect.succeed({
instructions: ["rules/*.md"],
instruction_origins: { "rules/*.md": { trusted: true, source: "global config" } },
}),
})
yield* provideInstance(project)(
Effect.gen(function* () {
const svc = yield* Instruction.Service
const results = yield* svc.system()
expect(results.join("\n")).toContain("environment secret")
}).pipe(Effect.provide(layer(home, config))),
)
}),
(prior) =>
Effect.sync(() => {
if (prior.flag === undefined) delete process.env.KILO_DISABLE_PROJECT_CONFIG
else process.env.KILO_DISABLE_PROJECT_CONFIG = prior.flag
if (prior.secret === undefined) delete process.env.KILO_INSTRUCTION_GLOBAL_PATTERN_SECRET
else process.env.KILO_INSTRUCTION_GLOBAL_PATTERN_SECRET = prior.secret
}),
),
)
it.live("does not trust project markdown selected by a trusted relative instruction", () =>
provideTmpdirInstance((dir) => {
const config = TestConfig.layer({
get: () =>
Effect.succeed({
instructions: ["AGENTS.md"],
instruction_origins: { "AGENTS.md": { trusted: true, source: "global config" } },
}),
})
return Effect.gen(function* () {
const name = "KILO_INSTRUCTION_RELATIVE_SECRET"
process.env[name] = "environment secret"
yield* write(path.join(dir, "AGENTS.md"), `{env:${name}}`)
const svc = yield* Instruction.Service
const results = yield* svc.system()
expect(results.join("\n")).not.toContain("environment secret")
delete process.env[name]
}).pipe(Effect.provide(layer(path.join(dir, "global"), config)))
}),
)
it.live("does not trust a global-path instruction selected by project config", () =>
Effect.gen(function* () {
const dir = yield* tmpdirScoped()
const project = path.join(dir, "project")
const home = path.join(dir, "global")
const item = path.join(home, "private.md")
const secret = path.join(dir, "secret.txt")
const name = "KILO_INSTRUCTION_SELECTED_SECRET"
process.env[name] = "environment secret"
yield* write(path.join(project, "README.md"), "project")
yield* write(secret, "file secret")
yield* write(item, [`{file:${secret}}`, `{env:${name}}`].join("\n"))
const config = TestConfig.layer({
get: () =>
Effect.succeed({
instructions: [item],
instruction_origins: {
[item]: { trusted: false, source: path.join(project, "kilo.json"), root: project },
},
}),
})
yield* provideInstance(project)(
Effect.gen(function* () {
const svc = yield* Instruction.Service
const results = yield* svc.system()
expect(results.join("\n")).not.toContain("file secret")
expect(results.join("\n")).not.toContain("environment secret")
}).pipe(Effect.provide(layer(home, config))),
)
delete process.env[name]
}),
)
it.live("trusts a global-path instruction declared by trusted config", () =>
Effect.gen(function* () {
const dir = yield* tmpdirScoped()
const project = path.join(dir, "project")
const home = path.join(dir, "global")
const item = path.join(home, "private.md")
const secret = path.join(dir, "secret.txt")
const name = "KILO_INSTRUCTION_TRUSTED_SECRET"
process.env[name] = "environment secret"
yield* write(path.join(project, "README.md"), "project")
yield* write(secret, "file secret")
yield* write(item, [`{file:${secret}}`, `{env:${name}}`].join("\n"))
const config = TestConfig.layer({
get: () =>
Effect.succeed({
instructions: [item],
instruction_origins: { [item]: { trusted: true, source: "global config" } },
}),
})
yield* provideInstance(project)(
Effect.gen(function* () {
const svc = yield* Instruction.Service
const results = yield* svc.system()
expect(results.join("\n")).toContain("file secret")
expect(results.join("\n")).toContain("environment secret")
}).pipe(Effect.provide(layer(home, config))),
)
delete process.env[name]
}),
)
it.live("applies in-project file substitutions to nearby AGENTS.md", () =>
provideTmpdirInstance((dir) =>
Effect.gen(function* () {
process.env.KILO_INSTRUCTION_TEST = "env content"
yield* write(path.join(dir, "subdir", "guide.md"), "file content")
yield* write(
path.join(dir, "subdir", "AGENTS.md"),
["# Instructions", "", "{file:guide.md}", "{env:KILO_INSTRUCTION_TEST}"].join("\n"),
)
yield* write(path.join(dir, "subdir", "AGENTS.md"), ["# Instructions", "", "{file:guide.md}"].join("\n"))
yield* write(path.join(dir, "subdir", "nested", "file.ts"), "const value = 1")
const svc = yield* Instruction.Service
@@ -59,11 +190,44 @@ describe("instruction markdown substitutions", () => {
expect(results).toHaveLength(1)
expect(results[0].content).toContain("file content")
expect(results[0].content).toContain("env content")
expect(results[0].content).not.toContain("{file:")
expect(results[0].content).not.toContain("{env:")
delete process.env.KILO_INSTRUCTION_TEST
}).pipe(Effect.provide(layer(dir))),
}).pipe(Effect.provide(layer(path.join(dir, "global")))),
),
)
it.live("omits nearby project instructions with environment substitutions", () =>
provideTmpdirInstance((dir) =>
Effect.gen(function* () {
const name = "KILO_INSTRUCTION_PROJECT_SECRET"
process.env[name] = "environment secret"
yield* write(path.join(dir, "subdir", "AGENTS.md"), `{env:${name}}`)
yield* write(path.join(dir, "subdir", "nested", "file.ts"), "const value = 1")
const svc = yield* Instruction.Service
const results = yield* svc.resolve([], path.join(dir, "subdir", "nested", "file.ts"), MessageID.ascending())
expect(results).toEqual([])
delete process.env[name]
}).pipe(Effect.provide(layer(path.join(dir, "global")))),
),
)
it.live("preserves substitutions in trusted global instructions", () =>
provideTmpdirInstance((dir) =>
Effect.gen(function* () {
const name = "KILO_INSTRUCTION_GLOBAL_SECRET"
process.env[name] = "environment secret"
const home = path.join(dir, "global")
yield* write(path.join(home, "guide.md"), "file secret")
yield* write(path.join(home, "AGENTS.md"), [`{file:guide.md}`, `{env:${name}}`].join("\n"))
const svc = yield* Instruction.Service
const results = yield* svc.system()
expect(results.join("\n")).toContain("file secret")
expect(results.join("\n")).toContain("environment secret")
delete process.env[name]
}).pipe(Effect.provide(layer(path.join(dir, "global")))),
),
)
})
@@ -137,26 +137,75 @@ Actual description here.`
).toBe(true)
})
test("applies markdown substitutions to workflow content", async () => {
process.env.KILO_WORKFLOW_TEST = "env content"
test("applies in-project file substitutions to project workflow content", async () => {
await using tmp = await tmpdir({
init: async (dir) => {
const workflowsDir = path.join(dir, ".kilo", "workflows")
await Bun.write(path.join(dir, "guide.md"), "file content")
await Bun.write(
path.join(workflowsDir, "workflow.md"),
["# Workflow", "", "{file:../../guide.md}", "{env:KILO_WORKFLOW_TEST}"].join("\n"),
["# Workflow", "", "{file:../../guide.md}"].join("\n"),
)
},
})
try {
const workflows = await WorkflowsMigrator.discoverWorkflows(tmp.path, true)
const workflows = await WorkflowsMigrator.discoverWorkflows(tmp.path, true)
expect(workflows[0].content).toContain("file content")
expect(workflows[0].content).toContain("env content")
expect(workflows[0].content).toContain("file content")
})
test("skips environment substitutions in project workflows", async () => {
const name = "KILO_WORKFLOW_PROJECT_SECRET"
const prior = process.env[name]
process.env[name] = "environment secret"
try {
await using tmp = await tmpdir({
init: async (dir) => {
await Bun.write(path.join(dir, ".kilo", "workflows", "workflow.md"), `{env:${name}}`)
await Bun.write(path.join(dir, ".kilo", "workflows", "safe.md"), "safe workflow")
},
})
const warnings: string[] = []
const workflows = await WorkflowsMigrator.discoverWorkflows(tmp.path, true, warnings)
expect(workflows.map((item) => item.name)).toEqual(["safe"])
expect(
warnings.some((warning) => warning.includes("workflow") && warning.includes("environment references")),
).toBe(true)
} finally {
delete process.env.KILO_WORKFLOW_TEST
if (prior === undefined) delete process.env[name]
else process.env[name] = prior
}
})
test("preserves file and environment substitutions in trusted global workflows", async () => {
const name = "KILO_WORKFLOW_GLOBAL_SECRET"
const prior = process.env[name]
process.env[name] = "environment secret"
try {
await using tmp = await tmpdir({
init: async (dir) => {
await Bun.write(path.join(dir, "secret.txt"), "file secret")
await Bun.write(
path.join(dir, ".kilo", "workflows", "trusted.md"),
[`{file:../../secret.txt}`, `{env:${name}}`].join("\n"),
)
await Bun.write(path.join(dir, "project", "README.md"), "project")
await Bun.write(path.join(dir, "project", ".kilo", "workflows", "trusted.md"), `{env:${name}}`)
},
})
const warnings: string[] = []
const workflows = await withHome(tmp.path, () =>
WorkflowsMigrator.discoverWorkflows(path.join(tmp.path, "project"), false, warnings),
)
const workflow = workflows.find((item) => item.source === "global" && item.name === "trusted")
expect(workflow?.content).toContain("file secret")
expect(workflow?.content).toContain("environment secret")
expect(warnings.some((warning) => warning.includes("trusted"))).toBe(true)
} finally {
if (prior === undefined) delete process.env[name]
else process.env[name] = prior
}
})
})