test(kbn-101-02a): resolve tsx CLI directly, exact dev:local contract, honest timeout bound

Three reviewer blockers, all verified against source and by measurement before
acting on them.

C1 — the fixture's `tsx` was `apps/gateway/node_modules/.bin/tsx`, which is not
the TSX CLI but a pnpm-generated /bin/sh launcher that **exports NODE_PATH**
naming four repository module roots into the process it starts. A fixture whose
stated invariant is that the child receives no Node loader controls was shipping
a `tsx` that reinstated one. Resolving `tsx/dist/cli.mjs` directly deletes the
wrapper, the NODE_PATH injection, three coreutils symlinks and an off-PATH
package-layout symlink at once: PATH is now two names, `node` and `tsx`. Targets
are realpath-resolved and asserted to be executable files at module scope,
before the first child, and the symlinks derive from those checked values.
Observed child key set carries neither NODE_PATH nor NODE_OPTIONS.

C3 — the dev:local contract was a substring check followed by arbitrary shell
execution: the manifest case asserted the script *contained* src/local-dev.ts,
and the helper then handed the whole string to `sh -c`. "curl … | sh; tsx
src/local-dev.ts" satisfied it and ran. Now one module constant spelled once,
asserted with equality in the manifest case and re-asserted in the executing
helper against the value about to run. No metacharacter denylist — equality
already excludes them, and a denylist here would repeat the mistake the PATH
allowlist was built to correct.

C2 — the timeout bound is corrected rather than closed. `exec` removes the outer
shell, but the required route is itself multi-process: TSX spawns an esbuild
service child and a loader/eval child, and the latter is where route code runs.
Measured six out of six times under this exact spawn shape, that child survives
SIGKILL to its TSX parent, is reparented, and completes its work afterward —
while sandbox teardown removes directories it still holds. So exact equality
plus `exec` prohibits *injected* shell content and does not close the gap; the
launcher the contract *permits* is the one that leaks. The timeout control is
named and commented for the single-process property it actually measures and is
not offered as containment evidence.

Containment design is at architecture adjudication and is deliberately not
implemented here: every process in the TSX tree runs in the verifier's own
process group and session, so the obvious repair would signal the test runner.

RED unchanged: 83 failed | 17 passed -> 83 failed | 18 passed (101), the one
added case being the timeout control passing as a harness control. Prettier,
tsc and ESLint clean; zero /tmp/kbn-02a-* residue; casts unchanged at six.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01ESFAnh2t9HmLwng8oW95St
This commit is contained in:
shaggyandClaude Opus 5 committed 2026-08-13 00:57:05 -05:00
1 parent a266c88b7f
commit 517e1adad3
2 files changed
+185 -67

No files matched your search

