fix(security): close kanban worker auto-approve gap for dangerous commands

[webdevtodayjason]

What

Kanban-dispatched worker subprocesses — the primary execution vehicle for long-horizon, unattended multi-agent work in Hermes — fall through every approval-context check in tools/approval.py and silently auto-approve any non-hardline dangerous command, with only a logger.warning() left behind.

Why

Same bug class as #30882 (P0, fixed in #34497): an execution surface the approval system doesn't recognize defaults to trusted-by-omission instead of trusted-by-explicit-config. That fix's own description states the philosophy plainly: "The approval gate is a documented heuristic in SECURITY.md, not a security boundary; this restores its intended behavior — it doesn't add one."

Unlike #30882 (a ContextVar-propagation regression in threads), this is a plain omission, not a regression — kanban workers are separate subprocess processes (not threads sharing contextvars), and the approval-context question was simply never wired in when kanban dispatch was built.

hermes_cli/kanban_db.py's worker-spawn env construction (_default_spawn) sets 15+ environment variables (HERMES_KANBAN_TASK, HERMES_KANBAN_WORKSPACE, etc.) but never any of the four flags tools/approval.py checks to recognize a non-interactive context: HERMES_CRON_SESSION, HERMES_GATEWAY_SESSION, HERMES_SESSION_PLATFORM, HERMES_INTERACTIVE. All three approval-gate functions (check_dangerous_command, check_all_command_guards, check_execute_code_guard) therefore fall through to the bare non-interactive auto-approve branch.

Change

Mirrors the proven cron_mode pattern exactly — not a new mechanism:

