Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,15 @@ jobs:
go-version: '1.26.x'
- name: install gosec
run: go install github.com/securego/gosec/v2/cmd/gosec@latest
# Tracks: golang.org/x/vuln/cmd/govulncheck@v1.1.4
# Pinned to an explicit version so a compromised or buggy upstream
# release cannot silently land in CI between Renovate bumps. The
# tracked tag in this comment must move in lockstep with the
# `@vX.Y.Z` below — same convention as the Trivy action pin in the
# `image-scan` job (#68) and the Dockerfile base-image digest pins
# (#32).
- name: install govulncheck
run: go install golang.org/x/vuln/cmd/govulncheck@latest
run: go install golang.org/x/vuln/cmd/govulncheck@v1.1.4
- name: gosec
run: gosec ./...
- name: govulncheck
Expand Down
51 changes: 51 additions & 0 deletions docs/knowledge/codebase/41.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Ticket #41 — pin `govulncheck` install version in `ci.yml`

Closes the only remaining gap in the relay's CI vuln-scan posture: the `install govulncheck` step in the `security` job was still pulling `@latest`, the same supply-chain anti-pattern the tool itself is meant to guard against. A compromised or buggy upstream release would silently land in CI on the next workflow run with no code change and no reviewer signal. Replaced with an explicit semver tag, mirroring the pinning convention #68 established for the Trivy action and #32 established for Dockerfile base digests.

## Implementation

- **`.github/workflows/ci.yml`** — single-line change on the `go install` invocation of `govulncheck`: `@latest` → `@v1.1.4`. Added a six-line comment block above the step naming the tracked tag and the rationale, so a Renovate bump's diff is reviewable against the comment in lockstep.
- **Why a semver tag, not a commit SHA.** GitHub Action `uses:` references can be tag-swapped by a malicious maintainer (which is why #68's Trivy pin uses a SHA). The Go module ecosystem has a stronger guarantee: `proxy.golang.org` content-addresses every published version and `sum.golang.org` rejects any tag-swap once observed. The workflow does not override `GOPROXY` or disable `GOSUMDB`, so `@v1.1.4` is materially immutable for `go install`. Semver also reads cleaner in diffs and Renovate tracks it well.
- **`gosec@latest` left untouched.** Explicitly out of scope per the ticket body's *Technical Notes* — one concern per ticket, separate follow-up if/when wanted.
- **No `tools.go` introduced.** The Go-idiomatic alternative (pin via a `//go:build tools` file in `go.mod`) was considered and deferred in the architect spec: structurally larger than the AC allows, no existing `tools.go` pattern in the repo, and two inline pins are below the threshold where the abstraction pays for itself. Surface as a follow-up when a third tool needs pinning.
- **No Go source or Makefile changes.** `make lint`'s local `govulncheck` invocation is intentionally separate — local-side pinning has different ergonomics and is not what AC #1 names.

## Patterns established

- **Pin every `go install` invocation in CI to an explicit semver tag, with a `# Tracks: <module>@<version>` comment alongside.** Same two-line readability shape as the Dockerfile base digests (#32) and the Trivy action pin (#68). The comment is the structural smell-detector: a PR that bumps the `@vX.Y.Z` without moving the comment is inconsistent on its face and obvious at code review. The comment is required, not optional.
- **`@latest` is forbidden in CI workflow `run:` lines.** Forbidden by convention (advisory) for now — no programmatic enforcement. A CI lint that rejects `@latest` in `.github/workflows/*.yml` would be the deterministic backstop and is a reasonable follow-up if a regression ever lands; not built here under the evidence-based-fix-selection rule.
- **Same pinning posture, three different mechanisms.** Dockerfile bases pin by `@sha256:` digest (registry tags are mutable). GitHub Actions pin by commit SHA (action tags are mutable). `go install` pins by semver tag (Go module proxy + checksum DB make tags effectively immutable). The shared invariant — "no moving references in CI" — is what matters; the mechanism follows the ecosystem's strongest guarantee.

## Verification

AC-defined verification is self-evident on the PR's own CI run:

1. **Pinned version exists and is fetchable.** The `security` job's `install govulncheck` step succeeds — `go install golang.org/x/vuln/cmd/govulncheck@v1.1.4` resolves through `proxy.golang.org`, checksum-verifies against `sum.golang.org`, and installs.
2. **`govulncheck ./...` still passes against current `go.mod`.** The downstream `govulncheck` step runs to completion with no reachable CVEs. Confirms the pinned version is functionally compatible with the relay's current source tree.

## Failure modes (now load-bearing in CI)

- **Pinned version removed from `proxy.golang.org`.** `go install` fails loudly, `security` job goes red. Desired behaviour — a removed upstream release should block merges until a human bumps the pin, not silently degrade.
- **Checksum-DB mismatch (theoretical tag swap).** `go install` aborts with `verifying module: checksum mismatch`. Same loud red; same correct response.
- **Renovate bumps the pin.** Surfaces as an ordinary PR; reviewer sanity-checks the `# Tracks:` comment matches the new `@vX.Y.Z` and that the upstream changelog is benign.

## Out of scope (delegated)

- **`gosec@latest`.** Same anti-pattern one line above; separate ticket if wanted.
- **Periodic re-scan-on-cron.** Lives in #69.
- **Image-side CVE scanning.** Lives in #68 (already shipped).
- **A CI lint forbidding `@latest` in workflow files.** Programmatic enforcer for this convention; reasonable follow-up, not warranted yet.
- **`tools.go` / `go.mod` pin pattern.** Considered, deferred — see *Implementation* above.

## Lessons learned

- **The supply-chain scanner is itself part of the supply chain.** The same logic that pins the relay's runtime dependencies has to apply to the tools auditing them. `@latest` on a `go install` line for `govulncheck` is the exact failure shape `govulncheck` is meant to detect for the relay's `go.mod` — the inconsistency was the bug. Carry forward: any tool whose job is to gate supply-chain risk must itself be pinned by an immutable reference, never `@latest` / `@main` / `@HEAD`.
- **Pick the strongest immutability the ecosystem offers, not a universal mechanism.** It is tempting to "pin everything by commit SHA" for symmetry. But the right pin is the one the ecosystem makes hardest to forge: digest for OCI registries, commit SHA for GitHub Actions, semver tag for Go modules (because `sum.golang.org` is doing the work a SHA would do elsewhere). Forcing SHA pinning on Go installs would lose readability without adding security.
- **The `# Tracks:` comment is doing real work.** Without it, a Renovate bump (or worse, a hand-rolled PR) that swaps the `@vX.Y.Z` to a maliciously-rebased pseudo-version is a one-token diff with no co-signal for the reviewer. With it, an inconsistent `(comment, run line)` pair is the smell. Same shape we adopted for Dockerfile digests and the Trivy action SHA — this is now the house convention for any pinned third-party reference in CI / build infra.

## Cross-links

- [Architect spec](../../specs/architecture/41-govulncheck-pin-version.md) — full design rationale, alternatives considered, security review.
- [Ticket #68 codebase notes](68.md) — PR-time Trivy image CVE scan; established the pinned-third-party + `# Tracks:` pattern this ticket reuses on the Go-install side.
- [Ticket #32 codebase notes](32.md) — Dockerfile base digest pins; root of the convention this ticket extends.
- [Threat model](../../threat-model.md) — § *Supply chain — Go dependencies*; this ticket tightens the `govulncheck`-as-merge-gate control without changing the residual-risk story (a malicious release by an authentic upstream maintainer remains the named gap; pinning narrows the window but does not close it — Renovate-time review does).
Loading