diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 58065ae06..fc83c3670 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -108,6 +108,26 @@ 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. 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 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, 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 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/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 512 { + t.Skip() + } + segments, trailing, ok := oraclePath("/" + path) + request, err := parseRaw("/" + path) + if err != nil || !ok { + 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) + } + // 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/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..cd4bd71d3 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,20 @@ 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, +// 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(log.HTTPMiddleware) + 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"}) }) @@ -87,6 +100,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) @@ -117,19 +131,24 @@ func NewHandler(s ResourceStore, auth *Authenticator, engine string, options ... 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) 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) 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 +156,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/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) diff --git a/services/agents-api/internal/api/routing.go b/services/agents-api/internal/api/routing.go new file mode 100644 index 000000000..fec1eebed --- /dev/null +++ b/services/agents-api/internal/api/routing.go @@ -0,0 +1,228 @@ +package api + +import ( + "crypto/rand" + "encoding/hex" + "fmt" + "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. 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) { + // 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 { + // A parsed request path has only valid escapes; this is unreachable. + http.Error(w, "Invalid request path.", http.StatusBadRequest) + return + } + 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, rawPath + r = r.WithContext(r.Context()) + r.URL = &canonical + } + next.ServeHTTP(w, r) + }) +} + +// 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 { + 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 { + 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) + 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..9d7ba643a --- /dev/null +++ b/services/agents-api/internal/api/routing_test.go @@ -0,0 +1,872 @@ +package api + +import ( + "bufio" + "context" + "encoding/json" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "net/url" + "regexp" + "slices" + "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/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" + 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}$`) + +// 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 +} + +// 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 } + +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 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() + 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)}} + 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) + } + 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) + } + } + // 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) + } + 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"}), + 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/") || 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 || 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) { + 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 := 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 < 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. + 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, ""}, + // 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+`"`)) { + 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) + } + } + // 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) + } + 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, +// 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) + 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"}, + {"/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"}, + {"/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) + } +} + +// 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", + "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, 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) + } + f.Fuzz(func(t *testing.T, path string) { + if strings.ContainsAny(path, " ?#") || len(path) > 512 { + t.Skip() + } + segments, trailing, ok := oraclePath("/" + path) + if !ok { + t.Skip() + } + handler, _, _ := routingFixture(t) + 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() + } + 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) + } + 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() +} 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) 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()} } 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: