Files
curo1305 708fd7fad0 docs(phase-6.2): record planning complete — 4 plans verified, state updated
- ROADMAP.md: progress table → Planned; wave annotations already added by planner
- STATE.md: phase 6.2 row → Planned (4 plans, 3 waves); session note added
- 06.2-03-PLAN.md: remove incorrect SHARE-03/SHARE-05 from requirements field
- 06.2-RESEARCH.md: mark Open Questions section as RESOLVED
- 06.2-UI-SPEC.md: add to version control (was untracked)

Verification: 0 blockers, 2 cosmetic warnings fixed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-31 11:41:32 +02:00

289 lines
20 KiB
Markdown

---
phase: "06.2"
plan: "03"
type: execute
wave: 1
depends_on:
- "06.2-01"
files_modified:
- backend/api/documents.py
- backend/services/storage.py
- frontend/src/views/DocumentView.vue
- frontend/src/api/client.js
- backend/tests/test_documents.py
autonomous: true
requirements: []
# cloud-delete (D-01..D-04) is covered via phase success criteria; no named REQUIREMENTS.md ID
must_haves:
truths:
- "Deleting a cloud document calls the cloud provider's delete_object, not MinIO's"
- "Cloud delete failure returns HTTP 200 with cloud_delete_failed: true in the response body (not a hard 4xx/5xx)"
- "remove_only=true deletes only the DB row, leaving the cloud file intact, and skips quota decrement"
- "Cloud document deletes do NOT decrement the user's quota (cloud docs never charged quota at upload)"
- "Frontend shows CloudDeleteWarningModal when cloud_delete_failed response is received"
- "User can confirm 'Remove from app' which calls DELETE ?remove_only=true and navigates away"
artifacts:
- path: "backend/api/documents.py"
provides: "cloud-aware delete_document endpoint with remove_only query param"
contains: "remove_only"
- path: "backend/services/storage.py"
provides: "skip_quota guard in delete_document service function"
contains: "skip_quota"
- path: "frontend/src/views/DocumentView.vue"
provides: "CloudDeleteWarningModal inline block; remove_only confirm path"
contains: "showCloudDeleteWarning"
key_links:
- from: "backend/api/documents.py"
to: "storage.get_storage_backend_for_document()"
via: "cloud routing before MinIO path"
pattern: "get_storage_backend_for_document"
- from: "backend/api/documents.py"
to: "services/storage.delete_document(skip_quota=True)"
via: "skip_quota parameter for cloud docs"
pattern: "skip_quota"
- from: "frontend/src/views/DocumentView.vue"
to: "DELETE /api/documents/{id}?remove_only=true"
via: "confirmRemoveOnly() handler called from modal CTA"
pattern: "remove_only=true"
---
<objective>
Close the cloud-delete propagation gap in a single vertical slice. The default delete button now propagates to the cloud provider. A structured error response (cloud_delete_failed: true) triggers a warning modal in the frontend. The "Remove from app" path uses ?remove_only=true to delete only the DB record. Cloud docs skip quota decrement.
Purpose: Users who delete cloud-stored documents no longer create orphaned files on the provider. This is a correctness fix: the app claimed to delete documents but only removed the DB row.
Output: Modified api/documents.py (cloud routing + remove_only param), modified services/storage.py (skip_quota guard), modified DocumentView.vue (warning modal + remove_only path), new client.js deleteDocument function variant, three promoted test stubs.
</objective>
<execution_context>
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-RESEARCH.md
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md
</execution_context>
<context>
@/Users/nik/Documents/Progamming/document_scanner/.planning/ROADMAP.md
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-CONTEXT.md
<interfaces>
<!-- Key types and contracts the executor needs. Extracted from codebase. -->
From backend/services/storage.py:delete_document (current signature):
async def delete_document(session: AsyncSession, doc_id: str) -> bool:
# Always calls _backend().delete_object() (MinIO singleton)
# Always runs quota decrement UPDATE
# Returns False if doc not found, True on success
From backend/storage/__init__.py:
async def get_storage_backend_for_document(
document: Document,
user: User,
session: AsyncSession,
) -> StorageBackend:
# Returns MinIOBackend for storage_backend == "minio"
# For cloud docs: loads CloudConnection, decrypts HKDF creds, returns cloud backend
# Raises HTTPException(503) if connection not found/inactive
From backend/api/admin.py lines 527-539 (canonical cloud delete pattern):
for doc in cloud_docs:
try:
backend = await get_storage_backend_for_document(doc, user, session)
await backend.delete_object(doc.object_key)
except Exception:
pass # best-effort
From backend/api/documents.py (existing delete endpoint stub — find the @router.delete("/{doc_id}") handler):
# Existing handler: calls await storage.delete_document(session, doc_id)
# Must be extended with remove_only query param and cloud routing
From frontend/src/api/client.js (existing deleteDocument function — search for "deleteDocument"):
# Current implementation calls DELETE /api/documents/{id} via request() wrapper
# request() calls res.json() — this is correct for 200 responses
# New behavior: parse response body for cloud_delete_failed flag
From frontend/src/views/DocumentView.vue (existing confirmDelete pattern — search for "confirmDelete" or "handleDelete"):
# Existing: window.confirm() then deleteDocument() then router.push('/')
# New: after API call, check response.cloud_delete_failed — if true, show modal
From .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md C-3:
Cloud Delete Warning Modal — inline in DocumentView.vue:
Fixed overlay: bg-black/40 flex items-center justify-center z-50
Panel: bg-white rounded-2xl shadow-xl p-6 max-w-sm w-full mx-4
Heading: "Cloud delete failed" (text-lg font-semibold text-gray-900 mb-2)
Body: "The file could not be deleted from {provider}. Remove it from DocuVault anyway? The file will remain on {provider}."
Warning icon: Heroicons ExclamationTriangleIcon inline SVG, w-5 h-5 text-amber-500
Primary CTA: "Remove from app" — bg-red-600 hover:bg-red-700 text-white text-sm px-4 py-2 rounded-lg
Secondary: "Cancel" — border border-gray-300 text-gray-700 text-sm px-4 py-2 rounded-lg hover:bg-gray-50
role="dialog" aria-modal="true" aria-labelledby="cloud-delete-modal-title"
@click.self closes modal; Cancel abandons delete; "Remove from app" calls DELETE ?remove_only=true
</interfaces>
</context>
<tasks>
<task type="auto" tdd="true">
<name>Task 1: Backend — cloud-aware delete routing + skip_quota + remove_only param</name>
<files>backend/api/documents.py, backend/services/storage.py, backend/tests/test_documents.py</files>
<read_first>
- backend/services/storage.py — read lines 143-179 (full delete_document function) to understand current MinIO path and quota decrement logic that must be preserved for MinIO docs
- backend/api/documents.py — find and read the existing @router.delete("/{doc_id}") handler (search for "router.delete" or "delete_document") to see its current signature and body
- backend/storage/__init__.py — read get_storage_backend_for_document signature (lines 53-132) to confirm it takes (document: Document, user: User, session: AsyncSession)
- backend/api/admin.py — read lines 520-545 to see the canonical cloud delete pattern (get_storage_backend_for_document + backend.delete_object in try/except)
- backend/tests/test_documents.py — read the full file to understand conftest fixtures (async_client, auth_user, db_session) and _make_doc or equivalent helper patterns
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-RESEARCH.md — Pattern 1 (cloud routing preferred in API layer), Pitfall 1 (skip_quota), Pitfall 2 (MinIO no-op on missing keys)
</read_first>
<behavior>
- test_delete_cloud_document_propagates: Create a Document with storage_backend="google_drive". Mock get_storage_backend_for_document to return a mock backend. DELETE /api/documents/{id}. Assert the mock backend's delete_object was called once with the document's object_key. Assert quota UPDATE was NOT executed (cloud docs never charged quota).
- test_delete_cloud_document_failure: Create a Document with storage_backend="google_drive". Mock get_storage_backend_for_document to return a mock backend whose delete_object raises Exception("provider error"). DELETE /api/documents/{id}. Assert HTTP 200. Assert response body has cloud_delete_failed=True and success=False. Assert the DB row is NOT deleted (doc still exists after the call).
- test_delete_cloud_remove_only: Create a Document with storage_backend="google_drive". DELETE /api/documents/{id}?remove_only=true WITHOUT mocking the cloud backend. Assert HTTP 200. Assert DB row is deleted. Assert quota UPDATE was NOT executed.
</behavior>
<action>
Make two file changes:
CHANGE 1 — backend/services/storage.py: add skip_quota parameter to delete_document():
Change the function signature to `async def delete_document(session: AsyncSession, doc_id: str, skip_quota: bool = False) -> bool:`.
Wrap the existing quota decrement block in `if not skip_quota:` so it only runs when skip_quota is False (i.e., for MinIO documents). The MinIO `_backend().delete_object(doc.object_key)` call stays where it is — it is only reached for MinIO docs once the API layer routing is correct (the API layer will handle cloud routing before calling this function).
CHANGE 2 — backend/api/documents.py: add remove_only param + cloud routing to the delete endpoint:
Add `remove_only: bool = Query(default=False)` to the existing delete_document endpoint handler signature.
In the handler body, BEFORE the call to `storage.delete_document()`, add cloud routing logic:
If `doc.storage_backend != "minio"` and `not remove_only`:
- Try: call `cloud_backend = await get_storage_backend_for_document(doc, current_user, session)`; then `await cloud_backend.delete_object(doc.object_key)`
- Except Exception: return JSONResponse(status_code=200, content={"success": False, "cloud_delete_failed": True, "detail": "Cloud provider delete failed. You can remove from app only."})
- (If cloud delete succeeds, fall through to DB delete with skip_quota=True)
If `doc.storage_backend != "minio"` (regardless of remove_only): call `storage.delete_document(session, str(doc.id), skip_quota=True)`
If `doc.storage_backend == "minio"`: call `storage.delete_document(session, str(doc.id), skip_quota=False)` (existing behavior)
The import for `get_storage_backend_for_document` must be added at the top of documents.py (or lazily inside the handler body following the lazy-import pattern already in the file). Also import `JSONResponse` from `fastapi.responses` if not already imported. Add `from fastapi import Query` if not already imported.
CRITICAL: The cloud delete exception handler must NOT include the exception message `str(exc)` in the response body. The generic detail string is sufficient. Log the exception to stderr internally if desired: `print(f"[cloud-delete] provider error: {exc}", file=sys.stderr)`.
Then in backend/tests/test_documents.py: promote the three xfail stubs to real tests using unittest.mock.patch and the async_client fixture pattern established in the existing file. Mock `api.documents.get_storage_backend_for_document` (the import path used in documents.py) or `storage.get_storage_backend_for_document` depending on how the import appears in the delete handler. Use `AsyncMock` for the mock backend's delete_object method.
</action>
<verify>
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_documents.py::test_delete_cloud_document_propagates tests/test_documents.py::test_delete_cloud_document_failure tests/test_documents.py::test_delete_cloud_remove_only -x -v 2>&1 | tail -20</automated>
</verify>
<done>
- All three promoted tests pass
- `grep "skip_quota" backend/services/storage.py` returns a match
- `grep "remove_only" backend/api/documents.py` returns a match
- `grep "cloud_delete_failed" backend/api/documents.py` returns a match
- `grep "get_storage_backend_for_document" backend/api/documents.py` returns a match
- `pytest tests/test_documents.py -x -q` exits 0
</done>
</task>
<task type="auto">
<name>Task 2: Frontend — CloudDeleteWarningModal + remove_only path in DocumentView</name>
<files>frontend/src/views/DocumentView.vue, frontend/src/api/client.js</files>
<read_first>
- frontend/src/views/DocumentView.vue — read the full file to find: existing confirmDelete (or equivalent delete handler) function, existing router.push('/') navigation, how the document object is loaded, and the template structure where the modal should be inserted
- frontend/src/api/client.js — search for "deleteDocument" or the function that calls DELETE /api/documents/{id} to understand the current implementation; also read lines 399-428 (fetchDocumentContent) to understand the raw fetch pattern used for authenticated non-JSON responses
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md — C-3 component contract for the CloudDeleteWarningModal, Copywriting Contract for exact copy, State Inventory for states required
</read_first>
<action>
Make two file changes:
CHANGE 1 — frontend/src/api/client.js:
Find the existing deleteDocument function. Modify it to accept an optional `removeOnly = false` parameter. The delete endpoint call should append `?remove_only=true` to the URL when removeOnly is true: `/api/documents/${docId}?remove_only=true`. The response is always JSON (HTTP 200 for both success and cloud_delete_failed), so keep `res.json()` via the existing `request()` wrapper. The function should return the parsed JSON body (not throw on success) so callers can inspect `cloud_delete_failed`.
If the existing deleteDocument function uses `request()` and it throws on non-2xx, wrap accordingly: HTTP 200 with cloud_delete_failed body is a valid 2xx response so `request()` will return it normally.
Add a second function `deleteDocumentRemoveOnly(docId)` that calls `deleteDocument(docId, true)` — a convenience wrapper for the remove_only path called from the modal CTA.
CHANGE 2 — frontend/src/views/DocumentView.vue:
Add two new reactive refs to the script section:
- `showCloudDeleteWarning` (boolean, default false)
- `cloudProviderName` (string, default 'your cloud storage')
Modify the existing delete handler (confirmDelete or equivalent). After the delete API call, instead of always navigating to '/', check the response:
- If `response.cloud_delete_failed === true`: set `cloudProviderName.value` from document's storage_backend (map "google_drive" → "Google Drive", "onedrive" → "OneDrive", "nextcloud" → "Nextcloud", "webdav" → "WebDAV", fallback to "your cloud storage"); set `showCloudDeleteWarning.value = true`; do NOT navigate
- Otherwise (success): navigate to `/` as before
Add a `confirmRemoveOnly()` async function:
- Call `await api.deleteDocumentRemoveOnly(props.docId)` (or however the document ID is referenced in DocumentView)
- On success: `showCloudDeleteWarning.value = false`; navigate to `/`
- On error: show an inline error message within the modal (reuse existing error display pattern)
Add `cancelCloudDeleteWarning()` function: set `showCloudDeleteWarning.value = false`; abort — document is NOT deleted.
In the template, add the cloud delete warning modal as an inline conditional block (`v-if="showCloudDeleteWarning"`) following the C-3 contract from UI-SPEC:
- Fixed overlay with `@click.self="cancelCloudDeleteWarning"`
- Panel with `role="dialog"` `aria-modal="true"` `aria-labelledby="cloud-delete-modal-title"`
- Heading id="cloud-delete-modal-title": "Cloud delete failed"
- ExclamationTriangleIcon inline SVG (w-5 h-5 text-amber-500): use the Heroicons stroke SVG path for the triangle: `<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z" />`
- Body text using cloudProviderName: `The file could not be deleted from {{ cloudProviderName }}. Remove it from DocuVault anyway? The file will remain on {{ cloudProviderName }}.`
- Buttons: "Remove from app" (@click="confirmRemoveOnly") and "Cancel" (@click="cancelCloudDeleteWarning")
- Exact Tailwind classes from UI-SPEC C-3 (bg-red-600, bg-white rounded-2xl, etc.)
</action>
<verify>
<automated>cd /Users/nik/Documents/Progamming/document_scanner/frontend && grep -n "showCloudDeleteWarning\|cloud_delete_failed\|removeOnly\|remove_only" src/views/DocumentView.vue src/api/client.js | head -20</automated>
</verify>
<done>
- `grep "showCloudDeleteWarning" frontend/src/views/DocumentView.vue` returns at least 2 matches (ref + template v-if)
- `grep "cloud-delete-modal-title" frontend/src/views/DocumentView.vue` returns a match
- `grep "remove_only" frontend/src/api/client.js` returns a match
- `grep "Remove from app" frontend/src/views/DocumentView.vue` returns a match
- No console errors when building: `cd frontend && npm run build 2>&1 | grep -i error | head -10` returns empty (or pre-existing errors only)
</done>
</task>
</tasks>
<threat_model>
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| browser → DELETE /api/documents/{id} | remove_only query param is user-supplied; must not bypass ownership checks |
| api/documents.py → cloud backend | cloud credentials must not appear in the error response returned to the browser |
| api/documents.py → services/storage.py | skip_quota flag must be set correctly to prevent quota underflow |
## STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-06.2-03-01 | Tampering | Quota underflow on cloud delete | mitigate | `skip_quota=True` passed to `delete_document()` for all non-minio documents; cloud docs never had quota charged at upload |
| T-06.2-03-02 | Information Disclosure | Cloud credential exposure in error response | mitigate | Exception caught as generic `except Exception`; only a fixed string "Cloud provider delete failed." returned to client — `str(exc)` is logged to stderr only, never serialized to JSON response |
| T-06.2-03-03 | Elevation of Privilege | remove_only param bypasses ownership | accept | Ownership assertion (`doc.user_id != current_user.id → 404`) occurs BEFORE the remove_only branch — authenticated user must own the document regardless of query param value |
| T-06.2-03-04 | Spoofing | Silent MinIO no-op for cloud docs | mitigate | Cloud routing happens before any MinIO call for non-minio documents — `get_storage_backend_for_document()` returns the cloud backend, not the MinIO singleton (Pitfall 2 from RESEARCH.md) |
| T-06.2-SC | Tampering | npm/pip/cargo installs | accept | No new packages installed in this plan |
</threat_model>
<verification>
After both tasks complete:
```
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_documents.py -x -q
```
Expected: exits 0, 3 promoted cloud-delete tests pass, all pre-existing tests still pass.
```
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest -v 2>&1 | tail -20
```
Expected: zero failures.
Frontend build check:
```
cd /Users/nik/Documents/Progamming/document_scanner/frontend && npm run build 2>&1 | grep -c "error" || echo "0 errors"
```
</verification>
<success_criteria>
- DELETE /api/documents/{id} for a cloud doc calls cloud backend delete_object — confirmed by test_delete_cloud_document_propagates
- Cloud delete failure returns HTTP 200 with cloud_delete_failed=True — confirmed by test_delete_cloud_document_failure
- remove_only=true skips cloud, removes DB row, skips quota decrement — confirmed by test_delete_cloud_remove_only
- Cloud doc deletes never decrement quota (skip_quota=True path)
- DocumentView.vue shows CloudDeleteWarningModal when cloud_delete_failed is received
- "Remove from app" calls DELETE ?remove_only=true and navigates to /
- All test_documents.py tests pass
</success_criteria>
<output>
Create `.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-03-SUMMARY.md` when done.
</output>