Skip to content

fix(security): close fail-open paths found by the integration suite - #7886

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix/integration-test-findings
Sep 16, 2026
Merged

waleedlatif1 merged 1 commit into
stagingfrom
fix/integration-test-findings

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Six fixes for issues surfaced while running the integration suite. Each was confirmed against the source before being fixed, and each carries a regression test that was verified to fail without its fix.

  • Permission groups stopped governing on a failed payment. Enforcement read the usable-subscription set (active only), so a past_due organization resolved to entitled: falseconfig: null — which denies nothing, and therefore lifted every restriction the organization had configured, silently, for the whole dunning window. The billing summary meanwhile reported the subscription as active, and the admin could not open permission-group settings to notice. Governance now reads its own resolver: every entitled status counts, the billing block does not apply, and a read failure throws rather than answering "no restrictions".
  • One-time tokens lived for 24 hours. Redeeming one returns a session cookie for the session it points at, so an unredeemed token was a bearer credential for that session until it expired. The socket handshake mints a fresh token per attempt and the desktop handoff sets its own expiry on the row, so nothing needed the long window; it is now 2 minutes.
  • The auth plugin's own password-reset endpoints were reachable. They reach the same mailer as /api/auth/forget-password without its per-recipient budget or its audit record. Now refused by shape rather than by an exact list, so a renamed or added alias cannot reopen the path. The verification-code sender takes its purpose from the request body and sends reset mail when asked for it, so it is refused for that purpose and left reachable for the one the product actually sends — the resend button on /verify.
  • Password reset answered 500 for an invalid token. An expired or reused link is the caller's to fix, and the library reports it as a 400-class error; re-emitting it as a 500 paged on a routine action and returned the library's own wording to an unauthenticated caller. The reset half now answers 400 with fixed copy. The request half answers exactly as it answers a success, because that route replies identically whether or not an account exists and a new status class would have told a caller which addresses are registered.
  • Invitation tokens went to read-only members. The token stands in for being the invitee or an admin on the invitation detail route, and its only use in the product is the admin-only copy-link action. It is now returned only to callers who may manage the workspace, and terminal invitations no longer leave the server at all.
  • Usage routes returned 500 for an out-of-range date. The calendar check re-serialized the parsed date, which throws for a month or day out of range; inside a refinement that escapes validation entirely and answers a malformed query string with a 500. It now uses z.iso.date(), the leap-year-aware calendar check zod already ships, which cannot throw.

Type of Change

  • Bug fix

Testing

Rebased onto staging after the SSO merge. Reviewed by three independent passes (correctness/regression, cleanup, simplify) whose findings are folded in — including two the review caught that the first round missed: the verification sender reached the same mailer under a different purpose, and routing the management API through the governance reader would have opened it for an organization whose settings page still 404s.

Tested manually. bun run lint, bun run type-check, bun run check:audits (46 audits), bun run check:api-validation, and bun run docs-manifest:check all pass; 3,720 tests pass across the affected surface. Every new test was checked against the unfixed code first to confirm it fails there — one initially did not, and was rewritten to assert the query filter rather than a mocked row.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Sep 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 16, 2026 7:41pm UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the previously reported password-reset mail bypass is fixed and no new actionable failures remain.

Summary

This PR closes several fail-open and information-exposure paths across authentication, permission governance, invitation handling, and usage-query validation.

  • Restricts Better Auth password-reset aliases and OTP purposes to preserve application-owned rate limiting and auditing.
  • Shortens one-time-token validity and normalizes unauthenticated Better Auth client errors.
  • Keeps permission-group governance active through past-due billing states and propagates subscription-read failures.
  • Withholds invitation bearer tokens from read-only members and filters terminal invitations server-side.
  • Replaces throwable date refinement logic with Zod’s calendar-aware ISO date validation.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Incoming request] --> B{Auth catch-all?}
    B -->|Password-reset alias| C[Return 404]
    B -->|OTP sender| D{type is email-verification?}
    D -->|No or unreadable| C
    D -->|Yes| E[Forward to Better Auth]
    B -->|Application reset route| F[Apply rate limits and audit]
    F --> G{Better Auth result}
    G -->|Expected client refusal| H[Return fixed non-enumerating response]
    G -->|Server failure| I[Log and return fixed 500 response]

    J[Resolve permission policy] --> K{Subscription-backed entitlement?}
    K -->|No| L[Governance active]
    K -->|Yes| M[Read active and past_due subscriptions]
    M -->|Read fails| N[Propagate error]
    M -->|Enterprise row exists| L
    M -->|No qualifying row| O[Governance inactive]
Loading

Reviews (2) · Last reviewed commit: "fix(security): close fail-open paths fou..."

Comment thread apps/sim/app/api/auth/[...all]/route.ts Outdated
@waleedlatif1
waleedlatif1 force-pushed the fix/integration-test-findings branch from 54e3401 to 7ad6309 Compare September 16, 2026 19:08
- Keep an organization's permission groups governing while its payment is
  failing: enforcement read the usable-subscription set, so a past-due
  card resolved to "no permission group", which denies nothing and lifted
  every restriction the organization had configured
- Shorten the one-time token lifetime from 24 hours to 2 minutes; the
  token redeems for a session cookie, so an unredeemed one was a bearer
  credential for that session until it expired
- Refuse the plugin's password-reset endpoints by shape, and refuse the
  verification sender when it is asked for a reset: both reach the same
  mailer as the application route without its per-recipient budget
- Answer an expired or reused reset link with a 400 and fixed copy rather
  than a 500 carrying the library's wording, while keeping the request
  half indistinguishable from a success so it discloses no addresses
- Return an invitation token only to callers who may manage the
  workspace, and stop sending terminal invitations to the client at all
- Validate usage dates with the calendar check zod already ships, which
  cannot throw out of validation the way the hand-rolled round trip did
@waleedlatif1
waleedlatif1 force-pushed the fix/integration-test-findings branch from 7ad6309 to b7560e3 Compare September 16, 2026 19:41
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 28 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/hooks/queries/invitations.ts
@waleedlatif1
waleedlatif1 merged commit f6bc723 into staging Sep 16, 2026
33 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/integration-test-findings branch September 16, 2026 19:52

This branch was previously deployed

1 inactive deployment
Preview b7560e3f Deployed Sep 16, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant