fix(19.7): RA-WR-03 a guard error while re-filtering no longer narrows a session

catalog.allowedReport returns the allowed lists together with the first
provider error. Widening (stateful) and the stateless refresh leave the
cached server and its recorded fingerprint untouched on an error, so the
next request retries instead of the tools staying hidden until the next
rotation or event. A fan-out recompute still applies its fail-closed diff
but records no fingerprint, so the binding's next request re-filters.

TestWideningGuardError fails the guard during a widening on both paths and
during a permissions recompute, then asserts that the next request lists
every tool once the provider answers again.
This commit is contained in:
2026-10-23 20:07:51 +02:00
parent f604e6de64
commit e7630e6762
5 changed files with 146 additions and 28 deletions

View File

@@ -41,7 +41,7 @@ Use cases:
- Fan-out (D-26): every `pact.MCPSubjectChanged` fired on the application is published through the store and applied on every instance: a permission or link change recomputes the tools and prompts of the subject's sessions and stateless servers (the SDK then sends `notifications/tools/list_changed` and `notifications/prompts/list_changed`), a revocation or deletion closes those sessions, drops their records and evicts the stateless servers; a link event also drops the link cache at once. A revocation naming `TokenID` ends only the sessions whose subject under the event's guard is that user with that token id and no client id (one personal token); one naming `ClientID` ends only that OAuth client's sessions of the user; one naming neither, and every deletion, ends all sessions of the subject.
- Client address: before resolving a bearer, pier puts the client address on the resolvers' context with `towel.WithClientIP`, taken from the host when it offers one (surf applies `http.trusted_proxies`) or from the request's `RemoteAddr`, so a resolver can log a failed attempt with it.
- Hybrid protocol (D-32): a request whose `Mcp-Protocol-Version` is 2026-07-28 or later, or whose first JSON-RPC method is `server/discover`, goes to the SDK's stateless handler over an in-process LRU of per-identity servers keyed by binding (capacity `mcp.sessions.max_per_binding` times 16); every other request goes to the stateful router. Both run behind the same bearer verification. A server evicted from the LRU closes its sessions, and a stateless `subscriptions/listen` stream re-verifies its bearer every minute and ends when it no longer resolves to its binding, as the stateful GET stream does.
- Widening on return (A-WR-04): when a binding comes back with a different identity (more or fewer scopes, a backend subject added or dropped), its stateful session or stateless server is re-filtered on that request, so newly allowed tools and prompts appear at once (with `list_changed`) instead of waiting for an event. Token ids play no part in this, so an access-token rotation alone re-filters nothing. A stateless server is shared by every live token of its binding, so a return with less reach only adds there and never removes: the receiving middleware hides, per request, what that request's identity may not use, and a call of a wider token is never cut off by a narrower one.
- Widening on return (A-WR-04): when a binding comes back with a different identity (more or fewer scopes, a backend subject added or dropped), its stateful session or stateless server is re-filtered on that request, so newly allowed tools and prompts appear at once (with `list_changed`) instead of waiting for an event. Token ids play no part in this, so an access-token rotation alone re-filters nothing. A stateless server is shared by every live token of its binding, so a return with less reach only adds there and never removes: the receiving middleware hides, per request, what that request's identity may not use, and a call of a wider token is never cut off by a narrower one. A guard provider error during this re-filtering is not a refusal: the cached server stays as it was and the next request retries (logged as `pier: widening deferred`); a fan-out recompute that meets one hides the failing items and leaves the server to be re-filtered by the binding's next request.
- Protected resource metadata (RFC 9728, `pier.PRMScopes`): `GET /.well-known/oauth-protected-resource` and `GET /.well-known/oauth-protected-resource<mcp.path>` answer the resource, the authorization server (the published verifier's resource origin, else `app.url`), the supported scopes and the bearer method; a `mcp.resource` that differs from the verifier's resource is a boot warning.
- Prompts (D-28): prefixed and filtered like tools, required arguments checked before `Render` runs; a prompt never dispatches a route. A prompt declared with `Writes` (it directs tools that are not read-only) sits under the read/write ceiling and is hidden from an identity without `write`.
- A 401 carries `WWW-Authenticate: Bearer resource_metadata="<app.url>/.well-known/oauth-protected-resource<mcp.path>", scope="read"`; a browser request whose `Origin` is not in `mcp.allowed_origins` is refused with 403 before any body is read.

View File

@@ -105,9 +105,9 @@ func (p *Pier) recomputeSession(ctx context.Context, ls *localSession, relink bo
p.identities.refreshBackend(ctx, &id)
ls.identity.set(id)
}
tools, prompts := p.catalog.allowed(ctx, id)
tools, prompts, err := p.catalog.allowedReport(ctx, id)
applyDiff(ls.server, p, id, tools, prompts, ls.tools, ls.prompts)
ls.fingerprint = id.fingerprint()
ls.fingerprint = recomputedFingerprint(id, err)
ls.mu.Unlock()
}
@@ -121,13 +121,25 @@ func (p *Pier) recomputeStateless(ctx context.Context, e *statelessEntry, relink
p.identities.refreshBackend(ctx, &id)
e.identity.set(id)
}
tools, prompts := p.catalog.allowed(ctx, id)
tools, prompts, err := p.catalog.allowedReport(ctx, id)
applyDiff(e.server, p, id, tools, prompts, e.tools, e.prompts)
e.fingerprint = id.fingerprint()
e.fingerprint = recomputedFingerprint(id, err)
e.widened = nil
e.mu.Unlock()
}
// recomputedFingerprint is the fingerprint a recompute records: the
// identity's, or none when a guard check failed (the failing items stay
// hidden, which is safe for an event), so the next request of the binding
// re-filters instead of keeping them hidden until another event (RA-WR-03).
func recomputedFingerprint(id Identity, err error) string {
if err != nil {
logger().Warn("pier: recompute incomplete, re-filtered on the next request", "binding", id.Binding, "error", err)
return ""
}
return id.fingerprint()
}
// widen re-filters a stateful session's server for the identity of the
// request that reached it when that identity's fingerprint differs from
// the one the server was last filtered for (A-WR-04): a binding that
@@ -146,7 +158,14 @@ func (p *Pier) widen(ctx context.Context, srv serverFeatures, id Identity, fp *s
// still end within mcp.call_timeout: the caller holds a lock.
ctx, cancel := p.guardContext(ctx)
defer cancel()
tools, prompts := p.catalog.allowed(ctx, id)
// A failed check is not a refusal: the server and *fp stay as they are,
// so the next request retries instead of the tools staying hidden until
// the fingerprint changes again (RA-WR-03).
tools, prompts, err := p.catalog.allowedReport(ctx, id)
if err != nil {
logger().Warn("pier: widening deferred", "binding", id.Binding, "error", err)
return
}
applyDiff(srv, p, id, tools, prompts, haveTools, havePrompts)
*fp = next
}

