From c7a88ba396e67cdf90bfc8682de0e83046758841 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 30 Aug 2026 05:46:29 +0000 Subject: [PATCH 1/3] cli: let update install the desktop app it was told to skip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `diskpush update` re-ran the installer with --cli-only whenever the manifest said desktop:false, so an install made from a tty or over SSH — where has_desktop_session() finds no DISPLAY — could never gain the desktop app, however many times it was updated, and printed nothing to say so. The desktop app looked stale because it had never been installed. A CLI-only install is now left to the installer's own detection, so a machine that has a desktop gets the desktop app on its next update. A desktop install is pinned the other way, to --desktop: updating one from cron or SSH has no DISPLAY to detect, and auto-detect would quietly strip the app out from under a working desktop and orphan its files. --cli-only and --desktop on the update command override both, and a Surfaces: line now says which it chose. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CBc6KV8dL7oLS5okq15B4W --- apps/cli/src/commands/self.ts | 36 ++++++++++++++++++++- apps/cli/src/help.ts | 2 ++ apps/cli/src/installer-flags.test.ts | 48 ++++++++++++++++++++++++++++ apps/cli/src/self-update.ts | 4 +-- 4 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 apps/cli/src/installer-flags.test.ts diff --git a/apps/cli/src/commands/self.ts b/apps/cli/src/commands/self.ts index 518d1c0..1edafe8 100644 --- a/apps/cli/src/commands/self.ts +++ b/apps/cli/src/commands/self.ts @@ -51,6 +51,38 @@ export function readManifest(env: NodeJS.ProcessEnv = process.env): InstallManif } } +/** + * Which surface flags to hand the installer when re-running it to upgrade. + * + * The installer with no flag auto-detects a desktop session, so a CLI-only + * install is deliberately left to that detection: a machine that has grown a + * desktop since it was first installed gets the desktop app on its next + * update, which is what "update" is expected to mean. Pinning it to + * `--cli-only` — as this used to — meant an install made from a tty or over + * SSH could never gain the desktop app, no matter how many times it updated, + * and said nothing about why. + * + * A desktop install is pinned the other way, to `--desktop`. Updating one from + * a tty (a cron job, an SSH session) has no DISPLAY to detect, so auto-detect + * would quietly downgrade it to CLI-only and strip the app out from under a + * running desktop. + * + * An explicit flag on the update command always wins, so either direction + * stays reachable by hand. + */ +export function installerFlags(manifest: InstallManifest, parsed?: ParsedArgv): string[] { + if (parsed && hasFlag(parsed, '--cli-only')) return ['--cli-only'] + if (parsed && hasFlag(parsed, '--desktop')) return ['--desktop'] + return manifest.desktop ? ['--desktop'] : [] +} + +/** What `Surfaces:` prints, so an update never silently skips the desktop app. */ +function describeSurfaces(flags: string[]): string { + if (flags.includes('--cli-only')) return 'CLI only (--cli-only)' + if (flags.includes('--desktop')) return 'CLI and desktop app' + return 'CLI, plus the desktop app if this machine has a desktop session' +} + /** * Re-runs the installer that put this copy here. It is idempotent, so * updating is installing again, and the manifest remembers where it came @@ -73,8 +105,10 @@ export async function runUpdate(parsed: ParsedArgv, output: Output): Promise { + it('leaves a CLI-only install to the installer to auto-detect', () => { + // The bug this exists to prevent: pinning --cli-only meant an install made + // from a tty could never gain the desktop app, however often it updated. + expect(installerFlags(manifest(false))).toEqual([]) + }) + + it('pins a desktop install to --desktop', () => { + // Updating from a cron job or an SSH session has no DISPLAY to detect, so + // auto-detect would strip the app out from under a working desktop. + expect(installerFlags(manifest(true))).toEqual(['--desktop']) + }) + + it('honours an explicit --cli-only on a desktop install', () => { + const parsed = parseArgv(['update', '--cli-only']) + expect(installerFlags(manifest(true), parsed)).toEqual(['--cli-only']) + }) + + it('honours an explicit --desktop on a CLI-only install', () => { + const parsed = parseArgv(['update', '--desktop']) + expect(installerFlags(manifest(false), parsed)).toEqual(['--desktop']) + }) + + it('prefers --cli-only when both are somehow passed', () => { + const parsed = parseArgv(['update', '--cli-only', '--desktop']) + expect(installerFlags(manifest(false), parsed)).toEqual(['--cli-only']) + }) + + it('adds no flag for a plain update of a CLI-only install', () => { + const parsed = parseArgv(['update']) + expect(installerFlags(manifest(false), parsed)).toEqual([]) + }) +}) diff --git a/apps/cli/src/self-update.ts b/apps/cli/src/self-update.ts index 5a6cb48..91eb5f4 100644 --- a/apps/cli/src/self-update.ts +++ b/apps/cli/src/self-update.ts @@ -3,7 +3,7 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs' import { dirname, join } from 'node:path' import { VERSION } from './help.js' import type { Output } from './output.js' -import { readManifest, stateDirectory } from './commands/self.js' +import { installerFlags, readManifest, stateDirectory } from './commands/self.js' /** * Startup update check. @@ -105,7 +105,7 @@ export async function autoUpdate(command: string | null, output: Output, force = return 'failed' } - const run = spawnSync('sh', ['-s', '--', ...(manifest.desktop ? [] : ['--cli-only'])], { + const run = spawnSync('sh', ['-s', '--', ...installerFlags(manifest)], { input: script.stdout, stdio: ['pipe', 'ignore', 'inherit'], }) From c825f4cba41b927fe4ca930dd5369cb6d11364f5 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 30 Aug 2026 05:46:38 +0000 Subject: [PATCH 2/3] desktop: serve the renderer over a scheme, because file:// 404s every asset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The window loaded out/index.html with loadFile, so the page origin was file://. Next emits root-absolute asset URLs (/_next/static/...), which under file:// resolve against the filesystem root — file:///_next/... — so the stylesheet and all five chunks 404ed. What shipped was the prerendered HTML with zero CSS rules applied and no React: unstyled Times New Roman, native form controls, and a permanent "Loading…". It reads as a broken UI rather than a broken path, which is why it survived a UI pass — the export was verified over http://, where those same absolute paths resolve fine. Next cannot emit relative ones: next/font rejects an assetPrefix without a leading slash, and the CSP here sends base-uri 'none', so a tag is out too. Instead the bundle gets an origin of its own via a standard scheme, which also makes the existing default-src 'self' mean the bundle rather than the whole disk. Files are read with fs, not net.fetch('file://'), so it keeps working inside the packaged asar. Verified in headless Chromium against the real export: 6 failed requests and 0 CSS rules before, 0 failed requests and 124 after, with React hydrating and the dark theme and Geist both applied. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CBc6KV8dL7oLS5okq15B4W --- .../desktop/electron/main/bundle-path.test.ts | 57 ++++++++++++++++++ apps/desktop/electron/main/bundle-path.ts | 59 +++++++++++++++++++ apps/desktop/electron/main/index.ts | 46 +++++++++++++-- 3 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 apps/desktop/electron/main/bundle-path.test.ts create mode 100644 apps/desktop/electron/main/bundle-path.ts diff --git a/apps/desktop/electron/main/bundle-path.test.ts b/apps/desktop/electron/main/bundle-path.test.ts new file mode 100644 index 0000000..74ced17 --- /dev/null +++ b/apps/desktop/electron/main/bundle-path.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from 'vitest' +import { join } from 'node:path' +import { contentTypeFor, resolveBundlePath } from './bundle-path.js' + +const ROOT = '/opt/diskpush/out' + +describe('resolveBundlePath', () => { + it('serves index.html for the origin root', () => { + expect(resolveBundlePath(ROOT, '/')).toBe(join(ROOT, 'index.html')) + }) + + it('resolves the root-absolute asset paths Next emits', () => { + // The whole reason the bundle scheme exists: under file:// these resolved + // against the filesystem root and every one of them 404'd. + expect(resolveBundlePath(ROOT, '/_next/static/css/app.css')).toBe(join(ROOT, '_next/static/css/app.css')) + expect(resolveBundlePath(ROOT, '/_next/static/chunks/main-app.js')).toBe(join(ROOT, '_next/static/chunks/main-app.js')) + }) + + it('decodes percent-escapes in a filename', () => { + expect(resolveBundlePath(ROOT, '/_next/a%20b.css')).toBe(join(ROOT, '_next/a b.css')) + }) + + it('refuses to escape the bundle', () => { + for (const attempt of ['/../../../etc/passwd', '/_next/../../etc/passwd', '/%2e%2e/%2e%2e/etc/passwd']) { + expect(resolveBundlePath(ROOT, attempt)).toBeNull() + } + }) + + it('refuses a malformed escape and an embedded NUL', () => { + expect(resolveBundlePath(ROOT, '/%')).toBeNull() + expect(resolveBundlePath(ROOT, '/index.html%00.png')).toBeNull() + }) + + it('does not treat a sibling directory as inside the bundle', () => { + // String-prefix checks without a separator let /opt/diskpush/outside through. + expect(resolveBundlePath('/opt/diskpush/out', '/../outside/secret')).toBeNull() + }) +}) + +describe('contentTypeFor', () => { + it('types the assets a Next export is made of', () => { + // Served as octet-stream, a stylesheet is a stylesheet the renderer ignores. + expect(contentTypeFor('/out/_next/static/css/app.css')).toBe('text/css; charset=utf-8') + expect(contentTypeFor('/out/_next/static/chunks/main.js')).toBe('text/javascript; charset=utf-8') + expect(contentTypeFor('/out/index.html')).toBe('text/html; charset=utf-8') + expect(contentTypeFor('/out/_next/static/media/geist.woff2')).toBe('font/woff2') + }) + + it('ignores case in the extension', () => { + expect(contentTypeFor('/out/LOGO.PNG')).toBe('image/png') + }) + + it('falls back for anything unrecognised', () => { + expect(contentTypeFor('/out/data.bin')).toBe('application/octet-stream') + expect(contentTypeFor('/out/LICENSE')).toBe('application/octet-stream') + }) +}) diff --git a/apps/desktop/electron/main/bundle-path.ts b/apps/desktop/electron/main/bundle-path.ts new file mode 100644 index 0000000..6660380 --- /dev/null +++ b/apps/desktop/electron/main/bundle-path.ts @@ -0,0 +1,59 @@ +import { extname, join, normalize, sep } from 'node:path' + +/** + * Maps a bundle-scheme pathname to a file inside the exported renderer, or + * null when it escapes. + * + * Kept apart from index.ts so the tests can import it: index.ts registers a + * privileged scheme at module scope, so importing it outside Electron throws. + */ +export function resolveBundlePath(root: string, pathname: string): string | null { + let decoded: string + try { + decoded = decodeURIComponent(pathname) + } catch { + // A malformed escape is not a path we should guess at. + return null + } + // A NUL byte truncates the path in some syscalls; refuse rather than normalise. + if (decoded.includes('\0')) return null + + const target = normalize(join(root, decoded === '/' ? 'index.html' : decoded)) + // A crafted ../ must not turn the bundle scheme into a reader for the disk. + if (target !== root && !target.startsWith(root + sep)) return null + return target +} + +/** + * Content types for what a Next export actually contains. + * + * The bundle is read with fs rather than `net.fetch('file://…')` because the + * packaged app is an asar archive: fs is asar-aware, so it reads straight out + * of it, and nothing here has to know whether the app is packaged. That means + * the response carries no type of its own, and a stylesheet served as + * octet-stream is a stylesheet the renderer ignores. + */ +const CONTENT_TYPES: Record = { + '.html': 'text/html; charset=utf-8', + '.js': 'text/javascript; charset=utf-8', + '.mjs': 'text/javascript; charset=utf-8', + '.css': 'text/css; charset=utf-8', + '.json': 'application/json; charset=utf-8', + '.txt': 'text/plain; charset=utf-8', + '.svg': 'image/svg+xml', + '.png': 'image/png', + '.jpg': 'image/jpeg', + '.jpeg': 'image/jpeg', + '.gif': 'image/gif', + '.webp': 'image/webp', + '.ico': 'image/x-icon', + '.woff': 'font/woff', + '.woff2': 'font/woff2', + '.ttf': 'font/ttf', + '.otf': 'font/otf', + '.map': 'application/json; charset=utf-8', +} + +export function contentTypeFor(path: string): string { + return CONTENT_TYPES[extname(path).toLowerCase()] ?? 'application/octet-stream' +} diff --git a/apps/desktop/electron/main/index.ts b/apps/desktop/electron/main/index.ts index 1890343..13e5b64 100644 --- a/apps/desktop/electron/main/index.ts +++ b/apps/desktop/electron/main/index.ts @@ -1,6 +1,8 @@ -import { join } from 'node:path' +import { readFile } from 'node:fs/promises' +import { join, normalize } from 'node:path' import { fileURLToPath } from 'node:url' -import { app, BrowserWindow, shell } from 'electron' +import { app, BrowserWindow, protocol, shell } from 'electron' +import { contentTypeFor, resolveBundlePath } from './bundle-path.js' import { registerIpc } from './ipc.js' import { checkForUpdates } from './services/updater.js' import { closeAllSessions } from './services/sessions.js' @@ -19,6 +21,41 @@ const here = join(fileURLToPath(import.meta.url), '..') const isDev = !app.isPackaged && process.env.DISKPUSH_DEV === '1' const DEV_URL = 'http://localhost:3210' +/** + * The exported renderer is served over a real scheme rather than loaded from + * file://. + * + * Next emits root-absolute asset URLs (`/_next/static/...`) and refuses to emit + * relative ones — `next/font` rejects an assetPrefix without a leading slash. + * Under file:// those resolve against the filesystem root, so every stylesheet + * and chunk 404s: the window shows unstyled prerendered HTML that never + * hydrates. A standard scheme gives the bundle an origin, so the same absolute + * paths resolve inside it, and CSP `'self'` means the bundle instead of the + * whole disk. + */ +const APP_SCHEME = 'diskpush-app' +const APP_ORIGIN = `${APP_SCHEME}://bundle` + +// Must run before the app is ready, hence module scope rather than whenReady. +protocol.registerSchemesAsPrivileged([ + { scheme: APP_SCHEME, privileges: { standard: true, secure: true, supportFetchAPI: true } }, +]) + +/** Serves the exported renderer, and nothing outside it. */ +function serveBundle(): void { + const root = normalize(join(here, '..', '..', 'out')) + protocol.handle(APP_SCHEME, async (request) => { + const target = resolveBundlePath(root, new URL(request.url).pathname) + if (!target) return new Response('Forbidden', { status: 403 }) + try { + const body = await readFile(target) + return new Response(body, { headers: { 'content-type': contentTypeFor(target) } }) + } catch { + return new Response('Not found', { status: 404 }) + } + }) +} + function createWindow(): BrowserWindow { const window = new BrowserWindow({ width: 1360, @@ -43,7 +80,7 @@ function createWindow(): BrowserWindow { // Navigation is pinned to the app itself. A renderer compromise should not // be able to point the window at somewhere else. window.webContents.on('will-navigate', (event, url) => { - const allowed = isDev ? url.startsWith(DEV_URL) : url.startsWith('file://') + const allowed = isDev ? url.startsWith(DEV_URL) : url.startsWith(APP_ORIGIN) if (!allowed) event.preventDefault() }) @@ -78,13 +115,14 @@ function createWindow(): BrowserWindow { }) if (isDev) void window.loadURL(DEV_URL) - else void window.loadFile(join(here, '..', '..', 'out', 'index.html')) + else void window.loadURL(`${APP_ORIGIN}/index.html`) return window } app.whenReady().then(() => { registerIpc() + if (!isDev) serveBundle() createWindow() // Not awaited: a slow or unreachable GitHub must not delay the window. From 766ac88426e8ce7d432e663f48eb27ca41f40c68 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 30 Aug 2026 05:50:27 +0000 Subject: [PATCH 3/3] desktop: make the smoke test see the renderer, not just the launch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The launch check dies at the display, so it never loads a page — which is why it watched v0.2.0 ship a window where the stylesheet and all five chunks 404ed and reported ok. Two checks that need no display now run first: every root-absolute asset in the export must resolve to a real file through the same function that serves it, and the main process must still register the bundle scheme and must not load the renderer with loadFile. The second is the one with teeth. Assets resolve whatever the window does with them; what broke the app was the page origin, so the mechanism is pinned rather than only the paths. Verified by reintroducing loadFile into the built main and watching the check fail. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CBc6KV8dL7oLS5okq15B4W --- scripts/smoke-desktop.mjs | 80 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/scripts/smoke-desktop.mjs b/scripts/smoke-desktop.mjs index 53da5ce..4e61ec0 100755 --- a/scripts/smoke-desktop.mjs +++ b/scripts/smoke-desktop.mjs @@ -12,12 +12,90 @@ * *format* compatibility at runtime, and the unit tests never start Electron. */ import { spawnSync } from 'node:child_process' -import { existsSync } from 'node:fs' +import { existsSync, readFileSync } from 'node:fs' import { dirname, join, resolve } from 'node:path' const desktop = resolve(import.meta.dirname, '..', 'apps', 'desktop') const electron = join(desktop, 'node_modules', 'electron', 'dist', 'electron') +/** + * Every asset the exported page asks for must resolve to a real file through + * the same function that serves it. + * + * The launch check below cannot see this: it dies at the display, so the + * renderer never runs. That is exactly how v0.2.0 shipped a window in which + * the stylesheet and all five chunks 404ed — Next emits root-absolute URLs + * (`/_next/static/...`), the app loaded the page over file://, and those + * resolved against the filesystem root instead of the bundle. The result was + * unstyled prerendered HTML that never hydrated, and nothing failed. + */ +async function checkRendererAssets() { + const out = join(desktop, 'out') + const index = join(out, 'index.html') + if (!existsSync(index)) { + console.error('out/index.html is missing; run pnpm --filter @diskpush/desktop build first.') + process.exit(1) + } + + const helper = join(desktop, 'dist-electron', 'main', 'bundle-path.js') + if (!existsSync(helper)) { + console.error('dist-electron is missing; run pnpm --filter @diskpush/desktop build first.') + process.exit(1) + } + const { resolveBundlePath } = await import(helper) + const html = readFileSync(index, 'utf8') + // Only root-absolute references: those are the ones file:// resolved wrongly. + const referenced = [...html.matchAll(/(?:href|src)="(\/[^"]*)"/g)].map((m) => m[1]) + + if (referenced.length === 0) { + console.error('FAIL: no assets referenced by out/index.html — the export looks broken.') + process.exit(1) + } + + const missing = referenced.filter((url) => { + const target = resolveBundlePath(out, url.split(/[?#]/)[0]) + return !target || !existsSync(target) + }) + + if (missing.length > 0) { + console.error(`FAIL: ${missing.length} of ${referenced.length} renderer assets do not resolve inside the bundle.\n`) + missing.slice(0, 10).forEach((url) => console.error(` ${url}`)) + console.error('\nThe window would render unstyled and never hydrate.') + process.exit(1) + } + + console.log(`ok: all ${referenced.length} renderer assets resolve inside the bundle.`) +} + +/** + * The renderer must not be loaded over file:// again. + * + * Checking that the assets resolve is not enough on its own: they resolve + * whatever the window does with them. What broke the app was the *origin* — + * loadFile gives the page a file:// origin, under which the root-absolute URLs + * above point at the filesystem root rather than the bundle. So pin the + * mechanism, not just the paths. + */ +function checkRendererDelivery() { + const main = readFileSync(join(desktop, 'dist-electron', 'main', 'index.js'), 'utf8') + + if (!main.includes('registerSchemesAsPrivileged')) { + console.error('FAIL: the main process no longer registers a scheme for the bundle.') + process.exit(1) + } + if (/\.loadFile\s*\(/.test(main)) { + console.error('FAIL: the main process loads the renderer with loadFile, which gives it a file:// origin.') + console.error("Next's root-absolute asset URLs resolve against the filesystem root there, so the") + console.error('window renders unstyled and never hydrates. Serve the bundle over its scheme instead.') + process.exit(1) + } + + console.log('ok: the renderer is served over its own scheme, not file://.') +} + +await checkRendererAssets() +checkRendererDelivery() + if (!existsSync(electron)) { console.error('electron binary not found; run pnpm install first.') process.exit(1)