fix(19.7): RA-WR-05 keep prompt Render errors out of the client's answer
promptHandler logs a failing Render's error with the prompt name and answers the fixed JSON-RPC internal error "pier: the prompt could not be rendered", as dispatch does for tool calls (A-WR-08), so database or provider text never reaches the agent or its transcript. TestPromptRenderError renders a prompt that fails with a database error naming its host and asserts, on both paths, that the client sees only the fixed text and the cause is in the log.
This commit is contained in:
@@ -121,7 +121,7 @@ What pier does with it at boot:
|
||||
- **Input.** `Input` is the zero value of a struct. A `path:"id"` field fills the `{id}` segment, a `query:"q"` field goes to the query string, every other field forms the JSON body; a `jsonschema:"..."` tag is the field's description and a field without `omitempty` is required. pier reflects the input schema from the struct, validates every call's arguments against it, and forwards the body as the agent sent it minus the path and query keys, so an update route that checks key presence sees exactly what the agent set. A `GET` or `DELETE` tool may carry only path and query fields, and an embedded struct on one fails boot (its promoted fields would be body fields). A `path:` field must be required: one with `omitempty` or `omitzero` fails boot. At call time pier passes a route only path values the HTTP mux could produce: a missing value, an empty, `.` or `..` segment, or a `/` inside a single-segment value is a tool error before the route runs. Numbers keep their literal digits in path and query values, so integer ids above 2^53 stay exact. `Output`, when set, is the struct the 2xx body matches and becomes the output schema.
|
||||
- **Annotations.** `GET` is read-only and idempotent, `PUT` idempotent, `DELETE` destructive; `ReadOnly`, `Destructive`, `Idempotent` and `OpenWorld` override. The read-only hint also decides what a read-only grant sees (below).
|
||||
- **Results.** A 2xx body is returned as text plus structured content (an object as is, any other JSON wrapped as `{"data": …}`); a 4xx or 5xx is a tool error carrying the route's own body, so the agent reads the real validation errors. A failure of the guard provider itself (for example a database error while resolving the principal) is the fixed tool error `pier: the tool could not be run for this caller`, logged server-side. The dispatched request carries the MCP request's `Host`, `X-Forwarded-Proto` and `X-Forwarded-Host`, so a route that builds absolute links builds them as over HTTP.
|
||||
- **Prompts.** A `pact.MCPPrompt` is prefixed and filtered like a tool, names its guard, declares its arguments, and renders its messages in `Render`; required arguments are checked first. A prompt never dispatches a route. Set `Writes` on a prompt whose text directs tools that are not read-only (for example "draft a post, then create it"): it then sits under the same read/write ceiling as those tools, so a read-only grant is not offered a prompt it cannot follow.
|
||||
- **Prompts.** A `pact.MCPPrompt` is prefixed and filtered like a tool, names its guard, declares its arguments, and renders its messages in `Render`; required arguments are checked first. A prompt never dispatches a route. An error `Render` returns is logged, and the agent gets only a fixed "the prompt could not be rendered" error. Set `Writes` on a prompt whose text directs tools that are not read-only (for example "draft a post, then create it"): it then sits under the same read/write ceiling as those tools, so a read-only grant is not offered a prompt it cannot follow.
|
||||
- **Descriptions.** `Title` and `Description` are static text, at most 1024 characters, passed through unchanged. Keep them factual one-liners and never build them from data: tool text is read by the model, so data-derived text would be a prompt-injection surface.
|
||||
|
||||
## Identity and sessions
|
||||
|
||||
@@ -43,7 +43,7 @@ Use cases:
|
||||
- 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. Past capacity the least recently used idle server is evicted (and its sessions closed); a server that is serving a request, a `subscriptions/listen` stream included, is never evicted, so capacity never ends a quiet listener or a tool call in flight, and the LRU stays above capacity while every server is busy. 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. 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`.
|
||||
- Prompts (D-28): prefixed and filtered like tools, required arguments checked before `Render` runs; a prompt never dispatches a route. A `Render` error is logged server-side and answered with the fixed JSON-RPC error `pier: the prompt could not be rendered`, so no database or provider error reaches the agent. 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.
|
||||
- One audit line per tool call (`pier: tool call`: tool, guard, user id, admin id, client id, token kind, session prefix, status, duration) and one per session step (`pier: session`: `open`, `rehydrate`, `end`, `binding_refused`, `revoked`, `stream_closed`, `expired`), never arguments, bodies or tokens. A tool call served on the stateful path without a session logs the session `ephemeral`, one on the stateless path `stateless`.
|
||||
|
||||
|
||||
@@ -2,7 +2,9 @@ package pier_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -61,3 +63,39 @@ func TestPromptWriteCeiling(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestPromptRenderError pins RA-WR-05: a prompt whose Render fails answers a
|
||||
// fixed JSON-RPC error on both paths; the cause (here a database error
|
||||
// naming its host and user) is logged server-side and never reaches the
|
||||
// client.
|
||||
func TestPromptRenderError(t *testing.T) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||
defer cancel()
|
||||
const cause = `failed to connect to host=db.internal user=summer database=app: dial tcp 10.0.0.5:5432`
|
||||
broken := summarizePrompt()
|
||||
broken.Name = "broken"
|
||||
broken.Render = func(context.Context, map[string]string) ([]pact.MCPPromptMessage, error) {
|
||||
return nil, errors.New(cause)
|
||||
}
|
||||
in := startInstance(t, newApp(t, appSpec{guards: []pact.MCPGuard{&acmeGuard{}}}),
|
||||
acmePlugin{tools: []pact.MCPTool{echoTool()}, prompts: []pact.MCPPrompt{broken}})
|
||||
t.Cleanup(in.pier().StopReaper)
|
||||
logs := captureLogs(t)
|
||||
|
||||
for _, version := range []string{"", "newest"} {
|
||||
cs := sdkClient(t, ctx, in.url(), "oas_at_test", clientOpts{version: version})
|
||||
_, err := cs.GetPrompt(ctx, &mcp.GetPromptParams{Name: "acme_broken", Arguments: map[string]string{"topic": "x"}})
|
||||
if err == nil {
|
||||
t.Fatalf("version %q: a failing Render answered no error", version)
|
||||
}
|
||||
if strings.Contains(err.Error(), "db.internal") || strings.Contains(err.Error(), "10.0.0.5") {
|
||||
t.Fatalf("version %q: the Render cause reached the client: %v", version, err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "pier: the prompt could not be rendered") {
|
||||
t.Fatalf("version %q: error = %v, want the fixed text", version, err)
|
||||
}
|
||||
}
|
||||
if !strings.Contains(logs.String(), "db.internal") {
|
||||
t.Fatalf("the Render cause was not logged: %s", logs.String())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -706,7 +706,13 @@ func (p *Pier) toolHandler(t *tool, session Identity) mcp.ToolHandler {
|
||||
}
|
||||
}
|
||||
|
||||
// promptHandler checks the required arguments and renders the prompt.
|
||||
// promptRenderFailed is the JSON-RPC error text of a prompt whose Render
|
||||
// failed; the cause is logged, never sent (RA-WR-05).
|
||||
const promptRenderFailed = "pier: the prompt could not be rendered"
|
||||
|
||||
// promptHandler checks the required arguments and renders the prompt. A
|
||||
// Render error is logged server-side and answered with a fixed text, so a
|
||||
// database or provider error never reaches the agent (as for dispatch).
|
||||
func (p *Pier) promptHandler(pr *prompt) mcp.PromptHandler {
|
||||
return func(ctx context.Context, req *mcp.GetPromptRequest) (*mcp.GetPromptResult, error) {
|
||||
args := map[string]string{}
|
||||
@@ -722,7 +728,8 @@ func (p *Pier) promptHandler(pr *prompt) mcp.PromptHandler {
|
||||
}
|
||||
messages, err := pr.decl.Render(ctx, args)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
logger().Warn("pier: prompt render failed", "prompt", pr.name, "error", err)
|
||||
return nil, &jsonrpc.Error{Code: jsonrpc.CodeInternalError, Message: promptRenderFailed}
|
||||
}
|
||||
res := &mcp.GetPromptResult{Description: pr.decl.Description, Messages: []*mcp.PromptMessage{}}
|
||||
for _, m := range messages {
|
||||
|
||||
Reference in New Issue
Block a user