From ffdea67329f8477a9b060c2b392a173c5a600abf Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:47:05 -0700 Subject: [PATCH 1/5] fix(chat): show deployment passwords to admins --- .../chat/manage/[id]/password/route.test.ts | 139 ++++++++++++++++ .../api/chat/manage/[id]/password/route.ts | 77 +++++++++ .../deploy-modal/components/chat/chat.tsx | 63 +++++++- .../components/chat/utils.test.ts | 8 + .../deploy-modal/components/chat/utils.ts | 8 + .../components/deploy-modal/deploy-modal.tsx | 1 + .../ui/generated-password-input.test.tsx | 148 ++++++++++++++++++ .../ui/generated-password-input.tsx | 69 ++++++-- apps/sim/hooks/queries/chats.ts | 22 +++ apps/sim/lib/api/contracts/chats.ts | 19 +++ packages/audit/src/types.ts | 1 + packages/testing/src/mocks/audit.mock.ts | 1 + scripts/check-api-validation-contracts.ts | 4 +- 13 files changed, 542 insertions(+), 18 deletions(-) create mode 100644 apps/sim/app/api/chat/manage/[id]/password/route.test.ts create mode 100644 apps/sim/app/api/chat/manage/[id]/password/route.ts create mode 100644 apps/sim/components/ui/generated-password-input.test.tsx diff --git a/apps/sim/app/api/chat/manage/[id]/password/route.test.ts b/apps/sim/app/api/chat/manage/[id]/password/route.test.ts new file mode 100644 index 00000000000..a1621612f38 --- /dev/null +++ b/apps/sim/app/api/chat/manage/[id]/password/route.test.ts @@ -0,0 +1,139 @@ +/** + * @vitest-environment node + */ +import { + auditMock, + auditMockFns, + authMockFns, + encryptionMock, + encryptionMockFns, + workflowsApiUtilsMock, + workflowsApiUtilsMockFns, +} from '@sim/testing' +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockCheckChatAccess } = vi.hoisted(() => ({ + mockCheckChatAccess: vi.fn(), +})) + +const mockCreateErrorResponse = workflowsApiUtilsMockFns.mockCreateErrorResponse +const mockDecryptSecret = encryptionMockFns.mockDecryptSecret +const mockRecordAudit = auditMockFns.mockRecordAudit + +vi.mock('@sim/audit', () => auditMock) +vi.mock('@/app/api/workflows/utils', () => workflowsApiUtilsMock) +vi.mock('@/lib/core/security/encryption', () => encryptionMock) +vi.mock('@/app/api/chat/utils', () => ({ + checkChatAccess: mockCheckChatAccess, +})) + +import { GET } from '@/app/api/chat/manage/[id]/password/route' + +const passwordChat = { + id: 'chat-123', + workflowId: 'workflow-123', + identifier: 'test-chat', + title: 'Test Chat', + authType: 'password', + password: 'encrypted-password', +} + +function makeRequest() { + return new NextRequest('http://localhost:3000/api/chat/manage/chat-123/password') +} + +function callGet() { + return GET(makeRequest(), { params: Promise.resolve({ id: 'chat-123' }) }) +} + +describe('Chat Password Reveal API Route', () => { + beforeEach(() => { + vi.clearAllMocks() + + authMockFns.mockGetSession.mockResolvedValue({ + user: { id: 'user-id', name: 'Test User', email: 'user@example.com' }, + }) + + mockCreateErrorResponse.mockImplementation((message, status = 500) => { + return new Response(JSON.stringify({ error: message }), { + status, + headers: { 'Content-Type': 'application/json' }, + }) + }) + + mockDecryptSecret.mockResolvedValue({ decrypted: 'super-secret' }) + mockCheckChatAccess.mockResolvedValue({ + hasAccess: true, + chat: passwordChat, + workspaceId: 'workspace-123', + }) + }) + + it('should return 401 when user is not authenticated', async () => { + authMockFns.mockGetSession.mockResolvedValue(null) + + const response = await callGet() + + expect(response.status).toBe(401) + const data = await response.json() + expect(data.error).toBe('Unauthorized') + expect(mockDecryptSecret).not.toHaveBeenCalled() + }) + + it('should return 404 when chat not found or access denied', async () => { + mockCheckChatAccess.mockResolvedValue({ hasAccess: false }) + + const response = await callGet() + + expect(response.status).toBe(404) + const data = await response.json() + expect(data.error).toBe('Chat not found or access denied') + expect(mockCheckChatAccess).toHaveBeenCalledWith('chat-123', 'user-id') + expect(mockDecryptSecret).not.toHaveBeenCalled() + }) + + it('should return 404 when the chat has no password set', async () => { + mockCheckChatAccess.mockResolvedValue({ + hasAccess: true, + chat: { ...passwordChat, authType: 'public', password: null }, + workspaceId: 'workspace-123', + }) + + const response = await callGet() + + expect(response.status).toBe(404) + const data = await response.json() + expect(data.error).toBe('This chat does not have a password set') + expect(mockDecryptSecret).not.toHaveBeenCalled() + }) + + it('should return the decrypted password and record an audit event', async () => { + const response = await callGet() + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.password).toBe('super-secret') + expect(mockDecryptSecret).toHaveBeenCalledWith('encrypted-password') + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + workspaceId: 'workspace-123', + actorId: 'user-id', + action: 'chat.password_viewed', + resourceId: 'chat-123', + }) + ) + expect(response.headers.get('Cache-Control')).toBe('private, no-store') + }) + + it('should return 500 when decryption fails', async () => { + mockDecryptSecret.mockRejectedValue(new Error('Decryption failed')) + + const response = await callGet() + + expect(response.status).toBe(500) + const data = await response.json() + expect(data.error).toBe('Decryption failed') + expect(mockRecordAudit).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/api/chat/manage/[id]/password/route.ts b/apps/sim/app/api/chat/manage/[id]/password/route.ts new file mode 100644 index 00000000000..ca5c7e111af --- /dev/null +++ b/apps/sim/app/api/chat/manage/[id]/password/route.ts @@ -0,0 +1,77 @@ +import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' +import type { NextRequest } from 'next/server' +import { NextResponse } from 'next/server' +import { getChatPasswordContract } from '@/lib/api/contracts/chats' +import { parseRequest } from '@/lib/api/server' +import { getSession } from '@/lib/auth' +import { decryptSecret } from '@/lib/core/security/encryption' +import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { checkChatAccess } from '@/app/api/chat/utils' +import { createErrorResponse } from '@/app/api/workflows/utils' + +export const dynamic = 'force-dynamic' + +const logger = createLogger('ChatPasswordAPI') +const PRIVATE_NO_STORE = { 'Cache-Control': 'private, no-store' } as const + +/** + * GET endpoint that reveals a chat deployment's current password. + * Restricted to workspace admins (checkChatAccess requires admin permission + * on the workflow's workspace); each reveal is recorded in the audit log. + */ +export const GET = withRouteHandler( + async (request: NextRequest, context: { params: Promise<{ id: string }> }) => { + try { + const session = await getSession() + + if (!session) { + return createErrorResponse('Unauthorized', 401) + } + + const parsed = await parseRequest(getChatPasswordContract, request, context) + if (!parsed.success) return parsed.response + + const { id: chatId } = parsed.data.params + + const { + hasAccess, + chat: chatRecord, + workspaceId: chatWorkspaceId, + } = await checkChatAccess(chatId, session.user.id) + + if (!hasAccess || !chatRecord) { + return createErrorResponse('Chat not found or access denied', 404) + } + + if (chatRecord.authType !== 'password' || !chatRecord.password) { + return createErrorResponse('This chat does not have a password set', 404) + } + + const { decrypted } = await decryptSecret(chatRecord.password) + + recordAudit({ + workspaceId: chatWorkspaceId || null, + actorId: session.user.id, + actorName: session.user.name, + actorEmail: session.user.email, + action: AuditAction.CHAT_PASSWORD_VIEWED, + resourceType: AuditResourceType.CHAT, + resourceId: chatId, + resourceName: chatRecord.title, + description: `Viewed the password for chat deployment "${chatRecord.title}"`, + metadata: { + identifier: chatRecord.identifier, + workflowId: chatRecord.workflowId, + }, + request, + }) + + return NextResponse.json({ password: decrypted }, { headers: PRIVATE_NO_STORE }) + } catch (error) { + logger.error('Error revealing chat password:', error) + return createErrorResponse(getErrorMessage(error, 'Failed to reveal chat password'), 500) + } + } +) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx index f08467fe3e6..1ff5adad716 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx @@ -31,6 +31,7 @@ import { type ChatFormData, useCreateChat, useDeleteChat, + useRevealChatPassword, useUpdateChat, } from '@/hooks/queries/chats' import type { ChatDetail } from '@/hooks/queries/deployments' @@ -41,6 +42,7 @@ import { getPasswordPlaceholder, hasExistingPassword, isPasswordRequired, + shouldConfirmPasswordChange, } from './utils' const logger = createLogger('ChatDeploy') @@ -57,6 +59,7 @@ interface ChatDeployProps { onRefetchChat: () => Promise chatSubmitting: boolean setChatSubmitting: (submitting: boolean) => void + canRevealPassword: boolean onValidationChange?: (isValid: boolean) => void showDeleteConfirmation?: boolean setShowDeleteConfirmation?: (show: boolean) => void @@ -97,6 +100,7 @@ export function ChatDeploy({ onRefetchChat, chatSubmitting, setChatSubmitting, + canRevealPassword, onValidationChange, showDeleteConfirmation: externalShowDeleteConfirmation, setShowDeleteConfirmation: externalSetShowDeleteConfirmation, @@ -106,6 +110,7 @@ export function ChatDeploy({ }: ChatDeployProps) { const [imageUrl, setImageUrl] = useState(null) const [internalShowDeleteConfirmation, setInternalShowDeleteConfirmation] = useState(false) + const [showPasswordChangeConfirmation, setShowPasswordChangeConfirmation] = useState(false) const showDeleteConfirmation = externalShowDeleteConfirmation !== undefined @@ -213,9 +218,7 @@ export function ChatDeploy({ } }, [existingChat, isLoadingChat]) - const handleSubmit = async (e?: React.FormEvent) => { - if (e) e.preventDefault() - + const submitChat = async (passwordChangeConfirmed = false) => { if (chatSubmitting) return setChatSubmitting(true) @@ -227,14 +230,20 @@ export function ChatDeploy({ try { if (!validateForm()) { newTab?.close() - setChatSubmitting(false) return } if (!isIdentifierValid && formData.identifier !== existingChat?.identifier) { newTab?.close() setError('identifier', 'Please wait for identifier validation to complete') - setChatSubmitting(false) + return + } + + if ( + !passwordChangeConfirmed && + shouldConfirmPasswordChange(Boolean(existingChat?.id), formData.authType, formData.password) + ) { + setShowPasswordChangeConfirmation(true) return } @@ -283,6 +292,11 @@ export function ChatDeploy({ } } + const handleSubmit = async (event: React.FormEvent) => { + event.preventDefault() + await submitChat() + } + const handleDelete = async () => { if (!existingChat || !existingChat.id) return @@ -306,6 +320,11 @@ export function ChatDeploy({ } } + const handleConfirmPasswordChange = async () => { + setShowPasswordChangeConfirmation(false) + await submitChat(true) + } + if (isLoadingChat) { return } @@ -404,6 +423,8 @@ export function ChatDeploy({ + + = { } function AuthSelector({ + chatId, + canRevealPassword, authType, savedAuthType, password, @@ -651,6 +691,7 @@ function AuthSelector({ }: AuthSelectorProps) { const [emailError, setEmailError] = useState('') const [invalidEmailItems, setInvalidEmailItems] = useState([]) + const revealPasswordMutation = useRevealChatPassword() const emailsRef = useRef(emails) const invalidEmailItemsRef = useRef(invalidEmailItems) @@ -756,9 +797,19 @@ function AuthSelector({ value={password} onChange={onPasswordChange} disabled={disabled} - placeholder={getPasswordPlaceholder(hasExistingPassword)} + placeholder={hasExistingPassword ? '' : getPasswordPlaceholder(false)} required={!hasExistingPassword} + fetchCurrentPassword={ + canRevealPassword && chatId && hasExistingPassword + ? () => revealPasswordMutation.mutateAsync({ chatId }) + : undefined + } /> + {canRevealPassword && revealPasswordMutation.isError && ( +

+ Failed to load the current password +

+ )}

{getPasswordHelperText(hasExistingPassword)}

diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.test.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.test.ts index ce35a849135..4c4a45f4d75 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.test.ts @@ -4,6 +4,7 @@ import { getPasswordPlaceholder, hasExistingPassword, isPasswordRequired, + shouldConfirmPasswordChange, } from './utils' describe.concurrent('chat password state', () => { @@ -29,4 +30,11 @@ describe.concurrent('chat password state', () => { expect(getPasswordPlaceholder(false)).toBe('Enter password') expect(getPasswordHelperText(false)).toBe('This password will be required to access your chat') }) + + it('confirms a password change only for an existing password deployment with a new value', () => { + expect(shouldConfirmPasswordChange(true, 'password', 'new-password')).toBe(true) + expect(shouldConfirmPasswordChange(true, 'password', '')).toBe(false) + expect(shouldConfirmPasswordChange(true, 'public', 'new-password')).toBe(false) + expect(shouldConfirmPasswordChange(false, 'password', 'new-password')).toBe(false) + }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts index bb1967aba9c..630a1eed887 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts @@ -19,6 +19,14 @@ export function isPasswordRequired( return authType === 'password' && !existingPassword && !password.trim() } +export function shouldConfirmPasswordChange( + hasExistingChat: boolean, + authType: AuthType, + password: string +): boolean { + return hasExistingChat && authType === 'password' && password.length > 0 +} + export function getPasswordPlaceholder(existingPassword: boolean): string { return existingPassword ? 'Enter new password to change' : 'Enter password' } diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/deploy-modal.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/deploy-modal.tsx index aa33d96cc92..d46e57a420c 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/deploy-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/deploy-modal.tsx @@ -576,6 +576,7 @@ export function DeployModal({ onRefetchChat={handleRefetchChat} chatSubmitting={chatSubmitting} setChatSubmitting={setChatSubmitting} + canRevealPassword={userPermissions.canAdmin} onValidationChange={setIsChatFormValid} onDeploymentComplete={handleCloseModal} onDeployed={handleChatDeployed} diff --git a/apps/sim/components/ui/generated-password-input.test.tsx b/apps/sim/components/ui/generated-password-input.test.tsx new file mode 100644 index 00000000000..fa7132d84ab --- /dev/null +++ b/apps/sim/components/ui/generated-password-input.test.tsx @@ -0,0 +1,148 @@ +/** + * @vitest-environment jsdom + */ +import { act, type ButtonHTMLAttributes, type InputHTMLAttributes, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { GeneratedPasswordInput } from '@/components/ui/generated-password-input' + +vi.mock('@sim/emcn', () => ({ + Button: ({ + children, + variant: _variant, + ...props + }: { + children?: ReactNode + variant?: string + } & ButtonHTMLAttributes) => , + ChipInput: ({ + endAdornment, + error: _error, + ...props + }: { + endAdornment?: ReactNode + error?: boolean + } & InputHTMLAttributes) => ( +
+ + {endAdornment} +
+ ), + Loader: () => , + Tooltip: { + Root: ({ children }: { children?: ReactNode }) => children, + Trigger: ({ children }: { children?: ReactNode }) => children, + Content: () => null, + }, +})) + +let container: HTMLDivElement +let root: Root + +interface RenderInputOptions { + fetchCurrentPassword?: () => Promise + onChange?: (value: string) => void + showGenerate?: boolean +} + +function renderInput({ + fetchCurrentPassword, + onChange = vi.fn(), + showGenerate = false, +}: RenderInputOptions = {}) { + act(() => { + root.render( + + ) + }) +} + +function passwordInput(): HTMLInputElement { + const input = container.querySelector('[data-testid="password-input"]') + if (!input) throw new Error('Password input was not rendered') + return input +} + +function passwordButton(label: string): HTMLButtonElement { + const button = container.querySelector(`[aria-label="${label}"]`) + if (!button) throw new Error(`${label} button was not rendered`) + return button +} + +describe('GeneratedPasswordInput', () => { + beforeEach(() => { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + container.remove() + vi.clearAllMocks() + }) + + it('shows a masked placeholder without fetching the saved password', () => { + const fetchCurrentPassword = vi.fn().mockResolvedValue('saved-secret') + + renderInput({ fetchCurrentPassword }) + + expect(fetchCurrentPassword).not.toHaveBeenCalled() + expect(passwordInput()).toHaveAttribute('type', 'password') + expect(passwordInput()).toHaveValue('') + expect(passwordInput()).toHaveAttribute('placeholder', '••••••••') + }) + + it('fetches the saved password on reveal and reuses it when toggled', async () => { + const fetchCurrentPassword = vi.fn().mockResolvedValue('saved-secret') + + renderInput({ fetchCurrentPassword }) + await act(async () => passwordButton('Show password').click()) + + expect(fetchCurrentPassword).toHaveBeenCalledOnce() + expect(passwordInput()).toHaveAttribute('type', 'text') + expect(passwordInput()).toHaveValue('saved-secret') + + act(() => passwordButton('Hide password').click()) + expect(passwordInput()).toHaveAttribute('type', 'password') + + await act(async () => passwordButton('Show password').click()) + expect(fetchCurrentPassword).toHaveBeenCalledOnce() + }) + + it('stays masked when loading the saved password fails', async () => { + renderInput({ fetchCurrentPassword: vi.fn().mockRejectedValue(new Error('Failed')) }) + + await act(async () => passwordButton('Show password').click()) + + expect(passwordInput()).toHaveAttribute('type', 'password') + expect(passwordInput()).toHaveValue('') + expect(passwordInput()).toHaveAttribute('placeholder', '••••••••') + }) + + it('stays empty when no saved-password loader is provided', () => { + renderInput() + + expect(passwordInput()).toHaveAttribute('type', 'password') + expect(passwordInput()).toHaveValue('') + expect(passwordInput()).not.toHaveAttribute('placeholder', '••••••••') + }) + + it('reveals a generated password without fetching the saved password', () => { + const fetchCurrentPassword = vi.fn().mockResolvedValue('saved-secret') + const onChange = vi.fn() + renderInput({ fetchCurrentPassword, onChange, showGenerate: true }) + + act(() => passwordButton('Generate password').click()) + + expect(fetchCurrentPassword).not.toHaveBeenCalled() + expect(passwordInput()).toHaveAttribute('type', 'text') + expect(onChange).toHaveBeenCalledWith(expect.stringMatching(/^.{24}$/)) + }) +}) diff --git a/apps/sim/components/ui/generated-password-input.tsx b/apps/sim/components/ui/generated-password-input.tsx index 9993878d60d..8f495a63776 100644 --- a/apps/sim/components/ui/generated-password-input.tsx +++ b/apps/sim/components/ui/generated-password-input.tsx @@ -1,10 +1,12 @@ 'use client' import { useEffect, useState } from 'react' -import { Button, ChipInput, Tooltip } from '@sim/emcn' +import { Button, ChipInput, Loader, Tooltip } from '@sim/emcn' import { Check, Clipboard, Eye, EyeOff, RefreshCw } from 'lucide-react' import { generatePassword } from '@/lib/core/security/encryption' +const MASKED_PASSWORD = '••••••••' + interface GeneratedPasswordInputProps { value: string onChange: (value: string) => void @@ -15,6 +17,11 @@ interface GeneratedPasswordInputProps { required?: boolean autoComplete?: string error?: boolean + /** + * Resolves the currently saved password when the Show toggle is clicked. + * While hidden, an empty field displays a masked placeholder. + */ + fetchCurrentPassword?: () => Promise } /** @@ -31,9 +38,12 @@ export function GeneratedPasswordInput({ required = false, autoComplete = 'new-password', error = false, + fetchCurrentPassword, }: GeneratedPasswordInputProps) { const [showPassword, setShowPassword] = useState(false) const [copySuccess, setCopySuccess] = useState(false) + const [currentPassword, setCurrentPassword] = useState(null) + const [isFetchingCurrent, setIsFetchingCurrent] = useState(false) useEffect(() => { if (!copySuccess) return @@ -41,17 +51,50 @@ export function GeneratedPasswordInput({ return () => clearTimeout(timer) }, [copySuccess]) + const displayValue = currentPassword ?? value + const displayPlaceholder = fetchCurrentPassword && !displayValue ? MASKED_PASSWORD : placeholder + const copyToClipboard = () => { - navigator.clipboard.writeText(value) + navigator.clipboard.writeText(displayValue) setCopySuccess(true) } + const handleChange = (nextValue: string) => { + setCurrentPassword(null) + onChange(nextValue) + } + + const handleGeneratePassword = () => { + handleChange(generatePassword(24)) + setShowPassword(true) + } + + const toggleShowPassword = async () => { + if (showPassword) { + setShowPassword(false) + return + } + + if (!displayValue && fetchCurrentPassword) { + setIsFetchingCurrent(true) + try { + setCurrentPassword(await fetchCurrentPassword()) + } catch { + return + } finally { + setIsFetchingCurrent(false) + } + } + + setShowPassword(true) + } + return ( onChange(e.target.value)} + placeholder={displayPlaceholder} + value={displayValue} + onChange={(e) => handleChange(e.target.value)} disabled={disabled} required={required} autoComplete={autoComplete} @@ -64,7 +107,7 @@ export function GeneratedPasswordInput({ diff --git a/apps/sim/hooks/queries/chats.ts b/apps/sim/hooks/queries/chats.ts index b0e4c37a4ab..c1ce61a442a 100644 --- a/apps/sim/hooks/queries/chats.ts +++ b/apps/sim/hooks/queries/chats.ts @@ -11,6 +11,7 @@ import { type DeployedChatAuthBody, type DeployedChatConfig, deleteChatContract, + getChatPasswordContract, getDeployedChatConfigContract, requestChatEmailOtpContract, type UpdateChatBody, @@ -378,3 +379,24 @@ export function useDeleteChat() { }, }) } + +interface RevealChatPasswordVariables { + chatId: string +} + +/** + * Mutation hook that fetches a chat deployment's current password for + * workspace admins. Modeled as a mutation (despite the GET) so the decrypted + * password is never retained in the query cache. + */ +export function useRevealChatPassword() { + return useMutation({ + mutationFn: async ({ chatId }: RevealChatPasswordVariables): Promise => { + const result = await requestJson(getChatPasswordContract, { params: { id: chatId } }) + return result.password + }, + onError: (error) => { + logger.error('Failed to reveal chat password', { error }) + }, + }) +} diff --git a/apps/sim/lib/api/contracts/chats.ts b/apps/sim/lib/api/contracts/chats.ts index 528e20c5c28..499e2032b25 100644 --- a/apps/sim/lib/api/contracts/chats.ts +++ b/apps/sim/lib/api/contracts/chats.ts @@ -86,6 +86,11 @@ export const deleteChatResponseSchema = z.object({ message: z.string(), }) +export const chatPasswordResponseSchema = z.object({ + password: z.string(), +}) +export type ChatPasswordResponse = z.output + export const deployedChatConfigSchema = z.object({ id: z.string(), title: z.string(), @@ -292,3 +297,17 @@ export const deleteChatContract = defineRouteContract({ schema: deleteChatResponseSchema, }, }) + +/** + * Admin-only reveal of a chat deployment's current password. The route + * decrypts the stored password after re-verifying workspace admin access. + */ +export const getChatPasswordContract = defineRouteContract({ + method: 'GET', + path: '/api/chat/manage/[id]/password', + params: chatIdParamsSchema, + response: { + mode: 'json', + schema: chatPasswordResponseSchema, + }, +}) diff --git a/packages/audit/src/types.ts b/packages/audit/src/types.ts index 43b409bcc3d..cb94f7fa435 100644 --- a/packages/audit/src/types.ts +++ b/packages/audit/src/types.ts @@ -18,6 +18,7 @@ export const AuditAction = { CHAT_DEPLOYED: 'chat.deployed', CHAT_UPDATED: 'chat.updated', CHAT_DELETED: 'chat.deleted', + CHAT_PASSWORD_VIEWED: 'chat.password_viewed', // Custom Blocks (deploy-as-block) CUSTOM_BLOCK_PUBLISHED: 'custom_block.published', diff --git a/packages/testing/src/mocks/audit.mock.ts b/packages/testing/src/mocks/audit.mock.ts index 7c708b92e7d..377bbc4e6ec 100644 --- a/packages/testing/src/mocks/audit.mock.ts +++ b/packages/testing/src/mocks/audit.mock.ts @@ -40,6 +40,7 @@ export const auditMock = { CHAT_DEPLOYED: 'chat.deployed', CHAT_UPDATED: 'chat.updated', CHAT_DELETED: 'chat.deleted', + CHAT_PASSWORD_VIEWED: 'chat.password_viewed', CREDENTIAL_CREATED: 'credential.created', CREDENTIAL_UPDATED: 'credential.updated', CREDENTIAL_RENAMED: 'credential.renamed', diff --git a/scripts/check-api-validation-contracts.ts b/scripts/check-api-validation-contracts.ts index 7c744ed7c6b..d799d13d14a 100644 --- a/scripts/check-api-validation-contracts.ts +++ b/scripts/check-api-validation-contracts.ts @@ -9,8 +9,8 @@ const QUERY_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/queries') const SELECTOR_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/selectors') const BASELINE = { - totalRoutes: 1003, - zodRoutes: 1003, + totalRoutes: 1004, + zodRoutes: 1004, nonZodRoutes: 0, } as const From a3d1c0ee86ba7ad3611baa56457eea3c5e440a93 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Sat, 1 Aug 2026 18:15:20 -0700 Subject: [PATCH 2/5] fix(chat): reject whitespace-only passwords --- .../sim/app/api/chat/manage/[id]/route.test.ts | 18 ++++++++++++++++++ .../deploy-modal/components/chat/chat.tsx | 4 ++++ .../deploy-modal/components/chat/utils.test.ts | 8 ++++++++ .../deploy-modal/components/chat/utils.ts | 6 +++++- apps/sim/lib/api/contracts/chats.ts | 11 +++++++++-- 5 files changed, 44 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/chat/manage/[id]/route.test.ts b/apps/sim/app/api/chat/manage/[id]/route.test.ts index 81ab09ae02e..04a68cb661d 100644 --- a/apps/sim/app/api/chat/manage/[id]/route.test.ts +++ b/apps/sim/app/api/chat/manage/[id]/route.test.ts @@ -429,6 +429,24 @@ describe('Chat Edit API Route', () => { expect(data.error).toBe('Password is required when using password protection') }) + it('rejects a whitespace-only replacement password', async () => { + authMockFns.mockGetSession.mockResolvedValue({ + user: { id: 'user-id' }, + }) + + const req = new NextRequest('http://localhost:3000/api/chat/manage/chat-123', { + method: 'PATCH', + body: JSON.stringify({ authType: 'password', password: ' ' }), + }) + const response = await PATCH(req, { params: Promise.resolve({ id: 'chat-123' }) }) + + expect(response.status).toBe(400) + const data = await response.json() + expect(data.error).toBe('Password cannot contain only whitespace') + expect(mockCheckChatAccess).not.toHaveBeenCalled() + expect(mockEncryptSecret).not.toHaveBeenCalled() + }) + it('should keep the existing password when updating a password-protected chat', async () => { authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'user-id' }, diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx index 1ff5adad716..b2d2f575687 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx @@ -42,6 +42,7 @@ import { getPasswordPlaceholder, hasExistingPassword, isPasswordRequired, + isWhitespaceOnlyPassword, shouldConfirmPasswordChange, } from './utils' @@ -158,6 +159,8 @@ export function ChatDeploy({ if (isPasswordRequired(formData.authType, formData.password, existingPassword)) { newErrors.password = 'Password is required when using password protection' + } else if (formData.authType === 'password' && isWhitespaceOnlyPassword(formData.password)) { + newErrors.password = 'Password cannot contain only whitespace' } if ( @@ -180,6 +183,7 @@ export function ChatDeploy({ Boolean(formData.title.trim()) && formData.selectedOutputBlocks.length > 0 && !isPasswordRequired(formData.authType, formData.password, existingPassword) && + (formData.authType !== 'password' || !isWhitespaceOnlyPassword(formData.password)) && ((formData.authType !== 'email' && formData.authType !== 'sso') || formData.emails.length > 0) useEffect(() => { diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.test.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.test.ts index 4c4a45f4d75..16c41e65652 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.test.ts @@ -4,6 +4,7 @@ import { getPasswordPlaceholder, hasExistingPassword, isPasswordRequired, + isWhitespaceOnlyPassword, shouldConfirmPasswordChange, } from './utils' @@ -24,6 +25,12 @@ describe.concurrent('chat password state', () => { expect(isPasswordRequired('password', '', true)).toBe(false) }) + it('identifies whitespace-only password values', () => { + expect(isWhitespaceOnlyPassword(' ')).toBe(true) + expect(isWhitespaceOnlyPassword('')).toBe(false) + expect(isWhitespaceOnlyPassword(' password ')).toBe(false) + }) + it('returns copy that matches the stored-password state', () => { expect(getPasswordPlaceholder(true)).toBe('Enter new password to change') expect(getPasswordHelperText(true)).toBe('Leave empty to keep the current password') @@ -34,6 +41,7 @@ describe.concurrent('chat password state', () => { it('confirms a password change only for an existing password deployment with a new value', () => { expect(shouldConfirmPasswordChange(true, 'password', 'new-password')).toBe(true) expect(shouldConfirmPasswordChange(true, 'password', '')).toBe(false) + expect(shouldConfirmPasswordChange(true, 'password', ' ')).toBe(false) expect(shouldConfirmPasswordChange(true, 'public', 'new-password')).toBe(false) expect(shouldConfirmPasswordChange(false, 'password', 'new-password')).toBe(false) }) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts index 630a1eed887..d59293a355c 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts @@ -19,12 +19,16 @@ export function isPasswordRequired( return authType === 'password' && !existingPassword && !password.trim() } +export function isWhitespaceOnlyPassword(password: string): boolean { + return password.length > 0 && password.trim().length === 0 +} + export function shouldConfirmPasswordChange( hasExistingChat: boolean, authType: AuthType, password: string ): boolean { - return hasExistingChat && authType === 'password' && password.length > 0 + return hasExistingChat && authType === 'password' && password.trim().length > 0 } export function getPasswordPlaceholder(existingPassword: boolean): string { diff --git a/apps/sim/lib/api/contracts/chats.ts b/apps/sim/lib/api/contracts/chats.ts index 499e2032b25..da9db8334bd 100644 --- a/apps/sim/lib/api/contracts/chats.ts +++ b/apps/sim/lib/api/contracts/chats.ts @@ -4,6 +4,13 @@ import { defineRouteContract } from '@/lib/api/contracts/types' export const chatAuthTypeSchema = z.enum(['public', 'password', 'email', 'sso']) export type ChatAuthType = z.output +export const chatDeploymentPasswordSchema = z + .string() + .refine( + (password) => password.length === 0 || password.trim().length > 0, + 'Password cannot contain only whitespace' + ) + export const chatIdParamsSchema = z.object({ id: z.string().min(1), }) @@ -38,7 +45,7 @@ export const createChatBodySchema = z.object({ description: z.string().optional(), customizations: chatCustomizationsSchema, authType: chatAuthTypeSchema.default('public'), - password: z.string().optional(), + password: chatDeploymentPasswordSchema.optional(), allowedEmails: z.array(z.string()).optional().default([]), outputConfigs: z.array(chatOutputConfigSchema).optional().default([]), /** When true, clients may receive thinking SSE if they also send the protocol header. Default off. */ @@ -59,7 +66,7 @@ export const updateChatBodySchema = z.object({ description: z.string().optional(), customizations: chatCustomizationsSchema.optional(), authType: chatAuthTypeSchema.optional(), - password: z.string().optional(), + password: chatDeploymentPasswordSchema.optional(), allowedEmails: z.array(z.string()).optional(), outputConfigs: z.array(chatOutputConfigSchema).optional(), includeThinking: z.boolean().optional(), From aaccfcf20fae5541e0840d522ff12cc244e0fc19 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Sat, 1 Aug 2026 19:42:40 -0700 Subject: [PATCH 3/5] fix(chat): preserve password visibility on regenerate --- .../ui/generated-password-input.test.tsx | 14 +++++++++++++- .../sim/components/ui/generated-password-input.tsx | 1 - 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/apps/sim/components/ui/generated-password-input.test.tsx b/apps/sim/components/ui/generated-password-input.test.tsx index fa7132d84ab..4111c6a18c8 100644 --- a/apps/sim/components/ui/generated-password-input.test.tsx +++ b/apps/sim/components/ui/generated-password-input.test.tsx @@ -134,7 +134,7 @@ describe('GeneratedPasswordInput', () => { expect(passwordInput()).not.toHaveAttribute('placeholder', '••••••••') }) - it('reveals a generated password without fetching the saved password', () => { + it('keeps a generated password hidden when the field is hidden', () => { const fetchCurrentPassword = vi.fn().mockResolvedValue('saved-secret') const onChange = vi.fn() renderInput({ fetchCurrentPassword, onChange, showGenerate: true }) @@ -142,6 +142,18 @@ describe('GeneratedPasswordInput', () => { act(() => passwordButton('Generate password').click()) expect(fetchCurrentPassword).not.toHaveBeenCalled() + expect(passwordInput()).toHaveAttribute('type', 'password') + expect(onChange).toHaveBeenCalledWith(expect.stringMatching(/^.{24}$/)) + }) + + it('keeps a generated password visible when the field is visible', async () => { + const fetchCurrentPassword = vi.fn().mockResolvedValue('saved-secret') + const onChange = vi.fn() + renderInput({ fetchCurrentPassword, onChange, showGenerate: true }) + + await act(async () => passwordButton('Show password').click()) + act(() => passwordButton('Generate password').click()) + expect(passwordInput()).toHaveAttribute('type', 'text') expect(onChange).toHaveBeenCalledWith(expect.stringMatching(/^.{24}$/)) }) diff --git a/apps/sim/components/ui/generated-password-input.tsx b/apps/sim/components/ui/generated-password-input.tsx index 8f495a63776..62eb2dab78f 100644 --- a/apps/sim/components/ui/generated-password-input.tsx +++ b/apps/sim/components/ui/generated-password-input.tsx @@ -66,7 +66,6 @@ export function GeneratedPasswordInput({ const handleGeneratePassword = () => { handleChange(generatePassword(24)) - setShowPassword(true) } const toggleShowPassword = async () => { From e50777ff18b4123226e3078ca39488622f28b752 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 1 Aug 2026 19:58:17 -0700 Subject: [PATCH 4/5] fix(chat): harden password reveal and close deployment lockout paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups from a security review of the password reveal endpoint. The permission model itself was correct — the reveal is gated on workspace admin via the canonical resolver, so derived org-admin access is honored. These address secret handling and validation around it. - Cap set-path passwords at the same 1024 chars the chat login accepts. Neither the input nor the schema bounded length, so a longer password saved fine and then failed the login POST on length before auth ran, locking every visitor out permanently. - Discard the revealed password when the field is hidden. It previously stayed in state and in the input's DOM value with Copy still armed, so the field read as hidden while still handing out the plaintext. - Evict the decrypted password from the mutation cache on unmount, and correct the TSDoc claiming it was never retained — it sat in the MutationCache for the default five minutes after the modal closed. - Validate the password inside performChatDeploy, the writer both callers must use. The copilot deploy_chat tool bypasses the route contract and could still store a whitespace-only or over-long password, or create a password-protected chat with no password at all. - Stop echoing raw decryption errors from the reveal endpoint. - Only persist a new password when the chat ends up password-protected; PATCH { authType: 'email', password } used to re-arm the secret that the auth-type branch had just cleared. Also replaces the hand-rolled copy state with useCopyToClipboard, which fixes an unawaited clipboard write that surfaced as an unhandled rejection and a "Copied" confirmation shown even when the write failed. Co-Authored-By: Claude --- .../chat/manage/[id]/password/route.test.ts | 8 +- .../api/chat/manage/[id]/password/route.ts | 8 +- apps/sim/app/api/chat/manage/[id]/route.ts | 8 +- .../ui/generated-password-input.test.tsx | 38 +++++++- .../ui/generated-password-input.tsx | 31 +++---- apps/sim/hooks/queries/chats.ts | 12 ++- .../lib/api/contracts/chats.password.test.ts | 63 +++++++++++++ apps/sim/lib/api/contracts/chats.ts | 18 +++- .../orchestration/chat-deploy.test.ts | 92 +++++++++++++++++++ .../workflows/orchestration/chat-deploy.ts | 25 +++++ 10 files changed, 272 insertions(+), 31 deletions(-) create mode 100644 apps/sim/lib/api/contracts/chats.password.test.ts create mode 100644 apps/sim/lib/workflows/orchestration/chat-deploy.test.ts diff --git a/apps/sim/app/api/chat/manage/[id]/password/route.test.ts b/apps/sim/app/api/chat/manage/[id]/password/route.test.ts index a1621612f38..e366fe5b73e 100644 --- a/apps/sim/app/api/chat/manage/[id]/password/route.test.ts +++ b/apps/sim/app/api/chat/manage/[id]/password/route.test.ts @@ -126,14 +126,16 @@ describe('Chat Password Reveal API Route', () => { expect(response.headers.get('Cache-Control')).toBe('private, no-store') }) - it('should return 500 when decryption fails', async () => { - mockDecryptSecret.mockRejectedValue(new Error('Decryption failed')) + it('should return 500 without echoing the decryption error', async () => { + mockDecryptSecret.mockRejectedValue( + new Error('Invalid encrypted value format. Expected "iv:encrypted:authTag"') + ) const response = await callGet() expect(response.status).toBe(500) const data = await response.json() - expect(data.error).toBe('Decryption failed') + expect(data.error).toBe('Failed to reveal chat password') expect(mockRecordAudit).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/app/api/chat/manage/[id]/password/route.ts b/apps/sim/app/api/chat/manage/[id]/password/route.ts index ca5c7e111af..ddc88e9594e 100644 --- a/apps/sim/app/api/chat/manage/[id]/password/route.ts +++ b/apps/sim/app/api/chat/manage/[id]/password/route.ts @@ -1,6 +1,5 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { createLogger } from '@sim/logger' -import { getErrorMessage } from '@sim/utils/errors' import type { NextRequest } from 'next/server' import { NextResponse } from 'next/server' import { getChatPasswordContract } from '@/lib/api/contracts/chats' @@ -71,7 +70,12 @@ export const GET = withRouteHandler( return NextResponse.json({ password: decrypted }, { headers: PRIVATE_NO_STORE }) } catch (error) { logger.error('Error revealing chat password:', error) - return createErrorResponse(getErrorMessage(error, 'Failed to reveal chat password'), 500) + /** + * Deliberately opaque: the only errors that reach here come from + * decryption, whose messages describe the stored ciphertext's shape. + * The logged error carries the detail for operators. + */ + return createErrorResponse('Failed to reveal chat password', 500) } } ) diff --git a/apps/sim/app/api/chat/manage/[id]/route.ts b/apps/sim/app/api/chat/manage/[id]/route.ts index ec2a8ede3dc..af90c4fedeb 100644 --- a/apps/sim/app/api/chat/manage/[id]/route.ts +++ b/apps/sim/app/api/chat/manage/[id]/route.ts @@ -241,7 +241,13 @@ export const PATCH = withRouteHandler( } } - if (encryptedPassword) { + /** + * Only store a new password when the chat ends up password-protected. + * Applying it unconditionally re-armed the secret that the branch above + * just cleared, so `PATCH { authType: 'email', password }` persisted an + * encrypted password on an email-gated chat. + */ + if (encryptedPassword && (authType ?? existingChat[0].authType) === 'password') { updateData.password = encryptedPassword } diff --git a/apps/sim/components/ui/generated-password-input.test.tsx b/apps/sim/components/ui/generated-password-input.test.tsx index 4111c6a18c8..e50ad2d4823 100644 --- a/apps/sim/components/ui/generated-password-input.test.tsx +++ b/apps/sim/components/ui/generated-password-input.test.tsx @@ -6,6 +6,10 @@ import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { GeneratedPasswordInput } from '@/components/ui/generated-password-input' +const { mockCopy } = vi.hoisted(() => ({ + mockCopy: vi.fn(async () => true), +})) + vi.mock('@sim/emcn', () => ({ Button: ({ children, @@ -34,6 +38,7 @@ vi.mock('@sim/emcn', () => ({ Trigger: ({ children }: { children?: ReactNode }) => children, Content: () => null, }, + useCopyToClipboard: () => ({ copied: false, copy: mockCopy }), })) let container: HTMLDivElement @@ -43,17 +48,19 @@ interface RenderInputOptions { fetchCurrentPassword?: () => Promise onChange?: (value: string) => void showGenerate?: boolean + value?: string } function renderInput({ fetchCurrentPassword, onChange = vi.fn(), showGenerate = false, + value = '', }: RenderInputOptions = {}) { act(() => { root.render( { expect(passwordInput()).toHaveAttribute('placeholder', '••••••••') }) - it('fetches the saved password on reveal and reuses it when toggled', async () => { + it('fetches the saved password on reveal', async () => { const fetchCurrentPassword = vi.fn().mockResolvedValue('saved-secret') renderInput({ fetchCurrentPassword }) @@ -108,12 +115,37 @@ describe('GeneratedPasswordInput', () => { expect(fetchCurrentPassword).toHaveBeenCalledOnce() expect(passwordInput()).toHaveAttribute('type', 'text') expect(passwordInput()).toHaveValue('saved-secret') + }) + + it('discards the saved password when hidden and re-fetches on the next reveal', async () => { + const fetchCurrentPassword = vi.fn().mockResolvedValue('saved-secret') + + renderInput({ fetchCurrentPassword }) + await act(async () => passwordButton('Show password').click()) act(() => passwordButton('Hide password').click()) + expect(passwordInput()).toHaveAttribute('type', 'password') + expect(passwordInput()).toHaveValue('') + expect(passwordInput()).toHaveAttribute('placeholder', '••••••••') + expect(passwordButton('Copy password')).toBeDisabled() await act(async () => passwordButton('Show password').click()) - expect(fetchCurrentPassword).toHaveBeenCalledOnce() + + expect(fetchCurrentPassword).toHaveBeenCalledTimes(2) + expect(passwordInput()).toHaveValue('saved-secret') + }) + + it('keeps an edited value when hidden', async () => { + const fetchCurrentPassword = vi.fn().mockResolvedValue('saved-secret') + const onChange = vi.fn() + + renderInput({ fetchCurrentPassword, onChange, value: 'typed-secret' }) + await act(async () => passwordButton('Show password').click()) + act(() => passwordButton('Hide password').click()) + + expect(fetchCurrentPassword).not.toHaveBeenCalled() + expect(passwordInput()).toHaveValue('typed-secret') }) it('stays masked when loading the saved password fails', async () => { diff --git a/apps/sim/components/ui/generated-password-input.tsx b/apps/sim/components/ui/generated-password-input.tsx index 62eb2dab78f..b8211479969 100644 --- a/apps/sim/components/ui/generated-password-input.tsx +++ b/apps/sim/components/ui/generated-password-input.tsx @@ -1,7 +1,7 @@ 'use client' -import { useEffect, useState } from 'react' -import { Button, ChipInput, Loader, Tooltip } from '@sim/emcn' +import { useState } from 'react' +import { Button, ChipInput, Loader, Tooltip, useCopyToClipboard } from '@sim/emcn' import { Check, Clipboard, Eye, EyeOff, RefreshCw } from 'lucide-react' import { generatePassword } from '@/lib/core/security/encryption' @@ -41,24 +41,13 @@ export function GeneratedPasswordInput({ fetchCurrentPassword, }: GeneratedPasswordInputProps) { const [showPassword, setShowPassword] = useState(false) - const [copySuccess, setCopySuccess] = useState(false) const [currentPassword, setCurrentPassword] = useState(null) const [isFetchingCurrent, setIsFetchingCurrent] = useState(false) - - useEffect(() => { - if (!copySuccess) return - const timer = setTimeout(() => setCopySuccess(false), 2000) - return () => clearTimeout(timer) - }, [copySuccess]) + const { copied, copy } = useCopyToClipboard() const displayValue = currentPassword ?? value const displayPlaceholder = fetchCurrentPassword && !displayValue ? MASKED_PASSWORD : placeholder - const copyToClipboard = () => { - navigator.clipboard.writeText(displayValue) - setCopySuccess(true) - } - const handleChange = (nextValue: string) => { setCurrentPassword(null) onChange(nextValue) @@ -71,6 +60,14 @@ export function GeneratedPasswordInput({ const toggleShowPassword = async () => { if (showPassword) { setShowPassword(false) + /** + * Discard the fetched password instead of masking it. Keeping it would + * leave the plaintext in the input's DOM value and keep Copy armed while + * the field reads as hidden. A later reveal re-fetches, which also keeps + * the audit log at one entry per disclosure. An edited value lives in + * `value` and is deliberately untouched. + */ + setCurrentPassword(null) return } @@ -124,16 +121,16 @@ export function GeneratedPasswordInput({ - {copySuccess ? 'Copied' : 'Copy'} + {copied ? 'Copied' : 'Copy'} diff --git a/apps/sim/hooks/queries/chats.ts b/apps/sim/hooks/queries/chats.ts index c1ce61a442a..ec69f65f270 100644 --- a/apps/sim/hooks/queries/chats.ts +++ b/apps/sim/hooks/queries/chats.ts @@ -385,12 +385,18 @@ interface RevealChatPasswordVariables { } /** - * Mutation hook that fetches a chat deployment's current password for - * workspace admins. Modeled as a mutation (despite the GET) so the decrypted - * password is never retained in the query cache. + * Mutation hook that fetches a chat deployment's current password for workspace + * admins. Modeled as a mutation (despite the GET) because revealing a secret is + * an audited, explicitly-triggered action, not cacheable read state. + * + * `gcTime: 0` evicts the decrypted password from the mutation cache as soon as + * the last observer unmounts, rather than letting it sit there for the default + * five minutes after the deploy modal closes. While the modal is open the caller + * holds the plaintext anyway, and it discards it when the field is hidden. */ export function useRevealChatPassword() { return useMutation({ + gcTime: 0, mutationFn: async ({ chatId }: RevealChatPasswordVariables): Promise => { const result = await requestJson(getChatPasswordContract, { params: { id: chatId } }) return result.password diff --git a/apps/sim/lib/api/contracts/chats.password.test.ts b/apps/sim/lib/api/contracts/chats.password.test.ts new file mode 100644 index 00000000000..3aad3347859 --- /dev/null +++ b/apps/sim/lib/api/contracts/chats.password.test.ts @@ -0,0 +1,63 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + chatDeploymentPasswordSchema, + createChatBodySchema, + deployedChatAuthBodySchema, + deployedChatPostBodySchema, + updateChatBodySchema, +} from '@/lib/api/contracts/chats' + +const createBody = { + workflowId: 'wf-1', + identifier: 'my-chat', + title: 'Support', + customizations: { primaryColor: 'var(--brand-hover)', welcomeMessage: 'Hi' }, +} + +describe('chat deployment password contract', () => { + it('accepts the empty string, which means "keep the stored password"', () => { + expect(chatDeploymentPasswordSchema.safeParse('').success).toBe(true) + }) + + it('rejects a whitespace-only password', () => { + const result = chatDeploymentPasswordSchema.safeParse(' ') + expect(result.success).toBe(false) + expect(result.error?.issues[0].message).toBe('Password cannot contain only whitespace') + }) + + it('preserves surrounding whitespace, which login compares byte-exact', () => { + expect(chatDeploymentPasswordSchema.parse(' hunter2 ')).toBe(' hunter2 ') + }) + + /** + * The security-relevant invariant: a password long enough to save must still + * be short enough to submit. If the set path outgrew the login path, the + * deployment would be permanently unreachable — the login POST would 400 on + * length before authentication ever ran. + */ + it('caps length at the same boundary the deployed-chat login enforces', () => { + const atLimit = 'a'.repeat(1024) + const overLimit = 'a'.repeat(1025) + + expect(chatDeploymentPasswordSchema.safeParse(atLimit).success).toBe(true) + expect(chatDeploymentPasswordSchema.safeParse(overLimit).success).toBe(false) + + expect(deployedChatAuthBodySchema.safeParse({ password: atLimit }).success).toBe(true) + expect(deployedChatPostBodySchema.safeParse({ password: atLimit }).success).toBe(true) + }) + + it('applies to both the create and update bodies', () => { + const tooLong = 'a'.repeat(1025) + + expect(createChatBodySchema.safeParse({ ...createBody, password: ' ' }).success).toBe(false) + expect(createChatBodySchema.safeParse({ ...createBody, password: tooLong }).success).toBe(false) + expect(createChatBodySchema.safeParse({ ...createBody, password: 'ok' }).success).toBe(true) + + expect(updateChatBodySchema.safeParse({ password: ' ' }).success).toBe(false) + expect(updateChatBodySchema.safeParse({ password: tooLong }).success).toBe(false) + expect(updateChatBodySchema.safeParse({ password: '' }).success).toBe(true) + }) +}) diff --git a/apps/sim/lib/api/contracts/chats.ts b/apps/sim/lib/api/contracts/chats.ts index da9db8334bd..2755536a5a2 100644 --- a/apps/sim/lib/api/contracts/chats.ts +++ b/apps/sim/lib/api/contracts/chats.ts @@ -4,8 +4,22 @@ import { defineRouteContract } from '@/lib/api/contracts/types' export const chatAuthTypeSchema = z.enum(['public', 'password', 'email', 'sso']) export type ChatAuthType = z.output +/** + * Shared cap for chat deployment passwords. The set path and the deployed-chat + * login path must agree: a password long enough to save but too long to submit + * would lock every visitor out of the deployment permanently. + */ +const MAX_CHAT_PASSWORD_CHARS = 1024 + +/** + * Password accepted when setting or changing a chat deployment's password. The + * empty string is allowed and means "keep the stored password"; a whitespace-only + * value is rejected because the login form refuses to submit one, which would + * strand the deployment behind an unenterable password. + */ export const chatDeploymentPasswordSchema = z .string() + .max(MAX_CHAT_PASSWORD_CHARS, 'Password is too long') .refine( (password) => password.length === 0 || password.trim().length > 0, 'Password cannot contain only whitespace' @@ -127,7 +141,7 @@ export const deployedChatConfigSchema = z.object({ export type DeployedChatConfig = z.output export const deployedChatAuthBodySchema = z.object({ - password: z.string().max(1024, 'Password is too long').optional(), + password: z.string().max(MAX_CHAT_PASSWORD_CHARS, 'Password is too long').optional(), email: z.string().email('Invalid email format').optional().or(z.literal('')), }) export type DeployedChatAuthBody = z.input @@ -149,7 +163,7 @@ export const deployedChatFileSchema = z.object({ export const deployedChatPostBodySchema = z.object({ input: z.string().max(MAX_CHAT_INPUT_CHARS, 'Input is too long').optional(), - password: z.string().max(1024, 'Password is too long').optional(), + password: z.string().max(MAX_CHAT_PASSWORD_CHARS, 'Password is too long').optional(), email: z.string().email('Invalid email format').optional().or(z.literal('')), conversationId: z.string().max(256, 'Conversation ID is too long').optional(), files: z diff --git a/apps/sim/lib/workflows/orchestration/chat-deploy.test.ts b/apps/sim/lib/workflows/orchestration/chat-deploy.test.ts new file mode 100644 index 00000000000..7b48bab36e1 --- /dev/null +++ b/apps/sim/lib/workflows/orchestration/chat-deploy.test.ts @@ -0,0 +1,92 @@ +/** + * @vitest-environment node + */ +import { queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockGetWorkflowDeploymentSummary, mockPerformFullDeploy, mockCheckNeedsRedeployment } = + vi.hoisted(() => ({ + mockGetWorkflowDeploymentSummary: vi.fn(), + mockPerformFullDeploy: vi.fn(), + mockCheckNeedsRedeployment: vi.fn(), + })) + +vi.mock('@/lib/workflows/orchestration/deploy', () => ({ + getWorkflowDeploymentSummary: mockGetWorkflowDeploymentSummary, + performFullDeploy: mockPerformFullDeploy, +})) + +vi.mock('@/app/api/workflows/utils', () => ({ + checkNeedsRedeployment: mockCheckNeedsRedeployment, +})) + +import { performChatDeploy } from '@/lib/workflows/orchestration/chat-deploy' + +const basePayload = { + workflowId: 'workflow-1', + userId: 'user-1', + identifier: 'my-chat', + title: 'Support', +} + +describe('performChatDeploy password guards', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockGetWorkflowDeploymentSummary.mockResolvedValue({ + activeDeployment: { id: 'deployment-1' }, + latestDeploymentAttempt: { status: 'active' }, + warnings: [], + }) + mockCheckNeedsRedeployment.mockResolvedValue(false) + }) + + afterAll(() => { + resetDbChainMock() + }) + + /** + * The copilot `deploy_chat` tool reaches this function without a route + * contract, so these guards are the only thing standing between an agent and + * a deployment nobody can log into. + */ + it('rejects a whitespace-only password before touching the deployment', async () => { + await expect( + performChatDeploy({ ...basePayload, authType: 'password', password: ' ' }) + ).resolves.toEqual({ + success: false, + error: 'Password cannot contain only whitespace', + }) + + expect(mockGetWorkflowDeploymentSummary).not.toHaveBeenCalled() + }) + + it('rejects a password longer than the chat login accepts', async () => { + await expect( + performChatDeploy({ ...basePayload, authType: 'password', password: 'a'.repeat(1025) }) + ).resolves.toEqual({ + success: false, + error: 'Password is too long', + }) + + expect(mockGetWorkflowDeploymentSummary).not.toHaveBeenCalled() + }) + + it('rejects a whitespace-only password regardless of auth type', async () => { + await expect( + performChatDeploy({ ...basePayload, authType: 'public', password: ' ' }) + ).resolves.toEqual({ + success: false, + error: 'Password cannot contain only whitespace', + }) + }) + + it('rejects password protection with no password and no stored one', async () => { + queueTableRows(schemaMock.chat, []) + + await expect(performChatDeploy({ ...basePayload, authType: 'password' })).resolves.toEqual({ + success: false, + error: 'Password is required when using password protection', + }) + }) +}) diff --git a/apps/sim/lib/workflows/orchestration/chat-deploy.ts b/apps/sim/lib/workflows/orchestration/chat-deploy.ts index 3bfe35d3de7..106cb090df0 100644 --- a/apps/sim/lib/workflows/orchestration/chat-deploy.ts +++ b/apps/sim/lib/workflows/orchestration/chat-deploy.ts @@ -4,6 +4,7 @@ import { chat } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { generateId } from '@sim/utils/id' import { and, eq, isNull } from 'drizzle-orm' +import { chatDeploymentPasswordSchema } from '@/lib/api/contracts/chats' import { encryptSecret } from '@/lib/core/security/encryption' import { getBaseUrl } from '@/lib/core/utils/urls' import { @@ -68,6 +69,20 @@ export async function performChatDeploy( includeToolCalls = false, } = params + /** + * Validate the password here rather than only at the HTTP boundary. The + * copilot `deploy_chat` tool reaches this function without going through a + * route contract, so a whitespace-only or over-long password would otherwise + * be encrypted and stored — and neither can ever be submitted through the + * chat login form, permanently locking visitors out of the deployment. + */ + if (password !== undefined) { + const validatedPassword = chatDeploymentPasswordSchema.safeParse(password) + if (!validatedPassword.success) { + return { success: false, error: validatedPassword.error.issues[0].message } + } + } + const customizations = { primaryColor: params.customizations?.primaryColor || 'var(--brand-hover)', welcomeMessage: params.customizations?.welcomeMessage || 'Hi there! How can I help you today?', @@ -125,6 +140,16 @@ export async function performChatDeploy( .where(and(eq(chat.workflowId, workflowId), isNull(chat.archivedAt))) .limit(1) + /** + * A password-protected chat must end up with a stored password. Both HTTP + * routes already reject this; without the same guard here a copilot + * `deploy_chat` call could create one with no password, which fails closed at + * login with an opaque "Authentication configuration error". + */ + if (authType === 'password' && !encryptedPassword && !existingDeployment?.password) { + return { success: false, error: 'Password is required when using password protection' } + } + let chatId: string if (existingDeployment) { chatId = existingDeployment.id From e011ed5302c4e3d3967ac8811d4857154654aeda Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 1 Aug 2026 20:06:34 -0700 Subject: [PATCH 5/5] fix(chat): correct password-change confirmation gate and stale reveal error Addresses both open Bugbot findings. - shouldConfirmPasswordChange keyed on "a chat exists" rather than "a password exists", so switching a public chat to password protection for the first time asked the admin to confirm changing a password that was never set. It now takes the existing-password signal the component already computes. - A failed reveal left "Failed to load the current password" on screen while the admin typed or generated a replacement, because the mutation only drops its error on the next attempt. Editing or regenerating now resets it. Co-Authored-By: Claude --- .../deploy-modal/components/chat/chat.tsx | 14 ++++++++++++-- .../deploy-modal/components/chat/utils.test.ts | 7 ++++++- .../deploy-modal/components/chat/utils.ts | 10 ++++++++-- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx index b2d2f575687..cf69b197258 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx @@ -245,7 +245,7 @@ export function ChatDeploy({ if ( !passwordChangeConfirmed && - shouldConfirmPasswordChange(Boolean(existingChat?.id), formData.authType, formData.password) + shouldConfirmPasswordChange(existingPassword, formData.authType, formData.password) ) { setShowPasswordChangeConfirmation(true) return @@ -700,6 +700,16 @@ function AuthSelector({ const emailsRef = useRef(emails) const invalidEmailItemsRef = useRef(invalidEmailItems) + /** + * Editing or regenerating the password clears a failed reveal. The mutation + * only drops its error on the next attempt, so it would otherwise keep + * reporting a stale failure over a field the admin has already moved on from. + */ + const handlePasswordChange = (value: string) => { + if (revealPasswordMutation.isError) revealPasswordMutation.reset() + onPasswordChange(value) + } + useEffect(() => { emailsRef.current = emails }, [emails]) @@ -799,7 +809,7 @@ function AuthSelector({ { expect(getPasswordHelperText(false)).toBe('This password will be required to access your chat') }) - it('confirms a password change only for an existing password deployment with a new value', () => { + it('confirms only when a stored password is actually being replaced', () => { expect(shouldConfirmPasswordChange(true, 'password', 'new-password')).toBe(true) expect(shouldConfirmPasswordChange(true, 'password', '')).toBe(false) expect(shouldConfirmPasswordChange(true, 'password', ' ')).toBe(false) expect(shouldConfirmPasswordChange(true, 'public', 'new-password')).toBe(false) + }) + + it('does not confirm when the deployment has no password to replace', () => { expect(shouldConfirmPasswordChange(false, 'password', 'new-password')).toBe(false) + expect(hasExistingPassword({ authType: 'public', hasPassword: false })).toBe(false) + expect(hasExistingPassword({ authType: 'password', hasPassword: true })).toBe(true) }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts index d59293a355c..ee67d26baea 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/utils.ts @@ -23,12 +23,18 @@ export function isWhitespaceOnlyPassword(password: string): boolean { return password.length > 0 && password.trim().length === 0 } +/** + * Whether submitting should confirm before overwriting the stored password. + * Only a genuine replacement warrants the prompt — keying this on "a chat + * exists" asked an admin to confirm changing a password that was never set, + * e.g. when switching a public chat to password protection for the first time. + */ export function shouldConfirmPasswordChange( - hasExistingChat: boolean, + existingPassword: boolean, authType: AuthType, password: string ): boolean { - return hasExistingChat && authType === 'password' && password.trim().length > 0 + return existingPassword && authType === 'password' && password.trim().length > 0 } export function getPasswordPlaceholder(existingPassword: boolean): string {