build: /audit-feature user-preferences
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
This commit is contained in:
parent
c1d08d327b
commit
e4f397b482
73
.sdlc/features/user-preferences/audit.md
Normal file
73
.sdlc/features/user-preferences/audit.md
Normal file
@ -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.
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user