Skip to content

Commit 23d40ca

Browse files
madmax1301claude
andcommitted
security: H-2 cookies-path allowlist, H-5 path-traversal, M-1 rate-limit, M-2 security-headers
Closes 4 of 5 Bald-Cluster items from the 2026-05-12 audit. H-3 (Subsonic plaintext password) is deferred — needs a live Navidrome test setup. H-2 (High) — yt-dlp cookies_path arbitrary-file-read Cookie paths must now live inside config.YOUTUBE_COOKIES_DIR (env- overridable, default /app/data), be regular files (symlinks rejected before resolve()), and have a .txt/.netscape/.cookies extension. The UI-configurable path was the attack surface — even with H-1 fixed (non-root container), letting yt-dlp open /app/data/jobs.db as cookie file would surface DB contents in yt-dlp error messages. H-5 (High) — Path-traversal in MP3 target paths _sanitize_path and _sanitize_filename now replace ".." sequences and strip leading dots BEFORE the char-blacklist. The blacklist removed slashes but left the two dots, so an album="../../etc" survived and mkdir(parents=True) climbed out of the Navidrome music root. Defense-in-depth: containment-check via Path.resolve().relative_to( library_root) in get_target_path before mkdir — catches future sanitize gaps or callers that bypass _sanitize_path. M-1 (Medium) — Bulk-endpoint rate limits Custom in-memory sliding-window limiter in utils/rate_limit.py. Per (client_ip, route) deque with timestamp-based cleanup. Decisions: - Custom over slowapi: slowapi requires `request: Request` as first endpoint arg, would shadow the existing Pydantic body params and force a body-rename across all four target endpoints - Key uses utils.auth.client_ip (= the H-7 trusted-proxy-aware helper from 03d24f7), so traffic through Traefik isn't collapsed onto the proxy IP Limits: csv-import 20/h, spotify-history 10/h, url-download 60/h, track-download 120/h. Track-download deliberately the highest because it's UI-click-driven (e.g. album with 30 tracks = 30 fast clicks). M-2 (Medium) — Security headers middleware Four no-brainer headers via FastAPI HTTP middleware: - X-Content-Type-Options: nosniff - X-Frame-Options: DENY - Referrer-Policy: strict-origin-when-cross-origin - Permissions-Policy (geolocation/microphone/camera/payment/usb = ()) CSP intentionally left out — would break SvelteKit inline-styles and the album-art img-src would need to thread through ALBUM_ART_ALLOWED_HOSTS. Worth its own tuning pass and follow-up commit. HSTS left to the TLS-terminating reverse proxy (Traefik typically handles this with its own Headers middleware). New env var: YOUTUBE_COOKIES_DIR (default /app/data) for H-2. py_compile clean across all touched files. Refs: - ~/SecondBrain/10-Projects/Tonus/security-audit-2026-05-12.md - v0.4.0 was the Sofort-Cluster (8b01746) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a7dcf41 commit 23d40ca

6 files changed

Lines changed: 226 additions & 20 deletions

File tree

CHANGELOG.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,48 @@ On a `git tag -a vX.Y.Z`, move the relevant entries into a new dated section.
1010

1111
---
1212

13+
## [0.4.1] — 2026-05-12
14+
15+
Security-Patch — closes 4 of 5 items from the **Bald-Cluster** of the
16+
2026-05-12 audit. **H-3** (Subsonic plaintext password) is deferred
17+
because it needs a live Navidrome test environment.
18+
19+
### Security
20+
21+
- **H-2 (High)** — yt-dlp `cookies_path` arbitrary-file-read blocked.
22+
Cookie paths must now live inside `config.YOUTUBE_COOKIES_DIR`
23+
(env-overridable, default `/app/data`), be regular files (no symlinks
24+
followed), and have a `.txt` / `.netscape` / `.cookies` extension.
25+
Configurable via *Settings → Verbindungen* but with hard guard-rails.
26+
- **H-5 (High)** — Path-traversal in MP3 target paths fixed.
27+
`_sanitize_path` and `_sanitize_filename` now replace `..` sequences
28+
and strip leading dots BEFORE the char-blacklist. Plus defense-in-depth
29+
containment check via `Path.resolve().relative_to(library_root)` in
30+
`get_target_path` before `mkdir`.
31+
- **M-1 (Medium)** — Bulk-Endpoint rate limits added: CSV-Import 20/h,
32+
Spotify-History 10/h, URL-Download 60/h, Track-Download 120/h. Sliding-
33+
window counter per `(client_ip, route)`, uses the H-7 trusted-proxy-aware
34+
`client_ip` helper (no Reverse-Proxy collapsing onto the proxy IP).
35+
- **M-2 (Medium)** — Security headers middleware: `X-Content-Type-Options:
36+
nosniff`, `X-Frame-Options: DENY`, `Referrer-Policy:
37+
strict-origin-when-cross-origin`, `Permissions-Policy`. CSP intentionally
38+
deferred — would break SvelteKit inline-styles without a tuning pass.
39+
40+
### Pending
41+
42+
- **H-3 (High)** — Subsonic plaintext password. Refactor to Subsonic
43+
Token-Auth (`?u=user&s=salt&t=md5(password+salt)`) requires a live
44+
Navidrome instance for verification. Tracked in the SecondBrain audit
45+
doc.
46+
47+
### New env vars
48+
49+
| Var | Default | Purpose |
50+
|---|---|---|
51+
| `YOUTUBE_COOKIES_DIR` | `/app/data` | H-2: allowed root for `YOUTUBE_COOKIES_PATH` |
52+
53+
---
54+
1355
## [0.4.0] — 2026-05-12
1456

