From eb208579e3b91cc6394b04583424d422fa6a8277 Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 09:22:23 -0700 Subject: [PATCH] refactor(daemon): centralize username path-segment sanitizer Extract the inline path-traversal check from PendingStore into a shared storage::safe_user::join_user_segment helper. RecoveryStore (next) reuses it. Co-Authored-By: Claude Opus 4.7 (1M context) --- daemon/src/storage/mod.rs | 1 + daemon/src/storage/pending.rs | 12 +++------ daemon/src/storage/safe_user.rs | 45 +++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 9 deletions(-) create mode 100644 daemon/src/storage/safe_user.rs diff --git a/daemon/src/storage/mod.rs b/daemon/src/storage/mod.rs index 0301050..1d7d54b 100644 --- a/daemon/src/storage/mod.rs +++ b/daemon/src/storage/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod credentials; pub(crate) mod pending; pub(crate) mod policy; +pub(crate) mod safe_user; pub(crate) mod userdb; diff --git a/daemon/src/storage/pending.rs b/daemon/src/storage/pending.rs index cd2cc26..16d4a35 100644 --- a/daemon/src/storage/pending.rs +++ b/daemon/src/storage/pending.rs @@ -24,15 +24,9 @@ impl PendingStore { } fn user_path(&self, user: &str) -> Result { - if user.is_empty() - || user.contains('/') - || user.contains('\0') - || user == "." - || user == ".." - { - return Err(PendingError::InvalidUser(user.to_string())); - } - Ok(self.dir.join(user)) + super::safe_user::join_user_segment(&self.dir, user).map_err(|e| match e { + super::safe_user::SegmentError::Invalid(s) => PendingError::InvalidUser(s), + }) } pub fn set(&self, user: &str, flag: &PendingFlag) -> Result<(), PendingError> { diff --git a/daemon/src/storage/safe_user.rs b/daemon/src/storage/safe_user.rs new file mode 100644 index 0000000..fdf84a7 --- /dev/null +++ b/daemon/src/storage/safe_user.rs @@ -0,0 +1,45 @@ +//! Shared username sanitizer for path-segment use across pending/recovery stores. + +use std::path::{Path, PathBuf}; +use thiserror::Error; + +#[derive(Debug, Error)] +pub(crate) enum SegmentError { + #[error("invalid username segment: {0:?}")] + Invalid(String), +} + +/// Reject any username that could escape a single path segment. Centralized +/// so pending / recovery / future stores don't drift on what's "safe." +pub(crate) fn join_user_segment(dir: &Path, user: &str) -> Result { + if user.is_empty() + || user == "." + || user == ".." + || user.contains('/') + || user.contains('\0') + { + return Err(SegmentError::Invalid(user.to_string())); + } + Ok(dir.join(user)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn accepts_normal_username() { + let p = join_user_segment(Path::new("/tmp/x"), "alice").unwrap(); + assert_eq!(p, PathBuf::from("/tmp/x/alice")); + } + + #[test] + fn rejects_traversal_and_separators() { + for evil in ["", ".", "..", "a/b", "../etc", "x\0y"] { + assert!( + join_user_segment(Path::new("/tmp/x"), evil).is_err(), + "{evil:?} should be rejected" + ); + } + } +}