From ee5bbda9f47f5437a78480893fa3bfd5d2f37c4e Mon Sep 17 00:00:00 2001 From: Erik Dakoda Date: Tue, 3 Nov 2026 12:43:41 -0500 Subject: [PATCH 1/3] fix(orm): skip a @@validate rule on update when the update sends none of its fields The update data schema checked @@validate rules against the update payload, which holds only the fields the update changes. A field left out is ABSENT, and `==` / `!=` treat ABSENT as null. So a rule such as `useCase != null || modelName != null` rejected every update that left out both fields, although the stored row satisfies it and the update cannot change that. - `addCustomValidation` takes `options.partial`: a rule is skipped for data that sends none of the fields it references - the ORM update data schema passes `partial: true` A rule that references a field the update sends is checked as before. Fixes #2904 Co-Authored-By: Claude Opus 5.5 --- packages/orm/src/client/zod/factory.ts | 9 +++- packages/zod/src/utils.ts | 16 +++++- tests/regression/test/issue-2904.test.ts | 68 ++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 tests/regression/test/issue-2904.test.ts diff --git a/packages/orm/src/client/zod/factory.ts b/packages/orm/src/client/zod/factory.ts index 53014d4b1..77bc08dc5 100644 --- a/packages/orm/src/client/zod/factory.ts +++ b/packages/orm/src/client/zod/factory.ts @@ -2117,11 +2117,16 @@ export class ZodSchemaFactory< } }); + // an update payload holds only the fields it changes const uncheckedUpdateSchema = this.extraValidationsEnabled - ? ZodUtils.addCustomValidation(z.strictObject(uncheckedVariantFields), modelDef.attributes) + ? ZodUtils.addCustomValidation(z.strictObject(uncheckedVariantFields), modelDef.attributes, undefined, { + partial: true, + }) : z.strictObject(uncheckedVariantFields); const checkedUpdateSchema = this.extraValidationsEnabled - ? ZodUtils.addCustomValidation(z.strictObject(checkedVariantFields), modelDef.attributes) + ? ZodUtils.addCustomValidation(z.strictObject(checkedVariantFields), modelDef.attributes, undefined, { + partial: true, + }) : z.strictObject(checkedVariantFields); const result = !hasRelation ? uncheckedUpdateSchema : z.union([uncheckedUpdateSchema, checkedUpdateSchema]); diff --git a/packages/zod/src/utils.ts b/packages/zod/src/utils.ts index db773096e..5e1bba7da 100644 --- a/packages/zod/src/utils.ts +++ b/packages/zod/src/utils.ts @@ -351,6 +351,10 @@ export function collectFieldRefs(expr: Expression): Set { * Omit `presentFields` (or pass `undefined`) to apply all rules * unconditionally, which is the correct behaviour for full-model schemas. * + * Pass `options.partial` when the data is an update payload, which holds only + * the fields it changes. A rule is then skipped for data that sends none of + * the fields it references: the update cannot change the rule's result. + * * Note: `@@validate` conditions are restricted by the ZModel compiler to * scalar fields of the same model only — relation fields are a compile error. * A flat field-name set is therefore sufficient. @@ -359,6 +363,7 @@ export function addCustomValidation( schema: z.ZodSchema, attributes: readonly AttributeApplication[] | undefined, presentFields?: ReadonlySet, + options?: { partial?: boolean }, ): z.ZodSchema { const attrs = attributes?.filter((a) => a.name === '@@validate'); if (!attrs || attrs.length === 0) { @@ -386,7 +391,7 @@ export function addCustomValidation( if (pathExpr && ExpressionUtils.isArray(pathExpr)) { path = pathExpr.items.map((e) => ExpressionUtils.getLiteralValue(e) as string); } - result = applyValidation(result, expr, message, path); + result = applyValidation(result, expr, message, path, options?.partial ?? false); } return result; } @@ -396,6 +401,7 @@ function applyValidation( expr: Expression, message: string | undefined, path: string[] | undefined, + partial: boolean, ) { const options: Parameters[1] = {}; if (message) { @@ -404,7 +410,13 @@ function applyValidation( if (path) { options.path = path; } - return schema.refine((data) => Boolean(evalExpression(data, expr)), options); + const refs = partial ? [...collectFieldRefs(expr)] : []; + return schema.refine((data) => { + if (partial && !refs.some((ref) => typeof data === 'object' && data !== null && ref in data)) { + return true; + } + return Boolean(evalExpression(data, expr)); + }, options); } function evalExpression(data: any, expr: Expression): unknown { diff --git a/tests/regression/test/issue-2904.test.ts b/tests/regression/test/issue-2904.test.ts new file mode 100644 index 000000000..51340e031 --- /dev/null +++ b/tests/regression/test/issue-2904.test.ts @@ -0,0 +1,68 @@ +import { createTestClient } from '@zenstackhq/testtools'; +import { describe, expect, it } from 'vitest'; + +// https://github.com/zenstackhq/zenstack/issues/2904 +describe('Regression for issue 2904', () => { + const schema = ` +model User { + id Int @id @default(autoincrement()) + bots Bot[] +} + +model Bot { + id Int @id @default(autoincrement()) + name String + useCase String? + modelName String? + owner User? @relation(fields: [ownerId], references: [id]) + ownerId Int? + + @@validate(useCase != null || modelName != null, 'needs a use case or a model') +} +`; + + it('accepts an update that sends none of the fields a rule references', async () => { + const db = await createTestClient(schema); + const user = await db.user.create({ data: {} }); + const bot = await db.bot.create({ data: { name: 'a', useCase: 'chat', ownerId: user.id } }); + + await expect(db.bot.update({ where: { id: bot.id }, data: { name: 'b' } })).resolves.toMatchObject({ + name: 'b', + useCase: 'chat', + }); + await expect(db.bot.updateMany({ where: { id: bot.id }, data: { name: 'c' } })).resolves.toEqual({ + count: 1, + }); + await expect( + db.bot.upsert({ + where: { id: bot.id }, + create: { name: 'x', useCase: 'chat' }, + update: { name: 'd' }, + }), + ).resolves.toMatchObject({ name: 'd' }); + await expect( + db.user.update({ + where: { id: user.id }, + data: { bots: { update: { where: { id: bot.id }, data: { name: 'e' } } } }, + include: { bots: true }, + }), + ).resolves.toMatchObject({ bots: [{ name: 'e' }] }); + }); + + it('still checks a rule when the update sends a field it references', async () => { + const db = await createTestClient(schema); + const bot = await db.bot.create({ data: { name: 'a', useCase: 'chat' } }); + + await expect( + db.bot.update({ where: { id: bot.id }, data: { useCase: null, modelName: null } }), + ).toBeRejectedByValidation(['needs a use case or a model']); + await expect( + db.bot.update({ where: { id: bot.id }, data: { useCase: null, modelName: 'gpt' } }), + ).resolves.toMatchObject({ useCase: null, modelName: 'gpt' }); + }); + + it('still checks every rule on create', async () => { + const db = await createTestClient(schema); + await expect(db.bot.create({ data: { name: 'a' } })).toBeRejectedByValidation(['needs a use case or a model']); + }); +}); From b2101ef866559388d393d47c86dc6084bf777128 Mon Sep 17 00:00:00 2001 From: Erik Dakoda Date: Tue, 3 Nov 2026 15:03:24 -0500 Subject: [PATCH 2/3] fix(orm): check a partial-update rule when a relation sets its foreign key or it reads `this.field` Two gaps in the partial check, both found in review: - A relation field in the update, such as `author: { disconnect: true }`, changes the foreign key the rule reads (`authorId`). The relation field now counts as sending its foreign keys, so the rule is still checked. - `collectFieldRefs` did not collect `this.field`, so a rule written that way had no references and was always skipped on update. It now collects the first member when the receiver is `this`. Co-Authored-By: Claude Opus 5.5 --- packages/orm/src/client/zod/factory.ts | 30 ++++++++++++--- packages/zod/src/utils.ts | 49 +++++++++++++++++++++--- tests/regression/test/issue-2904.test.ts | 47 +++++++++++++++++++++++ 3 files changed, 114 insertions(+), 12 deletions(-) diff --git a/packages/orm/src/client/zod/factory.ts b/packages/orm/src/client/zod/factory.ts index 77bc08dc5..b0c4bd04d 100644 --- a/packages/orm/src/client/zod/factory.ts +++ b/packages/orm/src/client/zod/factory.ts @@ -176,6 +176,17 @@ export class ZodSchemaFactory< return Object.entries(modelDef.fields).filter(([, def]) => def.type !== 'Unsupported'); } + // maps each relation field of a model to the foreign-key fields it sets + private getRelationForeignKeys(model: string): Record { + const result: Record = {}; + for (const [name, fieldDef] of this.getModelFields(model)) { + if (fieldDef.relation?.fields?.length) { + result[name] = fieldDef.relation.fields; + } + } + return result; + } + private shouldIncludeRelations(options?: CreateSchemaOptions): boolean { return options?.relationDepth === undefined || options.relationDepth > 0; } @@ -2118,15 +2129,22 @@ export class ZodSchemaFactory< }); // an update payload holds only the fields it changes + const partialOptions = { partial: true, relationForeignKeys: this.getRelationForeignKeys(model) }; const uncheckedUpdateSchema = this.extraValidationsEnabled - ? ZodUtils.addCustomValidation(z.strictObject(uncheckedVariantFields), modelDef.attributes, undefined, { - partial: true, - }) + ? ZodUtils.addCustomValidation( + z.strictObject(uncheckedVariantFields), + modelDef.attributes, + undefined, + partialOptions, + ) : z.strictObject(uncheckedVariantFields); const checkedUpdateSchema = this.extraValidationsEnabled - ? ZodUtils.addCustomValidation(z.strictObject(checkedVariantFields), modelDef.attributes, undefined, { - partial: true, - }) + ? ZodUtils.addCustomValidation( + z.strictObject(checkedVariantFields), + modelDef.attributes, + undefined, + partialOptions, + ) : z.strictObject(checkedVariantFields); const result = !hasRelation ? uncheckedUpdateSchema : z.union([uncheckedUpdateSchema, checkedUpdateSchema]); diff --git a/packages/zod/src/utils.ts b/packages/zod/src/utils.ts index 5e1bba7da..a9e9515c3 100644 --- a/packages/zod/src/utils.ts +++ b/packages/zod/src/utils.ts @@ -331,7 +331,12 @@ export function collectFieldRefs(expr: Expression): Set { e.items.forEach(walk); break; case 'member': - walk(e.receiver); + if (e.receiver.kind === 'this') { + // `this.field` references `field` + if (e.members[0]) refs.add(e.members[0]); + } else { + walk(e.receiver); + } break; // literal / null / this / binding — no field refs } @@ -363,7 +368,18 @@ export function addCustomValidation( schema: z.ZodSchema, attributes: readonly AttributeApplication[] | undefined, presentFields?: ReadonlySet, - options?: { partial?: boolean }, + options?: { + /** + * The data holds only some of the model's fields, as in an update. A rule is then + * skipped when the data sends none of the fields it references. + */ + partial?: boolean; + /** + * Maps each relation field to the foreign-key fields it sets. In partial mode, a + * relation field in the data counts as sending its foreign keys. + */ + relationForeignKeys?: Record; + }, ): z.ZodSchema { const attrs = attributes?.filter((a) => a.name === '@@validate'); if (!attrs || attrs.length === 0) { @@ -391,17 +407,36 @@ export function addCustomValidation( if (pathExpr && ExpressionUtils.isArray(pathExpr)) { path = pathExpr.items.map((e) => ExpressionUtils.getLiteralValue(e) as string); } - result = applyValidation(result, expr, message, path, options?.partial ?? false); + result = applyValidation( + result, + expr, + message, + path, + options?.partial ? partialTriggerFields(expr, options.relationForeignKeys) : undefined, + ); } return result; } +// The fields whose presence in partial data means a rule must be checked: the fields the rule +// references, plus every relation field that sets one of them. +function partialTriggerFields(expr: Expression, relationForeignKeys: Record = {}) { + const refs = collectFieldRefs(expr); + const triggers = [...refs]; + for (const [relation, foreignKeys] of Object.entries(relationForeignKeys)) { + if (foreignKeys.some((fk) => refs.has(fk))) { + triggers.push(relation); + } + } + return triggers; +} + function applyValidation( schema: z.ZodSchema, expr: Expression, message: string | undefined, path: string[] | undefined, - partial: boolean, + partialTriggers: string[] | undefined, ) { const options: Parameters[1] = {}; if (message) { @@ -410,9 +445,11 @@ function applyValidation( if (path) { options.path = path; } - const refs = partial ? [...collectFieldRefs(expr)] : []; return schema.refine((data) => { - if (partial && !refs.some((ref) => typeof data === 'object' && data !== null && ref in data)) { + if ( + partialTriggers && + !partialTriggers.some((field) => typeof data === 'object' && data !== null && field in data) + ) { return true; } return Boolean(evalExpression(data, expr)); diff --git a/tests/regression/test/issue-2904.test.ts b/tests/regression/test/issue-2904.test.ts index 51340e031..79dd4a587 100644 --- a/tests/regression/test/issue-2904.test.ts +++ b/tests/regression/test/issue-2904.test.ts @@ -65,4 +65,51 @@ model Bot { const db = await createTestClient(schema); await expect(db.bot.create({ data: { name: 'a' } })).toBeRejectedByValidation(['needs a use case or a model']); }); + + it('checks a rule when a relation field sets a foreign key the rule references', async () => { + const db = await createTestClient(` +model User { + id Int @id @default(autoincrement()) + posts Post[] +} + +model Post { + id Int @id @default(autoincrement()) + title String + author User? @relation(fields: [authorId], references: [id]) + authorId Int? + + @@validate(authorId != null, 'needs an author') +} +`); + const user = await db.user.create({ data: {} }); + const post = await db.post.create({ data: { title: 'a', authorId: user.id } }); + + await expect(db.post.update({ where: { id: post.id }, data: { title: 'b' } })).resolves.toMatchObject({ + title: 'b', + }); + await expect( + db.post.update({ where: { id: post.id }, data: { author: { disconnect: true } } }), + ).toBeRejectedByValidation(['needs an author']); + }); + + it('checks a rule that references a field through this', async () => { + const db = await createTestClient(` +model Item { + id Int @id @default(autoincrement()) + name String + amount Int + + @@validate(this.amount > 0, 'amount must be positive') +} +`); + const item = await db.item.create({ data: { name: 'a', amount: 1 } }); + + await expect(db.item.update({ where: { id: item.id }, data: { name: 'b' } })).resolves.toMatchObject({ + name: 'b', + }); + await expect(db.item.update({ where: { id: item.id }, data: { amount: -1 } })).toBeRejectedByValidation([ + 'amount must be positive', + ]); + }); }); From 1535d7211a1053cca427967c0bb22dc1147d2110 Mon Sep 17 00:00:00 2001 From: Erik Dakoda Date: Tue, 3 Nov 2026 17:19:45 -0500 Subject: [PATCH 3/3] fix(orm): do not check a partial-update rule for a nested update of its relation A relation field in the update counted as sending its foreign keys whatever it held. So `author: { update: { name: 'x' } }` triggered a rule on `authorId`. The payload has no `authorId`, so the rule read it as null and rejected the update, although a nested `update` writes to the related row only. A relation field now counts only when it holds an operation other than `update`. Co-Authored-By: Claude Opus 5.5 --- packages/orm/src/client/zod/factory.ts | 4 +- packages/zod/src/utils.ts | 69 +++++++++++++++++------- tests/regression/test/issue-2904.test.ts | 35 ++++++++++++ 3 files changed, 89 insertions(+), 19 deletions(-) diff --git a/packages/orm/src/client/zod/factory.ts b/packages/orm/src/client/zod/factory.ts index b0c4bd04d..288403078 100644 --- a/packages/orm/src/client/zod/factory.ts +++ b/packages/orm/src/client/zod/factory.ts @@ -176,7 +176,9 @@ export class ZodSchemaFactory< return Object.entries(modelDef.fields).filter(([, def]) => def.type !== 'Unsupported'); } - // maps each relation field of a model to the foreign-key fields it sets + /** + * Maps each relation field of a model to the foreign-key fields it sets. + */ private getRelationForeignKeys(model: string): Record { const result: Record = {}; for (const [name, fieldDef] of this.getModelFields(model)) { diff --git a/packages/zod/src/utils.ts b/packages/zod/src/utils.ts index a9e9515c3..a8309d292 100644 --- a/packages/zod/src/utils.ts +++ b/packages/zod/src/utils.ts @@ -307,8 +307,8 @@ export function addListValidation( } /** - * Recursively collects all field names referenced by `kind: 'field'` nodes - * inside an expression tree. + * Recursively collects all field names referenced by `kind: 'field'` nodes and + * by `this.field` member nodes inside an expression tree. */ export function collectFieldRefs(expr: Expression): Set { const refs = new Set(); @@ -376,7 +376,8 @@ export function addCustomValidation( partial?: boolean; /** * Maps each relation field to the foreign-key fields it sets. In partial mode, a - * relation field in the data counts as sending its foreign keys. + * relation field in the data counts as sending its foreign keys, unless it holds + * only a nested `update`, which cannot change them. */ relationForeignKeys?: Record; }, @@ -412,31 +413,66 @@ export function addCustomValidation( expr, message, path, - options?.partial ? partialTriggerFields(expr, options.relationForeignKeys) : undefined, + options?.partial ? partialTriggers(expr, options.relationForeignKeys) : undefined, ); } return result; } -// The fields whose presence in partial data means a rule must be checked: the fields the rule -// references, plus every relation field that sets one of them. -function partialTriggerFields(expr: Expression, relationForeignKeys: Record = {}) { +type PartialTriggers = { + /** The fields the rule references. */ + fields: string[]; + /** The relation fields that set a foreign key the rule references. */ + relations: string[]; +}; + +/** + * Returns the fields and relation fields that make a rule apply to partial data. + */ +function partialTriggers( + expr: Expression, + relationForeignKeys: Record = {}, +): PartialTriggers { const refs = collectFieldRefs(expr); - const triggers = [...refs]; - for (const [relation, foreignKeys] of Object.entries(relationForeignKeys)) { - if (foreignKeys.some((fk) => refs.has(fk))) { - triggers.push(relation); - } + return { + fields: [...refs], + relations: Object.entries(relationForeignKeys) + .filter(([, foreignKeys]) => foreignKeys.some((fk) => refs.has(fk))) + .map(([relation]) => relation), + }; +} + +/** + * Tells whether a relation payload can change the foreign keys of its relation. Every nested + * operation can, except `update`, which writes to the related row only. + */ +function relationChangesForeignKey(payload: unknown) { + return typeof payload === 'object' && payload !== null && Object.keys(payload).some((op) => op !== 'update'); +} + +/** + * Tells whether partial data sends a field or a foreign-key change that a rule depends on. + */ +function partialDataTriggersRule(data: unknown, triggers: PartialTriggers) { + if (typeof data !== 'object' || data === null) { + return false; } - return triggers; + return ( + triggers.fields.some((field) => field in data) || + triggers.relations.some((relation) => relationChangesForeignKey((data as Record)[relation])) + ); } +/** + * Adds one `@@validate` rule to `schema` as a refinement. With `triggers`, the data is partial and + * the refinement passes for data that does not trigger the rule. + */ function applyValidation( schema: z.ZodSchema, expr: Expression, message: string | undefined, path: string[] | undefined, - partialTriggers: string[] | undefined, + triggers: PartialTriggers | undefined, ) { const options: Parameters[1] = {}; if (message) { @@ -446,10 +482,7 @@ function applyValidation( options.path = path; } return schema.refine((data) => { - if ( - partialTriggers && - !partialTriggers.some((field) => typeof data === 'object' && data !== null && field in data) - ) { + if (triggers && !partialDataTriggersRule(data, triggers)) { return true; } return Boolean(evalExpression(data, expr)); diff --git a/tests/regression/test/issue-2904.test.ts b/tests/regression/test/issue-2904.test.ts index 79dd4a587..100fd73c0 100644 --- a/tests/regression/test/issue-2904.test.ts +++ b/tests/regression/test/issue-2904.test.ts @@ -93,6 +93,41 @@ model Post { ).toBeRejectedByValidation(['needs an author']); }); + it('accepts a nested update of a relation whose foreign key a rule references', async () => { + const db = await createTestClient(` +model User { + id Int @id @default(autoincrement()) + name String? + posts Post[] +} + +model Post { + id Int @id @default(autoincrement()) + title String + author User? @relation(fields: [authorId], references: [id]) + authorId Int? + + @@validate(authorId != null, 'needs an author') +} +`); + const user = await db.user.create({ data: {} }); + const post = await db.post.create({ data: { title: 'a', authorId: user.id } }); + + await expect( + db.post.update({ + where: { id: post.id }, + data: { author: { update: { name: 'x' } } }, + include: { author: true }, + }), + ).resolves.toMatchObject({ authorId: user.id, author: { name: 'x' } }); + await expect( + db.post.update({ + where: { id: post.id }, + data: { author: { update: { name: 'y' }, disconnect: true } }, + }), + ).toBeRejectedByValidation(['needs an author']); + }); + it('checks a rule that references a field through this', async () => { const db = await createTestClient(` model Item {