docs(06.2): add phase verification report
This commit is contained in:
+165
@@ -0,0 +1,165 @@
|
||||
---
|
||||
phase: "06.2-close-v1-sharing-cloud-delete-csv-export-gaps"
|
||||
verified: "2026-05-31T12:00:00Z"
|
||||
status: human_needed
|
||||
score: 5/5
|
||||
overrides_applied: 0
|
||||
human_verification:
|
||||
- test: "Security gate — run bandit -r backend/ and confirm zero HIGH severity findings"
|
||||
expected: "bandit reports zero HIGH severity issues in modified files (shares.py, audit.py, documents.py, storage.py)"
|
||||
why_human: "bandit requires the full Python environment with all deps installed in a compatible Python 3.10+ environment; local Python is 3.9.6 which hits Google API core version warnings"
|
||||
- test: "Security gate — run pip audit and npm audit --audit-level=high"
|
||||
expected: "Zero critical/high CVEs from pip audit; zero high/critical from npm audit"
|
||||
why_human: "Requires running in the project's Docker environment; cannot verify without network access and correct env setup"
|
||||
- test: "Cloud delete modal UX flow — delete a cloud-stored document in the browser"
|
||||
expected: "When cloud delete fails, a modal appears with 'Cloud delete failed' heading and 'Remove from app' / 'Cancel' buttons; clicking 'Remove from app' deletes the DB row and navigates to /"
|
||||
why_human: "Real-time modal behavior with cloud provider mock requires browser interaction"
|
||||
- test: "ShareModal permission toggle — open the sharing modal for a shared document"
|
||||
expected: "Each shared recipient row shows two toggle buttons ('View' / 'Edit'); active button has indigo highlight; clicking inactive button sends PATCH and shows updated state optimistically"
|
||||
why_human: "Visual state and optimistic-update behavior require browser interaction"
|
||||
- test: "AuditLogTab daily exports section — open admin audit log panel"
|
||||
expected: "Daily exports section visible below pagination; when no MinIO exports exist shows 'No daily exports available.'; when exports exist shows date dropdown + Download button"
|
||||
why_human: "Requires admin account in running app with MinIO populated by Celery daily export task"
|
||||
---
|
||||
|
||||
# Phase 06.2: Close v1 Sharing + Cloud-Delete + CSV Export Gaps — Verification Report
|
||||
|
||||
**Phase Goal:** Close remaining v1 gaps — sharing edge cases (SHARE-03/SHARE-05), cloud document deletion propagation to the remote backend, and CSV export + daily export UI for the admin audit log (ADMIN-06).
|
||||
**Verified:** 2026-05-31T12:00:00Z
|
||||
**Status:** human_needed
|
||||
**Re-verification:** No — initial verification
|
||||
|
||||
## Goal Achievement
|
||||
|
||||
### Observable Truths (from ROADMAP.md Success Criteria)
|
||||
|
||||
| # | Truth | Status | Evidence |
|
||||
|---|-------|--------|---------|
|
||||
| 1 | Documents shared with others display a "Shared" badge reading `doc.is_shared`, not `doc.share_count` | VERIFIED | `DocumentCard.vue` line 31: `v-if="doc.is_shared"`. `grep "share_count" DocumentCard.vue` returns no match. Backend `list_documents` populates `is_shared` from `Share.document_id` set query. |
|
||||
| 2 | Owner can set permission to "view" or "edit" at share creation and toggle per-recipient; PATCH /api/shares/{id} enforces IDOR (404 on wrong owner) | VERIFIED | `shares.py`: `ShareCreate.permission` field with `field_validator`; `grant_share` uses `permission=body.permission`; `PATCH /{share_id}` handler at line 246 with `share.owner_id != current_user.id → 404`. `test_share_patch_idor` passes (confirmed by test run: 50 passed, 4 xfailed). |
|
||||
| 3 | Deleting a cloud document propagates to cloud provider; failure shows warning modal with "Remove from app" fallback; `?remove_only=true` removes only the DB record; cloud docs never affect quota on delete | VERIFIED | `documents.py` lines 638-654: cloud routing block calls `get_storage_backend_for_document` then `backend.delete_object`; on exception returns `JSONResponse(200, {cloud_delete_failed: True})`; `remove_only=true` skips cloud call; `skip_quota=is_cloud` guards quota decrement. `DocumentView.vue` line 114: `v-if="showCloudDeleteWarning"` modal with `confirmRemoveOnly` handler. Three promoted tests pass. |
|
||||
| 4 | Admin can download filtered audit log CSV via fetch+Blob (not `window.location.href`); audit log entries show user handles; user filter accepts handles (not UUIDs) | VERIFIED | `adminExportAuditLogCsv()` in `client.js` uses raw `fetch()` + `Blob()` + `<a>` click — no `window.location.href`. `grep "window.location.href" AuditLogTab.vue` → no match. `audit.py`: `list_audit_log` accepts `user_handle: Optional[str]`, resolves to UUID internally; returns `user_handle` and `actor_handle` fields in every item via `_audit_to_dict_with_handles`. Five promoted tests pass. |
|
||||
| 5 | Admin can list and download Celery daily audit export files from a new section in the Audit Log tab | VERIFIED | `audit.py` lines 168-239: two new endpoints `GET /audit-log/daily-exports` and `GET /audit-log/daily-exports/{date}`. `AuditLogTab.vue` lines 129-165: "Daily exports" section with date `<select>` and Download button. `client.js`: `adminListDailyExports()` and `adminDownloadDailyExport(date)` functions present. `test_daily_exports_list` and `test_daily_export_download` pass. |
|
||||
|
||||
**Score:** 5/5 truths verified
|
||||
|
||||
### Requirements Cross-Reference
|
||||
|
||||
Requirements declared across all four plans: **SHARE-03, SHARE-05, ADMIN-06** (plus cloud-delete, covered under phase success criteria with no named REQUIREMENTS.md ID).
|
||||
|
||||
The user instruction also mentions **SHARE-02** and **STORE-06**.
|
||||
|
||||
| Requirement | Source Plan | Description | Status | Evidence |
|
||||
|-------------|------------|-------------|--------|---------|
|
||||
| SHARE-03 | 06.2-01, 06.2-02 | Shared access view-only by default; owner controls permission level | SATISFIED | `ShareCreate.permission` field defaults to "view" with validator; PATCH endpoint allows toggling; `test_share_create_with_permission` and `test_share_patch_permission` pass |
|
||||
| SHARE-05 | 06.2-01, 06.2-02 | Documents shared with others display a "shared" indicator in the owner's list view | SATISFIED | `DocumentCard.vue` uses `doc.is_shared`; `test_share_indicator_in_owner_list` pre-exists and passes |
|
||||
| ADMIN-06 | 06.2-01, 06.2-04 | Admin audit log viewer filtered by date range, user, and action type (metadata only) | SATISFIED | Handle-enriched query, `user_handle` filter, daily export endpoints, CSV export fixed; 5 promoted tests pass |
|
||||
| SHARE-02 | Not claimed in any plan | "Shared with me" virtual folder for recipient | NOT IN SCOPE — pre-existing coverage | SHARE-02 is mentioned only in CONTEXT.md as already implemented via `SharedView.vue`. Not a gap this phase closed. SHARE-02 tests pass as part of the pre-existing test_shares.py suite (test_shared_with_me, test_share_no_quota_impact). No gap existed. |
|
||||
| STORE-06 | Not claimed in any plan | Atomic quota decrement on document delete | NOT IN SCOPE — pre-existing + partially extended | STORE-06 base implementation pre-exists in `storage.delete_document`. Phase 06.2 added `skip_quota=True` for cloud docs (which never charged quota). The ROADMAP.md phase gate notes `test_delete_decrements_quota` under INTEGRATION=1 belongs to Phase 6.1 gates, not 6.2. The `skip_quota` extension in this phase is correct (cloud docs were never counted). |
|
||||
|
||||
**Note on SHARE-02 and STORE-06:** Neither requirement ID appears in any Phase 06.2 PLAN frontmatter `requirements:` field, meaning they were not claimed as deliverables of this phase. The pre-existing implementation satisfies both. The user's instruction to cross-reference these IDs has been honored — neither is a gap introduced or created by this phase.
|
||||
|
||||
### Required Artifacts
|
||||
|
||||
| Artifact | Expected | Status | Details |
|
||||
|----------|----------|--------|---------|
|
||||
| `backend/api/shares.py` | `SharePermissionPatch` model + PATCH endpoint | VERIFIED | `class SharePermissionPatch` at line 51; `@router.patch("/{share_id}")` at line 246; IDOR check at line 264 |
|
||||
| `backend/api/audit.py` | `_audit_to_dict_with_handles`, `user_handle` filter, two daily-export endpoints | VERIFIED | All four elements present and substantive |
|
||||
| `backend/api/documents.py` | `remove_only` query param + cloud routing | VERIFIED | `remove_only: bool = Query(default=False)` at line 607; cloud routing block at lines 638-653 |
|
||||
| `backend/services/storage.py` | `skip_quota` parameter on `delete_document` | VERIFIED | `skip_quota: bool = False` in signature at line 143; guarded quota decrement at line 167 |
|
||||
| `frontend/src/views/DocumentView.vue` | `CloudDeleteWarningModal` inline block | VERIFIED | `v-if="showCloudDeleteWarning"` at line 114; `confirmRemoveOnly` at line 139; `cloudProviderName` ref at line 173 |
|
||||
| `frontend/src/components/documents/DocumentCard.vue` | `v-if="doc.is_shared"` badge fix | VERIFIED | Line 31: `v-if="doc.is_shared"` — `share_count` not present |
|
||||
| `frontend/src/components/sharing/ShareModal.vue` | Permission dropdown + View/Edit toggle | VERIFIED | `aria-label="Permission level"` at line 41; `handlePermissionChange` at line 176 |
|
||||
| `frontend/src/stores/documents.js` | `updateSharePermission` action | VERIFIED | Lines 171-172: `updateSharePermission(shareId, permission)` calls `api.updateSharePermission` |
|
||||
| `frontend/src/api/client.js` | `adminExportAuditLogCsv`, `adminListDailyExports`, `adminDownloadDailyExport` | VERIFIED | All three functions present with fetch+Blob pattern |
|
||||
| `backend/tests/test_shares.py` | 3 promoted tests (create_with_permission, patch_permission, patch_idor) | VERIFIED | All three are real integration tests; no `pytest.xfail` body |
|
||||
| `backend/tests/test_audit.py` | 5 promoted tests (includes_user_handle, filter_by_handle, filter_unknown_handle, daily_exports_list, daily_export_download) | VERIFIED | All five are real integration tests |
|
||||
| `backend/tests/test_documents.py` | 3 promoted tests (propagates, failure, remove_only) | VERIFIED | All three are real integration tests |
|
||||
|
||||
### Key Link Verification
|
||||
|
||||
| From | To | Via | Status | Details |
|
||||
|------|----|----|--------|---------|
|
||||
| `ShareModal.vue` | `PATCH /api/shares/{id}` | `docsStore.updateSharePermission(shareId, permission)` | VERIFIED | `handlePermissionChange` → `docsStore.updateSharePermission` (line 184) → `api.updateSharePermission` in client.js → `PATCH /api/shares/${shareId}` |
|
||||
| `shares.py PATCH` | `Share.owner_id` | IDOR check — 404 on mismatch | VERIFIED | Line 264: `if share is None or share.owner_id != current_user.id: raise HTTPException(404, ...)` |
|
||||
| `documents.py` | `storage.get_storage_backend_for_document()` | Cloud routing before MinIO path | VERIFIED | Lines 40 (import) and 640 (call) |
|
||||
| `documents.py` | `services/storage.delete_document(skip_quota=True)` | `skip_quota` param for cloud docs | VERIFIED | Line 654: `ok = await storage.delete_document(session, doc_id, skip_quota=is_cloud)` |
|
||||
| `DocumentView.vue` | `DELETE /api/documents/{id}?remove_only=true` | `confirmRemoveOnly()` handler | VERIFIED | `confirmRemoveOnly` at line 281 calls `api.deleteDocumentRemoveOnly`; `deleteDocumentRemoveOnly` in client.js calls `deleteDocument(id, true)` which appends `?remove_only=true` |
|
||||
| `audit.py list_audit_log` | User table (aliased twice) | `outerjoin` on user_id and actor_id FKs | VERIFIED | Lines 139-149: `UserSubject = aliased(User)`, `UserActor = aliased(User)`, two `outerjoin` calls |
|
||||
| `audit.py list_daily_exports` | MinIO audit-logs bucket | `asyncio.to_thread(_list)` | VERIFIED | Line 198: `items = await asyncio.to_thread(_list)` |
|
||||
| `AuditLogTab.vue:exportCsv` | `adminExportAuditLogCsv()` in client.js | fetch() + Blob URL | VERIFIED | Line 243: `await api.adminExportAuditLogCsv({...})`; `adminExportAuditLogCsv` uses raw fetch not `request()` wrapper |
|
||||
|
||||
### Data-Flow Trace (Level 4)
|
||||
|
||||
| Artifact | Data Variable | Source | Produces Real Data | Status |
|
||||
|----------|--------------|--------|--------------------|--------|
|
||||
| `AuditLogTab.vue` | `entries` | `api.adminListAuditLog()` → `GET /api/admin/audit-log` → SQLAlchemy query with handle JOIN | Yes — DB query via `_build_filtered_query_with_handles` | FLOWING |
|
||||
| `AuditLogTab.vue` | `dailyExports` | `api.adminListDailyExports()` → `GET /api/admin/audit-log/daily-exports` → MinIO `list_objects` via `asyncio.to_thread` | Yes — MinIO bucket listing | FLOWING |
|
||||
| `ShareModal.vue` | `shares` | `docsStore.listShares(doc.id)` → `GET /api/shares?document_id=X` | Yes — DB query with Join | FLOWING |
|
||||
| `DocumentView.vue` | `showCloudDeleteWarning` | `api.deleteDocument()` response: `resp.cloud_delete_failed === true` | Yes — triggered by real API response | FLOWING |
|
||||
|
||||
### Behavioral Spot-Checks
|
||||
|
||||
| Behavior | Command | Result | Status |
|
||||
|----------|---------|--------|--------|
|
||||
| All promoted tests pass | `python3 -m pytest tests/test_shares.py tests/test_audit.py tests/test_documents.py -x -q` | 50 passed, 4 xfailed | PASS |
|
||||
| Full suite exits 0 (excluding pre-existing xfail) | `python3 -m pytest -q` | 1 failed (pre-existing `test_extract_docx` ModuleNotFoundError), 343 passed, 5 skipped, 8 xfailed | PASS — pre-existing failure documented |
|
||||
| `window.location.href` absent from AuditLogTab.vue | `grep "window.location.href" AuditLogTab.vue` | No output | PASS |
|
||||
| Date regex validation present | `grep "fullmatch" audit.py` | Line 216: `re.fullmatch(r"\d{4}-\d{2}-\d{2}", date)` | PASS |
|
||||
| IDOR protection: two owner checks in shares.py | `grep "share.owner_id != current_user.id" shares.py` | Lines 264 and 295 | PASS |
|
||||
| `doc.is_shared` used (not `share_count`) | `grep "is_shared\|share_count" DocumentCard.vue` | Line 31: `doc.is_shared`; no `share_count` | PASS |
|
||||
| `SharePermissionPatch` class exists | `grep "class SharePermissionPatch" shares.py` | Line 51 | PASS |
|
||||
| `_audit_to_dict_with_handles` used in both endpoints | Count matches in audit.py | Definition + `list_audit_log` + `export_audit_log` uses | PASS — Pitfall 7 compliance confirmed |
|
||||
|
||||
### Anti-Patterns Found
|
||||
|
||||
No TBD, FIXME, or XXX markers found in any file modified by this phase. The one `placeholder` occurrence found (`documents.py` line 20) is in a pre-existing docstring comment about `doc.user_id=None guard in /confirm is a Wave 2 placeholder` — this describes a historical implementation note, is not in modified code, and does not affect the phase deliverables.
|
||||
|
||||
| File | Line | Pattern | Severity | Impact |
|
||||
|------|------|---------|----------|--------|
|
||||
| None | — | — | — | No blockers found |
|
||||
|
||||
### Human Verification Required
|
||||
|
||||
Phase gates from ROADMAP.md include security agent checks that cannot be verified by grep alone:
|
||||
|
||||
#### 1. Security Gate — bandit static analysis
|
||||
|
||||
**Test:** Run `cd backend && bandit -r . --skip B104,B608 2>&1 | grep HIGH | grep -v test`
|
||||
**Expected:** Zero HIGH severity findings in modified files (shares.py, audit.py, documents.py, services/storage.py)
|
||||
**Why human:** bandit requires Python 3.10+ environment; local Python is 3.9.6 which triggers version warnings; also requires the full dependency graph loaded
|
||||
|
||||
#### 2. Security Gate — pip audit + npm audit
|
||||
|
||||
**Test:** Run `pip audit` in backend environment and `npm audit --audit-level=high` in frontend
|
||||
**Expected:** Zero critical/high CVEs from pip audit; zero high/critical from npm audit
|
||||
**Why human:** Requires the project's Docker environment with pinned dependency versions and network access
|
||||
|
||||
#### 3. Cloud Delete Modal UX Flow
|
||||
|
||||
**Test:** In a running app with a cloud-connected document, click Delete. When the cloud provider rejects the delete (simulate via a mock or a real provider with revoked credentials), observe the modal behavior.
|
||||
**Expected:** Modal appears with "Cloud delete failed" heading, "Remove from app" CTA, and "Cancel" button. Clicking "Remove from app" removes the document from the DB and navigates to /. Clicking "Cancel" closes modal and leaves document intact.
|
||||
**Why human:** Real-time modal appearance, correct provider name mapping ("google_drive" → "Google Drive"), and navigation behavior require browser interaction
|
||||
|
||||
#### 4. ShareModal Permission Toggle Interaction
|
||||
|
||||
**Test:** Open the sharing modal for a document shared with at least one recipient. Observe the permission toggle per row. Click the inactive button ("Edit" if current is "View").
|
||||
**Expected:** Active button has indigo background. Inactive button is gray. Clicking inactive button optimistically updates state, sends PATCH, and shows new state. On API error, reverts and shows "Failed to update permission."
|
||||
**Why human:** Optimistic-update behavior, rollback on error, and disabled-during-inflight state require browser interaction
|
||||
|
||||
#### 5. AuditLogTab Daily Exports Section
|
||||
|
||||
**Test:** Log in as admin, navigate to audit log tab, scroll to bottom of page.
|
||||
**Expected:** "Daily exports" section visible below pagination with border-t separator. If MinIO has no daily export files, shows "No daily exports available." italic text. If files exist, shows date dropdown and Download button.
|
||||
**Why human:** Requires running app with admin account; MinIO population by Celery `export_audit_log_daily` task is environment-dependent
|
||||
|
||||
### Gaps Summary
|
||||
|
||||
No technical gaps found. All must-haves are verified in the codebase. The one pre-existing test failure (`test_extract_docx` — `ModuleNotFoundError`) is documented in prior phases and is unrelated to Phase 06.2 deliverables.
|
||||
|
||||
The phase is blocked from `passed` status solely because of the mandatory security gate checks (bandit, pip audit, npm audit) required by ROADMAP.md phase gates and CLAUDE.md security protocol. These are policy gates, not implementation gaps.
|
||||
|
||||
---
|
||||
|
||||
_Verified: 2026-05-31T12:00:00Z_
|
||||
_Verifier: Claude (gsd-verifier)_
|
||||
Reference in New Issue
Block a user