Security Review¶

Find exploitable risks early, provide concrete fixes, and keep recommendations practical for engineering delivery.

Review Principles¶

• Prioritize exploitable risk over style issues.
• Ground every claim in code/config/runtime evidence.
• Distinguish confirmed vulnerabilities from hypotheses.
• Provide reproducible proof for high-risk findings.
• Map findings to standards for auditability.
• If evidence is missing, state Not found in repo.
• Fail closed: if a mandatory gate cannot be executed, state it explicitly and do not claim full coverage.

Evidence Confidence (Mandatory)¶

Each finding must include one confidence label:

• confirmed: vulnerable path proven by code and/or reproducible execution.
• likely: strong evidence with one missing runtime assumption.
• suspected: weak evidence requiring additional data.

Do not report P0/P1 without confirmed or explicit justification.

False-Positive Suppression Rules¶

Before publishing a finding, check suppression conditions:

1. Existing upstream guard already blocks the path.
2. Input is not attacker-controlled at trust boundary.
3. Sink is parameterized/safely encoded by framework guarantees.
4. Environment-only theoretical risk without reachable path.

If suppressed:

• keep a short note under Open questions / assumptions
• mark as suppressed (not a finding)
• explain blocking control and residual risk

Severity Model¶

• P0 Critical: immediate compromise (RCE, auth bypass, key exfiltration, payment tampering).
• P1 High: strong exploit path (injection, IDOR, sensitive data leak, broken authz).
• P2 Medium: meaningful defense gap likely to become exploitable.
• P3 Low: hardening improvement.

Remediation SLA (Default)¶

Use this unless the team provides stricter policy:

• P0: mitigation immediately, full fix within 24h.
• P1: fix within 3 business days.
• P2: fix within 14 calendar days.
• P3: backlog with planned milestone.

If SLA differs, state the project policy explicitly.

Baseline Diff Mode (Mandatory When Baseline Exists)¶

When previous review artifacts exist, compare current findings with baseline and output:

• new: not present in baseline
• regressed: existed before and severity/confidence worsened
• unchanged: still present without material change
• resolved: removed since baseline

If no baseline exists, state Baseline not found.

Review Depth Selection (Mandatory First Step)¶

Before starting, classify review depth based on change scope:

Trigger signals that force Standard or Deep regardless of file count:

• Auth/authz middleware or handler changes
• Crypto, TLS, or secret-management code changes
• Payment/financial transaction paths
• New HTTP/gRPC endpoints exposed
• Dockerfile, K8s manifest, or CI pipeline security config changes
• go.mod / go.sum dependency changes
• Any file under internal/auth/, internal/crypto/, pkg/security/, or equivalent

When Lite is selected, record: Review depth: Lite (N files changed, no security-sensitive paths). Gates B/C/E skipped per scope policy.

Fast Pass (Lite Only)¶

If ALL conditions are met during Lite triage:

1. All 10 Gate D domains classify as N/A
2. All 11 scenario checklists classify as N/A
3. Secret pattern sweep is clean
4. No constructor/acquisition calls found in Gate A

Then output a condensed report instead of the full Output Contract:

• Review depth + rationale (mandatory)
• Fast Pass: all domains N/A, all scenarios N/A, no findings.
• JSON summary with pass: true and zero counts (mandatory)
• Gate F uncovered risk list (mandatory, may be empty)

This avoids verbose N/A tables for truly benign changes while preserving audit traceability.

Fixed Process + Mandatory Gates¶

The following process is mandatory for Standard and Deep reviews. Lite reviews follow the subset noted above.

1. Scope the change and select review depth.
2. Map trust boundaries.
3. Run scenario checks.
4. Run focused automation checks.
5. Run Gate A: constructor-release pairing audit.
6. Run Gate B: Go resource inventory scan.
7. Run Gate C: third-party lifecycle contract verification.
8. Run Gate D: Go secure-coding 10-domain coverage (for Go repos).
9. Verify exploitability.
10. Run Gate E: second-pass falsification review.
11. Apply suppression filter.
12. Compare with baseline (if available).
13. Report findings first.
14. Provide remediation plan and risk acceptance entries.
15. Provide Gate F: uncovered risk list.

If any mandatory gate cannot be executed, record it under Uncovered Risk List and downgrade confidence where applicable.

Applicability-First Execution (Mandatory)¶

