Skip to content

Commit 66220e4

Browse files
committed
Merge remote-tracking branch 'origin/staging' into zoho-6157-validate
# Conflicts: # scripts/check-api-validation-contracts.ts
2 parents ca50798 + 1377256 commit 66220e4

42 files changed

Lines changed: 2967 additions & 460 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import {
5+
auditMock,
6+
auditMockFns,
7+
authMockFns,
8+
encryptionMock,
9+
encryptionMockFns,
10+
workflowsApiUtilsMock,
11+
workflowsApiUtilsMockFns,
12+
} from '@sim/testing'
13+
import { NextRequest } from 'next/server'
14+
import { beforeEach, describe, expect, it, vi } from 'vitest'
15+
16+
const { mockCheckChatAccess } = vi.hoisted(() => ({
17+
mockCheckChatAccess: vi.fn(),
18+
}))
19+
20+
const mockCreateErrorResponse = workflowsApiUtilsMockFns.mockCreateErrorResponse
21+
const mockDecryptSecret = encryptionMockFns.mockDecryptSecret
22+
const mockRecordAudit = auditMockFns.mockRecordAudit
23+
24+
vi.mock('@sim/audit', () => auditMock)
25+
vi.mock('@/app/api/workflows/utils', () => workflowsApiUtilsMock)
26+
vi.mock('@/lib/core/security/encryption', () => encryptionMock)
27+
vi.mock('@/app/api/chat/utils', () => ({
28+
checkChatAccess: mockCheckChatAccess,
29+
}))
30+
31+
import { GET } from '@/app/api/chat/manage/[id]/password/route'
32+
33+
const passwordChat = {
34+
id: 'chat-123',
35+
workflowId: 'workflow-123',
36+
identifier: 'test-chat',
37+
title: 'Test Chat',
38+
authType: 'password',
39+
password: 'encrypted-password',
40+
}
41+
42+
function makeRequest() {
43+
return new NextRequest('http://localhost:3000/api/chat/manage/chat-123/password')
44+
}
45+
46+
function callGet() {
47+
return GET(makeRequest(), { params: Promise.resolve({ id: 'chat-123' }) })
48+
}
49+
50+
describe('Chat Password Reveal API Route', () => {
51+
beforeEach(() => {
52+
vi.clearAllMocks()
53+
54+
authMockFns.mockGetSession.mockResolvedValue({
55+
user: { id: 'user-id', name: 'Test User', email: 'user@example.com' },
56+
})
57+
58+
mockCreateErrorResponse.mockImplementation((message, status = 500) => {
59+
return new Response(JSON.stringify({ error: message }), {
60+
status,
61+
headers: { 'Content-Type': 'application/json' },
62+
})
63+
})
64+
65+
mockDecryptSecret.mockResolvedValue({ decrypted: 'super-secret' })
66+
mockCheckChatAccess.mockResolvedValue({
67+
hasAccess: true,
68+
chat: passwordChat,
69+
workspaceId: 'workspace-123',
70+
})
71+
})
72+
73+
it('should return 401 when user is not authenticated', async () => {
74+
authMockFns.mockGetSession.mockResolvedValue(null)
75+
76+
const response = await callGet()
77+
78+
expect(response.status).toBe(401)
79+
const data = await response.json()
80+
expect(data.error).toBe('Unauthorized')
81+
expect(mockDecryptSecret).not.toHaveBeenCalled()
82+
})
83+
84+
it('should return 404 when chat not found or access denied', async () => {
85+
mockCheckChatAccess.mockResolvedValue({ hasAccess: false })
86+
87+
const response = await callGet()
88+
89+
expect(response.status).toBe(404)
90+
const data = await response.json()
91+
expect(data.error).toBe('Chat not found or access denied')
92+
expect(mockCheckChatAccess).toHaveBeenCalledWith('chat-123', 'user-id')
93+
expect(mockDecryptSecret).not.toHaveBeenCalled()
94+
})
95+
96+
it('should return 404 when the chat has no password set', async () => {
97+
mockCheckChatAccess.mockResolvedValue({
98+
hasAccess: true,
99+
chat: { ...passwordChat, authType: 'public', password: null },
100+
workspaceId: 'workspace-123',
101+
})
102+
103+
const response = await callGet()
104+
105+
expect(response.status).toBe(404)
106+
const data = await response.json()
107+
expect(data.error).toBe('This chat does not have a password set')
108+
expect(mockDecryptSecret).not.toHaveBeenCalled()
109+
})
110+
111+
it('should return the decrypted password and record an audit event', async () => {
112+
const response = await callGet()
113+
114+
expect(response.status).toBe(200)
115+
const data = await response.json()
116+
expect(data.password).toBe('super-secret')
117+
expect(mockDecryptSecret).toHaveBeenCalledWith('encrypted-password')
118+
expect(mockRecordAudit).toHaveBeenCalledWith(
119+
expect.objectContaining({
120+
workspaceId: 'workspace-123',
121+
actorId: 'user-id',
122+
action: 'chat.password_viewed',
123+
resourceId: 'chat-123',
124+
})
125+
)
126+
expect(response.headers.get('Cache-Control')).toBe('private, no-store')
127+
})
128+
129+
it('should return 500 without echoing the decryption error', async () => {
130+
mockDecryptSecret.mockRejectedValue(
131+
new Error('Invalid encrypted value format. Expected "iv:encrypted:authTag"')
132+
)
133+
134+
const response = await callGet()
135+
136+
expect(response.status).toBe(500)
137+
const data = await response.json()
138+
expect(data.error).toBe('Failed to reveal chat password')
139+
expect(mockRecordAudit).not.toHaveBeenCalled()
140+
})
141+
})
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
2+
import { createLogger } from '@sim/logger'
3+
import type { NextRequest } from 'next/server'
4+
import { NextResponse } from 'next/server'
5+
import { getChatPasswordContract } from '@/lib/api/contracts/chats'
6+
import { parseRequest } from '@/lib/api/server'
7+
import { getSession } from '@/lib/auth'
8+
import { decryptSecret } from '@/lib/core/security/encryption'
9+
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
10+
import { checkChatAccess } from '@/app/api/chat/utils'
11+
import { createErrorResponse } from '@/app/api/workflows/utils'
12+
13+
export const dynamic = 'force-dynamic'
14+
15+
const logger = createLogger('ChatPasswordAPI')
16+
const PRIVATE_NO_STORE = { 'Cache-Control': 'private, no-store' } as const
17+
18+
/**
19+
* GET endpoint that reveals a chat deployment's current password.
20+
* Restricted to workspace admins (checkChatAccess requires admin permission
21+
* on the workflow's workspace); each reveal is recorded in the audit log.
22+
*/
23+
export const GET = withRouteHandler(
24+
async (request: NextRequest, context: { params: Promise<{ id: string }> }) => {
25+
try {
26+
const session = await getSession()
27+
28+
if (!session) {
29+
return createErrorResponse('Unauthorized', 401)
30+
}
31+
32+
const parsed = await parseRequest(getChatPasswordContract, request, context)
33+
if (!parsed.success) return parsed.response
34+
35+
const { id: chatId } = parsed.data.params
36+
37+
const {
38+
hasAccess,
39+
chat: chatRecord,
40+
workspaceId: chatWorkspaceId,
41+
} = await checkChatAccess(chatId, session.user.id)
42+
43+
if (!hasAccess || !chatRecord) {
44+
return createErrorResponse('Chat not found or access denied', 404)
45+
}
46+
47+
if (chatRecord.authType !== 'password' || !chatRecord.password) {
48+
return createErrorResponse('This chat does not have a password set', 404)
49+
}
50+
51+
const { decrypted } = await decryptSecret(chatRecord.password)
52+
53+
recordAudit({
54+
workspaceId: chatWorkspaceId || null,
55+
actorId: session.user.id,
56+
actorName: session.user.name,
57+
actorEmail: session.user.email,
58+
action: AuditAction.CHAT_PASSWORD_VIEWED,
59+
resourceType: AuditResourceType.CHAT,
60+
resourceId: chatId,
61+
resourceName: chatRecord.title,
62+
description: `Viewed the password for chat deployment "${chatRecord.title}"`,
63+
metadata: {
64+
identifier: chatRecord.identifier,
65+
workflowId: chatRecord.workflowId,
66+
},
67+
request,
68+
})
69+
70+
return NextResponse.json({ password: decrypted }, { headers: PRIVATE_NO_STORE })
71+
} catch (error) {
72+
logger.error('Error revealing chat password:', error)
73+
/**
74+
* Deliberately opaque: the only errors that reach here come from
75+
* decryption, whose messages describe the stored ciphertext's shape.
76+
* The logged error carries the detail for operators.
77+
*/
78+
return createErrorResponse('Failed to reveal chat password', 500)
79+
}
80+
}
81+
)

