fix(ci): follow bounded apt-get into composite actions, and cap it at the caller

.github/checks-manifest.yaml

@@ -36,9 +36,9 @@ checks:
     reason: "#9440: local CI prediction must select only bounded Flutter generator, desktop-flow, and Windows KG-worker checks for the final PR diff"
   - id: workflow-apt-network-bounds
     command: ["bash", "scripts/run-workflow-apt-network-bounds.sh"]
-    triggers: [".github/workflows/**", ".github/scripts/check_workflow_apt_network_bounds.py", ".github/scripts/test_check_workflow_apt_network_bounds.py", "scripts/run-workflow-apt-network-bounds.sh", ".github/checks-manifest.yaml"]
+    triggers: [".github/workflows/**", ".github/actions/**", ".github/scripts/check_workflow_apt_network_bounds.py", ".github/scripts/test_check_workflow_apt_network_bounds.py", "scripts/run-workflow-apt-network-bounds.sh", ".github/checks-manifest.yaml"]
     lanes: ["local", "ci"]
-    reason: "apt has no built-in timeout; a stalled Azure mirror burns the job ceiling and reports an opaque cancellation. #11872 bounded two workflows by hand and still left an install line unbounded under a comment claiming otherwise"
+    reason: "apt has no built-in timeout; a stalled Azure mirror burns the job ceiling and reports an opaque cancellation. #11872 bounded two workflows by hand and still left an install line unbounded under a comment claiming otherwise. #12194 proposes moving a bounded install into a composite action, so the guard follows apt into .github/actions/** and enforces the step ceiling at each caller, where timeout-minutes is a valid key"
   - id: pre-push-hatch-disclosure
     command: ["python3", ".github/scripts/check_pre_push_hatch_disclosure.py"]
     triggers: ["scripts/pre-push", ".github/scripts/check_pre_push_hatch_disclosure.py", ".github/checks-manifest.yaml"]

.github/scripts/check_workflow_apt_network_bounds.py

@@ -18,6 +18,24 @@
     mirror fails over quickly instead of hanging.
   * `timeout-minutes` on the enclosing step, as a backstop for anything the
     acquire options do not cover (dpkg, a debconf prompt, a wedged post-install).
