docs(phase-12.1): add security threat verification
This commit is contained in:
@@ -0,0 +1,90 @@
|
|||||||
|
# SECURITY.md — Phase 12.1: fix-nextcloud-root-listing-and-sync-visibility
|
||||||
|
|
||||||
|
**Phase:** 12.1 — fix-nextcloud-root-listing-and-sync-visibility
|
||||||
|
**Audit date:** 2026-06-22
|
||||||
|
**ASVS Level:** 2
|
||||||
|
**block_on:** HIGH (open threats)
|
||||||
|
**Auditor:** gsd-secure-phase (automated)
|
||||||
|
**Threats Closed:** 21/21
|
||||||
|
**Open Threats:** 0
|
||||||
|
**Warnings:** 1 (unregistered_flag: none; supply-chain pip audit evidence gap — see below)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Threat Verification
|
||||||
|
|
||||||
|
| Threat ID | Category | Disposition | Status | Evidence |
|
||||||
|
|-----------|----------|-------------|--------|----------|
|
||||||
|
| T-12.1-01 | Identity integrity | mitigate | CLOSED | `backend/tests/test_cloud_provider_contract.py:589` — `TestProviderHostileIdentityRejection` class; line 311 asserts `resource.connection_id` equals trusted caller; line 315 asserts `resource.user_id` equals trusted caller. CloudResource construction in `webdav_backend.py:317–331` copies caller-supplied `connection_id` and `user_id` directly — provider payload cannot override them. |
|
||||||
|
| T-12.1-02 | SSRF | mitigate | CLOSED | `backend/storage/cloud_utils.py:45` — `validate_cloud_url()` rejects private/loopback/link-local/RFC-1918 ranges via socket.getaddrinfo DNS resolution; `normalize_nextcloud_url()` at line 144 rejects userinfo (line 188), query strings (line 196), fragments (line 200), non-HTTPS (line 184), and calls `validate_cloud_url()` before return (line 230). `webdav_backend.py:78` calls `validate_cloud_url` at construction and before every `asyncio.to_thread` call. onedrive_backend.py adds `_GRAPH_HOST` cross-origin guard on `@odata.nextLink`. |
|
||||||
|
| T-12.1-03 | Partial page authorizes deletion | mitigate | CLOSED | `backend/services/cloud_items.py:189` — `reconcile_cloud_listing` soft-deletes unseen items only inside `if listing.complete:` branch. `apply_listing_and_finalize` at line 309 gates the entire fresh+delete path on `listing.complete`. `test_cloud_provider_contract.py:438` — `test_provider_page_failure_is_incomplete` asserts `complete=False` on any fetch/parse failure across all four providers. |
|
||||||
|
| T-12.1-04 | Browse downloads or mutates provider content | mitigate | CLOSED | `backend/tests/test_cloud_provider_contract.py:476` — `test_provider_listing_never_downloads_or_mutates` installs failing spies on `get_object`, `put_object`, `delete_object`, `upload_to`, `download_from`, `copy`, `move`. `cloud_base.py` abstract interface has no mutation methods. `cloud_items.py:6` — service docstring: "No function calls the quota service or alters quotas.used_bytes." |
|
||||||
|
| T-12.1-05 | Secrets/provider data leak through fixtures or failures | mitigate | CLOSED | All four fixture files contain synthetic-only data: `nextcloud_root.xml:3`, `webdav_root.xml:3`, `google_drive_pages.json:2`, `onedrive_pages.json:2` each carry a comment asserting no real credentials/hostnames/tokens. Provider exceptions in `webdav_backend.py:303–305` return `CloudListing(items=(), complete=False)` without propagating exception text. |
|
||||||
|
| T-12.1-06 | False fresh state hides provider failure | mitigate | CLOSED | `backend/api/cloud/browse.py:274` calls `apply_listing_and_finalize` — not bare `update_folder_state("fresh")`. `backend/tasks/cloud_tasks.py:144` calls `apply_listing_and_finalize`. `cloud_items.py:319–342` — incomplete path sets `refresh_state="warning"` and never assigns `"fresh"`. Summary verification search confirmed no unconditional `refresh_state="fresh"` remains in browse.py or cloud_tasks.py. |
|
||||||
|
| T-12.1-07 | Incomplete listing deletes cached metadata | mitigate | CLOSED | `backend/services/cloud_items.py:189` — deletion block gated by `if listing.complete:`. `apply_listing_and_finalize` at line 290 documents: incomplete path "never deletes unseen children". `test_cloud_items.py` — `test_partial_items_upsert_without_deleting_unseen_children` confirms this invariant. |
|
||||||
|
| T-12.1-08 | Error overwrites last known-good evidence | mitigate | CLOSED | `backend/services/cloud_items.py:320–336` — incomplete branch: comment "Deliberately: do NOT update fs.last_refreshed_at"; only `refresh_state`, `error_code`, `error_message`, and `updated_at` are touched. `update_folder_state` at line 373 advances `last_refreshed_at` only when `refresh_state == "fresh"`. |
|
||||||
|
| T-12.1-09 | Provider errors leak secrets or URLs | mitigate | CLOSED | `backend/services/cloud_items.py:261–266` — `ListingResult` docstring: "never raw exception text"; `warning_message` at line 341 is a static string. `browse.py:303–305` — exception path sets `error_message="Provider temporarily unavailable. Retrying in background."` — no `str(exc)` interpolation. |
|
||||||
|
| T-12.1-10 | Refresh changes bytes/quota | mitigate | CLOSED | `backend/services/cloud_items.py:6,181,298` — three explicit "no quota" docstring assertions. No `get_object`, `put_object`, MinIO, or quota imports in `cloud_items.py`. `test_cloud_provider_contract.py:476` — mutation spy assertions remain active for all four providers. |
|
||||||
|
| T-12.1-11 | DocuVault UUID sent as provider path | mitigate | CLOSED | `frontend/src/views/CloudFolderView.vue:103` — `router.push` uses `item.provider_item_id` as route param, not `item.id`. `CloudFolderTreeItem.vue:64` — `parentRef = props.folder.provider_item_id ?? props.folder.id`. `CloudProviderTreeItem.vue:62` — `loadChildren` filters by `i.kind === 'folder'` and passes opaque refs to API. Contract tests: `test_cloud_provider_contract.py:318` — `provider_item_id` must be non-empty for every resource. |
|
||||||
|
| T-12.1-12 | UI falsely reports provider synchronization | mitigate | CLOSED | `frontend/src/views/CloudFolderView.vue:141–146` — `freshness.refresh_state` mapped verbatim from server response with `?? 'warning'` fallback (never `'fresh'`); `refreshedAt` maps `freshness.last_refreshed_at ?? null` — no `new Date()`. `cloudConnections.js:131–138` — `setBrowseState` stores verbatim server values. `grep -rn "new Date()"` in CloudFolderView.vue and cloudConnections.js returned no matches. |
|
||||||
|
| T-12.1-13 | Provider reference injected into provider-specific URL | mitigate | CLOSED | `frontend/src/views/CloudFolderView.vue:103` — named route object `{ name: 'cloud-folder', params: { ... folderId: item.provider_item_id } }` — Vue Router handles encoding; no string interpolation of opaque refs into route strings. `handleBreadcrumbNavigate` at line 122 uses the same named route pattern. |
|
||||||
|
| T-12.1-14 | Secrets persisted in frontend state | mitigate | CLOSED | `frontend/src/stores/cloudConnections.js:9–22` — `saveLastFolder`/`loadLastFolder` store only `folderId` string in `sessionStorage`; key prefix is `docuvault:cloud:folder:`; no tokens/credentials written. `selectConnection` at line 121–129 clears all browse state without touching sessionStorage. `cloudConnections.js` imports no JWT or credential stores. |
|
||||||
|
| T-12.1-15 | Parallel browser/tree logic drifts | mitigate | CLOSED | `frontend/src/views/CloudFolderView.vue:2,27` — template uses `<StorageBrowser>` exclusively; `StorageBrowser.vue` is the only import for browsing. `CloudProviderTreeItem.vue:29` and `CloudFolderTreeItem.vue:31` — both wrap `TreeItem` from `../ui/TreeItem.vue`; no parallel expand/collapse implementation. |
|
||||||
|
| T-12.1-16 | Live credentials/full URL leak through pytest/logs | mitigate | CLOSED | `backend/tests/test_nextcloud_live.py:70–72` — credentials read from `os.environ.get()` only; no `.env` file load. `_assert_no_secrets_in_string` at line 129 asserts on captured stdout/stderr after every live test (lines 244–245, 349, 510–511, 620–621). Skip message at line 84 names only variable keys, not values. Diagnostic at line 324 prints `unexpected_count` only — "names withheld". |
|
||||||
|
| T-12.1-17 | Live smoke mutates/downloads user data | mitigate | CLOSED | `backend/tests/test_nextcloud_live.py:102–128` — `NetworkMethodGuard` transport intercepts and rejects `PUT`, `POST`, `PATCH`, `DELETE`, `MKCOL`, `MOVE`, `COPY` and byte GET requests before network dispatch. `test_nextcloud_adapter_read_only_root_metadata` at line 203 asserts absence of `get_object`, `put_object`, `delete_object`, `upload_to`, `download_from`, `copy`, `move` calls. |
|
||||||
|
| T-12.1-18 | Live connection crosses user/admin boundary | mitigate | CLOSED | `backend/tests/test_nextcloud_live.py:365,460` — `test_nextcloud_connection_id_browse_as_designated_user` provisions `testuser@docuvault.example` as designated regular user; at line 460 provisions a foreign user and asserts 404 when accessing the designated user's connection-ID endpoint; admin user denial similarly tested. IDOR comment at line 460: "T-12.1-18". |
|
||||||
|
| T-12.1-19 | Count-only test accepts wrong root | mitigate | CLOSED | `backend/tests/fixtures/cloud/nextcloud_expected_root.json:2` — `"owner_confirmed": true`. `test_nextcloud_live.py:547` — `assert manifest.get("owner_confirmed") is True` blocks execution if flag absent. Exact set equality enforced: test asserts actual names exactly equal expected names set, not count. |
|
||||||
|
| T-12.1-20 | Unexpected provider names become persisted sensitive metadata | mitigate | CLOSED | `backend/tests/test_nextcloud_live.py:314,319,324` — `unexpected_count = len(actual_names - _CANDIDATE_NAMES)` computed in memory; diagnostic prints only the integer count with "(names withheld)" annotation; unexpected names never written to fixture, log, or artifact. |
|
||||||
|
| T-12.1-SC | Supply-chain or committed-secret regression | mitigate | CLOSED (WARNING) | Bandit: 0 HIGH findings across all plans (SUMMARY-01:129, SUMMARY-04:95). npm audit: 0 high/critical vulnerabilities (SUMMARY-04:96). Gitleaks: 0 new findings in Phase 12.1 files (SUMMARY-04:97). `git ls-files .env*` — only `.env.example` tracked (SUMMARY-04:98). **WARNING: `pip audit` was required by every plan's verify command but its result is not explicitly recorded by name in any plan summary's security evidence table.** The gate was specified and the verify chain includes it; absence of an explicit recorded outcome is a documentation gap, not a confirmed failure. No HIGH supply-chain CVE finding was reported. |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Unregistered Flags
|
||||||
|
|
||||||
|
Plans 01–04 SUMMARY files each contain a `## Threat Flags` section. Plan 03 SUMMARY line 201 records: "None — this plan is frontend-only with no new network endpoints, auth paths, file access patterns, or schema changes." Plan 04 SUMMARY records the same. No unregistered attack surface was flagged by the executor during any plan in Phase 12.1.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Accepted Risks
|
||||||
|
|
||||||
|
None. All 21 threats are mitigated.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Security Gate Evidence (Phase 12.1 aggregate)
|
||||||
|
|
||||||
|
| Gate | Result | Source |
|
||||||
|
|------|--------|--------|
|
||||||
|
| Bandit HIGH/MEDIUM | 0 findings across Plans 01–04 | SUMMARY-01:129, SUMMARY-04:95 |
|
||||||
|
| pip audit | Gate ran per verify command; result not explicitly recorded by name | All PLAN verify chains include `pip audit` |
|
||||||
|
| npm audit (high/critical) | 0 vulnerabilities | SUMMARY-04:96 |
|
||||||
|
| Gitleaks secret scan | 0 new findings in Phase 12.1 files | SUMMARY-04:97 |
|
||||||
|
| Tracked .env files | None (only .env.example) | SUMMARY-04:98 |
|
||||||
|
| Docker Compose config | All required variables resolve cleanly | SUMMARY-04:99 |
|
||||||
|
| Full backend test suite | 595 pass (1 pre-existing unrelated failure) | SUMMARY-04 |
|
||||||
|
| Full frontend test suite | 376 pass | SUMMARY-04 |
|
||||||
|
| Production build | Clean | SUMMARY-04 |
|
||||||
|
| Live smoke (opt-in) | Skipped in CI — requires env vars | SUMMARY-04 |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Explicit Phase 13/14 Exclusions
|
||||||
|
|
||||||
|
This phase covers cloud metadata listing and freshness only. The following are out of scope and not audited:
|
||||||
|
|
||||||
|
- Phase 13: file upload, create-folder, rename, move, delete provider mutations
|
||||||
|
- Phase 14: byte caching, presigned URL generation, local download
|
||||||
|
|
||||||
|
No mutation or byte-cache code was introduced in Phase 12.1. The `CloudResourceAdapter` abstract interface deliberately contains no mutation methods.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Inherited Controls (from Phase 12)
|
||||||
|
|
||||||
|
The following controls from Phase 12 remain in force and were not regressed by Phase 12.1:
|
||||||
|
|
||||||
|
- IDOR protection via `resolve_owned_connection` (owner UUID check on every browse request)
|
||||||
|
- Credential encryption via HKDF+Fernet per-user key derivation (`cloud_utils.py`)
|
||||||
|
- `credentials_enc` never returned in any API response schema
|
||||||
|
- Admin endpoints blocked from cloud browse content
|
||||||
|
- All connection/item endpoints assert `resource.user_id == current_user.id`
|
||||||
Reference in New Issue
Block a user