From 2013754837223dd3774e2118517786d2137e8534 Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Tue, 14 Apr 2026 13:22:13 +0000
Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?=
 =?UTF-8?q?=20Fix=20path=20traversal=20in=20snapshot=20management?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Added `validate_file_name` in `src/utils.rs` with strict alphanumeric/dash/underscore allowlist.
- Validated user input for both snapshot Save and Restore actions to prevent path traversal vectors.

Co-authored-by: bitcoiner-dev <75873427+bitcoiner-dev@users.noreply.github.com>
---
 .jules/sentinel.md       |  5 +++++
 src/commands/snapshot.rs |  2 ++
 src/utils.rs             | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+)

diff --git a/.jules/sentinel.md b/.jules/sentinel.md
index 986d7ff..c1edd2a 100644
--- a/.jules/sentinel.md
+++ b/.jules/sentinel.md
@@ -7,3 +7,8 @@
 **Vulnerability:** The `maybe_write_text` utility function was using `std::fs::write`, which resulted in sensitive data (like PSBT files and offers) being saved with insecure default file permissions, making them readable by other users on a shared system.
 **Learning:** Even generic utility functions used for saving user-requested command outputs must use secure file permissions (`0o600`) if the data they handle (like PSBTs and offers) is sensitive.
 **Prevention:** Always use `crate::paths::write_secure_file` instead of `std::fs::write` for all file writing operations that might contain sensitive material in this codebase.
+
+## 2024-03-24 - Path Traversal in Snapshot Management
+**Vulnerability:** The `snapshot` command directly interpolated user-provided names into file paths (`snap_dir.join(format!("{name}.json"))`) without validation.
+**Learning:** This allowed path traversal (e.g., `../something`) which could lead to arbitrary file reads or writes outside the dedicated snapshot directory, potentially exposing sensitive data.
+**Prevention:** Always sanitize and validate user-provided file names using an explicit allowlist (like `crate::utils::validate_file_name`) before joining them to directory paths.
diff --git a/src/commands/snapshot.rs b/src/commands/snapshot.rs
index 7be1d9e..d4b1143 100644
--- a/src/commands/snapshot.rs
+++ b/src/commands/snapshot.rs
@@ -12,6 +12,7 @@ pub async fn run(cli: &Cli, args: &SnapshotArgs) -> Result<CommandOutput, AppErr
 
     match &args.action {
         SnapshotAction::Save { name, overwrite } => {
+            crate::utils::validate_file_name(name)?;
             let source = read_profile(&profile_path)?;
             let destination = snap_dir.join(format!("{name}.json"));
             if destination.exists() && !(*overwrite || cli.yes) {
@@ -27,6 +28,7 @@ pub async fn run(cli: &Cli, args: &SnapshotArgs) -> Result<CommandOutput, AppErr
             })
         }
         SnapshotAction::Restore { name } => {
+            crate::utils::validate_file_name(name)?;
             if !confirm(&format!("Are you sure you want to restore snapshot '{name}'? This will overwrite your current profile."), cli) {
                 return Err(AppError::Internal("aborted by user".to_string()));
             }
diff --git a/src/utils.rs b/src/utils.rs
index 8e3996f..15344d5 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -219,3 +219,36 @@ pub fn parse_indices(s: Option<&str>) -> Result<Vec<usize>, AppError> {
     }
     Ok(indices)
 }
+
+pub fn validate_file_name(name: &str) -> Result<(), AppError> {
+    if name.is_empty() {
+        return Err(AppError::Invalid("file name cannot be empty".to_string()));
+    }
+    for c in name.chars() {
+        if !c.is_ascii_alphanumeric() && c != '_' && c != '-' {
+            return Err(AppError::Invalid(format!(
+                "invalid character in file name: '{}'",
+                c
+            )));
+        }
+    }
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_validate_file_name() {
+        assert!(validate_file_name("valid_name-123").is_ok());
+        assert!(validate_file_name("valid").is_ok());
+
+        assert!(validate_file_name("").is_err());
+        assert!(validate_file_name("../invalid").is_err());
+        assert!(validate_file_name("invalid/name").is_err());
+        assert!(validate_file_name("invalid\\name").is_err());
+        assert!(validate_file_name("invalid name").is_err());
+        assert!(validate_file_name("invalid.name").is_err());
+    }
+}