fix(safety): fail closed when the decision hook cannot check a call; read more command forms in base protection - #92
Conversation
The PreToolUse gate for Claude Code, Codex and Qwen Code always exited 0 and printed nothing when it could not get an answer, so a missing or refused socket, a hung or broken gateway, or an unreadable answer let a call that base protection would deny (sudo, writes outside the project) run unchecked. The launch now sets CANVASTTY_RUNTIME_FAIL_CLOSED=1 in the gate's own hook command; it installs the gate only when base protection is on or a decision plugin applies. With the flag, every way the check can fail is the CLI's deny JSON with "CanvasTTY safety check unavailable: this tool call was not run. Retry it, or ask the person how to proceed." The gateway marks an ask that stands for its own failure as unavailable, so Codex and Qwen Code (which cannot ask) deny it while Claude Code still asks the person. The OpenCode guard fails closed the same way. "none" is now told apart from an unreadable answer. Answered calls do the same work as before.
…ection
Base protection returned no deny for many destructive or outside-project
commands because it did not see the command or its target:
- a command after do/then/else/if/while/until/! in the same segment;
- env -i (its -i was taken as a value flag), env -S, busybox/toybox
applets, script -c CMD and BSD script FILE CMD;
- perl -i / ruby -i (perl is an interpreter, so the sed/perl branch was
unreachable);
- find -L/-H/-P/-O2/-f before the start folders (which also made
find -L dir -exec rm {} + inside the project look like rm .);
- cp/mv/install/ln -t DIR, tar -C DIR -x..., --directory=, unzip -o
(taken for 7z's -oDIR), bundled curl -fsSLo / wget -qO / -qP,
--output=, --output-dir, and curl/wget side files;
- curl -fsSLo f URL && sh f was not download-and-run.
Wrappers now have their own value-flag tables, and each form is tested
against targets outside (denied like the plain command) and inside the
project (allowed).
|
I reviewed
Please cover standalone |
…n either order
classifyFetch read curl's cookie-jar and header-dump files only inside a bundled cluster of two or more
letters, so `curl -c FILE` and `curl -D FILE` recorded no write, nor did --cookie-jar, --dump-header,
--trace, --trace-ascii, --stderr, --libcurl, --etag-save, --hsts, --alt-svc or a `-w '%output{FILE}'`
format. It also resolved -O as soon as it saw it, in the current folder, and then skipped --output-dir
because a destination was already set: `curl -O --output-dir /outside URL` looked like a write here.
A relative -o was never joined to --output-dir, which curl does (`--output-dir D -o F` writes D/F).
wget's -a, --output-file, --append-output, --save-cookies, --rejected-log and --warc-file were not read,
nor an attached `-O<file>`.
curl and wget are now read option by option from one table per program: a short option of its own,
attached or in a cluster, `--long VALUE` and `--long=VALUE` all go through the same path. The download's
files are resolved after the whole line is read, so -O and -o land in --output-dir wherever it stands,
one file per URL for repeated -O or --remote-name-all. `-` and /dev/null (and NUL) write no file, so
`curl -o /dev/null -w '%{http_code}' URL` and `curl -D /dev/null` are no longer refused. Other fetchers
keep their previous reading.
|
Thanks for the review. Both gaps reproduced, along with several others in the same parser. They are fixed in one new commit on top of Final head: Commit: Standalone
|
…e base for this change
|
The earlier findings about standalone P1: respect curl's operation boundaries at commandFacts.ts:658–702 collects all outputs into one array and keeps only the last For a session rooted in the current project, with an existing writable sibling directory: curl --output-dir ../outside -o a https://example.com/a \
--next --output-dir . -o b https://example.com/bcurl writes the first response to P2: do not treat Line 714 records a write to curl --output-dir ../outside https://example.comWithout The curl documentation for --output-dir explicitly describes both its dependence on output options and its scope up to the next These are static-review findings; I have not run the examples or tests. Both issues are also inherited by #94 and #95, so please carry the fixes into those branches. |
…-output-dir alone write nothing curl resets its per-transfer options at --next (-:, also inside a short cluster), so --output-dir of one operation does not move the -o / -O files of another. classifyFetch collected every output of the line and applied the last --output-dir to all of them, so `curl --output-dir ../out -o a URL --next --output-dir . -o b URL` was read as two writes inside the project. Each operation is now resolved with its own outputs, -O count, --remote-name-all and --output-dir, and the facts are combined. --output-dir without -o, -O or --remote-name-all sends the response to standard output; it no longer counts as a write to that folder. Side files (cookie jar, header dump, trace, ...) and real outputs are still judged.
|
Thanks. Both findings reproduced and are fixed in one new commit on top of Final head: Commit: P1: operation boundaries at
|
Problem
Base protection and plugin decisions reach Claude Code, Codex and Qwen Code through one
PreToolUsecommand hook,permission-gate.mjs. These CLIs block a tool call only when the hook prints a deny (or, for command hooks, exits with 2). The gate always exited 0, and whenever it got no answer it printed nothing. So whenever CanvasTTY could not answer, a call base protection would refuse (sudo,rm -rfoutside the project, writes to~/.zshrc) ran unchecked. That happens when the socket is missing or refused, the gateway hangs or throws, or the answer is unreadable. The OpenCode guard behaved the same way.I reproduced this with the real Claude Code 2.1.281 on a local Ollama model (
qwen3.5:9b). With the gateway gone,touch <project>/blocked.txtran, and the new real-Claude test fails onmainwith "the unchecked call did not run".A related audit finding (#11) is fixed in a separate commit. Base protection also returned no deny for many destructive or outside-project forms, because it did not see the command or its target at all.
What changes
1. The decision hook fails closed (commit 1)
CANVASTTY_RUNTIME_FAIL_CLOSED=1into that hook's own command (POSIXVAR='1' …, Windowsset "VAR=1" && …), next to the existing budget variable.CanvasTTY safety check unavailable: this tool call was not run. Retry it, or ask the person how to proceed.hookSpecificOutput.permissionDecision: "deny"on stdout with exit 0. That contract is already covered by the existing tests for all three CLIs and is verified live on Claude 2.1.281. No exit 2 is used, because the existing gate documents that exit 2 means something else for some CLIs.askthat stands for its own failure asunavailable: true: the deadline passed, the handler threw or rejected, or the handler returned a behavior it cannot read. Claude Code can ask, so it still getsaskand asks the person. Codex and Qwen Code cannot take an ask from a hook, so the fail-closed gate denies instead.opencode-decisions.mjs, active only when its decisions flag is set at launch) fails closed the same way: the tool call throws the same message.parseDecisionnow tells a real"none"(no verdict) apart from an unreadable answer (null), so "no verdict" still prints nothing and the call runs.Failure table, session launched with the decision hook
Without the flag (a gate started some other way), every row keeps its "before" behavior. The tests check this row by row.
One gap remains: a hook that cannot start at all (a broken install where node or Electron is missing) exits non-zero and the CLI runs the call. That can only be closed inside each CLI.
2. Base protection reads more command forms (commit 2, audit #11)
All of the forms below returned no deny (
denyRule(analyzeAction)gavenull) while the plain command is denied. Each is now denied outside the project exactly like the plain command, and stays allowed with a target inside the project:for …; do rm -rf OUT; done,if …; then …,else,elif,while/until … do,! rm -rf OUT,if rm -rf OUT; then …env -i rm …(its-iwas taken as a value flag),env -i PATH=… rm …,env --ignore-environment,env -S 'rm …',env -C OUT rm -rf xbusybox rm …,busybox sh -c '…',toybox rm …script -q -c "rm -rf OUT" /dev/null,script -qc …,script --command …, BSDscript -q /dev/null rm -rf OUTperl -pi -e … OUT/f,perl -i -pe,-pi.bak,-pie,ruby -pi -e …(the sed/perl branch was unreachable because perl is an interpreter)find -L OUT -delete,-H,-P,-O2, BSDfind -f OUTcp -t OUT a,--target-directory[=]OUT,install -t,ln -s -ttar -C OUT -xzf a.tgz,tar -x -f a.tar --directory=OUT,tar --directory OUT -xf,bsdtar -C OUT -xfunzip -o a.zip -d OUT,unzip -oq …(-owas taken for 7z's-oDIR)curl -fsSLo OUT/x URL,curl -sLoOUT/x,--output=,--output-dir OUT -O,curl -c OUT/jar,curl -D OUT/headers,wget -qO OUT/x,wget -qP OUT,wget -qo OUT/logcurl -fsSLo i.sh URL && sh i.sh,curl -sLoi.sh …; bash i.sh,--output=,--output-dir … -O … && sh …,wget -qO i.sh … && sh ./i.shEach wrapper (
env,time,nice,ionice,timeout,stdbuf,exec,caffeinate,watch,xargs) now has its own table of value flags instead of one shared list.rsync -t(preserve times) is not read as a target directory. One over-block went away:find -L build -exec rm {} +inside the project was refused asrm ..Security rationale
PreToolUseonBash|Write|Edit|MultiEdit|NotebookEdit, CodexBash|apply_patch|Edit|Write, Qwen shell and write tools, OpenCode shell and file writes), and only for sessions where the person has something deciding. Lifecycle hooks stay fail-open, because a failed status update must never block work.Tests
Failing first: before the fix, the new fail-closed tests had 8 of 9 failing (logs kept locally), the new base-protection table had 94 failing cases, and the real-Claude test failed on
main's gate.tests/permission-gate-fail-closed.test.mjsruns the real gate process against fakes and the realRuntimeGateway. For each failure mode it checks the flag on and off × the Claude, Codex and Qwen hook input shapes:It also checks that normal answers are unchanged, including base protection turned off mid-session, and that the flag is in the Claude, Codex and Qwen hook commands, with the Windows form too. A launch with protection off and no plugin gets no hook. The OpenCode guard fails closed.
tests/permission-gate-fail-closed-real.test.mjsruns the real Claude Code CLI on a local Ollama model, under a throwaway HOME. The hook is installed by the realAgentRuntimeBridge. A Bashtouchruns while the gateway answers, and aftergateway.close()the same kind of call does not run: the file is absent and the tool result carries the fail-closed message. It is skipped withoutclaudeor without a local Ollama server that already hasqwen3.5:9b,gpt-oss:20borCANVASTTY_TEST_OLLAMA_MODEL. Nothing is pulled. Passed locally with Claude 2.1.281 andqwen3.5:9b(about 2.5 min).tests/base-protection.test.mjs: a new table covers every form above, with two outside targets (absolute and../) and two inside targets each. It also covers download-and-run through every output-flag spelling, and ordinary uses of the same programs that must stay allowed.tests/decision-hooks.test.mjs: theparseDecision("none")expectation is updated for the new "no verdict" versus "unreadable" distinction.Gates, all run under a throwaway HOME:
npm run typecheck,npm test(1024 tests, 0 failures, 0 skipped: the real-Claude test ran),npm run build,npm run audit:secrets,npm run test:evenall pass.Compatibility with open PRs
The branch starts at
origin/main(31f287d) and has two commits. It merges cleanly with #89 (perf/runtime-costs) and with #90 (perf/claude-http-hooks), each alone and all three together. On the merged tree, typecheck passes and the decision, fail-closed, base-protection, budget and #90 HTTP-hook tests (the #90 real-Claude one included) pass: 52 of 52. #90 keepsPreToolUseon the helper, so the fail-closed gate covers it unchanged.