From fefd6db283daccb21791e9dc2a2c620108f0c465 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Sat, 15 Aug 2026 21:51:38 +0200 Subject: [PATCH 1/2] fix: reject lone surrogates in process paths --- src/adapters/agent-transport.ts | 10 +++ tests/adapters/process-transport.test.ts | 99 ++++++++++++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/src/adapters/agent-transport.ts b/src/adapters/agent-transport.ts index 1d790e4..c80be65 100644 --- a/src/adapters/agent-transport.ts +++ b/src/adapters/agent-transport.ts @@ -749,6 +749,16 @@ function checkPath( if (containsNul(value)) { return invalid; } + // A path is an exact-transmission string like argv and stdin: it crosses the + // native string boundary on its way to `spawn`, and an unpaired code unit is + // substituted with U+FFFD there. The executable actually launched, or the + // directory the child actually runs in, would then be a *different* path than + // the one validated here. Checked before the byte measurement, because the + // measurement of an ill-formed path already describes the substitution rather + // than the path the caller supplied. + if (containsLoneSurrogate(value)) { + return invalid; + } if (utf8ByteLength(value) > TRANSPORT_BOUNDS.MAX_PATH_BYTES) { return invalid; } diff --git a/tests/adapters/process-transport.test.ts b/tests/adapters/process-transport.test.ts index 7cf161c..517f8bd 100644 --- a/tests/adapters/process-transport.test.ts +++ b/tests/adapters/process-transport.test.ts @@ -2409,6 +2409,105 @@ describe('invokeAgentProcess — boundary', () => { } }); + it.each(ILL_FORMED)( + 'refuses an executable path holding %s before process creation', + async (_label, value) => { + const exchange = await invokeAgentProcess( + makeSpec({ executablePath: `${NEVER_SPAWNED}${value}` }), + makeLimits(), + ); + + expect(exchange.outcome).toBe('SPEC_REJECTED'); + expect(exchange.rejection).toBe('EXECUTABLE_INVALID'); + expect(exchange.terminationScope).toBe('NOT_REQUIRED'); + expect(exchange.exitCode).toBeNull(); + expect(exchange.terminatingSignal).toBeNull(); + expect(exchange.stdout).toBe(''); + expect(exchange.stderr).toBe(''); + }, + ); + + it.each(ILL_FORMED)( + 'refuses a working directory holding %s before process creation', + async (_label, value) => { + // The executable is the real, spawnable stub interpreter, so nothing but a + // refusal that precedes spawn can produce `SPEC_REJECTED` here. + const exchange = await invokeAgentProcess( + makeSpec({ workingDirectory: `${NEVER_SPAWNED}${value}` }), + makeLimits(), + ); + + expect(exchange.outcome).toBe('SPEC_REJECTED'); + expect(exchange.rejection).toBe('WORKING_DIRECTORY_INVALID'); + expect(exchange.terminationScope).toBe('NOT_REQUIRED'); + expect(exchange.exitCode).toBeNull(); + expect(exchange.terminatingSignal).toBeNull(); + expect(exchange.stdout).toBe(''); + expect(exchange.stderr).toBe(''); + }, + ); + + it('starts no process at all when a path is ill-formed', async () => { + const directory = makeTempDirectory(); + try { + const marker = join(directory, 'ran'); + const script = `require("node:fs").writeFileSync(${JSON.stringify(marker)},"ran");`; + + // The identical stub with a well-formed working directory, so the marker is + // known to be a real signal rather than a script that never worked. + const accepted = await invokeAgentProcess( + makeSpec({ args: ['-e', script], workingDirectory: directory }), + makeLimits(), + ); + expect(accepted.outcome).toBe('EXITED'); + expect(existsSync(marker)).toBe(true); + rmSync(marker); + + const refusedDirectory = await invokeAgentProcess( + makeSpec({ args: ['-e', script], workingDirectory: `${directory}\uD800` }), + makeLimits(), + ); + + expect(refusedDirectory.outcome).toBe('SPEC_REJECTED'); + expect(refusedDirectory.rejection).toBe('WORKING_DIRECTORY_INVALID'); + expect(existsSync(marker)).toBe(false); + + const refusedExecutable = await invokeAgentProcess( + makeSpec({ + executablePath: `${NODE_EXECUTABLE}\uDC00`, + args: ['-e', script], + workingDirectory: directory, + }), + makeLimits(), + ); + + expect(refusedExecutable.outcome).toBe('SPEC_REJECTED'); + expect(refusedExecutable.rejection).toBe('EXECUTABLE_INVALID'); + expect(existsSync(marker)).toBe(false); + } finally { + removeTempDirectory(directory); + } + }); + + it('accepts a working directory holding a supplementary-plane character', async () => { + // The control for the rule above: a valid pair is two UTF-16 code units and + // must still pass path validation, and the child must actually run there. + const directory = mkdtempSync(join(tmpdir(), 'agentbridge-pr010-\u{1F600}-')); + try { + const exchange = await runStub(STUB.PRINT_CWD, [], { workingDirectory: directory }); + + expect(exchange.outcome).toBe('EXITED'); + // Compared as the suite compares any reported working directory, because + // Windows may report a different case than it was given. The pair itself + // has no case mapping, so it is still compared exactly. + expect(exchange.stdout.toLowerCase()).toBe(directory.toLowerCase()); + // Not the substitution an ill-formed path would have produced. + expect(exchange.stdout).not.toContain('�'); + } finally { + removeTempDirectory(directory); + } + }); + it('delivers a well-formed environment name and value to the child exactly', async () => { const name = 'AGENTBRIDGE_\u{1F600}'; const value = 'before \u{1F600} after \u{10000}'; From 98a34be85347cd80b60d1150256b66cf7aeda221 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Sat, 15 Aug 2026 22:55:54 +0200 Subject: [PATCH 2/2] test: make path marker proof conclusive --- tests/adapters/process-transport.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/adapters/process-transport.test.ts b/tests/adapters/process-transport.test.ts index 517f8bd..f0e4573 100644 --- a/tests/adapters/process-transport.test.ts +++ b/tests/adapters/process-transport.test.ts @@ -3,6 +3,7 @@ import { EventEmitter } from 'node:events'; import { readdirSync, existsSync, + mkdirSync, mkdtempSync, readFileSync, rmSync, @@ -2449,7 +2450,14 @@ describe('invokeAgentProcess — boundary', () => { it('starts no process at all when a path is ill-formed', async () => { const directory = makeTempDirectory(); + // The path Node substitutes for the ill-formed one at the native boundary. + // It must exist, or a regression that dropped the validation would still + // leave the marker absent — because `spawn` failed on a missing directory, + // not because the transport refused. Creating it makes the marker the only + // thing standing between a regression and a passing test. + const replacementDirectory = `${directory}\uFFFD`; try { + mkdirSync(replacementDirectory); const marker = join(directory, 'ran'); const script = `require("node:fs").writeFileSync(${JSON.stringify(marker)},"ran");`; @@ -2485,6 +2493,7 @@ describe('invokeAgentProcess — boundary', () => { expect(refusedExecutable.rejection).toBe('EXECUTABLE_INVALID'); expect(existsSync(marker)).toBe(false); } finally { + removeTempDirectory(replacementDirectory); removeTempDirectory(directory); } });