From 8c3fdf30044d844cad7a37b880c02baf75be679e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 21:02:58 +0000 Subject: [PATCH 1/3] fix(objectql): a file field's read verdict no longer depends on whether the projection lists id A reader refused sys_file read is served a file field by the parent-derived verdict, matched on the owning record's id. find/findOne now read id for any projection that names a file field and cut it back off the answer with the formula widening, so select= answers what select=id, does, and a refused reader stays refused. Claude-Session: https://claude.ai/code/session_01FEkQ5nso8iiU7cfQSEAkX5 Co-authored-by: Claude --- ...engine-file-hydrate-parent-derived.test.ts | 29 ++- packages/objectql/src/engine.ts | 51 +++- ...a-file-field-projection-without-id.test.ts | 223 ++++++++++++++++++ ...wned-file-hydration-door-agreement.test.ts | 12 + 4 files changed, 304 insertions(+), 11 deletions(-) create mode 100644 packages/rest/src/data-file-field-projection-without-id.test.ts diff --git a/packages/objectql/src/engine-file-hydrate-parent-derived.test.ts b/packages/objectql/src/engine-file-hydrate-parent-derived.test.ts index 08f20c2115a..ad74221df49 100644 --- a/packages/objectql/src/engine-file-hydrate-parent-derived.test.ts +++ b/packages/objectql/src/engine-file-hydrate-parent-derived.test.ts @@ -384,14 +384,35 @@ describe('a refused sys_file read still hydrates a file the record being read ow expect(rows.d5.attachment).toEqual(asFileReader.d5.attachment); }); - it('a read whose projection leaves out id cannot name its owner record, so every id keeps the marker', async () => { + it('[#26672] a read whose projection leaves out id still names its owner record — and answers no id it did not name', async () => { const { engine, calls } = await boot(); const rows = await engine.find('doc', { fields: ['attachment'], context: READER_CTX }); + const withId = byId(await engine.find('doc', { fields: ['id', 'attachment'], context: READER_CTX })); + + // The same verdict as the projection that names `id`: d1 owns OWNED and + // hydrates it, d3 holds a copy of it and keeps the marker. + expect(rows).toContainEqual({ attachment: hydrated(OWNED, 'owned.pdf') }); + expect(rows).toContainEqual({ attachment: refused(OWNED) }); + expect(withId.d1.attachment).toEqual(hydrated(OWNED, 'owned.pdf')); + expect(withId.d3.attachment).toEqual(refused(OWNED)); + for (const row of rows) expect(Object.keys(row)).toEqual(['attachment']); + expect(calls[0]?.ownerIds).toContain('d1'); + + const one = await engine.findOne('doc', { where: { id: 'd1' }, fields: ['attachment'], context: READER_CTX }); + expect(one).toEqual({ attachment: hydrated(OWNED, 'owned.pdf') }); + }); - expect(rows.find((r: any) => r.attachment?.id === OWNED && !r.attachment.metadataRefused)).toBeUndefined(); - expect(rows.filter((r: any) => r.attachment != null)).toContainEqual({ attachment: refused(OWNED) }); - expect(calls).toEqual([]); + it('[#26672] …and a reader the verdict refuses is still refused when the projection leaves out id', async () => { + const { engine } = await boot({ allow: [] }); + + const rows = await engine.find('doc', { fields: ['attachment'], context: READER_CTX }); + expect(rows.filter((r: any) => r.attachment != null)).toEqual( + [refused(OWNED), refused(OWNED), refused(TOMBSTONE_OWNED), refused(PENDING_OWNED), refused(OTHER_OBJECT)] + .map((attachment) => ({ attachment })), + ); + const one = await engine.findOne('doc', { where: { id: 'd1' }, fields: ['attachment'], context: READER_CTX }); + expect(one).toEqual({ attachment: refused(OWNED) }); }); it('the happy path costs no warn; the one debug line counts what the verdict allowed', async () => { diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 1878bcbf651..9e058e86736 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -1796,6 +1796,8 @@ function planFormulaProjection( * on driver-sql (a projection is exactly the named columns — no `id` unless it * is named) and pinned against it in `engine-formula-projection-trim.test.ts`. * `id` is cut like any other widened column when the caller did not name it. + * [#26672] The `id` {@link withFileOwnerKey} adds for a file field's owner + * verdict is cut the same way. * * ## Where it runs, and why there * @@ -1833,6 +1835,35 @@ function rowsWithoutFormulaWidening(rows: T, widened: ReadonlySet | u return rows.map((row) => withoutFormulaWidening(row, widened)) as unknown as T; } +/** + * [#26672] A projection that names a file field reads the record's `id` too, + * and the key is cut back off the answer like the formula widening is. + * + * File-reference hydration judges a reader refused `sys_file` read by the + * record that OWNS each file (#22637, `readParentDerivedFiles`), matched on + * that record's `id`. A projection that left `id` out handed it rows with no + * key, so a file the reader may read answered `{ id, metadataRefused: true }` + * to `select=` and the full value to `select=id,`. The + * verdict must not depend on whether the caller listed `id`, so the driver is + * asked for it here and {@link withoutFormulaWidening} trims it at the + * `return`: the caller still gets exactly the columns it named, and a reader + * the verdict refuses is still refused. + * + * Touches only an explicit projection that names a file field and not `id`; + * a projection the formula planner widened already carries `id`. + */ +function withFileOwnerKey( + schema: any, + requestedFields: string[] | undefined, + planned: { projected?: string[]; widened?: string[] }, +): { projected?: string[]; widened?: string[] } { + if (!schema?.fields || !Array.isArray(requestedFields) || requestedFields.length === 0) return planned; + const projected = planned.projected ?? requestedFields; + if (projected.includes('id')) return planned; + if (!requestedFields.some((f) => FILE_REFERENCE_TYPES.has((schema.fields as any)[f]?.type))) return planned; + return { projected: [...projected, 'id'], widened: [...(planned.widened ?? []), 'id'] }; +} + /** * [#7095] ORDER BY a field whose value is computed on read — refused HERE, on * the engine's own public boundary, and no longer only at the REST ingress. @@ -12950,9 +12981,11 @@ export class ObjectQL implements IObjectQLEngine { * Returns, per allowed file id, the owning record's id and its servable row * — `null` when the row is owned and allowed but not servable, which the * caller reads as the bare id a `sys_file` reader would see. An id absent - * from the map keeps the refused marker. Unwired, no record ids (a - * projection that left out `id`), or a failure: an empty map — every id - * stays refused, the answer before this existed. + * from the map keeps the refused marker. Unwired, no record ids, or a + * failure: an empty map — every id stays refused, the answer before this + * existed. [#26672] `find` / `findOne` read `id` for any projection that + * names a file field ({@link withFileOwnerKey}), so a caller's projection + * that leaves `id` out does not land here. */ private async readParentDerivedFiles( objectName: string, @@ -13501,10 +13534,12 @@ export class ObjectQL implements IObjectQLEngine { // driver, where the #3821 ladder answered it with EVERY field. assertProjectionHasNoDottedPaths(object, 'find', _findSchema, ast.fields); const _findFormula = planFormulaProjection(_findSchema, ast.fields); - if (_findFormula.projected) ast.fields = _findFormula.projected; + // [#26672] …and a file field's owner key — see `withFileOwnerKey`. + const _findProjection = withFileOwnerKey(_findSchema, ast.fields, _findFormula); + if (_findProjection.projected) ast.fields = _findProjection.projected; // [#22300] The columns that widening added, cut back off the answer at the // `return` — see `withoutFormulaWidening`. - const _findWidened = _findFormula.widened ? new Set(_findFormula.widened) : undefined; + const _findWidened = _findProjection.widened ? new Set(_findProjection.widened) : undefined; // Drop any requested PLAIN field that doesn't exist on the schema. // Without this, drivers (notably SqlDriver) emit `SELECT unknown_col @@ -13820,9 +13855,11 @@ export class ObjectQL implements IObjectQLEngine { // [#7642] Caller's own projection, before planning rewrites it — see `find`. const _findOneRequestedFields = Array.isArray(ast.fields) ? [...ast.fields] : undefined; const _findOneFormula = planFormulaProjection(_findOneSchema, ast.fields); - if (_findOneFormula.projected) ast.fields = _findOneFormula.projected; + // [#26672] Same as `find`: a file field's owner key is read too. + const _findOneProjection = withFileOwnerKey(_findOneSchema, ast.fields, _findOneFormula); + if (_findOneProjection.projected) ast.fields = _findOneProjection.projected; // [#22300] Same as `find`: what the widening added is cut at the `return`. - const _findOneWidened = _findOneFormula.widened ? new Set(_findOneFormula.widened) : undefined; + const _findOneWidened = _findOneProjection.widened ? new Set(_findOneProjection.widened) : undefined; // Drop unknown PLAIN fields — see the equivalent block in `find()` for // the rationale, and for why this tolerance is plain-columns-only ([#7589] diff --git a/packages/rest/src/data-file-field-projection-without-id.test.ts b/packages/rest/src/data-file-field-projection-without-id.test.ts new file mode 100644 index 00000000000..f3dd375adc9 --- /dev/null +++ b/packages/rest/src/data-file-field-projection-without-id.test.ts @@ -0,0 +1,223 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#26672] A file field's read verdict does not depend on whether the caller's + * projection lists `id`. + * + * ## The defect + * + * A reader refused `sys_file` read (every member whose application grants no + * `sys_file` read) is served a file field by the parent-derived verdict of + * #22637: a file whose `ref_object` / `ref_id` name the record being read + * hydrates when the download door's field-owned verdict allows this reader. + * That verdict is judged per RECORD, by the record's `id`. A projection that + * left `id` out (`GET /data/:object?select=attachment`) handed the hydration + * rows with no `id`, so no file could be matched to its owner and every one + * answered `{ id, metadataRefused: true }` — while `select=id,attachment` + * answered the full `{ id, name, size, mimeType, url }` to the same reader. + * + * ## What is pinned, on the composed REST harness + * + * `RestServer` → `ObjectStackProtocolImplementation` → `ObjectQL` → a real + * `SqlDriver` on better-sqlite3, through the three read doors that reach the + * hydration: `GET /data/:object` (`select`), `POST /data/:object/query` + * (`fields`) and `GET /data/:object/:id` (`select`, the `findOne` door). + * + * - The permitted reader gets the full value with or without `id`, and no + * `id` key the caller did not name. + * - CONTROL the same projection with `id` answers as it did. + * - CONTROL a reader the field-owned verdict refuses is still refused, with + * or without `id`: nothing an unauthorised caller sees is widened. + * + * ## The stand-ins, and only these + * + * - The security layer's `sys_file` refusal: an engine middleware that + * throws `PERMISSION_DENIED` for every non-system `sys_file` read, the + * shape plugin-security's `file-field-hydration-refused.test.ts` measures + * on the real `SecurityPlugin`. + * - The field-owned verdict the storage plugin hands the engine: one that + * allows the reader and nobody else. The real verdict's agreement with the + * download door is pinned in service-storage's + * `field-owned-file-hydration-door-agreement.test.ts`. + * + * `@objectstack/objectql` resolves through its `exports` to `dist/` here, so + * an ablation of the engine is observed only after a rebuild of that package. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +const OBJECT = 'rq_filed_doc'; +const SYS = { isSystem: true }; +const READER = 'u_reader'; +const STRANGER = 'u_stranger'; +const FILE_ID = 'f_26672_owned'; + +const hydrated = { + id: FILE_ID, name: 'contract.pdf', size: 2048, mimeType: 'application/pdf', url: `/api/v1/storage/files/${FILE_ID}`, +}; +const refused = { id: FILE_ID, metadataRefused: true }; + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +const refusal = () => + Object.assign(new Error('You do not have permission to perform this action.'), { + name: 'PermissionDeniedError', code: 'PERMISSION_DENIED', status: 403, statusCode: 403, + }); + +const rowsOf = (res: any): Array> => res._json?.records ?? res._json?.data ?? res._json; +const recordOf = (res: any): Record => res._json?.record ?? res._json; + +describe('[#26672] a file field reads the same with or without `id` in the projection', () => { + let engine: ObjectQL; + let caller = READER; + let list: (query: Record) => Promise; + let query: (body: Record) => Promise; + let getById: (id: string, query: Record) => Promise; + + beforeAll(async () => { + engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) as any, + true, + ); + await engine.init(); + engine.registry.registerObject({ + name: OBJECT, + label: 'Filed doc', + fields: { + title: { name: 'title', type: 'text' }, + attachment: { name: 'attachment', type: 'file' }, + }, + } as any); + // The storage service's `sys_file`, as far as the hydration reads it. + engine.registry.registerObject({ + name: 'sys_file', + label: 'System File', + fields: { + key: { name: 'key', type: 'text' }, + name: { name: 'name', type: 'text' }, + mime_type: { name: 'mime_type', type: 'text' }, + size: { name: 'size', type: 'number' }, + status: { name: 'status', type: 'text' }, + ref_object: { name: 'ref_object', type: 'text' }, + ref_id: { name: 'ref_id', type: 'text' }, + ref_field: { name: 'ref_field', type: 'text' }, + }, + } as any); + await engine.syncSchemas(); + await installDefaultOrganization(engine); + + await engine.insert('sys_file', { + id: FILE_ID, key: `files/${FILE_ID}.pdf`, name: 'contract.pdf', mime_type: 'application/pdf', size: 2048, + status: 'committed', ref_object: OBJECT, ref_id: 'd1', ref_field: 'attachment', + } as any, { context: SYS } as any); + await engine.insert(OBJECT, { id: 'd1', title: 'Signed', attachment: FILE_ID } as any, { context: SYS } as any); + + engine.registerMiddleware(async (opCtx: any, next: () => Promise) => { + if (!opCtx.context?.isSystem) throw refusal(); + return next(); + }, { object: 'sys_file' }); + engine.registerFieldOwnedFileReadAuthorizer(async (_object, ownerIds, context) => + new Set(context?.userId === READER ? ownerIds : [])); + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: caller }); + rest.registerRoutes(); + const routes = rest.getRoutes(); + const route = (method: string, path: string) => { + const found = routes.find((r: any) => r.method === method && r.path === path); + expect(found).toBeDefined(); + return found!; + }; + const listRoute = route('GET', '/api/v1/data/:object'); + const queryRoute = route('POST', '/api/v1/data/:object/query'); + const getRoute = route('GET', '/api/v1/data/:object/:id'); + list = async (q) => { + const res = makeRes(); + await listRoute.handler({ params: { object: OBJECT }, query: q } as any, res); + return res; + }; + query = async (body) => { + const res = makeRes(); + await queryRoute.handler({ params: { object: OBJECT }, body } as any, res); + return res; + }; + getById = async (id, q) => { + const res = makeRes(); + await getRoute.handler({ params: { object: OBJECT, id }, query: q } as any, res); + return res; + }; + }); + + afterAll(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + }); + + const as = (user: string) => { caller = user; }; + + it('the refusal it starts from: the reader may not read sys_file directly', async () => { + const direct = await engine.find('sys_file', { context: { userId: READER } } as any).then(() => null, (e: any) => e); + expect({ code: direct?.code, status: direct?.statusCode ?? direct?.status }).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); + }); + + it('GET /data/:object?select=: a permitted reader gets the full value, and no `id` it did not name', async () => { + as(READER); + const res = await list({ select: 'attachment' }); + expect(res._status ?? 200).toBe(200); + expect(rowsOf(res)).toEqual([{ attachment: hydrated }]); + }); + + it('POST /data/:object/query with fields: []: the same', async () => { + as(READER); + const res = await query({ fields: ['attachment'] }); + expect(res._status ?? 200).toBe(200); + expect(rowsOf(res)).toEqual([{ attachment: hydrated }]); + }); + + it('GET /data/:object/:id?select= (the findOne door): the same', async () => { + as(READER); + const res = await getById('d1', { select: 'attachment' }); + expect(res._status ?? 200).toBe(200); + const record = recordOf(res); + expect(record.attachment).toEqual(hydrated); + expect(record).not.toHaveProperty('id'); + }); + + it('CONTROL select=id, answers as it did', async () => { + as(READER); + expect(rowsOf(await list({ select: 'id,attachment' }))).toEqual([{ id: 'd1', attachment: hydrated }]); + expect(rowsOf(await query({ fields: ['id', 'attachment'] }))).toEqual([{ id: 'd1', attachment: hydrated }]); + expect(recordOf(await getById('d1', { select: 'id,attachment' }))).toMatchObject({ id: 'd1', attachment: hydrated }); + }); + + it('CONTROL a reader the field-owned verdict refuses is still refused, with or without `id`', async () => { + as(STRANGER); + expect(rowsOf(await list({ select: 'attachment' }))).toEqual([{ attachment: refused }]); + expect(rowsOf(await list({ select: 'id,attachment' }))).toEqual([{ id: 'd1', attachment: refused }]); + expect(rowsOf(await query({ fields: ['attachment'] }))).toEqual([{ attachment: refused }]); + expect(rowsOf(await query({ fields: ['id', 'attachment'] }))).toEqual([{ id: 'd1', attachment: refused }]); + const byId = recordOf(await getById('d1', { select: 'attachment' })); + expect(byId.attachment).toEqual(refused); + expect(byId).not.toHaveProperty('id'); + expect(recordOf(await getById('d1', { select: 'id,attachment' })).attachment).toEqual(refused); + }); +}); diff --git a/packages/services/service-storage/src/field-owned-file-hydration-door-agreement.test.ts b/packages/services/service-storage/src/field-owned-file-hydration-door-agreement.test.ts index 34a2dc3f55c..28c5d246203 100644 --- a/packages/services/service-storage/src/field-owned-file-hydration-door-agreement.test.ts +++ b/packages/services/service-storage/src/field-owned-file-hydration-door-agreement.test.ts @@ -360,6 +360,18 @@ describe('#22637 — a file field\'s metadata and the download door agree, for a expect((await readAs('contract', 'c_open')).annexes[4]).toEqual(refused(F_PUBLIC)); }); + it('[#26672] a projection that leaves out id gets the same verdict, and no id it did not name', async () => { + const [contract] = await engine.find('contract', { where: { id: 'c_open' }, fields: ['signed_pdf'], context: READER_CTX }); + expect(contract).toEqual({ signed_pdf: hydrated(F_OWNED, 'owned.pdf') }); + expect(await engine.findOne('decision', { where: { id: 'dec_allow' }, fields: ['proof'], context: READER_CTX })) + .toEqual({ proof: hydrated(F_DEC_ALLOW, 'allow.pdf') }); + // The refusals do not move: the delegate's deny, and a copied-in id. + expect(await engine.findOne('decision', { where: { id: 'dec_deny' }, fields: ['proof'], context: READER_CTX })) + .toEqual({ proof: refused(F_DEC_DENY) }); + const [annexes] = await engine.find('contract', { where: { id: 'c_open' }, fields: ['annexes'], context: READER_CTX }); + expect(annexes.annexes[2]).toEqual(refused(F_VAULT)); + }); + it('one read, every case: each element of the multi-value field is judged on its own', async () => { const contract = await readAs('contract', 'c_open'); expect(contract.annexes).toEqual([ From 845325d3993485e8697b7e34d9a93046d9c8016d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 21:04:58 +0000 Subject: [PATCH 2/3] chore(changeset): patch for the file-field projection without id Claude-Session: https://claude.ai/code/session_01FEkQ5nso8iiU7cfQSEAkX5 Co-authored-by: Claude --- .changeset/26672-file-field-select-without-id.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .changeset/26672-file-field-select-without-id.md diff --git a/.changeset/26672-file-field-select-without-id.md b/.changeset/26672-file-field-select-without-id.md new file mode 100644 index 00000000000..298ac672c37 --- /dev/null +++ b/.changeset/26672-file-field-select-without-id.md @@ -0,0 +1,16 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): a file field reads the same whether or not the projection lists `id` + +Clause-②: no +Metadata-protocol: none + +A reader who may not read `sys_file` (every member whose application grants no `sys_file` read) is served a file field by the verdict of the record that owns the file, matched on that record's `id`. A read whose projection left `id` out, such as `GET /api/v1/data/OBJECT?select=FILE_FIELD`, gave that verdict no record key, so a file the reader may read came back as `{ id, metadataRefused: true }`, while `select=id,FILE_FIELD` answered the full `{ id, name, size, mimeType, url }`. + +`find` and `findOne` now read `id` for any projection that names a file field and cut it back off the answer, the same way a formula's widened columns are cut. So: + +- A permitted reader gets the full file value with or without `id` in `select` / `fields`, on `GET /data/:object`, `POST /data/:object/query` and `GET /data/:object/:id`. +- The answer still carries exactly the columns the caller named: no `id` is added to a row that did not ask for it. +- A reader the owning record's verdict refuses is still answered `{ id, metadataRefused: true }`, with or without `id`. From 6ea02bc0b048573946633ae4d7aa63803ce782d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 21:48:08 +0000 Subject: [PATCH 3/3] test(rest): type the direct sys_file read in the projection-without-id door test Claude-Session: https://claude.ai/code/session_01FEkQ5nso8iiU7cfQSEAkX5 Co-authored-by: Claude --- packages/rest/src/data-file-field-projection-without-id.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rest/src/data-file-field-projection-without-id.test.ts b/packages/rest/src/data-file-field-projection-without-id.test.ts index f3dd375adc9..373809eb7fa 100644 --- a/packages/rest/src/data-file-field-projection-without-id.test.ts +++ b/packages/rest/src/data-file-field-projection-without-id.test.ts @@ -175,7 +175,7 @@ describe('[#26672] a file field reads the same with or without `id` in the proje const as = (user: string) => { caller = user; }; it('the refusal it starts from: the reader may not read sys_file directly', async () => { - const direct = await engine.find('sys_file', { context: { userId: READER } } as any).then(() => null, (e: any) => e); + const direct = await engine.find('sys_file', { context: { userId: READER } }).then(() => null, (e: any) => e); expect({ code: direct?.code, status: direct?.statusCode ?? direct?.status }).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); });