diff --git a/crates/design-challenge/src/screenshot.rs b/crates/design-challenge/src/screenshot.rs index f5641ebe9..4a4585095 100644 --- a/crates/design-challenge/src/screenshot.rs +++ b/crates/design-challenge/src/screenshot.rs @@ -6,8 +6,16 @@ //! sandbox CSP meant for browser embedding, and the stored artifact is the //! capture source of truth. `--no-sandbox` is required because Chromium's //! renderer sandbox needs user namespaces / `CAP_SYS_ADMIN`, which Docker -//! containers do not grant; the container boundary plus the scriptless -//! sanitized artifact is the sandbox. +//! containers do not grant. +//! +//! Network isolation: Chromium shares the design-challenge netns (high-trust +//! `base` network). All `http(s)` subresource / navigation attempts are forced +//! through `design-egress-proxy` (`--proxy-server` + `--proxy-bypass-list= +//! <-loopback>`) so the same internal-target blocklist that guards miner +//! sandboxes also covers screenshot SSRF (gateway admin, metadata, postgres, +//! socket-proxy). Capture documents also carry a nonce-locked CSP that blocks +//! unintended scripts and navigations (CLI Chromium has no Playwright route +//! hooks). use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; @@ -29,10 +37,29 @@ const DEFAULT_MAX_HEIGHT: u32 = 12_000; const DEFAULT_TIMEOUT_SECS: u64 = 90; /// Virtual-time budget per render so remote images settle (ms). const VIRTUAL_TIME_BUDGET_MS: u32 = 10_000; +/// Default forward proxy for screenshot Chromium (`DESIGN_SCREENSHOT_PROXY`). +const DEFAULT_SCREENSHOT_PROXY: &str = "http://design-egress-proxy:8094"; /// Marker the height probe writes into ``. const HEIGHT_MARKER: &str = "SHOTH="; /// Height probe appended to the throwaway capture document (never shipped). -const MEASURE_SCRIPT: &str = "<script>addEventListener('load',function(){setTimeout(function(){var d=document,e=d.documentElement,b=d.body;d.title='SHOTH='+Math.max(e.scrollHeight,b?b.scrollHeight:0)},50)})</script>"; +/// Nonce `designshot1` must match `CAPTURE_CSP` `script-src`. +const MEASURE_SCRIPT: &str = "<script nonce=\"designshot1\">addEventListener('load',function(){setTimeout(function(){var d=document,e=d.documentElement,b=d.body;d.title='SHOTH='+Math.max(e.scrollHeight,b?b.scrollHeight:0)},50)})</script>"; +/// Capture-document CSP: nonce script only; block connect/nav; allow public +/// img/font/style so CDN assets still paint (they still traverse the egress +/// proxy blocklist). +const CAPTURE_CSP: &str = "default-src 'none'; \ +img-src data: https: http:; \ +style-src 'unsafe-inline' data: https: http:; \ +font-src data: https: http:; \ +script-src 'nonce-designshot1'; \ +connect-src 'none'; \ +frame-src 'none'; \ +object-src 'none'; \ +media-src 'none'; \ +worker-src 'none'; \ +base-uri 'none'; \ +form-action 'none'; \ +navigate-to 'none'"; /// Capture knobs resolved from env at call time (tests build them directly). #[derive(Debug, Clone)] @@ -45,6 +72,9 @@ struct CaptureConfig { max_height: u32, /// Total attempts (initial + retries). attempts: u32, + /// Forward proxy URL (`None` / empty = direct; prod compose always sets + /// `design-egress-proxy`). + proxy: Option<String>, } impl CaptureConfig { @@ -72,10 +102,21 @@ impl CaptureConfig { )), max_height: env_u32("DESIGN_SCREENSHOT_MAX_HEIGHT", DEFAULT_MAX_HEIGHT), attempts: 2, + proxy: screenshot_proxy_from_env(), } } } +/// Resolve `DESIGN_SCREENSHOT_PROXY`. Unset → egress proxy default; empty +/// string → disable (local stub tests / operators debugging without compose). +fn screenshot_proxy_from_env() -> Option<String> { + match std::env::var("DESIGN_SCREENSHOT_PROXY") { + Ok(v) if v.is_empty() => None, + Ok(v) => Some(v), + Err(_) => Some(DEFAULT_SCREENSHOT_PROXY.to_owned()), + } +} + /// Capture a full-page PNG of sanitized `html` (best-effort). /// /// Two Chromium passes per attempt: measure the rendered height (probe script @@ -144,6 +185,7 @@ fn capture_once( stamp, attempt, cfg.timeout, + cfg.proxy.as_deref(), ) }); if !ok { @@ -173,7 +215,7 @@ fn measure_height( let profile = profile_dir(work_dir, stamp, attempt, "dom"); let out = run_with_timeout( Command::new(bin) - .args(base_args(&profile)) + .args(base_args(&profile, cfg.proxy.as_deref())) .arg("--dump-dom") .arg(url), cfg.timeout, @@ -193,11 +235,12 @@ fn shoot( stamp: u128, attempt: u32, timeout: Duration, + proxy: Option<&str>, ) -> bool { let profile = profile_dir(work_dir, stamp, attempt, "png"); let res = run_with_timeout( Command::new(bin) - .args(base_args(&profile)) + .args(base_args(&profile, proxy)) .arg(format!("--screenshot={}", out.display())) .arg(format!("--window-size={WIDTH},{height}")) .arg(url), @@ -208,11 +251,11 @@ fn shoot( } /// Shared headless flags. `--no-sandbox`: the renderer sandbox needs userns / -/// `CAP_SYS_ADMIN`, unavailable in Docker — the container plus the scriptless -/// sanitized artifact is the security boundary. `--disable-dev-shm-usage`: -/// Docker caps `/dev/shm` at 64MiB, which crashes tall renders. -fn base_args(profile: &Path) -> Vec<String> { - [ +/// `CAP_SYS_ADMIN`, unavailable in Docker — network isolation is the egress +/// proxy + capture CSP below. `--disable-dev-shm-usage`: Docker caps +/// `/dev/shm` at 64MiB, which crashes tall renders. +fn base_args(profile: &Path, proxy: Option<&str>) -> Vec<String> { + let mut args: Vec<String> = [ "--headless=new", "--no-sandbox", "--disable-setuid-sandbox", @@ -220,10 +263,12 @@ fn base_args(profile: &Path) -> Vec<String> { "--disable-gpu", "--disable-crash-reporter", "--disable-breakpad", + "--disable-background-networking", "--no-first-run", "--hide-scrollbars", "--force-color-profile=srgb", "--run-all-compositor-stages-before-draw", + "--block-new-web-contents", ] .into_iter() .map(str::to_owned) @@ -231,7 +276,15 @@ fn base_args(profile: &Path) -> Vec<String> { format!("--virtual-time-budget={VIRTUAL_TIME_BUDGET_MS}"), format!("--user-data-dir={}", profile.display()), ]) - .collect() + .collect(); + if let Some(p) = proxy.filter(|s| !s.is_empty()) { + // Route all http(s) — including loopback / link-local — through the + // design-egress-proxy blocklist. `<-loopback>` removes Chrome's + // implicit bypass of localhost (and related) targets. + args.push(format!("--proxy-server={p}")); + args.push("--proxy-bypass-list=<-loopback>".into()); + } + args } /// Spawn → poll → kill on timeout. `Command::wait_timeout` is unstable, so @@ -259,10 +312,13 @@ fn run_with_timeout(cmd: &mut Command, timeout: Duration) -> Option<Output> { } /// Wrap the sanitized fragment (ammonia unwraps the html/head/body shell) -/// into a capture document. The only script is our own height probe. +/// into a capture document. The only script is our own height probe (CSP +/// nonce); capture CSP blocks other scripts and navigations. fn render_doc(fragment: &str) -> String { format!( - "<!DOCTYPE html><html><head><meta charset=\"utf-8\"><title>shot{fragment}{MEASURE_SCRIPT}" + "\ +\ +shot{fragment}{MEASURE_SCRIPT}" ) } @@ -325,6 +381,9 @@ mod tests { timeout: Duration::from_secs(5), max_height: DEFAULT_MAX_HEIGHT, attempts: 1, + // Stub browsers do not need a real proxy; empty env would still + // default to design-egress-proxy in from_env(). + proxy: None, } } @@ -451,9 +510,58 @@ mod tests { assert!(doc.starts_with("")); assert!(doc.contains("
hello
")); assert!(doc.contains("SHOTH=")); + assert!(doc.contains("Content-Security-Policy")); + assert!(doc.contains("nonce-designshot1")); + assert!(doc.contains("navigate-to 'none'")); + assert!(doc.contains("nonce=\"designshot1\"")); assert!(doc.ends_with("")); } + #[test] + fn base_args_force_egress_proxy_including_loopback() { + let profile = PathBuf::from("/tmp/shot-profile"); + let args = base_args(&profile, Some("http://design-egress-proxy:8094")); + assert!(args + .iter() + .any(|a| a == "--proxy-server=http://design-egress-proxy:8094")); + assert!(args.iter().any(|a| a == "--proxy-bypass-list=<-loopback>")); + assert!(args.iter().any(|a| a == "--block-new-web-contents")); + let direct = base_args(&profile, None); + assert!(direct.iter().all(|a| !a.starts_with("--proxy-server"))); + } + + #[test] + fn capture_passes_proxy_flags_to_browser() { + let dir = std::env::temp_dir().join(format!("shot-proxy-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let fake = dir.join("fake.png"); + std::fs::write(&fake, FAKE_PNG).unwrap(); + let args_log = dir.join("args.log"); + let stub = write_stub( + &dir, + &format!( + "#!/bin/sh\nprintf '%s\\n' \"$*\" >> \"{log}\"\nout=\"\"\nfor a in \"$@\"; do case \"$a\" in --screenshot=*) out=\"${{a#--screenshot=}}\";; esac; done\ncase \"$*\" in *--dump-dom*) echo 'SHOTH=900'; exit 0;; esac\nif [ -n \"$out\" ]; then cp \"{png}\" \"$out\"; exit 0; fi\nexit 1\n", + log = args_log.display(), + png = fake.display() + ), + ); + let mut cfg = test_cfg(&stub); + cfg.proxy = Some("http://design-egress-proxy:8094".into()); + let png = capture_with(&cfg, "

