From 8effa015d3a36b5299fd45d71388f615901ba958 Mon Sep 17 00:00:00 2001 From: humble-little-bear Date: Thu, 8 Oct 2026 13:02:49 +0800 Subject: [PATCH 1/3] fix: recognize the offckb daemon when its CLI path contains spaces verifyDaemonIdentity tokenized the flat command line from 'ps -o args=' (macOS, or Linux without a readable /proc) with a quote-aware splitter. ps never quotes, so a CLI entry under a path with spaces (macOS ~/Library/Application Support/...) was cut at the first space, the script token never matched and 'offckb node stop' refused to stop a live daemon. Match the command line instead: try every space as the executable/script boundary and every later space (or the end) as the script end, accepting only a Node executable followed by exactly this installation's CLI entry (realpath-normalized, case-insensitive on Windows, quoted or not). The exact-argv path (/proc) and the start-time check are unchanged. Found during the review of #531. --- .changeset/daemon-stop-spaced-path.md | 5 ++ src/util/daemon.ts | 117 +++++++++++++++++++------- tests/daemon-command-line.test.ts | 75 +++++++++++++++++ tests/daemon-locale.test.ts | 53 ++++++++++++ 4 files changed, 219 insertions(+), 31 deletions(-) create mode 100644 .changeset/daemon-stop-spaced-path.md create mode 100644 tests/daemon-command-line.test.ts diff --git a/.changeset/daemon-stop-spaced-path.md b/.changeset/daemon-stop-spaced-path.md new file mode 100644 index 0000000..baf2ec8 --- /dev/null +++ b/.changeset/daemon-stop-spaced-path.md @@ -0,0 +1,5 @@ +--- +'@offckb/cli': patch +--- + +- daemon: identify a running offckb daemon by anchoring the node executable and CLI script path in the process command line, so `offckb node stop` works when the CLI path contains spaces (e.g. macOS `~/Library/Application Support/...`) diff --git a/src/util/daemon.ts b/src/util/daemon.ts index c398566..6b426f8 100644 --- a/src/util/daemon.ts +++ b/src/util/daemon.ts @@ -375,25 +375,14 @@ export async function getProcessCommandLine(pid: number): Promise return info.cmdline; } -// Split a flat command line into tokens, honoring single/double quotes. Used -// on platforms without /proc; the first two tokens (executable, script) are -// all the identity check consumes, and both are spawned by us without shell -// metacharacters, so a simple tokenizer suffices. -function splitCommandLine(cmdline: string): string[] { - const tokens: string[] = []; - const pattern = /"([^"]*)"|'([^']*)'|(\S+)/g; - let match: RegExpExecArray | null; - while ((match = pattern.exec(cmdline)) !== null) { - tokens.push(match[1] ?? match[2] ?? match[3]); - } - return tokens; -} - // Resolve symlinks on both sides before comparing (nvm shims, // /usr/bin/node → /etc/alternatives, symlinked install prefixes). A path -// that cannot be resolved still compares by its absolute form. -function normalizePathForCompare(candidate: string): string { - const resolved = path.resolve(candidate); +// that cannot be resolved still compares by its absolute form. Paths of +// another platform (unit tests) are only resolved lexically. +function normalizePathForCompare(candidate: string, platform: NodeJS.Platform = process.platform): string { + const pathApi = platform === 'win32' ? path.win32 : path.posix; + const resolved = pathApi.resolve(candidate); + if (platform !== process.platform) return resolved; try { return fs.realpathSync(resolved); } catch { @@ -401,11 +390,76 @@ function normalizePathForCompare(candidate: string): string { } } -function executableLooksLikeNode(executable: string): boolean { - if (normalizePathForCompare(executable) === normalizePathForCompare(process.execPath)) { +// Windows file systems are case-insensitive. +function samePath(a: string, b: string, platform: NodeJS.Platform): boolean { + return platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b; +} + +function executableLooksLikeNode(executable: string, platform: NodeJS.Platform = process.platform): boolean { + if ( + platform === process.platform && + samePath(normalizePathForCompare(executable), normalizePathForCompare(process.execPath), platform) + ) { return true; } - return NODE_EXECUTABLE_NAMES.has(path.basename(executable).toLowerCase()); + const pathApi = platform === 'win32' ? path.win32 : path.posix; + return NODE_EXECUTABLE_NAMES.has(pathApi.basename(executable).toLowerCase()); +} + +// A piece of a flat command line, optionally wrapped in one pair of quotes +// (Windows quotes arguments that contain spaces). Stray quotes inside an +// unquoted piece mean the split point is not a real argument boundary. +function unquoteCommandLinePiece(piece: string): string | null { + if ( + piece.length >= 2 && + ((piece.startsWith('"') && piece.endsWith('"')) || (piece.startsWith("'") && piece.endsWith("'"))) + ) { + return piece.slice(1, -1); + } + if (piece.length === 0 || piece.includes('"')) return null; + return piece; +} + +// Bounds the split search on a hostile or unusually long command line; real +// executable + script paths have far fewer spaces. +const MAX_COMMAND_LINE_SPLIT_POINTS = 64; + +/** + * Whether a flat command line — `ps -o args=` (argv joined by single spaces, + * never quoted) or a Windows CommandLine (arguments with spaces quoted) — + * starts with ` `, followed by the end or a + * space. + * + * The line is NOT tokenized: unquoted paths may contain spaces (macOS + * `~/Library/Application Support/...`), so splitting at the first space + * would cut the executable or the script apart. Instead every space is tried + * as the executable/script boundary and every later space (or the end) as + * the script's end. A split only matches when the executable is a Node + * runtime and the script resolves to exactly `expectedScript` — the same + * strict identity as for an exact argv, without guessing token boundaries. + */ +export function commandLineRunsScript( + cmdline: string, + expectedScript: string, + platform: NodeJS.Platform = process.platform, +): boolean { + const text = cmdline.trim(); + const expected = normalizePathForCompare(expectedScript, platform); + const spaces: number[] = []; + for (let i = 0; i < text.length && spaces.length < MAX_COMMAND_LINE_SPLIT_POINTS; i++) { + if (text[i] === ' ') spaces.push(i); + } + const scriptEnds = [...spaces, text.length]; + for (const executableEnd of spaces) { + const executable = unquoteCommandLinePiece(text.slice(0, executableEnd)); + if (executable == null || !executableLooksLikeNode(executable, platform)) continue; + for (const scriptEnd of scriptEnds) { + if (scriptEnd <= executableEnd + 1) continue; + const script = unquoteCommandLinePiece(text.slice(executableEnd + 1, scriptEnd)); + if (script != null && samePath(normalizePathForCompare(script, platform), expected, platform)) return true; + } + } + return false; } /** @@ -417,7 +471,10 @@ function executableLooksLikeNode(executable: string): boolean { * 2. its first argument must be THIS installation's CLI entry script * (resolveCliEntry of the verifying process, realpath-normalized) — * the pid file's scriptPath is never consulted, so a stale or forged - * pid file cannot lend our identity to an unrelated process; + * pid file cannot lend our identity to an unrelated process. When only + * a flat cmdline is available (ps / Windows), that is matched by + * commandLineRunsScript rather than by tokenizing, so paths that + * contain spaces still resolve; * 3. when the pid file carries a real startedAt (everything written by * current versions does), the process start time must match it, which * defeats PID reuse. If the process start time cannot be determined @@ -432,21 +489,19 @@ export async function verifyDaemonIdentity(pid: number, metadata: PidMetadata): // Without our own entry point we cannot establish identity at all. return false; } - const expectedScript = normalizePathForCompare(cliEntry); - const info = await getProcessInfo(pid); if (!info) return false; - let tokens: string[] | null = null; if (info.argv != null) { - tokens = info.argv; - } else if (info.cmdline != null) { - tokens = splitCommandLine(info.cmdline); + // Exact argv (Linux /proc): compare the first two arguments directly. + if (info.argv.length < 2) return false; + if (!executableLooksLikeNode(info.argv[0])) return false; + if (!samePath(normalizePathForCompare(info.argv[1]), normalizePathForCompare(cliEntry), process.platform)) { + return false; + } + } else if (info.cmdline == null || !commandLineRunsScript(info.cmdline, cliEntry)) { + return false; } - if (tokens == null || tokens.length < 2) return false; - - if (!executableLooksLikeNode(tokens[0])) return false; - if (normalizePathForCompare(tokens[1]) !== expectedScript) return false; const recordedMs = Date.parse(metadata.startedAt); if (Number.isFinite(recordedMs) && recordedMs > 0) { diff --git a/tests/daemon-command-line.test.ts b/tests/daemon-command-line.test.ts new file mode 100644 index 0000000..b63a4c6 --- /dev/null +++ b/tests/daemon-command-line.test.ts @@ -0,0 +1,75 @@ +import path from 'path'; +import { commandLineRunsScript } from '../src/util/daemon'; + +const POSIX_NODE = '/usr/local/bin/node'; +const WIN_NODE = 'C:\\Program Files\\nodejs\\node.exe'; + +describe('commandLineRunsScript (posix)', () => { + const script = '/Users/dev/Library/Application Support/offckb-nodejs/build/index.js'; + + it('matches an unquoted ps -o args= line whose paths contain spaces', () => { + expect(commandLineRunsScript(`${POSIX_NODE} ${script} node --daemon`, script, 'darwin')).toBe(true); + expect(commandLineRunsScript(`/usr/bin/node ${script}`, script, 'linux')).toBe(true); + expect(commandLineRunsScript(`nodejs ${script} fiber start --daemon`, script, 'linux')).toBe(true); + }); + + it('matches when the node executable itself also lives under a spaced path', () => { + const node = '/Users/dev/Library/Application Support/fnm/node'; + expect(commandLineRunsScript(`${node} ${script} node`, script, 'darwin')).toBe(true); + }); + + it('rejects a foreign script, a truncated script and a non-node host', () => { + expect(commandLineRunsScript(`${POSIX_NODE} ${script}-other node`, script, 'darwin')).toBe(false); + expect(commandLineRunsScript(`${POSIX_NODE} ${script.slice(0, -3)} node`, script, 'darwin')).toBe(false); + expect(commandLineRunsScript(`/bin/bash ${script}`, script, 'darwin')).toBe(false); + expect(commandLineRunsScript(`${POSIX_NODE}`, script, 'darwin')).toBe(false); + expect(commandLineRunsScript('', script, 'darwin')).toBe(false); + }); + + it('rejects a path that only shares a prefix with the expected script', () => { + expect( + commandLineRunsScript( + `${POSIX_NODE} /Users/dev/Library/Application Support/offckb-nodejs/build/index.js.bak node`, + script, + 'darwin', + ), + ).toBe(false); + }); +}); + +describe('commandLineRunsScript (windows)', () => { + const script = 'C:\\Users\\Jane Doe\\AppData\\Local\\offckb-nodejs\\build\\index.js'; + + it('matches a Win32_Process CommandLine that quotes arguments containing spaces', () => { + expect(commandLineRunsScript(`"${WIN_NODE}" "${script}" node --daemon`, script, 'win32')).toBe(true); + expect(commandLineRunsScript(`"${WIN_NODE}" "${script}"`, script, 'win32')).toBe(true); + }); + + it('matches when only the script needs quoting', () => { + expect(commandLineRunsScript(`C:\\nodejs\\node.exe "${script}" node`, script, 'win32')).toBe(true); + }); + + it('matches an unquoted Windows command line when neither path has spaces', () => { + const plain = 'C:\\offckb\\build\\index.js'; + expect(commandLineRunsScript(`C:\\nodejs\\node.exe ${plain} node --daemon`, plain, 'win32')).toBe(true); + }); + + it('rejects a foreign script and a non-node host, case-insensitively for the script', () => { + expect(commandLineRunsScript(`"${WIN_NODE}" "${script}.bak"`, script, 'win32')).toBe(false); + expect(commandLineRunsScript(`"C:\\Windows\\System32\\cmd.exe" "${script}"`, script, 'win32')).toBe(false); + expect( + commandLineRunsScript( + `"${WIN_NODE}" "C:\\Users\\Jane Doe\\AppData\\Local\\offckb-nodejs\\build\\INDEX.JS" node`, + script, + 'win32', + ), + ).toBe(true); + }); +}); + +describe('commandLineRunsScript (posix absolute paths without spaces still work)', () => { + it('matches a simple absolute path', () => { + const script = path.posix.join('/opt', 'offckb', 'build', 'index.js'); + expect(commandLineRunsScript(`${POSIX_NODE} ${script} node`, script, 'linux')).toBe(true); + }); +}); diff --git a/tests/daemon-locale.test.ts b/tests/daemon-locale.test.ts index 118fd30..c5d98ba 100644 --- a/tests/daemon-locale.test.ts +++ b/tests/daemon-locale.test.ts @@ -153,4 +153,57 @@ describe('daemon locale and process probing', () => { const verified = await verifyDaemonIdentity(12345, metadata); expect(verified).toBe(false); }); + + it('verifies a daemon whose CLI path contains spaces (unquoted ps -o args= output)', async () => { + // Use a temporary absolute path with a space as the CLI entry so the + // live OFFCKB_CLI_PATH matches the command line we feed the probe. + const fs = require('fs') as typeof import('fs'); + const path = require('path') as typeof import('path'); + const os = require('os') as typeof import('os'); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'offckb spaced-')); + const script = path.join(dir, 'index.js'); + fs.writeFileSync(script, ''); + process.env.OFFCKB_CLI_PATH = script; + const startedAt = new Date(2026, 7, 13, 12, 36, 26); + + mockExecFile.mockImplementation((file: string, args: string[], options: any, callback?: any) => { + const cb = typeof options === 'function' ? options : callback; + if (file === 'ps') { + if (args.includes('args=')) { + // ps never quotes: the space in the script path is a bare space. + cb(null, `${process.execPath} ${script} node --daemon`, ''); + return; + } + if (args.includes('lstart=')) { + cb(null, 'Wed Aug 13 12:36:26 2026', ''); + return; + } + } + cb(new Error(`Unexpected command: ${file}`), '', ''); + }); + + const metadata: PidMetadata = { + pid: 12345, + scriptPath: script, + startedAt: startedAt.toISOString(), + }; + expect(await verifyDaemonIdentity(12345, metadata)).toBe(true); + + // A foreign script under a sibling spaced directory must not match. + mockExecFile.mockImplementation((file: string, args: string[], options: any, callback?: any) => { + const cb = typeof options === 'function' ? options : callback; + if (file === 'ps' && args.includes('args=')) { + cb(null, `${process.execPath} ${script}-other node --daemon`, ''); + return; + } + if (file === 'ps' && args.includes('lstart=')) { + cb(null, 'Wed Aug 13 12:36:26 2026', ''); + return; + } + cb(new Error(`Unexpected command: ${file}`), '', ''); + }); + expect(await verifyDaemonIdentity(12345, metadata)).toBe(false); + + fs.rmSync(dir, { recursive: true, force: true }); + }); }); From b0b7da72739a3e9cbc87eb2e6e239c713ca2dd4d Mon Sep 17 00:00:00 2001 From: humble-little-bear Date: Thu, 8 Oct 2026 13:07:13 +0800 Subject: [PATCH 2/3] fix: compare daemon paths with the host path module, not process.platform The previous commit chose posix/win32 path rules from process.platform. Tests (and anything else) that override process.platform then compared paths with the wrong rules: on a Windows runner the node stop tests set platform to 'linux', so the '/path/to/offckb' token from the ps mock was resolved with posix rules while the CLI entry is a native 'D:\path\to\offckb', and every identity check failed. Default to the host's own path module (as before #533), including realpath and Windows case-insensitivity; the explicit platform argument is only used by unit tests that exercise another OS's command lines. --- src/util/daemon.ts | 51 +++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/src/util/daemon.ts b/src/util/daemon.ts index 6b426f8..93e0d3d 100644 --- a/src/util/daemon.ts +++ b/src/util/daemon.ts @@ -375,14 +375,26 @@ export async function getProcessCommandLine(pid: number): Promise return info.cmdline; } +// Path rules for comparing process paths. `platform` is only passed by unit +// tests that exercise another OS's command lines; by default the host's own +// `path` module is used (not process.platform, which tests may override). +type PathRules = { api: path.PlatformPath; native: boolean; caseInsensitive: boolean }; + +function pathRules(platform?: NodeJS.Platform): PathRules { + if (platform == null) { + return { api: path, native: true, caseInsensitive: path.sep === '\\' }; + } + const api = platform === 'win32' ? path.win32 : path.posix; + return { api, native: (platform === 'win32') === (path.sep === '\\'), caseInsensitive: platform === 'win32' }; +} + // Resolve symlinks on both sides before comparing (nvm shims, // /usr/bin/node → /etc/alternatives, symlinked install prefixes). A path // that cannot be resolved still compares by its absolute form. Paths of -// another platform (unit tests) are only resolved lexically. -function normalizePathForCompare(candidate: string, platform: NodeJS.Platform = process.platform): string { - const pathApi = platform === 'win32' ? path.win32 : path.posix; - const resolved = pathApi.resolve(candidate); - if (platform !== process.platform) return resolved; +// another OS (unit tests) are only resolved lexically. +function normalizePathForCompare(candidate: string, rules: PathRules = pathRules()): string { + const resolved = rules.api.resolve(candidate); + if (!rules.native) return resolved; try { return fs.realpathSync(resolved); } catch { @@ -391,19 +403,15 @@ function normalizePathForCompare(candidate: string, platform: NodeJS.Platform = } // Windows file systems are case-insensitive. -function samePath(a: string, b: string, platform: NodeJS.Platform): boolean { - return platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b; +function samePath(a: string, b: string, rules: PathRules): boolean { + return rules.caseInsensitive ? a.toLowerCase() === b.toLowerCase() : a === b; } -function executableLooksLikeNode(executable: string, platform: NodeJS.Platform = process.platform): boolean { - if ( - platform === process.platform && - samePath(normalizePathForCompare(executable), normalizePathForCompare(process.execPath), platform) - ) { +function executableLooksLikeNode(executable: string, rules: PathRules = pathRules()): boolean { + if (rules.native && samePath(normalizePathForCompare(executable), normalizePathForCompare(process.execPath), rules)) { return true; } - const pathApi = platform === 'win32' ? path.win32 : path.posix; - return NODE_EXECUTABLE_NAMES.has(pathApi.basename(executable).toLowerCase()); + return NODE_EXECUTABLE_NAMES.has(rules.api.basename(executable).toLowerCase()); } // A piece of a flat command line, optionally wrapped in one pair of quotes @@ -438,13 +446,10 @@ const MAX_COMMAND_LINE_SPLIT_POINTS = 64; * runtime and the script resolves to exactly `expectedScript` — the same * strict identity as for an exact argv, without guessing token boundaries. */ -export function commandLineRunsScript( - cmdline: string, - expectedScript: string, - platform: NodeJS.Platform = process.platform, -): boolean { +export function commandLineRunsScript(cmdline: string, expectedScript: string, platform?: NodeJS.Platform): boolean { + const rules = pathRules(platform); const text = cmdline.trim(); - const expected = normalizePathForCompare(expectedScript, platform); + const expected = normalizePathForCompare(expectedScript, rules); const spaces: number[] = []; for (let i = 0; i < text.length && spaces.length < MAX_COMMAND_LINE_SPLIT_POINTS; i++) { if (text[i] === ' ') spaces.push(i); @@ -452,11 +457,11 @@ export function commandLineRunsScript( const scriptEnds = [...spaces, text.length]; for (const executableEnd of spaces) { const executable = unquoteCommandLinePiece(text.slice(0, executableEnd)); - if (executable == null || !executableLooksLikeNode(executable, platform)) continue; + if (executable == null || !executableLooksLikeNode(executable, rules)) continue; for (const scriptEnd of scriptEnds) { if (scriptEnd <= executableEnd + 1) continue; const script = unquoteCommandLinePiece(text.slice(executableEnd + 1, scriptEnd)); - if (script != null && samePath(normalizePathForCompare(script, platform), expected, platform)) return true; + if (script != null && samePath(normalizePathForCompare(script, rules), expected, rules)) return true; } } return false; @@ -496,7 +501,7 @@ export async function verifyDaemonIdentity(pid: number, metadata: PidMetadata): // Exact argv (Linux /proc): compare the first two arguments directly. if (info.argv.length < 2) return false; if (!executableLooksLikeNode(info.argv[0])) return false; - if (!samePath(normalizePathForCompare(info.argv[1]), normalizePathForCompare(cliEntry), process.platform)) { + if (!samePath(normalizePathForCompare(info.argv[1]), normalizePathForCompare(cliEntry), pathRules())) { return false; } } else if (info.cmdline == null || !commandLineRunsScript(info.cmdline, cliEntry)) { From 06d53c253539cf7734beb892b7cfab496fd45b1d Mon Sep 17 00:00:00 2001 From: humble-little-bear Date: Thu, 8 Oct 2026 13:11:21 +0800 Subject: [PATCH 3/3] test: keep the real stale-miner process listing out of node command unit tests On windows-latest, node-terminal-rpc timed out (5 s) in its first test: since #531, foreground startNode/nodeDevnet runs reapStaleMiners, which lists processes through PowerShell CIM on Windows and can take seconds on a cold runner (the suite already took ~5.4 s on the previous run). The second failure in that file was fallout from the timed-out test. Mock reapStaleMiners in node-terminal-rpc, node-quiet-mode and node-command like node-supervisor already does; the reaper has its own tests. --- tests/node-command.test.ts | 8 ++++++++ tests/node-quiet-mode.test.ts | 8 ++++++++ tests/node-terminal-rpc.test.ts | 8 ++++++++ 3 files changed, 24 insertions(+) diff --git a/tests/node-command.test.ts b/tests/node-command.test.ts index 9b7e661..619108f 100644 --- a/tests/node-command.test.ts +++ b/tests/node-command.test.ts @@ -14,6 +14,14 @@ const mockStatSync = jest.fn(); const mockCloseSync = jest.fn(); const mockWaitForNodeReady = jest.fn(); +// The stale-miner reaper lists real OS processes (PowerShell CIM on Windows, +// which alone can take seconds); it has its own tests, so keep it out of +// these command-flow unit tests. +jest.mock('../src/devnet/stale-miner', () => ({ + ...jest.requireActual('../src/devnet/stale-miner'), + reapStaleMiners: jest.fn().mockResolvedValue([]), +})); + jest.mock('child_process', () => ({ ...jest.requireActual('child_process'), spawn: (...args: unknown[]) => mockSpawn(...args), diff --git a/tests/node-quiet-mode.test.ts b/tests/node-quiet-mode.test.ts index a687071..168b039 100644 --- a/tests/node-quiet-mode.test.ts +++ b/tests/node-quiet-mode.test.ts @@ -8,6 +8,14 @@ const mockTcpListenAddress = jest.fn(); const loggerInfo = jest.fn(); const loggerWarn = jest.fn(); +// The stale-miner reaper lists real OS processes (PowerShell CIM on Windows, +// which alone can take seconds); it has its own tests, so keep it out of +// these command-flow unit tests. +jest.mock('../src/devnet/stale-miner', () => ({ + ...jest.requireActual('../src/devnet/stale-miner'), + reapStaleMiners: jest.fn().mockResolvedValue([]), +})); + jest.mock('child_process', () => ({ ...jest.requireActual('child_process'), spawn: (...args: unknown[]) => mockSpawn(...args), diff --git a/tests/node-terminal-rpc.test.ts b/tests/node-terminal-rpc.test.ts index 1ee5b0f..d4e0fc8 100644 --- a/tests/node-terminal-rpc.test.ts +++ b/tests/node-terminal-rpc.test.ts @@ -9,6 +9,14 @@ const mockInitChainIfNeeded = jest.fn(); const mockDevnetConfigHasTerminalRpc = jest.fn(); const mockWaitForNodeReady = jest.fn(); +// The stale-miner reaper lists real OS processes (PowerShell CIM on Windows, +// which alone can take seconds); it has its own tests, so keep it out of +// these command-flow unit tests. +jest.mock('../src/devnet/stale-miner', () => ({ + ...jest.requireActual('../src/devnet/stale-miner'), + reapStaleMiners: jest.fn().mockResolvedValue([]), +})); + jest.mock('child_process', () => ({ ...jest.requireActual('child_process'), spawn: (...args: unknown[]) => mockSpawn(...args),