diff --git a/.changes/session-id-path-validation.md b/.changes/session-id-path-validation.md new file mode 100644 index 000000000..1203f4692 --- /dev/null +++ b/.changes/session-id-path-validation.md @@ -0,0 +1 @@ +- A session file whose header id is not a plain file name (for example `../x` or an absolute path) is now treated as an invalid session, so its artifacts can no longer be written outside the session-artifacts folder. diff --git a/crates/pa-cli/src/interactive_mode.rs b/crates/pa-cli/src/interactive_mode.rs index 27f35cacf..ce34b7df0 100644 --- a/crates/pa-cli/src/interactive_mode.rs +++ b/crates/pa-cli/src/interactive_mode.rs @@ -731,9 +731,6 @@ fn continue_recent_view( let cwd = options.config.cwd.clone(); let path = pa_core::session::discovery::find_most_recent_session_for_cwd(&session_dir, &cwd)?; let header = pa_core::session::manager::read_session_header(&path)?; - if header.id.is_empty() { - return None; - } Some(ContinueRecentView { session_id: header.id.clone(), notice: format!( diff --git a/crates/pa-core/src/session/discovery.rs b/crates/pa-core/src/session/discovery.rs index 7af71461e..c142e1fa3 100644 --- a/crates/pa-core/src/session/discovery.rs +++ b/crates/pa-core/src/session/discovery.rs @@ -128,7 +128,7 @@ pub fn scan_session_headers(session_dir: &Path) -> Vec { continue; } if let Some(header) = read_session_header(&path) { - if header.id.is_empty() || header.cwd.is_empty() { + if header.cwd.is_empty() { continue; } headers.push(SessionHeaderInfo { diff --git a/crates/pa-core/src/session/manager/tests.rs b/crates/pa-core/src/session/manager/tests.rs index d8dd28973..a0fc53f72 100644 --- a/crates/pa-core/src/session/manager/tests.rs +++ b/crates/pa-core/src/session/manager/tests.rs @@ -394,3 +394,33 @@ fn fork_from_rejects_a_non_regular_source() { ) ); } + +#[test] +fn open_treats_a_path_shaped_header_id_as_corrupted() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path().join("sessions"); + std::fs::create_dir_all(&dir).unwrap(); + let artifacts_root = tmp.path().join("session-artifacts"); + let file = dir.join("crafted.jsonl"); + for crafted in ["../../outside", "/abs/outside"] { + let line = serde_json::json!({ + "type": "session", "version": 3, "id": crafted, + "timestamp": "2026-01-01T00:00:00.000Z", "cwd": "/w" + }) + .to_string(); + std::fs::write(&file, format!("{line}\n")).unwrap(); + let manager = SessionManager::open(tmp.path(), &dir, &file); + let artifact_dir = manager.get_session_artifact_dir().unwrap(); + assert_eq!( + ( + artifact_dir.parent(), + read_session_header(&file).map(|header| header.id), + ), + ( + Some(artifacts_root.as_path()), + Some(manager.get_session_id().to_owned()), + ), + "{crafted}: the corrupted-header reset mints a fresh id and rewrites the file", + ); + } +} diff --git a/crates/pa-daemon/src/session_archive.rs b/crates/pa-daemon/src/session_archive.rs index 2f9cc1709..6d5f34dae 100644 --- a/crates/pa-daemon/src/session_archive.rs +++ b/crates/pa-daemon/src/session_archive.rs @@ -201,9 +201,6 @@ fn collect_candidates( let Some(header) = crate::session_store::read_session_header(&path) else { continue; }; - if header.id.is_empty() { - continue; - } candidates.push(SweepCandidate { protected: protected.contains(&canonical_session_path(&path)), session_id: header.id, diff --git a/crates/pa-daemon/src/session_store/read.rs b/crates/pa-daemon/src/session_store/read.rs index 8ae280420..9513cf751 100644 --- a/crates/pa-daemon/src/session_store/read.rs +++ b/crates/pa-daemon/src/session_store/read.rs @@ -121,11 +121,11 @@ pub fn read_session_header(path: &Path) -> Option { parse_session_header_line(&first) } -/// A session file is valid when its first line is a `session` header with an -/// id, judged on the bounded header read. +/// A session file is valid when its first line is a `session` header, judged +/// on the bounded header read. #[must_use] pub fn is_valid_session_file(path: &Path) -> bool { - read_session_header_bounded(path).is_some_and(|header| !header.id.is_empty()) + read_session_header_bounded(path).is_some() } impl SessionFile { diff --git a/crates/pa-types/src/session.rs b/crates/pa-types/src/session.rs index d1b357a5b..f3d3a36d2 100644 --- a/crates/pa-types/src/session.rs +++ b/crates/pa-types/src/session.rs @@ -47,6 +47,10 @@ pub struct SessionHeader { /// (the workspace's `serde_json` runs with `preserve_order`). #[serde(default, skip_serializing_if = "Option::is_none")] pub version: Option, + /// A single file name: the session's artifact dir joins it under + /// `session-artifacts/`, so a header id that names another path is a + /// malformed header. + #[serde(deserialize_with = "deserialize_session_id")] pub id: String, /// ISO-8601 creation timestamp. pub timestamp: String, @@ -61,6 +65,19 @@ pub struct SessionHeader { pub rest: JsonMap, } +fn deserialize_session_id<'de, D: serde::Deserializer<'de>>( + deserializer: D, +) -> Result { + let id = String::deserialize(deserializer)?; + if matches!(id.as_str(), "" | "." | "..") || id.contains(['/', '\\', ':']) { + return Err(serde::de::Error::invalid_value( + serde::de::Unexpected::Str(&id), + &"a session id that is a single file name", + )); + } + Ok(id) +} + // --------------------------------------------------------------------------- // Coding-agent message roles (JSONL `message` entries) // --------------------------------------------------------------------------- @@ -722,4 +739,33 @@ mod tests { fn non_object_entry_is_rejected() { assert!(serde_json::from_str::(r#""not an entry""#).is_err()); } + + #[test] + fn session_header_rejects_ids_that_are_not_a_file_name() { + let ids = [ + "019a0000-0000-7000-8000-000000000000", + "../../outside", + "/abs/outside", + "a\\b", + "C:outside", + "..", + ".", + "", + ]; + let parsed: Vec = ids + .iter() + .map(|id| { + let line = serde_json::json!({ + "type": "session", "version": 3, "id": id, + "timestamp": "2026-01-01T00:00:00.000Z", "cwd": "/w" + }) + .to_string(); + matches!(entry(&line), FileEntry::Header { .. }) + }) + .collect(); + assert_eq!( + parsed, + [true, false, false, false, false, false, false, false] + ); + } }