From b04c6e95f60e0ab9bf926bbf566faa9ec523a43b Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 4 Jun 2026 05:23:53 -0700 Subject: [PATCH] fix(approval): catch perl/ruby -i as a separate flag token The salvaged pattern matched -i only inside the first flag token, so `perl -p -i -e '...' config.yaml` (the -i split out after -p) slipped through. Widen to match a -...i flag token anywhere in the args; still no false positive on `perl -e` code eval or config reads. Adds tests for the separate-token, backup-suffix, and read-safe forms. --- tests/tools/test_approval.py | 24 ++++++++++++++++++++++++ tools/approval.py | 7 ++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index 221fd7f68..dc9eace27 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -449,6 +449,30 @@ class TestHermesConfigWriteProtection: ) assert dangerous is True + def test_perl_in_place_separate_flag_token(self): + # The -i flag does not have to be the first token. `perl -p -i -e` + # splits the in-place flag out as its own token after -p; the pattern + # must catch it the same as `perl -i -pe`. + dangerous, key, desc = detect_dangerous_command( + "perl -p -i -e 's/approvals.mode: on/approvals.mode: off/' ~/.hermes/config.yaml" + ) + assert dangerous is True + + def test_perl_in_place_backup_suffix(self): + # `perl -i.bak` keeps a backup but still mutates the file in place. + dangerous, key, desc = detect_dangerous_command( + "perl -i.bak -pe 's/x/y/' ~/.hermes/config.yaml" + ) + assert dangerous is True + + def test_perl_eval_no_inplace_safe(self): + # `perl -e` with no -i flag is code evaluation, not file mutation — + # the perl/ruby -i pattern must not fire on it. + dangerous, key, desc = detect_dangerous_command( + "perl -wne 'print' ~/.hermes/config.yaml" + ) + assert dangerous is False + def test_read_is_safe(self): # Reading config is not a write — must not trip. dangerous, key, desc = detect_dangerous_command("cat ~/.hermes/config.yaml") diff --git a/tools/approval.py b/tools/approval.py index 766d4f31b..8e52f292f 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -446,7 +446,12 @@ DANGEROUS_PATTERNS = [ # perl -i and ruby -i perform the same in-place mutation as sed -i but are # not caught by the -e/-c script-execution pattern above (which targets code # evaluation, not file mutation). Pairs the sed -i coverage from #14639. - (rf'\b(?:perl|ruby)\s+-[^\s]*i.*(?:{_HERMES_CONFIG_PATH}|{_HERMES_ENV_PATH})', "in-place edit of Hermes config/env (perl/ruby)"), + # The -i flag can appear as its own token after other flags + # (`perl -p -i -e ... config.yaml`), combined (`perl -pi -e`), or with a + # backup suffix (`perl -i.bak`). Match any flag token containing `i` + # anywhere in the args, not just the first token — `perl -e '...'` (code + # eval, no -i) does not trip because it has no `-...i` flag token. + (rf'\b(?:perl|ruby)\b.*(?:^|\s)-[^\s]*i\b.*(?:{_HERMES_CONFIG_PATH}|{_HERMES_ENV_PATH})', "in-place edit of Hermes config/env (perl/ruby)"), # Script execution via heredoc — bypasses the -e/-c flag patterns above. # `python3 << 'EOF'` feeds arbitrary code via stdin without -c/-e flags. (r'\b(python[23]?|perl|ruby|node)\s+<<', "script execution via heredoc"),