From 9a0b3cb6b74848d1b59d519d6c6c13266a47e249 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 20:12:01 +0000 Subject: [PATCH 01/12] Check Agents API JSON bodies in one shared gate Every Agents API JSON route now reads its body through readJSONObject before route-specific decoding, validation or lookup. It requires a JSON Content-Type, applies the route's body limit, then rejects invalid UTF-8, malformed JSON, repeated keys at any depth and non-object roots with the official messages, and treats a zero-length body or null as {} (HP-09..15). The local 'Duplicate parameter' error and the metadata last-value path are retired. --- services/agents-api/internal/api/agents.go | 2 +- .../agents-api/internal/api/agents_update.go | 2 +- .../internal/api/claude_admission_test.go | 1 + .../internal/api/configuration_validation.go | 18 +- .../api/configuration_validation_test.go | 38 +- .../agents-api/internal/api/credentials.go | 2 +- .../internal/api/credentials_test.go | 1 + .../internal/api/credentials_update.go | 2 +- .../internal/api/credentials_update_test.go | 1 + .../internal/api/disabled_tools_test.go | 1 + .../internal/api/environment_creation_test.go | 4 + .../internal/api/environment_files_create.go | 10 +- .../api/environment_files_create_test.go | 1 + .../api/environment_files_wire_test.go | 27 +- .../internal/api/environment_input_test.go | 3 + .../internal/api/environment_templates.go | 2 +- .../api/environment_templates_test.go | 1 + .../api/function_configuration_test.go | 2 + .../internal/api/function_inputs_test.go | 1 + services/agents-api/internal/api/handler.go | 7 +- .../agents-api/internal/api/handler_test.go | 5 +- .../agents-api/internal/api/harness_test.go | 1 + .../internal/api/hosted_environment_test.go | 1 + services/agents-api/internal/api/inputs.go | 20 +- .../agents-api/internal/api/inputs_test.go | 4 + .../agents-api/internal/api/json_request.go | 176 ++++++++ .../internal/api/json_request_test.go | 406 ++++++++++++++++++ .../internal/api/resource_query_test.go | 2 + .../agents-api/internal/api/routing_test.go | 14 +- .../internal/api/sandbox_selector_test.go | 1 + .../internal/api/session_admission_test.go | 2 + .../api/session_creation_stream_test.go | 3 + .../api/session_environment_http_test.go | 1 + .../api/session_initial_input_test.go | 2 + .../internal/api/session_metadata.go | 16 +- .../internal/api/session_request_test.go | 1 + .../internal/api/session_semantics_test.go | 2 + .../agents-api/internal/api/subagents_test.go | 1 + .../internal/api/text_configuration_test.go | 2 + services/agents-api/internal/api/vaults.go | 2 +- .../agents-api/internal/api/vaults_test.go | 6 +- .../agent_execution_defaults_http_test.go | 1 + .../configuration_validation_public_test.go | 9 +- .../creation_stream_settlement_public_test.go | 1 + ...onment_file_write_semantics_public_test.go | 1 + .../internal/store/harness_onboarding_test.go | 1 + .../internal/store/initial_files_http_test.go | 1 + .../store/remote_mcp_credentials_test.go | 1 + .../internal/store/remote_mcp_test.go | 1 + .../store/request_body_public_test.go | 143 ++++++ .../store/saved_web_search_public_test.go | 1 + .../session_model_execution_http_test.go | 1 + 52 files changed, 867 insertions(+), 88 deletions(-) create mode 100644 services/agents-api/internal/api/json_request_test.go create mode 100644 services/agents-api/internal/store/request_body_public_test.go diff --git a/services/agents-api/internal/api/agents.go b/services/agents-api/internal/api/agents.go index ed97cbb38..04eed9231 100644 --- a/services/agents-api/internal/api/agents.go +++ b/services/agents-api/internal/api/agents.go @@ -31,7 +31,7 @@ type AgentStore interface { // @Failure 400,401,413,500 {object} v1.ErrorResponse // @Router /agents [post] func (h *Handler) createAgent(w http.ResponseWriter, r *http.Request) { - raw, ok := readJSONBody(w, r) + raw, ok := readJSONObject(w, r) if !ok { return } diff --git a/services/agents-api/internal/api/agents_update.go b/services/agents-api/internal/api/agents_update.go index 45f5a0ec1..fddb61238 100644 --- a/services/agents-api/internal/api/agents_update.go +++ b/services/agents-api/internal/api/agents_update.go @@ -23,7 +23,7 @@ import ( // @Failure 400,401,404,413,500 {object} v1.ErrorResponse // @Router /agents/{agent_id} [post] func (h *Handler) updateAgent(w http.ResponseWriter, r *http.Request) { - raw, ok := readJSONBody(w, r) + raw, ok := readJSONObject(w, r) if !ok { return } diff --git a/services/agents-api/internal/api/claude_admission_test.go b/services/agents-api/internal/api/claude_admission_test.go index dde53634c..41a5345d7 100644 --- a/services/agents-api/internal/api/claude_admission_test.go +++ b/services/agents-api/internal/api/claude_admission_test.go @@ -56,6 +56,7 @@ func TestClaudeSessionConfigurationAdmission(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(body)) request.Header.Set("Authorization", "Bearer test-api-key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() handler.ServeHTTP(response, request) want := http.StatusBadRequest diff --git a/services/agents-api/internal/api/configuration_validation.go b/services/agents-api/internal/api/configuration_validation.go index 5f922ae19..94e120135 100644 --- a/services/agents-api/internal/api/configuration_validation.go +++ b/services/agents-api/internal/api/configuration_validation.go @@ -230,15 +230,14 @@ func checkValue(path string, raw json.RawMessage, s shape) error { return nil } -// checkMembers rejects unknown and repeated members in document order, then -// validates the values in document order and the required members in the pinned -// order. Names match exactly, so a name that differs from a member only by case -// is unknown. encoding/json matches names case-insensitively and merges repeated -// objects into pointer structs, so only an object whose members each appear once -// with their exact names decodes to the checked values. +// checkMembers rejects unknown members in document order, then validates the +// values in document order and the required members in the pinned order. Names +// match exactly, so a name that differs from a member only by case is unknown, +// although encoding/json would match it case-insensitively. Repeated members, +// which encoding/json would merge, never reach it: the shared body gate +// (readJSONObject) rejects them first. func checkMembers(path string, raw json.RawMessage, members []member) error { keys, fields := orderedMembers(raw) - seen := make(map[string]bool, len(keys)) for _, key := range keys { if _, known := findMember(members, key); !known { if !echoableField(key) { @@ -246,11 +245,6 @@ func checkMembers(path string, raw json.RawMessage, members []member) error { } return &fieldError{param: joinPath(path, key), message: fmt.Sprintf("Unknown parameter: '%s'.", joinPath(path, key))} } - if seen[key] { - // A local message: the official response to a repeated member is unobserved. - return &fieldError{param: joinPath(path, key), message: fmt.Sprintf("Duplicate parameter: '%s'.", joinPath(path, key))} - } - seen[key] = true } for _, key := range keys { spec, _ := findMember(members, key) diff --git a/services/agents-api/internal/api/configuration_validation_test.go b/services/agents-api/internal/api/configuration_validation_test.go index 575b74295..b76115978 100644 --- a/services/agents-api/internal/api/configuration_validation_test.go +++ b/services/agents-api/internal/api/configuration_validation_test.go @@ -108,22 +108,24 @@ func TestAgentConfigurationProtocolErrorsUseOfficialFields(t *testing.T) { {"schema array", `"text":{"format":{"type":"json_schema","schema":[]}}`, "{p}text.format.schema", "Invalid type for '{p}text.format.schema': expected an object with string keys and unknown value values, but got an array instead."}, {"reasoning array", `"reasoning":[]`, "{p}reasoning", "Invalid type for '{p}reasoning': expected an object, but got an array instead."}, {"case variant", `"reasoning":{"Effort":"high"}`, "{p}reasoning.Effort", "Unknown parameter: '{p}reasoning.Effort'."}, - // encoding/json merges repeated objects and matches names case-insensitively, - // so repeated members and case variants reject anywhere in the tree (local message). - {"merged text", `"text":{"format":{"type":"json_schema","schema":{"type":"array"}}},"text":{"verbosity":"low"}`, "{p}text", "Duplicate parameter: '{p}text'."}, - {"merged reasoning", `"reasoning":{"Effort":"high"},"reasoning":{}`, "{p}reasoning", "Duplicate parameter: '{p}reasoning'."}, - {"merged location", `"tools":[{"type":"web_search","mode":"disabled","location":{"city":"Paris"},"location":{"country":null}}]`, "{p}tools[0].location", "Duplicate parameter: '{p}tools[0].location'."}, + // encoding/json merges repeated objects and matches names case-insensitively. + // The shared body gate rejects repeated keys anywhere in the tree with a null + // param (HP-11); case variants are unknown members. + {"merged text", `"text":{"format":{"type":"json_schema","schema":{"type":"array"}}},"text":{"verbosity":"low"}`, "", "Invalid body: duplicate JSON key 'text' at '{p}text'. Duplicate JSON keys are not supported."}, + {"merged reasoning", `"reasoning":{"Effort":"high"},"reasoning":{}`, "", "Invalid body: duplicate JSON key 'reasoning' at '{p}reasoning'. Duplicate JSON keys are not supported."}, + {"merged location", `"tools":[{"type":"web_search","mode":"disabled","location":{"city":"Paris"},"location":{"country":null}}]`, "", "Invalid body: duplicate JSON key 'location' at '{p}tools.location'. Duplicate JSON keys are not supported."}, {"case variant member", `"reasoning":{"effort":"high","EFFORT":"max"}`, "{p}reasoning.EFFORT", "Unknown parameter: '{p}reasoning.EFFORT'."}, - {"repeated scalar", `"reasoning":{"effort":"high","effort":"bogus"}`, "{p}reasoning.effort", "Duplicate parameter: '{p}reasoning.effort'."}, - {"repeated root", `"service_tier":"auto","service_tier":"flex"`, "{p}service_tier", "Duplicate parameter: '{p}service_tier'."}, - {"repeated tool type", `"tools":[{"type":"function","type":"function","name":"f","description":"","parameters":{}}]`, "{p}tools[0].type", "Duplicate parameter: '{p}tools[0].type'."}, - {"repeated nested", `"multi_agent":{"enabled":true,"enabled":false}`, "{p}multi_agent.enabled", "Duplicate parameter: '{p}multi_agent.enabled'."}, + {"repeated scalar", `"reasoning":{"effort":"high","effort":"bogus"}`, "", "Invalid body: duplicate JSON key 'effort' at '{p}reasoning.effort'. Duplicate JSON keys are not supported."}, + {"repeated root", `"service_tier":"auto","service_tier":"flex"`, "", "Invalid body: duplicate JSON key 'service_tier' at '{p}service_tier'. Duplicate JSON keys are not supported."}, + {"repeated tool type", `"tools":[{"type":"function","type":"function","name":"f","description":"","parameters":{}}]`, "", "Invalid body: duplicate JSON key 'type' at '{p}tools.type'. Duplicate JSON keys are not supported."}, + {"repeated nested", `"multi_agent":{"enabled":true,"enabled":false}`, "", "Invalid body: duplicate JSON key 'enabled' at '{p}multi_agent.enabled'. Duplicate JSON keys are not supported."}, {"case variant root", `"Text":{}`, "{p}Text", "Unknown parameter: '{p}Text'."}, {"case variant nested", `"text":{"Verbosity":"low"}`, "{p}text.Verbosity", "Unknown parameter: '{p}text.Verbosity'."}, {"case variant tool", `"tools":[{"type":"web_search","mode":"disabled","Location":{}}]`, "{p}tools[0].Location", "Unknown parameter: '{p}tools[0].Location'."}, - {"unknown first", `"tool_choice":"auto","text":{},"text":{}`, "{p}tool_choice", "Unknown parameter: '{p}tool_choice'."}, - {"repeat first", `"text":{},"text":{},"tool_choice":"auto"`, "{p}text", "Duplicate parameter: '{p}text'."}, - {"repeat before values", `"reasoning":{"effort":"bogus"},"text":{},"text":{}`, "{p}text", "Duplicate parameter: '{p}text'."}, + // The body gate reports a repeated key before any member or value check. + {"unknown first", `"tool_choice":"auto","text":{},"text":{}`, "", "Invalid body: duplicate JSON key 'text' at '{p}text'. Duplicate JSON keys are not supported."}, + {"repeat first", `"text":{},"text":{},"tool_choice":"auto"`, "", "Invalid body: duplicate JSON key 'text' at '{p}text'. Duplicate JSON keys are not supported."}, + {"repeat before values", `"reasoning":{"effort":"bogus"},"text":{},"text":{}`, "", "Invalid body: duplicate JSON key 'text' at '{p}text'. Duplicate JSON keys are not supported."}, {"enabled null", `"multi_agent":{"enabled":null}`, "{p}multi_agent.enabled", "Invalid type for '{p}multi_agent.enabled': expected a boolean, but got null instead."}, {"maximum number", `"multi_agent":{"enabled":true,"max_concurrent_subagents":1.5}`, "{p}multi_agent.max_concurrent_subagents", "Invalid type for '{p}multi_agent.max_concurrent_subagents': expected an integer, but got a number instead."}, {"maximum negative", `"multi_agent":{"enabled":true,"max_concurrent_subagents":-99999999999999999999}`, "{p}multi_agent.max_concurrent_subagents", "Invalid '{p}multi_agent.max_concurrent_subagents': integer below minimum value. Expected a value >= 1."}, @@ -171,14 +173,18 @@ func TestSessionAgentProtocolErrors(t *testing.T) { {`{"agent":{"model":"m","metadata":{}},"environment":{"type":"none"}}`, "agent.metadata", "Unknown parameter: 'agent.metadata'."}, {`{"agent":{"model":null},"environment":{"type":"none"}}`, "agent.model", "Invalid type for 'agent.model': expected a string, but got null instead."}, {`{"agent":{"model":4},"environment":{"type":"none"}}`, "agent.model", "Invalid type for 'agent.model': expected a string, but got an integer instead."}, - // The Session body keeps its decoder, which replaces a repeated agent whole. - {`{"agent":{"model":"m"},"agent":{"model":"m","model":"n"},"environment":{"type":"none"}}`, "agent.model", "Duplicate parameter: 'agent.model'."}, + // The body gate reports the first repeated key in document order. + {`{"agent":{"model":"m"},"agent":{"model":"m","model":"n"},"environment":{"type":"none"}}`, "", "Invalid body: duplicate JSON key 'agent' at 'agent'. Duplicate JSON keys are not supported."}, } { - assertConfigurationError(t, credentialRequest(h, http.MethodPost, "/v1/agents/sessions", tc.body), "invalid_request_error", param(tc.param), tc.message) + want := param(tc.param) + if tc.param == "" { + want = nil + } + assertConfigurationError(t, credentialRequest(h, http.MethodPost, "/v1/agents/sessions", tc.body), "invalid_request_error", want, tc.message) } for _, path := range []string{"/v1/agents", "/v1/agents/" + uuid.NewString()} { assertConfigurationError(t, credentialRequest(h, http.MethodPost, path, `{"model":4}`), "invalid_request_error", param("model"), "Invalid type for 'model': expected a string, but got an integer instead.") - assertConfigurationError(t, credentialRequest(h, http.MethodPost, path, `{"model":"m","model":"n"}`), "invalid_request_error", param("model"), "Duplicate parameter: 'model'.") + assertConfigurationError(t, credentialRequest(h, http.MethodPost, path, `{"model":"m","model":"n"}`), "invalid_request_error", nil, "Invalid body: duplicate JSON key 'model' at 'model'. Duplicate JSON keys are not supported.") } // Saved Agent creation requires a model; updates and Session overrides do not. assertConfigurationError(t, credentialRequest(h, http.MethodPost, "/v1/agents", `{"name":"x"}`), "invalid_request_error", param("model"), "Missing required parameter: 'model'.") diff --git a/services/agents-api/internal/api/credentials.go b/services/agents-api/internal/api/credentials.go index d8a94d0a4..068d9d6c4 100644 --- a/services/agents-api/internal/api/credentials.go +++ b/services/agents-api/internal/api/credentials.go @@ -35,7 +35,7 @@ type CredentialStore interface { // @Router /vaults/{vault_id}/credentials [post] func (h *Handler) createCredential(w http.ResponseWriter, r *http.Request) { vaultID := credentialPathID(r, "vault_id") - raw, ok := readJSONBody(w, r) + raw, ok := readJSONObject(w, r) if !ok { return } diff --git a/services/agents-api/internal/api/credentials_test.go b/services/agents-api/internal/api/credentials_test.go index 6f3017cfa..a506509b4 100644 --- a/services/agents-api/internal/api/credentials_test.go +++ b/services/agents-api/internal/api/credentials_test.go @@ -53,6 +53,7 @@ func credentialRequest(h http.Handler, method, path, body string) *httptest.Resp r := httptest.NewRequest(method, path, strings.NewReader(body)) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") r.Header.Set("X-Tenant-ID", "untrusted") w := httptest.NewRecorder() h.ServeHTTP(w, r) diff --git a/services/agents-api/internal/api/credentials_update.go b/services/agents-api/internal/api/credentials_update.go index d99c9c874..731cea456 100644 --- a/services/agents-api/internal/api/credentials_update.go +++ b/services/agents-api/internal/api/credentials_update.go @@ -23,7 +23,7 @@ import ( // @Router /vaults/{vault_id}/credentials/{credential_id} [post] func (h *Handler) updateCredential(w http.ResponseWriter, r *http.Request) { vaultID, id := credentialPathID(r, "vault_id"), credentialPathID(r, "credential_id") - raw, ok := readJSONBody(w, r) + raw, ok := readJSONObject(w, r) if !ok { return } diff --git a/services/agents-api/internal/api/credentials_update_test.go b/services/agents-api/internal/api/credentials_update_test.go index 315ddfe46..2af2a7317 100644 --- a/services/agents-api/internal/api/credentials_update_test.go +++ b/services/agents-api/internal/api/credentials_update_test.go @@ -95,6 +95,7 @@ func TestCredentialUpdateUsesExistingBoundariesAndSafeErrors(t *testing.T) { } if mode != "missing beta" { r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") } w := httptest.NewRecorder() h.ServeHTTP(w, r) diff --git a/services/agents-api/internal/api/disabled_tools_test.go b/services/agents-api/internal/api/disabled_tools_test.go index b64aebb86..0f8004f2e 100644 --- a/services/agents-api/internal/api/disabled_tools_test.go +++ b/services/agents-api/internal/api/disabled_tools_test.go @@ -55,6 +55,7 @@ func TestDisabledToolAdmissionPrecedesPersistence(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(`{"agent":{"model":"model","tools":`+tools+`},"environment":{"type":"none"},"input":"Use only the enabled tools."}`)) req.Header.Set("Authorization", "Bearer test-api-key") req.Header.Set("OpenAI-Beta", "agents=v1") + req.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() h.ServeHTTP(response, req) if response.Code != 400 || store.tenant != "" { diff --git a/services/agents-api/internal/api/environment_creation_test.go b/services/agents-api/internal/api/environment_creation_test.go index 1612d1165..35a71755a 100644 --- a/services/agents-api/internal/api/environment_creation_test.go +++ b/services/agents-api/internal/api/environment_creation_test.go @@ -86,6 +86,7 @@ func TestSelfHostedEmptyCreationAndStream(t *testing.T) { request.Header.Set("X-Forwarded-Host", "forged.example") request.Header.Set("Authorization", "Bearer key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") request.Header.Set("Idempotency-Key", "empty-environment") response, err := server.Client().Do(request) if err != nil { @@ -184,6 +185,7 @@ func TestSelfHostedCreationRejectsBeforePersistence(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(body)) request.Header.Set("Authorization", "Bearer key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() handler.ServeHTTP(response, request) if response.Code != http.StatusBadRequest || fixture.input.Engine != "" || fixture.session.ID != "" { @@ -202,6 +204,7 @@ func TestSelfHostedCreationRequiresOperatorExecution(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(body)) request.Header.Set("Authorization", "Bearer key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() handler.ServeHTTP(response, request) var failure v1.ErrorResponse @@ -219,6 +222,7 @@ func TestHostedCreationRequiresOperatorExecution(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(body)) request.Header.Set("Authorization", "Bearer key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() handler.ServeHTTP(response, request) var failure v1.ErrorResponse diff --git a/services/agents-api/internal/api/environment_files_create.go b/services/agents-api/internal/api/environment_files_create.go index 9184b211a..e8e58836e 100644 --- a/services/agents-api/internal/api/environment_files_create.go +++ b/services/agents-api/internal/api/environment_files_create.go @@ -42,16 +42,16 @@ func WithEnvironmentFileWriter(writer EnvironmentFileWriter) Option { // @Failure 400,401,404,409,413,500,503 {object} v1.ErrorResponse // @Router /agents/environments/{environment_id}/files [post] func (h *Handler) createEnvironmentFile(w http.ResponseWriter, r *http.Request) { + const maxJSON = int64(((proto.WorkspaceWriteMaxBytes+2)/3)*4 + (16 << 10)) + raw, ok := readJSONObjectLimit(w, r, maxJSON, "Inline upload exceeds this service's bounded file limit.") + if !ok { + return + } environment, err := h.store.GetEnvironment(r.Context(), tenantID(r), chi.URLParam(r, "environment_id")) if err != nil { writeStoreError(w, r, err) return } - const maxJSON = int64(((proto.WorkspaceWriteMaxBytes+2)/3)*4 + (16 << 10)) - raw, ok := readJSONBodyLimit(w, r, maxJSON, "Inline upload exceeds this service's bounded file limit.") - if !ok { - return - } var request v1.EnvironmentFileCreateRequest fields := []string{"type", "path", "data", "file_id"} if field, found := unknownBodyField(raw, fields...); found { diff --git a/services/agents-api/internal/api/environment_files_create_test.go b/services/agents-api/internal/api/environment_files_create_test.go index 32d4f9749..df856f2f5 100644 --- a/services/agents-api/internal/api/environment_files_create_test.go +++ b/services/agents-api/internal/api/environment_files_create_test.go @@ -59,6 +59,7 @@ func requestCreateEnvironmentFile(h http.Handler, id, body, key string) *httptes r := httptest.NewRequest(http.MethodPost, "/v1/agents/environments/"+id+"/files", strings.NewReader(body)) r.Header.Set("Authorization", "Bearer "+key) r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(environmentFilesRecorder{w}, r) return w diff --git a/services/agents-api/internal/api/environment_files_wire_test.go b/services/agents-api/internal/api/environment_files_wire_test.go index 925627013..f05f3d3ed 100644 --- a/services/agents-api/internal/api/environment_files_wire_test.go +++ b/services/agents-api/internal/api/environment_files_wire_test.go @@ -161,9 +161,23 @@ func TestEnvironmentFileCreateFieldErrors(t *testing.T) { t.Fatal("rejected body was written", body) } } - // Validation without an official sample keeps the local code, including a - // malformed body whose first key is unknown. - for _, body := range []string{`{"foo":1,`, `{"type":"inline",`, `{"foo":1} {}`, `{"type":"inline","path":"/workspace/a"}`, `{"type":"inline","path":"/workspace/a","data":"?"}`, `{"type":"inline","path":"/workspace/a","data":"","file_id":"x"}`, `[]`} { + // The shared body gate rejects malformed bodies, including one whose first + // key is unknown, and non-object roots (HP-09, HP-12). + for body, message := range map[string]string{ + `{"foo":1,`: "Invalid body: failed to parse JSON value. Please check the value to ensure it is valid JSON. (Common errors include trailing commas, missing closing brackets, missing quotation marks, etc.)", + `{"type":"inline",`: "Invalid body: failed to parse JSON value. Please check the value to ensure it is valid JSON. (Common errors include trailing commas, missing closing brackets, missing quotation marks, etc.)", + `{"foo":1} {}`: "Invalid body: failed to parse JSON value. Please check the value to ensure it is valid JSON. (Common errors include trailing commas, missing closing brackets, missing quotation marks, etc.)", + `[]`: "Invalid type: expected an object, but got an array instead.", + } { + h, f := environmentFileCreateHandler(t) + w := requestCreateEnvironmentFile(h, f.environment.ID, body, "files-key") + assertListQueryError(t, w, "invalid_request_error", nil, message) + if f.writes != 0 { + t.Fatal("rejected body was written", body) + } + } + // Validation without an official sample keeps the local code. + for _, body := range []string{`{"type":"inline","path":"/workspace/a"}`, `{"type":"inline","path":"/workspace/a","data":"?"}`, `{"type":"inline","path":"/workspace/a","data":"","file_id":"x"}`} { h, f := environmentFileCreateHandler(t) w := requestCreateEnvironmentFile(h, f.environment.ID, body, "files-key") assertListQueryError(t, w, "invalid_request", nil, "Invalid resource identifier or request limits.") @@ -180,7 +194,6 @@ func TestEnvironmentFileCreateFieldErrors(t *testing.T) { `line\u2028separator`: false, `bell\u0007`: false, `caf\u00e9 \u5b57`: true, - "\xff\xfe": false, `\ud800`: false, `\ufffd`: false, } { @@ -198,8 +211,12 @@ func TestEnvironmentFileCreateFieldErrors(t *testing.T) { assertListQueryError(t, w, "invalid_request_error", nil, "Unknown parameter.") } } - // Foreign Environments stay missing before any body inspection. + // Invalid UTF-8 is rejected by the shared body gate (HP-10). h, f := environmentFileCreateHandler(t) + w := requestCreateEnvironmentFile(h, f.environment.ID, "{\"type\":\"inline\",\"path\":\"/workspace/a\",\"data\":\"\",\"\xff\xfe\":1}", "files-key") + assertListQueryError(t, w, "invalid_request_error", nil, "Invalid body: encountered a unicode decode error when parsing this JSON value. Please check the value to ensure it is valid unicode.") + // Foreign Environments stay missing before route-specific body validation. + h, f = environmentFileCreateHandler(t) body := `{"type":"inline","path":"/workspace/a","data":"","extra_field":1}` foreign := requestCreateEnvironmentFile(h, f.environment.ID, body, "other-key") missing := requestCreateEnvironmentFile(h, uuid.NewString(), body, "files-key") diff --git a/services/agents-api/internal/api/environment_input_test.go b/services/agents-api/internal/api/environment_input_test.go index 0258100c0..3397dc2fc 100644 --- a/services/agents-api/internal/api/environment_input_test.go +++ b/services/agents-api/internal/api/environment_input_test.go @@ -32,6 +32,7 @@ func TestPublicEnvironmentInputFailureMappings(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions/session/events", strings.NewReader(`{"events":[{"type":"agent.session.input.message","input":[{"role":"user","content":[{"type":"input_text","text":"Start"}]}]}]}`)) request.Header.Set("Authorization", "Bearer test-api-key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") request.Header.Set("Idempotency-Key", "retained-request") response := httptest.NewRecorder() handler.ServeHTTP(response, request) @@ -73,6 +74,7 @@ func TestPreparedEnvironmentInputWaitExtendsOnlyItsResponseDeadline(t *testing.T create := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(`{"agent":{"model":"MiniMax-M3"},"environment":`+environmentJSON+`,"input":"Prepare the response deadline fixture."}`)) create.Header.Set("Authorization", "Bearer key") create.Header.Set("OpenAI-Beta", "agents=v1") + create.Header.Set("Content-Type", "application/json") created := httptest.NewRecorder() handler.ServeHTTP(created, create) if created.Code != http.StatusCreated { @@ -94,6 +96,7 @@ func TestPreparedEnvironmentInputWaitExtendsOnlyItsResponseDeadline(t *testing.T } request.Header.Set("Authorization", "Bearer key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") type result struct { response *http.Response err error diff --git a/services/agents-api/internal/api/environment_templates.go b/services/agents-api/internal/api/environment_templates.go index ea2ee3f7e..d82edc8fc 100644 --- a/services/agents-api/internal/api/environment_templates.go +++ b/services/agents-api/internal/api/environment_templates.go @@ -70,7 +70,7 @@ func templateResponse(t store.EnvironmentTemplate) v1.EnvironmentTemplate { } func readTemplateInput(w http.ResponseWriter, r *http.Request) (store.EnvironmentTemplateInput, bool) { - raw, ok := readJSONBodyLimit(w, r, 16*1024*1024, "Request exceeds 16 MiB.") + raw, ok := readJSONObjectLimit(w, r, 16*1024*1024, "Request exceeds 16 MiB.") if !ok { return store.EnvironmentTemplateInput{}, false } diff --git a/services/agents-api/internal/api/environment_templates_test.go b/services/agents-api/internal/api/environment_templates_test.go index 15143375c..f9cc2f7b2 100644 --- a/services/agents-api/internal/api/environment_templates_test.go +++ b/services/agents-api/internal/api/environment_templates_test.go @@ -26,6 +26,7 @@ func TestTemplateConfigurationRejectsUnqualifiedInputs(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/v1/agents/environments/templates", strings.NewReader(`{"env":{"PATH":"confidential-canary"}}`)) req.Header.Set("Authorization", "Bearer test-api-key") req.Header.Set("OpenAI-Beta", "agents=v1") + req.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() h.ServeHTTP(response, req) if response.Code != http.StatusBadRequest || strings.Contains(response.Body.String(), "confidential-canary") { diff --git a/services/agents-api/internal/api/function_configuration_test.go b/services/agents-api/internal/api/function_configuration_test.go index 791f9b52f..c996a8856 100644 --- a/services/agents-api/internal/api/function_configuration_test.go +++ b/services/agents-api/internal/api/function_configuration_test.go @@ -18,6 +18,7 @@ func TestPublicFunctionConfiguration(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(`{"agent":{"model":"model"`+suffix+`},"environment":{"type":"none"},"input":"Use the configured function when needed."}`)) req.Header.Set("Authorization", "Bearer test-api-key") req.Header.Set("OpenAI-Beta", "agents=v1") + req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, req) if w.Code != 201 { @@ -57,6 +58,7 @@ func TestPublicFunctionConfigurationRejectsInvalidOrUnsupported(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(`{"agent":{"model":"model","tools":`+raw+`},"environment":{"type":"none"},"input":"Use the configured function when needed."}`)) req.Header.Set("Authorization", "Bearer test-api-key") req.Header.Set("OpenAI-Beta", "agents=v1") + req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, req) if w.Code != 400 || s.tenant != "" { diff --git a/services/agents-api/internal/api/function_inputs_test.go b/services/agents-api/internal/api/function_inputs_test.go index 5d7a6db82..5fb1414ed 100644 --- a/services/agents-api/internal/api/function_inputs_test.go +++ b/services/agents-api/internal/api/function_inputs_test.go @@ -19,6 +19,7 @@ func submitResultRequest(t *testing.T, body string, failure error) (*httptest.Re r := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions/session/events", strings.NewReader(body)) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, r) return w, recorder diff --git a/services/agents-api/internal/api/handler.go b/services/agents-api/internal/api/handler.go index cd4bd71d3..f7f023577 100644 --- a/services/agents-api/internal/api/handler.go +++ b/services/agents-api/internal/api/handler.go @@ -6,7 +6,6 @@ import ( "encoding/json" "errors" "fmt" - "io" "net/http" v1 "github.com/MiniMax-AI-Dev/parsar/contracts/agents-api/v1" @@ -180,7 +179,7 @@ func (h *Handler) routes() *chi.Mux { // @Failure 400,401,404,409,413,500,503 {object} v1.ErrorResponse // @Router /agents/sessions [post] func (h *Handler) createSession(w http.ResponseWriter, r *http.Request) { - raw, ok := readJSONBodyLimit(w, r, 16*1024*1024, "Request exceeds 16 MiB.") + raw, ok := readJSONObjectLimit(w, r, 16*1024*1024, "Request exceeds 16 MiB.") if !ok { return } @@ -194,10 +193,6 @@ func (h *Handler) createSession(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "invalid_request", "Request must be a JSON object containing supported fields.") return } - if err := decoder.Decode(new(any)); err != io.EOF { - writeError(w, http.StatusBadRequest, "invalid_request", "Request must contain exactly one JSON object.") - return - } input, err := request.validated() if err != nil { if !writeFieldError(w, err) { diff --git a/services/agents-api/internal/api/handler_test.go b/services/agents-api/internal/api/handler_test.go index 595938520..7244886b9 100644 --- a/services/agents-api/internal/api/handler_test.go +++ b/services/agents-api/internal/api/handler_test.go @@ -74,6 +74,7 @@ func TestHTTPConfigurationAndTenantIdentity(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions?tenant_id=untrusted-tenant", strings.NewReader(body)) request.Header.Set("Authorization", "Bearer test-api-key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") request.Header.Set("Idempotency-Key", "retry-key") request.Header.Set("X-Tenant-ID", "untrusted-tenant") w := httptest.NewRecorder() @@ -98,6 +99,7 @@ func TestSessionResponseReasoningKeysAreExplicit(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(`{"agent":{"model":"requested-model"},"environment":{"type":"none"},"input":"hello"}`)) request.Header.Set("Authorization", "Bearer test-api-key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, request) var body struct { @@ -135,7 +137,7 @@ func TestHTTPRejectsUntrustedOrUnsupportedRequests(t *testing.T) { {"unknown saved agent", "Bearer test-api-key", "agents=v1", "/v1/agents/sessions", strings.Replace(valid, `"agent":`, `"agent_id":"saved","agent":`, 1), 404}, {"unknown agent option", "Bearer test-api-key", "agents=v1", "/v1/agents/sessions", strings.Replace(valid, `"model":`, `"tools":[{}],"model":`, 1), 400}, {"multiple objects", "Bearer test-api-key", "agents=v1", "/v1/agents/sessions", valid + `{}`, 400}, - {"no object", "Bearer test-api-key", "agents=v1", "/v1/agents/sessions", `null`, 400}, + {"null body as empty object", "Bearer test-api-key", "agents=v1", "/v1/agents/sessions", `null`, 400}, {"large body", "Bearer test-api-key", "agents=v1", "/v1/agents/sessions", `{"agent":{"model":"` + strings.Repeat("x", 16*1024*1024) + `"}}`, 413}, } { t.Run(test.name, func(t *testing.T) { @@ -143,6 +145,7 @@ func TestHTTPRejectsUntrustedOrUnsupportedRequests(t *testing.T) { r := httptest.NewRequest(http.MethodPost, test.path, strings.NewReader(test.body)) r.Header.Set("Authorization", test.auth) r.Header.Set("OpenAI-Beta", test.beta) + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, r) var response v1.ErrorResponse diff --git a/services/agents-api/internal/api/harness_test.go b/services/agents-api/internal/api/harness_test.go index d560affee..5c1bb35a4 100644 --- a/services/agents-api/internal/api/harness_test.go +++ b/services/agents-api/internal/api/harness_test.go @@ -39,6 +39,7 @@ func TestSessionHarnessAdmission(t *testing.T) { r := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(`{"agent":{"model":"fixture"`+tc.extension+tc.extra+`},"environment":`+tc.environment+`,"input":"Run on the selected harness."}`)) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, r) if w.Code != tc.status || s.input.Engine != tc.engine { diff --git a/services/agents-api/internal/api/hosted_environment_test.go b/services/agents-api/internal/api/hosted_environment_test.go index a0ced24d1..04ea2e235 100644 --- a/services/agents-api/internal/api/hosted_environment_test.go +++ b/services/agents-api/internal/api/hosted_environment_test.go @@ -98,6 +98,7 @@ func TestHostedCreationUsesExecutionAdmission(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(body)) request.Header.Set("Authorization", "Bearer key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() handler.ServeHTTP(response, request) // The recorder deliberately rejects both creation methods. Its rejection diff --git a/services/agents-api/internal/api/inputs.go b/services/agents-api/internal/api/inputs.go index aeed6e20d..9a33d45de 100644 --- a/services/agents-api/internal/api/inputs.go +++ b/services/agents-api/internal/api/inputs.go @@ -1,10 +1,9 @@ package api import ( + "bytes" "context" "encoding/json" - "errors" - "io" "net/http" "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/proto" @@ -36,22 +35,17 @@ func WithExecution(s InputSubmitter) Option { return func(h *Handler) { h.inputs // @Failure 400,401,404,409,413,500,503 {object} v1.ErrorResponse // @Router /agents/sessions/{session_id}/events [post] func (h *Handler) createEvents(w http.ResponseWriter, r *http.Request) { + raw, ok := readJSONObject(w, r) + if !ok { + return + } var request struct { Events []json.RawMessage `json:"events"` } - decoder := json.NewDecoder(http.MaxBytesReader(w, r.Body, 1024*1024)) + decoder := json.NewDecoder(bytes.NewReader(raw)) decoder.DisallowUnknownFields() if err := decoder.Decode(&request); err != nil { - var large *http.MaxBytesError - if errors.As(err, &large) { - writeError(w, http.StatusRequestEntityTooLarge, "request_too_large", "Request exceeds 1 MiB.") - } else { - writeError(w, http.StatusBadRequest, "invalid_request", "Invalid Session input event request.") - } - return - } - if decoder.Decode(new(any)) != io.EOF { - writeError(w, http.StatusBadRequest, "invalid_request", "Request must contain exactly one JSON object.") + writeError(w, http.StatusBadRequest, "invalid_request", "Invalid Session input event request.") return } key := r.Header.Get("Idempotency-Key") diff --git a/services/agents-api/internal/api/inputs_test.go b/services/agents-api/internal/api/inputs_test.go index e5d4b53ce..64b807d13 100644 --- a/services/agents-api/internal/api/inputs_test.go +++ b/services/agents-api/internal/api/inputs_test.go @@ -31,6 +31,7 @@ func TestPublicInputAdmission(t *testing.T) { r := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions/session-id/events", strings.NewReader(body)) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") r.Header.Set("Idempotency-Key", "batch-key") r.Header.Set("X-Tenant-ID", "forged") w := httptest.NewRecorder() @@ -65,6 +66,7 @@ func TestPublicInputRejectsUnsupportedOrMalformedBatch(t *testing.T) { r := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions/id/events", strings.NewReader(body)) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, r) if w.Code != 400 || recorder.inputs != nil { @@ -90,6 +92,7 @@ func TestPublicInputWhitespaceTextAdmission(t *testing.T) { r := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions/session-id/events", strings.NewReader(body)) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, r) if w.Code != 202 || w.Body.Len() != 0 || len(recorder.inputs) != 1 || recorder.inputs[0].Kind != "message" { @@ -114,6 +117,7 @@ func TestPublicInputWhitespaceTextAdmission(t *testing.T) { r := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions/session-id/events", strings.NewReader(body)) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, r) if w.Code != 400 || recorder.inputs != nil || w.Body.String() != emptyInputError { diff --git a/services/agents-api/internal/api/json_request.go b/services/agents-api/internal/api/json_request.go index 636017861..d0b9ced48 100644 --- a/services/agents-api/internal/api/json_request.go +++ b/services/agents-api/internal/api/json_request.go @@ -1,11 +1,21 @@ package api import ( + "bytes" + "encoding/json" "errors" + "fmt" "io" "net/http" + "slices" + "strings" + "unicode/utf8" + + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/echotext" ) +// readJSONBody reads a bounded raw body. Agents API JSON routes use +// readJSONObject instead; DELETE and internal routes keep this reader. func readJSONBody(w http.ResponseWriter, r *http.Request) ([]byte, bool) { return readJSONBodyLimit(w, r, 1024*1024, "Request exceeds 1 MiB.") } @@ -23,3 +33,169 @@ func readJSONBodyLimit(w http.ResponseWriter, r *http.Request, limit int64, mess } return nil, false } + +// Official request body errors (HP-09..HP-15), reported with a null param. +var ( + errBodyContentType = &fieldError{message: "expected request with Content-Type: application/json"} + errBodyUnicode = &fieldError{message: "Invalid body: encountered a unicode decode error when parsing this JSON value. Please check the value to ensure it is valid unicode."} + errBodyParse = &fieldError{message: "Invalid body: failed to parse JSON value. Please check the value to ensure it is valid JSON. (Common errors include trailing commas, missing closing brackets, missing quotation marks, etc.)"} + // errBodyDuplicateKey omits a key or path that cannot be echoed; see echotext.Allowed. + errBodyDuplicateKey = &fieldError{message: "Invalid body: duplicate JSON key. Duplicate JSON keys are not supported."} +) + +// readJSONObject is the shared body gate of every Agents API JSON route. +func readJSONObject(w http.ResponseWriter, r *http.Request) ([]byte, bool) { + return readJSONObjectLimit(w, r, 1024*1024, "Request exceeds 1 MiB.") +} + +// readJSONObjectLimit runs before any route-specific decoding, validation or +// lookup. It requires a JSON Content-Type before reading, applies the route's +// body limit, then checks the whole body with the official parse semantics: +// valid UTF-8, exactly one JSON value, no repeated object key at any depth and +// an object root. A zero-length body or null becomes {}. Routes then decode +// the returned object with their own rules. +func readJSONObjectLimit(w http.ResponseWriter, r *http.Request, limit int64, message string) ([]byte, bool) { + if !jsonContentType(r.Header.Get("Content-Type")) { + writeFieldError(w, errBodyContentType) + return nil, false + } + raw, ok := readJSONBodyLimit(w, r, limit, message) + if !ok { + return nil, false + } + object, err := jsonObjectBody(raw) + if err != nil { + writeFieldError(w, err) + return nil, false + } + return object, true +} + +// jsonContentType accepts application/json and application/*+json media types +// case-insensitively, with any parameters. +func jsonContentType(value string) bool { + mediaType, _, _ := strings.Cut(value, ";") + mediaType = strings.ToLower(strings.TrimSpace(mediaType)) + if mediaType == "application/json" { + return true + } + subtype, ok := strings.CutPrefix(mediaType, "application/") + return ok && len(subtype) > len("+json") && strings.HasSuffix(subtype, "+json") +} + +func jsonObjectBody(raw []byte) ([]byte, error) { + if len(raw) == 0 { + return []byte(`{}`), nil + } + if !utf8.Valid(raw) { + return nil, errBodyUnicode + } + // A byte order mark, a whitespace-only body or trailing data is invalid JSON. + if !json.Valid(raw) { + return nil, errBodyParse + } + if key, path, found := duplicateJSONKey(raw); found { + if !echotext.Allowed(key) || !echotext.Allowed(path) { + return nil, errBodyDuplicateKey + } + return nil, &fieldError{message: fmt.Sprintf("Invalid body: duplicate JSON key '%s' at '%s'. Duplicate JSON keys are not supported.", key, path)} + } + switch kind := jsonValueKind(raw); kind { + case "an object": + return raw, nil + case "null": + return []byte(`{}`), nil + default: + // An array root is rejected too; the official service treats [] as {} (HP-14). + return nil, &fieldError{message: fmt.Sprintf("Invalid type: expected an object, but got %s instead.", kind)} + } +} + +// duplicateJSONKey returns the first repeated object key of a valid JSON value +// in document order, with its path of object keys joined by '.'. Array indices +// are omitted, as observed officially: 'metadata.k' and 'tools.type'. Keys are +// compared after unescaping. The scan is linear, and the path is built only for +// the reported key. +func duplicateJSONKey(raw []byte) (key, path string, found bool) { + type frame struct { + object bool + expectKey bool + keys [][]byte // the keys of a small object + set map[string]struct{} // every key once an object is large + member []byte // the key whose value is being read + } + var stack []frame + for i := 0; i < len(raw); { + switch raw[i] { + case '{', '[': + if len(stack) < cap(stack) { + // Reuse the key slice of an earlier sibling at this depth. + stack = stack[:len(stack)+1] + keys := stack[len(stack)-1].keys[:0] + stack[len(stack)-1] = frame{keys: keys} + } else { + stack = append(stack, frame{}) + } + top := &stack[len(stack)-1] + top.object, top.expectKey = raw[i] == '{', raw[i] == '{' + i++ + case '}', ']': + stack = stack[:len(stack)-1] + i++ + case ',': + if top := &stack[len(stack)-1]; top.object { + top.expectKey = true + } + i++ + case '"': + end := i + 1 + for { + end += bytes.IndexAny(raw[end:], "\\\"") + if raw[end] == '"' { + break + } + end += 2 + } + end++ + if len(stack) == 0 || !stack[len(stack)-1].expectKey { + i = end + continue + } + top := &stack[len(stack)-1] + name := raw[i+1 : end-1] + if bytes.IndexByte(name, '\\') >= 0 { + var decoded string + _ = json.Unmarshal(raw[i:end], &decoded) + name = []byte(decoded) + } + repeated := false + if top.set != nil { + _, repeated = top.set[string(name)] + top.set[string(name)] = struct{}{} + } else { + repeated = slices.ContainsFunc(top.keys, func(k []byte) bool { return bytes.Equal(k, name) }) + top.keys = append(top.keys, name) + if len(top.keys) > 16 { + top.set = make(map[string]struct{}, 32) + for _, k := range top.keys { + top.set[string(k)] = struct{}{} + } + } + } + if repeated { + var segments []string + for _, f := range stack[:len(stack)-1] { + if f.object { + segments = append(segments, string(f.member)) + } + } + return string(name), strings.Join(append(segments, string(name)), "."), true + } + top.member, top.expectKey = name, false + i = end + default: + i++ + } + } + return "", "", false +} diff --git a/services/agents-api/internal/api/json_request_test.go b/services/agents-api/internal/api/json_request_test.go new file mode 100644 index 000000000..0e21676bc --- /dev/null +++ b/services/agents-api/internal/api/json_request_test.go @@ -0,0 +1,406 @@ +package api + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/proto" + "github.com/google/uuid" +) + +// Official body messages (HP-09..HP-15). +const ( + bodyParseMessage = "Invalid body: failed to parse JSON value. Please check the value to ensure it is valid JSON. (Common errors include trailing commas, missing closing brackets, missing quotation marks, etc.)" + bodyUnicodeMessage = "Invalid body: encountered a unicode decode error when parsing this JSON value. Please check the value to ensure it is valid unicode." + bodyContentTypeMessage = "expected request with Content-Type: application/json" +) + +func duplicateKeyMessage(key, path string) string { + return "Invalid body: duplicate JSON key '" + key + "' at '" + path + "'. Duplicate JSON keys are not supported." +} + +func TestJSONObjectBodyChecks(t *testing.T) { + long := strings.Repeat("k", 257) + for body, message := range map[string]string{ + // B1 (HP-09). + `{"name":`: bodyParseMessage, + `{"name":"a"}x`: bodyParseMessage, + `{"name":"a"}{}`: bodyParseMessage, + "\ufeff{\"name\":\"a\"}": bodyParseMessage, + " \n ": bodyParseMessage, + `{"a":1,}`: bodyParseMessage, + `{"a":'b'}`: bodyParseMessage, + `nul`: bodyParseMessage, + `{"a":"\q"}`: bodyParseMessage, + strings.Repeat("[", 10001) + strings.Repeat("]", 10001): bodyParseMessage, + // B2 (HP-10): invalid UTF-8 anywhere, before any parse error. + "{\"name\":\"scan6-\xff\"}": bodyUnicodeMessage, + "{\"\xc3\x28\":1}": bodyUnicodeMessage, + "{\"name\":\xff": bodyUnicodeMessage, + "\xef\xbb": bodyUnicodeMessage, + // B3 (HP-11): object keys joined by '.', array indices omitted. + `{"name":"a","name":"b"}`: duplicateKeyMessage("name", "name"), + `{"metadata":{"k":"1","k":"2"}}`: duplicateKeyMessage("k", "metadata.k"), + `{"tools":[{"type":"function","type":"function"}]}`: duplicateKeyMessage("type", "tools.type"), + `{"a":[[{"b":1}],[{"c":{"d":1,"d":2}}]]}`: duplicateKeyMessage("d", "a.c.d"), + `{"a":1,"a":2}`: duplicateKeyMessage("a", "a"), + `{"x":{"k":1},"y":{"k":1},"x":2}`: duplicateKeyMessage("x", "x"), + `{"n":1e400,"s":"}","n":1}`: duplicateKeyMessage("n", "n"), + `{"":1,"":2}`: duplicateKeyMessage("", ""), + `{"":{"a":1,"a":2}}`: duplicateKeyMessage("a", ".a"), + `[{"a":1,"a":2}]`: duplicateKeyMessage("a", "a"), + `{"events":[{"input":[{"content":[{"text":"x","text":"y"}]}]}]}`: duplicateKeyMessage("text", "events.input.content.text"), + // Bounded echo (echotext.Allowed) for the key and its path. + `{"` + long + `":1,"` + long + `":2}`: "Invalid body: duplicate JSON key. Duplicate JSON keys are not supported.", + `{"` + long[:200] + `":{"` + long[:60] + `":1,"` + long[:60] + `":2}}`: "Invalid body: duplicate JSON key. Duplicate JSON keys are not supported.", + `{"tab\tkey":1,"tab\tkey":2}`: "Invalid body: duplicate JSON key. Duplicate JSON keys are not supported.", + `{"\ud800":1,"\ud800":2}`: "Invalid body: duplicate JSON key. Duplicate JSON keys are not supported.", + `{"<>":{"café":1,"café":2}}`: duplicateKeyMessage("café", "<>.café"), + // B4 (HP-12, HP-14). + `"scan6"`: "Invalid type: expected an object, but got a string instead.", + `5`: "Invalid type: expected an object, but got an integer instead.", + ` -1.5e3 `: "Invalid type: expected an object, but got a number instead.", + `true`: "Invalid type: expected an object, but got a boolean instead.", + `[]`: "Invalid type: expected an object, but got an array instead.", + `[{"model":"m"}]`: "Invalid type: expected an object, but got an array instead.", + } { + _, err := jsonObjectBody([]byte(body)) + field, ok := err.(*fieldError) + if !ok || field.param != "" || field.message != message { + t.Errorf("%.80q: got %v, want %q", body, err, message) + } + } + // Paths are built only for the reported key, so deep bodies with long keys + // stay linear in time and memory. + depth, key := 4000, strings.Repeat("k", 1000) + deep := strings.Repeat(`{"`+key+`":[`, depth) + `{"a":1,"a":2}` + strings.Repeat("]}", depth) + if _, err := jsonObjectBody([]byte(deep)); err != errBodyDuplicateKey { + t.Errorf("deep duplicate: %v", err) + } + deep = strings.Repeat(`{"b":[`, 100) + `{"a":1,"a":2}` + strings.Repeat("]}", 100) + if _, err := jsonObjectBody([]byte(deep)); err == nil || err.Error() != duplicateKeyMessage("a", strings.Repeat("b.", 100)+"a") { + t.Errorf("nested duplicate: %v", err) + } + // B5 (HP-13) and B7: null and a zero-length body are {}; objects are unchanged. + for body, want := range map[string]string{ + ``: `{}`, + `null`: `{}`, + " \tnull\r\n": `{}`, + `{}`: `{}`, + ` {"a":[{"b":1},{"b":2}],"c":{"b":3}} `: ` {"a":[{"b":1},{"b":2}],"c":{"b":3}} `, + `{"x_agents_core":{"model_provider":null},"metadata":{"k":"v"}}`: `{"x_agents_core":{"model_provider":null},"metadata":{"k":"v"}}`, + `{"name":"é😀"}`: `{"name":"é😀"}`, + } { + got, err := jsonObjectBody([]byte(body)) + if err != nil || string(got) != want { + t.Errorf("%q: got %q %v, want %q", body, got, err, want) + } + } +} + +func TestJSONContentType(t *testing.T) { + for value, want := range map[string]bool{ + "application/json": true, + "application/json; charset=utf-8": true, + "Application/JSON": true, + " application/json ;charset=UTF-8": true, + "application/merge-patch+json": true, + "APPLICATION/VND.API+JSON; x=y": true, + "": false, + "text/plain": false, + "application/x-www-form-urlencoded": false, + "multipart/form-data; boundary=x": false, + "text/json": false, + "application/jsonx": false, + "application/json-seq": false, + "application/+json": false, + "application/json+xml": false, + "application / json": false, + "application/octet-stream; t=+json": false, + "application/x-json-stream; t=a+json": false, + } { + if got := jsonContentType(value); got != want { + t.Errorf("%q: got %t, want %t", value, got, want) + } + } +} + +// jsonRoute is one Agents API JSON route family. The gate precedes any lookup, +// so the fixture store is never reached by a rejected body. +type jsonRoute struct{ name, path string } + +func agentsJSONRoutes() []jsonRoute { + id := uuid.NewString() + return []jsonRoute{ + {"agent create", "/v1/agents"}, + {"agent update", "/v1/agents/" + id}, + {"vault create", "/v1/vaults"}, + {"credential create", "/v1/vaults/" + id + "/credentials"}, + {"credential update", "/v1/vaults/" + id + "/credentials/" + id}, + {"template create", "/v1/agents/environments/templates"}, + {"template update", "/v1/agents/environments/templates/" + id}, + {"environment file create", "/v1/agents/environments/" + id + "/files"}, + {"session create", "/v1/agents/sessions"}, + {"session update", "/v1/agents/sessions/" + id}, + {"session events", "/v1/agents/sessions/" + id + "/events"}, + } +} + +func bodyGateRequest(h http.Handler, path, contentType string, body []byte, headers ...string) *httptest.ResponseRecorder { + var reader io.Reader = http.NoBody + if body != nil { + reader = bytes.NewReader(body) + } + r := httptest.NewRequest(http.MethodPost, path, reader) + r.Header.Set("Authorization", "Bearer test-api-key") + r.Header.Set("OpenAI-Beta", "agents=v1") + if contentType != "" { + r.Header.Set("Content-Type", contentType) + } + for i := 0; i+1 < len(headers); i += 2 { + r.Header.Set(headers[i], headers[i+1]) + } + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + return w +} + +func officialBodyError(message string) string { + encoded, _ := json.Marshal(message) + return `{"error":{"message":` + string(encoded) + `,"type":"invalid_request_error","code":"invalid_request_error","param":null}}` + "\n" +} + +// Every Agents API JSON route rejects B1-B4 and B6 before any decoding, lookup +// or write, with the official fields (HP-09..HP-15). +func TestAgentsJSONRoutesShareBodyGate(t *testing.T) { + cases := []struct { + name, contentType string + body []byte + message string + }{ + {"malformed", "application/json", []byte(`{"name":`), bodyParseMessage}, + {"trailing garbage", "application/json", []byte(`{"name":"a"}x`), bodyParseMessage}, + {"two objects", "application/json", []byte(`{"name":"a"}{}`), bodyParseMessage}, + {"bom", "application/json", []byte("\ufeff{}"), bodyParseMessage}, + {"whitespace", "application/json", []byte(" \n "), bodyParseMessage}, + {"invalid utf-8", "application/json", []byte("{\"name\":\"scan6-\xff\"}"), bodyUnicodeMessage}, + {"duplicate", "application/json", []byte(`{"name":"a","name":"b"}`), duplicateKeyMessage("name", "name")}, + {"nested duplicate", "application/json", []byte(`{"metadata":{"k":"1","k":"2"}}`), duplicateKeyMessage("k", "metadata.k")}, + {"string root", "application/json", []byte(`"scan6"`), "Invalid type: expected an object, but got a string instead."}, + {"array root", "application/json", []byte(`[]`), "Invalid type: expected an object, but got an array instead."}, + {"no content type", "", []byte(`{"name":"a"}`), bodyContentTypeMessage}, + {"text/plain", "text/plain", []byte(`{"name":"a"}`), bodyContentTypeMessage}, + {"form", "application/x-www-form-urlencoded", []byte(`{"name":"a"}`), bodyContentTypeMessage}, + {"bodyless without content type", "", nil, bodyContentTypeMessage}, + // B6 precedes the body limit. + {"large text/plain", "text/plain", bytes.Repeat([]byte(" "), 17<<20), bodyContentTypeMessage}, + } + for _, route := range agentsJSONRoutes() { + h, s := validationHandler(t) + for _, tc := range cases { + w := bodyGateRequest(h, route.path, tc.contentType, tc.body, "Idempotency-Key", "gate-key") + if w.Code != http.StatusBadRequest || w.Body.String() != officialBodyError(tc.message) { + t.Errorf("%s %s: %d %s", route.name, tc.name, w.Code, w.Body) + } + } + if s.writes != 0 { + t.Fatalf("%s: rejected body reached storage", route.name) + } + } +} + +// Authentication and Beta handling precede the gate; the body limit follows +// the Content-Type check and precedes parsing. +func TestAgentsJSONBodyGateOrder(t *testing.T) { + h, s := validationHandler(t) + for _, route := range agentsJSONRoutes() { + r := httptest.NewRequest(http.MethodPost, route.path, strings.NewReader(`{"name":`)) + r.Header.Set("Authorization", "Bearer test-api-key") + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + if w.Code != http.StatusBadRequest || strings.Contains(w.Body.String(), "Invalid body") { + t.Errorf("%s: missing Beta: %d %s", route.name, w.Code, w.Body) + } + r = httptest.NewRequest(http.MethodPost, route.path, strings.NewReader(`{"name":`)) + r.Header.Set("OpenAI-Beta", "agents=v1") + w = httptest.NewRecorder() + h.ServeHTTP(w, r) + if w.Code != http.StatusUnauthorized { + t.Errorf("%s: missing auth: %d %s", route.name, w.Code, w.Body) + } + // An oversized malformed body reports its route limit. + size := 16<<20 + 1 + if strings.HasSuffix(route.path, "/files") { + size = ((proto.WorkspaceWriteMaxBytes+2)/3)*4 + (16 << 10) + 1 + } + w = bodyGateRequest(h, route.path, "application/json", append([]byte(`{"name":`), bytes.Repeat([]byte("x"), size)...)) + if w.Code != http.StatusRequestEntityTooLarge { + t.Errorf("%s: oversized: %d %s", route.name, w.Code, w.Body) + } + } + if s.writes != 0 { + t.Fatal("rejected body reached storage") + } +} + +// B5 and B7: a zero-length body or null is {}, accepted JSON media types keep +// valid bodies unchanged, and route validation still follows the gate. +func TestAgentsJSONBodyGateKeepsValidBodies(t *testing.T) { + h, s := validationHandler(t) + param := func(value string) *string { return &value } + for _, body := range []string{``, `null`} { + assertConfigurationError(t, bodyGateRequest(h, "/v1/agents", "application/json", []byte(body)), "invalid_request_error", param("model"), "Missing required parameter: 'model'.") + assertConfigurationError(t, bodyGateRequest(h, "/v1/agents/sessions/"+uuid.NewString(), "application/json", []byte(body)), "invalid_request_error", nil, "At least one update field is required") + if w := bodyGateRequest(h, "/v1/agents/"+uuid.NewString(), "application/json", []byte(body)); w.Code != http.StatusOK { + t.Fatalf("empty Agent update %q: %d %s", body, w.Code, w.Body) + } + if w := bodyGateRequest(h, "/v1/vaults", "application/json", []byte(body)); w.Code != http.StatusCreated { + t.Fatalf("empty Vault create %q: %d %s", body, w.Code, w.Body) + } + } + if s.writes != 4 { + t.Fatalf("writes = %d", s.writes) + } + for _, contentType := range []string{"application/json", "application/json; charset=utf-8", "Application/JSON", "application/merge-patch+json"} { + if w := bodyGateRequest(h, "/v1/agents", contentType, []byte(`{"model":"m","name":"a","x_agents_core":{"model_provider":null}}`)); w.Code != http.StatusCreated { + t.Fatalf("%s: %d %s", contentType, w.Code, w.Body) + } + } + assertConfigurationError(t, bodyGateRequest(h, "/v1/agents", "application/json", []byte(`{"model":"m","tool_choice":"auto"}`)), "invalid_request_error", param("tool_choice"), "Unknown parameter: 'tool_choice'.") + assertConfigurationError(t, bodyGateRequest(h, "/v1/agents/"+uuid.NewString(), "application/json", []byte(`{"model":4}`)), "invalid_request_error", param("model"), "Invalid type for 'model': expected a string, but got an integer instead.") + if w := bodyGateRequest(h, "/v1/agents", "application/json", []byte(`{"model":"`+strings.Repeat("x", 1<<20)+`"}`)); w.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("limit: %d %s", w.Code, w.Body) + } +} + +// DELETE, multipart uploads and Core extension routes keep their own body +// handling without the Content-Type rule. +func TestBodyGateExcludedRoutes(t *testing.T) { + h, _ := validationHandler(t) + for _, path := range []string{"/v1/agents/" + uuid.NewString(), "/v1/vaults/" + uuid.NewString(), "/v1/agents/sessions/" + uuid.NewString()} { + r := httptest.NewRequest(http.MethodDelete, path, strings.NewReader(`{"name":`)) + r.Header.Set("Authorization", "Bearer test-api-key") + r.Header.Set("OpenAI-Beta", "agents=v1") + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + if w.Code != http.StatusBadRequest || strings.Contains(w.Body.String(), "Content-Type") || strings.Contains(w.Body.String(), "Invalid body") { + t.Errorf("DELETE %s: %d %s", path, w.Code, w.Body) + } + } + for _, request := range []struct{ path, contentType string }{ + {"/v1/files", "multipart/form-data; boundary=x"}, + {"/v1/skills", "multipart/form-data; boundary=x"}, + {"/core/v1/environments/" + uuid.NewString() + "/executor-credentials/", ""}, + } { + w := bodyGateRequest(h, request.path, request.contentType, []byte("--x--\r\n")) + if strings.Contains(w.Body.String(), bodyContentTypeMessage) || strings.Contains(w.Body.String(), "Invalid body") { + t.Errorf("POST %s: %d %s", request.path, w.Code, w.Body) + } + } +} + +// referenceDuplicateJSONKey walks encoding/json tokens; the byte scanner must +// agree with it. +func referenceDuplicateJSONKey(raw []byte) (string, string, bool) { + type container struct { + keys map[string]bool + member string + inside bool + } + decoder := json.NewDecoder(bytes.NewReader(raw)) + decoder.UseNumber() + var stack []*container + for { + token, err := decoder.Token() + if err != nil { + return "", "", false + } + if delim, ok := token.(json.Delim); ok && (delim == '}' || delim == ']') { + stack = stack[:len(stack)-1] + if len(stack) > 0 { + stack[len(stack)-1].inside = false + } + continue + } + var top *container + if len(stack) > 0 { + top = stack[len(stack)-1] + } + if top != nil && top.keys != nil && !top.inside { + key := token.(string) + if top.keys[key] { + var segments []string + for _, c := range stack[:len(stack)-1] { + if c.keys != nil { + segments = append(segments, c.member) + } + } + return key, strings.Join(append(segments, key), "."), true + } + top.keys[key], top.member, top.inside = true, key, true + continue + } + if delim, ok := token.(json.Delim); ok { + next := &container{} + if delim == '{' { + next.keys = map[string]bool{} + } + stack = append(stack, next) + } else if top != nil { + top.inside = false + } + } +} + +func TestDuplicateJSONKeyMatchesReference(t *testing.T) { + names := []string{`a`, `b`, `a`, `a\"b`, `a\\b`, ``, `k,:{}[]`, `\ud800`, `�`, `é`, `é`} + values := []string{`1`, `-2.5e3`, `true`, `null`, `"x,y:{}[]"`, `"\"a\":1"`, `"\\"`, `[]`, `{}`} + random := uint64(1) + next := func(n int) int { + random = random*6364136223846793005 + 1442695040888963407 + return int(random>>33) % n + } + var value func(depth int) string + value = func(depth int) string { + switch choice := next(4); { + case depth > 3 || choice == 0: + return values[next(len(values))] + case choice == 1: + items := make([]string, next(4)) + for i := range items { + items[i] = value(depth + 1) + } + return "[" + strings.Join(items, ",") + "]" + default: + members := make([]string, next(6)) + for i := range members { + members[i] = `"` + names[next(len(names))] + fmt.Sprint(next(3)) + `": ` + value(depth+1) + } + return "{" + strings.Join(members, ",") + "}" + } + } + duplicates := 0 + for range 20000 { + body := []byte(value(0)) + if !json.Valid(body) { + t.Fatalf("invalid generated body %s", body) + } + key, path, found := duplicateJSONKey(body) + wantKey, wantPath, wantFound := referenceDuplicateJSONKey(body) + if key != wantKey || path != wantPath || found != wantFound { + t.Fatalf("%s: got %q %q %t, want %q %q %t", body, key, path, found, wantKey, wantPath, wantFound) + } + if found { + duplicates++ + } + } + if duplicates < 1000 || duplicates > 19000 { + t.Fatalf("unbalanced generated bodies: %d duplicates", duplicates) + } +} diff --git a/services/agents-api/internal/api/resource_query_test.go b/services/agents-api/internal/api/resource_query_test.go index 75817eac1..dd016ff65 100644 --- a/services/agents-api/internal/api/resource_query_test.go +++ b/services/agents-api/internal/api/resource_query_test.go @@ -108,6 +108,7 @@ func TestSingleResourceRoutesIgnoreUnknownQueryKeys(t *testing.T) { r := httptest.NewRequest(route.method, route.path+query, strings.NewReader(route.body)) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, r) if w.Code != http.StatusNotFound { @@ -215,6 +216,7 @@ func TestEnvironmentFileCreateIgnoresUnknownQueryKeys(t *testing.T) { r := httptest.NewRequest(http.MethodPost, "/v1/agents/environments/"+id+"/files"+query, strings.NewReader(body)) r.Header.Set("Authorization", "Bearer "+key) r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(environmentFilesRecorder{w}, r) return w diff --git a/services/agents-api/internal/api/routing_test.go b/services/agents-api/internal/api/routing_test.go index 9d7ba643a..6024dc172 100644 --- a/services/agents-api/internal/api/routing_test.go +++ b/services/agents-api/internal/api/routing_test.go @@ -137,8 +137,9 @@ func routingHeaders(pairs ...string) http.Header { // project and beta are the pinned SDK's headers. var ( - project = []string{"Authorization", "Bearer " + routingKey} - beta = []string{"OpenAI-Beta", "agents=v1"} + project = []string{"Authorization", "Bearer " + routingKey} + beta = []string{"OpenAI-Beta", "agents=v1"} + jsonBody = []string{"Content-Type", "application/json"} ) func withHeaders(parts ...[]string) http.Header { @@ -231,12 +232,13 @@ func TestNonCanonicalPathsServeTheCleanRoute(t *testing.T) { 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) + posted := withHeaders(project, beta, jsonBody) + updated := serve(handler, http.MethodPost, "/v1//agents/"+id, `{"metadata":{"route":"double-slash"}}`, posted) 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) { + created := serve(handler, http.MethodPost, "/v1//agents", `{}`, posted) + if want := serve(handler, http.MethodPost, "/v1/agents", `{}`, posted); 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) @@ -256,7 +258,7 @@ func TestNonCanonicalPathsServeTheCleanRoute(t *testing.T) { 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 { + if got := serve(handler, http.MethodPost, "/v1/agents/", `{"model":"fixture"}`, withHeaders(project, beta, jsonBody)); got.Code != http.StatusNotFound { t.Fatalf("trailing-slash create = %d %s", got.Code, got.Body) } } diff --git a/services/agents-api/internal/api/sandbox_selector_test.go b/services/agents-api/internal/api/sandbox_selector_test.go index 33c16e1b8..f08227249 100644 --- a/services/agents-api/internal/api/sandbox_selector_test.go +++ b/services/agents-api/internal/api/sandbox_selector_test.go @@ -41,6 +41,7 @@ func TestSandboxSelectorUsesOnlyCoreSessionExtension(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(tc.body)) request.Header.Set("Authorization", "Bearer key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() handler.ServeHTTP(response, request) if tc.accepted { diff --git a/services/agents-api/internal/api/session_admission_test.go b/services/agents-api/internal/api/session_admission_test.go index 78ca38fcd..a9f62344d 100644 --- a/services/agents-api/internal/api/session_admission_test.go +++ b/services/agents-api/internal/api/session_admission_test.go @@ -29,6 +29,7 @@ func TestSessionAdmissionRejectsBeforeResourceOrExecutionAccess(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(body)) request.Header.Set("Authorization", "Bearer "+token) request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") request.Header.Set("Idempotency-Key", "retained-creation-key") response := httptest.NewRecorder() handler.ServeHTTP(response, request) @@ -55,6 +56,7 @@ func TestSessionEmptyUpdateRejectsBeforeResourceAccess(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions/unknown", strings.NewReader(`{}`)) request.Header.Set("Authorization", "Bearer "+token) request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() handler.ServeHTTP(response, request) if token == "invalid" { diff --git a/services/agents-api/internal/api/session_creation_stream_test.go b/services/agents-api/internal/api/session_creation_stream_test.go index dce8d8d57..fc34143c3 100644 --- a/services/agents-api/internal/api/session_creation_stream_test.go +++ b/services/agents-api/internal/api/session_creation_stream_test.go @@ -209,6 +209,7 @@ func (h *creationStreamHarness) open(method, path, body string) (<-chan sseFrame } request.Header.Set("Authorization", "Bearer key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") request.Header.Set("Idempotency-Key", "creation") response, err := h.server.Client().Do(request) if err != nil { @@ -618,6 +619,7 @@ func TestCreationRetryStreamEndsImmediately(t *testing.T) { } request.Header.Set("Authorization", "Bearer key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") request.Header.Set("Idempotency-Key", "creation") response, err := h.server.Client().Do(request) if err != nil { @@ -718,6 +720,7 @@ func TestCreationStreamCapabilityIsCheckedBeforeCreation(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(creationBody)) request.Header.Set("Authorization", "Bearer key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() handler.ServeHTTP(response, request) if response.Code != http.StatusServiceUnavailable || admission.calls.Load() != 0 { diff --git a/services/agents-api/internal/api/session_environment_http_test.go b/services/agents-api/internal/api/session_environment_http_test.go index 6aea2dc2f..c7edb366a 100644 --- a/services/agents-api/internal/api/session_environment_http_test.go +++ b/services/agents-api/internal/api/session_environment_http_test.go @@ -75,6 +75,7 @@ func TestSelfHostedSessionHTTPReadListMetadataAndLiveStream(t *testing.T) { r.Host = "untrusted-host.example" r.Header.Set("Authorization", "Bearer key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() handler.ServeHTTP(w, r) if w.Code != http.StatusOK { diff --git a/services/agents-api/internal/api/session_initial_input_test.go b/services/agents-api/internal/api/session_initial_input_test.go index a4421b315..44b39119b 100644 --- a/services/agents-api/internal/api/session_initial_input_test.go +++ b/services/agents-api/internal/api/session_initial_input_test.go @@ -16,6 +16,7 @@ func TestInitialInputUsesExecutionAdmissionAndSharedMessageValidation(t *testing r := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(`{"agent":{"model":"test-model"},"environment":{"type":"none"},"input":`+value+`}`)) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") r.Header.Set("Idempotency-Key", "create-key") w := httptest.NewRecorder() h.ServeHTTP(w, r) @@ -76,6 +77,7 @@ func createWithInput(h http.Handler, input string) *httptest.ResponseRecorder { r := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(`{"agent":{"model":"test-model"},"environment":{"type":"none"},"input":`+input+`}`)) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, r) return w diff --git a/services/agents-api/internal/api/session_metadata.go b/services/agents-api/internal/api/session_metadata.go index d73b96e73..2fbe81617 100644 --- a/services/agents-api/internal/api/session_metadata.go +++ b/services/agents-api/internal/api/session_metadata.go @@ -26,7 +26,7 @@ import ( // @Failure 400,401,404,413,500 {object} v1.ErrorResponse // @Router /agents/sessions/{session_id} [post] func (h *Handler) updateSession(w http.ResponseWriter, r *http.Request) { - raw, ok := readJSONBody(w, r) + raw, ok := readJSONObject(w, r) if !ok { return } @@ -70,6 +70,7 @@ func (h *Handler) updateSession(w http.ResponseWriter, r *http.Request) { // metadataTypeError reports the first non-string value of a request body's // top-level metadata object in document order, before generic body decoding // can reject it. Other body and metadata shapes keep their existing errors. +// The shared body gate has already rejected repeated keys. func metadataTypeError(body []byte) error { var fields map[string]json.RawMessage if json.Unmarshal(body, &fields) != nil { @@ -79,11 +80,6 @@ func metadataTypeError(body []byte) error { if token, err := decoder.Token(); err != nil || token != json.Delim('{') { return nil } - // A duplicate key keeps its first position and is checked with its last value - // only. Typed decoding rejects a non-string at any occurrence, so a body whose - // earlier duplicate is not a string falls back to the generic decoding error. - var keys []string - values := map[string]json.RawMessage{} for decoder.More() { token, err := decoder.Token() key, isKey := token.(string) @@ -91,13 +87,7 @@ func metadataTypeError(body []byte) error { if err != nil || !isKey || decoder.Decode(&value) != nil { return nil } - if _, seen := values[key]; !seen { - keys = append(keys, key) - } - values[key] = value - } - for _, key := range keys { - if kind := jsonValueKind(values[key]); kind != "a string" { + if kind := jsonValueKind(value); kind != "a string" { return &fieldError{param: "metadata." + key, message: fmt.Sprintf("Invalid type for 'metadata.%s': expected a string, but got %s instead.", key, kind)} } } diff --git a/services/agents-api/internal/api/session_request_test.go b/services/agents-api/internal/api/session_request_test.go index ae0f61f54..fe2fa633e 100644 --- a/services/agents-api/internal/api/session_request_test.go +++ b/services/agents-api/internal/api/session_request_test.go @@ -44,6 +44,7 @@ func TestSessionCreateFieldPresence(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(body)) request.Header.Set("Authorization", "Bearer test-api-key") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() handler.ServeHTTP(response, request) if response.Code != tc.status { diff --git a/services/agents-api/internal/api/session_semantics_test.go b/services/agents-api/internal/api/session_semantics_test.go index e82810070..bb8ffa076 100644 --- a/services/agents-api/internal/api/session_semantics_test.go +++ b/services/agents-api/internal/api/session_semantics_test.go @@ -43,6 +43,7 @@ func TestEmptyEventBatchAuthorizesWithoutExecutionEffects(t *testing.T) { r := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions/"+id+"/events", strings.NewReader(`{"events":[]}`)) r.Header.Set("Authorization", "Bearer "+token) r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") r.Header.Set("Idempotency-Key", key) r.Header.Set("X-Tenant-ID", "forged") w := httptest.NewRecorder() @@ -71,6 +72,7 @@ func TestEmptyEventBatchAuthorizesWithoutExecutionEffects(t *testing.T) { r := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions/owned/events", strings.NewReader(`{"events":[{"type":"agent.session.input.cancel"}]}`)) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") r.Header.Set("Idempotency-Key", "same-key") w := httptest.NewRecorder() h.ServeHTTP(w, r) diff --git a/services/agents-api/internal/api/subagents_test.go b/services/agents-api/internal/api/subagents_test.go index 0ce549b30..736579b66 100644 --- a/services/agents-api/internal/api/subagents_test.go +++ b/services/agents-api/internal/api/subagents_test.go @@ -268,6 +268,7 @@ func TestSubagentRoutesExposeOnlyOfficialReads(t *testing.T) { r := httptest.NewRequest(method, "/v1/agents/sessions/session/subagents"+route.path, nil) r.Header.Set("Authorization", "Bearer test-api-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.ServeHTTP(w, r) if w.Code != http.StatusMethodNotAllowed || s.calls != 0 { diff --git a/services/agents-api/internal/api/text_configuration_test.go b/services/agents-api/internal/api/text_configuration_test.go index 79221d2c3..78ee6d2e8 100644 --- a/services/agents-api/internal/api/text_configuration_test.go +++ b/services/agents-api/internal/api/text_configuration_test.go @@ -24,6 +24,7 @@ func TestTextConfigurationHTTP(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(`{"agent":{"model":"example"`+tc.text+`},"environment":{"type":"none"},"input":"Describe the configured response format."}`)) req.Header.Set("Authorization", "Bearer test-api-key") req.Header.Set("OpenAI-Beta", "agents=v1") + req.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() h.ServeHTTP(response, req) if response.Code != 201 { @@ -48,6 +49,7 @@ func TestTextConfigurationHTTP(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(`{"agent":{"model":"example","text":`+invalid+`},"environment":{"type":"none"},"input":"Describe the configured response format."}`)) req.Header.Set("Authorization", "Bearer test-api-key") req.Header.Set("OpenAI-Beta", "agents=v1") + req.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() h.ServeHTTP(response, req) if response.Code != 400 || s.tenant != "" { diff --git a/services/agents-api/internal/api/vaults.go b/services/agents-api/internal/api/vaults.go index 217a2255d..90b547435 100644 --- a/services/agents-api/internal/api/vaults.go +++ b/services/agents-api/internal/api/vaults.go @@ -32,7 +32,7 @@ type VaultStore interface { // @Failure 400,401,413,500 {object} v1.ErrorResponse // @Router /vaults [post] func (h *Handler) createVault(w http.ResponseWriter, r *http.Request) { - raw, ok := readJSONBody(w, r) + raw, ok := readJSONObject(w, r) if !ok { return } diff --git a/services/agents-api/internal/api/vaults_test.go b/services/agents-api/internal/api/vaults_test.go index e3a5d01f5..2d2a3bef4 100644 --- a/services/agents-api/internal/api/vaults_test.go +++ b/services/agents-api/internal/api/vaults_test.go @@ -62,6 +62,7 @@ func vaultRequest(h http.Handler, method, path, body string) *httptest.ResponseR r := httptest.NewRequest(method, path, strings.NewReader(body)) r.Header.Set("Authorization", "Bearer vault-key") r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") r.Header.Set("X-Tenant-ID", "untrusted-tenant") w := httptest.NewRecorder() h.ServeHTTP(w, r) @@ -75,6 +76,9 @@ func TestVaultResourceProjectionWithoutExecution(t *testing.T) { metadata map[string]any }{ {`{}`, nil, map[string]any{}}, + // A zero-length body or null is {} (HP-13). + {``, nil, map[string]any{}}, + {`null`, nil, map[string]any{}}, {`{"metadata":null}`, nil, map[string]any{}}, {`{"name":" 凭据库 \n","metadata":{"team":"engineering"}}`, "凭据库", map[string]any{"team": "engineering"}}, {`{"name":" ` + strings.Repeat("界", 85) + `x "}`, strings.Repeat("界", 85) + "x", map[string]any{}}, @@ -98,7 +102,7 @@ func TestVaultResourceProjectionWithoutExecution(t *testing.T) { func TestVaultResourceInvalidRequestsDoNotReachStore(t *testing.T) { for _, body := range []string{ - `null`, `[]`, `{} {}`, `{"name":null}`, `{"name":1}`, `{"name":""}`, `{"name":" \n\t "}`, + `[]`, `{} {}`, `{"name":null}`, `{"name":1}`, `{"name":""}`, `{"name":" \n\t "}`, `{"name":"` + strings.Repeat("界", 85) + `xx"}`, `{"metadata":[]}`, `{"metadata":{"key":null}}`, `{"metadata":{"key":1}}`, `{"tenant_id":"untrusted"}`, `{"credentials":[]}`, } { diff --git a/services/agents-api/internal/store/agent_execution_defaults_http_test.go b/services/agents-api/internal/store/agent_execution_defaults_http_test.go index 1ba42b3d6..a30e6faac 100644 --- a/services/agents-api/internal/store/agent_execution_defaults_http_test.go +++ b/services/agents-api/internal/store/agent_execution_defaults_http_test.go @@ -40,6 +40,7 @@ func TestAgentExecutionDefaultsPublicSnapshotAndPrecedence(t *testing.T) { r := httptest.NewRequest(method, path, strings.NewReader(body)) r.Header.Set("Authorization", "Bearer "+token) r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") r.Header.Set("Idempotency-Key", key) w := httptest.NewRecorder() handler.ServeHTTP(w, r) diff --git a/services/agents-api/internal/store/configuration_validation_public_test.go b/services/agents-api/internal/store/configuration_validation_public_test.go index d37b5e2b0..bb8158c9a 100644 --- a/services/agents-api/internal/store/configuration_validation_public_test.go +++ b/services/agents-api/internal/store/configuration_validation_public_test.go @@ -81,10 +81,11 @@ func TestAgentConfigurationValidationRejectsWithoutWritesPostgres(t *testing.T) {"X01", `"tool_choice":"auto"`, "{p}tool_choice", "Unknown parameter: '{p}tool_choice'."}, {"M01", `"multi_agent":{}`, "{p}multi_agent.enabled", "Missing required parameter: '{p}multi_agent.enabled'."}, {"M02", `"multi_agent":{"enabled":true,"max_concurrent_subagents":0}`, "{p}multi_agent.max_concurrent_subagents", "Invalid '{p}multi_agent.max_concurrent_subagents': integer below minimum value. Expected a value >= 1, but got 0 instead."}, - // Repeated members would merge when decoded, and names match case-insensitively (local message). - {"merged text", `"text":{"format":{"type":"json_schema","schema":{"type":"array"}}},"text":{"verbosity":"low"}`, "{p}text", "Duplicate parameter: '{p}text'."}, - {"merged reasoning", `"reasoning":{"Effort":"high"},"reasoning":{}`, "{p}reasoning", "Duplicate parameter: '{p}reasoning'."}, - {"merged location", `"tools":[{"type":"web_search","mode":"disabled","location":{"city":"Paris"},"location":{"country":null}}]`, "{p}tools[0].location", "Duplicate parameter: '{p}tools[0].location'."}, + // Repeated members would merge when decoded: the shared body gate rejects + // them with a null param (HP-11). Names match case-insensitively when decoded. + {"merged text", `"text":{"format":{"type":"json_schema","schema":{"type":"array"}}},"text":{"verbosity":"low"}`, "", "Invalid body: duplicate JSON key 'text' at '{p}text'. Duplicate JSON keys are not supported."}, + {"merged reasoning", `"reasoning":{"Effort":"high"},"reasoning":{}`, "", "Invalid body: duplicate JSON key 'reasoning' at '{p}reasoning'. Duplicate JSON keys are not supported."}, + {"merged location", `"tools":[{"type":"web_search","mode":"disabled","location":{"city":"Paris"},"location":{"country":null}}]`, "", "Invalid body: duplicate JSON key 'location' at '{p}tools.location'. Duplicate JSON keys are not supported."}, {"case variant", `"reasoning":{"effort":"high","EFFORT":"max"}`, "{p}reasoning.EFFORT", "Unknown parameter: '{p}reasoning.EFFORT'."}, } expect := func(tc struct{ name, fields, param, message string }, prefix string) string { diff --git a/services/agents-api/internal/store/creation_stream_settlement_public_test.go b/services/agents-api/internal/store/creation_stream_settlement_public_test.go index 4d135d04a..5d1305a41 100644 --- a/services/agents-api/internal/store/creation_stream_settlement_public_test.go +++ b/services/agents-api/internal/store/creation_stream_settlement_public_test.go @@ -36,6 +36,7 @@ func openStream(t *testing.T, server *httptest.Server, token, method, path, body } request.Header.Set("Authorization", "Bearer "+token) request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") if key != "" { request.Header.Set("Idempotency-Key", key) } diff --git a/services/agents-api/internal/store/environment_file_write_semantics_public_test.go b/services/agents-api/internal/store/environment_file_write_semantics_public_test.go index 4c51697dd..7968732c2 100644 --- a/services/agents-api/internal/store/environment_file_write_semantics_public_test.go +++ b/services/agents-api/internal/store/environment_file_write_semantics_public_test.go @@ -82,6 +82,7 @@ func TestEnvironmentFileCreateRejectionsLeaveNoReceiptOrConsumption(t *testing.T r, _ := http.NewRequest(http.MethodPost, server.URL+"/v1/agents/environments/"+id+"/files", strings.NewReader(body)) r.Header.Set("Authorization", "Bearer "+key) r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") resp, err := server.Client().Do(r) if err != nil { done <- fileCreateResponse{} diff --git a/services/agents-api/internal/store/harness_onboarding_test.go b/services/agents-api/internal/store/harness_onboarding_test.go index a35252648..7318ccd97 100644 --- a/services/agents-api/internal/store/harness_onboarding_test.go +++ b/services/agents-api/internal/store/harness_onboarding_test.go @@ -73,6 +73,7 @@ func TestThirdHarnessPublicOnboarding(t *testing.T) { req := httptest.NewRequest(method, path, strings.NewReader(body)) req.Header.Set("Authorization", "Bearer "+token) req.Header.Set("OpenAI-Beta", "agents=v1") + req.Header.Set("Content-Type", "application/json") res := httptest.NewRecorder() handler.ServeHTTP(res, req) if res.Code != status { diff --git a/services/agents-api/internal/store/initial_files_http_test.go b/services/agents-api/internal/store/initial_files_http_test.go index bb65673fe..d080d55fe 100644 --- a/services/agents-api/internal/store/initial_files_http_test.go +++ b/services/agents-api/internal/store/initial_files_http_test.go @@ -47,6 +47,7 @@ func TestInitialFilesHTTPInlineLimitsAndRetry(t *testing.T) { r := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", bytes.NewReader(body)) r.Header.Set("Authorization", "Bearer "+token) r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") r.Header.Set("Idempotency-Key", key) w := httptest.NewRecorder() handler.ServeHTTP(w, r) diff --git a/services/agents-api/internal/store/remote_mcp_credentials_test.go b/services/agents-api/internal/store/remote_mcp_credentials_test.go index 86e4737e1..4ef583fc7 100644 --- a/services/agents-api/internal/store/remote_mcp_credentials_test.go +++ b/services/agents-api/internal/store/remote_mcp_credentials_test.go @@ -46,6 +46,7 @@ func TestSelfHostedServiceMCPRejectionDoesNotRequireCredentialDecryption(t *test request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(string(raw))) request.Header.Set("Authorization", "Bearer test-token") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() handler.ServeHTTP(response, request) if response.Code != http.StatusBadRequest || strings.Contains(response.Body.String(), "synthetic-token") || strings.Contains(response.Body.String(), "ciphertext") || strings.Contains(response.Body.String(), "mcp_credentials") { diff --git a/services/agents-api/internal/store/remote_mcp_test.go b/services/agents-api/internal/store/remote_mcp_test.go index 740decca7..595bef037 100644 --- a/services/agents-api/internal/store/remote_mcp_test.go +++ b/services/agents-api/internal/store/remote_mcp_test.go @@ -44,6 +44,7 @@ func TestSelfHostedServiceMCPRejectedWithoutWrites(t *testing.T) { request := httptest.NewRequest(http.MethodPost, "/v1/agents/sessions", strings.NewReader(string(raw))) request.Header.Set("Authorization", "Bearer test-token") request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() handler.ServeHTTP(response, request) diff --git a/services/agents-api/internal/store/request_body_public_test.go b/services/agents-api/internal/store/request_body_public_test.go new file mode 100644 index 000000000..172cc2c5c --- /dev/null +++ b/services/agents-api/internal/store/request_body_public_test.go @@ -0,0 +1,143 @@ +package store_test + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/device" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/api" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/credentialcrypto" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" + "github.com/google/uuid" +) + +// Every Agents API JSON route checks its body in the shared gate before any +// lookup or write (HP-09..HP-15). Bodies that would otherwise write, such as an +// invalid UTF-8 name stored as U+FFFD, a last-value-wins duplicate or an update +// sent as text/plain, change nothing for the owner or another tenant. +func TestRequestBodyGateRejectsWithoutWritesPostgres(t *testing.T) { + // An isolated database keeps the no-write digest independent of other tests. + _, pool := store.NewManagedTestStore(t) + cipher, err := credentialcrypto.New(bytes.Repeat([]byte{64}, 32)) + if err != nil { + t.Fatal(err) + } + s := store.NewWithCredentialCipher(pool, cipher) + owner, foreign, ownerTenant := uuid.NewString(), uuid.NewString(), uuid.NewString() + auth, err := api.NewAuthenticator([]api.APIKey{ + {OrganizationID: "test-org", ProjectID: uuid.NewString(), SubjectKind: "service_account", SubjectID: "body-owner", TokenSHA256: device.HashCredential(owner), TenantID: ownerTenant}, + {OrganizationID: "test-org", ProjectID: uuid.NewString(), SubjectKind: "service_account", SubjectID: "body-foreign", TokenSHA256: device.HashCredential(foreign), TenantID: uuid.NewString()}, + }) + if err != nil { + t.Fatal(err) + } + h, err := api.NewHandler(s, auth, "codex", api.WithExecution(s)) + if err != nil { + t.Fatal(err) + } + server := httptest.NewServer(h) + defer server.Close() + client := pathIDClient{t: t, server: server} + + agent := client.created(owner, "/v1/agents", `{"model":"body-model","name":"body-agent","metadata":{"k":"v"}}`) + vault := client.created(owner, "/v1/vaults", `{"name":"body-vault"}`) + credential := client.created(owner, "/v1/vaults/"+vault+"/credentials", `{"name":"body","auth":{"type":"static_bearer","mcp_server_url":"https://mcp.example/mcp","token":"body-token"}}`) + template := client.created(owner, "/v1/agents/environments/templates", `{"name":"body-template"}`) + session := client.created(owner, "/v1/agents/sessions", `{"agent":{"model":"body-model"},"environment":{"type":"none"},"input":"Keep this Session.","metadata":{"k":"v"}}`) + prepared, err := s.CreateSession(t.Context(), ownerTenant, store.CreateSessionInput{Creator: store.FixtureCreator(), Engine: "codex", IdempotencyKey: "body-environment", + Configuration: json.RawMessage(`{"agent":{"model":"body-model"},"environment":{"type":"self_hosted","workspace_directory":"/workspace","capability_directories":[]}}`)}) + if err != nil || prepared.Environment == nil { + t.Fatal("fixture Environment", err) + } + before := databaseDigest(t, pool) + + // Each valid body would write; the single "gate" string sits at the + // dotted object-key path at. + routes := []struct{ path, body, at string }{ + {"/v1/agents", `{"model":"m","name":"gate"}`, "name"}, + {"/v1/agents/" + agent, `{"metadata":{"k":"gate"}}`, "metadata.k"}, + {"/v1/vaults", `{"name":"gate"}`, "name"}, + {"/v1/vaults/" + vault + "/credentials", `{"name":"body","auth":{"type":"static_bearer","mcp_server_url":"https://mcp.example/mcp","token":"gate"}}`, "auth.token"}, + {"/v1/vaults/" + vault + "/credentials/" + credential, `{"auth":{"type":"static_bearer","token":"gate"}}`, "auth.token"}, + {"/v1/agents/environments/templates", `{"name":"gate"}`, "name"}, + {"/v1/agents/environments/templates/" + template, `{"name":"gate"}`, "name"}, + {"/v1/agents/environments/" + prepared.Environment.ID + "/files", `{"type":"inline","path":"/workspace/body","data":"gate"}`, "data"}, + {"/v1/agents/sessions", `{"agent":{"model":"m"},"environment":{"type":"none"},"input":"gate"}`, "input"}, + {"/v1/agents/sessions/" + session, `{"metadata":{"k":"gate"}}`, "metadata.k"}, + {"/v1/agents/sessions/" + session + "/events", `{"events":[{"type":"agent.session.input.message","input":[{"role":"user","content":[{"type":"input_text","text":"gate"}]}]}]}`, "events.input.content.text"}, + } + official := func(message string) string { + encoded, _ := json.Marshal(message) + return `{"error":{"message":` + string(encoded) + `,"type":"invalid_request_error","code":"invalid_request_error","param":null}}` + "\n" + } + parse := official("Invalid body: failed to parse JSON value. Please check the value to ensure it is valid JSON. (Common errors include trailing commas, missing closing brackets, missing quotation marks, etc.)") + contentType := official("expected request with Content-Type: application/json") + for _, route := range routes { + lastKey := route.at[strings.LastIndex(route.at, ".")+1:] + for _, tc := range []struct { + name, contentType string + body []byte + want string + }{ + {"trailing garbage", "application/json", []byte(route.body + "x"), parse}, + {"two objects", "application/json", []byte(route.body + "{}"), parse}, + {"bom", "application/json", []byte("\xef\xbb\xbf" + route.body), parse}, + {"invalid utf-8", "application/json", []byte(strings.Replace(route.body, `"gate"`, "\"gate\xff\"", 1)), official("Invalid body: encountered a unicode decode error when parsing this JSON value. Please check the value to ensure it is valid unicode.")}, + {"duplicate", "application/json", []byte(strings.Replace(route.body, `"gate"`, `"first","`+lastKey+`":"gate"`, 1)), official("Invalid body: duplicate JSON key '" + lastKey + "' at '" + route.at + "'. Duplicate JSON keys are not supported.")}, + {"string root", "application/json", []byte(`"gate"`), official("Invalid type: expected an object, but got a string instead.")}, + {"no content type", "", []byte(route.body), contentType}, + {"text/plain", "text/plain", []byte(route.body), contentType}, + {"form", "application/x-www-form-urlencoded", []byte(route.body), contentType}, + {"bodyless", "", nil, contentType}, + } { + for _, token := range []string{owner, foreign} { + if status, body := client.do(token, http.MethodPost, route.path, tc.contentType, tc.body); status != http.StatusBadRequest || body != tc.want { + t.Errorf("%s %s (owner %t): %d %s", route.path, tc.name, token == owner, status, body) + } + } + } + } + if after := databaseDigest(t, pool); !mapsEqual(before, after) { + t.Fatal("a rejected body changed persisted state") + } + + // B7: each valid body passes the gate unchanged; route handling decides. + for _, route := range routes { + status, body := client.do(owner, http.MethodPost, route.path, "application/json", []byte(route.body)) + if strings.Contains(body, "Invalid body") || strings.Contains(body, "Content-Type") || strings.Contains(body, "Invalid type") { + t.Errorf("valid %s: %d %s", route.path, status, body) + } + } + // B5 and B7: an empty update and accepted JSON media types still write. + for _, body := range []string{``, `null`} { + status, updated := client.do(owner, http.MethodPost, "/v1/agents/"+agent, "application/json", []byte(body)) + var fields map[string]any + if status != http.StatusOK || json.Unmarshal([]byte(updated), &fields) != nil || fields["name"] != "body-agent" || fields["metadata"].(map[string]any)["k"] != "gate" { + t.Fatalf("empty update %q: %d %s", body, status, updated) + } + } + for _, media := range []string{"application/json; charset=utf-8", "Application/JSON", "application/merge-patch+json"} { + status, updated := client.do(owner, http.MethodPost, "/v1/agents/"+agent, media, []byte(`{"name":"`+media+`"}`)) + if status != http.StatusOK || !strings.Contains(updated, `"name":"`+media+`"`) { + t.Fatalf("%s update: %d %s", media, status, updated) + } + } + status, created := client.do(owner, http.MethodPost, "/v1/vaults", "application/json", []byte(`null`)) + if status != http.StatusCreated || !strings.Contains(created, `"name":null`) { + t.Fatalf("null Vault create: %d %s", status, created) + } + missingModel := `{"error":{"message":"Missing required parameter: 'model'.","type":"invalid_request_error","code":"invalid_request_error","param":"model"}}` + "\n" + for _, body := range []string{``, `null`} { + if status, response := client.do(owner, http.MethodPost, "/v1/agents", "application/json", []byte(body)); status != http.StatusBadRequest || response != missingModel { + t.Fatalf("empty Agent create %q: %d %s", body, status, response) + } + } + // The other tenant's view is unchanged. + if status, body := client.do(foreign, http.MethodGet, "/v1/agents", "", nil); status != http.StatusOK || !strings.Contains(body, `"data":[]`) { + t.Fatalf("foreign list: %d %s", status, body) + } +} diff --git a/services/agents-api/internal/store/saved_web_search_public_test.go b/services/agents-api/internal/store/saved_web_search_public_test.go index 3c4de540c..ce84d7339 100644 --- a/services/agents-api/internal/store/saved_web_search_public_test.go +++ b/services/agents-api/internal/store/saved_web_search_public_test.go @@ -156,6 +156,7 @@ func TestSavedWebSearchPostgres(t *testing.T) { } request.Header.Set("Authorization", "Bearer "+owner) request.Header.Set("OpenAI-Beta", "agents=v1") + request.Header.Set("Content-Type", "application/json") request.Header.Set("Idempotency-Key", key) response, err := http.DefaultClient.Do(request) if err != nil { diff --git a/services/agents-api/internal/store/session_model_execution_http_test.go b/services/agents-api/internal/store/session_model_execution_http_test.go index f806b7578..743f4709d 100644 --- a/services/agents-api/internal/store/session_model_execution_http_test.go +++ b/services/agents-api/internal/store/session_model_execution_http_test.go @@ -30,6 +30,7 @@ func TestModelExecutionHTTPWriteOnlyAndStrictAdmission(t *testing.T) { r := httptest.NewRequest(method, path, strings.NewReader(body)) r.Header.Set("Authorization", "Bearer "+token) r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "application/json") r.Header.Set("Idempotency-Key", key) w := httptest.NewRecorder() handler.ServeHTTP(w, r) From e24ff04422424d9328b2d7cf24c7515a15f743f6 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 20:34:45 +0000 Subject: [PATCH 02/12] Assert the official body errors in the pinned-SDK scripts The Agent, Agent update, Vault, Credential, Credential rotation and Session metadata scripts send raw bytes with and without the JSON Content-Type and assert the shared gate's official messages without writes (HP-09..15). A zero-length body or null is {} where the route accepts it. --- .../agents-api/tests/official_agent_update.py | 21 ++++++++-- services/agents-api/tests/official_agents.py | 16 +++++-- services/agents-api/tests/official_body.py | 42 +++++++++++++++++++ .../tests/official_credential_rotation.py | 9 +++- .../agents-api/tests/official_credentials.py | 9 +++- .../tests/official_session_metadata.py | 15 +++++-- services/agents-api/tests/official_vaults.py | 7 +++- 7 files changed, 102 insertions(+), 17 deletions(-) create mode 100644 services/agents-api/tests/official_body.py diff --git a/services/agents-api/tests/official_agent_update.py b/services/agents-api/tests/official_agent_update.py index 488c6e0fe..9b4b8e7fa 100644 --- a/services/agents-api/tests/official_agent_update.py +++ b/services/agents-api/tests/official_agent_update.py @@ -7,6 +7,8 @@ import httpx2 from openai import OpenAI +import official_body + def main(): base, token, foreign, restarted = sys.argv[1:] @@ -46,14 +48,27 @@ def main(): k: v for k, v in updated.to_dict().items() if k != "updated_at"} assert sessions.retrieve(old.id) == old updated = touched - for body in (None, [], {"model": None}, {"model": 3}, {"name": "x" * 129}, + for body in ([], {"model": None}, {"model": 3}, {"name": "x" * 129}, {"metadata": {"bad": None}}, {"text": {"unexpected": True}}, {"metadata": {"replace": "no"}, "instructions": False}, {"updated_at": 1}, {"tools": [{"type": "unknown"}]}): - response = http.post(endpoint, headers=headers, content="null" if body is None else None, - json=body if body is not None else None) + response = http.post(endpoint, headers=headers, json=body) assert response.status_code == 400, (body, response.status_code, response.text) assert agents.retrieve(original.id) == updated + # The shared body gate rejects before the lookup and any write (HP-09..HP-15), + # including a valid update sent without the JSON Content-Type. + for target, auth in ((original.id, headers), (original.id, headers | {"Authorization": "Bearer " + foreign})): + official_body.check(http, base + "/v1/agents/" + target, auth, official_body.rejected('{"name":"gate","metadata":{"k":"gate"}}', "name", "name") + + official_body.rejected('{"name":"gate","metadata":{"k":"gate"}}', "k", "metadata.k")) + assert agents.retrieve(original.id) == updated + # A zero-length body or null is the documented empty update (HP-13). + for content in ("", "null"): + response = http.post(endpoint, headers=headers | official_body.JSON, content=content) + assert response.status_code == 200, response.text + touched = agents.retrieve(original.id) + assert {k: v for k, v in touched.to_dict().items() if k != "updated_at"} == { + k: v for k, v in updated.to_dict().items() if k != "updated_at"} + updated = touched # Configuration protocol errors (TV-01..03) use the official fields and precede # the Agent lookup, so owned, foreign and missing Agents get the same response. for body, param, message in [ diff --git a/services/agents-api/tests/official_agents.py b/services/agents-api/tests/official_agents.py index 898fa9ccb..291baaad0 100644 --- a/services/agents-api/tests/official_agents.py +++ b/services/agents-api/tests/official_agents.py @@ -6,6 +6,8 @@ import httpx2 from openai import AuthenticationError, BadRequestError, NotFoundError +import official_body + def verify_agents(client, other, invalid, expect_error): agents = client.beta.agents @@ -139,11 +141,17 @@ def verify_agents(client, other, invalid, expect_error): assert response.status_code == 400 and response.json()["error"]["code"] == "invalid_request_error" assert response.json()["error"]["param"] is None assert len(list(agents.list())) == count - for content in ("{}", "null", "[]", '{"model":"x"} {}'): - assert raw.post(base, headers=headers, content=content).status_code == 400 - assert raw.post(base, headers=headers, content='{"model":"' + "x" * (1024 * 1024) + '"}').status_code == 413 + # The shared body gate rejects before any write (HP-09..HP-15); a zero-length + # body or null is {} and reports the missing model (HP-13). + official_body.check(raw, base, headers, official_body.rejected('{"model":"resource-model","name":"gate"}', "name", "name")) + json_headers = {**headers, **official_body.JSON} + for content in ("", "{}", "null"): + response = raw.post(base, headers=json_headers, content=content) + assert response.status_code == 400 and response.json()["error"]["param"] == "model", response.text + assert raw.post(base, headers=json_headers, content='{"model":"' + "x" * (1024 * 1024) + '"}').status_code == 413 # tenant_id is an ignored query key; it never selects another tenant. - assert raw.post(base, headers=headers, params={"tenant_id": "other"}, content="{}").status_code == 400 + assert raw.post(base, headers=json_headers, params={"tenant_id": "other"}, content="{}").status_code == 400 + assert len(list(agents.list())) == count plain = raw.get(base + "/" + saved[0].id, headers=headers) scoped = raw.get(base + "/" + saved[0].id, headers=headers, params={"tenant_id": "other"}) assert plain.status_code == scoped.status_code == 200 and scoped.json() == plain.json() diff --git a/services/agents-api/tests/official_body.py b/services/agents-api/tests/official_body.py new file mode 100644 index 000000000..fefbc5140 --- /dev/null +++ b/services/agents-api/tests/official_body.py @@ -0,0 +1,42 @@ +"""Official errors of the shared Agents API JSON body gate (HP-09..HP-15).""" + +PARSE = ("Invalid body: failed to parse JSON value. Please check the value to ensure it is valid JSON. " + "(Common errors include trailing commas, missing closing brackets, missing quotation marks, etc.)") +UNICODE = ("Invalid body: encountered a unicode decode error when parsing this JSON value. " + "Please check the value to ensure it is valid unicode.") +CONTENT_TYPE = "expected request with Content-Type: application/json" +JSON = {"Content-Type": "application/json"} + + +def duplicate(key, path): + return f"Invalid body: duplicate JSON key '{key}' at '{path}'. Duplicate JSON keys are not supported." + + +def rejected(valid, key, path, value="gate"): + """Cases that must write nothing. valid is a compact JSON object body that would + write; it contains the string member key:value at the dotted object-key path.""" + body = valid.encode() + member = f'"{key}":"{value}"'.encode() + assert member in body, valid + return [ + (JSON, body + b"x", PARSE), + (JSON, body + b"{}", PARSE), + (JSON, b"\xef\xbb\xbf" + body, PARSE), + (JSON, b" \n ", PARSE), + (JSON, body.replace(member, member[:-1] + b'\xff"', 1), UNICODE), + (JSON, body.replace(member, f'"{key}":"first",'.encode() + member, 1), duplicate(key, path)), + (JSON, b'"scan6"', "Invalid type: expected an object, but got a string instead."), + (JSON, b"[]", "Invalid type: expected an object, but got an array instead."), + ({}, body, CONTENT_TYPE), + ({"Content-Type": "text/plain"}, body, CONTENT_TYPE), + ({"Content-Type": "application/x-www-form-urlencoded"}, body, CONTENT_TYPE), + ({}, b"", CONTENT_TYPE), + ] + + +def check(http, url, headers, cases): + for extra, content, message in cases: + response = http.post(url, headers={**headers, **extra}, content=content) + assert response.status_code == 400 and response.json()["error"] == { + "type": "invalid_request_error", "code": "invalid_request_error", "param": None, + "message": message}, (content[:80], response.status_code, response.text) diff --git a/services/agents-api/tests/official_credential_rotation.py b/services/agents-api/tests/official_credential_rotation.py index 6d263c88c..ea943f66c 100644 --- a/services/agents-api/tests/official_credential_rotation.py +++ b/services/agents-api/tests/official_credential_rotation.py @@ -1,10 +1,13 @@ """Public token replacement without changing Credential or Session identity.""" +import json import uuid import httpx2 from openai import AuthenticationError, BadRequestError, NotFoundError +import official_body + def verify_credential_rotation(client, other, invalid, peer, saved_vaults, saved_credentials, canary, expect_error): vaults, foreign_vault = saved_vaults @@ -73,8 +76,10 @@ def metadata(response, previous): ] for body in invalid_bodies: safe(raw.post(endpoint, headers=headers, json=body), 400) - for body in ("null", "[]", "{} {}"): - safe(raw.post(endpoint, headers=headers, content=body), 400) + # The shared body gate rejects before any write (HP-09..HP-15); null is {}. + official_body.check(raw, endpoint, headers, official_body.rejected( + json.dumps({"auth": {"type": "static_bearer", "token": "gate"}}, separators=(",", ":")), "token", "auth.token")) + safe(raw.post(endpoint, headers={**headers, **official_body.JSON}, content="null"), 400) for override in ({"auth": None}, {"auth": {"type": "static_bearer", "token": None}}): error = expect_error(BadRequestError, lambda: credentials.update( original.id, vault_id=vault.id, **replacement, extra_body=override)) diff --git a/services/agents-api/tests/official_credentials.py b/services/agents-api/tests/official_credentials.py index 81a157769..0980fc007 100644 --- a/services/agents-api/tests/official_credentials.py +++ b/services/agents-api/tests/official_credentials.py @@ -1,11 +1,14 @@ """Static-bearer Credential metadata through the pinned SDK and real HTTP.""" +import json import time import uuid import httpx2 from openai import AuthenticationError, BadRequestError, InternalServerError, NotFoundError +import official_body + def verify_credential(body, vault_id, name, destination): assert set(body) == {"id", "auth", "created_at", "name", "object", "updated_at", "vault_id"} @@ -83,8 +86,10 @@ def safe_body(response, status): for body in invalid_requests: response = raw.post(endpoint, headers=headers, json=body) assert safe_body(response, 400)["error"]["type"] == "invalid_request_error" - for body in ("null", "[]", "{} {}"): - safe_body(raw.post(endpoint, headers=headers, content=body), 400) + # The shared body gate rejects before any write (HP-09..HP-15); null is {}. + official_body.check(raw, endpoint, headers, official_body.rejected( + json.dumps({**request, "name": "gate"}, separators=(",", ":")), "name", "name")) + safe_body(raw.post(endpoint, headers={**headers, **official_body.JSON}, content="null"), 400) for override in ({"name": None}, {"auth": None}, {"auth": {**auth, "token": None}}): error = expect_error(BadRequestError, lambda: credentials.create(vault.id, **request, extra_body=override)) safe_body(error.response, 400) diff --git a/services/agents-api/tests/official_session_metadata.py b/services/agents-api/tests/official_session_metadata.py index 2e22d1072..6b37c4cfd 100644 --- a/services/agents-api/tests/official_session_metadata.py +++ b/services/agents-api/tests/official_session_metadata.py @@ -6,6 +6,8 @@ import httpx2 from openai import AuthenticationError, BadRequestError, ConflictError, NotFoundError +import official_body + def without_metadata(session): return {key: value for key, value in session.to_dict().items() if key != "metadata"} @@ -76,11 +78,16 @@ def assert_metadata(session, expected): error = expect_error(BadRequestError, lambda: sessions.update(first.id, metadata=metadata)) assert error.body["param"] == param assert sessions.retrieve(first.id) == current - for body in ["", "null", "[]", "1", "{}{}", '{"metadata":']: - response = raw.post(url, headers=headers, content=body) - assert response.status_code == 400 and response.json()["error"]["code"] == "invalid_request" + # The shared body gate (HP-09..HP-15); a zero-length body or null is an + # empty update, which still requires metadata (HP-13). + official_body.check(raw, url, headers, official_body.rejected('{"metadata":{"k":"gate"}}', "k", "metadata.k")) + json_headers = {**headers, **official_body.JSON} + for body in ["", "null"]: + response = raw.post(url, headers=json_headers, content=body) + assert response.status_code == 400 and response.json()["error"]["message"] == "At least one update field is required" + assert sessions.retrieve(first.id) == current oversized = json.dumps({"metadata": {"key": "x" * (1024 * 1024)}}) - response = raw.post(url, headers=headers, content=oversized) + response = raw.post(url, headers=json_headers, content=oversized) assert response.status_code == 413 and response.json()["error"]["code"] == "request_too_large" assert raw.patch(url, headers=headers, json={"metadata": {}}).status_code == 405 empty = raw.post(url, headers=headers, json={}) diff --git a/services/agents-api/tests/official_vaults.py b/services/agents-api/tests/official_vaults.py index 781901be1..6528a4193 100644 --- a/services/agents-api/tests/official_vaults.py +++ b/services/agents-api/tests/official_vaults.py @@ -6,6 +6,8 @@ import httpx2 from openai import AuthenticationError, BadRequestError, NotFoundError +import official_body + def verify_vault(body, name, metadata): assert set(body) == {"id", "object", "created_at", "name", "metadata"} @@ -79,8 +81,9 @@ def verify_vaults(client, other, invalid, peer, binding, expect_error): response = raw.post(base, headers=headers, json=request) assert response.status_code == 400 and response.json()["error"]["code"] == "invalid_request_error" assert response.json()["error"]["param"] == param - for content in ("null", "[]", "{} {}"): - assert raw.post(base, headers=headers, content=content).status_code == 400 + # The shared body gate rejects before any write (HP-09..HP-15). + official_body.check(raw, base, headers, official_body.rejected('{"name":"gate","metadata":{"k":"v"}}', "name", "name")) + official_body.check(raw, base, headers, [(official_body.JSON, b'{"metadata":{"k":"1","k":"2"}}', official_body.duplicate("k", "metadata.k"))]) for resource_id in (str(uuid.uuid4()), "invalid-vault", str(uuid.UUID(int=0)), foreign.id): response = raw.get(base + "/" + resource_id, headers=headers) From 424d66b415b225b063be9a088f388604beb6d946 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 20:34:52 +0000 Subject: [PATCH 03/12] Document the shared request body gate Record the HP-09..15 alignment rows, register the JB evidence code and describe the gate in CONTRIBUTING and the Agent create annotation. --- CONTRIBUTING.md | 7 ++ .../official-semantics-alignment.md | 89 ++++++++++++++++--- contracts/agents-api/openapi.yaml | 17 ++-- contracts/agents-api/operation-evidence.md | 7 +- services/agents-api/internal/api/agents.go | 2 +- 5 files changed, 98 insertions(+), 24 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fc83c3670..4cb791b45 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -86,6 +86,13 @@ that family. Record uncertain range/lookup behavior separately; do not reproduce observed upstream server failures as compatibility behavior. See `contracts/agents-api/list-query-semantics.md` for the bounded evidence. +Every Agents API JSON route reads its body through the shared gate +(`readJSONObject`) before route decoding, validation or lookup. It requires a JSON +Content-Type, applies the route's body limit and rejects invalid UTF-8, malformed +JSON, repeated keys and non-object roots with the official messages; an empty +body or null becomes `{}`. DELETE, multipart, Core extension and internal routes +keep their own readers. See +`contracts/agents-api/official-semantics-alignment.md#request-body-parsing--september-23`. Report validation failures with official evidence through the typed field error, which emits `invalid_request_error` with the observed param and message; keep other local codes until their official fields are sampled. Every 409 has type diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index 390165711..927713054 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -393,21 +393,20 @@ Decisions: local limits and codes, and before harness admission. It is not a JSON Schema engine: function and output schemas, `request_metadata` values, MCP `transport` members, `metadata` and `x_agents_core` stay with their existing parsers. -- In each object, a union's `type` is checked first. Unknown and repeated members - are then reported in document order, followed by member values in document - order and missing required members in the pinned order. The whole object is - checked before the C2/C3 conflicts, and tools before `text`. The official order +- In each object, a union's `type` is checked first. Unknown members are then + reported in document order, followed by member values in document order and + missing required members in the pinned order. The whole object is checked + before the C2/C3 conflicts, and tools before `text`. The official order between several errors in one body was not observed. - Member names match exactly, so a name that differs from a member only by case, - such as `reasoning.Effort`, is an unknown parameter. A member repeated anywhere - in the checked tree returns 400 `invalid_request_error` with its path as param - and the local message `Duplicate parameter: ''.`; the official response - is unobserved. Both are needed because encoding/json matches names - case-insensitively and merges repeated objects into the decoded structs: - checking only the last copy let `"text"` given twice store an array-root schema, - and `{"reasoning":{"Effort":"high"},"reasoning":{}}` store `effort: high`. - Members left to their parsers are checked on the decoded values, so they cannot - differ from what is stored. + such as `reasoning.Effort`, is an unknown parameter. This is needed because + encoding/json matches names case-insensitively. It also merges repeated + objects into the decoded structs; a key repeated anywhere in the body is now + rejected earlier by the shared body gate with the official message (see + [Request body parsing](#request-body-parsing--september-23)), which replaces + this batch's local `Duplicate parameter: ''.` error. Members left to their + parsers are checked on the decoded values, so they cannot differ from what is + stored. - Observed expected-kind phrases are `an object`, `a boolean` and `an object with string keys and unknown value values`. At unsampled positions Core uses `a string` (also for enum members), `an integer` and `an array`, and @@ -872,3 +871,67 @@ daemon-enabled composition and checks that no Agents API request is redirected. `official_http_routing.py` updates an Agent through base URL `/v1//`, checks `_request_id` and the error `request_id`, and the raw checks in the other official scripts now expect the Beta check first. + +## Request body parsing — September 23 + +Every Agents API JSON request body now passes one shared gate, before any +route-specific decoding, validation or lookup, with the official parse semantics. +Evidence is HP-09..HP-15 of the HTTP protocol campaign scan, recorded privately in +`~/.parsar/remediation/20260923/campaign-scan-6/http-protocol/` (`findings.json`, +`REPORT.txt`, raw requests in `official-ledger.jsonl`, labels `C01`–`C20`, +`U01`–`U11`, `S1`): one owned Agent, created, updated and deleted (404 confirmed), +without a Session or model. The official records cover Agent create and update; +the other routes are assumed to share the official parser. + +| Row | Case | Core behavior | +| --- | --- | --- | +| B1 | Malformed JSON, trailing data, two concatenated values, a UTF-8 byte order mark or a whitespace-only body (`C01`, `C06`, `C07`, `C12`, `C20`, `U01`, `U05`, `U06`) | 400, type and code `invalid_request_error`, param null: "Invalid body: failed to parse JSON value. Please check the value to ensure it is valid JSON. (Common errors include trailing commas, missing closing brackets, missing quotation marks, etc.)". Agent update no longer returns `unsupported_or_invalid_configuration`. | +| B2 | Invalid UTF-8 anywhere in the body (`C13`) | 400 with the same fields: "Invalid body: encountered a unicode decode error when parsing this JSON value. Please check the value to ensure it is valid unicode." Previously the bytes were stored as U+FFFD. | +| B3 | A repeated object key at any depth (`C08`, `C15`, `C16`, `U07`) | 400 with the same fields: "Invalid body: duplicate JSON key '' at ''. Duplicate JSON keys are not supported." The path joins object keys with `.` and omits array indices: `name`, `metadata.k`, `tools.type`. The first repeat in document order is reported. Previously metadata, Vaults and Templates kept the last value, and Agent configuration returned the local "Duplicate parameter". | +| B4 | A valid root that is not an object (`C05`) | 400 with the same fields: "Invalid type: expected an object, but got instead." with the existing kind phrases (`a string`, `an integer`, `a number`, `a boolean`). | +| B5 | A zero-length body or `null` (`C02`, `C03`, `U02`, `U03`) | Treated as `{}`: Agent create reports the missing `model`, Agent update is the documented empty update, Session update keeps its "At least one update field is required" rejection, and Vault create creates an unnamed Vault. A whitespace-only body stays B1. | +| B6 | Content-Type missing, `text/plain` or form-encoded, including a bodyless POST without Content-Type (`C09`–`C11`, `C19`, `U08`–`U10`) | 400 with the same fields, "expected request with Content-Type: application/json", checked before the body is read. `application/json` and `application/*+json` are accepted case-insensitively, with parameters (`S1`, `U11`, `C17`, `C18`). Previously Core ignored the header and applied the update. | +| B7 | Valid bodies | Unchanged, including unknown-member errors, configuration validation, route body limits (413) and Core extensions such as `x_agents_core`. | + +Order: authentication and Beta handling as before, then B6, then the route's body +limit, then B2, B1, B3 and B4/B5, then route validation. The gate covers Agent +create and update, Vault create, Credential create and update, Template create +and update, Environment Files create, Session create and update, and Session +events. Environment Files create now reads its body before the Environment +lookup; a missing Environment with a valid body still returns 404. + +Decisions: + +- An array root is rejected with the B4 message. The official service treats `[]` + as `{}` (HP-14); that upstream anomaly is not copied. +- The duplicate key and path are repeated only when each is at most 256 bytes of + printable UTF-8, the shared `echotext` rule; otherwise the message is "Invalid + body: duplicate JSON key. Duplicate JSON keys are not supported." A lone + surrogate escape decodes to U+FFFD and is therefore not repeated either. +- Invalid UTF-8 is checked before JSON syntax; the official order for a body with + both faults was not observed. +- Not gated: DELETE routes, which keep their empty-body rule, the multipart Files + and Skills uploads, Skills update (a non-Beta API with its own observed error + fields), the Core extension `/core/v1/*` routes, including executor credential + issuance, and the internal daemon, sandbox and node routes. + +Unchanged: the schema, route validation and error codes of valid bodies, and +lone surrogate escapes such as `"\ud800"`, which JSON permits and encoding/json +still decodes to U+FFFD; the official treatment of such escapes was not observed. + +Caller check: the TypeScript client sends `application/json` with every JSON body +(an empty Agent update sends `{}`), Core Web uses that client, the Go client uses +the pinned openai-go SDK, the Python acceptance tools send JSON through the pinned +SDK or `json=`, and the documentation has no JSON POST examples. The Parsar +product repository does not call these routes. + +Go tests cover the gate on its own (B1–B5, the echo bound, accepted media types, +deep bodies and a differential check of the linear duplicate-key scan against an +encoding/json token walk) and on all eleven route families (B1–B4, B6, the order against +authentication, Beta and the body limit, B5/B7 and the excluded routes). A +real-PostgreSQL test sends B1–B4 and B6 to every route family as the owner and +as tenant B under a whole-database digest, then checks B5/B7 writes. The pinned-SDK +scripts `official_agents.py`, `official_agent_update.py`, `official_vaults.py`, +`official_credentials.py`, `official_credential_rotation.py` and +`official_session_metadata.py` assert the official messages and no writes. +Independent acceptance is recorded separately by the coordinator. diff --git a/contracts/agents-api/openapi.yaml b/contracts/agents-api/openapi.yaml index c51455a40..b0276a5a6 100644 --- a/contracts/agents-api/openapi.yaml +++ b/contracts/agents-api/openapi.yaml @@ -2930,13 +2930,16 @@ paths: description: 'Persists configuration independently of execution. Names over 128 characters and metadata outside 16 string pairs with 64-character keys and 512-character values return invalid_request_error with the official param; - U+0000 in stored strings is rejected as a local storage limit. Missing, unknown, - repeated, wrongly typed or unsupported enum members of the pinned configuration - shapes (tools, text, reasoning, service_tier, multi_agent) return invalid_request_error - with the JSON path as param; duplicate function names, repeated web_search - or tool_search and non-object schema root types return it with a null param. - Supports model/name/instructions/metadata, explicit reasoning and service - tiers, multi_agent, text/json_schema, function/tool_search/programmatic_tool_calling/web_search + U+0000 in stored strings is rejected as a local storage limit. As on every + Agents API JSON route, a non-JSON Content-Type, invalid UTF-8, malformed JSON, + a repeated key at any depth or a non-object root returns invalid_request_error + with a null param and the official message before other checks; an empty or + null body is {}. Missing, unknown, wrongly typed or unsupported enum members + of the pinned configuration shapes (tools, text, reasoning, service_tier, + multi_agent) return invalid_request_error with the JSON path as param; duplicate + function names, repeated web_search or tool_search and non-object schema root + types return it with a null param. Supports model/name/instructions/metadata, + explicit reasoning and service tiers, multi_agent, text/json_schema, function/tool_search/programmatic_tool_calling/web_search and HTTP MCP with nullable credential_id, service origin (omitted or null on HTTP transport is saved as service) and boolean required defaulting to false. Saving credential_id grants no access: Session admission checks attached diff --git a/contracts/agents-api/operation-evidence.md b/contracts/agents-api/operation-evidence.md index a53f89b4b..61b9b8a91 100644 --- a/contracts/agents-api/operation-evidence.md +++ b/contracts/agents-api/operation-evidence.md @@ -1,6 +1,6 @@ # Pinned operation evidence inventory — 2026-09-23 -Baseline inventory of main `b5715912f09333e2b4449ec6f0eecaabce44c9b7`. The Session admission batch below updates creation and metadata validation, the list query tolerance batch (L) updates list and resource query handling, the validation error batch (X) updates field error codes/params, malformed path IDs and U+0000 handling, the Environment Files wire batch (G) updates Files.create/list status, envelope, query, path and empty-page behavior, and the creation stream settlement batch (J) updates creation SSE lifetime/snapshot, terminal Turn usage and Turn start order, and the Session deletion batch (Z) updates the deletion lifecycle, and the Agent configuration validation batch (M) updates saved and inline Agent configuration errors, the whitespace input batch (P) admits whitespace-only message text, the list cursor error batch (CE) updates unresolved `after` cursor errors on every list, the input conflict batch (CF) gives every 409 type `conflict_error` and aligns Session input conflicts and tool result target errors, and the saved web_search batch (SW) saves every pinned `web_search` mode while Session admission keeps rejecting enabled search, and the workspace file write batch (FW) aligns Files.create parent creation, no-replacement and the inline size bound, and the item serialization batch (SR) aligns Item/event null fields, assistant message event framing, reasoning keys and the Session usage rule, and the MCP credential selection batch (MV) saves an omitted HTTP MCP origin as `service`, projects the implicitly selected Session credential and aligns selection errors, and the hosted initialization failure batch (HF) aligns failed hosted provisioning: Session status/error, failure events, stream end and later-input 409, and the HTTP routing and header batch (RH) serves canonical paths without redirects, checks OpenAI-Beta before authentication and aligns 401 envelopes, HEAD, `Allow` and request ID headers on every operation; historical evidence retains its original revision and scope. This inventory guides repeated qualification and does not assert complete compatibility. +Baseline inventory of main `b5715912f09333e2b4449ec6f0eecaabce44c9b7`. The Session admission batch below updates creation and metadata validation, the list query tolerance batch (L) updates list and resource query handling, the validation error batch (X) updates field error codes/params, malformed path IDs and U+0000 handling, the Environment Files wire batch (G) updates Files.create/list status, envelope, query, path and empty-page behavior, and the creation stream settlement batch (J) updates creation SSE lifetime/snapshot, terminal Turn usage and Turn start order, and the Session deletion batch (Z) updates the deletion lifecycle, and the Agent configuration validation batch (M) updates saved and inline Agent configuration errors, the whitespace input batch (P) admits whitespace-only message text, the list cursor error batch (CE) updates unresolved `after` cursor errors on every list, the input conflict batch (CF) gives every 409 type `conflict_error` and aligns Session input conflicts and tool result target errors, and the saved web_search batch (SW) saves every pinned `web_search` mode while Session admission keeps rejecting enabled search, and the workspace file write batch (FW) aligns Files.create parent creation, no-replacement and the inline size bound, and the item serialization batch (SR) aligns Item/event null fields, assistant message event framing, reasoning keys and the Session usage rule, and the MCP credential selection batch (MV) saves an omitted HTTP MCP origin as `service`, projects the implicitly selected Session credential and aligns selection errors, and the hosted initialization failure batch (HF) aligns failed hosted provisioning: Session status/error, failure events, stream end and later-input 409, and the HTTP routing and header batch (RH) serves canonical paths without redirects, checks OpenAI-Beta before authentication and aligns 401 envelopes, HEAD, `Allow` and request ID headers on every operation, and the JSON body batch (JB) checks every Agents API JSON request body in one shared gate; historical evidence retains its original revision and scope. This inventory guides repeated qualification and does not assert complete compatibility. Baseline: `contracts/agents-api/upstream.json`, SDK **3.13.0**, upstream commit **d7c41efee1b0802b79f3f88a678ef2052b06e9ce**, `OpenAI-Beta: agents=v1`. AGENTS.md and relevant CONTRIBUTING.md compatibility, ownership and evidence rules govern this inventory. @@ -53,6 +53,7 @@ Repository paths below are relative to the inspected worktree; private evidence | MV | [MCP origin and credential selection](official-semantics-alignment.md#mcp-origin-and-credential-selection--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/mcp-vaults/findings.json` MV-01..03 with raw records in `official-ledger.jsonl` (`AG1-minimal-and-nulls`, `S1-T1-create-stream`, `S1-after-c1-delete-session-get`, `S1-final-session-get`, `ERR-UNATTACHED`, `ERR-URL-MISMATCH`, `ERR-AMBIGUOUS`, `ERR-CREDENTIAL-BOGUS`, `ERR-VAULT-BOGUS`): owned Agents, three owned Sessions, two Vaults and four Credentials, all deleted. Rows M1–M9 of that section. Go API/store and real-PostgreSQL tenant A/B HTTP tests with a no-write digest, pinned-SDK official client scripts; live acceptance is recorded with the batch. | | HF | [Hosted initialization failure](official-semantics-alignment.md#hosted-initialization-failure--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/hosted-init/findings.json` HI-01..04 with raw records under `official/` (`006-S2-create-setup-exit3`, `007-S2-events`, `009-S3-events`, `021-S2-env-after-failed`, `024-S2-session-after-failed`, `025-S3-session-after-failed`, `026-S2-input-after-failure`, `027-S3-input-after-failure`, `038-S2-delete`, `041-S3-delete`): two owned `openai_hosted` Sessions without a Turn (setup exit 3, nonexistent Python package), both deleted. Rows H1–H8 of that section; HI-05/06 stay deferred. Python initializer receipt tests, Go contract/API tests, real-PostgreSQL managed-Worker store tests with a controlled Provider and an HTTP test with tenant B and a canary, TypeScript client and Web tests; live Docker acceptance is recorded with the batch. | | RH | [HTTP routing and response headers](official-semantics-alignment.md#http-routing-and-response-headers--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/http-protocol/findings.json` HP-02, 03, 05, 07, 17–20, 23, 24 (HP-04, 21, 22, 26 kept) with raw records in `official-ledger.jsonl` (labels `A01`–`A11`, `B01`, `B08`, `R04`–`R18`, `S2-retrieve-baseline`, `C01-malformed`, `X1-delete-owned`) and `REPORT.txt`: one owned Agent (deleted), 90 requests without a Session or model. Rows RH1–RH11 apply to every operation's HTTP layer; HEAD on the events stream and content downloads is a documented local 405. | +| JB | [Request body parsing](official-semantics-alignment.md#request-body-parsing--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/http-protocol/findings.json` HP-09..15 with raw records in `official-ledger.jsonl` (labels `C01`–`C20`, `U01`–`U11`, `S1`) and `REPORT.txt`: one owned Agent (deleted, 404 confirmed), no Session or model. Rows B1–B7 of that section; HP-14 (`[]` as `{}`) is a recorded upstream anomaly, not copied. Official records cover Agent create/update; rows 1, 3, 6, 8, 11, 27, 29, 31, 34, 38 and 40 share the gate. Go gate and all-route handler tests, a real-PostgreSQL owner/tenant B no-write digest across every route family and pinned-SDK scripts; independent acceptance is recorded with the batch. | ## Per-operation evidence matrix @@ -60,9 +61,9 @@ Paths in the appendix include `/v1`. SDK names here omit `client.`. `P` means pa | # | SDK operation | Implemented behavior | Official wire observation | Core validation | Known difference / unverified semantics | | --- | --- | --- | --- | --- | --- | -| 1 | beta.agents.create | P: saved configuration, 201; metadata/name errors use official code and param; configuration protocol errors use official code, JSON-path param and message; repeated tools and non-object schema roots reject; every pinned `web_search` mode is saved, omitted/null mode as `live`; omitted/null HTTP MCP `connection_origin` is saved as `service` | R `agent-create-supported`; initial unsupported model case 400; X VA-07/08/09; M TV-01/02 `AC01`/`AC02`; SW TV-05 `type-only`, `mode-null`, `mode-cached`, `mode-cached-full`; MV MV-01 `AG1-minimal-and-nulls` | C DB create; T saved-Agent execution references; X DB field errors and U+0000 no-write; M DB configuration errors, no-write and tenant replay; SW DB exact tool bytes on create/retrieve/list, admission rejection without writes and tenant B; MV DB omitted/null origin equals explicit | Model-derived reasoning defaults and unsupported configurations; enabled `web_search` is saved but rejects at Session admission; multi-error order, unsampled kind phrases and complete default/null/errors unknown | +| 1 | beta.agents.create | P: saved configuration, 201; metadata/name errors use official code and param; configuration protocol errors use official code, JSON-path param and message; repeated tools and non-object schema roots reject; every pinned `web_search` mode is saved, omitted/null mode as `live`; omitted/null HTTP MCP `connection_origin` is saved as `service`; the shared body gate rejects malformed, invalid UTF-8, repeated-key and non-object bodies and a non-JSON Content-Type with the official messages, and an empty or null body is `{}` | R `agent-create-supported`; initial unsupported model case 400; X VA-07/08/09; M TV-01/02 `AC01`/`AC02`; SW TV-05 `type-only`, `mode-null`, `mode-cached`, `mode-cached-full`; MV MV-01 `AG1-minimal-and-nulls`; JB HP-09..13/15 `C01`–`C20`, `S1` | C DB create; T saved-Agent execution references; X DB field errors and U+0000 no-write; M DB configuration errors, no-write and tenant replay; SW DB exact tool bytes on create/retrieve/list, admission rejection without writes and tenant B; MV DB omitted/null origin equals explicit; JB DB body gate no-write digest with tenant B | Model-derived reasoning defaults and unsupported configurations; enabled `web_search` is saved but rejects at Session admission; multi-error order, unsampled kind phrases and complete default/null/errors unknown | | 2 | beta.agents.retrieve | P: tenant-owned saved read | R `agent-read`, `agent-read-deleted` | C DB own/foreign/deleted read | Full field defaults and inline-vs-saved lifetime | -| 3 | beta.agents.update | P: atomic replacements; empty body touches timestamp; metadata/name errors use official code and param; configuration errors as for create, before the Agent lookup; `web_search` and MCP origin projections as for create | R `agent-patch-metadata`, `agent-null-fields`, `agent-noop`, `agent-nested-reasoning`, rejection labels; X VA-07/08/09/10; M TV-01..03 and TV-07 update labels (`F01`–`X01`, `W04`–`M02`); SW `update-disabled`, `update-omitted-low`, `update-live-domains-empty`, `retrieve` | C DB no-op/unchanged snapshot; resource implementation-validation.md actual PostgreSQL SDK update tests; M DB owned/foreign/missing/malformed-ID replay, K1 values and isolation; SW DB update projections, retrieve/list bytes and same-key Session retry; MV DB null-origin update | Model-dependent default recomputation; uncommon nested/null/error variants | +| 3 | beta.agents.update | P: atomic replacements; empty body touches timestamp; metadata/name errors use official code and param; configuration errors as for create, before the Agent lookup; `web_search` and MCP origin projections as for create; body gate as for create, before the Agent lookup, so a zero-length or null body is the empty update | R `agent-patch-metadata`, `agent-null-fields`, `agent-noop`, `agent-nested-reasoning`, rejection labels; X VA-07/08/09/10; M TV-01..03 and TV-07 update labels (`F01`–`X01`, `W04`–`M02`); SW `update-disabled`, `update-omitted-low`, `update-live-domains-empty`, `retrieve`; JB HP-09/11/13/15 `U01`–`U11` | C DB no-op/unchanged snapshot; resource implementation-validation.md actual PostgreSQL SDK update tests; M DB owned/foreign/missing/malformed-ID replay, K1 values and isolation; SW DB update projections, retrieve/list bytes and same-key Session retry; MV DB null-origin update; JB DB body gate no-write digest with tenant B and pinned-SDK raw bytes | Model-dependent default recomputation; uncommon nested/null/error variants | | 4 | beta.agents.list | P: scoped cursor list; unknown keys ignored, limit 0/above 100 clamp; any unresolved cursor, malformed included, is the missing 404 | R `agent-list-empty-scoped`, `agent-list-limit101`; L VA-01/02/03/04/18; CE ERR-13 `cur-agents-random`, `cur-agents-othertype-session` | L DB `official_list_query.py` tenant A/B; CE DB cursor matrix tenant A/B | Core page capacity 100; no inferred official cap. Overflowing limits unsampled | | 5 | beta.agents.delete | P: resource deletion | R `cleanup-agent`, subsequent 404 | C DB delete/post-delete | Referenced/in-flight/repeated-delete exact parity | | 6 | beta.agents.sessions.create | P: JSON/live SSE 201, saved/inline frozen config, initial messages, native profiles; inline agent configuration errors use official fields with `agent.` params before the input requirement, and saved records with repeated tools or non-object schema roots reject admission; fresh creation SSE sends the JSON 201 projection, then ends right after the first idle recorded when a Turn ends or an input reservation stops being pending, or any failed, never sending later events; nothing admitted ends after `created`; a silent settlement ends after events up to the cursor read with a settled projection in one snapshot. A same-key stream retry returns 201, sends no events and ends at once; whitespace-only text is admitted and stored verbatim, while empty text/content/input keep the local 400; omitted/null HTTP MCP origin is `service`; MCP credential selection errors use the official status, code, null param and message after the input requirement, with one message for missing, foreign and unattached references, and write nothing | S `create-1/2.json`, `omitted-input.json`, `null-input.json`, `empty-array-input.json`, `retry-status-original/repeat.json`; H stream; J creation streams closed after idle, open through requires_action; M TV-01..04 `SC01`–`SC11`; P SES-01..03 whitespace-only string and message input 201, verbatim Item; MV MV-01 `S1-T1-create-stream`, MV-03 `ERR-UNATTACHED`, `ERR-URL-MISMATCH`, `ERR-AMBIGUOUS`, `ERR-CREDENTIAL-BOGUS`, `ERR-VAULT-BOGUS` | N Live none admission and retry; C Live three hosted profiles; D/T/K/I recorded additional workflows; J DB creation-stream lifetime/snapshot/retry; M DB inline and saved-override configuration errors without writes; P DB verbatim whitespace Items and unchanged empty-input 400 without writes; MV DB M1–M8 tenant A/B, byte-identical not-found bodies and no-write digest | Session admission batch removes idle `none` creation; local idempotent create still differs from two official IDs. Stream retry, self-hosted, hosted and no-input stream lifetimes are local; work drained before a silent-settlement read can still be sent. Many input/tool/environment combinations restricted; harness admission limits keep the local code (TV-06). Empty-input 400s keep the local code and message; `["", text]` parts are accepted but unobserved officially (SES-08); Claude SDK and MiniMax Code reject whitespace-only messages at admission as a declared native limitation (W6); the unknown-Vault message stays `Resource not found.`; a deleted selected credential still fails at dispatch rather than input (MV-04) | diff --git a/services/agents-api/internal/api/agents.go b/services/agents-api/internal/api/agents.go index 04eed9231..77150d824 100644 --- a/services/agents-api/internal/api/agents.go +++ b/services/agents-api/internal/api/agents.go @@ -20,7 +20,7 @@ type AgentStore interface { } // @Summary Create a reusable Agent -// @Description Persists configuration independently of execution. Names over 128 characters and metadata outside 16 string pairs with 64-character keys and 512-character values return invalid_request_error with the official param; U+0000 in stored strings is rejected as a local storage limit. Missing, unknown, repeated, wrongly typed or unsupported enum members of the pinned configuration shapes (tools, text, reasoning, service_tier, multi_agent) return invalid_request_error with the JSON path as param; duplicate function names, repeated web_search or tool_search and non-object schema root types return it with a null param. Supports model/name/instructions/metadata, explicit reasoning and service tiers, multi_agent, text/json_schema, function/tool_search/programmatic_tool_calling/web_search and HTTP MCP with nullable credential_id, service origin (omitted or null on HTTP transport is saved as service) and boolean required defaulting to false. Saving credential_id grants no access: Session admission checks attached Vault ownership and destination. MCP allowed_tools preserves null versus empty; saved HTTP transport includes empty headers. Model-derived reasoning defaults, other MCP variants and public retry conformance remain incomplete. web_search saves every pinned mode: omitted or null mode is saved as live and omitted or null context_size as medium; allowed_domains preserves null versus empty and a present location, including {}, includes all four keys with null for omitted ones, as observed officially (req_db41d2f6261b4abfb69465eafe719ab5, req_165d53b88445490b9146d8272c54134d). Session execution accepts only explicit disabled web_search and disabled programmatic_tool_calling through qualified Runtime controls; saved enabled forms reject at Session admission. Session execution admits only its supported configuration subset. +// @Description Persists configuration independently of execution. Names over 128 characters and metadata outside 16 string pairs with 64-character keys and 512-character values return invalid_request_error with the official param; U+0000 in stored strings is rejected as a local storage limit. As on every Agents API JSON route, a non-JSON Content-Type, invalid UTF-8, malformed JSON, a repeated key at any depth or a non-object root returns invalid_request_error with a null param and the official message before other checks; an empty or null body is {}. Missing, unknown, wrongly typed or unsupported enum members of the pinned configuration shapes (tools, text, reasoning, service_tier, multi_agent) return invalid_request_error with the JSON path as param; duplicate function names, repeated web_search or tool_search and non-object schema root types return it with a null param. Supports model/name/instructions/metadata, explicit reasoning and service tiers, multi_agent, text/json_schema, function/tool_search/programmatic_tool_calling/web_search and HTTP MCP with nullable credential_id, service origin (omitted or null on HTTP transport is saved as service) and boolean required defaulting to false. Saving credential_id grants no access: Session admission checks attached Vault ownership and destination. MCP allowed_tools preserves null versus empty; saved HTTP transport includes empty headers. Model-derived reasoning defaults, other MCP variants and public retry conformance remain incomplete. web_search saves every pinned mode: omitted or null mode is saved as live and omitted or null context_size as medium; allowed_domains preserves null versus empty and a present location, including {}, includes all four keys with null for omitted ones, as observed officially (req_db41d2f6261b4abfb69465eafe719ab5, req_165d53b88445490b9146d8272c54134d). Session execution accepts only explicit disabled web_search and disabled programmatic_tool_calling through qualified Runtime controls; saved enabled forms reject at Session admission. Session execution admits only its supported configuration subset. // @Tags Agents // @Accept json // @Produce json From c4f3e232dff80f2babfaf0c9d4f8710b2dfdabd3 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 21:59:13 +0000 Subject: [PATCH 04/12] Scan request bodies in linear memory and parse media types strictly The duplicate-key scan keeps key positions in the body instead of copying every key: small objects compare hashed positions and objects with more than 16 keys use an open-addressing set of 4-byte positions, compared after unescaping. A 16 MiB body of short keys now allocates 16 MiB instead of 116 MiB, still in one linear pass. The same pass rejects string escapes that form a lone or mis-paired UTF-16 surrogate with the official parse error (req_1a9b7680d615454ca97c816b25e2f401). The JSON Content-Type is parsed with mime.ParseMediaType, so malformed media types get the Content-Type error. Tests extend the differential check to large objects, bound allocations on many short keys, prove the Beta-before-Content-Type order and drop the excluded-route test that did not reach its routes. --- .../internal/api/configuration_validation.go | 5 +- .../api/environment_files_wire_test.go | 4 +- .../agents-api/internal/api/json_request.go | 323 ++++++++++++++---- .../internal/api/json_request_test.go | 226 ++++++++---- 4 files changed, 411 insertions(+), 147 deletions(-) diff --git a/services/agents-api/internal/api/configuration_validation.go b/services/agents-api/internal/api/configuration_validation.go index 94e120135..2c768821e 100644 --- a/services/agents-api/internal/api/configuration_validation.go +++ b/services/agents-api/internal/api/configuration_validation.go @@ -398,8 +398,9 @@ func containsString(values []string, value string) bool { return false } -// orderedMembers returns every key of an object in document order, including -// repeated keys, and each key's last value. The input is valid JSON. +// orderedMembers returns every key of an object in document order with its +// value. The input is valid JSON without repeated keys, which the shared body +// gate rejects. func orderedMembers(raw json.RawMessage) ([]string, map[string]json.RawMessage) { decoder := json.NewDecoder(bytes.NewReader(raw)) fields := map[string]json.RawMessage{} diff --git a/services/agents-api/internal/api/environment_files_wire_test.go b/services/agents-api/internal/api/environment_files_wire_test.go index f05f3d3ed..cbdcfcee5 100644 --- a/services/agents-api/internal/api/environment_files_wire_test.go +++ b/services/agents-api/internal/api/environment_files_wire_test.go @@ -194,7 +194,6 @@ func TestEnvironmentFileCreateFieldErrors(t *testing.T) { `line\u2028separator`: false, `bell\u0007`: false, `caf\u00e9 \u5b57`: true, - `\ud800`: false, `\ufffd`: false, } { h, f := environmentFileCreateHandler(t) @@ -215,6 +214,9 @@ func TestEnvironmentFileCreateFieldErrors(t *testing.T) { h, f := environmentFileCreateHandler(t) w := requestCreateEnvironmentFile(h, f.environment.ID, "{\"type\":\"inline\",\"path\":\"/workspace/a\",\"data\":\"\",\"\xff\xfe\":1}", "files-key") assertListQueryError(t, w, "invalid_request_error", nil, "Invalid body: encountered a unicode decode error when parsing this JSON value. Please check the value to ensure it is valid unicode.") + // A lone surrogate escape is a parse error (req_1a9b7680d615454ca97c816b25e2f401). + w = requestCreateEnvironmentFile(h, f.environment.ID, `{"type":"inline","path":"/workspace/a","data":"","\ud800":1}`, "files-key") + assertListQueryError(t, w, "invalid_request_error", nil, "Invalid body: failed to parse JSON value. Please check the value to ensure it is valid JSON. (Common errors include trailing commas, missing closing brackets, missing quotation marks, etc.)") // Foreign Environments stay missing before route-specific body validation. h, f = environmentFileCreateHandler(t) body := `{"type":"inline","path":"/workspace/a","data":"","extra_field":1}` diff --git a/services/agents-api/internal/api/json_request.go b/services/agents-api/internal/api/json_request.go index d0b9ced48..88a87dd52 100644 --- a/services/agents-api/internal/api/json_request.go +++ b/services/agents-api/internal/api/json_request.go @@ -5,10 +5,12 @@ import ( "encoding/json" "errors" "fmt" + "hash/maphash" "io" + "mime" "net/http" - "slices" "strings" + "unicode/utf16" "unicode/utf8" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/echotext" @@ -72,10 +74,13 @@ func readJSONObjectLimit(w http.ResponseWriter, r *http.Request, limit int64, me } // jsonContentType accepts application/json and application/*+json media types -// case-insensitively, with any parameters. +// case-insensitively, with well-formed parameters. A malformed media type is +// rejected like a missing one. func jsonContentType(value string) bool { - mediaType, _, _ := strings.Cut(value, ";") - mediaType = strings.ToLower(strings.TrimSpace(mediaType)) + mediaType, _, err := mime.ParseMediaType(value) + if err != nil { + return false + } if mediaType == "application/json" { return true } @@ -91,10 +96,15 @@ func jsonObjectBody(raw []byte) ([]byte, error) { return nil, errBodyUnicode } // A byte order mark, a whitespace-only body or trailing data is invalid JSON. - if !json.Valid(raw) { + // Key positions are 31-bit; route limits keep bodies far below that. + if uint64(len(raw)) >= keyEscaped || !json.Valid(raw) { return nil, errBodyParse } - if key, path, found := duplicateJSONKey(raw); found { + key, path, found, err := scanJSON(raw) + if err != nil { + return nil, err + } + if found { if !echotext.Allowed(key) || !echotext.Allowed(path) { return nil, errBodyDuplicateKey } @@ -111,91 +121,260 @@ func jsonObjectBody(raw []byte) ([]byte, error) { } } -// duplicateJSONKey returns the first repeated object key of a valid JSON value -// in document order, with its path of object keys joined by '.'. Array indices -// are omitted, as observed officially: 'metadata.k' and 'tools.type'. Keys are -// compared after unescaping. The scan is linear, and the path is built only for -// the reported key. -func duplicateJSONKey(raw []byte) (key, path string, found bool) { - type frame struct { - object bool - expectKey bool - keys [][]byte // the keys of a small object - set map[string]struct{} // every key once an object is large - member []byte // the key whose value is being read - } - var stack []frame +// keyEscaped marks a key position whose string contains escapes. A position is +// the offset of the key's opening quote plus one, so zero means empty. +const keyEscaped = 1 << 31 + +// smallObjectKeys is the number of keys an object compares linearly before it +// switches to an open-addressing set. +const smallObjectKeys = 16 + +type scannedKey struct { + position uint32 + hash uint64 +} + +type scanFrame struct { + object, expectKey bool + member uint32 // the position of the key whose value is being read + base, count int // this object's keys in keyScanner.small while it is small + table []uint32 // key positions once the object is large +} + +// keyScanner finds repeated keys without copying them: keys are positions in +// the body, compared and hashed after unescaping only when they contain escapes. +type keyScanner struct { + raw []byte + seed maphash.Seed + frames []scanFrame + small []scannedKey + left, right []byte // unescaping buffers +} + +// scanJSON checks a valid JSON value in one linear pass in document order. A +// string escape that forms a lone or mis-paired UTF-16 surrogate is a parse +// error, as observed officially (req_1a9b7680d615454ca97c816b25e2f401); valid +// pairs are accepted. It returns the first repeated object key, compared after +// unescaping, with its path of object keys joined by '.'. Array indices are +// omitted, as observed officially: 'metadata.k' and 'tools.type'. Names differing +// only in case are distinct keys. +func scanJSON(raw []byte) (key, path string, found bool, err error) { + s := keyScanner{raw: raw, seed: maphash.MakeSeed()} for i := 0; i < len(raw); { - switch raw[i] { + switch c := raw[i]; c { case '{', '[': - if len(stack) < cap(stack) { - // Reuse the key slice of an earlier sibling at this depth. - stack = stack[:len(stack)+1] - keys := stack[len(stack)-1].keys[:0] - stack[len(stack)-1] = frame{keys: keys} - } else { - stack = append(stack, frame{}) - } - top := &stack[len(stack)-1] - top.object, top.expectKey = raw[i] == '{', raw[i] == '{' + s.frames = append(s.frames, scanFrame{object: c == '{', expectKey: c == '{', base: len(s.small)}) i++ case '}', ']': - stack = stack[:len(stack)-1] + closed := &s.frames[len(s.frames)-1] + s.small = s.small[:closed.base] + *closed = scanFrame{} + s.frames = s.frames[:len(s.frames)-1] i++ case ',': - if top := &stack[len(stack)-1]; top.object { + if top := &s.frames[len(s.frames)-1]; top.object { top.expectKey = true } i++ case '"': - end := i + 1 - for { - end += bytes.IndexAny(raw[end:], "\\\"") - if raw[end] == '"' { - break + end, escaped, ok := scanString(raw, i) + if !ok { + return "", "", false, errBodyParse + } + if n := len(s.frames); n > 0 && s.frames[n-1].expectKey { + position := uint32(i + 1) + if escaped { + position |= keyEscaped + } + if s.insert(&s.frames[n-1], position) { + key, path := s.duplicate(position) + return key, path, true, nil } - end += 2 } - end++ - if len(stack) == 0 || !stack[len(stack)-1].expectKey { - i = end + i = end + default: + i++ + } + } + return "", "", false, nil +} + +// scanString returns the offset after the string starting at raw[quote] and +// whether it contains escapes, or false for an unpaired surrogate escape. +func scanString(raw []byte, quote int) (end int, escaped, ok bool) { + for i := quote + 1; ; { + switch raw[i] { + case '"': + return i + 1, escaped, true + case '\\': + escaped = true + if raw[i+1] != 'u' { + i += 2 continue } - top := &stack[len(stack)-1] - name := raw[i+1 : end-1] - if bytes.IndexByte(name, '\\') >= 0 { - var decoded string - _ = json.Unmarshal(raw[i:end], &decoded) - name = []byte(decoded) - } - repeated := false - if top.set != nil { - _, repeated = top.set[string(name)] - top.set[string(name)] = struct{}{} - } else { - repeated = slices.ContainsFunc(top.keys, func(k []byte) bool { return bytes.Equal(k, name) }) - top.keys = append(top.keys, name) - if len(top.keys) > 16 { - top.set = make(map[string]struct{}, 32) - for _, k := range top.keys { - top.set[string(k)] = struct{}{} - } + r := hex4(raw[i+2 : i+6]) + i += 6 + if utf16.IsSurrogate(r) { + if r >= 0xdc00 || raw[i] != '\\' || raw[i+1] != 'u' { + return 0, true, false } - } - if repeated { - var segments []string - for _, f := range stack[:len(stack)-1] { - if f.object { - segments = append(segments, string(f.member)) - } + if low := hex4(raw[i+2 : i+6]); low < 0xdc00 || low > 0xdfff { + return 0, true, false } - return string(name), strings.Join(append(segments, string(name)), "."), true + i += 6 } - top.member, top.expectKey = name, false - i = end default: i++ } } - return "", "", false +} + +func hex4(digits []byte) rune { + var r rune + for _, c := range digits { + switch { + case c >= 'a': + c -= 'a' - 10 + case c >= 'A': + c -= 'A' - 10 + default: + c -= '0' + } + r = r<<4 | rune(c) + } + return r +} + +// content returns the raw bytes of the key at position, without quotes. +func (s *keyScanner) content(position uint32) []byte { + start := int(position&^keyEscaped) - 1 + for i := start + 1; ; i++ { + switch s.raw[i] { + case '"': + return s.raw[start+1 : i] + case '\\': + i++ + } + } +} + +func (s *keyScanner) hash(position uint32) uint64 { + if position&keyEscaped == 0 { + return maphash.Bytes(s.seed, s.content(position)) + } + s.left = appendUnescaped(s.left[:0], s.content(position)) + return maphash.Bytes(s.seed, s.left) +} + +// equal compares two keys after unescaping. Unescaped keys compare in place: +// their contents contain no quote, so a length mismatch fails at a quote. +func (s *keyScanner) equal(a, b uint32) bool { + if a&keyEscaped == 0 && b&keyEscaped == 0 { + content := s.content(b) + start := int(a) - 1 + end := start + 1 + len(content) + return end < len(s.raw) && s.raw[end] == '"' && bytes.Equal(s.raw[start+1:end], content) + } + s.left = appendUnescaped(s.left[:0], s.content(a)) + s.right = appendUnescaped(s.right[:0], s.content(b)) + return bytes.Equal(s.left, s.right) +} + +// insert records a key of the top object and reports whether it repeats one. +func (s *keyScanner) insert(f *scanFrame, position uint32) bool { + f.member, f.expectKey = position, false + hash := s.hash(position) + if f.table == nil { + for _, key := range s.small[f.base : f.base+f.count] { + if key.hash == hash && s.equal(key.position, position) { + return true + } + } + s.small = append(s.small, scannedKey{position, hash}) + if f.count++; f.count > smallObjectKeys { + f.table = make([]uint32, 4*smallObjectKeys) + for _, key := range s.small[f.base:] { + s.place(f.table, key.position, key.hash) + } + s.small = s.small[:f.base] + } + return false + } + // Linear probing at a load factor of at most 3/4. + if (f.count+1)*4 > len(f.table)*3 { + grown := make([]uint32, 2*len(f.table)) + for _, key := range f.table { + if key != 0 { + s.place(grown, key, s.hash(key)) + } + } + f.table = grown + } + mask := uint64(len(f.table) - 1) + for i := hash & mask; ; i = (i + 1) & mask { + switch key := f.table[i]; { + case key == 0: + f.table[i] = position + f.count++ + return false + case s.equal(key, position): + return true + } + } +} + +func (s *keyScanner) place(table []uint32, position uint32, hash uint64) { + mask := uint64(len(table) - 1) + i := hash & mask + for table[i] != 0 { + i = (i + 1) & mask + } + table[i] = position +} + +// duplicate returns the repeated key and its path through the enclosing objects. +func (s *keyScanner) duplicate(position uint32) (string, string) { + var segments []string + for _, f := range s.frames[:len(s.frames)-1] { + if f.object { + segments = append(segments, string(appendUnescaped(nil, s.content(f.member)))) + } + } + key := string(appendUnescaped(nil, s.content(position))) + return key, strings.Join(append(segments, key), ".") +} + +// appendUnescaped decodes the contents of a valid JSON string whose surrogate +// escapes are paired. +func appendUnescaped(dst, s []byte) []byte { + for i := 0; i < len(s); i++ { + if s[i] != '\\' { + dst = append(dst, s[i]) + continue + } + i++ + switch s[i] { + case 'b': + dst = append(dst, '\b') + case 'f': + dst = append(dst, '\f') + case 'n': + dst = append(dst, '\n') + case 'r': + dst = append(dst, '\r') + case 't': + dst = append(dst, '\t') + case 'u': + r := hex4(s[i+1 : i+5]) + i += 4 + if utf16.IsSurrogate(r) { + r = utf16.DecodeRune(r, hex4(s[i+3:i+7])) + i += 6 + } + dst = utf8.AppendRune(dst, r) + default: + dst = append(dst, s[i]) + } + } + return dst } diff --git a/services/agents-api/internal/api/json_request_test.go b/services/agents-api/internal/api/json_request_test.go index 0e21676bc..c223adee0 100644 --- a/services/agents-api/internal/api/json_request_test.go +++ b/services/agents-api/internal/api/json_request_test.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/http/httptest" + "runtime" "strings" "testing" @@ -45,23 +46,43 @@ func TestJSONObjectBodyChecks(t *testing.T) { "{\"name\":\xff": bodyUnicodeMessage, "\xef\xbb": bodyUnicodeMessage, // B3 (HP-11): object keys joined by '.', array indices omitted. - `{"name":"a","name":"b"}`: duplicateKeyMessage("name", "name"), - `{"metadata":{"k":"1","k":"2"}}`: duplicateKeyMessage("k", "metadata.k"), - `{"tools":[{"type":"function","type":"function"}]}`: duplicateKeyMessage("type", "tools.type"), - `{"a":[[{"b":1}],[{"c":{"d":1,"d":2}}]]}`: duplicateKeyMessage("d", "a.c.d"), - `{"a":1,"a":2}`: duplicateKeyMessage("a", "a"), - `{"x":{"k":1},"y":{"k":1},"x":2}`: duplicateKeyMessage("x", "x"), - `{"n":1e400,"s":"}","n":1}`: duplicateKeyMessage("n", "n"), - `{"":1,"":2}`: duplicateKeyMessage("", ""), - `{"":{"a":1,"a":2}}`: duplicateKeyMessage("a", ".a"), - `[{"a":1,"a":2}]`: duplicateKeyMessage("a", "a"), + `{"name":"a","name":"b"}`: duplicateKeyMessage("name", "name"), + `{"metadata":{"k":"1","k":"2"}}`: duplicateKeyMessage("k", "metadata.k"), + `{"tools":[{"type":"function","type":"function"}]}`: duplicateKeyMessage("type", "tools.type"), + `{"a":[[{"b":1}],[{"c":{"d":1,"d":2}}]]}`: duplicateKeyMessage("d", "a.c.d"), + `{"a":1,"\u0061":2}`: duplicateKeyMessage("a", "a"), + `{"x":{"k":1},"y":{"k":1},"x":2}`: duplicateKeyMessage("x", "x"), + `{"n":1e400,"s":"}","n":1}`: duplicateKeyMessage("n", "n"), + `{"":1,"":2}`: duplicateKeyMessage("", ""), + `{"":{"a":1,"a":2}}`: duplicateKeyMessage("a", ".a"), + `[{"a":1,"a":2}]`: duplicateKeyMessage("a", "a"), `{"events":[{"input":[{"content":[{"text":"x","text":"y"}]}]}]}`: duplicateKeyMessage("text", "events.input.content.text"), // Bounded echo (echotext.Allowed) for the key and its path. `{"` + long + `":1,"` + long + `":2}`: "Invalid body: duplicate JSON key. Duplicate JSON keys are not supported.", `{"` + long[:200] + `":{"` + long[:60] + `":1,"` + long[:60] + `":2}}`: "Invalid body: duplicate JSON key. Duplicate JSON keys are not supported.", `{"tab\tkey":1,"tab\tkey":2}`: "Invalid body: duplicate JSON key. Duplicate JSON keys are not supported.", - `{"\ud800":1,"\ud800":2}`: "Invalid body: duplicate JSON key. Duplicate JSON keys are not supported.", - `{"<>":{"café":1,"café":2}}`: duplicateKeyMessage("café", "<>.café"), + `{"\u2028":1,"\u2028":2}`: "Invalid body: duplicate JSON key. Duplicate JSON keys are not supported.", + // A lone or mis-paired surrogate escape is invalid JSON, in keys and values, + // before a later duplicate (official req_1a9b7680d615454ca97c816b25e2f401). + `{"name":"\ud800"}`: bodyParseMessage, + `{"name":"\udc00"}`: bodyParseMessage, + `{"name":"a\uD83D"}`: bodyParseMessage, + `{"name":"\ud83d\u0041"}`: bodyParseMessage, + `{"name":"\ud83d\ud83d"}`: bodyParseMessage, + `{"name":"\ude00\ud83d"}`: bodyParseMessage, + `{"name":"\ud83d\n\ude00"}`: bodyParseMessage, + `{"\ud800":1,"\ud800":2}`: bodyParseMessage, + `["\ud800"]`: bodyParseMessage, + `"\ud800"`: bodyParseMessage, + `{"a":1,"b":"\udfff","a":2}`: bodyParseMessage, + `{"a":1,"a":"\udfff"}`: duplicateKeyMessage("a", "a"), + // Names differing only in case are distinct keys; escapes compare decoded. + `{"k":1,"K":2,"\u004b":3}`: duplicateKeyMessage("K", "K"), + `{"\ud83d\ude00":1,"😀":2}`: duplicateKeyMessage("😀", "😀"), + `{"a\\":1,"a\u005c":2}`: duplicateKeyMessage(`a\`, `a\`), + `{"a\/b":1,"a/b":2}`: duplicateKeyMessage("a/b", "a/b"), + `{"x":{"\u0078":{"a\"b":1,"a\u0022b":2}}}`: duplicateKeyMessage(`a"b`, `x.x.a"b`), + `{"<>":{"caf\u00e9":1,"café":2}}`: duplicateKeyMessage("café", "<>.café"), // B4 (HP-12, HP-14). `"scan6"`: "Invalid type: expected an object, but got a string instead.", `5`: "Invalid type: expected an object, but got an integer instead.", @@ -95,7 +116,9 @@ func TestJSONObjectBodyChecks(t *testing.T) { `{}`: `{}`, ` {"a":[{"b":1},{"b":2}],"c":{"b":3}} `: ` {"a":[{"b":1},{"b":2}],"c":{"b":3}} `, `{"x_agents_core":{"model_provider":null},"metadata":{"k":"v"}}`: `{"x_agents_core":{"model_provider":null},"metadata":{"k":"v"}}`, - `{"name":"é😀"}`: `{"name":"é😀"}`, + `{"name":"\u00e9\ud83d\ude00"}`: `{"name":"\u00e9\ud83d\ude00"}`, + `{"k":1,"K":2,"Metadata":{},"metadata":{}}`: `{"k":1,"K":2,"Metadata":{},"metadata":{}}`, + `{"a\\":1,"a\\\\":2,"a\"":3,"a\u005cb":4}`: `{"a\\":1,"a\\\\":2,"a\"":3,"a\u005cb":4}`, } { got, err := jsonObjectBody([]byte(body)) if err != nil || string(got) != want { @@ -106,24 +129,32 @@ func TestJSONObjectBodyChecks(t *testing.T) { func TestJSONContentType(t *testing.T) { for value, want := range map[string]bool{ - "application/json": true, - "application/json; charset=utf-8": true, - "Application/JSON": true, - " application/json ;charset=UTF-8": true, - "application/merge-patch+json": true, - "APPLICATION/VND.API+JSON; x=y": true, - "": false, - "text/plain": false, - "application/x-www-form-urlencoded": false, - "multipart/form-data; boundary=x": false, - "text/json": false, - "application/jsonx": false, - "application/json-seq": false, - "application/+json": false, - "application/json+xml": false, - "application / json": false, - "application/octet-stream; t=+json": false, - "application/x-json-stream; t=a+json": false, + "application/json": true, + "application/json; charset=utf-8": true, + "Application/JSON": true, + " application/json ;charset=UTF-8": true, + "application/merge-patch+json": true, + "APPLICATION/VND.API+JSON; x=y": true, + "": false, + "text/plain": false, + "application/x-www-form-urlencoded": false, + "multipart/form-data; boundary=x": false, + "text/json": false, + "application/jsonx": false, + "application/json-seq": false, + "application/+json": false, + "application/json+xml": false, + "application / json": false, + "application/foo bar+json": false, + "application/json garbage": false, + "application/json; charset": false, + "application/json; charset=utf-8; charset=latin1": false, + "application/json;;": false, + "application/json, text/plain": false, + "application/json; charset=\"utf-8\"": true, + "application/json;": true, + "application/octet-stream; t=+json": false, + "application/x-json-stream; t=a+json": false, } { if got := jsonContentType(value); got != want { t.Errorf("%q: got %t, want %t", value, got, want) @@ -220,15 +251,18 @@ func TestAgentsJSONRoutesShareBodyGate(t *testing.T) { func TestAgentsJSONBodyGateOrder(t *testing.T) { h, s := validationHandler(t) for _, route := range agentsJSONRoutes() { + // A bad Content-Type and a malformed body prove the order. r := httptest.NewRequest(http.MethodPost, route.path, strings.NewReader(`{"name":`)) r.Header.Set("Authorization", "Bearer test-api-key") + r.Header.Set("Content-Type", "text/plain") w := httptest.NewRecorder() h.ServeHTTP(w, r) - if w.Code != http.StatusBadRequest || strings.Contains(w.Body.String(), "Invalid body") { + if w.Code != http.StatusBadRequest || !strings.Contains(w.Body.String(), `"invalid_beta"`) { t.Errorf("%s: missing Beta: %d %s", route.name, w.Code, w.Body) } r = httptest.NewRequest(http.MethodPost, route.path, strings.NewReader(`{"name":`)) r.Header.Set("OpenAI-Beta", "agents=v1") + r.Header.Set("Content-Type", "text/plain") w = httptest.NewRecorder() h.ServeHTTP(w, r) if w.Code != http.StatusUnauthorized { @@ -279,32 +313,6 @@ func TestAgentsJSONBodyGateKeepsValidBodies(t *testing.T) { } } -// DELETE, multipart uploads and Core extension routes keep their own body -// handling without the Content-Type rule. -func TestBodyGateExcludedRoutes(t *testing.T) { - h, _ := validationHandler(t) - for _, path := range []string{"/v1/agents/" + uuid.NewString(), "/v1/vaults/" + uuid.NewString(), "/v1/agents/sessions/" + uuid.NewString()} { - r := httptest.NewRequest(http.MethodDelete, path, strings.NewReader(`{"name":`)) - r.Header.Set("Authorization", "Bearer test-api-key") - r.Header.Set("OpenAI-Beta", "agents=v1") - w := httptest.NewRecorder() - h.ServeHTTP(w, r) - if w.Code != http.StatusBadRequest || strings.Contains(w.Body.String(), "Content-Type") || strings.Contains(w.Body.String(), "Invalid body") { - t.Errorf("DELETE %s: %d %s", path, w.Code, w.Body) - } - } - for _, request := range []struct{ path, contentType string }{ - {"/v1/files", "multipart/form-data; boundary=x"}, - {"/v1/skills", "multipart/form-data; boundary=x"}, - {"/core/v1/environments/" + uuid.NewString() + "/executor-credentials/", ""}, - } { - w := bodyGateRequest(h, request.path, request.contentType, []byte("--x--\r\n")) - if strings.Contains(w.Body.String(), bodyContentTypeMessage) || strings.Contains(w.Body.String(), "Invalid body") { - t.Errorf("POST %s: %d %s", request.path, w.Code, w.Body) - } - } -} - // referenceDuplicateJSONKey walks encoding/json tokens; the byte scanner must // agree with it. func referenceDuplicateJSONKey(raw []byte) (string, string, bool) { @@ -358,49 +366,123 @@ func referenceDuplicateJSONKey(raw []byte) (string, string, bool) { } } +// The scan agrees with the reference on small objects and on objects with more +// than smallObjectKeys members, which use the open-addressing set. func TestDuplicateJSONKeyMatchesReference(t *testing.T) { - names := []string{`a`, `b`, `a`, `a\"b`, `a\\b`, ``, `k,:{}[]`, `\ud800`, `�`, `é`, `é`} - values := []string{`1`, `-2.5e3`, `true`, `null`, `"x,y:{}[]"`, `"\"a\":1"`, `"\\"`, `[]`, `{}`} + names := []string{`a`, `b`, `\u0061`, `a\"b`, `a\\b`, `a\u005cb`, ``, `k,:{}[]`, `\ud83d\ude00`, `😀`, `\ufffd`, `é`, `\u00e9`, `K`, `k`} + values := []string{`1`, `-2.5e3`, `true`, `null`, `"x,y:{}[]"`, `"\"a\":1"`, `"\\"`, `"\ud83d\ude00"`, `[]`, `{}`} random := uint64(1) next := func(n int) int { random = random*6364136223846793005 + 1442695040888963407 return int(random>>33) % n } - var value func(depth int) string - value = func(depth int) string { + large := 0 + var value func(depth, width, suffixes int) string + value = func(depth, width, suffixes int) string { switch choice := next(4); { case depth > 3 || choice == 0: return values[next(len(values))] case choice == 1: items := make([]string, next(4)) for i := range items { - items[i] = value(depth + 1) + items[i] = value(depth+1, width, suffixes) } return "[" + strings.Join(items, ",") + "]" default: - members := make([]string, next(6)) + members := make([]string, next(width)) + if len(members) > smallObjectKeys { + large++ + } + // Members of a large object nest less deeply, in small objects. + child, childWidth := depth+1, width + if width > smallObjectKeys { + child, childWidth = depth+2, 6 + } for i := range members { - members[i] = `"` + names[next(len(names))] + fmt.Sprint(next(3)) + `": ` + value(depth+1) + members[i] = `"` + names[next(len(names))] + fmt.Sprint(next(suffixes)) + `": ` + value(child, childWidth, suffixes) } return "{" + strings.Join(members, ",") + "}" } } duplicates := 0 - for range 20000 { - body := []byte(value(0)) + for i := range 20000 { + width, suffixes := 6, 3 + if i%2 == 1 { + width, suffixes = 60, 200 + } + body := []byte(value(0, width, suffixes)) if !json.Valid(body) { t.Fatalf("invalid generated body %s", body) } - key, path, found := duplicateJSONKey(body) + key, path, found, err := scanJSON(body) wantKey, wantPath, wantFound := referenceDuplicateJSONKey(body) - if key != wantKey || path != wantPath || found != wantFound { - t.Fatalf("%s: got %q %q %t, want %q %q %t", body, key, path, found, wantKey, wantPath, wantFound) + if err != nil || key != wantKey || path != wantPath || found != wantFound { + t.Fatalf("%s: got %q %q %t %v, want %q %q %t", body, key, path, found, err, wantKey, wantPath, wantFound) } if found { duplicates++ } } - if duplicates < 1000 || duplicates > 19000 { - t.Fatalf("unbalanced generated bodies: %d duplicates", duplicates) + if duplicates < 1000 || duplicates > 19000 || large < 1000 { + t.Fatalf("unbalanced generated bodies: %d duplicates, %d large objects", duplicates, large) + } + // Deterministic large objects: the repeat at every position, escaped forms + // and nesting inside a large object. + for size := smallObjectKeys + 1; size <= 300; size += 7 { + for _, repeat := range []int{0, smallObjectKeys - 1, smallObjectKeys, size / 2, size - 1} { + members := make([]string, size) + for i := range members { + members[i] = fmt.Sprintf(`"k%d":{"k%d":[{"x":1}]}`, i, i) + } + valid := []byte("{" + strings.Join(members, ",") + "}") + if _, _, found, err := scanJSON(valid); found || err != nil { + t.Fatalf("size %d: false duplicate %v", size, err) + } + escaped := fmt.Sprintf(`"\u006b%d":2`, repeat) + body := []byte("{" + strings.Join(append(members, escaped), ",") + "}") + key, path, found, err := scanJSON(body) + if want := fmt.Sprintf("k%d", repeat); !found || err != nil || key != want || path != want { + t.Fatalf("size %d repeat %d: %q %q %t %v", size, repeat, key, path, found, err) + } + } + } +} + +// A body of many short keys needs memory proportional to its key count, not +// a copy of every key: 116 MiB was allocated for this 16 MiB body before, and +// about 16 MiB, mostly the growing key-position set, is allocated now. +func TestDuplicateJSONKeyMemory(t *testing.T) { + body := manyShortKeys(16 << 20) + runtime.GC() + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + _, _, found, err := scanJSON(body) + runtime.ReadMemStats(&after) + if found || err != nil { + t.Fatal(found, err) + } + if allocated := after.TotalAlloc - before.TotalAlloc; allocated > 2*uint64(len(body)) { + t.Fatalf("allocated %d MiB for a %d MiB body", allocated>>20, len(body)>>20) + } +} + +func BenchmarkDuplicateJSONKeyManyShortKeys(b *testing.B) { + body := manyShortKeys(16 << 20) + b.ReportAllocs() + b.SetBytes(int64(len(body))) + for b.Loop() { + if _, _, found, err := scanJSON(body); found || err != nil { + b.Fatal(found, err) + } + } +} + +func manyShortKeys(size int) []byte { + var body bytes.Buffer + body.WriteString("{") + for i := 0; body.Len() < size; i++ { + fmt.Fprintf(&body, `"k%07d":0,`, i) } + body.WriteString(`"z":0}`) + return body.Bytes() } From 637771d195d4d05457ac4a4bfdeab6b5a560545a Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 21:59:14 +0000 Subject: [PATCH 05/12] Match request member names exactly encoding/json matched a case variant such as Metadata, Input or a nested Role to the field and let the last copy win, so Session create, events and nested credential and input objects accepted and stored it. The official service treats such names as distinct keys (req_6ba2a50c71a4410f87a1baac855e82df). caseVariantMember makes a case variant an unknown member at any struct depth, in decodeInputObject and the Session create and events decoders, rejected with each route's existing unknown-member error before any write. The Credential auth type is read exactly too. --- .../internal/api/credentials_oauth.go | 10 +- .../internal/api/function_inputs.go | 5 + services/agents-api/internal/api/handler.go | 4 +- services/agents-api/internal/api/inputs.go | 4 +- .../agents-api/internal/api/json_members.go | 114 ++++++++++++++++++ .../internal/api/json_members_test.go | 113 +++++++++++++++++ .../store/request_body_public_test.go | 18 +++ .../tests/official_session_requests.py | 4 + 8 files changed, 265 insertions(+), 7 deletions(-) create mode 100644 services/agents-api/internal/api/json_members.go create mode 100644 services/agents-api/internal/api/json_members_test.go diff --git a/services/agents-api/internal/api/credentials_oauth.go b/services/agents-api/internal/api/credentials_oauth.go index d7b7d1734..ba7159cc4 100644 --- a/services/agents-api/internal/api/credentials_oauth.go +++ b/services/agents-api/internal/api/credentials_oauth.go @@ -31,14 +31,14 @@ func credentialExpiry(value *string) bool { return err == nil } +// credentialAuthType reads the exact type member; a case variant is unknown. func credentialAuthType(raw json.RawMessage) string { - var value struct { - Type string `json:"type"` - } - if json.Unmarshal(raw, &value) != nil { + var fields map[string]json.RawMessage + var value string + if json.Unmarshal(raw, &fields) != nil || json.Unmarshal(fields["type"], &value) != nil { return "" } - return value.Type + return value } func oauthCredentialCreate(raw json.RawMessage, name string) (store.CreateOAuthCredentialInput, error) { diff --git a/services/agents-api/internal/api/function_inputs.go b/services/agents-api/internal/api/function_inputs.go index 5cbe20840..40f2f9991 100644 --- a/services/agents-api/internal/api/function_inputs.go +++ b/services/agents-api/internal/api/function_inputs.go @@ -3,6 +3,7 @@ package api import ( "bytes" "encoding/json" + "reflect" "slices" v1 "github.com/MiniMax-AI-Dev/parsar/contracts/agents-api/v1" @@ -62,6 +63,10 @@ func decodeInputObject(raw json.RawMessage, value any, allowed ...string) error return store.ErrInvalidInput } } + // Nested members match exactly too; see caseVariantMember. + if caseVariantMember(raw, reflect.TypeOf(value)) { + return store.ErrInvalidInput + } decoder := json.NewDecoder(bytes.NewReader(raw)) decoder.DisallowUnknownFields() if decoder.Decode(value) != nil { diff --git a/services/agents-api/internal/api/handler.go b/services/agents-api/internal/api/handler.go index f7f023577..9f41c033b 100644 --- a/services/agents-api/internal/api/handler.go +++ b/services/agents-api/internal/api/handler.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net/http" + "reflect" v1 "github.com/MiniMax-AI-Dev/parsar/contracts/agents-api/v1" "github.com/MiniMax-AI-Dev/parsar/internal/obs/log" @@ -189,7 +190,8 @@ func (h *Handler) createSession(w http.ResponseWriter, r *http.Request) { var request decodedSessionRequest decoder := json.NewDecoder(bytes.NewReader(raw)) decoder.DisallowUnknownFields() - if err := decoder.Decode(&request); err != nil { + // A case variant of a member, such as Metadata, is an unknown member. + if caseVariantMember(raw, reflect.TypeOf(request)) || decoder.Decode(&request) != nil { writeError(w, http.StatusBadRequest, "invalid_request", "Request must be a JSON object containing supported fields.") return } diff --git a/services/agents-api/internal/api/inputs.go b/services/agents-api/internal/api/inputs.go index 9a33d45de..195d6b11e 100644 --- a/services/agents-api/internal/api/inputs.go +++ b/services/agents-api/internal/api/inputs.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "net/http" + "reflect" "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/proto" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" @@ -44,7 +45,8 @@ func (h *Handler) createEvents(w http.ResponseWriter, r *http.Request) { } decoder := json.NewDecoder(bytes.NewReader(raw)) decoder.DisallowUnknownFields() - if err := decoder.Decode(&request); err != nil { + // A case variant of events is an unknown member; events decode exactly too. + if caseVariantMember(raw, reflect.TypeOf(request)) || decoder.Decode(&request) != nil { writeError(w, http.StatusBadRequest, "invalid_request", "Invalid Session input event request.") return } diff --git a/services/agents-api/internal/api/json_members.go b/services/agents-api/internal/api/json_members.go new file mode 100644 index 000000000..87209cb4a --- /dev/null +++ b/services/agents-api/internal/api/json_members.go @@ -0,0 +1,114 @@ +package api + +import ( + "encoding/json" + "reflect" + "strings" + "sync" +) + +// caseVariantMember reports whether raw has an object key, at any depth that +// decodes into a struct of type t, that encoding/json would match to a field +// only case-insensitively. The official service matches member names exactly, +// so such a key is an unknown member, never an alias of the field +// (req_6ba2a50c71a4410f87a1baac855e82df). Unknown keys and values of other types +// are left to the decoder. raw has passed the shared body gate. +func caseVariantMember(raw []byte, t reflect.Type) bool { + for t.Kind() == reflect.Pointer { + t = t.Elem() + } + if t == rawMessageType || reflect.PointerTo(t).Implements(unmarshalerType) { + return false + } + switch t.Kind() { + case reflect.Struct: + var members map[string]json.RawMessage + if json.Unmarshal(raw, &members) != nil { + return false + } + fields := jsonFields(t) + for key, value := range members { + field, exact := fields[key] + if exact { + if caseVariantMember(value, field) { + return true + } + continue + } + for name := range fields { + if strings.EqualFold(name, key) { + return true + } + } + } + case reflect.Map: + var members map[string]json.RawMessage + if json.Unmarshal(raw, &members) != nil { + return false + } + for _, value := range members { + if caseVariantMember(value, t.Elem()) { + return true + } + } + case reflect.Slice, reflect.Array: + var items []json.RawMessage + if json.Unmarshal(raw, &items) != nil { + return false + } + for _, item := range items { + if caseVariantMember(item, t.Elem()) { + return true + } + } + } + return false +} + +var ( + rawMessageType = reflect.TypeFor[json.RawMessage]() + unmarshalerType = reflect.TypeFor[json.Unmarshaler]() + jsonFieldCache sync.Map // reflect.Type -> map[string]reflect.Type +) + +// jsonFields returns the JSON member names of a struct type with their field +// types, including promoted fields of untagged embedded structs, where the +// shallowest field of a name wins as in encoding/json. +func jsonFields(t reflect.Type) map[string]reflect.Type { + if cached, ok := jsonFieldCache.Load(t); ok { + return cached.(map[string]reflect.Type) + } + fields := map[string]reflect.Type{} + depths := map[string]int{} + var collect func(t reflect.Type, depth int) + collect = func(t reflect.Type, depth int) { + for i := range t.NumField() { + field := t.Field(i) + tag := field.Tag.Get("json") + if tag == "-" { + continue + } + name, _, _ := strings.Cut(tag, ",") + embedded := field.Type + if embedded.Kind() == reflect.Pointer { + embedded = embedded.Elem() + } + if field.Anonymous && name == "" && embedded.Kind() == reflect.Struct { + collect(embedded, depth+1) + continue + } + if !field.IsExported() { + continue + } + if name == "" { + name = field.Name + } + if seen, ok := depths[name]; !ok || depth < seen { + fields[name], depths[name] = field.Type, depth + } + } + } + collect(t, 0) + jsonFieldCache.Store(t, fields) + return fields +} diff --git a/services/agents-api/internal/api/json_members_test.go b/services/agents-api/internal/api/json_members_test.go new file mode 100644 index 000000000..01af0cbe1 --- /dev/null +++ b/services/agents-api/internal/api/json_members_test.go @@ -0,0 +1,113 @@ +package api + +import ( + "encoding/json" + "net/http" + "reflect" + "strings" + "testing" + + "github.com/google/uuid" +) + +func TestCaseVariantMember(t *testing.T) { + type inner struct { + Name string `json:"name"` + } + type embedded struct { + Shared string `json:"shared"` + Hidden string `json:"hidden"` + } + type outer struct { + embedded + Hidden json.RawMessage `json:"hidden"` + Items []inner `json:"items"` + Named map[string]*inner `json:"named"` + Skipped string `json:"-"` + Plain string + Nested *inner `json:"nested,omitempty"` + } + typ := reflect.TypeFor[outer]() + for body, want := range map[string]bool{ + `{"shared":"a","hidden":{"Name":1},"items":[{"name":"a"}],"named":{"K":{"name":"b"}},"Plain":"c","nested":{"name":"d"},"unknown":1}`: false, + `{"Shared":"a"}`: true, + `{"HIDDEN":{}}`: true, + `{"items":[{"name":"a"},{"NAME":"b"}]}`: true, + `{"named":{"k":{"Name":"b"}}}`: true, + `{"nested":{"nAme":"d"}}`: true, + `{"plain":"c"}`: true, + `{"Skipped":"x"}`: false, + `{"sHaReD":1,"shared":2}`: true, + `{"items":"not an array","named":null}`: false, + `{"ſhared":"long s folds to s"}`: true, + } { + if got := caseVariantMember([]byte(body), typ); got != want { + t.Errorf("%s: got %t, want %t", body, got, want) + } + } +} + +// A case variant of a member is an unknown member, rejected with the route's +// existing unknown-member error instead of replacing the field +// (req_6ba2a50c71a4410f87a1baac855e82df). +func TestCaseVariantMembersAreUnknown(t *testing.T) { + h, s := validationHandler(t) + session := `{"agent":{"model":"m"},"environment":{"type":"none"},"input":"hi"` + unknownSession := `{"error":{"message":"Request must be a JSON object containing supported fields.","type":"invalid_request_error","code":"invalid_request","param":null}}` + "\n" + for _, body := range []string{ + session + `,"Metadata":{"k":"v"}}`, + session + `,"metadata":{"k":"v"},"Metadata":{"k":"w"}}`, + session + `,"Input":"replaced"}`, + session + `,"STREAM":true}`, + } { + if w := bodyGateRequest(h, "/v1/agents/sessions", "application/json", []byte(body)); w.Code != http.StatusBadRequest || w.Body.String() != unknownSession { + t.Errorf("%s: %d %s", body, w.Code, w.Body) + } + } + // Nested members keep their existing unknown-member errors. + for _, body := range []string{ + session + `,"x_agents_core":{"Model_Provider":null}}`, + `{"agent":{"model":"m"},"environment":{"type":"none","Type":"self_hosted"},"input":"hi"}`, + `{"agent":{"model":"m"},"environment":{"type":"none"},"input":[{"role":"assistant","Role":"user","content":[{"type":"input_text","text":"hi"}]}]}`, + } { + if w := bodyGateRequest(h, "/v1/agents/sessions", "application/json", []byte(body)); w.Code != http.StatusBadRequest { + t.Errorf("%s: %d %s", body, w.Code, w.Body) + } + } + events := "/v1/agents/sessions/" + uuid.NewString() + "/events" + unknownEvents := `{"error":{"message":"Invalid Session input event request.","type":"invalid_request_error","code":"invalid_request","param":null}}` + "\n" + if w := bodyGateRequest(h, events, "application/json", []byte(`{"Events":[]}`)); w.Code != http.StatusBadRequest || w.Body.String() != unknownEvents { + t.Errorf("Events: %d %s", w.Code, w.Body) + } + message := `{"events":[{"type":"agent.session.input.message","input":[{"role":"assistant","Role":"user","content":[{"type":"input_text","text":"hi"}]}]}]}` + if w := bodyGateRequest(h, events, "application/json", []byte(message)); w.Code != http.StatusBadRequest { + t.Errorf("message Role: %d %s", w.Code, w.Body) + } + if s.writes != 0 { + t.Fatalf("a case variant reached storage: %d writes", s.writes) + } + // Map keys are free: metadata keys differing in case are distinct. + w := bodyGateRequest(h, "/v1/vaults", "application/json", []byte(`{"metadata":{"K":"1","k":"2"}}`)) + if w.Code != http.StatusCreated || !strings.Contains(w.Body.String(), `"metadata":{"K":"1","k":"2"}`) { + t.Fatalf("metadata keys: %d %s", w.Code, w.Body) + } +} + +func TestCredentialCaseVariantsAreUnknown(t *testing.T) { + h, f, _ := credentialHandler(t) + path := "/v1/vaults/" + f.credential.VaultID + "/credentials" + refresh := `"refresh":{"client_id":"c","refresh_token":"r","token_endpoint":"https://issuer.example/token","token_endpoint_auth":{"type":"client_secret_post","client_secret":"s"}` + auth := `{"name":"n","auth":{"type":"mcp_oauth","mcp_server_url":"https://mcp.example/tools","access_token":"a",` + refresh + for _, body := range []string{ + auth + `,"Client_ID":"other"}}}`, + auth + `,"token_endpoint_auth":{"type":"client_secret_post","client_secret":"s","Client_Secret":"t"}}}}`, + `{"name":"n","auth":{"Type":"static_bearer","mcp_server_url":"https://mcp.example/tools","token":"t"}}`, + } { + if w := credentialRequest(h, http.MethodPost, path, body); w.Code != http.StatusBadRequest || f.calls != 0 { + t.Errorf("%s: %d %s", body, w.Code, w.Body) + } + } + if w := credentialRequest(h, http.MethodPost, path, auth+`}}}`); w.Code != http.StatusCreated || f.calls != 1 { + t.Fatalf("exact names: %d %s", w.Code, w.Body) + } +} diff --git a/services/agents-api/internal/store/request_body_public_test.go b/services/agents-api/internal/store/request_body_public_test.go index 172cc2c5c..a1f3ba23c 100644 --- a/services/agents-api/internal/store/request_body_public_test.go +++ b/services/agents-api/internal/store/request_body_public_test.go @@ -101,6 +101,23 @@ func TestRequestBodyGateRejectsWithoutWritesPostgres(t *testing.T) { } } } + // Member names match exactly: a case variant is an unknown member, never an + // alias whose value replaces the field (req_6ba2a50c71a4410f87a1baac855e82df). + message := `[{"role":"assistant","Role":"user","content":[{"type":"input_text","text":"case"}]}]` + for _, request := range []struct{ path, body string }{ + {"/v1/agents/sessions", `{"agent":{"model":"m"},"environment":{"type":"none"},"input":"case","Metadata":{"k":"v"}}`}, + {"/v1/agents/sessions", `{"agent":{"model":"m"},"environment":{"type":"none"},"Input":"case"}`}, + {"/v1/agents/sessions", `{"agent":{"model":"m"},"environment":{"type":"none"},"input":` + message + `}`}, + {"/v1/agents/sessions/" + session + "/events", `{"Events":[{"type":"agent.session.input.cancel"}]}`}, + {"/v1/agents/sessions/" + session + "/events", `{"events":[{"type":"agent.session.input.message","input":` + message + `}]}`}, + {"/v1/vaults/" + vault + "/credentials", `{"name":"case","auth":{"type":"mcp_oauth","mcp_server_url":"https://mcp.example/mcp","access_token":"a","refresh":{"client_id":"c","Client_ID":"d","refresh_token":"r","token_endpoint":"https://issuer.example/token","token_endpoint_auth":{"type":"none"}}}}`}, + } { + for _, token := range []string{owner, foreign} { + if status, body := client.do(token, http.MethodPost, request.path, "application/json", []byte(request.body)); status != http.StatusBadRequest && status != http.StatusNotFound { + t.Errorf("%s %s: %d %s", request.path, request.body, status, body) + } + } + } if after := databaseDigest(t, pool); !mapsEqual(before, after) { t.Fatal("a rejected body changed persisted state") } @@ -141,3 +158,4 @@ func TestRequestBodyGateRejectsWithoutWritesPostgres(t *testing.T) { t.Fatalf("foreign list: %d %s", status, body) } } + diff --git a/services/agents-api/tests/official_session_requests.py b/services/agents-api/tests/official_session_requests.py index e82d4c78d..f2fc66020 100644 --- a/services/agents-api/tests/official_session_requests.py +++ b/services/agents-api/tests/official_session_requests.py @@ -18,6 +18,10 @@ def verify_session_create_requests(client, spec): ({"metadata": {"label": None}}, "metadata.label"), ({"metadata": {"empty": "", "label": None}}, "metadata.label"), ({"metadata": {"label": 0}}, "metadata.label"), ({"metadata": []}, None), + # Member names match exactly: a case variant is an unknown member, not an + # alias that replaces the field (req_6ba2a50c71a4410f87a1baac855e82df). + ({"Metadata": {"label": "case"}}, None), ({"Input": "Case variant."}, None), ({"STREAM": False}, None), + ({"environment": {**spec["environment"], "Type": "self_hosted"}}, None), ] headers = {"Authorization": f"Bearer {client.api_key}", "OpenAI-Beta": "agents=v1"} with httpx2.Client(trust_env=False, timeout=10) as raw: From f364509aa262c1cc16381af0f32a71b22d6afdef Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 21:59:15 +0000 Subject: [PATCH 06/12] Prove the excluded routes and the no-write claims A PostgreSQL test sends DELETE, Files and Skills uploads, Skills update and executor credential requests with real storage and shows they keep their own body handling. The Vault, Credential and rotation scripts recount or reread their resources around the body gate checks. --- .../store/request_body_public_test.go | 84 +++++++++++++++++++ .../tests/official_credential_rotation.py | 1 + .../agents-api/tests/official_credentials.py | 2 + services/agents-api/tests/official_vaults.py | 2 + 4 files changed, 89 insertions(+) diff --git a/services/agents-api/internal/store/request_body_public_test.go b/services/agents-api/internal/store/request_body_public_test.go index a1f3ba23c..91c72c4a1 100644 --- a/services/agents-api/internal/store/request_body_public_test.go +++ b/services/agents-api/internal/store/request_body_public_test.go @@ -3,6 +3,7 @@ package store_test import ( "bytes" "encoding/json" + "mime/multipart" "net/http" "net/http/httptest" "strings" @@ -159,3 +160,86 @@ func TestRequestBodyGateRejectsWithoutWritesPostgres(t *testing.T) { } } +// DELETE routes, the multipart Files and Skills uploads, Skills update and the +// Core extension routes keep their own body handling, without the Content-Type +// rule or the official body messages. +func TestRequestBodyGateExcludedRoutesPostgres(t *testing.T) { + _, pool := store.NewTestStore(t) + cipher, err := credentialcrypto.New(bytes.Repeat([]byte{65}, 32)) + if err != nil { + t.Fatal(err) + } + s := store.NewWithCredentialCipher(pool, cipher) + token, tenant := uuid.NewString(), uuid.NewString() + auth, err := api.NewAuthenticator([]api.APIKey{{OrganizationID: "test-org", ProjectID: uuid.NewString(), SubjectKind: "service_account", SubjectID: "excluded-owner", TokenSHA256: device.HashCredential(token), TenantID: tenant}}) + if err != nil { + t.Fatal(err) + } + h, err := api.NewHandler(s, auth, "codex", api.WithExecution(s), api.WithSkills(s), api.WithSourceFiles(s)) + if err != nil { + t.Fatal(err) + } + server := httptest.NewServer(h) + defer server.Close() + client := pathIDClient{t: t, server: server} + gated := func(body string) bool { + return strings.Contains(body, "expected request with Content-Type") || strings.Contains(body, "Invalid body") || strings.Contains(body, "Invalid type") + } + upload := func(field, name string, content []byte, extra ...string) (string, []byte) { + var buffer bytes.Buffer + form := multipart.NewWriter(&buffer) + for i := 0; i+1 < len(extra); i += 2 { + if err := form.WriteField(extra[i], extra[i+1]); err != nil { + t.Fatal(err) + } + } + part, err := form.CreateFormFile(field, name) + if err != nil { + t.Fatal(err) + } + if _, err := part.Write(content); err != nil { + t.Fatal(err) + } + if err := form.Close(); err != nil { + t.Fatal(err) + } + return form.FormDataContentType(), buffer.Bytes() + } + contentType, body := upload("file", "excluded.txt", []byte("excluded"), "purpose", "user_data") + if status, response := client.do(token, http.MethodPost, "/v1/files", contentType, body); status != http.StatusOK || gated(response) { + t.Fatalf("Files upload: %d %s", status, response) + } + contentType, body = upload("files", "proof.zip", store.SkillArchive(t, "excluded-skill")) + status, response := client.do(token, http.MethodPost, "/v1/skills", contentType, body) + var skill struct{ ID string } + if status != http.StatusOK || json.Unmarshal([]byte(response), &skill) != nil || skill.ID == "" { + t.Fatalf("Skills upload: %d %s", status, response) + } + if status, response := client.do(token, http.MethodPost, "/v1/skills/"+skill.ID, "", []byte(`{"default_version":"1"}`)); status != http.StatusOK || gated(response) { + t.Fatalf("Skills update without Content-Type: %d %s", status, response) + } + if status, response := client.do(token, http.MethodPost, "/v1/skills/"+skill.ID, "text/plain", []byte(`{"default_version":`)); status != http.StatusBadRequest || gated(response) { + t.Fatalf("malformed Skills update: %d %s", status, response) + } + // Executor credential issuance keeps its own reader and errors. + environment := "/core/v1/environments/" + uuid.NewString() + "/executor-credentials" + for _, request := range []struct { + contentType, body string + status int + }{{"", `{"key_id":"` + uuid.NewString() + `"}`, http.StatusNotFound}, {"text/plain", `{"key_id":`, http.StatusBadRequest}, {"", "", http.StatusBadRequest}} { + if status, response := client.do(token, http.MethodPost, environment, request.contentType, []byte(request.body)); status != request.status || gated(response) || !strings.Contains(response, `"error"`) { + t.Errorf("executor credential %q: %d %s", request.body, status, response) + } + } + // DELETE keeps its empty-body rule and needs no Content-Type. + agent := client.created(token, "/v1/agents", `{"model":"excluded-model"}`) + vault := client.created(token, "/v1/vaults", `{"name":"excluded"}`) + if status, response := client.do(token, http.MethodDelete, "/v1/vaults/"+vault, "text/plain", []byte(`{"name":`)); status != http.StatusBadRequest || gated(response) { + t.Fatalf("Vault delete with a body: %d %s", status, response) + } + for _, path := range []string{"/v1/agents/" + agent, "/v1/vaults/" + vault} { + if status, response := client.do(token, http.MethodDelete, path, "", nil); status != http.StatusOK || gated(response) { + t.Fatalf("DELETE %s: %d %s", path, status, response) + } + } +} diff --git a/services/agents-api/tests/official_credential_rotation.py b/services/agents-api/tests/official_credential_rotation.py index ea943f66c..da61f3a2a 100644 --- a/services/agents-api/tests/official_credential_rotation.py +++ b/services/agents-api/tests/official_credential_rotation.py @@ -80,6 +80,7 @@ def metadata(response, previous): official_body.check(raw, endpoint, headers, official_body.rejected( json.dumps({"auth": {"type": "static_bearer", "token": "gate"}}, separators=(",", ":")), "token", "auth.token")) safe(raw.post(endpoint, headers={**headers, **official_body.JSON}, content="null"), 400) + assert credentials.retrieve(original.id, vault_id=vault.id) == current for override in ({"auth": None}, {"auth": {"type": "static_bearer", "token": None}}): error = expect_error(BadRequestError, lambda: credentials.update( original.id, vault_id=vault.id, **replacement, extra_body=override)) diff --git a/services/agents-api/tests/official_credentials.py b/services/agents-api/tests/official_credentials.py index 0980fc007..ae5c05871 100644 --- a/services/agents-api/tests/official_credentials.py +++ b/services/agents-api/tests/official_credentials.py @@ -87,9 +87,11 @@ def safe_body(response, status): response = raw.post(endpoint, headers=headers, json=body) assert safe_body(response, 400)["error"]["type"] == "invalid_request_error" # The shared body gate rejects before any write (HP-09..HP-15); null is {}. + count = len(list(credentials.list(vault_id=vault.id))) official_body.check(raw, endpoint, headers, official_body.rejected( json.dumps({**request, "name": "gate"}, separators=(",", ":")), "name", "name")) safe_body(raw.post(endpoint, headers={**headers, **official_body.JSON}, content="null"), 400) + assert len(list(credentials.list(vault_id=vault.id))) == count for override in ({"name": None}, {"auth": None}, {"auth": {**auth, "token": None}}): error = expect_error(BadRequestError, lambda: credentials.create(vault.id, **request, extra_body=override)) safe_body(error.response, 400) diff --git a/services/agents-api/tests/official_vaults.py b/services/agents-api/tests/official_vaults.py index 6528a4193..a8ff4f47b 100644 --- a/services/agents-api/tests/official_vaults.py +++ b/services/agents-api/tests/official_vaults.py @@ -82,8 +82,10 @@ def verify_vaults(client, other, invalid, peer, binding, expect_error): assert response.status_code == 400 and response.json()["error"]["code"] == "invalid_request_error" assert response.json()["error"]["param"] == param # The shared body gate rejects before any write (HP-09..HP-15). + count = len(list(vaults.list())) official_body.check(raw, base, headers, official_body.rejected('{"name":"gate","metadata":{"k":"v"}}', "name", "name")) official_body.check(raw, base, headers, [(official_body.JSON, b'{"metadata":{"k":"1","k":"2"}}', official_body.duplicate("k", "metadata.k"))]) + assert len(list(vaults.list())) == count for resource_id in (str(uuid.uuid4()), "invalid-vault", str(uuid.UUID(int=0)), foreign.id): response = raw.get(base + "/" + resource_id, headers=headers) From 1b04de0a766f414f8757dd421be1d6a4fe7dbec2 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 21:59:15 +0000 Subject: [PATCH 07/12] Document surrogates, exact member names and the linear scan --- CONTRIBUTING.md | 9 ++-- .../official-semantics-alignment.md | 49 +++++++++++++------ contracts/agents-api/operation-evidence.md | 2 +- 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4cb791b45..803eff622 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -89,9 +89,12 @@ observed upstream server failures as compatibility behavior. See Every Agents API JSON route reads its body through the shared gate (`readJSONObject`) before route decoding, validation or lookup. It requires a JSON Content-Type, applies the route's body limit and rejects invalid UTF-8, malformed -JSON, repeated keys and non-object roots with the official messages; an empty -body or null becomes `{}`. DELETE, multipart, Core extension and internal routes -keep their own readers. See +JSON (including unpaired surrogate escapes), repeated keys and non-object roots +with the official messages; an empty body or null becomes `{}`. DELETE, multipart, +Core extension and internal routes keep their own readers. Member names match +exactly: decode request objects with `decodeInputObject`, or check +`caseVariantMember` before another decoder, so that encoding/json never matches +a case variant to a field. See `contracts/agents-api/official-semantics-alignment.md#request-body-parsing--september-23`. Report validation failures with official evidence through the typed field error, which emits `invalid_request_error` with the observed param and message; keep diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index 927713054..b8461b47b 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -881,16 +881,20 @@ Evidence is HP-09..HP-15 of the HTTP protocol campaign scan, recorded privately `REPORT.txt`, raw requests in `official-ledger.jsonl`, labels `C01`–`C20`, `U01`–`U11`, `S1`): one owned Agent, created, updated and deleted (404 confirmed), without a Session or model. The official records cover Agent create and update; -the other routes are assumed to share the official parser. +the other routes are assumed to share the official parser. Two later owned probes +in `~/.parsar/remediation/20260924/http-json-body/official/results.json` created +nothing: a lone high surrogate escape (`req_1a9b7680d615454ca97c816b25e2f401`) and +Agent create with `metadata` and `Metadata` but no `model` +(`req_6ba2a50c71a4410f87a1baac855e82df`). | Row | Case | Core behavior | | --- | --- | --- | -| B1 | Malformed JSON, trailing data, two concatenated values, a UTF-8 byte order mark or a whitespace-only body (`C01`, `C06`, `C07`, `C12`, `C20`, `U01`, `U05`, `U06`) | 400, type and code `invalid_request_error`, param null: "Invalid body: failed to parse JSON value. Please check the value to ensure it is valid JSON. (Common errors include trailing commas, missing closing brackets, missing quotation marks, etc.)". Agent update no longer returns `unsupported_or_invalid_configuration`. | +| B1 | Malformed JSON, trailing data, two concatenated values, a UTF-8 byte order mark, a whitespace-only body (`C01`, `C06`, `C07`, `C12`, `C20`, `U01`, `U05`, `U06`), or a string escape that forms a lone or mis-paired UTF-16 surrogate, such as `"\ud800"`, in a key or value (`req_1a9b7680d615454ca97c816b25e2f401`) | 400, type and code `invalid_request_error`, param null: "Invalid body: failed to parse JSON value. Please check the value to ensure it is valid JSON. (Common errors include trailing commas, missing closing brackets, missing quotation marks, etc.)". Agent update no longer returns `unsupported_or_invalid_configuration`. Valid surrogate pairs are accepted; lone surrogates were previously stored as U+FFFD. | | B2 | Invalid UTF-8 anywhere in the body (`C13`) | 400 with the same fields: "Invalid body: encountered a unicode decode error when parsing this JSON value. Please check the value to ensure it is valid unicode." Previously the bytes were stored as U+FFFD. | -| B3 | A repeated object key at any depth (`C08`, `C15`, `C16`, `U07`) | 400 with the same fields: "Invalid body: duplicate JSON key '' at ''. Duplicate JSON keys are not supported." The path joins object keys with `.` and omits array indices: `name`, `metadata.k`, `tools.type`. The first repeat in document order is reported. Previously metadata, Vaults and Templates kept the last value, and Agent configuration returned the local "Duplicate parameter". | +| B3 | A repeated object key at any depth (`C08`, `C15`, `C16`, `U07`) | 400 with the same fields: "Invalid body: duplicate JSON key '' at ''. Duplicate JSON keys are not supported." The path joins object keys with `.` and omits array indices: `name`, `metadata.k`, `tools.type`. Keys compare after unescaping and case-sensitively: `metadata` and `Metadata` are distinct keys (`req_6ba2a50c71a4410f87a1baac855e82df`). The first repeat in document order is reported. Previously metadata, Vaults and Templates kept the last value, and Agent configuration returned the local "Duplicate parameter". | | B4 | A valid root that is not an object (`C05`) | 400 with the same fields: "Invalid type: expected an object, but got instead." with the existing kind phrases (`a string`, `an integer`, `a number`, `a boolean`). | | B5 | A zero-length body or `null` (`C02`, `C03`, `U02`, `U03`) | Treated as `{}`: Agent create reports the missing `model`, Agent update is the documented empty update, Session update keeps its "At least one update field is required" rejection, and Vault create creates an unnamed Vault. A whitespace-only body stays B1. | -| B6 | Content-Type missing, `text/plain` or form-encoded, including a bodyless POST without Content-Type (`C09`–`C11`, `C19`, `U08`–`U10`) | 400 with the same fields, "expected request with Content-Type: application/json", checked before the body is read. `application/json` and `application/*+json` are accepted case-insensitively, with parameters (`S1`, `U11`, `C17`, `C18`). Previously Core ignored the header and applied the update. | +| B6 | Content-Type missing, `text/plain` or form-encoded, including a bodyless POST without Content-Type (`C09`–`C11`, `C19`, `U08`–`U10`) | 400 with the same fields, "expected request with Content-Type: application/json", checked before the body is read. `application/json` and `application/*+json` are accepted case-insensitively, with parameters (`S1`, `U11`, `C17`, `C18`); a malformed media type, such as `application/foo bar+json` or a conflicting repeated parameter, is rejected the same way. Previously Core ignored the header and applied the update. | | B7 | Valid bodies | Unchanged, including unknown-member errors, configuration validation, route body limits (413) and Core extensions such as `x_agents_core`. | Order: authentication and Beta handling as before, then B6, then the route's body @@ -906,8 +910,18 @@ Decisions: as `{}` (HP-14); that upstream anomaly is not copied. - The duplicate key and path are repeated only when each is at most 256 bytes of printable UTF-8, the shared `echotext` rule; otherwise the message is "Invalid - body: duplicate JSON key. Duplicate JSON keys are not supported." A lone - surrogate escape decodes to U+FFFD and is therefore not repeated either. + body: duplicate JSON key. Duplicate JSON keys are not supported." +- The gate scans each body once in linear time. It keeps key positions in the + body, not copies: objects with more than 16 keys use an open-addressing set of + 4-byte positions, so a body of many short keys allocates about its own size. +- Member names match exactly on every gated route. encoding/json would match a + case variant such as `Metadata`, `Input` or a nested `Role` to the field and + let the last copy win; such a key is now an unknown member at any depth, + rejected with the route's existing unknown-member error before any write. + Agent configuration already did this. On Agent create, a body with `Metadata` + and no `model` still reports the unknown member first, while the official + service reported the missing `model`; the official order between several + errors in one body remains unaligned. - Invalid UTF-8 is checked before JSON syntax; the official order for a body with both faults was not observed. - Not gated: DELETE routes, which keep their empty-body rule, the multipart Files @@ -915,9 +929,7 @@ Decisions: fields), the Core extension `/core/v1/*` routes, including executor credential issuance, and the internal daemon, sandbox and node routes. -Unchanged: the schema, route validation and error codes of valid bodies, and -lone surrogate escapes such as `"\ud800"`, which JSON permits and encoding/json -still decodes to U+FFFD; the official treatment of such escapes was not observed. +Unchanged: the schema and the route validation and error codes of valid bodies. Caller check: the TypeScript client sends `application/json` with every JSON body (an empty Agent update sends `{}`), Core Web uses that client, the Go client uses @@ -925,13 +937,18 @@ the pinned openai-go SDK, the Python acceptance tools send JSON through the pinn SDK or `json=`, and the documentation has no JSON POST examples. The Parsar product repository does not call these routes. -Go tests cover the gate on its own (B1–B5, the echo bound, accepted media types, -deep bodies and a differential check of the linear duplicate-key scan against an -encoding/json token walk) and on all eleven route families (B1–B4, B6, the order against -authentication, Beta and the body limit, B5/B7 and the excluded routes). A -real-PostgreSQL test sends B1–B4 and B6 to every route family as the owner and -as tenant B under a whole-database digest, then checks B5/B7 writes. The pinned-SDK +Go tests cover the gate on its own (B1–B5, surrogate escapes, the echo bound, +media type parsing, deep bodies, a differential check of the duplicate-key scan +against an encoding/json token walk on small and large objects, and an allocation +bound on many short keys), case-variant members, and all eleven route families +(B1–B4, B6, the order against authentication, Beta and the body limit, B5/B7). A +real-PostgreSQL test sends B1–B4, B6 and case-variant members to every route +family as the owner and as tenant B under a whole-database digest, then checks +B5/B7 writes; another exercises the excluded DELETE, Files and Skills upload, +Skills update and executor credential routes with real storage. The pinned-SDK scripts `official_agents.py`, `official_agent_update.py`, `official_vaults.py`, `official_credentials.py`, `official_credential_rotation.py` and -`official_session_metadata.py` assert the official messages and no writes. +`official_session_metadata.py` assert the official messages and recount or reread +the resources to show no writes; `official_session_requests.py` rejects +case-variant members without creating a Session. Independent acceptance is recorded separately by the coordinator. diff --git a/contracts/agents-api/operation-evidence.md b/contracts/agents-api/operation-evidence.md index 61b9b8a91..a75521d56 100644 --- a/contracts/agents-api/operation-evidence.md +++ b/contracts/agents-api/operation-evidence.md @@ -53,7 +53,7 @@ Repository paths below are relative to the inspected worktree; private evidence | MV | [MCP origin and credential selection](official-semantics-alignment.md#mcp-origin-and-credential-selection--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/mcp-vaults/findings.json` MV-01..03 with raw records in `official-ledger.jsonl` (`AG1-minimal-and-nulls`, `S1-T1-create-stream`, `S1-after-c1-delete-session-get`, `S1-final-session-get`, `ERR-UNATTACHED`, `ERR-URL-MISMATCH`, `ERR-AMBIGUOUS`, `ERR-CREDENTIAL-BOGUS`, `ERR-VAULT-BOGUS`): owned Agents, three owned Sessions, two Vaults and four Credentials, all deleted. Rows M1–M9 of that section. Go API/store and real-PostgreSQL tenant A/B HTTP tests with a no-write digest, pinned-SDK official client scripts; live acceptance is recorded with the batch. | | HF | [Hosted initialization failure](official-semantics-alignment.md#hosted-initialization-failure--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/hosted-init/findings.json` HI-01..04 with raw records under `official/` (`006-S2-create-setup-exit3`, `007-S2-events`, `009-S3-events`, `021-S2-env-after-failed`, `024-S2-session-after-failed`, `025-S3-session-after-failed`, `026-S2-input-after-failure`, `027-S3-input-after-failure`, `038-S2-delete`, `041-S3-delete`): two owned `openai_hosted` Sessions without a Turn (setup exit 3, nonexistent Python package), both deleted. Rows H1–H8 of that section; HI-05/06 stay deferred. Python initializer receipt tests, Go contract/API tests, real-PostgreSQL managed-Worker store tests with a controlled Provider and an HTTP test with tenant B and a canary, TypeScript client and Web tests; live Docker acceptance is recorded with the batch. | | RH | [HTTP routing and response headers](official-semantics-alignment.md#http-routing-and-response-headers--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/http-protocol/findings.json` HP-02, 03, 05, 07, 17–20, 23, 24 (HP-04, 21, 22, 26 kept) with raw records in `official-ledger.jsonl` (labels `A01`–`A11`, `B01`, `B08`, `R04`–`R18`, `S2-retrieve-baseline`, `C01-malformed`, `X1-delete-owned`) and `REPORT.txt`: one owned Agent (deleted), 90 requests without a Session or model. Rows RH1–RH11 apply to every operation's HTTP layer; HEAD on the events stream and content downloads is a documented local 405. | -| JB | [Request body parsing](official-semantics-alignment.md#request-body-parsing--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/http-protocol/findings.json` HP-09..15 with raw records in `official-ledger.jsonl` (labels `C01`–`C20`, `U01`–`U11`, `S1`) and `REPORT.txt`: one owned Agent (deleted, 404 confirmed), no Session or model. Rows B1–B7 of that section; HP-14 (`[]` as `{}`) is a recorded upstream anomaly, not copied. Official records cover Agent create/update; rows 1, 3, 6, 8, 11, 27, 29, 31, 34, 38 and 40 share the gate. Go gate and all-route handler tests, a real-PostgreSQL owner/tenant B no-write digest across every route family and pinned-SDK scripts; independent acceptance is recorded with the batch. | +| JB | [Request body parsing](official-semantics-alignment.md#request-body-parsing--september-23); private `~/.parsar/remediation/20260923/campaign-scan-6/http-protocol/findings.json` HP-09..15 with raw records in `official-ledger.jsonl` (labels `C01`–`C20`, `U01`–`U11`, `S1`) and `REPORT.txt`: one owned Agent (deleted, 404 confirmed), no Session or model; plus `~/.parsar/remediation/20260924/http-json-body/official/results.json` (`lone-high-surrogate`, `case-variant-metadata`), two requests that created nothing. Rows B1–B7 of that section; HP-14 (`[]` as `{}`) is a recorded upstream anomaly, not copied. Official records cover Agent create/update; rows 1, 3, 6, 8, 11, 27, 29, 31, 34, 38 and 40 share the gate. Go gate and all-route handler tests, a real-PostgreSQL owner/tenant B no-write digest across every route family and pinned-SDK scripts; independent acceptance is recorded with the batch. | ## Per-operation evidence matrix From 7a3c74cf2ee4080af3368ae19f72a384bb71861f Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 23:04:57 +0000 Subject: [PATCH 08/12] Keep hash bits with body key positions and read bodies in a doubling buffer The duplicate-key set stores 32 hash bits beside each key position, so probes compare keys only when the bits match. Duplicate paths are built with exact-size strings, and the deep-body test now bounds its allocation. The shared reader grows its buffer by doubling: io.ReadAll allocated about five times a large body. --- .../agents-api/internal/api/json_request.go | 52 ++++++++++++------- .../internal/api/json_request_test.go | 18 +++++-- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/services/agents-api/internal/api/json_request.go b/services/agents-api/internal/api/json_request.go index 88a87dd52..5223a1aee 100644 --- a/services/agents-api/internal/api/json_request.go +++ b/services/agents-api/internal/api/json_request.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "hash/maphash" - "io" "mime" "net/http" "strings" @@ -23,9 +22,12 @@ func readJSONBody(w http.ResponseWriter, r *http.Request) ([]byte, bool) { } func readJSONBodyLimit(w http.ResponseWriter, r *http.Request, limit int64, message string) ([]byte, bool) { - raw, err := io.ReadAll(http.MaxBytesReader(w, r.Body, limit)) + // A doubling buffer allocates about twice the body in total; io.ReadAll's + // smaller growth steps allocate about five times a large body. + var body bytes.Buffer + _, err := body.ReadFrom(http.MaxBytesReader(w, r.Body, limit)) if err == nil { - return raw, true + return body.Bytes(), true } var tooLarge *http.MaxBytesError if errors.As(err, &tooLarge) { @@ -138,7 +140,7 @@ type scanFrame struct { object, expectKey bool member uint32 // the position of the key whose value is being read base, count int // this object's keys in keyScanner.small while it is small - table []uint32 // key positions once the object is large + table []uint64 // hash tag << 32 | key position, once the object is large } // keyScanner finds repeated keys without copying them: keys are positions in @@ -292,44 +294,45 @@ func (s *keyScanner) insert(f *scanFrame, position uint32) bool { } s.small = append(s.small, scannedKey{position, hash}) if f.count++; f.count > smallObjectKeys { - f.table = make([]uint32, 4*smallObjectKeys) + f.table = make([]uint64, 4*smallObjectKeys) for _, key := range s.small[f.base:] { - s.place(f.table, key.position, key.hash) + place(f.table, key.position, key.hash) } s.small = s.small[:f.base] } return false } - // Linear probing at a load factor of at most 3/4. + // Linear probing at a load factor of at most 3/4. The high 32 hash bits + // stored with each position skip comparing keys that cannot be equal. if (f.count+1)*4 > len(f.table)*3 { - grown := make([]uint32, 2*len(f.table)) - for _, key := range f.table { - if key != 0 { - s.place(grown, key, s.hash(key)) + grown := make([]uint64, 2*len(f.table)) + for _, slot := range f.table { + if key := uint32(slot); key != 0 { + place(grown, key, s.hash(key)) } } f.table = grown } - mask := uint64(len(f.table) - 1) + mask, tag := uint64(len(f.table)-1), hash>>32 for i := hash & mask; ; i = (i + 1) & mask { - switch key := f.table[i]; { - case key == 0: - f.table[i] = position + switch slot := f.table[i]; { + case slot == 0: + f.table[i] = tag<<32 | uint64(position) f.count++ return false - case s.equal(key, position): + case slot>>32 == tag && s.equal(uint32(slot), position): return true } } } -func (s *keyScanner) place(table []uint32, position uint32, hash uint64) { +func place(table []uint64, position uint32, hash uint64) { mask := uint64(len(table) - 1) i := hash & mask for table[i] != 0 { i = (i + 1) & mask } - table[i] = position + table[i] = hash>>32<<32 | uint64(position) } // duplicate returns the repeated key and its path through the enclosing objects. @@ -337,13 +340,22 @@ func (s *keyScanner) duplicate(position uint32) (string, string) { var segments []string for _, f := range s.frames[:len(s.frames)-1] { if f.object { - segments = append(segments, string(appendUnescaped(nil, s.content(f.member)))) + segments = append(segments, s.text(f.member)) } } - key := string(appendUnescaped(nil, s.content(position))) + key := s.text(position) return key, strings.Join(append(segments, key), ".") } +// text returns the unescaped key at position. +func (s *keyScanner) text(position uint32) string { + if position&keyEscaped == 0 { + return string(s.content(position)) + } + s.left = appendUnescaped(s.left[:0], s.content(position)) + return string(s.left) +} + // appendUnescaped decodes the contents of a valid JSON string whose surrogate // escapes are paired. func appendUnescaped(dst, s []byte) []byte { diff --git a/services/agents-api/internal/api/json_request_test.go b/services/agents-api/internal/api/json_request_test.go index c223adee0..9dc3a041d 100644 --- a/services/agents-api/internal/api/json_request_test.go +++ b/services/agents-api/internal/api/json_request_test.go @@ -97,13 +97,21 @@ func TestJSONObjectBodyChecks(t *testing.T) { t.Errorf("%.80q: got %v, want %q", body, err, message) } } - // Paths are built only for the reported key, so deep bodies with long keys - // stay linear in time and memory. + // Paths are built only for the reported key, so a deep body with long keys + // allocates linearly: building the path at every level would need gigabytes. depth, key := 4000, strings.Repeat("k", 1000) deep := strings.Repeat(`{"`+key+`":[`, depth) + `{"a":1,"a":2}` + strings.Repeat("]}", depth) - if _, err := jsonObjectBody([]byte(deep)); err != errBodyDuplicateKey { + runtime.GC() + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + _, err := jsonObjectBody([]byte(deep)) + runtime.ReadMemStats(&after) + if err != errBodyDuplicateKey { t.Errorf("deep duplicate: %v", err) } + if allocated := after.TotalAlloc - before.TotalAlloc; allocated > 4*uint64(len(deep)) { + t.Errorf("deep duplicate allocated %d bytes for %d", allocated, len(deep)) + } deep = strings.Repeat(`{"b":[`, 100) + `{"a":1,"a":2}` + strings.Repeat("]}", 100) if _, err := jsonObjectBody([]byte(deep)); err == nil || err.Error() != duplicateKeyMessage("a", strings.Repeat("b.", 100)+"a") { t.Errorf("nested duplicate: %v", err) @@ -450,7 +458,7 @@ func TestDuplicateJSONKeyMatchesReference(t *testing.T) { // A body of many short keys needs memory proportional to its key count, not // a copy of every key: 116 MiB was allocated for this 16 MiB body before, and -// about 16 MiB, mostly the growing key-position set, is allocated now. +// about 32 MiB in total, the growing set of 8-byte slots, is allocated now. func TestDuplicateJSONKeyMemory(t *testing.T) { body := manyShortKeys(16 << 20) runtime.GC() @@ -461,7 +469,7 @@ func TestDuplicateJSONKeyMemory(t *testing.T) { if found || err != nil { t.Fatal(found, err) } - if allocated := after.TotalAlloc - before.TotalAlloc; allocated > 2*uint64(len(body)) { + if allocated := after.TotalAlloc - before.TotalAlloc; allocated > 5*uint64(len(body))/2 { t.Fatalf("allocated %d MiB for a %d MiB body", allocated>>20, len(body)>>20) } } From 77c93e8a8ee6fc77e1e3d3ce2b1c8f68002fe46c Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 23:04:57 +0000 Subject: [PATCH 09/12] Check member names by walking the body before decoding caseVariantMember loaded every object level into a map and added about 231 MiB to a 16 MiB Session create. inexactMember walks the bytes once without copying them and reports the first key that is not exactly a member name, unknown or a case variant, before the decoder, which with DisallowUnknownFields formatted an error for every unknown key. The Session metadata check reads only the metadata member. A 16 MiB Session create of unknown keys now allocates about 96 MiB instead of 482 MiB, and a route-level test bounds Session create and events. The nested cases now fail only without the check. --- .../internal/api/function_inputs.go | 5 +- services/agents-api/internal/api/handler.go | 5 +- services/agents-api/internal/api/inputs.go | 5 +- .../agents-api/internal/api/json_members.go | 180 ++++++++++++++---- .../internal/api/json_members_test.go | 60 +++++- .../internal/api/session_metadata.go | 6 +- 6 files changed, 203 insertions(+), 58 deletions(-) diff --git a/services/agents-api/internal/api/function_inputs.go b/services/agents-api/internal/api/function_inputs.go index 40f2f9991..6c877ada8 100644 --- a/services/agents-api/internal/api/function_inputs.go +++ b/services/agents-api/internal/api/function_inputs.go @@ -63,8 +63,9 @@ func decodeInputObject(raw json.RawMessage, value any, allowed ...string) error return store.ErrInvalidInput } } - // Nested members match exactly too; see caseVariantMember. - if caseVariantMember(raw, reflect.TypeOf(value)) { + // Nested members match exactly too; see inexactMember. The raw value is + // valid JSON here, as Unmarshal accepted it. + if inexactMember(raw, reflect.TypeOf(value)) { return store.ErrInvalidInput } decoder := json.NewDecoder(bytes.NewReader(raw)) diff --git a/services/agents-api/internal/api/handler.go b/services/agents-api/internal/api/handler.go index 9f41c033b..0eec95774 100644 --- a/services/agents-api/internal/api/handler.go +++ b/services/agents-api/internal/api/handler.go @@ -190,8 +190,9 @@ func (h *Handler) createSession(w http.ResponseWriter, r *http.Request) { var request decodedSessionRequest decoder := json.NewDecoder(bytes.NewReader(raw)) decoder.DisallowUnknownFields() - // A case variant of a member, such as Metadata, is an unknown member. - if caseVariantMember(raw, reflect.TypeOf(request)) || decoder.Decode(&request) != nil { + // An unknown member, including a case variant such as Metadata, is rejected + // before decoding; see inexactMember. + if inexactMember(raw, reflect.TypeOf(request)) || decoder.Decode(&request) != nil { writeError(w, http.StatusBadRequest, "invalid_request", "Request must be a JSON object containing supported fields.") return } diff --git a/services/agents-api/internal/api/inputs.go b/services/agents-api/internal/api/inputs.go index 195d6b11e..c8d441bd6 100644 --- a/services/agents-api/internal/api/inputs.go +++ b/services/agents-api/internal/api/inputs.go @@ -45,8 +45,9 @@ func (h *Handler) createEvents(w http.ResponseWriter, r *http.Request) { } decoder := json.NewDecoder(bytes.NewReader(raw)) decoder.DisallowUnknownFields() - // A case variant of events is an unknown member; events decode exactly too. - if caseVariantMember(raw, reflect.TypeOf(request)) || decoder.Decode(&request) != nil { + // An unknown member, including a case variant of events, is rejected before + // decoding; see inexactMember. + if inexactMember(raw, reflect.TypeOf(request)) || decoder.Decode(&request) != nil { writeError(w, http.StatusBadRequest, "invalid_request", "Invalid Session input event request.") return } diff --git a/services/agents-api/internal/api/json_members.go b/services/agents-api/internal/api/json_members.go index 87209cb4a..bac5922cf 100644 --- a/services/agents-api/internal/api/json_members.go +++ b/services/agents-api/internal/api/json_members.go @@ -1,68 +1,143 @@ package api import ( + "bytes" "encoding/json" "reflect" "strings" "sync" ) -// caseVariantMember reports whether raw has an object key, at any depth that -// decodes into a struct of type t, that encoding/json would match to a field -// only case-insensitively. The official service matches member names exactly, -// so such a key is an unknown member, never an alias of the field -// (req_6ba2a50c71a4410f87a1baac855e82df). Unknown keys and values of other types -// are left to the decoder. raw has passed the shared body gate. -func caseVariantMember(raw []byte, t reflect.Type) bool { +// inexactMember reports whether raw, a valid JSON value, has an object key that +// is not exactly a member name of the struct it decodes into, at any depth. It +// runs before a decoder with DisallowUnknownFields, which rejects unknown keys +// too but matches names case-insensitively and lets the last copy win: the +// official service treats a case variant such as Metadata as an unknown member +// (req_6ba2a50c71a4410f87a1baac855e82df). It walks the bytes once without +// copying them and stops at the first such key, so a body of unknown keys costs +// no allocation before the route's error. +func inexactMember(raw []byte, t reflect.Type) bool { + w := memberWalker{raw: raw} + _, found := w.value(skipSpace(raw, 0), t) + return found +} + +type memberWalker struct { + raw []byte + scratch []byte // an unescaped member name +} + +// value walks the value at raw[i] as type t and returns the offset after it. +func (w *memberWalker) value(i int, t reflect.Type) (int, bool) { for t.Kind() == reflect.Pointer { t = t.Elem() } if t == rawMessageType || reflect.PointerTo(t).Implements(unmarshalerType) { - return false + return skipValue(w.raw, i), false } - switch t.Kind() { - case reflect.Struct: - var members map[string]json.RawMessage - if json.Unmarshal(raw, &members) != nil { - return false + switch { + case w.raw[i] == '{' && (t.Kind() == reflect.Struct || t.Kind() == reflect.Map): + var fields map[string]reflect.Type + if t.Kind() == reflect.Struct { + fields = jsonFields(t) } - fields := jsonFields(t) - for key, value := range members { - field, exact := fields[key] - if exact { - if caseVariantMember(value, field) { - return true + i = skipSpace(w.raw, i+1) + for w.raw[i] != '}' { + end := stringEnd(w.raw, i) + name := w.raw[i+1 : end-1] + i = skipSpace(w.raw, skipSpace(w.raw, end)+1) + child := t + if fields == nil { + child = t.Elem() + } else { + if bytes.IndexByte(name, '\\') >= 0 { + w.scratch = appendUnescaped(w.scratch[:0], name) + name = w.scratch } - continue - } - for name := range fields { - if strings.EqualFold(name, key) { - return true + var exact bool + if child, exact = fields[string(name)]; !exact { + return 0, true } } + var found bool + if i, found = w.value(i, child); found { + return 0, true + } + i = skipComma(w.raw, skipSpace(w.raw, i)) } - case reflect.Map: - var members map[string]json.RawMessage - if json.Unmarshal(raw, &members) != nil { - return false - } - for _, value := range members { - if caseVariantMember(value, t.Elem()) { - return true + return i + 1, false + case w.raw[i] == '[' && (t.Kind() == reflect.Array || t.Kind() == reflect.Slice && t.Elem().Kind() != reflect.Uint8): + i = skipSpace(w.raw, i+1) + for w.raw[i] != ']' { + var found bool + if i, found = w.value(i, t.Elem()); found { + return 0, true } + i = skipComma(w.raw, skipSpace(w.raw, i)) + } + return i + 1, false + } + return skipValue(w.raw, i), false +} + +func skipSpace(raw []byte, i int) int { + for i < len(raw) && (raw[i] == ' ' || raw[i] == '\t' || raw[i] == '\n' || raw[i] == '\r') { + i++ + } + return i +} + +// skipComma steps over a separating comma and the space after it. +func skipComma(raw []byte, i int) int { + if raw[i] == ',' { + return skipSpace(raw, i+1) + } + return i +} + +// stringEnd returns the offset after the string starting at raw[quote]. +func stringEnd(raw []byte, quote int) int { + for i := quote + 1; ; { + q := i + bytes.IndexByte(raw[i:], '"') + backslashes := 0 + for q-backslashes > i && raw[q-backslashes-1] == '\\' { + backslashes++ } - case reflect.Slice, reflect.Array: - var items []json.RawMessage - if json.Unmarshal(raw, &items) != nil { - return false + if backslashes%2 == 0 { + return q + 1 } - for _, item := range items { - if caseVariantMember(item, t.Elem()) { - return true + i = q + 1 + } +} + +// skipValue returns the offset after the value starting at raw[i]. +func skipValue(raw []byte, i int) int { + depth := 0 + for { + switch raw[i] { + case '"': + i = stringEnd(raw, i) + if depth == 0 { + return i + } + continue + case '{', '[': + depth++ + case '}', ']': + if depth--; depth == 0 { + return i + 1 + } + default: + if depth == 0 { + // A number or literal ends at a delimiter or the end of the body. + for i < len(raw) && !strings.ContainsRune(",]} \t\r\n", rune(raw[i])) { + i++ + } + return i } } + i++ } - return false } var ( @@ -112,3 +187,28 @@ func jsonFields(t reflect.Type) map[string]reflect.Type { jsonFieldCache.Store(t, fields) return fields } + +// objectMember returns the value of the exact member name of a valid JSON +// object without decoding the other members, or nil. +func objectMember(raw []byte, name string) []byte { + i := skipSpace(raw, 0) + if i == len(raw) || raw[i] != '{' { + return nil + } + var scratch []byte + for i = skipSpace(raw, i+1); raw[i] != '}'; { + end := stringEnd(raw, i) + key := raw[i+1 : end-1] + if bytes.IndexByte(key, '\\') >= 0 { + scratch = appendUnescaped(scratch[:0], key) + key = scratch + } + i = skipSpace(raw, skipSpace(raw, end)+1) + valueEnd := skipValue(raw, i) + if string(key) == name { + return raw[i:valueEnd] + } + i = skipComma(raw, skipSpace(raw, valueEnd)) + } + return nil +} diff --git a/services/agents-api/internal/api/json_members_test.go b/services/agents-api/internal/api/json_members_test.go index 01af0cbe1..00c853335 100644 --- a/services/agents-api/internal/api/json_members_test.go +++ b/services/agents-api/internal/api/json_members_test.go @@ -1,16 +1,19 @@ package api import ( + "bytes" "encoding/json" + "fmt" "net/http" "reflect" + "runtime" "strings" "testing" "github.com/google/uuid" ) -func TestCaseVariantMember(t *testing.T) { +func TestInexactMember(t *testing.T) { type inner struct { Name string `json:"name"` } @@ -29,19 +32,23 @@ func TestCaseVariantMember(t *testing.T) { } typ := reflect.TypeFor[outer]() for body, want := range map[string]bool{ - `{"shared":"a","hidden":{"Name":1},"items":[{"name":"a"}],"named":{"K":{"name":"b"}},"Plain":"c","nested":{"name":"d"},"unknown":1}`: false, + `{"shared":"a","hidden":{"Name":1},"items":[{"name":"a"}],"named":{"K":{"name":"b"}},"Plain":"c","nested":{"name":"d"}}`: false, + ` { "shared" : "a" , "items" : [ { "name" : "a\"}" } , null ] , "named" : { } } `: false, + `{"sh\u0061red":"escaped exact name"}`: false, + `{"unknown":1}`: true, + `{"items":[{"name":"a","other":1}]}`: true, `{"Shared":"a"}`: true, `{"HIDDEN":{}}`: true, `{"items":[{"name":"a"},{"NAME":"b"}]}`: true, `{"named":{"k":{"Name":"b"}}}`: true, `{"nested":{"nAme":"d"}}`: true, `{"plain":"c"}`: true, - `{"Skipped":"x"}`: false, + `{"Skipped":"x"}`: true, `{"sHaReD":1,"shared":2}`: true, `{"items":"not an array","named":null}`: false, `{"ſhared":"long s folds to s"}`: true, } { - if got := caseVariantMember([]byte(body), typ); got != want { + if got := inexactMember([]byte(body), typ); got != want { t.Errorf("%s: got %t, want %t", body, got, want) } } @@ -64,11 +71,11 @@ func TestCaseVariantMembersAreUnknown(t *testing.T) { t.Errorf("%s: %d %s", body, w.Code, w.Body) } } - // Nested members keep their existing unknown-member errors. + // A nested case variant that encoding/json alone would accept as the field; + // without the check this creates a Session. for _, body := range []string{ - session + `,"x_agents_core":{"Model_Provider":null}}`, - `{"agent":{"model":"m"},"environment":{"type":"none","Type":"self_hosted"},"input":"hi"}`, `{"agent":{"model":"m"},"environment":{"type":"none"},"input":[{"role":"assistant","Role":"user","content":[{"type":"input_text","text":"hi"}]}]}`, + `{"agent":{"model":"m"},"environment":{"type":"none"},"input":[{"role":"user","content":[{"type":"input_text","text":"hi"}],"TYPE":"message"}]}`, } { if w := bodyGateRequest(h, "/v1/agents/sessions", "application/json", []byte(body)); w.Code != http.StatusBadRequest { t.Errorf("%s: %d %s", body, w.Code, w.Body) @@ -111,3 +118,42 @@ func TestCredentialCaseVariantsAreUnknown(t *testing.T) { t.Fatalf("exact names: %d %s", w.Code, w.Body) } } + +// A route rejects a body of unknown keys at the first one, before decoding: +// reading the body, the gate and the member check allocate a small multiple of +// the body in total. Before this batch, Session create allocated about 30 times +// such a body, decoding every member and formatting an error for each key. +func TestUnknownMembersRejectWithLinearAllocation(t *testing.T) { + h, s := validationHandler(t) + unknownKeys := func(prefix string, size int) []byte { + var body bytes.Buffer + body.WriteString(prefix) + for i := 0; body.Len() < size; i++ { + fmt.Fprintf(&body, `"k%07d":0,`, i) + } + body.WriteString(`"z":0}`) + return body.Bytes() + } + for _, tc := range []struct { + path, message string + body []byte + }{ + {"/v1/agents/sessions", "Request must be a JSON object containing supported fields.", unknownKeys(`{"agent":{"model":"m"},"environment":{"type":"none"},"input":"hi","metadata":{"k":"v"},`, 16<<20-64)}, + {"/v1/agents/sessions/" + uuid.NewString() + "/events", "Invalid Session input event request.", unknownKeys(`{"events":[],`, 1<<20-64)}, + } { + runtime.GC() + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + w := bodyGateRequest(h, tc.path, "application/json", tc.body) + runtime.ReadMemStats(&after) + if w.Code != http.StatusBadRequest || !strings.Contains(w.Body.String(), tc.message) { + t.Fatalf("%s: %d %s", tc.path, w.Code, w.Body) + } + if allocated := after.TotalAlloc - before.TotalAlloc; allocated > 8*uint64(len(tc.body)) { + t.Errorf("%s allocated %d MiB for a %d MiB body", tc.path, allocated>>20, len(tc.body)>>20) + } + } + if s.writes != 0 { + t.Fatal("rejected body reached storage") + } +} diff --git a/services/agents-api/internal/api/session_metadata.go b/services/agents-api/internal/api/session_metadata.go index 2fbe81617..718eaecd8 100644 --- a/services/agents-api/internal/api/session_metadata.go +++ b/services/agents-api/internal/api/session_metadata.go @@ -72,11 +72,7 @@ func (h *Handler) updateSession(w http.ResponseWriter, r *http.Request) { // can reject it. Other body and metadata shapes keep their existing errors. // The shared body gate has already rejected repeated keys. func metadataTypeError(body []byte) error { - var fields map[string]json.RawMessage - if json.Unmarshal(body, &fields) != nil { - return nil - } - decoder := json.NewDecoder(bytes.NewReader(fields["metadata"])) + decoder := json.NewDecoder(bytes.NewReader(objectMember(body, "metadata"))) if token, err := decoder.Token(); err != nil || token != json.Delim('{') { return nil } From 4605fa97c25dc695858e83e40bf749ffa96fed57 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 23:04:57 +0000 Subject: [PATCH 10/12] Document the allocation-light member check and hash bits --- CONTRIBUTING.md | 2 +- .../agents-api/official-semantics-alignment.md | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 803eff622..0ca10ec02 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -93,7 +93,7 @@ JSON (including unpaired surrogate escapes), repeated keys and non-object roots with the official messages; an empty body or null becomes `{}`. DELETE, multipart, Core extension and internal routes keep their own readers. Member names match exactly: decode request objects with `decodeInputObject`, or check -`caseVariantMember` before another decoder, so that encoding/json never matches +`inexactMember` before another decoder, so that encoding/json never matches a case variant to a field. See `contracts/agents-api/official-semantics-alignment.md#request-body-parsing--september-23`. Report validation failures with official evidence through the typed field error, diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index b8461b47b..1d9b9f1ee 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -911,13 +911,21 @@ Decisions: - The duplicate key and path are repeated only when each is at most 256 bytes of printable UTF-8, the shared `echotext` rule; otherwise the message is "Invalid body: duplicate JSON key. Duplicate JSON keys are not supported." -- The gate scans each body once in linear time. It keeps key positions in the +- The gate scans each body once in linear time and keeps key positions in the body, not copies: objects with more than 16 keys use an open-addressing set of - 4-byte positions, so a body of many short keys allocates about its own size. + 8-byte slots, a position and 32 hash bits that skip comparing unequal keys. A + body of many short keys allocates about twice its size in the gate. Bodies are + read into a doubling buffer, about twice their size in total. - Member names match exactly on every gated route. encoding/json would match a case variant such as `Metadata`, `Input` or a nested `Role` to the field and let the last copy win; such a key is now an unknown member at any depth, - rejected with the route's existing unknown-member error before any write. + rejected with the route's existing unknown-member error before any write. A + walk over the body bytes checks the member names before the route's decoder + runs and stops at the first unknown or case-variant key, and the Session + metadata check reads only the `metadata` member. A whole Session create of + 16 MiB of unknown keys now allocates about 96 MiB in total and events about + six times a 1 MiB body, instead of 482 MiB and 9 MiB before this batch, when + the decoder formatted an error for every unknown key. Agent configuration already did this. On Agent create, a body with `Metadata` and no `model` still reports the unknown member first, while the official service reported the missing `model`; the official order between several @@ -940,7 +948,8 @@ product repository does not call these routes. Go tests cover the gate on its own (B1–B5, surrogate escapes, the echo bound, media type parsing, deep bodies, a differential check of the duplicate-key scan against an encoding/json token walk on small and large objects, and an allocation -bound on many short keys), case-variant members, and all eleven route families +bound on many short keys), case-variant members, route-level allocation bounds for +unknown keys on Session create and events, and all eleven route families (B1–B4, B6, the order against authentication, Beta and the body limit, B5/B7). A real-PostgreSQL test sends B1–B4, B6 and case-variant members to every route family as the owner and as tenant B under a whole-database digest, then checks From 1a2929864e74c671c1cbd0159a7d3caf914ab2cf Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 23:35:00 +0000 Subject: [PATCH 11/12] Decode unpaired surrogate escapes as U+FFFD without panicking appendUnescaped assumed paired surrogates and sliced past the end of an exact-capacity key such as "\ud800". HTTP cannot reach it, as the gate rejects these escapes first, but inexactMember could. Like encoding/json it now decodes an unpaired or mis-paired surrogate as U+FFFD, so the key matches no member, and keeps a truncated escape as it is. Tests call both functions on lone and mis-paired surrogates in keys, nested objects and arrays. --- .../internal/api/json_members_test.go | 55 +++++++++++++++++-- .../agents-api/internal/api/json_request.go | 22 ++++++-- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/services/agents-api/internal/api/json_members_test.go b/services/agents-api/internal/api/json_members_test.go index 00c853335..ca6c28447 100644 --- a/services/agents-api/internal/api/json_members_test.go +++ b/services/agents-api/internal/api/json_members_test.go @@ -119,10 +119,12 @@ func TestCredentialCaseVariantsAreUnknown(t *testing.T) { } } -// A route rejects a body of unknown keys at the first one, before decoding: -// reading the body, the gate and the member check allocate a small multiple of -// the body in total. Before this batch, Session create allocated about 30 times -// such a body, decoding every member and formatting an error for each key. +// A route rejects a body of unknown top-level keys at the first one, before +// decoding: reading the body, the gate and the member check allocate a small +// multiple of the body in total. Before this batch, Session create allocated +// about 30 times such a body, decoding every member and formatting an error for +// each key. Unknown keys nested under agent or environment still pass the +// existing object decoding and stay linear, at or below the earlier cost. func TestUnknownMembersRejectWithLinearAllocation(t *testing.T) { h, s := validationHandler(t) unknownKeys := func(prefix string, size int) []byte { @@ -157,3 +159,48 @@ func TestUnknownMembersRejectWithLinearAllocation(t *testing.T) { t.Fatal("rejected body reached storage") } } + +// Unpaired surrogate escapes, which the body gate rejects, decode as U+FFFD and +// never panic, also at the end of an exact-capacity slice. +func TestUnpairedSurrogatesNeverPanic(t *testing.T) { + exact := func(s string) []byte { return append(make([]byte, 0, len(s)), s...) } + for escaped, want := range map[string]string{ + `\ud800`: "\ufffd", + `\udc00`: "\ufffd", + `a\ud83d`: "a\ufffd", + `\ud83d\u0041`: "\ufffdA", + `\ud83d\ud83d`: "\ufffd\ufffd", + `\ude00\ud83d`: "\ufffd\ufffd", + `\ud83d\n\ude00`: "\ufffd\n\ufffd", + `\ud83d\ude00`: "😀", + `\ud83d\u`: "\ufffd\\u", + `\u00`: "\\u00", + `a\`: "a\\", + } { + if got := string(appendUnescaped(nil, exact(escaped))); got != want { + t.Errorf("%s: got %q, want %q", escaped, got, want) + } + } + type inner struct { + Name string `json:"name"` + } + type outer struct { + X inner `json:"x"` + Items []inner `json:"items"` + } + typ := reflect.TypeFor[outer]() + for _, body := range []string{ + `{"x":{"\ud800":1}}`, + `{"x":{"name\udc00":1}}`, + `{"items":[{"\ud83d\u0041":1}]}`, + `{"\ud83d":{"name":1}}`, + `{"x":{"n\ud83d\ud83d":1}}`, + } { + if !inexactMember(exact(body), typ) { + t.Errorf("%s: an unpaired surrogate key matched a member", body) + } + } + if inexactMember(exact(`{"x":{"n\u0061me":"\ud800"}}`), typ) { + t.Error("an unpaired surrogate value affected member names") + } +} diff --git a/services/agents-api/internal/api/json_request.go b/services/agents-api/internal/api/json_request.go index 5223a1aee..fb24d4d4f 100644 --- a/services/agents-api/internal/api/json_request.go +++ b/services/agents-api/internal/api/json_request.go @@ -22,8 +22,8 @@ func readJSONBody(w http.ResponseWriter, r *http.Request) ([]byte, bool) { } func readJSONBodyLimit(w http.ResponseWriter, r *http.Request, limit int64, message string) ([]byte, bool) { - // A doubling buffer allocates about twice the body in total; io.ReadAll's - // smaller growth steps allocate about five times a large body. + // A doubling buffer allocates two to four times the body in total, four near + // the limit; io.ReadAll's smaller growth steps allocate 4.4 to 6.1 times. var body bytes.Buffer _, err := body.ReadFrom(http.MaxBytesReader(w, r.Body, limit)) if err == nil { @@ -356,14 +356,19 @@ func (s *keyScanner) text(position uint32) string { return string(s.left) } -// appendUnescaped decodes the contents of a valid JSON string whose surrogate -// escapes are paired. +// appendUnescaped decodes the contents of a JSON string. Like encoding/json, +// it decodes an unpaired or mis-paired surrogate escape as U+FFFD, so such a +// key matches no member name; the body gate rejects these escapes earlier. A +// truncated escape, which valid JSON cannot contain, is kept as it is. func appendUnescaped(dst, s []byte) []byte { for i := 0; i < len(s); i++ { if s[i] != '\\' { dst = append(dst, s[i]) continue } + if i+1 == len(s) || s[i+1] == 'u' && i+6 > len(s) { + return append(dst, s[i:]...) + } i++ switch s[i] { case 'b': @@ -380,8 +385,13 @@ func appendUnescaped(dst, s []byte) []byte { r := hex4(s[i+1 : i+5]) i += 4 if utf16.IsSurrogate(r) { - r = utf16.DecodeRune(r, hex4(s[i+3:i+7])) - i += 6 + low := rune(-1) + if r < 0xdc00 && i+7 <= len(s) && s[i+1] == '\\' && s[i+2] == 'u' { + low = hex4(s[i+3 : i+7]) + } + if r = utf16.DecodeRune(r, low); r != utf8.RuneError { + i += 6 + } } dst = utf8.AppendRune(dst, r) default: From 3c1a8d25fd5a53b04bafbd9f9cc9919f478e10f7 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 23:35:00 +0000 Subject: [PATCH 12/12] State the measured body and member-check allocations precisely The 96 MiB Session create figure holds for unknown top-level keys; unknown keys under agent or environment keep the existing object decoding, linear and at or below main. The doubling reader allocates two to four times the body, against 4.4 to 6.1 times for io.ReadAll. Correct a stale test comment on case-insensitive names. --- .../official-semantics-alignment.md | 20 +++++++++++-------- .../configuration_validation_public_test.go | 3 ++- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index 1d9b9f1ee..de5347f36 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -915,21 +915,25 @@ Decisions: body, not copies: objects with more than 16 keys use an open-addressing set of 8-byte slots, a position and 32 hash bits that skip comparing unequal keys. A body of many short keys allocates about twice its size in the gate. Bodies are - read into a doubling buffer, about twice their size in total. + read into a doubling buffer, which allocates two to four times the body in + total (four near the route limit), against 4.4 to 6.1 times for `io.ReadAll`. - Member names match exactly on every gated route. encoding/json would match a case variant such as `Metadata`, `Input` or a nested `Role` to the field and let the last copy win; such a key is now an unknown member at any depth, - rejected with the route's existing unknown-member error before any write. A - walk over the body bytes checks the member names before the route's decoder - runs and stops at the first unknown or case-variant key, and the Session - metadata check reads only the `metadata` member. A whole Session create of - 16 MiB of unknown keys now allocates about 96 MiB in total and events about - six times a 1 MiB body, instead of 482 MiB and 9 MiB before this batch, when - the decoder formatted an error for every unknown key. + rejected with the route's existing unknown-member error before any write. Agent configuration already did this. On Agent create, a body with `Metadata` and no `model` still reports the unknown member first, while the official service reported the missing `model`; the official order between several errors in one body remains unaligned. +- A walk over the body bytes checks member names before a decoder runs and stops + at the first unknown or case-variant key, and the Session metadata check reads + only the `metadata` member. For unknown top-level keys this makes rejection + cheap: a whole 16 MiB Session create allocates about 96 MiB and events about + six times a 1 MiB body, instead of 482 MiB and 9 MiB before this batch, when + the decoder formatted an error for every unknown key. Unknown keys nested under + `agent` or `environment` still pass the existing object decoding of + `decodeInputObject` and stay linear, at or below main: about 779 and 871 MiB + for 16 MiB bodies, against 850 and 889 MiB on main. - Invalid UTF-8 is checked before JSON syntax; the official order for a body with both faults was not observed. - Not gated: DELETE routes, which keep their empty-body rule, the multipart Files diff --git a/services/agents-api/internal/store/configuration_validation_public_test.go b/services/agents-api/internal/store/configuration_validation_public_test.go index bb8158c9a..160f1170b 100644 --- a/services/agents-api/internal/store/configuration_validation_public_test.go +++ b/services/agents-api/internal/store/configuration_validation_public_test.go @@ -82,7 +82,8 @@ func TestAgentConfigurationValidationRejectsWithoutWritesPostgres(t *testing.T) {"M01", `"multi_agent":{}`, "{p}multi_agent.enabled", "Missing required parameter: '{p}multi_agent.enabled'."}, {"M02", `"multi_agent":{"enabled":true,"max_concurrent_subagents":0}`, "{p}multi_agent.max_concurrent_subagents", "Invalid '{p}multi_agent.max_concurrent_subagents': integer below minimum value. Expected a value >= 1, but got 0 instead."}, // Repeated members would merge when decoded: the shared body gate rejects - // them with a null param (HP-11). Names match case-insensitively when decoded. + // them with a null param (HP-11). Member names match exactly, so a case + // variant is an unknown member. {"merged text", `"text":{"format":{"type":"json_schema","schema":{"type":"array"}}},"text":{"verbosity":"low"}`, "", "Invalid body: duplicate JSON key 'text' at '{p}text'. Duplicate JSON keys are not supported."}, {"merged reasoning", `"reasoning":{"Effort":"high"},"reasoning":{}`, "", "Invalid body: duplicate JSON key 'reasoning' at '{p}reasoning'. Duplicate JSON keys are not supported."}, {"merged location", `"tools":[{"type":"web_search","mode":"disabled","location":{"city":"Paris"},"location":{"country":null}}]`, "", "Invalid body: duplicate JSON key 'location' at '{p}tools.location'. Duplicate JSON keys are not supported."},