Skip to content

Commit 59d302d

Browse files
ymc9claude
andcommitted
fix(policy): skip m2m update-policy check for newly created side on connect
When creating a model with a nested many-to-many `connect`, the join table insert triggered an update-policy check on the just-created entity. Because the connection doesn't exist yet at check time, relation-based policies (e.g. `parents?[id == auth().id]`) always evaluated to false, causing a spurious "not updatable" error. Fix by embedding a lightweight marker in the insert query's end-modifier comment so the policy handler can identify the newly-created side and skip its circular update check. The connected side's update policy is still enforced. Adds regression test for issue #2531. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 80f364a commit 59d302d

File tree

3 files changed

+126
-7
lines changed

3 files changed

+126
-7
lines changed

packages/orm/src/client/crud/operations/base.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,8 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
554554
}
555555

556556
if (fromRelation && m2m) {
557-
// connect many-to-many relation
557+
// connect many-to-many relation; mark the newly created model so the
558+
// policy plugin can skip the circular update-policy check on it
558559
await this.handleManyToManyRelation(
559560
kysely,
560561
'connect',
@@ -565,6 +566,7 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
565566
m2m.otherField,
566567
[createdEntity],
567568
m2m.joinTable,
569+
m2m.otherModel,
568570
);
569571
}
570572

@@ -647,6 +649,10 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
647649
return parentFkFields;
648650
}
649651

652+
// Marker embedded in the INSERT end modifier to signal to the policy plugin
653+
// that one side of the join was just created in this operation.
654+
static readonly M2M_CREATED_MODEL_MARKER_PREFIX = '/*ZS:M2M_CREATED:';
655+
650656
private async handleManyToManyRelation<Action extends 'connect' | 'disconnect'>(
651657
kysely: AnyKysely,
652658
action: Action,
@@ -657,6 +663,7 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
657663
rightField: string,
658664
rightEntities: any[],
659665
joinTable: string,
666+
newlyCreatedModel?: string,
660667
): Promise<void> {
661668
if (rightEntities.length === 0) {
662669
return;
@@ -694,6 +701,14 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
694701
)
695702
// case for `INSERT IGNORE` syntax
696703
.$if(this.dialect.insertIgnoreMethod === 'ignore', (qb) => qb.ignore())
704+
// embed a marker so the policy plugin knows which model was just created
705+
.$if(newlyCreatedModel !== undefined, (qb) =>
706+
qb.modifyEnd(
707+
sql.raw(
708+
`${BaseOperationHandler.M2M_CREATED_MODEL_MARKER_PREFIX}${newlyCreatedModel}*/`,
709+
),
710+
),
711+
)
697712
.execute();
698713
} else {
699714
const eb = expressionBuilder<any, any>();
@@ -829,7 +844,9 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
829844
}
830845

831846
case 'connect': {
832-
await this.connectRelation(kysely, relationModel, subPayload, fromRelationContext);
847+
// contextModel was just created; pass it so the policy plugin can
848+
// skip the circular update-policy check on the newly created side
849+
await this.connectRelation(kysely, relationModel, subPayload, fromRelationContext, contextModel);
833850
break;
834851
}
835852

@@ -839,7 +856,7 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
839856
if (!found) {
840857
await this.create(kysely, relationModel, item.create, fromRelationContext);
841858
} else {
842-
await this.connectRelation(kysely, relationModel, found, fromRelationContext);
859+
await this.connectRelation(kysely, relationModel, found, fromRelationContext, contextModel);
843860
}
844861
}
845862
break;
@@ -1910,7 +1927,13 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
19101927

19111928
// #region relation manipulation
19121929

