Repository navigation
Commit f782f17
fix(storage): a refused attach tombstones the uploader's never-attached file, outside the refused write's unit of work (#22542)
Fixes #22466
Clause-②: no
## Summary
When the attachment gate refuses a `sys_attachment` insert, the refused
`file_id` now goes to the lifecycle. The lifecycle tombstones the
caller's own never-attached upload with the same tombstone a
last-join-row removal writes (`status: 'deleted'`, `deleted_at`). From
there the file follows the existing path: the 30-day `ttl`, then the
reap guard, which re-checks `findFileHolder` at sweep time. There is no
new sweeper, no new lifecycle trigger, and no change to `sys_file`'s
lifecycle declaration. The tombstone runs outside the refused write's
unit of work, so a caller's transaction rolling back does not undo it.
This is direction (a) as triage ruled it (comment `6080400758`). It
covers every refusal leg of the gate on `main`, including the
master-detail leg PR #22513 added (unlock `6087255379`).
Files: `packages/services/service-storage/src/attachment-lifecycle.ts`,
`packages/services/service-storage/src/attachment-access-hooks.ts`, a
new pin file
`packages/services/service-storage/src/attachment-refused-attach-tombstone.test.ts`,
and a `patch` changeset.
## Before: the reproduction on `origin/main` 5910b5e
A real `ObjectQL` engine over sqlite `:memory:`, with the storage
lifecycle hooks and the attach gate installed as `StorageServicePlugin`
installs them. The caller `u1` uploaded `f1`: committed, `attachments`
scope, `owner_id` `u1`.
| refusal leg | answer | `sys_file` after | join rows | transactions
opened by the insert |
| --- | --- | --- | --- | --- |
| sharing `deny` | 403 `ATTACHMENT_PARENT_ACCESS` | `committed`,
`deleted_at` null | 0 | 0 |
| sharing non-verdict | 403 `ATTACHMENT_PARENT_ACCESS` | `committed`,
`deleted_at` null | 0 | 0 |
| master-detail check `deny` | 403 `ATTACHMENT_PARENT_ACCESS` |
`committed`, `deleted_at` null | 0 | 0 |
| master-detail check `unresolvable` | 403 `ATTACHMENT_PARENT_ACCESS` |
`committed`, `deleted_at` null | 0 | 0 |
Nothing later reaches the file. The tombstone hooks fire only when a
file loses its last join row, and the declared lifecycle nominates only
`deleted_at` (`ttl`) or `pending` (`retention`) rows.
## Where the refusal throws, and where the tombstone is written
- **The refusal throws** from the gate's `beforeInsert` hook, inside
`engine.insert`'s middleware chain. Every refusal leg funnels through
one `mayEditParent` false, then one `forbid('ATTACHMENT_PARENT_ACCESS',
…)`. That single site makes the one call to the new tombstoner, so no
leg can drift. A rejection from either check (an outage, with its own
`503`) never reaches that line, and it is not tombstoned.
- **The transaction boundary, measured:** `engine.insert` opens no
transaction of its own. A driver `beginTransaction` spy counted 0 calls
during a refused insert. The generic `/data` create door (`createData`
in the protocol, read from source) calls it without one. A system write
made while the refusal unwinds lands and survives. Inside a
caller-opened `engine.transaction` (an `atomic` batch, an explicit
`transaction()`), the same write joins the caller's transaction through
the engine's ambient store (ADR-0034) and was rolled back. So mechanism
assumption 2 ("the refusal throws inside the insert's transaction") is
half false: the insert has none, and only a caller's own unit of work
can roll the write back.
- **The tombstone is written** by a run detached from the refusal's
async context. `createRefusedAttachTombstoner` is created when the gate
is installed (`kernel:ready`, outside every engine operation) and
captures `AsyncLocalStorage.snapshot()` there. Each refusal schedules
its run inside that snapshot on a later event-loop turn. The engine's
ambient transaction store holds nothing there, so the run reads and
writes on its own connection after the refusal has left the operation.
The refused write never awaits it. Awaiting an out-of-transaction query
from inside an open single-connection transaction is the deadlock
ADR-0034 describes. A detached one queues until the transaction releases
the connection.
- **Reuse:** the tombstone write and the "live attachments file"
predicate are now one function each (`writeTombstone`,
`isLiveAttachmentsFile`), shared by `tombstoneOrphanedFiles` (behaviour
unchanged) and the refusal path. "Is anything still holding it" is
`findFileHolder`, the one definition the reap guard, the download path
and the inventory already use.
## The conditions, read after the refusal
The file is tombstoned only when all of these hold:
- the file is `attachments`-scope and `committed`;
- nothing holds it (`findFileHolder`: zero join rows and no `ref_*`
owner);
- the refused caller is its uploader (`isFileUploader`, the upload
ownership rule).
Triage named the uploader `uploaded_by`. `sys_file` has no such column:
its uploader is `owner_id`, which the upload doors stamp from the
session. So that is the column the condition reads. The refusal does not
wait on any of this, and the refusal envelope is byte-identical whether
the file is the caller's, another user's, or unknown. The refusal
discloses nothing about the file, not even through its timing (mechanism
assumption 4).
## A client retry of the same `file_id` inside the 30-day window
Measured on `main` before the change: a tombstoned file that is attached
again is revived by the existing `afterInsert` leg (`committed`,
`deleted_at` null). Pinned after the change:
- **Admitted on retry** (for example, the grant changed): 201, the file
is attached and revived to `committed`.
- **Refused on retry:** the same refusal (code, status, message and
object identical), and the file stays tombstoned with its first
`deleted_at` untouched (run verdict `kept: not a committed
attachments-scope file`).
There is no silent half-state in either case.
## Pins and ablations
The tombstone runs detached, so every pin waits on the one debug line
each run ends on (its own verdict) rather than on a timer.
| pin | asserts |
| --- | --- |
| one refusal per leg (5 cases) | sharing `deny`, a sharing non-verdict,
master-detail `deny`, master-detail `unresolvable`, degraded mode (no
sharing service, unreadable parent): 403 `ATTACHMENT_PARENT_ACCESS`,
then `tombstoned`, `status: 'deleted'`, `deleted_at` set, 0 join rows |
| the sweep reclaims it | the real `LifecycleService.sweep` over the
real `SystemFile` schema (its declared `ttl`), clock at +31 days, real
`createSysFileReapGuard`: row reaped, one byte delete of its key; an
attached control file survives |
| default door, no transaction | `beginTransaction` not called; the
tombstone lands |
| caller transaction rolls back | a caller write in the same unit of
work is gone (the rollback is real); the tombstone survives it; no warn
|
| control: admitted attach | `committed`, 1 join row, no run scheduled |
| control: `ref_*` lineage | `kept: still held (field-owner)`,
`committed` |
| control: field scope, not uploader, held elsewhere, pending, unknown
id | each kept with its reason; nothing written |
| retry | the two answers above |
| refusal unchanged | identical envelope for own, another user's and
unknown file; no file id in it; only the caller's own file moved |
Each ablation ran through `scripts/ablation-replace.mjs` (WRAP mode, the
anchor must hit, the mutation is proven on disk), and the restore was
proven blob-equal to HEAD with an empty `git diff HEAD`:
| ablation | expected | observed |
| --- | --- | --- |
| A1: drop the gate's call to the tombstoner | every pin that waits on a
verdict goes red; the admitted control stays green | 16 failed, 1 passed
(the admitted control) |
| A2: drop the snapshot (run in the refusal's own context) | only the
caller-transaction pin goes red | 1 failed: `no run verdict for sys_file
f1; warn lines: […failed to tombstone sys_file f1 after a refused attach
(The database refused to run this query for object 'sys_file' …)]`. The
run inherited the caller's closed transaction. |
| A3: drop the uploader condition | the not-uploader control and the
no-leak pin go red | 2 failed |
| A4: holder check by join rows only | the `ref_*` control goes red | 1
failed |
## Verification
All on HEAD `0862db087` (the branch with `origin/main` `faf634850`
merged), after rebuilding the `@objectstack/service-storage...` closure:
- `pnpm --filter @objectstack/service-storage test`: 48 files, 821 tests
passed.
- `pnpm --filter @objectstack/service-storage typecheck`: exit 0,
including `check:test-typecheck` (the new pin file compiles under
`tsconfig.test.json`).
- Gates: `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 67 commands. All 67 ran with exit 0,
and `--ran` reconciled `67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN`, a
zero derived from the recorded exit codes. Also run: `pnpm
check:durability-log-level` and `pnpm check:startup-registry-verdict`,
both exit 0.
- An earlier run of the same families went red on
`check:query-options-erasure`: the pin file had 3 `as any` engine-option
erasures (test surface 236 → 239). They are typed now, and the gate
holds at 236.
- Wording round, HEAD `714081a66`: the changeset text only, with no
source or test change. All 67 derived families re-ran. 65 exited 0,
including every changeset gate (`check-adr-0087-registration`,
`check-changeset-no-major`, `check-empty-changeset`,
`check:changeset-gate-self-tests`, `check:pm-changeset-deadline-census`)
and `check-issue-citations` (the new `#22547` reference resolves).
`check:dual-build-cjs-loads` and `check:i18n` answered PREREQUISITE NOT
MET (exit 3), so they are not measured on this head: the recreated
worktree has only the `@objectstack/service-storage...` closure built.
Both were green on `0862db087`, and neither reads a changeset.
- Lint, a declared narrowing: `pnpm exec eslint --no-inline-config
--format json` over the 3 changed TS files reported 3 files, 0 errors, 0
warnings. `eslint.config.mjs` sets no type-aware option (0 hits for
`projectService` or `project:`), so this diff cannot move another file's
lint verdict. The full `pnpm lint` is CI's.
## Acceptance notes
- **Mechanism assumption 1** holds on the four gate legs measured on
`main`, and the degraded leg is pinned after the change. The
create-grant leg (plugin-security's object CRUD check) is different. It
refuses inside plugin-security's data middleware before `next()`, so the
storage gate never runs and the file stays `committed`. A model in the
same rig showed this: an outer middleware refusing first, the storage
gate asked 0 times, 0 run verdicts, the file `committed`. No
service-storage seam inside this card's file surface sees that refusal.
See the out-of-lane findings; it is tracked in #22547.
- **Mechanism assumption 2** is half false, as measured above.
**Assumption 3** holds: one `writeTombstone` and one
`isLiveAttachmentsFile` now serve both triggers. **Assumption 4** holds:
everything is read under system context, nothing reaches the caller, and
the refusal never waits on it.
- **The update verb, an observation:** a refused `sys_attachment` update
whose payload re-points `file_id` at a fresh upload would leave that
upload committed and unheld the same way. No producer writes such an
update today (the console never updates `file_id`), so it is not covered
or filed here. Carrier: none.
- **The last-join-row path, an observation:** `tombstoneOrphanedFiles`
asks only join rows, not the `ref_*` limb. That is benign: the sweep's
`findFileHolder` re-check un-tombstones a field-owned file, and
downloads and hydration ask the same question. It is unchanged here.
Carrier: none.
- **A refusal later in the same insert:** a refusal after the storage
gate admits the insert (validation, a write-image check that runs after
`beforeInsert`, a driver fault) is not this gate's refusal and is not
covered. It belongs to the same family as the findings below.
## Out-of-lane findings (for the seat to file)
The seat filed this family as #22547 (`Blocked-by: #22466`). How to
cover it is triage's call on that card; nothing about it rides this PR.
- **One family: attach refusals decided outside the storage gate leave
the same orphan.** Filed as #22547. The create-grant refusal
(plugin-security CRUD check, 403 `PERMISSION_DENIED`, raised in the
middleware registered in its `start()`, before `next()`) and the
`enable.files` refusal (plugin-audit `enforceFilesCapability`, 403
`FILES_DISABLED`) both refuse an attach without the storage gate
refusing. The uploaded file stays committed and unheld.
- Reach: the card's own measured producer. hotcrm#2029's run on 17.7.0
used a read-only `sys_attachment` grant: the console upload, then the
final attach answered 403, leaving a committed `sys_file` with no
attachment. Today the console hides Upload without the create grant, so
the remaining reach is a grant that changes between render and click.
- Covering it needs a seam that sees every refusal of a `sys_attachment`
insert. That is either an outermost storage middleware (only an
`init()`-phase registration would precede plugin-security's
`start()`-time one, and it would be the first such registration in the
repo), or the refusing plugins, which are outside this card's file
surface.
- Dedupe words: attach create grant refusal orphan sys_file ·
PERMISSION_DENIED sys_attachment insert tombstone · FILES_DISABLED
refused attach orphan · refusal outside storage gate never reaches
tombstone
---
_Generated by [Claude
Code](https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent ce78ff7 commit f782f17
4 files changed
Lines changed: 606 additions & 13 deletions
File tree
- .changeset
- packages/services/service-storage/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 | + | |
Lines changed: 25 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
10 | | - | |
11 | | - | |
12 | | - | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
13 | 14 | | |
14 | 15 | | |
15 | 16 | | |
| |||
26 | 27 | | |
27 | 28 | | |
28 | 29 | | |
29 | | - | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
30 | 33 | | |
31 | 34 | | |
32 | 35 | | |
| |||
371 | 374 | | |
372 | 375 | | |
373 | 376 | | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
374 | 386 | | |
375 | 387 | | |
376 | 388 | | |
| |||
573 | 585 | | |
574 | 586 | | |
575 | 587 | | |
| 588 | + | |
| 589 | + | |
| 590 | + | |
| 591 | + | |
| 592 | + | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
576 | 596 | | |
577 | 597 | | |
578 | 598 | | |
| |||
Lines changed: 169 additions & 8 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
3 | 4 | | |
| 5 | + | |
| 6 | + | |
4 | 7 | | |
5 | 8 | | |
6 | 9 | | |
| |||
15 | 18 | | |
16 | 19 | | |
17 | 20 | | |
18 | | - | |
19 | | - | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
20 | 25 | | |
21 | 26 | | |
22 | 27 | | |
| |||
87 | 92 | | |
88 | 93 | | |
89 | 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 | + | |
90 | 121 | | |
91 | 122 | | |
92 | 123 | | |
| |||
110 | 141 | | |
111 | 142 | | |
112 | 143 | | |
113 | | - | |
114 | | - | |
115 | | - | |
116 | | - | |
117 | | - | |
118 | | - | |
| 144 | + | |
| 145 | + | |
119 | 146 | | |
120 | 147 | | |
121 | 148 | | |
| |||
413 | 440 | | |
414 | 441 | | |
415 | 442 | | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
416 | 577 | | |
417 | 578 | | |
418 | 579 | | |
| |||
0 commit comments