Repository navigation
Commit 5fb8d15
fix(plugin-security): Everyone and Guest position descriptions read as admin copy (#23987)
Fixes #23919
Clause-②: no
## What changed
Setup's Positions page (`/apps/setup/metadata/position`) lists each
built-in position with its `description`. The two audience anchors'
descriptions carried design-record ids; they now read as plain admin
copy. The docblock above `AUDIENCE_ANCHOR_METADATA`
(`packages/plugins/plugin-security/src/builtin-positions.ts`), which
already cited ADR-0090 D5/D9, now ties each id to the sentence it
governs. Wording only: names, labels, bindings and the boot path are
unchanged.
| Position | Before | After |
|---|---|---|
| `everyone` | "... Permission sets bound to it are the default grants
for the tenant (ADR-0090 D5). High-privilege sets cannot be bound here."
| "... Permission sets bound to it are the default grants for the
tenant. High-privilege sets cannot be bound here." |
| `guest` | "... Bindings face the strictest checks — named objects
only, read-mostly, never a wildcard (ADR-0090 D9)." | "... Bindings face
the strictest checks — named objects only, read-mostly, never a
wildcard." |
## The first read: what the boot path does with a stored built-in row
whose description differs
Read at `origin/main` `68ffa5dc1a`, before any edit.
1. **The boot keeps the row and writes nothing.** No boot step writes a
`sys_position` row: the built-in and declared position seeders are
retired (`builtin-positions.ts` module doc, lines 16-19; ADR-0131 D2, C3
stage 6a). `builtin-positions.boot.test.ts` already pins this in its
three existing-database scenarios: the census equals `LEGACY_ROWS` and
the write ledger is empty (the case "writes no sys_position row, and
leaves an existing database's rows as they were").
2. **No census reports it as drift.** The only code that classifies
these rows is the upgrade ceremony (`os migrate`).
`packages/cli/src/utils/organization-ownership-inventory.ts` gives
`sys_position` fate 2, mirror deletion, with the `SEEDED` selector
(`managed_by` in package / platform / system). Only `AUTHORED` rows are
converted to definitions, so a platform-managed built-in row is deleted,
and it is never converted or compared.
3. **No reader shows the stored description.** No runtime package
declares the `sys_position` row object any more (C3 stage 8; header of
`src/__tests__/legacy-catalog-objects.testkit.ts`). The Positions nav
item is `/apps/setup/metadata/position?scope=environment`
(`security-plugin.ts:1587`). That is the metadata door, and it lists the
declarations. The only runtime read of a `sys_position` row,
`idSpellingHints` in `position-catalog-refusal.ts`, reads only the row's
`name`.
So an upgraded tenant's administrator sees the declaration's text, the
same as on a fresh install. The stored text is leftover data that
nothing reads, and `os migrate` deletes it. The claim's stop branch
assumed that existing tenants would keep the ADR ids if the boot kept
the stored text. Read 3 shows that assumption is false, so this PR
changes the declaration only and leaves the boot path untouched. A
boot-time refresh of these rows would add a `sys_position` write that
ADR-0131 C3 stage 6a retired, and nobody would see the result. The only
precedent that refreshed platform-managed rows from code, `os meta
resync`, was deleted in `9e9e6f97fc`.
## Pins
- `src/objects/admin-copy-vocabulary.test.ts` (from the family's
precedent, PR #23917) adds `securityBuiltinPositions` to its population.
The label and description of all six built-in positions are judged, and
the population is pinned to include `everyone`, `guest`,
`platform_admin` and `org_admin`.
- `src/builtin-positions.boot.test.ts` adds one case to each
existing-database scenario (`single`, `single + organization`,
`walled`). For `everyone` and `guest` it asserts three things: the
stored row's text (`LEGACY_ROWS`, unchanged) differs from the
declaration; the census row still holds that stored text; and the
security catalog read answers the declaration's text. The `LEGACY_ROWS`
docblock now says that the anchors' stored text is older than their
declarations.
## Verification
The implementation commit is `bebfe4cf6a`. The final head `d3a4d92391`
merges `origin/main` `2054a0888c` into it. None of the four incoming
commits touches plugin-security, its dependencies, or `packages/spec`
source.
- Build first: `pnpm --filter '@objectstack/plugin-security^...' build`
(the upstream dependency closure) — exit 0. After the merge, `turbo run
build --filter=!@objectstack/docs` — 75/75 tasks, exit 0.
- `pnpm --filter @objectstack/plugin-security exec vitest run
--maxWorkers=2 src/objects/admin-copy-vocabulary.test.ts
src/builtin-positions.boot.test.ts`: 2 files, 63 passed. That is 55 boot
cases (12 scenarios, 3 of them new existing-database cases) plus 8
vocabulary cases (1 new). Re-run on `d3a4d92391` together with
`builtin-positions.test.ts` and
`builtin-positions.declaration-phase.test.ts`: 4 files, 73 passed.
- `pnpm --filter @objectstack/plugin-security test` (at `bebfe4cf6a`):
188 files passed; 3736 passed, 45 skipped.
- `pnpm --filter @objectstack/plugin-security typecheck`: exit 0 (`tsc
--noEmit`, the scripts project, and `check:test-typecheck` over
`tsconfig.test.json`: 0 errors).
- **Ablation (one time, through `scripts/ablation-replace.mjs`, on the
committed tree):** I put `(ADR-0090 D9)` back into the `guest`
declaration. Anchor `never a wildcard.',` went x1 to x0, and the blob
went `9a36e5037bb6` to `1e2122e49ffb`. Result: 4 failed, 59 passed. The
vocabulary case failed on `guest.description (an ADR number, a decision
id)`, and all three new existing-database cases failed. Restore: blob
`9a36e5037bb6` == HEAD, and `git diff HEAD` is empty. The test imports
the module from `src`, so no `dist/` leg was needed.
- Translation bundles: `pnpm check:i18n` exit 0. plugin-security has 7
bundles, all in sync, and nothing to regenerate: the bundles hold object
copy, and position descriptions never reach them.
- Gates, on the final head `d3a4d92391`, after the build: `node
scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 65 commands for this diff, the same
list as at `bebfe4cf6a`. All 65 exited 0, with each exit captured before
any pipe. `--ran`: 65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN. On the
first run at `bebfe4cf6a`, `check:i18n` and `check:dual-build-cjs-loads`
answered exit 3 (PREREQUISITE NOT MET: no `dist/`) until the build had
run.
- Not run locally (CI's): `test:repo`, the dogfood shards, and the
workspace type-check lanes.
## Acceptance notes
- What existing tenants see after the upgrade: the new text, the same as
fresh installs. It is pinned by the existing-database case above. Their
stored `everyone`/`guest` rows keep the older text until `os migrate`
deletes them as seeded mirrors. This PR does not touch those rows.
- The four identity positions' descriptions come from
`BUILTIN_IDENTITY_METADATA` in `@objectstack/spec`. They carry no
design-record vocabulary today, and the new population case now guards
them too.
---
_Generated by [Claude
Code](https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent adce192 commit 5fb8d15
4 files changed
Lines changed: 77 additions & 11 deletions
File tree
- .changeset
- packages/plugins/plugin-security/src
- objects
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
| 42 | + | |
| 43 | + | |
43 | 44 | | |
44 | 45 | | |
45 | 46 | | |
| |||
66 | 67 | | |
67 | 68 | | |
68 | 69 | | |
| 70 | + | |
69 | 71 | | |
70 | 72 | | |
71 | 73 | | |
| |||
482 | 484 | | |
483 | 485 | | |
484 | 486 | | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
485 | 506 | | |
486 | 507 | | |
487 | 508 | | |
| |||
549 | 570 | | |
550 | 571 | | |
551 | 572 | | |
552 | | - | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
553 | 577 | | |
554 | 578 | | |
555 | 579 | | |
| |||
573 | 597 | | |
574 | 598 | | |
575 | 599 | | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
576 | 603 | | |
577 | 604 | | |
578 | 605 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
80 | | - | |
81 | | - | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
82 | 92 | | |
83 | 93 | | |
84 | 94 | | |
85 | 95 | | |
86 | 96 | | |
87 | | - | |
| 97 | + | |
88 | 98 | | |
89 | 99 | | |
90 | 100 | | |
91 | 101 | | |
92 | | - | |
| 102 | + | |
93 | 103 | | |
94 | 104 | | |
95 | 105 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
4 | | - | |
5 | | - | |
| 4 | + | |
| 5 | + | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
| 8 | + | |
| 9 | + | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
15 | | - | |
16 | | - | |
| 16 | + | |
| 17 | + | |
17 | 18 | | |
18 | 19 | | |
19 | 20 | | |
| |||
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
| 29 | + | |
28 | 30 | | |
29 | 31 | | |
30 | 32 | | |
| |||
120 | 122 | | |
121 | 123 | | |
122 | 124 | | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
123 | 137 | | |
124 | 138 | | |
125 | 139 | | |
| |||
0 commit comments