m12-pr11 TD-7: billing fail-closed in production

backend/app/api/deps.py

@@ -108,6 +108,27 @@ async def get_current_user_optional(
 _billing_logger = _structlog.get_logger()
 
 
+def _billing_fail_closed_enabled() -> bool:
+    """Whether billing enforcement should fail-closed on backing-store errors.
+
+    Default policy:
+      * ``ENVIRONMENT=production`` → fail-closed (return True).
+      * Otherwise → fail-open (legacy behaviour).
+      * ``BILLING_FAIL_CLOSED`` env var (``1``/``true``/``yes``/``on`` or
+        ``0``/``false``/``no``/``off``) overrides the default in either
+        direction so ops can flip without a redeploy and tests can pin
+        the mode.
+
+    Read at call time (not module import) so monkeypatched env vars in
+    tests are honoured.
+    """
+    import os
+    raw = os.getenv("BILLING_FAIL_CLOSED")
+    if raw is not None:
+        return raw.strip().lower() in {"1", "true", "yes", "on"}
+    return os.getenv("ENVIRONMENT", "development").strip().lower() == "production"
+
+
 async def check_usage_guard(current_user: Dict[str, Any]) -> None:
     """Backstop per-user/per-platform cap that runs BEFORE billing.
 
@@ -145,14 +166,37 @@ async def check_billing_limit(feature: str, current_user: Dict[str, Any]) -> Non
     """Check billing limit for a feature. Raises 402 if over limit.
 
     If the user has no org (testing/free solo mode), the check is skipped.
+
+    **Fail-closed behaviour (TD-7 / m12-pr11):** when the org-fetch path
+    raises (e.g. Supabase outage), we historically swallowed the error
+    and treated the user as "no org", which silently disabled billing
+    enforcement for every authenticated request. That is fail-open and
+    unacceptable in production. We now consult
+    :func:`_billing_fail_closed_enabled` — when true (default in
+    ``ENVIRONMENT=production``, opt-in everywhere else via
+    ``BILLING_FAIL_CLOSED=1``) we raise 503 instead of skipping the
+    check. Local dev / CI keep the legacy permissive behaviour so tests
+    that don't seed an org still pass.
     """
     from app.services.org import OrgService
     from app.services.billing import BillingService
 
     org_service = OrgService()
     try:
         orgs = await org_service.get_user_orgs(current_user["id"])
-    except Exception:
+    except Exception as exc:
+        _billing_logger.error(
+            "billing_org_fetch_failed",
+            user_id=current_user.get("id"),
+            feature=feature,
+            error=str(exc)[:200],
+            fail_closed=_billing_fail_closed_enabled(),
+        )
+        if _billing_fail_closed_enabled():
+            raise HTTPException(
+                status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
+                detail="Billing service is temporarily unavailable. Please try again.",
+            )
         orgs = []
 
     if not orgs:

backend/tests/unit/test_billing_fail_closed.py

@@ -0,0 +1,178 @@
+"""Tests for TD-7 / m12-pr11 billing fail-closed behaviour."""
+from __future__ import annotations
+
+import pytest
+from fastapi import HTTPException
+
+from app.api import deps
+
+
+# ── _billing_fail_closed_enabled ──────────────────────────────────────
+
+
+def test_default_dev_is_fail_open(monkeypatch):
+    monkeypatch.delenv("BILLING_FAIL_CLOSED", raising=False)
+    monkeypatch.delenv("ENVIRONMENT", raising=False)
+    assert deps._billing_fail_closed_enabled() is False
+
+
+def test_production_default_is_fail_closed(monkeypatch):
+    monkeypatch.delenv("BILLING_FAIL_CLOSED", raising=False)
+    monkeypatch.setenv("ENVIRONMENT", "production")
+    assert deps._billing_fail_closed_enabled() is True
+
+
+def test_env_override_forces_fail_closed_in_dev(monkeypatch):
+    monkeypatch.setenv("ENVIRONMENT", "development")
+    monkeypatch.setenv("BILLING_FAIL_CLOSED", "1")
+    assert deps._billing_fail_closed_enabled() is True
+
+
+def test_env_override_forces_fail_open_in_production(monkeypatch):
+    monkeypatch.setenv("ENVIRONMENT", "production")
+    monkeypatch.setenv("BILLING_FAIL_CLOSED", "0")
+    assert deps._billing_fail_closed_enabled() is False
+
+
+@pytest.mark.parametrize("truthy", ["1", "true", "TRUE", "Yes", "on"])
+def test_env_truthy_variants(monkeypatch, truthy):
+    monkeypatch.setenv("BILLING_FAIL_CLOSED", truthy)
+    assert deps._billing_fail_closed_enabled() is True
+
+
+@pytest.mark.parametrize("falsy", ["0", "false", "no", "off", ""])
+def test_env_falsy_variants(monkeypatch, falsy):
+    monkeypatch.setenv("BILLING_FAIL_CLOSED", falsy)
+    monkeypatch.delenv("ENVIRONMENT", raising=False)
+    assert deps._billing_fail_closed_enabled() is False
+
+
+# ── check_billing_limit org-fetch failure path ────────────────────────
+
+
+class _FakeOrgService:
+    def __init__(self, *, orgs=None, raises=None):
+        self._orgs = orgs or []
+        self._raises = raises
+
+    async def get_user_orgs(self, user_id):
+        if self._raises is not None:
+            raise self._raises
+        return self._orgs
+
+
+class _FakeBillingService:
+    def __init__(self, allowed=True, raises=None):
+        self._allowed = allowed
+        self._raises = raises
+
+    async def check_limit(self, org_id, feature):
+        if self._raises is not None:
+            raise self._raises
+        return self._allowed
+
+
+@pytest.fixture
+def patch_services(monkeypatch):
+    """Patch OrgService / BillingService imports inside check_billing_limit."""
+    state = {"org": _FakeOrgService(), "billing": _FakeBillingService()}
+
+    import app.services.org as org_mod
+    import app.services.billing as billing_mod
+
+    monkeypatch.setattr(org_mod, "OrgService", lambda: state["org"])
+    monkeypatch.setattr(billing_mod, "BillingService", lambda: state["billing"])
+    return state
+
+
+@pytest.mark.asyncio
+async def test_org_fetch_failure_fails_open_in_dev(monkeypatch, patch_services):
+    """Legacy permissive path: dev/test envs swallow the error."""
+    monkeypatch.delenv("BILLING_FAIL_CLOSED", raising=False)
+    monkeypatch.delenv("ENVIRONMENT", raising=False)
+    patch_services["org"] = _FakeOrgService(raises=RuntimeError("supabase down"))
+
+    # Should silently return (no orgs → skip enforcement).
+    await deps.check_billing_limit("resume_generation", {"id": "user-1"})
+
+
+@pytest.mark.asyncio
+async def test_org_fetch_failure_fails_closed_in_production(
+    monkeypatch, patch_services
+):
+    monkeypatch.delenv("BILLING_FAIL_CLOSED", raising=False)
+    monkeypatch.setenv("ENVIRONMENT", "production")
+    patch_services["org"] = _FakeOrgService(raises=RuntimeError("supabase down"))
+
+    with pytest.raises(HTTPException) as exc:
+        await deps.check_billing_limit("resume_generation", {"id": "user-1"})
+    assert exc.value.status_code == 503
+    assert "Billing service" in exc.value.detail
+
+
+@pytest.mark.asyncio
+async def test_org_fetch_failure_fails_closed_when_env_override(
+    monkeypatch, patch_services
+):
+    monkeypatch.setenv("ENVIRONMENT", "development")
+    monkeypatch.setenv("BILLING_FAIL_CLOSED", "1")
+    patch_services["org"] = _FakeOrgService(raises=RuntimeError("boom"))
+
+    with pytest.raises(HTTPException) as exc:
+        await deps.check_billing_limit("resume_generation", {"id": "user-1"})
+    assert exc.value.status_code == 503
+
+
+@pytest.mark.asyncio
+async def test_org_fetch_failure_fails_open_when_override_off(
+    monkeypatch, patch_services
+):
+    monkeypatch.setenv("ENVIRONMENT", "production")
+    monkeypatch.setenv("BILLING_FAIL_CLOSED", "0")
+    patch_services["org"] = _FakeOrgService(raises=RuntimeError("boom"))
+
+    # Override forces fail-open even in production — should not raise.
+    await deps.check_billing_limit("resume_generation", {"id": "user-1"})
+
+
+@pytest.mark.asyncio
+async def test_no_orgs_skips_enforcement(monkeypatch, patch_services):
+    """Genuine no-org user (solo mode) is unaffected by fail-closed mode."""
+    monkeypatch.setenv("BILLING_FAIL_CLOSED", "1")
+    patch_services["org"] = _FakeOrgService(orgs=[])
+    # Should return cleanly — no org → skip.
+    await deps.check_billing_limit("resume_generation", {"id": "user-1"})
+
+
+@pytest.mark.asyncio
+async def test_org_present_then_billing_check_failure_still_503(
+    monkeypatch, patch_services
+):
+    """Pre-existing 503 path on billing.check_limit() error is preserved."""
+    monkeypatch.delenv("BILLING_FAIL_CLOSED", raising=False)
+    monkeypatch.delenv("ENVIRONMENT", raising=False)
+    patch_services["org"] = _FakeOrgService(orgs=[{"id": "org-1"}])
+    patch_services["billing"] = _FakeBillingService(raises=RuntimeError("billing down"))
+
+    with pytest.raises(HTTPException) as exc:
+        await deps.check_billing_limit("resume_generation", {"id": "user-1"})
+    assert exc.value.status_code == 503
+
+
+@pytest.mark.asyncio
+async def test_org_present_and_allowed_passes(monkeypatch, patch_services):
+    monkeypatch.delenv("BILLING_FAIL_CLOSED", raising=False)
+    patch_services["org"] = _FakeOrgService(orgs=[{"id": "org-1"}])
+    patch_services["billing"] = _FakeBillingService(allowed=True)
+    await deps.check_billing_limit("resume_generation", {"id": "user-1"})
+
+
+@pytest.mark.asyncio
+async def test_org_present_and_denied_raises_402(monkeypatch, patch_services):
+    monkeypatch.delenv("BILLING_FAIL_CLOSED", raising=False)
+    patch_services["org"] = _FakeOrgService(orgs=[{"id": "org-1"}])
+    patch_services["billing"] = _FakeBillingService(allowed=False)
+
+    with pytest.raises(HTTPException) as exc:
+        await deps.check_billing_limit("resume_generation", {"id": "user-1"})
+    assert exc.value.status_code == 402

docs/architecture/WORLD_CLASS_ARCHITECTURE_BLUEPRINT.md

@@ -1041,7 +1041,7 @@ Items that are not P0/P1 but must be tracked. Reviewed quarterly.
 | TD-4 | `requirements.txt` unpinned ranges | Reproducibility | Add `requirements.lock` (pip-compile) |
 | TD-5 | Python 3.11 in CI vs 3.13 local | Dev/CI drift | Pin one and document |
 | TD-6 | Multiple root-level `*_PLAN.md` | No canonical doc → drift | Archive to `docs/_archive/` (this PR) |
-| TD-7 | `_billing_logger` swallows org-fetch errors | Fail-open on Supabase outage | Make fail-closed in prod |
+| TD-7 | `_billing_logger` swallows org-fetch errors | Fail-open on Supabase outage | SHIPPED (m12-pr11) — `check_billing_limit` now consults `_billing_fail_closed_enabled()`: defaults to fail-closed (503) in `ENVIRONMENT=production`, fail-open elsewhere; `BILLING_FAIL_CLOSED=1`/`0` overrides in either direction so ops can flip without redeploy |
 | TD-8 | No mutation testing | Critical paths under-tested | Stage A end |
 | TD-9 | No per-region observability sharding | One Sentry org = noise | Stage C |
 | TD-10 | Dual-namespace lazy imports in `ai_engine/client.py` | Smell of unsettled package layout | Resolved by P1-6 |