From a01d4f7a1634db51f8e04f65e4d65416d041d330 Mon Sep 17 00:00:00 2001 From: CyberWolf Date: Wed, 4 Nov 2026 12:42:26 +0800 Subject: [PATCH 1/2] fix(release): harden package verification and release manifests (#80 follow-up) --- .github/workflows/release.yml | 33 +++--- scripts/__tests__/package-load-check.test.mjs | 105 ++++++++++++++++++ .../__tests__/verify-package-arch.test.mjs | 58 +++++++++- scripts/package-load-check.cjs | 16 ++- scripts/verify-package-arch.mjs | 25 ++++- 5 files changed, 213 insertions(+), 24 deletions(-) create mode 100644 scripts/__tests__/package-load-check.test.mjs diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b390193..6a08d11 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,6 +12,9 @@ jobs: # Only build assets on prereleased to avoid duplicate builds if: github.event.action == 'prereleased' strategy: + # Let the other arches finish, so one run shows every failure. The run still concludes as + # failed, and upload-manifests only promotes from a successful one. + fail-fast: false matrix: # Each arch builds on its own native runner: npm and Prisma only install the prebuilt # binaries of the host arch, and electron-builder doesn't fetch those of another arch. @@ -20,18 +23,23 @@ jobs: - os: macos-15-intel platform: mac arch: x64 + manifest: latest-mac.yml - os: macos-latest platform: mac arch: arm64 + manifest: latest-mac.yml - os: windows-latest platform: win arch: x64 + manifest: latest.yml - os: ubuntu-latest platform: linux arch: x64 + manifest: latest-linux.yml - os: ubuntu-24.04-arm platform: linux arch: arm64 + manifest: latest-linux-arm64.yml runs-on: ${{ matrix.os }} @@ -89,6 +97,16 @@ jobs: APPLE_APP_SPECIFIC_PASSWORD: ${{ secrets.APPLE_APP_SPECIFIC_PASSWORD }} APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} + # The exact file rather than a glob, which other yml files in release/ would satisfy. A missing + # manifest fails the run here, before any asset is published, not at promotion to stable. + - name: Save update manifest as artifact + uses: actions/upload-artifact@v4 + with: + name: update-manifests-${{ matrix.platform }}-${{ matrix.arch }} + path: release/${{ matrix.manifest }} + if-no-files-found: error + retention-days: 30 + - name: Upload release assets (installers only) uses: softprops/action-gh-release@v2 with: @@ -103,13 +121,6 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - name: Save update manifests as artifacts - uses: actions/upload-artifact@v4 - with: - name: update-manifests-${{ matrix.platform }}-${{ matrix.arch }} - path: release/*.yml - retention-days: 30 - # Upload auto-update manifests only when release is promoted to stable upload-manifests: if: github.event.action == 'released' @@ -146,14 +157,6 @@ jobs: echo "Downloaded manifests:" find manifests -type f -name "*.yml" | sort - - name: Filter out builder-debug.yml - run: | - # Remove builder-debug.yml from manifests directory (exists in all platform artifacts) - find manifests -name "builder-debug.yml" -delete - - echo "Remaining manifests:" - find manifests -type f -name "*.yml" | sort - # electron-updater reads one latest-mac.yml and picks the file for its arch from it. - name: Merge macOS manifests run: | diff --git a/scripts/__tests__/package-load-check.test.mjs b/scripts/__tests__/package-load-check.test.mjs new file mode 100644 index 0000000..3e3f2f0 --- /dev/null +++ b/scripts/__tests__/package-load-check.test.mjs @@ -0,0 +1,105 @@ +import { spawnSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +const SCRIPT = fileURLToPath(new URL('../package-load-check.cjs', import.meta.url)); + +const FAKE_SHARP = ` +module.exports = () => ({ png: () => ({ toBuffer: async () => Buffer.alloc(0) }) }); +module.exports.versions = { vips: 'fake' }; +`; +const FAKE_CANVAS = 'module.exports = { createCanvas: () => ({ getContext: () => ({}) }) };'; + +/** A Prisma client that fails where the test's environment tells it to, as a packaged one might. */ +const FAKE_PRISMA = ` +const fs = require('node:fs'); +const { FAIL_QUERY, FAIL_DISCONNECT, FAIL_REMOVE } = process.env; + +if (FAIL_REMOVE) { + fs.rmSync = (_dir, options) => { + throw new Error(options.maxRetries > 0 ? 'EBUSY after retrying' : 'EBUSY without retrying'); + }; +} + +module.exports.PrismaClient = class { + async $queryRaw() { + if (FAIL_QUERY) throw new Error('engine not found'); + } + async $disconnect() { + if (FAIL_DISCONNECT) throw new Error('disconnect failed'); + } +}; +`; + +describe('package-load-check', () => { + let root; + let tempDir; + + /** Runs the script against a fake package, keeping its temp files in a directory of their own. */ + const runCheck = (failures = {}) => + spawnSync(process.execPath, [SCRIPT, path.join(root, 'resources')], { + encoding: 'utf8', + env: { ...process.env, TMPDIR: tempDir, TEMP: tempDir, TMP: tempDir, ...failures }, + }); + + const putModule = (name, source) => { + const dir = path.join(root, 'resources', 'app.asar', 'node_modules', name); + mkdirSync(dir, { recursive: true }); + writeFileSync(path.join(dir, 'index.js'), source); + }; + + beforeEach(() => { + root = mkdtempSync(path.join(tmpdir(), 'package-load-check-')); + tempDir = path.join(root, 'tmp'); + mkdirSync(tempDir); + putModule('sharp', FAKE_SHARP); + putModule('@napi-rs/canvas', FAKE_CANVAS); + putModule('pdf-to-png-converter', ''); + putModule('@prisma/client', FAKE_PRISMA); + }); + + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + it('passes and removes its temp directory when everything loads', () => { + const result = runCheck(); + + expect(result.stdout).toContain('load ok Prisma query engine'); + expect(result.status).toBe(0); + expect(readdirSync(tempDir)).toEqual([]); + }); + + it('fails when a native module does not work', () => { + const result = runCheck({ FAIL_QUERY: '1' }); + + expect(result.stderr).toContain('load FAIL Prisma query engine: engine not found'); + expect(result.status).toBe(1); + }); + + it('only warns when the temp directory cannot be removed, after retrying', () => { + const result = runCheck({ FAIL_REMOVE: '1' }); + + expect(result.stdout).toContain('load ok Prisma query engine'); + expect(result.stderr).toMatch(/warning: could not remove .*: EBUSY after retrying/); + expect(result.status).toBe(0); + }); + + it('only warns when Prisma fails to disconnect, and still removes the temp directory', () => { + const result = runCheck({ FAIL_DISCONNECT: '1' }); + + expect(result.stderr).toContain('warning: could not disconnect Prisma: disconnect failed'); + expect(result.status).toBe(0); + expect(readdirSync(tempDir)).toEqual([]); + }); + + it('keeps the original error when cleanup fails too', () => { + const result = runCheck({ FAIL_QUERY: '1', FAIL_DISCONNECT: '1', FAIL_REMOVE: '1' }); + + expect(result.stderr).toContain('load FAIL Prisma query engine: engine not found'); + expect(result.status).toBe(1); + }); +}); diff --git a/scripts/__tests__/verify-package-arch.test.mjs b/scripts/__tests__/verify-package-arch.test.mjs index de8ee14..d1ec810 100644 --- a/scripts/__tests__/verify-package-arch.test.mjs +++ b/scripts/__tests__/verify-package-arch.test.mjs @@ -1,8 +1,12 @@ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { spawnSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; +import { fileURLToPath } from 'node:url'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { parseBinaryHeader, resolvePackageLayout, verifyBinaries } from '../verify-package-arch.mjs'; +import { loadCheckProblems, parseBinaryHeader, resolvePackageLayout, verifyBinaries } from '../verify-package-arch.mjs'; + +const SCRIPT = fileURLToPath(new URL('../verify-package-arch.mjs', import.meta.url)); const CPU_TYPE = { x64: 0x01000007, arm64: 0x0100000c }; @@ -83,6 +87,56 @@ describe('resolvePackageLayout', () => { }); }); +describe('loadCheckProblems', () => { + it('accepts a clean exit', () => { + expect(loadCheckProblems({ status: 0, signal: null })).toEqual([]); + }); + + it('reports the exit code of a failed check', () => { + expect(loadCheckProblems({ status: 1, signal: null })).toEqual(['Load check failed with exit code 1']); + }); + + it('names the signal when the check was killed, as there is no exit code then', () => { + expect(loadCheckProblems({ status: null, signal: 'SIGSEGV' })).toEqual(['Load check was killed by signal SIGSEGV']); + }); + + it('prefers the spawn error, which explains a timeout better than the signal it sends', () => { + const error = new Error('spawnSync ETIMEDOUT'); + + expect(loadCheckProblems({ error, status: null, signal: 'SIGTERM' })).toEqual([ + 'Load check could not run: spawnSync ETIMEDOUT', + ]); + }); +}); + +describe('command line entry', () => { + // An unknown platform makes main() fail right away, so a non-zero exit proves that it ran. + const run = (script) => spawnSync(process.execPath, [script, '--platform', 'nope'], { encoding: 'utf8' }); + + it('runs when started directly', () => { + const result = run(SCRIPT); + + expect(result.status).toBe(1); + expect(result.stderr).toContain('Unknown --platform "nope"'); + }); + + // Creating a symlink needs extra privileges on Windows. + it.skipIf(process.platform === 'win32')('runs when started through a symlink, rather than passing silently', () => { + const dir = mkdtempSync(path.join(tmpdir(), 'verify-package-arch-link-')); + try { + const link = path.join(dir, 'verify.mjs'); + symlinkSync(SCRIPT, link); + + const result = run(link); + + expect(result.status).toBe(1); + expect(result.stderr).toContain('Unknown --platform "nope"'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); + describe('verifyBinaries', () => { let resourcesDir; let executable; diff --git a/scripts/package-load-check.cjs b/scripts/package-load-check.cjs index 62f016b..214a5ee 100644 --- a/scripts/package-load-check.cjs +++ b/scripts/package-load-check.cjs @@ -13,6 +13,15 @@ const path = require('node:path'); const appModules = path.join(process.argv[2], 'app.asar', 'node_modules'); const loadFromApp = (name) => require(path.join(appModules, name)); +/** Cleanup is not what's being checked: a failure here must neither fail the check nor mask its error. */ +async function cleanUp(description, action) { + try { + await action(); + } catch (error) { + console.warn(` warning: could not ${description}: ${error.message}`); + } +} + async function checkSharp() { const sharp = loadFromApp('sharp'); await sharp({ create: { width: 1, height: 1, channels: 3, background: '#000' } }).png().toBuffer(); @@ -35,8 +44,11 @@ async function checkPrisma() { try { await client.$queryRaw`SELECT 1`; } finally { - await client.$disconnect(); - fs.rmSync(tempDir, { recursive: true, force: true }); + await cleanUp('disconnect Prisma', () => client.$disconnect()); + // Windows can hold the db file for a moment after the engine disconnects. + await cleanUp(`remove ${tempDir}`, () => + fs.rmSync(tempDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 200 }), + ); } } diff --git a/scripts/verify-package-arch.mjs b/scripts/verify-package-arch.mjs index ddeec81..ad054bc 100644 --- a/scripts/verify-package-arch.mjs +++ b/scripts/verify-package-arch.mjs @@ -18,7 +18,7 @@ */ import { spawnSync } from 'node:child_process'; -import { closeSync, existsSync, openSync, readFileSync, readSync, readdirSync } from 'node:fs'; +import { closeSync, existsSync, openSync, readFileSync, readSync, readdirSync, realpathSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { parseArgs } from 'node:util'; @@ -185,6 +185,14 @@ export function verifyBinaries({ resourcesDir, executable, platform, arch }) { return { binaries, problems }; } +/** @returns {string[]} problems found in the spawnSync result of the load check */ +export function loadCheckProblems({ error, status, signal }) { + if (error) return [`Load check could not run: ${error.message}`]; + if (signal) return [`Load check was killed by signal ${signal}`]; + if (status !== 0) return [`Load check failed with exit code ${status}`]; + return []; +} + /** Loads the native modules in the packaged Electron. @returns {string[]} problems */ function runLoadCheck({ resourcesDir, executable }) { const result = spawnSync(executable, [LOAD_CHECK_SCRIPT, resourcesDir], { @@ -195,9 +203,7 @@ function runLoadCheck({ resourcesDir, executable }) { if (result.stdout) process.stdout.write(result.stdout); if (result.stderr) process.stderr.write(result.stderr); - if (result.error) return [`Load check could not run: ${result.error.message}`]; - if (result.status !== 0) return [`Load check failed with exit code ${result.status}`]; - return []; + return loadCheckProblems(result); } function main() { @@ -249,7 +255,16 @@ function main() { console.log(`\nPackage ${platform}-${arch} is OK.`); } -if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { +/** + * Node resolves symlinks in this module's URL but not in argv[1], so comparing them directly would + * skip main() when started through a symlink, and the gate would pass with exit code 0. + */ +function isEntryPoint() { + const entry = process.argv[1]; + return Boolean(entry) && existsSync(entry) && realpathSync(entry) === realpathSync(fileURLToPath(import.meta.url)); +} + +if (isEntryPoint()) { try { main(); } catch (error) { From f60244a1dc08a49367e34ccca021a91773378505 Mon Sep 17 00:00:00 2001 From: CyberWolf Date: Thu, 5 Nov 2026 12:12:04 +0800 Subject: [PATCH 2/2] fix(release): keep load check cleanup from failing the release gate Route the emptyCwd removal added on master through cleanUp, with retries. Update the load check's test fakes for the master changes (canvas re-encode, PDF rendering, sharp removal). Replace the manifest artifact on a re-run, and correct the comment on what a missing manifest stops under fail-fast: false. --- .github/workflows/release.yml | 4 ++- scripts/__tests__/package-load-check.test.mjs | 29 +++++++++++++++---- scripts/package-load-check.cjs | 4 ++- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 457b934..c4dcb98 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -102,7 +102,8 @@ jobs: APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} # The exact file rather than a glob, which other yml files in release/ would satisfy. A missing - # manifest fails the run here, before any asset is published, not at promotion to stable. + # manifest fails the run here, before this job publishes its assets, not at promotion to stable. + # A re-run rebuilds the installers, so its manifest has to replace the one describing the old ones. - name: Save update manifest as artifact uses: actions/upload-artifact@v4 with: @@ -110,6 +111,7 @@ jobs: path: release/${{ matrix.manifest }} if-no-files-found: error retention-days: 30 + overwrite: true - name: Upload release assets (installers only) uses: softprops/action-gh-release@v2 diff --git a/scripts/__tests__/package-load-check.test.mjs b/scripts/__tests__/package-load-check.test.mjs index 3e3f2f0..2d08adf 100644 --- a/scripts/__tests__/package-load-check.test.mjs +++ b/scripts/__tests__/package-load-check.test.mjs @@ -7,11 +7,25 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; const SCRIPT = fileURLToPath(new URL('../package-load-check.cjs', import.meta.url)); -const FAKE_SHARP = ` -module.exports = () => ({ png: () => ({ toBuffer: async () => Buffer.alloc(0) }) }); -module.exports.versions = { vips: 'fake' }; +/** A canvas whose every pixel reads back black, which satisfies the checks that count dark pixels. */ +const FAKE_CANVAS = ` +module.exports = { + createCanvas: () => ({ + encode: async () => Buffer.alloc(8), + getContext: () => ({ + fillRect() {}, + drawImage() {}, + getImageData: (_x, _y, width, height) => ({ data: new Uint8ClampedArray(width * height * 4) }), + }), + }), + loadImage: async () => ({ width: 20, height: 10 }), +}; +`; + +/** As many pages as the sample PDF has; the fake canvas decides what they look like. */ +const FAKE_PDF_RENDERER = ` +exports.renderPdfPages = async () => Array.from({ length: 5 }, () => ({ content: Buffer.alloc(0) })); `; -const FAKE_CANVAS = 'module.exports = { createCanvas: () => ({ getContext: () => ({}) }) };'; /** A Prisma client that fails where the test's environment tells it to, as a packaged one might. */ const FAKE_PRISMA = ` @@ -55,10 +69,12 @@ describe('package-load-check', () => { root = mkdtempSync(path.join(tmpdir(), 'package-load-check-')); tempDir = path.join(root, 'tmp'); mkdirSync(tempDir); - putModule('sharp', FAKE_SHARP); putModule('@napi-rs/canvas', FAKE_CANVAS); putModule('pdf-to-png-converter', ''); putModule('@prisma/client', FAKE_PRISMA); + const rendererDir = path.join(root, 'resources', 'app.asar', 'dist', 'main'); + mkdirSync(rendererDir, { recursive: true }); + writeFileSync(path.join(rendererDir, 'pdf-page-renderer.js'), FAKE_PDF_RENDERER); }); afterEach(() => { @@ -80,11 +96,12 @@ describe('package-load-check', () => { expect(result.status).toBe(1); }); - it('only warns when the temp directory cannot be removed, after retrying', () => { + it('only warns when its temp directories cannot be removed, after retrying', () => { const result = runCheck({ FAIL_REMOVE: '1' }); expect(result.stdout).toContain('load ok Prisma query engine'); expect(result.stderr).toMatch(/warning: could not remove .*: EBUSY after retrying/); + expect(result.stderr).not.toContain('EBUSY without retrying'); expect(result.status).toBe(0); }); diff --git a/scripts/package-load-check.cjs b/scripts/package-load-check.cjs index 049f567..c72315e 100644 --- a/scripts/package-load-check.cjs +++ b/scripts/package-load-check.cjs @@ -119,7 +119,9 @@ async function main() { } process.chdir(os.tmpdir()); - fs.rmSync(emptyCwd, { recursive: true, force: true }); + await cleanUp(`remove ${emptyCwd}`, () => + fs.rmSync(emptyCwd, { recursive: true, force: true, maxRetries: 5, retryDelay: 200 }), + ); process.exit(failed ? 1 : 0); }