Repository navigation
🤖 tests: bug-bash sandbox self-check and CI job (D2) - #6085
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fb6315ff1
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The app check subscribed to the app's exit only after it sent SIGTERM. An app that had already exited during startup never fires that event again, so the check hung until the CI job's timeout (Codex review on #6085). It now subscribes at spawn, stops polling /health once the app is gone, and reports the app's exit code. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
- sandbox/selfCheck.ts (in the container) and selfCheckRun.ts (host, `make bug-bash-sandbox-check`, $0 against a fake upstream, no key): one line per check. The container runs as the host uid (nonzero), with CapEff 0, NoNewPrivs 1, seccomp 2 and the host's boot ID. /, /repo/src, /repo/node_modules and /repo/dist are read-only (EROFS), the home is empty, /home holds no host home, and no docker.sock or ~/.docker exists. Only lo exists, with no IPv4 route. Loopback gives ECONNREFUSED, 172.17.0.1 and 1.1.1.1 give ENETUNREACH, and DNS fails (EAI_AGAIN or ENOTFOUND). No credential env name is set. The proxy socket is a unix socket on a read-only mount. One allowed proxy call passes, and the P1-P5 cases refuse. A model-driven app started by startApp has all three XUM_DISABLE_* switches. - The host side checks that the fake upstream saw exactly the allowed call and no key. Two more jobs export a path-traversal frame and an oversized frame: each must refuse with its own message, exit 4, clean up, and leave no file outside the output folder. - launch.ts: a fourth job kind, `self-check [--export-fixture traversal|oversize] --output .e2e/<dir>`, with a fixed command and the model-driven guard. entry.ts accepts --export-fixture only for that command. - launch.ts procShowsAll(): the last /proc line of mountinfo is the mount in effect, so it reads that one, not the first (D1 readiness review). - Makefile: `test-bugbash-repros-sandbox` runs the repros of fixed bugs through the launcher on the mock app AI. - pr.yml: new `Test / Bug-bash sandbox` job (no secrets) runs both targets; Required needs it. E2E shard 1 keeps its host repros for now. - docs/AGENTS.md: the sandbox checks, the CI job and what the launcher refuses. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
The app check subscribed to the app's exit only after it sent SIGTERM. An app that had already exited during startup never fires that event again, so the check hung until the CI job's timeout (Codex review on #6085). It now subscribes at spawn, stops polling /health once the app is gone, and reports the app's exit code. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
189de75 to
d09d4e6
Compare
selfCheckRun.ts accepted any non-empty checks.jsonl from the container, so a self-check that stopped early, or lost a check, still passed (D2 readiness review). It now requires every one of the 31 container checks by name, once each, and fails with the missing and unexpected names. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d09d4e6888
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s every job's upstream From the Codex review on #6085: - A stopped job now ends the self-check with Stopped (exit 130 or 143) and starts no other job, as for any launcher job. An unknown cleanup still outranks the stop. - Each export-fixture job runs the whole selfCheck.ts too, so it makes its own allowed call and P1-P5 probes. The upstream is now checked per job (exactly one allowed call, no forbidden one, nothing else, no key), not only after the first job. - The in-container /health probe has a 2 s timeout, so a server that accepts and never answers cannot hang the check. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d8a3a7e26
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…r a stop The round-3 change skipped the stop when a job's cleanup was unknown, so the self-check went on to the next job with the aborted signal and exited 130 or 143 instead of 3 (Codex review on #6085). exitFor()'s rule now applies: an unknown cleanup ends the run at once with exit 3, and outranks a stop. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 545ddea634
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Review loop stopped: this PR is at its 6-round Codex cap and waits for a waiver. Head
The proposed fix: one small patch with both fixes, plus a test where Generated with |
…on a refusal From the last Codex review round on #6085: - F1: "no capabilities" read only CapEff, so a container that kept a capability in its permitted, inheritable, bounding or ambient set still passed, and a permitted capability can be made effective again. The check now requires CapInh, CapPrm, CapEff, CapBnd and CapAmb all to be zero, and names any nonzero or missing set. With the launcher's flags all five are zero. - F2: a Refusal that Session raises after the preflight (root, rootless Docker, Docker Desktop) came back in the job outcome, so the self-check recorded failed checks, started the other jobs and exited 1. It now re-throws the outcome's error after the unknown-cleanup and stop checks: a Refusal exits 2 and no further job starts. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
This comment has been minimized.
This comment has been minimized.
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Summary
The bug-bash sandbox gets a self-check that costs nothing (
make bug-bash-sandbox-check), a target that runs the repros of fixed bugs through the sandbox launcher (make test-bugbash-repros-sandbox), and a new required CI job with no secrets that runs both. This is PR D2 of the #5714 plan, stacked on D1 (#6082). It closes the sandbox CI job (#5931) and the runner docs (#5932).Refs #5714 #5931 #5932
Implementation
sandbox/selfCheck.tsin the container,sandbox/selfCheckRun.tson the host). It runs three jobs through the real launcher against a fake upstream, with no key. It prints one line per check and exits 0 only when every check passes:/,/repo/src,/repo/node_modulesand/repo/distare read-only (EROFS). The home is empty and writable./homeholds no host home, and nodocker.sockor~/.dockerexists.lo, and no IPv4 route exists. In Node probes, loopback gives ECONNREFUSED, 172.17.0.1 and 1.1.1.1 give ENETUNREACH, and DNS fails (EAI_AGAIN or ENOTFOUND). No credential env name is set.startApp.tshas all threeXUM_DISABLE_*switches.CONTAINER_CHECKS), and fails with the missing and unexpected names. A partial report never passes./healthprobe has a 2 s timeout.launch.ts,entry.ts): a fourth job kind,self-check [--export-fixture traversal|oversize] --output .e2e/<dir>. It has a fixed command and is model-driven, so it gets the proxy and the guard.entry.tsaccepts--export-fixtureonly for that command./procis mounted more than once, it reads the last/procline of mountinfo, which is the mount in effect, not the first.bug-bash-sandbox-checkandtest-bugbash-repros-sandbox. The second runs every repro of a fixed bug (known-failureexcluded) through the launcher, on the mock app AI..github/workflows/pr.yml): the new jobTest / Bug-bash sandbox(bugbash-sandbox) runs both targets without secrets, andRequiredneeds it. The launcher pulls the pinned image by digest without credentials. E2E shard 1 keeps its host repros until this job proves stable.docs/AGENTS.md, regeneratedbuiltInSkillContent.generated.ts): the two targets, the CI job, and what the launcher refuses with exit 2.CI cost
The
bugbash-sandboxjob runs its own full build (build-main build-renderer build-static) on every PR where thetestsfilter is true. That costs CI minutes. It adds nothing to the app's perf path.Size
The diff is 584 lines, over the 500-line target, because
selfCheck.tsalone is 291 lines.Validation
Items 1–3 ran on the pushed head. Item 4 ran on
6fb6315ff1: the later commits change only the self-check, which the repro run does not use.make static-checkpasses.bun test ./tests/bugbash/: 255 pass, 0 fail. The procShowsAll test fails when the change reverts to the first line.make bug-bash-sandbox-check: 54 passed, 0 failed. All three job containers were removed.make test-bugbash-repros-sandbox: 26 files and 50 tests passed, and the container was removed.no-new-privilegesremoved from the container flags: 2 checks failed and it exited 1.missing: no DNS.Follow-ups
These are tracked, not fixed in this PR.
bipis not probed. The "no IPv4 route" check (routes.length === 0) already proves the claim.builtInSkillContent.generated.tswith the next docs change to this section.Risks
Test and CI code only. The new required job depends on Docker on the Linux runner and on an anonymous pull of the pinned image from ghcr.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$160.50