diff --git a/crates/tui/src/config.rs b/crates/tui/src/config.rs index 7382699e3b..4c5bbe3d41 100644 --- a/crates/tui/src/config.rs +++ b/crates/tui/src/config.rs @@ -2595,6 +2595,13 @@ pub struct Config { #[serde(skip)] pub loaded_config_path: Option, + /// Runtime-only receipt that a higher-precedence layer supplied `[network]` + /// — a managed overlay. The resolved policy is then not the user + /// document's to replace: a mid-session re-read of that document must not + /// widen what the higher layer set. + #[serde(skip)] + pub(crate) network_layer_is_managed: bool, + /// A resolved startup snapshot never reads remembered route choices again. /// False means an explicit config/profile owns the route instead. #[serde(skip)] @@ -3059,6 +3066,33 @@ impl NetworkPolicyToml { } } +/// Re-read the `[network]` table from a configuration document. +/// +/// Deliberately narrower than [`Config::load`]: this runs on the tool-context +/// build path, where re-applying the environment, managed, and credential +/// layers would be both wasteful and wrong. Only the document's own table comes +/// back — the same table `/network allow ` edits. +/// +/// `None` means there is nothing usable to adopt: the document is missing, +/// unreadable, unparseable, or carries no `[network]` table. Callers keep the +/// policy they already hold, which is the conservative direction for an +/// allow/deny gate. +/// +/// Known limitation: `[profiles..network]` is not consulted. `/network` +/// does not write it either, so a live session and the command agree on this +/// base table. +#[must_use] +pub fn network_policy_from_document(path: &Path) -> Option { + #[derive(Deserialize)] + struct NetworkTable { + network: Option, + } + + let contents = fs::read_to_string(path).ok()?; + let parsed: NetworkTable = toml::from_str(&contents).ok()?; + parsed.network.map(NetworkPolicyToml::into_runtime) +} + /// `[lsp]` table — mirrors [`crate::lsp::LspConfig`]. Documented in /// `config.example.toml`. When omitted, defaults from `LspConfig::default()` /// apply (enabled, 5 s poll, 20 diagnostics/file, errors only, no overrides). @@ -10447,6 +10481,8 @@ fn merge_config(base: Config, override_cfg: Config) -> Config { notifications: override_cfg.notifications.or(base.notifications), approval: override_cfg.approval.or(base.approval), network: override_cfg.network.or(base.network), + network_layer_is_managed: override_cfg.network_layer_is_managed + || base.network_layer_is_managed, verifier: override_cfg.verifier.or(base.verifier), advisor: override_cfg.advisor.or(base.advisor), skills: merge_skills_config(base.skills, override_cfg.skills), @@ -10972,6 +11008,12 @@ fn apply_managed_overrides(config: &mut Config) -> Result<()> { } merged.base_url_env_receipt = BaseUrlEnvReceipt::NoOwner; } + // A managed `[network]` table outranks the user document. Record that the + // resolved policy is not the document's to replace, so a mid-session + // re-read of that document folds onto it instead of widening it. + if managed.network.is_some() { + merged.network_layer_is_managed = true; + } *config = merged; Ok(()) } diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index f279cf821f..d56411c750 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -7104,6 +7104,59 @@ impl Engine { ) } + /// The network decider this turn runs under: the live policy, re-read from + /// the document the session was launched with. + /// + /// `self.config.network_policy` is the snapshot `Engine::new` took when the + /// session was spawned. `/network allow ` edits `config.toml` and + /// promises "Retry the command now.", but it cannot reach that snapshot — + /// so without this re-read the host stayed refused until the next engine + /// spawn. Same shape as the workspace trust list loaded below, which also + /// re-reads per tool-context build so `/trust add` lands mid-session; a + /// hand edit of `[network]` lands through it too. + /// + /// Both directions have to land. `/network deny ` writes the same + /// `[network]` table `/network allow` does, and a session that started + /// without one adopts it the moment it appears — otherwise tightening a + /// policy mid-session would need the restart this exists to remove. + /// + /// A session that started gated keeps the policy it holds when the document + /// stops carrying a `[network]` table, so removing the table cannot leave a + /// session ungated by accident. + /// + /// The session cache rides along on a refresh, so a host approved through + /// the approval prompt survives it. An adopted table has no cache to + /// inherit: the session had no decider, so nothing was ever approved under + /// one. + fn current_network_decider(&self) -> Option { + let Some(path) = self.api_config.loaded_config_path.as_deref() else { + return self.config.network_policy.clone(); + }; + let Some(document) = crate::config::network_policy_from_document(path) else { + // The document carries no `[network]` table. Keep what the session + // resolved: removing a table must not ungate a gated run. + return self.config.network_policy.clone(); + }; + match self.config.network_policy.as_ref() { + // A managed overlay or a Fleet denial produced this policy, so the + // document is a lower layer: it may add hosts, but it may not widen + // the fallback or lift a denial. + Some(decider) + if decider.is_authoritative() || self.api_config.network_layer_is_managed => + { + Some( + decider + .with_policy_refreshed(decider.policy().folded_with_lower_layer(document)), + ) + } + Some(decider) => Some(decider.with_policy_refreshed(document)), + // Nothing resolved a policy for this session, so there is no higher + // layer to protect. Adopt the table the moment it exists — that is + // how `/network deny ` lands where there was no policy yet. + None => Some(crate::network_policy::NetworkPolicyDecider::with_default_audit(document)), + } + } + /// Build one tool context from the already-resolved turn authority and /// route. A preview owns values that are deliberately not installed on the /// session; rebuilding either from `self.session` would give it the prior @@ -7210,8 +7263,8 @@ impl Engine { ctx.memory_path = Some(self.config.memory_path.clone()); } - if let Some(decider) = self.config.network_policy.as_ref() { - ctx = ctx.with_network_policy(decider.clone()); + if let Some(decider) = self.current_network_decider() { + ctx = ctx.with_network_policy(decider); } // Adaptive evidence routing is engine-native and opt-in @@ -7420,8 +7473,8 @@ impl Engine { } pool = pool.with_backend(crate::mcp::McpBackend::from_config(&self.api_config)); pool = pool.with_disallowed_tools(self.config.disallowed_tools.clone().unwrap_or_default()); - if let Some(decider) = self.config.network_policy.as_ref() { - pool = pool.with_network_policy(decider.clone()); + if let Some(decider) = self.current_network_decider() { + pool = pool.with_network_policy(decider); } // The self-serve login tool honors the same pre-registered redirect // overrides `/mcp login` uses, or providers with pinned callback diff --git a/crates/tui/src/core/engine/tests/test_cases_12.rs b/crates/tui/src/core/engine/tests/test_cases_12.rs index 0028b5b4d4..ca9d2fbc25 100644 --- a/crates/tui/src/core/engine/tests/test_cases_12.rs +++ b/crates/tui/src/core/engine/tests/test_cases_12.rs @@ -1581,4 +1581,156 @@ fn turn_tool_context_uses_planned_authority_and_route_not_installed_session() { .model, "planned-next-model" ); +} + +/// `/network allow ` edits `config.toml` and tells the operator to retry +/// the command. The engine holds its policy by value, so that retry only works +/// when the tool context re-reads the document — this is the regression guard +/// for a session that kept refusing a host the file already allowed. +#[test] +fn tool_context_network_policy_follows_the_config_document_mid_session() { + use crate::network_policy::{Decision, DecisionToml, NetworkPolicy, NetworkPolicyDecider}; + + let dir = tempdir().expect("temp dir"); + let config_path = dir.path().join("config.toml"); + fs::write( + &config_path, + "[network]\ndefault = \"prompt\"\nallow = [\"api.github.com\"]\n", + ) + .expect("write config"); + + let api_config = Config { + loaded_config_path: Some(config_path), + ..Config::default() + }; + + // The snapshot the engine was spawned with does not allow the host yet. + let engine_config = EngineConfig { + network_policy: Some(NetworkPolicyDecider::new( + NetworkPolicy { + default: DecisionToml::Prompt, + ..NetworkPolicy::default() + }, + None, + )), + ..EngineConfig::default() + }; + let (engine, _handle) = Engine::new(engine_config, &api_config); + + let authority = crate::core::authority::TurnAuthority::from_effective_fields( + AppMode::Agent, + true, + true, + true, + ApprovalMode::Bypass, + ); + let route = TurnRouteContext { + provider: ProviderKind::Deepseek, + model: "network-refresh-model".to_string(), + capabilities: codewhale_config::route::RouteCapabilities::default(), + limits: None, + client: None, + api_config: Box::new(Config::default()), + locale_tag: engine.config.locale_tag.clone(), + role_models: engine.subagent_role_models(), + auto_model: false, + reasoning_effort: None, + reasoning_effort_auto: false, + }; + + let context = engine.build_tool_context_for_turn(&authority, &route); + let decider = context + .network_policy + .as_ref() + .expect("the configured policy is injected"); + assert_eq!( + decider.evaluate("api.github.com", "Bash"), + Decision::Allow, + "the document already allows the host; the retry must see it" + ); + assert_eq!( + decider.evaluate("unlisted.example.com", "Bash"), + Decision::Prompt, + "a host the document does not name still prompts" + ); +} + +/// `/network deny ` writes the same `[network]` table `/network allow` +/// does. A session that started without one has to adopt it — otherwise +/// tightening the policy mid-session needs the restart this fix removes — and a +/// session that started gated must not lose its policy because the table went +/// away. +#[test] +fn tool_context_adopts_a_policy_written_mid_session_and_never_ungates_one() { + use crate::network_policy::{Decision, DecisionToml, NetworkPolicy, NetworkPolicyDecider}; + + let dir = tempdir().expect("temp dir"); + let config_path = dir.path().join("config.toml"); + fs::write( + &config_path, + "[network]\ndefault = \"prompt\"\ndeny = [\"evil.example.com\"]\n", + ) + .expect("write config"); + + let api_config = Config { + loaded_config_path: Some(config_path.clone()), + ..Config::default() + }; + + // A session that started with no `[network]` table at all adopts the deny. + let (engine, _handle) = Engine::new(EngineConfig::default(), &api_config); + let authority = crate::core::authority::TurnAuthority::from_effective_fields( + AppMode::Agent, + true, + true, + true, + ApprovalMode::Bypass, + ); + let route = TurnRouteContext { + provider: ProviderKind::Deepseek, + model: "network-adopt-model".to_string(), + capabilities: codewhale_config::route::RouteCapabilities::default(), + limits: None, + client: None, + api_config: Box::new(Config::default()), + locale_tag: engine.config.locale_tag.clone(), + role_models: engine.subagent_role_models(), + auto_model: false, + reasoning_effort: None, + reasoning_effort_auto: false, + }; + let context = engine.build_tool_context_for_turn(&authority, &route); + let decider = context + .network_policy + .as_ref() + .expect("a table written mid-session is adopted"); + assert_eq!( + decider.evaluate("evil.example.com", "Bash"), + Decision::Deny, + "the deny direction must land mid-session too" + ); + + // Removing the table must not ungate a session that started gated. + fs::write(&config_path, "# `[network]` removed\n").expect("rewrite config"); + let engine_config = EngineConfig { + network_policy: Some(NetworkPolicyDecider::new( + NetworkPolicy { + default: DecisionToml::Deny, + ..NetworkPolicy::default() + }, + None, + )), + ..EngineConfig::default() + }; + let (gated, _handle) = Engine::new(engine_config, &api_config); + let context = gated.build_tool_context_for_turn(&authority, &route); + let decider = context + .network_policy + .as_ref() + .expect("a gated session stays gated"); + assert_eq!( + decider.evaluate("unlisted.example.com", "Bash"), + Decision::Deny, + "removing the table must not hand a gated session an open policy" + ); } \ No newline at end of file diff --git a/crates/tui/src/lib.rs b/crates/tui/src/lib.rs index 64eb9a00b1..e1023ca55b 100644 --- a/crates/tui/src/lib.rs +++ b/crates/tui/src/lib.rs @@ -14734,13 +14734,19 @@ fn exec_network_policy( // Fleet caps are an outer authority boundary: user configuration may // narrow them further, but it may never widen an explicit network denial. if outer_network_access == Some(false) { - return Some(crate::network_policy::NetworkPolicyDecider::new( - crate::network_policy::NetworkPolicy { - default: crate::network_policy::DecisionToml::Deny, - ..crate::network_policy::NetworkPolicy::default() - }, - None, - )); + // A Fleet denial is an outer authority the user's document may never + // widen, so mark the decider authoritative: a mid-session re-read of + // that document folds onto it instead of replacing it. + return Some( + crate::network_policy::NetworkPolicyDecider::new( + crate::network_policy::NetworkPolicy { + default: crate::network_policy::DecisionToml::Deny, + ..crate::network_policy::NetworkPolicy::default() + }, + None, + ) + .with_authoritative(), + ); } config.network.clone().map(|toml_cfg| { crate::network_policy::NetworkPolicyDecider::with_default_audit(toml_cfg.into_runtime()) diff --git a/crates/tui/src/network_policy.rs b/crates/tui/src/network_policy.rs index 3fa32375a9..2f43cf6aed 100644 --- a/crates/tui/src/network_policy.rs +++ b/crates/tui/src/network_policy.rs @@ -122,6 +122,23 @@ fn default_decision() -> DecisionToml { DecisionToml::Prompt } +/// The stricter of two fallback decisions, ordering `Deny` < `Prompt` < `Allow`. +/// Used when a lower-precedence layer is folded onto a policy an authority set. +fn stricter_decision(left: DecisionToml, right: DecisionToml) -> DecisionToml { + fn rank(decision: DecisionToml) -> u8 { + match decision { + DecisionToml::Deny => 0, + DecisionToml::Prompt => 1, + DecisionToml::Allow => 2, + } + } + if rank(left) <= rank(right) { + left + } else { + right + } +} + fn default_audit() -> bool { true } @@ -171,6 +188,62 @@ impl From for DecisionToml { } impl NetworkPolicy { + /// Fold a lower-precedence layer (the user's config document) onto this + /// policy without letting it widen what a higher authority set. + /// + /// The fold is deliberately asymmetric, because the two layers are not + /// peers: + /// + /// * `deny` is the **union**. Either layer saying no is a no. + /// * `default` keeps the **stricter** of the two. + /// * When the authority's fallback is `Deny`, the lower layer contributes + /// **nothing that can allow**: `allow`, `proxy`, and the fake-IP CIDRs + /// stay the authority's own. `decide` checks `allow` *before* falling back + /// to `default`, so adopting a lower `allow` entry under a + /// deny-by-default authority would let the document lift exactly the + /// denial the authority set. + /// * Otherwise the authority is prompt-by-default, and the lower layer may + /// do what `/network allow ` writes: name additional hosts. Those + /// merge with the authority's own rather than replacing them, so a + /// refresh cannot erase an administrator's allowances either. + /// * `audit` is the one field that widens by `true`: either layer asking for + /// the audit log keeps it, and a document cannot silence it under an + /// authority. + #[must_use] + pub fn folded_with_lower_layer(&self, lower: Self) -> Self { + fn union(mut base: Vec, extra: Vec) -> Vec { + for entry in extra { + if !base.iter().any(|existing| existing == &entry) { + base.push(entry); + } + } + base + } + + let deny_by_default = self.default == DecisionToml::Deny; + let (allow, proxy, proxy_fake_ip_cidrs) = if deny_by_default { + ( + self.allow.clone(), + self.proxy.clone(), + self.proxy_fake_ip_cidrs.clone(), + ) + } else { + ( + union(self.allow.clone(), lower.allow), + union(self.proxy.clone(), lower.proxy), + union(self.proxy_fake_ip_cidrs.clone(), lower.proxy_fake_ip_cidrs), + ) + }; + Self { + default: stricter_decision(self.default, lower.default), + allow, + deny: union(self.deny.clone(), lower.deny), + proxy, + proxy_fake_ip_cidrs, + audit: self.audit || lower.audit, + } + } + /// Decide what to do for a single outbound call to `host`. /// /// **Deny-wins precedence**: if `host` matches any entry in `deny`, the @@ -461,6 +534,12 @@ pub struct NetworkPolicyDecider { /// A resolved IP inside one of these ranges bypasses the restricted-IP SSRF /// block; real private/loopback/link-local/metadata IPs are unaffected. trusted_fakeip_cidrs: Vec<(Ipv4Addr, u8)>, + /// The policy was produced by an authority the user's config document may + /// not override — a managed layer, or a Fleet denial the user is not + /// allowed to widen. A mid-session re-read of that document then folds onto + /// this policy instead of replacing it. See + /// [`NetworkPolicy::folded_with_lower_layer`]. + authoritative: bool, } impl NetworkPolicyDecider { @@ -477,9 +556,24 @@ impl NetworkPolicyDecider { cache: NetworkSessionCache::new(), auditor, trusted_fakeip_cidrs, + authoritative: false, } } + /// Mark this policy as set by an authority the user document may not + /// override. See the `authoritative` field. + #[must_use] + pub fn with_authoritative(mut self) -> Self { + self.authoritative = true; + self + } + + /// Whether an authority set this policy rather than the user document. + #[must_use] + pub fn is_authoritative(&self) -> bool { + self.authoritative + } + /// Register IPv4 CIDR ranges to treat as benign fake-IP placeholders. /// Invalid CIDR strings are skipped. See [`Self::is_trusted_fakeip_addr`]. #[must_use] @@ -524,6 +618,43 @@ impl NetworkPolicyDecider { Self::new(policy, auditor) } + /// Rebuild this decider against a policy re-read from disk, keeping the + /// session cache so an approval already granted in this session survives + /// the refresh. + /// + /// The engine snapshots its policy when it is spawned, and `/network allow + /// ` only edits the configuration document — it cannot reach that + /// snapshot. Callers hand the re-read table here instead of waiting for a + /// restart. The auditor keeps its log path and follows the new policy's + /// `audit` switch. + /// + /// Extra CIDRs registered through [`Self::with_trusted_fakeip_cidrs`] are + /// not carried over; the refresh re-derives them from `policy`. Configured + /// fake-IP ranges already ride in the policy, and no production caller + /// registers extras. + #[must_use] + pub fn with_policy_refreshed(&self, policy: NetworkPolicy) -> Self { + let auditor = match self.auditor.as_ref() { + Some(existing) => Some(NetworkAuditor::new( + existing.path().to_path_buf(), + policy.audit_enabled(), + )), + None if policy.audit_enabled() => NetworkAuditor::default_path(true), + None => None, + }; + Self { + trusted_fakeip_cidrs: policy + .proxy_fake_ip_cidrs + .iter() + .filter_map(|cidr| parse_trusted_fakeip_cidr(cidr)) + .collect(), + policy, + cache: self.cache.clone(), + auditor, + authoritative: self.authoritative, + } + } + /// Inspect the policy. #[must_use] pub fn policy(&self) -> &NetworkPolicy { @@ -844,6 +975,190 @@ mod tests { ); } + #[test] + fn refreshed_policy_adopts_new_table_without_retracting_session_approvals() { + let decider = NetworkPolicyDecider::new(mk(Decision::Prompt, &[], &[]), None); + decider.approve_session("approved.example.com", "fetch_url"); + + // Mid-session `/network allow api.github.com` reaches the session as a + // re-read table rather than a restart. + let refreshed = + decider.with_policy_refreshed(mk(Decision::Prompt, &["api.github.com"], &[])); + + assert_eq!( + refreshed.evaluate("api.github.com", "Bash"), + Decision::Allow, + "the re-read table is in force" + ); + assert_eq!( + refreshed.evaluate("approved.example.com", "fetch_url"), + Decision::Allow, + "an approval already granted this session survives the refresh" + ); + assert_eq!( + refreshed.evaluate("unlisted.example.com", "Bash"), + Decision::Prompt, + "hosts the table does not name keep the default" + ); + } + + #[test] + fn refreshed_policy_honours_a_new_deny_list() { + let decider = NetworkPolicyDecider::new(mk(Decision::Allow, &[], &[]), None); + assert_eq!( + decider.evaluate("evil.example.com", "fetch_url"), + Decision::Allow + ); + + let refreshed = + decider.with_policy_refreshed(mk(Decision::Allow, &[], &["evil.example.com"])); + + assert_eq!( + refreshed.evaluate("evil.example.com", "fetch_url"), + Decision::Deny, + "a deny entry written after spawn is enforced without a restart" + ); + } + + #[test] + fn refreshed_policy_keeps_the_authority_marking() { + let decider = + NetworkPolicyDecider::new(mk(Decision::Deny, &[], &[]), None).with_authoritative(); + assert!(decider.is_authoritative()); + let refreshed = decider.with_policy_refreshed(mk(Decision::Allow, &[], &[])); + assert!( + refreshed.is_authoritative(), + "a refreshed authority is still an authority" + ); + } + + #[test] + fn folding_a_document_never_widens_an_authority() { + // Assert what the folded policy *decides*, not the shape of its fields: + // `decide` checks `allow` before falling back to `default`, so a field + // assertion can look safe while the verdict is `Allow`. + + // What a Fleet denial or a managed overlay resolved to: deny by default. + let authority = NetworkPolicy { + default: DecisionToml::Deny, + deny: vec!["blocked.example.com".to_string()], + ..NetworkPolicy::default() + }; + // What the user's document asks for on a mid-session re-read. + let document = NetworkPolicy { + default: DecisionToml::Allow, + allow: vec!["api.github.com".to_string()], + proxy: vec!["proxy.example.com".to_string()], + proxy_fake_ip_cidrs: vec!["198.18.0.0/15".to_string()], + audit: false, + ..NetworkPolicy::default() + }; + + let folded = authority.folded_with_lower_layer(document); + + assert_eq!( + folded.decide("api.github.com"), + Decision::Deny, + "a document cannot lift a deny-by-default authority's denial" + ); + assert_eq!( + folded.decide("blocked.example.com"), + Decision::Deny, + "a denial the authority set survives the fold" + ); + assert_eq!( + folded.decide("unlisted.example.com"), + Decision::Deny, + "an unlisted host still falls back to the authority's denial" + ); + assert!( + folded.proxy.is_empty() && folded.proxy_fake_ip_cidrs.is_empty(), + "a document cannot hand itself a fake-IP SSRF exception under a denial" + ); + assert!( + folded.audit, + "a document cannot silence the audit log under an authority" + ); + + // A prompt-by-default authority is the case `/network allow` exists for: + // the document may name hosts, and it may not erase the authority's own. + let prompt_authority = NetworkPolicy { + default: DecisionToml::Prompt, + allow: vec!["internal.example.com".to_string()], + deny: vec!["blocked.example.com".to_string()], + ..NetworkPolicy::default() + }; + let folded = prompt_authority.folded_with_lower_layer(NetworkPolicy { + default: DecisionToml::Allow, + allow: vec!["api.github.com".to_string()], + ..NetworkPolicy::default() + }); + assert_eq!( + folded.decide("api.github.com"), + Decision::Allow, + "the document may still name hosts when the authority prompts" + ); + assert_eq!( + folded.decide("internal.example.com"), + Decision::Allow, + "the authority's own allowance survives the fold" + ); + assert_eq!( + folded.decide("unlisted.example.com"), + Decision::Prompt, + "the document cannot widen the authority's fallback to allow" + ); + assert_eq!( + folded.decide("blocked.example.com"), + Decision::Deny, + "either layer's denial wins" + ); + + // The plain user path replaces the policy outright, so + // `/network default allow` still lands there. + let plain = NetworkPolicy { + default: DecisionToml::Prompt, + ..NetworkPolicy::default() + }; + assert_eq!( + plain + .folded_with_lower_layer(NetworkPolicy { + default: DecisionToml::Allow, + ..NetworkPolicy::default() + }) + .default, + DecisionToml::Prompt + ); + } + + /// A managed overlay may legitimately resolve `default = "allow"`. Its fold + /// takes the union branch, and the document's own fallback can only narrow + /// it — nothing there is more permissive than the authority's own policy, + /// because that policy already allowed everything. + #[test] + fn folding_an_allow_by_default_authority_only_narrows() { + let authority = NetworkPolicy { + default: DecisionToml::Allow, + ..NetworkPolicy::default() + }; + let folded = authority.folded_with_lower_layer(NetworkPolicy { + default: DecisionToml::Deny, + allow: vec!["named.example.com".to_string()], + ..NetworkPolicy::default() + }); + + assert_eq!( + folded.decide("named.example.com"), + Decision::Allow, + "the authority allowed everything already; naming a host is not a widening" + ); + assert_eq!( + folded.decide("other.example.com"), + Decision::Deny, + "the document narrowed the fallback, which is the direction it may move" + ); + } + #[test] fn approve_persistent_writes_back_to_policy() { let policy = mk(Decision::Prompt, &[], &[]);