diff --git a/.changeset/config-file-substitution-trust.md b/.changeset/config-file-substitution-trust.md index 3ae377de41e..3bbc2bfd358 100644 --- a/.changeset/config-file-substitution-trust.md +++ b/.changeset/config-file-substitution-trust.md @@ -2,4 +2,4 @@ "@kilocode/cli": patch --- -Prevent project config from reading local files or environment variables via `{file:...}` / `{env:...}` references. These references now resolve only in trusted user-owned config (global config, `KILO_CONFIG`, well-known org config), closing a path where a malicious project could exfiltrate local files through a provider API key. +Harden config credential substitution against untrusted project config. Environment references (`{env:VAR}`) now resolve only in trusted config (global config, `KILO_CONFIG`, `KILO_CONFIG_CONTENT`, and org/MDM-managed config); a project-committed `kilo.json` / `opencode.json` can no longer use them. File references (`{file:...}`) still work in project config but are confined to the project root, so absolute paths, `../` traversal, and symlink escapes are rejected. This closes a path where a malicious repository could exfiltrate local secrets to an attacker-controlled `baseURL`. diff --git a/packages/kilo-docs/pages/code-with-ai/agents/custom-models.md b/packages/kilo-docs/pages/code-with-ai/agents/custom-models.md index 103043b6443..3793825dd30 100644 --- a/packages/kilo-docs/pages/code-with-ai/agents/custom-models.md +++ b/packages/kilo-docs/pages/code-with-ai/agents/custom-models.md @@ -385,8 +385,12 @@ You can also set options that apply to all models from a provider: | Option | Type | Description | |---|---|---| -| `apiKey` | `string` | API key (supports `{env:VAR}` syntax) | +| `apiKey` | `string` | API key (supports `{env:VAR}` and `{file:...}` syntax in trusted config — see note below) | | `baseURL` | `string` | Override the provider's base API URL | + +{% callout type="warning" title="{env:} / {file:} only resolve in trusted config" %} +`{env:VAR}` and `{file:...}` references in `apiKey` (or any option) are resolved **only** when the config lives in a trusted location: your global config (`~/.config/kilo`), a config passed via `KILO_CONFIG` / `KILO_CONFIG_CONTENT`, or organization/MDM-managed config. A project-level `kilo.json` / `opencode.json` committed to a repository **cannot** resolve `{env:VAR}` — the reference is ignored and a warning is logged, so a provider configured this way in a repo will not authenticate. This prevents a malicious repository from exfiltrating your secrets to an attacker-controlled `baseURL` just by being opened. `{file:...}` still works in project config, but only for files that stay inside the project root (absolute paths, `../` traversal, and symlink escapes are rejected). Keep provider credentials in your global config. +{% /callout %} | `timeout` | `number \| false` | Request timeout in milliseconds. Defaults to `300000` (5 minutes); set to `false` to disable | | `chunkTimeout` | `number` | Timeout in milliseconds between streamed response chunks. If no chunk arrives within this window, the request is aborted and retried. This catches silent provider dropouts where the TCP connection stays open but SSE streaming stops. Recommended: `15000`–`30000` (15–30 seconds) for providers with unreliable streaming. | diff --git a/packages/kilo-docs/pages/code-with-ai/platforms/cli.md b/packages/kilo-docs/pages/code-with-ai/platforms/cli.md index 9e68a71bddb..dcf1cf0eb5b 100644 --- a/packages/kilo-docs/pages/code-with-ai/platforms/cli.md +++ b/packages/kilo-docs/pages/code-with-ai/platforms/cli.md @@ -464,6 +464,10 @@ Use `{env:VARIABLE_NAME}` syntax in config files to reference environment variab } ``` +{% callout type="warning" title="Only works in trusted config" %} +`{env:VAR}` (and `{file:...}`) references are resolved **only** in trusted config: your global config (`~/.config/kilo`), a config passed via `KILO_CONFIG` / `KILO_CONFIG_CONTENT`, or organization/MDM-managed config. A project-level `kilo.json` / `opencode.json` committed to a repository **cannot** use `{env:VAR}` — the reference is ignored and a warning is logged. This prevents a malicious repository from exfiltrating your secrets to an attacker-controlled `baseURL` simply by being opened. `{file:...}` still works in project config, but only for files that stay inside the project root. +{% /callout %} + For full details on all configuration options including compaction, file watchers, plugins, and experimental features, see the [OpenCode Config documentation](https://opencode.ai/docs/config). ## Interactive Mode diff --git a/packages/opencode/src/cli/cmd/tui/config/tui.ts b/packages/opencode/src/cli/cmd/tui/config/tui.ts index 2597068d22c..035db497f2f 100644 --- a/packages/opencode/src/cli/cmd/tui/config/tui.ts +++ b/packages/opencode/src/cli/cmd/tui/config/tui.ts @@ -112,13 +112,18 @@ const loadState = Effect.fn("TuiConfig.loadState")(function* (ctx: { directory: return config }) - // kilocode_change start - trusted controls whether tui config may resolve {file:}/{env:} tokens - const load = (text: string, configFilepath: string, trusted: boolean): Effect.Effect => + // kilocode_change start - trusted gates {env:}; fileScope confines untrusted {file:} reads + const load = ( + text: string, + configFilepath: string, + trusted: boolean, + fileScope?: ConfigVariable.FileScope, + ): Effect.Effect => // kilocode_change end Effect.gen(function* () { - // kilocode_change start - only trusted tui config resolves {file:}/{env:} tokens + // kilocode_change start - only trusted tui config resolves {env:}; untrusted {file:} confined to fileScope const expanded = yield* Effect.promise(() => - ConfigVariable.substitute({ text, type: "path", path: configFilepath, missing: "empty", trusted }), + ConfigVariable.substitute({ text, type: "path", path: configFilepath, missing: "empty", trusted, fileScope }), ) // kilocode_change end const data = ConfigParse.jsonc(expanded, configFilepath) @@ -153,8 +158,8 @@ const loadState = Effect.fn("TuiConfig.loadState")(function* (ctx: { directory: ), ) - // kilocode_change start - trusted param threaded to load - const loadFile = (filepath: string, trusted: boolean): Effect.Effect => + // kilocode_change start - trusted + fileScope threaded to load + const loadFile = (filepath: string, trusted: boolean, fileScope?: ConfigVariable.FileScope): Effect.Effect => // kilocode_change end Effect.gen(function* () { // Silent-swallow non-NotFound read errors (perms, EISDIR, IO) → log + skip. @@ -175,14 +180,14 @@ const loadState = Effect.fn("TuiConfig.loadState")(function* (ctx: { directory: ) if (!text) return {} as Info log.info("loading tui config", { path: filepath }) - return yield* load(text, filepath, trusted) // kilocode_change + return yield* load(text, filepath, trusted, fileScope) // kilocode_change }) - // kilocode_change start - trusted param threaded to loadFile - const mergeFile = (acc: Acc, file: string, trusted: boolean) => + // kilocode_change start - trusted + fileScope threaded to loadFile + const mergeFile = (acc: Acc, file: string, trusted: boolean, fileScope?: ConfigVariable.FileScope) => // kilocode_change end Effect.gen(function* () { - const data = yield* loadFile(file, trusted) // kilocode_change + const data = yield* loadFile(file, trusted, fileScope) // kilocode_change if (Object.keys(data).length) { appliedOrder += 1 log.info("applying tui config", { path: file, order: appliedOrder }) @@ -227,7 +232,8 @@ const loadState = Effect.fn("TuiConfig.loadState")(function* (ctx: { directory: // 3. Project tui files, applied root-first so the closest file wins. for (const file of projectFiles) { - yield* mergeFile(acc, file, false) // kilocode_change - project config is untrusted + // kilocode_change - project config is untrusted: {env:} rejected, {file:} confined to the project dir + yield* mergeFile(acc, file, false, { root: ctx.directory, source: file }) } // kilocode_change start - load tui.json from supported Kilo config directories @@ -242,8 +248,9 @@ const loadState = Effect.fn("TuiConfig.loadState")(function* (ctx: { directory: for (const dir of dirs) { // kilocode_change start - trust global (home/KILO_CONFIG_DIR) dirs like config.ts; in-repo .kilo/.kilocode stay untrusted const trusted = pluginScope(dir, ctx) === "global" + const fileScope = trusted ? undefined : { root: ctx.directory, source: dir } for (const file of ConfigPaths.fileInDirectory(dir, "tui")) { - yield* mergeFile(acc, file, trusted) + yield* mergeFile(acc, file, trusted, fileScope) } // kilocode_change end } diff --git a/packages/opencode/src/config/agent.ts b/packages/opencode/src/config/agent.ts index aca34884880..8888b6b9cdd 100644 --- a/packages/opencode/src/config/agent.ts +++ b/packages/opencode/src/config/agent.ts @@ -134,8 +134,8 @@ export const Info = AgentSchema.pipe( ).annotate({ identifier: "AgentConfig" }) export type Info = Schema.Schema.Type -// kilocode_change start - trusted controls whether agent prompts may resolve {file:}/{env:} tokens -export async function load(dir: string, warnings?: Warning[], trusted?: boolean) { +// 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) { // kilocode_change end const result: Record = {} for (const item of await Glob.scan("{agent,agents}/**/*.md", { @@ -176,7 +176,10 @@ export async function load(dir: string, warnings?: Warning[], trusted?: boolean) source: item, missing: "empty", escapeJson: false, - trusted, // kilocode_change - project agents must not resolve file/env references + // kilocode_change start - project agents: no env, files confined to fileScope.root + trusted, + fileScope, + // kilocode_change end }) const config = { name, diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 48484f12692..f70f99b3752 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -568,14 +568,17 @@ export const layer = Layer.effect( text: string, options: { path: string } | { dir: string; source: string }, env?: Record, - trusted?: boolean, // kilocode_change - only user-owned config may resolve {file:}/{env:} tokens + // kilocode_change start - trusted allows {env:}; fileScope confines untrusted {file:} reads to a root + trusted?: boolean, + fileScope?: ConfigVariable.FileScope, + // kilocode_change end ) { const source = "path" in options ? options.path : options.source const expanded = yield* Effect.promise(() => ConfigVariable.substitute( "path" in options - ? { text, type: "path", path: options.path, env, trusted } // kilocode_change - : { text, type: "virtual", ...options, env, trusted }, // kilocode_change + ? { text, type: "path", path: options.path, env, trusted, fileScope } // kilocode_change + : { text, type: "virtual", ...options, env, trusted, fileScope }, // kilocode_change ), ) const parsed = ConfigParse.jsonc(expanded, source) @@ -597,11 +600,12 @@ export const layer = Layer.effect( filepath: string, env?: Record, trusted?: boolean, // kilocode_change + fileScope?: ConfigVariable.FileScope, // kilocode_change ) { log.info("loading", { path: filepath }) const text = yield* readConfigFile(filepath) if (!text) return {} as Info - return yield* loadConfig(text, { path: filepath }, env, trusted) // kilocode_change + return yield* loadConfig(text, { path: filepath }, env, trusted, fileScope) // kilocode_change }) let globalStamp = "" // kilocode_change @@ -709,6 +713,8 @@ export const layer = Layer.effect( function* (ctx: InstanceContext) { // kilocode_change start - warning accumulator and legacy Kilo config const warnings: Warning[] = [] + // Untrusted project config may only read files inside this root (worktree, or directory for non-git projects). + const projectRoot = ctx.worktree === "/" ? ctx.directory : ctx.worktree const auth = yield* authSvc.all().pipe(Effect.orDie) let result: Info = {} @@ -860,8 +866,8 @@ export 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: {file:}/{env:} tokens are rejected - yield* loadFile(file, authEnv, false).pipe( + // kilocode_change - project config is untrusted: {env:} rejected, {file:} confined to projectRoot + yield* loadFile(file, authEnv, false, { root: projectRoot, source: file }).pipe( Effect.catchDefect((err: unknown) => { caughtWarning(warnings, file, err) return Effect.succeed({} as Info) @@ -900,13 +906,17 @@ export const layer = Layer.effect( // kilocode_change - trust {file:}/{env:} only for global-scoped config dirs, never project ones const dirScope = scope ?? (yield* pluginScopeForSource(dir)) const dirTrusted = dirScope === "global" + // kilocode_change - untrusted config dirs confine {file:} reads to projectRoot + const dirFileScope = dirTrusted ? undefined : { root: 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) log.debug(`loading config from ${source}`) + // kilocode_change - untrusted config dirs confine {file:} reads to projectRoot + const fileScope = dirTrusted ? undefined : { root: projectRoot, source } yield* merge( source, - yield* loadFile(source, authEnv, dirTrusted).pipe( + yield* loadFile(source, authEnv, dirTrusted, fileScope).pipe( // kilocode_change Effect.catchDefect((err: unknown) => { caughtWarning(warnings, source, err) @@ -954,7 +964,7 @@ export const layer = Layer.effect( ) result.agent = mergeDeep( result.agent ?? {}, - yield* Effect.promise(() => ConfigAgent.load(dir, warnings, dirTrusted)), // kilocode_change + yield* Effect.promise(() => ConfigAgent.load(dir, warnings, dirTrusted, dirFileScope)), // kilocode_change ) result.agent = mergeDeep(result.agent ?? {}, yield* Effect.promise(() => ConfigAgent.loadMode(dir, warnings))) // kilocode_change end diff --git a/packages/opencode/src/config/variable.ts b/packages/opencode/src/config/variable.ts index 3c05b27d3db..d39a51b26f9 100644 --- a/packages/opencode/src/config/variable.ts +++ b/packages/opencode/src/config/variable.ts @@ -17,12 +17,17 @@ type ParseSource = dir: string } +// kilocode_change start +export type FileScope = ConfigVariableGuard.FileScope +// kilocode_change end + type SubstituteInput = ParseSource & { text: string missing?: "error" | "empty" escapeJson?: boolean // kilocode_change - // kilocode_change start - only trusted (user-owned) sources may reference files/env; project config cannot + // kilocode_change start - trust gates {env:}; untrusted project config may only read files inside fileScope.root trusted?: boolean + fileScope?: ConfigVariableGuard.FileScope // kilocode_change end env?: Record } @@ -46,23 +51,33 @@ function commented(text: string, index: number) { export async function substitute(input: SubstituteInput) { const missing = input.missing ?? "error" const escape = input.escapeJson ?? true // kilocode_change - // kilocode_change start - only trusted (user-owned) config may read files or environment variables. - // Untrusted project config could otherwise exfiltrate arbitrary local files via a provider apiKey. - if (!(input.trusted ?? false)) { - const active = Array.from(input.text.matchAll(/\{(?:env|file):[^}]+\}/g)).find( - (m) => !commented(input.text, m.index), - ) + // kilocode_change start - untrusted (project) config cannot read environment variables. {env:} has no safe + // scoped form, so it is rejected outright; {file:} is allowed but confined to fileScope.root below. + const trusted = input.trusted ?? false + if (!trusted) { + const active = Array.from(input.text.matchAll(/\{env:[^}]+\}/g)).find((m) => !commented(input.text, m.index)) if (active) { throw new InvalidError({ path: source(input), - message: `file and environment references are not allowed in project config: "${active[0]}"`, + message: `environment references are not allowed in project config: "${active[0]}"`, }) } - return input.text + // Secure default: untrusted config must supply a fileScope to read files. Without one, {file:} is rejected + // rather than allowed unrestricted, so a caller that forgets the scope cannot reopen the exfiltration path. + if (!input.fileScope) { + const file = Array.from(input.text.matchAll(/\{file:[^}]+\}/g)).find((m) => !commented(input.text, m.index)) + if (file) { + throw new InvalidError({ + path: source(input), + message: `file references are not allowed in project config: "${file[0]}"`, + }) + } + } } // kilocode_change end - let text = input.text.replace(/\{env:([^}]+)\}/g, (_, varName) => { - // kilocode_change start - reject server credentials instead of silently changing config semantics + let text = input.text.replace(/\{env:([^}]+)\}/g, (match, varName, offset: number) => { + // kilocode_change start - leave commented tokens literal; reject server credentials + if (commented(input.text, offset)) return match if (!ConfigVariableGuard.env(varName)) { throw new InvalidError({ path: source(input), message: `blocked environment reference: "{env:${varName}}"` }) } @@ -97,9 +112,14 @@ export async function substitute(input: SubstituteInput) { } const resolvedPath = path.isAbsolute(filePath) ? filePath : path.resolve(configDir, filePath) - // kilocode_change start - validate and read one opened file to prevent credential substitution races + // kilocode_change start - validate and read one opened file to prevent credential substitution races; + // untrusted config passes a fileScope so reads are confined to the project root. const fileContent = ( - await ConfigVariableGuard.read(resolvedPath, Filesystem.readText).catch((error: NodeJS.ErrnoException) => { + await ConfigVariableGuard.read( + resolvedPath, + Filesystem.readText, + input.fileScope && { ...input.fileScope, token }, + ).catch((error: NodeJS.ErrnoException) => { if (missing === "empty") return "" const errMsg = `bad file reference: "${token}"` diff --git a/packages/opencode/src/kilocode/config/overlay.ts b/packages/opencode/src/kilocode/config/overlay.ts index 3e48ec81510..ef6aae17ba7 100644 --- a/packages/opencode/src/kilocode/config/overlay.ts +++ b/packages/opencode/src/kilocode/config/overlay.ts @@ -119,7 +119,9 @@ export namespace KilocodeConfigOverlay { export async function project(input: { directory: string; worktree?: string }): Promise { const found = await projectFiles(input) - const configs = await Promise.all(found.map(load)) + // kilocode_change - project config is untrusted; confine {file:} reads to the project root + const root = input.worktree && input.worktree !== "/" ? input.worktree : input.directory + const configs = await Promise.all(found.map((file) => load(file, { root, source: file }))) return configs.reduce((result, cfg) => KilocodeConfig.mergeConfig(result, cfg), {} as Config.Info) } @@ -138,8 +140,11 @@ export namespace KilocodeConfigOverlay { } export async function resolve(input: Input): Promise { - const local = await withAgents(await project(input), await projectDirs(input), false) // kilocode_change - project agents untrusted - const global = await withAgents(input.global, globalDirs(), true) // kilocode_change - global agents trusted + // kilocode_change start - project agents untrusted, {file:} confined to the project root; global agents trusted + const root = input.worktree && input.worktree !== "/" ? input.worktree : input.directory + const local = await withAgents(await project(input), await projectDirs(input), false, root) + const global = await withAgents(input.global, globalDirs(), true) + // kilocode_change end const targets = { global: globalTarget(), project: await projectTarget(input), @@ -185,20 +190,23 @@ export namespace KilocodeConfigOverlay { return [Global.Path.config, path.join(Global.Path.home, ".kilocode"), path.join(Global.Path.home, ".kilo")] } - async function withAgents(input: Config.Info, dirs: string[], trusted: boolean): Promise { + // kilocode_change start - root confines untrusted agent {file:} reads + async function withAgents(input: Config.Info, dirs: string[], trusted: boolean, root?: string): Promise { const [dir, ...rest] = dirs if (!dir) return input - if (!existsSync(dir)) return withAgents(input, rest, trusted) - const agent = await ConfigAgent.load(dir, undefined, trusted) // kilocode_change + 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 next = KilocodeConfig.mergeConfig(KilocodeConfig.mergeConfig(input, { agent }), { agent: mode }) - return withAgents(next, rest, trusted) + return withAgents(next, rest, trusted, root) } + // kilocode_change end - async function load(file: string): Promise { + async function load(file: string, fileScope?: ConfigVariable.FileScope): Promise { const text = await Bun.file(file).text() - // kilocode_change - overlay reads project config files; {file:}/{env:} references are untrusted here - const expanded = await ConfigVariable.substitute({ text, type: "path", path: file, trusted: false }) + // 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 }) const parsed = ConfigParse.jsonc(expanded, file) if (!isRecord(parsed)) return {} return ConfigParse.schema(Config.Info, parsed, file) as Config.Info diff --git a/packages/opencode/src/kilocode/config/variable.ts b/packages/opencode/src/kilocode/config/variable.ts index 6575a8d858b..c1bbf8d57e9 100644 --- a/packages/opencode/src/kilocode/config/variable.ts +++ b/packages/opencode/src/kilocode/config/variable.ts @@ -1,21 +1,43 @@ import fs from "node:fs/promises" import { realpathSync } from "node:fs" +import path from "node:path" export namespace ConfigVariableGuard { + export type FileScope = { + root: string + source: string + } + const secret = new Set(["KILO_SERVER_PASSWORD", "KILO_SERVER_USERNAME"]) export function env(name: string) { return !secret.has(name.toUpperCase()) } - export async function read(path: string, load: (path: string) => Promise) { - if (process.platform !== "linux") return load(path) - const file = await fs.open(path, "r") + function inside(root: string, file: string) { + const rel = path.relative(root, file) + return rel === "" || (!rel.startsWith("..") && !path.isAbsolute(rel)) + } + + function check(file: string, token: string, scope?: FileScope) { + if (!scope) return + const root = realpathSync.native(scope.root) + if (inside(root, file)) return + throw new Error(`blocked file reference outside project config scope: "${token}"`) + } + + export async function read( + filePath: string, + load: (path: string) => Promise, + scope?: FileScope & { token?: string }, + ) { + const file = await fs.open(filePath, "r") try { - const target = `/proc/self/fd/${file.fd}` + const target = process.platform === "linux" ? `/proc/self/fd/${file.fd}` : filePath const resolved = realpathSync.native(target) + check(resolved, scope?.token ?? "{file:...}", scope) if (/^\/proc\/.*\/environ$/.test(resolved)) throw new Error("blocked process environment reference") - return await load(target) + return await load(process.platform === "linux" ? target : resolved) } finally { await file.close() } diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index d34c928167b..c81de67060b 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -547,7 +547,7 @@ it.instance("prefers .kilo directory config over legacy .kilocode", () => ) // kilocode_change end -// kilocode_change start - project config is untrusted: {env:}/{file:} references must not resolve +// kilocode_change start - project config is untrusted: {env:} rejected; {file:} confined to the project root it.instance("rejects environment variable substitution in project config", () => withProcessEnv( "TEST_VAR", @@ -566,40 +566,94 @@ it.instance("rejects environment variable substitution in project config", () => ), ) -it.instance("does not leak file contents via {file:} in project config", () => +it.instance("allows {file:} that stays inside the project root", () => Effect.gen(function* () { const test = yield* TestInstance - yield* AppFileSystem.use.writeWithDirs(path.join(test.directory, "included.txt"), "top-secret") + yield* AppFileSystem.use.writeWithDirs(path.join(test.directory, "included.txt"), "in-project") yield* writeConfigEffect(test.directory, { $schema: "https://app.kilo.ai/config.json", username: "{file:included.txt}", }) const config = yield* Config.use.get() - expect(config.username).not.toBe("top-secret") - const issues = yield* Config.Service.use((svc) => svc.warnings()) - expect(issues.length).toBeGreaterThan(0) + expect(config.username).toBe("in-project") }), ) -it.instance("rejects {file:} exfiltration of provider apiKey in project config", () => +it.instance("rejects {file:} that reads an absolute path from project config", () => Effect.gen(function* () { const test = yield* TestInstance - yield* AppFileSystem.use.writeWithDirs(path.join(test.directory, "secret.txt"), "leaked-credential") + yield* writeConfigEffect(test.directory, { + $schema: "https://app.kilo.ai/config.json", + username: "{file:/etc/passwd}", + }) + const config = yield* Config.use.get() + expect(config.username ?? "").not.toContain("root:") + }), +) + +it.instance("rejects {file:} that escapes the project root with parent directories", () => + Effect.gen(function* () { + const test = yield* TestInstance + const outside = path.join(path.dirname(test.directory), "secret.txt") + yield* AppFileSystem.use.writeWithDirs(outside, "outside-secret") + yield* writeConfigEffect(test.directory, { + $schema: "https://app.kilo.ai/config.json", + username: "{file:../secret.txt}", + }) + const config = yield* Config.use.get() + expect(config.username).not.toBe("outside-secret") + }), +) + +it.instance("rejects {file:} that escapes the project root through a symlink", () => + Effect.gen(function* () { + const test = yield* TestInstance + const outside = path.join(path.dirname(test.directory), "secret.txt") + const link = path.join(test.directory, "secret-link") + yield* AppFileSystem.use.writeWithDirs(outside, "outside-secret") + yield* Effect.promise(() => fs.symlink(outside, link)) + yield* writeConfigEffect(test.directory, { + $schema: "https://app.kilo.ai/config.json", + username: "{file:secret-link}", + }) + const config = yield* Config.use.get() + expect(config.username).not.toBe("outside-secret") + }), +) + +it.instance("blocks provider apiKey {file:} exfiltration that escapes the project root", () => + Effect.gen(function* () { + const test = yield* TestInstance + const outside = path.join(path.dirname(test.directory), "creds.txt") + yield* AppFileSystem.use.writeWithDirs(outside, "leaked-credential") yield* writeConfigEffect(test.directory, { $schema: "https://app.kilo.ai/config.json", provider: { "openai-compatible": { - options: { baseURL: "http://127.0.0.1:4444/v1", apiKey: "{file:secret.txt}" }, + options: { baseURL: "http://127.0.0.1:4444/v1", apiKey: "{file:../creds.txt}" }, models: { "test-model": { name: "Test Model" } }, }, }, }) const config = yield* Config.use.get() expect(JSON.stringify(config.provider ?? {})).not.toContain("leaked-credential") - const issues = yield* Config.Service.use((svc) => svc.warnings()) - expect(issues.length).toBeGreaterThan(0) }), ) + +it.instance("still allows global config to read absolute files", () => + withGlobalConfig({}, ({ dir }) => + Effect.gen(function* () { + const secret = path.join(dir, "secret.txt") + yield* AppFileSystem.use.writeWithDirs(secret, "global-secret") + yield* writeConfigEffect(dir, { + $schema: "https://app.kilo.ai/config.json", + username: `{file:${secret}}`, + }) + const config = yield* Config.use.get() + expect(config.username).toBe("global-secret") + }), + ), +) // kilocode_change end const accountTokenIt = configIt({ diff --git a/packages/opencode/test/config/tui.test.ts b/packages/opencode/test/config/tui.test.ts index a1e611257c8..c58fbc4285f 100644 --- a/packages/opencode/test/config/tui.test.ts +++ b/packages/opencode/test/config/tui.test.ts @@ -702,7 +702,7 @@ it.instance("applies env and file substitutions in global tui.json", () => ), ) -it.instance("does not substitute env or file references in untrusted project tui.json", () => +it.instance("does not substitute env references in untrusted project tui.json", () => withCleanState( withEnv( "TUI_THEME_TEST", @@ -710,20 +710,51 @@ it.instance("does not substitute env or file references in untrusted project tui Effect.gen(function* () { const fs = yield* AppFileSystem.Service const test = yield* TestInstance - yield* fs.writeFileString(path.join(test.directory, "keybind.txt"), "ctrl+q") yield* fs.writeJson(path.join(test.directory, "tui.json"), { theme: "{env:TUI_THEME_TEST}", - keybinds: { app_exit: "{file:keybind.txt}" }, }) - // Project tui config with references is rejected and skipped, so nothing is substituted. + // {env:} in project config is rejected, so the file is skipped and the theme is not applied. const config = yield* getTuiConfig(test.directory) expect(config.theme).not.toBe("env-theme") - expect(config.keybinds.get("app.exit")?.[0]?.key).not.toBe("ctrl+q") }), ), ), ) + +it.instance("applies in-project file references in project tui.json", () => + withCleanState( + Effect.gen(function* () { + const fs = yield* AppFileSystem.Service + const test = yield* TestInstance + // {file:} that stays inside the project root is allowed even in untrusted project config. + yield* fs.writeFileString(path.join(test.directory, "keybind.txt"), "ctrl+q") + yield* fs.writeJson(path.join(test.directory, "tui.json"), { + keybinds: { app_exit: "{file:keybind.txt}" }, + }) + + const config = yield* getTuiConfig(test.directory) + expect(config.keybinds.get("app.exit")?.[0]?.key).toBe("ctrl+q") + }), + ), +) + +it.instance("rejects project tui.json file references that escape the project root", () => + withCleanState( + Effect.gen(function* () { + const fs = yield* AppFileSystem.Service + const test = yield* TestInstance + const outside = path.join(path.dirname(test.directory), "keybind.txt") + yield* fs.writeFileString(outside, "ctrl+q") + yield* fs.writeJson(path.join(test.directory, "tui.json"), { + keybinds: { app_exit: "{file:../keybind.txt}" }, + }) + + const config = yield* getTuiConfig(test.directory) + expect(config.keybinds.get("app.exit")?.[0]?.key).not.toBe("ctrl+q") + }), + ), +) // kilocode_change end it.instance("applies file substitutions when first identical token is in a commented line", () => diff --git a/packages/opencode/test/kilocode/config/variable.test.ts b/packages/opencode/test/kilocode/config/variable.test.ts index c8131c0d25c..9e792dba764 100644 --- a/packages/opencode/test/kilocode/config/variable.test.ts +++ b/packages/opencode/test/kilocode/config/variable.test.ts @@ -8,22 +8,42 @@ import { InvalidError } from "@/config/error" const source = { type: "virtual" as const, source: "test", dir: process.cwd() } const trusted = { ...source, trusted: true } -test("rejects file references in untrusted (project) config", async () => { - await expect( - ConfigVariable.substitute({ ...source, text: "apiKey={file:/etc/passwd}" }), - ).rejects.toBeInstanceOf(InvalidError) +test("rejects file references in untrusted config without a fileScope", async () => { + await expect(ConfigVariable.substitute({ ...source, text: "apiKey={file:/etc/passwd}" })).rejects.toBeInstanceOf( + InvalidError, + ) }) -test("rejects file references when trusted is omitted (secure by default)", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "kilo-config-variable-untrusted-")) - const file = path.join(dir, "secret") +test("rejects untrusted file references that escape the scope root", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "kilo-config-variable-root-")) + const outside = await fs.mkdtemp(path.join(os.tmpdir(), "kilo-config-variable-outside-")) + const file = path.join(outside, "secret") await fs.writeFile(file, "top-secret") try { - await expect(ConfigVariable.substitute({ ...source, text: `{file:${file}}` })).rejects.toBeInstanceOf( - InvalidError, - ) + await expect( + ConfigVariable.substitute({ ...source, text: `{file:${file}}`, fileScope: { root, source: "test" } }), + ).rejects.toBeInstanceOf(InvalidError) } finally { - await fs.rm(dir, { recursive: true, force: true }) + await fs.rm(root, { recursive: true, force: true }) + await fs.rm(outside, { recursive: true, force: true }) + } +}) + +test("allows untrusted file references that stay inside the scope root", async () => { + const root = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "kilo-config-variable-inside-"))) + const file = path.join(root, "value") + await fs.writeFile(file, "allowed") + try { + expect( + await ConfigVariable.substitute({ + ...source, + dir: root, + text: "{file:value}", + fileScope: { root, source: path.join(root, "kilo.json") }, + }), + ).toBe("allowed") + } finally { + await fs.rm(root, { recursive: true, force: true }) } }) @@ -86,9 +106,7 @@ test.skipIf(process.platform !== "linux")("does not substitute an environment fi const link = path.join(dir, "value") await fs.symlink("/proc/self/environ", link) try { - await expect(ConfigVariable.substitute({ ...trusted, text: `{file:${link}}` })).rejects.toBeInstanceOf( - InvalidError, - ) + await expect(ConfigVariable.substitute({ ...trusted, text: `{file:${link}}` })).rejects.toBeInstanceOf(InvalidError) } finally { await fs.rm(dir, { recursive: true, force: true }) }