diff --git a/.gitignore b/.gitignore index b3e17c411e..a52be32a1a 100644 --- a/.gitignore +++ b/.gitignore @@ -18,4 +18,3 @@ target/ .DS_Store .vscode/ .idea/ -.port_sessions/ diff --git a/.jules/sentinel.md b/.jules/sentinel.md index ab05e218c0..62d5110fb2 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -1,3 +1,8 @@ +## 2025-04-11 - Path Traversal Vulnerability in Session IDs +**Vulnerability:** The Python `src/session_store.py` and Rust `rust/crates/runtime/src/session_control.rs` allow path traversal (e.g. `../` or `/`) via the `session_id` property when saving and loading sessions. +**Learning:** This exposes a critical vulnerability where an attacker controlling the `session_id` can write or read arbitrary `.json` / `.jsonl` files on the filesystem. The session identifiers must be strictly validated to exclude path separators ('/', '\') and special segments ('.', '..') to mitigate path traversal vulnerabilities. +**Prevention:** Explicitly validate input IDs to ensure they only contain safe characters (e.g., alphanumeric, hyphens, underscores) or strictly reject path separators and directory traversal sequences. + ## 2024-04-10 - Path Traversal in Session Storage **Vulnerability:** Path traversal existed in both the Python (`src/session_store.py`) and Rust (`rust/crates/runtime/src/session_control.rs`) implementations because unsanitized `session_id` strings were used directly in file paths. **Learning:** Both reference implementations lacked central validation logic for system identifiers derived from external/user input. diff --git a/rust/crates/runtime/src/session_control.rs b/rust/crates/runtime/src/session_control.rs index 11b7905f00..9136cc828f 100644 --- a/rust/crates/runtime/src/session_control.rs +++ b/rust/crates/runtime/src/session_control.rs @@ -74,12 +74,17 @@ impl SessionStore { &self.workspace_root } - pub fn create_handle(&self, session_id: &str) -> Result { + pub fn validate_session_id(session_id: &str) -> Result<(), SessionControlError> { if !is_valid_session_id(session_id) { - return Err(SessionControlError::Format(format!( - "Invalid session ID: {session_id}" - ))); + return Err(SessionControlError::Format( + "Invalid session ID".to_string(), + )); } + Ok(()) + } + + pub fn create_handle(&self, session_id: &str) -> Result { + Self::validate_session_id(session_id)?; let id = session_id.to_string(); let path = self .sessions_root @@ -120,11 +125,7 @@ impl SessionStore { } pub fn resolve_managed_path(&self, session_id: &str) -> Result { - if !is_valid_session_id(session_id) { - return Err(SessionControlError::Format(format!( - "Invalid session ID: {session_id}" - ))); - } + Self::validate_session_id(session_id)?; for extension in [PRIMARY_SESSION_EXTENSION, LEGACY_SESSION_EXTENSION] { let path = self.sessions_root.join(format!("{session_id}.{extension}")); if path.exists() { @@ -353,9 +354,9 @@ pub fn create_managed_session_handle_for( session_id: &str, ) -> Result { if !is_valid_session_id(session_id) { - return Err(SessionControlError::Format(format!( - "Invalid session ID: {session_id}" - ))); + return Err(SessionControlError::Format( + "Invalid session ID".to_string(), + )); } let id = session_id.to_string(); let path = @@ -420,9 +421,9 @@ pub fn resolve_managed_session_path_for( session_id: &str, ) -> Result { if !is_valid_session_id(session_id) { - return Err(SessionControlError::Format(format!( - "Invalid session ID: {session_id}" - ))); + return Err(SessionControlError::Format( + "Invalid session ID".to_string(), + )); } let directory = managed_sessions_dir_for(base_dir)?; for extension in [PRIMARY_SESSION_EXTENSION, LEGACY_SESSION_EXTENSION] { @@ -742,7 +743,7 @@ mod tests { .expect("session message should save"); let handle = store .create_handle(&session.session_id) - .expect("handle should create"); + .expect("should create handle"); let session = session.with_persistence_path(handle.path.clone()); session .save_to_path(&handle.path) diff --git a/src/session_store.py b/src/session_store.py index a2f1600aa4..b0fbd122a4 100644 --- a/src/session_store.py +++ b/src/session_store.py @@ -17,14 +17,14 @@ class StoredSession: def validate_session_id(session_id: str) -> None: + if not session_id: + raise ValueError("Invalid session ID: cannot be empty") if "/" in session_id or "\\" in session_id: - raise ValueError(f"Invalid session ID: contains path separators ({session_id})") + raise ValueError("Invalid session ID: contains path separators") if session_id in (".", ".."): - raise ValueError(f"Invalid session ID: cannot be '.' or '..' ({session_id})") + raise ValueError("Invalid session ID: cannot be '.' or '..'") if ".." in session_id: - raise ValueError( - f"Invalid session ID: contains directory traversal ('..') ({session_id})" - ) + raise ValueError("Invalid session ID: contains directory traversal ('..')") def save_session(session: StoredSession, directory: Path | None = None) -> Path: