From ae407d52f020cf12d56f8c2412aa76a5712125d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:18:19 +0000 Subject: [PATCH 1/3] fix(objectql): ask canReadObject('sys_file') before the hydration's caller read resolveFileReferences read sys_file as the caller and took the refused arm on the throw, so plugin-security logged an access-denied WARN on every ordinary record read by a reader without sys_file read. The engine now asks the security service's canReadObject first, through the slot ObjectQLPlugin already wires, and serves a refused caller the same refused arm without the read. Unasked (no service, no canReadObject, a throwing ask) the read goes to the middleware as before. The refused arm moves into serveRefusedFileHydration, shared by the ask and the catch. Claude-Session: https://claude.ai/code/session_01YXbt8cZ1Adcz5iV8AdwBXJ Co-authored-by: Claude --- .../src/engine-file-hydrate-refused.test.ts | 139 ++++++++++++++++- packages/objectql/src/engine.ts | 140 +++++++++++++----- 2 files changed, 240 insertions(+), 39 deletions(-) diff --git a/packages/objectql/src/engine-file-hydrate-refused.test.ts b/packages/objectql/src/engine-file-hydrate-refused.test.ts index a02fb995ce9..94256eaf018 100644 --- a/packages/objectql/src/engine-file-hydrate-refused.test.ts +++ b/packages/objectql/src/engine-file-hydrate-refused.test.ts @@ -27,11 +27,18 @@ * `file-field-hydration-refused.test.ts`; this file holds the seam's own * branches with a fake DRIVER (not a fake engine) and an engine middleware * standing in for the security layer's refusal. + * + * [#24335] ASKED BEFORE READ: with a security service in the engine's slot, + * the engine asks its `canReadObject('sys_file')` first and a refused caller + * is served the refused arm without the `sys_file` read — so the security + * layer, which logs every refusal at WARN, sees no access attempt. The last + * block pins that against the same fixture, and that the served body is the + * one the refused read produced. */ import { describe, it, expect } from 'vitest'; import { FileRefusedValueSchema, valueSchemaFor } from '@objectstack/spec/data'; -import { ObjectQL } from './engine'; +import { ObjectQL, SECURITY_SERVICE_SOURCE_SLOT } from './engine'; const FILE_ID = 'f_7cQx2Lm90a'; const SECOND_ID = 'f_zz11yy22xx'; @@ -142,9 +149,11 @@ function makeCapturingLogger() { * field), a registered `sys_file`, and three rows: one holding a file, one * holding two files plus a legacy URL, one holding none. When `refuseReader` * is set, a middleware refuses every non-system `sys_file` read the way the - * security layer's CRUD gate does. + * security layer's CRUD gate does, and records each read it refuses in + * `attempts` (the reads plugin-security would log a denial for). `security`, + * when given, is what the engine's security-service slot resolves. */ -async function boot(opts: { sysFileRead?: () => Promise; refuseReader?: boolean } = {}) { +async function boot(opts: { sysFileRead?: () => Promise; refuseReader?: boolean; security?: unknown } = {}) { const logger = makeCapturingLogger(); const engine = new ObjectQL({ logger } as any); const { driver, storeFor } = makeDriver(opts.sysFileRead ?? (async () => COMMITTED_ROWS)); @@ -167,10 +176,17 @@ async function boot(opts: { sysFileRead?: () => Promise; refuseReader?: b status: { type: 'text' }, }, } as any); + const attempts: any[] = []; + if (opts.security !== undefined) { + (engine as unknown as Record)[SECURITY_SERVICE_SOURCE_SLOT] = () => opts.security; + } if (opts.refuseReader) { engine.registerMiddleware( async (ctx: any, next: () => Promise) => { - if (!ctx.context?.isSystem) throw refusal(); + if (!ctx.context?.isSystem) { + attempts.push(ctx.context); + throw refusal(); + } return next(); }, { object: 'sys_file' }, @@ -184,7 +200,7 @@ async function boot(opts: { sysFileRead?: () => Promise; refuseReader?: b gallery: [FILE_ID, SECOND_ID, 'https://cdn.example.com/legacy.png'], }); storeFor('doc').set('d3', { id: 'd3', title: 'Empty', attachment: null, gallery: null }); - return { engine, logger }; + return { engine, logger, attempts }; } const byId = (rows: any[]) => Object.fromEntries(rows.map((r) => [r.id, r])); @@ -296,3 +312,116 @@ describe('a refused sys_file read marks the file field refused — not absent', expect(logger.lines.debug.filter((l: any) => /sys_file read refused/.test(l.msg))).toEqual([]); }); }); + +/** + * A security service the engine's slot resolves, answering `canReadObject` as + * `admits` says (the fixture's middleware refuses every non-system reader, so + * `false` is the true answer for READER_CTX), and recording every ask. + */ +function makeSecurity(admits: (object: string, context: any) => boolean | Promise) { + const asks: Array<{ object: string; context: any }> = []; + const svc: Record = { + getReadFilter: async () => undefined, + canReadObject: async (object: string, context: any) => { + asks.push({ object, context }); + return admits(object, context); + }, + }; + return { svc, asks }; +} + +describe('[#24335] the engine asks before it reads sys_file — a refused reader makes no access attempt', () => { + it('a reader the admission refuses is served the refused arm and never reaches the sys_file read', async () => { + const { svc, asks } = makeSecurity((_o, ctx) => ctx?.isSystem === true); + const { engine, attempts } = await boot({ refuseReader: true, security: svc }); + + const rows = byId(await engine.find('doc', { context: READER_CTX })); + + // No read of sys_file reached the security layer: nothing for it to log. + expect(attempts).toEqual([]); + expect(rows.d1.attachment).toEqual({ id: FILE_ID, metadataRefused: true }); + // One ask per read, about sys_file, with the caller's own envelope. + expect(asks).toHaveLength(1); + expect(asks[0].object).toBe('sys_file'); + expect(asks[0].context).toMatchObject(READER_CTX); + expect(Object.keys(asks[0].context).filter((k) => k.startsWith('__'))).toEqual([]); + }); + + it('the body is byte-identical to the one the refused read served, by list and by id', async () => { + const asked = await boot({ refuseReader: true, security: makeSecurity(() => false).svc }); + const unasked = await boot({ refuseReader: true }); + + const listAsked = await asked.engine.find('doc', { context: READER_CTX }); + const listUnasked = await unasked.engine.find('doc', { context: READER_CTX }); + const oneAsked = await asked.engine.findOne('doc', { where: { id: 'd2' }, context: READER_CTX }); + const oneUnasked = await unasked.engine.findOne('doc', { where: { id: 'd2' }, context: READER_CTX }); + + // The comparison is armed: the unasked engine did attempt, and was refused. + expect(unasked.attempts.length).toBeGreaterThan(0); + expect(asked.attempts).toEqual([]); + expect(oneAsked?.gallery?.[0]).toEqual({ id: FILE_ID, metadataRefused: true }); + expect(JSON.stringify(listAsked)).toBe(JSON.stringify(listUnasked)); + expect(JSON.stringify(oneAsked)).toBe(JSON.stringify(oneUnasked)); + }); + + it('the reader\'s own sys_file read is still refused, and still reaches the security layer', async () => { + const { engine, attempts } = await boot({ refuseReader: true, security: makeSecurity(() => false).svc }); + + const err = await engine.find('sys_file', { context: READER_CTX }).catch((e: unknown) => e); + + expect((err as any)?.code).toBe('PERMISSION_DENIED'); + expect((err as any)?.statusCode).toBe(403); + expect(attempts).toHaveLength(1); + }); + + it('CONTROL a reader the admission admits still reads sys_file as itself, and is hydrated', async () => { + const { svc, asks } = makeSecurity(() => true); + const { engine } = await boot({ security: svc }); + + const rows = byId(await engine.find('doc', { context: READER_CTX })); + + expect(asks).toHaveLength(1); + expect(rows.d1.attachment).toEqual({ + id: FILE_ID, + name: 'contract.pdf', + size: 1024, + mimeType: 'application/pdf', + url: `/api/v1/storage/files/${FILE_ID}`, + }); + }); + + it('CONTROL an admitted read the security layer still refuses takes the same refused arm', async () => { + const { engine, attempts } = await boot({ refuseReader: true, security: makeSecurity(() => true).svc }); + + const rows = byId(await engine.find('doc', { context: READER_CTX })); + + expect(attempts).toHaveLength(1); + expect(rows.d1.attachment).toEqual({ id: FILE_ID, metadataRefused: true }); + }); + + it('CONTROL unasked — no canReadObject, or an ask that throws — the read goes to the security layer as before', async () => { + const noMethod = await boot({ refuseReader: true, security: { getReadFilter: async () => undefined } }); + const throwing = await boot({ + refuseReader: true, + security: makeSecurity(() => { + throw new Error('admission unavailable'); + }).svc, + }); + + for (const { engine, attempts } of [noMethod, throwing]) { + const rows = byId(await engine.find('doc', { context: READER_CTX })); + expect(attempts).toHaveLength(1); + expect(rows.d1.attachment).toEqual({ id: FILE_ID, metadataRefused: true }); + } + }); + + it('CONTROL a record holding no file id asks nothing', async () => { + const { svc, asks } = makeSecurity(() => false); + const { engine } = await boot({ refuseReader: true, security: svc }); + + const row = await engine.findOne('doc', { where: { id: 'd3' }, context: READER_CTX }); + + expect(row?.id).toBe('d3'); + expect(asks).toEqual([]); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 7ca9a353fe8..0186fa3cf4e 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -13305,10 +13305,19 @@ export class ObjectQL implements IObjectQLEngine { * that record — and hydrates when it allows. Every other id keeps the * refused marker. See {@link readParentDerivedFiles}. * - * Batched: at most one `sys_file` `id $in […]` read per call (no N+1) — and, - * on the refused arm only, one more system read plus one verdict call; zero - * reads when no field holds a string value (the blob-only case), so the step - * is free for objects that have not adopted references. + * ASKED BEFORE READ (#24335): the caller's `sys_file` read is issued only + * when the security service admits it — {@link callerMayReadSysFile}, the + * middleware's own object-level read gate. A caller it refuses goes + * straight to the refused arm, the answer that read's refusal produced, so + * the served value is the same and no access attempt is made: the security + * layer logs no denial for a read the reader never asked for. A direct + * `sys_file` read by that reader is refused, and logged, exactly as before. + * + * Batched: one admission ask and at most one `sys_file` `id $in […]` read + * per call (no N+1) — and, on the refused arm only, one more system read + * plus one verdict call; zero of each when no field holds a string value + * (the blob-only case), so the step is free for objects that have not + * adopted references. */ private async resolveFileReferences( objectName: string, @@ -13355,6 +13364,13 @@ export class ObjectQL implements IObjectQLEngine { const uniqueIds = [...new Set(candidateIds)]; if (uniqueIds.length === 0) return records; // blob-only / empty → zero cost + // [#24335] Ask before reading: a caller the security service refuses + // `sys_file` read is served the refused arm without the read whose + // refusal would have produced it. + if (!(await this.callerMayReadSysFile(execCtx))) { + return this.serveRefusedFileHydration(objectName, records, fileFields, uniqueIds, execCtx); + } + // One batched sys_file read. `__expandRead` mirrors the lookup-expansion // sub-read (a system-built marker, never from client input). Metadata only — // byte-download authorization is enforced at the /files/:fileId resolver. @@ -13408,37 +13424,12 @@ export class ObjectQL implements IObjectQLEngine { // them. The marker stays on everything else: an id another record owns // (copied in), an attachment-only or unclaimed file, an id with no row, // and every id when the verdict is unwired or cannot be asked. + // + // [#24335] A caller the security service's admission already refused + // never reaches this read (see {@link callerMayReadSysFile}); this arm + // still serves a refusal the admission did not foresee, the same way. if (isReadRefusal(error)) { - const owned = await this.readParentDerivedFiles(objectName, records, uniqueIds, execCtx); - this.logger.debug('sys_file read refused for this caller; file fields carry the refused marker', { - object: objectName, - fields: fileFields, - refusedIds: uniqueIds.length, - parentDerivedIds: owned.size, - }); - const refused = (id: string): FileRefusedValue => ({ id, metadataRefused: true }); - // Judged per ELEMENT and per RECORD: the verdict covers a file only on - // the record whose field owns it, so the same id copied into a second - // record is still refused there. An owned, allowed id with no servable - // row reads as the bare id — what a `sys_file` reader gets for it. - const valueFor = (record: any, v: unknown): unknown => { - if (!isFileIdToken(v)) return v; - const hit = owned.get(v); - if (!hit || record?.id == null || hit.ownerId !== String(record.id)) return refused(v); - return hit.row ? toFileValue(hit.row) : v; - }; - for (const record of records) { - for (const fieldName of fileFields) { - const val = record[fieldName]; - if (val == null) continue; - if (Array.isArray(val)) { - record[fieldName] = val.map((v: unknown) => valueFor(record, v)); - } else { - record[fieldName] = valueFor(record, val); - } - } - } - return records; + return this.serveRefusedFileHydration(objectName, records, fileFields, uniqueIds, execCtx); } // Everything else (connection drop, timeout, a database-level permission // fault, query error) means the rows may well exist and simply were not @@ -13485,6 +13476,87 @@ export class ObjectQL implements IObjectQLEngine { return records; } + /** + * [#24335] May this caller read `sys_file` at all? Asked of the registered + * security service's `canReadObject` BEFORE {@link resolveFileReferences} + * issues the caller's own `sys_file` read, as the parent-record read gate + * asks before it reads a parent object ("A skipped object is not an access + * attempt", plugin-audit). `canReadObject` is the security middleware's own + * object-level read gate, arm for arm, and answers without logging or + * throwing; the read it spares is one that gate would refuse, and + * plugin-security logs every refusal at WARN — a line on every ordinary + * record read by a reader without `sys_file` read, burying the real denials. + * + * Asked with the caller's envelope minus its operation-private `__` keys + * (`withoutOperationPrivateKeys`, as {@link readParentDerivedFiles} asks its + * verdict): no second derivation of authority, and no muting of the + * middleware's log — the reader's own `sys_file` read still logs its denial. + * + * It only ever NARROWS what is read: `true` still reads as the caller, and + * that read still decides. Unasked ⇒ `true`, the read goes to the middleware + * as it did before: no usable security service, one whose contract omits the + * optional `canReadObject`, or an ask that throws. Any other answer than + * `true` ⇒ the refused arm, which is what that read's refusal served. + */ + private async callerMayReadSysFile(execCtx?: ExecutionContext): Promise { + const security = this.resolveReadableIdsSecurity(); + if (security.kind !== 'usable' || typeof security.svc.canReadObject !== 'function') return true; + const caller = withoutOperationPrivateKeys((execCtx ?? {}) as Record); + try { + return (await security.svc.canReadObject('sys_file', caller as ExecutionContext)) === true; + } catch { + return true; + } + } + + /** + * The refused arm of {@link resolveFileReferences}: this caller may not read + * `sys_file`, whether its admission said so up front (#24335) or its read was + * refused. Each id a record of this result owns is put to the field-owned + * verdict ({@link readParentDerivedFiles}, #22637) and hydrates when it + * allows; every other id becomes `{ id, metadataRefused: true }` + * (`FileRefusedValueSchema`). Logged at `debug`: the answer is handed to + * the caller in the value. + */ + private async serveRefusedFileHydration( + objectName: string, + records: any[], + fileFields: string[], + uniqueIds: string[], + execCtx?: ExecutionContext, + ): Promise { + const owned = await this.readParentDerivedFiles(objectName, records, uniqueIds, execCtx); + this.logger.debug('sys_file read refused for this caller; file fields carry the refused marker', { + object: objectName, + fields: fileFields, + refusedIds: uniqueIds.length, + parentDerivedIds: owned.size, + }); + const refused = (id: string): FileRefusedValue => ({ id, metadataRefused: true }); + // Judged per ELEMENT and per RECORD: the verdict covers a file only on + // the record whose field owns it, so the same id copied into a second + // record is still refused there. An owned, allowed id with no servable + // row reads as the bare id — what a `sys_file` reader gets for it. + const valueFor = (record: any, v: unknown): unknown => { + if (!isFileIdToken(v)) return v; + const hit = owned.get(v); + if (!hit || record?.id == null || hit.ownerId !== String(record.id)) return refused(v); + return hit.row ? toFileValue(hit.row) : v; + }; + for (const record of records) { + for (const fieldName of fileFields) { + const val = record[fieldName]; + if (val == null) continue; + if (Array.isArray(val)) { + record[fieldName] = val.map((v: unknown) => valueFor(record, v)); + } else { + record[fieldName] = valueFor(record, val); + } + } + } + return records; + } + /** * The servable subset of some `sys_file` rows, keyed by id — the rule both * arms of {@link resolveFileReferences} serve by, so a row hydrates under From 3bf2088705b75136976fe3990add91945952ffd3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:26:29 +0000 Subject: [PATCH 2/3] test(dogfood): a file-field record read logs no sys_file access denial Booted pin on the real SecurityPlugin and StorageServicePlugin: a member's by-id and list reads of a record holding its own file log no security denial and serve the body the no-ask read serves byte for byte; the no-ask arm still logs the denial (armed); a direct sys_file read is still 403 PERMISSION_DENIED with one denial logged; a sys_file reader is unchanged. Claude-Session: https://claude.ai/code/session_01YXbt8cZ1Adcz5iV8AdwBXJ Co-authored-by: Claude --- ...-field-hydration-no-denial.dogfood.test.ts | 284 ++++++++++++++++++ 1 file changed, 284 insertions(+) create mode 100644 packages/qa/dogfood/test/file-field-hydration-no-denial.dogfood.test.ts diff --git a/packages/qa/dogfood/test/file-field-hydration-no-denial.dogfood.test.ts b/packages/qa/dogfood/test/file-field-hydration-no-denial.dogfood.test.ts new file mode 100644 index 00000000000..365df2f4a0a --- /dev/null +++ b/packages/qa/dogfood/test/file-field-hydration-no-denial.dogfood.test.ts @@ -0,0 +1,284 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// A record read whose only `sys_file` touch is file-field hydration logs no +// access-denied WARN. +// +// The engine expands a stored file id (ADR-0104 D3) by reading `sys_file`, and +// it used to issue that read AS THE CALLER. A member who may read the record +// but not `sys_file` — the platform default, `member_default` grants no +// `sys_file` read — was refused it, and plugin-security logs every refusal at +// WARN: one `[Security] Access denied: operation 'find' on object 'sys_file'` +// line per by-id read and per list read, for every such reader, for a read the +// reader never asked for, while the body served was right (the field-owned +// file hydrates, #22637). The lines buried the real denials. +// +// The engine now asks the security service's `canReadObject('sys_file')` +// first — as the parent-record read gates ask before they read a parent — and +// serves a refused caller the field-owned arm without the read. What this +// file holds, on a real org-bound boot with the real SecurityPlugin and the +// real StorageServicePlugin (real bytes, the real field-owned verdict): +// +// - a member's by-id and list reads of a record holding its own file log NO +// denial, and serve the file hydrated; +// - their bodies are byte-identical to the bodies served when the security +// service offers no `canReadObject` (a defined state of the contract, in +// which the engine reads as the caller, as it did before) — asked of the +// same boot, so the comparison is about the ask and nothing else; +// - that no-ask read still logs the denial, so the comparison is armed; +// - a direct `GET /data/sys_file/:id` by the same member is still `403 +// PERMISSION_DENIED`, and still logs its denial exactly once; +// - a reader granted `sys_file` read is unchanged: no denial, the same +// hydrated value, the same body on both arms. +// +// `@objectstack/objectql` resolves through its BUILT output here (a ledgered +// pair in `scripts/check-test-source-alias.mjs`), so a verdict on a change to +// it is a verdict on its last build. + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { mkdtempSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PermissionSetSchema, type PermissionSet } from '@objectstack/spec/security'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; +import { StorageServicePlugin } from '@objectstack/service-storage'; +import { PlatformObjectsPlugin } from '@objectstack/platform-objects/plugin'; +import { assertArmed, principalArmed } from './armed.js'; +import { catalogPermissionSet } from './catalog-permission-set.js'; + +const SYS = { isSystem: true } as const; +const DOC = 'fhn_doc'; +const MEMBER_EMAIL = 'fhn-member@verify.test'; +const READER_EMAIL = 'fhn-file-reader@verify.test'; + +const fixtureStack = defineStack({ + manifest: { + id: 'com.dogfood.file-hydration-no-denial', + namespace: 'fhn', + version: '0.0.0', + type: 'app', + name: 'File Hydration No-Denial Fixture', + description: 'One record object with a file field, readable by every member.', + }, + objects: [ + ObjectSchema.create({ + name: DOC, + label: 'Hydration Doc', + pluralLabel: 'Hydration Docs', + sharingModel: 'public_read_write', + fields: { + name: Field.text({ label: 'Name', required: true }), + doc: Field.file({ label: 'Doc' }), + }, + }), + ], +}); + +/** The member app grant: the record object, no wildcard, so no `sys_file` read. */ +const memberBaselineSet: PermissionSet = PermissionSetSchema.parse({ + name: 'fhn_member_baseline', + label: 'File-hydration fixture — the member app grant, no wildcard', + objects: { [DOC]: { allowRead: true, allowCreate: true, allowEdit: true } }, +}); + +/** The control's grant: object-level `sys_file` read on top of the baseline. */ +const fileReaderSet: PermissionSet = PermissionSetSchema.parse({ + name: 'fhn_file_reader', + label: 'File-hydration fixture — object-level sys_file read', + objects: { sys_file: { allowRead: true } }, +}); + +/** The denial plugin-security logs, as a matcher (its developer sentence's opener). */ +const DENIAL = '[Security] Access denied'; + +describe('file-field hydration: a record read is not a sys_file access attempt', () => { + let stack: VerifyStack; + let ql: any; + let adminTok: string; + let memberTok: string; + let readerTok: string; + let fileId: string; + let recordId: string; + /** The kernel logger every plugin context shares — plugin-security's `ctx.logger`. */ + let logger: { warn: (...args: unknown[]) => void }; + /** The registered `security` service, the object the engine resolves on every read. */ + let security: { canReadObject?: unknown }; + + /** Every security denial the stack logs while `run` is in flight. */ + const denialsDuring = async (run: () => Promise): Promise<{ value: T; denials: string[] }> => { + const spy = vi.spyOn(logger, 'warn'); + try { + const value = await run(); + return { value, denials: spy.mock.calls.map((args) => String(args[0])).filter((w) => w.includes(DENIAL)) }; + } finally { + spy.mockRestore(); + } + }; + + /** `run`, with the security service offering no `canReadObject` — the contract's optional-method state. */ + const withoutReadAdmission = async (run: () => Promise): Promise => { + const own = Object.getOwnPropertyDescriptor(security, 'canReadObject'); + expect(own, 'the registered service carries canReadObject as its own member').toBeTruthy(); + delete security.canReadObject; + try { + return await run(); + } finally { + Object.defineProperty(security, 'canReadObject', own!); + } + }; + + /** One read of `path` as `token`: its status, its body text, and the denials it logged. */ + const read = async (token: string, path: string) => { + const { value, denials } = await denialsDuring(async () => { + const res = await stack.apiAs(token, 'GET', path); + return { status: res.status, text: await res.text() }; + }); + return { ...value, denials }; + }; + + /** The file field of the fixture record, read off a by-id or a list body. */ + const docOf = (text: string): unknown => { + const body = JSON.parse(text); + const rec = body.records?.find((r: any) => String(r.id) === recordId) ?? body.record ?? body.data ?? body; + return rec?.doc; + }; + + const hydrated = () => ({ + id: fileId, + name: 'contract.txt', + size: 5, + mimeType: 'text/plain', + url: `/api/v1/storage/files/${fileId}`, + }); + + /** Upload real bytes as the admin and return the `sys_file` id (scope `user`, the field lineage). */ + async function uploadFile(name: string): Promise { + const auth = { Authorization: `Bearer ${adminTok}` }; + const presign = await stack.api('/storage/upload/presigned', { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...auth }, + body: JSON.stringify({ filename: name, mimeType: 'text/plain', size: 5, scope: 'user' }), + }); + expect(presign.status, 'presign').toBe(200); + const { data } = (await presign.json()) as any; + expect(data.fileId, 'presign carried no fileId').toBeTruthy(); + const put = await stack.raw(String(data.uploadUrl).replace(/^https?:\/\/[^/]+/, ''), { + method: 'PUT', + headers: data.headers ?? { 'content-type': 'text/plain' }, + body: 'hello', + }); + expect(put.status, 'raw PUT').toBeLessThan(300); + const complete = await stack.api('/storage/upload/complete', { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...auth }, + body: JSON.stringify({ fileId: data.fileId }), + }); + expect(complete.status, 'complete').toBe(200); + return String(data.fileId); + } + + beforeAll(async () => { + const rootDir = mkdtempSync(join(tmpdir(), 'fhn-dogfood-')); + stack = await bootStack(fixtureStack as never, { + security: new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, memberBaselineSet, fileReaderSet], + fallbackPermissionSet: memberBaselineSet.name, + }), + extraPlugins: [ + new PlatformObjectsPlugin(), + new StorageServicePlugin({ adapter: 'local', local: { rootDir }, bindToSettings: false }), + ], + // Org-bound: the member holds `org_member`, the position the platform's + // member floors are domained to. + orgContext: true, + }); + adminTok = await stack.signIn(); + memberTok = await stack.signUp(MEMBER_EMAIL); + readerTok = await stack.signUp(READER_EMAIL); + ql = await stack.kernel.getServiceAsync('objectql'); + security = await stack.kernel.getServiceAsync('security'); + logger = (stack.kernel as unknown as { logger: { warn: (...args: unknown[]) => void } }).logger; + + const readerId = (await ql.findOne('sys_user', { where: { email: READER_EMAIL }, context: SYS }))?.id; + const set = await catalogPermissionSet(ql, fileReaderSet.name); + expect(set, 'fixture permission set served by the security catalog').toBeTruthy(); + await ql.insert('sys_user_permission_set', { user_id: readerId, permission_set: set.name }, { context: { ...SYS } }); + + fileId = await uploadFile('contract.txt'); + const row = await ql.insert(DOC, { name: 'contract', doc: fileId }, { context: { ...SYS } }); + recordId = String((row as any).id ?? row); + // The field-reference hooks claimed the file for the record: it is + // field-owned, which is what the refused reader's hydration serves by. + const file = await ql.findOne('sys_file', { where: { id: fileId }, context: SYS }); + expect(file?.ref_object).toBe(DOC); + expect(String(file?.ref_id)).toBe(recordId); + + await assertArmed([ + principalArmed({ + stack, + token: memberTok, + who: 'the member holding the app grant and no sys_file read', + positions: ['org_member'], + permissions: [], + control: 'object-level read on the fixture object only, by the fallback baseline', + disarmedBy: 'a member that resolved a sys_file grant would never meet the refusal this file is about', + }), + principalArmed({ + stack, + token: readerTok, + who: 'the reader granted object-level sys_file read', + positions: ['org_member'], + permissions: [fileReaderSet.name], + control: 'object-level read on sys_file, granted by an explicit permission set', + disarmedBy: 'a reader that never resolved the grant would be the member again, and the control would measure nothing', + }), + ]); + }, 180_000); + + afterAll(async () => { + await stack?.stop(); + }); + + for (const [door, path] of [ + ['by id', () => `/data/${DOC}/${recordId}`], + ['list', () => `/data/${DOC}`], + ] as const) { + it(`${door}: the member's read logs no denial, and serves the no-ask read's body byte for byte`, async () => { + const before = await withoutReadAdmission(() => read(memberTok, path())); + // Armed: without the ask, the member's read is refused sys_file and + // that refusal is logged — the line this file is about. + expect(before.status).toBe(200); + expect(before.denials).toHaveLength(1); + expect(before.denials[0]).toContain("on object 'sys_file'"); + + const after = await read(memberTok, path()); + expect(after.status).toBe(200); + expect(after.denials).toEqual([]); + expect(after.text).toBe(before.text); + // The field-owned file is hydrated (#22637), not marked refused. + expect(docOf(after.text)).toEqual(hydrated()); + }, 60_000); + + it(`${door}: CONTROL a reader with sys_file read is unchanged — no denial, the same file, the same body`, async () => { + const before = await withoutReadAdmission(() => read(readerTok, path())); + const after = await read(readerTok, path()); + expect(before.status).toBe(200); + expect(after.status).toBe(200); + expect(before.denials).toEqual([]); + expect(after.denials).toEqual([]); + expect(after.text).toBe(before.text); + expect(docOf(after.text)).toEqual(hydrated()); + }, 60_000); + } + + it('the member\'s direct sys_file read is still 403 PERMISSION_DENIED, and logs its denial exactly once', async () => { + const res = await read(memberTok, `/data/sys_file/${fileId}`); + expect(res.status).toBe(403); + const body = JSON.parse(res.text); + expect(body.code ?? body.error?.code).toBe('PERMISSION_DENIED'); + expect(res.denials).toHaveLength(1); + expect(res.denials[0]).toContain("on object 'sys_file'"); + }); +}); From 2b67dd3d718eee122e26c6ca4fc11cb35522ecc2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:31:07 +0000 Subject: [PATCH 3/3] chore(changeset): objectql patch for the hydration's sys_file ask Claude-Session: https://claude.ai/code/session_01YXbt8cZ1Adcz5iV8AdwBXJ Co-authored-by: Claude --- .changeset/24335-file-hydration-no-denial.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .changeset/24335-file-hydration-no-denial.md diff --git a/.changeset/24335-file-hydration-no-denial.md b/.changeset/24335-file-hydration-no-denial.md new file mode 100644 index 00000000000..c7813744eab --- /dev/null +++ b/.changeset/24335-file-hydration-no-denial.md @@ -0,0 +1,17 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): a record read that hydrates a file field no longer makes a `sys_file` access attempt for a reader without `sys_file` read + +The engine expands a stored file id by reading `sys_file`, and it issued that read as the caller. A member who may read the record but not `sys_file` (the platform default) was refused it, and the security layer logged `[Security] Access denied: operation 'find' on object 'sys_file'` at WARN on every by-id and list read by that member, although the body served was right. Those lines buried the real denials in the operator's log. + +The engine now asks the security service's `canReadObject('sys_file')` first, the same object-level read gate the security middleware applies, and serves a refused caller the field-owned answer without the read. Nothing else changes: + +- The served body is byte-identical, by id and by list, for a reader with and without `sys_file` read. A file owned by the record still hydrates, and every other id still reads `{ id, metadataRefused: true }`. +- A direct `GET /api/v1/data/sys_file/:id` by the same reader still answers `403 PERMISSION_DENIED` and still logs its denial. +- With no security service, or one that offers no `canReadObject`, the engine reads as the caller, as before. + +The ask is one cached permission check per read, not per row. + +Clause-②: no (no accept set or public surface moves: the served bodies and every refusal are unchanged; owes the seat's review of record at `TIER_DEFAULT` before the queue)