From b0ef7f17081fd36db6d9ab6adfc8b64737f57a9b Mon Sep 17 00:00:00 2001 From: luren Date: Fri, 25 Sep 2026 17:17:40 +0800 Subject: [PATCH 1/4] shell: Enforce script network policy for TextView images --- Cargo.lock | 2 + crates/base/src/text/node.rs | 12 +- crates/base/src/text/state.rs | 3 + crates/base/src/text/text_view.rs | 20 + crates/shell/Cargo.toml | 4 + crates/shell/README.md | 11 + crates/shell/src/engine/quickjs/mod.rs | 13 +- crates/shell/src/materialize.rs | 11 +- crates/shell/src/materialize/text_view.rs | 390 ++++++++++++++++++ .../shell/src/materialize/text_view/tests.rs | 376 +++++++++++++++++ crates/shell/src/spec.rs | 46 ++- 11 files changed, 866 insertions(+), 22 deletions(-) create mode 100644 crates/shell/src/materialize/text_view.rs create mode 100644 crates/shell/src/materialize/text_view/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 4d41384eb5..0ac5eb185f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4525,6 +4525,7 @@ dependencies = [ "gpui-pre", "gpui-pre-platform", "gpui-pre-reqwest", + "image", "instant", "libc", "quickjs-jit", @@ -4536,6 +4537,7 @@ dependencies = [ "serde_json", "sha2 0.10.9", "smallvec", + "smol", "tracing", "tungstenite", "wait-timeout", diff --git a/crates/base/src/text/node.rs b/crates/base/src/text/node.rs index 392570e211..f4c51b5579 100644 --- a/crates/base/src/text/node.rs +++ b/crates/base/src/text/node.rs @@ -1935,6 +1935,7 @@ pub(crate) struct NodeContext { pub(crate) code_block_actions: Option>, pub(crate) code_block_highlighter: Option>, pub(crate) table_actions: Option>, + pub(crate) image_source: Option>, pub(crate) link_click_handler: Option>, pub(crate) markdown_extensions: Arc, /// This frame's streamed fade-in, when any text is still fading. @@ -1944,6 +1945,13 @@ pub(crate) struct NodeContext { } impl NodeContext { + fn image_source(&self, image: &ImageNode) -> ImageSource { + match &self.image_source { + Some(resolve) => resolve(&image.url), + None => image.source(), + } + } + pub(super) fn add_ref(&mut self, identifier: SharedString, link: LinkMark) { self.link_refs.insert(identifier, link); } @@ -2131,7 +2139,7 @@ impl Paragraph { } let link_click_handler = node_cx.link_click_handler.clone(); child_nodes.push( - img(image.source()) + img(node_cx.image_source(image)) .id(ix) .object_fit(ObjectFit::Contain) .max_w(relative(1.)) @@ -2343,7 +2351,7 @@ impl Paragraph { } items.push(InlineFlowItem::Image { - source: image.source(), + source: node_cx.image_source(image), link: image.link.clone(), title: image.title(), width: image.width, diff --git a/crates/base/src/text/state.rs b/crates/base/src/text/state.rs index 0efab77104..635aa55ac1 100644 --- a/crates/base/src/text/state.rs +++ b/crates/base/src/text/state.rs @@ -113,6 +113,7 @@ pub struct TextViewState { pub(super) code_block_actions: Option>, pub(super) code_block_highlighter: Option>, pub(super) table_actions: Option>, + pub(super) image_source: Option>, pub(super) link_click_handler: Option>, pub(super) markdown_extensions: Arc, @@ -244,6 +245,7 @@ impl TextViewState { code_block_highlighter: None, table_actions: None, link_click_handler: None, + image_source: None, markdown_extensions: Arc::default(), is_selecting: false, preserve_inline_selection: false, @@ -887,6 +889,7 @@ impl Render for TextViewState { code_block_highlighter: self.code_block_highlighter.clone(), table_actions: self.table_actions.clone(), link_click_handler: self.link_click_handler.clone(), + image_source: self.image_source.clone(), markdown_extensions: self.markdown_extensions.clone(), stream_fade, range_highlights: self.range_highlights.clone(), diff --git a/crates/base/src/text/text_view.rs b/crates/base/src/text/text_view.rs index 692102e86d..f725356559 100644 --- a/crates/base/src/text/text_view.rs +++ b/crates/base/src/text/text_view.rs @@ -74,6 +74,8 @@ impl TextViewDefaults { pub(crate) type TableActionsFn = dyn Fn(&TableData, &mut Window, &mut App) -> AnyElement + Send + Sync; +pub(crate) type ImageSourceFn = dyn Fn(&gpui::SharedUri) -> gpui::ImageSource + Send + Sync; + pub(crate) type LinkClickHandlerFn = dyn Fn(&SharedString, &ClickEvent, &mut Window, &mut App) + Send + Sync; @@ -129,6 +131,7 @@ pub struct TextView { code_block_highlighter: Option>, table_actions: Option>, link_click_handler: Option>, + image_source: Option>, markdown_extensions: Arc, motion: Option, } @@ -174,6 +177,7 @@ impl TextView { code_block_highlighter: None, table_actions: None, link_click_handler: None, + image_source: None, markdown_extensions: Arc::default(), motion: None, } @@ -196,6 +200,7 @@ impl TextView { code_block_highlighter: None, table_actions: None, link_click_handler: None, + image_source: None, markdown_extensions: Arc::default(), motion: None, } @@ -218,11 +223,25 @@ impl TextView { code_block_highlighter: None, table_actions: None, link_click_handler: None, + image_source: None, markdown_extensions: Arc::default(), motion: None, } } + /// Overrides the source of every document image, including embedded data URLs. + /// + /// Used for both rendering and intrinsic-size measurement. The returned source + /// is authoritative: pending or failed loads never fall back to the document URL. + /// Without this override, images use Base's default URI and data URL handling. + pub fn image_source(mut self, resolver: F) -> Self + where + F: Fn(&gpui::SharedUri) -> gpui::ImageSource + Send + Sync + 'static, + { + self.image_source = Some(Arc::new(resolver)); + self + } + /// Set [`TextViewStyle`]. pub fn style(mut self, style: TextViewStyle) -> Self { self.text_view_style = Some(style); @@ -579,6 +598,7 @@ impl Element for TextView { state.code_block_highlighter = code_block_highlighter; state.table_actions = self.table_actions.clone(); state.link_click_handler = self.link_click_handler.clone(); + state.image_source = self.image_source.clone(); state.set_markdown_extensions(self.markdown_extensions.clone(), cx); if let Some(motion) = &self.motion { state.set_motion(motion.clone()); diff --git a/crates/shell/Cargo.toml b/crates/shell/Cargo.toml index b6ca5c99d4..0cd9983977 100644 --- a/crates/shell/Cargo.toml +++ b/crates/shell/Cargo.toml @@ -58,11 +58,15 @@ tungstenite = { version = "0.29", features = ["rustls-tls-webpki-roots", "url"], anyhow.workspace = true instant.workspace = true +# Decode only bytes fetched through the document's network grant. Codec features +# are shared with GPUI, whose image loader previously decoded these documents. +image = { version = "0.25", default-features = false } schemars.workspace = true semver.workspace = true serde.workspace = true serde_json.workspace = true smallvec.workspace = true +smol.workspace = true tracing.workspace = true [dev-dependencies] diff --git a/crates/shell/README.md b/crates/shell/README.md index b60951d9ab..211c5c365d 100644 --- a/crates/shell/README.md +++ b/crates/shell/README.md @@ -382,6 +382,17 @@ limits. Every redirect target must be granted; HTTPS downgrade is refused, as are cross-origin POST replays and cross-origin redirects carrying Authorization or any caller-supplied header. +Images in `TextView.html` and `TextView.markdown` use the document's captured +network grant, including inline images and intrinsic-size measurement. Only +absolute HTTP(S) URLs authorized for GET can load; relative, scheme-less, +`data:`, `file:`, custom-scheme and credential-bearing URLs are refused. Each +redirect is re-authorized, with at most 10 redirects and no HTTPS downgrade. +Requests have a 30-second timeout and an 8 MiB response limit. Image loading +never falls back to the host's unrestricted URI loader. Each TextView and +policy identity has its own cache, released with its native element state; +a broader grant cannot populate a cache used by a narrower grant. The ordinary +application-asset `image(path)` API and default link handling are unchanged. + Import `WebSocket` from `websocket`; `WebSocket.connect(url, { headers })` resolves after the handshake and returns async `read`, `write`, and `close` methods for text and binary messages. Frames and messages are limited to 8 MiB. Connect/handshake and writes have 30-second diff --git a/crates/shell/src/engine/quickjs/mod.rs b/crates/shell/src/engine/quickjs/mod.rs index 8f1162ece4..666e036bde 100644 --- a/crates/shell/src/engine/quickjs/mod.rs +++ b/crates/shell/src/engine/quickjs/mod.rs @@ -7147,11 +7147,14 @@ impl ShellRuntime { "markdown" => crate::spec::TextViewFormat::Markdown, _ => return Err(Exception::throw_type(&ctx, "TextView format must be html or markdown")), }; - Ok(upgrade(&text_view_runtime, &ctx)?.push_node(Component::TextView { - id: id.into(), - text: text.into(), - format, - })) + Ok(upgrade(&text_view_runtime, &ctx)?.push_node(Component::TextView( + crate::spec::TextViewSpec { + id: id.into(), + text: text.into(), + format, + policy: crate::scope::policy(), + }, + ))) }), )?; text_constructor(&globals, "__svg", runtime.clone(), Component::Svg)?; diff --git a/crates/shell/src/materialize.rs b/crates/shell/src/materialize.rs index 442c104f1a..7f1624b8d8 100644 --- a/crates/shell/src/materialize.rs +++ b/crates/shell/src/materialize.rs @@ -136,6 +136,7 @@ use gpui_base::{ }; mod components; +mod text_view; use crate::{ capability::is_openable_url, @@ -1059,10 +1060,10 @@ fn materialize_component( cx, ) } - Component::TextView { id, text, format } => { - let mut view = match format { - crate::spec::TextViewFormat::Html => TextView::html(id, text), - crate::spec::TextViewFormat::Markdown => TextView::markdown(id, text), + Component::TextView(spec) => { + let mut view = match spec.format { + crate::spec::TextViewFormat::Html => TextView::html(spec.id, spec.text), + crate::spec::TextViewFormat::Markdown => TextView::markdown(spec.id, spec.text), } .style(TextViewStyle::from_theme(&Theme::global(cx))); if let Some(selectable) = behavior.selectable { @@ -1095,7 +1096,7 @@ fn materialize_component( }); } Styled::style(&mut view).refine(&refinement); - view.into_any_element() + text_view::PolicyTextView::new(view, spec.policy).into_any_element() } Component::Text(value) => { // A text run, not a `div` holding one. GPUI implements diff --git a/crates/shell/src/materialize/text_view.rs b/crates/shell/src/materialize/text_view.rs new file mode 100644 index 0000000000..e69583bf29 --- /dev/null +++ b/crates/shell/src/materialize/text_view.rs @@ -0,0 +1,390 @@ +//! Document images use the describing script's grant, not the host's ambient +//! image loader. Scope the cache across every phase: InlineFlow also creates +//! image elements during prepaint, and table sizing can measure them in layout. + +use std::{ + cell::RefCell, + collections::{HashMap, HashSet}, + io::Cursor, + rc::Rc, + sync::Arc, + time::Duration, +}; + +use gpui::{ + App, Context, Element, ElementId, Entity, EntityId, GlobalElementId, ImageCache, + ImageCacheError, InspectorElementId, IntoElement, LayoutId, RenderImage, Resource, + SharedString, SharedUri, SvgRenderer, Task, Window, http_client::HttpClient, +}; +use gpui_base::TextView; +use image::AnimationDecoder as _; +use smol::io::AsyncReadExt as _; + +use crate::{Capabilities, capability::is_openable_url, policy::Policy}; + +const MAX_IMAGE_BYTES: u64 = 8 * 1024 * 1024; +const MAX_REDIRECTS: usize = 10; +const IMAGE_TIMEOUT: Duration = Duration::from_secs(30); + +type ImageResult = Result, ImageCacheError>; + +/// A transparent wrapper: keep TextView's identity, layout and interactions. +/// No script callback can replace this cache or reach the default image loader. +pub(crate) struct PolicyTextView { + view: TextView, + policy: Rc, + cache_id: SharedString, +} + +impl PolicyTextView { + pub(super) fn new(view: TextView, policy: Rc) -> Self { + // A cache's authority never changes. Including the retained policy identity + // also separates old and replacement snapshots with the same element id. + let cache_id = format!( + "{}/shell-images/{:p}", + view.id().expect("TextView has an element id"), + Rc::as_ptr(&policy), + ) + .into(); + Self { + view: view.image_source(|uri| uri.clone().into()), + policy, + cache_id, + } + } +} + +impl IntoElement for PolicyTextView { + type Element = Self; + + fn into_element(self) -> Self { + self + } +} + +impl Element for PolicyTextView { + type RequestLayoutState = ( + ::RequestLayoutState, + Entity, + ); + type PrepaintState = ::PrepaintState; + + fn id(&self) -> Option { + self.view.id() + } + + fn source_location(&self) -> Option<&'static std::panic::Location<'static>> { + self.view.source_location() + } + + fn request_layout( + &mut self, + id: Option<&GlobalElementId>, + inspector_id: Option<&InspectorElementId>, + window: &mut Window, + cx: &mut App, + ) -> (LayoutId, Self::RequestLayoutState) { + let policy = self.policy.clone(); + let images = window.use_keyed_state(self.cache_id.clone(), cx, move |_, cx| { + DocumentImages::new(policy, cx) + }); + let (layout, state) = window.with_image_cache(Some(images.clone().into()), |window| { + self.view.request_layout(id, inspector_id, window, cx) + }); + (layout, (state, images)) + } + + fn prepaint( + &mut self, + id: Option<&GlobalElementId>, + inspector_id: Option<&InspectorElementId>, + bounds: gpui::Bounds, + state: &mut Self::RequestLayoutState, + window: &mut Window, + cx: &mut App, + ) -> Self::PrepaintState { + window.with_image_cache(Some(state.1.clone().into()), |window| { + self.view + .prepaint(id, inspector_id, bounds, &mut state.0, window, cx) + }) + } + + fn paint( + &mut self, + id: Option<&GlobalElementId>, + inspector_id: Option<&InspectorElementId>, + bounds: gpui::Bounds, + state: &mut Self::RequestLayoutState, + prepaint: &mut Self::PrepaintState, + window: &mut Window, + cx: &mut App, + ) { + window.with_image_cache(Some(state.1.clone().into()), |window| { + self.view + .paint(id, inspector_id, bounds, &mut state.0, prepaint, window, cx); + }); + } +} + +/// Each keyed TextView owns a separate cache. A URL loaded by a broader grant +/// must not satisfy the same URL under a different grant after a redirect. +pub(crate) struct DocumentImages { + policy: Rc, + entries: HashMap, +} + +impl DocumentImages { + fn new(policy: Rc, cx: &mut Context) -> Self { + cx.on_release(|images: &mut Self, cx| images.clear(cx)) + .detach(); + Self { + policy, + entries: HashMap::new(), + } + } + + fn clear(&mut self, cx: &mut App) { + for entry in self.entries.values() { + entry.drop_image(cx); + } + // Dropping entries cancels their pending tasks as well. + self.entries.clear(); + } +} + +impl ImageCache for DocumentImages { + fn load( + &mut self, + resource: &Resource, + window: &mut Window, + cx: &mut App, + ) -> Option { + let Resource::Uri(uri) = resource else { + return Some(Err(denied_image())); + }; + // Authorize before consulting any cache, including a failed entry. + let mut url = match image_url(self.policy.capabilities(), uri.as_ref()) { + Ok(url) => url, + Err(error) => return Some(Err(error)), + }; + url.set_fragment(None); + let key: SharedUri = url.as_str().to_owned().into(); + let entry = self + .entries + .entry(key) + .or_insert_with(|| ImageEntry::new(url, self.policy.capabilities().clone(), cx)); + let mut state = entry.state.borrow_mut(); + if let Some(result) = &state.result { + Some(result.clone()) + } else { + state.views.insert(window.current_view()); + None + } + } +} + +struct ImageEntry { + state: Rc>, + _task: Task<()>, +} + +#[derive(Default)] +struct ImageLoad { + result: Option, + views: HashSet, +} + +impl ImageEntry { + fn new(url: reqwest::Url, capabilities: Capabilities, cx: &mut App) -> Self { + let state = Rc::new(RefCell::new(ImageLoad::default())); + let client = cx.http_client(); + let renderer = cx.svg_renderer(); + let timeout = cx.background_executor().timer(IMAGE_TIMEOUT); + let task = cx.background_executor().spawn(async move { + smol::future::race( + async move { + let bytes = request_image(client, capabilities, url).await?; + decode_image(bytes, renderer) + }, + async move { + timeout.await; + Err(ImageCacheError::Asset( + "TextView image request timed out".into(), + )) + }, + ) + .await + }); + let weak = Rc::downgrade(&state); + let task = cx.spawn(async move |cx| { + let result = task.await; + let Some(state) = weak.upgrade() else { + return; + }; + let views = { + let mut state = state.borrow_mut(); + state.result = Some(result); + std::mem::take(&mut state.views) + }; + cx.update(|cx| { + for view in views { + cx.notify(view); + } + }); + }); + Self { state, _task: task } + } + + fn drop_image(&self, cx: &mut App) { + if let Some(Ok(image)) = &self.state.borrow().result { + cx.drop_image(image.clone(), None); + } + } +} + +fn denied_image() -> ImageCacheError { + ImageCacheError::Asset( + "TextView images require an absolute HTTP(S) URL and a capabilities.network GET grant" + .into(), + ) +} + +fn image_url(capabilities: &Capabilities, value: &str) -> Result { + if !is_openable_url(value) { + return Err(denied_image()); + } + let url = reqwest::Url::parse(value).map_err(|_| denied_image())?; + // Do not turn document-supplied userinfo into implicit HTTP credentials. + if !url.username().is_empty() || url.password().is_some() { + return Err(denied_image()); + } + if !capabilities.may_request( + url.scheme(), + url.host_str().unwrap_or_default(), + url.port(), + "GET", + url.path(), + ) { + return Err(denied_image()); + } + Ok(url) +} + +async fn request_image( + client: Arc, + capabilities: Capabilities, + mut url: reqwest::Url, +) -> Result, ImageCacheError> { + for redirects in 0..=MAX_REDIRECTS { + url = image_url(&capabilities, url.as_str())?; + url.set_fragment(None); + // The host transport must never follow a redirect on our behalf. + let mut response = client.get(url.as_str(), ().into(), false).await?; + if matches!(response.status().as_u16(), 301 | 302 | 303 | 307 | 308) { + if redirects == MAX_REDIRECTS { + return Err(ImageCacheError::Asset( + "Too many TextView image redirects".into(), + )); + } + let next = response + .headers() + .get("location") + .and_then(|location| location.to_str().ok()) + .and_then(|location| url.join(location).ok()) + .ok_or_else(|| ImageCacheError::Asset("Invalid TextView image redirect".into()))?; + if url.scheme() == "https" && next.scheme() == "http" { + return Err(ImageCacheError::Asset( + "TextView image HTTPS downgrade refused".into(), + )); + } + // The next iteration re-authorizes scheme, host, port, GET and path. + url = next; + continue; + } + if !response.status().is_success() { + return Err(ImageCacheError::Asset( + format!("TextView image request returned {}", response.status()).into(), + )); + } + let mut bytes = Vec::new(); + response + .body_mut() + .take(MAX_IMAGE_BYTES + 1) + .read_to_end(&mut bytes) + .await?; + if bytes.len() as u64 > MAX_IMAGE_BYTES { + return Err(ImageCacheError::Asset( + "TextView image exceeds the 8 MiB limit".into(), + )); + } + return Ok(bytes); + } + unreachable!("the final redirect is rejected above") +} + +/// Match GPUI's resource decoding without handing it the document URL again. +/// Otherwise decoding would silently start a second, ungated HTTP request. +fn decode_image(bytes: Vec, renderer: SvgRenderer) -> ImageResult { + let Ok(format) = image::guess_format(&bytes) else { + return renderer + .render_single_frame(&bytes, 1.0) + .map_err(Into::into); + }; + let frames = match format { + image::ImageFormat::Gif => animation_frames( + image::codecs::gif::GifDecoder::new(Cursor::new(&bytes))?.into_frames(), + )?, + image::ImageFormat::WebP => { + let mut decoder = image::codecs::webp::WebPDecoder::new(Cursor::new(&bytes))?; + if decoder.has_animation() { + let _ = decoder.set_background_color(image::Rgba([0, 0, 0, 0])); + animation_frames(decoder.into_frames())? + } else { + static_frame(decoder)? + } + } + _ => static_frame( + image::ImageReader::with_format(Cursor::new(&bytes), format).into_decoder()?, + )?, + }; + Ok(Arc::new(RenderImage::new(frames))) +} + +fn static_frame( + mut decoder: impl image::ImageDecoder, +) -> Result, ImageCacheError> { + let orientation = decoder.orientation()?; + let mut image = image::DynamicImage::from_decoder(decoder)?; + image.apply_orientation(orientation); + let mut data = image.into_rgba8(); + for pixel in data.chunks_exact_mut(4) { + pixel.swap(0, 2); + } + Ok(smallvec::smallvec![image::Frame::new(data)]) +} + +fn animation_frames( + frames: image::Frames<'_>, +) -> Result, ImageCacheError> { + let mut decoded = smallvec::SmallVec::new(); + for frame in frames { + match frame { + Ok(mut frame) => { + for pixel in frame.buffer_mut().chunks_exact_mut(4) { + pixel.swap(0, 2); + } + decoded.push(frame); + } + Err(error) => tracing::debug!(%error, "Skipping an invalid TextView image frame"), + } + } + if decoded.is_empty() { + return Err(ImageCacheError::Asset( + "TextView image has no decodable frames".into(), + )); + } + Ok(decoded) +} + +#[cfg(test)] +mod tests; diff --git a/crates/shell/src/materialize/text_view/tests.rs b/crates/shell/src/materialize/text_view/tests.rs new file mode 100644 index 0000000000..9c79e99bc1 --- /dev/null +++ b/crates/shell/src/materialize/text_view/tests.rs @@ -0,0 +1,376 @@ +use super::*; +use crate::{HttpRequestGrant, ScriptView, ShellRuntime}; +use gpui::{ + ParentElement as _, Render, Styled as _, TestAppContext, VisualTestContext, div, + http_client::{AsyncBody, FakeHttpClient, Method, RedirectPolicy, Response}, +}; +use std::{ops::Deref as _, sync::Mutex}; + +// A two-pixel PNG generated in memory, not an external fixture or URL. +const PNG: &[u8] = &[ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01, 0x08, 0x06, 0x00, 0x00, 0x00, 0xf4, 0x22, 0x7f, + 0x8a, 0x00, 0x00, 0x00, 0x11, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x63, 0x14, 0x32, 0x09, 0xfb, + 0xcf, 0xc0, 0xc0, 0xc0, 0x00, 0x00, 0x09, 0x0d, 0x01, 0x9d, 0xf7, 0x66, 0xcd, 0x15, 0x00, 0x00, + 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82, +]; + +type Requests = Arc>>; + +fn get_grant(paths: &[&str]) -> Capabilities { + Capabilities::new().http_requests([HttpRequestGrant::new( + "images.example", + ["GET"], + paths.iter().copied(), + [] as [&str; 0], + )]) +} + +fn recording_client( + reply: impl Fn(&str) -> Response + Send + Sync + 'static, +) -> (Arc, Requests) { + let requests = Requests::default(); + let recorded = requests.clone(); + let client = FakeHttpClient::create(move |request| { + assert_eq!(request.method(), Method::GET); + assert_eq!( + request.extensions().get::(), + Some(&RedirectPolicy::NoFollow), + "the transport must not follow redirects before authorization", + ); + let url = request.uri().to_string(); + recorded.lock().unwrap().push(url.clone()); + let response = reply(&url); + async move { Ok(response) } + }); + (client, requests) +} + +fn png_response() -> Response { + Response::builder() + .status(200) + .header("content-type", "image/png") + .body(PNG.to_vec().into()) + .unwrap() +} + +fn fetch( + client: Arc, + capabilities: Capabilities, +) -> Result, ImageCacheError> { + smol::block_on(request_image( + client, + capabilities, + reqwest::Url::parse("https://images.example/image.png").unwrap(), + )) +} + +fn redirect(status: u16, target: &str) -> Response { + Response::builder() + .status(status) + .header("location", target) + .body(().into()) + .unwrap() +} + +#[test] +fn document_image_urls_obey_get_grants() { + let exact = get_grant(&["/image.png"]); + for (url, allowed) in [ + ("https://images.example/image.png", true), + ( + "https://images.example:443/image.png?size=small#preview", + true, + ), + ("https://images.example/other.png", false), + ("https://other.example/image.png", false), + ("https://images.example:8443/image.png", false), + ("http://images.example/image.png", false), + ("data:image/png;base64,AAAA", false), + ("file:///image.png", false), + ("custom:image.png", false), + ("//images.example/image.png", false), + ("/image.png", false), + ("image.png", false), + ("https://", false), + ("https://user:password@images.example/image.png", false), + ] { + assert_eq!(image_url(&exact, url).is_ok(), allowed, "{url}"); + assert!(image_url(&Capabilities::new(), url).is_err(), "{url}"); + if let Ok(parsed) = reqwest::Url::parse(url) { + let (client, requests) = recording_client(|_| png_response()); + assert_eq!( + smol::block_on(request_image(client, exact.clone(), parsed)).is_ok(), + allowed, + "{url}", + ); + assert_eq!( + requests.lock().unwrap().len(), + usize::from(allowed), + "{url}" + ); + } + } + let post_only = Capabilities::new().http_requests([HttpRequestGrant::new( + "images.example", + ["POST"], + ["/image.png"], + [] as [&str; 0], + )]); + assert!(image_url(&post_only, "https://images.example/image.png").is_err()); + let (client, requests) = recording_client(|_| png_response()); + assert!(fetch(client, post_only).is_err()); + assert!(requests.lock().unwrap().is_empty()); +} + +#[test] +fn document_image_redirects_reauthorize_each_hop() { + for target in [ + "https://other.example/image.png", + "https://images.example/not-granted.png", + "https://images.example:8443/image.png", + "file:///image.png", + "data:image/png;base64,AAAA", + "custom:image.png", + "https://user:password@images.example/image.png", + ] { + let (client, requests) = recording_client(move |_| redirect(302, target)); + let result = fetch(client, get_grant(&["/image.png"])); + assert!(result.is_err(), "{target}"); + assert_eq!(requests.lock().unwrap().len(), 1, "{target}"); + } + // Even a legacy grant to both protocols must not allow HTTPS downgrade. + let (client, requests) = recording_client(|_| redirect(302, "http://images.example/image.png")); + let result = fetch( + client, + Capabilities::new().network_hosts(["images.example".to_owned()]), + ); + assert!(result.is_err()); + assert_eq!(requests.lock().unwrap().len(), 1); +} + +#[gpui::test] +fn document_image_redirects_preserve_authorized_get(cx: &mut TestAppContext) { + for status in [301, 302, 303, 307, 308] { + let (client, requests) = recording_client(move |url| { + if url.ends_with("/image.png") { + redirect(status, "/final.png") + } else { + png_response() + } + }); + let bytes = fetch(client, get_grant(&["/image.png", "/final.png"])).unwrap(); + assert_eq!(bytes, PNG); + let image = cx + .update(|cx| decode_image(bytes, cx.svg_renderer())) + .unwrap(); + assert_eq!(image.size(0), gpui::size(2.into(), 1.into())); + assert_eq!(&image.as_bytes(0).unwrap()[..4], &[0x56, 0x34, 0x12, 0xff]); + assert_eq!( + *requests.lock().unwrap(), + [ + "https://images.example/image.png", + "https://images.example/final.png", + ], + ); + } +} + +#[test] +fn document_image_requests_bound_bodies_and_redirects() { + for (status, body, allowed) in [ + (200, vec![0; MAX_IMAGE_BYTES as usize], true), + (200, vec![0; MAX_IMAGE_BYTES as usize + 1], false), + (404, Vec::new(), false), + (302, Vec::new(), false), // Missing Location must not trigger a fallback. + ] { + let (client, requests) = recording_client(move |_| { + Response::builder() + .status(status) + .body(body.clone().into()) + .unwrap() + }); + let result = fetch(client, get_grant(&["/image.png"])); + assert_eq!(result.is_ok(), allowed); + assert_eq!(requests.lock().unwrap().len(), 1); + } + let (client, requests) = recording_client(|_| redirect(302, "/image.png")); + assert!(fetch(client, get_grant(&["/image.png"])).is_err()); + assert_eq!(requests.lock().unwrap().len(), MAX_REDIRECTS + 1); +} + +struct Documents(Vec>); + +impl Render for Documents { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + div().size_full().children(self.0.iter().cloned()) + } +} + +fn source(format: &str, document: &str, scrollable: bool) -> String { + let document = serde_json::to_string(document).unwrap(); + format!( + r#" +import {{ View }} from "gpui-kit"; +import {{ TextView }} from "gpui-base"; +export default class Document extends View {{ + render() {{ return TextView.{format}("document", {document}).scrollable({scrollable}); }} +}} +"#, + ) +} + +fn mount_documents( + cx: &mut TestAppContext, + source: &str, + capabilities: Vec, +) -> VisualTestContext { + cx.update(crate::init); + let runtime = ShellRuntime::new_isolated().expect("runtime"); + cx.update(|cx| runtime.set_global(cx)); + let view_type = runtime + .load_source("document-images.js", source) + .expect("source"); + let window = cx.add_window(move |window, cx| { + let views = capabilities + .into_iter() + .enumerate() + .map(|(ix, capabilities)| { + let policy = Rc::new( + Policy::new() + .with_application(format!("document-{ix}")) + .with_capabilities(capabilities), + ); + runtime + .instantiate_view_with_policy(&view_type, policy, window, cx) + .expect("view") + }) + .collect(); + Documents(views) + }); + VisualTestContext::from_window(*window.deref(), cx) +} + +fn draw_documents(cx: &mut VisualTestContext) { + for _ in 0..4 { + cx.update(|window, cx| window.draw(cx).clear(cx)); + cx.run_until_parked(); + } +} + +#[gpui::test] +fn text_view_document_images_require_policy(cx: &mut TestAppContext) { + for (format, document) in [ + ("markdown", "![image](https://images.example/image.png)"), + ( + "markdown", + "Before ![image](https://images.example/image.png) after", + ), + ( + "markdown", + "| Image |\n| --- |\n| ![image](https://images.example/image.png) |", + ), + ("html", r#""#), + ( + "html", + r#"

Before after

"#, + ), + ( + "html", + r#"
"#, + ), + ] { + for scrollable in [false, true] { + for allowed in [false, true] { + let mut app = cx.new_app(); + let (client, requests) = recording_client(|_| png_response()); + app.update(|cx| cx.set_http_client(client)); + let capabilities = if allowed { + get_grant(&["/image.png"]) + } else { + Capabilities::new() + }; + let mut context = mount_documents( + &mut app, + &source(format, document, scrollable), + vec![capabilities], + ); + draw_documents(&mut context); + assert_eq!( + requests.lock().unwrap().len(), + usize::from(allowed), + "{format}, scrollable={scrollable}, allowed={allowed}: {document}", + ); + // Repainting must use this document's cache, not refetch or fall back. + draw_documents(&mut context); + assert_eq!(requests.lock().unwrap().len(), usize::from(allowed)); + app.quit(); + } + } + } +} + +#[gpui::test] +fn text_view_data_images_cannot_bypass_policy(cx: &mut TestAppContext) { + // This valid image would otherwise be decoded directly by Base, bypassing + // ImageCache::load entirely. Check the asset cache as well as the transport. + let data = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAIAAAABCAYAAAD0In+KAAAAEUlEQVR4nGMUMgn7z8DAwAAACQ0BnfdmzRUAAAAASUVORK5CYII="; + for (format, document) in [ + ("markdown", format!("Before ![image]({data}) after")), + ("html", format!(r#""#)), + ] { + let mut app = cx.new_app(); + let (client, requests) = recording_client(|_| png_response()); + app.update(|cx| cx.set_http_client(client)); + let mut context = mount_documents( + &mut app, + &source(format, &document, false), + vec![Capabilities::new()], + ); + draw_documents(&mut context); + assert!(requests.lock().unwrap().is_empty()); + let image = Arc::new(gpui::Image::from_bytes( + gpui::ImageFormat::Png, + PNG.to_vec(), + )); + assert!(!context.update(|_, cx| gpui::ImageSource::Image(image).is_asset_cached(cx))); + app.quit(); + } +} + +#[gpui::test] +fn text_view_document_images_keep_policies_separate(cx: &mut TestAppContext) { + let (client, requests) = recording_client(|url| { + if url.ends_with("/image.png") { + redirect(302, "/final.png") + } else { + png_response() + } + }); + cx.update(|cx| cx.set_http_client(client)); + let mut context = mount_documents( + cx, + &source( + "markdown", + "![image](https://images.example/image.png)", + false, + ), + vec![ + get_grant(&["/image.png", "/final.png"]), + get_grant(&["/image.png"]), + Capabilities::new(), + ], + ); + draw_documents(&mut context); + let mut actual = requests.lock().unwrap().clone(); + actual.sort(); + assert_eq!( + actual, + [ + "https://images.example/final.png", + "https://images.example/image.png", + "https://images.example/image.png", + ], + "only the first document may follow the redirect; none may borrow another cache", + ); +} diff --git a/crates/shell/src/spec.rs b/crates/shell/src/spec.rs index eb2ddd63c2..63909405df 100644 --- a/crates/shell/src/spec.rs +++ b/crates/shell/src/spec.rs @@ -198,6 +198,36 @@ pub(crate) enum TextViewFormat { Markdown, } +/// A document keeps the authority of the script that described it. Image +/// loads happen in later native rendering phases, after the script scope ends. +#[derive(Clone)] +pub(crate) struct TextViewSpec { + pub(crate) id: SharedString, + pub(crate) text: SharedString, + pub(crate) format: TextViewFormat, + pub(crate) policy: Rc, +} + +impl std::fmt::Debug for TextViewSpec { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("TextViewSpec") + .field("id", &self.id) + .field("text", &self.text) + .field("format", &self.format) + .finish_non_exhaustive() + } +} + +impl PartialEq for TextViewSpec { + fn eq(&self, other: &Self) -> bool { + self.id == other.id + && self.text == other.text + && self.format == other.format + && Rc::ptr_eq(&self.policy, &other.policy) + } +} + /// Which constructor produced a node. #[derive(Clone, Debug, PartialEq)] pub(crate) enum Component { @@ -205,11 +235,7 @@ pub(crate) enum Component { HFlex, VFlex, Module(ModuleComponentSpec), - TextView { - id: SharedString, - text: SharedString, - format: TextViewFormat, - }, + TextView(TextViewSpec), /// A retained nested script view. The frozen description keeps the entity /// itself alive, so releasing the numeric handle cannot invalidate a frame /// that was already published. @@ -647,7 +673,7 @@ impl Component { Component::HFlex => "h_flex", Component::VFlex => "v_flex", Component::Module(_) => "module_component", - Component::TextView { .. } => "TextView", + Component::TextView(_) => "TextView", Component::ChildView(_) => "child_view", Component::Text(_) => "text", Component::Registered(component) => component.name(), @@ -1292,14 +1318,14 @@ impl SpecArena { " {}.{} {:?} props={:?}", spec.module, spec.component, spec.id, spec.props )), - Component::TextView { id, text, format } => out.push_str(&format!( + Component::TextView(spec) => out.push_str(&format!( " {} {:?} {:?}", - match format { + match spec.format { TextViewFormat::Html => "html", TextViewFormat::Markdown => "markdown", }, - id, - text, + spec.id, + spec.text, )), Component::Text(value) | Component::Button(value) From 55dfe3649943ca71c759a991647501108f886f6f Mon Sep 17 00:00:00 2001 From: luren Date: Fri, 25 Sep 2026 17:23:56 +0800 Subject: [PATCH 2/4] shell: Reuse GPUI assets for policy-bound document images --- crates/shell/src/materialize.rs | 2 +- crates/shell/src/materialize/text_view.rs | 251 ++++-------------- .../shell/src/materialize/text_view/tests.rs | 3 +- 3 files changed, 59 insertions(+), 197 deletions(-) diff --git a/crates/shell/src/materialize.rs b/crates/shell/src/materialize.rs index 7f1624b8d8..df46801458 100644 --- a/crates/shell/src/materialize.rs +++ b/crates/shell/src/materialize.rs @@ -1096,7 +1096,7 @@ fn materialize_component( }); } Styled::style(&mut view).refine(&refinement); - text_view::PolicyTextView::new(view, spec.policy).into_any_element() + text_view::with_policy(view, spec.policy, window, cx).into_any_element() } Component::Text(value) => { // A text run, not a `div` holding one. GPUI implements diff --git a/crates/shell/src/materialize/text_view.rs b/crates/shell/src/materialize/text_view.rs index e69583bf29..f31b0a975d 100644 --- a/crates/shell/src/materialize/text_view.rs +++ b/crates/shell/src/materialize/text_view.rs @@ -1,20 +1,11 @@ -//! Document images use the describing script's grant, not the host's ambient -//! image loader. Scope the cache across every phase: InlineFlow also creates -//! image elements during prepaint, and table sizing can measure them in layout. +//! Fetch document images with the creating script's policy. GPUI owns asynchronous +//! loading, caching and completion notifications; the document owns their lifetime. -use std::{ - cell::RefCell, - collections::{HashMap, HashSet}, - io::Cursor, - rc::Rc, - sync::Arc, - time::Duration, -}; +use std::{collections::HashSet, io::Cursor, rc::Rc, sync::Arc, time::Duration}; use gpui::{ - App, Context, Element, ElementId, Entity, EntityId, GlobalElementId, ImageCache, - ImageCacheError, InspectorElementId, IntoElement, LayoutId, RenderImage, Resource, - SharedString, SharedUri, SvgRenderer, Task, Window, http_client::HttpClient, + App, Asset, Element, ImageCacheError, ImageSource, RenderImage, SharedString, SharedUri, + SvgRenderer, WeakEntity, Window, http_client::HttpClient, }; use gpui_base::TextView; use image::AnimationDecoder as _; @@ -28,179 +19,73 @@ const IMAGE_TIMEOUT: Duration = Duration::from_secs(30); type ImageResult = Result, ImageCacheError>; -/// A transparent wrapper: keep TextView's identity, layout and interactions. -/// No script callback can replace this cache or reach the default image loader. -pub(crate) struct PolicyTextView { +pub(super) fn with_policy( view: TextView, policy: Rc, - cache_id: SharedString, -} - -impl PolicyTextView { - pub(super) fn new(view: TextView, policy: Rc) -> Self { - // A cache's authority never changes. Including the retained policy identity - // also separates old and replacement snapshots with the same element id. - let cache_id = format!( - "{}/shell-images/{:p}", - view.id().expect("TextView has an element id"), - Rc::as_ptr(&policy), - ) - .into(); - Self { - view: view.image_source(|uri| uri.clone().into()), + window: &mut Window, + cx: &mut App, +) -> TextView { + let key = SharedString::from(format!( + "{}/shell-images/{:p}", + view.id().expect("TextView has an element id"), + Rc::as_ptr(&policy), + )); + let images = window.use_keyed_state(key, cx, |_, cx| { + let owner = cx.weak_entity(); + cx.on_release(move |images: &mut DocumentImages, cx| { + for uri in &images.used { + let source = (owner.clone(), uri.clone()); + if let Some(Ok(image)) = cx.fetch_asset::(&source) { + cx.drop_image(image, None); + } + cx.remove_asset::(&source); + } + }) + .detach(); + DocumentImages { policy, - cache_id, + used: HashSet::new(), } - } -} - -impl IntoElement for PolicyTextView { - type Element = Self; - - fn into_element(self) -> Self { - self - } -} - -impl Element for PolicyTextView { - type RequestLayoutState = ( - ::RequestLayoutState, - Entity, - ); - type PrepaintState = ::PrepaintState; - - fn id(&self) -> Option { - self.view.id() - } - - fn source_location(&self) -> Option<&'static std::panic::Location<'static>> { - self.view.source_location() - } - - fn request_layout( - &mut self, - id: Option<&GlobalElementId>, - inspector_id: Option<&InspectorElementId>, - window: &mut Window, - cx: &mut App, - ) -> (LayoutId, Self::RequestLayoutState) { - let policy = self.policy.clone(); - let images = window.use_keyed_state(self.cache_id.clone(), cx, move |_, cx| { - DocumentImages::new(policy, cx) - }); - let (layout, state) = window.with_image_cache(Some(images.clone().into()), |window| { - self.view.request_layout(id, inspector_id, window, cx) - }); - (layout, (state, images)) - } - - fn prepaint( - &mut self, - id: Option<&GlobalElementId>, - inspector_id: Option<&InspectorElementId>, - bounds: gpui::Bounds, - state: &mut Self::RequestLayoutState, - window: &mut Window, - cx: &mut App, - ) -> Self::PrepaintState { - window.with_image_cache(Some(state.1.clone().into()), |window| { - self.view - .prepaint(id, inspector_id, bounds, &mut state.0, window, cx) - }) - } - - fn paint( - &mut self, - id: Option<&GlobalElementId>, - inspector_id: Option<&InspectorElementId>, - bounds: gpui::Bounds, - state: &mut Self::RequestLayoutState, - prepaint: &mut Self::PrepaintState, - window: &mut Window, - cx: &mut App, - ) { - window.with_image_cache(Some(state.1.clone().into()), |window| { - self.view - .paint(id, inspector_id, bounds, &mut state.0, prepaint, window, cx); - }); - } + }); + view.image_source(move |uri| { + let images = images.clone(); + let uri = uri.clone(); + ImageSource::Custom(Arc::new(move |window, cx| { + images.update(cx, |images, _| { + images.used.insert(uri.clone()); + }); + window.use_asset::(&(images.downgrade(), uri.clone()), cx) + })) + }) } -/// Each keyed TextView owns a separate cache. A URL loaded by a broader grant -/// must not satisfy the same URL under a different grant after a redirect. -pub(crate) struct DocumentImages { +struct DocumentImages { + // Retaining the immutable policy also keeps its identity from being reused. policy: Rc, - entries: HashMap, + used: HashSet, } -impl DocumentImages { - fn new(policy: Rc, cx: &mut Context) -> Self { - cx.on_release(|images: &mut Self, cx| images.clear(cx)) - .detach(); - Self { - policy, - entries: HashMap::new(), - } - } +struct DocumentImage; - fn clear(&mut self, cx: &mut App) { - for entry in self.entries.values() { - entry.drop_image(cx); - } - // Dropping entries cancels their pending tasks as well. - self.entries.clear(); - } -} +impl Asset for DocumentImage { + // The owner scopes both successful and failed loads to this view and policy. + // A weak reference lets releasing the view cancel pending loads. + type Source = (WeakEntity, SharedUri); + type Output = ImageResult; -impl ImageCache for DocumentImages { fn load( - &mut self, - resource: &Resource, - window: &mut Window, + (owner, uri): Self::Source, cx: &mut App, - ) -> Option { - let Resource::Uri(uri) = resource else { - return Some(Err(denied_image())); - }; - // Authorize before consulting any cache, including a failed entry. - let mut url = match image_url(self.policy.capabilities(), uri.as_ref()) { - Ok(url) => url, - Err(error) => return Some(Err(error)), - }; - url.set_fragment(None); - let key: SharedUri = url.as_str().to_owned().into(); - let entry = self - .entries - .entry(key) - .or_insert_with(|| ImageEntry::new(url, self.policy.capabilities().clone(), cx)); - let mut state = entry.state.borrow_mut(); - if let Some(result) = &state.result { - Some(result.clone()) - } else { - state.views.insert(window.current_view()); - None - } - } -} - -struct ImageEntry { - state: Rc>, - _task: Task<()>, -} - -#[derive(Default)] -struct ImageLoad { - result: Option, - views: HashSet, -} - -impl ImageEntry { - fn new(url: reqwest::Url, capabilities: Capabilities, cx: &mut App) -> Self { - let state = Rc::new(RefCell::new(ImageLoad::default())); + ) -> impl Future + Send + 'static { + let capabilities = owner + .upgrade() + .map(|images| images.read(cx).policy.capabilities().clone()); let client = cx.http_client(); let renderer = cx.svg_renderer(); let timeout = cx.background_executor().timer(IMAGE_TIMEOUT); - let task = cx.background_executor().spawn(async move { + async move { + let capabilities = capabilities.ok_or_else(denied_image)?; + let url = image_url(&capabilities, uri.as_ref())?; smol::future::race( async move { let bytes = request_image(client, capabilities, url).await?; @@ -214,30 +99,6 @@ impl ImageEntry { }, ) .await - }); - let weak = Rc::downgrade(&state); - let task = cx.spawn(async move |cx| { - let result = task.await; - let Some(state) = weak.upgrade() else { - return; - }; - let views = { - let mut state = state.borrow_mut(); - state.result = Some(result); - std::mem::take(&mut state.views) - }; - cx.update(|cx| { - for view in views { - cx.notify(view); - } - }); - }); - Self { state, _task: task } - } - - fn drop_image(&self, cx: &mut App) { - if let Some(Ok(image)) = &self.state.borrow().result { - cx.drop_image(image.clone(), None); } } } diff --git a/crates/shell/src/materialize/text_view/tests.rs b/crates/shell/src/materialize/text_view/tests.rs index 9c79e99bc1..42d42f1bf8 100644 --- a/crates/shell/src/materialize/text_view/tests.rs +++ b/crates/shell/src/materialize/text_view/tests.rs @@ -1,7 +1,8 @@ use super::*; use crate::{HttpRequestGrant, ScriptView, ShellRuntime}; use gpui::{ - ParentElement as _, Render, Styled as _, TestAppContext, VisualTestContext, div, + Context, Entity, IntoElement, ParentElement as _, Render, Styled as _, TestAppContext, + VisualTestContext, div, http_client::{AsyncBody, FakeHttpClient, Method, RedirectPolicy, Response}, }; use std::{ops::Deref as _, sync::Mutex}; From e5ac52fca2f7384f263b5bf8ac91d9b6e0d6a894 Mon Sep 17 00:00:00 2001 From: luren Date: Fri, 25 Sep 2026 17:43:26 +0800 Subject: [PATCH 3/4] shell: Initialize TextView image state during layout --- crates/shell/src/materialize.rs | 2 +- crates/shell/src/materialize/text_view.rs | 75 ++++++++++++++++++++++- crates/shell/src/plugin.rs | 30 +++++++++ 3 files changed, 104 insertions(+), 3 deletions(-) diff --git a/crates/shell/src/materialize.rs b/crates/shell/src/materialize.rs index df46801458..91452c0889 100644 --- a/crates/shell/src/materialize.rs +++ b/crates/shell/src/materialize.rs @@ -1096,7 +1096,7 @@ fn materialize_component( }); } Styled::style(&mut view).refine(&refinement); - text_view::with_policy(view, spec.policy, window, cx).into_any_element() + text_view::with_policy(view, spec.policy).into_any_element() } Component::Text(value) => { // A text run, not a `div` holding one. GPUI implements diff --git a/crates/shell/src/materialize/text_view.rs b/crates/shell/src/materialize/text_view.rs index f31b0a975d..b6291700b8 100644 --- a/crates/shell/src/materialize/text_view.rs +++ b/crates/shell/src/materialize/text_view.rs @@ -4,7 +4,8 @@ use std::{collections::HashSet, io::Cursor, rc::Rc, sync::Arc, time::Duration}; use gpui::{ - App, Asset, Element, ImageCacheError, ImageSource, RenderImage, SharedString, SharedUri, + App, Asset, Bounds, Element, ElementId, GlobalElementId, ImageCacheError, ImageSource, + InspectorElementId, IntoElement, LayoutId, Pixels, RenderImage, SharedString, SharedUri, SvgRenderer, WeakEntity, Window, http_client::HttpClient, }; use gpui_base::TextView; @@ -19,7 +20,77 @@ const IMAGE_TIMEOUT: Duration = Duration::from_secs(30); type ImageResult = Result, ImageCacheError>; -pub(super) fn with_policy( +pub(super) fn with_policy(view: TextView, policy: Rc) -> impl IntoElement { + PolicyTextView { view, policy } +} + +/// CLI check constructs elements without drawing. Only layout may initialize +/// the keyed image owner; GPUI's Asset still owns all loading and notifications. +struct PolicyTextView { + view: TextView, + policy: Rc, +} + +impl IntoElement for PolicyTextView { + type Element = Self; + + fn into_element(self) -> Self { + self + } +} + +impl Element for PolicyTextView { + type RequestLayoutState = ::RequestLayoutState; + type PrepaintState = ::PrepaintState; + + fn id(&self) -> Option { + self.view.id() + } + + fn source_location(&self) -> Option<&'static std::panic::Location<'static>> { + self.view.source_location() + } + + fn request_layout( + &mut self, + id: Option<&GlobalElementId>, + inspector_id: Option<&InspectorElementId>, + window: &mut Window, + cx: &mut App, + ) -> (LayoutId, Self::RequestLayoutState) { + self.view = image_sources(self.view.clone(), self.policy.clone(), window, cx); + self.view.request_layout(id, inspector_id, window, cx) + } + + fn prepaint( + &mut self, + id: Option<&GlobalElementId>, + inspector_id: Option<&InspectorElementId>, + bounds: Bounds, + state: &mut Self::RequestLayoutState, + window: &mut Window, + cx: &mut App, + ) -> Self::PrepaintState { + self.view + .prepaint(id, inspector_id, bounds, state, window, cx) + } + + fn paint( + &mut self, + id: Option<&GlobalElementId>, + inspector_id: Option<&InspectorElementId>, + bounds: Bounds, + state: &mut Self::RequestLayoutState, + prepaint: &mut Self::PrepaintState, + window: &mut Window, + cx: &mut App, + ) { + self.view + .paint(id, inspector_id, bounds, state, prepaint, window, cx); + } +} + +fn image_sources( view: TextView, policy: Rc, window: &mut Window, diff --git a/crates/shell/src/plugin.rs b/crates/shell/src/plugin.rs index 50f4861159..d7912434c1 100644 --- a/crates/shell/src/plugin.rs +++ b/crates/shell/src/plugin.rs @@ -1823,6 +1823,36 @@ mod tests { assert_eq!(metrics.materializations(), 1); } + #[gpui::test] + fn runtime_check_materializes_text_views_without_drawing(cx: &mut TestAppContext) { + cx.update(crate::init); + let application = TempTree::new("check-text-views"); + std::fs::write( + application.path().join("main.js"), + r##" + import { div, View } from "gpui-kit"; + import { TextView } from "gpui-base"; + export default class App extends View { + render() { + return div() + .child(TextView.markdown("markdown", "# Markdown")) + .child(TextView.html("html", "

HTML

")); + } + } + "##, + ) + .expect("application source"); + let runtime = ShellRuntime::new_isolated().expect("runtime"); + let window = cx.add_window(|_, _| gpui::Empty); + let mut context = VisualTestContext::from_window(*window.deref(), cx); + // CLI check only constructs elements; no layout or drawing is active. + let description = context + .update(|window, cx| runtime.check(application.path(), window, cx)) + .expect("check TextViews outside the drawing lifecycle"); + assert!(description.contains("# Markdown")); + assert!(description.contains("

HTML

")); + } + #[gpui::test] fn runtime_load_uses_the_manifest_entry_without_plugin_manager_ceremony( cx: &mut TestAppContext, From fe5ad9a688cc0d369c5b89e9535f18d01710b054 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Fri, 25 Sep 2026 23:13:18 +0800 Subject: [PATCH 4/4] shell: Refuse TextView SVG images that reference local files GPUI renders an SVG whose href is not a data URL from the local file it names, so an SVG fetched under a GET grant could draw files the script has no grant to read. Parse the SVG with a resolver that records such an href before rendering, and refuse the image when it has one. Co-Authored-By: Claude Opus 5.5 (1M context) --- Cargo.lock | 1 + crates/shell/Cargo.toml | 2 + crates/shell/README.md | 3 +- crates/shell/src/materialize/text_view.rs | 41 ++++++++++++++++++- .../shell/src/materialize/text_view/tests.rs | 18 ++++++++ 5 files changed, 63 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0ac5eb185f..a3be1f0621 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4540,6 +4540,7 @@ dependencies = [ "smol", "tracing", "tungstenite", + "usvg 0.46.0", "wait-timeout", "windows 0.58.0", ] diff --git a/crates/shell/Cargo.toml b/crates/shell/Cargo.toml index 0cd9983977..32c9370090 100644 --- a/crates/shell/Cargo.toml +++ b/crates/shell/Cargo.toml @@ -67,6 +67,8 @@ serde.workspace = true serde_json.workspace = true smallvec.workspace = true smol.workspace = true +# Only to find the files an SVG image references; GPUI's version, which renders it. +usvg = { version = "0.46", default-features = false } tracing.workspace = true [dev-dependencies] diff --git a/crates/shell/README.md b/crates/shell/README.md index 211c5c365d..a48c48eb7d 100644 --- a/crates/shell/README.md +++ b/crates/shell/README.md @@ -385,7 +385,8 @@ or any caller-supplied header. Images in `TextView.html` and `TextView.markdown` use the document's captured network grant, including inline images and intrinsic-size measurement. Only absolute HTTP(S) URLs authorized for GET can load; relative, scheme-less, -`data:`, `file:`, custom-scheme and credential-bearing URLs are refused. Each +`data:`, `file:`, custom-scheme and credential-bearing URLs are refused, as +is an SVG image whose `` references a file. Each redirect is re-authorized, with at most 10 redirects and no HTTPS downgrade. Requests have a 30-second timeout and an 8 MiB response limit. Image loading never falls back to the host's unrestricted URI loader. Each TextView and diff --git a/crates/shell/src/materialize/text_view.rs b/crates/shell/src/materialize/text_view.rs index b6291700b8..a4ee06a63c 100644 --- a/crates/shell/src/materialize/text_view.rs +++ b/crates/shell/src/materialize/text_view.rs @@ -1,7 +1,16 @@ //! Fetch document images with the creating script's policy. GPUI owns asynchronous //! loading, caching and completion notifications; the document owns their lifetime. -use std::{collections::HashSet, io::Cursor, rc::Rc, sync::Arc, time::Duration}; +use std::{ + collections::HashSet, + io::Cursor, + rc::Rc, + sync::{ + Arc, + atomic::{AtomicBool, Ordering}, + }, + time::Duration, +}; use gpui::{ App, Asset, Bounds, Element, ElementId, GlobalElementId, ImageCacheError, ImageSource, @@ -258,6 +267,7 @@ async fn request_image( /// Otherwise decoding would silently start a second, ungated HTTP request. fn decode_image(bytes: Vec, renderer: SvgRenderer) -> ImageResult { let Ok(format) = image::guess_format(&bytes) else { + refuse_svg_file_references(&bytes)?; return renderer .render_single_frame(&bytes, 1.0) .map_err(Into::into); @@ -282,6 +292,35 @@ fn decode_image(bytes: Vec, renderer: SvgRenderer) -> ImageResult { Ok(Arc::new(RenderImage::new(frames))) } +/// GPUI renders an SVG `` whose href is not a data URL from the local +/// file it names, which would draw files the script has no grant to read. +/// Parse the SVG with a resolver that records such an href, and refuse the +/// image when it has one. +fn refuse_svg_file_references(bytes: &[u8]) -> Result<(), ImageCacheError> { + let references_file = Arc::new(AtomicBool::new(false)); + let options = usvg::Options { + image_href_resolver: usvg::ImageHrefResolver { + resolve_data: usvg::ImageHrefResolver::default_data_resolver(), + resolve_string: Box::new({ + let references_file = references_file.clone(); + move |_, _| { + references_file.store(true, Ordering::Relaxed); + None + } + }), + }, + ..Default::default() + }; + usvg::Tree::from_data(bytes, &options) + .map_err(|error| ImageCacheError::Usvg(Arc::new(error)))?; + if references_file.load(Ordering::Relaxed) { + return Err(ImageCacheError::Asset( + "TextView SVG images cannot reference files".into(), + )); + } + Ok(()) +} + fn static_frame( mut decoder: impl image::ImageDecoder, ) -> Result, ImageCacheError> { diff --git a/crates/shell/src/materialize/text_view/tests.rs b/crates/shell/src/materialize/text_view/tests.rs index 42d42f1bf8..070874aba7 100644 --- a/crates/shell/src/materialize/text_view/tests.rs +++ b/crates/shell/src/materialize/text_view/tests.rs @@ -375,3 +375,21 @@ fn text_view_document_images_keep_policies_separate(cx: &mut TestAppContext) { "only the first document may follow the redirect; none may borrow another cache", ); } + +#[gpui::test] +fn document_svg_images_cannot_read_local_files(cx: &mut TestAppContext) { + let path = std::env::temp_dir().join(format!("gpui-shell-svg-{}.png", std::process::id())); + std::fs::write(&path, PNG).unwrap(); + let svg = format!( + r#""#, + path.display() + ); + let decoded = cx.update(|cx| decode_image(svg.into_bytes(), cx.svg_renderer())); + std::fs::remove_file(&path).unwrap(); + assert!(decoded.is_err()); + + // An SVG that references no file still decodes. + let svg = r#""#; + let decoded = cx.update(|cx| decode_image(svg.as_bytes().to_vec(), cx.svg_renderer())); + assert!(decoded.is_ok()); +}