diff --git a/.changeset/sandbox-default-flag.md b/.changeset/sandbox-default-flag.md new file mode 100644 index 00000000000..1d14fdfc96c --- /dev/null +++ b/.changeset/sandbox-default-flag.md @@ -0,0 +1,5 @@ +--- +"@kilocode/cli": patch +--- + +Keep sandboxing disabled by default unless the experimental sandbox setting or an explicit session toggle enables it. diff --git a/packages/opencode/src/kilocode/sandbox/policy.ts b/packages/opencode/src/kilocode/sandbox/policy.ts index 5add0d231ab..0a322878a67 100644 --- a/packages/opencode/src/kilocode/sandbox/policy.ts +++ b/packages/opencode/src/kilocode/sandbox/policy.ts @@ -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(sessionID: SessionID, effect: Effect.Effect) { 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(sessionID: SessionID, guard: Effect.Effect) 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 diff --git a/packages/opencode/test/kilocode/sandbox/config-network.test.ts b/packages/opencode/test/kilocode/sandbox/config-network.test.ts index 9a347d2f72a..d4a54b87b86 100644 --- a/packages/opencode/test/kilocode/sandbox/config-network.test.ts +++ b/packages/opencode/test/kilocode/sandbox/config-network.test.ts @@ -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)))) }) diff --git a/packages/opencode/test/kilocode/sandbox/session.test.ts b/packages/opencode/test/kilocode/sandbox/session.test.ts index 0f950ab7590..8cb90f4a825 100644 --- a/packages/opencode/test/kilocode/sandbox/session.test.ts +++ b/packages/opencode/test/kilocode/sandbox/session.test.ts @@ -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() diff --git a/packages/opencode/test/kilocode/sandbox/shell-network.test.ts b/packages/opencode/test/kilocode/sandbox/shell-network.test.ts index b2c6ec22a88..f05fbeaa21b 100644 --- a/packages/opencode/test/kilocode/sandbox/shell-network.test.ts +++ b/packages/opencode/test/kilocode/sandbox/shell-network.test.ts @@ -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) diff --git a/packages/opencode/test/kilocode/sandbox/state.test.ts b/packages/opencode/test/kilocode/sandbox/state.test.ts index c861b01d5b8..82fdc19fd98 100644 --- a/packages/opencode/test/kilocode/sandbox/state.test.ts +++ b/packages/opencode/test/kilocode/sandbox/state.test.ts @@ -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) diff --git a/packages/opencode/test/kilocode/task-nesting.test.ts b/packages/opencode/test/kilocode/task-nesting.test.ts index 3791cd33faa..ef8e2e497b6 100644 --- a/packages/opencode/test/kilocode/task-nesting.test.ts +++ b/packages/opencode/test/kilocode/task-nesting.test.ts @@ -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 } } }, ), )