chore(ci): add pinact pre-commit hook to enforce SHA-pinned actions - #2509
Conversation
Site previewPreview: https://fc1a979b-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 2:56 PM UTC · Completed 3:08 PM UTC |
ReviewFindingsHigh
Previous runReviewFindingsHigh
Medium
Labels: PR enforces SHA-pinning of GitHub Actions as a supply chain security hardening measure. Previous run (2)ReviewFindingsHigh
Medium
Previous run (3)ReviewFindingsHigh
Medium
Previous run (4)ReviewFindingsHigh
Medium
Previous runReviewFindingsHigh
Medium
Labels: PR modifies CI workflow and pre-commit configuration for SHA-pinning enforcement. Previous run (5)ReviewFindingsHigh
Medium
Low
Previous run (6)ReviewFindingsHigh
Medium
Low
Labels: PR modifies CI workflow and pre-commit configuration for SHA-pinning enforcement. |
| - id: shellcheck | ||
| args: ["-x", "-e", "SC1091,SC2001,SC2016"] | ||
|
|
||
| - repo: local |
There was a problem hiding this comment.
[medium] incomplete-implementation
The pinact hook enforces SHA-pinned actions, but the repository is not yet SHA-pinned. Only sandbox-images.yml uses SHA-pinned actions; the other 15 workflow files contain 56 semver-pinned action references. Enabling this hook without first SHA-pinning will cause immediate pre-commit failures.
Suggested fix: Ensure PR #2508 (SHA-pinning the actions) merges before this PR, or adjust sequencing so the enforcement hook is added only after the repository is compliant.
| - id: shellcheck | ||
| args: ["-x", "-e", "SC1091,SC2001,SC2016"] | ||
|
|
||
| - repo: local |
There was a problem hiding this comment.
[low] hook-organization
The pinact hook is placed in a new repo: local block rather than being added to the existing local hooks block. The codebase already has multiple repo: local blocks, so this is a minor inconsistency.
| hooks: | ||
| - id: pinact | ||
| name: pinact (SHA-pin check) | ||
| entry: pinact run --fix=false --no-api |
There was a problem hiding this comment.
[low] scope-clarity
The pinact hook file pattern matches both .github/workflows/ and internal/scaffold/fullsend-repo/.github/workflows/, but the scaffold workflows currently have 0 SHA-pinned actions. The PR description does not mention the scaffold scope.
| echo "1f4e0ef7f6554a6ed33dd7ac144fb2e1bbed98598e7af973042fc5cd43951c9a /tmp/lychee.tar.gz" | sha256sum -c | ||
| tar xzf /tmp/lychee.tar.gz -C /usr/local/bin --strip-components=1 lychee-x86_64-unknown-linux-gnu/lychee | ||
|
|
||
| - name: Install pinact |
There was a problem hiding this comment.
[low] installation-consistency
The pinact tar command differs from the lychee installation pattern (--strip-components=1). This is correct behavior since the tarballs have different internal structures.
9aa8f24 to
0b6dea8
Compare
f64eecd to
4fef525
Compare
PR Summary by QodoCI: add pinact pre-commit hook to enforce SHA-pinned GitHub Actions Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1. Pinact not installed everywhere
|
| - id: pinact | ||
| name: pinact (SHA-pin check) | ||
| entry: pinact run --fix=false --no-api | ||
| language: system | ||
| files: | |
There was a problem hiding this comment.
1. Pinact not installed everywhere 🐞 Bug ☼ Reliability
The new pre-commit pinact hook is configured with language: system, so any environment running pre-commit must have the pinact binary on PATH, but the repo’s bootstrap and agent post-scripts that invoke pre-commit don’t install it. This can break make lint-all (after make bootstrap) and the post-code/post-fix authoritative pre-commit checks with Executable "pinact" not found.
Agent Prompt
## Issue description
A new pre-commit hook runs `pinact` as a system-installed binary (`language: system`). CI installs pinact in `.github/workflows/lint.yml`, but other common execution paths that run `pre-commit` (developer bootstrap + agent post-scripts) do not install pinact, causing those paths to fail with a missing executable.
## Issue Context
- `make bootstrap` claims to install the tools needed for pre-commit workflows.
- Agent runner post-scripts (`post-code.sh`, `post-fix.sh`) run an “authoritative pre-commit” on changed files and currently install other required system tools (e.g., lychee, uv) but not pinact.
## Fix Focus Areas
- Add a pinned, checksum-verified pinact install to the developer bootstrap flow:
- Makefile[37-77]
- Add a pinned, checksum-verified pinact install (arch-aware like lychee) before running `pre-commit` in agent runner scripts:
- internal/scaffold/fullsend-repo/scripts/post-code.sh[268-336]
- internal/scaffold/fullsend-repo/scripts/post-fix.sh[183-245]
(Optionally, if you want pinact available in sandboxes without network, bake it into the relevant image Containerfile(s) too.)
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
🤖 Finished Review · ✅ Success · Started 11:04 PM UTC · Completed 11:13 PM UTC |
| echo "1f4e0ef7f6554a6ed33dd7ac144fb2e1bbed98598e7af973042fc5cd43951c9a /tmp/lychee.tar.gz" | sha256sum -c | ||
| tar xzf /tmp/lychee.tar.gz -C /usr/local/bin --strip-components=1 lychee-x86_64-unknown-linux-gnu/lychee | ||
|
|
||
| - name: Install pinact |
There was a problem hiding this comment.
[low] installation-consistency
The pinact tar command omits --strip-components unlike the lychee installation. This is correct behavior since the upstream tarballs have different internal structures.
4fef525 to
693ed6a
Compare
|
🤖 Finished Review · ✅ Success · Started 6:31 PM UTC · Completed 6:43 PM UTC |
|
🤖 Review · ❌ Terminated · Started 6:59 PM UTC · Ended 7:10 PM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| - repo: local | ||
| hooks: | ||
| - id: pinact | ||
| name: pinact (SHA-pin check) |
There was a problem hiding this comment.
[medium] logic error / coverage gap
The pinact pre-commit hook files regex only matches .github/workflows/ and internal/scaffold/fullsend-repo/.github/workflows/, but .pinact.yaml also configures pinact to scan .github/actions/*/action.yml. There are four composite action directories under .github/actions/. Edits to those files will not trigger the pre-commit hook, so unpinned action references could be introduced without the hook catching them.
Suggested fix: Add |.github/actions/ to the files regex in the pinact pre-commit hook definition so it matches all paths that .pinact.yaml covers.
|
🤖 Finished Review · ✅ Success · Started 6:59 PM UTC · Completed 7:10 PM UTC |
Add a pre-commit hook that runs `pinact run --fix=false --no-api` to verify all GitHub Actions references use full-length commit SHAs. The --no-api flag ensures the check is offline-only (syntactic SHA presence) so it won't break when new action versions are released. Also install pinact in the CI lint workflow so the hook passes there. Depends-on: #1055 (auto-detect pre-commit tool dependencies) Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
The new pinact pre-commit hook correctly caught these unpinned actions. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
a62ae64 to
5aefa4f
Compare
|
🤖 Finished Review · ✅ Success · Started 10:38 AM UTC · Completed 10:49 AM UTC |
| - repo: local | ||
| hooks: | ||
| - id: pinact | ||
| name: pinact (SHA-pin check) |
There was a problem hiding this comment.
[medium] logic error / coverage gap
The pinact pre-commit hook files regex only matches .github/workflows/ and internal/scaffold/fullsend-repo/.github/workflows/, but .pinact.yaml also configures pinact to scan .github/actions/*/action.yml. At least one composite action (.github/actions/setup-gcp/action.yml) contains an external uses: reference. Edits to composite actions under .github/actions/ will not trigger the pre-commit hook, so unpinned action references could be introduced without the hook catching them.
Suggested fix: Add |.github/actions/ to the files regex in the pinact pre-commit hook definition so it matches all paths that .pinact.yaml covers.
Add pinact installation to `make bootstrap` so local devs get the SHA-pin checker without manual setup. Expand the pre-commit hook's files regex to also match `.github/actions/`, aligning it with the paths configured in `.pinact.yaml`. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 11:07 AM UTC · Completed 11:18 AM UTC |
|
🤖 Finished Retro · ✅ Success · Started 11:26 AM UTC · Completed 11:35 AM UTC |
Retro: PR #2509 —
|
…commit-hook chore(ci): add pinact pre-commit hook to enforce SHA-pinned actions
Summary
pinact run --fix=false --no-apito verify all GitHub Actions references use full-length commit SHAsThe
--no-apiflag ensures the check is purely syntactic (40-char SHA presence) — it won't break when new action versions are released upstream.Dependencies
Test plan
pre-commit run pinact --all-filespasses locally🤖 Generated with Claude Code