refactor(permissions): make hasWorkspaceAdminAccess and getUserEntityPermissions thin wrappers of checkWorkspaceAccess (#5430)

hasWorkspaceAdminAccess and getUserEntityPermissions('workspace', ...)
each independently re-implemented the same getWorkspaceWithOwner +
getEffectiveWorkspacePermission fetch that checkWorkspaceAccess already
does — this is exactly the duplication that let the custom-blocks POST
handler drift into two separate DB round-trips for one permission
resolution (fixed separately). Centralizing all three onto one
implementation means a future logic change or bug fix only has to
happen once, and any future caller that (accidentally) calls two of
these functions together is now at least drawing from one consistent
definition of "effective permission" instead of two that could diverge.

Adds a `permission: PermissionType | null` field to the WorkspaceAccess
return shape so getUserEntityPermissions doesn't need a second
independent getEffectiveWorkspacePermission call to get the raw value.

No behavior change: verified analytically (canAdmin is exactly
permission === 'admin' per PERMISSION_RANK) and confirmed by an
independent security review focused on argument-order and
privilege-escalation risks in this kind of refactor. 616 existing
tests across lib/workspaces, lib/credentials, lib/invitations,
lib/copilot/vfs, and the affected API routes pass unmodified (plus 2
test assertions updated for the new additive field).
This commit is contained in:
Waleed
2026-07-05 10:59:26 -07:00
committed by GitHub
parent 732e4ec975
commit 20e86543fb
2 changed files with 15 additions and 14 deletions
@@ -772,6 +772,7 @@ describe('Permission Utils', () => {
canWrite: false,
canAdmin: false,
workspace: null,
permission: null,
})
})
@@ -793,6 +794,7 @@ describe('Permission Utils', () => {
canWrite: true,
canAdmin: true,
workspace: { id: 'workspace123', ownerId: 'user123' },
permission: 'admin',
})
})
+13 -14
View File
@@ -33,6 +33,8 @@ export interface WorkspaceAccess {
canWrite: boolean
canAdmin: boolean
workspace: WorkspaceWithOwner | null
/** The viewer's raw effective permission, or `null` when the workspace doesn't exist or they have none. */
permission: PermissionType | null
}
/**
@@ -143,7 +145,14 @@ export async function checkWorkspaceAccess(
const ws = await getWorkspaceWithOwner(workspaceId)
if (!ws) {
return { exists: false, hasAccess: false, canWrite: false, canAdmin: false, workspace: null }
return {
exists: false,
hasAccess: false,
canWrite: false,
canAdmin: false,
workspace: null,
permission: null,
}
}
const permission = await getEffectiveWorkspacePermission(userId, ws)
@@ -151,7 +160,7 @@ export async function checkWorkspaceAccess(
const canWrite = permissionSatisfies(permission, 'write')
const canAdmin = permissionSatisfies(permission, 'admin')
return { exists: true, hasAccess, canWrite, canAdmin, workspace: ws }
return { exists: true, hasAccess, canWrite, canAdmin, workspace: ws, permission }
}
/**
@@ -200,11 +209,7 @@ export async function getUserEntityPermissions(
entityId: string
): Promise<PermissionType | null> {
if (entityType === 'workspace') {
const ws = await getWorkspaceWithOwner(entityId)
if (!ws) {
return null
}
return getEffectiveWorkspacePermission(userId, ws)
return (await checkWorkspaceAccess(entityId, userId)).permission
}
const result = await db
@@ -387,13 +392,7 @@ export async function hasWorkspaceAdminAccess(
userId: string,
workspaceId: string
): Promise<boolean> {
const ws = await getWorkspaceWithOwner(workspaceId)
if (!ws) {
return false
}
return (await getEffectiveWorkspacePermission(userId, ws)) === 'admin'
return (await checkWorkspaceAccess(workspaceId, userId)).canAdmin
}
/**