Repository navigation
Commit 605b3e2
fix(plugin-security): an on-behalf-of by-id write never admits a row its delegator's own by-id write refuses (ADR-0090 D10) (#24792)
Fixes #24746
Clause-②: no
## What was wrong
ADR-0090 D10 says an on-behalf-of (delegated) principal never exceeds
its delegator. The by-id write's pre-image check (`security-plugin.ts`,
step 2.7) already composes the delegator's own write filter beside the
principal's. But it composed that filter differently from the filter the
delegator's own direct write of the same row gets. Where an app-authored
row-level write policy bounds the delegator, an OAuth agent acting for
that delegator could therefore be admitted for a row that the
delegator's own by-id write refuses. Since #24745 (the #24646 card),
explain's on-behalf-of write verdict composes the same delegated path,
so it gave the same admit.
## The fix, in rule terms
Every row gate that judges a delegated by-id write now also composes the
delegator's filter as the delegator's own direct write of that row gets
it, and requires both compositions to pass:
- **Step 2.7, the pre-image check** of a by-id `update` or `delete`.
This is the card's gate.
- **Step 3.6, the post-image check** of the row an `update` stores.
- **Step 2.8, the master gate** of a `controlled_by_parent` detail
write, on the delegator's pass.
`delegatorOwnWriteFloorOptions` is the one helper. It returns the
composition knobs the delegator's own direct by-id write of the row
would get, asked as the delegator and not as the delegated context. The
three gates each AND that composition beside the one they already used.
**The change only narrows.** A write is admitted only where today's
composition admits it AND the delegator's own composition admits it. No
row refused before is admitted. Writes that are not delegated, bulk
writes, and every other gate are unchanged.
**Explain.** `security.explain`'s on-behalf-of record verdict composes
the same two filters (`delegatorRecordWriteFilter` in
`explain-engine.ts`). Its master leg already runs the door's own
`assertControlledByParentWrite` for both principals, so the step 2.8
change reaches explain without a second edit. Explain's record verdict
still matches the door: every rig 4 row of
`explain-enforce-parity.test.ts` passes, and so do 14 new rig 4 rows for
this card.
### Bounded in-place fix: steps 3.6 and 2.8
The claim's file surface names step 2.7. I measured on `origin/main`
(`795ff708d8`), in both postures: the same rule broke at two more gates
of the same delegated write, for the same reason, in the same file:
- the post-image check;
- the master gate's delegator pass, for detail `update` and `delete`.
All four conditions of the bounded in-place exemption hold:
1. It is the same defect class.
2. The fix is the identical mechanical construction, from one helper.
3. `security-plugin.ts` is held by this claim alone.
4. It uses the same test family, with no new verification surface.
The evidence is the pins below and ablations B and C. The claim's
declared file surface needs the same amendment. That is the seat's
write, and it is named in the report.
## Not changed, deliberately
There are shapes where the delegator's own direct write admits a row
that the delegated write has always refused. Those rows stay refused.
Two such rows are pinned as unchanged in the new file, and rig 4 already
pins two more. Making the delegated write match the delegator's own
composition exactly there would WIDEN a security boundary. That is the
maintainer's call, so it is not done here. It goes to the seat as an
open question.
This PR does not touch `plugin-sharing`, `rls-compiler.ts`,
`packages/spec/**` or the shipped permission sets.
## Evidence
**Measured first, on `origin/main` `795ff708d8`.** This was a real
stack: ObjectQL with the SQL driver, `SecurityPlugin`, the sharing
service and middleware, and contexts from `resolveAuthzContext` /
`assembleExecutionContext` with the `mcp_agent_data_write` OAuth
ceiling. Under both `isolated` and `single`, the delegator's own write
was refused with 403 `PERMISSION_DENIED`, and the write on its behalf
was admitted, for:
- the by-id `update` and `delete`;
- the stored-row check;
- the master-gated detail `update` and `delete`.
Explain admitted those writes as well. After the fix, every one of them
is refused for both callers. Explain matches the door on each, and the
controls are unchanged.
**Tests, at head `e6f4cff0ce`:**
- `pnpm --filter @objectstack/plugin-security test`: 191 files, 3972
passed, 45 skipped. Exit 0.
- `pnpm --filter @objectstack/plugin-security typecheck`: `tsc --noEmit`
for `src`, scripts and the test layer (`check:test-typecheck`: 0 files,
0 errors). Exit 0.
- New `src/delegated-write-never-exceeds-delegator.test.ts`: 22 passed.
Each case asserts both callers' outcomes with the ADR-0112 envelope
(`code` and `status`), and explain's `record.visible` beside the door.
It covers both postures, and every refusal has the delegator's own
request as its control.
- `src/explain-enforce-parity.test.ts`: 164 passed. The 14 new rig 4
rows are labelled `#24746`.
**Ablations.** Each removes one leg's new composition through
`scripts/ablation-replace.mjs`. The anchor must hit once, and the blob
must change on disk. Each run was restored with `git checkout HEAD`, and
the restore was proven by blob equal to HEAD and an empty `git diff
HEAD`.
| Leg removed | Result |
|---|---|
| A: step 2.7 pre-image | 4 red: the delegated `delete` cells, in the
new file and in rig 4. The `update` cells stay green because the step
3.6 leg refuses them too, so `update` has two legs. |
| B: step 3.6 post-image | 2 red: the stored-row cells. |
| C: step 2.8 master gate | 6 red: the detail `update` and `delete`
cells, door and explain. |
| D: explain's second composition | 10 red: explain's verdict for the
delegated `update` and `delete` cells. The door still refuses. |
| All four at once (the old composition) | 18 red: every breach pin in
both files. Every control and every unchanged-refusal row stays green. |
**Gates.** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` on this diff derived 69 commands. Each was
run, and its exit was recorded before any pipe. `--ran` reconciliation:
69 derived, 67 run with exit 0, 2 NOT MEASURED, 0 unrun.
The two NOT MEASURED gates are `check:dual-build-cjs-loads` and
`check:i18n`. Both exited 3, PREREQUISITE NOT MET, because they read a
whole-repo build that this container does not have. CI runs them.
The changeset is
`.changeset/24746-delegated-write-never-exceeds-delegator.md`, a
`@objectstack/plugin-security` patch.
## Acceptance notes
- The delegator's pass in the master gate is recognised as the pass
where the gate's `context` carries no delegation link while the
operation context does. This is the same reading the existing floor
exclusion uses there, which reads the link off the operation context.
The method's only caller is the chain walk in
`assertControlledByParentWrite`, which passes both through unchanged.
- Explain does not model the step 3.6 post-image check, because its
record verdict has no payload. This is unchanged by this PR.
- I did not run `test:repo` locally. It is CI's run (`Test Core`).
---
_Generated by [Claude
Code](https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 12d1476 commit 605b3e2
5 files changed
Lines changed: 601 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 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 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 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 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 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
0 commit comments