From a7a8377658aab7aefac8c385946657b4be3c7404 Mon Sep 17 00:00:00 2001 From: pcontrerasp Date: Fri, 14 Aug 2026 04:08:01 -0700 Subject: [PATCH] fix(macos): grant the inherited terminals so sandboxed TUIs can enter raw mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seatbelt matches ioctl rules by device path. The generated profile allowed file-ioctl on the literals /dev/tty, /dev/null, /dev/zero, /dev/random, /dev/urandom and /dev/dtracehelper, but a terminal is a pty slave (/dev/ttysNNN) that the /dev/tty alias does not cover. TIOCSETA/TIOCSETAW therefore returned EPERM and no interactive program could enter raw mode. Both filed symptoms follow from that single cause. With the terminal stuck in canonical+ECHO mode it echoes whatever arrives on its input back to the display, so the application's own protocol traffic becomes visible text: capability replies such as XTVERSION and DA1 (#419), and the Kitty Keyboard Protocol encodings and mouse reports that appear when a key is pressed (#391). Input stays line-buffered in both. allowPty already existed and fixed this, but it is documented nowhere, so #419 was filed eight months after it shipped concluding no workaround existed. It is also wider than the common case needs: (allow pseudo-tty) plus read, write and ioctl over every /dev/ttys, which reaches other terminals the same user owns. Left unset, the profile now grants file-ioctl — and only file-ioctl, which is all raw mode needs — on the terminals the child inherits, and does not emit pseudo-tty, so allocating new ptys stays opt-in. Every distinct device across fds 0, 1 and 2 gets a rule, because stdio can span two terminals and granting only the first leaves the other returning EPERM. Paths that are not pty slaves are filtered out rather than trusted, since the parameter is exported and the value reaches a (literal ...) rule. allowPty: true keeps the existing broad rules for tmux, script, expect and node-pty; allowPty: false behaves identically to unset — inherited-terminal ioctl only, no wide grant — so an explicit false cannot silently reproduce the raw-mode bug this fixes; only allowPty: true is special. There is deliberately no "no pty rules" state: an inherited terminal a process never touches is harmless to make ioctl-able. The terminals are never detected while wrapping. Wrapping returns a command string and the caller chooses stdio afterwards, so a terminal detected then is a guess about a decision not yet made: wrap under pipes and launch into a fresh pty and the rule is missing, wrap under terminal A and launch under B and the rule names the wrong device. WrapWithSandboxOptions gains inheritsStdio, an assertion by a caller that it spawns with this process's stdio inherited, and only that resolves them. The CLI passes it; library consumers opt in. Node exposes no ttyname(3) and fs.realpathSync('/dev/fd/0') returns '/dev/fd/0' on macOS rather than the device, so devices are resolved by matching the device number of each tty descriptor against the /dev/ttys* entries. Every failure path logs, because a silent one reproduces exactly the bug this fixes: no rule, no error, no raw mode. Verified on a pty made a genuine controlling terminal (setsid + TIOCSCTTY): raw mode succeeds with no allowPty key; the terminal reports -echo, which is #391's mechanism; injecting the KKP Ctrl+C sequence is no longer echoed back as literal text, where the pre-fix path returns it verbatim as ^[[99;5u; and TIOCSTI keystroke injection stays denied in both modes. That denial is this profile's doing and is pinned by a test rather than asserted in prose — macOS itself permits TIOCSTI to an unprivileged process when the descriptor is its own controlling terminal. Also corrects the violation-monitoring predicate in the README: sandbox-exec execs into the target, so violations are attributed to the child and a predicate on the sandbox-exec process name matches nothing. Reported as #419 and #391. --- README.md | 8 +- src/cli.ts | 11 +- src/sandbox/macos-sandbox-utils.ts | 161 ++++++++- src/sandbox/sandbox-config.ts | 9 +- src/sandbox/sandbox-manager.ts | 27 ++ test/helpers/pty-ctty.py | 82 +++++ test/helpers/pty-kkp.py | 109 ++++++ test/helpers/pty-split.py | 71 ++++ test/sandbox/macos-pty-default.test.ts | 468 +++++++++++++++++++++++++ 9 files changed, 941 insertions(+), 5 deletions(-) create mode 100644 test/helpers/pty-ctty.py create mode 100644 test/helpers/pty-kkp.py create mode 100644 test/helpers/pty-split.py create mode 100644 test/sandbox/macos-pty-default.test.ts diff --git a/README.md b/README.md index 4a20ab77..dd61fd98 100644 --- a/README.md +++ b/README.md @@ -389,6 +389,10 @@ Examples: - `enableWeakerNestedSandbox` - Enable weaker sandbox mode for Docker environments (boolean, default: false) - `enableWeakerNetworkIsolation` - Allow access to `com.apple.trustd.agent` in the macOS sandbox (boolean, default: false). This is needed for Go programs (`gh`, `gcloud`, `terraform`, `kubectl`, etc.) to verify TLS certificates when using `httpProxyPort` with a MITM proxy and custom CA. **Security warning:** enabling this opens a potential data exfiltration vector through the trustd service. - `allowAppleEvents` - Allow sending Apple Events and Launch Services open requests from the macOS sandbox (boolean, default: false). Without this, commands like `open`, `osascript`, and anything that opens URLs or scripts other apps via AppleScript fail with AppleScript error `-600` ("Application isn't running") or LaunchServices errors (`-10822`, `-54`). **Security warning:** enabling this means the sandbox no longer provides code-execution isolation. A sandboxed command can launch other applications via `open` with no user prompt, and anything it launches runs outside the sandbox's filesystem and network restrictions; scripting already-running apps via Apple Events is additionally gated by the user's per-app TCC automation consent. Embedders should only source this option from trusted user-level configuration — never from project-local files in a checked-out repository, which would let an attacker-authored project elevate its own sandbox permissions. +- `allowPty` - Pseudo-terminal access in the macOS sandbox (boolean, macOS only). **Leave it unset** for the default: the sandboxed process is granted `file-ioctl` — and only `file-ioctl` — on the terminals it inherited on stdin/stdout/stderr, which is what an interactive TUI needs to enter raw mode (`tcsetattr` / `process.stdin.setRawMode()`). Without it the terminal never leaves canonical mode and capability replies and mouse events echo as literal text. Each inherited terminal gets its own rule. + - `true` widens the grant to `(allow pseudo-tty)` plus read/write/ioctl on **every** pty. Programs that allocate their own pty need this: `tmux`, `script`, `expect`, anything built on `node-pty`. + - `false` behaves the same as unset — inherited-terminal `file-ioctl` only. To give the process no terminal at all, spawn it with piped stdio; the default then emits nothing. + - **Library callers must opt in.** The `srt` CLI gets this automatically (it spawns with `stdio: 'inherit'`). `wrapWithSandbox()` and `wrapWithSandboxArgv()` return a command you spawn yourself, so they emit no terminal rule unless you pass `{ inheritsStdio: true }`. ### Common Configuration Recipes @@ -733,9 +737,11 @@ When a sandboxed process attempts to access a restricted resource: ```bash # View sandbox violations in real-time -log stream --predicate 'process == "sandbox-exec"' --style syslog +log stream --predicate 'eventMessage CONTAINS "deny("' --style syslog ``` +Match on the message, not on `process == "sandbox-exec"`: `sandbox-exec` execs into the target, so the kernel attributes each violation to the child that hit it (`node`, `bash`, the sandboxed binary), and a predicate on the `sandbox-exec` process name returns nothing. + **Linux**: Bubblewrap doesn't provide built-in violation reporting. Use `strace` to trace system calls and identify blocked operations: ```bash diff --git a/src/cli.ts b/src/cli.ts index 5d65450a..42a0154b 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -293,6 +293,8 @@ async function main(): Promise { let child if (process.platform === 'win32') { // env carries the proxy vars the sandboxed child must inherit. + // No inheritsStdio here: it only drives the macOS terminal grant, + // and this branch is Windows-only. const { argv, env } = await SandboxManager.wrapWithSandboxArgv(command) child = spawn(argv[0], argv.slice(1), { @@ -301,8 +303,13 @@ async function main(): Promise { env, }) } else { - const sandboxedCommand = - await SandboxManager.wrapWithSandbox(command) + const sandboxedCommand = await SandboxManager.wrapWithSandbox( + command, + undefined, + undefined, + undefined, + { inheritsStdio: true }, + ) child = spawn(sandboxedCommand, { shell: true, stdio: 'inherit', diff --git a/src/sandbox/macos-sandbox-utils.ts b/src/sandbox/macos-sandbox-utils.ts index a23dee5f..d8f55453 100644 --- a/src/sandbox/macos-sandbox-utils.ts +++ b/src/sandbox/macos-sandbox-utils.ts @@ -1,6 +1,8 @@ import { quote } from '../utils/shell-quote.js' import { spawn } from 'child_process' +import * as fs from 'fs' import * as path from 'path' +import * as tty from 'tty' import { logForDebugging } from '../utils/debug.js' import { whichSync } from '../utils/which.js' import { @@ -57,7 +59,32 @@ export interface MacOSSandboxParams { */ maskedFileBinds?: Array<{ realPath: string; fakePath: string }> ignoreViolations?: IgnoreViolationsConfig | undefined + /** + * Pseudo-terminal access. Leave unset for the default: the terminals the + * child inherits are made ioctl-able and nothing else is, so an + * interactive TUI can enter raw mode. `true` widens that to every pty, + * which programs that allocate their own (tmux, script, node-pty) need. + * `false` behaves identically to unset — both grant only inherited-terminal + * ioctl; only `true` is special. (There is intentionally no "no pty rules" + * state: an inherited terminal a process never touches is harmless to make + * ioctl-able, and a distinct `false` was a footgun that reproduced the + * raw-mode bug the default fixes.) + */ allowPty?: boolean + /** + * Terminals to grant `file-ioctl` on in the default (`allowPty` not `true`) + * mode. Always caller-supplied and never defaulted: this function builds a + * command string, and whoever spawns it decides the child's stdio + * afterwards, so a terminal detected here would be a guess. Callers that + * know the child inherits their stdio resolve them with + * {@link resolveInheritedStdioTtys} — `SandboxManager` does this for + * `inheritsStdio`, and tests inject paths directly. + * + * Entries that are not pty slave devices are ignored: the value reaches a + * `(literal ...)` rule, and the parameter is exported, so the shape is + * enforced here rather than trusted. + */ + inheritedTtys?: string[] allowGitConfig?: boolean /** * Directories to emit as `safe.directory` via `GIT_CONFIG_*` env @@ -632,6 +659,95 @@ function generateWriteRules( return rules } +/** + * Resolve every distinct pty this process holds on stdin, stdout or stderr, + * in that order. Empty when none of them is one. + * + * All three, not just the first: a process can be launched with its stdio + * split across different terminals, and a program that reads keys from one + * while sizing the other (TIOCGWINSZ) needs both. Emitting a rule per device + * costs one profile line each. + * + * Deliberately *not* named for the controlling terminal: a process can own a + * controlling terminal while its stdio is redirected, and can hold tty stdio + * that is not its controlling terminal. What matters here is the device a + * child would inherit, which is exactly this. + * + * Seatbelt matches ioctl rules on the device's path, and a terminal is a pty + * slave (`/dev/ttysNNN`) — the `/dev/tty` alias in the base profile does not + * cover it. Node exposes no `ttyname(3)`, and `/dev/fd/N` does not resolve to + * the device on macOS, so the device number of the inherited descriptor is + * matched against the `/dev/ttys*` entries. + */ +/** + * The filesystem/tty probes {@link resolveInheritedStdioTtys} needs, behind an + * interface so the rdev-matching logic can be unit-tested in-process with + * synthetic inputs. Exercising it for real requires a genuine pty on fd 0/1/2, + * which a test runner does not have; injecting these lets a unit test drive the + * matched, no-match, dedup and every error path without one, while production + * uses {@link REAL_TTY_PROBES}. The end-to-end tests still verify the real fs. + */ +export interface TtyProbes { + isatty: (fd: number) => boolean + /** Names under `/dev` that begin with `ttys`. */ + listPtySlaves: () => string[] + rdevOfFd: (fd: number) => number + rdevOfPath: (path: string) => number +} + +const REAL_TTY_PROBES: TtyProbes = { + isatty: fd => tty.isatty(fd), + listPtySlaves: () => + fs.readdirSync('/dev').filter(name => name.startsWith('ttys')), + rdevOfFd: fd => fs.fstatSync(fd).rdev, + rdevOfPath: path => fs.statSync(path).rdev, +} + +export function resolveInheritedStdioTtys( + probes: TtyProbes = REAL_TTY_PROBES, +): string[] { + const ttyFds = [0, 1, 2].filter(fd => probes.isatty(fd)) + if (ttyFds.length === 0) return [] + + let slaves: string[] + try { + slaves = probes.listPtySlaves() + } catch (err) { + // Silence here would reproduce the very bug this exists to fix: no rule, + // no error, and a TUI that cannot enter raw mode. + logForDebugging(`[Sandbox macOS] cannot scan /dev for pty slaves: ${err}`) + return [] + } + + const found: string[] = [] + for (const fd of ttyFds) { + let rdev: number + try { + rdev = probes.rdevOfFd(fd) + } catch (err) { + logForDebugging(`[Sandbox macOS] cannot fstat tty fd ${fd}: ${err}`) + continue + } + const match = slaves.find(name => { + try { + return probes.rdevOfPath(`/dev/${name}`) === rdev + } catch { + return false // racing device teardown; keep scanning + } + }) + if (match === undefined) { + logForDebugging( + `[Sandbox macOS] fd ${fd} is a tty but no /dev/ttys* matches its ` + + `device number ${rdev}; it will get no ioctl rule`, + ) + continue + } + const devicePath = `/dev/${match}` + if (!found.includes(devicePath)) found.push(devicePath) + } + return found +} + /** * Generate complete sandbox profile */ @@ -646,6 +762,7 @@ function generateSandboxProfile({ allowLocalBinding, allowMachLookup, allowPty, + inheritedTtys, allowGitConfig = false, enableWeakerNetworkIsolation = false, allowAppleEvents = false, @@ -661,6 +778,7 @@ function generateSandboxProfile({ allowLocalBinding?: boolean allowMachLookup?: string[] allowPty?: boolean + inheritedTtys?: string[] allowGitConfig?: boolean enableWeakerNetworkIsolation?: boolean allowAppleEvents?: boolean @@ -961,8 +1079,18 @@ function generateSandboxProfile({ } } - // Pseudo-terminal (pty) support + // Pseudo-terminal (pty) support. + // Filtered, not trusted: these paths reach a `(literal ...)` rule and + // MacOSSandboxParams is exported, so a caller (or a future config-sourced + // value) must not be able to grant ioctl on an arbitrary file. + const ttyGrants = (inheritedTtys ?? []).filter(device => + /^\/dev\/ttys[0-9]+$/.test(device), + ) if (allowPty) { + // Explicit opt-in: every pty, which programs that allocate their own + // (tmux, script, node-pty) need. This also lets the sandboxed process + // read from and write to other terminals the same user owns, so it + // stays opt-in rather than becoming the default. profile.push('') profile.push('; Pseudo-terminal (pty) support') profile.push('(allow pseudo-tty)') @@ -974,6 +1102,31 @@ function generateSandboxProfile({ profile.push(' (literal "/dev/ptmx")') profile.push(' (regex #"^/dev/ttys")') profile.push(')') + } else if (ttyGrants.length > 0) { + // Default: the terminals the child inherits, and no others. This is the + // path for both an unset `allowPty` and an explicit `allowPty: false` — + // the two behave identically, so `false` is not a footgun that silently + // reproduces the raw-mode bug below; only `true` widens the grant. Without + // an ioctl rule covering these devices, TIOCSETA/TIOCSETAW return EPERM, no + // TUI can enter raw mode, and the terminal echoes capability replies, key + // encodings and mouse events back as literal text (issues #419, #391). + // Those descriptors are already open and inherited, so this grants no + // terminal the caller did not already have. + profile.push('') + profile.push('; Pseudo-terminal (pty) support: inherited terminals only') + // ioctl alone, deliberately: it is all raw mode needs (TIOCSETA/ + // TIOCSETAW), and it is the only grant that stays harmless when the + // caller does not hand these terminals to the child. wrapWithSandbox() + // returns a string and library callers pick stdio afterwards, so a + // child spawned with piped stdio would otherwise be able to open the + // *parent's* terminals by path and read or write them. Measured: with a + // write config present — which every profile-generating path has, since + // an unrestricted one short-circuits before reaching here — reopening the + // device by path is refused while raw mode still works. Programs that + // legitimately reopen a terminal by name need `allowPty: true`. + for (const device of ttyGrants) { + profile.push(`(allow file-ioctl (literal ${escapePath(device)}))`) + } } return profile.join('\n') @@ -1010,6 +1163,7 @@ export function wrapCommandWithSandboxMacOS( setEnvVars, maskedFileBinds, allowPty, + inheritedTtys, allowGitConfig = false, gitSafeDirectories, enableWeakerNetworkIsolation = false, @@ -1075,6 +1229,11 @@ export function wrapCommandWithSandboxMacOS( allowLocalBinding, allowMachLookup, allowPty, + // Never resolved here. This function returns a command string and the + // caller chooses the child's stdio afterwards, so a terminal detected + // now is a guess about a decision that has not been made yet. Only a + // caller that knows the child inherits its stdio may pass them. + inheritedTtys, allowGitConfig, enableWeakerNetworkIsolation, allowAppleEvents, diff --git a/src/sandbox/sandbox-config.ts b/src/sandbox/sandbox-config.ts index 0d33790b..15d552d2 100644 --- a/src/sandbox/sandbox-config.ts +++ b/src/sandbox/sandbox-config.ts @@ -1107,7 +1107,14 @@ export const SandboxRuntimeConfigSchema = z allowPty: z .boolean() .optional() - .describe('Allow pseudo-terminal (pty) operations (macOS only)'), + .describe( + 'Pseudo-terminal (pty) access, macOS only. Unset or false grants ' + + 'file-ioctl on the terminals the child inherits so an interactive ' + + 'TUI can enter raw mode — but only when the caller asserts that ' + + 'inheritance (the CLI does; library callers pass inheritsStdio to ' + + 'wrapWithSandbox). true widens this to every pty, needed by ' + + 'programs that allocate their own (tmux, script, node-pty).', + ), seccomp: SeccompConfigSchema.optional().describe( 'Custom seccomp binary paths (Linux only).', ), diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index 176b7b44..1a0e29b9 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -50,6 +50,7 @@ import { import { wrapCommandWithSandboxMacOS, startMacOSSandboxLogMonitor, + resolveInheritedStdioTtys, } from './macos-sandbox-utils.js' import { startLinuxSandboxViolationMonitor, @@ -1503,6 +1504,22 @@ export type WrapWithSandboxOptions = { * reported as the violation's `command`. Defaults to `command`. */ commandText?: string + /** + * Declares that the wrapped command will be spawned with this process's + * stdio inherited, so the terminals on our fds are the terminals the child + * gets. On macOS this is what lets the profile grant `file-ioctl` on each + * of those devices, which an interactive TUI needs to enter raw mode. + * + * Off by default, and deliberately a caller's assertion rather than + * something detected here: wrapping returns a command string and the + * caller picks stdio afterwards, so detection at wrap time would guess at + * a decision not yet made — wrap under pipes then launch into a fresh + * pty, or wrap under terminal A then launch under terminal B, and the + * guess is wrong. Pass it only when you spawn with `stdio: 'inherit'`. + * Ignored when `allowPty` is `true` (that mode already grants every pty); + * honored for both unset and `false`, which share the inherited-only grant. + */ + inheritsStdio?: boolean } async function wrapWithSandbox( @@ -1664,6 +1681,16 @@ async function wrapWithSandbox( allowMachLookup: getAllowMachLookup(), ignoreViolations: getIgnoreViolations(), allowPty, + // Only when the caller asserts the child inherits our stdio, and only + // in the default mode. `allowPty: true` already covers every pty, so + // resolving the inherited set adds nothing; every other value (unset + // or an explicit `false`) takes the inherited-only default, which is + // why the guard is `!== true` rather than `=== undefined` — `false` + // must behave identically to unset, not suppress the grant. + inheritedTtys: + allowPty !== true && options?.inheritsStdio + ? resolveInheritedStdioTtys() + : undefined, allowGitConfig: getAllowGitConfig(), gitSafeDirectories, enableWeakerNetworkIsolation: getEnableWeakerNetworkIsolation(), diff --git a/test/helpers/pty-ctty.py b/test/helpers/pty-ctty.py new file mode 100644 index 00000000..5dd7c85c --- /dev/null +++ b/test/helpers/pty-ctty.py @@ -0,0 +1,82 @@ +#!/usr/bin/env python3 +"""Run argv on a pty that is genuinely the child's CONTROLLING terminal. + +Attaching a pty to stdio is not enough for terminal-permission tests. TIOCSTI +is permitted to an unprivileged caller when the fd is its *controlling* +terminal and returns EACCES when it is not, so a harness that skips setsid() +plus TIOCSCTTY measures the missing controlling terminal rather than the +sandbox policy under test. + +Exits with the child's status, or 124 if the deadline killed it, so a crash or +hang is visible at the process level instead of only as an absent marker in +stdout. + +Usage: pty-ctty.py [args...] +""" +import fcntl +import os +import pty +import select +import subprocess +import sys +import time + +TIOCSCTTY = 0x20007461 # macOS +DEADLINE = float(os.environ.get("PTY_DEADLINE", "60")) +EXIT_TIMEOUT = 124 # timeout(1)'s convention + + +def main() -> int: + master, slave = pty.openpty() + + def make_controlling() -> None: + os.setsid() + # The slave fd explicitly, rather than fd 0. Relying on fd 0 assumes + # CPython has already run its dup2 before preexec_fn, which is true + # today but is not a documented guarantee. + fcntl.ioctl(slave, TIOCSCTTY, 0) + + proc = subprocess.Popen( + sys.argv[1:], + stdin=slave, + stdout=slave, + stderr=slave, + preexec_fn=make_controlling, + pass_fds=(slave,), + close_fds=True, + ) + os.close(slave) + + out = b"" + timed_out = False + end = time.time() + DEADLINE + while True: + if time.time() >= end: + timed_out = True + break + ready, _, _ = select.select([master], [], [], 0.2) + if ready: + try: + chunk = os.read(master, 65536) + except OSError: + break + if not chunk: + break + out += chunk + elif proc.poll() is not None: + break + + if proc.poll() is None: + proc.kill() + status = proc.wait() + + sys.stdout.write(out.decode(errors="replace").replace("\r\n", "\n")) + if timed_out: + print(f"[harness] deadline of {DEADLINE}s expired", file=sys.stderr) + return EXIT_TIMEOUT + # A signalled child reports -N from wait(); report it the way a shell does. + return status if status >= 0 else 128 - status + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/test/helpers/pty-kkp.py b/test/helpers/pty-kkp.py new file mode 100644 index 00000000..9b1d578c --- /dev/null +++ b/test/helpers/pty-kkp.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python3 +"""Reproduce issue #391 deterministically, with no keypress. + +#391 reports that pressing Ctrl+C under `srt claude` displays the literal text +`^[[99;5u` — the Kitty Keyboard Protocol encoding — instead of it being handled +as a key. That is the signature of a terminal still in canonical+ECHO mode: the +tty driver echoes whatever arrives on its input back to the display, so the +application's own protocol traffic becomes visible text. + +The symptom is therefore reproducible without a human: give the child a pty, +let it try to enter raw mode, then write the KKP sequence to the master and see +whether the tty echoes it back. + + raw mode entered -> ECHO off -> nothing echoed -> prints KKP-ECHOED=NO + raw mode refused -> ECHO on -> bytes come back -> prints KKP-ECHOED=YES + +The child must print READY_MARKER once it has tried to enter raw mode. That is +both the injection trigger and the liveness proof: without it, a child that +died at startup would echo nothing and look identical to a passing run. + +Usage: pty-kkp.py [args...] +""" +import fcntl +import os +import pty +import select +import subprocess +import sys +import time + +TIOCSCTTY = 0x20007461 # macOS +KKP_CTRL_C = b"\x1b[99;5u" # CSI 99 ; 5 u — 'c' with Ctrl, verbatim from #391 +READY_MARKER = b"KKP-CHILD-READY" +READY_TIMEOUT = 30.0 +COLLECT_SECONDS = 2.0 + + +def main() -> int: + master, slave = pty.openpty() + + def make_controlling() -> None: + os.setsid() + fcntl.ioctl(slave, TIOCSCTTY, 0) + + proc = subprocess.Popen( + sys.argv[1:], + stdin=slave, + stdout=slave, + stderr=slave, + preexec_fn=make_controlling, + pass_fds=(slave,), + close_fds=True, + ) + os.close(slave) + + # Wait for the child to say it has tried raw mode, rather than sleeping a + # fixed interval: a loaded machine would otherwise get the injection while + # the tty was still in canonical mode and fail spuriously. + preamble = b"" + ready_by = time.time() + READY_TIMEOUT + while READY_MARKER not in preamble and time.time() < ready_by: + r, _, _ = select.select([master], [], [], 0.2) + if not r: + continue + try: + preamble += os.read(master, 4096) + except OSError: + break + + if READY_MARKER not in preamble: + # Never inject blind: no marker means the child never got far enough, + # and "nothing was echoed" would then be a vacuous pass. + if proc.poll() is None: + proc.kill() + proc.wait() + os.close(master) + print("KKP-ECHOED=UNKNOWN") + print(f"child never signalled readiness; captured={preamble!r}") + return 1 + + os.write(master, KKP_CTRL_C) + + echoed = b"" + end = time.time() + COLLECT_SECONDS + while time.time() < end: + ready, _, _ = select.select([master], [], [], 0.2) + if not ready: + continue + try: + chunk = os.read(master, 4096) + except OSError: + break + if not chunk: + break + echoed += chunk + + if proc.poll() is None: + proc.kill() + proc.wait() + os.close(master) + + leaked = KKP_CTRL_C in echoed or b"[99;5u" in echoed + print(f"KKP-ECHOED={'YES' if leaked else 'NO'}") + print(f"captured={echoed!r}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/test/helpers/pty-split.py b/test/helpers/pty-split.py new file mode 100644 index 00000000..58e0e5ae --- /dev/null +++ b/test/helpers/pty-split.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 +"""Run argv with stdin on one pty and stdout/stderr on a DIFFERENT pty. + +Exists to exercise the case a single-pty harness cannot reach: a process whose +inherited descriptors point at two distinct terminals. Granting only the first +one leaves operations on the other returning EPERM, and a harness that puts all +three fds on one device can never catch that. + +Child stdout (second pty) is echoed to our stdout. + +Usage: pty-split.py [args...] +""" +import os +import pty +import select +import subprocess +import sys +import time + +DEADLINE = float(os.environ.get("PTY_DEADLINE", "60")) +EXIT_TIMEOUT = 124 + + +def main() -> int: + in_master, in_slave = pty.openpty() + out_master, out_slave = pty.openpty() + + proc = subprocess.Popen( + sys.argv[1:], + stdin=in_slave, + stdout=out_slave, + stderr=out_slave, + close_fds=True, + ) + os.close(in_slave) + os.close(out_slave) + + out = b"" + timed_out = False + end = time.time() + DEADLINE + while True: + if time.time() >= end: + timed_out = True + break + ready, _, _ = select.select([out_master], [], [], 0.2) + if ready: + try: + chunk = os.read(out_master, 65536) + except OSError: + break + if not chunk: + break + out += chunk + elif proc.poll() is not None: + break + + if proc.poll() is None: + proc.kill() + status = proc.wait() + os.close(in_master) + os.close(out_master) + + sys.stdout.write(out.decode(errors="replace").replace("\r\n", "\n")) + if timed_out: + print(f"[harness] deadline of {DEADLINE}s expired", file=sys.stderr) + return EXIT_TIMEOUT + return status if status >= 0 else 128 - status + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/test/sandbox/macos-pty-default.test.ts b/test/sandbox/macos-pty-default.test.ts new file mode 100644 index 00000000..c58f3709 --- /dev/null +++ b/test/sandbox/macos-pty-default.test.ts @@ -0,0 +1,468 @@ +import { describe, it, expect, beforeAll, afterAll } from 'bun:test' +import { spawnSync } from 'node:child_process' +import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { + wrapCommandWithSandboxMacOS, + resolveInheritedStdioTtys, + type TtyProbes, +} from '../../src/sandbox/macos-sandbox-utils.js' +import { isMacOS } from '../helpers/platform.js' + +const CTTY = join(import.meta.dir, '../helpers/pty-ctty.py') +const UTILS = join(import.meta.dir, '../../src/sandbox/macos-sandbox-utils.ts') +const CLI = join(import.meta.dir, '../../src/cli.ts') + +/** The e2e cases drive the real CLI through a pty; both are hard deps. */ +const hasDeps = ['python3', 'bun'].every( + bin => spawnSync('command', ['-v', bin], { shell: true }).status === 0, +) + +/** + * Seatbelt matches ioctl rules by device path, and a terminal is a pty slave + * (`/dev/ttysNNN`) that the base profile's `/dev/tty` literal does not cover. + * Without a rule for it, TIOCSETA/TIOCSETAW return EPERM, no TUI can enter raw + * mode, and the terminal echoes capability replies, KKP key encodings and + * mouse events as literal text — issues #419 and #391. + */ +describe.if(isMacOS)('macOS pty rules: inherited-stdio terminal grant', () => { + const BROAD_REGEX = '(regex #"^/dev/ttys")' + const PSEUDO_TTY = '(allow pseudo-tty)' + const TTY_A = '/dev/ttys991' + const TTY_B = '/dev/ttys992' + + // A read restriction is required, otherwise the wrapper short-circuits and + // returns the bare command with no profile at all. + const baseParams = { + command: 'true', + needsNetworkRestriction: false, + readConfig: { denyOnly: ['/work/priv'] }, + writeConfig: undefined, + } + + it('grants ioctl on the given terminal, and nothing more', () => { + const profile = wrapCommandWithSandboxMacOS({ + ...baseParams, + inheritedTtys: [TTY_A], + }) + + expect(profile).toContain(`(allow file-ioctl (literal "${TTY_A}"))`) + // ioctl is all raw mode needs. A read/write grant would be policy surface + // that buys nothing here, and would matter if the child never receives + // this terminal. + expect(profile).not.toContain( + `(allow file-read* file-write* (literal "${TTY_A}"))`, + ) + expect(profile).not.toContain(BROAD_REGEX) + expect(profile).not.toContain(PSEUDO_TTY) + }) + + it('grants every distinct inherited terminal, not just the first', () => { + // stdio split across two terminals: a program reading keys from one while + // sizing the other needs both, and granting only the first leaves the + // second returning EPERM. + const profile = wrapCommandWithSandboxMacOS({ + ...baseParams, + inheritedTtys: [TTY_A, TTY_B], + }) + + expect(profile).toContain(`(allow file-ioctl (literal "${TTY_A}"))`) + expect(profile).toContain(`(allow file-ioctl (literal "${TTY_B}"))`) + }) + + it('ignores paths that are not pty slave devices', () => { + // The parameter is exported and reaches a (literal ...) rule, so the + // shape is enforced rather than trusted. + const profile = wrapCommandWithSandboxMacOS({ + ...baseParams, + inheritedTtys: ['/etc/passwd', '/dev/ttysNOPE', TTY_A], + }) + + expect(profile).not.toContain('/etc/passwd') + expect(profile).not.toContain('/dev/ttysNOPE') + expect(profile).toContain(`(allow file-ioctl (literal "${TTY_A}"))`) + }) + + it('emits no pty rules when no terminal is passed', () => { + // The wrapper never detects one on its own: the caller decides stdio + // after this returns, so detection here would be a guess. + const profile = wrapCommandWithSandboxMacOS({ ...baseParams }) + + expect(profile).not.toContain(PSEUDO_TTY) + expect(profile).not.toContain(BROAD_REGEX) + expect(profile).not.toContain('/dev/ttys') + }) + + it('grants every pty when allowPty is true', () => { + const profile = wrapCommandWithSandboxMacOS({ + ...baseParams, + allowPty: true, + inheritedTtys: [TTY_A], + }) + + expect(profile).toContain(PSEUDO_TTY) + expect(profile).toContain(BROAD_REGEX) + expect(profile).toContain('(literal "/dev/ptmx")') + expect(profile).not.toContain(`(allow file-ioctl (literal "${TTY_A}"))`) + }) + + it('treats allowPty:false identically to unset (inherited ioctl, no wide grant)', () => { + // `false` collapses into the default rather than emitting nothing: an + // explicit `false` and an absent flag must produce the same profile, so + // `false` cannot silently reproduce the raw-mode bug. Only `true` widens. + const asFalse = wrapCommandWithSandboxMacOS({ + ...baseParams, + allowPty: false, + inheritedTtys: [TTY_A], + }) + const asUnset = wrapCommandWithSandboxMacOS({ + ...baseParams, + inheritedTtys: [TTY_A], + }) + + expect(asFalse).toContain(`(allow file-ioctl (literal "${TTY_A}"))`) + expect(asFalse).not.toContain(PSEUDO_TTY) + expect(asFalse).not.toContain(BROAD_REGEX) + expect(asFalse).toBe(asUnset) + }) +}) + +describe('resolveInheritedStdioTtys: rdev matching (injected probes)', () => { + // Injected probes exercise the match/no-match/dedup and every error path in + // process. The real-fs path needs a genuine pty on fd 0/1/2 and is covered by + // the e2e resolver tests below. + const probes = ( + over: Partial & { ttys?: number[] }, + ): TtyProbes => ({ + isatty: fd => (over.ttys ?? []).includes(fd), + listPtySlaves: () => ['ttys001', 'ttys002', 'ttys003'], + rdevOfFd: () => 0, + rdevOfPath: () => -1, + ...over, + }) + + it('returns [] when no fd is a tty', () => { + expect(resolveInheritedStdioTtys(probes({ ttys: [] }))).toEqual([]) + }) + + it('resolves a tty fd to the device with the matching rdev', () => { + const p = probes({ + ttys: [1], + rdevOfFd: () => 42, + rdevOfPath: path => (path === '/dev/ttys002' ? 42 : 0), + }) + expect(resolveInheritedStdioTtys(p)).toEqual(['/dev/ttys002']) + }) + + it('deduplicates when two fds share one device', () => { + const p = probes({ + ttys: [1, 2], + rdevOfFd: () => 7, + rdevOfPath: path => (path === '/dev/ttys001' ? 7 : 0), + }) + expect(resolveInheritedStdioTtys(p)).toEqual(['/dev/ttys001']) + }) + + it('returns every distinct device across fds', () => { + const p = probes({ + ttys: [0, 1], + rdevOfFd: fd => (fd === 0 ? 7 : 9), + rdevOfPath: path => + path === '/dev/ttys001' ? 7 : path === '/dev/ttys003' ? 9 : 0, + }) + expect(resolveInheritedStdioTtys(p)).toEqual([ + '/dev/ttys001', + '/dev/ttys003', + ]) + }) + + it('returns [] when /dev cannot be scanned', () => { + const p = probes({ + ttys: [1], + listPtySlaves: () => { + throw new Error('EACCES') + }, + }) + expect(resolveInheritedStdioTtys(p)).toEqual([]) + }) + + it('skips an fd whose rdev cannot be read, keeps the others', () => { + const p = probes({ + ttys: [0, 1], + rdevOfFd: fd => { + if (fd === 0) throw new Error('EBADF') + return 9 + }, + rdevOfPath: path => (path === '/dev/ttys003' ? 9 : 0), + }) + expect(resolveInheritedStdioTtys(p)).toEqual(['/dev/ttys003']) + }) + + it('skips a slave whose rdev cannot be read (racing teardown)', () => { + const p = probes({ + ttys: [1], + rdevOfFd: () => 42, + rdevOfPath: () => { + throw new Error('ENOENT') + }, + }) + expect(resolveInheritedStdioTtys(p)).toEqual([]) + }) + + it('skips an fd with no matching device', () => { + const p = probes({ ttys: [1], rdevOfFd: () => 999, rdevOfPath: () => 0 }) + expect(resolveInheritedStdioTtys(p)).toEqual([]) + }) +}) + +describe.if(isMacOS)('macOS pty rules: harness dependencies', () => { + it('has python3 and bun available', () => { + // The Seatbelt-behaviour tests below are gated on these. Without this + // check a macOS runner missing either would quietly reduce the suite to + // string assertions and still report green. + expect(hasDeps).toBe(true) + }) +}) + +describe.if(isMacOS && hasDeps)('macOS pty rules: resolver', () => { + const runOnCtty = (...args: string[]) => + spawnSync('python3', [CTTY, ...args], { + encoding: 'utf8', + timeout: 120_000, + env: { ...process.env, PTY_DEADLINE: '60' }, + }) + + it('resolves the real device when a terminal is attached', () => { + // The rdev-to-/dev/ttysNNN matching is the one genuinely non-obvious piece + // of logic here, and the piped case below cannot reach it. + const probe = ` + import { resolveInheritedStdioTtys } from ${JSON.stringify(UTILS)} + console.log('RESOLVED:' + JSON.stringify(resolveInheritedStdioTtys())) + ` + const res = runOnCtty('bun', '--eval', probe) + const match = /RESOLVED:(\[.*\])/.exec(res.stdout ?? '') + + expect(match).not.toBeNull() + const devices = JSON.parse(match![1]) as string[] + expect(devices.length).toBeGreaterThan(0) + for (const device of devices) expect(device).toMatch(/^\/dev\/ttys\d+$/) + }) + + it('resolves BOTH devices when stdio spans two terminals', () => { + // The case a single-pty harness cannot reach. Returning on the first + // matching fd leaves the other terminal without a rule, so operations + // against it still fail with EPERM. + const probe = ` + import { resolveInheritedStdioTtys } from ${JSON.stringify(UTILS)} + console.log('RESOLVED:' + JSON.stringify(resolveInheritedStdioTtys())) + ` + const res = spawnSync( + 'python3', + [ + join(import.meta.dir, '../helpers/pty-split.py'), + 'bun', + '--eval', + probe, + ], + { encoding: 'utf8', timeout: 120_000 }, + ) + const match = /RESOLVED:(\[.*\])/.exec(res.stdout ?? '') + + expect(match).not.toBeNull() + const devices = JSON.parse(match![1]) as string[] + expect(devices.length).toBe(2) + expect(new Set(devices).size).toBe(2) + for (const device of devices) expect(device).toMatch(/^\/dev\/ttys\d+$/) + }) + + it('resolves nothing when stdio is piped', () => { + const probe = ` + import { resolveInheritedStdioTtys } from ${JSON.stringify(UTILS)} + console.log('RESOLVED:' + JSON.stringify(resolveInheritedStdioTtys())) + ` + const res = spawnSync('bun', ['--eval', probe], { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + }) + + expect(res.status).toBe(0) + expect(res.stdout).toContain('RESOLVED:[]') + }) + + it('emits a rule only when the caller asserts inheritsStdio', () => { + // The sandbox-manager gate: wrapping returns a string and the caller picks + // stdio afterwards, so a library consumer gets nothing unless it says so. + const probe = ` + import { SandboxManager } from ${JSON.stringify(join(import.meta.dir, '../../src/index.ts'))} + await SandboxManager.initialize({ + filesystem: { denyRead: ['/work/priv'], allowWrite: ['/tmp'], denyWrite: [] }, + network: { allowedDomains: [], deniedDomains: [] }, + }) + const re = /dev\\/ttys[0-9]+/ + const without = await SandboxManager.wrapWithSandbox('true') + const With = await SandboxManager.wrapWithSandbox('true', undefined, undefined, undefined, { inheritsStdio: true }) + console.log('GATE:' + JSON.stringify({ without: re.test(without), with: re.test(With) })) + await SandboxManager.reset() + ` + const res = runOnCtty('bun', '--eval', probe) + const match = /GATE:(\{.*\})/.exec(res.stdout ?? '') + + expect(match).not.toBeNull() + expect(JSON.parse(match![1])).toEqual({ without: false, with: true }) + }) + + it('allowPty:false resolves inherited ttys like unset (manager gate)', () => { + // Regression for the `!== true` gate: an explicit `allowPty: false` must + // still resolve and grant the inherited terminals — inherited-only ioctl, + // no `pseudo-tty` — not be suppressed the way a `=== undefined` gate did. + const probe = ` + import { SandboxManager } from ${JSON.stringify(join(import.meta.dir, '../../src/index.ts'))} + await SandboxManager.initialize({ + filesystem: { denyRead: ['/work/priv'], allowWrite: ['/tmp'], denyWrite: [] }, + network: { allowedDomains: [], deniedDomains: [] }, + allowPty: false, + }) + const re = /dev\\/ttys[0-9]+/ + const asFalse = await SandboxManager.wrapWithSandbox('true', undefined, undefined, undefined, { inheritsStdio: true }) + console.log('FALSEGATE:' + JSON.stringify({ + hasInheritedDevice: re.test(asFalse), + hasPseudoTty: /pseudo-tty/.test(asFalse), + })) + await SandboxManager.reset() + ` + const res = runOnCtty('bun', '--eval', probe) + const match = /FALSEGATE:(\{.*\})/.exec(res.stdout ?? '') + + expect(match).not.toBeNull() + expect(JSON.parse(match![1])).toEqual({ + hasInheritedDevice: true, + hasPseudoTty: false, + }) + }) +}) + +/** + * End-to-end through the CLI on a pty that is genuinely the child's + * controlling terminal. These are the tests that would have caught #419: the + * unit tests above pin generated strings, which cannot tell you whether + * Seatbelt actually permits the ioctl. + */ +describe.if(isMacOS && hasDeps)('macOS pty rules: end to end', () => { + const DIR = join(tmpdir(), 'srt-pty-e2e-' + Date.now()) + const SETTINGS = join(DIR, 'settings.json') + const SETTINGS_BROAD = join(DIR, 'settings-allowpty.json') + + // The probe reports the errno so a refusal can be identified rather than + // merely counted: EPERM (1) is Seatbelt, EACCES (13) is the kernel saying + // this is not the caller's controlling terminal. + // + // Measured caveat: inside the sandbox Seatbelt refuses BEFORE the kernel's + // controlling-terminal check, so a harness that lost the controlling + // terminal also reports EPERM there. The errno assertion alone therefore + // cannot prove the harness is sound — the unsandboxed control test below is + // what does that, and it fails with EACCES if the prerequisite breaks. + const INJECTED = 'TIOCSTI-INJECTED-MARKER' + const TIOCSTI_PROBE = [ + 'import fcntl,sys', + 'TIOCSTI=0x80017472', + 'try:', + ` [fcntl.ioctl(sys.stdin.fileno(), TIOCSTI, c.encode()) for c in ${JSON.stringify(INJECTED)}]`, + ' print("TIOCSTI-ALLOWED")', + 'except OSError as e: print("TIOCSTI-DENIED errno=%d" % e.errno)', + ].join('\n') + + beforeAll(() => { + mkdirSync(DIR, { recursive: true }) + const base = { + filesystem: { denyRead: [], allowWrite: [DIR], denyWrite: [] }, + network: { allowedDomains: [], deniedDomains: [] }, + } + // No allowPty key at all: this is the default path users get. + writeFileSync(SETTINGS, JSON.stringify(base)) + writeFileSync(SETTINGS_BROAD, JSON.stringify({ ...base, allowPty: true })) + }) + + afterAll(() => { + if (existsSync(DIR)) rmSync(DIR, { recursive: true, force: true }) + }) + + const SPAWN_OPTS = { + encoding: 'utf8' as const, + timeout: 120_000, + env: { ...process.env, PTY_DEADLINE: '90' }, + } + + /** Through the sandbox: the real CLI, on a controlling terminal. */ + const runSandboxed = (shellCommand: string, settings = SETTINGS) => + spawnSync( + 'python3', + [CTTY, 'bun', CLI, '--settings', settings, '-c', shellCommand], + SPAWN_OPTS, + ) + + /** The same harness with no sandbox in between, for control cases. */ + const runUnsandboxed = (...args: string[]) => + spawnSync('python3', [CTTY, ...args], SPAWN_OPTS) + + it('lets a sandboxed program enter raw mode with no allowPty key', () => { + const res = runSandboxed('stty raw; echo "STTY-RC=$?"') + expect(res.stdout).toContain('STTY-RC=0') + }) + + it('does not echo an injected KKP key encoding back as text (#391)', () => { + // The byte-level reproduction of #391: write the Kitty Keyboard Protocol + // encoding of Ctrl+C to the pty master and read back. A terminal left in + // canonical+ECHO mode returns it verbatim, which is exactly the `^[[99;5u` + // the issue reports seeing on screen; in raw mode nothing comes back. + const res = spawnSync( + 'python3', + [ + join(import.meta.dir, '../helpers/pty-kkp.py'), + 'bun', + CLI, + '--settings', + SETTINGS, + '-c', + // The marker is the harness's injection trigger AND this test's + // liveness proof: a child that died at startup echoes nothing, which + // would otherwise be indistinguishable from a pass. + 'stty raw 2>/dev/null; echo KKP-CHILD-READY; sleep 3', + ], + { encoding: 'utf8', timeout: 120_000 }, + ) + + expect(res.stdout).toContain('KKP-ECHOED=NO') + expect(res.stdout).not.toContain('KKP-ECHOED=YES') + expect(res.stdout).not.toContain('KKP-ECHOED=UNKNOWN') + }) + + // Control. Every TIOCSTI assertion below is only meaningful if the harness + // really hands the child a CONTROLLING terminal, because macOS returns + // EACCES for a terminal that is merely attached. This test fails if that + // prerequisite ever breaks, so the denial tests cannot pass vacuously. + it('proves the harness grants a controlling terminal: TIOCSTI works unsandboxed', () => { + const res = runUnsandboxed('python3', '-c', TIOCSTI_PROBE) + + expect(res.stdout).toContain('TIOCSTI-ALLOWED') + // The injected text is echoed back by the terminal, which is the injection + // actually landing rather than merely being permitted. + expect(res.stdout).toContain(INJECTED) + }) + + it('refuses keystroke injection with EPERM in the default mode', () => { + const res = runSandboxed(`python3 -c '${TIOCSTI_PROBE}'`) + + expect(res.stdout).toContain('TIOCSTI-DENIED errno=1') + expect(res.stdout).not.toContain('TIOCSTI-ALLOWED') + }) + + it('refuses keystroke injection with EPERM under allowPty: true', () => { + // The broad mode grants ioctl over every pty, so this is where an + // injection primitive would appear first if the rules ever widened. + const res = runSandboxed(`python3 -c '${TIOCSTI_PROBE}'`, SETTINGS_BROAD) + + expect(res.stdout).toContain('TIOCSTI-DENIED errno=1') + expect(res.stdout).not.toContain('TIOCSTI-ALLOWED') + }) +})