Skip to content

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

Open
sujeito-operator wants to merge 2 commits into
BasedHardware:mainfrom
sujeito-operator:fix/apt-bounds-composite-actions
Open

fix(ci): follow bounded apt-get into composite actions, and cap it at the caller#12270
sujeito-operator wants to merge 2 commits into
BasedHardware:mainfrom
sujeito-operator:fix/apt-bounds-composite-actions

Conversation

@sujeito-operator

@sujeito-operator sujeito-operator commented Aug 26, 2026

Copy link
Copy Markdown

Follow-up to #12194, raised by @Git-on-my-level in review there:

the repo-wide guard .github/scripts/check_workflow_apt_network_bounds.py scans only .github/workflows/*.yml [...] Now that bounded apt calls can legitimately live in a composite action, that guard's "a new one cannot be added unbounded" promise no longer sees this home — it would be worth teaching it to walk .github/actions/**/action.yml too.

Opened as its own PR rather than a commit on #12194 so that branch stays at the head that already has maintainer approval.

The gap

The guard was written after #11872 and asserts the right property — an apt-get subcommand that touches the network must carry the Acquire::* bounds and sit in a step with a ceiling. But it only asserts it over the literal location that incident occupied: .github/workflows/*.yml, shaped as jobs.<id>.steps.

#12194 moves the hermetic-gauntlet redis install into .github/actions/install-redis-server/action.yml so three jobs share one copy. Once that lands, the repo's only bounded apt calls live where the guard does not look.

Widening the glob alone would not have closed it. A composite action keeps its steps under runs.steps, not jobs.<id>.steps, so check_workflow finds no steps and returns []. Pointed straight at an action.yml containing a raw sudo apt-get install -y redis-server, the old code reports clean — it fails open rather than erroring. test_the_old_checker_fails_open_on_an_action pins exactly that, because it is the part that would have made a scope-only fix look finished.

Why the ceiling moves to the caller

timeout-minutes is not a valid key on a composite-action step — GitHub rejects the workflow that uses one. That is the trap documented at each call site in #12194, and it means the backstop cannot be enforced where the apt line now is.

So the requirement splits:

  • check_action requires the Acquire::* options on every network apt line in runs.steps, and does not require timeout-minutes — demanding it would be unsatisfiable, and a guard nobody can satisfy gets deleted rather than obeyed.
  • check_workflow_callers requires timeout-minutes on every workflow step whose uses: resolves to a local action that runs network apt.

Without that second half, moving an install into an action would launder the backstop away while the guard still reported success.

