--- phase: 08-stack-upgrade-backend-decomposition plan: 05 type: execute wave: 2 depends_on: [08-03] files_modified: - backend/api/documents/__init__.py - backend/api/documents/upload.py - backend/api/documents/crud.py - backend/api/documents/content.py - backend/api/documents/shared.py - backend/api/documents.py autonomous: true requirements: [CODE-02, CODE-08] tags: [backend-decomposition, documents, sub-router] must_haves: truths: - "backend/api/documents/ is a Python package with sub-modules upload.py, crud.py, content.py, shared.py and __init__.py" - "All document endpoints respond on the same URL paths as before" - "Every document endpoint enforces get_current_user (regular user) and the ownership assertion `resource.user_id == current_user.id`" - "_CLOUD_PROVIDERS frozenset and shared Pydantic models live in shared.py, not duplicated across sub-modules" - "No sub-router declares a prefix; only api/documents/__init__.py carries prefix=/api/documents" - "Old backend/api/documents.py monolith is deleted" - "main.py import `from api.documents import router as documents_router` continues to work" artifacts: - path: "backend/api/documents/__init__.py" provides: "Router aggregator with prefix=/api/documents; includes upload_router, crud_router, content_router" contains: "router = APIRouter(prefix" - path: "backend/api/documents/upload.py" provides: "Presigned URL + direct upload + confirm handlers (POST /upload-url, POST /upload, POST /{id}/confirm)" contains: "async def request_upload_url" - path: "backend/api/documents/crud.py" provides: "List/get/patch/delete + re-classify endpoint per D-08" contains: "async def list_documents" - path: "backend/api/documents/content.py" provides: "Range-aware content streaming endpoint + _parse_range helper" contains: "async def stream_document_content" - path: "backend/api/documents/shared.py" provides: "UploadUrlRequest, DocumentPatch Pydantic models, _CLOUD_PROVIDERS constant" contains: "_CLOUD_PROVIDERS = frozenset" key_links: - from: "backend/main.py" to: "backend/api/documents/__init__.py" via: "from api.documents import router as documents_router" pattern: "from api.documents import router as documents_router" - from: "backend/api/documents/upload.py and crud.py" to: "backend/api/documents/shared.py" via: "from api.documents.shared import _CLOUD_PROVIDERS, UploadUrlRequest, DocumentPatch" pattern: "from api.documents.shared import" --- Decompose the 852-line `backend/api/documents.py` monolith into the focused package `backend/api/documents/` per locked D-06 (4 sub-modules) and D-08 (re-classify endpoint placement is researcher/planner choice — placed in `crud.py` because it operates on an existing document with the same ownership-check pattern as get/patch/delete). Zero URL changes, zero behavior changes. Purpose: CODE-02 (decomposition) + CODE-08 (single-definition discipline for `UploadUrlRequest`, `DocumentPatch`, `_CLOUD_PROVIDERS`). Output: `backend/api/documents/` package; `backend/api/documents.py` monolith deleted. @$HOME/.claude/get-shit-done/workflows/execute-plan.md @$HOME/.claude/get-shit-done/templates/summary.md @.planning/PROJECT.md @.planning/ROADMAP.md @.planning/STATE.md @.planning/phases/08-stack-upgrade-backend-decomposition/08-CONTEXT.md @.planning/phases/08-stack-upgrade-backend-decomposition/08-RESEARCH.md @.planning/phases/08-stack-upgrade-backend-decomposition/08-PATTERNS.md @backend/api/documents.py @backend/api/folders.py @backend/main.py @CLAUDE.md Endpoint to sub-module map (locked per RESEARCH.md §"api/documents.py → api/documents/ package"): upload.py: request_upload_url (POST /upload-url, line 93-135), upload_document (POST /upload, line 137-296), confirm_upload (POST /{id}/confirm, line 298-404) crud.py: list_documents (GET "", line 406-529), get_document (GET /{id}, line 531-576), patch_document (PATCH /{id}, line 578-630), delete_document (DELETE /{id}, line 632-705), classify_document (POST /{id}/classify, line 707-742) — placed here per D-08 content.py: stream_document_content (GET /{id}/content, line 765 onwards) + _parse_range helper (line 744-760) Pydantic model + constant to file map: shared.py: _CLOUD_PROVIDERS frozenset (line 59), UploadUrlRequest (line 66-69), DocumentPatch (line 71-88) Package aggregator pattern for backend/api/documents/__init__.py: from fastapi import APIRouter from api.documents.upload import router as upload_router from api.documents.crud import router as crud_router from api.documents.content import router as content_router router = APIRouter(prefix="/api/documents", tags=["documents"]) router.include_router(upload_router) router.include_router(crud_router) router.include_router(content_router) Task 1: Create backend/api/documents/ package — shared.py, upload.py, crud.py, content.py, __init__.py backend/api/documents/__init__.py, backend/api/documents/shared.py, backend/api/documents/upload.py, backend/api/documents/crud.py, backend/api/documents/content.py - backend/api/documents.py lines 1-852 (full source — every endpoint, every Pydantic model, every helper, every import) - backend/api/folders.py (ownership assertion pattern: `if doc is None or doc.user_id != current_user.id: raise HTTPException(status_code=404, ...)`) - .planning/phases/08-stack-upgrade-backend-decomposition/08-PATTERNS.md sections for `backend/api/documents/*` (exact import lists, sub-router declaration, ownership assertion pattern) - CLAUDE.md §"Key Architectural Rules" (atomic quota UPDATE pattern, ownership checks; preserved verbatim) This task creates the package and ALL five files. Execute in this order so the package is in a valid state on disk after each sub-step: (A) Create `backend/api/documents/shared.py`. Add `from __future__ import annotations`. Import `from typing import Optional` and `from pydantic import BaseModel, field_validator`. Move the constant `_CLOUD_PROVIDERS = frozenset({"google_drive", "onedrive", "nextcloud", "webdav"})` from `documents.py` line 59. Move `UploadUrlRequest` (line 66-69) and `DocumentPatch` (line 71-88) verbatim, including the `filename_no_path_separators` validator which is a security validator at the API boundary (RESEARCH.md confirms it stays in the Pydantic model — D-11 analysis). (B) Create `backend/api/documents/upload.py`. Header: `from __future__ import annotations`. Imports per PATTERNS.md §"backend/api/documents/upload.py" plus `from api.documents.shared import UploadUrlRequest, _CLOUD_PROVIDERS`. `router = APIRouter()` — NO prefix (D-04). Move handler functions verbatim including decorators: `request_upload_url` (lines 93-135), `upload_document` (lines 137-296), `confirm_upload` (lines 298-404). Every handler injects `current_user: User = Depends(get_current_user)` and asserts ownership on any pre-existing document lookup using the pattern `if doc is None or doc.user_id != current_user.id: raise HTTPException(status_code=404, ...)`. (C) Create `backend/api/documents/crud.py`. Header + imports per PATTERNS.md §"backend/api/documents/crud.py" plus `from api.documents.shared import DocumentPatch, _CLOUD_PROVIDERS`. `router = APIRouter()` — NO prefix. Move handlers verbatim: `list_documents` (406-529), `get_document` (531-576), `patch_document` (578-630), `delete_document` (632-705), `classify_document` (707-742) — placed here per D-08. Each handler injects `current_user: User = Depends(get_current_user)` and includes the ownership assertion. (D) Create `backend/api/documents/content.py`. Header: `from __future__ import annotations`. Imports per PATTERNS.md §"backend/api/documents/content.py" (FastAPI APIRouter, Depends, HTTPException, Request; FastAPI responses StreamingResponse; SQLAlchemy AsyncSession; deps.auth.get_current_user; deps.db.get_db). `router = APIRouter()` — NO prefix. Move `_parse_range(range_header: str, file_size: int)` helper verbatim from line 744-760 as a private module-level function. Move `stream_document_content` handler verbatim from line 765 onwards. The handler injects `current_user: User = Depends(get_current_user)` and asserts ownership before serving bytes (DOC-04 / SEC-04). (E) Create `backend/api/documents/__init__.py`. Contents per PATTERNS.md §"backend/api/documents/__init__.py": import APIRouter from fastapi, import `router as upload_router`, `router as crud_router`, `router as content_router` from the three sub-modules, declare `router = APIRouter(prefix="/api/documents", tags=["documents"])`, call `router.include_router(upload_router)`, `router.include_router(crud_router)`, `router.include_router(content_router)`. ONLY aggregation — no helpers, no models, no logic. To avoid Python's package-vs-module name collision, immediately after creating the package directory, rename `backend/api/documents.py` to `backend/api/documents_OLD_REMOVE_IN_TASK_2.py`. Task 2 verifies tests pass and then deletes the renamed file. The route registration order matters: `crud.py` registers `GET ""` (matches `/api/documents`), and `crud.py` registers `GET /{doc_id}` which could shadow `upload.py`'s `POST /upload-url` if route precedence is wrong. FastAPI matches by exact path + method, so HTTP-method differentiation prevents collision; verify by listing routes after include. cd backend && python -c "from api.documents import router; paths = sorted({(r.path, list(r.methods)[0] if r.methods else '') for r in router.routes}); print('\n'.join(f'{m} {p}' for p, m in paths))" - Files exist: `backend/api/documents/__init__.py`, `backend/api/documents/shared.py`, `backend/api/documents/upload.py`, `backend/api/documents/crud.py`, `backend/api/documents/content.py` - `grep -v '^#' backend/api/documents/__init__.py | grep -c 'router = APIRouter(prefix="/api/documents"'` returns 1 - `grep -v '^#' backend/api/documents/upload.py | grep -c 'router = APIRouter()'` returns 1 (NO prefix) - `grep -v '^#' backend/api/documents/crud.py | grep -c 'router = APIRouter()'` returns 1 (NO prefix) - `grep -v '^#' backend/api/documents/content.py | grep -c 'router = APIRouter()'` returns 1 (NO prefix) - `grep -c "_CLOUD_PROVIDERS = frozenset" backend/api/documents/shared.py` returns 1 - `grep -rn "_CLOUD_PROVIDERS = frozenset" backend/api/documents/` returns exactly one match (in shared.py) - `grep -c "Depends(get_current_user)" backend/api/documents/upload.py` ≥ 3 - `grep -c "Depends(get_current_user)" backend/api/documents/crud.py` ≥ 5 - `grep -c "Depends(get_current_user)" backend/api/documents/content.py` ≥ 1 - `grep -c "doc.user_id != current_user.id\\|user_id != current_user.id" backend/api/documents/crud.py` ≥ 3 (ownership checks on get/patch/delete and classify) - `cd backend && python -c "from api.documents import router; assert len(router.routes) >= 9"` exits 0 (3 upload + 5 crud + 1 content = 9) Package exists with all 5 files; sub-router count totals 9; no sub-router carries a prefix; ownership checks preserved; monolith renamed (not yet deleted). Task 2: Run URL regression suite, then delete the old monolith backend/api/documents.py - backend/tests/test_documents.py (full file — confirms which endpoints are exercised) - backend/api/documents_OLD_REMOVE_IN_TASK_2.py (renamed monolith from task 1) Run `cd backend && pytest tests/test_documents.py -x -v` to confirm every document URL still responds correctly (URLs unchanged, response shapes unchanged, ownership 404s preserved, quota behavior preserved). If ANY test fails, do NOT delete the renamed monolith — diagnose, fix, and re-run. Only after `tests/test_documents.py` and `tests/test_shares.py` (because shares.py uses Document lookups) both pass, delete `backend/api/documents_OLD_REMOVE_IN_TASK_2.py`. Re-run the full backend suite `cd backend && pytest -v` to confirm no other tests regressed. cd backend && pytest tests/test_documents.py tests/test_shares.py -x -v 2>&1 | tail -15 && test ! -f backend/api/documents.py && test ! -f backend/api/documents_OLD_REMOVE_IN_TASK_2.py && echo "old monolith deleted" - `test ! -f backend/api/documents.py` exits 0 - `test ! -f backend/api/documents_OLD_REMOVE_IN_TASK_2.py` exits 0 - `test -d backend/api/documents` exits 0 - `cd backend && pytest tests/test_documents.py -x` exits 0 - `cd backend && pytest tests/test_shares.py -x` exits 0 - `cd backend && pytest -v` exits 0 with no new failures vs. baseline - `grep -rn "^class UploadUrlRequest" backend/api/documents/` returns exactly one match (in shared.py) - `grep -rn "^class DocumentPatch" backend/api/documents/` returns exactly one match (in shared.py) Old documents.py file deleted; full backend suite green; Pydantic models defined exactly once. ## Trust Boundaries | Boundary | Description | |----------|-------------| | Regular user → document endpoints | Every handler enforces `get_current_user` AND `resource.user_id == current_user.id` to prevent IDOR (DOC-04 / SEC-04) | | Browser → MinIO presigned PUT URL | Generated server-side scoped to `{user_id}/{document_id}/{uuid4}`; never includes admin or other user paths | | Range-header → byte offset arithmetic | `_parse_range` must reject malformed input; copied verbatim from monolith | ## STRIDE Threat Register | Threat ID | Category | Component | Disposition | Mitigation Plan | |-----------|----------|-----------|-------------|-----------------| | T-08-05-01 | Spoofing/Elevation | IDOR on document endpoints | mitigate | All handlers copied verbatim including `current_user.id` ownership assertion. Acceptance criterion grep counts ≥ 3 ownership checks in crud.py. | | T-08-05-02 | Tampering | Sub-router prefix doubling | mitigate | All three sub-routers declare `router = APIRouter()` with NO prefix; only `__init__.py` carries `prefix="/api/documents"`. | | T-08-05-03 | Information Disclosure | DocumentPatch.filename validator | mitigate | `filename_no_path_separators` validator preserved verbatim in shared.py (path traversal defense). | | T-08-05-04 | Tampering | Atomic quota UPDATE invariant | mitigate | `confirm_upload` and `delete_document` copied verbatim — atomic `UPDATE quotas SET used_bytes = used_bytes + $delta WHERE …` pattern preserved (CLAUDE.md non-negotiable). | | T-08-05-05 | Denial of Service | Circular import via `__init__.py` | mitigate | `__init__.py` does ONLY aggregation; shared models in `shared.py`. | | T-08-05-SC | Supply Chain | No new packages | accept | This plan installs zero new packages. | - `cd backend && pytest tests/test_documents.py tests/test_shares.py -x -v` — zero failures - `cd backend && pytest -v` — full suite zero failures - `cd backend && python -c "from main import app; doc_paths = sorted({r.path for r in app.routes if r.path.startswith('/api/documents')}); print('\\n'.join(doc_paths))"` lists at minimum: `/api/documents`, `/api/documents/upload-url`, `/api/documents/upload`, `/api/documents/{doc_id}`, `/api/documents/{doc_id}/confirm`, `/api/documents/{doc_id}/classify`, `/api/documents/{doc_id}/content` - `backend/api/documents/` package with 5 files - `backend/api/documents.py` deleted - All document URL paths unchanged, response shapes unchanged - Every handler preserves ownership assertion - All tests pass Create `.planning/phases/08-stack-upgrade-backend-decomposition/08-05-SUMMARY.md` when done. Include: (a) full list of document paths emitted by `app.routes` before vs. after (must be identical), (b) `tests/test_documents.py` test count, (c) confirmation that the `_CLOUD_PROVIDERS` constant exists exactly once in the package.