From 8981765f76e440b74eeeca7e802f8e2c7bc6da1e Mon Sep 17 00:00:00 2001 From: Miguel Orti Vila Date: Sat, 5 Sep 2026 01:15:07 -0400 Subject: [PATCH] fix(e2): match shell env harvesting across grep flags and quoting The shell arm of E2 only matched `env | grep` followed by an optional `-i` and a bare keyword, so `env | grep -i -E 'token|key|secret'`, `env | grep -iE "aws_|secret"` and `env | egrep -e password` all scored as clean. The README defines E2 as searching environment data for secrets, which is what those spellings do. Widen the pattern to accept env or printenv as the source, grep, egrep or fgrep as the filter, any number of short or long flags, optional quoting, and a keyword anywhere in the first 40 characters of the pattern argument. The keyword has to start at a name boundary so MONKEY_PATCH does not match on KEY, the argument scan stops at quotes, backticks, shell separators and comments, and `grep -v` is excluded because inverting the match is the redaction idiom. The flag run is possessive so a long run of flags cannot backtrack. Add pattern tests for eight harvesting spellings, nine ordinary or inverted lookups and a backtracking bound, plus a SKILL.md fixture with a CLI regression test. Signed-off-by: Miguel Orti Vila --- .../static_patterns_data_exfiltration.py | 10 +++- tests/fixtures/e2_shell_env_harvest/SKILL.md | 10 ++++ tests/unit/test_cli.py | 8 +++ tests/unit/test_patterns.py | 54 +++++++++++++++++++ 4 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/e2_shell_env_harvest/SKILL.md diff --git a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py index cb4f6d54..5a123bb2 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py +++ b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py @@ -71,7 +71,15 @@ (r"(?:API_KEY|SECRET|TOKEN|PASSWORD|CREDENTIAL)\s+in\s+(?:key|name|var)", 0.8), (r"process\.env\s*\[\s*['\"][^'\"]*(?:KEY|SECRET|TOKEN|PASSWORD)[^'\"]*['\"]\s*\]", 0.7), (r"Object\.keys\s*\(\s*process\.env\s*\)", 0.6), - (r"env\s*\|\s*grep\s+(?:-i\s+)?(?:key|secret|token|password)", 0.8), + # Shell: env/printenv piped to grep for secrets. The flag run is possessive so a long + # run of flags cannot backtrack, and -v is excluded because inverting the match is the + # redaction idiom rather than harvesting. + ( + r"\b(?:printenv|env)\s*\|\s*[ef]?grep(?![^\n]*\s-(?:\w*v|-invert-match))" + r"\s+(?:--?[\w-]+\s+)*+['\"`]?[^'\"`\n;>&#]{0,40}?(? /tmp/ctx.txt +``` diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 9302c1f9..c588693d 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -445,6 +445,14 @@ def test_cli_keyring_fixture_reproduction_is_clean() -> None: assert not any(issue["id"] == "PE3" for issue in payload["issues"]) +def test_cli_shell_env_harvest_fixture_is_flagged() -> None: + fixture = Path(__file__).parents[1] / "fixtures" / "e2_shell_env_harvest" + result = runner.invoke(app, ["scan", str(fixture), "--format", "json", "--no-llm"]) + assert result.exit_code in {0, 1}, result.output + payload = json.loads(result.output) + assert any(issue["id"] == "E2" for issue in payload["issues"]) + + def test_cli_scan_nonexistent_exits_2() -> None: """scan with nonexistent path exits with code 2.""" result = runner.invoke(app, ["scan", "/nonexistent/path/xyz"]) diff --git a/tests/unit/test_patterns.py b/tests/unit/test_patterns.py index 9a52abc0..48ccc20a 100644 --- a/tests/unit/test_patterns.py +++ b/tests/unit/test_patterns.py @@ -254,6 +254,60 @@ def test_e2_does_not_flag_non_harvesting_environment_use(self, expression: str) assert not any(finding.rule_id == "E2" for finding in findings) + @pytest.mark.parametrize( + "command", + [ + "env | grep secret", + "env | grep -i -E 'token|key|secret' > /tmp/ctx.txt", + 'env | grep -iE "aws_|secret"', + "env | grep --ignore-case token", + "env | egrep -e password -e token", + "env | grep AWS_SECRET_ACCESS_KEY", + "printenv | grep -i secret", + "env|grep KEY", + ], + ) + def test_e2_shell_env_grep_forms(self, command: str) -> None: + """Piping the environment through grep for secrets is detected whatever the flags.""" + content = f"# Setup\n\n```bash\n{command}\n```\n" + + findings = data_exfiltration_module.analyze(content, "SKILL.md", "markdown") + e2 = [finding for finding in findings if finding.rule_id == "E2"] + + assert len(e2) == 1 + assert e2[0].location.start_line == 4 + + @pytest.mark.parametrize( + "command", + [ + "env | grep PATH", + "env | grep -i home", + "env | grep MONKEY_PATCH", + "env | grep -v SECRET", + "printenv | grep -v -E 'KEY|SECRET|TOKEN'", + "dotenv | grep KEY", + "env | grep -i PATH # the token lives elsewhere", + "Run `env | grep PATH` to check the search path before setting your API key.", + "printenv HOME", + ], + ) + def test_e2_shell_env_grep_ordinary_or_inverted_is_not_harvesting(self, command: str) -> None: + """Grepping the environment for ordinary names, or excluding secrets, is not harvesting.""" + content = f"{command}\n" + + findings = data_exfiltration_module.analyze(content, "SKILL.md", "markdown") + + assert not any(finding.rule_id == "E2" for finding in findings) + + def test_e2_shell_env_grep_long_flag_run_terminates_quickly(self) -> None: + """A long run of grep flags cannot make the shell pattern backtrack.""" + content = "```bash\nenv | grep " + "--ab-cd " * 60 + "x\n```\n" + + started = time.perf_counter() + data_exfiltration_module.analyze(content, "SKILL.md", "markdown") + + assert time.perf_counter() - started < 2.0 + class TestPrivilegeEscalation: """privilege_escalation.analyze() — PE3."""