diff --git a/Security-audit.md b/Security-audit.md index 7a99bc8..b033238 100644 --- a/Security-audit.md +++ b/Security-audit.md @@ -54,13 +54,17 @@ The application should remain behind the production reverse proxy, with real DNS ### SA-002: Raw infrastructure errors are returned to clients -**Severity:** High -**Evidence:** SMTP and Mastodon routes interpolate exception text into `503`/`502` responses. Setup and email-address routes also expose mail-delivery exception text. +**Severity:** High, remediated in current worktree +**Evidence before remediation:** SMTP and Mastodon routes interpolated exception text into `503`/`502` responses. Setup and email-address routes also exposed mail-delivery exception text. **Impact:** Error responses can disclose SMTP hostnames, ports, TLS/library details, upstream response bodies, internal network information, or sensitive URL fragments. -**Recommendation:** Log technical details server-side with a request/correlation ID and return a stable public message with a short reference ID. Redact credentials, authorization headers, reset tokens, OTP data, and secret-bearing URLs. Add tests asserting that representative exception text is absent from HTTP responses. +**Current state:** The application assigns a request ID at middleware entry, returns it in `X-Request-ID`, logs technical exception summaries server-side after redacting authorization values, tokens, passwords, secrets, OTP/code values, and secret-bearing URL query values, and returns stable public messages with a reference ID. SMTP setup/admin/email errors and Mastodon registration/callback errors no longer expose raw exception text. Regression tests verify representative exception and secret text is absent from HTTP responses. -**Priority:** High. +**Residual impact:** Logging currently uses the application logger rather than a centralized protected sink. Request-ID trust, log retention, access control, and structured redaction should be reviewed in deployment. + +**Recommendation:** Keep public errors stable and reference-based, export redacted logs to a protected centralized system, define retention and access controls, and never log authorization headers or secret-bearing request data. + +**Priority:** Completed in code; centralized logging and operational controls remain. ### SA-003: First-run setup is unauthenticated and lacks application-level body limits diff --git a/VIBE/CHAT_LOG.md b/VIBE/CHAT_LOG.md index 17de7ae..f06e950 100644 --- a/VIBE/CHAT_LOG.md +++ b/VIBE/CHAT_LOG.md @@ -1,5 +1,11 @@ # Chat Log +### User +For SA-002, log technical details server-side with a request/correlation ID, return a stable public message with a short reference ID, redact secrets, and test that exception text is absent from responses. + +### Assistant outcome +Added request-ID middleware and stable public error references. SMTP, setup, email-verification, and Mastodon exception paths now log redacted technical summaries server-side without exposing raw exception text, credentials, authorization values, reset tokens, OTP data, or secret-bearing URL values. Added regression coverage and updated SA-002 documentation. + ### User Remedy SA-001: Logout uses non-standard token transport. diff --git a/VIBE/PROMPTS.md b/VIBE/PROMPTS.md index 1f4c18b..dcb1339 100644 --- a/VIBE/PROMPTS.md +++ b/VIBE/PROMPTS.md @@ -201,6 +201,7 @@ 195. Perform a new security audit overwriting Security-audit.md with new and remaining issues. 195. Update SA-012 and README for the implemented refresh-token lifecycle, revocation behavior, and refresh endpoint. 196. Remedy SA-001: migrate logout from JSON token transport to the Authorization bearer header. +197. Implement SA-002: replace raw infrastructure errors with redacted server-side logging, request IDs, and stable public reference messages. ## Future entries diff --git a/backend/app/api/admin.py b/backend/app/api/admin.py index a644309..a859f85 100644 --- a/backend/app/api/admin.py +++ b/backend/app/api/admin.py @@ -5,7 +5,7 @@ import json from datetime import datetime, timedelta, timezone from uuid import uuid4 -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Request from pydantic import BaseModel from backend.app.api.dependencies import require_admin @@ -23,8 +23,11 @@ from backend.app.services.theme_service import THEMES, get_enabled_themes, save_ from backend.app.services.secret_store import encrypt_secret from backend.app.core.config import settings from backend.app.services.audit_service import record_audit_event +from backend.app.core.errors import public_error, redacted_error, request_id +import logging router = APIRouter() +logger = logging.getLogger(__name__) class AdminPluginUpdate(BaseModel): @@ -112,7 +115,7 @@ def reset_user_otp(user_id: str, current_user: dict = Depends(require_admin)): @router.post('/users', status_code=201) -def create_user(payload: AdminUserCreate, current_user: dict = Depends(require_admin)): +def create_user(payload: AdminUserCreate, request: Request, current_user: dict = Depends(require_admin)): username = payload.username.strip() email = payload.email.strip() if not username or not email or len(payload.password) < 8: @@ -142,7 +145,8 @@ def create_user(payload: AdminUserCreate, current_user: dict = Depends(require_a try: send_verification_email(row['email'], row['username'], verification_url) except Exception as error: - raise HTTPException(status_code=503, detail=f'User created but verification email could not be sent: {error}') from error + logger.error('User verification email failed request_id=%s error=%s', request_id(request), redacted_error(error)) + raise HTTPException(status_code=503, detail=public_error(request, 'User created but verification email could not be sent.')) from error record_audit_event(current_user['id'], 'user_created', 'user', row['id'], details={'is_admin': bool(payload.is_admin)}) return public_user(row) @@ -188,7 +192,7 @@ def update_admin_smtp_settings(payload: AdminSmtpUpdate, current_user: dict = De @router.post('/smtp/test') -def validate_admin_smtp(payload: AdminSmtpUpdate, current_user: dict = Depends(require_admin)): +def validate_admin_smtp(payload: AdminSmtpUpdate, request: Request, current_user: dict = Depends(require_admin)): values = validate_smtp_values(payload, get_smtp_settings()) now = datetime.now(timezone.utc) with get_connection() as conn: @@ -209,7 +213,8 @@ def validate_admin_smtp(payload: AdminSmtpUpdate, current_user: dict = Depends(r try: send_test_email(current_user['email'], values) except Exception as error: - raise HTTPException(status_code=503, detail=f'SMTP validation failed: {error}') from error + logger.error('SMTP validation failed request_id=%s error=%s', request_id(request), redacted_error(error)) + raise HTTPException(status_code=503, detail=public_error(request, 'SMTP validation failed.')) from error sends = int(rate.get('sends', 0)) + 1 updated_rate = {'sends': sends, 'last_sent': now.isoformat()} if sends >= 5: diff --git a/backend/app/api/mastodon.py b/backend/app/api/mastodon.py index 9a1d772..28f0ec4 100644 --- a/backend/app/api/mastodon.py +++ b/backend/app/api/mastodon.py @@ -10,12 +10,15 @@ from starlette.requests import Request from backend.app.api.dependencies import get_current_user from backend.app.services.mastodon_oauth import finish_authorization, start_authorization +from backend.app.core.errors import public_error, redacted_error, request_id +import logging router = APIRouter() +logger = logging.getLogger(__name__) @router.get('/oauth/start') -def oauth_start(instance: str = 'mastodon.social', user: dict = Depends(get_current_user)): +def oauth_start(request: Request, instance: str = 'mastodon.social', user: dict = Depends(get_current_user)): try: authorization_url = start_authorization(user['id'], instance) except HTTPError as error: @@ -27,7 +30,8 @@ def oauth_start(instance: str = 'mastodon.social', user: dict = Depends(get_curr headers=headers, ) from error except Exception as error: - raise HTTPException(status_code=502, detail=f'Could not register with Mastodon: {error}') from error + logger.error('Mastodon registration failed request_id=%s error=%s', request_id(request), redacted_error(error)) + raise HTTPException(status_code=502, detail=public_error(request, 'Could not register with Mastodon.')) from error return {'authorization_url': authorization_url} @@ -38,5 +42,6 @@ def oauth_callback(request: Request, code: str | None = None, state: str | None try: finish_authorization(code, state) except Exception as callback_error: - return RedirectResponse(f'/profile?mastodon_error={quote(str(callback_error))}') + logger.error('Mastodon callback failed request_id=%s error=%s', request_id(request), redacted_error(callback_error)) + return RedirectResponse(f'/profile?mastodon_error={quote(public_error(request, "Could not complete Mastodon authorization."))}') return RedirectResponse('/profile?mastodon=connected') \ No newline at end of file diff --git a/backend/app/api/setup.py b/backend/app/api/setup.py index be00deb..664e5f3 100644 --- a/backend/app/api/setup.py +++ b/backend/app/api/setup.py @@ -5,14 +5,17 @@ from datetime import datetime, timedelta, timezone import json from uuid import uuid4 -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter, HTTPException, Request from pydantic import BaseModel from backend.app.core.config import settings from backend.app.database import get_connection, hash_password from backend.app.services.email_service import get_smtp_settings, save_smtp_settings, send_test_email +from backend.app.core.errors import public_error, redacted_error, request_id +import logging router = APIRouter() +logger = logging.getLogger(__name__) class SetupRequest(BaseModel): @@ -92,7 +95,7 @@ def save_configuration(payload: SetupRequest): @router.post('/test-mail') -def test_mail(payload: TestMailRequest | None = None): +def test_mail(request: Request, payload: TestMailRequest | None = None): if has_administrator(): raise HTTPException(status_code=409, detail='LinkLog is already configured') pending = get_pending_setup() @@ -119,7 +122,8 @@ def test_mail(payload: TestMailRequest | None = None): try: send_test_email(payload.email if payload and payload.email else pending['email']) except Exception as error: - raise HTTPException(status_code=503, detail=f'SMTP test mail could not be sent: {error}') from error + logger.error('SMTP test mail failed request_id=%s error=%s', request_id(request), redacted_error(error)) + raise HTTPException(status_code=503, detail=public_error(request, 'SMTP test mail could not be sent.')) from error sends = int(rate.get('sends', 0)) + 1 updated_rate = {'sends': sends, 'last_sent': now.isoformat()} diff --git a/backend/app/api/user_config.py b/backend/app/api/user_config.py index fd2f781..cbf7fa3 100644 --- a/backend/app/api/user_config.py +++ b/backend/app/api/user_config.py @@ -7,7 +7,7 @@ from io import BytesIO from datetime import datetime, timedelta, timezone from uuid import uuid4 -from fastapi import APIRouter, Depends, File, HTTPException, UploadFile +from fastapi import APIRouter, Depends, File, HTTPException, Request, UploadFile from PIL import Image, UnidentifiedImageError from pydantic import BaseModel @@ -20,8 +20,11 @@ from backend.app.services.email_service import send_verification_email, smtp_con from backend.app.core.config import settings from backend.app.services.secret_store import decrypt_secret, encrypt_secret from backend.app.services.audit_service import record_audit_event +from backend.app.core.errors import public_error, redacted_error, request_id +import logging router = APIRouter() +logger = logging.getLogger(__name__) MAX_AVATAR_BYTES = 2 * 1024 * 1024 MAX_AVATAR_PIXELS = 25_000_000 @@ -186,7 +189,7 @@ def get_additional_emails(user: dict = Depends(get_current_user)): @router.post('/emails', status_code=201) -def add_additional_email(payload: AdditionalEmail, user: dict = Depends(get_current_user)): +def add_additional_email(payload: AdditionalEmail, request: Request, user: dict = Depends(get_current_user)): email = payload.email.strip().lower() if email == user['email'].lower(): raise HTTPException(status_code=409, detail='This is already the primary email address') @@ -209,7 +212,8 @@ def add_additional_email(payload: AdditionalEmail, user: dict = Depends(get_curr try: send_verification_email(email_address, user['username'], verification_url) except Exception as error: - raise HTTPException(status_code=503, detail=f'Email address added but verification email could not be sent: {error}') from error + logger.error('Additional email verification failed request_id=%s error=%s', request_id(request), redacted_error(error)) + raise HTTPException(status_code=503, detail=public_error(request, 'Email address added but verification email could not be sent.')) from error with get_connection() as conn: conn.execute( '''INSERT INTO app_settings (name, value, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP) @@ -221,7 +225,7 @@ def add_additional_email(payload: AdditionalEmail, user: dict = Depends(get_curr @router.post('/emails/{address_id}/resend') -def resend_additional_email(address_id: str, user: dict = Depends(get_current_user)): +def resend_additional_email(address_id: str, request: Request, user: dict = Depends(get_current_user)): now = datetime.now(timezone.utc) setting_name = f'email_verify_rate:{address_id}' with get_connection() as conn: @@ -245,7 +249,8 @@ def resend_additional_email(address_id: str, user: dict = Depends(get_current_us try: send_verification_email(email, user['username'], verification_url) except Exception as error: - raise HTTPException(status_code=503, detail=f'Verification email could not be sent: {error}') from error + logger.error('Verification email resend failed request_id=%s error=%s', request_id(request), redacted_error(error)) + raise HTTPException(status_code=503, detail=public_error(request, 'Verification email could not be sent.')) from error sends = int(rate.get('sends', 0)) + 1 updated = {'sends': sends, 'last_sent': now.isoformat()} if sends >= 5: diff --git a/backend/app/core/errors.py b/backend/app/core/errors.py new file mode 100644 index 0000000..e7dbc67 --- /dev/null +++ b/backend/app/core/errors.py @@ -0,0 +1,23 @@ +import re +from uuid import uuid4 + +from fastapi import Request + + +SENSITIVE_PATTERN = re.compile( + r'(?i)(authorization\s*[:=]\s*bearer\s+[^\s,;]+|' + r'(?:token|password|secret|otp|code)(?:[_-](?:token|password|secret|code))?\s*[:=]\s*[^\s,;&]+|' + r'([?&](?:token|code|password|secret|otp)=[^&#\s]+))' +) + + +def request_id(request: Request) -> str: + return getattr(request.state, 'request_id', None) or str(uuid4()) + + +def redacted_error(error: Exception) -> str: + return SENSITIVE_PATTERN.sub('[REDACTED]', str(error)) + + +def public_error(request: Request, message: str) -> str: + return f'{message} Reference: {request_id(request)}' \ No newline at end of file diff --git a/backend/app/main.py b/backend/app/main.py index 51e59ae..8bc5649 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1,8 +1,9 @@ ## Copyright © 2026 Olaf Kolkman ## SPDX-License-Identifier: GPL-3.0-or-later -from fastapi import FastAPI +from fastapi import FastAPI, Request import logging +from uuid import uuid4 from fastapi.responses import HTMLResponse from fastapi.responses import RedirectResponse from fastapi.staticfiles import StaticFiles @@ -24,6 +25,15 @@ from backend.app.services.link_service import get_public_profile, list_public_li logging.basicConfig(level=getattr(logging, settings.log_level, logging.INFO)) app = FastAPI(title='LinkLog API', version=settings.version) + + +@app.middleware('http') +async def add_request_id(request: Request, call_next): + request.state.request_id = request.headers.get('X-Request-ID') or str(uuid4()) + response = await call_next(request) + response.headers['X-Request-ID'] = request.state.request_id + return response + app.mount('/static', StaticFiles(directory='frontend/static'), name='static') app.mount('/media', StaticFiles(directory=AVATARS_DIR), name='media') app.include_router(auth_router, prefix='/api/auth') diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index c9dab5d..66f7b62 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -227,7 +227,31 @@ def test_admin_reports_smtp_validation_errors(): 'smtp_use_tls': False, }) assert failed_validation.status_code == 503 - assert 'connection refused' in failed_validation.json()['detail'] + assert failed_validation.json()['detail'].startswith('SMTP validation failed. Reference: ') + assert 'connection refused' not in failed_validation.json()['detail'] + assert failed_validation.headers['X-Request-ID'] + + +def test_request_id_is_preserved_and_sensitive_error_text_is_not_returned(): + headers = login_headers() + with get_connection() as conn: + conn.execute('DELETE FROM app_settings WHERE name = ?', ('admin_smtp_mail_rate',)) + conn.commit() + with patch('backend.app.api.admin.send_test_email', side_effect=RuntimeError('password=super-secret token=abc123')): + response = client.post( + '/api/admin/smtp/test', + headers={**headers, 'X-Request-ID': 'audit-test-123'}, + json={ + 'smtp_host': 'smtp.example.com', + 'smtp_port': 2525, + 'smtp_from': 'admin@example.com', + }, + ) + assert response.status_code == 503 + assert response.headers['X-Request-ID'] == 'audit-test-123' + assert response.json()['detail'] == 'SMTP validation failed. Reference: audit-test-123' + assert 'super-secret' not in response.text + assert 'abc123' not in response.text def test_admin_can_add_list_and_remove_users():