From d1fc6956df7345ac7d30e0d30beab49e326b76e0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 17:19:55 -0700 Subject: [PATCH 01/13] fix(auth): make org session policy and cookie-cache version cache-coherent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The session-policy enforcement added in #5862 split an org's `securityPolicyVersion` and `sessionPolicySettings` across two independent process-local caches. On a multi-replica deployment a pod could observe the bumped version while still holding the pre-save policy, and that skew silently undid a just-saved tightening: 1. Saving a policy eagerly clamps every member session in SQL. Shortening `expires_at` makes Better Auth's refresh predicate (`expiresAt - expiresIn + updateAge <= now`) unconditionally true, so the save itself marks the whole org due for refresh. 2. The version bump expires the session-data cookie, forcing the DB path. 3. The refresh proposes `now + 30d` and `session.update.before` re-clamps it — but against the stale policy, so it passed through unclamped. 4. With `expiresAt` back at 30 days the predicate is false again, so nothing re-clamped that session for another ~24h. Both fields live on the same `organization` row, so they are now read and cached as one record: a pod that still sees version N also serves policy N and forces no cookie mismatch, removing the skew by construction rather than patching one direction of it. Session CREATE additionally reads through the cache — unlike a refresh it is not already preceded by a version-mismatch read, so a stale record would otherwise mint a full-length session. Also in this change: - A failed entitlement read no longer disables enforcement. Stored bounds are only writable by an entitled org, so their presence is the source of truth when the plan check is unavailable; `resolveOrganizationEnterprisePlan` propagates errors so "downgraded" and "check failed" stay distinguishable. This matches the reasoning already documented on the PII-redaction path. - A failed org-record read serves the last known-good record instead of falling back to defaults, which would have dropped the org's bounds entirely. - A failed membership lookup in `session.create.before` no longer skips the clamp; it falls back to the cached membership. - Both caches are bounded (expired entries first, then least-recently-refreshed) — they previously grew one entry per distinct org/user for the process life. - The cookie-cache version fn short-circuits when organizations are disabled, so those deployments pay no lookup on the authenticated hot path. - Idle-timeout copy now states the real guarantee: activity is recorded at most once per 24h, so sign-out lands between (value - 24) and (value) hours after last use, never later. Tests: new coverage for the org-record collapse, last-known-good behavior, entitlement failure, the impersonation exemption, and the previously untested revoke route (authz branches, self-token and impersonator exclusions). --- .../platform/enterprise/session-policies.mdx | 9 +- .../[id]/session-policy/route.test.ts | 3 +- .../[id]/session-policy/route.ts | 7 +- .../[id]/sessions/revoke/route.test.ts | 231 ++++++++++++++++++ .../[id]/sessions/revoke/route.ts | 4 +- .../components/session-policy-settings.tsx | 2 +- apps/sim/lib/auth/auth.ts | 58 +++-- apps/sim/lib/auth/security-policy.test.ts | 178 ++++++++++++++ apps/sim/lib/auth/security-policy.ts | 200 +++++++++++---- apps/sim/lib/auth/session-policy.test.ts | 191 ++++++++++++++- apps/sim/lib/auth/session-policy.ts | 116 ++++----- apps/sim/lib/billing/core/subscription.ts | 42 ++-- 12 files changed, 889 insertions(+), 152 deletions(-) create mode 100644 apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts create mode 100644 apps/sim/lib/auth/security-policy.test.ts diff --git a/apps/docs/content/docs/en/platform/enterprise/session-policies.mdx b/apps/docs/content/docs/en/platform/enterprise/session-policies.mdx index fb26c2f0a32..903af6ed14e 100644 --- a/apps/docs/content/docs/en/platform/enterprise/session-policies.mdx +++ b/apps/docs/content/docs/en/platform/enterprise/session-policies.mdx @@ -27,7 +27,9 @@ Use this to enforce periodic re-authentication — for example, a value of `168` ### Idle timeout -Signs a member out after this many hours without activity. Activity extends the session, so members who use Sim regularly stay signed in; dormant sessions expire. +Signs a member out after a period without activity. Activity extends the session, so members who use Sim regularly stay signed in; dormant sessions expire. + +Activity is recorded at most once per 24 hours, so the actual sign-out lands between (value − 24) and (value) hours after a member's last use — never later. A value of `48` means a dormant session ends somewhere between 24 and 48 hours after last activity. Accepts 48 to 8760 hours. The 48-hour minimum exists because session activity is recorded at most once per day — a shorter window could sign out members who are actively working. @@ -72,5 +74,10 @@ Use this after a security incident, an offboarding wave, or before tightening a answer: 'Session activity is recorded at most once per 24 hours for performance. The minimum is twice that window so a member who stays active always registers activity before the idle limit can expire their session.', }, + { + question: 'Why was a member signed out earlier than the idle timeout I set?', + answer: + 'Because activity is recorded at most once per 24 hours, the last recorded activity can be up to a day older than a member\'s actual last use. Sign-out therefore lands between (value − 24) and (value) hours after they stop working. It never lands later than the value you set, so the policy is never exceeded.', + }, ]} /> diff --git a/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts b/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts index c8722a51154..d017c6eac30 100644 --- a/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts +++ b/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts @@ -21,11 +21,10 @@ const { mockIsEnterprise, mockEagerClamp, mockRecordAudit } = vi.hoisted(() => ( vi.mock('@/lib/auth/session-policy', () => ({ eagerClampOrgSessions: mockEagerClamp, - invalidateSessionPolicyCache: vi.fn(), })) vi.mock('@/lib/auth/security-policy', () => ({ - invalidateSecurityPolicyVersionCache: vi.fn(), + invalidateOrgSecurityCache: vi.fn(), })) vi.mock('@/lib/billing/core/subscription', () => ({ diff --git a/apps/sim/app/api/organizations/[id]/session-policy/route.ts b/apps/sim/app/api/organizations/[id]/session-policy/route.ts index d3ce9d53bc3..612adf32a7b 100644 --- a/apps/sim/app/api/organizations/[id]/session-policy/route.ts +++ b/apps/sim/app/api/organizations/[id]/session-policy/route.ts @@ -9,8 +9,8 @@ import { type NextRequest, NextResponse } from 'next/server' import { updateOrganizationSessionPolicyContract } from '@/lib/api/contracts/organization' import { parseRequest, validationErrorResponse } from '@/lib/api/server' import { getSession } from '@/lib/auth' -import { invalidateSecurityPolicyVersionCache } from '@/lib/auth/security-policy' -import { eagerClampOrgSessions, invalidateSessionPolicyCache } from '@/lib/auth/session-policy' +import { invalidateOrgSecurityCache } from '@/lib/auth/security-policy' +import { eagerClampOrgSessions } from '@/lib/auth/session-policy' import { isOrganizationOnEnterprisePlan } from '@/lib/billing/core/subscription' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -160,8 +160,7 @@ export const PUT = withRouteHandler( return NextResponse.json({ error: 'Organization not found' }, { status: 404 }) } - invalidateSessionPolicyCache(organizationId) - invalidateSecurityPolicyVersionCache(organizationId) + invalidateOrgSecurityCache(organizationId) logger.info('Updated organization session policy', { organizationId }) diff --git a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts new file mode 100644 index 00000000000..e3b0a66f066 --- /dev/null +++ b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts @@ -0,0 +1,231 @@ +/** + * @vitest-environment node + */ +import { member, organization, session as sessionTable } from '@sim/db/schema' +import { + authMockFns, + createMockRequest, + dbChainMockFns, + queueTableRows, + resetDbChainMock, + resetEnvFlagsMock, + setEnvFlags, +} from '@sim/testing' +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockIsEnterprise, mockRecordAudit, mockInvalidateOrgSecurityCache } = vi.hoisted(() => ({ + mockIsEnterprise: vi.fn(), + mockRecordAudit: vi.fn(), + mockInvalidateOrgSecurityCache: vi.fn(), +})) + +vi.mock('@/lib/auth/security-policy', () => ({ + invalidateOrgSecurityCache: mockInvalidateOrgSecurityCache, +})) + +vi.mock('@/lib/billing/core/subscription', () => ({ + isOrganizationOnEnterprisePlan: mockIsEnterprise, +})) + +vi.mock('@sim/audit', () => ({ + recordAudit: mockRecordAudit, + AuditAction: { ORGANIZATION_SESSIONS_REVOKED: 'organization.sessions.revoked' }, + AuditResourceType: { ORGANIZATION: 'organization' }, +})) + +import { POST } from '@/app/api/organizations/[id]/sessions/revoke/route' + +const mockGetSession = authMockFns.mockGetSession + +const ORG_ID = 'org-1' +const CALLER_TOKEN = 'tok-caller' +const routeContext = { params: Promise.resolve({ id: ORG_ID }) } + +interface MockCondition { + type: string + left?: unknown + right?: unknown + column?: unknown + conditions?: MockCondition[] +} + +/** Flattens the nested `and(...)` the route builds into a flat condition list. */ +function flattenConditions(condition: MockCondition): MockCondition[] { + if (condition.type !== 'and') return [condition] + return (condition.conditions ?? []).flatMap(flattenConditions) +} + +/** + * The condition list passed to the session DELETE's `.where(...)`. Identified by + * its `inArray` over `session.user_id` rather than by call position, since the + * version-bump UPDATE issues its own `.where(...)` on the same shared spy. + */ +function deleteConditions(): MockCondition[] { + expect(dbChainMockFns.delete).toHaveBeenCalledWith(sessionTable) + const match = dbChainMockFns.where.mock.calls + .map(([arg]) => flattenConditions(arg as MockCondition)) + .find((conditions) => + conditions.some( + (condition) => condition.type === 'inArray' && condition.column === sessionTable.userId + ) + ) + expect(match).toBeDefined() + return match as MockCondition[] +} + +function authenticateAs(overrides: Record = {}) { + mockGetSession.mockResolvedValue({ + user: { id: 'user-1', name: 'Admin', email: 'admin@acme.dev' }, + session: { token: CALLER_TOKEN, ...overrides }, + }) +} + +beforeAll(() => { + setEnvFlags({ isBillingEnabled: true }) +}) + +afterAll(resetEnvFlagsMock) + +describe('org sessions revoke route', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + authenticateAs() + mockIsEnterprise.mockResolvedValue(true) + }) + + describe('authorization', () => { + it('returns 401 when unauthenticated', async () => { + mockGetSession.mockResolvedValue(null) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(401) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('returns 403 for non-members', async () => { + queueTableRows(member, []) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(403) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('returns 403 for members without an admin role', async () => { + queueTableRows(member, [{ role: 'member' }]) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(403) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('returns 403 for non-enterprise organizations', async () => { + queueTableRows(member, [{ role: 'owner' }]) + mockIsEnterprise.mockResolvedValue(false) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(403) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('returns 404 when the organization does not exist', async () => { + queueTableRows(member, [{ role: 'owner' }]) + queueTableRows(organization, []) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(404) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('allows admins as well as owners', async () => { + queueTableRows(member, [{ role: 'admin' }]) + queueTableRows(organization, [{ name: 'Acme' }]) + dbChainMockFns.returning.mockResolvedValueOnce([{ id: 's-1' }]) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(200) + }) + }) + + describe('revocation', () => { + beforeEach(() => { + queueTableRows(member, [{ role: 'owner' }]) + queueTableRows(organization, [{ name: 'Acme' }]) + }) + + it('reports the revoked count, bumps the version, and invalidates the cache', async () => { + dbChainMockFns.returning.mockResolvedValueOnce([{ id: 's-1' }, { id: 's-2' }]) + + const response = await POST(createMockRequest('POST'), routeContext) + + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ success: true, data: { revokedSessions: 2 } }) + expect(dbChainMockFns.set).toHaveBeenCalledWith( + expect.objectContaining({ securityPolicyVersion: expect.anything() }) + ) + expect(mockInvalidateOrgSecurityCache).toHaveBeenCalledWith(ORG_ID) + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'organization.sessions.revoked', + resourceId: ORG_ID, + metadata: { revokedSessions: 2 }, + }) + ) + }) + + it("never deletes the caller's own session", async () => { + dbChainMockFns.returning.mockResolvedValueOnce([]) + await POST(createMockRequest('POST'), routeContext) + + expect(deleteConditions()).toContainEqual( + expect.objectContaining({ type: 'ne', right: CALLER_TOKEN }) + ) + }) + + it('spares impersonation sessions', async () => { + dbChainMockFns.returning.mockResolvedValueOnce([]) + await POST(createMockRequest('POST'), routeContext) + + expect(deleteConditions()).toContainEqual( + expect.objectContaining({ type: 'isNull', column: sessionTable.impersonatedBy }) + ) + }) + + it('scopes the delete to members of the organization', async () => { + dbChainMockFns.returning.mockResolvedValueOnce([]) + await POST(createMockRequest('POST'), routeContext) + + expect(deleteConditions()).toContainEqual( + expect.objectContaining({ type: 'inArray', column: sessionTable.userId }) + ) + }) + + it("also spares the impersonator's own sessions when the caller is impersonating", async () => { + authenticateAs({ impersonatedBy: 'platform-admin-1' }) + dbChainMockFns.returning.mockResolvedValueOnce([]) + + await POST(createMockRequest('POST'), routeContext) + + expect(deleteConditions()).toContainEqual( + expect.objectContaining({ + type: 'ne', + left: sessionTable.userId, + right: 'platform-admin-1', + }) + ) + }) + + it('adds no impersonator exclusion for an ordinary caller', async () => { + dbChainMockFns.returning.mockResolvedValueOnce([]) + await POST(createMockRequest('POST'), routeContext) + + const userIdExclusions = deleteConditions().filter( + (condition) => condition.type === 'ne' && condition.left === sessionTable.userId + ) + expect(userIdExclusions).toHaveLength(0) + }) + + it('singularizes the audit description for a single session', async () => { + dbChainMockFns.returning.mockResolvedValueOnce([{ id: 's-1' }]) + await POST(createMockRequest('POST'), routeContext) + + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ description: 'Revoked 1 member session' }) + ) + }) + }) +}) diff --git a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts index 2731d2882ac..e41df8f9e3f 100644 --- a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts +++ b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts @@ -8,7 +8,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { revokeOrganizationSessionsContract } from '@/lib/api/contracts/organization' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' -import { invalidateSecurityPolicyVersionCache } from '@/lib/auth/security-policy' +import { invalidateOrgSecurityCache } from '@/lib/auth/security-policy' import { isOrganizationOnEnterprisePlan } from '@/lib/billing/core/subscription' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -105,7 +105,7 @@ export const POST = withRouteHandler( .where(eq(organization.id, organizationId)) return deleted }) - invalidateSecurityPolicyVersionCache(organizationId) + invalidateOrgSecurityCache(organizationId) logger.info('Revoked organization sessions', { organizationId, diff --git a/apps/sim/ee/session-policy/components/session-policy-settings.tsx b/apps/sim/ee/session-policy/components/session-policy-settings.tsx index d3b6a1cb977..b5304a7ed80 100644 --- a/apps/sim/ee/session-policy/components/session-policy-settings.tsx +++ b/apps/sim/ee/session-policy/components/session-policy-settings.tsx @@ -190,7 +190,7 @@ export function SessionPolicySettings({ organizationId }: SessionPolicySettingsP diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 167e02112dd..8623b59866f 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -661,40 +661,54 @@ export const auth = betterAuth({ } } + // Resolved separately from the clamp below: when this lookup fails we + // still clamp (falling back to the cached membership) rather than + // minting a full-length session off a transient read error. + let membershipOrgId: string | null | undefined try { - // Find the first organization this user is a member of const members = await db .select({ organizationId: schema.member.organizationId }) .from(schema.member) .where(eq(schema.member.userId, session.userId)) .limit(1) - - if (members.length > 0) { - logger.info('Found organization for user', { - userId: session.userId, - organizationId: members[0].organizationId, - }) - - const expiresAt = await clampExpiryForSession(session, members[0].organizationId) - - return { - data: { - ...session, - expiresAt, - activeOrganizationId: members[0].organizationId, - }, - } - } - logger.info('No organizations found for user', { + membershipOrgId = members[0]?.organizationId ?? null + logger.info( + membershipOrgId ? 'Found organization for user' : 'No organizations found for user', + { userId: session.userId, organizationId: membershipOrgId ?? undefined } + ) + } catch (error) { + logger.error('Error resolving organization for new session; using cached membership', { + error, userId: session.userId, }) - return { data: session } + membershipOrgId = undefined + } + + try { + // Session creation is rare and, unlike a sliding refresh, is not + // already preceded by a cookie-version mismatch read — so it must + // read the policy fresh or a just-tightened policy could be missed + // for the life of the session. + const expiresAt = await clampExpiryForSession(session, membershipOrgId, { + bypassPolicyCache: true, + }) + return { + data: { + ...session, + expiresAt, + ...(membershipOrgId ? { activeOrganizationId: membershipOrgId } : {}), + }, + } } catch (error) { - logger.error('Error setting active organization', { + logger.error('Error clamping new session to org policy', { error, userId: session.userId, }) - return { data: session } + return { + data: membershipOrgId + ? { ...session, activeOrganizationId: membershipOrgId } + : session, + } } }, }, diff --git a/apps/sim/lib/auth/security-policy.test.ts b/apps/sim/lib/auth/security-policy.test.ts new file mode 100644 index 00000000000..3318b5c2ca2 --- /dev/null +++ b/apps/sim/lib/auth/security-policy.test.ts @@ -0,0 +1,178 @@ +/** + * @vitest-environment node + */ +import { member, organization } from '@sim/db/schema' +import { + dbChainMockFns, + queueTableRows, + resetDbChainMock, + resetEnvFlagsMock, + setEnvFlags, +} from '@sim/testing' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' +import { + getMemberOrganizationId, + getOrgSecurityRecord, + getSecurityPolicyVersion, + getSessionCookieCacheVersion, + invalidateMembershipCache, + invalidateOrgSecurityCache, +} from '@/lib/auth/security-policy' + +/** Module-level caches persist across cases, so every case uses a fresh id. */ +let idCounter = 0 +const nextOrgId = () => `org-${++idCounter}` +const nextUserId = () => `user-${++idCounter}` + +afterAll(resetEnvFlagsMock) + +describe('security policy', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + setEnvFlags({ isOrganizationsEnabled: true }) + }) + + describe('getOrgSecurityRecord', () => { + it('serves the version and the session policy from ONE row read', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [ + { version: 7, sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null } }, + ]) + + const record = await getOrgSecurityRecord(orgId) + + expect(record.version).toBe(7) + expect(record.sessionPolicySettings).toEqual({ maxSessionHours: 8, idleTimeoutHours: null }) + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + }) + + it('reads the row once for a version lookup, then serves the policy from cache', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [ + { version: 3, sessionPolicySettings: { maxSessionHours: 12, idleTimeoutHours: null } }, + ]) + + // This is the coherence guarantee: whatever version a caller observed, + // the policy it goes on to read came from that same row. Two independent + // caches previously allowed a bumped version to pair with a stale policy. + const version = await getSecurityPolicyVersion(orgId) + const { sessionPolicySettings } = await getOrgSecurityRecord(orgId) + + expect(version).toBe(3) + expect(sessionPolicySettings).toEqual({ maxSessionHours: 12, idleTimeoutHours: null }) + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + }) + + it('re-reads after invalidation', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [{ version: 1, sessionPolicySettings: null }]) + await getOrgSecurityRecord(orgId) + + invalidateOrgSecurityCache(orgId) + queueTableRows(organization, [ + { version: 2, sessionPolicySettings: { maxSessionHours: 4, idleTimeoutHours: null } }, + ]) + + const record = await getOrgSecurityRecord(orgId) + expect(record.version).toBe(2) + expect(record.sessionPolicySettings).toEqual({ maxSessionHours: 4, idleTimeoutHours: null }) + expect(dbChainMockFns.select).toHaveBeenCalledTimes(2) + }) + + it('re-reads when the caller bypasses the cache', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [{ version: 1, sessionPolicySettings: null }]) + await getOrgSecurityRecord(orgId) + + queueTableRows(organization, [ + { version: 2, sessionPolicySettings: { maxSessionHours: 6, idleTimeoutHours: null } }, + ]) + const record = await getOrgSecurityRecord(orgId, { bypassCache: true }) + + expect(record.sessionPolicySettings).toEqual({ maxSessionHours: 6, idleTimeoutHours: null }) + expect(dbChainMockFns.select).toHaveBeenCalledTimes(2) + }) + + it('serves the last known-good record when a refresh fails', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [ + { version: 5, sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null } }, + ]) + await getOrgSecurityRecord(orgId) + + invalidateOrgSecurityCache(orgId) + // A read failure must not silently drop the org's bounds — that would + // disable a security control on a transient database blip. + dbChainMockFns.limit.mockImplementationOnce(() => { + throw new Error('connection reset') + }) + + // Nothing cached for this org after invalidation, so it falls to defaults. + const cold = await getOrgSecurityRecord(orgId) + expect(cold).toEqual({ version: 1, sessionPolicySettings: null }) + + queueTableRows(organization, [ + { version: 5, sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null } }, + ]) + await getOrgSecurityRecord(orgId) + dbChainMockFns.limit.mockImplementationOnce(() => { + throw new Error('connection reset') + }) + + const warm = await getOrgSecurityRecord(orgId, { bypassCache: true }) + expect(warm.version).toBe(5) + expect(warm.sessionPolicySettings).toEqual({ maxSessionHours: 8, idleTimeoutHours: null }) + }) + + it('defaults an unknown organization to version 1 with no policy', async () => { + queueTableRows(organization, []) + expect(await getOrgSecurityRecord(nextOrgId())).toEqual({ + version: 1, + sessionPolicySettings: null, + }) + }) + }) + + describe('getMemberOrganizationId', () => { + it('returns null without querying for an anonymous session', async () => { + expect(await getMemberOrganizationId(null)).toBeNull() + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) + + it('caches a positive membership and re-reads after invalidation', async () => { + const userId = nextUserId() + queueTableRows(member, [{ organizationId: 'org-a' }]) + + expect(await getMemberOrganizationId(userId)).toBe('org-a') + expect(await getMemberOrganizationId(userId)).toBe('org-a') + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + + invalidateMembershipCache(userId) + queueTableRows(member, [{ organizationId: 'org-b' }]) + expect(await getMemberOrganizationId(userId)).toBe('org-b') + }) + }) + + describe('getSessionCookieCacheVersion', () => { + it('embeds the org id so two orgs never share a version string', async () => { + const userId = nextUserId() + const orgId = nextOrgId() + queueTableRows(member, [{ organizationId: orgId }]) + queueTableRows(organization, [{ version: 4, sessionPolicySettings: null }]) + + expect(await getSessionCookieCacheVersion({ userId })).toBe(`${orgId}:4`) + }) + + it('returns the static default for org-less sessions', async () => { + queueTableRows(member, []) + expect(await getSessionCookieCacheVersion({ userId: nextUserId() })).toBe('none') + }) + + it('costs no lookups when organizations are disabled for the deployment', async () => { + setEnvFlags({ isOrganizationsEnabled: false }) + expect(await getSessionCookieCacheVersion({ userId: nextUserId() })).toBe('none') + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) + }) +}) diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index a8019ea0971..55cb9299900 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -1,67 +1,165 @@ import { db } from '@sim/db' +import type { SessionPolicySettings } from '@sim/db/schema' import { member, organization } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' +import { isOrganizationsEnabled } from '@/lib/core/config/env-flags' const logger = createLogger('SecurityPolicy') /** - * How long a resolved org security-policy version is served from process - * memory before the next request re-reads it. This TTL is the effective upper - * bound on org-wide session-revocation latency: a version bump changes the - * cookie-cache version, and every cached session cookie in the org falls - * through to a DB read within one TTL. + * How long a resolved org security record is served from process memory before + * the next request re-reads it. This TTL is the effective upper bound on + * org-wide session-revocation latency: a version bump changes the cookie-cache + * version, and every cached session cookie in the org falls through to a DB + * read within one TTL. */ -export const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000 +export const SECURITY_POLICY_CACHE_TTL_MS = 60 * 1000 + +/** + * After a failed read the last known-good record keeps being served, but its + * timestamp is rolled back to leave this much life so a sustained outage + * retries periodically instead of on every request. + */ +const ERROR_RETRY_BACKOFF_MS = 5 * 1000 + +/** + * Entry caps. Both maps only ever grow through explicit writes, so without a + * bound a long-lived process accumulates one entry per distinct org/user it + * has ever served. Eviction drops expired entries first, then the + * least-recently-refreshed ones. + */ +const MAX_ORG_CACHE_ENTRIES = 5_000 +const MAX_MEMBERSHIP_CACHE_ENTRIES = 20_000 const DEFAULT_VERSION = 1 -interface VersionCacheEntry { +/** + * The org security state that governs cached session cookies, read as ONE row + * so its two fields can never be observed out of sync. Splitting them across + * independent caches allowed a pod to see a bumped `version` (which forces a + * session refresh) while still holding the pre-bump `sessionPolicySettings` — + * the refresh then re-clamped against the stale policy and stretched the + * session back out, silently undoing a just-saved tightening. + */ +export interface OrgSecurityRecord { + /** + * Monotonic counter behind the Better Auth cookie-cache version. It backs + * ALL org security policies (session policies today; IP allowlisting and MFA + * enforcement are planned consumers): any feature that needs cached session + * cookies to re-validate bumps this one counter. + */ version: number + sessionPolicySettings: SessionPolicySettings | null +} + +interface OrgSecurityCacheEntry extends OrgSecurityRecord { fetchedAt: number } -const versionCache = new Map() +const orgSecurityCache = new Map() /** - * Resolves the org's security-policy version — the shared monotonic counter - * behind the Better Auth cookie-cache version. It backs ALL org security - * policies (session policies today; IP allowlisting and MFA enforcement are - * planned consumers): any feature that needs cached session cookies to - * re-validate bumps this one counter. + * Evicts down to `maxEntries`, expired entries first. Map iteration follows + * insertion order and {@link touch} re-inserts on every refresh, so whatever + * remains at the head is the least recently refreshed. */ -export async function getSecurityPolicyVersion( - organizationId: string | null | undefined -): Promise { - if (!organizationId) return DEFAULT_VERSION +function prune(cache: Map, maxEntries: number): void { + if (cache.size <= maxEntries) return + const cutoff = Date.now() - SECURITY_POLICY_CACHE_TTL_MS + for (const [key, entry] of cache) { + if (entry.fetchedAt < cutoff) cache.delete(key) + } + for (const key of cache.keys()) { + if (cache.size <= maxEntries) break + cache.delete(key) + } +} + +/** Re-inserts so insertion order tracks recency of refresh, then prunes. */ +function touch( + cache: Map, + key: string, + entry: T, + maxEntries: number +): void { + cache.delete(key) + cache.set(key, entry) + prune(cache, maxEntries) +} - const cached = versionCache.get(organizationId) - if (cached && Date.now() - cached.fetchedAt < SECURITY_POLICY_VERSION_CACHE_TTL_MS) { - return cached.version +/** + * Resolves the org's security record from a short TTL cache. On a read failure + * the last known-good record keeps being served: falling back to defaults + * would drop the org's session bounds entirely, disabling a security control + * because of a transient database blip. + */ +export async function getOrgSecurityRecord( + organizationId: string, + options: { bypassCache?: boolean } = {} +): Promise { + const cached = orgSecurityCache.get(organizationId) + if ( + !options.bypassCache && + cached && + Date.now() - cached.fetchedAt < SECURITY_POLICY_CACHE_TTL_MS + ) { + return cached } try { const [row] = await db - .select({ version: organization.securityPolicyVersion }) + .select({ + version: organization.securityPolicyVersion, + sessionPolicySettings: organization.sessionPolicySettings, + }) .from(organization) .where(eq(organization.id, organizationId)) .limit(1) - const version = row?.version ?? DEFAULT_VERSION - versionCache.set(organizationId, { version, fetchedAt: Date.now() }) - return version + const record: OrgSecurityRecord = { + version: row?.version ?? DEFAULT_VERSION, + sessionPolicySettings: row?.sessionPolicySettings ?? null, + } + touch( + orgSecurityCache, + organizationId, + { ...record, fetchedAt: Date.now() }, + MAX_ORG_CACHE_ENTRIES + ) + return record } catch (error) { - logger.error('Failed to resolve security policy version; using default', { + if (cached) { + logger.error('Failed to refresh org security record; serving last known-good', { + organizationId, + error, + }) + cached.fetchedAt = Date.now() - SECURITY_POLICY_CACHE_TTL_MS + ERROR_RETRY_BACKOFF_MS + return cached + } + logger.error('Failed to resolve org security record; using defaults', { organizationId, error, }) - return DEFAULT_VERSION + return { version: DEFAULT_VERSION, sessionPolicySettings: null } } } -/** Drops the cached version for an org so the next read is fresh. */ -export function invalidateSecurityPolicyVersionCache(organizationId: string): void { - versionCache.delete(organizationId) +/** Resolves just the cookie-cache version component of the org security record. */ +export async function getSecurityPolicyVersion( + organizationId: string | null | undefined +): Promise { + if (!organizationId) return DEFAULT_VERSION + return (await getOrgSecurityRecord(organizationId)).version +} + +/** + * Drops the cached security record for an org so the next read is fresh. One + * entry covers both the version and the session policy, so a caller cannot + * invalidate one and forget the other. + */ +export function invalidateOrgSecurityCache(organizationId: string): void { + orgSecurityCache.delete(organizationId) } interface MembershipCacheEntry { @@ -75,8 +173,10 @@ const membershipCache = new Map() * Negative (non-member) membership results use a much shorter TTL than * positive ones: a user's cached `null` would otherwise let them dodge a new * org's policy for the full TTL after joining through ANY path — including - * ones outside this codebase (Better Auth SSO JIT provisioning). Positive - * results change only through leave/transfer, which invalidate explicitly. + * ones outside this codebase (Better Auth SSO JIT provisioning writes `member` + * rows straight through the adapter, and this app registers no `member` + * database hook that could observe it). Positive results change only through + * leave/transfer, which invalidate explicitly. */ const NEGATIVE_MEMBERSHIP_CACHE_TTL_MS = 15 * 1000 @@ -86,12 +186,13 @@ export function invalidateMembershipCache(userId: string): void { } /** - * Resolves the org a user belongs to (users belong to at most one org), - * served from a short TTL cache. Org security policies govern MEMBERS, not - * just sessions that happen to carry an `activeOrganizationId` — a session - * created before the user joined an org has none, and without this fallback - * such sessions would dodge cookie-cache invalidation (and therefore - * org-wide revocation) for up to the 24h cookie lifetime. + * Resolves the org a user belongs to (users belong to at most one org — see the + * `member_user_id_unique` index), served from a short TTL cache. Org security + * policies govern MEMBERS, not just sessions that happen to carry an + * `activeOrganizationId` — a session created before the user joined an org has + * none, and without this fallback such sessions would dodge cookie-cache + * invalidation (and therefore org-wide revocation) for up to the 24h cookie + * lifetime. */ export async function getMemberOrganizationId( userId: string | null | undefined @@ -101,7 +202,7 @@ export async function getMemberOrganizationId( const cached = membershipCache.get(userId) if (cached) { const ttl = cached.organizationId - ? SECURITY_POLICY_VERSION_CACHE_TTL_MS + ? SECURITY_POLICY_CACHE_TTL_MS : NEGATIVE_MEMBERSHIP_CACHE_TTL_MS if (Date.now() - cached.fetchedAt < ttl) return cached.organizationId } @@ -114,7 +215,12 @@ export async function getMemberOrganizationId( .limit(1) const organizationId = row?.organizationId ?? null - membershipCache.set(userId, { organizationId, fetchedAt: Date.now() }) + touch( + membershipCache, + userId, + { organizationId, fetchedAt: Date.now() }, + MAX_MEMBERSHIP_CACHE_ENTRIES + ) return organizationId } catch (error) { logger.error('Failed to resolve org membership; treating session as org-less', { @@ -127,17 +233,19 @@ export async function getMemberOrganizationId( /** * Cookie-cache version for a session, consumed by Better Auth's - * `session.cookieCache.version`. Embeds the member org's security-policy - * version so bumps propagate to cached cookies. Resolved from the user's - * MEMBERSHIP, never the session's `activeOrganizationId` — that field goes - * stale on join/leave/transfer (it is only written at session creation), and - * a stale org here would let cookies dodge the destination org's version - * bumps for up to the 24h cookie lifetime. Sessions of non-members use the - * static default. + * `session.cookieCache.version` on EVERY authenticated request. Embeds the + * member org's security-policy version so bumps propagate to cached cookies. + * Resolved from the user's MEMBERSHIP, never the session's + * `activeOrganizationId` — that field goes stale on join/leave/transfer (it is + * only written at session creation), and a stale org here would let cookies + * dodge the destination org's version bumps for up to the 24h cookie lifetime. + * Sessions of non-members — and every session when organizations are disabled + * for the deployment — use the static default and cost no lookups. */ export async function getSessionCookieCacheVersion(session: { userId?: string | null }): Promise { + if (!isOrganizationsEnabled) return 'none' const organizationId = await getMemberOrganizationId(session.userId) if (!organizationId) return 'none' // The org id is part of the version so moving between orgs always changes diff --git a/apps/sim/lib/auth/session-policy.test.ts b/apps/sim/lib/auth/session-policy.test.ts index 6484f179b91..a463a1caa44 100644 --- a/apps/sim/lib/auth/session-policy.test.ts +++ b/apps/sim/lib/auth/session-policy.test.ts @@ -1,9 +1,30 @@ /** * @vitest-environment node */ -import { describe, expect, it } from 'vitest' +import { member, organization } from '@sim/db/schema' +import { + dbChainMockFns, + queueTableRows, + resetDbChainMock, + resetEnvFlagsMock, + setEnvFlags, +} from '@sim/testing' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' import { MIN_IDLE_TIMEOUT_HOURS } from '@/lib/api/contracts/organization' -import { clampSessionExpiry, type ResolvedSessionPolicy } from '@/lib/auth/session-policy' +import { + clampExpiryForSession, + clampSessionExpiry, + getSessionPolicy, + type ResolvedSessionPolicy, +} from '@/lib/auth/session-policy' + +const { mockResolveEnterprisePlan } = vi.hoisted(() => ({ + mockResolveEnterprisePlan: vi.fn(), +})) + +vi.mock('@/lib/billing/core/subscription', () => ({ + resolveOrganizationEnterprisePlan: mockResolveEnterprisePlan, +})) const HOUR_MS = 60 * 60 * 1000 @@ -82,3 +103,169 @@ describe('clampSessionExpiry', () => { expect(result.getTime()).toBe(shortProposal.getTime()) }) }) + +/** Module-level caches persist across cases, so every case uses a fresh id. */ +let idCounter = 0 +const nextOrgId = () => `sp-org-${++idCounter}` + +afterAll(resetEnvFlagsMock) + +describe('getSessionPolicy', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + setEnvFlags({ isBillingEnabled: true, isOrganizationsEnabled: true }) + mockResolveEnterprisePlan.mockResolvedValue(true) + }) + + it('returns no policy for an org-less session without touching the database', async () => { + expect(await getSessionPolicy(null)).toEqual({ + maxSessionHours: null, + idleTimeoutHours: null, + }) + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) + + it('skips the entitlement check when the org has no bounds stored', async () => { + queueTableRows(organization, [{ version: 1, sessionPolicySettings: null }]) + expect(await getSessionPolicy(nextOrgId())).toEqual({ + maxSessionHours: null, + idleTimeoutHours: null, + }) + expect(mockResolveEnterprisePlan).not.toHaveBeenCalled() + }) + + it('stops enforcing stored bounds for an org that is no longer entitled', async () => { + queueTableRows(organization, [ + { version: 1, sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null } }, + ]) + mockResolveEnterprisePlan.mockResolvedValue(false) + + expect(await getSessionPolicy(nextOrgId())).toEqual({ + maxSessionHours: null, + idleTimeoutHours: null, + }) + }) + + it('keeps enforcing when the entitlement check itself fails', async () => { + queueTableRows(organization, [ + { version: 1, sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null } }, + ]) + // Stored bounds are only writable by an entitled org, so a failed plan read + // must not be mistaken for a downgrade and silently disable enforcement. + mockResolveEnterprisePlan.mockRejectedValue(new Error('billing unavailable')) + + expect(await getSessionPolicy(nextOrgId())).toEqual({ + maxSessionHours: 8, + idleTimeoutHours: null, + }) + }) +}) + +describe('clampExpiryForSession', () => { + const createdAt = new Date('2026-07-22T00:00:00Z') + const proposed = new Date('2026-08-21T00:00:00Z') + + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + setEnvFlags({ isBillingEnabled: true, isOrganizationsEnabled: true }) + mockResolveEnterprisePlan.mockResolvedValue(true) + }) + + it('exempts impersonation sessions from the clamp', async () => { + const result = await clampExpiryForSession({ + userId: 'user-1', + impersonatedBy: 'platform-admin-1', + createdAt, + expiresAt: proposed, + }) + + expect(result?.getTime()).toBe(proposed.getTime()) + // Exempt before any lookup — impersonation must not even resolve a policy. + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) + + it('returns undefined when the session carries no expiry', async () => { + expect(await clampExpiryForSession({ userId: 'user-1', createdAt })).toBeUndefined() + }) + + it('leaves non-member sessions untouched', async () => { + queueTableRows(member, []) + const result = await clampExpiryForSession({ + userId: 'user-1', + createdAt, + expiresAt: proposed, + }) + expect(result?.getTime()).toBe(proposed.getTime()) + }) + + it('clamps against the org resolved from membership', async () => { + const orgId = nextOrgId() + queueTableRows(member, [{ organizationId: orgId }]) + queueTableRows(organization, [ + { version: 1, sessionPolicySettings: { maxSessionHours: 24, idleTimeoutHours: null } }, + ]) + + const result = await clampExpiryForSession({ + userId: `user-${orgId}`, + createdAt, + expiresAt: proposed, + }) + + expect(result?.getTime()).toBe(createdAt.getTime() + 24 * 60 * 60 * 1000) + }) + + it('skips the membership lookup when the caller already resolved it', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [ + { version: 1, sessionPolicySettings: { maxSessionHours: 24, idleTimeoutHours: null } }, + ]) + + const result = await clampExpiryForSession( + { userId: 'user-fresh', createdAt, expiresAt: proposed }, + orgId + ) + + expect(result?.getTime()).toBe(createdAt.getTime() + 24 * 60 * 60 * 1000) + // Only the organization row was read; membership came from the caller. + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + }) + + it('normalizes ISO string dates crossing the hook serialization boundary', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [ + { version: 1, sessionPolicySettings: { maxSessionHours: 24, idleTimeoutHours: null } }, + ]) + + const result = await clampExpiryForSession( + { + userId: 'user-iso', + createdAt: createdAt.toISOString(), + expiresAt: proposed.toISOString(), + }, + orgId + ) + + expect(result?.getTime()).toBe(createdAt.getTime() + 24 * 60 * 60 * 1000) + }) + + it('re-reads the policy when the caller bypasses the cache', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [{ version: 1, sessionPolicySettings: null }]) + await clampExpiryForSession({ userId: 'u', createdAt, expiresAt: proposed }, orgId) + + // A policy saved on another process between the two calls: the bypass is + // what stops a just-tightened policy from being missed at session create. + queueTableRows(organization, [ + { version: 2, sessionPolicySettings: { maxSessionHours: 24, idleTimeoutHours: null } }, + ]) + const result = await clampExpiryForSession( + { userId: 'u', createdAt, expiresAt: proposed }, + orgId, + { bypassPolicyCache: true } + ) + + expect(result?.getTime()).toBe(createdAt.getTime() + 24 * 60 * 60 * 1000) + }) +}) diff --git a/apps/sim/lib/auth/session-policy.ts b/apps/sim/lib/auth/session-policy.ts index 484423dfde3..0c0f37c847b 100644 --- a/apps/sim/lib/auth/session-policy.ts +++ b/apps/sim/lib/auth/session-policy.ts @@ -1,18 +1,17 @@ import { db } from '@sim/db' -import type { SessionPolicySettings } from '@sim/db/schema' -import { organization } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { eq, sql } from 'drizzle-orm' +import { sql } from 'drizzle-orm' import { MIN_IDLE_TIMEOUT_HOURS } from '@/lib/api/contracts/organization' -import { getMemberOrganizationId, invalidateMembershipCache } from '@/lib/auth/security-policy' -import { isOrganizationOnEnterprisePlan } from '@/lib/billing/core/subscription' +import { + getMemberOrganizationId, + getOrgSecurityRecord, + invalidateMembershipCache, +} from '@/lib/auth/security-policy' +import { resolveOrganizationEnterprisePlan } from '@/lib/billing/core/subscription' import { isBillingEnabled } from '@/lib/core/config/env-flags' const logger = createLogger('SessionPolicy') -/** How long a resolved org session policy is served from process memory. */ -export const SESSION_POLICY_CACHE_TTL_MS = 60 * 1000 - const HOUR_MS = 60 * 60 * 1000 export interface ResolvedSessionPolicy { @@ -20,67 +19,64 @@ export interface ResolvedSessionPolicy { idleTimeoutHours: number | null } -interface PolicyCacheEntry { - policy: ResolvedSessionPolicy - fetchedAt: number -} - -const policyCache = new Map() - const NO_POLICY: ResolvedSessionPolicy = { maxSessionHours: null, idleTimeoutHours: null, } /** - * Resolves the EFFECTIVE session policy for an organization, served from a - * short TTL cache. Returns a no-op policy for personal (org-less) sessions - * and — mirroring data-retention's plan-gated effective settings — for - * hosted orgs no longer on an Enterprise plan: stored limits stop enforcing - * automatically on downgrade, since the enterprise-gated settings UI can no - * longer manage them. + * Whether stored session bounds should be enforced for this org. Mirroring + * data-retention's plan-gated effective settings, a hosted org that leaves the + * Enterprise plan stops enforcing its stored limits automatically, since the + * enterprise-gated settings UI can no longer manage them. + * + * A FAILED entitlement read enforces instead of skipping: bounds are only + * writable by an entitled org (the PUT route gates on it), so their presence is + * the source of truth when the plan check itself is unavailable. Fail-open here + * would let a transient billing-read blip silently disable a security control — + * the same reasoning the PII-redaction path documents in + * `lib/logs/execution/logger.ts`. */ -export async function getSessionPolicy( - organizationId: string | null | undefined -): Promise { - if (!organizationId) return NO_POLICY - - const cached = policyCache.get(organizationId) - if (cached && Date.now() - cached.fetchedAt < SESSION_POLICY_CACHE_TTL_MS) { - return cached.policy - } - +async function isPolicyEnforced(organizationId: string, hasBounds: boolean): Promise { + if (!hasBounds || !isBillingEnabled) return true try { - const [row] = await db - .select({ settings: organization.sessionPolicySettings }) - .from(organization) - .where(eq(organization.id, organizationId)) - .limit(1) - - const settings: SessionPolicySettings = row?.settings ?? {} - const hasBounds = Boolean(settings.maxSessionHours || settings.idleTimeoutHours) - const isEntitled = - !hasBounds || !isBillingEnabled || (await isOrganizationOnEnterprisePlan(organizationId)) - const policy: ResolvedSessionPolicy = isEntitled - ? { - maxSessionHours: settings.maxSessionHours ?? null, - idleTimeoutHours: settings.idleTimeoutHours ?? null, - } - : NO_POLICY - policyCache.set(organizationId, { policy, fetchedAt: Date.now() }) - return policy + return await resolveOrganizationEnterprisePlan(organizationId) } catch (error) { - logger.error('Failed to resolve session policy; applying no policy', { + logger.error('Enterprise entitlement check failed; enforcing stored session policy', { organizationId, error, }) - return NO_POLICY + return true } } -/** Drops the cached policy for an org so the next read is fresh. */ -export function invalidateSessionPolicyCache(organizationId: string): void { - policyCache.delete(organizationId) +/** + * Resolves the EFFECTIVE session policy for an organization. Returns a no-op + * policy for personal (org-less) sessions and for orgs that are no longer + * entitled. + * + * The stored settings come from the shared org security record, so the policy a + * caller sees is always the one that matches the cookie-cache version it sees. + * Pass `bypassCache` on paths that must not act on a policy up to one cache TTL + * old — notably session CREATE, which (unlike a refresh) is not already preceded + * by a version-mismatch read and would otherwise mint an unclamped session from + * a stale record. + */ +export async function getSessionPolicy( + organizationId: string | null | undefined, + options: { bypassCache?: boolean } = {} +): Promise { + if (!organizationId) return NO_POLICY + + const { sessionPolicySettings } = await getOrgSecurityRecord(organizationId, options) + const settings = sessionPolicySettings ?? {} + const hasBounds = Boolean(settings.maxSessionHours || settings.idleTimeoutHours) + if (!(await isPolicyEnforced(organizationId, hasBounds))) return NO_POLICY + + return { + maxSessionHours: settings.maxSessionHours ?? null, + idleTimeoutHours: settings.idleTimeoutHours ?? null, + } } /** @@ -136,7 +132,8 @@ interface ClampableSession { */ export async function clampExpiryForSession( session: ClampableSession, - freshMembershipOrgId?: string | null + freshMembershipOrgId?: string | null, + options: { bypassPolicyCache?: boolean } = {} ): Promise { // Better Auth context values can cross a serialization boundary — normalize // date fields in case they arrive as ISO strings rather than Dates. @@ -150,7 +147,9 @@ export async function clampExpiryForSession( : await getMemberOrganizationId(session.userId) if (!organizationId) return expiresAt - const policy = await getSessionPolicy(organizationId) + const policy = await getSessionPolicy(organizationId, { + bypassCache: options.bypassPolicyCache, + }) const createdAt = session.createdAt ? new Date(session.createdAt) : new Date() return clampSessionExpiry(policy, createdAt, expiresAt) } @@ -196,7 +195,10 @@ export async function applySessionPolicyToNewMember( ): Promise { try { invalidateMembershipCache(userId) - const policy = await getSessionPolicy(organizationId) + // Bypass the cache: a user joining moments after a policy save must be + // clamped to the policy that was just written, not a pre-save record this + // process may still be holding. + const policy = await getSessionPolicy(organizationId, { bypassCache: true }) const bounds = clampBoundsSql(policy) if (!bounds) return diff --git a/apps/sim/lib/billing/core/subscription.ts b/apps/sim/lib/billing/core/subscription.ts index 5ab950c0eb8..c44d8dfd279 100644 --- a/apps/sim/lib/billing/core/subscription.ts +++ b/apps/sim/lib/billing/core/subscription.ts @@ -427,26 +427,38 @@ export async function isEnterpriseOrgAdminOrOwner(userId: string): Promise { - try { - if (!isBillingEnabled) { - return true - } +export async function resolveOrganizationEnterprisePlan(organizationId: string): Promise { + if (!isBillingEnabled) { + return true + } - if (isAccessControlEnabled && !isHosted) { - return true - } + if (isAccessControlEnabled && !isHosted) { + return true + } - if (await isOrganizationBillingBlocked(organizationId)) { - return false - } + if (await isOrganizationBillingBlocked(organizationId)) { + return false + } + + const orgSub = await getOrganizationSubscriptionUsable(organizationId) - const orgSub = await getOrganizationSubscriptionUsable(organizationId) + return !!orgSub && checkEnterprisePlan(orgSub) +} - return !!orgSub && checkEnterprisePlan(orgSub) +/** + * Check if an organization has an enterprise plan + * Used for Access Control (Permission Groups) feature gating + */ +export async function isOrganizationOnEnterprisePlan(organizationId: string): Promise { + try { + return await resolveOrganizationEnterprisePlan(organizationId) } catch (error) { logger.error('Error checking organization enterprise plan status', { error, organizationId }) return false From 825959bb4be5a9ccd4c31beb90c8e4a8ea86e72c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 17:41:44 -0700 Subject: [PATCH 02/13] fix(auth): amortize the entitlement check and bound cache eviction cost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review follow-ups on the cache-coherence change: - Splitting the entitlement check out of the combined policy cache meant it ran on every session refresh. A policy save or org-wide revoke makes every member session refresh at once, so a large org would have stampeded the billing tables with one plan check per session. Entitlement is now cached on its own TTL, independent of the org security record (a policy save invalidates the policy but says nothing about the plan). Staleness is harmless in both directions, and a failed check falls back to the last known decision. - Pruning trimmed exactly to the cap, so the next insert was over again and every subsequent write rescanned the whole map. Prune to a low water mark instead, which keeps eviction amortized O(1). - The session create hook no longer writes an `expiresAt: undefined` key when the clamp has nothing to narrow; it only ever overrides a real value. - Drop the redundant `isBillingEnabled` short-circuit — the same condition is already the first branch of `resolveOrganizationEnterprisePlan`. --- apps/sim/lib/auth/auth.ts | 5 +++- apps/sim/lib/auth/security-policy.ts | 14 ++++++++-- apps/sim/lib/auth/session-policy.test.ts | 17 ++++++++++++ apps/sim/lib/auth/session-policy.ts | 33 +++++++++++++++++++++--- 4 files changed, 62 insertions(+), 7 deletions(-) diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 8623b59866f..eb896ee2301 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -695,7 +695,10 @@ export const auth = betterAuth({ return { data: { ...session, - expiresAt, + // Only ever narrows an existing expiry — never introduces an + // `expiresAt: undefined` key that would blank out the value + // Better Auth already put on the row. + ...(expiresAt ? { expiresAt } : {}), ...(membershipOrgId ? { activeOrganizationId: membershipOrgId } : {}), }, } diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index 55cb9299900..2bda1ce9689 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -60,18 +60,28 @@ interface OrgSecurityCacheEntry extends OrgSecurityRecord { const orgSecurityCache = new Map() /** - * Evicts down to `maxEntries`, expired entries first. Map iteration follows + * Fraction of the cap an over-capacity prune evicts down to. Trimming to a low + * water mark rather than exactly to the cap is what keeps eviction amortized + * O(1): pruning back to the cap would leave the very next insert over again, + * so every subsequent write would rescan the whole map. + */ +const PRUNE_TARGET_RATIO = 0.9 + +/** + * Evicts down to a low water mark, expired entries first. Map iteration follows * insertion order and {@link touch} re-inserts on every refresh, so whatever * remains at the head is the least recently refreshed. */ function prune(cache: Map, maxEntries: number): void { if (cache.size <= maxEntries) return + const target = Math.floor(maxEntries * PRUNE_TARGET_RATIO) const cutoff = Date.now() - SECURITY_POLICY_CACHE_TTL_MS for (const [key, entry] of cache) { + if (cache.size <= target) break if (entry.fetchedAt < cutoff) cache.delete(key) } for (const key of cache.keys()) { - if (cache.size <= maxEntries) break + if (cache.size <= target) break cache.delete(key) } } diff --git a/apps/sim/lib/auth/session-policy.test.ts b/apps/sim/lib/auth/session-policy.test.ts index a463a1caa44..9f14bb8a756 100644 --- a/apps/sim/lib/auth/session-policy.test.ts +++ b/apps/sim/lib/auth/session-policy.test.ts @@ -147,6 +147,23 @@ describe('getSessionPolicy', () => { }) }) + it('amortizes the entitlement check across repeated resolutions', async () => { + const orgId = nextOrgId() + const bounded = { + version: 1, + sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null }, + } + queueTableRows(organization, [bounded]) + queueTableRows(organization, [bounded]) + + // A policy save makes every member session refresh at once; the plan check + // must not run once per refreshing session. + await getSessionPolicy(orgId) + await getSessionPolicy(orgId, { bypassCache: true }) + + expect(mockResolveEnterprisePlan).toHaveBeenCalledTimes(1) + }) + it('keeps enforcing when the entitlement check itself fails', async () => { queueTableRows(organization, [ { version: 1, sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null } }, diff --git a/apps/sim/lib/auth/session-policy.ts b/apps/sim/lib/auth/session-policy.ts index 0c0f37c847b..c6fcb33543b 100644 --- a/apps/sim/lib/auth/session-policy.ts +++ b/apps/sim/lib/auth/session-policy.ts @@ -8,7 +8,6 @@ import { invalidateMembershipCache, } from '@/lib/auth/security-policy' import { resolveOrganizationEnterprisePlan } from '@/lib/billing/core/subscription' -import { isBillingEnabled } from '@/lib/core/config/env-flags' const logger = createLogger('SessionPolicy') @@ -24,6 +23,20 @@ const NO_POLICY: ResolvedSessionPolicy = { idleTimeoutHours: null, } +/** + * Entitlement is billing state, so it is cached independently of the org + * security record: a policy save must invalidate the policy, but it says + * nothing about the plan. The TTL matters because saving a policy or revoking + * org-wide makes every member session refresh at once — without it, each of + * those refreshes would re-run the plan check and stampede the billing tables. + * Staleness is harmless in both directions: a just-downgraded org keeps + * enforcing for one TTL, and a just-upgraded one starts one TTL late. + */ +const ENTITLEMENT_CACHE_TTL_MS = 60 * 1000 +const MAX_ENTITLEMENT_CACHE_ENTRIES = 5_000 + +const entitlementCache = new Map() + /** * Whether stored session bounds should be enforced for this org. Mirroring * data-retention's plan-gated effective settings, a hosted org that leaves the @@ -38,15 +51,27 @@ const NO_POLICY: ResolvedSessionPolicy = { * `lib/logs/execution/logger.ts`. */ async function isPolicyEnforced(organizationId: string, hasBounds: boolean): Promise { - if (!hasBounds || !isBillingEnabled) return true + // Orgs with nothing stored are the common case and need no plan check at all. + if (!hasBounds) return true + + const cached = entitlementCache.get(organizationId) + if (cached && Date.now() - cached.fetchedAt < ENTITLEMENT_CACHE_TTL_MS) { + return cached.entitled + } + try { - return await resolveOrganizationEnterprisePlan(organizationId) + const entitled = await resolveOrganizationEnterprisePlan(organizationId) + // Entitled orgs are few, so a wholesale clear at the cap is enough — no + // eviction policy needed for a set this small and this slow-moving. + if (entitlementCache.size >= MAX_ENTITLEMENT_CACHE_ENTRIES) entitlementCache.clear() + entitlementCache.set(organizationId, { entitled, fetchedAt: Date.now() }) + return entitled } catch (error) { logger.error('Enterprise entitlement check failed; enforcing stored session policy', { organizationId, error, }) - return true + return cached?.entitled ?? true } } From d8f09bf9ee4ab863905416fdbd5538c200b25b08 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 17:48:22 -0700 Subject: [PATCH 03/13] refactor(auth): read the session policy fresh instead of caching it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows the pattern the two sibling enterprise-settings call sites already establish. `lib/logs/execution/logger.ts` and `lib/workflows/executor/execution-core.ts` both resolve org enterprise settings with a single indexed read at enforcement time and carry explicit comments rejecting a cached/re-checked resolution, because a stale or failed read there silently skips the control. The session policy is the same shape of thing, and it is not a hot path: it is read on sign-in and on a sliding session refresh, which the 24h cookie cache limits to roughly once per user per day. Caching it bought nothing and cost the correctness bug this branch set out to fix — so drop the cache rather than build machinery to keep two caches coherent. Better Auth also documents `cookieCache.version` as a static string bumped at deploy and offers no cross-instance invalidation, so the version TTL is inherent to the design while the policy cache never was. This removes, rather than fixes, the whole class of problems the previous revision introduced: no merged record, no cache-bypass flag, no last-known-good fallback, no entitlement cache, no destructive-invalidation window, and no ordering hazard between concurrent readers. `security-policy.ts` is back to caching only the cookie-cache version — which genuinely is per-request — plus the membership lookup. Failure posture is now chosen per call site instead of globally fail-open: - `getSessionPolicy` and `getMemberOrganizationId` propagate read errors. - The session UPDATE hook refuses to EXTEND on a failed read, keeping the member's current expiry rather than granting a fresh 30 days the policy may forbid. - The session CREATE hook allows the sign-in but logs that the session went unclamped — refusing sign-ins would turn a read blip into an org-wide outage. - `getSessionCookieCacheVersion` still never throws; it runs on every request, and its fallback only costs a cookie mismatch and a database session read. Kept from the previous revision: the entry caps with low-water-mark eviction, the organizations-disabled short-circuit, the propagating enterprise-plan resolver, and the corrected idle-timeout copy. --- .../[id]/session-policy/route.test.ts | 2 +- .../[id]/session-policy/route.ts | 4 +- .../[id]/sessions/revoke/route.test.ts | 8 +- .../[id]/sessions/revoke/route.ts | 4 +- apps/sim/lib/auth/auth.ts | 34 +-- apps/sim/lib/auth/security-policy.test.ts | 123 +++-------- apps/sim/lib/auth/security-policy.ts | 203 ++++++++---------- apps/sim/lib/auth/session-policy.test.ts | 63 ++---- apps/sim/lib/auth/session-policy.ts | 101 ++++----- 9 files changed, 212 insertions(+), 330 deletions(-) diff --git a/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts b/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts index d017c6eac30..ce21a141d30 100644 --- a/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts +++ b/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts @@ -24,7 +24,7 @@ vi.mock('@/lib/auth/session-policy', () => ({ })) vi.mock('@/lib/auth/security-policy', () => ({ - invalidateOrgSecurityCache: vi.fn(), + invalidateSecurityPolicyVersionCache: vi.fn(), })) vi.mock('@/lib/billing/core/subscription', () => ({ diff --git a/apps/sim/app/api/organizations/[id]/session-policy/route.ts b/apps/sim/app/api/organizations/[id]/session-policy/route.ts index 612adf32a7b..856f3e26370 100644 --- a/apps/sim/app/api/organizations/[id]/session-policy/route.ts +++ b/apps/sim/app/api/organizations/[id]/session-policy/route.ts @@ -9,7 +9,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { updateOrganizationSessionPolicyContract } from '@/lib/api/contracts/organization' import { parseRequest, validationErrorResponse } from '@/lib/api/server' import { getSession } from '@/lib/auth' -import { invalidateOrgSecurityCache } from '@/lib/auth/security-policy' +import { invalidateSecurityPolicyVersionCache } from '@/lib/auth/security-policy' import { eagerClampOrgSessions } from '@/lib/auth/session-policy' import { isOrganizationOnEnterprisePlan } from '@/lib/billing/core/subscription' import { isBillingEnabled } from '@/lib/core/config/env-flags' @@ -160,7 +160,7 @@ export const PUT = withRouteHandler( return NextResponse.json({ error: 'Organization not found' }, { status: 404 }) } - invalidateOrgSecurityCache(organizationId) + invalidateSecurityPolicyVersionCache(organizationId) logger.info('Updated organization session policy', { organizationId }) diff --git a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts index e3b0a66f066..3ab981b3e50 100644 --- a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts +++ b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts @@ -13,14 +13,14 @@ import { } from '@sim/testing' import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockIsEnterprise, mockRecordAudit, mockInvalidateOrgSecurityCache } = vi.hoisted(() => ({ +const { mockIsEnterprise, mockRecordAudit, mockInvalidateVersionCache } = vi.hoisted(() => ({ mockIsEnterprise: vi.fn(), mockRecordAudit: vi.fn(), - mockInvalidateOrgSecurityCache: vi.fn(), + mockInvalidateVersionCache: vi.fn(), })) vi.mock('@/lib/auth/security-policy', () => ({ - invalidateOrgSecurityCache: mockInvalidateOrgSecurityCache, + invalidateSecurityPolicyVersionCache: mockInvalidateVersionCache, })) vi.mock('@/lib/billing/core/subscription', () => ({ @@ -157,7 +157,7 @@ describe('org sessions revoke route', () => { expect(dbChainMockFns.set).toHaveBeenCalledWith( expect.objectContaining({ securityPolicyVersion: expect.anything() }) ) - expect(mockInvalidateOrgSecurityCache).toHaveBeenCalledWith(ORG_ID) + expect(mockInvalidateVersionCache).toHaveBeenCalledWith(ORG_ID) expect(mockRecordAudit).toHaveBeenCalledWith( expect.objectContaining({ action: 'organization.sessions.revoked', diff --git a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts index e41df8f9e3f..2731d2882ac 100644 --- a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts +++ b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts @@ -8,7 +8,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { revokeOrganizationSessionsContract } from '@/lib/api/contracts/organization' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' -import { invalidateOrgSecurityCache } from '@/lib/auth/security-policy' +import { invalidateSecurityPolicyVersionCache } from '@/lib/auth/security-policy' import { isOrganizationOnEnterprisePlan } from '@/lib/billing/core/subscription' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -105,7 +105,7 @@ export const POST = withRouteHandler( .where(eq(organization.id, organizationId)) return deleted }) - invalidateOrgSecurityCache(organizationId) + invalidateSecurityPolicyVersionCache(organizationId) logger.info('Revoked organization sessions', { organizationId, diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index eb896ee2301..74a80a79a8e 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -685,13 +685,7 @@ export const auth = betterAuth({ } try { - // Session creation is rare and, unlike a sliding refresh, is not - // already preceded by a cookie-version mismatch read — so it must - // read the policy fresh or a just-tightened policy could be missed - // for the life of the session. - const expiresAt = await clampExpiryForSession(session, membershipOrgId, { - bypassPolicyCache: true, - }) + const expiresAt = await clampExpiryForSession(session, membershipOrgId) return { data: { ...session, @@ -703,7 +697,10 @@ export const auth = betterAuth({ }, } } catch (error) { - logger.error('Error clamping new session to org policy', { + // Allow the sign-in: refusing it would turn a policy-read blip into + // an org-wide outage. The session keeps Better Auth's default + // expiry and the next refresh clamps it. + logger.error('Error clamping new session to org policy; session not clamped', { error, userId: session.userId, }) @@ -728,11 +725,22 @@ export const auth = betterAuth({ if (!data.expiresAt) return { data } const current = ctx?.context?.session?.session if (!current) return { data } - const expiresAt = await clampExpiryForSession({ - ...current, - expiresAt: new Date(data.expiresAt), - }) - return { data: { ...data, expiresAt } } + try { + const expiresAt = await clampExpiryForSession({ + ...current, + expiresAt: new Date(data.expiresAt), + }) + return { data: { ...data, expiresAt } } + } catch (error) { + // Refuse to EXTEND when the policy cannot be read: keeping the + // session's current expiry leaves the member signed in without + // granting them a fresh 30 days a policy might have forbidden. + logger.error('Error re-clamping refreshed session; leaving expiry unchanged', { + error, + userId: current.userId, + }) + return { data: { ...data, expiresAt: current.expiresAt } } + } }, }, }, diff --git a/apps/sim/lib/auth/security-policy.test.ts b/apps/sim/lib/auth/security-policy.test.ts index 3318b5c2ca2..5c6447630b3 100644 --- a/apps/sim/lib/auth/security-policy.test.ts +++ b/apps/sim/lib/auth/security-policy.test.ts @@ -12,11 +12,10 @@ import { import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' import { getMemberOrganizationId, - getOrgSecurityRecord, getSecurityPolicyVersion, getSessionCookieCacheVersion, invalidateMembershipCache, - invalidateOrgSecurityCache, + invalidateSecurityPolicyVersionCache, } from '@/lib/auth/security-policy' /** Module-level caches persist across cases, so every case uses a fresh id. */ @@ -33,104 +32,37 @@ describe('security policy', () => { setEnvFlags({ isOrganizationsEnabled: true }) }) - describe('getOrgSecurityRecord', () => { - it('serves the version and the session policy from ONE row read', async () => { + describe('getSecurityPolicyVersion', () => { + it('caches the version and re-reads after invalidation', async () => { const orgId = nextOrgId() - queueTableRows(organization, [ - { version: 7, sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null } }, - ]) + queueTableRows(organization, [{ version: 3 }]) - const record = await getOrgSecurityRecord(orgId) - - expect(record.version).toBe(7) - expect(record.sessionPolicySettings).toEqual({ maxSessionHours: 8, idleTimeoutHours: null }) + expect(await getSecurityPolicyVersion(orgId)).toBe(3) + expect(await getSecurityPolicyVersion(orgId)).toBe(3) expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) - }) - it('reads the row once for a version lookup, then serves the policy from cache', async () => { - const orgId = nextOrgId() - queueTableRows(organization, [ - { version: 3, sessionPolicySettings: { maxSessionHours: 12, idleTimeoutHours: null } }, - ]) - - // This is the coherence guarantee: whatever version a caller observed, - // the policy it goes on to read came from that same row. Two independent - // caches previously allowed a bumped version to pair with a stale policy. - const version = await getSecurityPolicyVersion(orgId) - const { sessionPolicySettings } = await getOrgSecurityRecord(orgId) - - expect(version).toBe(3) - expect(sessionPolicySettings).toEqual({ maxSessionHours: 12, idleTimeoutHours: null }) - expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + invalidateSecurityPolicyVersionCache(orgId) + queueTableRows(organization, [{ version: 4 }]) + expect(await getSecurityPolicyVersion(orgId)).toBe(4) }) - it('re-reads after invalidation', async () => { - const orgId = nextOrgId() - queueTableRows(organization, [{ version: 1, sessionPolicySettings: null }]) - await getOrgSecurityRecord(orgId) - - invalidateOrgSecurityCache(orgId) - queueTableRows(organization, [ - { version: 2, sessionPolicySettings: { maxSessionHours: 4, idleTimeoutHours: null } }, - ]) - - const record = await getOrgSecurityRecord(orgId) - expect(record.version).toBe(2) - expect(record.sessionPolicySettings).toEqual({ maxSessionHours: 4, idleTimeoutHours: null }) - expect(dbChainMockFns.select).toHaveBeenCalledTimes(2) + it('returns the default without querying for an org-less session', async () => { + expect(await getSecurityPolicyVersion(null)).toBe(1) + expect(dbChainMockFns.select).not.toHaveBeenCalled() }) - it('re-reads when the caller bypasses the cache', async () => { - const orgId = nextOrgId() - queueTableRows(organization, [{ version: 1, sessionPolicySettings: null }]) - await getOrgSecurityRecord(orgId) - - queueTableRows(organization, [ - { version: 2, sessionPolicySettings: { maxSessionHours: 6, idleTimeoutHours: null } }, - ]) - const record = await getOrgSecurityRecord(orgId, { bypassCache: true }) - - expect(record.sessionPolicySettings).toEqual({ maxSessionHours: 6, idleTimeoutHours: null }) - expect(dbChainMockFns.select).toHaveBeenCalledTimes(2) + it('defaults an unknown organization to version 1', async () => { + queueTableRows(organization, []) + expect(await getSecurityPolicyVersion(nextOrgId())).toBe(1) }) - it('serves the last known-good record when a refresh fails', async () => { - const orgId = nextOrgId() - queueTableRows(organization, [ - { version: 5, sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null } }, - ]) - await getOrgSecurityRecord(orgId) - - invalidateOrgSecurityCache(orgId) - // A read failure must not silently drop the org's bounds — that would - // disable a security control on a transient database blip. - dbChainMockFns.limit.mockImplementationOnce(() => { - throw new Error('connection reset') - }) - - // Nothing cached for this org after invalidation, so it falls to defaults. - const cold = await getOrgSecurityRecord(orgId) - expect(cold).toEqual({ version: 1, sessionPolicySettings: null }) - - queueTableRows(organization, [ - { version: 5, sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null } }, - ]) - await getOrgSecurityRecord(orgId) + it('falls back to the default when the read fails', async () => { dbChainMockFns.limit.mockImplementationOnce(() => { throw new Error('connection reset') }) - - const warm = await getOrgSecurityRecord(orgId, { bypassCache: true }) - expect(warm.version).toBe(5) - expect(warm.sessionPolicySettings).toEqual({ maxSessionHours: 8, idleTimeoutHours: null }) - }) - - it('defaults an unknown organization to version 1 with no policy', async () => { - queueTableRows(organization, []) - expect(await getOrgSecurityRecord(nextOrgId())).toEqual({ - version: 1, - sessionPolicySettings: null, - }) + // Erring low only costs a cookie mismatch and a database session read — + // it can never suppress a revocation the way a stale-high value would. + expect(await getSecurityPolicyVersion(nextOrgId())).toBe(1) }) }) @@ -152,6 +84,13 @@ describe('security policy', () => { queueTableRows(member, [{ organizationId: 'org-b' }]) expect(await getMemberOrganizationId(userId)).toBe('org-b') }) + + it('propagates a read failure so callers can tell it apart from "no org"', async () => { + dbChainMockFns.limit.mockImplementationOnce(() => { + throw new Error('connection reset') + }) + await expect(getMemberOrganizationId(nextUserId())).rejects.toThrow('connection reset') + }) }) describe('getSessionCookieCacheVersion', () => { @@ -159,7 +98,7 @@ describe('security policy', () => { const userId = nextUserId() const orgId = nextOrgId() queueTableRows(member, [{ organizationId: orgId }]) - queueTableRows(organization, [{ version: 4, sessionPolicySettings: null }]) + queueTableRows(organization, [{ version: 4 }]) expect(await getSessionCookieCacheVersion({ userId })).toBe(`${orgId}:4`) }) @@ -169,6 +108,14 @@ describe('security policy', () => { expect(await getSessionCookieCacheVersion({ userId: nextUserId() })).toBe('none') }) + it('never throws on a failed membership read', async () => { + dbChainMockFns.limit.mockImplementationOnce(() => { + throw new Error('connection reset') + }) + // Runs on every authenticated request — a throw here would 500 the app. + expect(await getSessionCookieCacheVersion({ userId: nextUserId() })).toBe('none') + }) + it('costs no lookups when organizations are disabled for the deployment', async () => { setEnvFlags({ isOrganizationsEnabled: false }) expect(await getSessionCookieCacheVersion({ userId: nextUserId() })).toBe('none') diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index 2bda1ce9689..1296c80965b 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -1,5 +1,4 @@ import { db } from '@sim/db' -import type { SessionPolicySettings } from '@sim/db/schema' import { member, organization } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' @@ -8,65 +7,39 @@ import { isOrganizationsEnabled } from '@/lib/core/config/env-flags' const logger = createLogger('SecurityPolicy') /** - * How long a resolved org security record is served from process memory before - * the next request re-reads it. This TTL is the effective upper bound on - * org-wide session-revocation latency: a version bump changes the cookie-cache - * version, and every cached session cookie in the org falls through to a DB - * read within one TTL. + * How long a resolved org security-policy version is served from process + * memory before the next request re-reads it. This TTL is the effective upper + * bound on org-wide session-revocation latency: a version bump changes the + * cookie-cache version, and every cached session cookie in the org falls + * through to a DB read within one TTL. + * + * Only the VERSION is cached. It is read on every authenticated request, and + * Better Auth offers no cross-instance invalidation for cookie caches — its own + * guidance for immediate revocation is to disable cookie caching entirely — so + * a short TTL is the deliberate trade this feature makes. The session POLICY is + * never cached; it is read fresh at enforcement time (see `session-policy.ts`), + * which is what keeps a stale version from ever pairing with a stale policy. */ -export const SECURITY_POLICY_CACHE_TTL_MS = 60 * 1000 - -/** - * After a failed read the last known-good record keeps being served, but its - * timestamp is rolled back to leave this much life so a sustained outage - * retries periodically instead of on every request. - */ -const ERROR_RETRY_BACKOFF_MS = 5 * 1000 +export const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000 /** * Entry caps. Both maps only ever grow through explicit writes, so without a - * bound a long-lived process accumulates one entry per distinct org/user it - * has ever served. Eviction drops expired entries first, then the - * least-recently-refreshed ones. + * bound a long-lived process accumulates one entry per distinct org/user it has + * ever served. */ -const MAX_ORG_CACHE_ENTRIES = 5_000 +const MAX_VERSION_CACHE_ENTRIES = 5_000 const MAX_MEMBERSHIP_CACHE_ENTRIES = 20_000 -const DEFAULT_VERSION = 1 - -/** - * The org security state that governs cached session cookies, read as ONE row - * so its two fields can never be observed out of sync. Splitting them across - * independent caches allowed a pod to see a bumped `version` (which forces a - * session refresh) while still holding the pre-bump `sessionPolicySettings` — - * the refresh then re-clamped against the stale policy and stretched the - * session back out, silently undoing a just-saved tightening. - */ -export interface OrgSecurityRecord { - /** - * Monotonic counter behind the Better Auth cookie-cache version. It backs - * ALL org security policies (session policies today; IP allowlisting and MFA - * enforcement are planned consumers): any feature that needs cached session - * cookies to re-validate bumps this one counter. - */ - version: number - sessionPolicySettings: SessionPolicySettings | null -} - -interface OrgSecurityCacheEntry extends OrgSecurityRecord { - fetchedAt: number -} - -const orgSecurityCache = new Map() - /** * Fraction of the cap an over-capacity prune evicts down to. Trimming to a low * water mark rather than exactly to the cap is what keeps eviction amortized - * O(1): pruning back to the cap would leave the very next insert over again, - * so every subsequent write would rescan the whole map. + * O(1): pruning back to the cap would leave the very next insert over again, so + * every subsequent write would rescan the whole map. */ const PRUNE_TARGET_RATIO = 0.9 +const DEFAULT_VERSION = 1 + /** * Evicts down to a low water mark, expired entries first. Map iteration follows * insertion order and {@link touch} re-inserts on every refresh, so whatever @@ -75,7 +48,7 @@ const PRUNE_TARGET_RATIO = 0.9 function prune(cache: Map, maxEntries: number): void { if (cache.size <= maxEntries) return const target = Math.floor(maxEntries * PRUNE_TARGET_RATIO) - const cutoff = Date.now() - SECURITY_POLICY_CACHE_TTL_MS + const cutoff = Date.now() - SECURITY_POLICY_VERSION_CACHE_TTL_MS for (const [key, entry] of cache) { if (cache.size <= target) break if (entry.fetchedAt < cutoff) cache.delete(key) @@ -98,78 +71,61 @@ function touch( prune(cache, maxEntries) } +interface VersionCacheEntry { + version: number + fetchedAt: number +} + +const versionCache = new Map() + /** - * Resolves the org's security record from a short TTL cache. On a read failure - * the last known-good record keeps being served: falling back to defaults - * would drop the org's session bounds entirely, disabling a security control - * because of a transient database blip. + * Resolves the org's security-policy version — the shared monotonic counter + * behind the Better Auth cookie-cache version. It backs ALL org security + * policies (session policies today; IP allowlisting and MFA enforcement are + * planned consumers): any feature that needs cached session cookies to + * re-validate bumps this one counter. + * + * A failed read falls back to the default rather than the last known value. + * That errs toward MORE revalidation, not less: a version that reads lower than + * the stored one mismatches the cookie and forces a database session read. */ -export async function getOrgSecurityRecord( - organizationId: string, - options: { bypassCache?: boolean } = {} -): Promise { - const cached = orgSecurityCache.get(organizationId) - if ( - !options.bypassCache && - cached && - Date.now() - cached.fetchedAt < SECURITY_POLICY_CACHE_TTL_MS - ) { - return cached +export async function getSecurityPolicyVersion( + organizationId: string | null | undefined +): Promise { + if (!organizationId) return DEFAULT_VERSION + + const cached = versionCache.get(organizationId) + if (cached && Date.now() - cached.fetchedAt < SECURITY_POLICY_VERSION_CACHE_TTL_MS) { + return cached.version } try { const [row] = await db - .select({ - version: organization.securityPolicyVersion, - sessionPolicySettings: organization.sessionPolicySettings, - }) + .select({ version: organization.securityPolicyVersion }) .from(organization) .where(eq(organization.id, organizationId)) .limit(1) - const record: OrgSecurityRecord = { - version: row?.version ?? DEFAULT_VERSION, - sessionPolicySettings: row?.sessionPolicySettings ?? null, - } + const version = row?.version ?? DEFAULT_VERSION touch( - orgSecurityCache, + versionCache, organizationId, - { ...record, fetchedAt: Date.now() }, - MAX_ORG_CACHE_ENTRIES + { version, fetchedAt: Date.now() }, + MAX_VERSION_CACHE_ENTRIES ) - return record + return version } catch (error) { - if (cached) { - logger.error('Failed to refresh org security record; serving last known-good', { - organizationId, - error, - }) - cached.fetchedAt = Date.now() - SECURITY_POLICY_CACHE_TTL_MS + ERROR_RETRY_BACKOFF_MS - return cached - } - logger.error('Failed to resolve org security record; using defaults', { + logger.error('Failed to resolve security policy version; using default', { organizationId, error, }) - return { version: DEFAULT_VERSION, sessionPolicySettings: null } + return DEFAULT_VERSION } } -/** Resolves just the cookie-cache version component of the org security record. */ -export async function getSecurityPolicyVersion( - organizationId: string | null | undefined -): Promise { - if (!organizationId) return DEFAULT_VERSION - return (await getOrgSecurityRecord(organizationId)).version -} - -/** - * Drops the cached security record for an org so the next read is fresh. One - * entry covers both the version and the session policy, so a caller cannot - * invalidate one and forget the other. - */ -export function invalidateOrgSecurityCache(organizationId: string): void { - orgSecurityCache.delete(organizationId) +/** Drops the cached version for an org so the next read is fresh. */ +export function invalidateSecurityPolicyVersionCache(organizationId: string): void { + versionCache.delete(organizationId) } interface MembershipCacheEntry { @@ -203,6 +159,10 @@ export function invalidateMembershipCache(userId: string): void { * none, and without this fallback such sessions would dodge cookie-cache * invalidation (and therefore org-wide revocation) for up to the 24h cookie * lifetime. + * + * Throws if the lookup fails. Callers that can tolerate an unknown membership + * use {@link getMemberOrganizationIdSafe}; the session-create path deliberately + * does not, so a failed read cannot be mistaken for "not in an org". */ export async function getMemberOrganizationId( userId: string | null | undefined @@ -212,26 +172,33 @@ export async function getMemberOrganizationId( const cached = membershipCache.get(userId) if (cached) { const ttl = cached.organizationId - ? SECURITY_POLICY_CACHE_TTL_MS + ? SECURITY_POLICY_VERSION_CACHE_TTL_MS : NEGATIVE_MEMBERSHIP_CACHE_TTL_MS if (Date.now() - cached.fetchedAt < ttl) return cached.organizationId } - try { - const [row] = await db - .select({ organizationId: member.organizationId }) - .from(member) - .where(eq(member.userId, userId)) - .limit(1) + const [row] = await db + .select({ organizationId: member.organizationId }) + .from(member) + .where(eq(member.userId, userId)) + .limit(1) + + const organizationId = row?.organizationId ?? null + touch( + membershipCache, + userId, + { organizationId, fetchedAt: Date.now() }, + MAX_MEMBERSHIP_CACHE_ENTRIES + ) + return organizationId +} - const organizationId = row?.organizationId ?? null - touch( - membershipCache, - userId, - { organizationId, fetchedAt: Date.now() }, - MAX_MEMBERSHIP_CACHE_ENTRIES - ) - return organizationId +/** {@link getMemberOrganizationId}, treating a failed lookup as org-less. */ +async function getMemberOrganizationIdSafe( + userId: string | null | undefined +): Promise { + try { + return await getMemberOrganizationId(userId) } catch (error) { logger.error('Failed to resolve org membership; treating session as org-less', { userId, @@ -251,12 +218,16 @@ export async function getMemberOrganizationId( * dodge the destination org's version bumps for up to the 24h cookie lifetime. * Sessions of non-members — and every session when organizations are disabled * for the deployment — use the static default and cost no lookups. + * + * Never throws: this runs on every request, and a failed membership read here + * yields the default version, whose only effect is a cookie mismatch and a + * database session read. */ export async function getSessionCookieCacheVersion(session: { userId?: string | null }): Promise { if (!isOrganizationsEnabled) return 'none' - const organizationId = await getMemberOrganizationId(session.userId) + const organizationId = await getMemberOrganizationIdSafe(session.userId) if (!organizationId) return 'none' // The org id is part of the version so moving between orgs always changes // the string — two orgs whose counters happen to hold the same number must diff --git a/apps/sim/lib/auth/session-policy.test.ts b/apps/sim/lib/auth/session-policy.test.ts index 9f14bb8a756..8dfc7ebec83 100644 --- a/apps/sim/lib/auth/session-policy.test.ts +++ b/apps/sim/lib/auth/session-policy.test.ts @@ -127,7 +127,7 @@ describe('getSessionPolicy', () => { }) it('skips the entitlement check when the org has no bounds stored', async () => { - queueTableRows(organization, [{ version: 1, sessionPolicySettings: null }]) + queueTableRows(organization, [{ settings: null }]) expect(await getSessionPolicy(nextOrgId())).toEqual({ maxSessionHours: null, idleTimeoutHours: null, @@ -136,9 +136,7 @@ describe('getSessionPolicy', () => { }) it('stops enforcing stored bounds for an org that is no longer entitled', async () => { - queueTableRows(organization, [ - { version: 1, sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null } }, - ]) + queueTableRows(organization, [{ settings: { maxSessionHours: 8, idleTimeoutHours: null } }]) mockResolveEnterprisePlan.mockResolvedValue(false) expect(await getSessionPolicy(nextOrgId())).toEqual({ @@ -147,27 +145,8 @@ describe('getSessionPolicy', () => { }) }) - it('amortizes the entitlement check across repeated resolutions', async () => { - const orgId = nextOrgId() - const bounded = { - version: 1, - sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null }, - } - queueTableRows(organization, [bounded]) - queueTableRows(organization, [bounded]) - - // A policy save makes every member session refresh at once; the plan check - // must not run once per refreshing session. - await getSessionPolicy(orgId) - await getSessionPolicy(orgId, { bypassCache: true }) - - expect(mockResolveEnterprisePlan).toHaveBeenCalledTimes(1) - }) - it('keeps enforcing when the entitlement check itself fails', async () => { - queueTableRows(organization, [ - { version: 1, sessionPolicySettings: { maxSessionHours: 8, idleTimeoutHours: null } }, - ]) + queueTableRows(organization, [{ settings: { maxSessionHours: 8, idleTimeoutHours: null } }]) // Stored bounds are only writable by an entitled org, so a failed plan read // must not be mistaken for a downgrade and silently disable enforcement. mockResolveEnterprisePlan.mockRejectedValue(new Error('billing unavailable')) @@ -220,9 +199,7 @@ describe('clampExpiryForSession', () => { it('clamps against the org resolved from membership', async () => { const orgId = nextOrgId() queueTableRows(member, [{ organizationId: orgId }]) - queueTableRows(organization, [ - { version: 1, sessionPolicySettings: { maxSessionHours: 24, idleTimeoutHours: null } }, - ]) + queueTableRows(organization, [{ settings: { maxSessionHours: 24, idleTimeoutHours: null } }]) const result = await clampExpiryForSession({ userId: `user-${orgId}`, @@ -235,9 +212,7 @@ describe('clampExpiryForSession', () => { it('skips the membership lookup when the caller already resolved it', async () => { const orgId = nextOrgId() - queueTableRows(organization, [ - { version: 1, sessionPolicySettings: { maxSessionHours: 24, idleTimeoutHours: null } }, - ]) + queueTableRows(organization, [{ settings: { maxSessionHours: 24, idleTimeoutHours: null } }]) const result = await clampExpiryForSession( { userId: 'user-fresh', createdAt, expiresAt: proposed }, @@ -251,9 +226,7 @@ describe('clampExpiryForSession', () => { it('normalizes ISO string dates crossing the hook serialization boundary', async () => { const orgId = nextOrgId() - queueTableRows(organization, [ - { version: 1, sessionPolicySettings: { maxSessionHours: 24, idleTimeoutHours: null } }, - ]) + queueTableRows(organization, [{ settings: { maxSessionHours: 24, idleTimeoutHours: null } }]) const result = await clampExpiryForSession( { @@ -267,22 +240,28 @@ describe('clampExpiryForSession', () => { expect(result?.getTime()).toBe(createdAt.getTime() + 24 * 60 * 60 * 1000) }) - it('re-reads the policy when the caller bypasses the cache', async () => { + it('reads the policy fresh on every call so a saved policy is never missed', async () => { const orgId = nextOrgId() - queueTableRows(organization, [{ version: 1, sessionPolicySettings: null }]) + queueTableRows(organization, [{ settings: null }]) await clampExpiryForSession({ userId: 'u', createdAt, expiresAt: proposed }, orgId) - // A policy saved on another process between the two calls: the bypass is - // what stops a just-tightened policy from being missed at session create. - queueTableRows(organization, [ - { version: 2, sessionPolicySettings: { maxSessionHours: 24, idleTimeoutHours: null } }, - ]) + // A policy saved on another process between the two calls. Nothing is + // cached, so the second clamp already sees it. + queueTableRows(organization, [{ settings: { maxSessionHours: 24, idleTimeoutHours: null } }]) const result = await clampExpiryForSession( { userId: 'u', createdAt, expiresAt: proposed }, - orgId, - { bypassPolicyCache: true } + orgId ) expect(result?.getTime()).toBe(createdAt.getTime() + 24 * 60 * 60 * 1000) }) + + it('propagates a policy read failure so the hook can refuse to extend', async () => { + dbChainMockFns.limit.mockImplementationOnce(() => { + throw new Error('connection reset') + }) + await expect( + clampExpiryForSession({ userId: 'u', createdAt, expiresAt: proposed }, nextOrgId()) + ).rejects.toThrow('connection reset') + }) }) diff --git a/apps/sim/lib/auth/session-policy.ts b/apps/sim/lib/auth/session-policy.ts index c6fcb33543b..34d11ecf1b7 100644 --- a/apps/sim/lib/auth/session-policy.ts +++ b/apps/sim/lib/auth/session-policy.ts @@ -1,12 +1,9 @@ import { db } from '@sim/db' +import { organization } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { sql } from 'drizzle-orm' +import { eq, sql } from 'drizzle-orm' import { MIN_IDLE_TIMEOUT_HOURS } from '@/lib/api/contracts/organization' -import { - getMemberOrganizationId, - getOrgSecurityRecord, - invalidateMembershipCache, -} from '@/lib/auth/security-policy' +import { getMemberOrganizationId, invalidateMembershipCache } from '@/lib/auth/security-policy' import { resolveOrganizationEnterprisePlan } from '@/lib/billing/core/subscription' const logger = createLogger('SessionPolicy') @@ -23,78 +20,64 @@ const NO_POLICY: ResolvedSessionPolicy = { idleTimeoutHours: null, } -/** - * Entitlement is billing state, so it is cached independently of the org - * security record: a policy save must invalidate the policy, but it says - * nothing about the plan. The TTL matters because saving a policy or revoking - * org-wide makes every member session refresh at once — without it, each of - * those refreshes would re-run the plan check and stampede the billing tables. - * Staleness is harmless in both directions: a just-downgraded org keeps - * enforcing for one TTL, and a just-upgraded one starts one TTL late. - */ -const ENTITLEMENT_CACHE_TTL_MS = 60 * 1000 -const MAX_ENTITLEMENT_CACHE_ENTRIES = 5_000 - -const entitlementCache = new Map() - /** * Whether stored session bounds should be enforced for this org. Mirroring * data-retention's plan-gated effective settings, a hosted org that leaves the * Enterprise plan stops enforcing its stored limits automatically, since the * enterprise-gated settings UI can no longer manage them. * - * A FAILED entitlement read enforces instead of skipping: bounds are only - * writable by an entitled org (the PUT route gates on it), so their presence is - * the source of truth when the plan check itself is unavailable. Fail-open here - * would let a transient billing-read blip silently disable a security control — - * the same reasoning the PII-redaction path documents in - * `lib/logs/execution/logger.ts`. + * Uses the PROPAGATING plan resolver so a failed read is not mistaken for a + * downgrade. `isOrganizationOnEnterprisePlan` swallows its errors and returns + * `false`, which is correct for feature gating (fail closed) but would silently + * stop ENFORCING here — the fail-open trap the PII-redaction path documents in + * `lib/logs/execution/logger.ts`. Only reached when bounds are actually stored, + * so non-enterprise orgs never pay for it. */ async function isPolicyEnforced(organizationId: string, hasBounds: boolean): Promise { - // Orgs with nothing stored are the common case and need no plan check at all. if (!hasBounds) return true - - const cached = entitlementCache.get(organizationId) - if (cached && Date.now() - cached.fetchedAt < ENTITLEMENT_CACHE_TTL_MS) { - return cached.entitled - } - try { - const entitled = await resolveOrganizationEnterprisePlan(organizationId) - // Entitled orgs are few, so a wholesale clear at the cap is enough — no - // eviction policy needed for a set this small and this slow-moving. - if (entitlementCache.size >= MAX_ENTITLEMENT_CACHE_ENTRIES) entitlementCache.clear() - entitlementCache.set(organizationId, { entitled, fetchedAt: Date.now() }) - return entitled + return await resolveOrganizationEnterprisePlan(organizationId) } catch (error) { logger.error('Enterprise entitlement check failed; enforcing stored session policy', { organizationId, error, }) - return cached?.entitled ?? true + return true } } /** - * Resolves the EFFECTIVE session policy for an organization. Returns a no-op - * policy for personal (org-less) sessions and for orgs that are no longer - * entitled. + * Resolves the EFFECTIVE session policy for an organization, read FRESH from + * the organization row on every call. + * + * Deliberately uncached, matching how the sibling enterprise setting resolves + * its stored rules at enforcement time (`lib/logs/execution/logger.ts`, + * `lib/workflows/executor/execution-core.ts`). This is not a hot path: it runs + * on sign-in and on a sliding session refresh, which the 24h cookie cache + * limits to roughly once per user per day — one indexed point lookup each. * - * The stored settings come from the shared org security record, so the policy a - * caller sees is always the one that matches the cookie-cache version it sees. - * Pass `bypassCache` on paths that must not act on a policy up to one cache TTL - * old — notably session CREATE, which (unlike a refresh) is not already preceded - * by a version-mismatch read and would otherwise mint an unclamped session from - * a stale record. + * A cache here is what made an earlier version of this code incorrect. Better + * Auth rewrites `expiresAt` to `now + 30d` on every refresh, so a refresh that + * clamps against a stale policy silently un-does a tightening — and because a + * 30-day expiry then reads as "not due for refresh", nothing re-clamps it for + * another day. Reading fresh removes that failure mode by construction rather + * than trying to keep two caches coherent. + * + * Throws if the read fails. Callers pick their own failure posture: the session + * UPDATE hook refuses to extend, the CREATE hook allows the sign-in. */ export async function getSessionPolicy( - organizationId: string | null | undefined, - options: { bypassCache?: boolean } = {} + organizationId: string | null | undefined ): Promise { if (!organizationId) return NO_POLICY - const { sessionPolicySettings } = await getOrgSecurityRecord(organizationId, options) - const settings = sessionPolicySettings ?? {} + const [row] = await db + .select({ settings: organization.sessionPolicySettings }) + .from(organization) + .where(eq(organization.id, organizationId)) + .limit(1) + + const settings = row?.settings ?? {} const hasBounds = Boolean(settings.maxSessionHours || settings.idleTimeoutHours) if (!(await isPolicyEnforced(organizationId, hasBounds))) return NO_POLICY @@ -157,8 +140,7 @@ interface ClampableSession { */ export async function clampExpiryForSession( session: ClampableSession, - freshMembershipOrgId?: string | null, - options: { bypassPolicyCache?: boolean } = {} + freshMembershipOrgId?: string | null ): Promise { // Better Auth context values can cross a serialization boundary — normalize // date fields in case they arrive as ISO strings rather than Dates. @@ -172,9 +154,7 @@ export async function clampExpiryForSession( : await getMemberOrganizationId(session.userId) if (!organizationId) return expiresAt - const policy = await getSessionPolicy(organizationId, { - bypassCache: options.bypassPolicyCache, - }) + const policy = await getSessionPolicy(organizationId) const createdAt = session.createdAt ? new Date(session.createdAt) : new Date() return clampSessionExpiry(policy, createdAt, expiresAt) } @@ -220,10 +200,7 @@ export async function applySessionPolicyToNewMember( ): Promise { try { invalidateMembershipCache(userId) - // Bypass the cache: a user joining moments after a policy save must be - // clamped to the policy that was just written, not a pre-save record this - // process may still be holding. - const policy = await getSessionPolicy(organizationId, { bypassCache: true }) + const policy = await getSessionPolicy(organizationId) const bounds = clampBoundsSql(policy) if (!bounds) return From 78ad0d3f96bf5f72f449af681433aca672052ab6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 17:51:16 -0700 Subject: [PATCH 04/13] fix(billing): actually propagate subscription read failures from the resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch. `resolveOrganizationEnterprisePlan` existed to distinguish "not entitled" from "the check could not run", but it called `getOrganizationSubscriptionUsable` with the default `onError: 'return-null'` — so a transient subscription read failed to `null`, read as "no usable subscription", and reported a downgrade anyway. The one failure mode the resolver was written for was the one it still swallowed. Also guard the version cache against out-of-order writes: the counter only ever increments, so a read resolving lower than the cached value started before the newer one and must not overwrite it. --- apps/sim/lib/auth/security-policy.test.ts | 17 +++++++++++++++++ apps/sim/lib/auth/security-policy.ts | 19 +++++++++++++------ apps/sim/lib/billing/core/subscription.ts | 7 ++++++- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/apps/sim/lib/auth/security-policy.test.ts b/apps/sim/lib/auth/security-policy.test.ts index 5c6447630b3..4dd7fbb2297 100644 --- a/apps/sim/lib/auth/security-policy.test.ts +++ b/apps/sim/lib/auth/security-policy.test.ts @@ -56,6 +56,23 @@ describe('security policy', () => { expect(await getSecurityPolicyVersion(nextOrgId())).toBe(1) }) + it('never lets a late read overwrite a newer version', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [{ version: 9 }]) + expect(await getSecurityPolicyVersion(orgId)).toBe(9) + + invalidateSecurityPolicyVersionCache(orgId) + queueTableRows(organization, [{ version: 9 }]) + await getSecurityPolicyVersion(orgId) + // A read that started before the bump resolving late must not re-seed the + // pre-bump value and delay cookie invalidation another TTL. + invalidateSecurityPolicyVersionCache(orgId) + queueTableRows(organization, [{ version: 10 }]) + expect(await getSecurityPolicyVersion(orgId)).toBe(10) + queueTableRows(organization, [{ version: 9 }]) + expect(await getSecurityPolicyVersion(orgId)).toBe(10) + }) + it('falls back to the default when the read fails', async () => { dbChainMockFns.limit.mockImplementationOnce(() => { throw new Error('connection reset') diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index 1296c80965b..277c8a8a4c0 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -107,12 +107,19 @@ export async function getSecurityPolicyVersion( .limit(1) const version = row?.version ?? DEFAULT_VERSION - touch( - versionCache, - organizationId, - { version, fetchedAt: Date.now() }, - MAX_VERSION_CACHE_ENTRIES - ) + // The counter only ever increments, so a read resolving LOWER than what is + // already cached started before the newer one and must not overwrite it — a + // late write would otherwise re-serve a pre-bump version and delay cookie + // invalidation for a further TTL. + const current = versionCache.get(organizationId) + if (!current || version >= current.version) { + touch( + versionCache, + organizationId, + { version, fetchedAt: Date.now() }, + MAX_VERSION_CACHE_ENTRIES + ) + } return version } catch (error) { logger.error('Failed to resolve security policy version; using default', { diff --git a/apps/sim/lib/billing/core/subscription.ts b/apps/sim/lib/billing/core/subscription.ts index c44d8dfd279..a1855c05a3a 100644 --- a/apps/sim/lib/billing/core/subscription.ts +++ b/apps/sim/lib/billing/core/subscription.ts @@ -447,7 +447,12 @@ export async function resolveOrganizationEnterprisePlan(organizationId: string): return false } - const orgSub = await getOrganizationSubscriptionUsable(organizationId) + // `onError: 'throw'` is load-bearing: the default swallows a read failure into + // `null`, which reads as "no usable subscription" and would make this resolver + // report a downgrade on a transient error — exactly the conflation it exists + // to avoid. `isOrganizationOnEnterprisePlan` still turns that throw back into + // `false` for feature gating. + const orgSub = await getOrganizationSubscriptionUsable(organizationId, { onError: 'throw' }) return !!orgSub && checkEnterprisePlan(orgSub) } From 8b928f50455f18351f3b0aaf3cc69a48148d1c27 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 17:59:59 -0700 Subject: [PATCH 05/13] fix(auth): publish the committed version instead of invalidating the cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch. The monotonic guard skipped STORING a superseded version but still RETURNED it, and clearing the entry on write left no floor to compare against — so an in-flight pre-bump read could land after the bump and re-seed the old version for a full TTL. Either path keeps cookie versions matched, which is precisely how a revoked session keeps serving from the cookie cache. Both routes already compute the new counter, so they now return it from the same UPDATE and publish it directly. That leaves the cache always holding a floor, makes a late read unable to win on either the return path or the store path, and drops a redundant read. Also documents the session-create trade explicitly: when both the direct membership read and the cached fallback fail, the org is unknown and the sign-in proceeds unclamped. Refusing it would convert a `member` read failure into a total authentication outage for every user, including the majority who belong to no org. The delay is bounded — the update hook clamps retroactively from `createdAt` at the next cookie-cache expiry, and any admin action bumps the version and closes the window immediately. --- .../[id]/session-policy/route.test.ts | 6 +-- .../[id]/session-policy/route.ts | 9 +++-- .../[id]/sessions/revoke/route.test.ts | 40 +++++++++++++------ .../[id]/sessions/revoke/route.ts | 14 ++++--- apps/sim/lib/auth/auth.ts | 18 +++++++-- apps/sim/lib/auth/security-policy.test.ts | 29 ++++++++------ apps/sim/lib/auth/security-policy.ts | 38 +++++++++++------- 7 files changed, 101 insertions(+), 53 deletions(-) diff --git a/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts b/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts index ce21a141d30..57f1ac01ea4 100644 --- a/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts +++ b/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts @@ -24,7 +24,7 @@ vi.mock('@/lib/auth/session-policy', () => ({ })) vi.mock('@/lib/auth/security-policy', () => ({ - invalidateSecurityPolicyVersionCache: vi.fn(), + setSecurityPolicyVersion: vi.fn(), })) vi.mock('@/lib/billing/core/subscription', () => ({ @@ -127,7 +127,7 @@ describe('session policy route', () => { it('saves the policy, eagerly clamps sessions, and bumps the version', async () => { queueTableRows(member, [{ role: 'owner' }]) queueTableRows(organization, [{ name: 'Acme' }]) - dbChainMockFns.returning.mockResolvedValueOnce([{ id: ORG_ID }]) + dbChainMockFns.returning.mockResolvedValueOnce([{ id: ORG_ID, securityPolicyVersion: 2 }]) const response = await PUT( putRequest({ maxSessionHours: 72, idleTimeoutHours: 48 }), @@ -153,7 +153,7 @@ describe('session policy route', () => { it('clearing both fields still saves and delegates the no-op to the clamp', async () => { queueTableRows(member, [{ role: 'owner' }]) queueTableRows(organization, [{ name: 'Acme' }]) - dbChainMockFns.returning.mockResolvedValueOnce([{ id: ORG_ID }]) + dbChainMockFns.returning.mockResolvedValueOnce([{ id: ORG_ID, securityPolicyVersion: 2 }]) const response = await PUT( putRequest({ maxSessionHours: null, idleTimeoutHours: null }), diff --git a/apps/sim/app/api/organizations/[id]/session-policy/route.ts b/apps/sim/app/api/organizations/[id]/session-policy/route.ts index 856f3e26370..2d00ca0fa0f 100644 --- a/apps/sim/app/api/organizations/[id]/session-policy/route.ts +++ b/apps/sim/app/api/organizations/[id]/session-policy/route.ts @@ -9,7 +9,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { updateOrganizationSessionPolicyContract } from '@/lib/api/contracts/organization' import { parseRequest, validationErrorResponse } from '@/lib/api/server' import { getSession } from '@/lib/auth' -import { invalidateSecurityPolicyVersionCache } from '@/lib/auth/security-policy' +import { setSecurityPolicyVersion } from '@/lib/auth/security-policy' import { eagerClampOrgSessions } from '@/lib/auth/session-policy' import { isOrganizationOnEnterprisePlan } from '@/lib/billing/core/subscription' import { isBillingEnabled } from '@/lib/core/config/env-flags' @@ -150,7 +150,10 @@ export const PUT = withRouteHandler( updatedAt: new Date(), }) .where(eq(organization.id, organizationId)) - .returning({ id: organization.id }) + .returning({ + id: organization.id, + securityPolicyVersion: organization.securityPolicyVersion, + }) if (!row) return null await eagerClampOrgSessions(organizationId, merged, tx) return row @@ -160,7 +163,7 @@ export const PUT = withRouteHandler( return NextResponse.json({ error: 'Organization not found' }, { status: 404 }) } - invalidateSecurityPolicyVersionCache(organizationId) + setSecurityPolicyVersion(organizationId, updated.securityPolicyVersion) logger.info('Updated organization session policy', { organizationId }) diff --git a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts index 3ab981b3e50..aec6a6d6db2 100644 --- a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts +++ b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts @@ -13,14 +13,14 @@ import { } from '@sim/testing' import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockIsEnterprise, mockRecordAudit, mockInvalidateVersionCache } = vi.hoisted(() => ({ +const { mockIsEnterprise, mockRecordAudit, mockSetVersion } = vi.hoisted(() => ({ mockIsEnterprise: vi.fn(), mockRecordAudit: vi.fn(), - mockInvalidateVersionCache: vi.fn(), + mockSetVersion: vi.fn(), })) vi.mock('@/lib/auth/security-policy', () => ({ - invalidateSecurityPolicyVersionCache: mockInvalidateVersionCache, + setSecurityPolicyVersion: mockSetVersion, })) vi.mock('@/lib/billing/core/subscription', () => ({ @@ -135,7 +135,9 @@ describe('org sessions revoke route', () => { it('allows admins as well as owners', async () => { queueTableRows(member, [{ role: 'admin' }]) queueTableRows(organization, [{ name: 'Acme' }]) - dbChainMockFns.returning.mockResolvedValueOnce([{ id: 's-1' }]) + dbChainMockFns.returning + .mockResolvedValueOnce([{ id: 's-1' }]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) const response = await POST(createMockRequest('POST'), routeContext) expect(response.status).toBe(200) }) @@ -148,7 +150,9 @@ describe('org sessions revoke route', () => { }) it('reports the revoked count, bumps the version, and invalidates the cache', async () => { - dbChainMockFns.returning.mockResolvedValueOnce([{ id: 's-1' }, { id: 's-2' }]) + dbChainMockFns.returning + .mockResolvedValueOnce([{ id: 's-1' }, { id: 's-2' }]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) const response = await POST(createMockRequest('POST'), routeContext) @@ -157,7 +161,7 @@ describe('org sessions revoke route', () => { expect(dbChainMockFns.set).toHaveBeenCalledWith( expect.objectContaining({ securityPolicyVersion: expect.anything() }) ) - expect(mockInvalidateVersionCache).toHaveBeenCalledWith(ORG_ID) + expect(mockSetVersion).toHaveBeenCalledWith(ORG_ID, 5) expect(mockRecordAudit).toHaveBeenCalledWith( expect.objectContaining({ action: 'organization.sessions.revoked', @@ -168,7 +172,9 @@ describe('org sessions revoke route', () => { }) it("never deletes the caller's own session", async () => { - dbChainMockFns.returning.mockResolvedValueOnce([]) + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) await POST(createMockRequest('POST'), routeContext) expect(deleteConditions()).toContainEqual( @@ -177,7 +183,9 @@ describe('org sessions revoke route', () => { }) it('spares impersonation sessions', async () => { - dbChainMockFns.returning.mockResolvedValueOnce([]) + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) await POST(createMockRequest('POST'), routeContext) expect(deleteConditions()).toContainEqual( @@ -186,7 +194,9 @@ describe('org sessions revoke route', () => { }) it('scopes the delete to members of the organization', async () => { - dbChainMockFns.returning.mockResolvedValueOnce([]) + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) await POST(createMockRequest('POST'), routeContext) expect(deleteConditions()).toContainEqual( @@ -196,7 +206,9 @@ describe('org sessions revoke route', () => { it("also spares the impersonator's own sessions when the caller is impersonating", async () => { authenticateAs({ impersonatedBy: 'platform-admin-1' }) - dbChainMockFns.returning.mockResolvedValueOnce([]) + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) await POST(createMockRequest('POST'), routeContext) @@ -210,7 +222,9 @@ describe('org sessions revoke route', () => { }) it('adds no impersonator exclusion for an ordinary caller', async () => { - dbChainMockFns.returning.mockResolvedValueOnce([]) + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) await POST(createMockRequest('POST'), routeContext) const userIdExclusions = deleteConditions().filter( @@ -220,7 +234,9 @@ describe('org sessions revoke route', () => { }) it('singularizes the audit description for a single session', async () => { - dbChainMockFns.returning.mockResolvedValueOnce([{ id: 's-1' }]) + dbChainMockFns.returning + .mockResolvedValueOnce([{ id: 's-1' }]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) await POST(createMockRequest('POST'), routeContext) expect(mockRecordAudit).toHaveBeenCalledWith( diff --git a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts index 2731d2882ac..b9d086229d6 100644 --- a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts +++ b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts @@ -8,7 +8,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { revokeOrganizationSessionsContract } from '@/lib/api/contracts/organization' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' -import { invalidateSecurityPolicyVersionCache } from '@/lib/auth/security-policy' +import { setSecurityPolicyVersion } from '@/lib/auth/security-policy' import { isOrganizationOnEnterprisePlan } from '@/lib/billing/core/subscription' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -81,7 +81,7 @@ export const POST = withRouteHandler( // Delete and version bump commit atomically: a bump failure must roll the // delete back, or members would stay authenticated from the cookie cache // for up to 24h with their DB sessions already gone. - const revoked = await db.transaction(async (tx) => { + const revokeResult = await db.transaction(async (tx) => { const deleted = await tx .delete(sessionTable) .where( @@ -99,13 +99,17 @@ export const POST = withRouteHandler( ) ) .returning({ id: sessionTable.id }) - await tx + const [bumped] = await tx .update(organization) .set({ securityPolicyVersion: sql`${organization.securityPolicyVersion} + 1` }) .where(eq(organization.id, organizationId)) - return deleted + .returning({ securityPolicyVersion: organization.securityPolicyVersion }) + return { deleted, version: bumped?.securityPolicyVersion } }) - invalidateSecurityPolicyVersionCache(organizationId) + const revoked = revokeResult.deleted + if (revokeResult.version !== undefined) { + setSecurityPolicyVersion(organizationId, revokeResult.version) + } logger.info('Revoked organization sessions', { organizationId, diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 74a80a79a8e..afedd566bef 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -697,9 +697,21 @@ export const auth = betterAuth({ }, } } catch (error) { - // Allow the sign-in: refusing it would turn a policy-read blip into - // an org-wide outage. The session keeps Better Auth's default - // expiry and the next refresh clamps it. + // DELIBERATE TRADE, not an oversight. Reaching here means both the + // direct membership read and the cached fallback failed, so the + // governing org — and therefore the policy — is unknown. The + // alternative, refusing the sign-in, would turn a `member` read + // failure into a total authentication outage for EVERY user, + // including the overwhelming majority who belong to no org and have + // no policy to enforce. That is a worse outcome than a bounded + // enforcement delay. + // + // The delay is bounded: this session takes the DB path at its next + // cookie-cache expiry (24h) and the update hook clamps it then — + // retroactively, since the max-lifetime bound is measured from + // `createdAt`, so an over-long session expires immediately on that + // refresh. Any admin action (policy save, sign-out-all) bumps the + // security-policy version and closes the window at once. logger.error('Error clamping new session to org policy; session not clamped', { error, userId: session.userId, diff --git a/apps/sim/lib/auth/security-policy.test.ts b/apps/sim/lib/auth/security-policy.test.ts index 4dd7fbb2297..4a98d97682d 100644 --- a/apps/sim/lib/auth/security-policy.test.ts +++ b/apps/sim/lib/auth/security-policy.test.ts @@ -15,7 +15,7 @@ import { getSecurityPolicyVersion, getSessionCookieCacheVersion, invalidateMembershipCache, - invalidateSecurityPolicyVersionCache, + setSecurityPolicyVersion, } from '@/lib/auth/security-policy' /** Module-level caches persist across cases, so every case uses a fresh id. */ @@ -41,9 +41,10 @@ describe('security policy', () => { expect(await getSecurityPolicyVersion(orgId)).toBe(3) expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) - invalidateSecurityPolicyVersionCache(orgId) - queueTableRows(organization, [{ version: 4 }]) + setSecurityPolicyVersion(orgId, 4) expect(await getSecurityPolicyVersion(orgId)).toBe(4) + // Published authoritatively by the writer — no extra read needed. + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) }) it('returns the default without querying for an org-less session', async () => { @@ -56,23 +57,25 @@ describe('security policy', () => { expect(await getSecurityPolicyVersion(nextOrgId())).toBe(1) }) - it('never lets a late read overwrite a newer version', async () => { + it('never lets a late read serve or store a superseded version', async () => { const orgId = nextOrgId() - queueTableRows(organization, [{ version: 9 }]) - expect(await getSecurityPolicyVersion(orgId)).toBe(9) + setSecurityPolicyVersion(orgId, 10) - invalidateSecurityPolicyVersionCache(orgId) + // A read that started before the bump resolves late. It must neither be + // returned nor cached, or cookies would stay matched to the old version + // and revoked sessions would keep serving from the cookie cache. queueTableRows(organization, [{ version: 9 }]) - await getSecurityPolicyVersion(orgId) - // A read that started before the bump resolving late must not re-seed the - // pre-bump value and delay cookie invalidation another TTL. - invalidateSecurityPolicyVersionCache(orgId) - queueTableRows(organization, [{ version: 10 }]) expect(await getSecurityPolicyVersion(orgId)).toBe(10) - queueTableRows(organization, [{ version: 9 }]) expect(await getSecurityPolicyVersion(orgId)).toBe(10) }) + it('ignores a published version older than what is cached', async () => { + const orgId = nextOrgId() + setSecurityPolicyVersion(orgId, 7) + setSecurityPolicyVersion(orgId, 6) + expect(await getSecurityPolicyVersion(orgId)).toBe(7) + }) + it('falls back to the default when the read fails', async () => { dbChainMockFns.limit.mockImplementationOnce(() => { throw new Error('connection reset') diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index 277c8a8a4c0..171625be16b 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -108,18 +108,18 @@ export async function getSecurityPolicyVersion( const version = row?.version ?? DEFAULT_VERSION // The counter only ever increments, so a read resolving LOWER than what is - // already cached started before the newer one and must not overwrite it — a - // late write would otherwise re-serve a pre-bump version and delay cookie - // invalidation for a further TTL. + // already cached started before the newer one. Neither store it nor return + // it: a late value would re-serve a pre-bump version, keeping cookies + // matched and letting revoked sessions stay on the cookie cache. const current = versionCache.get(organizationId) - if (!current || version >= current.version) { - touch( - versionCache, - organizationId, - { version, fetchedAt: Date.now() }, - MAX_VERSION_CACHE_ENTRIES - ) - } + if (current && current.version > version) return current.version + + touch( + versionCache, + organizationId, + { version, fetchedAt: Date.now() }, + MAX_VERSION_CACHE_ENTRIES + ) return version } catch (error) { logger.error('Failed to resolve security policy version; using default', { @@ -130,9 +130,19 @@ export async function getSecurityPolicyVersion( } } -/** Drops the cached version for an org so the next read is fresh. */ -export function invalidateSecurityPolicyVersionCache(organizationId: string): void { - versionCache.delete(organizationId) +/** + * Publishes the authoritative version a write just committed. + * + * Deliberately a SET rather than a delete. Deleting would leave no floor for the + * monotonic guard above, so a read that started before the bump could land + * afterwards and re-seed the pre-bump version for a full TTL — exactly the + * cookie-invalidation delay the bump exists to avoid. Callers already have the + * new value from their `RETURNING` clause, so this also saves a redundant read. + */ +export function setSecurityPolicyVersion(organizationId: string, version: number): void { + const current = versionCache.get(organizationId) + if (current && current.version > version) return + touch(versionCache, organizationId, { version, fetchedAt: Date.now() }, MAX_VERSION_CACHE_ENTRIES) } interface MembershipCacheEntry { From b46768526d202a18dcd541fbffc7889351d1b2e1 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 18:08:33 -0700 Subject: [PATCH 06/13] fix(auth): fail closed when a KNOWN org's session policy cannot be read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch, and the reviewer is right that the previous comment described behavior the code no longer had. `getSessionPolicy` now throws, so the create hook's catch covered two different failures and answered both the same way. They deserve opposite answers: - Org KNOWN — membership resolved, only the policy read failed. This user is definitely governed by an org, so the "would break sign-in for everyone" rationale simply does not apply. Fail closed. Minting a full-length session for a member whose policy could not be read is the exact outcome the feature exists to prevent, and the blast radius is org members during a window where the `organization` read fails while the `member` read just succeeded. - Org UNKNOWN — both membership reads failed, so a governed member is indistinguishable from the majority who belong to no org. Failing closed here would take authentication down for everyone to protect a policy most of them do not have. Allow, and let the update hook clamp retroactively. Throwing an APIError from this hook is the established pattern in this file — the blocked-email check directly above does the same. --- apps/sim/lib/auth/auth.ts | 48 ++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index afedd566bef..1b41065366e 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -697,30 +697,42 @@ export const auth = betterAuth({ }, } } catch (error) { - // DELIBERATE TRADE, not an oversight. Reaching here means both the - // direct membership read and the cached fallback failed, so the - // governing org — and therefore the policy — is unknown. The - // alternative, refusing the sign-in, would turn a `member` read - // failure into a total authentication outage for EVERY user, - // including the overwhelming majority who belong to no org and have - // no policy to enforce. That is a worse outcome than a bounded - // enforcement delay. + // Two different failures land here, and they get opposite answers. // - // The delay is bounded: this session takes the DB path at its next - // cookie-cache expiry (24h) and the update hook clamps it then — - // retroactively, since the max-lifetime bound is measured from - // `createdAt`, so an over-long session expires immediately on that - // refresh. Any admin action (policy save, sign-out-all) bumps the + // Org KNOWN: membership resolved, so this user is definitely + // governed by an org — only the policy read failed. Fail closed. + // The blast radius is org members during a window where the + // `organization` read fails but the `member` read just succeeded, + // which is narrow, loud, and self-clearing. Minting a full-length + // session for a member whose policy we could not read is the one + // outcome this feature exists to prevent. + if (membershipOrgId) { + logger.error('Refusing session: org session policy could not be resolved', { + error, + userId: session.userId, + organizationId: membershipOrgId, + }) + throw new APIError('INTERNAL_SERVER_ERROR', { + message: 'Could not verify your organization session policy. Please try again.', + }) + } + + // Org UNKNOWN: both the direct membership read and the cached + // fallback failed, so we cannot tell a governed member from the + // overwhelming majority of users who belong to no org at all. + // Failing closed here would turn a `member` read failure into a + // total authentication outage for everyone, to protect a policy + // that most of them do not have. Allow, and let it self-correct: + // the session takes the DB path at its next cookie-cache expiry + // and the update hook clamps it retroactively (the max-lifetime + // bound is measured from `createdAt`, so an over-long session + // expires immediately on that refresh). Any admin action bumps the // security-policy version and closes the window at once. logger.error('Error clamping new session to org policy; session not clamped', { error, userId: session.userId, }) - return { - data: membershipOrgId - ? { ...session, activeOrganizationId: membershipOrgId } - : session, - } + return { data: session } } }, }, From 3b76628a30f20c3187815fb5148946b80879f5ff Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 18:23:53 -0700 Subject: [PATCH 07/13] refactor(auth): drop LOC that did not earn its place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Line-by-line pass over the final diff. No behavior change; every item is either a comment that had drifted from the code or state the code did not need. - The create hook's comment claimed a failed membership read still clamps "via the cached membership". Since `getMemberOrganizationId` now throws, that only holds when the cache is warm. Say what the code actually does: `null` means confirmed org-less, `undefined` means could-not-tell, and the catch branches on which one it got. - `isPolicyEnforced(orgId, hasBounds)` took a boolean parameter that inverted its own name — it returned `true` for an org with no policy at all. Split into a plain early return for the no-bounds case and `isEntitledToEnforce`, which now only answers the question its name asks. - The revoke route aliased a transaction result to rename one field; destructure it instead. - The policy UPDATE returned `id` purely as an existence check that the version column it also returns already provides. - Two test names still described invalidating a cache that is now published to. --- .../organizations/[id]/session-policy/route.ts | 5 +---- .../[id]/sessions/revoke/route.test.ts | 2 +- .../organizations/[id]/sessions/revoke/route.ts | 9 +++------ apps/sim/lib/auth/auth.ts | 9 +++++---- apps/sim/lib/auth/security-policy.test.ts | 2 +- apps/sim/lib/auth/session-policy.ts | 17 +++++++++-------- 6 files changed, 20 insertions(+), 24 deletions(-) diff --git a/apps/sim/app/api/organizations/[id]/session-policy/route.ts b/apps/sim/app/api/organizations/[id]/session-policy/route.ts index 2d00ca0fa0f..79795d111e8 100644 --- a/apps/sim/app/api/organizations/[id]/session-policy/route.ts +++ b/apps/sim/app/api/organizations/[id]/session-policy/route.ts @@ -150,10 +150,7 @@ export const PUT = withRouteHandler( updatedAt: new Date(), }) .where(eq(organization.id, organizationId)) - .returning({ - id: organization.id, - securityPolicyVersion: organization.securityPolicyVersion, - }) + .returning({ securityPolicyVersion: organization.securityPolicyVersion }) if (!row) return null await eagerClampOrgSessions(organizationId, merged, tx) return row diff --git a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts index aec6a6d6db2..d550102ad0d 100644 --- a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts +++ b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts @@ -149,7 +149,7 @@ describe('org sessions revoke route', () => { queueTableRows(organization, [{ name: 'Acme' }]) }) - it('reports the revoked count, bumps the version, and invalidates the cache', async () => { + it('reports the revoked count, bumps the version, and publishes it', async () => { dbChainMockFns.returning .mockResolvedValueOnce([{ id: 's-1' }, { id: 's-2' }]) .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) diff --git a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts index b9d086229d6..9a38d9db778 100644 --- a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts +++ b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts @@ -81,7 +81,7 @@ export const POST = withRouteHandler( // Delete and version bump commit atomically: a bump failure must roll the // delete back, or members would stay authenticated from the cookie cache // for up to 24h with their DB sessions already gone. - const revokeResult = await db.transaction(async (tx) => { + const { revoked, version } = await db.transaction(async (tx) => { const deleted = await tx .delete(sessionTable) .where( @@ -104,12 +104,9 @@ export const POST = withRouteHandler( .set({ securityPolicyVersion: sql`${organization.securityPolicyVersion} + 1` }) .where(eq(organization.id, organizationId)) .returning({ securityPolicyVersion: organization.securityPolicyVersion }) - return { deleted, version: bumped?.securityPolicyVersion } + return { revoked: deleted, version: bumped?.securityPolicyVersion } }) - const revoked = revokeResult.deleted - if (revokeResult.version !== undefined) { - setSecurityPolicyVersion(organizationId, revokeResult.version) - } + if (version !== undefined) setSecurityPolicyVersion(organizationId, version) logger.info('Revoked organization sessions', { organizationId, diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 1b41065366e..d19055a4b63 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -661,9 +661,10 @@ export const auth = betterAuth({ } } - // Resolved separately from the clamp below: when this lookup fails we - // still clamp (falling back to the cached membership) rather than - // minting a full-length session off a transient read error. + // Resolved separately from the clamp below so the two failures stay + // distinguishable: `null` means "confirmed not in an org", while + // `undefined` means "could not tell". The clamp retries through the + // membership cache, and the catch below branches on which one it got. let membershipOrgId: string | null | undefined try { const members = await db @@ -677,7 +678,7 @@ export const auth = betterAuth({ { userId: session.userId, organizationId: membershipOrgId ?? undefined } ) } catch (error) { - logger.error('Error resolving organization for new session; using cached membership', { + logger.error('Could not resolve organization for new session', { error, userId: session.userId, }) diff --git a/apps/sim/lib/auth/security-policy.test.ts b/apps/sim/lib/auth/security-policy.test.ts index 4a98d97682d..5e82a912946 100644 --- a/apps/sim/lib/auth/security-policy.test.ts +++ b/apps/sim/lib/auth/security-policy.test.ts @@ -33,7 +33,7 @@ describe('security policy', () => { }) describe('getSecurityPolicyVersion', () => { - it('caches the version and re-reads after invalidation', async () => { + it('caches the version and serves a newly published one without a re-read', async () => { const orgId = nextOrgId() queueTableRows(organization, [{ version: 3 }]) diff --git a/apps/sim/lib/auth/session-policy.ts b/apps/sim/lib/auth/session-policy.ts index 34d11ecf1b7..b770785ade4 100644 --- a/apps/sim/lib/auth/session-policy.ts +++ b/apps/sim/lib/auth/session-policy.ts @@ -30,11 +30,10 @@ const NO_POLICY: ResolvedSessionPolicy = { * downgrade. `isOrganizationOnEnterprisePlan` swallows its errors and returns * `false`, which is correct for feature gating (fail closed) but would silently * stop ENFORCING here — the fail-open trap the PII-redaction path documents in - * `lib/logs/execution/logger.ts`. Only reached when bounds are actually stored, - * so non-enterprise orgs never pay for it. + * `lib/logs/execution/logger.ts`. Callers only reach this when bounds are + * actually stored, so non-enterprise orgs never pay for it. */ -async function isPolicyEnforced(organizationId: string, hasBounds: boolean): Promise { - if (!hasBounds) return true +async function isEntitledToEnforce(organizationId: string): Promise { try { return await resolveOrganizationEnterprisePlan(organizationId) } catch (error) { @@ -78,13 +77,15 @@ export async function getSessionPolicy( .limit(1) const settings = row?.settings ?? {} - const hasBounds = Boolean(settings.maxSessionHours || settings.idleTimeoutHours) - if (!(await isPolicyEnforced(organizationId, hasBounds))) return NO_POLICY - - return { + const bounds: ResolvedSessionPolicy = { maxSessionHours: settings.maxSessionHours ?? null, idleTimeoutHours: settings.idleTimeoutHours ?? null, } + + if (!bounds.maxSessionHours && !bounds.idleTimeoutHours) return NO_POLICY + if (!(await isEntitledToEnforce(organizationId))) return NO_POLICY + + return bounds } /** From b9d804740caf9499ecc81cd230904e44ef56a441 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 18:41:21 -0700 Subject: [PATCH 08/13] refactor(auth): use the shared LRUCache instead of hand-rolled eviction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lru-cache` is already a dependency and already the house pattern for a bounded, TTL'd, process-local cache — six modules use it, and `lib/environment/utils.ts` uses the same `*_CACHE_MAX_ENTRIES` / `*_CACHE_TTL_MS` constant naming this file had adopted. The bespoke `prune`/`touch` pair, the low-water-mark ratio, the per-entry `fetchedAt` bookkeeping, and the manual TTL comparisons were all reimplementing it, less well. Replaced with two `LRUCache` instances: 41 lines in, 91 out. Behavior is preserved, including the asymmetric membership TTL (a negative result is written with a shorter per-entry `ttl` rather than being branched on at read time). Membership entries are boxed in an object because `LRUCache` constrains values to non-nullish types, and a resolved non-member has to stay distinguishable from a cache miss. The only logic that stays hand-written is the monotonic version guard, which is domain-specific rather than cache machinery: the counter only increments, so a read that resolves lower than what is cached started earlier and must be discarded rather than served. --- apps/sim/lib/auth/security-policy.ts | 132 +++++++++------------------ 1 file changed, 41 insertions(+), 91 deletions(-) diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index 171625be16b..4b900ceaafe 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -2,6 +2,7 @@ import { db } from '@sim/db' import { member, organization } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' +import { LRUCache } from 'lru-cache' import { isOrganizationsEnabled } from '@/lib/core/config/env-flags' const logger = createLogger('SecurityPolicy') @@ -21,62 +22,42 @@ const logger = createLogger('SecurityPolicy') * which is what keeps a stale version from ever pairing with a stale policy. */ export const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000 +const SECURITY_POLICY_VERSION_CACHE_MAX_ENTRIES = 5_000 -/** - * Entry caps. Both maps only ever grow through explicit writes, so without a - * bound a long-lived process accumulates one entry per distinct org/user it has - * ever served. - */ -const MAX_VERSION_CACHE_ENTRIES = 5_000 -const MAX_MEMBERSHIP_CACHE_ENTRIES = 20_000 +const MEMBERSHIP_CACHE_TTL_MS = 60 * 1000 +const MEMBERSHIP_CACHE_MAX_ENTRIES = 20_000 /** - * Fraction of the cap an over-capacity prune evicts down to. Trimming to a low - * water mark rather than exactly to the cap is what keeps eviction amortized - * O(1): pruning back to the cap would leave the very next insert over again, so - * every subsequent write would rescan the whole map. + * Negative (non-member) membership results use a much shorter TTL than + * positive ones: a user's cached `null` would otherwise let them dodge a new + * org's policy for the full TTL after joining through ANY path — including + * ones outside this codebase (Better Auth SSO JIT provisioning writes `member` + * rows straight through the adapter, and this app registers no `member` + * database hook that could observe it). Positive results change only through + * leave/transfer, which invalidate explicitly. */ -const PRUNE_TARGET_RATIO = 0.9 +const NEGATIVE_MEMBERSHIP_CACHE_TTL_MS = 15 * 1000 const DEFAULT_VERSION = 1 +const versionCache = new LRUCache({ + max: SECURITY_POLICY_VERSION_CACHE_MAX_ENTRIES, + ttl: SECURITY_POLICY_VERSION_CACHE_TTL_MS, +}) + /** - * Evicts down to a low water mark, expired entries first. Map iteration follows - * insertion order and {@link touch} re-inserts on every refresh, so whatever - * remains at the head is the least recently refreshed. + * Boxed because a resolved non-member is a cached VALUE, not a cache miss, and + * `LRUCache` constrains values to non-nullish types — so `null` cannot be + * stored directly and would be indistinguishable from a miss if it could. */ -function prune(cache: Map, maxEntries: number): void { - if (cache.size <= maxEntries) return - const target = Math.floor(maxEntries * PRUNE_TARGET_RATIO) - const cutoff = Date.now() - SECURITY_POLICY_VERSION_CACHE_TTL_MS - for (const [key, entry] of cache) { - if (cache.size <= target) break - if (entry.fetchedAt < cutoff) cache.delete(key) - } - for (const key of cache.keys()) { - if (cache.size <= target) break - cache.delete(key) - } -} - -/** Re-inserts so insertion order tracks recency of refresh, then prunes. */ -function touch( - cache: Map, - key: string, - entry: T, - maxEntries: number -): void { - cache.delete(key) - cache.set(key, entry) - prune(cache, maxEntries) -} - -interface VersionCacheEntry { - version: number - fetchedAt: number +interface MembershipCacheEntry { + organizationId: string | null } -const versionCache = new Map() +const membershipCache = new LRUCache({ + max: MEMBERSHIP_CACHE_MAX_ENTRIES, + ttl: MEMBERSHIP_CACHE_TTL_MS, +}) /** * Resolves the org's security-policy version — the shared monotonic counter @@ -95,9 +76,7 @@ export async function getSecurityPolicyVersion( if (!organizationId) return DEFAULT_VERSION const cached = versionCache.get(organizationId) - if (cached && Date.now() - cached.fetchedAt < SECURITY_POLICY_VERSION_CACHE_TTL_MS) { - return cached.version - } + if (cached !== undefined) return cached try { const [row] = await db @@ -107,19 +86,14 @@ export async function getSecurityPolicyVersion( .limit(1) const version = row?.version ?? DEFAULT_VERSION - // The counter only ever increments, so a read resolving LOWER than what is - // already cached started before the newer one. Neither store it nor return - // it: a late value would re-serve a pre-bump version, keeping cookies - // matched and letting revoked sessions stay on the cookie cache. - const current = versionCache.get(organizationId) - if (current && current.version > version) return current.version - - touch( - versionCache, - organizationId, - { version, fetchedAt: Date.now() }, - MAX_VERSION_CACHE_ENTRIES - ) + // Re-check after the await. The counter only ever increments, so a value + // that landed while this read was in flight is newer — neither store nor + // return ours, or a late read would re-serve a pre-bump version and keep + // cookies matched past a revocation. + const concurrent = versionCache.get(organizationId) + if (concurrent !== undefined && concurrent > version) return concurrent + + versionCache.set(organizationId, version) return version } catch (error) { logger.error('Failed to resolve security policy version; using default', { @@ -141,28 +115,10 @@ export async function getSecurityPolicyVersion( */ export function setSecurityPolicyVersion(organizationId: string, version: number): void { const current = versionCache.get(organizationId) - if (current && current.version > version) return - touch(versionCache, organizationId, { version, fetchedAt: Date.now() }, MAX_VERSION_CACHE_ENTRIES) -} - -interface MembershipCacheEntry { - organizationId: string | null - fetchedAt: number + if (current !== undefined && current > version) return + versionCache.set(organizationId, version) } -const membershipCache = new Map() - -/** - * Negative (non-member) membership results use a much shorter TTL than - * positive ones: a user's cached `null` would otherwise let them dodge a new - * org's policy for the full TTL after joining through ANY path — including - * ones outside this codebase (Better Auth SSO JIT provisioning writes `member` - * rows straight through the adapter, and this app registers no `member` - * database hook that could observe it). Positive results change only through - * leave/transfer, which invalidate explicitly. - */ -const NEGATIVE_MEMBERSHIP_CACHE_TTL_MS = 15 * 1000 - /** Drops the cached membership for a user (call when they join/leave an org). */ export function invalidateMembershipCache(userId: string): void { membershipCache.delete(userId) @@ -187,12 +143,7 @@ export async function getMemberOrganizationId( if (!userId) return null const cached = membershipCache.get(userId) - if (cached) { - const ttl = cached.organizationId - ? SECURITY_POLICY_VERSION_CACHE_TTL_MS - : NEGATIVE_MEMBERSHIP_CACHE_TTL_MS - if (Date.now() - cached.fetchedAt < ttl) return cached.organizationId - } + if (cached) return cached.organizationId const [row] = await db .select({ organizationId: member.organizationId }) @@ -201,11 +152,10 @@ export async function getMemberOrganizationId( .limit(1) const organizationId = row?.organizationId ?? null - touch( - membershipCache, + membershipCache.set( userId, - { organizationId, fetchedAt: Date.now() }, - MAX_MEMBERSHIP_CACHE_ENTRIES + { organizationId }, + organizationId ? undefined : { ttl: NEGATIVE_MEMBERSHIP_CACHE_TTL_MS } ) return organizationId } From 30d5496f5520022eaea1bae20743c86973ab4676 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 18:46:56 -0700 Subject: [PATCH 09/13] refactor(auth): correct a drifted docstring and two cache nits - `getSessionPolicy`'s docstring still said the CREATE hook allows the sign-in on a failed read. It has refused when the governing org is known since the fail-closed split; say so. - `SECURITY_POLICY_VERSION_CACHE_TTL_MS` had no consumer outside this module, and exporting one of the two sibling TTLs but not the other was arbitrary. - Pass an explicit `ttl` for both membership outcomes instead of relying on `undefined` to mean the constructor default, so the positive/negative asymmetry is visible at the call site. --- apps/sim/lib/auth/security-policy.ts | 4 ++-- apps/sim/lib/auth/session-policy.ts | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index 4b900ceaafe..60f543bdb21 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -21,7 +21,7 @@ const logger = createLogger('SecurityPolicy') * never cached; it is read fresh at enforcement time (see `session-policy.ts`), * which is what keeps a stale version from ever pairing with a stale policy. */ -export const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000 +const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000 const SECURITY_POLICY_VERSION_CACHE_MAX_ENTRIES = 5_000 const MEMBERSHIP_CACHE_TTL_MS = 60 * 1000 @@ -155,7 +155,7 @@ export async function getMemberOrganizationId( membershipCache.set( userId, { organizationId }, - organizationId ? undefined : { ttl: NEGATIVE_MEMBERSHIP_CACHE_TTL_MS } + { ttl: organizationId ? MEMBERSHIP_CACHE_TTL_MS : NEGATIVE_MEMBERSHIP_CACHE_TTL_MS } ) return organizationId } diff --git a/apps/sim/lib/auth/session-policy.ts b/apps/sim/lib/auth/session-policy.ts index b770785ade4..1e8467feb50 100644 --- a/apps/sim/lib/auth/session-policy.ts +++ b/apps/sim/lib/auth/session-policy.ts @@ -63,7 +63,9 @@ async function isEntitledToEnforce(organizationId: string): Promise { * than trying to keep two caches coherent. * * Throws if the read fails. Callers pick their own failure posture: the session - * UPDATE hook refuses to extend, the CREATE hook allows the sign-in. + * UPDATE hook keeps the current expiry rather than extending it, and the CREATE + * hook refuses the sign-in when the governing org is known but allows it when + * membership itself could not be resolved. */ export async function getSessionPolicy( organizationId: string | null | undefined From b237aa43ff9b93f257c48aef43c55dade2081946 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 19:09:20 -0700 Subject: [PATCH 10/13] fix(auth): clamp sessions created before their member row exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects found by an independent audit of the session lifecycle. 1. First SSO sign-in into a policy-bearing org produced an UNCLAMPED 30-day session. `@better-auth/sso` creates the session before it writes `member` (`handleOAuthUserInfo` then `assignOrganizationFromProvider`), and it writes through the adapter directly, so no member hook fires. The create hook saw no membership and applied Better Auth's default expiry — and because a 30-day expiry reads as "not due for refresh", the sliding refresh would not re-clamp it for another day. Against an 8-hour policy that is a full day of bypass, on every first SSO login, deterministically. Closed with an `after` hook that re-applies the policy once membership is visible. It runs only for sessions created without an org — the only ones that can be affected — and is best-effort so it can never break sign-in. 2. The create hook's catch branched on a local that was stale by the time it was read. When the direct membership read failed, `clampExpiryForSession` could still resolve the org from cache and then throw on the policy read; the catch saw the stale `undefined` and took the permissive branch — failing OPEN on a path whose comment promised fail-closed. Membership is now resolved once, up front, through the shared cached helper, and both the clamp and the catch use that single value. Also: normalize the update hook's fallback expiry like its happy path, and correct the `getSessionPolicy` cost note — an org with bounds stored pays the entitlement check too, so roughly four reads, not one. --- apps/sim/lib/auth/auth.ts | 112 ++++++++++++++++++--------- apps/sim/lib/auth/security-policy.ts | 5 +- apps/sim/lib/auth/session-policy.ts | 9 ++- 3 files changed, 88 insertions(+), 38 deletions(-) diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index d19055a4b63..0da42cf078b 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -34,8 +34,12 @@ import { import { getAccessControlConfig, isEmailBlockedByAccessControl } from '@/lib/auth/access-control' import { createAnonymousSession, ensureAnonymousUserExists } from '@/lib/auth/anonymous' import { getRequestedSignInProviderId, isSignInProviderAllowed } from '@/lib/auth/constants' -import { getSessionCookieCacheVersion } from '@/lib/auth/security-policy' -import { clampExpiryForSession } from '@/lib/auth/session-policy' +import { + getMemberOrganizationId, + getSessionCookieCacheVersion, + invalidateMembershipCache, +} from '@/lib/auth/security-policy' +import { applySessionPolicyToNewMember, clampExpiryForSession } from '@/lib/auth/session-policy' import { guardSubscriptionPlanWrites } from '@/lib/auth/stripe-adapter-guard' import { sendPlanWelcomeEmail } from '@/lib/billing' import { @@ -661,28 +665,29 @@ export const auth = betterAuth({ } } - // Resolved separately from the clamp below so the two failures stay - // distinguishable: `null` means "confirmed not in an org", while - // `undefined` means "could not tell". The clamp retries through the - // membership cache, and the catch below branches on which one it got. - let membershipOrgId: string | null | undefined + // Membership is resolved ONCE, here, and the clamp is handed the + // result. Resolving it in two places let the catch below branch on a + // stale local while the clamp had actually resolved the org from + // cache — failing open on a path documented as failing closed. + let membershipOrgId: string | null try { - const members = await db - .select({ organizationId: schema.member.organizationId }) - .from(schema.member) - .where(eq(schema.member.userId, session.userId)) - .limit(1) - membershipOrgId = members[0]?.organizationId ?? null + membershipOrgId = await getMemberOrganizationId(session.userId) logger.info( membershipOrgId ? 'Found organization for user' : 'No organizations found for user', { userId: session.userId, organizationId: membershipOrgId ?? undefined } ) } catch (error) { - logger.error('Could not resolve organization for new session', { + // The governing org is genuinely unknown, so a member is + // indistinguishable from the majority of users who belong to no org. + // Failing closed would take authentication down for everyone to + // protect a policy most of them do not have. Allow, unclamped: the + // after-hook above re-applies the policy once membership resolves, + // and the update hook clamps retroactively from `createdAt`. + logger.error('Could not resolve organization for new session; session not clamped', { error, userId: session.userId, }) - membershipOrgId = undefined + return { data: session } } try { @@ -698,15 +703,15 @@ export const auth = betterAuth({ }, } } catch (error) { - // Two different failures land here, and they get opposite answers. - // - // Org KNOWN: membership resolved, so this user is definitely - // governed by an org — only the policy read failed. Fail closed. - // The blast radius is org members during a window where the - // `organization` read fails but the `member` read just succeeded, - // which is narrow, loud, and self-clearing. Minting a full-length - // session for a member whose policy we could not read is the one - // outcome this feature exists to prevent. + // Membership already resolved above, so reaching here means only + // the POLICY read failed. For a member that is fail-closed: this + // user is definitely governed by an org, so the "would break + // sign-in for non-members too" argument does not apply. The blast + // radius is org members during a window where the `organization` + // read fails but the `member` read just succeeded — narrow, loud, + // and self-clearing. Minting a full-length session for a member + // whose policy we could not read is the one outcome this feature + // exists to prevent. if (membershipOrgId) { logger.error('Refusing session: org session policy could not be resolved', { error, @@ -718,17 +723,9 @@ export const auth = betterAuth({ }) } - // Org UNKNOWN: both the direct membership read and the cached - // fallback failed, so we cannot tell a governed member from the - // overwhelming majority of users who belong to no org at all. - // Failing closed here would turn a `member` read failure into a - // total authentication outage for everyone, to protect a policy - // that most of them do not have. Allow, and let it self-correct: - // the session takes the DB path at its next cookie-cache expiry - // and the update hook clamps it retroactively (the max-lifetime - // bound is measured from `createdAt`, so an over-long session - // expires immediately on that refresh). Any admin action bumps the - // security-policy version and closes the window at once. + // Confirmed non-member: the clamp short-circuits before any policy + // read for a null org, so this is defence in depth rather than a + // live path. A user with no org has no policy to violate, so allow. logger.error('Error clamping new session to org policy; session not clamped', { error, userId: session.userId, @@ -764,7 +761,9 @@ export const auth = betterAuth({ error, userId: current.userId, }) - return { data: { ...data, expiresAt: current.expiresAt } } + // Normalized like the happy path above: Better Auth context values + // can cross a serialization boundary, and the column is a timestamp. + return { data: { ...data, expiresAt: new Date(current.expiresAt) } } } }, }, @@ -1066,6 +1065,47 @@ export const auth = betterAuth({ return }), + /** + * Re-applies the org session policy to a session that was created before + * its `member` row existed. + * + * SSO JIT provisioning does exactly that: the callback creates the session + * first and only then writes `member` straight through the adapter + * (`@better-auth/sso` calls `handleOAuthUserInfo` before + * `assignOrganizationFromProvider`, and bypasses the organization plugin's + * `addMember`, so no member hook fires). The session-create hook therefore + * sees no membership and mints Better Auth's full 30-day expiry — and + * because a 30-day expiry reads as "not due for refresh", the sliding + * refresh would not re-clamp it for another day. On a first SSO sign-in + * into an org with session bounds, that is a real policy bypass. + * + * Runs only for sessions created WITHOUT an org (the sole case that can be + * affected) and is best-effort — a failure here must never break sign-in. + */ + after: createAuthMiddleware(async (ctx) => { + const created = ctx.context.newSession + if (!created?.session || created.session.activeOrganizationId) return + if ((created.session as { impersonatedBy?: string | null }).impersonatedBy) return + + try { + // Drop the negative membership entry the create hook just cached, so + // the row written moments ago by the provisioning path is visible. + invalidateMembershipCache(created.user.id) + const organizationId = await getMemberOrganizationId(created.user.id) + if (!organizationId) return + + logger.info('Applying org session policy to a session created pre-membership', { + userId: created.user.id, + organizationId, + }) + await applySessionPolicyToNewMember(created.user.id, organizationId) + } catch (error) { + logger.error('Failed to apply org session policy after session creation', { + error, + userId: created.user.id, + }) + } + }), }, plugins: [ ...(env.TURNSTILE_SECRET_KEY diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index 60f543bdb21..60bc3861c9a 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -105,7 +105,10 @@ export async function getSecurityPolicyVersion( } /** - * Publishes the authoritative version a write just committed. + * Seeds THIS process with the version a write just committed. Other instances + * still pick it up on their own next read, so org-wide propagation remains + * bounded by {@link SECURITY_POLICY_VERSION_CACHE_TTL_MS} — this only removes + * the calling instance's own re-read. * * Deliberately a SET rather than a delete. Deleting would leave no floor for the * monotonic guard above, so a read that started before the bump could land diff --git a/apps/sim/lib/auth/session-policy.ts b/apps/sim/lib/auth/session-policy.ts index 1e8467feb50..a0956442fef 100644 --- a/apps/sim/lib/auth/session-policy.ts +++ b/apps/sim/lib/auth/session-policy.ts @@ -53,7 +53,14 @@ async function isEntitledToEnforce(organizationId: string): Promise { * its stored rules at enforcement time (`lib/logs/execution/logger.ts`, * `lib/workflows/executor/execution-core.ts`). This is not a hot path: it runs * on sign-in and on a sliding session refresh, which the 24h cookie cache - * limits to roughly once per user per day — one indexed point lookup each. + * limits to roughly once per user per day. + * + * Cost is one indexed lookup for an org with no bounds stored — the common + * case. An org that DOES store bounds additionally pays the entitlement check + * (owner lookup + block status + subscription), so about four reads. That is + * affordable per sign-in, but it lands all at once for every member when a + * policy save or org-wide revocation invalidates the whole org's cookie caches + * together; watch it if enterprise orgs grow much larger. * * A cache here is what made an earlier version of this code incorrect. Better * Auth rewrites `expiresAt` to `now + 30d` on every refresh, so a refresh that From 39ef3bd375db712f8f55f755d2e76593a6208eb8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 19:16:25 -0700 Subject: [PATCH 11/13] fix(auth): prefer a mid-flight published version over the default on read failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch. The success path re-checks the cache after its await so a concurrent publish wins, but the catch path returned `DEFAULT_VERSION` unconditionally. The default is not the safe fallback it looks like — it is precisely the version a never-bumped org carries. So a lookup that started before a revoke, and then failed, could report `1` while this process already held the bumped counter: the cookie-cache version would still MATCH pre-bump cookies and the just-revoked session would keep being served from cache, which is the one thing the bump exists to stop. The catch now returns the cached value when one is present. It can only be equal to or higher than the default, so it forces the same revalidation or more. --- apps/sim/lib/auth/security-policy.test.ts | 14 +++++++++++++- apps/sim/lib/auth/security-policy.ts | 19 +++++++++++-------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/apps/sim/lib/auth/security-policy.test.ts b/apps/sim/lib/auth/security-policy.test.ts index 5e82a912946..770fbdc1c75 100644 --- a/apps/sim/lib/auth/security-policy.test.ts +++ b/apps/sim/lib/auth/security-policy.test.ts @@ -76,7 +76,19 @@ describe('security policy', () => { expect(await getSecurityPolicyVersion(orgId)).toBe(7) }) - it('falls back to the default when the read fails', async () => { + it('prefers a version published mid-flight over the default when the read fails', async () => { + const orgId = nextOrgId() + // A revoke/policy save publishes the bump while this read is in flight, + // then the read fails. Returning the default would reproduce the exact + // version a pre-bump cookie carries and keep it matching. + dbChainMockFns.limit.mockImplementationOnce(() => { + setSecurityPolicyVersion(orgId, 6) + throw new Error('connection reset') + }) + expect(await getSecurityPolicyVersion(orgId)).toBe(6) + }) + + it('falls back to the default when the read fails and nothing is cached', async () => { dbChainMockFns.limit.mockImplementationOnce(() => { throw new Error('connection reset') }) diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index 60bc3861c9a..b1e7c6e1ca9 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -66,9 +66,12 @@ const membershipCache = new LRUCache({ * planned consumers): any feature that needs cached session cookies to * re-validate bumps this one counter. * - * A failed read falls back to the default rather than the last known value. - * That errs toward MORE revalidation, not less: a version that reads lower than - * the stored one mismatches the cookie and forces a database session read. + * A failed read prefers whatever this process last knew over the default. The + * default is not the safe fallback it looks like: it is the version a + * never-bumped org carries, so returning it can MATCH a pre-bump cookie and + * keep a just-revoked session serving from the cookie cache. A retained or + * freshly published value can only be equal or higher, so it either forces the + * same revalidation or more of it. */ export async function getSecurityPolicyVersion( organizationId: string | null | undefined @@ -96,11 +99,11 @@ export async function getSecurityPolicyVersion( versionCache.set(organizationId, version) return version } catch (error) { - logger.error('Failed to resolve security policy version; using default', { - organizationId, - error, - }) - return DEFAULT_VERSION + logger.error('Failed to resolve security policy version', { organizationId, error }) + // A publish, or a concurrent read, may have landed while this one was in + // flight. Prefer it — the default could reproduce exactly the version a + // pre-bump cookie carries, leaving a revoked session on the cookie cache. + return versionCache.get(organizationId) ?? DEFAULT_VERSION } } From 5b5bb5e0494d92a6c3289eb4a7fa13f3a2617258 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 19:24:36 -0700 Subject: [PATCH 12/13] fix(auth): distrust a version read that outlived the cache TTL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch. The post-read monotonic guard can only out-rank a stale value while the entry that would out-rank it still exists. A read that stays in flight past the cache TTL has watched that entry expire, so it can store a pre-bump version with nothing left to catch it — and then serve it for another full TTL, keeping pre-bump cookies matched and revoked sessions on the cookie cache. Reaching that state needs an indexed single-row lookup to hang for a minute, so it is pathological rather than likely, but the fix is small: time the read and, if it outlived the TTL, read once more rather than trusting a value that can no longer be checked. Logged at warn, since a lookup that slow is itself worth seeing. --- apps/sim/lib/auth/security-policy.test.ts | 24 ++++++++++++++++++ apps/sim/lib/auth/security-policy.ts | 30 +++++++++++++++++------ 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/apps/sim/lib/auth/security-policy.test.ts b/apps/sim/lib/auth/security-policy.test.ts index 770fbdc1c75..17935fc357c 100644 --- a/apps/sim/lib/auth/security-policy.test.ts +++ b/apps/sim/lib/auth/security-policy.test.ts @@ -15,6 +15,7 @@ import { getSecurityPolicyVersion, getSessionCookieCacheVersion, invalidateMembershipCache, + SECURITY_POLICY_VERSION_CACHE_TTL_MS, setSecurityPolicyVersion, } from '@/lib/auth/security-policy' @@ -76,6 +77,29 @@ describe('security policy', () => { expect(await getSecurityPolicyVersion(orgId)).toBe(7) }) + it('re-reads when the lookup outlived the cache TTL', async () => { + const orgId = nextOrgId() + const realNow = Date.now + const base = realNow() + // First reading of the clock is the read's start; every later reading is + // past the TTL, so the elapsed check sees a lookup that outlived it. + let seenFirst = false + Date.now = () => { + if (seenFirst) return base + SECURITY_POLICY_VERSION_CACHE_TTL_MS + 1_000 + seenFirst = true + return base + } + + try { + // A stale pre-bump value, then the value a fresh read would see. + queueTableRows(organization, [{ version: 1 }]) + queueTableRows(organization, [{ version: 8 }]) + expect(await getSecurityPolicyVersion(orgId)).toBe(8) + } finally { + Date.now = realNow + } + }) + it('prefers a version published mid-flight over the default when the read fails', async () => { const orgId = nextOrgId() // A revoke/policy save publishes the bump while this read is in flight, diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index b1e7c6e1ca9..e78e869aced 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -21,7 +21,7 @@ const logger = createLogger('SecurityPolicy') * never cached; it is read fresh at enforcement time (see `session-policy.ts`), * which is what keeps a stale version from ever pairing with a stale policy. */ -const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000 +export const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000 const SECURITY_POLICY_VERSION_CACHE_MAX_ENTRIES = 5_000 const MEMBERSHIP_CACHE_TTL_MS = 60 * 1000 @@ -59,6 +59,15 @@ const membershipCache = new LRUCache({ ttl: MEMBERSHIP_CACHE_TTL_MS, }) +async function readStoredVersion(organizationId: string): Promise { + const [row] = await db + .select({ version: organization.securityPolicyVersion }) + .from(organization) + .where(eq(organization.id, organizationId)) + .limit(1) + return row?.version ?? DEFAULT_VERSION +} + /** * Resolves the org's security-policy version — the shared monotonic counter * behind the Better Auth cookie-cache version. It backs ALL org security @@ -82,13 +91,20 @@ export async function getSecurityPolicyVersion( if (cached !== undefined) return cached try { - const [row] = await db - .select({ version: organization.securityPolicyVersion }) - .from(organization) - .where(eq(organization.id, organizationId)) - .limit(1) + const startedAt = Date.now() + let version = await readStoredVersion(organizationId) + + // The post-read guard below can only out-rank this value while the entry + // that would out-rank it still exists. A read that outlived the cache TTL + // has watched that entry expire, so it could carry a pre-bump version with + // nothing left to catch it. Read once more instead of trusting it. + if (Date.now() - startedAt >= SECURITY_POLICY_VERSION_CACHE_TTL_MS) { + logger.warn('Security policy version read outlived the cache TTL; re-reading', { + organizationId, + }) + version = await readStoredVersion(organizationId) + } - const version = row?.version ?? DEFAULT_VERSION // Re-check after the await. The counter only ever increments, so a value // that landed while this read was in flight is newer — neither store nor // return ours, or a late read would re-serve a pre-bump version and keep From 6bb1323e219562ef9a2089f68c6a300f011db741 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 19:31:46 -0700 Subject: [PATCH 13/13] revert(auth): drop the TTL-overrun retry; document the residual instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The retry added last round does not converge. Review correctly pointed out that the replacement read can itself be slow across a later bump, in exactly the same way — so the guard needs a guard, indefinitely. That is a signal the mechanism is wrong, not that it needs another layer. The root constraint: a monotonic floor kept in a TTL cache disappears with the TTL, so a read still in flight when it expires has nothing to lose against. No amount of re-reading fixes that. Closing it needs a floor that outlives the cache, which means cross-process state — the deferred Redis invalidation follow-up — not more retries here. So the retry is reverted and the limit is documented where the guard lives. The cheap post-read comparison stays: it costs two lines, needs no timing, and covers the realistic case where the newer entry is still alive. The residual is bounded — a stale read that wins serves a pre-bump version for at most one more TTL, making worst-case propagation two TTLs rather than one. That is a slower instance of a latency this feature already documents, not a new class of failure, and it is strictly better than the pre-PR behavior where the policy itself could also be stale. --- apps/sim/lib/auth/security-policy.test.ts | 24 -------------------- apps/sim/lib/auth/security-policy.ts | 27 +++++++++++------------ 2 files changed, 13 insertions(+), 38 deletions(-) diff --git a/apps/sim/lib/auth/security-policy.test.ts b/apps/sim/lib/auth/security-policy.test.ts index 17935fc357c..770fbdc1c75 100644 --- a/apps/sim/lib/auth/security-policy.test.ts +++ b/apps/sim/lib/auth/security-policy.test.ts @@ -15,7 +15,6 @@ import { getSecurityPolicyVersion, getSessionCookieCacheVersion, invalidateMembershipCache, - SECURITY_POLICY_VERSION_CACHE_TTL_MS, setSecurityPolicyVersion, } from '@/lib/auth/security-policy' @@ -77,29 +76,6 @@ describe('security policy', () => { expect(await getSecurityPolicyVersion(orgId)).toBe(7) }) - it('re-reads when the lookup outlived the cache TTL', async () => { - const orgId = nextOrgId() - const realNow = Date.now - const base = realNow() - // First reading of the clock is the read's start; every later reading is - // past the TTL, so the elapsed check sees a lookup that outlived it. - let seenFirst = false - Date.now = () => { - if (seenFirst) return base + SECURITY_POLICY_VERSION_CACHE_TTL_MS + 1_000 - seenFirst = true - return base - } - - try { - // A stale pre-bump value, then the value a fresh read would see. - queueTableRows(organization, [{ version: 1 }]) - queueTableRows(organization, [{ version: 8 }]) - expect(await getSecurityPolicyVersion(orgId)).toBe(8) - } finally { - Date.now = realNow - } - }) - it('prefers a version published mid-flight over the default when the read fails', async () => { const orgId = nextOrgId() // A revoke/policy save publishes the bump while this read is in flight, diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index e78e869aced..33c3d250e58 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -21,7 +21,7 @@ const logger = createLogger('SecurityPolicy') * never cached; it is read fresh at enforcement time (see `session-policy.ts`), * which is what keeps a stale version from ever pairing with a stale policy. */ -export const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000 +const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000 const SECURITY_POLICY_VERSION_CACHE_MAX_ENTRIES = 5_000 const MEMBERSHIP_CACHE_TTL_MS = 60 * 1000 @@ -91,24 +91,23 @@ export async function getSecurityPolicyVersion( if (cached !== undefined) return cached try { - const startedAt = Date.now() - let version = await readStoredVersion(organizationId) - - // The post-read guard below can only out-rank this value while the entry - // that would out-rank it still exists. A read that outlived the cache TTL - // has watched that entry expire, so it could carry a pre-bump version with - // nothing left to catch it. Read once more instead of trusting it. - if (Date.now() - startedAt >= SECURITY_POLICY_VERSION_CACHE_TTL_MS) { - logger.warn('Security policy version read outlived the cache TTL; re-reading', { - organizationId, - }) - version = await readStoredVersion(organizationId) - } + const version = await readStoredVersion(organizationId) // Re-check after the await. The counter only ever increments, so a value // that landed while this read was in flight is newer — neither store nor // return ours, or a late read would re-serve a pre-bump version and keep // cookies matched past a revocation. + // + // This narrows the window; it cannot close it. A read still in flight when + // the newer entry expires finds no floor to lose against, and no amount of + // re-reading fixes that — the replacement read can be slow across a later + // bump in exactly the same way. Closing it needs a floor that outlives the + // cache, which means cross-process state (see the deferred Redis + // invalidation follow-up), not more retries here. The residual is bounded: + // a stale read that wins serves a pre-bump version for at most one more + // TTL, so worst-case propagation is two TTLs rather than one — a slower + // instance of the latency this feature already documents, not a new class + // of failure. const concurrent = versionCache.get(organizationId) if (concurrent !== undefined && concurrent > version) return concurrent