Health: 72/100 | Confidence: 82/100
PR #849 improves type safety and deploy reliability, but introduces two high-blast-radius issues: (1) the billing-mode resolver's boolean interface for an env-driven flag makes production dev-mode activation likely when callers coerce strings, and (2) the cross-site OAuth exchange places a new DB lookup after key minting, so transient failures orphan API keys and deny users their credentials. Both have exact, low-risk fixes. The deploy-platform workflow addition is a solid reliability improvement. Overall health is good with critical gaps in env-var parsing and auth-side-effect ordering.
PR #849 refactors the billing-mode gate into a pure function with exhaustive tests—a solid security improvement. However, the auth callback changes introduce two material issues: (1) unvalidated email/name from the platform OAuth response are persisted directly to the DB, creating injection and stored-payload risks; and (2) the backfill try/catch swallows all exceptions, masking unexpected failures. Test coverage for the auth path is good but missing a case for non-string platform identity fields. The server.ts refactor is clean and defensive. I could not inspect lib/billing-mode.ts or worker.ts directly because they were omitted from the provided diff context, so confidence is tempered by that blind spot.
1. 🔴 [HIGH] Billing-mode dev-router can be forced active in production by caller-side boolean coercion
File: products/sandbox/api/src/lib/billing-mode.ts
The resolveBillingMode interface declares sandboxBillingDevForceEnabled as boolean | undefined, but environment variables are strings at runtime. Callers typically bridge this gap with !!envVar or Boolean(envVar), which turns the string 'false' into true. If the production host is also misconfigured with NODE_ENV=development, the resolver returns 'dev' and surfaces the dangerous dev billing router. Additionally, the stripe secret check does not trim whitespace, so a whitespace-only string is treated as configured and pushes the app into Stripe mode with invalid credentials.
Blast radius: Production billing path — dev mode exposes free-plan transitions to any authenticated caller if NODE_ENV=development and a caller coerces a string env var
Recommended fix: Change the interface to accept string | undefined for the env-driven flag and apply strict === 'true' parsing inside the resolver. Also trim Stripe secrets before truthiness checks.
File: products/platform/api/src/routes/cross-site.ts
The diff inserts db.query.user.findFirst after keys.createKey has already minted result.key but before the response is returned. If the new user-row query throws (transient DB error, connectivity blip), the exception bubbles out without returning result.key to the client. The key-service call is a side-effect that is not rolled back by any visible transaction, so the key remains in the key store but the client has no record of it. The inline comment about a 'catch-rollback path' reveals awareness of partial-failure risk but does not address the already-minted key.
Blast radius: Auth exchange path — users are denied their API key while a ghost key remains in the platform key store; retrying mints a second key
Recommended fix: Move the identity lookup to occur before key minting so that any failure mode prevents key creation, eliminating orphaned keys entirely.
File: products/sandbox/api/src/routes/auth.ts
The auth callback blindly persists email and name from the platform OAuth exchange response without format validation, length limits, or sanitization. If the platform identity provider returns malformed, excessively long, or malicious payloads (e.g., HTML/JS, control characters), they are stored verbatim in customer records and may be rendered in downstream dashboards, creating stored-XSS or injection vectors.
Recommended fix: Validate email format with a regex, enforce max lengths (email ≤254, name ≤256), and trim name before persisting.
File: products/sandbox/api/src/routes/auth.ts
The try/catch around customers.updateCustomer swallows every exception type, including unexpected programming errors and non-constraint DB failures. This masks bugs, complicates incident response, and could hide data-loss scenarios because operators only see a generic warning with no stack trace.
Recommended fix: Distinguish constraint violations from unexpected errors: log unexpected errors at error level with a stack trace, and keep warnings only for known operational failures.
File: products/sandbox/api/tests/auth-routes.test.ts
The new auth tests cover happy-path, backfill, legacy omission, and constraint-failure scenarios, but do not verify that non-string email/name values from the platform (e.g., null, number, object) are correctly ignored. This is a regression risk if the platform contract drifts.
Recommended fix: Add a test that feeds numeric/object email/name values and asserts the customer is created with synthetic placeholders and updateCustomer is not called.