Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/openhuman/agent/turn_origin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,34 @@ mod tests {
assert!(observed.is_none());
}

/// The `hosted::orchestration::effect_executor::run_local_agent` spawn site
/// (#5508 / #5499): the device-tool bridge fires the local sub-agent from a
/// bare `tokio::spawn` where there is **no ambient turn** to inherit — unlike
/// the four sites PR #5465 fixed with `capture`/`propagate`, `capture()` here
/// is `None`, so `with_inherited_origin` would leave the task `Unknown` and
/// the gate would refuse every external-effect tool (`cron_add`, shell, …).
/// The fix scopes an **explicit** `Cli` origin on the spawned task instead
/// (device automation past the Master-chat gate is trusted, turn-less
/// internal dispatch). This pins that shape: nothing to inherit on the
/// parent, a real `Cli` origin observed across the spawn boundary.
#[tokio::test]
async fn explicit_cli_origin_survives_a_turnless_spawn() {
// No outer scope — exactly the device-tool bridge's situation.
assert!(
capture().is_none(),
"precondition: the effect_executor spawn has no ambient origin to inherit"
);

let observed = tokio::spawn(with_origin(AgentTurnOrigin::Cli, async { current() }))
.await
.expect("spawned task panicked");

assert!(
matches!(observed, Some(AgentTurnOrigin::Cli)),
"the explicitly-scoped Cli origin must be visible on the spawned task, got {observed:?}"
);
}