hi

", &dir.join("work")); + assert_eq!(png.as_deref(), Some(FAKE_PNG)); + let logged = std::fs::read_to_string(&args_log).unwrap(); + assert!( + logged.contains("--proxy-server=http://design-egress-proxy:8094"), + "{logged}" + ); + assert!( + logged.contains("--proxy-bypass-list=<-loopback>"), + "{logged}" + ); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn png_tuple_sha_and_b64() { let (path, b64, raw, sha, bytes) = png_artifact_tuple(FAKE_PNG); diff --git a/crates/design-sanitize/src/lib.rs b/crates/design-sanitize/src/lib.rs index 030ae0672..5826b444f 100644 --- a/crates/design-sanitize/src/lib.rs +++ b/crates/design-sanitize/src/lib.rs @@ -61,6 +61,23 @@ pub struct SanitizeResult { pub artifact_digest: String, } +/// Control-plane / metadata hostnames that must never be fetched from miner +/// HTML (screenshot Chromium or a future viewer). Mirrors +/// `design-egress-proxy::BLOCKED_HOSTNAMES` plus localhost aliases. +const BLOCKED_URL_HOSTS: &[&str] = &[ + "localhost", + "gateway", + "postgres", + "socket-proxy", + "design-challenge", + "prism-challenge", + "design-egress-proxy", + "updater", + "site-api", + "evil-gateway", + "metadata.google.internal", +]; + fn ammonia_builder() -> ammonia::Builder<'static> { let mut b = ammonia::Builder::default(); // Drop scriptable / navigational sinks; ammonia also strips on* handlers. @@ -233,6 +250,14 @@ pub fn sanitize_html(raw: &str) -> (String, SanitizeReport) { notes.push("css_blocked_inline".into()); } + // Neutralize http(s) href/src pointing at control-plane / metadata / + // RFC1918. Legitimate CDN URLs stay; residual CSS `url(...)` SSRF is + // covered by screenshot Chromium's egress-proxy force. + let (cleaned, ssrf_stripped) = neutralize_ssrf_urls(&cleaned); + if ssrf_stripped { + notes.push("ssrf_url_neutralized".into()); + } + ( cleaned, SanitizeReport { @@ -243,6 +268,125 @@ pub fn sanitize_html(raw: &str) -> (String, SanitizeReport) { ) } +/// True when `host` names loopback, metadata, or a compose control-plane +/// service (case-insensitive, trailing-dot tolerant). +#[must_use] +pub fn host_blocked_for_miner_url(host: &str) -> bool { + let h = host.trim_end_matches('.').to_ascii_lowercase(); + h == "localhost" + || h.ends_with(".internal") + || h.ends_with(".localhost") + || BLOCKED_URL_HOSTS.contains(&h.as_str()) +} + +/// True when a literal IP is non-public (loopback, link-local/metadata, +/// RFC1918, CGNAT, etc.). +#[must_use] +pub fn ip_blocked_for_miner_url(ip: &std::net::IpAddr) -> bool { + match ip { + std::net::IpAddr::V4(v4) => { + let o = v4.octets(); + o[0] == 0 + || o[0] == 10 + || o[0] == 127 + || (o[0] == 100 && (64..=127).contains(&o[1])) + || (o[0] == 169 && o[1] == 254) + || (o[0] == 172 && (16..=31).contains(&o[1])) + || (o[0] == 192 && o[1] == 168) + || o[0] >= 224 + } + std::net::IpAddr::V6(v6) => { + if let Some(mapped) = v6.to_ipv4_mapped() { + return ip_blocked_for_miner_url(&std::net::IpAddr::V4(mapped)); + } + let seg = v6.segments(); + v6.is_unspecified() + || v6.is_loopback() + || (seg[0] & 0xffc0) == 0xfe80 + || (seg[0] & 0xfe00) == 0xfc00 + || (seg[0] & 0xff00) == 0xff00 + } + } +} + +/// Extract host from an absolute or protocol-relative http(s) URL. +fn http_url_host(url: &str) -> Option { + let trimmed = url.trim(); + let lower = trimmed.to_ascii_lowercase(); + let rest = if lower.starts_with("https://") { + &trimmed["https://".len()..] + } else if lower.starts_with("http://") { + &trimmed["http://".len()..] + } else if lower.starts_with("//") { + &trimmed["//".len()..] + } else { + return None; + }; + let authority = rest.split(['/', '?', '#']).next().unwrap_or(""); + if authority.is_empty() { + return None; + } + // Drop userinfo. + let hostport = authority.rsplit('@').next().unwrap_or(authority); + let host = if hostport.starts_with('[') { + hostport + .trim_start_matches('[') + .split(']') + .next() + .unwrap_or("") + } else { + hostport.split(':').next().unwrap_or(hostport) + }; + if host.is_empty() { + None + } else { + Some(host.to_owned()) + } +} + +/// True when an http(s) / protocol-relative URL targets a blocked host or IP. +#[must_use] +pub fn url_looks_ssrf(url: &str) -> bool { + let Some(host) = http_url_host(url) else { + return false; + }; + if host_blocked_for_miner_url(&host) { + return true; + } + if let Ok(ip) = host.parse::() { + return ip_blocked_for_miner_url(&ip); + } + false +} + +/// Rewrite `href` / `src` (and a few cousins) that point at internal targets +/// to `#` so screenshot Chromium never even queues the request. Public CDN +/// URLs are left alone. +fn neutralize_ssrf_urls(html: &str) -> (String, bool) { + let Ok(re) = + Regex::new(r#"(?i)\s(href|src|poster|cite|formaction|action)\s*=\s*("([^"]*)"|'([^']*)')"#) + else { + return (html.to_owned(), false); + }; + let mut stripped = false; + let out = re + .replace_all(html, |caps: ®ex::Captures<'_>| { + let attr = caps.get(1).map_or("href", |m| m.as_str()); + let val = caps + .get(3) + .or_else(|| caps.get(4)) + .map_or("", |m| m.as_str()); + if url_looks_ssrf(val) { + stripped = true; + format!(" {attr}=\"#\"") + } else { + caps[0].to_owned() + } + }) + .into_owned(); + (out, stripped) +} + /// Drop or empty inline `style` attributes that fail [`filter_css`]. fn filter_inline_styles(html: &str) -> (String, bool) { let Ok(re) = Regex::new(r#"(?i)\sstyle\s*=\s*("([^"]*)"|'([^']*)')"#) else { @@ -573,4 +717,34 @@ body { margin: 0; background: var(--bg); } assert!(!out.to_ascii_lowercase().contains("