feat(security): let an unattended session declare what it may run when the approval classifier is down

[OmarB97]

The fail-closed site

tools/approval.py::_smart_approve ends in a bare except Exception that returns "escalate":

    except Exception as e:
        logger.warning("Smart approvals: LLM call failed (%s: %s) — escalating ...")
        return "escalate"

"escalate" means ask a human. Under approvals.mode: smart that verdict reaches check_all_command_guards (Phase 2.5) and check_execute_code_guard, and both then take the human-approval path. With a person at the keyboard that is exactly right, and it should stay that way.

In an unattended run there is no person. Observed live on 2026-08-02, in a hermes desktop spawn --delegated session: every flagged command spent the full approvals.timeout (60s) waiting for an approval nobody was going to give, and was then BLOCKED — or, with no notify callback registered, came straight back as status: "pending_approval" and halted the operation. The run never made a decision; it just stopped.

Two triggers produced it. The classifier is itself an auxiliary.* call, so on a single-slot local server it self-contended with the session's own in-flight turn and timed out; separately, an unkeyed auxiliary lane made it raise on every call. Both are now mitigated by auxiliary.route (#340). What #340 did not change is the shape of the failure: classifier unavailability still degrades to "ask a human" rather than to a declared policy. That is what this closes.

Why an allowlist is the right degradation

The three obvious alternatives are all worse:

• Fail open on classifier error. Turns an infrastructure outage into a blanket bypass of the approval gate. No.
• Retry / longer timeout. The failure is not transient — an unkeyed lane raises identically every time. It buys latency, not a decision.
• Auto-approve in unattended contexts. Same as fail-open, wearing a hat.

What is actually missing is a decision the operator already knows how to make. An operator spawning "run the Godot export in this worktree" knows in advance that godot and git inside that worktree are fine, and knows it before the run starts. The allowlist is a place to write that down, so the run degrades to the operator's own policy instead of to a stall.

It is scoped as narrowly as the failure it answers:

• Only when the classifier was unreachable. A verdict of ESCALATE means the classifier is up and unsure — a human should still decide. DENY still denies. _smart_approve's return alphabet is unchanged (a dozen tests and the plugin observer hooks key off it); "was never consulted" travels beside the verdict on a thread-local that the caller clears immediately before each call, so a stubbed _smart_approve always reports "available".
• Only where there is nobody to ask. Interactive CLI is excluded: the prompt reaches someone immediately, so there is nothing to degrade from.
• Only for env_type == "local". A declared root names a directory on this machine; the same string means somewhere else inside a container or across SSH.
• Not for execute_code. An allowlist names executables and a worktree; an execute_code payload is a Python script with no argv to judge. That path stays fail-closed.

What the matcher refuses

Matching is on parsed argv plus the resolved working directory, never a regex or substring over the raw shell string — quoting alone defeats those. tools/declared_allowlist.py, ~100 lines of policy, deliberately paranoid because the command text is written by a model that may itself be prompt-injected.

A character allowlist runs first, before anything is parsed, so a command that could chain or substitute is refused on its syntax and never gets judged on a benign-looking first word:

Attempt	Refused because
git status ; rm -rf /tmp/gone	; is not in the character allowlist
git status && rm -rf /tmp/gone, ||	same
git status | tee /etc/passwd, > /etc/passwd	pipe / redirect
git status $(rm -rf /tmp/gone), backticks	command substitution
git status ${IFS}foo	parameter expansion
git clean -fdx *, ~	glob / home expansion — the result cannot be checked
git status + newline + rm ...	newline separator
git clean -fdx \; rm ...	backslash escapes

Then shlex.split, and the remaining checks:

Attempt	Refused because
git -C /etc status	/etc resolves outside the declared root
git clean -fdx /tmp, ../outside	argument outside the root
git clean -fdx escape where escape → /somewhere/else	Path.resolve() follows the symlink and judges the target
git --git-dir=/etc/git clean -fdx	the value half of --opt= is checked too
running the command from outside the root	the resolved cwd must be inside it
GIT_DIR=/etc/git git clean -fdx	leading environment assignment
git -c core.pager=/bin/sh status, -c diff.external=python3, --pager=sh	an argument naming a program-bearing command — a declared git running an undeclared shell
/usr/bin/git, ./git when git was declared	a declaration names one exact executable
declaring sh, python, sudo, env, ssh, docker, xargs, timeout …	program-bearing: they run whatever their arguments name, so allowlisting one allowlists everything
declaring commands with no root, or a relative root	an unscoped git could act on any repository on the machine

Every token after argv[0] is treated as a possible path, including bare names — git add link where link is a symlink out of the worktree is precisely what a "does it look like a path?" heuristic would wave through. Tokens that genuinely are not paths resolve under the working directory, which is already inside the root, so ordinary invocations are unaffected.

The git -c core.pager=/bin/sh row came out of probing the finished matcher rather than reasoning about it, and it is worth calling out because the first version got it wrong: -c is a bare flag so it was skipped, and core.pager=/bin/sh does not start with -, so checking the whole token resolved it to an innocent-looking relative name under the worktree — while the half git actually executes is an absolute path to a shell. Both halves of a key=value argument are now checked, and a program-bearing name is refused wherever it appears rather than only in the executable slot. There is a regression test per variant, and a companion test asserting the strictness costs none of the invocations people actually run (git commit -m 'ship it', git log --format=%H, git config user.name=me, godot --headless --export-release …).

