Skip to content

fix(validate-go): route govulncheck through the SHA-pinned fork for all repos#278

Closed
devantler wants to merge 1 commit into
mainfrom
claude/ci-govulncheck-fork-only
Closed

fix(validate-go): route govulncheck through the SHA-pinned fork for all repos#278
devantler wants to merge 1 commit into
mainfrom
claude/ci-govulncheck-fork-only

Conversation

@devantler
Copy link
Copy Markdown
Contributor

🤖 Generated by the Daily AI Assistant

Problem — the validate-go-project Vulnerability Scan wedges SHA-pin-enforcing consumers

ksail's entire Go PR lane (#4986, dependabot #4945/#4905, and every future Go PR) has been failing the required 🛡️ Vulnerability Scan check. The failure is not a govulncheck finding — the job dies at workflow-prep:

##[error]The actions actions/checkout@v4.1.1 and actions/setup-go@v5.0.0 are not allowed
in devantler-tech/ksail because all actions must be pinned to a full-length commit SHA.

Root cause: the upstream golang/govulncheck-action@v1.0.4 composite tag-pins its own actions/checkout@v4.1.1 and actions/setup-go@v5.0.0. GitHub resolves the actions of every uses: in a job regardless of its if:, so the current dual-path design (official action on the no-allowlist branch, fork on the allowlist branch) resolves both — and the upstream action's transitive tag refs are rejected at prep time by any consumer that enforces a "pin every action to a full-length commit SHA" repository ruleset (ksail does). The job never starts; the lane wedges. main stays green because govulncheck is pull_request-only, hiding it from the default-branch view.

This is a regression introduced when we adopted the upstream action (the previous in-house JSON-mode scan had no nested-action dependency).

Fix

Route all repos through the thin devantler/govulncheck-action fork (@feat/allow-file), which SHA-pins its sub-actions (actions/checkout@…v6.0.2, actions/setup-go@…v6.2.0). The upstream action is no longer referenced anywhere in the job, so no tag-pinned transitive deps reach the consumer's ruleset.

Behaviour is unchanged — the fork is a strict superset of upstream:

  • no .govulncheck-allow.txtallow-file input is empty → fork runs govulncheck strict, byte-equivalent to the upstream action (any reachable advisory fails).
  • .govulncheck-allow.txt present → JSON-mode allowlist that risk-accepts only the listed advisory IDs (the existing opt-in path, unchanged).

Collapses the two if-gated scan steps into one (the fork branches internally on allow-file). Net −23/+23 across the workflow + a README line.

Trade-off to weigh (⚠️ reverses part of #276's design)

#276 deliberately kept the upstream action for no-allowlist repos to minimise the fork's blast radius. That goal is now in direct conflict with the org's own SHA-pinned-actions security policy: as shown above, the upstream action is fundamentally incompatible with that ruleset, even on the unused branch. Routing everyone through the fork widens the fork's blast radius but is required for SHA-pin consumers and is strictly more secure (SHA-pinned sub-actions). The escape hatch is documented inline: swap back to golang/govulncheck-action once it ships the allow-file feature and SHA-pins its sub-actions.

Validation

  • actionlint clean (only the pre-existing code-quality-scope false-positive, unrelated).
  • Behaviour parity reasoned from the fork's action.yml: empty allow-filegovulncheck -format text ./... (same as upstream); non-empty ⇒ JSON-mode allowlist.
  • Once merged + released, ksail (pinned validate-go-project.yaml@v5.4.x) needs a ref bump to the new release to pick this up — its Go PR lane then unwedges.

Draft for your promotion — the trade-off above is your call.

…ll repos

The upstream golang/govulncheck-action's composite action.yml tag-pins its own
actions/checkout@v4.1.1 and actions/setup-go@v5.0.0. GitHub resolves the actions
of every `uses:` in a job regardless of its `if:`, so merely *referencing* the
upstream action in the no-allowlist branch failed the whole Vulnerability Scan
job at workflow-prep time on any consumer that enforces a "pin every action to a
full-length commit SHA" ruleset (e.g. ksail) — wedging that consumer's entire Go
PR lane before any scan ran.

Route all repos through the devantler/govulncheck-action fork (which SHA-pins its
sub-actions). Behaviour is unchanged: with no .govulncheck-allow.txt the allow-file
input is empty and the fork runs strict (byte-equivalent to upstream); an allowlist
file takes the JSON-mode risk-acceptance path. Collapses the two if-gated scan
steps into one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 2, 2026 20:28
@github-project-automation github-project-automation Bot moved this to 🫴 Ready in 🌊 Project Board Jun 2, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the reusable Validate Go Project workflow to ensure the govulncheck vulnerability scan works in repositories that enforce “pin every action to a full-length commit SHA” rulesets by removing all references to the upstream golang/govulncheck-action and routing all consumers through the SHA-pinned devantler/govulncheck-action fork.

Changes:

  • Collapses the dual govulncheck steps (upstream vs fork) into a single fork-based step, with conditional allowlist behavior via the allow-file input.
  • Updates workflow inline documentation to explain why the upstream action cannot be referenced (transitive tag-pins + workflow-prep resolution).
  • Updates the README feature description to match the new “always fork” approach and rationale.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
README.md Updates Validate Go Project documentation to reflect using the SHA-pinned fork for govulncheck in all repos.
.github/workflows/validate-go-project.yaml Removes upstream govulncheck action reference and uses the fork unconditionally, with conditional allowlist selection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 381 to +385
repo-checkout: false
go-version-file: go.mod
allow-file: .govulncheck-allow.txt
# Empty ⇒ strict (fail on any reachable advisory, like the upstream action);
# set ⇒ JSON-mode allowlist that risk-accepts only the listed advisory IDs.
allow-file: ${{ steps.govulncheck-allow.outputs.present == 'true' && '.govulncheck-allow.txt' || '' }}
@devantler devantler marked this pull request as ready for review June 2, 2026 21:04
@devantler
Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Assistant

Superseded by #279, which carries the identical fix (route govulncheck through the SHA-pinned devantler/govulncheck-action fork on all paths) in a cleaner form — the two conditional steps collapse into one, and #279 is mergeStateStatus: CLEAN with no unresolved review threads (this PR is BLOCKED on a copilot-pull-request-reviewer thread). Closing this duplicate to avoid stacking two PRs on the same concern; the fix lands via #279.

@devantler devantler closed this Jun 2, 2026
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants