Merge pull request #11898 from Kilo-Org/alluring-flyingfish

fix(cli): keep sandbox disabled by default
This commit is contained in:
Marius
2026-07-03 15:38:07 +02:00
committed by GitHub
7 changed files with 106 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"
@@ -24,11 +23,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 initial(
chosen: boolean | undefined,
pref: boolean | undefined,
@@ -37,7 +31,7 @@ function initial(
): Snapshot {
if (chosen !== undefined) return { enabled: chosen, mode, version: 0 }
if (pref !== undefined) return { enabled: pref, mode, version: 0 }
return secure({ enabled: cfgDefault, mode, version: 0 })
return { enabled: cfgDefault, mode, version: 0 }
}
const resolveInitial = Effect.fn("SandboxPolicy.resolveInitial")(function* (directory: string, sessionID: SessionID) {
@@ -47,7 +41,6 @@ const resolveInitial = Effect.fn("SandboxPolicy.resolveInitial")(function* (dire
const mode = cfg.experimental?.sandbox_restrict_network === false ? "allow" : "deny"
return initial(chosen?.enabled, pref, cfg.experimental?.sandbox ?? false, mode)
})
function locked<A, E, R>(sessionID: SessionID, effect: Effect.Effect<A, E, R>) {
return Effect.acquireUseRelease(
Effect.sync(() => {
@@ -160,7 +153,7 @@ 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. The
// persisted per-directory preference (last toggled state) is the next precedence, so new sessions
// inherit the last /sandbox choice. secure-by-default only applies when neither is present.
// inherit the last /sandbox choice. The config default applies when neither is present.
const next = yield* resolveInitial(directory, sessionID)
yield* Effect.promise(() => SandboxStore.write(directory, sessionID, next))
snapshots.set(key(directory, sessionID), next)
@@ -239,7 +232,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(
@@ -233,10 +249,10 @@ it.instance("persists an authless toggle to later sessions", () =>
const second = SessionID.make("ses_sandbox_authless_inherit")
if (!(yield* SandboxPolicy.status(first)).available) return
expect((yield* SandboxPolicy.toggle(first)).enabled).toBe(false)
expect(yield* execute(first, sandboxed)).toBe(false)
expect((yield* SandboxPolicy.status(second)).enabled).toBe(false)
expect(yield* execute(second, sandboxed)).toBe(false)
expect((yield* SandboxPolicy.toggle(first)).enabled).toBe(true)
expect(yield* execute(first, sandboxed)).toBe(true)
expect((yield* SandboxPolicy.status(second)).enabled).toBe(true)
expect(yield* execute(second, sandboxed)).toBe(true)
}),
)
@@ -269,18 +285,18 @@ it.instance("isolates concurrent session overrides and clears them", () =>
}
// Seed second with its own stored snapshot before any toggle, so its state
// stays independent of the per-directory preference that toggles now persist.
expect((yield* SandboxPolicy.status(second)).enabled).toBe(true)
expect((yield* SandboxPolicy.status(second)).enabled).toBe(false)
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)
expect((yield* SandboxPolicy.toggle(second)).enabled).toBe(false)
expect((yield* SandboxPolicy.status(first)).enabled).toBe(true)
yield* SandboxPolicy.retire(first, (yield* TestInstance).directory, Effect.void)
// retire clears first's stored snapshot; it re-seeds from the persisted
// per-directory preference, which holds the last toggle (second -> true).
expect((yield* SandboxPolicy.status(first)).enabled).toBe(true)
expect((yield* SandboxPolicy.status(second)).enabled).toBe(true)
// per-directory preference, which holds the last toggle (second -> false).
expect((yield* SandboxPolicy.status(first)).enabled).toBe(false)
expect((yield* SandboxPolicy.status(second)).enabled).toBe(false)
}),
)
@@ -299,7 +315,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)
}),
)
@@ -323,7 +339,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)
}),
)
@@ -369,6 +385,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 } } },
),
)