diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bf87a7b42..db71ce333 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -76,6 +76,18 @@ behavior separately; do not reproduce observed upstream server failures as compatibility behavior. See `contracts/agents-api/list-query-semantics.md` for the bounded evidence. +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. A malformed path +identifier must produce exactly the response of a well-formed missing one on that +route, including invalid bodies, queries and storage availability: resolve it to +the never-assigned maximum UUID and let the missing path run, or reject it +directly only where the lookup is the next check. Malformed list cursors and +request-body references keep their own errors. Reject U+0000 in metadata +explicitly with its `metadata.` param; other stored strings rely on the +PostgreSQL error mapping, so keep each request's writes in one transaction. See +`contracts/agents-api/official-semantics-alignment.md`. + Keep runtime state, test artifacts and build output under `~/.parsar/`. Require absolute user-supplied working directories. Keep credentials out of source and logs. Update this guide when architecture, ownership or generated contracts change. diff --git a/contracts/agents-api/README.md b/contracts/agents-api/README.md index 0eba7be91..57c90ac98 100644 --- a/contracts/agents-api/README.md +++ b/contracts/agents-api/README.md @@ -239,7 +239,8 @@ upgrade the protocol. authentication and Beta header as other resources. The response contains only `id`, `object: vault`, `created_at`, `name` and `metadata`. Omitted name stays null; explicit null is rejected. Supplied strings are trimmed and must contain 1–256 - UTF-8 bytes. Omitted/null metadata becomes `{}` and values must be strings. + UTF-8 bytes. Omitted/null metadata becomes `{}`; a non-string value returns + `invalid_request_error` with param `metadata.`. Session-specific metadata pair/character limits do not apply. The existing 64 KiB encoded metadata and 1 MiB HTTP body bounds are local implementation limits. Creation does not start execution. Retrieval maps missing, malformed and @@ -479,7 +480,10 @@ The Store's internal DTO is not the upstream response model. The API layer must validate and resolve the upstream schema before persistence, and report only supported options. For example, upstream metadata is limited to 16 pairs with 64-character keys and 512-character values; a storage byte limit is not a -replacement for that public validation. +replacement for that public validation. Violations return `invalid_request_error` +with the official `metadata` or `metadata.` param. U+0000 in stored strings +is a local PostgreSQL limit and returns 400 without writing; see the +[validation error batch](official-semantics-alignment.md#validation-error-fields--september-23). Use the pinned official Python client against the actual service, with response validation enabled, for supported Session/Turn/Items operations, pagination, streaming, diff --git a/contracts/agents-api/list-query-semantics.md b/contracts/agents-api/list-query-semantics.md index 1554fb972..1ab5c81ee 100644 --- a/contracts/agents-api/list-query-semantics.md +++ b/contracts/agents-api/list-query-semantics.md @@ -168,11 +168,12 @@ unsampled inputs: These remain registered differences and are not changed here: repeated Files `purpose` values (SFT-18); unsampled overflowing limits; unknown and repeated keys on the Environment Files list, which keeps its own strict parser (its limit range -errors now use the Beta code through the shared limit reader); malformed non-UUID -path IDs (SES-28); metadata and name error envelopes (VA-07/08/09) and U+0000 -(VA-10); Skill sole-version deletion and number reuse (SFT-01/02); Session deletion -lifecycle (SES-29/30); whitespace input (SES-01..04); Template network forms and -codes (SFT-20/21/22); and response defaults (VA-11, SES-23/25). +errors now use the Beta code through the shared limit reader); Skill sole-version +deletion and number reuse (SFT-01/02); Session deletion lifecycle (SES-29/30); +whitespace input (SES-01..04); Template network forms (SFT-21/22); and response +defaults (VA-11, SES-23/25). Malformed path IDs (SES-28), metadata and name error +fields (VA-07/08/09), U+0000 (VA-10) and Template network codes (SFT-20) are +addressed by the [validation error batch](official-semantics-alignment.md#validation-error-fields--september-23). ### Acceptance boundary diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index db2fe79ca..63428f2bc 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -136,3 +136,65 @@ checks passed independently. Rebase onto main `6a3131e` preserved every batch pa The combined tree at `4981580` passed API, execution, contract and dedicated-PostgreSQL Environment scheduling/initial-input/creation-stream regressions. No new E2B, OAuth provider or native capability combination was qualified. + +## Validation error fields — September 23 + +This batch aligns validation failures that Core already rejected with the +official `code` and `param` fields. It does not change any limit. Evidence comes +from the campaign scan at main `284cbcf`, recorded privately in +`~/.parsar/remediation/20260923/campaign-scan-1/{vaults-agents,sessions,skills-files-templates}/findings.json` +(VA-07, VA-08, VA-09, VA-10, SES-28 and SFT-20), plus the September 22 Session +observation that `{"metadata":{"a":null}}` returns param `metadata.a`. + +| Row | Case | Core behavior | +| --- | --- | --- | +| M1–M3 | More than 16 metadata pairs, a key over 64 characters, a value over 512 characters (Agent create/update, Session create/update) | 400 with type and code `invalid_request_error`, param `metadata` or `metadata.`, and the observed official message with the actual count or length. Pairs are checked before keys and values, and keys in sorted order. | +| M4 | A non-string metadata value: integer, number, boolean, object, array or null (Agent create/update, Session create/update and Vault create; neither Core nor the pinned SDK has a Vault update) | 400 `invalid_request_error`, param `metadata.`, message `Invalid type for 'metadata.': expected a string, but got instead.` The first such value in document order is reported before the generic whole-body error. Templates accept no metadata. | +| M5 | Vault metadata size | Unchanged: no pair or length limits, only the local 64 KiB storage bound. | +| N1 | Agent `name` over 128 characters | 400 `invalid_request_error`, param `name`, observed message. Empty and untrimmed names stay accepted. | +| U1 | U+0000 in a stored string | Never 500 and nothing is written. Metadata keys and values report `metadata.`; other strings return 400 `invalid_request_error` with a null param. This is a local limit: PostgreSQL text and jsonb cannot store U+0000, while the official service accepts and echoes it. | +| I1/I2 | A malformed path identifier on any Beta resource route, and on Files, Skills and Skill versions | Byte-for-byte the response of a well-formed missing identifier on that route, including invalid bodies and queries, and a deployment without credential encryption. Foreign, missing and malformed identifiers stay indistinguishable. | +| T1 | Template network rejections (wildcard, port, scheme, IPv6, empty host, empty/null/omitted list with `restricted`, more than 100 domains, domains with another access) and the shared inline Session network | 400 `invalid_request_error` with a null param. Accepted hostname forms are unchanged; other unsupported installation fields keep `unsupported_or_invalid_configuration`. | + +Decisions: + +- A typed field error carries the param and message through the existing error + writer. Metadata type errors are found by reading the metadata object in + document order before generic decoding; limit checks keep their previous + position, so validation order relative to lookups (SES-33) is unchanged. +- U+0000 is checked explicitly in metadata, so the param is exact. All other + stored strings rely on mapping PostgreSQL `22021` (U+0000 or invalid UTF-8 in + a text parameter) and `22P05` (`\u0000` in jsonb) to 400 with the generic + message "Request text contains characters this service cannot store or compare, + such as U+0000 or invalid UTF-8." The same mapping covers query filters, for + example `agent_id=%ff` on the Session list. The persisted string fields are too + many to check one by one, and the database is the single place that knows which + strings are stored. The failing statement aborts its transaction; real-PostgreSQL + tests compare every public table before and after the rejected requests. +- A malformed path identifier resolves to the maximum UUID, which Core never + assigns because it only generates version 4 and 5 UUIDs. The request then + follows exactly the missing-identifier path, including body, query and storage + checks. Routes whose lookup is the next check keep their direct not-found + response. Malformed list cursors and request-body references are unchanged: + Session, Turn, Item, Subagent, Artifact, Agent, Vault and Credential cursors + still return 400 `invalid_request`, and Template cursors keep their existing + not-found response. +- Network messages are Core wording; the official prose is not copied. +- Documented message difference for M2: the official message abbreviated a + 65-character key as `'KKK...KKK'`. That single sample of identical characters + cannot reveal the abbreviation rule, so Core quotes the full key. Status, type, + code and param match. + +Deferred and unchanged: accepting and storing U+0000; hostname forms accepted +officially (SFT-21) and `disabled` with domains, which the official service +accepts (SFT-22); non-canonical UUID spellings such as uppercase, braces or +`urn:uuid:` still resolve to the same resource; Skill sole-version deletion and +number reuse; Session deletion lifecycle; whitespace input; response defaults; +the Environment Files list query parser; and the Files `limit=abc` code. + +Go handler tests cover every row. Real-PostgreSQL tests replay every path-ID +route for malformed, missing and foreign identifiers (tenant B), with valid and +invalid bodies and queries, and replay U+0000 on every create/update family with +a database digest proving no writes. The pinned-SDK acceptance scripts assert the +new codes, params and messages. Independent real-Core acceptance is recorded +separately by the coordinator. diff --git a/contracts/agents-api/openapi.yaml b/contracts/agents-api/openapi.yaml index 0155205f6..7d5aabee9 100644 --- a/contracts/agents-api/openapi.yaml +++ b/contracts/agents-api/openapi.yaml @@ -2196,7 +2196,10 @@ paths: post: consumes: - application/json - description: 'Persists configuration independently of execution. Supports model/name/instructions/metadata, + 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. Supports model/name/instructions/metadata, explicit reasoning and service tiers, multi_agent, text/json_schema, function/tool_search/programmatic_tool_calling and HTTP MCP with nullable credential_id and explicit service origin and boolean required defaulting to false. Saving credential_id grants no access: Session @@ -2343,8 +2346,9 @@ paths: - application/json description: Preserves omitted fields and replaces supplied fields using shared saved-configuration validation. Null name/instructions clear; null or empty - metadata clears all pairs. Existing Session snapshots are unchanged. Empty - updates advance updated_at without changing saved fields. Nested replacement/null + metadata clears all pairs. Name and metadata validation errors return invalid_request_error + with the official param. Existing Session snapshots are unchanged. Empty updates + advance updated_at without changing saved fields. Nested replacement/null defaults, model-derived reasoning and exact hosted error behavior remain incompletely verified. parameters: @@ -2699,7 +2703,8 @@ paths: inline/referenced Skill ZIPs, Plugin ZIPs and workspace-contained capability directories. Omitted/null network defaults to enabled. Restricted network requires 1–100 exact ASCII hostnames; other host forms and populated unsupported - installations are rejected before persistence without echoing input. No compute + installations are rejected before persistence without echoing input. Network + policy rejections return invalid_request_error with a null param. No compute is allocated. Exact hosted error/retry semantics remain unverified. parameters: - description: agents=v1 @@ -2839,7 +2844,8 @@ paths: are encrypted and omitted from responses. Capability directories are snapshotted after setup. Environment MCP execution requires a qualified native transport and runtime network policy. Empty updates advance updated_at without changing - saved fields or confidential contents. + saved fields or confidential contents. Network policy rejections return invalid_request_error + with a null param. parameters: - description: agents=v1 in: header @@ -2986,7 +2992,9 @@ paths: An attached Vault with no matching credential may remain anonymous; missing keys or failed credential lookup/decryption never fall back to anonymous execution. Omitted stream defaults to false; stream and agent_id cannot be null. Metadata - may be null, but its values must be strings. Initial input accepts a string + may be null; non-string values and limit violations return invalid_request_error + with a metadata or metadata. param. Hosted network policy rejections + return invalid_request_error with a null param. Initial input accepts a string or ordered user-message array. Codex and Claude SDK on none and qualified openai_hosted also accept inline PNG/JPEG image content; other image combinations and remote URLs are unsupported. None initial input atomically starts a Turn; @@ -3199,9 +3207,12 @@ paths: - application/json description: The metadata field is required in an update body. Send null or {} to clear it, or supply an object to replace all pairs. Up to 16 string - pairs, with keys at most 64 characters and values at most 512 characters. - Execution configuration and activity are unchanged. Returns the same safe - Environment and pending-input activity projection as Session retrieval. + pairs, with keys at most 64 characters and values at most 512 characters; + violations and non-string values return invalid_request_error with a metadata + or metadata. param. U+0000 is rejected as a local storage limit. Malformed, + missing and foreign Session IDs share the not-found response. Execution configuration + and activity are unchanged. Returns the same safe Environment and pending-input + activity projection as Session retrieval. parameters: - description: agents=v1 in: header @@ -4824,8 +4835,10 @@ paths: description: Creates a project-owned Vault independently of execution. Omitted name stays null; a supplied string is trimmed and must contain 1–256 UTF-8 bytes. Explicit null name is invalid. Omitted/null metadata becomes an empty - object; values must be strings. Metadata has a local 64 KiB encoded storage - bound. Credentials, Session binding and hosted error/retry parity remain incomplete. + object; non-string values return invalid_request_error with a metadata. + param. Metadata has a local 64 KiB encoded storage bound. U+0000 in stored + strings is rejected as a local storage limit. Credentials, Session binding + and hosted error/retry parity remain incomplete. parameters: - description: agents=v1 in: header diff --git a/contracts/agents-api/operation-evidence.md b/contracts/agents-api/operation-evidence.md index 8ac76c37c..bcd32745b 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, and the list query tolerance batch (L) updates list and resource query handling; 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, and the validation error batch (X) updates field error codes/params, malformed path IDs and U+0000 handling; 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. @@ -36,6 +36,7 @@ Repository paths below are relative to the inspected worktree; private evidence | O | `services/agents-api/oauth-credentials.md:144`: recorded genuine Keycloak 26.7.4 PKCE grants, real TLS MCP and Kimi execution. Codex Basic client-auth initial/refresh/restart/replacement/revocation/delete; Claude POST initial/refresh. Trusted `none` profiles only; not MiniMax, hosted, arbitrary-provider or complete error equivalence. | | V | [Resource selector and error qualification](resource-selector-semantics.md); private September23 `skill-version-alignment/official/` and `files-error-alignment/official-probe.json`: owned Skill/Template/Session selector observations and seven missing File/cursor requests. Preserve each probe phase and distinguish actual hosted execution from metadata-only reads. | | W | `~/.parsar/remediation/20260923/file-resource-semantics/official-files/` and `official-skills/`: 56 owned resource requests, no Sessions/models; [qualified observations and limitations](file-resource-semantics.md). | +| X | [Validation error fields](official-semantics-alignment.md#validation-error-fields--september-23); private `~/.parsar/remediation/20260923/campaign-scan-1/{vaults-agents,sessions,skills-files-templates}/findings.json` VA-07/08/09/10, SES-28, SFT-20 and the September 22 Session `metadata.a` null observation. Metadata/name field errors, U+0000 local limit, malformed path IDs and Template network codes; Go handler and real-PostgreSQL route/no-write tests, no model execution. | | L | [List query tolerance](list-query-semantics.md#list-query-tolerance--september-23-2026); private `~/.parsar/remediation/20260923/campaign-scan-1/{vaults-agents,sessions,skills-files-templates}/findings.json`: owned-collection unknown/repeated keys, limit bounds, Vault status union and Files empty purpose, plus unknown keys on a deleted Vault read and Agent delete. Rows A1–D2 of that section; no model execution. | ## Per-operation evidence matrix @@ -44,20 +45,20 @@ 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 | R `agent-create-supported`; initial unsupported model case 400 | C DB create; T saved-Agent execution references | Model-derived reasoning defaults and unsupported configurations; complete default/null/errors unknown | +| 1 | beta.agents.create | P: saved configuration, 201; metadata/name errors use official code and param | R `agent-create-supported`; initial unsupported model case 400; X VA-07/08/09 | C DB create; T saved-Agent execution references; X DB field errors and U+0000 no-write | Model-derived reasoning defaults and unsupported configurations; 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 | R `agent-patch-metadata`, `agent-null-fields`, `agent-noop`, `agent-nested-reasoning`, rejection labels | C DB no-op/unchanged snapshot; resource implementation-validation.md actual PostgreSQL SDK update tests | 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 | R `agent-patch-metadata`, `agent-null-fields`, `agent-noop`, `agent-nested-reasoning`, rejection labels; X VA-07/08/09/10 | C DB no-op/unchanged snapshot; resource implementation-validation.md actual PostgreSQL SDK update tests | 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 | R `agent-list-empty-scoped`, `agent-list-limit101`; L VA-01/02/03/04/18 | L DB `official_list_query.py` 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 | S `create-1/2.json`, `omitted-input.json`, `null-input.json`, `empty-array-input.json`, `retry-status-original/repeat.json`; H stream | N Live none admission and retry; C Live three hosted profiles; D/T/K/I recorded additional workflows | Session admission batch removes idle `none` creation; local idempotent create still differs from two official IDs. Many input/tool/environment combinations restricted | -| 7 | beta.agents.sessions.retrieve | P: persisted state, required actions, usage | S `retrieve-1.json`, `session-after-1.json`; H recovered state | C Live history; T pending actions; H Core acceptance recorded | Complete statuses/actions/lifecycle timing; Claude/MiniMax public usage remains null | -| 8 | beta.agents.sessions.update | P: metadata-only replacement/clear | S `metadata-replace/null/empty/omit/invalid-value.json`; `update-agent.json` uses newer unpinned field | N Live completed Session metadata rejection/clear/isolation; recorded active controlled metadata coverage | Session admission batch changes empty update to observed official 400; Session agent update belongs to baseline upgrade, not fixed-pin operation gap | +| 7 | beta.agents.sessions.retrieve | P: persisted state, required actions, usage; malformed ID equals missing | S `retrieve-1.json`, `session-after-1.json`; H recovered state; X SES-28 | C Live history; T pending actions; H Core acceptance recorded | Complete statuses/actions/lifecycle timing; Claude/MiniMax public usage remains null | +| 8 | beta.agents.sessions.update | P: metadata-only replacement/clear; metadata errors use official code and `metadata`/`metadata.` param | S `metadata-replace/null/empty/omit/invalid-value.json`; `update-agent.json` uses newer unpinned field | N Live completed Session metadata rejection/clear/isolation; recorded active controlled metadata coverage | Session admission batch changes empty update to observed official 400; Session agent update belongs to baseline upgrade, not fixed-pin operation gap | | 9 | beta.agents.sessions.list | P: Agent filter, full envelope, cursor paging; unknown keys ignored, limit 0/above 100 clamp | S `list-filter.json`, `list-empty-after.json`, `list-owned-cross-filter-cursor.json`, limit/order/unknown-query samples; L SES-10/11/15/16/17 | C Live order/cursors/empty/tenant checks; L DB tenant A/B | Core page capacity 100; official cap unknown. Eventual visibility sample is not a required delay | | 10 | beta.agents.sessions.delete | P: public deletion, owned managed cleanup, user compute retained | S cleanup files 200/deleted; retry-session active cleanup initially 409 | D recorded real cleanup; C cleanup separately recorded | Physical purge/retention and all active/unknown-effect races; caller compute ownership preserved | | 11 | beta.agents.sessions.events.create | P: 202/empty body, empty-array authenticated no-op, text/cancel/function admission | S `second-turn-create.json`, `events-empty/null.json`; H second-input | C Live real continuation/no-op; T qualified message/result/cancel workflows | Mixed prepared-environment batches, native receipt vs durable acceptance, cancel-before-result-publication timing; unqualified content/tools | | 12 | beta.agents.sessions.events.stream | P: live-only SSE, typed persisted projections | H create/reconnect frames; no historical frames in sampled idle interval | C Live; H recorded three-harness disconnect/recovery; T pending actions | Full SSE/Item variants/order; child deltas settle late; no replay guarantee or observer-disconnect proof for every state | -| 13 | beta.agents.sessions.turns.retrieve | P: persisted root/child Turn identity | H/S contain Turn list payloads; no isolated positive retrieve raw request identified in this set | Recorded H/B scoped Turn recovery; C history uses list | Distinguish list-shape evidence from retrieve wire qualification; full lifecycle/usage | -| 14 | beta.agents.sessions.turns.list | P: full envelope, ordered root+child history; limit outside 1–100 rejects with the Beta code | S `turns-final/empty-page/limit-high/order-empty.json`; H both directions; L SES-14/15/16/17 | C Live paging; H/B real child/root identity recorded; L DB tenant A/B | All interleavings, same-timestamp paging, interim/failed usage | +| 13 | beta.agents.sessions.turns.retrieve | P: persisted root/child Turn identity; malformed ID equals missing | H/S contain Turn list payloads; no isolated positive retrieve raw request identified in this set; X SES-28 official `turn_` 404 | Recorded H/B scoped Turn recovery; C history uses list | Distinguish list-shape evidence from retrieve wire qualification; full lifecycle/usage | +| 14 | beta.agents.sessions.turns.list | P: full envelope, ordered root+child history; limit outside 1–100 rejects with the Beta code | S `turns-final/empty-page/limit-high/order-empty.json`; H both directions; L SES-14/15/16/17; X SES-28 | C Live paging; H/B real child/root identity recorded; L DB tenant A/B | All interleavings, same-timestamp paging, interim/failed usage | | 15 | beta.agents.sessions.items.list | P: scoped root Items, full envelope; limit 0/above 100 clamp within the pinned 1–100 page | S `items-final/empty-page/limit-high.json`; H both directions; L SES-12/13/15/16/17 | C Live; H/T recorded content/coordination/result variants; L DB tenant A/B | Full Item union. Newer turn_id filter excluded from pin | | 16 | beta.agents.sessions.artifacts.retrieve | P: immutable captured metadata | None located | D recorded live Docker/user-managed workspace output | Exact hosted metadata/default/error and capture-edge parity | | 17 | beta.agents.sessions.artifacts.list | P: scoped stored list | None located | D recorded live output enumeration | Paging during capture/delete; unchanged-file republishing | @@ -72,12 +73,12 @@ Paths in the appendix include `/v1`. SDK names here omit `client.`. `P` means pa | 26 | beta.agents.environments.retrieve | P: durable status, safe configured installation metadata | None located | D/I/K recorded native readiness and metadata | All lifecycle timing and installation inventory; configured metadata is not arbitrary workspace discovery | | 27 | beta.agents.environments.files.create | P: inline/source-file copy to qualified workspace | None located | F/D/K recorded real copy, hashes/consumption/retention | 50 MiB local bound, parent/path/overwrite/error/unknown-write semantics | | 28 | beta.agents.environments.files.list | P: direct regular-file directory, opaque cursor | None located | F/D/K recorded live workspace listing | 1,024-entry prefilter bound; no recursion/symlinks; exact defaults/path/errors/mutation invalidation unknown | -| 29 | beta.agents.environments.templates.create | P: reusable network/files/env/setup/packages/Skills/Plugins/capability config, 201 | R `template-create`; V nullable Skill selector projection | C DB; I/K recorded real frozen-reference initialization | Restricted forms and unqualified combinations; complete hosted initialization semantics | +| 29 | beta.agents.environments.templates.create | P: reusable network/files/env/setup/packages/Skills/Plugins/capability config, 201; network rejections use `invalid_request_error` | R `template-create`; V nullable Skill selector projection; X SFT-20 | C DB; I/K recorded real frozen-reference initialization | Restricted forms and unqualified combinations; complete hosted initialization semantics | | 30 | beta.agents.environments.templates.retrieve | P: safe resource read | R `template-read`, deleted owned read; V nullable Skill selector projection | C DB; I/K recorded reference workflow | Full field/default/redaction parity; no live-secret projection inference | -| 31 | beta.agents.environments.templates.update | P: field replacement/null clearing, empty timestamp touch | R `template-patch`, `template-null`, `template-noop`; V nullable Skill selector projection | C DB no-op; I/K recorded frozen Session behavior | Template update and referencing Session selection are distinct; composition and null inheritance are qualified separately in environment-templates.md/template-null-selection.md; uncommon fields/errors remain unverified | +| 31 | beta.agents.environments.templates.update | P: field replacement/null clearing, empty timestamp touch; network rejections use `invalid_request_error` | R `template-patch`, `template-null`, `template-noop`; V nullable Skill selector projection; X SFT-20 | C DB no-op; I/K recorded frozen Session behavior | Template update and referencing Session selection are distinct; composition and null inheritance are qualified separately in environment-templates.md/template-null-selection.md; uncommon fields/errors remain unverified | | 32 | beta.agents.environments.templates.list | P: scoped cursor list; unknown keys ignored, limit 0/above 100 clamp | R `template-list-empty-scoped`; L SFT-11/23/24 | Recorded resource DB checks; L DB tenant A/B | Multipage mutation and default parity | | 33 | beta.agents.environments.templates.delete | P: delete resource, preserve committed Session snapshot | R `cleanup` at Template path, post-delete read | C DB; K recorded live deletion then continuation | Concurrent references/delete and exact errors | -| 34 | beta.agents.vaults.create | P: tenant resource, 201 | R `vault-create`, `vault-empty-token-fixture` | C DB; O recorded MCP attachment workflow | Archive lifecycle, full defaults and selection parity | +| 34 | beta.agents.vaults.create | P: tenant resource, 201; non-string metadata reports `metadata.`; no pair/length limits | R `vault-create`, `vault-empty-token-fixture`; X VA-08/10/18 | C DB; O recorded MCP attachment workflow; X DB U+0000 no-write | Archive lifecycle, full defaults and selection parity | | 35 | beta.agents.vaults.retrieve | P: safe metadata/status | R `vault-read`, post-cleanup read | C DB deleted/error checks; O recorded lifecycle | Full archived-state and visibility semantics | | 36 | beta.agents.vaults.list | P: stored status filter (scalar and `status[]` union), separate clamping pagination | R `vault-list-empty-scoped`; L VA-02/03/04/05/06/18 | Recorded resource DB coverage; L DB tenant A/B | Real archive transitions; official negative-limit 400 is a recorded pin conflict | | 37 | beta.agents.vaults.delete | P: atomic credential cascade, frozen attachment boundaries | R `cleanup` at Vault path | C DB deletion; O recorded grant lifecycle | Archive vs delete, already-delivered tokens and active effects | @@ -98,14 +99,14 @@ Paths in the appendix include `/v1`. SDK names here omit `client.`. `P` means pa | 52 | skills.delete | P: remove owned source, retain committed Session content | None located | K recorded DB and Live source deletion/continuation | Exact hosted deletion/idempotence/default/latest semantics | | 53 | skills.content.retrieve | P: unversioned content selects default | W distinct default1/latest2 bytes and default2 transition | K recorded DB plus joint resource acceptance | Headers/errors and source deletion/read races | | 54 | skills.versions.create | P: immutable increasing version; optional default change | V second version default=false preserves default1/latest2 | K recorded DB resources | Broader numbering/default/top-level metadata/error/null semantics; upload limits | -| 55 | skills.versions.retrieve | P: owned immutable version metadata | V immediate version1 read404, bounded delayed read200 | K recorded DB resources and Live concrete Session freeze | Visibility timing is observational; full selector/metadata/error parity unqualified | +| 55 | skills.versions.retrieve | P: owned immutable version metadata; malformed version path equals missing | V immediate version1 read404, bounded delayed read200 | K recorded DB resources and Live concrete Session freeze | Visibility timing is observational; full selector/metadata/error parity unqualified | | 56 | skills.versions.list | P: scoped version cursor list; limit 0 empty page with has_more | V delayed owned list contains created versions; L SFT-08/09 | K recorded DB resource checks; L DB zero page | Exact ordering/cursors/default and concurrent version mutation | | 57 | skills.versions.delete | P: nondefault deletion; default rejects invalid_value/version | W two-version default400; latest200 with parent pointer fallback | Skill store and joint resource acceptance | Sole deletion/number reuse unverified; observed stale official version reads are not emulated | | 58 | skills.versions.content.retrieve | P: decrypt/read immutable concrete bundle | W v1/v2 ZIP members and markers | K recorded DB/live consumption; joint resource acceptance | Content headers/errors and source deletion/read races | ## Remaining gaps without task ordering -1. **Public generic semantics:** sampled create/event/envelope/error/no-op corrections are merged. L aligns unknown/repeated list keys, sampled limit bounds and single-resource unknown keys. Resource-by-resource omissions/null/default/error params, malformed path IDs, overflowing limits, Environment Files list query tolerance, list caps, concurrent mutation and deletion require separate evidence. Metadata U+0000 remains an implementation-validation question from the prior audit, not a newly reproduced result here. +1. **Public generic semantics:** sampled create/event/envelope/error/no-op corrections are merged. L aligns unknown/repeated list keys, sampled limit bounds and single-resource unknown keys. X gives malformed path IDs on every Beta, Files and Skills route the exact missing-resource response, reports metadata/name field errors with official code and param, maps Template network rejections to `invalid_request_error`, and rejects U+0000 in stored strings as a documented local limit (the official service stores it). Other resource-by-resource omissions/null/default/error params, overflowing limits, Environment Files list query tolerance, list caps, concurrent mutation and deletion require separate evidence. 2. **Session differences:** The Session admission batch removes idle `none` creation and empty metadata update. Local durable creation idempotency remains an explicit difference. Whitespace-only input succeeds officially but is rejected by the existing Core message validator; this newly observed difference is queued separately. Session agent updates, newer Environment shapes and root Item turn_id are baseline-upgrade questions. 3. **Template/Skill composition:** shared env/files/setup/packages selection is covered by the composition batch; template-reference null network/capability lists are covered by the null-selection batch. Official derived capability-directory projection remains different. Skill content/default metadata are covered by file-resource-semantics.md; sole-version deletion, visibility and broader numbering/error behavior remain unverified. 4. **Execution coverage:** use T's qualified matrix, not a blanket missing-image/structured-output claim. MiniMax functions/service MCP, optional tool combinations, unsupported images/placements and broader native lifecycle are explicit restrictions. PTC omission retains approved native behavior; Claude/MiniMax public Usage remains null; child settlement cadence/native close limits remain visible. No second executor/model loop or guessed counters are justified. diff --git a/services/agents-api/README.md b/services/agents-api/README.md index 52e1feb0e..985361dc4 100644 --- a/services/agents-api/README.md +++ b/services/agents-api/README.md @@ -123,7 +123,10 @@ native continuity and same-tenant device bindings. The public API applies schema validation/defaults before storage. Internal bounds are 64 KiB for metadata and 512 KiB for configuration. Keep credentials out of both. Public metadata permits at most 16 string pairs, 64-character keys and 512-character values; storage bounds -do not replace those rules. Tenant identity comes from authenticated credentials, +do not replace those rules. Violations and non-string values return +`invalid_request_error` with a `metadata` or `metadata.` param. PostgreSQL +cannot store U+0000, so requests containing it in any stored string return 400 +before anything is written; this is a local limit, not hosted parity. Tenant identity comes from authenticated credentials, never metadata or a caller-supplied business identity. ## Internal Turn persistence diff --git a/services/agents-api/internal/api/agents.go b/services/agents-api/internal/api/agents.go index ba7af6ed5..851e45e6c 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. Supports model/name/instructions/metadata, explicit reasoning and service tiers, multi_agent, text/json_schema, function/tool_search/programmatic_tool_calling and HTTP MCP with nullable credential_id and explicit service origin 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, enabled web_search and public retry conformance remain incomplete. Explicit disabled web_search can be saved; Session execution also accepts explicit disabled programmatic_tool_calling through qualified Runtime controls. 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. Supports model/name/instructions/metadata, explicit reasoning and service tiers, multi_agent, text/json_schema, function/tool_search/programmatic_tool_calling and HTTP MCP with nullable credential_id and explicit service origin 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, enabled web_search and public retry conformance remain incomplete. Explicit disabled web_search can be saved; Session execution also accepts explicit disabled programmatic_tool_calling through qualified Runtime controls. Session execution admits only its supported configuration subset. // @Tags Agents // @Accept json // @Produce json @@ -35,6 +35,9 @@ func (h *Handler) createAgent(w http.ResponseWriter, r *http.Request) { if !ok { return } + if writeFieldError(w, metadataTypeError(raw)) { + return + } var request v1.CreateAgentRequest if decodeInputObject(raw, &request, "model", "name", "instructions", "metadata", "multi_agent", "reasoning", "service_tier", "text", "tools", "x_agents_core") != nil { writeError(w, http.StatusBadRequest, "invalid_request", "Request must be a JSON object containing supported fields.") @@ -42,7 +45,9 @@ func (h *Handler) createAgent(w http.ResponseWriter, r *http.Request) { } input, err := resolveSavedAgent(request) if err != nil { - writeError(w, http.StatusBadRequest, "unsupported_or_invalid_configuration", err.Error()) + if !writeFieldError(w, err) { + writeError(w, http.StatusBadRequest, "unsupported_or_invalid_configuration", err.Error()) + } return } agent, err := h.store.CreateAgent(r.Context(), tenantID(r), input) diff --git a/services/agents-api/internal/api/agents_update.go b/services/agents-api/internal/api/agents_update.go index f71a361ef..04ca88d2d 100644 --- a/services/agents-api/internal/api/agents_update.go +++ b/services/agents-api/internal/api/agents_update.go @@ -11,7 +11,7 @@ import ( ) // @Summary Update a reusable Agent -// @Description Preserves omitted fields and replaces supplied fields using shared saved-configuration validation. Null name/instructions clear; null or empty metadata clears all pairs. Existing Session snapshots are unchanged. Empty updates advance updated_at without changing saved fields. Nested replacement/null defaults, model-derived reasoning and exact hosted error behavior remain incompletely verified. +// @Description Preserves omitted fields and replaces supplied fields using shared saved-configuration validation. Null name/instructions clear; null or empty metadata clears all pairs. Name and metadata validation errors return invalid_request_error with the official param. Existing Session snapshots are unchanged. Empty updates advance updated_at without changing saved fields. Nested replacement/null defaults, model-derived reasoning and exact hosted error behavior remain incompletely verified. // @Tags Agents // @Accept json // @Produce json @@ -29,13 +29,15 @@ func (h *Handler) updateAgent(w http.ResponseWriter, r *http.Request) { } input, err := resolveAgentUpdate(raw) if err != nil { - writeError(w, http.StatusBadRequest, "unsupported_or_invalid_configuration", err.Error()) + if !writeFieldError(w, err) { + writeError(w, http.StatusBadRequest, "unsupported_or_invalid_configuration", err.Error()) + } return } id := chi.URLParam(r, "agent_id") if !validAgentID(id) { - writeStoreError(w, r, store.ErrNotFound) - return + // Storage validation precedes the lookup; follow the missing-Agent path. + id = store.UnknownResourceID } updated, err := h.store.UpdateAgent(r.Context(), tenantID(r), id, input) if err != nil { @@ -46,6 +48,9 @@ func (h *Handler) updateAgent(w http.ResponseWriter, r *http.Request) { } func resolveAgentUpdate(raw []byte) (store.UpdateAgentInput, error) { + if err := metadataTypeError(raw); err != nil { + return store.UpdateAgentInput{}, err + } var request v1.UpdateAgentRequest if decodeInputObject(raw, &request, "model", "name", "instructions", "metadata", "multi_agent", "reasoning", "service_tier", "text", "tools", "x_agents_core") != nil { return store.UpdateAgentInput{}, errors.New("Request must be a JSON object containing supported fields.") diff --git a/services/agents-api/internal/api/credentials.go b/services/agents-api/internal/api/credentials.go index 5cb61813e..d8a94d0a4 100644 --- a/services/agents-api/internal/api/credentials.go +++ b/services/agents-api/internal/api/credentials.go @@ -34,10 +34,7 @@ type CredentialStore interface { // @Failure 400,401,404,413,500,503 {object} v1.ErrorResponse // @Router /vaults/{vault_id}/credentials [post] func (h *Handler) createCredential(w http.ResponseWriter, r *http.Request) { - vaultID, ok := credentialResourceID(w, r, "vault_id") - if !ok { - return - } + vaultID := credentialPathID(r, "vault_id") raw, ok := readJSONBody(w, r) if !ok { return @@ -110,6 +107,8 @@ func (h *Handler) getCredential(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, credentialResponse(credential)) } +// credentialResourceID rejects a malformed Vault or Credential identifier with +// the not-found response. Use it only where the lookup is the next check. func credentialResourceID(w http.ResponseWriter, r *http.Request, param string) (string, bool) { id, err := uuid.Parse(chi.URLParam(r, param)) if err != nil || id == uuid.Nil { @@ -119,6 +118,16 @@ func credentialResourceID(w http.ResponseWriter, r *http.Request, param string) return id.String(), true } +// credentialPathID resolves a malformed identifier to one that never exists, +// so body, query and storage checks run exactly as for a missing identifier. +func credentialPathID(r *http.Request, param string) string { + id, err := uuid.Parse(chi.URLParam(r, param)) + if err != nil || id == uuid.Nil { + return store.UnknownResourceID + } + return id.String() +} + func credentialResponse(c store.Credential) v1.Credential { auth := v1.CredentialAuth{Type: c.AuthType, MCPServerURL: c.MCPServerURL} if c.OAuth != nil { diff --git a/services/agents-api/internal/api/credentials_list.go b/services/agents-api/internal/api/credentials_list.go index 6183e9f8f..f54ad5c70 100644 --- a/services/agents-api/internal/api/credentials_list.go +++ b/services/agents-api/internal/api/credentials_list.go @@ -22,10 +22,7 @@ import ( // @Failure 400,401,404,500 {object} v1.ErrorResponse // @Router /vaults/{vault_id}/credentials [get] func (h *Handler) listCredentials(w http.ResponseWriter, r *http.Request) { - vaultID, ok := credentialResourceID(w, r, "vault_id") - if !ok { - return - } + vaultID := credentialPathID(r, "vault_id") options, statuses, ok := readVaultPage(w, r) if !ok { return diff --git a/services/agents-api/internal/api/credentials_list_test.go b/services/agents-api/internal/api/credentials_list_test.go index e86201e85..d9bcc4686 100644 --- a/services/agents-api/internal/api/credentials_list_test.go +++ b/services/agents-api/internal/api/credentials_list_test.go @@ -63,8 +63,10 @@ func TestCredentialListRejectsInvalidInputBeforeStorage(t *testing.T) { t.Fatal("invalid query reached storage", suffix, w.Code) } } + // A malformed parent follows the missing-Vault path, after query validation. h, f, _ := credentialHandler(t) - if w := credentialRequest(h, "GET", "/v1/vaults/invalid/credentials", ""); w.Code != 404 || f.calls != 0 { - t.Fatal("invalid parent reached storage") + f.err = store.ErrNotFound + if w := credentialRequest(h, "GET", "/v1/vaults/invalid/credentials", ""); w.Code != 404 || f.vault != store.UnknownResourceID { + t.Fatal("invalid parent was not resolved as a missing Vault", w.Code, f.vault) } } diff --git a/services/agents-api/internal/api/credentials_update.go b/services/agents-api/internal/api/credentials_update.go index 6ae3fee19..d99c9c874 100644 --- a/services/agents-api/internal/api/credentials_update.go +++ b/services/agents-api/internal/api/credentials_update.go @@ -22,14 +22,7 @@ import ( // @Failure 400,401,404,413,500,503 {object} v1.ErrorResponse // @Router /vaults/{vault_id}/credentials/{credential_id} [post] func (h *Handler) updateCredential(w http.ResponseWriter, r *http.Request) { - vaultID, ok := credentialResourceID(w, r, "vault_id") - if !ok { - return - } - id, ok := credentialResourceID(w, r, "credential_id") - if !ok { - return - } + vaultID, id := credentialPathID(r, "vault_id"), credentialPathID(r, "credential_id") raw, ok := readJSONBody(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 1d670d9f3..315ddfe46 100644 --- a/services/agents-api/internal/api/credentials_update_test.go +++ b/services/agents-api/internal/api/credentials_update_test.go @@ -69,6 +69,12 @@ func TestCredentialUpdateUsesExistingBoundariesAndSafeErrors(t *testing.T) { h, f, _ := credentialHandler(t) path := "/v1/vaults/" + f.credential.VaultID + "/credentials/" + f.credential.ID method, status := "POST", http.StatusBadRequest + // Malformed identifiers reach storage only as the never-assigned ID, + // after body validation, exactly like a well-formed missing identifier. + malformed := strings.Contains(mode, "invalid") || strings.Contains(mode, "zero") + if malformed { + f.err = store.ErrNotFound + } switch mode { case "method": method, status = "PATCH", http.StatusMethodNotAllowed @@ -92,8 +98,8 @@ func TestCredentialUpdateUsesExistingBoundariesAndSafeErrors(t *testing.T) { } w := httptest.NewRecorder() h.ServeHTTP(w, r) - if w.Code != status || f.calls != 0 { - t.Fatal("update boundary changed", mode, w.Code) + if w.Code != status || !malformed && f.calls != 0 || malformed && (f.calls != 1 || f.vault != store.UnknownResourceID && f.id != store.UnknownResourceID) { + t.Fatal("update boundary changed", mode, w.Code, f.vault, f.id) } } for _, tc := range []struct { diff --git a/services/agents-api/internal/api/environment_templates.go b/services/agents-api/internal/api/environment_templates.go index 7ad261b3a..0e66cc828 100644 --- a/services/agents-api/internal/api/environment_templates.go +++ b/services/agents-api/internal/api/environment_templates.go @@ -75,6 +75,9 @@ func readTemplateInput(w http.ResponseWriter, r *http.Request) (store.Environmen return store.EnvironmentTemplateInput{}, false } in, err := decodeTemplateInput(raw) + if writeFieldError(w, err) { + return in, false + } if err != nil { writeError(w, http.StatusBadRequest, "unsupported_or_invalid_configuration", "Template fields are invalid or require unsupported initialization. Name, enabled/disabled or exact-domain restricted network, initial files, env, system/npm/Python packages, setup commands inline/referenced Skill ZIPs, Plugin ZIPs and workspace capability directories are supported.") return in, false @@ -83,7 +86,7 @@ func readTemplateInput(w http.ResponseWriter, r *http.Request) (store.Environmen } // @Summary Create an Environment Template -// @Description Saves tenant-owned hosted configuration. Supports nullable name, enabled/disabled or exact-domain restricted network, initial inline/file_id files, confidential env, ordered setup_commands, system/npm/Python packages inline/referenced Skill ZIPs, Plugin ZIPs and workspace-contained capability directories. Omitted/null network defaults to enabled. Restricted network requires 1–100 exact ASCII hostnames; other host forms and populated unsupported installations are rejected before persistence without echoing input. No compute is allocated. Exact hosted error/retry semantics remain unverified. +// @Description Saves tenant-owned hosted configuration. Supports nullable name, enabled/disabled or exact-domain restricted network, initial inline/file_id files, confidential env, ordered setup_commands, system/npm/Python packages inline/referenced Skill ZIPs, Plugin ZIPs and workspace-contained capability directories. Omitted/null network defaults to enabled. Restricted network requires 1–100 exact ASCII hostnames; other host forms and populated unsupported installations are rejected before persistence without echoing input. Network policy rejections return invalid_request_error with a null param. No compute is allocated. Exact hosted error/retry semantics remain unverified. // @Tags Environment Templates // @Accept json // @Produce json @@ -126,7 +129,7 @@ func (h *Handler) getEnvironmentTemplate(w http.ResponseWriter, r *http.Request) } // @Summary Update an Environment Template -// @Description Supplied fields replace atomically; omitted fields remain unchanged. Null name clears and null network resets to the pinned enabled default. Existing Session snapshots and creation retries remain unchanged. Initial files replace as a list; null/empty clears. File data is encrypted separately and excluded from response metadata. Skills replace as a list; null/empty clears. Skill archives are encrypted separately and omitted from responses. Plugins and capability directories replace as lists; null/empty clears. Plugin archives are encrypted and omitted from responses. Capability directories are snapshotted after setup. Environment MCP execution requires a qualified native transport and runtime network policy. Empty updates advance updated_at without changing saved fields or confidential contents. +// @Description Supplied fields replace atomically; omitted fields remain unchanged. Null name clears and null network resets to the pinned enabled default. Existing Session snapshots and creation retries remain unchanged. Initial files replace as a list; null/empty clears. File data is encrypted separately and excluded from response metadata. Skills replace as a list; null/empty clears. Skill archives are encrypted separately and omitted from responses. Plugins and capability directories replace as lists; null/empty clears. Plugin archives are encrypted and omitted from responses. Capability directories are snapshotted after setup. Environment MCP execution requires a qualified native transport and runtime network policy. Empty updates advance updated_at without changing saved fields or confidential contents. Network policy rejections return invalid_request_error with a null param. // @Tags Environment Templates // @Accept json // @Produce json diff --git a/services/agents-api/internal/api/errors.go b/services/agents-api/internal/api/errors.go index 4d7d9c49d..01af33b62 100644 --- a/services/agents-api/internal/api/errors.go +++ b/services/agents-api/internal/api/errors.go @@ -39,6 +39,31 @@ func writeError(w http.ResponseWriter, status int, code, message string, param . writeJSON(w, status, v1.ErrorResponse{Error: v1.APIError{Message: message, Type: kind, Code: errorCode, Param: errorParam}}) } +const unstorableTextMessage = "Request text contains characters this service cannot store or compare, such as U+0000 or invalid UTF-8." + +// fieldError is a request validation failure reported with the official +// invalid_request_error code. An empty param serializes as null. +type fieldError struct { + param, message string +} + +func (e *fieldError) Error() string { return e.message } + +// writeFieldError reports a fieldError and returns false for any other error, +// which keeps its caller's existing local code. +func writeFieldError(w http.ResponseWriter, err error) bool { + var field *fieldError + if !errors.As(err, &field) { + return false + } + if field.param == "" { + writeError(w, http.StatusBadRequest, "invalid_request_error", field.message) + } else { + writeError(w, http.StatusBadRequest, "invalid_request_error", field.message, field.param) + } + return true +} + func writeStoreError(w http.ResponseWriter, r *http.Request, err error, notFoundParam ...string) { switch { case errors.Is(err, store.ErrDefaultSkillVersion): @@ -68,6 +93,10 @@ func writeStoreError(w http.ResponseWriter, r *http.Request, err error, notFound writeError(w, http.StatusConflict, "idempotency_conflict", "This idempotency key was used with different input.") case errors.Is(err, store.ErrInvalidInput): writeError(w, http.StatusBadRequest, "invalid_request", "Invalid resource identifier or request limits.") + case store.UnstorableText(err): + // A documented local limit: PostgreSQL text and jsonb cannot store U+0000, + // and text parameters, including query filters, reject invalid UTF-8. + writeError(w, http.StatusBadRequest, "invalid_request_error", unstorableTextMessage) default: // Driver errors can include submitted values; do not log the raw error. log.Ctx(r.Context()).Error("agents-api persistence operation failed") diff --git a/services/agents-api/internal/api/handler.go b/services/agents-api/internal/api/handler.go index 5c95e3896..099211726 100644 --- a/services/agents-api/internal/api/handler.go +++ b/services/agents-api/internal/api/handler.go @@ -1,6 +1,7 @@ package api import ( + "bytes" "context" "encoding/json" "errors" @@ -126,7 +127,7 @@ func NewHandler(s ResourceStore, auth *Authenticator, engine string, options ... // createSession atomically reserves or admits initial text with the Session. // @Summary Create an execution Session -// @Description Supports inline configuration or a tenant-owned saved agent_id with per-Session field replacements. Execution supports model/instructions, text verbosity, non-deferred function tools, adapter-qualified multi_agent with persisted Subagent reads, implicit reasoning, service tier auto and environment type none, subject to the configured engine. Codex additionally supports HTTP MCP with explicit service origin, native allowed_tools and boolean required defaulting to false. Session vault_ids attach only project-owned Vaults; credential_id selects an attached static bearer credential for the exact HTTPS URL, while null/omission selects a unique match or remains anonymous. Ambiguous selection rejects creation. Frozen private selections never populate an omitted public credential_id; missing decryption configuration fails dispatch without anonymous fallback. Required initialization uses native startup before the first native Turn, including cold resume, and requires a separately advertised capability; exact hosted creation timing and error parity remain unverified. Other MCP origins and OAuth remain unsupported. The self_hosted profile requires Codex, an absolute workspace_directory and empty capability_directories, with optional non-deferred function tools and HTTP MCP using explicit service origin, optionally authenticated by the attached Vault rules. Remote MCP and remote Bearer authentication each require separately advertised combination support; old peers cannot receive unsupported work. Omitted/null capability_directories use the empty-list default; self_hosted requires configured execution plus executor registry. Claude SDK currently requires medium verbosity and object-root function schemas. It supports anonymous or attached static-bearer service-origin HTTP MCP on none with boolean required and separately advertised MCP/bearer/required runtime support. Required servers must be connected before the first native input is released; pending or failed startup rejects execution. The shared Vault selection and immutable binding rules apply; unsupported native labels/tool names reject before persistence. An attached Vault with no matching credential may remain anonymous; missing keys or failed credential lookup/decryption never fall back to anonymous execution. Omitted stream defaults to false; stream and agent_id cannot be null. Metadata may be null, but its values must be strings. Initial input accepts a string or ordered user-message array. Codex and Claude SDK on none and qualified openai_hosted also accept inline PNG/JPEG image content; other image combinations and remote URLs are unsupported. None initial input atomically starts a Turn; self_hosted initial input is reserved while returning its Environment connection target, with execution deferred to native readiness and Session failure on initial timeout. Initial input is required for none and for streamed creation outside self_hosted. Omitted/null input remains valid for non-streaming hosted and self_hosted creation. With stream=true, returns live Session events starting at creation; disconnect does not cancel execution. New Sessions retain their authenticated creator; all creation retries require the same typed subject, including across key rotation. Saved-Agent retries and inline requests using Vault attachments or credential references retain caller intent independently of later resource changes; unrelated inline retries preserve resolved/default equivalences. Unknown historical creators reject retries; known creators without recorded intent retain resolved-snapshot retry rules. These conflict policies are local and not verified hosted parity. Creation retries observe future events without replay; retry with stream=false to retrieve the Session. Claude SDK on none and Core-managed Docker openai_hosted supports qualified object-root json_schema output with medium verbosity, single-Agent execution and ordinary functions. Hosted execution reuses native workspace tools and Files/Artifacts; Skills, Plugins, capability directories, HTTP MCP, Subagent and tool_search combinations remain unqualified, including inherited template contents. Other non-text initial input remains unsupported. Basic Codex and Claude SDK openai_hosted creation requires an explicitly configured managed provider. The Claude workspace profile supports non-deferred function tools with text or successful inline PNG/JPEG results alongside native workspace tools; HTTP MCP remains unsupported. Idle Sessions provision automatically; initial provisioning has no caller connection action. Network defaults to enabled; disabled and restricted exact ASCII hostnames are supported. Restricted policy requires 1–100 allowed domains. Unsupported hostname forms and startup installations are rejected. Confidential env, system/npm/Python packages and ordered setup commands use the shared initialization lifecycle; requested network applies after setup. Initial inline and tenant-owned file_id files freeze encrypted bytes before provisioning, then install through the common Core lifecycle before native execution or live Files access. With a template reference, omitted/null files, env, packages and setup_commands inherit. Non-null files and command lists replace; env overlays by key; each package manager inherits on omission/null and otherwise replaces its list. Empty lists clear their selected field. Tenant-owned environment_template_id references inherit omitted/null network and allow only narrowing overrides. Inline hosted network:null retains the enabled default; updating a Template with network:null resets its saved policy to enabled. Core freezes effective configuration; template updates/deletion do not alter Session snapshots or same-intent creation retries. Inline or tenant-owned skill_reference Skills share initialization. Templates preserve default/latest/explicit selectors; Session creation freezes concrete metadata and encrypted content atomically. Skill, Plugin and capability-directory list omission/null inherit; a non-null list replaces, including empty-list clearing. Omitted/null Skill version selectors resolve the default version. Source deletion/default updates cannot change committed Session Skill contents. Deferred function discovery uses type-only tool_search and per-function defer_loading in the qualified single-agent Claude environment:none function profile, including qualified inline image messages and text results. Explicit web_search mode disabled and programmatic_tool_calling enabled false use frozen common Runtime controls. Enabled forms remain unqualified. Omitted programmatic configuration preserves native behavior, a documented difference from the official default-on behavior. Other combinations remain unqualified; see the operation coverage. +// @Description Supports inline configuration or a tenant-owned saved agent_id with per-Session field replacements. Execution supports model/instructions, text verbosity, non-deferred function tools, adapter-qualified multi_agent with persisted Subagent reads, implicit reasoning, service tier auto and environment type none, subject to the configured engine. Codex additionally supports HTTP MCP with explicit service origin, native allowed_tools and boolean required defaulting to false. Session vault_ids attach only project-owned Vaults; credential_id selects an attached static bearer credential for the exact HTTPS URL, while null/omission selects a unique match or remains anonymous. Ambiguous selection rejects creation. Frozen private selections never populate an omitted public credential_id; missing decryption configuration fails dispatch without anonymous fallback. Required initialization uses native startup before the first native Turn, including cold resume, and requires a separately advertised capability; exact hosted creation timing and error parity remain unverified. Other MCP origins and OAuth remain unsupported. The self_hosted profile requires Codex, an absolute workspace_directory and empty capability_directories, with optional non-deferred function tools and HTTP MCP using explicit service origin, optionally authenticated by the attached Vault rules. Remote MCP and remote Bearer authentication each require separately advertised combination support; old peers cannot receive unsupported work. Omitted/null capability_directories use the empty-list default; self_hosted requires configured execution plus executor registry. Claude SDK currently requires medium verbosity and object-root function schemas. It supports anonymous or attached static-bearer service-origin HTTP MCP on none with boolean required and separately advertised MCP/bearer/required runtime support. Required servers must be connected before the first native input is released; pending or failed startup rejects execution. The shared Vault selection and immutable binding rules apply; unsupported native labels/tool names reject before persistence. An attached Vault with no matching credential may remain anonymous; missing keys or failed credential lookup/decryption never fall back to anonymous execution. Omitted stream defaults to false; stream and agent_id cannot be null. Metadata may be null; non-string values and limit violations return invalid_request_error with a metadata or metadata. param. Hosted network policy rejections return invalid_request_error with a null param. Initial input accepts a string or ordered user-message array. Codex and Claude SDK on none and qualified openai_hosted also accept inline PNG/JPEG image content; other image combinations and remote URLs are unsupported. None initial input atomically starts a Turn; self_hosted initial input is reserved while returning its Environment connection target, with execution deferred to native readiness and Session failure on initial timeout. Initial input is required for none and for streamed creation outside self_hosted. Omitted/null input remains valid for non-streaming hosted and self_hosted creation. With stream=true, returns live Session events starting at creation; disconnect does not cancel execution. New Sessions retain their authenticated creator; all creation retries require the same typed subject, including across key rotation. Saved-Agent retries and inline requests using Vault attachments or credential references retain caller intent independently of later resource changes; unrelated inline retries preserve resolved/default equivalences. Unknown historical creators reject retries; known creators without recorded intent retain resolved-snapshot retry rules. These conflict policies are local and not verified hosted parity. Creation retries observe future events without replay; retry with stream=false to retrieve the Session. Claude SDK on none and Core-managed Docker openai_hosted supports qualified object-root json_schema output with medium verbosity, single-Agent execution and ordinary functions. Hosted execution reuses native workspace tools and Files/Artifacts; Skills, Plugins, capability directories, HTTP MCP, Subagent and tool_search combinations remain unqualified, including inherited template contents. Other non-text initial input remains unsupported. Basic Codex and Claude SDK openai_hosted creation requires an explicitly configured managed provider. The Claude workspace profile supports non-deferred function tools with text or successful inline PNG/JPEG results alongside native workspace tools; HTTP MCP remains unsupported. Idle Sessions provision automatically; initial provisioning has no caller connection action. Network defaults to enabled; disabled and restricted exact ASCII hostnames are supported. Restricted policy requires 1–100 allowed domains. Unsupported hostname forms and startup installations are rejected. Confidential env, system/npm/Python packages and ordered setup commands use the shared initialization lifecycle; requested network applies after setup. Initial inline and tenant-owned file_id files freeze encrypted bytes before provisioning, then install through the common Core lifecycle before native execution or live Files access. With a template reference, omitted/null files, env, packages and setup_commands inherit. Non-null files and command lists replace; env overlays by key; each package manager inherits on omission/null and otherwise replaces its list. Empty lists clear their selected field. Tenant-owned environment_template_id references inherit omitted/null network and allow only narrowing overrides. Inline hosted network:null retains the enabled default; updating a Template with network:null resets its saved policy to enabled. Core freezes effective configuration; template updates/deletion do not alter Session snapshots or same-intent creation retries. Inline or tenant-owned skill_reference Skills share initialization. Templates preserve default/latest/explicit selectors; Session creation freezes concrete metadata and encrypted content atomically. Skill, Plugin and capability-directory list omission/null inherit; a non-null list replaces, including empty-list clearing. Omitted/null Skill version selectors resolve the default version. Source deletion/default updates cannot change committed Session Skill contents. Deferred function discovery uses type-only tool_search and per-function defer_loading in the qualified single-agent Claude environment:none function profile, including qualified inline image messages and text results. Explicit web_search mode disabled and programmatic_tool_calling enabled false use frozen common Runtime controls. Enabled forms remain unqualified. Omitted programmatic configuration preserves native behavior, a documented difference from the official default-on behavior. Other combinations remain unqualified; see the operation coverage. // @Tags Sessions // @Accept json // @Produce json,text/event-stream @@ -138,16 +139,18 @@ func NewHandler(s ResourceStore, auth *Authenticator, engine string, options ... // @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.") + if !ok { + return + } + if writeFieldError(w, metadataTypeError(raw)) { + return + } var request decodedSessionRequest - decoder := json.NewDecoder(http.MaxBytesReader(w, r.Body, 16*1024*1024)) + decoder := json.NewDecoder(bytes.NewReader(raw)) decoder.DisallowUnknownFields() if err := decoder.Decode(&request); err != nil { - var tooLarge *http.MaxBytesError - if errors.As(err, &tooLarge) { - writeError(w, http.StatusRequestEntityTooLarge, "request_too_large", "Request exceeds 16 MiB.") - } else { - writeError(w, http.StatusBadRequest, "invalid_request", "Request must be a JSON object containing supported fields.") - } + 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 { @@ -156,7 +159,9 @@ func (h *Handler) createSession(w http.ResponseWriter, r *http.Request) { } input, err := request.validated() if err != nil { - writeError(w, http.StatusBadRequest, "invalid_request", "Request fields have invalid types or null values.") + if !writeFieldError(w, err) { + writeError(w, http.StatusBadRequest, "invalid_request", "Request fields have invalid types or null values.") + } return } key := r.Header.Get("Idempotency-Key") @@ -235,7 +240,9 @@ func (h *Handler) createSession(w http.ResponseWriter, r *http.Request) { if h.recoverSessionCreation(w, r, key, creationRequest, input.Stream) { return } - writeError(w, http.StatusBadRequest, "unsupported_or_invalid_configuration", err.Error()) + if !writeFieldError(w, err) { + writeError(w, http.StatusBadRequest, "unsupported_or_invalid_configuration", err.Error()) + } return } if input.Environment.Type == "self_hosted" && (h.inputs == nil || h.executorURL == "") { diff --git a/services/agents-api/internal/api/hosted_environment.go b/services/agents-api/internal/api/hosted_environment.go index ec6e6430e..b9318de8b 100644 --- a/services/agents-api/internal/api/hosted_environment.go +++ b/services/agents-api/internal/api/hosted_environment.go @@ -9,8 +9,13 @@ import ( "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" ) +// errNetworkPolicy reports a network policy outside the qualified forms. The +// official service rejects these with invalid_request_error and a null param. +var errNetworkPolicy = &fieldError{message: "network access must be enabled, disabled or restricted; restricted access requires 1–100 exact ASCII hostnames, and allowed_domains is only accepted with restricted access."} + // decodeHostedEnvironment keeps unsupported installations explicit, while // accepting the protocol's omitted/null/empty defaults for the basic profile. +// Unsupported fields are reported before network policy, independently of map order. func decodeHostedEnvironment(raw json.RawMessage) (*v1.Environment, error) { var fields map[string]json.RawMessage if json.Unmarshal(raw, &fields) != nil { @@ -25,14 +30,6 @@ func decodeHostedEnvironment(raw json.RawMessage) (*v1.Environment, error) { switch name { case "type": case "network": - if bytes.Equal(bytes.TrimSpace(value), []byte("null")) { - continue - } - var network v1.EnvironmentNetworkInput - if decodeInputObject(value, &network, "access", "allowed_domains") != nil || (agentnetwork.Policy{Access: network.Access, AllowedDomains: network.AllowedDomains}).Validate() != nil { - return nil, store.ErrInvalidInput - } - env.Network = &network case "files": files, err := decodeInitialFiles(value) if err != nil { @@ -54,6 +51,16 @@ func decodeHostedEnvironment(raw json.RawMessage) (*v1.Environment, error) { return nil, store.ErrInvalidInput } } + if value, supplied := fields["network"]; supplied && !bytes.Equal(bytes.TrimSpace(value), []byte("null")) { + var network v1.EnvironmentNetworkInput + if decodeInputObject(value, &network, "access", "allowed_domains") != nil { + return nil, store.ErrInvalidInput + } + if (agentnetwork.Policy{Access: network.Access, AllowedDomains: network.AllowedDomains}).Validate() != nil { + return nil, errNetworkPolicy + } + env.Network = &network + } return env, nil } diff --git a/services/agents-api/internal/api/saved_configuration.go b/services/agents-api/internal/api/saved_configuration.go index 2b5368305..32421a0fa 100644 --- a/services/agents-api/internal/api/saved_configuration.go +++ b/services/agents-api/internal/api/saved_configuration.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "errors" + "fmt" "slices" "unicode/utf8" @@ -20,12 +21,14 @@ func resolveSavedAgent(input v1.CreateAgentRequest) (store.CreateAgentInput, err // Update requests reuse field validation without requiring an omitted model. func resolveSavedFields(input v1.CreateAgentRequest) (store.CreateAgentInput, error) { - if input.Name != nil && utf8.RuneCountInString(*input.Name) > 128 { - return store.CreateAgentInput{}, errors.New("name must be at most 128 characters.") + if input.Name != nil { + if length := utf8.RuneCountInString(*input.Name); length > 128 { + return store.CreateAgentInput{}, &fieldError{param: "name", message: fmt.Sprintf("Invalid 'name': string too long. Expected a string with maximum length 128, but got a string with length %d instead.", length)} + } } metadata, err := stringMetadata(input.Metadata) if err != nil { - return store.CreateAgentInput{}, errors.New("metadata values must be strings.") + return store.CreateAgentInput{}, err } if err := validateMetadata(metadata); err != nil { return store.CreateAgentInput{}, err diff --git a/services/agents-api/internal/api/session_metadata.go b/services/agents-api/internal/api/session_metadata.go index 75b35a468..d73b96e73 100644 --- a/services/agents-api/internal/api/session_metadata.go +++ b/services/agents-api/internal/api/session_metadata.go @@ -1,17 +1,20 @@ package api import ( + "bytes" "encoding/json" - "errors" + "fmt" + "maps" "net/http" + "slices" + "strings" "unicode/utf8" - "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" "github.com/go-chi/chi/v5" ) // @Summary Update execution Session metadata -// @Description The metadata field is required in an update body. Send null or {} to clear it, or supply an object to replace all pairs. Up to 16 string pairs, with keys at most 64 characters and values at most 512 characters. Execution configuration and activity are unchanged. Returns the same safe Environment and pending-input activity projection as Session retrieval. +// @Description The metadata field is required in an update body. Send null or {} to clear it, or supply an object to replace all pairs. Up to 16 string pairs, with keys at most 64 characters and values at most 512 characters; violations and non-string values return invalid_request_error with a metadata or metadata. param. U+0000 is rejected as a local storage limit. Malformed, missing and foreign Session IDs share the not-found response. Execution configuration and activity are unchanged. Returns the same safe Environment and pending-input activity projection as Session retrieval. // @Tags Sessions // @Accept json // @Produce json @@ -27,6 +30,9 @@ func (h *Handler) updateSession(w http.ResponseWriter, r *http.Request) { if !ok { return } + if writeFieldError(w, metadataTypeError(raw)) { + return + } var request struct { Metadata json.RawMessage `json:"metadata"` } @@ -34,29 +40,26 @@ func (h *Handler) updateSession(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "invalid_request", "Request must be a JSON object containing supported fields.") return } - var session store.Session - var err error if len(request.Metadata) == 0 { writeError(w, http.StatusBadRequest, "invalid_request_error", "At least one update field is required") return - } else { - var values map[string]*string - if err := json.Unmarshal(request.Metadata, &values); err != nil { + } + var values map[string]*string + if err := json.Unmarshal(request.Metadata, &values); err != nil { + writeError(w, http.StatusBadRequest, "invalid_request", "metadata must be null or an object with string values.") + return + } + metadata, err := stringMetadata(values) + if err == nil { + err = validateMetadata(metadata) + } + if err != nil { + if !writeFieldError(w, err) { writeError(w, http.StatusBadRequest, "invalid_request", "metadata must be null or an object with string values.") - return - } - var metadata map[string]string - metadata, err = stringMetadata(values) - if err != nil { - writeError(w, http.StatusBadRequest, "invalid_request", "metadata values must be strings.") - return } - if err := validateMetadata(metadata); err != nil { - writeError(w, http.StatusBadRequest, "invalid_request", err.Error()) - return - } - session, err = h.store.UpdateSessionMetadata(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), metadata) + return } + session, err := h.store.UpdateSessionMetadata(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), metadata) if err != nil { writeStoreError(w, r, err) return @@ -64,24 +67,103 @@ func (h *Handler) updateSession(w http.ResponseWriter, r *http.Request) { h.respondSession(w, r, session) } +// 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. +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"])) + 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) + var value json.RawMessage + 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" { + return &fieldError{param: "metadata." + key, message: fmt.Sprintf("Invalid type for 'metadata.%s': expected a string, but got %s instead.", key, kind)} + } + } + return nil +} + +func jsonValueKind(value json.RawMessage) string { + value = bytes.TrimSpace(value) + if len(value) == 0 { + return "null" + } + switch value[0] { + case '"': + return "a string" + case '{': + return "an object" + case '[': + return "an array" + case 't', 'f': + return "a boolean" + case 'n': + return "null" + } + if bytes.ContainsAny(value, ".eE") { + return "a number" + } + return "an integer" +} + func stringMetadata(values map[string]*string) (map[string]string, error) { metadata := make(map[string]string, len(values)) - for key, value := range values { - if value == nil { - return nil, store.ErrInvalidInput + for _, key := range slices.Sorted(maps.Keys(values)) { + if values[key] == nil { + return nil, &fieldError{param: "metadata." + key, message: fmt.Sprintf("Invalid type for 'metadata.%s': expected a string, but got null instead.", key)} } - metadata[key] = *value + metadata[key] = *values[key] } return metadata, nil } +// validateMetadata applies the pinned pair and character limits, then the +// local U+0000 storage limit. Sorted keys keep repeated errors stable. func validateMetadata(metadata map[string]string) error { if len(metadata) > 16 { - return errors.New("metadata supports at most 16 pairs.") + return &fieldError{param: "metadata", message: fmt.Sprintf("Invalid 'metadata': too many properties. Expected an object with at most 16 properties, but got an object with %d properties instead.", len(metadata))} + } + for _, key := range slices.Sorted(maps.Keys(metadata)) { + if length := utf8.RuneCountInString(key); length > 64 { + return &fieldError{param: "metadata." + key, message: fmt.Sprintf("Invalid property name in 'metadata': '%s' is too long. Expected a string with maximum length 64, but got a string with length %d instead.", key, length)} + } + if length := utf8.RuneCountInString(metadata[key]); length > 512 { + return &fieldError{param: "metadata." + key, message: fmt.Sprintf("Invalid 'metadata.%s': string too long. Expected a string with maximum length 512, but got a string with length %d instead.", key, length)} + } } - for key, value := range metadata { - if utf8.RuneCountInString(key) > 64 || utf8.RuneCountInString(value) > 512 { - return errors.New("metadata keys must be at most 64 characters and values at most 512 characters.") + return metadataCharacterError(metadata) +} + +// metadataCharacterError rejects U+0000, which PostgreSQL text and jsonb cannot +// store. The official service accepts it; this is a documented local limit. +func metadataCharacterError(metadata map[string]string) error { + for _, key := range slices.Sorted(maps.Keys(metadata)) { + if strings.ContainsRune(key, 0) { + return &fieldError{param: "metadata." + key, message: fmt.Sprintf("Invalid property name in 'metadata': '%s' contains U+0000, which this service cannot store.", key)} + } + if strings.ContainsRune(metadata[key], 0) { + return &fieldError{param: "metadata." + key, message: fmt.Sprintf("Invalid 'metadata.%s': string contains U+0000, which this service cannot store.", key)} } } return nil diff --git a/services/agents-api/internal/api/session_request_test.go b/services/agents-api/internal/api/session_request_test.go index 73e739686..ae0f61f54 100644 --- a/services/agents-api/internal/api/session_request_test.go +++ b/services/agents-api/internal/api/session_request_test.go @@ -13,27 +13,29 @@ import ( func TestSessionCreateFieldPresence(t *testing.T) { // Pinned SessionCreateParams: stream and agent_id are not nullable; - // metadata is nullable, but its values must be strings. + // metadata is nullable, but its values must be strings. Invalid metadata + // values use the official code and a metadata. param. for _, tc := range []struct { name, fields string status int metadata map[string]string + param string }{ - {"omitted", ``, 201, nil}, - {"false stream", `,"stream":false`, 201, nil}, - {"null stream", `,"stream":null`, 400, nil}, - {"whitespace null stream", `,"stream": null `, 400, nil}, - {"string stream", `,"stream":"false"`, 400, nil}, - {"numeric stream", `,"stream":0`, 400, nil}, - {"null agent ID", `,"agent_id":null`, 400, nil}, - {"numeric agent ID", `,"agent_id":0`, 400, nil}, - {"null metadata", `,"metadata":null`, 201, nil}, - {"empty metadata", `,"metadata":{}`, 201, nil}, - {"string metadata", `,"metadata":{"empty":"","label":"中文🧪"}`, 201, map[string]string{"empty": "", "label": "中文🧪"}}, - {"null metadata value", `,"metadata":{"label":null}`, 400, nil}, - {"mixed metadata values", `,"metadata":{"empty":"","label":null}`, 400, nil}, - {"numeric metadata value", `,"metadata":{"label":0}`, 400, nil}, - {"array metadata", `,"metadata":[]`, 400, nil}, + {"omitted", ``, 201, nil, ""}, + {"false stream", `,"stream":false`, 201, nil, ""}, + {"null stream", `,"stream":null`, 400, nil, ""}, + {"whitespace null stream", `,"stream": null `, 400, nil, ""}, + {"string stream", `,"stream":"false"`, 400, nil, ""}, + {"numeric stream", `,"stream":0`, 400, nil, ""}, + {"null agent ID", `,"agent_id":null`, 400, nil, ""}, + {"numeric agent ID", `,"agent_id":0`, 400, nil, ""}, + {"null metadata", `,"metadata":null`, 201, nil, ""}, + {"empty metadata", `,"metadata":{}`, 201, nil, ""}, + {"string metadata", `,"metadata":{"empty":"","label":"中文🧪"}`, 201, map[string]string{"empty": "", "label": "中文🧪"}, ""}, + {"null metadata value", `,"metadata":{"label":null}`, 400, nil, "metadata.label"}, + {"mixed metadata values", `,"metadata":{"empty":"","label":null}`, 400, nil, "metadata.label"}, + {"numeric metadata value", `,"metadata":{"label":0}`, 400, nil, "metadata.label"}, + {"array metadata", `,"metadata":[]`, 400, nil, ""}, } { t.Run(tc.name, func(t *testing.T) { saved := &recordingStore{} @@ -52,7 +54,11 @@ func TestSessionCreateFieldPresence(t *testing.T) { t.Fatal("invalid request reached persistence") } var failure v1.ErrorResponse - if json.Unmarshal(response.Body.Bytes(), &failure) != nil || failure.Error.Code == nil || *failure.Error.Code != "invalid_request" { + code := "invalid_request" + if tc.param != "" { + code = "invalid_request_error" + } + if json.Unmarshal(response.Body.Bytes(), &failure) != nil || failure.Error.Code == nil || *failure.Error.Code != code || (tc.param == "") != (failure.Error.Param == nil) || tc.param != "" && *failure.Error.Param != tc.param { t.Fatalf("invalid error response: %s", response.Body) } return diff --git a/services/agents-api/internal/api/validation_errors_test.go b/services/agents-api/internal/api/validation_errors_test.go new file mode 100644 index 000000000..28b713c93 --- /dev/null +++ b/services/agents-api/internal/api/validation_errors_test.go @@ -0,0 +1,275 @@ +package api + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + v1 "github.com/MiniMax-AI-Dev/parsar/contracts/agents-api/v1" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgconn" +) + +// validationStore counts every persistence attempt so rejected requests can +// prove that validation ran before any write. +type validationStore struct { + ResourceStore + writes int +} + +func (s *validationStore) CreateAgent(_ context.Context, tenant string, input store.CreateAgentInput) (store.SavedAgent, error) { + s.writes++ + return store.SavedAgent{ID: uuid.NewString(), TenantID: tenant, Configuration: input.Configuration, Metadata: input.Metadata, CreatedAt: time.Unix(1700000000, 0), UpdatedAt: time.Unix(1700000000, 0)}, nil +} + +func (s *validationStore) UpdateAgent(_ context.Context, tenant, id string, input store.UpdateAgentInput) (store.SavedAgent, error) { + s.writes++ + return store.SavedAgent{ID: id, TenantID: tenant, Configuration: json.RawMessage(`{"model":"validation-model"}`), Metadata: map[string]string{}}, nil +} + +func (s *validationStore) CreateVault(_ context.Context, tenant string, input store.CreateVaultInput) (store.Vault, error) { + s.writes++ + return store.Vault{ID: uuid.NewString(), TenantID: tenant, Name: input.Name, Metadata: input.Metadata}, nil +} + +func (s *validationStore) UpdateSessionMetadata(_ context.Context, tenant, id string, metadata map[string]string) (store.Session, error) { + s.writes++ + return store.Session{ID: id, TenantID: tenant, Metadata: metadata, Configuration: json.RawMessage(`{"agent":{"id":"agent_validation","model":"validation-model"},"environment":{"type":"none"}}`)}, nil +} + +func (s *validationStore) CreateSession(_ context.Context, tenant string, input store.CreateSessionInput) (store.Session, error) { + s.writes++ + return store.Session{ID: uuid.NewString(), TenantID: tenant, Metadata: input.Metadata, Configuration: input.Configuration}, nil +} + +func (s *validationStore) CreateEnvironmentTemplate(context.Context, string, store.EnvironmentTemplateInput) (store.EnvironmentTemplate, error) { + s.writes++ + return store.EnvironmentTemplate{ID: uuid.NewString(), NetworkAccess: "enabled"}, nil +} + +func (s *validationStore) UpdateEnvironmentTemplate(_ context.Context, _, id string, input store.EnvironmentTemplateInput) (store.EnvironmentTemplate, error) { + s.writes++ + return store.EnvironmentTemplate{ID: id, NetworkAccess: input.NetworkAccess, AllowedDomains: input.AllowedDomains}, nil +} + +func validationHandler(t *testing.T) (http.Handler, *validationStore) { + t.Helper() + s := &validationStore{} + h, recording, _ := testHandler(t, WithExecution(&inputRecorder{ResourceStore: s})) + recording.ResourceStore = s + return h, s +} + +func errorFields(t *testing.T, w *httptest.ResponseRecorder) (string, *string, string) { + t.Helper() + var response struct { + Error struct { + Type, Message string + Code string + Param *string + } + } + if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil { + t.Fatalf("invalid error body %d %s", w.Code, w.Body) + } + if response.Error.Type != "invalid_request_error" { + t.Fatalf("error type = %s", w.Body) + } + return response.Error.Code, response.Error.Param, response.Error.Message +} + +func TestMetadataValidationUsesOfficialFields(t *testing.T) { + seventeen := map[string]string{} + sixteen := map[string]string{} + for i := range 17 { + seventeen[fmt.Sprintf("k%02d", i)] = "v" + if i < 16 { + sixteen[fmt.Sprintf("k%02d", i)] = "v" + } + } + encode := func(value any) string { + raw, err := json.Marshal(value) + if err != nil { + t.Fatal(err) + } + return string(raw) + } + key65, key64 := strings.Repeat("K", 65), strings.Repeat("雪", 64) + operations := []struct { + name, method, path, body string + limited bool + }{ + {"agent create", http.MethodPost, "/v1/agents", `{"model":"validation-model","metadata":%s}`, true}, + {"agent update", http.MethodPost, "/v1/agents/" + uuid.NewString(), `{"metadata":%s}`, true}, + {"session create", http.MethodPost, "/v1/agents/sessions", `{"agent":{"model":"validation-model"},"environment":{"type":"none"},"input":"Validate metadata.","metadata":%s}`, true}, + {"session update", http.MethodPost, "/v1/agents/sessions/" + uuid.NewString(), `{"metadata":%s}`, true}, + {"vault create", http.MethodPost, "/v1/vaults", `{"metadata":%s}`, false}, + } + // limit marks pinned count/length limits, which Vaults do not apply (M5). + rejected := []struct { + name, metadata, param, message string + limit bool + }{ + {"too many pairs", encode(seventeen), "metadata", "Invalid 'metadata': too many properties. Expected an object with at most 16 properties, but got an object with 17 properties instead.", true}, + {"long key", `{"` + key65 + `":"v"}`, "metadata." + key65, "Invalid property name in 'metadata': '" + key65 + "' is too long. Expected a string with maximum length 64, but got a string with length 65 instead.", true}, + {"long value", `{"k":"` + strings.Repeat("雪", 513) + `"}`, "metadata.k", "Invalid 'metadata.k': string too long. Expected a string with maximum length 512, but got a string with length 513 instead.", true}, + {"integer", `{"k":1}`, "metadata.k", "Invalid type for 'metadata.k': expected a string, but got an integer instead.", false}, + {"number", `{"k":1.5}`, "metadata.k", "Invalid type for 'metadata.k': expected a string, but got a number instead.", false}, + {"boolean", `{"k":false}`, "metadata.k", "Invalid type for 'metadata.k': expected a string, but got a boolean instead.", false}, + {"object", `{"k":{"nested":"v"}}`, "metadata.k", "Invalid type for 'metadata.k': expected a string, but got an object instead.", false}, + {"array", `{"k":["v"]}`, "metadata.k", "Invalid type for 'metadata.k': expected a string, but got an array instead.", false}, + {"null", `{"a":null}`, "metadata.a", "Invalid type for 'metadata.a': expected a string, but got null instead.", false}, + {"document order", `{"z":"v","b":null,"a":1}`, "metadata.b", "Invalid type for 'metadata.b': expected a string, but got null instead.", false}, + {"type before count", `{"k00":"v","k01":"v","k02":"v","k03":"v","k04":"v","k05":"v","k06":"v","k07":"v","k08":"v","k09":"v","k10":"v","k11":"v","k12":"v","k13":"v","k14":"v","k15":"v","k16":2}`, "metadata.k16", "Invalid type for 'metadata.k16': expected a string, but got an integer instead.", false}, + {"null character value", `{"k":"a\u0000b"}`, "metadata.k", "Invalid 'metadata.k': string contains U+0000, which this service cannot store.", false}, + {"null character key", `{"a\u0000b":"v"}`, "metadata.a\x00b", "Invalid property name in 'metadata': 'a\x00b' contains U+0000, which this service cannot store.", false}, + } + accepted := []struct{ name, metadata string }{ + {"boundary pairs", encode(sixteen)}, + {"boundary key", `{"` + key64 + `":"v"}`}, + {"boundary value", `{"k":"` + strings.Repeat("雪", 512) + `"}`}, + {"empty", `{}`}, + {"null", `null`}, + } + for _, op := range operations { + t.Run(op.name, func(t *testing.T) { + h, s := validationHandler(t) + for _, tc := range rejected { + w := credentialRequest(h, op.method, op.path, fmt.Sprintf(op.body, tc.metadata)) + if tc.limit && !op.limited { + // Vault metadata keeps only its storage bound (M5). + if w.Code != http.StatusCreated { + t.Fatalf("%s: Vault metadata limit changed: %d %s", tc.name, w.Code, w.Body) + } + continue + } + code, param, message := errorFields(t, w) + if w.Code != http.StatusBadRequest || code != "invalid_request_error" || param == nil || *param != tc.param || message != tc.message { + t.Fatalf("%s: %d %s", tc.name, w.Code, w.Body) + } + } + if want := map[bool]int{true: 0, false: 3}[op.limited]; s.writes != want { + t.Fatalf("rejected metadata reached storage: %d writes", s.writes) + } + // Type errors precede the generic whole-body error. + body := strings.Replace(fmt.Sprintf(op.body, `{"k":1}`), "{", `{"unsupported_field":true,`, 1) + w := credentialRequest(h, op.method, op.path, body) + if code, param, _ := errorFields(t, w); w.Code != http.StatusBadRequest || code != "invalid_request_error" || param == nil || *param != "metadata.k" { + t.Fatalf("metadata type did not precede generic error: %d %s", w.Code, w.Body) + } + for _, tc := range accepted { + before := s.writes + w := credentialRequest(h, op.method, op.path, fmt.Sprintf(op.body, tc.metadata)) + if w.Code >= 300 || s.writes != before+1 { + t.Fatalf("%s rejected: %d %s", tc.name, w.Code, w.Body) + } + } + }) + } +} + +func TestAgentNameLengthUsesOfficialFields(t *testing.T) { + h, s := validationHandler(t) + for _, path := range []string{"/v1/agents", "/v1/agents/" + uuid.NewString()} { + for _, name := range []string{strings.Repeat("n", 129), strings.Repeat("雪", 129)} { + w := credentialRequest(h, http.MethodPost, path, `{"model":"validation-model","name":"`+name+`"}`) + code, param, message := errorFields(t, w) + if w.Code != http.StatusBadRequest || code != "invalid_request_error" || param == nil || *param != "name" || message != "Invalid 'name': string too long. Expected a string with maximum length 128, but got a string with length 129 instead." { + t.Fatalf("%s: %d %s", path, w.Code, w.Body) + } + } + if s.writes != 0 { + t.Fatal("rejected name reached storage") + } + for _, name := range []string{strings.Repeat("雪", 128), "", " untrimmed "} { + before := s.writes + w := credentialRequest(h, http.MethodPost, path, `{"model":"validation-model","name":"`+name+`"}`) + if w.Code >= 300 || s.writes != before+1 { + t.Fatalf("name %q rejected: %d %s", name, w.Code, w.Body) + } + } + s.writes = 0 + } +} + +func TestTemplateNetworkRejectionsUseOfficialCode(t *testing.T) { + h, s := validationHandler(t) + domains := make([]string, 101) + for i := range domains { + domains[i] = fmt.Sprintf("d%d.example.com", i) + } + many, err := json.Marshal(domains) + if err != nil { + t.Fatal(err) + } + rejected := []string{ + `{"access":"restricted","allowed_domains":["*.example.com"]}`, + `{"access":"restricted","allowed_domains":["example.com:443"]}`, + `{"access":"restricted","allowed_domains":["https://example.com"]}`, + `{"access":"restricted","allowed_domains":["2001:db8::1"]}`, + `{"access":"restricted","allowed_domains":[""]}`, + `{"access":"restricted","allowed_domains":[]}`, + `{"access":"restricted","allowed_domains":null}`, + `{"access":"restricted","allowed_domains":` + string(many) + `}`, + `{"access":"enabled","allowed_domains":["example.com"]}`, + } + template := uuid.NewString() + for _, network := range rejected { + for _, request := range []struct{ path, body string }{ + {"/v1/agents/environments/templates", `{"name":"network","network":` + network + `}`}, + {"/v1/agents/environments/templates/" + template, `{"network":` + network + `}`}, + {"/v1/agents/sessions", `{"agent":{"model":"validation-model"},"environment":{"type":"openai_hosted","network":` + network + `}}`}, + } { + w := credentialRequest(h, http.MethodPost, request.path, request.body) + code, param, message := errorFields(t, w) + if w.Code != http.StatusBadRequest || code != "invalid_request_error" || param != nil || message != errNetworkPolicy.message { + t.Fatalf("%s %s: %d %s", request.path, network, w.Code, w.Body) + } + } + } + if s.writes != 0 { + t.Fatal("rejected network reached storage") + } + // Other unsupported template installation fields keep their local code. + for _, body := range []string{`{"unsupported":true,"network":{"access":"restricted","allowed_domains":["*.example.com"]}}`, `{"network":{"access":"restricted","allowed_domains":"example.com"}}`, `{"packages":{"cargo":["x"]}}`} { + w := credentialRequest(h, http.MethodPost, "/v1/agents/environments/templates", body) + if code, _, _ := errorFields(t, w); w.Code != http.StatusBadRequest || code != "unsupported_or_invalid_configuration" { + t.Fatalf("%s: %d %s", body, w.Code, w.Body) + } + } + // The hostname grammar is unchanged. + for _, network := range []string{`{"access":"restricted","allowed_domains":["Example.COM","localhost","example.com","example.com"]}`, `{"access":"disabled"}`, `{"access":"enabled"}`, `null`} { + before := s.writes + w := credentialRequest(h, http.MethodPost, "/v1/agents/environments/templates/"+template, `{"network":`+network+`}`) + if w.Code != http.StatusOK || s.writes != before+1 { + t.Fatalf("%s rejected: %d %s", network, w.Code, w.Body) + } + } +} + +func TestUnstorableTextMapsToInvalidRequest(t *testing.T) { + for _, tc := range []struct { + err error + status int + }{ + {fmt.Errorf("create agent: %w", &pgconn.PgError{Code: "22P05"}), http.StatusBadRequest}, + {fmt.Errorf("create vault: %w", &pgconn.PgError{Code: "22021"}), http.StatusBadRequest}, + {&pgconn.PgError{Code: "23505"}, http.StatusInternalServerError}, + } { + w := httptest.NewRecorder() + writeStoreError(w, httptest.NewRequest(http.MethodPost, "/v1/agents", nil), tc.err) + var response v1.ErrorResponse + if w.Code != tc.status || json.Unmarshal(w.Body.Bytes(), &response) != nil || response.Error.Param != nil { + t.Fatalf("%v: %d %s", tc.err, w.Code, w.Body) + } + if tc.status == http.StatusBadRequest && (response.Error.Code == nil || *response.Error.Code != "invalid_request_error" || response.Error.Type != "invalid_request_error" || response.Error.Message != unstorableTextMessage) { + t.Fatalf("%v: %s", tc.err, w.Body) + } + } +} diff --git a/services/agents-api/internal/api/vaults.go b/services/agents-api/internal/api/vaults.go index 27b129686..217a2255d 100644 --- a/services/agents-api/internal/api/vaults.go +++ b/services/agents-api/internal/api/vaults.go @@ -21,7 +21,7 @@ type VaultStore interface { } // @Summary Create a Vault -// @Description Creates a project-owned Vault independently of execution. Omitted name stays null; a supplied string is trimmed and must contain 1–256 UTF-8 bytes. Explicit null name is invalid. Omitted/null metadata becomes an empty object; values must be strings. Metadata has a local 64 KiB encoded storage bound. Credentials, Session binding and hosted error/retry parity remain incomplete. +// @Description Creates a project-owned Vault independently of execution. Omitted name stays null; a supplied string is trimmed and must contain 1–256 UTF-8 bytes. Explicit null name is invalid. Omitted/null metadata becomes an empty object; non-string values return invalid_request_error with a metadata. param. Metadata has a local 64 KiB encoded storage bound. U+0000 in stored strings is rejected as a local storage limit. Credentials, Session binding and hosted error/retry parity remain incomplete. // @Tags Vaults // @Accept json // @Produce json @@ -36,6 +36,9 @@ func (h *Handler) createVault(w http.ResponseWriter, r *http.Request) { if !ok { return } + if writeFieldError(w, metadataTypeError(raw)) { + return + } var request struct { Name json.RawMessage `json:"name"` Metadata map[string]*string `json:"metadata"` @@ -58,10 +61,16 @@ func (h *Handler) createVault(w http.ResponseWriter, r *http.Request) { } input.Name = &trimmed } + // Vault metadata has no pair or character limits, only the storage limits. var err error input.Metadata, err = stringMetadata(request.Metadata) + if err == nil { + err = metadataCharacterError(input.Metadata) + } if err != nil { - writeError(w, http.StatusBadRequest, "invalid_request", "metadata values must be strings.") + if !writeFieldError(w, err) { + writeError(w, http.StatusBadRequest, "invalid_request", "metadata values must be strings.") + } return } vault, err := h.store.CreateVault(r.Context(), tenantID(r), input) diff --git a/services/agents-api/internal/api/vaults_test.go b/services/agents-api/internal/api/vaults_test.go index 2bfa12193..e3a5d01f5 100644 --- a/services/agents-api/internal/api/vaults_test.go +++ b/services/agents-api/internal/api/vaults_test.go @@ -38,6 +38,12 @@ func (f *vaultResourceFixture) GetVault(_ context.Context, tenant, id string) (s return f.vault, f.err } +// Vaults are not Agents: /v1/agents/vaults updates an unknown Agent ID, which +// resolves as a missing Agent after body validation. +func (f *vaultResourceFixture) UpdateAgent(context.Context, string, string, store.UpdateAgentInput) (store.SavedAgent, error) { + return store.SavedAgent{}, store.ErrNotFound +} + func vaultResourceHandler(t *testing.T) (http.Handler, *vaultResourceFixture) { t.Helper() f := &vaultResourceFixture{vault: store.Vault{ID: uuid.NewString(), TenantID: uuid.NewString(), Metadata: map[string]string{}, CreatedAt: time.Unix(1700000000, 0)}} diff --git a/services/agents-api/internal/store/environment_templates.go b/services/agents-api/internal/store/environment_templates.go index 08d363b0a..6d006b07a 100644 --- a/services/agents-api/internal/store/environment_templates.go +++ b/services/agents-api/internal/store/environment_templates.go @@ -130,10 +130,8 @@ func (s *Store) UpdateEnvironmentTemplate(ctx context.Context, tenantID, templat if err != nil { return EnvironmentTemplate{}, err } - id, err := parseID(templateID) - if err != nil { - return EnvironmentTemplate{}, ErrNotFound - } + // Sealing runs before the update lookup, so a malformed ID must take the same path. + id := parsePathID(templateID) var name pgtype.Text if in.Name != nil { name = pgtype.Text{String: *in.Name, Valid: true} diff --git a/services/agents-api/internal/store/environments.go b/services/agents-api/internal/store/environments.go index cf9649206..761aad8f9 100644 --- a/services/agents-api/internal/store/environments.go +++ b/services/agents-api/internal/store/environments.go @@ -51,10 +51,7 @@ func (s *Store) GetEnvironment(ctx context.Context, tenantID, environmentID stri if err != nil { return Environment{}, err } - id, err := parseID(environmentID) - if err != nil { - return Environment{}, err - } + id := parsePathID(environmentID) row, err := s.queries.GetEnvironment(ctx, sqlc.GetEnvironmentParams{TenantID: tenant, ID: id}) return environmentFromRow(row.Environment, row.TenantID, row.Configuration, err) } diff --git a/services/agents-api/internal/store/export_test.go b/services/agents-api/internal/store/export_test.go index 3cdda8e9c..a592b4b89 100644 --- a/services/agents-api/internal/store/export_test.go +++ b/services/agents-api/internal/store/export_test.go @@ -22,3 +22,6 @@ func FixtureExecutorPrincipal(t *testing.T, s *Store, tenant string) identity.Pr } return p } + +// SkillArchive builds a minimal valid Skill archive for public HTTP fixtures. +func SkillArchive(t *testing.T, marker string) []byte { return skillArchive(t, marker) } diff --git a/services/agents-api/internal/store/path_id_semantics_public_test.go b/services/agents-api/internal/store/path_id_semantics_public_test.go new file mode 100644 index 000000000..599b5ece4 --- /dev/null +++ b/services/agents-api/internal/store/path_id_semantics_public_test.go @@ -0,0 +1,427 @@ +package store_test + +import ( + "bytes" + "encoding/json" + "io" + "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" + "github.com/jackc/pgx/v5/pgxpool" +) + +type pathIDClient struct { + t *testing.T + server *httptest.Server +} + +func (c pathIDClient) do(token, method, path, contentType string, body []byte) (int, string) { + c.t.Helper() + request, err := http.NewRequest(method, c.server.URL+path, bytes.NewReader(body)) + if err != nil { + c.t.Fatal(err) + } + request.Header.Set("Authorization", "Bearer "+token) + request.Header.Set("OpenAI-Beta", "agents=v1") + if contentType != "" { + request.Header.Set("Content-Type", contentType) + } + response, err := http.DefaultClient.Do(request) + if err != nil { + c.t.Fatal(err) + } + defer response.Body.Close() + raw, err := io.ReadAll(response.Body) + if err != nil { + c.t.Fatal(err) + } + return response.StatusCode, string(raw) +} + +func (c pathIDClient) created(token, path, body string) string { + c.t.Helper() + status, raw := c.do(token, http.MethodPost, path, "application/json", []byte(body)) + var value struct{ ID string } + if (status != http.StatusOK && status != http.StatusCreated) || json.Unmarshal([]byte(raw), &value) != nil || value.ID == "" { + c.t.Fatalf("fixture %s: %d %s", path, status, raw) + } + return value.ID +} + +// databaseDigest detects any row insertion, update or deletion in public tables. +func databaseDigest(t *testing.T, pool *pgxpool.Pool) map[string]string { + t.Helper() + rows, err := pool.Query(t.Context(), `SELECT table_name FROM information_schema.tables WHERE table_schema = 'public' AND table_type = 'BASE TABLE' ORDER BY table_name`) + if err != nil { + t.Fatal(err) + } + var tables []string + for rows.Next() { + var table string + if err := rows.Scan(&table); err != nil { + t.Fatal(err) + } + tables = append(tables, table) + } + rows.Close() + digest := make(map[string]string, len(tables)) + for _, table := range tables { + var value string + query := `SELECT count(*)::text || ':' || coalesce(md5(string_agg(t::text, ',' ORDER BY t::text)), '') FROM "` + table + `" t` + if err := pool.QueryRow(t.Context(), query).Scan(&value); err != nil { + t.Fatal(table, err) + } + digest[table] = value + } + return digest +} + +func TestMalformedPathIDsMatchMissingPostgres(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{61}, 32)) + if err != nil { + t.Fatal(err) + } + s := store.NewWithCredentialCipher(pool, cipher) + owner, foreign := uuid.NewString(), uuid.NewString() + ownerTenant := uuid.NewString() + auth, err := api.NewAuthenticator([]api.APIKey{ + {OrganizationID: "test-org", ProjectID: uuid.NewString(), SubjectKind: "service_account", SubjectID: "path-owner", TokenSHA256: device.HashCredential(owner), TenantID: ownerTenant}, + {OrganizationID: "test-org", ProjectID: uuid.NewString(), SubjectKind: "service_account", SubjectID: "path-foreign", TokenSHA256: device.HashCredential(foreign), TenantID: uuid.NewString()}, + }) + if err != nil { + t.Fatal(err) + } + h, err := api.NewHandler(s, auth, "codex", api.WithExecution(s), api.WithSubagents(s), api.WithSkills(s), api.WithSourceFiles(s), api.WithSessionArtifacts(s)) + if err != nil { + t.Fatal(err) + } + server := httptest.NewServer(h) + defer server.Close() + client := pathIDClient{t: t, server: server} + + vault := client.created(owner, "/v1/vaults", `{"name":"path-owner"}`) + credential := client.created(owner, "/v1/vaults/"+vault+"/credentials", `{"name":"path","auth":{"type":"static_bearer","mcp_server_url":"https://mcp.example/mcp","token":"path-token"}}`) + agent := client.created(owner, "/v1/agents", `{"model":"path-model"}`) + template := client.created(owner, "/v1/agents/environments/templates", `{"name":"path-template"}`) + session := client.created(owner, "/v1/agents/sessions", `{"agent":{"model":"path-model"},"environment":{"type":"none"},"input":"Keep this Session."}`) + status, raw := client.do(owner, http.MethodGet, "/v1/agents/sessions/"+session+"/turns", "", nil) + var turns struct{ Data []struct{ ID string } } + if status != http.StatusOK || json.Unmarshal([]byte(raw), &turns) != nil || len(turns.Data) != 1 { + t.Fatalf("fixture Turn: %d %s", status, raw) + } + turn := turns.Data[0].ID + hosted, err := s.CreateSession(t.Context(), ownerTenant, store.CreateSessionInput{Creator: store.FixtureCreator(), Engine: "codex", IdempotencyKey: "path-environment", + Configuration: json.RawMessage(`{"agent":{"model":"path-model"},"environment":{"type":"self_hosted","workspace_directory":"/workspace","capability_directories":[]}}`)}) + if err != nil || hosted.Environment == nil { + t.Fatal("fixture Environment", err) + } + environment := hosted.Environment.ID + skill, err := s.CreateSkill(t.Context(), ownerTenant, store.SkillArchive(t, "path-skill")) + if err != nil { + t.Fatal(err) + } + var upload bytes.Buffer + form := multipart.NewWriter(&upload) + if err := form.WriteField("purpose", "user_data"); err != nil { + t.Fatal(err) + } + part, err := form.CreateFormFile("file", "path.txt") + if err != nil { + t.Fatal(err) + } + if _, err := part.Write([]byte("path file")); err != nil { + t.Fatal(err) + } + if err := form.Close(); err != nil { + t.Fatal(err) + } + status, raw = client.do(owner, http.MethodPost, "/v1/files", form.FormDataContentType(), upload.Bytes()) + var file struct{ ID string } + if status != http.StatusOK || json.Unmarshal([]byte(raw), &file) != nil || file.ID == "" { + t.Fatalf("fixture File: %d %s", status, raw) + } + + // Each route lists its path segments. Every owned segment exists, so a + // replaced segment is the first lookup that can fail on that route. + type route struct { + method, body string + segments []string + query string + } + missing := uuid.NewString() + routes := []route{ + {"GET", "", []string{"/v1/vaults/", vault}, ""}, + {"DELETE", "", []string{"/v1/vaults/", vault}, ""}, + {"GET", "", []string{"/v1/vaults/", vault, "/credentials"}, ""}, + {"POST", `{"name":"path","auth":{"type":"static_bearer","mcp_server_url":"https://mcp.example/mcp","token":"t"}}`, []string{"/v1/vaults/", vault, "/credentials"}, ""}, + {"GET", "", []string{"/v1/vaults/", vault, "/credentials/", credential}, ""}, + {"POST", `{"auth":{"type":"static_bearer","token":"t"}}`, []string{"/v1/vaults/", vault, "/credentials/", credential}, ""}, + {"DELETE", "", []string{"/v1/vaults/", vault, "/credentials/", credential}, ""}, + {"GET", "", []string{"/v1/agents/", agent}, ""}, + {"POST", `{}`, []string{"/v1/agents/", agent}, ""}, + {"DELETE", "", []string{"/v1/agents/", agent}, ""}, + {"GET", "", []string{"/v1/agents/environments/templates/", template}, ""}, + {"POST", `{}`, []string{"/v1/agents/environments/templates/", template}, ""}, + {"DELETE", "", []string{"/v1/agents/environments/templates/", template}, ""}, + {"GET", "", []string{"/v1/agents/environments/", environment}, ""}, + {"GET", "", []string{"/v1/agents/environments/", environment, "/files"}, ""}, + {"POST", `{}`, []string{"/v1/agents/environments/", environment, "/files"}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session}, ""}, + {"POST", `{"metadata":{}}`, []string{"/v1/agents/sessions/", session}, ""}, + {"DELETE", "", []string{"/v1/agents/sessions/", session}, ""}, + {"POST", `{"events":[]}`, []string{"/v1/agents/sessions/", session, "/events"}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/events"}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/items"}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/turns"}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/turns/", turn}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/subagents"}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/subagents/", missing}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/subagents/", missing, "/items"}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/subagents/", missing, "/turns"}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/subagents/", missing, "/turns/", missing}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/subagents/", missing, "/turns/", missing, "/items"}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/artifacts"}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/artifacts/", missing}, ""}, + {"GET", "", []string{"/v1/agents/sessions/", session, "/artifacts/", missing, "/content"}, ""}, + {"DELETE", "", []string{"/v1/agents/sessions/", session, "/artifacts/", missing}, ""}, + {"GET", "", []string{"/v1/files/", file.ID}, ""}, + {"GET", "", []string{"/v1/files/", file.ID, "/content"}, ""}, + {"DELETE", "", []string{"/v1/files/", file.ID}, ""}, + {"GET", "", []string{"/v1/skills/", skill.ID}, ""}, + {"POST", `{"default_version":"1"}`, []string{"/v1/skills/", skill.ID}, ""}, + {"DELETE", "", []string{"/v1/skills/", skill.ID}, ""}, + {"GET", "", []string{"/v1/skills/", skill.ID, "/content"}, ""}, + {"GET", "", []string{"/v1/skills/", skill.ID, "/versions"}, ""}, + {"GET", "", []string{"/v1/skills/", skill.ID, "/versions/", "1"}, ""}, + {"DELETE", "", []string{"/v1/skills/", skill.ID, "/versions/", "1"}, ""}, + {"GET", "", []string{"/v1/skills/", skill.ID, "/versions/", "1", "/content"}, ""}, + } + wellFormedMissing := func(owned string) string { + switch { + case strings.HasPrefix(owned, "file-"): + return "file-" + uuid.NewString() + case strings.HasPrefix(owned, "skill_"): + return "skill_" + uuid.NewString() + case owned == "1": + return "999" + } + return uuid.NewString() + } + malformed := func(owned string) []string { + values := []string{"not-a-uuid", "sess_0123456789abcdef0123456789abcdef", "00000000-0000-0000-0000-000000000000", strings.ToUpper(owned) + "0"} + switch { + case strings.HasPrefix(owned, "file-"), strings.HasPrefix(owned, "skill_"): + values = append(values, strings.TrimPrefix(strings.TrimPrefix(owned, "file-"), "skill_"), owned+"x") + case owned == "1": + values = []string{"abc", "0", "-1", "01", "1.0", "latest"} + } + return values + } + path := func(segments []string, index int, value string) string { + parts := append([]string{}, segments...) + parts[index] = value + return strings.Join(parts, "") + } + contentType := func(body string) string { + if body == "" { + return "" + } + return "application/json" + } + before := databaseDigest(t, pool) + checked := 0 + for _, r := range routes { + for index := 1; index < len(r.segments); index += 2 { + owned := r.segments[index] + wantStatus, wantBody := client.do(owner, r.method, path(r.segments, index, wellFormedMissing(owned)), contentType(r.body), []byte(r.body)) + if wantStatus != http.StatusNotFound { + t.Fatalf("%s %s missing segment %d: %d %s", r.method, strings.Join(r.segments, ""), index, wantStatus, wantBody) + } + for _, value := range malformed(owned) { + status, body := client.do(owner, r.method, path(r.segments, index, value), contentType(r.body), []byte(r.body)) + if status != wantStatus || body != wantBody { + t.Errorf("%s %s: malformed %q = %d %s; missing = %d %s", r.method, path(r.segments, index, "{id}"), value, status, body, wantStatus, wantBody) + } + checked++ + } + } + // A foreign tenant cannot distinguish the owner's complete path from a missing one. + wantStatus, wantBody := client.do(foreign, r.method, path(r.segments, 1, wellFormedMissing(r.segments[1])), contentType(r.body), []byte(r.body)) + status, body := client.do(foreign, r.method, strings.Join(r.segments, ""), contentType(r.body), []byte(r.body)) + malformedStatus, malformedBody := client.do(foreign, r.method, path(r.segments, 1, "not-a-uuid"), contentType(r.body), []byte(r.body)) + if wantStatus != http.StatusNotFound || status != wantStatus || body != wantBody || malformedStatus != wantStatus || malformedBody != wantBody { + t.Errorf("%s %s: foreign %d %s, malformed %d %s; missing %d %s", r.method, strings.Join(r.segments, ""), status, body, malformedStatus, malformedBody, wantStatus, wantBody) + } + checked++ + } + // Equality also holds when the request itself is invalid: body and query + // validation run exactly as for a well-formed missing identifier. + compare := func(r route, target, contentType string, body []byte) { + t.Helper() + for index := 1; index < len(r.segments); index += 2 { + owned := r.segments[index] + wantStatus, wantBody := client.do(target, r.method, path(r.segments, index, wellFormedMissing(owned))+r.query, contentType, body) + for _, value := range malformed(owned) { + status, got := client.do(target, r.method, path(r.segments, index, value)+r.query, contentType, body) + if status != wantStatus || got != wantBody { + t.Errorf("%s %s%s %s: malformed %q = %d %s; missing = %d %s", r.method, path(r.segments, index, "{id}"), r.query, body, value, status, got, wantStatus, wantBody) + } + checked++ + } + } + } + for _, r := range routes { + switch r.method { + case http.MethodPost: + for _, body := range []string{`{"unsupported_field":true}`, `[]`, `not json`} { + compare(r, owner, "application/json", []byte(body)) + } + case http.MethodDelete: + compare(r, owner, "application/json", []byte(`{"unexpected":true}`)) + default: + for _, query := range []string{"?limit=abc", "?order=sideways", "?limit=101&after=not-a-uuid"} { + r.query = query + compare(r, owner, "", nil) + } + } + } + for _, r := range []route{ + {method: "POST", body: `{"name":" ","auth":{"type":"static_bearer","mcp_server_url":"https://mcp.example/mcp","token":"t"}}`, segments: []string{"/v1/vaults/", vault, "/credentials"}}, + {method: "POST", body: `{"name":"path","auth":{"type":"static_bearer","mcp_server_url":"http://mcp.example/mcp","token":"t"}}`, segments: []string{"/v1/vaults/", vault, "/credentials"}}, + {method: "POST", body: `{"auth":{"type":"static_bearer","token":""}}`, segments: []string{"/v1/vaults/", vault, "/credentials/", credential}}, + {method: "POST", body: `{"auth":{"type":"mcp_oauth"}}`, segments: []string{"/v1/vaults/", vault, "/credentials/", credential}}, + {method: "POST", body: `{"name":"` + strings.Repeat("n", 129) + `"}`, segments: []string{"/v1/agents/", agent}}, + {method: "POST", body: `{"metadata":{"k":1}}`, segments: []string{"/v1/agents/", agent}}, + {method: "POST", body: `{"instructions":"` + strings.Repeat("x", 600*1024) + `"}`, segments: []string{"/v1/agents/", agent}}, + {method: "POST", body: `{"events":[{"type":"agent.session.input.message","input":"Admit this input."}]}`, segments: []string{"/v1/agents/sessions/", session, "/events"}}, + {method: "POST", body: `{"network":{"access":"restricted"}}`, segments: []string{"/v1/agents/environments/templates/", template}}, + {method: "POST", body: `{"packages":{"cargo":["x"]}}`, segments: []string{"/v1/agents/environments/templates/", template}}, + {method: "POST", body: `{}`, segments: []string{"/v1/agents/sessions/", session}}, + {method: "POST", body: `{"metadata":{"k":null}}`, segments: []string{"/v1/agents/sessions/", session}}, + {method: "POST", body: `{"events":null}`, segments: []string{"/v1/agents/sessions/", session, "/events"}}, + {method: "POST", body: `{"events":[{"type":"unknown"}]}`, segments: []string{"/v1/agents/sessions/", session, "/events"}}, + {method: "POST", body: `{"default_version":"abc"}`, segments: []string{"/v1/skills/", skill.ID}}, + {method: "POST", body: `{"default_version":""}`, segments: []string{"/v1/skills/", skill.ID}}, + {method: "GET", query: "?status=bogus", segments: []string{"/v1/vaults/", vault, "/credentials"}}, + {method: "GET", query: "?path=relative", segments: []string{"/v1/agents/environments/", environment, "/files"}}, + } { + compare(r, owner, contentType(r.body), []byte(r.body)) + } + // Skill version creation validates the multipart archive before the Skill lookup. + versionRoute := route{method: http.MethodPost, segments: []string{"/v1/skills/", skill.ID, "/versions"}} + for _, upload := range []struct { + archive []byte + fields map[string]string + }{ + {store.SkillArchive(t, "path-version"), nil}, + {store.SkillArchive(t, "path-version-default"), map[string]string{"default": "true"}}, + {[]byte("not a ZIP archive"), nil}, + {store.SkillArchive(t, "path-version-invalid-default"), map[string]string{"default": "maybe"}}, + } { + var body bytes.Buffer + form := multipart.NewWriter(&body) + for name, value := range upload.fields { + if err := form.WriteField(name, value); err != nil { + t.Fatal(err) + } + } + part, err := form.CreateFormFile("files", "proof.zip") + if err != nil { + t.Fatal(err) + } + if _, err := part.Write(upload.archive); err != nil { + t.Fatal(err) + } + if err := form.Close(); err != nil { + t.Fatal(err) + } + compare(versionRoute, owner, form.FormDataContentType(), body.Bytes()) + missingPath := "/v1/skills/skill_" + uuid.NewString() + "/versions" + wantStatus, wantBody := client.do(foreign, http.MethodPost, missingPath, form.FormDataContentType(), body.Bytes()) + for _, target := range []string{strings.Join(versionRoute.segments, ""), "/v1/skills/not-a-skill/versions"} { + if status, got := client.do(foreign, http.MethodPost, target, form.FormDataContentType(), body.Bytes()); status != wantStatus || got != wantBody { + t.Errorf("foreign Skill version %s = %d %s; missing %d %s", target, status, got, wantStatus, wantBody) + } + } + if upload.fields == nil && string(upload.archive) != "not a ZIP archive" && wantStatus != http.StatusNotFound { + t.Errorf("valid Skill version upload to a missing Skill = %d %s", wantStatus, wantBody) + } + checked++ + } + if checked < 600 { + t.Fatalf("route matrix checked only %d cases", checked) + } + + // Storage availability checks also run before the lookup of a missing identifier. + h, err = api.NewHandler(store.New(pool), auth, "codex") + if err != nil { + t.Fatal(err) + } + unconfigured := httptest.NewServer(h) + defer unconfigured.Close() + keyless := pathIDClient{t: t, server: unconfigured} + for _, r := range []route{ + {method: "POST", body: `{"name":"path","auth":{"type":"static_bearer","mcp_server_url":"https://mcp.example/mcp","token":"t"}}`, segments: []string{"/v1/vaults/", vault, "/credentials"}}, + {method: "POST", body: `{"auth":{"type":"static_bearer","token":"t"}}`, segments: []string{"/v1/vaults/", vault, "/credentials/", credential}}, + {method: "POST", body: `{"env":{"PATH_ID":"value"}}`, segments: []string{"/v1/agents/environments/templates/", template}}, + } { + for index := 1; index < len(r.segments); index += 2 { + wantStatus, wantBody := keyless.do(owner, r.method, path(r.segments, index, uuid.NewString()), "application/json", []byte(r.body)) + status, body := keyless.do(owner, r.method, path(r.segments, index, "not-a-uuid"), "application/json", []byte(r.body)) + if status != wantStatus || body != wantBody { + t.Errorf("keyless %s %s: malformed %d %s; missing %d %s", r.method, path(r.segments, index, "{id}"), status, body, wantStatus, wantBody) + } + } + } + + // Malformed list cursors remain invalid requests rather than missing resources. + for _, list := range []string{"/turns", "/items", "/subagents", "/artifacts"} { + status, body := client.do(owner, http.MethodGet, "/v1/agents/sessions/"+session+list+"?after=not-a-uuid", "", nil) + if status != http.StatusBadRequest || !strings.Contains(body, `"code":"invalid_request"`) { + t.Errorf("malformed %s cursor = %d %s", list, status, body) + } + missingStatus, missingBody := client.do(owner, http.MethodGet, "/v1/agents/sessions/"+missing+list+"?after=not-a-uuid", "", nil) + status, body = client.do(owner, http.MethodGet, "/v1/agents/sessions/not-a-uuid"+list+"?after=not-a-uuid", "", nil) + if missingStatus != http.StatusNotFound || status != missingStatus || body != missingBody { + t.Errorf("malformed Session with malformed %s cursor = %d %s; missing %d %s", list, status, body, missingStatus, missingBody) + } + } + // Top-level cursors keep their existing family behavior; Templates already + // resolved malformed cursors as missing before this change. + for list, want := range map[string]int{"/v1/agents/sessions": 400, "/v1/agents": 400, "/v1/vaults": 400, "/v1/vaults/" + vault + "/credentials": 400, "/v1/agents/environments/templates": 404} { + if status, body := client.do(owner, http.MethodGet, list+"?after=not-a-uuid", "", nil); status != want { + t.Errorf("malformed %s cursor = %d %s; want %d", list, status, body, want) + } + } + if after := databaseDigest(t, pool); !mapsEqual(before, after) { + t.Fatal("malformed, missing or foreign path identifiers changed persisted state") + } + // Owned resources remain readable after the rejected mutations. + for _, owned := range []string{"/v1/vaults/" + vault, "/v1/vaults/" + vault + "/credentials/" + credential, "/v1/agents/" + agent, "/v1/agents/environments/templates/" + template, "/v1/agents/sessions/" + session, "/v1/agents/sessions/" + session + "/turns/" + turn, "/v1/files/" + file.ID, "/v1/skills/" + skill.ID + "/versions/1"} { + if status, body := client.do(owner, http.MethodGet, owned, "", nil); status != http.StatusOK { + t.Errorf("owned %s = %d %s", owned, status, body) + } + } +} + +func mapsEqual(a, b map[string]string) bool { + if len(a) != len(b) { + return false + } + for key, value := range a { + if b[key] != value { + return false + } + } + return true +} diff --git a/services/agents-api/internal/store/session_artifacts.go b/services/agents-api/internal/store/session_artifacts.go index 3c5866588..69d5df217 100644 --- a/services/agents-api/internal/store/session_artifacts.go +++ b/services/agents-api/internal/store/session_artifacts.go @@ -57,6 +57,10 @@ func (s *Store) ListSessionArtifacts(ctx context.Context, tenantID, sessionID, e } } if cursor != "" { + // A malformed cursor remains an invalid request, unlike a path identifier. + if _, err := parseID(cursor); err != nil { + return ArtifactPage{}, err + } after, err := s.GetSessionArtifact(ctx, tenantID, sessionID, cursor) if err != nil { return ArtifactPage{}, err @@ -148,7 +152,7 @@ func (s *Store) DeleteSessionArtifact(ctx context.Context, tenantID, sessionID, } func artifactLookup(tenantID, sessionID, artifactID string) (sqlc.GetSessionArtifactParams, error) { - ids, err := turnLookup(tenantID, sessionID, artifactID) + ids, err := publicTurnLookup(tenantID, sessionID, artifactID) return sqlc.GetSessionArtifactParams{TenantID: ids.TenantID, SessionID: ids.SessionID, ID: ids.ID}, err } diff --git a/services/agents-api/internal/store/session_events.go b/services/agents-api/internal/store/session_events.go index 37aa47631..4618af6a2 100644 --- a/services/agents-api/internal/store/session_events.go +++ b/services/agents-api/internal/store/session_events.go @@ -82,10 +82,7 @@ func (s *Store) SessionEventCursor(ctx context.Context, tenantID, sessionID stri if err != nil { return 0, err } - id, err := parseID(sessionID) - if err != nil { - return 0, err - } + id := parsePathID(sessionID) cursor, err := s.queries.SessionEventCursor(ctx, sqlc.SessionEventCursorParams{TenantID: tenant, ID: id}) if errors.Is(err, pgx.ErrNoRows) { return 0, ErrNotFound diff --git a/services/agents-api/internal/store/session_metadata.go b/services/agents-api/internal/store/session_metadata.go index de72980d8..f4e779624 100644 --- a/services/agents-api/internal/store/session_metadata.go +++ b/services/agents-api/internal/store/session_metadata.go @@ -15,10 +15,7 @@ func (s *Store) UpdateSessionMetadata(ctx context.Context, tenantID, sessionID s if err != nil { return Session{}, err } - id, err := parseID(sessionID) - if err != nil { - return Session{}, err - } + id := parsePathID(sessionID) encoded, err := encodeMetadata(metadata) if err != nil { return Session{}, err diff --git a/services/agents-api/internal/store/session_metadata_test.go b/services/agents-api/internal/store/session_metadata_test.go index 8e50c4189..3cc1b51d9 100644 --- a/services/agents-api/internal/store/session_metadata_test.go +++ b/services/agents-api/internal/store/session_metadata_test.go @@ -63,7 +63,8 @@ func TestSessionMetadataPreservesCreationAndExecutionData(t *testing.T) { {uuid.NewString(), first.ID, nil, ErrNotFound}, {tenant, uuid.NewString(), nil, ErrNotFound}, {"invalid", first.ID, nil, ErrInvalidInput}, - {tenant, "invalid", nil, ErrInvalidInput}, + // A malformed path identifier is indistinguishable from a missing Session. + {tenant, "invalid", nil, ErrNotFound}, {tenant, first.ID, map[string]string{"large": strings.Repeat("x", 64*1024)}, ErrInvalidInput}, } { if _, err := s.UpdateSessionMetadata(ctx, test.tenant, test.session, test.metadata); !errors.Is(err, test.want) { diff --git a/services/agents-api/internal/store/session_transaction.go b/services/agents-api/internal/store/session_transaction.go index f16575d06..54f742317 100644 --- a/services/agents-api/internal/store/session_transaction.go +++ b/services/agents-api/internal/store/session_transaction.go @@ -25,9 +25,12 @@ func (s *Store) withSessionState(ctx context.Context, tenantID, sessionID string if err != nil { return err } - id, err := parseID(sessionID) - if err != nil { - return err + // Public paths resolve malformed IDs as missing; internal callers keep parseID. + id := parsePathID(sessionID) + if !public { + if id, err = parseID(sessionID); err != nil { + return err + } } begin := func(ctx context.Context, apply func(pgx.Tx) error) error { return pgx.BeginFunc(ctx, s.pool, apply) diff --git a/services/agents-api/internal/store/sessions.go b/services/agents-api/internal/store/sessions.go index 19882d41c..e0a2dc2db 100644 --- a/services/agents-api/internal/store/sessions.go +++ b/services/agents-api/internal/store/sessions.go @@ -184,10 +184,7 @@ func (s *Store) GetSession(ctx context.Context, tenantID, sessionID string) (Ses if err != nil { return Session{}, err } - id, err := parseID(sessionID) - if err != nil { - return Session{}, err - } + id := parsePathID(sessionID) row, err := s.queries.GetSession(ctx, sqlc.GetSessionParams{TenantID: tenant, ID: id}) if errors.Is(err, pgx.ErrNoRows) { return Session{}, ErrNotFound @@ -214,6 +211,10 @@ func (s *Store) ListSessions(ctx context.Context, tenantID, cursor string, limit params.AgentID = pgtype.Text{String: *agentID, Valid: true} } if cursor != "" { + // A malformed cursor remains an invalid request, unlike a path identifier. + if _, err := parseID(cursor); err != nil { + return SessionPage{}, err + } after, err := s.GetSession(ctx, tenantID, cursor) if err != nil { return SessionPage{}, err @@ -249,6 +250,22 @@ func parseID(value string) (pgtype.UUID, error) { return pgtype.UUID{Bytes: id, Valid: true}, nil } +// UnknownResourceID never names a stored resource: Core assigns version 4 or 5 +// UUIDs, and the maximum UUID is neither. +var UnknownResourceID = uuid.Max.String() + +// parsePathID parses a caller-supplied resource path identifier. A value that +// cannot name a resource resolves to UnknownResourceID, so the request follows +// exactly the path of a well-formed missing identifier, including validation +// order. List cursors and request-body references keep parseID. +func parsePathID(value string) pgtype.UUID { + id, err := parseID(value) + if err != nil { + return pgtype.UUID{Bytes: uuid.Max, Valid: true} + } + return id +} + func sessionFromRow(row sqlc.Session) (Session, error) { session := Session{ID: uuid.UUID(row.ID.Bytes).String(), TenantID: uuid.UUID(row.TenantID.Bytes).String(), Engine: row.Engine, CreatedAt: row.CreatedAt.Time, RequiredActions: []v1.FunctionCallAction{}} creator, err := sessionCreator(row.CreatorKind, row.CreatorID) diff --git a/services/agents-api/internal/store/skill_versions.go b/services/agents-api/internal/store/skill_versions.go index 61adc5d41..2a5c6c145 100644 --- a/services/agents-api/internal/store/skill_versions.go +++ b/services/agents-api/internal/store/skill_versions.go @@ -13,7 +13,7 @@ import ( var ErrDefaultSkillVersion = errors.New("cannot delete the default skill version") func (s *Store) CreateSkillVersion(ctx context.Context, tenantID, skillID string, archive []byte, makeDefault bool) (SkillVersion, error) { - tenant, id, err := skillIDs(tenantID, skillID) + tenant, id, err := skillPathIDs(tenantID, skillID) if err != nil { return SkillVersion{}, err } @@ -44,14 +44,11 @@ func (s *Store) CreateSkillVersion(ctx context.Context, tenantID, skillID string } func (s *Store) GetSkillVersion(ctx context.Context, tenantID, skillID, version string) (SkillVersion, error) { - tenant, id, err := skillIDs(tenantID, skillID) - if err != nil { - return SkillVersion{}, err - } - number, err := skillVersionNumber(version) + tenant, id, err := skillPathIDs(tenantID, skillID) if err != nil { return SkillVersion{}, err } + number := skillPathVersion(version) row, err := s.queries.GetSkillVersion(ctx, sqlc.GetSkillVersionParams{TenantID: tenant, SkillID: id, Version: number}) if errors.Is(err, pgx.ErrNoRows) { err = ErrNotFound @@ -61,14 +58,11 @@ func (s *Store) GetSkillVersion(ctx context.Context, tenantID, skillID, version // ReadSkillVersion reads metadata and encrypted bytes from one authorized row. func (s *Store) ReadSkillVersion(ctx context.Context, tenantID, skillID, version string) (SkillVersion, []byte, error) { - tenant, id, err := skillIDs(tenantID, skillID) - if err != nil { - return SkillVersion{}, nil, err - } - number, err := skillVersionNumber(version) + tenant, id, err := skillPathIDs(tenantID, skillID) if err != nil { return SkillVersion{}, nil, err } + number := skillPathVersion(version) row, err := s.queries.ReadSkillVersion(ctx, sqlc.ReadSkillVersionParams{TenantID: tenant, SkillID: id, Version: number}) if errors.Is(err, pgx.ErrNoRows) { err = ErrNotFound @@ -93,14 +87,11 @@ func (s *Store) openSkillVersion(row sqlc.SkillVersion) (SkillVersion, []byte, e } func (s *Store) DeleteSkillVersion(ctx context.Context, tenantID, skillID, version string) (SkillVersion, error) { - tenant, id, err := skillIDs(tenantID, skillID) - if err != nil { - return SkillVersion{}, err - } - number, err := skillVersionNumber(version) + tenant, id, err := skillPathIDs(tenantID, skillID) if err != nil { return SkillVersion{}, err } + number := skillPathVersion(version) var result SkillVersion err = pgx.BeginFunc(ctx, s.pool, func(tx pgx.Tx) error { q := s.queries.WithTx(tx) @@ -129,7 +120,7 @@ func (s *Store) DeleteSkillVersion(ctx context.Context, tenantID, skillID, versi // ReadDefaultSkillVersion selects the pointer and immutable content in one read. func (s *Store) ReadDefaultSkillVersion(ctx context.Context, tenantID, skillID string) (SkillVersion, []byte, error) { - tenant, id, err := skillIDs(tenantID, skillID) + tenant, id, err := skillPathIDs(tenantID, skillID) if err != nil { return SkillVersion{}, nil, err } diff --git a/services/agents-api/internal/store/skills.go b/services/agents-api/internal/store/skills.go index d91316729..bc6f3608e 100644 --- a/services/agents-api/internal/store/skills.go +++ b/services/agents-api/internal/store/skills.go @@ -60,7 +60,7 @@ func (s *Store) CreateSkill(ctx context.Context, tenantID string, archive []byte } func (s *Store) GetSkill(ctx context.Context, tenantID, skillID string) (Skill, error) { - tenant, id, err := skillIDs(tenantID, skillID) + tenant, id, err := skillPathIDs(tenantID, skillID) if err != nil { return Skill{}, err } @@ -72,7 +72,7 @@ func (s *Store) GetSkill(ctx context.Context, tenantID, skillID string) (Skill, } func (s *Store) UpdateSkillDefault(ctx context.Context, tenantID, skillID, version string) (Skill, error) { - tenant, id, err := skillIDs(tenantID, skillID) + tenant, id, err := skillPathIDs(tenantID, skillID) if err != nil { return Skill{}, err } @@ -101,7 +101,7 @@ func (s *Store) UpdateSkillDefault(ctx context.Context, tenantID, skillID, versi } func (s *Store) DeleteSkill(ctx context.Context, tenantID, skillID string) error { - tenant, id, err := skillIDs(tenantID, skillID) + tenant, id, err := skillPathIDs(tenantID, skillID) if err != nil { return err } @@ -151,6 +151,28 @@ func skillVersionNumber(value string) (int64, error) { return number, nil } +// skillPathIDs resolves a Skill path identifier. Like parsePathID, a malformed +// value resolves to an identifier that never exists, so the request follows the +// missing-Skill path. Request-body references keep skillIDs. +func skillPathIDs(tenantID, skillID string) (pgtype.UUID, pgtype.UUID, error) { + tenant, id, err := skillIDs(tenantID, skillID) + if errors.Is(err, ErrNotFound) { + return tenant, pgtype.UUID{Bytes: uuid.Max, Valid: true}, nil + } + return tenant, id, err +} + +// skillPathVersion resolves a version path segment. Versions start at 1, so a +// malformed segment resolves to the never-assigned version 0 and follows the +// missing-version path. Request-body selectors keep skillVersionNumber. +func skillPathVersion(value string) int64 { + number, err := skillVersionNumber(value) + if err != nil { + return 0 + } + return number +} + func skillFromRow(row sqlc.Skill) Skill { return Skill{ID: "skill_" + uuid.UUID(row.ID.Bytes).String(), Name: row.Name, Description: row.Description, CreatedAt: row.CreatedAt.Time, DefaultVersion: row.DefaultVersion, LatestVersion: row.LatestVersion} } diff --git a/services/agents-api/internal/store/skills_list.go b/services/agents-api/internal/store/skills_list.go index 16e9c6cd6..12673925d 100644 --- a/services/agents-api/internal/store/skills_list.go +++ b/services/agents-api/internal/store/skills_list.go @@ -53,7 +53,7 @@ func (s *Store) ListSkills(ctx context.Context, tenantID, after string, limit in } func (s *Store) ListSkillVersions(ctx context.Context, tenantID, skillID, after string, limit int, ascending bool) (SkillVersionPage, error) { - tenant, id, err := skillIDs(tenantID, skillID) + tenant, id, err := skillPathIDs(tenantID, skillID) if err != nil { return SkillVersionPage{}, err } diff --git a/services/agents-api/internal/store/subagent_reads.go b/services/agents-api/internal/store/subagent_reads.go index bf6612578..f8cf49220 100644 --- a/services/agents-api/internal/store/subagent_reads.go +++ b/services/agents-api/internal/store/subagent_reads.go @@ -13,11 +13,7 @@ import ( ) func publicSubagent(ctx context.Context, q *sqlc.Queries, session pgtype.UUID, id string) (v1.Subagent, error) { - parsed, err := parseID(id) - if err != nil { - return v1.Subagent{}, err - } - row, err := q.GetPublicSubagent(ctx, sqlc.GetPublicSubagentParams{SessionID: session, ID: parsed}) + row, err := q.GetPublicSubagent(ctx, sqlc.GetPublicSubagentParams{SessionID: session, ID: parsePathID(id)}) if errors.Is(err, pgx.ErrNoRows) { return v1.Subagent{}, ErrNotFound } @@ -56,6 +52,10 @@ func (s *Store) ListSubagents(ctx context.Context, tenant, session, after string err := s.withPublicSession(ctx, tenant, session, func(ctx context.Context, q *sqlc.Queries, sid pgtype.UUID) error { p := sqlc.ListPublicSubagentsParams{SessionID: sid, Ascending: asc, PageLimit: int32(limit + 1), AfterID: pgtype.UUID{Valid: true}} if after != "" { + // A malformed cursor remains an invalid request, unlike a path identifier. + if _, err := parseID(after); err != nil { + return err + } cursor, err := publicSubagent(ctx, q, sid, after) if err != nil { return err @@ -84,11 +84,7 @@ func (s *Store) ListSubagents(ctx context.Context, tenant, session, after string } func childTurn(ctx context.Context, q *sqlc.Queries, session pgtype.UUID, child, id string) (sqlc.SubagentTurn, error) { - parsed, err := parseID(id) - if err != nil { - return sqlc.SubagentTurn{}, err - } - row, err := q.GetChildTurn(ctx, sqlc.GetChildTurnParams{SessionID: session, ID: parsed}) + row, err := q.GetChildTurn(ctx, sqlc.GetChildTurnParams{SessionID: session, ID: parsePathID(id)}) if errors.Is(err, pgx.ErrNoRows) || (err == nil && uuid.UUID(row.SubagentID.Bytes).String() != child) { return row, ErrNotFound } @@ -140,6 +136,9 @@ func (s *Store) ListSubagentTurns(ctx context.Context, tenant, session, child, a childID, _ := parseID(child) p := sqlc.ListChildTurnsParams{SessionID: sid, SubagentID: childID, Ascending: asc, PageLimit: int32(limit + 1), AfterID: pgtype.UUID{Valid: true}} if after != "" { + if _, err := parseID(after); err != nil { + return err + } row, err := childTurn(ctx, q, sid, child, after) if err != nil { return err diff --git a/services/agents-api/internal/store/turn_reads.go b/services/agents-api/internal/store/turn_reads.go index 83fec7d09..4311ed653 100644 --- a/services/agents-api/internal/store/turn_reads.go +++ b/services/agents-api/internal/store/turn_reads.go @@ -25,6 +25,10 @@ func (s *Store) ListTurns(ctx context.Context, tenantID, sessionID, cursor strin session, _ := parseID(sessionID) params := sqlc.ListTurnsParams{TenantID: tenant, SessionID: session, PageLimit: int32(limit + 1), AfterID: pgtype.UUID{Valid: true}, Ascending: ascending} if cursor != "" { + // A malformed cursor remains an invalid request, unlike a path identifier. + if _, err := parseID(cursor); err != nil { + return TurnPage{}, err + } after, err := s.GetTurn(ctx, tenantID, sessionID, cursor) if err != nil { return TurnPage{}, err diff --git a/services/agents-api/internal/store/turns.go b/services/agents-api/internal/store/turns.go index b37367bc0..a7f5c7700 100644 --- a/services/agents-api/internal/store/turns.go +++ b/services/agents-api/internal/store/turns.go @@ -48,7 +48,7 @@ type TurnTransition struct { } func (s *Store) GetTurn(ctx context.Context, tenantID, sessionID, turnID string) (Turn, error) { - params, err := turnLookup(tenantID, sessionID, turnID) + params, err := publicTurnLookup(tenantID, sessionID, turnID) if err != nil { return Turn{}, err } @@ -167,6 +167,13 @@ func turnLookup(tenantID, sessionID, turnID string) (sqlc.GetTurnParams, error) return p, err } +// publicTurnLookup resolves caller-supplied path identifiers for a Turn or a +// Turn-scoped resource. Unparsable values are indistinguishable from missing ones. +func publicTurnLookup(tenantID, sessionID, turnID string) (sqlc.GetTurnParams, error) { + tenant, err := parseID(tenantID) + return sqlc.GetTurnParams{TenantID: tenant, SessionID: parsePathID(sessionID), ID: parsePathID(turnID)}, err +} + func turnFromRow(row sqlc.Turn) Turn { return Turn{ ID: uuid.UUID(row.ID.Bytes).String(), SessionID: uuid.UUID(row.SessionID.Bytes).String(), Status: row.Status, diff --git a/services/agents-api/internal/store/unstorable_text.go b/services/agents-api/internal/store/unstorable_text.go new file mode 100644 index 000000000..33155cb3d --- /dev/null +++ b/services/agents-api/internal/store/unstorable_text.go @@ -0,0 +1,16 @@ +package store + +import ( + "errors" + + "github.com/jackc/pgx/v5/pgconn" +) + +// UnstorableText reports PostgreSQL rejecting client text it cannot represent: +// U+0000 or invalid UTF-8 in a text parameter (22021), or a jsonb \u0000 escape +// (22P05). The rejected statement stores nothing, and writes sharing its +// transaction roll back. +func UnstorableText(err error) bool { + var databaseError *pgconn.PgError + return errors.As(err, &databaseError) && (databaseError.Code == "22021" || databaseError.Code == "22P05") +} diff --git a/services/agents-api/internal/store/unstorable_text_public_test.go b/services/agents-api/internal/store/unstorable_text_public_test.go new file mode 100644 index 000000000..27568f846 --- /dev/null +++ b/services/agents-api/internal/store/unstorable_text_public_test.go @@ -0,0 +1,121 @@ +package store_test + +import ( + "archive/zip" + "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" +) + +// PostgreSQL text and jsonb cannot store U+0000. Every persisted client string +// containing it must be rejected as a client error before anything is written. +func TestUnstorableTextRejectsWithoutWritesPostgres(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{62}, 32)) + if err != nil { + t.Fatal(err) + } + s := store.NewWithCredentialCipher(pool, cipher) + token := uuid.NewString() + auth, err := api.NewAuthenticator([]api.APIKey{{OrganizationID: "test-org", ProjectID: uuid.NewString(), SubjectKind: "service_account", SubjectID: "nul-owner", TokenSHA256: device.HashCredential(token), TenantID: uuid.NewString()}}) + 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} + vault := client.created(token, "/v1/vaults", `{"name":"nul-vault"}`) + agent := client.created(token, "/v1/agents", `{"model":"nul-model"}`) + template := client.created(token, "/v1/agents/environments/templates", `{"name":"nul-template"}`) + session := client.created(token, "/v1/agents/sessions", `{"agent":{"model":"nul-model"},"environment":{"type":"none"},"input":"Keep this Session."}`) + before := databaseDigest(t, pool) + const nul = `a\u0000b` + keyParam, valueParam := "metadata.a\x00b", "metadata.k" + for _, tc := range []struct { + name, method, path, body string + param *string + }{ + {"vault metadata value", "POST", "/v1/vaults", `{"metadata":{"k":"` + nul + `"}}`, &valueParam}, + {"vault metadata key", "POST", "/v1/vaults", `{"metadata":{"` + nul + `":"v"}}`, &keyParam}, + {"agent create metadata", "POST", "/v1/agents", `{"model":"m","metadata":{"k":"` + nul + `"}}`, &valueParam}, + {"agent update metadata", "POST", "/v1/agents/" + agent, `{"metadata":{"` + nul + `":"v"}}`, &keyParam}, + {"session create metadata", "POST", "/v1/agents/sessions", `{"agent":{"model":"m"},"environment":{"type":"none"},"input":"hi","metadata":{"k":"` + nul + `"}}`, &valueParam}, + {"session update metadata", "POST", "/v1/agents/sessions/" + session, `{"metadata":{"k":"` + nul + `"}}`, &valueParam}, + {"vault name", "POST", "/v1/vaults", `{"name":"` + nul + `"}`, nil}, + {"credential name", "POST", "/v1/vaults/" + vault + "/credentials", `{"name":"` + nul + `","auth":{"type":"static_bearer","mcp_server_url":"https://mcp.example/mcp","token":"t"}}`, nil}, + {"agent name", "POST", "/v1/agents", `{"model":"m","name":"` + nul + `"}`, nil}, + {"agent model", "POST", "/v1/agents", `{"model":"` + nul + `"}`, nil}, + {"agent instructions", "POST", "/v1/agents", `{"model":"m","instructions":"` + nul + `"}`, nil}, + {"agent tool", "POST", "/v1/agents", `{"model":"m","tools":[{"type":"function","name":"f","description":"` + nul + `","parameters":{"type":"object"}}]}`, nil}, + {"agent update name", "POST", "/v1/agents/" + agent, `{"name":"` + nul + `"}`, nil}, + {"template create name", "POST", "/v1/agents/environments/templates", `{"name":"` + nul + `"}`, nil}, + {"template update name", "POST", "/v1/agents/environments/templates/" + template, `{"name":"` + nul + `"}`, nil}, + {"session input", "POST", "/v1/agents/sessions", `{"agent":{"model":"m"},"environment":{"type":"none"},"input":"` + nul + `"}`, nil}, + {"session instructions", "POST", "/v1/agents/sessions", `{"agent":{"model":"m","instructions":"` + nul + `"},"environment":{"type":"none"},"input":"hi"}`, nil}, + {"session event input", "POST", "/v1/agents/sessions/" + session + "/events", `{"events":[{"type":"agent.session.input.message","input":[{"role":"user","content":[{"type":"input_text","text":"` + nul + `"}]}]}]}`, nil}, + } { + status, body := client.do(token, tc.method, tc.path, "application/json", []byte(tc.body)) + var response struct { + Error struct { + Type, Code string + Param *string + } + } + if status != http.StatusBadRequest || json.Unmarshal([]byte(body), &response) != nil || response.Error.Type != "invalid_request_error" || response.Error.Code != "invalid_request_error" || + (tc.param == nil) != (response.Error.Param == nil) || tc.param != nil && *response.Error.Param != *tc.param { + t.Errorf("%s: %d %s", tc.name, status, body) + } + } + + // Skill metadata parsed from an uploaded archive uses the same mapping. + var archive bytes.Buffer + writer := zip.NewWriter(&archive) + file, err := writer.Create("proof/SKILL.md") + if err != nil { + t.Fatal(err) + } + if _, err := file.Write([]byte("---\nname: proof\ndescription: \"a\\0b\"\n---\nProof.")); err != nil { + t.Fatal(err) + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + var upload bytes.Buffer + form := multipart.NewWriter(&upload) + part, err := form.CreateFormFile("files", "proof.zip") + if err != nil { + t.Fatal(err) + } + if _, err := part.Write(archive.Bytes()); err != nil { + t.Fatal(err) + } + if err := form.Close(); err != nil { + t.Fatal(err) + } + if status, body := client.do(token, http.MethodPost, "/v1/skills", form.FormDataContentType(), upload.Bytes()); status != http.StatusBadRequest || !strings.Contains(body, `"code":"invalid_request_error"`) { + t.Errorf("skill description: %d %s", status, body) + } + // Invalid UTF-8 in a query filter reaches the same mapping, so its message is generic. + status, body := client.do(token, http.MethodGet, "/v1/agents/sessions?agent_id=%ff", "", nil) + if status != http.StatusBadRequest || body != `{"error":{"message":"Request text contains characters this service cannot store or compare, such as U+0000 or invalid UTF-8.","type":"invalid_request_error","code":"invalid_request_error","param":null}}`+"\n" { + t.Errorf("invalid UTF-8 filter: %d %s", status, body) + } + if after := databaseDigest(t, pool); !mapsEqual(before, after) { + t.Error("rejected U+0000 strings changed persisted state") + } +} diff --git a/services/agents-api/tests/official_agents.py b/services/agents-api/tests/official_agents.py index f442074cd..308918405 100644 --- a/services/agents-api/tests/official_agents.py +++ b/services/agents-api/tests/official_agents.py @@ -100,6 +100,27 @@ def verify_agents(client, other, invalid, expect_error): assert response.status_code == 400, (fields, response.status_code) assert response.json()["error"]["type"] == "invalid_request_error" expect_error(BadRequestError, lambda: agents.create(model="resource-model", extra_body=fields)) + # Rejections with official evidence report its code, param and message. + field_errors = [ + ({"name": "x" * 129}, "name", "Invalid 'name': string too long. Expected a string with maximum length 128, but got a string with length 129 instead."), + ({"metadata": {"k": 1}}, "metadata.k", "Invalid type for 'metadata.k': expected a string, but got an integer instead."), + ({"metadata": {"k": None}}, "metadata.k", "Invalid type for 'metadata.k': expected a string, but got null instead."), + ({"metadata": {str(i): "v" for i in range(17)}}, "metadata", "Invalid 'metadata': too many properties. Expected an object with at most 16 properties, but got an object with 17 properties instead."), + ({"metadata": {"x" * 65: "v"}}, "metadata." + "x" * 65, "Invalid property name in 'metadata': '" + "x" * 65 + "' is too long. Expected a string with maximum length 64, but got a string with length 65 instead."), + ({"metadata": {"k": "v" * 513}}, "metadata.k", "Invalid 'metadata.k': string too long. Expected a string with maximum length 512, but got a string with length 513 instead."), + ({"metadata": {"k": "a\x00b"}}, "metadata.k", "Invalid 'metadata.k': string contains U+0000, which this service cannot store."), + ] + count = len(list(agents.list())) + for fields, param, message in field_errors: + response = raw.post(base, headers=headers, json={"model": "resource-model", **fields}) + assert response.status_code == 400 and response.json()["error"] == { + "type": "invalid_request_error", "code": "invalid_request_error", "param": param, "message": message}, response.text + # PostgreSQL cannot store U+0000 in other strings either; this local limit has no field param. + for fields in ({"name": "a\x00b"}, {"instructions": "a\x00b"}, {"model": "a\x00b"}): + response = raw.post(base, headers=headers, json={"model": "resource-model", **fields}) + 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 diff --git a/services/agents-api/tests/official_client.py b/services/agents-api/tests/official_client.py index a8dddbf16..934bef867 100644 --- a/services/agents-api/tests/official_client.py +++ b/services/agents-api/tests/official_client.py @@ -241,6 +241,14 @@ def expect_error(error, operation): expect_error(NotFoundError, lambda: turns.retrieve(turn_ids[0], session_id=first.id)) expect_error(NotFoundError, lambda: turns.list(first.id, after=turn_ids[0])) expect_error(BadRequestError, lambda: turns.list(turn_session.id, limit=101)) + # Unparsable identifiers share the missing-resource response; + # malformed list cursors remain invalid requests. + for malformed in ("sess_" + uuid.uuid4().hex, "invalid"): + expect_error(NotFoundError, lambda: sessions.retrieve(malformed)) + expect_error(NotFoundError, lambda: turns.list(malformed)) + expect_error(NotFoundError, lambda: sessions.items.list(malformed)) + expect_error(NotFoundError, lambda: turns.retrieve("turn_" + uuid.uuid4().hex, session_id=turn_session.id)) + expect_error(BadRequestError, lambda: turns.list(turn_session.id, after="invalid")) saved_items = verify_items(a, b, invalid, turn_session.id, first.id, turn_ids, expect_error) request_sessions.append(verify_active_session_metadata(a, turn_session.id)) referenced, reference_retry = verify_agent_references(a, b, expect_error) diff --git a/services/agents-api/tests/official_environment_network.py b/services/agents-api/tests/official_environment_network.py index 1e3bfb1be..1430740e1 100644 --- a/services/agents-api/tests/official_environment_network.py +++ b/services/agents-api/tests/official_environment_network.py @@ -33,7 +33,9 @@ def verify_network_resources(client, foreign, http, agent): {'access': 'enabled', 'allowed_domains': ['example.com']}, ]: for target in [endpoint, root + '/agents/environments/templates']: - assert http.post(target, headers=headers, json={'network': network}).status_code == 400 + response = http.post(target, headers=headers, json={'network': network}) + assert response.status_code == 400 and response.json()['error']['code'] == 'invalid_request_error' + assert response.json()['error']['param'] is None assert templates.retrieve(template.id).network.to_dict() == NETWORK for override in [{'access': 'enabled'}, {'access': 'restricted', 'allowed_domains': ['sub.example.com']}, {'access': 'restricted', 'allowed_domains': ['example.org']}]: diff --git a/services/agents-api/tests/official_environment_retrieve.py b/services/agents-api/tests/official_environment_retrieve.py index 198581db5..7e7c019ed 100644 --- a/services/agents-api/tests/official_environment_retrieve.py +++ b/services/agents-api/tests/official_environment_retrieve.py @@ -82,8 +82,11 @@ def rejected(url, status, code, request_headers=headers, method="GET"): pass else: raise AssertionError("absent Environment was exposed through the SDK") - for malformed in ("invalid", str(uuid.UUID(int=0))): - rejected(base + "/v1/agents/environments/" + malformed, 400, "invalid_request") + # Malformed identifiers cannot be distinguished from missing Environments. + missing = http.get(base + "/v1/agents/environments/" + str(uuid.uuid4()), headers=headers) + for malformed in ("invalid", str(uuid.UUID(int=0)), "env_" + uuid.uuid4().hex): + rejected(base + "/v1/agents/environments/" + malformed, 404, "not_found_error") + assert http.get(base + "/v1/agents/environments/" + malformed, headers=headers).content == missing.content rejected(endpoint, 404, "not_found_error", headers | {"Authorization": "Bearer " + settings["foreign_token"]}) for authorization in (None, "Bearer invalid", "Bearer " + settings["executor_token"]): request_headers = {"OpenAI-Beta": "agents=v1"} diff --git a/services/agents-api/tests/official_session_metadata.py b/services/agents-api/tests/official_session_metadata.py index 5ebd66c3d..2e22d1072 100644 --- a/services/agents-api/tests/official_session_metadata.py +++ b/services/agents-api/tests/official_session_metadata.py @@ -50,10 +50,6 @@ def assert_metadata(session, expected): invalid_fields = [ {"metadata": []}, {"metadata": "value"}, {"metadata": False}, - {"metadata": {"key": None}}, {"metadata": {"key": 1}}, - {"metadata": {"key": []}}, {"metadata": {"key": {}}}, - {"metadata": {str(i): "value" for i in range(17)}}, - {"metadata": {"界" * 65: "value"}}, {"metadata": {"key": "🧪" * 513}}, {"agent": spec["agent"]}, {"environment": spec["environment"]}, {"tenant_id": str(uuid.uuid4())}, {"Metadata": {}}, ] @@ -62,6 +58,24 @@ def assert_metadata(session, expected): assert response.status_code == 400 and response.json()["error"]["code"] == "invalid_request" expect_error(BadRequestError, lambda: sessions.update(first.id, extra_body=fields)) assert sessions.retrieve(first.id) == current + # Metadata values and limits report the official code, param and message. + field_errors = [ + ({"key": None}, "metadata.key", "Invalid type for 'metadata.key': expected a string, but got null instead."), + ({"key": 1}, "metadata.key", "Invalid type for 'metadata.key': expected a string, but got an integer instead."), + ({"key": []}, "metadata.key", "Invalid type for 'metadata.key': expected a string, but got an array instead."), + ({"key": {}}, "metadata.key", "Invalid type for 'metadata.key': expected a string, but got an object instead."), + ({str(i): "value" for i in range(17)}, "metadata", "Invalid 'metadata': too many properties. Expected an object with at most 16 properties, but got an object with 17 properties instead."), + ({"界" * 65: "value"}, "metadata." + "界" * 65, "Invalid property name in 'metadata': '" + "界" * 65 + "' is too long. Expected a string with maximum length 64, but got a string with length 65 instead."), + ({"key": "🧪" * 513}, "metadata.key", "Invalid 'metadata.key': string too long. Expected a string with maximum length 512, but got a string with length 513 instead."), + ({"key": "a\x00b"}, "metadata.key", "Invalid 'metadata.key': string contains U+0000, which this service cannot store."), + ] + for metadata, param, message in field_errors: + response = raw.post(url, headers=headers, json={"metadata": metadata}) + assert response.status_code == 400 and response.json()["error"] == { + "type": "invalid_request_error", "code": "invalid_request_error", "param": param, "message": message} + 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" @@ -82,7 +96,11 @@ def assert_metadata(session, expected): expect_error(BadRequestError, lambda: sessions.update(str(uuid.uuid4()))) expect_error(BadRequestError, lambda: other.beta.agents.sessions.update(first.id)) expect_error(AuthenticationError, lambda: invalid.beta.agents.sessions.update(first.id)) - expect_error(BadRequestError, lambda: sessions.update("invalid-id", metadata={})) + for malformed in ("invalid-id", "sess_" + uuid.uuid4().hex, str(uuid.UUID(int=0))): + missing = raw.post(str(client.base_url).rstrip("/") + "/agents/sessions/" + str(uuid.uuid4()), headers=headers, json={"metadata": {}}) + response = raw.post(str(client.base_url).rstrip("/") + "/agents/sessions/" + malformed, headers=headers, json={"metadata": {}}) + assert missing.status_code == response.status_code == 404 and response.json() == missing.json() + expect_error(NotFoundError, lambda: sessions.update(malformed, metadata={})) expect_error(BadRequestError, lambda: sessions.update(first.id, extra_headers={"OpenAI-Beta": ""})) assert sessions.retrieve(first.id) == current print("Session metadata: pinned SDK and raw HTTP replacement, clearing, omission, limits, authentication, tenant isolation, unchanged configuration and creation retry identity passed.") diff --git a/services/agents-api/tests/official_session_requests.py b/services/agents-api/tests/official_session_requests.py index 2582b0a7b..54519155a 100644 --- a/services/agents-api/tests/official_session_requests.py +++ b/services/agents-api/tests/official_session_requests.py @@ -11,24 +11,26 @@ def verify_session_create_requests(client, spec): # agent_id is str and stream is a boolean literal union without None. sessions = client.beta.agents.sessions before = {session.id for session in sessions.list()} + # Non-string metadata values use the official code and metadata. param. invalid = [ - {"stream": None}, {"stream": "false"}, {"stream": 0}, - {"agent_id": None}, {"agent_id": 0}, - {"metadata": {"label": None}}, - {"metadata": {"empty": "", "label": None}}, - {"metadata": {"label": 0}}, {"metadata": []}, + ({"stream": None}, None), ({"stream": "false"}, None), ({"stream": 0}, None), + ({"agent_id": None}, None), ({"agent_id": 0}, None), + ({"metadata": {"label": None}}, "metadata.label"), + ({"metadata": {"empty": "", "label": None}}, "metadata.label"), + ({"metadata": {"label": 0}}, "metadata.label"), ({"metadata": []}, None), ] headers = {"Authorization": f"Bearer {client.api_key}", "OpenAI-Beta": "agents=v1"} with httpx2.Client(trust_env=False, timeout=10) as raw: - for fields in invalid: + for fields, param in invalid: + code = "invalid_request_error" if param else "invalid_request" response = raw.post(str(client.base_url).rstrip("/") + "/agents/sessions", headers=headers, json={**spec, **fields}) assert response.status_code == 400, (fields, response.status_code) - assert response.json()["error"]["code"] == "invalid_request" + assert response.json()["error"]["code"] == code and response.json()["error"]["param"] == param try: sessions.create(**spec, extra_body=fields) except BadRequestError as error: - assert error.body["code"] == "invalid_request" + assert error.body["code"] == code and error.body["param"] == param else: raise AssertionError(f"Official client accepted invalid fields: {fields}") assert {session.id for session in sessions.list()} == before diff --git a/services/agents-api/tests/official_vaults.py b/services/agents-api/tests/official_vaults.py index 0ecc6cdf4..ee28c8089 100644 --- a/services/agents-api/tests/official_vaults.py +++ b/services/agents-api/tests/official_vaults.py @@ -71,6 +71,14 @@ def verify_vaults(client, other, invalid, peer, binding, expect_error): assert response.status_code == 400 assert response.json()["error"]["type"] == "invalid_request_error" expect_error(BadRequestError, lambda: vaults.create(extra_body=request)) + # Metadata value types use the official code and param. U+0000 is a local + # storage limit: metadata reports its key, while other strings have no param. + for request, param in (({"metadata": {"bad": 1}}, "metadata.bad"), ({"metadata": {"bad": None}}, "metadata.bad"), + ({"metadata": {"k": "a\x00b"}}, "metadata.k"), ({"metadata": {"a\x00b": "v"}}, "metadata.a\x00b"), + ({"name": "a\x00b"}, None)): + 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