fix(skills): gate load_skill through the same trust pipeline as invoke_skill#6062
Merged
Conversation
github-actions
Bot
added
bug
bug-ops
enabled auto-merge (squash)
July 11, 2026 17:45
…e_skill SkillLoaderExecutor returned raw SKILL.md bodies for any registered skill, including Blocked and Quarantined ones, with no refusal or sanitization — the turn-level TrustGateExecutor gate never inspects the skill_name argument itself, so a load_skill call naming a Blocked/Quarantined skill bypassed the entire trust defense-in-depth pipeline that invoke_skill already enforced. Extract the shared trust-gating pipeline (Blocked refusal, sanitize/wrap by trust level, blake3 integrity re-check, skill_name sanitization) into a new SkillTrustGate used by both SkillLoaderExecutor and SkillInvokeExecutor, and add an agent_setup::build_skill_executors helper that constructs both executors around one shared trust_snapshot Arc, replacing five duplicated inline construction sites across runner.rs, acp.rs, and daemon.rs. Closes #6050 Closes #6049
bug-ops
force-pushed
the
fix/6050-load-skill-trust-check
branch
from
July 11, 2026 17:50
db6ba43 to
a752453
Compare
This was referenced Jul 11, 2026
bug-ops
added a commit
that referenced
this pull request
Jul 11, 2026
…rough SkillTrustGate SqliteStore::set_requires_trust_check had zero production callers, so the per-invocation blake3 integrity re-check could never be armed on any real deployment despite being fully wired on the consumption side since #4306/ #6062. Adds --require-check to `zeph skill trust <name> <level>` (CLI) and `/skill trust <name> <level> --require-check` (in-session), both calling the previously-unreachable setter. Separately, `zeph skill invoke <name>` was a hand-rolled reimplementation of skill trust gating that predated #6062's consolidation onto the shared SkillTrustGate: it never checked requires_trust_check and echoed the raw unsanitized skill name into its not-found error. SkillTrustGate and SkillBodyResolution are now pub at the zeph-core crate root, and SkillCommand::Invoke calls SkillTrustGate::resolve_body directly so the CLI preview can no longer drift from the agent-facing tools.
bug-ops
added a commit
that referenced
this pull request
Jul 11, 2026
…rough SkillTrustGate SqliteStore::set_requires_trust_check had zero production callers, so the per-invocation blake3 integrity re-check could never be armed on any real deployment despite being fully wired on the consumption side since #4306/ #6062. Adds --require-check to `zeph skill trust <name> <level>` (CLI) and `/skill trust <name> <level> --require-check` (in-session), both calling the previously-unreachable setter. Separately, `zeph skill invoke <name>` was a hand-rolled reimplementation of skill trust gating that predated #6062's consolidation onto the shared SkillTrustGate: it never checked requires_trust_check and echoed the raw unsanitized skill name into its not-found error. SkillTrustGate and SkillBodyResolution are now pub at the zeph-core crate root, and SkillCommand::Invoke calls SkillTrustGate::resolve_body directly so the CLI preview can no longer drift from the agent-facing tools.
bug-ops
added a commit
that referenced
this pull request
Jul 11, 2026
…rough SkillTrustGate (#6090) SqliteStore::set_requires_trust_check had zero production callers, so the per-invocation blake3 integrity re-check could never be armed on any real deployment despite being fully wired on the consumption side since #4306/ #6062. Adds --require-check to `zeph skill trust <name> <level>` (CLI) and `/skill trust <name> <level> --require-check` (in-session), both calling the previously-unreachable setter. Separately, `zeph skill invoke <name>` was a hand-rolled reimplementation of skill trust gating that predated #6062's consolidation onto the shared SkillTrustGate: it never checked requires_trust_check and echoed the raw unsanitized skill name into its not-found error. SkillTrustGate and SkillBodyResolution are now pub at the zeph-core crate root, and SkillCommand::Invoke calls SkillTrustGate::resolve_body directly so the CLI preview can no longer drift from the agent-facing tools.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SkillLoaderExecutor(theload_skilltool) returned rawSKILL.mdbodies for any registered skill, including ones classifiedBlockedorQuarantined, with no refusal or sanitization. The turn-levelTrustGateExecutorgate does not close this gap: it only deniesload_skillwhen the turn's foldedeffective_trustisQuarantined, and never inspects theskill_nameargument itself — so aload_skillcall naming aBlocked/Quarantinedskill bypassed the entire trust defense-in-depth pipeline thatinvoke_skillalready enforced.skill_namesanitization on every output path) into a newSkillTrustGate, used by bothSkillLoaderExecutorandSkillInvokeExecutorso the two tools cannot drift apart in the future.agent_setup::build_skill_executorshelper that constructs both executors around one sharedtrust_snapshotArc, replacing five duplicated inline construction sites acrosssrc/runner.rs,src/acp.rs, andsrc/daemon.rs(production and test helpers).Test plan
cargo +nightly fmt --checkcargo clippy --profile ci --workspace --all-targets --features "desktop,ide,server,chat,pdf,scheduler,testing" -- -D warningscargo nextest run --config-file .github/nextest.toml --workspace --features "desktop,ide,server,chat,pdf,scheduler" --lib --bins(12835 passed, 35 skipped, 0 failed)cargo test --doc --workspace --features "desktop,ide,server,chat,pdf,scheduler"RUSTFLAGS="-D warnings" RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-deps --workspace --features "desktop,ide,server,chat,pdf,scheduler"skill_loader.rsmirroringskill_invoker.rs's trust tests: blocked-refused-without-body-read, non-trusted-sanitized, quarantined-wrapped, trusted-verbatim, missing-row-defaults-to-Trusted, not-found sanitizesskill_name, tamperedrequires_trust_checkskill caught by integrity re-check.local/testing/playbooks/skill-trust-cli.mdand.local/testing/coverage-status.mdupdatedCloses #6050
Closes #6049