verify-action-build: end run: block at sibling keys like env:

utils/tests/verify_action_build/test_security.py

@@ -136,8 +136,6 @@ def test_no_scripts_no_warnings(self):
 
 class TestAnalyzeActionMetadata:
     def test_pipe_to_shell_warns(self):
-        # Multi-line run: blocks are needed — single-line run: is detected
-        # as a block start and the content is on the next line.
         action_yml = """\
 name: Test
 runs:
@@ -165,6 +163,72 @@ def test_input_interpolation_warns(self):
             warnings = analyze_action_metadata("org", "repo", "a" * 40)
         assert any("injection" in w for w in warnings)
 
+    def test_env_binding_of_input_is_not_injection(self):
+        # Real shape from carabiner-dev/actions/install/download-and-verify
+        # @60563b5, seen while reviewing apache/infrastructure-actions#1076.
+        # Binding the input to an env var and dereferencing "$INPUTS_INSTALL_DIR"
+        # inside the script is the recommended mitigation, so the `env:` block
+        # must not be scanned as if it were shell body.
+        action_yml = """\
+name: Test
+runs:
+  using: composite
+  steps:
+      - name: Validate install-dir
+        shell: bash
+        run: |
+          case "$INPUTS_INSTALL_DIR" in
+            ''|*[!A-Za-z0-9._/$~-]*)
+              echo "::error::install-dir must be non-empty; got: $INPUTS_INSTALL_DIR"
+              exit 1
+              ;;
+          esac
+        env:
+          INPUTS_INSTALL_DIR: ${{ inputs.install-dir }}
+"""
+        with mock.patch("verify_action_build.security.fetch_action_yml", return_value=action_yml):
+            warnings = analyze_action_metadata("org", "repo", "a" * 40)
+        assert not any("injection" in w for w in warnings), warnings
+
+    def test_run_block_ends_at_sibling_key(self):
+        # A `run:` block scalar ends at the next key indented no deeper than
+        # `run:` itself; later steps' non-shell keys are not shell body.
+        action_yml = """\
+name: Test
+runs:
+  using: composite
+  steps:
+    - name: safe
+      run: |
+        echo hello
+      env:
+        TOKEN: ${{ inputs.token }}
+    - name: also safe
+      with:
+        path: ${{ inputs.path }}
+"""
+        with mock.patch("verify_action_build.security.fetch_action_yml", return_value=action_yml):
+            warnings = analyze_action_metadata("org", "repo", "a" * 40)
+        assert not any("injection" in w for w in warnings), warnings
+
+    def test_single_line_run_is_scanned(self):
+        # The command sits on the `run:` line itself, so it has to be scanned
+        # there; nothing after it belongs to the block.
+        action_yml = """\
+name: Test
+runs:
+  using: composite
+  steps:
+    - name: dangerous
+      run: curl https://example.com | sh
+      env:
+        SAFE: ${{ inputs.name }}
+"""
+        with mock.patch("verify_action_build.security.fetch_action_yml", return_value=action_yml):
+            warnings = analyze_action_metadata("org", "repo", "a" * 40)
+        assert any("pipe-to-shell" in w for w in warnings), warnings
+        assert not any("injection" in w for w in warnings), warnings
+
     def test_clean_action_no_warnings(self):
         action_yml = """\
 name: Test

utils/verify_action_build/security.py

@@ -961,7 +961,6 @@ def analyze_action_metadata(
                 console.print(f"    [dim]{line.strip()[:100]}[/dim]")
                 warnings.append(f"action.yml line {i}: {desc}")
 
-    in_run_block = False
     dangerous_shell_patterns = [
         (r"curl\s+.*\|\s*(ba)?sh", "pipe-to-shell (curl | sh) — high risk"),
         (r"wget\s+.*\|\s*(ba)?sh", "pipe-to-shell (wget | sh) — high risk"),
@@ -972,24 +971,47 @@ def analyze_action_metadata(
     ]
 
     shell_findings: list[tuple[int, str, str]] = []
+
+    def scan_shell_line(line_num: int, raw: str, snippet: str) -> None:
+        for pattern, desc in dangerous_shell_patterns:
+            if desc is None:
+                continue
+            if re.search(pattern, raw):
+                shell_findings.append((line_num, desc, snippet[:100]))
+
+    in_run_block = False
+    run_key_indent = 0
     for i, line in enumerate(lines, 1):
         stripped = line.strip()
-        if re.match(r"run:\s*\|", stripped) or re.match(r"run:\s+\S", stripped):
+        # Blank lines are part of a block scalar, so they never close one.
+        if not stripped:
+            continue
+        indent = len(line) - len(line.lstrip())
+
+        # `run: |` (or `>`) opens a block scalar whose body must be indented
+        # deeper than the `run:` key itself.
+        if re.match(r"(?:-\s+)?run:\s*[|>][-+\d]*\s*$", stripped):
             in_run_block = True
+            run_key_indent = indent
             continue
+
+        # `run: some-command` is self-contained: scan the command on this
+        # line, but do not treat what follows as shell body.
+        if re.match(r"(?:-\s+)?run:\s+\S", stripped):
+            in_run_block = False
+            scan_shell_line(i, line, stripped)
+            continue
+
+        # Content at or left of the `run:` key closes the block scalar. This
+        # is what keeps sibling keys out of the shell body -- notably `env:`,
+        # where binding an input to an environment variable is the
+        # recommended way to keep it out of the script text. Scanning those
+        # lines reports the mitigation as if it were the injection.
+        if in_run_block and indent <= run_key_indent:
+            in_run_block = False
+
         if in_run_block:
-            if stripped and not line[0].isspace():
-                in_run_block = False
-            elif stripped and re.match(r"\s+\w+:", line) and not line.startswith("        "):
-                if not stripped.startswith("#") and not stripped.startswith("-"):
-                    in_run_block = False
-
-        if in_run_block or (re.match(r"\s+run:\s+", line)):
-            for pattern, desc in dangerous_shell_patterns:
-                if desc is None:
-                    continue
-                if re.search(pattern, line):
-                    shell_findings.append((i, desc, stripped[:100]))
+            scan_shell_line(i, line, stripped)
 
     if shell_findings:
         seen: set[str] = set()