feat: HTTP security hardening (CSP, CSRF, session tag, body caps)
- Trusted app-shell responses now carry a strict script-src CSP plus
X-Frame-Options: DENY and nosniff (img-src stays permissive for external
evidence thumbnails; the GUI uses no inline scripts).
- Session cookie stores an HMAC-derived tag, not the raw COPYRIGHTER_AUTH_TOKEN,
so a leaked cookie no longer reveals the token/bearer/.env secret.
- Cookie-authenticated state-changing requests require a same-origin
Origin/Referer (CSRF); header-authenticated requests are not forgeable.
- _body rejects negative/non-integer Content-Length (was reaching
rfile.read(-1), an unbounded read-to-EOF) and returns a controlled 400.
- Handler timeout=30 bounds slow-client thread exhaustion; uploads use
exclusive-create ('xb') to close a concurrent same-name overwrite TOCTOU.
Tests: app-shell security headers, negative Content-Length -> 400, cookie tag
(not raw token), cookie-authed cross-origin POST -> 403 / same-origin -> 200.
This commit is contained in:
parent
1803b4e0b8
commit
4ef12f5f95
2 changed files with 200 additions and 15 deletions
|
|
@ -1,6 +1,7 @@
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import base64
|
import base64
|
||||||
|
import hashlib
|
||||||
import hmac
|
import hmac
|
||||||
import json
|
import json
|
||||||
import mimetypes
|
import mimetypes
|
||||||
|
|
@ -35,6 +36,17 @@ class _PayloadTooLarge(Exception):
|
||||||
"""Raised when a request body exceeds _MAX_BODY_BYTES."""
|
"""Raised when a request body exceeds _MAX_BODY_BYTES."""
|
||||||
|
|
||||||
|
|
||||||
|
# Content types treated as the trusted operator app shell, which receives the
|
||||||
|
# strict script-src CSP + framing/nosniff headers. Both javascript spellings are
|
||||||
|
# listed because mimetypes maps .js to text/javascript on Python 3.12+.
|
||||||
|
_APP_SHELL_CONTENT_TYPES = {
|
||||||
|
"text/html",
|
||||||
|
"application/javascript",
|
||||||
|
"text/javascript",
|
||||||
|
"text/css",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
# Media routes loaded via <img>/<link>, which cannot send an Authorization
|
# Media routes loaded via <img>/<link>, which cannot send an Authorization
|
||||||
# header — these rely on the session cookie instead.
|
# header — these rely on the session cookie instead.
|
||||||
_MEDIA_PREFIXES = (
|
_MEDIA_PREFIXES = (
|
||||||
|
|
@ -51,6 +63,19 @@ _PROTECTED_PREFIXES = ("/api/", *_MEDIA_PREFIXES)
|
||||||
# load media without ever carrying the token in a URL.
|
# load media without ever carrying the token in a URL.
|
||||||
_SESSION_COOKIE = "copyrighter_session"
|
_SESSION_COOKIE = "copyrighter_session"
|
||||||
|
|
||||||
|
# Methods that change state — cookie-authenticated ones get an Origin/Referer
|
||||||
|
# CSRF check (a header-authenticated request cannot be forged cross-site because
|
||||||
|
# browsers never auto-attach an Authorization header).
|
||||||
|
_STATE_CHANGING_METHODS = {"POST", "PATCH", "PUT", "DELETE"}
|
||||||
|
|
||||||
|
|
||||||
|
def _session_tag(token: str) -> str:
|
||||||
|
# Deterministic HMAC of the token so the session cookie carries a derived
|
||||||
|
# tag, not the raw COPYRIGHTER_AUTH_TOKEN. A leaked cookie can still ride the
|
||||||
|
# session but no longer reveals the token itself (which is also the .env
|
||||||
|
# secret / Authorization bearer). No server-side session store needed.
|
||||||
|
return hmac.new(token.encode("utf-8"), b"copyrighter-session-v1", hashlib.sha256).hexdigest()
|
||||||
|
|
||||||
|
|
||||||
def build_server(
|
def build_server(
|
||||||
host: str,
|
host: str,
|
||||||
|
|
@ -64,6 +89,9 @@ def build_server(
|
||||||
|
|
||||||
class CopyrighterHandler(BaseHTTPRequestHandler):
|
class CopyrighterHandler(BaseHTTPRequestHandler):
|
||||||
server_version = "CopyrighterHTTP/0.1"
|
server_version = "CopyrighterHTTP/0.1"
|
||||||
|
# Bound how long a slow/idle client can hold a worker thread's socket, so
|
||||||
|
# a handful of slowloris-style connections cannot exhaust the thread pool.
|
||||||
|
timeout = 30
|
||||||
|
|
||||||
def do_GET(self) -> None:
|
def do_GET(self) -> None:
|
||||||
path = _path(self.path)
|
path = _path(self.path)
|
||||||
|
|
@ -186,6 +214,9 @@ def build_server(
|
||||||
elif path.startswith("/api/collections/candidates/") and path.endswith("/promote"):
|
elif path.startswith("/api/collections/candidates/") and path.endswith("/promote"):
|
||||||
candidate_id = unquote(path.split("/")[4])
|
candidate_id = unquote(path.split("/")[4])
|
||||||
self._json(store.promote_collection_candidate(candidate_id, body))
|
self._json(store.promote_collection_candidate(candidate_id, body))
|
||||||
|
elif path.startswith("/api/collections/candidates/") and path.endswith("/dismiss"):
|
||||||
|
candidate_id = unquote(path.split("/")[4])
|
||||||
|
self._json(store.dismiss_collection_candidate(candidate_id))
|
||||||
elif path.startswith("/api/knowledge/") and path.endswith("/promote-watchlist"):
|
elif path.startswith("/api/knowledge/") and path.endswith("/promote-watchlist"):
|
||||||
entry_id = unquote(path.split("/")[3])
|
entry_id = unquote(path.split("/")[3])
|
||||||
self._json(store.promote_watchlist_entry(entry_id))
|
self._json(store.promote_watchlist_entry(entry_id))
|
||||||
|
|
@ -268,13 +299,40 @@ def build_server(
|
||||||
jar = SimpleCookie()
|
jar = SimpleCookie()
|
||||||
jar.load(cookie_header)
|
jar.load(cookie_header)
|
||||||
morsel = jar.get(_SESSION_COOKIE)
|
morsel = jar.get(_SESSION_COOKIE)
|
||||||
if morsel is not None and hmac.compare_digest(morsel.value, auth_token):
|
if morsel is not None and hmac.compare_digest(morsel.value, _session_tag(auth_token)):
|
||||||
|
# Cookie-authenticated state changes are CSRF-forgeable, so
|
||||||
|
# require a same-origin Origin/Referer for them.
|
||||||
|
if self.command in _STATE_CHANGING_METHODS and not self._origin_ok():
|
||||||
|
self._json({"error": "invalid origin"}, HTTPStatus.FORBIDDEN)
|
||||||
|
return False
|
||||||
return True
|
return True
|
||||||
self._json({"error": "unauthorized"}, HTTPStatus.UNAUTHORIZED)
|
self._json({"error": "unauthorized"}, HTTPStatus.UNAUTHORIZED)
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
def _origin_ok(self) -> bool:
|
||||||
|
# Accept only requests whose Origin (preferred) or Referer resolves to
|
||||||
|
# the same host we were reached on. If neither header is present we
|
||||||
|
# cannot verify, so allow (older/non-browser clients) rather than break
|
||||||
|
# the app — the classic cross-site POST always carries an Origin.
|
||||||
|
host = self.headers.get("Host", "")
|
||||||
|
origin = self.headers.get("Origin", "")
|
||||||
|
if origin:
|
||||||
|
return urlparse(origin).netloc == host
|
||||||
|
referer = self.headers.get("Referer", "")
|
||||||
|
if referer:
|
||||||
|
return urlparse(referer).netloc == host
|
||||||
|
return True
|
||||||
|
|
||||||
def _body(self) -> dict[str, object]:
|
def _body(self) -> dict[str, object]:
|
||||||
length = int(self.headers.get("Content-Length", "0") or "0")
|
# Parse defensively: a non-integer header must not leak Python's parse
|
||||||
|
# error, and a negative length must not slip past the size cap into
|
||||||
|
# rfile.read(-1) (an unbounded read-to-EOF that exhausts memory).
|
||||||
|
try:
|
||||||
|
length = int(self.headers.get("Content-Length", "0") or "0")
|
||||||
|
except ValueError:
|
||||||
|
raise ValueError("invalid Content-Length")
|
||||||
|
if length < 0:
|
||||||
|
raise ValueError("invalid Content-Length")
|
||||||
if length > _MAX_BODY_BYTES:
|
if length > _MAX_BODY_BYTES:
|
||||||
raise _PayloadTooLarge()
|
raise _PayloadTooLarge()
|
||||||
if not length:
|
if not length:
|
||||||
|
|
@ -286,9 +344,10 @@ def build_server(
|
||||||
|
|
||||||
def _maybe_set_session_cookie(self) -> None:
|
def _maybe_set_session_cookie(self) -> None:
|
||||||
if getattr(self, "_issue_session_cookie", False):
|
if getattr(self, "_issue_session_cookie", False):
|
||||||
|
# Store the derived tag, never the raw token.
|
||||||
self.send_header(
|
self.send_header(
|
||||||
"Set-Cookie",
|
"Set-Cookie",
|
||||||
f"{_SESSION_COOKIE}={auth_token}; HttpOnly; SameSite=Strict; Path=/",
|
f"{_SESSION_COOKIE}={_session_tag(auth_token)}; HttpOnly; SameSite=Strict; Path=/",
|
||||||
)
|
)
|
||||||
|
|
||||||
def _json(self, payload: object, status: HTTPStatus = HTTPStatus.OK) -> None:
|
def _json(self, payload: object, status: HTTPStatus = HTTPStatus.OK) -> None:
|
||||||
|
|
@ -340,6 +399,21 @@ def build_server(
|
||||||
# don't leak the URL via Referer.
|
# don't leak the URL via Referer.
|
||||||
self.send_header("Cache-Control", "no-store")
|
self.send_header("Cache-Control", "no-store")
|
||||||
self.send_header("Referrer-Policy", "no-referrer")
|
self.send_header("Referrer-Policy", "no-referrer")
|
||||||
|
elif content_type.split(";", 1)[0] in _APP_SHELL_CONTENT_TYPES:
|
||||||
|
# Trusted app shell (HTML/JS/CSS). The GUI loads only same-origin
|
||||||
|
# scripts (no inline <script>, no CDN), so a script-src 'self'
|
||||||
|
# policy is compatible and blocks injected scripts. img-src stays
|
||||||
|
# permissive because evidence thumbnails are external result URLs.
|
||||||
|
self.send_header("X-Content-Type-Options", "nosniff")
|
||||||
|
self.send_header("X-Frame-Options", "DENY")
|
||||||
|
self.send_header(
|
||||||
|
"Content-Security-Policy",
|
||||||
|
"default-src 'self'; script-src 'self'; "
|
||||||
|
"style-src 'self' 'unsafe-inline'; "
|
||||||
|
"img-src 'self' data: blob: https: http:; "
|
||||||
|
"font-src 'self' data:; connect-src 'self'; "
|
||||||
|
"base-uri 'self'; object-src 'none'; frame-ancestors 'none'",
|
||||||
|
)
|
||||||
self._maybe_set_session_cookie()
|
self._maybe_set_session_cookie()
|
||||||
self.end_headers()
|
self.end_headers()
|
||||||
self.wfile.write(data)
|
self.wfile.write(data)
|
||||||
|
|
@ -373,8 +447,7 @@ def _save_submission_upload(image_store: LocalSubmissionImageStore, raw_image: o
|
||||||
image_dir = image_store.root / "images"
|
image_dir = image_store.root / "images"
|
||||||
image_dir.mkdir(parents=True, exist_ok=True)
|
image_dir.mkdir(parents=True, exist_ok=True)
|
||||||
safe_name = _safe_upload_name(filename)
|
safe_name = _safe_upload_name(filename)
|
||||||
target = _unique_upload_path(image_dir, safe_name)
|
target = _write_unique_upload(image_dir, safe_name, content)
|
||||||
target.write_bytes(content)
|
|
||||||
return target.stem
|
return target.stem
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -384,15 +457,20 @@ def _safe_upload_name(filename: str) -> str:
|
||||||
return f"{stem}{path.suffix.lower()}"
|
return f"{stem}{path.suffix.lower()}"
|
||||||
|
|
||||||
|
|
||||||
def _unique_upload_path(directory: Path, filename: str) -> Path:
|
def _write_unique_upload(directory: Path, filename: str, content: bytes) -> Path:
|
||||||
candidate = directory / filename
|
# Exclusive create ('xb') so two concurrent uploads of the same name can't
|
||||||
if not candidate.exists():
|
# both pass an exists() check and then overwrite each other — the loser gets
|
||||||
return candidate
|
# FileExistsError and advances to the next -N suffix.
|
||||||
path = Path(filename)
|
path = Path(filename)
|
||||||
for index in range(2, 1000):
|
candidates = [directory / filename]
|
||||||
candidate = directory / f"{path.stem}-{index}{path.suffix}"
|
candidates.extend(directory / f"{path.stem}-{index}{path.suffix}" for index in range(2, 1000))
|
||||||
if not candidate.exists():
|
for candidate in candidates:
|
||||||
|
try:
|
||||||
|
with open(candidate, "xb") as handle:
|
||||||
|
handle.write(content)
|
||||||
return candidate
|
return candidate
|
||||||
|
except FileExistsError:
|
||||||
|
continue
|
||||||
raise ValueError("too many files with the same submission image name")
|
raise ValueError("too many files with the same submission image name")
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -266,6 +266,54 @@ def test_http_server_exposes_health_bootstrap_and_mutations(tmp_path: Path):
|
||||||
assert next(provider for provider in providers if provider["id"] == "naver")["enabled"] is False
|
assert next(provider for provider in providers if provider["id"] == "naver")["enabled"] is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_app_shell_sends_security_headers(tmp_path: Path):
|
||||||
|
# 2026-07-11 #6: the trusted operator shell must carry CSP / framing / nosniff
|
||||||
|
# headers so an injected script or a framing attempt is blocked by the browser.
|
||||||
|
import http.client
|
||||||
|
|
||||||
|
static_dir, image_store, store = _fixtures(tmp_path)
|
||||||
|
server = build_server(
|
||||||
|
host="127.0.0.1", port=0, store=store, image_store=image_store, static_dir=static_dir
|
||||||
|
)
|
||||||
|
_start(server)
|
||||||
|
try:
|
||||||
|
conn = http.client.HTTPConnection("127.0.0.1", server.server_port, timeout=5)
|
||||||
|
conn.request("GET", "/")
|
||||||
|
resp = conn.getresponse()
|
||||||
|
resp.read()
|
||||||
|
csp = resp.getheader("Content-Security-Policy")
|
||||||
|
assert csp is not None and "script-src 'self'" in csp
|
||||||
|
assert resp.getheader("X-Frame-Options") == "DENY"
|
||||||
|
assert resp.getheader("X-Content-Type-Options") == "nosniff"
|
||||||
|
conn.close()
|
||||||
|
finally:
|
||||||
|
server.shutdown()
|
||||||
|
|
||||||
|
|
||||||
|
def test_negative_content_length_is_rejected(tmp_path: Path):
|
||||||
|
# 2026-07-11 M10: a negative Content-Length must be rejected (400), never
|
||||||
|
# reach rfile.read(-1) which would read to EOF and exhaust memory.
|
||||||
|
import http.client
|
||||||
|
|
||||||
|
static_dir, image_store, store = _fixtures(tmp_path)
|
||||||
|
server = build_server(
|
||||||
|
host="127.0.0.1", port=0, store=store, image_store=image_store, static_dir=static_dir
|
||||||
|
)
|
||||||
|
_start(server)
|
||||||
|
try:
|
||||||
|
conn = http.client.HTTPConnection("127.0.0.1", server.server_port, timeout=5)
|
||||||
|
conn.putrequest("POST", "/api/submissions/SUB-API1/decision")
|
||||||
|
conn.putheader("Content-Type", "application/json")
|
||||||
|
conn.putheader("Content-Length", "-1")
|
||||||
|
conn.endheaders()
|
||||||
|
resp = conn.getresponse()
|
||||||
|
resp.read()
|
||||||
|
assert resp.status == 400
|
||||||
|
conn.close()
|
||||||
|
finally:
|
||||||
|
server.shutdown()
|
||||||
|
|
||||||
|
|
||||||
def test_http_server_rejects_enabling_unconfigured_google_custom_search(tmp_path: Path):
|
def test_http_server_rejects_enabling_unconfigured_google_custom_search(tmp_path: Path):
|
||||||
static_dir, image_store, _ = _fixtures(tmp_path)
|
static_dir, image_store, _ = _fixtures(tmp_path)
|
||||||
store = CopyrighterStore(
|
store = CopyrighterStore(
|
||||||
|
|
@ -1333,11 +1381,13 @@ def test_protected_routes_require_token_when_configured(tmp_path: Path):
|
||||||
response = urlopen(authed)
|
response = urlopen(authed)
|
||||||
assert response.status == 200
|
assert response.status == 200
|
||||||
set_cookie = response.headers.get("Set-Cookie")
|
set_cookie = response.headers.get("Set-Cookie")
|
||||||
assert set_cookie and "copyrighter_session=secret" in set_cookie
|
assert set_cookie and "HttpOnly" in set_cookie
|
||||||
assert "HttpOnly" in set_cookie
|
# 2026-07-11 #7: the cookie carries a derived tag, never the raw token.
|
||||||
|
session_value = set_cookie.split("copyrighter_session=")[1].split(";")[0]
|
||||||
|
assert session_value != "secret"
|
||||||
|
|
||||||
# The cookie alone authenticates (mimics a browser <img> media load).
|
# The cookie alone authenticates (mimics a browser <img> media load).
|
||||||
cookie_req = Request(base + "/api/bootstrap", headers={"Cookie": "copyrighter_session=secret"})
|
cookie_req = Request(base + "/api/bootstrap", headers={"Cookie": f"copyrighter_session={session_value}"})
|
||||||
assert urlopen(cookie_req).status == 200
|
assert urlopen(cookie_req).status == 200
|
||||||
|
|
||||||
# A wrong cookie is rejected; the token never travels in the URL.
|
# A wrong cookie is rejected; the token never travels in the URL.
|
||||||
|
|
@ -1353,6 +1403,63 @@ def test_protected_routes_require_token_when_configured(tmp_path: Path):
|
||||||
server.shutdown()
|
server.shutdown()
|
||||||
|
|
||||||
|
|
||||||
|
def test_cookie_authed_state_change_requires_same_origin(tmp_path: Path):
|
||||||
|
# 2026-07-11 #10: a cookie-authenticated POST from a foreign Origin (classic
|
||||||
|
# CSRF) is rejected; header-authed requests are not forgeable.
|
||||||
|
import http.client
|
||||||
|
|
||||||
|
static_dir, image_store, store = _fixtures(tmp_path)
|
||||||
|
server = build_server(
|
||||||
|
host="127.0.0.1", port=0, store=store, image_store=image_store,
|
||||||
|
static_dir=static_dir, auth_token="secret",
|
||||||
|
)
|
||||||
|
_start(server)
|
||||||
|
port = server.server_port
|
||||||
|
try:
|
||||||
|
conn = http.client.HTTPConnection("127.0.0.1", port, timeout=5)
|
||||||
|
conn.request("GET", "/api/bootstrap", headers={"Authorization": "Bearer secret"})
|
||||||
|
resp = conn.getresponse()
|
||||||
|
resp.read()
|
||||||
|
session = resp.getheader("Set-Cookie").split("copyrighter_session=")[1].split(";")[0]
|
||||||
|
conn.close()
|
||||||
|
|
||||||
|
body = json.dumps({"decision": "held", "memo": "x"})
|
||||||
|
conn2 = http.client.HTTPConnection("127.0.0.1", port, timeout=5)
|
||||||
|
conn2.request(
|
||||||
|
"POST",
|
||||||
|
"/api/submissions/SUB-API1/decision",
|
||||||
|
body=body,
|
||||||
|
headers={
|
||||||
|
"Cookie": f"copyrighter_session={session}",
|
||||||
|
"Origin": "http://evil.example",
|
||||||
|
"Content-Type": "application/json",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
resp2 = conn2.getresponse()
|
||||||
|
resp2.read()
|
||||||
|
assert resp2.status == 403
|
||||||
|
conn2.close()
|
||||||
|
|
||||||
|
# Same-origin cookie POST succeeds.
|
||||||
|
conn3 = http.client.HTTPConnection("127.0.0.1", port, timeout=5)
|
||||||
|
conn3.request(
|
||||||
|
"POST",
|
||||||
|
"/api/submissions/SUB-API1/decision",
|
||||||
|
body=body,
|
||||||
|
headers={
|
||||||
|
"Cookie": f"copyrighter_session={session}",
|
||||||
|
"Origin": f"http://127.0.0.1:{port}",
|
||||||
|
"Content-Type": "application/json",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
resp3 = conn3.getresponse()
|
||||||
|
resp3.read()
|
||||||
|
assert resp3.status == 200
|
||||||
|
conn3.close()
|
||||||
|
finally:
|
||||||
|
server.shutdown()
|
||||||
|
|
||||||
|
|
||||||
def test_oversized_body_rejected_with_413(tmp_path: Path):
|
def test_oversized_body_rejected_with_413(tmp_path: Path):
|
||||||
static_dir, image_store, store = _fixtures(tmp_path)
|
static_dir, image_store, store = _fixtures(tmp_path)
|
||||||
server = build_server(host="127.0.0.1", port=0, store=store, image_store=image_store, static_dir=static_dir)
|
server = build_server(host="127.0.0.1", port=0, store=store, image_store=image_store, static_dir=static_dir)
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue