Repository navigation
feat(skills): withhold untrusted skills, and vouch for them by hash - #82
Merged
Merged
Conversation
Containment bounds what a skill can *reach*; it does nothing about the threat that matters, which is that a `SKILL.md` is instructions the model obeys. This adds the other half — trust decides whether a skill is offered at all. An untrusted skill is **neither listed nor readable**, not listed-with-a-warning. The listing is itself a foothold: only `name` and `description` are needed to steer a model, and both are injected for every discovered skill whether or not the body is ever read. Withholding it means unvetted text never enters the context window. Trust is **declared by the operator, never by the skill**. A `SKILL.md` cannot vouch for itself — a hostile one would simply say it is trusted — so the record lives outside it, in `$XDG_CONFIG_HOME/agent/skills.trust.json`, keyed by the skill's **content hash** over its instructions and bundled files. "I reviewed this" is worth nothing if the bytes can change underneath the review, so an edit un-vouches the skill (pinned by a test). Decisions, and why: - The operator's own trees are trusted by default — `--skills`, the project's `./skills`, the user-level shelf. Nothing arrives here without them putting it there (there is no fetch), so a promotion step for every skill they wrote would be friction with no threat behind it. - `--skills-untrusted <dir>` names a directory taken from elsewhere: that is the quarantine, and it needs no per-skill ceremony. - A **blocked** verdict refuses a skill whatever else says — a positive finding is not overridden by a directory being trusted. Nothing writes `blocked` yet (the scan step will); the precedence is pinned by a test now. - A record that cannot be parsed is an **error**, not an empty record: silently ignoring it would quietly un-block whatever it blocked. `--skills-trust <name>` records a skill's hash and exits; `--skills-list` prints the discovered skills with their level, hash and status, and needs no MCP server. Refs #78 — this is the trust half; the supply chain (fetch/cache/version) stays a decision.
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
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.
What
The trust half of #78 — the half that actually addresses "is this skill malicious". Containment (#81) bounds what a skill can reach; this decides whether the model is given it at all.
An untrusted skill is neither listed nor readable. Not listed-with-a-warning: the listing is itself a foothold, because only
nameanddescriptionare needed to steer a model and both are injected for every discovered skill whether or not the body is ever read. Withholding means unvetted text never enters the context window.Trust is declared by the operator, never by the skill — a hostile
SKILL.mdwould simply say it is trusted — so the record lives outside it, in$XDG_CONFIG_HOME/agent/skills.trust.json, keyed by the skill's content hash over its instructions and bundled files. An edit therefore un-vouches the skill: "I reviewed this" is worth nothing if the bytes can change underneath the review.Decisions (worth a look in review)
--skills, project./skills, the user shelf)--skills-untrusted <dir>is the quarantineblockedyet — the scan step will — but the precedence is pinned by a test now.Surface
--skills-untrusted <dir>(repeatable; also a config keyskillsUntrusted)--skills-trust <name>— records the hash, then exits--skills-list— prints level, hash and trust status; needs no MCP serverTests
src/skill-trust.test.ts(12): hash stability and change (instructions and bundled files), default trust, the quarantine, vouch-by-hash including the edit case, blocked-over-trusted, unreadable-skill refusal, record round-trip, malformed records, entry replacement, path resolution.--skills-listwith no server; a marked directory withheld from both the prompt and the tool list with the reason on stderr; vouch-then-use.pnpm build/typecheck/lintgreen;pnpm test362 passed / 5 skipped; coverage 90.7 / 84.4 / 89.9 / 90.7.Refs #78. The supply chain (fetch/cache/version) remains a decision and is untouched.