fix: four safe audit findings — entitlement, attribution, storage - #66
Open
keithfawcett wants to merge 1 commit into
Open
fix: four safe audit findings — entitlement, attribution, storage#66keithfawcett wants to merge 1 commit into
keithfawcett wants to merge 1 commit into
Conversation
Mechanical, low-risk fixes from the audit (Codex-verified as safe-to-fix): - Enterprise self-assign (billing.ts): POST /billing/plan accepted plan='enterprise', which counts as active + enables white-label with no Stripe subscription. Restrict the setter schema to flex/revshare (signup already excludes enterprise). - Delinquent keeps white-label (white-label.ts): isWhiteLabelEntitled keyed only on a non-null subscription id, so an unpaid/paused/canceled sub kept white-label + custom domain live for free. Route it through hasActivePlan, which already rejects delinquent statuses (past_due still entitled — Stripe is dunning). - Future-click attribution (attribution.ts): the window check only rejected ages > window, so a click that happened AFTER the event (negative age) could attribute an earlier conversion during backlog/backdated processing. Reject ageMs < 0 too. - Upload ENOENT (storage.ts): the fs backend created only the storage root, so a nested key (tenants/<id>/logos/...) failed on a fresh self-host install. mkdir the full parent path. Tests: delinquent-status white-label cases, a nested-key storage write, the enterprise-rejection route check, and a negative-age attribution case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Four mechanical, low-regression-risk findings from the audit, each Codex-verified as safe-to-fix. Bundled since they're small; each has a test.
1. Enterprise self-assignment (entitlement) — HIGH
POST /billing/planacceptedplan='enterprise'from the fullBILLING_PLANSenum. Enterprise counts as active with no Stripe subscription and can enable white-label directly — so any tenant could grant itself permanent paid entitlement for free. Signup already excludes it; this restricts the authenticated in-place setter toflex/revshare.2. Delinquent subscription keeps white-label (entitlement) — MEDIUM
isWhiteLabelEntitledkeyed only on a non-null subscription id, ignoring the mirrored status — sounpaid/paused/canceledkept white-label branding + the custom domain live for free. Now routes throughhasActivePlan(the canonical gate), which rejects delinquent statuses.past_duestays entitled (Stripe is still dunning), matchinghasActivePlan.3. Future-click attribution (money correctness) — MEDIUM
The window check rejected only
age > window, not negative ages, so during backlog/backdated attribution a click that happened after the event could take (or share) that earlier conversion. Now rejectsageMs < 0.4. Filesystem upload ENOENT (self-host) — MEDIUM
The fs storage backend created only the storage root before writing, but keys nest (
tenants/<id>/logos/...), so the first upload on a fresh self-host installENOENTed. Nowmkdirs the full parent path.Tests
Delinquent-status white-label cases (
unpaid/paused/canceledfalse,past_duetrue), a nested-key storage write + overwrite, the enterprise-rejection route check, and a negative-age attribution case. Full typecheck + lint (0 errors) clean.🤖 Generated with Claude Code