Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/26672-file-field-select-without-id.md
Original file line number Diff line number Diff line change
@@ -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`.
29 changes: 25 additions & 4 deletions packages/objectql/src/engine-file-hydrate-parent-derived.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
51 changes: 44 additions & 7 deletions packages/objectql/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -1833,6 +1835,35 @@ function rowsWithoutFormulaWidening<T>(rows: T, widened: ReadonlySet<string> | 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=<file field>` and the full value to `select=id,<file field>`. 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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down
223 changes: 223 additions & 0 deletions packages/rest/src/data-file-field-projection-without-id.test.ts
Original file line number Diff line number Diff line change
@@ -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<Record<string, unknown>> => res._json?.records ?? res._json?.data ?? res._json;
const recordOf = (res: any): Record<string, unknown> => 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<string, unknown>) => Promise<any>;
let query: (body: Record<string, unknown>) => Promise<any>;
let getById: (id: string, query: Record<string, unknown>) => Promise<any>;

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<void>) => {
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 } }).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=<file field>: 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: [<file field>]: 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=<file field> (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,<file field> 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);
});
});
Loading
Loading