The cwd the matcher judges is the one the command will actually run in: terminal_tool now resolves workdir/session-cwd before the guard rather than after it. Judging the session default while an explicit workdir= sent the command elsewhere would enforce the root against the wrong directory — there is a test for exactly that.

Where a declaration comes from

Both surfaces are empty by default.

Per delegation (wins over the config default, never written to disk):

hermes desktop spawn --delegated \
  --allow-command godot --allow-command git \
  --allow-command-root /Users/me/Workspaces/game \
  "Run the export and report what broke"

CLI → spawn-control.ts → renderer → session.create (allowed_commands / allowed_command_root) → tools.approval.set_session_command_allowlist, keyed to the same session key the guards read back. This mirrors the --toolsets / --goal pins exactly, including the "a declaration that cannot be honored FAILS the create" rule — handing back a session that silently has no allowlist is the stall the caller was avoiding. Cleared on session teardown.

Profile-wide, for a box that runs unattended (config.yaml, not .env — this is behaviour, not a secret):

approvals:
  delegated_allowlist:
    commands: ["godot", "git"]
    root: "/Users/me/Workspaces/game"     # required

Every policy approval is logged at WARNING with the command and the reason, and shows up in the transcript as an approval note. Nobody watched it happen, so the record has to stand on its own.

Unchanged

• No declaration → byte-identical behaviour. The shipped default is empty; the resolver returns None and control falls through to the same fail-closed path.
• The hardline blocklist, the sudo-stdin guard, approvals.deny, the yolo/mode: off bypasses and the container fast-paths all run before any of this and are untouched.
• The non-interactive early return in check_all_command_guards (~:3245) and the cron path are unchanged — neither stalls today, so neither needed a degradation.
• _smart_approve's return contract is unchanged.

Testing

scripts/run_tests.sh only.

• tests/tools/test_declared_command_allowlist.py (new, 74 tests) — the required coverage: no declaration → classifier failure still escalates; declared + unreachable + matching → policy-approved with a logged reason; declared + non-matching → still escalates; the evasion table above; plus a working-classifier-ESCALATE and a working-classifier-DENY case proving the allowlist does not override a reachable classifier, and an end-to-end terminal_tool pair proving the cwd plumbing is live rather than dead code. The classifier is made unavailable by having agent.auxiliary_client.call_llm raise — the real fail-closed site, not a stubbed _smart_approve.
• tests/tui_gateway/test_declared_allowlist_session_scope.py (new) — session.create binding, pairing/shape refusals, no session or lease left behind by a refused create, teardown cleanup.
• tests/hermes_cli/test_desktop_spawn.py, apps/desktop/electron/spawn-control.test.ts, apps/desktop/src/app/session/session-overrides.test.ts — wire shape and the flag combinations argparse cannot express.
• Kept green: tests/tools/test_smart_approval_injection.py (22), tests/tools/test_execute_code_approval_cluster.py (27), tests/tools/test_approval_plugin_hooks.py, tests/tools/test_approval.py, the terminal-tool suites.
• TS: npx vitest run on both touched files, tsc --noEmit on the renderer and electron projects, eslint clean.

Full sweep of tests/tools tests/tui_gateway tests/hermes_cli — 831 files, 18,301 passed / 19 failed. Every one of the 19 was re-run on a detached origin/main worktree with none of this applied, and reproduces there:

File	Fails	Why
test_gateway_service.py, test_service_manager.py, test_signal_handler_kanban_worker.py, test_gateway_wsl.py	4 / 2 / 1 / 2	systemd + WSL tooling that does not exist on macOS
test_execution_flag_detection.py	3	BSD man/sort vs the GNU invocation grammar the fixtures encode
test_file_tools.py, test_mcp_tool_issue_948.py	3 / 1	identical on origin/main
test_approval.py::test_nonrecursive_verification_artifact_cleanup_is_not_dangerous	1	tempfile.gettempdir() mocked to /tmp; macOS resolves /private/tmp
test_base_environment.py::TestAtomicSnapshotConcurrencyBehavioral	1	known concurrency flake, red on origin/main too
test_approved_command_clean_slate.py::test_execute_code_non_approved_still_interrupts_on_stale_bit	1	load-dependent race (asserts a 0.5s script is killed before it prints); 2 of 3 standalone runs on origin/main fail

One trap worth writing down for anyone reproducing this locally: run_tests.sh re-exports HOME, and tools/approval.py loads command_allowlist from ~/.hermes/config.yaml at import. A single command_allowlist: ['execute_code'] entry on the host — one "Always Approve" click anywhere on the machine, at any point — makes is_approved() short-circuit and turns 9 approval tests red with a bare {'approved': True, 'message': None} that reads exactly like a regression in this change. It bit me mid-session on files that had been green an hour earlier at the same commit. Run the approval files with HOME=$(mktemp -d) HERMES_PYTHON=<absolute path to the venv python> scripts/run_tests.sh … before believing a local failure there.

Platform: macOS 15 (Darwin 25.6.0), Python 3.11 venv.

🤖 Generated with Claude Code