From d4b753a74178df67ea4318e72041b7447bde4f6d Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Sat, 11 Apr 2026 12:54:47 +0000
Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?=
 =?UTF-8?q?=20Fix=20Path=20Traversal=20in=20Snapshot=20Command?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

🚨 Severity: CRITICAL
💡 Vulnerability: The snapshot name parameter was used directly in `Path::join` without validation, allowing a malicious user to overwrite or read files outside the designated snapshots directory using payloads like `../../../etc/passwd`.
🎯 Impact: Local privilege escalation, arbitrary file overwrite, or sensitive data leakage (such as private keys or system files) depending on the context.
🔧 Fix: Added strict path validation (`validate_file_name`) to `src/utils.rs` allowing only alphanumeric characters, underscores, and dashes. Applied this to `Save` and `Restore` snapshot functions. Also fixed an insecure directory creation issue by using `create_secure_dir_all`.
✅ Verification: Ensure the provided snapshot name is rejected if it contains malicious sequences. Run `cargo test` and verify expected behavior in save and restore actions.

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

diff --git a/.jules/sentinel.md b/.jules/sentinel.md
index 986d7ff..fbfb06f 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-04-11 - Path Traversal Vulnerability
+**Vulnerability:** The snapshot action in `src/commands/snapshot.rs` was prone to a path traversal vulnerability because the snapshot name was used to join paths without validating the input, allowing directory traversal with payloads such as `../../../etc/passwd`.
+**Learning:** Never trust user input to safely construct file paths using `Path::join`. Malicious payloads can bypass intended directory limitations.
+**Prevention:** Validate that input intended for file or directory creation/access adheres strictly to an allowlist of valid characters (like alphanumeric, underscores, and dashes) using `validate_file_name`.
diff --git a/src/commands/snapshot.rs b/src/commands/snapshot.rs
index 7be1d9e..7c4f462 100644
--- a/src/commands/snapshot.rs
+++ b/src/commands/snapshot.rs
@@ -7,11 +7,12 @@ use std::fs;
 pub async fn run(cli: &Cli, args: &SnapshotArgs) -> Result<CommandOutput, AppError> {
     let profile_path = profile_path(cli)?;
     let snap_dir = snapshot_dir(cli)?;
-    fs::create_dir_all(&snap_dir)
+    crate::paths::create_secure_dir_all(&snap_dir)
         .map_err(|e| AppError::Config(format!("failed to create snapshot dir: {e}")))?;
 
     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..a698106 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -185,6 +185,22 @@ pub fn resolve_psbt_source(
     ))
 }
 
+pub fn validate_file_name(name: &str) -> Result<(), AppError> {
+    if name.is_empty() {
+        return Err(AppError::Invalid("file name cannot be empty".to_string()));
+    }
+    if !name
+        .chars()
+        .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
+    {
+        return Err(AppError::Invalid(
+            "file name can only contain alphanumeric characters, underscores, and dashes"
+                .to_string(),
+        ));
+    }
+    Ok(())
+}
+
 pub fn parse_indices(s: Option<&str>) -> Result<Vec<usize>, AppError> {
     let s = match s {
         Some(s) => s,