security-review

name	security-review
description	Security review checklists, threat model, severity classification, and supply chain verification for Java/Spring Boot applications. Load when conducting security reviews.
compatibility	["claude-code","github-copilot","opencode","junie-cli"]
metadata	{"version":"1.0","author":"team"}

Core Security Principles

This review enforces four non-negotiable laws: security as an emergent property, defense in depth, least privilege, fail secure. They are harness-owned, defined in tdd-principles.md § Secure by Design. How this project meets them — its trust boundaries and the stack's high-bar defaults — lives in the project-owned docs/security-principles.md, the same brief the feature-implementer designs against. Read both before reviewing and enforce what they say, not remembered defaults. This skill holds the exhaustive checklist that turns the laws and defaults into specific, gradeable items.

Security Checklist

Path Traversal and File Operations

• Input paths resolved to absolute path before use
• No directory traversal via crafted input (../, symlinks)
• File operations restricted to configured directories
• Symlinks not followed (or explicitly validated)
• Output files written only to expected locations
• No file writes outside the designated directory tree
• Agent/development temp files only in .scratch/tmp/, never system /tmp

Input Injection

• User-derived content escaped before inclusion in output (HTML, JSON, etc.)
• No inputs passed to shell commands or Runtime.exec() / ProcessBuilder
• No inputs used in string interpolation for SQL, LDAP, or other query languages
• Regex patterns bounded (no ReDoS via catastrophic backtracking)
• Log injection prevented (newlines stripped/escaped in log values)

HTML Output Safety (if applicable)

• All user-derived content escaped before HTML insertion
• <, >, &, ", ' properly escaped in text content and attributes
• No inline JavaScript in generated HTML
• No external resource loading (<script>, <link>, <img> with remote URLs)
• href attributes use relative paths only, not javascript: or data: URIs

JSON Safety

• Jackson configured with safe defaults (no polymorphic deserialization)
• No @JsonTypeInfo annotations that enable arbitrary class instantiation
• Corrupted data files handled gracefully (not crash)
• JSON parsing uses safe defaults

Credential and Sensitive Data Handling

• Tokens never logged (even at debug level)
• Credentials not hardcoded in source
• Credentials loaded from environment/config, not CLI args (ps shows args)
• Sensitive data not included in error messages
• No credentials in URLs (use headers instead)

Input Validation

• Paths validated (exists, correct type, readable/writable)
• Date strings validated before parsing
• Configuration values validated at startup
• No integer overflow in size or count handling

Network Security (if applicable)

• Connection timeouts set on all HTTP operations
• No hardcoded URLs
• TLS configuration appropriate for deployment context
• Responses from external services treated as untrusted

Resource Management

• No unbounded memory allocation
• File handles properly closed (try-with-resources)
• Stream operations do not hold references to large collections
• Graceful behavior under high load

Dependency Security

• Framework versions checked for known CVEs
• Jackson version checked for deserialization vulnerabilities
• No unnecessary dependencies in build.gradle
• Dependencies from approved sources only (see docs/system-design.md)

Logging Safety

• No sensitive data in log output
• SLF4J parameterized logging (no string concatenation that evaluates eagerly)
• No System.out.println or System.err.println
• Log messages include sufficient context for debugging

Java-Specific Security Checks

Concurrency Safety

• Singleton beans hold no unsynchronized mutable state (one instance serves every request)
• Shared collections use concurrent types or stay thread-confined
• Non-thread-safe classes (SimpleDateFormat) are not shared across threads
• Executors are bounded and shut down on close

Error Handling

• Security-relevant exceptions (auth, validation, integrity) surface to callers, never caught and dropped
• Exception messages don't leak internal details to external callers
• Cause chains preserved for internal debugging
• API boundaries return controlled errors, not raw stack traces

Type Safety

• No raw generic types; casts guarded by instanceof
• Null contracts explicit at boundaries; no unchecked Optional.get()
• Collection and array bounds guarded before access
• Reflection and deserialization of untrusted classes justified, or absent

IDE-Assisted Checks (optional)

When an IDE semantic oracle is available, use it to complement (never replace) the Grep patterns above: check the resolved dependency set for the Dependency Security checklist, and answer access-control / route-exposure questions by resolving security-relevant symbols and their references rather than text-matching config. The latter is required, not optional: when the oracle is connected, an access-control / route-exposure claim that turns on how a symbol or its references resolve (e.g. "this endpoint is the only unauthenticated caller", "the filter chain covers this route") must cite the search_symbol / get_symbol_info call that backs it (see intellij-idea § Cite the call that backs a claim) — without the oracle, cite the grep and label it the weaker basis. The resolved-dependency check stays an accelerator; a client without an oracle relies on Grep alone. Tool mechanics — and the Actuator alternative for the live bean/route graph — live in the intellij-idea skill.

Severity Classification

Rate by reachability and the harm an attacker gains, not by which bucket the issue's name suggests. Severity drives the blocked gate, so a reachable medium outranks an unreachable critical.

CRITICAL (BLOCKED)

• Credential exposure in logs or errors
• Remote code execution vectors (unsafe Jackson deserialization, shell injection)
• Authentication bypass
• Unvalidated external input to sensitive operations

HIGH (BLOCKED)

• Path traversal allowing writes outside designated directories
• Missing input validation on external data
• Unbounded memory allocation (response size limits)
• File handles leaked (no try-with-resources)
• ReDoS-vulnerable regex patterns

MEDIUM

• Sensitive data in verbose error messages
• Missing timeouts on network operations
• Missing Content-Security-Policy in generated HTML
• Verbose logging in production default

LOW

• Information disclosure in health endpoints
• Missing rate limiting
• Dependency not on latest patch version

Detection Patterns

Use Grep to search for dangerous code patterns during review:

Pattern	What It Detects
append|concat|format|printf|+.*html|\.write( in src/main/java/	Unescaped output
Runtime|ProcessBuilder|exec( in src/main/java/	Shell execution
enableDefaultTyping|JsonTypeInfo|WRAPPER_ARRAY in src/main/java/	Unsafe Jackson config
Files\.|FileWriter|FileOutputStream|BufferedWriter in src/main/java/	File operations
followLinks|NOFOLLOW in src/main/java/	Symlink handling
/tmp/ in src/main/java/	System tmp usage (should use .scratch/tmp/)

Supply Chain Verification

Run the dependency check when the project configures it:

./gradlew dependencyCheckAnalyze    # OWASP Dependency-Check, if configured
./gradlew dependencies              # resolved dependency tree

dependencyCheckAnalyze matches resolved artifacts against the NVD; judge each finding by reachability per § Severity Classification, not by raw score. Without the plugin, no NVD match runs in this review — the reviewer has no network access. Instead, read the framework versions (Spring Boot, Jackson) from the dependencies output and report them under a "not verified against the NVD" finding, so a human or CI closes the check. Report only checks that actually ran — an un-run check is "not run", never clean.