diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fc83c3670..0ca10ec02 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -86,6 +86,16 @@ 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 (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 +`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, 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..de5347f36 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,97 @@ 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. 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, 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`. 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`); 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 +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." +- 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 + 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, 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. + 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 + 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 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 +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, 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, 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 +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 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/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..a75521d56 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; 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 @@ -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 ed97cbb38..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 @@ -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..2c768821e 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) @@ -404,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/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_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/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..cbdcfcee5 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,8 +194,6 @@ func TestEnvironmentFileCreateFieldErrors(t *testing.T) { `line\u2028separator`: false, `bell\u0007`: false, `caf\u00e9 \u5b57`: true, - "\xff\xfe": false, - `\ud800`: false, `\ufffd`: false, } { h, f := environmentFileCreateHandler(t) @@ -198,8 +210,15 @@ 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.") + // 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}` 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.go b/services/agents-api/internal/api/function_inputs.go index 5cbe20840..6c877ada8 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,11 @@ func decodeInputObject(raw json.RawMessage, value any, allowed ...string) error return store.ErrInvalidInput } } + // 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)) decoder.DisallowUnknownFields() if decoder.Decode(value) != nil { 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..0eec95774 100644 --- a/services/agents-api/internal/api/handler.go +++ b/services/agents-api/internal/api/handler.go @@ -6,8 +6,8 @@ import ( "encoding/json" "errors" "fmt" - "io" "net/http" + "reflect" v1 "github.com/MiniMax-AI-Dev/parsar/contracts/agents-api/v1" "github.com/MiniMax-AI-Dev/parsar/internal/obs/log" @@ -180,7 +180,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 } @@ -190,14 +190,12 @@ 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 { + // 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 } - 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..c8d441bd6 100644 --- a/services/agents-api/internal/api/inputs.go +++ b/services/agents-api/internal/api/inputs.go @@ -1,11 +1,11 @@ package api import ( + "bytes" "context" "encoding/json" - "errors" - "io" "net/http" + "reflect" "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/proto" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" @@ -36,22 +36,19 @@ 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.") + // 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 } 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_members.go b/services/agents-api/internal/api/json_members.go new file mode 100644 index 000000000..bac5922cf --- /dev/null +++ b/services/agents-api/internal/api/json_members.go @@ -0,0 +1,214 @@ +package api + +import ( + "bytes" + "encoding/json" + "reflect" + "strings" + "sync" +) + +// 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 skipValue(w.raw, i), 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) + } + 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 + } + 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)) + } + 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++ + } + if backslashes%2 == 0 { + return q + 1 + } + 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++ + } +} + +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 +} + +// 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 new file mode 100644 index 000000000..ca6c28447 --- /dev/null +++ b/services/agents-api/internal/api/json_members_test.go @@ -0,0 +1,206 @@ +package api + +import ( + "bytes" + "encoding/json" + "fmt" + "net/http" + "reflect" + "runtime" + "strings" + "testing" + + "github.com/google/uuid" +) + +func TestInexactMember(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"}}`: 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"}`: true, + `{"sHaReD":1,"shared":2}`: true, + `{"items":"not an array","named":null}`: false, + `{"ſhared":"long s folds to s"}`: true, + } { + if got := inexactMember([]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) + } + } + // A nested case variant that encoding/json alone would accept as the field; + // without the check this creates a Session. + for _, body := range []string{ + `{"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) + } + } + 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) + } +} + +// 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 { + 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") + } +} + +// 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 636017861..fb24d4d4f 100644 --- a/services/agents-api/internal/api/json_request.go +++ b/services/agents-api/internal/api/json_request.go @@ -1,19 +1,33 @@ package api import ( + "bytes" + "encoding/json" "errors" - "io" + "fmt" + "hash/maphash" + "mime" "net/http" + "strings" + "unicode/utf16" + "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.") } 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 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 { - return raw, true + return body.Bytes(), true } var tooLarge *http.MaxBytesError if errors.As(err, &tooLarge) { @@ -23,3 +37,366 @@ 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 well-formed parameters. A malformed media type is +// rejected like a missing one. +func jsonContentType(value string) bool { + mediaType, _, err := mime.ParseMediaType(value) + if err != nil { + return false + } + 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. + // Key positions are 31-bit; route limits keep bodies far below that. + if uint64(len(raw)) >= keyEscaped || !json.Valid(raw) { + return nil, errBodyParse + } + key, path, found, err := scanJSON(raw) + if err != nil { + return nil, err + } + if 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)} + } +} + +// 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 []uint64 // hash tag << 32 | key position, 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 c := raw[i]; c { + case '{', '[': + s.frames = append(s.frames, scanFrame{object: c == '{', expectKey: c == '{', base: len(s.small)}) + i++ + case '}', ']': + 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 := &s.frames[len(s.frames)-1]; top.object { + top.expectKey = true + } + i++ + case '"': + 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 + } + } + 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 + } + 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 low := hex4(raw[i+2 : i+6]); low < 0xdc00 || low > 0xdfff { + return 0, true, false + } + i += 6 + } + default: + i++ + } + } +} + +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([]uint64, 4*smallObjectKeys) + for _, key := range s.small[f.base:] { + 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. 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([]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, tag := uint64(len(f.table)-1), hash>>32 + for i := hash & mask; ; i = (i + 1) & mask { + switch slot := f.table[i]; { + case slot == 0: + f.table[i] = tag<<32 | uint64(position) + f.count++ + return false + case slot>>32 == tag && s.equal(uint32(slot), position): + return true + } + } +} + +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] = hash>>32<<32 | uint64(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, s.text(f.member)) + } + } + 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 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': + 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) { + 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: + 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 new file mode 100644 index 000000000..9dc3a041d --- /dev/null +++ b/services/agents-api/internal/api/json_request_test.go @@ -0,0 +1,496 @@ +package api + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "runtime" + "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,"\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.", + `{"\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.", + ` -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 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) + 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) + } + // 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":"\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 { + 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/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) + } + } +} + +// 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() { + // 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_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 { + 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) + } +} + +// 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 + } + } +} + +// 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`, `\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 + } + 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, width, suffixes) + } + return "[" + strings.Join(items, ",") + "]" + default: + 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(suffixes)) + `": ` + value(child, childWidth, suffixes) + } + return "{" + strings.Join(members, ",") + "}" + } + } + duplicates := 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, err := scanJSON(body) + wantKey, wantPath, wantFound := referenceDuplicateJSONKey(body) + 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 || 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 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() + 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 > 5*uint64(len(body))/2 { + 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() +} 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..718eaecd8 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,20 +70,12 @@ 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 { - 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 } - // 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 +83,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..160f1170b 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,12 @@ 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). 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."}, {"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..91c72c4a1 --- /dev/null +++ b/services/agents-api/internal/store/request_body_public_test.go @@ -0,0 +1,245 @@ +package store_test + +import ( + "bytes" + "encoding/json" + "mime/multipart" + "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) + } + } + } + } + // 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") + } + + // 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) + } +} + +// 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/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) 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..da61f3a2a 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,11 @@ 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) + 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 81a157769..ae5c05871 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,12 @@ 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 {}. + 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_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_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: diff --git a/services/agents-api/tests/official_vaults.py b/services/agents-api/tests/official_vaults.py index 781901be1..a8ff4f47b 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,11 @@ 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). + 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)