Skip to content

Commit 531e8d1

Browse files
committed
fix(orm): restore HasMany includes when PK/FK has a field access policy (#2674)
1 parent ed01275 commit 531e8d1

5 files changed

Lines changed: 227 additions & 13 deletions

File tree

packages/orm/src/client/crud/dialects/lateral-join-dialect-base.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
getDelegateDescendantModels,
1010
getManyToManyRelation,
1111
isRelationField,
12+
joinKeyRef,
1213
requireField,
1314
requireIdFields,
1415
requireModel,
@@ -139,14 +140,17 @@ export abstract class LateralJoinDialectBase<Schema extends SchemaDef> extends B
139140
const relationIds = requireIdFields(this.schema, relationModel);
140141
invariant(parentIds.length === 1, 'many-to-many relation must have exactly one id field');
141142
invariant(relationIds.length === 1, 'many-to-many relation must have exactly one id field');
143+
// Use raw-alias refs so field access policies wrapping PK/FK in CASE WHEN NULL do not break the join.
144+
const relationIdRef = joinKeyRef(this.schema, relationModel, relationModelAlias, relationIds[0]!);
145+
const parentIdRef = joinKeyRef(this.schema, model, parentAlias, parentIds[0]!);
142146
query = query.where((eb) =>
143147
eb(
144-
eb.ref(`${relationModelAlias}.${relationIds[0]}`),
148+
eb.ref(relationIdRef),
145149
'in',
146150
eb
147151
.selectFrom(m2m.joinTable)
148152
.select(`${m2m.joinTable}.${m2m.otherFkName}`)
149-
.whereRef(`${parentAlias}.${parentIds[0]}`, '=', `${m2m.joinTable}.${m2m.parentFkName}`),
153+
.whereRef(parentIdRef, '=', `${m2m.joinTable}.${m2m.parentFkName}`),
150154
),
151155
);
152156
} else {

packages/orm/src/client/crud/dialects/sqlite.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
getDelegateDescendantModels,
2222
getManyToManyRelation,
2323
getRelationForeignKeyFieldPairs,
24+
joinKeyRef,
2425
requireField,
2526
requireIdFields,
2627
requireModel,
@@ -359,32 +360,39 @@ export class SqliteCrudDialect<Schema extends SchemaDef> extends BaseCrudDialect
359360
const relationIds = requireIdFields(this.schema, relationModel);
360361
invariant(parentIds.length === 1, 'many-to-many relation must have exactly one id field');
361362
invariant(relationIds.length === 1, 'many-to-many relation must have exactly one id field');
363+
// Use raw-alias refs so field access policies wrapping PK/FK in CASE WHEN NULL do not break the join.
364+
const relationIdRef = joinKeyRef(this.schema, relationModel, relationModelAlias, relationIds[0]!);
365+
const parentIdRef = joinKeyRef(this.schema, model, parentAlias, parentIds[0]!);
362366
selectModelQuery = selectModelQuery.where((eb) =>
363367
eb(
364-
eb.ref(`${relationModelAlias}.${relationIds[0]}`),
368+
eb.ref(relationIdRef),
365369
'in',
366370
eb
367371
.selectFrom(m2m.joinTable)
368372
.select(`${m2m.joinTable}.${m2m.otherFkName}`)
369-
.whereRef(`${parentAlias}.${parentIds[0]}`, '=', `${m2m.joinTable}.${m2m.parentFkName}`),
373+
.whereRef(parentIdRef, '=', `${m2m.joinTable}.${m2m.parentFkName}`),
370374
),
371375
);
372376
} else {
373377
const { keyPairs, ownedByModel } = getRelationForeignKeyFieldPairs(this.schema, model, relationField);
374378
keyPairs.forEach(({ fk, pk }) => {
375379
if (ownedByModel) {
376-
// the parent model owns the fk
380+
// BelongsTo: model owns the FK, relation has the PK.
381+
// Use raw alias only for the PK side. The FK side stays as a plain ref so that
382+
// denying the FK intentionally hides the relation (the join evaluates to NULL).
377383
selectModelQuery = selectModelQuery.whereRef(
378-
`${relationModelAlias}.${pk}`,
384+
joinKeyRef(this.schema, relationModel, relationModelAlias, pk),
379385
'=',
380386
`${parentAlias}.${fk}`,
381387
);
382388
} else {
383-
// the relation side owns the fk
389+
// HasMany: relation owns the FK, model has the PK.
390+
// Use raw alias on both sides: the child's FK may be denied (to hide which parent
391+
// it belongs to) but the parent must still be able to fetch its children.
384392
selectModelQuery = selectModelQuery.whereRef(
385-
`${relationModelAlias}.${fk}`,
393+
joinKeyRef(this.schema, relationModel, relationModelAlias, fk),
386394
'=',
387-
`${parentAlias}.${pk}`,
395+
joinKeyRef(this.schema, model, parentAlias, pk),
388396
);
389397
}
390398
});

packages/orm/src/client/query-utils.ts

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,34 @@ export function isTypeDef(schema: SchemaDef, type: string) {
234234
return !!schema.typeDefs?.[type];
235235
}
236236

237+
/**
238+
* Prefix added to PK/FK columns in field-policy subqueries so join conditions
239+
* can reference the raw (never-nulled) value even when the field itself is
240+
* covered by a @deny rule that wraps it in CASE WHEN … THEN NULL.
241+
*/
242+
export const JOIN_KEY_RAW_PREFIX = '__zs_raw_';
243+
244+
/**
245+
* Returns true when the field has at least one @deny or @allow attribute declared,
246+
* meaning the policy handler may wrap it in CASE WHEN … THEN NULL in SELECT
247+
* subqueries and the join condition must use the raw-alias instead.
248+
*/
249+
export function fieldHasConditionalReadPolicy(schema: SchemaDef, model: string, field: string): boolean {
250+
const fieldDef = getField(schema, model, field);
251+
return fieldDef?.attributes?.some((a) => a.name === '@deny' || a.name === '@allow') ?? false;
252+
}
253+
254+
/**
255+
* Returns the column reference to use on the given side of a join condition.
256+
* When the field carries a @deny or @allow policy the policy handler adds a raw alias
257+
* (JOIN_KEY_RAW_PREFIX + fieldName) so the join is not broken by CASE WHEN NULL.
258+
*/
259+
export function joinKeyRef(schema: SchemaDef, model: string, tableAlias: string, field: string): string {
260+
return fieldHasConditionalReadPolicy(schema, model, field)
261+
? `${tableAlias}.${JOIN_KEY_RAW_PREFIX}${field}`
262+
: `${tableAlias}.${field}`;
263+
}
264+
237265
export function buildJoinPairs(
238266
schema: SchemaDef,
239267
model: string,
@@ -242,14 +270,25 @@ export function buildJoinPairs(
242270
relationModelAlias: string,
243271
): [string, string][] {
244272
const { keyPairs, ownedByModel } = getRelationForeignKeyFieldPairs(schema, model, relationField);
273+
const relationModel = requireField(schema, model, relationField).type;
245274

246275
return keyPairs.map(({ fk, pk }) => {
247276
if (ownedByModel) {
248-
// the parent model owns the fk
249-
return [`${relationModelAlias}.${pk}`, `${modelAlias}.${fk}`];
277+
// BelongsTo: model owns the FK, relation has the PK.
278+
// Use raw alias only for the PK side. The FK side stays as a plain ref so that
279+
// denying the FK intentionally hides the relation (the join evaluates to NULL).
280+
return [
281+
joinKeyRef(schema, relationModel, relationModelAlias, pk),
282+
`${modelAlias}.${fk}`,
283+
];
250284
} else {
251-
// the relation side owns the fk
252-
return [`${relationModelAlias}.${fk}`, `${modelAlias}.${pk}`];
285+
// HasMany: relation owns the FK, model has the PK.
286+
// Use raw alias on both sides: the child's FK may be denied (to hide which parent it
287+
// belongs to) but the parent must still be able to fetch its children.
288+
return [
289+
joinKeyRef(schema, relationModel, relationModelAlias, fk),
290+
joinKeyRef(schema, model, modelAlias, pk),
291+
];
253292
}
254293
});
255294
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,19 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
697697
);
698698
hasPolicies = hasPolicies || fieldHasPolicies;
699699
selections.push(selection);
700+
701+
// When a PK or FK field is wrapped in CASE WHEN … THEN NULL by a field access policy,
702+
// also expose the raw value under a stable alias so join conditions are not broken.
703+
// Used for PK (HasMany parent) and FK (HasMany child). BelongsTo parent FK is kept as
704+
// a plain ref intentionally: denying that FK is designed to hide the relation entirely.
705+
if (fieldHasPolicies && (fieldDef.id || fieldDef.foreignKeyFor)) {
706+
const rawAlias = `${QueryUtils.JOIN_KEY_RAW_PREFIX}${fieldDef.name}`;
707+
selections.push(
708+
SelectionNode.create(
709+
AliasNode.create(ColumnNode.create(fieldDef.name), IdentifierNode.create(rawAlias)),
710+
),
711+
);
712+
}
700713
}
701714

702715
if (!hasPolicies) {
Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
import { createPolicyTestClient } from '@zenstackhq/testtools';
2+
import { describe, expect, it } from 'vitest';
3+
4+
// https://github.com/zenstackhq/zenstack/issues/2674
5+
describe('Regression for issue #2674', () => {
6+
it('@deny on PK should not break include (HasMany)', async () => {
7+
const db = await createPolicyTestClient(
8+
`
9+
model User {
10+
id String @id @default(cuid())
11+
role String
12+
@@allow('all', true)
13+
}
14+
15+
model Post {
16+
id Int @id @default(autoincrement()) @deny('all', auth().role != 'ADMIN')
17+
title String @unique
18+
comments Comment[]
19+
@@allow('all', true)
20+
}
21+
22+
model Comment {
23+
id Int @id @default(autoincrement())
24+
content String
25+
postId Int
26+
post Post @relation(fields: [postId], references: [id])
27+
@@allow('all', true)
28+
}
29+
`,
30+
);
31+
32+
const admin = { id: 'admin-1', role: 'ADMIN' };
33+
const user = { id: 'user-1', role: 'USER' };
34+
35+
await db.$setAuth(admin).post.create({
36+
data: {
37+
title: 'Test Post',
38+
comments: { create: [{ content: 'Comment 1' }, { content: 'Comment 2' }] },
39+
},
40+
});
41+
42+
// Admin sees everything normally
43+
const adminResult = await db.$setAuth(admin).post.findUnique({
44+
where: { title: 'Test Post' },
45+
include: { comments: true },
46+
});
47+
expect(adminResult?.id).toBeGreaterThan(0);
48+
expect(adminResult?.comments).toHaveLength(2);
49+
50+
// USER role: id is denied (returns null) but include should still populate comments
51+
const userResult = await db.$setAuth(user).post.findUnique({
52+
where: { title: 'Test Post' },
53+
include: { comments: true },
54+
});
55+
expect(userResult?.id).toBeNull();
56+
expect(userResult?.comments).toHaveLength(2);
57+
});
58+
59+
it('@deny on FK hides the BelongsTo relation (by design)', async () => {
60+
const db = await createPolicyTestClient(
61+
`
62+
model User {
63+
id String @id @default(cuid())
64+
role String
65+
@@allow('all', true)
66+
}
67+
68+
model Post {
69+
id Int @id @default(autoincrement())
70+
title String
71+
comments Comment[]
72+
@@allow('all', true)
73+
}
74+
75+
model Comment {
76+
id Int @id @default(autoincrement())
77+
content String
78+
postId Int @deny('all', auth().role != 'ADMIN')
79+
post Post @relation(fields: [postId], references: [id])
80+
@@allow('all', true)
81+
}
82+
`,
83+
);
84+
85+
const admin = { id: 'admin-1', role: 'ADMIN' };
86+
const user = { id: 'user-1', role: 'USER' };
87+
88+
const post = await db.$setAuth(admin).post.create({
89+
data: { title: 'Test Post' },
90+
});
91+
92+
const comment = await db.$setAuth(admin).comment.create({
93+
data: { content: 'A comment', postId: post.id },
94+
});
95+
96+
// USER role: postId is denied — both the FK value and the relation are hidden (by design)
97+
const userResult = await db.$setAuth(user).comment.findUnique({
98+
where: { id: comment.id },
99+
include: { post: true },
100+
});
101+
expect(userResult?.postId).toBeNull();
102+
expect(userResult?.post).toBeNull();
103+
});
104+
105+
it('@deny on both PK and FK should not break include', async () => {
106+
const db = await createPolicyTestClient(
107+
`
108+
model User {
109+
id String @id @default(cuid())
110+
role String
111+
@@allow('all', true)
112+
}
113+
114+
model Post {
115+
id Int @id @default(autoincrement()) @deny('all', auth().role != 'ADMIN')
116+
title String @unique
117+
comments Comment[]
118+
@@allow('all', true)
119+
}
120+
121+
model Comment {
122+
id Int @id @default(autoincrement())
123+
content String
124+
postId Int @deny('all', auth().role != 'ADMIN')
125+
post Post @relation(fields: [postId], references: [id])
126+
@@allow('all', true)
127+
}
128+
`,
129+
);
130+
131+
const admin = { id: 'admin-1', role: 'ADMIN' };
132+
const user = { id: 'user-1', role: 'USER' };
133+
134+
await db.$setAuth(admin).post.create({
135+
data: {
136+
title: 'Test Post',
137+
comments: { create: [{ content: 'C1' }, { content: 'C2' }] },
138+
},
139+
});
140+
141+
// Find by non-denied field (title) so the WHERE is not affected by @deny on id
142+
const userResult = await db.$setAuth(user).post.findUnique({
143+
where: { title: 'Test Post' },
144+
include: { comments: true },
145+
});
146+
expect(userResult?.id).toBeNull();
147+
expect(userResult?.comments).toHaveLength(2);
148+
expect(userResult?.comments[0]?.postId).toBeNull();
149+
});
150+
});

0 commit comments

Comments
 (0)