fix(permissions): validate network access flow
This commit is contained in:
@ -1,7 +1,7 @@
|
||||
//! Codex CLI permission projector (lot LP3-2).
|
||||
//!
|
||||
//! Produces the **permission-relevant** part of Codex's `config.toml`
|
||||
//! (`sandbox_mode` / `approval_policy`) plus the matching launch args
|
||||
//! (`sandbox_mode` / `approval_policy` / `sandbox_workspace_write.network_access`) plus the matching launch args
|
||||
//! (`--sandbox` / `--ask-for-approval`) and, for workspace-write postures, the
|
||||
//! project root as an additional writable directory (`--add-dir`). The
|
||||
//! posture→mode derivation is extracted verbatim from the former
|
||||
@ -11,7 +11,7 @@
|
||||
//! Unlike Claude's seed, Codex's `config.toml` is **co-owned** (it also carries the
|
||||
//! `mcp_servers.idea` table and the `projects.*` trust entries, which are MCP/trust
|
||||
//! concerns, not permissions). The projector therefore emits a
|
||||
//! [`ProjectedFile::MergeToml`] limited to the two permission keys it manages —
|
||||
//! [`ProjectedFile::MergeToml`] limited to the permission keys/table it manages —
|
||||
//! everything else in the file is preserved by the fold, and the file is **never**
|
||||
//! deleted on swap (hence an empty `owned_replace_paths`).
|
||||
|
||||
@ -19,6 +19,7 @@ use domain::permission::{
|
||||
EffectivePermissions, PermissionProjection, PermissionProjector, Posture, ProjectedFile,
|
||||
ProjectionContext, ProjectorKey,
|
||||
};
|
||||
use domain::NetworkPolicy;
|
||||
|
||||
use super::toml_string;
|
||||
|
||||
@ -27,10 +28,13 @@ use super::toml_string;
|
||||
/// the file lives at `.codex/config.toml` relative to the run dir.
|
||||
const CONFIG_REL_PATH: &str = ".codex/config.toml";
|
||||
|
||||
/// The two top-level keys this projector manages in `config.toml`. Everything else
|
||||
/// The top-level keys this projector manages in `config.toml`. Everything else
|
||||
/// (MCP table, trust entries, user keys) is preserved by the merge.
|
||||
const MANAGED_KEYS: [&str; 2] = ["sandbox_mode", "approval_policy"];
|
||||
|
||||
/// Codex workspace-write sandbox table owned by IdeA for network projection.
|
||||
const SANDBOX_WORKSPACE_WRITE_TABLE: &str = "sandbox_workspace_write";
|
||||
|
||||
/// Projects [`EffectivePermissions`] into Codex's sandbox/approval config + args.
|
||||
///
|
||||
/// Pure: `project` only computes the plan; the launch path merges the TOML fragment
|
||||
@ -46,23 +50,31 @@ impl PermissionProjector for CodexPermissionProjector {
|
||||
fn project(
|
||||
&self,
|
||||
eff: Option<&EffectivePermissions>,
|
||||
network: Option<NetworkPolicy>,
|
||||
ctx: &ProjectionContext,
|
||||
) -> PermissionProjection {
|
||||
// Product invariant: nothing posed ⇒ nothing projected. Codex keeps its
|
||||
// native sandbox/approval defaults (no args, no managed keys written).
|
||||
// No filesystem/bash policy ⇒ no sandbox/approval projection. Network is
|
||||
// orthogonal and still gets an explicit env override to avoid stale inheritance.
|
||||
let Some(permissions) = eff else {
|
||||
return PermissionProjection::empty();
|
||||
return PermissionProjection {
|
||||
files: vec![codex_network_file(network)],
|
||||
env: codex_network_env(network),
|
||||
..PermissionProjection::empty()
|
||||
};
|
||||
};
|
||||
let sandbox = codex_sandbox_mode(permissions);
|
||||
let approval = codex_approval_policy(permissions);
|
||||
let network_access = codex_network_access(network);
|
||||
|
||||
// Permission-only TOML fragment (escaped exactly like the former
|
||||
// `set_top_level_toml_value`). The mcp_servers/trust tables are NOT a
|
||||
// permission concern and stay with the MCP wiring (LP3-3).
|
||||
let contents = format!(
|
||||
"sandbox_mode = {}\napproval_policy = {}\n",
|
||||
"sandbox_mode = {}\napproval_policy = {}\n\n[{}]\nnetwork_access = {}\n",
|
||||
toml_string(sandbox),
|
||||
toml_string(approval),
|
||||
SANDBOX_WORKSPACE_WRITE_TABLE,
|
||||
network_access,
|
||||
);
|
||||
|
||||
let mut args = vec![
|
||||
@ -79,12 +91,12 @@ impl PermissionProjector for CodexPermissionProjector {
|
||||
PermissionProjection {
|
||||
files: vec![ProjectedFile::MergeToml {
|
||||
rel_path: CONFIG_REL_PATH.to_owned(),
|
||||
managed_tables: Vec::new(),
|
||||
managed_tables: vec![SANDBOX_WORKSPACE_WRITE_TABLE.to_owned()],
|
||||
managed_keys: MANAGED_KEYS.iter().map(|k| (*k).to_owned()).collect(),
|
||||
contents,
|
||||
}],
|
||||
args,
|
||||
env: Vec::new(),
|
||||
env: codex_network_env(network),
|
||||
}
|
||||
}
|
||||
|
||||
@ -94,6 +106,38 @@ impl PermissionProjector for CodexPermissionProjector {
|
||||
}
|
||||
}
|
||||
|
||||
fn codex_network_file(network: Option<NetworkPolicy>) -> ProjectedFile {
|
||||
ProjectedFile::MergeToml {
|
||||
rel_path: CONFIG_REL_PATH.to_owned(),
|
||||
managed_tables: vec![SANDBOX_WORKSPACE_WRITE_TABLE.to_owned()],
|
||||
managed_keys: Vec::new(),
|
||||
contents: format!(
|
||||
"[{}]\nnetwork_access = {}\n",
|
||||
SANDBOX_WORKSPACE_WRITE_TABLE,
|
||||
codex_network_access(network),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
fn codex_network_env(network: Option<NetworkPolicy>) -> Vec<(String, String)> {
|
||||
// Codex inherits the parent environment by default. Always set the variable for
|
||||
// Codex launches so a stale `CODEX_SANDBOX_NETWORK_DISABLED=1` in IdeA's own
|
||||
// environment cannot leak into a newly allowed child.
|
||||
let disabled = if codex_network_access(network) {
|
||||
"0"
|
||||
} else {
|
||||
"1"
|
||||
};
|
||||
vec![(
|
||||
"CODEX_SANDBOX_NETWORK_DISABLED".to_owned(),
|
||||
disabled.to_owned(),
|
||||
)]
|
||||
}
|
||||
|
||||
fn codex_network_access(network: Option<NetworkPolicy>) -> bool {
|
||||
matches!(network, Some(NetworkPolicy::Allow))
|
||||
}
|
||||
|
||||
fn codex_sandbox_mode(permissions: &EffectivePermissions) -> &'static str {
|
||||
match permissions.fallback() {
|
||||
Posture::Deny => "read-only",
|
||||
@ -130,10 +174,33 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn project_none_is_empty() {
|
||||
let proj = CodexPermissionProjector.project(None, &ctx());
|
||||
assert!(proj.files.is_empty());
|
||||
let proj = CodexPermissionProjector.project(None, None, &ctx());
|
||||
assert_eq!(proj.files.len(), 1);
|
||||
assert!(proj.args.is_empty());
|
||||
assert!(proj.env.is_empty());
|
||||
match &proj.files[0] {
|
||||
ProjectedFile::MergeToml {
|
||||
rel_path,
|
||||
managed_tables,
|
||||
managed_keys,
|
||||
contents,
|
||||
} => {
|
||||
assert_eq!(rel_path, CONFIG_REL_PATH);
|
||||
assert_eq!(
|
||||
managed_tables,
|
||||
&vec![SANDBOX_WORKSPACE_WRITE_TABLE.to_owned()]
|
||||
);
|
||||
assert!(managed_keys.is_empty());
|
||||
assert!(
|
||||
contents.contains("[sandbox_workspace_write]\nnetwork_access = false"),
|
||||
"network is denied by default: {contents:?}"
|
||||
);
|
||||
}
|
||||
ProjectedFile::Replace { .. } => panic!("Codex must emit a MergeToml file"),
|
||||
}
|
||||
assert_eq!(
|
||||
proj.env,
|
||||
vec![("CODEX_SANDBOX_NETWORK_DISABLED".to_owned(), "1".to_owned())]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -152,8 +219,12 @@ mod tests {
|
||||
(Posture::Ask, "workspace-write", "on-request"),
|
||||
(Posture::Allow, "workspace-write", "never"),
|
||||
] {
|
||||
let proj = CodexPermissionProjector.project(Some(&eff(posture)), &ctx());
|
||||
assert!(proj.env.is_empty(), "Codex projection carries no env");
|
||||
let proj = CodexPermissionProjector.project(Some(&eff(posture)), None, &ctx());
|
||||
assert_eq!(
|
||||
proj.env,
|
||||
vec![("CODEX_SANDBOX_NETWORK_DISABLED".to_owned(), "1".to_owned())],
|
||||
"Codex projection denies network by default"
|
||||
);
|
||||
|
||||
// -- The single MergeToml file, with the two managed permission keys.
|
||||
assert_eq!(proj.files.len(), 1, "exactly one file projected");
|
||||
@ -165,7 +236,10 @@ mod tests {
|
||||
contents,
|
||||
} => {
|
||||
assert_eq!(rel_path, CONFIG_REL_PATH);
|
||||
assert!(managed_tables.is_empty(), "no managed tables");
|
||||
assert_eq!(
|
||||
managed_tables,
|
||||
&vec![SANDBOX_WORKSPACE_WRITE_TABLE.to_owned()]
|
||||
);
|
||||
assert_eq!(
|
||||
managed_keys,
|
||||
&vec!["sandbox_mode".to_owned(), "approval_policy".to_owned()]
|
||||
@ -178,6 +252,10 @@ mod tests {
|
||||
contents.contains(&format!("approval_policy = \"{approval}\"")),
|
||||
"posture {posture:?}: contents={contents:?}"
|
||||
);
|
||||
assert!(
|
||||
contents.contains("[sandbox_workspace_write]\nnetwork_access = false"),
|
||||
"network is denied by default: {contents:?}"
|
||||
);
|
||||
}
|
||||
ProjectedFile::Replace { .. } => panic!("Codex must emit a MergeToml file"),
|
||||
}
|
||||
@ -210,4 +288,34 @@ mod tests {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn network_policy_maps_to_stale_proof_env() {
|
||||
for (network, disabled, network_access) in [
|
||||
(Some(NetworkPolicy::Allow), "0", true),
|
||||
(Some(NetworkPolicy::Deny), "1", false),
|
||||
(Some(NetworkPolicy::Ask), "1", false),
|
||||
(None, "1", false),
|
||||
] {
|
||||
let proj =
|
||||
CodexPermissionProjector.project(Some(&eff(Posture::Allow)), network, &ctx());
|
||||
assert_eq!(
|
||||
proj.env,
|
||||
vec![(
|
||||
"CODEX_SANDBOX_NETWORK_DISABLED".to_owned(),
|
||||
disabled.to_owned()
|
||||
)],
|
||||
"network={network:?}"
|
||||
);
|
||||
match &proj.files[0] {
|
||||
ProjectedFile::MergeToml { contents, .. } => assert!(
|
||||
contents.contains(&format!(
|
||||
"[sandbox_workspace_write]\nnetwork_access = {network_access}"
|
||||
)),
|
||||
"network={network:?}: {contents:?}"
|
||||
),
|
||||
ProjectedFile::Replace { .. } => panic!("Codex must emit a MergeToml file"),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user