From ca5026134889f6bd0fb856b2d9abb74ac9c80fa1 Mon Sep 17 00:00:00 2001 From: Lahfir Date: Wed, 1 Jul 2026 07:03:03 -0700 Subject: [PATCH] fix(session): degrade any unreadable manifest, not just symlinks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit read_manifest already treated symlinked and corrupt-JSON manifests as a skipped (None) entry, but a plain PermissionDenied on one session's session.json propagated and aborted list_sessions — and therefore gc — for every other session. Degrade all non-NotFound open/read errors the same way so one restricted directory can no longer break enumeration or garbage collection. --- crates/core/src/session/mod.rs | 7 +++--- crates/core/src/session/session_tests.rs | 29 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/crates/core/src/session/mod.rs b/crates/core/src/session/mod.rs index 05fcba2a..97f23b3f 100644 --- a/crates/core/src/session/mod.rs +++ b/crates/core/src/session/mod.rs @@ -107,11 +107,12 @@ pub fn read_manifest(session_id: &str) -> Result, AppErr let mut file = match open_session_file(&path) { Ok(file) => file, Err(err) if err.kind() == ErrorKind::NotFound => return Ok(None), - Err(err) if is_symlinked(&path) => return Ok(ignore_unreadable_manifest(&path, &err)), - Err(err) => return Err(err.into()), + Err(err) => return Ok(ignore_unreadable_manifest(&path, &err)), }; let mut json = String::new(); - file.read_to_string(&mut json)?; + if let Err(err) = file.read_to_string(&mut json) { + return Ok(ignore_unreadable_manifest(&path, &err)); + } match serde_json::from_str(&json) { Ok(manifest) => Ok(Some(manifest)), Err(err) => Ok(ignore_unreadable_manifest(&path, &err)), diff --git a/crates/core/src/session/session_tests.rs b/crates/core/src/session/session_tests.rs index 27beb677..7a3aacec 100644 --- a/crates/core/src/session/session_tests.rs +++ b/crates/core/src/session/session_tests.rs @@ -226,6 +226,35 @@ fn corrupt_manifest_is_ignored_not_fatal() { assert!(!listed.iter().any(|id| id == "corruptsess")); } +#[cfg(unix)] +#[test] +fn unreadable_manifest_is_skipped_not_fatal_for_list_and_gc() { + use std::os::unix::fs::PermissionsExt; + + if unsafe { libc::geteuid() } == 0 { + return; + } + let _guard = HomeGuard::new(); + let good = start_session(StartSessionOptions { + name: None, + trace: SessionTraceMode::Off, + force: true, + }) + .unwrap(); + let bad_dir = session_dir("unreadablesess").unwrap(); + fs::create_dir_all(&bad_dir).unwrap(); + let manifest = bad_dir.join("session.json"); + fs::write(&manifest, b"{}").unwrap(); + fs::set_permissions(&manifest, fs::Permissions::from_mode(0o000)).unwrap(); + + let listed: Vec = list_sessions().unwrap().into_iter().map(|m| m.id).collect(); + assert!(listed.contains(&good.id)); + assert!(!listed.iter().any(|id| id == "unreadablesess")); + assert!(read_manifest("unreadablesess").unwrap().is_none()); + + fs::set_permissions(&manifest, fs::Permissions::from_mode(0o600)).unwrap(); +} + #[test] fn gc_leaves_recently_created_unended_session() { let _guard = HomeGuard::new();