From 45f9728faca82dfad23b64d43f4a60215cd8eee3 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 11 Sep 2026 09:54:49 +0800 Subject: [PATCH] Allow LiveKit regional CSP hosts LiveKit Cloud can redirect clients to regional signaling hosts. Allow its Cloud wildcard beside exact HTTP and WebSocket origins. Reject URL spellings unsafe for CSP sources. --- src/config.rs | 34 +++++++++++++++++----- src/livekit.rs | 9 +++--- src/web/policy.rs | 58 +++++++++++++++++++++++++------------- tests/config.rs | 12 ++++++++ tests/unit/web/policy.rs | 61 ++++++++++++++++++++++++---------------- tests/web.rs | 27 ++++++++++++++++++ 6 files changed, 145 insertions(+), 56 deletions(-) diff --git a/src/config.rs b/src/config.rs index 14146c9..0afd54e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -506,23 +506,43 @@ pub(crate) fn livekit_scheme(url: &str) -> Option<&'static (&'static str, bool, }) } -/// Rejects what `url_origin` in `web.rs` would otherwise have to cope with: a -/// scheme nothing here knows, or a scheme with no host after it. The URL never -/// appears in the message, because the message reaches stderr and a LiveKit URL -/// can carry a query string. +pub(crate) fn livekit_host_is_csp_safe(authority: &str) -> bool { + let Some((host, port)) = split_authority(authority) else { + return false; + }; + let host_is_safe = if authority.starts_with('[') { + host.parse::().is_ok() + } else { + !host.is_empty() + && host + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'.' | b'-')) + }; + host_is_safe && port.is_none_or(|port| port.parse::().is_ok_and(|port| port != 0)) +} + +/// Rejects what the URL's consumers would otherwise have to cope with: an +/// unknown scheme, no host, or host syntax that cannot become a CSP source. +/// The URL never appears in the message, because the message reaches stderr +/// and a LiveKit URL can carry a query string. pub fn validate_livekit_url(url: &str, production: bool) -> Result<(), String> { let trimmed = url.trim(); let Some((scheme, encrypted, _)) = livekit_scheme(trimmed) else { return Err("LIVEKIT_URL must start with wss://, https://, ws:// or http://".to_string()); }; - if trimmed[scheme.len()..] + let host = trimmed[scheme.len()..] .split(['/', '?', '#']) .next() .unwrap_or_default() - .is_empty() - { + .rsplit('@') + .next() + .unwrap_or_default(); + if host.is_empty() { return Err("LIVEKIT_URL has a scheme but no host".to_string()); } + if !livekit_host_is_csp_safe(host) { + return Err("LIVEKIT_URL host contains invalid characters".to_string()); + } if production && !encrypted { return Err("LIVEKIT_URL must use wss:// or https:// when NODE_ENV=production".to_string()); } diff --git a/src/livekit.rs b/src/livekit.rs index 43b4fef..ce9ff75 100644 --- a/src/livekit.rs +++ b/src/livekit.rs @@ -1863,16 +1863,15 @@ async fn on_interruption( // What Gemini heard is the whole diagnosis. It interrupts on its own voice // activity detection, so a cut with the candidate mid-sentence is barge-in - // working, and a cut with nothing transcribed is the microphone hearing the - // interviewer through the candidate's speakers. The line reported the size - // of the loss and left the cause to guesswork across a whole session of - // them. + // working. A cut with nothing transcribed is usually speaker echo or room + // noise, and naming those makes the log actionable without pretending the + // server can tell them apart. let heard = context.turns.candidate.tail(80); eprintln!( "timing: Gemini cut its own turn, {:.1}s of it unplayed; candidate audio so far: {}", unplayed.as_secs_f64(), if heard.is_empty() { - "(nothing transcribed)" + "(nothing transcribed; check speaker echo or background noise)" } else { heard } diff --git a/src/web/policy.rs b/src/web/policy.rs index 882fb52..4a69635 100644 --- a/src/web/policy.rs +++ b/src/web/policy.rs @@ -81,20 +81,14 @@ pub(crate) fn content_security_policy(config: &WebServerConfig) -> String { // LiveKit Cloud answers `/settings/regions` with a regional hostname and // the SDK retries there, so the configured host alone is not enough. - // Self-hosted deployments have no such indirection and keep their exact - // origins. - // - // One pass, so a third source of endpoints is added to the iterator above - // and both the exact origins and the wildcard follow from it. The wildcard - // names the domain it was granted for, because a deployment on one cloud - // domain has no reason to reach the other. - let mut cloud: Vec<&'static str> = Vec::new(); + // Keeping the wildcard beside the endpoint's two exact origins means a + // future endpoint source cannot add only the initial signaling host and + // bring this browser-only failure back. for url in endpoints { - connect.extend(livekit_origins(url)); - if let Some(domain) = livekit_cloud_domain(url).filter(|it| !cloud.contains(it)) { - cloud.push(domain); - connect.push(format!("https://*.{domain}")); - connect.push(format!("wss://*.{domain}")); + for source in livekit_connect_sources(url) { + if !connect.contains(&source) { + connect.push(source); + } } } if !config.production { @@ -141,10 +135,7 @@ pub(crate) fn content_security_policy(config: &WebServerConfig) -> String { /// HTTPS calls to the same host, region settings among them. Naming only the /// `wss://` origin lets the socket open and then blocks those, which surfaces /// as a connection that fails for no stated reason. -pub(crate) fn livekit_origins(url: &str) -> Vec { - let Some(origin) = url_origin(url) else { - return Vec::new(); - }; +fn livekit_origins_for_csp_origin(origin: &str) -> Vec { let Some((scheme, host)) = origin.split_once("://") else { return Vec::new(); }; @@ -153,9 +144,36 @@ pub(crate) fn livekit_origins(url: &str) -> Vec { "ws" => "http", "https" => "wss", "http" => "ws", - _ => return vec![origin], + _ => return Vec::new(), }; - vec![format!("{sibling}://{host}"), origin] + vec![format!("{sibling}://{host}"), origin.to_string()] +} + +/// The origin spelling CSP accepts. A LiveKit URL may carry userinfo for a +/// server-side client, but credentials are not part of a CSP host source and +/// would make the browser discard the source that needs to admit the socket. +fn csp_origin(url: &str) -> Option { + let origin = url_origin(url)?; + crate::config::livekit_scheme(&origin)?; + let (scheme, authority) = origin.split_once("://")?; + let host = authority.rsplit('@').next()?; + crate::config::livekit_host_is_csp_safe(host).then(|| format!("{scheme}://{host}")) +} + +/// Every CSP source a LiveKit endpoint needs. Cloud projects redirect the +/// browser to a regional hostname, while a self-hosted project has only the +/// exact HTTP and WebSocket origins from the CSP-safe endpoint. +fn livekit_connect_sources(url: &str) -> Vec { + let Some(origin) = csp_origin(url) else { + return Vec::new(); + }; + + let mut sources = livekit_origins_for_csp_origin(&origin); + if let Some(domain) = livekit_cloud_domain(&origin) { + sources.push(format!("https://*.{domain}")); + sources.push(format!("wss://*.{domain}")); + } + sources } /// The two domains `livekit-client.js` treats as LiveKit Cloud, which is what @@ -207,7 +225,7 @@ pub(crate) fn livekit_http_origin(url: &str) -> Option { /// configure and the server will start on. Every reader below then matches the /// scheme exactly, and each one fails differently on the uppercase form: the /// quota probe finds no HTTP origin and treats the project as available, so an -/// exhausted project is never passed over, and `livekit_origins` finds no +/// exhausted project is never passed over, and the CSP origin pairing finds no /// sibling, so the CSP omits the `https://` origin the SDK needs for /// `/settings/regions` and the socket opens onto blocked requests. One /// normalization at the boundary is the alternative to three readers each diff --git a/tests/config.rs b/tests/config.rs index eabbf0d..73f68f5 100644 --- a/tests/config.rs +++ b/tests/config.rs @@ -737,12 +737,24 @@ fn plaintext_livekit_is_local_only() { for url in [ "wss://example.livekit.cloud", "https://example.livekit.cloud", + "wss://example.livekit.cloud:443", + "wss://[2001:db8::1]:7880", ] { assert!(codetrial::config::validate_livekit_url(url, true).is_ok()); } // Prefix matching alone let all of these through. for url in [ "wss://", + "wss://@", + "wss://project.example;script-src=*", + "wss://project.example'", + "wss://project.example,evil.example", + "wss://project.example*", + "wss://project.example:notaport", + "wss://project.example:0", + "wss://project.example:65536", + "wss://[2001:db8::1", + "wss://[not-ipv6]", "https://", "example.livekit.cloud", "ftp://x.example", diff --git a/tests/unit/web/policy.rs b/tests/unit/web/policy.rs index e2c5f10..30666e2 100644 --- a/tests/unit/web/policy.rs +++ b/tests/unit/web/policy.rs @@ -56,19 +56,31 @@ fn the_policy_names_the_host_the_avatar_model_is_fetched_from() { assert!(connect.contains(AVATAR_MODEL_ORIGIN), "{connect}"); } -/// both fail over to a regional host and both need the wildcard. Each gets -/// its own: a deployment on one has no reason to reach the other. +/// Both cloud domains fail over to regional hosts and need their own wildcard. +/// A deployment on one has no reason to reach the other. #[test] -fn each_cloud_domain_gets_only_its_own_wildcard() { - let cloud = policy_for("wss://example.livekit.cloud"); - assert!(cloud.contains("https://*.livekit.cloud"), "{cloud}"); - assert!(cloud.contains("wss://*.livekit.cloud"), "{cloud}"); - assert!(!cloud.contains("*.livekit.run"), "{cloud}"); +fn cloud_domains_admit_regional_hosts_without_widening_to_each_other() { + for (url, domain, other_domain) in [ + ( + "wss://example.livekit.cloud", + "livekit.cloud", + "livekit.run", + ), + ("wss://example.livekit.run", "livekit.run", "livekit.cloud"), + ] { + let policy = policy_for(url); + assert!(policy.contains(&format!("https://*.{domain}")), "{policy}"); + assert!(policy.contains(&format!("wss://*.{domain}")), "{policy}"); + assert!(!policy.contains(&format!("*.{other_domain}")), "{policy}"); + } - let run = policy_for("wss://example.livekit.run"); - assert!(run.contains("https://*.livekit.run"), "{run}"); - assert!(run.contains("wss://*.livekit.run"), "{run}"); - assert!(!run.contains("*.livekit.cloud"), "{run}"); + // Regional signaling hosts have labels below both the project and cloud + // domains. The configured project's wildcard must therefore cover this + // shape, not merely a sibling project directly under `livekit.cloud`. + assert_eq!( + livekit_cloud_domain("wss://conversation-xxx.otokyo1b.production.livekit.cloud"), + Some("livekit.cloud") + ); } /// A self-hosted server has no region indirection, so widening its policy @@ -132,6 +144,17 @@ fn a_malformed_url_is_refused_one_term_at_a_time() { ); } +/// A server-side URL may need userinfo, but a CSP source is only a scheme and +/// host. Leaving the userinfo in makes the browser discard the entry, which is +/// a silent connection failure for a self-hosted deployment. +#[test] +fn csp_origins_strip_userinfo() { + let policy = policy_for("wss://key:secret@project.example:7880"); + assert!(policy.contains("https://project.example:7880"), "{policy}"); + assert!(policy.contains("wss://project.example:7880"), "{policy}"); + assert!(!policy.contains("key:secret"), "{policy}"); +} + /// `validate_livekit_url` accepts the scheme case-insensitively, so an /// uppercase one is a URL the server starts on. Every reader here matches /// the scheme exactly, and each fails differently: the quota probe finds no @@ -149,14 +172,6 @@ fn an_uppercase_scheme_is_normalized_before_anything_matches_on_it() { Some("https://host.example".to_string()), "without this the quota probe never runs and the project is assumed available" ); - assert_eq!( - livekit_origins("WSS://host.example"), - vec![ - "https://host.example".to_string(), - "wss://host.example".to_string() - ], - "the sibling origin is what the SDK reaches for regions" - ); // All four schemes, not just the one a cloud deployment uses. A self-hosted // LiveKit is reached over `ws://` in development, and the arm that pairs it @@ -168,11 +183,9 @@ fn an_uppercase_scheme_is_normalized_before_anything_matches_on_it() { ("https://host.example", "wss://host.example"), ("http://host.example", "ws://host.example"), ] { - assert_eq!( - livekit_origins(configured), - vec![sibling.to_string(), configured.to_string()], - "{configured} must also name {sibling}" - ); + let policy = policy_for(configured); + assert!(policy.contains(sibling), "{configured}: {policy}"); + assert!(policy.contains(configured), "{configured}: {policy}"); } let policy = policy_for("WSS://uppercase-scheme.livekit.cloud"); diff --git a/tests/web.rs b/tests/web.rs index 537f29e..df837be 100644 --- a/tests/web.rs +++ b/tests/web.rs @@ -677,6 +677,33 @@ async fn responses_carry_baseline_security_headers() { server.abort(); } +/// `web_service` is public, so its caller may not have used the binary's +/// configuration loader. Invalid endpoints must cost only their CSP sources. +#[tokio::test] +async fn public_web_service_omits_malformed_livekit_origins() { + for (url, forbidden) in [ + ("wss://project.example;frame-src=*", "project.example"), + ("wss://project.example:notaport", "project.example"), + ("wss://[2001:db8::1", "2001:db8::1"), + ("ftp://project.livekit.cloud", "livekit.cloud"), + ] { + let mut config = web_config(); + config.pool = primary_pool(url, "devkey", "devsecret"); + let (base, server) = spawn_web_server(config).await; + let policy = reqwest::get(&base) + .await + .unwrap() + .headers() + .get("content-security-policy") + .unwrap() + .to_str() + .unwrap() + .to_string(); + assert!(!policy.contains(forbidden), "{url}: {policy}"); + server.abort(); + } +} + #[tokio::test] async fn production_policy_names_no_loopback_origins() { let (base, server) = spawn_web_server(WebServerConfig {