diff --git a/.changeset/sec-r16-attach-verb-vouched-file.md b/.changeset/sec-r16-attach-verb-vouched-file.md new file mode 100644 index 00000000000..3f29e07a783 --- /dev/null +++ b/.changeset/sec-r16-attach-verb-vouched-file.md @@ -0,0 +1,16 @@ +--- +'@objectstack/core': patch +'@objectstack/objectql': patch +'@objectstack/service-storage': patch +--- + +fix(service-storage): a member attaches a file through the attach verb again; the verb vouches for the one file it created + +Clause-②: no +Metadata-protocol: none + +A signed-in member who may attach files can attach one again through the server-side attach verb (`attachFileAs` on the `file-attachments` service), and so can a print to a record's attachments (`printRecordToFile`). Since the write-time reference check began judging a `lookup` value through the caller's own read door, the verb's `sys_attachment` insert — written as the caller — was refused over its `file_id`: a member who may attach need not read `sys_file`, so the file the verb had just created answered `reference_not_found` and the call failed. + +The rule now: a service verb that has created a record in the same call may vouch, on its own write, for the one reference to that record (`vouchForReference`, `@objectstack/core`). The write-time reference check does not ask the caller's read door for exactly that object, field and id, and still probes that the record exists. The attach verb vouches for its own `sys_attachment.file_id` and for nothing else; every other gate on the insert still judges the caller (the permission to create an attachment, the parent-edit attach rule, `enable.files`, the file-reference gate, row- and field-level security, and the `uploaded_by` stamp). + +Unchanged: no caller gains a read of `sys_file` or of anything else. A `sys_file` id sent through any data door — REST, MCP, a flow, an import — is judged through the caller's read door exactly as before, the file the verb attached included. The mark is a symbol-keyed context property that no request body, query, stored row or flow definition can spell. diff --git a/packages/core/src/security/index.ts b/packages/core/src/security/index.ts index 58ca3a3a563..5095f0d2da8 100644 --- a/packages/core/src/security/index.ts +++ b/packages/core/src/security/index.ts @@ -537,6 +537,16 @@ export { keepsFlowWriteReferenceCheck, } from './flow-write-reference-check.js'; +// The mark a service verb's own write carries for one reference to a record +// the same call authorized and created. Set by service-storage's +// file-attachments writer, read by objectql's write-time reference check. +export { + VOUCHED_REFERENCES, + vouchForReference, + isVouchedReference, + type VouchedReference, +} from './vouched-reference.js'; + // #30205 — a banned user's open work: the per-object counts, the optional bulk // hand-over (both read and written as the caller), and the ban-in-force // predicate objectql's write-time reference check refuses a new assignment by. diff --git a/packages/core/src/security/vouched-reference.ts b/packages/core/src/security/vouched-reference.ts new file mode 100644 index 00000000000..65d934a2d80 --- /dev/null +++ b/packages/core/src/security/vouched-reference.ts @@ -0,0 +1,100 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The mark a service's own write carries for ONE reference value it vouches + * for: a record the same call has itself authorized and created. + * + * ## The rule it answers to + * + * A write that is not a system write may name, in a `lookup`, `user` or + * `master_detail` field, only a record its caller's own read door returns; a + * record the caller cannot read answers `reference_not_found` + * (`assertReferencesResolve`, objectql). That rule judges a reference the + * CALLER chose. Some service verbs write, as the caller, a row that links to a + * record the verb created a moment earlier in the same call, after asking its + * own authorization question first. The file-attachments verb is the case: it + * asks the attach rule (`assertMayAttach`), stores a new `sys_file` for the + * caller, then inserts the `sys_attachment` naming it AS the caller, so every + * gate on that insert judges the caller. The caller did not choose that file + * id, and a caller who may attach files need not read `sys_file` at all, so + * the read-door question has nothing to judge there; asking it refuses the + * verb outright. + * + * ## What it admits, and what it does not + * + * The mark names one `(object, field, id)`. The reference check reads it for + * exactly that value on a write to that object: the caller's read door is not + * asked for it, and the elevated, tenant-kept existence probe still is, so an + * id that names no row of the caller's organization is refused as before. + * Everything else about the write stays the caller's: every other field and + * every other value in the same field is judged through the caller's read door, + * and nothing about RLS, FLS, the owner and audit stamps, the object's hooks + * or the caller's read of anything changes. It grants no read. + * + * ## Where it is set — and where it is never set + * + * Only by a service verb, on the one write it makes for the record it just + * created, after that verb's own authorization: today the file-attachments + * writer's `sys_attachment` insert (`FileAttachmentService.attachFileAs`, + * service-storage). ⛔ Never on a context a request, an import, a flow, an + * agent tool or any other generic data door hands the engine: a reference such + * a door carries is the caller's choice, and the read-door rule judges it. + * + * ## Why a symbol, and why it is not an `ExecutionContext` field + * + * The same reasons as `FLOW_WRITE_REFERENCE_CHECK`: no request body, query + * string, stored row or flow definition can spell a symbol, so no author can + * set it; it is in no schema; `Symbol.for` keeps it one key across the ESM and + * CJS builds; and, being undeclared, it does not survive + * `ExecutionContextSchema.parse`. It rides a spread of the context as any own + * property does, so a write a hook sets off under the vouched write carries it + * too, and is admitted only for the same object, field and id. + */ + +/** The context key of the mark. See the module header. */ +export const VOUCHED_REFERENCES: unique symbol = Symbol.for('objectstack.core.vouchedReferences'); + +/** One reference value a service vouches for: `field` of a write to `object`, naming `id`. */ +export interface VouchedReference { + readonly object: string; + readonly field: string; + readonly id: string; +} + +/** + * `context` with `reference` added to the references it vouches for — a fresh + * object; `context` is not changed. Called only by a service verb, for a + * record the same call created (see the module header). + */ +export function vouchForReference( + context: T, + reference: VouchedReference, +): T & { [VOUCHED_REFERENCES]: ReadonlyArray } { + const held = (context as { [VOUCHED_REFERENCES]?: unknown })[VOUCHED_REFERENCES]; + const prior = Array.isArray(held) ? (held as ReadonlyArray) : []; + const entry: VouchedReference = Object.freeze({ + object: String(reference.object), + field: String(reference.field), + id: String(reference.id), + }); + return { ...context, [VOUCHED_REFERENCES]: Object.freeze([...prior, entry]) }; +} + +/** + * Does `context` vouch for `id` in `field` of a write to `object`? `false` for + * every other value, field, object and context. + */ +export function isVouchedReference(context: unknown, object: unknown, field: unknown, id: unknown): boolean { + if (context === null || typeof context !== 'object') return false; + if (typeof object !== 'string' || typeof field !== 'string') return false; + if (typeof id !== 'string' && typeof id !== 'number') return false; + const held = (context as { [VOUCHED_REFERENCES]?: unknown })[VOUCHED_REFERENCES]; + if (!Array.isArray(held)) return false; + const value = String(id); + return held.some( + (r) => r !== null && typeof r === 'object' + && (r as VouchedReference).object === object + && (r as VouchedReference).field === field + && (r as VouchedReference).id === value, + ); +} diff --git a/packages/objectql/src/engine-referential-integrity.ts b/packages/objectql/src/engine-referential-integrity.ts index 6fc9b11c36b..ffa5f04153e 100644 --- a/packages/objectql/src/engine-referential-integrity.ts +++ b/packages/objectql/src/engine-referential-integrity.ts @@ -52,7 +52,7 @@ import { referenceTargetOf, type EngineQueryOptions } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; -import { isUserBanInForce, keepsFlowWriteReferenceCheck, USER_BAN_FIELDS } from '@objectstack/core'; +import { isUserBanInForce, isVouchedReference, keepsFlowWriteReferenceCheck, USER_BAN_FIELDS } from '@objectstack/core'; import { ValidationError, buildFieldError } from './validation/record-validator.js'; import { servedAsStored } from './engine-top-level.js'; import type { ObjectQL } from './engine.js'; @@ -219,6 +219,16 @@ export function referencesCheckedBeforeRules(context: unknown): boolean { * and a value the row already stores is not judged again (#29062). `tree` * keeps the elevated probe alone. * + * One value is not the caller's choice: a reference a service verb vouches for + * on its own write (`vouchForReference`, `@objectstack/core`), naming a record + * the same call authorized and created — the file-attachments writer's + * `sys_attachment.file_id`, inserted as the caller after its own attach check. + * For exactly that object, field and id the caller's read door is not asked; + * the elevated, tenant-kept existence probe still is. Every other value, field + * and object on the write is judged as above, and the mark comes from no + * request: it is a symbol-keyed context property no body, query or stored row + * can spell. + * * Fails OPEN when the target cannot be checked (unregistered object, no * driver, a probe that throws): an integrity check that cannot run must not * invent a rejection, and the alternative — refusing every write to an object @@ -301,8 +311,12 @@ export async function assertReferencesResolve( if (v === null || v === undefined || v === '') continue; if (typeof v === 'object') continue; // [#22519, #29029] A master-detail header, a lookup target and a user - // must also be one the caller can read. - const callerReads = CALLER_READ_REFERENCE_TYPES.has(def?.type); + // must also be one the caller can read — except the one value a service + // verb vouches for on its own write: a record the same call authorized + // and created (`vouchForReference`, @objectstack/core). Its existence is + // still probed; nothing else on the write is admitted by it. + const callerReads = CALLER_READ_REFERENCE_TYPES.has(def?.type) + && !isVouchedReference(context, schema?.name, name, v); if (def?.type !== 'master_detail' && target === BAN_CHECKED_USER_OBJECT) { // [#30205] One probe answers both: the row's existence and its ban. // [#29029] The caller's read door comes first: a user it does not diff --git a/packages/objectql/src/engine-unreadable-reference-target.test.ts b/packages/objectql/src/engine-unreadable-reference-target.test.ts index 5fa2bfc3b19..7a2299eee80 100644 --- a/packages/objectql/src/engine-unreadable-reference-target.test.ts +++ b/packages/objectql/src/engine-unreadable-reference-target.test.ts @@ -31,7 +31,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { resolveThrownHttpError } from '@objectstack/types'; -import { markFlowWriteReferenceCheck } from '@objectstack/core'; +import { markFlowWriteReferenceCheck, vouchForReference, VOUCHED_REFERENCES } from '@objectstack/core'; import { ObjectQL } from './engine.js'; import { ValidationError } from './validation/record-validator.js'; @@ -498,3 +498,74 @@ describe('[#29029] the update\'s reference forgiveness compares only stored valu }); }); }); + +describe('a reference a service verb vouches for on its own write: that one value skips the caller\'s read door, nothing else does', () => { + let engine: ObjectQL; + let targetReads: TargetRead[]; + let storeFor: (o: string) => Map>; + + beforeEach(() => { + ({ engine, targetReads, storeFor } = makeEngine()); + }); + + /** Alice's context, vouching for `id` in `field` of a write to `object`. */ + const vouched = (id: string, field = 'duty', object = ITEM, base: Record = ALICE) => + vouchForReference({ ...base }, { object, field, id }); + const insert = (object: string, payload: Record, context: Record) => + engine.insert(object, { title: 'new', ...payload }, { context }); + const items = (object = ITEM) => [...storeFor(object).values()]; + + it('the vouched value lands although the caller cannot read it; its existence is still probed, elevated', async () => { + await insert(ITEM, { duty: UNREADABLE }, vouched(UNREADABLE)); + expect(items().some((r) => r.duty === UNREADABLE)).toBe(true); + // No read door asked for it; the existence probe ran. + expect(targetReads.map(({ operation, elevated }) => ({ operation, elevated }))).toEqual([ + { operation: 'findOne', elevated: true }, + ]); + }); + + it('a vouched id that names no row is still refused reference_not_found', async () => { + expect(envelopeOf(await refusalOf(() => insert(ITEM, { duty: NOWHERE }, vouched(NOWHERE))))).toEqual(notFound('duty')); + expect(items()).toEqual([]); + }); + + it('control: another unreadable id in the vouched field is refused as before', async () => { + const err = await refusalOf(() => insert(ITEM, { duty: 't_flagged_other' }, vouched(UNREADABLE))); + expect(envelopeOf(err)).toEqual(notFound('duty')); + expect(items()).toEqual([]); + }); + + it('control: the vouched id in another field of the same write is refused as before', async () => { + const err = await refusalOf(() => insert(ITEM, { duty: UNREADABLE, assignee: UNREADABLE }, vouched(UNREADABLE))); + expect(envelopeOf(err)).toEqual(notFound('assignee')); + expect(items()).toEqual([]); + }); + + it('control: the vouched id on a write to another object is refused as before', async () => { + const err = await refusalOf(() => insert(RULED, { duty: UNREADABLE }, vouched(UNREADABLE))); + expect(envelopeOf(err)).toEqual(notFound('duty')); + expect(items(RULED)).toEqual([]); + }); + + it('control: a multi-valued field admits the vouched member only, and refuses another unreadable one beside it', async () => { + const err = await refusalOf(() => insert(ITEM, { duties: [UNREADABLE, 't_flagged_other'] }, vouched(UNREADABLE, 'duties'))); + expect(envelopeOf(err)).toEqual(notFound('duties')); + await insert(ITEM, { duties: [UNREADABLE, 't_mine'] }, vouched(UNREADABLE, 'duties')); + expect(items().some((r) => Array.isArray(r.duties) && r.duties.includes(UNREADABLE))).toBe(true); + }); + + it('control: the same value without the mark is refused, and a string key of the same name is no mark', async () => { + expect(envelopeOf(await refusalOf(() => insert(ITEM, { duty: UNREADABLE }, { ...ALICE })))).toEqual(notFound('duty')); + // What a JSON body or a stored row could spell: the symbol's description as a plain key. + const spelled = { ...ALICE, [String(VOUCHED_REFERENCES.description)]: [{ object: ITEM, field: 'duty', id: UNREADABLE }] }; + expect(envelopeOf(await refusalOf(() => insert(ITEM, { duty: UNREADABLE }, spelled)))).toEqual(notFound('duty')); + expect(items()).toEqual([]); + }); + + it('control: a caller whose read of the target object is refused outright is admitted for the vouched value only', async () => { + await insert(ITEM, { duty: 't_mine' }, vouched('t_mine', 'duty', ITEM, CAROL)); + expect(items().some((r) => r.duty === 't_mine')).toBe(true); + const err = await refusalOf(() => insert(ITEM, { duty: 't_mine_2' }, vouched('t_mine', 'duty', ITEM, CAROL))); + expect(envelopeOf(err)).toEqual(notFound('duty')); + }); +}); diff --git a/packages/qa/dogfood/test/print-service-archive.dogfood.test.ts b/packages/qa/dogfood/test/print-service-archive.dogfood.test.ts index 8dc1209de07..2ee736aa20c 100644 --- a/packages/qa/dogfood/test/print-service-archive.dogfood.test.ts +++ b/packages/qa/dogfood/test/print-service-archive.dogfood.test.ts @@ -26,6 +26,12 @@ // default record page; `showcase_invoice` declares `enable.files` for it, and // its select RLS (`owner == current_user.email`) is what hides another user's // invoice from a contributor. +// +// A second member, `print-attacher`, holds the contributor's set plus the one +// grant an app ships when it lets members attach files: `sys_attachment` +// read and create, and nothing on `sys_file`. Their print archives as THEIR +// write — the attachment names them — while `sys_file` stays unreadable to +// them through the data engine. import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { mkdtempSync, rmSync } from 'node:fs'; @@ -34,6 +40,9 @@ import { join } from 'node:path'; import type { VerifyStack } from '@objectstack/verify'; import { StorageServicePlugin } from '@objectstack/service-storage'; import { AuditPlugin } from '@objectstack/plugin-audit'; +import { SecurityPlugin, appSecurityPluginOptions, securityDefaultPermissionSets } from '@objectstack/plugin-security'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import showcaseStack from '@objectstack/example-showcase'; import { bootShowcase } from './showcase-boot.js'; import { PrintServicePlugin } from '../../../services/service-print/src/print-service-plugin.js'; import { principalSessionMinterOf } from '../../../services/service-print/src/principal-session.js'; @@ -54,6 +63,13 @@ const PAGE = 'showcase_invoice_document'; const PDF = Buffer.from('%PDF-1.4\n% print-service dogfood\n%%EOF\n'); const PRINT_ENV = ['OS_PRINT_CHROMIUM_PATH', 'OS_PRINT_BROWSER_ENDPOINT', 'OS_PRINT_CONSOLE_URL']; +/** The grant an app ships to let members attach files: `sys_attachment` only, nothing on `sys_file`. */ +const attacherSet = PermissionSetSchema.parse({ + name: 'print_dogfood_attacher', + label: 'Print dogfood: attach files', + objects: { sys_attachment: { allowRead: true, allowCreate: true } }, +}); + interface Refusal { code: unknown; status: unknown; @@ -83,6 +99,8 @@ describe('print service on a real showcase boot', () => { })); const renderPdf = vi.fn(async () => PDF); let memberToken: string; + let attacherToken: string; + let attacherInvoice: string; let auditorToken: string; let adminToken: string; let memberEmail: string; @@ -96,6 +114,12 @@ describe('print service on a real showcase boot', () => { } rootDir = mkdtempSync(join(tmpdir(), 'print-dogfood-')); stack = await bootShowcase({ + // The showcase's own composition (`appSecurityPluginOptions`, what the + // harness passes by default), plus the attach grant above. + security: new SecurityPlugin({ + ...appSecurityPluginOptions(showcaseStack), + defaultPermissionSets: [...securityDefaultPermissionSets, attacherSet], + }), extraPlugins: [ new StorageServicePlugin({ adapter: 'local', local: { rootDir }, bindToSettings: false }), new AuditPlugin(), @@ -112,6 +136,13 @@ describe('print service on a real showcase boot', () => { const member = await ql.findOne('sys_user', { where: { email: memberEmail } }, { context: SYS }); await ql.insert('sys_user_permission_set', { user_id: member!.id, permission_set: 'showcase_contributor' }, { context: SYS }); adminToken = await stack.signIn(); + // The member who may attach files: the contributor's set and the attach grant. + const attacherEmail = 'print-attacher@dogfood.test'; + attacherToken = await stack.signUp(attacherEmail); + const attacher = await ql.findOne('sys_user', { where: { email: attacherEmail } }, { context: SYS }); + for (const permission_set of ['showcase_contributor', attacherSet.name]) { + await ql.insert('sys_user_permission_set', { user_id: attacher!.id, permission_set }, { context: SYS }); + } // A reader who may read every invoice and edit none: the app's own `showcase_auditor`. const auditorEmail = 'print-auditor@dogfood.test'; auditorToken = await stack.signUp(auditorEmail); @@ -123,6 +154,7 @@ describe('print service on a real showcase boot', () => { ql.insert('showcase_invoice', { name, owner, account: seeded.account, status: 'draft' }, { context: SYS }); ownInvoice = String((await invoice('INV-PRINT-OWN', memberEmail)).id); othersInvoice = String((await invoice('INV-PRINT-OTHER', 'someone-else@dogfood.test')).id); + attacherInvoice = String((await invoice('INV-PRINT-ATTACHER', attacherEmail)).id); service = new PrintService({ placement: { @@ -246,6 +278,51 @@ describe('print service on a real showcase boot', () => { expect(attachment!.uploaded_by).toBe(ctx.userId); }); + it('a member who may attach files archives their own invoice: the attachment is their write, and sys_file stays unreadable to them', async () => { + const ctx = await stack.contextFor(attacherToken); + // Preconditions: the member reads their invoice, and is refused a read of `sys_file`. + expect(await ql.findOne('showcase_invoice', { where: { id: attacherInvoice } }, { context: ctx })).not.toBeNull(); + expect(await refusalOf(ql.find('sys_file', { limit: 1 }, { context: ctx }))).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); + const out = await service.printRecordToFile(request(attacherInvoice), ctx); + const file = await ql.findOne('sys_file', { where: { id: out.fileId } }, { context: SYS }); + expect(file).toMatchObject({ status: 'committed', mime_type: 'application/pdf', scope: 'attachments', owner_id: ctx.userId }); + const attachment = await ql.findOne('sys_attachment', { where: { id: out.attachmentId } }, { context: SYS }); + expect(attachment).toMatchObject({ + parent_object: 'showcase_invoice', parent_id: attacherInvoice, file_id: out.fileId, uploaded_by: ctx.userId, + }); + // Still not readable after the archive: the print widened no read. + expect(await refusalOf(ql.findOne('sys_file', { where: { id: out.fileId } }, { context: ctx }))).toEqual({ + code: 'PERMISSION_DENIED', + status: 403, + }); + }); + + it('control: the same member’s own sys_attachment insert, naming a file they own and cannot read, is refused reference_not_found', async () => { + const ctx = await stack.contextFor(attacherToken); + // A file the member owns, written by the system as the upload doors write one. + const fileId = 'print_dogfood_attacher_file'; + await ql.insert('sys_file', { + id: fileId, key: `attachments/${fileId}/own.pdf`, name: 'own.pdf', mime_type: 'application/pdf', size: PDF.byteLength, + scope: 'attachments', acl: 'private', status: 'committed', owner_id: ctx.userId, + ...(ctx.tenantId ? { organization_id: ctx.tenantId } : {}), + }, { context: SYS }); + let refusal: { code?: unknown; fields?: Array<{ field?: unknown; code?: unknown }> } | undefined; + try { + await ql.insert( + 'sys_attachment', + { parent_object: 'showcase_invoice', parent_id: attacherInvoice, file_id: fileId, file_name: 'own.pdf' }, + { context: ctx }, + ); + } catch (err) { + refusal = err as typeof refusal; + } + expect(refusal, 'the insert was refused').toBeDefined(); + expect([refusal!.code, (refusal!.fields ?? []).map((f) => [f.field, f.code])]).toEqual([ + 'VALIDATION_FAILED', + [['file_id', 'reference_not_found']], + ]); + }); + it('with the real auth mint: the render loads as the caller, and the session is gone once the print returns', async () => { const ctx = await stack.contextFor(adminToken); const minter = principalSessionMinterOf(stack.kernel.getService('auth')); diff --git a/packages/services/service-storage/src/file-attachment-service.test.ts b/packages/services/service-storage/src/file-attachment-service.test.ts index f85eb0c18c0..e57891523c1 100644 --- a/packages/services/service-storage/src/file-attachment-service.test.ts +++ b/packages/services/service-storage/src/file-attachment-service.test.ts @@ -34,6 +34,7 @@ import { promises as fs } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { ObjectQL } from '@objectstack/objectql'; +import { resolveThrownHttpError } from '@objectstack/types'; import { SqlDriver } from '@objectstack/driver-sql'; import { installAuditWriters } from '@objectstack/plugin-audit'; import { SysAttachment } from '@objectstack/platform-objects/audit'; @@ -520,3 +521,120 @@ describe('the file-attachments writer AS THE REQUESTING USER: assertMayAttach an await expectNothingLeft(ql, storage); }); }); + +describe('attachFileAs for a member who cannot read sys_file: the verb vouches for the one file it created', () => { + // A member as a real deployment holds one: the create grant on + // `sys_attachment` and none on `sys_file`. The middleware below stands in for + // the object-permission layer: a non-system read of `sys_file` is refused + // outright, as an ungranted object read is (`PERMISSION_DENIED` / 403), and + // so is Bob's read of the parent object. The write-time reference check asks + // the caller's read door for every `lookup` value, `sys_attachment.file_id` + // included. + const ALICE = { userId: 'usr_alice', tenantId: 'org_a', positions: ['org_member'] }; + const BOB = { userId: 'usr_bob', tenantId: 'org_a', positions: ['org_member'] }; + const asCaller = (a: IFileAttachmentService) => a as IFileAttachmentService & ICallerFileAttachmentService; + const READS = new Set(['find', 'findOne', 'count', 'aggregate']); + const readRefusal = (object: string) => + Object.assign(new Error(`[Security] Access denied: read on ${object}`), { + name: 'PermissionDeniedError', code: 'PERMISSION_DENIED', status: 403, statusCode: 403, + }); + + async function bootMember(opts: { sharing?: unknown } = {}) { + const booted = await boot(opts); + booted.ql.registerMiddleware(async (ctx: any, next: () => Promise) => { + if (READS.has(ctx.operation) && ctx.context?.isSystem !== true) { + if (ctx.object === 'sys_file') throw readRefusal('sys_file'); + if (ctx.object === 'att_case' && ctx.context?.userId === BOB.userId) throw readRefusal('att_case'); + } + await next(); + }); + return booted; + } + + /** A committed file the member owns, written by the system as the upload doors write one. */ + const seedOwnFile = (ql: ObjectQL) => + ql.insert('sys_file', { + id: 'file_alice', key: 'attachments/file_alice/n.txt', name: 'n.txt', mime_type: 'text/plain', size: 1, + scope: 'attachments', acl: 'private', status: 'committed', owner_id: 'usr_alice', organization_id: 'org_a', + }, SYS); + + it('precondition: the member cannot read sys_file, not even the file they own', async () => { + const { ql } = await bootMember(); + await seedOwnFile(ql); + const err = await refusalOf(ql.findOne('sys_file', { where: { id: 'file_alice' }, context: ALICE })); + expect([err.code, err.status]).toEqual(['PERMISSION_DENIED', 403]); + }); + + it('attaches as the member: one committed sys_file owned by them, one sys_attachment stamped uploaded_by them', async () => { + const { ql, attachments, storage } = await bootMember(); + const out = await asCaller(attachments).attachFileAs(input(), ALICE as never); + const files = await rows(ql, 'sys_file'); + expect(files).toHaveLength(1); + expect(files[0]).toMatchObject({ id: out.fileId, status: 'committed', owner_id: 'usr_alice', organization_id: 'org_a' }); + const links = await rows(ql, 'sys_attachment'); + expect(links).toHaveLength(1); + expect(links[0]).toMatchObject({ + id: out.attachmentId, parent_object: 'att_case', parent_id: 'case_a', file_id: out.fileId, + uploaded_by: 'usr_alice', organization_id: 'org_a', + }); + expect(await storedKeys(storage)).toEqual([files[0].key]); + }); + + it('a producer’s sequence (assertMayAttach, then attachFileAs — the print archive’s) attaches as the member', async () => { + const sharing = { checkEdit: vi.fn(async () => 'allow') }; + const { ql, attachments } = await bootMember({ sharing }); + await asCaller(attachments).assertMayAttach('att_case', 'case_a', ALICE as never); + const out = await asCaller(attachments).attachFileAs(input({ filename: 'printed.pdf', mimeType: 'application/pdf' }), ALICE as never); + const [link] = await rows(ql, 'sys_attachment'); + expect(link).toMatchObject({ id: out.attachmentId, file_id: out.fileId, uploaded_by: 'usr_alice' }); + }); + + it('control: a member the attach rule refuses is still refused, by assertMayAttach and by attachFileAs, and nothing is left', async () => { + const sharing = { checkEdit: vi.fn(async () => 'deny') }; + const { ql, attachments, storage } = await bootMember({ sharing }); + const pre = await refusalOf(asCaller(attachments).assertMayAttach('att_case', 'case_a', ALICE as never)); + expect([pre.code, pre.status]).toEqual(['ATTACHMENT_PARENT_ACCESS', 403]); + const r = await refusalOf(asCaller(attachments).attachFileAs(input(), ALICE as never)); + expect([r.code, r.status]).toEqual(['ATTACHMENT_PARENT_ACCESS', 403]); + await expectNothingLeft(ql, storage); + }); + + it('control: a parent the member cannot read is refused RECORD_NOT_FOUND / 404, and nothing is left', async () => { + const { ql, attachments, storage } = await bootMember(); + const r = await refusalOf(asCaller(attachments).attachFileAs(input(), BOB as never)); + expect([r.code, r.status]).toEqual(['RECORD_NOT_FOUND', 404]); + await expectNothingLeft(ql, storage); + }); + + /** What the member's own `sys_attachment` insert through the data engine answers. */ + async function directInsertRefusal(ql: ObjectQL, fileId: string) { + const err = await refusalOf( + ql.insert( + 'sys_attachment', + { parent_object: 'att_case', parent_id: 'case_a', file_id: fileId, file_name: 'direct.eml' }, + { context: ALICE }, + ), + ); + const envelope = resolveThrownHttpError(err); + return { + code: envelope.code, + status: envelope.status, + fields: ((err as any).fields ?? []).map((f: any) => [f.field, f.code]), + }; + } + const REFERENCE_NOT_FOUND = { code: 'VALIDATION_FAILED', status: 400, fields: [['file_id', 'reference_not_found']] }; + + it('control: the member’s own insert of sys_attachment through the data engine, naming a file they cannot read, is refused reference_not_found', async () => { + const { ql } = await bootMember(); + await seedOwnFile(ql); + expect(await directInsertRefusal(ql, 'file_alice')).toEqual(REFERENCE_NOT_FOUND); + expect(await rows(ql, 'sys_attachment')).toEqual([]); + }); + + it('the vouch is the verb’s call alone: after attachFileAs, the member’s own insert naming the same file is refused reference_not_found', async () => { + const { ql, attachments } = await bootMember(); + const out = await asCaller(attachments).attachFileAs(input(), ALICE as never); + expect(await directInsertRefusal(ql, out.fileId)).toEqual(REFERENCE_NOT_FOUND); + expect(await rows(ql, 'sys_attachment')).toHaveLength(1); + }); +}); diff --git a/packages/services/service-storage/src/file-attachment-service.ts b/packages/services/service-storage/src/file-attachment-service.ts index 9e935c0f318..c8ea9362d3a 100644 --- a/packages/services/service-storage/src/file-attachment-service.ts +++ b/packages/services/service-storage/src/file-attachment-service.ts @@ -12,6 +12,9 @@ import type { IStorageService, } from '@objectstack/spec/contracts'; import type { ExecutionContext } from '@objectstack/spec/kernel'; +// The one reference this verb's caller-scoped insert vouches for: the file it +// created in the same call (see step ⑤). +import { vouchForReference } from '@objectstack/core'; import { StorageMetadataStore, type StorageWriteContext } from './metadata-store.js'; // The attach rule's one verdict (parent EDIT), asked here before a producer // spends work on a file it could not attach. @@ -343,7 +346,18 @@ export class FileAttachmentService implements IFileAttachmentService, ICallerFil mime_type: request.mimeType, size, }, - { context }, + // As the caller, this insert names the one file this call created + // in ③ for the caller, after reading the parent as the caller: the + // call vouches for that file id and for no other value + // (`vouchForReference`, @objectstack/core). The write-time reference + // check then does not ask the caller's read door for it — a caller + // who may attach need not read `sys_file` — and still probes that it + // exists. Every other gate on the insert judges the caller as + // before: the object permission to create an attachment, the attach + // rule (parent EDIT), `enable.files`, the file-reference gate, RLS, + // FLS and the `uploaded_by` stamp. The system context is exempt from + // the read-door question already, so it carries no mark. + { context: caller ? vouchForReference(context, { object: 'sys_attachment', field: 'file_id', id: fileId }) : context }, )) as { id?: unknown } | null | undefined; if (row?.id === undefined || row.id === null || row.id === '') { throw new Error('attachFile: the sys_attachment insert answered no row id, so the attachment cannot be named');