To control review cost and avoid unnecessary depth, execute in two phases:

• Phase 1 (triage): classify each Go domain as Applicable or N/A from changed files + adjacent call paths.
• Phase 2 (deep review): run detailed checks and domain-specific tooling only for Applicable domains.

Rules:

• N/A is allowed only with a one-line reason tied to code evidence.
• Do not mark N/A if there is any trigger signal (relevant imports, touched config, related middleware, DB/crypto/TLS paths, dependency changes).
• Domain-specific reproducer/tests are required only for Applicable domains with findings.

N/A Judgment Examples¶

Anti-pattern: marking a domain N/A when imports or adjacent call paths contain trigger signals (e.g., database/sql imported → Domain 2 must be Applicable).

Mandatory Gate Definitions¶

Gate A: Constructor-Release Pairing (Mandatory)¶

For changed code and immediately related call paths, enumerate and verify pairings for every constructor/acquisition call:

• Constructors/acquisition: New*, Open*, Acquire*, Begin*, Dial*, Listen*, Create*, WithCancel/WithTimeout/WithDeadline.
• Required pairings: Close, Release, Rollback/Commit, Stop, Cancel, or explicit ownership transfer documented in code.

Output requirement:

• Include a short pairing table in analysis notes.
• Any missing or ambiguous pairing is at least P2 unless proven harmless.

Gate B: Go Resource Inventory (Mandatory for Go)¶

Scan all changed code for resource acquisition without matching release. Covers: rows, stmt, tx, conn, file, http.Response.Body, net.Listener, driver objects, goroutine, context cancel, io.Pipe.

Key checks: closed on both success and error paths; no defer inside loops; goroutines have bounded lifecycle; WithTimeout paired with defer cancel().

Reference: See references/go-secure-coding.md § Gate B for the full resource inventory table and anti-patterns.

Gate C: Third-Party Lifecycle Contract Verification (Mandatory)¶

When code uses driver/framework objects with non-obvious lifecycle rules (for example godror, sql extensions, SDK clients):

• Verify lifecycle requirements from primary sources (library source code and/or official docs).
• Cite exactly what contract was used for the decision.
• If no contract can be verified, mark confidence at most suspected and list under Uncovered Risk List.

Gate D: Go Secure-Coding 10-Domain Coverage (Mandatory for Go)¶

For Go repositories, score coverage for these 10 domains:

1. Randomness safety — crypto/rand for secrets; math/rand OK for non-security use.
2. Injection + SQL lifecycle — parameterized SQL, ORDER BY allowlist, rows.Close/Err, Commit/Rollback.
3. Sensitive data handling — mask logs, opaque error messages, response DTO minimization.
4. Secret/config management — no hardcoded secrets, env fail-fast, nolint:gosec with rationale.
5. TLS safety — MinVersion >= TLS1.2, no InsecureSkipVerify in production.
6. Crypto primitives — bcrypt/argon2id for passwords, AEAD for encryption, subtle.ConstantTimeCompare.
7. Concurrency safety — go test -race clean, no TOCTOU in auth/balance, no unsynchronized map access.
8. Go injection sinks — html/template not text/template, exec.Command arg separation, filepath.Join traversal.
9. Static scanner posture — gosec triaged, suppressed nolint has rationale.
10. Dependency posture — govulncheck source-mode reachability.

Execution: D1 triage (Applicable/N/A) → D2 deep review on applicable domains only.

Output: each domain PASS/FAIL/N/A with one-line evidence. Any FAIL with exploitable path becomes a finding.

Reference: See references/go-secure-coding.md § Gate D for detailed checks, code examples, and decision tables per domain.

Gate E: Second-Pass Falsification Review (Mandatory)¶

After first-pass findings, run a dedicated second pass to disprove your own conclusion:

• Ask: "What critical issue would I have missed if first pass over-focused on exploitability class X?"
• Focus on availability, consistency, lifecycle, and partial-failure paths.
• Specifically re-check: transaction boundaries, rollback guarantees, cleanup on error/panic, idempotency race windows.

Output requirement:

• Add a one-line summary in report: Second-pass falsification completed: yes/no.

Gate F: Uncovered Risk List (Mandatory)¶

Always output unresolved coverage gaps to avoid false completeness.

Each item must include:

• Area not covered
• Why not covered (tool/env/access/time)
• Security impact if the gap hides a defect
• Recommended follow-up action and owner suggestion

