diff --git a/backend/app/main.py b/backend/app/main.py index 364bd70..f8c18ce 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1,6 +1,8 @@ """FastAPI entrypoint for the DMS backend service.""" -from fastapi import FastAPI, Request +from typing import Awaitable, Callable + +from fastapi import FastAPI, Request, Response from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse @@ -14,6 +16,18 @@ from app.services.typesense_index import ensure_typesense_collection settings = get_settings() +UPLOAD_ENDPOINT_PATH = "/api/v1/documents/upload" +UPLOAD_ENDPOINT_METHOD = "POST" + + +def _is_upload_size_guard_target(request: Request) -> bool: + """Returns whether upload request-size enforcement applies to this request. + + Upload-size validation is intentionally scoped to the upload POST endpoint so CORS + preflight OPTIONS requests can pass through CORSMiddleware. + """ + + return request.method.upper() == UPLOAD_ENDPOINT_METHOD and request.url.path == UPLOAD_ENDPOINT_PATH def create_app() -> FastAPI: @@ -30,10 +44,13 @@ def create_app() -> FastAPI: app.include_router(api_router, prefix="/api/v1") @app.middleware("http") - async def enforce_upload_request_size(request: Request, call_next): - """Rejects upload requests without deterministic length or exceeding configured limits.""" + async def enforce_upload_request_size( + request: Request, + call_next: Callable[[Request], Awaitable[Response]], + ) -> Response: + """Rejects only POST upload bodies without deterministic length or with oversized request totals.""" - if request.url.path.endswith("/api/v1/documents/upload"): + if _is_upload_size_guard_target(request): content_length = request.headers.get("content-length", "").strip() if not content_length: return JSONResponse( diff --git a/backend/tests/test_upload_request_size_middleware.py b/backend/tests/test_upload_request_size_middleware.py new file mode 100644 index 0000000..43efa21 --- /dev/null +++ b/backend/tests/test_upload_request_size_middleware.py @@ -0,0 +1,270 @@ +"""Regression tests for upload request-size middleware scope and preflight handling.""" + +from __future__ import annotations + +import importlib +import sys +import unittest +from pathlib import Path +from types import ModuleType, SimpleNamespace +from typing import Any, Awaitable, Callable + + +BACKEND_ROOT = Path(__file__).resolve().parents[1] +if str(BACKEND_ROOT) not in sys.path: + sys.path.insert(0, str(BACKEND_ROOT)) + + +def _install_main_import_stubs() -> dict[str, ModuleType | None]: + """Installs lightweight module stubs required for importing app.main in isolation.""" + + previous_modules: dict[str, ModuleType | None] = { + name: sys.modules.get(name) + for name in [ + "fastapi", + "fastapi.middleware", + "fastapi.middleware.cors", + "fastapi.responses", + "app.api.router", + "app.core.config", + "app.db.base", + "app.services.app_settings", + "app.services.handwriting_style", + "app.services.storage", + "app.services.typesense_index", + ] + } + + fastapi_stub = ModuleType("fastapi") + + class _Response: + """Minimal response base class for middleware typing compatibility.""" + + class _FastAPI: + """Captures middleware registration behavior used by app.main tests.""" + + def __init__(self, *_args: object, **_kwargs: object) -> None: + self.http_middlewares: list[Any] = [] + + def add_middleware(self, *_args: object, **_kwargs: object) -> None: + """Accepts middleware registrations without side effects.""" + + def include_router(self, *_args: object, **_kwargs: object) -> None: + """Accepts router registration without side effects.""" + + def middleware( + self, + middleware_type: str, + ) -> Callable[[Callable[..., Any]], Callable[..., Any]]: + """Registers request middleware functions for later invocation in tests.""" + + def decorator(func: Callable[..., Any]) -> Callable[..., Any]: + if middleware_type == "http": + self.http_middlewares.append(func) + return func + + return decorator + + def on_event( + self, + *_args: object, + **_kwargs: object, + ) -> Callable[[Callable[..., Any]], Callable[..., Any]]: + """Returns no-op startup and shutdown decorators.""" + + def decorator(func: Callable[..., Any]) -> Callable[..., Any]: + return func + + return decorator + + fastapi_stub.FastAPI = _FastAPI + fastapi_stub.Request = object + fastapi_stub.Response = _Response + sys.modules["fastapi"] = fastapi_stub + + fastapi_middleware_stub = ModuleType("fastapi.middleware") + sys.modules["fastapi.middleware"] = fastapi_middleware_stub + + fastapi_middleware_cors_stub = ModuleType("fastapi.middleware.cors") + + class _CORSMiddleware: + """Placeholder CORS middleware class accepted by FastAPI.add_middleware.""" + + fastapi_middleware_cors_stub.CORSMiddleware = _CORSMiddleware + sys.modules["fastapi.middleware.cors"] = fastapi_middleware_cors_stub + + fastapi_responses_stub = ModuleType("fastapi.responses") + + class _JSONResponse: + """Simple JSONResponse stand-in exposing status code and payload fields.""" + + def __init__(self, *, status_code: int, content: dict[str, Any]) -> None: + self.status_code = status_code + self.content = content + + fastapi_responses_stub.JSONResponse = _JSONResponse + sys.modules["fastapi.responses"] = fastapi_responses_stub + + api_router_stub = ModuleType("app.api.router") + api_router_stub.api_router = object() + sys.modules["app.api.router"] = api_router_stub + + config_stub = ModuleType("app.core.config") + + def get_settings() -> SimpleNamespace: + """Returns minimal settings consumed by app.main during test import.""" + + return SimpleNamespace( + cors_origins=["http://localhost:5173"], + max_upload_request_size_bytes=1024, + ) + + config_stub.get_settings = get_settings + sys.modules["app.core.config"] = config_stub + + db_base_stub = ModuleType("app.db.base") + + def init_db() -> None: + """No-op database initializer for middleware scope tests.""" + + db_base_stub.init_db = init_db + sys.modules["app.db.base"] = db_base_stub + + app_settings_stub = ModuleType("app.services.app_settings") + + def ensure_app_settings() -> None: + """No-op settings initializer for middleware scope tests.""" + + app_settings_stub.ensure_app_settings = ensure_app_settings + sys.modules["app.services.app_settings"] = app_settings_stub + + handwriting_style_stub = ModuleType("app.services.handwriting_style") + + def ensure_handwriting_style_collection() -> None: + """No-op handwriting collection initializer for middleware scope tests.""" + + handwriting_style_stub.ensure_handwriting_style_collection = ensure_handwriting_style_collection + sys.modules["app.services.handwriting_style"] = handwriting_style_stub + + storage_stub = ModuleType("app.services.storage") + + def ensure_storage() -> None: + """No-op storage initializer for middleware scope tests.""" + + storage_stub.ensure_storage = ensure_storage + sys.modules["app.services.storage"] = storage_stub + + typesense_stub = ModuleType("app.services.typesense_index") + + def ensure_typesense_collection() -> None: + """No-op Typesense collection initializer for middleware scope tests.""" + + typesense_stub.ensure_typesense_collection = ensure_typesense_collection + sys.modules["app.services.typesense_index"] = typesense_stub + + return previous_modules + + +def _restore_main_import_stubs(previous_modules: dict[str, ModuleType | None]) -> None: + """Restores module table entries captured before installing app.main test stubs.""" + + for module_name, previous in previous_modules.items(): + if previous is None: + sys.modules.pop(module_name, None) + else: + sys.modules[module_name] = previous + + +class UploadRequestSizeMiddlewareTests(unittest.IsolatedAsyncioTestCase): + """Verifies upload request-size middleware ignores preflight and guards only upload POST.""" + + @classmethod + def setUpClass(cls) -> None: + """Installs import stubs and imports app.main once for middleware extraction.""" + + cls._previous_modules = _install_main_import_stubs() + cls.main_module = importlib.import_module("app.main") + + @classmethod + def tearDownClass(cls) -> None: + """Removes imported module and restores pre-existing module table entries.""" + + sys.modules.pop("app.main", None) + _restore_main_import_stubs(cls._previous_modules) + + def _http_middleware( + self, + ) -> Callable[[object, Callable[[object], Awaitable[object]]], Awaitable[object]]: + """Returns the registered HTTP middleware callable from the stubbed FastAPI app.""" + + return self.main_module.app.http_middlewares[0] + + async def test_options_preflight_skips_upload_content_length_guard(self) -> None: + """OPTIONS preflight requests for upload endpoint continue without Content-Length enforcement.""" + + request = SimpleNamespace( + method="OPTIONS", + url=SimpleNamespace(path="/api/v1/documents/upload"), + headers={}, + ) + expected_response = object() + call_next_count = 0 + + async def call_next(_request: object) -> object: + nonlocal call_next_count + call_next_count += 1 + return expected_response + + response = await self._http_middleware()(request, call_next) + + self.assertIs(response, expected_response) + self.assertEqual(call_next_count, 1) + + async def test_post_upload_without_content_length_is_rejected(self) -> None: + """Upload POST requests remain blocked when Content-Length is absent.""" + + request = SimpleNamespace( + method="POST", + url=SimpleNamespace(path="/api/v1/documents/upload"), + headers={}, + ) + call_next_count = 0 + + async def call_next(_request: object) -> object: + nonlocal call_next_count + call_next_count += 1 + return object() + + response = await self._http_middleware()(request, call_next) + + self.assertEqual(response.status_code, 411) + self.assertEqual( + response.content, + {"detail": "Content-Length header is required for document uploads"}, + ) + self.assertEqual(call_next_count, 0) + + async def test_post_non_upload_path_skips_upload_content_length_guard(self) -> None: + """Content-Length enforcement does not run for non-upload POST requests.""" + + request = SimpleNamespace( + method="POST", + url=SimpleNamespace(path="/api/v1/documents"), + headers={}, + ) + expected_response = object() + call_next_count = 0 + + async def call_next(_request: object) -> object: + nonlocal call_next_count + call_next_count += 1 + return expected_response + + response = await self._http_middleware()(request, call_next) + + self.assertIs(response, expected_response) + self.assertEqual(call_next_count, 1) + + +if __name__ == "__main__": + unittest.main() diff --git a/doc/README.md b/doc/README.md index 6e63f07..0803de6 100644 --- a/doc/README.md +++ b/doc/README.md @@ -9,4 +9,4 @@ This directory contains technical documentation for DMS. - `api-contract.md` - API endpoint contract grouped by route module, including token auth roles, upload limits, and settings or processing-log security constraints - `data-model-reference.md` - database entity definitions and lifecycle states - `operations-and-configuration.md` - runtime operations, hardened compose defaults, security environment variables, and persisted settings configuration and read-sanitization behavior -- `frontend-design-foundation.md` - frontend visual system, tokens, UI implementation rules, processing-log timeline behavior, and settings helper-copy guidance +- `frontend-design-foundation.md` - frontend visual system, tokens, UI implementation rules, authenticated media delivery under API token auth, processing-log timeline behavior, and settings helper-copy guidance diff --git a/doc/api-contract.md b/doc/api-contract.md index 8fa94c7..0b5ac8d 100644 --- a/doc/api-contract.md +++ b/doc/api-contract.md @@ -89,7 +89,8 @@ Primary implementation modules: - `ask`: returns `conflicts` if duplicate checksum is detected - `replace`: creates new document linked to replaced document id - `duplicate`: creates additional document record - - request rejected with `411` when `Content-Length` is missing + - upload `POST` request rejected with `411` when `Content-Length` is missing + - `OPTIONS /documents/upload` CORS preflight bypasses upload `Content-Length` enforcement - request rejected with `413` when file count, per-file size, or total request size exceeds configured limits ## Search diff --git a/doc/frontend-design-foundation.md b/doc/frontend-design-foundation.md index 35016f0..48c5897 100644 --- a/doc/frontend-design-foundation.md +++ b/doc/frontend-design-foundation.md @@ -49,6 +49,13 @@ Do not hardcode new palette or spacing values in component styles when a token a - Do not render queued headers before their animation starts, even when polling returns batched updates. - Preserve existing header content format and fold/unfold detail behavior as lines are revealed. +## Authenticated Media Delivery + +- Document previews and thumbnails must load through authenticated fetch flows in `frontend/src/lib/api.ts`, then render via temporary object URLs. +- Direct `window.open` calls for protected media endpoints are not allowed because browser navigation requests do not include the API token header. +- Download actions for original files and markdown exports must use authenticated blob fetches plus controlled browser download triggers. +- Revoke all temporary object URLs after replacement, unmount, or completion to prevent browser memory leaks. + ## Extension Checklist When adding or redesigning a UI area: diff --git a/doc/operations-and-configuration.md b/doc/operations-and-configuration.md index 5dabc54..bf19a35 100644 --- a/doc/operations-and-configuration.md +++ b/doc/operations-and-configuration.md @@ -149,7 +149,8 @@ Retention settings are used by worker cleanup and by `POST /api/v1/processing/lo - scheme restrictions (`https` by default) - local/private-network blocking and per-request DNS revalidation checks for outbound runtime calls - Upload and archive safety guards are enforced: - - multipart upload requires `Content-Length` and enforces file-count, per-file size, and total request size limits + - `POST /api/v1/documents/upload` requires `Content-Length` and enforces file-count, per-file size, and total request size limits + - `OPTIONS /api/v1/documents/upload` CORS preflight is excluded from `Content-Length` enforcement - ZIP member count, per-member uncompressed size, total decompressed size, and compression-ratio guards - Processing logs redact sensitive payload and text fields, and trim endpoints enforce retention caps from runtime config. - Compose hardening defaults: diff --git a/frontend/package.json b/frontend/package.json index 947a504..78c84ec 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -5,6 +5,7 @@ "type": "module", "scripts": { "dev": "vite", + "test": "node --experimental-strip-types src/lib/api.test.ts", "build": "tsc -b && vite build", "preview": "vite preview --host 0.0.0.0 --port 4173" }, diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 73829ba..f50ed95 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -14,6 +14,7 @@ import SettingsScreen from './components/SettingsScreen'; import UploadSurface from './components/UploadSurface'; import { clearProcessingLogs, + downloadBlobFile, deleteDocument, exportContentsMarkdown, getAppSettings, @@ -117,15 +118,6 @@ export default function App(): JSX.Element { } }, []); - const downloadBlob = useCallback((blob: Blob, filename: string): void => { - const objectUrl = URL.createObjectURL(blob); - const anchor = document.createElement('a'); - anchor.href = objectUrl; - anchor.download = filename; - anchor.click(); - URL.revokeObjectURL(objectUrl); - }, []); - const loadCatalogs = useCallback(async (): Promise => { const [tags, paths, types] = await Promise.all([listTags(true), listPaths(true), listTypes(true)]); setKnownTags(tags); @@ -465,13 +457,13 @@ export default function App(): JSX.Element { only_trashed: documentView === 'trash', include_trashed: false, }); - downloadBlob(result.blob, result.filename); + downloadBlobFile(result.blob, result.filename); } catch (caughtError) { setError(caughtError instanceof Error ? caughtError.message : 'Failed to export selected markdown files'); } finally { setIsRunningBulkAction(false); } - }, [documentView, downloadBlob, selectedDocumentIds]); + }, [documentView, selectedDocumentIds]); const handleExportPath = useCallback(async (): Promise => { const trimmedPrefix = exportPathInput.trim(); @@ -487,13 +479,13 @@ export default function App(): JSX.Element { only_trashed: documentView === 'trash', include_trashed: false, }); - downloadBlob(result.blob, result.filename); + downloadBlobFile(result.blob, result.filename); } catch (caughtError) { setError(caughtError instanceof Error ? caughtError.message : 'Failed to export path markdown files'); } finally { setIsRunningBulkAction(false); } - }, [documentView, downloadBlob, exportPathInput]); + }, [documentView, exportPathInput]); const handleSaveSettings = useCallback(async (payload: AppSettingsUpdate): Promise => { setIsSavingSettings(true); diff --git a/frontend/src/components/DocumentCard.tsx b/frontend/src/components/DocumentCard.tsx index 394de50..b4acc99 100644 --- a/frontend/src/components/DocumentCard.tsx +++ b/frontend/src/components/DocumentCard.tsx @@ -1,12 +1,17 @@ /** * Card view for displaying document summary, preview, and metadata. */ -import { useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import type { JSX } from 'react'; import { Download, FileText, Trash2 } from 'lucide-react'; import type { DmsDocument } from '../types'; -import { contentMarkdownUrl, downloadUrl, thumbnailUrl } from '../lib/api'; +import { + downloadBlobFile, + downloadDocumentContentMarkdown, + downloadDocumentFile, + getDocumentThumbnailBlob, +} from '../lib/api'; /** * Defines properties accepted by the document card component. @@ -79,12 +84,59 @@ export default function DocumentCard({ onFilterTag, }: DocumentCardProps): JSX.Element { const [isTrashing, setIsTrashing] = useState(false); + const [thumbnailObjectUrl, setThumbnailObjectUrl] = useState(null); + const thumbnailObjectUrlRef = useRef(null); const createdDate = new Date(document.created_at).toLocaleString(); const status = statusPresentation(document.status); const compactPath = compactLogicalPath(document.logical_path, 180); const trashDisabled = isTrashView || document.status === 'trashed' || isTrashing; const trashTitle = trashDisabled ? 'Already in trash' : 'Move to trash'; + /** + * Loads thumbnail preview through authenticated fetch and revokes replaced object URLs. + */ + useEffect(() => { + const revokeThumbnailObjectUrl = (): void => { + if (!thumbnailObjectUrlRef.current) { + return; + } + URL.revokeObjectURL(thumbnailObjectUrlRef.current); + thumbnailObjectUrlRef.current = null; + }; + + if (!document.preview_available) { + revokeThumbnailObjectUrl(); + setThumbnailObjectUrl(null); + return; + } + + let cancelled = false; + const loadThumbnail = async (): Promise => { + try { + const blob = await getDocumentThumbnailBlob(document.id); + if (cancelled) { + return; + } + revokeThumbnailObjectUrl(); + const objectUrl = URL.createObjectURL(blob); + thumbnailObjectUrlRef.current = objectUrl; + setThumbnailObjectUrl(objectUrl); + } catch { + if (cancelled) { + return; + } + revokeThumbnailObjectUrl(); + setThumbnailObjectUrl(null); + } + }; + + void loadThumbnail(); + return () => { + cancelled = true; + revokeThumbnailObjectUrl(); + }; + }, [document.id, document.preview_available]); + return (
- {document.preview_available ? ( - {document.original_filename} + {document.preview_available && thumbnailObjectUrl ? ( + {document.original_filename} ) : (
{document.extension || 'file'}
)} @@ -173,7 +225,13 @@ export default function DocumentCard({ onClick={(event) => { event.preventDefault(); event.stopPropagation(); - window.open(downloadUrl(document.id), '_blank', 'noopener,noreferrer'); + void (async (): Promise => { + try { + const payload = await downloadDocumentFile(document.id); + downloadBlobFile(payload.blob, payload.filename); + } catch { + } + })(); }} >