feat(trust): scoped tokens — kb:read / kb:propose / kb:approve / kb:admin - #725
Open
minion1227 wants to merge 7 commits into
Open
feat(trust): scoped tokens — kb:read / kb:propose / kb:approve / kb:admin#725minion1227 wants to merge 7 commits into
minion1227 wants to merge 7 commits into
Conversation
…dmin a bearer token was all-or-nothing: hold it and you could call all 73 kb.* methods, kb.approve included. fine for a solo human, wrong for a ci job that should only read or a triage bot that should only propose. the config-level trusted-agent flag can only widen the gate; this is the first thing in vouch that can narrow it — withholding kb:approve by default *is* the review gate, expressed as a credential. four coarse scopes over the method list rather than a per-method allowlist, because a per-method grammar makes every new kb.* method a config migration for every deployment. two rules keep this safe to ship into existing deployments. an unscoped credential means all scopes, so every token issued before this keeps working exactly as it did — an empty scope set is "unrestricted", never "denied". and every method must be classified: METHOD_SCOPES is exhaustive over capabilities.METHODS with a test enforcing it, so a newly-added method cannot silently land unreachable for scoped callers. an unclassified method is denied to a scoped caller — fails closed, because a deny-list in a trust-centric system fails open. enforcement lives at the two dispatch points that already exist: handle_request for jsonl and http, and wrap_tool_fn for mcp, both routed through trust.require_scope so the three surfaces inherit one implementation. the check runs before the handler, so a refused call cannot have side effects on its way to being refused. the agent registry supplies the scopes: a registered subject's scopes ride onto VouchTrust at the http chokepoint, and an unregistered subject stays unscoped. registration now validates scopes, so a typo cannot mint a credential with powers nobody asked for. kb.capabilities reports the caller's effective scopes and allowed methods, so an agent discovers what it may do instead of failing method by method. the trust block only grows a scopes key when the credential is actually scoped. stacked on vouchdev#607: the registry is where scopes are stored. Closes vouchdev#608
the schema drift check compares schemas/ against what scripts/gen_schemas.py emits from models.py. adding the per-credential scopes block to Capabilities changed that output, so the committed schema went stale. this is the sixth registration site a new field can miss — CLAUDE.md's four, plus hot_memory's coverage map, plus this. worth a line in the contributor notes separately.
`METHOD_SCOPES` was exhaustive over `capabilities.METHODS` when this branch was written. four methods have merged to `test` since — kb.capture_correction (vouchdev#679), and kb.list_goals / kb.propose_goal / kb.set_goal_status (vouchdev#676) — leaving the table stale by four and all three matrix jobs red on `test_every_method_is_classified`. that failure is the guard working. by this branch's own rule an unclassified method is denied to a scoped caller, so merging as-is would have silently locked every scoped credential out of goal writes and correction capture. the right failure direction, but still a regression, and invisible from the diff. the classifications: - kb.list_goals -> kb:read. a listing that cannot change durable state, beside kb.list_sessions. - kb.propose_goal -> kb:propose. files a PENDING goal a human approves. - kb.capture_correction -> kb:propose. routes exclusively through propose_quoted_claim and has no import of approve. - kb.set_goal_status -> kb:approve, not kb:propose. this is the one that departs from the suggestion on the closing comment, and deliberately: it is a lifecycle op living in lifecycle.py beside supersede/archive/confirm, it mutates an already approved goal in place, and server.py documents it as "the only write path for goal status" — status moves never go through a second proposal. filing it under kb:propose would let a propose-only credential change durable state with no review, which is the exact boundary this module exists to hold. no other change: the scope machinery, the two safety rules and the tests are as reviewed. Co-authored-by: Cursor <cursoragent@cursor.com>
github-actions
Bot
added
cli
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
2 participants
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.
closes #608
supersedes #692, which you closed on the
METHOD_SCOPESstaleness. githubrefused to reopen it (
reopenPullRequestfails server-side now thattesthasmoved), so this is the same branch —
feat/scoped-tokens— brought current andcarrying one new commit. the scope machinery, the two safety rules and the tests
are exactly as you reviewed.
what changed since #692
merged current
test(through #693/#719/#720) and added the four missingentries to
METHOD_SCOPES. it is now exhaustive again: 77 methods, 77classified,
test_every_method_is_classifiedand the two-way guard both green,all three matrix jobs pass.
the one place this departs from your suggestion
you read the goal writes as
kb:propose. three of the four match that;kb.set_goal_statusis filed underkb:approveinstead, and it seemed worthsaying why rather than quietly differing.
it is a lifecycle op, not a proposal. it lives in
lifecycle.pybesidesupersede/archive/confirm— all alreadykb:approve— it mutates analready-approved goal in place, and
server.pydocuments it as "the only writepath for goal status", with
kb_propose_goaladding that "status moves afterapproval go through
kb.set_goal_status, never through a second proposal".so classifying it
kb:proposewould hand a propose-only credential the abilityto flip an approved goal to
doneorabandonedwith no review. that is asmaller hole than the one you closed this PR over, but it is the same kind of
hole, and it is the boundary this module exists to hold.
kb:readforkb.list_goalsfollows the same logic in the other direction — it is a listing,and filing it as a write would lock read-only credentials out of it for no
reason.
happy to move
set_goal_statustokb:proposeif you'd rather; it is aone-line change and your call, i just didn't want to make it silently.
the general problem
this PR went stale because
METHOD_SCOPEShas to trackMETHODSacross everyother PR that adds a method, and nothing warns you until the matrix goes red on
a branch that was green when it was written. the exhaustiveness test catches it,
which is the right safety net, but it catches it late. if that turns out to be a
recurring tax i am happy to follow up with something that fails at import rather
than at test time — out of scope here, and this PR should land as-is first.