Skip to content

Commit 934d719

Browse files
Azzerty23claude
andcommitted
feat(policy): surface error codes for single-row read violations
Extend policy error code surfacing to `findFirst`/`findUnique` (and their `OrThrow` variants): when the query returns 0 rows, a diagnostic query now checks whether the row exists but was filtered by a deny/allow rule that carries an `errorCode`. If so, the handler throws `REJECTED_BY_POLICY` with the relevant codes instead of silently returning `null`. `findMany` is unaffected — it continues to use filter-based enforcement and returns an empty array for denied rows. This holds even for `findMany({ take: 1 })`, which generates LIMIT 1 SQL but remains a multi-row ORM operation by name. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 4dcdcec commit 934d719

7 files changed

Lines changed: 235 additions & 88 deletions

File tree

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,16 @@ export const AllReadOperations = [...CoreReadOperations, 'findUniqueOrThrow', 'f
168168
*/
169169
export type AllReadOperations = (typeof AllReadOperations)[number];
170170

171+
/**
172+
* List of single-row read operations — `findUnique`/`findFirst` and their 'orThrow' variants.
173+
*/
174+
export const SingleRowReadOperations = ['findUnique', 'findFirst', 'findUniqueOrThrow', 'findFirstOrThrow'] as const;
175+
176+
/**
177+
* List of single-row read operations.
178+
*/
179+
export type SingleRowReadOperations = (typeof SingleRowReadOperations)[number];
180+
171181
/**
172182
* List of all write operations - simply an alias of CoreWriteOperations.
173183
*/
@@ -312,6 +322,9 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
312322
const r = await kysely.getExecutor().executeQuery(compiled);
313323
result = r.rows;
314324
} catch (err) {
325+
// Re-throw ORMErrors (e.g. policy violations with custom error codes) as-is
326+
// to avoid wrapping them in a generic DBQueryError and losing their type/code.
327+
if (err instanceof ORMError) throw err;
315328
throw createDBQueryError(`Failed to execute query: ${err}`, err, compiled.sql, compiled.parameters);
316329
}
317330

