Skip to content

fix: enforce propertyNames key pattern on the generated Signals schema - #40

Merged
damaz91 merged 1 commit into
Universal-Commerce-Protocol:mainfrom
vishkaty:enforce-propertynames
Aug 11, 2026
Merged

fix: enforce propertyNames key pattern on the generated Signals schema#40
damaz91 merged 1 commit into
Universal-Commerce-Protocol:mainfrom
vishkaty:enforce-propertynames

Conversation

@vishkaty

Copy link
Copy Markdown
Contributor

Description

propertyNames is not enforced on the one generated schema where it maps to an
extra-permitting object with named fields. quicktype's typescript-zod target
emits only the object shape, so both the key pattern (propertyNames) and the
extra-key retention (additionalProperties: true) are dropped.

Observed (on current main, committed models, src/spec_generated.ts):

const { CheckoutCreateRequestSignalsSchema } = require("./src/spec_generated");

CheckoutCreateRequestSignalsSchema.parse({
  "dev.ucp.buyer_ip": "1.2.3.4",
  "bogus KEY!": "x",
});
// succeeds; the malformed key is silently STRIPPED (z.object drops unknowns)

CheckoutCreateRequestSignalsSchema.parse({ "com.example.device_id": "abc" });
// succeeds but returns {}; the well-formed reverse-domain extra is LOST

Expected: the malformed key is rejected, and the well-formed reverse-domain
extra is preserved. signals.json requires every property name to match the
reverse-domain pattern ^[a-z][a-z0-9]*(?:\.[a-z][a-z0-9_]*)+$
(shopping/types/signals.json propertyNames.pattern, the same pattern as
shopping/types/reverse_domain_name.json), and it sets
additionalProperties: true, so a well-formed extra such as
com.example.device_id must be kept.

Why it happens / why CI did not catch it

signals.json declares propertyNames alongside named properties
(dev.ucp.buyer_ip, dev.ucp.user_agent) and additionalProperties: true.
quicktype emits it as a bare z.object({...}) with the two named fields.
z.object strips unknown keys by default, so extra keys are neither retained nor
checked against the key pattern. No test asserted key-pattern rejection, so the
dropped constraint shipped green.

Fix (source-driven, in the generation pipeline)

scripts/inject-schema-constraints.mjs already re-attaches the value constraints
quicktype drops (#34/#37). This extends it to the object-level propertyNames
key constraint: it scans the source schemas for objects that declare
propertyNames and carry named properties, reads the key pattern from the
source (inline pattern, or a $ref to e.g. reverse_domain_name.json — never
duplicated in code), and splices onto the matching z.object call:

.catchall(z.any())            // retain extras (additionalProperties: true)
.superRefine((value, ctx) => {
  for (const key of Object.keys(value)) {
    if (!/^[a-z][a-z0-9]*(?:\.[a-z][a-z0-9_]*)+$/.test(key)) {
      ctx.addIssue({ code: z.ZodIssueCode.custom, path: [key], message: ... });
    }
  }
})

.catchall(z.any()) keeps the reverse-domain extras a bare z.object would drop;
the .superRefine matches every property name (named fields and retained extras
alike) against the source pattern. The injected regex is byte-identical to
signals.json propertyNames.pattern.

RegExp.test is used deliberately. It implements JSON Schema's unanchored
pattern semantics, and for the source's ^...$-anchored pattern it matches
only end-of-input in ECMA-262 (no m flag), so a trailing-newline key
("com.example.k\n") is rejected. This is the JS analogue of the python-sdk#66
sibling fix, where Python's re.match had to become re.fullmatch because
re.match admits a trailing newline against a $-anchored pattern; JavaScript's
$ does not, and a pinned test asserts it.

Scope — propertyNames sites in the 2026-04-08 spec

The change deliberately handles only the propertyNames + named-properties
(extra-permitting object) shape. Every propertyNames site in the spec is
accounted for:

Source site Named props Generated shape Decision
shopping/types/signals.json (root) yes (dev.ucp.buyer_ip, dev.ucp.user_agent); additionalProperties: true z.objectCheckoutCreateRequestSignalsSchema (aliased LookupRequestSignalsSchema) Converted
ucp.json #/$defs/base services / capabilities / payment_handlers no (pure map) z.record(z.string(), V) in UcpResponseSchema; services also in UcpSchema Out of scope — see below
ucp.json #/$defs/requires capabilities no (pure map) not emitted (not reachable from the generation roots) Out of scope — not generated
ucp.json business supported_versions no (pure map) not emitted Out of scope — not generated
common/identity_linking.json config.scopes no (pure map) not emitted (capability config resolves to a generic record) Out of scope — not generated

Signals is the only propertyNames + named-properties object in the spec, so
the enforcement lands on it and its alias.

Note on the z.record registries (honest residual, not "already safe")

The pure dict-map registries that do project — UcpResponseSchema
capabilities / payment_handlers / services, and UcpSchema services
render as z.record(z.string(), V), whose z.string() key schema does not
enforce the reverse-domain pattern:

z.record(z.string(), z.any()).safeParse({ "bad KEY!": 1 }).success; // true

So in js-sdk these keys are currently unvalidated. This differs from python-sdk#66,
where the equivalent maps became dict[ReverseDomainName, V] and pydantic already
validates the keys — that reasoning does not carry over to zod. Enforcing
these needs a distinct mechanism (rewriting the z.record key schema to
z.record(z.string().regex(<pattern>), V)), not the object-scoped named-property
injector this PR extends. It is left as a scoped follow-up rather than folded in,
to keep this change focused; flagged here so it is not mistaken for already-safe.

minProperties (source: shopping/types/description.json,
shopping/types/available_payment_instrument.json) is a separate constraint
class with no propertyNames and is likewise left as a clean follow-up.

Known boundary

zod-core silently drops an own __proto__ key from every z.object (a
prototype-pollution safeguard) before .catchall / .superRefine run, so a
{"__proto__": ...} payload parses successfully with the key absent from the
output — dropped rather than rejected. The direction is safe: the key is not
preserved and the prototype is not polluted. This is an SDK-wide zod trait
(.passthrough() and .catchall() drop it identically), not specific to
Signals, so this PR pins the behavior with a test rather than special-casing one
schema with a raw-input wrapper. Every other pattern-violating own key —
including constructor and prototype — is rejected as normal; only the literal
__proto__ is affected.

Verification

  • Failing test first, on the committed models: malformed extra key accepted
    (stripped), reverse-domain extra lost. Green after regeneration. Kill-test:
    neutralizing the injector's propertyNames path turns the new semantic tests
    red (4 of 5), the named-field test stays green.
  • Rejects the malformed key; rejects the trailing-newline key ("com.example.k\n",
    anchor safety); preserves the well-formed reverse-domain extra; the named
    fields still populate; the alias enforces the same.
  • npm test: all green (48 tests).
  • Regenerated from the pinned spec (./generate_models.sh <ucp @ release/2026-04-08>)
    and normalized with the repo's pinned prettier hook; the diff is limited to the
    Signals schema, and a second regeneration is byte-identical, so the
    model-drift job stays green.
  • npm run build (cjs/esm/types) clean; full pre-commit on the changed files
    clean (prettier, codespell, whitespace, shebang/executable checks).

This is the js-sdk sibling of python-sdk#66 (the other SDK; prior art, not a
duplicate).

