build: /audit-feature user-preferences

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

@@ -0,0 +1,100 @@
+# Security Audit: User Preferences API
+
+## Summary
+
+**Overall Assessment: PASS**
+
+The User Preferences API feature demonstrates solid security practices. No critical or high-severity findings were identified. The implementation follows secure coding patterns throughout: parameterized SQL queries, strict input binding with unknown field rejection, proper authentication gating, and correct authorization checks with ownership verification. A small number of medium and low-severity findings relate to the auth toggle pattern and lack of rate limiting, both of which are platform-level concerns rather than feature-specific vulnerabilities.
+
+## Static Analysis Results
+
+| Tool | Result |
+|------|--------|
+| `go vet ./...` | **PASS** — No issues found |
+| `golangci-lint` | Not available in environment; skipped |
+
+## OWASP Assessment
+
+| Category | Status | Notes |
+|----------|--------|-------|
+| **A01 - Broken Access Control** | PASS | Self-access enforced for writes (`user.ID != userID` → 403). Admin read-only via `HasRole("admin")`. Tests cover self-access, admin read, admin write denial, and cross-user denial. |
+| **A02 - Cryptographic Failures** | PASS | No custom cryptography. JWT validation delegated to `pkg/auth` framework. No sensitive data stored beyond user ID. |
+| **A03 - Injection** | PASS | All SQL uses parameterized queries (`$1`, `$2` placeholders). No string interpolation in SQL. No command execution. No template rendering. |
+| **A04 - Insecure Design** | PASS | Hexagonal architecture cleanly separates concerns. Domain validation is pure and testable. Authorization logic is explicit in handlers. |
+| **A05 - Security Misconfiguration** | INFO | `AUTH_ENABLED=false` default in `.env.example` — auth is opt-in via environment toggle. Acceptable for local dev, but must be `true` in production. See Medium Finding M1. |
+| **A06 - Vulnerable Components** | PASS | Standard library + well-known dependencies (`go-chi`, `lib/pq`). No known vulnerable components. |
+| **A07 - Auth Failures** | PASS | Auth middleware applied to both endpoints via route group. Unauthenticated requests rejected with 401 when auth is enabled. |
+| **A08 - Data Integrity** | PASS | Strict JSON binding (`BindAndValidateStrict`) rejects unknown fields. Struct tag validation constrains enum values. Domain-layer validation provides defense in depth. |
+| **A09 - Logging & Monitoring** | PASS | Preference updates logged with `user_id`. DB errors logged server-side, generic message returned to client. No PII logged beyond user ID. |
+| **A10 - SSRF** | PASS | No outbound HTTP requests. No user-controlled URLs. |
+
+## Critical Findings
+
+None.
+
+## High Findings
+
+None.
+
+## Medium Findings
+
+### M1: Auth Toggle Defaults to Disabled
+
+**Location:** `internal/config/config.go:31`, `.env.example:17`
+
+**Description:** Authentication is controlled by the `AUTH_ENABLED` environment variable, which defaults to `false`. When auth is disabled, the preference endpoints are accessible without any authentication, meaning any client can read/write any user's preferences. While the handler still checks `auth.GetUser(ctx)` and returns 401 if nil, the auth middleware that populates the user context is not applied when `AUTH_ENABLED=false`.
+
+**Risk:** If deployed to production without explicitly setting `AUTH_ENABLED=true`, all preference endpoints would be unprotected.
+
+**Mitigation:** This is a platform-level pattern used across all services for local development. Production deployment pipelines should enforce `AUTH_ENABLED=true`. Consider adding a startup warning when auth is disabled in non-development environments.
+
+### M2: No Rate Limiting on Preference Updates
+
+**Description:** The PUT endpoint has no rate limiting. An authenticated user could make unlimited update requests, potentially causing excessive database writes.
+
+**Risk:** Low probability given auth requirement, but could enable denial-of-service against the database from a compromised account.
+
+**Mitigation:** Platform-level rate limiting (e.g., at ingress/API gateway) is the appropriate layer for this. Not a feature-specific concern, but worth noting.
+
+## Low Findings
+
+### L1: Dev Secret in .env.example
+
+**Location:** `.env.example:18`
+
+**Description:** The `.env.example` file contains `JWT_SECRET=dev-secret-change-in-production`. While `.env.example` is a template and not used in production, the presence of a weak secret value could be accidentally copied to production `.env` files.
+
+**Mitigation:** Standard practice for example files. The value is clearly labeled as a dev placeholder. Production secrets should be managed via secret management infrastructure, not `.env` files.
+
+### L2: User ID Logged on Preference Update
+
+**Location:** `internal/service/preferences.go:52`
+
+**Description:** `s.logger.Info("preferences updated", "user_id", userID)` logs the user ID on every preference update. User IDs are not PII by themselves (they are opaque identifiers), but could be used for correlation.
+
+**Mitigation:** This is appropriate audit logging. No remediation needed.
+
+## Recommendations
+
+1. **Ensure production deployment enforces `AUTH_ENABLED=true`** — Validate this in CI/CD pipeline or deployment config (platform concern, not feature-specific).
+2. **Consider platform-level rate limiting** — Apply rate limits at the API gateway layer for all write endpoints.
+3. **No feature-specific changes required** — The implementation follows secure patterns correctly.
+
+## Files Audited
+
+| File | Lines | Purpose |
+|------|-------|---------|
+| `internal/domain/preferences.go` | 89 | Domain types, validation, defaults |
+| `internal/domain/errors.go` | 18 | Domain error definitions |
+| `internal/port/preferences.go` | 18 | Repository interface |
+| `internal/adapter/memory/preferences.go` | 50 | In-memory adapter (tests) |
+| `internal/adapter/postgres/preferences.go` | 95 | PostgreSQL adapter with parameterized queries |
+| `internal/service/preferences.go` | 56 | Business logic service |
+| `internal/service/preferences_test.go` | 174 | Service unit tests |
+| `internal/api/handlers/preferences.go` | 154 | HTTP handlers with auth checks |
+| `internal/api/handlers/preferences_test.go` | 321 | Handler tests including auth scenarios |
+| `internal/api/routes.go` | 49 | Route registration with auth middleware |
+| `internal/api/spec.go` | 79 | OpenAPI specification |
+| `internal/config/config.go` | 34 | Service configuration |
+| `cmd/server/main.go` | 79 | Application wiring |
+| `.env.example` | 21 | Environment template |

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

@@ -25,7 +25,7 @@ phase_history:
       entered: 2026-02-09T01:51:09.30914694Z
 artifacts:
     audit:
-        status: pending
+        status: passed
         path: audit.md
     design:
         status: approved

.sdlc/state.yaml

@@ -6,9 +6,9 @@ active_work:
         - slug: user-preferences
           phase: review
 blocked: []
-last_updated: 2026-02-09T01:51:09.310136372Z
-last_action: TRANSITION
-last_actor: cli
+last_updated: 2026-02-09T01:53:39.766199074Z
+last_action: PASS_ARTIFACT
+last_actor: user
 history:
     - timestamp: 2026-02-08T18:17:02.968351745Z
       action: CREATE_FEATURE
@@ -120,3 +120,8 @@ history:
       feature: user-preferences
       actor: cli
       result: success
+    - timestamp: 2026-02-09T01:53:39.766198102Z
+      action: PASS_ARTIFACT
+      feature: user-preferences
+      actor: user
+      result: success