packages/orm/src/client/errors.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,9 @@ export class ORMError extends Error {
9696
* Custom error codes from every policy rule that contributed to this rejection.
9797
* Set via the optional third argument of `@@allow` / `@@deny`. Only available when
9898
* `reason` is `REJECTED_BY_POLICY` and at least one matching rule carries a code.
99-
* Note: surfaced for `create`, `post-update`, `update`, and `delete` violations.
100-
* `read` uses filter-based enforcement and does not throw policy errors.
99+
* Note: surfaced for `create`, `post-update`, `update`, `delete`, and single-row `read`
100+
* violations. For `read`, only `findFirst`/`findUnique`-equivalent queries (LIMIT 1)
101+
* where a denied row exists will throw; `findMany` uses filter-based enforcement.
101102
*/
102103
public policyCodes?: string[];
103104

packages/orm/src/client/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export {
1313
CoreReadOperations,
1414
CoreUpdateOperations,
1515
CoreWriteOperations,
16+
SingleRowReadOperations,
1617
} from './crud/operations/base';
1718
export { InputValidator } from './crud/validator';
1819
export { ORMError, ORMErrorReason, RejectedByPolicyReason } from './errors';
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { AsyncLocalStorage } from 'node:async_hooks';
2+
3+
/**
4+
* Per-query context shared between the ORM hook (`onQuery`) and the Kysely handler (`onKyselyQuery`).
5+
* - `operation`: ORM operation name (e.g. `findUnique`, `create`) — used to distinguish single-row reads
6+
* and to skip the read diagnostic check for nested SELECTs inside mutations
7+
* - `fetchPolicyCodes`: per-query override of the plugin-level option
8+
*/
9+
export type PolicyContext = {
10+
operation?: string;
11+
fetchPolicyCodes?: boolean;
12+
};
13+
14+
export const policyContextStorage = new AsyncLocalStorage<PolicyContext>();

packages/plugins/policy/src/plugin.ts

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,15 @@
1-
import { AsyncLocalStorage } from 'node:async_hooks';
21
import { type OnKyselyQueryArgs, type RuntimePlugin } from '@zenstackhq/orm';
32
import type { SchemaDef } from '@zenstackhq/orm/schema';
43
import { z } from 'zod';
4+
import { policyContextStorage } from './context';
55
import { check } from './functions';
66
import type { PolicyPluginOptions } from './options';
77
import { PolicyHandler } from './policy-handler';
88

99
export type { PolicyPluginOptions } from './options';
1010

11-
type PolicyQueryContext = Pick<PolicyPluginOptions, 'fetchPolicyCodes'>;
12-
13-
const policyContextStorage = new AsyncLocalStorage<PolicyQueryContext>();
14-
1511
type PolicyExtQueryArgs = {
12+
$read: Pick<PolicyPluginOptions, 'fetchPolicyCodes'>;
1613
$create: Pick<PolicyPluginOptions, 'fetchPolicyCodes'>;
1714
$update: Pick<PolicyPluginOptions, 'fetchPolicyCodes'>;
1815
$delete: Pick<PolicyPluginOptions, 'fetchPolicyCodes'>;
@@ -42,23 +39,24 @@ export class PolicyPlugin implements RuntimePlugin<SchemaDef, PolicyExtQueryArgs
4239
}
4340

4441
readonly queryArgs = {
42+
$read: fetchPolicyCodesSchema,
4543
$create: fetchPolicyCodesSchema,
4644
$update: fetchPolicyCodesSchema,
4745
$delete: fetchPolicyCodesSchema,
4846
};
4947

5048
// onQuery and onKyselyQuery are decoupled hook call sites with no shared argument path;
51-
// AsyncLocalStorage bridges the per-query fetchPolicyCodes arg into the Kysely executor.
49+
// AsyncLocalStorage bridges per-query context into the Kysely executor.
5250
onQuery(ctx: {
51+
operation: string;
5352
args: Record<string, unknown> | undefined;
5453
proceed: (args: Record<string, unknown> | undefined) => Promise<unknown>;
5554
[key: string]: unknown;
5655
}) {
57-
const fetchPolicyCodes = ctx.args?.['fetchPolicyCodes'] as boolean | undefined;
58-
if (fetchPolicyCodes !== undefined) {
59-
return policyContextStorage.run({ fetchPolicyCodes }, () => ctx.proceed(ctx.args));
60-
}
61-
return ctx.proceed(ctx.args);
56+
return policyContextStorage.run(
57+
{ operation: ctx.operation, fetchPolicyCodes: ctx.args?.['fetchPolicyCodes'] as boolean | undefined },
58+
() => ctx.proceed(ctx.args),
59+
);
6260
}
6361

6462
onKyselyQuery({ query, client, proceed }: OnKyselyQueryArgs<SchemaDef>) {

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

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { invariant } from '@zenstackhq/common-helpers';
22
import type { BaseCrudDialect, ClientContract, CRUD_EXT, ProceedKyselyQueryFunction } from '@zenstackhq/orm';
3-
import { getCrudDialect, QueryUtils, RejectedByPolicyReason, SchemaUtils } from '@zenstackhq/orm';
3+
import { CoreWriteOperations, getCrudDialect, QueryUtils, RejectedByPolicyReason, SchemaUtils, SingleRowReadOperations } from '@zenstackhq/orm';
44
import {
55
ExpressionUtils,
66
type BuiltinType,
@@ -42,6 +42,7 @@ import {
4242
} from 'kysely';
4343
import { match } from 'ts-pattern';
4444
import { ColumnCollector } from './column-collector';
45+
import { policyContextStorage } from './context';
4546
import { ExpressionTransformer } from './expression-transformer';
4647
import type { PolicyPluginOptions } from './options';
4748
import type { Policy, PolicyOperation } from './types';
@@ -59,6 +60,9 @@ import {
5960
trueNode,
6061
} from './utils';
6162

63+
const SINGLE_ROW_READ_OPERATIONS = new Set<string>(SingleRowReadOperations);
64+
const ORM_WRITE_OPERATIONS = new Set<string>(CoreWriteOperations);
65+
6266
export type CrudQueryNode = SelectQueryNode | InsertQueryNode | UpdateQueryNode | DeleteQueryNode;
6367

6468
export type MutationQueryNode = InsertQueryNode | UpdateQueryNode | DeleteQueryNode;
@@ -93,8 +97,13 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
9397
}
9498

9599
if (!this.isMutationQueryNode(node)) {
96-
// transform and proceed with read directly
97-
return proceed(this.transformNode(node));
100+
const selectNode = node as SelectQueryNode;
101+
const result = await proceed(this.transformNode(node));
102+
// When 0 rows returned on a single-row read, distinguish "not found" from policy denial
103+
if (result.rows.length === 0 && SINGLE_ROW_READ_OPERATIONS.has(policyContextStorage.getStore()?.operation ?? '')) {
104+
await this.postReadZeroRowsCheck(selectNode, proceed);
105+
}
106+
return result;
98107
}
99108

100109
const { mutationModel } = this.getMutationModel(node);
@@ -142,9 +151,9 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
142151
// Use > 0 negation (not === 0) because numAffectedRows is BigInt in some drivers
143152
if (!((result.numAffectedRows ?? 0) > 0)) {
144153
if (DeleteQueryNode.is(node)) {
145-
await this.postMutationZeroRowsCheck(mutationModel, 'delete', node.where?.where, proceed);
154+
await this.postZeroRowsCheck(mutationModel, 'delete', node.where?.where, proceed);
146155
} else if (UpdateQueryNode.is(node)) {
147-
await this.postMutationZeroRowsCheck(mutationModel, 'update', node.where?.where, proceed);
156+
await this.postZeroRowsCheck(mutationModel, 'update', node.where?.where, proceed);
148157
}
149158
}
150159

@@ -259,38 +268,47 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
259268
}
260269
}
261270

262-
// Checks if any row matching the original WHERE exists without the policy filter.
263-
// Called when numAffectedRows == 0 for UPDATE or DELETE.
271+
// Called when a single-row read returns 0 rows. Skips internal reads (read-back after mutation).
272+
private async postReadZeroRowsCheck(node: SelectQueryNode, proceed: ProceedKyselyQueryFunction): Promise<void> {
273+
if (ORM_WRITE_OPERATIONS.has(policyContextStorage.getStore()?.operation ?? '')) return;
274+
if (!node.from || node.from.froms.length !== 1) return;
275+
const extractedTable = this.extractTableName(node.from.froms[0]!);
276+
if (!extractedTable) return;
277+
const { model } = extractedTable;
278+
if (!QueryUtils.getModel(this.client.$schema, model)) return;
279+
return this.postZeroRowsCheck(model, 'read', node.where?.where, proceed);
280+
}
281+
282+
// Checks if any row matching WHERE exists without the policy filter.
264283
// If a row exists but was filtered by policy → throws REJECTED_BY_POLICY with codes.
265-
// If no row matches → returns silently (ORM layer handles "not found").
266-
// Combines existence check and code diagnostics into a single query.
267-
private async postMutationZeroRowsCheck(
284+
// If no row matches → returns silently.
285+
private async postZeroRowsCheck(
268286
model: string,
269-
operation: 'update' | 'delete',
270-
originalWhere: OperationNode | undefined,
287+
operation: 'read' | 'update' | 'delete',
288+
whereCondition: OperationNode | undefined,
271289
proceed: ProceedKyselyQueryFunction,
272290
) {
273-
if (this.isManyToManyJoinTable(model)) return;
274291
if (this.tryGetConstantPolicy(model, operation) === true) return;
275292
if (this.options.fetchPolicyCodes === false) return;
276293
const policiesWithCode = this.getModelPolicies(model, operation).filter((p) => p.code);
277-
// Skip if no policies carry an error code — nothing to surface.
278294
if (policiesWithCode.length === 0) return;
295+
if (this.isManyToManyJoinTable(model)) return;
279296

280-
const whereCondition = originalWhere ?? trueNode(this.dialect);
297+
// No WHERE clause means "match all rows" — use a literal TRUE so the existence sub-query is valid SQL.
298+
const where = whereCondition ?? trueNode(this.dialect);
281299

282300
const rowExistsInner = this.eb
283301
.selectFrom(model)
284302
.select(this.eb.lit(1).as('_'))
285-
.where(() => new ExpressionWrapper(whereCondition));
303+
.where(() => new ExpressionWrapper(where));
286304

287305
const codeSelections = policiesWithCode.map((policy, i) => {
288306
const condition = this.compilePolicyCondition(model, undefined, operation, policy);
289307
const violationCondition = policy.kind === 'allow' ? logicalNot(this.dialect, condition) : condition;
290308
const inner = this.eb
291309
.selectFrom(model)
292310
.select(this.eb.lit(1).as('_'))
293-
.where(() => new ExpressionWrapper(conjunction(this.dialect, [whereCondition, violationCondition])));
311+
.where(() => new ExpressionWrapper(conjunction(this.dialect, [where, violationCondition])));
294312
return SelectionNode.create(
295313
AliasNode.create(this.eb.exists(inner).toOperationNode(), IdentifierNode.create(`$c${i}`)),
296314
);
@@ -312,10 +330,7 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
312330
const row = result.rows[0] ?? {};
313331
if (!row.$exists) return;
314332

315-
const policyCodes = policiesWithCode
316-
? policiesWithCode.filter((_, i) => row[`$c${i}`]).map((p) => p.code!)
317-
: undefined;
318-
333+
const policyCodes = policiesWithCode.filter((_, i) => row[`$c${i}`]).map((p) => p.code!);
319334
throw createRejectedByPolicyError(model, RejectedByPolicyReason.NO_ACCESS, undefined, policyCodes);
320335
}
321336

0 commit comments

Comments
 (0)