diff --git a/electron/tests/crash-recovery.test.mjs b/electron/tests/crash-recovery.test.mjs new file mode 100644 index 0000000..ae21008 --- /dev/null +++ b/electron/tests/crash-recovery.test.mjs @@ -0,0 +1,208 @@ +// Crash-path coverage for the owned-child watchdog. +// +// The existing reap test in runtime.test.mjs drives `watchdog.close()`, which is +// the GRACEFUL path: close() ends the child's stdin and the child reaps from +// `reader.on('close')`. That is the shutdown a normal quit takes, and it is not +// the reason the watchdog exists. The crash guard exists for the case where the +// main process dies without running any code — SIGKILL, Task Manager, a renderer +// OOM taking the app down — leaving the sidecars parented to nothing. +// +// Under Tauri that case was covered by a kernel primitive with its own test +// (`windows_job_object_kills_assigned_child_on_drop`). The migration replaced it +// with a userland watchdog and did not replace the test, so the whole point of +// the component was unverified. These tests close that gap from both directions: +// the liveness-poll mechanism in isolation, and a real SIGKILL end to end. +import assert from 'node:assert/strict'; +import { spawn } from 'node:child_process'; +import { once } from 'node:events'; +import path from 'node:path'; +import process from 'node:process'; +import readline from 'node:readline'; +import test from 'node:test'; +import { setTimeout as delay } from 'node:timers/promises'; +import { fileURLToPath } from 'node:url'; +import { OwnedChildWatchdog } from '../watchdog.js'; + +const quietLogger = { info() {}, warn() {}, error() {}, crash() {} }; +const electronDir = fileURLToPath(new URL('..', import.meta.url)); +const watchdogScript = path.join(electronDir, 'child-watchdog.cjs'); + +// Sized off the watchdog's own worst case rather than an observed happy path: the +// win32 reap runs a Win32_Process CIM snapshot (spawnSync timeout 8s) and then a +// taskkill per tracked root (10s), and the liveness poll only ticks every 1500ms. +// A tighter budget fails on a loaded machine while the implementation is correct. +const REAP_BUDGET_MS = 60_000; +const POLL_INTERVAL_MS = 250; + +function isAlive(pid) { + try { + process.kill(pid, 0); + return true; + } catch (error) { + // EPERM means the pid exists but belongs to someone else — still alive. + return error?.code === 'EPERM'; + } +} + +async function waitForDeath(pid, budgetMs = REAP_BUDGET_MS) { + const deadline = Date.now() + budgetMs; + while (Date.now() < deadline) { + if (!isAlive(pid)) return true; + await delay(POLL_INTERVAL_MS); + } + return false; +} + +function spawnDisposable() { + return spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + detached: process.platform !== 'win32', + stdio: 'ignore', + windowsHide: true, + env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' }, + }); +} + +function killIfAlive(child) { + if (child?.pid && child.exitCode === null && child.signalCode === null) { + try { + child.kill('SIGKILL'); + } catch { + // Already gone. + } + } +} + +function spawnFixture(name) { + return spawn(process.execPath, [path.join(electronDir, 'tests', 'fixtures', name)], { + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true, + env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' }, + }); +} + +// Fixtures report their spawned pid on stdout once setup is complete. Waiting for +// that line before killing anything keeps a reap from being a false pass produced +// by killing the fixture mid-setup. +async function firstLine(child, stderrSink) { + const reader = readline.createInterface({ input: child.stdout, crlfDelay: Infinity }); + const line = await Promise.race([ + once(reader, 'line').then(([value]) => value), + once(child, 'exit').then(() => null), + delay(REAP_BUDGET_MS).then(() => null), + ]); + reader.close(); + assert.ok(line, `fixture never reported a pid. stderr: ${stderrSink.join('')}`); + return JSON.parse(line); +} + +function collectStderr(child) { + const chunks = []; + child.stderr.on('data', (chunk) => chunks.push(String(chunk))); + return chunks; +} + +test('the liveness poll alone reaps a tracked child after the guarded parent is SIGKILLed', async (t) => { + // Isolates the poll (`parentIsAlive` -> `reapAndExit`) from the stdin-close + // path: the watchdog is started by THIS process, so its stdin stays open for + // the whole test, while the process it is told to guard is a separate fixture. + // Killing that fixture leaves the pipe intact, so a reap can only come from the + // poll noticing the parent is gone. + const owner = spawnFixture('child-owner.mjs'); + t.after(() => killIfAlive(owner)); + const ownerStderr = collectStderr(owner); + const { childPid } = await firstLine(owner, ownerStderr); + + const watchdog = await OwnedChildWatchdog.start({ + scriptPath: watchdogScript, + logger: quietLogger, + parentPid: owner.pid, + }); + t.after(() => watchdog.close()); + await watchdog.track(childPid); + + assert.equal(isAlive(childPid), true, 'tracked child should be alive before the parent dies'); + + owner.kill('SIGKILL'); + await once(owner, 'exit'); + + assert.equal( + await waitForDeath(childPid), + true, + `tracked child ${childPid} survived its parent being SIGKILLed — the liveness poll is not reaping`, + ); +}); + +test('the watchdog refuses to kill a tracked pid the OS says is not its parent’s child', async (t) => { + // The ownership guard, and the reason the poll test above needs a real + // parent/child pair. A pid can be tracked in error, or reused by an unrelated + // process between track and reap; killing on the tracked pid alone would then + // take down something that was never ours. The watchdog only vetoes on a + // snapshot that POSITIVELY identifies the pid as parented elsewhere — a missing + // or unavailable snapshot must still reap, which is the leak fixed separately. + if (process.platform !== 'win32') { + t.skip('Parentage is established from the Win32_Process snapshot; POSIX uses the start fingerprint'); + return; + } + + const owner = spawnFixture('child-owner.mjs'); + t.after(() => killIfAlive(owner)); + const ownerStderr = collectStderr(owner); + await firstLine(owner, ownerStderr); + + // Parented by the TEST, never by `owner` — exactly the state the guard exists + // to refuse. + const bystander = spawnDisposable(); + t.after(() => killIfAlive(bystander)); + await once(bystander, 'spawn'); + + const watchdog = await OwnedChildWatchdog.start({ + scriptPath: watchdogScript, + logger: quietLogger, + parentPid: owner.pid, + }); + t.after(() => watchdog.close()); + await watchdog.track(bystander.pid); + + owner.kill('SIGKILL'); + await once(owner, 'exit'); + + // Give the reap the same budget the positive cases get, then assert the + // bystander is STILL alive. + await delay(20_000); + assert.equal( + isAlive(bystander.pid), + true, + `the watchdog killed pid ${bystander.pid}, which the OS reports as parented by this test rather than by ` + + 'the process it was guarding — the ownership guard is not holding', + ); +}); + +test('a SIGKILLed main process does not orphan its sidecar grandchild', async (t) => { + // End-to-end shape of the real failure, using the sidecar manager's OWN spawn + // options (`detached: false` on win32). Nothing in the killed process gets to + // run — no before-quit, no stopAll, no close. + // + // What this proves is the user-facing guarantee — a hard-killed app leaves no + // sidecar holding :8100 — NOT that the watchdog delivered it. On Windows libuv + // puts non-detached children in a KILL_ON_JOB_CLOSE job, so the OS reaps them + // first; confirmed by mutation, this test still passes with the liveness poll + // disabled. The watchdog itself is covered by the detached-child test above. + // Both matter: the job object is what actually protects Windows users today, + // and it is an undocumented libuv side effect that this test pins in place. + const parent = spawnFixture('crash-parent.mjs'); + t.after(() => killIfAlive(parent)); + const stderr = collectStderr(parent); + + const { grandchildPid } = await firstLine(parent, stderr); + assert.equal(isAlive(grandchildPid), true, 'grandchild should be alive before the crash'); + + parent.kill('SIGKILL'); + await once(parent, 'exit'); + + assert.equal( + await waitForDeath(grandchildPid), + true, + `grandchild ${grandchildPid} outlived a SIGKILLed parent — this is the orphaned-sidecar defect the ` + + 'watchdog replaced the Windows Job Object to prevent', + ); +}); diff --git a/electron/tests/fixtures/child-owner.mjs b/electron/tests/fixtures/child-owner.mjs new file mode 100644 index 0000000..d3ecb64 --- /dev/null +++ b/electron/tests/fixtures/child-owner.mjs @@ -0,0 +1,32 @@ +// Fixture for the liveness-poll crash test. Stands in for the Electron main +// process ONLY as the owner of a sidecar: it spawns a long-lived child, reports +// the pid, and idles. It deliberately does NOT start a watchdog — the test owns +// that, so the watchdog's stdin stays open when this process is killed and the +// reap can only come from the liveness poll rather than the stdin-close path. +// +// The child must genuinely be a child of THIS process: the watchdog refuses to +// kill a tracked pid the OS reports as parented elsewhere. +// +// `detached: true` on EVERY platform, deliberately diverging from how the sidecar +// manager spawns (`detached: false` on win32). Non-detached Windows children are +// reaped by libuv's own job object the moment this process dies, which would make +// the test pass whether or not the watchdog did anything — verified by mutation: +// disabling the liveness poll left a non-detached child dying anyway. Detaching +// puts the child outside that job so the watchdog is the only thing that can kill +// it, which is the whole point of the test. +// +// Not named *.test.mjs so the `electron/tests/**/*.test.mjs` runner ignores it. +import { spawn } from 'node:child_process'; +import process from 'node:process'; + +const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + detached: true, + stdio: 'ignore', + windowsHide: true, + env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' }, +}); +child.unref(); + +process.stdout.write(`${JSON.stringify({ childPid: child.pid })}\n`); + +setInterval(() => {}, 1000); diff --git a/electron/tests/fixtures/crash-parent.mjs b/electron/tests/fixtures/crash-parent.mjs new file mode 100644 index 0000000..4c736b1 --- /dev/null +++ b/electron/tests/fixtures/crash-parent.mjs @@ -0,0 +1,33 @@ +// Fixture for the end-to-end crash-path test. Plays the role of the Electron main +// process: starts the owned-child watchdog, spawns a long-lived grandchild, tracks +// it, then idles forever. The test SIGKILLs this process — no graceful close, no +// stopAll — and asserts the grandchild still dies. +// +// Not named *.test.mjs so the `electron/tests/**/*.test.mjs` runner ignores it. +import { spawn } from 'node:child_process'; +import path from 'node:path'; +import process from 'node:process'; +import { fileURLToPath } from 'node:url'; +import { OwnedChildWatchdog } from '../../watchdog.js'; + +const quietLogger = { info() {}, warn() {}, error() {}, crash() {} }; +const scriptPath = path.join(fileURLToPath(new URL('../../', import.meta.url)), 'child-watchdog.cjs'); + +const watchdog = await OwnedChildWatchdog.start({ scriptPath, logger: quietLogger }); + +// A bare interval keeps the grandchild alive with no exit path of its own, so if +// it dies it can only be because something reaped it. +const grandchild = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + detached: process.platform !== 'win32', + stdio: 'ignore', + windowsHide: true, + env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' }, +}); + +await watchdog.track(grandchild.pid); + +// The test waits on this line before killing us, so tracking is guaranteed to have +// completed first — otherwise the reap could be a false pass from a race. +process.stdout.write(`${JSON.stringify({ grandchildPid: grandchild.pid })}\n`); + +setInterval(() => {}, 1000); diff --git a/electron/tests/runtime.test.mjs b/electron/tests/runtime.test.mjs index 6605eab..66ce9c2 100644 --- a/electron/tests/runtime.test.mjs +++ b/electron/tests/runtime.test.mjs @@ -701,6 +701,79 @@ test('relocation guard adopts a legacy LSAT bank only while the new store is emp assert.equal(resolveLsatDataDir({ userDataPath, platform, env }).dataDir, newDir); }); +// The guard resolves the legacy directory from `platform` + `env`, so all three +// platform branches are testable from any host. Without this the macOS and Linux +// branches ship entirely unverified: the electron CI job runs windows-latest only, +// so a wrong path there would strand every Mac and Linux user's question bank and +// nothing would catch it. A fake statFile keeps the cases pure path resolution. +test('relocation guard resolves the legacy bank location on every platform', () => { + const userDataPath = path.join(path.sep, 'app-data', 'StudyVault'); + const cases = [ + { platform: 'win32', env: { APPDATA: path.join('C:', 'Users', 'x', 'AppData', 'Roaming') } }, + { platform: 'darwin', env: { HOME: path.join(path.sep, 'Users', 'x') } }, + { platform: 'linux', env: { XDG_DATA_HOME: path.join(path.sep, 'home', 'x', '.local', 'share') } }, + // Linux falls back to ~/.local/share when XDG_DATA_HOME is unset. + { platform: 'linux', env: { HOME: path.join(path.sep, 'home', 'x') } }, + // macOS keeps its own layout rather than borrowing the XDG one. + { platform: 'darwin', env: { HOME: path.join(path.sep, 'Users', 'x'), XDG_DATA_HOME: path.join(path.sep, 'ignored') } }, + ]; + + for (const { platform, env } of cases) { + const legacyDir = legacyLsatDataDir({ platform, env }); + assert.ok(legacyDir, `${platform} must resolve a legacy dir from ${JSON.stringify(env)}`); + assert.equal(path.basename(legacyDir), 'LSATLab', `${platform} legacy dir must be the LSATLab leaf`); + + // A populated legacy store with an empty current one is the adopt case, and it + // must hold identically on every platform. + const statFile = (target) => + target === lsatStorePath(legacyDir) ? { isFile: () => true, size: 4096 } : { isFile: () => true, size: 0 }; + const decision = resolveLsatDataDir({ userDataPath, platform, env, statFile }); + assert.equal(decision.dataDir, legacyDir, `${platform} should adopt the populated legacy bank`); + assert.equal(decision.relocated, true); + } + + assert.equal(legacyLsatDataDir({ platform: 'darwin', env: { HOME: path.join(path.sep, 'Users', 'x') } }).includes('Application Support'), true); +}); + +test('relocation guard never relocates onto itself and survives an unresolvable environment', () => { + // The same-path branch guards against "adopting" a directory that is already the + // active one, which would log a migration that never happened. It is unreachable + // while the leaves differ (`lsat-backend` vs `LSATLab`), and that is exactly what + // is worth pinning: if someone renames either constant into a collision, the + // guard would start relocating a directory onto itself and this fails first. + const populated = { isFile: () => true, size: 4096 }; + for (const platform of ['win32', 'darwin', 'linux']) { + const env = legacyAppDataEnv(path.join(path.sep, 'same-path-root'), platform); + const legacyDir = legacyLsatDataDir({ platform, env }); + const decision = resolveLsatDataDir({ + userDataPath: path.dirname(legacyDir), + platform, + env, + statFile: () => populated, + }); + assert.equal(decision.samePath, false, `${platform}: legacy and current dirs must not collide`); + assert.notEqual(decision.currentDir, decision.legacyDir); + // Both stores read as populated, so the current one must win outright. + assert.equal(decision.relocated, false, `${platform}: a populated current store is never displaced`); + assert.equal(decision.dataDir, decision.currentDir); + } + + // Unresolvable environment: no APPDATA, no HOME, no XDG_DATA_HOME. The guard runs + // during boot before any window exists, so throwing here would be an unrecoverable + // startup crash rather than a degraded sidecar. + for (const unresolvable of [{ platform: 'win32', env: {} }, { platform: 'darwin', env: {} }, { platform: 'linux', env: {} }]) { + assert.equal(legacyLsatDataDir(unresolvable), null); + const decision = resolveLsatDataDir({ + userDataPath: path.join(path.sep, 'app-data', 'StudyVault'), + ...unresolvable, + statFile: () => populated, + }); + assert.equal(decision.legacyDir, null); + assert.equal(decision.relocated, false, 'no legacy base means nothing to adopt'); + assert.equal(decision.dataDir, decision.currentDir); + } +}); + test('LSAT authorization predicate is exact and redirect requests do not receive the token', () => { assert.equal(isExactLsatApiUrl('http://127.0.0.1:8100/api/questions?q=1'), true); assert.equal(isExactLsatApiUrl('http://localhost:8100/api/questions'), false);