movie-night/SECURITY_AUDIT.md

# 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)