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

- 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

Author: waleedlatif1

apps/sim/app/api/auth/[...all]/route.test.ts

@@ -190,6 +190,76 @@ describe('auth catch-all route (DISABLE_AUTH get-session)', () => {
   })
 })
 
+describe('auth catch-all route password-reset mail', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it.each([
+    'request-password-reset',
+    'email-otp/request-password-reset',
+    'forget-password/email-otp',
+    /** Matched by shape, so a plugin version that renames or adds an alias cannot reopen it. */
+    'request-password-reset/v2',
+    'some-plugin/forget-password',
+  ])('blocks %s, which reaches the mailer without the per-recipient budget', async (path) => {
+    const req = createMockRequest('POST', undefined, {}, `http://localhost:3000/api/auth/${path}`)
+
+    const res = await POST(req)
+
+    expect(res.status).toBe(404)
+    expect(handlerMocks.betterAuthPOST).not.toHaveBeenCalled()
+    await expect(res.json()).resolves.toEqual({
+      error: 'Password reset is handled by application API routes.',
+    })
+  })
+
+  /** The resend button on /verify calls this directly, so blocking it would break verification. */
+  it('leaves the verification-code sender reachable for the purpose the product sends', async () => {
+    const req = createMockRequest(
+      'POST',
+      { email: 'someone@example.com', type: 'email-verification' },
+      {},
+      'http://localhost:3000/api/auth/email-otp/send-verification-otp'
+    )
+
+    await POST(req)
+
+    expect(handlerMocks.betterAuthPOST).toHaveBeenCalled()
+  })
+
+  /**
+   * The same endpoint takes the OTP purpose from the body, and `forget-password` there sends reset
+   * mail to any address named — blocking the reset paths while leaving this open renames the hole.
+   */
+  it.each(['forget-password', 'sign-in', 'change-email'])(
+    'refuses the verification sender asked for %s',
+    async (type) => {
+      const req = createMockRequest(
+        'POST',
+        { email: 'victim@example.com', type },
+        {},
+        'http://localhost:3000/api/auth/email-otp/send-verification-otp'
+      )
+
+      expect((await POST(req)).status).toBe(404)
+      expect(handlerMocks.betterAuthPOST).not.toHaveBeenCalled()
+    }
+  )
+
+  it('refuses the verification sender when the body cannot be read', async () => {
+    const req = createMockRequest(
+      'POST',
+      undefined,
+      {},
+      'http://localhost:3000/api/auth/email-otp/send-verification-otp'
+    )
+
+    expect((await POST(req)).status).toBe(404)
+    expect(handlerMocks.betterAuthPOST).not.toHaveBeenCalled()
+  })
+})
+
 describe('auth catch-all route organization mutations', () => {
   beforeEach(() => {
     vi.clearAllMocks()

apps/sim/app/api/auth/[...all]/route.ts

@@ -15,6 +15,42 @@ export const dynamic = 'force-dynamic'
 
 const { GET: betterAuthGET, POST: betterAuthPOST } = toNextJsHandler(auth.handler)
 const SAFE_ORGANIZATION_POST_PATHS = new Set(['organization/check-slug', 'organization/set-active'])
+/**
+ * Password-reset mail the plugin would send under a name Sim does not own.
+ *
+ * `/api/auth/forget-password` is an application route that owns the per-recipient budget (5 per 15
+ * minutes, keyed on the address) and writes the `PASSWORD_RESET_REQUESTED` audit record. Every
+ * plugin alias reaches the same mailer with only a per-IP default in front of it, which a caller
+ * spread across addresses walks straight past, at one victim's mailbox. Matched rather than listed,
+ * like the SSO and OAuth guards below, so a plugin version that renames or adds an alias cannot
+ * quietly reopen the path.
+ */
+function isBlockedPasswordResetPath(path: string): boolean {
+  return /(^|\/)(request-password-reset|forget-password)(\/|$)/.test(path)
+}
+
+/** The one OTP purpose a Sim surface sends: the resend button on `/verify`. */
+const ALLOWED_VERIFICATION_OTP_TYPE = 'email-verification'
+const VERIFICATION_OTP_SENDER_PATH = 'email-otp/send-verification-otp'
+
+/**
+ * The same mailer again, reached by asking the verification sender for a different purpose.
+ *
+ * `email-otp/send-verification-otp` takes the OTP `type` from the request body, and
+ * `forget-password` there sends reset mail to any address named — so blocking the reset paths
+ * above while leaving this one open would only rename the hole. The endpoint stays reachable for
+ * the purpose the product actually sends, and an unreadable body is refused rather than forwarded.
+ */
+async function isBlockedVerificationOtpSend(request: NextRequest, path: string): Promise<boolean> {
+  if (path !== VERIFICATION_OTP_SENDER_PATH) return false
+  // boundary-raw-json: the plugin owns this endpoint's schema; the guard reads one field to decide whether to forward the request at all
+  const body = await request
+    .clone()
+    .json()
+    .catch(() => null)
+  return (body as { type?: unknown } | null)?.type !== ALLOWED_VERIFICATION_OTP_TYPE
+}
+
 const OAUTH_CALLBACK_PATH_PREFIX = 'oauth2/callback/'
 const UNSUPPORTED_OIDC_PATHS = new Set([
   '.well-known/openid-configuration',
@@ -179,6 +215,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
     )
   }
 
+  if (isBlockedPasswordResetPath(path) || (await isBlockedVerificationOtpSend(request, path))) {
+    return NextResponse.json(
+      { error: 'Password reset is handled by application API routes.' },
+      { status: 404 }
+    )
+  }
+
   if (isBlockedOAuthProviderMutationPath(path)) {
     return NextResponse.json(
       { error: 'OAuth client registration is not available.' },

apps/sim/app/api/auth/forget-password/route.test.ts

@@ -64,6 +64,7 @@ vi.mock('@sim/logger', () => ({
   setRequestAuth: vi.fn(),
 }))
 
+import { APIError } from 'better-auth/api'
 import { POST } from '@/app/api/auth/forget-password/route'
 
 describe('Forget Password API Route', () => {
@@ -210,6 +211,24 @@ describe('Forget Password API Route', () => {
     expect(mockRequestPasswordReset).not.toHaveBeenCalled()
   })
 
+  /**
+   * The route answers identically whether or not an account exists, so a refusal must not become a
+   * status the success path never produces — that alone would tell a caller which addresses are
+   * registered. It is logged rather than surfaced.
+   */
+  it('answers a refusal Better Auth raises the way it answers a success', async () => {
+    mockRequestPasswordReset.mockRejectedValue(
+      new APIError('BAD_REQUEST', { message: 'invalid email' })
+    )
+
+    const response = await POST(createMockRequest('POST', { email: 'someone@example.com' }))
+
+    expect(response.status).toBe(200)
+    await expect(response.json()).resolves.toEqual({ success: true })
+    expect(mockLogger.error).not.toHaveBeenCalled()
+    expect(mockLogger.warn).toHaveBeenCalled()
+  })
+
   it('should handle auth service error with message', async () => {
     const errorMessage = 'User not found'
 
@@ -223,7 +242,9 @@ describe('Forget Password API Route', () => {
     const data = await response.json()
 
     expect(response.status).toBe(500)
-    expect(data.message).toBe(errorMessage)
+    /** An unrecognized failure is ours, and its wording is not for an unauthenticated caller. */
+    expect(data.message).toBe('Failed to send password reset email. Please try again later.')
+    expect(data.message).not.toContain(errorMessage)
 
     expect(mockLogger.error).toHaveBeenCalledWith('Error requesting password reset:', {
       error: expect.any(Error),

apps/sim/app/api/auth/forget-password/route.ts

@@ -7,6 +7,7 @@ import { type NextRequest, NextResponse } from 'next/server'
 import { forgetPasswordContract } from '@/lib/api/contracts'
 import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
 import { auth } from '@/lib/auth'
+import { getBetterAuthClientErrorStatus } from '@/lib/auth/better-auth-error'
 import {
   enforceIpRateLimitWithIndependentBackstop,
   enforceRecipientRateLimit,
@@ -92,18 +93,23 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
 
     return NextResponse.json({ success: true })
   } catch (error) {
+    /**
+     * A refusal Better Auth raises is not a server fault, but it must not become a distinguishable
+     * answer either: this route replies identically whether or not an account exists, and a status
+     * the success path never produces would tell a caller which addresses are registered. So it is
+     * logged and answered like a success — only the reset half, where the caller already holds the
+     * token and has nothing left to enumerate, surfaces the refusal.
+     */
+    const clientStatus = getBetterAuthClientErrorStatus(error)
+    if (clientStatus !== undefined) {
+      logger.warn('Rejected a password reset request', { status: clientStatus })
+      return NextResponse.json({ success: true })
+    }
+
     logger.error('Error requesting password reset:', { error })
 
     return NextResponse.json(
-      {
-        message:
-          // utils-lint-allow: returned to an unauthenticated caller, so a non-Error throw
-          // must surface the fixed copy rather than its own text — getErrorMessage would
-          // pass a thrown string straight through.
-          error instanceof Error
-            ? error.message
-            : 'Failed to send password reset email. Please try again later.',
-      },
+      { message: 'Failed to send password reset email. Please try again later.' },
       { status: 500 }
     )
   }

apps/sim/app/api/auth/reset-password/route.test.ts

@@ -46,6 +46,7 @@ vi.mock('@sim/logger', () => ({
   setRequestAuth: vi.fn(),
 }))
 
+import { APIError } from 'better-auth/api'
 import { POST } from '@/app/api/auth/reset-password/route'
 
 describe('Reset Password API Route', () => {
@@ -160,6 +161,22 @@ describe('Reset Password API Route', () => {
     expect(mockResetPassword).not.toHaveBeenCalled()
   })
 
+  it('refuses an invalid or expired token with a 400, not a server error', async () => {
+    // Better Auth reports a consumed, expired, or fabricated token as a 400-class APIError.
+    // Re-emitting that as a 500 paged on a routine click of a stale reset link.
+    mockResetPassword.mockRejectedValue(new APIError('BAD_REQUEST', { message: 'invalid token' }))
+
+    const response = await POST(
+      createMockRequest('POST', { token: 'expired-token', newPassword: 'newSecurePassword123!' })
+    )
+
+    expect(response.status).toBe(400)
+    await expect(response.json()).resolves.toMatchObject({
+      message: 'This reset link is invalid or has expired. Please request a new one.',
+    })
+    expect(mockLogger.error).not.toHaveBeenCalled()
+  })
+
   it('should handle auth service error with message', async () => {
     const errorMessage = 'Invalid or expired token'
 
@@ -174,7 +191,11 @@ describe('Reset Password API Route', () => {
     const data = await response.json()
 
     expect(response.status).toBe(500)
-    expect(data.message).toBe(errorMessage)
+    /** An unrecognized failure is ours, and its wording is not for an unauthenticated caller. */
+    expect(data.message).toBe(
+      'Failed to reset password. Please try again or request a new reset link.'
+    )
+    expect(data.message).not.toContain(errorMessage)
 
     expect(mockLogger.error).toHaveBeenCalledWith('Error during password reset:', {
       error: expect.any(Error),

apps/sim/app/api/auth/reset-password/route.ts

@@ -3,6 +3,7 @@ import { type NextRequest, NextResponse } from 'next/server'
 import { resetPasswordContract } from '@/lib/api/contracts'
 import { parseRequest } from '@/lib/api/server'
 import { auth } from '@/lib/auth'
+import { getBetterAuthClientErrorStatus } from '@/lib/auth/better-auth-error'
 import { enforceIpRateLimit, type TokenBucketConfig } from '@/lib/core/rate-limiter'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 
@@ -55,18 +56,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
 
     return NextResponse.json({ success: true })
   } catch (error) {
+    /** An expired or reused token is the caller's to fix; the fixed copy names the recovery. */
+    const clientStatus = getBetterAuthClientErrorStatus(error)
+    if (clientStatus !== undefined) {
+      logger.warn('Rejected a password reset', { status: clientStatus })
+      return NextResponse.json(
+        { message: 'This reset link is invalid or has expired. Please request a new one.' },
+        { status: 400 }
+      )
+    }
+
     logger.error('Error during password reset:', { error })
 
     return NextResponse.json(
-      {
-        message:
-          // utils-lint-allow: returned to an unauthenticated caller, so a non-Error throw
-          // must surface the fixed copy rather than its own text — getErrorMessage would
-          // pass a thrown string straight through.
-          error instanceof Error
-            ? error.message
-            : 'Failed to reset password. Please try again or request a new reset link.',
-      },
+      { message: 'Failed to reset password. Please try again or request a new reset link.' },
       { status: 500 }
     )
   }

apps/sim/app/api/auth/socket-token/route.ts

@@ -3,6 +3,7 @@ import { toError } from '@sim/utils/errors'
 import { headers } from 'next/headers'
 import { type NextRequest, NextResponse } from 'next/server'
 import { auth } from '@/lib/auth'
+import { getBetterAuthClientErrorStatus } from '@/lib/auth/better-auth-error'
 import { isAuthDisabled } from '@/lib/core/config/env-flags'
 import { enforceIpRateLimit } from '@/lib/core/rate-limiter'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -40,14 +41,11 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
 
     return NextResponse.json({ token: response.token })
   } catch (error) {
-    // better-auth's sessionMiddleware throws APIError("UNAUTHORIZED") with no message
-    // when the session is missing/expired — surface this as a 401, not a 500.
-    if (
-      error instanceof Error &&
-      ('statusCode' in error || 'status' in error) &&
-      ((error as Record<string, unknown>).statusCode === 401 ||
-        (error as Record<string, unknown>).status === 'UNAUTHORIZED')
-    ) {
+    /**
+     * better-auth's sessionMiddleware throws `APIError("UNAUTHORIZED")` with no message when the
+     * session is missing or expired — surface that as a 401, not a 500.
+     */
+    if (getBetterAuthClientErrorStatus(error) === 401) {
       logger.warn('Socket token request with invalid/expired session')
       return NextResponse.json({ error: 'Authentication required' }, { status: 401 })
     }

apps/sim/app/api/desktop/auth/handoff/route.ts

@@ -3,6 +3,7 @@ import { getErrorMessage } from '@sim/utils/errors'
 import { headers } from 'next/headers'
 import { type NextRequest, NextResponse } from 'next/server'
 import { auth } from '@/lib/auth'
+import { getBetterAuthClientErrorStatus } from '@/lib/auth/better-auth-error'
 import { createDesktopHandoffToken } from '@/lib/auth/desktop-handoff'
 import { enforceIpRateLimit } from '@/lib/core/rate-limiter'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -43,15 +44,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
     const token = await createDesktopHandoffToken(session.user.id)
     return NextResponse.json({ token })
   } catch (error) {
-    // Session creation runs the app's own `session.create.before` hook, which
-    // rejects access-controlled accounts with a Better Auth APIError. That is a
-    // permanent refusal, not a server fault — a 500 would tell the user to try
-    // again forever.
-    if (
-      error instanceof Error &&
-      'statusCode' in error &&
-      (error as Record<string, unknown>).statusCode === 403
-    ) {
+    /**
+     * Session creation runs the app's own `session.create.before` hook, which rejects
+     * access-controlled accounts with a Better Auth `APIError`. That is a permanent refusal, not a
+     * server fault — a 500 would tell the user to try again forever.
+     */
+    if (getBetterAuthClientErrorStatus(error) === 403) {
       logger.warn('Desktop handoff refused for this account', { userId: session.user.id })
       return NextResponse.json(
         { error: getErrorMessage(error, 'Access restricted') },

apps/sim/app/api/organizations/[id]/permission-groups/utils.ts

@@ -31,6 +31,12 @@ export async function authorizeOrgAccessControl(
     return NextResponse.json({ error: 'Admin permissions required' }, { status: 403 })
   }
 
+  /**
+   * The feature gate, deliberately, not the governance reader: the Access Control settings page is
+   * gated on the same plan check, so reading governance here would open the API for a past-due
+   * organization whose page still 404s. Restrictions keep applying through a dunning window —
+   * that is what the governance reader is for — but managing them follows the page.
+   */
   const entitled = await isOrganizationOnEnterprisePlan(organizationId)
   if (!entitled) {
     return NextResponse.json({ error: 'Access Control is an Enterprise feature' }, { status: 403 })

apps/sim/app/api/permission-groups/user/route.test.ts

@@ -21,6 +21,8 @@ vi.mock('@sim/platform-authz/workspace', async (importOriginal) => ({
 vi.mock('@/lib/workspaces/permissions/utils', () => ({ isOrganizationAdminOrOwner: mocks.admin }))
 vi.mock('@/lib/billing/core/subscription', () => ({
   isOrganizationOnEnterprisePlan: mocks.enterprise,
+  /** Permission resolution reads the governance axis; these tests drive both from one knob. */
+  isOrganizationGovernanceActive: mocks.enterprise,
 }))
 vi.mock('@/lib/permission-groups/resolve.server', async (importOriginal) => ({
   ...(await importOriginal<typeof import('@/lib/permission-groups/resolve.server')>()),
@@ -167,10 +169,11 @@ describe('user permission policy shared read', () => {
     expect(await (await get()).json()).toEqual({ ...unrestricted, entitled: true })
   })
   it('does not turn policy infrastructure failures into unrestricted access', async () => {
-    mocks.enterprise.mockImplementation(async (_organizationId, onError) => {
-      if (onError === 'throw') throw new Error('unavailable')
-      return false
-    })
+    /**
+     * The governance reader has no lenient mode — answering `false` on a failed read would mean
+     * "no permission group", which denies nothing — so a failure here is simply a rejection.
+     */
+    mocks.enterprise.mockRejectedValue(new Error('unavailable'))
     expect((await get()).status).toBe(500)
     await expect(
       readUserPermissionConfig.execute({ principal, input: { workspaceId: 'workspace' } })