From 74e59b8b689b00b07471d608f3865b721575ed4a Mon Sep 17 00:00:00 2001 From: srojk34 <286497132+srojk34@users.noreply.github.com> Date: Wed, 1 Jul 2026 13:20:23 +0300 Subject: [PATCH] fix(security): close abbreviated-flag bypasses in git/sudo approval patterns git's and sudo's option parsers resolve unambiguous long-flag prefixes, so `git reset --har`, `git branch --delete --force`, and `sudo --stdi`/`--ask` execute identically to their full-flag forms while evading the exact-string DANGEROUS_PATTERNS regexes that gate them. Verified live against real git and sudo binaries. Widen the patterns to accept unambiguous abbreviations, scoped narrowly enough to avoid colliding with sibling flags (--help, --soft/--mixed/--merge/--keep, --shell/--set-home). --- tests/tools/test_approval.py | 85 ++++++++++++++++++++++++++++++++++++ tools/approval.py | 27 +++++++++++- 2 files changed, 110 insertions(+), 2 deletions(-) diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index a536464ca7b..ec0406c9eba 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -1479,6 +1479,33 @@ class TestGitDestructiveOps: assert dangerous is True assert "reset" in desc.lower() or "hard" in desc.lower() + def test_git_reset_hard_abbreviated_har_detected(self): + # git's own option parser resolves unambiguous long-flag prefixes, + # so `git reset --har` executes identically to `--hard` (verified + # against a live git binary) — confirmed real bypass of the + # exact-string `--hard` pattern. + cmd = "git reset --har HEAD~3" + dangerous, _, desc = detect_dangerous_command(cmd) + assert dangerous is True + assert "reset" in desc.lower() or "hard" in desc.lower() + + def test_git_reset_hard_abbreviated_single_h_detected(self): + cmd = "git reset --h" + dangerous, _, _ = detect_dangerous_command(cmd) + assert dangerous is True + + def test_git_reset_soft_not_flagged(self): + """--soft doesn't discard uncommitted work; must not be flagged.""" + cmd = "git reset --soft HEAD~1" + dangerous, _, _ = detect_dangerous_command(cmd) + assert dangerous is False + + def test_git_reset_help_not_flagged(self): + """--help must not resolve as an abbreviation of --hard.""" + cmd = "git reset --help" + dangerous, _, _ = detect_dangerous_command(cmd) + assert dangerous is False + def test_git_push_force_detected(self): cmd = "git push --force origin main" dangerous, _, desc = detect_dangerous_command(cmd) @@ -1522,6 +1549,34 @@ class TestGitDestructiveOps: dangerous, _, _ = detect_dangerous_command(cmd) assert dangerous is True + def test_git_branch_long_flag_delete_force_detected(self): + # `--delete --force` performs the exact same unmerged-branch force + # delete as `-D` (verified live), but is a different token + # spelling entirely so the `-D\b` pattern never sees it. + cmd = "git branch --delete --force feature-branch" + dangerous, _, desc = detect_dangerous_command(cmd) + assert dangerous is True + assert "force delete" in desc.lower() + + def test_git_branch_short_delete_long_force_detected(self): + # `-d --force` is git's own documented equivalent of `-D`. + cmd = "git branch -d --force feature-branch" + dangerous, _, _ = detect_dangerous_command(cmd) + assert dangerous is True + + def test_git_branch_force_first_delete_detected(self): + cmd = "git branch --force --delete feature-branch" + dangerous, _, _ = detect_dangerous_command(cmd) + assert dangerous is True + + def test_git_branch_long_delete_without_force_not_flagged(self): + """Plain --delete (merged-only, equivalent to -d) has no force + token, so the new combined delete+force patterns must not fire — + only an actual force flag alongside it should trigger.""" + cmd = "git branch --delete feature-branch" + dangerous, _, _ = detect_dangerous_command(cmd) + assert dangerous is False + class TestChmodExecuteCombo: """chmod +x && ./ is the two-step social engineering pattern where a @@ -1689,6 +1744,25 @@ class TestDetectSudoStdin: is_dangerous, _, _ = detect_dangerous_command("sudo --askpass id") assert is_dangerous is True + def test_stdin_abbreviated_flag_detected(self): + # sudo's option parser resolves unambiguous long-flag prefixes + # just like git's does — `sudo --stdi` runs identically to + # `sudo --stdin` (verified against a live sudo binary: both + # produce the same "a password is required" outcome, versus a + # genuinely unrecognized option which errors differently). + is_dangerous, _, _ = detect_dangerous_command("sudo --stdi id") + assert is_dangerous is True + + def test_askpass_abbreviated_flag_detected(self): + # `--askpass` is the only sudo long option starting with "a", so + # any prefix from `--a` up resolves to it unambiguously. + is_dangerous, _, _ = detect_dangerous_command("sudo --ask id") + assert is_dangerous is True + + def test_askpass_single_char_abbreviation_detected(self): + is_dangerous, _, _ = detect_dangerous_command("sudo --a id") + assert is_dangerous is True + def test_two_sudo_invocations_second_caught(self): # The first sudo here is benign (no -S); the second has -S. # Lazy [^;|&\n]*? does NOT span past `;`, so re.search anchors @@ -1712,6 +1786,17 @@ class TestDetectSudoStdin: is_dangerous, _, _ = detect_dangerous_command("sudo -u root -i") assert is_dangerous is False + def test_sudo_set_home_not_confused_with_stdin_abbreviation(self): + # `--set-home` shares no prefix with `--stdin` beyond "--s", so + # the broadened `--st[a-z]*` pattern must not catch it. + is_dangerous, _, _ = detect_dangerous_command("sudo --set-home id") + assert is_dangerous is False + + def test_sudo_shell_flag_not_confused_with_stdin_abbreviation(self): + # `--shell` shares "--s" but not "--st" with `--stdin`. + is_dangerous, _, _ = detect_dangerous_command("sudo --shell id") + assert is_dangerous is False + def test_man_sudo_safe(self): is_dangerous, _, _ = detect_dangerous_command("man sudo") assert is_dangerous is False diff --git a/tools/approval.py b/tools/approval.py index b9da1df8a2a..cdf4f0923a9 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -655,11 +655,27 @@ DANGEROUS_PATTERNS = [ (r'\b(bash|sh|zsh|ksh)\s+<<', "shell execution via heredoc"), # Git destructive operations that can lose uncommitted work or rewrite # shared history. Not captured by rm/chmod/etc patterns. - (r'\bgit\s+reset\s+--hard\b', "git reset --hard (destroys uncommitted changes)"), + # `git reset --hard` accepts any unambiguous long-flag prefix (--h, + # --ha, --har, --hard) because git's own option parser resolves + # abbreviated long flags -- `--hard` is the only `git reset` mode + # starting with "h" (siblings are --soft/--mixed/--merge/--keep), so + # this cannot collide with another reset mode. It also does not match + # `--help`, which git special-cases before mode resolution. + (r'\bgit\s+reset\s+--h(?:a(?:r(?:d)?)?)?\b', "git reset --hard (destroys uncommitted changes)"), (r'\bgit\s+push\b.*--forc[a-z]*\b', "git force push (rewrites remote history)"), (r'\bgit\s+push\b.*-f\b', "git force push short flag (rewrites remote history)"), (r'\bgit\s+clean\s+-[^\s]*f', "git clean with force (deletes untracked files)"), (r'\bgit\s+branch\s+-D\b', "git branch force delete"), + # `-D` is shorthand for `-d --force`; the long-flag spellings + # (`--delete`, `--force`) are different tokens entirely, so they slip + # past the `-D\b` pattern above even though `git branch -d --force` + # and `git branch --delete --force` delete an unmerged branch exactly + # like `-D` does. Match delete+force in either order, bounded to the + # same command segment (not spanning `;`/`|`/`&`/newline) the same + # way the sudo patterns below do, to avoid contaminating an unrelated + # later command in the same script. + (r'\bgit\s+branch\b[^;|&\n]*?(?:-d\b|--delete\b)[^;|&\n]*?(?:-f\b|--force\b)', "git branch force delete (long flags)"), + (r'\bgit\s+branch\b[^;|&\n]*?(?:-f\b|--force\b)[^;|&\n]*?(?:-d\b|--delete\b)', "git branch force delete (long flags, force-first)"), # Script execution after chmod +x — catches the two-step pattern where # a script is first made executable then immediately run. The script # content may contain dangerous commands that individual patterns miss. @@ -677,7 +693,14 @@ DANGEROUS_PATTERNS = [ # are gated below. Lazy `[^;|&\n]*?` allows flag arguments (e.g. # `sudo -u root -S whoami`) without spanning command separators. See # #17873 category 4. - (r'\bsudo\b[^;|&\n]*?\s+(?:-s\b|--stdin\b|-a\b|--askpass\b)', + # sudo's own option parser (like git's) resolves unambiguous + # long-flag prefixes, so `sudo --stdi` runs identically to + # `sudo --stdin` and `sudo --ask` to `sudo --askpass` -- confirmed + # against a live sudo binary. `--st[a-z]*` and `--a[a-z]*` are safe + # to match broadly: per `man sudo`, `--stdin` is the only long option + # starting with "st" (siblings are --shell/--set-home) and + # `--askpass` is the only one starting with "a" at all. + (r'\bsudo\b[^;|&\n]*?\s+(?:-s\b|--st[a-z]*\b|-a\b|--a[a-z]*\b)', "sudo with privilege flag (stdin/askpass/shell/list)"), # Combined short-flag form: -nS, -ns, -sa, -las — sudo flags packed # into a single -X token. Catches the same threat class.