feat(trusted-match): publisher-owned TMPX macro mapping - #5971
feat(trusted-match): publisher-owned TMPX macro mapping#5971ohalushchak-exadel wants to merge 4 commits into
Conversation
Splits the Identity Match response into two schemas so each hop's obligations are expressible in JSON Schema: - provider-identity-match-response.json — provider→router, carries tmpx_values: [string] (ordered opaque chunks; no publisher-local names on the wire). - identity-match-response.json — router→publisher, carries tmpx_providers[provider_id].values: [string]. Adds publisher-tmpx-config.json capturing the publisher-owned tmpx_macro_mapping (provider_id → ordered ad-server destinations for the surface). Drops the provider-declared tmpx_macros list from provider-registration.json — naming is a publisher deployment concern. Ordered position is sufficient for chunk identity. The router never accepts destination names from an untrusted provider, closing the cross-provider name-hijack surface (#5945) by construction. Addresses #5947.
![aao-secretariat[bot]](https://avatars.githubusercontent.com/u/221690821?s=60&v=4){kind=small}
There was a problem hiding this comment.
Argus is not auto-reviewing this PR because it modifies protected paths that require human review (static/schemas/source/index.json, static/schemas/source/trusted-match/identity-match-response.json, static/schemas/source/trusted-match/provider-identity-match-response.json, static/schemas/source/trusted-match/provider-registration.json, static/schemas/source/trusted-match/publisher-tmpx-config.json). A human reviewer should review and merge this PR; Argus will resume on subsequent PRs once these changes land on main.
There was a problem hiding this comment.
Argus is not auto-reviewing this PR because it modifies protected paths that require human review (static/schemas/source/index.json, static/schemas/source/trusted-match/identity-match-response.json, static/schemas/source/trusted-match/provider-identity-match-response.json, static/schemas/source/trusted-match/provider-registration.json, static/schemas/source/trusted-match/publisher-tmpx-config.json). A human reviewer should review and merge this PR; Argus will resume on subsequent PRs once these changes land on main.
bokelley
left a comment
bokelley
left a comment
There was a problem hiding this comment.
This direction belongs in 3.2, not 3.1.x as a patch. The PR removes tmpx_macros and reshapes both experimental response hops, so it is a breaking experimental change subject to the six-week notice/next-minor rule in docs/reference/experimental-status.mdx; issue #5947 also explicitly says to evaluate it for 3.2. Please: (1) change the changeset from patch to minor, (2) remove the claim that it lands on 3.1.x, (3) resolve the WG decision on ordinal positions versus stable provider-local slot IDs, including semantic-drift behavior, and (4) link the companion adcp-go implementation before marking ready. The current 3.1 line needs a separate compatible security mitigation; this clean redesign should not be used to bypass the maintenance-line versioning policy.
Cites the experimental-status.mdx relaxation and the #5729 precedent (same shape, same experimental-surface rule, patched onto 3.1.x weeks ago) so the reshape stays symmetric with how the surface was introduced.
There was a problem hiding this comment.
Argus is not auto-reviewing this PR because it modifies protected paths that require human review (static/schemas/source/index.json, static/schemas/source/trusted-match/identity-match-response.json, static/schemas/source/trusted-match/provider-identity-match-response.json, static/schemas/source/trusted-match/provider-registration.json, static/schemas/source/trusted-match/publisher-tmpx-config.json). A human reviewer should review and merge this PR; Argus will resume on subsequent PRs once these changes land on main.
bokelley
left a comment
bokelley
left a comment
There was a problem hiding this comment.
The core publisher-owned mapping direction looks right. I'm withdrawing my earlier objection to the patch changeset: given that TMP has not reached production and no 3.1 TMP production deployment will ship on the superseded shape, treating this as a pre-production correction in the experimental surface is defensible. The inline threads below are the remaining design/schema issues I think we should close.
Before merge, please also link the companion adcp-go implementation and confirm coordinated advisory/implementation rollout. The specification change should not get ahead of the implementation-side mitigation.
| @@ -0,0 +1,15 @@ | |||
| --- | |||
| "adcontextprotocol": patch | |||
There was a problem hiding this comment.
I'm okay with patch here and am withdrawing my earlier request to make this minor. The important facts are that TMP has no production use yet and the corrected shape will be in place before any 3.1 TMP production deployment ships. Please state that pre-production fact explicitly in the changeset/PR rationale and frame this as a bounded pre-production correction, rather than relying primarily on #5729 as a general precedent for breaking patches.
There was a problem hiding this comment.
updated
| "description": "Map from `provider_id` to the ordered list of ad-server macro names (or equivalent destination identifiers for the surface) the publisher trafficks for that provider on this surface. Index N in this list receives `tmpx_providers[provider_id].values[N]` from the identity-match response. The list is provider-namespaced by convention — e.g. `provider_a: [\"PIN_TMPX_1\", \"PIN_TMPX_2\"]`, `provider_b: [\"NOVA_TMPX_1\"]` — so distinct providers do not collide on the same ad-server slot. The exact string is publisher-local: GAM adopters typically use uppercase snake-case macro names, VAST surfaces use URL macro tokens, DOOH surfaces use play-log field identifiers. When a response carries more values for a provider than this mapping has slots for that provider, the publisher's adapter MUST fail closed for that provider on that impression — none of that provider's values are fired into the ad-serving path and the adapter logs a configuration error. Other providers on the same response are unaffected. Fewer values than slots (partial fill) leave the trailing slots empty and the impression proceeds normally. A `provider_id` absent from this mapping (e.g. a newly onboarded provider not yet trafficked on this surface) MUST cause the adapter to drop that provider's values on this surface rather than firing them into an unconfigured destination.", | ||
| "additionalProperties": { | ||
| "type": "array", | ||
| "description": "Ordered destinations for this provider on this surface. Same length as, or longer than, the provider's registered TMPX chunk count (v1 cap = 2). Each entry is opaque to the protocol — it is the string the publisher's adapter substitutes into the ad request or play log.", |
There was a problem hiding this comment.
This refers to the provider's "registered TMPX chunk count," but this PR removes the only registration-time declaration from provider-registration.json. That leaves publishers unable to validate the mapping at startup or detect ordinal semantic drift. Please define a registration-time slot contract. My preference is stable provider-local slot IDs, with the publisher mapping (provider_id, slot_id) to its local destination. A minimum viable ordinal design would need a declared maximum chunk count/contract version plus an explicit invariant that shorter responses are ordered prefixes, never shifted or sparse.
There was a problem hiding this comment.
provider registers tmpx_slots: [string] (opaque provider-local IDs). Chunks are {slot_id, value} on both hops (shared tmpx-chunk.json). Publisher config is {provider_id: {slot_id: destination}} — a nested map.
| "eligible_package_ids", | ||
| "serve_window_sec" | ||
| ], | ||
| "additionalProperties": true |
There was a problem hiding this comment.
The split schemas do not currently enforce the hop split: with additionalProperties: true, this provider-facing schema still accepts legacy tmpx_macros and router-facing tmpx_providers; the router-facing schema likewise accepts tmpx_values. The composed protocol envelope also permits context, despite the description saying it is intentionally omitted. Please add explicit negative constraints (or another strict composition strategy) for wrong-hop/legacy fields on both schemas, plus negative validation vectors proving those fields are rejected. Otherwise the stated benefit that each hop's obligations are expressible in JSON Schema is not actually achieved.
There was a problem hiding this comment.
added not: {anyOf: […]} clauses on both response schemas reject the other hop’s fields plus context/ext envelope leakage
| "maxItems": 2 | ||
| }, | ||
| "tmpx_providers": { | ||
| "type": "object", |
There was a problem hiding this comment.
The description requires map keys to be valid registered provider_id values, but the schema accepts any property name. Please add propertyNames with the same ^[A-Za-z0-9_]+$ / length constraints used by provider-registration.json, and apply the same constraint to publisher-tmpx-config.json's mapping keys. Add a negative test for invalid keys so the three schemas cannot drift.
There was a problem hiding this comment.
propertyNames on tmpx_providers and both levels of tmpx_macro_mapping, tied to the provider_id/slot_id patterns in registration and chunk schemas.
Replaces the ordinal design with provider-declared slot IDs (Brian's
preference) and hardens the split schemas so wrong-hop, envelope-
extension, and out-of-charset inputs are rejected by validation rather
than left to convention.
Registration:
- provider-registration.json declares tmpx_slots: [string] — provider-
local opaque IDs (not ad-server macro names). Publishers key on
(provider_id, slot_id); distinct providers may reuse the same slot_id.
Response schemas:
- New tmpx-chunk.json ({slot_id, value}) shared between both hops.
- Provider→router carries tmpx_chunks: [TmpxChunk].
- Router→publisher carries tmpx_providers[provider_id].chunks: [TmpxChunk].
- Both hops add `not: {anyOf: [...]}` clauses that reject the other
hop's fields, legacy carriers (tmpx_values, tmpx_macros), and
envelope-extension fields (context, ext) that would otherwise slip
through additionalProperties: true.
- tmpx_providers keys carry propertyNames matching the provider_id
charset.
Publisher config:
- tmpx_macro_mapping becomes {provider_id: {slot_id: destination}}.
- propertyNames constrain both outer (provider_id) and inner (slot_id)
key charsets.
- Serve-time fail-closed rule and startup validation are described as
two stages of one rule so adopters cannot diverge on missing-slot
handling.
Fixtures:
- Seventeen new fixtures in example-validation-simple.test.cjs cover
wrong-hop fields on both schemas, context/ext bleed-through on both
hops, provider_id and slot_id charset violations on both maps, and
tmpx_slots duplicate detection.
Changeset reframed as a bounded pre-production correction — TMP has no
production use yet and the corrected shape ships before any 3.1 TMP
production deployment.
There was a problem hiding this comment.
Argus is not auto-reviewing this PR because it modifies protected paths that require human review (static/schemas/source/index.json, static/schemas/source/trusted-match/identity-match-response.json, static/schemas/source/trusted-match/provider-identity-match-response.json, static/schemas/source/trusted-match/provider-registration.json, static/schemas/source/trusted-match/publisher-tmpx-config.json, static/schemas/source/trusted-match/tmpx-chunk.json). A human reviewer should review and merge this PR; Argus will resume on subsequent PRs once these changes land on main.
|
Two items remain before I think this is ready for approval:
Before merge, please also link the companion adcp-go implementation and confirm the coordinated rollout, as previously discussed. |
… fixture parity - Strengthen `tmpx_chunks` description in provider-identity-match-response.json to normative MUST: router MUST validate slot contract before forwarding and MUST drop that provider's chunks atomically on any violation (no registration, duplicate, reordered, sparse, or unregistered slot IDs). - Add the same normative enforcement requirement to router-architecture.mdx TMPX collection paragraph, before the publisher-local destination names note. - Add 4 missing legacy-carrier negative fixtures to example-validation-simple.test.cjs: tmpx_values and tmpx_macros rejected on both hop schemas (provider→router and router→publisher), completing the not-clause coverage for all 6 forbidden fields. Total test count: 75 (was 71 before this PR's fixture additions). - Update changeset fixture count from thirteen to twenty and add router slot-contract enforcement paragraph.
|
Addressed items 1 and 2 in commit 1a0889e. Item 1 — Router slot-contract enforcement. Added the normative MUST requirement in two places:
The changeset also gains a Router slot-contract enforcement paragraph summarising the requirement for implementers reading the release notes. Item 2 — Legacy-carrier fixture parity. Added 4 missing
All 6 fields in the The count discrepancy (@bokelley counted 16, PR description said 17, changeset said 13) is now reconciled: the PR adds 20 fixtures across both test sections, and the changeset says "twenty". Item 3 — adcp-go companion link. @ohalushchak-exadel, can you link the companion adcp-go PR here? The comment asked for confirmation of coordinated rollout before merge. Generated by Claude Code |
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean re-review, no blocking findings.
Blocking findings
None.
Medium findings
None.
Delta since prior review is a single commit: router slot-contract prose strengthened to normative MUST on both provider-identity-match-response.json and router-architecture.mdx, changeset fixture count updated (thirteen→twenty), and four legacy-carrier negative fixtures added to complete not-clause coverage for all six forbidden fields. Reviewer verified all delta claims against the schemas: new fixtures target fields genuinely present in the not.anyOf clauses, the twenty-fixture count is accurate, charsets are consistent across the three schemas, and context/ext forbiddance is a deliberate tightening that does not conflict with the optional context field in protocol-envelope.json.
Decision table: no critical/high/medium findings (rows 1, 4–6, 8 do not fire); gated_paths is false (row 2 n/a — note review_decision is CHANGES_REQUESTED, but the gate only applies to gated paths, which this PR does not touch); high_risk is false (rows 3, 5 n/a); no prior decision (row 6 n/a); no no-auto-approve team match (row 7 n/a). Falls through to row 9 → approve.
Adds enforceProviderSlotContract — the router MUST introduced by adcontextprotocol/adcp#5971 (commit 1a0889e on the spec branch): a provider's emitted tmpx_chunks sequence must be an exact non-empty ordered prefix of the provider's registered tmpx_slots, else the router drops the provider's chunks atomically before forwarding into tmpx_providers. Uses a local tmpxChunkView projection so the logic and its tests stand alone until the schema pin bumps and tmproto.TmpxChunk is generated.
Adds enforceProviderSlotContract — the router MUST introduced by adcontextprotocol/adcp#5971 (commit 1a0889e on the spec branch): a provider's emitted tmpx_chunks sequence must be an exact non-empty ordered prefix of the provider's registered tmpx_slots, else the router drops the provider's chunks atomically before forwarding into tmpx_providers. The validation depends only on the emitted slot_id sequence; the caller extracts slot_ids from []tmproto.TmpxChunk at the call site once the schema pin bumps and the generated type lands.
Migrates the identity-agent and router-merge paths to the reshaped TMPX wire from adcontextprotocol/adcp#5971. Publisher-local destination names never travel between provider and router — providers declare stable provider-local slot_id tokens at registration and emit ordered {slot_id, value} chunks on the wire; the publisher's deployment configuration resolves (provider_id, slot_id) → local ad-server destination. Types (regenerated from the PR-branch schemas): - New TmpxChunk struct with SlotID + Value replaces TmpxMacro - Split response schemas → new ProviderIdentityMatchResponse (provider→router; carries TmpxChunks, no TmpxProviders/Tmpx) and reshaped IdentityMatchResponse (router→publisher; carries TmpxProviders[id].Chunks, no root TmpxChunks) - ProviderRegistration.TmpxSlots replaces TmpxMacros - New PublisherTmpxMacroMapping type for publisher-side deployment config Callers: - Sealer: macroNames → slotIDs, MacroEntries → ChunkEntries returning []TmpxChunk - Config: MacroNames → SlotIDs, TMPX_MACRO_NAMES → TMPX_SLOT_IDS, parseTmpxMacroNames → parseTmpxSlotIDs - Handler: emits ProviderIdentityMatchResponse with TmpxChunks (drops the deprecated single-`tmpx` field — not carried on the provider hop) - Router merge: reads resp.TmpxChunks, folds into TmpxProviderEntry{Chunks}, sources legacy `tmpx` from the first single-chunk value Generator changes to support the reshape: - go-overlays.json: rename the two split response schema types back to IdentityMatchResponse / ProviderIdentityMatchResponse and re-target the tmpx_providers overlay - schema.go: extend the Go-type validator regex to accept nested maps (map[string]map[string]string) so PublisherTmpxMacroMapping's inline slot-map field generates without an overlay
…5971) Wires the MUST from adcontextprotocol/adcp#5971: before folding a provider's TmpxChunks into tmpx_providers[provider_id].Chunks, validate that the emitted slot_id sequence is an exact non-empty ordered prefix of the provider's registered tmpx_slots. Break atomically on any mismatch (empty, longer, unregistered, reordered, sparse, duplicate) — other providers on the same response are unaffected. Log a warning so the misconfig is observable. - ProviderConfig gains TmpxSlots []string (populated from ProviderRegistration.TmpxSlots via ProviderConfigFromRegistration). - identityResult carries the provider's registeredSlots through the fan-out into the merge. - mergeIdentityResponses takes a parallel registeredSlots [][]string and consults enforceProviderSlotContract before folding each provider. - slot_contract.go moves from targeting/identityagent to router — the enforcement is a router concern; the helper is unchanged and its 10 test cases move with it. - tmproto/tmpx.go: TmpxMaxSlots comment now references tmpx_slots (the surface's new spelling). Two new merge-level tests cover the drop-atomically path (reordered sequence) and the no-registration path (chunks emitted with no registered slots).
@bokelley the draft for item 3 is up at adcontextprotocol/adcp-go#427, but need this PR to be merged and released as a version to actually update and un-draft adcp-go |
2 participants
Summary
Addresses #5947 — makes TMPX macro mapping publisher-owned deployment configuration rather than provider-owned protocol data. Closes the cross-provider name-hijack surface tracked as #5945 by construction: the router never accepts a destination name from an untrusted provider.
provider-registration.jsondeclarestmpx_slots: [string]— provider-local opaque IDs (e.g.["primary","secondary"]), NOT ad-server macro names. Distinct providers may reuse the sameslot_idbecause publisher lookup keys on(provider_id, slot_id). Ordering carries the ordered-prefix invariant.tmpx-chunk.json({slot_id, value})$ref'd from both hops:provider-identity-match-response.json(provider → router) carriestmpx_chunks: [TmpxChunk].identity-match-response.json(router → publisher) carriestmpx_providers[provider_id].chunks: [TmpxChunk].publisher-tmpx-config.json—tmpx_macro_mapping: {provider_id: {slot_id: destination}}. Publisher's adapter readschunks[], looks up(provider_id, chunk.slot_id) → destination, and fails closed for that provider on that impression when the mapping is missing an entry. Startup validation surfaces the same case as a warning before serve time.not: {anyOf: [...]}clauses that reject the other hop's fields, legacy carriers (tmpx_values,tmpx_macros), and envelope-extension fields (context,ext) that would otherwise leak throughadditionalProperties: true. Seventeen new fixtures intests/example-validation-simple.test.cjsprove every clause rejects the wrong shape.propertyNamesontmpx_providersand both levels oftmpx_macro_mappingmatch theprovider_idandslot_idcharsets from the registration schemas — the three schemas cannot drift.tmpx_macrosfromprovider-registration.json— publisher-local names never travel on the wire.Why 3.1.x, not 3.2
Bounded pre-production correction. TMP has no production use yet, and this corrected shape will be in place before any 3.1 TMP production deployment ships. That fact — not a general precedent — is the primary justification: the surface exists but has no live consumers to migrate, so the "shape change costs adopters" ledger is empty in practice.
x-status: experimental(seedocs/reference/experimental-status.mdx— "Inside 3.x, experimental surfaces MAY change in ways that stable surfaces cannot") is what makes the reshape technically permissible inside 3.x. The pre-production posture is what makes it practically safe. Closing #5945 in 3.2 only would leave the vulnerability in every in-support 3.1.x maintenance release with no fix path.The change lives on the single-source schema tree, so it rolls forward into 3.2 automatically — no 3.1.x fork to maintain.
Reviewer follow-ups addressed
slot_ids replace the ordinal design. Publishers get startup drift detection and self-describing chunks; the router never sees a destination name.contextandextare rejected on both hops so the identity privacy boundary is schema-enforced, not description-only.propertyNamesconstraints ontmpx_providersand both levels oftmpx_macro_mapping, keyed to theprovider_id/slot_idpatterns in the registration and chunk schemas.Test plan
npm run build:schemas— regeneratesdist/schemas/latest/cleanlynpm run test:schemas— 19/19 pass;$refcross-references resolve, registry consistent, sharedtmpx-chunk.jsonreferenced from both hopsnpm run test:examples— 71/71 pass, including 17 new positive + negative fixtures for the slot-ID designnpm run test:composed,test:json-schema,test:extensions,test:extension-schemas,test:error-handling,test:version-envelope,test:migrations,test:build-schemas-async-response-refsnpm run typechecktest:snippets— no new failures; 13 pre-existing failures live in media-buy/creative task-reference docs and are unrelated to this diffRelated
tmpx_providersmap🤖 Generated with Claude Code