build: /audit-feature user-preferences

.sdlc/features/user-preferences/audit.md

@@ -0,0 +1,113 @@
+# Security Audit: User Preferences API
+
+## Summary
+
+**Overall Assessment: PASS**
+
+The User Preferences API feature demonstrates a well-structured, security-conscious implementation. No critical or high severity findings were identified. The code follows established hexagonal architecture patterns, uses parameterized SQL queries, validates all input at the domain layer, and enforces authentication and authorization boundaries correctly. A few low-severity observations are noted below for consideration.
+
+## Static Analysis Results
+
+### go vet
+**Status:** Clean - no warnings or errors detected.
+
+```
+$ go vet ./...
+(no output - all clear)
+```
+
+### golangci-lint
+**Status:** Not installed in environment. Manual review performed in lieu of automated linting.
+
+### Tests
+**Status:** All 13 tests pass across 2 test suites (handler and service layers).
+
+## OWASP Assessment
+
+| Category | Status | Notes |
+|----------|--------|-------|
+| **A01 - Injection** | PASS | All SQL queries use parameterized placeholders (`$1`, `$2`). JSONB data is marshaled via Go's `encoding/json` and passed as parameters, not interpolated into SQL strings. No command execution or template rendering present. |
+| **A02 - Broken Authentication** | PASS | JWT authentication enforced via `auth.Middleware()` on all preference endpoints. JWT secret sourced from environment variable (`JWT_SECRET`), not hardcoded. Token validation uses the shared auth package with proper expiry checking. |
+| **A03 - Sensitive Data Exposure** | PASS | No secrets in code. JWT secret read from environment. Preferences contain only non-sensitive data (theme, language, notification toggle). Error messages do not leak internal details. No PII is logged beyond `user_id`. |
+| **A04 - XXE / Insecure Deserialization** | PASS | No XML parsing. JSON unmarshaling uses Go's standard `encoding/json` with typed struct binding (`app.BindAndValidate`). Domain validation rejects unknown keys and invalid value types before persistence. |
+| **A05 - Broken Access Control** | PASS | Every endpoint checks resource ownership via `checkOwnership()` which compares the authenticated user ID (from JWT context) against the `{user_id}` path parameter. Mismatch returns 403 Forbidden. Both GET and PUT perform this check before any data access. |
+| **A06 - Security Misconfiguration** | PASS (with note) | Auth is conditionally enabled via `AUTH_ENABLED` env var (see Medium finding M1). No debug modes or permissive CORS in this service. Health endpoint is correctly excluded from auth. |
+| **A07 - XSS** | N/A | Backend API only - no HTML rendering. JSON responses are safely serialized by Go's standard library. |
+| **A08 - Insecure Components** | PASS | Dependencies are standard Go ecosystem packages (`chi`, `google/uuid`). No known vulnerable dependencies identified. |
+| **A09 - Insufficient Logging** | PASS | Request logging via middleware (`RequestLogger`). Successful preference updates are logged with `user_id`. Auth failures logged by auth middleware. Recovery middleware catches panics. |
+| **A10 - SSRF** | PASS | No user-controlled URLs or outbound HTTP requests. The service only accepts inbound requests and queries PostgreSQL. |
+
+## Critical Findings
+
+None.
+
+## High Findings
+
+None.
+
+## Medium Findings
+
+### M1: Auth Toggle Could Bypass Authentication in Production
+
+**Severity:** Medium
+**Location:** `internal/api/routes.go:32`, `internal/config/config.go:31`
+
+The `AuthEnabled` config flag (controlled by `AUTH_ENABLED` env var) allows completely disabling authentication. If this is accidentally set to `false` or omitted in production, all preference endpoints become unauthenticated. Additionally, `auth.MustGetUser()` in the handler's `checkOwnership()` would panic when no user is in context, relying on the recovery middleware to return a 500 error.
+
+**Mitigation:** This is a common pattern in this codebase for local development convenience. Ensure production deployment configurations explicitly set `AUTH_ENABLED=true` and that infrastructure safeguards (e.g., deployment manifests, CI checks) validate this setting.
+
+## Low Findings
+
+### L1: No Request Body Size Limit
+
+**Severity:** Low
+**Location:** `internal/api/handlers/preferences.go:76`
+
+The PUT endpoint does not explicitly limit request body size. While the domain layer restricts preferences to 3 known keys, a malicious client could send a very large JSON body before validation occurs. The `app.BindAndValidate()` function may rely on framework defaults for body size limits.
+
+**Recommendation:** Verify that the `app` package or middleware enforces a reasonable body size limit (e.g., 1MB). If not, consider adding `http.MaxBytesReader` at the handler or middleware level.
+
+### L2: No Rate Limiting on Preference Updates
+
+**Severity:** Low
+**Location:** `internal/api/routes.go:42-43`
+
+There is no rate limiting on the PUT endpoint. A compromised or malicious client with a valid JWT could rapidly update preferences, generating unnecessary database writes.
+
+**Recommendation:** This is acknowledged as out of scope in the spec. Rate limiting should be handled at the infrastructure level (API gateway/ingress) rather than per-service.
+
+### L3: User ID Logged on Update Without Sanitization Context
+
+**Severity:** Low
+**Location:** `internal/service/preferences.go:55`
+
+The `user_id` is logged on successful update. While UUIDs are safe to log, the logging statement doesn't include the request ID for correlation. The middleware-level request logger handles this via context, but the service-level log entry may lack request tracing context depending on the logging configuration.
+
+**Recommendation:** Ensure structured logging propagates request IDs through context automatically (verify `logging.Logger` behavior).
+
+## Recommendations
+
+1. **Ensure `AUTH_ENABLED=true` in production** - Add a CI/deployment check that validates auth is enabled in production configurations.
+2. **Verify request body size limits** - Confirm the `app.BindAndValidate()` or middleware stack enforces a maximum request body size.
+3. **Add infrastructure-level rate limiting** - Configure rate limits at the API gateway for the preferences endpoints.
+4. **Consider domain-level unit tests** - The domain validation logic (`ValidatePreferences`, `ValidatePreferenceKey`, `ValidatePreferenceValue`) has no dedicated unit tests. While covered transitively through service tests, direct domain tests would improve coverage and serve as documentation.
+
+## Files Reviewed
+
+| File | Purpose |
+|------|---------|
+| `cmd/server/main.go` | Service entry point and wiring |
+| `internal/config/config.go` | Configuration loading |
+| `internal/domain/preferences.go` | Domain entity and validation |
+| `internal/domain/errors.go` | Domain error definitions |
+| `internal/port/preferences.go` | Repository interface |
+| `internal/adapter/postgres/preferences.go` | PostgreSQL repository |
+| `internal/service/preferences.go` | Business logic |
+| `internal/service/preferences_test.go` | Service unit tests |
+| `internal/api/handlers/preferences.go` | HTTP handlers |
+| `internal/api/handlers/preferences_test.go` | Handler integration tests |
+| `internal/api/handlers/health.go` | Health check handler |
+| `internal/api/routes.go` | Route registration |
+| `internal/api/spec.go` | OpenAPI specification |
+| `migrations/001_create_user_preferences.sql` | Database migration |
+| `migrations/migrations.go` | Embedded migration FS |

.sdlc/features/user-preferences/manifest.yaml

@@ -20,7 +20,7 @@ phase_history:
       entered: 2026-02-08T10:07:33.557335602Z
 artifacts:
     audit:
-        status: pending
+        status: passed
         path: audit.md
     design:
         status: approved

.sdlc/state.yaml

@@ -7,8 +7,8 @@ active_work:
           branch: feature/user-preferences
           phase: implementation
 blocked: []
-last_updated: 2026-02-08T10:51:22.657760934Z
-last_action: NEEDS_FIX_ARTIFACT
+last_updated: 2026-02-08T10:54:38.222285365Z
+last_action: PASS_ARTIFACT
 last_actor: user
 history:
     - timestamp: 2026-02-08T09:52:56.804287195Z
@@ -111,3 +111,8 @@ history:
       feature: user-preferences
       actor: user
       result: success
+    - timestamp: 2026-02-08T10:54:38.222283963Z
+      action: PASS_ARTIFACT
+      feature: user-preferences
+      actor: user
+      result: success