From d94aa83a129b9775257357a28e36106ea9f0e365 Mon Sep 17 00:00:00 2001 From: Benjamin Lu Date: Tue, 8 Sep 2026 15:40:15 -0700 Subject: [PATCH] fix(release): verify bootstrap python on the path that actually ships MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook fetched and verified app-wrapper/extraResources/bootstrap-python/, which stopped being the packaged path when #1484 moved Windows to todesktop-targets/ and this branch moved Linux there too. It kept reporting success over a tree nothing copies, leaving the CI check that runs before the upload as the only guard on the shipped payload. Resolve the destination from todesktop.json — the same config packaging reads — so verification cannot drift from packaging again. A staged directory is verified in place; a missing one is fetched into it, restoring the repair path the 0.6.4 post-mortem asked for. Covered by a test that drives the hook over a temp build tree. --- scripts/todesktop-beforeBuild.cjs | 100 ++++++++++++---- .../standalone/todesktopBeforeBuild.test.ts | 111 ++++++++++++++++++ 2 files changed, 188 insertions(+), 23 deletions(-) create mode 100644 src/main/sources/standalone/todesktopBeforeBuild.test.ts diff --git a/scripts/todesktop-beforeBuild.cjs b/scripts/todesktop-beforeBuild.cjs index 85d1aa77e..2e8340baf 100644 --- a/scripts/todesktop-beforeBuild.cjs +++ b/scripts/todesktop-beforeBuild.cjs @@ -45,6 +45,39 @@ module.exports = async ({ appDir, platform, arch }) => { const fs = await import('node:fs') const path = await import('node:path') + // The `from` path todesktop will actually copy this target's bootstrap-python + // from, read out of the same config that packaging uses. Resolution order + // matches todesktop's own: per-target, then per-platform, then the base list. + // Deriving it here is what keeps the verification below pointed at the tree + // that ships — hardcoding a path let #1484 move Windows to + // `todesktop-targets/` while this hook went on verifying the old location. + function resolveBootstrapFrom() { + const config = JSON.parse(fs.readFileSync(path.join(appDir, 'todesktop.json'), 'utf-8')) + const candidates = [ + config.targetOverrides?.[platform]?.[arch]?.extraResources, + config.platformOverrides?.[platform]?.extraResources, + config.extraResources, + ] + for (const list of candidates) { + const entry = list?.find((resource) => resource.to === 'bootstrap-python') + if (entry) return entry.from + } + return null + } + + function moveInto(source, dest) { + fs.mkdirSync(path.dirname(dest), { recursive: true }) + try { + fs.renameSync(source, dest) + } catch (err) { + // Cross-device rename fails on todesktop's builders when tmp and the + // build tree are on different mounts. + if (err.code !== 'EXDEV') throw err + fs.cpSync(source, dest, { recursive: true }) + fs.rmSync(source, { recursive: true, force: true }) + } + } + const key = `${platform}-${arch}` const bootstrapPlatform = PLATFORM_MAP[key] if (!bootstrapPlatform) { @@ -52,8 +85,14 @@ module.exports = async ({ appDir, platform, arch }) => { return } - console.log(`[todesktop:beforeBuild] Fetching bootstrap python for ${bootstrapPlatform}`) - const script = path.join(appDir, 'scripts', 'fetch-bootstrap-python.mjs') + const from = resolveBootstrapFrom() + if (!from) { + throw new Error( + `[todesktop:beforeBuild] todesktop.json declares no bootstrap-python resource for ${key}. ` + + `Refusing to build — the installer would ship without a git backend.` + ) + } + // todesktop layout: // /app-wrapper/app/ <- this is `appDir` (electron-builder appDirectory) // /app-wrapper/extraResources/ <- where extraResources.from is staged from @@ -63,34 +102,49 @@ module.exports = async ({ appDir, platform, arch }) => { // to `appDir`) but electron-builder still warned `file source doesn't exist // from=app-wrapper/extraResources/...` — and the dmg shipped without bootstrap-python. // Going up one level from `appDir` puts the archive where todesktop actually reads it. - const outDir = path.join(appDir, '..', 'extraResources', 'bootstrap-python') - // fetch-bootstrap-python.mjs now exits non-zero on failure, which bubbles - // up here via execSync. Don't wrap in try/catch — a failed fetch must fail - // the build (see 0.6.4 post-mortem: a swallowed fetch error shipped an - // installer with no bootstrap-python, stranding new installs). - execSync( - `node "${script}" --platform ${bootstrapPlatform} --output-dir "${outDir}"`, - { stdio: 'inherit', cwd: appDir } - ) + const destDir = path.join(appDir, '..', 'extraResources', from) + + // CI stages and uploads these directories, so the usual case is "already + // here" and the fetch is a repair path for an upload that arrived short. + if (fs.existsSync(destDir)) { + console.log(`[todesktop:beforeBuild] Found staged bootstrap python at ${destDir}`) + } else { + console.log(`[todesktop:beforeBuild] Fetching bootstrap python for ${bootstrapPlatform}`) + const script = path.join(appDir, 'scripts', 'fetch-bootstrap-python.mjs') + // The fetch script writes /, but the shipped path is + // named by todesktop.json, so fetch into a scratch dir and move it over. + const scratch = path.join(appDir, '..', 'extraResources', '.bootstrap-fetch') + // fetch-bootstrap-python.mjs now exits non-zero on failure, which bubbles + // up here via execSync. Don't wrap in try/catch — a failed fetch must fail + // the build (see 0.6.4 post-mortem: a swallowed fetch error shipped an + // installer with no bootstrap-python, stranding new installs). + execSync( + `node "${script}" --platform ${bootstrapPlatform} --output-dir "${scratch}"`, + { stdio: 'inherit', cwd: appDir } + ) + moveInto(path.join(scratch, bootstrapPlatform), destDir) + fs.rmSync(scratch, { recursive: true, force: true }) + } - // Defense-in-depth: even if the fetch script returns success, verify the - // expected binaries exist before handing control back to todesktop. A - // divergence between the fetch script's success criteria and what the app - // looks for at runtime would otherwise reproduce the same silent failure. - const expectedPython = path.join(outDir, bootstrapPlatform, PYTHON_BINARY[bootstrapPlatform]) + // Defense-in-depth: whether the directory was staged by CI or fetched just + // now, verify the expected binaries before handing control back to + // todesktop. This is the check that has to sit on the shipped path — an + // upload that dropped the tree, or a fetch whose success criteria drifted + // from what the app looks for at runtime, both look fine without it. + const expectedPython = path.join(destDir, PYTHON_BINARY[bootstrapPlatform]) if (!fs.existsSync(expectedPython)) { throw new Error( - `[todesktop:beforeBuild] fetch script returned success but ${expectedPython} is missing. ` + - `Refusing to build — the installer would not provide a git backend and "Latest Stable" ` + - `installs would silently strand on the bundled ComfyUI version.` + `[todesktop:beforeBuild] ${expectedPython} is missing. Refusing to build — the ` + + `installer would not provide a git backend and "Latest Stable" installs would ` + + `silently strand on the bundled ComfyUI version.` ) } - const expectedUv = path.join(outDir, bootstrapPlatform, UV_BINARY[bootstrapPlatform]) + const expectedUv = path.join(destDir, UV_BINARY[bootstrapPlatform]) if (!fs.existsSync(expectedUv)) { throw new Error( - `[todesktop:beforeBuild] fetch script returned success but ${expectedUv} is missing. ` + - `Refusing to build — the bootstrap archive predates bootstrap-v2 (no bundled uv). ` + - `Bump the default tag in fetch-bootstrap-python.mjs or publish the v2 archives.` + `[todesktop:beforeBuild] ${expectedUv} is missing. Refusing to build — the bootstrap ` + + `tree predates bootstrap-v2 (no bundled uv), or the upload arrived incomplete. Bump ` + + `the default tag in fetch-bootstrap-python.mjs or publish the v2 archives.` ) } console.log(`[todesktop:beforeBuild] Verified ${expectedPython} and ${expectedUv}`) diff --git a/src/main/sources/standalone/todesktopBeforeBuild.test.ts b/src/main/sources/standalone/todesktopBeforeBuild.test.ts new file mode 100644 index 000000000..e9b6428ba --- /dev/null +++ b/src/main/sources/standalone/todesktopBeforeBuild.test.ts @@ -0,0 +1,111 @@ +import fs from 'fs' +import os from 'os' +import path from 'path' +import { pathToFileURL } from 'url' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +// Exercises scripts/todesktop-beforeBuild.cjs, the last check standing between +// a short upload and a release that ships without a git backend (the 0.6.4 +// post-mortem). It lives here because vitest only collects `src/**/*.test.ts`. +type BeforeBuildHook = (ctx: { appDir: string; platform: string; arch: string }) => Promise + +const repoRoot = process.cwd() +const hookPath = path.join(repoRoot, 'scripts', 'todesktop-beforeBuild.cjs') + +interface ExtraResource { + from: string + to: string +} + +async function loadHook(): Promise { + const mod = (await import(pathToFileURL(hookPath).href)) as { default: BeforeBuildHook } + return mod.default +} + +/** The `from` todesktop.json declares for a target, i.e. the only path whose + * contents reach the packaged app. */ +function declaredFrom(platform: string, arch: string): string { + const config = JSON.parse(fs.readFileSync(path.join(repoRoot, 'todesktop.json'), 'utf-8')) as { + extraResources?: ExtraResource[] + targetOverrides?: Record> + platformOverrides?: Record + } + const lists = [ + config.targetOverrides?.[platform]?.[arch]?.extraResources, + config.platformOverrides?.[platform]?.extraResources, + config.extraResources + ] + for (const list of lists) { + const entry = list?.find((resource) => resource.to === 'bootstrap-python') + if (entry) return entry.from + } + throw new Error(`no bootstrap-python resource declared for ${platform}-${arch}`) +} + +let tmpRoot: string +let appDir: string +let extraResources: string + +beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'todesktop-hook-')) + appDir = path.join(tmpRoot, 'app-wrapper', 'app') + extraResources = path.join(tmpRoot, 'app-wrapper', 'extraResources') + fs.mkdirSync(appDir, { recursive: true }) + fs.mkdirSync(extraResources, { recursive: true }) + // The hook reads the real config to decide which path it must verify. + fs.copyFileSync(path.join(repoRoot, 'todesktop.json'), path.join(appDir, 'todesktop.json')) +}) + +afterEach(() => { + fs.rmSync(tmpRoot, { recursive: true, force: true }) +}) + +function stageBootstrap(platform: string, arch: string, binaries: string[]): string { + const destDir = path.join(extraResources, declaredFrom(platform, arch)) + for (const rel of binaries) { + const file = path.join(destDir, rel) + fs.mkdirSync(path.dirname(file), { recursive: true }) + fs.writeFileSync(file, '') + } + return destDir +} + +describe('todesktop beforeBuild hook', () => { + it('verifies the staged tree at the path todesktop.json declares', async () => { + stageBootstrap('linux', 'x64', ['bin/python3', 'bin/uv']) + const hook = await loadHook() + + // Resolves without network: a staged directory is never re-fetched. + await expect(hook({ appDir, platform: 'linux', arch: 'x64' })).resolves.toBeUndefined() + }) + + it('verifies the Windows target at its own staged path', async () => { + stageBootstrap('windows', 'arm64', ['python.exe', 'uv.exe']) + const hook = await loadHook() + + await expect(hook({ appDir, platform: 'windows', arch: 'arm64' })).resolves.toBeUndefined() + }) + + it('fails the build when the staged tree arrived without its Python', async () => { + // An upload that dropped the binary — the failure the hook exists to catch. + stageBootstrap('linux', 'x64', ['bin/uv']) + const hook = await loadHook() + + await expect(hook({ appDir, platform: 'linux', arch: 'x64' })).rejects.toThrow(/is missing/) + }) + + it('fails the build when the staged tree arrived without uv', async () => { + stageBootstrap('linux', 'x64', ['bin/python3']) + const hook = await loadHook() + + await expect(hook({ appDir, platform: 'linux', arch: 'x64' })).rejects.toThrow(/is missing/) + }) + + it('skips architectures that ship no bootstrap python', async () => { + // Linux ARM64 ships a placeholder directory and no interpreter, so there + // is nothing to verify and nothing to fetch. + const hook = await loadHook() + + await expect(hook({ appDir, platform: 'linux', arch: 'arm64' })).resolves.toBeUndefined() + }) +})