+77 -42
View File
@@ -25,7 +25,7 @@ import {
} from 'node:fs';
import { readFile, mkdtemp, realpath, rm } from 'node:fs/promises';
import { homedir, tmpdir } from 'node:os';
import { dirname, join } from 'node:path';
import { join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { types as utilTypes } from 'node:util';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
@@ -35,6 +35,7 @@ import type * as LocalDevNamespace from './local-dev.js';
const FORBIDDEN_DSN = 'postgresql://mosaic:mosaic@localhost:5433/mosaic';
const HERE = fileURLToPath(new URL('.', import.meta.url));
const REQUIRED_DEV_LOCAL_SCRIPT = 'exec tsx src/local-dev.ts';
// --- Z1-Z5 instruments. Nothing real is reachable through these. -------------
const bootstrapMock = vi.fn(async () => {});
@@ -273,7 +274,7 @@ describe('KBN-101-02a local-dev — the channel has its own command', () => {
// is the shape this item exists to remove.
const devLocal = manifest.scripts?.['dev:local'];
expect(devLocal, 'no dev:local script').toBeDefined();
expect(devLocal).toContain('src/local-dev.ts');
expect(devLocal).toBe(REQUIRED_DEV_LOCAL_SCRIPT);
});
it('leaves the production-like dev script untouched', async () => {
@@ -288,11 +289,12 @@ describe('KBN-101-02a local-dev — the channel has its own command', () => {
/**
* The package script, executed rather than read.
*
* The string assertions above are satisfied by `"dev:local": "tsx src/local-dev.ts"`
* pointed at a module that exports `runLocalDev()` and never calls it — which is
* exactly what this file is today: running that command right now exits 0 having
* started nothing and refused nothing. A route that silently succeeds is worse
* than one that is missing, because the operator's shell says it worked.
* The exact script assertion above is satisfied by a `dev:local` route using the
* required exec form pointed at a module that exports `runLocalDev()` and never
* calls it - which is exactly what this file is today: running that command
* right now exits 0 having started nothing and refused nothing. A route that
* silently succeeds is worse than one that is missing, because the operator's
* shell says it worked.
*
* Safety of running the real route in-band: the child inherits nothing and
* receives only the base allowlist plus names a case explicitly declares. No
@@ -342,37 +344,34 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo
const TRUSTED_SHELL = resolveTrustedShell();
function resolveFixtureCommand(name: string): string {
for (const directory of ['/usr/bin', '/bin']) {
const candidate = join(directory, name);
if (existsSync(candidate) && statSync(candidate).isFile()) {
return candidate;
}
}
throw new Error(`fixture command ${name} was not found in /usr/bin or /bin`);
}
function resolveTsxExecutable(): string {
for (const candidate of [
join(GATEWAY_DIR, 'node_modules', '.bin', 'tsx'),
join(REPO_ROOT, 'node_modules', '.bin', 'tsx'),
]) {
if (existsSync(candidate) && statSync(candidate).isFile()) {
const candidates = [
join(GATEWAY_DIR, 'node_modules', 'tsx', 'dist', 'cli.mjs'),
join(REPO_ROOT, 'node_modules', 'tsx', 'dist', 'cli.mjs'),
];
for (const candidate of candidates) {
if (existsSync(candidate)) {
return candidate;
}
}
throw new Error('tsx was not found in the gateway or repository node_modules/.bin');
throw new Error(`tsx CLI was not found at either candidate: ${candidates.join(', ')}`);
}
function validatePathCommandTarget(name: string, target: string): string {
const resolved = realpathSync(target);
if (!existsSync(resolved)) {
throw new Error(`${name} PATH target does not exist after realpath: ${resolved}`);
}
if (!statSync(resolved).isFile()) {
throw new Error(`${name} PATH target is not a file: ${resolved}`);
}
accessSync(resolved, fsConstants.X_OK);
return resolved;
}
const TSX_EXECUTABLE = resolveTsxExecutable();
const PATH_COMMAND_TARGETS = Object.freeze({
node: process.execPath,
tsx: TSX_EXECUTABLE,
// The pnpm-generated `tsx` launcher uses these three commands to locate and
// dispatch its JavaScript entrypoint.
dirname: resolveFixtureCommand('dirname'),
sed: resolveFixtureCommand('sed'),
uname: resolveFixtureCommand('uname'),
node: validatePathCommandTarget('node', process.execPath),
tsx: validatePathCommandTarget('tsx', resolveTsxExecutable()),
});
const PATH_COMMAND_NAMES = Object.freeze(Object.keys(PATH_COMMAND_TARGETS).sort());
@@ -419,13 +418,12 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo
* The complete base environment for one child. Built from nothing; nothing is
* inherited. A case may add only the explicitly declarable route inputs.
*
* `PATH` names one fixture-owned directory containing symlinks only to the
* commands the route needs. The workspace `.bin` directory carries real agent
* runtimes this repository depends on, while Node ordinarily lives in
* `/usr/bin`; the former construction therefore put thousands of executables
* on `PATH` while claiming to prevent an unrelated same-name executable from
* answering. A `#!/usr/bin/env node` shebang needs `node` on `PATH`, but `env`
* itself needs no entry because the shebang invokes it by absolute path.
* `PATH` names one fixture-owned directory containing only `node` and the
* real TSX CLI. The pnpm-generated `.bin/tsx` wrapper is deliberately rejected:
* it injects a Node loader control, and wrapper parity is not a fixture goal.
* This fixture-owned command boundary executes the same TSX CLI directly. Its
* `#!/usr/bin/env node` shebang needs `node` on `PATH`, but `env` needs no entry
* because the shebang invokes it by absolute path.
*
* `XDG_CONFIG_DIRS` and `XDG_DATA_DIRS` are set to private empty directories
* rather than left unset. Unset is not neutral for the XDG directory *lists*:
@@ -479,9 +477,6 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo
for (const [name, target] of Object.entries(PATH_COMMAND_TARGETS)) {
symlinkSync(target, join(sandboxBin, name));
}
// pnpm's shell launcher locates `../tsx/dist/cli.mjs` relative to `$0`.
// Preserve that layout outside PATH after moving `$0` into the sandbox bin.
symlinkSync(join(dirname(TSX_EXECUTABLE), '..', 'tsx'), join(sandbox, 'tsx'), 'dir');
return sandbox;
}
@@ -559,6 +554,7 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo
command: string,
args: string[],
extraEnv: Record<string, string> = {},
timeout = CHILD_TIMEOUT_MS,
): {
status: number | null;
signal: NodeJS.Signals | null;
@@ -571,7 +567,7 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo
cwd: GATEWAY_DIR,
env: buildHermeticChildEnv(sandbox, extraEnvSnapshot),
encoding: 'utf8',
timeout: CHILD_TIMEOUT_MS,
timeout,
killSignal: 'SIGKILL',
});
@@ -631,6 +627,10 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo
expect.fail('the package manifest has no dev:local script yet');
}
expect(
script,
`dev:local must be the exact required route; found ${JSON.stringify(script)}`,
).toBe(REQUIRED_DEV_LOCAL_SCRIPT);
return spawnHermetic(TRUSTED_SHELL, ['-c', script], extraEnv);
}
@@ -812,6 +812,41 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo
}
});
/**
* This control's arm is a single-process `exec node -e` sleeper. For that arm,
* SIGKILL reaches the process that matters and nothing survives to write; the
* two assertions below measure only that. The required route is itself
* multi-process: TSX spawns an esbuild service child and a loader/eval child,
* where the route code runs. Measured six out of six times under this exact
* spawn shape, that loader/eval child survives SIGKILL to its TSX parent, is
* reparented, and completes its work afterward. The exact `dev:local` script
* contract blocks injected shell content, but does not close this gap:
* the permitted launcher can leave a live process holding paths inside a
* sandbox the fixture is removing. Descendant containment is an open question
* deferred to the reviewer; no assertion in this file currently proves it.
*/
it('fixture control: timeout SIGKILLs the single-process arm and it writes no marker afterward', async () => {
const timeoutMs = 200;
const writeDelayMs = 1_000;
const markerName = 'timeout-survivor';
const markerScript =
`const { writeFileSync } = require('node:fs');` +
`setTimeout(() => writeFileSync(process.env.TMPDIR + '/${markerName}', 'SURVIVED'), ${writeDelayMs});`;
const result = spawnHermetic(
TRUSTED_SHELL,
['-c', `exec ${JSON.stringify(process.execPath)} -e ${JSON.stringify(markerScript)}`],
{},
timeoutMs,
);
const marker = join(result.sandbox, 'tmp', markerName);
expect(result.signal, `the timeout did not send SIGKILL: ${result.output}`).toBe('SIGKILL');
await new Promise((resolve) => setTimeout(resolve, writeDelayMs + 250));
expect(existsSync(marker), 'the killed route process survived long enough to write').toBe(
false,
);
});
/**
* The route-arm half of the same obligation, and it is PENDING EXECUTION until
* GREEN. At RED head there is no `dev:local` script, so `runDevLocal` fails on
+108 -25
View File
@@ -500,8 +500,13 @@ executable-resolution hole the allowlist exists to close.
**`PATH` was the last part of the child environment still built as a denylist, and calling it an
allowlist was false in a way that a measurement settles.** It named the two workspace
`node_modules/.bin` directories, the directory of the running Node binary, and the shell's
directory. On this host that is 4,093 distinct executable names — because `dirname(process.execPath)`
is `/usr/bin`. Worse than the count: `apps/gateway/node_modules/.bin` carries real agent-runtime
directory. Measured on the author's host that is 4,093 distinct executable names, and on the
reviewer's 2,650 — because `dirname(process.execPath)` is `/usr/bin`. Both numbers are stated with
whose run produced them on purpose. A count of what is reachable through `/usr/bin` is a property
of a host and its install state, not of the candidate tree, and presenting either figure bare would
repeat this item's own defect: a number offered as a property of the artifact when it is a property
of the observation. What does not vary between the two runs is the shape — an unbounded system
directory on a fixture `PATH`. Worse than the count: `apps/gateway/node_modules/.bin` carries real agent-runtime
executables this repository depends on directly — `pi` (`@mariozechner/pi-coding-agent`), `pi-ai`,
`openai`, `anthropic-ai-sdk`. At GREEN, any accidental resolution of one of those names from the
package script or its process tree reaches a real agent runtime with real capability. The hard
@@ -509,17 +514,32 @@ timeout bounds how long that runs; it does not prevent it. A directory list is n
allowlist, and the comment on that construction claimed it stopped exactly the resolution it
permitted.
`PATH` is now a single fixture-owned directory inside the per-run sandbox, holding symlinks and
nothing else: `node` (to `process.execPath`), `tsx` (to the executable the package route actually
resolves), and `dirname`, `sed`, `uname`. The last three are not decoration — the pnpm-generated
`tsx` launcher is a `/bin/sh` script that calls all three to locate its own JavaScript entrypoint,
so a `PATH` without them does not run the route at all. The launcher also finds
`../tsx/dist/cli.mjs` relative to `$0`, so the sandbox carries one directory symlink to the `tsx`
package at that relative location; it is not on `PATH`. `env` needs no entry either, because a
`#!/usr/bin/env node` shebang invokes `env` by absolute path and only `node` is resolved through
`PATH`. Measured: five reachable executable names, down from 4,093; `command -v pi` reports the real
`.bin/pi` under the old construction and not-found under the new one; `tsx --version` still succeeds
on the narrowed `PATH`.
`PATH` is now a single fixture-owned directory inside the per-run sandbox, holding two symlinks and
nothing else: `node` to `process.execPath`, and `tsx` to the TSX CLI artifact itself
(`tsx/dist/cli.mjs`). `env` needs no entry, because a `#!/usr/bin/env node` shebang invokes `env` by
absolute path and only `node` is resolved through `PATH`. Both targets are resolved through
`realpath` and asserted to be executable files at module scope — before the first child is created,
not inside a case that may run after other cases have already spawned — and the symlinks are built
from those checked values, so the construction and the control's identity assertions cannot drift
apart. Measured: two reachable executable names; `command -v pi` reports the real `.bin/pi` under
the old construction and not-found under the new one; `tsx --version` still succeeds.
Reaching two rather than five took a second correction, and the intermediate step is worth
recording because it was wrong in an instructive direction. The first narrowing symlinked
`apps/gateway/node_modules/.bin/tsx`, which is not the TSX CLI: it is a pnpm-generated `/bin/sh`
launcher. That forced three coreutils symlinks onto `PATH` (`dirname`, `sed`, `uname`, which the
launcher calls to locate its own entrypoint) and one off-`PATH` directory symlink to restore the
`$0`-relative package layout it expects. Five names and a layout fixture, all of them serving a
wrapper. The wrapper also **exports `NODE_PATH`** naming four real repository module roots into the
process it starts — so a fixture whose stated invariant is that the child receives no Node loader
controls was shipping a `tsx` that reinstated one. That was first recorded here as a bound; the
reviewer ruled it had to be fixed rather than bounded, and the ruling produced the smaller design
rather than a larger one. Executing the CLI directly deletes the wrapper, the `NODE_PATH`
injection, three symlinks and the layout fixture at once. Measured through the two-name `PATH`, the
child's key set is `HOME PATH PWD SHLVL TMPDIR XDG_CACHE_HOME XDG_CONFIG_DIRS XDG_CONFIG_HOME
XDG_DATA_DIRS XDG_DATA_HOME XDG_RUNTIME_DIR XDG_STATE_HOME` — neither `NODE_PATH` nor
`NODE_OPTIONS`. Wrapper parity was never a fixture goal; this is a fixture-owned command boundary
executing the same CLI the route executes.
The control does not assert that in prose. It splits the child's **observed** `PATH`, reads each
directory, collects every entry that is an executable file, and asserts that name set **equals**
@@ -534,15 +554,17 @@ become something else.
Two bounds on that claim, stated because the recurring defect in this item is prose crediting a
mechanism with a property it lacks. First, the claim is about **command resolution**, not about
everything the route can reach: the pnpm `tsx` launcher exports `NODE_PATH` naming four real
repository module roots into the process it starts, and the last of them
(`node_modules/.pnpm/node_modules`) exposes 775 packages — `@mariozechner/pi-coding-agent`,
`@anthropic-ai` and `openai` among them. Narrowing `PATH` removes those runtimes as _commands_; it
does not remove them as _modules_, and nothing at this layer could, because the route legitimately
runs repository code against repository `node_modules`. Second, executable enumeration is a
mechanical coverage check and is separate from reading source: the review that passed this fixture
before the hole was found was bounded by paths whose directory contents it never listed, which is
precisely why the guarantee is now measured by the fixture rather than observed by a reviewer.
everything the route can reach. Removing the wrapper removes the `NODE_PATH` the fixture was
injecting, and the two-name child carries no loader control at all — but at GREEN the route runs
repository code against repository `node_modules`, and `node_modules/.pnpm/node_modules` holds 775
packages including `@mariozechner/pi-coding-agent`, `@anthropic-ai` and `openai`. Those runtimes
are not resolvable as _commands_; they remain present as _modules_, and nothing at this layer could
change that without changing what the route is. "Not resolvable as a command" is the whole claim,
and "unreachable" is not a sentence this document gets to write. Second, executable enumeration is
a mechanical coverage check and is separate from reading source: the review that passed this
fixture before the hole was found was bounded by paths whose directory contents it never listed,
which is precisely why the guarantee is now measured by the fixture rather than observed by a
reviewer.
This is the third real-runtime-reachability incident in this window — the FL-09 ambient OpenCode
fall-through, the review command-substitution Claude launch, and now real agent executables on a
@@ -562,9 +584,70 @@ the command — so none carries host state, but they are additions and the docum
than rounding them away. The measured claim is: the shell-mediated child receives every allowlisted
name with the value the builder constructed for it, and every name beyond the allowlist is one of
those five. That measurement is of the control's own arm, which runs `sh -c 'exec node -e …'`. It
does not describe the GREEN route, which runs through the pnpm `tsx` launcher and will therefore
also carry the `NODE_PATH` that launcher exports. What the route's own process ends up holding is
unmeasured until there is a GREEN to measure.
does not describe the GREEN route. What the route's own process ends up holding is unmeasured until
there is a GREEN to measure.
**The package-script contract was a substring check followed by arbitrary shell execution, and the
two halves were never read together.** The manifest case asserted the script _contained_
`src/local-dev.ts`; the executing helper then handed the whole string to `sh -c`. So
`"curl … | sh; tsx src/local-dev.ts"` satisfied the assertion and was then run — as would anything
carrying `&`, `;`, `|`, a backtick, `$( )`, a redirect, or a second absolute command. Neither line
is wrong in isolation, which is why it survived several readings: a substring check is a reasonable
thing to write about a script you are only inspecting, and `sh -c` is a reasonable thing to write
about a script you have already validated. The defect exists only in the seam between them. The
contract is now a single module-level constant, `exec tsx src/local-dev.ts`, spelled exactly once in
the file, asserted with equality in the manifest case **and** re-asserted inside the executing
helper against the value about to run — the manifest is read twice, so the check that matters is the
one sitting on the string being executed. No metacharacter denylist backs it up: exact equality
already excludes every metacharacter anyone would enumerate, and a denylist here would repeat the
mistake the `PATH` allowlist was built to correct.
**The hard timeout does not contain the route's process tree, and the required script is the reason
— not an attacker's.** `spawnSync`'s `timeout` with `killSignal: 'SIGKILL'` signals the process it
directly invoked and nothing below it. Two measurements bound what that is worth, and both were
produced by probing rather than by reading the documentation.
The first is why the route must `exec`. With a backgrounded job — `node -e '…' & echo started` —
the shell returns 0 **immediately**, so the timeout never fires at all: the case observes a clean
exit, the descendant outlives the run, and sandbox teardown then races a live process still holding
paths inside the directory being removed. That is not merely a containment gap; it is a
**silent-pass** path, a control reporting success in exactly the scenario it exists to catch.
Requiring `exec ` at the head of the contract removes the intermediate shell so the signal reaches
the process that matters.
The second is the one that stops this item from claiming the pair is sufficient. `exec` removes the
outer shell; it does not make TSX one process. The selected CLI spawns an `esbuild --service` child
and a `node --require preflight.cjs --import loader.mjs --eval` child, and the second is where the
route's own code runs. Under the fixture's exact spawn shape, four trials out of four, the parent
received `SIGKILL` and that loader child **survived it**, was reparented away from the dead parent,
and went on to complete its work afterward — writing its marker while the fixture believed the
child was gone. The esbuild child did not survive or accumulate in those runs, so the survivor is
the loader child specifically. An earlier probe of the author's appeared to show the whole tree
dying; it enumerated descendants by the killed parent's PID, and reparenting had already moved them
out of that set, so the survivors were live and the filter structurally could not see them. That
probe is recorded here rather than discarded, because a measurement that cannot observe the
violation it is testing for is the same defect class as a review bounded by paths it never listed.
What follows is stated plainly because the temptation to round it off is exactly the failure mode
this item documents: **exact script equality plus `exec` is not process-tree cleanup.** It prohibits
_injected_ shell content, which is worth having on its own terms. But the launcher the contract
_permits_ is multi-process, so a timed-out route can still leave a live process inside a sandbox
being torn down. The fixture's timeout control asserts what it actually measures — a single-process
`exec node -e` sleeper is killed and writes nothing — and it is named for that property alone. No
assertion in this file establishes descendant containment, and the control is not offered as
evidence of it.
One further measurement decides what any containment design may look like, so it is recorded here
rather than left for whoever writes that design to rediscover: every process in the TSX tree runs in
**the verifier's own process group and session**. Parent, esbuild child and loader child all carry
the test runner's `pgid` and `sid`, because a spawn inherits both by default. The consequence is
sharp — the obvious repair, signalling the negative process-group id, would signal the test runner
itself. Group containment is therefore not a matter of sending a different signal to the tree as it
exists; it requires the child to be given a group of its own at spawn time before any group-directed
signal is safe to send. Which design does that, and whether the resulting emptiness proof is
reuse-safe to the reviewer's standard, is under adjudication and deliberately not settled by the
author. This section will record the ruling and the residual — including the one no group-based
design can close, a descendant that calls `setsid()` for itself and leaves the group.
**There is exactly one place in the file where a child process is created**, and that is the
mechanism, not tidiness. A control that made its own spawn would leave the route's spawn free to be