Repository navigation
Commit 1bb1da1
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
- .changeset
- content/docs
- permissions
- protocol/objectql
- packages
- objectql/src
- 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 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
167 | 167 | | |
168 | 168 | | |
169 | 169 | | |
170 | | - | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
171 | 174 | | |
172 | 175 | | |
173 | 176 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1163 | 1163 | | |
1164 | 1164 | | |
1165 | 1165 | | |
1166 | | - | |
| 1166 | + | |
| 1167 | + | |
| 1168 | + | |
| 1169 | + | |
| 1170 | + | |
1167 | 1171 | | |
1168 | 1172 | | |
1169 | 1173 | | |
| |||
1185 | 1189 | | |
1186 | 1190 | | |
1187 | 1191 | | |
1188 | | - | |
1189 | | - | |
1190 | | - | |
| 1192 | + | |
| 1193 | + | |
| 1194 | + | |
| 1195 | + | |
| 1196 | + | |
| 1197 | + | |
| 1198 | + | |
| 1199 | + | |
1191 | 1200 | | |
1192 | 1201 | | |
1193 | 1202 | | |
| |||
0 commit comments