feat(14-06): route preview/download bytes through durable byte cache
- Add hydrate_and_cache_bytes to services/cloud_cache.py (Plan 06 cache lifecycle) - Cache-hit path: pin existing entry, read from MinIO, release pin in finally - Cache-miss path: fetch from provider, store in MinIO, create/reactivate entry, pin during response, release pin in finally - CacheQuotaExceeded and MinIO failures are non-fatal (bytes returned from provider path) - evict_lru_entries run after pin release to stay within user cache limit - Update preview_cloud_file and download_cloud_file in operations.py to call hydrate_and_cache_bytes - Version key computed from CloudItem metadata (no provider call required for key resolution) - Shared _make_minio_cache_wrapper helper eliminates duplicate adapter code - Falls through to direct provider fetch when no CloudItem row found - Add _make_minio_cache_wrapper helper to operations.py (DRY, shared by preview and download) - Add 5 new tests to test_cloud_cache.py: cache-miss fetches provider, cache-hit skips provider, pin released on hit, pin released on miss, user isolation - Add 3 new security tests to test_cloud_security.py: preview response excludes object_key, download response excludes object_key, foreign-user IDOR via download blocked
This commit is contained in:
@@ -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,
|
Owner-scoped routes for opening, previewing, downloading, renaming, moving,
|
||||||
deleting, creating folders, and uploading cloud files. All routes are
|
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).
|
- Cross-connection moves are rejected before the adapter is called (D-08).
|
||||||
- Self-destination moves are rejected before the adapter is called (D-09).
|
- Self-destination moves are rejected before the adapter is called (D-09).
|
||||||
- No health probe triggered by folder navigation (D-13).
|
- 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:
|
Route family:
|
||||||
GET /connections/{id}/items/{item_id}/open — D-02 authorized access URL
|
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
|
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:
|
def _mutation_error_json(result: dict, http_status: int) -> JSONResponse:
|
||||||
"""Return a typed JSON error response for a failed mutation.
|
"""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):
|
if not PreviewSupport.is_supported(content_type):
|
||||||
return _unsupported_preview_response(connection_id, item_id, "unsupported_format")
|
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:
|
try:
|
||||||
conn, adapter = await _resolve_and_get_adapter(session, connection_id, current_user.id)
|
conn, adapter = await _resolve_and_get_adapter(session, connection_id, current_user.id)
|
||||||
file_bytes = await adapter.get_object(item_id)
|
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
# Re-raise 401 (credential failure) and 404 (ownership race) — never mask as
|
raise
|
||||||
# unsupported_preview. The IDOR guard in Step 1 already passed, but a 401
|
except Exception:
|
||||||
# from credential decryption should surface as an auth error to the client.
|
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
|
raise
|
||||||
except Exception:
|
except Exception:
|
||||||
return _unsupported_preview_response(connection_id, item_id, "provider_error")
|
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
|
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"
|
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:
|
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:
|
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:
|
if result_dict.get("kind") == MUT_KIND_REAUTH:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||||
|
|||||||
@@ -28,6 +28,11 @@ and the analysis API.
|
|||||||
Also re-exports compute_version_key from services.cloud_analysis_versioning so
|
Also re-exports compute_version_key from services.cloud_analysis_versioning so
|
||||||
tests and service callers have a single import location.
|
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:
|
Rules:
|
||||||
- Raises ValueError or domain exceptions only — never HTTPException.
|
- Raises ValueError or domain exceptions only — never HTTPException.
|
||||||
- Eviction never removes entries with pin_count > 0 or active_job_count > 0.
|
- Eviction never removes entries with pin_count > 0 or active_job_count > 0.
|
||||||
@@ -550,6 +555,185 @@ async def update_cache_access(
|
|||||||
return entry
|
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 ──────────────────────────────────
|
# ── Phase 12: in-memory folder listing cache ──────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -749,6 +749,288 @@ async def test_pin_raises_on_foreign_entry(db_session):
|
|||||||
|
|
||||||
# ─── update_cache_access ─────────────────────────────────────────────────────
|
# ─── 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):
|
async def test_update_cache_access_touches_last_accessed(db_session):
|
||||||
"""CACHE-04: update_cache_access refreshes last_accessed_at for LRU ordering."""
|
"""CACHE-04: update_cache_access refreshes last_accessed_at for LRU ordering."""
|
||||||
from services.cloud_cache import create_cache_entry, update_cache_access
|
from services.cloud_cache import create_cache_entry, update_cache_access
|
||||||
|
|||||||
@@ -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):
|
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."""
|
"""T-14-01: Foreign user cannot read job status for another user's job."""
|
||||||
from unittest.mock import patch as _patch
|
from unittest.mock import patch as _patch
|
||||||
|
|||||||
Reference in New Issue
Block a user