From 4a10e6051e5766f27be5c69c31f12d2201d1ff1d Mon Sep 17 00:00:00 2001 From: PageZ948 Date: Wed, 19 Aug 2026 23:12:56 +0000 Subject: [PATCH] fix: confirm proxy exit before disconnect --- main.js | 81 +++++++++++++++++++++---- src/electron-runtime.js | 14 +++-- src/proxy-launch.js | 109 +++++++++++++++++++++++++++------- test/proxy-launch.test.js | 92 ++++++++++++++++++++++++---- test/runtime-contract.test.js | 64 +++++++++++++++++++- 5 files changed, 308 insertions(+), 52 deletions(-) diff --git a/main.js b/main.js index 8a08077..65b3b9c 100644 --- a/main.js +++ b/main.js @@ -1,6 +1,6 @@ 'use strict'; -const { app, BrowserWindow, ipcMain, safeStorage, shell } = require('electron'); +const { app, BrowserWindow, dialog, ipcMain, safeStorage, shell } = require('electron'); const http = require('node:http'); const path = require('node:path'); const { pathToFileURL } = require('node:url'); @@ -23,6 +23,8 @@ let bridgeServer = null; let runtime = null; let pairingController = null; let firstRunPairingSecret = null; +let quitCleanupInProgress = false; +let allowConfirmedQuit = false; function getAppDirectory() { return app.isPackaged @@ -83,8 +85,8 @@ function registerIpcHandlers() { sendConnectionState(); return result; }); - registerIpc('disconnect', () => { - const state = runtime.disconnect(); + registerIpc('disconnect', async () => { + const state = await runtime.disconnect(); sendConnectionState(); return state; }); @@ -157,17 +159,72 @@ app.whenReady().then(() => { startBridgeServer(); }); +function closeBridgeServer() { + if (!bridgeServer) return Promise.resolve(); + const server = bridgeServer; + return new Promise((resolve, reject) => { + const finish = (error) => { + if (!error || error.code === 'ERR_SERVER_NOT_RUNNING') { + if (bridgeServer === server) bridgeServer = null; + resolve(); + return; + } + reject(error); + }; + try { server.close(finish); } catch (error) { finish(error); } + }); +} + +async function notifyQuitBlocked(state) { + sendConnectionState(); + const activeCount = state && Array.isArray(state.activeProxies) ? state.activeProxies.length : 0; + const activeProxiesRemain = activeCount > 0; + const processDetail = activeProxiesRemain + ? `${activeCount} app-owned proxy process${activeCount === 1 ? '' : 'es'} remain active.` + : 'No app-owned proxy process remains active.'; + const sessionDetail = state && state.connected + ? ' The Alta session is still connected; resolve the process error and try quitting again.' + : ' Resolve the shutdown error and try quitting again.'; + await dialog.showMessageBox({ + type: 'error', + title: activeProxiesRemain ? 'Alta Camera Proxy is still running' : 'Alta Camera Proxy could not close', + message: activeProxiesRemain + ? 'The app could not confirm that every proxy process exited.' + : 'The app could not complete shutdown safely.', + detail: `${processDetail}${sessionDetail}`, + buttons: ['OK'], + noLink: true, + }); +} + app.on('before-quit', (event) => { - if (runtime) { - const state = runtime.disconnect(); - if (state.activeProxies.length > 0) { - event.preventDefault(); - sendConnectionState(); - return; + if (allowConfirmedQuit) return; + event.preventDefault(); + if (quitCleanupInProgress) return; + quitCleanupInProgress = true; + + (async () => { + try { + const state = runtime + ? await runtime.disconnect() + : { success: true, connected: false, activeProxies: [] }; + if (!state.success || state.activeProxies.length > 0) { + await notifyQuitBlocked(state); + return; + } + await closeBridgeServer(); + if (runtime) runtime.sessionStore.dispose(); + allowConfirmedQuit = true; + app.quit(); + } catch { + const state = runtime + ? runtime.getConnectionState() + : { connected: false, activeProxies: [] }; + await notifyQuitBlocked(state); + } finally { + quitCleanupInProgress = false; } - runtime.sessionStore.dispose(); - } - if (bridgeServer) bridgeServer.close(); + })(); }); app.on('window-all-closed', () => { diff --git a/src/electron-runtime.js b/src/electron-runtime.js index cfdf200..77efd1e 100644 --- a/src/electron-runtime.js +++ b/src/electron-runtime.js @@ -292,21 +292,23 @@ class AppRuntime { } if (!processId || !deviceId) return { success: false, message: 'Proxy process is not owned by this app.' }; - const result = this.proxyManager.stopProxy(processId); + const result = await this.proxyManager.stopProxy(processId); if (result.success) this.proxyByDevice.delete(deviceId); return { ...result, deviceId }; } - disconnect() { + async disconnect() { const tracked = this._reconcileProxies(); - for (const proxy of tracked) { - try { this.proxyManager.stopProxy(proxy.processId); } catch {} - } + await Promise.all(tracked.map(async (proxy) => { + try { + await this.proxyManager.stopProxy(proxy.processId); + } catch {} + })); const remaining = this._reconcileProxies(); if (remaining.length > 0) { return { success: false, - message: 'Could not stop every active proxy. The Alta session remains connected.', + message: 'Could not confirm every active proxy exited. The Alta session remains connected.', ...this.getConnectionState(), }; } diff --git a/src/proxy-launch.js b/src/proxy-launch.js index 5ddfa8f..58dffb2 100644 --- a/src/proxy-launch.js +++ b/src/proxy-launch.js @@ -10,6 +10,7 @@ const MAX_HOST_LENGTH = 253; const DEVICE_ID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; const DNS_LABEL_PATTERN = /^[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?$/; const ALTA_SUFFIXES = ['.avasecurity.com', '.avigilon.com']; +const DEFAULT_STOP_TIMEOUT_MS = 5_000; class ProxyLaunchError extends Error { constructor(message, code) { @@ -70,7 +71,10 @@ function createProxyManager({ fs = nodeFs, spawn = nodeSpawn, platform = process.platform, - now = Date.now + now = Date.now, + stopTimeoutMs = DEFAULT_STOP_TIMEOUT_MS, + setTimeout = global.setTimeout, + clearTimeout = global.clearTimeout } = {}) { if (platform !== 'win32') { throw new ProxyLaunchError('Proxy helper is supported only on the Windows platform.', 'UNSUPPORTED_PLATFORM'); @@ -78,7 +82,9 @@ function createProxyManager({ if (typeof appDirectory !== 'string' || !nodePath.win32.isAbsolute(appDirectory)) { throw new ProxyLaunchError('Approved application directory must be an absolute path.', 'INVALID_APP_DIRECTORY'); } - if (typeof fs.existsSync !== 'function' || typeof spawn !== 'function' || typeof now !== 'function') { + if (typeof fs.existsSync !== 'function' || typeof spawn !== 'function' || typeof now !== 'function' || + !Number.isSafeInteger(stopTimeoutMs) || stopTimeoutMs <= 0 || + typeof setTimeout !== 'function' || typeof clearTimeout !== 'function') { throw new TypeError('Invalid proxy manager dependency.'); } @@ -90,6 +96,32 @@ function createProxyManager({ if (current && current.child === child) trackedChildren.delete(processId); } + function hasConfirmedExit(child) { + return (child.exitCode !== null && child.exitCode !== undefined) || + (child.signalCode !== null && child.signalCode !== undefined); + } + + function finishStopAttempt(entry, result) { + const attempt = entry.stopAttempt; + if (!attempt) return; + entry.stopAttempt = null; + entry.stopPromise = null; + clearTimeout(attempt.timer); + attempt.resolve(result); + } + + function confirmExit(entry) { + const current = trackedChildren.get(entry.processId); + if (!current || current.child !== entry.child) return; + trackedChildren.delete(entry.processId); + finishStopAttempt(entry, { + success: true, + processId: entry.processId, + deviceId: entry.deviceId, + status: 'stopped' + }); + } + function launchProxy(request) { if (!request || typeof request !== 'object' || Array.isArray(request)) { throw new ProxyLaunchError('Proxy launch request is invalid.', 'INVALID_REQUEST'); @@ -139,19 +171,24 @@ function createProxyManager({ processId: child.pid, deviceId, startedAt: now(), - status: 'running' + status: 'running', + stopAttempt: null, + stopPromise: null }; trackedChildren.set(entry.processId, entry); if (typeof child.once === 'function') { - child.once('exit', () => removeIfOwned(entry.processId, child)); - child.once('error', () => removeIfOwned(entry.processId, child)); + child.once('exit', () => confirmExit(entry)); + child.once('close', () => confirmExit(entry)); + child.once('error', () => { + if (hasConfirmedExit(child)) confirmExit(entry); + }); } return { success: true, ...safeMetadata(entry) }; } - function stopProxy(processId) { + async function stopProxy(processId) { if (!Number.isSafeInteger(processId) || processId <= 0) { throw new ProxyLaunchError('Process identifier is invalid.', 'INVALID_PROCESS_ID'); } @@ -159,28 +196,56 @@ function createProxyManager({ const entry = trackedChildren.get(processId); if (!entry) return { success: false, processId, status: 'not-tracked' }; - try { - const requested = entry.child.kill('SIGTERM'); - if (!requested) { - removeIfOwned(processId, entry.child); - return { - success: true, - processId, - deviceId: entry.deviceId, - status: 'already-exited' - }; - } - + if (hasConfirmedExit(entry.child)) { removeIfOwned(processId, entry.child); return { success: true, processId, deviceId: entry.deviceId, - status: 'stop-requested' + status: 'already-exited' }; + } + if (entry.stopPromise) return entry.stopPromise; + + entry.status = 'stopping'; + entry.stopPromise = new Promise((resolve) => { + const timer = setTimeout(() => { + finishStopAttempt(entry, { + success: false, + processId, + deviceId: entry.deviceId, + status: 'stop-timeout', + message: 'Timed out waiting for the tracked proxy process to exit.' + }); + }, stopTimeoutMs); + entry.stopAttempt = { resolve, timer }; + }); + const stopPromise = entry.stopPromise; + + try { + const requested = entry.child.kill('SIGTERM'); + if (!requested) { + if (hasConfirmedExit(entry.child)) { + removeIfOwned(processId, entry.child); + finishStopAttempt(entry, { + success: true, + processId, + deviceId: entry.deviceId, + status: 'already-exited' + }); + } else { + finishStopAttempt(entry, { + success: false, + processId, + deviceId: entry.deviceId, + status: 'stop-failed', + message: 'Unable to confirm that the tracked proxy process exited.' + }); + } + } } catch (error) { const permissionDenied = error && (error.code === 'EPERM' || error.code === 'EACCES'); - return { + finishStopAttempt(entry, { success: false, processId, deviceId: entry.deviceId, @@ -188,8 +253,9 @@ function createProxyManager({ message: permissionDenied ? 'Unable to stop the tracked proxy process: permission denied.' : 'Unable to stop the tracked proxy process.' - }; + }); } + return stopPromise; } function listTrackedProxies() { @@ -200,6 +266,7 @@ function createProxyManager({ } module.exports = { + DEFAULT_STOP_TIMEOUT_MS, HELPER_FILENAME, MAX_USERNAME_LENGTH, ProxyLaunchError, diff --git a/test/proxy-launch.test.js b/test/proxy-launch.test.js index 5a33273..10c4e64 100644 --- a/test/proxy-launch.test.js +++ b/test/proxy-launch.test.js @@ -24,6 +24,8 @@ class FakeChild extends EventEmitter { this.killResult = killResult; this.killCalls = []; this.killError = null; + this.exitCode = null; + this.signalCode = null; } kill(signal) { @@ -33,7 +35,13 @@ class FakeChild extends EventEmitter { } } -function createHarness({ helperExists = true, children = [new FakeChild(4101)] } = {}) { +function createHarness({ + helperExists = true, + children = [new FakeChild(4101)], + stopTimeoutMs = 5_000, + setTimeout = global.setTimeout, + clearTimeout = global.clearTimeout, +} = {}) { const calls = []; let childIndex = 0; const spawn = (...args) => { @@ -47,7 +55,10 @@ function createHarness({ helperExists = true, children = [new FakeChild(4101)] } fs: fsStub, spawn, platform: 'win32', - now: () => 1_777_777_777_777 + now: () => 1_777_777_777_777, + stopTimeoutMs, + setTimeout, + clearTimeout, }); return { manager, calls, children }; } @@ -239,32 +250,38 @@ test('tracks only safe process metadata and never exposes usernames', () => { assert.doesNotMatch(JSON.stringify(tracked), /proxy\.operator/); }); -test('stopping one tracked process leaves the other tracked process alive', () => { +test('stopping one tracked process waits for confirmed exit and leaves the other alive', async () => { const first = new FakeChild(4101); const second = new FakeChild(4102); const { manager } = createHarness({ children: [first, second] }); manager.launchProxy(validRequest()); manager.launchProxy(validRequest({ deviceId: '123e4567-e89b-42d3-a456-426614174001' })); - const result = manager.stopProxy(4101); + const stopping = manager.stopProxy(4101); + assert.deepEqual(manager.listTrackedProxies().map((item) => [item.processId, item.status]), [ + [4101, 'stopping'], + [4102, 'running'], + ]); + first.emit('exit', 0, 'SIGTERM'); + const result = await stopping; assert.deepEqual(result, { success: true, processId: 4101, deviceId: VALID_DEVICE_ID, - status: 'stop-requested' + status: 'stopped' }); assert.deepEqual(first.killCalls, ['SIGTERM']); assert.deepEqual(second.killCalls, []); assert.deepEqual(manager.listTrackedProxies().map((item) => item.processId), [4102]); }); -test('never stops an untracked PID', () => { +test('never stops an untracked PID', async () => { const child = new FakeChild(4101); const { manager } = createHarness({ children: [child] }); manager.launchProxy(validRequest()); - assert.deepEqual(manager.stopProxy(9999), { + assert.deepEqual(await manager.stopProxy(9999), { success: false, processId: 9999, status: 'not-tracked' @@ -272,21 +289,22 @@ test('never stops an untracked PID', () => { assert.deepEqual(child.killCalls, []); }); -test('reports already-exited and permission-denied states honestly', () => { +test('reports already-exited and permission-denied states honestly', async () => { const exited = new FakeChild(4101, false); + exited.exitCode = 0; const denied = new FakeChild(4102); denied.killError = Object.assign(new Error('operation not permitted'), { code: 'EPERM' }); const { manager } = createHarness({ children: [exited, denied] }); manager.launchProxy(validRequest()); manager.launchProxy(validRequest({ deviceId: '123e4567-e89b-42d3-a456-426614174001' })); - assert.deepEqual(manager.stopProxy(4101), { + assert.deepEqual(await manager.stopProxy(4101), { success: true, processId: 4101, deviceId: VALID_DEVICE_ID, status: 'already-exited' }); - assert.deepEqual(manager.stopProxy(4102), { + assert.deepEqual(await manager.stopProxy(4102), { success: false, processId: 4102, deviceId: '123e4567-e89b-42d3-a456-426614174001', @@ -296,6 +314,60 @@ test('reports already-exited and permission-denied states honestly', () => { assert.deepEqual(manager.listTrackedProxies().map((item) => item.processId), [4102]); }); +test('kill true is only a request and times out while the child remains tracked as stopping', async () => { + let timeoutCallback; + const child = new FakeChild(4101, true); + const { manager } = createHarness({ + children: [child], + stopTimeoutMs: 25, + setTimeout(callback, delay) { + assert.equal(delay, 25); + timeoutCallback = callback; + return 123; + }, + clearTimeout() {}, + }); + manager.launchProxy(validRequest()); + + const stopping = manager.stopProxy(4101); + assert.deepEqual(manager.listTrackedProxies().map(({ processId, status }) => ({ processId, status })), [ + { processId: 4101, status: 'stopping' }, + ]); + timeoutCallback(); + + assert.deepEqual(await stopping, { + success: false, + processId: 4101, + deviceId: VALID_DEVICE_ID, + status: 'stop-timeout', + message: 'Timed out waiting for the tracked proxy process to exit.', + }); + assert.equal(manager.listTrackedProxies()[0].status, 'stopping'); +}); + +test('kill false does not claim success without confirmed exit', async () => { + const child = new FakeChild(4101, false); + const { manager } = createHarness({ children: [child] }); + manager.launchProxy(validRequest()); + + assert.deepEqual(await manager.stopProxy(4101), { + success: false, + processId: 4101, + deviceId: VALID_DEVICE_ID, + status: 'stop-failed', + message: 'Unable to confirm that the tracked proxy process exited.', + }); + assert.equal(manager.listTrackedProxies()[0].status, 'stopping'); +}); + +test('generic child errors retain tracking unless process exit is confirmed', () => { + const child = new FakeChild(4101); + const { manager } = createHarness({ children: [child] }); + manager.launchProxy(validRequest()); + child.emit('error', new Error('transient process error')); + assert.deepEqual(manager.listTrackedProxies().map((entry) => entry.processId), [4101]); +}); + test('exit events remove only the matching owned child', () => { const original = new FakeChild(4101); const replacement = new FakeChild(4101); diff --git a/test/runtime-contract.test.js b/test/runtime-contract.test.js index 5d4d074..596befc 100644 --- a/test/runtime-contract.test.js +++ b/test/runtime-contract.test.js @@ -275,13 +275,13 @@ test('disconnect stops every owned proxy before clearing the Alta session', asyn }, }, }); - const state = runtime.disconnect(); + const state = await runtime.disconnect(); assert.deepEqual(stopped, [4101, 4102]); assert.equal(state.connected, false); assert.deepEqual(state.activeProxies, []); }); -test('disconnect reports failure truthfully and retains session when an owned proxy cannot stop', () => { +test('disconnect reports failure truthfully and retains session when an owned proxy cannot stop', async () => { const sessionStore = createSessionStore(); sessionStore.establish('https://customer.avasecurity.com', 'synthetic-cookie'); const tracked = [{ processId: 4101, deviceId: '550e8400-e29b-41d4-a716-446655440000', status: 'running', startedAt: 1 }]; @@ -293,12 +293,52 @@ test('disconnect reports failure truthfully and retains session when an owned pr stopProxy() { return { success: false, processId: 4101, status: 'permission-denied' }; }, }, }); - const state = runtime.disconnect(); + const state = await runtime.disconnect(); assert.equal(state.success, false); assert.equal(state.connected, true); assert.deepEqual(state.activeProxies.map((entry) => entry.processId), [4101]); }); +test('kill request without exit keeps proxy tracked and session connected until confirmed exit', async () => { + const { EventEmitter } = require('node:events'); + const { createProxyManager } = require('../src/proxy-launch'); + const sessionStore = createSessionStore(); + sessionStore.establish('https://customer.avasecurity.com', 'synthetic-cookie'); + const deviceId = '550e8400-e29b-41d4-a716-446655440000'; + const child = new EventEmitter(); + child.pid = 4101; + child.exitCode = null; + child.signalCode = null; + child.kill = () => true; + let timeoutCallback; + const proxyManager = createProxyManager({ + appDirectory: 'C:\\Program Files\\Alta Proxy Tool', + fs: { existsSync: () => true }, + spawn: () => child, + platform: 'win32', + stopTimeoutMs: 10, + setTimeout(callback) { timeoutCallback = callback; return 1; }, + clearTimeout() {}, + }); + proxyManager.launchProxy({ deploymentHost: 'customer.avasecurity.com', username: 'operator@example.com', deviceId }); + const runtime = new AppRuntime({ sessionStore, altaClient: {}, proxyManager }); + + const firstDisconnect = runtime.disconnect(); + assert.equal(proxyManager.listTrackedProxies()[0].status, 'stopping'); + timeoutCallback(); + const failed = await firstDisconnect; + assert.equal(failed.success, false); + assert.equal(failed.connected, true); + assert.deepEqual(failed.activeProxies, [{ deviceId, processId: 4101 }]); + + child.exitCode = 0; + child.emit('exit', 0, null); + const disconnected = await runtime.disconnect(); + assert.equal(disconnected.success, true); + assert.equal(disconnected.connected, false); + assert.deepEqual(disconnected.activeProxies, []); +}); + test('update runtime is check-only and opens only the fixed GitPeji releases page', async () => { const opened = []; const runtime = new AppRuntime({ @@ -340,6 +380,24 @@ test('preload and renderer expose only narrow, credential-free contracts', () => assert.match(html, /Bridge Pairing/); }); +test('Electron quit waits for confirmed cleanup and reports a safe failure instead of disposing', () => { + const main = read('main.js'); + const beforeQuit = main.slice(main.indexOf("app.on('before-quit'"), main.indexOf("app.on('window-all-closed'")); + const preventIndex = beforeQuit.indexOf('event.preventDefault()'); + const disconnectIndex = beforeQuit.indexOf('await runtime.disconnect()'); + const activeCheckIndex = beforeQuit.indexOf('state.activeProxies.length > 0'); + const disposeIndex = beforeQuit.indexOf('runtime.sessionStore.dispose()'); + const confirmedQuitIndex = beforeQuit.indexOf('allowConfirmedQuit = true'); + const quitIndex = beforeQuit.indexOf('app.quit()'); + + assert.ok(preventIndex >= 0 && preventIndex < disconnectIndex); + assert.ok(disconnectIndex < activeCheckIndex && activeCheckIndex < disposeIndex); + assert.ok(disposeIndex < confirmedQuitIndex && confirmedQuitIndex < quitIndex); + assert.match(main, /dialog\.showMessageBox/); + assert.match(main, /could not confirm that every proxy process exited/i); + assert.doesNotMatch(beforeQuit, /taskkill|pkill|wmic/i); +}); + test('source policy removes legacy credential IPC, shell launch, broad kill, and executable updater', () => { const production = ['main.js', 'preload.js', 'renderer.js', 'index.html', 'src/electron-runtime.js'] .map(read).join('\n');