fix(approval): tighten 'delete in root path' regex to system-root targets only

[briandevans]

Summary

The delete in root path rule in DANGEROUS_PATTERNS (tools/approval.py:215) matched ANY rm with an absolute path because the regex required only a single / after the optional flags. Tighten it so it fires on real system-root targets only — plain /, root globs, and the system top-level dirs already enumerated by HARDLINE_PATTERNS.

The bug

(r'\brm\s+(-[^\s]*\s+)*/', "delete in root path"),

The / after the flag group matches the leading / of any absolute path, regardless of where in the filesystem the target lives. Every rm /home/<user>/..., rm /tmp/..., rm /mnt/c/... is flagged as delete in root path and forced through an approval prompt — including routine cleanup of the agent's own working files. From the reporter's run:

rm -f /home/scott/hermes-home/fawn_lily_temp.html
↳ Reason: delete in root path

That's deleting one file in the user's home directory; nothing about it is near the system root.

The fix

Constrain what comes after the / to the genuinely-dangerous shapes:

(r'\brm\s+(-[^\s]*\s+)*/(\s|\*|$|\.\*|(bin|boot|dev|etc|lib|lib64|opt|proc|root|run|sbin|srv|sys|usr|var)(/|\s|$))', "delete in root path"),

System top-level dirs match the set already enumerated by the HARDLINE_PATTERNS rules at lines 144-148, so the two layers stay aligned. Recursive rm of any deeper path (including /home/...) is still caught by the separate -r/--recursive rules at lines 216-217 — coverage is unchanged for actually-recursive deletes.

Command	Old rule	New rule	Source of approval (if any)
rm -rf /	matches	matches	hardline + this rule
rm -rf /*	matches	matches	hardline + this rule
rm -rf /.*	matches	matches	this rule
rm -rf /etc/passwd	matches	matches	this rule
rm -rf /var/log	matches	matches	this rule
rm /usr/bin/something	matches	matches	this rule
rm /home/user/foo	matches (false positive)	does not match	none — non-recursive single-file rm
rm -f /home/user/foo	matches (false positive)	does not match	none — non-recursive single-file rm
rm /tmp/x	matches (false positive)	does not match	none — non-recursive single-file rm
rm -rf /mnt/c/Users/user/foo	matches (false positive)	does not match	recursive-delete rule still flags
rm -rf /home/user/.cache	matches (false positive)	does not match	recursive-delete rule still flags

Test plan

• Focused: tests/tools/test_approval.py — 143 passed (11 new in TestRmRootPathRegexPrecision)
• Adjacent: tests/tools/test_force_dangerous_override.py, test_approval_plugin_hooks.py, test_approval_heartbeat.py, test_cron_approval_mode.py — 37 passed
• Regression guard: each false-positive case in the table fails the old regex (old=True) and passes the new one (new=False); each true-positive case matches both. Verified end-to-end with re.compile(...).search(cmd).
• Confirmed rm -rf /home/user/.cache stays dangerous via the separate recursive delete rule (not via this rule), so coverage of recursive deletes is unchanged.

Pre-existing baseline failures in tests/tools/test_mcp_tool.py, test_mcp_oauth.py, test_mcp_reconnect_signal.py, test_tts_kittentts.py are orthogonal — those modules are untouched.

Related

• Fixes Over-broad DANGEROUS_PATTERNS regex flags every absolute path as 'delete in root path' #18083
• Aligns the system-dir enumeration in DANGEROUS_PATTERNS with the existing HARDLINE_PATTERNS set at tools/approval.py:144-148
• Independent from fix(approval): catch reverse-shell-via-flag and two-stage download-execute #17962 (different rules; reverse-shell-via-flag and two-stage download-execute)

[Copilot]

Pull request overview

This PR narrows the dangerous-command detection rule for “delete in root path” so it no longer flags every rm against an absolute path, focusing instead on truly system-root targets and known top-level system directories.

Changes:

• Tightened the DANGEROUS_PATTERNS regex for rm ... /... to only match /, root globs, /.*/-style root dotglob, and selected system directories.
• Added targeted regression tests ensuring common absolute paths like /home/..., /tmp/..., and /mnt/... no longer trigger the root-path rule while true root/system targets still do.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
tools/approval.py	Refines the rm root-path dangerous pattern to reduce false-positive approvals on routine absolute-path deletes.
tests/tools/test_approval.py	Adds regression coverage for true-positive and false-positive rm cases related to the tightened regex.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[alt-glitch]

Fixes #18083 — tightens regex to only match genuine system-root targets.

[alt-glitch]

Fixes #18083 — tightens regex to only match genuine system-root targets.

[briandevans]

@copilot Both findings addressed in commit 780936044:

Finding 1 (legacy approval key drift, line 214): Real backwards-compat regression — the old regex-derived legacy key rm\s+(-[^\s]*\s+)*/ would no longer alias to the canonical delete in root path after the regex tightening, silently dropping any pre-existing command_allowlist / session approval stored under that key. Added an explicit _HISTORICAL_LEGACY_KEYS table that aliases the prior legacy key to the current canonical description, and a regression test (test_prior_legacy_allowlist_key_still_approves) that loads the old key into _permanent_approved and asserts is_approved returns True against the current canonical key.

Finding 2 (duplicated system-dir list, line 215): The lists are intentionally different rather than drift, so I documented the contract in a code comment instead of unifying them. Specifically:

• HARDLINE recursive-delete list (/home /root /etc /usr /var /bin /sbin /boot /lib) covers recursive destruction of typical user-data and system roots; /home belongs here because rm -rf /home is unambiguous data loss.
• DANGEROUS "delete in root path" list (bin boot dev etc lib lib64 opt proc root run sbin srv sys usr var) covers single-file rm of system specials — rm /dev/sda, rm /proc/sysrq-trigger, rm /sys/... need a prompt. /home would re-introduce the false-positive on rm /home/<user>/file that this PR is fixing.

Unifying would either over-block (rm -rf /opt becomes hardline) or under-block (rm /dev/sda no longer prompts), so the comment makes the divergence explicit for future readers.

[briandevans]

Closing to keep the queue clean — tools/approval.py has been substantially refactored on main since this opened (#26829, sudo askpass/stdin, DELETE DOTALL bypass), so the regex precision concern may already be covered or shaped differently now. Happy to reopen with a rebased take if the 'delete in root path' precision concern from #18083 still reproduces.