fix(gosec): align to v2.27.1, enforce nosec policy flags, add missing justifications

[Copilot]

gosec was running three different versions across go.mod (v2.27.1), Makefile (v2.23.0), and CI (v2.22.11) — meaning CI ran a version predating G708/G709/G124 and several false-positive fixes. The existing #nosec G<id> -- <justification> policy was documented but not mechanically enforced.

Changes

• Version alignment — bumps gosec to v2.27.1 in all invocation sites: Makefile (security-gosec and tools targets) and .github/workflows/security-scan.yml
• Policy enforcement — adds -nosec-require-rules -nosec-require-justification flags to both the make security-gosec target and the CI gosec step, turning the existing annotation convention into a hard check
• Missing justifications — the only two annotations that would fail the new flags, both in pkg/cli/run_workflow_validation.go:

// Before
contentBytes, err := os.ReadFile(cleanLockPath) // #nosec G304

// After
contentBytes, err := os.ReadFile(cleanLockPath) // #nosec G304 -- path is sanitized with filepath.Clean() and derived from trusted CLI argument

[Copilot]

Pull request overview

This PR standardizes gosec usage across local tooling and CI by pinning a single gosec version and turning the existing #nosec G<id> -- <justification> convention into an enforced policy.

Changes:

• Align gosec installation/invocation to v2.27.1 in the Makefile and security-scan GitHub Actions workflow.
• Enforce suppression hygiene by enabling -nosec-require-rules and -nosec-require-justification in both local (make security-gosec) and CI scans.
• Add missing #nosec justifications in pkg/cli/run_workflow_validation.go.

Show a summary per file

File	Description
pkg/cli/run_workflow_validation.go	Adds #nosec justifications on lockfile reads to comply with the newly enforced policy.
Makefile	Pins gosec version and enforces #nosec rule+justification requirements for local security scans.
.github/workflows/security-scan.yml	Pins gosec version and enforces #nosec rule+justification requirements in CI.

Copilot's findings

Tip

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

• Files reviewed: 3/3 changed files
• Comments generated: 4

[github-actions]

🧪 Test Quality Sentinel completed test quality analysis.

No test files were added or modified in PR #37100 (fix(gosec): align to v2.27.1, enforce nosec policy flags, add missing justifications). Changed files: .github/workflows/security-scan.yml, Makefile, pkg/cli/run_workflow_validation.go — none are test files. Test Quality Sentinel skipped.

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #37100 does not have the implementation label (has_implementation_label=false) and has only 2 new lines in business logic directories (well under the 100-line threshold; requires_adr_by_default_volume=false).

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

⚠️ PR Code Quality Reviewer failed during code quality review.

[github-actions]

Skills-Based Review 🧠

Applied /diagnose, /zoom-out, and /improve-codebase-architecture — COMMENT only; the changes are correct and the alignment goal is achieved.

📋 Key Themes & Highlights

Key Themes

• G304 exclusion ambiguity: G304 is in the global --exclude list, making all per-line #nosec G304 annotations documentation-only rather than security-enforcing. The PR enforces annotation format (via -nosec-require-justification), not the underlying security rule. This is a valid policy choice but should be made explicit.
• Version guard gap: command -v gosec only checks binary existence, not version — developer drift is still possible.
• Version scattered across 3 sites: The exact pattern that caused the drift being fixed is not prevented from recurring.
• Duplicate read pattern: The two os.ReadFile(cleanLockPath) sites in run_workflow_validation.go now carry identical nosec annotations, increasing future maintenance burden.

Positive Highlights

• ✅ Clean, surgical change — 7 additions, 5 deletions, no logic changes
• ✅ Version alignment across all invocation sites is correct and well-motivated
• ✅ Enforcing -nosec-require-rules -nosec-require-justification upgrades the annotation policy from convention to a hard gate
• ✅ Justification text is accurate and specific for both fixed annotations
• ✅ PR description clearly explains the root cause and each change

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 2.3M · 315.8 AIC

