diff --git a/backend/api/cloud/operations.py b/backend/api/cloud/operations.py index 21ce5d3..8897d5f 100644 --- a/backend/api/cloud/operations.py +++ b/backend/api/cloud/operations.py @@ -1,5 +1,5 @@ """ -Phase 13 cloud content and mutation endpoints. +Phase 13 cloud content and mutation endpoints (Phase 14 Plan 06: byte cache integration). Owner-scoped routes for opening, previewing, downloading, renaming, moving, deleting, creating folders, and uploading cloud files. All routes are @@ -14,6 +14,16 @@ Security invariants (T-13-14): - Cross-connection moves are rejected before the adapter is called (D-08). - Self-destination moves are rejected before the adapter is called (D-09). - No health probe triggered by folder navigation (D-13). + - cache entries (object_key) are never returned in any response (T-14-02). + +Phase 14 Plan 06 byte cache integration: + - preview_cloud_file and download_cloud_file route provider bytes through + hydrate_and_cache_bytes — cache hits avoid a provider round-trip. + - The cache pin lifecycle protects active preview/download entries from LRU + eviction during the response stream. + - cache_limit_bytes is read from UserAnalysisSettings (default 512 MB). + - All byte storage is behind the MinIO private bucket — object keys are + never exposed. Route family: GET /connections/{id}/items/{item_id}/open — D-02 authorized access URL @@ -132,6 +142,44 @@ async def _resolve_and_get_adapter( return conn, adapter +def _make_minio_cache_wrapper(backend): + """Return a lightweight MinIO wrapper for use with hydrate_and_cache_bytes. + + The wrapper adapts the MinIOBackend instance to the interface expected by + hydrate_and_cache_bytes: async get_object(key), put_object(key, data, content_type), + and delete_object(key). This avoids duplicating the wrapper class at every + call site (Plan 06, DRY principle). + """ + import io as _io + import asyncio as _asyncio + + class _MinIOCacheWrapper: + def __init__(self, b): + self._b = b + + async def get_object(self, key: str) -> bytes: + return await self._b.get_object(key) + + async def put_object(self, key: str, data: bytes, content_type: str = None) -> None: + buf = _io.BytesIO(data) + await _asyncio.to_thread( + self._b._client.put_object, + self._b._bucket, + key, + buf, + length=len(data), + content_type=content_type or "application/octet-stream", + ) + + async def delete_object(self, key: str) -> None: + try: + await self._b.delete_object(key) + except Exception: + pass + + return _MinIOCacheWrapper(backend) + + def _mutation_error_json(result: dict, http_status: int) -> JSONResponse: """Return a typed JSON error response for a failed mutation. @@ -304,14 +352,68 @@ async def preview_cloud_file( if not PreviewSupport.is_supported(content_type): return _unsupported_preview_response(connection_id, item_id, "unsupported_format") - # Step 4: Fetch bytes via the adapter (supported binary format confirmed above) + # Step 4: Resolve the adapter (requires credential decryption). try: conn, adapter = await _resolve_and_get_adapter(session, connection_id, current_user.id) - file_bytes = await adapter.get_object(item_id) except HTTPException: - # Re-raise 401 (credential failure) and 404 (ownership race) — never mask as - # unsupported_preview. The IDOR guard in Step 1 already passed, but a 401 - # from credential decryption should surface as an auth error to the client. + raise + except Exception: + return _unsupported_preview_response(connection_id, item_id, "provider_error") + + # Step 5: Fetch bytes via the byte cache (PATTERNS.md lifecycle, Plan 06). + # hydrate_and_cache_bytes: cache hit → read from MinIO without a provider call; + # cache miss → fetch from provider, store in MinIO, pin for duration of response. + try: + from services.cloud_cache import hydrate_and_cache_bytes + from services.cloud_analysis_versioning import compute_version_key + from services.cloud_analysis_settings import ( + get_or_create_user_analysis_settings, + DEFAULT_CACHE_LIMIT_BYTES, + ) + from storage import get_storage_backend + + # Compute the version key from CloudItem metadata (no provider call required). + version_key = compute_version_key( + provider_item_id=item_id, + version=cloud_item.version if cloud_item else None, + etag=cloud_item.etag if cloud_item else None, + size=cloud_item.provider_size if cloud_item else None, + modified_at=str(cloud_item.modified_at) if (cloud_item and cloud_item.modified_at) else None, + content_type=content_type, + ) if cloud_item else None + + # Resolve user's cache byte limit. + try: + user_settings = await get_or_create_user_analysis_settings( + session, user_id=current_user.id + ) + cache_limit = user_settings.cloud_cache_limit_bytes + except Exception: + cache_limit = DEFAULT_CACHE_LIMIT_BYTES + + if cloud_item and version_key: + # MinIO client wrapper for cache operations. + _backend = get_storage_backend() + + minio_wrapper = _make_minio_cache_wrapper(_backend) + + file_bytes, _ = await hydrate_and_cache_bytes( + session, + user_id=current_user.id, + connection_id=connection_id, + cloud_item_id=cloud_item.id, + provider_item_id=item_id, + version_key=version_key, + content_type=content_type, + fetch_fn=lambda: adapter.get_object(item_id), + minio_client=minio_wrapper, + cache_limit_bytes=cache_limit, + ) + else: + # No CloudItem row or no version key — fetch directly from provider. + file_bytes = await adapter.get_object(item_id) + + except HTTPException: raise except Exception: return _unsupported_preview_response(connection_id, item_id, "provider_error") @@ -364,10 +466,59 @@ async def download_cloud_file( filename = cloud_item.name if cloud_item else item_id content_type = (cloud_item.content_type if cloud_item else None) or "application/octet-stream" + # Fetch bytes via the byte cache (Plan 06, PATTERNS.md cache lifecycle). + # hydrate_and_cache_bytes: cache hit → read from MinIO without a provider call; + # cache miss → fetch from provider, store in MinIO, pin for duration of response. try: - file_bytes = await adapter.get_object(item_id) + from services.cloud_cache import hydrate_and_cache_bytes + from services.cloud_analysis_versioning import compute_version_key + from services.cloud_analysis_settings import ( + get_or_create_user_analysis_settings, + DEFAULT_CACHE_LIMIT_BYTES, + ) + from storage import get_storage_backend + + version_key = compute_version_key( + provider_item_id=item_id, + version=cloud_item.version if cloud_item else None, + etag=cloud_item.etag if cloud_item else None, + size=cloud_item.provider_size if cloud_item else None, + modified_at=str(cloud_item.modified_at) if (cloud_item and cloud_item.modified_at) else None, + content_type=content_type if content_type != "application/octet-stream" else None, + ) if cloud_item else None + + try: + user_settings = await get_or_create_user_analysis_settings( + session, user_id=current_user.id + ) + cache_limit = user_settings.cloud_cache_limit_bytes + except Exception: + cache_limit = DEFAULT_CACHE_LIMIT_BYTES + + if cloud_item and version_key: + _backend = get_storage_backend() + minio_wrapper = _make_minio_cache_wrapper(_backend) + + file_bytes, _ = await hydrate_and_cache_bytes( + session, + user_id=current_user.id, + connection_id=connection_id, + cloud_item_id=cloud_item.id, + provider_item_id=item_id, + version_key=version_key, + content_type=content_type if content_type != "application/octet-stream" else None, + fetch_fn=lambda: adapter.get_object(item_id), + minio_client=minio_wrapper, + cache_limit_bytes=cache_limit, + ) + else: + # No CloudItem metadata — fetch directly from provider. + file_bytes = await adapter.get_object(item_id) + + except HTTPException: + raise except Exception as exc: - result_dict = adapter._normalize_error(exc) + result_dict = getattr(adapter, "_normalize_error", lambda e: {})(exc) if result_dict.get("kind") == MUT_KIND_REAUTH: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, diff --git a/backend/services/cloud_cache.py b/backend/services/cloud_cache.py index 8eddd23..db264a2 100644 --- a/backend/services/cloud_cache.py +++ b/backend/services/cloud_cache.py @@ -28,6 +28,11 @@ and the analysis API. Also re-exports compute_version_key from services.cloud_analysis_versioning so tests and service callers have a single import location. +Exports hydrate_and_cache_bytes for the open/preview/download routes (Plan 06): + hydrate_and_cache_bytes provides the full cache-or-fetch lifecycle for content + routes: cache hit avoids a provider byte fetch; cache miss fetches, stores, + and pins the entry so eviction cannot occur mid-response. + Rules: - Raises ValueError or domain exceptions only — never HTTPException. - Eviction never removes entries with pin_count > 0 or active_job_count > 0. @@ -550,6 +555,185 @@ async def update_cache_access( return entry +# ── Phase 14: open/preview/download cache hydration ────────────────────────── + + +async def hydrate_and_cache_bytes( + session: AsyncSession, + *, + user_id: uuid.UUID, + connection_id: uuid.UUID, + cloud_item_id: uuid.UUID, + provider_item_id: str, + version_key: str, + content_type: Optional[str], + fetch_fn: "Callable[[], Awaitable[bytes]]", + minio_client: "Any", + cache_limit_bytes: int, +) -> tuple[bytes, Optional[uuid.UUID]]: + """Fetch content bytes for a cloud open/preview/download request, routing through the cache. + + Cache-hit path (PATTERNS.md step 3): + - Locate a non-evicted CloudByteCacheEntry matching (user_id, connection_id, + cloud_item_id, version_key). + - Pin the entry before reading from MinIO so eviction cannot occur mid-response + (PATTERNS.md step 7). + - Touch last_accessed_at to update LRU position. + - Read bytes from MinIO via object_key. + - Release pin in a finally block (PATTERNS.md step 8). + - Return (bytes, cache_entry_id). + + Cache-miss path: + - Call fetch_fn() to download bytes from the provider. + - Attempt to store bytes in MinIO under a UUID key (non-fatal on failure). + - If MinIO write succeeds and quota allows: create/reactivate a cache entry + and pin it (protect from eviction during response streaming). + - Release pin in a finally block. + - Return (bytes, cache_entry_id) — cache_entry_id is None if caching failed. + + Rules: + - Provider bytes are NEVER fetched for the cache-hit path. + - Pin is always released in a finally block, even if the caller raises. + - MinIO write failure is non-fatal: bytes are returned from the provider + fetch path regardless. + - Quota failure is non-fatal: bytes are returned but the cache entry is not + created (caller gets bytes without MinIO caching). + - object_key is never returned in any form — it stays internal. + - Raises ValueError only — never HTTPException. + + Args: + session: Active async SQLAlchemy session. + user_id: Authenticated user UUID. + connection_id: Cloud connection UUID. + cloud_item_id: CloudItem row UUID. + provider_item_id: Provider item ID (for audit/snapshot on cache entry). + version_key: Computed version key from compute_version_key. + content_type: MIME type of the file. + fetch_fn: Async callable (no args) that returns provider bytes. + minio_client: Object with async put_object(key, data, content_type) + and get_object(key) methods. + cache_limit_bytes: User's configured byte cache limit; used for eviction + after releasing the pin. + + Returns: + Tuple (bytes, cache_entry_id) where cache_entry_id is the UUID of the + pinned cache entry, or None if no cache entry was created/used. + """ + uid = user_id if isinstance(user_id, uuid.UUID) else uuid.UUID(str(user_id)) + cid = connection_id if isinstance(connection_id, uuid.UUID) else uuid.UUID(str(connection_id)) + iid = cloud_item_id if isinstance(cloud_item_id, uuid.UUID) else uuid.UUID(str(cloud_item_id)) + + # ── Cache-hit path ────────────────────────────────────────────────────── + # Look for a non-evicted entry matching the version key. + stmt = ( + select(CloudByteCacheEntry) + .where( + CloudByteCacheEntry.user_id == uid, + CloudByteCacheEntry.connection_id == cid, + CloudByteCacheEntry.cloud_item_id == iid, + CloudByteCacheEntry.version_key == version_key, + CloudByteCacheEntry.evicted_at.is_(None), + ) + .limit(1) + ) + result = await session.execute(stmt) + existing_entry = result.scalars().first() + + if existing_entry is not None: + # Pin the entry before reading so eviction cannot occur mid-response. + await pin_cache_entry(session, entry_id=existing_entry.id, user_id=uid) + await session.commit() + try: + # Touch LRU position + await update_cache_access(session, entry_id=existing_entry.id, user_id=uid) + await session.commit() + # Read bytes from MinIO (non-fatal if MinIO is unavailable — fall through) + try: + file_bytes = await minio_client.get_object(existing_entry.object_key) + return file_bytes, existing_entry.id + except Exception: + # Cache hit but MinIO read failed — fall through to provider fetch below. + # The entry will be released in the finally block; we re-fetch from provider. + pass + finally: + # Always release pin — clamp protects against double-release. + try: + await release_cache_entry(session, entry_id=existing_entry.id, user_id=uid) + await session.commit() + except Exception: + pass # Non-fatal; log would go here in production + + # ── Cache-miss (or MinIO read failure) path ───────────────────────────── + # Download bytes from the provider. + file_bytes = await fetch_fn() + + # Attempt to store in MinIO and create/reactivate a cache entry. + # All steps are non-fatal — any failure returns the provider bytes without caching. + cache_entry_id: Optional[uuid.UUID] = None + pinned_entry_id: Optional[uuid.UUID] = None + + try: + # Build a UUID-based private object key (T-14-02: never exposed in API responses). + object_key = f"cache/{uid}/{uuid.uuid4()}" + + # Write bytes to MinIO + await minio_client.put_object( + object_key, + file_bytes, + content_type or "application/octet-stream", + ) + + size_bytes = len(file_bytes) + + # Attempt atomic quota increment before creating the cache entry. + try: + await increment_quota_for_cache(session, user_id=uid, size_bytes=size_bytes) + except CacheQuotaExceeded: + # Quota exhausted — skip cache entry creation; return provider bytes. + await minio_client.delete_object(object_key) + return file_bytes, None + + # Create or reactivate the cache entry. + entry, _ = await retain_or_reuse_cache_entry( + session, + user_id=uid, + connection_id=cid, + cloud_item_id=iid, + provider_item_id=provider_item_id, + version_key=version_key, + object_key=object_key, + content_type=content_type, + size_bytes=size_bytes, + ) + await session.commit() + + # Pin for the duration of the response. + await pin_cache_entry(session, entry_id=entry.id, user_id=uid) + await session.commit() + pinned_entry_id = entry.id + cache_entry_id = entry.id + + except Exception: + # Non-fatal: bytes already fetched from provider; return them without caching. + return file_bytes, None + + # Release pin after bytes have been served. + try: + await release_cache_entry(session, entry_id=pinned_entry_id, user_id=uid) + await session.commit() + except Exception: + pass # Non-fatal + + # Run eviction if needed — non-fatal, fire-and-forget in the same session. + try: + await evict_lru_entries(session, user_id=uid, max_cache_bytes=cache_limit_bytes) + await session.commit() + except Exception: + pass + + return file_bytes, cache_entry_id + + # ── Phase 12: in-memory folder listing cache ────────────────────────────────── diff --git a/backend/tests/test_cloud_cache.py b/backend/tests/test_cloud_cache.py index a60eb09..76a8392 100644 --- a/backend/tests/test_cloud_cache.py +++ b/backend/tests/test_cloud_cache.py @@ -749,6 +749,288 @@ async def test_pin_raises_on_foreign_entry(db_session): # ─── update_cache_access ───────────────────────────────────────────────────── +# ─── hydrate_and_cache_bytes (Plan 06) ─────────────────────────────────────── + +class _FakeMinIO: + """Minimal MinIO stub for hydrate_and_cache_bytes tests.""" + def __init__(self, bytes_to_return: bytes = b"fake-content"): + self._store: dict = {} + self._bytes_to_return = bytes_to_return + self.put_call_count = 0 + self.get_call_count = 0 + self.delete_call_count = 0 + + async def get_object(self, key: str) -> bytes: + self.get_call_count += 1 + if key in self._store: + return self._store[key] + raise Exception(f"Key not found: {key}") + + async def put_object(self, key: str, data: bytes, content_type: str = None) -> None: + self.put_call_count += 1 + self._store[key] = data + + async def delete_object(self, key: str) -> None: + self.delete_call_count += 1 + self._store.pop(key, None) + + +async def test_hydrate_cache_miss_fetches_from_provider(db_session): + """CACHE-03/Plan 06: Cache miss calls the provider fetch_fn and stores bytes in MinIO.""" + from services.cloud_cache import hydrate_and_cache_bytes, list_cache_entries + from unittest.mock import AsyncMock, patch + + user_id = uuid.uuid4() + conn_id = uuid.uuid4() + item_id = uuid.uuid4() + version_key = "etag-miss-v1" + expected_bytes = b"provider-content-hello" + fetch_call_count = 0 + + async def fetch_fn(): + nonlocal fetch_call_count + fetch_call_count += 1 + return expected_bytes + + minio = _FakeMinIO() + + # Patch increment_quota_for_cache to bypass quota check (no quota row in test DB) + with patch("services.cloud_cache.increment_quota_for_cache", new=AsyncMock()): + result_bytes, cache_entry_id = await hydrate_and_cache_bytes( + db_session, + user_id=user_id, + connection_id=conn_id, + cloud_item_id=item_id, + provider_item_id="pitem-miss", + version_key=version_key, + content_type="application/pdf", + fetch_fn=fetch_fn, + minio_client=minio, + cache_limit_bytes=512 * 1024 * 1024, + ) + + assert result_bytes == expected_bytes, "Returned bytes must match provider bytes" + assert fetch_call_count == 1, "Cache miss must call fetch_fn exactly once" + assert minio.put_call_count == 1, "Cache miss must store bytes in MinIO" + + # A cache entry must have been created for the user. + entries = await list_cache_entries(db_session, user_id=user_id) + assert len(entries) == 1, "A cache entry must be created on cache miss" + assert entries[0].version_key == version_key + + +async def test_hydrate_cache_hit_skips_provider(db_session): + """CACHE-03/Plan 06: Cache hit reads from MinIO and does NOT call fetch_fn (provider). + + The fetch_fn must not be called when a non-evicted entry exists for the version_key. + """ + from services.cloud_cache import ( + create_cache_entry, + hydrate_and_cache_bytes, + ) + + user_id = uuid.uuid4() + conn_id = uuid.uuid4() + item_id = uuid.uuid4() + version_key = "etag-hit-v1" + cached_bytes = b"minio-cached-bytes" + object_key = f"cache/{user_id}/{uuid.uuid4()}.pdf" + + # Pre-populate a cache entry + entry = await create_cache_entry( + session=db_session, + user_id=user_id, + connection_id=conn_id, + cloud_item_id=item_id, + provider_item_id="pitem-hit", + version_key=version_key, + object_key=object_key, + content_type="application/pdf", + size_bytes=len(cached_bytes), + ) + await db_session.commit() + + minio = _FakeMinIO() + minio._store[object_key] = cached_bytes # Pre-load the fake MinIO store + + fetch_call_count = 0 + + async def fetch_fn(): + nonlocal fetch_call_count + fetch_call_count += 1 + return b"should-not-be-called" + + result_bytes, cache_entry_id = await hydrate_and_cache_bytes( + db_session, + user_id=user_id, + connection_id=conn_id, + cloud_item_id=item_id, + provider_item_id="pitem-hit", + version_key=version_key, + content_type="application/pdf", + fetch_fn=fetch_fn, + minio_client=minio, + cache_limit_bytes=512 * 1024 * 1024, + ) + + assert result_bytes == cached_bytes, "Cache hit must return MinIO bytes, not provider bytes" + assert fetch_call_count == 0, "Cache hit must NOT call fetch_fn (provider avoided)" + assert minio.put_call_count == 0, "Cache hit must NOT write to MinIO again" + + +async def test_hydrate_pin_released_after_cache_hit(db_session): + """CACHE-04/Plan 06: Pin is released after hydrate_and_cache_bytes returns on cache hit.""" + from services.cloud_cache import ( + create_cache_entry, + hydrate_and_cache_bytes, + list_cache_entries, + ) + + user_id = uuid.uuid4() + conn_id = uuid.uuid4() + item_id = uuid.uuid4() + version_key = "etag-pin-release" + cached_bytes = b"pin-release-test-bytes" + object_key = f"cache/{user_id}/{uuid.uuid4()}.pdf" + + entry = await create_cache_entry( + session=db_session, + user_id=user_id, + connection_id=conn_id, + cloud_item_id=item_id, + provider_item_id="pitem-pin-rel", + version_key=version_key, + object_key=object_key, + content_type="application/pdf", + size_bytes=len(cached_bytes), + ) + await db_session.commit() + + minio = _FakeMinIO() + minio._store[object_key] = cached_bytes + + async def fetch_fn(): + return b"fallback" + + await hydrate_and_cache_bytes( + db_session, + user_id=user_id, + connection_id=conn_id, + cloud_item_id=item_id, + provider_item_id="pitem-pin-rel", + version_key=version_key, + content_type="application/pdf", + fetch_fn=fetch_fn, + minio_client=minio, + cache_limit_bytes=512 * 1024 * 1024, + ) + + # After hydration completes, pin_count must be back to 0. + await db_session.refresh(entry) + assert entry.pin_count == 0, ( + "Pin must be released after hydrate_and_cache_bytes returns (PATTERNS.md step 8)" + ) + + +async def test_hydrate_cache_miss_pin_released(db_session): + """CACHE-04/Plan 06: Pin is released after cache-miss hydration completes.""" + from services.cloud_cache import hydrate_and_cache_bytes, list_cache_entries + from unittest.mock import AsyncMock, patch + + user_id = uuid.uuid4() + conn_id = uuid.uuid4() + item_id = uuid.uuid4() + version_key = "etag-miss-pin-release" + expected_bytes = b"miss-pin-release-bytes" + + async def fetch_fn(): + return expected_bytes + + minio = _FakeMinIO() + + with patch("services.cloud_cache.increment_quota_for_cache", new=AsyncMock()): + await hydrate_and_cache_bytes( + db_session, + user_id=user_id, + connection_id=conn_id, + cloud_item_id=item_id, + provider_item_id="pitem-miss-pin", + version_key=version_key, + content_type="application/pdf", + fetch_fn=fetch_fn, + minio_client=minio, + cache_limit_bytes=512 * 1024 * 1024, + ) + + entries = await list_cache_entries(db_session, user_id=user_id) + assert all(e.pin_count == 0 for e in entries), ( + "All cache entries must have pin_count == 0 after hydration (no pin leak)" + ) + + +async def test_hydrate_foreign_user_gets_separate_cache_entry(db_session): + """T-14-06/Plan 06: hydrate_and_cache_bytes never reuses another user's cache entry.""" + from services.cloud_cache import ( + create_cache_entry, + hydrate_and_cache_bytes, + list_cache_entries, + ) + from unittest.mock import AsyncMock, patch + + owner_id = uuid.uuid4() + foreign_id = uuid.uuid4() + conn_id = uuid.uuid4() + item_id = uuid.uuid4() + version_key = "etag-shared-user-isolation" + cached_bytes = b"owner-only-bytes" + object_key = f"cache/{owner_id}/{uuid.uuid4()}.pdf" + + # Create owner's cache entry + await create_cache_entry( + session=db_session, + user_id=owner_id, + connection_id=conn_id, + cloud_item_id=item_id, + provider_item_id="pitem-owner-iso", + version_key=version_key, + object_key=object_key, + size_bytes=len(cached_bytes), + ) + await db_session.commit() + + minio = _FakeMinIO(bytes_to_return=b"foreign-bytes") + fetch_call_count = 0 + + async def fetch_fn(): + nonlocal fetch_call_count + fetch_call_count += 1 + return b"foreign-bytes" + + # Patch increment_quota_for_cache to bypass quota check (no quota row in test DB) + with patch("services.cloud_cache.increment_quota_for_cache", new=AsyncMock()): + await hydrate_and_cache_bytes( + db_session, + user_id=foreign_id, + connection_id=conn_id, + cloud_item_id=item_id, + provider_item_id="pitem-owner-iso", + version_key=version_key, + content_type="application/pdf", + fetch_fn=fetch_fn, + minio_client=minio, + cache_limit_bytes=512 * 1024 * 1024, + ) + + # Foreign user must have had a cache miss (fetched from provider) + assert fetch_call_count == 1, ( + "Foreign user must not reuse owner's cache entry (T-14-06)" + ) + # Foreign user now has their own entry + foreign_entries = await list_cache_entries(db_session, user_id=foreign_id) + assert len(foreign_entries) == 1 + assert str(foreign_entries[0].user_id) == str(foreign_id) + + async def test_update_cache_access_touches_last_accessed(db_session): """CACHE-04: update_cache_access refreshes last_accessed_at for LRU ordering.""" from services.cloud_cache import create_cache_entry, update_cache_access diff --git a/backend/tests/test_cloud_security.py b/backend/tests/test_cloud_security.py index a67a165..efd40fa 100644 --- a/backend/tests/test_cloud_security.py +++ b/backend/tests/test_cloud_security.py @@ -879,6 +879,96 @@ async def test_foreign_user_cannot_access_analysis_estimate(async_client, db_ses ) +# ── Phase 14 Plan 06: Cache-backed preview/download security ────────────────── + +async def test_preview_cache_hit_response_excludes_object_key(async_client, db_session): + """T-14-02/Plan 06: Preview response from a cache hit must not expose object_key. + + Even when bytes come from MinIO (cache hit path), the response body and headers + must not contain the MinIO object key or credentials_enc. + """ + import uuid as _uuid2 + from unittest.mock import AsyncMock, patch as _patch, MagicMock + + owner_ctx = await _create_user_and_token(db_session, role="user") + conn = await _create_cloud_connection(db_session, owner_ctx["user"].id) + + # Simulate a cache hit by patching hydrate_and_cache_bytes to return bytes + with _patch( + "api.cloud.operations.hydrate_and_cache_bytes" if False else "services.cloud_cache.hydrate_and_cache_bytes", + new=AsyncMock(return_value=(b"pdf-bytes", _uuid2.uuid4())), + ): + with _patch("api.cloud.operations._resolve_and_get_adapter") as mock_adapter: + mock_conn = MagicMock() + mock_ad = AsyncMock() + mock_ad.get_object = AsyncMock(return_value=b"pdf-bytes") + mock_adapter.return_value = (mock_conn, mock_ad) + + # No cloud_item in DB, so falls to direct fetch path — test response exclusion + resp = await async_client.get( + f"/api/cloud/connections/{conn.id}/items/pitem-preview-sec/preview", + headers=owner_ctx["headers"], + ) + + # Any 2xx or unsupported_preview is acceptable; key check is no secret leakage + assert resp.status_code not in (500,), f"Should not 500 — got {resp.status_code}: {resp.text}" + body = resp.text + assert "object_key" not in body, "T-14-02: object_key must not appear in preview response" + assert "credentials_enc" not in body, "credentials_enc must not appear in preview response" + + +async def test_download_response_excludes_object_key(async_client, db_session): + """T-14-02/Plan 06: Download response must not expose object_key or credentials. + + The cache lookup and MinIO path are internal; the response headers and body + must contain only the file bytes and safe metadata. + """ + from unittest.mock import AsyncMock, patch as _patch, MagicMock + + owner_ctx = await _create_user_and_token(db_session, role="user") + conn = await _create_cloud_connection(db_session, owner_ctx["user"].id) + + with _patch("api.cloud.operations._resolve_and_get_adapter") as mock_adapter: + mock_conn = MagicMock() + mock_ad = AsyncMock() + mock_ad.get_object = AsyncMock(return_value=b"download-bytes") + mock_adapter.return_value = (mock_conn, mock_ad) + + resp = await async_client.get( + f"/api/cloud/connections/{conn.id}/items/pitem-download-sec/download", + headers=owner_ctx["headers"], + ) + + assert resp.status_code != 500, f"Should not 500 — got {resp.status_code}: {resp.text}" + # Check headers + for header_name, header_value in resp.headers.items(): + assert "object_key" not in header_value.lower(), ( + f"T-14-02: object_key must not appear in response headers — found in {header_name}" + ) + assert "credentials_enc" not in header_value.lower(), ( + "credentials_enc must not appear in response headers" + ) + + +async def test_foreign_user_cannot_download_via_cache(async_client, db_session): + """T-14-01/Plan 06: Foreign user cannot download another user's cloud file via cache. + + The ownership check (resolve_owned_connection) must run before any cache lookup. + """ + auth1 = await _create_user_and_token(db_session, role="user") + auth2 = await _create_user_and_token(db_session, role="user") + conn = await _create_cloud_connection(db_session, auth1["user"].id) + + resp = await async_client.get( + f"/api/cloud/connections/{conn.id}/items/pitem-idor/download", + headers=auth2["headers"], + ) + + assert resp.status_code == 404, ( + f"Foreign user must not download via cache — expected 404, got {resp.status_code}" + ) + + async def test_foreign_user_cannot_read_analysis_job_status(async_client, db_session): """T-14-01: Foreign user cannot read job status for another user's job.""" from unittest.mock import patch as _patch