Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Jun 5, 2026
+7 −5
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
465ca3c
Initial plan
Copilot Jun 5, 2026
a8c53b3
fix: align gosec to v2.27.1, add nosec justifications, enforce nosec …
Copilot Jun 5, 2026
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
3 changes: 2 additions & 1 deletion .github/workflows/security-scan.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ jobs:

- name: Run Gosec
run: |
go install github.com/securego/gosec/v2/cmd/gosec@v2.22.11
go install github.com/securego/gosec/v2/cmd/gosec@v2.27.1
# Exclusions configured in .golangci.yml (linters-settings.gosec.exclude)
# Keep this list in sync with .golangci.yml for consistency
gosec -fmt sarif -out gosec-results.sarif -stdout -exclude-generated -track-suppressions \
-nosec-require-rules -nosec-require-justification \
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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.

-exclude=G101,G115,G204,G602,G301,G302,G304,G306 \
./...
Comment on lines 31 to 35

Expand Down
5 changes: 3 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,12 @@ security-scan: security-gosec security-govulncheck
.PHONY: security-gosec
security-gosec:
@echo "Running gosec security scanner..."
@command -v gosec >/dev/null || go install github.com/securego/gosec/v2/cmd/gosec@v2.23.0
@command -v gosec >/dev/null || go install github.com/securego/gosec/v2/cmd/gosec@v2.27.1
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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.

@# Exclusions configured in .golangci.yml (linters-settings.gosec.exclude)
@# Keep this list in sync with .golangci.yml for consistency
@GOPATH=$$(go env GOPATH); \
PATH="$$GOPATH/bin:$$PATH" gosec -fmt=json -out=gosec-report.json -stdout -exclude-generated -track-suppressions \
-nosec-require-rules -nosec-require-justification \
-exclude=G101,G115,G204,G602,G301,G302,G304,G306 \
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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

./...
Comment on lines 202 to 205
@echo "✓ Gosec scan complete (results in gosec-report.json)"
Expand Down Expand Up @@ -390,7 +391,7 @@ check-node-version:
tools: ## Install build-time tools from tools.go
@echo "Installing build tools..."
@go install github.com/rhysd/actionlint/cmd/actionlint@v1.7.11
@go install github.com/securego/gosec/v2/cmd/gosec@v2.23.0
@go install github.com/securego/gosec/v2/cmd/gosec@v2.27.1
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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.

@go install golang.org/x/tools/gopls@v0.21.1
@go install golang.org/x/vuln/cmd/govulncheck@v1.1.4
@echo "✓ Tools installed successfully"
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/run_workflow_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func IsRunnable(markdownPath string) (bool, error) {

// Read the lock file - path is sanitized using filepath.Clean() to prevent path traversal attacks.
// The lockPath is derived from markdownPath which comes from trusted sources (CLI arguments, validated workflow paths).
contentBytes, err := os.ReadFile(cleanLockPath) // #nosec G304
contentBytes, err := os.ReadFile(cleanLockPath) // #nosec G304 -- path is sanitized with filepath.Clean() and derived from trusted CLI argument
Comment on lines 46 to +48
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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.

if err != nil {
return false, fmt.Errorf("failed to read lock file: %w", err)
}
Expand Down Expand Up @@ -98,7 +98,7 @@ func getWorkflowInputs(markdownPath string) (map[string]*workflow.InputDefinitio

// Read the lock file - path is sanitized using filepath.Clean() to prevent path traversal attacks.
// The lockPath is derived from markdownPath which comes from trusted sources (CLI arguments, validated workflow paths).
contentBytes, err := os.ReadFile(cleanLockPath) // #nosec G304
contentBytes, err := os.ReadFile(cleanLockPath) // #nosec G304 -- path is sanitized with filepath.Clean() and derived from trusted CLI argument
Comment on lines 99 to +101
if err != nil {
return nil, fmt.Errorf("failed to read lock file: %w", err)
}
Expand Down
Loading