06-PATTERNS.md: maps 12 new/modified files to closest codebase analogs for Phase 6 (Performance & Production Hardening). 06-REVIEW-FIX.md: records all 16 review findings (7 critical + 9 warning) fixed in iteration 1 on 2026-06-04. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
7.8 KiB
phase, fixed_at, review_path, iteration, findings_in_scope, fixed, skipped, status
| phase | fixed_at | review_path | iteration | findings_in_scope | fixed | skipped | status |
|---|---|---|---|---|---|---|---|
| 06-performance-production-hardening | 2026-06-04T00:00:00Z | .planning/phases/06-performance-production-hardening/06-REVIEW.md | 1 | 16 | 16 | 0 | 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 <p v-else-if="fetchError" class="text-xs text-red-600 mt-1">{{ fetchError }}</p> 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_typefailure 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