fix(approval): close bare powershell Remove-Item bypass + add ri alias (review)
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.
This commit is contained in:
parent
4b92a8cd31
commit
b4342a83bb
2 changed files with 42 additions and 1 deletions
|
|
@ -124,6 +124,41 @@ class TestWindowsShellDestructiveCommands:
|
|||
assert key is not None
|
||||
assert desc == "PowerShell encoded command execution"
|
||||
|
||||
def test_powershell_bare_remove_item_requires_approval(self):
|
||||
# Regression: PowerShell runs the verb as the default positional arg,
|
||||
# so `powershell Remove-Item ...` with NO explicit -Command must still
|
||||
# be gated (the original pattern required -Command and missed this).
|
||||
dangerous, key, desc = detect_dangerous_command(
|
||||
r"powershell Remove-Item -Recurse -Force C:\tmp\hermes-victim"
|
||||
)
|
||||
assert dangerous is True
|
||||
assert key is not None
|
||||
assert desc == "Windows PowerShell destructive delete"
|
||||
|
||||
def test_pwsh_bare_remove_item_requires_approval(self):
|
||||
dangerous, key, desc = detect_dangerous_command(
|
||||
r"pwsh Remove-Item -Recurse C:\tmp\x"
|
||||
)
|
||||
assert dangerous is True
|
||||
assert "delete" in (desc or "").lower()
|
||||
|
||||
def test_powershell_ri_alias_requires_approval(self):
|
||||
# `ri` is the canonical Remove-Item alias.
|
||||
dangerous, key, desc = detect_dangerous_command(
|
||||
r"powershell ri -Recurse -Force C:\tmp\x"
|
||||
)
|
||||
assert dangerous is True
|
||||
assert desc == "Windows PowerShell destructive delete"
|
||||
|
||||
def test_powershell_benign_path_containing_del_not_flagged(self):
|
||||
# A benign file path that merely contains "del" must NOT trip the guard
|
||||
# (verb-position anchoring prevents matching inside a -File arg).
|
||||
dangerous, key, desc = detect_dangerous_command(
|
||||
r"powershell -File C:\del-logs\run.ps1"
|
||||
)
|
||||
assert dangerous is False
|
||||
assert key is None
|
||||
|
||||
def test_plain_text_does_not_trigger_windows_delete(self):
|
||||
dangerous, key, desc = detect_dangerous_command(
|
||||
"echo remember to del old notes"
|
||||
|
|
|
|||
|
|
@ -503,7 +503,13 @@ DANGEROUS_PATTERNS = [
|
|||
# Unix `rm`. Gate only when they are executed through cmd/powershell so
|
||||
# ordinary prose or filenames containing "del"/"rd" do not trip the guard.
|
||||
(r'\bcmd(?:\.exe)?\s+/(?:c|k)\s+.*\b(?:del|erase|rd|rmdir)\b', "Windows cmd destructive delete"),
|
||||
(r'\b(?:powershell|pwsh)(?:\.exe)?\b.*\s-(?:command|c)\b.*\b(?:remove-item|del|erase|rd|rmdir|rm)\b', "Windows PowerShell destructive delete"),
|
||||
# PowerShell/pwsh: the destructive verb runs as the default positional
|
||||
# argument, so `powershell Remove-Item ...` needs NO explicit -Command.
|
||||
# Anchor the verb to the command position (right after the shell name,
|
||||
# after any leading `-Flag` switches, and optionally after -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.
|
||||
(r'\b(?:powershell|pwsh)(?:\.exe)?\b(?:\s+-\S+)*\s+(?:-(?:command|c)\s+)?["\']?(?:remove-item|rmdir|erase|del|rd|ri|rm)\b', "Windows PowerShell destructive delete"),
|
||||
(r'\b(?:powershell|pwsh)(?:\.exe)?\b.*\s-(?:encodedcommand|enc|e)\b', "PowerShell encoded command execution"),
|
||||
(r'\bchmod\s+(-[^\s]*\s+)*(777|666|o\+[rwx]*w|a\+[rwx]*w)\b', "world/other-writable permissions"),
|
||||
(r'\bchmod\s+--recursive\b.*(777|666|o\+[rwx]*w|a\+[rwx]*w)', "recursive world/other-writable (long flag)"),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue