diff --git a/CHANGELOG.md b/CHANGELOG.md index 02d1d0891..16adf6825 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -175,6 +175,39 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). call site shared by both the startup catalog emit and every hot-reload — so the warning no longer depends on `SemanticMemory` being configured and fires at startup, not only after the first post-startup file change. +- `zeph-core`, `zeph-tools`, `zeph-common`, `zeph-subagent`: a skill that merely scored above + the matcher's similarity threshold this turn — never explicitly invoked by the model — could + drop the entire turn's `effective_trust` to `Quarantined` via the weakest-link fold in + `apply_skill_trust_and_gating`, denying every `bash`/`write`/MCP tool call for the rest of the + turn and every foreground-spawned subagent (issue #6701, security P1). Root causes and fixes: + - **RC-1**: `active_skill_names` was assigned directly from the matcher's output with no trust + filter. Added a trust-aware activation filter (`Agent::filter_active_skills_by_trust`) that + drops `Quarantined`/`Blocked` matches from the active set before the fold runs, while keeping + them visible in the `` catalog (`format_skills_catalog` now takes a trust-level + map and annotates entries with `trust="quarantined"`/`trust="blocked"`) so the model/operator + can still name them and promote with `zeph skill trust trusted`. A `tracing::warn!` + names the dropped skill and the promotion command. + - **RC-2**: retrieval-fallback mode (matcher unavailable or unconfigured) previously folded + trust over the full, unscored skill set even though only description-only catalog text is + injected in that mode. `apply_skill_trust_and_gating`'s effective-trust computation + (`compute_effective_trust`) now forces `Trusted` whenever `skill_fallback_mode` is set, + regardless of how many Quarantined/Blocked skills exist in the registry. + - **RC-3**: `SkillTrustGate::resolve_body` refused only `Blocked` skills — an explicit + `invoke_skill`/`load_skill` of a `Quarantined` skill returned its sanitized body but never + degraded the turn's trust for subsequent tool calls, contradicting the documented invariant. + It now folds the new `zeph_common::TurnTrustFloor` (a monotonic-downgrade-only shared cell, + `set`/`fold`/`get`) to `Quarantined` whenever it returns a Quarantined body. + - **RC-5**: subagent spawn/resume applied an inherited trust cap via a plain + `set_effective_trust(cap)`, which could restore trust above a floor already folded lower + earlier in the same task. `SpawnContext` gained a `turn_trust_floor` handle + (`Option`), and spawn/resume now calls `TurnTrustFloor::fold(cap)` on it when + present, falling back to the old `set_effective_trust` behavior otherwise. + - The weakest-link fold itself is unchanged as defense-in-depth for skills actually + injected/invoked this turn, and is now a single shared helper (`fold_weakest_trust`) used by + both `apply_skill_trust_and_gating` and the subagent trust-cap computation + (`parent_effective_trust_level`), which previously duplicated the same logic. + - RC-4 (AutoSkill draft-name collisions with native tool IDs) is tracked separately in #6702 + and intentionally out of scope here. - `zeph-channels`: `MAX_RETRY_SECS` (the upper bound `send_with_retry` clamps a `Retry-After` delay to) had no compile-time invariant guard (issue #6517). #6516 (closing #6496) filtered diff --git a/crates/zeph-common/src/lib.rs b/crates/zeph-common/src/lib.rs index f5fcb6cf4..240848c5d 100644 --- a/crates/zeph-common/src/lib.rs +++ b/crates/zeph-common/src/lib.rs @@ -44,6 +44,7 @@ pub mod text; pub mod timestamp; pub mod tool_classification; pub mod trust_level; +pub mod turn_trust_floor; pub mod types; /// Prefix embedded in tool output bodies when the full output was stored externally. @@ -65,6 +66,7 @@ pub use task_supervisor::{ }; pub use text::format_tokens; pub use trust_level::SkillTrustLevel; +pub use turn_trust_floor::TurnTrustFloor; pub use types::{ ProviderName, SessionId, SessionIdError, SkillName, StopHint, ToolDefinition, ToolName, }; diff --git a/crates/zeph-common/src/trust_level.rs b/crates/zeph-common/src/trust_level.rs index acbb0cc2e..7f0401224 100644 --- a/crates/zeph-common/src/trust_level.rs +++ b/crates/zeph-common/src/trust_level.rs @@ -103,6 +103,30 @@ impl SkillTrustLevel { } } + /// Inverse of [`severity`](Self::severity): reconstructs a level from its ordinal. + /// + /// Any value `>= 3` maps to [`Blocked`](Self::Blocked) — the most restrictive level — + /// so a corrupted or out-of-range stored ordinal fails closed rather than open. + /// + /// # Examples + /// + /// ```rust + /// use zeph_common::SkillTrustLevel; + /// + /// assert_eq!(SkillTrustLevel::from_severity(0), SkillTrustLevel::Trusted); + /// assert_eq!(SkillTrustLevel::from_severity(3), SkillTrustLevel::Blocked); + /// assert_eq!(SkillTrustLevel::from_severity(255), SkillTrustLevel::Blocked); + /// ``` + #[must_use] + pub const fn from_severity(v: u8) -> Self { + match v { + 0 => Self::Trusted, + 1 => Self::Verified, + 2 => Self::Quarantined, + _ => Self::Blocked, + } + } + /// Returns the string representation used for database storage. #[must_use] pub const fn as_str(self) -> &'static str { @@ -217,4 +241,25 @@ mod tests { SkillTrustLevel::Verified ); } + + #[test] + fn from_severity_round_trips_through_severity() { + for level in [ + SkillTrustLevel::Trusted, + SkillTrustLevel::Verified, + SkillTrustLevel::Quarantined, + SkillTrustLevel::Blocked, + ] { + assert_eq!(SkillTrustLevel::from_severity(level.severity()), level); + } + } + + #[test] + fn from_severity_out_of_range_fails_closed_to_blocked() { + assert_eq!(SkillTrustLevel::from_severity(4), SkillTrustLevel::Blocked); + assert_eq!( + SkillTrustLevel::from_severity(255), + SkillTrustLevel::Blocked + ); + } } diff --git a/crates/zeph-common/src/turn_trust_floor.rs b/crates/zeph-common/src/turn_trust_floor.rs new file mode 100644 index 000000000..04edc5822 --- /dev/null +++ b/crates/zeph-common/src/turn_trust_floor.rs @@ -0,0 +1,150 @@ +// SPDX-FileCopyrightText: 2026 Andrei G +// SPDX-License-Identifier: MIT OR Apache-2.0 + +//! Shared, monotonically-downgradable trust floor for a single agent turn (#6701). + +use std::sync::Arc; +use std::sync::atomic::{AtomicU8, Ordering}; + +use crate::SkillTrustLevel; + +/// Per-turn trust floor shared between a `ToolExecutor`'s trust gate and any skill-body +/// resolution path that can downgrade it mid-turn (e.g. an explicit `invoke_skill` of a +/// Quarantined skill). +/// +/// Wraps an `Arc` — cloning shares the same underlying cell, so every holder +/// observes the same value. Two operations mutate it: +/// +/// - [`set`](Self::set): turn-start assignment. Replaces the floor unconditionally — trust +/// may go up or down relative to the previous turn. Call once per turn, before any tool +/// dispatch. +/// - [`fold`](Self::fold): monotonic downgrade. The floor becomes `min(current, level)` — +/// it can never raise trust. Models "this turn's trust degraded because +/// prompt-injected/quarantined content was actually read" — the only way back up is a +/// fresh [`set`](Self::set) at the next turn boundary. +/// +/// # Examples +/// +/// ```rust +/// use zeph_common::{SkillTrustLevel, TurnTrustFloor}; +/// +/// let floor = TurnTrustFloor::new(SkillTrustLevel::Trusted); +/// floor.fold(SkillTrustLevel::Quarantined); +/// assert_eq!(floor.get(), SkillTrustLevel::Quarantined); +/// +/// // fold never raises trust, even toward a higher-trust argument. +/// floor.fold(SkillTrustLevel::Trusted); +/// assert_eq!(floor.get(), SkillTrustLevel::Quarantined); +/// +/// // set is a full turn-start reset — it can raise trust again. +/// floor.set(SkillTrustLevel::Trusted); +/// assert_eq!(floor.get(), SkillTrustLevel::Trusted); +/// ``` +#[derive(Clone, Debug)] +pub struct TurnTrustFloor(Arc); + +impl TurnTrustFloor { + /// Creates a new floor initialized to `initial`. + #[must_use] + pub fn new(initial: SkillTrustLevel) -> Self { + Self(Arc::new(AtomicU8::new(initial.severity()))) + } + + /// Turn-start assignment: replaces the floor unconditionally. + /// + /// Call once per turn, before any tool dispatch — never mid-turn, or a genuine + /// mid-turn downgrade (see [`fold`](Self::fold)) could be silently undone. + pub fn set(&self, level: SkillTrustLevel) { + self.0.store(level.severity(), Ordering::Relaxed); + } + + /// Monotonic downgrade: the floor becomes `min(current, level)`. + /// + /// Can never raise trust. The `Result` from the underlying CAS is intentionally + /// discarded — the closure always returns `Some`, so the update always succeeds. + pub fn fold(&self, level: SkillTrustLevel) { + let _ = self + .0 + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |cur| { + let current = SkillTrustLevel::from_severity(cur); + Some(current.min_trust(level).severity()) + }); + } + + /// Returns the current floor value. + #[must_use] + pub fn get(&self) -> SkillTrustLevel { + SkillTrustLevel::from_severity(self.0.load(Ordering::Relaxed)) + } +} + +impl Default for TurnTrustFloor { + /// Defaults to [`SkillTrustLevel::Trusted`] — the same starting point + /// `TrustGateExecutor::new` used before this type existed. + fn default() -> Self { + Self::new(SkillTrustLevel::Trusted) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn set_replaces_unconditionally_in_either_direction() { + let floor = TurnTrustFloor::new(SkillTrustLevel::Quarantined); + floor.set(SkillTrustLevel::Trusted); + assert_eq!(floor.get(), SkillTrustLevel::Trusted); + floor.set(SkillTrustLevel::Blocked); + assert_eq!(floor.get(), SkillTrustLevel::Blocked); + } + + #[test] + fn fold_lowers_trust() { + let floor = TurnTrustFloor::new(SkillTrustLevel::Trusted); + floor.fold(SkillTrustLevel::Quarantined); + assert_eq!(floor.get(), SkillTrustLevel::Quarantined); + } + + #[test] + fn fold_never_raises_trust() { + let floor = TurnTrustFloor::new(SkillTrustLevel::Quarantined); + floor.fold(SkillTrustLevel::Trusted); + assert_eq!( + floor.get(), + SkillTrustLevel::Quarantined, + "fold must never raise trust above the current floor" + ); + } + + #[test] + fn fold_to_blocked_is_sticky_until_a_fresh_set() { + let floor = TurnTrustFloor::new(SkillTrustLevel::Verified); + floor.fold(SkillTrustLevel::Blocked); + floor.fold(SkillTrustLevel::Trusted); + assert_eq!(floor.get(), SkillTrustLevel::Blocked); + floor.set(SkillTrustLevel::Trusted); + assert_eq!( + floor.get(), + SkillTrustLevel::Trusted, + "only a fresh turn-start set recovers trust, never fold" + ); + } + + #[test] + fn clone_shares_the_same_underlying_cell() { + let floor = TurnTrustFloor::new(SkillTrustLevel::Trusted); + let clone = floor.clone(); + clone.fold(SkillTrustLevel::Quarantined); + assert_eq!( + floor.get(), + SkillTrustLevel::Quarantined, + "clones must observe writes through any other clone" + ); + } + + #[test] + fn default_starts_trusted() { + assert_eq!(TurnTrustFloor::default().get(), SkillTrustLevel::Trusted); + } +} diff --git a/crates/zeph-core/src/agent/builder.rs b/crates/zeph-core/src/agent/builder.rs index 6ec53f675..cc677fbef 100644 --- a/crates/zeph-core/src/agent/builder.rs +++ b/crates/zeph-core/src/agent/builder.rs @@ -501,6 +501,21 @@ impl Agent { self } + /// Wire the shared per-turn trust floor (#6701) — the same handle + /// `zeph_tools::TrustGateExecutor::trust_floor()` returns for the executor built into this + /// agent's `tool_executor` chain. + /// + /// Enables two things: `SkillTrustGate::resolve_body` folding the turn's trust down on a + /// Quarantined body read (wired separately via `SkillLoaderExecutor`/`SkillInvokeExecutor`'s + /// own `with_turn_trust_floor`, sharing this same `Arc`), and subagent spawn folding an + /// inherited trust cap onto this exact cell (`SpawnContext::turn_trust_floor`) instead of + /// overwriting it via `set_effective_trust`. + #[must_use] + pub fn with_turn_trust_floor(mut self, floor: zeph_common::TurnTrustFloor) -> Self { + self.services.skill.turn_trust_floor = Some(floor); + self + } + /// Configure skill matching parameters (disambiguation, two-stage, confusability). #[must_use] pub fn with_skill_matching_config( diff --git a/crates/zeph-core/src/agent/context/assembly.rs b/crates/zeph-core/src/agent/context/assembly.rs index f76e64862..984435bd2 100644 --- a/crates/zeph-core/src/agent/context/assembly.rs +++ b/crates/zeph-core/src/agent/context/assembly.rs @@ -19,6 +19,59 @@ use crate::channel::Channel; use crate::context::build_system_prompt_with_instructions; use tracing::Instrument as _; +/// Weakest-link trust fold (#6701, D2): folds [`SkillTrustLevel::min_trust`] over `names`, +/// each resolved via `trust_map`, defaulting to [`SkillTrustLevel::Trusted`] when `names` is +/// empty or a name has no trust-map entry (a missing entry means "never classified", not +/// "known untrusted" — see [`SkillTrustLevel::MISSING_ENTRY_FALLBACK`]). +/// +/// This is the single shared implementation of the fold applied to skills whose bodies were +/// ACTUALLY injected/invoked this turn (not merely matched — see [`apply_skill_trust_and_gating`]'s +/// own doc comment for why proactive matching alone is no longer sufficient input). Used by +/// [`compute_effective_trust`], which both [`apply_skill_trust_and_gating`] (the turn's own +/// `effective_trust` gate) and `subagent_commands::parent_effective_trust_level`'s +/// no-floor-wired fallback path (the cap propagated to a spawned sub-agent's +/// `SpawnContext::max_trust_level`) route through — so they can never compute a fold result +/// inconsistent with each other. `parent_effective_trust_level` prefers reading the live +/// `TurnTrustFloor` directly when one is wired (#6701, S1/S3) — this fold-based path is its +/// fallback only, and remains the source of truth for the parent's own gate either way. +/// +/// [`SkillTrustLevel::min_trust`]: zeph_common::SkillTrustLevel::min_trust +/// [`SkillTrustLevel::Trusted`]: zeph_common::SkillTrustLevel::Trusted +/// [`SkillTrustLevel::MISSING_ENTRY_FALLBACK`]: zeph_common::SkillTrustLevel::MISSING_ENTRY_FALLBACK +pub(crate) fn fold_weakest_trust<'a>( + names: impl Iterator, + trust_map: &std::collections::HashMap, +) -> zeph_common::SkillTrustLevel { + names + .filter_map(|name| trust_map.get(name).map(|s| s.trust_level)) + .fold(zeph_common::SkillTrustLevel::Trusted, |acc, lvl| { + acc.min_trust(lvl) + }) +} + +/// Computes this turn's `effective_trust` (#6701): [`fold_weakest_trust`] over `active_names`, +/// forced to [`SkillTrustLevel::Trusted`] when `active_names` is empty OR +/// `skill_fallback_mode` is `true`. +/// +/// The `skill_fallback_mode` guard is D4 (closing RC-2): retrieval-fallback mode injects +/// description-only catalog text for every registered skill with no body/trust consequence, so +/// `effective_trust` MUST remain `Trusted` regardless of how many Quarantined/Blocked skills +/// exist in the registry — before #6701, this mode could silently lock every turn to +/// `Quarantined` defending against content that was never placed in the prompt. +/// +/// [`SkillTrustLevel::Trusted`]: zeph_common::SkillTrustLevel::Trusted +pub(crate) fn compute_effective_trust( + skill_fallback_mode: bool, + active_names: &[String], + trust_map: &std::collections::HashMap, +) -> zeph_common::SkillTrustLevel { + if skill_fallback_mode || active_names.is_empty() { + zeph_common::SkillTrustLevel::Trusted + } else { + fold_weakest_trust(active_names.iter().map(String::as_str), trust_map) + } +} + // ── Security event sink adapter ─────────────────────────────────────────────── // // Wraps the metrics watch-channel sender so `ContextService::prepare_context` @@ -824,7 +877,7 @@ impl Agent { // `QueryEmbedCache` for the cache-state semantics. let mut query_embed_cache = QueryEmbedCache::default(); - let (matched_indices, skill_fallback_mode, skills_to_record) = self + let (matched_indices, skill_fallback_mode, mut skills_to_record) = self .match_and_rank_skills( query, effective_query, @@ -835,11 +888,37 @@ impl Agent { .await; let matched_indices = self.filter_skills_missing_secrets(&all_meta, matched_indices); + // #6701 (S1): persisted so subagent_commands::parent_effective_trust_level's + // no-floor-wired fallback path can apply the same D4 guard as the parent's own gate. + self.services.skill.skill_fallback_mode = skill_fallback_mode; + + // #6701 (D1): resolve this turn's trust map once, here, so the activation filter below + // can drop Quarantined/Blocked matches BEFORE `active_skill_names` is assigned — the + // weakest-link fold in `apply_skill_trust_and_gating` never sees them. The same map is + // passed forward to `apply_skill_trust_and_gating` so it isn't re-resolved (and the + // trust snapshot isn't written twice) later in this same turn. + let trust_map = self.resolve_trust_map().await; + let matched_indices = Self::filter_active_skills_by_trust( + &all_meta, + matched_indices, + &trust_map, + skill_fallback_mode, + ); + self.services.skill.active_skill_names = matched_indices .iter() .filter_map(|&i| all_meta.get(i).map(|m| m.name.clone())) .collect(); + // #6701 (S7): a skill the D1 filter above dropped from `active_skill_names` (Quarantined + // or Blocked) was never actually activated this turn, so it must not be recorded as + // usage or feed the confidence/RL metrics either — both would otherwise count a skill + // whose body was never injected. + Self::filter_skills_to_record( + &mut skills_to_record, + &self.services.skill.active_skill_names, + ); + let skill_names = self.services.skill.active_skill_names.clone(); let total = all_meta.len(); self.update_metrics(|m| { @@ -862,8 +941,13 @@ impl Agent { let (all_skills, active_skills, matched_indices) = self.load_and_filter_skills_by_channel(&all_meta, &matched_indices); - let (trust_map, remaining_skills) = self - .apply_skill_trust_and_gating(&all_skills, &active_skills) + let remaining_skills = self + .apply_skill_trust_and_gating( + &all_skills, + &active_skills, + &trust_map, + skill_fallback_mode, + ) .await; // Build health_map: skill_name -> (posterior_mean, total_uses) for XML attributes. @@ -892,7 +976,10 @@ impl Agent { if !erl_suffix.is_empty() { skills_prompt.push_str(&erl_suffix); } - let catalog_prompt = format_skills_catalog(&remaining_skills); + // #6701 (D1): annotate catalog entries with their trust level so a skill dropped from + // `active_skill_names` by the activation filter above remains nameable/promotable. + let catalog_trust_levels = crate::skill_invoker::snapshot_map_to_trust_levels(&trust_map); + let catalog_prompt = format_skills_catalog(&remaining_skills, &catalog_trust_levels); self.services .skill .last_skills_prompt @@ -1656,22 +1743,18 @@ impl Agent { (all_skills, active_skills, matched_indices) } - /// Resolves per-skill trust levels, writes the per-turn trust snapshot (so - /// `SkillInvokeExecutor` can resolve trust without re-querying the store on every - /// tool call), filters `all_skills` down to the non-active catalog skills allowed by - /// trust, gates the tool executor to the most restrictive trust level among - /// `active_skills`, and fires PASTE speculative activation (#3642). + /// Resolves this turn's skill trust map, writing the per-turn trust snapshot (so + /// `SkillInvokeExecutor` can resolve trust without re-querying the store on every tool + /// call) on a fresh load, or reusing the previous turn's snapshot on a load failure. /// - /// Returns `(trust_map, remaining_skills)` for use by the prompt-formatting step. - async fn apply_skill_trust_and_gating( + /// Shared by [`filter_active_skills_by_trust`](Self::filter_active_skills_by_trust) (the D1 + /// activation filter, run before `active_skill_names` is assigned) and + /// [`apply_skill_trust_and_gating`](Self::apply_skill_trust_and_gating) (catalog filter + + /// weakest-link fold) — both callers this turn share the identical map from one DB read. + async fn resolve_trust_map( &mut self, - all_skills: &[Skill], - active_skills: &[Skill], - ) -> ( - std::collections::HashMap, - Vec, - ) { - let trust_map = match self.build_skill_trust_map().await { + ) -> std::collections::HashMap { + match self.build_skill_trust_map().await { crate::agent::trust_commands::SkillTrustMapLoad::Fresh(map) => { self.services.skill.trust_snapshot.write().clone_from(&map); map @@ -1679,8 +1762,9 @@ impl Agent { crate::agent::trust_commands::SkillTrustMapLoad::LoadFailed => { // Do NOT overwrite the persisted snapshot — leave it exactly as the previous // turn left it, and reuse it as this turn's local trust map too, so every - // downstream use (catalog filter, effective_trust fold, PASTE activation) - // sees the stale-but-real data instead of failing open to Trusted. + // downstream use (activation filter, catalog filter, effective_trust fold, + // PASTE activation) sees the stale-but-real data instead of failing open to + // Trusted. // // Residual: on the very first turn ever, `trust_snapshot` still holds its // `HashMap::new()` construction-time value, so a load failure on that one @@ -1688,13 +1772,101 @@ impl Agent { // fail-open-to-Trusted behavior). There is no "previous" state before the // first turn to fall back to — accepted per the issue's remediation scope. tracing::warn!( - "apply_skill_trust_and_gating: trust snapshot load failed, reusing \ - previous turn's snapshot (stale this turn)" + "resolve_trust_map: trust snapshot load failed, reusing previous turn's \ + snapshot (stale this turn)" ); self.services.skill.trust_snapshot.read().clone() } - }; + } + } + /// Trust-aware activation filter (#6701, D1): drops any matched index whose resolved + /// trust is `Quarantined`/`Blocked` from the active set BEFORE it is assigned to + /// `active_skill_names` — closing RC-1, where a skill merely scoring above the matcher's + /// similarity threshold (never explicitly invoked) could drop the whole turn's + /// `effective_trust` via the weakest-link fold in + /// [`apply_skill_trust_and_gating`](Self::apply_skill_trust_and_gating). + /// + /// Filtered skills are NOT lost — they remain visible in the `` catalog + /// (annotated `trust="quarantined"`/`trust="blocked"` by `format_skills_catalog`) so the + /// model can still name them to the operator, who can promote them with + /// `zeph skill trust trusted`. A skill missing from `trust_map` keeps the + /// `Trusted` fallback and is not filtered here. + /// + /// Skipped entirely when `skill_fallback_mode` is `true` (D4, closing RC-2): retrieval + /// fallback injects description-only text for every registered skill with no + /// body/trust consequence (see `match_and_rank_skills`'s doc comment), so filtering here + /// would only move a skill's description between prompt sections, never change what + /// enters the prompt or the weakest-link fold — the fold's own fallback-mode guard below + /// is the invariant that actually matters for D4. + fn filter_active_skills_by_trust( + all_meta: &[&SkillMeta], + matched_indices: Vec, + trust_map: &std::collections::HashMap, + skill_fallback_mode: bool, + ) -> Vec { + if skill_fallback_mode { + return matched_indices; + } + matched_indices + .into_iter() + .filter(|&i| { + let Some(meta) = all_meta.get(i) else { + return false; + }; + match trust_map.get(&meta.name) { + Some(snap) + if matches!( + snap.trust_level, + zeph_common::SkillTrustLevel::Quarantined + | zeph_common::SkillTrustLevel::Blocked + ) => + { + tracing::warn!( + skill = %meta.name, + trust = %snap.trust_level, + "skill matched but not activated this turn (trust={}); promote \ + with `zeph skill trust {} trusted` if this skill is safe", + snap.trust_level, + meta.name + ); + false + } + _ => true, + } + }) + .collect() + } + + /// Drops any name from `skills_to_record` that is not present in `active_names` (#6701, S7). + /// + /// `skills_to_record` is captured by `match_and_rank_skills` before the D1 activation filter + /// runs, so without this a Quarantined/Blocked skill the filter dropped from + /// `active_skill_names` would still be passed to `record_skill_usage` — polluting the + /// usage-count/confidence/RL-rerank learning signal with a skill whose body was never + /// actually injected this turn. + fn filter_skills_to_record(skills_to_record: &mut Vec, active_names: &[String]) { + let active_name_set: std::collections::HashSet<&str> = + active_names.iter().map(String::as_str).collect(); + skills_to_record.retain(|name| active_name_set.contains(name.as_str())); + } + + /// Filters `all_skills` down to the non-active catalog skills allowed by trust, gates the + /// tool executor to the most restrictive trust level among skills whose bodies were + /// actually injected this turn, and fires PASTE speculative activation (#3642). + /// + /// `trust_map` is the map [`resolve_trust_map`](Self::resolve_trust_map) already resolved + /// earlier this turn (before the D1 activation filter ran) — passed in rather than + /// re-resolved so the trust snapshot isn't written twice per turn. + /// + /// Returns `remaining_skills` for use by the prompt-formatting step. + async fn apply_skill_trust_and_gating( + &mut self, + all_skills: &[Skill], + active_skills: &[Skill], + trust_map: &std::collections::HashMap, + skill_fallback_mode: bool, + ) -> Vec { let remaining_skills: Vec = all_skills .iter() .filter(|s| { @@ -1714,35 +1886,36 @@ impl Agent { .cloned() .collect(); - // Deliberate weakest-link policy: fold the most restrictive trust level among ALL - // skills active this turn into a single `effective_trust` value applied to the - // executor gate (`TrustGateExecutor::set_effective_trust`). If ANY co-active skill is - // Quarantined, QUARANTINE_DENIED tools are denied for the WHOLE turn, regardless of - // which specific skill/tool a call targets — this prevents a Quarantined (potentially - // prompt-injected) skill's content from steering the model into invoking other - // tools/skills as a side channel. See #5729 for the resulting UX gap (an unrelated, + // Deliberate weakest-link policy: fold the most restrictive trust level among all + // skills active this turn (post-D1-filter — i.e. skills whose bodies were actually + // injected/invoked, never just proactively matched) into a single `effective_trust` + // value applied to the executor gate (`TrustGateExecutor::set_effective_trust`). If + // ANY co-active skill is Quarantined, QUARANTINE_DENIED tools are denied for the WHOLE + // turn, regardless of which specific skill/tool a call targets — this remains + // defense-in-depth against a Quarantined (potentially prompt-injected) skill's content + // steering the model into invoking other tools/skills as a side channel. D1 narrows + // this fold's *input* (Quarantined/Blocked matches never reach `active_skill_names`), + // it does not weaken the fold itself. See #5729 for the resulting UX gap (an unrelated, // non-quarantined skill's own `invoke_skill` call is also denied) and // `TrustGateExecutor::check_trust`'s doc comment for the matching rationale. - let effective_trust = if self.services.skill.active_skill_names.is_empty() { - zeph_common::SkillTrustLevel::Trusted - } else { - self.services - .skill - .active_skill_names - .iter() - .filter_map(|name| trust_map.get(name).map(|s| s.trust_level)) - .fold(zeph_common::SkillTrustLevel::Trusted, |acc, lvl| { - acc.min_trust(lvl) - }) - }; + // + // #6701 (D4): retrieval-fallback mode (`skill_fallback_mode`) injects description-only + // catalog text for every registered skill — no body enters the prompt, so nothing here + // should ever fold trust below Trusted, no matter how many Quarantined/Blocked skills + // exist in the registry. + let effective_trust = compute_effective_trust( + skill_fallback_mode, + &self.services.skill.active_skill_names, + trust_map, + ); self.tool_executor.set_effective_trust(effective_trust); // PASTE: rebuild tool→skill mapping and fire speculative dispatches. // Runs only when mode is Pattern or Both and PatternStore is initialized. - self.run_paste_skill_activation(active_skills, &trust_map) + self.run_paste_skill_activation(active_skills, trust_map) .await; - (trust_map, remaining_skills) + remaining_skills } /// Formats the `` prompt block for `active_skills`: dispatches @@ -1777,11 +1950,7 @@ impl Agent { { format_skills_prompt_compact(active_skills) } else { - let trust_levels: std::collections::HashMap = - trust_map - .iter() - .map(|(k, v)| (k.clone(), v.trust_level)) - .collect(); + let trust_levels = crate::skill_invoker::snapshot_map_to_trust_levels(trust_map); // GoSkills: experiment engine applies config overrides before context assembly, // so checking services.skill.group_structured here reflects any active A/B variation. @@ -2378,6 +2547,254 @@ mod tests { use zeph_context::assembler::{MAX_KEEP_TAIL_SCAN, memory_first_keep_tail}; use zeph_llm::provider::{Message, MessagePart, Role}; + // ── #6701: trust-aware skill activation and turn trust floor ──────────── + + fn trust_snapshot( + level: zeph_common::SkillTrustLevel, + ) -> crate::skill_invoker::SkillTrustSnapshot { + crate::skill_invoker::SkillTrustSnapshot { + trust_level: level, + requires_trust_check: false, + blake3_hash: String::new(), + } + } + + fn skill_meta_named(name: &str) -> SkillMeta { + SkillMeta { + name: name.to_owned(), + description: format!("{name} description"), + ..Default::default() + } + } + + // ── fold_weakest_trust (D2) ────────────────────────────────────────────── + + #[test] + fn fold_weakest_trust_empty_names_is_trusted() { + let trust_map = std::collections::HashMap::new(); + assert_eq!( + fold_weakest_trust(std::iter::empty(), &trust_map), + zeph_common::SkillTrustLevel::Trusted + ); + } + + #[test] + fn fold_weakest_trust_picks_least_trusted_across_names() { + let mut trust_map = std::collections::HashMap::new(); + trust_map.insert( + "a".to_string(), + trust_snapshot(zeph_common::SkillTrustLevel::Trusted), + ); + trust_map.insert( + "b".to_string(), + trust_snapshot(zeph_common::SkillTrustLevel::Quarantined), + ); + assert_eq!( + fold_weakest_trust(["a", "b"].into_iter(), &trust_map), + zeph_common::SkillTrustLevel::Quarantined, + "the least-trusted co-active skill must determine the fold result" + ); + } + + #[test] + fn fold_weakest_trust_missing_entry_defaults_to_trusted() { + let trust_map = std::collections::HashMap::new(); + assert_eq!( + fold_weakest_trust(["never-classified"].into_iter(), &trust_map), + zeph_common::SkillTrustLevel::Trusted, + "a name absent from trust_map must use the Trusted fallback, not be excluded" + ); + } + + // ── compute_effective_trust (D4, closing RC-2) ─────────────────────────── + + #[test] + fn compute_effective_trust_fallback_mode_stays_trusted_despite_quarantined_registry() { + let mut trust_map = std::collections::HashMap::new(); + trust_map.insert( + "quarantined-skill".to_string(), + trust_snapshot(zeph_common::SkillTrustLevel::Quarantined), + ); + let active_names = vec!["quarantined-skill".to_string()]; + + let result = compute_effective_trust(true, &active_names, &trust_map); + assert_eq!( + result, + zeph_common::SkillTrustLevel::Trusted, + "retrieval-fallback mode must never fold trust below Trusted (D4)" + ); + } + + #[test] + fn compute_effective_trust_non_fallback_mode_still_folds_quarantined() { + let mut trust_map = std::collections::HashMap::new(); + trust_map.insert( + "quarantined-skill".to_string(), + trust_snapshot(zeph_common::SkillTrustLevel::Quarantined), + ); + let active_names = vec!["quarantined-skill".to_string()]; + + let result = compute_effective_trust(false, &active_names, &trust_map); + assert_eq!( + result, + zeph_common::SkillTrustLevel::Quarantined, + "outside fallback mode, a genuinely active Quarantined skill must still fold down \ + (defense-in-depth is unchanged by D4)" + ); + } + + #[test] + fn compute_effective_trust_empty_active_names_is_trusted() { + let trust_map = std::collections::HashMap::new(); + assert_eq!( + compute_effective_trust(false, &[], &trust_map), + zeph_common::SkillTrustLevel::Trusted + ); + } + + #[test] + fn compute_effective_trust_mixed_trusted_and_quarantined_active_stays_quarantined() { + // Key invariant: "with one Trusted and one Quarantined skill matched in the same turn, + // effective_trust MUST remain Trusted" — no, per spec this invariant is about + // ACTIVATION (D1 drops Quarantined before this point). This test instead proves the + // fold's own weakest-link behavior is unchanged for names that DO reach it (D2's "never + // remove the fold for skills whose bodies were actually injected" invariant). + let mut trust_map = std::collections::HashMap::new(); + trust_map.insert( + "trusted-skill".to_string(), + trust_snapshot(zeph_common::SkillTrustLevel::Trusted), + ); + trust_map.insert( + "quarantined-skill".to_string(), + trust_snapshot(zeph_common::SkillTrustLevel::Quarantined), + ); + let active_names = vec!["trusted-skill".to_string(), "quarantined-skill".to_string()]; + + let result = compute_effective_trust(false, &active_names, &trust_map); + assert_eq!(result, zeph_common::SkillTrustLevel::Quarantined); + } + + // ── filter_active_skills_by_trust (D1, closing RC-1) ───────────────────── + + #[test] + fn filter_active_skills_by_trust_drops_quarantined_and_blocked_from_active_set() { + let trusted = skill_meta_named("trusted-skill"); + let quarantined = skill_meta_named("quarantined-skill"); + let blocked = skill_meta_named("blocked-skill"); + let unclassified = skill_meta_named("unclassified-skill"); + let all_meta: Vec<&SkillMeta> = vec![&trusted, &quarantined, &blocked, &unclassified]; + + let mut trust_map = std::collections::HashMap::new(); + trust_map.insert( + "trusted-skill".to_string(), + trust_snapshot(zeph_common::SkillTrustLevel::Trusted), + ); + trust_map.insert( + "quarantined-skill".to_string(), + trust_snapshot(zeph_common::SkillTrustLevel::Quarantined), + ); + trust_map.insert( + "blocked-skill".to_string(), + trust_snapshot(zeph_common::SkillTrustLevel::Blocked), + ); + // "unclassified-skill" intentionally absent — missing entry keeps the Trusted fallback + // and must NOT be filtered. + + let filtered = Agent::::filter_active_skills_by_trust( + &all_meta, + vec![0, 1, 2, 3], + &trust_map, + false, + ); + + let filtered_names: Vec<&str> = filtered + .iter() + .map(|&i| all_meta[i].name.as_str()) + .collect(); + assert_eq!( + filtered_names, + vec!["trusted-skill", "unclassified-skill"], + "Quarantined and Blocked matches must be dropped from the active set, \ + Trusted and unclassified must remain" + ); + } + + #[test] + fn filter_active_skills_by_trust_skipped_entirely_in_fallback_mode() { + let quarantined = skill_meta_named("quarantined-skill"); + let all_meta: Vec<&SkillMeta> = vec![&quarantined]; + let mut trust_map = std::collections::HashMap::new(); + trust_map.insert( + "quarantined-skill".to_string(), + trust_snapshot(zeph_common::SkillTrustLevel::Quarantined), + ); + + let filtered = Agent::::filter_active_skills_by_trust( + &all_meta, + vec![0], + &trust_map, + true, + ); + assert_eq!( + filtered, + vec![0], + "skill_fallback_mode must bypass the D1 filter (effective_trust's own fallback \ + guard is what actually matters for D4, see compute_effective_trust tests)" + ); + } + + #[test] + fn filter_active_skills_by_trust_index_out_of_bounds_is_dropped_not_panicked() { + let only = skill_meta_named("only-skill"); + let all_meta: Vec<&SkillMeta> = vec![&only]; + let trust_map = std::collections::HashMap::new(); + + let filtered = Agent::::filter_active_skills_by_trust( + &all_meta, + vec![0, 5], + &trust_map, + false, + ); + assert_eq!(filtered, vec![0]); + } + + // ── filter_skills_to_record (S7) ────────────────────────────────────────── + + #[test] + fn filter_skills_to_record_drops_names_absent_from_active_set() { + let mut skills_to_record = + vec!["trusted-skill".to_string(), "quarantined-skill".to_string()]; + let active_names = vec!["trusted-skill".to_string()]; + + Agent::::filter_skills_to_record(&mut skills_to_record, &active_names); + + assert_eq!( + skills_to_record, + vec!["trusted-skill".to_string()], + "a skill dropped from active_skill_names by the D1 filter must not be recorded as \ + usage — it was never actually activated this turn" + ); + } + + #[test] + fn filter_skills_to_record_keeps_all_when_all_are_active() { + let mut skills_to_record = vec!["a".to_string(), "b".to_string()]; + let active_names = vec!["a".to_string(), "b".to_string()]; + + Agent::::filter_skills_to_record(&mut skills_to_record, &active_names); + + assert_eq!(skills_to_record, vec!["a".to_string(), "b".to_string()]); + } + + #[test] + fn filter_skills_to_record_empty_active_set_drops_everything() { + let mut skills_to_record = vec!["a".to_string()]; + + Agent::::filter_skills_to_record(&mut skills_to_record, &[]); + + assert!(skills_to_record.is_empty()); + } + // ── effective_recall_timeout_ms tests (#2514) ──────────────────────────── #[test] diff --git a/crates/zeph-core/src/agent/context/mod.rs b/crates/zeph-core/src/agent/context/mod.rs index 319f09273..7087597e5 100644 --- a/crates/zeph-core/src/agent/context/mod.rs +++ b/crates/zeph-core/src/agent/context/mod.rs @@ -5,6 +5,10 @@ mod assembly; mod summarization; pub(super) use crate::text::truncate_to_chars as truncate_chars; +/// Re-exported so `agent::subagent_commands::parent_effective_trust_level`'s no-floor-wired +/// fallback path can apply the same D4 `skill_fallback_mode` guard as the parent's own gate +/// (#6701, S1), instead of duplicating the fold logic. +pub(super) use assembly::compute_effective_trust; #[cfg(test)] pub(super) use zeph_agent_context::state::CompactionOutcome; #[cfg(test)] diff --git a/crates/zeph-core/src/agent/mod.rs b/crates/zeph-core/src/agent/mod.rs index 84c8d2bf5..6c42ff3d9 100644 --- a/crates/zeph-core/src/agent/mod.rs +++ b/crates/zeph-core/src/agent/mod.rs @@ -312,7 +312,12 @@ impl Agent { }) .collect() }; - let skills_prompt = format_skills_catalog(&catalog_skills); + // No trust data has been loaded from the store yet at construction time — every skill + // resolves to `SkillTrustLevel::MISSING_ENTRY_FALLBACK` (Trusted), so no catalog entry + // gets a trust attribute here; the first turn's `reload_skills`/`apply_skill_trust_and_gating` + // refreshes this with real data. + let skills_prompt = + format_skills_catalog(&catalog_skills, &std::collections::HashMap::new()); let system_prompt = build_system_prompt(&skills_prompt, None); tracing::debug!(len = system_prompt.len(), "initial system prompt built"); tracing::trace!(prompt = %system_prompt, "full system prompt"); diff --git a/crates/zeph-core/src/agent/skill_reload.rs b/crates/zeph-core/src/agent/skill_reload.rs index 932093a97..42ae88641 100644 --- a/crates/zeph-core/src/agent/skill_reload.rs +++ b/crates/zeph-core/src/agent/skill_reload.rs @@ -244,6 +244,7 @@ impl Agent { } } #[tracing::instrument(name = "core.agent.reload_skills", skip_all, level = "debug")] + #[allow(clippy::too_many_lines)] // gate check + matcher rebuild + trust refresh + catalog prompt: one cohesive reload pipeline pub(super) async fn reload_skills(&mut self) { // #6031: single DRY choke point for the skill-hot-reload gate — covers every entry // point (runner/daemon/acp/serve) at once, instead of patching each `SkillWatcher` @@ -348,7 +349,9 @@ impl Agent { resources: zeph_skills::resource::SkillResources::default(), }) .collect(); - let skills_prompt = zeph_skills::prompt::format_skills_catalog(&catalog_skills); + let trust_levels = crate::skill_invoker::snapshot_map_to_trust_levels(&trust_map); + let skills_prompt = + zeph_skills::prompt::format_skills_catalog(&catalog_skills, &trust_levels); self.services .skill .last_skills_prompt diff --git a/crates/zeph-core/src/agent/state/mod.rs b/crates/zeph-core/src/agent/state/mod.rs index 53159b439..57dc4030e 100644 --- a/crates/zeph-core/src/agent/state/mod.rs +++ b/crates/zeph-core/src/agent/state/mod.rs @@ -89,6 +89,17 @@ pub(crate) struct SkillState { /// `SkillInvokeExecutor` can perform per-invocation re-hash when the flag is set. pub(crate) trust_snapshot: Arc>>, + /// Shared per-turn trust floor (#6701), the same cell the agent's `TrustGateExecutor` + /// reads. `None` until wired via `Agent::with_turn_trust_floor` — callers that never wire + /// one (e.g. some test fixtures) get pre-#6701 behavior at spawn time: a subagent trust + /// cap falls back to `set_effective_trust` instead of `TurnTrustFloor::fold`. + pub(crate) turn_trust_floor: Option, + /// Whether this turn's skill activation ran in retrieval-fallback mode (#6701, S1) — + /// mirrors the `skill_fallback_mode` local computed by `match_and_rank_skills` each turn. + /// Persisted so `subagent_commands::parent_effective_trust_level`'s no-floor-wired fallback + /// path can apply the same D4 guard `compute_effective_trust` applies to the parent's own + /// gate, instead of recomputing a fold blind to fallback mode. + pub(crate) skill_fallback_mode: bool, pub(crate) skill_paths: Vec, pub(crate) managed_dir: Option, pub(crate) trust_config: crate::config::TrustConfig, @@ -1571,6 +1582,8 @@ impl SkillState { Self { registry, trust_snapshot: Arc::new(RwLock::new(HashMap::new())), + turn_trust_floor: None, + skill_fallback_mode: false, skill_paths: Vec::new(), managed_dir: None, trust_config: crate::config::TrustConfig::default(), diff --git a/crates/zeph-core/src/agent/subagent_commands.rs b/crates/zeph-core/src/agent/subagent_commands.rs index b199fd56c..3f2cb95f5 100644 --- a/crates/zeph-core/src/agent/subagent_commands.rs +++ b/crates/zeph-core/src/agent/subagent_commands.rs @@ -928,6 +928,11 @@ impl Agent { // (foreground, background, and orchestration-driven via // `handle_scheduler_spawn_action`) is covered. max_trust_level: Some(self.parent_effective_trust_level()), + // #6701 (RC-5): shared handle to the same `TurnTrustFloor` cell this agent's own + // `TrustGateExecutor` reads, so the cap above is applied via `fold` (monotonic + // downgrade) rather than `set_effective_trust` (full overwrite) at spawn/resume — + // see `SpawnContext::turn_trust_floor`'s doc comment. + turn_trust_floor: self.services.skill.turn_trust_floor.clone(), // This helper's own three callers (`handle_agent_background`, // `handle_agent_spawn_foreground`, `handle_agent_resume`) are all dispatched from // the explicit `/agent spawn`/`/agent resume` slash command, so `Explicit` is the @@ -971,24 +976,23 @@ impl Agent { } /// Compute the parent session's own current effective trust level (issue #6493). /// - /// Mirrors the fold in [`crate::agent::context::assembly`]'s - /// `apply_skill_trust_and_gating` exactly, so the cap handed to a spawned sub-agent is - /// always consistent with what is actually enforced on the parent's own tool gate this - /// turn: the least-trusted level among all skills active this turn, or `Trusted` when no - /// skill is active. + /// When a `turn_trust_floor` is wired (#6701), reads it directly — it is the exact same + /// cell the parent's own `TrustGateExecutor` enforces against, so this is correct by + /// construction and also observes any mid-turn fold (e.g. an `invoke_skill` of a + /// Quarantined skill via `SkillTrustGate::resolve_body`, which `active_skill_names` alone + /// would miss — S3). Falls back to [`crate::agent::context::compute_effective_trust`] (D2, + /// with the D4 `skill_fallback_mode` guard — S1) only when no floor was wired, e.g. some + /// test fixtures that construct an `Agent` without `with_turn_trust_floor`. fn parent_effective_trust_level(&self) -> zeph_common::SkillTrustLevel { - if self.services.skill.active_skill_names.is_empty() { - return zeph_common::SkillTrustLevel::Trusted; + if let Some(floor) = &self.services.skill.turn_trust_floor { + return floor.get(); } let snapshot = self.services.skill.trust_snapshot.read(); - self.services - .skill - .active_skill_names - .iter() - .filter_map(|name| snapshot.get(name).map(|s| s.trust_level)) - .fold(zeph_common::SkillTrustLevel::Trusted, |acc, lvl| { - acc.min_trust(lvl) - }) + crate::agent::context::compute_effective_trust( + self.services.skill.skill_fallback_mode, + &self.services.skill.active_skill_names, + &snapshot, + ) } /// Extract recent parent messages for history propagation (Section 5.7 in spec). /// @@ -2084,6 +2088,81 @@ mod tests { ); } + /// #6701 (S1): before this fix, `parent_effective_trust_level` folded raw + /// `active_skill_names` with no `skill_fallback_mode` guard. In retrieval-fallback mode + /// `active_skill_names` is every registered skill (Quarantined/Blocked included), so a + /// subagent spawned during a fallback-mode turn would have been capped to Quarantined or + /// worse — a new lockout regression the D4 guard on the parent's OWN gate did not cover. + #[test] + fn build_spawn_context_ignores_fallback_mode_registry_trust_for_cap() { + let provider = mock_provider(vec![]); + let channel = MockChannel::new(vec![]); + let registry = create_test_registry(); + let mut agent = Agent::new( + provider, + channel, + registry, + None, + 5, + MockToolExecutor::no_tools(), + ); + // Simulate retrieval-fallback mode: every registered skill is "active" for catalog + // purposes, including one the operator has Blocked. + agent.services.skill.skill_fallback_mode = true; + agent.services.skill.active_skill_names = + vec!["trusted-skill".into(), "blocked-skill".into()]; + agent.services.skill.trust_snapshot.write().insert( + "blocked-skill".into(), + crate::skill_invoker::SkillTrustSnapshot { + trust_level: zeph_common::SkillTrustLevel::Blocked, + requires_trust_check: false, + blake3_hash: String::new(), + }, + ); + + let ctx = agent.build_spawn_context(&zeph_config::SubAgentConfig::default()); + assert_eq!( + ctx.max_trust_level, + Some(zeph_common::SkillTrustLevel::Trusted), + "skill_fallback_mode must force the subagent cap to Trusted regardless of registry \ + contents, matching the D4 guard applied to the parent's own gate" + ); + } + + /// #6701 (S1/S3): when a `turn_trust_floor` is wired, `parent_effective_trust_level` must + /// read it directly rather than recompute from `active_skill_names` — this is what makes + /// it observe a mid-turn fold (e.g. an `invoke_skill` of a Quarantined skill) that + /// `active_skill_names` alone would miss, and is also immune to the S1 fallback-mode bug + /// since the floor itself is already fallback-mode-aware. + #[test] + fn build_spawn_context_reads_wired_turn_trust_floor_directly() { + let provider = mock_provider(vec![]); + let channel = MockChannel::new(vec![]); + let registry = create_test_registry(); + let mut agent = Agent::new( + provider, + channel, + registry, + None, + 5, + MockToolExecutor::no_tools(), + ); + // No active skills and skill_fallback_mode is false — the no-floor fallback path would + // compute Trusted here. Wire a floor that was independently folded to Quarantined + // (e.g. by a mid-turn invoke_skill) to prove the floor wins. + let floor = zeph_common::TurnTrustFloor::new(zeph_common::SkillTrustLevel::Trusted); + floor.fold(zeph_common::SkillTrustLevel::Quarantined); + agent.services.skill.turn_trust_floor = Some(floor); + + let ctx = agent.build_spawn_context(&zeph_config::SubAgentConfig::default()); + assert_eq!( + ctx.max_trust_level, + Some(zeph_common::SkillTrustLevel::Quarantined), + "a wired turn_trust_floor must be read directly, reflecting mid-turn folds that \ + active_skill_names alone cannot see" + ); + } + /// Records every `set_effective_trust` call — unlike `MockToolExecutor`, which falls /// through to the trait's no-op default. Used by /// [`spawning_a_subagent_caps_trust_to_parent_effective_level`] to observe the trust level diff --git a/crates/zeph-core/src/skill_invoker.rs b/crates/zeph-core/src/skill_invoker.rs index 6a2dbc83b..f85933570 100644 --- a/crates/zeph-core/src/skill_invoker.rs +++ b/crates/zeph-core/src/skill_invoker.rs @@ -51,6 +51,17 @@ pub struct SkillTrustSnapshot { pub blake3_hash: String, } +/// Projects a [`SkillTrustSnapshot`] map down to just [`SkillTrustLevel`] — used by prompt +/// formatting functions (`format_skills_prompt`, `format_skills_catalog`) that only need the +/// level, not `requires_trust_check`/`blake3_hash`. +pub(crate) fn snapshot_map_to_trust_levels( + map: &HashMap, +) -> HashMap { + map.iter() + .map(|(k, v)| (k.clone(), v.trust_level)) + .collect() +} + /// Parameters for the `invoke_skill` tool call. #[derive(Debug, Deserialize, JsonSchema)] pub struct InvokeSkillParams { @@ -86,6 +97,15 @@ impl SkillInvokeExecutor { gate: SkillTrustGate::new(registry, trust_snapshot), } } + + /// Wires the shared per-turn trust floor (#6701) so an `invoke_skill` of a Quarantined + /// body folds the turn's trust down for its remainder — see + /// [`SkillTrustGate::with_turn_trust_floor`]. + #[must_use] + pub fn with_turn_trust_floor(mut self, turn_trust_floor: zeph_common::TurnTrustFloor) -> Self { + self.gate = self.gate.with_turn_trust_floor(turn_trust_floor); + self + } } impl ToolExecutor for SkillInvokeExecutor { diff --git a/crates/zeph-core/src/skill_loader.rs b/crates/zeph-core/src/skill_loader.rs index eb7e58f5b..611637ba6 100644 --- a/crates/zeph-core/src/skill_loader.rs +++ b/crates/zeph-core/src/skill_loader.rs @@ -79,6 +79,15 @@ impl SkillLoaderExecutor { gate: SkillTrustGate::new(registry, trust_snapshot), } } + + /// Wires the shared per-turn trust floor (#6701) so a `load_skill` preview of a + /// Quarantined body folds the turn's trust down for its remainder — see + /// [`SkillTrustGate::with_turn_trust_floor`]. + #[must_use] + pub fn with_turn_trust_floor(mut self, turn_trust_floor: zeph_common::TurnTrustFloor) -> Self { + self.gate = self.gate.with_turn_trust_floor(turn_trust_floor); + self + } } impl ToolExecutor for SkillLoaderExecutor { diff --git a/crates/zeph-core/src/skill_trust_gate.rs b/crates/zeph-core/src/skill_trust_gate.rs index 106f56eee..40c43d41c 100644 --- a/crates/zeph-core/src/skill_trust_gate.rs +++ b/crates/zeph-core/src/skill_trust_gate.rs @@ -20,7 +20,7 @@ use std::collections::HashMap; use std::sync::Arc; use parking_lot::RwLock; -use zeph_common::SkillTrustLevel; +use zeph_common::{SkillTrustLevel, TurnTrustFloor}; use zeph_skills::prompt::{sanitize_skill_text, wrap_quarantined}; use zeph_skills::registry::SkillRegistry; use zeph_skills::trust::compute_skill_hash; @@ -46,21 +46,27 @@ pub enum SkillBodyResolution { /// Shared registry + trust-snapshot pair backing both skill-body tool executors. /// -/// Cloning is cheap — both fields are `Arc`s. Construct one instance per executor from the same -/// `trust_snapshot` `Arc` so `load_skill` and `invoke_skill` see identical trust state within a -/// turn. +/// Cloning is cheap — all fields are `Arc`s (`TurnTrustFloor` wraps one internally). +/// Construct one instance per executor from the same `trust_snapshot` `Arc` (and the same +/// `turn_trust_floor`, when available) so `load_skill` and `invoke_skill` see identical +/// trust state within a turn. #[derive(Clone, Debug)] pub struct SkillTrustGate { registry: Arc>, trust_snapshot: Arc>>, + /// Shared per-turn trust floor (#6701), the same cell `TrustGateExecutor` reads. `None` + /// in contexts that never wired one (e.g. the `zeph skill invoke` CLI preview, which has + /// no live turn to degrade) — [`resolve_body`](Self::resolve_body) simply skips the fold + /// in that case, since there is no subsequent tool dispatch this turn to protect. + turn_trust_floor: Option, } impl SkillTrustGate { - /// Build a gate over `registry` and `trust_snapshot`. + /// Build a gate over `registry` and `trust_snapshot`, with no turn trust floor wired. /// - /// `trust_snapshot` should be the same `Arc` shared with any other trust-aware skill-body - /// consumer (see `agent_setup::build_skill_executors` in the binary crate) so they all - /// observe identical trust state. + /// Equivalent to [`with_turn_trust_floor`](Self::with_turn_trust_floor) with `None` — + /// prefer that constructor when a live agent turn's floor is available so a Quarantined + /// body read degrades the turn's trust (#6701, RC-3). /// /// # Examples /// @@ -83,9 +89,22 @@ impl SkillTrustGate { Self { registry, trust_snapshot, + turn_trust_floor: None, } } + /// Build a gate over `registry` and `trust_snapshot`, wired to the given turn trust floor. + /// + /// `turn_trust_floor` should be the same handle `TrustGateExecutor::trust_floor()` returns + /// for the live agent turn, so an explicit `invoke_skill`/`load_skill` of a Quarantined + /// skill folds the turn's trust floor down (#6701, RC-3) instead of leaving the gate's + /// weakest-link fold blind to bodies read outside the proactive-activation path. + #[must_use] + pub fn with_turn_trust_floor(mut self, turn_trust_floor: TurnTrustFloor) -> Self { + self.turn_trust_floor = Some(turn_trust_floor); + self + } + /// Resolve the trust snapshot entry for a skill. /// /// Returns `None` when no row exists — [`resolve_body`](Self::resolve_body) treats absence @@ -237,6 +256,14 @@ impl SkillTrustGate { sanitize_skill_text(&raw_body) }; let wrapped = if trust == SkillTrustLevel::Quarantined { + // #6701 (RC-3): an explicit invoke_skill/load_skill of a Quarantined skill + // is allowed (see specs/005-skills/spec.md § Agent-Invocable Skills), but + // reading its body now degrades the turn's trust floor for the remainder + // of the turn — closing the gap where invocation previously degraded + // nothing. Monotonic: never raises trust, only ever lowers it. + if let Some(floor) = &self.turn_trust_floor { + floor.fold(SkillTrustLevel::Quarantined); + } wrap_quarantined(&skill_name_safe, &sanitized) } else { sanitized @@ -425,6 +452,198 @@ mod tests { } } + // ── #6701 (RC-3, D3): resolve_body folds the turn trust floor on Quarantined bodies ── + + #[tokio::test] + async fn resolve_body_of_quarantined_skill_folds_turn_trust_floor() { + let dir = tempfile::tempdir().unwrap(); + let body = "quarantined skill body"; + let registry = make_registry_with_skill(dir.path(), "quarantined-skill", body); + let snapshots = HashMap::from([( + "quarantined-skill".to_owned(), + SkillTrustSnapshot { + trust_level: SkillTrustLevel::Quarantined, + requires_trust_check: false, + blake3_hash: String::new(), + }, + )]); + let floor = zeph_common::TurnTrustFloor::new(SkillTrustLevel::Trusted); + let gate = make_gate(registry, snapshots).with_turn_trust_floor(floor.clone()); + + assert_eq!( + floor.get(), + SkillTrustLevel::Trusted, + "sanity: starts Trusted" + ); + match gate.resolve_body("quarantined-skill").await.unwrap() { + SkillBodyResolution::Body(returned) => assert!(returned.contains("QUARANTINED")), + other => panic!("expected Body, got a different variant: {other:?}"), + } + assert_eq!( + floor.get(), + SkillTrustLevel::Quarantined, + "resolving a Quarantined body must fold the turn's trust floor down" + ); + } + + #[tokio::test] + async fn resolve_body_fold_never_raises_an_already_lower_floor() { + let dir = tempfile::tempdir().unwrap(); + let registry = make_registry_with_skill(dir.path(), "quarantined-skill", "body"); + let snapshots = HashMap::from([( + "quarantined-skill".to_owned(), + SkillTrustSnapshot { + trust_level: SkillTrustLevel::Quarantined, + requires_trust_check: false, + blake3_hash: String::new(), + }, + )]); + let floor = zeph_common::TurnTrustFloor::new(SkillTrustLevel::Blocked); + let gate = make_gate(registry, snapshots).with_turn_trust_floor(floor.clone()); + + let _ = gate.resolve_body("quarantined-skill").await.unwrap(); + assert_eq!( + floor.get(), + SkillTrustLevel::Blocked, + "fold(Quarantined) must not raise a floor already folded to Blocked" + ); + } + + #[tokio::test] + async fn resolve_body_of_trusted_skill_does_not_touch_turn_trust_floor() { + let dir = tempfile::tempdir().unwrap(); + let body = "trusted skill body"; + let registry = make_registry_with_skill(dir.path(), "trusted-skill", body); + let snapshots = HashMap::from([( + "trusted-skill".to_owned(), + SkillTrustSnapshot { + trust_level: SkillTrustLevel::Trusted, + requires_trust_check: false, + blake3_hash: String::new(), + }, + )]); + let floor = zeph_common::TurnTrustFloor::new(SkillTrustLevel::Trusted); + let gate = make_gate(registry, snapshots).with_turn_trust_floor(floor.clone()); + + let _ = gate.resolve_body("trusted-skill").await.unwrap(); + assert_eq!(floor.get(), SkillTrustLevel::Trusted); + } + + #[tokio::test] + async fn resolve_body_without_a_wired_floor_never_panics() { + // No `with_turn_trust_floor` call — must simply skip the fold, not panic. + let dir = tempfile::tempdir().unwrap(); + let registry = make_registry_with_skill(dir.path(), "quarantined-skill", "body"); + let snapshots = HashMap::from([( + "quarantined-skill".to_owned(), + SkillTrustSnapshot { + trust_level: SkillTrustLevel::Quarantined, + requires_trust_check: false, + blake3_hash: String::new(), + }, + )]); + let gate = make_gate(registry, snapshots); + match gate.resolve_body("quarantined-skill").await.unwrap() { + SkillBodyResolution::Body(returned) => assert!(returned.contains("QUARANTINED")), + other => panic!("expected Body, got a different variant: {other:?}"), + } + } + + // ── #6701 (S5): end-to-end RC-3 — resolve_body then a subsequent tool dispatch ── + + /// Minimal `ToolExecutor` that always allows, so the only thing under test is whether + /// `TrustGateExecutor` denies `bash` — never whether the inner executor itself would. + use zeph_tools::executor::ToolExecutor as _; + + #[derive(Debug)] + struct AlwaysOkExecutor; + + impl zeph_tools::executor::ToolExecutor for AlwaysOkExecutor { + async fn execute( + &self, + _response: &str, + ) -> Result, ToolError> { + Ok(None) + } + + async fn execute_tool_call( + &self, + call: &zeph_tools::executor::ToolCall, + ) -> Result, ToolError> { + Ok(Some(zeph_tools::executor::ToolOutput { + tool_name: call.tool_id.clone(), + summary: "ok".into(), + blocks_executed: 1, + filter_stats: None, + diff: None, + streamed: false, + terminal_id: None, + locations: None, + raw_response: None, + claim_source: None, + ..Default::default() + })) + } + + zeph_tools::tool_executor_no_inner_defaults!(); + } + + /// The spec's headline RC-3 invariant, exercised end-to-end rather than by asserting on + /// `floor.get()` alone: a shared `TurnTrustFloor` wired into BOTH a `SkillTrustGate` (as + /// `SkillInvokeExecutor`/`SkillLoaderExecutor` would be, in production) and a + /// `TrustGateExecutor` (as the agent's real tool gate would be) — `resolve_body` on a + /// Quarantined skill must fold the floor down, and a subsequent `bash` dispatch through the + /// gate sharing that exact floor must then be denied. + #[tokio::test] + async fn resolve_body_of_quarantined_skill_then_bash_dispatch_is_denied() { + let dir = tempfile::tempdir().unwrap(); + let registry = make_registry_with_skill(dir.path(), "quarantined-skill", "body"); + let snapshots = HashMap::from([( + "quarantined-skill".to_owned(), + SkillTrustSnapshot { + trust_level: SkillTrustLevel::Quarantined, + requires_trust_check: false, + blake3_hash: String::new(), + }, + )]); + let floor = zeph_common::TurnTrustFloor::new(SkillTrustLevel::Trusted); + let gate = make_gate(registry, snapshots).with_turn_trust_floor(floor.clone()); + // `from_legacy(&[], &[])` (no denied/confirm commands) resolves to Allow for bash, so + // the only thing under test is the trust-level gate, not the Supervised-mode + // confirmation-required default `PermissionPolicy::default()` would apply. + let trust_gate = zeph_tools::TrustGateExecutor::new( + AlwaysOkExecutor, + zeph_tools::PermissionPolicy::from_legacy(&[], &[]), + ) + .with_trust_floor(floor); + + // Sanity: bash is allowed before any Quarantined body has been read this turn. + let call = zeph_tools::executor::ToolCall { + tool_id: "bash".into(), + params: serde_json::Map::new(), + caller_id: None, + context: None, + tool_call_id: String::new(), + skill_name: None, + }; + assert!( + trust_gate.execute_tool_call(&call).await.is_ok(), + "bash must be allowed before any Quarantined body is read" + ); + + match gate.resolve_body("quarantined-skill").await.unwrap() { + SkillBodyResolution::Body(returned) => assert!(returned.contains("QUARANTINED")), + other => panic!("expected Body, got a different variant: {other:?}"), + } + + let result = trust_gate.execute_tool_call(&call).await; + assert!( + matches!(result, Err(ToolError::Blocked { .. })), + "a bash call in the same turn, after resolve_body returned a Quarantined body, \ + must be denied — got {result:?}" + ); + } + // ── resolve_require_check (#6087) ──────────────────────────────────────── #[test] diff --git a/crates/zeph-skills/src/prompt.rs b/crates/zeph-skills/src/prompt.rs index f175b905a..11c6867e9 100644 --- a/crates/zeph-skills/src/prompt.rs +++ b/crates/zeph-skills/src/prompt.rs @@ -541,17 +541,39 @@ pub fn format_skills_prompt_compact(skills: &[Skill]) -> String { out } +/// Formats the `` catalog block: name + description only, no bodies. +/// +/// `trust_levels` annotates each entry with `trust="quarantined"`/`trust="blocked"` when the +/// skill's resolved trust is not `Trusted`/`Verified` (#6701, D1) — this is how a skill dropped +/// from `active_skill_names` by the trust-aware activation filter remains discoverable and +/// nameable to the operator (`zeph skill trust trusted`) despite never being activated. +/// A skill absent from `trust_levels`, or resolved to `Trusted`/`Verified`, gets no attribute. +/// +/// The `trust="blocked"` case is defensive: in the `zeph-core` agent turn path, a `Blocked` +/// skill is already excluded from `skills` (and from `trust_levels`' effective domain) before +/// this function is called — per spec, `Blocked` is excluded from both catalog and actives, +/// unlike `Quarantined`, which D1 still surfaces here. This function itself makes no such +/// assumption about its caller, so the branch stays live for any consumer that passes an +/// unfiltered `skills`/`trust_levels` pair (e.g. a future CLI preview command). #[must_use] -pub fn format_skills_catalog(skills: &[Skill]) -> String { +pub fn format_skills_catalog( + skills: &[Skill], + trust_levels: &HashMap, +) -> String { if skills.is_empty() { return String::new(); } let mut out = String::from("\n"); for skill in skills { + let trust_attr = match trust_levels.get(skill.name()) { + Some(SkillTrustLevel::Quarantined) => " trust=\"quarantined\"", + Some(SkillTrustLevel::Blocked) => " trust=\"blocked\"", + _ => "", + }; let _ = writeln!( out, - " ", + " ", xml_escape(skill.name()), xml_escape(skill.description()), ); @@ -868,13 +890,13 @@ mod tests { #[test] fn format_skills_catalog_empty() { let empty: &[Skill] = &[]; - assert_eq!(format_skills_catalog(empty), ""); + assert_eq!(format_skills_catalog(empty, &HashMap::new()), ""); } #[test] fn format_skills_catalog_produces_other_skills_tag() { let skills = vec![make_skill("test", "A test skill.", "body")]; - let output = format_skills_catalog(&skills); + let output = format_skills_catalog(&skills, &HashMap::new()); assert!(output.starts_with("")); assert!(output.ends_with("")); assert!(output.contains("name=\"test\"")); @@ -882,6 +904,47 @@ mod tests { assert!(!output.contains("body")); } + /// #6701 (D1): a Quarantined/Blocked skill dropped from `active_skill_names` by the + /// trust-aware activation filter must still surface in the catalog, annotated so the + /// operator/model can see it exists and how to promote it. + #[test] + fn format_skills_catalog_annotates_quarantined_and_blocked_trust() { + let skills = vec![ + make_skill("q-skill", "Quarantined one.", "body"), + make_skill("b-skill", "Blocked one.", "body"), + make_skill("t-skill", "Trusted one.", "body"), + ]; + let mut trust_levels = HashMap::new(); + trust_levels.insert("q-skill".to_string(), SkillTrustLevel::Quarantined); + trust_levels.insert("b-skill".to_string(), SkillTrustLevel::Blocked); + trust_levels.insert("t-skill".to_string(), SkillTrustLevel::Trusted); + + let output = format_skills_catalog(&skills, &trust_levels); + assert!( + output.contains( + "name=\"q-skill\" description=\"Quarantined one.\" trust=\"quarantined\" />" + ), + "expected trust=\"quarantined\" attribute, got:\n{output}" + ); + assert!( + output.contains("name=\"b-skill\" description=\"Blocked one.\" trust=\"blocked\" />"), + "expected trust=\"blocked\" attribute, got:\n{output}" + ); + assert!( + output.contains("name=\"t-skill\" description=\"Trusted one.\" />"), + "Trusted skill must get no trust attribute, got:\n{output}" + ); + } + + /// A skill absent from `trust_levels` (never trust-classified) must get no attribute, + /// matching `SkillTrustLevel::MISSING_ENTRY_FALLBACK` (Trusted). + #[test] + fn format_skills_catalog_no_attribute_for_unclassified_skill() { + let skills = vec![make_skill("unknown", "desc", "body")]; + let output = format_skills_catalog(&skills, &HashMap::new()); + assert!(!output.contains("trust=")); + } + #[test] fn health_attrs_emitted_when_uses_at_threshold() { let skills = vec![make_skill("git", "Git helper.", "body")]; @@ -961,7 +1024,7 @@ mod tests { "compact: description not escaped" ); - let catalog = format_skills_catalog(&skills); + let catalog = format_skills_catalog(&skills, &HashMap::new()); assert!( catalog.contains("a&b<c>d"e"), "catalog: name not escaped" diff --git a/crates/zeph-subagent/src/manager/mod.rs b/crates/zeph-subagent/src/manager/mod.rs index f345843e7..48c4d57c3 100644 --- a/crates/zeph-subagent/src/manager/mod.rs +++ b/crates/zeph-subagent/src/manager/mod.rs @@ -150,6 +150,23 @@ pub struct SpawnContext { /// `None` means no cap is imposed by the parent (the sub-agent's own definition /// determines its trust level). pub max_trust_level: Option, + /// Shared per-turn trust floor (#6701) the cap in [`max_trust_level`][Self::max_trust_level] + /// is applied to. + /// + /// When `Some`, [`SubAgentManager::spawn`]/resume applies the cap via + /// [`zeph_common::TurnTrustFloor::fold`] on this handle directly — a monotonic downgrade + /// that can never raise trust — instead of calling `set_effective_trust` on the built + /// executor, which would be a full overwrite and could restore trust above a floor + /// already lowered earlier in the same task (e.g. by an explicit `invoke_skill` of a + /// Quarantined skill). `None` falls back to the pre-#6701 `set_effective_trust` behavior + /// (e.g. call sites/tests that construct an executor with no shared floor to fold). + /// + /// # Caller responsibility for nested spawns + /// + /// Like [`max_trust_level`][Self::max_trust_level], this field does **not** propagate + /// automatically — a sub-agent that spawns its own children must copy this field from + /// its received `SpawnContext` into the child's `SpawnContext`. + pub turn_trust_floor: Option, /// Tool names that this sub-agent is allowed to invoke, inherited from the parent. /// /// When `Some(set)`, the effective tool allowlist for the spawned agent is the diff --git a/crates/zeph-subagent/src/manager/spawn.rs b/crates/zeph-subagent/src/manager/spawn.rs index 161c6dc54..053ee4d36 100644 --- a/crates/zeph-subagent/src/manager/spawn.rs +++ b/crates/zeph-subagent/src/manager/spawn.rs @@ -762,7 +762,15 @@ impl SubAgentManager { ); if let Some(cap) = ctx.max_trust_level { - executor.set_effective_trust(cap); + // #6701 (RC-5): fold, never set — a plain set_effective_trust(cap) here would + // overwrite any downgrade already applied to the shared trust floor (e.g. by an + // earlier invoke_skill of a Quarantined skill in this same task), restoring trust + // above where it should sit. fold(cap) can only ever lower it. + if let Some(floor) = &ctx.turn_trust_floor { + floor.fold(cap); + } else { + executor.set_effective_trust(cap); + } } let (secret_request_tx, pending_secret_rx) = mpsc::channel::(4); @@ -1240,7 +1248,15 @@ impl SubAgentManager { if let Some(ctx) = spawn_context && let Some(cap) = ctx.max_trust_level { - executor.set_effective_trust(cap); + // #6701 (RC-5): fold, never set — see the identical rationale at the fresh-spawn + // call site above. This resume/rebuild path is exactly the "own turn rebuild" case + // the fix targets: a plain set here would restore trust above a floor already + // folded down earlier in the same task. + if let Some(floor) = &ctx.turn_trust_floor { + floor.fold(cap); + } else { + executor.set_effective_trust(cap); + } } let (secret_request_tx, pending_secret_rx) = mpsc::channel::(4); diff --git a/crates/zeph-subagent/src/manager/tests.rs b/crates/zeph-subagent/src/manager/tests.rs index 8ee77bad1..ea2327196 100644 --- a/crates/zeph-subagent/src/manager/tests.rs +++ b/crates/zeph-subagent/src/manager/tests.rs @@ -2025,6 +2025,204 @@ fn resume_with_spawn_context_applies_trust_cap_to_executor() { mgr.cancel(&new_id).unwrap(); } +/// #6701 (RC-5, D3): when a `turn_trust_floor` handle is present, the cap must be applied via +/// `TurnTrustFloor::fold` — a monotonic downgrade that can never restore trust above a floor +/// already folded lower earlier in this same task (e.g. by an explicit `invoke_skill` of a +/// Quarantined skill via `SkillTrustGate::resolve_body`) — instead of the executor's +/// `set_effective_trust`, which would be a full overwrite. +#[test] +fn resume_with_spawn_context_folds_never_raises_an_already_lower_floor() { + let rt = tokio::runtime::Runtime::new().unwrap(); + + let tmp = tempfile::tempdir().unwrap(); + let agent_id = "d0d00001-0000-0000-0000-000000000000"; + write_completed_meta(tmp.path(), agent_id, "bot"); + + let mut mgr = make_manager(); + mgr.definitions.push(sample_def()); + let cfg = make_cfg_with_dir(tmp.path()); + + let tracker = Arc::new(TrustTrackingExecutor { + recorded: Mutex::new(None), + }); + let executor: Arc = Arc::clone(&tracker) as _; + + // Floor already folded down to Blocked earlier in this task — Blocked is MORE + // restrictive than the Quarantined cap below. + let floor = zeph_common::TurnTrustFloor::new(SkillTrustLevel::Blocked); + let ctx = SpawnContext { + max_trust_level: Some(SkillTrustLevel::Quarantined), + turn_trust_floor: Some(floor.clone()), + ..SpawnContext::default() + }; + let (new_id, _) = rt + .block_on(mgr.resume( + "d0d00001", + "continue", + mock_provider(vec!["done"]), + executor, + None, + &cfg, + Some(&ctx), + )) + .unwrap(); + + assert_eq!( + floor.get(), + SkillTrustLevel::Blocked, + "fold(cap) must never restore trust above a floor already folded lower this task" + ); + assert_eq!( + *tracker.recorded.lock().unwrap(), + None, + "when turn_trust_floor is wired, the fold happens directly on the shared cell — the \ + executor's own set_effective_trust must not be called at all" + ); + + let _guard = rt.enter(); + mgr.cancel(&new_id).unwrap(); +} + +/// Companion to the above: a floor that starts ABOVE the cap must still be lowered by fold. +#[test] +fn resume_with_spawn_context_folds_lowers_a_higher_floor() { + let rt = tokio::runtime::Runtime::new().unwrap(); + + let tmp = tempfile::tempdir().unwrap(); + let agent_id = "d0d00002-0000-0000-0000-000000000000"; + write_completed_meta(tmp.path(), agent_id, "bot"); + + let mut mgr = make_manager(); + mgr.definitions.push(sample_def()); + let cfg = make_cfg_with_dir(tmp.path()); + + let executor: Arc = Arc::new(TrustTrackingExecutor { + recorded: Mutex::new(None), + }); + + let floor = zeph_common::TurnTrustFloor::new(SkillTrustLevel::Trusted); + let ctx = SpawnContext { + max_trust_level: Some(SkillTrustLevel::Quarantined), + turn_trust_floor: Some(floor.clone()), + ..SpawnContext::default() + }; + let (new_id, _) = rt + .block_on(mgr.resume( + "d0d00002", + "continue", + mock_provider(vec!["done"]), + executor, + None, + &cfg, + Some(&ctx), + )) + .unwrap(); + + assert_eq!( + floor.get(), + SkillTrustLevel::Quarantined, + "fold(cap) must lower a floor that started above (more trusted than) the cap" + ); + + let _guard = rt.enter(); + mgr.cancel(&new_id).unwrap(); +} + +/// #6701 (S4): the resume-path fold tests above only exercise the resume/rebuild call site +/// (`spawn.rs`'s `resume`). The fresh-spawn call site (`spawn.rs`'s `spawn`, applied right after +/// `build_filtered_executor`) has its own identical `fold`-vs-`set` branch and needs its own +/// coverage — a regression that broke only the fresh-spawn site would otherwise pass silently. +#[test] +fn spawn_with_context_folds_never_raises_an_already_lower_floor() { + let rt = tokio::runtime::Runtime::new().unwrap(); + + let mut mgr = make_manager(); + mgr.definitions.push(sample_def()); + + let tracker = Arc::new(TrustTrackingExecutor { + recorded: Mutex::new(None), + }); + let executor: Arc = Arc::clone(&tracker) as _; + + // Floor already folded down to Blocked earlier in this task — Blocked is MORE + // restrictive than the Quarantined cap below. + let floor = zeph_common::TurnTrustFloor::new(SkillTrustLevel::Blocked); + let ctx = SpawnContext { + max_trust_level: Some(SkillTrustLevel::Quarantined), + turn_trust_floor: Some(floor.clone()), + origin: super::SpawnOrigin::Explicit, + ..SpawnContext::default() + }; + let new_id = rt + .block_on(mgr.spawn( + "bot", + "go", + mock_provider(vec!["done"]), + executor, + None, + &SubAgentConfig::default(), + ctx, + )) + .unwrap(); + + assert_eq!( + floor.get(), + SkillTrustLevel::Blocked, + "fresh spawn's fold(cap) must never restore trust above a floor already folded lower" + ); + assert_eq!( + *tracker.recorded.lock().unwrap(), + None, + "when turn_trust_floor is wired, fresh spawn must fold directly on the shared cell — \ + the executor's own set_effective_trust must not be called at all" + ); + + let _guard = rt.enter(); + mgr.cancel(&new_id).unwrap(); +} + +/// Companion to the above: a floor that starts ABOVE the cap must still be lowered by fold, at +/// the fresh-spawn call site. +#[test] +fn spawn_with_context_folds_lowers_a_higher_floor() { + let rt = tokio::runtime::Runtime::new().unwrap(); + + let mut mgr = make_manager(); + mgr.definitions.push(sample_def()); + + let executor: Arc = Arc::new(TrustTrackingExecutor { + recorded: Mutex::new(None), + }); + + let floor = zeph_common::TurnTrustFloor::new(SkillTrustLevel::Trusted); + let ctx = SpawnContext { + max_trust_level: Some(SkillTrustLevel::Quarantined), + turn_trust_floor: Some(floor.clone()), + origin: super::SpawnOrigin::Explicit, + ..SpawnContext::default() + }; + let new_id = rt + .block_on(mgr.spawn( + "bot", + "go", + mock_provider(vec!["done"]), + executor, + None, + &SubAgentConfig::default(), + ctx, + )) + .unwrap(); + + assert_eq!( + floor.get(), + SkillTrustLevel::Quarantined, + "fresh spawn's fold(cap) must lower a floor that started above the cap" + ); + + let _guard = rt.enter(); + mgr.cancel(&new_id).unwrap(); +} + #[test] fn resume_with_spawn_context_wires_debug_dump_sink_to_resumed_loop() { // Regression test for #6391 (S1 follow-up): the production `/agent resume` call site diff --git a/crates/zeph-tools/src/shadow_probe.rs b/crates/zeph-tools/src/shadow_probe.rs index 72e465194..59a2ce153 100644 --- a/crates/zeph-tools/src/shadow_probe.rs +++ b/crates/zeph-tools/src/shadow_probe.rs @@ -38,9 +38,7 @@ use tracing::{Instrument as _, info_span}; use crate::SkillTrustLevel; use crate::executor::{ToolCall, ToolError, ToolExecutor, ToolOutput}; use crate::registry::ToolDef; -use crate::trust_gate::{ - is_quarantine_denied, quarantine_denial_message, trust_to_u8, u8_to_trust, -}; +use crate::trust_gate::{is_quarantine_denied, quarantine_denial_message}; /// Probe interface required by `ShadowProbeExecutor`. /// @@ -147,9 +145,7 @@ impl ShadowProbeExecutor { probe, turn_number, risk_level, - effective_trust: std::sync::atomic::AtomicU8::new(trust_to_u8( - SkillTrustLevel::Trusted, - )), + effective_trust: std::sync::atomic::AtomicU8::new(SkillTrustLevel::Trusted.severity()), } } @@ -162,7 +158,7 @@ impl ShadowProbeExecutor { } fn effective_trust(&self) -> SkillTrustLevel { - u8_to_trust( + SkillTrustLevel::from_severity( self.effective_trust .load(std::sync::atomic::Ordering::Relaxed), ) @@ -370,7 +366,7 @@ impl ToolExecutor for ShadowProbeExecutor { fn set_effective_trust(&self, level: crate::SkillTrustLevel) { self.effective_trust - .store(trust_to_u8(level), std::sync::atomic::Ordering::Relaxed); + .store(level.severity(), std::sync::atomic::Ordering::Relaxed); self.inner.set_effective_trust(level); } diff --git a/crates/zeph-tools/src/trust_gate.rs b/crates/zeph-tools/src/trust_gate.rs index d979f6e17..422f9cf7a 100644 --- a/crates/zeph-tools/src/trust_gate.rs +++ b/crates/zeph-tools/src/trust_gate.rs @@ -4,12 +4,10 @@ //! Trust-level enforcement layer for tool execution. use std::collections::HashSet; -use std::sync::{ - Arc, - atomic::{AtomicU8, Ordering}, -}; +use std::sync::Arc; use parking_lot::RwLock; +use zeph_common::TurnTrustFloor; use crate::SkillTrustLevel; @@ -49,29 +47,11 @@ pub(crate) fn quarantine_denial_message(tool_id: &str, active_skills: &[String]) } } -pub(crate) fn trust_to_u8(level: SkillTrustLevel) -> u8 { - match level { - SkillTrustLevel::Trusted => 0, - SkillTrustLevel::Verified => 1, - SkillTrustLevel::Quarantined => 2, - _ => 3, - } -} - -pub(crate) fn u8_to_trust(v: u8) -> SkillTrustLevel { - match v { - 0 => SkillTrustLevel::Trusted, - 1 => SkillTrustLevel::Verified, - 2 => SkillTrustLevel::Quarantined, - _ => SkillTrustLevel::Blocked, - } -} - /// Wraps an inner `ToolExecutor` and applies trust-level permission overlays. pub struct TrustGateExecutor { inner: T, policy: PermissionPolicy, - effective_trust: AtomicU8, + effective_trust: TurnTrustFloor, /// Sanitized IDs of all registered MCP tools. When a Quarantined skill is /// active, any tool whose ID appears in this set is denied — regardless of /// whether its name matches `QUARANTINE_DENIED`. Populated at startup by @@ -96,7 +76,7 @@ impl TrustGateExecutor { Self { inner, policy, - effective_trust: AtomicU8::new(trust_to_u8(SkillTrustLevel::Trusted)), + effective_trust: TurnTrustFloor::new(SkillTrustLevel::Trusted), mcp_tool_ids: Arc::new(RwLock::new(HashSet::new())), } } @@ -109,14 +89,38 @@ impl TrustGateExecutor { Arc::clone(&self.mcp_tool_ids) } + /// Replaces this gate's trust floor with an externally-owned, shared one (#6701). + /// + /// Use when another component (e.g. `SkillTrustGate`) must observe and fold the exact + /// same cell this gate reads in `check_trust` — pass the same + /// `TurnTrustFloor` to both instead of relying on [`trust_floor`](Self::trust_floor), + /// which can only be called *after* this gate already exists. + #[must_use] + pub fn with_trust_floor(mut self, floor: TurnTrustFloor) -> Self { + self.effective_trust = floor; + self + } + + /// Returns a clone of the shared per-turn trust floor (#6701). + /// + /// Cloning is cheap (an `Arc` clone) and shares the same underlying cell — callers that + /// need to downgrade trust from outside the `ToolExecutor` trait chain (e.g. a subagent + /// spawn applying an inherited trust cap, or `SkillTrustGate::resolve_body` degrading on + /// a Quarantined body read) can call [`TurnTrustFloor::fold`] on the returned handle + /// directly instead of routing a `set_effective_trust` call back down through every + /// wrapping executor layer. + #[must_use] + pub fn trust_floor(&self) -> TurnTrustFloor { + self.effective_trust.clone() + } + pub fn set_effective_trust(&self, level: SkillTrustLevel) { - self.effective_trust - .store(trust_to_u8(level), Ordering::Relaxed); + self.effective_trust.set(level); } #[must_use] pub fn effective_trust(&self) -> SkillTrustLevel { - u8_to_trust(self.effective_trust.load(Ordering::Relaxed)) + self.effective_trust.get() } fn is_mcp_tool(&self, tool_id: &str) -> bool { @@ -299,8 +303,7 @@ impl ToolExecutor for TrustGateExecutor { } fn set_effective_trust(&self, level: crate::SkillTrustLevel) { - self.effective_trust - .store(trust_to_u8(level), Ordering::Relaxed); + self.effective_trust.set(level); } /// Returns `true` when the current policy would require confirmation for `call`. @@ -912,6 +915,23 @@ mod tests { ); } + #[test] + fn trust_floor_handle_shares_state_with_gate() { + let gate = TrustGateExecutor::new(MockExecutor, PermissionPolicy::default()); + let floor = gate.trust_floor(); + assert_eq!(floor.get(), SkillTrustLevel::Trusted); + + // A downgrade issued through the gate's own set_effective_trust must be visible + // through the handle (same underlying cell). + gate.set_effective_trust(SkillTrustLevel::Quarantined); + assert_eq!(floor.get(), SkillTrustLevel::Quarantined); + + // A fold issued through the handle must be visible through the gate. + floor.set(SkillTrustLevel::Trusted); + floor.fold(SkillTrustLevel::Verified); + assert_eq!(gate.effective_trust(), SkillTrustLevel::Verified); + } + #[test] fn set_effective_trust_interior_mutability() { let gate = TrustGateExecutor::new(MockExecutor, PermissionPolicy::default()); diff --git a/src/acp.rs b/src/acp.rs index d86d39d26..8962688aa 100644 --- a/src/acp.rs +++ b/src/acp.rs @@ -1484,6 +1484,7 @@ where trust_config: zeph_core::config::TrustConfig, trust_snapshot: std::sync::Arc>>, + turn_trust_floor: zeph_common::TurnTrustFloor, quality_pipeline: Option>, rl_routing_enabled: bool, rl_learning_rate: f32, @@ -1562,6 +1563,7 @@ where }) .with_trust_config(deps.trust_config) .with_trust_snapshot(deps.trust_snapshot) + .with_turn_trust_floor(deps.turn_trust_floor) .with_quality_pipeline(deps.quality_pipeline) .with_rl_routing( deps.rl_routing_enabled, @@ -1779,7 +1781,7 @@ async fn spawn_acp_agent( } ex }; - let (skill_loader_executor, skill_invoke_executor, trust_snapshot) = + let (skill_loader_executor, skill_invoke_executor, trust_snapshot, turn_trust_floor) = agent_setup::build_skill_executors(®istry); // #5958: shared trajectory risk slot/signal queue, created here (rather than further below, @@ -1903,6 +1905,7 @@ async fn spawn_acp_agent( let (trust_gated, mcp_ids_handle) = crate::agent_setup::apply_common_tool_gating( zeph_tools::DynExecutor(base_composite), &permission_policy, + turn_trust_floor.clone(), ); crate::agent_setup::register_mcp_tool_ids(&mcp_ids_handle, &mcp_tools); @@ -2142,6 +2145,7 @@ async fn spawn_acp_agent( semantic_scan_provider, trust_config: d.trust_config.clone(), trust_snapshot: Arc::clone(&trust_snapshot), + turn_trust_floor: turn_trust_floor.clone(), quality_pipeline: d.quality_pipeline.clone(), rl_routing_enabled: d.rl_routing_enabled, rl_learning_rate: d.rl_learning_rate, @@ -3566,6 +3570,7 @@ mod tests { let (gated, mcp_ids_handle) = crate::agent_setup::apply_common_tool_gating( inner_executor, &zeph_tools::PermissionPolicy::default(), + zeph_common::TurnTrustFloor::default(), ); crate::agent_setup::register_mcp_tool_ids(&mcp_ids_handle, std::slice::from_ref(&mcp_tool)); zeph_tools::executor::ToolExecutor::set_effective_trust( @@ -3768,6 +3773,7 @@ mod tests { let (trust_gated, mcp_ids_handle) = crate::agent_setup::apply_common_tool_gating( inner_executor, &zeph_tools::PermissionPolicy::default(), + zeph_common::TurnTrustFloor::default(), ); crate::agent_setup::register_mcp_tool_ids(&mcp_ids_handle, std::slice::from_ref(&mcp_tool)); zeph_tools::ToolExecutor::set_effective_trust( @@ -4044,6 +4050,7 @@ mod tests { let (trust_gated, _mcp_ids_handle) = crate::agent_setup::apply_common_tool_gating( zeph_tools::DynExecutor(Arc::new(base_executor)), &zeph_tools::PermissionPolicy::default().with_autonomy(zeph_tools::AutonomyLevel::Full), + zeph_common::TurnTrustFloor::default(), ); let policy_config = zeph_tools::PolicyConfig { @@ -4231,6 +4238,7 @@ mod tests { let (gated, _mcp_ids_handle) = crate::agent_setup::apply_common_tool_gating( zeph_tools::DynExecutor(composite), &policy, + zeph_common::TurnTrustFloor::default(), ); (gated, trajectory_signal_queue) } @@ -4468,7 +4476,7 @@ mod tests { >, ) { let registry = Arc::new(RwLock::new(zeph_skills::registry::SkillRegistry::empty())); - let (skill_loader_executor, skill_invoke_executor, trust_snapshot) = + let (skill_loader_executor, skill_invoke_executor, trust_snapshot, _turn_trust_floor) = agent_setup::build_skill_executors(®istry); let mock_provider = @@ -5517,6 +5525,7 @@ mod tests { semantic_scan_provider: config.skills.semantic_scan_provider.as_str().to_owned(), trust_config: config.skills.trust.clone(), trust_snapshot: Arc::new(RwLock::new(std::collections::HashMap::new())), + turn_trust_floor: zeph_common::TurnTrustFloor::default(), quality_pipeline: None, rl_routing_enabled: config.skills.rl_routing_enabled, rl_learning_rate: config.skills.rl_learning_rate, diff --git a/src/agent_setup.rs b/src/agent_setup.rs index 73fe20eaf..8c0464146 100644 --- a/src/agent_setup.rs +++ b/src/agent_setup.rs @@ -1576,21 +1576,33 @@ pub(crate) fn build_quality_pipeline( /// `apply_skill_trust_and_gating` and read lock-free by both executors — callers must also /// thread it into `.with_trust_snapshot(...)` on the agent builder (the `SkillState` writer) so /// all three holders share the same instance. +/// +/// Also creates the shared per-turn [`zeph_common::TurnTrustFloor`] (#6701) and wires it into +/// both executors via `with_turn_trust_floor`, so an explicit `invoke_skill`/`load_skill` of a +/// Quarantined body folds the turn's trust down (RC-3). The floor is created here (rather than +/// inside `apply_common_tool_gating`) because this function's outputs feed into the composite +/// tree `apply_common_tool_gating` gates — callers MUST pass the returned floor into that call +/// so `TrustGateExecutor` shares the identical cell. +#[allow(clippy::type_complexity)] // 4-tuple of pre-existing types (executors + shared snapshot + trust floor); a type alias would only rename these once, at one call site pub(crate) fn build_skill_executors( registry: &Arc>, ) -> ( zeph_core::SkillLoaderExecutor, zeph_core::SkillInvokeExecutor, Arc>>, + zeph_common::TurnTrustFloor, ) { let trust_snapshot: Arc< RwLock>, > = Arc::new(RwLock::new(std::collections::HashMap::new())); + let turn_trust_floor = zeph_common::TurnTrustFloor::default(); let loader = - zeph_core::SkillLoaderExecutor::new(Arc::clone(registry), Arc::clone(&trust_snapshot)); + zeph_core::SkillLoaderExecutor::new(Arc::clone(registry), Arc::clone(&trust_snapshot)) + .with_turn_trust_floor(turn_trust_floor.clone()); let invoker = - zeph_core::SkillInvokeExecutor::new(Arc::clone(registry), Arc::clone(&trust_snapshot)); - (loader, invoker, trust_snapshot) + zeph_core::SkillInvokeExecutor::new(Arc::clone(registry), Arc::clone(&trust_snapshot)) + .with_turn_trust_floor(turn_trust_floor.clone()); + (loader, invoker, trust_snapshot, turn_trust_floor) } /// Wires a [`zeph_core::debug_dump::DebugDumper`] into `agent` for `dir`/`format`, shared by @@ -2166,11 +2178,19 @@ pub(crate) type McpToolIdsHandle = Arc> /// /// Returns the gated executor plus the MCP tool-id handle the caller must populate (via /// [`register_mcp_tool_ids`]) once the MCP tool list is known. +/// +/// `turn_trust_floor` seeds the gate's per-turn trust floor (#6701). Pass the same handle +/// returned by [`build_skill_executors`] so `TrustGateExecutor` and `SkillTrustGate` (via +/// `SkillLoaderExecutor`/`SkillInvokeExecutor`) share the identical cell — a call site with +/// no skill-executor counterpart to share with may pass a fresh +/// `zeph_common::TurnTrustFloor::default()`. pub(crate) fn apply_common_tool_gating( inner: zeph_tools::DynExecutor, permission_policy: &zeph_tools::PermissionPolicy, + turn_trust_floor: zeph_common::TurnTrustFloor, ) -> (zeph_tools::DynExecutor, McpToolIdsHandle) { - let gated = zeph_tools::TrustGateExecutor::new(inner, permission_policy.clone()); + let gated = zeph_tools::TrustGateExecutor::new(inner, permission_policy.clone()) + .with_trust_floor(turn_trust_floor); let handle = gated.mcp_tool_ids_handle(); (zeph_tools::DynExecutor(Arc::new(gated)), handle) } @@ -4303,6 +4323,7 @@ mod tests { let (gated, mcp_handle) = apply_common_tool_gating( zeph_tools::DynExecutor(inner), &zeph_tools::PermissionPolicy::default(), + zeph_common::TurnTrustFloor::default(), ); register_mcp_tool_ids(&mcp_handle, std::slice::from_ref(&mcp_tool)); gated.set_effective_trust(zeph_common::SkillTrustLevel::Quarantined); @@ -4347,7 +4368,11 @@ mod tests { )); let policy = zeph_tools::PermissionPolicy::default().with_autonomy(zeph_tools::AutonomyLevel::Full); - let (gated, mcp_handle) = apply_common_tool_gating(zeph_tools::DynExecutor(inner), &policy); + let (gated, mcp_handle) = apply_common_tool_gating( + zeph_tools::DynExecutor(inner), + &policy, + zeph_common::TurnTrustFloor::default(), + ); register_mcp_tool_ids(&mcp_handle, std::slice::from_ref(&mcp_tool)); gated.set_effective_trust(zeph_common::SkillTrustLevel::Trusted); @@ -4410,8 +4435,11 @@ mod tests { let inner: Arc = Arc::new(NoopExec); let policy = zeph_tools::PermissionPolicy::default().with_autonomy(zeph_tools::AutonomyLevel::Full); - let (trust_gated, _handle) = - apply_common_tool_gating(zeph_tools::DynExecutor(inner), &policy); + let (trust_gated, _handle) = apply_common_tool_gating( + zeph_tools::DynExecutor(inner), + &policy, + zeph_common::TurnTrustFloor::default(), + ); let executor = apply_policy_gate_chain(trust_gated, &PolicyGatePieces::default(), None, None); let result = executor.execute_tool_call(&make_tool_call("read")).await; @@ -4471,8 +4499,11 @@ mod tests { let inner: Arc = Arc::new(NoopExec); let policy = zeph_tools::PermissionPolicy::default().with_autonomy(zeph_tools::AutonomyLevel::Full); - let (trust_gated, _handle) = - apply_common_tool_gating(zeph_tools::DynExecutor(inner), &policy); + let (trust_gated, _handle) = apply_common_tool_gating( + zeph_tools::DynExecutor(inner), + &policy, + zeph_common::TurnTrustFloor::default(), + ); let executor = apply_policy_gate_chain(trust_gated, &pieces, Some(&audit_logger), None); let result = executor.execute_tool_call(&make_tool_call("shell")).await; diff --git a/src/daemon.rs b/src/daemon.rs index 16da17768..6bde4317a 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -398,6 +398,7 @@ where std::collections::HashMap, >, >, + turn_trust_floor: zeph_common::TurnTrustFloor, /// Same `Arc` used to build the `get_current_time` tool executor (#6361) — shared so the /// tool and the time-reminder injection agree on "now". clock: std::sync::Arc, @@ -436,6 +437,7 @@ where ) .with_trust_config(config.skills.trust.clone()) .with_trust_snapshot(deps.trust_snapshot) + .with_turn_trust_floor(deps.turn_trust_floor) .with_memory( deps.memory, deps.conversation_id, @@ -856,7 +858,7 @@ pub(crate) async fn run_daemon( std::sync::Arc::new(memory.sqlite().clone()), ) .with_conversation(conversation_id.0); - let (skill_loader_executor, skill_invoke_executor, trust_snapshot) = + let (skill_loader_executor, skill_invoke_executor, trust_snapshot, turn_trust_floor) = agent_setup::build_skill_executors(®istry); // Hoisted out of the composite-executor block below (rather than resolved twice) so it can // also be passed to `apply_code_rag_retriever` once the agent exists (#6022: previously the @@ -899,8 +901,11 @@ pub(crate) async fn run_daemon( // memory + overflow) behind one outermost TrustGateExecutor, matching runner.rs and ACP // (`src/acp.rs`). Previously only the base chain was gated here, so a Quarantined skill // could still reach `memory_save` and any MCP-sourced tool. - let (trust_gated, mcp_ids_handle) = - agent_setup::apply_common_tool_gating(inner_executor, &permission_policy); + let (trust_gated, mcp_ids_handle) = agent_setup::apply_common_tool_gating( + inner_executor, + &permission_policy, + turn_trust_floor.clone(), + ); agent_setup::register_mcp_tool_ids(&mcp_ids_handle, &mcp_tools); // #5958: shared trajectory risk slot — written by begin_turn(), read by PolicyGateExecutor. // Mirrors src/runner.rs; previously the daemon never created this, so TrajectorySentinel @@ -1136,6 +1141,7 @@ pub(crate) async fn run_daemon( mcp_shared_tools, provider_config_snapshot, trust_snapshot, + turn_trust_floor, clock, }; let agent = Box::pin(build_daemon_agent(deps, loopback_channel)).await; @@ -1554,6 +1560,7 @@ mod tests { trust_snapshot: std::sync::Arc::new(parking_lot::RwLock::new( std::collections::HashMap::new(), )), + turn_trust_floor: zeph_common::TurnTrustFloor::default(), clock: std::sync::Arc::new(zeph_common::SystemClock), }; @@ -1646,6 +1653,7 @@ mod tests { trust_snapshot: std::sync::Arc::new(parking_lot::RwLock::new( std::collections::HashMap::new(), )), + turn_trust_floor: zeph_common::TurnTrustFloor::default(), clock: std::sync::Arc::new(zeph_common::SystemClock), }; @@ -1727,6 +1735,7 @@ mod tests { trust_snapshot: std::sync::Arc::new(parking_lot::RwLock::new( std::collections::HashMap::new(), )), + turn_trust_floor: zeph_common::TurnTrustFloor::default(), clock: std::sync::Arc::new(zeph_common::SystemClock), }; @@ -2089,6 +2098,7 @@ mod tests { let (gated, mcp_ids_handle) = agent_setup::apply_common_tool_gating( inner_executor, &zeph_tools::PermissionPolicy::default(), + zeph_common::TurnTrustFloor::default(), ); agent_setup::register_mcp_tool_ids(&mcp_ids_handle, std::slice::from_ref(&mcp_tool)); gated.set_effective_trust(zeph_common::SkillTrustLevel::Quarantined); @@ -2236,6 +2246,7 @@ mod tests { let (trust_gated, mcp_ids_handle) = agent_setup::apply_common_tool_gating( inner_executor, &zeph_tools::PermissionPolicy::default(), + zeph_common::TurnTrustFloor::default(), ); agent_setup::register_mcp_tool_ids(&mcp_ids_handle, std::slice::from_ref(&mcp_tool)); zeph_tools::ToolExecutor::set_effective_trust( @@ -2479,6 +2490,7 @@ mod tests { let (trust_gated, _mcp_ids_handle) = agent_setup::apply_common_tool_gating( zeph_tools::DynExecutor(std::sync::Arc::new(base_executor)), &zeph_tools::PermissionPolicy::default().with_autonomy(zeph_tools::AutonomyLevel::Full), + zeph_common::TurnTrustFloor::default(), ); let policy_config = zeph_tools::PolicyConfig { @@ -2627,7 +2639,7 @@ mod tests { let registry = std::sync::Arc::new(RwLock::new(zeph_skills::registry::SkillRegistry::empty())); - let (skill_loader_executor, skill_invoke_executor, _trust_snapshot) = + let (skill_loader_executor, skill_invoke_executor, _trust_snapshot, _turn_trust_floor) = agent_setup::build_skill_executors(®istry); let composite = zeph_tools::CompositeExecutor::new( diff --git a/src/runner.rs b/src/runner.rs index e007bd0c6..eb903f58e 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -189,6 +189,7 @@ where trust_snapshot: std::sync::Arc< RwLock>, >, + turn_trust_floor: zeph_common::TurnTrustFloor, memory: std::sync::Arc, conversation_id: zeph_memory::ConversationId, session_sink: Option>, @@ -240,6 +241,7 @@ where ) .with_trust_config(config.skills.trust.clone()) .with_trust_snapshot(deps.trust_snapshot) + .with_turn_trust_floor(deps.turn_trust_floor) .with_memory( deps.memory, deps.conversation_id, @@ -2399,7 +2401,7 @@ pub(crate) async fn run(mut cli: Cli) -> anyhow::Result<()> { std::sync::Arc::new(memory.sqlite().clone()), ) .with_conversation(conversation_id.0); - let (skill_loader_executor, skill_invoke_executor, trust_snapshot) = + let (skill_loader_executor, skill_invoke_executor, trust_snapshot, turn_trust_floor) = agent_setup::build_skill_executors(®istry); let base: std::sync::Arc = std::sync::Arc::new(tool_setup.executor); @@ -2431,8 +2433,11 @@ pub(crate) async fn run(mut cli: Cli) -> anyhow::Result<()> { // #5610/#5886: shared TrustGateExecutor wrap, also used by ACP (`src/acp.rs`) and the // daemon (`src/daemon.rs`) so all three entry points gate the full executor tree through // one code path. - let (trust_gated, mcp_ids_handle) = - crate::agent_setup::apply_common_tool_gating(inner_executor, &permission_policy); + let (trust_gated, mcp_ids_handle) = crate::agent_setup::apply_common_tool_gating( + inner_executor, + &permission_policy, + turn_trust_floor.clone(), + ); let policy_gate_pieces = crate::agent_setup::build_policy_gate_pieces(config, &provider).await; let tool_executor = crate::agent_setup::apply_policy_gate_chain( trust_gated, @@ -2713,6 +2718,7 @@ pub(crate) async fn run(mut cli: Cli) -> anyhow::Result<()> { reload_rx, plugin_dirs_supplier, trust_snapshot, + turn_trust_floor, memory: std::sync::Arc::clone(&memory), conversation_id, session_sink: session_sink.clone(), @@ -5068,6 +5074,7 @@ mod tests { reload_rx, plugin_dirs_supplier: || Vec::::new(), trust_snapshot: std::sync::Arc::new(RwLock::new(std::collections::HashMap::new())), + turn_trust_floor: zeph_common::TurnTrustFloor::default(), memory: std::sync::Arc::clone(&memory), conversation_id, session_sink: None, @@ -5160,6 +5167,7 @@ mod tests { reload_rx, plugin_dirs_supplier: || Vec::::new(), trust_snapshot: std::sync::Arc::new(RwLock::new(std::collections::HashMap::new())), + turn_trust_floor: zeph_common::TurnTrustFloor::default(), memory: std::sync::Arc::clone(&memory), conversation_id, session_sink: None, diff --git a/src/serve/agent_factory.rs b/src/serve/agent_factory.rs index 8566d5b1b..c22b09c0b 100644 --- a/src/serve/agent_factory.rs +++ b/src/serve/agent_factory.rs @@ -215,7 +215,7 @@ pub(crate) async fn build_agent_factory( zeph_tools::DynExecutor(deps.tool_executor), )); - let (composed_base, trust_snapshot) = compose_session_tool_tree( + let (composed_base, trust_snapshot, turn_trust_floor) = compose_session_tool_tree( session_tool_executor, &deps.registry, &deps.memory, @@ -236,6 +236,7 @@ pub(crate) async fn build_agent_factory( &deps.policy_gate_pieces, deps.audit_logger.as_ref(), Some((&trajectory_risk_slot, &trajectory_signal_queue)), + turn_trust_floor.clone(), ); let build_agent = move |channel| { @@ -329,6 +330,12 @@ pub(crate) async fn build_agent_factory( // matching src/acp.rs/src/daemon.rs — without this, SkillLoaderExecutor/ // SkillInvokeExecutor's trust bookkeeping would never reach the Agent that reads it. .with_trust_snapshot(trust_snapshot) + // #6701 (S2): wire the same turn trust floor gate_serve_session_executor already + // installed into TrustGateExecutor above — matching src/acp.rs/src/daemon.rs/ + // src/runner.rs. Without this, services.skill.turn_trust_floor stays None for every + // serve session, so a spawned subagent's SpawnContext.turn_trust_floor would be None + // too, falling back to the pre-#6701 set_effective_trust behavior at spawn time. + .with_turn_trust_floor(turn_trust_floor) .with_rl_routing( deps.rl_routing_enabled, deps.rl_learning_rate, @@ -490,6 +497,7 @@ fn gate_serve_session_executor( &zeph_tools::TrajectoryRiskSlot, &zeph_tools::RiskSignalQueue, )>, + turn_trust_floor: zeph_common::TurnTrustFloor, ) -> ( zeph_tools::DynExecutor, crate::agent_setup::McpToolIdsHandle, @@ -497,6 +505,7 @@ fn gate_serve_session_executor( let (trust_gated, mcp_ids_handle) = crate::agent_setup::apply_common_tool_gating( zeph_tools::DynExecutor(tool_executor), permission_policy, + turn_trust_floor, ); // R7: serve has no MCP-provided tools yet (deps.rs's "Known gap") — this empty-slice call // is the exact seam a future MCP-wiring PR must populate with the connected tool list. @@ -618,6 +627,7 @@ pub(super) fn compose_session_tool_tree( ) -> ( Arc, Arc>>, + zeph_common::TurnTrustFloor, ) { let memory_executor = { let mut e = zeph_core::memory_tools::MemoryToolExecutor::with_validator( @@ -637,7 +647,7 @@ pub(super) fn compose_session_tool_tree( let overflow_executor = zeph_core::overflow_tools::OverflowToolExecutor::new(Arc::new(memory.sqlite().clone())) .with_conversation(conversation_id.0); - let (skill_loader_executor, skill_invoke_executor, trust_snapshot) = + let (skill_loader_executor, skill_invoke_executor, trust_snapshot, turn_trust_floor) = crate::agent_setup::build_skill_executors(registry); let composed: Arc = Arc::new(zeph_tools::CompositeExecutor::new( skill_loader_executor, @@ -652,7 +662,7 @@ pub(super) fn compose_session_tool_tree( ), ), )); - (composed, trust_snapshot) + (composed, trust_snapshot, turn_trust_floor) } /// Opens (and replays, per D-10) the durable event log for `session_id`, wraps it in a @@ -2056,6 +2066,7 @@ mod tests { &policy_gate_pieces, None, Some((&trajectory_risk_slot, &trajectory_signal_queue)), + zeph_common::TurnTrustFloor::default(), ); let denied = gated @@ -2148,6 +2159,7 @@ mod tests { &crate::agent_setup::PolicyGatePieces::default(), None, None, + zeph_common::TurnTrustFloor::default(), ); (gated, trajectory_signal_queue) } diff --git a/src/serve/deps.rs b/src/serve/deps.rs index 27458cbfe..08b5143a8 100644 --- a/src/serve/deps.rs +++ b/src/serve/deps.rs @@ -386,7 +386,7 @@ pub(crate) async fn assemble_serve_deps( validation_shell, zeph_tools::DynExecutor(Arc::clone(&tool_executor)), )); - let (composed_for_validation, _trust_snapshot) = + let (composed_for_validation, _trust_snapshot, _turn_trust_floor) = crate::serve::agent_factory::compose_session_tool_tree( tool_executor_for_validation, &core.registry,