diff --git a/.changeset/17852-record-proto-key-preparse-guard.md b/.changeset/17852-record-proto-key-preparse-guard.md new file mode 100644 index 0000000000..b1ed4c6c3a --- /dev/null +++ b/.changeset/17852-record-proto-key-preparse-guard.md @@ -0,0 +1,24 @@ +--- +'@objectstack/spec': minor +--- + +**BREAKING** for authored metadata — `ObjectSchema.fields` refuses a key named `__proto__`, `constructor` or `prototype`, and `AssignmentConfigSchema.assignments` (the `assignment` flow node's variable map) refuses a key named `__proto__` — both refused with a named, located error at parse time, rather than silently accepted and then silently mishandled (objectstack#17852, objectstack#18847). + +## Why + +zod's `z.record()` skips a `__proto__` own key entirely, above its own key schema — the record parser's `if (key === "__proto__") continue;` runs before `def.keyType._zod.run`, so no key grammar (a regex, `.refine()`, `.superRefine()`, even a key schema that rejects every string) can ever see that key. A document whose `fields` (or `assignments`) carried a `__proto__` own key — which `JSON.parse` produces routinely — used to parse as SUCCESS with that key silently missing from the output: the validator accepted a document and handed back a *different* document. `os build` writes the release artifact from that returned document, so the failure shape is success, silent, and irreversible into the shipped artifact. + +Two independent mechanisms close this, one per name class, because they are not reachable the same way: + +- `__proto__` is refused by a **pre-parse guard** that reads the raw input's own keys before the record ever parses, at both `ObjectSchema.fields` and `AssignmentConfigSchema.assignments`. +- `constructor` and `prototype` — which, unlike `__proto__`, DO reach the key schema unskipped — are refused by `ObjectSchema.fields`' own key grammar (they were ordinary lowercase words its regex already admitted). They are **not** refused at `AssignmentConfigSchema.assignments`: that slot's key type carries no grammar at all (`z.string().min(1)`), both names are legal flow-VARIABLE names measured to survive parse intact today, and no ruling narrows that slot's accept set for them — only its `__proto__` half moves. + +Measured: zero authored use of any of the three names as a `fields` key or an `assignments` variable name, across this repo, `examples/` and `objectui`. + +## Known gap, left open on purpose + +The guard runs at parse time only. It does not project into the published JSON Schema (`packages/spec/json-schema/**`) — the general gap that closes is tracked separately (objectstack#18670) and stays open after this change. + +Clause-②: yes (narrowing) + + diff --git a/content/docs/references/api/metadata.mdx b/content/docs/references/api/metadata.mdx index 0d10564574..20f2b973eb 100644 --- a/content/docs/references/api/metadata.mdx +++ b/content/docs/references/api/metadata.mdx @@ -945,7 +945,7 @@ Metadata query with filtering, sorting, and pagination | **systemFields** | `false \| { tenant?: boolean; audit?: boolean }` | optional | Opt out of, or selectively disable, registry-level system-field auto-injection. | | **datasource** | `string` | optional (default: `"default"`) | Target Datasource ID. "default" is the primary DB. | | **external** | `{ remoteName?: string; remoteSchema?: string; writable?: boolean; columnMap?: Record; … }` | optional | Remote table binding for federated (external) objects. | -| **fields** | `Record; description?: string; … }>` | ✅ | Field definitions map. Keys must be snake_case identifiers. | +| **fields** | `Record; description?: string; … }>` | ✅ | Field definitions map. Keys must be snake_case identifiers; "__proto__", "constructor" and "prototype" are refused. | | **indexes** | `{ name?: string; fields: string[]; unique?: boolean \| 'global' \| 'organization' }[]` | optional | Database performance indexes | | **fieldGroups** | `{ key: string; label: string; icon?: string; description?: string; … }[]` | optional | Ordered list of field groups (array order = display order). See ObjectFieldGroupSchema. | | **tenancy** | `{ enabled: boolean; tenantField?: string; organizationField?: string }` | optional | Multi-tenancy configuration for SaaS applications | diff --git a/content/docs/references/data/object.mdx b/content/docs/references/data/object.mdx index cde88732c5..9fffc852e3 100644 --- a/content/docs/references/data/object.mdx +++ b/content/docs/references/data/object.mdx @@ -150,7 +150,7 @@ const result = ApiMethod.parse(data); | **systemFields** | `false \| { tenant?: boolean; audit?: boolean }` | optional | Opt out of, or selectively disable, registry-level system-field auto-injection. | | **datasource** | `string` | optional (default: `"default"`) | Target Datasource ID. "default" is the primary DB. | | **external** | `{ remoteName?: string; remoteSchema?: string; writable?: boolean; columnMap?: Record; … }` | optional | Remote table binding for federated (external) objects. | -| **fields** | `Record; description?: string; … }>` | ✅ | Field definitions map. Keys must be snake_case identifiers. | +| **fields** | `Record; description?: string; … }>` | ✅ | Field definitions map. Keys must be snake_case identifiers; "__proto__", "constructor" and "prototype" are refused. | | **indexes** | `{ name?: string; fields: string[]; unique?: boolean \| 'global' \| 'organization' }[]` | optional | Database performance indexes | | **fieldGroups** | `{ key: string; label: string; icon?: string; description?: string; … }[]` | optional | Ordered list of field groups (array order = display order). See ObjectFieldGroupSchema. | | **tenancy** | `{ enabled: boolean; tenantField?: string; organizationField?: string }` | optional | Multi-tenancy configuration for SaaS applications | diff --git a/content/docs/references/system/migration.mdx b/content/docs/references/system/migration.mdx index 61e66cbca3..a610be963d 100644 --- a/content/docs/references/system/migration.mdx +++ b/content/docs/references/system/migration.mdx @@ -326,7 +326,7 @@ Create a new object | **systemFields** | `false \| { tenant?: boolean; audit?: boolean }` | optional | Opt out of, or selectively disable, registry-level system-field auto-injection. | | **datasource** | `string` | optional (default: `"default"`) | Target Datasource ID. "default" is the primary DB. | | **external** | `{ remoteName?: string; remoteSchema?: string; writable?: boolean; columnMap?: Record; … }` | optional | Remote table binding for federated (external) objects. | -| **fields** | `Record; description?: string; … }>` | ✅ | Field definitions map. Keys must be snake_case identifiers. | +| **fields** | `Record; description?: string; … }>` | ✅ | Field definitions map. Keys must be snake_case identifiers; "__proto__", "constructor" and "prototype" are refused. | | **indexes** | `{ name?: string; fields: string[]; unique?: boolean \| 'global' \| 'organization' }[]` | optional | Database performance indexes | | **fieldGroups** | `{ key: string; label: string; icon?: string; description?: string; … }[]` | optional | Ordered list of field groups (array order = display order). See ObjectFieldGroupSchema. | | **tenancy** | `{ enabled: boolean; tenantField?: string; organizationField?: string }` | optional | Multi-tenancy configuration for SaaS applications | @@ -611,7 +611,7 @@ Create a new object | **systemFields** | `false \| { tenant?: boolean; audit?: boolean }` | optional | Opt out of, or selectively disable, registry-level system-field auto-injection. | | **datasource** | `string` | optional (default: `"default"`) | Target Datasource ID. "default" is the primary DB. | | **external** | `{ remoteName?: string; remoteSchema?: string; writable?: boolean; columnMap?: Record; … }` | optional | Remote table binding for federated (external) objects. | -| **fields** | `Record; description?: string; … }>` | ✅ | Field definitions map. Keys must be snake_case identifiers. | +| **fields** | `Record; description?: string; … }>` | ✅ | Field definitions map. Keys must be snake_case identifiers; "__proto__", "constructor" and "prototype" are refused. | | **indexes** | `{ name?: string; fields: string[]; unique?: boolean \| 'global' \| 'organization' }[]` | optional | Database performance indexes | | **fieldGroups** | `{ key: string; label: string; icon?: string; description?: string; … }[]` | optional | Ordered list of field groups (array order = display order). See ObjectFieldGroupSchema. | | **tenancy** | `{ enabled: boolean; tenantField?: string; organizationField?: string }` | optional | Multi-tenancy configuration for SaaS applications | diff --git a/packages/spec/dropped-refinements.baseline.json b/packages/spec/dropped-refinements.baseline.json index 09732185ca..bd2f0e2cb7 100644 --- a/packages/spec/dropped-refinements.baseline.json +++ b/packages/spec/dropped-refinements.baseline.json @@ -3,7 +3,7 @@ "measured": { "zod": "4.4.3", "publishedSchemasWithDroppedRefinements": 204, - "droppedRefinementSites": 560, + "droppedRefinementSites": 569, "refinementSitesThatDidProject": 357, "refinementSitesWithNoJsonFormToCompare": 9 }, @@ -66,8 +66,9 @@ "manifest.objectExtensions.element", "manifest.objects.element", "manifest.objects.element.fieldGroups", - "manifest.objects.element.fields.valueType", - "manifest.objects.element.fields.valueType.currencyConfig", + "manifest.objects.element.fields.out.keyType", + "manifest.objects.element.fields.out.valueType", + "manifest.objects.element.fields.out.valueType.currencyConfig", "manifest.objects.element.lifecycle", "manifest.pages.element", "manifest.pages.element.slots.header.options[0].in.type", @@ -157,8 +158,9 @@ "data.options[1].manifest.objectExtensions.element", "data.options[1].manifest.objects.element", "data.options[1].manifest.objects.element.fieldGroups", - "data.options[1].manifest.objects.element.fields.valueType", - "data.options[1].manifest.objects.element.fields.valueType.currencyConfig", + "data.options[1].manifest.objects.element.fields.out.keyType", + "data.options[1].manifest.objects.element.fields.out.valueType", + "data.options[1].manifest.objects.element.fields.out.valueType.currencyConfig", "data.options[1].manifest.objects.element.lifecycle", "data.options[1].manifest.pages.element", "data.options[1].manifest.pages.element.slots.header.options[0].in.type", @@ -238,8 +240,9 @@ "options[1].manifest.objectExtensions.element", "options[1].manifest.objects.element", "options[1].manifest.objects.element.fieldGroups", - "options[1].manifest.objects.element.fields.valueType", - "options[1].manifest.objects.element.fields.valueType.currencyConfig", + "options[1].manifest.objects.element.fields.out.keyType", + "options[1].manifest.objects.element.fields.out.valueType", + "options[1].manifest.objects.element.fields.out.valueType.currencyConfig", "options[1].manifest.objects.element.lifecycle", "options[1].manifest.pages.element", "options[1].manifest.pages.element.slots.header.options[0].in.type", @@ -280,8 +283,9 @@ "data.packages.element.options[1].manifest.objectExtensions.element", "data.packages.element.options[1].manifest.objects.element", "data.packages.element.options[1].manifest.objects.element.fieldGroups", - "data.packages.element.options[1].manifest.objects.element.fields.valueType", - "data.packages.element.options[1].manifest.objects.element.fields.valueType.currencyConfig", + "data.packages.element.options[1].manifest.objects.element.fields.out.keyType", + "data.packages.element.options[1].manifest.objects.element.fields.out.valueType", + "data.packages.element.options[1].manifest.objects.element.fields.out.valueType.currencyConfig", "data.packages.element.options[1].manifest.objects.element.lifecycle", "data.packages.element.options[1].manifest.pages.element", "data.packages.element.options[1].manifest.permissions.element.objects.valueType.out", @@ -319,9 +323,10 @@ "data.actions.element.in", "data.actions.element.in.params.element.in", "data.fieldGroups", - "data.fields.valueType", - "data.fields.valueType.currencyConfig", - "data.fields.valueType.relatedListFilter.lazy", + "data.fields.out.keyType", + "data.fields.out.valueType", + "data.fields.out.valueType.currencyConfig", + "data.fields.out.valueType.relatedListFilter.lazy", "data.lifecycle", "data.listViews.valueType", "data.listViews.valueType.bulkActionDefs.element", @@ -378,7 +383,7 @@ }, "automation/AssignmentConfig": { "sites": [ - "assignments.valueType" + "assignments.out.valueType" ] }, "automation/AssignmentValue": { @@ -678,9 +683,10 @@ "actions.element.in", "actions.element.in.params.element.in", "fieldGroups", - "fields.valueType", - "fields.valueType.currencyConfig", - "fields.valueType.relatedListFilter.lazy", + "fields.out.keyType", + "fields.out.valueType", + "fields.out.valueType.currencyConfig", + "fields.out.valueType.relatedListFilter.lazy", "lifecycle", "listViews.valueType", "listViews.valueType.bulkActionDefs.element", @@ -948,7 +954,8 @@ "operations.element.options[3].object.actions.element.in", "operations.element.options[3].object.actions.element.in.params.element.in", "operations.element.options[3].object.fieldGroups", - "operations.element.options[3].object.fields.valueType", + "operations.element.options[3].object.fields.out.keyType", + "operations.element.options[3].object.fields.out.valueType", "operations.element.options[3].object.lifecycle", "operations.element.options[3].object.listViews.valueType", "operations.element.options[3].object.listViews.valueType.bulkActionDefs.element", @@ -965,9 +972,10 @@ "object.actions.element.in", "object.actions.element.in.params.element.in", "object.fieldGroups", - "object.fields.valueType", - "object.fields.valueType.currencyConfig", - "object.fields.valueType.relatedListFilter.lazy", + "object.fields.out.keyType", + "object.fields.out.valueType", + "object.fields.out.valueType.currencyConfig", + "object.fields.out.valueType.relatedListFilter.lazy", "object.lifecycle", "object.listViews.valueType", "object.listViews.valueType.bulkActionDefs.element", @@ -1007,7 +1015,8 @@ "options[3].object.actions.element.in", "options[3].object.actions.element.in.params.element.in", "options[3].object.fieldGroups", - "options[3].object.fields.valueType", + "options[3].object.fields.out.keyType", + "options[3].object.fields.out.valueType", "options[3].object.lifecycle", "options[3].object.listViews.valueType", "options[3].object.listViews.valueType.bulkActionDefs.element", diff --git a/packages/spec/src/automation/builtin-node-config.test.ts b/packages/spec/src/automation/builtin-node-config.test.ts index 78512a2b26..243a9ad93d 100644 --- a/packages/spec/src/automation/builtin-node-config.test.ts +++ b/packages/spec/src/automation/builtin-node-config.test.ts @@ -610,3 +610,58 @@ describe('assignment value envelope — an evaluated slot requires what the engi expect(ExpressionSchema.safeParse(BLANK_SOURCE).success).toBe(true); }); }); + +/** + * `AssignmentConfigSchema.assignments` — the `__proto__` half of #17852, + * filed on its own as #18847 and folded back into this ruling once PR #18688 + * released this file (maintainer ruling A/narrow, comment 5725370319). + * + * `__proto__` ONLY. This slot's key type is `z.string().min(1)` — no + * grammar — so unlike `ObjectSchema.fields` there is no key-refusal half to + * add: `constructor` and `prototype` are legal flow-VARIABLE names today and + * this ruling does not narrow that accept set. `__proto__` is refused for the + * same structural reason as the sibling slot: `z.record()`'s open-key branch + * skips it before any key schema — including `.min(1)` — ever runs. + */ +describe('AssignmentConfigSchema.assignments — __proto__ pre-parse guard, constructor/prototype UNCHANGED (#17852 / #18847)', () => { + it('refuses `assignments` carrying a `__proto__` own key, named at `assignments.__proto__`', () => { + // `JSON.parse` is what makes `__proto__` an OWN enumerable key — an + // object literal's `{ __proto__: ... }` sets the actual prototype + // instead, and would never reach `z.record()`'s open-key loop as a key + // at all. + const config = JSON.parse('{"assignments":{"total":"{amount}","__proto__":"{evil}"}}'); + const result = AssignmentConfigSchema.safeParse(config); + expect(result.success).toBe(false); + if (result.success) return; + const issue = result.error.issues.find((i) => i.path.join('.') === 'assignments.__proto__'); + expect(issue).toBeDefined(); + expect(issue?.code).toBe('custom'); + expect(issue?.message).toMatch(/__proto__/); + }); + + it('refuses `assignments` that is `__proto__` ALONE — no sibling key masks the drop', () => { + const config = JSON.parse('{"assignments":{"__proto__":"{evil}"}}'); + const result = AssignmentConfigSchema.safeParse(config); + expect(result.success).toBe(false); + if (result.success) return; + expect(result.error.issues.some((i) => i.path.join('.') === 'assignments.__proto__')).toBe(true); + }); + + it.each(['constructor', 'prototype'])( + 'PRESERVATION: `%s` remains a legal flow-variable name — no ruling narrowed this slot\'s accept set', + (name) => { + const result = AssignmentConfigSchema.safeParse({ assignments: { [name]: '{x}' } }); + expect(result.success).toBe(true); + if (!result.success) return; + expect((result.data.assignments as Record | undefined)?.[name]).toBe('{x}'); + }, + ); + + it('an absent `assignments` key still parses (the slot stays optional)', () => { + expect(AssignmentConfigSchema.safeParse({}).success).toBe(true); + }); + + it('an ordinary `assignments` map with no reserved names still parses', () => { + expect(AssignmentConfigSchema.safeParse({ assignments: { total: '{amount}' } }).success).toBe(true); + }); +}); diff --git a/packages/spec/src/automation/builtin-node-config.zod.ts b/packages/spec/src/automation/builtin-node-config.zod.ts index ccfd582705..72732bd015 100644 --- a/packages/spec/src/automation/builtin-node-config.zod.ts +++ b/packages/spec/src/automation/builtin-node-config.zod.ts @@ -82,6 +82,7 @@ import { z } from 'zod'; import { EvaluatedExpressionSchema } from '../shared/expression.zod'; import { lazySchema } from '../shared/lazy-schema'; import { strictObject } from '../shared/strict-object'; +import { refuseRecordProtoKey } from '../shared/record-proto-key-guard'; import { isExpressionEnvelopeShaped } from './flow-node-expression-paths'; /** What a rejected key on these contracts silently did before #4001 批 9. */ @@ -920,13 +921,21 @@ export const ASSIGNMENT_ARRAY_FORM_PRESCRIPTION = */ export const AssignmentConfigSchema = lazySchema(() => z.object({ /** Variable name → value; the canonical authoring surface. */ - assignments: z.record(z.string().min(1), AssignmentValueSchema, { - // The array form is a TYPE error on this slot; the message is the - // prescription, carried on the record's own `invalid_type` issue because - // an object-level refinement never runs once a property has failed its - // type (Zod aborts the object) — measured, not assumed. - error: (issue) => (Array.isArray(issue.input) ? ASSIGNMENT_ARRAY_FORM_PRESCRIPTION : undefined), - }).optional() + assignments: refuseRecordProtoKey( + z.record(z.string().min(1), AssignmentValueSchema, { + // The array form is a TYPE error on this slot; the message is the + // prescription, carried on the record's own `invalid_type` issue because + // an object-level refinement never runs once a property has failed its + // type (Zod aborts the object) — measured, not assumed. + error: (issue) => (Array.isArray(issue.input) ? ASSIGNMENT_ARRAY_FORM_PRESCRIPTION : undefined), + }), + // [objectstack#18847] `__proto__` ONLY. This slot's key type carries no + // grammar (`z.string().min(1)`), so `constructor` and `prototype` are + // legal flow-variable names today and are left legal — only `__proto__` + // is structurally unreachable by any key schema (see + // `refuseRecordProtoKey`'s docblock), so it alone is refused here. + 'assignments', + ).optional() .describe('Variables to set: each key is a variable name, each value a `{token}` template, a CEL value envelope, or a literal'), }) .catchall(z.unknown())); diff --git a/packages/spec/src/compose-stacks-merge-collection-refusal.test.ts b/packages/spec/src/compose-stacks-merge-collection-refusal.test.ts index b1f65674e8..474849802e 100644 --- a/packages/spec/src/compose-stacks-merge-collection-refusal.test.ts +++ b/packages/spec/src/compose-stacks-merge-collection-refusal.test.ts @@ -210,7 +210,19 @@ describe('composeStacks — the other two strategies are unchanged', () => { }); describe('the refusal set is derived from ObjectSchema.shape — pinned in both directions', () => { - /** Independent walk: strip wrappers, read through lazy/pipe, any union member counts. */ + /** + * Independent walk: strip wrappers, read through lazy/pipe, any union + * member counts. + * + * [objectstack#17852] A `pipe`'s two sides read differently depending on + * which combinator built it: `.pipe(x)` puts the ORIGINAL type in `in` and + * a derived one in `out`, while `z.preprocess(fn, schema)` puts a + * transform STAGE in `in` and the real, validated schema in `out` — the + * exact shape `ObjectSchema.fields` is wrapped in now (the pre-parse + * `__proto__` guard). Checking only `in`, as this walk did before, found a + * `transform` node for `fields` and silently stopped reporting it as a + * record. Checking both sides covers either convention. + */ function isCollection(schema: unknown, depth = 0): boolean { if (depth > 8) return false; const def = (schema as { _zod?: { def?: Record } })._zod?.def; @@ -219,7 +231,7 @@ describe('the refusal set is derived from ObjectSchema.shape — pinned in both if (type === 'array' || type === 'record') return true; if (['optional', 'nullable', 'default', 'prefault', 'readonly', 'nonoptional', 'catch'].includes(type)) return isCollection(def!.innerType, depth + 1); if (type === 'lazy') return isCollection((def!.getter as () => unknown)(), depth + 1); - if (type === 'pipe') return isCollection(def!.in, depth + 1); + if (type === 'pipe') return isCollection(def!.in, depth + 1) || isCollection(def!.out, depth + 1); if (type === 'union') return (def!.options as unknown[]).some((o) => isCollection(o, depth + 1)); return false; } diff --git a/packages/spec/src/data/object.test.ts b/packages/spec/src/data/object.test.ts index 82cf719a93..9ba594817a 100644 --- a/packages/spec/src/data/object.test.ts +++ b/packages/spec/src/data/object.test.ts @@ -2643,3 +2643,96 @@ describe('managedBy: retiring the overloaded `system` bucket (#3355)', () => { }); }); }); + +/** + * `ObjectSchema.fields` — the three JS-prototype names (objectstack#17852, + * maintainer ruling A/narrow, comment 5725370319). + * + * `z.record()`'s open-key branch skips `__proto__` with an unconditional + * `continue` ABOVE the key schema (zod v4 core, the record parser), so no key + * grammar — regex, `.refine()`, `.superRefine()` — can ever see it: a document + * whose `fields` carries `__proto__` used to parse as SUCCESS with the key + * silently missing from the output. `refuseRecordProtoKey` closes that by + * inspecting the RAW input before the record ever runs. `constructor` and + * `prototype` DO reach the key schema (they are ordinary lowercase words the + * snake_case regex already admitted) and are refused there instead. + * + * These pin BEHAVIOUR, not a version string (the ruling's own instruction): + * a zod bump that silently changed the `__proto__` skip, or that started + * letting `constructor`/`prototype` through some other path, breaks these + * without anyone reading zod's changelog first. + */ +describe('ObjectSchema.fields — __proto__ / constructor / prototype key refusal (#17852)', () => { + // `JSON.parse` is what makes `__proto__` land as an OWN enumerable key + // (an object literal's `{ __proto__: ... }` sets the actual prototype + // instead) — the exact shape the original defect report measured and the + // shape a JSON request body always produces. + const docWithProtoField = () => + JSON.parse( + '{"name":"lead","label":"Lead","fields":{"title":{"type":"text","label":"Title"},"__proto__":{"type":"text","label":"P"}}}', + ); + + it('refuses a document whose `fields` carries a `__proto__` own key', () => { + const result = ObjectSchema.safeParse(docWithProtoField()); + expect(result.success).toBe(false); + if (result.success) return; + const issue = result.error.issues.find((i) => i.path.join('.') === 'fields.__proto__'); + expect(issue).toBeDefined(); + expect(issue?.message).toMatch(/__proto__/); + expect(issue?.message).toMatch(/z\.record\(\)/); + }); + + it('refuses a document whose `fields` is `__proto__` ALONE (no other key masks the drop)', () => { + const result = ObjectSchema.safeParse( + JSON.parse('{"name":"lead","label":"Lead","fields":{"__proto__":{"type":"text","label":"P"}}}'), + ); + expect(result.success).toBe(false); + if (result.success) return; + expect(result.error.issues.some((i) => i.path.join('.') === 'fields.__proto__')).toBe(true); + }); + + it('never lets `fields.__proto__` reach the key grammar\'s own regex message', () => { + // The regression this guards: a key-grammar-only fix (the withdrawn 甲 + // ruling) cannot ever see `__proto__`, so if this ever starts asserting + // the SNAKE_CASE regex message instead of the pre-parse guard's own, the + // guard has been bypassed (e.g. reordered behind the record). + const result = ObjectSchema.safeParse(docWithProtoField()); + expect(result.success).toBe(false); + if (result.success) return; + const protoIssue = result.error.issues.find((i) => i.path.join('.') === 'fields.__proto__'); + expect(protoIssue?.code).toBe('custom'); + }); + + it.each(['constructor', 'prototype'])( + 'refuses `%s` as a fields key via the key grammar (reaches def.keyType._zod.run, unlike `__proto__`)', + (reserved) => { + const result = ObjectSchema.safeParse({ + name: 'lead', + label: 'Lead', + fields: { + title: { type: 'text', label: 'Title' }, + [reserved]: { type: 'text', label: 'Reserved' }, + }, + }); + expect(result.success).toBe(false); + if (result.success) return; + const issue = result.error.issues.find((i) => i.path.join('.') === `fields.${reserved}`); + expect(issue).toBeDefined(); + expect(issue?.code).toBe('invalid_key'); + // The key-grammar refusal's own message is nested under `.issues` — + // the top-level `invalid_key` issue's own `.message` is zod's fixed + // "Invalid key in record", so the reason lives one level down. + const nested = issue?.code === 'invalid_key' ? issue.issues : undefined; + expect(nested?.[0]?.message).toMatch(/constructor.*prototype|prototype.*constructor/s); + }, + ); + + it('still accepts an ordinary document with no reserved field names', () => { + const result = ObjectSchema.safeParse({ + name: 'lead', + label: 'Lead', + fields: { title: { type: 'text', label: 'Title' } }, + }); + expect(result.success).toBe(true); + }); +}); diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index 83a554dce1..ef29362325 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -15,6 +15,7 @@ import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { strictObject } from '../shared/strict-object'; import { ProtectionSchema } from '../shared/protection.zod'; import { retiredKey } from '../shared/retired-key'; +import { refuseRecordProtoKey } from '../shared/record-proto-key-guard'; import { FIELD_GROUP_KEY_PATTERN } from './field-group-layout'; export const ApiMethod = z.enum([ 'get', 'list', // Read @@ -1961,9 +1962,26 @@ const ObjectSchemaBase = strictObject( /** * Data Model */ - fields: z.record(z.string().regex(/^[a-z_][a-z0-9_]*$/, { - message: 'Field names must be lowercase snake_case (e.g., "first_name", "company", "annual_revenue")', - }), FieldSchema).describe('Field definitions map. Keys must be snake_case identifiers.'), + fields: refuseRecordProtoKey( + z.record( + z.string() + .regex(/^[a-z_][a-z0-9_]*$/, { + message: 'Field names must be lowercase snake_case (e.g., "first_name", "company", "annual_revenue")', + }) + // [objectstack#17852] `__proto__` cannot reach this key schema at + // all — zod's record parser skips it before the key ever runs (see + // `refuseRecordProtoKey`, which refuses it on the raw input + // instead). `constructor` and `prototype` DO reach here (ordinary + // lowercase words the regex above already admits), so they are + // refused explicitly — the changeset's "three JS-prototype names" + // sentence is only true once both mechanisms are in place. + .refine((key) => key !== 'constructor' && key !== 'prototype', { + message: 'Field names must not be "constructor" or "prototype" (reserved JavaScript prototype property names).', + }), + FieldSchema, + ), + 'fields', + ).describe('Field definitions map. Keys must be snake_case identifiers; "__proto__", "constructor" and "prototype" are refused.'), indexes: z.array(IndexSchema).optional().describe('Database performance indexes'), /** diff --git a/packages/spec/src/shared/record-proto-key-guard.test.ts b/packages/spec/src/shared/record-proto-key-guard.test.ts new file mode 100644 index 0000000000..4283e5a16a --- /dev/null +++ b/packages/spec/src/shared/record-proto-key-guard.test.ts @@ -0,0 +1,93 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { z } from 'zod'; +import { describe, expect, it } from 'vitest'; + +import { refuseRecordProtoKey } from './record-proto-key-guard'; + +/** + * Isolated pin for the guard itself, independent of either real consumer + * (`ObjectSchema.fields`, `AssignmentConfigSchema.assignments` — objectstack + * #17852 / #18847). Those two files pin the guard wired into a real + * authoring surface; this file pins the guard's own contract against a + * minimal record so a future change to either consumer schema cannot mask a + * regression here. + * + * These assertions are BEHAVIOUR pins, not a zod-version pin (as ordered): + * `packages/spec/package.json` pins `zod` at `^4.4.3`, and a bump inside + * that range must not silently change what gets refused. + */ +describe('refuseRecordProtoKey', () => { + const Guarded = refuseRecordProtoKey(z.record(z.string(), z.string()), 'things'); + + it('refuses a `__proto__` own key with a named, self-locating issue', () => { + // JSON.parse is what makes `__proto__` an OWN enumerable key — an object + // literal's `{ __proto__: ... }` sets the actual prototype instead and + // never reaches the record's open-key loop as a key at all. + const input = JSON.parse('{"a":"1","__proto__":"2"}'); + const result = Guarded.safeParse(input); + expect(result.success).toBe(false); + if (result.success) return; + expect(result.error.issues).toHaveLength(1); + const [issue] = result.error.issues; + expect(issue.code).toBe('custom'); + expect(issue.path).toEqual(['__proto__']); + // Named: which slot, which key. + expect(issue.message).toContain('`things`'); + expect(issue.message).toContain('__proto__'); + }); + + it('refuses a record that is `__proto__` ALONE — no sibling key masks the drop', () => { + const input = JSON.parse('{"__proto__":"2"}'); + const result = Guarded.safeParse(input); + expect(result.success).toBe(false); + }); + + it('the underlying record really would have silently dropped it — the control this guard exists to fail', () => { + // Same key type, no guard: proves the defect is real on the pinned zod, + // not merely asserted from the docblock's quoted source excerpt. + const Unguarded = z.record(z.string(), z.string()); + const input = JSON.parse('{"a":"1","__proto__":"2"}'); + const result = Unguarded.safeParse(input); + expect(result.success).toBe(true); + if (!result.success) return; + expect(Reflect.ownKeys(result.data)).toEqual(['a']); + }); + + it('leaves an ordinary record with no `__proto__` key untouched', () => { + const result = Guarded.safeParse({ a: '1', b: '2' }); + expect(result.success).toBe(true); + if (!result.success) return; + expect(result.data).toEqual({ a: '1', b: '2' }); + }); + + it('does not choke on non-object input — the record schema still reports its own type error', () => { + const result = Guarded.safeParse('nope'); + expect(result.success).toBe(false); + if (result.success) return; + expect(result.error.issues[0]!.code).toBe('invalid_type'); + }); + + it('does not choke on null or an array', () => { + expect(Guarded.safeParse(null).success).toBe(false); + expect(Guarded.safeParse([1, 2]).success).toBe(false); + }); + + it('composes with `.optional()` the same way the raw record does — undefined never runs the guard', () => { + const OptionalGuarded = refuseRecordProtoKey(z.record(z.string(), z.string()), 'things').optional(); + expect(OptionalGuarded.safeParse(undefined).success).toBe(true); + }); + + it('preserves the inner schema\'s own error option (`{ error }` still fires for its own cases)', () => { + const WithCustomError = refuseRecordProtoKey( + z.record(z.string(), z.string(), { + error: (issue) => (Array.isArray(issue.input) ? 'no arrays here' : undefined), + }), + 'things', + ); + const result = WithCustomError.safeParse(['nope']); + expect(result.success).toBe(false); + if (result.success) return; + expect(result.error.issues[0]!.message).toBe('no arrays here'); + }); +}); diff --git a/packages/spec/src/shared/record-proto-key-guard.ts b/packages/spec/src/shared/record-proto-key-guard.ts new file mode 100644 index 0000000000..de163dc718 --- /dev/null +++ b/packages/spec/src/shared/record-proto-key-guard.ts @@ -0,0 +1,105 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { z } from 'zod'; + +/** + * `refuseRecordProtoKey` — a pre-parse guard that refuses a `__proto__` own + * key on the RAW input, before `z.record()` ever gets to run its key schema. + * + * ## Why this exists (objectstack#17852) + * + * `$ZodRecord`'s open-key branch (zod v4 core, the record parser) reads: + * + * ```js + * for (const key of Reflect.ownKeys(input)) { + * if (key === "__proto__") continue; // <-- runs BEFORE the key schema + * if (!Object.prototype.propertyIsEnumerable.call(input, key)) continue; + * let keyResult = def.keyType._zod.run({ value: key, issues: [] }, ctx); + * ... + * } + * ``` + * + * The `continue` sits above `def.keyType._zod.run`, so **no key schema can + * ever see a `__proto__` key** — not a regex, not `.refine()`, not + * `.superRefine()`, not even a key schema that rejects every string. A + * document whose record carries `__proto__` as an own key (which + * `JSON.parse` produces routinely) parses as SUCCESS and the key is + * silently missing from the output — the record accepted a document and + * handed back a different one. Tightening the key schema does nothing for + * this one name; the only place left to refuse it is the raw input, ahead + * of the record entirely. That is what this wrapper does. + * + * `constructor` and `prototype` are deliberately NOT handled here: unlike + * `__proto__`, both reach the key schema unskipped, so a slot that wants to + * refuse them too does it in its own key grammar instead (see + * `ObjectSchema.fields` in `data/object.zod.ts`) — adding them to this guard + * would refuse a name for one slot (`assignments`) whose accept set no + * ruling has narrowed. + * + * @param schema - the `z.record(...)` (or any schema) to guard. The return + * type is cast back to `Schema` itself, matching the precedent at + * `ObjectSchema.apiMethods` (this file's `data/object.zod.ts` neighbour): + * a raw `z.preprocess(fn, schema)` would widen the AUTHORING (input) type + * to `unknown`, losing autocomplete/type-checking for every author who + * writes this slot as an object literal. The runtime guard is real; only + * the declared TS shape is preserved. + * @param slotLabel - the authored surface name, echoed in the refusal so a + * reader learns which slot rejected the document (e.g. `'fields'`). + */ +export function refuseRecordProtoKey( + schema: Schema, + slotLabel: string, +): Schema { + const guarded = z.preprocess((value, ctx) => { + if ( + value !== null && + typeof value === 'object' && + Reflect.ownKeys(value).some( + (key) => key === '__proto__' && Object.prototype.propertyIsEnumerable.call(value, key), + ) + ) { + ctx.addIssue({ + code: 'custom', + path: ['__proto__'], + message: + `\`${slotLabel}\` cannot contain a key named "__proto__". zod's z.record() ` + + 'silently drops this key from its parse output while reporting success ' + + '(the document is accepted and a DIFFERENT document, missing this key, is ' + + 'returned) — so it is refused here instead of being silently corrupted. ' + + 'Rename the key.', + }); + } + return value; + }, schema); + + // [objectstack#17852] `z.preprocess`'s `in` half is a `ZodTransform`, which + // unconditionally hardcodes `_zod.optin = "optional"` (zod v4 core, + // `$ZodTransform.init`) — a preprocess accepts any input, including + // `undefined`, REGARDLESS of whether the wrapped schema does. Left alone, + // that makes a REQUIRED slot (e.g. `ObjectSchema.fields`, which carries no + // `.optional()`) report as optional to anything that reads `optin`/`optout` + // instead of actually parsing: `$ZodObject`'s own requiredness check for an + // "input shape" JSON Schema (`objectProcessor`, `io === 'input'` branch) + // reads exactly this flag, so — measured — the published JSON Schema for + // `data/Object` silently dropped `fields` from its `required` array without + // this correction, while the RUNTIME parse still refuses a missing + // `fields` exactly as before (confirmed separately: `optout`, which + // governs the OTHER direction and this object's own accept/reject + // behaviour, already mirrors `schema`, unaffected by this bug — only the + // requiredness *declaration* was wrong). + // + // The patch lands on `def.in` — the inner `ZodTransform` — rather than on + // `guarded` itself, and that placement is load-bearing, not stylistic: + // every classic combinator this schema is chained with afterward + // (`.describe()`, `.optional()`, …) CLONES the outer pipe into a fresh + // instance whose `optin`/`optout` are RE-DERIVED from `def.in._zod.optin` + // (measured — a patch on the outer instance is silently dropped by the + // very first `.describe()` a caller chains). `def.in` itself is carried + // over by reference across every such clone, so patching it here is what + // makes the correction survive the callers' own `.describe()` / `.optional()` + // chaining below. + guarded._zod.def.in._zod.optin = schema._zod.optin; + guarded._zod.def.in._zod.optout = schema._zod.optout; + + return guarded as unknown as Schema; +} diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index 11f3c28421..d320cb4496 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -3087,9 +3087,15 @@ export function defineStack( throw new StackTriggerCapabilityRequiredError(`${header}\n\n${lines.join('\n')}`, triggerErrors); } - // Post-parse and advisory: the stack is valid and is returned unchanged. - // `locale` carries a default, so this has to run AFTER the parse or a row - // that omits the key would read as a missing floor it actually has. + // Post-parse and advisory only: this call has no effect on what is + // returned. `locale` carries a default, so this has to run AFTER the parse + // or a row that omits the key would read as a missing floor it actually + // has. [objectstack#17852] The stack is NOT "returned unchanged" — that + // was true of neither half of this function: the parse itself can drop an + // authored key a record's key schema never gets to see (the defect this + // card fixes), and `mergeActionsIntoObjects` below rewrites `actions` + // and/or `objects` (bound-action merge, `order` sort) before the result + // reaches the caller. warnEmailTemplateLocaleFloor(data); return mergeActionsIntoObjects(data);