#667 [security][tools] OS の隔離の中から制御端末へ TIOCSTI で入力を注入でき、/dev/pts の他の端末へ書ける

SECURITY.md

@@ -89,6 +89,49 @@ command before the shell starts.
   `sed -i` — and creating a hardlink to an inode outside the workspace. The only
   writable locations are the workspace, the run's temp directory, the known
   dependency caches, and device files such as `/dev/null`.
+- Terminal isolation (Issue #667): a command runs in its own session with no
+  controlling terminal (`setsid()` at spawn), so it cannot inject keystrokes into
+  the terminal the run was started from with `TIOCSTI` — the kernel only allows
+  `TIOCSTI` on the caller's controlling terminal, and the child no longer has
+  one. It also cannot name a terminal device by path from inside the isolation:
+  on Linux `/dev/pts` and `/dev/tty` (plus every existing `/dev/tty*` and
+  `/dev/console`) are outside both the write allow list and the read allow list,
+  and on macOS the profile re-denies `/dev/tty*` for writes and reads last, so a
+  command can neither write to a terminal's slave nor re-open the run's terminal
+  by path and read the keys the user types. `/dev/null`, `/dev/zero`,
+  `/dev/urandom`, and the rest of `/dev` stay usable. `/dev/ptmx` is still
+  openable, but that does not give a usable pty: the slave it would create
+  (`/dev/pts/*` on Linux, `/dev/ttys*` on macOS) is refused, so a confined
+  command cannot allocate a pty (`openpty`, `script`, `expect`, `pexpect` fail).
+  That pty loss is the cost of closing terminal reads and writes (Issue #667);
+  allocating a pty worked before #667. A terminal
+  standard input is replaced with `/dev/null`, so the command's standard input is
+  non-readable; the pre-#667 job-control check had left it non-readable only by
+  accident (the child sat in a background process group of the session's
+  terminal, and a read was refused with `SIGTTIN`, or `EIO` once that signal is
+  ignored). The visible change is to a program that opens the terminal
+  *directly*: an interactive pager waiting for keys on `/dev/tty` now sees no
+  controlling terminal.
+  Two cases are outside this boundary. A standard output or error that is already
+  the terminal keeps working from its inherited descriptor, which the read deny
+  does not re-check, so a confined command that inherited a terminal on
+  stdout/stderr could still read it; the confined launches set both to pipes, so
+  none inherits a terminal descriptor. And a command launched without the
+  isolation is not a boundary at all: it runs with the user's own privilege and,
+  because the launcher `setsid()`s every child, it can re-open the run's terminal
+  by path even though job control would otherwise have refused the read.
+  Every standard input is either set to `null` or a pipe, or inherited; an
+  inherited terminal stdin is the only case the `/dev/null` replacement changes.
+  The confined launches are the model Bash and its `bash:verify` interval, the
+  verifier/acceptance commands, and the runtime dependency setup; each sets its
+  `stdout` and `stderr` to pipes (`src/tools/bash.rs`,
+  `src/minimal_loop/verifier_env.rs`, `src/minimal_loop/dependency_setup.rs`).
+  The non-confined launches are the workspace `git` state check, the `lsof` port
+  lookups, the fetch, interaction, browser, pipeline, CLI-argv, and
+  declarative-check probes, the community `esbuild` bundle, the dev-server start,
+  and the backend availability probes. All of them run non-interactive commands
+  with a non-terminal standard input or an inherited one, so the change is a
+  non-readable stdin rather than a behavior change.
 - Read confinement (Issue #657) denies reading the home secret locations
   (`.ssh`, `.aws`, `.gnupg`, `.netrc`, `.git-credentials`, `.npmrc`, `.pypirc`,
   `.docker/config.json`, `.kube`, `.config/gh`, `.config/gcloud`) and, on macOS,

src/bounded_process.rs

@@ -50,7 +50,7 @@ impl BoundedProcessOutput {
 
 pub fn spawn_child(command: &mut Command) -> io::Result<Child> {
     apply_child_env_allowlist(command);
-    configure_process_group(command);
+    configure_child_session(command);
     command.spawn()
 }
 
@@ -399,7 +399,7 @@ pub fn terminate_process_group(child: &mut Child) {
 
 /// Resolves a spawned child PID into a process-group id that is safe to
 /// signal with a negative `kill` target. The child is spawned with
-/// `process_group(0)`, so its PID equals its group id, but `-1` would signal
+/// `setsid()`, so its PID equals its group id, but `-1` would signal
 /// every signalable process and `0`/`1` would target the caller or init.
 #[cfg(unix)]
 fn signalable_process_group(pid: u32) -> Option<i32> {
@@ -416,7 +416,7 @@ pub fn terminate_process_group_by_pid(pid: u32) -> io::Result<()> {
             ));
         };
         // SAFETY: `pgid` is greater than 1 and derived from a child spawned
-        // with `process_group(0)`, so `-pgid` names only that child's own
+        // with `setsid()`, so `-pgid` names only that child's own
         // process group, never pid 0/1 or the caller's group.
         unsafe {
             libc::kill(-pgid, libc::SIGTERM);
@@ -476,7 +476,7 @@ fn terminate_process_group_with_grace<F, C>(
     {
         if let Some(pgid) = signalable_process_group(child.id()) {
             // SAFETY: `pgid` is greater than 1 and was created with
-            // `configure_process_group(0)` for this child, so `-pgid` targets
+            // `configure_child_session()` for this child, so `-pgid` targets
             // only the child's process group, never pid 0/1 or the caller's.
             unsafe {
                 libc::kill(-pgid, libc::SIGTERM);
@@ -520,11 +520,83 @@ fn output_from_child(
     })
 }
 
-fn configure_process_group(command: &mut Command) {
+/// Put the child in a new session, and so a new process group, with no
+/// controlling terminal (Issue #667).
+///
+/// `setsid()` sets both the session id and the process-group id to the child's
+/// pid, so the child's group id still equals its pid and
+/// `terminate_process_group*` keeps signalling exactly the child's own group
+/// with `-pid`. Dropping the controlling terminal is the point: the kernel only
+/// allows `TIOCSTI` when the fd names the caller's controlling terminal, so a
+/// child in its own session can no longer inject keystrokes into the terminal
+/// the caller was started from (`/dev/tty` also stops resolving for it).
+///
+/// This replaces `Command::process_group(0)`: `setsid()` fails with `EPERM` for a
+/// process that is already a process-group leader, so the two cannot be combined
+/// and `setsid()` alone provides the group. Losing the controlling terminal also
+/// makes a terminal stdin readable, so [`detach_terminal_stdin`] points it at
+/// `/dev/null` in the same `pre_exec`.
+fn configure_child_session(command: &mut Command) {
     #[cfg(unix)]
     {
         use std::os::unix::process::CommandExt;
-        command.process_group(0);
+        // SAFETY: `pre_exec` runs after fork and before exec. The closure calls
+        // only `setsid`, `isatty`, `open`, `dup2`, and `close`; it does not
+        // allocate, lock, or log. A forked child is never a process-group leader
+        // (its pid is new), so `setsid` cannot fail with `EPERM`.
+        unsafe {
+            command.pre_exec(|| {
+                if libc::setsid() == -1 {
+                    return Err(std::io::Error::last_os_error());
+                }
+                detach_terminal_stdin()
+            });
+        }
+    }
+    #[cfg(not(unix))]
+    {
+        let _ = command;
+    }
+}
+
+/// Replace a terminal standard input with `/dev/null` in the freshly forked
+/// child.
+///
+/// Before `setsid()` the child sat in a background process group of the caller's
+/// session, so a read from the terminal was refused with `SIGTTIN` (or `EIO`
+/// once that signal is ignored). After `setsid()` the child has no controlling
+/// terminal, so the job-control check no longer applies and a read would succeed,
+/// letting a command read the keys the user types during a run. Pointing a
+/// terminal stdin at `/dev/null` restores a non-readable stdin. A stdin that is
+/// not a terminal (`Stdio::null()`, a pipe, or a file) is left unchanged, as are
+/// every standard output and error.
+///
+/// # pre_exec contract
+///
+/// This runs between `fork` and `exec`, so it uses only `isatty`, `open`,
+/// `dup2`, and `close` and never allocates, locks, or logs. Errors are built with
+/// [`std::io::Error::last_os_error`], which does not allocate.
+#[cfg(unix)]
+fn detach_terminal_stdin() -> std::io::Result<()> {
+    // SAFETY: this is the classic `isatty`/`open`/`dup2`/`close` sequence on the
+    // child's own descriptors; the path is a fixed static C string.
+    unsafe {
+        if libc::isatty(0) != 1 {
+            return Ok(());
+        }
+        let devnull = libc::open(c"/dev/null".as_ptr(), libc::O_RDONLY);
+        if devnull < 0 {
+            return Err(std::io::Error::last_os_error());
+        }
+        if libc::dup2(devnull, 0) < 0 {
+            let error = std::io::Error::last_os_error();
+            libc::close(devnull);
+            return Err(error);
+        }
+        if devnull != 0 {
+            libc::close(devnull);
+        }
+        Ok(())
     }
 }