Skip to content

Commit 1bb1da1

Browse files
feat(objectql,service-storage): a refused sys_file reader sees a field-owned file's metadata by the download door's verdict (#22637) (#22697)
Fixes #22637 Clause-②: yes ## What changes A reader who may read a record but not `sys_file` (the default: `member_default` grants no `sys_file` read) used to get every file field back as `{ id, metadataRefused: true }` (PR #22620). The download door judges the same file by the record that owns it (ADR-0104 D3) and served that reader the bytes. So the reader could download the file but could not see its name. This executes the maintainer's ruling B on #22624, verbatim as the card records it: > A file field's metadata follows the parent-derived verdict the download door already applies (ADR-0104 D3, `buildFileReadAuthorizer`): when the caller's `sys_file` read is refused, `resolveFileReferences` still hydrates a file whose `ref_object` / `ref_id` name the record being read (the delegate consulted where the owner object declares a `fileAccessDelegate`); a referenced id that does not point at that record keeps PR #22620's refused marker; direct `sys_file` queries and `public_read` are unchanged. - **From.** A reader with read on `contract` and no `sys_file` read gets `{ "signed_pdf": { "id": "f_7cQx2Lm90a", "metadataRefused": true } }`. - **To.** For a file whose `ref_object` / `ref_id` name the record being read, that reader gets `{ id, name, size, mimeType, url }`, exactly what a `sys_file` reader gets, when the door's field-owned verdict allows. A delegate-governed object gets the delegate's answer. - **Unchanged.** - Every other id keeps the marker: an id copied in from another record, an attachment-only or unclaimed file, a `public_read` file the record does not own, and an owned file the verdict refuses. - `sys_file`'s own read rule: a direct `sys_file` query by the same reader is still `PERMISSION_DENIED` (403). - `public_read` handling. - A `sys_file` reader's hydration. - The download door's verdict for every request. - An engine with no verdict registered. ## One verdict, two callers `ObjectQL` holds no service registry (no `getService` or kernel reference in `engine.ts`), so the engine cannot reach a declared `fileAccessDelegate` itself. Copying "delegate declared, so ask it and fail closed; otherwise read the record" into the engine would be the second predicate the ruling forbids. The verdict stays where it is and is handed over, the same way `registerHeldFileResolver` already hands over the tombstone holder question: - **`service-storage`** (`storage-service-plugin.ts`) - The field-owned arm of `buildFileReadAuthorizer` (the `:1237`-`:1265` range the ruling cites) moves, unchanged, into one module-private batched function, `readableFieldOwners(registry, engine, ownerObject, ownerIds, authz)`. It is not exported from the package. - The door calls it with its one owner id. The query is byte-identical (`where: { id }`, `fields: ['id']`, `limit: 1`, any row allows), and so is the fail-closed handling of the delegate. - `start()` registers the same function on the engine, beside `registerHeldFileResolver`, through the new duck-typed seam `registerFieldOwnedFileReadAuthorizer`. - **`objectql`** (`engine.ts`) - The new seam is `registerFieldOwnedFileReadAuthorizer` (type `FieldOwnedFileReadAuthorizer`, exported). - On the refused arm of `resolveFileReferences`, `readParentDerivedFiles` makes two reads. 1. One `sys_file` read in a `sudo()`-shaped context (`{ ...caller, isSystem: true }`, so the tenant wall and the transaction ride along). Its `where` asks for the ownership pair itself (`id $in`, `ref_object` this object, `ref_id $in` this result's record ids) and projects only seven columns. 2. One call to the registered verdict with every owner id, under the caller's envelope minus its operation-private `__` keys (`withoutOperationPrivateKeys`). - Allowed rows go through the same servability rule as the normal path (`servableFileRows`, extracted, shared) and render through one `toFileValue`. - Judged per element and per record: an id hydrates only on the record whose field owns it. - Unwired, a projection without `id`, or any failure: every id keeps the marker. A failure logs one `warn`. **Is the ownership match a second predicate?** No, by the ruling's own words. "Whose `ref_object` / `ref_id` name the record being read" is the ruling's scope condition, not an access verdict. The verdict is reached without a copy: the door's own function decides allow or deny, including the delegate. The system read is the posture the door already reads a file in (by id, system context, never the caller's `sys_file` grant). It serves only ids a record the caller just read holds. The read is deliberately narrower than the door, by the ruling. The door also serves a file's uploader and any file reachable through a `sys_attachment` parent. Hydration serves only a file owned by the record being read. ## Measured on `main` first The test was a scratch file in `plugin-security`, never committed and deleted after the run. It used real `ObjectQL` (source), the real `SecurityPlugin`, the real `StorageServicePlugin` and `SqlDriver`. The reader had read on the record's object and no `sys_file` read. Two committed files were seeded: one field-owned by record `d1`, and one owned by `d9` and copied into `d2`. - With `engine.ts` and `storage-service-plugin.ts` at `243dd3c625` (blobs `3177422431` and `0246a5bfe4`): `d1` read `{ id, metadataRefused: true }`, `d2` read `{ id, metadataRefused: true }`, and a direct `sys_file` query got `PERMISSION_DENIED` / 403. The premise holds on `main`. - At this branch: `d1` read `{ id, name: 'contract.pdf', size: 1024, mimeType: 'application/pdf', url }`, `d2` still read `{ id, metadataRefused: true }`, and the direct query still got `PERMISSION_DENIED` / 403. - The two files were restored from `HEAD`: blob equals `HEAD`, `git diff HEAD` is empty, and the porcelain status is clean. ## Pins - `packages/services/service-storage/src/field-owned-file-hydration-door-agreement.test.ts` (new). Both surfaces are asked about one fixture: the door is `GET /files/:fileId/url` as the reader, and the record read runs as the same reader. The fixture is one real `ObjectQL` + `SqlDriver`, with `StorageServicePlugin` booted and wired as it ships. - A field-owned file on the record read: the door returns 200 and the read hydrates in full. - A `fileAccessDelegate` that refuses: the door returns 403 `FILE_DOWNLOAD_DENIED` and the read keeps the marker. A delegate that allows: the door returns 200 and the read hydrates, and the delegate was asked by the reader for the owning record. Both decisions are readable by a raw record read, so the delegate is what decides. - An id copied out of a record the reader cannot read: refused on both surfaces. - The ruling's boundary: an id copied from a record the reader can read hydrates on its owner and keeps the marker where it was copied. An attachment-only file keeps the marker. - `public_read` is unchanged: anonymous door 200, and the refused reader's read keeps the marker. - A direct `sys_file` query is still refused. A multi-value field is judged per element. - Stand-ins: the `sys_file` refusal (middleware throwing `PERMISSION_DENIED`, the shape `plugin-security`'s `file-field-hydration-refused.test.ts` measures on the real layer), the permission store (unregistered objects read empty), and authentication by header. - `packages/objectql/src/engine-file-hydrate-parent-derived.test.ts` (new) covers the engine's half with a recorded stand-in verdict: - per-record pairing, mixed arrays, another object's same id, and a refusing verdict; - the verdict is asked once, with owner ids only and no `__` keys, and exactly one system `sys_file` read is made, with the ownership pair in its `where`; - an owned tombstone (held) and a pending upload, each read as a `sys_file` reader reads it, and a projection without `id`; - unwired, a failing system read and a throwing verdict (marker plus one `warn`); - controls: a `sys_file` reader, a direct `sys_file` query, an empty field. - Unedited and green: the door's existing tests (`mount-storage-routes`, `storage-routes`, `file-read-tenancy-posture-admission`, `storage-routes.authz-outage-relay` and the rest of `service-storage`), plus #22620's `engine-file-hydrate-refused.test.ts` and `plugin-security/src/file-field-hydration-refused.test.ts`. ## Ablation `node scripts/ablation-replace.mjs` (WRAP mode) replaced the anchor `const owned = await this.readParentDerivedFiles(objectName, records, uniqueIds, execCtx);` with the same call over `records.slice(0, 0)` plus a `debug` marker line. That removes the parent-derived arm. The anchor hit once (1 to 0), and the blob changed from `6a9455a5d3` to `bb576869bc`. - **objectql** (the test imports `./engine`, so it reads source): 10 failed and 6 passed. The first pin received `{ id: 'f_owned_1aa', metadataRefused: true }`. The failing-read and throwing-verdict cases went red too: with the arm gone, nothing is read, so their `warn` never comes. - **service-storage** (`@objectstack/objectql` resolves to `dist/`, a `KNOWN_UNALIASED_TEST_IMPORTS` pair): `objectql` was rebuilt with the mutation, and `ablation-dist-preflight` found the marker in 4 built files. Result: 4 failed and 5 passed. The field-owned pin received `{ id, metadataRefused: true }`, and the delegate-allow, owner-side-of-a-copy and multi-value pins failed too. The refusal-side pins stayed green, as they should. - **Restore.** The blob equals `HEAD` (`6a9455a5d3`), `git diff HEAD` is empty, and the porcelain status is clean. After a rebuild, `ablation-dist-preflight --absent` passed (marker absent from all 14 built files, tree clean), and both files went green again: 16 of 16 and 9 of 9. The ablation ran at `e6e7fbec66`. The merge of `origin/main` that followed did not change `engine.ts`, the `service-storage` package or either new test (the `git diff --stat` is empty). ## Verification at `9931bb55d3` (head, `origin/main` merged) - `pnpm --filter @objectstack/objectql exec vitest run --project local --maxWorkers=2`: 394 files, 7753 tests passed. `--project repo`: 1 file, 5 tests passed. - `pnpm --filter @objectstack/service-storage exec vitest run --maxWorkers=2`: 51 files, 846 tests passed. - `pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2 src/file-field-hydration-refused.test.ts`: 4 passed. - `typecheck`: `@objectstack/objectql` passed (exit 0; test layer 40 files / 234 errors / 65 pinned signatures, unchanged), and `@objectstack/service-storage` passed (exit 0; test layer clean). - Gates: `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` (no paths) derives 102 commands. All 102 ran after a full `turbo run build`, each with its exit code captured before any pipe, and all exited 0. `--ran` reconciles them as 102 derived, 102 run, 0 NOT MEASURED. Two red results on the first pass were real and are fixed: `check:query-options-erasure` (my tests erased query options, 236 to 240, now 236) and `check:slot-lookup` (a `getService` typed `any` in my test). - Lint, a narrowed run: `eslint --no-inline-config --format json` over the 5 changed `.ts` files reports 5 files, 0 errors and 0 warnings. - Population: `eslint.config.mjs`, the config `pnpm lint` (`eslint .`) runs. - Invariance: the config enables no type-aware linting (no `parserOptions.project`), so this diff cannot move any untouched file's verdict. ## Cross-lane paths Declared before any edit: - **`domain:services`**, on #6021 (comment 6097907456): - `packages/services/service-storage/src/storage-service-plugin.ts`: the extraction and the engine registration; - `packages/services/service-storage/src/field-owned-file-hydration-door-agreement.test.ts`: new. - **`domain:devx`**, on #6023 (comment 6098008938), prose only: - `content/docs/protocol/objectql/types.mdx`: the `file` read forms; - `content/docs/permissions/attachments-access.mdx`: the refused-reader clause. No generated page and no `content/docs/releases/` path. ## Acceptance notes - **`FileRefusedValueSchema`'s TSDoc** (`packages/spec`, "when the reader is refused the `sys_file` read that resolves it") is left as it is. It is now a slight overstatement: the marker means the read was refused and the parent-derived verdict did not allow it. Changing it would pull in `gen:schema` / `gen:docs`. The same holds for `fileAccessDelegate`'s TSDoc and `.describe()` ("authorizes downloads"). The delegate is now also consulted for a refused reader's metadata. Neither is false; both are incomplete. - **Narrower than the door.** An uploader without `sys_file` read, reading a delegate-governed record whose delegate refuses them, sees the marker although the door serves them the bytes (the door's owner arm). The ruling cites the field-owned arm only. - **A projection that leaves out `id`** cannot name its owner record, so every id keeps the marker. Not measured: whether any shipped caller projects file fields without `id`. - **Cost.** A refused hydration with owned ids makes one more system `sys_file` read and one verdict call: one batched owner read, or one delegate call per owner. The non-delegate arm re-reads records the caller has just read. That is redundant by construction (the ruling's own reduction), and it is the price of one predicate. - **No committed pin drives the full composition** (real `SecurityPlugin` + `StorageServicePlugin` + REST). The scratch measurement above did, uncommitted. A dogfood pin would live in `packages/qa` (`domain:cli`) and is not in this PR. Carrier: none. --- _Generated by [Claude Code](https://claude.ai/code/session_01Bw3y2DWhT9RPnrmDsNqEVG)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cb3bb93 commit 1bb1da1

8 files changed

Lines changed: 1251 additions & 65 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/objectql': minor
3+
'@objectstack/service-storage': minor
4+
---
5+
6+
A reader refused `sys_file` read now sees the name, size and type of a file the record they are reading owns, whenever the download door would serve them its bytes
7+
8+
Clause-②: yes
9+
10+
The download door already judges a field-owned file by the record that owns it (ADR-0104 D3): a declared `fileAccessDelegate` decides where the owner object names one, otherwise the caller's read of that record. Record file-field hydration judged the same file by `sys_file` object read instead, which the default member grants do not include. So one reader could download a contract's signed PDF but saw the field as `{ id, metadataRefused: true }`: the bytes yes, the name no.
11+
12+
- **From.** A reader with read on `contract` and no `sys_file` read gets `{ "signed_pdf": { "id": "f_7cQx2Lm90a", "metadataRefused": true } }` for every file id the record holds.
13+
- **To.** For a file whose `ref_object` / `ref_id` name the record being read, that reader gets the hydrated `{ "id": "f_7cQx2Lm90a", "name": "signed.pdf", "size": 2048, "mimeType": "application/pdf", "url": "/api/v1/storage/files/f_7cQx2Lm90a" }`, exactly as a `sys_file` reader does, when the door's field-owned verdict allows it. A delegate-governed object (for example `sys_approval_action`, delegate `approvals`) gets its delegate's answer, not the raw record read.
14+
- **Unchanged.** Every other id keeps `{ id, metadataRefused: true }`: an id copied in from another record, an attachment-only or unclaimed file, a `public_read` file the record does not own, and an owned file the delegate or the record read refuses. `sys_file`'s own read rule is unchanged, so a direct `sys_file` query by the same reader is still refused. A reader with `sys_file` read is served by their own read, as before. The download door's verdict for every request is unchanged.
15+
16+
**One verdict, two callers.** `@objectstack/service-storage` moves the download door's field-owned arm into one module-private function (not exported) and hands it to the engine through a new seam, `ObjectQL.registerFieldOwnedFileReadAuthorizer` (type `FieldOwnedFileReadAuthorizer`, exported from `@objectstack/objectql`), the same handover `registerHeldFileResolver` already makes. The engine never re-derives it. An engine with nothing registered keeps the refused marker for every id. On a refused hydration the engine makes one more `sys_file` read under the caller's tenant (it reads only ids whose ownership pair names a record in the result) and asks the verdict once per read.
17+
18+
**For consumers.** Nothing to change. A refused reader may now receive the full file object where it used to receive the marker, and code that already handles all three `expanded` read forms handles this.

‎content/docs/permissions/attachments-access.mdx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,10 @@ can be shared across records). Reclamation is handled by the platform LifecycleS
167167
sweep tidies it, and a file with no holder left still answers
168168
`FILE_NOT_FOUND` (404) and keeps its bare id in a record payload — except to
169169
a reader refused `sys_file` read, who gets `{ id, metadataRefused: true }` for
170-
every file id, held or not ([`file` read forms](/docs/protocol/objectql/types#file)).
170+
every file id the record being read does not own, or that the download door's
171+
field-owned verdict refuses them, held or not; a file the record owns and the
172+
door would serve them reads exactly as it does for a `sys_file` reader
173+
([`file` read forms](/docs/protocol/objectql/types#file)).
171174
- **`sys_upload_session`** — abandoned/terminal chunked-upload sessions are
172175
reaped, and a reap guard aborts the underlying backend multipart upload
173176
(S3 `AbortMultipartUpload` / local parts dir) first, so already-uploaded

‎content/docs/protocol/objectql/types.mdx‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,7 +1163,11 @@ with `multiple: true`) into one of **three** forms, and none of them fails the
11631163
record read that asked for it.
11641164

11651165
**The file object** — the caller's `sys_file` read found the file (a committed
1166-
row, or a tombstone something still holds). `url` is its only required member,
1166+
row, or a tombstone something still holds). A caller refused that read gets it
1167+
too for a file the record being read owns (its `ref_object` / `ref_id` name that
1168+
record) whenever the download door would serve them its bytes: the same
1169+
field-owned verdict — the object's declared `fileAccessDelegate`, or else the
1170+
caller's read of that record — decides both. `url` is its only required member,
11671171
derived from the id rather than stored:
11681172

11691173
```json
@@ -1185,9 +1189,14 @@ provisioned yet, or the read failed for any reason other than a refusal (an
11851189
outage, which the engine logs as a warning).
11861190

11871191
**`{ "id": "abc123", "metadataRefused": true }`** — the caller may read the record
1188-
but is refused the `sys_file` read. The value is the id the record holds plus the
1189-
marker, nothing else: no `url`, `name`, `size` or `mimeType`, because none was
1190-
read, and it says nothing about whether the file exists. It is distinct from an
1192+
but is refused the `sys_file` read, and the file is not one the record being read
1193+
owns that the download door's verdict lets them read: an id copied in from
1194+
another record, an attachment-only or unclaimed file (`acl: 'public_read'`
1195+
included), an owned file whose delegate or record read refuses them, or an owned
1196+
file whose verdict could not be asked (an outage, which the engine logs as a
1197+
warning). The value is the id the record holds plus the
1198+
marker, nothing else: no `url`, `name`, `size` or `mimeType`, because none is
1199+
served to this caller, and it says nothing about whether the file exists. It is distinct from an
11911200
empty field, which holds no file at all, and from the file object, which always
11921201
carries `url`.
11931202
[`FileRefusedValueSchema`](/docs/references/data/field-value#filerefusedvalue)

0 commit comments

Comments
 (0)