m12-pr11 TD-7: billing fail-closed in production#29
Open
BalaShankar9 wants to merge 1 commit into
Open
Conversation
Closes the silent fail-open in check_billing_limit's org-fetch path.
Problem
-------
When Supabase (or whatever backs OrgService.get_user_orgs) hiccups,
the existing handler swallowed the exception and treated the user
as "no org" — which immediately short-circuited billing
enforcement and let the request through unmetered. In production
that means a backing-store blip silently disables every billing
limit on the platform until the store recovers. Fail-open on a
revenue-protecting check is unacceptable.
Fix
---
backend/app/api/deps.py:
- New helper _billing_fail_closed_enabled():
* Default: fail-closed when ENVIRONMENT=production, fail-open
otherwise (preserves dev/test ergonomics — local runs without
a seeded org keep working).
* BILLING_FAIL_CLOSED env var (1/true/yes/on or 0/false/no/off)
overrides in either direction so ops can flip without a
redeploy and tests can pin the mode deterministically.
* Read at call time so monkeypatch.setenv works in tests.
- check_billing_limit now logs billing_org_fetch_failed
(structured, includes user_id, feature, error head, and the
fail-closed flag for forensics) and raises 503 when fail-closed
is on. Existing 503 path on billing.check_limit() failure is
preserved untouched.
backend/tests/unit/test_billing_fail_closed.py: 22 tests covering
the helper (defaults, env overrides, truthy/falsy variants) and
check_billing_limit (fail-open in dev, fail-closed in production,
override flips both ways, no-org genuine path unchanged,
billing.check_limit 503 path preserved, allowed → pass, denied →
402).
Blueprint TD-7 -> SHIPPED.
There was a problem hiding this comment.
Pull request overview
This PR hardens billing enforcement by preventing a fail-open path when the org lookup (Supabase-backed) errors, ensuring production requests don’t bypass billing limits during backing-store outages.
Changes:
- Added
_billing_fail_closed_enabled()to control fail-closed vs fail-open behavior based onENVIRONMENTandBILLING_FAIL_CLOSED. - Updated
check_billing_limitto log org-fetch failures and return 503 when fail-closed is enabled. - Added unit tests covering mode defaults, env overrides, and request-path behaviors for org-fetch/billing failures.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/architecture/WORLD_CLASS_ARCHITECTURE_BLUEPRINT.md | Marks TD-7 as shipped and documents the new billing fail-closed behavior. |
| backend/app/api/deps.py | Implements fail-closed toggle + 503 behavior on org-fetch errors with structured logging. |
| backend/tests/unit/test_billing_fail_closed.py | Adds unit tests validating toggle parsing and check_billing_limit behavior across scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+126
to
+131
| 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" | ||
|
|
||
|
|
Comment on lines
+188
to
+195
| _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(): |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TD-7 from the World Class Architecture Blueprint. Closes the silent fail-open in
check_billing_limit's org-fetch path.Problem
When Supabase (or whatever backs
OrgService.get_user_orgs) hiccups, the existing handler swallowed the exception and treated the user as "no org" — which immediately short-circuited billing enforcement and let the request through unmetered. In production a backing-store blip silently disables every billing limit on the platform until the store recovers. Fail-open on a revenue-protecting check is unacceptable.Fix
backend/app/api/deps.py:_billing_fail_closed_enabled():ENVIRONMENT=production, fail-open otherwise (preserves dev/test ergonomics — local runs without a seeded org keep working).BILLING_FAIL_CLOSEDenv var (1/true/yes/onor0/false/no/off) overrides in either direction so ops can flip without a redeploy and tests can pin the mode deterministically.monkeypatch.setenvworks in tests.check_billing_limitnow logsbilling_org_fetch_failed(structured, includesuser_id,feature, error head, and thefail_closedflag for forensics) and raises 503 when fail-closed is on. Existing 503 path onbilling.check_limit()failure is preserved untouched.Tests
backend/tests/unit/test_billing_fail_closed.py— 22 tests, 22 green:_billing_fail_closed_enableddefaults (dev → False, production → True).1/true/TRUE/Yes/on/0/false/no/off/"").check_billing_limit:BILLING_FAIL_CLOSED=1forces 503 even in dev.BILLING_FAIL_CLOSED=0forces fail-open even in production.billing.check_limit()failure preserved.Stack
Files: 3 changed, +224 / -2.
Blueprint TD-7 → SHIPPED.