Skip to content

Commit 9223379

Browse files
committed
fix(chat): close the copilot public-chat bypass, surface the dead end
Cursor Bugbot found two real gaps in the public-chat gate: The admin check only ran on the REST chat routes. Copilot deploys through deployWorkflowChat, which never called it and defaults authType to public — so a write member could still ship an unauthenticated chat, which is exactly the boundary the previous commit set out to close. The check now runs there too. The helper moved to lib/chat/permissions so all three deploy surfaces share one definition; lib/ must not import from app/, and a rule duplicated per callsite is a rule that drifts. Second finding: a permission group restricted to public-only left a non-admin with no selectable mode and a submit that would 403. That combination is intentional rather than a bug — deferring to the group would let any org grant editors public deploys by narrowing the allow-list — so the form explains the dead end and blocks the submit instead of widening the gate. Adds chat-deployments.public-auth.test.ts covering the copilot path, verified to fail when the new check is removed.
1 parent 7ef7963 commit 9223379

10 files changed

Lines changed: 243 additions & 19 deletions

File tree

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,11 @@ vi.mock('@/app/api/workflows/utils', () => workflowsApiUtilsMock)
5050
vi.mock('@/lib/core/security/encryption', () => encryptionMock)
5151
vi.mock('@/app/api/chat/utils', () => ({
5252
checkChatAccess: mockCheckChatAccess,
53+
}))
54+
vi.mock('@/lib/chat/permissions', () => ({
5355
canSetPublicChatAuth: mockCanSetPublicChatAuth,
5456
}))
57+
5558
vi.mock('@/ee/access-control/utils/permission-check', () => {
5659
class ChatDeployAuthNotAllowedError extends Error {
5760
constructor() {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { NextRequest } from 'next/server'
88
import { chatIdParamsSchema, updateChatContract } from '@/lib/api/contracts/chats'
99
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
1010
import { getSession } from '@/lib/auth'
11+
import { canSetPublicChatAuth } from '@/lib/chat/permissions'
1112
import { isDev } from '@/lib/core/config/env-flags'
1213
import { encryptSecret } from '@/lib/core/security/encryption'
1314
import { getEmailDomain } from '@/lib/core/utils/urls'
@@ -18,7 +19,7 @@ import {
1819
performChatUndeploy,
1920
performFullDeploy,
2021
} from '@/lib/workflows/orchestration'
21-
import { canSetPublicChatAuth, checkChatAccess } from '@/app/api/chat/utils'
22+
import { checkChatAccess } from '@/app/api/chat/utils'
2223
import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils'
2324
import {
2425
ChatDeployAuthNotAllowedError,

apps/sim/app/api/chat/route.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ vi.mock('@/app/api/workflows/utils', () => workflowsApiUtilsMock)
3434

3535
vi.mock('@/app/api/chat/utils', () => ({
3636
checkWorkflowAccessForChatCreation: mockCheckWorkflowAccessForChatCreation,
37+
}))
38+
39+
vi.mock('@/lib/chat/permissions', () => ({
3740
canSetPublicChatAuth: mockCanSetPublicChatAuth,
3841
}))
3942

apps/sim/app/api/chat/route.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import type { NextRequest } from 'next/server'
77
import { createChatContract } from '@/lib/api/contracts/chats'
88
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
99
import { getSession } from '@/lib/auth'
10+
import { canSetPublicChatAuth } from '@/lib/chat/permissions'
1011
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1112
import { performChatDeploy } from '@/lib/workflows/orchestration'
12-
import { canSetPublicChatAuth, checkWorkflowAccessForChatCreation } from '@/app/api/chat/utils'
13+
import { checkWorkflowAccessForChatCreation } from '@/app/api/chat/utils'
1314
import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils'
1415
import {
1516
ChatDeployAuthNotAllowedError,

apps/sim/app/api/chat/utils.permissions.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({
1313
}))
1414

1515
import { beforeEach, describe, expect, it, vi } from 'vitest'
16-
import { canSetPublicChatAuth, checkWorkflowAccessForChatCreation } from '@/app/api/chat/utils'
16+
import { canSetPublicChatAuth } from '@/lib/chat/permissions'
17+
import { checkWorkflowAccessForChatCreation } from '@/app/api/chat/utils'
1718

1819
/**
1920
* Chat deployment dropped from `admin` to `write` alongside the rest of the

apps/sim/app/api/chat/utils.ts

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,6 @@ import {
88
type DeploymentAuthResult,
99
validateDeploymentAuth,
1010
} from '@/lib/core/security/deployment-auth'
11-
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
12-
13-
/**
14-
* A chat deployed with `authType: 'public'` is invocable by anyone holding the
15-
* URL, with no authentication — the same unauthenticated exposure as a public
16-
* workflow API, which is admin-only. Deploying a chat itself only needs
17-
* `write`, so this gates the exposure rather than the deployment: an editor can
18-
* ship a password/email/SSO chat, but only an admin can make one public.
19-
*
20-
* Only the *transition to* public is gated. Editing an already-public chat, or
21-
* moving it off public, stays at `write` — neither increases exposure.
22-
*/
23-
export async function canSetPublicChatAuth(userId: string, workspaceId: string): Promise<boolean> {
24-
const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId)
25-
return permission === 'admin'
26-
}
2711

2812
export function setChatAuthCookie(
2913
response: NextResponse,

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,20 @@ export function ChatDeploy({
177177
return Object.keys(newErrors).length === 0
178178
}
179179

180+
/**
181+
* A public chat is admin-only (the server enforces the same rule on both the
182+
* REST routes and the copilot use case), so a non-admin selecting it must not
183+
* be able to submit into a 403. An already-public chat stays editable.
184+
*/
185+
const { canAdmin } = useUserPermissionsContext()
186+
const publicAuthBlocked =
187+
formData.authType === 'public' && !canAdmin && existingChat?.authType !== 'public'
188+
180189
const isFormValid =
181190
isIdentifierValid &&
182191
Boolean(formData.title.trim()) &&
183192
formData.selectedOutputBlocks.length > 0 &&
193+
!publicAuthBlocked &&
184194
!isPasswordRequired(formData.authType, formData.password, existingPassword) &&
185195
(formData.authType !== 'password' || !isWhitespaceOnlyPassword(formData.password)) &&
186196
((formData.authType !== 'email' && formData.authType !== 'sso') || formData.emails.length > 0)
@@ -726,6 +736,16 @@ function AuthSelector({
726736
(type) => allowedAuthTypes === null || allowedAuthTypes.includes(type) || type === savedAuthType
727737
)
728738

739+
/**
740+
* A permission group restricted to public-only leaves a non-admin with no
741+
* deployable mode: the org allows only public, and public is admin-only. That
742+
* combination is intentional, not a bug — deferring to the group here would
743+
* let any org grant editors public deploys by narrowing the allow-list — so
744+
* the form surfaces the dead end instead of letting a submit 403.
745+
*/
746+
const noDeployableAuthType =
747+
authOptions.length > 0 && authOptions.every((type) => type === 'public') && !canSetPublic
748+
729749
useEffect(() => {
730750
if (authOptions.length > 0 && !authOptions.includes(authType)) {
731751
onAuthTypeChange(authOptions[0])
@@ -769,6 +789,12 @@ function AuthSelector({
769789
)
770790
)}
771791
</ButtonGroup>
792+
{noDeployableAuthType && (
793+
<p className='mt-1 text-[var(--text-error)] text-caption'>
794+
This workspace only allows public chats, and only admins can deploy those. Ask an admin
795+
to deploy this chat or to allow another access mode.
796+
</p>
797+
)}
772798
</div>
773799

774800
{authType === 'password' && (

apps/sim/lib/chat/permissions.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
2+
3+
/**
4+
* A chat deployed with `authType: 'public'` is invocable by anyone holding the
5+
* URL, with no authentication — the same unauthenticated exposure as a public
6+
* workflow API, which is admin-only. Deploying a chat itself only needs
7+
* `write`, so this gates the exposure rather than the deployment: an editor can
8+
* ship a password/email/SSO chat, but only an admin can make one public.
9+
*
10+
* Only the *transition to* public is gated. Editing an already-public chat, or
11+
* moving it off public, stays `write` — neither increases exposure.
12+
*
13+
* Lives in `lib/` rather than beside the chat routes because three separate
14+
* surfaces deploy chats — the REST create and update routes and the copilot use
15+
* case — and a rule duplicated per callsite is a rule that drifts. The copilot
16+
* path in particular defaults `authType` to `public`, so a missing check there
17+
* silently reopens the boundary.
18+
*/
19+
export async function canSetPublicChatAuth(userId: string, workspaceId: string): Promise<boolean> {
20+
const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId)
21+
return permission === 'admin'
22+
}
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import type { Principal } from '@sim/auth/principal'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const { mocks } = vi.hoisted(() => ({
8+
mocks: {
9+
canSetPublicChatAuth: vi.fn(),
10+
validateChatDeployAuth: vi.fn(),
11+
resolveContext: vi.fn(),
12+
resolvePermission: vi.fn(),
13+
chatDeploy: vi.fn(),
14+
chatUndeploy: vi.fn(),
15+
audit: vi.fn(),
16+
},
17+
}))
18+
19+
vi.mock('@sim/audit', () => ({
20+
AuditAction: { CHAT_DEPLOYED: 'chat.deployed', CHAT_UNDEPLOYED: 'chat.undeployed' },
21+
AuditResourceType: { WORKFLOW: 'workflow' },
22+
recordAudit: mocks.audit,
23+
}))
24+
25+
vi.mock('@sim/platform-authz/workspace', () => ({
26+
permissionSatisfies: (actual: string | null, required: string) => {
27+
const rank = { read: 1, write: 2, admin: 3 } as const
28+
return (
29+
actual !== null && rank[actual as keyof typeof rank] >= rank[required as keyof typeof rank]
30+
)
31+
},
32+
resolveEffectiveWorkspacePermission: mocks.resolvePermission,
33+
}))
34+
35+
vi.mock('@sim/db', () => {
36+
// Chainable stub: every existing-deployment lookup resolves to no rows, so the
37+
// use case takes the "new chat" path where authType defaults to public.
38+
const chain: Record<string, unknown> = {}
39+
for (const method of ['select', 'from', 'where', 'limit']) {
40+
chain[method] = vi.fn(() => chain)
41+
}
42+
;(chain as { then?: unknown }).then = (resolve: (rows: unknown[]) => unknown) => resolve([])
43+
return { db: chain, chat: { workflowId: {}, identifier: {}, archivedAt: {}, id: {} } }
44+
})
45+
46+
vi.mock('drizzle-orm', () => ({
47+
and: vi.fn(),
48+
eq: vi.fn(),
49+
isNull: vi.fn(),
50+
}))
51+
52+
vi.mock('@/lib/chat/permissions', () => ({
53+
canSetPublicChatAuth: mocks.canSetPublicChatAuth,
54+
}))
55+
56+
vi.mock('@/ee/access-control/utils/permission-check', () => {
57+
class ChatDeployAuthNotAllowedError extends Error {}
58+
return { validateChatDeployAuth: mocks.validateChatDeployAuth, ChatDeployAuthNotAllowedError }
59+
})
60+
61+
vi.mock('@/lib/workflows/application/context', () => ({
62+
resolveActiveWorkflowApplicationContext: mocks.resolveContext,
63+
}))
64+
65+
vi.mock('@/lib/workflows/orchestration', () => ({
66+
performChatDeploy: mocks.chatDeploy,
67+
performChatUndeploy: mocks.chatUndeploy,
68+
}))
69+
70+
import { deployWorkflowChat } from '@/lib/workflows/application/chat-deployments'
71+
72+
const WRITE_PRINCIPAL: Principal = {
73+
kind: 'delegated',
74+
serviceId: 'copilot',
75+
subjectUserId: 'editor-1',
76+
workspaceId: 'workspace-1',
77+
delegationId: 'copilot-1',
78+
audience: 'sim:workflows',
79+
issuedAt: new Date('2026-08-08T00:00:00Z'),
80+
expiresAt: new Date('2999-08-08T00:00:00Z'),
81+
}
82+
83+
/**
84+
* Chat deployment is `write`, but a public chat is invocable by anyone with the
85+
* URL and no auth, so the exposure is admin-only. This use case is the copilot
86+
* path and it defaults `authType` to `public`, which makes it the easiest place
87+
* for the boundary to be silently bypassed — the REST routes enforce it
88+
* separately.
89+
*/
90+
describe('copilot chat deploy cannot bypass the public-chat admin gate', () => {
91+
beforeEach(() => {
92+
vi.clearAllMocks()
93+
mocks.resolvePermission.mockResolvedValue('write')
94+
mocks.resolveContext.mockResolvedValue({
95+
workflowId: 'workflow-1',
96+
workflow: { id: 'workflow-1', name: 'wf', userId: 'owner-1', workspaceId: 'workspace-1' },
97+
workspaceId: 'workspace-1',
98+
workspaceOrganizationId: null,
99+
allowPersonalApiKeys: true,
100+
billedAccountUserId: 'billing-owner-1',
101+
})
102+
mocks.chatDeploy.mockResolvedValue({
103+
success: true,
104+
chatId: 'chat-1',
105+
chatUrl: 'http://localhost:3000/chat/x',
106+
})
107+
})
108+
109+
it('rejects a write principal defaulting to public', async () => {
110+
mocks.canSetPublicChatAuth.mockResolvedValue(false)
111+
112+
await expect(
113+
deployWorkflowChat.execute({
114+
principal: WRITE_PRINCIPAL,
115+
input: {
116+
workflowId: 'workflow-1',
117+
identifier: 'my-chat',
118+
title: 'My Chat',
119+
versionName: 'v1',
120+
versionDescription: 'first',
121+
requestId: 'req-1',
122+
},
123+
})
124+
).rejects.toMatchObject({ code: 'forbidden' })
125+
126+
expect(mocks.chatDeploy).not.toHaveBeenCalled()
127+
})
128+
129+
it('allows a write principal to deploy a password-protected chat', async () => {
130+
mocks.canSetPublicChatAuth.mockResolvedValue(false)
131+
132+
await deployWorkflowChat.execute({
133+
principal: WRITE_PRINCIPAL,
134+
input: {
135+
workflowId: 'workflow-1',
136+
identifier: 'my-chat',
137+
title: 'My Chat',
138+
authType: 'password',
139+
password: 'placeholder-value',
140+
versionName: 'v1',
141+
versionDescription: 'first',
142+
requestId: 'req-1',
143+
},
144+
})
145+
146+
expect(mocks.chatDeploy).toHaveBeenCalled()
147+
expect(mocks.canSetPublicChatAuth).not.toHaveBeenCalled()
148+
})
149+
150+
it('allows an admin principal to deploy a public chat', async () => {
151+
mocks.canSetPublicChatAuth.mockResolvedValue(true)
152+
153+
await deployWorkflowChat.execute({
154+
principal: WRITE_PRINCIPAL,
155+
input: {
156+
workflowId: 'workflow-1',
157+
identifier: 'my-chat',
158+
title: 'My Chat',
159+
authType: 'public',
160+
versionName: 'v1',
161+
versionDescription: 'first',
162+
requestId: 'req-1',
163+
},
164+
})
165+
166+
expect(mocks.chatDeploy).toHaveBeenCalledWith(expect.objectContaining({ authType: 'public' }))
167+
})
168+
})

apps/sim/lib/workflows/application/chat-deployments.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
} from '@sim/auth/principal'
88
import { chat, db } from '@sim/db'
99
import { and, eq, isNull } from 'drizzle-orm'
10+
import { canSetPublicChatAuth } from '@/lib/chat/permissions'
1011
import { OrchestrationError } from '@/lib/core/orchestration/types'
1112
import { defineAuthorizedWorkflowUseCase } from '@/lib/workflows/application/authorized-workflow-use-case'
1213
import { resolveActiveWorkflowApplicationContext } from '@/lib/workflows/application/context'
@@ -141,6 +142,20 @@ export const deployWorkflowChat = defineAuthorizedWorkflowUseCase({
141142

142143
const subjectUserId = requirePrincipalSubjectUserId(principal)
143144
if (authType !== existingDeployment?.authType) {
145+
/**
146+
* Deploying a chat needs `write`, but a public chat is invocable by anyone
147+
* holding the URL with no authentication, so the exposure itself is
148+
* admin-only — the same boundary the REST chat routes enforce. This path
149+
* defaults `authType` to `public`, so without this check a `write`
150+
* principal could ship an unauthenticated chat through copilot.
151+
*/
152+
if (
153+
authType === 'public' &&
154+
!(await canSetPublicChatAuth(subjectUserId, context.workspaceId))
155+
) {
156+
throw new OrchestrationError('forbidden', 'Only admins can deploy a public chat')
157+
}
158+
144159
try {
145160
await validateChatDeployAuth(subjectUserId, context.workspaceId, authType)
146161
} catch (error) {

0 commit comments

Comments
 (0)