mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-21 16:02:20 +00:00
fix(config): close the config directory to other users
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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("<config>/history.d")` makes `<config>` 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");
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -9,7 +9,7 @@ pub fn path() -> Option<PathBuf> {
|
||||
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());
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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)) => {
|
||||
|
||||
@@ -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}"))?;
|
||||
|
||||
+1
-1
@@ -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);
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
+1
-1
@@ -721,7 +721,7 @@ pub fn to_yaml(t: &Theme) -> String {
|
||||
pub fn fork_to_file(t: &Theme) -> std::io::Result<String> {
|
||||
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;
|
||||
|
||||
Reference in New Issue
Block a user