Skip to content

fix(ci): bound runner isolation's apt calls so a stall fails, not hangs - #11760

Merged
kyle-sexton merged 2 commits into
mainfrom
fix/runner-isolation-apt-timeout
Nov 1, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
fix/runner-isolation-apt-timeout

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: every pr-verify and pr-review-main job hung in "Isolate the runner" from 2026-10-31T22:51Z; found live, no issue filed while driving issues to zero

Summary

.github/actions/setup-runner-isolation installs bubblewrap and socat with apt-get update and apt-get install and no timeout. From 22:51Z on 2026-10-31, every job that runs it stalled in that step and sat until its job timeout. That covered pr-verify's check-intent and check-intent-subscription (runs 41249968756, 41250230952, 41251122491, 41254234564, 41254411010, 41255111914, 41256182626) and pr-review-main's review-subscription (41249968733, 41250230869). With the key leg also stopping on class=credit-stop on PR_VERIFY_CLAUDE_API_KEY, no verify verdict could land, so verdict-gated merges stalled.

Fix

  • Each apt call runs under timeout 180, with Acquire::Retries=2, 30 s HTTP and HTTPS timeouts and a 60 s dpkg lock timeout. A stall now fails the attempt, and the existing 3-try loop retries.
  • When bwrap and socat are already on the image, apt is skipped.
  • Unchanged: the AppArmor sysctl, the fail-closed check that bwrap exists, and the sudo and docker removal step. A run that still cannot install fails in minutes rather than hours, and the lost-run reconciler re-runs it.

Verification

  • shellcheck on the step's script: clean.
  • No test pins the step's script. thread-scope.test.mjs lists the file's path only, and write-runner-never-rules.test.sh checks the uses: reference only.
  • The trigger of the stall is UNVERIFIED: a run's log cannot be read while it runs. The action is unchanged since 2026-10-05 (feat(pr-pipeline): split the runner into key-free read and write workflows #6382), which points to an Ubuntu mirror or apt stall on hosted runners. The fix bounds the step whatever the cause.

Related

  • The hung runs stay until their job timeout. Their PRs need a fresh verify event once this merges.
  • The API key's credit stop is a separate, owner-held secret rotation.

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits October 31, 2026 20:29
setup-runner-isolation ran `apt-get update` and `install` with no timeout.
Since 2026-10-31T22:51Z every pr-verify and pr-review-main job that runs it
has sat in "Isolate the runner" until its job timeout, so no verify verdict
lands. Each apt call now runs under `timeout 180` with Acquire and dpkg-lock
timeouts, and an image that already ships bwrap and socat skips apt. A run
that still cannot install fails closed, as before, and the lost-run
reconciler re-runs it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 1, 2026 00:42
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-01T00:43:19.050299Z f5bfd8f Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed dc18936048356af4b1f76f35d07c1ffac8925642: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #11760

I found no security issues.

Scope: one changed file, .github/actions/setup-runner-isolation/action.yml. I read every hunk of it and the whole head file. This action decides how pull request code is isolated, so I checked it as a trust control.

What changed: in the "Install subprocess isolation" step:

  • Each apt-get update and apt-get install call now runs under timeout 180.
  • Each call also gets bounded Acquire retries, HTTP and HTTPS timeouts, and a dpkg lock timeout.
  • When bwrap and socat are already on PATH, the step skips apt.

Does it widen isolation? No.

  • Fail-closed check: the check after the loop is unchanged (L44-L47). A timed-out or failed install still leaves no bwrap, and the step still exits 1.
  • Skip when the tools are present: I checked whether this new branch (L32-L34) lets planted binaries through. It doesn't, for two reasons:
    • The action runs before pull request head code (its description, L3), so no untrusted input has had a chance to put a fake bwrap or socat on PATH.
    • The base already relied on the same command -v bwrap lookup for its fail-closed gate, so the new branch trusts nothing the base didn't.
  • Other controls unchanged: the AppArmor userns sysctl and the "Remove sudo and docker access" step (L52-L67) are untouched. A timeout can only fail this step early. It can't skip the sudo and docker removal that follows, because a failed composite step stops the job.
  • Injection: the apt array is a fixed literal, with no github context or input interpolated into run:. Nothing new reaches a shell.

No token scope, trusted actor, kill switch or merge hold is touched. The PR context contains no embedded instructions aimed at the review.

Spend: none beyond the runner minutes the job already uses. The bounded timeouts shorten hung jobs.

critical-findings: 0

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed dc18936048356af4b1f76f35d07c1ffac8925642: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #11760, bound runner isolation's apt calls

Scope: 1 file changed, .github/actions/setup-runner-isolation/action.yml. I read its whole hunk and the full head file. The file does not match any TRUSTED PATHS: glob. There is no closing issue: the PR says the problem was found live and no issue was filed. The PR context file contains no embedded instructions.

Result: no findings.

What I checked in the changed Install subprocess isolation step:

  • Shell -e behaviour. Composite shell: bash runs with -eo pipefail. In timeout … update && timeout … install && break, the last command in the list is break. A failed or timed-out update or install therefore doesn't trip errexit, and the 3-attempt loop still retries, as it did before this change.
  • Bounds. Each apt call is capped at 180 s. DPkg::Lock::Timeout=60 and the 30 s HTTP/HTTPS acquire timeouts fit inside that cap. In the worst case all three attempts time out, which takes about 3 × (180 + 180 + 5) s ≈ 18 min, after which the existing bwrap check fails the step closed.
  • Skip check. The step runs command -v bwrap && command -v socat before each attempt. It skips apt when the image already has both tools, and it ends the loop early once an earlier attempt has installed them. Neither path weakens the fail-closed check below it.
  • Isolation unchanged. The Remove sudo and docker access step still runs afterwards, unconditionally. Bounding the apt calls changes nothing about when sudo is removed or what happens if removal fails.

I looked at one edge and decided it doesn't clear the bar. If timeout fires during dpkg's unpack/configure phase rather than during the download, dpkg can be left interrupted, and the later retries would then fail fast until someone runs dpkg --configure -a. For two small packages, the stall almost certainly happens in the acquire phase. Even if it doesn't, the run still fails closed within minutes, so I'm noting it here rather than raising it as a finding.

Verification: the PR's verification (shellcheck clean, and no test pins the script) is the author's claim and I did not check it. This lane had no shell, and I ran nothing.

Spend: none. The change doesn't alter turn limits, models, schedules or runner sizes.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit e48b56d Nov 1, 2026
32 checks passed
@kyle-sexton
kyle-sexton deleted the fix/runner-isolation-apt-timeout branch November 1, 2026 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant