ci: GHCR commit-SHA tag, OCI labels, and build provenance#3528
ci: GHCR commit-SHA tag, OCI labels, and build provenance#3528
Conversation
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
WalkthroughCI workflows and the Dockerfile were updated to add build provenance attestation and richer build metadata. Workflows grant Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/publish-webapp.yml (1)
97-122: ⚡ Quick winVerify the pinned SHA for
actions/attest-build-provenance@v4.1.0; also consider migrating toactions/attestfor new implementations.Two points:
SHA verification — the pin
a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32 # v4.1.0is confirmed to match the actualv4.1.0release tag.Preferred action for new implementations — as of version 4,
actions/attest-build-provenanceis a wrapper on top ofactions/attest, and GitHub explicitly states new implementations should useactions/attestinstead. The current usage is functional and permitted for existing workflows, but a forward-looking migration toactions/attestwould align with GitHub's stated direction.🤖 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-webapp.yml around lines 97 - 122, Confirm the pinned commit for actions/attest-build-provenance (a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32 referenced for v4.1.0) actually matches the git tag/release you intend and update the SHA if it does not; then consider replacing the step that uses actions/attest-build-provenance@a2bbfa25... with actions/attest for new implementations (or add a comment explaining intentional pinning/compatibility) so the workflow aligns with GitHub’s recommendation to use actions/attest going forward.
🤖 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.
Nitpick comments:
In @.github/workflows/publish-webapp.yml:
- Around line 97-122: Confirm the pinned commit for
actions/attest-build-provenance (a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32
referenced for v4.1.0) actually matches the git tag/release you intend and
update the SHA if it does not; then consider replacing the step that uses
actions/attest-build-provenance@a2bbfa25... with actions/attest for new
implementations (or add a comment explaining intentional pinning/compatibility)
so the workflow aligns with GitHub’s recommendation to use actions/attest going
forward.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7a622e48-8d39-49b8-a32f-2f2e1e4908ab
📒 Files selected for processing (2)
.github/workflows/publish-webapp.ymldocker/Dockerfile
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
🔇 Additional comments (3)
docker/Dockerfile (1)
97-110: LGTM — ARG scoping and LABEL syntax are correct.All five
ARGdeclarations are properly scoped to therunnerstage.BUILD_TIMESTAMP_RFC3339is deliberately not promoted toENV(it's only needed at build-time for the label), which is the right call. Docker correctly interpolatesARGvalues insideLABELinstructions. OCI label keys follow the spec..github/workflows/publish-webapp.yml (2)
3-7: LGTM — permission additions are correct.Both
id-token: write(already present) andattestations: write(added) are required for OIDC-based provenance signing and persisting the attestation to the registry.
62-66: LGTM — commit-SHA tag conditional is correct.Appending
${GITHUB_SHA}only when the mutablemaintag is being pushed is the right approach. Semver tags are already immutable by convention and don't need an extra SHA tag.
Per GitHub Actions reusable-workflow semantics, the GITHUB_TOKEN passed to a called workflow is at most the caller's job-level permissions. Without this, actions/attest-build-provenance fails at runtime even though the called workflow declares the scope.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/publish.yml (1)
90-98: 💤 Low valueAdd
attestations: writetopublish-worker-v4permissions if it adopts provenance attestation.
publish-webapp.ymlusesactions/attest-build-provenance@v4.1.0with anattestations: writepermission. Ifpublish-worker-v4.ymlfollows the same pattern in the future, it will requireattestations: writehere or the action will fail silently at runtime.🤖 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.yml around lines 90 - 98, The publish-worker-v4 job currently grants permissions (contents, packages, id-token) but is missing attestations: write; update the publish-worker-v4 job permissions block (job name publish-worker-v4, which uses the publish-worker-v4 workflow via "uses: ./.github/workflows/publish-worker-v4.yml") to include attestations: write alongside the existing permissions so any future use of actions/attest-build-provenance or similar attestation actions will have the required permission.
🤖 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.
Nitpick comments:
In @.github/workflows/publish.yml:
- Around line 90-98: The publish-worker-v4 job currently grants permissions
(contents, packages, id-token) but is missing attestations: write; update the
publish-worker-v4 job permissions block (job name publish-worker-v4, which uses
the publish-worker-v4 workflow via "uses:
./.github/workflows/publish-worker-v4.yml") to include attestations: write
alongside the existing permissions so any future use of
actions/attest-build-provenance or similar attestation actions will have the
required permission.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 10912c18-933f-4bbb-a34e-eec0dc5d04f6
📒 Files selected for processing (1)
.github/workflows/publish.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (32)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: typecheck / typecheck
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: audit
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🔇 Additional comments (1)
.github/workflows/publish.yml (1)
67-76: LGTM —attestations: writecorrectly scoped to thepublish-webappcaller job.Granting
attestations: writeat the caller job level is the right approach for reusable workflow calls: GitHub copies only the permissions explicitly declared in the caller's jobpermissionsblock into theGITHUB_TOKENavailable to the called workflow. Without this addition,actions/attest-build-provenancewould fail at runtime with a 403 even thoughid-token: writewas already present.
| - name: 🪪 Attest build provenance | ||
| uses: actions/attest-build-provenance@a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32 # v4.1.0 | ||
| with: | ||
| subject-name: ghcr.io/triggerdotdev/trigger.dev | ||
| subject-digest: ${{ steps.build_push.outputs.digest }} | ||
| push-to-registry: true |
There was a problem hiding this comment.
📝 Info: Attestation step uses continue-on-error to avoid blocking releases
The continue-on-error: true on the attestation step (.github/workflows/publish-webapp.yml:121) means that Sigstore/GHCR failures won't block the downstream publish-helm job. This is an intentional trade-off: the image is already pushed by this point, so blocking releases on attestation infrastructure issues would be worse than a missing attestation. However, this also means genuine configuration errors (e.g., wrong subject-name, missing permissions) will only show as warnings rather than failures. The comment in the workflow acknowledges this.
Was this helpful? React with 👍 or 👎 to provide feedback.
… path - release.yml's publish-docker job now grants attestations:write so the reusable-workflow chain (release.yml -> publish.yml -> publish-webapp.yml) carries the scope all the way to actions/attest-build-provenance. - continue-on-error on the attestation step itself: image is already pushed by the time this runs, so a Sigstore outage or GHCR referrer hiccup shouldn't fail the workflow and block the downstream publish-helm job. Real config errors still surface as a step warning.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/publish-webapp.yml (1)
122-122: 💤 Low valueConsider migrating to
actions/attest— the upstream-recommended replacement.
actions/attest-build-provenance@v4is a thin wrapper onactions/attest, and the maintainers recommend that new implementations useactions/attestdirectly instead. Since this is a new step, the migration is straightforward — the inputs (subject-name,subject-digest,push-to-registry) are identical.🤖 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-webapp.yml at line 122, Replace the step that currently uses "actions/attest-build-provenance@a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32" with the upstream-recommended "actions/attest" action (e.g., "actions/attest@v4"), keeping the same inputs (subject-name, subject-digest, push-to-registry) and any existing environment/permissions configuration; update the "uses: actions/attest-build-provenance@..." reference to "uses: actions/attest@v4" and verify the inputs and output names remain identical to ensure a drop-in replacement.
🤖 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.
Nitpick comments:
In @.github/workflows/publish-webapp.yml:
- Line 122: Replace the step that currently uses
"actions/attest-build-provenance@a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32" with
the upstream-recommended "actions/attest" action (e.g., "actions/attest@v4"),
keeping the same inputs (subject-name, subject-digest, push-to-registry) and any
existing environment/permissions configuration; update the "uses:
actions/attest-build-provenance@..." reference to "uses: actions/attest@v4" and
verify the inputs and output names remain identical to ensure a drop-in
replacement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: dcbd204e-7397-4cd1-afae-49aa4d7c2ad2
📒 Files selected for processing (2)
.github/workflows/publish-webapp.yml.github/workflows/release.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: audit
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🔇 Additional comments (6)
.github/workflows/release.yml (1)
168-172: LGTM — necessary permission addition for the reusable-workflow attestation chain.
attestations: writeis required at every caller level forGITHUB_TOKENpermissions to propagate throughrelease.yml → publish.yml → publish-webapp.ymlto theactions/attest-build-provenancestep..github/workflows/publish-webapp.yml (5)
3-7: LGTM — correct permission addition at the workflow level.
62-66: LGTM — commit-SHA tag is correctly constructed and scoped.
73-88: LGTM — RFC 3339 timestamp format is correct for the OCIcreatedlabel.
117-126: ⚡ Quick winThe pinned SHA
a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32correctly corresponds to thev4.1.0tag.
97-115: ⚡ Quick winThe concern about
depot/build-push-actionnot exposing adigestoutput is unfounded. The action (v1.17.0) does expose adigestoutput as a String type, matching the interface ofdocker/build-push-action. The attestation step will receive a valid digest and function correctly.> Likely an incorrect or invalid review comment.
3 participants
mainpushes (ghcr.io/triggerdotdev/trigger.dev:<sha>) so any commit can be resolved to a digest easily.source,revision,version,created) sodocker inspect, vulnerability scanners, andregistry browsers see source/commit/version directly.
actions/attest-build-provenance@v4.1.0(pinned by SHA), enablinggh attestation verify oci://...against the source commit and workflow.