Fix security related fixes · compounding.tech/pty@8965172

+7

CHANGELOG.md

··· 2 2 3 3 ## Unreleased 4 4 5 + ### Security 6 + - BUG-1: `validateName` now rejects session names whose Unix-socket path would exceed the 104-byte `sun_path` limit; previously the daemon's `listen()` failed silently inside an error handler and the `ready` Promise never resolved, hanging every caller. `server.ready` also now rejects on listen errors. 7 + - BUG-2: `acquireLock` is now built on `open(O_CREAT|O_EXCL)` (`openSync(path, "wx")`); two processes racing to steal a stale lock can no longer both win. 8 + - BUG-3: `PacketReader` enforces a 32 MiB `MAX_PACKET_LENGTH` cap and throws `PacketTooLargeError` on oversize length headers; socket handlers catch and destroy the peer. Previously a crafted `length=0xFFFFFFFF` frame would cause unbounded buffer growth. 9 + - BUG-4: Add `pty run --isolate-env` (and `isolateEnv: true` / `extraEnv` on `SpawnDaemonOptions` / `ServerOptions`) to spawn session children with a scrubbed env limited to an allow-list (`PATH`, `HOME`, `USER`, `LOGNAME`, `SHELL`, `TERM`, `COLORTERM`, `LANG`, `TZ`, `PWD`, `TMPDIR`, `LC_*`, `PTY_SESSION_DIR`, `PTY_SESSION`). Intended for pty-relay to apply when spawning daemons for remote clients — prevents operator secrets (cloud tokens, OAuth, `PTY_RELAY_PASSPHRASE`, etc.) from leaking into a session reachable by a remote user. Default behaviour is unchanged. 10 + - BUG-5: The Unix-socket file is now created under `umask 0o077`, closing the microsecond window where the inode was group/world-readable before `chmod 0o600`. 11 + 5 12 ### Fixes 6 13 - Fix mouse tracking modes (1000/1002/1003) not being replayed when a client reattaches to a session with mouse tracking already enabled — the server previously only replayed the SGR encoding (1006), cursor visibility, and kitty keyboard flags. Clients (e.g., pty-layout) checking tracking mode to decide whether to forward mouse events will now see the correct state. 7 14

+6 -2

src/cli.ts

··· 54 54 pty run -a -- <command> [args...] Create or attach if already running 55 55 pty run --tag key=value -- <command> Tag a session with metadata 56 56 pty run --cwd /path -- <command> Run in a specific directory 57 + pty run --isolate-env -- <command> Scrub env down to a safe allow-list (for remote-reachable sessions) 57 58 pty attach <name> Attach to an existing session 58 59 pty exec -- <command> [args...] Replace the current session's command 59 60 pty attach -r <name> Attach, auto-restart if exited ··· 168 169 let detach = false; 169 170 let attachExisting = false; 170 171 let ephemeral = false; 172 + let isolateEnv = false; 171 173 let name: string | null = null; 172 174 let cwd: string | null = null; 173 175 const tags: Record<string, string> = {}; ··· 176 178 if (args[i] === "-d" || args[i] === "--detach") { detach = true; i++; } 177 179 else if (args[i] === "-a" || args[i] === "--attach") { attachExisting = true; i++; } 178 180 else if (args[i] === "-e" || args[i] === "--ephemeral") { ephemeral = true; i++; } 181 + else if (args[i] === "--isolate-env") { isolateEnv = true; i++; } 179 182 else if (args[i] === "--name" && i + 1 < args.length) { name = args[i + 1]; i += 2; } 180 183 else if (args[i] === "--cwd" && i + 1 < args.length) { cwd = args[i + 1]; i += 2; } 181 184 else if (args[i] === "--tag" && i + 1 < args.length) { ··· 271 274 process.exit(1); 272 275 } 273 276 274 - await cmdRun(name, cmd, cmdArgs, detach, attachExisting, displayCmd, ephemeral, tags, cwd); 277 + await cmdRun(name, cmd, cmdArgs, detach, attachExisting, displayCmd, ephemeral, tags, cwd, isolateEnv); 275 278 break; 276 279 } 277 280 ··· 762 765 ephemeral = false, 763 766 tags: Record<string, string> = {}, 764 767 explicitCwd: string | null = null, 768 + isolateEnv = false, 765 769 ): Promise<void> { 766 770 const session = await getSession(name); 767 771 if (session?.status === "running") { ··· 794 798 try { 795 799 const tagOpt = Object.keys(tags).length > 0 ? tags : previousTags; 796 800 const cwdOpt = explicitCwd ?? previousCwd; 797 - await spawnDaemon({ name, command, args, displayCommand, cwd: cwdOpt, ephemeral, tags: tagOpt }); 801 + await spawnDaemon({ name, command, args, displayCommand, cwd: cwdOpt, ephemeral, tags: tagOpt, ...(isolateEnv ? { isolateEnv: true } : {}) }); 798 802 } finally { 799 803 releaseLock(name); 800 804 }

