Skip to content

feat(policy): add optional error code to @@allow/@@deny attributes#2751

Draft
Azzerty23 wants to merge 27 commits into
zenstackhq:devfrom
Azzerty23:refactor/query-context-sql-comment
Draft

feat(policy): add optional error code to @@allow/@@deny attributes#2751
Azzerty23 wants to merge 27 commits into
zenstackhq:devfrom
Azzerty23:refactor/query-context-sql-comment

Conversation

@Azzerty23

Copy link
Copy Markdown
Contributor

What this PR does

Adds an optional errorCode argument to @@allow and @@deny policy attributes. When an operation is rejected, every matching rule's code is collected and surfaced on ORMError.policyCodes, giving callers a reliable way to handle specific policy violations programmatically.

Fixes #1723

Supersedes #2636 — same feature, reworked internals. See Changes since #2636 below.


Schema syntax

The third argument accepts a string literal or an enum value:

enum OrderError {
  OUT_OF_STOCK
  CREDIT_EXCEEDED
  INVALID_STATUS_TRANSITION
}

model Order {
  ...
  @@deny('create',        total > auth().creditLimit,                            CREDIT_EXCEEDED)
  @@deny('post-update',   stock < 0,                                             OUT_OF_STOCK)
  @@deny('post-update',   before().status == 'shipped' && status != 'delivered', INVALID_STATUS_TRANSITION)
  @@deny('create,update', auth().accountStatus == 'suspended',                   'ACCOUNT_SUSPENDED')
  @@deny('read',          archived == true,                                      'ARCHIVED_ORDER')
  @@deny('delete',        auth().role != 'admin',                                'ADMIN_ONLY')
}

Error surfacing

ORMError.policyCodes contains the codes of all matching rules (not just the first), so a single failed operation can carry multiple codes:

try {
  await db.order.create({ data: { ... } });
} catch (err) {
  if (err instanceof ORMError && err.reason === ORMErrorReason.REJECTED_BY_POLICY) {
    // err.policyCodes: e.g. ['CREDIT_EXCEEDED', 'ACCOUNT_SUSPENDED']
  }
}

Scope: which operations surface codes

Operation Without error code With error code
create / post-update REJECTED_BY_POLICY (no codes) REJECTED_BY_POLICY + policyCodes
update / delete / findFirstOrThrow / findUniqueOrThrow NOT_FOUND REJECTED_BY_POLICY + policyCodes
findFirst / findUnique returns null returns null (never throws)
findMany returns [] returns [] (never throws)

