fix(approval): guard literal current-PID self-kill

[dorukardahan]

Summary

Guard literal kill <current Hermes PID> commands in the approval layer so an agent cannot silently terminate its own runtime.

The guard parses real command-position kill invocations and every PID operand instead of relying on one regex layout. It covers:

• plain and multi-PID commands
• numeric/named signals through compact, -s, -n, and --signal forms
• --, executable paths, grouped commands, and attached redirections
• nested recognized wrappers and environment assignments
• leading-zero and + PID spellings accepted by kill(1)

Signal 0, signal-list/table modes, and help/version modes remain approval-free because they do not send a terminating signal.

Root cause

Existing approval rules covered name-based Hermes termination (pkill hermes) and command-substitution forms (kill $(pgrep ...)), but static regex cannot know the current runtime PID. A dynamic parser is needed to distinguish executable kill syntax from quoted prose while handling real argv layouts.

Feedback and hardening addressed

• Removed the unrelated short 429 cooldown from this PR. Credential-wide 429 exhaustion keeps the existing behavior unless the provider supplies an explicit reset signal.
• Signal-zero probes include spaced, compact, equals, -n, leading-zero, and +0 forms.
• Attached listing forms such as -lTERM, -L9, and --list=TERM are recognized as non-sending modes.
• Leading-zero and + PID spellings normalize without integer conversion.
• All PID operands are inspected within the existing command parser bound.
• Recognized wrappers/assignments no longer stop after 12 prefixes; wrapper -- forms and supported execution options (command -p, exec, setsid, time, and env) are handled.
• Bundled short options retain their argument ownership (env -vu, exec -ca), and Bash time remains distinct from external GNU time.
• Bash builtin and external kill grammars stay distinct: unsupported GNU long options are not false positives, while trailing options cannot hide an already-sent builtin TERM.
• GNU env -S / --split-string remains under the existing execution-bearing-option approval policy instead of partially reimplementing its argv grammar.
• The self-PID comparison runs only for the local backend, where the approval worker and command share a proven PID namespace.
• The parser bound is enforced per executable shell segment, so appending ; cannot hide an uninspected suffix.
• Quoted inert text such as echo "kill <PID>" is not treated as execution.

Scope / risk

• The dynamic guard only matches the current Hermes process PID in a real kill command.
• Unrelated PIDs and non-sending kill modes are not newly blocked.
• Remote and container backends do not compare their PIDs with the local approval worker.
• Parser work remains bounded by the existing command-size limits.
• No credential cooldown behavior remains in the PR diff.

Verification

Current candidate: dbb3d17bfb59d5bb2a14f091eed335e9b03d3731

• scripts/run_tests.sh -j 3 tests/tools/test_approval.py tests/tools/test_command_guards.py tests/tools/test_execution_flag_detection.py -q → 611 passed
  • test_approval.py: 419 passed
  • test_execution_flag_detection.py: 159 passed
  • test_command_guards.py: 33 passed
• python -m pytest tests/tools/test_denial_circuit_breaker.py tests/tools/test_execute_code_approval_cluster.py -q → 37 passed
• python -m ruff check tools/approval.py tests/tools/test_approval.py
• python -m py_compile tools/approval.py tests/tools/test_approval.py
• git diff --check
• clean merge-tree against current main

[teknium1]

Thanks for covering a real self-termination gap: current tools/approval.py:639-648 covers name- and pgrep-based forms but not a literal current PID.

Problems

• agent/credential_pool.py:114 changes the fallback for every 429 lacking reset_at to 60–90 seconds. Current agent/agent_runtime_helpers.py:833-846 treats usage_limit_reached as a credential-exhaustion path, while agent/credential_pool.py:251-257 deliberately uses the one-hour fallback when no reset is available. This would re-probe quota-exhausted credentials without a provider signal that the limit is transient.
• The added kill option branch also matches kill -0 <own-pid>; signal 0 is a liveness check, not self-termination.

Suggested changes

• Preserve the credential-wide fallback unless a verified provider-specific transient signal justifies a shorter cooldown; retain the targeted model-scoped behavior introduced by 473d0dfe.
• Re-scope the PID guard to terminating signals and add a kill -0 non-match test.

Automated hermes-sweeper review.

[dorukardahan]

Follow-up hardening in ec269f5: replaced the single-layout self-PID regex with quote-aware command-position kill argv parsing. This closes -n, --, and multi-PID bypasses, accepts compact/equals signal-zero probes, and avoids quoted-text false positives. Verification: 464 combined approval/credential/auth tests pass.

[dorukardahan]

Rebased-by-merge onto current main in 2c4f816 (no force-push). The conflict was limited to tools/approval.py; current-main cleanup handling and the new self-PID parser were both preserved. Post-merge: full approval suite 341 passed; focused parser + credential/auth suites 168 passed; PR is mergeable.

[dorukardahan]

Refreshed this PR onto current main and narrowed it to the self-PID approval guard only; the unrelated 429 cooldown change is no longer present.

Hardening added during review:

• real kill(1) PID/signal/list/help spellings, including leading-zero/+ PIDs and signal-zero variants
• all PID operands within the existing parser bound
• nested wrappers/assignments, wrapper --, and supported command/exec/setsid/time/env execution options
• quoted inert text remains unflagged

Validation on exact head ec5c0fffa74f4e5f9cdf2405270b3fd4700f1a8f:

• focused approval/security suites: 603 passed, 0 failed
• Ruff, py_compile, and git diff --check: clean
• clean merge-tree against current main
• independent review's final sudo option-arity finding is covered by six new regressions on this head

[dorukardahan]

Follow-up on the final independent review:

• fixed outer-wrapper provenance for bare time; env / command / exec / sudo / nohup / setsid now classify its child as external GNU Time behavior
• retained Bash-keyword behavior for genuinely bare/nested time and builtin time
• added positive and negative regressions for both paths

Validation on exact head 85614cc54bcefffa316ec4ee41ee8e151a11997f:

• focused approval/security suites: 611 passed, 0 failed
• Ruff, py_compile, and git diff --check: clean

[dorukardahan]

CI follow-up on exact head dbb3d17bfb59d5bb2a14f091eed335e9b03d3731:

The failed slice used existing one-argument detector mocks. Local execution now preserves the established detect_dangerous_command(command) call contract; only nonlocal PID namespaces pass the explicit opt-out.

• previously failing CI files: 37 passed, 0 failed
• focused approval/security suites: 611 passed, 0 failed
• Ruff, py_compile, and git diff --check: clean