go-security-review

Go Security Review

Purpose

Identify exploitable security vulnerabilities in Go code. Scope is strictly security: injection, authentication/authorization, cryptography, secrets management, input validation, transport security, and HTTP hardening.

This skill does NOT cover: performance, concurrency (race conditions), code quality/style, test quality, error handling patterns, or business logic correctness — those belong to sibling vertical skills.

When To Use

• Code touches SQL queries, command execution, or file path operations
• Code handles user input, authentication, authorization, or session management
• Code involves HTTP handlers, TLS configuration, or cryptographic operations
• Code contains hardcoded string literals that may be secrets
• Security-focused PR review requested

When NOT To Use

• Performance optimization → go-performance-review
• Concurrency/race conditions → go-concurrency-review
• Error handling correctness → go-error-review
• Code style/lint → go-quality-review
• Test quality → go-test-review
• Business logic correctness → go-logic-review

Mandatory Gates

1) Execution Integrity Gate

Never claim gosec or any security tool ran unless it actually produced output. If not run: state reason + exact command.

2) Go Version Gate

Read go.mod for the go directive. Do NOT recommend version-specific features above project version. If inaccessible, record Go version: unknown.

3) Anti-Example Suppression Gate

Before reporting, verify finding is not a false positive. MUST quote specific code evidence satisfying the precondition. Category match alone is insufficient.

Embedded anti-examples for security domain:

• Speculative injection when input is internal/constant: Do NOT flag fmt.Sprintf in SQL when the interpolated value is a compile-time constant, config value, or internal enum. Trace data flow from input source to dangerous function — confirm user input actually reaches it.
• Over-cautious crypto on non-password use: Do NOT flag MD5/SHA1 for cache key derivation, content hashing, or checksums where collision resistance is not a security requirement. Flag ONLY for password hashing, auth tokens, or integrity verification of untrusted data.
• Context over-propagation: Do NOT flag "missing context.Context" when function is synchronous, short-lived, no I/O, no cancellable work.
• Rate limiting on internal-only endpoints: Do NOT flag rate limiting absence on endpoints not exposed to public traffic (internal mesh, admin behind VPN). Verify endpoint exposure before reporting.
• Insufficient evidence rule: Every finding must quote concrete code evidence. "This endpoint handles user input" is not sufficient — show the path from input to dangerous function.

4) Generated Code Exclusion Gate

Exclude: *.pb.go, *_gen.go, mock_*.go, wire_gen.go, *_string.go, files with // Code generated .* DO NOT EDIT. Note excluded files in Execution Status.

Workflow

1. Define scope — confirm files/diff under review. Apply Generated Code Exclusion Gate.
2. Gather evidence — read changed files, identify security-relevant patterns: SQL strings, os/exec, filepath, HTTP handlers, TLS config, crypto, hardcoded literals, auth middleware, URL fetching, template rendering.
3. Load references — always load go-security-patterns.md; load go-api-http-checklist.md when HTTP/API code present.
4. Evaluate checklist — execute ALL 16 items. For injection findings, trace data flow from input source to dangerous function.
5. Apply suppression — run candidates through anti-example gate → format output.

Grep-Gated Execution Protocol

This skill uses mechanical grep pre-scanning to guarantee zero missed checklist items. 14 of 16 items are grep-gated; 2 are semantic-only.

Execution Order

1. Identify target files (from dispatch prompt, or write raw snippet to $TMPDIR/review_snippet.go)
2. Run grep for all grep-gated checklist items against target files
3. HIT → run semantic analysis to confirm or reject (trace data flow for injection findings)
4. MISS → auto-mark NOT FOUND, skip semantic analysis
5. For compound patterns: run primary grep, then secondary pattern, apply AND/AND NOT logic
6. For semantic-only items (items 9, 12): full model reasoning
7. Report only FOUND items

Grep Audit Line

Include in Execution Status: Grep pre-scan: X/14 items hit, Z confirmed as findings (2 semantic-only)

Compound Pattern Protocol

• Item 3 (Path traversal): filepath\.Join\|os\.Open HIT → trace whether input comes from user request
• Item 8 (Sensitive data in logs): log\.\|slog\. HIT → check if logged value includes password/token/PII
• Item 10 (SSRF): http\.Get\|client\.Do HIT → trace whether URL comes from user input
• Item 15 (Timing attack): secret comparison HIT AND subtle.ConstantTimeCompare NOT found
• Item 16 (Input validation): Atoi\|ParseInt HIT → check if result used for allocation/slice sizing without bounds check

Security Checklist (16 Items)

All High severity unless marked (Medium).

Severity Rubric

High — Exploitable vulnerability: injection, auth bypass, data exposure, SSRF, hardcoded secrets, insecure TLS.

Medium — Requires specific conditions or defense-in-depth gap: missing headers, rate limiting, weak config defaults.

Evidence Rules

• Every finding: exact location (path:line), concrete impact, actionable fix with code example
• For injection: trace data flow — name the source (e.g., r.URL.Query().Get("q") at handler.go:23) and the sink (e.g., fmt.Sprintf at repo.go:67)
• No speculative findings — require evidence of user-controlled input reaching the vulnerable path
• Merge rule: same issue at ≥3 locations → one finding with location list

Output Format

Findings

[High|Medium] Short Title

• ID: SEC-NNN
• Location: path:line (or location list)
• Impact: What an attacker could achieve
• Evidence: Concrete code path showing the vulnerability
• Recommendation: Specific fix with code example
• Action: must-fix | follow-up

Suppressed Items

[Suppressed] Short Title

• Reason: Anti-example matched + specific evidence cited
• Residual risk: Brief note

Execution Status

• Go version: X.Y or unknown
• gosec: PASS | FAIL | Not available (reason + command)
• Grep pre-scan: X/14 items hit, Z confirmed as findings (2 semantic-only)
• Excluded (generated): list or None
• References loaded: list

Summary

1-2 lines. Count by severity.

Example Output

### Findings

#### [High] SQL Injection in User Search
- **ID:** SEC-001
- **Location:** `internal/repo/user.go:67`
- **Impact:** Attacker can execute arbitrary SQL via search parameter
- **Evidence:** `fmt.Sprintf("SELECT * FROM users WHERE name LIKE '%%%s%%'", name)` — `name` flows from `r.URL.Query().Get("q")` at handler.go:23 through SearchUsers() without sanitization
- **Recommendation:** Use parameterized query: `db.QueryContext(ctx, "SELECT * FROM users WHERE name LIKE ?", "%"+name+"%")`
- **Action:** must-fix

### Suppressed Items
#### [Suppressed] MD5 Usage in Cache Key Generation
- **Reason:** MD5 at cache.go:15 is used for cache key derivation from internal struct, not password hashing. Anti-example: "over-cautious crypto on non-password use"
- **Residual risk:** None — cache key collision is acceptable

### Execution Status
- Go version: 1.21
- gosec: Not available (command: `gosec ./...`)
- Grep pre-scan: 3/14 items hit, 1 confirmed as findings (2 semantic-only)
- Excluded (generated): None
- References loaded: go-security-patterns.md, go-api-http-checklist.md

### Summary
1 High finding (SQL injection). No Medium findings.

No-Finding Case

If no issues found: state No security findings identified. Still output Execution Status, Suppressed Items (if any), Summary.

Load References Selectively

Review Discipline

• Security only — never comment on performance, concurrency, style, tests, error handling, or logic
• Execute ALL 16 checklist items — do not skip because others produced High findings
• Trace data flow for every injection finding — name source and sink
• Prefer precision over volume — one well-evidenced finding beats five speculative warnings