fix: reject user property name containing dot - #783
Conversation
A nested value is stored under its dotted path, so `{"a.b": …}` and
`{"a": {"b": …}}` flatten to the same attribute key. Reject the literal
dotted name at schema create so the two cannot collide.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: b2d4b99 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Updated comments to clarify the purpose of rejectDottedPropertyNames function.
There was a problem hiding this comment.
Pull request overview
This PR tightens user JSON schema validation by rejecting property names that contain a dot (.), preventing ambiguity between literal dotted keys (e.g. {"a.b": ...}) and nested objects (e.g. {"a": {"b": ...}}) when attributes are flattened and referenced via dotted paths.
Changes:
- Add validation in
NewJSONSchemato reject dotted property names within schemaproperties. - Add tests asserting dotted property names are rejected and ordinary nesting remains allowed.
- Add a changeset bumping
@zitadel/server(minor) to reflect the user-visible behavior change.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/domain/json_schema.go | Introduces schema validation to reject dotted property names (needs broader schema traversal to cover $defs/$ref). |
| internal/domain/json_schema_test.go | Adds tests for dotted property name rejection and acceptance of ordinary nesting (missing $defs/$ref case). |
| .changeset/reject-dotted-schema-property-names.md | Documents the behavior change and bumps @zitadel/server version. |
A schema fetched from its URL is persisted straight from a struct literal, without passing through NewJSONSchema. CreateSchemaByUrl and the runtime auto-fetch both land there, so the name guard has to run at that site as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The walk only descended through `properties`, so a name declared under `$defs`, `allOf`, or `items` and pulled in by `$ref` slipped past and still produced an ambiguous attribute key. Walk every node instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds propertyNames to the properties map in user-schema.json, and makes user-property.json recurse into its own properties map so the rule reaches every level rather than only the first. An editor validating against the shipped dialect now flags a dotted name while authoring. Drops the resolver fetch-path guard added earlier: the POST path is the one worth covering in Go, and the remote path is better handled by validating a fetched schema against the meta-schema. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Drops the Go walk in favour of the meta-schema rule. Coverage is narrower — a name hidden under $defs, allOf, or items is not caught — but the rule now lives with the dialect, so an editor flags it while authoring rather than the server rejecting it after the fact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vitorbari
changed the title
vitorbari
changed the title
fforootd
left a comment
fforootd
left a comment
There was a problem hiding this comment.
Non-blocking recommendations, details inline — approval separate.
Verified locally on the PR head: TestTenantSchemaValidator_ValidateAgainstMetaSchema passes (both new cases), all 122 config tests pass (drift audit, Ajv dialect compile, shipped defaults/presets). The new user-property.json self-reference makes the schema graph cyclic, which I checked in all three consumers: the Go compiler caches the schema before Resolve (json_schema.go), Ajv resolves via filename registration, and editors resolve relative to .zitadel/meta/. Also nice that this lands before dot-notation flow fields ship — no grandfathered names to migrate.
| "description": "The name of a single user attribute." | ||
| }, | ||
| "additionalProperties": { | ||
| "$ref": "user-property.json" |
There was a problem hiding this comment.
The wire-contract mirrors don't carry the new rule: user-property.yaml still has additionalProperties: true in its nested properties map, and user-schema.yaml has no propertyNames — so API docs and generated clients won't surface a constraint the server now rejects. The yamls deliberately drop the allOf for codegen compat, but propertyNames + pattern is plain OAS 3.1 and likely survives generators. Worth mirroring there in a follow-up, or at minimum stating the dot rule in the yaml description.
| A user schema property name must be a single attribute name and may no longer | ||
| contain a dot. The rule lives in the user-schema meta-schema, so an editor | ||
| validating against the shipped dialect flags it while authoring, and the server | ||
| rejects it on create. |
There was a problem hiding this comment.
Recommend one more sentence here: the additionalProperties: true → $ref recursion is a broader tightening than the dot rule. Nested property values must now be objects valid against UserProperty (and draft 2020-12), so e.g. a boolean subschema "foo": true or {"type": 123} two levels down passed before and is rejected now. That's the part most likely to surprise someone resubmitting an existing schema, so the release notes should mention it.
| "type": "object", | ||
| "description": "A map of additional properties for the user definition, where the key is the property name and the value is the property schema", | ||
| "propertyNames": { | ||
| "pattern": "^[^.]+$", |
There was a problem hiding this comment.
The changeset's headline claim is that editors flag this while authoring, but meta-schemas.test.ts has no negative case for the new rule (the branding dialect tests do assert negatives, e.g. the http:// logo URL). A two-line Ajv case — dotted name fails, clean name passes — would pin the editor-facing artifact independently of the Go suite.
| "type": "object", | ||
| "description": "A map of additional properties for the user definition, where the key is the property name and the value is the property schema", | ||
| "propertyNames": { | ||
| "pattern": "^[^.]+$", |
There was a problem hiding this comment.
On the "to be fixed later" note: the escape-hatch list is a bit wider than $defs/allOf/items — since UserProperty keeps top-level additionalProperties: true and embeds the full 2020-12 meta-schema, patternProperties, additionalProperties-as-schema, and oneOf branches are the same class. When picking it up: the textbook mechanism is a $dynamicAnchor: meta dialect extension, which makes the constraint recursive across every subschema position in one move; an imperative walk at flow-definition validation time may end up simpler.
Summary
Update the user-meta-schema to prevent user property names containing dot
..Motivation:
Nested properties will be referenced with a dot notation on the flow-definition, so a property name itself can not contain a dot.
Flow-definition example:
{ "name": "address", "fields": ["address.street", "address.city"]}Notes
propertieschain at every depth. A name hidden under$defs,allOf, oritemsis not caught. (to be fixed later)ValidateAgainstMetaSchemaruns. A schema the resolver fetches from a URLis persisted without it. Unchanged by this PR.