Repository navigation
Commit 60dac8b
fix(runtime): a declarative update action refuses a caller with no edit grant 403 before the record lookup, as the data door does (#30878)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} -->
_Requested by **Elon** · [project
thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DEFxACyPwHxUwJy1F7vBLFD)_
domain:server#1 · os-elon-musk · session_016WV4czxVZYV4CcMiCbofw3
Fixes #30848
Clause-②: no
Metadata-protocol: none
Claim: comment 6679693194 (branch `claude/project-thread-y8qx4d`).
## 维护者速读
- 改了什么:声明式更新动作(`operation: 'update'`,REST `/actions` 与 MCP
`run_action`)在按"读不到这条记录"回 404 之前,先问调用者对该对象有没有编辑权;没有就回 403
`PERMISSION_DENIED`,与数据接口(`PATCH /data`、MCP `update_record`)对同一次写入的回答一致。
- 为什么改:只读无编辑权的用户改一条看不到的记录(或不存在的 id),动作门答 404、数据门答
403,同一次写入两个答案;文档写明"拒绝先于记录查找"、声明式更新"与用户编辑完全一致"。场景卡 #30802 因此无法写出唯一的预期拒绝。
- 风险与回滚:只在记录加载被拒时多问一次安全服务的
`explain(update)`,正常路径不变;有编辑权的人仍对"看不到的行"和"不存在的 id"回同一个 404,不形成存在性探测。单 PR
可整体回滚。
-
先看:`packages/runtime/src/action-execution.ts`(`refuseUngrantedDeclarativeUpdate`
与 `executeDeclarativeUpdateAction`
中它的调用点)、`packages/runtime/src/declarative-update-refusal-order.pin.test.ts`。
- 两分钟验证:`cd packages/runtime && pnpm exec vitest run --project local
src/declarative-update-refusal-order.pin.test.ts`,预期 20/20 通过。
- 需要你做的:无,席位落地。
## What changes
- New `refuseUngrantedDeclarativeUpdate` in `@objectstack/runtime`
(`action-execution.ts`). `executeDeclarativeUpdateAction`, the executor
both action doors call (REST `/actions` and MCP `run_action`), now calls
it before it consumes the subject-load verdict
(`refuseDeniedSubjectLoad`), and only when that load was denied.
- The ask is the security service's `explain({ object, operation:
'update' })`, the contract's "would the middleware allow this
operation?" answer, computed by the enforcement walk. The REST
import-template gate already asks `create` the same way. No row is named
in the ask, so the answer cannot depend on whether the id names a row.
Nothing is re-derived from permission sets, and nothing changes in
`plugin-security`.
- A denial, or an `explain` that throws, refuses with the data door's
own refusal: `403 PERMISSION_DENIED` with the middleware's user-facing
sentence (`permission_denied` via `renderOperationMessage`, the caller's
locale, the deployment's i18n override). With no security service, or
one without `explain`, nothing is refused here, as at the data door.
- A delivered row is unchanged: the write that follows carries the
caller's identity and the data plane answers its own 403. A caller who
may update the object keeps the not-found answer, one answer for a
hidden row and a missing id.
- Docblocks on `executeDeclarativeUpdateAction` updated to name the new
order.
### The line the card asked for
On `main` `a2f523aa`, `invokeBusinessAction` loads the subject at
`packages/runtime/src/action-execution.ts:4406` (`const subject = await
loadActionSubjectRecord(…)`), and the declarative branch refuses on that
load's verdict at `:4169` (`refuseDeniedSubjectLoad(objectName,
recordId, subject)` in `executeDeclarativeUpdateAction`) before the
write, the one place the object-level edit permission was asked, is
attempted. The REST `/actions` door follows the same sequence
(`domains/actions.ts`, its own load, then the same executor).
### Other subject-loading paths (checked, not changed)
- There is no declarative delete: `ActionSchema.operation` is
`z.enum(['update'])`.
- The other `refuseDeniedSubjectLoad` callers are read admissions, not
writes performed as the caller: the flow and script/body dispatch
(`refuseDeniedSubjectLoadBeforeDispatch`), the api forward that needs
the row, and the flow start record keys
(`refuseUnreadableFlowStartRecords`). No data door performs the same
write for the same caller, so there is no second answer to disagree
with. Their 404 is the read-admission answer by design (#15079, #16370).
They do not share this bug, and no follow-up is listed.
## Why
`content/docs/protocol/kernel/http-protocol.mdx`, "Refusals come before
the record lookup": a caller who may not update the object gets the same
403 whether or not the record id names a record.
`content/docs/protocol/objectui/actions.mdx`: a declarative update fires
with the caller's permissions "exactly as for a user edit". Both
sentences were false for the action door. The finding came from #30802 /
PR #30846 (`travel_expense` op 4), which needs one expected refusal for
this operation.
## Tests
`packages/runtime/src/declarative-update-refusal-order.pin.test.ts`
boots one kernel (the plugin set
`unresolved-object-name.actions-door.pin.test.ts` uses: ObjectQL,
sqlite-wasm, Hono, the app, platform objects, auth, security, sharing,
REST, dispatcher). Object `pin_step` is private OWD with a declarative
`decide` action. Two invited members: a reader (default set: read +
create) and an editor (an assigned set: read + create + edit). Each
member owns one row.
- Matrix: {reader, editor} × {own row, the other member's row, a missing
id} × four doors: `PATCH /data`, the MCP bridge's `update` (what
`update_record` calls), `POST /actions/pin_step/decide/:id`, and the MCP
bridge's `runAction` (what `run_action` calls). The MCP leg is driven
through the real `/mcp` route with a stand-in `mcp` service that calls
the two bridge members and reports the thrown `status` and `code`. The
tools' own envelope rendering is `@objectstack/mcp`'s and is not under
test here. Expected: the reader gets 403 `PERMISSION_DENIED` on all
three rows at every door; the editor gets 200 on its own row and 404
`RECORD_NOT_FOUND` on the other two at every door. A refused write
leaves the row untouched.
- Existence-oracle equality: within each door and for each caller, the
hidden row and the missing id get the same answer, body included, once
the id is written out (8 cases). For the reader, the visible row's
refusal at each door is that same answer too, sentence included (4
cases). So the action door's 403 carries the data plane's sentence.
- Preconditions: each member reads its own row and gets 404 on the
other's; the administrator reads both.
<details><summary>Baseline on main's action door (the
re-measure)</summary>
The pin run against `origin/main`'s `action-execution.ts` (test file
kept): 2 failed / 14 passed of the first 16 cases. Both failures were
reader × hidden row and reader × missing id, at exactly two doors:
```
"action MCP": expected { code: PERMISSION_DENIED, status: 403 } received { code: RECORD_NOT_FOUND, status: 404 }
"action REST": expected { code: PERMISSION_DENIED, status: 403 } received { code: RECORD_NOT_FOUND, status: 404 }
"data MCP": { code: PERMISSION_DENIED, status: 403 } (as expected)
```
Every other cell, including editor × hidden/missing (404 at all doors)
and both visible rows, already agreed. This is the card's table (filer's
probe on `c17719dd`) on current main. The filer's probe script needs PR
#30846's scenario harness worktree, so the re-measure went through this
booted pin instead.
</details>
<details><summary>Ablation (restored: blob equals HEAD, git diff HEAD
empty)</summary>
- Mutation via `scripts/ablation-replace.mjs`: the verdict is consumed
before the ask (`refuseDeniedSubjectLoad(…); await
refuseUngrantedDeclarativeUpdate(…)`), i.e. the old order. The anchor
went 1 → 0, and the blob went `97cc3fe3` → `7e1ef3a8`. The suite imports
runtime `src/` directly, so no dist rebuild is needed.
- Result: 4 failed / 16 passed. Reader × hidden and reader × missing are
red in the matrix, and the reader's visible-vs-missing equality is red
at `action REST` and `action MCP`. Every editor row and every data-door
row stays green.
- Restore: blob after restore `97cc3fe3` equals HEAD, and `git diff
HEAD` is empty.
- The first ablation attempt was a no-op and is not counted: its
replacement still contained the anchor, so `ablation-replace` refused
before running anything (anchor 1 → 1). The second attempt is the
reading above.
</details>
## Acceptance notes
- The ask is object-level `update` with no payload, so two pre-lookup
refusals the data door gives a caller who holds the update grant are not
asked here: a field-level write refusal on a field the payload names
(middleware step 2.5), and a read-only external datasource's write
guard. For such a caller writing a hidden row or a missing id, the
action door still answers 404 where the data door answers 403. Both
answers stay the same for the hidden row and the missing id, so there is
no oracle. This is not measured here, and the pinned matrix does not
reach it.
- A caller whose read of the object is refused but who holds edit: the
data door answers the read refusal. The action door asks only `update`,
so it keeps 404 for that caller. This is not measured here, and it needs
a permission set granting edit without read.
- `explain` denies a caller whose permission sets resolve empty. Since
ADR-0056 D2 the middleware's CRUD gate does too, so this matches.
## Verification
All on head `2dc4bdfa`, except where a line names `ad748b98`. That
earlier head differs only in docblocks and the changeset prose, plus one
line removed from the new function: the `isSystem` short-circuit,
dropped so that `explain` answers a system context as the middleware
does.
- `pnpm --filter @objectstack/runtime typecheck`: exit 0 (2dc4bdf).
- Focused: the new pin plus `action-declarative-update.test.ts`,
`action-invocation-audit.test.ts` and
`action-declarative-update-stale-summary.test.ts`: 4 files, 90 tests
passed (2dc4bdf).
- `@objectstack/runtime` local project, whole suite: 550 files, 8801
passed, 19 skipped (ad748b9).
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 60 families, all run on 2dc4bdf with exit
0; `--ran` reconciliation: 60 derived, 60 run, 0 NOT-MEASURED. The first
pass on ad748b9 had one red, `check:system-context-census` (a new
`isSystem` read in a symbol no census row names), cleared by dropping
that line. It also had one exit 3, `check:dual-build-cjs-loads`
(PREREQUISITE NOT MET, no whole-tree dist), which exits 0 on 2dc4bdf.
- `pnpm lint`: exit 0 (2dc4bdf).
- Changeset: `@objectstack/runtime` patch, carrying both declaration
lines.
- Head `5f17e7c8` (one commit on `2dc4bdfa`, test-only): the merge
queue's Dogfood Regression Gate (3/3) caught
`showcase-anonymous-deny-surfaces.dogfood.test.ts` pinning the old 404
for the no-edit member (run 42192137004). The case now asserts this PR's
403 `PERMISSION_DENIED` for the member, keeps #7867's ghost-id 404 with
the dev administrator who holds the edit grant, and compares the action
with the PATCH for both callers. Dogfood, full suite unsharded on
`5f17e7c8`: 423 files passed and 1 failed, 3537 tests passed and 2
failed. Both failures were `catalog-activation-door.dogfood.test.ts`
:149 and :167 timing out at 5000 ms under box load; that file passed
10/10 alone on the same head, and the fixed file passed 80/80.
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
(not touched)
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes.
- [x] 4. One issue, one branch off an up-to-date `main`, one pull
request.
- [x] 5. A change that publishes carries a changeset, and both
declaration lines are filled in here and in the changeset.
- [x] 6. `packages/spec` touched: not touched.
- [x] 7. No hand edits to `content/docs/references/`,
`packages/*/CHANGELOG.md` or `content/docs/releases/`.
- [x] 8. Title, body and comments are in English.
- [x] The *Before Submitting* list in CONTRIBUTING.md is done.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_016WV4czxVZYV4CcMiCbofw3
<!-- signed: domain:server#1 · os-elon-musk ·
session_016WV4cytLmJxfAmgPVjUbbW -->
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 98f640d commit 60dac8b
4 files changed
Lines changed: 519 additions & 17 deletions
File tree
- .changeset
- packages
- qa/dogfood/test
- runtime/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
581 | 581 | | |
582 | 582 | | |
583 | 583 | | |
584 | | - | |
585 | | - | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
| 588 | + | |
| 589 | + | |
| 590 | + | |
| 591 | + | |
| 592 | + | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
586 | 596 | | |
587 | 597 | | |
588 | 598 | | |
| |||
596 | 606 | | |
597 | 607 | | |
598 | 608 | | |
599 | | - | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
600 | 623 | | |
601 | 624 | | |
602 | 625 | | |
603 | 626 | | |
604 | 627 | | |
605 | | - | |
606 | | - | |
607 | | - | |
608 | | - | |
609 | | - | |
610 | | - | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
611 | 634 | | |
612 | | - | |
613 | 635 | | |
614 | | - | |
615 | | - | |
616 | 636 | | |
617 | 637 | | |
618 | 638 | | |
619 | 639 | | |
620 | | - | |
621 | | - | |
622 | | - | |
623 | | - | |
624 | | - | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
625 | 655 | | |
626 | 656 | | |
627 | 657 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
| 27 | + | |
27 | 28 | | |
28 | 29 | | |
29 | 30 | | |
| |||
3709 | 3710 | | |
3710 | 3711 | | |
3711 | 3712 | | |
| 3713 | + | |
| 3714 | + | |
| 3715 | + | |
| 3716 | + | |
| 3717 | + | |
| 3718 | + | |
| 3719 | + | |
| 3720 | + | |
| 3721 | + | |
| 3722 | + | |
| 3723 | + | |
| 3724 | + | |
| 3725 | + | |
| 3726 | + | |
| 3727 | + | |
| 3728 | + | |
| 3729 | + | |
| 3730 | + | |
| 3731 | + | |
| 3732 | + | |
| 3733 | + | |
| 3734 | + | |
| 3735 | + | |
| 3736 | + | |
| 3737 | + | |
| 3738 | + | |
| 3739 | + | |
| 3740 | + | |
| 3741 | + | |
| 3742 | + | |
| 3743 | + | |
| 3744 | + | |
| 3745 | + | |
| 3746 | + | |
| 3747 | + | |
| 3748 | + | |
| 3749 | + | |
| 3750 | + | |
| 3751 | + | |
| 3752 | + | |
| 3753 | + | |
| 3754 | + | |
| 3755 | + | |
| 3756 | + | |
| 3757 | + | |
| 3758 | + | |
| 3759 | + | |
| 3760 | + | |
| 3761 | + | |
| 3762 | + | |
| 3763 | + | |
| 3764 | + | |
| 3765 | + | |
| 3766 | + | |
| 3767 | + | |
| 3768 | + | |
| 3769 | + | |
| 3770 | + | |
| 3771 | + | |
| 3772 | + | |
| 3773 | + | |
| 3774 | + | |
| 3775 | + | |
| 3776 | + | |
| 3777 | + | |
| 3778 | + | |
| 3779 | + | |
| 3780 | + | |
| 3781 | + | |
| 3782 | + | |
| 3783 | + | |
| 3784 | + | |
| 3785 | + | |
| 3786 | + | |
| 3787 | + | |
| 3788 | + | |
| 3789 | + | |
| 3790 | + | |
| 3791 | + | |
| 3792 | + | |
| 3793 | + | |
| 3794 | + | |
3712 | 3795 | | |
3713 | 3796 | | |
3714 | 3797 | | |
| |||
4088 | 4171 | | |
4089 | 4172 | | |
4090 | 4173 | | |
| 4174 | + | |
| 4175 | + | |
| 4176 | + | |
| 4177 | + | |
| 4178 | + | |
4091 | 4179 | | |
4092 | 4180 | | |
4093 | 4181 | | |
| |||
4166 | 4254 | | |
4167 | 4255 | | |
4168 | 4256 | | |
| 4257 | + | |
| 4258 | + | |
| 4259 | + | |
| 4260 | + | |
| 4261 | + | |
| 4262 | + | |
| 4263 | + | |
| 4264 | + | |
4169 | 4265 | | |
4170 | 4266 | | |
4171 | 4267 | | |
| |||
0 commit comments