diff --git a/.changeset/secure-file-mentions.md b/.changeset/secure-file-mentions.md new file mode 100644 index 00000000000..0fad988f115 --- /dev/null +++ b/.changeset/secure-file-mentions.md @@ -0,0 +1,6 @@ +--- +"kilo-code": patch +"@kilocode/cli": patch +--- + +Enforce read and ignore permissions when file mentions add content to a prompt. diff --git a/packages/opencode/src/kilocode/reference/contains.ts b/packages/opencode/src/kilocode/reference/contains.ts new file mode 100644 index 00000000000..94b0850157a --- /dev/null +++ b/packages/opencode/src/kilocode/reference/contains.ts @@ -0,0 +1,20 @@ +import { Effect } from "effect" +import { AppFileSystem } from "@opencode-ai/core/filesystem" +import type { Reference } from "@/reference/reference" + +export namespace KiloReference { + export const contains = Effect.fn("KiloReference.contains")(function* (input: { + fs: Pick + references: Pick + target: string + }) { + const target = yield* input.fs.realPath(input.target).pipe(Effect.catch(() => Effect.succeed(input.target))) + const refs = yield* input.references.list() + for (const reference of refs) { + if (reference.kind !== "git") continue + const root = yield* input.fs.realPath(reference.path).pipe(Effect.catch(() => Effect.succeed(reference.path))) + if (AppFileSystem.contains(root, target)) return true + } + return false + }) +} diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 56d221bd310..eca717ae4eb 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -7,6 +7,7 @@ import { KiloSession } from "@/kilocode/session" // kilocode_change import { KiloCostPropagation } from "@/kilocode/session/cost-propagation" // kilocode_change import { KiloSessionProcessor } from "@/kilocode/session/processor" // kilocode_change import { KiloSessionOverflow } from "@/kilocode/session/overflow" // kilocode_change +import { KiloReference } from "@/kilocode/reference/contains" // kilocode_change import * as SandboxPolicy from "@/kilocode/sandbox/policy" // kilocode_change import { CommandTimeout } from "@/kilocode/command-timeout" // kilocode_change import { Suggestion } from "@/kilocode/suggestion" // kilocode_change @@ -61,6 +62,7 @@ import * as EffectLogger from "@opencode-ai/core/effect/logger" import { InstanceState } from "@/effect/instance-state" import { EffectBridge } from "@/effect/bridge" import { TaskTool, type TaskPromptOps } from "@/tool/task" +import { assertExternalDirectoryEffect } from "@/tool/external-directory" // kilocode_change import { SessionRunState } from "./run-state" import { RuntimeFlags } from "@/effect/runtime-flags" import { EventV2Bridge } from "@/event-v2-bridge" @@ -847,6 +849,11 @@ export const layer = Layer.effect( const reference = yield* references.get(name.slice(0, slash)) if (!reference || reference.kind === "invalid") return if (!AppFileSystem.contains(reference.path, filepath)) return + // kilocode_change start - symlinks must remain inside the configured reference root + const root = yield* fsys.realPath(reference.path).pipe(Effect.catch(() => Effect.succeed(reference.path))) + const real = yield* fsys.realPath(filepath).pipe(Effect.catch(() => Effect.succeed(filepath))) + if (!AppFileSystem.contains(root, real)) return + // kilocode_change end const target = path.relative(reference.path, filepath).split(path.sep).join("/") if (!target || target.startsWith("../") || target === "..") return @@ -966,19 +973,37 @@ export const layer = Layer.effect( const mime = (yield* fsys.isDir(filepath)) ? "application/x-directory" : part.mime const { read } = yield* registry.named() - const execRead = (args: Parameters[0], extra?: Tool.Context["extra"]) => { - const controller = new AbortController() - return read - .execute(args, { - sessionID: input.sessionID, - abort: controller.signal, - agent: input.agent!, - messageID: info.id, - extra: { bypassCwdCheck: true, ...extra }, - messages: [], - metadata: () => Effect.void, - ask: () => Effect.void, + // kilocode_change start - authorize prompt attachments like model-issued read calls + const controller = new AbortController() + const ask: Tool.Context["ask"] = (request) => + Effect.gen(function* () { + const session = yield* sessions.get(input.sessionID) + yield* KiloSessionPrompt.askPermission({ + permission, + agents, + sessions, + agent: ag, + session, + request: { + ...request, + sessionID: input.sessionID, + }, }) + }).pipe(Effect.orDie) + const ctx = (extra?: Tool.Context["extra"]): Tool.Context => ({ + sessionID: input.sessionID, + abort: controller.signal, + agent: ag.name, + messageID: info.id, + extra: { bypassCwdCheck: Boolean(referenceContext), ...extra }, + messages: [], + metadata: () => Effect.void, + ask, + }) + // kilocode_change end + const execRead = (args: Parameters[0], extra?: Tool.Context["extra"]) => { + return read + .execute(args, ctx(extra)) // kilocode_change - enforce read and external_directory permissions .pipe(Effect.onInterrupt(() => Effect.sync(() => controller.abort()))) } @@ -1110,10 +1135,55 @@ export const layer = Layer.effect( ] } + // kilocode_change start - direct binary and media attachments bypass ReadTool + const target = yield* fsys.realPath(filepath).pipe(Effect.catch(() => Effect.succeed(filepath))) + const access = yield* Effect.gen(function* () { + const instance = yield* InstanceState.context + const context = ctx() + const referenced = + Boolean(referenceContext) || + ((yield* references.contains(filepath)) && + (yield* KiloReference.contains({ fs: fsys, references, target }))) + yield* assertExternalDirectoryEffect(context, target, { + bypass: referenced, + kind: "file", + }) + yield* context.ask({ + permission: "read", + patterns: [ + ...new Set([path.relative(instance.worktree, filepath), path.relative(instance.worktree, target)]), + ], + always: ["*"], + metadata: {}, + }) + }).pipe(Effect.exit) + if (Exit.isFailure(access)) { + const error = Cause.squash(access.cause) + log.error("failed to read file", { error }) + const message = error instanceof Error ? error.message : String(error) + yield* bus.publish(Session.Event.Error, { + sessionID: input.sessionID, + error: new NamedError.Unknown({ message }).toObject(), + }) + return [ + ...(referenceContext + ? [{ ...referenceContext, messageID: info.id, sessionID: input.sessionID }] + : []), + { + messageID: info.id, + sessionID: input.sessionID, + type: "text", + synthetic: true, + text: `Read tool failed to read ${filepath} with the following error: ${message}`, + }, + ] + } + // kilocode_change end + // kilocode_change start - reject oversized user image files before reading and base64 allocation if (mime.startsWith("image/")) { const limit = (yield* config.get()).attachment?.image?.max_base64_bytes ?? Image.MAX_BASE64_BYTES - const stat = yield* fsys.stat(filepath).pipe(Effect.catch(Effect.die)) + const stat = yield* fsys.stat(target).pipe(Effect.catch(Effect.die)) const encoded = ((stat.size + 2n) / 3n) * 4n if (encoded > BigInt(limit)) return yield* Effect.die( @@ -1135,7 +1205,7 @@ export const layer = Layer.effect( type: "file", url: `data:${mime};base64,` + - Buffer.from(yield* fsys.readFile(filepath).pipe(Effect.catch(Effect.die))).toString("base64"), + Buffer.from(yield* fsys.readFile(target).pipe(Effect.catch(Effect.die))).toString("base64"), mime, filename: part.filename!, source: part.source, diff --git a/packages/opencode/src/tool/read.ts b/packages/opencode/src/tool/read.ts index 16c3156fa52..f33eda4ce7c 100644 --- a/packages/opencode/src/tool/read.ts +++ b/packages/opencode/src/tool/read.ts @@ -14,6 +14,7 @@ import { isPdfAttachment, sniffAttachmentMime } from "@/util/media" import { Reference } from "@/reference/reference" // kilocode_change start import * as Encoding from "../kilocode/encoding" +import { KiloReference } from "@/kilocode/reference/contains" import * as Extract from "../kilocode/tool/read-extract" import * as TextStream from "../kilocode/text-stream" // kilocode_change end @@ -202,21 +203,32 @@ export const ReadTool = Tool.define( filepath: string, items: string[], directory: string, - abort: AbortSignal, + worktree: string, + ctx: Tool.Context, ) { const entries = yield* fs.readDirectoryEntries(filepath).pipe(Effect.catch(() => Effect.succeed([]))) const types = new Map(entries.map((entry) => [entry.name, entry.type])) + const children = items + .filter((item) => !item.endsWith("/") && types.get(item) === "file") + .map((item) => path.join(filepath, item)) + if (children.length > 0) { + yield* ctx.ask({ + permission: "read", + patterns: children.map((child) => path.relative(worktree, child)), + always: ["*"], + metadata: {}, + }) + } const files = yield* Effect.forEach( - items.filter((item) => !item.endsWith("/") && types.get(item) === "file"), - Effect.fnUntraced(function* (item) { - const child = path.join(filepath, item) + children, + Effect.fnUntraced(function* (child) { const info = yield* fs.stat(child).pipe(Effect.catch(() => Effect.void)) if (info?.type !== "File") return const sample = yield* readSample(child, Number(info.size), SAMPLE_BYTES).pipe( Effect.catch(() => Effect.succeed(new Uint8Array())), ) if (isBinaryFile(child, sample)) return - const file = yield* lines(child, { limit: DEFAULT_READ_LIMIT, offset: 1 }, abort).pipe( + const file = yield* lines(child, { limit: DEFAULT_READ_LIMIT, offset: 1 }, ctx.abort).pipe( Effect.catch(() => Effect.void), ) if (!file) return @@ -245,8 +257,17 @@ export const ReadTool = Tool.define( if (process.platform === "win32") { filepath = AppFileSystem.normalizePath(filepath) } - yield* reference.ensure(filepath) - const title = path.relative(instance.worktree, filepath) + // kilocode_change start - authorize and read the canonical target of symlinks + const requested = filepath + yield* reference.ensure(requested) + const resolved = yield* fs.realPath(requested).pipe(Effect.catch(() => Effect.succeed(requested))) + const target = process.platform === "win32" ? AppFileSystem.normalizePath(resolved) : resolved + const title = path.relative(instance.worktree, requested) + const referenced = + (yield* reference.contains(requested)) && (yield* KiloReference.contains({ fs, references: reference, target })) + const patterns = [...new Set([requested, target].map((item) => path.relative(instance.worktree, item)))] + filepath = target + // kilocode_change end const stat = yield* fs.stat(filepath).pipe( Effect.catchIf( @@ -256,13 +277,13 @@ export const ReadTool = Tool.define( ) yield* assertExternalDirectoryEffect(ctx, filepath, { - bypass: Boolean(ctx.extra?.["bypassCwdCheck"]) || (yield* reference.contains(filepath)), + bypass: Boolean(ctx.extra?.["bypassCwdCheck"]) || referenced, // kilocode_change kind: stat?.type === "Directory" ? "directory" : "file", }) yield* ctx.ask({ permission: "read", - patterns: [path.relative(instance.worktree, filepath)], + patterns, // kilocode_change - deny either the requested path or symlink target always: ["*"], metadata: {}, }) @@ -278,7 +299,9 @@ export const ReadTool = Tool.define( const truncated = start + sliced.length < items.length // kilocode_change start const expand = Boolean(ctx.extra?.["includeDirectoryFiles"]) - const loaded = expand ? yield* readDirectoryFiles(filepath, sliced, instance.directory, ctx.abort) : [] + const loaded = expand + ? yield* readDirectoryFiles(filepath, sliced, instance.directory, instance.worktree, ctx) + : [] const content = loaded.map((item) => item.content).join("\n\n") // kilocode_change end diff --git a/packages/opencode/test/kilocode/session-prompt-permission-refresh.test.ts b/packages/opencode/test/kilocode/session-prompt-permission-refresh.test.ts index abc16c64d1b..e931ab24cbb 100644 --- a/packages/opencode/test/kilocode/session-prompt-permission-refresh.test.ts +++ b/packages/opencode/test/kilocode/session-prompt-permission-refresh.test.ts @@ -2,7 +2,10 @@ import { NodeFileSystem } from "@effect/platform-node" import { expect } from "bun:test" import { Effect, Exit, Fiber, Layer } from "effect" import { FetchHttpClient } from "effect/unstable/http" +import { rm, symlink } from "fs/promises" +import os from "os" import path from "path" +import { pathToFileURL } from "url" import { AppFileSystem } from "@opencode-ai/core/filesystem" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import * as Log from "@opencode-ai/core/util/log" @@ -202,6 +205,7 @@ function makeHttp() { } const it = testEffect(makeHttp()) +const symlinkIt = process.platform === "win32" ? it.live.skip : it.live const cfg = { provider: { @@ -248,6 +252,259 @@ function providerCfg(url: string) { } } +it.live( + "blocks @file content denied by .kilocodeignore", + () => + provideTmpdirServer( + Effect.fnUntraced(function* ({ dir }) { + const sentinel = "KILO_12133_MENTION_SENTINEL" + yield* Effect.promise(() => + Promise.all([ + Bun.write(path.join(dir, "my_file.txt"), sentinel), + Bun.write(path.join(dir, ".kilocodeignore"), "my_file.txt\n"), + ]), + ) + + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const permission = yield* Permission.Service + const session = yield* sessions.create({}) + const parts = yield* prompt.resolvePromptParts("Please list the contents of @my_file.txt") + const message = yield* prompt.prompt({ sessionID: session.id, noReply: true, parts }) + const text = message.parts + .filter((part) => part.type === "text") + .map((part) => part.text) + .join("\n") + + expect(parts.some((part) => part.type === "file" && part.filename === "my_file.txt")).toBe(true) + expect(text).not.toContain(sentinel) + expect(text).toContain("prevents you from using this specific tool call") + expect(message.parts.some((part) => part.type === "file")).toBe(false) + expect(yield* permission.list()).toEqual([]) + }), + { git: true, config: providerCfg }, + ), + 30_000, +) + +it.live( + "asks before adding @file content to the prompt", + () => + provideTmpdirServer( + Effect.fnUntraced(function* ({ dir }) { + const sentinel = "KILO_12133_ASK_SENTINEL" + const file = path.join(dir, "ask.txt") + yield* Effect.promise(() => Bun.write(file, sentinel)) + + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const permission = yield* Permission.Service + const session = yield* sessions.create({}) + const fiber = yield* prompt + .prompt({ + sessionID: session.id, + noReply: true, + parts: yield* prompt.resolvePromptParts("Read @ask.txt"), + }) + .pipe(Effect.forkScoped) + const pending = yield* pollWithTimeout( + Effect.gen(function* () { + const requests = yield* permission.list() + return requests.find((request) => request.sessionID === session.id && request.permission === "read") + }), + "file mention read permission was never requested", + ) + + expect(pending.patterns).toEqual(["ask.txt"]) + yield* permission.reply({ requestID: pending.id, reply: "once" }) + const exit = yield* Fiber.await(fiber) + expect(Exit.isSuccess(exit)).toBe(true) + if (Exit.isSuccess(exit)) { + const text = exit.value.parts + .filter((part) => part.type === "text") + .map((part) => part.text) + .join("\n") + expect(text).toContain(sentinel) + } + }), + { + git: true, + config: (url) => ({ + ...providerCfg(url), + permission: { read: { "*": "allow", "ask.txt": "ask" } }, + }), + }, + ), + 30_000, +) + +it.live( + "blocks denied files in directory and binary prompt attachments", + () => + provideTmpdirServer( + Effect.fnUntraced(function* ({ dir }) { + const folder = path.join(dir, "folder") + const binary = path.join(dir, "secret.bin") + const nested = "KILO_12133_DIRECTORY_SENTINEL" + const direct = "KILO_12133_BINARY_SENTINEL" + const fs = yield* AppFileSystem.Service + yield* fs.ensureDir(folder) + yield* Effect.promise(() => + Promise.all([ + Bun.write(path.join(folder, "public.txt"), "public content"), + Bun.write(path.join(folder, "private.txt"), nested), + Bun.write(binary, direct), + ]), + ) + + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const session = yield* sessions.create({}) + const directory = yield* prompt.prompt({ + sessionID: session.id, + noReply: true, + parts: yield* prompt.resolvePromptParts("Read @folder"), + }) + const file = yield* prompt.prompt({ + sessionID: session.id, + noReply: true, + parts: [ + { type: "text", text: "Read @secret.bin" }, + { type: "file", mime: "application/octet-stream", filename: "secret.bin", url: pathToFileURL(binary).href }, + ], + }) + const text = [...directory.parts, ...file.parts] + .filter((part) => part.type === "text") + .map((part) => part.text) + .join("\n") + + expect(text).not.toContain(nested) + expect(text).not.toContain(direct) + expect(text.match(/prevents you from using this specific tool call/g)).toHaveLength(2) + expect(file.parts.some((part) => part.type === "file")).toBe(false) + }), + { + git: true, + config: (url) => ({ + ...providerCfg(url), + permission: { + read: { + "*": "allow", + "folder/private.txt": "deny", + "secret.bin": "deny", + }, + }, + }), + }, + ), + 30_000, +) + +symlinkIt( + "checks read rules for both symlink names and targets", + () => + provideTmpdirServer( + Effect.fnUntraced(function* ({ dir }) { + const text = "KILO_12133_SYMLINK_TEXT_SENTINEL" + const binary = "KILO_12133_SYMLINK_BINARY_SENTINEL" + const privateText = path.join(dir, "private.txt") + const publicText = path.join(dir, "public.txt") + const privateBinary = path.join(dir, "private.bin") + const publicBinary = path.join(dir, "public.bin") + yield* Effect.promise(async () => { + await Promise.all([Bun.write(privateText, text), Bun.write(privateBinary, binary)]) + await Promise.all([symlink("private.txt", publicText), symlink("private.bin", publicBinary)]) + }) + + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const session = yield* sessions.create({}) + const mention = yield* prompt.prompt({ + sessionID: session.id, + noReply: true, + parts: yield* prompt.resolvePromptParts("Read @public.txt"), + }) + const attachment = yield* prompt.prompt({ + sessionID: session.id, + noReply: true, + parts: [ + { type: "text", text: "Read @public.bin" }, + { + type: "file", + mime: "application/octet-stream", + filename: "public.bin", + url: pathToFileURL(publicBinary).href, + }, + ], + }) + const content = [...mention.parts, ...attachment.parts] + .filter((part) => part.type === "text") + .map((part) => part.text) + .join("\n") + + expect(content).not.toContain(text) + expect(content).not.toContain(binary) + expect(content.match(/prevents you from using this specific tool call/g)).toHaveLength(2) + expect(attachment.parts.some((part) => part.type === "file")).toBe(false) + }), + { + git: true, + config: (url) => ({ + ...providerCfg(url), + permission: { + read: { + "*": "allow", + "private.txt": "deny", + "private.bin": "deny", + }, + }, + }), + }, + ), + 30_000, +) + +symlinkIt( + "does not trust symlink targets outside configured references", + () => + provideTmpdirServer( + Effect.fnUntraced(function* ({ dir }) { + const docs = path.join(dir, "docs") + const outside = path.join(os.tmpdir(), `kilo-12133-${crypto.randomUUID()}.txt`) + const sentinel = "KILO_12133_REFERENCE_SYMLINK_SENTINEL" + const fs = yield* AppFileSystem.Service + yield* fs.ensureDir(docs) + yield* Effect.promise(() => Bun.write(outside, sentinel)) + yield* Effect.addFinalizer(() => Effect.promise(() => rm(outside, { force: true }))) + yield* Effect.promise(() => symlink(outside, path.join(docs, "public.txt"))) + + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const session = yield* sessions.create({}) + const parts = yield* prompt.resolvePromptParts("Read @docs/public.txt") + const message = yield* prompt.prompt({ sessionID: session.id, noReply: true, parts }) + const content = message.parts + .filter((part) => part.type === "text") + .map((part) => part.text) + .join("\n") + + expect(parts.some((part) => part.type === "file" && part.filename === "docs/public.txt")).toBe(true) + expect(content).not.toContain(sentinel) + expect(content).toContain("prevents you from using this specific tool call") + expect(message.parts.some((part) => part.type === "file")).toBe(false) + }), + { + git: true, + config: (url) => ({ + ...providerCfg(url), + reference: { docs: "./docs" }, + permission: { read: "allow", external_directory: "deny" }, + }), + }, + ), + 30_000, +) + it.live("active tool calls use permissions changed after model streaming starts", () => provideTmpdirServer( Effect.fnUntraced(function* ({ dir, llm }) {