Repository navigation
Handoff limits: a token that may not hand off can't hand off through MCP either - #244
Merged
Merged
Conversation
…rent A parent whose stored limit is 0 can no longer hand off through satgate/delegate, and a parent already N levels down is refused at N. The Delegator asks the event publisher through the new HandoffLimiter interface before it mints or carves anything; a publisher that does not implement it (the default) changes nothing.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
matt-dean-git
marked this pull request as ready for review
October 16, 2026 19:55
…and carries it to the child's row HandoffLimiter now answers with the row the handoff is judged by (row id, the parent's depth, the limit), or that the parent should have a row and has none (refused: "this token can't hand off to a helper"). The decision is carried on the delegation event (parentRowId, parentDepth, parentLimit) so the child's row is written from the row that was read, and every child whose handoff a stored row decided is recorded before its key is returned (a child with credits whose row cannot be written gives its carve back). The rule is unexported (handoffAllowed): the gateway does not call it.
… = 0, with or without a stored limit
…v1/mint/delegate does; refusal logs carry no fingerprint or error text Round 5 (Sol B1/B2, Astra B1 on satgate-enterprise#517). - A depth caveat is read with the mint route's own reading (Sscanf %d, leading integer). A chain with a value that does not read, reads below zero, or reads as zero is refused before the stored read, any mint or carve, with or without a stored limit, whoever wrote it. Every other value is a cap, read the same way (3abc reads 3, 2.5 reads 2, 1e3 reads 1). - The stored-limit refusal logs drop the parent field (TokenID is a hash of the macaroon's identifier and signature) and log the error as a class only (errClass: sqlstate, canceled, deadline_exceeded, or the innermost type). - The malformed-value control in caveat_policy_test.go now expects the refusal (a holder-appended "nope" under a cap of 2).
This branch was successfully deployed
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.
The defect
satgate/delegateignores the handoff limit the owner stored for a token. A parent whose stored limit is 0 ("this token may not hand off") can hand off without limit through the MCP tool, and so can a parent already at its limit of N.pkg/mcpserver/delegation.go:179on main:parent.DelegationDepth > 0 && parent.Depth >= parent.DelegationDepth.DelegationDepthis read from the token's owndelegation_depthcaveat, positive values only (caveat_policy.go:97), so a 0 never refuses.delegation_tokens_v2.max_delegation_depth: nil unlimited, 0 may not hand off, N at most N levels). The HTTP handoff enforces it; this tool did not.Found by the review of the agent control plane page: https://github.com/SatGate-io/satgate/issues/comments/6220683233
The fix
mcpserver.HandoffLimiter: anEventPublisherthat can read the stored limit answersHandoffLimit(ctx, parent) (limit int, limited bool, err error).Delegator.Delegateasks it before it mints or carves anything (right after the existing budget-word checks). The one rule ismcpserver.HandoffAllowed: no limit is unlimited; 0 refuses; N refuses a parent already N levels down (parent.Depth, counted from the token'sparent =caveats).clientError, so it leavesHandleDelegateexactly like the depth-limit refusal beside it: JSON-RPC code -32000 (CodeBudgetExhausted), data{"error":"delegation_failed"}, messagethis token can't hand off to a helper(orthis token has reached its handoff limit (N levels)). Nothing is minted, carved, initialised or published, so there is no child row and no new receipt or event.ErrChildTokenFailedsentence; the cause is only logged.NoOpPublisher, every existing publisher) changes nothing: same behaviour as before.delegation_depthcaveat reading (a caveat of 0 is still ignored there, because an unset maxDepth and an explicit 0 mint the same caveat today and existing tokens rely on it).Tests (pkg/mcpserver, through
Delegator.HandleDelegate)delegation_handoff_limit_test.go,delegation_handoff_rule_test.go. Run against main (new files only,delegation.gofrom main),go test ./pkg/mcpserver -run 'StoredHandoff|Publisher'exits 1:TestStoredHandoffLimitZeroRefusesAndCreatesNothing: FAIL on main (want a refusal, got a result, a child token is issued)TestStoredHandoffLimitOneAllowsOneLevel: FAIL on main (the child's own handoff is not refused)TestStoredHandoffLimitUnreadableRefusesWithoutCreatingAnything: FAIL on main (a child is issued)TestStoredHandoffLimitNilIsUnlimited,...MissingRowIsUnlimited,TestPublisherWithoutHandoffLimiterKeepsTodaysBehaviour: pass on main and on this branch (nothing else changes: child and grandchild, budgets carved from the parent as before)TestHandoffAllowedRule: new API, does not compile on main.On this branch:
go vet ./...exit 0;go test -race -count=1 ./pkg/mcpserver/...exit 0.Bump size
For a consumer pinned at
78dd26fe2154(the satgate-enterprise pin on its main):git log --oneline 78dd26fe2154..<head> -- pkg | wc -l= 1 commit, one package (pkg/mcpserver:delegation.goplus two test files). Public main has no other commit underpkg/since that pin. (From the July pseudo-version54be71532e05named in the brief the count is 116.)The companion change in satgate-enterprise adds the reading side (
PersistingEventPublisher.HandoffLimit), which does nothing until gateway/go.mod takes a version with this commit. That PR does not bump it.Head SHA: b3c950d (round 2; round 1 was eb896fb)
Round 2 (review of the pair: satgate-enterprise#517, Grok 6226609395 and Astra 6226830580)
satgate/delegate now decides every handoff from the parent's own stored row, after the parent has been verified, and refuses when the row can't be found:
HandoffLimiteris nowHandoffRecord(ctx, parent) (rowID, depth, limit, limited, err), called with the authenticator's verified parent. rowID "" = no stored limit applies (today's behaviour); rowID set = the row the handoff is judged by, atdepth; an error withHandoffRecordMissing() == true= the parent should have a row and has none: refused, "this token can't hand off to a helper"; any other error: refused withErrChildTokenFailed. Basic types only, so a gateway can implement it while still building against an older version.EventParentRowID,EventParentDepth,EventParentLimit) so the gateway writes the child's row from the row it read (depth + 1, the same limit), not from a second lookup by budget id.ChildPersisterbefore its key is returned; a child with credits whose row cannot be written gets no key and its carve is given back.handoffAllowed); the gateway does not call it.Tests (pkg/mcpserver): the round 1 tests on the new interface, plus a missing row refused for both kinds of child with nothing created, the child row written from the decision before the key for both kinds, and a failed child-row write giving the carve back. The end-to-end MCP cases (policy and Cloud parents, a holder's added caveat, a missing row, limit 1 on both kinds of child) are in satgate-enterprise#517 and were run with this branch substituted for the module (-modfile): all pass; against round 1's eb896fb they fail.
go vet ./...exit 0;go test -race -count=1 ./pkg/mcpserver/...exit 0; gofmt clean.Bump size from the satgate-enterprise pin
78dd26fe2154: 2 commits underpkg/(round 1 and this one), one package,pkg/mcpserver.Round 3 (companion of satgate-enterprise#517; Grok NO-GO 6229299411, Astra NO-GO 6229325996)
No change to this branch in round 3. Head stays b3c950d. The three round 3 fixes are in satgate-enterprise#517 (simulator ids, policy-file zero by shadow decode, and the transfer refund), and none needed a public interface change: the enterprise budget adapter learns that a delegation finished from the delegation event the delegator already publishes after it has recorded the child and decided to return the key (
d.events.Publish(delegationEvent)inHandleDelegate), and learns that it failed from theRefundcall the delegator already makes (giveBack). Nothing here widens what a token can do.Gates from this worktree at b3c950d: gofmt -l pkg/mcpserver: no output;
go vet ./...exit 0;go test -race -count=1 ./pkg/mcpserver/...exit 0 (ok 36.974s). Bump size from the enterprise pin 78dd26f is unchanged: 2 commits underpkg/, one package.CI on this head (b3c950d, unchanged since round 2): Go, Node.js SDK, Python SDK pass (gh pr checks 244).
Round 4 (Sol NO-GO 6234247370, Astra NO-GO 6234257065 on enterprise #517)
Round 4 head: 591a309 (was b3c950d). Enterprise pair: satgate-enterprise#517 at 96e721ece39b0f45737647bb79deda599c21dd08. Report: cos/handoff-depth-1016/REPORT-r4.md on Dean.
satgate/delegate(pkg/mcpserver/delegation.go,carriesZeroDelegationDepth) now refuses, before any stored-limit read, mint, carve or Redis call, every token carrying adelegation_depth = 0caveat anywhere in its chain, whether the issuer wrote it or a holder appended it, and whether or not aHandoffLimiteris wired. Same answer as before for a limit-0 parent ("this token can't hand off to a helper"), one log line ("this token allows no further handoffs") with no token or tenant. The zero is read as/v1/mint/delegatereads it. Positive caps behave exactly as before, including the stricter reading of a remaining depth.Tests (
delegation_zero_depth_test.go): 16 cases (issuer zero, holder zero on unlimited and under a cap of 2, zero then -1 or 9, zero after 9, +0, 00) x with and without a store, both credit 0 and 25: refused, no read of the stored row, no spend, no child budget, no event, parent unchanged; the log line check; positive-cap controls. They fail on b3c950d and pass here. One existing subtest changed:TestDelegate_DepthCapIgnoresLaterWiden/delegation_depth_=_0(a holder zero used to be ignored; it now refuses the first handoff).Gates: gofmt clean, go vet ./... exit 0, go test -race -count=1 ./pkg/mcpserver/... exit 0 (463 PASS). CI: see gh pr checks 244.
Round 5 (companion of satgate-enterprise#517; Sol NO-GO 6238276507 B1 and B2, Astra NO-GO 6238042942 B1)
Head: 4d18b0b (was 591a309). One commit,
pkg/mcpserveronly.git merge-tree --write-treewith origin/main 0e60d5f: clean. Bump size from 78dd26f: 4 pkg commits, all inpkg/mcpserver(caveat_policy.go, delegation.go, errclass.go new, events.go).One answer for every
delegation_depthvalue./v1/mint/delegatereads a depth caveat withfmt.Sscanf("delegation_depth = %d")and refuses a token whose chain has a value that does not read, reads below zero, or reads as zero.Delegatenow refuses exactly that set, beforerefuseHolderBudget, the stored read, any mint or carve, with or without a stored limit, whoever wrote the caveat (readDelegationDepthChain, delegation.go:216; the refusal at :321). Every other value is a cap, read the same way: the smallest wins,3abcreads 3,2.5reads 2,1e3reads 1 (before, these were ignored and left the token uncapped on MCP). A caveat without thedelegation_depth =prefix is not a depth caveat, as on the mint route. Nothing is widened: every change refuses more or caps tighter; a value the mint route accepts is still accepted.TestDelegateDepthValuesGetTheMintRoutesAnswer(25 values, issuer, holder and split placements, with and without a store; refused cases touch no budget, row or event and never read the stored row; accepted cases stop at the cap). On 591a309: 60 of 112 subtests fail.TestDelegateUnreadableDepthRefusalLogsNoValue: the refusal line holds no value. The same corpus runs against the real mint route in the enterprise PR.caveat_policy_test.goTestDelegate_DepthCapIgnoresLaterWiden, subtestdelegation_depth = nope(a holder-appended unreadable value under a cap of 2) now expects the refusal, not a child under cap 2. The= 9subtest and every positive-cap control are unchanged.Stored-refusal logs carry no fingerprint and no error text.
delegation.go:275(parent has no stored row) logs no fields;:278(limit could not be read) logserroras a class only:database error (sqlstate X),canceled,deadline_exceeded, ordriver_error <type>(newerrclass.go; unit testTestErrClassIsClosed). Theparentfield (a hash of the macaroon's identifier and signature) is gone from both. Test:TestDelegateStoredRefusalLogsNoFingerprintOrErrorText(missing row, arbitrary error canary, database error, canceled); on 591a309 all four fail. Sol'ssol_public_logs_test.gopasses on this head. The enterprise PR runs both lines through a real MCP handler and Postgres.Gates:
gofmt -lclean,go vet ./...0,go test -race -count=1 ./pkg/mcpserver/...0 (839 PASS, 0 FAIL, 4 SKIP; the 4 areTestOddNumbersNeverEchoedAtProxysubtests that skip on 591a309 too).