From a1f96112b6266a69f5663a07e769b5bb1abb0d1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Pereira?= <484633+joaodrp@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:55:05 +0100 Subject: [PATCH 1/2] Create the state directory private `things.log` is opened without an explicit mode and the state directory is created with `create_dir_all`, so on a default umask they land at 0644 in a 0755 directory. That log holds every task title and note in plaintext, readable by any other local user; `auth.json` was the only thing already protected. `create_private_dir` narrows the directory it creates to 0700, tightening an existing one on upgrade too. The auth path applies it to the state directory itself; the sync path applies it to `append-log`, which leaves the state directory a plain intermediate when no `set-auth` has run. Its contents are 0600/0700 either way. --- src/auth.rs | 4 ++-- src/dirs.rs | 46 +++++++++++++++++++++++++++++++++++++++++++++- src/log_cache.rs | 2 +- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 3a3d4a3..aea52a5 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -7,7 +7,7 @@ use figment::{ }; use serde::{Deserialize, Serialize}; -use crate::dirs::auth_file_path; +use crate::dirs::{auth_file_path, create_private_dir}; #[derive(Debug, Clone, Serialize, Deserialize)] struct AuthPayload { @@ -75,7 +75,7 @@ pub fn write_auth(email: &str, password: &str) -> Result { .parent() .ok_or_else(|| anyhow!("Invalid auth file path"))? .to_path_buf(); - fs::create_dir_all(&parent).with_context(|| format!("Failed creating {}", parent.display()))?; + create_private_dir(&parent).with_context(|| format!("Failed creating {}", parent.display()))?; let payload = AuthPayload { email, password }; let serialized = serde_json::to_string(&payload)?; diff --git a/src/dirs.rs b/src/dirs.rs index e6675b4..ec038ae 100644 --- a/src/dirs.rs +++ b/src/dirs.rs @@ -1,4 +1,7 @@ -use std::{fs, path::PathBuf}; +use std::{ + fs, + path::{Path, PathBuf}, +}; const APP_NAME: &str = "things3"; const LEGACY_APP_NAME: &str = "things-cli"; @@ -44,3 +47,44 @@ pub fn append_log_dir() -> PathBuf { pub fn auth_file_path() -> PathBuf { app_state_dir().join("auth.json") } + +/// Create `dir` and narrow it to owner-only. The state directory holds the +/// Things Cloud password and an append log carrying every task title and note, +/// so the default 0755 leaves that readable by any other local user. +pub fn create_private_dir(dir: &Path) -> std::io::Result<()> { + fs::create_dir_all(dir)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(dir, fs::Permissions::from_mode(0o700))?; + } + Ok(()) +} + +#[cfg(all(test, unix))] +mod tests { + use super::*; + use std::os::unix::fs::PermissionsExt; + + fn mode_of(path: &Path) -> u32 { + fs::metadata(path).expect("metadata").permissions().mode() & 0o777 + } + + #[test] + fn creates_the_directory_private() { + let tmp = tempfile::tempdir().expect("tempdir"); + let dir = tmp.path().join("things3").join("append-log"); + create_private_dir(&dir).expect("create"); + assert_eq!(mode_of(&dir), 0o700); + } + + #[test] + fn narrows_an_existing_world_readable_directory() { + let tmp = tempfile::tempdir().expect("tempdir"); + let dir = tmp.path().join("things3"); + fs::create_dir_all(&dir).expect("seed"); + fs::set_permissions(&dir, fs::Permissions::from_mode(0o755)).expect("widen"); + create_private_dir(&dir).expect("create"); + assert_eq!(mode_of(&dir), 0o700); + } +} diff --git a/src/log_cache.rs b/src/log_cache.rs index 5587be5..565cfec 100644 --- a/src/log_cache.rs +++ b/src/log_cache.rs @@ -67,7 +67,7 @@ fn write_cursor( } pub fn sync_append_log(client: &mut ThingsCloudClient, cache_dir: &Path) -> Result<()> { - fs::create_dir_all(cache_dir)?; + crate::dirs::create_private_dir(cache_dir)?; let log_path = cache_dir.join("things.log"); let cursor_path = cache_dir.join("cursor.json"); From b4558bf31fb211211ec2973ee3959cf2dbdeca32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Pereira?= <484633+joaodrp@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:46:08 +0100 Subject: [PATCH 2/2] Create the credential file private before writing the password `write_auth` wrote the payload at the process umask and narrowed the mode only after the rename, leaving the plaintext password world-readable in between. The staging path is also a fixed `.tmp` name, so anything able to write the directory can pre-plant a symlink and have the password written through it. Create the staging file with `create_new` at `0600` instead, then restate the mode before writing: `open` filters the requested mode through the umask, so a restrictive one would otherwise leave the file unreadable to its owner. The rename carries the mode onto the final file. Adds a regression test that plants a symlink at the staging path and asserts the target is untouched. It fails on the previous implementation. --- src/auth.rs | 80 ++++++++++++++++++++++++++++++++++++++++-------- src/log_cache.rs | 3 +- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index aea52a5..1394340 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,4 +1,4 @@ -use std::fs; +use std::{fs, io::Write}; use anyhow::{Context, Result, anyhow}; use figment::{ @@ -69,29 +69,83 @@ pub fn load_auth() -> Result<(String, String)> { } pub fn write_auth(email: &str, password: &str) -> Result { - let (email, password) = validate_auth(email, password)?; let path = auth_file_path(); + write_auth_at(&path, email, password)?; + Ok(path) +} + +fn write_auth_at(path: &std::path::Path, email: &str, password: &str) -> Result<()> { + let (email, password) = validate_auth(email, password)?; let parent = path .parent() - .ok_or_else(|| anyhow!("Invalid auth file path"))? - .to_path_buf(); - create_private_dir(&parent).with_context(|| format!("Failed creating {}", parent.display()))?; + .ok_or_else(|| anyhow!("Invalid auth file path"))?; + create_private_dir(parent).with_context(|| format!("Failed creating {}", parent.display()))?; let payload = AuthPayload { email, password }; let serialized = serde_json::to_string(&payload)?; let tmp_path = path.with_extension("tmp"); - fs::write(&tmp_path, serialized) - .with_context(|| format!("Failed writing {}", tmp_path.display()))?; - fs::rename(&tmp_path, &path) - .with_context(|| format!("Failed finalizing {}", path.display()))?; + // Create the staging file already private: chmod'ing after the fact leaves + // the plaintext password world-readable in between, and create_new won't + // follow a symlink planted at tmp_path. The rename carries the mode over. + let mut opts = fs::OpenOptions::new(); + opts.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + opts.mode(0o600); + } + // Clear a stale staging file from an interrupted run; create_new refuses it. + let _ = fs::remove_file(&tmp_path); + let mut file = opts + .open(&tmp_path) + .with_context(|| format!("Failed writing {}", tmp_path.display()))?; + // open() filters the requested mode through the umask, which can clear + // owner bits too, so restate it before the password goes in. The file is + // already no wider than 0600, so this opens no window. #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - let mut perms = fs::metadata(&path)?.permissions(); - perms.set_mode(0o600); - fs::set_permissions(&path, perms)?; + fs::set_permissions(&tmp_path, fs::Permissions::from_mode(0o600)) + .with_context(|| format!("Failed securing {}", tmp_path.display()))?; } + file.write_all(serialized.as_bytes()) + .with_context(|| format!("Failed writing {}", tmp_path.display()))?; - Ok(path) + fs::rename(&tmp_path, path).with_context(|| format!("Failed finalizing {}", path.display()))?; + + Ok(()) +} + +#[cfg(all(test, unix))] +mod tests { + use super::*; + use std::os::unix::fs::PermissionsExt; + + #[test] + fn auth_file_is_created_private() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("auth.json"); + write_auth_at(&path, "user@example.com", "hunter2").expect("write auth"); + + let mode = fs::metadata(&path).expect("metadata").permissions().mode(); + assert_eq!(mode & 0o777, 0o600); + } + + #[test] + fn password_is_not_written_through_a_planted_symlink() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("auth.json"); + let victim = dir.path().join("victim"); + fs::write(&victim, "original").expect("seed victim"); + std::os::unix::fs::symlink(&victim, path.with_extension("tmp")).expect("plant symlink"); + + write_auth_at(&path, "user@example.com", "hunter2").expect("write auth"); + + assert_eq!( + fs::read_to_string(&victim).expect("victim"), + "original", + "the password must not be written through a symlink at the staging path" + ); + } } diff --git a/src/log_cache.rs b/src/log_cache.rs index 565cfec..2fa9e79 100644 --- a/src/log_cache.rs +++ b/src/log_cache.rs @@ -10,6 +10,7 @@ use serde_json::Value; use crate::{ client::ThingsCloudClient, + dirs::create_private_dir, store::{RawState, fold_item}, wire::wire_object::WireItem, }; @@ -67,7 +68,7 @@ fn write_cursor( } pub fn sync_append_log(client: &mut ThingsCloudClient, cache_dir: &Path) -> Result<()> { - crate::dirs::create_private_dir(cache_dir)?; + create_private_dir(cache_dir)?; let log_path = cache_dir.join("things.log"); let cursor_path = cache_dir.join("cursor.json");