From c6dea6b3f9586ff4f7e97215c67c1a56a2eb64c1 Mon Sep 17 00:00:00 2001 From: Priya Raman Date: Wed, 26 Aug 2026 19:21:24 +0000 Subject: [PATCH] fix(adapter-utils): validate SSH env-lab fixture state before signals Any local process running as the same user can write a fixture state file. A forged pid or an empty sshdConfigPath weakened the identity check that gates SIGTERM/SIGKILL: an empty string is a substring of every command line, so it matched any running process. readSshEnvLabFixtureState now rejects a state file unless the pid is a positive safe integer, every path field is absolute and rooted at the fixture directory the state was read from, and sshdConfigPath is non-empty and equals rootDir/sshd_config exactly. Rejection happens before any identity check or signal. Co-authored-by: Paperclip --- .../adapter-utils/src/ssh-fixture.test.ts | 69 ++++++++++++++++++- packages/adapter-utils/src/ssh.ts | 58 ++++++++++++++++ 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/packages/adapter-utils/src/ssh-fixture.test.ts b/packages/adapter-utils/src/ssh-fixture.test.ts index cc08f084d9..e66df7d16d 100644 --- a/packages/adapter-utils/src/ssh-fixture.test.ts +++ b/packages/adapter-utils/src/ssh-fixture.test.ts @@ -1,4 +1,4 @@ -import { execFile } from "node:child_process"; +import { execFile, spawn } from "node:child_process"; import { mkdir, mkdtemp, readFile, rm, stat, symlink, writeFile } from "node:fs/promises"; import net from "node:net"; import os from "node:os"; @@ -247,6 +247,73 @@ describe("ssh env-lab fixture", () => { await stopSshEnvLabFixture(restarted); }, SSH_FIXTURE_TEST_TIMEOUT_MS); + it("rejects a forged state file and cannot signal an unrelated local process", async () => { + const rootDir = await createFixtureRootDir(); + const statePath = path.join(rootDir, "state.json"); + + // A process this test does not own. A forged state must never be able + // to target it for SIGTERM or SIGKILL. + const bystander = spawn("sleep", ["30"], { stdio: "ignore" }); + const bystanderPid = bystander.pid; + if (!bystanderPid) { + throw new Error("Failed to spawn the bystander process for this regression test."); + } + + try { + const baseState = { + kind: "ssh_openbsd" as const, + bindHost: "127.0.0.1", + host: "127.0.0.1", + port: 0, + username: os.userInfo().username, + rootDir, + workspaceDir: path.join(rootDir, "workspace"), + statePath, + createdAt: new Date().toISOString(), + clientPrivateKeyPath: path.join(rootDir, "client_key"), + clientPublicKeyPath: path.join(rootDir, "client_key.pub"), + hostPrivateKeyPath: path.join(rootDir, "host_key"), + hostPublicKeyPath: path.join(rootDir, "host_key.pub"), + authorizedKeysPath: path.join(rootDir, "authorized_keys"), + knownHostsPath: path.join(rootDir, "known_hosts"), + sshdConfigPath: path.join(rootDir, "sshd_config"), + sshdLogPath: path.join(rootDir, "sshd.log"), + }; + + const forgedVariants = [ + // An empty sshdConfigPath used to defeat the identity check: an + // empty string is a substring of every command line. + { ...baseState, pid: bystanderPid, sshdConfigPath: "" }, + // A sshdConfigPath outside the fixture root. + { ...baseState, pid: bystanderPid, sshdConfigPath: "/etc/ssh/sshd_config" }, + // A non-positive pid. + { ...baseState, pid: 0 }, + { ...baseState, pid: -1 }, + ]; + + for (const forged of forgedVariants) { + await writeFile(statePath, JSON.stringify(forged, null, 2), { mode: 0o600 }); + + const status = await readSshEnvLabFixtureStatus(statePath); + expect(status.running).toBe(false); + expect(status.state).toBeNull(); + + const stopped = await stopSshEnvLabFixture(statePath); + expect(stopped).toBe(false); + } + + // No forged state ever reached the identity check or a signal call, + // so the bystander process is still alive. + expect(() => process.kill(bystanderPid, 0)).not.toThrow(); + } finally { + try { + process.kill(bystanderPid, "SIGKILL"); + } catch { + // Already gone. + } + } + }, SSH_FIXTURE_TEST_TIMEOUT_MS); + it("stops the fixture listener and frees its loopback port", async () => { const rootDir = await createFixtureRootDir(); const statePath = path.join(rootDir, "state.json"); diff --git a/packages/adapter-utils/src/ssh.ts b/packages/adapter-utils/src/ssh.ts index 60db25e266..2dc4d71bfc 100644 --- a/packages/adapter-utils/src/ssh.ts +++ b/packages/adapter-utils/src/ssh.ts @@ -1704,12 +1704,70 @@ export async function ensureSshWorkspaceReady( }; } +const SSH_ENV_LAB_FIXTURE_PATH_FIELDS = [ + "rootDir", + "workspaceDir", + "statePath", + "clientPrivateKeyPath", + "clientPublicKeyPath", + "hostPrivateKeyPath", + "hostPublicKeyPath", + "authorizedKeysPath", + "knownHostsPath", + "sshdConfigPath", + "sshdLogPath", +] as const satisfies readonly (keyof SshEnvLabFixtureState)[]; + +// True when candidate is an absolute path equal to rootDir or nested under +// it. Used to reject a state file whose paths point outside the fixture +// root it was read from. +function isPathRootedAt(candidate: string, rootDir: string): boolean { + if (!path.isAbsolute(candidate)) return false; + if (candidate === rootDir) return true; + const relative = path.relative(rootDir, candidate); + return ( + relative.length > 0 && + relative !== ".." && + !relative.startsWith(`..${path.sep}`) && + !path.isAbsolute(relative) + ); +} + +// The state file is untrusted input: any local process running as the same +// user can write one. A forged pid or an empty sshdConfigPath would weaken +// isSshEnvLabFixtureProcess's identity check — an empty string is a +// substring of every command line, so it would match any running process. +// Reject a state file that fails this check before any identity check or +// signal runs against it. +function isValidSshEnvLabFixtureState( + raw: SshEnvLabFixtureState, + expectedRootDir: string, +): boolean { + if (!Number.isSafeInteger(raw.pid) || raw.pid <= 0) return false; + if (raw.rootDir !== expectedRootDir) return false; + + for (const field of SSH_ENV_LAB_FIXTURE_PATH_FIELDS) { + const value = raw[field]; + if (typeof value !== "string" || !isPathRootedAt(value, expectedRootDir)) { + return false; + } + } + + const expectedSshdConfigPath = path.join(expectedRootDir, "sshd_config"); + if (raw.sshdConfigPath.length === 0 || raw.sshdConfigPath !== expectedSshdConfigPath) { + return false; + } + + return true; +} + export async function readSshEnvLabFixtureState( statePath: string, ): Promise { try { const raw = JSON.parse(await fs.readFile(statePath, "utf8")) as SshEnvLabFixtureState; if (!raw || raw.kind !== "ssh_openbsd") return null; + if (!isValidSshEnvLabFixtureState(raw, path.dirname(statePath))) return null; return raw; } catch { return null;