From 92682d06e50f9723dfa048b9388b19bf5756f52f Mon Sep 17 00:00:00 2001 From: Leigh Stillard Date: Thu, 21 May 2026 08:03:20 +1000 Subject: [PATCH] fix(security): replace wildcard CORS on core RPC with origin allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Replaces `Access-Control-Allow-Origin: *` on the in-process JSON-RPC server with an explicit allowlist tied to the origins we actually ship. Possession of the per-launch bearer token is no longer sufficient for an arbitrary browser origin to call `/rpc`. Closes #2262. ## What changes `src/core/jsonrpc.rs`: - New `is_origin_allowed(origin)` matches against: - Tauri v2 webview origins: `tauri://localhost`, `http://tauri.localhost`, `https://tauri.localhost` - Loopback hosts on any port: `http://127.0.0.1:*`, `http://localhost:*`, `http://[::1]:*` (Vite dev server, E2E harnesses) - Comma-separated env override `OPENHUMAN_CORE_ALLOWED_ORIGINS` for operator-controlled debug harnesses - `cors_middleware` reads the request's `Origin` header and passes it to `with_cors_headers`. - `with_cors_headers(response, origin)`: - Echoes the allowlisted origin in `Access-Control-Allow-Origin`. - Omits the header for disallowed origins (the browser then refuses to surface the response). - Always sets `Vary: Origin` so intermediate caches keep per-origin responses distinct. - Non-browser callers (no `Origin` header) are unaffected. - Logs a `tracing::warn!` line on rejected origins for diagnosis. `src/core/jsonrpc_cors_tests.rs` (new, wired via `#[path]` like `jsonrpc_tests.rs`): - Allowed Tauri origins return ACAO matching the request. - Loopback hosts (IPv4, IPv6 literal, named, with/without port) allowed. - Common disallowed origins (`https://attacker.example`, look-alike subdomains, `https://localhost`, `null`) refused with no ACAO header. - Missing `Origin` header → no ACAO, but `Vary: Origin` still set. - Env override allows exact-match additional origins; does not allow look-alike suffixes. - `Allow-Methods`, `Allow-Headers`, and `Max-Age` are always set. ## Why The bearer token is the only protection on `/rpc`. Possible token-leak paths include shared error logs, screenshots, Sentry breadcrumbs, or a malicious script inside a CEF child webview (Slack/Gmail/etc.). With wildcard CORS, possession of the token from *any* origin in any browser is sufficient — the desktop RPC surface includes shell tool calls, memory access, credential reads, and sending messages on the user's behalf, so the blast radius is high. ## Compatibility - Tauri webview (`coreRpcClient.ts` `fetch()` path): `tauri://localhost` / `http(s)://tauri.localhost` are allowlisted. No frontend change required. - `pnpm dev` (Vite at `http://localhost:1420`): allowlisted via the loopback rule. - E2E (`tauri-driver` on `:4444`, Appium): allowlisted via the loopback rule. - Non-browser callers (CLI, curl, integration tests that don't send `Origin`): unchanged. - Debug harnesses on non-loopback origins: set `OPENHUMAN_CORE_ALLOWED_ORIGINS=https://my-harness,...`. ## Test plan - [x] `cargo test --lib cors_tests` — 6 / 6 passing locally. - [x] `cargo check --lib` — clean. - [x] `cargo fmt --check` on changed files — clean. - [ ] Smoke `pnpm dev:app` on the Tauri shell and confirm the frontend RPC fetches still succeed. - [ ] CI: full test suite + coverage gate. ## Out of scope Tracked as follow-ups from the same security audit: - Webhook signature / replay verification. - OS-keychain backing for the encryption master key. - Prompt-injection enforcement (currently detect-only). - Dual-channel trust model for scraped third-party content vs. user instructions. ## Summary by CodeRabbit * **New Features** * Added CORS origin allowlisting with configurable environment variable support. * Only approved origins (Tauri, loopback, and configured) can access the API. * **Security** * Replaced permissive cross-origin access policy with restrictive allowlist that rejects unknown origins. * Disallowed origins no longer receive confirmation headers in responses. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2266?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) Co-authored-by: Leigh Stillard Co-authored-by: Steven Enamakel --- src/core/jsonrpc.rs | 97 +++++++++++++++++++++++-- src/core/jsonrpc_cors_tests.rs | 125 +++++++++++++++++++++++++++++++++ 2 files changed, 215 insertions(+), 7 deletions(-) create mode 100644 src/core/jsonrpc_cors_tests.rs diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index 442409290..bfc12a823 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -602,23 +602,102 @@ async fn http_request_log_middleware(req: Request, next: Next) -> Response { response } +/// Environment variable for additional comma-separated origins to allow. +/// Intended for debug harnesses and E2E setups that don't run on loopback — +/// e.g. `OPENHUMAN_CORE_ALLOWED_ORIGINS=https://e2e.internal,http://my-debugger:8080`. +const ALLOWED_ORIGINS_ENV: &str = "OPENHUMAN_CORE_ALLOWED_ORIGINS"; + +/// Decides whether a browser `Origin` header value is allowed to make +/// authenticated cross-origin requests against the local RPC server. +/// +/// The RPC server only ever serves three legitimate consumers: +/// 1. The bundled Tauri v2 webview — `tauri://localhost` on macOS/Linux and +/// `http(s)://tauri.localhost` on Windows. +/// 2. The Vite dev server during `pnpm dev` — any port on loopback hosts. +/// 3. Operator-controlled debug harnesses opted in via +/// `OPENHUMAN_CORE_ALLOWED_ORIGINS`. +/// +/// Anything else (a random web page that has somehow obtained the bearer +/// token via leaked logs / screenshots / a compromised third-party origin +/// loaded in a CEF child webview) must be refused — the bearer token alone +/// is not enough authorization without an origin binding. +pub(super) fn is_origin_allowed(origin: &str) -> bool { + // Tauri v2 webview origins. Windows uses an HTTP(S) custom host; macOS + // and Linux use the `tauri://` scheme. We accept both for portability. + if matches!( + origin, + "tauri://localhost" | "http://tauri.localhost" | "https://tauri.localhost" + ) { + return true; + } + + // Loopback origins on any port (Vite dev server, E2E driver, CLI tools). + if let Some(rest) = origin.strip_prefix("http://") { + let authority = rest.split('/').next().unwrap_or(""); + let host = if let Some(stripped) = authority.strip_prefix('[') { + // IPv6 literal: `[::1]:1420` → `::1` + stripped.split(']').next().unwrap_or("") + } else { + authority.split(':').next().unwrap_or("") + }; + if matches!(host, "127.0.0.1" | "localhost" | "::1") { + return true; + } + } + + // Env override: comma-separated exact matches. + if let Ok(extra) = std::env::var(ALLOWED_ORIGINS_ENV) { + for candidate in extra.split(',').map(str::trim).filter(|s| !s.is_empty()) { + if candidate == origin { + return true; + } + } + } + + false +} + /// Middleware for handling Cross-Origin Resource Sharing (CORS). +/// +/// Reads the request's `Origin` header before invoking the inner handler so +/// the same value can be echoed back (when allowed) on the response. async fn cors_middleware(req: Request, next: Next) -> Response { + let origin = req + .headers() + .get(header::ORIGIN) + .and_then(|v| v.to_str().ok()) + .map(str::to_owned); + if req.method() == Method::OPTIONS { - return with_cors_headers(StatusCode::NO_CONTENT.into_response()); + return with_cors_headers(StatusCode::NO_CONTENT.into_response(), origin.as_deref()); } let response = next.run(req).await; - with_cors_headers(response) + with_cors_headers(response, origin.as_deref()) } /// Injects CORS headers into a response. -fn with_cors_headers(mut response: Response) -> Response { +/// +/// If the request carried an `Origin` header and that origin is on the +/// allowlist, the value is echoed back in `Access-Control-Allow-Origin` and +/// `Vary: Origin` is set so intermediate caches keep per-origin responses +/// distinct. Disallowed origins receive no `Access-Control-Allow-Origin` +/// header at all — the browser will then refuse to surface the response to +/// the calling JS. Non-browser callers (no `Origin` header) are unaffected. +pub(super) fn with_cors_headers(mut response: Response, origin: Option<&str>) -> Response { let headers = response.headers_mut(); - headers.insert( - header::ACCESS_CONTROL_ALLOW_ORIGIN, - HeaderValue::from_static("*"), - ); + headers.insert(header::VARY, HeaderValue::from_static("Origin")); + + if let Some(o) = origin { + if is_origin_allowed(o) { + if let Ok(val) = HeaderValue::from_str(o) { + headers.insert(header::ACCESS_CONTROL_ALLOW_ORIGIN, val); + } + } else { + tracing::warn!("[cors] rejected disallowed origin: {}", o); + } + } + headers.insert( header::ACCESS_CONTROL_ALLOW_METHODS, HeaderValue::from_static("GET, POST, OPTIONS"), @@ -1508,3 +1587,7 @@ fn build_http_schema_dump() -> HttpSchemaDump { #[cfg(test)] #[path = "jsonrpc_tests.rs"] mod tests; + +#[cfg(test)] +#[path = "jsonrpc_cors_tests.rs"] +mod cors_tests; diff --git a/src/core/jsonrpc_cors_tests.rs b/src/core/jsonrpc_cors_tests.rs new file mode 100644 index 000000000..3454d9bda --- /dev/null +++ b/src/core/jsonrpc_cors_tests.rs @@ -0,0 +1,125 @@ +//! Unit tests for the CORS allowlist and header-emission logic in `jsonrpc.rs`. + +use axum::http::{header, StatusCode}; +use axum::response::{IntoResponse, Response}; + +use super::{is_origin_allowed, with_cors_headers, ALLOWED_ORIGINS_ENV}; + +fn ok_response() -> Response { + (StatusCode::OK, "").into_response() +} + +fn allow_origin(response: &Response) -> Option { + response + .headers() + .get(header::ACCESS_CONTROL_ALLOW_ORIGIN) + .and_then(|v| v.to_str().ok()) + .map(str::to_owned) +} + +#[test] +fn allows_tauri_webview_origins() { + for origin in [ + "tauri://localhost", + "http://tauri.localhost", + "https://tauri.localhost", + ] { + assert!(is_origin_allowed(origin), "expected {origin} to be allowed"); + let r = with_cors_headers(ok_response(), Some(origin)); + assert_eq!(allow_origin(&r).as_deref(), Some(origin)); + } +} + +#[test] +fn allows_loopback_with_any_port() { + for origin in [ + "http://127.0.0.1:1420", + "http://localhost:5173", + "http://[::1]:4444", + "http://localhost", + ] { + assert!(is_origin_allowed(origin), "expected {origin} to be allowed"); + let r = with_cors_headers(ok_response(), Some(origin)); + assert_eq!(allow_origin(&r).as_deref(), Some(origin)); + } +} + +#[test] +fn rejects_disallowed_origins() { + for origin in [ + "https://attacker.example", + "http://evil.localhost.attacker.example", + "https://127.0.0.1.attacker.example", + // HTTPS variant of localhost is NOT a configuration we ship — refuse. + "https://localhost", + "null", + ] { + assert!( + !is_origin_allowed(origin), + "expected {origin} to be rejected" + ); + let r = with_cors_headers(ok_response(), Some(origin)); + assert!( + allow_origin(&r).is_none(), + "disallowed origin {origin} leaked Access-Control-Allow-Origin" + ); + } +} + +#[test] +fn missing_origin_emits_no_acao_but_sets_vary() { + let r = with_cors_headers(ok_response(), None); + assert!(allow_origin(&r).is_none()); + assert_eq!( + r.headers().get(header::VARY).and_then(|v| v.to_str().ok()), + Some("Origin") + ); +} + +#[test] +fn env_override_allows_extra_origins() { + // SAFETY: this test mutates a process-global env var. No other test in + // this crate reads ALLOWED_ORIGINS_ENV, so parallel runs are safe; we + // still restore the previous value on exit to be a good citizen. + let prev = std::env::var(ALLOWED_ORIGINS_ENV).ok(); + unsafe { + std::env::set_var( + ALLOWED_ORIGINS_ENV, + "https://debug.internal, http://harness:9000", + ); + } + + assert!(is_origin_allowed("https://debug.internal")); + assert!(is_origin_allowed("http://harness:9000")); + assert!(!is_origin_allowed( + "https://debug.internal.attacker.example" + )); + + unsafe { + match prev { + Some(v) => std::env::set_var(ALLOWED_ORIGINS_ENV, v), + None => std::env::remove_var(ALLOWED_ORIGINS_ENV), + } + } +} + +#[test] +fn always_sets_methods_headers_and_max_age() { + let r = with_cors_headers(ok_response(), Some("tauri://localhost")); + let h = r.headers(); + assert_eq!( + h.get(header::ACCESS_CONTROL_ALLOW_METHODS) + .and_then(|v| v.to_str().ok()), + Some("GET, POST, OPTIONS") + ); + assert_eq!( + h.get(header::ACCESS_CONTROL_ALLOW_HEADERS) + .and_then(|v| v.to_str().ok()), + Some("Content-Type, Authorization") + ); + assert_eq!( + h.get(header::ACCESS_CONTROL_MAX_AGE) + .and_then(|v| v.to_str().ok()), + Some("86400") + ); +}