mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-30 23:14:37 +00:00
fix(security): replace wildcard CORS on core RPC with origin allowlist
## 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. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2266?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Leigh Stillard <leigh@stillard.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
This commit is contained in:
co-authored by
Steven Enamakel
parent
ec74c7346b
commit
92682d06e5
+90
-7
@@ -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;
|
||||
|
||||
@@ -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<String> {
|
||||
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")
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user