+
+COMPOSITE ACTIONS ARE IN SCOPE TOO, AND THE SECOND REQUIREMENT MOVES WHEN THEY ARE.
+A bounded apt call may now legitimately live in `.github/actions/*/action.yml` rather
+than in a workflow - #12194 moves the hermetic-gauntlet redis install into
+`.github/actions/install-redis-server` so three jobs share one copy. Scanning only
+`.github/workflows/*.yml` would leave that home unguarded, so "a new one cannot be
+added unbounded" would quietly stop being true for the only place the bounded calls
+still live.
+
+Pointing the old checker at an `action.yml` would NOT have caught it either: a
+composite action keeps its steps under `runs.steps`, not `jobs.<id>.steps`, so
+`check_workflow` finds no steps and returns clean. The blind spot fails open.
+
+`timeout-minutes` is not a valid key on a composite-action step - GitHub rejects the
+workflow that uses one - so the ceiling cannot be enforced where the apt line now is.
+It is enforced at every call site instead (`check_workflow_callers`). That split is
+the whole point: without it, moving an install into an action would launder away the
+backstop while the guard still reported success.
 """
 
 from __future__ import annotations
@@ -92,23 +110,137 @@ def check_workflow(path: Path) -> list[str]:
     return problems
 
 
+def _composite_steps(document: dict):
+    """Yield the steps of a composite action. Other action kinds have no `run:` steps.
+
+    A composite action's steps live under `runs.steps`, not `jobs.<id>.steps`, so
+    `_iter_steps` yields nothing for one. That is why pointing the workflow checker at an
+    `action.yml` returns clean rather than complaining: the shape simply does not match.
+    """
+    runs = document.get("runs")
+    if not isinstance(runs, dict) or runs.get("using") != "composite":
+        return
+    for index, step in enumerate(runs.get("steps") or []):
+        if isinstance(step, dict):
+            yield index, step
+
+
+def check_action(path: Path) -> list[str]:
+    """Composite actions: require the acquire bounds, but NOT `timeout-minutes`.
+
+    `timeout-minutes` is not a valid key on a composite-action step - GitHub rejects the
+    workflow that uses it. The backstop can therefore only live on the CALLER, which is
+    what `check_workflow_callers` enforces. Requiring it here would be unsatisfiable, and
+    a guard nobody can satisfy gets deleted rather than obeyed.
+    """
+    try:
+        document = yaml.safe_load(path.read_text(encoding="utf-8"))
+    except yaml.YAMLError as error:  # pragma: no cover - malformed YAML is another check's job
+        return [f"{path}: could not parse ({error.__class__.__name__})"]
+    if not isinstance(document, dict):
+        return []
+
+    problems: list[str] = []
+    for index, step in _composite_steps(document):
+        script = step.get("run")
+        if not isinstance(script, str):
+            continue
+        label = step.get("name") or f"step #{index}"
+        for line in _network_apt_lines(script):
+            missing = [option for option in REQUIRED_OPTIONS if option not in line]
+            if missing:
+                problems.append(
+                    f"{path}: {label!r}: apt-get network call is unbounded "
+                    f"(missing {', '.join(missing)}): {line}"
+                )
+    return problems
+
+
+def _action_reference(repo_root: Path, action_path: Path) -> str:
+    """The `uses:` string a workflow writes to call this local action."""
+    return "./" + action_path.parent.relative_to(repo_root).as_posix()
+
+
+def check_workflow_callers(path: Path, apt_action_refs: set[str]) -> list[str]:
+    """Every step that `uses:` an apt-running local action must cap itself.
+
+    This is the other half of the bound. The acquire options are enforced inside the
+    action; the ceiling cannot be, so it is enforced at each call site. Without this the
+    guard would report success on a tree where an unbounded-ceiling apt install had simply
+    been moved one file down.
+    """
+    if not apt_action_refs:
+        return []
+    try:
+        document = yaml.safe_load(path.read_text(encoding="utf-8"))
+    except yaml.YAMLError as error:  # pragma: no cover - malformed YAML is another check's job
+        return [f"{path}: could not parse ({error.__class__.__name__})"]
+    if not isinstance(document, dict):
+        return []
+
+    problems: list[str] = []
+    for job_name, index, step in _iter_steps(document):
+        uses = step.get("uses")
+        if not isinstance(uses, str) or uses.rstrip("/") not in apt_action_refs:
+            continue
+        if "timeout-minutes" not in step:
+            label = step.get("name") or f"step #{index}"
+            problems.append(
+                f"{path}: job {job_name!r}, {label!r}: uses {uses}, which runs apt-get over "
+                f"the network, but declares no timeout-minutes backstop"
+            )
+    return problems
+
+
 def main(argv: list[str]) -> int:
-    root = Path(argv[1]) if len(argv) > 1 else Path(".github/workflows")
-    paths = sorted(p for p in root.glob("*.yml")) + sorted(p for p in root.glob("*.yaml"))
+    # The argument is the REPOSITORY ROOT, because the check now spans two directories.
+    # The historical form - a path to the workflows directory - is still accepted so an
+    # existing caller does not silently start scanning nothing.
+    given = Path(argv[1]) if len(argv) > 1 else Path(".")
+    if given.name in ("workflows", "actions") and given.parent.name == ".github":
+        repo_root = given.parent.parent
+    else:
+        repo_root = given
+
+    workflow_root = repo_root / ".github" / "workflows"
+    action_root = repo_root / ".github" / "actions"
+
+    paths = (sorted(p for p in workflow_root.glob("*.yml"))
+             + sorted(p for p in workflow_root.glob("*.yaml")))
+    action_paths = (sorted(action_root.glob("*/action.yml"))
+                    + sorted(action_root.glob("*/action.yaml")))
+
     problems: list[str] = []
+    apt_action_refs: set[str] = set()
+    for path in action_paths:
+        problems.extend(check_action(path))
+        document = yaml.safe_load(path.read_text(encoding="utf-8"))
+        if isinstance(document, dict) and any(
+            _network_apt_lines(step.get("run"))
+            for _, step in _composite_steps(document)
+            if isinstance(step.get("run"), str)
+        ):
+            apt_action_refs.add(_action_reference(repo_root, path))
+
     for path in paths:
         problems.extend(check_workflow(path))
+        problems.extend(check_workflow_callers(path, apt_action_refs))
     if problems:
         print("Unbounded apt-get network calls in workflows:\n", file=sys.stderr)
         for problem in problems:
             print(f"  - {problem}", file=sys.stderr)
         print(
             "\nAdd -o Acquire::Retries=3 -o Acquire::http::Timeout=N -o Acquire::https::Timeout=N "
-            "to the apt-get line, and timeout-minutes to the step.",
+            "to the apt-get line, and timeout-minutes to the step. In a composite action the "
+            "ceiling belongs on each caller instead - timeout-minutes is not a valid key on a "
+            "composite-action step.",
             file=sys.stderr,
         )
         return 1
-    print(f"apt-get network bounds OK ({len(paths)} workflow files)")
+    print(
+        f"apt-get network bounds OK ({len(paths)} workflow files, "
+        f"{len(action_paths)} composite actions)"
+    )
     return 0
 
 

.github/scripts/test_check_workflow_apt_network_bounds.py

@@ -73,5 +73,120 @@ def test_repository_workflows_are_bounded(self) -> None:
         self.assertEqual(problems, [], "\n".join(problems))
 
 
+class CompositeActionBoundsTests(unittest.TestCase):
+    """A bounded apt call may live in a composite action; the guard must follow it there."""
+
+    def setUp(self) -> None:
+        self._tmp = tempfile.TemporaryDirectory()
+        self.addCleanup(self._tmp.cleanup)
+        self.tmp_path = Path(self._tmp.name)
+
+    def _action(self, run: str, *, name: str = "install-redis-server") -> Path:
+        directory = self.tmp_path / ".github" / "actions" / name
+        directory.mkdir(parents=True, exist_ok=True)
+        step = "  steps:\n    - name: install\n      shell: bash\n      run: |\n"
+        for line in run.splitlines():
+            step += f"        {line}\n"
+        path = directory / "action.yml"
+        path.write_text(f"name: {name}\nruns:\n  using: composite\n{step}")
+        return path
+
+    def _caller(self, uses: str, *, timeout: bool = True, name: str = "wf.yml") -> Path:
+        directory = self.tmp_path / ".github" / "workflows"
+        directory.mkdir(parents=True, exist_ok=True)
+        step = f"      - name: deps\n        uses: {uses}\n"
+        if timeout:
+            step += "        timeout-minutes: 5\n"
+        path = directory / name
+        path.write_text(
+            f"name: t\non: push\njobs:\n  j:\n    runs-on: ubuntu-latest\n    steps:\n{step}"
+        )
+        return path
+
+    def test_the_old_checker_fails_open_on_an_action(self) -> None:
+        """Why a separate entry point exists: the workflow shape does not match an action.
+
+        `check_workflow` looks under `jobs.<id>.steps`; a composite action keeps its steps
+        under `runs.steps`. Pointed at an action file it reports clean, so widening only
+        the glob would have produced a guard that scanned the file and still saw nothing.
+        """
+        path = self._action("sudo apt-get install -y redis-server")
+        self.assertEqual(guard.check_workflow(path), [])
+        self.assertTrue(guard.check_action(path))
+
+    def test_unbounded_apt_in_an_action_is_rejected(self) -> None:
+        path = self._action("sudo apt-get install -y redis-server")
+        problems = guard.check_action(path)
+        self.assertEqual(len(problems), 1, problems)
+        self.assertIn("Acquire::Retries", problems[0])
+
+    def test_bounded_apt_in_an_action_passes_without_step_timeout(self) -> None:
+        """`timeout-minutes` is not a valid key on a composite step, so it is not required.
+
+        Demanding it here would make the guard unsatisfiable for the one shape it now has
+        to cover.
+        """
+        path = self._action(f"{BOUNDED} update\n{BOUNDED} install --yes redis-server")
+        self.assertEqual(guard.check_action(path), [])
+
+    def test_non_composite_actions_are_left_alone(self) -> None:
+        directory = self.tmp_path / ".github" / "actions" / "js-action"
+        directory.mkdir(parents=True)
+        path = directory / "action.yml"
+        path.write_text("name: js\nruns:\n  using: node20\n  main: index.js\n")
+        self.assertEqual(guard.check_action(path), [])
+
+    def test_caller_of_an_apt_action_must_declare_a_ceiling(self) -> None:
+        """The backstop moves to the call site; nothing else can carry it."""
+        self._action(f"{BOUNDED} install --yes redis-server")
+        caller = self._caller("./.github/actions/install-redis-server", timeout=False)
+        problems = guard.check_workflow_callers(
+            caller, {"./.github/actions/install-redis-server"}
+        )
+        self.assertEqual(len(problems), 1, problems)
+        self.assertIn("timeout-minutes", problems[0])
+
+    def test_caller_with_a_ceiling_passes(self) -> None:
+        self._action(f"{BOUNDED} install --yes redis-server")
+        caller = self._caller("./.github/actions/install-redis-server", timeout=True)
+        self.assertEqual(
+            guard.check_workflow_callers(caller, {"./.github/actions/install-redis-server"}), []
+        )
+
+    def test_callers_of_other_actions_are_not_burdened(self) -> None:
+        caller = self._caller("./.github/actions/detect-changes", timeout=False)
+        self.assertEqual(
+            guard.check_workflow_callers(caller, {"./.github/actions/install-redis-server"}), []
+        )
+
+    def test_main_walks_both_trees_and_catches_the_laundered_ceiling(self) -> None:
+        """End to end: bounded options inside the action, no ceiling on the caller."""
+        self._action(f"{BOUNDED} update\n{BOUNDED} install --yes redis-server")
+        self._caller("./.github/actions/install-redis-server", timeout=False)
+        self.assertEqual(guard.main(["prog", str(self.tmp_path)]), 1)
+
+    def test_main_passes_when_both_halves_are_present(self) -> None:
+        self._action(f"{BOUNDED} update\n{BOUNDED} install --yes redis-server")
+        self._caller("./.github/actions/install-redis-server", timeout=True)
+        self.assertEqual(guard.main(["prog", str(self.tmp_path)]), 0)
+
+    def test_main_still_accepts_the_historical_workflows_argument(self) -> None:
+        """The old call form pointed at `.github/workflows`; it must not scan nothing."""
+        self._caller("./.github/actions/detect-changes", timeout=True)
+        workflows = self.tmp_path / ".github" / "workflows"
+        self.assertEqual(guard.main(["prog", str(workflows)]), 0)
+
+    def test_repository_actions_are_bounded(self) -> None:
+        root = Path(__file__).resolve().parents[1] / "actions"
+        problems: list[str] = []
+        for path in sorted(root.glob("*/action.yml")):
+            problems.extend(guard.check_action(path))
+        self.assertEqual(problems, [], "\n".join(problems))
+
+    def test_repository_tree_passes_end_to_end(self) -> None:
+        repo_root = Path(__file__).resolve().parents[2]
+        self.assertEqual(guard.main(["prog", str(repo_root)]), 0)
+
+
 if __name__ == "__main__":
     unittest.main()