Skip to content

fix(safety): fail closed when the decision hook cannot check a call; read more command forms in base protection - #92

Merged
howdeploy merged 4 commits into
howdeploy:mainfrom
BIackFIame:fix/permission-gate-fail-closed
Sep 28, 2026
Merged

howdeploy merged 4 commits into
howdeploy:mainfrom
BIackFIame:fix/permission-gate-fail-closed

Conversation

@BIackFIame

Copy link
Copy Markdown
Contributor

Problem

Base protection and plugin decisions reach Claude Code, Codex and Qwen Code through one PreToolUse command 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 -rf outside 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.txt ran, and the new real-Claude test fails on main with "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)

  • The launch installs the decision hook only when something decides for the session: base protection is on, or a decision plugin applies. It now also puts CANVASTTY_RUNTIME_FAIL_CLOSED=1 into that hook's own command (POSIX VAR='1' …, Windows set "VAR=1" && …), next to the existing budget variable.
  • With the flag, every way the check can fail becomes the CLI's deny, and the model reads:
    CanvasTTY safety check unavailable: this tool call was not run. Retry it, or ask the person how to proceed.
  • The deny uses the same channel as base protection's own deny: 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.
  • The gateway marks an ask that stands for its own failure as unavailable: 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 gets ask and asks the person. Codex and Qwen Code cannot take an ask from a hook, so the fail-closed gate denies instead.
  • The OpenCode guard (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.
  • parseDecision now tells a real "none" (no verdict) apart from an unreadable answer (null), so "no verdict" still prints nothing and the call runs.
  • Base protection off (the person's choice) and no decision plugin: the launch installs no decision hook, so nothing changes. If the person turns base protection off while a card runs, CanvasTTY answers "no verdict" and the call runs. Only a CanvasTTY that cannot answer at all still denies for that card until it is restarted.
  • Non-decision hooks (lifecycle, plugin hook runner) are unchanged and stay fail-open.
  • Latency: an answered call does exactly the same work as before (one env read). The deny path answers within the helper deadline (12 s by default, measured 12.1 s), well inside the CLI's hook timeout (15 s), so the CLI's own timeout never decides.

Failure table, session launched with the decision hook

Failure Claude before Claude after Codex / Qwen before Codex / Qwen after OpenCode before OpenCode after
Socket file missing (CanvasTTY quit, runtime dir cleaned) runs deny runs deny runs deny (throws)
Connection refused (stale socket) runs deny runs deny runs deny
No answer before the helper deadline (gateway hung) runs deny runs deny runs deny
Unreadable answer: not JSON, wrong request id, wrong version, unknown behavior, over 64 KB runs deny runs deny runs deny
Socket closed without an answer (unknown or revoked capability) runs deny runs deny runs deny
Gateway's own failure: handler throws or rejects, nonsense answer, gateway deadline ask ask (unchanged) runs deny runs deny
Hook input over 512 KB, not JSON, no tool name, or runtime identity missing runs deny runs deny n/a n/a
Normal answers: deny / ask / allow / no verdict unchanged unchanged unchanged unchanged unchanged unchanged

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) gave null) 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:

