--- phase: 06-performance-production-hardening fixed_at: 2026-06-04T00:00:00Z review_path: .planning/phases/06-performance-production-hardening/06-REVIEW.md iteration: 1 findings_in_scope: 16 fixed: 16 skipped: 0 status: all_fixed --- # Phase 6: Code Review Fix Report **Fixed at:** 2026-06-04 **Source review:** `.planning/phases/06-performance-production-hardening/06-REVIEW.md` **Iteration:** 1 **Summary:** - Findings in scope: 16 (7 Critical + 9 Warning) - Fixed: 16 - Skipped: 0 ## Fixed Issues ### CR-01: CLOUD_CREDS_KEY never passed to backend service **Files modified:** `docker-compose.yml` **Commit:** a8dbb02 **Applied fix:** Added `- CLOUD_CREDS_KEY=${CLOUD_CREDS_KEY}` to the backend service environment block, alongside WR-02, WR-05, WR-06, WR-08. --- ### CR-02: Grafana exposed with unauthenticated Admin-role access **Files modified:** `docker-compose.yml` **Commit:** a8dbb02 **Applied fix:** Changed Grafana env to disable anonymous access and use `${GRAFANA_ADMIN_USER:-admin}` / `${GRAFANA_ADMIN_PASSWORD:-changeme}` credentials. Changed port bindings for both Grafana (`3000:3000`) and Loki (`3100:3100`) to loopback-only (`127.0.0.1:3000:3000` and `127.0.0.1:3100:3100`). --- ### CR-03: `get_client_ip()` bypassed — raw `X-Forwarded-For` reads **Files modified:** `backend/api/cloud.py`, `backend/api/documents.py` **Commit:** 23c27ef **Applied fix:** Added `from deps.utils import get_client_ip` import to both files. Replaced all five raw `request.headers.get("X-Forwarded-For")` reads (cloud.py lines 629, 766; documents.py lines 276, 384, 670) with `get_client_ip(request)`. Also removed the "TRUST BOUNDARY" comments that noted the problem without fixing it. --- ### CR-04: `default_storage_backend` written to DB without allowlist validation **Files modified:** `backend/api/cloud.py` **Commit:** 3a6251c **Applied fix:** Added `_VALID_BACKENDS = frozenset({"minio", "google_drive", "onedrive", "nextcloud", "webdav"})` module constant and validation block in `update_default_storage()` that raises HTTP 422 for any value not in the allowlist. --- ### CR-05: Audit log leaks attempted email (PII) in `metadata_` **Files modified:** `backend/api/auth.py`, `frontend/src/components/admin/AuditLogTab.vue` **Commit:** aad7635 **Applied fix:** Added `import hashlib` to auth.py and replaced `{"attempted_email": str(body.email)}` with `{"attempted_email_hash": hashlib.sha256(str(body.email).encode()).hexdigest()[:16]}` in the login failure audit log call. Updated AuditLogTab.vue to display `entry.metadata_.attempted_email_hash` (with `hash:` prefix and monospace styling) instead of `entry.metadata_.attempted_email`. --- ### CR-06: `CorrelationIDMiddleware` binds `duration_ms` after response is delivered — value never logged **Files modified:** `backend/main.py` **Commit:** a37a910 **Applied fix:** Added `_response_status: int = 0` variable and `nonlocal _response_status` capture in the `send_with_header` closure to record the HTTP status code. After `await self.app(...)` computes `duration_ms`, now emits a structured log line via `structlog.get_logger("docuvault.access").info("request_complete", status_code=_response_status)` so `duration_ms` is written to the log before context is cleared. --- ### CR-07: `event_type` LIKE filter allows unvalidated user input with SQL wildcards **Files modified:** `backend/api/audit.py`, `backend/tests/test_audit.py` **Commit:** 10970d9 (fix), fb4ce29 (test update) **Applied fix:** Added `_VALID_EVENT_PREFIXES = frozenset({"auth", "document", "folder", "share", "admin", "cloud"})` module constant. Added validation before each of the three `.like()` call sites (`_build_filtered_query`, `_build_filtered_query_with_handles`, and the inline count query in `list_audit_log`). Changed LIKE pattern from `f"{event_type}%"` to `f"{event_type}.%"` to enforce true prefix semantics. Updated `test_audit_log_filter_by_event_type` to pass `"document"` prefix instead of the full `"document.uploaded"` event type string. --- ### WR-01: `auth_limiter` not reset between tests **Files modified:** `backend/tests/conftest.py` **Commit:** 4a57193 **Applied fix:** Added `from api.auth import limiter as auth_limiter` import and added `auth_limiter._storage.reset()` calls both before and after `yield` in the `reset_rate_limiter` autouse fixture. --- ### WR-02: `uvicorn --reload` in docker-compose production backend command **Files modified:** `docker-compose.yml` **Commit:** a8dbb02 **Applied fix:** Changed `command: uvicorn main:app --host 0.0.0.0 --port 8000 --reload` to `command: uvicorn main:app --host 0.0.0.0 --port 8000 --workers 2`. --- ### WR-03: Locust load test accesses document list as a bare list — shape mismatch **Files modified:** `backend/load_tests/locustfile.py` **Commit:** 013802a **Applied fix:** Changed `docs = resp.json()` to `docs = resp.json().get("items", [])` in the `get_document` task so it correctly handles the `{"items": [...], "total": N, ...}` response envelope. --- ### WR-04: `trusted_proxy` list missing `10.0.0.0/8` **Files modified:** `backend/deps/utils.py` **Commit:** b0d2406 **Applied fix:** Added `ipaddress.ip_network("10.0.0.0/8")` as the first entry in `_TRUSTED_PROXY_NETS`, covering cloud VPC, Kubernetes pod CIDRs, and custom Docker network configurations that use the 10.x.x.x range. --- ### WR-05: `celery-beat` service lacks container hardening **Files modified:** `docker-compose.yml` **Commit:** a8dbb02 **Applied fix:** Added `read_only: true`, `tmpfs: ["/tmp:mode=1777"]`, `cap_drop: [ALL]`, and `security_opt: ["no-new-privileges:true"]` to the celery-beat service. Changed the `command` to pass `--schedule /tmp/celerybeat-schedule` so the schedule file goes to the tmpfs mount instead of the read-only root filesystem. Removed the "NOT hardened" comment. --- ### WR-06: `LOG_JSON` hardcoded to `true` in docker-compose **Files modified:** `docker-compose.yml` **Commit:** a8dbb02 **Applied fix:** Changed `- LOG_JSON=${LOG_JSON:-false}` (was `- LOG_JSON=true #${LOG_JSON:-false}` in working tree) to `- LOG_JSON=${LOG_JSON:-true}` — defaults to `true` in production but allows developer override via `.env`. The default changed from `false` to `true` to match production logging intent. --- ### WR-07: `print()` used for cloud delete errors instead of structlog **Files modified:** `backend/api/documents.py` **Commit:** 7cd29e9 **Applied fix:** Added `import structlog as _structlog` and `_log = _structlog.get_logger(__name__)` at module level. Replaced `import sys; print(f"[cloud-delete] provider error: {exc}", file=sys.stderr)` with `_log.warning("cloud_delete_failed", provider=doc.storage_backend, error=str(exc))`. --- ### WR-08: `celery-worker` missing `SECRET_KEY` **Files modified:** `docker-compose.yml` **Commit:** a8dbb02 **Applied fix:** Added `- SECRET_KEY=${SECRET_KEY}` to the celery-worker service environment block. --- ### WR-09: `AuditLogTab.vue` silently swallows fetch errors — no user feedback **Files modified:** `frontend/src/components/admin/AuditLogTab.vue` **Commit:** 21366bd **Applied fix:** Added `const fetchError = ref(null)` reactive ref. Set `fetchError.value = null` at the start of `fetchLog()` and `fetchError.value = 'Failed to load audit log. Please try again.'` in the catch block. Added `

{{ fetchError }}

` between the loading state and empty state elements in the template. --- ## Test Results Backend test suite run after all fixes: - **366 passed**, 1 failed (pre-existing `test_extract_docx` — `ModuleNotFoundError: No module named 'docx'` in local dev environment, unrelated to these fixes), 6 skipped, 12 xfailed. - The `test_audit_log_filter_by_event_type` failure from the CR-07 fix was resolved by updating the test to use the prefix-based filter API. --- _Fixed: 2026-06-04_ _Fixer: Claude (gsd-code-fixer)_ _Iteration: 1_