View File

@@ -2,6 +2,7 @@ package pier
import (
"context"
"fmt"
"git.golem15.com/golem15/summercms/modules/pact"
)
@@ -15,53 +16,70 @@ import (
// per-identity server and re-checks every list and call for the current
// request's identity (D-08, D-25).
func (c *catalog) allowed(ctx context.Context, id Identity) ([]*tool, []*prompt) {
tools, prompts, _ := c.allowedReport(ctx, id)
return tools, prompts
}
// allowedReport is allowed plus the first provider error, so a caller that
// re-filters a cached server can tell a refusal from a failed check
// (RA-WR-03): widening leaves the server as it is on an error and retries on
// the next request, and a fan-out recompute marks the server for that retry.
// Every failed check is still logged and hides its item.
func (c *catalog) allowedReport(ctx context.Context, id Identity) ([]*tool, []*prompt, error) {
var first error
var tools []*tool
for _, t := range c.tools {
if c.allowsTool(ctx, id, t) {
ok, err := c.allowsTool(ctx, id, t)
if err != nil {
logger().Warn("pier: guard check failed", "tool", t.name, "guard", t.guardName, "error", err)
if first == nil {
first = fmt.Errorf("tool %s: %w", t.name, err)
}
continue
}
if ok {
tools = append(tools, t)
}
}
var prompts []*prompt
for _, p := range c.prompts {
if c.allowsPrompt(ctx, id, p) {
ok, err := c.allowsPrompt(ctx, id, p)
if err != nil {
logger().Warn("pier: guard check failed", "prompt", p.name, "guard", p.guardName, "error", err)
if first == nil {
first = fmt.Errorf("prompt %s: %w", p.name, err)
}
continue
}
if ok {
prompts = append(prompts, p)
}
}
return tools, prompts
return tools, prompts, first
}
func (c *catalog) allowsTool(ctx context.Context, id Identity, t *tool) bool {
func (c *catalog) allowsTool(ctx context.Context, id Identity, t *tool) (bool, error) {
subject := id.subjectFor(t.guardName)
if subject == nil {
return false
return false, nil
}
if !t.readOnly && !id.writes() {
return false
return false, nil
}
ok, err := allows(ctx, t.guard, *subject, t.req, t.decl.Permissions, t.method)
if err != nil {
logger().Warn("pier: guard check failed", "tool", t.name, "guard", t.guardName, "error", err)
return false
}
return ok
return allows(ctx, t.guard, *subject, t.req, t.decl.Permissions, t.method)
}
func (c *catalog) allowsPrompt(ctx context.Context, id Identity, p *prompt) bool {
func (c *catalog) allowsPrompt(ctx context.Context, id Identity, p *prompt) (bool, error) {
// A prompt that directs write tools sits under the read/write ceiling
// like a tool that is not read-only (P-28).
if p.decl.Writes && !id.writes() {
return false
return false, nil
}
subject := id.subjectFor(p.guardName)
if subject == nil {
return false
return false, nil
}
ok, err := allows(ctx, p.guard, *subject, pact.MCPRequirements{}, p.decl.Permissions, "")
if err != nil {
logger().Warn("pier: guard check failed", "prompt", p.name, "guard", p.guardName, "error", err)
return false
}
return ok
return allows(ctx, p.guard, *subject, pact.MCPRequirements{}, p.decl.Permissions, "")
}
// allows asks the provider, method-aware when it supports it.

View File

@@ -203,7 +203,12 @@ func (sp *statelessPath) refresh(ctx context.Context, e *statelessEntry, id Iden
}
ctx, cancel := sp.p.guardContext(ctx)
defer cancel()
tools, prompts := sp.p.catalog.allowed(ctx, id)
tools, prompts, err := sp.p.catalog.allowedReport(ctx, id)
if err != nil {
// Not recorded as widened: the next request retries (RA-WR-03).
logger().Warn("pier: widening deferred", "binding", id.Binding, "error", err)
return
}
addMissing(e.server, sp.p, id, tools, prompts, e.tools, e.prompts)
if e.widened == nil {
e.widened = map[string]bool{}

View File

@@ -2,6 +2,7 @@ package pier_test
import (
"context"
"errors"
"net/http"
"slices"
"sync/atomic"
@@ -179,3 +180,78 @@ func TestStatelessSharedBinding(t *testing.T) {
t.Fatalf("the wide token's call answered a tool error: %s", resultText(t, a.res))
}
}
// flakyGuard is the acme guard whose checks fail (as a pool timeout would)
// while fail is set.
type flakyGuard struct {
acmeGuard
fail atomic.Bool
}
func (g *flakyGuard) MCPAllows(context.Context, pact.MCPSubject, pact.MCPRequirements, []string) (bool, error) {
if g.fail.Load() {
return false, errors.New("pool timeout")
}
return true, nil
}
// TestWideningGuardError pins RA-WR-03: a guard error while a returning
// binding is re-filtered is not a refusal. The cached server is left as it
// is and the next request retries, so once the provider answers again the
// binding lists every tool it may use, on both paths. A fan-out recompute
// that fails hides the failing tools (safe for an event) and leaves the
// session to be re-filtered by its next request.
func TestWideningGuardError(t *testing.T) {
guard := &flakyGuard{}
verifier := newFakeVerifier(nil)
in := startInstance(t, newApp(t, appSpec{
mcp: "enabled: false\nsessions:\n store: memory\n",
guards: []pact.MCPGuard{guard},
verifier: verifier,
}), acmePlugin{tools: []pact.MCPTool{echoTool(), showTool()}})
t.Cleanup(in.pier().StopReaper)
readWrite := []string{"acme_echo", "acme_show"}
for _, path := range []struct {
name, version, token, client string
user uint
}{
{"stateless", "newest", "oas_at_flaky_stateless", "c30", 30},
{"stateful", "", "oas_at_flaky_stateful", "c31", 31},
} {
t.Run("widening on "+path.name, func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
verifier.setGrant(path.token, grant(path.user, path.client, path.user, "read"))
cs := sdkClient(t, ctx, in.url(), path.token, clientOpts{version: path.version})
if got := listNames(t, ctx, cs); !slices.Equal(got, []string{"acme_show"}) {
t.Fatalf("read grant lists %v", got)
}
verifier.setGrant(path.token, grant(path.user, path.client, path.user, "read", "write"))
guard.fail.Store(true)
listNames(t, ctx, cs) // the widening fails; this answer hides what errored
guard.fail.Store(false)
if got := listNames(t, ctx, cs); !slices.Equal(got, readWrite) {
t.Fatalf("after the provider recovered: %v, want %v", got, readWrite)
}
})
}
t.Run("recompute on an event", func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
verifier.setGrant("oas_at_flaky_event", grant(32, "c32", 32, "read", "write"))
changed := make(chan struct{}, 4)
cs := sdkClient(t, ctx, in.url(), "oas_at_flaky_event", clientOpts{changed: changed})
if got := listNames(t, ctx, cs); !slices.Equal(got, readWrite) {
t.Fatalf("before = %v", got)
}
guard.fail.Store(true)
fireSubjectChanged(t, in.app, "acme", 32, pact.MCPSubjectPermissions)
waitChanged(t, changed)
guard.fail.Store(false)
if got := listNames(t, ctx, cs); !slices.Equal(got, readWrite) {
t.Fatalf("after the provider recovered: %v, want %v", got, readWrite)
}
})
}