Skip to content

fix(desktop): stage remote files for desktop opens + hardened HTML preview (desktop PRD Phase 2) - #181

Merged
Kyzcreig merged 1 commit into
mainfrom
wt/desktop-phase2
Jul 3, 2026
Merged

Kyzcreig merged 1 commit into
mainfrom
wt/desktop-phase2

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Lane C1 of docs/desktop/2026-07-02-desktop-remote-mode-correctness-PRD.md (v1.2). Bytes-to-temp authenticated staging (BUG-B), blob+CSP+JS-disabled HTML preview (D-5/D-12), D-7 temp TTL lifecycle, no client file:// for gateway paths (BUG-C). Reviewed by Apollo.

Route remote desktop file opens through an authenticated bytes-to-temp path instead of token/download URLs or client-local file:// fallbacks. Add private temp lifecycle helpers with start/periodic TTL sweeps and harden remote HTML previews with blob URLs, CSP, and JavaScript disabled.\n\nVerified:\n- node --test apps/desktop/electron/gateway-temp-files.test.cjs\n- npm --workspace apps/desktop exec vitest run --environment jsdom src/lib/media.remote.test.ts src/lib/html-preview.test.ts src/lib/local-preview.remote.test.ts\n- npm --workspace apps/desktop exec eslint src/lib/media.ts src/lib/media.remote.test.ts src/lib/html-preview.ts src/lib/html-preview.test.ts src/lib/local-preview.ts src/lib/local-preview.remote.test.ts src/app/chat/right-rail/preview-pane.tsx electron/gateway-temp-files.cjs electron/gateway-temp-files.test.cjs electron/main.cjs\n- npm --workspace apps/desktop run typecheck\n- npm --workspace apps/desktop run test:desktop:platforms
@Kyzcreig
Kyzcreig force-pushed the wt/desktop-phase2 branch from ed0878b to 07eba79 Compare July 3, 2026 01:58
@greptile-apps

greptile-apps Bot commented Jul 3, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes two desktop remote-mode bugs (BUG-B, BUG-C): remote files are now staged to an authenticated temp directory via gateway-temp-files.cjs and opened with shell.openPath instead of handing an inaccessible file:// URL to the OS, and HTML files get a hardened blob-URL preview with JS disabled at both the Electron webview and CSP meta level.

  • gateway-temp-files.cjs — new module that decodes a gateway data-URL response, writes bytes to a private (0o600) temp file under app.getPath('temp')/hermes-desktop-gateway-files/, and runs a 30-minute TTL sweep to remove files older than 24 hours.
  • html-preview.ts — new module that injects a restrictive CSP meta tag (script-src 'none'; connect-src 'none'; …) into the HTML document and creates a blob URL, which is loaded into a javascript=no Electron webview for sandboxed rendering.
  • media.ts / local-preview.ts — mediaExternalUrl now always emits hermes-gateway-file://open?… for remote-mode paths (eliminating the previous token-in-URL approach), and the image branch of enrichPreviewTarget is fixed to set this URL.

Confidence Score: 3/5

The two new security layers (CSP hardening + JS-disabled webview) are well-designed, but a data-URL decoding bug in html-preview.ts can silently corrupt HTML files that contain + characters, which affects the primary new user-facing feature.

The +→space substitution in textFromDataUrl is the wrong transform for data URLs and will mangle any HTML that contains a literal + — operator symbols, C++ references, encoded characters — without any error or warning. The Node.js counterpart in gateway-temp-files.cjs correctly omits this transformation, making the inconsistency clear. The other items (revoked-blob-URL flash on file switch, frame-ancestors ineffective in a meta tag, blob URL in the address bar) are non-blocking UX and spec-accuracy issues.

apps/desktop/src/lib/html-preview.ts needs the + decoding fix before the HTML preview feature works correctly for files with that character.

Important Files Changed