[github-actions]

[/diagnose] command -v gosec checks only whether the binary exists, not its version. A developer who already has gosec v2.23.0 installed will run the old version locally, keeping local results divergent from CI despite this version-alignment fix.

💡 Suggested fix

Replace the existence guard with an explicit version check:

`@gosec_ver`=$$(gosec --version 2>/dev/null | grep -o 'v[0-9.]*' | head -1); \
if [ "$$gosec_ver" != "v2.27.1" ]; then \
  go install github.com/securego/gosec/v2/cmd/gosec@v2.27.1; \
fi

Alternatively, always install (unconditionally): go install github.com/securego/gosec/v2/cmd/gosec@v2.27.1 — fast when already at the right version thanks to Go module cache.

[github-actions]

[/diagnose] G304 is listed here in the global --exclude list, meaning gosec never checks for G304 violations anywhere in the codebase. The #nosec G304 -- ... per-line annotations this PR fixes are therefore dead code from a security-enforcement standpoint — gosec would not flag those os.ReadFile calls even without them.

💡 Why this matters and a suggested resolution

The hybrid approach (global exclusion + per-line annotations) has an invisible gap: any new os.ReadFile call added without a #nosec G304 annotation will also not be caught by gosec since G304 is excluded globally. The per-line justifications document intent for existing code but do not enforce it for future code.

Option A — Drop global exclusion, rely on per-line suppression (recommended):

• Remove G304 from this --exclude line and from .golangci.yml
• All existing os.ReadFile calls already carry #nosec G304 -- justification annotations
• Future unguarded calls will now be flagged by gosec

Option B — Accept documentation-only policy (current implicit choice):

• Add a comment here: # G304 excluded globally; per-line #nosec annotations are documentation-only audit trail
• Document this explicitly in .golangci.yml so reviewers understand the intent

[github-actions]

[/improve-codebase-architecture] The gosec version v2.27.1 is now hardcoded in three separate places: security-gosec target (line 198), tools target (here), and .github/workflows/security-scan.yml. Future version bumps require updating all three manually — one missed site is how the drift this PR fixes was introduced in the first place.

💡 Suggested: single-source the version

Define a variable at the top of the Makefile:

GOSEC_VERSION ?= v2.27.1

Then reference it everywhere:

go install github.com/securego/gosec/v2/cmd/gosec@$(GOSEC_VERSION)

For CI, either read the variable from the Makefile or define it in a shared env var / .tool-versions file. This makes the single source of truth mechanical rather than documentary.

[github-actions]

[/zoom-out] The identical lock-file read pattern appears twice in this file — once in IsRunnable (here, line 48) and once in getWorkflowInputs (line 101): same cleanLockPath, same os.ReadFile, same #nosec G304 annotation, same error message. This PR now requires both annotations to be kept in sync.

💡 Suggested: extract a shared helper

// readLockFile reads and returns the contents of the compiled lock file at lockPath.
// The caller is responsible for sanitizing lockPath with filepath.Clean() before calling.
func readLockFile(cleanLockPath string) ([]byte, error) {
    content, err := os.ReadFile(cleanLockPath) // #nosec G304 -- path is sanitized with filepath.Clean() and derived from trusted CLI argument
    if err != nil {
        return nil, fmt.Errorf("failed to read lock file: %w", err)
    }
    return content, nil
}

This centralises the nosec annotation in one place and removes the maintenance risk of the two sites diverging again.

[github-actions]

[/diagnose] pkg/cli/logs_download.go has two active-rule nosec annotations using single-dash format that are not covered by this PR:

// #nosec G305 - Path traversal is prevented ...
// #nosec G110 - Decompression bomb is mitigated ...

G305 and G110 are not in the global --exclude list, so they are actively suppressed by these annotations. The single-dash format (-) differs from the -- convention reinforced by this PR. While -nosec-require-justification likely accepts any non-empty text after the rule ID, adopting -- uniformly is worth confirming and patching to avoid surprises with future gosec versions.