From c69b0e131dd753ce73affbe309354f5243d74671 Mon Sep 17 00:00:00 2001 From: rdev-worker Date: Mon, 9 Feb 2026 01:53:51 +0000 Subject: [PATCH] build: /audit-feature user-preferences --- .sdlc/features/user-preferences/audit.md | 100 ++++++++++++++++++ .sdlc/features/user-preferences/manifest.yaml | 2 +- .sdlc/state.yaml | 11 +- 3 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 .sdlc/features/user-preferences/audit.md diff --git a/.sdlc/features/user-preferences/audit.md b/.sdlc/features/user-preferences/audit.md new file mode 100644 index 0000000..0fe9cac --- /dev/null +++ b/.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 | diff --git a/.sdlc/features/user-preferences/manifest.yaml b/.sdlc/features/user-preferences/manifest.yaml index f5106db..dfd3a28 100644 --- a/.sdlc/features/user-preferences/manifest.yaml +++ b/.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 diff --git a/.sdlc/state.yaml b/.sdlc/state.yaml index 400d03a..5135266 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-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