From f9b9f31dc7b86bbb5fee4ea38bd8a546081641f3 Mon Sep 17 00:00:00 2001 From: Ludovic Motte Date: Fri, 25 Sep 2026 15:47:14 +0200 Subject: [PATCH 1/3] feat(policy): field-level update policies on M2M relation fields Allow @allow('update', ...) and @deny('update', ...) on implicit many-to-many relation fields. The field-level policy takes precedence over the model-level update policy for that side of the relation; when no field-level policy is declared, the model-level policy applies (preserving backward compatibility). Both sides of the relation are checked on connect and disconnect. Only the 'update' action is allowed on M2M relation fields; 'read' and 'all' are rejected with an explicit error. - language: add isManyToManyField() helper, relax validator - policy: add buildM2mSidePolicyFilter(), use it in connect/disconnect - tests: e2e (connect-disconnect) + regression (issue-2382) Closes #2382 --- packages/language/src/utils.ts | 18 ++ .../attribute-application-validator.ts | 15 +- .../test/attribute-application.test.ts | 139 +++++++++++ packages/plugins/policy/src/policy-handler.ts | 25 +- .../migrated/connect-disconnect.test.ts | 234 ++++++++++++++++++ tests/regression/test/issue-2382.test.ts | 68 +++++ 6 files changed, 491 insertions(+), 8 deletions(-) create mode 100644 tests/regression/test/issue-2382.test.ts diff --git a/packages/language/src/utils.ts b/packages/language/src/utils.ts index aa2ecfe3e..650501fdc 100644 --- a/packages/language/src/utils.ts +++ b/packages/language/src/utils.ts @@ -166,6 +166,24 @@ export function isRelationshipField(field: DataField) { return isDataModel(field.type.reference?.ref); } +/** + * Returns if the given field is a many-to-many relation field, i.e. a relation field that is an + * array and has at least one opposite relation field on the referenced model that is also an + * array referencing back to the containing model. + */ +export function isManyToManyField(field: DataField) { + if (!isRelationshipField(field) || !field.type.array) { + return false; + } + + const oppositeModel = field.type.reference!.ref as DataModel; + const containingModel = field.$container as DataModel; + + return getAllFields(oppositeModel).some( + (f) => f !== field && f.type.array && f.type.reference?.ref?.name === containingModel.name, + ); +} + /** * Returns if the given field is a computed field. */ diff --git a/packages/language/src/validators/attribute-application-validator.ts b/packages/language/src/validators/attribute-application-validator.ts index cdb0ed969..0823b9437 100644 --- a/packages/language/src/validators/attribute-application-validator.ts +++ b/packages/language/src/validators/attribute-application-validator.ts @@ -38,6 +38,7 @@ import { isComputedField, isDataFieldReference, isDelegateModel, + isManyToManyField, isNativeTypeMappingAttribute, isRelationshipField, mapBuiltinTypeToExpressionType, @@ -348,18 +349,26 @@ export default class AttributeApplicationValidator implements AstValidator isBeforeInvocation(node))) { accept('error', `"before()" is not allowed in field-level policies`, { node: expr }); } - // relation fields are not allowed + // relation fields are not allowed, except for many-to-many fields which only support 'update' const field = attr.$container as DataField; if (isRelationshipField(field)) { - accept('error', `Field-level policies are not allowed for relation fields.`, { node: attr }); + if (isManyToManyField(field)) { + if (kinds.some((k) => k !== 'update')) { + accept('error', `Only 'update' policies are allowed on many-to-many relation fields`, { + node: attr, + }); + } + } else { + accept('error', `Field-level policies are not allowed for relation fields.`, { node: attr }); + } } if (isComputedField(field)) { diff --git a/packages/language/test/attribute-application.test.ts b/packages/language/test/attribute-application.test.ts index 9abe8ffe9..384861c20 100644 --- a/packages/language/test/attribute-application.test.ts +++ b/packages/language/test/attribute-application.test.ts @@ -370,6 +370,145 @@ describe('Attribute application validation tests', () => { ); }); + it('accepts update field-level policy on many-to-many relation fields', async () => { + await loadSchema(` + datasource db { + provider = 'sqlite' + url = 'file:./dev.db' + } + + model Foo { + id Int @id @default(autoincrement()) + bars Bar[] + @@allow('all', true) + } + + model Bar { + id Int @id @default(autoincrement()) + foos Foo[] @allow('update', true) + @@allow('all', true) + } + `); + }); + + it('accepts deny update field-level policy on many-to-many relation fields', async () => { + await loadSchema(` + datasource db { + provider = 'sqlite' + url = 'file:./dev.db' + } + + model Foo { + id Int @id @default(autoincrement()) + bars Bar[] + @@allow('all', true) + } + + model Bar { + id Int @id @default(autoincrement()) + foos Foo[] @deny('update', false) + @@allow('all', true) + } + `); + }); + + it('rejects read field-level policy on many-to-many relation fields', async () => { + await loadSchemaWithError( + ` + datasource db { + provider = 'sqlite' + url = 'file:./dev.db' + } + + model Foo { + id Int @id @default(autoincrement()) + bars Bar[] + @@allow('all', true) + } + + model Bar { + id Int @id @default(autoincrement()) + foos Foo[] @allow('read', true) + @@allow('all', true) + } + `, + `Only 'update' policies are allowed on many-to-many relation fields`, + ); + }); + + it('rejects all field-level policy on many-to-many relation fields', async () => { + await loadSchemaWithError( + ` + datasource db { + provider = 'sqlite' + url = 'file:./dev.db' + } + + model Foo { + id Int @id @default(autoincrement()) + bars Bar[] + @@allow('all', true) + } + + model Bar { + id Int @id @default(autoincrement()) + foos Foo[] @allow('all', true) + @@allow('all', true) + } + `, + `Only 'update' policies are allowed on many-to-many relation fields`, + ); + }); + + it('rejects comma-separated kinds including non-update on many-to-many relation fields', async () => { + await loadSchemaWithError( + ` + datasource db { + provider = 'sqlite' + url = 'file:./dev.db' + } + + model Foo { + id Int @id @default(autoincrement()) + bars Bar[] + @@allow('all', true) + } + + model Bar { + id Int @id @default(autoincrement()) + foos Foo[] @allow('read, update', true) + @@allow('all', true) + } + `, + `Only 'update' policies are allowed on many-to-many relation fields`, + ); + }); + + it('rejects update field-level policy on one-to-many relation fields', async () => { + await loadSchemaWithError( + ` + datasource db { + provider = 'sqlite' + url = 'file:./dev.db' + } + + model Foo { + id Int @id @default(autoincrement()) + bar Bar @relation(fields: [barId], references: [id]) + barId Int + @@allow('all', true) + } + + model Bar { + id Int @id @default(autoincrement()) + foos Foo[] @allow('update', true) + @@allow('all', true) + } + `, + `Field-level policies are not allowed for relation fields`, + ); + }); + it('rejects field-level policy on computed fields', async () => { await loadSchemaWithError( ` diff --git a/packages/plugins/policy/src/policy-handler.ts b/packages/plugins/policy/src/policy-handler.ts index b192d72b5..8571b5181 100644 --- a/packages/plugins/policy/src/policy-handler.ts +++ b/packages/plugins/policy/src/policy-handler.ts @@ -262,8 +262,8 @@ export class PolicyHandler extends OperationNodeTransf // the join table's fk columns are constrained by literal values for the sides that the // delete explicitly targets; only those sides can be checked upfront const sides = [ - { column: 'A', model: m2m.firstModel, idField: m2m.firstIdField }, - { column: 'B', model: m2m.secondModel, idField: m2m.secondIdField }, + { column: 'A', model: m2m.firstModel, field: m2m.firstField, idField: m2m.firstIdField }, + { column: 'B', model: m2m.secondModel, field: m2m.secondField, idField: m2m.secondIdField }, ] .map((side) => ({ ...side, value: this.extractEqualityValue(node.where?.where, side.column) })) .filter((side) => side.value !== undefined); @@ -281,7 +281,7 @@ export class PolicyHandler extends OperationNodeTransf .selectFrom(side.model) .where(this.eb(this.eb.ref(`${side.model}.${side.idField}`), '=', side.value)) .select(() => - new ExpressionWrapper(this.buildPolicyFilter(side.model, undefined, 'update')).as('_'), + new ExpressionWrapper(this.buildM2mSidePolicyFilter(side.model, side.field)).as('_'), ) .toOperationNode(), IdentifierNode.create(`$condition${index}`), @@ -885,6 +885,21 @@ export class PolicyHandler extends OperationNodeTransf return combinedPolicy; } + /** + * Builds the update policy filter for one side of an implicit many-to-many relation. + * + * If the relation field declares field-level `update` policies, they take precedence over + * the model-level policy for this side; otherwise the model-level `update` policy applies + * (preserving the pre-existing behavior). + */ + private buildM2mSidePolicyFilter(model: string, field: string): OperationNode { + const fieldPolicies = this.getFieldPolicies(model, field, 'update'); + if (fieldPolicies.length > 0) { + return this.buildFieldPolicyFilter(model, field, 'update'); + } + return this.buildPolicyFilter(model, undefined, 'update'); + } + // #endregion // #region helpers @@ -977,13 +992,13 @@ export class PolicyHandler extends OperationNodeTransf const eb = expressionBuilder(); - const filterA = this.buildPolicyFilter(m2m.firstModel, undefined, 'update'); + const filterA = this.buildM2mSidePolicyFilter(m2m.firstModel, m2m.firstField); const queryA = eb .selectFrom(m2m.firstModel) .where(eb(eb.ref(`${m2m.firstModel}.${m2m.firstIdField}`), '=', aValue)) .select(() => new ExpressionWrapper(filterA).as('_')); - const filterB = this.buildPolicyFilter(m2m.secondModel, undefined, 'update'); + const filterB = this.buildM2mSidePolicyFilter(m2m.secondModel, m2m.secondField); const queryB = eb .selectFrom(m2m.secondModel) .where(eb(eb.ref(`${m2m.secondModel}.${m2m.secondIdField}`), '=', bValue)) diff --git a/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts b/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts index ec3387271..71b47b1e0 100644 --- a/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts +++ b/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts @@ -377,4 +377,238 @@ describe('connect and disconnect tests', () => { }), ).toBeRejectedByPolicy(); }); + + it('inherits model-level update policy when no field-level policy is declared', async () => { + const db = await createPolicyTestClient( + ` + model M1 { + id String @id @default(uuid()) + value Int @default(0) + m2 M2[] + + @@allow('all', true) + } + + model M2 { + id String @id @default(uuid()) + value Int + deleted Boolean @default(false) + m1 M1[] + + @@allow('read,create', true) + @@allow('update', !deleted) + } + `, + { usePrismaPush: true }, + ); + const rawDb = db.$unuseAll(); + + await rawDb.m1.create({ data: { id: 'm1-1', value: 1 } }); + await rawDb.m2.create({ data: { id: 'm2-1', value: 1, deleted: false } }); + // both sides updatable -> connect allowed + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { connect: { id: 'm2-1' } } }, + }), + ).toResolveTruthy(); + + await rawDb.m2.create({ data: { id: 'm2-2', value: 1, deleted: true } }); + // m2-2 not updatable -> connect rejected + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { connect: { id: 'm2-2' } } }, + }), + ).toBeRejectedByPolicy(); + + // disconnect of an updatable side is allowed + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { disconnect: { id: 'm2-1' } } }, + }), + ).toResolveTruthy(); + }); + + it('field-level allow overrides restrictive model-level update policy', async () => { + const db = await createPolicyTestClient( + ` + model M1 { + id String @id @default(uuid()) + value Int @default(0) + m2 M2[] + + @@allow('all', true) + } + + model M2 { + id String @id @default(uuid()) + value Int + deleted Boolean @default(false) + m1 M1[] @allow('update', true) + + @@allow('read,create', true) + @@allow('update', !deleted) + } + `, + { usePrismaPush: true }, + ); + const rawDb = db.$unuseAll(); + + await rawDb.m1.create({ data: { id: 'm1-1', value: 1 } }); + await rawDb.m2.create({ data: { id: 'm2-1', value: 1, deleted: true } }); + // model-level policy would reject (deleted), but field-level allow overrides it + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { connect: { id: 'm2-1' } } }, + }), + ).toResolveTruthy(); + + // the override applies to disconnect as well + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { disconnect: { id: 'm2-1' } } }, + }), + ).toResolveTruthy(); + }); + + it('field-level deny overrides permissive model-level update policy', async () => { + const db = await createPolicyTestClient( + ` + model M1 { + id String @id @default(uuid()) + value Int @default(0) + m2 M2[] + + @@allow('all', true) + } + + model M2 { + id String @id @default(uuid()) + value Int + deleted Boolean @default(false) + m1 M1[] @deny('update', deleted) + + @@allow('read,create', true) + @@allow('update', !deleted) + } + `, + { usePrismaPush: true }, + ); + const rawDb = db.$unuseAll(); + + await rawDb.m1.create({ data: { id: 'm1-1', value: 1 } }); + await rawDb.m2.create({ data: { id: 'm2-1', value: 1, deleted: false } }); + // not deleted -> field-level deny not triggered, connect allowed + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { connect: { id: 'm2-1' } } }, + }), + ).toResolveTruthy(); + + await rawDb.m2.create({ data: { id: 'm2-2', value: 1, deleted: true } }); + // deleted -> field-level deny triggers, connect rejected + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { connect: { id: 'm2-2' } } }, + }), + ).toBeRejectedByPolicy(); + + // disconnect is also rejected for the denied side + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { disconnect: { id: 'm2-1' } } }, + }), + ).toResolveTruthy(); + }); + + it('field-level allow on a read-only model enables connect (issue #2382)', async () => { + const db = await createPolicyTestClient( + ` + model M1 { + id String @id @default(uuid()) + value Int @default(0) + m2 M2[] + + @@allow('all', true) + } + + model M2 { + id String @id @default(uuid()) + value Int + m1 M1[] @allow('update', true) + + @@allow('read,create', true) + } + `, + { usePrismaPush: true }, + ); + const rawDb = db.$unuseAll(); + + await rawDb.m1.create({ data: { id: 'm1-1', value: 1 } }); + await rawDb.m2.create({ data: { id: 'm2-1', value: 1 } }); + // M2 has no model-level update policy, but the field-level allow on m1 enables connect + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { connect: { id: 'm2-1' } } }, + }), + ).toResolveTruthy(); + + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { disconnect: { id: 'm2-1' } } }, + }), + ).toResolveTruthy(); + }); + + it('checks both sides of the relation', async () => { + const db = await createPolicyTestClient( + ` + model M1 { + id String @id @default(uuid()) + value Int @default(0) + m2 M2[] @deny('update', value > 0) + + @@allow('all', true) + } + + model M2 { + id String @id @default(uuid()) + value Int + m1 M1[] + + @@allow('all', true) + } + `, + { usePrismaPush: true }, + ); + const rawDb = db.$unuseAll(); + + await rawDb.m1.create({ data: { id: 'm1-1', value: 0 } }); + await rawDb.m2.create({ data: { id: 'm2-1', value: 1 } }); + // m1-1 value is 0 -> field-level deny not triggered, connect allowed + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { connect: { id: 'm2-1' } } }, + }), + ).toResolveTruthy(); + + await rawDb.m1.create({ data: { id: 'm1-2', value: 1 } }); + // m1-2 value is 1 -> field-level deny on the "source" side triggers, connect rejected + await expect( + db.m1.update({ + where: { id: 'm1-2' }, + data: { m2: { connect: { id: 'm2-1' } } }, + }), + ).toBeRejectedByPolicy(); + }); }); diff --git a/tests/regression/test/issue-2382.test.ts b/tests/regression/test/issue-2382.test.ts new file mode 100644 index 000000000..915b5e422 --- /dev/null +++ b/tests/regression/test/issue-2382.test.ts @@ -0,0 +1,68 @@ +import { createPolicyTestClient } from '@zenstackhq/testtools'; +import { describe, expect, it } from 'vitest'; + +// https://github.com/zenstackhq/zenstack/issues/2382 +describe('Regression for issue #2382', () => { + it('allows club owner to link/unlink read-only activities', async () => { + const db = await createPolicyTestClient( + ` +model User { + id String @id @default(uuid()) + clubs Club[] +} + +model Club { + id String @id @default(uuid()) + name String + ownerId String + owner User @relation(fields: [ownerId], references: [id]) + activities Activity[] + + @@allow('all', auth().id == ownerId) +} + +model Activity { + id String @id @default(uuid()) + name String + clubs Club[] @allow('update', auth() != null) + + @@allow('read', true) +} +`, + { usePrismaPush: true }, + ); + const rawDb = db.$unuseAll(); + + await rawDb.user.create({ data: { id: 'user-1' } }); + await rawDb.club.create({ data: { id: 'club-1', name: 'Chess Club', ownerId: 'user-1' } }); + await rawDb.activity.create({ data: { id: 'act-1', name: 'Tournament' } }); + + const ownerDb = db.$setAuth({ id: 'user-1' }); + + // Activity is read-only (no model-level update policy), but the field-level + // allow on `clubs` enables the club owner to link it — this was rejected before the fix + await expect( + ownerDb.club.update({ + where: { id: 'club-1' }, + data: { activities: { connect: { id: 'act-1' } } }, + }), + ).toResolveTruthy(); + + // disconnect also works + await expect( + ownerDb.club.update({ + where: { id: 'club-1' }, + data: { activities: { disconnect: { id: 'act-1' } } }, + }), + ).toResolveTruthy(); + + // a non-owner cannot link (Club is not visible to them — read policy filters it out) + const otherDb = db.$setAuth({ id: 'user-2' }); + await expect( + otherDb.club.update({ + where: { id: 'club-1' }, + data: { activities: { connect: { id: 'act-1' } } }, + }), + ).toBeRejectedNotFound(); + }); +}); From dad0f79dbccade5c91b5309fb4fc85751902ec5e Mon Sep 17 00:00:00 2001 From: Ludovic Motte Date: Fri, 25 Sep 2026 18:01:56 +0200 Subject: [PATCH 2/3] fix(policy): address review comments on M2M field-level update policies - language: isManyToManyField now matches the opposite field of the same relation (by @relation name), preventing false positives when a model pair has both a one-to-many and a many-to-many relation - policy: apply field-level update policy precedence to the join-table delete filter (buildM2mSidePolicyFilter), keeping model-level filter for reads - policy: preDeleteCheck now extracts IN constraints in addition to equality, so both sides of a disconnect are verified upfront - tests: exercise field-level deny on disconnect, reject update policy on one-to-many fields when a separate m2m relation exists, align m2m manipulation test with the new rejection behavior --- packages/language/src/utils.ts | 38 ++++++++-- .../test/attribute-application.test.ts | 26 +++++++ packages/plugins/policy/src/policy-handler.ts | 69 +++++++++++++------ tests/e2e/orm/policy/crud/update.test.ts | 6 +- .../migrated/connect-disconnect.test.ts | 7 +- 5 files changed, 115 insertions(+), 31 deletions(-) diff --git a/packages/language/src/utils.ts b/packages/language/src/utils.ts index 650501fdc..bdbcbb819 100644 --- a/packages/language/src/utils.ts +++ b/packages/language/src/utils.ts @@ -166,10 +166,29 @@ export function isRelationshipField(field: DataField) { return isDataModel(field.type.reference?.ref); } +/** + * Returns the name of the relation the given field belongs to, as declared in its `@relation` + * attribute, or `undefined` if the field has no `@relation` attribute or no explicit name. + */ +function getRelationName(field: DataField): string | undefined { + const relAttr = field.attributes.find((attr) => attr.decl.ref?.name === '@relation'); + if (!relAttr) { + return undefined; + } + for (const arg of relAttr.args) { + if (!arg.name || arg.name === 'name') { + if (isStringLiteral(arg.value)) { + return arg.value.value; + } + } + } + return undefined; +} + /** * Returns if the given field is a many-to-many relation field, i.e. a relation field that is an - * array and has at least one opposite relation field on the referenced model that is also an - * array referencing back to the containing model. + * array and whose opposite relation field on the referenced model (belonging to the same relation) + * is also an array referencing back to the containing model. */ export function isManyToManyField(field: DataField) { if (!isRelationshipField(field) || !field.type.array) { @@ -178,10 +197,19 @@ export function isManyToManyField(field: DataField) { const oppositeModel = field.type.reference!.ref as DataModel; const containingModel = field.$container as DataModel; + const relationName = getRelationName(field); - return getAllFields(oppositeModel).some( - (f) => f !== field && f.type.array && f.type.reference?.ref?.name === containingModel.name, - ); + return getAllFields(oppositeModel).some((f) => { + if (f === field || !f.type.array || f.type.reference?.ref?.name !== containingModel.name) { + return false; + } + // if the field declares an explicit relation name, the opposite field must belong to the + // same relation; otherwise any array field referencing back is the opposite + if (relationName !== undefined) { + return getRelationName(f) === relationName; + } + return true; + }); } /** diff --git a/packages/language/test/attribute-application.test.ts b/packages/language/test/attribute-application.test.ts index 384861c20..0ee7d8dea 100644 --- a/packages/language/test/attribute-application.test.ts +++ b/packages/language/test/attribute-application.test.ts @@ -509,6 +509,32 @@ describe('Attribute application validation tests', () => { ); }); + it('rejects update field-level policy on one-to-many relation fields when a separate m2m relation exists', async () => { + await loadSchemaWithError( + ` + datasource db { + provider = 'sqlite' + url = 'file:./dev.db' + } + + model Foo { + id Int @id @default(autoincrement()) + bar Bar @relation("one-to-many", fields: [barId], references: [id]) @allow('update', true) + barId Int + bars Bar[] @relation("m2m") + @@allow('all', true) + } + + model Bar { + id Int @id @default(autoincrement()) + foos Foo[] @relation("m2m") + @@allow('all', true) + } + `, + `Field-level policies are not allowed for relation fields`, + ); + }); + it('rejects field-level policy on computed fields', async () => { await loadSchemaWithError( ` diff --git a/packages/plugins/policy/src/policy-handler.ts b/packages/plugins/policy/src/policy-handler.ts index 8571b5181..f3f0109f7 100644 --- a/packages/plugins/policy/src/policy-handler.ts +++ b/packages/plugins/policy/src/policy-handler.ts @@ -265,8 +265,8 @@ export class PolicyHandler extends OperationNodeTransf { column: 'A', model: m2m.firstModel, field: m2m.firstField, idField: m2m.firstIdField }, { column: 'B', model: m2m.secondModel, field: m2m.secondField, idField: m2m.secondIdField }, ] - .map((side) => ({ ...side, value: this.extractEqualityValue(node.where?.where, side.column) })) - .filter((side) => side.value !== undefined); + .map((side) => ({ ...side, values: this.extractConstrainedValues(node.where?.where, side.column) })) + .filter((side) => side.values !== undefined); if (sides.length === 0) { return; @@ -279,7 +279,7 @@ export class PolicyHandler extends OperationNodeTransf AliasNode.create( this.eb .selectFrom(side.model) - .where(this.eb(this.eb.ref(`${side.model}.${side.idField}`), '=', side.value)) + .where(this.eb(this.eb.ref(`${side.model}.${side.idField}`), 'in', side.values!)) .select(() => new ExpressionWrapper(this.buildM2mSidePolicyFilter(side.model, side.field)).as('_'), ) @@ -302,30 +302,33 @@ export class PolicyHandler extends OperationNodeTransf } /** - * Finds the literal value that `column` is constrained to by a top-level conjunction of the given - * where clause, or `undefined` if there's no such constraint. + * Finds the literal values that `column` is constrained to by a top-level conjunction of the + * given where clause, or `undefined` if there's no such constraint. Supports both `=` and `IN` + * operators. * - * E.g., given the where clause `("A" = 1 AND "B" IN (2, 3))`, extracting column "A" returns `1`, - * while extracting column "B" returns `undefined` since it's not an equality constraint. + * E.g., given the where clause `("A" = 1 AND "B" IN (2, 3))`, extracting column "A" returns + * `[1]`, and extracting column "B" returns `[2, 3]`. */ - private extractEqualityValue(node: OperationNode | undefined, column: string): unknown { + private extractConstrainedValues(node: OperationNode | undefined, column: string): unknown[] | undefined { if (!node) { return undefined; } if (ParensNode.is(node)) { - return this.extractEqualityValue(node.node, column); + return this.extractConstrainedValues(node.node, column); } if (AndNode.is(node)) { - return this.extractEqualityValue(node.left, column) ?? this.extractEqualityValue(node.right, column); + return ( + this.extractConstrainedValues(node.left, column) ?? this.extractConstrainedValues(node.right, column) + ); } if (!BinaryOperationNode.is(node)) { return undefined; } - if (!OperatorNode.is(node.operator) || node.operator.operator !== '=') { + if (!OperatorNode.is(node.operator)) { return undefined; } @@ -336,10 +339,29 @@ export class PolicyHandler extends OperationNodeTransf if (leftOperand.column.column.name !== column) { return undefined; } - if (!ValueNode.is(rightOperand) || rightOperand.value === null || rightOperand.value === undefined) { + + if (node.operator.operator === '=') { + if (!ValueNode.is(rightOperand) || rightOperand.value === null || rightOperand.value === undefined) { + return undefined; + } + return [rightOperand.value]; + } + + if (node.operator.operator === 'in') { + if (PrimitiveValueListNode.is(rightOperand)) { + const values = rightOperand.values.filter((v) => v !== null && v !== undefined); + return values.length > 0 ? values : undefined; + } + if (ValueListNode.is(rightOperand)) { + const values = rightOperand.values.filter( + (v): v is ValueNode => ValueNode.is(v) && v.value !== null && v.value !== undefined, + ); + return values.length > 0 ? values.map((v) => v.value) : undefined; + } return undefined; } - return rightOperand.value; + + return undefined; } private async postUpdateCheck( @@ -1479,27 +1501,32 @@ export class PolicyHandler extends OperationNodeTransf // join table's permission: // - read: requires both sides to be readable - // - mutation: requires both sides to be updatable + // - mutation: requires both sides to be updatable, honoring field-level update policies + // on the relation fields when declared (see buildM2mSidePolicyFilter) - const checkForOperation = operation === 'read' ? 'read' : 'update'; + const isRead = operation === 'read'; const joinTable = alias ?? tableName; const aQuery = this.eb .selectFrom(m2m.firstModel) .whereRef(`${m2m.firstModel}.${m2m.firstIdField}`, '=', `${joinTable}.A`) .select(() => - new ExpressionWrapper(this.buildPolicyFilter(m2m.firstModel, undefined, checkForOperation)).as( - '$conditionA', - ), + new ExpressionWrapper( + isRead + ? this.buildPolicyFilter(m2m.firstModel, undefined, 'read') + : this.buildM2mSidePolicyFilter(m2m.firstModel, m2m.firstField), + ).as('$conditionA'), ); const bQuery = this.eb .selectFrom(m2m.secondModel) .whereRef(`${m2m.secondModel}.${m2m.secondIdField}`, '=', `${joinTable}.B`) .select(() => - new ExpressionWrapper(this.buildPolicyFilter(m2m.secondModel, undefined, checkForOperation)).as( - '$conditionB', - ), + new ExpressionWrapper( + isRead + ? this.buildPolicyFilter(m2m.secondModel, undefined, 'read') + : this.buildM2mSidePolicyFilter(m2m.secondModel, m2m.secondField), + ).as('$conditionB'), ); return this.eb.and([aQuery, bQuery]).toOperationNode(); diff --git a/tests/e2e/orm/policy/crud/update.test.ts b/tests/e2e/orm/policy/crud/update.test.ts index d7f3a8a21..feab4fe0d 100644 --- a/tests/e2e/orm/policy/crud/update.test.ts +++ b/tests/e2e/orm/policy/crud/update.test.ts @@ -1041,7 +1041,7 @@ model Group { db.user.update({ where: { id: 2 }, data: { groups: { connect: { id: 2 } } } }), ).toBeRejectedByPolicy(); - // disconnect rejected + // disconnect rejected because group is not updatable await db.$unuseAll().user.update({ where: { id: 2 }, data: { groups: { connect: { id: 2 } } } }); await expect( db.user.update({ @@ -1049,9 +1049,7 @@ model Group { data: { groups: { disconnect: { id: 2 } } }, include: { groups: true }, }), - ).resolves.toMatchObject({ - groups: [{ id: 2 }], // verify not disconnected - }); + ).toBeRejectedByPolicy(); // delete rejected await expect( diff --git a/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts b/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts index 71b47b1e0..5c45ad48b 100644 --- a/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts +++ b/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts @@ -519,13 +519,18 @@ describe('connect and disconnect tests', () => { }), ).toBeRejectedByPolicy(); + // mark m2-1 deleted after connecting -> field-level deny triggers on disconnect + await rawDb.m2.update({ + where: { id: 'm2-1' }, + data: { deleted: true }, + }); // disconnect is also rejected for the denied side await expect( db.m1.update({ where: { id: 'm1-1' }, data: { m2: { disconnect: { id: 'm2-1' } } }, }), - ).toResolveTruthy(); + ).toBeRejectedByPolicy(); }); it('field-level allow on a read-only model enables connect (issue #2382)', async () => { From d7fd652bfc7c01b7f3de682d177a975afecaf38e Mon Sep 17 00:00:00 2001 From: Ludovic Motte Date: Fri, 25 Sep 2026 18:31:38 +0200 Subject: [PATCH 3/3] fix(policy): aggregate M2M delete precheck to a single row The many-to-many delete precheck used a scalar subquery (SELECT ... WHERE id IN (...)) that returned one row per matching participant. With multiple IDs, PostgreSQL rejects it and other databases would only verify a single participant. Aggregate to COUNT(*) of updatable participants and reject when the count is below the number of distinct values, which verifies every participant while preserving the rejection for missing ones. Add a batch disconnect test covering the multi-ID case. --- packages/plugins/policy/src/policy-handler.ts | 16 +++-- .../migrated/connect-disconnect.test.ts | 63 +++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/packages/plugins/policy/src/policy-handler.ts b/packages/plugins/policy/src/policy-handler.ts index f3f0109f7..70bd2bbca 100644 --- a/packages/plugins/policy/src/policy-handler.ts +++ b/packages/plugins/policy/src/policy-handler.ts @@ -272,6 +272,13 @@ export class PolicyHandler extends OperationNodeTransf return; } + // For each side, check that no constrained participant exists that is not updatable. Using + // For each side, count the constrained participants that are updatable. A plain + // `SELECT ... IN (...)` would return one row per matching participant, which + // scalar subquery positions reject on some databases and which would only verify a single + // participant on others. Aggregating to a single `COUNT(*)` row verifies every participant: + // a participant is only counted if it exists and is updatable, so any missing or + // non-updatable participant lowers the count below the number of distinct values. const result = await proceed({ kind: 'SelectQueryNode', selections: sides.map((side, index) => @@ -280,9 +287,8 @@ export class PolicyHandler extends OperationNodeTransf this.eb .selectFrom(side.model) .where(this.eb(this.eb.ref(`${side.model}.${side.idField}`), 'in', side.values!)) - .select(() => - new ExpressionWrapper(this.buildM2mSidePolicyFilter(side.model, side.field)).as('_'), - ) + .where(() => new ExpressionWrapper(this.buildM2mSidePolicyFilter(side.model, side.field))) + .select((eb) => eb.fn('COUNT', [eb.lit(1)]).as('_')) .toOperationNode(), IdentifierNode.create(`$condition${index}`), ), @@ -291,7 +297,9 @@ export class PolicyHandler extends OperationNodeTransf } satisfies SelectQueryNode); for (const [index, side] of sides.entries()) { - if (!result.rows[0]?.[`$condition${index}`]) { + const distinctValues = new Set(side.values!).size; + const updatableCount = Number(result.rows[0]?.[`$condition${index}`] ?? 0); + if (updatableCount < distinctValues) { throw createRejectedByPolicyError( side.model, RejectedByPolicyReason.NO_ACCESS, diff --git a/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts b/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts index 5c45ad48b..26f99e830 100644 --- a/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts +++ b/tests/e2e/orm/policy/migrated/connect-disconnect.test.ts @@ -533,6 +533,69 @@ describe('connect and disconnect tests', () => { ).toBeRejectedByPolicy(); }); + it('batch disconnect verifies every participant in the delete precheck', async () => { + const db = await createPolicyTestClient( + ` + model M1 { + id String @id @default(uuid()) + value Int @default(0) + m2 M2[] + + @@allow('all', true) + } + + model M2 { + id String @id @default(uuid()) + value Int + deleted Boolean @default(false) + m1 M1[] + + @@allow('read,create', true) + @@allow('update', !deleted) + } + `, + { usePrismaPush: true }, + ); + const rawDb = db.$unuseAll(); + + await rawDb.m1.create({ data: { id: 'm1-1', value: 1 } }); + await rawDb.m2.create({ data: { id: 'm2-1', value: 1, deleted: false } }); + await rawDb.m2.create({ data: { id: 'm2-2', value: 1, deleted: false } }); + + // connect both + await db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { connect: [{ id: 'm2-1' }, { id: 'm2-2' }] } }, + }); + + // both updatable -> batch disconnect succeeds (the precheck must verify every participant + // without the scalar subquery returning more than one row) + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { disconnect: [{ id: 'm2-1' }, { id: 'm2-2' }] } }, + }), + ).toResolveTruthy(); + + // reconnect both + await db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { connect: [{ id: 'm2-1' }, { id: 'm2-2' }] } }, + }); + + // mark one deleted -> batch disconnect rejected because a participant is not updatable + await rawDb.m2.update({ + where: { id: 'm2-2' }, + data: { deleted: true }, + }); + await expect( + db.m1.update({ + where: { id: 'm1-1' }, + data: { m2: { disconnect: [{ id: 'm2-1' }, { id: 'm2-2' }] } }, + }), + ).toBeRejectedByPolicy(); + }); + it('field-level allow on a read-only model enables connect (issue #2382)', async () => { const db = await createPolicyTestClient( `