Filename Overview
apps/desktop/src/lib/html-preview.ts New HTML preview hardening module — contains a +→space encoding bug in the non-base64 data URL decode path that can silently corrupt HTML content
apps/desktop/electron/gateway-temp-files.cjs New temp-file staging module for remote gateway bytes; file permission handling, extension validation, and sweep lifecycle are all solid
apps/desktop/electron/main.cjs Wires hermes-gateway-file://open IPC handler and sweeper lifecycle into the app startup/quit cycle; async upgrade to openExternalUrl is handled correctly
apps/desktop/src/app/chat/right-rail/preview-pane.tsx Adds remote HTML blob-preview path; effect ordering means a just-revoked blob URL is briefly attempted in the webview when switching files, and the URL bar shows a raw blob: URL
apps/desktop/src/lib/media.ts Routes gateway opens through hermes-gateway-file:// instead of exposing tokens in download URLs; adds pathFromRemoteGatewayFileUrl helper cleanly
apps/desktop/src/lib/local-preview.ts Extends enrichPreviewTarget to set the gateway-file URL for remote images too, fixing the previous gap
apps/desktop/src/lib/html-preview.test.ts Tests CSP injection into head-present and headless HTML; no test for the non-base64 + encoding path
apps/desktop/electron/gateway-temp-files.test.cjs Good integration tests covering dir creation, file decode + naming, and sweep; all new behaviour exercised
apps/desktop/src/lib/media.remote.test.ts Updated tests correctly verify gateway-file URL routing and profile propagation
apps/desktop/src/lib/local-preview.remote.test.ts New test verifies remote mode returns the gateway-file URL without touching the local file system
apps/desktop/package.json Adds gateway-temp-files.test.cjs to the platform test command; no other changes

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant R as Renderer (preview-pane)
    participant M as Electron Main (main.cjs)
    participant GW as Remote Gateway API
    participant FS as Local Temp FS
    participant OS as OS Shell

    Note over R,OS: Remote HTML preview (blob path)
    R->>R: readDesktopFileDataUrl(path) via IPC
    R->>M: hermesDesktop.api /api/fs/read-data-url
    M->>GW: "GET /api/fs/read-data-url?path=..."
    GW-->>M: data URL (base64 or URL-encoded)
    M-->>R: dataUrl string
    R->>R: textFromDataUrl(dataUrl)
    R->>R: hardenHtmlForPreview(html) — injects CSP meta
    R->>R: URL.createObjectURL(blob) → blobUrl
    R->>R: "webview src=blobUrl, javascript=no"

    Note over R,OS: Remote file open (bytes-to-temp path)
    R->>M: "hermes:openExternal hermes-gateway-file://open?path=...&profile=..."
    M->>M: openGatewayFileUrl()
    M->>GW: "GET /api/fs/read-data-url?path=..."
    GW-->>M: data URL
    M->>FS: "stageGatewayDataUrlToTemp() → /tmp/hermes-desktop-gateway-files/gateway-{ts}-{nonce}.ext"
    M->>OS: shell.openPath(localPath)

    Note over M,FS: TTL sweep (every 30 min)
    M->>FS: sweepGatewayTempFiles() — removes files older than 24 h
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant R as Renderer (preview-pane)
    participant M as Electron Main (main.cjs)
    participant GW as Remote Gateway API
    participant FS as Local Temp FS
    participant OS as OS Shell

    Note over R,OS: Remote HTML preview (blob path)
    R->>R: readDesktopFileDataUrl(path) via IPC
    R->>M: hermesDesktop.api /api/fs/read-data-url
    M->>GW: "GET /api/fs/read-data-url?path=..."
    GW-->>M: data URL (base64 or URL-encoded)
    M-->>R: dataUrl string
    R->>R: textFromDataUrl(dataUrl)
    R->>R: hardenHtmlForPreview(html) — injects CSP meta
    R->>R: URL.createObjectURL(blob) → blobUrl
    R->>R: "webview src=blobUrl, javascript=no"

    Note over R,OS: Remote file open (bytes-to-temp path)
    R->>M: "hermes:openExternal hermes-gateway-file://open?path=...&profile=..."
    M->>M: openGatewayFileUrl()
    M->>GW: "GET /api/fs/read-data-url?path=..."
    GW-->>M: data URL
    M->>FS: "stageGatewayDataUrlToTemp() → /tmp/hermes-desktop-gateway-files/gateway-{ts}-{nonce}.ext"
    M->>OS: shell.openPath(localPath)

    Note over M,FS: TTL sweep (every 30 min)
    M->>FS: sweepGatewayTempFiles() — removes files older than 24 h
Loading

Reviews (1): Last reviewed commit: "fix(desktop): stage remote files for des..." | Re-trigger Greptile

return new TextDecoder().decode(bytesFromBase64(data))
}

return decodeURIComponent(data.replace(/\+/g, '%20'))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 + treated as space in non-base64 data URLs — corrupts HTML content

In RFC 2397 data URLs, + in the data portion is a literal +, not a space. The application/x-www-form-urlencoded convention of treating + as space does not apply here. Any HTML containing + characters (e.g., C++, math expressions, URL templates, or base64 inside inline styles) would silently decode with spaces substituted. Notice that the Node.js sibling in gateway-temp-files.cjs correctly uses bare decodeURIComponent(data) without this replacement.

Suggested change
return decodeURIComponent(data.replace(/\+/g, '%20'))
return decodeURIComponent(data)

"object-src 'none'",
"base-uri 'none'",
"form-action 'none'",
"frame-ancestors 'none'"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 frame-ancestors in a CSP <meta> tag is ignored by every browser

The CSP Level 3 spec explicitly states that the frame-ancestors directive MUST be ignored when delivered via a meta element. Browsers only honor it as an HTTP response header. The directive in HTML_PREVIEW_CSP therefore provides no framing protection for the blob URL — it should either be removed (to avoid a false sense of hardening) or the team should track it as a known limitation.

URL.revokeObjectURL(objectUrl)
}
}
}, [isRemoteHtmlPreview, previewFilePath, remoteHtmlReloadKey, target.url])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Revoked blob URL briefly loaded in webview when switching remote HTML files

When the user switches to a different remote HTML file, React runs the remote HTML effect's cleanup first (which calls URL.revokeObjectURL(oldBlobUrl)), then immediately runs the webview effect's setup with webviewUrl still equal to oldBlobUrl from the now-committed render, before setRemoteHtmlBlobUrl(null) has triggered a re-render. The webview is created pointing at a URL that has already been revoked, which can fire did-fail-load and briefly flash an error state. The loadError is cleared on the next effect cycle, so the final content is still correct, but the transient error is observable in Electron's console and potentially in the UI. Removing target.url from the webview effect's dependency array would avoid the premature setup — webviewUrl alone is sufficient to gate webview creation for the remote HTML path.

@Kyzcreig
Kyzcreig merged commit ec45319 into main Jul 3, 2026
24 checks passed
@Kyzcreig
Kyzcreig deleted the wt/desktop-phase2 branch July 3, 2026 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant