Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/sandbox-default-flag.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@kilocode/cli": patch
---

Keep sandboxing disabled by default unless the experimental sandbox setting or an explicit session toggle enables it.
13 changes: 3 additions & 10 deletions packages/opencode/src/kilocode/sandbox/policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand All @@ -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) {
Expand All @@ -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(() => {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))))
})
4 changes: 2 additions & 2 deletions packages/opencode/test/kilocode/sandbox/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
8 changes: 4 additions & 4 deletions packages/opencode/test/kilocode/sandbox/shell-network.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
Expand Down
59 changes: 38 additions & 21 deletions packages/opencode/test/kilocode/sandbox/state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
}),
)

Expand Down Expand Up @@ -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)
}),
)

Expand All @@ -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)
}),
)

Expand All @@ -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)
}),
)

Expand Down Expand Up @@ -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)
Expand Down
96 changes: 49 additions & 47 deletions packages/opencode/test/kilocode/task-nesting.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
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)
}),
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)
}),
{ config: { experimental: { sandbox: true } } },
),
)

Expand Down
Loading