Skip to content

fix(skills): stop a proactively-matched Quarantined skill from locking a turn's tools - #6710

Merged
bug-ops merged 1 commit into
mainfrom
fix/6701-quarantine-skill-lockout
Jul 28, 2026
Merged

fix(skills): stop a proactively-matched Quarantined skill from locking a turn's tools#6710
bug-ops merged 1 commit into
mainfrom
fix/6701-quarantine-skill-lockout

Conversation

@bug-ops

@bug-ops bug-ops commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Summary

A skill that merely scored above the matcher's similarity threshold for a turn — never explicitly invoked by the model — could drop the whole 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 subagent spawned in it. Inversely, an explicit invoke_skill of a Quarantined skill returned its sanitized body without downgrading effective_trust for the remainder of the turn, contradicting the documented invariant.

Root causes and fixes (RC-1..RC-3, RC-5; RC-4 split out to #6702):

  • RC-1: active_skill_names was assigned directly from the matcher's output with no trust filter. Agent::filter_active_skills_by_trust now drops Quarantined/Blocked matches from the active set before the fold runs, keeping them visible in the <other_skills> catalog (trust="quarantined"/"blocked" attribute) with a warning naming the promotion command (zeph skill trust <name> trusted).
  • RC-2: retrieval-fallback mode (matcher unavailable/unconfigured) folded trust over the full, unscored registry even though only description-only text is injected in that mode. compute_effective_trust now forces Trusted whenever skill_fallback_mode is set.
  • RC-3: SkillTrustGate::resolve_body refused only Blocked skills. invoke_skill/load_skill of a Quarantined skill now folds the new zeph_common::TurnTrustFloor (monotonic-downgrade-only) to Quarantined.
  • RC-5: subagent spawn/resume applied an inherited trust cap via a plain set_effective_trust, which could restore trust above an already-lower floor. SpawnContext now carries a TurnTrustFloor handle and folds the inherited cap onto it when present. parent_effective_trust_level also reads the floor directly instead of recomputing, closing a fallback-mode regression (found during adversarial review) where a spawned subagent could be capped by the weakest trust level anywhere in the registry instead of the turn's actual enforced trust.

The weakest-link fold itself is unchanged as defense-in-depth for skills actually injected/invoked this turn, now a single shared helper used by both apply_skill_trust_and_gating and the subagent trust-cap computation.

RC-4 (AutoSkill draft-name collisions with native tool IDs) is tracked separately in #6702 and intentionally out of scope here.

Process notes

  • Two adversarial-critique rounds: first verdict significant (found the fallback-mode subagent-cap regression above, plus a missed with_turn_trust_floor wiring gap in src/serve/agent_factory.rs, currently latent since serve wires no SubAgentManager); both fixed, second pass verdict approved.
  • Code review independently re-ran the full CI-matching check suite from a cold rebuild and re-grepped all four entry points (runner.rs/daemon.rs/acp.rs/serve/agent_factory.rs) for correct wiring rather than trusting prior claims — verdict approved.
  • Rebased onto origin/main after PR fix(skills): persist quarantined trust row for AutoSkill drafts at generation time #6705 (fix for AutoSkill trace-extraction drafts shadow native tool names and _quarantine/ has no effect on loading #6702) merged mid-review, touching the same skill_reload.rs file; auto-merged cleanly, full check suite re-run post-rebase.
  • Known, accepted residual (not fixed here, follow-up issue recommended): the parent's TrustGateExecutor trust-floor atomic is shared, not isolated, across parent + all spawned subagents, so a fully independent concurrent write path outside SpawnContext is not covered by this fix.

Closes #6701

Test plan

  • cargo +nightly fmt --check
  • cargo clippy --profile ci --workspace --all-targets --features "desktop,ide,server,chat,pdf,scheduler,testing" -- -D warnings
  • cargo nextest run --config-file .github/nextest.toml --workspace --features "desktop,ide,server,chat,pdf,scheduler" --lib --bins (15304 passed, 0 failed, 36 skipped)
  • RUSTFLAGS="-D warnings" RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-deps --workspace --features "desktop,ide,server,chat,pdf,scheduler"
  • gitleaks protect --staged
  • ~35 new unit/integration tests, including an end-to-end resolve_body(Quarantined) -> bash denied test and fresh-spawn + resume fold-never-raises subagent tests
  • Live-session verification — not yet run, tracked in .local/testing/playbooks/trust-gate.md section 5 and .local/testing/coverage-status.md

@github-actions github-actions Bot added bug Something isn't working size/XL Extra large PR (500+ lines) documentation Improvements or additions to documentation skills zeph-skills crate rust Rust code changes core zeph-core crate labels Jul 28, 2026
@bug-ops
bug-ops enabled auto-merge (squash) July 28, 2026 18:41
…g a turn's tools

A skill that merely scored above the matcher's similarity threshold for a
turn - never explicitly invoked by the model - could drop the whole 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 subagent spawned in it. Inversely, an explicit
invoke_skill of a Quarantined skill returned its sanitized body without
downgrading effective_trust for the remainder of the turn, contradicting the
documented invariant.

Root causes and fixes:

- RC-1: active_skill_names was assigned directly from the matcher's output
  with no trust filter. Agent::filter_active_skills_by_trust now drops
  Quarantined/Blocked matches from the active set before the fold runs,
  keeping them visible in the <other_skills> catalog (trust="quarantined"/
  "blocked" attribute) with a warning naming the promotion command.
- RC-2: retrieval-fallback mode (matcher unavailable/unconfigured) folded
  trust over the full, unscored registry even though only description-only
  text is injected in that mode. compute_effective_trust now forces Trusted
  whenever skill_fallback_mode is set.
- RC-3: SkillTrustGate::resolve_body refused only Blocked skills.
  invoke_skill/load_skill of a Quarantined skill now folds the new
  zeph_common::TurnTrustFloor (monotonic-downgrade-only) to Quarantined.
- RC-5: subagent spawn/resume applied an inherited trust cap via a plain
  set_effective_trust, which could restore trust above an already-lower
  floor. SpawnContext now carries a TurnTrustFloor handle and folds the
  inherited cap onto it when present. parent_effective_trust_level also
  reads the floor directly instead of recomputing, closing a fallback-mode
  regression where a spawned subagent could be capped by the weakest trust
  level anywhere in the registry instead of the turn's actual enforced
  trust.

The weakest-link fold itself is unchanged as defense-in-depth for skills
actually injected/invoked this turn, now a single shared helper used by both
apply_skill_trust_and_gating and the subagent trust-cap computation.

RC-4 (AutoSkill draft-name collisions with native tool IDs) is tracked
separately in #6702 and intentionally out of scope here.

Closes #6701
@bug-ops
bug-ops force-pushed the fix/6701-quarantine-skill-lockout branch from 457bc48 to 541588c Compare July 28, 2026 18:51
@bug-ops
bug-ops merged commit b798893 into main Jul 28, 2026
43 checks passed
@bug-ops
bug-ops deleted the fix/6701-quarantine-skill-lockout branch July 28, 2026 19:01
bug-ops added a commit that referenced this pull request Jul 28, 2026
…on (#6712)

update_trust_for_reloaded_skills's source_kind-mismatch branch adopted the
new source kind's default trust level whenever the stored level was
"active" (not Blocked). Since Quarantined is active and
[skills.trust] local_level defaults to Trusted, relocating a Quarantined
skill's unchanged-content directory out of managed_dir (Hub -> Local)
silently promoted it to Trusted on the next reload, with no review step.
The branch now takes SkillTrustLevel::min_trust of the stored level and the
new source kind's default, so a source_kind change can never move a
skill's effective trust higher than it already had. Symmetrically, moving
a Trusted skill into a source kind with a stricter default now correctly
demotes it too - an intentional fail-closed side effect of the same fix.

The adjacent hash-mismatch branch had the identical defect one line above:
a hash mismatch unconditionally assigned hash_mismatch_level (default
Quarantined) even when the existing row was an explicit operator Blocked,
silently un-blocking it on a one-byte edit. Also clamped through
min_trust.

skill_catalog_items (feeds the TUI mention picker) filtered out only
Blocked skills, so a Quarantined skill still appeared there as a normal,
invocable name. New SkillTrustLevel::is_hidden_from_catalog closes this -
SkillCatalogItem carries no trust field, so hiding is the only option on
this surface. The other two catalog sites originally in scope for this
fix turned out to already be correctly handled by the concurrently-merged
#6710/#6701 fix, which takes an annotate-not-hide strategy instead
(format_skills_catalog's trust_levels parameter renders a trust="..."
attribute, and #6710's own activation filter already excludes
Quarantined/Blocked from matching); this change does not alter that
design.

Closes #6707
Closes #6706
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working core zeph-core crate documentation Improvements or additions to documentation rust Rust code changes size/XL Extra large PR (500+ lines) skills zeph-skills crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proactively-matched Quarantined skill locks the whole turn (and every spawned subagent) out of bash/write/MCP tools

1 participant