phase: 82_security_headers
All completion criteria verified green — no defects found, nothing to fix. Final report: **Phase 82 (security headers) — final verification pass: all green** - Verified prior-run implementation: `app/core/security_headers.py` (pure-ASGI, header-only, exact A1 CSP), registration in `app/main.py` after `configure_caching` (outermost), unit/integration/E2E suites. - Deviation confirmed sound: `data:`-URI favicon (blocked by locked CSP) → static `frontend/assets/favicon.svg` in 5 templates + Containerfile `cp`; SVG element byte-identical to the old data-URI (verified programmatically); serves 200 with all three headers. - Curl check (server booted like e2e conftest, log: `/tmp/curl_security_headers_final.log`): `/`, `/api/health`, `/assets/styles.css`, `/nope` (404) → all three headers, CSP exactly `default-src 'self'; base-uri 'none'; frame-ancestors 'none'`. - `uv run pytest tests/unit/test_security_headers.py tests/integration/test_security_headers.py -v --no-cov` → 13 passed (incl. SSE byte-identity pin). - `uv run pytest tests/e2e/test_security_headers.py -v --no-cov` (isolated) → 2 passed (headers + zero CSP violations + painted page). - SSE tripwire `uv run pytest tests/e2e/test_chat_rag.py -v --no-cov` → 3 passed. - `uv run pytest --cov=app --cov-report=term-missing` → 1665 passed, app/ 99% (>90%); `uv run ruff check . && uv run pyright` → clean (0 errors). - `git diff --stat` limited to phase-82 files + the two documented deviations (favicon set, `tests/unit/__init__.py`); no `pyproject.toml`/`uv.lock`/JS diffs. - Commit + phase-dir move left to the harness per pipeline rules (not executed by me). Next pending phase: `83_chat_save_payload_limits`.
This commit is contained in:
@@ -1,71 +0,0 @@
|
||||
# Phase 82 — Security headers on every response: strict CSP, no-framing, nosniff
|
||||
|
||||
**Source:** `.agents/remediation_plan.md` SEC-04 (security audit 2026-09-07, severity Medium — "No security headers (CSP, X-Frame-Options / frame-ancestors)")
|
||||
**Story:** n/a (security hardening — audit-derived, no user story)
|
||||
**Context:** no response in the app carries `Content-Security-Policy`, `X-Frame-Options`, or `X-Content-Type-Options` (checked `app/main.py`, `app/core/caching.py`, all templates — none do). The frontend is No-CDN and was verified for this phase: **no inline `<style>` blocks, no `style="…"` attributes, no `onclick`/`on*` handlers, no `javascript:` URLs, no external hosts** in any `frontend/*.html` or the JS-injected markup (inline `<svg>` elements are CSP-legal; the two `el.style.…` CSSOM writes in `app.js` are not CSP-blocked). `app/main.py` is the single app-assembly point (middleware added in order: `SessionMiddleware` → `configure_caching`'s `CachingMiddleware` → static mount last). `app/core/caching.py` is the existing transport-layer middleware precedent (but it BUFFERS page bodies — this phase's middleware must not).
|
||||
|
||||
## Objective
|
||||
Every response — pages, static assets, API JSON, the SSE chat stream, even 404s — carries a strict same-origin CSP (`frame-ancestors 'none'` included → clickjacking closed), `X-Frame-Options: DENY`, and `X-Content-Type-Options: nosniff`, added by a header-only pure-ASGI middleware that never touches a body (the SSE stream passes through byte-identical).
|
||||
|
||||
## Audit basis (read this, not the chat)
|
||||
- Every page (including the admin sign-in and the admin-only views) can be embedded in a third-party page's iframe: a malicious LAN page can overlay the admin UI and trick the signed-in owner into clicking Sync / Revoke / Delete-source actions (audit PoC: `<iframe src="http://<server>:8000/git-sources.html">` with a benign-looking overlay). `frame-ancestors 'none'` (in the CSP) + `X-Frame-Options: DENY` (legacy fallback) close it.
|
||||
- The app serves no CSP today: the codebase is XSS-clean (escape-first renderer, `textContent` for data — audit-verified at every `innerHTML` site), but the CSP is the standard second line of defense against any future regression. It is cheap here precisely because of the No-CDN + no-inline-markup verification above: `default-src 'self'` is sufficient without any `'unsafe-inline'` — a genuinely strict policy.
|
||||
- `nosniff` is the one-line belt-and-braces against MIME-confusion on the static mount.
|
||||
|
||||
## Owner decisions (chat, 2026-09-07 — recorded per AGENTS.md rule 3)
|
||||
- **A1 — the exact policy:** `Content-Security-Policy: default-src 'self'; base-uri 'none'; frame-ancestors 'none'` — no `script-src`/`style-src`/`connect-src` entries (they inherit `default-src 'self'`); no `'unsafe-inline'` anywhere (verified unnecessary); no `report-uri`/`report-to` (no collector in the homelab — a report would just vanish).
|
||||
- **A2 — header-only, pure ASGI:** a small pure-ASGI middleware (not `BaseHTTPMiddleware`) — it wraps `send` and writes headers on `http.response.start` only; no body drain, no buffering, so the SSE chat stream and every other response pass through byte-identical (the explicit contrast with `CachingMiddleware`, which does buffer page bodies).
|
||||
- **A3 — outermost position:** registered in `app/main.py` AFTER `configure_caching(app)` (last `add_middleware` = outermost), so even responses built by the static catch-all / 404 handler carry the headers.
|
||||
- **A4 — the three headers only:** CSP + `X-Frame-Options: DENY` + `X-Content-Type-Options: nosniff`. No `Referrer-Policy`, no `Permissions-Policy`, no HSTS (the app serves plain homelab HTTP — HSTS would be wrong here; TLS is the documented non-goal).
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
- **`app/core/security_headers.py` (new)**:
|
||||
```python
|
||||
CSP = "default-src 'self'; base-uri 'none'; frame-ancestors 'none'"
|
||||
|
||||
class SecurityHeadersMiddleware:
|
||||
"""Header-only, pure-ASGI — never reads or buffers a body (SSE-safe)."""
|
||||
def __init__(self, app): self.app = app
|
||||
async def __call__(self, scope, receive, send):
|
||||
if scope["type"] != "http":
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
async def send_wrapper(message):
|
||||
if message["type"] == "http.response.start":
|
||||
headers = MutableHeaders(scope=message)
|
||||
headers["Content-Security-Policy"] = CSP
|
||||
headers["X-Frame-Options"] = "DENY"
|
||||
headers["X-Content-Type-Options"] = "nosniff"
|
||||
await send(message)
|
||||
await self.app(scope, receive, send_wrapper)
|
||||
```
|
||||
Module docstring: the audit basis (SEC-04), the No-CDN verification that makes `'unsafe-inline'` unnecessary (re-verify if inline scripts/styles are ever introduced — the policy would break the page loudly, which is the feature), the pure-ASGI rationale (body untouched — SSE stream byte-identical, unlike `CachingMiddleware`'s page buffering).
|
||||
- **`app/main.py`** — import + `app.add_middleware(SecurityHeadersMiddleware)` placed immediately AFTER the `configure_caching(app)` call (with a one-line comment: outermost on purpose — headers on every response incl. static 404s). Nothing else in `main.py` moves.
|
||||
- **Not touched:** `app/core/caching.py`, any template, any JS, the API layer, the No-CDN surface.
|
||||
|
||||
## Dependencies
|
||||
— (none; standalone transport hardening — wraps the whole app, no route or template change)
|
||||
|
||||
## Tasks
|
||||
1. `01_headers_middleware.md` — `app/core/security_headers.py` + unit tests (incl. the SSE-stream passthrough pin).
|
||||
2. `02_registration_and_integration.md` — `app/main.py` registration + integration tests on real app responses.
|
||||
3. `03_e2e_and_commit.md` — the dedicated Playwright suite + full gate + atomic commit.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit — `tests/unit/test_security_headers.py` (new): a minimal ASGI test-app wrapped by the middleware → a plain response carries all three headers with the exact values; a 404-shaped response carries them too; a **streaming** response (a generator yielding two chunks, `media_type="text/event-stream"`) — the body is consumed byte-identical (both chunks, in order) AND the headers are present (the SSE passthrough pin); a non-http scope (e.g. `websocket`) passes through with the app called and no `send` wrapping error.
|
||||
- Integration — `tests/integration/test_security_headers.py` (new): against the real app (the `conftest` client): `GET /` (a page) → all three headers, CSP exactly the A1 string; `GET /api/health` → all three; `GET /assets/styles.css` → all three; `GET /nope` (404) → all three. (If the existing `tests/integration/test_caching_revalidation.py` fixture pattern for the app client exists, reuse it — same setup, no new fixtures.)
|
||||
- E2E (mandatory, dedicated suite — this phase changes what the browser receives): `tests/e2e/test_security_headers.py` — (1) navigate to the chat page: the navigation `response.headers` carry all three (CSP exact); (2) the page actually works under the CSP — no CSP violations: collect `console` messages during load + one chat turn is NOT required (mock LLM is not needed for a header test) — assert zero console messages matching `/Content Security Policy/` and that the stylesheet applied (computed `background-color` of `body` is not `rgba(0, 0, 0, 0)` — the page painted, not a CSP-broken white page); (3) navigate to `/sources.html` (a second page) → headers present, no CSP violations. Run in isolation per AGENTS.md rule 9.
|
||||
- Coverage: **>90%** on `app/` — the new module is small and unit-exercised; `app/main.py`'s registration is covered by every integration/E2E boot.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `curl -sI localhost:8000/ | grep -iE "content-security-policy|x-frame-options|x-content-type"` shows all three with the exact A1 CSP value (manual check, output kept in the session log); same for `/api/health`, `/assets/styles.css`, and a 404 path.
|
||||
- [ ] The SSE chat stream is byte-identical through the middleware: the unit streaming pin + the existing chat E2E suites (e.g. `tests/e2e/test_chat_rag.py`) green — a body-touching regression would break them.
|
||||
- [ ] `uv run pytest tests/e2e/test_security_headers.py -v --no-cov` green **in isolation** — headers on real page loads + zero CSP violations + the page painted.
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] No template/JS/`pyproject.toml`/`uv.lock` diff; `git diff --stat` limited to the two new files, `app/main.py`, the three test files, phase files.
|
||||
- [ ] One atomic `--no-gpg-sign` commit (e.g. `fix(security): add CSP, X-Frame-Options and nosniff headers to every response`); phase dir moved to `.agents/phases/complete/`.
|
||||
|
||||
## Locked decisions
|
||||
- **No-CDN contract intact** — the CSP is `default-src 'self'` because the audit verified zero external hosts and zero inline scripts/styles; introducing either later breaks pages loudly (the policy does the enforcing).
|
||||
- **No HSTS / Referrer-Policy / Permissions-Policy** (owner A4) — plain homelab HTTP is the documented posture; the three headers above are the scope.
|
||||
- **SSE byte-identity** — the middleware wraps `send` for headers only; any body change is a phase failure (the chat E2E suites are the tripwire).
|
||||
@@ -1,23 +0,0 @@
|
||||
# Task 01 — The pure-ASGI header middleware + unit suite
|
||||
|
||||
**Phase:** `82_security_headers` · **Story:** n/a (security hardening — audit SEC-04)
|
||||
|
||||
## Objective
|
||||
`app/core/security_headers.py` exists exactly per the phase design block, and `tests/unit/test_security_headers.py` pins the header values, the 404 path, the SSE streaming passthrough (byte-identical body + headers present), and the non-http scope passthrough.
|
||||
|
||||
## Work
|
||||
1. `app/core/security_headers.py` (new) — implement per the phase overview's design block: the `CSP` constant (the exact A1 string), `SecurityHeadersMiddleware` as a pure-ASGI class wrapping `send` (headers written on `http.response.start` via `starlette.datastructures.MutableHeaders(scope=message)`), the `scope["type"] != "http"` fast path, and the module docstring content listed in the design block (audit basis, the No-CDN verification, the pure-ASGI/SSE rationale).
|
||||
2. `tests/unit/test_security_headers.py` (new):
|
||||
- a tiny ASGI `test_app` (a plain 200 `Response`-like dict sequence, or starlette's `Response` wrapped in an ASGI callable) + the middleware: assert all three headers with exact values;
|
||||
- a 404-shaped response (status 404) → same three headers;
|
||||
- a streaming response (an async generator yielding `data: a\n\n` then `data: b\n\n`, `media_type="text/event-stream"`) → consume the full body via the ASGI `receive`/`send` protocol (drive it with a small async client loop, or `httpx.AsyncClient(transport=ASGITransport(...))` if already used in the unit suite) → body is exactly the two frames in order AND the three headers are present (the SSE pin);
|
||||
- a `websocket` scope → the app is called, no crash, no header injection attempt.
|
||||
|
||||
## Testing & Quality
|
||||
- `uv run pytest tests/unit/test_security_headers.py -v` green.
|
||||
- Coverage: **>90%** on `app/core/security_headers.py` (every branch: http start, http other messages, non-http scope).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run pytest tests/unit/test_security_headers.py -v` — all tests green, including the SSE byte-identity pin.
|
||||
- [ ] `uv run ruff check app/core/security_headers.py tests/unit/test_security_headers.py && uv run pyright` clean.
|
||||
- [ ] `git diff --stat` for this task: exactly the two new files.
|
||||
@@ -1,26 +0,0 @@
|
||||
# Task 02 — Register the middleware + integration tests on real app responses
|
||||
|
||||
**Phase:** `82_security_headers` · **Story:** n/a (security hardening — audit SEC-04)
|
||||
|
||||
## Objective
|
||||
The middleware wraps the whole app (outermost), and the integration suite pins the three headers on a page, an API response, a static asset, and a 404 — against the real app.
|
||||
|
||||
## Work
|
||||
1. `app/main.py` — import `SecurityHeadersMiddleware` and add `app.add_middleware(SecurityHeadersMiddleware)` immediately AFTER the `configure_caching(app)` call, with the one-line comment from the design block (outermost on purpose: headers on every response, including static catch-all 404s). Do not reorder or re-add any existing middleware.
|
||||
2. `tests/integration/test_security_headers.py` (new) — use the same app/client fixture pattern as `tests/integration/test_caching_revalidation.py` (reuse its setup; do not duplicate the client factory if a shared helper exists):
|
||||
- `GET /` → 200 + all three headers; `Content-Security-Policy` exactly `default-src 'self'; base-uri 'none'; frame-ancestors 'none'`;
|
||||
- `GET /api/health` → 200 + all three;
|
||||
- `GET /assets/styles.css` → 200 + all three;
|
||||
- `GET /definitely-not-a-page` → 404 + all three (the static catch-all's 404 still carries them — the outermost-position proof);
|
||||
- a regression pin: the page body is still the rewritten HTML with `?v=` asset refs (assert one `?v=` occurrence in the body — the two middlewares coexist and the caching rewrite still runs, i.e. header middleware did not swallow/alter the CachingMiddleware's work).
|
||||
3. Run the existing caching integration suite (`tests/integration/test_caching_revalidation.py`) to prove no interaction regression (it must stay green untouched).
|
||||
|
||||
## Testing & Quality
|
||||
- `uv run pytest tests/integration/test_security_headers.py tests/integration/test_caching_revalidation.py -v` green.
|
||||
- Coverage: **>90%** on `app/` (the registration line is hit by every integration/E2E boot; no new uncovered branches).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] All four real-response pins pass (page / API / asset / 404) with the exact CSP string.
|
||||
- [ ] The `?v=` coexistence pin passes (caching rewrite intact under the new outermost middleware).
|
||||
- [ ] `tests/integration/test_caching_revalidation.py` green untouched.
|
||||
- [ ] `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -1,37 +0,0 @@
|
||||
# Task 03 — Dedicated Playwright suite + full gate + atomic commit
|
||||
|
||||
**Phase:** `82_security_headers` · **Story:** n/a (security hardening — audit SEC-04)
|
||||
|
||||
## Objective
|
||||
Prove the headers reach a real browser AND the strict CSP does not break the page (no violations, page painted), run the full gate, and land the phase as one atomic commit.
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/test_security_headers.py` (new) — follow the house E2E conventions (read `tests/e2e/conftest.py` + one existing suite, e.g. `test_header_consistency.py`, for the fixtures and the app-server pattern):
|
||||
- **Headers on a real navigation:** `page.goto` the chat page (the shell at `/`) → the navigation response's headers carry all three, CSP exactly the A1 string; `page.goto` `/sources.html` → same (a second page).
|
||||
- **No CSP violations:** attach a `page.on("console")` listener before navigation; after both loads assert zero console messages whose text matches `/Content Security Policy/i` (Chromium reports CSP denials to the console).
|
||||
- **The page painted (CSP not breaking assets):** after the chat-page load, `document.querySelector('body')`'s computed `background-color` is not `rgba(0, 0, 0, 0)` (the dark-tech palette applied — styles loaded under the CSP) and the shell's root element is present (e.g. `#main` — the page booted, not a white-broken document).
|
||||
- No LLM interaction needed (the page load is the contract) — the suite must not depend on the aipi endpoint.
|
||||
2. **Manual live check** (keep the output in the session log): `curl -sI localhost:8000/ | grep -iE "content-security-policy|x-frame-options|x-content-type"` + the same for `/api/health`, `/assets/styles.css`, `/nope` (404).
|
||||
3. **Full regression gate** (AGENTS.md rule 9):
|
||||
- `uv run pytest` — unit + integration green.
|
||||
- `uv run pytest --cov=app --cov-report=term-missing` — `app/` coverage **>90%**.
|
||||
- `uv run pytest tests/e2e/test_security_headers.py -v --no-cov` — green **in isolation** (this phase's mandatory E2E).
|
||||
- ONE chat-stream suite to prove SSE byte-identity end-to-end: `uv run pytest tests/e2e/test_chat_rag.py -v --no-cov` green (the mock-LLM chat suite — a body-touching regression in the middleware would break the stream).
|
||||
- `uv run ruff check . && uv run pyright` — clean.
|
||||
4. **Commit** (AGENTS.md rule 8 — one atomic, Conventional-Commits commit, always `--no-gpg-sign`), staging `app/core/security_headers.py`, `app/main.py`, the three test files, and the phase files:
|
||||
`fix(security): add CSP, X-Frame-Options and nosniff headers to every response`
|
||||
— body: security audit SEC-04 (2026-09-07) — no response carried framing/XSS-mitigation headers: any page (incl. the admin UI) could be iframe-embedded for clickjacking. A header-only pure-ASGI middleware (outermost) now sets `default-src 'self'; base-uri 'none'; frame-ancestors 'none'` (no 'unsafe-inline' — the No-CDN + no-inline-markup audit made the strict policy possible), `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff` on every response including 404s; the SSE stream passes byte-identical (unit streaming pin + chat E2E regression). No template/JS/dependency change.
|
||||
5. Move the phase directory: `mv .agents/phases/todo/82_security_headers .agents/phases/complete/` and include the move in the same commit.
|
||||
|
||||
## Testing & Quality
|
||||
- E2E: the dedicated suite green in isolation (headers + zero CSP violations + painted page).
|
||||
- Regression: `test_chat_rag.py` green (SSE byte-identity through the real middleware).
|
||||
- Coverage: >90% held.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run pytest tests/e2e/test_security_headers.py -v --no-cov` green in isolation (both pages, exact CSP, no violations, page painted).
|
||||
- [ ] `uv run pytest tests/e2e/test_chat_rag.py -v --no-cov` green (the stream is untouched).
|
||||
- [ ] The `curl` header check output (4 URLs) kept in the session log.
|
||||
- [ ] `uv run pytest` green; coverage >90%; ruff + pyright clean.
|
||||
- [ ] Exactly one new commit; `git show --stat HEAD` lists the two app files, the three test files, and the phase files (todo → complete move) — nothing else.
|
||||
- [ ] `.agents/phases/complete/82_security_headers/` exists; `todo/` no longer contains it.
|
||||
Reference in New Issue
Block a user