ci: rebuild release workflow on upstream matrix + full hardening#6
ci: rebuild release workflow on upstream matrix + full hardening#6Fieldnote-Echo wants to merge 2 commits into
Conversation
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
PR Summary by QodoCI: rebuild hardened multi-arch release workflow with gated scanning and attestations
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![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: 1924270969
ℹ️ 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".
| if [[ "$EVENT_NAME" == "workflow_dispatch" ]]; then | ||
| echo "source_ref=${DISPATCH_BRANCH}" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "source_ref=${PUSH_REF_NAME}" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
For release-branch pushes, source_ref is the mutable branch name rather than the triggering SHA. If another commit reaches release during this long matrix workflow, prepare, individual build legs, and target_commitish can resolve different branch tips, publishing code from a later commit under the earlier run's version; the OCI revision remains github.sha, making the metadata false as well. Resolve and propagate the checked-out commit SHA instead.
Useful? React with 👍 / 👎.
| BUILDKIT_STEP_LOG_MAX_SIZE: 10485760 | ||
| VERSION: ${{ needs.prepare-release.outputs.version_tag }} | ||
| REGISTRY: ${{ vars.REGISTRY || 'ghcr.io' }} | ||
| OWNER: ${{ vars.OWNER || 'remsky' }} |
There was a problem hiding this comment.
When the repository has no OWNER Actions variable, this fallback sends every bake push to ghcr.io/remsky, even though this Fieldnote-Echo fork and the previous workflow publish under ghcr.io/fieldnote-echo. The fork's GITHUB_TOKEN is normally not authorized to write packages in the upstream owner's namespace, so the first --push fails and no release is produced; default to the current repository owner or this fork's namespace.
Useful? React with 👍 / 👎.
| DOCKER_BUILDKIT: 1 | ||
| BUILDKIT_STEP_LOG_MAX_SIZE: 10485760 | ||
| VERSION: ${{ needs.prepare-release.outputs.version_tag }} | ||
| REGISTRY: ${{ vars.REGISTRY || 'ghcr.io' }} |
There was a problem hiding this comment.
When vars.REGISTRY is set to anything other than ghcr.io, bake and the manifest jobs target that registry, but every login remains hard-coded to GHCR and the blocking egress allowlists omit the configured host. Such a configured run therefore fails during push or manifest creation; either consistently authenticate and allow REGISTRY, or remove the nonfunctional override.
Useful? React with 👍 / 👎.
| fail-fast: false | ||
| matrix: | ||
| include: | ||
| - build_target: "cpu" |
There was a problem hiding this comment.
Adding the CPU and ROCm legs publishes targets whose base references remain mutable: docker/cpu/Dockerfile.optimized uses python:3.10, while docker/rocm/Dockerfile uses rocm/dev-ubuntu-24.04:6.4.4-complete; docker-bake.hcl only supplies digest overrides for the CUDA targets. A registry retag can therefore change these release artifacts without any source change, defeating the workflow's stated base-image pinning and reproducibility guarantees; add digest-pinned overrides before publishing these legs.
Useful? React with 👍 / 👎.
| Curated summary: [CHANGELOG.md](https://github.com/${{ github.repository }}/blob/${{ needs.prepare-release.outputs.version_tag }}/CHANGELOG.md) | ||
| generate_release_notes: true | ||
| draft: false | ||
| prerelease: false |
There was a problem hiding this comment.
When VERSION has a prerelease suffix, or a manual feature-branch build produces the workflow's branch-suffixed version, this explicit false still publishes the resulting GitHub Release as stable. It can consequently become the user-facing latest release even though promote-latest deliberately leaves the container :latest tags on the previous stable build; derive this flag from the version/source or skip GitHub Release creation for branch builds.
Useful? React with 👍 / 👎.
| @@ -166,59 +347,360 @@ jobs: | |||
| uses: github/codeql-action/upload-sarif@9e907b5e64f6b83e7804b09294d44122997950d6 # v4.32.3 | |||
There was a problem hiding this comment.
When the Code Scanning upload is rejected or temporarily unavailable, this step fails normally despite the preceding comment describing reporting as non-blocking. Because the gated Build and push step appears later and has the implicit success() condition, that reporting failure skips the push even when the vulnerability gate passed; make the SARIF report/upload explicitly continue-on-error or condition the push on the gate result instead.
Useful? React with 👍 / 👎.
Supersedes the single-job amd64-GPU-only release.yml (from harden/release- gating) with upstream's multi-arch pipeline, hardening layered on so we regain breadth without losing security. Best of both. Structure (upstream, 5 jobs): prepare-release -> build-images (6-leg matrix: cpu/gpu/gpu-cu128/rocm x amd64+arm64, native arm64 runners) -> create-manifests (version tags + index attest) -> promote-latest (rolling :latest, gated on ALL packages) -> create-release. Hardening (ours, per job): every action SHA-pinned; harden-runner egress:block first in every job; gated Trivy scan per leg BEFORE push (fixable CRITICAL, ignore-unfixed) + non-blocking CRITICAL,HIGH SARIF; Sigstore provenance + SBOM attested on the published multi-arch index; persist-credentials:false; least-privilege per-job permissions. Hardened + red-teamed via 4 adversarial lenses; all critical/major findings integrated: - shell-injection: inputs.branch_name (attacker-controllable) now flows through env vars, never spliced into run: text; OCI-tag allowlist sanitization - egress: added the build-time hosts block mode would have failed closed on (astral.sh/uv, apt mirrors on :80, rustup, archlinux rocblas pin, UniDic) - torn-release: split :latest into a promote-latest job gated on all packages; documented Trivy break-glass (refresh base digests or dated .trivyignore) - determinism: single CREATED timestamp shared across scan/push/annotations actionlint clean; 19 actions SHA-pinned; all 6 bake targets verified against docker-bake.hcl. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex/Qodo findings on PR #6: - OWNER default was upstream's 'remsky'; on this fork the token can't push to ghcr.io/remsky, so the first release would fail. Default to 'fieldnote-echo' (matches the prior single-job workflow). [Codex P1 + Qodo] - source_ref was a mutable branch name used for checkouts, OCI revision, and the release target_commitish — a branch advancing mid-run could build/attest/ tag different commits. Resolve the commit SHA once in prepare-release (source_sha output) and use it everywhere. Matters here because we attest provenance. [Codex P1 + Qodo, independently] - CPU (python:3.10 / -slim) and ROCm base images were unpinned while the workflow claims reproducible pinning; now that we publish those legs, pin them by digest. ARG-ified the Dockerfiles (tag defaults keep plain builds working) + digest args in docker-bake.hcl. [Codex P1] - SARIF report/upload could fail and skip the gated-clean push (implicit success() on the later push step). Marked both continue-on-error. [Codex P2] - prerelease was hardcoded false; a suffixed/branch version would publish as the user-facing stable release. Derive it from the version_tag suffix. [Codex P2] - Dropped the unauthenticated 'git fetch --tags' (checkout fetch-depth:0 already fetched tags; persist-credentials:false made the fetch anonymous). [Qodo] actionlint clean; all actions SHA-pinned; bake config parses with the new pins.
687a13b to
0bc6b5f
Compare
1924270 to
65867d6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65867d6faf
ℹ️ 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".
| SAFE_BRANCH=$(echo "$BRANCH_NAME" | tr '/' '-' | tr '[:upper:]' '[:lower:]' | tr -cd 'a-z0-9._-') | ||
| echo "version=${VERSION_PLAIN}-${SAFE_BRANCH}" >> "$GITHUB_OUTPUT" | ||
| echo "version_tag=v${VERSION_PLAIN}-${SAFE_BRANCH}" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
When two manual builds use distinct valid refs that normalize identically—such as feature/foo, feature-foo, or case-only variants—this transform gives them the same version tags while the concurrency group at line 64 still uses the raw branch names, so both runs may proceed simultaneously. Their per-architecture pushes can overwrite one another and produce a manifest assembled from different commits while its revision annotation names only one commit; append a commit-derived identifier or otherwise make the normalized value unique.
Useful? React with 👍 / 👎.
| SAFE_BRANCH=$(echo "$BRANCH_NAME" | tr '/' '-' | tr '[:upper:]' '[:lower:]' | tr -cd 'a-z0-9._-') | ||
| echo "version=${VERSION_PLAIN}-${SAFE_BRANCH}" >> "$GITHUB_OUTPUT" | ||
| echo "version_tag=v${VERSION_PLAIN}-${SAFE_BRANCH}" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
For a manually dispatched branch longer than the remaining space after v${VERSION_PLAIN}-, this unbounded suffix produces a tag exceeding the 128-character Docker/OCI tag limit. Git accepts substantially longer branch names, so the workflow can complete preparation and expensive builds only to fail when bake parses or pushes the generated tag; truncate the suffix while retaining a collision-resistant hash.
Useful? React with 👍 / 👎.
1 participant
Stacked on #5 (base
harden/release-gating). Rebuilds the release workflow on upstream's multi-arch matrix with our hardening layered on — the "best of both" end-state from the review: we get upstream's breadth back and keep the supply-chain security upstream lacks.Why
Our single-job workflow built one artifact (amd64 GPU); upstream's builds six (cpu/gpu/gpu-cu128/rocm × amd64+arm64) via a clean 4-job matrix with native arm64 runners and multi-arch manifest assembly. On the capability axis ours was strictly worse. This takes upstream's structure and adds what upstream is missing.
Structure (5 jobs)
prepare-release→build-images(6-leg matrix) →create-manifests(immutable:VERSIONfamily + attest) →promote-latest(rolling:latest*, gated on all packages) →create-release. Thepromote-latestsplit is new — it prevents:latestever pointing at a half-published set.Hardening (per job)
Every action SHA-pinned · harden-runner
egress-policy: blockfirst in every job · gated Trivy scan per leg before push (fixable CRITICAL,ignore-unfixed) + non-blocking CRITICAL,HIGH SARIF · Sigstore provenance + SBOM attested on the published multi-arch index ·persist-credentials: false· least-privilege permissions.Red-teamed (4 adversarial lenses; all critical/major fixed)
inputs.branch_nameis attacker-controllable viaworkflow_dispatch; it (and all context values) now flow throughenv:and are referenced as shell vars — never spliced intorun:text. Plus OCI-tag allowlist sanitization on the branch-derived tag.blockmode would have killed every build — the Docker builds fetch uv (astral.sh), OS packages over :80 apt mirrors, Rust (rustup), an Arch rocBLAS pin, and UniDic. All added to the per-job allowlists.:latestinto the gatedpromote-latestjob; documented a Trivy break-glass (refresh base digests, or a dated.trivyignore) for when the vuln DB flags a fixable CRITICAL weeks after a clean release.CREATEDtimestamp shared across scan/push/annotations so the scanned and pushed images are bit-identical.Verification
actionlintclean · 19 actions SHA-pinned (0 floating) · all 6 matrix legs map to realdocker-bake.hcltargets · harden-runner first in all 5 jobs.egress-policy: blockfails closed. The allowlists cover all known egress, but the first hardened run may hit an unlisted CDN/mirror shard (likeliest:raw.githubusercontent.comfor syft increate-manifests) — harden-runner names the blocked host in the log; add it and re-run. This PR is the review gate before any of it takes effect.Residual (documented, non-blocking): SBOM is amd64-representative for multi-arch packages (provenance is digest-exact); prerelease
VERSIONs with a-skip:latestpromotion (inherited from upstream's version logic); Trivy-DB drift can gate a previously-clean release (break-glass above).🤖 Generated with Claude Code