The image you pull is gated as strictly as the code that built it (WOR-871) - #488
Conversation
…871)
The release gate sat at `severity-cutoff: critical` while WOR-852 moved the
PR gate to `medium`. A fixable High could be published to users while a
fixable Medium blocked a branch — the strict gate guarding the proposal, the
loose one guarding the artifact people actually run.
Worse, and the real defect: publish-docker.yml never ran
check_grype_ignore_expiry.py. Grype drops the unknown `expiry` key silently,
so the release path honoured all 7 suppressions in .grype.yaml with nothing
policing whether they had lapsed. docker-security.yml has run that hook since
WOR-852; the path that reaches users did not.
Changes:
- run the expiry hook before both release scans (cache disabled: this
workflow signs what it publishes, and zizmor rightly flags a cached
setup-uv here as a cache-poisoning route into a signed artifact)
- severity-cutoff critical -> medium on amd64 and arm64
- workflow_dispatch, so a blocked release can be re-run instead of
stranding a tag with no image and a 404 in the README
- install docs state the actual bar and link .grype.yaml, because
"vulnerability-scanned" reads as "zero known CVEs" and 7 are suppressed
Tests written first, verified RED against the old workflow:
- the release gate is never looser than the PR gate
- the release gate enforces ignore expiry
- the release gate can be re-run by hand
Measured on BOTH architectures (previously arm64 was never scanned locally):
amd64 and arm64 carry the identical 3 High + 4 Medium CPython findings, all
already suppressed, so the new gate exits 0 on both today.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe Docker publish workflow remains tag-triggered, validates Grype suppression expiry, and gates amd64 and arm64 releases on fixable Medium-or-higher vulnerabilities. Tests enforce the workflow structure, and documentation describes the scanning and signing policy. ChangesDocker release security
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant GrypeConfig
participant Grype
participant DockerRegistry
GitHubActions->>GrypeConfig: Validate suppression expiry
GitHubActions->>Grype: Scan amd64 and arm64 release images
Grype-->>GitHubActions: Return gated and informational findings
GitHubActions->>DockerRegistry: Promote images when release gates pass
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
tests/test_grype_ignore_expiry.py (1)
258-260: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCorrect the cutoff-order explanation.
severity-cutoffis the minimum severity that fails the build. Lower entries in_STRICTNESSare stricter, not looser. Themin(...)value is therefore the strictest PR cutoff, notweakest_pr. The assertion matches lower-is-stricter ordering, but the comments and variable name state the opposite. Rename the order constant and usestrictest_pr. (github.com)Also applies to: 285-289
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_grype_ignore_expiry.py` around lines 258 - 260, Correct the severity ordering documentation and naming around _STRICTNESS: rename the order constant to reflect that lower entries are stricter, and update the min(...) result and related references from weakest_pr to strictest_pr. Preserve the existing assertion and cutoff behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-docker.yml:
- Around line 119-129: Update the “Grype ignores are dated and current” step to
use a reviewed, hash-pinned PyYAML requirement or committed lock instead of
resolving it through `uv run --no-project --with pyyaml`. Ensure the
`check_grype_ignore_expiry.py` hook continues running with reproducible
dependency versions before the Grype scans.
- Around line 141-145: Update the release-gate workflow around the Grype amd64
scan and multi-arch docker/build-push-action flow so the scanned image inputs
are exactly the platform images ultimately published in the manifest. Either add
a digest/composition-level verification covering both amd64 and arm64 outputs,
or reuse the already scanned platform artifacts when creating and pushing the
manifest; do not leave the gate scanning independently rebuilt images.
- Around line 20-26: In .github/workflows/publish-docker.yml, add an explicit
validation step before the build and signing steps that verifies
workflow_dispatch is running on a semver release tag and fails immediately with
a clear error when it is not; preserve normal tag-triggered release behavior. In
tests/test_grype_ignore_expiry.py lines 305-314, update the manual-rerun test to
assert the validation guard exists and that invalid branch refs fail fast before
build/sign execution.
In `@docs/install-docker.md`:
- Around line 8-10: Update the image security sentence in the installation
documentation to state that images are signed with cosign before release tags
are promoted, replacing the claim that signing occurs before publish. Preserve
the existing multi-architecture scanning and signing details.
In `@tests/test_grype_ignore_expiry.py`:
- Around line 292-302: Update test_the_release_gate_enforces_ignore_expiry to
parse PUBLISH_WORKFLOW as YAML and identify a workflow step whose run field
actually invokes check_grype_ignore_expiry.py. Assert that this executable
expiry step appears before every build-failing anchore/scan-action step, rather
than relying on a raw text search.
- Around line 262-271: Update _gate_cutoffs to preserve each qualifying scan
step’s architecture identity alongside its severity cutoff, rather than
returning severities alone. Update the related assertions around the referenced
test cases to require both amd64 and arm64 release gates, then compare each
architecture’s cutoff independently.
---
Nitpick comments:
In `@tests/test_grype_ignore_expiry.py`:
- Around line 258-260: Correct the severity ordering documentation and naming
around _STRICTNESS: rename the order constant to reflect that lower entries are
stricter, and update the min(...) result and related references from weakest_pr
to strictest_pr. Preserve the existing assertion and cutoff behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 76802c0c-e830-45aa-abc3-da37d2234793
📒 Files selected for processing (3)
.github/workflows/publish-docker.ymldocs/install-docker.mdtests/test_grype_ignore_expiry.py
…(WOR-871)
Three independent reviews of the previous commit converged on the same
finding: the `workflow_dispatch` I added as break-glass was a publishing hole.
GitHub runs a workflow FROM the dispatched ref, so a branch dispatch supplies
both the Dockerfile and this workflow — a branch could delete the release gate
and still publish under the repo's OIDC identity. And because `type=semver`
yields no tags on a non-tag ref, the build, the digest push and `cosign sign`
all SUCCEED; only promotion fails, leaving a signed orphan digest whose Fulcio
SAN is `@refs/heads/...`, which the verify command we document rejects.
It also bought nothing. "Re-run failed jobs" on the original tag-push run
already re-runs a blocked release and preserves `refs/tags/v*`.
Removed, with a comment explaining why, and a test that fails if it returns.
Also from those reviews:
- pin `pyyaml==6.0.3`. The hook runs in the job holding `id-token: write`
and `packages: write`; an unpinned resolve would let a malicious release
execute with the signing token. Every action here is SHA-pinned already.
- move the expiry check ABOVE the builds, so a lapsed date fails in seconds
instead of after ~25 minutes of QEMU arm64 build — the principle the
cosign preflight above it already states.
- `output-format: table` on both release scans. The action defaults to
sarif written to a file nothing uploads, so the release gate's findings
reached no human at all.
- assert the release path has exactly 2 scans. Without arity, deleting the
arm64 step left the test green while an architecture shipped unscanned.
- the expiry test now parses YAML instead of grepping file text, so
`if: false`, `continue-on-error` or a comment cannot satisfy it.
- `_gate_cutoffs` lowercases and asserts on a missing key: `Medium` is
valid config for the action and previously raised ValueError.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The merge-ready harness found three guards that pass without the property
actually holding, plus one comment stating a recovery path that does not work.
- the trigger check was a denylist of one string. `workflow_call` (a callee
inherits the caller's ref), `repository_dispatch`, `schedule` or
`push: branches:` each reopen the publishing hole while leaving
"workflow_dispatch" absent. Now an allowlist: triggers == {push: {tags}}.
- `len(release) == 2` never looked at `image:`. Duplicating the amd64 scan
and deleting arm64 stayed green while an architecture shipped unscanned.
Now asserts two DISTINCT images and that one reads the arm64 tarball.
- the expiry test proved the step existed, not that it runs BEFORE the
scans. Placed after them a lapsed suppression is still honoured by every
scan. Now asserts the step index precedes every scan step.
- the workflow comment claimed "Re-run failed jobs" recovers a blocked
release. It does not: a re-run replays the identical tree, so a lapsed
expiry or new CVE fails again deterministically. Replaced with the real
path — fix the finding, then delete and re-push the tag so the Fulcio SAN
stays on refs/tags/v*.
Also removes a five-line comment duplicated verbatim.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…871) The two release gates honour .grype.yaml, and grype's ignore rules filter the REPORT as well as the exit code — so a suppressed CVE was skipped by the gate and absent from its output. A green release log read "nothing found" while seven documented CVEs shipped in the image. Adds the non-blocking, ignore-free scan the PR path has had since WOR-852. fail-build: false, so it can never block a release; it only makes the release record honest. Flagged independently by all three post-code reviews. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The workflow pushes by digest, signs, THEN promotes human-readable tags. 'signed with cosign before publish' implied no registry artifact exists before signing, which is not the invariant. Flagged by CodeRabbit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…(WOR-871) This PR changed the release gate and CI ran 36 green checks, none of which were a scan. `publish-docker.yml` triggers only on `v*` tags, and nothing else watched it — so the whole change would first execute at a real release. That is the identical gap this workstream already fixed for docker-security.yml, reintroduced one file over. Adds the release workflow to the scan job's paths filter, with an assertion so it cannot regress. Found by an independent close-out review. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-docker.yml:
- Around line 212-220: Add a separate non-blocking Grype scan step for
docker-archive:${{ env.ARM64_TAR }} alongside the existing informational scan,
without GRYPE_CONFIG so it is ignore-free. Configure it with only-fixed: false,
fail-build: false, and output-format: table to report all arm64 findings.
In `@tests/test_grype_ignore_expiry.py`:
- Around line 321-325: Strengthen the release scan test around _scan_step_images
and PUBLISH_WORKFLOW by tracing the docker-archive input to its producing
docker/build-push-action step, then assert that step declares platforms:
linux/arm64. Retain the existing distinct-image and archive-input assertions,
but do not treat them as sufficient architecture coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f67e2552-e818-41c4-80c6-5963955d5932
📒 Files selected for processing (4)
.github/workflows/docker-security.yml.github/workflows/publish-docker.ymldocs/install-docker.mdtests/test_grype_ignore_expiry.py
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/install-docker.md
| - name: Grype full scan (including unfixed — informational) | ||
| uses: anchore/scan-action@e1165082ffb1fe366ebaf02d8526e7c4989ea9d2 # v7.4.0 | ||
| env: | ||
| GRYPE_CONFIG: .grype-informational.yaml | ||
| with: | ||
| image: worthless-proxy:scan-${{ github.sha }} | ||
| only-fixed: false | ||
| fail-build: false | ||
| output-format: table |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Report suppressed arm64 findings.
The informational scan reads only the amd64 image. The arm64 gate reads docker-archive:${{ env.ARM64_TAR }}, honors .grype.yaml, and scans only fixed findings. An arm64-only suppressed or unfixed CVE is absent from all release logs.
Add a non-blocking ignore-free scan for docker-archive:${{ env.ARM64_TAR }} with only-fixed: false and output-format: table.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/publish-docker.yml around lines 212 - 220, Add a separate
non-blocking Grype scan step for docker-archive:${{ env.ARM64_TAR }} alongside
the existing informational scan, without GRYPE_CONFIG so it is ignore-free.
Configure it with only-fixed: false, fail-build: false, and output-format: table
to report all arm64 findings.
… arm64 (WOR-871)
Two findings from CodeRabbit's second review, both arm64, both mine — I
mirrored only half the PR-gate pattern onto the release path.
- the informational scan read only the amd64 image. The arm64 gate reads
the tarball but honours .grype.yaml and only-fixed, so an arm64-only
suppressed or unfixable CVE appeared in NO release output at all. Adds
the matching arm64 informational scan.
- `docker-archive:` in the scan input proves a tarball, not an
architecture. Two amd64 builds with one exported to a tarball satisfied
every assertion while the arm64 image users pull had no gate. The test
now traces the tarball back to the build-push-action step that produced
it and asserts `platforms: linux/arm64`. Verified it fails when that
producer is flipped to amd64.
Also re-dispatches CI: the previous push created only a CodeQL run, leaving
four required checks stuck at "Expected — waiting for status to be reported".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The three CodeQL analyses on a32d2ad were cancelled mid-run by their own concurrency group when commits landed minutes apart. They are GitHub default-setup scans, not a repo workflow, so there is no rerun path — only a new SHA triggers a fresh analysis. They were green on the prior head. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|



TL;DR
Features
Before / After
Summary
The release gate sat at
severity-cutoff: criticalwhile WOR-852 moved the PR gate tomedium, so a fixable High could be published to users while a fixable Medium blocked a branch. Investigating that turned up a worse defect underneath:publish-docker.ymlnever ran the expiry hook, so the release path honoured every suppression in.grype.yamlwith nothing policing whether it had lapsed. WOR-871 closes both, records the real recovery path for a blocked release, and rewrites the install page to describe the real posture.Setup
Two Grype gates guard the container. The PR gate blocks merging a branch. The publish gate blocks shipping to GHCR — the artifact users actually
docker pulland run against their real API keys. The gate on the proposal was two tiers stricter than the gate on the artifact.Underneath that, a quieter problem. Grype's ignore schema has no
expiryfield and drops the key silently, soscripts/hooks/check_grype_ignore_expiry.pyis the only thing making the suppression list time-boxed rather than permanent. It runs at commit time and again indocker-security.yml. It has never run inpublish-docker.yml. Seven suppressions — three High, four Medium — were honoured on the release path with no expiry enforcement at all.And
publish-docker.ymlfires only onv*tag push. A scan that blocks mid-release leaves the tag existing with no image and no signature, and the pull command in the README returns 404. The recovery is deleting a tag or amending.grype.yamlwhile a release is stuck — which is exactly the pressure that turns a reasoned suppression into a rubber stamp.What
Why
criticalearned; a reader should be able to check the reasoning rather than trust an adjectiveHow
severity-cutoffmovescritical→mediumon both the amd64 step and the arm64docker-archive:step in.github/workflows/publish-docker.yml. The expiry hook runs before both, via a pinnedsetup-uvwithenable-cache: false— zizmor flags a cachedsetup-uvin a publishing workflow as a High cache-poisoning finding, correctly: a poisoned cache would reach the artifact that then gets cosign-signed. The step parses one small YAML file, so there is nothing to gain from caching it.A
workflow_dispatchbreak-glass trigger was added in the first commit and removed in the second, after three independent reviews converged on it being a publishing hole. GitHub runs a workflow from the dispatched ref, so a branch dispatch supplies both the Dockerfile and this workflow — a branch could delete the release gate and still publish under the repo's OIDC identity. Worse,type=semveryields no tags on a non-tag ref, so the build, the digest push andcosign signall succeed and only promotion fails: a signed orphan digest whose Fulcio SAN is@refs/heads/..., which the verify command in our own docs rejects. It also bought nothing — GitHub's "Re-run failed jobs" already re-runs a blocked release and preservesrefs/tags/v*. A test now fails if it comes back.The expiry check runs before the builds, so a lapsed date fails in seconds rather than after ~25 minutes of QEMU arm64 build — the principle the cosign preflight above it already states. It pins
pyyaml==6.0.3: that step runs in the job holdingid-token: writeandpackages: write, and every action in this file is SHA-pinned already. Both release scans gainoutput-format: table, because the action defaults to SARIF written to a file nothing uploads — the release gate's findings previously reached no human at all.Three guards land in
tests/test_grype_ignore_expiry.py, written before the fix and verified RED against the old workflow.test_the_release_gate_is_never_looser_than_the_pr_gatecompares the two workflows by severity ordinal rather than pinning a literal, so it keeps holding if either gate tightens later — the drift it catches is the one that actually happened. Pure YAML parsing: no Docker, no vulnerability database, no network.Follow-ups
WOR-872 — the scanned artifact is not provably the pushed artifact (three separate builds, no digest equality asserted). Raised by CodeRabbit and an independent security review; thread left open on this PR deliberately.
Publish the scan evidence as a cosign attestation on the digest so users verify rather than trust a sentence in the docs
WOR-691 — moving off the EOL Debian 12 base is the durable fix for the seven inherited CPython CVEs
Tests
tests/test_grype_ignore_expiry.py→ 22 passedworkflow_dispatchturns its test redmediumon both architectures → exit 0pre-commiton all changed files → passes, includingactionlintandzizmorWhat this does NOT do
only-fixed: truemeans a Critical with no upstream patch still publishes. The new informational scan now names it in the log; it does not block it.publish-docker.ymlfires only onv*tags, so its first genuine exercise is the next release. Local evidence covers grype's behaviour on both architectures, not the workflow end to end.🤖 Generated with Claude Code
Summary by CodeRabbit
Security
Documentation
Reliability