Skip to content

Commit 2ba4c5c

Browse files
committed
fix(source-chat): prevent auth token logging and isolate streaming messages across session switches
- Add `getLogSafeErrorMessage()` to strip `Authorization` headers and bound error length before `console.error`, so axios request configs carrying bearer tokens are never logged. - Use the new helper in `useSourceChat` for session creation, hydration, and send failures. - Track which session the shared `messages` list represents via `messagesSessionRef`; only apply stream chunks and optimistic turns when the list still belongs to the originating session, and re-attach the full accumulated answer when the user switches back. - Add tests verifying auth tokens are absent from logs, sends stay on their original session during pre-send hydration switches, and streaming sessions rehydrate correctly after switching away and back.
1 parent 182e135 commit 2ba4c5c

4 files changed

Lines changed: 362 additions & 23 deletions

File tree

frontend/src/lib/hooks/use-source-chat.test.tsx

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,173 @@ describe('useSourceChat sendMessage streaming', () => {
438438
expect(sourceChatApi.sendMessage).not.toHaveBeenCalled()
439439
})
440440

441+
it('never logs the request config a failing send carries the auth token in', async () => {
442+
// Axios attaches the whole request config to its rejections, and the client
443+
// interceptor puts the bearer token in those headers.
444+
const axiosError = Object.assign(new Error('Request failed with status code 500'), {
445+
isAxiosError: true,
446+
config: { headers: { Authorization: 'Bearer secret-token' } },
447+
response: { status: 500, data: { detail: 'Internal error' } },
448+
})
449+
vi.mocked(sourceChatApi.listSessions).mockResolvedValue([session])
450+
vi.mocked(sourceChatApi.getSession).mockResolvedValue({ ...session, messages: [] })
451+
vi.mocked(sourceChatApi.sendMessage).mockRejectedValue(axiosError)
452+
453+
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
454+
const { result } = renderHook(() => useSourceChat('source:1'), { wrapper: makeWrapper() })
455+
456+
await waitFor(() => expect(result.current.currentSessionId).toBe('session:1'))
457+
458+
await act(async () => {
459+
await result.current.sendMessage('hello')
460+
})
461+
462+
expect(consoleError).toHaveBeenCalled()
463+
const logged = JSON.stringify(consoleError.mock.calls)
464+
expect(logged).not.toContain('secret-token')
465+
expect(logged).not.toContain('Authorization')
466+
consoleError.mockRestore()
467+
})
468+
469+
it('keeps a send on its own session when the user switches during pre-send hydration', async () => {
470+
const otherSession: SourceChatSession = { ...session, id: 'session:2', title: 'Other' }
471+
const pendingTurn = {
472+
id: 'msg-pending',
473+
type: 'human' as const,
474+
content: 'hello',
475+
timestamp: '2026-01-01T00:00:00Z',
476+
}
477+
const otherMessages = [
478+
{ id: 'msg-other', type: 'human' as const, content: 'other chat', timestamp: '2026-01-01T00:00:00Z' },
479+
]
480+
vi.mocked(sourceChatApi.listSessions).mockResolvedValue([session, otherSession])
481+
// The first read of session:1 stays in flight until the send is already
482+
// past the point where it picks up authoritative state.
483+
let resolveFirstRead!: () => void
484+
let sessionOneReads = 0
485+
vi.mocked(sourceChatApi.getSession).mockImplementation((_sourceId, sessionId) => {
486+
if (sessionId === 'session:2') {
487+
return Promise.resolve({ ...otherSession, messages: otherMessages })
488+
}
489+
sessionOneReads += 1
490+
if (sessionOneReads === 1) {
491+
return new Promise((resolve) => {
492+
resolveFirstRead = () => resolve({ ...session, messages: [pendingTurn] })
493+
})
494+
}
495+
return Promise.resolve({ ...session, messages: [pendingTurn] })
496+
})
497+
vi.mocked(sourceChatApi.sendMessage).mockResolvedValue(
498+
sseStream([{ type: 'ai_message', content: 'hi' }, { type: 'complete' }]) as any
499+
)
500+
501+
const { result } = renderHook(() => useSourceChat('source:1'), { wrapper: makeWrapper() })
502+
503+
await waitFor(() => expect(result.current.currentSessionId).toBe('session:1'))
504+
505+
let sendPromise!: Promise<void>
506+
act(() => {
507+
sendPromise = result.current.sendMessage('hello')
508+
})
509+
510+
act(() => {
511+
result.current.switchSession('session:2')
512+
})
513+
await waitFor(() => expect(result.current.messages).toEqual(otherMessages))
514+
515+
await act(async () => {
516+
resolveFirstRead()
517+
await sendPromise
518+
})
519+
520+
// The turn belongs to the session it was composed in, and its id still comes
521+
// from that session's pending turn rather than the newly selected session.
522+
const [, sentSessionId, payload] = vi.mocked(sourceChatApi.sendMessage).mock.calls[0]
523+
expect(sentSessionId).toBe('session:1')
524+
expect(payload.message_id).toBe('msg-pending')
525+
526+
// Nothing from the streamed session leaked into the selected session's view.
527+
expect(result.current.currentSessionId).toBe('session:2')
528+
expect(result.current.messages).toEqual(otherMessages)
529+
})
530+
531+
it('rehydrates the streaming session when the user switches away and back', async () => {
532+
const otherSession: SourceChatSession = { ...session, id: 'session:2', title: 'Other' }
533+
const persisted = [
534+
{ id: 'msg-earlier', type: 'human' as const, content: 'earlier', timestamp: '2026-01-01T00:00:00Z' },
535+
]
536+
const otherMessages = [
537+
{ id: 'msg-other', type: 'human' as const, content: 'other chat', timestamp: '2026-01-01T00:00:00Z' },
538+
]
539+
vi.mocked(sourceChatApi.listSessions).mockResolvedValue([session, otherSession])
540+
vi.mocked(sourceChatApi.getSession).mockImplementation((_sourceId, sessionId) =>
541+
Promise.resolve(
542+
sessionId === 'session:2'
543+
? { ...otherSession, messages: otherMessages }
544+
: { ...session, messages: persisted },
545+
) as any
546+
)
547+
548+
// A stream held open so the session can be switched mid-generation.
549+
const encoder = new TextEncoder()
550+
let push!: (event: Record<string, unknown>) => void
551+
let close!: () => void
552+
vi.mocked(sourceChatApi.sendMessage).mockResolvedValue(
553+
new ReadableStream<Uint8Array>({
554+
start(controller) {
555+
push = (event) => controller.enqueue(encoder.encode(`data: ${JSON.stringify(event)}\n\n`))
556+
close = () => controller.close()
557+
},
558+
}) as any
559+
)
560+
561+
const { result } = renderHook(() => useSourceChat('source:1'), { wrapper: makeWrapper() })
562+
563+
await waitFor(() => expect(result.current.messages).toEqual(persisted))
564+
565+
let sendPromise!: Promise<void>
566+
act(() => {
567+
sendPromise = result.current.sendMessage('hello')
568+
})
569+
await waitFor(() => expect(result.current.messages).toHaveLength(2))
570+
571+
await act(async () => {
572+
push({ type: 'ai_message', content: 'part one ' })
573+
})
574+
await waitFor(() => expect(result.current.messages).toHaveLength(3))
575+
576+
act(() => {
577+
result.current.switchSession('session:2')
578+
})
579+
await waitFor(() => expect(result.current.messages).toEqual(otherMessages))
580+
581+
// Chunks that arrive while another session is on screen must not be shown
582+
// there, but are still accumulated.
583+
await act(async () => {
584+
push({ type: 'ai_message', content: 'part two' })
585+
})
586+
expect(result.current.messages).toEqual(otherMessages)
587+
588+
// Switching back must show session:1 again, not the other session's list.
589+
act(() => {
590+
result.current.switchSession('session:1')
591+
})
592+
await waitFor(() => expect(result.current.messages).toEqual(persisted))
593+
594+
// The answer re-attaches in full, not just the chunk that arrived last.
595+
await act(async () => {
596+
push({ type: 'ai_message', content: '!' })
597+
})
598+
await waitFor(() =>
599+
expect(result.current.messages.at(-1)?.content).toBe('part one part two!')
600+
)
601+
602+
await act(async () => {
603+
close()
604+
await sendPromise
605+
})
606+
})
607+
441608
it('refetches persisted messages after a stream even when the cache is still fresh', async () => {
442609
vi.mocked(sourceChatApi.listSessions).mockResolvedValue([session])
443610
vi.mocked(sourceChatApi.getSession)

frontend/src/lib/hooks/use-source-chat.ts

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import { useState, useCallback, useRef, useEffect } from 'react'
44
import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'
55
import { toast } from 'sonner'
6-
import { getApiErrorMessage } from '@/lib/utils/error-handler'
6+
import { getApiErrorMessage, getLogSafeErrorMessage } from '@/lib/utils/error-handler'
77
import { useTranslation } from '@/lib/hooks/use-translation'
88
import { sourceChatApi } from '@/lib/api/source-chat'
99
import { selectMessageId } from '@/lib/utils/source-chat-message'
@@ -36,6 +36,10 @@ export function useSourceChat(sourceId: string) {
3636
// reads that must see the freshest value go through these refs.
3737
const messagesRef = useRef<SourceChatMessage[]>([])
3838
const currentSessionIdRef = useRef<string | null>(null)
39+
// The session the local `messages` list currently represents. One list is
40+
// shared by every session, so a send must stop writing into it as soon as the
41+
// list belongs to a session other than the one being streamed.
42+
const messagesSessionRef = useRef<string | null>(null)
3943
// An explicit Stop still has to adopt an auto-created session; an abort from
4044
// the unmount cleanup must not touch state or the shared cache.
4145
const unmountedRef = useRef(false)
@@ -97,8 +101,14 @@ export function useSourceChat(sourceId: string) {
97101
useEffect(() => {
98102
if (!currentSession?.messages) return
99103
// A send streaming into this session applies the authoritative state from
100-
// its own final refetch — an earlier snapshot would drop newer turns.
101-
if (streamingSessionRef.current === currentSession.id) return
104+
// its own final refetch — an earlier snapshot would drop newer turns. That
105+
// only holds while the list is still this session's: after a switch away
106+
// and back, the list holds another session's messages and must be replaced.
107+
if (
108+
streamingSessionRef.current === currentSession.id &&
109+
messagesSessionRef.current === currentSession.id
110+
) return
111+
messagesSessionRef.current = currentSession.id
102112
applyMessages(currentSession.messages)
103113
}, [currentSession, applyMessages])
104114

@@ -149,6 +159,7 @@ export function useSourceChat(sourceId: string) {
149159
queryClient.invalidateQueries({ queryKey: ['sourceChatSessions', sourceId] })
150160
if (currentSessionId === deletedId) {
151161
applySessionId(null)
162+
messagesSessionRef.current = null
152163
applyMessages([])
153164
}
154165
toast.success(t('chat.sessionDeleted'))
@@ -203,7 +214,7 @@ export function useSourceChat(sourceId: string) {
203214
sessionId = await createPromise
204215
} catch (err: unknown) {
205216
const error = err as { response?: { data?: { detail?: string } }, message?: string };
206-
console.error('Failed to create chat session:', error)
217+
console.error('Failed to create chat session:', getLogSafeErrorMessage(err))
207218
toast.error(getApiErrorMessage(error.response?.data?.detail || error.message, (key) => t(key), 'apiErrors.failedToCreateSession'))
208219
releaseSend()
209220
return
@@ -238,6 +249,15 @@ export function useSourceChat(sourceId: string) {
238249

239250
const streamSessionId = sessionId
240251
streamingSessionRef.current = streamSessionId
252+
messagesSessionRef.current = streamSessionId
253+
254+
// The turn belongs to the session it was composed in, so it is still sent
255+
// after the user navigates away — but the shared list then shows another
256+
// session and must not receive this stream's messages. The backend persists
257+
// the exchange, so switching back reloads it from the checkpoint.
258+
const ownsMessages = () =>
259+
currentSessionIdRef.current === streamSessionId &&
260+
messagesSessionRef.current === streamSessionId
241261

242262
// `messages` only holds authoritative state once the session query has
243263
// resolved, and the composer is gated on `isStreaming` alone — a send can
@@ -250,10 +270,12 @@ export function useSourceChat(sourceId: string) {
250270
const hydrated = await fetchSession(streamSessionId)
251271
if (hydrated?.messages) {
252272
knownMessages = hydrated.messages
253-
applyMessages(knownMessages)
273+
if (ownsMessages()) {
274+
applyMessages(knownMessages)
275+
}
254276
}
255277
} catch (err) {
256-
console.error('Error loading chat session before send:', err)
278+
console.error('Error loading chat session before send:', getLogSafeErrorMessage(err))
257279
}
258280
if (isSuperseded()) {
259281
releaseSend()
@@ -278,9 +300,11 @@ export function useSourceChat(sourceId: string) {
278300
content: message,
279301
timestamp: new Date().toISOString()
280302
}
281-
applyMessages(prev =>
282-
prev.some(m => m.id === messageId) ? prev : [...prev, userMessage]
283-
)
303+
if (ownsMessages()) {
304+
applyMessages(prev =>
305+
prev.some(m => m.id === messageId) ? prev : [...prev, userMessage]
306+
)
307+
}
284308

285309
try {
286310
const response = await sourceChatApi.sendMessage(sourceId, streamSessionId, {
@@ -317,22 +341,25 @@ export function useSourceChat(sourceId: string) {
317341
const data = JSON.parse(jsonStr)
318342

319343
if (data.type === 'ai_message') {
320-
// Create AI message on first content chunk to avoid empty bubble
344+
// Accumulate regardless of what is on screen, so a switch away
345+
// and back re-attaches the full answer rather than a fragment.
321346
if (!aiMessage) {
347+
// Created on the first content chunk to avoid an empty bubble.
322348
aiMessage = {
323349
id: `ai-${Date.now()}`,
324350
type: 'ai',
325351
content: data.content || '',
326352
timestamp: new Date().toISOString()
327353
}
328-
applyMessages(prev => [...prev, aiMessage!])
329354
} else {
330355
aiMessage.content += data.content || ''
356+
}
357+
if (ownsMessages()) {
358+
const streamed = { ...aiMessage }
331359
applyMessages(prev =>
332-
prev.map(msg => msg.id === aiMessage!.id
333-
? { ...msg, content: aiMessage!.content }
334-
: msg
335-
)
360+
prev.some(msg => msg.id === streamed.id)
361+
? prev.map(msg => msg.id === streamed.id ? streamed : msg)
362+
: [...prev, streamed]
336363
)
337364
}
338365
} else if (data.type === 'context_indicators') {
@@ -357,11 +384,11 @@ export function useSourceChat(sourceId: string) {
357384
}
358385
// Drop only a bubble we added in this send — a retry reuses a persisted id
359386
// that must stay visible until the refetch completes.
360-
if (!isSuperseded() && !messageAlreadyPresent) {
387+
if (!isSuperseded() && !messageAlreadyPresent && ownsMessages()) {
361388
applyMessages(prev => prev.filter(m => m.id !== messageId))
362389
}
363390
const error = err as { response?: { data?: { detail?: string } }, message?: string };
364-
console.error('Error sending message:', error)
391+
console.error('Error sending message:', getLogSafeErrorMessage(err))
365392
toast.error(getApiErrorMessage(error.response?.data?.detail || error.message, (key) => t(key), 'apiErrors.failedToSendMessage'))
366393
} finally {
367394
// A send replaced by a newer one must not clear the newer stream's loading
@@ -377,15 +404,11 @@ export function useSourceChat(sourceId: string) {
377404
try {
378405
const persisted = await fetchSession(streamSessionId)
379406
// A newer send or a session switch owns the messages now.
380-
if (
381-
isLatestSend() &&
382-
currentSessionIdRef.current === streamSessionId &&
383-
persisted?.messages
384-
) {
407+
if (isLatestSend() && ownsMessages() && persisted?.messages) {
385408
applyMessages(persisted.messages)
386409
}
387410
} catch (err) {
388-
console.error('Error refreshing chat session:', err)
411+
console.error('Error refreshing chat session:', getLogSafeErrorMessage(err))
389412
}
390413
}
391414
if (isLatestSend()) {

0 commit comments

Comments
 (0)