Fix SSH dashboard CSP and remote completion rendering#723
Open
jugol wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/renderer/index.html, which already permits these loopback sources.message.completeas the canonical final assistant text in the dashboard event adapter.Why
SSH dashboard chat uses a local tunnel from the desktop app to the remote Hermes dashboard, for example:
The renderer then opens the dashboard sidecar WebSocket through that local tunnel:
In production, the main process overrides the page CSP through
webRequest.onHeadersReceived. That production CSP omitted loopback HTTP/WebSocket sources fromconnect-src, even though the renderer HTML CSP already includes them. Chromium can therefore block the intended SSH dashboard WebSocket before it reaches the tunnel, producing:The CSP change is limited to loopback only:
That matches the SSH tunnel design while keeping the policy scoped to local desktop-owned endpoints.
Dashboard completion rendering
While validating the SSH dashboard path against a remote Hermes dashboard, I also hit a separate dashboard rendering issue: streamed
message.deltatext can differ from the finalmessage.completetext. That is especially visible with non-English output when an interim partial fragment is malformed or incomplete.Before this change, the adapter replaced the pending assistant bubble only when the final text started with the streamed text. If the streamed text did not match, it concatenated the partial stream with the final answer, so the UI could show a garbled prefix followed by the correct final answer.
message.completeis the authoritative final assistant response, so this PR now replaces the pending assistant bubble with that final text instead of appending it.For remote/SSH dashboard sessions, the renderer now suppresses assistant
message.deltabubbles and waits formessage.complete. Local dashboard sessions keep the existing live assistant streaming behavior. This avoids showing transient malformed remote stream fragments while keeping the persisted/final transcript canonical.Validation
npm test -- --run tests/dashboard-event-adapter.test.ts src/renderer/src/screens/Chat/hooks/useDashboardChatTransport.test.tsxnpm run buildnpx vitest run tests/dashboard-csp.test.ts tests/askpass-security.test.tsnpm run typecheck:node