From 049f2665410719fbfbe619274d90492de7e9f84b Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 27 Jul 2026 10:45:48 +0200 Subject: [PATCH 1/3] fix(build): install per-machine so the Windows installer elevates (#264) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `win.requestedExecutionLevel: requireAdministrator` is applied only to Kudu.exe's manifest — app-builder-lib patches it in signAndEditResources and it never reaches the NSIS script. With `nsis.perMachine` unset it defaults to false, so installer.nsi compiled with `RequestExecutionLevel user`: Kudu-Setup-*.exe never asked for elevation, installed into %LOCALAPPDATA%\Programs\Kudu, and then tried to launch a binary whose manifest demands admin. Running the installer elevated by hand made the whole chain consistent, which is why that path worked. Two further consequences of the mismatch: - %LOCALAPPDATA%\Programs is user-writable and applyAutoLaunchWin32 registers Kudu.exe with RunLevel HighestAvailable, so an auto-elevating binary sat in a directory any process running as the user can rewrite. - updateInfo.isAdminRightsRequired is derived from perMachine, so the published metadata advertised false for an app that cannot run without admin. perMachine: true makes the installer request admin up front, install to Program Files, and set isAdminRightsRequired. Existing per-user installs are migrated by electron-builder, which runs uninstallOldVersion against HKEY_CURRENT_USER when the install mode is all. Adds a test pinning the app manifest and installer execution levels together, since nothing in the app's own code paths can catch this. --- electron-builder.yml | 8 ++++ package-lock.json | 73 +++++------------------------------ package.json | 2 + src/main/build-config.test.ts | 47 ++++++++++++++++++++++ 4 files changed, 66 insertions(+), 64 deletions(-) create mode 100644 src/main/build-config.test.ts diff --git a/electron-builder.yml b/electron-builder.yml index 46ce540a..74d7e23b 100644 --- a/electron-builder.yml +++ b/electron-builder.yml @@ -33,6 +33,14 @@ win: nsis: oneClick: true + # Must stay in sync with win.requestedExecutionLevel above. + # requestedExecutionLevel only patches the manifest of Kudu.exe — it has no + # effect on the installer. Left per-user (the default), the installer compiles + # with `RequestExecutionLevel user`, so it never elevates and then drops an + # admin-manifested exe into the user-writable %LOCALAPPDATA%\Programs. Going + # per-machine makes the installer request elevation up front, installs to + # Program Files, and marks isAdminRightsRequired in the update metadata. + perMachine: true allowToChangeInstallationDirectory: false deleteAppDataOnUninstall: false shortcutName: Kudu diff --git a/package-lock.json b/package-lock.json index 5f5a60fd..4c019325 100644 --- a/package-lock.json +++ b/package-lock.json @@ -34,6 +34,7 @@ "@electron/rebuild": "^4.0.3", "@tailwindcss/vite": "^4.2.1", "@types/better-sqlite3": "^7.6.13", + "@types/js-yaml": "^4.0.9", "@types/react": "^19.2.17", "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^5.1.4", @@ -42,6 +43,7 @@ "electron": "^41.3.0", "electron-builder": "^26.8.1", "electron-vite": "^5.0.0", + "js-yaml": "^4.3.0", "patch-package": "^8.0.1", "react": "^19.2.7", "react-dom": "^19.2.7", @@ -2737,6 +2739,13 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/js-yaml": { + "version": "4.0.9", + "resolved": "https://registry.npmjs.org/@types/js-yaml/-/js-yaml-4.0.9.tgz", + "integrity": "sha512-k4MGaQl5TGo/iipqb2UDG2UwjXziSWkh0uysQelTlJpX1qGlpUZYm8PnO4DxG1qBomtJUdYJ6qR6xdIah10JLg==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/keyv": { "version": "3.1.4", "resolved": "https://registry.npmjs.org/@types/keyv/-/keyv-3.1.4.tgz", @@ -4157,16 +4166,6 @@ "node": ">=18" } }, - "node_modules/conventional-commits-filter": { - "version": "6.0.1", - "resolved": "https://registry.npmjs.org/conventional-commits-filter/-/conventional-commits-filter-6.0.1.tgz", - "integrity": "sha512-cs+LadpH7Kpw0M3k8wurk+sOVVDAENA0iK4OBOrkL94j5lEVYRJ4j3zd2bhY9qgzyrPqthdcYT3axzRN7AliMg==", - "extraneous": true, - "license": "MIT", - "engines": { - "node": ">=22" - } - }, "node_modules/conventional-commits-parser": { "version": "7.1.0", "resolved": "https://registry.npmjs.org/conventional-commits-parser/-/conventional-commits-parser-7.1.0.tgz", @@ -5645,33 +5644,6 @@ "url": "https://ko-fi.com/dangreen" } }, - "node_modules/git-raw-commits/node_modules/conventional-commits-filter": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/conventional-commits-filter/-/conventional-commits-filter-5.0.0.tgz", - "integrity": "sha512-tQMagCOC59EVgNZcC5zl7XqO30Wki9i9J3acbUvkaosCT6JX3EeFwJD7Qqp4MCikRnzS18WXV3BLIQ66ytu6+Q==", - "extraneous": true, - "license": "MIT", - "engines": { - "node": ">=18" - } - }, - "node_modules/git-raw-commits/node_modules/conventional-commits-parser": { - "version": "6.4.0", - "resolved": "https://registry.npmjs.org/conventional-commits-parser/-/conventional-commits-parser-6.4.0.tgz", - "integrity": "sha512-tvRg7FIBNlyPzjdG8wWRlPHQJJHI7DylhtRGeU9Lq+JuoPh5BKpPRX83ZdLrvXuOSu5Eo/e7SzOQhU4Hd2Miuw==", - "extraneous": true, - "license": "MIT", - "dependencies": { - "@simple-libs/stream-utils": "^1.2.0", - "meow": "^13.0.0" - }, - "bin": { - "conventional-commits-parser": "dist/cli/index.js" - }, - "engines": { - "node": ">=18" - } - }, "node_modules/git-semver-tags": { "version": "8.0.1", "resolved": "https://registry.npmjs.org/git-semver-tags/-/git-semver-tags-8.0.1.tgz", @@ -5746,33 +5718,6 @@ "url": "https://ko-fi.com/dangreen" } }, - "node_modules/git-semver-tags/node_modules/conventional-commits-filter": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/conventional-commits-filter/-/conventional-commits-filter-5.0.0.tgz", - "integrity": "sha512-tQMagCOC59EVgNZcC5zl7XqO30Wki9i9J3acbUvkaosCT6JX3EeFwJD7Qqp4MCikRnzS18WXV3BLIQ66ytu6+Q==", - "extraneous": true, - "license": "MIT", - "engines": { - "node": ">=18" - } - }, - "node_modules/git-semver-tags/node_modules/conventional-commits-parser": { - "version": "6.4.0", - "resolved": "https://registry.npmjs.org/conventional-commits-parser/-/conventional-commits-parser-6.4.0.tgz", - "integrity": "sha512-tvRg7FIBNlyPzjdG8wWRlPHQJJHI7DylhtRGeU9Lq+JuoPh5BKpPRX83ZdLrvXuOSu5Eo/e7SzOQhU4Hd2Miuw==", - "extraneous": true, - "license": "MIT", - "dependencies": { - "@simple-libs/stream-utils": "^1.2.0", - "meow": "^13.0.0" - }, - "bin": { - "conventional-commits-parser": "dist/cli/index.js" - }, - "engines": { - "node": ">=18" - } - }, "node_modules/github-from-package": { "version": "0.0.0", "resolved": "https://registry.npmjs.org/github-from-package/-/github-from-package-0.0.0.tgz", diff --git a/package.json b/package.json index d3464e30..30057c32 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "@electron/rebuild": "^4.0.3", "@tailwindcss/vite": "^4.2.1", "@types/better-sqlite3": "^7.6.13", + "@types/js-yaml": "^4.0.9", "@types/react": "^19.2.17", "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^5.1.4", @@ -61,6 +62,7 @@ "electron": "^41.3.0", "electron-builder": "^26.8.1", "electron-vite": "^5.0.0", + "js-yaml": "^4.3.0", "patch-package": "^8.0.1", "react": "^19.2.7", "react-dom": "^19.2.7", diff --git a/src/main/build-config.test.ts b/src/main/build-config.test.ts new file mode 100644 index 00000000..56d5c360 --- /dev/null +++ b/src/main/build-config.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect } from 'vitest' +import { readFileSync } from 'fs' +import path from 'path' +import yaml from 'js-yaml' + +// Guards the packaging invariants that only show up once a user runs the +// installer — nothing in the app's own code paths can catch a regression here. +// See https://github.com/AdventDevInc/kudu/issues/264. + +const CONFIG_PATH = path.resolve(__dirname, '..', '..', 'electron-builder.yml') + +interface BuildConfig { + win?: { requestedExecutionLevel?: string } + nsis?: { + oneClick?: boolean + perMachine?: boolean + } +} + +const config = yaml.load(readFileSync(CONFIG_PATH, 'utf-8')) as BuildConfig + +describe('electron-builder.yml', () => { + it('requests admin for the app executable', () => { + // Kudu edits HKLM, system directories and other machine-wide state, so the + // manifest asks for elevation rather than re-launching at runtime. + expect(config.win?.requestedExecutionLevel).toBe('requireAdministrator') + }) + + it('installs per-machine whenever the app manifest requires admin', () => { + // requestedExecutionLevel is applied to Kudu.exe only; the NSIS installer + // has a separate execution level derived from nsis.perMachine. If they + // disagree, the installer runs unelevated and installs an auto-elevating + // binary into user-writable %LOCALAPPDATA%\Programs — which both breaks the + // install and is a local privilege-escalation path, since the auto-launch + // task runs that exe with RunLevel HighestAvailable. + if (config.win?.requestedExecutionLevel === 'requireAdministrator') { + expect(config.nsis?.perMachine).toBe(true) + } + }) + + it('keeps the one-click installer flow', () => { + // perMachine + oneClick is what makes electron-builder mark + // isAdminRightsRequired on the published update metadata, so electron-updater + // elevates when applying an update. + expect(config.nsis?.oneClick).toBe(true) + }) +}) From 183c7e9bb18c911c793a6d18e657381cad9fddc8 Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 27 Jul 2026 12:23:56 +0200 Subject: [PATCH 2/3] test(build): read electron-builder.yml without adding a dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The js-yaml devDependency added alongside the per-machine fix desynced package-lock.json — npm pruned the nested conventional-commits-parser and conventional-commits-filter entries it considered extraneous locally, which every CI job then rejected with `npm ci` EUSAGE. Restore package.json and package-lock.json to their state on main and read the two options the test cares about directly. The config is flat and hand-maintained, so a top-level block lookup is enough and costs no lockfile churn. --- package-lock.json | 73 ++++++++++++++++++++++++++++++----- package.json | 2 - src/main/build-config.test.ts | 34 ++++++++++------ 3 files changed, 86 insertions(+), 23 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4c019325..5f5a60fd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -34,7 +34,6 @@ "@electron/rebuild": "^4.0.3", "@tailwindcss/vite": "^4.2.1", "@types/better-sqlite3": "^7.6.13", - "@types/js-yaml": "^4.0.9", "@types/react": "^19.2.17", "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^5.1.4", @@ -43,7 +42,6 @@ "electron": "^41.3.0", "electron-builder": "^26.8.1", "electron-vite": "^5.0.0", - "js-yaml": "^4.3.0", "patch-package": "^8.0.1", "react": "^19.2.7", "react-dom": "^19.2.7", @@ -2739,13 +2737,6 @@ "dev": true, "license": "MIT" }, - "node_modules/@types/js-yaml": { - "version": "4.0.9", - "resolved": "https://registry.npmjs.org/@types/js-yaml/-/js-yaml-4.0.9.tgz", - "integrity": "sha512-k4MGaQl5TGo/iipqb2UDG2UwjXziSWkh0uysQelTlJpX1qGlpUZYm8PnO4DxG1qBomtJUdYJ6qR6xdIah10JLg==", - "dev": true, - "license": "MIT" - }, "node_modules/@types/keyv": { "version": "3.1.4", "resolved": "https://registry.npmjs.org/@types/keyv/-/keyv-3.1.4.tgz", @@ -4166,6 +4157,16 @@ "node": ">=18" } }, + "node_modules/conventional-commits-filter": { + "version": "6.0.1", + "resolved": "https://registry.npmjs.org/conventional-commits-filter/-/conventional-commits-filter-6.0.1.tgz", + "integrity": "sha512-cs+LadpH7Kpw0M3k8wurk+sOVVDAENA0iK4OBOrkL94j5lEVYRJ4j3zd2bhY9qgzyrPqthdcYT3axzRN7AliMg==", + "extraneous": true, + "license": "MIT", + "engines": { + "node": ">=22" + } + }, "node_modules/conventional-commits-parser": { "version": "7.1.0", "resolved": "https://registry.npmjs.org/conventional-commits-parser/-/conventional-commits-parser-7.1.0.tgz", @@ -5644,6 +5645,33 @@ "url": "https://ko-fi.com/dangreen" } }, + "node_modules/git-raw-commits/node_modules/conventional-commits-filter": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/conventional-commits-filter/-/conventional-commits-filter-5.0.0.tgz", + "integrity": "sha512-tQMagCOC59EVgNZcC5zl7XqO30Wki9i9J3acbUvkaosCT6JX3EeFwJD7Qqp4MCikRnzS18WXV3BLIQ66ytu6+Q==", + "extraneous": true, + "license": "MIT", + "engines": { + "node": ">=18" + } + }, + "node_modules/git-raw-commits/node_modules/conventional-commits-parser": { + "version": "6.4.0", + "resolved": "https://registry.npmjs.org/conventional-commits-parser/-/conventional-commits-parser-6.4.0.tgz", + "integrity": "sha512-tvRg7FIBNlyPzjdG8wWRlPHQJJHI7DylhtRGeU9Lq+JuoPh5BKpPRX83ZdLrvXuOSu5Eo/e7SzOQhU4Hd2Miuw==", + "extraneous": true, + "license": "MIT", + "dependencies": { + "@simple-libs/stream-utils": "^1.2.0", + "meow": "^13.0.0" + }, + "bin": { + "conventional-commits-parser": "dist/cli/index.js" + }, + "engines": { + "node": ">=18" + } + }, "node_modules/git-semver-tags": { "version": "8.0.1", "resolved": "https://registry.npmjs.org/git-semver-tags/-/git-semver-tags-8.0.1.tgz", @@ -5718,6 +5746,33 @@ "url": "https://ko-fi.com/dangreen" } }, + "node_modules/git-semver-tags/node_modules/conventional-commits-filter": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/conventional-commits-filter/-/conventional-commits-filter-5.0.0.tgz", + "integrity": "sha512-tQMagCOC59EVgNZcC5zl7XqO30Wki9i9J3acbUvkaosCT6JX3EeFwJD7Qqp4MCikRnzS18WXV3BLIQ66ytu6+Q==", + "extraneous": true, + "license": "MIT", + "engines": { + "node": ">=18" + } + }, + "node_modules/git-semver-tags/node_modules/conventional-commits-parser": { + "version": "6.4.0", + "resolved": "https://registry.npmjs.org/conventional-commits-parser/-/conventional-commits-parser-6.4.0.tgz", + "integrity": "sha512-tvRg7FIBNlyPzjdG8wWRlPHQJJHI7DylhtRGeU9Lq+JuoPh5BKpPRX83ZdLrvXuOSu5Eo/e7SzOQhU4Hd2Miuw==", + "extraneous": true, + "license": "MIT", + "dependencies": { + "@simple-libs/stream-utils": "^1.2.0", + "meow": "^13.0.0" + }, + "bin": { + "conventional-commits-parser": "dist/cli/index.js" + }, + "engines": { + "node": ">=18" + } + }, "node_modules/github-from-package": { "version": "0.0.0", "resolved": "https://registry.npmjs.org/github-from-package/-/github-from-package-0.0.0.tgz", diff --git a/package.json b/package.json index 30057c32..d3464e30 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,6 @@ "@electron/rebuild": "^4.0.3", "@tailwindcss/vite": "^4.2.1", "@types/better-sqlite3": "^7.6.13", - "@types/js-yaml": "^4.0.9", "@types/react": "^19.2.17", "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^5.1.4", @@ -62,7 +61,6 @@ "electron": "^41.3.0", "electron-builder": "^26.8.1", "electron-vite": "^5.0.0", - "js-yaml": "^4.3.0", "patch-package": "^8.0.1", "react": "^19.2.7", "react-dom": "^19.2.7", diff --git a/src/main/build-config.test.ts b/src/main/build-config.test.ts index 56d5c360..1b1254fe 100644 --- a/src/main/build-config.test.ts +++ b/src/main/build-config.test.ts @@ -1,29 +1,39 @@ import { describe, it, expect } from 'vitest' import { readFileSync } from 'fs' import path from 'path' -import yaml from 'js-yaml' // Guards the packaging invariants that only show up once a user runs the // installer — nothing in the app's own code paths can catch a regression here. // See https://github.com/AdventDevInc/kudu/issues/264. const CONFIG_PATH = path.resolve(__dirname, '..', '..', 'electron-builder.yml') +const CONFIG = readFileSync(CONFIG_PATH, 'utf-8') -interface BuildConfig { - win?: { requestedExecutionLevel?: string } - nsis?: { - oneClick?: boolean - perMachine?: boolean - } +// electron-builder.yml is hand-maintained and flat, so the two lookups below are +// done without pulling in a YAML parser — the repo has no direct one, and adding +// a dependency for a three-assertion test is not worth the lockfile churn. + +/** Lines belonging to a top-level `key:` block, up to the next unindented line. */ +function block(key: string): string[] { + const lines = CONFIG.split(/\r?\n/) + const start = lines.indexOf(`${key}:`) + if (start === -1) throw new Error(`electron-builder.yml has no top-level "${key}:" block`) + const rest = lines.slice(start + 1) + const end = rest.findIndex((line) => line.trim() !== '' && !line.startsWith(' ')) + return end === -1 ? rest : rest.slice(0, end) } -const config = yaml.load(readFileSync(CONFIG_PATH, 'utf-8')) as BuildConfig +/** Value of a direct ` key: value` child of a top-level block. */ +function option(key: string, name: string): string | undefined { + const line = block(key).find((l) => l.startsWith(` ${name}:`)) + return line?.slice(line.indexOf(':') + 1).trim() +} describe('electron-builder.yml', () => { it('requests admin for the app executable', () => { // Kudu edits HKLM, system directories and other machine-wide state, so the // manifest asks for elevation rather than re-launching at runtime. - expect(config.win?.requestedExecutionLevel).toBe('requireAdministrator') + expect(option('win', 'requestedExecutionLevel')).toBe('requireAdministrator') }) it('installs per-machine whenever the app manifest requires admin', () => { @@ -33,8 +43,8 @@ describe('electron-builder.yml', () => { // binary into user-writable %LOCALAPPDATA%\Programs — which both breaks the // install and is a local privilege-escalation path, since the auto-launch // task runs that exe with RunLevel HighestAvailable. - if (config.win?.requestedExecutionLevel === 'requireAdministrator') { - expect(config.nsis?.perMachine).toBe(true) + if (option('win', 'requestedExecutionLevel') === 'requireAdministrator') { + expect(option('nsis', 'perMachine')).toBe('true') } }) @@ -42,6 +52,6 @@ describe('electron-builder.yml', () => { // perMachine + oneClick is what makes electron-builder mark // isAdminRightsRequired on the published update metadata, so electron-updater // elevates when applying an update. - expect(config.nsis?.oneClick).toBe(true) + expect(option('nsis', 'oneClick')).toBe('true') }) }) From d08c03bf2ca2e66dc5d2534cd85699ac8eadce4c Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 27 Jul 2026 12:27:07 +0200 Subject: [PATCH 3/3] test(cleaner): give the batch-flush test room on the Windows runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `flushes in batches so a large clean never buffers everything` creates and deletes 1200 real files. That count is load-bearing — flushPending() fires every 500 records, so proving a remainder flush on top of two full batches needs more than 1000 items — and it is the slowest test in the suite at roughly 2s locally. On the Windows CI runner it crossed the 5s default and timed out. Raise the timeout for that one test rather than shrinking the fixture and losing the third-batch assertion. Unrelated to the installer fix; it surfaced as a flake on this branch. --- src/main/services/file-utils.clean-log.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/services/file-utils.clean-log.test.ts b/src/main/services/file-utils.clean-log.test.ts index da3de802..92e4b60b 100644 --- a/src/main/services/file-utils.clean-log.test.ts +++ b/src/main/services/file-utils.clean-log.test.ts @@ -119,6 +119,11 @@ describe('cleanItems deletion logging', () => { expect(state.recorded[0].category).toBe('system') }) + // The 1200 is load-bearing: flushPending() fires every 500 records, so it takes + // more than 1000 items to prove a remainder flush happens on top of two full + // batches. That means 1200 real files created and deleted, which runs ~2s on a + // developer machine but has timed out against the 5s default on the Windows CI + // runner. Give it room rather than weakening the assertion. it('flushes in batches so a large clean never buffers everything', async () => { state.keepDeletionLog = true seedItems(1200) @@ -130,7 +135,7 @@ describe('cleanItems deletion logging', () => { expect(state.recorded).toHaveLength(1200) // 500 + 500 + a final flush of the 200 remainder. expect(state.batches).toBe(3) - }) + }, 30_000) it('tags records with the calling surface, defaulting to local', async () => { state.keepDeletionLog = true