curo/kite
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
main
kite/.planning/phases/06.1-close-v1-audit-gaps/06.1-VERIFICATION.md
T
curo1305 3825f670a1 docs(phase-6.1): add VALIDATION.md and commit VERIFICATION.md 
VALIDATION.md: Nyquist audit — 3 gaps found, 2 resolved automated
(SHARE-03 permission field, SHARE-05 is_shared indicator), 1 escalated
to manual-only (STORE-06 requires INTEGRATION=1 PostgreSQL).

VERIFICATION.md: was untracked artifact from gsd-verifier run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-30 23:38:16 +02:00

193 lines
15 KiB
Markdown
Raw Permalink Blame History

---
phase: 06.1-close-v1-audit-gaps
verified: 2026-05-30T00:00:00Z
status: gaps_found
score: 9/11 must-haves verified
overrides_applied: 0
gaps:
- truth: "Admin audit log viewer tests verify filtered results (date range, user, action type actually narrow results)"
status: failed
reason: "None of the 4 audit tests pass filter query params and verify that filtered results are returned. test_audit_log_viewer tests response shape only — it seeds one entry and checks total >= 1, but does not pass start/end/user_id/event_type parameters and verify the narrowed result set. ROADMAP SC3 explicitly states 'filtered independently by date range, user, and action type'."
artifacts:
- path: "backend/tests/test_audit.py"
issue: "No test exercises GET /api/admin/audit-log?user_id=X, ?event_type=Y, or ?start=Z&end=W with assertions that only matching entries are returned"
missing:
- "A filter test that seeds two entries with different event_type values, queries with event_type filter, and asserts only one entry is returned"
- "Or alternatively: a single parametrized test passing each filter type and verifying count/content of filtered results"
- truth: "STORE-06 integration gate confirmed: test_delete_decrements_quota passes under INTEGRATION=1"
status: failed
reason: "test_delete_decrements_quota is marked @pytest.mark.xfail(strict=False, reason='requires PostgreSQL for atomic UUID-typed quota SQL'). The ROADMAP phase gate explicitly requires this test to pass under INTEGRATION=1. This cannot be verified without running the Docker Compose stack with a live PostgreSQL instance."
artifacts:
- path: "backend/tests/test_quota.py"
issue: "test_delete_decrements_quota at line 196 has @pytest.mark.xfail(strict=False) — passes as xfail on SQLite, requires INTEGRATION=1 to confirm as real pass"
missing:
- "Run: INTEGRATION=1 docker compose exec backend python -m pytest tests/test_quota.py::test_delete_decrements_quota -v and confirm PASSED (not XPASS or XFAIL)"
human_verification:
- test: "Run full test suite under INTEGRATION=1"
expected: "All 309 tests pass; test_delete_decrements_quota shows PASSED (not XFAIL/XPASS)"
why_human: "Requires live Docker Compose stack with PostgreSQL + MinIO + Redis to verify STORE-06 integration gate"
- test: "Manually call GET /api/admin/audit-log?event_type=document.uploaded with two different event types seeded"
expected: "Only entries matching the filter are returned; total reflects filtered count, not all entries"
why_human: "The behavioral correctness of each filter (date range, user_id, event_type) is not covered by any test — needs human or integration test to confirm the filter queries work"
---
# Phase 6.1: Close v1.0 Audit Gaps — Verification Report
**Phase Goal:** Close three v1.0 requirements — "Shared with me" virtual folder without recipient quota charge (SHARE-02), admin audit log viewer with date/user/action type filters (ADMIN-06). (STORE-06 quota decrement on delete was pre-existing; this phase closes the test coverage gaps.)
**Verified:** 2026-05-30
**Status:** gaps_found
**Re-verification:** No — initial verification
---
## Goal Achievement
### Observable Truths
| # | Truth | Status | Evidence |
|----|-------|--------|----------|
| 1 | Zero `pytest.xfail` calls in test_shares.py (7 real tests) | VERIFIED | `grep -n "pytest.xfail"` returns no matches; 7 real `async def test_*` functions confirmed |
| 2 | Zero `pytest.xfail` calls in test_audit.py (4 real tests) | VERIFIED | `grep -n "pytest.xfail"` returns no matches; 4 real `async def test_*` functions confirmed |
| 3 | `second_auth_user` fixture added to conftest.py | VERIFIED | `conftest.py` lines 229-265: `@pytest_asyncio.fixture async def second_auth_user(db_session)` with `user2_` handle prefix, Quota row, valid JWT |
| 4 | `test_share_no_quota_impact` proves recipient quota unchanged after share (SHARE-02) | VERIFIED | `test_shares.py` lines 143-167: POSTs share, GETs `/api/auth/me/quota` as recipient, asserts `quota["used_bytes"] == 0` |
| 5 | `test_shared_with_me` proves recipient sees doc with metadata-only response (no extracted_text) | VERIFIED | `test_shares.py` lines 95-135: asserts `"extracted_text" not in received_item` for every item in response; asserts `owner_handle` matches sharer |
| 6 | `test_share_revoke_wrong_owner_404` proves IDOR protection (T-04-04-02) | VERIFIED | `test_shares.py` lines 209-233: recipient DELETE returns 404, not 403 |
| 7 | `test_audit_log_no_doc_content` proves D-15 metadata safety invariant | VERIFIED | `test_audit.py` lines 76-104: checks `forbidden_keys = {"filename", "extracted_text", "password_hash", "credentials_enc"}` at top-level AND nested in `metadata_` (WR-01 fix applied at commit 57784f9) |
| 8 | `test_audit_log_regular_user_403` proves admin gate blocks regular users | VERIFIED | `test_audit.py` lines 107-113: sends request with `auth_user` headers (role="user"), asserts status 403 |
| 9 | `test_audit_log_export_csv` proves CSV export functional with correct content-type | VERIFIED | `test_audit.py` lines 116-152: asserts `content-type` starts with "text/csv", `content-disposition` contains "audit-export.csv", expected CSV header row present, AND forbidden fields absent from CSV body (WR-02 fix applied at commit 57784f9) |
| 10 | Admin audit log viewer tests verify filtered results (date/user/action type actually filter) | FAILED | No test passes filter query params to `/api/admin/audit-log`. `test_audit_log_viewer` seeds one entry and checks `total >= 1` — it does not verify that `?user_id=X`, `?event_type=Y`, or `?start=Z&end=W` return only matching entries |
| 11 | STORE-06 integration gate confirmed under INTEGRATION=1 | FAILED | `test_delete_decrements_quota` at `test_quota.py:196` has `@pytest.mark.xfail(strict=False, reason="requires PostgreSQL...")` — it runs as xfail on in-memory SQLite. The ROADMAP phase gate explicitly requires `INTEGRATION=1` confirmation; cannot verify without live Docker stack |
**Score:** 9/11 truths verified
---
### Deferred Items
None.
---
### Required Artifacts
| Artifact | Expected | Status | Details |
|----------|----------|--------|---------|
| `backend/tests/test_shares.py` | 7 real tests replacing xfail stubs; `pytestmark = pytest.mark.asyncio` | VERIFIED | 7 `async def test_*` functions; `pytestmark` at line 13; no `import os`; no `pytest.xfail` |
| `backend/tests/test_audit.py` | 4 real tests replacing xfail stubs; `pytestmark = pytest.mark.asyncio` | VERIFIED | 4 `async def test_*` functions; `pytestmark` at line 16; no `import os`; no `pytest.xfail` |
| `backend/tests/conftest.py` | `second_auth_user` fixture with `user2_` prefix, Quota row, JWT | VERIFIED | Lines 229-265; identical shape to `auth_user`; `@pytest_asyncio.fixture` decorated |
---
### Key Link Verification
| From | To | Via | Status | Details |
|------|----|-----|--------|---------|
| `test_shares.py:_make_doc()` | `db.models.Document` | ORM direct insert | VERIFIED | `from db.models import Document` inside helper; `db_session.add(doc); await db_session.commit()` |
| `test_shares.py:test_share_*` | `second_auth_user` fixture | pytest fixture parameter | VERIFIED | 5 of 7 tests accept `second_auth_user` as parameter |
| `test_audit.py:_seed_audit()` | `services.audit.write_audit_log` | lazy import inside helper | VERIFIED | `from services.audit import write_audit_log` inside function body (avoids import-ordering issues per SUMMARY) |
| `test_audit.py:test_*` | `/api/admin/audit-log` endpoint | `async_client.get()` | VERIFIED | Endpoint exists at `api/audit.py:85`; `Depends(get_current_admin)` applied |
| `api/audit.py:list_audit_log` | date/user/event_type filters | `_build_filtered_query()` | VERIFIED (implementation only) | `start`, `end`, `user_id`, `event_type` query params wired to `_build_filtered_query()` at line 101; implementation exists but behavioral correctness untested |
---
### Data-Flow Trace (Level 4)
| Artifact | Data Variable | Source | Produces Real Data | Status |
|----------|---------------|--------|-------------------|--------|
| `test_shares.py` | `items` in received response | `/api/shares/received` → `api/shares.py:list_shared_with_me` | Yes — queries `Share` join with `Document` and `User` ORM rows | FLOWING |
| `test_audit.py` | `items` in audit response | `/api/admin/audit-log` → `api/audit.py:list_audit_log` → `AuditLog` DB query | Yes — `_seed_audit()` inserts row via `write_audit_log()`, query reads from `AuditLog` table | FLOWING |
---
### Behavioral Spot-Checks
| Behavior | Command | Result | Status |
|----------|---------|--------|--------|
| No xfail in test_shares.py | `grep -c "pytest.xfail" backend/tests/test_shares.py` | 0 | PASS |
| No xfail in test_audit.py | `grep -c "pytest.xfail" backend/tests/test_audit.py` | 0 | PASS |
| 7 tests in test_shares.py | `grep -c "^async def test_" backend/tests/test_shares.py` | 7 | PASS |
| 4 tests in test_audit.py | `grep -c "^async def test_" backend/tests/test_audit.py` | 4 | PASS |
| second_auth_user fixture present | `grep -c "second_auth_user" backend/tests/conftest.py` | Present at line 229 | PASS |
| WR-01 fix: metadata_ uses full forbidden_keys set | `grep -n "forbidden_keys" test_audit.py` | Line 89 + line 101 both use `forbidden_keys` | PASS |
| WR-02 fix: CSV forbidden fields assertion present | `grep -n "forbidden_csv\|forbidden_field" test_audit.py` | Lines 148-152 | PASS |
---
### Probe Execution
No probes declared in PLAN files. Step 7c: SKIPPED (no probe-*.sh files found for this phase).
---
### Requirements Coverage
| Requirement | Source Plan | Description | Status | Evidence |
|-------------|------------|-------------|--------|----------|
| SHARE-01 | 06.1-01 | Share document by user handle | SATISFIED | `test_share_success` (201 response), `test_share_handle_not_found` (404), `test_share_duplicate` (409) |
| SHARE-02 | 06.1-01 | "Shared with me" virtual folder; no quota charged to recipient | SATISFIED | `test_shared_with_me` (virtual folder), `test_share_no_quota_impact` (used_bytes==0 after share) |
| SHARE-03 | 06.1-01 | View-only default sharing; owner controls permission | PARTIALLY SATISFIED | `api/shares.py` creates shares with `permission="view"` (hardcoded); no test verifies the permission field in response or that write access is blocked. Backend enforces view-only but test doesn't assert the `permission` field value in received items |
| SHARE-04 | 06.1-01 | Immediate share revocation | SATISFIED | `test_revoke_share` (DELETE 204, doc gone from received); `test_share_revoke_wrong_owner_404` (IDOR 404) |
| SHARE-05 | 06.1-01 | Shared indicator in owner's list view | NEEDS HUMAN | `is_shared` field exists in `api/documents.py` (lines 433-445, 498-510); `test_share_duplicate` is labeled as SHARE-05 in test file but tests duplicate prevention (409), not the shared indicator. No test asserts `is_shared=true` in the owner's document list after sharing. Frontend indicator untested. |
| ADMIN-06 | 06.1-02 | Admin audit log viewer filtered by date, user, action (metadata only) | PARTIALLY SATISFIED | Viewer shape, no-doc-content, admin gate, CSV export all tested. Filter behavioral correctness (passing params + verifying narrowed results) is NOT tested. |
**Orphaned requirements:** None — all IDs from PLAN frontmatter found in REQUIREMENTS.md.
**Note on SHARE-05 label mismatch:** `test_share_duplicate` is labeled `# SHARE-05: Duplicate share` in test_shares.py, but SHARE-05 in REQUIREMENTS.md is "Documents shared with others display a 'shared' indicator in the owner's list view." The duplicate-prevention test (409) corresponds more precisely to an enforcement invariant rather than the SHARE-05 user-visible feature. The `is_shared` field is implemented in `api/documents.py` from Phase 4 but lacks a dedicated test.
---
### Anti-Patterns Found
| File | Line | Pattern | Severity | Impact |
|------|------|---------|----------|--------|
| `backend/tests/conftest.py` | 150 | `dependency_overrides` set before `try/finally` guard (WR-03 from REVIEW.md — unfixed) | Warning | If `ASGITransport` raises during startup, `app.dependency_overrides` is never cleared, potentially corrupting subsequent tests in same process. Low probability but correctness gap. |
**Debt-marker check:** No `TBD`, `FIXME`, or `XXX` markers found in modified files (test_shares.py, test_audit.py, conftest.py).
---
### Human Verification Required
#### 1. STORE-06 Integration Gate
**Test:** Run `INTEGRATION=1 docker compose exec backend python -m pytest tests/test_quota.py::test_delete_decrements_quota -v`
**Expected:** `PASSED` (not `XFAIL`, not `XPASS`)
**Why human:** `test_delete_decrements_quota` runs as `xfail` on in-memory SQLite. The ROADMAP phase gate explicitly requires confirmation under live PostgreSQL (`INTEGRATION=1`). Cannot verify without running the Docker Compose stack.
#### 2. Audit Log Filter Behavioral Correctness
**Test:** Call `GET /api/admin/audit-log?event_type=document.uploaded` after seeding one `document.uploaded` and one `share.granted` entry; verify `total == 1` and the returned item has `event_type == "document.uploaded"`.
**Expected:** Only the matching entry is returned; total reflects filtered count.
**Why human:** No test in the promoted suite exercises filtering. The filter implementation exists in `api/audit.py` (`_build_filtered_query()`) but its correctness is only verified by running it. A programmatic spot-check would require a running server or a direct unit test.
#### 3. SHARE-05 Shared Indicator in UI
**Test:** Upload a document, share it with a second user, then view the owner's document list. Check whether a "shared" indicator appears on the document row.
**Expected:** The document shows a visual indicator (e.g., share icon) indicating it has been shared. `is_shared: true` in the API response for the owner's document list.
**Why human:** Frontend rendering cannot be verified by grep. `is_shared` field is present in `api/documents.py` but no test asserts `is_shared == true` in the owner's document list after sharing.
---
### Gaps Summary
Two blockers prevent full goal achievement:
**Gap 1 — Audit filter behavioral tests missing (ROADMAP SC3)**
ROADMAP success criterion 3 requires that an admin can view the audit log "filtered independently by date range, user, and action type." The implementation in `api/audit.py` supports all four filter parameters, but none of the 4 promoted tests verify that filtering actually narrows results. `test_audit_log_viewer` verifies response shape but never passes a filter. A failing filter implementation (e.g., accidentally always returning all entries) would not be caught.
**Gap 2 — STORE-06 integration gate unconfirmed**
The ROADMAP phase gate explicitly requires `test_delete_decrements_quota` to pass under `INTEGRATION=1` with a live PostgreSQL instance. The test is marked `@pytest.mark.xfail(strict=False)` and runs as xfail on SQLite. Whether the atomic `GREATEST(0, used_bytes - delta)` SQL executes correctly under PostgreSQL has not been confirmed in this phase.
**Non-blockers noted:**
- REQUIREMENTS.md checkboxes for SHARE-01..05, ADMIN-06, STORE-06 remain `[ ]` (tracking document not updated)
- WR-03 from REVIEW.md (async_client fixture teardown guard) remains unfixed in conftest.py — low-probability correctness gap
- SHARE-05 test label mismatch and missing `is_shared=true` assertion in owner document list
---
_Verified: 2026-05-30_
_Verifier: Claude (gsd-verifier)_
Reference in New Issue View Git Blame Copy Permalink