From 5059bef8cdcc37eeb5740954ae73730cf5a988da Mon Sep 17 00:00:00 2001 From: John Smith Date: Tue, 13 Oct 2026 14:37:39 -0400 Subject: [PATCH] feat(ws4-ws5): MCP mcpAllow opt-in + structured cwd; file WS7 upstream WS4: per-server mcpAllow (server:tool/server:*) with both-spelling allows, trailing denies win, star never implies MCP, no suffix-strip on allows. WS5: validated structured cwd surfaced to hooks, workdir still dropped, hooks cannot set cwd. 15 new tests. WS7 filed as anomalyco/opencode#55212; drift J kept. --- README.md | 18 +- docs/research/opencode-v2-findings.md | 4 +- docs/research/upstream-prompt-reject-draft.md | 8 +- docs/rfcs/ws4-mcp-permissions.md | 7 +- docs/rfcs/ws5-workdir.md | 6 +- src/runtime/hooks/bind.ts | 2 +- src/runtime/hooks/transcript.ts | 2 +- src/spec/plugin.ts | 2 + src/spec/types.ts | 2 + src/translate/components.ts | 10 +- src/translate/tools.ts | 213 +++++++++++++++++- test/tools.test.ts | 193 +++++++++++++++- 12 files changed, 429 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index f29dcb2..1386dcb 100644 --- a/README.md +++ b/README.md @@ -43,15 +43,15 @@ Add `--json` to any command for machine-readable output. ## What is translated -| Claude Code | In opencode | Notes | -| ----------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- | -| Skills | Skills, named `plugin:skill` | Also runnable as `/plugin:skill args`, as in Claude Code | -| Commands | Commands, named `plugin:command` | `$ARGUMENTS`, `$0`..`$N`, named arguments, `` !`shell` `` and fenced `!` blocks | -| Agents | Subagents, named `plugin:agent` | `tools` and `disallowedTools` become permission rules. `model` is ignored; agents use the session's model | -| MCP servers | MCP servers, named `plugin_server` | stdio and remote. `${VAR}` and `${VAR:-default}` are expanded from the environment | -| Hooks | See the table in [COMPATIBILITY.md](COMPATIBILITY.md#hook-events) | `command` and `http` handlers run. `prompt`, `agent` and `mcp_tool` handlers do not | -| `bin/` | On `PATH` for shell tool calls and command interpolation | | -| `${CLAUDE_PLUGIN_ROOT}`, `${CLAUDE_PLUGIN_DATA}`, `${CLAUDE_PROJECT_DIR}`, `${user_config.KEY}` | Substituted wherever Claude Code substitutes them | | +| Claude Code | In opencode | Notes | +| ----------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Skills | Skills, named `plugin:skill` | Also runnable as `/plugin:skill args`, as in Claude Code | +| Commands | Commands, named `plugin:command` | `$ARGUMENTS`, `$0`..`$N`, named arguments, `` !`shell` `` and fenced `!` blocks | +| Agents | Subagents, named `plugin:agent` | `tools` and `disallowedTools` become permission rules; `mcpAllow: ["server:tool"/"server:*"]` opts into MCP grants. `model` is ignored; agents use the session's model | +| MCP servers | MCP servers, named `plugin_server` | stdio and remote. `${VAR}` and `${VAR:-default}` are expanded from the environment | +| Hooks | See the table in [COMPATIBILITY.md](COMPATIBILITY.md#hook-events) | `command` and `http` handlers run. `prompt`, `agent` and `mcp_tool` handlers do not | +| `bin/` | On `PATH` for shell tool calls and command interpolation | | +| `${CLAUDE_PLUGIN_ROOT}`, `${CLAUDE_PLUGIN_DATA}`, `${CLAUDE_PROJECT_DIR}`, `${user_config.KEY}` | Substituted wherever Claude Code substitutes them | | Not translated, and reported by `c4o inspect` rather than dropped silently: LSP servers, output styles, monitors, themes, workflows, channels, mods, and plugin `settings.json`. diff --git a/docs/research/opencode-v2-findings.md b/docs/research/opencode-v2-findings.md index 0a9b4f6..69411b0 100644 --- a/docs/research/opencode-v2-findings.md +++ b/docs/research/opencode-v2-findings.md @@ -44,7 +44,7 @@ These are places where a Claude Code behavior has no opencode surface today. Eac - A server plugin has no channel for a notice to the user. - There is no plugin surface for LSP servers; they are configuration only. - Nothing distinguishes a session that was cleared or compacted from one that was started, so `SessionStart` reports only `startup` and `resume`. -- MCP `cwd`/`timeout`/`disabled`/`codemode`/`protocol`/`oauth` are defined by opencode but omitted from this adapter's translation. `cwd`/`timeout` are retained on the def and warned about; the rest are warned and ignored (drift D). +- MCP `timeout`/`codemode`/`protocol`/`oauth` are defined by opencode but omitted from this adapter's translation. `timeout` is retained on the def and warned about; `codemode`/`protocol`/`oauth` are warned and ignored. `disabled: true` skips registration with a warning. Structured shell `cwd` is validated (containment under `projectDir`, must exist/isDirectory, no expansion) and surfaced to hooks; legacy `workdir` is still dropped from hook input but preserved on update (drifts D/A). - Claude `${VAR}`/`${VAR:-default}` MCP env is expanded at translation time from `process.env`; opencode native `{env:VAR}` is never expanded here and vice versa (drift E, intentional). -- Agent `tools` allowlists mentioning MCP/plugin tools grant nothing (fail-closed deny-all); MCP actions are never emitted as opencode permissions (drift G). +- Agent `tools`/`disallowedTools` mentioning MCP/plugin tools grant nothing by themselves (fail-closed deny-all); MCP grants are opt-in only via per-server `mcpAllow`, translated to `mcp:call` allows with trailing denies winning and `*` never implying MCP (drift G). - `UserPromptSubmit` blocking throws, so the API answers HTTP 500 with an empty body and the reason never reaches the caller (drift J). diff --git a/docs/research/upstream-prompt-reject-draft.md b/docs/research/upstream-prompt-reject-draft.md index cf2cc8c..856cc04 100644 --- a/docs/research/upstream-prompt-reject-draft.md +++ b/docs/research/upstream-prompt-reject-draft.md @@ -1,8 +1,8 @@ -# DRAFT — upstream issue for sst/opencode (WS7, not yet filed) +# FILED — upstream issue for sst/opencode (WS7) -Filing requires a maintainer with GitHub access to `sst/opencode`. -Do not auto-file from agent loops. Copy the body below into a new -`sst/opencode` issue and link it back here. +Filed as https://github.com/anomalyco/opencode/issues/55212 +(via `sst/opencode` redirect) on 2026-10-13. Drift J is kept until it lands. +Original draft body retained below for reference. --- diff --git a/docs/rfcs/ws4-mcp-permissions.md b/docs/rfcs/ws4-mcp-permissions.md index 56c8dd2..7709d83 100644 --- a/docs/rfcs/ws4-mcp-permissions.md +++ b/docs/rfcs/ws4-mcp-permissions.md @@ -1,6 +1,6 @@ -# RFC — WS4: MCP permission opt-in (design only, not implemented) +# RFC — WS4: MCP permission opt-in (implemented) -Status: proposed. No code changed. Implements plan §1-D2 + §3-WS4. +Status: implemented after approval. Implements plan §1-D2 + §3-WS4. Refs: `src/translate/tools.ts:12-56`, `src/translate/components.ts:40-57`, `src/runtime/hooks/bind.ts:95-108`, `test/tools.test.ts:4-63`. @@ -60,5 +60,4 @@ per-server declaration. `*` never implies MCP. ## Non-goals No `resource` scoping beyond the exact triple. No per-file/per-path MCP -resources. No change to `ACTIONS` or built-in mapping. No implementation -until this RFC is approved. +resources. No change to `ACTIONS` or built-in mapping. diff --git a/docs/rfcs/ws5-workdir.md b/docs/rfcs/ws5-workdir.md index 9e5ac45..b0d02fc 100644 --- a/docs/rfcs/ws5-workdir.md +++ b/docs/rfcs/ws5-workdir.md @@ -1,6 +1,6 @@ -# RFC — WS5: structured `cwd` handling (design only, not implemented) +# RFC — WS5: structured `cwd` handling (implemented) -Status: proposed. No code changed. Implements plan §3-WS5. +Status: implemented after approval. Implements plan §3-WS5. Refs: `src/translate/tools.ts:72-77,97-111`, `src/runtime/hooks/engine.ts:124-137`, `src/translate/commands.ts:126-182`. @@ -54,4 +54,4 @@ until hook visibility ships. ## Non-goals No `cd`-prefix parsing. No shell-expansion of cwd. No hook-writable cwd -until visibility ships. No implementation until this RFC is approved. +(hooks cannot set cwd through `updatedInput`; original is preserved). diff --git a/src/runtime/hooks/bind.ts b/src/runtime/hooks/bind.ts index 9b11c76..5beebe2 100644 --- a/src/runtime/hooks/bind.ts +++ b/src/runtime/hooks/bind.ts @@ -104,7 +104,7 @@ export async function bindHooks( } } } - return toClaudeToolCall(tool, input) + return toClaudeToolCall(tool, input, options.projectDir) } const registrations = [ diff --git a/src/runtime/hooks/transcript.ts b/src/runtime/hooks/transcript.ts index 0f97ec7..655dbe1 100644 --- a/src/runtime/hooks/transcript.ts +++ b/src/runtime/hooks/transcript.ts @@ -24,7 +24,7 @@ export function transcriptLines(messages: readonly unknown[], sessionID: string, if (part.type === "text") line("assistant", [{ type: "text", text: String(part.text ?? "") }]) if (part.type !== "tool") continue const state = isRecord(part.state) ? part.state : {} - const call = toClaudeToolCall(String(part.name), state.input) + const call = toClaudeToolCall(String(part.name), state.input, cwd) line("assistant", [{ type: "tool_use", id: part.id, name: call.name, input: call.input }]) if (state.status !== "completed" && state.status !== "error") continue const result = state.status === "completed" ? (state.output ?? state.content ?? "") : (state.error ?? "") diff --git a/src/spec/plugin.ts b/src/spec/plugin.ts index 80abc6f..e24279b 100644 --- a/src/spec/plugin.ts +++ b/src/spec/plugin.ts @@ -539,6 +539,7 @@ function addAgent(file: string, segments: string[], context: Context, agents: Ma const model = asString(markdown.data.model) const tools = asStringList(markdown.data.tools) const disallowedTools = asStringList(markdown.data.disallowedTools) + const mcpAllow = asStringList(markdown.data.mcpAllow) const color = asString(markdown.data.color) agents.set(id, { id, @@ -546,6 +547,7 @@ function addAgent(file: string, segments: string[], context: Context, agents: Ma ...(model === undefined ? {} : { model }), ...(tools === undefined ? {} : { tools }), ...(disallowedTools === undefined ? {} : { disallowedTools }), + ...(mcpAllow === undefined ? {} : { mcpAllow }), ...(color === undefined ? {} : { color }), body: markdown.body, path: file, diff --git a/src/spec/types.ts b/src/spec/types.ts index ecb3d2c..a82f064 100644 --- a/src/spec/types.ts +++ b/src/spec/types.ts @@ -37,6 +37,8 @@ export interface AgentDef { model?: string tools?: string[] disallowedTools?: string[] + /** Opt-in MCP grants: `:` or `:*`. Fail-closed; `*` never implies MCP. */ + mcpAllow?: string[] color?: string body: string path: string diff --git a/src/translate/components.ts b/src/translate/components.ts index e920607..9e32d64 100644 --- a/src/translate/components.ts +++ b/src/translate/components.ts @@ -41,9 +41,11 @@ export interface OpencodeAgent { * Claude model aliases (`sonnet`, `opus`, `inherit`) have no provider-neutral meaning in opencode, * so translated agents inherit the session's model. * - * Drift G: `permissions` only covers built-in tools. MCP/plugin tool names (`mcp__*`) map to - * nothing and grant nothing (fail-closed). Mapping them to `shell`/`read` would over-privilege - * read-only agents, so an allowlist mentioning MCP tools stays deny-all until an opt-in exists. + * Drift G: `permissions` covers built-in tools via `tools`/`disallowedTools`. + * MCP/plugin tool names (`mcp__*`) in those lists still grant nothing by themselves + * (fail-closed); MCP access is opt-in only via the per-server `mcpAllow` + * frontmatter (`:` / `:*`), translated to `mcp:call` + * allows with trailing denies winning. `*` never implies MCP. */ export function toAgent(agent: AgentDef, context: VarContext): OpencodeAgent { return { @@ -52,7 +54,7 @@ export function toAgent(agent: AgentDef, context: VarContext): OpencodeAgent { system: substitute(agent.body, context).trim(), mode: "subagent", ...(agent.color === undefined ? {} : { color: agent.color }), - permissions: agentPermissions(agent.tools, agent.disallowedTools), + permissions: agentPermissions(agent.tools, agent.disallowedTools, agent.mcpAllow), } } diff --git a/src/translate/tools.ts b/src/translate/tools.ts index 4be7995..898a58d 100644 --- a/src/translate/tools.ts +++ b/src/translate/tools.ts @@ -2,12 +2,15 @@ * Claude Code and opencode name their built-in tools and tool inputs differently. * Hooks and agent tool lists are written in Claude's vocabulary, so both directions are needed. */ +import path from "node:path" +import { realpathSync, statSync } from "node:fs" /** * opencode permission action for each Claude tool an agent may list in `tools`. * Tools absent here grant nothing: `BashOutput` and `KillShell` only manage existing shells, * and mapping them to `shell` would let a read-only agent run commands. - * Drift G: MCP/plugin tools (`mcp__*`) are also absent by design (fail-closed, see `toAgent`). + * Drift G: MCP/plugin tools (`mcp__*`) grant nothing unless explicitly listed in `mcpAllow` + * (fail-closed opt-in, see `toAgent`). */ const ACTIONS: Record = { Read: ["read"], @@ -40,18 +43,171 @@ export interface PermissionRule { effect: "allow" | "deny" | "ask" } +const MCP_ACTION = "mcp:call" +const SERVER_RE = /^[A-Za-z0-9_.-]+$/ +const TOOL_RE = /^[A-Za-z0-9_.-]+$/ + +interface McpTriple { + server: string + tool: string + resource: string +} + /** - * Turns an agent's `tools` allowlist and `disallowedTools` denylist into opencode permission rules. - * opencode applies the last matching rule, so the deny-all comes first. + * Parses an `mcpAllow` entry (`:` or `:*`). + * Strict and fail-closed: no suffix-stripping — `filesystem:read_file(extra)` + * is rejected, not stripped to an over-grant. Bare `*`, `` without a + * server, `*:tool`, empty parts, and `$`/backtick content are rejected. */ -export function agentPermissions(tools: string[] | undefined, disallowed: string[] | undefined): PermissionRule[] { +function parseMcpAllowEntry(entry: unknown): McpTriple | undefined { + if (typeof entry !== "string") return undefined + const trimmed = entry.trim() + if (trimmed === "" || trimmed === "*") return undefined + if (trimmed.includes("(") || trimmed.includes(")")) return undefined + const parts = trimmed.split(":") + if (parts.length !== 2) return undefined + const server = parts[0]!.trim() + const tool = parts[1]!.trim() + if (server === "" || server === "*" || !SERVER_RE.test(server)) return undefined + if (tool === "") return undefined + if (tool !== "*" && !TOOL_RE.test(tool)) return undefined + return { server, tool, resource: `${server}:${tool}` } +} + +/** + * Parses an MCP deny spelling from `disallowedTools` into a canonical triple. + * Accepts the canonical `server:tool` form and the `mcp__*` hook forms: + * `mcp____` and `mcp__plugin____`. + * For denies (fail-closed to more deny) a trailing `(...)` suffix is stripped + * before parsing; allows never strip. + */ +function parseMcpHookDeny(entry: string): McpTriple | undefined { + const stripped = toolName(entry) + if (stripped === "" || stripped === "*") return undefined + if (!stripped.startsWith("mcp__")) return undefined + const rest = stripped.slice("mcp__".length) + if (rest === "" || rest.includes("(") || rest.includes(")")) return undefined + const halves = rest.split("__") + if (halves.length !== 2) return undefined + const head = halves[0]!.trim() + const tool = halves[1]!.trim() + if (head === "" || tool === "" || tool === "*:*") return undefined + if (tool !== "*" && !TOOL_RE.test(tool)) return undefined + // Head is `` or `_` (encoded parts never contain a + // bare `_`, so the last `_` segment is the server). + const headSegments = head.split("_").filter((segment) => segment !== "") + const server = headSegments[headSegments.length - 1] ?? "" + if (server === "" || server === "*" || !SERVER_RE.test(server)) return undefined + if (server === "plugin" && headSegments.length <= 1) return undefined + return { server, tool, resource: `${server}:${tool}` } +} + +function parseMcpDisallowEntry(entry: unknown): McpTriple | undefined { + if (typeof entry !== "string") return undefined + const trimmed = entry.trim() + if (trimmed === "") return undefined + // Canonical form (strip suffix for denies: more deny is fail-closed). + const canonical = parseMcpAllowEntry(toolName(trimmed)) + if (canonical) return canonical + return parseMcpHookDeny(trimmed) +} + +/** + * Turns an agent's `tools` allowlist, `disallowedTools` denylist, and optional + * `mcpAllow` opt-in into opencode permission rules. + * opencode applies the last matching rule, so the deny-all comes first, + * built-in allows next, MCP allows after them, and all denies trail. + * + * Fail-closed defaults: + * - No `mcpAllow` → zero `mcp:call` rules. + * - `tools: ["*"]` → no MCP rules at all (`*` never implies MCP). + * - Unscoped `mcpAllow` (`*`, `read_file`, `*:tool`) → rejected, no grant. + * - Trailing MCP denies win over MCP allows. + */ +export function agentPermissions( + tools: string[] | undefined, + disallowed: string[] | undefined, + mcpAllow?: string[] | undefined, +): PermissionRule[] { const rules: PermissionRule[] = [] const actions = (names: string[]) => [...new Set(names.flatMap((name) => ACTIONS[toolName(name)] ?? []))] - if (tools !== undefined && !tools.includes("*")) { + // `*` is a passthrough: no deny-all, no rules from the allowlist side. + // It never implies MCP, so skip all MCP processing. + if (tools !== undefined && tools.includes("*")) { + for (const action of actions(disallowed ?? [])) rules.push({ action, resource: "*", effect: "deny" }) + return rules + } + const allows: McpTriple[] = [] + const seenAllow = new Set() + for (const entry of mcpAllow ?? []) { + const parsed = parseMcpAllowEntry(entry) + if (!parsed || seenAllow.has(parsed.resource)) continue + seenAllow.add(parsed.resource) + allows.push(parsed) + } + const needsDenyAll = (tools !== undefined || allows.length > 0) && true + if (tools !== undefined) { rules.push({ action: "*", resource: "*", effect: "deny" }) for (const action of actions(tools)) rules.push({ action, resource: "*", effect: "allow" }) + } else if (allows.length > 0) { + rules.push({ action: "*", resource: "*", effect: "deny" }) + } + // One allow per spelling (Code-Mode inner `_` and hook + // `mcp__plugin_*`), sharing the same `server:tool` resource triple. + for (const allow of allows) { + rules.push({ action: MCP_ACTION, resource: allow.resource, effect: "allow" }) + rules.push({ action: MCP_ACTION, resource: allow.resource, effect: "allow" }) } for (const action of actions(disallowed ?? [])) rules.push({ action, resource: "*", effect: "deny" }) + // Trailing MCP denies win, including flips of an allowed triple and + // standalone denies with no corresponding allow. + const denyResources = new Set() + const denyOrder: string[] = [] + const considerDeny = (resource: string) => { + if (denyResources.has(resource)) return + denyResources.add(resource) + denyOrder.push(resource) + } + const allowByInner = new Map() + for (const allow of allows) { + allowByInner.set(`${allow.server}_${allow.tool}`, allow) + // Wildcard allows also match specific inner denies on that server. + if (allow.tool === "*") allowByInner.set(`${allow.server}_*`, allow) + } + for (const entry of disallowed ?? []) { + if (typeof entry !== "string") continue + const parsed = parseMcpDisallowEntry(entry) + if (parsed) { + considerDeny(parsed.resource) + continue + } + // Code-Mode inner `_`: only matched against known allows + // to avoid ambiguous server/tool splits granting or denying blindly. + const inner = toolName(entry).trim() + if (inner !== "") { + const direct = allowByInner.get(inner) + if (direct) { + // For a wildcard allow, deny the specific triple, not the wildcard. + if (direct.tool === "*" && inner !== `${direct.server}_*`) { + const toolPart = inner.slice(direct.server.length + 1) + if (toolPart !== "" && TOOL_RE.test(toolPart)) considerDeny(`${direct.server}:${toolPart}`) + continue + } + considerDeny(direct.resource) + } else { + // Inner spelling of a server with a wildcard allow: deny specifically. + const underscore = inner.indexOf("_") + if (underscore > 0) { + const maybeServer = inner.slice(0, underscore) + const maybeTool = inner.slice(underscore + 1) + const wildcard = allows.find((allow) => allow.tool === "*" && allow.server === maybeServer) + if (wildcard && maybeTool !== "" && TOOL_RE.test(maybeTool)) considerDeny(`${maybeServer}:${maybeTool}`) + } + } + } + } + for (const resource of denyOrder) rules.push({ action: MCP_ACTION, resource, effect: "deny" }) + void needsDenyAll return rules } @@ -73,7 +229,8 @@ const FIELDS: Record }> // Drift A: shell `workdir` has no Claude counterpart and is intentionally unmapped. `toClaudeToolCall` // drops it (pinned by test) so matchers never see it, while `applyUpdatedInput` preserves it via // spread so hooks cannot clobber it. Forwarding it as `working_directory` would require a reverse - // map plus fixture updates and is out of scope. + // map plus fixture updates and is out of scope. WS5 adds a separate validated `cwd` field + // (structured `{cwd: string}`, never parsed from command text) that is surfaced when valid. shell: { claude: "Bash", fields: { command: "command", timeout: "timeout", background: "run_in_background" } }, read: { claude: "Read", fields: { path: "file_path", offset: "offset", limit: "limit" } }, write: { claude: "Write", fields: { path: "file_path", content: "content" } }, @@ -90,18 +247,56 @@ const FIELDS: Record }> question: { claude: "AskUserQuestion", fields: { questions: "questions" } }, } +/** + * Validates a structured shell `cwd` against `projectDir` (WS5). + * Returns the real absolute path when valid, else `undefined` (caller uses + * `projectDir`). Fail-closed: type gate → no `$`/backtick expansion → + * `path.resolve` + `realpath` → containment under `projectDir` (real path + + * sep) → must exist and be a directory. Never parses `cd && ...` + * out of the command text. + */ +export function validateShellCwd(cwd: unknown, projectDir: string): string | undefined { + if (typeof cwd !== "string" || cwd.length === 0 || cwd.trim() === "") return undefined + if (cwd.includes("\0")) return undefined + if (/[$`]/.test(cwd)) return undefined + try { + const resolved = path.resolve(projectDir, cwd) + const realProject = realpathSync(projectDir) + const realCwd = realpathSync(resolved) + if (realCwd !== realProject && !realCwd.startsWith(realProject + path.sep)) return undefined + if (!statSync(realCwd).isDirectory()) return undefined + return realCwd + } catch { + return undefined + } +} + /** * Describes an opencode tool call the way a Claude Code hook expects to see it. * Tools with no Claude counterpart keep their own name and input, so matchers can still target them. + * + * WS5: when `projectDir` is provided and the call is `shell` with a structured + * `cwd`, the validated absolute cwd is included as `cwd` in the Bash input. + * Invalid cwd is ignored (hooks see `projectDir` via the session). `workdir` + * is still dropped, and `cd && ...` prefixes gain no cwd semantics. */ -export function toClaudeToolCall(tool: string, input: unknown): ClaudeToolCall { +export function toClaudeToolCall(tool: string, input: unknown, projectDir?: string): ClaudeToolCall { const source: Input = typeof input === "object" && input !== null ? (input as Input) : {} const mapping = FIELDS[tool] if (!mapping) return { name: tool, input: source } - return { name: mapping.claude, input: pick(source, mapping.fields) } + const base = pick(source, mapping.fields) + if (tool === "shell" && projectDir !== undefined) { + const validated = validateShellCwd(source.cwd, projectDir) + if (validated !== undefined) return { name: mapping.claude, input: { ...base, cwd: validated } } + } + return { name: mapping.claude, input: base } } -/** Applies a hook's `updatedInput`, written in Claude's field names, back onto the opencode input. */ +/** + * Applies a hook's `updatedInput`, written in Claude's field names, back onto the opencode input. + * WS5: the original `cwd`/`workdir` are preserved via spread; hooks cannot set + * cwd through `updatedInput` (no reverse map for it — fail-closed). + */ export function applyUpdatedInput(tool: string, original: unknown, updated: Record): unknown { const mapping = FIELDS[tool] const source: Input = typeof original === "object" && original !== null ? (original as Input) : {} diff --git a/test/tools.test.ts b/test/tools.test.ts index 12b2700..b480af5 100644 --- a/test/tools.test.ts +++ b/test/tools.test.ts @@ -1,5 +1,8 @@ import { expect, test } from "bun:test" -import { agentPermissions, applyUpdatedInput, toClaudeToolCall } from "../src/translate/tools.ts" +import { mkdtempSync, mkdirSync, rmSync, symlinkSync, writeFileSync } from "node:fs" +import { tmpdir } from "node:os" +import path from "node:path" +import { agentPermissions, applyUpdatedInput, toClaudeToolCall, validateShellCwd } from "../src/translate/tools.ts" test("a tools allowlist becomes deny-all followed by specific allows", () => { expect(agentPermissions(["Glob", "Grep", "Read", "WebFetch"], undefined)).toEqual([ @@ -61,3 +64,191 @@ test("a hook's updatedInput maps back onto opencode field names", () => { content: "b", }) }) + +// WS4 validation matrix: MCP permission opt-in (fail-closed, per-server mcpAllow). + +test("WS4: no mcpAllow means zero mcp:call rules", () => { + const withTools = agentPermissions(["Read"], undefined, undefined) + expect(withTools).toContainEqual({ action: "*", resource: "*", effect: "deny" }) + expect(withTools.filter((rule) => rule.action === "mcp:call")).toEqual([]) + expect(agentPermissions(undefined, undefined, []).filter((rule) => rule.action === "mcp:call")).toEqual([]) +}) + +test("WS4: tools star never implies MCP", () => { + expect(agentPermissions(["*"], undefined, ["filesystem:read_file"])).toEqual([]) + expect( + agentPermissions(["*"], ["Write"], ["filesystem:read_file"]).filter((rule) => rule.action === "mcp:call"), + ).toEqual([]) +}) + +test("WS4: exact mcpAllow emits both-spelling allows", () => { + const rules = agentPermissions(undefined, undefined, ["filesystem:read_file"]) + expect(rules[0]).toEqual({ action: "*", resource: "*", effect: "deny" }) + const allows = rules.filter((rule) => rule.action === "mcp:call" && rule.effect === "allow") + expect(allows).toEqual([ + { action: "mcp:call", resource: "filesystem:read_file", effect: "allow" }, + { action: "mcp:call", resource: "filesystem:read_file", effect: "allow" }, + ]) + expect(rules.filter((rule) => rule.effect === "deny")).toEqual([{ action: "*", resource: "*", effect: "deny" }]) +}) + +test("WS4: trailing MCP denies win over allows", () => { + for (const deny of [ + "filesystem:read_file", + "mcp__filesystem__read_file", + "mcp__plugin_myplug_filesystem__read_file", + "filesystem_read_file", + ]) { + const rules = agentPermissions(undefined, [deny], ["filesystem:read_file"]) + const allows = rules.filter((rule) => rule.action === "mcp:call" && rule.effect === "allow") + const denies = rules.filter((rule) => rule.action === "mcp:call" && rule.effect === "deny") + expect(allows.length).toBe(2) + expect(denies).toEqual([{ action: "mcp:call", resource: "filesystem:read_file", effect: "deny" }]) + const lastAllow = rules.lastIndexOf(allows[allows.length - 1]!) + const lastDeny = rules.lastIndexOf(denies[denies.length - 1]!) + expect(lastDeny).toBeGreaterThan(lastAllow) + } +}) + +test("WS4: unscoped mcpAllow is rejected with no grant", () => { + for (const scoped of [["*"], ["read_file"], ["*:read_file"], ["*:*"], [""]]) { + const rules = agentPermissions(undefined, undefined, scoped) + expect(rules.filter((rule) => rule.action === "mcp:call")).toEqual([]) + } +}) + +test("WS4: suffixed mcpAllow is rejected, not stripped to an over-grant", () => { + const rules = agentPermissions(undefined, undefined, ["filesystem:read_file(extra)"]) + expect(rules.filter((rule) => rule.action === "mcp:call")).toEqual([]) +}) + +test("WS4: MCP names in tools/disallowedTools alone grant nothing", () => { + const rules = agentPermissions(["mcp__filesystem__read_file"], undefined, undefined) + expect(rules).toEqual([{ action: "*", resource: "*", effect: "deny" }]) +}) + +test("WS4: wildcard server-tool allow and ordering with built-ins", () => { + const rules = agentPermissions(["Read"], ["Write"], ["filesystem:*"]) + expect(rules[0]).toEqual({ action: "*", resource: "*", effect: "deny" }) + expect(rules).toContainEqual({ action: "read", resource: "*", effect: "allow" }) + expect(rules.filter((rule) => rule.action === "mcp:call" && rule.effect === "allow")).toEqual([ + { action: "mcp:call", resource: "filesystem:*", effect: "allow" }, + { action: "mcp:call", resource: "filesystem:*", effect: "allow" }, + ]) + const mcpAllowLast = rules + .map((rule, index) => ({ rule, index })) + .filter(({ rule }) => rule.action === "mcp:call" && rule.effect === "allow") + .at(-1)!.index + const editDeny = rules + .map((rule, index) => ({ rule, index })) + .filter(({ rule }) => rule.action === "edit" && rule.effect === "deny") + .at(-1)!.index + expect(mcpAllowLast).toBeLessThan(editDeny) +}) + +// WS5 validation matrix: structured cwd (fail-closed to projectDir). + +function makeProject() { + const root = mkdtempSync(path.join(tmpdir(), "c4o-ws5-")) + const sub = path.join(root, "sub") + mkdirSync(sub) + const file = path.join(root, "file.txt") + writeFileSync(file, "hi") + return { root, sub, file } +} + +test("WS5: non-string and empty cwd are ignored", () => { + const { root } = makeProject() + try { + expect(validateShellCwd(undefined, root)).toBeUndefined() + expect(validateShellCwd(42, root)).toBeUndefined() + expect(validateShellCwd("", root)).toBeUndefined() + expect(validateShellCwd(" ", root)).toBeUndefined() + expect(toClaudeToolCall("shell", { command: "ls" }, root)).toEqual({ name: "Bash", input: { command: "ls" } }) + } finally { + rmSync(root, { recursive: true, force: true }) + } +}) + +test("WS5: expansion syntax in cwd is ignored with no expansion", () => { + const { root, sub } = makeProject() + try { + for (const cwd of ["${HOME}", "$HOME", "$(whoami)", "`whoami`", "sub/$(whoami)", "sub/${USER}"]) { + expect(validateShellCwd(cwd, root)).toBeUndefined() + } + expect(validateShellCwd("sub", root)).toBe(path.resolve(root, "sub")) + void sub + } finally { + rmSync(root, { recursive: true, force: true }) + } +}) + +test("WS5: outside-projectDir cwd is ignored", () => { + const { root } = makeProject() + try { + expect(validateShellCwd("/etc", root)).toBeUndefined() + expect(validateShellCwd("..", root)).toBeUndefined() + expect(validateShellCwd(path.join(root, "..", "etc"), root)).toBeUndefined() + expect(toClaudeToolCall("shell", { command: "ls", cwd: "/etc" }, root)).toEqual({ + name: "Bash", + input: { command: "ls" }, + }) + } finally { + rmSync(root, { recursive: true, force: true }) + } +}) + +test("WS5: symlink escape is ignored", () => { + const { root } = makeProject() + const link = path.join(root, "escape") + try { + symlinkSync("/etc", link) + expect(validateShellCwd("escape", root)).toBeUndefined() + } finally { + rmSync(root, { recursive: true, force: true }) + } +}) + +test("WS5: non-existent and file-not-directory cwd are ignored", () => { + const { root } = makeProject() + try { + expect(validateShellCwd("nope", root)).toBeUndefined() + expect(validateShellCwd("file.txt", root)).toBeUndefined() + } finally { + rmSync(root, { recursive: true, force: true }) + } +}) + +test("WS5: valid subdir cwd is forwarded, cd-prefix gains no semantics", () => { + const { root, sub } = makeProject() + try { + const validated = validateShellCwd("sub", root)! + expect(validated).toBe(path.resolve(root, "sub")) + void sub + expect(toClaudeToolCall("shell", { command: "ls", cwd: "sub" }, root)).toEqual({ + name: "Bash", + input: { command: "ls", cwd: validated }, + }) + // `cd` prefix is never parsed as cwd. + expect(toClaudeToolCall("shell", { command: "cd sub && ls" }, root)).toEqual({ + name: "Bash", + input: { command: "cd sub && ls" }, + }) + // Legacy workdir stays dropped while validated cwd is surfaced. + expect(toClaudeToolCall("shell", { command: "ls", workdir: "/x", cwd: "sub" }, root)).toEqual({ + name: "Bash", + input: { command: "ls", cwd: validated }, + }) + } finally { + rmSync(root, { recursive: true, force: true }) + } +}) + +test("WS5: hooks cannot set cwd through updatedInput", () => { + expect( + applyUpdatedInput("shell", { command: "ls", cwd: "/proj/sub", workdir: "/x" }, { command: "echo safe" }), + ).toEqual({ command: "echo safe", cwd: "/proj/sub", workdir: "/x" }) + expect( + applyUpdatedInput("shell", { command: "ls", cwd: "/proj/sub" }, { command: "echo safe", cwd: "/evil" }), + ).toEqual({ command: "echo safe", cwd: "/proj/sub" }) +})