From 21b01765468fae1f0c5a5d0895cb8dbc477884b3 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 20:39:42 +0000 Subject: [PATCH 01/12] Log a caller-visible request ID next to the trace IDs Add an opt-in request ID context value that ContextHandler emits as request_id beside trace_id and span_id. The shared HTTP middleware is unchanged; a service attaches the ID in its own middleware. --- internal/obs/log/context.go | 25 +++++++++++++++++++++++++ internal/obs/log/handler.go | 17 +++++++++++------ internal/obs/log/handler_test.go | 22 ++++++++++++++++++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/internal/obs/log/context.go b/internal/obs/log/context.go index c382d98d4..9b2c4fcdd 100644 --- a/internal/obs/log/context.go +++ b/internal/obs/log/context.go @@ -31,6 +31,31 @@ func TraceFromContext(ctx context.Context) (Carrier, bool) { return c, true } +// requestIDCtxKey keys a caller-visible request ID, kept next to the trace +// Carrier on the request context. +type requestIDCtxKey struct{} + +// WithRequestID returns ctx annotated with a caller-visible request ID, which +// ContextHandler emits as request_id next to trace_id. An empty ID is a no-op. +// HTTPMiddleware never sets one; a service that returns request IDs to its +// callers attaches them in its own middleware. +func WithRequestID(ctx context.Context, id string) context.Context { + if id == "" { + return ctx + } + return context.WithValue(ctx, requestIDCtxKey{}, id) +} + +// RequestIDFromContext returns the ID attached by WithRequestID, or +// ("", false) when none is present. +func RequestIDFromContext(ctx context.Context) (string, bool) { + if ctx == nil { + return "", false + } + id, ok := ctx.Value(requestIDCtxKey{}).(string) + return id, ok && id != "" +} + // Ctx returns a ctxLogger so `log.Ctx(ctx).Info(...)` reads cleaner // than `slog.Default().InfoContext(ctx, ...)`. ctxLogger is stateless // w.r.t. trace IDs — every call re-reads ctx so a child span minted diff --git a/internal/obs/log/handler.go b/internal/obs/log/handler.go index b41247f1e..e33dc1574 100644 --- a/internal/obs/log/handler.go +++ b/internal/obs/log/handler.go @@ -5,16 +5,18 @@ import ( "log/slog" ) -// AttrTraceID / AttrSpanID are slog attr keys for the W3C IDs. Stable -// snake_case so log scrapers can build dashboards on a fixed name. +// AttrTraceID / AttrSpanID are slog attr keys for the W3C IDs, and +// AttrRequestID for a caller-visible request ID. Stable snake_case so log +// scrapers can build dashboards on a fixed name. const ( - AttrTraceID = "trace_id" - AttrSpanID = "span_id" + AttrTraceID = "trace_id" + AttrSpanID = "span_id" + AttrRequestID = "request_id" ) // ContextHandler wraps a slog.Handler and injects trace_id/span_id -// from the record's ctx. Records with no Carrier pass through -// unchanged so background tasks don't get fake all-zero IDs. +// and any request_id from the record's ctx. Records with no Carrier pass +// through unchanged so background tasks don't get fake all-zero IDs. type ContextHandler struct { inner slog.Handler } @@ -37,6 +39,9 @@ func (h *ContextHandler) Handle(ctx context.Context, r slog.Record) error { slog.String(AttrSpanID, carrier.Span.String()), ) } + if id, ok := RequestIDFromContext(ctx); ok { + r.AddAttrs(slog.String(AttrRequestID, id)) + } return h.inner.Handle(ctx, r) } diff --git a/internal/obs/log/handler_test.go b/internal/obs/log/handler_test.go index ae9e11a18..fd15b872f 100644 --- a/internal/obs/log/handler_test.go +++ b/internal/obs/log/handler_test.go @@ -62,3 +62,25 @@ func TestContextHandlerWithAttrsPreservesInjection(t *testing.T) { t.Fatalf("static attr missing: %s", out) } } + +// A caller-visible request ID is logged next to the trace IDs; records +// without one carry no request_id attr. +func TestContextHandlerInjectsRequestID(t *testing.T) { + carrier, _ := ParseTraceparent("00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01") + var buf bytes.Buffer + logger := slog.New(NewContextHandler(slog.NewJSONHandler(&buf, nil))) + ctx := WithRequestID(WithTrace(context.Background(), carrier), "req_0123456789abcdef0123456789abcdef") + logger.InfoContext(ctx, "hello") + var got map[string]any + if err := json.Unmarshal(bytes.TrimSpace(buf.Bytes()), &got); err != nil { + t.Fatalf("unmarshal: %v\nraw=%s", err, buf.String()) + } + if got[AttrRequestID] != "req_0123456789abcdef0123456789abcdef" || got[AttrTraceID] != "0af7651916cd43dd8448eb211c80319c" { + t.Fatalf("request/trace IDs: %v", got) + } + buf.Reset() + logger.InfoContext(WithRequestID(context.Background(), ""), "no request") + if strings.Contains(buf.String(), AttrRequestID) { + t.Fatalf("request_id should be absent; got %s", buf.String()) + } +} From 687b3b56c2937aaf7665241a696a9aca5eff7ce6 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 20:39:48 +0000 Subject: [PATCH 02/12] Align Agents API routing, Beta order and response headers Serve every request on its canonical path instead of the ServeMux 301, which made the pinned SDK resend an update as a GET (HP-17). The canonicalizing handler wraps the complete server handler in both configurations, decodes percent-encoded unreserved characters (HP-18) and resolves empty and dot segments before any routing or authentication decision; other escapes stay encoded. HEAD runs GET routes without a body (HP-19); the events stream and content downloads answer 405. A 405 lists the route's methods in Allow (HP-20). Every response carries a fresh X-Request-Id, OpenAI-Version, OpenAI-Processing-Ms and nosniff (HP-23/24). On the Beta group the constant OpenAI-Beta check, now requiring exactly one agents=v1 value with the observed message, runs before authentication (HP-02/03/05). Every 401 has type invalid_request_error; Beta routes report a null code and Files, Skills and project extensions invalid_api_key only for a rejected Bearer credential (HP-07). --- services/agents-api/cmd/server/http_routes.go | 33 + .../agents-api/cmd/server/http_routes_test.go | 53 ++ services/agents-api/cmd/server/main.go | 13 +- services/agents-api/internal/api/auth.go | 34 +- services/agents-api/internal/api/auth_test.go | 4 +- services/agents-api/internal/api/errors.go | 4 +- .../agents-api/internal/api/errors_test.go | 60 +- services/agents-api/internal/api/handler.go | 20 +- .../agents-api/internal/api/handler_test.go | 7 +- services/agents-api/internal/api/routing.go | 187 ++++++ .../agents-api/internal/api/routing_test.go | 562 ++++++++++++++++++ services/agents-api/internal/api/skills.go | 2 + .../agents-api/internal/api/stream_test.go | 4 + 13 files changed, 939 insertions(+), 44 deletions(-) create mode 100644 services/agents-api/cmd/server/http_routes.go create mode 100644 services/agents-api/cmd/server/http_routes_test.go create mode 100644 services/agents-api/internal/api/routing.go create mode 100644 services/agents-api/internal/api/routing_test.go diff --git a/services/agents-api/cmd/server/http_routes.go b/services/agents-api/cmd/server/http_routes.go new file mode 100644 index 000000000..dc65ee35a --- /dev/null +++ b/services/agents-api/cmd/server/http_routes.go @@ -0,0 +1,33 @@ +package main + +import ( + "net/http" + + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/api" +) + +// daemonRoutes are the Runtime transport handlers served beside the API. Each +// authenticates its own callers. +type daemonRoutes struct { + gateway, enrollment, connection, nodeConnect http.Handler +} + +// serverHandler composes the daemon transport routes with the API handler. +// Path canonicalization wraps the whole composition, so the ServeMux, the API +// router and every middleware decide on the same canonical path, and the +// ServeMux never answers a redirect. The API handler canonicalizes again when +// served alone; the operation is idempotent. +func serverHandler(apiHandler http.Handler, daemon *daemonRoutes) http.Handler { + if daemon == nil { + return apiHandler + } + mux := http.NewServeMux() + mux.Handle("/api/v1/agent-daemon/", daemon.gateway) + mux.Handle("/api/v1/agent-daemon/enroll", daemon.enrollment) + mux.Handle("/api/v1/agent-daemon/connection", daemon.connection) + if daemon.nodeConnect != nil { + mux.Handle("/core/v1/sandbox/node/connect", daemon.nodeConnect) + } + mux.Handle("/", apiHandler) + return api.CanonicalPaths(mux) +} diff --git a/services/agents-api/cmd/server/http_routes_test.go b/services/agents-api/cmd/server/http_routes_test.go new file mode 100644 index 000000000..e1cad8288 --- /dev/null +++ b/services/agents-api/cmd/server/http_routes_test.go @@ -0,0 +1,53 @@ +package main + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// The daemon-enabled configuration canonicalizes paths before the ServeMux: +// no request is redirected, and a dirty or encoded path reaches exactly the +// handler its canonical path reaches, with the body intact (HP-17/HP-18). +func TestServerHandlerRoutesCanonicalPaths(t *testing.T) { + type observation struct{ route, method, path, rawPath, body string } + var seen []observation + sentinel := func(route string) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + seen = append(seen, observation{route, r.Method, r.URL.Path, r.URL.RawPath, string(body)}) + w.WriteHeader(http.StatusNoContent) + }) + } + handler := serverHandler(sentinel("api"), &daemonRoutes{gateway: sentinel("gateway"), enrollment: sentinel("enrollment"), + connection: sentinel("connection"), nodeConnect: sentinel("node")}) + for _, test := range []struct{ target, route, path, rawPath string }{ + {"/v1/agents/a", "api", "/v1/agents/a", ""}, + {"/v1//agents/a", "api", "/v1/agents/a", ""}, + {"//v1/agents", "api", "/v1/agents", ""}, + {"/v1/agents/x/../a", "api", "/v1/agents/a", ""}, + {"/v1/agents/agent%5Fa", "api", "/v1/agents/agent_a", ""}, + {"/v1/agents/a%2Fb", "api", "/v1/agents/a/b", "/v1/agents/a%2Fb"}, + {"/api/v1/agent-daemon/%2E%2E/%2E%2E/%2E%2E/v1/agents", "api", "/v1/agents", ""}, + {"/api/v1/agent-daemon/../../../core/v1/sandbox/nodes", "api", "/core/v1/sandbox/nodes", ""}, + {"/api/v1/agent-daemon%2Fenroll", "api", "/api/v1/agent-daemon/enroll", "/api/v1/agent-daemon%2Fenroll"}, + {"/api/v1/agent-daemon/ws", "gateway", "/api/v1/agent-daemon/ws", ""}, + {"/api/v1//agent-daemon/ws", "gateway", "/api/v1/agent-daemon/ws", ""}, + {"/v1/../api/v1/agent-daemon/enroll", "enrollment", "/api/v1/agent-daemon/enroll", ""}, + {"/v1/%2E%2E/api/v1/agent-daemon/connection", "connection", "/api/v1/agent-daemon/connection", ""}, + {"/api/v1/agent-daemon/%63onnection", "connection", "/api/v1/agent-daemon/connection", ""}, + {"/core/v1/sandbox/node//connect", "node", "/core/v1/sandbox/node/connect", ""}, + {"/core/v1/sandbox/nodes/%2E%2E/node/connect", "node", "/core/v1/sandbox/node/connect", ""}, + } { + seen = nil + request := httptest.NewRequest(http.MethodPost, test.target, strings.NewReader(`{"model":"x"}`)) + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + want := observation{test.route, http.MethodPost, test.path, test.rawPath, `{"model":"x"}`} + if response.Code != http.StatusNoContent || response.Header().Get("Location") != "" || len(seen) != 1 || seen[0] != want { + t.Errorf("%s = %d %v, want %v", test.target, response.Code, seen, want) + } + } +} diff --git a/services/agents-api/cmd/server/main.go b/services/agents-api/cmd/server/main.go index cd6fda27b..e9b31b9be 100644 --- a/services/agents-api/cmd/server/main.go +++ b/services/agents-api/cmd/server/main.go @@ -258,16 +258,13 @@ func run() error { return err } if daemonHandler != nil { - mux := http.NewServeMux() - mux.Handle("/api/v1/agent-daemon/", daemonHandler) - mux.Handle("/api/v1/agent-daemon/enroll", runtimeenrollment.EnrollmentHandler(executionStore)) - mux.Handle("/api/v1/agent-daemon/connection", runtimeenrollment.ConnectionHandler(executionStore, registry)) - + routes := daemonRoutes{gateway: daemonHandler, + enrollment: runtimeenrollment.EnrollmentHandler(executionStore), + connection: runtimeenrollment.ConnectionHandler(executionStore, registry)} if managedNodes != nil { - mux.Handle("/core/v1/sandbox/node/connect", managedNodes.hub) + routes.nodeConnect = managedNodes.hub } - mux.Handle("/", handler) - handler = mux + handler = serverHandler(handler, &routes) } addr := serverAddress() server := &http.Server{Addr: addr, Handler: handler, ReadHeaderTimeout: 10 * time.Second, ReadTimeout: 30 * time.Second, WriteTimeout: 30 * time.Second, IdleTimeout: 60 * time.Second} diff --git a/services/agents-api/internal/api/auth.go b/services/agents-api/internal/api/auth.go index da533fffb..b00b0456b 100644 --- a/services/agents-api/internal/api/auth.go +++ b/services/agents-api/internal/api/auth.go @@ -117,17 +117,33 @@ func matchesScopeHeader(r *http.Request, name, expected string) bool { type principalContextKey struct{} +const invalidBetaMessage = "To access the Agents API, set the 'OpenAI-Beta' header to 'agents=v1'." + +// authenticate guards the Beta group. As observed officially (HP-05), the +// constant OpenAI-Beta check runs first: it reads only that header, and its +// 400 carries no tenant or resource data. Every request that passes it is +// authenticated before routing reaches any handler, including the group's 404 +// and 405 responses. The header must have exactly one field value (HP-03). +// Every Beta 401 has a null code (HP-07). func (h *Handler) authenticate(next http.Handler) http.Handler { - return h.authenticateProject(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Header.Get("OpenAI-Beta") != "agents=v1" { - writeError(w, http.StatusBadRequest, "invalid_beta", "OpenAI-Beta: agents=v1 is required.") + authenticated := h.authenticateCaller(next, false) + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if values := r.Header.Values("OpenAI-Beta"); len(values) != 1 || values[0] != "agents=v1" { + writeError(w, http.StatusBadRequest, "invalid_beta", invalidBetaMessage) return } - next.ServeHTTP(w, r) - })) + authenticated.ServeHTTP(w, r) + }) } +// authenticateProject guards Files, Skills and Core project extensions, which +// ignore OpenAI-Beta. As observed on Files and Skills (HP-07), a 401 has a null +// code without a Bearer credential and invalid_api_key for a rejected one. func (h *Handler) authenticateProject(next http.Handler) http.Handler { + return h.authenticateCaller(next, true) +} + +func (h *Handler) authenticateCaller(next http.Handler, reportInvalidKey bool) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { principal, ok, err := h.resolvePrincipal(r) if err != nil { @@ -135,8 +151,14 @@ func (h *Handler) authenticateProject(next http.Handler) http.Handler { return } if !ok { + code := "" + // A Bearer credential was supplied: exactly one Authorization header + // in the Bearer scheme, rejected by key or scope headers. + if _, bearer := projectBearerDigest(r); reportInvalidKey && bearer { + code = "invalid_api_key" + } w.Header().Set("WWW-Authenticate", "Bearer") - writeError(w, http.StatusUnauthorized, "invalid_api_key", "A valid Agents API bearer key is required.") + writeError(w, http.StatusUnauthorized, code, "A valid Agents API bearer key is required.") return } next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), principalContextKey{}, principal))) diff --git a/services/agents-api/internal/api/auth_test.go b/services/agents-api/internal/api/auth_test.go index 7299f334c..816a68b6a 100644 --- a/services/agents-api/internal/api/auth_test.go +++ b/services/agents-api/internal/api/auth_test.go @@ -3,7 +3,6 @@ package api import ( "net/http" "net/http/httptest" - "strings" "testing" "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/device" @@ -115,7 +114,8 @@ func TestCallerPrincipalHeadersAndKeyRotation(t *testing.T) { if w.Code != test.status || called != (test.status == 200) { t.Fatalf("response = %d, called = %v", w.Code, called) } - if test.status == 401 && (!strings.Contains(w.Body.String(), "invalid_api_key") || w.Header().Get("WWW-Authenticate") != "Bearer") { + // Beta 401s have type invalid_request_error and a null code (HP-07). + if test.status == 401 && (w.Body.String() != `{"error":{"message":"A valid Agents API bearer key is required.","type":"invalid_request_error","code":null,"param":null}}`+"\n" || w.Header().Get("WWW-Authenticate") != "Bearer") { t.Fatalf("authentication response = %s", w.Body) } }) diff --git a/services/agents-api/internal/api/errors.go b/services/agents-api/internal/api/errors.go index f40e1ba5e..07386cce9 100644 --- a/services/agents-api/internal/api/errors.go +++ b/services/agents-api/internal/api/errors.go @@ -21,12 +21,12 @@ func writeJSON(w http.ResponseWriter, status int, value any) { // writeError reports every 409 with type conflict_error, as every observed // official conflict does (ERR-27); Core-only conflicts keep their own code. +// Every 401 has type invalid_request_error, as every observed official 401 +// does (HP-07); an empty code serializes as null. func writeError(w http.ResponseWriter, status int, code, message string, param ...string) { kind := "invalid_request_error" if status >= 500 { kind = "server_error" - } else if status == http.StatusUnauthorized { - kind = "authentication_error" } else if status == http.StatusConflict { kind = "conflict_error" } else if code == "not_found_error" || code == "invalid_beta" { diff --git a/services/agents-api/internal/api/errors_test.go b/services/agents-api/internal/api/errors_test.go index 4c91496e4..b768c375c 100644 --- a/services/agents-api/internal/api/errors_test.go +++ b/services/agents-api/internal/api/errors_test.go @@ -65,29 +65,49 @@ func TestInvalidCursorErrorFields(t *testing.T) { } } -func TestMissingBetaErrorAfterAuthentication(t *testing.T) { - for _, authenticated := range []bool{false, true} { - handler, _, _ := testHandler(t) - request := httptest.NewRequest(http.MethodGet, "/v1/agents/sessions", nil) - if authenticated { - request.Header.Set("Authorization", "Bearer test-api-key") - } - response := httptest.NewRecorder() - handler.ServeHTTP(response, request) - var body v1.ErrorResponse - if err := json.Unmarshal(response.Body.Bytes(), &body); err != nil { - t.Fatal(err) - } - status, kind, code := http.StatusUnauthorized, "authentication_error", "invalid_api_key" - if authenticated { - status, kind, code = http.StatusBadRequest, "invalid_beta", "invalid_beta" - } - if response.Code != status || body.Error.Type != kind || body.Error.Code == nil || *body.Error.Code != code { - t.Fatalf("authenticated=%v: %d %s", authenticated, response.Code, response.Body) - } +// The constant Beta check runs before authentication on the Beta group +// (HP-05): a missing Beta header is 400 invalid_beta with or without valid +// credentials, and only a request carrying it reaches the 401. +func TestMissingBetaErrorBeforeAuthentication(t *testing.T) { + for _, test := range []struct { + name, authorization, beta string + status int + kind string + code *string + }{ + {"no credentials", "", "", http.StatusBadRequest, "invalid_beta", ptr("invalid_beta")}, + {"invalid credentials", "Bearer wrong", "", http.StatusBadRequest, "invalid_beta", ptr("invalid_beta")}, + {"valid credentials", "Bearer test-api-key", "", http.StatusBadRequest, "invalid_beta", ptr("invalid_beta")}, + {"beta without credentials", "", "agents=v1", http.StatusUnauthorized, "invalid_request_error", nil}, + } { + t.Run(test.name, func(t *testing.T) { + handler, _, _ := testHandler(t) + request := httptest.NewRequest(http.MethodGet, "/v1/agents/sessions", nil) + if test.authorization != "" { + request.Header.Set("Authorization", test.authorization) + } + if test.beta != "" { + request.Header.Set("OpenAI-Beta", test.beta) + } + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + var body v1.ErrorResponse + if err := json.Unmarshal(response.Body.Bytes(), &body); err != nil { + t.Fatal(err) + } + if response.Code != test.status || body.Error.Type != test.kind || !equalOptional(body.Error.Code, test.code) || body.Error.Param != nil { + t.Fatalf("%d %s", response.Code, response.Body) + } + }) } } +func ptr(value string) *string { return &value } + +func equalOptional(got, want *string) bool { + return (got == nil) == (want == nil) && (got == nil || *got == *want) +} + // Session deletion conflicts use the observed official 409 fields. func TestSessionDeletionConflictError(t *testing.T) { response := httptest.NewRecorder() diff --git a/services/agents-api/internal/api/handler.go b/services/agents-api/internal/api/handler.go index 8e51383b0..dcc5e222c 100644 --- a/services/agents-api/internal/api/handler.go +++ b/services/agents-api/internal/api/handler.go @@ -15,6 +15,7 @@ import ( "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/identity" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" "github.com/go-chi/chi/v5" + "github.com/go-chi/chi/v5/middleware" "github.com/google/uuid" ) @@ -75,8 +76,16 @@ func NewHandler(s ResourceStore, auth *Authenticator, engine string, options ... } } } + return CanonicalPaths(h.routes()), nil +} + +// routes builds the router. HEAD runs the GET route without a body after the +// same authentication and Beta checks (HP-19). Routes that stream events or +// download content register an explicit HEAD 405 instead, so HEAD never holds +// a stream open or reads full content. +func (h *Handler) routes() *chi.Mux { router := chi.NewRouter() - router.Use(log.HTTPMiddleware) + router.Use(agentsResponseHeaders, log.HTTPMiddleware, middleware.GetHead) router.Get("/healthz", func(w http.ResponseWriter, _ *http.Request) { writeJSON(w, http.StatusOK, map[string]string{"status": "ok"}) }) @@ -87,6 +96,7 @@ func NewHandler(s ResourceStore, auth *Authenticator, engine string, options ... r.Get("/v1/files", h.listSourceFiles) r.Get("/v1/files/{file_id}", h.getSourceFile) r.Get("/v1/files/{file_id}/content", h.sourceFileContent) + r.Head("/v1/files/{file_id}/content", methodNotAllowed) r.Delete("/v1/files/{file_id}", h.deleteSourceFile) }) h.registerSandboxManagerRoutes(router) @@ -130,6 +140,7 @@ func NewHandler(s ResourceStore, auth *Authenticator, engine string, options ... r.Delete("/agents/sessions/{session_id}", h.deleteSession) r.Post("/agents/sessions/{session_id}/events", h.createEvents) r.Get("/agents/sessions/{session_id}/events", h.streamEvents) + r.Head("/agents/sessions/{session_id}/events", methodNotAllowed) r.Get("/agents/sessions/{session_id}/items", h.listItems) r.Get("/agents/sessions/{session_id}/turns", h.listTurns) r.Get("/agents/sessions/{session_id}/turns/{turn_id}", h.getTurn) @@ -137,15 +148,14 @@ func NewHandler(s ResourceStore, auth *Authenticator, engine string, options ... r.Get("/agents/sessions/{session_id}/artifacts", h.listSessionArtifacts) r.Get("/agents/sessions/{session_id}/artifacts/{artifact_id}", h.getSessionArtifact) r.Get("/agents/sessions/{session_id}/artifacts/{artifact_id}/content", h.sessionArtifactContent) + r.Head("/agents/sessions/{session_id}/artifacts/{artifact_id}/content", methodNotAllowed) r.Delete("/agents/sessions/{session_id}/artifacts/{artifact_id}", h.deleteSessionArtifact) r.NotFound(func(w http.ResponseWriter, _ *http.Request) { writeError(w, http.StatusNotFound, "unsupported_operation", "This API operation is not supported.") }) - r.MethodNotAllowed(func(w http.ResponseWriter, _ *http.Request) { - writeError(w, http.StatusMethodNotAllowed, "unsupported_operation", "This API method is not supported.") - }) + r.MethodNotAllowed(methodNotAllowed) }) - return router, nil + return router } // createSession atomically reserves or admits initial text with the Session. diff --git a/services/agents-api/internal/api/handler_test.go b/services/agents-api/internal/api/handler_test.go index 8588bca4a..595938520 100644 --- a/services/agents-api/internal/api/handler_test.go +++ b/services/agents-api/internal/api/handler_test.go @@ -146,7 +146,12 @@ func TestHTTPRejectsUntrustedOrUnsupportedRequests(t *testing.T) { w := httptest.NewRecorder() h.ServeHTTP(w, r) var response v1.ErrorResponse - if w.Code != test.status || json.Unmarshal(w.Body.Bytes(), &response) != nil || response.Error.Code == nil || *response.Error.Code == "" || s.tenant != "" { + if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil { + t.Fatalf("response = %d %s: %v", w.Code, w.Body, err) + } + // Beta 401s have a null code (HP-07); every other rejection names one. + coded := response.Error.Code != nil && *response.Error.Code != "" + if w.Code != test.status || coded == (test.status == http.StatusUnauthorized) || s.tenant != "" { t.Fatalf("response = %d %s, stored tenant = %s", w.Code, w.Body, s.tenant) } }) diff --git a/services/agents-api/internal/api/routing.go b/services/agents-api/internal/api/routing.go new file mode 100644 index 000000000..76286bb94 --- /dev/null +++ b/services/agents-api/internal/api/routing.go @@ -0,0 +1,187 @@ +package api + +import ( + "crypto/rand" + "encoding/hex" + "net/http" + "net/url" + "path" + "strconv" + "strings" + "time" + + "github.com/MiniMax-AI-Dev/parsar/internal/obs/log" + "github.com/go-chi/chi/v5" +) + +// CanonicalPaths serves each request on its canonical path, as the official +// service does (HP-17/HP-18), instead of the ServeMux redirect, which made +// clients resend a POST as a GET. It must wrap the complete server handler so +// that every routing, authentication and middleware decision sees only the +// rewritten path. Percent-encoded unreserved characters (RFC 3986 section 2.3) +// are decoded first, so an encoded dot segment is resolved like a literal one. +// Then empty and dot segments are resolved with ServeMux cleanPath semantics, +// keeping a trailing slash. Other escapes, such as %2F, stay encoded and never +// become separators, so every request reaches exactly the route and +// authentication of its canonical path written literally. It is idempotent. +func CanonicalPaths(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + escaped := cleanPath(decodeUnreserved(r.URL.EscapedPath())) + decoded, err := url.PathUnescape(escaped) + if err != nil { + // EscapedPath is always validly escaped; this is unreachable. + http.Error(w, "Invalid request path.", http.StatusBadRequest) + return + } + if decoded != r.URL.Path || escaped != r.URL.EscapedPath() { + canonical := *r.URL + canonical.Path, canonical.RawPath = decoded, "" + if canonical.EscapedPath() != escaped { + canonical.RawPath = escaped + } + r = r.WithContext(r.Context()) + r.URL = &canonical + } + next.ServeHTTP(w, r) + }) +} + +// decodeUnreserved decodes percent-encoded ALPHA, DIGIT, '-', '.', '_' and '~', +// which RFC 3986 treats as equivalent to their literal form. +func decodeUnreserved(escaped string) string { + if !strings.Contains(escaped, "%") { + return escaped + } + var decoded strings.Builder + decoded.Grow(len(escaped)) + for i := 0; i < len(escaped); i++ { + if escaped[i] == '%' && i+2 < len(escaped) { + if value, err := strconv.ParseUint(escaped[i+1:i+3], 16, 8); err == nil && unreserved(byte(value)) { + decoded.WriteByte(byte(value)) + i += 2 + continue + } + } + decoded.WriteByte(escaped[i]) + } + return decoded.String() +} + +func unreserved(c byte) bool { + return 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' || c == '-' || c == '.' || c == '_' || c == '~' +} + +// cleanPath matches net/http ServeMux path cleaning: collapse empty segments, +// resolve dot segments and keep a trailing slash. +func cleanPath(p string) string { + if p == "" { + return "/" + } + if p[0] != '/' { + p = "/" + p + } + cleaned := path.Clean(p) + if p[len(p)-1] == '/' && cleaned != "/" { + cleaned += "/" + } + return cleaned +} + +// agentsResponseHeaders adds the observed official response headers to every +// response of this handler, including errors, 401, 404, 405 and SSE streams +// (HP-23/HP-24): a fresh random X-Request-Id, attached to the request log +// context next to the trace carrier, OpenAI-Version, OpenAI-Processing-Ms at +// the time headers are written, and nosniff. Core's traceparent and +// Cache-Control extensions remain. Organization and project headers are not +// reported: Core's project scope is configured, not account-derived. +func agentsResponseHeaders(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + id := newRequestID() + header := w.Header() + header.Set("X-Request-Id", id) + header.Set("Openai-Version", "2020-10-01") + header.Set("X-Content-Type-Options", "nosniff") + writer := &processingTimeWriter{ResponseWriter: w, started: time.Now()} + next.ServeHTTP(writer, r.WithContext(log.WithRequestID(r.Context(), id))) + }) +} + +func newRequestID() string { + var id [16]byte + _, _ = rand.Read(id[:]) + return "req_" + hex.EncodeToString(id[:]) +} + +// processingTimeWriter reports the elapsed handling time when the final +// response headers are written, flushed or implied by the first body write. +// Unwrap keeps http.ResponseController deadlines and flushing available. +type processingTimeWriter struct { + http.ResponseWriter + started time.Time + stamped bool +} + +func (w *processingTimeWriter) stamp() { + if !w.stamped { + w.stamped = true + w.ResponseWriter.Header().Set("Openai-Processing-Ms", strconv.FormatInt(time.Since(w.started).Milliseconds(), 10)) + } +} + +func (w *processingTimeWriter) WriteHeader(status int) { + if status >= http.StatusOK { + w.stamp() + } + w.ResponseWriter.WriteHeader(status) +} + +func (w *processingTimeWriter) Write(body []byte) (int, error) { + w.stamp() + return w.ResponseWriter.Write(body) +} + +func (w *processingTimeWriter) FlushError() error { + w.stamp() + return http.NewResponseController(w.ResponseWriter).Flush() +} + +func (w *processingTimeWriter) Flush() { _ = w.FlushError() } + +func (w *processingTimeWriter) Unwrap() http.ResponseWriter { return w.ResponseWriter } + +// methodNotAllowed keeps Core's JSON 405 and adds the route's Allow header +// (HP-20). It also answers HEAD on routes that exclude it. +func methodNotAllowed(w http.ResponseWriter, r *http.Request) { + if allowed := allowedMethods(r); allowed != "" { + w.Header().Set("Allow", allowed) + } + writeError(w, http.StatusMethodNotAllowed, "unsupported_operation", "This API method is not supported.") +} + +// allowedMethods lists the methods routed for the request path in the official +// order (GET,HEAD,POST,DELETE). HEAD follows GET unless the route registers its +// own HEAD handler, which only ever excludes HEAD. +func allowedMethods(r *http.Request) string { + rctx := chi.RouteContext(r.Context()) + if rctx == nil || rctx.Routes == nil { + return "" + } + routePath := r.URL.RawPath + if routePath == "" { + routePath = r.URL.Path + } + routed := func(method string) bool { return rctx.Routes.Match(chi.NewRouteContext(), method, routePath) } + var allowed []string + if routed(http.MethodGet) { + allowed = append(allowed, http.MethodGet) + if !routed(http.MethodHead) { + allowed = append(allowed, http.MethodHead) + } + } + for _, method := range []string{http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete} { + if routed(method) { + allowed = append(allowed, method) + } + } + return strings.Join(allowed, ",") +} diff --git a/services/agents-api/internal/api/routing_test.go b/services/agents-api/internal/api/routing_test.go new file mode 100644 index 000000000..95d1fd104 --- /dev/null +++ b/services/agents-api/internal/api/routing_test.go @@ -0,0 +1,562 @@ +package api + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "regexp" + "strconv" + "strings" + "testing" + "time" + + "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/device" + "github.com/MiniMax-AI-Dev/parsar/internal/obs/log" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" + "github.com/go-chi/chi/v5" + "github.com/google/uuid" +) + +const ( + routingKey = "routing-project-key" + routingAdminKey = "routing-admin-key" + unauthorizedV1 = `{"error":{"message":"A valid Agents API bearer key is required.","type":"invalid_request_error","code":null,"param":null}}` + "\n" + invalidBetaV1 = `{"error":{"message":"To access the Agents API, set the 'OpenAI-Beta' header to 'agents=v1'.","type":"invalid_beta","code":"invalid_beta","param":null}}` + "\n" + notAllowedV1 = `{"error":{"message":"This API method is not supported.","type":"invalid_request_error","code":"unsupported_operation","param":null}}` + "\n" +) + +var requestIDPattern = regexp.MustCompile(`^req_[0-9a-f]{32}$`) + +// routingStore serves one saved Agent and records Agent lookups. Every other +// store method, including Environment executor credentials, panics through +// the nil embedded interfaces, so a test fails if a handler reaches it. +type routingStore struct { + ResourceStore + EnvironmentExecutorStore + tenant string + agent store.SavedAgent + lookups, updates []string +} + +func (s *routingStore) GetAgent(_ context.Context, tenant, id string) (store.SavedAgent, error) { + s.lookups = append(s.lookups, id) + if tenant != s.tenant || id != s.agent.ID { + return store.SavedAgent{}, store.ErrNotFound + } + return s.agent, nil +} + +func (s *routingStore) ListAgents(_ context.Context, tenant, _ string, _ int, _ bool) (store.AgentPage, error) { + if tenant != s.tenant { + return store.AgentPage{}, nil + } + return store.AgentPage{Agents: []store.SavedAgent{s.agent}}, nil +} + +func (s *routingStore) UpdateAgent(_ context.Context, tenant, id string, input store.UpdateAgentInput) (store.SavedAgent, error) { + s.updates = append(s.updates, id) + if tenant != s.tenant || id != s.agent.ID { + return store.SavedAgent{}, store.ErrNotFound + } + if input.Metadata != nil { + s.agent.Metadata = *input.Metadata + } + return s.agent, nil +} + +// missingFiles reports every File as missing. +type missingFiles struct{ SourceFileStore } + +func (missingFiles) GetSourceFile(context.Context, string, string) (store.SourceFile, error) { + return store.SourceFile{}, store.ErrNotFound +} + +// routingFixture returns the served handler and, for route enumeration, a +// router built from an identically configured Handler. Sandbox administration +// uses a zero Store, which panics if a handler is ever reached. +func routingFixture(t *testing.T) (http.Handler, *chi.Mux, *routingStore) { + t.Helper() + tenant := uuid.NewString() + auth, err := NewAuthenticator([]APIKey{{OrganizationID: "test-org", ProjectID: "test-project", SubjectKind: "service_account", SubjectID: "test-runner", TokenSHA256: device.HashCredential(routingKey), TenantID: tenant}}) + if err != nil { + t.Fatal(err) + } + admin, err := NewDeploymentAuthenticator([]string{device.HashCredential(routingAdminKey)}) + if err != nil { + t.Fatal(err) + } + s := &routingStore{tenant: tenant, agent: store.SavedAgent{ID: uuid.NewString(), TenantID: tenant, Metadata: map[string]string{}, + Configuration: json.RawMessage(`{"model":"fixture"}`), CreatedAt: time.Unix(1700000000, 0), UpdatedAt: time.Unix(1700000000, 0)}} + options := []Option{WithSandboxManager(&store.Store{}, admin), WithSourceFiles(missingFiles{})} + handler, err := NewHandler(s, auth, "codex", options...) + if err != nil { + t.Fatal(err) + } + h := &Handler{store: s, auth: auth, engine: "codex"} + for _, option := range options { + option(h) + } + return handler, h.routes(), s +} + +func routingHeaders(pairs ...string) http.Header { + header := http.Header{} + for i := 0; i < len(pairs); i += 2 { + header.Add(pairs[i], pairs[i+1]) + } + return header +} + +// project and beta are the pinned SDK's headers. +var ( + project = []string{"Authorization", "Bearer " + routingKey} + beta = []string{"OpenAI-Beta", "agents=v1"} +) + +func withHeaders(parts ...[]string) http.Header { + var pairs []string + for _, part := range parts { + pairs = append(pairs, part...) + } + return routingHeaders(pairs...) +} + +func serve(handler http.Handler, method, target, body string, header http.Header) *httptest.ResponseRecorder { + request := httptest.NewRequest(method, target, strings.NewReader(body)) + for name, values := range header { + request.Header[name] = values + } + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + return response +} + +func sameResponse(a, b *httptest.ResponseRecorder) bool { + return a.Code == b.Code && a.Body.String() == b.Body.String() && + a.Header().Get("Allow") == b.Header().Get("Allow") && a.Header().Get("WWW-Authenticate") == b.Header().Get("WWW-Authenticate") && + a.Header().Get("Content-Type") == b.Header().Get("Content-Type") +} + +// The canonical path decodes only unreserved escapes, then resolves empty and +// dot segments with ServeMux semantics, keeping a trailing slash (HP-17/HP-18). +func TestCanonicalPathsRewriteBeforeRouting(t *testing.T) { + for _, test := range []struct{ target, path, rawPath, query string }{ + {"/v1//agents/a", "/v1/agents/a", "", ""}, + {"//v1/agents?limit=1", "/v1/agents", "", "limit=1"}, + {"/v1/agents/x/../y", "/v1/agents/y", "", ""}, + {"/v1/./agents/.", "/v1/agents", "", ""}, + {"/v1/agents/", "/v1/agents/", "", ""}, + {"/v1/agents/x/../", "/v1/agents/", "", ""}, + {"/v1/agents/agent%5Fid%2d%7E", "/v1/agents/agent_id-~", "", ""}, + {"/v1/agents/%2E%2E/%2e%2E/core", "/core", "", ""}, + {"/v1/agents/.%2E/x", "/v1/x", "", ""}, + {"/v1/agents/a%2Fb", "/v1/agents/a/b", "/v1/agents/a%2Fb", ""}, + {"/v1/agents/..%2F..%2Fcore", "/v1/agents/../../core", "/v1/agents/..%2F..%2Fcore", ""}, + {"/v1/agents/a%20b", "/v1/agents/a b", "", ""}, + {"/v1/agents/%25", "/v1/agents/%", "", ""}, + } { + var seen *http.Request + handler := CanonicalPaths(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { seen = r })) + request := httptest.NewRequest(http.MethodPost, test.target, nil) + handler.ServeHTTP(httptest.NewRecorder(), request) + if seen.URL.Path != test.path || seen.URL.RawPath != test.rawPath || seen.URL.RawQuery != test.query || seen.Method != http.MethodPost { + t.Errorf("%s: path %q raw %q query %q", test.target, seen.URL.Path, seen.URL.RawPath, seen.URL.RawQuery) + } + again := httptest.NewRequest(http.MethodGet, seen.URL.RequestURI(), nil) + var repeated *http.Request + CanonicalPaths(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { repeated = r })).ServeHTTP(httptest.NewRecorder(), again) + if repeated.URL.EscapedPath() != seen.URL.EscapedPath() { + t.Errorf("%s: canonicalization is not idempotent: %q", test.target, repeated.URL.EscapedPath()) + } + } +} + +// Non-canonical and encoded paths are served, never redirected, and reach the +// same resource as the clean path: an update through // applies (HP-17), and a +// percent-encoded unreserved ID resolves (HP-18). Malformed IDs keep the 404. +func TestNonCanonicalPathsServeTheCleanRoute(t *testing.T) { + handler, _, s := routingFixture(t) + id := s.agent.ID + authenticated := withHeaders(project, beta) + clean := serve(handler, http.MethodGet, "/v1/agents/"+id, "", authenticated) + if clean.Code != http.StatusOK { + t.Fatalf("clean retrieve = %d %s", clean.Code, clean.Body) + } + for _, target := range []string{"/v1//agents/" + id, "/v1/agents//" + id, "//v1/agents/" + id, "/v1/agents/x/../" + id, "/v1/./agents/" + id, + "/v1/agents/" + strings.ReplaceAll(id, "-", "%2D"), "/v1/agents/" + strings.Replace(id, "-", "%2d", 1), "/%76%31/agents/" + id, + "/v1/agents/%2E%2E/agents/" + id} { + if got := serve(handler, http.MethodGet, target, "", authenticated); !sameResponse(got, clean) || got.Header().Get("Location") != "" { + t.Errorf("%s = %d %s", target, got.Code, got.Body) + } + } + for _, lookup := range s.lookups { + if lookup != id { + t.Fatalf("handler saw a non-canonical ID %q", lookup) + } + } + list := serve(handler, http.MethodGet, "/v1/agents", "", authenticated) + if got := serve(handler, http.MethodGet, "/v1//agents", "", authenticated); list.Code != http.StatusOK || !sameResponse(got, list) { + t.Fatalf("list through // = %d %s", got.Code, got.Body) + } + updated := serve(handler, http.MethodPost, "/v1//agents/"+id, `{"metadata":{"route":"double-slash"}}`, authenticated) + if updated.Code != http.StatusOK || s.agent.Metadata["route"] != "double-slash" || len(s.updates) != 1 || s.updates[0] != id { + t.Fatalf("update through // = %d %s, updates %v", updated.Code, updated.Body, s.updates) + } + created := serve(handler, http.MethodPost, "/v1//agents", `{}`, authenticated) + if want := serve(handler, http.MethodPost, "/v1/agents", `{}`, authenticated); created.Code != http.StatusBadRequest || !sameResponse(created, want) { + t.Fatalf("create through // = %d %s", created.Code, created.Body) + } + missing := serve(handler, http.MethodGet, "/v1/agents/"+uuid.NewString(), "", authenticated) + if missing.Code != http.StatusNotFound || !strings.Contains(missing.Body.String(), `"code":"not_found_error"`) { + t.Fatalf("missing = %d %s", missing.Code, missing.Body) + } + for _, target := range []string{"/v1/agents/x/../" + uuid.NewString(), "/v1/agents/" + strings.Replace(id, "-", "%2F", 1), "/v1/agents/%2D", "/v1/agents/" + id[:8] + "%00"} { + if got := serve(handler, http.MethodGet, target, "", authenticated); !sameResponse(got, missing) { + t.Errorf("%s = %d %s", target, got.Code, got.Body) + } + } + // Trailing slashes and unknown sub-routes keep the Beta group's 404 (R11). + unknown := serve(handler, http.MethodGet, "/v1/agents/"+id+"/unknown", "", authenticated) + for _, target := range []string{"/v1/agents/", "/v1/agents/" + id + "/", "/v1/agents/x/../", "/v1/agents/" + id + "/unknown", "/v1/agents//" + id + "/unknown"} { + got := serve(handler, http.MethodGet, target, "", authenticated) + if got.Code != http.StatusNotFound || !sameResponse(got, unknown) || !strings.Contains(got.Body.String(), `"code":"unsupported_operation"`) { + t.Errorf("%s = %d %s", target, got.Code, got.Body) + } + } + if got := serve(handler, http.MethodPost, "/v1/agents/", `{"model":"fixture"}`, authenticated); got.Code != http.StatusNotFound { + t.Fatalf("trailing-slash create = %d %s", got.Code, got.Body) + } +} + +func concretePath(pattern string) string { + parts := strings.Split(pattern, "/") + for i, part := range parts { + if strings.HasPrefix(part, "{") || part == "*" { + parts[i] = uuid.NewString() + } + } + return strings.Join(parts, "/") +} + +// dirtyVariants spells a clean path with empty, dot and encoded segments, +// including traversal from other route groups. +func dirtyVariants(clean string) []string { + first, rest, _ := strings.Cut(strings.TrimPrefix(clean, "/"), "/") + if rest != "" { + rest = "/" + rest + } + return []string{ + "/" + clean, + "/" + first + "//" + strings.TrimPrefix(rest, "/"), + "/" + first + "/." + rest, + "/" + first + "/zz/.." + rest, + "/" + first + "/zz/%2E%2E" + rest, + "/" + first + "/%2e" + rest, + "/%" + strings.ToUpper(strconv.FormatInt(int64(first[0]), 16)) + first[1:] + rest, + "/api/v1/agent-daemon/%2E%2E/%2E%2E/%2E%2E" + clean, + "/core/v1/sandbox/../../.." + clean, + "/v1/agents/%2E%2E/%2e%2e" + clean, + } +} + +// Security regression for path canonicalization (R1/R2) and Beta ordering +// (R6): every route except the explicitly self-authenticated ones rejects an +// unauthenticated request before any handler, and a dirty or encoded spelling +// of its path gives exactly the response of the clean path for every +// credential, so it can never reach another route group or skip its checks. +func TestEveryRouteAuthenticatesItsCanonicalPath(t *testing.T) { + handler, router, s := routingFixture(t) + selfAuthenticated := map[string]bool{"GET /healthz": false, "POST /core/v1/sandbox/enroll": false, "GET /core/v1/sandbox/node/identity": false} + credentials := []http.Header{{}, withHeaders(beta), withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta), + withHeaders([]string{"Authorization", "Basic " + routingKey}, beta), withHeaders([]string{"Authorization", "Bearer wrong"}), + withHeaders(project), withHeaders(project, []string{"OpenAI-Beta", "agents=v0"})} + routes := 0 + err := chi.Walk(router, func(method, route string, _ http.Handler, _ ...func(http.Handler) http.Handler) error { + if _, ok := selfAuthenticated[method+" "+route]; ok { + selfAuthenticated[method+" "+route] = true + return nil + } + routes++ + clean := concretePath(route) + admin := strings.HasPrefix(route, "/core/v1/sandbox/") + betaGroup := strings.HasPrefix(route, "/v1/") && !strings.HasPrefix(route, "/v1/files") && !strings.HasPrefix(route, "/v1/skills") + for _, header := range credentials { + projectKey := header.Get("Authorization") == "Bearer "+routingKey + betaHeader := header.Get("OpenAI-Beta") == "agents=v1" + adminKey := header.Get("Authorization") == "Bearer "+routingAdminKey + if admin && adminKey || !admin && projectKey && (betaHeader || !betaGroup) { + continue // Authenticated requests reach handlers. + } + want := serve(handler, method, clean, "", header) + if betaGroup && !betaHeader { + if want.Code != http.StatusBadRequest || want.Body.String() != invalidBetaV1 { + t.Errorf("%s %s without Beta = %d %s", method, clean, want.Code, want.Body) + } + } else if want.Code != http.StatusUnauthorized || want.Header().Get("WWW-Authenticate") != "Bearer" { + t.Errorf("%s %s unauthenticated = %d %s", method, clean, want.Code, want.Body) + } + for _, variant := range dirtyVariants(clean) { + if got := serve(handler, method, variant, "", header); !sameResponse(got, want) { + t.Errorf("%s %s = %d %s; clean path gives %d %s", method, variant, got.Code, got.Body, want.Code, want.Body) + } + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } + for route, found := range selfAuthenticated { + if !found { + t.Errorf("self-authenticated route %s is no longer registered", route) + } + } + if routes < 70 || len(s.lookups) != 0 || len(s.updates) != 0 { + t.Fatalf("walked %d routes; store lookups %v updates %v", routes, s.lookups, s.updates) + } + // Traversal into internal groups keeps their own authentication. + for _, test := range []struct { + target string + header http.Header + status int + code string + }{ + {"/v1/%2E%2E/core/v1/sandbox/nodes", withHeaders(project, beta), http.StatusUnauthorized, "invalid_admin_key"}, + {"/v1/agents/%2e%2e/%2e%2e/core/v1/sandbox/deployment", withHeaders(project, beta), http.StatusUnauthorized, "invalid_admin_key"}, + {"/v1/agents/..%2F..%2Fcore/v1/sandbox/nodes", withHeaders(project, beta), http.StatusNotFound, "unsupported_operation"}, + {"/core/v1/sandbox/%2E%2E/%2E%2E/%2E%2E/v1/agents", withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta), http.StatusUnauthorized, ""}, + {"/core/v1/sandbox/nodes%2F..%2F..%2F..%2Fv1%2Fagents", withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta), http.StatusNotFound, ""}, + } { + got := serve(handler, http.MethodGet, test.target, "", test.header) + if got.Code != test.status || (test.code != "" && !strings.Contains(got.Body.String(), `"code":"`+test.code+`"`)) { + t.Errorf("%s = %d %s", test.target, got.Code, got.Body) + } + } +} + +// Beta is checked first, and only one exact field value is accepted +// (HP-02/03/05, R11 agents=v0). Files and Skills ignore the header. +func TestOpenAIBetaHeaderPrecedesAuthentication(t *testing.T) { + handler, _, s := routingFixture(t) + for _, test := range []struct { + name string + header http.Header + status int + }{ + {"exact", withHeaders(project, beta), http.StatusOK}, + {"missing without credentials", http.Header{}, http.StatusBadRequest}, + {"missing with invalid bearer", routingHeaders("Authorization", "Bearer wrong"), http.StatusBadRequest}, + {"missing with valid bearer", withHeaders(project), http.StatusBadRequest}, + {"two lines", withHeaders(project, beta, []string{"OpenAI-Beta", "assistants=v2"}), http.StatusBadRequest}, + {"two lines without credentials", withHeaders(beta, []string{"OpenAI-Beta", "assistants=v2"}), http.StatusBadRequest}, + {"repeated exact line", withHeaders(project, beta, beta), http.StatusBadRequest}, + {"combined", withHeaders(project, []string{"OpenAI-Beta", "agents=v1, assistants=v2"}), http.StatusBadRequest}, + {"agents=v0", withHeaders(project, []string{"OpenAI-Beta", "agents=v0"}), http.StatusBadRequest}, + {"case", withHeaders(project, []string{"OpenAI-Beta", "Agents=v1"}), http.StatusBadRequest}, + {"empty", withHeaders(project, []string{"OpenAI-Beta", ""}), http.StatusBadRequest}, + {"beta without credentials", withHeaders(beta), http.StatusUnauthorized}, + } { + got := serve(handler, http.MethodGet, "/v1/agents/"+s.agent.ID, "", test.header) + if got.Code != test.status || (test.status == http.StatusBadRequest && got.Body.String() != invalidBetaV1) { + t.Errorf("%s = %d %s", test.name, got.Code, got.Body) + } + } + for _, header := range []http.Header{withHeaders(project), withHeaders(project, []string{"OpenAI-Beta", "foo=bar"})} { + if got := serve(handler, http.MethodGet, "/v1/files/file-missing", "", header); got.Code != http.StatusNotFound { + t.Errorf("Files with Beta %q = %d %s", header.Get("OpenAI-Beta"), got.Code, got.Body) + } + } + if got := serve(handler, http.MethodGet, "/v1/files/file-missing", "", http.Header{}); got.Code != http.StatusUnauthorized { + t.Errorf("Files without credentials = %d %s", got.Code, got.Body) + } +} + +// Every 401 is invalid_request_error (HP-07). Beta 401s have a null code; +// Files, Skills and Core project extensions report invalid_api_key only for a +// rejected Bearer credential. WWW-Authenticate and the message are unchanged. +func TestUnauthorizedEnvelopes(t *testing.T) { + handler, _, s := routingFixture(t) + invalidKey := strings.Replace(unauthorizedV1, `"code":null`, `"code":"invalid_api_key"`, 1) + for _, test := range []struct { + name string + authorization []string + scope []string + project string + }{ + {"missing", nil, nil, unauthorizedV1}, + {"basic", []string{"Basic " + routingKey}, nil, unauthorizedV1}, + {"empty bearer", []string{"Bearer"}, nil, unauthorizedV1}, + {"duplicate", []string{"Bearer " + routingKey, "Bearer " + routingKey}, nil, unauthorizedV1}, + {"invalid bearer", []string{"Bearer wrong"}, nil, invalidKey}, + {"wrong project", []string{"Bearer " + routingKey}, []string{"OpenAI-Project", "other"}, invalidKey}, + } { + header := withHeaders(beta, test.scope) + for _, value := range test.authorization { + header.Add("Authorization", value) + } + for _, route := range []struct{ method, path, want string }{ + {http.MethodGet, "/v1/agents/" + s.agent.ID, unauthorizedV1}, + {http.MethodGet, "/v1/files/file-missing", test.project}, + {http.MethodGet, "/v1/skills/skill-missing", test.project}, + {http.MethodDelete, "/core/v1/environments/" + uuid.NewString() + "/executor-credentials/" + uuid.NewString(), test.project}, + } { + got := serve(handler, route.method, route.path, "", header) + if got.Code != http.StatusUnauthorized || got.Body.String() != route.want || got.Header().Get("WWW-Authenticate") != "Bearer" { + t.Errorf("%s %s = %d %s", test.name, route.path, got.Code, got.Body) + } + } + } + got := serve(handler, http.MethodGet, "/core/v1/sandbox/nodes", "", http.Header{}) + if got.Code != http.StatusUnauthorized || !strings.Contains(got.Body.String(), `"type":"invalid_request_error","code":"invalid_admin_key"`) { + t.Fatalf("administrator 401 = %d %s", got.Code, got.Body) + } +} + +// A 405 keeps Core's JSON body and lists the route's methods (HP-20). Beta and +// authentication run first. +func TestMethodNotAllowedListsRouteMethods(t *testing.T) { + handler, _, s := routingFixture(t) + session := uuid.NewString() + for _, test := range []struct{ method, path, allow string }{ + {http.MethodPut, "/v1/agents/" + s.agent.ID, "GET,HEAD,POST,DELETE"}, + {http.MethodPatch, "/v1/agents/" + s.agent.ID, "GET,HEAD,POST,DELETE"}, + {http.MethodDelete, "/v1/agents", "GET,HEAD,POST"}, + {http.MethodPut, "/v1/agents/sessions/" + session + "/events", "GET,POST"}, + {http.MethodPost, "/v1/agents/sessions/" + session + "/turns", "GET,HEAD"}, + {http.MethodPost, "/v1/agents/sessions/" + session + "/artifacts/" + uuid.NewString() + "/content", "GET"}, + {http.MethodPut, "/v1//agents/" + s.agent.ID, "GET,HEAD,POST,DELETE"}, + // Wrong methods on Files and Skills paths reach the Beta group, as before. + {http.MethodPut, "/v1/files/file-missing", "GET,HEAD,DELETE"}, + } { + got := serve(handler, test.method, test.path, "", withHeaders(project, beta)) + if got.Code != http.StatusMethodNotAllowed || got.Body.String() != notAllowedV1 || got.Header().Get("Allow") != test.allow { + t.Errorf("%s %s = %d %q %s", test.method, test.path, got.Code, got.Header().Get("Allow"), got.Body) + } + } + if got := serve(handler, http.MethodPut, "/v1/agents/"+s.agent.ID, "", withHeaders(beta)); got.Code != http.StatusUnauthorized || got.Header().Get("Allow") != "" { + t.Fatalf("unauthenticated 405 = %d %s", got.Code, got.Body) + } + if got := serve(handler, http.MethodPut, "/v1/agents/"+s.agent.ID, "", withHeaders(project)); got.Code != http.StatusBadRequest { + t.Fatalf("Beta-less 405 = %d %s", got.Code, got.Body) + } +} + +// HEAD runs a GET route without a body after the same checks (HP-19). SSE and +// content-download routes answer 405 without opening a stream or reading content. +func TestHeadRequests(t *testing.T) { + handler, _, s := routingFixture(t) + server := httptest.NewServer(handler) + defer server.Close() + do := func(method, path string, header http.Header) (*http.Response, []byte) { + t.Helper() + request, err := http.NewRequest(method, server.URL+path, nil) + if err != nil { + t.Fatal(err) + } + request.Header = header + response, err := server.Client().Do(request) + if err != nil { + t.Fatal(err) + } + defer response.Body.Close() + body, _ := io.ReadAll(response.Body) + return response, body + } + authenticated := withHeaders(project, beta) + for _, path := range []string{"/v1/agents", "/v1/agents/" + s.agent.ID, "/v1//agents/" + s.agent.ID, "/v1/agents/" + uuid.NewString(), "/v1/files/file-missing", "/healthz"} { + get, getBody := do(http.MethodGet, path, authenticated) + head, headBody := do(http.MethodHead, path, authenticated) + if head.StatusCode != get.StatusCode || len(headBody) != 0 || head.ContentLength != int64(len(getBody)) || + head.Header.Get("Content-Type") != get.Header.Get("Content-Type") || !requestIDPattern.MatchString(head.Header.Get("X-Request-Id")) { + t.Errorf("HEAD %s = %d length %d %q; GET = %d %d", path, head.StatusCode, head.ContentLength, headBody, get.StatusCode, len(getBody)) + } + } + for _, test := range []struct { + header http.Header + status int + }{{withHeaders(project), http.StatusBadRequest}, {withHeaders(beta), http.StatusUnauthorized}} { + if head, _ := do(http.MethodHead, "/v1/agents", test.header); head.StatusCode != test.status { + t.Errorf("HEAD checks = %d, want %d", head.StatusCode, test.status) + } + } + session := "/v1/agents/sessions/" + uuid.NewString() + for _, test := range []struct{ path, allow string }{ + {session + "/events", "GET,POST"}, + {session + "/artifacts/" + uuid.NewString() + "/content", "GET"}, + {"/v1/files/file-missing/content", "GET"}, + {"/v1/skills/skill-missing/content", "GET"}, + {"/v1/skills/skill-missing/versions/1/content", "GET"}, + } { + head, _ := do(http.MethodHead, test.path, authenticated) + if head.StatusCode != http.StatusMethodNotAllowed || head.Header.Get("Allow") != test.allow || head.Header.Get("Content-Type") != "application/json" { + t.Errorf("HEAD %s = %d %q", test.path, head.StatusCode, head.Header.Get("Allow")) + } + if head, _ := do(http.MethodHead, test.path, withHeaders(beta)); head.StatusCode != http.StatusUnauthorized { + t.Errorf("unauthenticated HEAD %s = %d", test.path, head.StatusCode) + } + } +} + +// Every response carries a fresh request ID and the observed OpenAI headers +// (HP-23/HP-24), next to Core's traceparent and Cache-Control extensions. +func TestAgentsResponseHeaders(t *testing.T) { + handler, _, s := routingFixture(t) + seen := map[string]bool{} + for _, test := range []struct { + method, path string + header http.Header + status int + }{ + {http.MethodGet, "/healthz", nil, http.StatusOK}, + {http.MethodGet, "/v1/agents", withHeaders(project, beta), http.StatusOK}, + {http.MethodGet, "/v1/agents/" + s.agent.ID, withHeaders(project), http.StatusBadRequest}, + {http.MethodGet, "/v1/agents/" + s.agent.ID, withHeaders(beta), http.StatusUnauthorized}, + {http.MethodGet, "/v1/agents/" + uuid.NewString(), withHeaders(project, beta), http.StatusNotFound}, + {http.MethodGet, "/v1/agents/" + s.agent.ID + "/unknown", withHeaders(project, beta), http.StatusNotFound}, + {http.MethodPut, "/v1/agents/" + s.agent.ID, withHeaders(project, beta), http.StatusMethodNotAllowed}, + {http.MethodGet, "/v1/files/file-missing", nil, http.StatusUnauthorized}, + {http.MethodGet, "/core/v1/sandbox/nodes", nil, http.StatusUnauthorized}, + {http.MethodGet, "/unknown", nil, http.StatusNotFound}, + } { + got := serve(handler, test.method, test.path, "", test.header) + header := got.Header() + id := header.Get("X-Request-Id") + milliseconds, err := strconv.ParseInt(header.Get("Openai-Processing-Ms"), 10, 64) + if got.Code != test.status || !requestIDPattern.MatchString(id) || seen[id] || header.Get("Openai-Version") != "2020-10-01" || + err != nil || milliseconds < 0 || header.Get("X-Content-Type-Options") != "nosniff" || header.Get("Traceparent") == "" || + header.Get("Openai-Organization") != "" || header.Get("Openai-Project") != "" { + t.Errorf("%s %s = %d %v", test.method, test.path, got.Code, header) + } + if test.path != "/unknown" && header.Get("Cache-Control") != "no-store" { + t.Errorf("%s %s lost Cache-Control", test.method, test.path) + } + seen[id] = true + } + // The ID is attached to the request log context, and a stream that flushes + // before writing still reports its processing time. + var logged string + stream := agentsResponseHeaders(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + logged, _ = log.RequestIDFromContext(r.Context()) + w.Header().Set("Content-Type", "text/event-stream") + controller := http.NewResponseController(w) + if err := controller.SetWriteDeadline(time.Now().Add(time.Second)); err != nil { + t.Error(err) + } + if err := controller.Flush(); err != nil { + t.Error(err) + } + _, _ = io.WriteString(w, ": connected\n\n") + })) + server := httptest.NewServer(stream) + defer server.Close() + response, err := server.Client().Get(server.URL) + if err != nil { + t.Fatal(err) + } + _ = response.Body.Close() + if id := response.Header.Get("X-Request-Id"); !requestIDPattern.MatchString(id) || id != logged || response.Header.Get("Openai-Processing-Ms") == "" { + t.Fatalf("stream headers %v, logged %q", response.Header, logged) + } +} diff --git a/services/agents-api/internal/api/skills.go b/services/agents-api/internal/api/skills.go index 3fdcbeea8..ae326fb1c 100644 --- a/services/agents-api/internal/api/skills.go +++ b/services/agents-api/internal/api/skills.go @@ -33,11 +33,13 @@ func (h *Handler) registerSkillRoutes(r chi.Router) { r.Post("/v1/skills/{skill_id}", h.updateSkill) r.Delete("/v1/skills/{skill_id}", h.deleteSkill) r.Get("/v1/skills/{skill_id}/content", h.skillContent) + r.Head("/v1/skills/{skill_id}/content", methodNotAllowed) r.Post("/v1/skills/{skill_id}/versions", h.createSkillVersion) r.Get("/v1/skills/{skill_id}/versions", h.listSkillVersions) r.Get("/v1/skills/{skill_id}/versions/{version}", h.getSkillVersion) r.Delete("/v1/skills/{skill_id}/versions/{version}", h.deleteSkillVersion) r.Get("/v1/skills/{skill_id}/versions/{version}/content", h.skillVersionContent) + r.Head("/v1/skills/{skill_id}/versions/{version}/content", methodNotAllowed) } func (h *Handler) skillsReady(w http.ResponseWriter) bool { diff --git a/services/agents-api/internal/api/stream_test.go b/services/agents-api/internal/api/stream_test.go index 643037c3f..e4c70013a 100644 --- a/services/agents-api/internal/api/stream_test.go +++ b/services/agents-api/internal/api/stream_test.go @@ -102,6 +102,10 @@ func TestLiveStreamAuthDisconnectRecoveryAndServerDeadline(t *testing.T) { <-done } response := request("key") + // SSE responses carry the request ID and processing time (HP-23/HP-24). + if !requestIDPattern.MatchString(response.Header.Get("X-Request-Id")) || response.Header.Get("Openai-Processing-Ms") == "" { + t.Fatal("stream headers", response.Header) + } reader := bufio.NewReader(response.Body) if line, err := reader.ReadString('\n'); err != nil || line != ": connected\n" { t.Fatal(line, err) From 056819fc96d47339432b7b520278b2ce6deeef35 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 20:39:53 +0000 Subject: [PATCH 03/12] Recognize Core's invalid_request_error 401 in Core Web The connection probe accepts the current 401 envelope (type invalid_request_error, null code) and still recognizes the older invalid_api_key code. The result copy and fixtures no longer name the old code or Beta message. --- apps/web/e2e/fixture-sandbox.mjs | 2 +- .../src/components/ConnectionModal.test.tsx | 2 +- apps/web/src/components/ConnectionModal.tsx | 2 +- apps/web/src/lib/core-probe.test.ts | 11 ++++++++ apps/web/src/lib/core-probe.ts | 28 +++++++++++++------ docs/web/core-connection.md | 5 +++- 6 files changed, 38 insertions(+), 12 deletions(-) diff --git a/apps/web/e2e/fixture-sandbox.mjs b/apps/web/e2e/fixture-sandbox.mjs index 0c1664c06..6c6527f7a 100644 --- a/apps/web/e2e/fixture-sandbox.mjs +++ b/apps/web/e2e/fixture-sandbox.mjs @@ -28,7 +28,7 @@ export function handleSandboxFixture(request, response, url, sendJson, sendError if (path === "/__fixture/sandbox-add-node") { nodes.push({ ...node("node-enrolled", "Enrolled host"), provider }); sendJson(response, {}); return true; } const projectRoute = path === "/v1/sandbox/nodes" || /^\/v1\/agents\/sessions\/[^/]+\/sandbox-placement$/.test(path); if (projectRoute && request.headers["openai-beta"] !== "agents=v1") { - sendError(response, 400, "OpenAI-Beta: agents=v1 is required.", "invalid_beta"); return true; + sendError(response, 400, "To access the Agents API, set the 'OpenAI-Beta' header to 'agents=v1'.", "invalid_beta"); return true; } if (path === "/v1/sandbox/nodes") { sendJson(response, { data: nodes.map(({ id, name, online }) => ({ id, name, available: online })) }); return true; } if (/^\/v1\/agents\/sessions\/[^/]+\/sandbox-placement$/.test(path)) { diff --git a/apps/web/src/components/ConnectionModal.test.tsx b/apps/web/src/components/ConnectionModal.test.tsx index 1e4d0cba6..b7b9af535 100644 --- a/apps/web/src/components/ConnectionModal.test.tsx +++ b/apps/web/src/components/ConnectionModal.test.tsx @@ -188,7 +188,7 @@ describe("Connection probe status", () => { status: "complete", result: { kind: "unauthorized", executionReadiness: "unknown", httpStatus: 401 }, }, - "401 invalid_api_key", + "rejected the caller key (HTTP 401)", ], [ { diff --git a/apps/web/src/components/ConnectionModal.tsx b/apps/web/src/components/ConnectionModal.tsx index a5a41f8c5..2af11f128 100644 --- a/apps/web/src/components/ConnectionModal.tsx +++ b/apps/web/src/components/ConnectionModal.tsx @@ -183,7 +183,7 @@ function resultCopy(result: CoreProbeResult): { title: string; detail: string } case "unauthorized": return { title: "Authentication failed", - detail: "Core returned 401 invalid_api_key. Check the server-managed caller key or current-tab token.", + detail: "Core rejected the caller key (HTTP 401). Check the server-managed caller key or current-tab token.", }; case "protocol_mismatch": return { diff --git a/apps/web/src/lib/core-probe.test.ts b/apps/web/src/lib/core-probe.test.ts index c5eef9767..f7adbeaac 100644 --- a/apps/web/src/lib/core-probe.test.ts +++ b/apps/web/src/lib/core-probe.test.ts @@ -127,8 +127,19 @@ describe("Core connection probe", () => { expect(JSON.stringify(result)).not.toContain(token); }); + it.each([ + { type: "invalid_request_error", code: null, param: null, message: "A valid Agents API bearer key is required." }, + { type: "authentication_error", code: "invalid_api_key", param: null, message: "Older Core envelope." }, + ])("classifies the current and older Core 401 envelopes", async (error) => { + const result = await probeCore({ baseUrl: "/v1", fetch: recordingFetch(jsonResponse({ error }, 401), []) }); + + expect(result).toEqual({ kind: "unauthorized", executionReadiness: "unknown", httpStatus: 401 }); + }); + it.each([ jsonResponse({ error: { code: "gateway_auth_required" } }, 401), + jsonResponse({ error: { type: "invalid_request_error", code: "gateway_auth_required" } }, 401), + jsonResponse({ error: { type: "invalid_request_error" } }, 401), new Response("proxy login required", { status: 401 }), ])("does not claim invalid_api_key for a non-canonical 401", async (response) => { const result = await probeCore({ baseUrl: "/v1", fetch: recordingFetch(response, []) }); diff --git a/apps/web/src/lib/core-probe.ts b/apps/web/src/lib/core-probe.ts index 82de58b5a..a7d08a8ba 100644 --- a/apps/web/src/lib/core-probe.ts +++ b/apps/web/src/lib/core-probe.ts @@ -103,10 +103,22 @@ function isAgentListPage(value: unknown): boolean { return value.first_id === value.data[0]?.id && value.last_id === value.data.at(-1)?.id; } -async function readErrorCode(response: Response): Promise { +interface ProbeError { + type?: unknown; + code?: unknown; +} + +async function readError(response: Response): Promise { const envelope: unknown = await response.json(); if (!isRecord(envelope) || !isRecord(envelope.error)) return undefined; - return typeof envelope.error.code === "string" ? envelope.error.code : undefined; + return envelope.error; +} + +// Core reports a rejected Agents API caller as invalid_request_error with a +// null code, as the official service does; older Core used invalid_api_key. +// Other 401 bodies, such as an intermediary's login, are not Core's answer. +function isCoreUnauthorized(error: ProbeError | undefined): boolean { + return error?.code === "invalid_api_key" || (error?.type === "invalid_request_error" && error.code === null); } function classifyBodyReadFailure( @@ -192,19 +204,19 @@ export async function probeCore(options: CoreProbeOptions): Promise Date: Wed, 23 Sep 2026 20:43:00 +0000 Subject: [PATCH 04/12] Document HTTP routing and header alignment Record rows RH1-RH11 with the campaign scan 6 evidence, register the RH evidence code, correct the Beta check order and the 401 envelope in the contract documents, and add the canonical path rule to CONTRIBUTING. --- CONTRIBUTING.md | 16 +++++ contracts/agents-api/README.md | 7 +- .../official-semantics-alignment.md | 66 ++++++++++++++++++- contracts/agents-api/operation-evidence.md | 3 +- contracts/agents-api/runtime-history-api.md | 2 +- .../agents-api/runtime-observability-api.md | 2 +- 6 files changed, 90 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 58065ae06..3a8fffee6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -108,6 +108,22 @@ explicitly with its `metadata.` param; other stored strings rely on the PostgreSQL error mapping, so keep each request's writes in one transaction. See `contracts/agents-api/official-semantics-alignment.md`. +Serve requests on their canonical path and never redirect. `api.CanonicalPaths` +wraps the complete server handler in both configurations (the daemon ServeMux and +the API router alone), so every route group, middleware, authentication check and +handler sees one path: unreserved escapes decoded, empty and dot segments resolved +with ServeMux semantics, trailing slash kept, other escapes such as `%2F` left +encoded. Do not route or authorize on a path outside that wrapper. On the Beta +group the constant OpenAI-Beta check (exactly one `agents=v1` value) runs before +authentication, and authentication still precedes every Beta handler, 404 and 405. +Every 401 has type `invalid_request_error`: null code on Beta routes; on Files, +Skills and Core project extensions `invalid_api_key` only for a rejected Bearer +credential. Agents API responses carry a fresh `X-Request-Id` (also in the log +context), `OpenAI-Version`, `OpenAI-Processing-Ms` and nosniff through the API +router's own middleware, not the shared log middleware. HEAD runs GET routes; +streaming and content-download routes register an explicit HEAD 405 instead. A 405 +lists the route's methods in `Allow`. + Keep runtime state, test artifacts and build output under `~/.parsar/`. Require absolute user-supplied working directories. Keep credentials out of source and logs. Update this guide when architecture, ownership or generated contracts change. diff --git a/contracts/agents-api/README.md b/contracts/agents-api/README.md index d1950e825..298fd96cd 100644 --- a/contracts/agents-api/README.md +++ b/contracts/agents-api/README.md @@ -877,8 +877,11 @@ Caller keys now resolve an explicitly configured organization/project and typed user/service-account identity. An immutable project-to-tenant mapping is verified against PostgreSQL before startup. Optional official organization/project headers must match the key's authorized scope; ambiguous or conflicting headers use the -existing `401 invalid_api_key` response. This error policy is an implementation -choice, not verified hosted error parity. Project resource access remains shared +existing 401 response. Every 401 has type `invalid_request_error`, as observed +officially; Beta routes report a null code, while Files, Skills and Core project +extensions report `invalid_api_key` for a rejected Bearer credential and a null +code without one. This scope-header policy is an implementation choice, not +verified hosted error parity. Project resource access remains shared within the authorized project. New Sessions persist immutable creator kind/ID from the authenticated principal; ordinary and streaming creation retries require the same typed subject, including when recovering before saved-Agent lookup. Rotated diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index 4edf6d489..3520e7883 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -20,7 +20,7 @@ replacement baseline. A successful SDK parse alone is not conformance evidence. | OAuth grant create/replacement | Reject an explicitly empty access token and a replacement without mutable grant fields. Preserve previously qualified refresh/expiry/null handling. | | Input message discriminator | Omission remains valid; a supplied `type` must be `message`. Explicit null or empty strings reject through the shared initial/event decoder, matching the pinned literal type. | | Missing beta resource | HTTP 404 with `type` and `code` equal to `not_found_error`. Missing and foreign resources remain indistinguishable. | -| Missing required Beta header | HTTP 400 with `type` and `code` equal to `invalid_beta`, after authentication. | +| Missing required Beta header | HTTP 400 with `type` and `code` equal to `invalid_beta`, before authentication (see [HTTP routing and response headers](#http-routing-and-response-headers--september-23)). | | Missing non-beta File or Skill | HTTP 404 with `type: invalid_request_error`, `code: null`. Exact message, File `param` and additional detail payload remain outside this batch. | The resource comparison made 40 raw requests over six newly owned resources @@ -793,3 +793,67 @@ checks retrieve, list, the live GET stream and its end, the exact 409, tenant B projection, stream lifetime, wire shapes and error mapping; Python tests pin the initializer receipt, and the TypeScript client and Web unit tests pass. Live Docker acceptance is recorded separately by the coordinator. + +## HTTP routing and response headers — September 23 + +This batch aligns path handling, the Beta check, 401 envelopes and response +headers with campaign scan 6 at Core main `1eb60c27`, recorded privately in +`~/.parsar/remediation/20260923/campaign-scan-6/http-protocol/` (`findings.json` +HP-02..24, raw `official-ledger.jsonl` and `REPORT.txt`): 90 official requests on +one owned Agent, deleted afterwards, without a Session or model. Labels below are +ledger records. + +| Row | Case | Core behavior | +| --- | --- | --- | +| RH1 | `//`, `.` or `..` path segments (HP-17: `R10`, `R11`, `R17`, `R18`) | Served on the canonical path, never redirected. Empty and dot segments resolve with ServeMux semantics and a trailing slash is kept, so `/v1/agents/x/../` still reaches the trailing-slash 404. The former 301 made the pinned SDK resend an update as a GET and drop it. | +| RH2 | A percent-encoded unreserved character in the path (HP-18: `R12`) | Decoded before routing, including `%2E` dot segments. Other escapes, such as `%2F`, stay encoded and never separate segments. Malformed, missing and foreign IDs keep the single 404. | +| RH3 | HEAD on a GET route (HP-19: `R13`, `R16`) | The GET route runs after the same Beta and authentication checks; 200 with its headers and `Content-Length`, no body. The events stream and the File, Skill, Skill version and Artifact content downloads answer HEAD with Core's 405 instead, so HEAD never holds a stream open or reads content. That exclusion is a documented Core difference; official HEAD on those routes is unobserved. | +| RH4 | Unsupported method (HP-20: `R04`–`R06`) | Unchanged 405 JSON `unsupported_operation`, now with `Allow` listing the route's methods in the observed order, such as `GET,HEAD,POST,DELETE`. | +| RH5 | `X-Request-Id` (HP-23) | Every response of the Agents API handler, including 400, 401, 404, 405 and SSE streams, carries a fresh random `req_` plus 32 lowercase hex characters. The ID is attached to the request log context as `request_id` next to the trace carrier. | +| RH6 | No or invalid credentials without OpenAI-Beta on a Beta route (HP-05: `A06`, `A11`) | 400 `invalid_beta`: the constant Beta check now precedes authentication. Files, Skills and Core project extensions still ignore the header. | +| RH7 | Repeated OpenAI-Beta header lines (HP-03: `B08`) | 400 `invalid_beta` unless there is exactly one field value, equal to `agents=v1`. | +| RH8 | 401 (HP-07: `A01`–`A04`, `A07`–`A10`) | Type `invalid_request_error`. Beta routes report a null code for every failure. Files, Skills and Core project extensions report a null code without a Bearer credential (missing, other scheme, empty or repeated header) and `invalid_api_key` for a rejected one, including mismatched scope headers. Core's message and `WWW-Authenticate: Bearer` are kept. | +| RH9 | `invalid_beta` message (HP-02: `B01`) | "To access the Agents API, set the 'OpenAI-Beta' header to 'agents=v1'." | +| RH10 | Optional headers (HP-24) | `OpenAI-Version: 2020-10-01`, `OpenAI-Processing-Ms` and `X-Content-Type-Options: nosniff`. Organization and project headers are not reported: Core's project scope is configured, not account-derived. | +| RH11 | Trailing slashes and unknown sub-routes (HP-21), OPTIONS and CORS (HP-22), `Cache-Control` and `traceparent` (HP-26), `agents=v0` (HP-04) | Unchanged: 404 JSON, no CORS handling, Core's extension headers kept, `agents=v0` still rejected (an upstream anomaly, not copied). | + +Decisions: + +- One canonicalizing handler wraps the complete server handler in both server + configurations: around the ServeMux that also serves daemon, enrollment and node + transport, and around the API router when it is served alone. The ServeMux, the + router, every middleware, authentication check and handler see only the + rewritten path. A dirty or encoded path therefore reaches exactly the route + group and authentication of its canonical path written literally; internal + daemon, node and sandbox routes keep their own authentication. Decoding only + unreserved characters is RFC 3986 normalization, so a proxy that normalizes + URIs the same way sees the same route. +- The Beta check reads only a constant header and returns no tenant or resource + data. Moving it first changes only responses that were rejected either way: + every request that passes it is authenticated before the router reaches any + Beta handler, 404 or 405. +- Wrong methods and unknown sub-routes below `/v1/files` and `/v1/skills` still + reach the Beta group's 404 and 405 after its checks, as before; without the Beta + header they now report `invalid_beta` instead of 401. Official behavior there is + unobserved. +- Every Core 401 has type `invalid_request_error`, including the deployment + administrator (`invalid_admin_key`) and sandbox node (`invalid_node_credential`) + extensions, whose codes are unchanged. +- The response headers belong to the Agents API handler. Daemon, enrollment and + node transport routes do not carry them, and the shared log middleware is + unchanged. A caller-supplied request ID is not echoed; that header is not pinned. +- Core Web recognizes both the current 401 envelope and the older + `invalid_api_key` code in its connection probe. The TypeScript client already + exposes status, type and code without branching on them. The Parsar product + repository has no code branching on these 401 fields, and its Go client refuses + redirects. + +Go handler tests cover RH1–RH11, including a walk over every registered route: +unauthenticated requests, with and without the Beta header and with foreign +credentials, are rejected before any handler, and ten dirty and encoded spellings +of each path, including traversal from the daemon and sandbox prefixes, give the +clean path's exact response. A server test replays the daemon-enabled composition +and checks that no request is redirected. The pinned-SDK script +`official_http_routing.py` updates an Agent through base URL `/v1//`, checks +`_request_id` and the error `request_id`, and the raw checks in the other official +scripts now expect the Beta check first. diff --git a/contracts/agents-api/operation-evidence.md b/contracts/agents-api/operation-evidence.md index 82de16bf8..a53f89b4b 100644 --- a/contracts/agents-api/operation-evidence.md +++ b/contracts/agents-api/operation-evidence.md @@ -1,6 +1,6 @@ # Pinned operation evidence inventory — 2026-09-23 -Baseline inventory of main `b5715912f09333e2b4449ec6f0eecaabce44c9b7`. The Session admission batch below updates creation and metadata validation, the list query tolerance batch (L) updates list and resource query handling, the validation error batch (X) updates field error codes/params, malformed path IDs and U+0000 handling, the Environment Files wire batch (G) updates Files.create/list status, envelope, query, path and empty-page behavior, and the creation stream settlement batch (J) updates creation SSE lifetime/snapshot, terminal Turn usage and Turn start order, and the Session deletion batch (Z) updates the deletion lifecycle, and the Agent configuration validation batch (M) updates saved and inline Agent configuration errors, the whitespace input batch (P) admits whitespace-only message text, the list cursor error batch (CE) updates unresolved `after` cursor errors on every list, the input conflict batch (CF) gives every 409 type `conflict_error` and aligns Session input conflicts and tool result target errors, and the saved web_search batch (SW) saves every pinned `web_search` mode while Session admission keeps rejecting enabled search, and the workspace file write batch (FW) aligns Files.create parent creation, no-replacement and the inline size bound, and the item serialization batch (SR) aligns Item/event null fields, assistant message event framing, reasoning keys and the Session usage rule, and the MCP credential selection batch (MV) saves an omitted HTTP MCP origin as `service`, projects the implicitly selected Session credential and aligns selection errors, and the hosted initialization failure batch (HF) aligns failed hosted provisioning: Session status/error, failure events, stream end and later-input 409; historical evidence retains its original revision and scope. This inventory guides repeated qualification and does not assert complete compatibility. +Baseline inventory of main `b5715912f09333e2b4449ec6f0eecaabce44c9b7`. The Session admission batch below updates creation and metadata validation, the list query tolerance batch (L) updates list and resource query handling, the validation error batch (X) updates field error codes/params, malformed path IDs and U+0000 handling, the Environment Files wire batch (G) updates Files.create/list status, envelope, query, path and empty-page behavior, and the creation stream settlement batch (J) updates creation SSE lifetime/snapshot, terminal Turn usage and Turn start order, and the Session deletion batch (Z) updates the deletion lifecycle, and the Agent configuration validation batch (M) updates saved and inline Agent configuration errors, the whitespace input batch (P) admits whitespace-only message text, the list cursor error batch (CE) updates unresolved `after` cursor errors on every list, the input conflict batch (CF) gives every 409 type `conflict_error` and aligns Session input conflicts and tool result target errors, and the saved web_search batch (SW) saves every pinned `web_search` mode while Session admission keeps rejecting enabled search, and the workspace file write batch (FW) aligns Files.create parent creation, no-replacement and the inline size bound, and the item serialization batch (SR) aligns Item/event null fields, assistant message event framing, reasoning keys and the Session usage rule, and the MCP credential selection batch (MV) saves an omitted HTTP MCP origin as `service`, projects the implicitly selected Session credential and aligns selection errors, and the hosted initialization failure batch (HF) aligns failed hosted provisioning: Session status/error, failure events, stream end and later-input 409, and the HTTP routing and header batch (RH) serves canonical paths without redirects, checks OpenAI-Beta before authentication and aligns 401 envelopes, HEAD, `Allow` and request ID headers on every operation; historical evidence retains its original revision and scope. This inventory guides repeated qualification and does not assert complete compatibility. Baseline: `contracts/agents-api/upstream.json`, SDK **3.13.0**, upstream commit **d7c41efee1b0802b79f3f88a678ef2052b06e9ce**, `OpenAI-Beta: agents=v1`. AGENTS.md and relevant CONTRIBUTING.md compatibility, ownership and evidence rules govern this inventory. @@ -52,6 +52,7 @@ Repository paths below are relative to the inspected worktree; private evidence | SR | [Item serialization](history-events-usage.md#item-serialization-2026-09-23); private `~/.parsar/remediation/20260923/campaign-scan-2/events-tools/findings.json` EVT-09, EVT-10, EVT-13 with raw frames `official/streams-s1..s4.json` and Items pages `official/calls-s2.json` `s2-items-after-t1`, `calls-s4.json` `s4-items`; `campaign-scan-1/sessions/findings.json` SES-23/25 and `campaign-scan-1/vaults-agents/findings.json` VA-11; `campaign-scan-5/sessions-turns/findings.json` ST-03 with raw `official/calls.json`; live-kit EVT-24 `creation-stream-settlement/acceptance/candidate-evidence/codex-kimi/attempt-1/r1-*.json`. Rows S1–S8 of that section. Go contract, API and real-PostgreSQL store tests, Codex adapter usage tests, the pinned-SDK official client suite and TypeScript client/Web tests; live acceptance is recorded with the batch. | | MV | [MCP origin and credential selection](official-semantics-alignment.md#mcp-origin-and-credential-selection--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/mcp-vaults/findings.json` MV-01..03 with raw records in `official-ledger.jsonl` (`AG1-minimal-and-nulls`, `S1-T1-create-stream`, `S1-after-c1-delete-session-get`, `S1-final-session-get`, `ERR-UNATTACHED`, `ERR-URL-MISMATCH`, `ERR-AMBIGUOUS`, `ERR-CREDENTIAL-BOGUS`, `ERR-VAULT-BOGUS`): owned Agents, three owned Sessions, two Vaults and four Credentials, all deleted. Rows M1–M9 of that section. Go API/store and real-PostgreSQL tenant A/B HTTP tests with a no-write digest, pinned-SDK official client scripts; live acceptance is recorded with the batch. | | HF | [Hosted initialization failure](official-semantics-alignment.md#hosted-initialization-failure--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/hosted-init/findings.json` HI-01..04 with raw records under `official/` (`006-S2-create-setup-exit3`, `007-S2-events`, `009-S3-events`, `021-S2-env-after-failed`, `024-S2-session-after-failed`, `025-S3-session-after-failed`, `026-S2-input-after-failure`, `027-S3-input-after-failure`, `038-S2-delete`, `041-S3-delete`): two owned `openai_hosted` Sessions without a Turn (setup exit 3, nonexistent Python package), both deleted. Rows H1–H8 of that section; HI-05/06 stay deferred. Python initializer receipt tests, Go contract/API tests, real-PostgreSQL managed-Worker store tests with a controlled Provider and an HTTP test with tenant B and a canary, TypeScript client and Web tests; live Docker acceptance is recorded with the batch. | +| RH | [HTTP routing and response headers](official-semantics-alignment.md#http-routing-and-response-headers--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/http-protocol/findings.json` HP-02, 03, 05, 07, 17–20, 23, 24 (HP-04, 21, 22, 26 kept) with raw records in `official-ledger.jsonl` (labels `A01`–`A11`, `B01`, `B08`, `R04`–`R18`, `S2-retrieve-baseline`, `C01-malformed`, `X1-delete-owned`) and `REPORT.txt`: one owned Agent (deleted), 90 requests without a Session or model. Rows RH1–RH11 apply to every operation's HTTP layer; HEAD on the events stream and content downloads is a documented local 405. | ## Per-operation evidence matrix diff --git a/contracts/agents-api/runtime-history-api.md b/contracts/agents-api/runtime-history-api.md index 199a27afc..efe4d278f 100644 --- a/contracts/agents-api/runtime-history-api.md +++ b/contracts/agents-api/runtime-history-api.md @@ -134,7 +134,7 @@ separate sources and are never silently merged. | HTTP | Code | Meaning | | --- | --- | --- | | 400 | `unsupported_parameter` or `invalid_request` | Invalid query shape or range. | -| 401 | `authentication_error` | Missing or invalid authentication. | +| 401 | null (type `invalid_request_error`) | Missing or invalid authentication. | | 404 | `not_found` | Missing or foreign Session. | | 409 | `runtime_history_unsupported` | The Session has no supported managed Runtime history scope. | | 503 | `runtime_history_unavailable` | Durable history is unconfigured, timed out, unavailable, or returned malformed data. | diff --git a/contracts/agents-api/runtime-observability-api.md b/contracts/agents-api/runtime-observability-api.md index e30d4a37a..b8a5f569e 100644 --- a/contracts/agents-api/runtime-observability-api.md +++ b/contracts/agents-api/runtime-observability-api.md @@ -183,7 +183,7 @@ Use the existing Agents API error envelope. | --- | --- | --- | | 400 | `invalid_request_error` / `invalid_request_error` | List: a repeated supported query key, or an empty or invalid limit or order, with the shared Beta list messages. Unknown list query keys are ignored. | | 400 | `invalid_request_error` / `unsupported_parameter` | Single-Session retrieval with any query parameter. | -| 401 | `authentication_error` / `invalid_api_key` | Missing or invalid API authentication. | +| 401 | `invalid_request_error` / null code | Missing or invalid API authentication. | | 404 | `not_found_error` / `not_found_error` | Missing, malformed or foreign Session/cursor, indistinguishably, as for the [Session list cursor](list-query-semantics.md#list-cursor-errors--september-23-2026). | | 500 | `server_error` / `internal_error` | Integrity, ownership, or invalid provider evidence. | | 503 | `server_error` / `execution_unavailable` | Required Runtime observation service is not configured, or list collection exceeded its request budget. | From 8fbbc97485e8e71e49ed482d3435626aa766dbbd Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 20:43:10 +0000 Subject: [PATCH 05/12] Check routing and headers with the pinned SDK official_http_routing.py updates an Agent through base URL /v1//, reads _request_id and the error request_id, and checks HEAD, Allow, the Beta order and 401 envelopes. Raw checks in the other official scripts and the Go client service test now expect the Beta check before authentication and a null-code Beta 401. --- packages/agents-client/v1/service_test.go | 8 ++- .../agents-api/tests/official_agent_list.py | 4 +- services/agents-api/tests/official_agents.py | 4 +- services/agents-api/tests/official_auth.py | 6 +- services/agents-api/tests/official_client.py | 2 + .../tests/official_credential_delete.py | 4 +- .../tests/official_credential_list.py | 5 +- .../tests/official_credential_rotation.py | 4 +- .../agents-api/tests/official_credentials.py | 4 +- .../tests/official_environment_retrieve.py | 4 +- .../agents-api/tests/official_http_routing.py | 72 +++++++++++++++++++ .../agents-api/tests/official_vault_delete.py | 4 +- .../agents-api/tests/official_vault_list.py | 4 +- services/agents-api/tests/official_vaults.py | 5 +- 14 files changed, 116 insertions(+), 14 deletions(-) create mode 100644 services/agents-api/tests/official_http_routing.py diff --git a/packages/agents-client/v1/service_test.go b/packages/agents-client/v1/service_test.go index 13ddd6c0c..6abdc6cd5 100644 --- a/packages/agents-client/v1/service_test.go +++ b/packages/agents-client/v1/service_test.go @@ -5,6 +5,7 @@ import ( "errors" "os" "slices" + "strings" "testing" "time" @@ -108,5 +109,10 @@ func TestService(t *testing.T) { _, err = b.List(ctx, openai.BetaAgentSessionListParams{After: openai.String(first.ID)}) expectStatus(err, 404) _, err = invalid.Get(ctx, first.ID) - expectStatus(err, 401) + // Beta 401s are invalid_request_error with a null code and a request ID (HP-07/HP-23). + var unauthorized *openai.Error + if !errors.As(err, &unauthorized) || unauthorized.StatusCode != 401 || unauthorized.Type != "invalid_request_error" || + unauthorized.Code != "" || !strings.HasPrefix(unauthorized.Response.Header.Get("X-Request-Id"), "req_") { + t.Fatalf("expected SDK HTTP 401 error: %v", err) + } } diff --git a/services/agents-api/tests/official_agent_list.py b/services/agents-api/tests/official_agent_list.py index 3ea64fb3d..aef1af841 100644 --- a/services/agents-api/tests/official_agent_list.py +++ b/services/agents-api/tests/official_agent_list.py @@ -53,6 +53,8 @@ def verify_agent_list(client, other, invalid, saved, expect_error): assert missing.status_code == malformed.status_code == 404 and missing.json() == malformed.json() zero = raw.get(url, headers=headers, params={"limit": "0", "tenant_id": "other"}).json() assert [agent["id"] for agent in zero["data"]] == [desc[0].id] and zero["has_more"] is True - assert raw.get(url).status_code == 401 + # The Beta header is checked before authentication (HP-05). + assert raw.get(url).status_code == 400 + assert raw.get(url, headers={"OpenAI-Beta": "agents=v1"}).status_code == 401 print("Agent list: fixed SDK auto-pagination/raw HTTP, order/cursors, snapshots, isolation and local limit/envelope behavior passed; exact upstream defaults/caps/errors remain unverified.") return [agent.id for agent in asc] diff --git a/services/agents-api/tests/official_agents.py b/services/agents-api/tests/official_agents.py index 6cb781800..898fa9ccb 100644 --- a/services/agents-api/tests/official_agents.py +++ b/services/agents-api/tests/official_agents.py @@ -153,7 +153,9 @@ def verify_agents(client, other, invalid, expect_error): expect_error(AuthenticationError, lambda: invalid.beta.agents.retrieve(saved[0].id)) expect_error(AuthenticationError, lambda: invalid.beta.agents.create(model="x")) expect_error(BadRequestError, lambda: agents.create(model="x", extra_headers={"OpenAI-Beta": ""})) - assert raw.post(base, json={"model": "x"}).status_code == 401 + # The Beta header is checked before authentication (HP-05). + assert raw.post(base, json={"model": "x"}).json()["error"]["code"] == "invalid_beta" + assert raw.post(base, headers={"OpenAI-Beta": "agents=v1"}, json={"model": "x"}).status_code == 401 # The minimal pinned MCP tool saves its omitted origin as "service" (MV-01); # the environment origin remains an explicit gap. mcp = {"type": "mcp", "server_label": "x", "transport": {"type": "http", "server_url": "https://example.invalid"}} diff --git a/services/agents-api/tests/official_auth.py b/services/agents-api/tests/official_auth.py index 774672acc..768b24a7b 100644 --- a/services/agents-api/tests/official_auth.py +++ b/services/agents-api/tests/official_auth.py @@ -19,7 +19,8 @@ def verify_caller_principals(client, base, binding, token, rotated, peer, sessio try: invalid.beta.agents.sessions.retrieve(session.id) except AuthenticationError as error: - assert error.body["code"] == "invalid_api_key" + # Beta 401s are invalid_request_error with a null code (HP-07). + assert error.body["type"] == "invalid_request_error" and error.body["code"] is None else: raise AssertionError("Untrusted scope header was accepted") headers = [("Authorization", "Bearer " + token), ("OpenAI-Beta", "agents=v1")] @@ -29,7 +30,8 @@ def verify_caller_principals(client, base, binding, token, rotated, peer, sessio [('OpenAI-Organization', binding['organization_id']), ('OpenAI-Organization', 'other')], [('Authorization', 'Bearer ' + peer)]): response = raw.get(path, headers=headers + extra) - assert response.status_code == 401 and response.json()["error"]["code"] == "invalid_api_key" + error = response.json()["error"] + assert response.status_code == 401 and error["type"] == "invalid_request_error" and error["code"] is None response = raw.get(path, headers=headers + [("X-Tenant-ID", str(uuid.uuid4())), ("X-User-ID", "forged")]) assert response.status_code == 200 and response.json()["id"] == session.id diff --git a/services/agents-api/tests/official_client.py b/services/agents-api/tests/official_client.py index f95b39834..f533a524c 100644 --- a/services/agents-api/tests/official_client.py +++ b/services/agents-api/tests/official_client.py @@ -30,6 +30,7 @@ from official_credential_delete import verify_credential_deletion, verify_credential_deletion_recovery, verify_keyless_credential_deletion from official_vault_delete import verify_vault_deletion, verify_vault_deletion_recovery, verify_keyless_vault_deletion from official_agent_list import verify_agent_list +from official_http_routing import verify_http_routing from official_agent_references import verify_agent_references from official_session_requests import verify_session_create_requests from official_session_metadata import verify_session_metadata, verify_active_session_metadata @@ -163,6 +164,7 @@ def expect_error(error, operation): listed_files = verify_source_file_list(a, b, invalid, peer, expect_error) saved_agents = verify_agents(a, b, invalid, expect_error) listed_agents = verify_agent_list(a, b, invalid, saved_agents, expect_error) + verify_http_routing(base, tokens[0]) sessions = a.beta.agents.sessions spec = {"input": "Verify client fixture admission.", "agent": {"model": "requested-test-model", "instructions": "Keep the configuration."}, "environment": {"type": "none"}} headers = {"Idempotency-Key": "same-key"} diff --git a/services/agents-api/tests/official_credential_delete.py b/services/agents-api/tests/official_credential_delete.py index 89edd07d0..aef43b2d5 100644 --- a/services/agents-api/tests/official_credential_delete.py +++ b/services/agents-api/tests/official_credential_delete.py @@ -23,7 +23,9 @@ def verify_credential_deletion(client, other, invalid, peer, canary, expect_erro expect_error(AuthenticationError, lambda: invalid.beta.agents.vaults.credentials.delete(target.id, vault_id=vault.id)) with httpx2.Client(trust_env=False, timeout=10) as raw: url = endpoint + "/" + target.id - assert raw.delete(url).status_code == 401 + # The Beta header is checked before authentication (HP-05). + assert raw.delete(url).status_code == 400 + assert raw.delete(url, headers={"OpenAI-Beta": "agents=v1"}).status_code == 401 response = raw.delete(url, headers={"Authorization": headers["Authorization"]}) assert response.status_code == 400 and response.json()["error"]["code"] == "invalid_beta" # The body rejects; the unknown include key is ignored and never exposes the token. diff --git a/services/agents-api/tests/official_credential_list.py b/services/agents-api/tests/official_credential_list.py index 461925ece..2151c99f0 100644 --- a/services/agents-api/tests/official_credential_list.py +++ b/services/agents-api/tests/official_credential_list.py @@ -147,7 +147,10 @@ def safe_error(response, status): for params in invalid_queries: safe_error(raw.get(endpoint, headers=headers, params=params), 400) for suffix in (vault.id, "invalid-vault"): - safe_error(raw.get(base + suffix + "/credentials", params={"after": "invalid"}), 401) + # The Beta header is checked before authentication (HP-05). + safe_error(raw.get(base + suffix + "/credentials", params={"after": "invalid"}), 400) + safe_error(raw.get(base + suffix + "/credentials", headers={"OpenAI-Beta": "agents=v1"}, + params={"after": "invalid"}), 401) for beta in (None, "agents=v2"): auth = {"Authorization": headers["Authorization"]} if beta is not None: diff --git a/services/agents-api/tests/official_credential_rotation.py b/services/agents-api/tests/official_credential_rotation.py index 84e4eebd0..6d263c88c 100644 --- a/services/agents-api/tests/official_credential_rotation.py +++ b/services/agents-api/tests/official_credential_rotation.py @@ -94,7 +94,9 @@ def metadata(response, previous): original.id, vault_id=vault.id, **replacement)) expect_error(AuthenticationError, lambda: invalid.beta.agents.vaults.credentials.update( original.id, vault_id=vault.id, **replacement)) - safe(raw.post(endpoint, json=replacement), 401) + # The Beta header is checked before authentication (HP-05). + assert safe(raw.post(endpoint, json=replacement), 400)["error"]["code"] == "invalid_beta" + safe(raw.post(endpoint, headers={"OpenAI-Beta": "agents=v1"}, json=replacement), 401) assert safe(raw.post(endpoint, headers={"Authorization": headers["Authorization"]}, json=replacement), 400)["error"]["code"] == "invalid_beta" for scope in ({"OpenAI-Project": "other-project"}, {"OpenAI-Organization": "other-organization"}): diff --git a/services/agents-api/tests/official_credentials.py b/services/agents-api/tests/official_credentials.py index 2ccc6196c..81a157769 100644 --- a/services/agents-api/tests/official_credentials.py +++ b/services/agents-api/tests/official_credentials.py @@ -109,7 +109,9 @@ def safe_body(response, status): expect_error(AuthenticationError, lambda: credentials.retrieve(saved[0].id, vault_id=vault.id, extra_headers=scope)) for method, url in (("POST", endpoint), ("GET", endpoint + "/" + saved[0].id)): body = {"json": request} if method == "POST" else {} - safe_body(raw.request(method, url, **body), 401) + # The Beta header is checked before authentication (HP-05). + safe_body(raw.request(method, url, **body), 400) + safe_body(raw.request(method, url, headers={"OpenAI-Beta": "agents=v1"}, **body), 401) response = raw.request(method, url, headers={"Authorization": headers["Authorization"]}, **body) assert safe_body(response, 400)["error"]["code"] == "invalid_beta" # Unknown query keys are ignored: they neither select a tenant nor expose tokens. diff --git a/services/agents-api/tests/official_environment_retrieve.py b/services/agents-api/tests/official_environment_retrieve.py index 7e7c019ed..780d8632d 100644 --- a/services/agents-api/tests/official_environment_retrieve.py +++ b/services/agents-api/tests/official_environment_retrieve.py @@ -70,7 +70,7 @@ def rejected(url, status, code, request_headers=headers, method="GET"): assert response.status_code == status body = response.json() assert set(body) == {"error"} and body["error"]["code"] == code - assert body["error"]["type"] == ("authentication_error" if status == 401 else code if code in {"not_found_error", "invalid_beta"} else "invalid_request_error") + assert body["error"]["type"] == (code if code in {"not_found_error", "invalid_beta"} else "invalid_request_error") for private in (token, settings["peer_token"], settings["foreign_token"], settings["executor_token"], environment_id): assert private not in response.text @@ -92,7 +92,7 @@ def rejected(url, status, code, request_headers=headers, method="GET"): request_headers = {"OpenAI-Beta": "agents=v1"} if authorization is not None: request_headers["Authorization"] = authorization - rejected(endpoint, 401, "invalid_api_key", request_headers) + rejected(endpoint, 401, None, request_headers) for beta in (None, "agents=v2"): request_headers = {"Authorization": "Bearer " + token} if beta is not None: diff --git a/services/agents-api/tests/official_http_routing.py b/services/agents-api/tests/official_http_routing.py new file mode 100644 index 000000000..efa77df80 --- /dev/null +++ b/services/agents-api/tests/official_http_routing.py @@ -0,0 +1,72 @@ +"""Exercise path canonicalization and response headers with the pinned SDK and raw HTTP.""" + +import re + +import httpx2 +from openai import AuthenticationError, OpenAI + +REQUEST_ID = re.compile(r"req_[0-9a-f]{32}") + + +def verify_http_routing(base, token): + paths = [] + record = {"request": [lambda request: paths.append(request.url.raw_path.decode())]} + # HP-17: Core serves the canonical path instead of redirecting, so an update + # through a non-canonical base URL applies instead of becoming a GET. + with OpenAI(api_key=token, base_url=base + "/v1//", max_retries=0, + http_client=httpx2.Client(trust_env=False, timeout=10, event_hooks=record)) as sdk: + agent = sdk.beta.agents.create(model="routing-model", name="Routing") + updated = sdk.beta.agents.update(agent.id, metadata={"route": "double-slash"}) + assert updated.id == agent.id and updated.metadata == {"route": "double-slash"} + assert sdk.beta.agents.retrieve(agent.id).metadata == {"route": "double-slash"} + assert paths and all(path.startswith("/v1//agents") for path in paths), paths + # HP-23: the SDK exposes X-Request-Id on models and errors. + assert REQUEST_ID.fullmatch(agent._request_id) and REQUEST_ID.fullmatch(updated._request_id) + assert agent._request_id != updated._request_id + with OpenAI(api_key="invalid-routing-key", base_url=base + "/v1", max_retries=0, + http_client=httpx2.Client(trust_env=False, timeout=10)) as invalid: + try: + invalid.beta.agents.retrieve(agent.id) + except AuthenticationError as error: + assert REQUEST_ID.fullmatch(error.request_id) + assert error.body["type"] == "invalid_request_error" and error.body["code"] is None + else: + raise AssertionError("An invalid key was accepted") + + headers = {"Authorization": "Bearer " + token, "OpenAI-Beta": "agents=v1"} + resource = base + "/v1/agents/" + agent.id + with httpx2.Client(trust_env=False, timeout=10) as raw: + expected = raw.get(resource, headers=headers).json() + for url in (base + "/v1/agents/x/../" + agent.id, base + "/v1/agents/" + agent.id.replace("-", "%2D")): + response = raw.get(url, headers=headers) + assert response.status_code == 200 and response.json() == expected, url + # HP-19: HEAD answers GET routes without a body. + head = raw.head(resource, headers=headers) + assert head.status_code == 200 and head.content == b"" and int(head.headers["content-length"]) > 0 + # HP-20: 405 keeps Core's body and lists the route's methods. + response = raw.put(resource, headers=headers, json={}) + assert response.status_code == 405 and response.headers["allow"] == "GET,HEAD,POST,DELETE" + assert response.json()["error"]["code"] == "unsupported_operation" + # HP-05/HP-02/HP-07: Beta first, then the invalid_request_error 401. + response = raw.get(resource) + assert response.status_code == 400 and response.json()["error"] == { + "type": "invalid_beta", "code": "invalid_beta", "param": None, + "message": "To access the Agents API, set the 'OpenAI-Beta' header to 'agents=v1'."} + response = raw.get(resource, headers=[("OpenAI-Beta", "agents=v1"), ("OpenAI-Beta", "assistants=v2"), + ("Authorization", "Bearer " + token)]) + assert response.status_code == 400 and response.json()["error"]["code"] == "invalid_beta" + unauthorized = raw.get(resource, headers={"OpenAI-Beta": "agents=v1"}) + assert unauthorized.status_code == 401 and unauthorized.json()["error"]["code"] is None + files = raw.get(base + "/v1/files", headers={"Authorization": "Bearer invalid-routing-key"}) + assert files.status_code == 401 and files.json()["error"]["code"] == "invalid_api_key" + # HP-23/HP-24: every response carries a fresh ID and the OpenAI headers. + ids = set() + for response in (head, unauthorized, files, raw.put(resource, headers=headers, json={})): + assert REQUEST_ID.fullmatch(response.headers["x-request-id"]) and response.headers["x-request-id"] not in ids + ids.add(response.headers["x-request-id"]) + assert response.headers["openai-version"] == "2020-10-01" + assert int(response.headers["openai-processing-ms"]) >= 0 + assert response.headers["x-content-type-options"] == "nosniff" + assert raw.delete(resource, headers=headers).status_code == 200 + print("HTTP routing: pinned SDK update through a non-canonical path, request IDs, HEAD, Allow, " + "Beta-before-authentication and 401 envelopes passed.") diff --git a/services/agents-api/tests/official_vault_delete.py b/services/agents-api/tests/official_vault_delete.py index 8bede45b6..b69154cfe 100644 --- a/services/agents-api/tests/official_vault_delete.py +++ b/services/agents-api/tests/official_vault_delete.py @@ -26,7 +26,9 @@ def verify_vault_deletion(client, other, invalid, peer, canary, expect_error): auth_headers = {"Authorization": f"Bearer {client.api_key}", "OpenAI-Beta": "agents=v1"} endpoint = str(client.base_url).rstrip("/") + "/vaults/" with httpx2.Client(trust_env=False, timeout=10) as raw: - assert raw.delete(endpoint + target.id).status_code == 401 + # The Beta header is checked before authentication (HP-05). + assert raw.delete(endpoint + target.id).status_code == 400 + assert raw.delete(endpoint + target.id, headers={"OpenAI-Beta": "agents=v1"}).status_code == 401 response = raw.delete(endpoint + target.id, headers={"Authorization": auth_headers["Authorization"]}) assert response.status_code == 400 and response.json()["error"]["code"] == "invalid_beta" # The body rejects; the unknown include key is ignored. diff --git a/services/agents-api/tests/official_vault_list.py b/services/agents-api/tests/official_vault_list.py index 17ab9e6f6..05035af45 100644 --- a/services/agents-api/tests/official_vault_list.py +++ b/services/agents-api/tests/official_vault_list.py @@ -111,7 +111,9 @@ def sdk_page(values, has_more, query, **request): response = raw.get(endpoint, headers=headers, params=params) assert response.status_code == 400 assert response.json()["error"]["type"] == "invalid_request_error" - assert raw.get(endpoint).status_code == 401 + # The Beta header is checked before authentication (HP-05). + assert raw.get(endpoint).status_code == 400 + assert raw.get(endpoint, headers={"OpenAI-Beta": "agents=v1"}).status_code == 401 for beta in (None, "agents=v2"): auth = {"Authorization": headers["Authorization"]} if beta is not None: diff --git a/services/agents-api/tests/official_vaults.py b/services/agents-api/tests/official_vaults.py index ee28c8089..781901be1 100644 --- a/services/agents-api/tests/official_vaults.py +++ b/services/agents-api/tests/official_vaults.py @@ -96,7 +96,10 @@ def verify_vaults(client, other, invalid, peer, binding, expect_error): expect_error(AuthenticationError, lambda: invalid.beta.agents.vaults.create()) expect_error(AuthenticationError, lambda: invalid.beta.agents.vaults.retrieve(saved[0].id)) for suffix, method in (("", "POST"), ("/" + saved[0].id, "GET")): - assert raw.request(method, base + suffix, json={} if method == "POST" else None).status_code == 401 + # The Beta header is checked before authentication (HP-05). + assert raw.request(method, base + suffix, json={} if method == "POST" else None).status_code == 400 + assert raw.request(method, base + suffix, headers={"OpenAI-Beta": "agents=v1"}, + json={} if method == "POST" else None).status_code == 401 for beta in (None, "agents=v2"): request_headers = {"Authorization": headers["Authorization"]} if beta is not None: From 7dcbbc7a6480876711ed87f35ce0aaa97d1ddfe0 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 21:23:00 +0000 Subject: [PATCH 06/12] Canonicalize the request's own path spelling When the raw path held a byte net/url considers invalid, such as '{', '"' or non-ASCII, EscapedPath re-escaped the decoded Path, so %2F became a separator and ..%2F a dot segment: /v1/x{/..%2F..%2Fcore/v1/sandbox/nodes reached sandbox administration, and the same trick reached daemon enrollment and node transport. When nothing was rewritten, chi kept the original RawPath while the ServeMux used the re-escaped form. Build the canonical path from RawPath (or the default encoding when Go kept none), percent-encode invalid bytes, decode only unreserved escapes and set Path and RawPath consistently, so chi, the ServeMux and every middleware route on one string and %2F or %5C never separate segments. Raw request-line tests over real listeners and two differential fuzz targets cover both server configurations. --- .../agents-api/cmd/server/http_routes_test.go | 126 ++++++++++++++ services/agents-api/internal/api/routing.go | 54 ++++-- .../agents-api/internal/api/routing_test.go | 158 ++++++++++++++++++ 3 files changed, 325 insertions(+), 13 deletions(-) diff --git a/services/agents-api/cmd/server/http_routes_test.go b/services/agents-api/cmd/server/http_routes_test.go index e1cad8288..77529e341 100644 --- a/services/agents-api/cmd/server/http_routes_test.go +++ b/services/agents-api/cmd/server/http_routes_test.go @@ -1,11 +1,20 @@ package main import ( + "bufio" + "fmt" "io" + "net" "net/http" "net/http/httptest" + "strconv" "strings" "testing" + + "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/device" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/api" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" + "github.com/google/uuid" ) // The daemon-enabled configuration canonicalizes paths before the ServeMux: @@ -51,3 +60,120 @@ func TestServerHandlerRoutesCanonicalPaths(t *testing.T) { } } } + +// trapStore panics on every store call, marking a request that reached a handler. +type trapStore struct{ api.ResourceStore } + +// daemonComposition serves the real API handler beside sentinel daemon routes. +func daemonComposition(t testing.TB) http.Handler { + t.Helper() + auth, err := api.NewAuthenticator([]api.APIKey{{OrganizationID: "org", ProjectID: "project", SubjectKind: "service_account", + SubjectID: "runner", TokenSHA256: device.HashCredential("project-key"), TenantID: uuid.NewString()}}) + if err != nil { + t.Fatal(err) + } + admin, err := api.NewDeploymentAuthenticator([]string{device.HashCredential("admin-key")}) + if err != nil { + t.Fatal(err) + } + apiHandler, err := api.NewHandler(trapStore{}, auth, "codex", api.WithSandboxManager(&store.Store{}, admin)) + if err != nil { + t.Fatal(err) + } + sentinel := func(route string) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Sentinel", route+" "+r.URL.Path+" "+r.URL.RawPath) + w.WriteHeader(http.StatusNoContent) + }) + } + return serverHandler(apiHandler, &daemonRoutes{gateway: sentinel("gateway"), enrollment: sentinel("enrollment"), + connection: sentinel("connection"), nodeConnect: sentinel("node")}) +} + +func parseRaw(target string) (*http.Request, error) { + return http.ReadRequest(bufio.NewReader(strings.NewReader("GET " + target + " HTTP/1.1\r\nHost: example.test\r\n\r\n"))) +} + +func outcome(handler http.Handler, request *http.Request) (result string) { + defer func() { + if recover() != nil { + result = "handler reached" + } + }() + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + return fmt.Sprintf("%d %q %q %q %q", response.Code, response.Header().Get("X-Sentinel"), response.Body.String(), response.Header().Get("Allow"), response.Header().Get("Location")) +} + +// canonical returns the escaped path the composition routes a request on. +func canonical(request *http.Request) string { + var seen string + api.CanonicalPaths(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { seen = r.URL.EscapedPath() })).ServeHTTP(httptest.NewRecorder(), request) + return seen +} + +// In the daemon-enabled configuration, a raw path with bytes that are invalid +// in an escaped path cannot turn %2F into a separator to reach daemon, node or +// sandbox administration routes; it reaches what its canonical form reaches. +func TestServerHandlerRawPathsKeepEncodedSeparators(t *testing.T) { + handler := daemonComposition(t) + server := httptest.NewServer(handler) + defer server.Close() + for _, test := range []struct{ target, want string }{ + {"/v1/x{/..%2F..%2Fapi/v1/agent-daemon/enroll", "400"}, + {"/v1/x\"/..%2f..%2fapi/v1/agent-daemon/connection", "400"}, + {"/v1/\xc3\xa9/..%2F..%2Fcore/v1/sandbox/node/connect", "400"}, + {"/v1/x{/..%2F..%2Fcore/v1/sandbox/nodes", "400"}, + {"/v1/x\\/..%5C..%5Capi/v1/agent-daemon/ws", "400"}, + {"http://example.test/v1/x{/..%252F..%252Fapi/v1/agent-daemon/enroll", "400"}, + {"/v1/x{/../../api/v1/agent-daemon/enroll", "204"}, + {"/v1/x{/%2E%2E/%2E%2E/core/v1/sandbox/node/connect", "204"}, + } { + request, err := parseRaw(test.target) + if err != nil { + t.Fatal(err) + } + again, _ := parseRaw(canonical(request)) + got, want := outcome(handler, request), outcome(handler, again) + if got != want || !strings.HasPrefix(got, test.want+" ") { + t.Errorf("%s = %s; canonical form gives %s", test.target, got, want) + } + connection, err := net.Dial("tcp", server.Listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + _, _ = io.WriteString(connection, "GET "+test.target+" HTTP/1.1\r\nHost: example.test\r\nConnection: close\r\n\r\n") + response, err := http.ReadResponse(bufio.NewReader(connection), nil) + if err != nil || strconv.Itoa(response.StatusCode) != test.want { + t.Errorf("raw %s = %v %v", test.target, response, err) + } + _ = connection.Close() + } +} + +// Differential property for the daemon-enabled configuration: any request path +// reaches the same handler, with the same path, as its canonical form. +func FuzzServerHandlerRoutesLikeCanonicalForm(f *testing.F) { + for _, seed := range []string{"v1//agents", "v1/x{/..%2F..%2Fapi/v1/agent-daemon/enroll", "api/v1/agent-daemon%2Fenroll", + "v1/\xc3\xa9/../../api/v1/agent-daemon/ws", "core/v1/sandbox/node/%2E%2E/node/connect", "0\"%2F", "api/v1/agent-daemon", + "v1/x\\/..%5C..%5Capi/v1/agent-daemon/connection", "v1/agents/%252F%2e%2E/x"} { + f.Add(seed) + } + handler := daemonComposition(f) + f.Fuzz(func(t *testing.T, path string) { + if strings.ContainsAny(path, " ?#") || len(path) > 512 { + t.Skip() + } + request, err := parseRaw("/" + path) + if err != nil { + t.Skip() + } + again, err := parseRaw(canonical(request)) + if err != nil { + t.Fatal(err) + } + if got, want := outcome(handler, request), outcome(handler, again); got != want { + t.Fatalf("%q = %s; canonical %q gives %s", path, got, again.RequestURI, want) + } + }) +} diff --git a/services/agents-api/internal/api/routing.go b/services/agents-api/internal/api/routing.go index 76286bb94..ca46aedaf 100644 --- a/services/agents-api/internal/api/routing.go +++ b/services/agents-api/internal/api/routing.go @@ -3,6 +3,7 @@ package api import ( "crypto/rand" "encoding/hex" + "fmt" "net/http" "net/url" "path" @@ -18,27 +19,39 @@ import ( // service does (HP-17/HP-18), instead of the ServeMux redirect, which made // clients resend a POST as a GET. It must wrap the complete server handler so // that every routing, authentication and middleware decision sees only the -// rewritten path. Percent-encoded unreserved characters (RFC 3986 section 2.3) -// are decoded first, so an encoded dot segment is resolved like a literal one. -// Then empty and dot segments are resolved with ServeMux cleanPath semantics, -// keeping a trailing slash. Other escapes, such as %2F, stay encoded and never -// become separators, so every request reaches exactly the route and -// authentication of its canonical path written literally. It is idempotent. +// rewritten path. The canonical path is built from the raw request path: bytes +// that are not valid in an escaped path are percent-encoded, percent-encoded +// unreserved characters (RFC 3986 section 2.3) are decoded, so an encoded dot +// segment resolves like a literal one, and empty and dot segments are resolved +// with ServeMux cleanPath semantics, keeping a trailing slash. Other escapes, +// such as %2F and %5C, stay encoded and never become separators. Path and +// RawPath are then set consistently, so chi (which prefers RawPath), the +// ServeMux (which uses EscapedPath) and every middleware see the same path, and +// every spelling reaches exactly the route and authentication of its canonical +// form written literally. It is idempotent. func CanonicalPaths(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - escaped := cleanPath(decodeUnreserved(r.URL.EscapedPath())) + // RawPath is the request's own spelling whenever it differs from the + // default encoding of Path; EscapedPath would re-escape the decoded Path + // when RawPath holds an invalid byte, turning %2F into a separator. + raw := r.URL.RawPath + if raw == "" { + raw = r.URL.EscapedPath() + } + escaped := cleanPath(decodeUnreserved(escapeInvalid(raw))) decoded, err := url.PathUnescape(escaped) if err != nil { - // EscapedPath is always validly escaped; this is unreachable. + // A parsed request path has only valid escapes; this is unreachable. http.Error(w, "Invalid request path.", http.StatusBadRequest) return } - if decoded != r.URL.Path || escaped != r.URL.EscapedPath() { + rawPath := "" + if (&url.URL{Path: decoded}).EscapedPath() != escaped { + rawPath = escaped + } + if decoded != r.URL.Path || rawPath != r.URL.RawPath { canonical := *r.URL - canonical.Path, canonical.RawPath = decoded, "" - if canonical.EscapedPath() != escaped { - canonical.RawPath = escaped - } + canonical.Path, canonical.RawPath = decoded, rawPath r = r.WithContext(r.Context()) r.URL = &canonical } @@ -46,6 +59,21 @@ func CanonicalPaths(next http.Handler) http.Handler { }) } +// escapeInvalid percent-encodes every byte that may not appear literally in an +// escaped path (RFC 3986 pchar, plus the '[' and ']' that net/url accepts), such +// as '{', '"', a backslash, spaces and non-ASCII bytes. Existing escapes are kept. +func escapeInvalid(raw string) string { + var escaped strings.Builder + for i := 0; i < len(raw); i++ { + if c := raw[i]; unreserved(c) || strings.IndexByte("!$&'()*+,;=:@/%[]", c) >= 0 { + escaped.WriteByte(c) + } else { + fmt.Fprintf(&escaped, "%%%02X", c) + } + } + return escaped.String() +} + // decodeUnreserved decodes percent-encoded ALPHA, DIGIT, '-', '.', '_' and '~', // which RFC 3986 treats as equivalent to their literal form. func decodeUnreserved(escaped string) string { diff --git a/services/agents-api/internal/api/routing_test.go b/services/agents-api/internal/api/routing_test.go index 95d1fd104..7a90a23ff 100644 --- a/services/agents-api/internal/api/routing_test.go +++ b/services/agents-api/internal/api/routing_test.go @@ -1,11 +1,15 @@ package api import ( + "bufio" "context" "encoding/json" + "fmt" "io" + "net" "net/http" "net/http/httptest" + "net/url" "regexp" "strconv" "strings" @@ -560,3 +564,157 @@ func TestAgentsResponseHeaders(t *testing.T) { t.Fatalf("stream headers %v, logged %q", response.Header, logged) } } + +// parseRaw parses a request line exactly as net/http's server does. +func parseRaw(method, target string, header http.Header) (*http.Request, error) { + request, err := http.ReadRequest(bufio.NewReader(strings.NewReader(method + " " + target + " HTTP/1.1\r\nHost: example.test\r\n\r\n"))) + if err != nil { + return nil, err + } + for name, values := range header { + request.Header[name] = values + } + return request, nil +} + +// canonicalTarget returns the path CanonicalPaths routes a request on, after +// checking that chi (RawPath, else Path), the ServeMux (EscapedPath) and the +// decoded Path agree, that the path is clean and that a second pass keeps it. +func canonicalTarget(t *testing.T, request *http.Request) string { + t.Helper() + var seen *url.URL + CanonicalPaths(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { seen = r.URL })).ServeHTTP(httptest.NewRecorder(), request) + escaped := seen.EscapedPath() + decoded, err := url.PathUnescape(escaped) + if err != nil || decoded != seen.Path || (seen.RawPath != "" && seen.RawPath != escaped) || cleanPath(escaped) != escaped { + t.Fatalf("%s: inconsistent canonical URL path %q raw %q escaped %q", request.RequestURI, seen.Path, seen.RawPath, escaped) + } + again, err := parseRaw(http.MethodGet, escaped, nil) + if err != nil { + t.Fatalf("%s: canonical path %q does not parse: %v", request.RequestURI, escaped, err) + } + var repeated *url.URL + CanonicalPaths(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { repeated = r.URL })).ServeHTTP(httptest.NewRecorder(), again) + if repeated.EscapedPath() != escaped || repeated.RawPath != seen.RawPath || repeated.Path != seen.Path { + t.Fatalf("%s: canonicalization is not idempotent: %q", request.RequestURI, repeated.EscapedPath()) + } + return escaped +} + +// outcome summarizes a response; a panic means a trap store was reached. +func outcome(handler http.Handler, request *http.Request) (result string) { + defer func() { + if recover() != nil { + result = "handler reached" + } + }() + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + return fmt.Sprintf("%d %q allow=%q www-authenticate=%q", response.Code, response.Body.String(), response.Header().Get("Allow"), response.Header().Get("WWW-Authenticate")) +} + +// Every spelling of a request reaches the route group and authentication of +// its canonical form. Bytes that are invalid in an escaped path must not make +// %2F or %5C a separator (the ServeMux and EscapedPath re-escape the decoded +// Path in that case), in any case of hex digit or under double encoding. +func TestRawPathsReachTheirCanonicalRouteGroup(t *testing.T) { + handler, _, _ := routingFixture(t) + for _, test := range []struct { + target, canonical string + status int + }{ + {"/v1/x{/..%2F..%2Fcore/v1/sandbox/nodes", "/v1/x%7B/..%2F..%2Fcore/v1/sandbox/nodes", http.StatusBadRequest}, + {"/v1/x%7B/..%2F..%2Fcore/v1/sandbox/nodes", "/v1/x%7B/..%2F..%2Fcore/v1/sandbox/nodes", http.StatusBadRequest}, + {"/v1/x\"/..%2f..%2fcore/v1/sandbox/nodes", "/v1/x%22/..%2f..%2fcore/v1/sandbox/nodes", http.StatusBadRequest}, + {"/v1/x\\/..%5C..%5Ccore/v1/sandbox/nodes", "/v1/x%5C/..%5C..%5Ccore/v1/sandbox/nodes", http.StatusBadRequest}, + {"/v1/\xc3\xa9/..%2F..%2Fcore/v1/sandbox/nodes", "/v1/%C3%A9/..%2F..%2Fcore/v1/sandbox/nodes", http.StatusBadRequest}, + {"/v1/x|^`<>/..%252F..%252Fcore/v1/sandbox/nodes", "/v1/x%7C%5E%60%3C%3E/..%252F..%252Fcore/v1/sandbox/nodes", http.StatusBadRequest}, + {"http://example.test/v1/x{/..%2F..%2Fcore/v1/sandbox/nodes", "/v1/x%7B/..%2F..%2Fcore/v1/sandbox/nodes", http.StatusBadRequest}, + {"/0\"%2F", "/0%22%2F", http.StatusNotFound}, + {"/core/v1/sandbox/nodes{%2F..%2F..%2F..%2Fv1/agents", "/core/v1/sandbox/nodes%7B%2F..%2F..%2F..%2Fv1/agents", http.StatusUnauthorized}, + // Literal and unreserved-encoded dot segments do resolve. + {"/v1/x{/../../core/v1/sandbox/nodes", "/core/v1/sandbox/nodes", http.StatusUnauthorized}, + {"/v1/x{/%2E%2E/%2e%2E/core/v1/sandbox/nodes", "/core/v1/sandbox/nodes", http.StatusUnauthorized}, + {"http://example.test//v1/x{/..//../core/./v1/sandbox/nodes", "/core/v1/sandbox/nodes", http.StatusUnauthorized}, + } { + request, err := parseRaw(http.MethodGet, test.target, nil) + if err != nil { + t.Fatalf("%s: %v", test.target, err) + } + if got := canonicalTarget(t, request); got != test.canonical { + t.Errorf("%s: canonical %q, want %q", test.target, got, test.canonical) + } + canonical, _ := parseRaw(http.MethodGet, test.canonical, nil) + want := outcome(handler, canonical) + if got := outcome(handler, request); got != want || !strings.HasPrefix(got, strconv.Itoa(test.status)+" ") { + t.Errorf("%s = %s; canonical form gives %s", test.target, got, want) + } + } + // The same through a real listener, which reads the request line itself. + server := httptest.NewServer(handler) + defer server.Close() + for target, status := range map[string]string{ + "/v1/x{/..%2F..%2Fcore/v1/sandbox/nodes": "400", + "/v1/\xe2\x98\x83/..%2F..%2Fcore/v1/sandbox/deployment": "400", + "/v1/x{/../../core/v1/sandbox/nodes": "401", + } { + if got := rawStatus(t, server.Listener.Addr().String(), target); got != status { + t.Errorf("raw %q = %s, want %s", target, got, status) + } + } +} + +// rawStatus sends one request line over TCP and returns the response status code. +func rawStatus(t *testing.T, address, target string) string { + t.Helper() + connection, err := net.Dial("tcp", address) + if err != nil { + t.Fatal(err) + } + defer connection.Close() + if _, err := io.WriteString(connection, "GET "+target+" HTTP/1.1\r\nHost: example.test\r\nConnection: close\r\n\r\n"); err != nil { + t.Fatal(err) + } + response, err := http.ReadResponse(bufio.NewReader(connection), nil) + if err != nil { + t.Fatal(err) + } + _ = response.Body.Close() + return strconv.Itoa(response.StatusCode) +} + +var canonicalPathSeeds = []string{ + "v1/agents", "v1//agents/a", "v1/x{/..%2F..%2Fcore/v1/sandbox/nodes", "v1/x\"/..%2f..%2fcore/v1/sandbox/deployment", + "v1/x\\/..%5C..%5Ccore/v1/sandbox/nodes", "v1/\xc3\xa9/../../core/v1/sandbox/nodes", "v1/%2E%2E/core/v1/sandbox/nodes", + "v1/files/..%2F..%2Fv1/skills", "0\"%2F", "core/v1/sandbox/nodes{%2F..%2F..%2F..%2Fv1/agents", "v1/agents/%252F..", + "api/v1/agent-daemon/%2E%2E/%2E%2E/%2E%2E/v1/agents", "v1/x{/%2e./core/v1/sandbox/node/identity", "v1/agents/%7E%5F%2D%41", + "core/v1/environments/x/executor-credentials/%2E%2E/%2E%2E/%2E%2E/%2E%2E/core/v1/sandbox/nodes", +} + +// Differential property over arbitrary request paths: the routed outcome for +// no credentials, the Beta header and the administrator key equals that of +// the canonical form, and every layer sees one consistent canonical path. +func FuzzCanonicalPathsRouteLikeTheirCanonicalForm(f *testing.F) { + for _, seed := range canonicalPathSeeds { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, path string) { + if strings.ContainsAny(path, " ?#") || len(path) > 512 { + t.Skip() + } + handler, _, _ := routingFixture(t) + for _, header := range []http.Header{{}, withHeaders(beta), withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta)} { + request, err := parseRaw(http.MethodGet, "/"+path, header) + if err != nil { + t.Skip() + } + canonical, err := parseRaw(http.MethodGet, canonicalTarget(t, request), header) + if err != nil { + t.Fatal(err) + } + if got, want := outcome(handler, request), outcome(handler, canonical); got != want { + t.Fatalf("%q = %s; canonical %q gives %s", path, got, canonical.RequestURI, want) + } + } + }) +} From 8d3d63a03f49f4eef9f32af452873cc6aa7dd15f Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 21:23:28 +0000 Subject: [PATCH 07/12] Exclude HEAD on the live directory list and use the JSON 405 everywhere HEAD on the Environment Files list would run the live Worker directory read; it now answers 405 like the stream and content routes. The root router uses the JSON 405 with Allow for routes outside the Beta group and for unknown methods, and Allow resolves a mounted router's own root route, which chi's Find does not descend into. --- services/agents-api/internal/api/handler.go | 10 ++++--- services/agents-api/internal/api/routing.go | 15 ++++++++++- .../agents-api/internal/api/routing_test.go | 26 +++++++++++++++++-- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/services/agents-api/internal/api/handler.go b/services/agents-api/internal/api/handler.go index dcc5e222c..34e08dd1c 100644 --- a/services/agents-api/internal/api/handler.go +++ b/services/agents-api/internal/api/handler.go @@ -80,12 +80,15 @@ func NewHandler(s ResourceStore, auth *Authenticator, engine string, options ... } // routes builds the router. HEAD runs the GET route without a body after the -// same authentication and Beta checks (HP-19). Routes that stream events or -// download content register an explicit HEAD 405 instead, so HEAD never holds -// a stream open or reads full content. +// same authentication and Beta checks (HP-19). Routes that stream events, +// download content or read a live workspace directory register an explicit +// HEAD 405 instead, so HEAD never holds a stream open, reads full content or +// waits on a Runtime. Every 405, including unknown methods and routes outside +// the Beta group, has the JSON body and Allow header. func (h *Handler) routes() *chi.Mux { router := chi.NewRouter() router.Use(agentsResponseHeaders, log.HTTPMiddleware, middleware.GetHead) + router.MethodNotAllowed(methodNotAllowed) router.Get("/healthz", func(w http.ResponseWriter, _ *http.Request) { writeJSON(w, http.StatusOK, map[string]string{"status": "ok"}) }) @@ -127,6 +130,7 @@ func (h *Handler) routes() *chi.Mux { r.Delete("/agents/environments/templates/{environment_template_id}", h.deleteEnvironmentTemplate) r.Get("/agents/environments/{environment_id}", h.getEnvironment) r.Get("/agents/environments/{environment_id}/files", h.listEnvironmentFiles) + r.Head("/agents/environments/{environment_id}/files", methodNotAllowed) r.Post("/agents/environments/{environment_id}/files", h.createEnvironmentFile) r.Post("/agents/sessions", h.createSession) r.Get("/agents/sessions", h.listSessions) diff --git a/services/agents-api/internal/api/routing.go b/services/agents-api/internal/api/routing.go index ca46aedaf..fec1eebed 100644 --- a/services/agents-api/internal/api/routing.go +++ b/services/agents-api/internal/api/routing.go @@ -198,7 +198,20 @@ func allowedMethods(r *http.Request) string { if routePath == "" { routePath = r.URL.Path } - routed := func(method string) bool { return rctx.Routes.Match(chi.NewRouteContext(), method, routePath) } + routed := func(method string) bool { + pattern := rctx.Routes.Find(chi.NewRouteContext(), method, routePath) + if pattern == "" { + return false + } + // chi's Find stops at the exact path of a mounted router, which serves + // that path as its own "/" route. + for _, route := range rctx.Routes.Routes() { + if route.SubRoutes != nil && (pattern == strings.TrimSuffix(route.Pattern, "/*") || pattern == strings.TrimSuffix(route.Pattern, "*")) { + return route.SubRoutes.Find(chi.NewRouteContext(), method, "/") != "" + } + } + return true + } var allowed []string if routed(http.MethodGet) { allowed = append(allowed, http.MethodGet) diff --git a/services/agents-api/internal/api/routing_test.go b/services/agents-api/internal/api/routing_test.go index 7a90a23ff..a8f6f4c0d 100644 --- a/services/agents-api/internal/api/routing_test.go +++ b/services/agents-api/internal/api/routing_test.go @@ -439,6 +439,26 @@ func TestMethodNotAllowedListsRouteMethods(t *testing.T) { t.Errorf("%s %s = %d %q %s", test.method, test.path, got.Code, got.Header().Get("Allow"), got.Body) } } + // Routes outside the Beta group and unknown methods use the same 405. + executor := "/core/v1/environments/" + uuid.NewString() + "/executor-credentials" + for _, test := range []struct { + method, path, allow string + header http.Header + }{ + {http.MethodPost, "/healthz", "GET,HEAD", nil}, + {http.MethodPut, executor, "POST", withHeaders(project)}, + {http.MethodGet, executor + "/" + uuid.NewString(), "DELETE", withHeaders(project)}, + {"FOO", "/v1/agents", "GET,HEAD,POST", nil}, + {"FOO", "/v1//agents/" + s.agent.ID, "GET,HEAD,POST,DELETE", nil}, + } { + got := serve(handler, test.method, test.path, "", test.header) + if got.Code != http.StatusMethodNotAllowed || got.Body.String() != notAllowedV1 || got.Header().Get("Allow") != test.allow { + t.Errorf("%s %s = %d %q %s", test.method, test.path, got.Code, got.Header().Get("Allow"), got.Body) + } + } + if got := serve(handler, http.MethodPut, executor, "", nil); got.Code != http.StatusUnauthorized { + t.Fatalf("unauthenticated executor 405 = %d %s", got.Code, got.Body) + } if got := serve(handler, http.MethodPut, "/v1/agents/"+s.agent.ID, "", withHeaders(beta)); got.Code != http.StatusUnauthorized || got.Header().Get("Allow") != "" { t.Fatalf("unauthenticated 405 = %d %s", got.Code, got.Body) } @@ -447,8 +467,9 @@ func TestMethodNotAllowedListsRouteMethods(t *testing.T) { } } -// HEAD runs a GET route without a body after the same checks (HP-19). SSE and -// content-download routes answer 405 without opening a stream or reading content. +// HEAD runs a GET route without a body after the same checks (HP-19). SSE, +// content-download and live directory routes answer 405 without opening a +// stream, reading content or waiting on a Runtime. func TestHeadRequests(t *testing.T) { handler, _, s := routingFixture(t) server := httptest.NewServer(handler) @@ -488,6 +509,7 @@ func TestHeadRequests(t *testing.T) { session := "/v1/agents/sessions/" + uuid.NewString() for _, test := range []struct{ path, allow string }{ {session + "/events", "GET,POST"}, + {"/v1/agents/environments/" + uuid.NewString() + "/files", "GET,POST"}, {session + "/artifacts/" + uuid.NewString() + "/content", "GET"}, {"/v1/files/file-missing/content", "GET"}, {"/v1/skills/skill-missing/content", "GET"}, From 8ec20a4a9748612e07be5adabcb40e674d4d0069 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 21:23:28 +0000 Subject: [PATCH 08/12] Document raw-path canonicalization and review corrections Describe the canonical path source and the invalid-byte case, the HEAD exclusion of the directory list, Content-Length only for buffered HEAD bodies, the JSON 405 outside the Beta group, 401 types limited to the Agents API handler, the remaining daemon-prefix redirect, and the invalid_beta code in the Web guide. --- CONTRIBUTING.md | 16 +++++---- contracts/agents-api/README.md | 2 +- .../official-semantics-alignment.md | 34 +++++++++++++------ docs/web/core-connection.md | 4 +-- services/agents-api/cmd/server/http_routes.go | 5 +-- 5 files changed, 39 insertions(+), 22 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3a8fffee6..81472fe29 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -111,18 +111,22 @@ PostgreSQL error mapping, so keep each request's writes in one transaction. See Serve requests on their canonical path and never redirect. `api.CanonicalPaths` wraps the complete server handler in both configurations (the daemon ServeMux and the API router alone), so every route group, middleware, authentication check and -handler sees one path: unreserved escapes decoded, empty and dot segments resolved -with ServeMux semantics, trailing slash kept, other escapes such as `%2F` left -encoded. Do not route or authorize on a path outside that wrapper. On the Beta +handler sees one path. It starts from the request's own spelling, never a path +re-escaped from its decoded form: invalid bytes are percent-encoded, unreserved +escapes decoded, empty and dot segments resolved with ServeMux semantics, the +trailing slash kept, and other escapes such as `%2F` and `%5C` left encoded; +`Path` and `RawPath` are set consistently for chi and the ServeMux. Do not route +or authorize on a path outside that wrapper. On the Beta group the constant OpenAI-Beta check (exactly one `agents=v1` value) runs before authentication, and authentication still precedes every Beta handler, 404 and 405. -Every 401 has type `invalid_request_error`: null code on Beta routes; on Files, +Every Agents API 401 has type `invalid_request_error`: null code on Beta routes; on Files, Skills and Core project extensions `invalid_api_key` only for a rejected Bearer credential. Agents API responses carry a fresh `X-Request-Id` (also in the log context), `OpenAI-Version`, `OpenAI-Processing-Ms` and nosniff through the API router's own middleware, not the shared log middleware. HEAD runs GET routes; -streaming and content-download routes register an explicit HEAD 405 instead. A 405 -lists the route's methods in `Allow`. +streaming, content-download and live directory routes register an explicit HEAD +405 instead. Every 405 of the API router, unknown methods included, has the JSON +body and lists the route's methods in `Allow`. Keep runtime state, test artifacts and build output under `~/.parsar/`. Require absolute user-supplied working directories. Keep credentials out of source and diff --git a/contracts/agents-api/README.md b/contracts/agents-api/README.md index 298fd96cd..aaba9946c 100644 --- a/contracts/agents-api/README.md +++ b/contracts/agents-api/README.md @@ -877,7 +877,7 @@ Caller keys now resolve an explicitly configured organization/project and typed user/service-account identity. An immutable project-to-tenant mapping is verified against PostgreSQL before startup. Optional official organization/project headers must match the key's authorized scope; ambiguous or conflicting headers use the -existing 401 response. Every 401 has type `invalid_request_error`, as observed +existing 401 response. Every Agents API 401 has type `invalid_request_error`, as observed officially; Beta routes report a null code, while Files, Skills and Core project extensions report `invalid_api_key` for a rejected Bearer credential and a null code without one. This scope-header policy is an implementation choice, not diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index 3520e7883..c4e91b021 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -806,9 +806,9 @@ ledger records. | Row | Case | Core behavior | | --- | --- | --- | | RH1 | `//`, `.` or `..` path segments (HP-17: `R10`, `R11`, `R17`, `R18`) | Served on the canonical path, never redirected. Empty and dot segments resolve with ServeMux semantics and a trailing slash is kept, so `/v1/agents/x/../` still reaches the trailing-slash 404. The former 301 made the pinned SDK resend an update as a GET and drop it. | -| RH2 | A percent-encoded unreserved character in the path (HP-18: `R12`) | Decoded before routing, including `%2E` dot segments. Other escapes, such as `%2F`, stay encoded and never separate segments. Malformed, missing and foreign IDs keep the single 404. | -| RH3 | HEAD on a GET route (HP-19: `R13`, `R16`) | The GET route runs after the same Beta and authentication checks; 200 with its headers and `Content-Length`, no body. The events stream and the File, Skill, Skill version and Artifact content downloads answer HEAD with Core's 405 instead, so HEAD never holds a stream open or reads content. That exclusion is a documented Core difference; official HEAD on those routes is unobserved. | -| RH4 | Unsupported method (HP-20: `R04`–`R06`) | Unchanged 405 JSON `unsupported_operation`, now with `Allow` listing the route's methods in the observed order, such as `GET,HEAD,POST,DELETE`. | +| RH2 | A percent-encoded unreserved character in the path (HP-18: `R12`) | Decoded before routing, including `%2E` dot segments. The canonical path is built from the request's own path spelling; bytes that are invalid in an escaped path (such as `{`, `"`, a backslash or non-ASCII) are percent-encoded first. Other escapes, such as `%2F`, `%2f`, `%5C` and double encodings, stay encoded and never separate segments. Malformed, missing and foreign IDs keep the single 404. | +| RH3 | HEAD on a GET route (HP-19: `R13`, `R16`) | The GET route runs after the same Beta and authentication checks; 200 with its headers and no body. `Content-Length` is present when Go buffers the whole body (about 2 KiB) and omitted for larger responses. The events stream, the File, Skill, Skill version and Artifact content downloads and the live Environment Files directory list answer HEAD with Core's 405 instead, so HEAD never holds a stream open, reads content or waits on a Runtime. That exclusion is a documented Core difference; official HEAD on those routes is unobserved. | +| RH4 | Unsupported method (HP-20: `R04`–`R06`) | Unchanged 405 JSON `unsupported_operation`, now with `Allow` listing the route's methods in the observed order, such as `GET,HEAD,POST,DELETE`. Routes outside the Beta group, such as `/healthz` and executor credentials, and methods chi does not know, such as `FOO`, now use the same JSON 405 instead of chi's empty one; an unknown method is answered before the Beta and authentication checks, as before. | | RH5 | `X-Request-Id` (HP-23) | Every response of the Agents API handler, including 400, 401, 404, 405 and SSE streams, carries a fresh random `req_` plus 32 lowercase hex characters. The ID is attached to the request log context as `request_id` next to the trace carrier. | | RH6 | No or invalid credentials without OpenAI-Beta on a Beta route (HP-05: `A06`, `A11`) | 400 `invalid_beta`: the constant Beta check now precedes authentication. Files, Skills and Core project extensions still ignore the header. | | RH7 | Repeated OpenAI-Beta header lines (HP-03: `B08`) | 400 `invalid_beta` unless there is exactly one field value, equal to `agents=v1`. | @@ -825,9 +825,16 @@ Decisions: router, every middleware, authentication check and handler see only the rewritten path. A dirty or encoded path therefore reaches exactly the route group and authentication of its canonical path written literally; internal - daemon, node and sandbox routes keep their own authentication. Decoding only - unreserved characters is RFC 3986 normalization, so a proxy that normalizes - URIs the same way sees the same route. + daemon, node and sandbox routes keep their own authentication. The input is the + request's own path spelling (`RawPath` when Go keeps one), never a path + re-escaped from its decoded form, which would turn `%2F` into a separator when + the spelling holds a byte Go considers invalid. `Path` and `RawPath` are then + set consistently, so chi, which prefers `RawPath`, and the ServeMux, which uses + `EscapedPath`, route on the same string. Decoding only unreserved characters is + RFC 3986 normalization, so a proxy that normalizes URIs the same way sees the + same route. The ServeMux still redirects the exact daemon prefix + `/api/v1/agent-daemon` to `/api/v1/agent-daemon/`; that is daemon transport, not + an Agents API path. - The Beta check reads only a constant header and returns no tenant or resource data. Moving it first changes only responses that were rejected either way: every request that passes it is authenticated before the router reaches any @@ -836,9 +843,10 @@ Decisions: reach the Beta group's 404 and 405 after its checks, as before; without the Beta header they now report `invalid_beta` instead of 401. Official behavior there is unobserved. -- Every Core 401 has type `invalid_request_error`, including the deployment - administrator (`invalid_admin_key`) and sandbox node (`invalid_node_credential`) - extensions, whose codes are unchanged. +- Every 401 of the Agents API handler has type `invalid_request_error`, including + the deployment administrator (`invalid_admin_key`) and sandbox node + (`invalid_node_credential`) extensions, whose codes are unchanged. Daemon, + enrollment and node transport served beside it keep their own formats. - The response headers belong to the Agents API handler. Daemon, enrollment and node transport routes do not carry them, and the shared log middleware is unchanged. A caller-supplied request ID is not echoed; that header is not pinned. @@ -852,8 +860,12 @@ Go handler tests cover RH1–RH11, including a walk over every registered route: unauthenticated requests, with and without the Beta header and with foreign credentials, are rejected before any handler, and ten dirty and encoded spellings of each path, including traversal from the daemon and sandbox prefixes, give the -clean path's exact response. A server test replays the daemon-enabled composition -and checks that no request is redirected. The pinned-SDK script +clean path's exact response. Raw request-line tests over a real listener cover +invalid bytes, non-ASCII, `%2F`, `%2f`, `%5C`, double encoding and absolute-form +URIs in both server configurations, and two fuzz targets assert that any request +path reaches the same handler, route group and response as its canonical form, +with chi, the ServeMux and `Path` agreeing on it. A server test replays the +daemon-enabled composition and checks that no Agents API request is redirected. The pinned-SDK script `official_http_routing.py` updates an Agent through base URL `/v1//`, checks `_request_id` and the error `request_id`, and the raw checks in the other official scripts now expect the Beta check first. diff --git a/docs/web/core-connection.md b/docs/web/core-connection.md index c021592c5..ebb10596f 100644 --- a/docs/web/core-connection.md +++ b/docs/web/core-connection.md @@ -754,9 +754,9 @@ tenant ↔ organization/project mapping. Startup validates project mappings atomically. A conflict aborts startup rather than partially accepting the new configuration. -### `400 invalid_beta_header` +### `400 invalid_beta` -Every `/v1` Agents request must include exactly: +Every `/v1` Agents request must include exactly one header line: ```http OpenAI-Beta: agents=v1 diff --git a/services/agents-api/cmd/server/http_routes.go b/services/agents-api/cmd/server/http_routes.go index dc65ee35a..92cd0806f 100644 --- a/services/agents-api/cmd/server/http_routes.go +++ b/services/agents-api/cmd/server/http_routes.go @@ -15,8 +15,9 @@ type daemonRoutes struct { // serverHandler composes the daemon transport routes with the API handler. // Path canonicalization wraps the whole composition, so the ServeMux, the API // router and every middleware decide on the same canonical path, and the -// ServeMux never answers a redirect. The API handler canonicalizes again when -// served alone; the operation is idempotent. +// ServeMux never redirects a non-canonical path. Its only remaining redirect is +// the exact daemon prefix /api/v1/agent-daemon to /api/v1/agent-daemon/. The API +// handler canonicalizes again when served alone; the operation is idempotent. func serverHandler(apiHandler http.Handler, daemon *daemonRoutes) http.Handler { if daemon == nil { return apiHandler From 432ec67cf65ad86ec8e2566a62fb19e955f0dfb7 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 22:00:02 +0000 Subject: [PATCH 09/12] Cover project API key routes in the path security tests The route walk and both differential fuzz targets now include the project API key management routes (deployment administrator authority) and a derived project key: dirty and encoded spellings, including traversal into /core/v1/project-api-keys, reach exactly what their canonical path reaches, and a derived key authenticates the canonical Beta route like its static parent. The MCP credential selection comparison ignores the per-request X-Request-Id and processing time. --- .../official-semantics-alignment.md | 5 +- .../agents-api/cmd/server/http_routes_test.go | 15 +++- .../agents-api/internal/api/routing_test.go | 72 +++++++++++++++---- .../mcp_credential_selection_public_test.go | 2 + 4 files changed, 77 insertions(+), 17 deletions(-) diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index c4e91b021..1908bb88c 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -845,7 +845,10 @@ Decisions: unobserved. - Every 401 of the Agents API handler has type `invalid_request_error`, including the deployment administrator (`invalid_admin_key`) and sandbox node - (`invalid_node_credential`) extensions, whose codes are unchanged. Daemon, + (`invalid_node_credential`) extensions, whose codes are unchanged. Project API + key management keeps its deployment administrator authentication behind the + same canonical path; derived project keys authenticate exactly as their static + parent binding, under the Beta and Files rules above. Daemon, enrollment and node transport served beside it keep their own formats. - The response headers belong to the Agents API handler. Daemon, enrollment and node transport routes do not carry them, and the shared log middleware is diff --git a/services/agents-api/cmd/server/http_routes_test.go b/services/agents-api/cmd/server/http_routes_test.go index 77529e341..637ef7a0a 100644 --- a/services/agents-api/cmd/server/http_routes_test.go +++ b/services/agents-api/cmd/server/http_routes_test.go @@ -2,6 +2,7 @@ package main import ( "bufio" + "context" "fmt" "io" "net" @@ -64,6 +65,13 @@ func TestServerHandlerRoutesCanonicalPaths(t *testing.T) { // trapStore panics on every store call, marking a request that reached a handler. type trapStore struct{ api.ResourceStore } +// trapKeys finds no derived project API key; key management calls panic. +type trapKeys struct{ api.ProjectAPIKeyStore } + +func (trapKeys) ResolveProjectAPIKey(context.Context, string) (store.ProjectAPIKeyBinding, error) { + return store.ProjectAPIKeyBinding{}, store.ErrNotFound +} + // daemonComposition serves the real API handler beside sentinel daemon routes. func daemonComposition(t testing.TB) http.Handler { t.Helper() @@ -76,7 +84,7 @@ func daemonComposition(t testing.TB) http.Handler { if err != nil { t.Fatal(err) } - apiHandler, err := api.NewHandler(trapStore{}, auth, "codex", api.WithSandboxManager(&store.Store{}, admin)) + apiHandler, err := api.NewHandler(trapStore{}, auth, "codex", api.WithSandboxManager(&store.Store{}, admin), api.WithProjectAPIKeys(trapKeys{}, admin)) if err != nil { t.Fatal(err) } @@ -124,6 +132,8 @@ func TestServerHandlerRawPathsKeepEncodedSeparators(t *testing.T) { {"/v1/x\"/..%2f..%2fapi/v1/agent-daemon/connection", "400"}, {"/v1/\xc3\xa9/..%2F..%2Fcore/v1/sandbox/node/connect", "400"}, {"/v1/x{/..%2F..%2Fcore/v1/sandbox/nodes", "400"}, + {"/v1/x{/..%2F..%2Fcore/v1/project-api-keys/x", "400"}, + {"/v1/x{/../../core/v1/project-api-keys/x", "401"}, {"/v1/x\\/..%5C..%5Capi/v1/agent-daemon/ws", "400"}, {"http://example.test/v1/x{/..%252F..%252Fapi/v1/agent-daemon/enroll", "400"}, {"/v1/x{/../../api/v1/agent-daemon/enroll", "204"}, @@ -156,7 +166,8 @@ func TestServerHandlerRawPathsKeepEncodedSeparators(t *testing.T) { func FuzzServerHandlerRoutesLikeCanonicalForm(f *testing.F) { for _, seed := range []string{"v1//agents", "v1/x{/..%2F..%2Fapi/v1/agent-daemon/enroll", "api/v1/agent-daemon%2Fenroll", "v1/\xc3\xa9/../../api/v1/agent-daemon/ws", "core/v1/sandbox/node/%2E%2E/node/connect", "0\"%2F", "api/v1/agent-daemon", - "v1/x\\/..%5C..%5Capi/v1/agent-daemon/connection", "v1/agents/%252F%2e%2E/x"} { + "v1/x\\/..%5C..%5Capi/v1/agent-daemon/connection", "v1/agents/%252F%2e%2E/x", "v1/x{/..%2F..%2Fcore/v1/project-api-keys/x", + "core/v1/project-api-keys/x/%2E%2E/%2E%2E/%2E%2E/%2E%2E/api/v1/agent-daemon/enroll"} { f.Add(seed) } handler := daemonComposition(f) diff --git a/services/agents-api/internal/api/routing_test.go b/services/agents-api/internal/api/routing_test.go index a8f6f4c0d..814a84fb7 100644 --- a/services/agents-api/internal/api/routing_test.go +++ b/services/agents-api/internal/api/routing_test.go @@ -18,17 +18,19 @@ import ( "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/device" "github.com/MiniMax-AI-Dev/parsar/internal/obs/log" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/identity" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" "github.com/go-chi/chi/v5" "github.com/google/uuid" ) const ( - routingKey = "routing-project-key" - routingAdminKey = "routing-admin-key" - unauthorizedV1 = `{"error":{"message":"A valid Agents API bearer key is required.","type":"invalid_request_error","code":null,"param":null}}` + "\n" - invalidBetaV1 = `{"error":{"message":"To access the Agents API, set the 'OpenAI-Beta' header to 'agents=v1'.","type":"invalid_beta","code":"invalid_beta","param":null}}` + "\n" - notAllowedV1 = `{"error":{"message":"This API method is not supported.","type":"invalid_request_error","code":"unsupported_operation","param":null}}` + "\n" + routingKey = "routing-project-key" + routingDerivedKey = "routing-derived-key" + routingAdminKey = "routing-admin-key" + unauthorizedV1 = `{"error":{"message":"A valid Agents API bearer key is required.","type":"invalid_request_error","code":null,"param":null}}` + "\n" + invalidBetaV1 = `{"error":{"message":"To access the Agents API, set the 'OpenAI-Beta' header to 'agents=v1'.","type":"invalid_beta","code":"invalid_beta","param":null}}` + "\n" + notAllowedV1 = `{"error":{"message":"This API method is not supported.","type":"invalid_request_error","code":"unsupported_operation","param":null}}` + "\n" ) var requestIDPattern = regexp.MustCompile(`^req_[0-9a-f]{32}$`) @@ -70,6 +72,20 @@ func (s *routingStore) UpdateAgent(_ context.Context, tenant, id string, input s return s.agent, nil } +// routingKeys resolves one derived project API key to the static routing +// binding. Key management methods panic, so administrator handlers are traps. +type routingKeys struct { + ProjectAPIKeyStore + principal identity.Principal +} + +func (k routingKeys) ResolveProjectAPIKey(_ context.Context, digest string) (store.ProjectAPIKeyBinding, error) { + if digest != device.HashCredential(routingDerivedKey) { + return store.ProjectAPIKeyBinding{}, store.ErrNotFound + } + return store.ProjectAPIKeyBinding{BindingDigest: device.HashCredential(routingKey), Principal: k.principal}, nil +} + // missingFiles reports every File as missing. type missingFiles struct{ SourceFileStore } @@ -79,7 +95,8 @@ func (missingFiles) GetSourceFile(context.Context, string, string) (store.Source // routingFixture returns the served handler and, for route enumeration, a // router built from an identically configured Handler. Sandbox administration -// uses a zero Store, which panics if a handler is ever reached. +// uses a zero Store and project API key management a trap store, which panic +// if a handler is ever reached; derived project keys resolve normally. func routingFixture(t *testing.T) (http.Handler, *chi.Mux, *routingStore) { t.Helper() tenant := uuid.NewString() @@ -93,7 +110,11 @@ func routingFixture(t *testing.T) (http.Handler, *chi.Mux, *routingStore) { } s := &routingStore{tenant: tenant, agent: store.SavedAgent{ID: uuid.NewString(), TenantID: tenant, Metadata: map[string]string{}, Configuration: json.RawMessage(`{"model":"fixture"}`), CreatedAt: time.Unix(1700000000, 0), UpdatedAt: time.Unix(1700000000, 0)}} - options := []Option{WithSandboxManager(&store.Store{}, admin), WithSourceFiles(missingFiles{})} + static, ok := auth.staticBinding(device.HashCredential(routingKey)) + if !ok { + t.Fatal("static routing binding is missing") + } + options := []Option{WithSandboxManager(&store.Store{}, admin), WithProjectAPIKeys(routingKeys{principal: static}, admin), WithSourceFiles(missingFiles{})} handler, err := NewHandler(s, auth, "codex", options...) if err != nil { t.Fatal(err) @@ -200,6 +221,11 @@ func TestNonCanonicalPathsServeTheCleanRoute(t *testing.T) { t.Fatalf("handler saw a non-canonical ID %q", lookup) } } + // A derived project API key authenticates the canonical route like its parent. + derived := withHeaders([]string{"Authorization", "Bearer " + routingDerivedKey}, beta) + if got := serve(handler, http.MethodGet, "/v1//agents/%2e/"+id, "", derived); !sameResponse(got, clean) { + t.Fatalf("derived key through // = %d %s", got.Code, got.Body) + } list := serve(handler, http.MethodGet, "/v1/agents", "", authenticated) if got := serve(handler, http.MethodGet, "/v1//agents", "", authenticated); list.Code != http.StatusOK || !sameResponse(got, list) { t.Fatalf("list through // = %d %s", got.Code, got.Body) @@ -275,19 +301,22 @@ func TestEveryRouteAuthenticatesItsCanonicalPath(t *testing.T) { selfAuthenticated := map[string]bool{"GET /healthz": false, "POST /core/v1/sandbox/enroll": false, "GET /core/v1/sandbox/node/identity": false} credentials := []http.Header{{}, withHeaders(beta), withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta), withHeaders([]string{"Authorization", "Basic " + routingKey}, beta), withHeaders([]string{"Authorization", "Bearer wrong"}), - withHeaders(project), withHeaders(project, []string{"OpenAI-Beta", "agents=v0"})} + withHeaders(project), withHeaders(project, []string{"OpenAI-Beta", "agents=v0"}), + withHeaders([]string{"Authorization", "Bearer " + routingDerivedKey}), withHeaders([]string{"Authorization", "Bearer " + routingDerivedKey}, beta)} routes := 0 + walked := map[string]bool{} err := chi.Walk(router, func(method, route string, _ http.Handler, _ ...func(http.Handler) http.Handler) error { + walked[method+" "+route] = true if _, ok := selfAuthenticated[method+" "+route]; ok { selfAuthenticated[method+" "+route] = true return nil } routes++ clean := concretePath(route) - admin := strings.HasPrefix(route, "/core/v1/sandbox/") + admin := strings.HasPrefix(route, "/core/v1/sandbox/") || strings.HasPrefix(route, "/core/v1/project-api-keys/") betaGroup := strings.HasPrefix(route, "/v1/") && !strings.HasPrefix(route, "/v1/files") && !strings.HasPrefix(route, "/v1/skills") for _, header := range credentials { - projectKey := header.Get("Authorization") == "Bearer "+routingKey + projectKey := header.Get("Authorization") == "Bearer "+routingKey || header.Get("Authorization") == "Bearer "+routingDerivedKey betaHeader := header.Get("OpenAI-Beta") == "agents=v1" adminKey := header.Get("Authorization") == "Bearer "+routingAdminKey if admin && adminKey || !admin && projectKey && (betaHeader || !betaGroup) { @@ -312,12 +341,18 @@ func TestEveryRouteAuthenticatesItsCanonicalPath(t *testing.T) { if err != nil { t.Fatal(err) } + for _, route := range []string{"GET /core/v1/project-api-keys/{binding_digest}/", "POST /core/v1/project-api-keys/{binding_digest}/", + "DELETE /core/v1/project-api-keys/{binding_digest}/{key_id}", "GET /core/v1/sandbox/nodes", "DELETE /core/v1/environments/{environment_id}/executor-credentials/{key_id}"} { + if !walked[route] { + t.Errorf("route %s was not walked", route) + } + } for route, found := range selfAuthenticated { if !found { t.Errorf("self-authenticated route %s is no longer registered", route) } } - if routes < 70 || len(s.lookups) != 0 || len(s.updates) != 0 { + if routes < 73 || len(s.lookups) != 0 || len(s.updates) != 0 { t.Fatalf("walked %d routes; store lookups %v updates %v", routes, s.lookups, s.updates) } // Traversal into internal groups keeps their own authentication. @@ -332,6 +367,11 @@ func TestEveryRouteAuthenticatesItsCanonicalPath(t *testing.T) { {"/v1/agents/..%2F..%2Fcore/v1/sandbox/nodes", withHeaders(project, beta), http.StatusNotFound, "unsupported_operation"}, {"/core/v1/sandbox/%2E%2E/%2E%2E/%2E%2E/v1/agents", withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta), http.StatusUnauthorized, ""}, {"/core/v1/sandbox/nodes%2F..%2F..%2F..%2Fv1%2Fagents", withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta), http.StatusNotFound, ""}, + // Project API key management keeps deployment administrator authority. + {"/v1/%2E%2E/core/v1/project-api-keys/" + device.HashCredential(routingKey), withHeaders(project, beta), http.StatusUnauthorized, "invalid_admin_key"}, + {"/v1/agents//../../core/v1/project-api-keys/" + device.HashCredential(routingKey), withHeaders([]string{"Authorization", "Bearer " + routingDerivedKey}, beta), http.StatusUnauthorized, "invalid_admin_key"}, + {"/v1/x{/..%2F..%2Fcore/v1/project-api-keys/" + device.HashCredential(routingKey), http.Header{}, http.StatusBadRequest, "invalid_beta"}, + {"/core/v1/project-api-keys/" + device.HashCredential(routingKey) + "/%2E%2E/%2E%2E/%2E%2E/%2E%2E/v1/agents", withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta), http.StatusUnauthorized, ""}, } { got := serve(handler, http.MethodGet, test.target, "", test.header) if got.Code != test.status || (test.code != "" && !strings.Contains(got.Body.String(), `"code":"`+test.code+`"`)) { @@ -711,11 +751,14 @@ var canonicalPathSeeds = []string{ "v1/files/..%2F..%2Fv1/skills", "0\"%2F", "core/v1/sandbox/nodes{%2F..%2F..%2F..%2Fv1/agents", "v1/agents/%252F..", "api/v1/agent-daemon/%2E%2E/%2E%2E/%2E%2E/v1/agents", "v1/x{/%2e./core/v1/sandbox/node/identity", "v1/agents/%7E%5F%2D%41", "core/v1/environments/x/executor-credentials/%2E%2E/%2E%2E/%2E%2E/%2E%2E/core/v1/sandbox/nodes", + "v1/x{/..%2F..%2Fcore/v1/project-api-keys/x", "core/v1/project-api-keys/x/%2E%2E/%2E%2E/%2E%2E/%2E%2E/v1/agents", + "v1/%2E%2E/core/v1/project-api-keys/x/", "core/v1/project-api-keys//x/y", } // Differential property over arbitrary request paths: the routed outcome for -// no credentials, the Beta header and the administrator key equals that of -// the canonical form, and every layer sees one consistent canonical path. +// no credentials, the Beta header, the administrator key and a derived project +// API key equals that of the canonical form, and every layer sees one +// consistent canonical path. func FuzzCanonicalPathsRouteLikeTheirCanonicalForm(f *testing.F) { for _, seed := range canonicalPathSeeds { f.Add(seed) @@ -725,7 +768,8 @@ func FuzzCanonicalPathsRouteLikeTheirCanonicalForm(f *testing.F) { t.Skip() } handler, _, _ := routingFixture(t) - for _, header := range []http.Header{{}, withHeaders(beta), withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta)} { + for _, header := range []http.Header{{}, withHeaders(beta), withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta), + withHeaders([]string{"Authorization", "Bearer " + routingDerivedKey}, beta)} { request, err := parseRaw(http.MethodGet, "/"+path, header) if err != nil { t.Skip() diff --git a/services/agents-api/internal/store/mcp_credential_selection_public_test.go b/services/agents-api/internal/store/mcp_credential_selection_public_test.go index 361010b72..96f1f0cd5 100644 --- a/services/agents-api/internal/store/mcp_credential_selection_public_test.go +++ b/services/agents-api/internal/store/mcp_credential_selection_public_test.go @@ -71,6 +71,8 @@ func TestMCPCredentialSelectionPublicPostgres(t *testing.T) { // Per-response headers; every other header takes part in comparisons. response.Header.Del("Date") response.Header.Del("Traceparent") + response.Header.Del("X-Request-Id") + response.Header.Del("Openai-Processing-Ms") return selectionResponse{status: response.StatusCode, header: response.Header, body: raw.String()} } From a4161482a19b33efb70bafdcf1ef67d02612a70e Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 22:10:03 +0000 Subject: [PATCH 10/12] Check request headers on the hosted-failure stream The GET stream that ends after a hosted provisioning failure carries X-Request-Id and OpenAI-Processing-Ms like every Agents API response. --- services/agents-api/internal/api/hosted_failure_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/agents-api/internal/api/hosted_failure_test.go b/services/agents-api/internal/api/hosted_failure_test.go index 433217379..16b102395 100644 --- a/services/agents-api/internal/api/hosted_failure_test.go +++ b/services/agents-api/internal/api/hosted_failure_test.go @@ -137,6 +137,10 @@ func TestGetStreamEndsAfterHostedProvisioningFailure(t *testing.T) { t.Fatal(err) } defer response.Body.Close() + // The stream that ends on the failure carries the request headers (HP-23/24). + if !requestIDPattern.MatchString(response.Header.Get("X-Request-Id")) || response.Header.Get("Openai-Processing-Ms") == "" { + t.Fatal("stream headers", response.Header) + } lines := make(chan string, 32) go func() { defer close(lines) From 62088753e64fb4e08ad1a54aff31909d9d1bcf07 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 22:41:30 +0000 Subject: [PATCH 11/12] Exclude HEAD on Runtime observation and history reads HEAD on a Session Runtime observation, the observation list and Session Runtime history would sample a provider, export an observation or query the history backend. They now answer the JSON 405 with Allow, like the stream, content and directory routes. --- CONTRIBUTING.md | 4 ++-- contracts/agents-api/official-semantics-alignment.md | 2 +- services/agents-api/internal/api/handler.go | 12 ++++++++---- services/agents-api/internal/api/routing_test.go | 8 ++++++-- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 81472fe29..fc83c3670 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -124,8 +124,8 @@ Skills and Core project extensions `invalid_api_key` only for a rejected Bearer credential. Agents API responses carry a fresh `X-Request-Id` (also in the log context), `OpenAI-Version`, `OpenAI-Processing-Ms` and nosniff through the API router's own middleware, not the shared log middleware. HEAD runs GET routes; -streaming, content-download and live directory routes register an explicit HEAD -405 instead. Every 405 of the API router, unknown methods included, has the JSON +streaming, content-download, live directory, Runtime observation and Runtime +history routes register an explicit HEAD 405 instead. Every 405 of the API router, unknown methods included, has the JSON body and lists the route's methods in `Allow`. Keep runtime state, test artifacts and build output under `~/.parsar/`. Require diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index 1908bb88c..390165711 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -807,7 +807,7 @@ ledger records. | --- | --- | --- | | RH1 | `//`, `.` or `..` path segments (HP-17: `R10`, `R11`, `R17`, `R18`) | Served on the canonical path, never redirected. Empty and dot segments resolve with ServeMux semantics and a trailing slash is kept, so `/v1/agents/x/../` still reaches the trailing-slash 404. The former 301 made the pinned SDK resend an update as a GET and drop it. | | RH2 | A percent-encoded unreserved character in the path (HP-18: `R12`) | Decoded before routing, including `%2E` dot segments. The canonical path is built from the request's own path spelling; bytes that are invalid in an escaped path (such as `{`, `"`, a backslash or non-ASCII) are percent-encoded first. Other escapes, such as `%2F`, `%2f`, `%5C` and double encodings, stay encoded and never separate segments. Malformed, missing and foreign IDs keep the single 404. | -| RH3 | HEAD on a GET route (HP-19: `R13`, `R16`) | The GET route runs after the same Beta and authentication checks; 200 with its headers and no body. `Content-Length` is present when Go buffers the whole body (about 2 KiB) and omitted for larger responses. The events stream, the File, Skill, Skill version and Artifact content downloads and the live Environment Files directory list answer HEAD with Core's 405 instead, so HEAD never holds a stream open, reads content or waits on a Runtime. That exclusion is a documented Core difference; official HEAD on those routes is unobserved. | +| RH3 | HEAD on a GET route (HP-19: `R13`, `R16`) | The GET route runs after the same Beta and authentication checks; 200 with its headers and no body. `Content-Length` is present when Go buffers the whole body (about 2 KiB) and omitted for larger responses. The events stream, the File, Skill, Skill version and Artifact content downloads, the live Environment Files directory list and the Core Runtime observation (single and list) and Runtime history reads answer HEAD with Core's 405 instead, so HEAD never holds a stream open, reads content, samples a provider or queries telemetry. That exclusion is a documented Core difference; official HEAD on those routes is unobserved. | | RH4 | Unsupported method (HP-20: `R04`–`R06`) | Unchanged 405 JSON `unsupported_operation`, now with `Allow` listing the route's methods in the observed order, such as `GET,HEAD,POST,DELETE`. Routes outside the Beta group, such as `/healthz` and executor credentials, and methods chi does not know, such as `FOO`, now use the same JSON 405 instead of chi's empty one; an unknown method is answered before the Beta and authentication checks, as before. | | RH5 | `X-Request-Id` (HP-23) | Every response of the Agents API handler, including 400, 401, 404, 405 and SSE streams, carries a fresh random `req_` plus 32 lowercase hex characters. The ID is attached to the request log context as `request_id` next to the trace carrier. | | RH6 | No or invalid credentials without OpenAI-Beta on a Beta route (HP-05: `A06`, `A11`) | 400 `invalid_beta`: the constant Beta check now precedes authentication. Files, Skills and Core project extensions still ignore the header. | diff --git a/services/agents-api/internal/api/handler.go b/services/agents-api/internal/api/handler.go index 34e08dd1c..cd4bd71d3 100644 --- a/services/agents-api/internal/api/handler.go +++ b/services/agents-api/internal/api/handler.go @@ -81,10 +81,11 @@ func NewHandler(s ResourceStore, auth *Authenticator, engine string, options ... // routes builds the router. HEAD runs the GET route without a body after the // same authentication and Beta checks (HP-19). Routes that stream events, -// download content or read a live workspace directory register an explicit -// HEAD 405 instead, so HEAD never holds a stream open, reads full content or -// waits on a Runtime. Every 405, including unknown methods and routes outside -// the Beta group, has the JSON body and Allow header. +// download content, read a live workspace directory, sample Runtime +// observations or query Runtime history register an explicit HEAD 405 instead, +// so HEAD never holds a stream open, reads full content or does Runtime or +// telemetry work. Every 405, including unknown methods and routes outside the +// Beta group, has the JSON body and Allow header. func (h *Handler) routes() *chi.Mux { router := chi.NewRouter() router.Use(agentsResponseHeaders, log.HTTPMiddleware, middleware.GetHead) @@ -137,9 +138,12 @@ func (h *Handler) routes() *chi.Mux { r.Get("/agents/sessions/{session_id}", h.getSession) r.Get("/agents/sessions/{session_id}/execution-configuration", h.getSessionExecutionConfiguration) r.Get("/agents/sessions/{session_id}/runtime-observation", h.getRuntimeObservation) + r.Head("/agents/sessions/{session_id}/runtime-observation", methodNotAllowed) r.Get("/agents/runtime-observations", h.listRuntimeObservations) + r.Head("/agents/runtime-observations", methodNotAllowed) r.Get("/agents/runtime-history/capabilities", h.getRuntimeHistoryCapabilities) r.Get("/agents/sessions/{session_id}/runtime-history", h.getRuntimeHistory) + r.Head("/agents/sessions/{session_id}/runtime-history", methodNotAllowed) r.Post("/agents/sessions/{session_id}", h.updateSession) r.Delete("/agents/sessions/{session_id}", h.deleteSession) r.Post("/agents/sessions/{session_id}/events", h.createEvents) diff --git a/services/agents-api/internal/api/routing_test.go b/services/agents-api/internal/api/routing_test.go index 814a84fb7..6c88f4183 100644 --- a/services/agents-api/internal/api/routing_test.go +++ b/services/agents-api/internal/api/routing_test.go @@ -508,8 +508,8 @@ func TestMethodNotAllowedListsRouteMethods(t *testing.T) { } // HEAD runs a GET route without a body after the same checks (HP-19). SSE, -// content-download and live directory routes answer 405 without opening a -// stream, reading content or waiting on a Runtime. +// content-download, live directory and Runtime observation or history routes +// answer 405 without opening a stream, reading content or doing Runtime work. func TestHeadRequests(t *testing.T) { handler, _, s := routingFixture(t) server := httptest.NewServer(handler) @@ -550,6 +550,10 @@ func TestHeadRequests(t *testing.T) { for _, test := range []struct{ path, allow string }{ {session + "/events", "GET,POST"}, {"/v1/agents/environments/" + uuid.NewString() + "/files", "GET,POST"}, + {session + "/runtime-observation", "GET"}, + // POST and DELETE on this path route to Agent update and deletion. + {"/v1/agents/runtime-observations", "GET,POST,DELETE"}, + {session + "/runtime-history", "GET"}, {session + "/artifacts/" + uuid.NewString() + "/content", "GET"}, {"/v1/files/file-missing/content", "GET"}, {"/v1/skills/skill-missing/content", "GET"}, From d13e8efd89aeca6fb881b3ca7d602cb78bb1eac3 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 22:41:37 +0000 Subject: [PATCH 12/12] Check both path fuzz targets against an independent oracle The targets compared a spelling only with CanonicalPaths' own output, so reverting to EscapedPath still passed. An oracle now splits only on a literal '/', decodes each segment once and resolves only segments that decode to '.' or '..'. Both targets assert that the routed segments equal the oracle's and that the oracle's own spelling reaches the same route and response. With EscapedPath restored and no invalid-byte seeds, each target finds the %2F blocker within seconds. --- .../agents-api/cmd/server/http_routes_test.go | 99 ++++++++++++++++++- .../agents-api/internal/api/routing_test.go | 88 ++++++++++++++++- 2 files changed, 183 insertions(+), 4 deletions(-) diff --git a/services/agents-api/cmd/server/http_routes_test.go b/services/agents-api/cmd/server/http_routes_test.go index 637ef7a0a..bd3415236 100644 --- a/services/agents-api/cmd/server/http_routes_test.go +++ b/services/agents-api/cmd/server/http_routes_test.go @@ -8,6 +8,8 @@ import ( "net" "net/http" "net/http/httptest" + "net/url" + "slices" "strconv" "strings" "testing" @@ -171,12 +173,28 @@ func FuzzServerHandlerRoutesLikeCanonicalForm(f *testing.F) { f.Add(seed) } handler := daemonComposition(f) + // Every route of the sentinel composition reports the path it was served on, + // as chi reads it: RawPath when set, otherwise the decoded Path. + sentinel := func(route string) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + routed := r.URL.RawPath + if routed == "" { + routed = r.URL.EscapedPath() + } + w.Header().Set("X-Route", route) + w.Header().Set("X-Routed-Path", routed) + w.WriteHeader(http.StatusNoContent) + }) + } + observed := serverHandler(sentinel("api"), &daemonRoutes{gateway: sentinel("gateway"), enrollment: sentinel("enrollment"), + connection: sentinel("connection"), nodeConnect: sentinel("node")}) f.Fuzz(func(t *testing.T, path string) { if strings.ContainsAny(path, " ?#") || len(path) > 512 { t.Skip() } + segments, trailing, ok := oraclePath("/" + path) request, err := parseRaw("/" + path) - if err != nil { + if err != nil || !ok { t.Skip() } again, err := parseRaw(canonical(request)) @@ -186,5 +204,84 @@ func FuzzServerHandlerRoutesLikeCanonicalForm(f *testing.F) { if got, want := outcome(handler, request), outcome(handler, again); got != want { t.Fatalf("%q = %s; canonical %q gives %s", path, got, again.RequestURI, want) } + // Independently of CanonicalPaths: the ServeMux dispatches to the same + // route as the oracle's spelling, and the handler sees the oracle's segments. + oracle, err := parseRaw(oracleTarget(segments, trailing)) + if err != nil { + t.Fatal(err) + } + request, _ = parseRaw("/" + path) + served, expected := httptest.NewRecorder(), httptest.NewRecorder() + observed.ServeHTTP(served, request) + observed.ServeHTTP(expected, oracle) + if served.Code != expected.Code || served.Header().Get("X-Route") != expected.Header().Get("X-Route") { + t.Fatalf("%q = %d %s; oracle %q gives %d %s", path, served.Code, served.Header().Get("X-Route"), oracle.RequestURI, expected.Code, expected.Header().Get("X-Route")) + } + if served.Code == http.StatusNoContent { + if got, gotTrailing := routedSegments(served.Header().Get("X-Routed-Path")); !slices.Equal(got, segments) || gotTrailing != trailing { + t.Fatalf("%q is served on %q %v; the oracle gives %q %v", path, got, gotTrailing, segments, trailing) + } + } }) } + +// oraclePath derives the canonical segments of a raw request path without the +// implementation: split only on a literal '/', decode each segment once, treat +// a segment as a dot segment only if it decodes to "." or "..", and drop empty +// segments. Encoded separators such as %2F and %5C therefore stay inside their +// segment. It reports whether a trailing slash remains and whether every +// segment is validly escaped. +func oraclePath(raw string) (segments []string, trailing, ok bool) { + for _, part := range strings.Split(raw, "/") { + decoded, err := url.PathUnescape(part) + if err != nil { + return nil, false, false + } + switch decoded { + case "", ".": + case "..": + if len(segments) > 0 { + segments = segments[:len(segments)-1] + } + default: + segments = append(segments, decoded) + } + } + return segments, strings.HasSuffix(raw, "/") && len(segments) > 0, true +} + +// routedSegments splits a routed escaped path on literal '/' and decodes each +// segment once, so hex case does not affect the comparison. +func routedSegments(escaped string) (segments []string, trailing bool) { + trimmed := strings.TrimPrefix(escaped, "/") + trailing = strings.HasSuffix(trimmed, "/") + if trimmed = strings.TrimSuffix(trimmed, "/"); trimmed == "" { + return nil, false + } + for _, part := range strings.Split(trimmed, "/") { + decoded, _ := url.PathUnescape(part) + segments = append(segments, decoded) + } + return segments, trailing +} + +// oracleTarget spells oracle segments as a request path, escaping '%', '/' +// and every byte outside RFC 3986 pchar. +func oracleTarget(segments []string, trailing bool) string { + var target strings.Builder + for _, segment := range segments { + target.WriteByte('/') + for i := 0; i < len(segment); i++ { + c := segment[i] + if 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' || strings.IndexByte("-._~!$&'()*+,;=:@", c) >= 0 { + target.WriteByte(c) + } else { + fmt.Fprintf(&target, "%%%02X", c) + } + } + } + if trailing || len(segments) == 0 { + target.WriteByte('/') + } + return target.String() +} diff --git a/services/agents-api/internal/api/routing_test.go b/services/agents-api/internal/api/routing_test.go index 6c88f4183..9d7ba643a 100644 --- a/services/agents-api/internal/api/routing_test.go +++ b/services/agents-api/internal/api/routing_test.go @@ -11,6 +11,7 @@ import ( "net/http/httptest" "net/url" "regexp" + "slices" "strconv" "strings" "testing" @@ -771,20 +772,101 @@ func FuzzCanonicalPathsRouteLikeTheirCanonicalForm(f *testing.F) { if strings.ContainsAny(path, " ?#") || len(path) > 512 { t.Skip() } + segments, trailing, ok := oraclePath("/" + path) + if !ok { + t.Skip() + } handler, _, _ := routingFixture(t) - for _, header := range []http.Header{{}, withHeaders(beta), withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta), + for i, header := range []http.Header{{}, withHeaders(beta), withHeaders([]string{"Authorization", "Bearer " + routingAdminKey}, beta), withHeaders([]string{"Authorization", "Bearer " + routingDerivedKey}, beta)} { request, err := parseRaw(http.MethodGet, "/"+path, header) if err != nil { t.Skip() } - canonical, err := parseRaw(http.MethodGet, canonicalTarget(t, request), header) + target := canonicalTarget(t, request) + if got, gotTrailing := routedSegments(target); !slices.Equal(got, segments) || gotTrailing != trailing { + t.Fatalf("%q routes on %q %v; the oracle gives %q %v", path, got, gotTrailing, segments, trailing) + } + canonical, err := parseRaw(http.MethodGet, target, header) if err != nil { t.Fatal(err) } - if got, want := outcome(handler, request), outcome(handler, canonical); got != want { + got := outcome(handler, request) + if want := outcome(handler, canonical); got != want { t.Fatalf("%q = %s; canonical %q gives %s", path, got, canonical.RequestURI, want) } + // Requests that never reach a Beta handler must also match the + // oracle's own spelling, independently of CanonicalPaths. + if i < 3 { + oracle, err := parseRaw(http.MethodGet, oracleTarget(segments, trailing), header) + if err != nil { + t.Fatal(err) + } + if want := outcome(handler, oracle); got != want { + t.Fatalf("%q = %s; oracle %q gives %s", path, got, oracle.RequestURI, want) + } + } } }) } + +// oraclePath derives the canonical segments of a raw request path without the +// implementation: split only on a literal '/', decode each segment once, treat +// a segment as a dot segment only if it decodes to "." or "..", and drop empty +// segments. Encoded separators such as %2F and %5C therefore stay inside their +// segment. It reports whether a trailing slash remains and whether every +// segment is validly escaped. +func oraclePath(raw string) (segments []string, trailing, ok bool) { + for _, part := range strings.Split(raw, "/") { + decoded, err := url.PathUnescape(part) + if err != nil { + return nil, false, false + } + switch decoded { + case "", ".": + case "..": + if len(segments) > 0 { + segments = segments[:len(segments)-1] + } + default: + segments = append(segments, decoded) + } + } + return segments, strings.HasSuffix(raw, "/") && len(segments) > 0, true +} + +// routedSegments splits a routed escaped path on literal '/' and decodes each +// segment once, so hex case does not affect the comparison. +func routedSegments(escaped string) (segments []string, trailing bool) { + trimmed := strings.TrimPrefix(escaped, "/") + trailing = strings.HasSuffix(trimmed, "/") + if trimmed = strings.TrimSuffix(trimmed, "/"); trimmed == "" { + return nil, false + } + for _, part := range strings.Split(trimmed, "/") { + decoded, _ := url.PathUnescape(part) + segments = append(segments, decoded) + } + return segments, trailing +} + +// oracleTarget spells oracle segments as a request path, escaping '%', '/' +// and every byte outside RFC 3986 pchar. +func oracleTarget(segments []string, trailing bool) string { + var target strings.Builder + for _, segment := range segments { + target.WriteByte('/') + for i := 0; i < len(segment); i++ { + c := segment[i] + if 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' || strings.IndexByte("-._~!$&'()*+,;=:@", c) >= 0 { + target.WriteByte(c) + } else { + fmt.Fprintf(&target, "%%%02X", c) + } + } + } + if trailing || len(segments) == 0 { + target.WriteByte('/') + } + return target.String() +}