apps/sim/app/api/chat/manage/[id]/route.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,24 @@ describe('Chat Edit API Route', () => {
429429
expect(data.error).toBe('Password is required when using password protection')
430430
})
431431

432+
it('rejects a whitespace-only replacement password', async () => {
433+
authMockFns.mockGetSession.mockResolvedValue({
434+
user: { id: 'user-id' },
435+
})
436+
437+
const req = new NextRequest('http://localhost:3000/api/chat/manage/chat-123', {
438+
method: 'PATCH',
439+
body: JSON.stringify({ authType: 'password', password: ' ' }),
440+
})
441+
const response = await PATCH(req, { params: Promise.resolve({ id: 'chat-123' }) })
442+
443+
expect(response.status).toBe(400)
444+
const data = await response.json()
445+
expect(data.error).toBe('Password cannot contain only whitespace')
446+
expect(mockCheckChatAccess).not.toHaveBeenCalled()
447+
expect(mockEncryptSecret).not.toHaveBeenCalled()
448+
})
449+
432450
it('should keep the existing password when updating a password-protected chat', async () => {
433451
authMockFns.mockGetSession.mockResolvedValue({
434452
user: { id: 'user-id' },

apps/sim/app/api/chat/manage/[id]/route.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,13 @@ export const PATCH = withRouteHandler(
241241
}
242242
}
243243

244-
if (encryptedPassword) {
244+
/**
245+
* Only store a new password when the chat ends up password-protected.
246+
* Applying it unconditionally re-armed the secret that the branch above
247+
* just cleared, so `PATCH { authType: 'email', password }` persisted an
248+
* encrypted password on an email-gated chat.
249+
*/
250+
if (encryptedPassword && (authType ?? existingChat[0].authType) === 'password') {
245251
updateData.password = encryptedPassword
246252
}
247253

apps/sim/app/api/mothership/local-files/stage/route.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ import {
1313
} from '@/lib/copilot/request/http'
1414
import { encodeVfsSegment } from '@/lib/copilot/vfs/path-utils'
1515
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
16-
import { trackChatUpload } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
16+
import {
17+
trackChatUpload,
18+
WorkspaceFileKeyOwnershipError,
19+
} from '@/lib/uploads/contexts/workspace/workspace-file-manager'
1720
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
1821

1922
const logger = createLogger('StageLocalFileUploadAPI')
@@ -95,6 +98,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
9598
uploadPath: `uploads/${encodeVfsSegment(displayName)}`,
9699
})
97100
} catch (error) {
101+
if (error instanceof WorkspaceFileKeyOwnershipError) {
102+
// The caller supplied a key they may not bind — a client error, not ours.
103+
logger.warn('Rejected chat upload staging for an unowned storage key', {
104+
error: error.message,
105+
})
106+
return NextResponse.json({ error: 'Storage key is not available' }, { status: 403 })
107+
}
98108
logger.error('Failed to stage local file upload', error)
99109
return createInternalServerErrorResponse('Failed to stage local file upload')
100110
}

apps/sim/app/api/organizations/[id]/roster/route.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,8 @@ describe('GET /api/organizations/[id]/roster', () => {
174174
workspaceId: 'workspace-1',
175175
workspaceName: 'Workspace One',
176176
permission: 'admin',
177+
roleSource: 'org-admin',
178+
isBilledAccount: false,
177179
},
178180
],
179181
}),
@@ -193,6 +195,8 @@ describe('GET /api/organizations/[id]/roster', () => {
193195
workspaceId: 'workspace-1',
194196
workspaceName: 'Workspace One',
195197
permission: 'read',
198+
roleSource: 'explicit',
199+
isBilledAccount: false,
196200
},
197201
],
198202
}),

apps/sim/app/api/organizations/[id]/roster/route.ts

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,17 @@ export const GET = withRouteHandler(
8989
await expireStalePendingInvitationsForOrganization(organizationId)
9090

9191
const orgWorkspaces = await db
92-
.select({ id: workspace.id, name: workspace.name })
92+
.select({
93+
id: workspace.id,
94+
name: workspace.name,
95+
ownerId: workspace.ownerId,
96+
billedAccountUserId: workspace.billedAccountUserId,
97+
})
9398
.from(workspace)
9499
.where(and(eq(workspace.organizationId, organizationId), isNull(workspace.archivedAt)))
95100

96101
const orgWorkspaceIds = orgWorkspaces.map((ws) => ws.id)
97-
const workspaceNameById = new Map(orgWorkspaces.map((ws) => [ws.id, ws.name]))
102+
const workspaceById = new Map(orgWorkspaces.map((ws) => [ws.id, ws]))
98103
const memberUserIds = memberRows.map((row) => row.userId)
99104

100105
const memberPermissions =
@@ -117,11 +122,14 @@ export const GET = withRouteHandler(
117122

118123
const permissionsByUser = new Map<string, RosterWorkspaceAccess[]>()
119124
for (const row of memberPermissions) {
125+
const ws = workspaceById.get(row.workspaceId)
120126
const list = permissionsByUser.get(row.userId) ?? []
121127
list.push({
122128
workspaceId: row.workspaceId,
123-
workspaceName: workspaceNameById.get(row.workspaceId) ?? 'Workspace',
129+
workspaceName: ws?.name ?? 'Workspace',
124130
permission: row.permission,
131+
roleSource: ws?.ownerId === row.userId ? 'owner' : 'explicit',
132+
isBilledAccount: ws?.billedAccountUserId === row.userId,
125133
})
126134
permissionsByUser.set(row.userId, list)
127135
}
@@ -135,6 +143,14 @@ export const GET = withRouteHandler(
135143
workspaceId: ws.id,
136144
workspaceName: ws.name,
137145
permission: 'admin' as const,
146+
/**
147+
* Owner wins over the derived organization grant, matching
148+
* `getUsersWithPermissions` — otherwise the same person reads as
149+
* `owner` in the teammates list and `org-admin` here.
150+
*/
151+
roleSource:
152+
ws.ownerId === rosterMember.userId ? ('owner' as const) : ('org-admin' as const),
153+
isBilledAccount: ws.billedAccountUserId === rosterMember.userId,
138154
}))
139155
: (permissionsByUser.get(rosterMember.userId) ?? []),
140156
}
@@ -183,10 +199,13 @@ export const GET = withRouteHandler(
183199

184200
for (const row of externalPermissionRows) {
185201
const existing = externalMembersByUser.get(row.userId)
202+
const externalWorkspace = workspaceById.get(row.workspaceId)
186203
const workspaceAccess: RosterWorkspaceAccess = {
187204
workspaceId: row.workspaceId,
188-
workspaceName: workspaceNameById.get(row.workspaceId) ?? 'Workspace',
205+
workspaceName: externalWorkspace?.name ?? 'Workspace',
189206
permission: row.permission,
207+
roleSource: externalWorkspace?.ownerId === row.userId ? 'owner' : 'explicit',
208+
isBilledAccount: externalWorkspace?.billedAccountUserId === row.userId,
190209
}
191210

192211
if (existing) {
@@ -247,8 +266,11 @@ export const GET = withRouteHandler(
247266
const list = grantsByInvitation.get(row.invitationId) ?? []
248267
list.push({
249268
workspaceId: row.workspaceId,
250-
workspaceName: workspaceNameById.get(row.workspaceId) ?? 'Workspace',
269+
workspaceName: workspaceById.get(row.workspaceId)?.name ?? 'Workspace',
251270
permission: row.permission,
271+
/** A pending invitee holds no row yet, so nothing is inherited. */
272+
roleSource: 'explicit',
273+
isBilledAccount: false,
252274
})
253275
grantsByInvitation.set(row.invitationId, list)
254276
}
@@ -269,7 +291,7 @@ export const GET = withRouteHandler(
269291
const data = {
270292
members: rosterMembers,
271293
pendingInvitations,
272-
workspaces: orgWorkspaces,
294+
workspaces: orgWorkspaces.map((ws) => ({ id: ws.id, name: ws.name })),
273295
} satisfies OrganizationRoster
274296
return NextResponse.json({
275297
success: true,

0 commit comments

Comments
 (0)