From 4a93cfd8891c79c07c8ac7b88ae3b1cdd47cea6f Mon Sep 17 00:00:00 2001 From: 0xbyt4 <35742124+0xbyt4@users.noreply.github.com> Date: Thu, 12 Mar 2026 22:39:46 +0300 Subject: [PATCH] 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. --- tests/tools/test_approval.py | 27 +++++++++++++++++++++++++++ tools/approval.py | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index b95e865e5..a36b2e1f8 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -342,6 +342,33 @@ class TestFindExecFullPathRm: 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: """Tests for the 'view full command' option in prompt_dangerous_approval.""" diff --git a/tools/approval.py b/tools/approval.py index 83980893d..21baedbd0 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -63,7 +63,7 @@ def detect_dangerous_command(command: str) -> tuple: command_lower = command.lower() for pattern, description in DANGEROUS_PATTERNS: 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 (False, None, None)