Skip to content

Commit a08e059

Browse files
fix(objectql): a formula field and a CEL defaultValue answer current_user.can() from the security service (#20082) (#20138)
Fixes #20082 Clause-②: no (narrowing) A `formula` field and a CEL `defaultValue` that call `current_user.can(object, verb)` now get the acting subject's effective object permissions. That is the map PR #20079 wired for option visibility. It comes through `ObjectQL.registerEffectiveObjectPermissionsResolver` and formula's `toEvalPermissions`, and it is resolved at most once per engine operation. ## The two sites, base against head Measured one-shot through a real `ObjectQL` with a SQL driver (better-sqlite3 `:memory:`) and a real `SecurityPlugin`. The caller holds `crm_account: allowRead + allowEdit`, so `edit` is the granted verb and `delete` the denied one. Base is `55daf89d74`; head is this branch. The same cells are pinned permanently in `packages/objectql/src/engine-formula-default-permission.test.ts` with a resolver double. | site | granted | denied | no resolver | resolver throws | |:--|:--|:--|:--|:--| | formula field on `find`, `findOne`, insert echo, update echo | base `null`, no log; head `true` | base `null`, no log; head `false` | base `null`, no log; head `null` plus one `warn` per operation, `reason: 'no-permission-source'` | base `null`, no log; head `null` plus one `warn` per operation carrying the error, `reason: 'permission-resolution-failed'` | | CEL `defaultValue` on insert | base unset plus `warn`; head stores `true` | base unset plus `warn`; head stores `false` | unchanged: unset plus the existing `warn`, which carries formula's "carries no permission data" refusal | base unset plus `warn`, row written; head the insert is refused with the resolver's own error, and nothing is written | | a `required` field with that default | base refused `VALIDATION_FAILED` / `required`; head admitted (`true`) | head admitted (`false`) | unchanged: refused `VALIDATION_FAILED` / `required` | head refused with the resolver's own error | At base the resolver was asked 0 times by either site, whether it granted, withheld or threw. That is H1, confirmed. ## The rule each site follows (H3) - **Formula field (a read).** Today a formula that does not evaluate reads `null` and logs nothing: `applyFormulaPlan` assigns `r.ok ? … : null`. A read cannot refuse a row over one computed field, so a `can` formula with no map keeps that `null`. It is no longer silent: the engine logs one `warn` per operation naming the object, the `can` fields and the reason. A throw fails closed. It is never read as "no grants", which would make an empty map answer `false`, and never as a grant. - **CEL default (a write).** Today a default that does not evaluate is left unset with the `warn` "Failed to evaluate default expression". A `required` field so defaulted is then refused by required-validation (measured at base with the real plugin: `VALIDATION_FAILED`, field `d_req`, code `required`). With no resolver that rule is kept byte for byte: the absent member passes NO map. A throw fails closed the way the option gate does. A row whose `can` default needed the map is refused with the resolution's own error, re-raised untouched. Under `insertMany` only that row is refused, and the `validate()` preview rejects the same way. A row that supplies the field is never refused by a resolution it did not need. ## Where the map comes from, and how often (H2) - `permissionResolution(context)` is one lazy, memoised resolver ask per engine operation. It is `undefined`, meaning no map, when no resolver is registered or the operation has no acting user. The same conditions apply to the option gate. - **Read path.** `find` and `findOne` resolve after the driver returns, and only when a planned formula calls `can` and at least one row came back. They use `opCtx.context`, which is the context the security middleware already ran with. So `plugin-security`'s per-context permission-set memo serves the resolution. - **Write path.** One resolution per write is shared by the CEL defaults, the re-default after the static-`readonly` strip, the option gates and the formula fields on the response. `resolveOptionPermissions` now receives the write's resolution instead of calling the resolver itself. When it asks, and what a throw does, are unchanged; its 13-case suite passes as it was. - The "needs the map" test for both sites is `readsPermissionPredicate`, the option gate's own AST reading of a receiver `can` call. It is exported from `rule-validator.ts` so that there is one detector, not two. It is not re-exported from the package entry. Resolver asks per operation (pinned): | operation | base | head | |:--|--:|--:| | `find` over 4 rows with a `can` formula | 0 | 1 | | `findOne`, and a by-id update echo | 0 | 1 each | | insert with a `can` default and a `can` formula echo | 0 | 1 | | insert that picks a `can`-gated option AND defaults a `can` field AND echoes a `can` formula | 1 | 1 | | batch insert of 6 rows | 0 | 1 | | `validate()` over 2 rows | 0 | 1 | | two consecutive `find`s | 0 | 2 (never kept across operations) | | object with no `can` anywhere, even with a throwing resolver | 0 | 0 | | system read (no acting user) | 0 | 0 | ## Declaration (H4) - **Narrowing.** When the resolution fails, an insert whose `can` default needed it is now refused. At base that row was written with the field unset. So the changeset and this body carry `Clause-②: no (narrowing)`, a **BREAKING** banner and `adr-0087: not-required (no-migration-prescription)`. No authored key, stored shape, export or route changes. - **The claim reads `Clause-②: no`.** The arm is added here because the dispatch's H4 says a write whose default now fails closed is a narrowing to declare. The seat owns the claim line. - **Reachability of that path.** In the shipped composition, `SecurityPlugin`'s middleware resolves the same memoised permission sets before the write and already refuses on a resolution failure. The newly refused path is therefore reachable only when `buildEffectiveObjectPermissions` throws, when the map is off-shape, or with a third-party resolver. - **Widening.** No key, export or route is added. A `required` field defaulted by `can` is now admitted where it was always refused. That is a declared default finally evaluating, not a new surface. ## Tests Head is `4fcfbed346`. Each line names the commit it ran at. The only commits after `9e622fbedd` edit the new test file: they type its options objects and make its driver refuse unknown WHERE combinators. - **Base red.** The new pin against `engine.ts` and `rule-validator.ts` restored to `55daf89d74` (blob `a9ec130693` = base blob), then restored to HEAD (blobs `15ca43c2eb` and `90cec7aea7` = HEAD, `git diff HEAD` empty) gave `Tests 27 failed | 3 passed (30)`. For example, "expected null to be true" (granted `find`), "expected [] to have a length of 1 but got +0" (no-resolver warn), and "expected ValidationError: flag is required to be Error: permission store unreachable" (throw on a required default). The 3 that pass are controls that must hold on both sides: no-resolver on a required default, no `can` anywhere, and a system read. This ran at `ca6dea2249`, with 30 cases; the 31st case, the `validate()` rejection, was added after it. - **Head (`4fcfbed346`).** The new pin plus `engine-option-permission-predicate` gave `Test Files 2 passed (2)` and `Tests 44 passed (44)`, which is 31 plus 13. - **Ablation (at `ca6dea2249`; src-resolved, so no build).** I ran `scripts/ablation-replace.mjs` to replace the memo's `pending ??=` with `pending =`. The anchor went 1 to 0, the marker 0 to 1, and the blob went `15ca43c2eb` to `592f152bbd`. The pin then gave `Tests 4 failed | 26 passed (30)`: every "one resolution" cell got 2 or 3 asks. The file was restored with blob == HEAD and `git diff HEAD` empty. - **`@objectstack/objectql`.** `vitest run --project local` gave `Test Files 315 passed (315)` and `Tests 5373 passed (5373)`. It ran at `9e622fbedd`; the only later commit retypes the options objects in the new test file. `--project repo` gave 1 file, 5 tests passed. `typecheck` (src, scripts and the test layer) exited 0 at `4fcfbed346`, and the test layer still holds 40 files and 234 errors in the debt ledger. The new test file is in `tsconfig.test.json`'s program (`--listFiles`) with 0 errors. - **`@objectstack/plugin-security` (`9e622fbedd`).** The full suite, run with `objectql` aliased to source, gave `Test Files 135 passed (135)` and `Tests 2676 passed (2676)`. - **Dogfood (`55ec8feee6`).** On a closure built by turbo (64 tasks), `field-zoo-roundtrip`, `field-zoo-value-shape`, `showcase-static-readonly` (the re-default path), `showcase-fls-read-mask-strip` and `expression-conformance` gave `Test Files 5 passed (5)` and `Tests 109 passed (109)`. - **Targeted neighbours (`9e622fbedd`).** `engine-option-permission-predicate`, `rule-validator.option-visibility`, `engine-write-formula-hydration`, `engine-formula-scale`, `engine-cel-default-temporal-shape`, `engine-default-value-tokens` and `record-title`, run with the new pin, gave `Test Files 8 passed (8)` and `Tests 170 passed (170)`. - **eslint, narrowed (`4fcfbed346`).** `eslint --no-inline-config --format json` on the 3 changed `.ts` files found 3 files, 0 errors and 0 warnings. The population is read from `eslint.config.mjs`, and no file was reported ignored. The config sets no `parserOptions.project`, so no type-aware rule can move a verdict on an untouched file. ## Gates - **Derivation.** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `4fcfbed346` derived 65 commands. - **Run.** All 65 were run at `4fcfbed346`, and all 65 exited 0. - **Reconciliation.** `--ran` reported `65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN`. - **Fixed on the way.** Two families were red at `9e622fbedd` and are green at head: - `check:query-options-erasure`: the new test's options objects were erased to `any`, and the test surface grew from 236 to 244 sites. They are now typed, and the surface is back to 236. - `check:where-matcher`: the test driver read an unknown `$` key as a field name. It now refuses it. - **Prerequisite, then measured.** `check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET) on the first pass. Later gates in the list built the missing dists, and it exits 0 at head. A CJS/ESM load probe of `packages/objectql/dist` sees `ObjectQL` and `evaluateFormulaField` on both. - **`check-changeset-no-major`, fed this body as a `pull_request` payload (`--event`).** It reported `LEVEL AXIS: this PR declares clause-② no (narrowing), and no package whose packages/**/src/** it moves is graded patch`. - **`check-adr-0087-registration`.** It reported `1 declared-breaking changeset(s), each carrying an ADR-0087 disposition` and `[BREAKING+clause-②-narrowing] not-required (no-migration-prescription)`. - **`check-issue-citations --base 55daf89`.** Exit 0: `17 resolves`. - **Left to CI.** The derivation names 5 path-scheduled CI jobs and 4 type-check lanes. They are CI's own runs, and they are NOT MEASURED locally. ## Acceptance notes - `carrier:` 承接者:无. The expression-conformance ledger row `cel-formula` declares `failPolicy: 'fail-soft-log'`, but a formula that faults for any other reason still reads `null` with no log line (`applyFormulaPlan`'s `r.ok ? … : null`). This PR logs only the `can` case, which is its own. This is read off the code, not measured at a public door. - `carrier:` 承接者:无. `evaluateFormulaField` and `resolveRecordTitle` are synchronous and hold no resolver, so a title formula calling `can` still yields `null` there. The docblock and the changeset say so. - `carrier:` 承接者:无. Each `expand` of a related object is its own `find`, so it asks the resolver again. `plugin-security`'s per-context memo absorbs the set resolution; the map itself is rebuilt. - `carrier:` 承接者:无. A system read, which has no acting user, of a `can` formula reads `null` with no warn, as any `current_user` formula does with no subject. ## Deviations from the claim's file surface - `packages/objectql/src/validation/rule-validator.ts` gets `export` on `readsPermissionPredicate`, plus a three-line docblock note, so that there is one `can` detector. - In `engine.ts`, beyond the bodies of `applyFormulaPlan` and `applyFieldDefaults`: - their call sites in `find`, `findOne`, `insert`, `update` and `validate`; - `hydrateWriteFormulas`, which is now async and takes a permissions callback; - four private helpers; - `resolveOptionPermissions`, which now takes the shared resolution. H2 requires that for "at most once per write" across both uses. Its behaviour is unchanged. - `packages/core`, `packages/formula` and PR #20117's `aggregate` region are untouched. --- _Generated by [Claude Code](https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 95ab93f commit a08e059

4 files changed

Lines changed: 834 additions & 42 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
'@objectstack/objectql': minor
3+
---
4+
5+
fix(objectql): a `formula` field and a CEL `defaultValue` answer `current_user.can(object, verb)` from the security service (#20082)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) no authored key, stored shape, export or route changes; the narrowing is a runtime refusal when the permission resolution fails, and there is nothing for `objectstack migrate meta` to rewrite -->
10+
11+
**BREAKING**, for one fault path only, shipped as `minor` under the launch-window convention (`check-changeset-no-major` refuses `major` until GA, so this banner and the ADR-0087 disposition above carry the breaking-ness). An insert row whose CEL `defaultValue` calls `current_user.can(…)` is now refused with the resolution's own error when the registered security service fails to resolve the caller's effective object permissions. Before, that default was left unset with a `warn` and the row was written.
12+
13+
`@objectstack/formula` answers `can` from `EvalContext.permissions`, and neither engine site passed one. With the security plugin registered:
14+
15+
- a formula field calling `current_user.can(…)` read `null` on every `find`, `findOne` and write response, and logged nothing;
16+
- a CEL `defaultValue` calling it was left unset with the `warn` "Failed to evaluate default expression". A `required` field defaulted that way therefore refused every insert.
17+
18+
**What changes.** Both sites now evaluate with the acting subject's effective object permissions. That is the map `ISecurityService.getEffectiveObjectPermissions` returns, which an option's `visibleWhen` already reads.
19+
20+
- A formula field reads `true` or `false`.
21+
- A CEL default stores `true` or `false`, so a `required` field defaulted by `can` is admitted.
22+
23+
The engine asks the security service at most once per operation: once per `find` (not per row), and once per write, shared by its defaults, its `can`-gated options and the formula fields on its response. It asks only when a formula, or a default that will be applied, calls `can`, and only when the operation has an acting user. The answer is never kept past the operation.
24+
25+
**When there is no map.**
26+
27+
- No security service is registered. No permission data is passed, as before. A formula field still reads `null`, and each operation now logs one `warn` naming the object, the fields and `reason: 'no-permission-source'`. A default is still left unset with its existing `warn`.
28+
- The resolution fails: it throws, or it returns a map that is not the published shape. A formula field reads `null`, and one `warn` carries the error with `reason: 'permission-resolution-failed'`, because a read is not refused over one computed field. An insert row whose `can` default needed the map is refused with the resolution's own error. Under `insertMany` only that row is refused, and the `validate()` preview rejects the same way. Rows that supply the field, and objects whose defaults never call `can`, are unaffected.
29+
30+
In no case is `can()` answered `true` without a grant, or `false` from an empty map.
31+
32+
`evaluateFormulaField` (and `resolveRecordTitle`, which uses it) is synchronous and passes no map, so a formula calling `can` still yields `null` there.
33+
34+
No spec key, export or route is added or removed.

0 commit comments

Comments
 (0)