diff --git a/modules/pier/README.md b/modules/pier/README.md index 0d874a38..e82c731f 100644 --- a/modules/pier/README.md +++ b/modules/pier/README.md @@ -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` 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="/.well-known/oauth-protected-resource", scope="read"`; a browser request whose `Origin` is not in `mcp.allowed_origins` is refused with 403 before any body is read. diff --git a/modules/pier/events.go b/modules/pier/events.go index e9b0ebd9..03ac1a77 100644 --- a/modules/pier/events.go +++ b/modules/pier/events.go @@ -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 } diff --git a/modules/pier/filter.go b/modules/pier/filter.go index 9904ea47..2d30168e 100644 --- a/modules/pier/filter.go +++ b/modules/pier/filter.go @@ -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. diff --git a/modules/pier/stateless.go b/modules/pier/stateless.go index e4e77eb7..20325677 100644 --- a/modules/pier/stateless.go +++ b/modules/pier/stateless.go @@ -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{} diff --git a/modules/pier/widening_test.go b/modules/pier/widening_test.go index c03fb24b..68b56870 100644 --- a/modules/pier/widening_test.go +++ b/modules/pier/widening_test.go @@ -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) + } + }) +}