From 092972fe92df7c394805342d77e3b56f5f8c4ba8 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 23 Aug 2026 11:13:15 -0700 Subject: [PATCH] security: close capability, CSP and auth-bridge holes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit capabilities/default.json - Drop every `store:*` grant. `@tauri-apps/plugin-store` has no caller in `app/src`, and the plugin's `resolve_store_path` is a `PathBuf::push` against AppData — `push` discards the base for an absolute path, so the grant was an arbitrary host read/write from the webview. - Replace `opener:default` with a scoped `opener:allow-open-url` (http/https only). That drops `reveal_item_in_dir`, which the plugin does not scope-check and nothing here calls, and the unused mailto:/tel: scope. - Record the unscopable `drag:allow-start-drag` residual risk in `description`. tauri.conf.json - Add `form-action 'none'`, `base-uri 'none'`, `object-src 'none'`. `form-action` has no `default-src` fallback, so an injected auto-submitting form was unblocked even though `script-src 'self'` blocks XSS. - Remove the dead `asset:` / `https://asset.localhost` img-src and `data:` font-src grants; `blob:` stays (the file viewer uses it). auth_bridge - The reserved-port set covered only this project's mappings and the two browser-view ranges. It now also covers the gateway, STT and web-terminal host ports (configured value and shipped default, read off the settings models) and every other project's published host port. A container binding container-loopback 4000 / 9876 / 7681 while those services were stopped had that port mirrored onto the host, unauthenticated, within one poll. - Gate the host listener on fetch metadata: refuse a request that is a cross-site sub-resource, allow navigations (the OAuth redirect) and anything without `Sec-Fetch-*`. Non-HTTP connections are classified from their first line and forwarded verbatim. Residual risk is spelled out in the module docs. - Bound the forwards: max concurrent connections per port, a first-byte deadline enforced before any `docker exec` is created, and an idle timeout. browser_view/mod.rs - `pick_viewer_port` reads procfs with `/usr/bin/cat`, not a bare `cat` the container can shim via its writable PATH entry. - Treat port choice as check-then-bind: walk to the next free candidate when the viewer does not come up, instead of failing the start. BrowserTab.tsx - Sandbox the viewer iframe. Container-controlled content could `top.location` the app's webview away. `allow-top-navigation*` and `allow-popups-to-escape-sandbox` are deliberately absent. HelpDialog.tsx - Escape the quote characters in the entity pass and escape captured attribute values. `href="$2"` with `$2` = `[^)]+` let remote GitHub markdown close the attribute and open another, in a document rendered with `dangerouslySetInnerHTML`. web_terminal/terminal.html - SRI hashes plus `crossorigin` on the three jsdelivr bundles and the stylesheet, and a CSP for the page — it is served 0.0.0.0 behind a permissive CORS layer and nothing else gives it one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc --- app/src-tauri/capabilities/default.json | 20 +- app/src-tauri/gen/schemas/capabilities.json | 2 +- app/src-tauri/src/auth_bridge/mod.rs | 193 +++++- app/src-tauri/src/auth_bridge/tunnel.rs | 551 +++++++++++++++++- app/src-tauri/src/browser_view/mod.rs | 103 +++- app/src-tauri/src/web_terminal/terminal.html | 75 ++- app/src-tauri/tauri.conf.json | 2 +- app/src/components/layout/HelpDialog.test.tsx | 100 ++++ app/src/components/layout/HelpDialog.tsx | 63 +- .../components/projects/home/BrowserTab.tsx | 29 + 10 files changed, 1066 insertions(+), 72 deletions(-) create mode 100644 app/src/components/layout/HelpDialog.test.tsx diff --git a/app/src-tauri/capabilities/default.json b/app/src-tauri/capabilities/default.json index 702d43d..a5317e6 100644 --- a/app/src-tauri/capabilities/default.json +++ b/app/src-tauri/capabilities/default.json @@ -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" ] diff --git a/app/src-tauri/gen/schemas/capabilities.json b/app/src-tauri/gen/schemas/capabilities.json index a8b1260..f058044 100644 --- a/app/src-tauri/gen/schemas/capabilities.json +++ b/app/src-tauri/gen/schemas/capabilities.json @@ -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"]}} \ No newline at end of file +{"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"]}} \ No newline at end of file diff --git a/app/src-tauri/src/auth_bridge/mod.rs b/app/src-tauri/src/auth_bridge/mod.rs index bc2b7e0..7473cbc 100644 --- a/app/src-tauri/src/auth_bridge/mod.rs +++ b/app/src-tauri/src/auth_bridge/mod.rs @@ -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 { +/// 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 { let mut skip: HashSet = 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::() + .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 { + 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 { + 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 = 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)); } diff --git a/app/src-tauri/src/auth_bridge/tunnel.rs b/app/src-tauri/src/auth_bridge/tunnel.rs index 3882597..60c8ff3 100644 --- a/app/src-tauri/src/auth_bridge/tunnel.rs +++ b/app/src-tauri/src/auth_bridge/tunnel.rs @@ -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:` 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`, ``, ` - - + + + + +