fix: use description as pattern_key to prevent approval collisions
pattern_key was derived by splitting the regex on \b and taking [1], so patterns starting with the same word (e.g. find -exec rm and find -delete) produced the same key "find". Approving one silently approved the other. Using the unique description string as the key eliminates all collisions.
This commit is contained in:
parent
08081e5969
commit
4a93cfd889
2 changed files with 28 additions and 1 deletions
|
|
@ -342,6 +342,33 @@ class TestFindExecFullPathRm:
|
||||||
assert key is None
|
assert key is None
|
||||||
|
|
||||||
|
|
||||||
|
class TestPatternKeyUniqueness:
|
||||||
|
"""Bug: pattern_key is derived by splitting on \\b and taking [1], so
|
||||||
|
patterns starting with the same word (e.g. find -exec rm and find -delete)
|
||||||
|
produce the same key. Approving one silently approves the other."""
|
||||||
|
|
||||||
|
def test_find_exec_rm_and_find_delete_have_different_keys(self):
|
||||||
|
_, key_exec, _ = detect_dangerous_command("find . -exec rm {} \\;")
|
||||||
|
_, key_delete, _ = detect_dangerous_command("find . -name '*.tmp' -delete")
|
||||||
|
assert key_exec != key_delete, (
|
||||||
|
f"find -exec rm and find -delete share key {key_exec!r} — "
|
||||||
|
"approving one silently approves the other"
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_approving_find_exec_does_not_approve_find_delete(self):
|
||||||
|
"""Session approval for find -exec rm must not carry over to find -delete."""
|
||||||
|
_, key_exec, _ = detect_dangerous_command("find . -exec rm {} \\;")
|
||||||
|
_, key_delete, _ = detect_dangerous_command("find . -name '*.tmp' -delete")
|
||||||
|
session = "test_find_collision"
|
||||||
|
clear_session(session)
|
||||||
|
approve_session(session, key_exec)
|
||||||
|
assert is_approved(session, key_exec) is True
|
||||||
|
assert is_approved(session, key_delete) is False, (
|
||||||
|
"approving find -exec rm should not auto-approve find -delete"
|
||||||
|
)
|
||||||
|
clear_session(session)
|
||||||
|
|
||||||
|
|
||||||
class TestViewFullCommand:
|
class TestViewFullCommand:
|
||||||
"""Tests for the 'view full command' option in prompt_dangerous_approval."""
|
"""Tests for the 'view full command' option in prompt_dangerous_approval."""
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -63,7 +63,7 @@ def detect_dangerous_command(command: str) -> tuple:
|
||||||
command_lower = command.lower()
|
command_lower = command.lower()
|
||||||
for pattern, description in DANGEROUS_PATTERNS:
|
for pattern, description in DANGEROUS_PATTERNS:
|
||||||
if re.search(pattern, command_lower, re.IGNORECASE | re.DOTALL):
|
if re.search(pattern, command_lower, re.IGNORECASE | re.DOTALL):
|
||||||
pattern_key = pattern.split(r'\b')[1] if r'\b' in pattern else pattern[:20]
|
pattern_key = description
|
||||||
return (True, pattern_key, description)
|
return (True, pattern_key, description)
|
||||||
return (False, None, None)
|
return (False, None, None)
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue