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
This commit is contained in:
viyatb-oai
2026-08-13 04:08:23 +00:00
committed by copyberry
parent 363427b5e3
commit 902bd9e06b
10 changed files with 116 additions and 52 deletions
+2 -2
View File
@@ -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(
@@ -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
+4 -4
View File
@@ -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<Arc<dyn HistoryCell>>,
) -> Result<Vec<Arc<dyn HistoryCell>>, String> {
let mut thread = app_server
@@ -123,7 +123,7 @@ pub(super) async fn load_export_transcript(
&thread.cwd,
[item],
visibility,
codex_home,
config,
));
}
}
@@ -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| {
+40 -8
View File
@@ -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<Mutex<HashMap<PathBuf, String>>>,
}
impl InlineVisualizationContext {
#[cfg(test)]
pub(crate) fn new(codex_home: &Path, thread_id: ThreadId) -> Option<Self> {
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<Self> {
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<Item = &'a Path>,
) -> Option<Self> {
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()
}
}
@@ -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 = "<!--__INLINE_VISUALIZATION_FRAGMENT__-->";
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<PathBuf> {
pub(super) fn materialize_document(
path: &Path,
viewer_dir: &Path,
materialized_viewers: &Mutex<HashMap<PathBuf, String>>,
) -> std::io::Result<PathBuf> {
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)
}
@@ -244,12 +244,37 @@ fn viewer_reuses_path_and_refreshes_static_document() {
let (_codex_home, context) = context_with_fragment("<div>first</div>");
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"), "<div>second</div>").expect("update fragment");
let second_url = context.link_for("chart.html").expect("second viewer link");
+2 -2
View File
@@ -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);
+12 -13
View File
@@ -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<PathBuf>,
config: Option<Config>,
bg_tx: mpsc::UnboundedSender<BackgroundEvent>,
) -> PickerLoader {
let (request_tx, mut request_rx) = mpsc::unbounded_channel::<PickerLoadRequest>();
@@ -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<Vec<TranscriptPreviewLine>> {
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))
+8 -7
View File
@@ -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<TranscriptCells> {
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<Item = ThreadItem>,
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 {