test(ci): cover container-run.sh's pin, refusal and opt-out paths - #11483
Draft
basvandijk wants to merge 1 commit into
Draft
test(ci): cover container-run.sh's pin, refusal and opt-out paths#11483basvandijk wants to merge 1 commit into
basvandijk wants to merge 1 commit into
Conversation
Stacked on #11422, which makes container-run.sh pull the dev/build image only by the digest committed in ci/container/*.digest. Extend the test-container-run job in ci-main.yml with CI coverage for that: * A test-container-run-preflight job decides once whether the matrix runs (the container tooling changed, or master / rc-- / hotfix-), checks that ci/container/*.digest agree with the ic-dev pin in .devcontainer/ and the ic-build pins in .github/workflows/, and skips the matrix with a notice on a pull request whose ci/container/TAG is out of sync with the container inputs (the autobuild bot commit brings it in sync and re-triggers CI); anywhere else that is an error. * The matrix gains legs for a local image squatting on the tag (the pinned digest must run and the tag is re-pointed to it), a malformed pin, an unknown digest with and without a squatting local image, edited container inputs with and without CONTAINER_RUN_ALLOW_UNPINNED=1, and the docker runtime. Every refusal leg asserts a fast non-zero exit, the expected message, and that nothing was pulled, built or run. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The security-sensitive workflow expansion depends on runtime-specific CI behavior that cannot be fully validated statically.
Pull request overview
Adds CI coverage for digest pinning, refusal, and opt-out behavior in container-run.sh.
Changes:
- Adds a preflight job validating digest references and container input synchronization.
- Expands the container-run matrix with security, failure, opt-out, and Docker cases.
- Verifies refusal paths do not alter Podman state.
File summaries
| File | Description |
|---|---|
.github/workflows/ci-main.yml |
Adds preflight validation and expanded container-run test coverage. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Extend the
test-container-runjob inci-main.ymlwith CI coverage for the digest pinning that #11422 adds toci/container/container-run.sh.Stacked on #11422: the base of this PR is
bas/pin-container-run-image-by-digestand it only changes.github/workflows/ci-main.yml. Once #11422 has merged, the base is retargeted tomaster.Why
#11422 makes
container-run.shpull the dev/build image only by the digest committed inci/container/ic-dev.digest/ci/container/ic-build.digest, refuse tampered pins and edited container inputs, and fall back to a local image or build only withCONTAINER_RUN_ALLOW_UNPINNED=1. The two happy-path legs of the existingtest-container-runjob cover none of the refusal paths. This coverage was split out of #11422 to keep that review smaller.How
test-container-run-preflightjob runs once (a single runner) and decides whether the matrix runs at all:ci/container/**,ci-main.yml,ci-pr-only.yml,container-autobuild.yml) or onmaster,rc--*andhotfix-*;ci/container/*.digestare well-formed and that everyghcr.io/dfinity/ic-dev/ic-buildreference in.devcontainer/and.github/workflows/is exactly the pinned digest reference (all of them are written bycontainer-autobuild.yml); this check also runs when only those pins changed;ci/container/TAGmatches the container inputs. On a pull request an out-of-syncTAGis transient (the autobuild bot commit bringsTAGand*.digestin sync and re-triggers CI), so the matrix is skipped with a notice; anywhere else it is an error.test-container-runmatrix (fail-fast: false) gains, next to theic-devdefault and--image ic-buildhappy paths:ghcr.io/dfinity/ic-dev:<TAG>is ignored, the pinned digest is pulled and run, and the tag is re-pointed to it (verified viaRepoDigests);not-a-digestinic-dev.digest→ "refusing to pull an unpinned image";ci/container/Dockerfileedited after the preflight gate → "refusing to run an unpinned image";CONTAINER_RUN_ALLOW_UNPINNED=1an image cached under the computed tag is reused, and the output must say it is "NOT verified against a reviewed digest pin";CONTAINER_RUNTIME=docker.timeout 300, and not124), the expected message, and that the podman image store and container list are unchanged, i.e. nothing was pulled, built or run.