Repository navigation
Commit 045cfa4
fix(runtime): an action or job body refused for a missing capability answers the same coded 403 a hook body does, not a bare 500 (#27045)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XQ89qrdULQuk8UsPzHSD5UT)_
Fixes #26965
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:action 或 job 的沙箱 body 调用了没授予的能力(如没声明 `api.read` 却调
`ctx.api.object('x').find`)时,`POST /api/v1/actions/对象/动作` 不再回 `500
Internal server error`,而是和 hook 一样回 `403
INSUFFICIENT_PRIVILEGES`,消息点名是哪个 action/job、缺哪个能力,并提示加到「该 action/job
自己的」`body.capabilities`。
- 为什么改:#26963 之后同一道能力门对 hook 回 403、对 action 回 500,一道门两种答案;分诊裁决
A:一道门一个答案。
- 风险与回滚:能力检查本身一点没放松,被拒的调用照样不执行、不写数据;只改拒绝的作答方式。job 没有 HTTP 调用方,运行记录的
`error` 存这句话,error 日志行保留给运维。回滚即 revert 本 PR。
- 先看:`packages/runtime/src/sandbox/quickjs-runner.ts` 的
`throwCapabilityDenial`;`action-job-capability-denial-is-a-refusal.integration.test.ts`;`packages/core/src/utils/import-runner-sandbox-fault-row.test.ts`
的 §4。
- 两分钟验证:`pnpm --filter @objectstack/runtime exec vitest run
src/sandbox/action-job-capability-denial-is-a-refusal.integration.test.ts`,预期
16 项全过(5 个能力 × action / job,加对照)。
- 需要你做的:无,席位落地。
## What changes
- **`@objectstack/runtime`**, `quickjs-runner.ts`,
`throwCapabilityDenial`: the first statement `if (origin.kind !==
'hook') throwSandboxFault(vm, message);` is gone, so every origin's
denial crosses the VM as the coded refusal the hook arm already raised
(`code: 'INSUFFICIENT_PRIVILEGES'`, `status: 403`). The remedy names the
owner from the origin, in that one template: `Grant it by adding
'api.read' to the action's body.capabilities.` (or `hook's`, `job's`).
No per-origin builder; the two constants lose their `HOOK_` prefix
(`CAPABILITY_REFUSAL_CODE` / `_STATUS`, module-private). The capability
check itself is untouched.
- Docblocks that said a capability denial is the sandbox's own fault
(`SandboxError.innerMessage`, `SANDBOX_FAULT_PROP`,
`ErrorInfo.sandboxFault`, the pump loop's fault branch) now say it is a
refusal for every origin; the fault marker keeps the other host-side
faults.
- **Tests (runtime):**
- `capability-denial-is-a-fault.test.ts` is renamed
`capability-denial-is-a-refusal.test.ts` and its pin flipped: each in-VM
gate under an action origin hands the classifier `innerMessage` = the
refusal sentence (no `threw:` wrapper, no `SandboxError:` prefix),
`code: INSUFFICIENT_PRIVILEGES`, `status: 403`; one new case for a job
origin. The "denial the body catches and rethrows" and rejection-side
cases are unchanged.
- New `action-job-capability-denial-is-a-refusal.integration.test.ts`
(16): a real `ObjectQL` on `SqlDriver` (better-sqlite3), the real
`QuickJSScriptRunner`, the real `actionBodyRunnerFactory` /
`jobBodyRunnerFactory`, and the real `/actions` door
(`HttpDispatcher.handleActions`). Per token: the action answers `403
INSUFFICIENT_PRIVILEGES` naming action, capability and remedy, nothing
written, one `info` refusal line and no `error` line; the job rejects
with the same coded refusal and keeps its `error` line; controls with
the capability run.
- `nested-hook-refusal-is-a-rejection.test.ts`: the "capability denial
inside the action body is still a fault" case is flipped to the `403`
through the real door.
- `actions-fault-vs-rejection.test.ts` and three docblocks: wording only
(a timeout is still the sandbox's own fault; a capability denial is no
longer named as one).
- **Census (`@objectstack/core` `test:repo`):**
`import-runner-sandbox-fault-row.test.ts` is migrated to the new rule,
still derived from the producer and with no hand-written site list. The
`throwCapabilityDenial` call sites leave the fault population (the
`capability denial` fault kind is removed, and §1 now asserts no fault
site reads as a capability denial). The helper is read from its body: it
must not reach `throwSandboxFault`, its `new Error(…)` template is
rendered per gate, and its `code` / `status` are read from the
`Object.assign` (resolving the module constants). New §4 renders every
gate under each origin kind (`hook`, `action`, `job`) and holds the door
and the import row.
- **Dogfood:** new `action-capability-refusal.dogfood.test.ts` (2)
through the real stack's `POST /api/v1/actions/acr_note/ACTION`.
- Changeset: `@objectstack/runtime` patch.
## Why
Triage ruling A on #26965 (`6456639416`): one gate, one answer. After
#26963 a hook body's capability denial answered `403
INSUFFICIENT_PRIVILEGES` naming the hook and the capability, while an
action body's same call answered `500 INTERNAL_ERROR` "Internal server
error", telling neither the caller nor the author which action or which
capability. The #4431 argument (an action's own denial is a bug for the
operator) was set aside for hooks on #26903 and nothing in it is
specific to hooks. The operator signal stays server-side: an action's
refusal is logged once at `info` by the BodyRunner (`[BodyRunner]
sandboxed action refused`, with code and status) and the `/actions`
door's `console.error` line keeps the full wrapper; a job keeps its
`error` line.
## Acceptance notes
- **Import row, measured (census §4).** A coded 403 stays a row error:
`{ ok: false, action: 'failed', error: SENTENCE (the refusal sentence),
code: 'INSUFFICIENT_PRIVILEGES' }`. Before, the action-origin rendering
the census held read `error: 'Internal server error'`, `code:
'IMPORT_ROW_FAILED'`. The `ctx.api.object('OBJ')` gate names the
import's own object, and the door answers it `403`, not the
unknown-object heuristic's `404` (`mapDataError` reads the refusal's
code before any message limb). An import fires hooks only, so in
practice this is the hook row #26963 already produced; the census now
pins it.
- **Job.** A job reaches no HTTP caller. Its handler rejects with the
coded refusal; `DbJobAdapter` stores `err.message` as the run's `error`
(`db-job-adapter.ts`, `finishRun(runId, 'failed', …)`), which now reads
`job 'J' threw: SandboxError: capability 'X' not granted to job 'J'.
Grant it by adding 'X' to the job's body.capabilities.` The BodyRunner's
`[BodyRunner] sandboxed job threw` line stays at `error`.
- **`Clause-②: no`, measured.** No call that ran before is refused: the
denied call was refused before and is refused now (the gate is the same
code path), and every control with the capability passes. What moves is
the answer for an already-refused call: `500 INTERNAL_ERROR` becomes
`403 INSUFFICIENT_PRIVILEGES`, and the log-only `.message` gains the
`action 'X' threw:` wrapper the refusal relay uses.
- **Overlap with #26831** (open, touches `quickjs-runner.ts`): `git
merge-tree --write-tree` of this head with its head `a1dbd6ea` exits 0
with no conflict. Its catalogue resolver is hook-only and passes
host-relayed errors through, so an action or job denial does not meet
it.
- **Comments outside the claim's file surface still say a capability
denial is the sandbox's own fault** (stale since #26963 for hooks, and
now for every origin): `packages/types/src/data-error-classification.ts`
(~786–792, ~1382), `packages/core/src/utils/import-runner.ts` ~522,
`packages/rest/src/error-response.ts` ~708,
`external-datasource-routes.ts` ~450, `package-routes.ts` ~239,
`sharing-routes.ts` ~429.
`packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts`
keeps a hand-built `sandboxOwnFault("capability 'api.read' not granted
…")` row; it still passes because it pins the classifier on a
synthesised shape, which the producer no longer emits for a capability
denial. Comments and a synthetic row only, no behaviour; not touched
here (`carrier: none`).
- The console's display of the 403 is #26726's, not this card's.
## Verification
**At head `af5816664b`**, branched from `origin/main` `4bce21da` (which
carries #26963 as `e46e25c0`). Every build and test ran under
`os-verify-lock`.
- **Red first** (the new integration file, before the fix, on
`4bce21da`): 10 failed, 6 passed of 16. The 10 are exactly the five
action and five job denials; the action arm answered
`{"success":false,"error":{"code":"INTERNAL_ERROR","message":"Internal
server error","httpStatus":500}}: expected 500 to be 403`, the job arm
`expected undefined to be 'INSUFFICIENT_PRIVILEGES'`. The six controls
passed.
- **Ablation** (the fix committed first; `scripts/ablation-replace.mjs`
put the removed `if (origin.kind !== 'hook') throwSandboxFault(vm,
message);` back, runtime rebuilt, `ablation-dist-preflight` confirmed
the marker in `dist/`; restored to the HEAD blob `e31f4067`, `git diff
HEAD` empty, rebuilt, preflight `--absent` confirmed):
- runtime pins (the new integration file, the flipped
`capability-denial-is-a-refusal.test.ts`,
`nested-hook-refusal-is-a-rejection.test.ts`): 16 failed, 13 passed of
29; the 16 are every denial case.
- dogfood file: 1 failed, 1 passed (`expected 500 to be 403`, the body
`Internal server error`); restored: 2 of 2.
- core census: 1 failed of 40 (`throwCapabilityDenial reaches
throwSandboxFault: expected true to be false`).
- `pnpm --filter @objectstack/runtime test`: 466 files, 7597 passed, 19
skipped. `pnpm --filter @objectstack/runtime typecheck`: exit 0 (tsc +
`check:test-typecheck` OK).
- `vitest run --project repo` for every package with a
`vitest.repo-tests.json` except spec: core 94, objectql 5, rest 280 (18
skipped), runtime 948, types 11, all passed. Spec's repo project (about
16 min) is left to CI's Test Core.
- `pnpm --filter @objectstack/dogfood exec vitest run
test/action-capability-refusal.dogfood.test.ts`: 2 of 2, after building
the dogfood closure. Live log line from it: `INFO [BodyRunner] sandboxed
action refused
{"action":"acr_count_denied","code":"INSUFFICIENT_PRIVILEGES","status":403,...}`.
- **Dispatch gates:** `node scripts/pm/dispatch-gates.mjs --commands
--repo objectstack-ai/objectstack` derived 66 families on this diff; all
66 were run with each exit code captured before any pipe, and all 66
exited 0. Two first exited 3 (PREREQUISITE NOT MET, nothing measured):
`check-plugin-teardown-shape.mjs --self-test` on the shallow clone
(fetched its pinned fixture commit, then 48 cases pass) and
`check:dual-build-cjs-loads` before a full `pnpm build` (built, then 112
entry points across 69 packages load). `--ran` reconciliation: 66
derived, 66 run, 0 NOT-MEASURED, exit 0.
- `eslint --no-inline-config` on `packages/runtime`, the census file and
the new dogfood file: exit 0. `check-commit-card-trailers --range
origin/main..HEAD`: exit 0. `check-platform-checklist --changed-from
origin/main`: exit 0 (no anchored files).
- Dogfood: no full run owed (no `!`, no narrowing arm).
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
- [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_01FEkQ5opKoaTFmNRbYvKobt
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5opKoaTFmNRbYvKobt)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent fbc1656 commit 045cfa4
10 files changed
Lines changed: 576 additions & 167 deletions
File tree
- .changeset
- packages
- core/src/utils
- qa/dogfood/test
- runtime/src
- domains
- sandbox
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
59 | | - | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
71 | 71 | | |
72 | 72 | | |
73 | 73 | | |
| |||
105 | 105 | | |
106 | 106 | | |
107 | 107 | | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
114 | | - | |
115 | | - | |
116 | | - | |
117 | 108 | | |
118 | 109 | | |
119 | 110 | | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
120 | 117 | | |
121 | 118 | | |
122 | 119 | | |
| |||
130 | 127 | | |
131 | 128 | | |
132 | 129 | | |
133 | | - | |
| 130 | + | |
134 | 131 | | |
135 | 132 | | |
136 | 133 | | |
| |||
139 | 136 | | |
140 | 137 | | |
141 | 138 | | |
142 | | - | |
143 | 139 | | |
144 | 140 | | |
145 | 141 | | |
| |||
207 | 203 | | |
208 | 204 | | |
209 | 205 | | |
210 | | - | |
| 206 | + | |
211 | 207 | | |
212 | 208 | | |
213 | 209 | | |
| |||
246 | 242 | | |
247 | 243 | | |
248 | 244 | | |
249 | | - | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
250 | 248 | | |
251 | 249 | | |
252 | 250 | | |
253 | | - | |
254 | | - | |
255 | | - | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
256 | 259 | | |
257 | | - | |
258 | 260 | | |
259 | 261 | | |
260 | 262 | | |
| |||
265 | 267 | | |
266 | 268 | | |
267 | 269 | | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
268 | 285 | | |
269 | | - | |
270 | | - | |
271 | | - | |
272 | | - | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
273 | 289 | | |
274 | | - | |
275 | | - | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
276 | 311 | | |
277 | 312 | | |
278 | | - | |
279 | | - | |
280 | | - | |
281 | | - | |
282 | | - | |
283 | | - | |
284 | | - | |
| 313 | + | |
| 314 | + | |
285 | 315 | | |
286 | 316 | | |
287 | 317 | | |
288 | 318 | | |
289 | | - | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
290 | 349 | | |
291 | 350 | | |
292 | 351 | | |
| |||
296 | 355 | | |
297 | 356 | | |
298 | 357 | | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
299 | 363 | | |
300 | 364 | | |
301 | 365 | | |
| |||
349 | 413 | | |
350 | 414 | | |
351 | 415 | | |
352 | | - | |
353 | | - | |
| 416 | + | |
| 417 | + | |
354 | 418 | | |
355 | | - | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
356 | 422 | | |
357 | 423 | | |
358 | 424 | | |
359 | 425 | | |
360 | 426 | | |
361 | 427 | | |
362 | | - | |
363 | | - | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
364 | 431 | | |
365 | | - | |
366 | 432 | | |
| 433 | + | |
367 | 434 | | |
368 | 435 | | |
369 | 436 | | |
| |||
426 | 493 | | |
427 | 494 | | |
428 | 495 | | |
429 | | - | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
430 | 500 | | |
431 | 501 | | |
432 | 502 | | |
| |||
461 | 531 | | |
462 | 532 | | |
463 | 533 | | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
0 commit comments