mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Expose managed policy for browser settings imports (#39720)
## What changed - Add `in_app_browser.allow_external_browser_settings_import` to managed requirements, preserving explicit Boolean values through layered composition while leaving an omitted value unset. - Return the policy as `inAppBrowser.allowExternalBrowserSettingsImport` from `configRequirements/read` and include it in the generated protocol schemas. - Keep the import policy independent from the in-app browser feature flag and agent Browser Use requirements. ## Testing - Cover parsing, managed-layer precedence, invalid values, user and session override resistance, and app-server response serialization. GitOrigin-RevId: efa2621d2b1cf503f1bee2505d9914cb4fb7221d
This commit is contained in:
1 parent
6d020311f0
commit
1674b0a130
22 files changed
+454
-2
No files matched your search
@@ -21,6 +21,7 @@ use wildmatch::WildMatchPattern;
|
||||
use super::requirements_exec_policy::RequirementsExecPolicyToml;
|
||||
use crate::Constrained;
|
||||
use crate::ConstraintError;
|
||||
use crate::InAppBrowserRequirementsToml;
|
||||
use crate::ManagedAuthPolicy;
|
||||
use crate::ManagedHooksRequirementsToml;
|
||||
use crate::McpServerRequirement;
|
||||
@@ -942,6 +943,7 @@ pub struct ConfigRequirementsToml {
|
||||
pub allow_remote_control: Option<bool>,
|
||||
pub computer_use: Option<ComputerUseRequirementsToml>,
|
||||
pub browser_use: Option<BrowserUseRequirementsToml>,
|
||||
pub in_app_browser: Option<InAppBrowserRequirementsToml>,
|
||||
pub windows: Option<WindowsRequirementsToml>,
|
||||
#[serde(rename = "features", alias = "feature_requirements")]
|
||||
pub feature_requirements: Option<FeatureRequirementsToml>,
|
||||
@@ -1043,6 +1045,7 @@ pub struct ConfigRequirementsWithSources {
|
||||
pub allow_remote_control: Option<Sourced<bool>>,
|
||||
pub computer_use: Option<Sourced<ComputerUseRequirementsToml>>,
|
||||
pub browser_use: Option<Sourced<BrowserUseRequirementsToml>>,
|
||||
pub in_app_browser: Option<Sourced<InAppBrowserRequirementsToml>>,
|
||||
pub windows: Option<Sourced<WindowsRequirementsToml>>,
|
||||
pub feature_requirements: Option<Sourced<FeatureRequirementsToml>>,
|
||||
pub hooks: Option<Sourced<ManagedHooksRequirementsToml>>,
|
||||
@@ -1100,6 +1103,7 @@ impl ConfigRequirementsWithSources {
|
||||
allow_remote_control: _,
|
||||
computer_use: _,
|
||||
browser_use: _,
|
||||
in_app_browser: _,
|
||||
windows: _,
|
||||
feature_requirements: _,
|
||||
hooks: _,
|
||||
@@ -1150,6 +1154,7 @@ impl ConfigRequirementsWithSources {
|
||||
allow_remote_control,
|
||||
computer_use,
|
||||
browser_use,
|
||||
in_app_browser,
|
||||
windows,
|
||||
feature_requirements,
|
||||
hooks,
|
||||
@@ -1229,6 +1234,7 @@ impl ConfigRequirementsWithSources {
|
||||
allow_remote_control,
|
||||
computer_use,
|
||||
browser_use,
|
||||
in_app_browser,
|
||||
windows,
|
||||
feature_requirements,
|
||||
hooks,
|
||||
@@ -1267,6 +1273,7 @@ impl ConfigRequirementsWithSources {
|
||||
allow_remote_control: allow_remote_control.map(|sourced| sourced.value),
|
||||
computer_use: computer_use.map(|sourced| sourced.value),
|
||||
browser_use: browser_use.map(|sourced| sourced.value),
|
||||
in_app_browser: in_app_browser.map(|sourced| sourced.value),
|
||||
windows: windows.map(|sourced| sourced.value),
|
||||
feature_requirements: feature_requirements.map(|sourced| sourced.value),
|
||||
hooks: hooks.map(|sourced| sourced.value),
|
||||
@@ -1380,6 +1387,10 @@ impl ConfigRequirementsToml {
|
||||
.browser_use
|
||||
.as_ref()
|
||||
.is_none_or(BrowserUseRequirementsToml::is_empty)
|
||||
&& self
|
||||
.in_app_browser
|
||||
.as_ref()
|
||||
.is_none_or(|requirements| requirements == &InAppBrowserRequirementsToml::default())
|
||||
&& self
|
||||
.windows
|
||||
.as_ref()
|
||||
@@ -1581,6 +1592,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
allow_remote_control,
|
||||
computer_use,
|
||||
browser_use: _,
|
||||
in_app_browser: _,
|
||||
windows,
|
||||
feature_requirements,
|
||||
hooks,
|
||||
@@ -2108,6 +2120,7 @@ mod tests {
|
||||
allow_remote_control,
|
||||
computer_use,
|
||||
browser_use,
|
||||
in_app_browser,
|
||||
windows,
|
||||
feature_requirements,
|
||||
hooks,
|
||||
@@ -2161,6 +2174,8 @@ mod tests {
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
computer_use: computer_use.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
browser_use: browser_use.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
in_app_browser: in_app_browser
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
windows: windows.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
feature_requirements: feature_requirements
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
@@ -2457,6 +2472,7 @@ mod tests {
|
||||
allow_remote_control: Some(false),
|
||||
computer_use: Some(computer_use.clone()),
|
||||
browser_use: None,
|
||||
in_app_browser: None,
|
||||
windows: Some(windows.clone()),
|
||||
feature_requirements: Some(feature_requirements.clone()),
|
||||
hooks: None,
|
||||
@@ -2532,6 +2548,7 @@ mod tests {
|
||||
)),
|
||||
computer_use: Some(Sourced::new(computer_use, enforce_source.clone())),
|
||||
browser_use: None,
|
||||
in_app_browser: None,
|
||||
windows: Some(Sourced::new(windows, enforce_source.clone())),
|
||||
feature_requirements: Some(Sourced::new(
|
||||
feature_requirements,
|
||||
@@ -2585,6 +2602,7 @@ mod tests {
|
||||
allow_remote_control: None,
|
||||
computer_use: None,
|
||||
browser_use: None,
|
||||
in_app_browser: None,
|
||||
windows: None,
|
||||
feature_requirements: None,
|
||||
hooks: None,
|
||||
@@ -2643,6 +2661,7 @@ mod tests {
|
||||
allow_remote_control: None,
|
||||
computer_use: None,
|
||||
browser_use: None,
|
||||
in_app_browser: None,
|
||||
windows: None,
|
||||
feature_requirements: None,
|
||||
hooks: None,
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
use serde::Deserialize;
|
||||
|
||||
/// Managed requirements for the interactive in-app browser, not agent Browser Use.
|
||||
#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
|
||||
pub struct InAppBrowserRequirementsToml {
|
||||
/// Whether external browser settings may be imported. Only `Some(false)`
|
||||
/// denies import; omission is effectively `true`. Keep omission unset while
|
||||
/// composing managed layers so it cannot replace an explicit requirement.
|
||||
pub allow_external_browser_settings_import: Option<bool>,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "in_app_browser_requirements_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,144 @@
|
||||
use super::InAppBrowserRequirementsToml;
|
||||
use crate::ConfigRequirementsToml;
|
||||
use crate::ConfigRequirementsWithSources;
|
||||
use crate::RequirementSource;
|
||||
use crate::RequirementsLayerEntry;
|
||||
use crate::Sourced;
|
||||
use crate::compose_requirements_for_hostname;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
const EMPTY: &str = "[in_app_browser]";
|
||||
const ALLOW: &str = "[in_app_browser]\nallow_external_browser_settings_import = true";
|
||||
const DENY: &str = "[in_app_browser]\nallow_external_browser_settings_import = false";
|
||||
|
||||
fn parse(contents: &str) -> ConfigRequirementsToml {
|
||||
toml::from_str(contents).expect("parse requirements")
|
||||
}
|
||||
|
||||
fn source(name: &str) -> RequirementSource {
|
||||
RequirementSource::EnterpriseManaged {
|
||||
id: name.to_string(),
|
||||
name: name.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn import_requirement_preserves_unset_and_explicit_values() {
|
||||
for (contents, expected_table) in [
|
||||
("", None),
|
||||
(EMPTY, Some(None)),
|
||||
(ALLOW, Some(Some(true))),
|
||||
(DENY, Some(Some(false))),
|
||||
] {
|
||||
let requirements = parse(contents);
|
||||
assert_eq!(
|
||||
requirements,
|
||||
ConfigRequirementsToml {
|
||||
in_app_browser: expected_table.map(|value| InAppBrowserRequirementsToml {
|
||||
allow_external_browser_settings_import: value,
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
let configured = requirements
|
||||
.in_app_browser
|
||||
.as_ref()
|
||||
.and_then(|policy| policy.allow_external_browser_settings_import);
|
||||
assert_eq!(requirements.is_empty(), configured.is_none());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn import_requirement_uses_normal_managed_toml_precedence() {
|
||||
for (low, high, expected) in [
|
||||
("", EMPTY, None),
|
||||
(DENY, "", Some(DENY)),
|
||||
(DENY, EMPTY, Some(DENY)),
|
||||
(DENY, ALLOW, Some(ALLOW)),
|
||||
(ALLOW, DENY, Some(DENY)),
|
||||
] {
|
||||
let composed = compose_requirements_for_hostname(
|
||||
[
|
||||
RequirementsLayerEntry::from_toml(source("low"), low),
|
||||
RequirementsLayerEntry::from_toml(source("high"), high),
|
||||
],
|
||||
/*hostname*/ None,
|
||||
)
|
||||
.expect("compose managed import policy")
|
||||
.map(ConfigRequirementsWithSources::into_toml);
|
||||
assert_eq!(composed, expected.map(parse));
|
||||
}
|
||||
|
||||
let composed = compose_requirements_for_hostname(
|
||||
[RequirementsLayerEntry::from_toml(source("managed"), DENY)],
|
||||
/*hostname*/ None,
|
||||
)
|
||||
.expect("compose managed denial");
|
||||
assert_eq!(
|
||||
composed,
|
||||
Some(ConfigRequirementsWithSources {
|
||||
in_app_browser: Some(Sourced::new(
|
||||
InAppBrowserRequirementsToml {
|
||||
allow_external_browser_settings_import: Some(false),
|
||||
},
|
||||
source("managed"),
|
||||
)),
|
||||
..Default::default()
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_merge_uses_whole_table_precedence() {
|
||||
for (high, low, expected, expected_source) in [
|
||||
(EMPTY, DENY, EMPTY, "high"),
|
||||
("", ALLOW, ALLOW, "low"),
|
||||
(DENY, ALLOW, DENY, "high"),
|
||||
(ALLOW, DENY, ALLOW, "high"),
|
||||
] {
|
||||
let mut composed = ConfigRequirementsWithSources::default();
|
||||
composed.merge_unset_fields(source("high"), parse(high));
|
||||
composed.merge_unset_fields(source("low"), parse(low));
|
||||
assert_eq!(
|
||||
composed,
|
||||
ConfigRequirementsWithSources {
|
||||
in_app_browser: Some(Sourced::new(
|
||||
parse(expected).in_app_browser.expect("requirements table"),
|
||||
source(expected_source),
|
||||
)),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
assert_eq!(composed.into_toml(), parse(expected));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_import_requirement_is_rejected_even_when_overridden() {
|
||||
for invalid in ["\"false\"", "1", "[]", "{}"] {
|
||||
let contents =
|
||||
format!("[in_app_browser]\nallow_external_browser_settings_import = {invalid}");
|
||||
let error = toml::from_str::<ConfigRequirementsToml>(&contents)
|
||||
.expect_err("import requirement must be a boolean");
|
||||
assert!(
|
||||
error
|
||||
.to_string()
|
||||
.contains("allow_external_browser_settings_import"),
|
||||
"{error}",
|
||||
);
|
||||
let error = compose_requirements_for_hostname(
|
||||
[
|
||||
RequirementsLayerEntry::from_toml(source("invalid"), contents),
|
||||
RequirementsLayerEntry::from_toml(source("higher"), ALLOW),
|
||||
],
|
||||
/*hostname*/ None,
|
||||
)
|
||||
.expect_err("invalid managed layers must not be silently ignored");
|
||||
assert!(
|
||||
error
|
||||
.to_string()
|
||||
.contains("allow_external_browser_settings_import"),
|
||||
"{error}",
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -9,6 +9,7 @@ mod diagnostics;
|
||||
mod fingerprint;
|
||||
mod hook_config;
|
||||
mod host_name;
|
||||
mod in_app_browser_requirements;
|
||||
mod key_aliases;
|
||||
pub mod loader;
|
||||
mod marketplace_edit;
|
||||
@@ -119,6 +120,7 @@ pub use hook_config::HooksToml;
|
||||
pub use hook_config::ManagedHooksRequirementsToml;
|
||||
pub use hook_config::MatcherGroup;
|
||||
pub use host_name::host_name;
|
||||
pub use in_app_browser_requirements::InAppBrowserRequirementsToml;
|
||||
pub use marketplace_edit::MarketplaceConfigUpdate;
|
||||
pub use marketplace_edit::RemoveMarketplaceConfigOutcome;
|
||||
pub use marketplace_edit::record_user_marketplace;
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::*;
|
||||
use crate::ConfigRequirementsToml;
|
||||
use codex_file_system::CopyOptions;
|
||||
use codex_file_system::CreateDirectoryOptions;
|
||||
use codex_file_system::ExecutorFileSystemFuture;
|
||||
@@ -16,6 +17,44 @@ use tempfile::tempdir;
|
||||
|
||||
pub(super) struct TestFileSystem;
|
||||
|
||||
#[tokio::test]
|
||||
async fn managed_browser_import_denial_survives_user_and_session_config() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let allow = "[in_app_browser]\nallow_external_browser_settings_import = true";
|
||||
let deny = "[in_app_browser]\nallow_external_browser_settings_import = false";
|
||||
let requirements_path = tmp.path().join("requirements.toml");
|
||||
std::fs::write(&requirements_path, allow).expect("write system requirements");
|
||||
std::fs::write(tmp.path().join(CONFIG_TOML_FILE), allow).expect("write user config");
|
||||
let mut loader_overrides = LoaderOverrides::without_managed_config_for_tests();
|
||||
loader_overrides.system_requirements_path = Some(requirements_path);
|
||||
|
||||
let stack = load_config_layers_state(
|
||||
&TestFileSystem,
|
||||
tmp.path(),
|
||||
/*cwd*/ None,
|
||||
&[(
|
||||
"in_app_browser.allow_external_browser_settings_import".to_string(),
|
||||
TomlValue::Boolean(true),
|
||||
)],
|
||||
ConfigLoadOptions {
|
||||
loader_overrides,
|
||||
strict_config: false,
|
||||
cloud_config_bundle:
|
||||
crate::test_support::CloudConfigBundleFixture::enterprise_requirement(deny)
|
||||
.add_enterprise_requirement("[in_app_browser]")
|
||||
.into_loader(),
|
||||
},
|
||||
&crate::NoopThreadConfigLoader,
|
||||
)
|
||||
.await
|
||||
.expect("load managed browser import requirement");
|
||||
|
||||
assert_eq!(
|
||||
stack.requirements_toml(),
|
||||
&toml::from_str::<ConfigRequirementsToml>(deny).expect("expected requirement"),
|
||||
);
|
||||
}
|
||||
|
||||
impl ExecutorFileSystem for TestFileSystem {
|
||||
fn canonicalize<'a>(
|
||||
&'a self,
|
||||
|
||||
@@ -233,6 +233,7 @@ fn populate_merged_regular_fields_with_sources(
|
||||
allow_remote_control,
|
||||
computer_use,
|
||||
browser_use,
|
||||
in_app_browser,
|
||||
windows,
|
||||
feature_requirements,
|
||||
hooks: _,
|
||||
@@ -280,6 +281,7 @@ fn populate_merged_regular_fields_with_sources(
|
||||
set_sourced!(auto_review, &["auto_review"]);
|
||||
set_sourced!(computer_use, &["computer_use"]);
|
||||
set_sourced!(browser_use, &["browser_use"]);
|
||||
set_sourced!(in_app_browser, &["in_app_browser"]);
|
||||
set_sourced!(windows, &["windows"]);
|
||||
set_sourced!(feature_requirements, &["features", "feature_requirements"]);
|
||||
set_sourced!(mcp_servers, &["mcp_servers"]);
|
||||
|
||||
Reference in new issue
Block a user