diff --git a/SECURITY_AUDIT.md b/SECURITY_AUDIT.md new file mode 100644 index 0000000..220c581 --- /dev/null +++ b/SECURITY_AUDIT.md @@ -0,0 +1,111 @@ +# Security Audit — Movie Night + +**Date:** 2026-03-15 +**Auditor:** Claude (AI-assisted review) +**Scope:** Full codebase review (24 source files) +**Context:** Currently exposed only on Tailscale tailnet. Planning to expose via Pangolin to the internet — these findings should be resolved before that transition. + +--- + +## Critical / High + +### 1. Shared search links bypass authentication entirely +- **Location:** `app/routers/mood.py:187-207` +- **Issue:** `GET /api/history/shared/{entry_id}` is fully public. Sequential integer IDs make enumeration trivial. Anyone can view mood text, movie recommendations, and viewing patterns. +- **Fix:** Add a cryptographic share token (e.g. `secrets.token_urlsafe(16)`) stored alongside the history entry. Require the token as a query parameter to access the shared endpoint. + +### 2. Prompt injection via mood text +- **Location:** `app/services/llm/base.py:31-48` +- **Issue:** User-supplied `mood` is interpolated directly into the LLM prompt with no sanitization or length limit. A crafted input could manipulate rankings or attempt to extract the system prompt. +- **Mitigating factor:** The title/ID cross-validation in `app/routers/mood.py:87-94` is good defense-in-depth and limits the blast radius. +- **Fix:** (a) Add a max length on mood input (~500 chars). (b) Strip or escape prompt-like patterns. (c) Add an explicit instruction in the system prompt to ignore meta-instructions in user text. + +--- + +## Medium + +### 3. Container runs as root +- **Location:** `Dockerfile` +- **Issue:** No `USER` directive. The app runs as root inside the container. +- **Fix:** Add `RUN useradd -m -u 1000 appuser` and `USER appuser`. + +### 4. Insecure default session secret +- **Location:** `app/config.py` +- **Issue:** `session_secret` defaults to `"change-me-in-production"`. If `.env` is misconfigured, sessions are trivially forgeable. +- **Fix:** Remove the default and make it a required field (pydantic will error on startup if missing). + +### 5. No rate limiting on login +- **Location:** `app/routers/auth.py` +- **Issue:** `/api/auth/login` has no rate limiting, enabling brute-force attacks against Jellyfin credentials. +- **Fix:** Add a simple in-memory rate limiter (e.g. `slowapi` or a manual counter per IP with a 5-attempt/minute window). + +### 6. No security headers +- **Issue:** No `Content-Security-Policy`, `X-Frame-Options`, `X-Content-Type-Options`, or `Strict-Transport-Security` headers are set by the application or documented for the reverse proxy. +- **Fix:** Add a FastAPI middleware or document the required headers for the nginx/reverse proxy config. + +### 7. Poster proxy has no input validation or auth +- **Location:** `app/main.py:43-52` +- **Issue:** `item_id` is passed directly to Jellyfin with no format validation and no authentication required. This leaks library contents and is a mild SSRF vector. +- **Fix:** Validate `item_id` matches `^[a-f0-9]+$` (Jellyfin ID format) and require authentication. + +### 8. Arbitrary `additional_user_ids` accepted +- **Location:** `app/routers/mood.py:31` +- **Issue:** The client can pass any user IDs to filter watch state. No server-side validation that the IDs belong to the household. +- **Fix:** Maintain an allowlist of valid Jellyfin user IDs (or fetch them server-side) instead of trusting client input. + +### 9. Database directory permissions not set +- **Location:** `app/database.py:8-9` +- **Issue:** `os.makedirs` uses default umask. Other container processes could read/write the DB. +- **Fix:** `os.makedirs(..., mode=0o700, exist_ok=True)` + +### 10. Sync endpoint has no rate limit +- **Location:** `app/routers/library.py:58-62` +- **Issue:** Any authenticated user can spam `POST /api/library/sync`, spawning unbounded background tasks. +- **Fix:** Track last sync time and reject requests within a cooldown window (e.g. 10 minutes). + +--- + +## Low (easy wins) + +| # | Finding | Location | Fix | +|---|---------|----------|-----| +| 11 | No mood text length limit | `app/routers/mood.py:25` | Add `max_length=500` to Pydantic model | +| 12 | No auth event logging | `app/routers/auth.py` | Log failed/successful logins with IP + timestamp | +| 13 | No search history retention policy | `app/routers/mood.py` | Add periodic cleanup (e.g. 90-day TTL) | +| 14 | Tailwind loaded from CDN | `app/static/index.html:7` | Consider Subresource Integrity (`integrity=` attribute) | +| 15 | No `.dockerignore` for `.env` | `.dockerignore` | Ensure `.env` is listed | + +--- + +## What's done well + +- **Parameterized SQL everywhere** — no injection vectors found +- **Secure session cookies** — `HttpOnly`, `SameSite=Lax`, `Secure` flag, 32-byte random IDs +- **LLM output validation** — title/ID cross-check prevents hallucinated recommendations +- **Dependencies are current** — no known CVEs in pinned versions +- **No secrets in source** — `.env` is gitignored, `.env.example` is clean + +--- + +## Resolution priority + +**Must fix before internet exposure (via Pangolin):** +1. Shared search link auth (#1) +2. Container runs as root (#3) +3. Session secret default (#4) +4. Rate limiting on login (#5) +5. Security headers (#6) +6. Poster proxy validation (#7) +7. Mood text length limit (#11) + +**Should fix soon after:** +- Prompt injection hardening (#2) +- User ID validation (#8) +- DB permissions (#9) +- Sync rate limit (#10) +- Auth logging (#12) + +**Nice to have:** +- History retention (#13) +- Tailwind SRI (#14) +- Dockerignore check (#15) diff --git a/errors/AISelect_20260314_211804_Chrome.jpg.jpeg b/errors/AISelect_20260314_211804_Chrome.jpg.jpeg new file mode 100644 index 0000000..087b4f1 Binary files /dev/null and b/errors/AISelect_20260314_211804_Chrome.jpg.jpeg differ diff --git a/errors/AISelect_20260314_211823_Chrome.jpg.jpeg b/errors/AISelect_20260314_211823_Chrome.jpg.jpeg new file mode 100644 index 0000000..b36e71c Binary files /dev/null and b/errors/AISelect_20260314_211823_Chrome.jpg.jpeg differ diff --git a/errors/AISelect_20260314_211840_Chrome.jpg.jpeg b/errors/AISelect_20260314_211840_Chrome.jpg.jpeg new file mode 100644 index 0000000..a26f146 Binary files /dev/null and b/errors/AISelect_20260314_211840_Chrome.jpg.jpeg differ