1557
Security-Hardening-Release. Closes the entire **Sofort-Fix-Cluster** from the

backend/app.py

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,19 @@
5959
)
6060
from utils.worker import JobWorker
6161
from utils.auth import require_token, require_admin, auth_required, optional_token
62+
from utils.rate_limit import make_rate_limiter
6263

6364
ALLOWED_METADATA_PROVIDERS = frozenset({"deezer", "spotify"})
6465

66+
# Rate-Limiter (Audit M-1, 2026-05-12). Module-level damit alle Requests
67+
# denselben State teilen — Fresh-Per-Request würde den Counter resetten.
68+
# Werte konservativ-großzügig gewählt: legitime Bulk-User stoßen nicht an,
69+
# automatisierte Bursts werden gebremst.
70+
_rl_csv_import = make_rate_limiter(20, 3600) # 20 CSV-Imports/h
71+
_rl_spotify_history = make_rate_limiter(10, 3600) # 10 Spotify-History/h
72+
_rl_url_download = make_rate_limiter(60, 3600) # 60 URL-Downloads/h
73+
_rl_track_download = make_rate_limiter(120, 3600) # 120 Track-Downloads/h
74+
6575
# Extra download attempts after the first failure (each failure waits before retrying).
6676
MAX_DOWNLOAD_RETRIES_CAP = 5
6777

@@ -876,6 +886,30 @@ async def add_root_path(request: Request, call_next):
876886
return await call_next(request)
877887

878888