• HERMES_KANBAN_SESSION env flag set at worker spawn time (hermes_cli/kanban_db.py)
• approvals.kanban_mode config, default deny (matching cron_mode's default)
• _get_kanban_approval_mode() mirrors _get_cron_approval_mode() verbatim
• A parallel deny-by-default branch added to all three call sites in tools/approval.py, alongside the existing cron branch — not replacing it
• approvals.kanban_mode: approve remains available for users who've deliberately decided to trust their kanban workers, same as cron_mode

kanban_mode lives under approvals (not the kanban: config block) — that block has a known, separately-filed duplicate-key bug (#55779) that silently drops the first of two "kanban" keys in the same DEFAULT_CONFIG dict literal.

How to test

python -m pytest tests/tools/test_kanban_approval_mode.py tests/tools/test_cron_approval_mode.py tests/tools/test_execute_code_approval_cluster.py tests/tools/test_approval.py -v

Verified against the real, unmodified functions (not mocked) before writing any fix code:

# Real env captured from kanban_db._default_spawn() via a real task in a
# scratch kanban DB (subprocess.Popen intercepted, not launched):
captured_env["HERMES_CRON_SESSION"]      # None
captured_env["HERMES_GATEWAY_SESSION"]   # None
captured_env["HERMES_SESSION_PLATFORM"]  # None
captured_env["HERMES_INTERACTIVE"]       # None

# Feeding that exact env into a fresh process and calling the real,
# unmodified check_dangerous_command():
check_dangerous_command("chmod 777 /some/file", env_type="local")
# main (pre-fix):  {'approved': True, 'message': None}  -- silent auto-approve
# this PR (post-fix): {'approved': False, 'message': 'BLOCKED: ...'}
# approvals.kanban_mode: approve restores the opt-in pass-through

• All 19 new tests in test_kanban_approval_mode.py pass; all pre-existing cron/CLI/gateway tests across the 4 touched test files stay byte-stable when run per-file (matching scripts/run_tests.sh's per-file isolation).
• scripts/check-windows-footguns.py clean on the diff (pure env-var/config logic).

Platforms tested

macOS (logic-only change, no OS-specific code paths — pure env-var/config string matching).

Note

While testing I found an unrelated, pre-existing test-isolation issue (test_cron_approval_mode.py + test_approval.py leak shared module state when run together in one pytest process — confirmed on unmodified main too, invisible to CI because scripts/run_tests.sh isolates each test file into its own subprocess). Filed separately, not part of this diff.

Fixes #55945

[teknium1]

Thanks for addressing the unattended Kanban-worker path. The fail-closed policy direction is useful, but the current implementation does not reach that policy for the actual worker process.

Problems

• hermes_cli/kanban_db.py:8021-8048 launches hermes ... chat -q, while cli.py:15834-15836 unconditionally sets HERMES_INTERACTIVE=1. The new Kanban branch is nested under _run_approval_gate()'s not is_cli and not is_gateway condition (tools/approval.py:2092-2097), so the terminal/plugin path skips it. The new tests delete that marker and call guards directly, rather than exercising worker startup.
• _default_spawn() still copies all parent environment state (hermes_cli/kanban_db.py:7931). Inherited gateway/ask/cron state can preempt the new policy; check_execute_code_guard() handles cron before the proposed Kanban branch (tools/approval.py:3018-3038). This matches the detached-callback issue documented in linked #63183.

Suggested changes

• Make Kanban worker identity/policy explicit and higher precedence than ambient CLI/gateway/cron state, and add a spawn-to-guard regression test seeded with those markers.
• Document approvals.kanban_mode beside the existing approvals keys in website/docs/user-guide/security.md:32-48.

Automated hermes-sweeper review.

[teknium1]

_default_spawn() still begins from dict(os.environ), so HERMES_EXEC_ASK, gateway/cron markers, and process-level YOLO remain in the child. Those markers can select an existing approval path before this new Kanban marker is consulted; please establish an explicit worker context and test seeded parent markers.

[teknium1]

This branch is only reached after _run_approval_gate() has classified the caller as neither CLI nor gateway. A dispatcher worker runs hermes chat -q, and cli.py:15836 sets HERMES_INTERACTIVE=1 before processing that query, so the actual terminal/plugin worker path bypasses this branch. Make Kanban policy take precedence or ensure worker startup has an explicit noninteractive context.

[webdevtodayjason]

Good catch, that was exactly the hole. You're right that cli.main exports HERMES_INTERACTIVE=1 before the query runs, so the kanban branches were dead code for real workers, and _default_spawn copied the dispatcher's whole environment on top of that.

Fixed on the current head (df8cc2df5, "enforce kanban worker approval policy on the real worker path"). Three coordinated changes: HERMES_KANBAN_SESSION is now decided before any ambient context inference in all three gates with both arms returning, the spawn env scrubs the seven inherited markers alongside the existing HERMES_TUI pop, and worker startup no longer self-marks interactive when the kanban marker is set.

tests/tools/test_kanban_worker_real_path.py drives the real _default_spawn env through the real gate entry points: 12 of 14 cases fail on the old code and all pass now, existing approval suites unchanged. An independent security review also reproduced the bypass on current main (a real-spawn probe inherited yolo/gateway state and auto-approved rm -rf) and confirmed the reviewed head scrubs that state and denies the dangerous command, execute_code, and plugin escalation while still honoring an explicit kanban approve mode. Branch is rebased clean on current main now if you want another look.

[webdevtodayjason]

Both problems are fixed in 7aaf2d43f. You were right that the policy was dead code on the real worker path.

What changed: HERMES_KANBAN_SESSION is now decided before the ambient is_cli/is_gateway/is_ask inference and before the cron marker in all three gates (_run_approval_gate, check_all_command_guards, check_execute_code_guard), with both arms returning so ambient state can never reroute a worker. _default_spawn scrubs the seven inherited context markers (HERMES_INTERACTIVE, HERMES_GATEWAY_SESSION, HERMES_EXEC_ASK, HERMES_CRON_SESSION, HERMES_SESSION_PLATFORM, HERMES_SESSION_KEY, HERMES_YOLO_MODE) alongside the existing HERMES_TUI pop, and worker startup no longer self-marks HERMES_INTERACTIVE when the kanban marker is set, so a stdin=DEVNULL child stops advertising a prompt surface. Worker-local --yolo stays an explicit operator opt-in per the existing test; INHERITED parent yolo is neutralized by the spawn scrub, which lines up with #63183.

Testing: new tests/tools/test_kanban_worker_real_path.py (14 tests) builds the worker env with the real _default_spawn and drives the real gate entry points instead of deleting markers and calling guards directly. On the previous code 12 of 14 fail, for exactly your P1 and P2; all 14 pass with the fix. A human-surface spy asserts a worker is never routed to a prompt, smart approval, or a pending queue. Existing suites pass unchanged: test_kanban_approval_mode 17/17, test_execute_code_approval_cluster 24/24, test_approval 303/303. Full targeted sweep: 1498 passed, 0 failed. Three pre-existing test bugs surfaced during that sweep (verified pre-existing at the prior HEAD in a clean worktree) are fixed in the same commit and called out in the message.

[egilewski]

not enough evidence

I couldn't establish a coherent review tree for this security-boundary change. The deterministic replay of 7aaf2d43f3ea2f97fc1934c82d1f98197be7cfb5 onto current main (9b1028f2974f7b456285b23b28eac5336f71e13c) failed with patch_replay_conflict; the prepared checkout therefore remains the fallback PR head rather than a resolved current-main replay. The submitted branch's stale/conflicted state is informational and was not treated as a standalone blocker; the blocker is that neither the merge nor deterministic replay produced a coherent current-main review tree on which to validate the approval-policy behavior and search for residual bypasses. Please resolve or refresh the branch against current main so that security review can proceed.

Review setup: I reviewed a run-owned local rebase or patch replay against current GitHub main because the submitted branch is stale or conflicted; this does not mean the submitted branch itself merges cleanly.

Signed: GPT-5.6-sol-xhigh in Codex

[webdevtodayjason]

Rebased onto current main (fea838c). The single conflict was comment-only in hermes_cli/config.py — main's new timeout comment block landed in the same spot as this PR's kanban_mode block; kept both, and the approvals dict itself auto-merged. Approval test suites pass post-rebase (360 tests across test_kanban_approval_mode.py, test_approval.py, test_execute_code_approval_cluster.py). Branch is now mergeable against main — security review can re-run.

[egilewski]

looks mergeable

The PR closes the verified kanban-worker approval-context gap on the real spawn path. It gives workers an explicit unattended marker, removes inherited interactive, gateway, cron, session-cache, and yolo markers, prevents quiet worker startup from reasserting interactivity, and gives the terminal, plugin-escalation, and execute_code gates kanban-specific deny-by-default handling with an explicit profile-scoped approve override. A current-main snapshot reproduced inherited yolo/gateway state and auto-approval of a dangerous local command; the same real-spawn probe against the reviewed head scrubbed that state, denied the dangerous command, denied execute_code and plugin escalation, allowed a safe command, and honored explicit kanban approve mode. No source-backed residual bypass was found in the reviewed execution paths.

Security evidence:

• trust boundary: Untrusted kanban task content is executed by a detached child with stdin=DEVNULL. The dispatcher process and its ambient environment are a separate trust context: gateway routing, cron identity, prior session approvals, interactive state, and process yolo state must not be inherited as evidence that a human can approve the worker's actions. The profile's config.yaml is the operator-controlled policy source; HERMES_KANBAN_SESSION is the dispatcher-controlled child identity.
• source/sink/invariant: Sources are task-directed terminal commands, plugin pre-tool-call approval requests, and execute_code bodies. Sinks are local shell execution, plugin-sensitive operations, and arbitrary local Python/subprocess execution. The invariant is that a real kanban child with kanban_mode=deny may run commands that pass the existing dangerous-command and Tirith validators, but dangerous or warned commands, plugin escalations, and execute_code must not reach their sinks; inherited gateway/cron/session/yolo/interactive markers must neither bypass nor wedge that decision. kanban_mode=approve is the explicit operator override, while hardline and user deny rules retain their existing precedence.
• current-main reproduction: On bound current main 3a2b332, a focused probe invoked the real hermes_cli.kanban_db._default_spawn with Popen intercepted and a parent carrying gateway, cron, session, interactive, exec-ask, and yolo markers. The captured child environment had no HERMES_KANBAN_SESSION and retained HERMES_YOLO_MODE and HERMES_GATEWAY_SESSION; importing the current-main approval gate in that child environment made check_all_command_guards approve rm -rf /tmp/probe-target.
• PR-head or patch-replay validation: The checkout was exactly reviewed head df8cc2df51bdf8c7e2aa5502583204b71d3da377 and required no rebase or patch replay. Repeating the same intercepted real-spawn probe showed HERMES_KANBAN_SESSION=1 and absence of all reviewed ambient approval markers. With kanban_mode forced to deny, check_all_command_guards denied rm -rf /tmp/probe-target with the kanban policy message, check_execute_code_guard denied arbitrary Python, and request_tool_approval denied a plugin escalation. No prompt, gateway, or network approval surface was needed.
• positive/negative cases: Negative cases validated on the reviewed head were a dangerous rm command, execute_code containing a subprocess-capable import, and a plugin escalation for an SSH-sensitive write; all were denied under kanban_mode=deny. The positive safe-command case echo safe was allowed after a deterministic Tirith-allow result, and the explicit kanban_mode=approve case allowed the otherwise dangerous rm command. Test-source inspection also confirmed coverage for preserving non-approval parent environment while removing the security-sensitive ambient markers.
• residual bypass search: I traced all four changed approval entry points, their ordering against hardline rules, sudo-stdin protection, user deny rules, global yolo/mode-off behavior, permanent approvals, cron policy, interactive/gateway inference, and container isolation. I searched the repository for HERMES_KANBAN_SESSION, HERMES_CRON_SESSION, check_all_command_guards, check_dangerous_command, request_tool_approval, check_execute_code_guard, and all _default_spawn callers. The worker has one production spawn implementation, and the explicit kanban branch precedes ambient context inference and cron handling in each relevant gate. No additional kanban execution sink or inherited approval marker used by these gates was found.
• reviewer validation: The reviewed checkout matched df8cc2df51bdf8c7e2aa5502583204b71d3da377, and git diff --check, both focused reproduction/correction probes, and compileall for every changed production file plus the two principal kanban approval test files all passed; focused pytest could not run because the leased environment had no pytest module.

Uncertainty: CodeRabbit completed and raised two minor suggestions, but source adjudication found no production bypass: one is redundant on the real spawn path and one concerns only test-state isolation. The focused probe intercepted Popen and validated the exact constructed child environment and real gate functions; it did not launch a model-backed worker subprocess end to end.

Signed: GPT-5.6-sol-xhigh in Codex

[egilewski]

looks mergeable

The PR closes the verified kanban-worker approval-context gap on the real spawn path. It gives workers an explicit unattended marker, removes inherited interactive, gateway, cron, session-cache, exec-ask, and yolo markers, prevents quiet worker startup from reasserting interactivity, and gives the terminal, plugin-escalation, and execute_code gates kanban-specific deny-by-default handling with an explicit profile-scoped approve override. A current-main reproduction again showed inherited worker context approving a dangerous local command; the reviewed replay scrubbed that state, denied dangerous commands, execute_code, and plugin escalation without touching a human-approval surface, allowed a safe command, and retained the intended explicit approve mode. No source-backed residual bypass was found in the reviewed execution paths.

Security evidence:

• trust boundary: detached kanban workers have no human on stdin; dispatcher approval markers are not consent.
• source/sink/invariant: dangerous shell, sensitive plugin actions, and arbitrary execute_code must be denied by default in a kanban worker, while safe commands and explicit operator approval remain available.
• current-main reproduction: inherited worker context approved a dangerous command.
• PR-head or patch-replay validation: the real spawn path set the kanban identity, scrubbed inherited approval markers, and all four approval entry points enforced the policy.
• positive/negative cases: focused worker-path, kanban-mode, and execute_code approval tests passed.
• residual bypass search: no additional in-scope bypass was found.
• reviewer validation: the current-head replay, focused tests, compile checks, and source-backed probes support the clean verdict.

Not checked:

• Full test suite
• CodeRabbit review

Signed: GPT-5.6-sol-xhigh in Codex

[egilewski]

suggesting changes

The PR closes the direct kanban-worker auto-approval gap, but the new raw HERMES_KANBAN_SESSION checks do not honor the existing non_dispatcher_owned_context used by in-process cron jobs. A kanban worker can therefore run a cron job under kanban_mode: approve even when cron_mode: deny, allowing dangerous terminal commands and execute_code.

• [P2] Kanban approval mode overrides cron denial for in-process cron jobs (tools/approval.py:4152)
  run_job() sets the cron session context and enters non_dispatcher_owned_context() before invoking the cron agent, but the new approval branches in tools/approval.py gate solely on the raw HERMES_KANBAN_SESSION marker. A cron job launched from a kanban worker therefore still uses the kanban path in check_dangerous_command, check_all_command_guards, check_execute_code_guard, and request_tool_approval. With approvals.kanban_mode: approve and approvals.cron_mode: deny, dangerous-command, combined terminal, plugin-approval, and execute_code checks can return approved before cron policy is consulted. A prompt-controlled worker can invoke the cron run action to reach a broader trust mode than the cron profile selected.
  Remediation: Resolve kanban identity with the existing dispatcher-ownership ContextVar instead of the raw marker alone. Distinguish the non-dispatcher cron context from delegated children: when run_job binds non_dispatcher_owned_context/cron context, route dangerous-command, Tirith, plugin-approval, and execute_code checks through cron_mode. Continue treating delegated children as unattended kanban lineage rather than scrubbing HERMES_KANBAN_SESSION. Add mixed-mode tests for all entry points with kanban_mode: approve and cron_mode: deny (and the inverse).

Security evidence:

• trust boundary: The dispatcher sets HERMES_KANBAN_SESSION for unattended workers, while in-process cron jobs establish HERMES_CRON_SESSION and a non-dispatcher ownership context; the approval sinks are dangerous-command, combined terminal/Tirith, execute_code, and plugin approval.
• source/sink/invariant: Every unattended execution sink must use the policy for the execution that owns it. The cron scheduler establishes a distinct cron and non-dispatcher context, so a raw kanban marker is not sufficient to identify the dispatcher worker.
• current-main reproduction: Current main has cron-only policy branches and no HERMES_KANBAN_SESSION approval branch; the mixed-mode path is introduced by this PR, and the mixed-context case returns approved with kanban_mode: approve plus cron_mode: deny.
• PR-head or patch-replay validation: The PR-head source and direct mixed-context validation reproduce the issue.
• positive/negative cases: Positive and negative checks show that safe echo remains allowed under kanban deny, configured kanban approval allows the dangerous path, kanban deny blocks dangerous commands, plugin approval, and execute_code, while the missing negative case is a cron context nested in a kanban worker with kanban approval and cron denial.
• residual bypass search: The approval entry points and worker context setup were inspected; the new kanban branches use the raw marker and do not consult dispatcher ownership or the non-dispatcher cron context.
• reviewer validation: Source inspection and focused approval, worker, cron-isolation, and execute_code checks passed and cover the listed paths.

Not checked:

• Ruff validation

Signed: GPT-5.6-luna-max in Codex