fix(rest): route batch creates through the create ingress so readonly is enforced (#3835) - #3844
Closed
baozhoutao wants to merge 3 commits into
Closed
fix(rest): route batch creates through the create ingress so readonly is enforced (#3835)#3844baozhoutao wants to merge 3 commits into
readonly is enforced (#3835)#3844baozhoutao wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 104 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
github-actions
Bot
added
documentation
Contributor
Author
…ropped fields (#3794) An approval flow reported record writability wrong in both directions: what the user could change said "locked", and what they couldn't said "updated successfully". Both halves were missing signal, not wrong behaviour — the server did the right thing and told nobody. `rowFromRequest` now emits `locks_record`, read from the same `node_config_json` snapshot the `beforeUpdate` lock hook reads, with the same default-true. A client could previously only see "a request is pending" and had to guess whether that locked the record; the Console guessed "locked" every time, which hides the whole point of a `lockRecord: false` node. `POST /batch` (cross-object transactional batch) never wired `onFieldsDropped`, so the one write path the Console's master-detail record form takes was also the one path with no write-observability — a `readonlyWhen` strip there was completely silent. It now collects per-op events and returns them as a top-level `droppedFields` list tagged with each operation's index; omitted when empty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…dFields (#3794) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y` is enforced (#3835) `POST /batch` called `ql.insert` directly, and the engine's INSERT path is static-`readonly`-exempt by design (#3413) — the strip that stops a non-system caller from seeding a read-only column lives at the protocol's create ingress (#3043). So the same forged value was dropped on `POST /data/:object` and written through on `/batch`: one rule, two answers. Create ops now go through `p.createData`, the ingress itself, rather than a second copy of the strip at the REST layer. One ingress means a future change to its policy covers the batch for free, and the carve-outs it already encodes stay intact — the platform-object exemption (a `sys_`/`managedBy` object's own guard must REJECT a forged value, not silently swallow it) and the `isSystem` exemption. `trxCtx` is passed as the context, so the insert still joins the batch transaction and `$ref` resolution is unaffected; the ingress's `droppedFields` fold into the batch's per-op list. Update ops are untouched — the engine enforces both strips on its update path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3835.
readonlymeant two different things depending on which create endpoint you usedPOST /data/:objectruns the #3043 ingress strip, so a non-system caller cannot seed a read-only column — the field is dropped and reported. The cross-object transactional batch calledql.insertdirectly and skipped that ingress, and the engine's INSERT path is static-readonly-exempt by design (#3413 — the strip deliberately lives one layer up). So nothing enforced it on that route.Measured on the showcase,
showcase_contact.lead_score(readonly: true), same signed-in non-system user, identical payload:POST /data/showcase_contactlead_score = null,droppedFieldsreportedPOST /batch(action: 'create')lead_score = 999written, nothing saidnull,droppedFieldsreportedWhy route to the ingress instead of re-applying the strip here
Reading
stripReadonlyForInsertturned up a deliberate scope carve-out that a second copy would have lost: platform objects (sys_/managedBy) are exempt on purpose, because their own guards must reject a forged value with 403 (ADR-0086'smanaged_by/package_id, #3004'sowner_idanchor) — silently swallowing it would eat the very payload the guard exists to refuse.isSystemis exempt for the same class of reason.That rules out the "sink the strip into the engine" option I floated in the issue: the engine has no notion of that boundary. So the batch's create ops now go through
p.createData— the ingress itself. One create ingress, and a future change to its policy covers the batch for free (AGENTS.md PD #12: fix the producer, don't grow a second de-facto contract).Mechanics that had to keep working, and do:
trxCtxis passed as the context, so the insert joins this batch's transaction and rolls back with it (asserted in a test: the ingress receives the trx context, not a bare one).{ $ref: <opIndex> }—resultsnow holds the ingress's echoed record instead of the raw engine return;$refresolves off.ideither way. Verified live: an invoice + line batch produced a line whoseinvoicepoints at the invoice created in the same request.droppedFields— the ingress's events fold into the batch's per-op list (审批场景下记录可写性的反馈全线失真:能改的显示「已锁定」,改不了的提示「更新成功」 #3794), so a batch create reports strips the way an update does.Update ops are untouched — the engine enforces
readonly(#2948) andreadonlyWhen(#3042) on its own update path.Tests
3 new in
rest-batch-endpoint.test.ts: create ops go through the ingress with the transaction context; a forgedreadonlycolumn never reaches the engine and comes back indroppedFieldstagged with its op index;$refstill resolves through the ingress. The harness's protocol mock gained acreateDatathat delegates to the sameql.insert, so existing assertions still read offql.insert.rest: 413 passing.🤖 Generated with Claude Code