Anti-Examples (Common Review Mistakes)¶

These are structured examples of review mistakes this skill is designed to prevent. Each shows a wrong approach and the correct alternative.

AE-1: Style Finding Reported as Security Issue¶

Wrong: Reporting P3 — function has 200 lines, hard to review for security as a security finding. Correct: Code complexity is a code quality issue. Only report security findings when there is an exploitable or defense-gap path. If complexity obscures a real vulnerability, report the vulnerability itself with evidence.

AE-3: Over-Reporting False Positives¶

Wrong: P1 — math/rand used in pkg/display/shuffle.go:12 for randomizing quiz question order without checking if the output is security-relevant. Correct: Suppressed — math/rand usage is for display ordering of quiz questions; output is not attacker-exploitable and does not protect a security boundary (Suppression Rule 2).

AE-5: Missing Gate Reported as Full Coverage¶

Wrong: Report says "all gates passed" but go test -race was not run because test suite was unavailable. Correct: Record under Uncovered Risk List: "Gate D7 (Concurrency safety) — go test -race not executed because test suite has build errors. Impact: data races may exist undetected in changed packages. Recommended: fix test build and re-run."

For additional anti-examples (N/A without evidence, confirmed without reproducer, P0 acceptance without escalation, ignoring transitive call paths), see references/anti-examples.md.

Scenario Checklists¶

Run applicable scenarios from the full 11-scenario checklist:

1. Authentication / Authorization — auth on routes, authz before ops, IDOR, token lifecycle.
2. Input Validation / Injection / Uploads — schema rules, parameterized SQL, path traversal, upload controls, Go injection sinks.
3. Session / JWT / Cookie / CSRF — JWT validation, alg=none rejection, cookie flags, CSRF.
4. New Endpoints and Error Surface — authn/authz strategy, error leakage, CORS, idempotency, Go-specific (MaxBytesReader, timeouts).
5. Secrets / Crypto / Key Management — no hardcoded secrets, env-only loading, bcrypt/argon2id, constant-time compare.
6. Payment / Financial Transitions — server-side validation, replay protection, transaction boundaries, balance concurrency.
7. Sensitive Data Storage / Transmission — TLS, field masking, encryption-at-rest, response minimization.
8. Third-Party Integrations — timeout/retry bounds, signature verification, SSRF-safe URL, fail-safe behavior.
9. Supply Chain / Dependency / Build Path — govulncheck, gosec, dependency pinning.
10. Container / Deployment Security — Dockerfile non-root, image pinning, K8s securityContext, NetworkPolicy, CI secret store.
11. Concurrency Safety as Security Risk — TOCTOU, double-spend race, concurrent map crash, go test -race.

Reference: See references/scenario-checklists.md for the full checklist with per-item details and Go-specific subsections.

Focused Automation Gate¶

Run when tools are available; never claim results without running commands.

Execution policy:

• Always run low-cost baseline sweep (rg secrets patterns).
• Run expensive scanners according to Gate D applicability:
• If dependency/module graph changed or third-party risk is Applicable, run govulncheck.
• If security-sensitive Go code changed, run gosec on affected scope (or full repo when scope is unclear).
• If a scanner is skipped because the domain is N/A, record that explicitly in Automation Evidence.

# secret pattern sweep
rg -n "(AKIA[0-9A-Z]{16}|-----BEGIN (RSA|EC|OPENSSH|PRIVATE) KEY-----|ghp_[A-Za-z0-9]{36}|xox[baprs]-|AIza[0-9A-Za-z\-_]{35}|password\s*=|secret\s*=|token\s*=)" .

# Go race detector on changed packages (mandatory for Standard/Deep if tests exist)
go test -race -count=1 ./path/to/changed/...

# Go security scanners
gosec ./...
govulncheck ./...

# Optional cross-check: module exposure view (may include unreachable vulns)
govulncheck -mode=binary ./...

Tool Interpretation Rules (Mandatory)¶

• go test -race: any race detected is at least P2; races on auth/balance/permission state are P1 (CWE-367). Report goroutine stacks from race output.
• gosec: report rule ID, location, and whether finding is exploitable on reachable paths.
• govulncheck source mode: call-trace reachable vulns are high confidence (confirmed/likely).
• govulncheck -mode=binary: treat as exposure signal only; do not mark confirmed without source reachability or equivalent proof.
• Any suppressed nolint:gosec requires explicit rationale review; missing rationale is at least P3 hardening gap.

