Security Checks

---
name: security-checks
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
reads:
  - docs/security-principles.md
metadata:
  version: "1.1"
  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`](../tdd-workflow/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`](../../../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)
- [ ] No request-supplied text in Thymeleaf preprocessing (`__${...}__`) or other template-expression evaluation
- [ ] XML parsing disables external entity resolution (XXE)

### 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

### Deserialization Safety
- [ ] Jackson configured with safe defaults (no polymorphic deserialization)
- [ ] No `@JsonTypeInfo` annotations that enable arbitrary class instantiation
- [ ] YAML parsing uses SnakeYAML safe loading (no arbitrary-type construction)
- [ ] 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 — search the diff; `token`, `password`, `secret`, `key` are the starting set, not the list. Secrets take many names; the project's security brief and its trust-boundary map define what counts. Judge every hit in context.
- [ ] 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)
- [ ] Security-relevant randomness comes from `SecureRandom`, never `java.util.Random`

### 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
- [ ] A request-bound object is form-scoped: a `@ModelAttribute` or `@RequestBody` target carries only the fields the form edits, or the handler's `@InitBinder` sets an allow-list. A persisted type is never bound whole from a request that edits part of it (mass assignment)
- [ ] Every request-bound object that reaches a save carries `@Valid`, so the form's constraints hold on each path that persists it

### 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

### Pattern Consistency

Security as an emergent property (§ Core Security Principles) implies one way per concern: when two implementations secure the same concern differently, the divergence hides whichever one is wrong.

- [ ] A concern the codebase already secures (escaping, validation, resource handling) is secured the same way here
- [ ] Divergence from the neighboring implementation of the same concern carries an inline justification; unjustified divergence is a finding, even without its own exploit path
- [ ] Consistency judges how a secured concern is secured, never whether an unsecured one passes. Extending a pre-existing weakness to a new path is a finding; its description names the existing scope, and the new reach sets its severity
- [ ] On a fix round, the fix this review asked for stays on the slice's routes. A delta on a route or flow the slice's bullets do not name is `clarify` to `product-requirements-expert` on `changes_requested`, never approved through (review-workflow tag rule)
- [ ] A removed or weakened check — an auth annotation, an ownership test, a validation, an escaping call — is a finding unless the diff replaces it with an equal or stronger control

## 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 sweep in § Detection Patterns: 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.

Reachability is rated from the attacker path — the input, the boundary it crosses, the operation it reaches — so a `blocked` finding states that path concretely. A finding whose path stays conjectural is still reported, at the severity its demonstrated reach supports — never `blocked`.

### 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/`) |
| `@ModelAttribute\|@RequestBody\|setAllowedFields\|setDisallowedFields` in `src/main/java/` | Request binding targets and their field allow-lists |

## Supply Chain Verification

Run the dependency check when the project configures it:

```bash
./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. State that in one clause of an `approved_aspects` entry, naming the framework versions (Spring Boot, Jackson) read from the `dependencies` output; raise no finding and no recommendation for it. An unconfigured scanner is the project's standing gap, recorded once in its security brief, never restated per review: the grader reads a recommendation as a reservation against this change. Report only checks that actually ran — an un-run check is "not run", never clean.