1913-
protected async connectRelation(kysely: AnyKysely, model: string, data: any, fromRelation: FromRelationContext) {
1930+
protected async connectRelation(
1931+
kysely: AnyKysely,
1932+
model: string,
1933+
data: any,
1934+
fromRelation: FromRelationContext,
1935+
newlyCreatedModel?: string,
1936+
) {
19141937
const _data = this.normalizeRelationManipulationInput(model, data);
19151938
if (_data.length === 0) {
19161939
return;
@@ -1933,6 +1956,7 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
19331956
m2m.otherField!,
19341957
allIds,
19351958
m2m.joinTable,
1959+
newlyCreatedModel,
19361960
);
19371961
} else {
19381962
const { ownedByModel, keyPairs } = getRelationForeignKeyFieldPairs(

packages/plugins/policy/src/policy-handler.ts

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,7 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
784784
for (const values of valueRows) {
785785
if (isManyToManyJoinTable) {
786786
await this.enforcePreCreatePolicyForManyToManyJoinTable(
787+
node,
787788
mutationModel,
788789
fields,
789790
values.map((v) => v.node),
@@ -801,6 +802,7 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
801802
}
802803

803804
private async enforcePreCreatePolicyForManyToManyJoinTable(
805+
node: InsertQueryNode,
804806
tableName: string,
805807
fields: string[],
806808
values: OperationNode[],
@@ -823,15 +825,24 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
823825
invariant(aValue !== null && aValue !== undefined, 'A value cannot be null or undefined');
824826
invariant(bValue !== null && bValue !== undefined, 'B value cannot be null or undefined');
825827

828+
// If one side was just created in this operation, its update-policy check is
829+
// inherently circular (it has no relations yet), so we skip it. The create
830+
// policy for that side was already verified earlier in the operation.
831+
const newlyCreatedModel = this.extractNewlyCreatedModel(node);
832+
826833
const eb = expressionBuilder<any, any>();
827834

828-
const filterA = this.buildPolicyFilter(m2m.firstModel, undefined, 'update');
835+
const filterA = newlyCreatedModel === m2m.firstModel
836+
? trueNode(this.dialect)
837+
: this.buildPolicyFilter(m2m.firstModel, undefined, 'update');
829838
const queryA = eb
830839
.selectFrom(m2m.firstModel)
831840
.where(eb(eb.ref(`${m2m.firstModel}.${m2m.firstIdField}`), '=', aValue))
832841
.select(() => new ExpressionWrapper(filterA).as('_'));
833842

834-
const filterB = this.buildPolicyFilter(m2m.secondModel, undefined, 'update');
843+
const filterB = newlyCreatedModel === m2m.secondModel
844+
? trueNode(this.dialect)
845+
: this.buildPolicyFilter(m2m.secondModel, undefined, 'update');
835846
const queryB = eb
836847
.selectFrom(m2m.secondModel)
837848
.where(eb(eb.ref(`${m2m.secondModel}.${m2m.secondIdField}`), '=', bValue))
@@ -850,7 +861,7 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
850861
if (!result.rows[0]?.$conditionA) {
851862
throw createRejectedByPolicyError(
852863
m2m.firstModel,
853-
RejectedByPolicyReason.CANNOT_READ_BACK,
864+
RejectedByPolicyReason.NO_ACCESS,
854865
`many-to-many relation participant model "${m2m.firstModel}" not updatable`,
855866
);
856867
}
@@ -863,6 +874,19 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
863874
}
864875
}
865876

877+
private extractNewlyCreatedModel(node: InsertQueryNode): string | undefined {
878+
const prefix = '/*ZS:M2M_CREATED:';
879+
for (const modifier of node.endModifiers ?? []) {
880+
if (RawNode.is(modifier)) {
881+
const fragment = modifier.sqlFragments[0];
882+
if (fragment?.startsWith(prefix)) {
883+
return fragment.slice(prefix.length, -2); // strip prefix and trailing */
884+
}
885+
}
886+
}
887+
return undefined;
888+
}
889+
866890
private async enforcePreCreatePolicyForOne(
867891
model: string,
868892
fields: string[],
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { createPolicyTestClient } from '@zenstackhq/testtools';
2+
import { describe, expect, it } from 'vitest';
3+
4+
// https://github.com/zenstackhq/zenstack/issues/2531
5+
// Many-to-many update check failing with connect:
6+
// Creating a Child with a nested `connect` on a many-to-many relation incorrectly
7+
// fails with "model not updatable" because the update policy check runs before
8+
// the connection is established.
9+
describe('Regression for issue #2531', () => {
10+
const schema = `
11+
model User {
12+
id Int @id @default(autoincrement())
13+
name String
14+
children Child[]
15+
16+
@@allow('all', true)
17+
}
18+
19+
model Child {
20+
id Int @id @default(autoincrement())
21+
name String
22+
parents User[]
23+
24+
@@allow('create', auth() != null)
25+
@@allow('read,update', auth() != null && parents?[id == auth().id])
26+
}
27+
`;
28+
29+
it('should allow creating a child with connect to a many-to-many parent', async () => {
30+
const db = await createPolicyTestClient(schema, { usePrismaPush: true, debug: true });
31+
32+
const user = await db.user.create({ data: { name: 'Alice' } });
33+
34+
const authedDb = db.$setAuth(user);
35+
36+
// This should succeed: after the create+connect, the child will have
37+
// the authenticated user as a parent, satisfying the read-back policy.
38+
const child = await authedDb.child.create({
39+
data: {
40+
name: 'Child1',
41+
parents: {
42+
connect: { id: user.id },
43+
},
44+
},
45+
});
46+
47+
expect(child).toMatchObject({ name: 'Child1' });
48+
});
49+
50+
it('should deny creating a child without connecting to the authenticated user', async () => {
51+
const db = await createPolicyTestClient(schema, { usePrismaPush: true });
52+
53+
const user = await db.user.create({ data: { name: 'Alice' } });
54+
const otherUser = await db.user.create({ data: { name: 'Bob' } });
55+
56+
const authedDb = db.$setAuth(user);
57+
58+
// Connecting to a different user's id should fail the read-back policy
59+
// since the authenticated user is not a parent of the resulting child.
60+
await expect(
61+
authedDb.child.create({
62+
data: {
63+
name: 'Child2',
64+
parents: {
65+
connect: { id: otherUser.id },
66+
},
67+
},
68+
}),
69+
).toBeRejectedByPolicy();
70+
});
71+
});

0 commit comments

Comments
 (0)