Skip to content

fix: enforce propertyNames on extra-allow generated models - #66

Open
vishkaty wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
vishkaty:enforce-signals-propertynames
Open

fix: enforce propertyNames on extra-allow generated models#66
vishkaty wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
vishkaty:enforce-signals-propertynames

Conversation

@vishkaty

@vishkaty vishkaty commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Description

propertyNames is not enforced on the generated models where it maps to an
extra="allow" object with named fields.

Observed (on current main, committed models):

from ucp_sdk.models.schemas.shopping.types.signals import Signals

Signals(**{"dev.ucp.buyer_ip": "1.2.3.4", "bogus KEY!": "x"})
# accepted; "bogus KEY!" is kept in model_extra

Expected: the malformed key is rejected. signals.json requires every
property name to match the reverse-domain pattern
^[a-z][a-z0-9]*(?:\.[a-z][a-z0-9_]*)+$ (signals.json propertyNames.pattern,
the same pattern as shopping/types/reverse_domain_name.json). Well-formed
reverse-domain extras (e.g. com.example.device_id) must still be preserved,
since the schema sets additionalProperties: true.

Why it happens

signals.json declares propertyNames alongside named properties and
additionalProperties: true, so datamodel-code-generator emits it as a
BaseModel with model_config=ConfigDict(extra="allow") and the two named
fields. Extra keys are allowed but never checked against the key pattern.

By contrast, an object with propertyNames but no named properties (e.g.
ucp.json capabilities / services / payment_handlers /
supported_versions) is emitted as dict[ReverseDomainName, V], where
pydantic already validates the keys. This PR closes only the extra="allow"
BaseModel gap.

Fix (source-driven, in the generation pipeline)

postprocess_models.py scans the preprocessed schemas for objects that declare
propertyNames and carry named properties, reads the key pattern from the
source schema (inline pattern, or a $ref to e.g. reverse_domain_name.json
— never duplicated in code), and injects a model_validator(mode="after") that
matches every model_extra key against the pattern. This mirrors and extends
the post-generation model_validator approach already merged for minProperties
(the #49 constraint class).

Key matching uses re.fullmatch, not re.match: with a $-anchored pattern
re.match admits a trailing newline (Signals(**{"com.example.k\n": "x"})
would slip through), whereas re.fullmatch agrees with pydantic-core /
ECMA-262 (JSON Schema's regex dialect) key semantics — the exact behavior the
dict[ReverseDomainName, V] path already applies to its keys. The scanner also
warns if a resolved pattern is not ^/$-anchored, so a future unanchored
pattern is not silently over-restricted.

Scope

The only propertyNames + named-properties node in the 2026-04-08 spec is
Signals, so the enforcement lands on Signals and its three generated
request variants
SignalsCreateRequest, SignalsUpdateRequest,
SignalsCompleteRequest. Explicitly out of scope:

  • The dict-keyed propertyNames maps in ucp.json
    (capabilities/services/payment_handlers/supported_versions) — already
    enforced via dict[ReverseDomainName, V] key validation.
  • identity_linking.json config.scopes — the surrounding capability degrades
    to Any due to a separate allOf/$ref resolution issue, so no
    propertyNames-bearing model is emitted for it. That is a different defect and
    is not addressed here.

Verification

  • Failing test first, on the committed model (malformed extra key accepted),
    then green after regeneration. Kill-test: removing the injector call turns the
    new semantic tests red.
  • Malformed key rejected; trailing-newline key (com.example.k\n) rejected;
    well-formed reverse-domain extra preserved in model_extra; the named fields
    (dev.ucp.buyer_ip, dev.ucp.user_agent) still populate. Enforcement is
    covered on all four Signals models.
  • pip install -e . + python -m unittest discover -s tests -p "test_*.py":
    all green.
  • Regenerated with ./generate_models.sh 2026-04-08; the diff is limited to the
    propertyNames enforcement on the four Signals models, and regeneration is
    byte-identical (the model-drift equality check stays clean).
  • Pinned pre-commit (ruff + ruff-format) clean.

Category (Required)

  • SDK: Language-specific SDK updates and releases. (Requires DevOps Maintainer approval)

signals.json declares propertyNames (reverse-domain keys) alongside named
properties and additionalProperties:true. datamodel-code-generator emits this
as class Signals(BaseModel) with model_config=ConfigDict(extra="allow") and the
two named fields, so unknown (extra) keys are never checked against the key
pattern.

Observed: Signals(**{"dev.ucp.buyer_ip": "1.2.3.4", "bogus KEY!": "x"}) is
accepted and "bogus KEY!" is kept in model_extra.
Expected: the malformed key is rejected, because signals.json requires every
property name to match the reverse-domain pattern
^[a-z][a-z0-9]*(?:\.[a-z][a-z0-9_]*)+$ (signals.json propertyNames.pattern, the
same pattern as reverse_domain_name.json). Well-formed reverse-domain extras
(e.g. com.example.device_id) must still be preserved under extra="allow".

The already-enforced sibling case is the dict-keyed maps (e.g. ucp.json
capabilities/services/payment_handlers/supported_versions), emitted as
dict[ReverseDomainName, V] where pydantic validates the keys. The gap is only
the extra-allow BaseModel shape: propertyNames declared on an object that also
has named properties.

This extends the post-generation model_validator approach added for
minProperties (Universal-Commerce-Protocol#49-class): postprocess_models.py scans the preprocessed schemas
for objects that declare propertyNames AND carry named properties, reads the key
pattern from the source schema (inline pattern or a $ref to e.g.
reverse_domain_name.json, never duplicated in code), and injects a
model_validator(mode="after") that matches every model_extra key against the
pattern with re.fullmatch. fullmatch (not re.match) is used so a $-anchored
pattern does not admit a trailing newline (re.match lets $ match before a final
\n); this agrees with pydantic-core / ECMA-262 (JSON Schema's regex dialect)
key semantics, the same behavior the dict-keyed map path already applies. The
check reaches the base model and its generated request variants (Signals,
SignalsCreateRequest, SignalsUpdateRequest, SignalsCompleteRequest).

Out of scope: identity_linking.json scopes map degrades to Any due to an
allOf/$ref resolution problem, so no propertyNames-bearing model is emitted for
it (a separate defect); and the dict-keyed maps above already enforce their key
pattern.

Regenerated with ./generate_models.sh 2026-04-08; the diff is limited to the
propertyNames enforcement on the four Signals models and regeneration is
byte-identical.
@damaz91 damaz91 added the status:needs-triage Signal that the PR is ready for human triage label Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:needs-triage Signal that the PR is ready for human triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants