- Single source for the approval-derived bound: public human_wait_ceiling()
in tools/approval.py; the gate's lock-timeout helper delegates to it
instead of re-deriving timeout + margin (was duplicated in two modules
and reached for a private _get_approval_timeout).
- Shared _clamped_window_seconds() for the close-time accrual and the
open-window read, so the two clamps are identical by construction.
- Gate __init__ grows session_key kwarg; tests construct via the real
constructor instead of mutating privates post-hoc.
- Gateway test resolves its pending approval via resolve_gateway_approval()
(the production /deny path) instead of hand-rolling queue-entry internals.
- Docstring accuracy: human_wait_seconds monotonicity caveat under cap
eviction; s/pre_tool_block/pre_tool_call/ hook name.
Review-driven follow-up to the #79719 fix:
- Clamp the CLOSE-side accrual too: a wedged window that eventually closed
used to inject its full unclamped overstay into completed_seconds,
retroactively extending a running batch's deadline by hours. Both clamps
now share one ceiling helper (_human_wait_ceiling = approvals.timeout +
HUMAN_WAIT_MARGIN_S), and the gate's lock-timeout uses the same margin
constant so the bounds cannot drift apart.
- Evict idle sessions until the table is under the cap (was: at most one
per insert, so churn could outgrow _HUMAN_WAIT_MAX_SESSIONS). Entries
with an open window are still never evicted.
- Log (debug) instead of silently swallowing a failed session-key snapshot
in the gate constructor.
Tests: close-side clamp regression + table-cap assertion added; suite at
17 passed.
A tool wedged inside _ConcurrentToolAuthorizationGate hung the whole turn
forever (#79719): excluded_seconds() measured residency in gate.run() —
arbitrary code — so an open window grew 1:1 with wall clock and the batch
deadline's remaining was constant (remaining = deadline - window_started;
now cancels out). A hanging pre_tool_call plugin or an approval round-trip
to a dead client defeated the deadline entirely. The serialization lock was
also an unbounded acquire, so every other worker needing authorization
parked behind the wedged holder forever.
Fix, in two halves:
- tools/approval.py grows per-session human-wait accounting
(human_wait_window / human_wait_seconds). The two places that are
verifiably blocked on a HUMAN — the CLI approval prompt and the gateway
approval poll loop — mark their own windows. Both are intrinsically
bounded by approvals.timeout; the open-window read is additionally
clamped to that timeout plus a margin as belt-and-braces.
- _ConcurrentToolAuthorizationGate keeps only serialization, with a bounded
acquire (approvals.timeout + 60s; on expiry the prompt runs unserialized —
the same degradation the start-order gate accepted in #79705).
excluded_seconds() becomes a baseline-delta read of the session's
human-wait total.
A wedged plugin now contributes nothing to the exclusion, so the batch
times out at the normal deadline with correctly labeled results, while a
genuine approval wait — which can legitimately exceed any fixed bound —
still extends the deadline in full. E2E (real AIAgent, worktree imports):
wedged-plugin batch on main never ends (>30s observed, 3s deadline); with
the fix it ends at 3.0s. A 4s simulated approval over a 2s deadline
completes without a timeout label.
Closes#79719
A raw newline in the _CMDPOS start-position class made ANY multi-line
quoted argument look like a command boundary, so hermes send message
bodies, multi-line git commit -m messages, and heredoc text that merely
mentioned dangerous command names tripped the unconditional hardline
blocklist and could not run at all.
Mask newlines inside single/double quotes (detection-only, mirroring the
quote tracking in _iter_shell_command_starts) before building detection
variants. Real threats keep blocking: unquoted newlines stay command
separators, command substitutions inside quotes still anchor, and
_mark_command_starts still re-inserts newlines at genuine quote-aware
command starts. Masking runs on the RAW command before normalization,
which strips escapes and would otherwise corrupt quote state.
Regression tests cover both directions: multi-line quoted data passes
(hermes send, git commit -m, heredocs); bare/chained/substituted
shutdown-class and rm-floor commands still block.
When an approval prompt expired without a response, every CLI-side path
collapsed the timeout into the same 'deny' choice as an explicit user
refusal, so the agent was told the user denied the action when the user
simply never answered. The gateway wait already distinguished the two
('timed out without user response... Silence is not consent.'); this
brings the CLI/TUI/ACP surfaces to parity.
- prompt_dangerous_approval(): input()-path expiry now returns a distinct
'timeout' choice (still fail-closed).
- cli.py _approval_callback + hermes_cli/callbacks.py approval_callback:
deadline expiry returns 'timeout' instead of 'deny'.
- check_all_command_guards / _run_approval_gate CLI tails: 'timeout' maps
to outcome='timeout' with a 'timed out without user response... Silence
is not consent.' BLOCKED message (matching the gateway wording);
explicit deny keeps outcome='denied' and gains user_consent=False for
shape parity.
- computer_use: 'timeout' verdict threads through the CLI adapter and
yields a 'prompt timed out — the user did not respond' error instead of
'denied by user'.
- ACP permissions bridge: FutureTimeout returns 'timeout' (other failures
still 'deny'); elicitation maps 'timeout' to 'cancel' like the gateway's
unresolved outcome; codex wire mapping documents deny/timeout→decline.
- write_approval already treats unknown choices as 'stage, not drop', so
a timeout now stages the memory write instead of silently refusing it.
Every timeout path remains fail-closed — the action never runs; only the
classification reported to the agent changes.
Follow-up to the recovery-recipe commit on this branch, per review:
instead of only TELLING the model to re-author the payload via
write_file (2 turns), materialize the blocked command to
~/.hermes/cache/blocked-scripts/blocked-*.sh and point the recovery
at it directly: 'saved to <path> - review it, then run
terminal(command="bash <path>")' (1 turn).
Safety posture is unchanged or better:
- Nothing is executed here; the file is only written.
- The bash <path> follow-up goes through the normal execution
pipeline, including the referenced-script content guard, which
inspects script files named in commands - the payload is MORE
visible to policy than it was inline.
- Genuine hardline blocks (destructive ops) never save anything
(test-asserted).
- Save failures fall back to the previous manual write_file recipe.
- 7-day opportunistic cleanup of saved payloads.
Two block classes from production mining (250k-call window) that
models answered with blind rephrase-retries:
1. Parser-limit / malformed-payload hardline blocks (198x): these fire
on oversized inline payloads (heredocs, giant one-liners), not on a
forbidden operation - but the message read like a permanent ban.
The block now appends: 'RECOVERY: ... write the script to a file
with write_file, then run bash /path/script.sh - do not retry
inline.' Genuine hardline blocks (destructive filesystem
operations) are unchanged.
2. Backgrounding-wrapper blocks (200x): the guidance now spells out
the exact corrected call shape - 're-send WITHOUT the wrapper as
terminal(command="<cmd>", background=true,
notify_on_complete=true)' - instead of describing the feature
abstractly.
Replace the process-global HERMES_CRON_SESSION env var with a per-session
ContextVar so a cron tick in the gateway process cannot leak into unrelated
live gateway/API/TUI turns. The cron scheduler now sets the ContextVar
inside the job's try/finally scope and resets it on cleanup. Gateway, API
server, ACP adapter, and TUI gateway all pass cron_session='' to explicitly
mark their sessions as non-cron, masking any stale process env.
Co-authored-by: hinablue <hinablue@gmail.com>
Closes#37968
Review follow-up on the #76194 salvage: the readonly swap makes this
function leak the live config-cache 'approvals' sub-dict to callers.
All current callers are read-only (audited); the docstring now carries
the do-not-mutate contract for future ones.
The terminal-command guard path loaded config 2-3x per invocation via
load_config(), which pays a defensive deepcopy of the entire config on
every call (~356us of the ~376us warm-cache cost measured on a real
config.yaml). All six swapped call sites were audited read-only — every
caller takes scalar reads or iterates the returned structures; none
mutate (the save path at save_permanent_allowlist keeps load_config) —
so they now use load_config_readonly(), the API built for exactly this
(precedent: #74211, #74322; the one unsafe-site lesson from #56085's
salvage is covered by the mutation audit and a cache-integrity test).
Measured (real config.yaml, warm cache): load_config 376.0us ->
load_config_readonly 19.9us (18.9x); full guard pass
check_all_command_guards('ls -la','local') 930.7us -> 241.8us (3.85x).
Tests: new test_approval_config_readonly.py drives the real functions
against a temp HERMES_HOME — readonly call counts per function, a
no-deepcopy pin for the full guard pass, and cache-identity/integrity
checks. Existing test mocks retargeted from load_config to
load_config_readonly (same injection intent). Note: 6
test_approval_mode_parity failures are pre-existing ordering flakes —
identical with the change stashed on clean main.
Port from openai/codex#33464: GNU rm permutes options, so
`rm build/ -rf`, `rm build/ -r -f`, and `rm build/ --recursive
--force` are equivalent to the flags-first spellings — but every
existing rm pattern required the flag group BEFORE the path, so these
spellings ran with no approval prompt at all (proven live on main).
The hardline floor was NOT affected: protected paths (/, system dirs,
$HOME) match regardless of flag position because the hardline path
matcher does not require flags. The gap was the approval-prompt layer
for arbitrary paths.
New DANGEROUS_PATTERNS entry with a tempered operand run: cannot cross
command separators (; | & newline), quotes, or a bare -- end-of-options
separator (after --, -rf is a literal filename). Flag token must be
whitespace-anchored so the r inside long options like --registry does
not count.
9 positive + 7 negative shapes in tests; approval cluster (868 tests)
green.
Inspired by Claude Code 2.1.214, which added permission prompts for
container-CLI commands (including the Podman shim) carrying
daemon-redirect flags (--url, --connection, --identity, remote mode)
that previously ran without one.
A daemon redirect makes a local-looking command operate on a different
(often remote) daemon, silently acting on production infrastructure.
Any container-CLI invocation carrying a redirect now requires approval
regardless of subcommand:
- -H/--host and --context global flags (value required, global-flag
position only — bare -h help and run-level -h <hostname> stay allowed)
- context use (persistently switches the default daemon)
- podman --url/--connection/--identity and -r/--remote
- DOCKER_HOST=/DOCKER_CONTEXT=/CONTAINER_HOST=/CONTAINER_CONNECTION=
environment prefixes
Sibling-site widening: the existing container lifecycle rules matched
only the verb directly adjacent to the binary name, so a global flag or
a compose -f file flag slipped past the guard, and the legacy hyphenated
compose binary was never covered. They now tolerate global flags — the
same treatment the 'hermes ... gateway' rule already has — and match the
hyphenated compose binary.
Validation: 33 new tests; 339 pass in test_approval.py + new file;
442 pass across the adjacent guard suites; E2E battery of 12 dangerous
+ 17 safe commands via real imports, hot path ~330us/call.
After N consecutive guardian denials in a session the deny message escalates to a hard-stop instruction. Inspired by ChatGPT Work auto-review circuit breaker.
Adds an allow_session flag to the gateway approval payload so adapters
can render the session tier independently of the permanent tier. Matrix
gains a session reaction (🌀) and a reaction legend; pure-tirith prompts
now offer once/session/deny instead of collapsing to once/deny.
Salvaged from PR #67312, adapted to the allow_permanent semantics that
landed in #68597 (Always offered when any dangerous-pattern warning is
persistable; pure-tirith prompts stay session-max).
Three related messaging-approval fixes:
1. approvals.timeout default 60 -> 300. PR #63501 collapsed the gateway
wait onto the canonical approvals.timeout (previously
gateway_timeout=300), silently shrinking messaging approval windows
to 60s. Push-notification approvals routinely arrive later than a
minute; taps landed after the wait had already failed closed.
2. Stale-tap honesty: adapters resolved the approval AFTER rendering
'<checkmark> Approved by <user>' (Telegram/Discord/Slack), or ignored a zero
resolve count (WhatsApp Cloud/Feishu). A tap on an expired prompt
claimed approval while the command had already been denied. All
button paths now resolve first and render 'Approval expired -
command was not run' when nothing was waiting.
3. Mixed-warning prompts (dangerous pattern + tirith finding) now offer
Always: the persistence layer already permanently allowlists the
pattern key and downgrades the tirith key to session scope, but the
UI hid Always whenever ANY tirith warning was present. Pure-tirith
prompts still withhold Always (content findings are session-max by
design), and Smart-DENY overrides remain once-only.
_run_approval_gate's gateway branch only queued via submit_pending, so
plugin-escalated approvals never sent the interactive embed+buttons on
Discord/Telegram/Slack (#59413) - the user was never notified and the
action stayed silently blocked. Mirror check_dangerous_command's path:
when a session notify callback is registered, run the blocking
_await_gateway_decision round-trip (redacted payload, once/session/
always persistence, deny/timeout produce definitive BLOCKED outcomes);
fall back to submit_pending only when no callback exists.
Fixes#59413.
Adds approvals.deny to config.yaml — a list of fnmatch globs matched
against terminal commands. A match blocks unconditionally, BEFORE the
--yolo / /yolo / approvals.mode=off bypass, making it the user-editable
counterpart to the code-shipped hardline blocklist.
- Checked in both command gates (check_dangerous_command and
check_all_command_guards), after the hardline floor and sudo-stdin
guard, before the yolo bypass and permanent allowlist.
- Matching runs over the same normalized/deobfuscated command variants
as the dangerous-pattern detector, case-insensitive.
- Opt-in: empty/absent list is a no-op; behavior unchanged.
Supersedes the trust-engine approach from #21500 with a minimal
config-native design: the only capability the existing stack lacked
was deny-that-beats-yolo. Allow already exists (command_allowlist),
ask already exists (session approvals).
Follow-up to #56236: the broadened root token /[/.]*\** treats any run of
dots after the root slash as a collapse spelling, so a literal root-level
directory named '...' (rm -rf /...) was unconditionally hardline-blocked
with no approval path. Tighten the token to /(?:(?:\.\.?)?/)*(?:\.\.?)?\**
so each inter-slash segment must be exactly '.' or '..' — all real collapse
spellings (//, /., /./, /.., //*, ///, /../..) stay on the hardline floor
while literal dot-run dirs fall through to the softer DANGEROUS_PATTERNS
rules like every other real path.
* feat(approvals): /deny <reason> relays denial reason to the agent
Port from qwibitai/nanoclaw#2832 (reject with reason).
Gateway /deny now accepts an optional trailing reason (/deny <reason>
or /deny all <reason>). The reason rides on the per-session approval
entry through resolve_gateway_approval -> _await_gateway_decision and is
appended to the BLOCKED tool result the agent receives, so a declined
agent can adapt instead of only hearing 'denied'.
Adapted to hermes-agent's synchronous single-command /deny model: no DB
state, no second-message capture step, no migration. Reason is capped at
280 chars and threaded through both the terminal-command guard and the
execute_code guard. Plain /deny and the approve paths are unchanged.
- tools/approval.py: _ApprovalEntry.reason; resolve_gateway_approval gains
optional reason; _await_gateway_decision returns it; both gateway BLOCKED
messages include it
- gateway/slash_commands.py: parse leading 'all' + trailing reason
- locales/en.yaml: deny.denied_reason_{singular,plural}
- hermes_cli/commands.py: /deny args_hint '[all] [reason]'
- tests: 3 new (with-reason, all+reason, plain-deny regression)
* fix(ci): localize deny-reason keys across all locales + update interrupt-path assertions
CI surfaced two enforced invariants broken by the deny-with-reason change:
- test_i18n catalog-parity requires every locale to carry the same keys as
en.yaml with matching placeholders. Added deny.denied_reason_singular/plural
(with {count}/{reason}) to all 15 non-English locales.
- test_approval_interrupt asserts the exact dict from _await_gateway_decision,
which now carries a 'reason' key (None on the interrupt/timeout paths).
Extend the pre_tool_call plugin hook return contract with a new directive:
{"action": "approve", "message": "why this needs human confirmation"}
Previously a pre_tool_call hook could only veto a tool call (action: block)
or allow it silently. It could not escalate to the existing human-approval
flow. This unlocks user-defined runtime approval rules on ANY tool (HTTP
writes, file writes to sensitive paths, email sends), enforced at runtime —
resolving #51221 as a pure plugin, with no core approval.py rule schema.
Mechanism:
- get_pre_tool_call_directive() returns (action, message) for block|approve;
get_pre_tool_call_block_message() kept as a block-only back-compat shim.
- resolve_pre_tool_block() is the single dispatch-site chokepoint: fetches
the directive and, for approve, invokes the human gate; fail-closed to a
block on denial, timeout, or gate exception. ALL FOUR tool-dispatch sites
now call it: tool_executor (concurrent + sequential), agent_runtime_helpers,
and model_tools.handle_function_call.
- request_tool_approval() escalates via the SAME machinery as Tier-2
dangerous commands: session/permanent allowlist, prompt_dangerous_approval
(CLI) / submit_pending (gateway), [o]nce/[s]ession/[a]lways/[d]eny,
timeout fail-closed, approvals.cron_mode for cron contexts.
Architecture: extracted the shared decision core into _run_approval_gate(),
called by BOTH check_dangerous_command() and request_tool_approval() so the
fail-closed / cron / gateway / yolo / persist policy lives in ONE place and
cannot drift. Fixed a latent divergence — the plugin path now honors --yolo.
Approval grain: [a]lways is keyed on tool_name + a hash of the reason (an
explicit plugin rule_key overrides), so distinct reasons on the same tool
persist independently instead of one 'always' blanketing the whole tool.
Non-interactive: cron honors approvals.cron_mode (parity with commands); any
other non-interactive non-gateway context fails CLOSED for the plugin path
(the command path keeps its historical fail-open default, unchanged).
No new config schema, no new env vars, no new hook events.
Self-review follow-up on the salvaged approval-routing fix.
The initial adaptation re-read os.getenv("HERMES_YOLO_MODE") at session-build
time. That diverges from the repo's security invariant: HERMES_YOLO_MODE is
frozen into tools.approval._YOLO_MODE_FROZEN at import time precisely so a skill
running mid-process cannot set the env var and instantly flip the approval
bypass (a prompt-injection escalation path). A live re-read re-opened that hole
for the codex routing path.
- Add tools.approval.is_approval_bypass_active() — the canonical three-source
bypass check (frozen --yolo/HERMES_YOLO_MODE + session /yolo + approvals.mode
off) in one place. This is the 4th inline copy of that OR-chain (the three
sites in approval.py and tui_gateway/server.py:3121 all use the same idiom);
the helper is the shared chokepoint they can collapse onto.
- codex_runtime.py now calls is_approval_bypass_active() instead of the
hand-rolled mode-or-session check plus a runtime env re-read.
- Update the env-yolo test to patch _YOLO_MODE_FROZEN (the canonical test
pattern, e.g. tests/tools/test_yolo_mode.py) rather than setenv, which is
dead-on-arrival against the frozen constant.
Fail-closed default preserved on every branch; 28 integration + 77 session/yolo
tests pass; E2E confirms the real exec decision flips decline->accept only when
bypass is active.
git's and sudo's option parsers resolve unambiguous long-flag prefixes, so
`git reset --har`, `git branch --delete --force`, and `sudo --stdi`/`--ask`
execute identically to their full-flag forms while evading the exact-string
DANGEROUS_PATTERNS regexes that gate them. Verified live against real git
and sudo binaries. Widen the patterns to accept unambiguous abbreviations,
scoped narrowly enough to avoid colliding with sibling flags (--help,
--soft/--mixed/--merge/--keep, --shell/--set-home).
Rework follow-up on the Windows destructive-shell detection. The PowerShell
pattern required an explicit -Command/-c before the verb, but PowerShell runs
the verb as the DEFAULT POSITIONAL arg — so `powershell Remove-Item -Recurse
-Force C:\x` (no -Command) slipped through, the exact case the PR body claims
to close. Also missing the canonical `ri` alias.
Anchor the verb to the command position (after the shell name + any leading
-Flag switches + optional -Command/-c) so bare invocations are caught while a
benign path arg containing 'del'/'rm' (e.g. -File c:\del-logs\run.ps1) is not.
Add ri to the verb list. Mutation-verified regression tests for the bare
invocation, ri alias, and the benign-path negative.
Wrapping a catastrophic command in a bare subshell or brace group walked
straight past the unconditional hardline floor -- even under --yolo,
/yolo, approvals.mode=off, and cron approve mode. The command-substitution
forms were already caught; the bare paren / brace-group forms were the gap.
Rather than add the paren and brace openers to the flat _CMDPOS pattern
class (which cannot tell a real subshell opener from one sitting inside a
quoted argument, and would false-positive on ordinary prose such as a PR
title that merely mentions the trigger word), teach the existing
QUOTE-AWARE command-start tokenizer (_iter_shell_command_starts) to treat
the paren and brace openers as command starts, then emit a detection
variant that marks each real command start with a newline (already a
_CMDPOS separator). Openers inside quotes never register as starts, so
quoted arguments are left untouched while real subshell/brace bypasses now
anchor. One place covers every _CMDPOS rule (shutdown/reboot/init/
systemctl/telinit and the rm root/home/system floor).
Tests: subshell/brace bypasses added to the hardline-block, root-wipe, and
yolo-bypass sets; a regression set asserts quoted paren/brace prose is NOT
blocked (guards our own gh-pr-create workflow).
rm -rf //, /., /./, /.. and //* all resolve to / in the shell but slipped
past the root-filesystem hardline pattern, whose target group only matched
the literal / and /* tokens. They fell to the softer DANGEROUS_PATTERNS
'delete in root path' rule, which --yolo / approvals.mode=off / cron
approve-mode are designed to bypass — leaving the one unconditional floor
open to a full root wipe under yolo.
Broaden the root token from '/|/\\*|/ \\*' to '/[/.]*\\**' inside
_hardline_rm_path so any root-anchored path whose components collapse back
to / (repeated slashes plus ./.. segments) with an optional trailing glob
is caught. A trailing real segment (/tmp, /home, /.ssh) still fails to
match and stays with the softer rules.
Co-authored-by: kernel-t1 <214165399+kernel-t1@users.noreply.github.com>
kshitij
Jeffrey Quesnelle
Teknium
Alex Fournier
kshitij
spfcraze
Francesco Bonacci
Jeffrey Quesnelle
Joshua
hellno
Jake Long Vu
ansel-f
doncazper
srojk34
dsad
amathxbt