From aa766f0292e3f953355ee41055082a707f1070b4 Mon Sep 17 00:00:00 2001 From: TheRealDinghyDog Date: Fri, 10 Jul 2026 08:38:16 +0200 Subject: [PATCH] Deny gated permissions in headless runs; drop the session wildcard (#26) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Write turns previously created sessions with a bare `{permission: "*", action: "allow", pattern: "*"}` rule and answered every permission.asked with "always". OpenCode merges session rules AFTER the agent ruleset and resolves with the LAST matching rule, so the wildcard silently stripped the stock build agent's safety guards (external_directory/.env reads/doom_loop stay on "ask"), and the auto-"always" responder would have approved any that survived. Send no session-level permission rules at all — the stock build agent's ruleset (wildcard allow + ask-guards, plus its own tool-output allowances) is exactly the intended headless write profile — and reject every headless permission ask with a diagnostic naming the gated category. Under the stock agents an ask only fires when a guard trips, so routine write turns stay approval-free. Also documents that OpenCode permissions are approval controls rather than an OS sandbox, updates the fake fixture so workspace edits are ungated while a gated ask must be denied, and marks the original auto-approve design in the adaptation plan as superseded. Co-Authored-By: Claude Fable 5 --- README.md | 3 ++ docs/opencode-adaptation-plan.md | 1 + plugins/opencode/scripts/lib/opencode.mjs | 54 +++++++++++++++-------- tests/fake-opencode-fixture.mjs | 29 ++++++++---- tests/runtime.test.mjs | 13 +++--- 5 files changed, 67 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 876662f..2f0f09d 100644 --- a/README.md +++ b/README.md @@ -133,6 +133,9 @@ Use it when you want OpenCode to: > [!NOTE] > Depending on the task and the model you choose these tasks might take a long time and it's generally recommended to force the task to be in the background or move the agent to the background. +> [!IMPORTANT] +> Rescue tasks default to a write-capable run using OpenCode's stock `build` agent under your own OpenCode permission configuration. Permission categories OpenCode gates behind an approval prompt (external-directory access, `.env` reads, doom-loop protection) are automatically **denied** in these headless runs — the plugin never approves a gated request on your behalf. Note that OpenCode permissions are approval-level controls, not an operating-system sandbox; run the OpenCode server in a container or as a restricted user if you need a hard filesystem boundary. + It supports `--background`, `--wait`, `--resume`, and `--fresh`. If you omit `--resume` and `--fresh`, the plugin can offer to continue the latest rescue thread for this repo. Examples: diff --git a/docs/opencode-adaptation-plan.md b/docs/opencode-adaptation-plan.md index 46ebd18..0e4f03a 100644 --- a/docs/opencode-adaptation-plan.md +++ b/docs/opencode-adaptation-plan.md @@ -108,6 +108,7 @@ Claude Code ### Read-only vs write (the `sandbox` analog) - **Review / read-only task:** send the turn with `agent: "plan"` (OpenCode's read-only agent) and/or create the session with `permission` rules denying edits. - **`--write` task:** `agent: "build"` and create the session with `permission` rules set to `allow` for the edit/shell tools (equivalent to Codex `approvalPolicy: "never"` + `workspace-write`). In headless mode there's no human to approve, so the client must **auto-approve**: either pre-set `permission` on `POST /session`, or reply `allow` to `permission.asked` / `permission.v2.asked` events via `POST /session/{id}/permissions/{permID}`. Design the client to auto-approve in write mode and auto-deny-edits in read-only mode. **This is the single most important implementation detail to get right.** + - **Superseded (issue #26):** the auto-approve design above disables OpenCode's own safety guards. Session-level rules are merged *after* the agent ruleset with last-match-wins, so a broad session allow strips the stock `build` agent's `external_directory`/`.env`/`doom_loop` guards. The implementation now sends **no** session `permission` rules and **rejects** every headless `permission.asked` — under the stock agents an ask only fires when a guard trips. ### Model / effort mapping - `--model spark` → `openai/gpt-5.3-codex-spark`; `--model openai/gpt-5.4` → `{providerID:"openai", modelID:"gpt-5.4"}`; unset → OpenCode default. Add an alias map + a `provider/model` splitter. diff --git a/plugins/opencode/scripts/lib/opencode.mjs b/plugins/opencode/scripts/lib/opencode.mjs index 7af1f15..e80a337 100644 --- a/plugins/opencode/scripts/lib/opencode.mjs +++ b/plugins/opencode/scripts/lib/opencode.mjs @@ -103,14 +103,6 @@ function buildTaskSessionName(prompt) { return excerpt ? `${TASK_SESSION_PREFIX}: ${excerpt}` : TASK_SESSION_PREFIX; } -function buildWritePermissionRules() { - // OpenCode's PermissionRule.permission is a free-form string but only real - // permission keys take effect, and the built-in `build` agent's own defaults - // still leave some categories on "ask". Use the same wildcard-allow rule the - // `build` agent ships with so headless write turns never stall on approval. - return [{ permission: "*", action: "allow", pattern: "*" }]; -} - function buildCreateSessionParams(cwd, options = {}) { const write = Boolean(options.write); const agent = options.agent ?? (write ? WRITE_AGENT : READ_ONLY_AGENT); @@ -119,14 +111,22 @@ function buildCreateSessionParams(cwd, options = {}) { // BadRequest — the model is selected per-message instead (buildMessageParams). // `directory` is not accepted either: the session inherits it from the // `opencode serve` working directory, which server-lifecycle spawns with - // `cwd`. `title` must be a string when present. Read-only turns rely on the - // read-only `plan` agent instead of a permission override. + // `cwd`. `title` must be a string when present. + // + // Deliberately no session-level `permission` rules for either mode. OpenCode + // appends session rules AFTER the agent's ruleset and resolves each request + // with the LAST matching rule, so any broad session rule silently overrides + // the stock agents' safety guards (`external_directory`, `.env` reads, and + // `doom_loop` stay on "ask") — and a session-level guard would likewise + // clobber the agent's allowances for OpenCode's own tool-output directories. + // Write turns rely on the stock `build` agent, read-only turns on the + // read-only `plan` agent; guard categories that reach "ask" are denied + // headlessly in respondToPermission (issue #26). const rawTitle = options.title ?? options.threadName ?? null; const title = typeof rawTitle === "string" && rawTitle.trim() ? rawTitle : null; return { agent, - ...(title ? { title } : {}), - ...(write ? { permission: buildWritePermissionRules() } : {}) + ...(title ? { title } : {}) }; } @@ -378,8 +378,7 @@ function createTurnCaptureState(sessionID, options = {}) { recoveryError: null, response: null, fallbackTimer: null, - onProgress: options.onProgress ?? null, - write: Boolean(options.write) + onProgress: options.onProgress ?? null }; } @@ -495,20 +494,38 @@ function applyMessageParts(state, parts, sessionID) { } } +function describePermissionRequest(event) { + const category = + (typeof event?.permission === "string" ? event.permission : null) ?? + (typeof event?.properties?.permission === "string" ? event.properties.permission : null); + const rawPatterns = event?.patterns ?? event?.properties?.patterns ?? null; + const patterns = Array.isArray(rawPatterns) ? rawPatterns.filter((value) => typeof value === "string") : []; + if (!category) { + return null; + } + return patterns.length > 0 ? `${category}: ${patterns.join(", ")}` : category; +} + async function respondToPermission(client, state, event, sessionID) { const permissionID = extractPermissionId(event); if (!permissionID || !sessionID) { return; } - const response = state.write ? "always" : "reject"; + // Never self-approve, in write mode included. Headless turns have no human + // to ask, and under the stock agents a request only reaches "ask" when a + // safety guard trips (external-directory access, `.env` reads, doom-loop + // protection) or the user configured a category as interactive. Rejecting is + // the only answer that preserves those guards; the model sees the rejection + // and adapts (issue #26). + const described = describePermissionRequest(event); emitProgress( state.onProgress, - `${state.write ? "Allowing" : "Denying"} OpenCode permission request ${permissionID}.`, - state.write ? "running" : "investigating" + `Denying OpenCode permission request ${permissionID}${described ? ` (${described})` : ""}: headless runs never self-approve gated permissions.`, + "running" ); try { - await client.respondPermission(sessionID, permissionID, response); + await client.respondPermission(sessionID, permissionID, "reject"); } catch (error) { state.error = error; emitProgress(state.onProgress, `OpenCode permission response failed: ${error.message}`, "failed"); @@ -1118,7 +1135,6 @@ export async function runServerTurn(cwd, options = {}) { ), { onProgress: options.onProgress, - write, resumed: resumedSession, turnTimeoutMs: options.turnTimeoutMs } diff --git a/tests/fake-opencode-fixture.mjs b/tests/fake-opencode-fixture.mjs index 24cee8e..d4d8af6 100644 --- a/tests/fake-opencode-fixture.mjs +++ b/tests/fake-opencode-fixture.mjs @@ -159,11 +159,11 @@ function structuredOutputParts(body) { } async function waitForPermission(permissionID) { - await new Promise((resolve) => { - const timeout = setTimeout(resolve, 2000); - pendingPermissions.set(permissionID, () => { + return new Promise((resolve) => { + const timeout = setTimeout(() => resolve(null), 2000); + pendingPermissions.set(permissionID, (reply) => { clearTimeout(timeout); - resolve(); + resolve(reply ?? null); }); }); } @@ -185,10 +185,23 @@ async function handleMessage(req, res, sessionID) { emit({ type: "session.next.step.started", sessionID }); if (session.agent === "build" || body.agent === "build") { - const permissionID = "perm_" + messageID; - emit({ type: "permission.asked", sessionID, permissionID, permission: { id: permissionID, tool: "edit" } }); - await waitForPermission(permissionID); + // Workspace edits are covered by the stock build agent's wildcard allow + // and never produce a permission round-trip. emit({ type: "file.edited", sessionID, path: "generated.txt" }); + // A guard category (e.g. external_directory) reaches "ask". The companion + // must deny it; only an (incorrect) approval lets the gated edit proceed. + const permissionID = "perm_" + messageID; + emit({ + type: "permission.asked", + sessionID, + permissionID, + permission: { id: permissionID, tool: "edit" }, + patterns: ["/outside/workspace/secret.txt"] + }); + const reply = await waitForPermission(permissionID); + if (reply && reply.response !== "reject") { + emit({ type: "file.edited", sessionID, path: "/outside/workspace/secret.txt" }); + } } const finalText = prompt.includes("follow up") @@ -448,7 +461,7 @@ const server = http.createServer(async (req, res) => { saveState(state); const resolve = pendingPermissions.get(permissionID); pendingPermissions.delete(permissionID); - resolve?.(); + resolve?.(body); sendJson(res, { ok: true }); return; } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index c98e185..12fc5ad 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -659,7 +659,7 @@ test("foreground task runs through opencode serve and stores a visible session", } }); -test("write task auto-allows headless permission prompts and records touched files", { skip: LOCAL_LISTEN_SKIP }, () => { +test("write task denies gated permission asks and keeps stock agent guards (issue #26)", { skip: LOCAL_LISTEN_SKIP }, () => { const repo = makeTempDir(); const binDir = makeTempDir(); installFakeOpencode(binDir); @@ -674,17 +674,18 @@ test("write task auto-allows headless permission prompts and records touched fil assert.equal(result.status, 0, result.stderr); const payload = JSON.parse(result.stdout); + // Only the ungated workspace edit lands; the gated out-of-workspace edit + // stays denied, so it never shows up in touched files. assert.deepEqual(payload.touchedFiles, ["generated.txt"]); const fakeState = readFakeState(binDir); assert.equal(fakeState.sessions[0].agent, "build"); + // No session-level permission override: the stock build agent's ask-guards + // (external_directory, .env reads, doom_loop) must stay in effect. + assert.deepEqual(fakeState.sessions[0].permission, []); assert.equal(fakeState.permissions.length, 1); - assert.equal(fakeState.permissions[0].body.response, "always"); + assert.equal(fakeState.permissions[0].body.response, "reject"); assert.equal(fakeState.permissions[0].body.action, undefined); - assert.deepEqual( - fakeState.sessions[0].permission.filter((rule) => rule.action === "allow").map((rule) => rule.permission).sort(), - ["*"] - ); } finally { cleanupServer(repo, env); }