|
| 1 | +--- |
| 2 | +"@objectstack/service-storage": patch |
| 3 | +--- |
| 4 | + |
| 5 | +fix(service-storage): stamp the acting organization on the last two `sys_file` insert doors (#13547) |
| 6 | + |
| 7 | +`sys_file` declares no `tenancy` key, so `isTenancyDisabled()` reads `false` |
| 8 | +and the registry provisions `organization_id` on it. Four doors on the object |
| 9 | +had been given the acting organization one card at a time — `createFile` |
| 10 | +(#12745), `createSession` (#12928), and the `update`/`delete` halves (#13178) — |
| 11 | +and all four run through `StorageMetadataStore`, which threads a |
| 12 | +`StorageWriteContext` into `context.tenantId` so the platform's insert-side |
| 13 | +chokepoint can stamp the column. |
| 14 | + |
| 15 | +Two doors bypassed that store entirely and carried no organization at all: |
| 16 | + |
| 17 | +- `copyOwnedFile` (`file-reference-lifecycle.ts`) — the copy-on-claim |
| 18 | + lifecycle hook, which inserts a fresh `sys_file` whenever a record writes an |
| 19 | + id already owned by another field slot; |
| 20 | +- `materializeDataUri` (`backfill-file-references.ts`) — the operator backfill |
| 21 | + pass, which inserts one `sys_file` per inline `data:` URI it converts. |
| 22 | + |
| 23 | +Both passed `{ isSystem: true, [RAW_FILE_VALUES_CONTEXT_KEY]: true }`, so |
| 24 | +`buildDriverOptions` emitted no `DriverOptions.tenantId`, |
| 25 | +`SqlDriver.injectTenantOnInsert` had nothing to stamp from, and every row |
| 26 | +landed `organization_id = NULL`. The driver's tenant term is |
| 27 | +`(organization_id = :tenantId OR organization_id IS NULL)`, so those rows were |
| 28 | +reachable from **every** organization — including through the very update and |
| 29 | +delete doors #13178 had just scoped. |
| 30 | + |
| 31 | +⚠️ Nothing warned, and the silence was explained rather than reassuring: |
| 32 | +`isSystem` also sets `bypassTenantAudit = true`, which is exactly the guard |
| 33 | +`auditMissingTenant` returns at — so the `[tenant-audit]` line naming this |
| 34 | +defect ("writes will not be tenant-isolated") never fired for either door. |
| 35 | + |
| 36 | +Each door now threads the organization the platform can actually justify, as |
| 37 | +an execution context — ⛔ never as a column on the payload, so |
| 38 | +`resolveTenantField` / `injectTenantOnInsert` keep deciding whether the object |
| 39 | +has a tenant column and whether an explicit value wins: |
| 40 | + |
| 41 | +- the **copy** takes the organization of the write that triggered it, read |
| 42 | + from `HookContext.session.organizationId` (which ObjectQL's `buildSession()` |
| 43 | + copies verbatim from `ExecutionContext.tenantId`); |
| 44 | +- the **backfill** takes the organization of the record whose field held the |
| 45 | + bytes, resolved with the same `createWallOrganizationResolver` the `sys_file` |
| 46 | + organization sweep uses, so an object declaring `tenancy.tenantField` is read |
| 47 | + by the column it is really walled by. |
| 48 | + |
| 49 | +Both stamp exactly what that sweep would independently derive from the new |
| 50 | +file's field-reference holder, so the forward and repair halves agree by |
| 51 | +construction. The backfill needs **no** operator-supplied organization and |
| 52 | +deliberately takes none: one run spans every object and organization in the |
| 53 | +deployment, so a single supplied value would be stamped onto other tenants' |
| 54 | +files — and a wrongly-stamped row is walled into somebody else's tenant, which |
| 55 | +is strictly worse than a NULL row that stays reachable. |
| 56 | + |
| 57 | +Where no organization is in scope — a caller with no active org, an unwalled |
| 58 | +object, a legacy row that carries none — the `tenantId` key is omitted |
| 59 | +entirely and the write proceeds exactly as before. ⛔ Forward-stamping only: |
| 60 | +no existing `sys_file` row's organization is written by either door. |
0 commit comments