fix(vector-sync): address round-3 review — gate purge route, bound set, nits
- app.py: register /api/v1/vector-sync/purge only when vector_sync_enabled, so it returns 404 (not a 500 from get_qdrant_client) when sync is off - scanner: bound _consent_backstop_done so a long-running multi-tenant process with user churn can't grow it without limit (clears on overflow) - purge route: distinct 400 for a missing doc_types key; enforce the admin check even for an empty no-op request (destructive route) - tests: missing-key 400, admin-gated empty no-op, non-admin empty 403 The _consent_narrowed_doc_types precondition is enforced by its non-Optional frozenset[str] signature (ty rejects a None caller). The httpx.BasicAuth SonarCloud hotspot matches the existing webhook routes (false positive, credential from the app-password store) — left consistent for UI triage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
24b8000a71
commit
cef477b877
@@ -77,6 +77,11 @@ async def purge_doc_types_route(request: Request) -> JSONResponse:
|
||||
)
|
||||
|
||||
raw = body.get("doc_types")
|
||||
if raw is None:
|
||||
return JSONResponse(
|
||||
{"error": "Bad request", "message": "doc_types is required"},
|
||||
status_code=400,
|
||||
)
|
||||
if not isinstance(raw, list) or not all(isinstance(d, str) for d in raw):
|
||||
return JSONResponse(
|
||||
{
|
||||
@@ -86,8 +91,6 @@ async def purge_doc_types_route(request: Request) -> JSONResponse:
|
||||
status_code=400,
|
||||
)
|
||||
doc_types = [d for d in raw if d]
|
||||
if not doc_types:
|
||||
return JSONResponse({"purged": {}})
|
||||
# Bound the batch: there are only a handful of real indexed types, so a huge
|
||||
# list is abuse — cap it rather than fan out unbounded count+delete calls.
|
||||
if len(doc_types) > _MAX_PURGE_DOC_TYPES:
|
||||
@@ -107,7 +110,9 @@ async def purge_doc_types_route(request: Request) -> JSONResponse:
|
||||
if not nextcloud_host:
|
||||
raise ValueError("Nextcloud host not configured")
|
||||
|
||||
# Verify admin via the caller's own app password before any deletion.
|
||||
# Verify admin via the caller's own app password before any deletion —
|
||||
# enforced even for an empty (no-op) request, since this is a
|
||||
# destructive admin route.
|
||||
async with nextcloud_httpx_client(
|
||||
base_url=nextcloud_host,
|
||||
auth=httpx.BasicAuth(username, app_password),
|
||||
@@ -125,6 +130,9 @@ async def purge_doc_types_route(request: Request) -> JSONResponse:
|
||||
status_code=403,
|
||||
)
|
||||
|
||||
if not doc_types:
|
||||
return JSONResponse({"purged": {}})
|
||||
|
||||
purged = await purge_doc_types(doc_types)
|
||||
logger.info("Vector-sync purge by admin %s: %s", user_id, purged)
|
||||
return JSONResponse({"purged": purged})
|
||||
|
||||
@@ -2424,14 +2424,17 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None =
|
||||
Route("/api/v1/webhooks/{webhook_id}", delete_webhook, methods=["DELETE"])
|
||||
)
|
||||
# Vector-sync admin: purge indexed vectors by doc type (admin consent —
|
||||
# called by Astrolabe when a source is disabled for semantic search)
|
||||
routes.append(
|
||||
Route(
|
||||
"/api/v1/vector-sync/purge",
|
||||
purge_doc_types_route,
|
||||
methods=["POST"],
|
||||
# called by Astrolabe when a source is disabled for semantic search).
|
||||
# Gated on vector_sync_enabled: without it there is no Qdrant client, so
|
||||
# the purge would 500 rather than no-op.
|
||||
if settings.vector_sync_enabled:
|
||||
routes.append(
|
||||
Route(
|
||||
"/api/v1/vector-sync/purge",
|
||||
purge_doc_types_route,
|
||||
methods=["POST"],
|
||||
)
|
||||
)
|
||||
)
|
||||
# Access and scope management endpoints (ADR-022)
|
||||
routes.append(
|
||||
Route(
|
||||
|
||||
@@ -293,6 +293,13 @@ _TEXT_BACKSTOP_DOC_TYPES: tuple[str, ...] = tuple(sorted(INDEXED_DOC_TYPES - {"f
|
||||
# the type is allowed again, so a later re-disable re-triggers the backstop.
|
||||
_consent_backstop_done: set[tuple[str, str]] = set()
|
||||
|
||||
# Safety bound on the tracking set so a long-running multi-tenant process with
|
||||
# heavy user churn (deprovisioned users leave stale entries) can't grow it
|
||||
# without limit. At <= len(INDEXED_DOC_TYPES) entries per user this is generous;
|
||||
# on overflow we clear the whole set, which at worst re-fires the (idempotent)
|
||||
# backstop once for currently-disabled types.
|
||||
_CONSENT_BACKSTOP_MAX = 50_000
|
||||
|
||||
|
||||
async def _enqueue_deletes_for_disabled_types(
|
||||
user_id: str,
|
||||
@@ -366,6 +373,12 @@ async def _enqueue_deletes_for_disabled_types(
|
||||
queued += 1
|
||||
# Mark this (user, doc_type) backstopped for the current disable episode
|
||||
# so subsequent scans don't re-enqueue the same idempotent deletes.
|
||||
if len(_consent_backstop_done) >= _CONSENT_BACKSTOP_MAX:
|
||||
logger.info(
|
||||
"consent backstop tracking set hit %d entries; clearing",
|
||||
_CONSENT_BACKSTOP_MAX,
|
||||
)
|
||||
_consent_backstop_done.clear()
|
||||
_consent_backstop_done.add((user_id, doc_type))
|
||||
return queued
|
||||
|
||||
|
||||
@@ -112,10 +112,25 @@ def test_forbidden_when_not_admin(mocker):
|
||||
purge.assert_not_called()
|
||||
|
||||
|
||||
def test_empty_doc_types_is_noop(mocker):
|
||||
def test_missing_doc_types_key_returns_400(mocker):
|
||||
_patch_token(mocker)
|
||||
purge = _patch_purge(mocker)
|
||||
|
||||
client = TestClient(_build_app())
|
||||
resp = client.post("/api/v1/vector-sync/purge", json={})
|
||||
|
||||
assert resp.status_code == 400
|
||||
purge.assert_not_called()
|
||||
|
||||
|
||||
def test_empty_doc_types_is_admin_gated_noop(mocker):
|
||||
# An empty (no-op) request still requires admin — this is a destructive route.
|
||||
_patch_token(mocker, "admin")
|
||||
_patch_basic_auth(mocker, "admin")
|
||||
_patch_outbound_client(mocker)
|
||||
_patch_groups(mocker, ["admin"])
|
||||
purge = _patch_purge(mocker)
|
||||
|
||||
client = TestClient(_build_app())
|
||||
resp = client.post("/api/v1/vector-sync/purge", json={"doc_types": []})
|
||||
|
||||
@@ -124,6 +139,20 @@ def test_empty_doc_types_is_noop(mocker):
|
||||
purge.assert_not_called()
|
||||
|
||||
|
||||
def test_empty_doc_types_forbidden_for_non_admin(mocker):
|
||||
_patch_token(mocker, "bob")
|
||||
_patch_basic_auth(mocker, "bob")
|
||||
_patch_outbound_client(mocker)
|
||||
_patch_groups(mocker, ["users"])
|
||||
purge = _patch_purge(mocker)
|
||||
|
||||
client = TestClient(_build_app())
|
||||
resp = client.post("/api/v1/vector-sync/purge", json={"doc_types": []})
|
||||
|
||||
assert resp.status_code == 403
|
||||
purge.assert_not_called()
|
||||
|
||||
|
||||
def test_admin_purge_happy_path(mocker):
|
||||
_patch_token(mocker, "admin")
|
||||
_patch_basic_auth(mocker, "admin")
|
||||
|
||||
Reference in New Issue
Block a user