fix(sandbox): keep permissions outside confinement policy

This commit is contained in:
marius-kilocode
2026-06-22 20:35:20 +02:00
parent c55e804c1c
commit 6a8961e036
11 changed files with 69 additions and 291 deletions
+7 -32
View File
@@ -1,14 +1,13 @@
import { Context, Effect, PlatformError, Ref } from "effect"
import { Context, Effect, PlatformError } from "effect"
import { canonicalize, canonicalizeEntry, matches, normalize } from "./path"
import type { Profile } from "./profile"
export const CurrentProfile = Context.Reference<Ref.Ref<Profile> | undefined>("@kilocode/sandbox/CurrentProfile", {
export const CurrentProfile = Context.Reference<Profile | undefined>("@kilocode/sandbox/CurrentProfile", {
defaultValue: () => undefined,
})
export const current: Effect.Effect<Profile | undefined> = Effect.gen(function* () {
const ref = yield* CurrentProfile
return ref ? yield* Ref.get(ref) : undefined
return yield* CurrentProfile
})
export const enabled: Effect.Effect<boolean> = Effect.map(current, (profile) => profile !== undefined)
@@ -19,30 +18,7 @@ export function run<A, E, R>(
): Effect.Effect<A, E | PlatformError.PlatformError, R> {
return Effect.gen(function* () {
const value = yield* normalize(profile)
const ref = yield* Ref.make(value)
return yield* effect.pipe(Effect.provideService(CurrentProfile, ref))
})
}
export function grantWrite(
path: string,
kind: "literal" | "subtree" = "literal",
): Effect.Effect<void, PlatformError.PlatformError> {
return Effect.gen(function* () {
const ref = yield* CurrentProfile
if (!ref) return
const target = yield* canonicalize(path)
yield* Ref.update(ref, (profile) => {
const last = profile.filesystem.writeRules.at(-1)
if (last?.action === "allow" && last.rule.kind === kind && last.rule.path === target) return profile
return {
...profile,
filesystem: {
...profile.filesystem,
writeRules: [...profile.filesystem.writeRules, { rule: { path: target, kind }, action: "allow" as const }],
},
}
})
return yield* effect.pipe(Effect.provideService(CurrentProfile, value))
})
}
@@ -76,10 +52,9 @@ function assertTarget(
) {
yield* Effect.fail(denied(path, method))
}
if (profile.filesystem.allowWrite.some((rule) => matches(rule, target))) return
const rule = profile.filesystem.writeRules.findLast((item) => matches(item.rule, target))
if (rule?.action === "allow") return
yield* Effect.fail(denied(path, method))
if (!profile.filesystem.allowWrite.some((rule) => matches(rule, target))) {
yield* Effect.fail(denied(path, method))
}
})
}
+2 -10
View File
@@ -1,14 +1,6 @@
export type {
EnvironmentProfile,
FilesystemProfile,
NetworkProfile,
PathKind,
PathRule,
Profile,
WriteRule,
} from "./profile"
export type { EnvironmentProfile, FilesystemProfile, NetworkProfile, PathKind, PathRule, Profile } from "./profile"
export { canonicalize, canonicalizeEntry } from "./path"
export { CurrentProfile, assertEntry, assertWrite, current, enabled, grantWrite, run } from "./context"
export { CurrentProfile, assertEntry, assertWrite, current, enabled, run } from "./context"
export { decorateFileSystem, layer } from "./filesystem"
export { prepare, prepareCommand, support } from "./backend"
export type { Backend, Launch, PreparedLaunch, Support } from "./backend"
+1 -7
View File
@@ -1,7 +1,7 @@
import { lstatSync, readlinkSync, realpathSync } from "node:fs"
import path from "node:path"
import { Effect, PlatformError } from "effect"
import type { PathRule, Profile, WriteRule } from "./profile"
import type { PathRule, Profile } from "./profile"
function code(cause: unknown) {
if (typeof cause !== "object" || cause === null || !("code" in cause)) return undefined
@@ -68,11 +68,6 @@ export function normalize(profile: Profile): Effect.Effect<Profile, PlatformErro
return Effect.gen(function* () {
const allowWrite = yield* Effect.forEach(profile.filesystem.allowWrite, normalizeRule)
const denyWrite = yield* Effect.forEach(profile.filesystem.denyWrite, normalizeRule)
const writeRules = yield* Effect.forEach(
profile.filesystem.writeRules,
(item): Effect.Effect<WriteRule, PlatformError.PlatformError> =>
Effect.map(normalizeRule(item.rule), (rule) => ({ ...item, rule })),
)
const temporaryDirectory = profile.filesystem.temporaryDirectory
? yield* canonicalize(profile.filesystem.temporaryDirectory)
: undefined
@@ -82,7 +77,6 @@ export function normalize(profile: Profile): Effect.Effect<Profile, PlatformErro
filesystem: {
allowWrite,
denyWrite,
writeRules,
denyNames: profile.filesystem.denyNames,
...(temporaryDirectory === undefined ? {} : { temporaryDirectory }),
},
-6
View File
@@ -5,15 +5,9 @@ export interface PathRule {
readonly kind: PathKind
}
export interface WriteRule {
readonly rule: PathRule
readonly action: "allow" | "deny" | "ask"
}
export interface FilesystemProfile {
readonly allowWrite: ReadonlyArray<PathRule>
readonly denyWrite: ReadonlyArray<PathRule>
readonly writeRules: ReadonlyArray<WriteRule>
readonly denyNames: ReadonlyArray<string>
readonly temporaryDirectory?: string | undefined
}
+2 -13
View File
@@ -36,27 +36,16 @@ function policy(profile: Profile) {
params.push({ key, value: rule.path })
return filter(rule, key)
})
const rules = profile.filesystem.writeRules.map((item, index) => {
const key = `WRITE_RULE_${index}`
params.push({ key, value: item.rule.path })
return { ...item, key }
})
const external = rules.flatMap((item, index) => {
if (item.action !== "allow") return []
const later = rules.slice(index + 1).flatMap((rule) => exclude(rule.rule, rule.key))
return [`(require-all ${filter(item.rule, item.key)} ${later.join(" ")})`]
})
const deny = profile.filesystem.denyWrite.flatMap((rule, index) => {
const key = `DENY_WRITE_${index}`
params.push({ key, value: rule.path })
return exclude(rule, key)
})
const names = profile.filesystem.denyNames.map((name) => `(require-not (regex #"(^|/)${escape(name)}(/|$)"))`)
const sources = [...allow, ...external]
const write =
sources.length === 0
allow.length === 0
? ""
: `(allow file-write*\n (require-all\n (require-any ${sources.join(" ")})\n ${[...deny, ...names].join("\n ")}\n )\n)`
: `(allow file-write*\n (require-all\n (require-any ${allow.join(" ")})\n ${[...deny, ...names].join("\n ")}\n )\n)`
return {
value: [base, "; reads are not confined by the file-level sandbox\n(allow file-read*)", write].join("\n"),
params,
@@ -8,7 +8,6 @@ function makeProfile(): Profile {
filesystem: {
allowWrite: [{ path: "/workspace", kind: "subtree" }],
denyWrite: [{ path: "/workspace/.git", kind: "subtree" }],
writeRules: [],
denyNames: [".git"],
},
network: { mode: "deny", allowedHosts: ["example.com"] },
@@ -41,31 +40,6 @@ describe("sandbox launch preparation", () => {
expect(result.args.slice(-3)).toEqual(["--", "/bin/echo", "hello"])
})
test("preserves ordered external write rules in the process policy", () => {
const profile = makeProfile()
const result = generate(
{
...profile,
filesystem: {
...profile.filesystem,
writeRules: [
{ rule: { path: "/tmp", kind: "subtree" }, action: "allow" },
{ rule: { path: "/tmp/private", kind: "subtree" }, action: "ask" },
{ rule: { path: "/tmp/private/approved", kind: "subtree" }, action: "allow" },
],
},
},
launch,
{ available: true },
)
const policy = result.args[1]
expect(policy).toContain('(require-not (subpath (param "WRITE_RULE_1")))')
expect(policy).toContain('(subpath (param "WRITE_RULE_2"))')
expect(result.args).toContain("-DWRITE_RULE_0=/tmp")
expect(result.args).toContain("-DWRITE_RULE_1=/tmp/private")
expect(result.args).toContain("-DWRITE_RULE_2=/tmp/private/approved")
})
test("places shell commands inside the sandbox backend", () => {
const result = generate(
makeProfile(),
+15 -78
View File
@@ -2,17 +2,16 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test"
import { mkdir, mkdtemp, realpath, rm, symlink } from "node:fs/promises"
import { tmpdir } from "node:os"
import path from "node:path"
import { Deferred, Effect } from "effect"
import { assertWrite, current, enabled, grantWrite, run, type Profile } from "../src"
import { Effect } from "effect"
import { assertWrite, current, enabled, run, type Profile } from "../src"
function makeProfile(
allowWrite: Profile["filesystem"]["allowWrite"],
denyWrite: Profile["filesystem"]["denyWrite"] = [],
denyNames: Profile["filesystem"]["denyNames"] = [],
writeRules: Profile["filesystem"]["writeRules"] = [],
): Profile {
return {
filesystem: { allowWrite, denyWrite, writeRules, denyNames },
filesystem: { allowWrite, denyWrite, denyNames },
network: { mode: "allow", allowedHosts: [] },
environment: { deny: [], set: {} },
}
@@ -39,87 +38,25 @@ describe("sandbox profile context", () => {
expect(await Effect.runPromise(current)).toBeUndefined()
})
test("keeps grants isolated across concurrent runs", async () => {
const target = path.join(root, "granted.txt")
const ready = await Effect.runPromise(Deferred.make<void>())
const profile = makeProfile([])
const result = await Effect.runPromise(
Effect.all(
[
run(
profile,
Effect.gen(function* () {
yield* grantWrite(target)
yield* Deferred.succeed(ready, undefined)
yield* assertWrite(target)
return true
}),
),
run(
profile,
Effect.gen(function* () {
yield* Deferred.await(ready)
const error = yield* assertWrite(target).pipe(Effect.flip)
return error.reason._tag
}),
),
],
{ concurrency: "unbounded" },
),
)
expect(result).toEqual([true, "PermissionDenied"])
})
test("lets an explicit grant override an ordered ask rule", async () => {
const target = path.join(root, "approved", "file.txt")
const profile = makeProfile(
[],
[],
[],
[
{ rule: { path: root, kind: "subtree" }, action: "allow" },
{ rule: { path: path.join(root, "approved"), kind: "subtree" }, action: "ask" },
],
)
const before = await Effect.runPromise(run(profile, assertWrite(target).pipe(Effect.flip)))
expect(before.reason._tag).toBe("PermissionDenied")
await Effect.runPromise(
run(
profile,
Effect.gen(function* () {
yield* grantWrite(path.join(root, "approved"), "subtree")
yield* assertWrite(target)
}),
),
)
})
test("applies deny rules before overlapping allows and grants", async () => {
const denied = path.join(root, ".git")
const target = path.join(denied, "config")
const profile = makeProfile([{ path: root, kind: "subtree" }], [{ path: denied, kind: "subtree" }])
test("rejects writes outside the allowed roots", async () => {
const error = await Effect.runPromise(
run(
profile,
Effect.gen(function* () {
yield* grantWrite(target)
return yield* assertWrite(target).pipe(Effect.flip)
}),
),
run(makeProfile([]), assertWrite(path.join(root, "outside.txt")).pipe(Effect.flip)),
)
expect(error.reason._tag).toBe("PermissionDenied")
})
test("applies denied path names to future external grants", async () => {
test("applies deny rules before overlapping allows", async () => {
const denied = path.join(root, ".git")
const target = path.join(denied, "config")
const profile = makeProfile([{ path: root, kind: "subtree" }], [{ path: denied, kind: "subtree" }])
const error = await Effect.runPromise(run(profile, assertWrite(target).pipe(Effect.flip)))
expect(error.reason._tag).toBe("PermissionDenied")
})
test("applies denied path names under allowed roots", async () => {
const target = path.join(root, "external", ".git", "config")
const error = await Effect.runPromise(
run(
makeProfile([], [], [".git"]),
Effect.gen(function* () {
yield* grantWrite(path.join(root, "external"), "subtree")
return yield* assertWrite(target).pipe(Effect.flip)
}),
),
run(makeProfile([{ path: root, kind: "subtree" }], [], [".git"]), assertWrite(target).pipe(Effect.flip)),
)
expect(error.reason._tag).toBe("PermissionDenied")
})
@@ -13,7 +13,6 @@ function makeProfile(root: string, temporaryDirectory?: string): Profile {
filesystem: {
allowWrite: [{ path: root, kind: "subtree" }],
denyWrite: [],
writeRules: [],
denyNames: [],
...(temporaryDirectory === undefined ? {} : { temporaryDirectory }),
},
@@ -1,37 +1,15 @@
import { Effect } from "effect"
import { isAbsolute, resolve as pathResolve } from "node:path"
import { Global } from "@opencode-ai/core/global"
import { run as runSandbox, grantWrite, type PathRule, type Profile } from "@kilocode/sandbox"
import { run as runSandbox, type PathRule, type Profile } from "@kilocode/sandbox"
import { Config } from "@/config/config"
import { InstanceState } from "@/effect/instance-state"
import { Permission } from "@/permission"
import type { InstanceContext } from "@/project/instance-context"
function root(path: string): PathRule {
return { path, kind: "subtree" }
}
function pattern(value: string, ctx: InstanceContext): PathRule | undefined {
const input = value.trim()
if (input === "*") return root(pathResolve("/"))
const match = input.match(/^(.*?)[\\/]\*+$/)
const path = match?.[1] ?? input
if (!path || path.includes("*") || path.includes("?")) return undefined
return {
path: isAbsolute(path) ? path : pathResolve(ctx.directory, path),
kind: match ? "subtree" : "literal",
}
}
function rules(ctx: InstanceContext, cfg: Config.Info) {
return Permission.fromConfig(cfg.permission ?? {}).flatMap((rule) => {
if (rule.permission !== "external_directory") return []
const item = pattern(rule.pattern, ctx)
return item ? [{ action: rule.action, rule: item }] : []
})
}
export function profile(ctx: InstanceContext, cfg: Config.Info): Profile {
export function profile(ctx: InstanceContext): Profile {
const writable = [
...(ctx.worktree === "/" ? [] : [ctx.worktree]),
ctx.directory,
@@ -49,7 +27,6 @@ export function profile(ctx: InstanceContext, cfg: Config.Info): Profile {
filesystem: {
allowWrite: writable,
denyWrite: [],
writeRules: rules(ctx, cfg),
denyNames: [".git"],
temporaryDirectory: Global.Path.tmp,
},
@@ -73,21 +50,6 @@ export function execute<A, E, R>(effect: Effect.Effect<A, E, R>) {
const config = yield* Config.Service
const cfg = yield* config.get()
if (!cfg.experimental?.sandbox) return yield* effect
return yield* runSandbox(profile(yield* InstanceState.context, cfg), effect)
})
}
export function approved(input: {
permission: string
patterns: readonly string[]
metadata: Record<string, unknown>
}) {
return Effect.gen(function* () {
if (input.permission !== "external_directory" || input.metadata.access === "read") return
const ctx = yield* InstanceState.context
for (const value of input.patterns) {
const rule = pattern(value, ctx)
if (rule) yield* grantWrite(rule.path, rule.kind)
}
return yield* runSandbox(profile(yield* InstanceState.context), effect)
})
}
+11 -10
View File
@@ -37,8 +37,10 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
const run = yield* EffectBridge.make()
const plugin = yield* Plugin.Service
const permission = yield* Permission.Service
const agents = yield* Agent.Service // kilocode_change
const sessions = yield* Session.Service // kilocode_change
// kilocode_change start
const agents = yield* Agent.Service
const sessions = yield* Session.Service
// kilocode_change end
const registry = yield* ToolRegistry.Service
const mcp = yield* MCP.Service
const truncate = yield* Truncate.Service
@@ -51,8 +53,8 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
extra: { model: input.model, bypassAgentCheck: input.bypassAgentCheck, promptOps: input.promptOps },
agent: input.agent.name,
messages: input.messages,
metadata: (val) => input.processor.metadata(options.toolCallId, val), // kilocode_change
// kilocode_change start - resolve permissions at ask time so active tools see config edits
// kilocode_change start
metadata: (val) => input.processor.metadata(options.toolCallId, val),
ask: (req) =>
KiloSessionPrompt.askPermission({
permission,
@@ -65,12 +67,9 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
sessionID: input.session.id,
tool: { messageID: input.processor.message.id, callID: options.toolCallId },
},
}).pipe(
Effect.tap(() => SandboxPolicy.approved(req)), // kilocode_change - grant this tool call's approved external writes
Effect.orDie,
),
// kilocode_change end
}).pipe(Effect.orDie),
})
// kilocode_change end
for (const item of yield* registry.tools({
modelID: ModelID.make(input.model.api.id),
@@ -90,7 +89,9 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
{ tool: item.id, sessionID: ctx.sessionID, callID: ctx.callID },
{ args },
)
const result = yield* SandboxPolicy.execute(item.execute(args, ctx)) // kilocode_change
// kilocode_change start
const result = yield* SandboxPolicy.execute(item.execute(args, ctx))
// kilocode_change end
const output = {
...result,
attachments: result.attachments?.map((attachment) => ({
@@ -1,5 +1,5 @@
import { describe, expect, test } from "bun:test"
import { Config } from "@/config/config"
import { Global } from "@opencode-ai/core/global"
import { profile } from "@/kilocode/sandbox/policy"
import type { InstanceContext } from "@/project/instance-context"
import { ProjectID } from "@/project/schema"
@@ -11,80 +11,41 @@ const ctx: InstanceContext = {
id: ProjectID.make("sandbox-policy-test"),
worktree: "/workspace",
time: { created: 0, updated: 0 },
sandboxes: [],
sandboxes: ["/workspace/sandbox"],
},
}
function config(permission: NonNullable<Config.Info["permission"]>): Config.Info {
return { permission }
}
function paths(input: ReturnType<typeof profile>, key: "allowWrite" | "denyWrite") {
return input.filesystem[key].map((rule) => rule.path)
}
function rules(input: ReturnType<typeof profile>) {
return input.filesystem.writeRules
function paths() {
return profile(ctx).filesystem.allowWrite.map((rule) => rule.path)
}
describe("sandbox policy", () => {
test("keeps project roots writable when external directories are denied", () => {
const result = profile(ctx, config({ external_directory: "deny" }))
expect(paths(result, "allowWrite")).toContain(ctx.directory)
expect(paths(result, "denyWrite")).not.toContain("/")
test("allows project and Kilo state roots", () => {
const result = paths()
expect(result).toContain(ctx.worktree)
expect(result).toContain(ctx.directory)
expect(result).toContain(ctx.project.sandboxes?.[0])
expect(result).toContain(Global.Path.data)
expect(result).toContain(Global.Path.state)
expect(result).toContain(Global.Path.tmp)
})
test("turns a later ask rule into an exclusion from an earlier allow", () => {
const result = profile(
ctx,
config({
external_directory: {
"/tmp/*": "allow",
"/tmp/private/*": "ask",
},
}),
test("does not derive writable roots from tool permissions", () => {
expect(new Set(paths())).toEqual(
new Set([
ctx.worktree,
ctx.directory,
...(ctx.project.sandboxes ?? []),
Global.Path.data,
Global.Path.cache,
Global.Path.config,
Global.Path.state,
Global.Path.tmp,
Global.Path.bin,
Global.Path.log,
Global.Path.repos,
]),
)
expect(rules(result)).toEqual([
{ action: "allow", rule: { path: "/tmp", kind: "subtree" } },
{ action: "ask", rule: { path: "/tmp/private", kind: "subtree" } },
])
})
test("allows a later narrow rule after an earlier deny", () => {
const result = profile(
ctx,
config({
external_directory: {
"/tmp/*": "deny",
"/tmp/public/*": "allow",
},
}),
)
expect(rules(result)).toEqual([
{ action: "deny", rule: { path: "/tmp", kind: "subtree" } },
{ action: "allow", rule: { path: "/tmp/public", kind: "subtree" } },
])
})
test("preserves exact paths and supports trailing double-star directory rules", () => {
const result = profile(
ctx,
config({
external_directory: {
"/tmp/exact": "allow",
"/var/cache/**": "allow",
},
}),
)
expect(rules(result)).toEqual([
{ action: "allow", rule: { path: "/tmp/exact", kind: "literal" } },
{ action: "allow", rule: { path: "/var/cache", kind: "subtree" } },
])
})
test("hard-protects git metadata under every current and future writable root", () => {
const result = profile(ctx, config({ external_directory: "allow" }))
expect(result.filesystem.denyNames).toContain(".git")
expect(rules(result)).toContainEqual({ action: "allow", rule: { path: "/", kind: "subtree" } })
expect(profile(ctx).filesystem.denyNames).toContain(".git")
})
})