quicktype's typescript-zod target emits only an object's shape, so the
`propertyNames` key constraint and `additionalProperties: true` on
`shopping/types/signals.json` are both dropped from the generated
`CheckoutCreateRequestSignalsSchema` (src/spec_generated.ts).

Observed (current main, committed models):

  CheckoutCreateRequestSignalsSchema.parse({
    "dev.ucp.buyer_ip": "1.2.3.4", "bogus KEY!": "x",
  });
  // succeeds; "bogus KEY!" is silently stripped rather than rejected
  CheckoutCreateRequestSignalsSchema.parse({ "com.example.device_id": "a" });
  // succeeds but returns {}; the reverse-domain extra is lost

Expected: the malformed key is rejected and the reverse-domain extra is
preserved. signals.json requires every property name to match
`^[a-z][a-z0-9]*(?:\.[a-z][a-z0-9_]*)+$` (signals.json propertyNames.pattern,
the same pattern as shopping/types/reverse_domain_name.json), and sets
`additionalProperties: true`.

Fix (source-driven, in scripts/inject-schema-constraints.mjs): scan the source
schemas for objects that declare propertyNames alongside named properties, read
the key pattern from source (inline pattern or a $ref, never hand-copied), and
splice `.catchall(z.any()).superRefine(...)` onto the matching z.object so extras
are retained (additionalProperties: true) and every key is checked. RegExp.test
matches JSON Schema's unanchored pattern semantics and, for the `^...$`-anchored
source pattern, rejects a trailing-newline key in ECMA-262 (no `m` flag) — the JS
analogue of the python-sdk#66 sibling's re.match -> re.fullmatch fix; pinned by a
test.

Scope: Signals is the only propertyNames + named-properties object in the
2026-04-08 spec. The pure dict-map propertyNames registries (ucp.json
services/capabilities/payment_handlers) render as z.record(z.string(), V), which
does not enforce the key pattern; unlike python-sdk#66's dict[ReverseDomainName]
they are a genuine residual, left as a scoped follow-up needing a distinct
z.record-key mechanism. Other propertyNames sites (requires.capabilities,
supported_versions, identity_linking scopes) are not emitted.

Sibling of python-sdk#66. Regeneration from the pinned spec is byte-identical
(model-drift job stays green); npm test, npm run build, and pre-commit are clean.
@damaz91 damaz91 added status:needs-triage Signal that the PR is ready for human triage status:under-review and removed status:needs-triage Signal that the PR is ready for human triage labels Aug 10, 2026
@damaz91
damaz91 merged commit 78351ce into Universal-Commerce-Protocol:main Aug 11, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants