From 8b6c1702654db819e7cab185f9f6e796160567f5 Mon Sep 17 00:00:00 2001
From: Logan Cusano <logan@simplestepsolutions.com>
Date: Tue, 1 Sep 2026 02:45:08 -0400
Subject: [PATCH] Close viewer-triggerable OpenAI spend on incident summarize
 (server-26#81)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

POST /incidents/{id}/summarize was gated by require_service_or_firebase_token,
which accepts any authenticated Firebase user including role "viewer". That
route spends OpenAI credits via the background summarizer. The call-side
equivalent was already moved to require_admin_token; this brings the incident
side in line with it.

The frontend's two "summarize now" buttons on the incident detail page are
already gated behind isAdmin, so this backend change matches existing UI
behavior exactly and does not break any viewer/operator surface — it only
closes direct-API access for non-admins.

Swept every other route in incidents.py: list/get are reads with no spend and
correctly stay open to any signed-in user; create/update/delete/link/unlink
were already require_admin_token. No other sibling route needed changing.

Adds test_incident_summarize_auth.py pinning the dependency wiring directly
(the convention used in test_admin_feature_flags.py), so a future revert back
to the weak dependency fails a test immediately.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 drb-c2-core/app/routers/incidents.py          |  2 +-
 .../tests/test_incident_summarize_auth.py     | 52 +++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 drb-c2-core/tests/test_incident_summarize_auth.py

diff --git a/drb-c2-core/app/routers/incidents.py b/drb-c2-core/app/routers/incidents.py
index 77305a1..5ba509e 100644
--- a/drb-c2-core/app/routers/incidents.py
+++ b/drb-c2-core/app/routers/incidents.py
@@ -97,7 +97,7 @@ async def delete_incident(incident_id: str, _: dict = Depends(require_admin_toke
 async def summarize_incident(
     incident_id: str,
     background_tasks: BackgroundTasks,
-    decoded: dict = Depends(require_service_or_firebase_token),
+    decoded: dict = Depends(require_admin_token),
 ):
     """Immediately run the summarizer for a specific incident."""
     from app.internal.summarizer import _summarize_incident
diff --git a/drb-c2-core/tests/test_incident_summarize_auth.py b/drb-c2-core/tests/test_incident_summarize_auth.py
new file mode 100644
index 0000000..6457544
--- /dev/null
+++ b/drb-c2-core/tests/test_incident_summarize_auth.py
@@ -0,0 +1,52 @@
+"""
+server-26#81 — any signed-in viewer could trigger OpenAI summary spend.
+
+``POST /incidents/{incident_id}/summarize`` was gated by
+``require_service_or_firebase_token``, which accepts ANY authenticated
+Firebase user (including role "viewer"), not just admins. Hitting the route
+spends OpenAI credits via the background summarizer task. The call-side
+equivalent (``PATCH /calls/{id}/transcript``) was already moved to
+``require_admin_token``; the incident side was not moved with it.
+
+Following the wiring-test convention in test_admin_feature_flags.py
+(``test_features_routes_use_the_agent_dependency_and_others_do_not``): assert
+against the route's actual dependant.dependencies rather than round-tripping
+through TestClient, so this pins the credential wiring itself and would fail
+immediately if someone reverts the dependency back to the weak one.
+"""
+from app.internal import auth
+from app.routers import incidents
+
+
+def _deps(path: str, method: str) -> set:
+    for r in incidents.router.routes:
+        if r.path == path and method in r.methods:
+            return {d.call for d in r.dependant.dependencies}
+    raise AssertionError(f"no route {method} {path}")
+
+
+def test_summarize_incident_requires_admin_not_any_firebase_user():
+    deps = _deps("/incidents/{incident_id}/summarize", "POST")
+    assert auth.require_admin_token in deps
+    assert auth.require_service_or_firebase_token not in deps
+
+
+def test_read_only_incident_routes_still_accept_any_signed_in_user():
+    """Guards against an overcorrection: reads are not spend, they stay open
+    to any authenticated viewer."""
+    assert auth.require_service_or_firebase_token in _deps("/incidents", "GET")
+    assert auth.require_service_or_firebase_token in _deps("/incidents/{incident_id}", "GET")
+
+
+def test_other_mutating_incident_routes_are_still_admin_only():
+    """Unchanged by this fix, but pinned so a future edit can't quietly
+    loosen them while touching this file."""
+    for path, method in [
+        ("/incidents/summarize", "POST"),
+        ("/incidents", "POST"),
+        ("/incidents/{incident_id}", "PUT"),
+        ("/incidents/{incident_id}", "DELETE"),
+        ("/incidents/{incident_id}/calls/{call_id}", "POST"),
+        ("/incidents/{incident_id}/calls/{call_id}", "DELETE"),
+    ]:
+        assert auth.require_admin_token in _deps(path, method), f"{method} {path}"