889+
@app.middleware("http")
890+
async def add_security_headers(request: Request, call_next):
891+
"""Standard-Security-Header für jede Response (Audit M-2, 2026-05-12).
892+
893+
Vier No-Brainer-Header die nicht die UI brechen können:
894+
- X-Content-Type-Options: nosniff — blockt MIME-Type-Sniffing-Attacks
895+
- X-Frame-Options: DENY — blockt Clickjacking via iframe-Embedding
896+
- Referrer-Policy: strict-origin-when-cross-origin — limitiert Referer-Leak
897+
- Permissions-Policy — disabled Browser-APIs die Tonus nicht braucht
898+
CSP bleibt bewusst weg — würde SvelteKit-inline-styles brechen und braucht
899+
eigenes Tuning (eigenes Backlog-Item). HSTS macht Sinn nur HINTER TLS-Proxy
900+
(Traefik) und wird vom Proxy selbst gesetzt — hier kein zweites Set.
901+
"""
902+
response = await call_next(request)
903+
response.headers.setdefault("X-Content-Type-Options", "nosniff")
904+
response.headers.setdefault("X-Frame-Options", "DENY")
905+
response.headers.setdefault("Referrer-Policy", "strict-origin-when-cross-origin")
906+
response.headers.setdefault(
907+
"Permissions-Policy",
908+
"geolocation=(), microphone=(), camera=(), payment=(), usb=()",
909+
)
910+
return response
911+
912+
879913
@app.get("/api/metadata/providers")
880914
async def metadata_providers():
881915
"""Available metadata sources and server default."""
@@ -1294,7 +1328,7 @@ async def move_queue_item(req: QueueMoveRequest, _: None = Depends(require_token
12941328

12951329

12961330
@app.post("/api/download")
1297-
async def download_track(request: DownloadRequest, background_tasks: BackgroundTasks, _: None = Depends(require_token)):
1331+
async def download_track(request: DownloadRequest, background_tasks: BackgroundTasks, _: None = Depends(require_token), __: None = Depends(_rl_track_download)):
12981332
"""Start downloading a track"""
12991333
if request.location not in ["local", "navidrome"]:
13001334
request.location = "local"
@@ -2116,7 +2150,7 @@ def get_recommendations(request: RecommendationRequest):
21162150

21172151

21182152
@app.post("/api/import/csv")
2119-
async def import_csv(request: CsvImportRequest, _: None = Depends(require_token)):
2153+
async def import_csv(request: CsvImportRequest, _: None = Depends(require_token), __: None = Depends(_rl_csv_import)):
21202154
"""
21212155
CSV-Import (persistent): speichert Job in SQLite, Worker holt ihn ab.
21222156
Gibt sofort eine job_id zurück — Status unter /api/import/jobs/{job_id}/status pollbar.
@@ -2316,7 +2350,7 @@ class SpotifyHistoryImportRequest(BaseModel):
23162350

23172351

23182352
@app.post("/api/import/spotify-history")
2319-
async def import_spotify_history(req: SpotifyHistoryImportRequest, _: None = Depends(require_token)):
2353+
async def import_spotify_history(req: SpotifyHistoryImportRequest, _: None = Depends(require_token), __: None = Depends(_rl_spotify_history)):
23202354
"""Importiert eine oder mehrere Spotify-Extended-Streaming-History-JSONs.
23212355
23222356
Pipeline-Integration:
@@ -2879,7 +2913,7 @@ def url_download_and_process(
28792913

28802914

28812915
@app.post("/api/url/download")
2882-
async def url_download(req: URLDownloadRequest, background_tasks: BackgroundTasks, _: None = Depends(require_token)):
2916+
async def url_download(req: URLDownloadRequest, background_tasks: BackgroundTasks, _: None = Depends(require_token), __: None = Depends(_rl_url_download)):
28832917
"""Phase 1: Direkter Download via URL ohne Spotify/Deezer-Match.
28842918
28852919
Funktioniert für YouTube, SoundCloud, Bandcamp, Vimeo … alles was yt-dlp kennt.

backend/config.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,16 @@ def navidrome_libraries_public() -> List[Dict[str, Any]]:
8686
# YouTube Configuration
8787
YOUTUBE_COOKIES_PATH = os.getenv("YOUTUBE_COOKIES_PATH", "") # Path to YouTube cookies file (Netscape format) for yt-dlp
8888

89+
# H-2 Audit 2026-05-12: Allowed-Root für YOUTUBE_COOKIES_PATH.
90+
# yt-dlp liest die Cookies-Datei direkt von Disk. Ohne Restriction konnte
91+
# der Operator via Settings → Verbindungen einen beliebigen Pfad eintragen
92+
# (z.B. /etc/passwd oder /app/data/jobs.db) — yt-dlp würde versuchen die
93+
# Datei als Cookies-Netscape-Format zu parsen. Kombiniert mit H-1 (Container
94+
# als root) wäre das ein arbitrary-file-read; H-1 ist gefixt, dies hier ist
95+
# defense-in-depth. Bind-Mount-Konvention: Operator legt die Cookies-Datei
96+
# in das App-Daten-Volume, das standardmäßig auf /app/data gemountet wird.
97+
YOUTUBE_COOKIES_DIR = os.getenv("YOUTUBE_COOKIES_DIR", "/app/data")
98+
8999
# yt-dlp Anti-Detection / Rate-Limiting
90100
# ratelimit: Bytes/sec für jeden einzelnen Download. 1.5 MB/s ist fast nie
91101
# für die User wahrnehmbar (3-min Track in 12 s) und reduziert das Profil

backend/services/navidrome.py

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,22 @@ def get_target_path(self, track_info: Dict, file_extension: str, library_root: O
470470

471471
# Create directory: <root>/Artist/Album/
472472
target_dir = self._library_root(library_root) / artist_name / album_name
473+
474+
# H-5 defense-in-depth: containment-check VOR mkdir. Die sanitize-
475+
# Steps oben sollten Traversal eigentlich verhindern — der check
476+
# fängt zukünftige Sanitize-Lücken oder Caller die _sanitize_path
477+
# umgehen ab. relative_to raised ValueError wenn target_dir nicht
478+
# innerhalb library_root liegt.
479+
root_resolved = self._library_root(library_root).resolve()
480+
target_resolved = target_dir.resolve()
481+
try:
482+
target_resolved.relative_to(root_resolved)
483+
except ValueError:
484+
raise ValueError(
485+
f"Path-traversal blocked (Audit H-5): target {target_resolved} "
486+
f"outside library root {root_resolved}"
487+
)
488+
473489
target_dir.mkdir(parents=True, exist_ok=True)
474490

475491
# Build filename
@@ -577,22 +593,29 @@ def _trigger_scan(self) -> bool:
577593
return False
578594

579595
def _sanitize_path(self, path: str) -> str:
580-
"""Remove invalid characters from path"""
596+
"""Remove invalid characters from path; block path-traversal (Audit H-5).
597+
598+
Strippt erst dotdot-Sequenzen und führende Dots, dann die Standard-
599+
Zeichen-Blacklist. Ohne den dotdot-Fix konnte ein Track mit
600+
``album="../../etc"`` per ``root/artist/<traversed>`` außerhalb der
601+
Library landen — die Blacklist entfernte zwar den Slash, aber die
602+
zwei Punkte blieben, und mkdir hat sie als parent-traversal akzeptiert.
603+
"""
581604
import re
582-
# Remove invalid characters
605+
path = path.replace("..", "_")
606+
path = path.lstrip(".")
583607
path = re.sub(r'[<>:"/\\|?*]', '', path)
584-
# Replace multiple spaces with single space
585608
path = re.sub(r'\s+', ' ', path)
586-
# Trim
587-
return path.strip()
588-
609+
result = path.strip()
610+
return result or "_"
611+
589612
def _sanitize_filename(self, filename: str) -> str:
590-
"""Remove invalid characters from filename"""
613+
"""Remove invalid characters from filename; block path-traversal (Audit H-5)."""
591614
import re
592-
# Remove invalid characters
615+
filename = filename.replace("..", "_")
616+
filename = filename.lstrip(".")
593617
filename = re.sub(r'[<>:"/\\|?*]', '', filename)
594-
# Replace multiple spaces with single space
595618
filename = re.sub(r'\s+', ' ', filename)
596-
# Trim
597-
return filename.strip()
619+
result = filename.strip()
620+
return result or "_"
598621

backend/services/youtube.py

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -246,12 +246,40 @@ def _resolve_downloaded_audio(
246246
return None
247247

248248
def _add_cookies_to_opts(self, ydl_opts: Dict) -> Dict:
249-
"""Add cookies to yt-dlp options if configured."""
250-
if self.cookies_path and os.path.exists(self.cookies_path):
251-
ydl_opts['cookiefile'] = self.cookies_path
252-
print(f"Using YouTube cookies from: {self.cookies_path}")
253-
elif self.cookies_path:
249+
"""Add cookies to yt-dlp options if configured AND path-validated (Audit H-2).
250+
251+
Vier Validation-Stufen vor dem ``cookiefile``-Inject:
252+
1. Pfad existiert
253+
2. Pfad ist eine reguläre Datei (kein Symlink — sonst könnte ein
254+
Symlink-in-Cookies-Dir auf ``/etc/passwd`` zeigen)
255+
3. Extension ∈ {.txt, .netscape, .cookies} — yt-dlp erwartet
256+
Netscape-Format, andere Files werden silently rejected
257+
4. Resolved-Path liegt innerhalb von ``config.YOUTUBE_COOKIES_DIR``
258+
— verhindert arbitrary-file-read über UI-konfigurierbaren Pfad
259+
Bei Failure: Warning-Print + Skip (kein Throw, damit Resolver weiter
260+
läuft auch wenn Cookie-File misconfiguriert ist).
261+
"""
262+
if not self.cookies_path:
263+
return ydl_opts
264+
cp = Path(self.cookies_path)
265+
if not cp.exists():
254266
print(f"Warning: Cookie file specified but not found: {self.cookies_path}")
267+
return ydl_opts
268+
if cp.is_symlink() or not cp.is_file():
269+
print(f"Warning: Cookie path is symlink or not a regular file (H-2 reject): {self.cookies_path}")
270+
return ydl_opts
271+
if cp.suffix.lower() not in (".txt", ".netscape", ".cookies"):
272+
print(f"Warning: Cookie file extension not in whitelist (H-2 reject): {self.cookies_path}")
273+
return ydl_opts
274+
try:
275+
root_resolved = Path(config.YOUTUBE_COOKIES_DIR).resolve()
276+
cookies_resolved = cp.resolve()
277+
cookies_resolved.relative_to(root_resolved)
278+
except (ValueError, OSError):
279+
print(f"Warning: Cookie path outside YOUTUBE_COOKIES_DIR={config.YOUTUBE_COOKIES_DIR} (H-2 reject): {self.cookies_path}")
280+
return ydl_opts
281+
ydl_opts['cookiefile'] = str(cookies_resolved)
282+
print(f"Using YouTube cookies from: {self.cookies_path}")
255283
return ydl_opts
256284

257285
def calculate_similarity(self, str1: str, str2: str) -> float:

backend/utils/rate_limit.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
"""In-Memory Rate-Limiter als FastAPI-Dependency (Audit M-1, 2026-05-12).
2+
3+
Sliding-Window-Counter pro (client_ip, route) — verhindert DoS-Bursts und
4+
Quota-Burn auf den Bulk-Endpoints (CSV-Import, Spotify-History-Import, URL-
5+
Download, Track-Download). Nutzt die ``utils.auth.client_ip``-Helper mit dem
6+
H-7 trusted-proxy-Check, damit Bans nicht auf der Reverse-Proxy-IP collapsieren
7+
(slowapi's eingebauter ``get_remote_address`` würde das tun).
8+
9+
Design-Notes
10+
------------
11+
* **In-Memory, kein Redis** — Tonus läuft typisch als Single-Container; eine
12+
Process-Restart leert den State, was bei einem Patch-Deploy gewünscht ist
13+
("frischer Limit-Counter nach Restart, kein hängender Ban").
14+
* **deque mit pop-from-left** — O(1) für append + O(k) für Expiry-Cleanup,
15+
k = Anzahl entfernter alter Einträge. Klassisches Sliding-Window-Pattern.
16+
* **Bounded Memory** — wenn ein Client nichts mehr macht, leert sich seine
17+
Deque automatisch beim nächsten Request derselben IP+Route, weil alle
18+
Timestamps älter als Window sind und gepopt werden.
19+
* **Per-Route-Key** — selbe IP kann gleichzeitig nahe am CSV-Import-Limit
20+
UND am URL-Download-Limit sein, ohne sich gegenseitig zu blockieren.
21+
"""
22+
from __future__ import annotations
23+
24+
import time
25+
from collections import defaultdict, deque
26+
from typing import Callable, Deque, Dict
27+
28+
from fastapi import HTTPException, Request, status
29+
30+
31+
_rate_limit_state: Dict[str, Deque[float]] = defaultdict(deque)
32+
33+
34+
def make_rate_limiter(max_calls: int, window_seconds: int) -> Callable[[Request], None]:
35+
"""Erzeugt eine FastAPI-Dependency die bei Limit-Exceed 429 throwt.
36+
37+
Args:
38+
max_calls: Maximale Calls pro Window.
39+
window_seconds: Window-Größe in Sekunden.
40+
41+
Returns:
42+
Dependency-Funktion, mit ``Depends(...)`` einsetzbar.
43+
44+
Beispiel:
45+
>>> _limit_csv = make_rate_limiter(20, 3600)
46+
>>> @app.post("/api/import/csv")
47+
... async def import_csv(req: CsvImportRequest, _: None = Depends(_limit_csv)):
48+
... ...
49+
"""
50+
from utils.auth import client_ip # lazy import — vermeidet Circular
51+
52+
def dep(request: Request) -> None:
53+
ip = client_ip(request) or "unknown"
54+
key = f"{ip}:{request.url.path}"
55+
now = time.time()
56+
window = _rate_limit_state[key]
57+
# Alte Timestamps aus dem Window-Tail wegpoppen (sliding cleanup)
58+
cutoff = now - window_seconds
59+
while window and window[0] < cutoff:
60+
window.popleft()
61+
if len(window) >= max_calls:
62+
raise HTTPException(
63+
status_code=status.HTTP_429_TOO_MANY_REQUESTS,
64+
detail=f"Rate limit exceeded: {max_calls} requests / {window_seconds}s",
65+
headers={"Retry-After": str(window_seconds)},
66+
)
67+
window.append(now)
68+
69+
return dep

0 commit comments

Comments
 (0)