mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
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
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user