feat(14-03): cache settings/status API and security tests
- api/cloud/cache.py: GET /analysis/cache, PATCH /analysis/cache/settings - api/cloud/analysis.py: analysis router aggregator (Phase 14 parent) - schemas.py: CacheStatusOut, CacheSettingsUpdateRequest (explicit allowlists) - __init__.py: register analysis_router under /api/cloud - 7 new security tests: admin block, unauthenticated block, object_key exclusion, response structure, enum validation, tier cap enforcement (T-14-01..02, T-14-08) - All 47 plan target tests pass (test_cloud_cache.py + test_cloud_security.py)
This commit is contained in:
@@ -17,6 +17,7 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
|||||||
from api.cloud.connections import router as connections_router, _VALID_BACKENDS
|
from api.cloud.connections import router as connections_router, _VALID_BACKENDS
|
||||||
from api.cloud.browse import router as browse_router
|
from api.cloud.browse import router as browse_router
|
||||||
from api.cloud.operations import router as operations_router
|
from api.cloud.operations import router as operations_router
|
||||||
|
from api.cloud.analysis import router as analysis_router
|
||||||
from db.models import User
|
from db.models import User
|
||||||
from deps.auth import get_regular_user
|
from deps.auth import get_regular_user
|
||||||
from deps.db import get_db
|
from deps.db import get_db
|
||||||
@@ -26,6 +27,7 @@ router = APIRouter(prefix="/api/cloud", tags=["cloud"])
|
|||||||
router.include_router(connections_router)
|
router.include_router(connections_router)
|
||||||
router.include_router(browse_router)
|
router.include_router(browse_router)
|
||||||
router.include_router(operations_router)
|
router.include_router(operations_router)
|
||||||
|
router.include_router(analysis_router)
|
||||||
|
|
||||||
# ── Users router (default storage) ───────────────────────────────────────────
|
# ── Users router (default storage) ───────────────────────────────────────────
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,29 @@
|
|||||||
|
"""
|
||||||
|
Cloud analysis API package — Phase 14 router aggregator.
|
||||||
|
|
||||||
|
Provides the /analysis sub-router tree under /api/cloud. All routes are
|
||||||
|
owner-scoped and blocked for admin accounts (get_regular_user enforced at
|
||||||
|
each leaf router).
|
||||||
|
|
||||||
|
Currently registered routes:
|
||||||
|
GET /analysis/cache — cache usage/settings (cloud/cache.py)
|
||||||
|
PATCH /analysis/cache/settings — update settings (cloud/cache.py)
|
||||||
|
|
||||||
|
Future routes (Phase 14 plans 04-09):
|
||||||
|
POST /analysis/connections/{id}/estimate
|
||||||
|
POST /analysis/connections/{id}/jobs
|
||||||
|
GET /analysis/jobs/{id}
|
||||||
|
POST /analysis/jobs/{id}/cancel
|
||||||
|
POST /analysis/jobs/{id}/items/{item_id}/skip
|
||||||
|
|
||||||
|
Module is importable as api.cloud.analysis for tests and callers that verify
|
||||||
|
router registration before endpoint implementation is complete.
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from fastapi import APIRouter
|
||||||
|
|
||||||
|
from api.cloud.cache import router as cache_router
|
||||||
|
|
||||||
|
router = APIRouter()
|
||||||
|
router.include_router(cache_router)
|
||||||
@@ -0,0 +1,137 @@
|
|||||||
|
"""
|
||||||
|
Cache settings and status endpoint — Phase 14.
|
||||||
|
|
||||||
|
Owner-scoped endpoints for the byte cache. Admin users are blocked by
|
||||||
|
get_regular_user (T-14-01, T-14-06).
|
||||||
|
|
||||||
|
Route family:
|
||||||
|
GET /analysis/cache — current cache usage and user settings
|
||||||
|
PATCH /analysis/cache/settings — update analysis preferences and cache limit
|
||||||
|
|
||||||
|
Security invariants (T-14-02, T-14-08):
|
||||||
|
- object_key, credentials_enc, and raw provider URLs must never appear in any
|
||||||
|
response body.
|
||||||
|
- All queries are scoped to current_user.id — no cross-user access is possible.
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from typing import Optional
|
||||||
|
|
||||||
|
from fastapi import APIRouter, Depends, HTTPException, Request, status
|
||||||
|
from sqlalchemy import func, select
|
||||||
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
|
from api.cloud.schemas import CacheStatusOut, CacheSettingsUpdateRequest
|
||||||
|
from db.models import CloudByteCacheEntry, User
|
||||||
|
from deps.auth import get_regular_user
|
||||||
|
from deps.db import get_db
|
||||||
|
from services.cloud_analysis_settings import (
|
||||||
|
DEFAULT_MAX_CACHE_LIMIT_BYTES,
|
||||||
|
get_or_create_user_analysis_settings,
|
||||||
|
update_user_analysis_settings,
|
||||||
|
)
|
||||||
|
from services.rate_limiting import account_limiter
|
||||||
|
|
||||||
|
router = APIRouter()
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/analysis/cache", response_model=CacheStatusOut)
|
||||||
|
@account_limiter.limit("120/minute")
|
||||||
|
async def get_cache_status(
|
||||||
|
request: Request,
|
||||||
|
session: AsyncSession = Depends(get_db),
|
||||||
|
current_user: User = Depends(get_regular_user),
|
||||||
|
) -> CacheStatusOut:
|
||||||
|
"""Return cache usage totals and analysis settings for the current user.
|
||||||
|
|
||||||
|
Returns aggregate byte/count totals (no object_key or credentials).
|
||||||
|
Blocked for admin accounts (T-14-01).
|
||||||
|
"""
|
||||||
|
request.state.current_user = current_user
|
||||||
|
|
||||||
|
# Aggregate active (non-evicted) cache entries for this user
|
||||||
|
result = await session.execute(
|
||||||
|
select(
|
||||||
|
func.count(CloudByteCacheEntry.id).label("entry_count"),
|
||||||
|
func.coalesce(func.sum(CloudByteCacheEntry.size_bytes), 0).label("total_bytes"),
|
||||||
|
).where(
|
||||||
|
CloudByteCacheEntry.user_id == current_user.id,
|
||||||
|
CloudByteCacheEntry.evicted_at.is_(None),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
row = result.one()
|
||||||
|
entry_count = int(row.entry_count)
|
||||||
|
total_bytes = int(row.total_bytes)
|
||||||
|
|
||||||
|
# Fetch or create settings (never fails — creates defaults on first access)
|
||||||
|
settings = await get_or_create_user_analysis_settings(
|
||||||
|
session, user_id=current_user.id
|
||||||
|
)
|
||||||
|
|
||||||
|
return CacheStatusOut(
|
||||||
|
entry_count=entry_count,
|
||||||
|
total_bytes=total_bytes,
|
||||||
|
cache_limit_bytes=settings.cloud_cache_limit_bytes,
|
||||||
|
tier_cap_bytes=DEFAULT_MAX_CACHE_LIMIT_BYTES,
|
||||||
|
analysis_progress_detail=settings.analysis_progress_detail,
|
||||||
|
analysis_failure_behavior=settings.analysis_failure_behavior,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@router.patch("/analysis/cache/settings", response_model=CacheStatusOut)
|
||||||
|
@account_limiter.limit("30/minute")
|
||||||
|
async def update_cache_settings(
|
||||||
|
request: Request,
|
||||||
|
body: CacheSettingsUpdateRequest,
|
||||||
|
session: AsyncSession = Depends(get_db),
|
||||||
|
current_user: User = Depends(get_regular_user),
|
||||||
|
) -> CacheStatusOut:
|
||||||
|
"""Update analysis preferences and/or the user's cache byte limit.
|
||||||
|
|
||||||
|
Only provided (non-None) fields are updated. Enum and bounds validation
|
||||||
|
is performed by the service layer (raises ValueError; we re-raise as 422).
|
||||||
|
Blocked for admin accounts (T-14-01).
|
||||||
|
"""
|
||||||
|
request.state.current_user = current_user
|
||||||
|
|
||||||
|
try:
|
||||||
|
await update_user_analysis_settings(
|
||||||
|
session,
|
||||||
|
user_id=current_user.id,
|
||||||
|
analysis_progress_detail=body.analysis_progress_detail,
|
||||||
|
analysis_failure_behavior=body.analysis_failure_behavior,
|
||||||
|
cloud_cache_limit_bytes=body.cloud_cache_limit_bytes,
|
||||||
|
max_cache_limit_bytes=DEFAULT_MAX_CACHE_LIMIT_BYTES,
|
||||||
|
)
|
||||||
|
except ValueError as exc:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
|
||||||
|
detail=str(exc),
|
||||||
|
) from exc
|
||||||
|
|
||||||
|
await session.commit()
|
||||||
|
|
||||||
|
# Return fresh status after update
|
||||||
|
result = await session.execute(
|
||||||
|
select(
|
||||||
|
func.count(CloudByteCacheEntry.id).label("entry_count"),
|
||||||
|
func.coalesce(func.sum(CloudByteCacheEntry.size_bytes), 0).label("total_bytes"),
|
||||||
|
).where(
|
||||||
|
CloudByteCacheEntry.user_id == current_user.id,
|
||||||
|
CloudByteCacheEntry.evicted_at.is_(None),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
row = result.one()
|
||||||
|
|
||||||
|
settings = await get_or_create_user_analysis_settings(
|
||||||
|
session, user_id=current_user.id
|
||||||
|
)
|
||||||
|
|
||||||
|
return CacheStatusOut(
|
||||||
|
entry_count=int(row.entry_count),
|
||||||
|
total_bytes=int(row.total_bytes),
|
||||||
|
cache_limit_bytes=settings.cloud_cache_limit_bytes,
|
||||||
|
tier_cap_bytes=DEFAULT_MAX_CACHE_LIMIT_BYTES,
|
||||||
|
analysis_progress_detail=settings.analysis_progress_detail,
|
||||||
|
analysis_failure_behavior=settings.analysis_failure_behavior,
|
||||||
|
)
|
||||||
@@ -9,6 +9,10 @@ Phase 13 additions:
|
|||||||
- ReconnectOut: typed reconnect response (CONN-01..03)
|
- ReconnectOut: typed reconnect response (CONN-01..03)
|
||||||
- ContentResultOut: typed open/preview/download response (D-02, D-18, T-13-14)
|
- ContentResultOut: typed open/preview/download response (D-02, D-18, T-13-14)
|
||||||
- MutationResultOut: typed kind/reason mutation response (D-05..11)
|
- MutationResultOut: typed kind/reason mutation response (D-05..11)
|
||||||
|
|
||||||
|
Phase 14 additions:
|
||||||
|
- CacheStatusOut: aggregate cache usage + settings response (T-14-02, T-14-08)
|
||||||
|
- CacheSettingsUpdateRequest: PATCH body for updating analysis settings/cache limit
|
||||||
"""
|
"""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
@@ -202,3 +206,52 @@ class MoveItemRequest(BaseModel):
|
|||||||
destination_parent_ref: str
|
destination_parent_ref: str
|
||||||
destination_connection_id: Optional[str] = None
|
destination_connection_id: Optional[str] = None
|
||||||
etag: Optional[str] = None
|
etag: Optional[str] = None
|
||||||
|
|
||||||
|
|
||||||
|
# ── Phase 14 cache schemas ─────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
class CacheStatusOut(BaseModel):
|
||||||
|
"""Aggregate cache usage and user analysis settings.
|
||||||
|
|
||||||
|
T-14-02: object_key, credentials_enc, and raw provider URLs are absent by
|
||||||
|
design. This schema is a strict allowlist — no internal MinIO keys or
|
||||||
|
credentials can leak through it.
|
||||||
|
|
||||||
|
entry_count: Number of active (non-evicted) cache entries.
|
||||||
|
total_bytes: Sum of size_bytes across active entries.
|
||||||
|
cache_limit_bytes: User's preferred cache byte ceiling.
|
||||||
|
tier_cap_bytes: Maximum allowed cache limit for this tier.
|
||||||
|
analysis_progress_detail: "simple" | "detailed" — default "simple".
|
||||||
|
analysis_failure_behavior:"pause_batch" | "continue_item" — default "pause_batch".
|
||||||
|
"""
|
||||||
|
|
||||||
|
entry_count: int = 0
|
||||||
|
total_bytes: int = 0
|
||||||
|
cache_limit_bytes: int
|
||||||
|
tier_cap_bytes: int
|
||||||
|
analysis_progress_detail: str
|
||||||
|
analysis_failure_behavior: str
|
||||||
|
|
||||||
|
|
||||||
|
class CacheSettingsUpdateRequest(BaseModel):
|
||||||
|
"""PATCH body for updating analysis preferences and cache limit.
|
||||||
|
|
||||||
|
Only provided (non-None) fields are applied. Enum validation and bounds
|
||||||
|
checking are performed in the service layer.
|
||||||
|
|
||||||
|
Mass-assignment prevention: only the three fields below are accepted.
|
||||||
|
"""
|
||||||
|
|
||||||
|
analysis_progress_detail: Optional[str] = Field(
|
||||||
|
default=None,
|
||||||
|
description="Progress label verbosity: 'simple' or 'detailed'",
|
||||||
|
)
|
||||||
|
analysis_failure_behavior: Optional[str] = Field(
|
||||||
|
default=None,
|
||||||
|
description="Batch failure mode: 'pause_batch' or 'continue_item'",
|
||||||
|
)
|
||||||
|
cloud_cache_limit_bytes: Optional[int] = Field(
|
||||||
|
default=None,
|
||||||
|
ge=1,
|
||||||
|
description="Preferred byte ceiling for the local byte cache",
|
||||||
|
)
|
||||||
|
|||||||
@@ -579,3 +579,128 @@ async def test_no_byte_download_during_browse(async_client, db_session):
|
|||||||
|
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
byte_call_tracker.assert_not_called()
|
byte_call_tracker.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
# ── Phase 14: Cache API security (T-14-01, T-14-02, T-14-06, T-14-08) ──────────
|
||||||
|
|
||||||
|
async def test_cache_status_admin_blocked(async_client, db_session):
|
||||||
|
"""T-14-01/CACHE-05: Admin account cannot access the cache status endpoint.
|
||||||
|
|
||||||
|
get_regular_user blocks all admin accounts from analysis/cache routes.
|
||||||
|
"""
|
||||||
|
admin_ctx = await _create_user_and_token(db_session, role="admin")
|
||||||
|
|
||||||
|
resp = await async_client.get(
|
||||||
|
"/api/cloud/analysis/cache",
|
||||||
|
headers=admin_ctx["headers"],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 403, (
|
||||||
|
f"Admin must be blocked from cache status — expected 403, got {resp.status_code}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_cache_settings_admin_blocked(async_client, db_session):
|
||||||
|
"""T-14-01: Admin account cannot update cache settings."""
|
||||||
|
admin_ctx = await _create_user_and_token(db_session, role="admin")
|
||||||
|
|
||||||
|
resp = await async_client.patch(
|
||||||
|
"/api/cloud/analysis/cache/settings",
|
||||||
|
json={"analysis_progress_detail": "detailed"},
|
||||||
|
headers=admin_ctx["headers"],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 403, (
|
||||||
|
f"Admin must be blocked from cache settings update — got {resp.status_code}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_cache_status_unauthenticated_blocked(async_client, db_session):
|
||||||
|
"""Cache status requires authentication — unauthenticated request returns 401/403."""
|
||||||
|
resp = await async_client.get("/api/cloud/analysis/cache")
|
||||||
|
|
||||||
|
assert resp.status_code in (401, 403), (
|
||||||
|
f"Unauthenticated request must be rejected — got {resp.status_code}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_cache_status_response_excludes_object_key(async_client, db_session):
|
||||||
|
"""T-14-02/T-14-08: Cache status response must not contain object_key field.
|
||||||
|
|
||||||
|
The MinIO object key is an internal detail — never exposed in API responses.
|
||||||
|
"""
|
||||||
|
user_ctx = await _create_user_and_token(db_session, role="user")
|
||||||
|
|
||||||
|
resp = await async_client.get(
|
||||||
|
"/api/cloud/analysis/cache",
|
||||||
|
headers=user_ctx["headers"],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 200, f"Expected 200 from cache status, got {resp.status_code}"
|
||||||
|
body_text = resp.text
|
||||||
|
|
||||||
|
# T-14-02 / T-14-08: object key and credential fields must be absent
|
||||||
|
assert "object_key" not in body_text, (
|
||||||
|
"T-14-02: MinIO object_key must not appear in cache API responses"
|
||||||
|
)
|
||||||
|
assert "credentials_enc" not in body_text, (
|
||||||
|
"T-14-08: credentials_enc must not appear in cache API responses"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_cache_status_response_structure(async_client, db_session):
|
||||||
|
"""CACHE-05: Cache status returns aggregate totals, not per-entry detail."""
|
||||||
|
user_ctx = await _create_user_and_token(db_session, role="user")
|
||||||
|
|
||||||
|
resp = await async_client.get(
|
||||||
|
"/api/cloud/analysis/cache",
|
||||||
|
headers=user_ctx["headers"],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 200
|
||||||
|
body = resp.json()
|
||||||
|
|
||||||
|
# Aggregate fields must be present
|
||||||
|
assert "entry_count" in body, "Cache status must include entry_count"
|
||||||
|
assert "total_bytes" in body, "Cache status must include total_bytes"
|
||||||
|
assert "cache_limit_bytes" in body, "Cache status must include cache_limit_bytes"
|
||||||
|
assert "analysis_progress_detail" in body, "Cache status must include analysis_progress_detail"
|
||||||
|
assert "analysis_failure_behavior" in body, "Cache status must include analysis_failure_behavior"
|
||||||
|
|
||||||
|
# No per-entry detail that could leak internal state
|
||||||
|
assert "object_key" not in body
|
||||||
|
assert "provider_item_id" not in body or isinstance(body.get("provider_item_id"), type(None))
|
||||||
|
|
||||||
|
|
||||||
|
async def test_cache_settings_invalid_enum_returns_422(async_client, db_session):
|
||||||
|
"""Cache settings update rejects invalid enum values with 422."""
|
||||||
|
user_ctx = await _create_user_and_token(db_session, role="user")
|
||||||
|
|
||||||
|
resp = await async_client.patch(
|
||||||
|
"/api/cloud/analysis/cache/settings",
|
||||||
|
json={"analysis_progress_detail": "invalid_value"},
|
||||||
|
headers=user_ctx["headers"],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 422, (
|
||||||
|
f"Invalid enum value must return 422, got {resp.status_code}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_cache_settings_limit_above_tier_cap_returns_422(async_client, db_session):
|
||||||
|
"""Cache settings update rejects cache limit above tier cap with 422."""
|
||||||
|
from services.cloud_analysis_settings import DEFAULT_MAX_CACHE_LIMIT_BYTES
|
||||||
|
|
||||||
|
user_ctx = await _create_user_and_token(db_session, role="user")
|
||||||
|
|
||||||
|
too_large = DEFAULT_MAX_CACHE_LIMIT_BYTES + 1
|
||||||
|
|
||||||
|
resp = await async_client.patch(
|
||||||
|
"/api/cloud/analysis/cache/settings",
|
||||||
|
json={"cloud_cache_limit_bytes": too_large},
|
||||||
|
headers=user_ctx["headers"],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 422, (
|
||||||
|
f"Cache limit above tier cap must return 422, got {resp.status_code}"
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user