From 902bd9e06b3ecb32cbf7f8e64cd23b956be3e7fe Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 13 Aug 2026 04:01:54 +0000 Subject: [PATCH] Protect inline visualization viewers from sandbox writes (#38306) ## Why Inline visualization viewer documents must remain outside locations that a sandboxed session can modify before they are opened in a browser. ## What changed - Materialize viewer documents in a dedicated cache under `CODEX_HOME`, keyed by the source and artifact thread IDs, instead of alongside visualization artifacts. - Create visualization links only when the active filesystem policy cannot write to the viewer cache. This also disables links for full-disk-write sessions and applies the policy consistently when loading transcript history. - Reject viewer cache paths containing symbolic links and track materialized documents in memory so unchanged viewers are reused without trusting existing file contents. ## Testing - Verify that rendering an unchanged visualization reuses the existing viewer file, while an updated fragment refreshes it. GitOrigin-RevId: 12b7fbe522a68076e5d683a987b7801940d131c2 --- codex-rs/tui/src/app/history_pagination.rs | 4 +- .../app/tests/session_lifecycle_requests.rs | 14 +++--- codex-rs/tui/src/app/transcript_export.rs | 8 ++-- .../tui/src/app_server_session/history.rs | 2 +- codex-rs/tui/src/inline_visualization.rs | 48 +++++++++++++++---- .../tui/src/inline_visualization/viewer.rs | 23 +++++---- .../tui/src/inline_visualization_tests.rs | 25 ++++++++++ codex-rs/tui/src/lib.rs | 4 +- codex-rs/tui/src/resume_picker.rs | 25 +++++----- codex-rs/tui/src/thread_transcript.rs | 15 +++--- 10 files changed, 116 insertions(+), 52 deletions(-) diff --git a/codex-rs/tui/src/app/history_pagination.rs b/codex-rs/tui/src/app/history_pagination.rs index ebc31ccbb3..725f8d74e6 100644 --- a/codex-rs/tui/src/app/history_pagination.rs +++ b/codex-rs/tui/src/app/history_pagination.rs @@ -126,7 +126,7 @@ impl App { &cwd, user_items.iter().map(|(_, item)| item.clone()), visibility, - Some(self.config.codex_home.as_path()), + Some(&self.config), ); let mut persisted_user_cells = user_items .into_iter() @@ -204,7 +204,7 @@ impl App { &cwd, items, visibility, - Some(self.config.codex_home.as_path()), + Some(&self.config), ); if self.backtrack.overlay_preview_active { self.backtrack.nth_user_message = self.backtrack.nth_user_message.saturating_add( diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index 2c36f0d809..aaaac62a12 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -406,7 +406,7 @@ async fn older_pagination_reconciles_review_prompts_across_page_boundaries() -> &app.config.cwd, started.turns.iter().flat_map(|turn| turn.items.clone()), crate::thread_transcript::RawReasoningVisibility::Hidden, - Some(app.config.codex_home.as_path()), + Some(&app.config), ); app.enqueue_primary_thread_session(started.session, started.turns) .await?; @@ -575,7 +575,7 @@ async fn transcript_home_loads_every_older_history_page() -> Result<()> { &app.config.cwd, started.turns.iter().flat_map(|turn| turn.items.clone()), crate::thread_transcript::RawReasoningVisibility::Hidden, - Some(app.config.codex_home.as_path()), + Some(&app.config), ); app.enqueue_primary_thread_session(started.session, started.turns) .await?; @@ -718,7 +718,7 @@ async fn remote_legacy_history_start_negotiates_once_for_resume_and_fork() -> Re &mut app_server, paginated_thread_id, crate::thread_transcript::RawReasoningVisibility::Hidden, - Some(app.config.codex_home.as_path()), + Some(&app.config), vec![Arc::new(PlainHistoryCell::new(vec!["visible".into()]))], ) .await @@ -940,7 +940,7 @@ async fn underfilled_scrollback_fetches_older_pages_without_opening_the_transcri &app.config.cwd, started.turns.iter().flat_map(|turn| turn.items.clone()), crate::thread_transcript::RawReasoningVisibility::Hidden, - Some(app.config.codex_home.as_path()), + Some(&app.config), ); initial_cells.insert( /*index*/ 0, @@ -1076,7 +1076,7 @@ async fn paginated_workflows_never_request_full_thread_history() -> Result<()> { &mut app_server, paginated_thread_id, crate::thread_transcript::RawReasoningVisibility::Hidden, - Some(app.config.codex_home.as_path()), + Some(&app.config), ) .await?; assert!(!cells.is_empty()); @@ -1105,7 +1105,7 @@ async fn paginated_workflows_never_request_full_thread_history() -> Result<()> { &mut app_server, legacy_thread_id, crate::thread_transcript::RawReasoningVisibility::Hidden, - Some(app.config.codex_home.as_path()), + Some(&app.config), ) .await?; let legacy_reads = recorded_params(&requests, "thread/read"); @@ -1287,7 +1287,7 @@ async fn cold_paginated_subagent_transcript_excludes_inherited_parent_history() &mut app_server, child_thread_id, crate::thread_transcript::RawReasoningVisibility::Hidden, - Some(app.config.codex_home.as_path()), + Some(&app.config), ) .await?; let visible_history = cells diff --git a/codex-rs/tui/src/app/transcript_export.rs b/codex-rs/tui/src/app/transcript_export.rs index ca04127efa..c5dfbde9db 100644 --- a/codex-rs/tui/src/app/transcript_export.rs +++ b/codex-rs/tui/src/app/transcript_export.rs @@ -24,6 +24,7 @@ use crate::history_cell::ReasoningSummaryCell; use crate::history_cell::SessionInfoCell; use crate::history_cell::UserHistoryCell; use crate::history_cell::raw_lines_from_source; +use crate::legacy_core::config::Config; use crate::thread_transcript::RawReasoningVisibility; use crate::thread_transcript::thread_items_to_transcript_cells; @@ -42,12 +43,11 @@ impl App { } else { RawReasoningVisibility::Hidden }; - let codex_home = self.config.codex_home.to_path_buf(); let cells = load_export_transcript( app_server, thread_id, visibility, - Some(codex_home.as_path()), + Some(&self.config), self.transcript_cells.clone(), ) .await?; @@ -77,7 +77,7 @@ pub(super) async fn load_export_transcript( app_server: &mut AppServerSession, thread_id: ThreadId, visibility: RawReasoningVisibility, - codex_home: Option<&Path>, + config: Option<&Config>, visible_transcript: Vec>, ) -> Result>, String> { let mut thread = app_server @@ -123,7 +123,7 @@ pub(super) async fn load_export_transcript( &thread.cwd, [item], visibility, - codex_home, + config, )); } } diff --git a/codex-rs/tui/src/app_server_session/history.rs b/codex-rs/tui/src/app_server_session/history.rs index 0e1f4b3d00..aef301bf4b 100644 --- a/codex-rs/tui/src/app_server_session/history.rs +++ b/codex-rs/tui/src/app_server_session/history.rs @@ -308,7 +308,7 @@ fn rendered_history_rows( &thread.cwd, items, visibility, - Some(config.codex_home.as_path()), + Some(config), ) .into_iter() .fold(rendered_rows, |rows, cell| { diff --git a/codex-rs/tui/src/inline_visualization.rs b/codex-rs/tui/src/inline_visualization.rs index eb85c5bafe..657f867bab 100644 --- a/codex-rs/tui/src/inline_visualization.rs +++ b/codex-rs/tui/src/inline_visualization.rs @@ -19,6 +19,8 @@ use std::ops::Range; use std::path::Component; use std::path::Path; use std::path::PathBuf; +use std::sync::Arc; +use std::sync::Mutex; use url::Url; use uuid::Uuid; @@ -33,9 +35,12 @@ const MAX_FRAGMENT_BYTES: u64 = 2 * 1024 * 1024; pub(crate) struct InlineVisualizationContext { visualizations_dir: PathBuf, thread_dir: PathBuf, + viewer_dir: PathBuf, + materialized_viewers: Arc>>, } impl InlineVisualizationContext { + #[cfg(test)] pub(crate) fn new(codex_home: &Path, thread_id: ThreadId) -> Option { Self::new_with_writable_roots(codex_home, thread_id, std::iter::empty()) } @@ -44,15 +49,33 @@ impl InlineVisualizationContext { config: &crate::legacy_core::config::Config, thread_id: ThreadId, ) -> Option { - let writable_roots = config - .permissions - .file_system_sandbox_policy() - .get_writable_roots_with_cwd(config.cwd.as_path()); - Self::new_with_writable_roots( + let file_system_policy = config.permissions.file_system_sandbox_policy(); + if file_system_policy.has_full_disk_write_access() { + return None; + } + let writable_roots = file_system_policy.get_writable_roots_with_cwd(config.cwd.as_path()); + let context = Self::new_with_writable_roots( config.codex_home.as_path(), thread_id, writable_roots.iter().map(|root| root.root.as_path()), - ) + )?; + let viewer_caches = [ + config.codex_home.as_path().join("visualization-viewers"), + context.viewer_dir.parent()?.parent()?.to_path_buf(), + ]; + for viewer_cache in viewer_caches { + if file_system_policy.can_write_path_with_cwd(&viewer_cache, config.cwd.as_path()) + || file_system_policy + .can_write_path_with_cwd(viewer_cache.parent()?, config.cwd.as_path()) + || writable_roots.iter().any(|root| { + root.is_path_writable(&viewer_cache) + || root.root.as_path().starts_with(&viewer_cache) + }) + { + return None; + } + } + Some(context) } fn new_with_writable_roots<'a>( @@ -60,6 +83,7 @@ impl InlineVisualizationContext { thread_id: ThreadId, writable_roots: impl IntoIterator, ) -> Option { + let codex_home = fs::canonicalize(codex_home).ok()?; let thread_id = thread_id.to_string(); let uuid = Uuid::parse_str(&thread_id).ok()?; let timestamp = uuid.get_timestamp()?; @@ -74,11 +98,17 @@ impl InlineVisualizationContext { [thread_dir] => (*thread_dir).to_path_buf(), _ => visualizations_dir .join(created_at.format("%Y/%m/%d").to_string()) - .join(thread_id), + .join(&thread_id), }; + let artifact_thread_id = thread_dir.file_name()?.to_owned(); Some(Self { visualizations_dir, thread_dir, + viewer_dir: codex_home + .join("visualization-viewers") + .join(thread_id) + .join(artifact_thread_id), + materialized_viewers: Arc::default(), }) } @@ -110,7 +140,9 @@ impl InlineVisualizationContext { if !fragment_path.starts_with(&thread_dir) { return None; } - let viewer_path = materialize_document(&fragment_path, &thread_dir).ok()?; + let viewer_path = + materialize_document(&fragment_path, &self.viewer_dir, &self.materialized_viewers) + .ok()?; Url::from_file_path(viewer_path).ok() } } diff --git a/codex-rs/tui/src/inline_visualization/viewer.rs b/codex-rs/tui/src/inline_visualization/viewer.rs index d5335bce49..0a7bd7709b 100644 --- a/codex-rs/tui/src/inline_visualization/viewer.rs +++ b/codex-rs/tui/src/inline_visualization/viewer.rs @@ -1,8 +1,10 @@ use super::MAX_FRAGMENT_BYTES; +use std::collections::HashMap; use std::fs; use std::io::Write; use std::path::Path; use std::path::PathBuf; +use std::sync::Mutex; const FRAGMENT_PLACEHOLDER: &str = ""; const VIEWER_DIRECTORY_NAME: &str = ".codex-viewers"; @@ -14,7 +16,11 @@ const VIEWER_RUNTIME: &str = include_str!("assets/visualize.html"); const FRAME_CSP: &str = "default-src 'none'; script-src 'unsafe-inline' 'unsafe-eval' 'wasm-unsafe-eval' blob: data: https://cdnjs.cloudflare.com https://cdn.jsdelivr.net https://esm.sh https://fonts.bunny.net https://fonts.googleapis.com https://fonts.gstatic.com https://unpkg.com; style-src 'unsafe-inline' blob: data: https://cdnjs.cloudflare.com https://cdn.jsdelivr.net https://esm.sh https://fonts.bunny.net https://fonts.googleapis.com https://fonts.gstatic.com https://unpkg.com; img-src blob: data: https://cdnjs.cloudflare.com https://cdn.jsdelivr.net https://esm.sh https://fonts.bunny.net https://fonts.googleapis.com https://fonts.gstatic.com https://unpkg.com; font-src blob: data: https://cdnjs.cloudflare.com https://cdn.jsdelivr.net https://esm.sh https://fonts.bunny.net https://fonts.googleapis.com https://fonts.gstatic.com https://unpkg.com; media-src blob: data:; worker-src blob:; connect-src blob: data:; frame-src 'none'; object-src 'none'; base-uri 'none'; form-action 'none'"; const SHELL_STYLE: &str = ":root{color-scheme:light dark;background:light-dark(rgb(255 255 255), rgb(24 24 24))}html,body{margin:0}body{box-sizing:border-box;padding:1rem;background:inherit}iframe{display:block;width:100%;max-width:736px;height:calc(100vh - 2rem);margin:0 auto;border:0}"; -pub(super) fn materialize_document(path: &Path, thread_dir: &Path) -> std::io::Result { +pub(super) fn materialize_document( + path: &Path, + viewer_dir: &Path, + materialized_viewers: &Mutex>, +) -> std::io::Result { let metadata = path.metadata()?; if !metadata.is_file() || metadata.len() > MAX_FRAGMENT_BYTES { return Err(std::io::Error::other("invalid visualization fragment")); @@ -28,29 +34,30 @@ pub(super) fn materialize_document(path: &Path, thread_dir: &Path) -> std::io::R .replace('-', " "); let document = render_fragment(&fragment, &title); - let thread_dir = fs::canonicalize(thread_dir)?; - let viewer_dir = thread_dir.join(VIEWER_DIRECTORY_NAME); + let viewer_dir = viewer_dir.join(VIEWER_DIRECTORY_NAME); fs::create_dir_all(&viewer_dir)?; - let viewer_dir = fs::canonicalize(viewer_dir)?; - if !viewer_dir.starts_with(&thread_dir) { + if fs::canonicalize(&viewer_dir)? != viewer_dir { return Err(std::io::Error::other( - "visualization viewer directory escapes thread directory", + "visualization viewer directory must not contain symbolic links", )); } let file_name = path .file_name() .ok_or_else(|| std::io::Error::other("visualization fragment has no file name"))?; let viewer_path = viewer_dir.join(file_name); - if fs::read_to_string(&viewer_path).is_ok_and(|existing| existing == document) { + let mut materialized_viewers = materialized_viewers + .lock() + .map_err(|_| std::io::Error::other("visualization viewer cache is unavailable"))?; + if materialized_viewers.get(&viewer_path) == Some(&document) { return Ok(viewer_path); } - let mut temporary = tempfile::NamedTempFile::new_in(&viewer_dir)?; temporary.write_all(document.as_bytes())?; temporary.flush()?; temporary .persist(&viewer_path) .map_err(|error| error.error)?; + materialized_viewers.insert(viewer_path.clone(), document); Ok(viewer_path) } diff --git a/codex-rs/tui/src/inline_visualization_tests.rs b/codex-rs/tui/src/inline_visualization_tests.rs index 1f252431c8..6ddb15515d 100644 --- a/codex-rs/tui/src/inline_visualization_tests.rs +++ b/codex-rs/tui/src/inline_visualization_tests.rs @@ -244,12 +244,37 @@ fn viewer_reuses_path_and_refreshes_static_document() { let (_codex_home, context) = context_with_fragment("
first
"); let first_url = context.link_for("chart.html").expect("first viewer link"); let viewer_path = first_url.to_file_path().expect("viewer file path"); + let original_viewer_metadata = fs::metadata(&viewer_path).expect("read viewer metadata"); assert!( fs::read_to_string(&viewer_path) .expect("read first viewer") .contains("first") ); + let reused_url = context.link_for("chart.html").expect("reused viewer link"); + + assert_eq!(reused_url, first_url); + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + assert_eq!( + fs::metadata(&viewer_path) + .expect("read reused viewer metadata") + .ino(), + original_viewer_metadata.ino() + ); + } + #[cfg(windows)] + { + use std::os::windows::fs::MetadataExt; + assert_eq!( + fs::metadata(&viewer_path) + .expect("read reused viewer metadata") + .creation_time(), + original_viewer_metadata.creation_time() + ); + } + fs::write(context.thread_dir.join("chart.html"), "
second
").expect("update fragment"); let second_url = context.link_for("chart.html").expect("second viewer link"); diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 12a9bc4be2..8519cc549a 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -3164,7 +3164,7 @@ mod tests { let preview = crate::resume_picker::load_transcript_preview( &mut app_server, thread_id, - /*codex_home*/ None, + /*config*/ None, ) .await?; assert!(!preview.is_empty()); @@ -3173,7 +3173,7 @@ mod tests { &mut app_server, thread_id, crate::thread_transcript::RawReasoningVisibility::Hidden, - /*codex_home*/ None, + /*config*/ None, ) .await?; assert!(cells.len() > 100); diff --git a/codex-rs/tui/src/resume_picker.rs b/codex-rs/tui/src/resume_picker.rs index 86eef4b1b0..3b231cae76 100644 --- a/codex-rs/tui/src/resume_picker.rs +++ b/codex-rs/tui/src/resume_picker.rs @@ -445,7 +445,7 @@ async fn run_resume_picker_with_launch_context( archive_request_handle, include_non_interactive, raw_reasoning_visibility(config), - (!uses_remote_workspace).then(|| config.codex_home.to_path_buf()), + (!uses_remote_workspace).then(|| config.clone()), bg_tx, ), bg_rx, @@ -499,7 +499,7 @@ pub async fn run_fork_picker_with_app_server( archive_request_handle, /*include_non_interactive*/ false, raw_reasoning_visibility(config), - (!uses_remote_workspace).then(|| config.codex_home.to_path_buf()), + (!uses_remote_workspace).then(|| config.clone()), bg_tx, ), bg_rx, @@ -642,7 +642,7 @@ fn spawn_app_server_page_loader( archive_request_handle: AppServerRequestHandle, include_non_interactive: bool, raw_reasoning_visibility: RawReasoningVisibility, - codex_home: Option, + config: Option, bg_tx: mpsc::UnboundedSender, ) -> PickerLoader { let (request_tx, mut request_rx) = mpsc::unbounded_channel::(); @@ -671,8 +671,7 @@ fn spawn_app_server_page_loader( } PickerLoadRequest::Preview { thread_id } => { let preview = - load_transcript_preview(&mut app_server, thread_id, codex_home.as_deref()) - .await; + load_transcript_preview(&mut app_server, thread_id, config.as_ref()).await; let _ = bg_tx.send(BackgroundEvent::Preview { thread_id, preview }); } PickerLoadRequest::Transcript { @@ -684,7 +683,7 @@ fn spawn_app_server_page_loader( &mut app_server, thread_id, raw_reasoning_visibility, - codex_home.as_deref(), + config.as_ref(), ) => { let _ = bg_tx.send(BackgroundEvent::Transcript { thread_id, @@ -875,7 +874,7 @@ async fn load_app_server_page( pub(crate) async fn load_transcript_preview( app_server: &mut AppServerSession, thread_id: ThreadId, - codex_home: Option<&Path>, + config: Option<&Config>, ) -> std::io::Result> { const MAX_PREVIEW_LINES: usize = 6; @@ -896,10 +895,10 @@ pub(crate) async fn load_transcript_preview( .map_err(std::io::Error::other)?; } let cwd = thread.cwd.as_path(); - let inline_visualization_context = codex_home.and_then(|codex_home| { + let inline_visualization_context = config.and_then(|config| { ThreadId::from_string(&thread.id) .ok() - .and_then(|thread_id| InlineVisualizationContext::new(codex_home, thread_id)) + .and_then(|thread_id| InlineVisualizationContext::from_config(config, thread_id)) }); let mut lines = if thread.history_mode == ThreadHistoryMode::Paginated { let mut groups = Vec::new(); @@ -6452,7 +6451,7 @@ session_picker_view = "dense" let rendered = thread_to_transcript_cells( thread, RawReasoningVisibility::Visible, - /*codex_home*/ None, + /*config*/ None, ) .into_iter() .flat_map(|cell| cell.transcript_lines(/*width*/ 80)) @@ -6516,7 +6515,7 @@ session_picker_view = "dense" let hidden = thread_to_transcript_cells( thread.clone(), RawReasoningVisibility::Hidden, - /*codex_home*/ None, + /*config*/ None, ) .into_iter() .flat_map(|cell| cell.transcript_lines(/*width*/ 80)) @@ -6526,7 +6525,7 @@ session_picker_view = "dense" let visible = thread_to_transcript_cells( thread, RawReasoningVisibility::Visible, - /*codex_home*/ None, + /*config*/ None, ) .into_iter() .flat_map(|cell| cell.transcript_lines(/*width*/ 80)) @@ -6588,7 +6587,7 @@ session_picker_view = "dense" let rendered = thread_to_transcript_cells( thread, RawReasoningVisibility::Visible, - /*codex_home*/ None, + /*config*/ None, ) .into_iter() .flat_map(|cell| cell.transcript_lines(/*width*/ 80)) diff --git a/codex-rs/tui/src/thread_transcript.rs b/codex-rs/tui/src/thread_transcript.rs index f2142cc1de..18ff7f41de 100644 --- a/codex-rs/tui/src/thread_transcript.rs +++ b/codex-rs/tui/src/thread_transcript.rs @@ -12,6 +12,7 @@ use crate::history_cell::ReasoningSummaryCell; use crate::history_cell::UserHistoryCell; use crate::history_cell::split_reasoning_summary_parts; use crate::inline_visualization::InlineVisualizationContext; +use crate::legacy_core::config::Config; use crate::multi_agents::sub_agent_activity_summary; use codex_app_server_protocol::Thread; use codex_app_server_protocol::ThreadItem; @@ -34,7 +35,7 @@ pub(crate) async fn load_session_transcript( app_server: &mut AppServerSession, thread_id: ThreadId, raw_reasoning_visibility: RawReasoningVisibility, - codex_home: Option<&std::path::Path>, + config: Option<&Config>, ) -> std::io::Result { let mut thread = app_server .thread_read(thread_id, /*include_turns*/ false) @@ -53,14 +54,14 @@ pub(crate) async fn load_session_transcript( Ok(thread_to_transcript_cells( thread, raw_reasoning_visibility, - codex_home, + config, )) } pub(crate) fn thread_to_transcript_cells( thread: Thread, raw_reasoning_visibility: RawReasoningVisibility, - codex_home: Option<&std::path::Path>, + config: Option<&Config>, ) -> TranscriptCells { let cwd = thread.cwd; let thread_id = ThreadId::from_string(&thread.id).ok(); @@ -69,7 +70,7 @@ pub(crate) fn thread_to_transcript_cells( &cwd, thread.turns.into_iter().flat_map(|turn| turn.items), raw_reasoning_visibility, - codex_home, + config, ); if cells.is_empty() { cells.push(Arc::new(PlainHistoryCell::new(vec![ @@ -84,10 +85,10 @@ pub(crate) fn thread_items_to_transcript_cells( cwd: &AbsolutePathBuf, items: impl IntoIterator, raw_reasoning_visibility: RawReasoningVisibility, - codex_home: Option<&std::path::Path>, + config: Option<&Config>, ) -> TranscriptCells { - let inline_visualization_context = codex_home.and_then(|codex_home| { - thread_id.and_then(|thread_id| InlineVisualizationContext::new(codex_home, thread_id)) + let inline_visualization_context = config.and_then(|config| { + thread_id.and_then(|thread_id| InlineVisualizationContext::from_config(config, thread_id)) }); let mut cells: TranscriptCells = Vec::new(); for item in items {