mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(config): guard browser allow-all runtime toggle
## Summary - refuse `openhuman.config_set_browser_allow_all(enabled=true)` unless `OPENHUMAN_BROWSER_ALLOW_ALL_RPC_ENABLE=1` is present - keep runtime disable available so an already-enabled process can re-enforce the browser allowlist - update the RPC schema text and unit coverage for rejected enable, override-enabled toggle, and disable behavior Closes #1899 ## Testing - [x] `cargo fmt --all --check` - [x] `git diff --check` - [x] `cargo test -p openhuman set_browser_allow_all --lib` attempted; blocked before test execution because `whisper-rs-sys` could not find `clang.dll` / `libclang.dll` (`LIBCLANG_PATH` unset) ## Checklist - [x] I have tested the changes locally or documented why a local test could not complete. - [x] I have kept the change scoped to the linked issue. - [x] I have updated relevant tests or documentation. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Browser allow-all feature now requires explicit operator approval via environment variable to enable, restricting access to authorized users only. * Disabling the feature remains unrestricted. * Enhanced security audit logging for all browser allow-all state changes. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2312?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: aqilaziz <gonzes7@gmail.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
This commit is contained in:
co-authored by
Steven Enamakel
parent
b9925e0bf4
commit
08bbe3bc42
+37
-19
@@ -418,6 +418,9 @@ pub struct RuntimeFlagsOut {
|
||||
pub log_prompts: bool,
|
||||
}
|
||||
|
||||
const BROWSER_ALLOW_ALL_ENV: &str = "OPENHUMAN_BROWSER_ALLOW_ALL";
|
||||
const BROWSER_ALLOW_ALL_RPC_ENABLE_ENV: &str = "OPENHUMAN_BROWSER_ALLOW_ALL_RPC_ENABLE";
|
||||
|
||||
/// Returns a full configuration snapshot for the UI.
|
||||
pub async fn get_config_snapshot(config: &Config) -> Result<RpcOutcome<serde_json::Value>, String> {
|
||||
let snapshot = snapshot_config_json(config)?;
|
||||
@@ -932,13 +935,14 @@ pub async fn workspace_onboarding_flag_resolve(
|
||||
|
||||
/// Returns the current state of runtime-only flags.
|
||||
pub fn get_runtime_flags() -> RpcOutcome<RuntimeFlagsOut> {
|
||||
RpcOutcome::single_log(
|
||||
RuntimeFlagsOut {
|
||||
browser_allow_all: env_flag_enabled("OPENHUMAN_BROWSER_ALLOW_ALL"),
|
||||
log_prompts: env_flag_enabled("OPENHUMAN_LOG_PROMPTS"),
|
||||
},
|
||||
"runtime flags read",
|
||||
)
|
||||
RpcOutcome::single_log(runtime_flags(), "runtime flags read")
|
||||
}
|
||||
|
||||
fn runtime_flags() -> RuntimeFlagsOut {
|
||||
RuntimeFlagsOut {
|
||||
browser_allow_all: env_flag_enabled(BROWSER_ALLOW_ALL_ENV),
|
||||
log_prompts: env_flag_enabled("OPENHUMAN_LOG_PROMPTS"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Updates the `OPENHUMAN_BROWSER_ALLOW_ALL` environment flag.
|
||||
@@ -951,18 +955,32 @@ pub fn get_runtime_flags() -> RpcOutcome<RuntimeFlagsOut> {
|
||||
///
|
||||
/// `is_private_host` checks still apply to the resolved IP, so this
|
||||
/// flag does not unlock loopback / RFC1918 destinations.
|
||||
pub fn set_browser_allow_all(enabled: bool) -> RpcOutcome<RuntimeFlagsOut> {
|
||||
let was_enabled = env_flag_enabled("OPENHUMAN_BROWSER_ALLOW_ALL");
|
||||
if enabled {
|
||||
std::env::set_var("OPENHUMAN_BROWSER_ALLOW_ALL", "1");
|
||||
} else {
|
||||
std::env::remove_var("OPENHUMAN_BROWSER_ALLOW_ALL");
|
||||
pub fn set_browser_allow_all(enabled: bool) -> Result<RpcOutcome<RuntimeFlagsOut>, String> {
|
||||
if enabled && !env_flag_enabled(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV) {
|
||||
tracing::warn!(
|
||||
"[SECURITY] refused browser allow-all enable via RPC: \
|
||||
set {BROWSER_ALLOW_ALL_ENV}=1 at startup or explicitly set \
|
||||
{BROWSER_ALLOW_ALL_RPC_ENABLE_ENV}=1 before using the runtime toggle"
|
||||
);
|
||||
return Err(format!(
|
||||
"Refusing to enable {BROWSER_ALLOW_ALL_ENV} via RPC. Start OpenHuman with \
|
||||
{BROWSER_ALLOW_ALL_ENV}=1, or set {BROWSER_ALLOW_ALL_RPC_ENABLE_ENV}=1 for an \
|
||||
explicit operator-approved runtime override."
|
||||
));
|
||||
}
|
||||
let now_enabled = env_flag_enabled("OPENHUMAN_BROWSER_ALLOW_ALL");
|
||||
let flags = RuntimeFlagsOut {
|
||||
browser_allow_all: now_enabled,
|
||||
log_prompts: env_flag_enabled("OPENHUMAN_LOG_PROMPTS"),
|
||||
};
|
||||
|
||||
let was_enabled = env_flag_enabled(BROWSER_ALLOW_ALL_ENV);
|
||||
if enabled {
|
||||
unsafe {
|
||||
std::env::set_var(BROWSER_ALLOW_ALL_ENV, "1");
|
||||
}
|
||||
} else {
|
||||
unsafe {
|
||||
std::env::remove_var(BROWSER_ALLOW_ALL_ENV);
|
||||
}
|
||||
}
|
||||
let flags = runtime_flags();
|
||||
let now_enabled = flags.browser_allow_all;
|
||||
|
||||
if was_enabled != now_enabled {
|
||||
if now_enabled {
|
||||
@@ -984,7 +1002,7 @@ pub fn set_browser_allow_all(enabled: bool) -> RpcOutcome<RuntimeFlagsOut> {
|
||||
} else {
|
||||
"[SECURITY] browser allow-all flag set to disabled"
|
||||
};
|
||||
RpcOutcome::single_log(flags, log_msg)
|
||||
Ok(RpcOutcome::single_log(flags, log_msg))
|
||||
}
|
||||
|
||||
/// Checks if a specific onboarding flag file exists in the workspace.
|
||||
|
||||
@@ -149,30 +149,47 @@ fn get_runtime_flags_reads_env_overrides() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn set_browser_allow_all_toggles_env_var() {
|
||||
fn set_browser_allow_all_rejects_enable_without_operator_override() {
|
||||
let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let before = std::env::var("OPENHUMAN_BROWSER_ALLOW_ALL").ok();
|
||||
let before = std::env::var(BROWSER_ALLOW_ALL_ENV).ok();
|
||||
let before_override = std::env::var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV).ok();
|
||||
|
||||
let _ = set_browser_allow_all(true);
|
||||
assert!(env_flag_enabled("OPENHUMAN_BROWSER_ALLOW_ALL"));
|
||||
unsafe {
|
||||
std::env::remove_var(BROWSER_ALLOW_ALL_ENV);
|
||||
std::env::remove_var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV);
|
||||
}
|
||||
|
||||
let _ = set_browser_allow_all(false);
|
||||
assert!(!env_flag_enabled("OPENHUMAN_BROWSER_ALLOW_ALL"));
|
||||
let err = set_browser_allow_all(true).expect_err("runtime enable should require override");
|
||||
assert!(
|
||||
err.contains("Refusing to enable OPENHUMAN_BROWSER_ALLOW_ALL via RPC"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
assert!(!env_flag_enabled(BROWSER_ALLOW_ALL_ENV));
|
||||
|
||||
unsafe {
|
||||
match before {
|
||||
Some(v) => std::env::set_var("OPENHUMAN_BROWSER_ALLOW_ALL", v),
|
||||
None => std::env::remove_var("OPENHUMAN_BROWSER_ALLOW_ALL"),
|
||||
Some(v) => std::env::set_var(BROWSER_ALLOW_ALL_ENV, v),
|
||||
None => std::env::remove_var(BROWSER_ALLOW_ALL_ENV),
|
||||
}
|
||||
match before_override {
|
||||
Some(v) => std::env::set_var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV, v),
|
||||
None => std::env::remove_var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn set_browser_allow_all_emits_security_audit_log() {
|
||||
fn set_browser_allow_all_toggles_env_var_when_operator_override_is_set() {
|
||||
let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let before = std::env::var("OPENHUMAN_BROWSER_ALLOW_ALL").ok();
|
||||
let before = std::env::var(BROWSER_ALLOW_ALL_ENV).ok();
|
||||
let before_override = std::env::var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV).ok();
|
||||
|
||||
let enable_outcome = set_browser_allow_all(true);
|
||||
unsafe {
|
||||
std::env::remove_var(BROWSER_ALLOW_ALL_ENV);
|
||||
std::env::set_var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV, "1");
|
||||
}
|
||||
|
||||
let enable_outcome = set_browser_allow_all(true).expect("override should allow runtime enable");
|
||||
assert_eq!(enable_outcome.logs.len(), 1);
|
||||
let enable_log = &enable_outcome.logs[0];
|
||||
assert!(
|
||||
@@ -184,8 +201,9 @@ fn set_browser_allow_all_emits_security_audit_log() {
|
||||
"enable log should mention enabled state: {enable_log}"
|
||||
);
|
||||
assert!(enable_outcome.value.browser_allow_all);
|
||||
assert!(env_flag_enabled(BROWSER_ALLOW_ALL_ENV));
|
||||
|
||||
let disable_outcome = set_browser_allow_all(false);
|
||||
let disable_outcome = set_browser_allow_all(false).expect("runtime disable should always work");
|
||||
assert_eq!(disable_outcome.logs.len(), 1);
|
||||
let disable_log = &disable_outcome.logs[0];
|
||||
assert!(
|
||||
@@ -197,11 +215,49 @@ fn set_browser_allow_all_emits_security_audit_log() {
|
||||
"disable log should mention disabled state: {disable_log}"
|
||||
);
|
||||
assert!(!disable_outcome.value.browser_allow_all);
|
||||
assert!(!env_flag_enabled(BROWSER_ALLOW_ALL_ENV));
|
||||
|
||||
unsafe {
|
||||
match before {
|
||||
Some(v) => std::env::set_var("OPENHUMAN_BROWSER_ALLOW_ALL", v),
|
||||
None => std::env::remove_var("OPENHUMAN_BROWSER_ALLOW_ALL"),
|
||||
Some(v) => std::env::set_var(BROWSER_ALLOW_ALL_ENV, v),
|
||||
None => std::env::remove_var(BROWSER_ALLOW_ALL_ENV),
|
||||
}
|
||||
match before_override {
|
||||
Some(v) => std::env::set_var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV, v),
|
||||
None => std::env::remove_var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn set_browser_allow_all_disable_does_not_require_operator_override() {
|
||||
let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let before = std::env::var(BROWSER_ALLOW_ALL_ENV).ok();
|
||||
let before_override = std::env::var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV).ok();
|
||||
|
||||
unsafe {
|
||||
std::env::set_var(BROWSER_ALLOW_ALL_ENV, "1");
|
||||
std::env::remove_var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV);
|
||||
}
|
||||
|
||||
let disable_outcome =
|
||||
set_browser_allow_all(false).expect("runtime disable should not require override");
|
||||
assert!(
|
||||
disable_outcome.logs[0].contains("[SECURITY]"),
|
||||
"disable log should be audit-tagged: {:?}",
|
||||
disable_outcome.logs
|
||||
);
|
||||
assert!(!disable_outcome.value.browser_allow_all);
|
||||
assert!(!env_flag_enabled(BROWSER_ALLOW_ALL_ENV));
|
||||
|
||||
unsafe {
|
||||
match before {
|
||||
Some(v) => std::env::set_var(BROWSER_ALLOW_ALL_ENV, v),
|
||||
None => std::env::remove_var(BROWSER_ALLOW_ALL_ENV),
|
||||
}
|
||||
match before_override {
|
||||
Some(v) => std::env::set_var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV, v),
|
||||
None => std::env::remove_var(BROWSER_ALLOW_ALL_RPC_ENABLE_ENV),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -591,11 +591,11 @@ pub fn schemas(function: &str) -> ControllerSchema {
|
||||
"set_browser_allow_all" => ControllerSchema {
|
||||
namespace: "config",
|
||||
function: "set_browser_allow_all",
|
||||
description: "Set OPENHUMAN_BROWSER_ALLOW_ALL runtime flag.",
|
||||
description: "Disable browser allow-all mode, or enable it only when operator opt-in is present.",
|
||||
inputs: vec![FieldSchema {
|
||||
name: "enabled",
|
||||
ty: TypeSchema::Bool,
|
||||
comment: "Whether to enable browser allow-all mode.",
|
||||
comment: "Whether to enable browser allow-all mode. Runtime enable is refused unless OPENHUMAN_BROWSER_ALLOW_ALL_RPC_ENABLE=1.",
|
||||
required: true,
|
||||
}],
|
||||
outputs: vec![FieldSchema {
|
||||
@@ -1067,7 +1067,7 @@ fn handle_resolve_api_url(_params: Map<String, Value>) -> ControllerFuture {
|
||||
fn handle_set_browser_allow_all(params: Map<String, Value>) -> ControllerFuture {
|
||||
Box::pin(async move {
|
||||
let payload = deserialize_params::<SetBrowserAllowAllParams>(params)?;
|
||||
to_json(config_rpc::set_browser_allow_all(payload.enabled))
|
||||
to_json(config_rpc::set_browser_allow_all(payload.enabled)?)
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user