feat(v2): write-side helpers + format_schema $ref sandboxing#1890
Conversation
Four reviewers (security, ad-tech-protocol, code, dx) converged on five findings. Addressing all five plus polish items. ### Security ship-blockers - **Sibling-merge precedence flipped safety.** Constraint keywords on `$ref` (e.g. `additionalProperties: true` next to a `$ref` whose referent says `false`) silently overrode the referenced subschema. Per JSON Schema 2020-12, $ref siblings are an additional constraint, not an override. Sibling allowlist now limited to annotation keys (description, title, $comment, examples, default, deprecated, readOnly, writeOnly, $schema); constraint siblings raise `invalid_ref`. Merge spread reversed: referent wins on collision. - **`http://mirror...` accepted on the mirror branch.** Spec requires https. Dropped the http accept in `assertRefAllowed`. - **JSON Pointer __proto__ / constructor / prototype traversal.** JSON.parse can produce objects with own __proto__; hasOwnProperty alone doesn't block. Added FORBIDDEN_POINTER_SEGMENTS reject set. ### Protocol compliance - **Mirror domain inconsistency in the spec.** format_schema $ref section references mirror.adcontextprotocol.org; the surrounding 3.1 migration text moved the AAO mirror to creative.adcontextprotocol.org. Filed adcontextprotocol/adcp#4862. Interim: DEFAULT_MIRROR_HOSTS accepts both. DEFAULT_MIRROR_HOST kept as @deprecated single alias. ### DX - **formatIdsFromOptions now throws on no-v1-form** with a diagnostic naming the capability_id/format_kind and the reason. Matches formatIdsForCapability's fail-closed posture. Added tryFormatIdsFromOptions non-throwing variant. - **JSDoc breadcrumbs** on V2AugmentedProduct + AgentClient.createMediaBuy pointing to the write-side helpers + a worked example. ### Tests 8 new sandbox tests + 3 new write-side tests + fixture updates for the new sibling rule. 117/117 v2+projection+format-schema tests passing. Typecheck + format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Expert review round complete. Ran 4 agents in parallel (security, ad-tech-protocol, code-reviewer-deep, dx-expert) + CI in parallel. CI was green on the original push; expert review surfaced five convergent findings plus polish items. Pushed Ship-blockers fixed
DX polish
Tests8 new sandbox tests (constraint-sibling rejection, annotation-allowlist, https-only mirror, Will admin-merge when CI is green on this push. |
|
Acknowledged — all five blockers addressed in Generated by Claude Code |
|
Noted — standing by. No triage action taken. Triaged by Claude Code. Session: https://claude.ai/code/session_01BNY6uH2CkZhq3EgyQxctrk Generated by Claude Code |
Two adopter-facing additions on top of 7.10's V2 projection layer. ### Write-side helpers (`src/lib/v2/projection/write-side.ts`) After `agent.getProducts()` returns the V2-mental-model response, V2 buyers need to construct `format_ids[]` for `create_media_buy` packages. The spec at 3.1-beta still carries only `format_ids[]` on PackageRequest (see adcontextprotocol/adcp#4842 for the upstream proposal to add `capability_id` on the create side). Until that lands, buyers bridge through the read-projection layer's `v1_format_ref[]` annotation. - `formatIdsFromOptions(decl)` — extract format_ids[] from a chosen V2 declaration; returns [] for canonical_formats_only opt-outs and inherently-v2 canonicals (sponsored_placement, agent_placement, image_carousel, responsive_creative). - `formatIdsForCapability(product, capabilityId)` — round-trip a stored capability_id back to format_ids[]; throws on missing with a diagnostic listing the available capability_ids. Both helpers deprecate once adcontextprotocol/adcp#4842 lands and the SDK ships a V2-native write path. ### $ref sandboxing (`src/lib/v2/format-schema/sandbox-refs.ts`) Layered on `fetchFormatSchema` from 7.10. Implements the spec's normative $ref rules from product-format-declaration.json#format_schema: - Intra-document `#/...` JSON Pointer (RFC 6901) — resolved against the parent doc; cycles trip the depth limit. - Same-origin external refs — allowed; fetched and inlined. RFC 3986 §6 origin normalization. - AAO mirror namespace (default `mirror.adcontextprotocol.org`, configurable) — allowed. - file:// — rejected unconditionally. - Cross-origin — rejected with a structured diagnostic naming both the parent origin and the offending target. Bounds default to spec ceilings: depth ≤ 8, count ≤ 256. External fetches use ssrfSafeFetch (HTTPS-only + SSRF + 1 MiB body cap + 5 s timeout). Per-$ref digest verification is NOT enforced by default — parent format_schema.uri@digest is the trust anchor and same-origin / mirror refs inherit trust. Callers can swap in a custom `fetchExternal` to enforce per-$ref digests. Also exports `DEFAULT_MAX_KEYWORDS` (10_000) and `DEFAULT_VALIDATION_BUDGET_MS` (250) constants for the future Ajv-wiring layer (not enforced by the sandboxer itself). ### Tests 22 new tests (8 write-side + 14 sandbox), all passing. 32 in the broader format-schema test set pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four reviewers (security, ad-tech-protocol, code, dx) converged on five findings. Addressing all five plus polish items. ### Security ship-blockers - **Sibling-merge precedence flipped safety.** Constraint keywords on `$ref` (e.g. `additionalProperties: true` next to a `$ref` whose referent says `false`) silently overrode the referenced subschema. Per JSON Schema 2020-12, $ref siblings are an additional constraint, not an override. Sibling allowlist now limited to annotation keys (description, title, $comment, examples, default, deprecated, readOnly, writeOnly, $schema); constraint siblings raise `invalid_ref`. Merge spread reversed: referent wins on collision. - **`http://mirror...` accepted on the mirror branch.** Spec requires https. Dropped the http accept in `assertRefAllowed`. - **JSON Pointer __proto__ / constructor / prototype traversal.** JSON.parse can produce objects with own __proto__; hasOwnProperty alone doesn't block. Added FORBIDDEN_POINTER_SEGMENTS reject set. ### Protocol compliance - **Mirror domain inconsistency in the spec.** format_schema $ref section references mirror.adcontextprotocol.org; the surrounding 3.1 migration text moved the AAO mirror to creative.adcontextprotocol.org. Filed adcontextprotocol/adcp#4862. Interim: DEFAULT_MIRROR_HOSTS accepts both. DEFAULT_MIRROR_HOST kept as @deprecated single alias. ### DX - **formatIdsFromOptions now throws on no-v1-form** with a diagnostic naming the capability_id/format_kind and the reason. Matches formatIdsForCapability's fail-closed posture. Added tryFormatIdsFromOptions non-throwing variant. - **JSDoc breadcrumbs** on V2AugmentedProduct + AgentClient.createMediaBuy pointing to the write-side helpers + a worked example. ### Tests 8 new sandbox tests + 3 new write-side tests + fixture updates for the new sibling rule. 117/117 v2+projection+format-schema tests passing. Typecheck + format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-existing format drift on main that CI flagged after the rebase. One line: shallowEqual nested-call ternary was over-wrapped. Prettier's preferred shape fits on one line at our print width.
51a51a4 to
c2b26e6
Compare
) * feat(v2): sync to 3.1.0-beta.2 + native capability_ids write path 3.1.0-beta.2 closed two upstream issues #1890 had filed: - adcp#4844 added `capability_ids?: string[]` to PackageRequest + Package (echo). V2 mental model is now end-to-end on the spec side. - adcp#4866 deprecated `mirror.adcontextprotocol.org` (never provisioned). `creative.adcontextprotocol.org` is the single trust anchor for `format_schema` $ref resolution. ### Sync surface - Bump BETA_VERSION in `scripts/sync-3-1-beta-schemas.ts` + `scripts/generate-3-1-beta-types.ts` from beta.1 to beta.2. - Add `3.1.0-beta.2` to `COMPATIBLE_PREFIX` in `sync-version.ts` (regenerates `version.ts` `COMPATIBLE_ADCP_VERSIONS`). - Add `3.1.0-beta.2` (preferred) to `versionsToTry` in `canonical-properties.ts` + `registry.ts` so projection loaders pick up the current beta. ### Native v2 write path - New `packageRefsForCapabilities(product, capabilityIds[])` helper. Returns `{capability_ids, format_ids}` ready to spread into a PackageRequest. Implements the spec's dual-emission convention (v2 sellers route by capability_ids; v1-only sellers — which ignore unknown fields per additionalProperties:true — fall back to format_ids). Throws compose-time on missing capability_id (matches seller-side UNSUPPORTED_FEATURE rejection). De-dupes v1 refs across declarations. - Bridge helpers (`formatIdsFromOptions` / `tryFormatIdsFromOptions` / `formatIdsForCapability`) marked @deprecated — exported indefinitely for v1-only callers and existing code, but new V2 work should use packageRefsForCapabilities. - `createMediaBuy` JSDoc updated with the dual-emission example. ### $ref trust anchor - `DEFAULT_MIRROR_HOSTS` collapsed to single anchor (`creative.adcontextprotocol.org`). Legacy `mirror.adcontextprotocol.org` is no longer accepted. ### Tests - 6 new `packageRefsForCapabilities` cases (dual emission, v2-only, missing capability throw, de-dup, empty input, spreadable shape). - Mirror-host test updated for the single-anchor default. - 123/123 v2+projection+format-schema tests passing locally. - Typecheck + format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(v2): address 4-expert review on #1896 ### Ship-blockers - **Empty format_ids: [] violates wire minItems: 1.** When every chosen capability is V2-only, packageRefsForCapabilities now OMITS format_ids entirely from the result rather than emitting an empty array. Spec's "neither present" fallback fires for v1-only sellers in that case. `PackageFormatRefs.format_ids` typed optional. - **De-dup key dropped dimensions.** Key was `${agent_url}::${id}` only; multi-size declarations sharing `{agent_url, id}` but different `width`/`height`/`duration_ms` silently collapsed. Key now includes the dimensional discriminators. Tests cover both true-dup collapse and sized-sibling preservation. - **`legacy*` rename, drop `@deprecated`.** `formatIdsFromOptions` / `tryFormatIdsFromOptions` / `formatIdsForCapability` → `legacyFormatIdsFromOptions` / `tryLegacyFormatIdsFromOptions` / `legacyFormatIdsForCapability`. Semantic narrowing instead of deprecation: the helpers solve a different problem (single-target v1 payload) than packageRefsForCapabilities (dual-emission V2 payload). `@deprecated` would have created ESLint noise for legitimate v1-only callers and eroded the warning surface for actually-deprecated APIs later. Old names not preserved — rename predates first 7.10 npm. ### Should-fix - **CapabilityIdsLookupError** with `.code` in { unknown_capability_id | capability_ids_not_published | empty_input | invalid_product }. Mirrors the spec's distinct UNSUPPORTED_FEATURE failure modes so adopters can branch fallback logic without regex. - **Empty input throws** (`empty_input`) rather than returning a confusing `{capability_ids: [], format_ids: []}`. - **`capability_ids_not_published` distinct reason** when the product publishes format_options[] but none carry capability_id at all. Error message points at legacyFormatIdsFromOptions for the fallback. - **`invalid_product` guard** when caller passes the array instead of a single product element (`Array.isArray(product)` → "did you pass `products` instead of `products[0]`?"). - **Spread-order JSDoc warning** on PackageFormatRefs.format_ids explaining that explicit `format_ids` overrides MUST come AFTER `...refs` in spread order. - **createMediaBuy JSDoc** now shows the resulting wire payload (post- spread) so adopters debugging UNSUPPORTED_FEATURE can verify both fields landed. - **Error message** lists capability_id-bearing entries only and notes count of unaddressable format_options[] entries. - **Stale doc cleanup** post mirror-collapse: sandbox-refs.ts file header, mirrorHost/mirrorHosts JSDoc, test rig comment all updated to reflect single-anchor default. ### Tests - Existing tests updated for the rename. - 6 new packageRefsForCapabilities cases (omit-not-empty, structured error codes for unknown/not-published/empty/invalid-product, sized- dup preservation, true-dup collapse, mixed-publishing error message). - 127/127 v2+projection+format-schema tests passing. Typecheck + format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Two adopter-facing additions on top of 7.10's V2 projection layer (PR #1882):
1. Write-side helpers —
formatIdsFromOptions/formatIdsForCapabilityBridges the spec gap documented at adcontextprotocol/adcp#4842 —
PackageRequestat 3.1-beta still carries onlyformat_ids[](nocapability_idv2 path). V2 buyers readingformat_options[]fromgetProductsneed to translate their chosen declaration back throughv1_format_ref[]to writecreate_media_buy. These helpers are the canonical place that bridge lives so adopters don't inlinedecl.v1_format_refpluckers everywhere.Returns
[]when the declaration has no v1 form (canonical_formats_only: trueor inherently-v2 canonical). Helpers deprecate once adcontextprotocol/adcp#4842 lands.2.
resolveSchemaRefs—$refsandboxing + DoS boundsImplements the spec's normative
$refrules fromproduct-format-declaration.json#format_schema:#/...: resolved against parent doc (RFC 6901). Cycles trip the depth limit.mirror.adcontextprotocol.org): allowed.file://: rejected unconditionally.External fetches go through
ssrfSafeFetch(HTTPS + SSRF + 1 MiB cap + 5 s timeout). Per-\$refdigest verification is opt-in via customfetchExternal— parentformat_schema.uri@digestis the trust anchor; same-origin / mirror refs inherit trust.Also exports
DEFAULT_MAX_KEYWORDS(10_000) andDEFAULT_VALIDATION_BUDGET_MS(250) constants for the future Ajv-wiring layer (manifest validator) — the sandboxer itself doesn't enforce them.Tests
22 new (8 write-side + 14 sandbox), all passing. 32 in the broader format-schema test set pass.
Out of scope (still deferred)
sync_creativeswiring).create_media_buyrequests — waiting on PackageRequest lacks v2 capability_id path — V2 mental model breaks at create_media_buy boundary adcp#4842.Test plan
🤖 Generated with Claude Code