From 7042ca04be4389f065680cb43f7e0e6647f7c102 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Mon, 13 Apr 2026 12:22:34 +0200 Subject: [PATCH 1/2] fix: harden darwin runtime checks Change-Id: Iaeca2c0da8ed798a17be726622c8f82fb4eaefec Signed-off-by: Thomas Kosiewski --- src/cli/commands/doctor.ts | 16 ++++- src/host/hostMain.ts | 7 +- src/pty/createPty.ts | 56 +++++++++++++++ src/renderer/browserPath.ts | 2 +- src/storage/sessionPaths.ts | 87 +++++++++++++++++++++-- test/integration/renderer-backend.test.ts | 13 +++- test/unit/commands/doctor.test.ts | 11 ++- test/unit/storage/sessionPaths.test.ts | 7 +- 8 files changed, 181 insertions(+), 18 deletions(-) diff --git a/src/cli/commands/doctor.ts b/src/cli/commands/doctor.ts index 3f6feb0..efe17d4 100644 --- a/src/cli/commands/doctor.ts +++ b/src/cli/commands/doctor.ts @@ -19,7 +19,7 @@ import { type Socket, } from 'node:net'; import { homedir, tmpdir } from 'node:os'; -import { join, normalize } from 'node:path'; +import { dirname, join, normalize } from 'node:path'; import process from 'node:process'; import type { CommandContext } from '../context.js'; @@ -28,6 +28,7 @@ import { emitSuccess } from '../output.js'; import type { CapabilityEntry } from '../../renderer/capabilities.js'; import { createPty } from '../../pty/createPty.js'; +import { resolveDefaultPlaywrightBrowsersPath } from '../../renderer/browserPath.js'; import { discoverCapabilities } from '../../renderer/capabilities.js'; import { artifactPath, @@ -217,7 +218,16 @@ function resolvePlaywrightBrowserCachePath(): string { return normalize(overridePath); } - return join(resolveSystemHomeDirectory(), '.cache', 'ms-playwright'); + const browserCachePath = resolveDefaultPlaywrightBrowsersPath( + resolveSystemHomeDirectory(), + process.platform, + ); + assert( + browserCachePath !== null, + `unsupported platform for default Playwright browser cache resolution: ${process.platform}`, + ); + + return normalize(browserCachePath); } function getDoctorDependencies( @@ -533,6 +543,8 @@ export async function runSocketViabilityCheck( overrides, async (sessionDirectory, deps) => { const socketFile = socketPath(sessionDirectory); + await deps.mkdir(dirname(socketFile), { recursive: true }); + let server: Server | null = null; let client: Socket | null = null; let acceptedConnection = false; diff --git a/src/host/hostMain.ts b/src/host/hostMain.ts index 6d4b47b..39b0e74 100644 --- a/src/host/hostMain.ts +++ b/src/host/hostMain.ts @@ -1,5 +1,6 @@ import crypto from 'node:crypto'; -import { rename, rm } from 'node:fs/promises'; +import { mkdir, rename, rm } from 'node:fs/promises'; +import { dirname } from 'node:path'; import process from 'node:process'; import { ulid } from 'ulid'; @@ -666,8 +667,7 @@ export async function runHost(sessionId: string): Promise { return `${command}\nprintf '%s%s\\n' '${markerPart1}' '${markerPart2}'\n`; })() : `${command}\n`; - const encoded = `${encodePaste(injectedText)}${encodeKey('enter')}`; - pty.write(encoded); + pty.write(injectedText); lastActivityAt = Date.now(); const seq = await eventLog.append('input_run', { @@ -1247,6 +1247,7 @@ export async function runHost(sessionId: string): Promise { try { await writeManifest(mPath, state.snapshot()); + await mkdir(dirname(sPath), { recursive: true }); if (!isSessionRunning(state)) { await initiateShutdown(); diff --git a/src/pty/createPty.ts b/src/pty/createPty.ts index 95da38d..e3cd44e 100644 --- a/src/pty/createPty.ts +++ b/src/pty/createPty.ts @@ -1,3 +1,6 @@ +import { chmodSync, statSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { dirname, join } from 'node:path'; import process from 'node:process'; import type { IPty } from 'node-pty'; @@ -13,6 +16,57 @@ export interface PtyOptions { term: string; } +const EXECUTABLE_PERMISSION_MASK = 0o111; +const require = createRequire(import.meta.url); +const NODE_PTY_PACKAGE_DIRECTORY = dirname( + require.resolve('node-pty/package.json'), +); + +function resolveDarwinSpawnHelperPath(): string | null { + if (process.platform !== 'darwin') { + return null; + } + + const prebuildDirectory = + process.arch === 'arm64' + ? 'darwin-arm64' + : process.arch === 'x64' + ? 'darwin-x64' + : null; + if (prebuildDirectory === null) { + return null; + } + + return join( + NODE_PTY_PACKAGE_DIRECTORY, + 'prebuilds', + prebuildDirectory, + 'spawn-helper', + ); +} + +function ensureDarwinSpawnHelperExecutable(): void { + const helperPath = resolveDarwinSpawnHelperPath(); + if (helperPath === null) { + return; + } + + try { + const helperStats = statSync(helperPath); + if (!helperStats.isFile()) { + return; + } + + if ((helperStats.mode & EXECUTABLE_PERMISSION_MASK) !== 0) { + return; + } + + chmodSync(helperPath, helperStats.mode | EXECUTABLE_PERMISSION_MASK); + } catch { + // Best-effort repair; node-pty will still surface a clear spawn failure. + } +} + export function createPty(options: PtyOptions): IPty { const { command, cwd, cols, rows, env, term } = options; @@ -35,6 +89,8 @@ export function createPty(options: PtyOptions): IPty { invariant(typeof entryValue === 'string', 'PTY env values must be strings'); } + ensureDarwinSpawnHelperExecutable(); + return spawn(file, command.slice(1), { cwd, cols, diff --git a/src/renderer/browserPath.ts b/src/renderer/browserPath.ts index c3530a2..d3699ff 100644 --- a/src/renderer/browserPath.ts +++ b/src/renderer/browserPath.ts @@ -24,7 +24,7 @@ function isNonEmptyString(value: unknown): value is string { return typeof value === 'string' && value.length > 0; } -function resolveDefaultPlaywrightBrowsersPath( +export function resolveDefaultPlaywrightBrowsersPath( capturedHome: string, platform: NodeJS.Platform, ): string | null { diff --git a/src/storage/sessionPaths.ts b/src/storage/sessionPaths.ts index 5ff6a70..abaf6a9 100644 --- a/src/storage/sessionPaths.ts +++ b/src/storage/sessionPaths.ts @@ -1,12 +1,13 @@ -import { dirname, isAbsolute, resolve } from 'node:path'; +import crypto from 'node:crypto'; +import { basename, dirname, isAbsolute, resolve } from 'node:path'; -import { - EVENT_LOG_FILENAME, - MANIFEST_FILENAME, - SOCKET_FILENAME, -} from '../config/defaults.js'; +import { EVENT_LOG_FILENAME, MANIFEST_FILENAME } from '../config/defaults.js'; import { invariant } from '../util/assert.js'; +const SOCKET_ROOT_DIRECTORY = '/tmp/agent-tty'; +const SOCKET_HOME_ID_LENGTH = 8; +const SOCKET_ID_LENGTH = 12; + function assertNonEmptyString( value: string, label: string, @@ -66,6 +67,78 @@ export function eventLogPath(sessionDirectory: string): string { return childPath(sessionDirectory, EVENT_LOG_FILENAME); } +function resolveSocketDirectory(home: string): string { + assertAbsolutePath(home, 'home'); + + const directory = resolve( + SOCKET_ROOT_DIRECTORY, + crypto + .createHash('sha256') + .update(resolve(home)) + .digest('hex') + .slice(0, SOCKET_HOME_ID_LENGTH), + ); + invariant( + dirname(directory) === resolve(SOCKET_ROOT_DIRECTORY), + 'socket directory must stay within the socket root directory', + ); + invariant( + basename(directory).length === SOCKET_HOME_ID_LENGTH, + 'socket home identifier must have the expected length', + ); + + return directory; +} + +function deriveSessionIdentity(sessionDirectory: string): { + home: string; + sessionId: string; +} { + assertAbsolutePath(sessionDirectory, 'sessionDir'); + + const normalizedSessionDirectory = resolve(sessionDirectory); + const sessionId = basename(normalizedSessionDirectory); + assertSessionId(sessionId); + + const sessionsRoot = dirname(normalizedSessionDirectory); + const home = dirname(sessionsRoot); + + invariant( + sessionsRoot === resolve(home, 'sessions'), + 'session directory must stay within the sessions root', + ); + + return { + home, + sessionId, + }; +} + +function socketFileId(sessionId: string): string { + assertSessionId(sessionId); + + const digest = crypto + .createHash('sha256') + .update(sessionId) + .digest('hex') + .slice(0, SOCKET_ID_LENGTH); + invariant( + digest.length === SOCKET_ID_LENGTH, + 'socket file identifier must have the expected length', + ); + + return digest; +} + export function socketPath(sessionDirectory: string): string { - return childPath(sessionDirectory, SOCKET_FILENAME); + const { home, sessionId } = deriveSessionIdentity(sessionDirectory); + const socketDirectory = resolveSocketDirectory(home); + const socketFile = resolve(socketDirectory, socketFileId(sessionId)); + + invariant( + dirname(socketFile) === socketDirectory, + 'socket path must stay within the socket directory', + ); + + return socketFile; } diff --git a/test/integration/renderer-backend.test.ts b/test/integration/renderer-backend.test.ts index eb31064..9a53011 100644 --- a/test/integration/renderer-backend.test.ts +++ b/test/integration/renderer-backend.test.ts @@ -4,6 +4,7 @@ import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { resolveDefaultPlaywrightBrowsersPath } from '../../src/renderer/browserPath.js'; import { resolveProfile } from '../../src/renderer/profiles.js'; import type { ReplayInput } from '../../src/renderer/types.js'; import { GhosttyWebBackend } from '../../src/renderer/ghosttyWeb/index.js'; @@ -127,9 +128,19 @@ describe('GhosttyWebBackend integration', { timeout: 120_000 }, () => { await backend.boot(); + const expectedBrowserCachePath = resolveDefaultPlaywrightBrowsersPath( + previousHome, + process.platform, + ); + if (expectedBrowserCachePath === null) { + throw new Error( + `expected a default Playwright browser cache path for ${process.platform}`, + ); + } + expect(backend.isBooted).toBe(true); expect(process.env.PLAYWRIGHT_BROWSERS_PATH).toBe( - join(previousHome, '.cache', 'ms-playwright'), + expectedBrowserCachePath, ); } finally { if (previousBrowsersPath === undefined) { diff --git a/test/unit/commands/doctor.test.ts b/test/unit/commands/doctor.test.ts index 0d5d3f7..5d54fb6 100644 --- a/test/unit/commands/doctor.test.ts +++ b/test/unit/commands/doctor.test.ts @@ -17,6 +17,7 @@ import { runSocketViabilityCheck, type DoctorDependencies, } from '../../../src/cli/commands/doctor.js'; +import { resolveDefaultPlaywrightBrowsersPath } from '../../../src/renderer/browserPath.js'; const QUICK_TIMEOUT_MS = 5_000; @@ -278,7 +279,15 @@ describe('doctor command', () => { const systemHome = await realpath( await mkdtemp(join(tmpdir(), 'agent-tty-system-home-')), ); - const browserCachePath = join(systemHome, '.cache', 'ms-playwright'); + const browserCachePath = resolveDefaultPlaywrightBrowsersPath( + systemHome, + process.platform, + ); + if (browserCachePath === null) { + throw new Error( + `expected a default Playwright browser cache path for ${process.platform}`, + ); + } delete process.env.PLAYWRIGHT_BROWSERS_PATH; process.env.HOME = systemHome; await mkdir(join(browserCachePath, 'chromium-1234'), { recursive: true }); diff --git a/test/unit/storage/sessionPaths.test.ts b/test/unit/storage/sessionPaths.test.ts index 2ec28a0..2649228 100644 --- a/test/unit/storage/sessionPaths.test.ts +++ b/test/unit/storage/sessionPaths.test.ts @@ -1,3 +1,4 @@ +import crypto from 'node:crypto'; import { mkdtemp, realpath, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -48,7 +49,9 @@ afterEach(async () => { describe('session paths', () => { it('builds session-specific absolute paths', () => { const home = '/tmp/agent-tty-home'; + const sessionId = 'session-01'; const directory = sessionDir(home, 'session-01'); + const expectedSocketPath = `/tmp/agent-tty/${crypto.createHash('sha256').update(home).digest('hex').slice(0, 8)}/${crypto.createHash('sha256').update(sessionId).digest('hex').slice(0, 12)}`; expect(directory).toBe('/tmp/agent-tty-home/sessions/session-01'); expect(manifestPath(directory)).toBe( @@ -57,9 +60,7 @@ describe('session paths', () => { expect(eventLogPath(directory)).toBe( '/tmp/agent-tty-home/sessions/session-01/events.jsonl', ); - expect(socketPath(directory)).toBe( - '/tmp/agent-tty-home/sessions/session-01/host.sock', - ); + expect(socketPath(directory)).toBe(expectedSocketPath); }); it('asserts on invalid path helper inputs', () => { From eae56c1cc5a17e63221c4398fc2d78be03f1ae06 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Mon, 13 Apr 2026 13:22:37 +0200 Subject: [PATCH 2/2] ci: make git install smoke opt-in Change-Id: I078d05a4c4f6b6e28501e0431c2aeb6c199a38e5 Signed-off-by: Thomas Kosiewski --- scripts/smoke-install.mjs | 69 +++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/scripts/smoke-install.mjs b/scripts/smoke-install.mjs index bae4f94..c6de9fc 100755 --- a/scripts/smoke-install.mjs +++ b/scripts/smoke-install.mjs @@ -12,13 +12,14 @@ import { packRelease } from './pack-release.mjs'; const projectRoot = resolve(fileURLToPath(new URL('..', import.meta.url))); const npmCliPath = process.env.npm_execpath; -const supportedArgs = new Set(['--skip-build']); +const supportedArgs = new Set(['--exercise-git-install', '--skip-build']); for (const argument of process.argv.slice(2)) { assert(supportedArgs.has(argument), `unsupported argument: ${argument}`); } const skipBuild = process.argv.includes('--skip-build'); +const exerciseGitInstall = process.argv.includes('--exercise-git-install'); const packageJson = JSON.parse( await readFile(join(projectRoot, 'package.json'), 'utf8'), @@ -437,44 +438,48 @@ try { ]); await verifyInstalledCli('Tarball', tarballInstallPrefix); - logStep( - 'Preparing clean git source to exercise npm prepare for git installs...', - ); - const gitInstallPrefix = join(tempRoot, 'git-prefix'); - const { gitUrl } = await createGitInstallSource(); + if (exerciseGitInstall) { + logStep( + 'Preparing clean git source to exercise npm prepare for git installs...', + ); + const gitInstallPrefix = join(tempRoot, 'git-prefix'); + const { gitUrl } = await createGitInstallSource(); - logStep('Installing from git dependency URL into isolated prefix...'); - const gitInstallResult = runNpm( - ['install', '-g', '--prefix', gitInstallPrefix, gitUrl], - { allowFailure: true }, - ); + logStep('Installing from git dependency URL into isolated prefix...'); + const gitInstallResult = runNpm( + ['install', '-g', '--prefix', gitInstallPrefix, gitUrl], + { allowFailure: true }, + ); - if (gitInstallResult.status === 0) { - await verifyInstalledCli('Git', gitInstallPrefix); - logStep('Git dependency install route succeeded.'); + if (gitInstallResult.status === 0) { + await verifyInstalledCli('Git', gitInstallPrefix); + logStep('Git dependency install route succeeded.'); + } else { + assert( + isKnownGitInstallCaveat(gitInstallResult), + [ + 'git dependency install failed in an unexpected way', + gitInstallResult.stdout.length === 0 + ? '' + : `stdout:\n${gitInstallResult.stdout}`, + gitInstallResult.stderr.length === 0 + ? '' + : `stderr:\n${gitInstallResult.stderr}`, + ] + .filter((line) => line.length > 0) + .join('\n\n'), + ); + logStep( + 'Git dependency install matched the known caveat path; tarball fallback remains the guaranteed route.', + ); + } } else { - assert( - isKnownGitInstallCaveat(gitInstallResult), - [ - 'git dependency install failed in an unexpected way', - gitInstallResult.stdout.length === 0 - ? '' - : `stdout:\n${gitInstallResult.stdout}`, - gitInstallResult.stderr.length === 0 - ? '' - : `stderr:\n${gitInstallResult.stderr}`, - ] - .filter((line) => line.length > 0) - .join('\n\n'), - ); logStep( - 'Git dependency install matched the known caveat path; tarball fallback remains the guaranteed route.', + 'Skipping git dependency install route; tarball install is the supported packaging path.', ); } - logStep( - 'Packaging smoke passed: tarball route succeeded, and the current git-install behavior was validated.', - ); + logStep('Packaging smoke passed: tarball route succeeded.'); } finally { await rm(tempRoot, { recursive: true, force: true }); }