Repository navigation
Commit 0c6b7b8
fix(runtime): a non-admin approver's decision from the request page reaches the approvals routes instead of answering 404 (#28708)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XC2DvEE5kBwmAEa8vy2MpWm)_
Fixes #28657
Clause-②: yes (widening: on `POST /api/v1/actions/:object/:action` and
MCP `run_action`, a same-origin `type: 'api'` action whose subject row
the caller's own scope does not deliver, and whose request needs no
field of that row, is forwarded and answers its target's answer where it
answered `404 RECORD_NOT_FOUND`)
Metadata-protocol: none
## 维护者速读
- **改了什么**:动作门(`/api/v1/actions/...` 与 MCP `run_action`)在调用者读不到主记录时,不再先回
404;对不需要该记录字段的同源 `type:'api'` 动作照常转发,由目标接口自己鉴权并给出答复。
- **为什么改**:审批详情页的同意、驳回、评论、补充信息等按钮都是这种动作。普通成员按设计没有 `sys_approval_request`
的数据读权限,所以非管理员审批人在详情页做任何决定都被 404 挡住,18.3.0 发不了。
- **风险与回滚**:低。只动 api
转发这一支;目标接口用调用者自己的凭证鉴权,等同于调用者直接调它,不多泄露任何东西。需要记录字段才能拼请求的 api
动作、flow、脚本、声明式更新照旧回 404。回滚即还原 `refuseDeniedSubjectLoadBeforeDispatch`
两处调用。
- **先看**:`packages/runtime/src/action-execution.ts`(新的共享判定
`refuseDeniedSubjectLoadBeforeDispatch` /
`apiActionRequestReadsSubjectRow` /
`apiForwardSubjectRecord`);`packages/runtime/src/domains/actions.ts`(REST
门那一行);`packages/verify/src/action-door-api-forward-denied-subject.test.ts`(真实审批栈上的
HTTP 钉子)。
- **两分钟验证**:`pnpm --filter @objectstack/verify exec vitest run
src/action-door-api-forward-denied-subject.test.ts`,预期 3/3 通过(main 上 2
个失败,都是 404 `RECORD_NOT_FOUND`)。
- **需要你做的**:无,席位落地。
## What changes
- `packages/runtime/src/action-execution.ts`
- `refuseDeniedSubjectLoadBeforeDispatch(action, objectName, recordId,
subject)`: contract point 3 as both doors' dispatch reads it. It returns
without refusing exactly when the load was denied and
`forwardsOnDeniedSubjectLoad(action)` holds; otherwise it is
`refuseDeniedSubjectLoad`, unchanged.
- `forwardsOnDeniedSubjectLoad(action)`: not a declarative update
(`operation` before `type`), `type: 'api'`, a same-origin `target`, and
`apiActionRequestReadsSubjectRow(action)` false.
- `apiActionRequestReadsSubjectRow(action)`: true for a `{field}` path
token in `target` other than `{id}`, or a `recordIdParam` read from a
`recordIdField` other than `id`. Read off the declaration before
substitution, so a token that only looks like a field errs toward the
404.
- `apiForwardSubjectRecord(recordId, subject)`: the row the forward's
request is built from. A delivered row as it was; on a denied load `{
id: recordId }` and nothing else.
- `dispatchAdmittedBusinessAction` (MCP `run_action`) calls the two at
the same point the REST door does.
- `apiActionForwardRefusal` reads a target's bare string `error` (the
approvals routes' `{ code, error }`) as the refusal's message, before
the "its target answered HTTP N" fallback. This is the card's 400
prescription relay: it was this code path, so it rides along (ruling 6).
- `packages/runtime/src/domains/actions.ts`:
`dispatchAdmittedRestAction` calls
`refuseDeniedSubjectLoadBeforeDispatch` where it called
`refuseDeniedSubjectLoad`, and builds the api request from
`apiForwardSubjectRecord`.
- `.changeset/28657-api-action-forward-denied-subject.md`:
`@objectstack/runtime` patch (`files[]` ships `dist`, which carries the
change).
How ruling 2 is enforced: the forward never sees the load's leftovers.
`buildApiActionRequest` is handed `apiForwardSubjectRecord(...)`, which
on a denied load is built from the routed `recordId` alone. A
declaration that needs a row field is caught one step earlier by
`apiActionRequestReadsSubjectRow`, so it is refused with today's 404
before the loop check, the injector or the builder run. Pinned by the
helper case (a denied load whose leftover row carries `email` yields `{
id }` only) and by the door cases for `{email}` in the path and
`recordIdField: 'email'` (404, no forward, on both doors).
Untouched, as ruled: `pending-approver-read-grant.ts`, the member grant
on `sys_approval_request`, `packages/spec/**`, `ui/**`, the flow,
script/body and declarative-update branches, and `resume_run`.
## Tests
<details><summary>The HTTP-layer pin on a real stack: red on base
e4ec4f2, green on head</summary>
`packages/verify/src/action-door-api-forward-denied-subject.test.ts`
boots the real stack with `bootStack` (`requires: ['automation',
'triggers', 'approvals']`, the providers `serve` mounts for those
tokens): the runtime dispatcher, the real approvals service and its
decision routes, the real in-process forward. No fetch is doubled. A
private object, a two-approver `unanimous` flow; the admin creates the
record, and plain members act.
1. Control: the approver cannot read the request row through the data
door (403 `PERMISSION_DENIED`).
2. The approver's `POST
/api/v1/actions/sys_approval_request/approval_approve`
`{"recordId":AREQ,"params":{"comment":…}}` answers 200;
`pending_approvers` goes from both approvers to the other one; one
`approve` row with the approver as `actor_id` and the comment; `GET
/api/v1/approvals/requests/AREQ` reads `decision_progress` `{ behavior:
'unanimous', got: 1, need: 2 }`.
3. A member on no slate, through the same door, gets exactly the status
and code the decision route answers them directly (403 `FORBIDDEN`), the
route's own sentence as the message, and nothing moves.
Base (`packages/runtime/src/action-execution.ts` and
`domains/actions.ts` written from `e4ec4f22c5`, runtime rebuilt,
`ablation-dist-preflight --absent` on the new helper's name):
```
× Approve answers 200 and the request moves: 1 of 2, the approver off the slate, an approve row
× a member on no slate gets the decision route's own refusal through the door — never a 200, never the row
AssertionError: {"success":false,"error":{"code":"RECORD_NOT_FOUND","message":"Record areq_… not found in sys_approval_request","httpStatus":404}}: expected 404 to be 200
AssertionError: {"success":false,"error":{"code":"RECORD_NOT_FOUND",…}}: expected 404 to be 403
Tests 2 failed | 1 passed (3)
```
Head: `Test Files 1 passed (1) · Tests 3 passed (3) · VERDICT
command-exit 0`. Restore proven by blob hashes equal to HEAD, `git
status --porcelain` empty, rebuild, and the preflight finding the marker
in `dist/index.js` and `dist/index.cjs`.
</details>
<details><summary>Ablation: only the api-branch move reverted, the HTTP
pin goes red</summary>
`node scripts/ablation-replace.mjs --file
packages/runtime/src/domains/actions.ts --anchor
"actionExec.refuseDeniedSubjectLoadBeforeDispatch(actionDef, objectName,
recordId, subject);" --replacement
"actionExec.refuseDeniedSubjectLoad(objectName, recordId ??
'ablation-28657', subject);" -- bash ablate-inner.sh`
```
ablation-replace: ok mutation landed: anchor 1 -> 0, blob a762e11 -> 3d61252a815c
build exit 0
✓ dist/: marker present in 2 built files -- the ablation is live in the artifact the suite consumes.
× Approve answers 200 and the request moves: 1 of 2, the approver off the slate, an approve row
× a member on no slate gets the decision route's own refusal through the door — never a 200, never the row
Tests 2 failed | 1 passed (3)
ablation-replace: ok restored: blob == HEAD (a762e11) and `git diff HEAD` is empty
tree after restore (want empty): []
✓ dist/: marker absent from all 12 built files
verify pin (restored): Tests 3 passed (3)
```
</details>
<details><summary>Runtime pins (doors over fakes): ruling 2, parity, the
400 relay</summary>
New `describe('[#28657] …')` in
`packages/runtime/src/domains/api-action-forward.test.ts`, 10 cases:
- REST: the approve shape on a row the caller cannot read is forwarded
to `/api/v1/approvals/requests/areq_hidden/approve` with the caller's
bearer, and the target's 200 is the answer.
- REST: the target's 403 `{ code: 'FORBIDDEN', error }` is the answer,
its sentence as the message.
- REST: a `recordIdParam` read from `id` is seeded from the routed id
alone.
- REST, ruling 2: `{email}` in the path and `recordIdField: 'email'` on
a denied load answer 404 `RECORD_NOT_FOUND` and nothing is forwarded;
the firing control on a readable row forwards with the row's field.
- REST, ruling 6: a 400 `{ code: 'VALIDATION_FAILED', error }` is
relayed with its sentence, not "its target answered HTTP 400".
- MCP `run_action`: the same approve shape on a denied load is forwarded
and answers `ok: true`; the two row-field shapes are thrown 404
`RECORD_NOT_FOUND` with nothing forwarded; the `{ code, error }`
sentence rides the throw.
- The shared helpers, case by case.
On base (`-t 28657`): 7 failed, 3 passed. The 3 that pass on base are
the ruling-2 404 cases and their firing control, which pin behaviour
that must not change. On head: `Tests 68 passed (68)`.
Ruling 3 (every non-api branch keeps the 404) stays pinned by the
existing cases, all green on head:
`action-door-record-load-denied.test.ts` "[#16370] flow door — a denied
caller-scope load is refused before the run exists" (REST and MCP),
"[#16370] script/body door — the same verdict, the same refusal" (REST
and MCP), and `http-dispatcher.actions-type-dispatch.test.ts` "[#16370]
a ROW-SCOPED flow whose record never loaded is REFUSED — no run is
created". The declarative update is excluded by
`forwardsOnDeniedSubjectLoad` (pinned in the helper case) and runs ahead
of the call in both doors.
MCP parity with `approval_approve` itself is not reachable: the
`sys_approval_request` actions declare no `ai.exposed`, so `run_action`
refuses them at the system-object exposure gate before the subject load,
on base and on head alike. Parity is pinned on the same declaration
shape on an exposed object.
</details>
<details><summary>Live probe: the card's reproduction on a stock
showcase, step 7</summary>
Boot (no `--ui`: the probe is HTTP only): `OS_PORT=PORT pnpm -C
examples/app-showcase exec objectstack dev --seed-admin --no-compile -p
PORT -d file:DIR/data.db`, after `pnpm exec turbo run build
--filter='!@objectstack/docs'` (117 tasks). Then, with curl and an
`Origin` header: admin `POST /api/v1/auth/sign-in/email`; `POST
/api/v1/auth/organization/invite-member` `{"email":X,"role":"member"}`;
X `POST /api/v1/auth/sign-up/email`; admin `POST
/api/v1/data/sys_user_position` for `contributor` and `finance`; X `POST
/api/v1/data/showcase_invoice`
`{"name":…,"account":ACTIVE_ACCOUNT_ID,"owner":X_EMAIL}` and `PATCH
…/ID` `{"status":"sent","issued_on":"2026-11-01"}`; then step 7.
Base (runtime rebuilt from `e4ec4f22c5`, fresh database):
```
before: {"status":"pending","decision_progress":{"behavior":"unanimous","got":0,"need":2},"can_act":true}
STEP 7 action door approve: 404 {"success":false,"error":{"code":"RECORD_NOT_FOUND","message":"Record areq_dab05fb4-… not found in sys_approval_request","httpStatus":404}}
after: {"status":"pending","decision_progress":{"behavior":"unanimous","got":0,"need":2},"can_act":true}
actions: [{"action":"submit",…}]
```
Head (fresh database):
```
before: {"status":"pending","decision_progress":{"behavior":"unanimous","got":0,"need":2},"can_act":true}
STEP 7 action door approve: 200 {"success":true,"data":{"request":{"id":"areq_70509601-…","status":"pending",…
after: {"status":"pending","decision_progress":{"behavior":"unanimous","got":1,"need":2},"pending":["(the other approver)"],"can_act":false}
actions: [{"action":"submit",…},{"action":"approve","actor_id":"(X)"}]
```
X leaves `pending_approvers`, an `approve` row exists, and X's `can_act`
is false, which is the card's step 6.
</details>
<details><summary>Packages, gates, lint</summary>
- `pnpm --filter @objectstack/runtime test`: `Test Files 504 passed
(504) · Tests 8196 passed | 19 skipped (8215)`.
- `pnpm --filter @objectstack/runtime typecheck` and `pnpm --filter
@objectstack/verify typecheck` (each includes `check:test-typecheck`, so
the new test files are type-checked): exit 0.
- `eslint --no-inline-config packages/runtime packages/verify`: exit 0.
- `node scripts/pm/dispatch-gates.mjs --commands` on the final head
`f3768a46`: 61 commands, every one exit 0 (`--ran` reconciliation: "61
derived famil(ies) accounted for — 61 run, 0 NOT-MEASURED"). The list
with each exit code is in the card's report.
</details>
## Acceptance notes
- The approval actions are not `ai.exposed`, so an agent cannot run them
through `run_action` before or after this change. Exposing them is a
plugin-approvals decision outside this card.
- The approvals routes answer `{ code, error }` rather than the ADR-0112
envelope. This PR only reads that sentence at the forward; the routes'
own shape is unchanged.
## Verification
- Real-stack HTTP pin (PR CI runs it in Test Core: `@objectstack/verify`
is downstream of runtime): `pnpm --filter @objectstack/verify exec
vitest run src/action-door-api-forward-denied-subject.test.ts`.
- Live: the probe above, on the showcase.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5mze6z6hhTUauqWMmZ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent c01690e commit 0c6b7b8
5 files changed
Lines changed: 540 additions & 5 deletions
File tree
- .changeset
- packages
- runtime/src
- domains
- verify/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 | + | |
| 15 | + | |
| 16 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1980 | 1980 | | |
1981 | 1981 | | |
1982 | 1982 | | |
| 1983 | + | |
| 1984 | + | |
| 1985 | + | |
| 1986 | + | |
| 1987 | + | |
| 1988 | + | |
1983 | 1989 | | |
1984 | 1990 | | |
| 1991 | + | |
1985 | 1992 | | |
1986 | 1993 | | |
1987 | 1994 | | |
| |||
3356 | 3363 | | |
3357 | 3364 | | |
3358 | 3365 | | |
| 3366 | + | |
| 3367 | + | |
| 3368 | + | |
| 3369 | + | |
| 3370 | + | |
| 3371 | + | |
| 3372 | + | |
| 3373 | + | |
| 3374 | + | |
| 3375 | + | |
| 3376 | + | |
| 3377 | + | |
| 3378 | + | |
| 3379 | + | |
| 3380 | + | |
| 3381 | + | |
| 3382 | + | |
| 3383 | + | |
| 3384 | + | |
| 3385 | + | |
| 3386 | + | |
| 3387 | + | |
| 3388 | + | |
| 3389 | + | |
| 3390 | + | |
| 3391 | + | |
| 3392 | + | |
| 3393 | + | |
| 3394 | + | |
| 3395 | + | |
| 3396 | + | |
| 3397 | + | |
| 3398 | + | |
| 3399 | + | |
| 3400 | + | |
| 3401 | + | |
| 3402 | + | |
| 3403 | + | |
| 3404 | + | |
| 3405 | + | |
| 3406 | + | |
| 3407 | + | |
| 3408 | + | |
| 3409 | + | |
| 3410 | + | |
| 3411 | + | |
| 3412 | + | |
| 3413 | + | |
| 3414 | + | |
| 3415 | + | |
| 3416 | + | |
| 3417 | + | |
| 3418 | + | |
| 3419 | + | |
| 3420 | + | |
| 3421 | + | |
| 3422 | + | |
| 3423 | + | |
| 3424 | + | |
| 3425 | + | |
| 3426 | + | |
| 3427 | + | |
| 3428 | + | |
| 3429 | + | |
| 3430 | + | |
| 3431 | + | |
| 3432 | + | |
| 3433 | + | |
| 3434 | + | |
| 3435 | + | |
| 3436 | + | |
| 3437 | + | |
| 3438 | + | |
| 3439 | + | |
| 3440 | + | |
| 3441 | + | |
| 3442 | + | |
| 3443 | + | |
| 3444 | + | |
| 3445 | + | |
| 3446 | + | |
| 3447 | + | |
| 3448 | + | |
| 3449 | + | |
| 3450 | + | |
| 3451 | + | |
| 3452 | + | |
| 3453 | + | |
| 3454 | + | |
| 3455 | + | |
| 3456 | + | |
| 3457 | + | |
| 3458 | + | |
| 3459 | + | |
| 3460 | + | |
| 3461 | + | |
3359 | 3462 | | |
3360 | 3463 | | |
3361 | 3464 | | |
| |||
3867 | 3970 | | |
3868 | 3971 | | |
3869 | 3972 | | |
3870 | | - | |
| 3973 | + | |
| 3974 | + | |
| 3975 | + | |
| 3976 | + | |
| 3977 | + | |
3871 | 3978 | | |
3872 | 3979 | | |
3873 | 3980 | | |
| |||
3889 | 3996 | | |
3890 | 3997 | | |
3891 | 3998 | | |
3892 | | - | |
| 3999 | + | |
3893 | 4000 | | |
3894 | 4001 | | |
3895 | 4002 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
879 | 879 | | |
880 | 880 | | |
881 | 881 | | |
882 | | - | |
| 882 | + | |
| 883 | + | |
| 884 | + | |
| 885 | + | |
| 886 | + | |
| 887 | + | |
| 888 | + | |
883 | 889 | | |
884 | 890 | | |
885 | 891 | | |
| |||
903 | 909 | | |
904 | 910 | | |
905 | 911 | | |
906 | | - | |
| 912 | + | |
| 913 | + | |
| 914 | + | |
| 915 | + | |
| 916 | + | |
907 | 917 | | |
908 | 918 | | |
909 | 919 | | |
| |||
0 commit comments