fix(cli): keep sandbox disabled by default

This commit is contained in:
marius-kilocode
2026-07-02 21:22:36 +02:00
parent f6e0e87cfe
commit 067fcf51f8
7 changed files with 107 additions and 88 deletions
+5
View File
@@ -0,0 +1,5 @@
---
"@kilocode/cli": patch
---
Keep sandboxing disabled by default unless the experimental sandbox setting or an explicit session toggle enables it.
@@ -2,7 +2,6 @@ import { readFileSync, statSync } from "node:fs"
import path from "node:path"
import { Effect, Semaphore } from "effect"
import { Global } from "@opencode-ai/core/global"
import { Flag } from "@opencode-ai/core/flag/flag"
import { backendSupport, run as runSandbox, unrestricted, type Profile } from "@kilocode/sandbox"
import { Bus } from "@/bus"
import { Config } from "@/config/config"
@@ -23,11 +22,6 @@ function key(directory: string, sessionID: SessionID) {
return directory + "\0" + sessionID
}
function secure(snapshot: Snapshot): Snapshot {
if (Flag.KILO_SERVER_PASSWORD) return snapshot
return { ...snapshot, enabled: true, mode: "deny" }
}
function locked<A, E, R>(sessionID: SessionID, effect: Effect.Effect<A, E, R>) {
return Effect.acquireUseRelease(
Effect.sync(() => {
@@ -141,11 +135,11 @@ const snapshot = Effect.fn("SandboxPolicy.snapshot")(function* (sessionID: Sessi
// A session's create-time kilocode.sandbox toggle takes precedence over the config default, so a
// session moved or created with an explicit choice keeps that choice instead of resetting.
const chosen = yield* SandboxState.read(sessionID)
const next = secure({
const next: Snapshot = {
enabled: chosen?.enabled ?? cfg.experimental?.sandbox ?? false,
mode: cfg.experimental?.sandbox_restrict_network === false ? "allow" : "deny",
version: 0,
})
}
yield* Effect.promise(() => SandboxStore.write(directory, sessionID, next))
snapshots.set(key(directory, sessionID), next)
return { directory, state: next }
@@ -180,13 +174,13 @@ function change<E, R>(sessionID: SessionID, guard: Effect.Effect<unknown, E, R>)
yield* guard
const stored = yield* read(directory, sessionID)
const cfg = stored ? undefined : yield* (yield* Config.Service).get()
const current =
const current: Snapshot =
stored ??
secure({
{
enabled: cfg?.experimental?.sandbox ?? false,
mode: cfg?.experimental?.sandbox_restrict_network === false ? "allow" : "deny",
version: 0,
})
}
const support = backendSupport({ mode: current.mode, allowedHosts: [] })
const status = {
directory,
@@ -224,7 +218,7 @@ export const inherit = Effect.fn("SandboxPolicy.inherit")(function* (
parentID,
Effect.gen(function* () {
const stored = yield* read(directory, parentID)
const parent = stored ?? (fallback && secure({ ...fallback, version: 0 }))
const parent: Snapshot | undefined = stored ?? (fallback && { ...fallback, version: 0 })
if (!parent) return
// Only persist the parent snapshot when it actually belongs to this directory. A fallback
// carries confinement from another directory (e.g. forking into a worktree) and must not be
@@ -74,7 +74,7 @@ restricted.live("keeps network restriction enabled by default when the sandbox i
}).pipe(Effect.ensuring(Effect.promise(() => target.server.stop(true))))
})
open.live("keeps network denied without authenticated server control", () => {
open.live("allows network when restriction is disabled without authenticated server control", () => {
const target = server()
return Effect.gen(function* () {
const http = yield* HttpClient.HttpClient
@@ -86,10 +86,11 @@ open.live("keeps network denied without authenticated server control", () => {
)
if (!backendSupport().available) {
expect(Exit.isSuccess(exit)).toBe(true)
expect(target.requests()).toBe(1)
return
}
expect(status.enabled).toBe(true)
expect(Exit.isFailure(exit)).toBe(true)
expect(target.requests()).toBe(0)
expect(Exit.isSuccess(exit)).toBe(true)
expect(target.requests()).toBe(1)
}).pipe(Effect.ensuring(Effect.promise(() => target.server.stop(true))))
})
@@ -92,8 +92,8 @@ describe("sandbox session cleanup", () => {
yield* provideInstance(dir)(SandboxPolicy.toggle(info.id))
yield* provideInstance(worktree)(SandboxPolicy.toggle(info.id))
expect((yield* Effect.promise(() => SandboxStore.read(dir, info.id)))?.enabled).toBe(false)
expect((yield* Effect.promise(() => SandboxStore.read(worktree, info.id)))?.enabled).toBe(false)
expect((yield* Effect.promise(() => SandboxStore.read(dir, info.id)))?.enabled).toBe(true)
expect((yield* Effect.promise(() => SandboxStore.read(worktree, info.id)))?.enabled).toBe(true)
yield* session.remove(info.id)
expect(yield* Effect.promise(() => SandboxStore.read(dir, info.id))).toBeUndefined()
expect(yield* Effect.promise(() => SandboxStore.read(worktree, info.id))).toBeUndefined()
@@ -135,7 +135,7 @@ describe("model shell network integration", () => {
)
test.skipIf(process.platform !== "darwin" && process.platform !== "linux")(
"keeps spawned shell network denied without authenticated server control",
"honors configured shell network access without authenticated server control",
async () => {
const effect = Effect.gen(function* () {
const root = yield* tmpdirScoped()
@@ -156,9 +156,9 @@ describe("model shell network integration", () => {
provideInstance(root),
Effect.provide(configured(true)),
)
expect(allow.output).not.toContain("model-shell-network-ok")
expect(allow.metadata.exit).not.toBe(0)
expect(allowed.accepted()).toBe(0)
expect(allow.output).toContain("model-shell-network-ok")
expect(allow.metadata.exit).toBe(0)
expect(allowed.accepted()).toBe(1)
expect(deny.output).not.toContain("model-shell-network-ok")
expect(deny.metadata.exit).not.toBe(0)
expect(denied.accepted()).toBe(0)
@@ -11,6 +11,7 @@ import { Bus } from "@/bus"
import { Config } from "@/config/config"
import * as Network from "@/kilocode/sandbox/network"
import * as SandboxPolicy from "@/kilocode/sandbox/policy"
import { SandboxStore } from "@/kilocode/sandbox/store"
import { SessionID } from "@/session/schema"
import { TestInstance } from "../../fixture/fixture"
import { testEffect } from "../../lib/effect"
@@ -190,13 +191,28 @@ it.instance("snapshots the primary kilo config for the session lifetime", () =>
),
)
it.instance("keeps authless config-off sessions confined", () =>
Effect.gen(function* () {
const id = SessionID.make("ses_sandbox_default_off")
const status = yield* SandboxPolicy.status(id)
expect(status.enabled).toBe(status.available)
expect(yield* execute(id, sandboxed)).toBe(status.available)
}),
it.instance("does not enable authless sessions without the experimental sandbox flag", () =>
Effect.acquireUseRelease(
Effect.sync(() => {
const password = Flag.KILO_SERVER_PASSWORD
Flag.KILO_SERVER_PASSWORD = undefined
return password
}),
() =>
Effect.gen(function* () {
const test = yield* TestInstance
const id = SessionID.make("ses_sandbox_default_off")
const status = yield* SandboxPolicy.status(id)
const state = yield* Effect.promise(() => SandboxStore.read(test.directory, id))
expect(state?.enabled).toBe(false)
expect(state?.mode).toBe("deny")
expect(state?.version).toBe(0)
expect(status.enabled).toBe(false)
expect(yield* execute(id, sandboxed)).toBe(false)
}),
(password) => Effect.sync(() => (Flag.KILO_SERVER_PASSWORD = password)),
),
)
it.instance(
@@ -226,15 +242,15 @@ it.instance(
{ config: { experimental: { sandbox: true } } },
)
it.instance("trusted toggles disable only one authless session", () =>
it.instance("trusted toggles enable only one authless config-off session", () =>
Effect.gen(function* () {
const first = SessionID.make("ses_sandbox_override_on")
const second = SessionID.make("ses_sandbox_default_remains_off")
if (!(yield* SandboxPolicy.status(first)).available) return
expect((yield* SandboxPolicy.toggle(first)).enabled).toBe(false)
expect(yield* execute(first, sandboxed)).toBe(false)
expect(yield* execute(second, sandboxed)).toBe(true)
expect((yield* SandboxPolicy.toggle(first)).enabled).toBe(true)
expect(yield* execute(first, sandboxed)).toBe(true)
expect(yield* execute(second, sandboxed)).toBe(false)
}),
)
@@ -248,14 +264,14 @@ it.instance("isolates concurrent session overrides and clears them", () =>
return
}
expect((yield* SandboxPolicy.toggle(first)).enabled).toBe(false)
expect((yield* SandboxPolicy.status(second)).enabled).toBe(true)
expect((yield* SandboxPolicy.toggle(second)).enabled).toBe(false)
expect((yield* SandboxPolicy.toggle(first)).enabled).toBe(true)
expect((yield* SandboxPolicy.status(second)).enabled).toBe(false)
expect((yield* SandboxPolicy.toggle(second)).enabled).toBe(true)
expect((yield* SandboxPolicy.status(first)).enabled).toBe(false)
yield* SandboxPolicy.retire(first, (yield* TestInstance).directory, Effect.void)
expect((yield* SandboxPolicy.toggle(second)).enabled).toBe(false)
expect((yield* SandboxPolicy.status(first)).enabled).toBe(true)
expect((yield* SandboxPolicy.status(second)).enabled).toBe(true)
yield* SandboxPolicy.retire(first, (yield* TestInstance).directory, Effect.void)
expect((yield* SandboxPolicy.status(first)).enabled).toBe(false)
expect((yield* SandboxPolicy.status(second)).enabled).toBe(false)
}),
)
@@ -274,7 +290,7 @@ it.instance("serializes concurrent toggles for a session", () =>
const id = SessionID.make("ses_sandbox_concurrent")
if (!(yield* SandboxPolicy.status(id)).available) return
yield* Effect.all([SandboxPolicy.toggle(id), SandboxPolicy.toggle(id)], { concurrency: "unbounded" })
expect((yield* SandboxPolicy.status(id)).enabled).toBe(true)
expect((yield* SandboxPolicy.status(id)).enabled).toBe(false)
}),
)
@@ -298,7 +314,7 @@ it.instance("prevents a queued toggle from restoring a retired override", () =>
yield* Fiber.join(removal)
expect(Exit.isFailure(yield* Fiber.join(pending))).toBe(true)
const status = yield* SandboxPolicy.status(id)
expect(status.enabled).toBe(status.available)
expect(status.enabled).toBe(false)
}),
)
@@ -344,6 +360,7 @@ it.instance("enforces writes only while the macOS session override is active", (
svc.spawn(ChildProcess.make("/usr/bin/touch", [file])).pipe(Effect.flatMap((child) => child.exitCode)),
)
expect((yield* SandboxPolicy.toggle(id)).enabled).toBe(true)
expect(Number(yield* execute(id, run(inside)))).toBe(0)
expect(Number(yield* execute(id, run(external)))).not.toBe(0)
expect(Number(yield* execute(id, run(git)))).not.toBe(0)
@@ -384,56 +384,58 @@ describe("Kilo task nesting", () => {
)
it.live("refreshes inherited restrictions when resuming a task child", () =>
provideTmpdirInstance(() =>
Effect.gen(function* () {
const sessions = yield* Session.Service
const { chat, assistant } = yield* seed()
const support = yield* SandboxPolicy.status(chat.id)
yield* sessions.setPermission({
sessionID: chat.id,
permission: [{ permission: "bash", pattern: "*", action: "deny" }],
})
const child = yield* sessions.create({ parentID: chat.id, title: "Existing child" })
if (support.available) {
yield* SandboxPolicy.toggle(child.id)
expect((yield* SandboxPolicy.status(child.id)).enabled).toBe(false)
}
const tool = yield* TaskTool
const def = yield* tool.init()
provideTmpdirInstance(
() =>
Effect.gen(function* () {
const sessions = yield* Session.Service
const { chat, assistant } = yield* seed()
const support = yield* SandboxPolicy.status(chat.id)
yield* sessions.setPermission({
sessionID: chat.id,
permission: [{ permission: "bash", pattern: "*", action: "deny" }],
})
const child = yield* sessions.create({ parentID: chat.id, title: "Existing child" })
if (support.available) {
yield* SandboxPolicy.toggle(child.id)
expect((yield* SandboxPolicy.status(child.id)).enabled).toBe(false)
}
const tool = yield* TaskTool
const def = yield* tool.init()
const exec = () =>
def.execute(
{
description: "inspect bug",
prompt: "look into the cache key path",
subagent_type: "explore",
task_id: child.id,
},
{
sessionID: chat.id,
messageID: assistant.id,
agent: "build",
abort: new AbortController().signal,
extra: { promptOps: stubOps() },
messages: [],
metadata: () => Effect.void,
ask: () => Effect.void,
},
const exec = () =>
def.execute(
{
description: "inspect bug",
prompt: "look into the cache key path",
subagent_type: "explore",
task_id: child.id,
},
{
sessionID: chat.id,
messageID: assistant.id,
agent: "build",
abort: new AbortController().signal,
extra: { promptOps: stubOps() },
messages: [],
metadata: () => Effect.void,
ask: () => Effect.void,
},
)
yield* exec()
const first = yield* sessions.get(child.id)
if (support.available) expect((yield* SandboxPolicy.status(child.id)).enabled).toBe(true)
const count = first.permission?.filter((rule) => rule.permission === "bash").length
yield* exec()
const resumed = yield* sessions.get(child.id)
expect(resumed.permission).toEqual(
expect.arrayContaining([{ permission: "bash", pattern: "*", action: "deny" }]),
)
yield* exec()
const first = yield* sessions.get(child.id)
if (support.available) expect((yield* SandboxPolicy.status(child.id)).enabled).toBe(true)
const count = first.permission?.filter((rule) => rule.permission === "bash").length
yield* exec()
const resumed = yield* sessions.get(child.id)
expect(resumed.permission).toEqual(
expect.arrayContaining([{ permission: "bash", pattern: "*", action: "deny" }]),
)
expect(count).toBeGreaterThan(0)
expect(resumed.permission?.filter((rule) => rule.permission === "bash")).toHaveLength(count ?? 0)
}),
expect(count).toBeGreaterThan(0)
expect(resumed.permission?.filter((rule) => rule.permission === "bash")).toHaveLength(count ?? 0)
}),
{ config: { experimental: { sandbox: true } } },
),
)