Skip to content

ci: single-source the gosec version in .gosec-version - #1227

Draft
IAmJSD wants to merge 3 commits into
LeanerCloud:mainfrom
IAmJSD:fix/issue-347
Draft

IAmJSD wants to merge 3 commits into
LeanerCloud:mainfrom
IAmJSD:fix/issue-347

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Root cause

The gosec version was written out in several places that each had to be bumped by hand. On the base this PR started from, those were ci.yml (go install ...gosec@v2.28.0), pre-commit.yml (an install step and a cache key), the Makefile (GOSEC_VERSION?=v2.28.0) and scripts/gosec-hook.sh (GOSEC_VERSION="2.28.0"). Comments in pre-commit.yml, .pre-commit-config.yaml and the hook said it again. #1145 (merged since) moved the CI, hook and make security-scan-go installs into scripts/run-gosec.sh, which still kept its own GOSEC_VERSION="v2.28.0" next to the Makefile's install-dev-tools pin and pre-commit.yml's. All of them agree on v2.28.0 today, but a bump that misses one puts local scans and the CI scan that uploads to GitHub Security on different gosec versions. Nothing checked that they agree.

The issue also lists stale comments and a dead install step. Both still exist on current main.

Fix

The version stays at v2.28.0. All pins already agreed, so this PR doesn't bump it.

  • .gosec-version (new, one line: v2.28.0) is now the only place the version is written. It follows the go-version-file: go.mod idea, but as a plain file because gosec isn't a module dependency. It is read by:

    • scripts/run-gosec.sh (from ci(gosec): run one rule set from a shared script in the hook, Makefile and CI #1145), which the ci.yml gosec step, the pre-commit hook, make security-scan-go and scripts/security-scan.sh all call. It reads and validates the file instead of its GOSEC_VERSION literal. A missing file, or one that isn't exactly vMAJOR.MINOR.PATCH, exits 2 before go runs or the cache is touched. The cache path (~/.cache/gosec/v2.28.0-<go version>/gosec) is unchanged, so existing caches are reused, not reinstalled.
    • the Makefile: GOSEC_VERSION?=$(shell cat …/.gosec-version) for install-dev-tools. The path is resolved from the Makefile's own location, and make GOSEC_VERSION=… install-dev-tools still overrides it.
  • scripts/test-run-gosec.sh (from ci(gosec): run one rule set from a shared script in the hook, Makefile and CI #1145) reads the pin for its fake gosec from .gosec-version instead of sed-ing it out of run-gosec.sh.

  • pre-commit.yml:

    • The dead Install gosec step is deleted, and gosec is dropped from the tool-cache key and name. The gosec hook is SKIPped in that job, and the hook installs its own copy under ~/.cache/gosec through run-gosec.sh anyway, so ~/go/bin/gosec never ran even before the SKIP. A comment there records why gosec isn't installed.
    • The stale comment saying it tracks "ci.yml's securego/gosec Action" is gone with the step.
    • The SKIP comment now points to .gosec-version instead of restating v2.28.0.
  • Makefile: the stale comment no longer claims sync with database-migration.yml, which pins none of these tools.

  • Comments in ci.yml, .pre-commit-config.yaml, gosec-hook.sh and security-scan.sh name .gosec-version as the version source.

  • Guard: scripts/check-gosec-pin.sh (new). It runs as a step in the existing security-scan job, right after Set up Go. The step uses always() and only needs checkout, so a finding doesn't skip the scanners. It is not a new job, so ci-success needs: is untouched. It runs git grep over every tracked text file, not a list of known sites, so a new workflow or script is covered automatically. It fails on:

    1. .gosec-version missing, untracked, or not exactly vMAJOR.MINOR.PATCH. That rejects 2.28.0, v2.28, latest, empty, two lines, CRLF and trailing comments.
    2. A version literal next to "gosec" (any case): install refs, GOSEC_VERSION=/?=/: assignments, cache keys, cache paths, image tags, a go.mod requirement, and prose like runs gosec v2.28.0. Prose counts because a restated version goes stale on the next bump.
    3. A gosec ref that isn't a variable: gosec@latest, securego/gosec@<sha> (Action).
    4. An install site (any line naming securego/gosec/v2/cmd/gosec@, so continued go install \ lines count) in a file that never mentions .gosec-version. At least one install site must exist, so an empty match can't pass.

    Exempt: CHANGELOG.md and docs/audits/ (historical records), and the guard and its self-test, which spell out the patterns. Exit codes: 0 clean, 1 findings, 2 usage error, the same scheme as check-gcp-secret-scope.sh.

  • Self-test: scripts/test-check-gosec-pin.sh (new, 43 cases, offline, a few seconds). It runs in the same CI step. Fixtures are throwaway git repos, run with GIT_CONFIG_GLOBAL=/dev/null. It covers:

    • clean trees and near-misses that must pass, such as the gocyclo pin, gosec-results.sarif and the codeql # v4.36.0 comment.
    • each violation shape.
    • usage errors.
    • whether the readers really read the file: make -n install-dev-tools with .gosec-version set to v9.8.7 must install gosec@v9.8.7. The real run-gosec.sh, run directly and through the real hook with a stub go, must call go install …gosec@v9.8.7 into its cache, and must stop with exit 2 without calling go or creating the cache when the file is missing or malformed.
    • the real tree.

Closes #347

Verification

Local shell, fixture and real-tool evidence on Linux (bash 5) after merging current origin/main; macOS (bash 5 and /bin/bash 3.2) was also covered before the merge. No Go code changed. The merged tree still builds and go vets, and go test ./cmd/server/ (which parses ci.yml) passes.

  • bash scripts/check-gosec-pin.sh: OK: gosec v2.28.0 is pinned only in .gosec-version; 2 install site(s) read it. The two sites are run-gosec.sh and the Makefile.
  • bash scripts/test-check-gosec-pin.sh: 43 passed, 0 failed. bash scripts/test-run-gosec.sh: all pass.
  • Fail before. Before the merge, on an archive of origin/main (2f1dbca) with only the new check and test added, check-gosec-pin.sh exited 1 with 21 findings and the self-test failed its reader and real-tree cases.
  • Mutation probes on an archive of this branch: see the latest PR comment for the table on the merged head.
  • Real gosec: bash scripts/gosec-hook.sh cmd/gen-permissions/main.go exits 0 and reuses the existing cached v2.28.0 binary, with no reinstall. make -n install-dev-tools prints go install github.com/securego/gosec/v2/cmd/gosec@v2.28.0, and make -n security-scan-go prints only scripts/run-gosec.sh.
  • Linters: shellcheck 0.11.0 on every touched script: clean. actionlint 1.7.12 on ci.yml and pre-commit.yml: clean.
  • git-secrets: I ran it in a fresh git init with an isolated HOME, after scripts/setup-git-secrets.sh and --register-aws, with the changed files tracked. The scan exits 0, and a planted fake key is flagged.

Interaction with other PRs

Out of scope

  • Other tool pins (golangci-lint, gocyclo, trivy, govulncheck) are still duplicated between workflows and the Makefile. Not in this issue.
  • Known guard limitation, documented in the script header: a version stored in a variable whose name doesn't contain "gosec" (V=2.28.0 … gosec@$V) isn't caught by the literal rule. Rule 4 still requires that file to mention .gosec-version.

Maintainer: please mirror the issue's triage labels onto this PR: priority/p3, severity/low, urgency/eventually, impact/internal, effort/xs, type/chore, triaged.

🤖 Generated with Claude Code

The gosec version was hard-coded in ci.yml, pre-commit.yml, the Makefile
and scripts/gosec-hook.sh, plus comments that restated it. A bump that
missed one site would leave local scans and CI's SARIF upload on
different gosec rule sets.

- Add .gosec-version (v2.28.0, unchanged) as the only place the version
  is written. ci.yml's Security Scanning job, the Makefile and the
  pre-commit hook read it; the hook validates it and stops with exit 2
  on a missing or malformed file.
- Delete pre-commit.yml's gosec install step and drop gosec from its
  tool-cache key. The gosec hook is SKIPped in that job and installs its
  own copy under ~/.cache anyway, so ~/go/bin/gosec was never run.
- Fix the stale comments: pre-commit.yml no longer claims to track a
  securego/gosec Action, and the Makefile no longer claims sync with
  database-migration.yml, which pins none of these tools.
- security-scan.sh no longer suggests installing gosec@latest.
- Add scripts/check-gosec-pin.sh, run in the Security Scanning job: it
  fails on a gosec version literal or a non-variable gosec ref in any
  tracked file, and on an install site that does not read
  .gosec-version. scripts/test-check-gosec-pin.sh covers both directions
  and asserts that the Makefile and the hook really read the file.

Closes LeanerCloud#347

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@IAmJSD IAmJSD added impact/internal Team-internal only type/chore Maintenance / non-user-visible triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/eventually No deadline effort/xs Trivial / one-liner labels Oct 20, 2026
Resolve conflicts with LeanerCloud#1145 (scripts/run-gosec.sh, issue LeanerCloud#340), which
moved the gosec install, rule set and module list into one shared script
called by the pre-commit hook, `make security-scan-go`, CI and
security-scan.sh, with its own GOSEC_VERSION="v2.28.0" literal.

- scripts/gosec-hook.sh, scripts/security-scan.sh, ci.yml gosec step and
  .pre-commit-config.yaml: take main's run-gosec.sh delegation; only the
  comments now name .gosec-version as the version source.
- scripts/run-gosec.sh: read and validate .gosec-version (the validation
  this branch had in gosec-hook.sh) instead of the literal, before Go runs
  or the cache is touched. The cache path is unchanged for v2.28.0.
- scripts/test-run-gosec.sh: read the pin from .gosec-version.
- scripts/test-check-gosec-pin.sh: the reader cases now run run-gosec.sh
  (directly and through the hook) with a stub go.
- Makefile, pre-commit.yml, check-gosec-pin.sh: comments name run-gosec.sh
  as the reader.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD

IAmJSD commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

I merged origin/main (a14dc8f) to resolve the conflicts. New head: ebb97f56. There were no review findings on the previous head (46cbf54).

Conflicts. All of them come from #1145 (scripts/run-gosec.sh, issue #340). It moved the gosec install, rule set and module list into one script that CI, the hook, make security-scan-go and security-scan.sh call, and that script kept its own GOSEC_VERSION="v2.28.0". Both intents are kept: main's single runner, with the version read only from .gosec-version.

  • .github/workflows/ci.yml (gosec step): I took main's scripts/run-gosec.sh --sarif-dir call. This branch's direct go install …@$(cat .gosec-version) is dropped, and the comment now names .gosec-version as the version source. This PR's "Assert the gosec version is pinned only in .gosec-version" step sits next to main's "Run gosec wrapper self-tests" step.
  • scripts/gosec-hook.sh: I took main's thin wrapper. This branch's read_gosec_version moved into run-gosec.sh (below). Only the header comment changed: it names .gosec-version, and exit 2 covers a missing or malformed file.
  • scripts/security-scan.sh: I took main's version, which removes the not-installed branch, so this branch's install-hint change is moot. The comment names .gosec-version.
  • .pre-commit-config.yaml: main's comment, plus that run-gosec.sh installs the version in .gosec-version.

Follow-on edits needed so the merged tree keeps one pin:

  • scripts/run-gosec.sh: its literal is replaced by reading and validating .gosec-version (^v[0-9]+\.[0-9]+\.[0-9]+$, exit 2). Validation runs before go or the cache is touched. The cache path ~/.cache/gosec/v2.28.0-<gover>/ is unchanged.
  • scripts/test-run-gosec.sh: reads the pin from .gosec-version instead of sed-ing run-gosec.sh.
  • scripts/test-check-gosec-pin.sh: the reader cases now run the real run-gosec.sh, directly and through the real hook, with a stub go. A missing, 2.28.0 or latest file must exit 2 with no go call and no cache. v9.8.7 must lead to go install …gosec@v9.8.7 into the cache. There are now 43 cases.
  • Comments in the Makefile, pre-commit.yml and check-gosec-pin.sh named the old readers or the old ~/.cache/pre-commit-gosec path. They now name run-gosec.sh.
  • PR body updated to match: two install sites, ci(gosec): run one rule set from a shared script in the hook, Makefile and CI #1145 merged, and the rule-set and security-scan.sh out-of-scope items dropped because ci(gosec): run one rule set from a shared script in the hook, Makefile and CI #1145 fixed them.

Verification on ebb97f56 (Linux, Go 1.26.9):

  • check-gosec-pin.sh: OK … 2 install site(s) read it (run-gosec.sh and the Makefile).

  • test-check-gosec-pin.sh: 43/0. test-run-gosec.sh: all 17 pass.

  • Real gosec-hook.sh cmd/gen-permissions/main.go: exit 0, cached v2.28.0 reused with no reinstall. make -n install-dev-tools prints gosec@v2.28.0. make -n security-scan-go prints only scripts/run-gosec.sh.

  • shellcheck 0.11.0 on all 6 touched scripts and actionlint 1.7.12 on ci.yml/pre-commit.yml: clean.

  • go build ./.... go vet, with and without -tags integration, on ./cmd/server/ and ./internal/database/postgres/migrations/ (their tests read ci.yml). go test on both packages: ok. No Go code changed in this PR, so golangci-lint and gocyclo don't apply and there are no integration tests to run.

  • git-secrets in a fresh repo with an isolated HOME, with the changed files tracked: exit 0, and a planted fake key is flagged (exit 1). test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.

  • Mutation probes on an archive of the head:

    mutation check self-test
    none 0 0
    run-gosec.sh back to GOSEC_VERSION="v2.28.0" 1 1
    run-gosec.sh validation disabled 0 1
    run-gosec.sh gosec@latest 1 1
    Makefile GOSEC_VERSION?=v2.28.0 1 1
    pre-commit.yml cache key gosec-v2.28.0 re-added 1 1
    bump only .gosec-version to v2.29.0 0 0

Nothing is left open on this PR. The known govulncheck failure (GO-2026-6809, fixed by #1236) is unrelated.

@cristim

cristim commented Oct 25, 2026

Copy link
Copy Markdown
Member

Independent review 2 (claude-fleet, cold): REQUEST_CHANGES ebb97f5

The change itself is sound. It needs a rebase.

Findings:

  • [blocking] .github/workflows/ci.yml and scripts/run-gosec.sh: git merge-tree --write-tree origin/main ebb97f56 exits 1, with conflicts in both files.
    • Since a14dc8f, main has picked up ci(security): cache gosec results and binary across runs #1398/ci(security): save the gosec binary only when it exists #1399 (gosec result and binary caches keyed on run-gosec.sh --print-versions) and fix(deps): bump Go to 1.26.10 for GO-2026-6809 #1236 (go1.26.10).
    • I built a trial resolution: main plus git apply --reject of this diff. Every code hunk applies, and only two comment-only hunks are rejected (the ci.yml gosec step comment and the run-gosec.sh header). The resulting tree passes everything below.
    • Two comments on main will be stale after the rebase:
      • ci.yml:1146 says the pin is "read from scripts/run-gosec.sh so the pin lives in one place".
      • ci.yml:1215 says run-gosec.sh "owns the pinned gosec version".
    • Main's inputs_hash does not hash .gosec-version, but that is fine. results_key and bin_key both embed the gosec_version from --print-versions, which reads .gosec-version, so a bump still changes both keys.
  • [non-blocking] Merge commit ebb97f5 is not a pure merge. git show --remerge-diff shows changes beyond conflict resolution:
    • new pin read and validation logic in scripts/run-gosec.sh, a file the branch had not touched before;
    • a rewrite of the reader tests in scripts/test-check-gosec-pin.sh;
    • a change to scripts/test-run-gosec.sh;
    • comment edits in the Makefile, pre-commit.yml and check-gosec-pin.sh.
    • The merge message discloses all of this, but the behaviour change lives in a merge commit. Land it as an ordinary commit when rebasing.
  • [non-blocking] Makefile:21 reads .gosec-version without validating it. With the file missing, make -n install-dev-tools prints a cat error and produces go install .../gosec@. That fails loudly, so this is acceptable.
  • [non-blocking] Size: check-gosec-pin.sh (149 lines) plus a 302-line self-test guard a pin that, after ci(gosec): run one rule set from a shared script in the hook, Makefile and CI #1145, has only two readers. The deletion probe shows the value: without the guard, base's five leftover literals would have gone unnoticed (pre-commit.yml x3, the Makefile and run-gosec.sh). Probes M5-M7 show every rule is exercised.

Attacked:

  • Root cause: base hard-codes the gosec version in those five places. On the merged tree, the guard reports 2 install site(s), both reading .gosec-version.
  • Blast radius:
    • git grep for run-gosec, gosec-hook, GOSEC_VERSION and gosec-version finds only the files the PR changes, with no Docker or other consumers.
    • REPO_ROOT comes from git rev-parse, so every caller finds the file.
  • Edge cases: CRLF and two-line pins are rejected, and both are tested. Trailing blank lines are accepted because $(cat) strips them, which is harmless.
  • Mutation probes on a scratch copy, each restored and confirmed identical with cmp. All 7 were caught:
    • M1, a literal pin in run-gosec.sh: 5 failures.
    • M2, the format regex removed: 2 failures.
    • M3, a silent fallback when the file is missing: 2 failures.
    • M4, a literal pin in the Makefile: 2 failures.
    • M5, guard rule 4 disabled: 2 failures.
    • M6, rule 2 made case-sensitive: 4 failures.
    • M7, the zero-install-site check disabled: 1 failure.

Verified locally, with GOTOOLCHAIN=go1.26.10 and no cloud credentials, under the heavy lock:

  • Trial merged tree:
    • go build ./...: ok.
    • run-gosec.sh --print-versions prints gosec_version=v2.28.0 and go_version=go1.26.10.
    • check-gosec-pin.sh: OK.
    • test-check-gosec-pin.sh: 43 passed, 0 failed.
    • test-run-gosec.sh: exit 0.
    • shellcheck on all five scripts: clean.
    • make -n install-dev-tools produces gosec@v2.28.0.
  • Head: the same results.
  • Fail-before, with the guard and tests on base:
    • The guard exits 1 with 10 findings.
    • test-check-gosec-pin.sh: 35 passed, 8 failed.
    • test-run-gosec.sh: 4 failures.
  • No Go packages, migrations or DB code changed.

Reviewed by an independent agent with no author context (second reviewer). Review only: nothing was pushed, and the GitHub review state is unchanged.

Bring the branch up to date with main.

Conflicted files:
- scripts/run-gosec.sh: main (LeanerCloud#1398) added --print-versions and a header
  paragraph describing it; this branch replaced the GOSEC_VERSION literal
  with a read of .gosec-version and documented that in the same spot.
  Kept both paragraphs. --print-versions now reports the version read
  from .gosec-version, so CI's gosec cache keys follow the single pin.
- .github/workflows/ci.yml: main (LeanerCloud#1398) added the results-cache
  short-circuit at the top of the "Run gosec Security Scanner" step and
  reworded the run-gosec.sh comment; this branch reworded that comment to
  point at .gosec-version. Kept main's short-circuit and env, with the
  branch's comment.

check-gosec-pin.sh, test-check-gosec-pin.sh and test-run-gosec.sh pass
on the merged tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD

IAmJSD commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Merged origin/main (8f186a8) into this branch. New head: d7ffc4a.

Conflict resolution:

  • scripts/run-gosec.sh: main (ci(security): cache gosec results and binary across runs #1398) added --print-versions and a header paragraph that describes it. This branch documents the .gosec-version read in the same place. I kept both paragraphs. --print-versions now reports the version read from .gosec-version, so CI's gosec binary and results cache keys follow the single pin.
  • .github/workflows/ci.yml: main (ci(security): cache gosec results and binary across runs #1398) added the results-cache short-circuit and its env to the "Run gosec Security Scanner" step, and reworded the run-gosec.sh comment. I kept main's short-circuit and env, and used this branch's comment, which points at .gosec-version.

Checks on the merged head:

  • check-gosec-pin.sh reports "OK: gosec v2.28.0 is pinned only in .gosec-version; 2 install site(s) read it". test-check-gosec-pin.sh passes 43 of 43, test-run-gosec.sh passes, and actionlint and shellcheck are clean.
  • build, vet (also with -tags integration) and e2e vet pass. go test ./cmd/server/ and the short migrations tests also pass; both read ci.yml.
  • The secrets scan is clean and the planted key was flagged. The allowlist test passes.

@IAmJSD

IAmJSD commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE d7ffc4a39f00592f305264f1275eeee2daf07cd7

Blocking:

  • none

Non-blocking:

  • Makefile:20: GOSEC_VERSION?=$(shell cat $(GOSEC_VERSION_FILE)) doesn't validate the file the way run-gosec.sh does. If .gosec-version is missing, make install-dev-tools runs go install ...gosec@, which fails with a vague go error instead of naming the file. The CI guard catches a missing or malformed file on every PR, so this only matters locally.
  • The guard's rule 2 (gosec near any x.y.z literal, prose included) is deliberately broad. A future doc line such as "gosec flagged this in Go 1.24.1" would trip it. That's acceptable given the self-documenting error message, but expect the occasional false positive.

Attacked:

  • Checked every acceptance criterion: one source (.gosec-version), read by run-gosec.sh (used by the ci.yml step, the hook and make security-scan-go) and by the Makefile. Both stale comments are fixed, and the dead install step and cache key are gone from pre-commit.yml.
  • Checked that the new ci.yml step reuses the existing id: checkout and adds no job, so ci-success needs: is untouched. Checked that --print-versions (used for CI cache keys) still goes through validation. Checked CRLF, two-line and empty files against the $(cat) + anchored regex.
  • Ran: check-gosec-pin.sh (OK, 2 sites), test-check-gosec-pin.sh (43/43), test-run-gosec.sh (all pass), shellcheck on all touched scripts (clean), actionlint on ci.yml and pre-commit.yml (clean), make -n install-dev-tools (installs @v2.28.0), go test ./cmd/server/ (parses ci.yml, ok).
  • Fail-before: I restored run-gosec.sh, Makefile and pre-commit.yml from origin/main. test-check-gosec-pin.sh then failed 8 of 43 (reader, validation and real-tree cases), and check-gosec-pin.sh reported 9 findings. I restored the branch afterwards.

Body-stale:

  • none found. The body's claims (2 install sites, 43 cases, cache path unchanged) match what I ran.

Reviewer: rB4. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/internal Team-internal only priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/eventually No deadline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: single-source the gosec pin across ci.yml, pre-commit.yml, Makefile and gosec-hook.sh [platform part]

2 participants