From a93d5b3bf75a58770cdf6d08efb614c6df2418b9 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:54:04 +0800 Subject: [PATCH] fix(remote): the SSH bootstrap makes its scratch dir, never adopts one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same hole as the local one, on the far side of the connection and in generated shell rather than Rust — and on a shared server, which is where several accounts actually coexist. __tty7_d=${TMPDIR:-/tmp}/tty7-zdotdir-$$ command mkdir -p "$__tty7_d" 2>/dev/null command cat > "$__tty7_d/.zshrc" <<'EOF' ... ... && export ZDOTDIR="$__tty7_d" `mkdir -p` succeeds on a directory that already exists, and $$ is the remote shell's pid, which every other account on that box can read. So the startup files went into whatever held the name, and its owner could rewrite them in the moment before zsh read them. Now `mkdir -m 700` without `-p`, which fails when the path exists, with the writes moved inside the `if`. A taken name means nothing is written, the `[ -s ... ]` guards that were already there see nothing, and the login falls through to the plain `exec -l` those guards exist for. Run rather than reasoned about. Against a directory planted at the name with mode 777 and an `evil` .zshrc: the fixed script leaves ZDOTDIR empty and the plant untouched, while the `mkdir -p` version exported the planted path for zsh to read. Both scripts pass `sh -n`, and the ordinary path still creates drwx------ and exports ZDOTDIR. --- CHANGELOG.md | 8 ++ .../tty7-core/src/daemon/shell_integration.rs | 79 ++++++++++++++++++- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8dd8aa7..c8129c9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -153,6 +153,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- The bootstrap tty7 runs on a remote machine now creates its own scratch + directory instead of adopting one. The startup files a remote shell sources + were written into `${TMPDIR:-/tmp}/tty7-zdotdir-$$`, created with `mkdir -p`, + which succeeds on a directory that is already there. On a shared server + another account can read that pid and take the name first, and then rewrite + those files between tty7 writing them and the shell reading them. The + directory is now created exclusively at mode 700; if the name is taken + nothing is written and the login falls through to a plain login shell. - A shell's scratch directory is now created by tty7 alone, closed to other accounts. The startup files a pane's shell sources — the redirectors `ZDOTDIR` points zsh at, and bash's and nu's equivalents — were written into diff --git a/crates/tty7-core/src/daemon/shell_integration.rs b/crates/tty7-core/src/daemon/shell_integration.rs index b522c593..9112e003 100644 --- a/crates/tty7-core/src/daemon/shell_integration.rs +++ b/crates/tty7-core/src/daemon/shell_integration.rs @@ -1448,6 +1448,29 @@ pub mod remote { format!("'{}'", s.replace('\\', r"\\").replace('\'', r"\'")) } + /// Open `$__tty7_d` as a directory this login made, or leave the block + /// unentered. + /// + /// The far end sources what goes in here, and the name is + /// `-$$` — the remote shell's pid, which on a shared server every + /// other account can read out of `/proc` and get to first. `mkdir -p` + /// succeeds on a directory that is already there, so the startup files + /// went into whatever held the name, and its owner could rewrite them + /// before the shell read them. + /// + /// `mkdir` without `-p` fails when the path exists, and `-m 700` puts the + /// mode on the creating syscall rather than a chmod after it. The caller + /// closes the `if`; everything it writes is inside, so a name already + /// taken means the files are never written, the `[ -s ... ]` guards that + /// already exist see nothing, and the login falls through to the plain + /// `exec -l` those guards were written for. + fn open_scratch_dir(prefix: &str) -> String { + format!( + "__tty7_d=${{TMPDIR:-/tmp}}/{prefix}-$$\n\ + if command mkdir -m 700 \"$__tty7_d\" 2>/dev/null; then\n" + ) + } + fn write_file(out: &mut String, name: &str, body: &str) { out.push_str(&format!( "command cat > \"$__tty7_d/{name}\" <<'{HEREDOC}'\n{}\n{HEREDOC}\n", @@ -1457,8 +1480,7 @@ pub mod remote { fn zsh_bootstrap(shell_path: &str) -> String { let mut out = String::new(); - out.push_str("__tty7_d=${TMPDIR:-/tmp}/tty7-zdotdir-$$\n"); - out.push_str("command mkdir -p \"$__tty7_d\" 2>/dev/null\n"); + out.push_str(&open_scratch_dir("tty7-zdotdir")); let mut guard = String::new(); for (name, contents) in zsh_redirectors() { @@ -1473,6 +1495,7 @@ pub mod remote { out.push_str(&format!( "{guard}export ZDOTDIR=\"$__tty7_d\" TTY7_RM_DIR=\"$__tty7_d\"\n" )); + out.push_str("fi\n"); out.push_str(&format!("exec {} -l\n", shell_quote(shell_path))); out } @@ -1480,8 +1503,7 @@ pub mod remote { fn bash_bootstrap(shell_path: &str) -> String { let quoted = shell_quote(shell_path); let mut out = String::new(); - out.push_str("__tty7_d=${TMPDIR:-/tmp}/tty7-bashrc-$$\n"); - out.push_str("command mkdir -p \"$__tty7_d\" 2>/dev/null\n"); + out.push_str(&open_scratch_dir("tty7-bashrc")); write_file( &mut out, "bashrc", @@ -1491,6 +1513,7 @@ pub mod remote { out.push_str("export TTY7_RM_DIR=\"$__tty7_d\"\n"); out.push_str(&format!("exec {quoted} --rcfile \"$__tty7_d/bashrc\" -i\n")); out.push_str("fi\n"); + out.push_str("fi\n"); out.push_str(&format!("exec {quoted} -l\n")); out } @@ -1539,6 +1562,54 @@ fi use super::super::{BASH_INTEGRATION, ZSH_INTEGRATION}; use super::*; + /// The remote bootstrap makes its scratch directory or writes nothing. + /// + /// The far end sources what goes in there, and the name is the remote + /// shell's `$$` — which on a shared server every other account can + /// read and get to first. `mkdir -p` took whatever held the name, so + /// the startup files were written into a directory somebody else owned + /// and could rewrite before the shell read it. + /// + /// Run against a planted directory, the fixed script leaves `ZDOTDIR` + /// empty and the plant untouched; the `mkdir -p` version exported the + /// planted path. Verified by hand that way — what is pinned here is + /// the shape that made the difference. + #[test] + fn a_remote_bootstrap_creates_its_scratch_dir_rather_than_adopting_one() { + for (shell, path) in [ + (RemoteShell::Zsh, "/bin/zsh"), + (RemoteShell::Bash, "/bin/bash"), + ] { + let script = bootstrap_command(shell, path); + assert!( + script.contains("if command mkdir -m 700 \"$__tty7_d\" 2>/dev/null; then"), + "{shell:?} does not create its own scratch directory:\n{script}" + ); + assert!( + !script.contains("mkdir -p"), + "{shell:?} still adopts a directory that is already there:\n{script}" + ); + // Everything written has to sit inside that `if`, or a taken + // name means writing to `$__tty7_d` while it is somebody + // else's — or, with the variable cleared, to `/`. + let opened = script.find("; then").expect("the guard is there"); + let closed = script.rfind("\nfi\n").expect("the guard is closed"); + let writes = script.match_indices("command cat > "); + for (at, _) in writes { + assert!( + at > opened && at < closed, + "{shell:?} writes a startup file outside the guard:\n{script}" + ); + } + // And the fallback the guard drops into is still a plain login + // shell, which is what makes refusing a taken name safe. + assert!( + script.trim_end().ends_with(&format!("exec '{path}' -l")), + "{shell:?} has no plain login shell to fall back to:\n{script}" + ); + } + } + #[test] fn probe_reads_the_line_after_the_marker() { assert_eq!(