create and post-update piggyback code collection on the policy checks that already run — no additional round-trips. update/delete/*OrThrow use filter-based enforcement, so distinguishing "not found" from "denied" requires a diagnostic query; it only runs when at least one rule on the model carries a code, so the opt-in is implicit via the schema.

Performance opt-out

// plugin-level: off for all queries on this client
new PolicyPlugin({ fetchPolicyCodes: false })

// query-level: off (or back on) for a single call
await db.order.update({ where: { id }, data: { ... }, fetchPolicyCodes: false });
await db.order.update({ where: { id }, data: { ... }, fetchPolicyCodes: true });  // overrides plugin-level false

Backward compatibility

Purely additive. No changes to the existing error message format or ORMError structure. Callers that don't use error codes see no behavioral change. policyCodes is undefined (not []) when no codes are present.


Changes since #2636

The feature surface is unchanged; the plumbing that carries per-call context (fetchPolicyCodes, originating ORM operation) from the client API down to onKyselyQuery hooks was rewritten:

  • SQL-comment query context instead of an AsyncLocalStorage bridge. The ORM now serializes a QueryContext (model, CRUD kind, top-level ORM operation, plugin args) as a trailing -- $$context: comment on generated queries; the executor deserializes and strips it before plugins and the driver see the SQL. This removes the node:async_hooks dependency entirely (no browser-bundle concerns) and makes the context robust across transaction/executor boundaries.
  • Fewer round-trips. Post-mutation rejection determination and diagnostics are merged into a single query where possible.
  • MySQL update pre-load: selective read-policy bypass. On dialects without RETURNING, the ORM pre-loads entity IDs with a SELECT before a top-level UPDATE. That pre-load must bypass the policy read filter (otherwise a read-denied row silently becomes NOT_FOUND, masking update-deny codes) — but only the policy filter. It is now tagged with a bypassReadPolicy query context that the policy plugin honors, while every other onKyselyQuery plugin (e.g. soft-delete) still transforms the query.

🤖 Generated with Claude Code

https://claude.ai/code/session_011wibXxJtELqNeDYwu3gyTU

Azzerty23 and others added 26 commits May 1, 2026 14:10
Adds an optional third `errorCode` string argument to @@Allow, @@deny,
@Allow, and @deny policy attributes. When a create or post-update
operation is rejected, the matching rule's code is surfaced on
ORMError.policyCode so callers can handle specific policy violations
without parsing error messages.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t-match only

Changes `ORMError.policyCode` to `policyCodes: string[]` so every rule
that contributed to a rejection (deny rules that fired, allow rules that
failed) is reported. The diagnostic query path is also collapsed from N
sequential per-rule round-trips into a single batched query with one
EXISTS column per coded policy, which is both faster and correct for the
multi-code case.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a `fetchPolicyCodes` flag to `PolicyPluginOptions` (plugin-level default)
and as a per-query arg on `$create`/`$update`, using AsyncLocalStorage to
bridge the flag into the Kysely executor. Set to `false` to skip the extra
diagnostic query when error codes are not needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… errorCode support

- Fix bug: allow rules in diagnostic queries now use EXISTS(NOT condition)
  so a batch violation is detected even when some rows pass (previously
  NOT EXISTS(condition) was used, which only fired when ALL rows failed)
- Simplify evaluatePolicyDiagnostics: negate at build time so the filter
  is a uniform row[`$c${i}`] check instead of kind-branching
- Remove errorCode param from field-level @allow/@deny — codes are not
  surfaced at runtime for field-level policies, so the grammar and
  validator no longer accept them
- Remove the 200-char length limit check from validateCustomErrorCode
- Add 3 new tests: batch updateMany allow violation, enum code on
  post-update, mixed enum + string codes
- Tighten fetchPolicyCodes opt-out assertions to toBeRejectedByPolicy(undefined, [])

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds post-check logic that runs when 0 rows are affected by an update or
delete, distinguishing "row not found" from "denied by policy" and
collecting matching policy codes in a single combined diagnostic query.
Extends fetchPolicyCodes query-level override to $delete, and adds
corresponding E2E tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…wsCheck

Rename and tighten the zero-rows diagnostic helper for update/delete:
- Skip the diagnostic query entirely when no policies carry an error code,
  removing an unnecessary round-trip for the common case.
- Fix BigInt comparison: use `> 0` negation instead of `=== 0` so
  numAffectedRows is handled correctly across drivers.
- Reorder delete/update branches so the more specific guard (delete has
  no `using` restriction) runs first.
- Inline the fetchPolicyCodes guard into a single early-exit path.

Tests: add "does not throw for nonexistent row" cases for update and delete,
and a focused test asserting the opt-in behaviour — rules without an errorCode
keep the existing NotFound error, rules with an errorCode surface
RejectedByPolicy instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ctations

Move the fetchPolicyCodes===false early-return before policy filtering so the
diagnostic query is skipped entirely for update/delete, making those operations
behave identically to a model with no error codes (NOT_FOUND). Update test
expectations accordingly and add an explicit equivalence test.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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>
…r dialects without RETURNING

On dialects lacking RETURNING support (e.g. MySQL), the ORM pre-loads
entity ID fields before an UPDATE to identify the row for re-reading.
This pre-load ran through onKyselyQuery plugin hooks, causing a
read-policy denial to surface as "Record not found" before the UPDATE
executed — masking the correct error code.

Introduce internalQueryContextStorage (AsyncLocalStorage) to mark these
internal pre-load queries; ZenStackQueryExecutor now skips all
onKyselyQuery hooks when the flag is set.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add a browser export condition to @zenstackhq/orm so that browser bundlers
(Turbopack, webpack) use an empty stub instead of dist/index.mjs, which
imports node:async_hooks via internal-context.ts. Also include the Node.js
version in the CI pnpm cache key to prevent native module (better-sqlite3)
ABI mismatches after a Node.js upgrade.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On non-RETURNING dialects (MySQL), the ORM pre-loads entity IDs before
an UPDATE. If the row is read-denied, the pre-load returns null and the
UPDATE never runs, masking the real update-deny error code.

Adds `requiresUpdatePreloadBypassReadPolicy` to the dialect interface
(true for MySQL), `executeQueryDirect` to ZenStackQueryExecutor to run
a query without onKyselyQuery interceptors, and `readUniqueDirect` in
the base operation handler to use it for the pre-load step.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rectly to executeQueryDirect

Eliminates the withConnectionProvider/SingleConnectionProvider/scopedExecutor
indirection in readUniqueDirect. executeQueryDirect now takes a DatabaseConnection
parameter and delegates straight to internalExecuteQuery, removing the redundant
provideConnection wrapper and double ensureProperQueryResult call. Also removes the
dead ?? null on the bypass branch and trims the 8-line narration comment.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eries for dialects without RETURNING"

This reverts commit 885a04c.
Embed the top-level ORM call context (model, CRUD kind, API method,
plugin ext query args) into generated queries as a trailing
`-- $$context:` comment, and surface it to `onKyselyQuery` hooks via a
new `queryContext` field on `OnKyselyQueryArgs`.

- uncomment `CONTEXT_COMMENT_PREFIX` and implement `makeContextComment`
  (no-op when no plugin registers `onKyselyQuery`)
- capture the call context in `createPromise` using the final args
  (after `onQuery` overrides); handlers carry it via an immutable
  `withCallContext` clone so parallel `proceed` calls can't interleave
- extract the context from the query AST (`endModifiers`) at executor
  entry and strip it so neither hooks nor the database driver ever see
  the comment
- share plugin ext-args schema resolution between the zod factory and
  the new `extractPluginQueryArgs` helper

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K9XE6XoZuquE1W8Q3j3W8h
…uery context

Drop the `onQuery` hook and the AsyncLocalStorage store that bridged it
to `onKyselyQuery`; read the ORM operation and `fetchPolicyCodes` from
the `queryContext` now provided in the hook args instead. The zod
`queryArgs` schemas stay, as input validation and as the source of
truth for which ext args get embedded into the context.

Also drop the `@types/node` dev dependency, no longer needed without
`node:async_hooks`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K9XE6XoZuquE1W8Q3j3W8h
The browser export stub only existed to keep `node:async_hooks` out of
client bundles; with the policy plugin no longer using
AsyncLocalStorage there is no such import left in the workspace.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K9XE6XoZuquE1W8Q3j3W8h
…ction

Rejection determination and code diagnostics now happen in the same
query as the policy check instead of a follow-up diagnostic query:

- pre-create: the check query carries one `EXISTS(...) AS $c<i>` column
  per coded policy, so a rejected create resolves its codes without a
  second roundtrip; the check also covers all rows of a multi-row
  INSERT at once (one query per statement instead of one per row),
  reporting the union of codes across violating rows
- post-update: the code diagnostic columns are appended to the
  post-update check query itself, removing the extra query on rejection

The zero-rows check was already a single query and is unchanged, as is
the per-row loop for implicit many-to-many join tables.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K9XE6XoZuquE1W8Q3j3W8h
…ract

PolicyExtQueryArgs declared its per-operation keys as required, so a
client extended via $use(new PolicyPlugin()) was no longer assignable
to ClientContract<Schema> (whose ext query args default to {}), breaking
the CLI proxy build. Make the keys optional (matching ExtQueryArgsBase)
and annotate queryArgs so $use infers the optional shape, then drop the
now-unneeded casts in the CLI proxy action.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wibXxJtELqNeDYwu3gyTU
The pre-load SELECT that resolves entity IDs before a top-level UPDATE on
non-RETURNING dialects used `executeQueryDirect`, skipping every
`onKyselyQuery` interceptor. This broke the soft-delete plugin: the
pre-load could see soft-deleted rows, so updating an already
soft-deleted delegate row resolved instead of throwing not-found.

Tag the pre-load with a `bypassReadPolicy` query context instead and let
the policy plugin skip its read filter for such SELECTs, while all other
plugins still transform the query. `executeQueryDirect` is now unused
and removed.

Also fix the post-update roundtrip-count test expectation on MySQL: the
[UPDATE, SELECT] sequence only holds for RETURNING dialects — MySQL
additionally needs the ORM id pre-load (now visible to interceptors),
the before-update entity load, and the post-update row read-back.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wibXxJtELqNeDYwu3gyTU
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 465204f5-fa7d-4311-8f21-f5194330574b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gitguardian

gitguardian Bot commented Jul 6, 2026

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
33485392 Triggered Generic Private Key 582b93b packages/cli/test/proxy.test.ts View secret
17897114 Triggered Generic Password 23a8253 .github/workflows/build-test.yml View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

The trigger was switched to bare `push:` to run CI on fork branches
during development; restore the upstream `pull_request` configuration.
The matrix-aware pnpm cache key is kept.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wibXxJtELqNeDYwu3gyTU
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Allow custom messages in policies

1 participant