Repository navigation
Commit 520f66f
fix(plugin-security): PermissionDeniedError carries status beside statusCode, so a share-link permission refusal answers 403 at both doors (#21429)
Fixes #21405
Clause-②: no
## What was wrong
`PermissionDeniedError` (`plugin-security/src/errors.ts`) declared
`statusCode = 403` and no `status`. It was the only error class in that
module that did. A door that reads `status` alone derived no status from
it. `plugin-sharing`'s share-link route door reads `err?.status ?? 500`,
and that door serves `/api/v1/share-links` on the standalone server.
Measured on a showcase boot (`@objectstack/verify`, with the app's own
default profile), as a plain member, at `main` 6d67ad5:
| request | plugin route door | runtime dispatcher `/share-links` domain
|
|---|---|---|
| `POST /share-links` on a `showcase_client_brief` record the member
cannot read | 500 `PERMISSION_DENIED` | 403 `PERMISSION_DENIED` |
| `GET /share-links` | 500 `PERMISSION_DENIED` | 403 `PERMISSION_DENIED`
|
The dispatcher door was driven in-process, the way
`@objectstack/verify`'s own handle drives it: an `HttpDispatcher` over
the same booted kernel, with the same bearer token.
## The change (the triage ruling on the card)
- `PermissionDeniedError` carries `readonly status = 403` beside
`statusCode = 403`. The code, the message, `details`, `developerMessage`
and `statusCode` are unchanged. No door is edited, and no status helper
is added anywhere.
- The module's "Why each carries BOTH `status` and `statusCode`" note
now names the readers as they stand today. The share-link route door and
the sandbox boundary's passthrough read `status` alone.
`errorFromThrown` and `mapDataError` read both spellings. The note used
to say `mapDataError` reads `status` alone, which stopped being true
when it learned both spellings.
## Pins
- **Enumeration** (`plugin-security/src/errors.test.ts`). Every `Error`
subclass that `errors.ts` exports is constructed, and each must carry a
numeric `status` and `statusCode` with equal values. The population is
read from the module's exports, so a class added later is checked
without being listed. A floor names the eight classes exported today, so
the enumeration cannot pass over nothing.
- **One refusal, both doors**
(`runtime/src/domains/share-links-enforcement-context.test.ts`, the new
`[#21405]` block). The harness has one engine double with the whole
`SecurityPlugin` middleware booted on it, one `ShareLinkService` and one
envelope. It drives both production entries: `registerShareLinkRoutes`
mounted on a route recorder, and `handleShareLinksRequest` over the
dispatcher's own `errorFromThrown`. A create by a caller with no
`allowRead` on the object answers 403 `PERMISSION_DENIED` through both
doors, under the `single` and the `group` posture, and writes no link.
- **Create only.** PR #21403 (for #21328, merged into this branch) made
a member's own list a self-scoped read. Measured on the showcase boot
after that merge: `GET /share-links` answers 200 through both doors,
with no filter, with the Share dialog's object and record filter, and
with `includeRevoked`. No list request reaches the refusal any more, so
no list case is pinned. Before the merge, a list case was written, and
it went red under the ablation below.
- The existing `[#6649]` shared-catch case drove the production class to
reach the dispatcher catch's `statusCode` channel. The class now carries
`status` as well, so the case adds a `statusCode`-only throw beside it,
and that channel stays pinned.
## Tests (head 01bb1de unless noted)
- Runtime: `share-links-enforcement-context`,
`data-permission-denied-envelope`, `permission-denied-error-parity` and
`share-links-internal-hash-probe` give 4 files, 39 passed.
- `pnpm --filter @objectstack/runtime typecheck` is OK. Its test layer
holds 27 files / 190 errors / 68 signatures in its ledger, unchanged.
- `pnpm --filter @objectstack/plugin-security typecheck` is OK.
`errors.test.ts` is in the `tsconfig.test.json` program (`--listFiles`:
1 hit).
- `pnpm --filter @objectstack/plugin-security test`: 161 files, 3502
passed, 33 skipped. `pnpm --filter @objectstack/plugin-sharing test`: 38
files, 928 passed. Both ran at 46b09ea. Since then, the only change in
either package is a comment in `errors.test.ts`.
- Importers whose answer could move: the 13 `rest` test files that
import `@objectstack/plugin-security` give 209 passed. The dogfood
share-link files (`share-links-self-list`,
`showcase-client-liaison-fixtures`, `audit-log-internal-fields`) pass.
Both ran at 46b09ea.
- Gates: `dispatch-gates --commands` at 01bb1de derives 67 commands,
and all 67 exit 0. The `--ran` reconciliation reports 67 derived, 67 run
and 0 NOT-MEASURED, derived from the recorded exit codes. At an earlier
head, `check:dual-build-cjs-loads` first exited 3 (`PREREQUISITE NOT
MET`: 8 unbuilt packages). Those were built, and every later run exits
0.
- Lint (narrowed): `eslint --no-inline-config --format json` over the 3
changed TS files reports 3 files, 0 errors and 0 warnings, and the
config resolves for each file. `eslint.config.mjs` enables no type-aware
linting (its own note: no `parserOptions.project`, no typed rules), so
this diff cannot move a verdict on an untouched file. The full `pnpm
lint` is CI's.
## Ablations
Each leg ran from a committed head through
`scripts/ablation-replace.mjs`: the anchor hit, the blob changed, and
the restore was proven by an empty `git diff HEAD`. `plugin-security`
resolves from `dist/` in the runtime and dogfood suites, so each of
those legs rebuilt it, and `scripts/ablation-dist-preflight.mjs` proved
the marker in `dist/` (4 files) and then absent (all 6 files, tree
clean).
| mutation in `errors.ts` | suite | red | green |
|---|---|---|---|
| `status` renamed off `PermissionDeniedError` (`status_ablated_21405`)
| runtime `share-links-enforcement-context` (head d9f9aab) | 2: both
`[#21405]` postures, plugin door `expected 500 to be 403` | 18, every
`[#6649]` dispatcher case included (that door reads `statusCode`) |
| same | `errors.test.ts` | 2: the enumeration (`PermissionDeniedError:
status=undefined statusCode=403`) and the 403 case | 1 (the floor) |
| same, at b7fdf64 | the showcase dogfood pin then on this branch
(create and list, both doors) | 2: create 500 vs 403, list 500 vs 403 |
1 (persona) |
| a scratch `export class AblatedStatusCodeOnlyError extends Error {
readonly statusCode = 418; }` | `errors.test.ts` | 1:
`AblatedStatusCodeOnlyError: status=undefined statusCode=418 — declare
both` | 2 |
| `status = 404` on `PermissionDeniedError` | `errors.test.ts` | 2:
`status 404 !== statusCode 403` and the 403 case | 1 |
The restore legs pass: runtime 20/20 and `errors.test.ts` 3/3. The first
attempt at the planted-class leg did not run. `ablation-replace` refused
it before any test, because its replacement contained the anchor, so the
anchor count did not fall. It was redone with an anchor the replacement
does not contain.
## Docs
`content/docs/**` (outside `releases/`) and `skills/**` hold no sentence
this change makes false. The status table in
`protocol/kernel/error-handling.mdx` gives 403 for insufficient
permissions, and that is now true at the share-link door.
`permissions/field-level-security.mdx` shows a partial dump of the
thrown error without `status`. The dump states nothing false, so it is
left alone (an edit was made on this branch and reverted).
## Acceptance notes
- **Pin location.** The claim put the route pin in
`packages/qa/dogfood/test/` or beside the plugin. A dogfood version came
first: a showcase boot, both doors, create and list. It passed, and it
went red under ablation. It reached the dispatcher door by importing
`runtime/src/http-dispatcher.ts`, and `check:test-source-alias` refused
that (exit 1): four new unaliased artifact imports into dogfood
(`metadata-protocol`, `observability`, `rest`, `service-datasource`).
Fixing that means aliasing them in dogfood's vitest config, which
changes every dogfood boot. The plugin packages cannot import runtime,
because of the dependency direction. The runtime package already imports
both doors, so the pin moved there, beside the dispatcher's own
`[#6649]` block.
- **Doors this fixes, named and not edited.** Each reads `status` alone:
- `plugin-sharing/src/share-link-routes.ts`, five catches (create, list,
revoke, resolve, messages). Create and list are measured above. Resolve
and messages read under the system context, so they never meet this
refusal. Revoke is not measured.
- The sandbox boundary. `SANDBOX_ERROR_PASSTHROUGH` in
`runtime/src/sandbox/quickjs-runner.ts` carries `code`, `fields`,
`status` and `userMessage`, but not `statusCode`. A
`PermissionDeniedError` from a host call inside a sandboxed body now
crosses with its 403. Not measured.
- `metadata-protocol/src/protocol.ts`, the `deleteMetaItem` catch
(`e.status = err?.status ?? 500`). Not measured.
- `service-analytics/src/analytics-service.ts`,
`hasDeclaredErrorEnvelope` (a numeric `status` plus a `code`). A
`PermissionDeniedError` now counts as declared and is re-thrown before
the missing-source heuristic. Not measured.
- `runtime/src/domains/actions.ts`, the `setActionActive` catch
(`status`, else 503). Not measured, and this refusal is unlikely there.
- **Readers whose wire answer does not move.**
- `rest`'s `resolveErrorResponse` passthrough reads `status` alone. A
`PermissionDeniedError` used to fall through to `mapDataError`, which
answered 403 `PERMISSION_DENIED` with the message and `object`. It now
takes the passthrough arm, with the same status, code and `object`. The
message passes the 500-character client bound and the
declared-code-prefix strip, and neither changes a message inside those
limits. This is read from the code; the 13 `rest` test files above pass.
- `rest-server`'s analytics envelope reader ① now answers this refusal
where ①b answered it, with the same status and code. Read from the code,
not measured.
- `plugin-auth`'s `createOAuthClient` catch reads `status` alone, but it
only meets better-auth errors.
- **A stale comment in a door file, not edited.**
`runtime/src/domains/share-links.ts` (the catch's docblock) says the
enforcement refusals "carry `statusCode`, not `status`". That is no
longer true of `PermissionDeniedError`. Carrier: whoever next touches
that file; no carrier is named.
- `plugin-security/src/packaged-permission-set-lock.ts` holds two more
403 classes outside `errors.ts`. Both already carry both spellings. As
ruled, the enumeration covers `errors.ts` exports only.
- #21329 (`createLink` refusals) is not addressed here. Triage orders it
after this card, and its pins can now read either door.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 8b123c0 commit 520f66f
4 files changed
Lines changed: 265 additions & 24 deletions
File tree
- .changeset
- packages
- plugins/plugin-security/src
- runtime/src/domains
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 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 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
27 | 40 | | |
28 | 41 | | |
29 | 42 | | |
| 43 | + | |
30 | 44 | | |
31 | 45 | | |
32 | 46 | | |
| |||
61 | 75 | | |
62 | 76 | | |
63 | 77 | | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
70 | 88 | | |
71 | 89 | | |
72 | 90 | | |
| |||
412 | 430 | | |
413 | 431 | | |
414 | 432 | | |
415 | | - | |
416 | | - | |
417 | | - | |
418 | | - | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
419 | 436 | | |
420 | 437 | | |
421 | 438 | | |
| |||
Lines changed: 143 additions & 14 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
| 55 | + | |
55 | 56 | | |
56 | | - | |
| 57 | + | |
57 | 58 | | |
58 | 59 | | |
59 | 60 | | |
| |||
133 | 134 | | |
134 | 135 | | |
135 | 136 | | |
136 | | - | |
137 | | - | |
| 137 | + | |
| 138 | + | |
138 | 139 | | |
139 | 140 | | |
140 | 141 | | |
| |||
550 | 551 | | |
551 | 552 | | |
552 | 553 | | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
553 | 559 | | |
554 | 560 | | |
555 | 561 | | |
| |||
635 | 641 | | |
636 | 642 | | |
637 | 643 | | |
638 | | - | |
639 | | - | |
640 | | - | |
641 | | - | |
642 | | - | |
643 | | - | |
644 | | - | |
645 | | - | |
646 | | - | |
647 | | - | |
648 | | - | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
649 | 664 | | |
650 | 665 | | |
651 | 666 | | |
| |||
917 | 932 | | |
918 | 933 | | |
919 | 934 | | |
| 935 | + | |
| 936 | + | |
| 937 | + | |
| 938 | + | |
| 939 | + | |
| 940 | + | |
| 941 | + | |
| 942 | + | |
| 943 | + | |
| 944 | + | |
| 945 | + | |
| 946 | + | |
| 947 | + | |
| 948 | + | |
| 949 | + | |
| 950 | + | |
| 951 | + | |
| 952 | + | |
| 953 | + | |
| 954 | + | |
| 955 | + | |
| 956 | + | |
| 957 | + | |
| 958 | + | |
| 959 | + | |
| 960 | + | |
| 961 | + | |
| 962 | + | |
| 963 | + | |
| 964 | + | |
| 965 | + | |
| 966 | + | |
| 967 | + | |
| 968 | + | |
| 969 | + | |
| 970 | + | |
| 971 | + | |
| 972 | + | |
| 973 | + | |
| 974 | + | |
| 975 | + | |
| 976 | + | |
| 977 | + | |
| 978 | + | |
| 979 | + | |
| 980 | + | |
| 981 | + | |
| 982 | + | |
| 983 | + | |
| 984 | + | |
| 985 | + | |
| 986 | + | |
| 987 | + | |
| 988 | + | |
| 989 | + | |
| 990 | + | |
| 991 | + | |
| 992 | + | |
| 993 | + | |
| 994 | + | |
| 995 | + | |
| 996 | + | |
| 997 | + | |
| 998 | + | |
| 999 | + | |
| 1000 | + | |
| 1001 | + | |
| 1002 | + | |
| 1003 | + | |
| 1004 | + | |
| 1005 | + | |
| 1006 | + | |
| 1007 | + | |
| 1008 | + | |
| 1009 | + | |
| 1010 | + | |
| 1011 | + | |
| 1012 | + | |
| 1013 | + | |
| 1014 | + | |
| 1015 | + | |
| 1016 | + | |
| 1017 | + | |
| 1018 | + | |
| 1019 | + | |
| 1020 | + | |
| 1021 | + | |
| 1022 | + | |
| 1023 | + | |
| 1024 | + | |
| 1025 | + | |
| 1026 | + | |
| 1027 | + | |
| 1028 | + | |
| 1029 | + | |
| 1030 | + | |
| 1031 | + | |
| 1032 | + | |
| 1033 | + | |
| 1034 | + | |
| 1035 | + | |
| 1036 | + | |
| 1037 | + | |
| 1038 | + | |
| 1039 | + | |
| 1040 | + | |
| 1041 | + | |
| 1042 | + | |
| 1043 | + | |
| 1044 | + | |
| 1045 | + | |
| 1046 | + | |
| 1047 | + | |
| 1048 | + | |
0 commit comments