From 4eff83cdeccaca475a128aa030593eb67f97a5c2 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 17 Sep 2026 20:12:02 +0530 Subject: [PATCH] fix(approval): deobfuscate every command word in one detection variant (#113535) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _command_detection_variants yielded one FULL-LENGTH variant per quoted or escaped command word. A heredoc body of quoted lines ("key": "value", …) is hundreds of quoted command words, so both detection passes scanned O(words * len) characters: on a 16 KB / 460-line command the hardline pass alone took 7.8 s and the dangerous pass 15 s even with the launchctl lookahead anchored (33 KB: 36 s hardline), all while holding the GIL on the gateway loop. Build a single variant with every command word deobfuscated instead. The obfuscation catch ($(echo rm), r''m, ${0/x/r}m …) is unchanged — the same deobfuscated words appear, in one string — and the variant count on the issue's input drops from 923 to 4 (36 s -> 0.26 s hardline, verdict unchanged). detect_hardline_command needs no bound and keeps its YOLO semantics: a benign heredoc is neither blocked nor prompted. Root cause identified in #113943 by @Tranquil-Flow (variant explosion as the second compounding factor); its cumulative-work budget is superseded by removing the explosion. Co-authored-by: Tranquil-Flow <66773372+Tranquil-Flow@users.noreply.github.com> --- tests/tools/test_approval.py | 26 +++++++++++++++++++++++++- tools/approval_detection.py | 29 +++++++++++++++++++++++------ 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index 7a0524273d..0aa36032a2 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -10,7 +10,7 @@ from unittest.mock import patch as mock_patch import pytest import tools.approval as approval_module -from tools import approval_context +from tools import approval_context, approval_detection from tools import approval_smart from hermes_constants import get_hermes_home from tools.approval import approve_session, detect_dangerous_command, detect_hardline_command, is_approved, load_permanent, prompt_dangerous_approval @@ -1162,6 +1162,30 @@ class TestLaunchctlGatewayLifecycle: assert "launchd" in desc.lower() +class TestQuotedCommandWordVariants: + """#113535: a heredoc body of quoted lines is hundreds of quoted command words; one full-length + detection variant per word made both detection passes O(words * len) and stalled the gateway.""" + + def test_many_quoted_command_words_stay_bounded_in_both_passes(self): + cmd = "\n".join(f'"key{i}": "line {i} with some text"' for i in range(460)) + start = time.monotonic() + assert detect_hardline_command(cmd) == (False, None) + assert detect_dangerous_command(cmd) == (False, None, None) + elapsed = time.monotonic() - start + assert elapsed < 5.0, f"detection took {elapsed:.2f}s for a {len(cmd)}-char command" + + def test_obfuscated_command_words_still_detected_when_merged_into_one_variant(self): + cmd = 'echo "one"; $(echo rm) -rf ~/.ssh; echo "two"; r\'\'m -rf ~/.gnupg' + dangerous, _, desc = detect_dangerous_command(cmd) + assert dangerous is True + assert "delete" in desc.lower(), desc + # Nested spans (the backtick word and the substitution inside it) overlap, so they cannot share a + # variant; the inner one must land in a second-round variant instead of being dropped. + variants = list(approval_detection._command_detection_variants('echo `$("echo" rm) -rf ~/.ssh`')) + assert any("echo `rm -rf ~/.ssh`" in v for v in variants), variants + assert any("echo `$(echo rm) -rf ~/.ssh`" in v for v in variants), variants + + class TestGitDestructiveOps: """git reset --hard, push --force, clean -f, branch -D can destroy work and rewrite shared history. Not covered by rm/chmod patterns. diff --git a/tools/approval_detection.py b/tools/approval_detection.py index b03cd9ccd3..ea00c83c43 100644 --- a/tools/approval_detection.py +++ b/tools/approval_detection.py @@ -1413,12 +1413,29 @@ def _command_detection_variants(command: str): yield faithful # Quoting/escaping can spell an executable in pieces (r\m, r''m). Keep that deobfuscation scoped # to command words so arguments don't false-positive. - for word_start, word_end, word in _iter_shell_command_word_spans(normalized): - deobfuscated = _deobfuscate_shell_word_for_detection(word) - if deobfuscated and deobfuscated != word: - variant = normalized[:word_start] + deobfuscated + normalized[word_end:] - if fresh(variant): - yield variant + # One variant with EVERY command word deobfuscated, not one full-length variant per word: a heredoc + # body of quoted lines has hundreds of quoted command words, and per-word variants made both + # detection passes O(words * len) — minutes of GIL-held regex on a 15 KB command (#113535). + # Spans arrive out of order (loop/conditional bodies after their keywords) and can nest (a + # backtick word and the command inside it), so apply them sorted; spans overlapping an applied + # one wait for the next round, one combined variant per nesting level. + pending = sorted( + ((word_start, word_end, deobfuscated) for word_start, word_end, word in _iter_shell_command_word_spans(normalized) + if (deobfuscated := _deobfuscate_shell_word_for_detection(word)) and deobfuscated != word), + key=lambda span: span[:2], + ) + while pending: + applied, carry, cursor = [], [], 0 + for span in pending: + if span[0] < cursor: + carry.append(span) + else: + applied.append(span) + cursor = span[1] + variant = _splice(normalized, applied) + if fresh(variant): + yield variant + pending = carry def _is_verification_artifact_cleanup(command: str) -> bool: