mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
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).
This commit is contained in:
parent
723ccda275
commit
74e59b8b68
2 changed files with 110 additions and 2 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue