From c39ec1e3f26928520e061d1158dd68b10b2db7c8 Mon Sep 17 00:00:00 2001 From: Floom Date: Fri, 26 Jun 2026 00:02:57 +0200 Subject: [PATCH] fix(api): resolve legacy approval batch shares --- apps/api/routers/approvals.py | 76 ++++++++++-- apps/api/services/public_worker.py | 4 +- .../tests/test_approvals_batch_share_link.py | 114 +++++++++++++----- 3 files changed, 149 insertions(+), 45 deletions(-) diff --git a/apps/api/routers/approvals.py b/apps/api/routers/approvals.py index 228464fb1..060b63df8 100644 --- a/apps/api/routers/approvals.py +++ b/apps/api/routers/approvals.py @@ -65,6 +65,9 @@ from services.worker_access import _delete_worker_impl from services.product_events import emit_approval_decided from services.share_links import ( + _ensure_standalone_share_links_table, + _load_standalone_share_row, + _revoke_standalone_share_link, _standalone_share_url, ) @@ -150,10 +153,45 @@ def _revoke_approvals_batch_share_link(*, repos: Repositories, token: str, owner revoked_at=now_iso(), ) if not revoked: - raise HTTPException(status_code=404, detail="Approval batch link not found") + legacy_row = _load_standalone_share_row(token) + if ( + not legacy_row + or str(legacy_row.get("entity_type") or "") != "approvals_batch" + or str(legacy_row.get("owner_id") or "") != owner_id + ): + raise HTTPException(status_code=404, detail="Approval batch link not found") + _coerce_approvals_batch_share_row(legacy_row) + legacy_revoked = _revoke_standalone_share_link( + entity_type="approvals_batch", + entity_id=str(legacy_row.get("entity_id") or ""), + owner_id=owner_id, + ) + if not legacy_revoked.get("revoked"): + raise HTTPException(status_code=404, detail="Approval batch link not found") return {"status": "revoked", "entity_type": "approvals_batch"} +def _revoke_legacy_approvals_batch_standalone_links_for_workspace( + *, + workspace_id: str, + owner_id: str, +) -> int: + _ensure_standalone_share_links_table() + from db import get_db + + with get_db() as conn: + cursor = conn.execute( + """ + DELETE FROM standalone_share_links + WHERE entity_type = 'approvals_batch' + AND entity_id = ? + AND owner_id = ? + """, + (workspace_id, owner_id), + ) + return int(cursor.rowcount or 0) + + def _revoke_all_approvals_batch_share_links_for_workspace( *, repos: Repositories, @@ -173,6 +211,10 @@ def _revoke_all_approvals_batch_share_links_for_workspace( owner_id=owner_id, revoked_at=now_iso(), ) + revoked += _revoke_legacy_approvals_batch_standalone_links_for_workspace( + workspace_id=workspace_id, + owner_id=owner_id, + ) return {"status": "revoked", "entity_type": "approvals_batch", "revoked": int(revoked)} @@ -617,19 +659,25 @@ def _public_batch_approval_item(approval: Dict[str, Any], repos: Repositories) - return public +def _coerce_approvals_batch_share_row(row: Dict[str, Any]) -> Dict[str, Any]: + workspace_id = _safe_workspace_id(str(row.get("workspace_id") or row.get("entity_id") or "")) + owner_id = str(row.get("owner_id") or "") + if str(row.get("entity_type") or "approvals_batch") != "approvals_batch": + raise HTTPException(status_code=404, detail="Approval batch link not found") + if not workspace_id or not owner_id: + raise HTTPException(status_code=404, detail="Approval batch link not found") + return { + "entity_type": "approvals_batch", + "entity_id": workspace_id, + "owner_id": owner_id, + "created_at": row.get("created_at"), + } + + def _load_approvals_batch_share(token: str, repos: Repositories) -> Dict[str, Any]: row = _load_approval_batch_share_row(token, repos) if row: - workspace_id = _safe_workspace_id(str(row.get("workspace_id") or "")) - if not workspace_id: - raise HTTPException(status_code=404, detail="Approval batch link not found") - return { - "entity_type": "approvals_batch", - "entity_id": workspace_id, - "owner_id": str(row.get("owner_id") or ""), - "created_at": row.get("created_at"), - } - + return _coerce_approvals_batch_share_row(row) raise HTTPException(status_code=404, detail="Approval batch link not found") @@ -1080,10 +1128,14 @@ def _public_approvals_batch_payload( repos: Repositories, *, request: Request | None = None, + share: Dict[str, Any] | None = None, ) -> Dict[str, Any]: if request is not None: _enforce_public_batch_rate_limit(request, token, action=False) - share = _load_approvals_batch_share(token, repos) + if share is None: + share = _load_approvals_batch_share(token, repos) + else: + share = _coerce_approvals_batch_share_row(share) workspace_id = str(share["entity_id"]) owner_id = str(share["owner_id"]) rows = _list_pending_approvals_for_workspace( diff --git a/apps/api/services/public_worker.py b/apps/api/services/public_worker.py index 701b9600d..62aa9d95e 100644 --- a/apps/api/services/public_worker.py +++ b/apps/api/services/public_worker.py @@ -292,7 +292,9 @@ def _standalone_share_payload( if entity_type == "run": return _public_run_share(row, repos) if entity_type == "approvals_batch": - raise HTTPException(status_code=404, detail="Share link not found") + from routers.approvals import _public_approvals_batch_payload + + return _public_approvals_batch_payload(token, repos, request=request, share=row) raise HTTPException(status_code=404, detail="Share link not found") try: diff --git a/apps/api/tests/test_approvals_batch_share_link.py b/apps/api/tests/test_approvals_batch_share_link.py index 36d9d0e84..0ea57ccb2 100644 --- a/apps/api/tests/test_approvals_batch_share_link.py +++ b/apps/api/tests/test_approvals_batch_share_link.py @@ -155,6 +155,44 @@ def _old_deterministic_batch_token(*, workspace_id: str, owner_id: str, approval return f"fls_{digest[:48]}" +def _insert_legacy_standalone_batch_share( + *, + token: str, + workspace_id: str = "local-default", + owner_id: str = OWNER, +) -> None: + db = importlib.import_module("db") + with db.get_db() as conn: + conn.executescript( + """ + CREATE TABLE IF NOT EXISTS standalone_share_links ( + token_hash TEXT PRIMARY KEY, + entity_type TEXT NOT NULL, + entity_id TEXT NOT NULL, + file_path TEXT NOT NULL DEFAULT '', + owner_id TEXT NOT NULL, + created_at TEXT NOT NULL, + UNIQUE(entity_type, entity_id, file_path, owner_id) + ); + """ + ) + conn.execute( + """ + INSERT INTO standalone_share_links + (token_hash, entity_type, entity_id, file_path, owner_id, created_at) + VALUES (?, ?, ?, ?, ?, ?) + """, + ( + hashlib.sha256(token.encode("utf-8")).hexdigest(), + "approvals_batch", + workspace_id, + "", + owner_id, + "2026-06-24T00:00:00Z", + ), + ) + + def _collect_keys(value) -> set[str]: if isinstance(value, dict): out = {str(key) for key in value} @@ -383,9 +421,13 @@ def test_batch_share_link_revoke_and_expiry_reject_public_resolution(client_and_ anon = TestClient(client.app, raise_server_exceptions=False) revoked = anon.get(f"/approvals/public-batch/{revoked_token}") expired = anon.get(f"/approvals/public-batch/{expiring_token}") + revoked_standalone = anon.get(f"/s/{revoked_token}") + expired_standalone = anon.get(f"/s/{expiring_token}") assert revoked.status_code == 404 assert expired.status_code == 404 + assert revoked_standalone.status_code == 404 + assert expired_standalone.status_code == 404 def test_batch_share_link_revoke_all_for_workspace_kills_sibling_links(client_and_main): @@ -525,7 +567,7 @@ def test_public_batch_legacy_raw_token_table_is_hard_rejected(client_and_main): assert [item["id"] for item in reshared_response.json()["approvals"]] == ["apr_legacy"] -def test_public_batch_legacy_standalone_approvals_batch_link_is_hard_rejected(client_and_main): +def test_standalone_legacy_approvals_batch_link_resolves_via_s_route(client_and_main): client, main = client_and_main legacy_token = "fls_legacy_standalone" _seed_approval( @@ -535,42 +577,50 @@ def test_public_batch_legacy_standalone_approvals_batch_link_is_hard_rejected(cl worker_id="w_standalone_legacy", ) - db = importlib.import_module("db") - with db.get_db() as conn: - conn.executescript( - """ - CREATE TABLE IF NOT EXISTS standalone_share_links ( - token_hash TEXT PRIMARY KEY, - entity_type TEXT NOT NULL, - entity_id TEXT NOT NULL, - file_path TEXT NOT NULL DEFAULT '', - owner_id TEXT NOT NULL, - created_at TEXT NOT NULL, - UNIQUE(entity_type, entity_id, file_path, owner_id) - ); - """ - ) - conn.execute( - """ - INSERT INTO standalone_share_links - (token_hash, entity_type, entity_id, file_path, owner_id, created_at) - VALUES (?, ?, ?, ?, ?, ?) - """, - ( - hashlib.sha256(legacy_token.encode("utf-8")).hexdigest(), - "approvals_batch", - "local-default", - "", - OWNER, - "2026-06-24T00:00:00Z", - ), - ) + _insert_legacy_standalone_batch_share(token=legacy_token) from fastapi.testclient import TestClient anon = TestClient(client.app, raise_server_exceptions=False) assert anon.get(f"/approvals/public-batch/{legacy_token}").status_code == 404 - assert anon.get(f"/s/{legacy_token}").status_code == 404 + response = anon.get(f"/s/{legacy_token}") + assert response.status_code == 200, response.text + body = response.json() + assert body["entity_type"] == "approvals_batch" + assert [item["id"] for item in body["approvals"]] == ["apr_standalone_legacy"] + _assert_no_independent_batch_item_bearers(body) + + +def test_legacy_standalone_approvals_batch_link_is_revoked_by_single_and_all_revoke(client_and_main): + client, main = client_and_main + _seed_approval( + main, + approval_id="apr_standalone_legacy_revoke", + run_id="run_standalone_legacy_revoke", + worker_id="w_standalone_legacy_revoke", + ) + + from fastapi.testclient import TestClient + + anon = TestClient(client.app, raise_server_exceptions=False) + + single_token = "fls_legacy_standalone_single" + _insert_legacy_standalone_batch_share(token=single_token) + assert anon.get(f"/s/{single_token}").status_code == 200 + + single = client.post(f"/approvals/batch-share-link/{single_token}/revoke") + assert single.status_code == 200, single.text + assert single.json() == {"status": "revoked", "entity_type": "approvals_batch"} + assert anon.get(f"/s/{single_token}").status_code == 404 + + all_token = "fls_legacy_standalone_all" + _insert_legacy_standalone_batch_share(token=all_token) + assert anon.get(f"/s/{all_token}").status_code == 200 + + all_response = client.post("/approvals/batch-share-link/revoke-all") + assert all_response.status_code == 200, all_response.text + assert all_response.json() == {"status": "revoked", "entity_type": "approvals_batch", "revoked": 1} + assert anon.get(f"/s/{all_token}").status_code == 404 def test_public_batch_allows_floom_preflight_for_decision(client_and_main):