From b66ea194db28f0e8b6eb7bb0a7e29d7c9806e4d7 Mon Sep 17 00:00:00 2001 From: man4ish Date: Sun, 4 Oct 2026 10:36:09 -0500 Subject: [PATCH] fix: billing_client.py was calling billing-service's subscription route without its /billing router prefix organization_has_active_plan() called http://billing-service:8005/organizations/{id}/subscription, but billing-service's own router is mounted at /billing (app/routers/billing.py's APIRouter(prefix="/billing")) -- the real path is /billing/organizations/{id}/subscription. Every call this function ever made 404'd against the real service, which the fail- closed-on-404 branch then correctly (but wrongly, given the actual cause) treated as "no subscription at all" -- blocking self-service API key creation for every organization, always, regardless of real subscription status. Caught via a live-stack smoke test, not this module's own unit suite: every test here mocks httpx.get directly, so none of them ever exercised the real URL against the real billing-service router. The one test that came closest (asserting the URL) used .endswith(), which is satisfied by both the correct and the broken path -- tightened to an exact match so this exact bug class can't silently recur. Co-Authored-By: Claude Sonnet 5 --- app/services/billing_client.py | 4 ++-- tests/test_billing_client.py | 19 +++++++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/app/services/billing_client.py b/app/services/billing_client.py index bc103f4..0f9fab5 100644 --- a/app/services/billing_client.py +++ b/app/services/billing_client.py @@ -4,7 +4,7 @@ one cross-service call it needs -- omnibioai-billing owns subscription state, omnibioai-auth doesn't duplicate it. -Calls omnibioai-billing's existing GET /organizations/{id}/subscription +Calls omnibioai-billing's existing GET /billing/organizations/{id}/subscription (no new billing-service endpoint, no new shared secret): that route already accepts any validly-signed platform JWT whose org_id claim matches the organization being queried (see omnibioai-billing's @@ -52,7 +52,7 @@ def organization_has_active_plan(organization_id: int) -> bool: }) try: resp = httpx.get( - f"{settings.BILLING_SERVICE_URL}/organizations/{organization_id}/subscription", + f"{settings.BILLING_SERVICE_URL}/billing/organizations/{organization_id}/subscription", headers={"Authorization": f"Bearer {token}"}, timeout=3, ) diff --git a/tests/test_billing_client.py b/tests/test_billing_client.py index b2cfa53..758ee29 100644 --- a/tests/test_billing_client.py +++ b/tests/test_billing_client.py @@ -1,12 +1,19 @@ """app/services/billing_client.py: the one cross-service call behind design audit gap #9's "self-service key creation gated on an active billing plan." Mocks httpx.get directly -- no real omnibioai-billing -call is ever made in this suite. +call is ever made in this suite. See +test_sends_a_token_scoped_to_the_requested_organization's own comment +for why that one assertion is an exact match, not endswith -- a +real-stack smoke test (not this mocked suite) is what actually caught +the URL this module originally called being wrong (missing billing- +service's own /billing router prefix), so every call it ever made 404'd +in production despite this whole suite passing throughout. Developer: Manish Kumar """ from unittest.mock import MagicMock, patch +from app.core.config import settings from app.services import billing_client @@ -78,7 +85,15 @@ def fake_get(url, headers=None, timeout=None): with patch.object(billing_client.httpx, "get", side_effect=fake_get): billing_client.organization_has_active_plan(99) - assert captured["url"].endswith("/organizations/99/subscription") + # Exact match, not endswith: billing-service's router is mounted at + # /billing (app/routers/billing.py's own APIRouter(prefix="/billing") + # in omnibioai-billing) -- a bare /organizations/99/subscription + # also satisfies endswith(".../organizations/99/subscription") while + # actually 404ing against the real service, which is exactly the bug + # this exact-match assertion exists to catch (found via a live + # integration check against the real billing-service, not by any + # mocked unit test -- see this module's own git history). + assert captured["url"] == f"{settings.BILLING_SERVICE_URL}/billing/organizations/99/subscription" claims = decode_token(captured["token"]) assert claims["org_id"] == 99 assert claims["type"] == "access"