- 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>
20 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | must_haves | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 06.2 | 03 | execute | 1 |
|
|
true |
|
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.
<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>
@/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.mdFrom 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
Task 1: Backend — cloud-aware delete routing + skip_quota + remove_only param backend/api/documents.py, backend/services/storage.py, backend/tests/test_documents.py - 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) - 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. 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); thenawait 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.
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
- 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
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: setcloudProviderName.valuefrom document's storage_backend (map "google_drive" → "Google Drive", "onedrive" → "OneDrive", "nextcloud" → "Nextcloud", "webdav" → "WebDAV", fallback to "your cloud storage"); setshowCloudDeleteWarning.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.)
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
grep "showCloudDeleteWarning" frontend/src/views/DocumentView.vuereturns at least 2 matches (ref + template v-if)grep "cloud-delete-modal-title" frontend/src/views/DocumentView.vuereturns a matchgrep "remove_only" frontend/src/api/client.jsreturns a matchgrep "Remove from app" frontend/src/views/DocumentView.vuereturns a match- No console errors when building:
cd frontend && npm run build 2>&1 | grep -i error | head -10returns empty (or pre-existing errors only)
<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> |
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"
<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>