Merge branch 'fix/sec' into integration/round-1

This commit is contained in:
2026-08-23 11:46:17 -07:00
10 changed files with 1066 additions and 72 deletions
+5 -15
View File
@@ -1,6 +1,6 @@
{
"identifier": "default",
"description": "Default capabilities for Triple-C",
"description": "Default capabilities for Triple-C. Every entry here is an IPC command a compromised webview can call directly, so the set is kept to what the frontend actually uses. Two notes on what is deliberately absent and what is deliberately accepted: (1) the `store:*` grants were removed — nothing in `app/src` uses `@tauri-apps/plugin-store`, and the plugin's `resolve_store_path` is a `PathBuf::push` against AppData, which `push` discards outright when handed an absolute path, so the grant was an arbitrary host-file read/write primitive (`plugin:store|load` + `set` + `save` on `~/.claude/settings.json` is host code execution). (2) `drag:allow-start-drag` stays, and cannot be scoped — `tauri-plugin-drag` takes the item paths from the caller and has no scope mechanism, so a compromised webview could call `startDrag({ item: ['~/.ssh/id_rsa'] })` against any host path the user can read. It is not a silent exfiltration primitive: the drag only delivers anything if the user completes a real drop onto a real target, and the OS shows the drag under the cursor while it is in flight. Removing it would remove drag-out from the Files pane (`stage_container_file_for_drag`), whose fallback is the explicit \"Save to host…\" action. Accepted residual risk, recorded here rather than fixed.",
"windows": ["main"],
"permissions": [
"core:default",
@@ -15,20 +15,10 @@
"dialog:allow-message",
"dialog:allow-ask",
"dialog:allow-confirm",
"store:default",
"store:allow-get",
"store:allow-set",
"store:allow-delete",
"store:allow-keys",
"store:allow-values",
"store:allow-entries",
"store:allow-length",
"store:allow-load",
"store:allow-reset",
"store:allow-save",
"store:allow-clear",
"opener:default",
"opener:allow-open-url",
{
"identifier": "opener:allow-open-url",
"allow": [{ "url": "http://*" }, { "url": "https://*" }]
},
"drag:default",
"drag:allow-start-drag"
]
+1 -1
View File
@@ -1 +1 @@
{"default":{"identifier":"default","description":"Default capabilities for Triple-C","local":true,"windows":["main"],"permissions":["core:default","core:event:default","core:event:allow-emit","core:event:allow-listen","core:event:allow-unlisten","core:event:allow-emit-to","dialog:default","dialog:allow-open","dialog:allow-save","dialog:allow-message","dialog:allow-ask","dialog:allow-confirm","store:default","store:allow-get","store:allow-set","store:allow-delete","store:allow-keys","store:allow-values","store:allow-entries","store:allow-length","store:allow-load","store:allow-reset","store:allow-save","store:allow-clear","opener:default","opener:allow-open-url","drag:default","drag:allow-start-drag"]}}
{"default":{"identifier":"default","description":"Default capabilities for Triple-C. Every entry here is an IPC command a compromised webview can call directly, so the set is kept to what the frontend actually uses. Two notes on what is deliberately absent and what is deliberately accepted: (1) the `store:*` grants were removed — nothing in `app/src` uses `@tauri-apps/plugin-store`, and the plugin's `resolve_store_path` is a `PathBuf::push` against AppData, which `push` discards outright when handed an absolute path, so the grant was an arbitrary host-file read/write primitive (`plugin:store|load` + `set` + `save` on `~/.claude/settings.json` is host code execution). (2) `drag:allow-start-drag` stays, and cannot be scoped — `tauri-plugin-drag` takes the item paths from the caller and has no scope mechanism, so a compromised webview could call `startDrag({ item: ['~/.ssh/id_rsa'] })` against any host path the user can read. It is not a silent exfiltration primitive: the drag only delivers anything if the user completes a real drop onto a real target, and the OS shows the drag under the cursor while it is in flight. Removing it would remove drag-out from the Files pane (`stage_container_file_for_drag`), whose fallback is the explicit \"Save to host…\" action. Accepted residual risk, recorded here rather than fixed.","local":true,"windows":["main"],"permissions":["core:default","core:event:default","core:event:allow-emit","core:event:allow-listen","core:event:allow-unlisten","core:event:allow-emit-to","dialog:default","dialog:allow-open","dialog:allow-save","dialog:allow-message","dialog:allow-ask","dialog:allow-confirm",{"identifier":"opener:allow-open-url","allow":[{"url":"http://*"},{"url":"https://*"}]},"drag:default","drag:allow-start-drag"]}}
+177 -16
View File
@@ -334,7 +334,10 @@ async fn poll_loop(
Ok(text) => {
exec_failures = 0;
let discovered = proc_net::parse_loopback_listeners(&text);
let skip = skipped_ports(&project);
// Re-read every tick: a project can gain a port mapping and the
// gateway/STT/web-terminal ports can be re-pointed while the
// bridge is running, and a stale reservation set is a hole.
let skip = skipped_ports(&project, &store.list(), &app_settings(&app));
if reconcile(&container_id, &discovered, &skip, &state).await {
emit_status(&app, &project_id, &state, true).await;
}
@@ -377,24 +380,101 @@ async fn poll_loop(
}
}
/// Ports Docker already handles for this project. A container port that is
/// explicitly published has a host-side path already, and the mapping's host
/// port is a binding we must not fight over.
/// Every port this project's bridge must not take.
///
/// [`RESERVED_CONTAINER_PORTS`] is folded in as well: those are container
/// loopback listeners another feature owns and exposes on its own,
/// authenticated terms.
fn skipped_ports(project: &crate::models::Project) -> HashSet<u16> {
/// The bridge's rule is "a container loopback listener on port N becomes an
/// **unauthenticated** host listener on port N". That is only safe for ports
/// nothing else on the host owns, so everything that *is* owned has to be
/// enumerated here. Four sources:
///
/// 1. **This project's own published ports** — a container port that Docker
/// already publishes has a host-side path, and the mapping's host port is a
/// binding we must not fight over.
/// 2. **Every other project's published host ports.** The container names the
/// *host* port, so project A's container listening on 8080 would otherwise
/// have the bridge bind host 8080 — the port project B publishes on. Only
/// the host end of another project's mapping is reserved: its container end
/// is a number inside a different network namespace and means nothing here.
/// 3. **This app's own host services** — the LiteLLM gateway, the STT sidecar
/// and the web terminal. All three are off by default and bind on demand, so
/// first-come would win: a container that binds container-loopback 4000
/// while the gateway is stopped gets host `127.0.0.1:4000` mirrored to it
/// within one [`POLL_INTERVAL`], after which the gateway cannot start and
/// anything on the host dialling 4000 — including *other project
/// containers*, which reach the gateway by host address — is talking to the
/// squatting container instead. The web terminal is the worst of the three,
/// because its access token travels in the URL query. Both the *configured*
/// port and the shipped default are reserved: the configured one is what the
/// service will bind next, and the default is what it falls back to for a
/// fresh profile or a settings file that failed to parse.
/// 4. [`RESERVED_CONTAINER_PORTS`] and [`RESERVED_HOST_PORTS`] — the
/// browser-view pane's two ends, which it exposes on its own authenticated
/// terms.
///
/// Pure on purpose: everything it needs is passed in, so the whole reservation
/// policy is unit-testable without a store, a container or an app handle.
fn skipped_ports(
project: &crate::models::Project,
all_projects: &[crate::models::Project],
settings: &crate::models::AppSettings,
) -> HashSet<u16> {
let mut skip: HashSet<u16> = project
.port_mappings
.iter()
.flat_map(|m| [m.container_port, m.host_port])
.collect();
// Other projects: host end only.
skip.extend(
all_projects
.iter()
.filter(|p| p.id != project.id)
.flat_map(|p| p.port_mappings.iter().map(|m| m.host_port)),
);
skip.extend(app_service_host_ports(settings));
skip.extend(RESERVED_CONTAINER_PORTS.clone());
skip.extend(RESERVED_HOST_PORTS.clone());
skip
}
/// Current app settings, or defaults if the state is not reachable.
///
/// Falling back rather than unwrapping matters: the reservation set is a safety
/// rail, and a rail that panics the poller when it cannot read its input is
/// worse than one that falls back to the shipped port numbers — which are what
/// the services use anyway until someone changes them.
fn app_settings(app: &AppHandle) -> crate::models::AppSettings {
use tauri::Manager;
app.try_state::<crate::AppState>()
.map(|state| state.settings_store.get())
.unwrap_or_default()
}
/// Host ports this app's own sibling services bind, configured value and
/// shipped default alike.
///
/// Read off the settings models rather than restated as literals here: a
/// duplicated port number is exactly the kind of constant that drifts silently,
/// and the failure mode of drift is a reservation that no longer covers the
/// service it was written for.
fn app_service_host_ports(settings: &crate::models::AppSettings) -> Vec<u16> {
use crate::models::{SttSettings, WebTerminalSettings};
vec![
// LiteLLM gateway (`docker/gateway.rs`).
settings.gateway.port,
crate::models::default_gateway_port(),
// Speech-to-text sidecar (`docker/stt.rs`).
settings.stt.port,
SttSettings::default().port,
// Remote web terminal (`web_terminal/server.rs`) — binds 0.0.0.0, and
// its access token is in the URL query.
settings.web_terminal.port,
WebTerminalSettings::default().port,
]
}
// ─────────────────────────────────────────────────────────────────────────────
// Reservations
// ─────────────────────────────────────────────────────────────────────────────
@@ -591,7 +671,7 @@ async fn emit_status(
#[cfg(test)]
mod tests {
use super::*;
use crate::models::{PortMapping, Project, ProjectPath};
use crate::models::{AppSettings, PortMapping, Project, ProjectPath};
fn project_with_mappings(mappings: Vec<(u16, u16)>) -> Project {
let mut p = Project::new(
@@ -612,9 +692,14 @@ mod tests {
p
}
/// The common case: one project, no siblings, stock settings.
fn skip_for(project: &Project) -> HashSet<u16> {
skipped_ports(project, std::slice::from_ref(project), &AppSettings::default())
}
#[test]
fn ports_already_published_by_docker_are_skipped() {
let skip = skipped_ports(&project_with_mappings(vec![(3000, 3000), (8081, 8080)]));
let skip = skip_for(&project_with_mappings(vec![(3000, 3000), (8081, 8080)]));
assert!(skip.contains(&3000));
// Both ends of an asymmetric mapping are off limits: the container port
// is already reachable, and the host port is Docker's binding.
@@ -624,20 +709,96 @@ mod tests {
}
#[test]
fn no_mappings_means_nothing_but_the_reserved_ranges_are_skipped() {
let skip = skipped_ports(&project_with_mappings(vec![]));
fn no_mappings_means_nothing_but_the_reservations_are_skipped() {
let settings = AppSettings::default();
let project = project_with_mappings(vec![]);
let skip = skip_for(&project);
let mut expected: HashSet<u16> = RESERVED_CONTAINER_PORTS.collect();
expected.extend(RESERVED_HOST_PORTS);
expected.extend(app_service_host_ports(&settings));
assert_eq!(skip, expected);
// The ranges and the service ports are disjoint, so nothing above is
// accidentally counting the same port twice.
assert_eq!(
skip.len(),
RESERVED_CONTAINER_PORTS.clone().count() + RESERVED_HOST_PORTS.clone().count()
RESERVED_CONTAINER_PORTS.clone().count()
+ RESERVED_HOST_PORTS.clone().count()
+ 3
);
}
#[test]
fn this_apps_own_host_services_are_never_taken() {
// The bug this guards: the reserved set used to cover only the
// browser-view ranges and this project's own mappings, so a container
// binding container-loopback 4000 / 9876 / 7681 while the matching
// service was stopped had that port mirrored, unauthenticated, onto the
// host — taking the gateway's, the STT sidecar's or the web terminal's
// door before they could bind it.
let settings = AppSettings::default();
let skip = skip_for(&project_with_mappings(vec![]));
assert!(skip.contains(&settings.gateway.port), "LiteLLM gateway port");
assert!(skip.contains(&settings.stt.port), "STT sidecar port");
assert!(skip.contains(&settings.web_terminal.port), "web terminal port");
// The shipped defaults, spelled out once so a change to any of them is
// a change to this assertion and not a silent narrowing.
assert!(skip.contains(&4000));
assert!(skip.contains(&9876));
assert!(skip.contains(&7681));
}
#[test]
fn a_reconfigured_service_port_is_reserved_alongside_its_default() {
let mut settings = AppSettings::default();
settings.gateway.port = 4321;
settings.stt.port = 9000;
settings.web_terminal.port = 8443;
let project = project_with_mappings(vec![]);
let skip = skipped_ports(&project, std::slice::from_ref(&project), &settings);
for port in [4321, 9000, 8443] {
assert!(skip.contains(&port), "configured port {} should be reserved", port);
}
// The default stays reserved too: it is what the service falls back to
// for a fresh profile or an unparseable settings file, so leaving it
// open is leaving the same squat available one restart later.
for port in [4000, 9876, 7681] {
assert!(skip.contains(&port), "default port {} should be reserved", port);
}
}
#[test]
fn another_projects_published_host_port_is_not_stolen() {
// The container names the *host* port. Without this, project A's
// container listening on 8080 takes the host 8080 that project B
// publishes on — the bridge wins the race whenever B's container is not
// running yet.
let mine = project_with_mappings(vec![]);
let mut theirs = project_with_mappings(vec![(8080, 3000)]);
theirs.id = format!("{}-other", mine.id);
let skip = skipped_ports(
&mine,
&[mine.clone(), theirs.clone()],
&AppSettings::default(),
);
assert!(skip.contains(&8080), "another project's host port");
// …but not the other project's *container* port: that number lives in a
// different network namespace and means nothing on this host, and
// reserving it would refuse a legitimate login callback for no reason.
assert!(!skip.contains(&3000));
}
#[test]
fn the_browser_views_host_ports_are_never_taken() {
// The bridge binds *host* ports chosen by the container, so without
// this it can take the port the browser-view proxy will want later —
// that pane binds on demand, so first-come would win.
let skip = skipped_ports(&project_with_mappings(vec![]));
let skip = skip_for(&project_with_mappings(vec![]));
for port in RESERVED_HOST_PORTS {
assert!(skip.contains(&port), "host port {} should be reserved", port);
}
@@ -693,14 +854,14 @@ mod tests {
// Mirroring these would publish an ungated second door to the
// Playwright dashboard, which the pane deliberately keeps behind a
// token-checking listener.
let skip = skipped_ports(&project_with_mappings(vec![]));
let skip = skip_for(&project_with_mappings(vec![]));
for port in RESERVED_CONTAINER_PORTS {
assert!(skip.contains(&port), "port {} should be reserved", port);
}
assert!(!skip.contains(&(RESERVED_CONTAINER_PORTS.end() + 1)));
// Reservations coexist with Docker's own published ports.
let skip = skipped_ports(&project_with_mappings(vec![(3000, 3000)]));
let skip = skip_for(&project_with_mappings(vec![(3000, 3000)]));
assert!(skip.contains(RESERVED_CONTAINER_PORTS.start()));
assert!(skip.contains(&3000));
}
+542 -9
View File
@@ -13,8 +13,48 @@
//! The exec plumbing itself is *not* reimplemented here: it comes from
//! [`crate::docker::exec::create_attached_exec`], the same helper the
//! interactive terminal sessions are built on.
//!
//! ## What the host listener is, and is not
//!
//! The listener is **not authenticated**, and cannot be. The port number is
//! chosen by whatever CLI is logging in, the redirect URL is the provider's, and
//! nothing in that chain can be taught to present a token — so there is no path
//! token to add. Anything that can reach `127.0.0.1:<port>` on this host reaches
//! the container-side listener. That includes **any web page the user has open**,
//! which can port-scan loopback from script.
//!
//! Two things narrow that, and neither is a substitute for the other:
//!
//! * The whole feature is opt-in per project, off by default, and only mirrors
//! ports while its container is running.
//! * [`web_request_verdict`] refuses the one case that is unambiguously a web
//! page reaching in: a request whose fetch metadata says it is a cross-site
//! **sub-resource** (`fetch`, `XMLHttpRequest`, `<img>`, `<script src>`,
//! `<iframe>`). Cross-site *navigations* are allowed, because that is exactly
//! what an OAuth redirect is.
//!
//! The residual risk, stated plainly rather than papered over: a client that
//! sends no `Sec-Fetch-Site` header at all is not filtered — that is every
//! non-browser client (which is the point; `curl`, a CLI, the container's own
//! probe must all still work) but also any browser predating fetch metadata
//! (Chrome < 76, Firefox < 90, Safari < 16.4). A page can also still reach the
//! port with a top-level navigation it opens itself (`window.open`), which
//! carries `Sec-Fetch-Mode: navigate` and is indistinguishable from the redirect
//! the bridge exists to deliver. And nothing here inspects *what* is behind the
//! port: if the container has something more interesting than a throwaway OAuth
//! listener on loopback, a same-machine caller reaches it.
//!
//! ## Bounds
//!
//! Every accepted connection costs a `docker exec`, and the number of
//! connections is decided by whoever can reach the port. So each forward caps
//! concurrent connections ([`MAX_CONNECTIONS`]), refuses a client that opens a
//! socket and then says nothing ([`FIRST_BYTE_TIMEOUT`], enforced *before* the
//! exec is created), and drops a connection the container has gone quiet on
//! ([`IDLE_TIMEOUT`]).
use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr};
use std::time::Duration;
use bollard::container::LogOutput;
use futures_util::StreamExt;
@@ -30,6 +70,38 @@ use super::proc_net::PortFamily;
/// only needs to not be pathological.
const PUMP_BUF: usize = 16 * 1024;
/// Concurrent connections one forwarded port will carry.
///
/// Each one is a `docker exec`, and the client side is anything on the host that
/// can dial loopback — including a web page in a loop. A login callback is one
/// connection, occasionally a handful; this is generous for that and still a
/// bound the engine will not notice.
const MAX_CONNECTIONS: usize = 16;
/// How long an accepted connection has to send its first byte before it is
/// dropped, *without* a `docker exec` ever being created for it.
///
/// This is a deliberate narrowing of what the bridge carries: a client that
/// connects and says nothing is not the HTTP OAuth callback this exists for, and
/// forwarding it costs a container exec for a socket that may never speak. A
/// server-speaks-first protocol behind a bridged port would be refused by this;
/// that is the trade, and it is the only protocol shape affected.
const FIRST_BYTE_TIMEOUT: Duration = Duration::from_secs(5);
/// How long a live connection may go with nothing coming back from the container
/// before it is torn down. Generous, because a bridged port is not always a
/// short OAuth callback — but finite, so an abandoned connection cannot pin an
/// exec forever.
const IDLE_TIMEOUT: Duration = Duration::from_secs(600);
/// Ceiling on the request head buffered for [`web_request_verdict`]. Real heads
/// are well under 8 KiB; past this we stop looking and forward what we have.
const MAX_HEAD: usize = 32 * 1024;
/// How long the rest of a request head has, once the first line has identified
/// the connection as HTTP. Only a stalled or hostile client reaches it.
const HEAD_TIMEOUT: Duration = Duration::from_secs(10);
/// Aborts a task when dropped, so a cancelled parent can never leave a detached
/// child running.
struct AbortOnDrop(JoinHandle<()>);
@@ -167,6 +239,22 @@ async fn accept_loop(
match accepted {
Ok((stream, peer)) => {
// Reap first, so the cap counts *live* connections rather than
// every one this listener has ever accepted.
while conns.try_join_next().is_some() {}
if conns.len() >= MAX_CONNECTIONS {
// Dropping the stream closes it. Better than queueing: the
// client side is whatever can dial loopback, so a queue is
// just a slower way to run out of execs.
log::warn!(
"Auth bridge: refusing connection from {} to bridged port {} — \
{} concurrent connections already open on it",
peer,
port,
MAX_CONNECTIONS
);
continue;
}
log::debug!("Auth bridge: connection from {} to bridged port {}", peer, port);
let _ = stream.set_nodelay(true);
conns.spawn(tunnel_connection(
@@ -195,9 +283,224 @@ async fn accept_optional(
}
}
/// Carry one accepted host connection into the container over `socat`.
async fn tunnel_connection(container_id: String, target: String, stream: TcpStream, port: u16) {
tunnel_connection_with_prelude(container_id, target, stream, port, Vec::new()).await
/// Carry one accepted host connection into the container over `socat`, after
/// deciding it is not a web page reaching into loopback.
///
/// Nothing is forwarded until that decision is made, so a refused request never
/// reaches the container at all — not even a `docker exec`.
async fn tunnel_connection(container_id: String, target: String, mut stream: TcpStream, port: u16) {
let head = match read_leading_bytes(&mut stream).await {
Ok(head) => head,
Err(e) => {
log::debug!(
"Auth bridge: dropping connection to bridged port {} before forwarding: {}",
port,
e
);
return;
}
};
if let LeadingBytes::HttpRequest { buffer, head_len } = &head {
// Authorize against the head slice only. Parsing past the blank line is
// how a request *body* gets read as headers — a cross-site `fetch` with
// a `text/plain` body is not preflighted, so it can put any line it
// likes in there.
let head_text = String::from_utf8_lossy(&buffer[..*head_len]);
if web_request_verdict(&head_text) == Verdict::RefuseCrossSite {
log::warn!(
"Auth bridge: refused a cross-site sub-resource request to bridged port {} — \
a web page, not a login redirect",
port
);
let _ = refuse(&mut stream).await;
return;
}
}
// The bytes already off the socket go back on the wire first, byte-exact.
tunnel_connection_with_prelude(container_id, target, stream, port, head.into_buffer()).await
}
/// What the first bytes of an accepted connection turned out to be.
enum LeadingBytes {
/// An HTTP request whose head we have in full. `head_len` is one past the
/// blank line; `buffer` may hold pipelined body bytes beyond it.
HttpRequest { buffer: Vec<u8>, head_len: usize },
/// Not HTTP, or HTTP we gave up on reading. Forwarded verbatim, ungated.
Opaque(Vec<u8>),
}
impl LeadingBytes {
fn into_buffer(self) -> Vec<u8> {
match self {
LeadingBytes::HttpRequest { buffer, .. } => buffer,
LeadingBytes::Opaque(buffer) => buffer,
}
}
}
/// Read just enough of the connection to classify it, without consuming
/// anything the caller cannot replay.
///
/// Bails out to [`LeadingBytes::Opaque`] the moment the first line proves this
/// is not HTTP, so a non-HTTP protocol pays one line of latency and no more.
/// The only hard failure is silence: a client that sends nothing within
/// [`FIRST_BYTE_TIMEOUT`] is dropped before an exec is spent on it.
async fn read_leading_bytes(stream: &mut TcpStream) -> Result<LeadingBytes, String> {
let mut buf: Vec<u8> = Vec::with_capacity(1024);
let mut chunk = [0u8; 1024];
let mut deadline = tokio::time::Instant::now() + FIRST_BYTE_TIMEOUT;
loop {
let n = match tokio::time::timeout_at(deadline, stream.read(&mut chunk)).await {
Ok(Ok(0)) if buf.is_empty() => {
return Err("closed before sending anything".to_string())
}
// A half-close after some bytes is legitimate; forward what we have.
Ok(Ok(0)) => return Ok(LeadingBytes::Opaque(buf)),
Ok(Ok(n)) => n,
Ok(Err(e)) => return Err(format!("read failed: {}", e)),
Err(_) if buf.is_empty() => {
return Err(format!(
"sent nothing within {}s",
FIRST_BYTE_TIMEOUT.as_secs()
))
}
// Bytes arrived but the head never finished. Fail open: this is a
// gate on top of the bridge, not the bridge's reason to exist.
Err(_) => return Ok(LeadingBytes::Opaque(buf)),
};
buf.extend_from_slice(&chunk[..n]);
// Once the first line is complete we know whether to keep reading.
if let Some(eol) = buf.iter().position(|b| *b == b'\n') {
if !is_http_request_line(&buf[..eol]) {
return Ok(LeadingBytes::Opaque(buf));
}
deadline = deadline.max(tokio::time::Instant::now() + HEAD_TIMEOUT);
} else if buf.len() > MAX_HEAD {
return Ok(LeadingBytes::Opaque(buf));
}
if let Some(head_len) = find_head_end(&buf) {
return Ok(LeadingBytes::HttpRequest {
buffer: buf,
head_len,
});
}
if buf.len() > MAX_HEAD {
return Ok(LeadingBytes::Opaque(buf));
}
}
}
/// Whether a first line looks like `METHOD target HTTP/1.x`.
fn is_http_request_line(line: &[u8]) -> bool {
let line = String::from_utf8_lossy(line);
let line = line.trim_end_matches(['\r', '\n']);
let mut parts = line.split(' ');
let (Some(method), Some(target), Some(version), None) =
(parts.next(), parts.next(), parts.next(), parts.next())
else {
return false;
};
!method.is_empty()
&& method.chars().all(|c| c.is_ascii_uppercase())
&& !target.is_empty()
&& (version == "HTTP/1.1" || version == "HTTP/1.0")
}
/// Index just past the blank line terminating an HTTP head, if it has arrived.
/// Tolerates a bare-LF terminator, which some minimal clients still emit.
fn find_head_end(buf: &[u8]) -> Option<usize> {
buf.windows(4)
.position(|w| w == b"\r\n\r\n")
.map(|i| i + 4)
.or_else(|| buf.windows(2).position(|w| w == b"\n\n").map(|i| i + 2))
}
/// Tell a refused caller why, then close. Plain text and `Connection: close` —
/// there is no session here to keep alive.
async fn refuse(stream: &mut TcpStream) -> std::io::Result<()> {
const BODY: &str = "This port is bridged from a container by Triple-C for a sign-in \
callback. It is not an API for web pages to call.\n";
let response = format!(
"HTTP/1.1 403 Forbidden\r\n\
Content-Type: text/plain; charset=utf-8\r\n\
Content-Length: {}\r\n\
Cache-Control: no-store\r\n\
Connection: close\r\n\r\n{}",
BODY.len(),
BODY
);
stream.write_all(response.as_bytes()).await?;
stream.shutdown().await
}
// ─────────────────────────────────────────────────────────────────────────────
// The gate — pure, so it can be tested without sockets
// ─────────────────────────────────────────────────────────────────────────────
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum Verdict {
/// Forward it. Either it is not a browser, or the browser says this is a
/// navigation or a same-origin request.
Allow,
/// Fetch metadata says a document on another site pulled this in as a
/// sub-resource. No login flow looks like that.
RefuseCrossSite,
}
/// Decide whether an HTTP request head arriving on a bridged port may be
/// forwarded into the container.
///
/// Deliberately fail-open — see the module docs for exactly what that leaves
/// uncovered. The only refusal is the case with no innocent reading:
/// `Sec-Fetch-Site` says another site, and `Sec-Fetch-Mode` says this is not a
/// navigation. `Sec-Fetch-*` are forbidden header names, so page script cannot
/// set or clear them.
pub(crate) fn web_request_verdict(head: &str) -> Verdict {
let mut lines = head.split(['\r', '\n']).filter(|l| !l.is_empty());
// Skip the request line.
if lines.next().is_none() {
return Verdict::Allow;
}
let mut site: Option<&str> = None;
let mut mode: Option<&str> = None;
for line in lines {
let Some((name, value)) = line.split_once(':') else {
continue;
};
let value = value.trim();
match name.trim().to_ascii_lowercase().as_str() {
// A duplicate of either header is header smuggling, not a client.
// Refuse rather than pick a winner: last-occurrence-wins is what
// turns a smuggling primitive into a bypass.
"sec-fetch-site" if site.is_some() => return Verdict::RefuseCrossSite,
"sec-fetch-mode" if mode.is_some() => return Verdict::RefuseCrossSite,
"sec-fetch-site" => site = Some(value),
"sec-fetch-mode" => mode = Some(value),
_ => {}
}
}
let Some(site) = site else {
// No fetch metadata: a CLI, `curl`, or a browser old enough not to send
// it. Not something this gate can judge.
return Verdict::Allow;
};
if site.eq_ignore_ascii_case("same-origin") || site.eq_ignore_ascii_case("none") {
return Verdict::Allow;
}
// `navigate` is precisely the OAuth redirect: the provider sends the browser
// to `http://localhost:<port>/callback`, cross-site, as a document load.
// Refusing it would refuse the feature.
if mode.is_none_or(|m| m.eq_ignore_ascii_case("navigate")) {
return Verdict::Allow;
}
Verdict::RefuseCrossSite
}
/// As [`tunnel_connection`], but `prelude` is written into the container first,
@@ -250,21 +553,36 @@ pub async fn tunnel_connection_with_prelude(
}
let mut buf = vec![0u8; PUMP_BUF];
loop {
match host_rx.read(&mut buf).await {
Ok(0) => break,
Ok(n) => {
// Idle-bounded. Without this a client that connects, sends a
// request and then never speaks or closes holds the exec open for
// as long as the container runs.
match tokio::time::timeout(IDLE_TIMEOUT, host_rx.read(&mut buf)).await {
Ok(Ok(0)) | Err(_) => break,
Ok(Ok(n)) => {
if input.write_all(&buf[..n]).await.is_err() || input.flush().await.is_err() {
break;
}
}
Err(_) => break,
Ok(Err(_)) => break,
}
}
}));
// Container → host. This direction is authoritative: when the exec's output
// stream ends, socat has exited and the connection is over.
while let Some(chunk) = output.next().await {
// stream ends, socat has exited and the connection is over. It is also the
// one that decides the connection is dead: nothing back from the container
// for `IDLE_TIMEOUT` tears the whole thing down, exec included.
while let Some(chunk) = match tokio::time::timeout(IDLE_TIMEOUT, output.next()).await {
Ok(chunk) => chunk,
Err(_) => {
log::debug!(
"Auth bridge: bridged port {} idle for {}s — closing the tunnel",
port,
IDLE_TIMEOUT.as_secs()
);
None
}
} {
match chunk {
// Only stdout is payload. The exec is created with tty = false
// precisely so Docker demultiplexes these, keeping socat's stderr
@@ -293,3 +611,218 @@ pub async fn tunnel_connection_with_prelude(
// Explicit: stop reading from the host now that the container side is gone.
drop(upstream);
}
#[cfg(test)]
mod tests {
use super::*;
fn head(lines: &[&str]) -> String {
format!("{}\r\n\r\n", lines.join("\r\n"))
}
#[test]
fn a_cli_callback_with_no_fetch_metadata_is_forwarded() {
// The overwhelmingly common case, and the reason the gate fails open:
// `curl`, a CLI's own probe, and anything not a browser send none of
// these headers, and none of them can be judged from the wire.
let verdict = web_request_verdict(&head(&[
"GET /callback?code=abc HTTP/1.1",
"Host: localhost:41733",
"User-Agent: curl/8.5.0",
]));
assert_eq!(verdict, Verdict::Allow);
}
#[test]
fn the_oauth_redirect_is_forwarded_even_though_it_is_cross_site() {
// This is the feature. The provider bounces the browser to
// `http://localhost:<port>/callback`, which is cross-site and a
// navigation. Refusing it would refuse every login the bridge exists
// for.
for site in ["cross-site", "same-site"] {
let verdict = web_request_verdict(&head(&[
"GET /callback?code=abc&state=xyz HTTP/1.1",
"Host: localhost:41733",
&format!("Sec-Fetch-Site: {}", site),
"Sec-Fetch-Mode: navigate",
"Sec-Fetch-Dest: document",
]));
assert_eq!(verdict, Verdict::Allow, "site={}", site);
}
}
#[test]
fn a_form_post_callback_is_forwarded() {
// `response_mode=form_post` providers POST the callback as a
// navigation. Still a navigation, still allowed.
let verdict = web_request_verdict(&head(&[
"POST /callback HTTP/1.1",
"Host: localhost:41733",
"Origin: https://login.microsoftonline.com",
"Sec-Fetch-Site: cross-site",
"Sec-Fetch-Mode: navigate",
]));
assert_eq!(verdict, Verdict::Allow);
}
#[test]
fn a_cross_site_subresource_from_a_web_page_is_refused() {
// The case the gate exists for: a page the user happens to have open
// scanning loopback and poking whatever answers.
for mode in ["cors", "no-cors", "same-origin", "websocket"] {
let verdict = web_request_verdict(&head(&[
"GET /admin HTTP/1.1",
"Host: 127.0.0.1:41733",
"Origin: https://evil.example",
"Sec-Fetch-Site: cross-site",
&format!("Sec-Fetch-Mode: {}", mode),
]));
assert_eq!(verdict, Verdict::RefuseCrossSite, "mode={}", mode);
}
}
#[test]
fn the_containers_own_same_origin_requests_are_forwarded() {
let verdict = web_request_verdict(&head(&[
"GET /style.css HTTP/1.1",
"Host: localhost:41733",
"Sec-Fetch-Site: same-origin",
"Sec-Fetch-Mode: no-cors",
]));
assert_eq!(verdict, Verdict::Allow);
// `none` is a user-initiated load — typed URL, bookmark.
let verdict = web_request_verdict(&head(&[
"GET / HTTP/1.1",
"Host: localhost:41733",
"Sec-Fetch-Site: none",
"Sec-Fetch-Mode: navigate",
]));
assert_eq!(verdict, Verdict::Allow);
}
#[test]
fn duplicated_fetch_metadata_is_refused_rather_than_resolved() {
// Last-occurrence-wins is what turns any header-smuggling primitive
// into a bypass, and no real client sends two.
let verdict = web_request_verdict(&head(&[
"GET /x HTTP/1.1",
"Sec-Fetch-Site: cross-site",
"Sec-Fetch-Mode: cors",
"Sec-Fetch-Site: same-origin",
]));
assert_eq!(verdict, Verdict::RefuseCrossSite);
}
#[test]
fn only_the_head_is_ever_judged() {
// A cross-site `text/plain` POST is not preflighted, so its *body* is
// fully attacker-chosen. `tunnel_connection` slices at the blank line
// before calling in; this pins that the slice is what gets judged.
let raw = "POST /x HTTP/1.1\r\n\
Sec-Fetch-Site: cross-site\r\n\
Sec-Fetch-Mode: cors\r\n\
Content-Type: text/plain\r\n\r\n\
Sec-Fetch-Site: same-origin\r\n";
let head_len = find_head_end(raw.as_bytes()).expect("head terminator");
let head = &raw[..head_len];
assert!(!head.contains("same-origin"), "the forged line must be past the slice");
assert_eq!(web_request_verdict(head), Verdict::RefuseCrossSite);
// And if the slice were ever got wrong, the duplicate rule is the
// backstop: a forged `Sec-Fetch-*` line is by construction a second
// copy of one the browser already sent, which is refused outright
// rather than resolved in the forgery's favour.
assert_eq!(web_request_verdict(raw), Verdict::RefuseCrossSite);
}
#[test]
fn a_non_http_first_line_is_never_treated_as_a_request() {
// Bridged ports are not all HTTP. Anything whose first line is not a
// request line is forwarded verbatim rather than parsed.
assert!(!is_http_request_line(b"\x16\x03\x01\x02\x00\x01"));
assert!(!is_http_request_line(b"*1\r"));
assert!(!is_http_request_line(b"SSH-2.0-OpenSSH_9.6"));
assert!(!is_http_request_line(b"GET /x HTTP/2.0"));
assert!(!is_http_request_line(b"get /x HTTP/1.1"));
assert!(is_http_request_line(b"GET /x HTTP/1.1\r"));
assert!(is_http_request_line(b"POST /callback?code=a%20b HTTP/1.0"));
}
#[test]
fn head_end_is_found_for_both_terminators() {
assert_eq!(find_head_end(b"GET / HTTP/1.1\r\n\r\nBODY"), Some(18));
assert_eq!(find_head_end(b"GET / HTTP/1.1\n\nBODY"), Some(16));
assert_eq!(find_head_end(b"GET / HTTP/1.1\r\nHost: x\r\n"), None);
}
#[tokio::test]
async fn a_client_that_says_nothing_never_costs_a_container_exec() {
// Every accepted connection would otherwise spawn a `docker exec`
// immediately, so silence was free for the caller and expensive here.
let listener = TcpListener::bind(SocketAddr::from((Ipv4Addr::LOCALHOST, 0)))
.await
.expect("bind");
let addr = listener.local_addr().expect("addr");
let accept = tokio::spawn(async move {
let (mut stream, _) = listener.accept().await.expect("accept");
read_leading_bytes(&mut stream).await
});
let _client = TcpStream::connect(addr).await.expect("connect");
let started = tokio::time::Instant::now();
let result = accept.await.expect("join");
assert!(result.is_err(), "silence should not be forwarded");
assert!(
started.elapsed() >= FIRST_BYTE_TIMEOUT,
"should have waited out the first-byte grace period"
);
}
#[tokio::test]
async fn a_non_http_client_is_classified_from_its_first_line_alone() {
let listener = TcpListener::bind(SocketAddr::from((Ipv4Addr::LOCALHOST, 0)))
.await
.expect("bind");
let addr = listener.local_addr().expect("addr");
let accept = tokio::spawn(async move {
let (mut stream, _) = listener.accept().await.expect("accept");
read_leading_bytes(&mut stream).await
});
let mut client = TcpStream::connect(addr).await.expect("connect");
client.write_all(b"SSH-2.0-OpenSSH_9.6\r\n").await.expect("write");
let result = accept.await.expect("join").expect("classified");
// Verbatim, and without waiting for a head terminator that will never
// come — the whole buffer is replayed into the tunnel.
assert!(matches!(result, LeadingBytes::Opaque(_)));
assert_eq!(result.into_buffer(), b"SSH-2.0-OpenSSH_9.6\r\n");
}
#[tokio::test]
async fn an_http_head_is_read_whole_and_replayed_whole() {
let listener = TcpListener::bind(SocketAddr::from((Ipv4Addr::LOCALHOST, 0)))
.await
.expect("bind");
let addr = listener.local_addr().expect("addr");
let accept = tokio::spawn(async move {
let (mut stream, _) = listener.accept().await.expect("accept");
read_leading_bytes(&mut stream).await
});
let raw = b"POST /callback HTTP/1.1\r\nHost: localhost\r\nContent-Length: 4\r\n\r\ncode";
let mut client = TcpStream::connect(addr).await.expect("connect");
client.write_all(raw).await.expect("write");
let result = accept.await.expect("join").expect("classified");
match &result {
LeadingBytes::HttpRequest { buffer, head_len } => {
assert_eq!(&buffer[*head_len..], b"code", "body must survive the peek");
assert!(!buffer[..*head_len].ends_with(b"code"));
}
LeadingBytes::Opaque(_) => panic!("should have been recognised as HTTP"),
}
assert_eq!(result.into_buffer(), raw.to_vec());
}
}
+84 -19
View File
@@ -304,21 +304,7 @@ impl BrowserViewManager {
// a container with no dashboard makes this a no-op.
let _ = kill_dashboard(&container_id, &cli_entry).await;
let container_port = pick_viewer_port(&container_id).await?;
launch_viewer(&container_id, &cli_entry, container_port).await?;
// Wait for it to actually answer, and learn the entry URL while we're
// there — see `probe_entry_path` for why that matters. This, not the
// launcher's stdout, is the readiness signal: verified that the
// "Listening on …" line is printed only on the very first start.
let entry_path = match wait_until_ready(&container_id, container_port).await {
Ok(path) => path,
Err(e) => {
let log = read_viewer_log(&container_id).await;
let _ = kill_dashboard(&container_id, &cli_entry).await;
return Err(explain_start_failure(&e, &log));
}
};
let (container_port, entry_path) = start_viewer(&container_id, &cli_entry).await?;
let token = generate_token();
// `--host 127.0.0.1` is ours to set, so the family is known and there is
@@ -601,12 +587,91 @@ async fn read_viewer_log(container_id: &str) -> String {
// Readiness, ports, URLs
// ─────────────────────────────────────────────────────────────────────────────
/// First port in [`VIEWER_PORTS`] that nothing in the container is listening on.
async fn pick_viewer_port(container_id: &str) -> Result<u16, String> {
/// How many free ports a start will try before giving up.
///
/// More than one because port choice is a check-then-bind: the free list comes
/// from a snapshot of the container's `/proc/net/tcp`, and anything in the
/// container may bind the port we picked before the dashboard gets to it. One
/// retry per lost race is the recovery; the cap is what stops a container that
/// binds every candidate from holding a start open for
/// `MAX_PORT_ATTEMPTS × READY_TIMEOUT`.
const MAX_PORT_ATTEMPTS: usize = 3;
/// Get a viewer listening inside the container and return the port it is on
/// plus the path the pane should load.
///
/// ## The check/bind race
///
/// [`pick_viewer_port`] reads a *snapshot* of container listeners; the dashboard
/// binds some milliseconds later. Nothing here can make that atomic — the bind
/// happens in another process, in another namespace, and `playwright-cli show`
/// reports the port it actually took only on a first-ever start (see
/// [`wait_until_ready`]). What is possible is to stop treating the first
/// candidate as the only one: if the port we picked does not come up, walk to
/// the next free candidate rather than failing the whole start.
///
/// Residual, stated rather than glossed: a container-side process that binds the
/// candidate port *and answers HTTP* is indistinguishable from the dashboard at
/// this layer, and the pane would then front it. What contains that is
/// downstream — the host proxy is loopback-only and token-gated, and the pane's
/// iframe is sandboxed — not this function.
async fn start_viewer(container_id: &str, cli_entry: &str) -> Result<(u16, String), String> {
let mut tried: Vec<u16> = Vec::new();
let mut last: Option<String> = None;
for _ in 0..MAX_PORT_ATTEMPTS {
// Re-read the listener snapshot each attempt: the port that was free a
// moment ago is exactly the one we may have just lost.
let port = match pick_viewer_port(container_id, &tried).await {
Ok(p) => p,
Err(e) => {
// Report why the *attempts* failed, not just "nothing free":
// the exhausted range is the symptom, the last start failure is
// the thing the user can act on.
return Err(match last {
Some(prev) => format!("{} ({})", e, prev),
None => e,
});
}
};
tried.push(port);
launch_viewer(container_id, cli_entry, port).await?;
// Wait for it to actually answer, and learn the entry URL while we're
// there — see `probe_entry_path` for why that matters. This, not the
// launcher's stdout, is the readiness signal: verified that the
// "Listening on …" line is printed only on the very first start.
match wait_until_ready(container_id, port).await {
Ok(path) => return Ok((port, path)),
Err(e) => {
let log = read_viewer_log(container_id).await;
// Always kill before retrying: the dashboard is a singleton, so
// a launcher that came up on some *other* port would otherwise
// make every further attempt a no-op that silently ignores the
// port we asked for.
let _ = kill_dashboard(container_id, cli_entry).await;
last = Some(explain_start_failure(&e, &log));
}
}
}
Err(last.unwrap_or_else(|| "The Playwright viewer did not start.".to_string()))
}
/// First port in [`VIEWER_PORTS`] that nothing in the container is listening on
/// and that this start has not already tried.
async fn pick_viewer_port(container_id: &str, tried: &[u16]) -> Result<u16, String> {
let text = exec_oneshot(
container_id,
vec![
"cat".to_string(),
// Absolute path, deliberately, for the same reason the auth bridge
// uses one: `container/Dockerfile` puts a container-writable
// directory first on `PATH`, so a bare `cat` is a name the container
// can rebind to a shim. A shimmed listener list is a shimmed answer
// to "which port is free" — i.e. the container choosing which port
// the viewer, and therefore the host-side proxy, ends up on.
"/usr/bin/cat".to_string(),
"/proc/net/tcp".to_string(),
"/proc/net/tcp6".to_string(),
],
@@ -616,7 +681,7 @@ async fn pick_viewer_port(container_id: &str) -> Result<u16, String> {
let taken = proc_net::parse_loopback_listeners(&text);
VIEWER_PORTS
.clone()
.find(|p| !taken.contains_key(p))
.find(|p| !taken.contains_key(p) && !tried.contains(p))
.ok_or_else(|| {
format!(
"No free port in {}{} inside the container for the Playwright viewer.",
+71 -4
View File
@@ -3,11 +3,78 @@
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">
<!--
This page is served by an axum server bound 0.0.0.0 (remote access is the
feature) behind a permissive CORS layer, and it fronts a shell in a container.
It gets a CSP of its own because nothing else gives it one: the app's
`tauri.conf.json` CSP covers the desktop webview, never this document.
`default-src 'none'` is the base, so anything not named below is refused
outright. What is named:
script-src jsdelivr for the three xterm bundles, plus 'unsafe-inline' for
this page's own inline <script>. Nonces/hashes were considered
and rejected: the file is a static `include_str!()` asset, so a
hash would have to be recomputed by hand on every edit to the
script, and the failure mode of getting that wrong is a terminal
that silently will not start.
style-src the xterm stylesheet, this page's <style>, and the one inline
`style=` attribute below (style attributes need 'unsafe-inline').
connect-src the WebSocket back to this same server. `ws:`/`wss:` as schemes
rather than an origin, because the host and port are whatever
the user reached this page on and are not knowable at build time.
form-action / base-uri / object-src / frame-ancestors — all 'none'. Note
`frame-ancestors` is ignored in a <meta> CSP; it is here as a
statement of intent, and the real protection would be a response
header from `server.rs`.
Deliberately absent: 'unsafe-eval', and any origin other than jsdelivr.
-->
<meta http-equiv="Content-Security-Policy" content="
default-src 'none';
script-src 'unsafe-inline' https://cdn.jsdelivr.net;
style-src 'unsafe-inline' https://cdn.jsdelivr.net;
img-src 'self' data:;
font-src 'self' data:;
connect-src 'self' ws: wss:;
form-action 'none';
base-uri 'none';
object-src 'none';
frame-ancestors 'none';
">
<title>Triple-C Web Terminal</title>
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@xterm/xterm@5.5.0/css/xterm.min.css">
<script src="https://cdn.jsdelivr.net/npm/@xterm/xterm@5.5.0/lib/xterm.min.js"></script>
<script src="https://cdn.jsdelivr.net/npm/@xterm/addon-fit@0.10.0/lib/addon-fit.min.js"></script>
<script src="https://cdn.jsdelivr.net/npm/@xterm/addon-web-links@0.11.0/lib/addon-web-links.min.js"></script>
<!--
Subresource Integrity on every CDN asset.
Without it this page executes whatever jsdelivr returns, inside a document
that holds the web terminal's access token and drives a shell in a container —
an upstream compromise, a hijacked package version or a MITM on a phone's
network is arbitrary code with that reach. The hashes below were computed
from the exact bytes at these pinned versions. `crossorigin="anonymous"` is
required for SRI to be checked on a cross-origin fetch.
Bumping a version means recomputing its hash:
curl -sS <url> | openssl dgst -sha384 -binary | openssl base64 -A
A mismatched hash blocks the asset, so a stale hash shows up immediately as a
terminal that does not render — never as an unverified load.
These are still remote loads: the remote terminal does not work with no
internet on the client side, and vendoring the ~300 KB of minified xterm into
this file would fix that. It was not done here — SRI already closes the
integrity half, which is the security half, and the availability half is a
separate call about binary size and diff readability.
-->
<link rel="stylesheet"
href="https://cdn.jsdelivr.net/npm/@xterm/xterm@5.5.0/css/xterm.min.css"
integrity="sha384-tStR1zLfWgsiXCF3IgfB3lBa8KmBe/lG287CL9WCeKgQYcp1bjb4/+mwN6oti4Co"
crossorigin="anonymous">
<script src="https://cdn.jsdelivr.net/npm/@xterm/xterm@5.5.0/lib/xterm.min.js"
integrity="sha384-J4qzUjBl1FxyLsl/kQPQIOeINsmp17OHYXDOMpMxlKX53ZfYsL+aWHpgArvOuof9"
crossorigin="anonymous"></script>
<script src="https://cdn.jsdelivr.net/npm/@xterm/addon-fit@0.10.0/lib/addon-fit.min.js"
integrity="sha384-XGqKrV8Jrukp1NITJbOEHwg01tNkuXr6uB6YEj69ebpYU3v7FvoGgEg23C1Gcehk"
crossorigin="anonymous"></script>
<script src="https://cdn.jsdelivr.net/npm/@xterm/addon-web-links@0.11.0/lib/addon-web-links.min.js"
integrity="sha384-S1biLeI8L/bFduIVvCxbn/l4EtaG4nTqQjGF7qCYTbsGXGFe8KgIKXtw4+UWxprv"
crossorigin="anonymous"></script>
<style>
:root {
--bg-primary: #1a1b26;
+1 -1
View File
@@ -22,7 +22,7 @@
}
],
"security": {
"csp": "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' blob: asset: https://asset.localhost; font-src 'self' data:; connect-src 'self' ipc: http://ipc.localhost; frame-src http://127.0.0.1:47820 http://127.0.0.1:47821 http://127.0.0.1:47822 http://127.0.0.1:47823 http://127.0.0.1:47824 http://127.0.0.1:47825 http://127.0.0.1:47826 http://127.0.0.1:47827"
"csp": "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' blob:; font-src 'self'; connect-src 'self' ipc: http://ipc.localhost; frame-src http://127.0.0.1:47820 http://127.0.0.1:47821 http://127.0.0.1:47822 http://127.0.0.1:47823 http://127.0.0.1:47824 http://127.0.0.1:47825 http://127.0.0.1:47826 http://127.0.0.1:47827; form-action 'none'; base-uri 'none'; object-src 'none'"
}
},
"bundle": {
@@ -0,0 +1,100 @@
import { describe, it, expect } from "vitest";
import { renderMarkdown } from "./HelpDialog";
/**
* `renderMarkdown` builds HTML by regex substitution and the result is handed
* to `dangerouslySetInnerHTML`. Its input is the help document, which is
* fetched from GitHub at runtime — remote, versioned by someone else, and not
* something the app gets to trust. These tests pin the escaping.
*/
/**
* Parse rendered HTML and return its first anchor, asserting that *no* element
* anywhere in the output grew an attribute outside the allowed set. A broken
* attribute value is only interesting if it becomes an attribute, so the check
* has to run through a real parser rather than over the string.
*/
const ALLOWED_ATTRS = new Set(["class", "href", "target", "rel", "id"]);
function onlyAnchor(html: string): HTMLAnchorElement {
const doc = new DOMParser().parseFromString(html, "text/html");
for (const el of Array.from(doc.body.querySelectorAll("*"))) {
for (const name of attrNames(el)) {
expect(ALLOWED_ATTRS.has(name), `unexpected attribute ${name}`).toBe(true);
}
}
const anchors = doc.querySelectorAll("a");
expect(anchors.length).toBeGreaterThan(0);
return anchors[0] as HTMLAnchorElement;
}
/** Attribute names the parser actually saw on an element. */
function attrNames(el: Element): string[] {
return Array.from(el.attributes).map((a) => a.name);
}
describe("renderMarkdown escaping", () => {
it("escapes the quote characters an attribute value is delimited by", () => {
const html = renderMarkdown('He said "hi" and it\'s fine.');
expect(html).not.toMatch(/said "hi"/);
expect(html).toContain("&quot;hi&quot;");
expect(html).toContain("it&#39;s");
});
it("does not let a link target break out of href=\"…\"", () => {
// The sink: the URL capture is `[^)]+`, which includes `"` and spaces, and
// the value lands directly inside `href="…"`. Asserted through the DOM,
// not by string matching — the payload text legitimately survives *inside*
// the attribute value; what must not happen is it becoming an attribute.
const a = onlyAnchor(
renderMarkdown(
'[click](https://example.com/" onmouseover="steal() formaction="https://evil.example)',
),
);
expect(attrNames(a)).toEqual(["class", "href", "target", "rel"]);
expect(a.getAttribute("href")).toContain('" onmouseover="');
});
it("does not let an in-document anchor break out of href=\"#…\"", () => {
const a = onlyAnchor(
renderMarkdown('[jump](#top" onfocus="steal() autofocus="x)'),
);
expect(attrNames(a)).toEqual(["class", "href"]);
});
it("does not let a bare URL break out of href=\"…\"", () => {
const a = onlyAnchor(
renderMarkdown('See https://example.com/a"onmouseover="steal()\n'),
);
expect(attrNames(a)).toEqual(["class", "href", "target", "rel"]);
});
it("still renders ordinary links intact", () => {
const html = renderMarkdown("[docs](https://example.com/a?x=1&y=2)");
// `&` was entity-escaped by the first pass and must not be escaped twice.
expect(html).toContain('href="https://example.com/a?x=1&amp;y=2"');
expect(html).not.toContain("&amp;amp;");
expect(html).toContain('target="_blank"');
expect(html).toContain('rel="noopener noreferrer"');
expect(html).toContain(">docs</a>");
});
it("still renders an in-document anchor link intact", () => {
const html = renderMarkdown("[jump](#getting-started)");
expect(html).toContain('href="#getting-started"');
});
it("keeps header slugs stable across the new quote escaping", () => {
// The regression this guards: quotes now become entities *before*
// `slugify` sees them, and an entity's letters would otherwise survive
// into the id ("claude39s-setup"), silently breaking every
// `[…](#claudes-setup)` in the document.
expect(renderMarkdown("## Claude's setup")).toContain('id="claudes-setup"');
expect(renderMarkdown('## The "safe" mode')).toContain('id="the-safe-mode"');
});
it("still refuses to emit raw tags from the source document", () => {
const html = renderMarkdown("<img src=x onerror=alert(1)>");
expect(html).not.toContain("<img");
expect(html).toContain("&lt;img");
});
});
+56 -7
View File
@@ -12,21 +12,67 @@ function slugify(text: string): string {
return text
.toLowerCase()
.replace(/<[^>]+>/g, "") // strip HTML tags (e.g. from inline code)
// Quote characters are escaped to entities before this runs (see
// `renderMarkdown`). Drop those two entities whole, so a header with an
// apostrophe or a quote slugifies to what it did when the character was
// simply stripped — otherwise every such anchor id silently changes and
// the in-document links pointing at it stop resolving. `&amp;`/`&lt;`/
// `&gt;` are deliberately not in this list: they were already entities
// before, so their existing (odd) slugs are the established ones.
.replace(/&quot;|&#39;/g, "")
.replace(/[^\w\s-]/g, "") // remove non-word chars except spaces/dashes
.replace(/\s+/g, "-") // spaces to dashes
.replace(/-+/g, "-") // collapse consecutive dashes
.replace(/^-|-$/g, ""); // trim leading/trailing dashes
}
/** Simple markdown-to-HTML converter for the help content. */
function renderMarkdown(md: string): string {
/**
* Escape a captured markdown value that is about to be interpolated into an
* HTML *attribute* value.
*
* `renderMarkdown` entity-escapes the whole document first, but that pass only
* covered `&`, `<` and `>` — not the quote characters, which is all an
* attribute value is delimited by. `[x](https://a" onload="…)` therefore closed
* `href="` and started a new attribute, because the URL capture is `[^)]+` and
* `"` is in `[^)]`. The document is remote GitHub markdown, so that capture is
* not ours to trust.
*
* Only quotes are escaped here: `&`, `<` and `>` have already been converted by
* the caller, and re-escaping the `&` would double-encode every `&amp;` in a
* query string.
*/
function attr(value: string): string {
return value.replace(/"/g, "&quot;").replace(/'/g, "&#39;");
}
/**
* Simple markdown-to-HTML converter for the help content.
*
* Exported for `HelpDialog.test.tsx`: the output goes to
* `dangerouslySetInnerHTML`, so the escaping rules below are security rules and
* need to be asserted rather than assumed.
*/
export function renderMarkdown(md: string): string {
let html = md;
// Normalize line endings
html = html.replace(/\r\n/g, "\n");
// Escape HTML entities (but we'll re-introduce tags below)
html = html.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;");
// Escape HTML entities (but we'll re-introduce tags below).
//
// The quote characters are part of this on purpose. Everything below builds
// HTML by regex substitution, and several of those substitutions drop a
// capture straight into an attribute value (`href="$2"`). Leaving `"` and `'`
// live meant a link target could close the attribute and open another one —
// in a document fetched from GitHub at runtime and handed to
// `dangerouslySetInnerHTML`. Escaping here closes every such sink at the
// source; `attr()` below is the belt to this pair of braces.
html = html
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;")
.replace(/'/g, "&#39;");
// Fenced code blocks (```...```)
html = html.replace(/```(\w*)\n([\s\S]*?)```/g, (_m, _lang, code) => {
@@ -84,13 +130,15 @@ function renderMarkdown(md: string): string {
// Markdown-style anchor links [text](#anchor)
html = html.replace(
/\[([^\]]+)\]\(#([^)]+)\)/g,
'<a class="help-link" href="#$2">$1</a>',
(_m, text: string, anchor: string) =>
`<a class="help-link" href="#${attr(anchor)}">${text}</a>`,
);
// Markdown-style external links [text](url)
html = html.replace(
/\[([^\]]+)\]\((https?:\/\/[^)]+)\)/g,
'<a class="help-link" href="$2" target="_blank" rel="noopener noreferrer">$1</a>',
(_m, text: string, url: string) =>
`<a class="help-link" href="${attr(url)}" target="_blank" rel="noopener noreferrer">${text}</a>`,
);
// Unordered list items (- ...)
@@ -117,7 +165,8 @@ function renderMarkdown(md: string): string {
// Links - convert bare URLs to clickable links (skip already-wrapped URLs)
html = html.replace(
/(?<!="|'>)(https?:\/\/[^\s<)]+)/g,
'<a class="help-link" href="$1" target="_blank" rel="noopener noreferrer">$1</a>',
(_m, url: string) =>
`<a class="help-link" href="${attr(url)}" target="_blank" rel="noopener noreferrer">${url}</a>`,
);
// Wrap remaining loose text lines in paragraphs
@@ -457,6 +457,35 @@ export default function BrowserTab({ project, active }: Props) {
// host-side gate checks before anything reaches the container.
src={status.url ?? undefined}
title={`Playwright browser view for ${project.name}`}
// What is framed here is served by a process inside the container,
// which is the untrusted side of this app. Unsandboxed, it could
// simply set `top.location` and navigate the *app's* webview
// somewhere of its choosing — the frame is cross-origin, so it cannot
// read the app, but steering the whole window is not something a
// viewer pane should be able to do.
//
// The allowances are what the Playwright dashboard actually needs and
// no more:
// allow-scripts — it is an application, not a document.
// allow-same-origin — it must reach its own WebSocket and assets,
// and the host-side gate recognises the pane's
// own sub-resource requests by their
// `Origin`/`Referer`; an opaque origin would
// send `null` and be refused. This does not
// grant access to *this* app: 127.0.0.1:4782x
// is a different origin from the app's.
// allow-forms/-modals/-downloads/-popups — dashboard UI affordances
// (trace download, confirm dialogs, opening a
// page in a new window).
//
// Deliberately absent, and the point of the attribute:
// `allow-top-navigation`, `allow-top-navigation-by-user-activation`
// and `allow-popups-to-escape-sandbox`. Do not add them.
//
// No `referrerPolicy` either: the gate in `browser_view/proxy.rs`
// reads the token out of a same-origin `Referer`, so stripping it
// would break the pane.
sandbox="allow-scripts allow-same-origin allow-forms allow-modals allow-downloads allow-popups"
className="flex-1 min-h-0 w-full border-0 bg-[var(--bg-primary)]"
/>
) : live ? (