feat(policy): add optional error code to @@allow/@@deny attributes#2751
feat(policy): add optional error code to @@allow/@@deny attributes#2751Azzerty23 wants to merge 27 commits into
Conversation
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
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
| 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
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- 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
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 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
What this PR does
Adds an optional
errorCodeargument to@@allowand@@denypolicy attributes. When an operation is rejected, every matching rule's code is collected and surfaced onORMError.policyCodes, giving callers a reliable way to handle specific policy violations programmatically.Fixes #1723
Schema syntax
The third argument accepts a string literal or an enum value:
Error surfacing
ORMError.policyCodescontains the codes of all matching rules (not just the first), so a single failed operation can carry multiple codes:Scope: which operations surface codes
create/post-updateREJECTED_BY_POLICY(no codes)REJECTED_BY_POLICY+policyCodesupdate/delete/findFirstOrThrow/findUniqueOrThrowNOT_FOUNDREJECTED_BY_POLICY+policyCodesfindFirst/findUniquenullnull(never throws)findMany[][](never throws)createandpost-updatepiggyback code collection on the policy checks that already run — no additional round-trips.update/delete/*OrThrowuse 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
Backward compatibility
Purely additive. No changes to the existing error message format or
ORMErrorstructure. Callers that don't use error codes see no behavioral change.policyCodesisundefined(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 toonKyselyQueryhooks was rewritten: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 thenode:async_hooksdependency entirely (no browser-bundle concerns) and makes the context robust across transaction/executor boundaries.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 becomesNOT_FOUND, masking update-deny codes) — but only the policy filter. It is now tagged with abypassReadPolicyquery context that the policy plugin honors, while every otheronKyselyQueryplugin (e.g. soft-delete) still transforms the query.🤖 Generated with Claude Code
https://claude.ai/code/session_011wibXxJtELqNeDYwu3gyTU