diff --git a/README.md b/README.md index 6d41aee..aaf0ad9 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ agent: default thinking: medium cwd: /home/you/project skills: debug, refactor, review, test plugins: guard, time -approvals: on for write_file, edit_file, multi_edit, bash, mcp__* +approvals: ask for write_file, edit_file, multi_edit, bash, mcp__* /help for commands > why does the pagination test fail? @@ -64,11 +64,12 @@ installed and honours `.gitignore`. `list_dir` gives an ignore-aware tree so it blindly to orient, and `read_many_files` pulls a batch in one round trip. `read_file` refuses binaries rather than filling the context with mojibake. -**Edits with your approval.** Every `write_file`, `edit_file`, `multi_edit`, and `bash` call -stops for a `y`/`a`/`n` decision, with a coloured diff for edits. `multi_edit` is atomic, so -a failing match leaves the file untouched rather than half-changed. The `guard` plugin -refuses irreversible commands outright — `rm -rf`, `git reset --hard`, force pushes, -`DROP TABLE` — and `--yolo` cannot bypass it. +**Edits with your approval, gated per command.** `write_file`, `edit_file`, `multi_edit`, and +`bash` stop for a `y`/`a`/`n` decision, with a coloured diff for edits. Rules match the command or +path rather than the tool, so `git *` can run unprompted while everything else still asks — +answering `a` whitelists that pattern, not the whole tool. `.env` and `.pem` files are refused on +read outright. The `guard` plugin refuses irreversible commands ahead of any of it — `rm -rf`, +`git reset --hard`, force pushes, `DROP TABLE` — and `--yolo` cannot bypass it. **Shows its work.** Reasoning streams to a collapsed panel you can expand with `ctrl-r`, the tool in flight is named as it runs, and `bash` output streams live instead of arriving all at @@ -113,6 +114,7 @@ the flags are. |---|---| | [Configuration](docs/configuration.md) | config file, provider presets, environment, every flag | | [Tools](docs/tools.md) | every tool, tool sets and what they cost, the approval model | +| [Permissions](docs/permissions.md) | allow/ask/deny rules, patterns, defaults, the repeat guard | | [Agents and thinking](docs/agents.md) | variants, thinking levels, step caps, which to reach for | | [Skills](docs/skills.md) | the bundled skills, writing your own, why the catalogue is split | | [Plugins](docs/plugins.md) | the interface, the guard and its limits, builtin versus installed | diff --git a/ROADMAP.md b/ROADMAP.md index 3dcbdd9..fc49e1d 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -115,15 +115,48 @@ outright — a plugin that can block tool calls could otherwise lie about blocki thirds and red at 90, so a turn about to lose history says so first. Aligned command menu and registry tables, and `/skills` and `/plugins` name the origin of every entry. +**Permission rules.** Approval moved from a list of tool names to rules matched against the +call's subject: the command for `bash`, the path for a file tool. `bash` used to be a single +yes/no covering `git status` and `rm -rf`, so a user pressing `a` once during a batch removed the +gate for both — the check was strongest when it mattered least. Rules let `git *` run while +everything else asks, `always` grants the pattern rather than the tool, `.env` and `.pem` are +refused on read outright, and a call repeated identically three times in one turn asks even when +allowed. `--yolo` folds `ask` into `allow` and still cannot reach a deny rule or the guard. + +Surveyed Claude Code, Codex, opencode, and phi before writing it. Three of the four had already +moved to per-pattern rules; the shape here is closest to opencode's, with the credential deny and +the repeat guard taken from it directly. + --- ## Next +### MCP without the schema tax + +Every MCP tool's schema is in the prompt on every request, and `toolSets` does not gate them: a +twenty-tool server costs roughly 2,750 tokens a turn whether the model touches it or not. phi's +answer is three meta-tools — `mcp_list`, `mcp_inspect`, `mcp_call` — with the prompt naming only +the servers, so a hundred servers cost almost nothing until one is called. Worth keeping direct +registration as an option: for a two-tool server the indirection is the more expensive of the two. + +### Custom commands from a file + +A markdown file becoming a slash command, with `$ARGUMENTS`, `$1`, `` !`cmd` `` for shell output, +and `@path` for a file. Every comparable CLI has this and none of it is hard; it is missing because +nothing forced the issue. + +### Undo a turn + +opencode has `/undo` and `/redo`, Claude Code has `/rewind` over file checkpoints. There is +`/resume` here, which restores a whole session, and nothing that steps one turn back. The honest +limit is the same for everyone: a `bash` command's effects cannot be snapshotted, so this covers +file-tool edits and says so. + ### Lossless-enough compaction -Compaction keeps the model's memory of a turn now, but it still says nothing about the messages -it discarded, so the model can contradict its own earlier decision with confidence. A summary of -the discarded span costs one cheap call and removes the whole class of problem. +Compaction keeps the model's memory of a turn now, but it still says nothing about the messages it +discarded, so the model can contradict its own earlier decision with confidence. A summary of the +discarded span costs one cheap call and removes the whole class of problem. ### Cost control @@ -174,6 +207,19 @@ agent can read. step for task-list freshness, which defeats a naive cache; splitting the stable prefix from the volatile suffix would fix that. +**External hooks.** phi and both first-party CLIs let a script sit in the tool loop: a directory +with a manifest and an executable, one JSON object in on stdin, one out. phi's `pre_tool` can +rewrite the tool's input as well as allow or deny, which the compiled plugin interface here cannot +express. The reason it is here rather than in Next is that it needs a trust story — Codex hashes +each hook and refuses to run one until you review it, which is the right shape and more work than +the feature. + +**OS-level sandboxing.** The strongest thing in this class, and Codex is the one that has it: +Seatbelt on macOS, Landlock and seccomp on Linux, a separate mechanism on Windows, with network +egress governed by domain rules. Permission rules gate the *call*; a sandbox governs what the +process can then reach. Three platform-specific implementations, and OpenAI moved Codex to Rust +partly for this. A half-built sandbox is worse than none, because people would trust it. + --- ## Declined @@ -193,3 +239,14 @@ worse answer on a codebase that fits in a grep. **Tool call retries on model error.** A model that produced a malformed call will usually produce it again. Surfacing the error teaches it more than a silent retry. + +**A client/server split.** opencode runs a server and treats its TUI as one client of an OpenAPI +endpoint, which is what lets IDE extensions and a web client exist. It is the right architecture +for that product. Here it would add a protocol, a port, and an auth story to serve a second client +nobody has asked for. + +**LSP integration.** opencode ships it and its own documentation says the honest thing: +*"not always a net positive... in many projects it is better to have the agent run lint, typecheck, +or other diagnostic CLI tools directly."* Language servers drift out of sync, use real memory, and +vary by version. `bash bun run typecheck` puts the same errors in front of the model with none of +that, and `AGENTS.md` is where the command belongs. diff --git a/TODO.md b/TODO.md index bce7fb3..239d6e1 100644 --- a/TODO.md +++ b/TODO.md @@ -50,6 +50,31 @@ are both assembled at boot, so a mid-session install does nothing until then. ## Next +### MCP without the schema tax + +Every MCP tool's schema goes into the prompt today, so twenty tools from one server cost roughly +2,750 tokens per request whether the model uses them or not. `toolSets` does not gate them. + +phi solves this with three meta-tools — `mcp_list`, `mcp_inspect`, `mcp_call` — and a prompt that +names only the servers. A hundred servers then cost almost nothing until one is called. + +- [ ] `mcp_list` / `mcp_inspect` / `mcp_call` replacing per-tool registration +- [ ] The prompt lists server names, not schemas +- [ ] Calls go through the same permission rules and guard as a built-in +- [ ] Keep per-tool registration as an option: a two-tool server is cheaper registered directly +- [ ] Test: a configured server contributes no schema to the request until `mcp_call` + +### Custom commands from a file + +Every other CLI in this class has these and they are cheap: a markdown file becomes a slash +command, with `$ARGUMENTS`, `$1`, `` !`cmd` `` for shell output, and `@path` for a file. + +- [ ] `.shiro/commands/*.md` and `~/.shiro-neko/commands/*.md`, name from the filename +- [ ] Frontmatter for `description` and `agent` +- [ ] `$ARGUMENTS` and positional `$1` +- [ ] `` !`cmd` `` substituted before the prompt is sent, with the guard applied to it +- [ ] Test: a command with a shell substitution reaches the model with the output inlined + ### `web_fetch` - [ ] URL to markdown, size-capped @@ -74,6 +99,16 @@ does not fan out. - [ ] `task` accepts several investigations and runs them together - [ ] Test: two delegated searches overlap in time rather than queueing +### Undo a turn + +Every comparable CLI has this: opencode `/undo` and `/redo`, Claude Code `/rewind` with +checkpoints. There is `/resume` here, which restores a session, and nothing that walks one back. + +- [ ] Snapshot files before each prompt, capped at the 100 most recent +- [ ] `/undo` restores files, conversation, or both; `/redo` reverses it +- [ ] Say plainly what is not covered: a `bash` command's effects cannot be snapshotted +- [ ] Test: an edit is reverted, and the model's own record of it goes with it + --- ## Maintenance @@ -84,6 +119,8 @@ does not fan out. real tokenizer - [ ] `listPaths` walks up to 5000 files once per session. Fine for a repo, wasteful in a monorepo, and it never notices a file created after the first `@` +- [ ] `MUTATING_TOOLS` is now only used by tests and docs; the permission defaults are what + actually gate a write. Either delete it or make the defaults derive from it --- @@ -100,6 +137,10 @@ Not bugs exactly, but things that will bite someone. fail on the other. The prompt states the platform; it does not translate. - **An unknown name in `toolSets` is dropped silently.** The header line shows which sets actually loaded, but a typo reads as "that set is off" rather than as a mistake. +- **Permission rules gate the call, not what it does.** `bash` with `git *` allowed will run a + `git` alias that shells out to anything, and there is no sandbox around the shell. Codex solves + this with OS-level isolation — Seatbelt, Landlock, a Windows equivalent — which is three + platform-specific implementations and not something to half-ship. - **The reasoning panel is per-turn, not per-step.** Reasoning from an early step stays on screen through later ones until the turn ends. - **An interrupted command's effects are unknown, and the model is told so.** Nothing can know @@ -139,3 +180,8 @@ Kept for one release, then deleted. - [x] `/registry`: browse, search, install, and remove external skills and plugins. Skills are shown in full before install; plugins are a validated manifest of deny rules, never code - [x] Context shown as a percentage of the compaction threshold, amber at two thirds, red at 90 +- [x] **Permission rules per command and path**, replacing the per-tool list. `bash` was one + yes/no for `git status` and `rm -rf`, so pressing `a` once removed the gate for both. + Rules match the call's subject, `always` grants a pattern rather than the tool, `.env` and + `.pem` are refused on read, and an identical call repeated three times in a turn asks even + when allowed diff --git a/docs/configuration.md b/docs/configuration.md index b1a661a..f9c8b86 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -22,6 +22,9 @@ Written by `/provider`, editable by hand. Every field is optional. "maxRetries": 3, "plugins": ["guard", "time"], "toolSets": ["edit-plus", "git"], + "permission": { + "bash": { "*": "ask", "git *": "allow" } + }, "registryUrl": "https://example.com/my-registry/index.json", "mcpServers": { "fs": { "command": "npx", "args": ["-y", "@modelcontextprotocol/server-filesystem", "."] } @@ -41,6 +44,7 @@ Written by `/provider`, editable by hand. Every field is optional. | `maxRetries` | retries per model call for transient failures. Default 3 | | `plugins` | which builtin plugins to enable. Omit for `["guard", "time"]` | | `toolSets` | optional tool sets beyond `core`: `edit-plus`, `git`. Omit for all of them. See [tools](tools.md) | +| `permission` | which calls run, ask, or are refused, matched per command or path. See [permissions](permissions.md) | | `registryUrl` | index for `/registry`. Omit for the default. See [registry](registry.md) | | `mcpServers` | see [MCP](mcp.md) | @@ -212,7 +216,7 @@ interesting — see [memory](memory.md#the-pruning-repair). ## Config that changes behaviour subtly -Three fields do more than they look like they do. +Four fields do more than they look like they do. **`thinking`** costs money and latency on every turn, not just hard ones. `off` on a hard problem produces confident wrong answers; `max` on a rename wastes cents and seconds. The agent variants @@ -222,6 +226,10 @@ already pick sensible levels — see [agents](agents.md). expected, check the startup header for which sets loaded: an unrecognised name is dropped silently, so `"gti"` reads as "git is off". See [tools](tools.md#tool-sets). +**`permission`** replaces a tool's defaults rather than merging with them, so +`{ "read_file": { "*": "allow" } }` also allows reading `.env`. Order inside a rule table decides +the outcome, since later rules win. See [permissions](permissions.md). + **`registryUrl`** is the whole trust decision for installed skills and plugins. There are no signatures, so pointing it at an index means trusting whoever controls that URL — including for whatever they publish later. See [registry](registry.md). diff --git a/docs/permissions.md b/docs/permissions.md new file mode 100644 index 0000000..b7c3cc2 --- /dev/null +++ b/docs/permissions.md @@ -0,0 +1,253 @@ +# Permissions + +Which tool calls run, which stop to ask, and which are refused. + +```json +{ + "permission": { + "bash": { "*": "ask", "git *": "allow", "bun test": "allow" }, + "edit_file": { "*": "ask", "src/generated/*": "deny" }, + "read_file": { "*": "allow", "*.env": "deny" } + } +} +``` + +Three decisions: `allow` runs it, `ask` prompts, `deny` refuses without asking. + +## Why rules match the command, not the tool + +The obvious design gates by tool name: `bash` needs approval, `read_file` does not. That is what +this used to do, and it fails for a reason worth stating. + +`bash` covers `git status` and `rm -rf` equally. A user working through a batch of edits presses +`a` — always allow — on the first prompt, and every later command runs unasked, including the one +they would have refused. The gate was strongest at the moment it mattered least and gone by the +time it mattered. + +Rules match the **subject** of a call: the command for `bash`, the path for anything touching a +file, the pattern for a search. `git *` can be allowed while `*` still asks, so the prompts that +remain are the ones worth reading. + +## Subjects per tool + +| Tool | Matched against | +|---|---| +| `bash` | the command, e.g. `git status --porcelain` | +| `read_file` `write_file` `edit_file` `multi_edit` `list_dir` | the path | +| `read_many_files` | every path in the batch; one match is enough | +| `glob` `grep` | the pattern | +| `git_diff` `git_log` `git_blame` | the path, when given | +| `git_show` | the ref | +| `task` | the subagent description | +| `skill` | the skill name | +| everything else | `*` only | + +A tool with no subject — `git_status` takes no arguments — matches `*` and nothing narrower. That +is why a rule for it is a plain decision rather than a pattern table: + +```json +{ "permission": { "git_status": "allow" } } +``` + +## Patterns + +`*` spans any characters including newlines, `?` matches exactly one. Everything else is literal: +`*.env` does not match `configxenv`, because a rule you wrote by hand should mean what it looks +like. + +Newlines matter for `bash`, where a command can legitimately contain one: + +```json +{ "permission": { "bash": { "git commit *": "allow" } } } +``` + +That still matches `git commit -m "line one\nline two"`. + +## Order is the whole rule + +Later rules win. A config reads top to bottom, so put the catch-all first and narrow after it: + +```json +{ "permission": { "bash": { "*": "ask", "git *": "allow", "git push *": "ask" } } } +``` + +`git status` is allowed, `git push origin main` asks again. Reverse those last two and `git push` +is allowed, because the broader rule now comes last. + +This is plain precedence with no special case for `deny`, and that is deliberate. An earlier +version made `deny` win wherever it sat, on the theory that a refusal should be impossible to +undo by accident. It made the most useful shape in the system unexpressible: + +```json +{ "permission": { "edit_file": { "*": "deny", "src/generated/*": "allow" } } } +``` + +Default-deny with narrow exceptions is what a careful user writes, and it is the same shape as +`*.env` denied while `*.env.example` is allowed. Refusals that must never be configurable belong +in the [guard plugin](plugins.md), which runs ahead of this and which `--yolo` cannot reach. + +## Defaults + +With no `permission` config: + +| Tool | Default | +|---|---| +| `read_file` `read_many_files` | `allow`, except `*.env`, `*.env.*`, and `*.pem` which are `deny`. `*.env.example` is allowed | +| `glob` `grep` `list_dir` | `allow` | +| the git tools | `allow` — they cannot mutate anything | +| `task`, and every session tool | `allow` — they touch the agent's own state | +| `write_file` `edit_file` `multi_edit` `bash` | `ask` | +| anything else, including every `mcp__*` tool | `ask` | + +Credentials are denied on read rather than gated, because there is no recovery. A model that +greps for a config value and finds a secret has that secret in its context, on the wire, and in +the session file on disk. `.env.example` is the one such file that is safe and the one a model +usually wants. + +An unrecognised tool asks. MCP tools are third-party code with unknown side effects, so +`mcp__fs__read_file` sounding harmless is not a reason to let it through. + +## Config replaces a default, it does not merge + +Anything you say about a tool replaces its default for that tool entirely: + +```json +{ "permission": { "read_file": { "*": "allow" } } } +``` + +That allows reading `.env`. Merging pattern-by-pattern would leave you unable to remove a default +deny rule, and a safety feature you cannot switch off is one people work around instead. + +A wildcard in a tool key covers a family: + +```json +{ "permission": { "mcp__*": "deny" } } +``` + +## What `always` grants + +Answering `a` at a prompt whitelists a **pattern**, not the tool. For `bash` that is the command's +first word: + +``` +bash wants to run +git status --porcelain +y allow once | a always allow bash git * | n deny +``` + +Approving that runs `git log` and `git diff` unprompted for the rest of the session, and still +asks about `npm publish`. Everything else falls back to `*`, because a path pattern guessed from +one path is more often wrong than useful. + +Grants last for the session and are never written to disk. They also cannot outrank a rule: with +`"rm *": "deny"` in your config, `a` on a different command does not unlock `rm`. + +## The repeat guard + +A tool called three times in one turn with **identical** input asks for approval even when the +rules allow it: + +``` +read_file is repeating the same call +allowed by the rules, but this is the third identical call this turn +src/session.ts +``` + +A model repeating an identical call is not making progress: either it is ignoring the result, or +the result is not what it needed. `bash: allow` is a statement about which commands are safe, not +permission to run one in a loop until the step limit ends the turn. + +The count is per turn and resets on the next prompt, so a tool used once in each of several turns +never trips it. + +## Order of checks + +``` +guard plugin refusal; --yolo cannot reach it + → permission rules allow / ask / deny, matched on the subject + → repeat guard an allowed call that has now repeated 3x asks anyway + → the tool runs +``` + +A denied call **provably never executes**: the decision is enforced by the SDK, which never +reaches the tool's `execute`. A tool cannot forget to honour a denial because the tool is not +consulted. See [architecture](architecture.md#why-approval-goes-through-the-sdk). + +## `--yolo` + +Folds `ask` into `allow`. It does not touch `deny`, from your config or from the defaults, and it +does not reach the guard plugin. So `--yolo` still refuses to read `.env` and still refuses +`rm -rf`. + +In headless mode there is no terminal to prompt on, so an `ask` is denied and the model is told. +`--yolo` is what makes a headless run able to write. See [headless](headless.md). + +## A typo never widens access + +An unrecognised decision string is dropped when the config is parsed, which leaves the pattern +unmatched and the tool on its default: + +```json +{ "permission": { "bash": { "*": "alow" } } } +``` + +`bash` asks, as it would with no config at all. The alternative — treating an unparseable value as +`allow` — would mean one misspelling silently removing the gate. + +## Worked examples + +**Trust the tools you run constantly, keep the rest gated.** + +```json +{ + "permission": { + "bash": { + "*": "ask", + "git status*": "allow", + "git diff*": "allow", + "git log*": "allow", + "bun test*": "allow", + "bun run typecheck": "allow" + } + } +} +``` + +**Let it edit source, never touch generated output or CI config.** + +```json +{ + "permission": { + "edit_file": { "*": "ask", "src/*": "allow", "src/generated/*": "deny", ".github/*": "deny" }, + "write_file": { "*": "ask", "src/generated/*": "deny", ".github/*": "deny" } + } +} +``` + +**Read-only, enforced by rules rather than by asking.** The `plan` agent variant does this by +withholding the tools, which is stronger; use rules when you want the tools present but inert. + +```json +{ "permission": { "write_file": "deny", "edit_file": "deny", "multi_edit": "deny", "bash": "deny" } } +``` + +**An unattended job that may commit but never push.** + +```json +{ + "permission": { + "bash": { "*": "deny", "git add *": "allow", "git commit *": "allow", "bun test*": "allow" } + } +} +``` + +## What this is not + +Rules gate the *call*, not what the command does once it runs. `bash` with `git *` allowed will +run `git config --global` and a `git` alias that shells out to anything. There is no sandbox: no +`seccomp`, no Seatbelt, no filesystem jail around the shell. + +`jail()` keeps the **file tools** inside the workspace — see +[tools](tools.md#path-safety) — and the guard plugin refuses a list of irreversible commands. Both +are worth having and neither is containment. Run the agent in a container or a VM when the +workspace is not the trust boundary. diff --git a/docs/tools.md b/docs/tools.md index cfcdca5..0d60e9f 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -2,40 +2,45 @@ ## The approval model -Three categories. +Every call resolves to `allow`, `ask`, or `deny` through a rule matched against the call's +subject — the command for `bash`, the path for a file tool. [Permissions](permissions.md) is the +full reference; the short version: -**Free.** Read-only, no prompt: `read_file`, `read_many_files`, `glob`, `grep`, `list_dir`, -`task`, and the whole git set. +**Allowed by default.** Read-only tools and anything touching the agent's own state: +`read_file`, `read_many_files`, `glob`, `grep`, `list_dir`, `task`, the whole git set, +`todo_write`, `remember`, `recall`, `forget`, `skill`, `ask`, and anything a plugin marks +auto-approved. -**Session tools.** Also free, because they touch the agent's own state rather than your -files: `todo_write`, `remember`, `recall`, `forget`, `skill`, `ask`, and anything a plugin -marks auto-approved. +**Denied by default.** `*.env`, `*.env.*`, and `*.pem` on read. Not gated, refused: a secret that +reaches the context is on the wire and in the session file, and there is no taking it back. +`*.env.example` is allowed. -**Gated.** Every call stops for a decision: `write_file`, `edit_file`, `multi_edit`, `bash`, -and every `mcp__*` tool. +**Asked by default.** `write_file`, `edit_file`, `multi_edit`, `bash`, and every `mcp__*` tool. ``` -edit_file wants to run -src/users.ts +2 -1 - export function paginate(offset: number, total: number) { - - if (offset < total) return next(); - + if (offset <= total) return next(); - } -y allow once | a always allow edit_file | n deny +bash wants to run +git status --porcelain +y allow once | a always allow bash git * | n deny ``` -`a` whitelists that tool for the rest of the session. `n` tells the model it was denied and -to ask what to do instead. `--yolo` skips all prompts. +`a` whitelists the **pattern**, not the tool: approving `git status` runs `git log` unprompted and +still asks about `npm publish`. `n` tells the model it was denied and to ask what to do instead. -The approval is enforced by the SDK, not by the tools. A denied call **provably never -executes**: the SDK never reaches the tool's `execute`, so a tool cannot forget to honour a -denial or opt out of the check. See [architecture](architecture.md#why-approval-goes-through-the-sdk). +A rule turns the common cases off entirely: -MCP tools are gated as a group because they are third-party code with unknown side effects — -`mcp__fs__read_file` sounds harmless and might not be. See [MCP](mcp.md). +```json +{ "permission": { "bash": { "*": "ask", "git *": "allow", "bun test*": "allow" } } } +``` -**The guard runs before all of this.** It is not an approval — it is a refusal, and `--yolo` -does not reach it. See [plugins](plugins.md). +Three more things sit around the rules: + +- **The guard plugin refuses first.** It is not an approval, and `--yolo` does not reach it. See + [plugins](plugins.md). +- **A repeated call asks anyway.** The same tool with identical input three times in one turn stops + for approval even when allowed — a model repeating itself is not making progress. +- **The SDK enforces the decision.** A denied call provably never executes, because the SDK never + reaches the tool's `execute`. A tool cannot forget to honour a denial. See + [architecture](architecture.md#why-approval-goes-through-the-sdk). ## Tool sets diff --git a/src/cli.tsx b/src/cli.tsx index c877d55..8a650a0 100644 --- a/src/cli.tsx +++ b/src/cli.tsx @@ -59,6 +59,7 @@ config: ${configPath()} { "provider": "openai", "model": "gpt-5", "apiKey": "...", "agent": "default", "thinking": "medium", "plugins": ["guard", "time"], "toolSets": ["edit-plus", "git"], "registryUrl": "https://...", + "permission": { "bash": { "*": "ask", "git *": "allow" } }, "mcpServers": { "fs": { "command": "npx", "args": ["-y", "@modelcontextprotocol/server-filesystem", "."] } } } env: SHIRO_PROVIDER SHIRO_MODEL SHIRO_BASE_URL SHIRO_API_KEY @@ -253,6 +254,7 @@ const session = new Session({ plugins, agent: agentVariant, ...(cfg.toolSets ? { toolSets: cfg.toolSets } : {}), + ...(cfg.permission ? { permissions: cfg.permission } : {}), // Headless has no one to answer, so the tool is withheld rather than left to hang. ...(headless ? {} : { ask: askBridge.ask }), ...(memory ? { memory } : {}), @@ -492,7 +494,11 @@ const header = [ memory && memory.all().length > 0 ? `memory: ${memory.all().length} notes about this project` : undefined, mcp && Object.keys(mcp.tools).length > 0 ? `mcp: ${Object.keys(mcp.tools).length} tools` : undefined, ...(mcp?.errors ?? []).map((e) => `mcp ${e.server} failed: ${e.message}`), - yolo ? 'approvals: OFF (--yolo)' : 'approvals: on for write_file, edit_file, multi_edit, bash, mcp__*', + yolo + ? 'approvals: OFF (--yolo), but deny rules and the guard still apply' + : cfg.permission + ? `approvals: rules for ${Object.keys(cfg.permission).join(', ')}, defaults elsewhere` + : 'approvals: ask for write_file, edit_file, multi_edit, bash, mcp__*', cfg.toolSets ? `tool sets: core, ${cfg.toolSets.join(', ')}` : undefined, '/help for commands', ] diff --git a/src/config.ts b/src/config.ts index 59f60d9..9fb72d4 100644 --- a/src/config.ts +++ b/src/config.ts @@ -6,6 +6,7 @@ import { homedir } from 'node:os'; import { join } from 'node:path'; import { withFallback, type FallbackEvent } from './fallback'; import type { McpServerConfig } from './mcp'; +import { parsePermissions, type PermissionConfig } from './permission'; import { isToolSetName, type ToolSetName } from './tools'; export type ProviderName = 'anthropic' | 'openai'; @@ -27,6 +28,8 @@ export type Config = { plugins?: string[]; /** Optional tool sets to offer beyond `core`; omit for all of them. */ toolSets?: ToolSetName[]; + /** Which tool calls run, ask, or are refused. Omit for the defaults. */ + permission?: PermissionConfig; /** Index for `/registry`. Omit for the default one. */ registryUrl?: string; mcpServers?: Record; @@ -91,6 +94,10 @@ export async function loadConfig(): Promise { ...(file.thinking ? { thinking: file.thinking } : {}), ...(Array.isArray(file.plugins) ? { plugins: file.plugins } : {}), ...(Array.isArray(file.toolSets) ? { toolSets: file.toolSets.filter(isToolSetName) } : {}), + ...(() => { + const permission = parsePermissions(file.permission); + return permission ? { permission } : {}; + })(), ...(typeof file.registryUrl === 'string' ? { registryUrl: file.registryUrl } : {}), ...(file.mcpServers ? { mcpServers: file.mcpServers } : {}), }; diff --git a/src/permission.ts b/src/permission.ts new file mode 100644 index 0000000..4402ddf --- /dev/null +++ b/src/permission.ts @@ -0,0 +1,296 @@ +/** + * Permission rules: which tool calls run, which ask, which are refused. + * + * The old model gated by tool name alone, which made `bash` a single yes/no for + * both `git status` and `rm -rf`. A user holding `a` through a batch approves the + * second along with the first, so the gate stopped meaning anything. Rules match + * the tool's *subject* — the command, the path, the pattern — so `git *` can be + * allowed while `*` still asks. + * + * Pure on purpose: no IO, no UI, so precedence and matching are testable without + * a terminal or a model. + */ + +export type Decision = 'allow' | 'ask' | 'deny'; + +/** A rule set for one tool: subject pattern to decision. `*` is the catch-all. */ +export type ToolRules = Record; + +/** Either one decision for every call, or per-subject rules. */ +export type PermissionEntry = Decision | ToolRules; + +export type PermissionConfig = Record; + +const DECISIONS = new Set(['allow', 'ask', 'deny']); + +export const isDecision = (v: unknown): v is Decision => typeof v === 'string' && DECISIONS.has(v as Decision); + +/** + * Glob-ish matching: `*` spans any characters, `?` exactly one. + * + * Not a regex, deliberately. A rule comes from a config file a human wrote, and + * `rm *` should mean what it looks like rather than "rm followed by anything, + * where `*` is a quantifier on a space". + */ +export function matchPattern(pattern: string, subject: string): boolean { + if (pattern === '*') return true; + const escaped = pattern.replace(/[.+^${}()|[\]\\]/g, '\\$&'); + const source = `^${escaped.replaceAll('*', '[\\s\\S]*').replaceAll('?', '[\\s\\S]')}$`; + try { + return new RegExp(source).test(subject); + } catch { + return false; + } +} + +/** + * The text a rule for this tool matches against. + * + * One field per tool, chosen so the rule reads like the thing being gated: a + * command for `bash`, a path for anything touching the filesystem, the query for + * a search. A tool with no obvious subject matches only `*`, which is why this + * returns undefined rather than an empty string — an empty subject would match + * a `*` rule and a `?` rule differently for no good reason. + */ +export function subjectOf(tool: string, input: unknown): string | undefined { + if (input === null || typeof input !== 'object') return undefined; + const o = input as Record; + + const str = (key: string) => (typeof o[key] === 'string' ? (o[key] as string) : undefined); + + switch (tool) { + case 'bash': + return str('command'); + case 'read_file': + case 'write_file': + case 'edit_file': + case 'multi_edit': + case 'list_dir': + case 'git_blame': + return str('path'); + case 'read_many_files': { + // A batch read is gated on the paths it asks for, so one bad path in + // twenty is enough to trigger a rule. + const files = o['files']; + if (!Array.isArray(files)) return undefined; + const paths = files + .map((f) => (f && typeof f === 'object' ? (f as { path?: unknown }).path : undefined)) + .filter((p): p is string => typeof p === 'string'); + return paths.length > 0 ? paths.join(' ') : undefined; + } + case 'glob': + return str('pattern'); + case 'grep': + return str('pattern'); + case 'git_diff': + case 'git_log': + return str('path'); + case 'git_show': + return str('ref'); + case 'task': + return str('description'); + case 'skill': + return str('name'); + default: + return undefined; + } +} + +/** + * A read_many_files subject is several paths at once, so a rule has to match if + * it matches any of them: denying `*.env` must catch a batch that includes one. + */ +const subjectsFor = (tool: string, subject: string): string[] => + tool === 'read_many_files' ? subject.split(' ') : [subject]; + +export type Resolved = { decision: Decision; pattern: string | undefined }; + +/** + * The decision for one call, with the pattern that produced it. + * + * Later rules win, so a config reads top to bottom: put `*` first and narrow + * after it. Object key order is insertion order for string keys, which is what + * makes that stable. + * + * Plain last-match-wins, with no special case for `deny`. An earlier attempt made + * deny win outright, on the theory that a deny rule should be impossible to undo + * by accident. It made the most useful configuration in the system unexpressible: + * + * "edit_file": { "*": "deny", "src/generated/*": "allow" } + * + * Default-deny with narrow allows is what a careful user writes, and a narrow + * exception to a broad deny is the same shape as `*.env` denied but + * `*.env.example` allowed. Refusals that must never be configurable live in the + * guard plugin instead, which runs ahead of this and which `--yolo` cannot reach. + */ +export function resolve(rules: PermissionEntry | undefined, tool: string, input: unknown): Resolved { + if (rules === undefined) return { decision: 'ask', pattern: undefined }; + if (isDecision(rules)) return { decision: rules, pattern: undefined }; + + const subject = subjectOf(tool, input); + let hit: Resolved = { decision: 'ask', pattern: undefined }; + + for (const [pattern, decision] of Object.entries(rules)) { + if (!isDecision(decision)) continue; + const matched = + pattern === '*' || + (subject !== undefined && subjectsFor(tool, subject).some((s) => matchPattern(pattern, s))); + if (matched) hit = { decision, pattern }; + } + + return hit; +} + +/** + * Defaults, applied when the config says nothing about a tool. + * + * Read-only tools run; anything that writes or executes asks. `.env` is denied on + * read because a model that greps for a config value will find a credential, and + * "it was in the context" is not recoverable. + */ +export const DEFAULT_PERMISSIONS: PermissionConfig = { + read_file: { '*': 'allow', '*.env': 'deny', '*.env.*': 'deny', '*.env.example': 'allow', '*.pem': 'deny' }, + read_many_files: { '*': 'allow', '*.env': 'deny', '*.env.*': 'deny', '*.env.example': 'allow', '*.pem': 'deny' }, + write_file: 'ask', + edit_file: 'ask', + multi_edit: 'ask', + bash: 'ask', +}; + +/** Session, plugin, and read-only tools that never gate. */ +const FREE = new Set([ + 'glob', + 'grep', + 'list_dir', + 'task', + 'todo_write', + 'remember', + 'recall', + 'forget', + 'skill', + 'ask', + 'current_time', + 'git_status', + 'git_diff', + 'git_log', + 'git_show', + 'git_blame', +]); + +export type PermissionOptions = { + config?: PermissionConfig; + /** --yolo: fold `ask` into `allow`. Never touches `deny`. */ + yolo?: boolean; + /** Names that never prompt whatever the rules say, e.g. the subagent tool. */ + autoApprove?: readonly string[]; +}; + +/** + * Rules for a tool, config over defaults. + * + * Merged per tool rather than per pattern: a config that says anything about + * `bash` replaces the default for `bash` entirely. Merging pattern-by-pattern + * would leave a user unable to remove a default deny rule, which is the kind of + * surprise that ends with someone disabling the whole system. + */ +function entryFor(tool: string, config: PermissionConfig | undefined): PermissionEntry | undefined { + if (config && tool in config) return config[tool]; + if (config && !(tool in config)) { + for (const [pattern, entry] of Object.entries(config)) { + if (pattern.includes('*') && matchPattern(pattern, tool)) return entry; + } + } + if (tool in DEFAULT_PERMISSIONS) return DEFAULT_PERMISSIONS[tool]; + if (FREE.has(tool)) return 'allow'; + // Unknown tool, which in practice means MCP or a plugin: ask. + return 'ask'; +} + +export class Permissions { + private readonly config: PermissionConfig | undefined; + private readonly yolo: boolean; + private readonly autoApprove: Set; + /** Patterns approved with `always` for the rest of this session. */ + private readonly granted = new Map>(); + + constructor(opts: PermissionOptions = {}) { + this.config = opts.config; + this.yolo = opts.yolo ?? false; + this.autoApprove = new Set(opts.autoApprove ?? []); + } + + /** + * What a session-wide `always` would whitelist. + * + * A bash command becomes its first word plus `*`, so approving `git status` + * approves `git *` and not every command ever. Anything else falls back to the + * tool's own catch-all, since a path pattern guessed from one path is more + * likely to be wrong than useful. + */ + suggest(tool: string, input: unknown): string { + if (tool !== 'bash') return '*'; + const command = subjectOf(tool, input); + if (!command) return '*'; + const head = command.trim().split(/\s+/)[0]; + return head ? `${head} *` : '*'; + } + + /** Records an `always` decision as a pattern rather than a bare tool name. */ + grant(tool: string, pattern: string): void { + const set = this.granted.get(tool) ?? new Set(); + set.add(pattern); + this.granted.set(tool, set); + } + + granted_(tool: string): string[] { + return [...(this.granted.get(tool) ?? [])]; + } + + /** The decision for one call, and which pattern decided it. */ + check(tool: string, input: unknown): Resolved { + const resolved = resolve(entryFor(tool, this.config), tool, input); + if (resolved.decision === 'deny') return resolved; + + if (this.autoApprove.has(tool)) return { decision: 'allow', pattern: undefined }; + + const subject = subjectOf(tool, input); + for (const pattern of this.granted.get(tool) ?? []) { + if (pattern === '*' || (subject !== undefined && matchPattern(pattern, subject))) { + return { decision: 'allow', pattern }; + } + } + + if (resolved.decision === 'ask' && this.yolo) return { decision: 'allow', pattern: resolved.pattern }; + return resolved; + } +} + +/** + * Parses a `permission` config block, dropping anything malformed. + * + * A typo must not silently widen access. An unrecognised decision string is + * dropped, which leaves the pattern unmatched and the tool on its default — + * `ask` for anything that writes. + */ +export function parsePermissions(raw: unknown): PermissionConfig | undefined { + if (!raw || typeof raw !== 'object') return undefined; + const out: PermissionConfig = {}; + + for (const [tool, entry] of Object.entries(raw as Record)) { + if (isDecision(entry)) { + out[tool] = entry; + continue; + } + if (!entry || typeof entry !== 'object') continue; + + const rules: ToolRules = {}; + for (const [pattern, decision] of Object.entries(entry as Record)) { + if (isDecision(decision)) rules[pattern] = decision; + } + if (Object.keys(rules).length > 0) out[tool] = rules; + } + + return Object.keys(out).length > 0 ? out : undefined; +} + +export { FREE as FREE_TOOLS }; diff --git a/src/session.ts b/src/session.ts index 4939631..504def6 100644 --- a/src/session.ts +++ b/src/session.ts @@ -12,25 +12,26 @@ import { createAskTool, type AskFn } from './ask'; import type { Instructions } from './instructions'; import type { Memory } from './memory'; import { Notebook, type NotebookState } from './notebook'; +import { Permissions, type PermissionConfig } from './permission'; import type { PluginHost } from './plugins'; import { systemPrompt } from './prompt'; import { prunePreservingItems } from './prune'; import { createSkillTool, renderSkills, type Skill } from './skills'; -import { - MUTATING_TOOLS, - disabledToolNames, - onBashOutput, - tools as builtinTools, - type ToolSetName, -} from './tools'; +import { disabledToolNames, onBashOutput, tools as builtinTools, type ToolSetName } from './tools'; export type ApprovalRequest = { approvalId: string; toolName: string; input: unknown; + /** The rule that decided this needs asking, when one did. */ + matchedPattern?: string; + /** What `always` would whitelist, e.g. `git *` rather than every bash call. */ + suggestedPattern: string; + /** Set when the call is being asked about because it repeated, not because of a rule. */ + repeated?: boolean; }; -/** 'once' runs this call only; 'always' whitelists the tool for the rest of the session. */ +/** 'once' runs this call only; 'always' whitelists the suggested pattern for the session. */ export type ApprovalDecision = 'once' | 'always' | 'deny'; export type AgentEvent = @@ -57,6 +58,8 @@ export type SessionOptions = { extraTools?: ToolSet; /** Tool sets offered this session; omit for all of them. `core` is always on. */ toolSets?: readonly ToolSetName[]; + /** Rules deciding which calls run, ask, or are refused. Omit for the defaults. */ + permissions?: PermissionConfig; /** Tool names that never prompt, e.g. the read-only subagent tool. */ autoApprove?: readonly string[]; /** Prune the history once the estimated token count crosses this. */ @@ -86,6 +89,13 @@ const estimateTokens = (messages: ModelMessage[]) => Math.round(JSON.stringify(m /** Estimated tokens at which the wire history is pruned. */ const DEFAULT_COMPACT_THRESHOLD = 120_000; +/** Identical calls in one turn before an allowed tool is asked about anyway. */ +const REPEAT_LIMIT = 3; + +const callKey = (toolName: string, input: unknown) => `${toolName}:${JSON.stringify(input ?? null)}`; + +type ApprovalContext = Pick; + export class Session { readonly messages: ModelMessage[]; readonly tools: ToolSet; @@ -94,7 +104,9 @@ export class Session { outputTokens = 0; private model: LanguageModel; private variant: AgentVariant; - private readonly alwaysAllow = new Set(); + private readonly permissions: Permissions; + /** Calls seen this turn, for the repeat guard. Cleared per turn, not per step. */ + private readonly seen = new Map(); private controller: AbortController | undefined; constructor(private readonly opts: SessionOptions) { @@ -112,13 +124,16 @@ export class Session { }; this.tools = { ...builtinTools, ...sessionTools, ...(opts.plugins?.tools ?? {}), ...(opts.extraTools ?? {}) }; - for (const name of [ - ...(opts.autoApprove ?? []), - ...(opts.plugins?.autoApprove ?? []), - ...Object.keys(sessionTools), - ]) { - this.alwaysAllow.add(name); - } + this.permissions = new Permissions({ + ...(opts.permissions ? { config: opts.permissions } : {}), + ...(opts.yolo ? { yolo: true } : {}), + autoApprove: [ + ...(opts.autoApprove ?? []), + ...(opts.plugins?.autoApprove ?? []), + // A session tool touches the agent's own state, not the workspace. + ...Object.keys(sessionTools), + ], + }); } setModel(model: LanguageModel): void { @@ -186,32 +201,67 @@ export class Session { }); } - /** Tools that mutate the workspace, plus every externally provided MCP tool. */ - private needsApproval(name: string): boolean { - return (MUTATING_TOOLS as readonly string[]).includes(name) || name.startsWith('mcp__'); + /** + * How many times this exact call has already been made this turn. + * + * A model that repeats an identical call is not making progress: either it is + * ignoring the result or the result is not what it needed. Three is the point + * where that stops looking like a coincidence. + */ + private repeatCount(toolName: string, input: unknown): number { + const key = callKey(toolName, input); + const count = (this.seen.get(key) ?? 0) + 1; + this.seen.set(key, count); + return count; } /** * Approval decisions, evaluated per call by the SDK. * - * A plugin guard denies outright and is checked before anything else, so `--yolo` - * cannot bypass it. Only after the guard passes does yolo or the mutating-tool - * rule decide whether the user is asked. + * Order matters, and each step exists for a different reason: + * + * 1. A plugin guard refuses outright. `--yolo` cannot reach it, because a + * refusal is a policy decision rather than a permission question. + * 2. Permission rules decide allow / ask / deny, matched against the call's + * subject — the command, the path — not just the tool name. + * 3. An allowed call that has now repeated three times identically is asked + * about anyway. A rule saying `bash: allow` is a statement about which + * commands are safe, not permission to run one in a loop forever. + * + * `why` collects what the UI needs to explain the prompt, keyed by call, because + * the SDK's own approval request carries only the tool name and input. */ - private toolApproval(notices: string[]) { + private toolApproval(notices: string[], why: Map) { return async ({ toolCall }: { toolCall: { toolName: string; input: unknown } }) => { + const { toolName, input } = toolCall; + const blocked = await this.opts.plugins?.guard({ - toolName: toolCall.toolName, - input: toolCall.input, + toolName, + input, cwd: this.opts.cwd ?? process.cwd(), }); if (blocked) { notices.push(blocked); return { type: 'denied' as const, reason: blocked }; } - if (this.opts.yolo) return undefined; - if (!this.needsApproval(toolCall.toolName)) return undefined; - if (this.alwaysAllow.has(toolCall.toolName)) return undefined; + + const { decision, pattern } = this.permissions.check(toolName, input); + if (decision === 'deny') { + const reason = pattern + ? `Refused by the permission rule ${toolName}: "${pattern}" = deny.` + : `Refused by the permission rules: ${toolName} is denied.`; + notices.push(reason); + return { type: 'denied' as const, reason }; + } + + const repeats = this.repeatCount(toolName, input); + if (decision === 'allow' && repeats < REPEAT_LIMIT) return undefined; + + why.set(callKey(toolName, input), { + ...(pattern ? { matchedPattern: pattern } : {}), + suggestedPattern: this.permissions.suggest(toolName, input), + ...(decision === 'allow' ? { repeated: true } : {}), + }); return 'user-approval' as const; }; } @@ -243,6 +293,9 @@ export class Session { this.controller = new AbortController(); const signal = this.controller.signal; const threshold = this.compactThreshold(); + // Per turn, not per step: a tool called once in each of three steps is the + // loop this guards against. + this.seen.clear(); const outputs: Extract[] = []; onBashOutput(({ toolCallId, chunk }) => { @@ -269,6 +322,7 @@ export class Session { const pending: ApprovalRequest[] = []; const compactions: Extract[] = []; const guardNotices: string[] = []; + const why = new Map(); let sawError = false; const result = streamText({ @@ -278,7 +332,7 @@ export class Session { tools: this.tools, activeTools: this.activeTools(), reasoning: sdkReasoning(this.variant.thinking), - toolApproval: this.toolApproval(guardNotices), + toolApproval: this.toolApproval(guardNotices, why), stopWhen: isStepCount(this.variant.maxSteps ?? this.opts.maxSteps ?? 50), maxRetries: this.opts.maxRetries ?? 3, abortSignal: signal, @@ -336,16 +390,20 @@ export class Session { case 'tool-error': yield { type: 'tool-error', id: part.toolCallId, name: part.toolName, error: part.error }; break; - case 'tool-approval-request': + case 'tool-approval-request': { // A guard denial is answered by the SDK itself and arrives flagged // automatic; queueing it would prompt the user for a settled call. if (part.isAutomatic) break; + const context = why.get(callKey(part.toolCall.toolName, part.toolCall.input)); pending.push({ approvalId: part.approvalId, toolName: part.toolCall.toolName, input: part.toolCall.input, + suggestedPattern: '*', + ...context, }); break; + } case 'tool-approval-response': if (!part.approved) yield { type: 'tool-denied', name: part.toolCall.toolName }; break; @@ -393,8 +451,10 @@ export class Session { const responses: ToolApprovalResponse[] = []; for (const req of pending) { - const decision = this.alwaysAllow.has(req.toolName) ? 'always' : await this.opts.askApproval(req); - if (decision === 'always') this.alwaysAllow.add(req.toolName); + const decision = await this.opts.askApproval(req); + // `always` records the pattern the tool suggested, so approving + // `git status` whitelists `git *` rather than every command. + if (decision === 'always') this.permissions.grant(req.toolName, req.suggestedPattern); responses.push({ type: 'tool-approval-response', approvalId: req.approvalId, diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 7d2f769..e458cce 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -195,12 +195,25 @@ function Approval({ pending }: { pending: Pending }) { return ( - {pending.req.toolName} wants to run + {pending.req.repeated + ? `${pending.req.toolName} is repeating the same call` + : `${pending.req.toolName} wants to run`} + {pending.req.repeated && ( + + allowed by the rules, but this is the third identical call this turn + + )} + {!pending.req.repeated && pending.req.matchedPattern && pending.req.matchedPattern !== '*' && ( + {`matched ${pending.req.toolName}: "${pending.req.matchedPattern}"`} + )} - y allow once | a always allow {pending.req.toolName} |{' '} - n deny + y allow once | a always allow{' '} + {pending.req.suggestedPattern === '*' + ? pending.req.toolName + : `${pending.req.toolName} ${pending.req.suggestedPattern}`}{' '} + | n deny ); diff --git a/test/permission.test.ts b/test/permission.test.ts new file mode 100644 index 0000000..4267f92 --- /dev/null +++ b/test/permission.test.ts @@ -0,0 +1,232 @@ +import { expect, test } from 'bun:test'; +import { + DEFAULT_PERMISSIONS, + Permissions, + matchPattern, + parsePermissions, + resolve, + subjectOf, +} from '../src/permission'; + +test('a bare * matches anything, including an empty subject', () => { + expect(matchPattern('*', 'anything at all')).toBe(true); + expect(matchPattern('*', '')).toBe(true); +}); + +test('* spans any characters, ? exactly one', () => { + expect(matchPattern('git *', 'git status --porcelain')).toBe(true); + expect(matchPattern('git *', 'gitstatus')).toBe(false); + expect(matchPattern('src/?.ts', 'src/a.ts')).toBe(true); + expect(matchPattern('src/?.ts', 'src/ab.ts')).toBe(false); +}); + +test('regex metacharacters in a pattern are literal', () => { + // A rule written by a human. `.` must not match any character, or `*.env` + // would also match `xxenv`. + expect(matchPattern('*.env', 'config.env')).toBe(true); + expect(matchPattern('*.env', 'configxenv')).toBe(false); + expect(matchPattern('a+b', 'a+b')).toBe(true); + expect(matchPattern('a+b', 'aab')).toBe(false); +}); + +test('* matches across newlines, since a bash command can contain one', () => { + expect(matchPattern('git *', 'git commit -m "line one\nline two"')).toBe(true); +}); + +test('bash is matched on its command', () => { + expect(subjectOf('bash', { command: 'git status' })).toBe('git status'); +}); + +test('file tools are matched on their path', () => { + for (const tool of ['read_file', 'write_file', 'edit_file', 'multi_edit', 'list_dir']) { + expect(subjectOf(tool, { path: 'src/app.ts' }), tool).toBe('src/app.ts'); + } +}); + +test('a batch read is matched on every path it asks for', () => { + const subject = subjectOf('read_many_files', { files: [{ path: 'a.ts' }, { path: 'b.ts' }] }); + expect(subject).toBe('a.ts b.ts'); +}); + +test('search tools are matched on the pattern, git_show on the ref', () => { + expect(subjectOf('grep', { pattern: 'needle' })).toBe('needle'); + expect(subjectOf('glob', { pattern: 'src/**' })).toBe('src/**'); + expect(subjectOf('git_show', { ref: 'HEAD~2' })).toBe('HEAD~2'); +}); + +test('a tool with no subject field yields undefined, not an empty string', () => { + expect(subjectOf('git_status', {})).toBeUndefined(); + expect(subjectOf('bash', { notCommand: 'x' })).toBeUndefined(); + expect(subjectOf('bash', null)).toBeUndefined(); +}); + +test('a plain decision applies to every call', () => { + expect(resolve('allow', 'bash', { command: 'anything' }).decision).toBe('allow'); + expect(resolve('deny', 'bash', { command: 'anything' }).decision).toBe('deny'); +}); + +test('no rules at all means ask', () => { + expect(resolve(undefined, 'bash', { command: 'x' }).decision).toBe('ask'); +}); + +test('the last matching rule wins, so a config reads top to bottom', () => { + const rules = { '*': 'ask' as const, 'git *': 'allow' as const }; + expect(resolve(rules, 'bash', { command: 'git status' }).decision).toBe('allow'); + expect(resolve(rules, 'bash', { command: 'npm test' }).decision).toBe('ask'); +}); + +test('the resolved pattern is reported, so the UI can say what decided', () => { + const resolved = resolve({ '*': 'ask' as const, 'git *': 'allow' as const }, 'bash', { command: 'git log' }); + expect(resolved.pattern).toBe('git *'); +}); + +test('a narrow allow after a broad deny is honoured', () => { + // Default-deny with narrow allows is what a careful user writes, and it has to + // be expressible. Refusals that must never be configurable live in the guard + // plugin, which runs ahead of this. + const rules = { '*': 'deny' as const, 'src/generated/*': 'allow' as const }; + expect(resolve(rules, 'edit_file', { path: 'src/generated/api.ts' }).decision).toBe('allow'); + expect(resolve(rules, 'edit_file', { path: 'src/app.ts' }).decision).toBe('deny'); +}); + +test('a broad allow after a narrow deny wins, so order is the whole rule', () => { + const denyFirst = { 'rm *': 'deny' as const, '*': 'allow' as const }; + const denyLast = { '*': 'allow' as const, 'rm *': 'deny' as const }; + expect(resolve(denyFirst, 'bash', { command: 'rm -rf build' }).decision).toBe('allow'); + expect(resolve(denyLast, 'bash', { command: 'rm -rf build' }).decision).toBe('deny'); +}); + +test('a rule on a subject a tool does not have matches nothing but *', () => { + const rules = { 'src/*': 'allow' as const }; + expect(resolve(rules, 'git_status', {}).decision).toBe('ask'); + expect(resolve({ '*': 'allow' as const }, 'git_status', {}).decision).toBe('allow'); +}); + +test('one bad path in a batch read is enough to trigger a rule', () => { + const rules = { '*': 'allow' as const, '*.env': 'deny' as const }; + const input = { files: [{ path: 'src/app.ts' }, { path: 'config/.env' }] }; + expect(resolve(rules, 'read_many_files', input).decision).toBe('deny'); +}); + +test('the defaults let reads through and gate writes', () => { + const p = new Permissions(); + expect(p.check('read_file', { path: 'src/app.ts' }).decision).toBe('allow'); + expect(p.check('grep', { pattern: 'x' }).decision).toBe('allow'); + expect(p.check('write_file', { path: 'src/app.ts' }).decision).toBe('ask'); + expect(p.check('edit_file', { path: 'src/app.ts' }).decision).toBe('ask'); + expect(p.check('multi_edit', { path: 'src/app.ts' }).decision).toBe('ask'); + expect(p.check('bash', { command: 'echo hi' }).decision).toBe('ask'); +}); + +test('credentials are denied on read by default', () => { + const p = new Permissions(); + expect(p.check('read_file', { path: '.env' }).decision).toBe('deny'); + expect(p.check('read_file', { path: 'config/.env.local' }).decision).toBe('deny'); + expect(p.check('read_file', { path: 'certs/key.pem' }).decision).toBe('deny'); + // The example file is the one that is safe to read, and it is the one a model + // most often wants. + expect(p.check('read_file', { path: '.env.example' }).decision).toBe('allow'); +}); + +test('the git tools and session tools never gate', () => { + const p = new Permissions(); + for (const tool of ['git_status', 'git_diff', 'git_log', 'todo_write', 'remember', 'skill']) { + expect(p.check(tool, {}).decision, tool).toBe('allow'); + } +}); + +test('an unknown tool asks, which is what an MCP tool is', () => { + const p = new Permissions(); + expect(p.check('mcp__fs__write', { path: 'x' }).decision).toBe('ask'); +}); + +test('a wildcard tool key covers a family of tools', () => { + const p = new Permissions({ config: { 'mcp__*': 'deny' } }); + expect(p.check('mcp__fs__read', {}).decision).toBe('deny'); + expect(p.check('read_file', { path: 'a.ts' }).decision).toBe('allow'); +}); + +test('config for a tool replaces its defaults rather than merging', () => { + // Otherwise a default deny could never be removed, which is the kind of + // surprise that ends with the whole system switched off. + const p = new Permissions({ config: { read_file: { '*': 'allow' } } }); + expect(p.check('read_file', { path: '.env' }).decision).toBe('allow'); +}); + +test('yolo folds ask into allow and leaves deny alone', () => { + const p = new Permissions({ yolo: true, config: { bash: { '*': 'ask', 'rm *': 'deny' } } }); + expect(p.check('bash', { command: 'echo hi' }).decision).toBe('allow'); + expect(p.check('bash', { command: 'rm -rf /' }).decision).toBe('deny'); +}); + +test('yolo does not override a configured deny on reads either', () => { + const p = new Permissions({ yolo: true }); + expect(p.check('read_file', { path: '.env' }).decision).toBe('deny'); +}); + +test('autoApprove bypasses ask but not deny', () => { + const p = new Permissions({ autoApprove: ['task', 'bash'], config: { bash: { '*': 'ask', 'rm *': 'deny' } } }); + expect(p.check('task', { description: 'search' }).decision).toBe('allow'); + expect(p.check('bash', { command: 'echo hi' }).decision).toBe('allow'); + expect(p.check('bash', { command: 'rm -rf /' }).decision).toBe('deny'); +}); + +test('always suggests a command prefix for bash, not the whole command', () => { + const p = new Permissions(); + expect(p.suggest('bash', { command: 'git status --porcelain' })).toBe('git *'); + expect(p.suggest('bash', { command: 'bun test' })).toBe('bun *'); +}); + +test('always suggests the catch-all for anything but bash', () => { + const p = new Permissions(); + // A path pattern guessed from one path is more often wrong than useful. + expect(p.suggest('edit_file', { path: 'src/app.ts' })).toBe('*'); +}); + +test('a granted pattern approves matching later calls only', () => { + const p = new Permissions(); + p.grant('bash', 'git *'); + + expect(p.check('bash', { command: 'git log' }).decision).toBe('allow'); + expect(p.check('bash', { command: 'git push' }).decision).toBe('allow'); + expect(p.check('bash', { command: 'npm publish' }).decision).toBe('ask'); +}); + +test('a granted pattern is still bound by a later deny rule', () => { + const p = new Permissions({ config: { bash: { '*': 'ask', 'rm *': 'deny' } } }); + p.grant('bash', '*'); + expect(p.check('bash', { command: 'rm -rf build' }).decision).toBe('deny'); + expect(p.check('bash', { command: 'echo hi' }).decision).toBe('allow'); +}); + +test('parsePermissions keeps valid entries and drops the rest', () => { + const parsed = parsePermissions({ + bash: 'allow', + edit_file: { '*': 'ask', 'src/*': 'allow' }, + broken: 'maybe', + alsoBroken: { 'src/*': 'perhaps' }, + notAnEntry: 42, + }); + + expect(parsed).toEqual({ bash: 'allow', edit_file: { '*': 'ask', 'src/*': 'allow' } }); +}); + +test('a typo in a decision never widens access', () => { + // "alow" is dropped, so the tool falls back to its default, which asks. + const p = new Permissions({ config: parsePermissions({ bash: { '*': 'alow' } }) }); + expect(p.check('bash', { command: 'rm -rf /' }).decision).toBe('ask'); +}); + +test('parsePermissions returns undefined for nothing usable', () => { + expect(parsePermissions(undefined)).toBeUndefined(); + expect(parsePermissions('allow')).toBeUndefined(); + expect(parsePermissions({})).toBeUndefined(); + expect(parsePermissions({ bash: 'nonsense' })).toBeUndefined(); +}); + +test('every mutating tool has a default, so none can be added without one', () => { + const { MUTATING_TOOLS } = require('../src/tools') as { MUTATING_TOOLS: readonly string[] }; + for (const tool of MUTATING_TOOLS) { + expect(DEFAULT_PERMISSIONS[tool], tool).toBeDefined(); + } +}); diff --git a/test/session-features.test.ts b/test/session-features.test.ts index 4fa3a23..349901d 100644 --- a/test/session-features.test.ts +++ b/test/session-features.test.ts @@ -1,6 +1,9 @@ import { expect, test } from 'bun:test'; import { MockLanguageModelV4, simulateReadableStream } from 'ai/test'; import type { LanguageModelV4CallOptions, LanguageModelV4StreamPart } from '@ai-sdk/provider'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import { variantByName } from '../src/agents'; import { Memory } from '../src/memory'; import { createHost } from '../src/plugins'; @@ -10,6 +13,16 @@ import { loadSkills } from '../src/skills'; import { MUTATING_TOOLS, TOOL_SETS, TOOL_SET_NAMES, isToolSetName, toolSetOf } from '../src/tools'; import { GIT_TOOL_NAMES } from '../src/tools-git'; +function inTempDir(fn: () => Promise): Promise { + const orig = process.cwd(); + const dir = mkdtempSync(join(tmpdir(), 'shiro-perm-')); + process.chdir(dir); + return fn().finally(() => { + process.chdir(orig); + rmSync(dir, { recursive: true, force: true }); + }); +} + const usage = { inputTokens: { total: 5, noCache: 5, cacheRead: 0, cacheWrite: 0 }, outputTokens: { total: 2 }, @@ -296,3 +309,218 @@ test('isToolSetName accepts the real sets only, so a typo in config is ignored', for (const name of TOOL_SET_NAMES) expect(isToolSetName(name)).toBe(true); expect(isToolSetName('gti')).toBe(false); }); + +const bashCall = (id: string, command: string) => toolCall(id, 'bash', { command }); + +test('a pattern that allows skips the prompt entirely', async () => { + let n = 0; + const session = new Session({ + model: new MockLanguageModelV4({ + doStream: async () => (n++ === 0 ? stream(bashCall('c1', 'echo allowed')) : stream(text('done'))), + }), + askApproval: async () => { + throw new Error('an allowed pattern must not prompt'); + }, + permissions: { bash: { '*': 'ask', 'echo *': 'allow' } }, + }); + + const kinds: string[] = []; + for await (const ev of session.send('say something')) kinds.push(ev.type); + expect(kinds).toContain('tool-result'); + expect(kinds).not.toContain('tool-denied'); +}); + +test('a pattern that denies refuses without asking, and the tool never runs', async () => + inTempDir(async () => { + let asked = 0; + let n = 0; + const session = new Session({ + model: new MockLanguageModelV4({ + doStream: async () => + n++ === 0 ? stream(bashCall('c1', 'rm -rf build > out.txt')) : stream(text('understood')), + }), + askApproval: async () => { + asked++; + return 'once'; + }, + // Deliberately not the guard plugin: this is the rule engine refusing. + plugins: undefined, + permissions: { bash: { '*': 'ask', 'rm *': 'deny' } }, + }); + + const events: string[] = []; + const notices: string[] = []; + for await (const ev of session.send('clean up')) { + events.push(ev.type); + if (ev.type === 'notice') notices.push(ev.text); + } + + expect(asked).toBe(0); + expect(events).toContain('tool-denied'); + expect(notices.join()).toContain('"rm *" = deny'); + expect(await Bun.file(join(process.cwd(), 'out.txt')).exists()).toBe(false); + })); + +test('a command outside the allowed pattern still asks', async () => { + const asked: string[] = []; + let n = 0; + const session = new Session({ + model: new MockLanguageModelV4({ + doStream: async () => (n++ === 0 ? stream(bashCall('c1', 'npm publish')) : stream(text('done'))), + }), + askApproval: async (req) => { + asked.push(req.toolName); + return 'deny'; + }, + permissions: { bash: { '*': 'ask', 'git *': 'allow' } }, + }); + + for await (const _ of session.send('publish it')) void _; + expect(asked).toEqual(['bash']); +}); + +test('always records the suggested pattern, so a sibling command runs unprompted', async () => { + const asked: string[] = []; + let n = 0; + const session = new Session({ + model: new MockLanguageModelV4({ + doStream: async () => { + const i = n++; + if (i === 0) return stream(bashCall('c1', 'git status')); + if (i === 1) return stream(bashCall('c2', 'git log')); + if (i === 2) return stream(bashCall('c3', 'npm test')); + return stream(text('done')); + }, + }), + askApproval: async (req) => { + asked.push(String((req.input as { command?: string }).command)); + return req.suggestedPattern === 'git *' ? 'always' : 'deny'; + }, + }); + + for await (const _ of session.send('inspect the repo')) void _; + + // git status is approved as `git *`, so git log never reaches the prompt. + expect(asked).toEqual(['git status', 'npm test']); +}); + +test('the prompt carries the pattern that matched and what always would grant', async () => { + const seen: { matched?: string; suggested: string }[] = []; + let n = 0; + const session = new Session({ + model: new MockLanguageModelV4({ + doStream: async () => (n++ === 0 ? stream(bashCall('c1', 'npm test')) : stream(text('done'))), + }), + askApproval: async (req) => { + seen.push({ ...(req.matchedPattern ? { matched: req.matchedPattern } : {}), suggested: req.suggestedPattern }); + return 'deny'; + }, + permissions: { bash: { '*': 'ask' } }, + }); + + for await (const _ of session.send('test it')) void _; + expect(seen).toEqual([{ matched: '*', suggested: 'npm *' }]); +}); + +test('a third identical call is asked about even when the rules allow it', async () => + inTempDir(async () => { + await Bun.write(join(process.cwd(), 'note.txt'), 'hello'); + + const asked: boolean[] = []; + let n = 0; + const session = new Session({ + model: new MockLanguageModelV4({ + doStream: async () => { + const i = n++; + return i < 4 ? stream(toolCall(`c${i}`, 'read_file', { path: 'note.txt' })) : stream(text('done')); + }, + }), + askApproval: async (req) => { + asked.push(req.repeated === true); + return 'once'; + }, + }); + + for await (const _ of session.send('read it repeatedly')) void _; + + // Two identical reads pass; the third and fourth are asked about, flagged as + // repeats rather than as rule matches. + expect(asked).toEqual([true, true]); + })); + +test('the repeat guard counts per turn, not for the life of the session', async () => + inTempDir(async () => { + await Bun.write(join(process.cwd(), 'note.txt'), 'hello'); + + let asks = 0; + let n = 0; + const session = new Session({ + model: new MockLanguageModelV4({ + doStream: async () => { + const i = n++ % 3; + return i < 2 ? stream(toolCall(`c${i}`, 'read_file', { path: 'note.txt' })) : stream(text('done')); + }, + }), + askApproval: async () => { + asks++; + return 'once'; + }, + }); + + for await (const _ of session.send('first turn')) void _; + for await (const _ of session.send('second turn')) void _; + + // Two reads per turn, twice: never three in one turn, so never asked. + expect(asks).toBe(0); + })); + +test('the guard plugin still refuses ahead of the rules, and yolo cannot reach it', async () => { + let asked = 0; + let n = 0; + const session = new Session({ + yolo: true, + model: new MockLanguageModelV4({ + doStream: async () => (n++ === 0 ? stream(bashCall('c1', 'rm -rf /')) : stream(text('understood'))), + }), + askApproval: async () => { + asked++; + return 'once'; + }, + plugins: createHost([guardPlugin]), + permissions: { bash: 'allow' }, + }); + + const notices: string[] = []; + for await (const ev of session.send('clean up')) if (ev.type === 'notice') notices.push(ev.text); + + expect(asked).toBe(0); + expect(notices.join()).toContain('recursive or forced delete'); +}); + +test('reading a credential is refused by default', async () => + inTempDir(async () => { + await Bun.write(join(process.cwd(), '.env'), 'SECRET=hunter2'); + + let n = 0; + const session = new Session({ + model: new MockLanguageModelV4({ + doStream: async () => (n++ === 0 ? stream(toolCall('c1', 'read_file', { path: '.env' })) : stream(text('ok'))), + }), + askApproval: async () => { + throw new Error('a denied read must not prompt'); + }, + }); + + const events: string[] = []; + const notices: string[] = []; + for await (const ev of session.send('read the env file')) { + events.push(ev.type); + if (ev.type === 'notice') notices.push(ev.text); + } + + expect(events).toContain('tool-denied'); + expect(notices.join()).toContain('deny'); + // The secret must not reach the transcript either. + expect(JSON.stringify(session.messages)).not.toContain('hunter2'); + })); +