Repository navigation
Commit 6e3aa75
fix(plugin-security): an organization-less permission-set read resolves organization-less rows only (#20584)
Fixes #20555
Clause-②: no
## The measurement: reached
The card's question was measured before any fix, on `main` at
`1c761c0d`, which already contains PR #20540 (merged as `f6ceddc3`). The
probe is committed on this branch as `bf72f620` and later became the
pin. It runs a real `ObjectQL` over a real `SqlDriver` (better-sqlite3
`:memory:`), the real platform object definitions and the real
`SecurityPlugin`, resolved through the `security` service it registers.
Principals are built by `buildContextForUser`, which runs the same
`resolveUserAuthzGrants` the request path uses.
What it read, stated abstractly:
- **Reached.** The principal is a member of one organization only and
has no organization active. It resolved permission sets that a different
organization had authored under the names of built-in roles it holds as
positions. The organization-less by-name read returned the other
organization's rows. Their `systemPermissions` reached the resolved
sets, and their object map (view-all and modify-all included) reached
`getEffectiveObjectPermissions`. The capabilities did not reach the core
envelope's `systemPermissions`, because §6b of `resolveUserAuthzGrants`
resolves by id. They arrived only through plugin-security's by-name
fold.
- **Control.** With the principal's own organization active, the read
was scoped and returned none of those rows.
- **A second face of the same read.** An organization-less principal
holding a global grant resolved another organization's same-named copy
of the granted set, not the global row the grant names.
Mechanism assumptions from the dispatch, each measured:
- **A1 holds.** `resolvePermissionSetsForContextUnmemoized` folds
positions and permissions into one request. The loader's by-name read
carries no tenant when none is active, and the driver's
`applyTenantScope` reads "no tenant" as an unscoped path.
`resolveOwnOrganizationRow(rows, undefined)` then returns whichever row
came first.
- **A2 holds, at runtime.** After `f6ceddc3`, a resolution with no
organization still lists every current membership's role in `positions`,
plus the audience anchor.
- **A3 holds.** `reserved-identity-names.ts` guards `sys_position.name`
and `sys_user_position.position` only. The package-owned collision
refusal (ADR-0086 D4) is about package ownership. Neither one touches an
organization-authored `sys_permission_set.name`.
- **A4: nothing legitimate is dropped.** The pin also holds the global
grants an organization-less principal really holds. A global position
assignment folded onto a global same-named set still resolves, and so
does a global user grant to a global set. Both resolve identically with
the principal's own organization active.
## The fix: one mechanism
The change lands in
`packages/plugins/plugin-security/src/security-plugin.ts`, in the
permission-set loader built in `start()`. With no active organization,
the by-name read now asks for organization-less rows only
(`organization_id: null`). A row scoped to an organization applies only
while that organization is active, and an organization-less row applies
everywhere. This is the rule `resolveUserAuthzGrants` already applies to
grant rows, and it matches ADR-0123 D2 ("tenant-scoped reads resolve to
nothing"). A read with an active organization is unchanged.
- **Why a predicate in the read, not a filter after it.** The read keeps
its `limit`. Filtered afterwards, other organizations' copies of a name
could fill the page and push out the global row the caller does hold.
`organization_id: null` is the spec's has-no-value predicate. The
organizations runtime already reads by it.
- **No reserved-identity guard.** The scoped fold alone closes the
cross-organization reach, so no second mechanism is added. Inside one
organization, a set named after a position still folds for that
organization's own members while it is active. That is the governed-fold
question, which is already warned about at runtime. It is not this card.
- **Blast radius.** Every consumer of `resolvePermissionSetsForContext`
reads through this loader: the data-plane middleware,
`getEffectiveObjectPermissions`, the delegated-admin gate, explain and
`/me/apps`. So the fix reaches all of them from this one place.
`packages/core/src/security/**` is not touched.
- **Public surface.** Nothing reachable from the package's `exports`
entry changes: no new export and no new option.
## Pins and their ablation
`packages/plugins/plugin-security/src/orgless-position-name-fold.test.ts`
holds 10 cases:
- a precondition: the principal really holds the colliding names;
- six negative pins, one fact each;
- three keep-pins: the global grants, the same grants with the
principal's own organization active, and a control that the authored set
does resolve for its own organization's member.
Ablation, from the committed state at `c3237a7b`. The loader's read was
put back unscoped through `scripts/ablation-replace.mjs` (anchor hit 1
time, blob `026ca66a` to `69cc688b`). A `trap` restored the file, and
the restore was proven against the HEAD blob.
```text
× the by-name read returns no other organization's row
× the authored sets are not among the resolved sets
× their systemPermissions do not reach the principal
× their object map does not reach the effective map
× a GLOBAL grant resolves the global row it names, not another organization's same-named copy
× …and the same-named copy's systemPermissions do not reach that principal
Tests 6 failed | 4 passed (10)
ABLATION RESTORED: packages/plugins/plugin-security/src/security-plugin.ts blob 026ca66 == HEAD
```
The four that stay green are the precondition and the three keep-pins,
which is the expected direction. On the first ablation run, the
global-grant pin stayed green: the SQL driver returns the page ordered
by id, and the global row's id sorted first. The fixture ids were
reordered so that the other organization's copy sorts first
(`c60be715`). The pin was re-ablated red after that.
## Local verification (all at `c3237a7b`)
- `pnpm --filter @objectstack/plugin-security exec vitest run`: 144
files, 3062 passed and 16 skipped.
- `pnpm --filter @objectstack/plugin-security run typecheck`: green. The
new test file is in the `tsconfig.test.json` program, confirmed with
`--listFiles`.
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 63 commands. All 63 were run, and
`--ran` reports 63 run, 0 NOT-MEASURED, 0 UNRUN.
- `check:type-check-debt` first answered exit 3 at this head: the
ablation restore left the source newer than `dist/*.d.ts`. It answered 0
after `pnpm --filter @objectstack/plugin-security build`.
- Earlier in the round, `check:engine-double-contract` flagged by-id and
write verbs on the pin's observed engine. The loader under test calls
`find` only, so the double now exposes `find` alone, and the ledger is
unchanged.
- A targeted dogfood subset ran against the rebuilt `dist/` (the fix
marker was confirmed in `dist/index.mjs` and `dist/index.js`):
`sharing-rule-org-less-caller`, `me-apps-and-everyone-baseline`,
`two-doors-permission`, `showcase-permission-zoo` and
`showcase-crud-persona-matrix`. 5 files, 87 tests passed.
- Lint, narrowed and stated as such. `eslint --no-inline-config --format
json` over the two touched TypeScript files reports 2 files, 0 errors
and 0 warnings. `eslint.config.mjs` enables no type-aware linting: every
`parserOptions` carries only `ecmaVersion` and `sourceType`, and there
is no `parserOptions.project`. So this diff cannot move a verdict on any
untouched file. The full `pnpm lint` run is CI's.
## Acceptance notes
- **Boundary, not a regression.** A principal with no organization
active no longer resolves a permission-set row that is stamped with an
organization. Could a real principal have relied on that? It would need
a global grant pointing at an organization-stamped set, or a global
position named like one. No producer in this repository writes either.
`bootstrapPlatformAdmin` points its global grant at an organization-less
row, and Setup stamps both the grant and the set with the active
organization. The remedy, stated in the changeset, is to make the
organization active or to grant the set globally.
- **Seeders are untouched.** `resolveOwnOrganizationRow(rows,
undefined)` still returns the first row for the seeders' own
single-posture pass. The fix changes the enforcement read only.
- **A docblock to recheck.** The docblock on `callerOrganizationId` says
a single-posture caller carries no organization. A single-posture
session that has an active organization does carry one into
`ctx.tenantId`, so that sentence is worth rechecking when the file is
next touched. This PR does not change that behaviour.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 8c87d26 commit 6e3aa75
3 files changed
Lines changed: 358 additions & 2 deletions
File tree
- .changeset
- packages/plugins/plugin-security/src
| 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 | |
|---|---|---|---|
| |||
| 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 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1521 | 1521 | | |
1522 | 1522 | | |
1523 | 1523 | | |
| 1524 | + | |
| 1525 | + | |
| 1526 | + | |
| 1527 | + | |
| 1528 | + | |
| 1529 | + | |
| 1530 | + | |
| 1531 | + | |
| 1532 | + | |
| 1533 | + | |
| 1534 | + | |
| 1535 | + | |
| 1536 | + | |
| 1537 | + | |
| 1538 | + | |
| 1539 | + | |
| 1540 | + | |
| 1541 | + | |
| 1542 | + | |
| 1543 | + | |
| 1544 | + | |
| 1545 | + | |
| 1546 | + | |
| 1547 | + | |
| 1548 | + | |
| 1549 | + | |
| 1550 | + | |
| 1551 | + | |
1524 | 1552 | | |
1525 | 1553 | | |
1526 | | - | |
| 1554 | + | |
1527 | 1555 | | |
1528 | 1556 | | |
1529 | 1557 | | |
| |||
1546 | 1574 | | |
1547 | 1575 | | |
1548 | 1576 | | |
1549 | | - | |
| 1577 | + | |
| 1578 | + | |
| 1579 | + | |
1550 | 1580 | | |
1551 | 1581 | | |
1552 | 1582 | | |
| |||
0 commit comments