Form Rule outside
for …; do rm -rf OUT; done, if …; then …, else, elif, while/until … do, ! rm -rf OUT, if rm -rf OUT; then … delete-outside
env -i rm … (its -i was taken as a value flag), env -i PATH=… rm …, env --ignore-environment, env -S 'rm …', env -C OUT rm -rf x delete-outside
busybox rm …, busybox sh -c '…', toybox rm … delete-outside
script -q -c "rm -rf OUT" /dev/null, script -qc …, script --command …, BSD script -q /dev/null rm -rf OUT delete-outside
perl -pi -e … OUT/f, perl -i -pe, -pi.bak, -pie, ruby -pi -e … (the sed/perl branch was unreachable because perl is an interpreter) write-outside
find -L OUT -delete, -H, -P, -O2, BSD find -f OUT delete-outside
cp -t OUT a, --target-directory[=]OUT, install -t, ln -s -t write-outside
tar -C OUT -xzf a.tgz, tar -x -f a.tar --directory=OUT, tar --directory OUT -xf, bsdtar -C OUT -xf write-outside
unzip -o a.zip -d OUT, unzip -oq … (-o was taken for 7z's -oDIR) write-outside
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/log write-outside
curl -fsSLo i.sh URL && sh i.sh, curl -sLoi.sh …; bash i.sh, --output=, --output-dir … -O … && sh …, wget -qO i.sh … && sh ./i.sh download-exec

Each 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 as rm ..

Security rationale

  • Base protection is a guard the person turns on and expects to hold. A guard that disappears whenever its backend is slow, restarting or broken is worst exactly when something is already wrong, and it gives an agent a reason to hang or crash the gateway. Failing closed turns "CanvasTTY is unavailable" into a visible, recoverable refusal. The model is told to retry or ask the person, and nothing destructive runs unchecked.
  • Fail closed applies only to decision events (PreToolUse on Bash|Write|Edit|MultiEdit|NotebookEdit, Codex Bash|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.
  • The flag lives in the hook's own command in the launch's settings, not in the agent's environment. So the agent cannot turn it off by editing its shell environment, and a gate without the flag behaves exactly as before.
  • Claude Code keeps asking the person on gateway failure. Denying there would take away the person's choice for no gain.
  • The same deny channel is used for every answer. A fail-closed deny is exactly as strong as a base-protection deny and adds no new parsing surface in any CLI.

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.mjs runs the real gate process against fakes and the real RuntimeGateway. For each failure mode it checks the flag on and off × the Claude, Codex and Qwen hook input shapes:

    • socket missing;
    • connection refused (a stale socket file left by a killed listener);
    • no answer, with the deadline checked against the hook timeout;
    • six malformed answers (not JSON, another request's id, unknown behavior, wrong version, closed without an answer, over the size bound);
    • the gateway's own failure (handler throws, rejects late, returns nonsense, never answers);
    • an unknown capability;
    • unreadable hook input (over 512 KB, not JSON, no tool name, no runtime identity).

    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.mjs runs the real Claude Code CLI on a local Ollama model, under a throwaway HOME. The hook is installed by the real AgentRuntimeBridge. A Bash touch runs while the gateway answers, and after gateway.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 without claude or without a local Ollama server that already has qwen3.5:9b, gpt-oss:20b or CANVASTTY_TEST_OLLAMA_MODEL. Nothing is pulled. Passed locally with Claude 2.1.281 and qwen3.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: the parseDecision("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:even all 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 keeps PreToolUse on the helper, so the fail-closed gate covers it unchanged.

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).

Copy link
Copy Markdown
Owner

I reviewed 13c4c40. The fail-closed change is valuable, but the command-parser changes appear to leave some of the advertised curl cases uncovered. This is a static review; I have not executed these cases.

  • Standalone side-output flags: commandFacts.ts:628–638 handles cookie/header files only inside the bundled-short-option branch, which requires at least two letters. Consequently, curl -sc /outside/jar URL and curl -sD /outside/headers URL are recognized, but curl -c /outside/jar URL and curl -D /outside/headers URL appear to record no write target. The description lists the standalone forms; the added tests cover only the bundled forms.
  • Output-directory ordering: commandFacts.ts:610–626 records the remote-name destination immediately when it sees -O. For curl -O --output-dir /outside https://example.com/file, that records a target in the current directory before reading --output-dir. The final directory fallback is then skipped because explicit is already true.

Please cover standalone -c/-D and both orders of -O/--output-dir, with targets inside and outside the project. These are remaining parser gaps, not evidence that this PR introduces those bypasses or that the separate fail-closed change should be held back.

…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.
@BIackFIame

Copy link
Copy Markdown
Contributor Author

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 13c4c40 with no history rewrite. The new test was run against 13c4c40 first: 33 of its 46 forms were not refused with an outside target, and the -O --output-dir build … && sh build/i.sh case was not seen as download-and-run.

Final head: e4c46da258d405844b054c65b337c4d6cd923361.

Commit: e4c46da258d405844b054c65b337c4d6cd923361

Standalone -c / -D (and the other side-output flags)

Root cause. classifyFetch read curl's cookie-jar and header-dump letters only inside a bundled cluster of two or more letters. Your two examples showed this: curl -c FILE and curl -D FILE recorded no write. None of these were read in any spelling either:

  • --cookie-jar and --dump-header;
  • --trace, --trace-ascii, --stderr, --libcurl, --etag-save, --hsts and --alt-svc;
  • a -w '%output{FILE}' or %output{>>FILE} format, which curl 8 writes to FILE.

On the wget side, -a, --output-file, --append-output, --save-cookies, --rejected-log, --warc-file and an attached -O<file> were not read.

Fix. curl and wget are now read option by option from one table per program that says what each option names: the download, its folder, a side file, or a write-out format. A short option on its own, attached (-cFILE), inside a cluster (-sSc FILE, -fsSLD FILE), --long VALUE and --long=VALUE all go through the same path. I kept the = form even though curl 8.7 rejects --opt=value: reading it costs nothing and matches the existing --output= handling. Long options that take a value (--header, --data, --user and others) skip that value, so the value is not read as a flag.

- and harmless devices (/dev/null, NUL) write no file. This also removes an existing over-block: curl -o /dev/null -w '%{http_code}' URL and curl -sSo /dev/null URL were refused as write-outside at 13c4c40. With standalone -D recognized, curl -D /dev/null URL would have been refused as well.

-O / -o and --output-dir in either order

Root cause. -O was resolved into the current folder as soon as it was read, and explicit then suppressed the --output-dir fallback. There was also a related gap: a relative -o was never joined to --output-dir. I checked with curl 8.7.1 using a file:// URL:

  • -o rel.txt --output-dir d writes d/rel.txt;
  • -o /abs/x --output-dir d writes under d/ (curl joins the folder even to an absolute path).

So curl -o x --output-dir /outside URL was judged as an in-project write.

Fix. The download's files are resolved after the whole line has been read:

  • curl's -o and -O files land in --output-dir wherever it stands, joined the way curl joins them;
  • each -O takes the next URL's name, and --remote-name-all names every URL;
  • an output folder or name that cannot be expanded (a substitution or an unknown variable) stays unresolved, as other targets do;
  • wget keeps its reading: -O is the file, and -P is the folder its default name lands in;
  • the other fetchers (iwr, aria2c, http and others) keep exactly their previous reading.

Tests

tests/base-protection.test.mjs: "curl and wget: every spelling of an output or side file, and --output-dir in either order, is judged where it lands".

  • 46 forms, each with targets outside (/…/elsewhere and ../elsewhere, expected write-outside) and inside (build, <project>/build, expected no deny). They cover standalone, attached, bundled and long spellings of every flag above, and -O --output-dir, --output-dir -O, -fsSLO --output-dir, -o x --output-dir, --output-dir -o x, --output-dir=, --remote-name-all with two URLs, and -O URL --output-dir D -O URL2.
  • Download-and-run: curl --output-dir build -o i.sh URL && sh build/i.sh, the reverse order, curl -O --output-dir build …/i.sh && sh build/i.sh, and wget -a build/log -O i.sh URL && sh i.sh.
  • Ordinary uses stay allowed: curl -D -, --trace -, --stderr -, -o -, -c -, -b/--cookie (read only), -w '%{http_code}', -w @file, -H 'Host: …', -K, -o /dev/null, -D /dev/null -c /dev/null, -w '%output{/dev/stderr}', wget -O /dev/null, wget -a /dev/null, wget --load-cookies, and curl -4 -sS.

The existing wrapped-form test, including its bundled -sc/-sD cases, is unchanged and passes. The CHANGELOG line (EN/RU/ZH) now lists these forms.

Also checked. curl -o ~/x and --output-dir ~ -O are refused; curl -o "$(pwd)/../x" and --output-dir "$D" stay unresolved (not denied), as before. curl … | sh is still pipe-to-shell, and wget URL/a.sh && sh a.sh is still download-and-run.

Gates on e4c46da: npm run typecheck, npm run build and npm run audit:secrets pass. On the full suite (node --test --test-concurrency=2, 1025 tests), the first run had one failure outside this change: an ENOTEMPTY in the temp-folder cleanup hook of tests/even-g2-controller.test.mjs. That file passed twice on its own, and a second full run passed.

Copy link
Copy Markdown
Owner

The earlier findings about standalone -c / -D and the order of -O / --output-dir are addressed in the current code and regression tests. Two issues remain in classifyFetch at e4c46da; please fix these before merge.

P1: respect curl's operation boundaries at --next / -:.

commandFacts.ts:658–702 collects all outputs into one array and keeps only the last dir. It ignores --next, although curl resets local options at that boundary.

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/b

curl writes the first response to ../outside/a. The analyzer applies the final . to both outputs and therefore misses write-outside. Please resolve each operation's outputs with that operation's options, then combine the facts. Add regression cases for both --next and its -: spelling, including reversed inside/outside order.

P2: do not treat --output-dir alone as a file write.

Line 714 records a write to dir when there are no output files. This incorrectly blocks:

curl --output-dir ../outside https://example.com

Without -o / -O (or another file-writing option), the response goes to stdout. Please remove that inferred write while preserving checks for actual output and side files, and add a regression asserting no base-protection denial for this case.

The curl documentation for --output-dir explicitly describes both its dependence on output options and its scope up to the next --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.
@BIackFIame

Copy link
Copy Markdown
Contributor Author

Thanks. Both findings reproduced and are fixed in one new commit on top of e4c46da, with no history rewrite. I ran the new test against e4c46da first, and it failed there.

Final head: d2fddde22b5200e67d754b7abd1d0eff6badb482.

Commit: d2fddde22b5200e67d754b7abd1d0eff6badb482

P1: operation boundaries at --next / -:

Root cause. classifyFetch put every -o and -O of the line into one list and kept only the last --output-dir. It then joined that folder to all outputs. curl resets per-transfer options at --next, so at e4c46da your example curl --output-dir ../outside -o a URL --next --output-dir . -o b URL was read as two writes inside the project. I checked this with curl 8.7.1 using file:// URLs: the first file lands in the first operation's folder and the second in the working folder. curl also ends an operation at -: inside a short cluster (-s:).

Fix. curl is now read one operation at a time. --next, a standalone -: and : inside a short cluster each close the current operation. Each operation resolves its own -o files, -O count, --remote-name-all and --output-dir, taking -O names from that operation's URLs. The write facts of all operations are then combined. Side files (cookie jar, header dump, trace and the rest) are still recorded as they are read, whichever operation they belong to. wget has no operation separator, so its whole line remains one operation.

P2: --output-dir alone is not a write

Root cause. When the line had no output file, the old code recorded a write to the --output-dir folder. Without -o, -O or --remote-name-all, curl writes the response to standard output. I confirmed that curl --output-dir d file://… creates nothing in d.

Fix. That inferred write is removed. An operation with --output-dir but no output files records nothing for the folder. Real outputs in the same or another operation, and side files, are still judged. I checked wget for the same pattern: wget -P DIR URL really saves the URL's file into DIR (wget writes to a file by default), so its folder write stays.

Tests

tests/base-protection.test.mjs: "curl: each operation between --next / -: uses its own --output-dir, and --output-dir alone writes nothing". Every case runs with both --next and -:, and with both /…/elsewhere and ../elsewhere as the outside target:

  • Denied as write-outside:
    • your example with the outside folder first, and the same in reversed order (outside folder in the second operation);
    • -O with --output-dir ../out in the first operation and build in the second, and the reverse;
    • --output-dir= spelling;
    • --output-dir placed after -o in either operation;
    • -s: inside a cluster.
  • Allowed: a folder in one operation does not move the other operation's file (-o a URL --next --output-dir OUT URL, and the reverse), plus inside-only forms.
  • Download-and-run: a file downloaded by the second operation into its own folder and then run (… --next --output-dir build -o i.sh URL && sh build/i.sh).
  • --output-dir alone is not denied: curl --output-dir OUT URL, --output-dir=OUT, and --output-dir after the URL.
  • Side files next to a lone --output-dir are still denied: -D OUT/h, and -c OUT/jar with --output-dir build.

Typecheck and the full suite (1026 tests) pass at d2fddde.

howdeploy added a commit that referenced this pull request Sep 28, 2026
Integrate the reviewed performance and reliability changes from #89, #90, #92, #93, #94, and #95.
@howdeploy
howdeploy merged commit b57b9dd into howdeploy:main Sep 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants