Produce gosec SARIF via golangci-lint for suppression parity#561
Conversation
The security workflow ran standalone gosec v2.25.0 with -no-fail and uploaded the SARIF to code scanning. Standalone gosec cannot read the repo's //nolint:gosec suppressions or .golangci.yml gosec excludes (its only conventions are #nosec / //gosec:disable), so every accepted site appeared as an open alert (159 on main) and any PR touching an annotated exec site turned the non-required "gosec" code-scanning check red with a new alert (#294, #307) — recurring noise with no signal, since the required Lint gate (golangci-lint) already enforces gosec with those suppressions applied. Generate the SARIF with the same pinned golangci-lint action and version as test.yml's Lint job, restricted to gosec via --enable-only, so the Security tab reflects exactly what the enforcing gate enforces. A jq step rewrites the SARIF driver name to match the historical standalone uploads (so the first main upload supersedes them, closing all 159 open alerts as "fixed") and restores per-rule G### identity that golangci-lint flattens to a single "gosec" ruleId. Tradeoff: golangci-lint's SARIF carries no driver.rules metadata, so future findings lose gosec's rich rule help text in the Security tab — acceptable since the expected steady state is zero findings.
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
There was a problem hiding this comment.
Pull request overview
Aligns the Security workflow’s SARIF generation with the required golangci-lint gate by producing gosec findings via golangci-lint (gosec-only), so //nolint:gosec and .golangci.yml gosec excludes apply consistently and code-scanning alerts stop regressing on accepted exec sites.
Changes:
- Switch SARIF generation from standalone
gosectogolangci-lintrestricted to gosec (--enable-only gosec) with--issues-exit-code 0to keep the job advisory. - Post-process the SARIF with
jqto preserve historical tool continuity and restore per-ruleG###identity inruleId.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
.github/workflows/security.yml:118
- The
jq capture(...)call will throw if a result message doesn’t start withG###:(or ifresultsis missing), which would fail the workflow even though the scan is meant to be advisory. Wrapping the capture intry/catchand making the results traversal optional avoids brittle failures while preserving the per-ruleruleIdrewrite when the pattern is present.
jq '.runs[].tool.driver.name = "Golang security checks by gosec"
| .runs[].results[] |= (.ruleId = ((.message.text // "" | capture("^(?<g>G[0-9]{3}):").g) // .ruleId))' \
gosec-results.sarif > tmp.sarif && mv tmp.sarif gosec-results.sarif
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
This reverts commit 7c4eddf.
github-actions
Bot
added
bug
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
.github/workflows/security.yml:118
- The jq filter uses
capture(...)without an error-suppressing?/try. In jq,capturethrows when the regex doesn’t match, so if any SARIF result message ever lacks a leadingG###:(or.message.textis missing/empty), this step will fail and prevent SARIF upload. Make the extraction resilient so non-matching messages simply keep the existing ruleId.
jq '.runs[].tool.driver.name = "Golang security checks by gosec"
| .runs[].results[] |= (.ruleId = ((.message.text // "" | capture("^(?<g>G[0-9]{3}):").g) // .ruleId))' \
gosec-results.sarif > tmp.sarif && mv tmp.sarif gosec-results.sarif
jq's capture throws on non-matching input and // does not catch exceptions, so a gosec message without a leading G### code would fail the continuity step and block the SARIF upload. Suppress errors with ? on both the capture pipeline and the results traversal so non-matching messages keep their existing ruleId and a run without results passes through untouched.
Problem
The code-scanning "gosec" check turns red on any PR whose changed lines include an annotated exec site — most recently #555 (alert #307,
refreshClaudeMarketplace'sexec.CommandContext), and #294 before it. Recurring noise, zero signal.Root cause: two scanners, one suppression convention. The enforcing gate is golangci-lint (required Lint check), which honors
//nolint:gosecand.golangci.ymlgosec excludes. The security workflow, however, generated its SARIF with standalone gosec, which can only read#nosec///gosec:disable—//nolint:goseccan never match (verified in gosec source; no version bump fixes it). So the SARIF perpetually reported every accepted site (163 findings, 159 open alerts on main), and each new annotated site became a "new" alert failing the PR check.Fix
Generate the SARIF with the same pinned golangci-lint action + version as test.yml's Lint job, restricted to gosec via
--enable-only gosec. The Security tab now reflects exactly what the required gate enforces — one suppression source of truth.--issues-exit-code 0keeps the job advisory; a real escape still fails the required Lint check, and code scanning still alerts on it.A jq step before upload:
Golang security checks by gosec, so the upload lands in the historical tool stream (same tool + same derived category.github/workflows/security.yml:gosec) — the first upload on main supersedes the old analyses and closes all 159 open alerts as "fixed", no manual dismissals;G###identity (golangci-lint flattens every finding toruleId: "gosec"; the G-code leads each message text).Accepted tradeoff: golangci-lint's SARIF has no
driver.rulesmetadata, so future findings lose gosec's rich rule descriptions in the Security tab. Per-rule identity is preserved via the jq derivation, and the expected steady state is zero findings.Verification
golangci-lint v2.11.1 run --enable-only gosec --build-tags dev --issues-exit-code 0 --output.sarif.path …): valid SARIF, 0 results.//nolint:gosec, regenerated — raw SARIF showsruleId: "gosec"with messageG204: Subprocess launched…; after the jq step,ruleId: "G204". Reverted.Golang security checks by gosec, category.github/workflows/security.yml:gosec, 163 results / 159 open alerts onrefs/heads/main. Job name and workflow path unchanged → same derived category.bin/cigreen.Pre-merge: verify this PR's analysis landed in the same tool/category stream with
results_count == 0. Post-merge: open standalone-gosec alerts on main drop to zero (incl. #307, closed as "fixed").Refs #555 (alert #307), supersedes the dismiss-in-Security-tab playbook.
Summary by cubic
Aligns code scanning with the enforced linter by generating
gosecSARIF viagolangci-lint, so//nolint:gosecand.golangci.ymlsuppressions apply and noisy alerts stop. The job stays advisory; real issues still fail the required Lint check.gosecthroughgolangci/golangci-lint-actionwithversion: v2.11.1,--enable-only gosec,--build-tags dev, and--issues-exit-code 0to match the required Lint gate.jq: set driver name to "Golang security checks by gosec", restore per-ruleG###ruleId, and make the rewrite error-resilient using?so missing codes or empty results don’t break the upload; then upload viagithub/codeql-action/upload-sarif. This supersedes standalonegosecanalyses and closes false positives while keeping future alerts in sync withgolangci-lint.Written for commit e753df1. Summary will update on new commits.