Skip to content

Commit 029fa18

Browse files
committed
fix tests
1 parent e13b7fc commit 029fa18

4 files changed

Lines changed: 61 additions & 20 deletions

File tree

apps/sim/app/api/skills/[id]/members/route.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,14 @@ export const GET = withRouteHandler(async (_request: NextRequest, context: Route
9191
return NextResponse.json({ error: 'Not found' }, { status: 404 })
9292
}
9393

94-
const entries = await listSkillMembers({
95-
id: actor.skill.id,
96-
workspaceId: actor.skill.workspaceId,
97-
workspaceShared: actor.skill.workspaceShared,
98-
})
94+
const entries = await listSkillMembers(
95+
{
96+
id: actor.skill.id,
97+
workspaceId: actor.skill.workspaceId,
98+
workspaceShared: actor.skill.workspaceShared,
99+
},
100+
{ role: actor.role }
101+
)
99102

100103
const members = entries.map((entry) => ({
101104
...entry,

apps/sim/lib/skills/access.test.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,10 @@ describe('listSkillMembers', () => {
349349
])
350350
)
351351

352-
const entries = await listSkillMembers({ id: 's1', workspaceId: 'ws', workspaceShared: true })
352+
const entries = await listSkillMembers(
353+
{ id: 's1', workspaceId: 'ws', workspaceShared: true },
354+
{ role: 'admin' }
355+
)
353356
const byUser = new Map(entries.map((e) => [e.userId, e]))
354357

355358
expect(byUser.get('boss')).toMatchObject({
@@ -384,7 +387,10 @@ describe('listSkillMembers', () => {
384387
])
385388
)
386389

387-
const entries = await listSkillMembers({ id: 's1', workspaceId: 'ws', workspaceShared: false })
390+
const entries = await listSkillMembers(
391+
{ id: 's1', workspaceId: 'ws', workspaceShared: false },
392+
{ role: 'admin' }
393+
)
388394

389395
// Derived access can never be broken by explicit rows: the workspace admin
390396
// stays an active admin even with a stale revoked row.
@@ -396,4 +402,25 @@ describe('listSkillMembers', () => {
396402
roleSource: 'workspace-admin',
397403
})
398404
})
405+
406+
it('hides deny markers from non-admin viewers', async () => {
407+
dbState.results = [
408+
[{ id: 'row-1', userId: 'denied', role: 'member', status: 'revoked', joinedAt: null }],
409+
]
410+
mockGetUsersWithPermissions.mockResolvedValue(
411+
roster([
412+
{ userId: 'denied', permissionType: 'write' },
413+
{ userId: 'reader', permissionType: 'read' },
414+
])
415+
)
416+
417+
const entries = await listSkillMembers(
418+
{ id: 's1', workspaceId: 'ws', workspaceShared: true },
419+
{ role: 'member' }
420+
)
421+
422+
// A member viewer sees the active roster only — who was explicitly denied
423+
// is admin-only data.
424+
expect(entries.map((e) => e.userId)).toEqual(['reader'])
425+
})
399426
})

apps/sim/lib/skills/access.ts

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -175,19 +175,24 @@ export interface SkillMemberEntry {
175175
}
176176

177177
/**
178-
* The canonical member roster for a skill: every CURRENT workspace member
179-
* mapped through {@link resolveSkillRole}, so the list always matches what
180-
* enforcement grants. Workspace admins surface as derived admins, explicit
181-
* active rows as their role, revoked rows as removed (deny) entries, and —
182-
* while the skill is workspace-shared — remaining members as implicit members.
183-
* Explicit rows for users no longer in the workspace are ignored, exactly as
184-
* enforcement ignores them.
178+
* The canonical member roster for a skill, scoped to what the viewer may see:
179+
* every CURRENT workspace member mapped through {@link resolveSkillRole}, so
180+
* the list always matches what enforcement grants. Workspace admins surface as
181+
* derived admins, explicit active rows as their role, and — while the skill is
182+
* workspace-shared — remaining members as implicit members. Revoked rows are
183+
* deliberate per-skill deny markers and exist to be restored, so they are
184+
* included only for skill-admin viewers; other members never learn who was
185+
* denied. Explicit rows for users no longer in the workspace are ignored,
186+
* exactly as enforcement ignores them.
185187
*/
186-
export async function listSkillMembers(skillRow: {
187-
id: string
188-
workspaceId: string
189-
workspaceShared: boolean
190-
}): Promise<SkillMemberEntry[]> {
188+
export async function listSkillMembers(
189+
skillRow: {
190+
id: string
191+
workspaceId: string
192+
workspaceShared: boolean
193+
},
194+
viewer: { role: SkillMemberRole }
195+
): Promise<SkillMemberEntry[]> {
191196
const [explicitRows, workspaceMembers] = await Promise.all([
192197
db
193198
.select({
@@ -215,7 +220,10 @@ export async function listSkillMembers(skillRow: {
215220
workspaceAccess: { hasAccess: true, canAdmin },
216221
})
217222

218-
if (role === null && row?.status !== 'revoked') continue
223+
if (role === null) {
224+
// Entries only a deny marker would produce are admin-only data.
225+
if (row?.status !== 'revoked' || viewer.role !== 'admin') continue
226+
}
219227

220228
entries.push({
221229
id: row?.id ?? `${canAdmin ? 'workspace-admin' : 'workspace'}-${wsMember.userId}`,

packages/testing/src/mocks/audit.mock.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ export const auditMock = {
129129
SKILL_CREATED: 'skill.created',
130130
SKILL_UPDATED: 'skill.updated',
131131
SKILL_DELETED: 'skill.deleted',
132+
SKILL_MEMBER_ADDED: 'skill_member.added',
133+
SKILL_MEMBER_REMOVED: 'skill_member.removed',
134+
SKILL_MEMBER_ROLE_CHANGED: 'skill_member.role_changed',
132135
TABLE_CREATED: 'table.created',
133136
TABLE_UPDATED: 'table.updated',
134137
TABLE_DELETED: 'table.deleted',

0 commit comments

Comments
 (0)