From c5b5fa8089954219302092c03b8efc2024d33df6 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Tue, 1 Sep 2026 20:37:56 +0000 Subject: [PATCH] Support thread forks from symlinked session roots (#42135) ## Why Forking a resumed paginated thread failed when the managed `sessions` directory was a symlink, even though the rollout remained under that managed root. ## What changed - Validate rollout lineage references against the canonical `sessions` and `archived_sessions` roots. - Accept rollouts beneath symlinked managed roots while rejecting nested symlinks that escape those roots. ## Testing - Cover forking a resumed rollout under a symlinked `sessions` root. - Cover rejection of rollout lineage paths that escape through a nested symlink. GitOrigin-RevId: 0128ecde7087120ef510baa3ed62cdbb33073117 --- .../app-server/tests/suite/v2/thread_fork.rs | 56 +++++++++++++++++++ .../thread-store/src/local/rollout_lineage.rs | 31 ++++++++-- .../src/local/rollout_lineage_tests.rs | 31 ++++++++++ 3 files changed, 113 insertions(+), 5 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/thread_fork.rs b/codex-rs/app-server/tests/suite/v2/thread_fork.rs index 5d6729bfbd..830d7e713d 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_fork.rs @@ -1508,6 +1508,62 @@ async fn thread_fork_creates_reference_backed_paginated_thread() -> Result<()> { Ok(()) } +#[cfg(unix)] +#[tokio::test] +async fn thread_fork_accepts_resumed_rollout_under_symlinked_sessions_root() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + let external = TempDir::new()?; + let external_sessions = external.path().join("sessions"); + std::fs::create_dir_all(external_sessions.as_path())?; + std::os::unix::fs::symlink( + external_sessions.as_path(), + codex_home.path().join("sessions"), + )?; + MockResponsesConfig::new(&server.uri()).write(codex_home.path())?; + + let conversation_id = create_fake_paginated_rollout( + codex_home.path(), + "2025-01-05T12-00-00", + "2025-01-05T12:00:00Z", + "Saved user message", + Some("mock_provider"), + /*git_info*/ None, + )?; + let source_path = rollout_path( + codex_home.path(), + "2025-01-05T12-00-00", + conversation_id.as_str(), + ); + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build_initialized() + .await?; + + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id: conversation_id.clone(), + path: Some(source_path), + ..Default::default() + }) + .await?; + let _: ThreadResumeResponse = + timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??; + + let fork_id = mcp + .send_thread_fork_request(ThreadForkParams { + thread_id: conversation_id.clone(), + ..Default::default() + }) + .await?; + let ThreadForkResponse { thread, .. } = + timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(fork_id)).await??; + + assert_eq!(thread.forked_from_id, Some(conversation_id)); + Ok(()) +} + #[tokio::test] async fn thread_fork_warns_for_paginated_full_history_hydration() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; diff --git a/codex-rs/thread-store/src/local/rollout_lineage.rs b/codex-rs/thread-store/src/local/rollout_lineage.rs index e290756ee4..ae3e1bec62 100644 --- a/codex-rs/thread-store/src/local/rollout_lineage.rs +++ b/codex-rs/thread-store/src/local/rollout_lineage.rs @@ -94,11 +94,32 @@ impl LocalThreadStore { } let rollout_path = match representation { LineageRepresentation::Existing => rollout_path, - LineageRepresentation::PlainForReference => super::helpers::scoped_rollout_path( - self.config.codex_home.clone(), - rollout_path.as_path(), - "Codex home", - )?, + LineageRepresentation::PlainForReference => { + let outside_codex_home = || ThreadStoreError::InvalidRequest { + message: format!( + "rollout path `{}` must be in Codex home directory", + rollout_path.display() + ), + }; + let canonical_rollout_path = std::fs::canonicalize(rollout_path.as_path()) + .map_err(|_| outside_codex_home())?; + // Resume can retain either the logical Codex home path or its canonical + // target. Keep references inside canonical managed roots so nested symlinks + // cannot escape them. + let is_managed_rollout = [ + self.config.codex_home.join(codex_rollout::SESSIONS_SUBDIR), + self.config + .codex_home + .join(codex_rollout::ARCHIVED_SESSIONS_SUBDIR), + ] + .into_iter() + .filter_map(|root| std::fs::canonicalize(root).ok()) + .any(|root| canonical_rollout_path.starts_with(root)); + if !is_managed_rollout { + return Err(outside_codex_home()); + } + canonical_rollout_path + } }; let meta = codex_rollout::read_session_meta_line(rollout_path.as_path()) .await diff --git a/codex-rs/thread-store/src/local/rollout_lineage_tests.rs b/codex-rs/thread-store/src/local/rollout_lineage_tests.rs index 2935ac3889..4f4f762aec 100644 --- a/codex-rs/thread-store/src/local/rollout_lineage_tests.rs +++ b/codex-rs/thread-store/src/local/rollout_lineage_tests.rs @@ -15,6 +15,37 @@ use super::super::LocalThreadStore; use super::super::test_support::test_config; use super::RolloutLineageSegment; +#[cfg(unix)] +#[tokio::test] +async fn rejects_reference_lineage_escaping_symlinked_sessions_root() { + let home = TempDir::new().expect("temp dir"); + let external = TempDir::new().expect("external temp dir"); + let external_sessions = external.path().join("sessions"); + fs::create_dir_all(external_sessions.as_path()).expect("create external sessions"); + std::os::unix::fs::symlink(external_sessions.as_path(), home.path().join("sessions")) + .expect("symlink sessions"); + let escaped = TempDir::new().expect("escaped temp dir"); + let escaped_rollouts = escaped.path().join("rollouts"); + fs::create_dir_all(escaped_rollouts.as_path()).expect("create escaped rollouts"); + std::os::unix::fs::symlink(escaped_rollouts.as_path(), external_sessions.join("2026")) + .expect("symlink nested sessions directory"); + let store = LocalThreadStore::new(test_config(home.path()), /*state_db*/ None); + let thread_id = ThreadId::default(); + write_rollout( + home.path(), + thread_id, + /*history_base*/ None, + /*next_ordinal*/ 3, + ); + + let error = store + .resolve_rollout_lineage_for_reference(thread_id) + .await + .expect_err("escaping reference lineage should be rejected"); + + assert!(error.to_string().contains("must be in Codex home")); +} + #[tokio::test] async fn resolves_nested_lineage_with_empty_intermediate_segments() { let home = TempDir::new().expect("temp dir");