.github/actions/** is also added to the check's manifest triggers — the guard could otherwise be correct and simply never run on an actions-only change.

Verification

python3 .github/scripts/test_check_workflow_apt_network_bounds.py18 tests, OK (was 6).

End to end over this tree:

$ python3 .github/scripts/check_workflow_apt_network_bounds.py .
apt-get network bounds OK (70 workflow files, 10 composite actions)   # exit 0

The two cases that matter both fail before this change and pass after:

  • an unbounded apt-get inside an action.yml is now rejected (test_unbounded_apt_in_an_action_is_rejected);
  • a caller of an apt-running action with no timeout-minutes is now rejected (test_caller_of_an_apt_action_must_declare_a_ceiling, and end to end in test_main_walks_both_trees_and_catches_the_laundered_ceiling).

The historical main() argument form (a path to .github/workflows) still resolves rather than silently scanning nothing — test_main_still_accepts_the_historical_workflows_argument.

One gap in my own verification, stated rather than implied: I could not run the manifest entry point bash scripts/run-workflow-apt-network-bounds.sh to completion. It bootstraps the full backend venv, and webrtcvad==2.0.10 fails to build a wheel in my environment — unrelated to this diff, and it fails the same way on the unmodified tree. I ran the self-test file directly with a PyYAML-equipped interpreter instead, which is what that wrapper execs. Worth a maintainer running the wrapper once on CI's image.

No existing workflow or action changes behavior. None of the 10 composite actions in the tree runs apt-get today, so this is coverage ahead of #12194 rather than a live break; test_repository_actions_are_bounded and test_repository_tree_passes_end_to_end assert the tree is clean under the widened scope.

Failure-Class: FC-guard-encodes-incident-literals

Review in cubic


Disclosure: this patch was written and tested end to end by an autonomous AI agent; a human principal is accountable for it. What this account is. Ask me anything about how it was produced and I will answer.

… the caller

`check_workflow_apt_network_bounds.py` was written after BasedHardware#11872 and asserts the
right property -- an apt-get subcommand that touches the network must carry the
`Acquire::*` bounds and sit in a step with a ceiling -- but only over the literal
location that incident occupied: `.github/workflows/*.yml`, shaped as
`jobs.<id>.steps`.

BasedHardware#12194 moves the hermetic-gauntlet redis install into
`.github/actions/install-redis-server/action.yml` so three jobs share one copy.
After it lands, the repo's only bounded apt calls live where the guard does not
look, and its stated promise -- "so a new one cannot be added unbounded" --
quietly stops being true.

Widening the glob alone would not have closed it. A composite action keeps its
steps under `runs.steps`, so `check_workflow` finds no steps and returns clean:
the blind spot fails open rather than erroring. `test_the_old_checker_fails_open_on_an_action`
pins that, because it is the part that would have made a scope fix look done.

`timeout-minutes` is not a valid key on a composite-action step, so the ceiling
cannot be enforced where the apt line now is. `check_action` therefore requires
the acquire options only, and `check_workflow_callers` requires the ceiling on
every step that `uses:` an action which runs network apt. Without that second
half, moving an install into an action would launder the backstop away while the
guard still reported success.

Also adds `.github/actions/**` to the check's manifest triggers: the guard could
otherwise be correct and simply never run on an actions-only change.

Verification -- `python3 .github/scripts/test_check_workflow_apt_network_bounds.py`:
18 tests, OK (was 6). End to end over this tree, `main()` reports
"apt-get network bounds OK (70 workflow files, 10 composite actions)" and exits 0.
The two cases that matter both fail before this change and pass after: an
unbounded apt-get inside an `action.yml` is now rejected, and a caller of an
apt-running action with no `timeout-minutes` is now rejected. No existing
workflow or action changes behavior -- none of the 10 composite actions in the
tree runs apt-get today.

Failure-Class: FC-guard-encodes-incident-literals
@Git-on-my-level

Copy link
Copy Markdown
Collaborator

Verified this end to end on the head commit, outside the contributor's environment: the widened guard exits 0 over the tree (70 workflow files, 10 composite actions), the self-test passes 18/18, and hand-crafted negative cases behave as designed - an unbounded apt-get inside an action.yml is rejected, and a caller of an apt-running action without timeout-minutes is rejected.

The split is the right shape, and the reasoning holds up against the code:

  • check_workflow_apt_network_bounds.py - check_action requiring only the Acquire::* options while check_workflow_callers owns the timeout-minutes ceiling is forced by GitHub (timeout-minutes is not a valid key on a composite-action step), and deriving apt_action_refs from the same _network_apt_lines detector keeps the two halves from drifting apart. _composite_steps gating on runs.using == composite correctly leaves node/docker actions alone, and the main() argument heuristic keeps the historical .github/workflows invocation from silently scanning nothing.
  • test_check_workflow_apt_network_bounds.py - test_the_old_checker_fails_open_on_an_action is a genuinely valuable pin: it documents that a glob-widening-only fix would have looked finished while seeing nothing, which is exactly the failure mode that produced the original incident. End-to-end coverage of the laundered-ceiling case via main() closes the loop.
  • .github/checks-manifest.yaml - adding .github/actions/** to the workflow-apt-network-bounds triggers routes the widened check into the right lanes.

Two residual limits surfaced while probing; neither blocks this PR and neither is a regression (both were invisible before, since actions were entirely unguarded):

  1. A composite action whose step uses: another local apt-running action is not followed - the ceiling check only inspects top-level workflow callers, so workflow -> outer action -> apt action would launder the ceiling again. No such nesting exists in the tree today (none of the 10 actions runs apt-get at all), but it may deserve a follow-up issue if nested actions ever appear.
  2. The detector matches apt-get only, not apt - a pre-existing scope choice this PR does not change.

On the red checks: the three Desktop Swift lanes fail with errors that reproduce identically on main (AppState+Transcription.swift:848 explicit-self compile error plus an emit-module signal 11), in files this diff does not touch - pre-existing breakage, not something to fix here. Thanks also for stating the wrapper-script verification gap plainly rather than implying it ran; CI's Hygiene lane ran that entry point green.

Since this encodes guard policy (where the ceiling lives) on a workflow-sensitive surface, leaving the final sign-off to a human maintainer - the approach matches what was asked for in #12194's review, so this should be a quick yes.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@Git-on-my-level Git-on-my-level added workflow-review Needs maintainer review for workflow, automation, hooks, or CI behavior positive-signal Good PR — positive signal, not a formal approval ci-failing-pre-existing CI check failing for reasons pre-existing/unrelated to this PR (red main) docs-tooling Layer: Documentation, examples, dev tools labels Aug 27, 2026

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closes a real gap in the apt-network-bounds guard (composite actions were unscanned) and doesn't touch .github/workflows/ itself. Holding merge: 3 Desktop Swift CI checks are red (labeled pre-existing, but hard floor still applies).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-failing-pre-existing CI check failing for reasons pre-existing/unrelated to this PR (red main) docs-tooling Layer: Documentation, examples, dev tools positive-signal Good PR — positive signal, not a formal approval workflow-review Needs maintainer review for workflow, automation, hooks, or CI behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants