Repository navigation
Commit 8a058d8
fix(plugin-security): under group, a no-active-organization insert gets the ADR-0123 D2 403, not a 500 (#24819)
Fixes #24784
Clause-②: no
## What changed
On the `group` tenancy posture (any union-scope posture), a member whose
session has no active organization sent a create on a tenant-scoped
object. The Layer 0 write wall in `plugin-security` let it through: its
scope pre-test (`callerHasOrganizationScope`) reads the membership set
under a union-scope posture, and a member's set is not empty. But a new
row lands in the active organization (ADR-0105 D5), and there was none.
So the engine met the row as an organization-less system write and
refused it with `500 ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED`. Nothing
was written.
The wall now refuses that insert itself, with the ADR-0123 D2 answer
`isolated` already gave the same session: `403 PERMISSION_DENIED`,
"[Security] Access denied: 'OBJECT' is scoped to an organization, and
this session has no active organization — so this insert has no
organization to place the record in. Join or select an active
organization and retry."
`packages/plugins/plugin-security/src/security-plugin.ts`, the step 3.7
write wall:
- `carriesActiveOrganization(context)` is new, beside
`callerHasOrganizationScope`. The non-union branch of
`callerHasOrganizationScope` now calls it, with the same answer as
before.
- The ADR-0123 D2 throw is one local function,
`refuseNoActiveOrganization`. The scope pre-test throws it as before,
with the same message, details and operator text. The new arm throws it
too, with its own operator text, which says what the context does and
does not carry under this posture.
- The new arm applies to an `insert`, under a union-scope posture, when
the caller's own Layer 0 applies to the object and the context carries
no `tenantId`. It judges the stored image, in the post-hook seam the
engine already runs (`postHookWriteImageCheck`). That is the row after
the `beforeInsert` chain, the same rows the engine's own organization
check (`resolveSystemInsertOrganization`) reads one step later. It runs
first in that seam, so the session's state is the reason reported.
- The `canWriteObject` doc names the new arm among the post-image
refusals it does not judge.
`no-active-organization-write-refusal.test.ts`: the control "a NON-EMPTY
membership set lands, even with no active organization" pinned exactly
the branch this card changes. Its terminal stands in for the engine, so
it never met the 500. It is replaced by the refusal case and its
single-fact control: the same set with an active organization lands.
`no-active-organization-insert-union-scope.test.ts` is new. It runs on
the real stack: ObjectQL, SqlDriver and SecurityPlugin, booted the way
`tenant-wall-post-hook-image.test.ts` boots them.
## Why the stored image, and not `organizationWallRefusal`
I measured the base before choosing. Under `group` today, an insert
whose own app `beforeInsert` hook places the row in an organization the
membership wall admits is admitted, while `isolated` refuses it ahead of
the hooks. A row-less change to the predicate in
`organizationWallRefusal` would refuse it too. That turns a 2xx into a
403, which is outside "the change turns a 500 into the designed 403".
The stored image asks exactly the engine's question, so every answer
this PR changes was a 500 before.
The cost: `canWriteObject` is asked about no row. It keeps answering
`true` for this caller's create preview, as it did before. Its doc
already lists post-image refusals as outside its answer, and now names
this arm there.
## Readings: real stack, in process
A member of `org_a` and `org_c`, no active organization, on a public
tenant object. Base `a43405d596`, fix `cca0ba958a`:
| case | `group` before | `group` after | `isolated` before and after |
|:--|:--|:--|:--|
| create | 500 `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` | 403
`PERMISSION_DENIED` | 403 `PERMISSION_DENIED` |
| create, a payload naming a member organization | 500 | 403 | 403 |
| platform operator with no active org, public tenant object | 500 | 403
| 403 |
| platform operator with no active org, private object | 500 | 500
(unchanged, see notes) | 500 |
| create whose app hook places the row in a member organization | 2xx |
2xx (unchanged) | 403 |
| same member with an active organization: create | 2xx, row in `org_a`
| 2xx, row in `org_a` | not run |
| update by id / by predicate of a member organization's row | 200 | 200
| 404 (row not visible) |
| delete of a member organization's row | 200 | 200 | 404 |
| read | member orgs' rows | member orgs' rows | not run |
The engine's static read-only strip drops a session payload's
`organization_id` on insert. So a payload value never decides where a
session's new row lands, before or after this change.
## Which verbs ADR-0123 D2 and the checklist item cover
- `access-security.no-active-org-session-semantics`: acceptance[1] and
step 3 test a create, and acceptance[5], acceptance[6] and negative[3]
require no 500 anywhere. Covered for insert, single and bulk.
- Update: ADR-0123 D2 names update. Under `group` it states the scope as
the membership set. A no-active-org member's update of a row that set
admits answers 200 before and after, and the row keeps its own
organization. Nothing has nowhere to land, so it is not changed.
- Delete: this is the ADR-0123 D2 boundary, and it is not changed.
## Tests (head `94f661fbea`, the merge of `origin/main` `307ce4c4b4`)
- The real-stack pin on the base `a43405d596`, before the fix: 4 failed,
2 passed. `group` answered `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` /
500 where `isolated` answered `PERMISSION_DENIED` / 403. The fourth
failure was the active-org control's sort order in the test itself,
fixed before the fix commit.
- `pnpm --filter @objectstack/plugin-security test`: 193 files passed,
4013 passed, 45 skipped.
- `pnpm --filter @objectstack/plugin-security typecheck`: exit 0, and
`check:test-typecheck` is OK. `tsc -p tsconfig.test.json --listFiles`
lists both test files among the package's test files.
- Both ran in one `&&`-joined command under the verify lock, after the
closure build (turbo, `--filter='@objectstack/plugin-security...'`,
19/19 tasks). The lock printed `VERDICT command-exit 0`.
Ablation, on the committed fix, run on `cca0ba958a` and again on
`94f661fbea`. `node scripts/ablation-replace.mjs` replaced the arm's
last conjunct with a constant false, which puts the old predicate back:
- On disk: the anchor went from 1 to 0, the replacement from 0 to 1, and
the blob changed (`0f12f626` to `4c7f7ebf` on the final head).
- Result, both times: 4 failed, 22 passed. The ablated `group` case
received `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED`, the
equality-with-`isolated` case failed, and the unit case was not refused.
Every control stayed green.
- Restored with `git checkout HEAD -- PATH`: the blob equals HEAD's, and
`git diff HEAD` and `git status` are empty.
- The first attempt was a no-op. Its replacement contained the anchor,
so the tool refused and restored before any test ran. It is discarded.
## Gates (local, head `94f661fbea`)
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`, with no paths, derived 69 commands for this
diff. All 69 exited 0, and each exit was captured before any pipe.
- `--ran` over that record answered "69 derived, 69 run, 0 NOT-MEASURED,
0 UNRUN" and exited 0.
- `check:i18n` and `check:dual-build-cjs-loads` first answered exit 3
(`PREREQUISITE NOT MET`, nothing measured) on an earlier head. Both
measured green once the builds they name existed.
- These run in CI, not locally: the repo-wide scans (`pnpm lint` first),
`test:repo`, Test Core, Build Core and the Type Check lanes. The
main-only battery (dogfood, Temporal Conformance) runs after merge. This
PR's check-runs decide them.
- The derivation warned STALE TREE: `origin/main` was 6 commits past
this head's merge base when it last ran. The queue rebuilds on current
`main`.
## Acceptance notes
- ADR-0123 D2 says "Under `group`, 'no organization scope' means an
empty membership set". That sentence now describes the scope pre-test
only. A union-scope insert with no active organization is also refused,
with the same answer. This is the card's ruled answer (triage record
6285293541). An ADR addendum or status-line amendment is a governed docs
change, not part of this PR.
- Same family, not changed here (the dispatch keeps the operator path as
it is): a platform operator with no active organization, creating on a
posture-permitting tenant object, gets `500
ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` under both `group` and
`isolated`. This is an in-process reading only, not measured at the HTTP
door. The card's second candidate, the engine mapping a non-system
caller to a 4xx, would cover it.
- Observed in passing, in process, not measured at the HTTP door: under
`group`, a member with active organization `org_a` whose payload names
`org_c` from the same set gets a 2xx, and the row is stored in `org_a`.
The payload value is dropped by the engine's read-only strip, not
refused. The organizations runtime's stamping comment says a supplied
value is admitted or refused, never rewritten.
- `packages/objectql/src/engine.ts` still says, at the insert post-image
seam, that the Layer 0 tenant wall judges only the pre-hook image. That
has been stale since the wall gained its stored-image judgement. It is
out of bounds for this PR.
- Named for reference only: #24783 holds the round's evidence and
remains open; #21476 is the anonymous public-form submit, a different
caller; #24343 is NULL `organization_id` rows from framework writers and
remains open. This PR changes none of them.
---
_Generated by [Claude
Code](https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 7282a95 commit 8a058d8
4 files changed
Lines changed: 373 additions & 20 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 | + | |
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
41 | | - | |
42 | | - | |
43 | | - | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
| |||
289 | 289 | | |
290 | 290 | | |
291 | 291 | | |
292 | | - | |
293 | | - | |
294 | | - | |
295 | | - | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
296 | 299 | | |
297 | | - | |
| 300 | + | |
298 | 301 | | |
299 | 302 | | |
300 | 303 | | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
301 | 314 | | |
302 | 315 | | |
303 | 316 | | |
| |||
0 commit comments