Repository navigation
ci: pre-commit gosec hook and CI enforce DIFFERENT gosec rule sets [cli part] #1394
Description
Duplication audit found a FUNCTIONAL divergence, not just DRY: scripts/gosec-hook.sh (#1376) excludes ~15 gosec rules and hand-mirrors ci.yml's module list, while the CI Security Scanning job runs gosec with NO -exclude. So a local pre-commit pass does NOT predict CI (the hook is more lenient), defeating the hook's purpose (feedback_gosec_nosec_not_nolint says local+CI must agree). Plus version drift: ci.yml on main is v2.26.1, the hook is v2.28.0, #1384 bumps CI to v2.28.0.
Fix: single source of truth - a shared scripts/run-gosec.sh (version + module list + flags) invoked by BOTH the pre-commit hook and the CI job, so they enforce IDENTICAL rules. Land #1384 (CI->v2.28.0) before/with #1376 so versions match. Remove the hook's extra -exclude list (or apply the same list in CI, deliberately).
Findings from the 2026-09-02 codebase audit
Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.
A14-012 (medium)
A third divergent gosec rule set exists beyond the hook and CI. Makefile:159 runs gosec -fmt=json -out=gosec-report.json -exclude=G101,G104,G115,G204,G301,G304,G402,G505 ./..., while ci.yml:694-705 runs gosec with no -exclude at all. So make security-scan silences eight rule classes, including the TLS and InsecureSkipVerify-adjacent ones, and a clean local run predicts nothing about the CI job. A shared scripts/run-gosec.sh should be invoked from the Makefile target too, not just from the hook and the CI job, or the divergence simply moves. The separate defect that this target's trailing echo swallows gosec's exit status is already tracked in #1594's Makefile item. Audit finding A14-012.
Scope after the split
The CUDly monorepo was split into four repos. This issue is now scoped to only cloud-commitments-cli's own scripts/gosec-hook.sh, .github/workflows/ci.yml, and Makefile. The version-drift part of the original finding is already moot here: both the hook and CI are pinned to gosec v2.28.0. The functional rule-set divergence (hook excludes 15 rules, CI excludes none, Makefile excludes a different 8) is still live in this repo.
The identical divergence exists in the other three split repos; each got its own tracking issue:
- ci: pre-commit gosec hook, CI and Makefile enforce DIFFERENT gosec rule sets [go part] cloud-commitments-go#112
- ci: pre-commit gosec hook, CI and Makefile enforce DIFFERENT gosec rule sets [mcp part] cloud-commitments-mcp#16
- ci: pre-commit gosec hook, CI and Makefile enforce DIFFERENT gosec rule sets [platform part] cloud-commitments-platform#340
Acceptance criteria (this repo)
- A single
scripts/run-gosec.sh(version + module list +-excludeflags in one place) is added and invoked by the pre-commit hook, the CI job, and theMakefilesecurity-scan-gotarget. - The hook's and Makefile's extra
-excludelists are either removed (rules enforced everywhere) or deliberately applied in CI too, with the decision documented in the script. - A local
make security-scan-go(or pre-commit hook run) and the CI job report the same findings for the same commit.
Activity
cristim commentedon Jul 28, 2026
Reviewed commit:
be11bdcb5. Note:origin/mainmoved to3e9660d06during the review; re-verify against currentmainbefore changing code, since a finding may have been fixed or moved.Confirmed still present on
3e9660d06. Adding the exact suppression lists, a second excluding site this issue does not mention, and one detail that makes the list look better justified than it is.Where
scripts/gosec-hook.sh:38--exclude=G101,G104,G115,G117,G118,G122,G204,G301,G304,G402,G505,G702,G703,G705,G706Makefile:149--exclude=G101,G104,G115,G204,G301,G304,G402,G505(a second, different exclusion list, so there are two divergent local rule sets, not one).github/workflows/ci.yml:337- CI's gosec passes no-excludeflags at allWhat is being suppressed locally but enforced in CI
Class-wide, not site-specific: hardcoded credentials (G101), unhandled errors (G104), integer overflow conversion (G115), subprocess with variable input (G204), directory permissions (G301), file path from variable (G304), TLS
MinVersionunset (G402), weak crypto (G505).Detail worth knowing before the fix
Seven of the fifteen IDs in
gosec-hook.sh-G117,G118,G122,G702,G703,G705,G706- are not gosec rule IDs at the pinned version and are inert. The list is therefore both longer and less deliberate than it appears; roughly half of it suppresses nothing. Whoever does the fix should not assume each entry represents a considered decision that needs preserving.Failure scenario
The developer-facing gate that exists to prevent a CI failure cannot reproduce it. A new
exec.Commandon a variable, or a TLS config with noMinVersion, passes pre-commit and pre-push cleanly, then failsSecurity Scanningafter push - reddening the branch and costing a full CI round trip per occurrence. Every one of those round trips is a case where the hook did the opposite of its job.Fix direction
As proposed here: one shared
scripts/run-gosec.sh(version, module list, flags) invoked by both the hook and the CI job. Two additions:Makefile:149'ssecurity-scan-gotarget in the consolidation, otherwise a third rule set survives.#nosec G<rule> -- <guard>comment at the specific site, which both tools honour, rather than a class-wide-exclude. This matches the repo's existingfeedback_gosec_nosec_not_nolintconvention.Note that
Makefile:146-153also never fails on findings even when gosec runs (the target's exit status is that of a trailing successecho), so consolidating the rule set alone will not make the local gate load-bearing; that half is filed as a MEDIUM in the groupedchore(ci)issue from this review.Related: #1297 covers the version drift across the same declaration sites; these two are best landed as one "single source of truth for gosec" change.
[-]ci: pre-commit gosec hook and CI enforce DIFFERENT gosec rule sets[/-][+]ci: pre-commit gosec hook and CI enforce DIFFERENT gosec rule sets [cli part][/+]cristim commentedon Sep 27, 2026
This is a cross-component issue from before the CUDly monorepo split into four repos. It's now scoped to this repo's own gosec hook/CI/Makefile divergence. The identical divergence in the other three split repos got its own tracking issue:
cristim commentedon Oct 7, 2026
claimed by cc-cli-w4
cristim commentedon Oct 8, 2026
Merged normally as
7055a1abd41bf19d2adf220a7f28791ab9efee1e. Its tree023d33980e3e1b79eca5b01f97ff827111102ebdexactly matches the independently reviewed head. The actual post-merge Bash 3.2 contract check passed; post-merge build/test run37730179191 and pre-commit run37730179314 both completed successfully at this merge SHA.Issue1394 is closed. All source branches, tests, metadata, and verification artifacts were preserved. CodeRabbit review was quota-waived with independent evidence; retrospective full review is recorded for the coordinator's existing cadence. Embedded gosec assessment remains #2152. Local act simulation hit GitHub408 twice; actual CI and exact local run-block evidence passed.