From c1003074cda56e88e2314124c0e3e11f65a90f2a Mon Sep 17 00:00:00 2001 From: Sucukdeluxe <259325684+Sucukdeluxe@users.noreply.github.com> Date: Thu, 13 Aug 2026 21:34:30 +0200 Subject: [PATCH] Make history and remote message handling fail closed --- lib/config-store.js | 38 ++++++++++++++++++++++++++++--------- lib/remote-server.js | 11 +++++++++++ tests/config-store.test.js | 33 ++++++++++++++++++++++++++++++++ tests/remote-server.test.js | 24 +++++++++++++++++++++++ 4 files changed, 97 insertions(+), 9 deletions(-) diff --git a/lib/config-store.js b/lib/config-store.js index 67593ee..d17e17c 100644 --- a/lib/config-store.js +++ b/lib/config-store.js @@ -236,15 +236,35 @@ class ConfigStore { fs.renameSync(tmp, this.historyPath); } - _writeHistoryFileAtomic(arr) { - return new Promise((resolve, reject) => { - const tmp = this.historyPath + '.tmp'; - fs.writeFile(tmp, JSON.stringify(arr), 'utf-8', (err) => { - if (err) return reject(err); - try { fs.renameSync(tmp, this.historyPath); } catch (e) { return reject(e); } - resolve(); - }); - }); + async _writeHistoryFileAtomic(arr) { + const tmp = this.historyPath + '.tmp'; + let handle; + let operationError; + try { + handle = await fs.promises.open(tmp, 'w'); + await handle.writeFile(JSON.stringify(arr), 'utf-8'); + await handle.sync(); + } catch (error) { + operationError = error; + } + if (handle) { + try { + await handle.close(); + } catch (error) { + if (!operationError) operationError = error; + } + } + if (operationError) throw operationError; + await fs.promises.rename(tmp, this.historyPath); + let directoryHandle; + try { + directoryHandle = await fs.promises.open(path.dirname(this.historyPath), 'r'); + await directoryHandle.sync(); + } catch (error) { + if (!['EINVAL', 'EISDIR', 'EPERM', 'ENOTSUP'].includes(error.code)) throw error; + } finally { + if (directoryHandle) await directoryHandle.close(); + } } _quiescedWriteError() { diff --git a/lib/remote-server.js b/lib/remote-server.js index f52bb63..0cd3deb 100644 --- a/lib/remote-server.js +++ b/lib/remote-server.js @@ -98,6 +98,17 @@ class RemoteServer { const client = this._clients.get(ws); if (!client) return; + if (!msg || typeof msg !== 'object' || Array.isArray(msg)) { + if (!client.authenticated) { + authReceived = true; + clearTimeout(authTimeout); + this._recordFailedAttempt(ip); + ws.close(4002, 'Invalid token'); + this._clients.delete(ws); + } + return; + } + if (!client.authenticated) { authReceived = true; clearTimeout(authTimeout); diff --git a/tests/config-store.test.js b/tests/config-store.test.js index 07bb5b4..6462839 100644 --- a/tests/config-store.test.js +++ b/tests/config-store.test.js @@ -261,6 +261,39 @@ describe('ConfigStore', () => { assert.equal(history[104].id, 'batch-104'); }); + it('durably syncs migrated history before replacing the live file', async () => { + store._historyMigrated = true; + fs.writeFileSync(store.historyPath, '[]', 'utf-8'); + const originalOpen = fs.promises.open; + const originalRename = fs.promises.rename; + let synced = false; + let renamed = false; + fs.promises.open = async (...args) => { + const handle = await originalOpen(...args); + const originalSync = handle.sync.bind(handle); + handle.sync = async () => { + await originalSync(); + synced = true; + }; + return handle; + }; + fs.promises.rename = async (...args) => { + assert.equal(synced, true); + renamed = true; + return originalRename(...args); + }; + + try { + await store.appendHistory({ id: 'durable', files: [] }); + } finally { + fs.promises.open = originalOpen; + fs.promises.rename = originalRename; + } + + assert.equal(renamed, true); + assert.deepEqual(store.loadHistory().map(entry => entry.id), ['durable']); + }); + it('clearHistory empties the array', async () => { await store.appendHistory({ id: 'test', files: [] }); assert.equal(store.loadHistory().length, 1); diff --git a/tests/remote-server.test.js b/tests/remote-server.test.js index 7056325..90f1bce 100644 --- a/tests/remote-server.test.js +++ b/tests/remote-server.test.js @@ -67,4 +67,28 @@ describe('RemoteServer', () => { assert.notStrictEqual(end, -1); assert.match(source.slice(start, end), /host:\s*'127\.0\.0\.1'/); }); + + it('rejects non-object JSON before authentication without throwing', () => { + const { EventEmitter } = require('node:events'); + const RemoteServer = require('../lib/remote-server'); + const server = new RemoteServer(); + const socket = new EventEmitter(); + socket.close = (code) => { + socket.closeCode = code; + }; + socket.send = () => {}; + server._config = { + token: 'test-token-123', + allowlist: [], + diagnosticMode: false, + onCreateCaptureWindow: () => {}, + onDestroyCaptureWindow: () => {}, + onSignalingToCapture: () => {} + }; + + server._handleConnection(socket, { socket: { remoteAddress: '127.0.0.1' } }); + assert.doesNotThrow(() => socket.emit('message', Buffer.from('null'))); + assert.strictEqual(socket.closeCode, 4002); + socket.emit('close'); + }); });