From 1ebee808d160b19cf5a07af096a84731255ef079 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:31:37 +0800 Subject: [PATCH] fix(config): close the config directory to other users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It holds `history` — every command with its cwd and exit status — plus the SSH profiles in `config.json`, `session.json` and `views.json`. `machine.json` and `appearance.json` are written 0600 and the sockets are 0600, but those four are not, and the directory around them was made with plain `create_dir_all`: 0755 under the stock umask of 022, with the history file 0644 inside it. Another account on the machine could read the lot. The rule already existed twice. `daemon::history` closes its own subdirectory, saying "closing the directory is what makes the mode of what is inside it moot", and `transport::bind` closes the socket's parent when it is the config dir. Neither covered the directory holding everything else, and the daemon reaches it first through the pidfile, the singleton lock and the TCP endpoint, none of which closed anything. `ensure_private_dir` closes every directory the call creates, and the config directory itself even when it already existed — so an install made by an earlier build is repaired rather than left open for its lifetime. Directories it did not create and that are not ours are left alone, which is the distinction `transport::bind` drew with `owns_parent`: $HOME and ~/.config are on this walk on a first run. Verified against a running daemon rather than only in a test: with umask 022 a fresh config dir came out drwxr-xr-x before and drwx------ after, and a directory chmodded back to 755 was closed again on the next start. The unit test caught a hole in the first attempt — closing only the leaf left the config directory, which `create_dir_all` had just made, exactly as open as before. --- CHANGELOG.md | 9 ++ crates/tty7-core/src/core/config.rs | 103 +++++++++++++++++++++- crates/tty7-core/src/core/session.rs | 2 +- crates/tty7-core/src/daemon/pidfile.rs | 2 +- crates/tty7-core/src/daemon/scrollback.rs | 2 +- crates/tty7-core/src/daemon/singleton.rs | 2 +- crates/tty7-core/src/daemon/transport.rs | 2 +- src/main.rs | 2 +- src/terminal/history.rs | 2 +- src/ui/presets.rs | 2 +- 10 files changed, 119 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c7e0239..828460d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -153,6 +153,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- The config directory is now created closed to other users, and one an earlier + build left open is repaired on the next write. It holds the command history — + every command with its working directory and exit status — along with the SSH + profiles in `config.json`, the session's directories and the workspace tree, + and it was created under the process umask: 022 on a stock box, which makes + the directory 0755 and the history file 0644. On a shared machine another + account could read all of it. The daemon's own history subdirectory already + closed itself for exactly this reason, as did the unix socket's parent; the + directory holding the rest did not. - A control request whose handler fails outright is now answered with an error instead of silently dropped. Every other way out already answered — even a request the queue was too full to accept — but a panic inside the handler diff --git a/crates/tty7-core/src/core/config.rs b/crates/tty7-core/src/core/config.rs index 12446683..58c24f82 100644 --- a/crates/tty7-core/src/core/config.rs +++ b/crates/tty7-core/src/core/config.rs @@ -905,7 +905,7 @@ impl Config { return; }; if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); + let _ = ensure_private_dir(parent); } match serde_json::to_string_pretty(self) { Ok(text) => { @@ -1105,6 +1105,57 @@ fn quarantine_path(path: &std::path::Path) -> PathBuf { .unwrap_or(base) } +/// Create `dir`, and close it to this user. +/// +/// The config directory holds the command history, the SSH profiles, the +/// session's working directories and the workspace tree. Not all of that is +/// written with a private mode of its own — `history` is appended by the app +/// under the process umask, which is 022 on a stock box, so the file lands +/// 0644 — and the directory it sits in was created the same way. Between them +/// nothing was closed, and on a shared machine another account could read +/// every command you had run, with its directory and its exit status. +/// +/// `daemon::history` already says this about the subdirectory it writes: +/// "closing the directory is what makes the mode of what is inside it moot". +/// This is that rule for the directory holding it, which is where the rest of +/// the state lives. +/// +/// Applied on every call rather than only when the directory is new, so a +/// config directory an earlier build left open is closed the next time +/// anything writes to it. +pub fn ensure_private_dir(dir: &std::path::Path) -> std::io::Result<()> { + // Every directory this call brings into being, not just the last one. + // `create_dir_all("/history.d")` makes `` too, and closing + // only the leaf would leave the directory holding `config.json`, `history` + // and `session.json` exactly as open as before. + // + // Ancestors that already existed are left alone: `$HOME` and `~/.config` + // are not ours to re-permission, and on a first run they are the ones this + // walk stops at. + #[cfg(unix)] + let fresh: Vec<&std::path::Path> = dir.ancestors().take_while(|p| !p.exists()).collect(); + std::fs::create_dir_all(dir)?; + // Windows has no umask; the config directory inherits an ACL that is + // already per-user, which is the same reason `daemon::history` skips it. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + let private = std::fs::Permissions::from_mode(0o700); + for made in fresh { + let _ = std::fs::set_permissions(made, private.clone()); + } + // The config directory itself even when it was already there, so one + // an earlier build left open is repaired rather than skipped. Only + // that one: a directory this call did not create and that is not ours + // is somebody else's to permission, which is the distinction + // `transport::bind` drew with `owns_parent` before this existed. + if config_dir().is_some_and(|c| c == dir) { + let _ = std::fs::set_permissions(dir, private); + } + } + Ok(()) +} + pub fn write_atomic(path: &std::path::Path, bytes: &[u8]) -> std::io::Result<()> { write_atomic_mode(path, bytes, false) } @@ -2114,6 +2165,56 @@ mod tests { } } + #[cfg(unix)] + #[test] + fn the_config_directory_is_closed_to_other_users() { + use std::os::unix::fs::PermissionsExt as _; + let _guard = lock_config_file(); + let mode_of = + |p: &std::path::Path| std::fs::metadata(p).unwrap().permissions().mode() & 0o777; + + // Every directory the call brings into being, not only the last one. + // A stock box runs umask 022, which would leave these 0755 and the + // `history` file inside them 0644 — readable by every other account. + let scratch = std::env::temp_dir().join(format!("tty7-privdir-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&scratch); + let nested = scratch.join("history.d"); + super::ensure_private_dir(&nested).expect("the directory is created"); + for made in [&scratch, &nested] { + assert_eq!( + mode_of(made), + 0o700, + "{} is open, so anyone on this machine can read what is in it", + made.display() + ); + } + + // A directory that was already there and is not ours stays as it is: + // `$HOME` and `~/.config` are on this walk on a first run, and neither + // is this code's to re-permission. + std::fs::set_permissions(&nested, std::fs::Permissions::from_mode(0o755)).unwrap(); + super::ensure_private_dir(&nested.join("deeper")).expect("still there"); + assert_eq!( + mode_of(&nested), + 0o755, + "a directory this call did not create was re-permissioned anyway" + ); + let _ = std::fs::remove_dir_all(&scratch); + + // The config directory is the exception, so one an earlier build left + // open is repaired the next time anything writes to it rather than + // staying open for the life of the install. + pin_config_dir(); + let cfg = config_dir().expect("the pinned config dir resolves"); + std::fs::set_permissions(&cfg, std::fs::Permissions::from_mode(0o755)).unwrap(); + super::ensure_private_dir(&cfg).expect("the config dir is there"); + assert_eq!( + mode_of(&cfg), + 0o700, + "a config directory an earlier build left open stayed open" + ); + } + #[test] fn write_atomic_replaces_contents_and_leaves_no_temp() { let dir = TestDir::new("atomic"); diff --git a/crates/tty7-core/src/core/session.rs b/crates/tty7-core/src/core/session.rs index 8cef3ba2..2de7038a 100644 --- a/crates/tty7-core/src/core/session.rs +++ b/crates/tty7-core/src/core/session.rs @@ -495,7 +495,7 @@ impl WindowViews { return; }; if let Some(parent) = path.parent() - && let Err(e) = std::fs::create_dir_all(parent) + && let Err(e) = crate::core::config::ensure_private_dir(parent) { log::warn!("failed to create views dir {}: {e}", parent.display()); return; diff --git a/crates/tty7-core/src/daemon/pidfile.rs b/crates/tty7-core/src/daemon/pidfile.rs index 73d14680..85cff228 100644 --- a/crates/tty7-core/src/daemon/pidfile.rs +++ b/crates/tty7-core/src/daemon/pidfile.rs @@ -9,7 +9,7 @@ pub fn path() -> Option { pub(crate) fn write_current() { let Some(path) = path() else { return }; if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); + let _ = crate::core::config::ensure_private_dir(parent); } if let Err(e) = std::fs::write(&path, std::process::id().to_string()) { log::warn!("could not write pidfile {}: {e}", path.display()); diff --git a/crates/tty7-core/src/daemon/scrollback.rs b/crates/tty7-core/src/daemon/scrollback.rs index 270b36b1..e75b8d42 100644 --- a/crates/tty7-core/src/daemon/scrollback.rs +++ b/crates/tty7-core/src/daemon/scrollback.rs @@ -168,7 +168,7 @@ pub fn save(pane_id: u64, segments: &[Segment]) { let Some(parent) = path.parent().map(|p| p.to_path_buf()) else { return; }; - if let Err(e) = std::fs::create_dir_all(&parent) { + if let Err(e) = crate::core::config::ensure_private_dir(&parent) { log::debug!("no scrollback directory ({e}); pane {pane_id} is not persisted"); return; } diff --git a/crates/tty7-core/src/daemon/singleton.rs b/crates/tty7-core/src/daemon/singleton.rs index 0eac90d1..1691e647 100644 --- a/crates/tty7-core/src/daemon/singleton.rs +++ b/crates/tty7-core/src/daemon/singleton.rs @@ -117,7 +117,7 @@ pub fn claim() -> Claim { return Claim::Unavailable("no config directory".into()); }; if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); + let _ = crate::core::config::ensure_private_dir(parent); } match open_exclusive(&path) { Ok(Some(file)) => { diff --git a/crates/tty7-core/src/daemon/transport.rs b/crates/tty7-core/src/daemon/transport.rs index 3632f9bf..719c9eef 100644 --- a/crates/tty7-core/src/daemon/transport.rs +++ b/crates/tty7-core/src/daemon/transport.rs @@ -438,7 +438,7 @@ mod imp_windows { let path = port_path_named(file) .ok_or_else(|| anyhow::anyhow!("could not resolve {file} path (no config dir)"))?; if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); + let _ = config::ensure_private_dir(parent); } let listener = TcpListener::bind(loopback(0)) .map_err(|e| anyhow::anyhow!("bind 127.0.0.1:0 failed: {e}"))?; diff --git a/src/main.rs b/src/main.rs index f2af1c71..f8f27519 100644 --- a/src/main.rs +++ b/src/main.rs @@ -37,7 +37,7 @@ fn spawn_config_watcher(cx: &mut App) { let Some(dir) = crate::core::config::config_dir_path() else { return; }; - let _ = std::fs::create_dir_all(&dir); + let _ = crate::core::config::ensure_private_dir(&dir); const DEBOUNCE: std::time::Duration = std::time::Duration::from_millis(200); diff --git a/src/terminal/history.rs b/src/terminal/history.rs index 0e72f24c..11d1e39f 100644 --- a/src/terminal/history.rs +++ b/src/terminal/history.rs @@ -233,7 +233,7 @@ pub fn append(scope: &Scope, cmd: &str, cwd: Option<&Path>, ts: u64, exit: Optio return; }; if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); + let _ = tty7_core::core::config::ensure_private_dir(parent); } let cwd = match cwd.and_then(Path::to_str) { Some(c) if looks_absolute(c) && !c.contains(['\t', '\n', '\r']) => c, diff --git a/src/ui/presets.rs b/src/ui/presets.rs index bbac35ce..e48d3f52 100644 --- a/src/ui/presets.rs +++ b/src/ui/presets.rs @@ -721,7 +721,7 @@ pub fn to_yaml(t: &Theme) -> String { pub fn fork_to_file(t: &Theme) -> std::io::Result { let dir = themes_dir() .ok_or_else(|| std::io::Error::new(std::io::ErrorKind::NotFound, "no themes directory"))?; - std::fs::create_dir_all(&dir)?; + crate::core::config::ensure_private_dir(&dir)?; let base = format!("{}-custom", t.id.trim_end_matches("-custom")); let mut stem = base.clone(); let mut n = 2;