From 032f3bb55f85ce2b2cc07cea54edf59b23abfcc4 Mon Sep 17 00:00:00 2001 From: Marius Date: Mon, 13 Jul 2026 14:17:59 +0200 Subject: [PATCH] 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 --- .changeset/guard-markdown-substitutions.md | 5 + packages/opencode/src/config/agent.ts | 24 ++- packages/opencode/src/config/command.ts | 13 +- packages/opencode/src/config/config.ts | 70 ++++++- packages/opencode/src/config/markdown.ts | 12 +- .../src/kilocode/config-validation.ts | 8 +- .../opencode/src/kilocode/config/markdown.ts | 62 +++--- .../opencode/src/kilocode/config/overlay.ts | 4 +- .../src/kilocode/session/instruction.ts | 8 +- packages/opencode/src/kilocode/tui/config.ts | 2 + .../src/kilocode/workflows-migrator.ts | 38 +++- packages/opencode/src/session/instruction.ts | 55 ++++- packages/opencode/src/skill/index.ts | 91 +++++++-- .../opencode/test/config/markdown.test.ts | 10 +- .../test/kilocode/config-resilience.test.ts | 91 +++++++++ .../test/kilocode/config/config.test.ts | 2 +- .../test/kilocode/config/markdown.test.ts | 59 ++++++ .../session/instruction-substitution.test.ts | 190 ++++++++++++++++-- .../test/kilocode/workflows-migrator.test.ts | 65 +++++- 19 files changed, 693 insertions(+), 116 deletions(-) create mode 100644 .changeset/guard-markdown-substitutions.md create mode 100644 packages/opencode/test/kilocode/config/markdown.test.ts diff --git a/.changeset/guard-markdown-substitutions.md b/.changeset/guard-markdown-substitutions.md new file mode 100644 index 0000000000..e30ef003c3 --- /dev/null +++ b/.changeset/guard-markdown-substitutions.md @@ -0,0 +1,5 @@ +--- +"@kilocode/cli": patch +--- + +Block environment and out-of-project file substitutions in project markdown configuration. diff --git a/packages/opencode/src/config/agent.ts b/packages/opencode/src/config/agent.ts index f8efe7f932..b0bc8ad35f 100644 --- a/packages/opencode/src/config/agent.ts +++ b/packages/opencode/src/config/agent.ts @@ -135,7 +135,13 @@ export const Info = AgentSchema.pipe( export type Info = Schema.Schema.Type // kilocode_change start - trusted gates {env:}; fileScope confines untrusted agent prompt {file:} reads -export async function load(dir: string, warnings?: Warning[], trusted?: boolean, fileScope?: ConfigVariable.FileScope) { +export async function load( + dir: string, + warnings?: Warning[], + trusted = false, + fileScope?: ConfigVariable.FileScope, + sourceScope?: ConfigVariable.FileScope, +) { // kilocode_change end const result: Record = {} for (const item of await Glob.scan("{agent,agents}/**/*.md", { @@ -144,7 +150,9 @@ export async function load(dir: string, warnings?: Warning[], trusted?: boolean, dot: true, symlink: true, })) { - const md = await ConfigMarkdown.parse(item).catch(async (err) => { + // kilocode_change start + const md = await ConfigMarkdown.parse(item, { trusted, fileScope, sourceScope }).catch(async (err) => { + // kilocode_change end const message = ConfigMarkdown.FrontmatterError.isInstance(err) ? err.data.message : `Failed to parse agent ${item}` @@ -211,7 +219,13 @@ export async function load(dir: string, warnings?: Warning[], trusted?: boolean, } // kilocode_change start -export async function loadMode(dir: string, warnings?: Warning[]) { +export async function loadMode( + dir: string, + warnings?: Warning[], + trusted = false, + fileScope?: ConfigVariable.FileScope, + sourceScope?: ConfigVariable.FileScope, +) { // kilocode_change end const result: Record = {} for (const item of await Glob.scan("{mode,modes}/*.md", { @@ -220,7 +234,9 @@ export async function loadMode(dir: string, warnings?: Warning[]) { dot: true, symlink: true, })) { - const md = await ConfigMarkdown.parse(item).catch(async (err) => { + // kilocode_change start + const md = await ConfigMarkdown.parse(item, { trusted, fileScope, sourceScope }).catch(async (err) => { + // kilocode_change end const message = ConfigMarkdown.FrontmatterError.isInstance(err) ? err.data.message : `Failed to parse mode ${item}` diff --git a/packages/opencode/src/config/command.ts b/packages/opencode/src/config/command.ts index a25a70e49f..c735580b7b 100644 --- a/packages/opencode/src/config/command.ts +++ b/packages/opencode/src/config/command.ts @@ -12,6 +12,7 @@ import { Bus } from "@/bus" import { NamedError } from "@opencode-ai/core/util/error" import { KilocodeConfig } from "@/kilocode/config/config" import type { Warning } from "./config" +import type { ConfigVariable } from "./variable" // kilocode_change end const log = Log.create({ service: "config" }) @@ -29,7 +30,13 @@ export type Info = Schema.Schema.Type const decodeInfo = Schema.decodeUnknownExit(Info) // kilocode_change start -export async function load(dir: string, warnings?: Warning[]) { +export async function load( + dir: string, + warnings?: Warning[], + trusted = false, + fileScope?: ConfigVariable.FileScope, + sourceScope?: ConfigVariable.FileScope, +) { // kilocode_change end const result: Record = {} for (const item of await Glob.scan("{command,commands}/**/*.md", { @@ -38,7 +45,9 @@ export async function load(dir: string, warnings?: Warning[]) { dot: true, symlink: true, })) { - const md = await ConfigMarkdown.parse(item).catch(async (err) => { + // kilocode_change start + const md = await ConfigMarkdown.parse(item, { trusted, fileScope, sourceScope }).catch(async (err) => { + // kilocode_change end const message = ConfigMarkdown.FrontmatterError.isInstance(err) ? err.data.message : `Failed to parse command ${item}` diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index cac0ef6a79..1d4405791a 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -55,6 +55,7 @@ import { Git } from "@/git" import { KilocodeDefaultPlugins } from "@/kilocode/config/default-plugins" import { KilocodeGlobalConfigStamp } from "@/kilocode/config/global-stamp" import { SandboxConfig } from "@/kilocode/sandbox/config" +import type { KilocodeMarkdown } from "@/kilocode/config/markdown" import { IndexingConfig as KiloIndexingConfig, IndexingSchema as KiloIndexingSchema, @@ -444,6 +445,10 @@ export type Info = DeepMutable> & { // plugin_origins is derived state, not a persisted config field. It keeps each winning plugin spec together // with the file and scope it came from so later runtime code can make location-sensitive decisions. plugin_origins?: ConfigPlugin.Origin[] + // kilocode_change start - derived provenance for markdown paths selected by config + instruction_origins?: Record + skill_path_origins?: Record + // kilocode_change end } type State = { @@ -523,7 +528,14 @@ function patchJsonc(input: string, patch: unknown, path: string[] = []): string } function writable(info: Info) { - const { plugin_origins: _plugin_origins, ...next } = info + // kilocode_change start - derived provenance is runtime-only and must never be persisted + const { + plugin_origins: _plugin_origins, + instruction_origins: _instruction_origins, + skill_path_origins: _skill_path_origins, + ...next + } = info + // kilocode_change end return next } @@ -731,6 +743,14 @@ export const layer = Layer.effect( }), ) result = mergeConfigConcatArrays(result, legacy.config) + // Legacy rules are discovered from fixed global/project directories, so their paths safely identify the + // source boundary even though the migrator returns them as one merged instruction list. + result.instruction_origins = Object.fromEntries( + (legacy.config.instructions ?? []).map((item) => { + const trusted = !containsPath(item, ctx) + return [item, { trusted, source: item, root: trusted ? undefined : projectRoot }] + }), + ) warnings.push(...legacy.warnings) const orgModes = yield* Effect.promise(() => KilocodeConfig.loadOrganizationModes(auth)) @@ -773,10 +793,36 @@ export const layer = Layer.effect( }) // kilocode_change start - const merge = Effect.fnUntraced(function* (source: string, next: Info, kind?: ConfigPlugin.Scope) { + const origins = ( + prev: Record | undefined, + values: readonly string[], + trusted: boolean, + source: string, + ) => { + const result = { ...prev } + for (const value of values) { + if (result[value]?.trusted) continue + result[value] = { trusted, source, root: trusted ? undefined : projectRoot } + } + return result + } + + const merge = Effect.fnUntraced(function* ( + source: string, + next: Info, + kind?: ConfigPlugin.Scope, + sourceTrusted?: boolean, + ) { const scope = kind ?? (yield* pluginScopeForSource(source)) + const trusted = sourceTrusted ?? scope === "global" const scoped = KilocodeConfig.scopeIndexing(SandboxConfig.scope(next, scope), scope) result = mergeConfigConcatArrays(result, scoped) + if (next.instructions?.length) { + result.instruction_origins = origins(result.instruction_origins, next.instructions, trusted, source) + } + if (next.skills?.paths?.length) { + result.skill_path_origins = origins(result.skill_path_origins, next.skills.paths, trusted, source) + } return yield* mergePluginOrigins(source, scoped.plugin, scope) }) // kilocode_change end @@ -861,6 +907,8 @@ export const layer = Layer.effect( return Effect.succeed({} as Info) }), ), + undefined, + true, ) // kilocode_change end log.debug("loaded custom config", { path: Flag.KILO_CONFIG }) @@ -911,9 +959,12 @@ export const layer = Layer.effect( const scope = primarySet.has(dir) ? "local" : undefined // kilocode_change - trust {file:}/{env:} only for global-scoped config dirs, never project ones const dirScope = scope ?? (yield* pluginScopeForSource(dir)) - const dirTrusted = dirScope === "global" + const dirTrusted = dir === Flag.KILO_CONFIG_DIR || dirScope === "global" // kilocode_change - untrusted config dirs confine {file:} reads to projectRoot const dirFileScope = dirTrusted ? undefined : { root: projectRoot, source: dir } + const dirSourceScope = dirTrusted + ? undefined + : { root: primarySet.has(dir) ? path.dirname(dir) : projectRoot, source: dir } if (KilocodeConfig.isConfigDir(dir, Flag.KILO_CONFIG_DIR)) { for (const file of KilocodeConfig.ALL_CONFIG_FILES) { const source = path.join(dir, file) @@ -929,7 +980,8 @@ export const layer = Layer.effect( return Effect.succeed({} as Info) }), ), - dirScope, // kilocode_change + dirScope, + dirTrusted, ) result.agent ??= {} result.mode ??= {} @@ -966,13 +1018,16 @@ export const layer = Layer.effect( // kilocode_change start - propagate parse errors to the Warning accumulator result.command = mergeDeep( result.command ?? {}, - yield* Effect.promise(() => ConfigCommand.load(dir, warnings)), + yield* Effect.promise(() => ConfigCommand.load(dir, warnings, dirTrusted, dirFileScope, dirSourceScope)), ) result.agent = mergeDeep( result.agent ?? {}, - yield* Effect.promise(() => ConfigAgent.load(dir, warnings, dirTrusted, dirFileScope)), // kilocode_change + yield* Effect.promise(() => ConfigAgent.load(dir, warnings, dirTrusted, dirFileScope, dirSourceScope)), + ) + result.agent = mergeDeep( + result.agent ?? {}, + yield* Effect.promise(() => ConfigAgent.loadMode(dir, warnings, dirTrusted, dirFileScope, dirSourceScope)), ) - result.agent = mergeDeep(result.agent ?? {}, yield* Effect.promise(() => ConfigAgent.loadMode(dir, warnings))) // kilocode_change end // kilocode_change - Auto-discovered plugins under config directories are already local files, so ConfigPlugin.load // returns normalized Specs and we only need to attach origin metadata here. @@ -1001,6 +1056,7 @@ export const layer = Layer.effect( }), ), "local", + true, ) // kilocode_change end } diff --git a/packages/opencode/src/config/markdown.ts b/packages/opencode/src/config/markdown.ts index 02081e6bbd..a04576baab 100644 --- a/packages/opencode/src/config/markdown.ts +++ b/packages/opencode/src/config/markdown.ts @@ -68,18 +68,22 @@ export function fallbackSanitization(content: string): string { return content.replace(frontmatter, () => processed) } -export async function parse(filePath: string) { - const template = await Filesystem.readText(filePath) +// kilocode_change start - accept source trust and confine untrusted markdown source reads +export async function parse(filePath: string, options: KilocodeMarkdown.Options) { + const template = options.trusted + ? await Filesystem.readText(filePath) + : await KilocodeMarkdown.read(filePath, options) + // kilocode_change end // kilocode_change start - substitute content and retry invalid frontmatter with permissive sanitization try { const md = matter(template) - md.content = await KilocodeMarkdown.substitute(md.content, filePath) // kilocode_change + md.content = await KilocodeMarkdown.substitute(md.content, filePath, options) // kilocode_change return md } catch { try { const md = matter(fallbackSanitization(template)) - md.content = await KilocodeMarkdown.substitute(md.content, filePath) // kilocode_change + md.content = await KilocodeMarkdown.substitute(md.content, filePath, options) // kilocode_change return md } catch (err) { throw new FrontmatterError( diff --git a/packages/opencode/src/kilocode/config-validation.ts b/packages/opencode/src/kilocode/config-validation.ts index 681c043f04..384c91f565 100644 --- a/packages/opencode/src/kilocode/config-validation.ts +++ b/packages/opencode/src/kilocode/config-validation.ts @@ -73,7 +73,13 @@ export namespace ConfigValidation { let md: Awaited> try { - md = await ConfigMarkdown.parse(filepath) + const trusted = path.isAbsolute(filepath) && ConfigProtection.isAbsolute(filepath) + const ctx = Instance.current + const root = ctx.worktree === "/" ? ctx.directory : ctx.worktree + md = await ConfigMarkdown.parse(filepath, { + trusted, + fileScope: trusted ? undefined : { root, source: filepath }, + }) // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (e: any) { const msg = ConfigMarkdown.FrontmatterError.isInstance(e) diff --git a/packages/opencode/src/kilocode/config/markdown.ts b/packages/opencode/src/kilocode/config/markdown.ts index 2b8147a766..eac361e126 100644 --- a/packages/opencode/src/kilocode/config/markdown.ts +++ b/packages/opencode/src/kilocode/config/markdown.ts @@ -1,38 +1,42 @@ -import os from "os" -import path from "path" +import { ConfigVariable } from "@/config/variable" +import { InvalidError } from "@/config/error" import { Filesystem } from "@/util/filesystem" +import { ConfigVariableGuard } from "./variable" export namespace KilocodeMarkdown { - function ref(token: string) { - const file = token.replace(/^\{file:/, "").replace(/\}$/, "") - if (file.startsWith("~/")) return path.join(os.homedir(), file.slice(2)) - return file + export type Source = { + trusted: boolean + source: string + root?: string } - export async function substitute(text: string, item: string) { - const body = text.replace(/\{env:([^}]+)\}/g, (_, name) => process.env[name] || "") - const matches = Array.from(body.matchAll(/\{file:[^}]+\}/g)) - if (!matches.length) return body + export type Options = { + trusted: boolean + fileScope?: ConfigVariable.FileScope + sourceScope?: ConfigVariable.FileScope + } - const dir = path.dirname(item) - const chunks = await Promise.all( - matches.map(async (match, i) => { - const token = match[0] - const index = match.index ?? 0 - const prev = matches[i - 1] - const cursor = prev ? (prev.index ?? 0) + prev[0].length : 0 - const head = body.slice(cursor, index) - const start = body.lastIndexOf("\n", index - 1) + 1 - const prefix = body.slice(start, index).trimStart() - if (prefix.startsWith("//")) return head + token + export function read(item: string, options: Options) { + if (options.trusted) return Filesystem.readText(item) + const scope = options.sourceScope ?? options.fileScope + if (!scope) { + throw new InvalidError({ + path: item, + message: "project markdown cannot be read without a project scope", + }) + } + return ConfigVariableGuard.read(item, { ...scope, token: `markdown source "${item}"` }) + } - const file = ref(token) - const target = path.isAbsolute(file) ? file : path.resolve(dir, file) - const content = await Filesystem.readText(target).catch(() => "") - return head + content.trim() - }), - ) - const last = matches.at(-1) - return chunks.join("") + (last ? body.slice((last.index ?? 0) + last[0].length) : "") + export function substitute(text: string, item: string, options: Options) { + return ConfigVariable.substitute({ + text, + type: "path", + path: item, + missing: "empty", + escapeJson: false, + trusted: options.trusted, + fileScope: options.fileScope, + }) } } diff --git a/packages/opencode/src/kilocode/config/overlay.ts b/packages/opencode/src/kilocode/config/overlay.ts index 4d67a66e85..5b02ee9c45 100644 --- a/packages/opencode/src/kilocode/config/overlay.ts +++ b/packages/opencode/src/kilocode/config/overlay.ts @@ -199,8 +199,8 @@ export namespace KilocodeConfigOverlay { if (!dir) return input if (!existsSync(dir)) return withAgents(input, rest, trusted, root) const fileScope = trusted || !root ? undefined : { root, source: dir } - const agent = await ConfigAgent.load(dir, undefined, trusted, fileScope) - const mode = await ConfigAgent.loadMode(dir) + const agent = await ConfigAgent.load(dir, undefined, trusted, fileScope, fileScope) + const mode = await ConfigAgent.loadMode(dir, undefined, trusted, fileScope, fileScope) const next = KilocodeConfig.mergeConfig(KilocodeConfig.mergeConfig(input, { agent }), { agent: mode }) return withAgents(next, rest, trusted, root) } diff --git a/packages/opencode/src/kilocode/session/instruction.ts b/packages/opencode/src/kilocode/session/instruction.ts index cd3ec481a1..202877892c 100644 --- a/packages/opencode/src/kilocode/session/instruction.ts +++ b/packages/opencode/src/kilocode/session/instruction.ts @@ -1,7 +1,11 @@ import { KilocodeMarkdown } from "../config/markdown" export namespace KilocodeInstruction { - export function content(text: string, item: string) { - return KilocodeMarkdown.substitute(text, item) + export function content(text: string, item: string, options: KilocodeMarkdown.Options) { + return KilocodeMarkdown.substitute(text, item, options) + } + + export async function read(item: string, options: KilocodeMarkdown.Options) { + return content(await KilocodeMarkdown.read(item, options), item, options) } } diff --git a/packages/opencode/src/kilocode/tui/config.ts b/packages/opencode/src/kilocode/tui/config.ts index 5d61466649..ddb8f354db 100644 --- a/packages/opencode/src/kilocode/tui/config.ts +++ b/packages/opencode/src/kilocode/tui/config.ts @@ -115,6 +115,8 @@ export namespace KilocodeTuiConfig { function writable(config: Patch | TuiConfig.Info, defaults = true): Editable { const result = { ...config } as Record delete result.plugin_origins + delete result.instruction_origins + delete result.skill_path_origins const keybinds: Record = defaults ? Object.fromEntries(KilocodeKeybinds.list().map((item) => [item.id, item.default])) : {} diff --git a/packages/opencode/src/kilocode/workflows-migrator.ts b/packages/opencode/src/kilocode/workflows-migrator.ts index c8c475c578..df5a5c2a01 100644 --- a/packages/opencode/src/kilocode/workflows-migrator.ts +++ b/packages/opencode/src/kilocode/workflows-migrator.ts @@ -1,8 +1,8 @@ import * as fs from "fs/promises" import * as path from "path" import os from "os" -import type { Config } from "../config/config" import type { ConfigCommand } from "../config/command" +import { InvalidError } from "../config/error" import { Filesystem } from "../util/filesystem" import { KilocodeMarkdown } from "./config/markdown" import { KilocodePaths } from "./paths" @@ -55,12 +55,30 @@ export namespace WorkflowsMigrator { return undefined } - async function loadWorkflowsFromDir(dir: string, source: "global" | "project"): Promise { + async function loadWorkflowsFromDir( + dir: string, + source: "global" | "project", + root?: string, + warnings: string[] = [], + ): Promise { if (!(await Filesystem.isDir(dir))) return [] const files = await findWorkflowFiles(dir) const workflows: KilocodeWorkflow[] = [] for (const file of files) { - const content = await KilocodeMarkdown.substitute(await fs.readFile(file, "utf-8"), file) + const options = { + trusted: source === "global", + fileScope: source === "project" && root ? { root, source: file } : undefined, + } + const content = await KilocodeMarkdown.read(file, options) + .then((text) => KilocodeMarkdown.substitute(text, file, options)) + .catch((err) => { + const message = InvalidError.isInstance(err) ? err.data.message : undefined + warnings.push( + `Skipped workflow '${extractNameFromFilename(file)}': ${message ?? (err instanceof Error ? err.message : String(err))}`, + ) + return undefined + }) + if (content === undefined) continue workflows.push({ name: extractNameFromFilename(file), path: file, @@ -71,23 +89,27 @@ export namespace WorkflowsMigrator { return workflows } - export async function discoverWorkflows(projectDir: string, skipGlobalPaths?: boolean): Promise { + export async function discoverWorkflows( + projectDir: string, + skipGlobalPaths?: boolean, + warnings: string[] = [], + ): Promise { const workflows: KilocodeWorkflow[] = [] if (!skipGlobalPaths) { // 1. VSCode extension global storage (primary location for global workflows) const vscodeWorkflowsDir = path.join(KilocodePaths.vscodeGlobalStorage(), "workflows") - workflows.push(...(await loadWorkflowsFromDir(vscodeWorkflowsDir, "global"))) + workflows.push(...(await loadWorkflowsFromDir(vscodeWorkflowsDir, "global", undefined, warnings))) // 2. Home directories ~/.kilocode/workflows and ~/.kilo/workflows for (const dir of globalWorkflowsDirs()) { - workflows.push(...(await loadWorkflowsFromDir(dir, "global"))) + workflows.push(...(await loadWorkflowsFromDir(dir, "global", undefined, warnings))) } } // 3. Project workflows (.kilo/workflows/ and .kilocode/workflows/) for (const dir of KILO_WORKFLOWS_DIRS) { - workflows.push(...(await loadWorkflowsFromDir(path.join(projectDir, dir), "project"))) + workflows.push(...(await loadWorkflowsFromDir(path.join(projectDir, dir), "project", projectDir, warnings))) } return workflows @@ -108,7 +130,7 @@ export namespace WorkflowsMigrator { const warnings: string[] = [] const commands: Record = {} - const workflows = await discoverWorkflows(options.projectDir, options.skipGlobalPaths) + const workflows = await discoverWorkflows(options.projectDir, options.skipGlobalPaths, warnings) // Deduplicate by name (project takes precedence over global) const workflowsByName = new Map() diff --git a/packages/opencode/src/session/instruction.ts b/packages/opencode/src/session/instruction.ts index bc67f7dfbd..c6776ebe1f 100644 --- a/packages/opencode/src/session/instruction.ts +++ b/packages/opencode/src/session/instruction.ts @@ -9,6 +9,7 @@ import { AppFileSystem } from "@opencode-ai/core/filesystem" import { withTransientReadRetry } from "@/util/effect-http-client" import { Global } from "@opencode-ai/core/global" import { KilocodeInstruction } from "@/kilocode/session/instruction" // kilocode_change +import type { KilocodeMarkdown } from "@/kilocode/config/markdown" // kilocode_change import type { MessageV2 } from "./message-v2" import type { MessageID } from "./schema" @@ -91,11 +92,23 @@ export const layer: Layer.Layer< return yield* fs.globUp(instruction, root, root).pipe(Effect.catch(() => Effect.succeed([] as string[]))) // kilocode_change }) - const read = Effect.fnUntraced(function* (filepath: string) { - const content = yield* fs.readFileString(filepath).pipe(Effect.catch(() => Effect.succeed(""))) // kilocode_change - return yield* Effect.promise(() => KilocodeInstruction.content(content, filepath)) // kilocode_change + // kilocode_change start - project instructions cannot read env or files outside the project root + const options = Effect.fnUntraced(function* (filepath: string, origin?: KilocodeMarkdown.Source) { + const ctx = yield* InstanceState.context + const root = ctx.worktree === "/" ? ctx.directory : ctx.worktree + const trusted = origin?.trusted ?? false + return { + trusted, + fileScope: trusted ? undefined : { root: origin?.root ?? root, source: origin?.source ?? filepath }, + } }) + const read = Effect.fnUntraced(function* (filepath: string, origin?: KilocodeMarkdown.Source) { + const opts = yield* options(filepath, origin) + return yield* Effect.promise(() => KilocodeInstruction.read(filepath, opts).catch(() => "")) + }) + // kilocode_change end + const fetch = Effect.fnUntraced(function* (url: string) { const res = yield* http.execute(HttpClientRequest.get(url)).pipe( Effect.timeout(5000), @@ -111,14 +124,21 @@ export const layer: Layer.Layer< s.claims.delete(messageID) }) - const systemPaths = Effect.fn("Instruction.systemPaths")(function* () { + // kilocode_change start - retain declaration provenance through instruction path expansion + const systemSources = Effect.fn("Instruction.systemSources")(function* () { const config = yield* cfg.get() const ctx = yield* InstanceState.context - const paths = new Set() + const root = ctx.worktree === "/" ? ctx.directory : ctx.worktree + const paths = new Map() + const add = (item: string, origin: KilocodeMarkdown.Source) => { + const filepath = path.resolve(item) + if (paths.get(filepath)?.trusted) return + paths.set(filepath, origin) + } for (const file of globalFiles) { if (yield* fs.existsSafe(file)) { - paths.add(path.resolve(file)) + add(file, { trusted: true, source: file }) break } } @@ -130,7 +150,7 @@ export const layer: Layer.Layer< .findUp(file, ctx.directory, ctx.worktree) .pipe(Effect.catch(() => Effect.succeed([]))) if (matches.length > 0) { - matches.forEach((item) => paths.add(path.resolve(item))) + matches.forEach((item) => add(item, { trusted: false, source: item, root })) break } } @@ -149,25 +169,38 @@ export const layer: Layer.Layer< }) : relative(instruction) ).pipe(Effect.catch(() => Effect.succeed([] as string[]))) - matches.forEach((item) => paths.add(path.resolve(item))) + const declared = config.instruction_origins?.[raw] ?? { trusted: false, source: raw, root } + const trusted = declared.trusted && (path.isAbsolute(instruction) || Flag.KILO_DISABLE_PROJECT_CONFIG) + const origin = { ...declared, trusted, root: trusted ? undefined : (declared.root ?? root) } + matches.forEach((item) => add(item, origin)) } } return paths }) + const systemPaths = Effect.fn("Instruction.systemPaths")(function* () { + return new Set((yield* systemSources()).keys()) + }) + // kilocode_change end + const system = Effect.fn("Instruction.system")(function* () { const config = yield* cfg.get() - const paths = yield* systemPaths() + const sources = yield* systemSources() // kilocode_change + const paths = Array.from(sources.keys()) // kilocode_change const urls = (config.instructions ?? []).filter( (item) => item.startsWith("https://") || item.startsWith("http://"), ) - const files = yield* Effect.forEach(Array.from(paths), read, { concurrency: 8 }) + // kilocode_change start + const files = yield* Effect.forEach(Array.from(sources.entries()), (item) => read(item[0], item[1]), { + concurrency: 8, + }) + // kilocode_change end const remote = yield* Effect.forEach(urls, fetch, { concurrency: 4 }) return [ - ...Array.from(paths).flatMap((item, i) => (files[i] ? [`Instructions from: ${item}\n${files[i]}`] : [])), + ...paths.flatMap((item, i) => (files[i] ? [`Instructions from: ${item}\n${files[i]}`] : [])), // kilocode_change ...urls.flatMap((item, i) => (remote[i] ? [`Instructions from: ${item}\n${remote[i]}`] : [])), ] }) diff --git a/packages/opencode/src/skill/index.ts b/packages/opencode/src/skill/index.ts index b2345e2587..1f2958f18a 100644 --- a/packages/opencode/src/skill/index.ts +++ b/packages/opencode/src/skill/index.ts @@ -18,6 +18,7 @@ import { BUILTIN_SKILLS } from "../kilocode/skills/builtin" // kilocode_change import { primaryPaths } from "../kilocode/primary-worktree" // kilocode_change import { Git } from "@/git" // kilocode_change import { isRecord } from "@/util/record" +import { Flag } from "@opencode-ai/core/flag/flag" // kilocode_change const log = Log.create({ service: "skill" }) const CLAUDE_EXTERNAL_DIR = ".claude" @@ -79,15 +80,24 @@ type State = { dirs: Set } +// kilocode_change start - retain markdown trust provenance through discovery +type Match = { + path: string + trusted: boolean + root?: string + sourceRoot?: string +} + type DiscoveryState = { - matches: string[] + matches: Match[] dirs: string[] } type ScanState = { - matches: Set + matches: Map dirs: Set } +// kilocode_change end export interface Interface { readonly get: (name: string) => Effect.Effect @@ -97,19 +107,29 @@ export interface Interface { readonly available: (agent?: Agent.Info) => Effect.Effect } -const add = Effect.fnUntraced(function* (state: State, match: string, bus: Bus.Interface) { +// kilocode_change start +const add = Effect.fnUntraced(function* (state: State, match: Match, bus: Bus.Interface) { + const source = match.sourceRoot ?? match.root + // kilocode_change end const md = yield* Effect.tryPromise({ - try: () => ConfigMarkdown.parse(match), + // kilocode_change start - project skills cannot read env or files outside the project root + try: () => + ConfigMarkdown.parse(match.path, { + trusted: match.trusted, + fileScope: match.trusted || !match.root ? undefined : { root: match.root, source: match.path }, + sourceScope: match.trusted || !source ? undefined : { root: source, source: match.path }, + }), + // kilocode_change end catch: (err) => err, }).pipe( Effect.catch( Effect.fnUntraced(function* (err) { const message = ConfigMarkdown.FrontmatterError.isInstance(err) ? err.data.message - : `Failed to parse skill ${match}` + : `Failed to parse skill ${match.path}` // kilocode_change const { Session } = yield* Effect.promise(() => import("@/session/session")) yield* bus.publish(Session.Event.Error, { error: new NamedError.Unknown({ message }).toObject() }) - log.error("failed to load skill", { skill: match, err }) + log.error("failed to load skill", { skill: match.path, err }) // kilocode_change return undefined }), ), @@ -123,15 +143,15 @@ const add = Effect.fnUntraced(function* (state: State, match: string, bus: Bus.I log.warn("duplicate skill name", { name: md.data.name, existing: state.skills[md.data.name].location, - duplicate: match, + duplicate: match.path, // kilocode_change }) } - state.dirs.add(path.dirname(match)) + state.dirs.add(path.dirname(match.path)) // kilocode_change state.skills[md.data.name] = { name: md.data.name, description: md.data.description, - location: match, + location: match.path, // kilocode_change content: md.content, } }) @@ -140,7 +160,7 @@ const scan = Effect.fnUntraced(function* ( state: ScanState, root: string, pattern: string, - opts?: { dot?: boolean; scope?: string }, + opts?: { dot?: boolean; scope?: string; trusted?: boolean; root?: string; sourceRoot?: string }, // kilocode_change ) { const matches = yield* Effect.tryPromise({ try: () => @@ -161,7 +181,14 @@ const scan = Effect.fnUntraced(function* ( ) for (const match of matches) { - state.matches.add(match) + // kilocode_change start + state.matches.set(match, { + path: match, + trusted: opts?.trusted ?? false, + root: opts?.root, + sourceRoot: opts?.sourceRoot, + }) + // kilocode_change end state.dirs.add(path.dirname(match)) } }) @@ -176,7 +203,8 @@ const discoverSkills = Effect.fnUntraced(function* ( directory: string, worktree: string, ) { - const state: ScanState = { matches: new Set(), dirs: new Set() } + const state: ScanState = { matches: new Map(), dirs: new Set() } // kilocode_change + const projectRoot = worktree === "/" ? directory : worktree // kilocode_change - project substitution boundary const externalDirs: string[] = [] if (!disableExternalSkills) { @@ -186,24 +214,45 @@ const discoverSkills = Effect.fnUntraced(function* ( for (const dir of externalDirs) { const root = path.join(global.home, dir) if (!(yield* fsys.isDir(root))) continue - yield* scan(state, root, EXTERNAL_SKILL_PATTERN, { dot: true, scope: "global" }) + yield* scan(state, root, EXTERNAL_SKILL_PATTERN, { dot: true, scope: "global", trusted: true }) // kilocode_change } // kilocode_change start const local = yield* fsys .up({ targets: externalDirs, start: directory, stop: worktree }) .pipe(Effect.catch(() => Effect.succeed([] as string[]))) - const upDirs = [...(yield* primaryPaths(directory, worktree, externalDirs)), ...local] + const fallbacks = yield* primaryPaths(directory, worktree, externalDirs) // kilocode_change + const upDirs = [...fallbacks, ...local] // kilocode_change end for (const root of upDirs) { - yield* scan(state, root, EXTERNAL_SKILL_PATTERN, { dot: true, scope: "project" }) + const scope = fallbacks.includes(root) ? path.dirname(root) : projectRoot // kilocode_change + // kilocode_change start + yield* scan(state, root, EXTERNAL_SKILL_PATTERN, { + dot: true, + scope: "project", + root: projectRoot, + sourceRoot: scope, + }) + // kilocode_change end } } const configDirs = yield* config.directories() + const primary = new Set(yield* primaryPaths(directory, worktree, [".kilocode", ".kilo"])) // kilocode_change for (const dir of configDirs) { - yield* scan(state, dir, KILO_SKILL_PATTERN) + // kilocode_change start - global and explicit KILO_CONFIG_DIR skills are trusted; project and primary-checkout + // skills remain confined to the active project boundary. + const rel = path.relative(projectRoot, dir) + const local = primary.has(dir) || rel === "" || (!rel.startsWith("..") && !path.isAbsolute(rel)) + const trusted = dir === Flag.KILO_CONFIG_DIR || !local + const sourceRoot = primary.has(dir) ? path.dirname(dir) : projectRoot + yield* scan(state, dir, KILO_SKILL_PATTERN, { + trusted, + root: trusted ? undefined : projectRoot, + sourceRoot: trusted ? undefined : sourceRoot, + }) + // kilocode_change end } const cfg = yield* config.get() @@ -215,18 +264,22 @@ const discoverSkills = Effect.fnUntraced(function* ( continue } - yield* scan(state, dir, SKILL_PATTERN) + // kilocode_change start - trust follows the config source that declared the path, never the selected path. + const origin = cfg.skill_path_origins?.[item] + const trusted = origin?.trusted === true && path.isAbsolute(expanded) + yield* scan(state, dir, SKILL_PATTERN, { trusted, root: trusted ? undefined : (origin?.root ?? projectRoot) }) + // kilocode_change end } for (const url of cfg.skills?.urls ?? []) { const pulledDirs = yield* discovery.pull(url) for (const dir of pulledDirs) { - yield* scan(state, dir, SKILL_PATTERN) + yield* scan(state, dir, SKILL_PATTERN, { root: dir }) // kilocode_change - downloaded markdown is untrusted } } return { - matches: Array.from(state.matches), + matches: Array.from(state.matches.values()), // kilocode_change dirs: Array.from(state.dirs), } }) diff --git a/packages/opencode/test/config/markdown.test.ts b/packages/opencode/test/config/markdown.test.ts index 90f1a74ddb..762d026691 100644 --- a/packages/opencode/test/config/markdown.test.ts +++ b/packages/opencode/test/config/markdown.test.ts @@ -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() diff --git a/packages/opencode/test/kilocode/config-resilience.test.ts b/packages/opencode/test/kilocode/config-resilience.test.ts index a93de35099..12949e44cf 100644 --- a/packages/opencode/test/kilocode/config-resilience.test.ts +++ b/packages/opencode/test/kilocode/config-resilience.test.ts @@ -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) => { diff --git a/packages/opencode/test/kilocode/config/config.test.ts b/packages/opencode/test/kilocode/config/config.test.ts index 89ee93e5a4..60af98df46 100644 --- a/packages/opencode/test/kilocode/config/config.test.ts +++ b/packages/opencode/test/kilocode/config/config.test.ts @@ -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") diff --git a/packages/opencode/test/kilocode/config/markdown.test.ts b/packages/opencode/test/kilocode/config/markdown.test.ts new file mode 100644 index 0000000000..400487aa39 --- /dev/null +++ b/packages/opencode/test/kilocode/config/markdown.test.ts @@ -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 + } +}) diff --git a/packages/opencode/test/kilocode/session/instruction-substitution.test.ts b/packages/opencode/test/kilocode/session/instruction-substitution.test.ts index d466808967..a49df9b462 100644 --- a/packages/opencode/test/kilocode/session/instruction-substitution.test.ts +++ b/packages/opencode/test/kilocode/session/instruction-substitution.test.ts @@ -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")))), ), ) }) diff --git a/packages/opencode/test/kilocode/workflows-migrator.test.ts b/packages/opencode/test/kilocode/workflows-migrator.test.ts index afb40c3c5e..acac9630b7 100644 --- a/packages/opencode/test/kilocode/workflows-migrator.test.ts +++ b/packages/opencode/test/kilocode/workflows-migrator.test.ts @@ -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 } }) })