Repository navigation
Commit 35ef501
Fixes #22445
Clause-②: yes
## Patch round 1 — contract review 6080151175, finding ①-8
**The defect the review found.** plugin-security's read mask
(`field-masker.ts` `maskRecord`) deletes a hidden field, but a field
under a partial-masking rule keeps its key and gets a masked value. The
round-0 column filter kept every key the read door returned
(`hasOwnProperty`). So for a partially masked column the preview judged
the raw stored value, which gave any caller who can read the row a
predicate over a value they only see masked.
**The mechanism now (`engine.ts`, `storedRows` block).** A stored column
is kept only where the read door served this caller that very value.
That is decided by `servedAsStored(served, stored)`, a module-private
structural comparison:
- `Object.is` decides primitives.
- `Date` compares by time; arrays and plain objects compare recursively.
- Any other object kind, and nesting past 16 levels, answers "not the
same" (fail closed).
Every other column is judged as empty. That covers a hidden column (key
deleted), a partially masked column (key kept, value replaced), a masked
secret, an `internal` column, and a read hook's rewrite. So the verdict
never depends on a value the caller could not have read in full.
- **File fields.** The visibility read passes
`RAW_FILE_VALUES_CONTEXT_KEY`, the spec-declared opt-out for a caller
whose subject is the stored form. A readable file reference therefore
compares as the id it stores, not as the read door's expanded object.
- **Why value equality and not a declared signal.** The engine can see a
field's `maskingRule` but not whether this caller holds the unmask
permission; plugin-security decides that. A declared-signal rule would
judge every masking-rule field empty for every caller, unmasked ones
included, and would miss any transformer that is not a `maskingRule` (a
masked secret, a read hook). The comparison asks the read door's own
answer for this caller.
- **Representation, measured on head `1f4ca1504b`.** A throwaway probe
(not committed) wrote one row of every common type through a real
engine, then compared the write's prior-row read with the read door's
row under a user context.
- `driver-sql` (SQLite, `better-sqlite3`): 21 columns, 0 differences.
That covers number, currency, percent, a 11-digit number, boolean true
and false, date, datetime, JSON, multiselect, select, text, empty text,
plus `id` and the seven injected system columns.
- `driver-memory`: 16 columns, 0 differences.
- Every pair was deep-equal, so no plain readable column turns empty
because of serialization on either driver. The formula case stays pinned
(read door evaluates it, store holds none, preview agrees with the
write).
- **Write-gate probe (optional defence in depth): not added.** The
related-row read sits behind `registerWriteGateProbe` because it is
SYSTEM-elevated. The stored-row read is not elevated: after this filter
its image holds only values the read door served this caller unchanged,
which the same caller can already read with `findOne`. Gating it behind
the write probe would protect nothing further. It would also make a
preview's verdict depend on a probe that this read does not need.
**Pins added (`validate-update-stored-row.test.ts` now 18 cases; one new
file
`packages/rest/src/validate-update-stored-row-representation.test.ts`, 2
cases).**
- **(d2)** A middleware shaped like the security plugin's mask keeps the
`country` key and replaces its value with its mask. The precondition
asserts the read door serves `country: '**'`. The verdict for `{
province: 'zj', id }` is deep-equal whether `cn` or `us` is stored:
`province` `invalid_option`, judged as empty.
- **(d3) Control.** A caller served the column unmasked gets the stored
row's verdict: admitted on `cn`; `province` `invalid_option` on `us`.
- **(d4) Representation control, real driver.** On `driver-sql` /
SQLite, eight `n_COLUMN` fields carry a `requiredWhen` over number,
boolean, date, datetime, JSON, multiselect, select and text. Clearing
every `n_COLUMN` gets `required` on all eight, from the preview and from
the by-id update alike, so each rule judged the stored value. Control:
clearing the columns too changes the answer (only the number rule
faults: `null > 10`), so an empty column would have shown.
- **File reference.** The read door expands the file id (precondition).
Clearing the memo the stored attachment requires gets `memo` `required`
from both the preview and the write.
**Ablations through `scripts/ablation-replace.mjs`.** Each anchor hit
once, and each restore was proven by blob == HEAD with `git diff HEAD`
empty.
- `servedAsStored` condition removed (back to key presence only): **1
failed / 17 passed** (d2).
- `RAW_FILE_VALUES_CONTEXT_KEY` removed from the visibility read: **1
failed / 17 passed** (the file pin).
**Sentences corrected.**
- The `storedRows` block comment and the `validate()` docblock now state
the served-as-stored rule.
- The changeset's "Read under the caller's access" paragraph now states
the same rule and the measured representation. `Clause-②: yes` and the
`minor` levels are unchanged.
- `protocol.zod.ts` says nothing about masking, and its sentence ("a row
the caller cannot read is judged on the supplied keys alone") stays
true, so it is untouched.
**Base.** `origin/main` was merged (`a6be68f2e6`, 9 commits). The merge
driver deferred `content/docs/references/api/protocol.mdx`. It was
regenerated from the merged tree by `check:generated` in its regenerate
mode and committed in `1f18eaddc0`, which discharged the deferral. The
joint diff carries the `label` row from #22323 plus this PR's `mode`
text. The branch is now 3 commits behind `origin/main` (`f66c440de9`).
Those commits touch none of this PR's files, so they were not merged.
**Evidence on head `f874bb5600`** (builds and tests ran under
`os-verify-lock`):
- `pnpm --filter @objectstack/objectql test`: **391 files, 7716
passed**; `test:repo` 5 passed.
- `pnpm --filter @objectstack/core test`: **85 files, 2261 passed**;
`test:repo` 48 passed.
- Typecheck exit 0 for objectql, core and rest, each with
`check:test-typecheck` OK.
- Rest: the nine `import-*.test.ts` files plus the new representation
file: **10 files, 115 passed**. This ran after building
`@objectstack/rest^...` and `driver-memory`; the objectql dist carries
`servedAsStored`.
- Narrowed lint: `eslint --no-inline-config --format json` over the 8
changed `.ts` files gave 8 results, 0 errors, 0 warnings. The config
enables no type-aware linting, so the diff cannot move a verdict on an
untouched file.
- Gates: `dispatch-gates --commands` derived **117** commands at
`f874bb5600`. All 117 ran, with exit codes captured before any pipe, and
all ended **exit 0**. Reconciliation with `--ran`: `117 derived, 117
run, 0 NOT-MEASURED, 0 UNRUN`.
- Two of the 117 first answered **exit 3** (prerequisite not met, no
measurement) inside the battery: `check:skill-examples` and
`check:dual-build-cjs-loads`. Both lacked package dists in the fresh
worktree. The battery's own `check:type-check-debt` build then supplied
the dists, and both were re-run at the same head: exit 0.
- Also run: `check:adr-anchors` exit 0 and `check-nul-bytes` exit 0.
## What this changes
An `update`-mode `validate()` preview now judges what the by-id update
judges: the stored row merged with the patch. Before this change, the
preview judged the patch alone.
- **Engine (`packages/objectql/src/engine.ts`, `validate()`).** A row
that carries its `id` is judged against the row that id names. The `id`
is the address every update door folds into the payload, and it is
classified by the write's own `resolveEngineUpdateDispatch`. The stored
row becomes the rules' `previous`, and their record is that row merged
with the patch, as on the by-id branch of `update()`. A reference that a
traversing rule reads is resolved from the same merge. That retires the
named limit the old `:13177` comment stated.
- **One source for the stored row.** The by-id update's prior-row read
is extracted into `readUpdatePriorRow`, and both `update()` and the
preview call it. It is the same driver read, with the same
`buildDriverOptions` and the same declared-columns shaping. The preview
therefore judges the row as stored, not the read door's served image.
- **Read under the caller's access (assumption 4; corrected in patch
round 1).** Before it reads anything, the preview asks the read door
(`findOne` under the caller's own context) whether the caller can see
the row. If the read door returns nothing, the preview judges the row on
the patch alone. A hidden row therefore gets exactly the verdict a
missing row gets. A stored column is kept only where the read door
served this caller that very value. A column the read door hid, or
served transformed (a partial mask keeps the key and replaces the
value), is judged as empty. A refused read propagates as raised.
- **Import runner (`packages/core/src/utils/import-runner.ts`).** The
dry run of a matched row calls `validateData` with `{ ...data, id:
matched.id }`. That is the same fold `updateData` applies to the write.
Nothing new crosses the protocol, and `ValidateDataRequestSchema` gains
no key.
- **The refusal names the real cause (`cel-fault.ts`,
`rule-validator.ts`).** Sometimes a rule reads a column the object
declares and the judged record does not hold it (a preview with no
stored row). The refusal used to say "which this object does not
declare". It now says the value was not supplied and no stored row was
read. `describeCelFault` takes an optional `isDeclaredColumn`, which the
four rule-validator refusal builders answer from the object's fields:
field rule, option gate, script and conditional. The answer is true only
for a key the source reads directly off `record` / `previous`. An
undeclared key keeps the old sentence. Hook conditions pass nothing and
are unchanged.
- **Spec text (cross-lane, `domain:spec`).** The
`ValidateDataRequest.mode` description said "`update` judges only the
supplied keys, matching a PATCH". The `ValidateDataResponse` note said
the preview has no prior record. This change makes both partly false, so
both are reworded. No key or type changes.
`content/docs/references/api/protocol.mdx` was regenerated with
`check:generated --fix`.
## Premise, measured on `440bed63e7` before the fix
- **Showcase object as shipped.** `engine.validate('showcase_cascade', {
province: 'zj' }, { mode: 'update' })` refuses `province` with
`rule_violation` / `unevaluable`. That is the option gate since #22402.
The real by-id update `{ province: 'zj' }` over a stored `country: 'cn'`
row is admitted.
- **The card's `note` call.** The shipped `showcase_cascade` declares no
`note` field, at `440bed63e7` and at the card's `3054516ef1` alike. So
`{ note: 'x' }` throws `INVALID_FIELD` on the shipped object. The card's
`note` measurement needs a fixture.
- **With a fixture `note` whose `requiredWhen` reads `country`.** Both
card calls refuse `note` unevaluable, and `{ province: 'zj' }` also
refuses `province`. The message was "The predicate reads 'country',
which this object does not declare". Passing `id` changed nothing,
because the preview read no row. The by-id update admits.
## Pins, round 0
(`packages/objectql/src/validate-update-stored-row.test.ts`, 15 cases
then, 18 now;
`packages/core/src/utils/import-runner-update-preview-address.test.ts`,
3 cases)
- **(a)** The card's two calls, `{ province: 'zj' }` and `{ note: 'x'
}`, are admitted over a stored `country: 'cn'` row, and the by-id update
admits the same patches. Also, a formula column that the read door
evaluates is judged the way the write judges it: preview and write
agree.
- **(b) Control.** `{ country: 'us' }` gets `note` `required`, and `{
province: 'ca' }` gets `province` `invalid_option`. Preview and write
give the same findings, and the store is unchanged.
- **(c)** With no address, or with an id naming no row, the
`requiredWhen` and option refusals say "its value was not supplied" and
never "which this object does not declare". A `validations[]` script
rule and a conditional rule do the same. Control: an undeclared key
(`contry`) keeps "does not declare".
- **(d)** The read scope comes from a middleware shaped like the
RLS/sharing ones. The owner's preview judges the stored row. A caller
who cannot read the row gets a verdict deep-equal to the one a
nonexistent id gets, whether the hidden row holds `cn` or `us`. A column
masked from the caller is judged as empty, with the same verdict
whichever value is stored, and the control caller who can read it gets
the stored row's verdict.
- **(e)** The real door is `runImport` through
`ObjectStackProtocolImplementation` over a kernel with `ObjectQLPlugin`.
The update-mode dry run of a matched row admits `province` and `note`
and agrees with the committed import. Control: `country: 'us'` fails
`note` `required` in both. The core test pins that the dry run carries
the matched id, that the matched id wins over an id cell (as the write's
fold makes it win), and that a create row gets no id.
### Round 0: red before the fix, and ablations (each from a committed
state)
- **Reverse verification.** The four fix files were restored to
`440bed63e7` and core was rebuilt; the dist marker count fell from 1 to
0. The pin file then went **10 failed / 4 passed**. Red: (a) ×2, (b)
`province: 'ca'`, (c) ×3, (d) owner and masked-column, (e) ×2. Green, as
expected: (b) `country: 'us'`, the (c) undeclared control, (d)
hidden-row equality, and the (e) control. Restore leg: blobs equal HEAD,
`git diff HEAD` empty, core rebuilt, dist marker back to 1, **14/14
green**. The core address test against the base runner: **2 failed / 1
passed** (the control passed).
- **Ablations through `scripts/ablation-replace.mjs`.** Each anchor hit
once, and each restore was proven by blob == HEAD with `git diff HEAD`
empty.
- Image = read door's row: 1 failed (the formula pin).
- Column filter removed: 1 failed (the masked column).
- Visibility gate bypassed: 2 failed (the hidden row and the masked
column).
- The first attempt at the image ablation was a refused no-op. Its
replacement contained the anchor, so the tool refused, restored, and ran
nothing. It was rerun with a non-overlapping replacement.
## Tests, round 0 (superseded by the patch round 1 section above)
All runs are on head `31ca6a891a` unless a line says otherwise. Builds
and tests went through `scripts/pm/os-verify-lock.sh`.
- **Package suites.** These ran on `45c2877b8f`. The commits after it
change only the changeset and the two pin files, and the pin files were
re-run on `31ca6a891a`.
- `pnpm --filter @objectstack/objectql test`: **391 files, 7713
passed**.
- `pnpm --filter @objectstack/objectql run test:repo`: 1 file, 5 passed.
- `pnpm --filter @objectstack/core test` (run on `d6fb574835`): 84
files, 2235 passed. `test:repo`: 3 files, 48 passed.
- `pnpm --filter @objectstack/spec test`: 629 files, 18779 passed, 1
todo.
- **Pin files.** `validate-update-stored-row.test.ts` **15 passed**.
`import-runner-update-preview-address.test.ts` **3 passed**.
- **Typecheck.** `pnpm --filter @objectstack/objectql run typecheck` and
`pnpm --filter @objectstack/core run typecheck` both exit 0. Each
includes `check:test-typecheck` OK, with the new test files compiled
under each package's `tsconfig.test.json`.
- **Downstream consumer (the public import door through dist).** I built
`@objectstack/rest^...`, then ran the nine
`packages/rest/src/import-*.test.ts` files (dry-run parity, integration,
job, dropped-fields, row report, schema drift, run-automations
agreement, historical readonly insert, unique violation): **9 files, 113
passed**. The objectql and core dists were confirmed to carry the change
(`readUpdatePriorRow`: 4 hits; the `...data, id: existing.id` fold: 1
hit).
- **Spec.** `pnpm --filter @objectstack/spec build`, then
`check:generated --fix`. That regenerated only
`content/docs/references/api/protocol.mdx`. A re-run reports all 15
artifacts up to date.
- **Gates.** `node scripts/pm/dispatch-gates.mjs --repo
objectstack-ai/objectstack --commands` derived **117** commands at
`31ca6a891a`. All 117 ran with exit codes captured before any pipe, and
all **exit 0**. Reconciliation with `--ran`: `117 derived, 117 run, 0
NOT-MEASURED, 0 UNRUN`. An earlier run of the same battery at
`f4fc729b08` found two real reds, both in my test file, and fixed them:
- `check:error-code-casing`: a one-line `code: 'rule_violation'` with no
`field`. The assertion now names `field: '_record'`.
- `check:query-options-erasure`: an `as any` on a `findOne` options bag.
The cast was removed.
- The same run had two prerequisite exit-3 results:
`check:skill-examples` needed `client-react` built, and
`check:dual-build-cjs-loads` needed more dists. Both measured green on
the final run.
- Also run: `pnpm check:adr-anchors` exit 0, and `node
scripts/check-nul-bytes.mjs` exit 0.
- **Lint, narrowed (a measurement, not a skip).** `pnpm exec eslint
--no-inline-config --format json` over the 7 changed `.ts` files
returned 7 results, 0 errors and 0 warnings. Those files are inside
`eslint.config.mjs`'s `packages/**/*.{ts,tsx,mts,cts}` and
`**/*.{ts,...}` populations. The config enables no type-aware linting
(no `parserOptions.project` / `projectService` anywhere), so this diff
cannot move a verdict on any file it does not touch. The repo-wide `pnpm
lint` is CI's.
## Acceptance notes
- **Preview/write drift, same family, measured on this branch (not fixed
here).** `validate()` binds no master-detail `parent` header, in either
mode. On a detail object whose field carries `requiredWhen:
"parent.status == 'sent'"`, the preview refuses with `rule_violation` /
`unevaluable` (unbound `parent`), both for an insert under a draft
header and for an update of a stored line. The real insert and the real
by-id update admit both. The import dry run reaches this through
`previewVerdict` -> `validateData`. Reported for the seat to file; not
filed here.
- **Named limit kept, as dispatched.** The update preview still runs no
`readonlyWhen` or primary-key strip. It now holds the stored row those
strips would judge, so closing this limit is possible, but it is out of
this card's scope. The `ValidateDataResponse` note keeps stating it.
- **Observed, not filed.** A `requiredWhen` that reads a `formula`
column (`record.doubled > 100`) faults with a null overload on every
by-id update. The write's prior row is the stored row, which holds no
formula value. The preview now agrees with the write here, and a pin
holds that agreement. Whether `os validate` refuses such a predicate was
not measured.
- **Observed, not filed.** `ValidateDataIssueSchema` declares `{ field,
code, message }`. The engine's findings, which `validateData` relays,
also carry `constraint`, and an option refusal carries `value`. The pin
file reads `constraint` through a typed accessor for that reason.
- **New read surface.** The update preview's visibility read goes
through `findOne`, so it fires the object's `beforeFind` / `afterFind`
hooks once per addressed row. The related-row read already did the same
through `find`. No write hook runs.
- **The card's `note` call.** It needed a fixture field: the shipped
`showcase_cascade` declares no `note`. The shipped object's own
`province` pick reproduces the defect.
- **Base (round 0).** `origin/main` was merged once (`b7c02e713f`).
Patch round 1 merged it again; see that section.
- **Cross-lane.** `packages/spec/src/api/protocol.zod.ts` is edited:
description text and a JSDoc note only, no key or type. The changeset
bumps `@objectstack/spec` at minor along with `@objectstack/objectql`
and `@objectstack/core`.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Bw3y2DWhT9RPnrmDsNqEVG)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6abd265 commit 35ef501
10 files changed
Lines changed: 962 additions & 43 deletions
File tree
- .changeset
- content/docs/references/api
- packages
- core/src/utils
- objectql/src
- validation
- rest/src
- spec/src/api
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2985 | 2985 | | |
2986 | 2986 | | |
2987 | 2987 | | |
2988 | | - | |
| 2988 | + | |
2989 | 2989 | | |
2990 | 2990 | | |
2991 | 2991 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1022 | 1022 | | |
1023 | 1023 | | |
1024 | 1024 | | |
1025 | | - | |
| 1025 | + | |
| 1026 | + | |
| 1027 | + | |
| 1028 | + | |
| 1029 | + | |
| 1030 | + | |
| 1031 | + | |
| 1032 | + | |
| 1033 | + | |
| 1034 | + | |
| 1035 | + | |
| 1036 | + | |
1026 | 1037 | | |
1027 | 1038 | | |
1028 | 1039 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
28 | 31 | | |
29 | 32 | | |
30 | 33 | | |
| |||
106 | 109 | | |
107 | 110 | | |
108 | 111 | | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
109 | 124 | | |
110 | 125 | | |
111 | 126 | | |
| |||
134 | 149 | | |
135 | 150 | | |
136 | 151 | | |
137 | | - | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
138 | 157 | | |
139 | 158 | | |
140 | 159 | | |
| |||
0 commit comments