157 lines
8.0 KiB
Markdown
157 lines
8.0 KiB
Markdown
---
|
|
phase: "12.1"
|
|
plan: "01"
|
|
subsystem: cloud-storage-backends
|
|
status: complete
|
|
tags: [nextcloud, webdav, google-drive, onedrive, cloud-resource-adapter, contract-tests, security]
|
|
completed: "2026-06-22"
|
|
duration_seconds: 823
|
|
task_count: 4
|
|
file_count: 14
|
|
|
|
requires: []
|
|
provides:
|
|
- "Canonical four-provider CloudResourceAdapter contract suite"
|
|
- "Nextcloud list_folder signature defect eliminated"
|
|
- "normalize_nextcloud_url shared helper"
|
|
- "OneDrive @odata.nextLink cross-origin guard"
|
|
affects: [cloud-browse-api, cloud-backend-factory, provider-tests]
|
|
|
|
tech_stack:
|
|
added: []
|
|
patterns:
|
|
- "TDD RED/GREEN — contract tests written first, production defect confirmed, then implementation fixed"
|
|
- "normalize_nextcloud_url idempotent URL derivation with SSRF passthrough"
|
|
- "OneDrive cross-origin nextLink validation before following"
|
|
|
|
key_files:
|
|
created:
|
|
- backend/tests/test_cloud_provider_contract.py
|
|
- backend/tests/fixtures/cloud/nextcloud_root.xml
|
|
- backend/tests/fixtures/cloud/webdav_root.xml
|
|
- backend/tests/fixtures/cloud/google_drive_pages.json
|
|
- backend/tests/fixtures/cloud/onedrive_pages.json
|
|
modified:
|
|
- backend/storage/nextcloud_backend.py
|
|
- backend/storage/cloud_utils.py
|
|
- backend/storage/cloud_backend_factory.py
|
|
- backend/storage/onedrive_backend.py
|
|
- backend/tests/test_cloud_backends.py
|
|
- backend/tests/test_webdav_backend.py
|
|
- backend/main.py
|
|
- frontend/package.json
|
|
- CLAUDE.md
|
|
- README.md
|
|
|
|
decisions:
|
|
- "Remove NextcloudBackend.list_folder override so Nextcloud inherits canonical WebDAVBackend method (direct fix for P0)"
|
|
- "normalize_nextcloud_url lives in cloud_utils.py — called at factory construction, not in every request"
|
|
- "OneDrive nextLink restricted to graph.microsoft.com origin — cross-origin returns complete=False with prior items retained"
|
|
- "asyncio import restored to nextcloud_backend.py (health_check uses asyncio.to_thread)"
|
|
---
|
|
|
|
# Phase 12.1 Plan 01: Restore Cloud Adapter Contract — Summary
|
|
|
|
**One-liner:** Removed incompatible `NextcloudBackend.list_folder(folder_path="")` override, added `normalize_nextcloud_url()`, cross-origin OneDrive nextLink guard, and parametrized four-provider contract suite.
|
|
|
|
## What Was Built
|
|
|
|
### Task 1 — TDD RED: Four-provider contract tests and fixtures
|
|
|
|
- `backend/tests/test_cloud_provider_contract.py` — 48 parametrized tests across Nextcloud, WebDAV, Google Drive, OneDrive covering:
|
|
- Canonical signature verification (`connection_id, user_id, parent_ref=None, page_token=None`)
|
|
- Return type is always `CloudListing` (never `list[dict]`)
|
|
- Trusted caller identity (`connection_id`, `user_id`) is never overrideable by provider response
|
|
- `provider_item_id` is set; `kind` is exactly "file" or "folder"
|
|
- `parent_ref` is propagated to each resource
|
|
- Absent optional metadata normalizes to `None`
|
|
- All pages consumed before `complete=True`; page failure → `complete=False`
|
|
- Forbidden-operation spies for `get_object`, `put_object`, `delete_object`, etc.
|
|
- Synthetic credential-free fixtures: `nextcloud_root.xml`, `webdav_root.xml`, `google_drive_pages.json`, `onedrive_pages.json`
|
|
- 9 Nextcloud-specific tests confirmed RED (TypeError on canonical invocation) as expected
|
|
|
|
### Task 2 — GREEN: Repair Nextcloud/WebDAV listing
|
|
|
|
- Removed `async def list_folder(self, folder_path: str = "") -> list[dict]` from `NextcloudBackend` — root cause of P0 defect
|
|
- `NextcloudBackend` now inherits `WebDAVBackend.list_folder` via standard Python inheritance
|
|
- Added `normalize_nextcloud_url(base_url, username)` to `cloud_utils.py`:
|
|
- Accepts bare origin, origin with subpath, already-canonical URL
|
|
- Username percent-encoded as single path segment
|
|
- HTTPS-only; rejects userinfo/query/fragment
|
|
- Idempotent — no double-append of WebDAV suffix
|
|
- SSRF-validated via existing `validate_cloud_url()` before return
|
|
- `cloud_backend_factory.py` calls `normalize_nextcloud_url` before `NextcloudBackend` construction; `ValueError` from DNS failures is caught gracefully
|
|
- Added `TestNextcloudBackendNoListFolderOverride` and `TestNextcloudUrlNormalization` to `test_webdav_backend.py`
|
|
- Restored `asyncio` import (was inadvertently removed with the legacy `list_folder`; `health_check` still uses `asyncio.to_thread`)
|
|
|
|
### Task 3 — Drive/OneDrive contract completion
|
|
|
|
- Added `_GRAPH_HOST` constant to `onedrive_backend.py`; `@odata.nextLink` hostname validated against `graph.microsoft.com` before following; cross-origin → `complete=False` with prior items retained
|
|
- Added `TestOneDriveCrossOriginNextLink`: cross-origin rejection, same-origin follows, page failure retains prior items
|
|
- Added `TestGoogleDriveAuthFailureControl`: 401 → `complete=False`, page1+page2 error retains page1 items
|
|
|
|
### Task 4 — Documentation and version bump
|
|
|
|
- Version bumped to `0.2.3` in `backend/main.py` and `frontend/package.json`
|
|
- `CLAUDE.md` updated: current state, shared module map, Nextcloud no-override rule
|
|
- `README.md` updated: version, Nextcloud URL normalization note, OneDrive nextLink origin guard
|
|
|
|
## Deviations from Plan
|
|
|
|
### Auto-fixed Issues
|
|
|
|
**1. [Rule 1 - Bug] Missing asyncio import in NextcloudBackend after legacy method removal**
|
|
- **Found during:** Task 4 (full test suite run)
|
|
- **Issue:** Removing the legacy `list_folder` method also removed the only active use of `asyncio` in the file, but the `health_check` method still calls `asyncio.to_thread`. This caused `test_nextcloud_connect_persists` and `test_same_provider_connections_are_independent` to fail.
|
|
- **Fix:** Re-added `import asyncio` to `nextcloud_backend.py`
|
|
- **Files modified:** `backend/storage/nextcloud_backend.py`
|
|
- **Commit:** 6212873
|
|
|
|
**2. [Rule 2 - Missing security] OneDrive @odata.nextLink cross-origin validation**
|
|
- **Found during:** Task 3 (plan specifies "reject cross-origin/untrusted continuation URLs")
|
|
- **Issue:** `onedrive_backend.py` followed `@odata.nextLink` without verifying the target stays on `graph.microsoft.com`; an SSRF-adjacent CSRF vector
|
|
- **Fix:** Added `_GRAPH_HOST` constant and `urllib.parse` hostname check before following any pagination URL
|
|
- **Files modified:** `backend/storage/onedrive_backend.py`
|
|
- **Commit:** 805fe44
|
|
|
|
## Test Evidence
|
|
|
|
| Suite | Tests | Status |
|
|
|-------|-------|--------|
|
|
| `test_cloud_provider_contract.py` | 48 | All pass |
|
|
| `test_cloud_backends.py` | 67 | All pass |
|
|
| `test_webdav_backend.py` | 29 | All pass (9 warnings for sync tests with asyncio mark — module-level `pytestmark` issue, tests behave correctly) |
|
|
| `test_cloud_security.py` | 19 | All pass |
|
|
| Full backend suite | 585 | 1 pre-existing failure (`test_extract_docx` — missing `python-docx` module, unrelated) |
|
|
|
|
## Security Verification
|
|
|
|
Bandit scanned `nextcloud_backend.py`, `cloud_utils.py`, `cloud_backend_factory.py`, `onedrive_backend.py` — zero HIGH/MEDIUM/LOW findings.
|
|
|
|
Threat model coverage:
|
|
|
|
| Threat ID | Mitigation | Evidence |
|
|
|-----------|-----------|---------|
|
|
| T-12.1-01 | Provider response fields cannot override connection_id/user_id | `TestProviderHostileIdentityRejection` in contract suite |
|
|
| T-12.1-02 | SSRF through Nextcloud URL normalization | `normalize_nextcloud_url` always calls `validate_cloud_url`; HTTPS-only; rejects userinfo |
|
|
| T-12.1-03 | Partial page authorizes deletion | `complete=False` on error; `test_provider_page_failure_is_incomplete` |
|
|
| T-12.1-04 | Browse downloads or mutates provider content | `test_provider_listing_never_downloads_or_mutates` spies on forbidden methods |
|
|
| T-12.1-05 | Secrets in fixtures or error messages | Synthetic fixtures only; controlled exceptions |
|
|
|
|
## Known Stubs
|
|
|
|
None — all contract tests use real provider implementations (not mocks of the contract itself).
|
|
|
|
## Self-Check: PASSED
|
|
|
|
| Check | Result |
|
|
|-------|--------|
|
|
| `backend/tests/test_cloud_provider_contract.py` | FOUND |
|
|
| `backend/tests/fixtures/cloud/nextcloud_root.xml` | FOUND |
|
|
| `backend/storage/nextcloud_backend.py` | FOUND |
|
|
| `backend/storage/cloud_utils.py` | FOUND |
|
|
| Commit eb68fac (RED tests) | FOUND |
|
|
| Commit 2b46f74 (GREEN fix) | FOUND |
|
|
| Commit 805fe44 (Drive/OneDrive) | FOUND |
|
|
| Commit 6212873 (docs, asyncio fix) | FOUND |
|