Language/Framework Extension Hooks¶

Default checklist targets Go services. For non-Go stacks, replace Go-specific Gate D domains with the stack-specific reference. All other gates, scenario checklists, severity model, and output contract remain unchanged. If mixed stack, split findings by module.

Standards Mapping (Mandatory)¶

Include mapping for each finding when applicable:

• CWE-xxx
• OWASP ASVS <section>

If unclear, use Mapping: TBD with reason.

Output Contract¶

Return outputs in this order. Fields are graded MUST / SHOULD / MAY per review depth:

1) Findings (P0 -> P3)¶

Each finding includes:

• Title
• Severity
• Confidence (confirmed/likely/suspected)
• Mapping (CWE / OWASP ASVS)
• File/line
• Exploit path
• Impact
• Minimal reproducer (required for confirmed P0/P1)
• Recommended fix
• Suggested regression/negative test
• Baseline status (new/regressed/unchanged)

One-Shot Finding Example¶

SEC-001: IDOR — Any authenticated user can access other users' orders

• Severity: P1 High
• Confidence: confirmed
• Mapping: CWE-639 (Authorization Bypass Through User-Controlled Key) / OWASP ASVS V4.1.2
• File/line: internal/handler/order.go:47
• Exploit path: GET /api/orders/:id extracts id from URL path and passes it directly to repo.GetOrder(id) without verifying order.UserID == ctx.UserID(). Any authenticated user can read any order by iterating IDs.
• Impact: Full horizontal privilege escalation on order data (PII, payment amounts, addresses).
• Reproducer:

  # User A's token, requesting User B's order
  curl -H "Authorization: Bearer <tokenA>" https://api.example.com/api/orders/ORDER-9999
  # Returns 200 with User B's order details
  

• Recommended fix:

  order, err := h.repo.GetOrder(ctx, orderID)
  if err != nil { ... }
  if order.UserID != auth.UserIDFrom(ctx) {
      return echo.NewHTTPError(http.StatusNotFound, "order not found")
  }
  

  Return 404 (not 403) to avoid confirming the order exists.
• Regression test: Add TestGetOrder_CrossUser_Returns404 — create order as User A, request as User B, assert 404.
• Baseline status: new

2) Go 10-Domain Coverage (Required for Go repos)¶

• Domains 1..10 with PASS/FAIL/N/A
• Applicability per domain (Applicable or N/A with reason)
• One-line evidence per domain (deep evidence required only for Applicable domains)
• Total PASS count and key failed domains

3) Automation Evidence¶

• Command list actually executed
• Key outputs (short)
• Tools skipped/unavailable and reason (including N/A applicability skips)

4) Open questions / assumptions¶

5) Risk Acceptance Register¶

P0 findings MUST NOT be accepted without VP-level or equivalent sign-off; record the approver explicitly. P1 findings require tech-lead-level sign-off.

For each accepted risk entry:

• Finding ID
• Reason for acceptance
• Compensating controls
• Approver (name and role)
• Owner
• Expiry/review date

6) Remediation Plan¶

• Immediate
• Short-term
• Backlog

7) Machine-Readable Summary (JSON)¶

Also output a compact JSON block for CI/inbox ingestion:

{
  "summary": {
    "pass": false,
    "score": "10/14",
    "baseline": "present"
  },
  "counts": {
    "p0": 0,
    "p1": 1,
    "p2": 2,
    "p3": 1
  },
  "changes": {
    "new": 2,
    "regressed": 1,
    "unchanged": 1,
    "resolved": 0
  },
  "go_domains": {
    "required": true,
    "total": 10,
    "pass": 7,
    "fail": 2,
    "na": 1
  },
  "findings": [
    {
      "id": "SEC-001",
      "severity": "P1",
      "confidence": "confirmed",
      "status": "new",
      "cwe": "CWE-639",
      "asvs": "V4",
      "file": "internal/handler/account.go:88"
    }
  ]
}

8) Hardening suggestions¶

9) Uncovered Risk List (Mandatory)¶

Load References Selectively¶

Read only the references needed for the current review. The review depth and language determine what to load.

Lite reviews must NOT load go-secure-coding.md — Gate B/C/E are skipped and domain triage uses only SKILL.md definitions.

Bundled Assets¶