#[tokio::test]
async fn current_returns_none_outside_scope() {
assert!(current().is_none());
Expand Down
55 changes: 44 additions & 11 deletions src/openhuman/hosted/orchestration/effect_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,44 @@ async fn run_local_agent(args: &Value, cycle_id: &str) -> Result<Value, String>
let task_id = cycle_id.to_string();
let run_args = args.clone();
let bg_task_id = task_id.clone();
tokio::spawn(async move {
if let Err(e) =
run_local_agent_and_forward(&counterpart, &session_id, &bg_task_id, &agent_id, run_args)
.await
{
log::warn!(
target: LOG,
"[orchestration] run_local_agent.forward_failed task={bg_task_id}: {e}"
);
}
});
// Scope an explicit turn origin around the spawned sub-agent work.
//
// This runs on a fresh `tokio::spawn` task with no agent turn on the stack,
// and `AGENT_TURN_ORIGIN` is a `tokio::task_local` that does NOT cross
// `tokio::spawn` — so `turn_origin::current()` here is `None`, the approval
// gate reads `AgentTurnOrigin::Unknown`, and every external_effect tool the
// sub-agent calls (cron_add / cron_update / shell / …) is refused as "no
// origin label" (issues #5508, #5499). PR #5465 fixed the four canonical
// spawn sites via capture/propagate; this is the residual one.
//
// Unlike those sites there is no ambient origin to inherit (the device-tool
// bridge is not itself an agent turn), so `with_inherited_origin(None, …)`
// would stay `Unknown` and keep failing closed. We instead scope an explicit
// `AgentTurnOrigin::Cli`: this only runs after the device-authoritative
// Master-chat gate in `dispatch_device_tool` has passed (a compromised cloud
// brain cannot reach here for an A2A cycle), so it is trusted, turn-less,
// device-initiated internal dispatch — exactly the label `mcp::registry`'s
// credential-help turn and `task_dispatcher` use for the same shape.
let bg = crate::openhuman::agent::turn_origin::with_origin(
crate::openhuman::agent::turn_origin::AgentTurnOrigin::Cli,
async move {
if let Err(e) = run_local_agent_and_forward(
&counterpart,
&session_id,
&bg_task_id,
&agent_id,
run_args,
)
.await
{
log::warn!(
target: LOG,
"[orchestration] run_local_agent.forward_failed task={bg_task_id}: {e}"
);
}
},
);
tokio::spawn(bg);
Ok(json!({
"accepted": true,
"taskId": task_id,
Expand All @@ -190,6 +217,12 @@ async fn run_local_agent(args: &Value, cycle_id: &str) -> Result<Value, String>
/// sub-agent; without it the nested spawn failed `NoParentContext`
/// ("spawn_async_subagent called outside of an agent turn").
///
/// The `AGENT_TURN_ORIGIN` task-local is a **separate** axis and is scoped by
/// [`run_local_agent`] around the spawn (as [`AgentTurnOrigin::Cli`]), not here —
/// `turn_origin.rs` forbids manufacturing a label inside `run_subagent`. Without
/// that outer scope the sub-agent's external_effect tools (cron_add / shell / …)
/// would reach the approval gate as `Unknown` and be refused (#5508, #5499).
///
/// We call `run_subagent` (synchronous, real `output`) rather than the
/// `spawn_async_subagent` tool wrapper on purpose: the wrapper defaults to the
/// async path (returning a `[async_subagent_ref]`, not the answer) and gates on
Expand Down
72 changes: 68 additions & 4 deletions src/openhuman/security/approval/gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,9 +833,14 @@ impl ApprovalGate {
return (
GateOutcome::Deny {
reason: format!(
"{POLICY_DENIED_MARKER} Tool '{tool_name}' rejected: agent turn has \
no origin label. Refusing external_effect tool from unlabelled call \
site."
"{POLICY_DENIED_MARKER} '{tool_name}' was blocked because this agent \
turn is missing its origin label, so the approval gate cannot decide \
who requested the action. Scheduling and other external-effect tools \
(e.g. cron_add / cron_update) are refused when the turn has no origin. \
This is an internal wiring gap, not something you did — the work most \
likely ran on a background task that did not carry the turn's origin \
forward; retry from a normal chat turn, or report it so the spawn site \
can be fixed."
),
},
None,
Expand Down Expand Up @@ -2155,7 +2160,15 @@ mod tests {
.await;

match outcome {
GateOutcome::Deny { reason } => assert!(reason.contains("no origin label")),
// The deny message is specific and actionable (issues #5508 / #5499,
// 2nd acceptance criterion): it names the missing origin label, calls
// out the scheduling/external-effect tools it affects, and frames it
// as an internal wiring gap rather than user error.
GateOutcome::Deny { reason } => {
assert!(reason.contains("origin label"), "reason was: {reason}");
assert!(reason.contains("cron_add"), "reason was: {reason}");
assert!(reason.contains("external-effect"), "reason was: {reason}");
}
other => panic!("expected deny, got {other:?}"),
}
}
Expand Down Expand Up @@ -2932,6 +2945,57 @@ mod tests {
assert!(matches!(outcome, GateOutcome::Allow));
}

/// Regression for #5508 / #5499: an external-effect scheduling tool
/// (`cron_add`) that runs on a freshly-spawned, turn-less task — the exact
/// shape of `hosted::orchestration::effect_executor::run_local_agent`, which
/// fires the local sub-agent from a bare `tokio::spawn` with no agent turn on
/// the stack — must NOT be `Unknown`-denied once the spawn site scopes an
/// explicit `AgentTurnOrigin::Cli` (the residual site PR #5465 did not cover).
///
/// Both halves run inside a `tokio::spawn` so the assertion exercises the real
/// task boundary the fix crosses: `AGENT_TURN_ORIGIN` is a `tokio::task_local`
/// that does not survive `spawn`, so the origin the gate reads is whatever the
/// spawned future scopes for itself — nothing, or the fix's explicit label.
#[tokio::test]
async fn cron_add_on_a_turnless_spawn_resolves_to_a_real_origin_not_unknown_denied() {
let (gate, _dir) = test_gate();
let gate = Arc::new(gate);

// Precondition — mirrors the bug before the fix: a bare `tokio::spawn`
// with no ambient origin (capture() would yield None) reaches the gate as
// `Unknown`, and the scheduling tool is refused as "no origin label".
let g = gate.clone();
let denied = tokio::spawn(async move {
g.intercept("cron_add", "schedule a job", serde_json::json!({}))
.await
})
.await
.expect("spawned task panicked");
match denied {
GateOutcome::Deny { reason } => {
assert!(reason.contains("origin label"), "reason was: {reason}")
}
other => panic!("unlabelled turn-less spawn must fail closed, got {other:?}"),
}

// With the fix: `run_local_agent` scopes an explicit `Cli` origin around
// the spawned sub-agent work, so the same `cron_add` call now resolves to
// a real origin and is allowed (device-tool automation past the
// Master-chat gate) instead of being denied as unlabelled.
let g = gate.clone();
let allowed = tokio::spawn(turn_origin::with_origin(AgentTurnOrigin::Cli, async move {
g.intercept("cron_add", "schedule a job", serde_json::json!({}))
.await
}))
.await
.expect("spawned task panicked");
assert!(
matches!(allowed, GateOutcome::Allow),
"an explicit Cli origin scoped across the spawn must resolve cron_add \
to a real origin and allow it, got {allowed:?}"
);
}

#[tokio::test]
async fn intercept_with_external_channel_origin_persists_and_ttl_denies() {
// Non-web channel inbound (Telegram / Discord / Slack / etc.):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1371,7 +1371,7 @@ async fn approval_rpc_decision_paths_persist_always_allow_and_recent_audit() {
match &no_chat.0 {
openhuman_core::openhuman::security::approval::GateOutcome::Deny { reason } => {
assert!(
reason.contains("no origin label"),
reason.contains("origin label"),
"unlabelled call should be denied for missing origin: {reason}"
);
}
Expand Down
Loading