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/src/main/build-config.test.ts b/src/main/build-config.test.ts new file mode 100644 index 00000000..1b1254fe --- /dev/null +++ b/src/main/build-config.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect } from 'vitest' +import { readFileSync } from 'fs' +import path from 'path' + +// 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') + +// 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) +} + +/** 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(option('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 (option('win', 'requestedExecutionLevel') === 'requireAdministrator') { + expect(option('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(option('nsis', 'oneClick')).toBe('true') + }) +}) 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