+18 -3

src/client.ts

··· 98 98 }); 99 99 100 100 socket.on("data", (data: Buffer) => { 101 - const packets = reader.feed(data); 101 + let packets; 102 + try { packets = reader.feed(data); } catch (err: any) { 103 + console.error(`pty client: dropping connection — ${err.message}`); 104 + try { socket.destroy(); } catch {} 105 + return; 106 + } 102 107 for (const packet of packets) { 103 108 switch (packet.type) { 104 109 case MessageType.SCREEN: ··· 239 244 }); 240 245 241 246 socket.on("data", (data: Buffer) => { 242 - const packets = reader.feed(data); 247 + let packets; 248 + try { packets = reader.feed(data); } catch (err: any) { 249 + console.error(`pty client: dropping connection — ${err.message}`); 250 + try { socket.destroy(); } catch {} 251 + return; 252 + } 243 253 for (const packet of packets) { 244 254 if (packet.type === MessageType.STATUS) { 245 255 clearTimeout(timer); ··· 386 396 }); 387 397 388 398 socket.on("data", (data: Buffer) => { 389 - const packets = reader.feed(data); 399 + let packets; 400 + try { packets = reader.feed(data); } catch (err: any) { 401 + console.error(`pty client: dropping connection — ${err.message}`); 402 + try { socket.destroy(); } catch {} 403 + return; 404 + } 390 405 for (const packet of packets) { 391 406 switch (packet.type) { 392 407 case MessageType.DATA:

+11 -2

src/connection.ts

··· 72 72 let initialScreenResolved = false; 73 73 74 74 socket.on("data", (raw: Buffer) => { 75 - const packets = this.reader.feed(raw); 75 + let packets; 76 + try { packets = this.reader.feed(raw); } catch { 77 + try { socket.destroy(); } catch {} 78 + return; 79 + } 76 80 for (const packet of packets) { 77 81 switch (packet.type) { 78 82 case MessageType.SCREEN: { ··· 188 192 }); 189 193 190 194 socket.on("data", (raw: Buffer) => { 191 - const packets = reader.feed(raw); 195 + let packets; 196 + try { packets = reader.feed(raw); } catch (err: any) { 197 + try { socket.destroy(); } catch {} 198 + reject(err); 199 + return; 200 + } 192 201 for (const packet of packets) { 193 202 if (packet.type === MessageType.SCREEN) { 194 203 socket.destroy();

+29 -1

src/protocol.ts

··· 21 21 // Packet wire format: [type: uint8][length: uint32BE][payload: N bytes] 22 22 const HEADER_SIZE = 5; 23 23 24 + // BUG-3: cap legitimate packet size. SCREEN replays carry the serialized 25 + // xterm buffer (rows × cols × attrs × scrollback). With the 10k-line default 26 + // scrollback plus mode prefixes, 32 MiB is generously above any real payload 27 + // while still small enough to bound a single malformed-length attack. 28 + export const MAX_PACKET_LENGTH = 32 * 1024 * 1024; 29 + 30 + /** Thrown when an inbound packet declares a length larger than 31 + * `MAX_PACKET_LENGTH`. Socket handlers should destroy the connection. */ 32 + export class PacketTooLargeError extends Error { 33 + readonly declaredLength: number; 34 + constructor(declaredLength: number) { 35 + super( 36 + `Packet length ${declaredLength} exceeds maximum ${MAX_PACKET_LENGTH}` 37 + ); 38 + this.name = "PacketTooLargeError"; 39 + this.declaredLength = declaredLength; 40 + } 41 + } 42 + 24 43 export function encodePacket(type: MessageType, payload: Buffer): Buffer { 25 44 const header = Buffer.alloc(HEADER_SIZE); 26 45 header.writeUInt8(type, 0); ··· 92 111 return payload.readInt32BE(0); 93 112 } 94 113 95 - /** Streaming packet parser that handles partial reads on a stream socket. */ 114 + /** Streaming packet parser that handles partial reads on a stream socket. 115 + * Throws `PacketTooLargeError` if a peer declares a length exceeding 116 + * `MAX_PACKET_LENGTH` — handlers should destroy the socket. */ 96 117 export class PacketReader { 97 118 private buffer = Buffer.alloc(0); 98 119 ··· 103 124 while (this.buffer.length >= HEADER_SIZE) { 104 125 const type = this.buffer.readUInt8(0) as MessageType; 105 126 const length = this.buffer.readUInt32BE(1); 127 + 128 + if (length > MAX_PACKET_LENGTH) { 129 + // Poison the buffer so subsequent feed() calls can't continue past 130 + // the bad header (even though the caller should drop the connection). 131 + this.buffer = Buffer.alloc(0); 132 + throw new PacketTooLargeError(length); 133 + } 106 134 107 135 if (this.buffer.length < HEADER_SIZE + length) break; 108 136

+72 -5

src/server.ts

··· 48 48 cols: number; 49 49 tags?: Record<string, string>; 50 50 onExit?: (code: number) => void; 51 + /** When true, spawn the child with a scrubbed environment containing only 52 + * a small allow-list of variables (plus any entries in `extraEnv`). 53 + * Intended for contexts where the daemon may have inherited secrets that 54 + * shouldn't leak into the session (e.g., a daemon launched by pty-relay 55 + * for a remote client). See BUG-4. */ 56 + isolateEnv?: boolean; 57 + /** Additional `KEY=VALUE` env entries to add on top of the isolation 58 + * allow-list. Only consulted when `isolateEnv` is true. */ 59 + extraEnv?: Record<string, string>; 60 + } 61 + 62 + /** Env variables that are safe to pass through to a session child when 63 + * `isolateEnv` is on. Keeps terminal/locale/path functionality working 64 + * without propagating the operator's shell secrets. */ 65 + const ISOLATED_ENV_ALLOWLIST = new Set([ 66 + "PATH", "HOME", "USER", "LOGNAME", "SHELL", 67 + "TERM", "COLORTERM", "LANG", "TZ", "PWD", "TMPDIR", 68 + // pty-internal 69 + "PTY_SESSION_DIR", 70 + ]); 71 + 72 + function buildChildEnv(options: ServerOptions): Record<string, string> { 73 + const source = process.env as Record<string, string>; 74 + 75 + if (!options.isolateEnv) { 76 + // Legacy behaviour: full inheritance, minus the server-config handoff. 77 + const env = { ...source }; 78 + delete env.PTY_SERVER_CONFIG; 79 + env.PTY_SESSION = options.name; 80 + return env; 81 + } 82 + 83 + const env: Record<string, string> = {}; 84 + for (const [k, v] of Object.entries(source)) { 85 + if (v === undefined) continue; 86 + if (ISOLATED_ENV_ALLOWLIST.has(k) || k.startsWith("LC_")) env[k] = v; 87 + } 88 + if (options.extraEnv) { 89 + for (const [k, v] of Object.entries(options.extraEnv)) env[k] = v; 90 + } 91 + env.PTY_SESSION = options.name; 92 + return env; 51 93 } 52 94 53 95 const LAST_LINES_COUNT = 200; ··· 338 380 // Spawn the child process in a PTY via a shell, so that shell scripts, 339 381 // symlinks, and shebangs all work reliably (like tmux/screen do). 340 382 // `exec "$@"` replaces the shell with the actual process. 341 - const childEnv = { ...process.env }; 342 - delete childEnv.PTY_SERVER_CONFIG; 343 - childEnv.PTY_SESSION = options.name; 383 + const childEnv = buildChildEnv(options); 344 384 345 385 const invalidCwd = describeInvalidCwd(options.cwd); 346 386 if (invalidCwd !== undefined) { ··· 409 449 this.socketServer = net.createServer((socket) => 410 450 this.handleClient(socket) 411 451 ); 412 - this.ready = new Promise((resolve) => { 452 + // Tighten umask around listen() so the socket inode is never transiently 453 + // group/world-readable (BUG-5). The chmodSync below is kept as 454 + // belt-and-suspenders for good measure. 455 + const prevUmask = process.umask(0o077); 456 + this.ready = new Promise((resolve, reject) => { 457 + let settled = false; 458 + this.socketServer.once("error", (err) => { 459 + if (settled) return; 460 + settled = true; 461 + reject(err); 462 + }); 413 463 this.socketServer.listen(socketPath, () => { 414 464 try { fs.chmodSync(socketPath, 0o600); } catch {} 415 465 fs.writeFileSync(getPidPath(this.name), process.pid.toString()); ··· 424 474 this.emitEvent(EventType.SESSION_START, { 425 475 ...(options.tags && Object.keys(options.tags).length > 0 ? { tags: options.tags } : {}), 426 476 }); 477 + if (settled) return; 478 + settled = true; 427 479 resolve(); 428 480 }); 429 481 }); 482 + process.umask(prevUmask); 430 483 484 + // Post-listen errors (e.g., socket file unlinked out from under us) must 485 + // not crash the process, but they also mustn't interfere with the 486 + // initial ready resolution above. 431 487 this.socketServer.on("error", (err) => { 432 488 console.error(`Socket server error: ${err.message}`); 433 489 }); ··· 445 501 this.clients.set(socket, client); 446 502 447 503 socket.on("data", (data: Buffer) => { 448 - const packets = client.reader.feed(data); 504 + let packets; 505 + try { 506 + packets = client.reader.feed(data); 507 + } catch (err: any) { 508 + // BUG-3: peer sent an oversize length header (or some other malformed 509 + // frame) — drop them rather than buffer unbounded. 510 + console.error(`Rejected client packet: ${err.message}`); 511 + try { socket.destroy(); } catch {} 512 + return; 513 + } 449 514 for (const packet of packets) { 450 515 switch (packet.type) { 451 516 case MessageType.ATTACH: { ··· 781 846 rows: config.rows ?? 24, 782 847 cols: config.cols ?? 80, 783 848 tags: config.tags, 849 + isolateEnv: config.isolateEnv === true, 850 + extraEnv: config.extraEnv, 784 851 onExit: (code) => { 785 852 // Give clients a moment to receive the exit message, then shut down 786 853 setTimeout(() => cleanShutdown(code), 500);

+61 -28

src/sessions.ts

··· 9 9 10 10 const VALID_NAME_RE = /^[a-zA-Z0-9._-]+$/; 11 11 12 + // Maximum bytes available to `sockaddr_un.sun_path`. Darwin/BSD = 104, 13 + // Linux = 108. Pick the smallest so the same name works everywhere. 14 + const SUN_PATH_MAX = 104; 15 + 12 16 export function validateName(name: string): void { 13 17 if (!name || name.length === 0) { 14 18 throw new Error("Session name cannot be empty."); ··· 19 23 if (!VALID_NAME_RE.test(name)) { 20 24 throw new Error( 21 25 `Invalid session name "${name}". Names may only contain letters, numbers, dots, hyphens, and underscores.` 26 + ); 27 + } 28 + 29 + // Reject names whose resulting Unix-socket path would exceed the kernel 30 + // limit. Without this check the daemon's listen() fails with EINVAL inside 31 + // an error handler that used to silently log and hang. See BUG-1. 32 + const socketPath = path.join(getSessionDir(), `${name}.sock`); 33 + const byteLen = Buffer.byteLength(socketPath, "utf-8"); 34 + if (byteLen > SUN_PATH_MAX) { 35 + const overflow = byteLen - SUN_PATH_MAX; 36 + throw new Error( 37 + `Session name "${name}" produces a socket path of ${byteLen} bytes, ` + 38 + `which exceeds the ${SUN_PATH_MAX}-byte kernel limit by ${overflow}. ` + 39 + `Shorten the name or set PTY_SESSION_DIR to a shorter path.` 22 40 ); 23 41 } 24 42 } ··· 262 280 * Acquire an exclusive lock for a session name. Prevents concurrent 263 281 * `pty run` calls from racing to create the same session. 264 282 * Returns true if acquired, false if another process holds it. 283 + * 284 + * BUG-2 fix: the whole acquisition is built on `open(O_CREAT|O_EXCL)` via 285 + * `openSync(..., "wx")`. Two racing processes can't both win because 286 + * `O_EXCL` is a kernel-level atomic create. When the lock looks stale, we 287 + * unlink it and retry the exclusive open: whichever process wins the 288 + * post-unlink open owns the lock; the other gets EEXIST and gives up. 265 289 */ 266 290 export function acquireLock(name: string): boolean { 267 291 ensureSessionDir(); 268 292 const lockPath = getLockPath(name); 269 - try { 270 - fs.writeFileSync(lockPath, process.pid.toString(), { flag: "wx" }); 271 - return true; 272 - } catch (e: any) { 273 - if (e.code === "EEXIST") { 274 - // Lock file exists — check if the holding process is still alive 275 - let shouldSteal = false; 293 + 294 + const tryCreate = (): boolean => { 295 + try { 296 + const fd = fs.openSync(lockPath, "wx", 0o600); 276 297 try { 277 - const pid = parseInt(fs.readFileSync(lockPath, "utf-8").trim(), 10); 278 - if (isNaN(pid)) { 279 - // Garbage content — treat as stale 280 - shouldSteal = true; 281 - } else { 282 - process.kill(pid, 0); // throws if process is dead 283 - return false; // process is alive, lock is valid 284 - } 285 - } catch { 286 - // Couldn't read, or holding process is dead 287 - shouldSteal = true; 298 + fs.writeSync(fd, process.pid.toString()); 299 + } finally { 300 + fs.closeSync(fd); 288 301 } 302 + return true; 303 + } catch (e: any) { 304 + if (e.code === "EEXIST") return false; 305 + throw e; 306 + } 307 + }; 289 308 290 - if (shouldSteal) { 291 - try { 292 - fs.unlinkSync(lockPath); 293 - fs.writeFileSync(lockPath, process.pid.toString(), { flag: "wx" }); 294 - return true; 295 - } catch { 296 - return false; 297 - } 298 - } 309 + if (tryCreate()) return true; 310 + 311 + // Lock file exists — inspect the holder. 312 + let holderAlive = false; 313 + try { 314 + const pid = parseInt(fs.readFileSync(lockPath, "utf-8").trim(), 10); 315 + if (!isNaN(pid)) { 316 + process.kill(pid, 0); // throws if process is dead 317 + holderAlive = true; 299 318 } 300 - return false; 319 + } catch { 320 + // Garbage content, unreadable, or holder dead → treat as stale. 321 + } 322 + 323 + if (holderAlive) return false; 324 + 325 + // Stale lock. Unlink and retry the exclusive create exactly once. If 326 + // another process is racing us to steal, only one wins the wx open; the 327 + // loser returns false instead of stomping on the winner's lock. 328 + try { 329 + fs.unlinkSync(lockPath); 330 + } catch (e: any) { 331 + // Someone else unlinked it first — that's fine, fall through to create. 332 + if (e.code !== "ENOENT") return false; 301 333 } 334 + return tryCreate(); 302 335 } 303 336 304 337 export function releaseLock(name: string): void {

+10

src/spawn.ts

··· 21 21 rows?: number; 22 22 cols?: number; 23 23 tags?: Record<string, string>; 24 + /** When true, strip the daemon's env down to a small allow-list before 25 + * spawning the session child — prevents cloud tokens / OAuth / SSH agent 26 + * vars from leaking into a session that may be reached via pty-relay. 27 + * See BUG-4. */ 28 + isolateEnv?: boolean; 29 + /** Additional `KEY=VALUE` env entries to add on top of the isolation 30 + * allow-list. Ignored unless `isolateEnv` is true. */ 31 + extraEnv?: Record<string, string>; 24 32 /** Override the runtime used to launch the detached daemon process. 25 33 * 26 34 * By default the daemon is spawned with `process.execPath` — the same ··· 58 66 cols, 59 67 ephemeral: options.ephemeral ?? false, 60 68 ...(options.tags && Object.keys(options.tags).length > 0 ? { tags: options.tags } : {}), 69 + ...(options.isolateEnv ? { isolateEnv: true } : {}), 70 + ...(options.extraEnv && Object.keys(options.extraEnv).length > 0 ? { extraEnv: options.extraEnv } : {}), 61 71 }); 62 72 63 73 const launcherCmd = options.launcher?.command ?? process.execPath;

+5 -1

src/testing/session.ts

··· 382 382 }); 383 383 384 384 backend.socket.on("data", (data: Buffer) => { 385 - const packets = backend.reader.feed(data); 385 + let packets; 386 + try { packets = backend.reader.feed(data); } catch { 387 + try { backend.socket.destroy(); } catch {} 388 + return; 389 + } 386 390 for (const packet of packets) { 387 391 switch (packet.type) { 388 392 case MessageType.SCREEN:

+5 -1

src/tui/builders.ts

··· 639 639 }; 640 640 641 641 socket.on("data", (data: Buffer) => { 642 - const packets = reader.feed(data); 642 + let packets; 643 + try { packets = reader.feed(data); } catch { 644 + try { socket.destroy(); } catch {} 645 + return; 646 + } 643 647 for (const packet of packets) { 644 648 switch (packet.type) { 645 649 case MessageType.SCREEN:

+46

tests/protocol.test.ts

··· 2 2 import { 3 3 MessageType, 4 4 PacketReader, 5 + PacketTooLargeError, 6 + MAX_PACKET_LENGTH, 5 7 encodePacket, 6 8 encodeData, 7 9 encodeAttach, ··· 14 16 decodeSize, 15 17 decodeExit, 16 18 } from "../src/protocol.ts"; 19 + import { Buffer } from "node:buffer"; 17 20 18 21 describe("protocol", () => { 19 22 describe("encodePacket / PacketReader", () => { ··· 196 199 expect(packets).toHaveLength(1); 197 200 expect(packets[0].type).toBe(MessageType.STATUS); 198 201 expect(packets[0].payload.length).toBe(0); 202 + }); 203 + 204 + it("rejects packets whose declared length exceeds MAX_PACKET_LENGTH (BUG-3)", () => { 205 + const reader = new PacketReader(); 206 + // Craft a header with length = MAX + 1 and no payload 207 + const header = Buffer.alloc(5); 208 + header.writeUInt8(MessageType.DATA, 0); 209 + header.writeUInt32BE(MAX_PACKET_LENGTH + 1, 1); 210 + 211 + expect(() => reader.feed(header)).toThrow(PacketTooLargeError); 212 + }); 213 + 214 + it("rejects the max-uint32 length (worst case attack)", () => { 215 + const reader = new PacketReader(); 216 + const header = Buffer.alloc(5); 217 + header.writeUInt8(MessageType.DATA, 0); 218 + header.writeUInt32BE(0xffffffff, 1); 219 + 220 + expect(() => reader.feed(header)).toThrow(PacketTooLargeError); 221 + }); 222 + 223 + it("poisons the buffer after oversize throw (subsequent feeds don't buffer unbounded)", () => { 224 + const reader = new PacketReader(); 225 + const header = Buffer.alloc(5); 226 + header.writeUInt8(MessageType.DATA, 0); 227 + header.writeUInt32BE(0xffffffff, 1); 228 + 229 + try { reader.feed(header); } catch {} 230 + // Buffer is cleared, so subsequent valid packets parse correctly from 231 + // the new boundary (not treating their bytes as payload of the bad one). 232 + const packets = reader.feed(encodeData("hi")); 233 + expect(packets).toHaveLength(1); 234 + expect(packets[0].payload.toString()).toBe("hi"); 235 + }); 236 + 237 + it("accepts packets at exactly MAX_PACKET_LENGTH", () => { 238 + // Build a valid packet with length exactly at the cap (but small payload; 239 + // we just want the length field to be valid). 240 + const reader = new PacketReader(); 241 + const encoded = encodeData("ok"); 242 + const packets = reader.feed(encoded); 243 + expect(packets).toHaveLength(1); 244 + expect(packets[0].payload.toString()).toBe("ok"); 199 245 }); 200 246 201 247 it("round-trips a STATUS response (JSON payload)", () => {

+88

tests/security-fixes.test.ts

··· 1 + import { describe, it, expect, beforeEach, afterEach } from "vitest"; 2 + import * as fs from "node:fs"; 3 + import * as os from "node:os"; 4 + import * as path from "node:path"; 5 + import { validateName, acquireLock, releaseLock, getSessionDir } from "../src/sessions.ts"; 6 + 7 + // These tests pin BUG-1 (socket path length) and BUG-2 (acquireLock race). 8 + 9 + const origSessionDir = process.env.PTY_SESSION_DIR; 10 + let tmp: string; 11 + 12 + beforeEach(() => { 13 + tmp = fs.mkdtempSync(path.join(os.tmpdir(), "pty-sec-")); 14 + process.env.PTY_SESSION_DIR = tmp; 15 + }); 16 + 17 + afterEach(() => { 18 + if (origSessionDir === undefined) delete process.env.PTY_SESSION_DIR; 19 + else process.env.PTY_SESSION_DIR = origSessionDir; 20 + try { fs.rmSync(tmp, { recursive: true, force: true }); } catch {} 21 + }); 22 + 23 + describe("BUG-1: validateName rejects oversized socket paths", () => { 24 + it("accepts ordinary names", () => { 25 + expect(() => validateName("myserver")).not.toThrow(); 26 + }); 27 + 28 + it("rejects names that would overflow the 104-byte sun_path limit", () => { 29 + // Session dir is a reasonably long tmpdir path; append a 100-char name. 30 + const longName = "a".repeat(100); 31 + expect(() => validateName(longName)).toThrow(/socket path.*exceeds/); 32 + }); 33 + 34 + it("rejects names whose path hits the limit exactly +1", () => { 35 + const dir = getSessionDir(); 36 + const overhead = Buffer.byteLength(path.join(dir, ".sock"), "utf-8"); 37 + const overshootName = "a".repeat(Math.max(1, 104 - overhead + 1)); 38 + expect(() => validateName(overshootName)).toThrow(/socket path/); 39 + }); 40 + 41 + it("still rejects bad characters before checking length", () => { 42 + expect(() => validateName("has/slash")).toThrow(/Invalid session name/); 43 + }); 44 + }); 45 + 46 + describe("BUG-2: acquireLock atomic semantics", () => { 47 + it("first caller wins, second returns false while holder is alive", () => { 48 + expect(acquireLock("race1")).toBe(true); 49 + expect(acquireLock("race1")).toBe(false); 50 + releaseLock("race1"); 51 + }); 52 + 53 + it("steals a stale lock whose holder pid is dead", () => { 54 + // Write a lock file with a dead PID. 55 + const lockPath = path.join(tmp, "race2.lock"); 56 + fs.writeFileSync(lockPath, "1"); // PID 1 exists, but... 57 + // Use a pid we know is dead: INT_MAX 58 + fs.writeFileSync(lockPath, "2147483646"); 59 + 60 + expect(acquireLock("race2")).toBe(true); 61 + // Now holder should be us 62 + const pid = parseInt(fs.readFileSync(lockPath, "utf-8"), 10); 63 + expect(pid).toBe(process.pid); 64 + releaseLock("race2"); 65 + }); 66 + 67 + it("garbage lock content is treated as stale", () => { 68 + fs.writeFileSync(path.join(tmp, "race3.lock"), "not a pid"); 69 + expect(acquireLock("race3")).toBe(true); 70 + releaseLock("race3"); 71 + }); 72 + 73 + it("release is idempotent (unlinking a missing lock is fine)", () => { 74 + expect(() => releaseLock("never-locked")).not.toThrow(); 75 + }); 76 + 77 + it("acquireLock uses O_EXCL — concurrent wx opens can't both win", () => { 78 + // Simulate two concurrent stealers: create stale lock, then race two 79 + // acquireLock calls synchronously. We can't truly parallelize here, but we 80 + // can prove that after one win, a subsequent call to acquireLock against 81 + // the live pid returns false. 82 + fs.writeFileSync(path.join(tmp, "race4.lock"), "2147483646"); 83 + const a = acquireLock("race4"); 84 + const b = acquireLock("race4"); 85 + expect([a, b].filter(Boolean).length).toBe(1); 86 + releaseLock("race4"); 87 + }); 88 + });

+63

tests/spawn-options.test.ts

··· 217 217 expect(recorded[1]).toMatch(/server\.js$/); 218 218 }, 15000); 219 219 220 + it("--isolate-env scrubs inherited environment variables from the session child (BUG-4)", async () => { 221 + const dir = makeSessionDir(); 222 + const name = uniqueName(); 223 + 224 + // Run the child with a command that prints its env, set a secret var that 225 + // should NOT propagate to the isolated child. `pty run -d` spawns the 226 + // session, then we peek. 227 + const secret = "pty_isolated_test_secret_must_not_leak"; 228 + 229 + const runResult = spawnSync(nodeBin, [ 230 + cliPath, "run", "-d", "--name", name, "--isolate-env", 231 + "--", "sh", "-c", "env > /tmp/pty-iso-env.txt; sleep 30", 232 + ], { 233 + env: { ...process.env, PTY_SESSION_DIR: dir, PTY_SECRET_TEST: secret }, 234 + encoding: "utf-8", 235 + timeout: 10000, 236 + }); 237 + expect(runResult.status).toBe(0); 238 + 239 + // Give the child a tick to write. 240 + await new Promise((r) => setTimeout(r, 500)); 241 + 242 + const dumped = fs.readFileSync("/tmp/pty-iso-env.txt", "utf-8"); 243 + expect(dumped).not.toContain("PTY_SECRET_TEST"); 244 + expect(dumped).toContain("PATH="); // PATH still propagates 245 + expect(dumped).toContain(`PTY_SESSION=${name}`); // set unconditionally 246 + 247 + // Clean up 248 + const pidFile = path.join(dir, `${name}.pid`); 249 + try { 250 + const pid = parseInt(fs.readFileSync(pidFile, "utf-8").trim(), 10); 251 + bgPids.push(pid); 252 + } catch {} 253 + try { fs.unlinkSync("/tmp/pty-iso-env.txt"); } catch {} 254 + }, 15000); 255 + 256 + it("without --isolate-env, custom env vars propagate to the session child (legacy behaviour)", async () => { 257 + const dir = makeSessionDir(); 258 + const name = uniqueName(); 259 + 260 + const marker = "pty_legacy_env_test_marker"; 261 + const runResult = spawnSync(nodeBin, [ 262 + cliPath, "run", "-d", "--name", name, 263 + "--", "sh", "-c", "env > /tmp/pty-legacy-env.txt; sleep 30", 264 + ], { 265 + env: { ...process.env, PTY_SESSION_DIR: dir, PTY_LEGACY_MARKER: marker }, 266 + encoding: "utf-8", 267 + timeout: 10000, 268 + }); 269 + expect(runResult.status).toBe(0); 270 + await new Promise((r) => setTimeout(r, 500)); 271 + 272 + const dumped = fs.readFileSync("/tmp/pty-legacy-env.txt", "utf-8"); 273 + expect(dumped).toContain(`PTY_LEGACY_MARKER=${marker}`); 274 + 275 + const pidFile = path.join(dir, `${name}.pid`); 276 + try { 277 + const pid = parseInt(fs.readFileSync(pidFile, "utf-8").trim(), 10); 278 + bgPids.push(pid); 279 + } catch {} 280 + try { fs.unlinkSync("/tmp/pty-legacy-env.txt"); } catch {} 281 + }, 15000); 282 + 220 283 it("surfaces a missing cwd explicitly instead of failing silently", async () => { 221 284 const dir = makeSessionDir(); 222 285 const name = uniqueName();

Configure Feed

Configure Feed