diff --git a/.sdlc/features/user-preferences/audit.md b/.sdlc/features/user-preferences/audit.md new file mode 100644 index 0000000..0441924 --- /dev/null +++ b/.sdlc/features/user-preferences/audit.md @@ -0,0 +1,73 @@ +# Security Audit: User Preferences API + +## Summary +**Overall Assessment: PASS** + +The User Preferences API feature has a small, well-contained attack surface. The implementation follows defensive coding practices: parameterized SQL queries, allowlist-based input validation, and proper error mapping. No critical or high severity findings were identified. Two medium findings and two low/informational findings are noted below. + +## Static Analysis Results + +| Tool | Result | +|------|--------| +| `go vet ./...` | PASS - no warnings or errors | +| `golangci-lint` | Not available in environment (not installed) | + +## OWASP Assessment + +| Category | Status | Notes | +|----------|--------|-------| +| **A01 - Broken Access Control** | ADVISORY | No authentication or authorization on endpoints. Spec explicitly marks auth as out-of-scope, expecting upstream gateway enforcement. See Medium Finding #1. | +| **A02 - Cryptographic Failures** | PASS | No cryptographic operations. No sensitive data stored (theme, language, notifications only). | +| **A03 - Injection** | PASS | All SQL queries use parameterized placeholders (`$1`, `$2`, `$3`). JSONB values marshaled by `encoding/json` and passed as parameters. No string concatenation in queries. | +| **A04 - Insecure Design** | PASS | Hexagonal architecture with clean separation of concerns. Domain validation via allowlists. Deep merge logic is well-tested. | +| **A05 - Security Misconfiguration** | PASS | `.env.example` has `APP_DEBUG=true` and `AUTH_ENABLED=false` but these are example/development defaults, not production config. `.env` files are gitignored. | +| **A06 - Vulnerable Components** | PASS | Uses standard library and well-maintained dependencies (`chi`, `uuid`, `sqlx`). No known vulnerabilities in deps used by this feature. | +| **A07 - Auth Failures** | ADVISORY | Auth is opt-in and not enabled for preference routes. Intentional per spec - see Medium Finding #1. | +| **A08 - Data Integrity** | PASS | All preference values validated against allowlists before persistence. `theme` restricted to `light/dark/system`, `digest` to `daily/weekly/never`, `language` must be non-empty. | +| **A09 - Logging & Monitoring** | PASS | Successful updates are logged with `user_id`. Errors bubble through `app.Wrap()` which handles error logging. No PII is logged (user_id is a UUID, not a name/email). | +| **A10 - SSRF** | PASS | No outbound HTTP requests. No user-controlled URLs. Service only communicates with PostgreSQL. | + +## Critical Findings +None. + +## High Findings +None. + +## Medium Findings + +### M1: No Authentication or Authorization on Endpoints +**Severity:** Medium +**Location:** `internal/api/routes.go:29-30` +**Description:** Both GET and PUT preference endpoints are publicly accessible without authentication. Any caller can read or modify any user's preferences by knowing their UUID. +**Mitigating Factors:** The spec explicitly declares auth as out-of-scope. The design document states this service is a backend store, not a user-facing endpoint, with upstream services/gateways expected to enforce authorization. UUIDs are not guessable. +**Recommendation:** When this service is exposed beyond internal networks, add `auth.Middleware()` to the preference route group. Consider adding authorization checks to ensure the authenticated user matches the `user_id` path parameter. + +### M2: No Request Body Size Limit Explicitly Configured +**Severity:** Medium +**Location:** `cmd/server/main.go`, `internal/api/routes.go` +**Description:** There is no explicit request body size limit configured for the PUT endpoint. While the framework likely has a default, a very large malicious JSON payload could consume memory. +**Mitigating Factors:** The `app.BindAndValidate()` function and the framework's default body limits likely cap this. The strict JSON binding with `validate:"required"` also limits accepted fields. +**Recommendation:** Consider adding an explicit `http.MaxBytesReader` or framework-level body size limit (e.g., 64KB would be more than sufficient for preference payloads). + +## Low/Informational Findings + +### L1: `.env.example` Contains Development Credentials +**Severity:** Low +**Location:** `.env.example:18,21` +**Description:** The `.env.example` file contains `JWT_SECRET=dev-secret-change-in-production` and `DATABASE_URL` with default dev credentials (`dev:dev`). These are clearly marked as development values and `.env` files are properly gitignored. +**Mitigating Factors:** This is a `.env.example` template, not an actual `.env` file. The `.gitignore` properly excludes `.env`, `.env.local`, and `*.env`. +**Recommendation:** No action required. The naming convention is clear. + +### L2: Language Field Validation is Minimal +**Severity:** Low +**Location:** `internal/domain/preferences.go:58-60` +**Description:** The `language` field only validates that it's non-empty. It does not validate against the BCP 47 standard referenced in the spec. While this is not a security vulnerability, it allows arbitrary string values. +**Mitigating Factors:** The value is stored in a JSONB column and never used in SQL construction, templates, or URL building. The risk is limited to data quality, not security. +**Recommendation:** Consider adding BCP 47 format validation if language values are used in locale-sensitive operations downstream. + +## Recommendations + +1. **Before production deployment:** Add authentication middleware to the preference route group and implement authorization checks to verify the requesting user matches the `user_id` in the path. +2. **Optional hardening:** Add explicit request body size limits for the PUT endpoint. +3. **Data quality:** Consider BCP 47 validation for the language field if used for locale operations. +4. **No immediate action required** - the current implementation is secure for its intended use as an internal backend service with external auth enforcement. diff --git a/.sdlc/features/user-preferences/manifest.yaml b/.sdlc/features/user-preferences/manifest.yaml index 4a78c36..67c3f61 100644 --- a/.sdlc/features/user-preferences/manifest.yaml +++ b/.sdlc/features/user-preferences/manifest.yaml @@ -25,7 +25,7 @@ phase_history: entered: 2026-02-09T02:45:34.10464457Z artifacts: audit: - status: pending + status: passed path: audit.md design: status: approved diff --git a/.sdlc/state.yaml b/.sdlc/state.yaml index cc28af8..c7dda42 100644 --- a/.sdlc/state.yaml +++ b/.sdlc/state.yaml @@ -6,9 +6,9 @@ active_work: - slug: user-preferences phase: review blocked: [] -last_updated: 2026-02-09T02:45:34.105623962Z -last_action: TRANSITION -last_actor: cli +last_updated: 2026-02-09T02:47:26.239831583Z +last_action: PASS_ARTIFACT +last_actor: user history: - timestamp: 2026-02-09T02:15:19.934883983Z action: CREATE_FEATURE @@ -105,3 +105,8 @@ history: feature: user-preferences actor: cli result: success + - timestamp: 2026-02-09T02:47:26.239830501Z + action: PASS_ARTIFACT + feature: user-preferences + actor: user + result: success