fix(access-requests): scope previews and audit events correctly (#7873)

Author: waleedlatif1

apps/sim/lib/core/application/authorized-workspace-use-case.test.ts

@@ -40,6 +40,7 @@ vi.mock('@sim/platform-authz/workspace', () => ({
 
 import { AuditAction, AuditResourceType } from '@sim/audit'
 import { defineAuthorizedWorkspaceUseCase, defineWorkspaceOperation } from '@/lib/core/application'
+import { recordProjectedUseCaseAuditEntries } from '@/lib/core/application/authorized-workspace-use-case'
 import { resolveCurrentOutboundRoute } from '@/lib/core/network/context.server'
 import type { OrchestrationError } from '@/lib/core/orchestration/types'
 import { CREDENTIAL_GROUP_CREDENTIAL_USE_ACTION } from '@/lib/resource-policies/registry'
@@ -494,3 +495,34 @@ describe('defineAuthorizedWorkspaceUseCase', () => {
     )
   })
 })
+
+describe('projected audit workspace attribution', () => {
+  beforeEach(() => vi.clearAllMocks())
+
+  it.each([
+    { override: undefined, expected: 'workspace-1' },
+    { override: 'workspace-2', expected: 'workspace-2' },
+    { override: null, expected: null },
+  ])('records the canonical workspace override $override', ({ override, expected }) => {
+    recordProjectedUseCaseAuditEntries(
+      operation,
+      'workspace-1',
+      sessionPrincipal,
+      undefined,
+      [
+        {
+          action: AuditAction.FILE_UPDATED,
+          resourceType: AuditResourceType.FILE,
+          workspaceId: override,
+        },
+      ],
+      'organization-1'
+    )
+    expect(mocks.recordAudit).toHaveBeenCalledExactlyOnceWith(
+      expect.objectContaining({
+        workspaceId: expected,
+        metadata: expect.objectContaining({ organizationId: 'organization-1' }),
+      })
+    )
+  })
+})

apps/sim/lib/core/application/authorized-workspace-use-case.ts

@@ -17,8 +17,8 @@ import type { OrchestrationRequestContext } from '@/lib/core/orchestration/types
 import type { ResourcePolicyBinding } from '@/lib/resource-policies/registry'
 
 export interface WorkspaceUseCaseAuditEntry {
-  /** Canonical workspace affected by a cross-workspace mutation, when different from its authorization scope. */
-  workspaceId?: string
+  /** Canonical affected workspace; null keeps an organization event outside the authorization workspace. */
+  workspaceId?: string | null
   action: AuditActionType
   resourceType: AuditResourceTypeValue
   resourceId?: string
@@ -106,7 +106,7 @@ export function recordProjectedUseCaseAuditEntries(
   const attribution: PrincipalAuditAttribution = resolvePrincipalAuditAttribution(principal)
   for (const entry of entries) {
     recordAudit({
-      workspaceId: entry.workspaceId ?? workspaceId,
+      workspaceId: entry.workspaceId === undefined ? workspaceId : entry.workspaceId,
       actorId: attribution.actorId,
       actorName: attribution.actorName,
       action: entry.action,

apps/sim/lib/permission-access-requests/README.md

@@ -15,7 +15,7 @@ Members request access from locked features, the block picker, or **My access re
 - Fulfillment updates the current governing group. The preview lists every required change, including parent restrictions, and a conservative upper bound of affected people/workspaces. It does not create individual grants or move members between groups.
 - Approval rechecks administrator authority, requester membership identity, workspace ownership, entitlement at admission, organization preference, group resolution, and the preview fingerprint. Membership, policy, or scope changes require a fresh review or request.
 - The group/credit-limit update, final request record, and notification enqueue share one database transaction. The final decision stores its original change and impact for history; later policy edits do not rewrite it.
-- One pending request per requester/scope/target is enforced by a database index and organization serialization. Member cap requests share an organization-wide key. Submission is bounded to 25 requests per rolling day and 100 pending requests per requester/organization, in addition to HTTP rate admission.
+- One pending request per requester/scope/target is enforced by a database index and organization serialization. Member cap requests share an organization-wide key. Submission is bounded to 100 requests per rolling 24 hours and 100 pending requests per requester/organization, in addition to HTTP rate admission.
 - A usage request increases the existing member credit cap. It does not change the pooled organization budget, buy credits, or alter temporary request-rate limits.
 - Notifications recheck current membership and reviewer authority. Outbox fan-out is bounded and replay-safe; delivery to an email provider remains at-least-once across a crash after send.
 

apps/sim/lib/permission-access-requests/application/requests.test.ts

@@ -285,7 +285,7 @@ describe('create access requests', () => {
 
   it.each([
     { pending: 100, daily: 0, message: '100 pending requests' },
-    { pending: 0, daily: 25, message: 'maximum of 25 requests today' },
+    { pending: 0, daily: 100, message: '100 requests in the last 24 hours' },
   ])('enforces bounded admissions ($message)', async ({ pending, daily, message }) => {
     queueTableRows(permissionAccessRequest, [])
     queueTableRows(permissionAccessRequest, [{ total: pending }])
@@ -297,6 +297,22 @@ describe('create access requests', () => {
     expect(mocks.outbox).not.toHaveBeenCalled()
   })
 
+  it.each([25, 99])(
+    'allows a new request after %s submissions in the rolling window',
+    async (daily) => {
+      queueTableRows(permissionAccessRequest, [])
+      queueTableRows(permissionAccessRequest, [{ total: 0 }])
+      queueTableRows(permissionAccessRequest, [{ total: daily }])
+      dbChainMockFns.returning.mockResolvedValueOnce([stored()])
+      const result = await createAccessRequest.execute({ principal, input: { scope, target } })
+      expect(result.changed).toBe(true)
+      expect(dbChainMockFns.insert).toHaveBeenCalledWith(permissionAccessRequest)
+      expect(mocks.audit.mock.calls[0][4]).toEqual([
+        expect.objectContaining({ workspaceId: 'workspace', resourceId: 'request' }),
+      ])
+    }
+  )
+
   it('normalizes member cap requests to one organization-wide request across workspaces', async () => {
     const cap = stored({
       target: { kind: 'usage_limit', id: 'member' },
@@ -315,6 +331,9 @@ describe('create access requests', () => {
       input: { scope, target: { kind: 'usage_limit', id: 'member' } },
     })
     expect(first.changed).toBe(true)
+    expect(mocks.audit.mock.calls[0][4]).toEqual([
+      expect.objectContaining({ workspaceId: null, resourceId: cap.id }),
+    ])
     expect(dbChainMockFns.values).toHaveBeenCalledWith(
       expect.objectContaining({
         workspaceId: null,
@@ -489,6 +508,27 @@ describe('discovery and request history', () => {
 })
 
 describe('cancel access requests', () => {
+  it.each(['workspace', null])(
+    'keeps cancellation in the stored request scope %s',
+    async (workspaceId) => {
+      const row = stored({
+        workspaceId,
+        ...(workspaceId === null
+          ? {
+              scopeKey: 'organization:organization:member-limit',
+              target: { kind: 'usage_limit', id: 'member' },
+            }
+          : {}),
+      })
+      mocks.stored.mockResolvedValue(row)
+      dbChainMockFns.returning.mockResolvedValueOnce([{ ...row, status: 'cancelled' }])
+      await cancelAccessRequest.execute({ principal, input: { scope, requestId: row.id } })
+      expect(mocks.audit.mock.calls[0][4]).toEqual([
+        expect.objectContaining({ workspaceId, resourceId: row.id }),
+      ])
+    }
+  )
+
   it('allows cancellation while disabled and does not resend a decision for terminal requests', async () => {
     mocks.enabled.mockResolvedValue(false)
     dbChainMockFns.returning.mockResolvedValueOnce([stored({ status: 'cancelled' })])

apps/sim/lib/permission-access-requests/application/requests.ts

@@ -18,6 +18,11 @@ import {
   listAccessRequestTargets,
   loadAccessRequestCatalog,
 } from '@/lib/permission-access-requests/catalog'
+import {
+  ACCESS_REQUEST_MAX_DAILY_SUBMISSIONS,
+  ACCESS_REQUEST_MAX_PENDING,
+  ACCESS_REQUEST_SUBMISSION_WINDOW_MS,
+} from '@/lib/permission-access-requests/constants'
 import {
   PERMISSION_ACCESS_REQUEST_CREATED_EVENT,
   PERMISSION_ACCESS_REQUEST_DECIDED_EVENT,
@@ -151,7 +156,7 @@ export const discoverAccessRequests = defineAuthorizedAccessRequestUseCase({
           eq(permissionAccessRequest.status, 'pending')
         )
       )
-      .limit(100)
+      .limit(ACCESS_REQUEST_MAX_PENDING)
     const pendingMemberLimit = pending.find((row) => row.targetKey === 'usage_limit:member')
     const memberLimitMembershipMatches = pendingMemberLimit
       ? await hasCurrentMemberLimitMembership(
@@ -351,10 +356,10 @@ export const createAccessRequest = defineAuthorizedAccessRequestUseCase({
           eq(permissionAccessRequest.status, 'pending')
         )
       )
-    if ((outstanding?.total ?? 0) >= 100)
+    if ((outstanding?.total ?? 0) >= ACCESS_REQUEST_MAX_PENDING)
       throw new OrchestrationError(
         'conflict',
-        'You have 100 pending requests. Cancel an existing request before sending another.'
+        `You have ${ACCESS_REQUEST_MAX_PENDING} pending requests. Cancel an existing request before sending another.`
       )
     const [daily] = await executor
       .select({ total: count() })
@@ -363,13 +368,16 @@ export const createAccessRequest = defineAuthorizedAccessRequestUseCase({
         and(
           eq(permissionAccessRequest.organizationId, organizationId),
           eq(permissionAccessRequest.requesterId, principal.userId),
-          gte(permissionAccessRequest.createdAt, new Date(Date.now() - 86_400_000))
+          gte(
+            permissionAccessRequest.createdAt,
+            new Date(Date.now() - ACCESS_REQUEST_SUBMISSION_WINDOW_MS)
+          )
         )
       )
-    if ((daily?.total ?? 0) >= 25)
+    if ((daily?.total ?? 0) >= ACCESS_REQUEST_MAX_DAILY_SUBMISSIONS)
       throw new OrchestrationError(
         'conflict',
-        'You have sent the maximum of 25 requests today. Try again tomorrow.'
+        `You have sent ${ACCESS_REQUEST_MAX_DAILY_SUBMISSIONS} requests in the last 24 hours. Try again later.`
       )
     const [row] = await executor
       .insert(permissionAccessRequest)
@@ -402,6 +410,7 @@ export const createAccessRequest = defineAuthorizedAccessRequestUseCase({
                   action: AuditAction.PERMISSION_ACCESS_REQUEST_CLOSED,
                   resourceType: AuditResourceType.PERMISSION_ACCESS_REQUEST,
                   resourceId: result.closedRequest.id,
+                  workspaceId: result.closedRequest.workspaceId,
                   metadata: { target: result.closedRequest.target },
                 },
               ]
@@ -413,6 +422,7 @@ export const createAccessRequest = defineAuthorizedAccessRequestUseCase({
                 : AuditAction.PERMISSION_ACCESS_REQUEST_CREATED,
             resourceType: AuditResourceType.PERMISSION_ACCESS_REQUEST,
             resourceId: result.request.id,
+            workspaceId: result.request.workspaceId,
             metadata: { target: result.request.target },
           },
         ]
@@ -492,6 +502,7 @@ export const cancelAccessRequest = defineAuthorizedAccessRequestUseCase({
             action: AuditAction.PERMISSION_ACCESS_REQUEST_CANCELLED,
             resourceType: AuditResourceType.PERMISSION_ACCESS_REQUEST,
             resourceId: result.request.id,
+            workspaceId: result.request.workspaceId,
           },
         ]
       : [],

apps/sim/lib/permission-access-requests/application/review.test.ts

@@ -1,6 +1,7 @@
 /**
  * @vitest-environment node
  */
+import { AuditAction } from '@sim/audit'
 import { db } from '@sim/db'
 import {
   organizationMemberUsageLimit,
@@ -407,6 +408,44 @@ describe('permission request review', () => {
     expect(mocks.outbox).toHaveBeenCalledOnce()
   })
 
+  it.each(['workspace', null])(
+    'attributes a declined request to its stored scope %s',
+    async (workspaceId) => {
+      mocks.stored.mockResolvedValue(stored({ workspaceId }))
+      dbChainMockFns.returning.mockResolvedValueOnce([stored({ workspaceId, status: 'declined' })])
+      await resolveAccessRequest.execute({
+        principal,
+        input: { ...input, decision: { action: 'decline', reason: 'Use the existing provider.' } },
+      })
+      expect(mocks.audit.mock.calls[0][4]).toEqual([
+        expect.objectContaining({
+          action: AuditAction.PERMISSION_ACCESS_REQUEST_DECLINED,
+          workspaceId,
+        }),
+      ])
+    }
+  )
+
+  it('keeps request fulfillment workspace-scoped and the group change organization-scoped', async () => {
+    const before = await preview()
+    queueWorkspace()
+    dbChainMockFns.returning.mockResolvedValueOnce([stored({ status: 'fulfilled' })])
+    await resolveAccessRequest.execute({
+      principal,
+      input: { ...input, decision: { action: 'apply', expectedFingerprint: before.fingerprint } },
+    })
+    const [, defaultWorkspaceId, , , entries] = mocks.audit.mock.calls[0]
+    expect(defaultWorkspaceId).toBeNull()
+    expect(entries).toEqual([
+      expect.objectContaining({
+        action: AuditAction.PERMISSION_ACCESS_REQUEST_FULFILLED,
+        workspaceId: 'workspace',
+      }),
+      expect.objectContaining({ action: AuditAction.PERMISSION_GROUP_UPDATED }),
+    ])
+    expect(entries[1]).not.toHaveProperty('workspaceId')
+  })
+
   it('does not apply or notify a second time after resolution', async () => {
     mocks.stored.mockResolvedValue(stored({ status: 'fulfilled' }))
     const result = await resolveAccessRequest.execute({
@@ -417,6 +456,7 @@ describe('permission request review', () => {
     expect(dbChainMockFns.update).not.toHaveBeenCalled()
     expect(mocks.outbox).not.toHaveBeenCalled()
     expect(mocks.catalog).not.toHaveBeenCalled()
+    expect(mocks.audit.mock.calls[0][4]).toEqual([])
   })
 
   it('reads fulfilled history from its stored decision without rebuilding the catalog', async () => {
@@ -483,7 +523,11 @@ describe('member limit review', () => {
     })
     queueLimit(2000)
     dbChainMockFns.returning.mockResolvedValueOnce([
-      stored({ status: 'fulfilled', target: { kind: 'usage_limit', id: 'member' } }),
+      stored({
+        workspaceId: null,
+        status: 'fulfilled',
+        target: { kind: 'usage_limit', id: 'member' },
+      }),
     ])
     await resolveAccessRequest.execute({
       principal,
@@ -498,6 +542,16 @@ describe('member limit review', () => {
     })
     expect(mocks.setLimit).toHaveBeenCalledWith('organization', 'requester', 15, 'admin', db)
     expect(dbChainMockFns.update).not.toHaveBeenCalledWith(permissionGroup)
+    const [, defaultWorkspaceId, , , entries] = mocks.audit.mock.calls[0]
+    expect(defaultWorkspaceId).toBeNull()
+    expect(entries).toEqual([
+      expect.objectContaining({
+        action: AuditAction.PERMISSION_ACCESS_REQUEST_FULFILLED,
+        workspaceId: null,
+      }),
+      expect.objectContaining({ action: AuditAction.ORG_MEMBER_USAGE_LIMIT_CHANGED }),
+    ])
+    expect(entries[1]).not.toHaveProperty('workspaceId')
     expect(mocks.outbox).toHaveBeenCalledWith(db, PERMISSION_ACCESS_REQUEST_DECIDED_EVENT, {
       requestId: 'request',
     })

apps/sim/lib/permission-access-requests/application/review.ts

@@ -363,6 +363,7 @@ export const resolveAccessRequest = defineAuthorizedAccessRequestUseCase({
             : AuditAction.PERMISSION_ACCESS_REQUEST_DECLINED,
         resourceType: AuditResourceType.PERMISSION_ACCESS_REQUEST,
         resourceId: result.request.id,
+        workspaceId: result.request.workspaceId,
         metadata: {
           target: result.request.target,
           requesterId: result.request.requester.id,

apps/sim/lib/permission-access-requests/constants.ts

@@ -2,3 +2,6 @@ export const ACCESS_REQUEST_LIST_PAGE_SIZE = 25
 export const ACCESS_REQUEST_MAX_OFFSET = 1_000_000
 export const ACCESS_REQUEST_MAX_SEARCH_LENGTH = 200
 export const ACCESS_REQUEST_MAX_ID_LENGTH = 128
+export const ACCESS_REQUEST_MAX_DAILY_SUBMISSIONS = 100
+export const ACCESS_REQUEST_MAX_PENDING = 100
+export const ACCESS_REQUEST_SUBMISSION_WINDOW_MS = 24 * 60 * 60 * 1000