fix(subagent): keep the detached-agent floor, re-gate on narrowing, deliver agent approvals while busy

Follow-up to d1655c424 after an adversarial review (ok=false). The
live-posture half stays; this fixes the rest.

1. HIGH - floor restored for detached agents. d1655c424 judged every agent
   call as foreground, so a detached agent could run `rm -rf /usr`, `dd
   of=/dev/sda` or `curl | sh` under Full Access (and Auto-Review/Ask
   loosened too). An agent call is Background again when it starts
   detached OR the agent itself is detached (no foreground turn owns it:
   gate_runtime.foreground_children is None). The real over-block is fixed
   at the root instead: (a) command_safety's literal DANGEROUS_PATTERNS
   matched `rm -rf /` as a substring of every absolute path (`rm -rf
   /home/me/proj/build` was "delete root"); a pattern must now not run on
   into a longer path. (b) The auto-review destroyer check treated any
   absolute forced-delete target as a system path; it now takes the
   workspace (already passed to AutoReviewContext) and clears a target
   strictly inside it (lexical and symlink-resolved; the root itself, a
   `..` escape and a symlink hop out still hold). The old foreground test
   is restored.
2. Posture re-read after the verdict. A guardian or a prompt can take a
   while; the child now re-reads the live posture after its gate and gates
   again if it narrowed (LiveRuntimeAuthority::narrows, the parent's rule).
   A posture change during a guardian review abandons the review and
   re-gates (Full Access then runs the call at once).
3. One posture projection. Engine::live_tool_context and LivePosture::apply
   share project_turn_authority (trust, approval, shell policy, sandbox,
   Plan network hint); agent calls clamp shell to the agent's own grant.
   Deleted the no-op posture copies in child_runtime.
4. "Gets stuck" - an agent's approval answer was only routed while the
   engine was idle or itself awaiting approval; while the parent turn
   streamed or ran tools nobody read it. EngineHandle now hands an agent's
   answer to the waiting agent directly (SubAgentManager), so the engine's
   two routing branches and route_child_approval_decision are deleted.
5. Guardian: agents show "Auto-Review checking '<tool>'" progress; the
   unavailable/timeout message names Full Access and no longer says
   "Switch to Ask" unconditionally.
6. CHANGELOG: the Linux no-new-privileges limit on runtime Full Access.
7. Engine test moved out from under another test's doc comment.

Evidence (macOS, focused, CARGO_BUILD_JOBS=2, RUST_MIN_STACK=16777216):
- Negative controls (fix reverted in code, tests kept) - all 3 FAILED:
  a_detached_agents_catastrophic_shell_is_held_in_every_posture (dd ran:
  "0+0 records in"), a_posture_narrowed_during_a_prompt_regates_the_call
  (call ran: "held"), an_agent_approval_answer_reaches_the_agent_while_the
  _engine_is_busy (Elapsed - agent never answered).
- With fix: codewhale-execpolicy all suites 250 passed, 0 failed;
  tools::subagent + fleet:: + tui::auto_review + core::authority +
  core::engine + tui::approval: 2032 passed, 0 failed, 4 ignored;
  tools::shell + tui::ui + runtime_threads + session_lifecycle +
  pending_requests + acp_server: 1519 passed, 0 failed, 3 ignored.
- rustfmt --edition 2024 --check clean on touched files.
Not reproduced/proven: guardian abandon-on-switch has no test (the test
guardian fails fast); no real Linux run.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
This commit is contained in:
Hunter BandClaude Opus 5.5 committed 2026-09-29 17:37:03 -07:00
1 parent d1655c4246
commit 05264125ea
12 files changed
+445 -155

No files matched your search

+10
View File
@@ -37,6 +37,16 @@ quieter, and Fleet runs can be checked before they spend anything.
### Fixed
- Agents follow the Permissions you choose while they run: switching to
Full Access reaches an agent that is already working, instead of leaving
it with the Auto-Review guardian that denied it. Tightening reaches it too.
An agent's answer to an approval prompt is delivered while the main
session is busy, and a forced delete inside the workspace is no longer
treated as a catastrophic system delete. On Linux, Full Access switched on
at runtime cannot lift the kernel no-new-privileges flag set at startup,
so `sudo` and setuid helpers still fail; start with
`sandbox_mode = "danger-full-access"` or `CODEWHALE_NO_NEW_PRIVS=0` if
agents need them.
- Switching providers keeps a model set only in the root `default_text_model`
with the provider it belongs to. Switching away and back (`/provider` in the
TUI, or the desktop app's model chip) used to land on the provider's catalog
+11 -2
View File
@@ -1814,9 +1814,18 @@ pub fn analyze_command(command: &str) -> SafetyAnalysis {
// Check for dangerous patterns first. The token-aware pass above handles
// spacing and quoting variants; these literal patterns remain as a compact
// fallback for legacy shapes.
// fallback for legacy shapes. A pattern must not run on into a longer
// path: `rm -rf /` is the root, `rm -rf /home/me/project/build` is not.
for (pattern, reason) in DANGEROUS_PATTERNS {
if command_lower.contains(&pattern.to_lowercase()) {
let pattern = pattern.to_lowercase();
if command_lower.match_indices(&pattern).any(|(at, _)| {
command_lower[at + pattern.len()..]
.chars()
.next()
.is_none_or(|next| {
!(next.is_alphanumeric() || matches!(next, '_' | '-' | '.' | '/'))
})
}) {
return SafetyAnalysis::dangerous(
command,
vec![(*reason).to_string()],
+65 -69
View File
@@ -752,6 +752,10 @@ pub struct EngineHandle {
compaction_cancellation: Arc<StdMutex<CompactionCancellationState>>,
/// Read-only view of the engine's turn-phase heartbeat (#6184).
turn_heartbeat: Arc<turn_heartbeat::TurnHeartbeat>,
/// Where an agent's pending approval waits. An answer to one is handed
/// straight to it: the engine may be streaming or running tools and not
/// reading approvals for a long time.
subagent_manager: SharedSubAgentManager,
}
const MAX_PENDING_COMPACTION_CANCELLATIONS: usize = 64;
@@ -1038,7 +1042,7 @@ pub struct Engine {
}
#[derive(Debug, Clone, PartialEq, Eq)]
struct LiveRuntimeAuthority {
pub(crate) struct LiveRuntimeAuthority {
mode: AppMode,
allow_shell: bool,
trust_mode: bool,
@@ -1087,7 +1091,7 @@ impl LiveRuntimeAuthority {
///
/// A call the user approved under `prior` stays approved under a posture
/// that is equal or broader; only a narrowing sends it back for a retry.
fn narrows(&self, prior: &Self) -> bool {
pub(crate) fn narrows(&self, prior: &Self) -> bool {
fn posture_rank(mode: ApprovalMode) -> u8 {
match mode {
ApprovalMode::Never => 0,
@@ -1157,7 +1161,8 @@ impl LiveRuntimeAuthorityState {
/// posture: an agent spawned under Auto-Review went on asking the guardian,
/// and being denied, after the person had granted Full Access. Each agent call
/// now re-reads the posture the person last chose — the same cell the TUI
/// decides that agent's approval prompts against — and the sandbox it implies.
/// decides that agent's approval prompts against — through the projection
/// the parent turn uses (trust, approval, shell policy, sandbox).
#[derive(Clone)]
pub(crate) struct LivePosture {
state: Arc<StdMutex<LiveRuntimeAuthorityState>>,
@@ -1166,26 +1171,54 @@ pub(crate) struct LivePosture {
}
impl LivePosture {
/// Project the live posture onto one agent call: approval, and the
/// sandbox that posture implies. Shell and trust stay as the agent's grant
/// narrowed them.
pub(crate) fn apply(&self, context: &mut ToolContext) {
let live = self
.state
/// The posture the person last chose.
pub(crate) fn read(&self) -> LiveRuntimeAuthority {
self.state
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.authority
.clone();
context.auto_approve = live.auto_approve;
context.approval_mode = live.approval_mode;
context.elevated_sandbox_policy = Some(crate::core::authority::sandbox_policy_for_turn(
live.mode,
live.approval_mode,
live.configured_sandbox_mode.as_deref(),
&self.workspace,
self.network_access,
));
.clone()
}
/// Project the live posture onto one agent call — the same projection the
/// parent turn applies to its own calls — and return what was read.
pub(crate) fn apply(&self, context: &mut ToolContext) -> LiveRuntimeAuthority {
let live = self.read();
project_turn_authority(
context,
&TurnAuthority::from_effective_fields(
live.mode,
live.allow_shell,
live.trust_mode,
live.auto_approve,
live.approval_mode,
),
&self.workspace,
live.configured_sandbox_mode.as_deref(),
self.network_access,
);
live
}
}
/// Project one permission authority onto a tool context: trust, approval,
/// shell policy, and the sandbox that posture implies. The parent turn and
/// every agent call use this one projection.
fn project_turn_authority(
context: &mut ToolContext,
authority: &TurnAuthority,
workspace: &Path,
configured_sandbox_mode: Option<&str>,
network_access: crate::core::authority::SandboxNetworkAccess,
) {
context.trust_mode = authority.trust_mode;
context.auto_approve = authority.auto_approve;
context.approval_mode = authority.approval_mode;
context.set_shell_policy(authority.shell_policy());
context.elevated_sandbox_policy =
Some(authority.sandbox_policy(workspace, configured_sandbox_mode, network_access));
context.shell_network_denied_hint =
matches!(authority.mode, AppMode::Plan).then(|| PLAN_SHELL_NETWORK_DENIED_HINT.to_string());
}
#[cfg(test)]
@@ -2088,6 +2121,7 @@ impl Engine {
live_runtime_authority,
compaction_cancellation,
turn_heartbeat: Arc::clone(&engine.turn_heartbeat),
subagent_manager: Arc::clone(&engine.subagent_manager),
};
(engine, handle)
@@ -2828,14 +2862,10 @@ impl Engine {
completion = self.rx_subagent_completion.recv(), if subagent_wake_armed => {
return completion.map(EngineRunInput::SubAgentCompletion);
}
// A background child may be waiting on a person's answer
// while the parent turn is idle: route it. Any other
// decision has no waiter and is dropped, as before.
decision = self.rx_approval.recv() => {
if let Some(decision) = decision {
self.route_child_approval_decision(decision).await;
}
}
// No call of this engine awaits approval while idle, and
// the handle hands agent answers to the agent directly:
// a stale decision has no waiter and is dropped.
_ = self.rx_approval.recv() => {}
update = async {
match self.mcp_boot_rx.as_mut() {
Some(rx) => rx.recv().await,
@@ -2875,40 +2905,6 @@ impl Engine {
}
}
/// Deliver an approval decision to a child waiting on it. Returns whether
/// a child took it; the parent's own awaiting call keeps every other id.
async fn route_child_approval_decision(
&self,
decision: super::engine::approval::ApprovalDecision,
) -> bool {
use crate::tools::subagent::{ChildApprovalOutcome, SubAgentManager};
let (id, outcome) = match &decision {
super::engine::approval::ApprovalDecision::Approved { id, .. } => {
(id.clone(), ChildApprovalOutcome::Approved)
}
super::engine::approval::ApprovalDecision::Denied { id, .. } => {
(id.clone(), ChildApprovalOutcome::Denied)
}
// A child has no timeout outcome of its own (#6101); an expired
// card is a deny for whichever call it was answering.
super::engine::approval::ApprovalDecision::TimedOut { id } => {
(id.clone(), ChildApprovalOutcome::Denied)
}
super::engine::approval::ApprovalDecision::Unavailable { id } => {
(id.clone(), ChildApprovalOutcome::Unavailable)
}
// A sandbox retry only exists for the parent's own tool call.
super::engine::approval::ApprovalDecision::RetryWithPolicy { .. } => return false,
};
if !SubAgentManager::is_child_approval_id(&id) {
return false;
}
self.subagent_manager
.write()
.await
.resolve_child_approval(&id, outcome)
}
/// Whether the idle loop should poll for background shell completion: a
/// background job is running or has finished without being claimed yet.
/// Plain interactive sessions arm exactly like goal sessions — a finished
@@ -6403,19 +6399,15 @@ impl Engine {
self.session.auto_approve,
self.session.approval_mode,
);
context.trust_mode = authority.trust_mode;
context.auto_approve = authority.auto_approve;
context.approval_mode = authority.approval_mode;
context.set_shell_policy(authority.shell_policy());
context.elevated_sandbox_policy = Some(authority.sandbox_policy(
project_turn_authority(
&mut context,
&authority,
&self.session.workspace,
self.api_config.sandbox_mode.as_deref(),
crate::core::authority::SandboxNetworkAccess::from_config(
self.api_config.sandbox_network_access,
),
));
context.shell_network_denied_hint = matches!(authority.mode, AppMode::Plan)
.then(|| PLAN_SHELL_NETWORK_DENIED_HINT.to_string());
);
Some(context)
}
@@ -8277,6 +8269,10 @@ pub(crate) fn mock_engine_handle() -> MockEngineHandle {
live_runtime_authority,
compaction_cancellation,
turn_heartbeat: turn_heartbeat::TurnHeartbeat::new(),
subagent_manager: crate::tools::subagent::new_shared_subagent_manager(
std::env::temp_dir(),
1,
),
};
MockEngineHandle {
+4 -6
View File
@@ -272,12 +272,10 @@ impl Engine {
).await?;
return Ok(ApprovalResult::RetryWithPolicy(policy));
}
// A child prompt answered while the parent itself is
// waiting: hand it to the child instead of dropping it.
other => {
self.route_child_approval_decision(other).await;
continue;
}
// A stale answer for another call: no waiter here. (An
// agent's answer never arrives here; the handle hands
// it to the agent directly.)
_ => continue,
}
}
}
+37 -16
View File
@@ -487,6 +487,35 @@ impl EngineHandle {
}
}
/// Deliver one approval answer. An agent's pending call gets it directly
/// — the engine may be streaming or running tools and not reading
/// approvals — and everything else goes to the engine's own waiting call.
async fn send_approval(&self, decision: ApprovalDecision) -> Result<()> {
use crate::tools::subagent::ChildApprovalOutcome;
let child = match &decision {
ApprovalDecision::Approved { id, .. } => Some((id, ChildApprovalOutcome::Approved)),
ApprovalDecision::Denied { id, .. } => Some((id, ChildApprovalOutcome::Denied)),
// An agent has no timeout outcome of its own (#6101): an expired
// card is a deny for whichever call it was answering.
ApprovalDecision::TimedOut { id } => Some((id, ChildApprovalOutcome::Denied)),
ApprovalDecision::Unavailable { id } => Some((id, ChildApprovalOutcome::Unavailable)),
// A sandbox retry only exists for the parent's own tool call.
ApprovalDecision::RetryWithPolicy { .. } => None,
};
if let Some((id, outcome)) = child
&& crate::tools::subagent::SubAgentManager::is_child_approval_id(id)
&& self
.subagent_manager
.write()
.await
.resolve_child_approval(id, outcome)
{
return Ok(());
}
self.tx_approval.send(decision).await?;
Ok(())
}
/// Approve a pending tool call because a person said yes.
pub async fn approve_tool_call(&self, id: impl Into<String>) -> Result<()> {
self.approve_tool_call_by(id, ApprovalDecider::User).await
@@ -499,10 +528,8 @@ impl EngineHandle {
id: impl Into<String>,
by: ApprovalDecider,
) -> Result<()> {
self.tx_approval
.send(ApprovalDecision::Approved { id: id.into(), by })
.await?;
Ok(())
self.send_approval(ApprovalDecision::Approved { id: id.into(), by })
.await
}
/// Deny a pending tool call because a person said no.
@@ -516,20 +543,16 @@ impl EngineHandle {
id: impl Into<String>,
by: ApprovalDecider,
) -> Result<()> {
self.tx_approval
.send(ApprovalDecision::Denied { id: id.into(), by })
.await?;
Ok(())
self.send_approval(ApprovalDecision::Denied { id: id.into(), by })
.await
}
/// Deny a pending tool call because its interactive approval card
/// expired (#6101). Kept distinct from [`Self::deny_tool_call`] so the
/// receipt records a timeout instead of an operator denial.
pub async fn deny_tool_call_timed_out(&self, id: impl Into<String>) -> Result<()> {
self.tx_approval
.send(ApprovalDecision::TimedOut { id: id.into() })
.await?;
Ok(())
self.send_approval(ApprovalDecision::TimedOut { id: id.into() })
.await
}
/// Resolve a pending tool call whose request could not be put in front
@@ -537,10 +560,8 @@ impl EngineHandle {
/// conversation). Kept distinct from [`Self::deny_tool_call`] so neither
/// the receipt nor the model message claims the person denied it.
pub async fn deny_tool_call_unavailable(&self, id: impl Into<String>) -> Result<()> {
self.tx_approval
.send(ApprovalDecision::Unavailable { id: id.into() })
.await?;
Ok(())
self.send_approval(ApprovalDecision::Unavailable { id: id.into() })
.await
}
/// Retry a tool call with an elevated sandbox policy a person chose.
+1 -1
View File
@@ -74,7 +74,7 @@ impl ReviewerOutcome {
"Auto-Review guardian denied tool '{tool_name}': {reason}. Do not work around this denial; find a materially safer path or stop."
))),
Self::Unavailable { reason } => Err(ToolError::permission_denied(format!(
"Auto-Review guardian unavailable ({reason}); the call was denied (fail closed). Switch to Ask to review this call yourself."
"Auto-Review guardian unavailable ({reason}); the call was denied (fail closed). To run it, the person can switch Permissions to Full Access, or to Ask where a prompt can be answered."
))),
Self::Cancelled => Err(ToolError::cancelled(
"Auto-Review guardian request cancelled",
+65 -15
View File
@@ -628,26 +628,17 @@ fn registry_instruction_is_in_the_initial_prompt_only_when_mcp_is_enabled() {
assert!(!prompt.contains(MCP_REGISTRY_FIRST_INSTRUCTION_SOURCE));
}
/// The regression this test exists for. A real DeepSeek turn — "build a
/// self-contained HTML focus timer, read a local fixture, verify it" — spent
/// its steps on five `tool_search` calls for `registry_sync`, a deferred-schema
/// retry, and reasoning about starting a browser MCP server, because the
/// always-visible instruction ordered Registry discovery *before* code
/// execution or a manual implementation and named two tools that are not in the
/// catalog head.
///
/// Two properties keep that from coming back, and both are about this prompt,
/// not about a second policy surface: discovery is never ordered ahead of
/// ordinary local work, and the deferred-tool cost of reaching the Registry
/// tools is stated where the model reads about them.
/// The engine hands every agent the live posture cell, not a copy: a
/// posture the person publishes after the agent's context was built is what
/// the agent's next call runs under, sandbox included.
/// the agent's next call runs under: approval, shell, sandbox.
#[test]
fn agent_tool_contexts_follow_the_published_posture() {
let (engine, handle) = Engine::new(EngineConfig::default(), &Config::default());
let context = engine.build_tool_context(AppMode::Agent, false);
let before = context.clone().with_live_posture();
let mut before = context.clone();
before
.refresh_live_posture()
.expect("engine contexts carry the live cell");
assert!(!before.auto_approve);
assert_ne!(
before.elevated_sandbox_policy,
@@ -661,15 +652,70 @@ fn agent_tool_contexts_follow_the_published_posture() {
ApprovalMode::Bypass,
None,
);
let after = context.with_live_posture();
let mut after = context;
after.refresh_live_posture();
assert!(after.auto_approve);
assert_eq!(after.approval_mode, ApprovalMode::Bypass);
assert_eq!(
after.elevated_sandbox_policy,
Some(crate::sandbox::SandboxPolicy::DangerFullAccess)
);
// Narrowing reaches it too: Plan drops shell and makes the sandbox read-only.
handle.publish_turn_authority(
AppMode::Plan,
true,
false,
false,
ApprovalMode::Suggest,
None,
);
after.refresh_live_posture();
assert!(!after.auto_approve);
assert_eq!(after.shell_policy, crate::worker_profile::ShellPolicy::None);
assert_eq!(
after.elevated_sandbox_policy,
Some(crate::sandbox::SandboxPolicy::ReadOnly)
);
}
/// "Gets stuck": an agent's approval answer only reached the agent when the
/// engine was idle or itself awaiting approval. While the parent turn
/// streamed or ran tools nobody read it, and the agent waited on. The handle
/// now hands it to the agent directly — here with no engine running at all.
#[tokio::test]
async fn an_agent_approval_answer_reaches_the_agent_while_the_engine_is_busy() {
let mock = mock_engine_handle();
let id = format!("agent:agent_busy:approval:{}", uuid::Uuid::new_v4());
let (_, waiting) = mock
.handle
.subagent_manager
.write()
.await
.register_child_approval("agent_busy", &id, "bash", "held")
.expect("register");
mock.handle.approve_tool_call(id).await.expect("send");
let outcome = tokio::time::timeout(std::time::Duration::from_secs(2), waiting)
.await
.expect("the waiting agent is answered without the engine")
.expect("answer");
assert_eq!(
outcome,
crate::tools::subagent::ChildApprovalOutcome::Approved
);
}
/// The regression this test exists for. A real DeepSeek turn — "build a
/// self-contained HTML focus timer, read a local fixture, verify it" — spent
/// its steps on five `tool_search` calls for `registry_sync`, a deferred-schema
/// retry, and reasoning about starting a browser MCP server, because the
/// always-visible instruction ordered Registry discovery *before* code
/// execution or a manual implementation and named two tools that are not in the
/// catalog head.
///
/// Two properties keep that from coming back, and both are about this prompt,
/// not about a second policy surface: discovery is never ordered ahead of
/// ordinary local work, and the deferred-tool cost of reaching the Registry
/// tools is stated where the model reads about them.
#[test]
fn registry_instruction_does_not_gate_ordinary_local_work() {
let (engine, _handle) = Engine::new(EngineConfig::default(), &Config::default());
@@ -25495,6 +25541,10 @@ fn engine_handle_try_send_does_not_block_when_op_channel_is_full() {
))),
compaction_cancellation: Arc::new(StdMutex::new(CompactionCancellationState::default())),
turn_heartbeat: turn_heartbeat::TurnHeartbeat::new(),
subagent_manager: crate::tools::subagent::new_shared_subagent_manager(
std::env::temp_dir(),
1,
),
};
// Fill the op channel with one message (capacity = 1).
+18 -6
View File
@@ -859,13 +859,25 @@ impl ToolContext {
self
}
/// Re-read the session's live permission posture (see `live_posture`).
#[must_use]
pub(crate) fn with_live_posture(mut self) -> Self {
if let Some(live) = self.live_posture.clone() {
live.apply(&mut self);
/// Re-read the session's live permission posture (see `live_posture`) and
/// return what was read; `None` when this context has no live source.
pub(crate) fn refresh_live_posture(
&mut self,
) -> Option<crate::core::engine::LiveRuntimeAuthority> {
let live = self.live_posture.clone()?;
Some(live.apply(self))
}
/// Resolves once the live posture differs from what it is now; never for
/// a context without a live source.
pub(crate) async fn live_posture_moved(&self) {
let Some(live) = self.live_posture.as_ref() else {
return std::future::pending().await;
};
let start = live.read();
while live.read() == start {
tokio::time::sleep(std::time::Duration::from_millis(250)).await;
}
self
}
/// Stamp tool work with the sub-agent that owns it.
+58 -21
View File
@@ -3203,8 +3203,6 @@ impl SubAgentRuntime {
#[must_use]
pub fn child_runtime(&self) -> Self {
let mut child_context = self.context.clone();
child_context.auto_approve = self.context.auto_approve;
child_context.approval_mode = self.context.approval_mode;
let cancel_token = self.cancel_token.child_token();
child_context.cancel_token = Some(cancel_token.clone());
Self {
@@ -18303,6 +18301,8 @@ enum ChildGateVerdict {
Proceed,
/// Refuse the call with this reason (returned to the child model).
Deny(String),
/// The person changed the posture while the gate waited; gate again.
PostureChanged,
}
impl SubAgentToolRegistry {
@@ -18468,6 +18468,17 @@ impl SubAgentToolRegistry {
}
}
/// Re-read the session's live posture onto this child's call context,
/// never wider than the child's own shell grant.
fn refresh_posture(
&self,
context: &mut ToolContext,
) -> Option<crate::core::engine::LiveRuntimeAuthority> {
let posture = context.refresh_live_posture();
context.set_shell_policy(context.shell_policy.min_with(self.grant.shell_policy()));
posture
}
/// Emit a transcript-visible receipt for a decision made on this child's
/// call without a person seeing a prompt (the audit log has the record).
#[allow(clippy::too_many_arguments)]
@@ -18572,12 +18583,15 @@ impl SubAgentToolRegistry {
return ChildGateVerdict::Deny(reason);
}
let workspace_trusted = crate::config::is_workspace_trusted(&workspace);
// Judged exactly like the parent's own call: destructive *detached*
// work holds in every posture, and nothing else is background.
let detached = self
.registry
.get(name)
.is_some_and(|spec| spec.starts_detached_for(input));
// Destructive work nobody is watching holds in every posture: a call
// that starts detached, or any call of an agent that runs detached
// (no foreground turn owns it). A foreground agent's call is judged
// exactly like the parent's own.
let detached = self.gate_runtime.foreground_children.is_none()
|| self
.registry
.get(name)
.is_some_and(|spec| spec.starts_detached_for(input));
let review_context = AutoReviewContext::from_tool_call(
name,
input,
@@ -18639,6 +18653,7 @@ impl SubAgentToolRegistry {
input,
&review_context,
&held_reason,
posture,
)
.await
}
@@ -18692,6 +18707,7 @@ impl SubAgentToolRegistry {
/// Auto-Review: ask the one-shot model guardian about a held call, using
/// the child's own session client, and turn its answer into a verdict
/// plus a transcript receipt. Any failure denies (fail closed).
#[allow(clippy::too_many_arguments)]
async fn consult_child_guardian(
&self,
agent_id: &str,
@@ -18700,6 +18716,7 @@ impl SubAgentToolRegistry {
input: &Value,
review_context: &crate::tui::auto_review::AutoReviewContext<'_>,
held_reason: &str,
posture: &ToolContext,
) -> ChildGateVerdict {
use crate::core::engine::reviewer::{ReviewerOutcome, consult_reviewer};
use crate::core::events::{ToolGate, ToolGateVerdict};
@@ -18713,12 +18730,22 @@ impl SubAgentToolRegistry {
.gate_runtime
.client
.effective_route_envelope(self.gate_runtime.client.model(), chrono::Utc::now());
let review = consult_reviewer(
&self.gate_runtime.client,
&context_text,
&self.gate_runtime.cancel_token,
)
.await;
record_agent_progress(
&self.gate_runtime,
agent_id,
AgentProgressEventMeta::new(AgentWorkerStatus::Running).with_tool(name.to_string()),
format!("Auto-Review checking '{name}'"),
);
// A posture change mid-review abandons it: the call is gated again
// under what the person just chose (Full Access runs it at once).
let review = tokio::select! {
review = consult_reviewer(
&self.gate_runtime.client,
&context_text,
&self.gate_runtime.cancel_token,
) => review,
() = posture.live_posture_moved() => return ChildGateVerdict::PostureChanged,
};
// A provider-success reply carries usage even when it is incomplete or
// semantically invalid. Record before interpreting the verdict so the
// fail-closed path cannot erase spend. Pre-dispatch cancellation and
@@ -19557,18 +19584,28 @@ impl SubAgentToolRegistry {
// Role posture and the execution envelope below stay authoritative:
// this gate can only decide whether a call the role permits also
// clears the session's approval boundary.
let context = self
let mut context = self
.registry
.context()
.clone()
.with_live_posture()
.with_owner_agent(self.owner_agent_id.clone(), self.owner_agent_name.clone())
.with_origin_tool_call_id(tool_id.to_string());
if let ChildGateVerdict::Deny(reason) = self
.gate_held_call(agent_id, tool_id, name, &input, &context)
.await
{
return Err(admission_denied(reason));
let mut posture = self.refresh_posture(&mut context);
loop {
let verdict = self
.gate_held_call(agent_id, tool_id, name, &input, &context)
.await;
if let ChildGateVerdict::Deny(reason) = verdict {
return Err(admission_denied(reason));
}
// A guardian or a person can take a while. Run under the posture
// as it is now: gate again if the person narrowed it meanwhile.
let now = self.refresh_posture(&mut context);
let narrowed = matches!((&now, &posture), (Some(now), Some(then)) if now.narrows(then));
posture = now;
if verdict == ChildGateVerdict::Proceed && !narrowed {
break;
}
}
reject_subagent_terminal_takeover(name, &input).map_err(as_denied)?;
if self.write_is_denied() {
+90 -8
View File
@@ -24682,7 +24682,9 @@ mod child_permission_gate {
/// parent's: only a genuinely detached start keeps the floor.
#[tokio::test]
async fn full_access_child_runs_what_the_parent_turn_runs() {
let (registry, mut rx, _) = worker_registry(ApprovalMode::Bypass, true, true, None);
let (mut registry, mut rx, _) = worker_registry(ApprovalMode::Bypass, true, true, None);
// A foreground agent: the parent turn owns and waits on it.
registry.gate_runtime.foreground_children = Some(Arc::new(ForegroundChildRegistry::new()));
let workspace = registry.gate_runtime.context.workspace.clone();
let build = workspace.join("build");
std::fs::create_dir_all(build.join("out")).unwrap();
@@ -24715,6 +24717,89 @@ mod child_permission_gate {
assert!(drain_gate_receipts(&mut rx).is_empty());
}
/// Review finding on d1655c424: judging agent calls as foreground let a
/// detached agent run catastrophic work. Nobody watches a detached agent,
/// so the floor holds for it in every posture, Full Access included.
#[tokio::test]
async fn a_detached_agents_catastrophic_shell_is_held_in_every_posture() {
for mode in [
ApprovalMode::Bypass,
ApprovalMode::Auto,
ApprovalMode::Suggest,
ApprovalMode::Never,
] {
let (registry, mut rx, _) = worker_registry(
mode,
mode == ApprovalMode::Bypass,
false,
Some(unreachable_client()),
);
assert!(registry.gate_runtime.foreground_children.is_none());
// Harmless if it ever ran; classified as a raw-device write.
let err = registry
.execute(
"agent_gate",
"bash",
json!({"command": "dd if=/dev/zero of=/dev/null count=0"}),
)
.await
.expect_err("a detached agent's device write is held");
// Ask turns the hold into a prompt no person can answer here; every
// other posture blocks it outright. Either way it never runs.
assert!(
err.to_string().contains("destructive background")
|| (mode == ApprovalMode::Suggest
&& err.to_string().contains("cannot raise a prompt")),
"{mode:?}: {err}"
);
let receipts = drain_gate_receipts(&mut rx);
assert!(
receipts
.iter()
.all(|receipt| receipt.0 == ToolGate::AutoReviewDeterministic),
"{mode:?}: the floor never reaches a guardian: {receipts:?}"
);
}
}
/// Review finding on d1655c424: the posture was read once, before the
/// gate. A person who tightened Permissions while an agent's prompt was
/// open still had that call run under the old posture once approved.
#[tokio::test]
async fn a_posture_narrowed_during_a_prompt_regates_the_call() {
let live = crate::core::engine::LivePosture::for_tests(
&std::env::temp_dir(),
ApprovalMode::Suggest,
);
let (registry, mut rx, manager) = worker_registry_with_live_posture(
ApprovalMode::Suggest,
false,
true,
None,
Some(live.clone()),
);
let call = registry.execute("agent_gate", "bash", json!({"command": "echo held | cat"}));
let answer = async {
let id = loop {
match rx.recv().await.expect("event stream") {
Event::ApprovalRequired { id, .. } => break id,
_ => continue,
}
};
// The person tightens Permissions, then answers the open card.
live.switch_for_tests(ApprovalMode::Never);
assert!(
manager
.write()
.await
.resolve_child_approval(&id, ChildApprovalOutcome::Approved)
);
};
let (result, ()) = tokio::join!(call, answer);
let err = result.expect_err("the call is gated again under the narrower posture");
assert!(err.to_string().contains("requires approval"), "{err}");
}
#[tokio::test]
async fn full_access_runs_ordinary_shell_but_still_hard_blocks_the_safety_floor() {
let (registry, mut rx, _) = worker_registry(ApprovalMode::Bypass, true, true, None);
@@ -24724,14 +24809,11 @@ mod child_permission_gate {
.expect("Full Access runs ordinary shell without a prompt");
assert!(output.contains("full-access"), "{output}");
assert!(drain_gate_receipts(&mut rx).is_empty());
// Destructive *detached* work holds in every posture, exactly as it
// does for a detached parent start, so Full Access fails closed here.
// This harness agent is detached (no foreground turn owns it), and
// destructive detached work holds in every posture, exactly as it
// does for a detached parent start: Full Access fails closed here.
let err = registry
.execute(
"agent_gate",
"Bash",
json!({"command": "rm -rf /usr", "background": true}),
)
.execute("agent_gate", "bash", json!({"command": "rm -rf /usr"}))
.await
.expect_err("destructive background shell stays blocked in Full Access");
assert!(
+6 -1
View File
@@ -58,7 +58,12 @@ pub fn classify_stakes(
return ApprovalStakes::Routine;
}
let semantic_name = canonical_action_alias(tool_name, params);
match crate::tui::auto_review::ToolActionKind::from_tool_call(semantic_name, params, category) {
match crate::tui::auto_review::ToolActionKind::from_tool_call(
semantic_name,
params,
category,
None,
) {
crate::tui::auto_review::ToolActionKind::Publish
| crate::tui::auto_review::ToolActionKind::Destructive => ApprovalStakes::Critical,
_ => ApprovalStakes::Elevated,
+80 -10
View File
@@ -87,11 +87,18 @@ impl ToolActionKind {
#[must_use]
pub fn from_tool_name(tool_name: &str, category: ToolCategory) -> Self {
Self::from_tool_call(tool_name, &Value::Null, category)
Self::from_tool_call(tool_name, &Value::Null, category, None)
}
/// `workspace`, when known, lets a forced delete of a path inside it stay
/// ordinary work instead of a catastrophic system-path delete.
#[must_use]
pub fn from_tool_call(tool_name: &str, params: &Value, category: ToolCategory) -> Self {
pub fn from_tool_call(
tool_name: &str,
params: &Value,
category: ToolCategory,
workspace: Option<&std::path::Path>,
) -> Self {
let qualified = action_qualified_tool_name(tool_name, params);
let normalized = qualified.to_ascii_lowercase();
let normalized = normalized.as_str();
@@ -128,7 +135,9 @@ impl ToolActionKind {
if matches!(category, ToolCategory::Shell) && shell_params_are_publish_like(params) {
return Self::Publish;
}
if matches!(category, ToolCategory::Shell) && shell_params_are_destructive_like(params) {
if matches!(category, ToolCategory::Shell)
&& shell_params_are_destructive_like(params, workspace)
{
return Self::Destructive;
}
@@ -425,7 +434,7 @@ impl<'a> AutoReviewContext<'a> {
} else {
classify_risk(tool_name, category, params)
};
let action_kind = ToolActionKind::from_tool_call(tool_name, params, category);
let action_kind = ToolActionKind::from_tool_call(tool_name, params, category, workspace);
Self {
tool_name,
category,
@@ -1017,7 +1026,7 @@ fn shell_params_are_publish_like(params: &Value) -> bool {
/// path). This is what keeps the background/headless durable-review floor
/// armed now that the floor no longer treats every non-read-only command as
/// destructive (#3883).
fn shell_params_are_destructive_like(params: &Value) -> bool {
fn shell_params_are_destructive_like(params: &Value, workspace: Option<&std::path::Path>) -> bool {
let Some(command) = params
.get("command")
.or_else(|| params.get("cmd"))
@@ -1031,7 +1040,7 @@ fn shell_params_are_destructive_like(params: &Value) -> bool {
.any(|segment| {
codewhale_execpolicy::command_safety::analyze_command(segment).level
== codewhale_execpolicy::command_safety::SafetyLevel::Dangerous
|| segment_is_device_or_filesystem_destroyer(segment)
|| segment_is_device_or_filesystem_destroyer(segment, workspace)
})
}
@@ -1042,12 +1051,15 @@ fn shell_params_are_destructive_like(params: &Value) -> bool {
/// `mkfs`, and forced recursive deletion of an absolute system path — so a
/// background/headless call in YOLO cannot run them without durable review
/// (#3883 follow-up; the earlier narrowing lost this coverage).
fn segment_is_device_or_filesystem_destroyer(segment: &str) -> bool {
fn segment_is_device_or_filesystem_destroyer(
segment: &str,
workspace: Option<&std::path::Path>,
) -> bool {
// A command may be piped (`cat x | dd of=/dev/sda`); each stage is its own
// effective command, so check every pipe stage.
segment
.split('|')
.any(stage_is_device_or_filesystem_destroyer)
.any(|stage| stage_is_device_or_filesystem_destroyer(stage, workspace))
}
/// Strip a surrounding pair of single or double quotes from a shell token so
@@ -1121,7 +1133,10 @@ fn effective_command_tokens<'a>(tokens: &'a [&'a str]) -> &'a [&'a str] {
&tokens[i..]
}
fn stage_is_device_or_filesystem_destroyer(stage: &str) -> bool {
fn stage_is_device_or_filesystem_destroyer(
stage: &str,
workspace: Option<&std::path::Path>,
) -> bool {
let raw_tokens: Vec<&str> = stage.split_whitespace().collect();
let tokens = effective_command_tokens(&raw_tokens);
let Some(cmd) = tokens
@@ -1162,7 +1177,7 @@ fn stage_is_device_or_filesystem_destroyer(stage: &str) -> bool {
} else if let Some(flags) = token.strip_prefix('-') {
recursive |= flags.contains('r') || flags.contains('R');
force |= flags.contains('f');
} else if token.starts_with('/') {
} else if token.starts_with('/') && !strictly_inside(workspace, token) {
abs_system_target = true;
}
}
@@ -1171,6 +1186,28 @@ fn stage_is_device_or_filesystem_destroyer(stage: &str) -> bool {
false
}
/// Whether absolute `target` resolves strictly below `workspace` — the
/// workspace root itself, a `..` escape, or a symlink hop out of it do not.
fn strictly_inside(workspace: Option<&std::path::Path>, target: &str) -> bool {
use crate::tools::spec::normalize_path;
let Some(workspace) = workspace else {
return false;
};
let target = std::path::Path::new(target);
let lexical = normalize_path(target);
let resolved = target.canonicalize().unwrap_or_else(|_| lexical.clone());
let roots = [
normalize_path(workspace),
workspace.canonicalize().unwrap_or_default(),
];
let below = |path: &std::path::Path| {
roots
.iter()
.any(|root| !root.as_os_str().is_empty() && path.starts_with(root) && path != root)
};
below(&lexical) && below(&resolved)
}
fn shell_tokens_are_publish_like(tokens: &[&str]) -> bool {
if git_tag_tokens_are_publish_like(tokens) {
return true;
@@ -1693,6 +1730,39 @@ mod tests {
}
}
/// A forced delete of an absolute path inside the workspace is ordinary
/// cleanup, not a system-tree destroyer. The workspace root itself, a
/// `..` escape, a symlink hop out, and a system path all still hold.
#[test]
fn absolute_forced_delete_inside_the_workspace_is_not_a_destroyer() {
let workspace = tempfile::tempdir().expect("tempdir");
let root = workspace.path();
std::fs::create_dir_all(root.join("build")).unwrap();
#[cfg(unix)]
std::os::unix::fs::symlink("/usr", root.join("escape")).unwrap();
let held = |command: String| {
let ctx = AutoReviewContext::from_tool_call(
"exec_shell",
&json!({ "command": command, "background": true }),
RunOrigin::Background,
ApprovalMode::Bypass,
true,
Some(root),
);
AutoReviewPolicy::default()
.evaluate(&ctx)
.built_in_safety_gate
};
let at = |rel: &str| root.join(rel).display().to_string();
assert!(!held(format!("rm -rf {}", at("build"))));
assert!(!held(format!("rm -rf {}", at("not/yet/there"))));
assert!(held(format!("rm -rf {}", root.display())));
assert!(held(format!("rm -rf {}", at("build/../.."))));
#[cfg(unix)]
assert!(held(format!("rm -rf {}/", at("escape"))));
assert!(held("rm -rf /usr".to_string()));
}
#[test]
fn ordinary_dd_and_workspace_rm_do_not_trip_the_destroyer_check() {
let policy = AutoReviewPolicy::default();