From 4464fd0b813ad13035e042f6ad7c63194cfc12c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 21:37:19 +0000 Subject: [PATCH 01/21] wip: a service verb vouches for the one file reference it created Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- packages/core/src/security/index.ts | 10 ++ .../core/src/security/vouched-reference.ts | 100 ++++++++++++++++++ .../src/engine-referential-integrity.ts | 20 +++- ...engine-unreadable-reference-target.test.ts | 73 ++++++++++++- .../src/file-attachment-service.ts | 16 ++- 5 files changed, 214 insertions(+), 5 deletions(-) create mode 100644 packages/core/src/security/vouched-reference.ts 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..b19817939e9 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 } as any); + 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/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'); From 59d71074dfab92eb3f5160689bfa3a0be24ba656 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 21:42:34 +0000 Subject: [PATCH 02/21] test(service-storage): a member attaches through attachFileAs while sys_file stays unreadable Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../src/file-attachment-service.test.ts | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) 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..9b77280c5b7 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,100 @@ 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; + } + + it('precondition: the member cannot read sys_file, not even the file they own', async () => { + const { ql } = await bootMember(); + await 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); + const err = await refusalOf(ql.findOne('sys_file', { where: { id: 'file_alice' }, context: ALICE } as any)); + 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); + }); + + 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 — the very file the verb attached included', async () => { + const { ql, attachments } = await bootMember(); + const out = await asCaller(attachments).attachFileAs(input(), ALICE as never); + const err = await refusalOf( + ql.insert( + 'sys_attachment', + { parent_object: 'att_case', parent_id: 'case_a', file_id: out.fileId, file_name: 'again.eml' }, + { context: ALICE } as any, + ), + ); + const envelope = resolveThrownHttpError(err); + expect([envelope.code, envelope.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(((err as any).fields ?? []).map((f: any) => [f.field, f.code])).toEqual([['file_id', 'reference_not_found']]); + expect(await rows(ql, 'sys_attachment')).toHaveLength(1); + }); +}); From 98c5826af7f32852b701d3d4d768c123fa9377da Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 21:44:49 +0000 Subject: [PATCH 03/21] test(dogfood): a member who may attach files archives a print as their own write Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../print-service-archive.dogfood.test.ts | 77 +++++++++++++++++++ .../src/file-attachment-service.test.ts | 38 ++++++--- 2 files changed, 106 insertions(+), 9 deletions(-) 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 9b77280c5b7..5adb0a332b6 100644 --- a/packages/services/service-storage/src/file-attachment-service.test.ts +++ b/packages/services/service-storage/src/file-attachment-service.test.ts @@ -551,12 +551,16 @@ describe('attachFileAs for a member who cannot read sys_file: the verb vouches f return booted; } - it('precondition: the member cannot read sys_file, not even the file they own', async () => { - const { ql } = await bootMember(); - await ql.insert('sys_file', { + /** 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 } as any)); expect([err.code, err.status]).toEqual(['PERMISSION_DENIED', 403]); }); @@ -602,19 +606,35 @@ describe('attachFileAs for a member who cannot read sys_file: the verb vouches f await expectNothingLeft(ql, storage); }); - 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 — the very file the verb attached included', async () => { - const { ql, attachments } = await bootMember(); - const out = await asCaller(attachments).attachFileAs(input(), ALICE as never); + /** 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: out.fileId, file_name: 'again.eml' }, + { parent_object: 'att_case', parent_id: 'case_a', file_id: fileId, file_name: 'direct.eml' }, { context: ALICE } as any, ), ); const envelope = resolveThrownHttpError(err); - expect([envelope.code, envelope.status]).toEqual(['VALIDATION_FAILED', 400]); - expect(((err as any).fields ?? []).map((f: any) => [f.field, f.code])).toEqual([['file_id', 'reference_not_found']]); + 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); }); }); From 78b1698943a45bd2a67059a7c64b699e77cc7e61 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 21:54:35 +0000 Subject: [PATCH 04/21] chore(changeset): the attach verb vouches for the one file it created Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .changeset/sec-r16-attach-verb-vouched-file.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .changeset/sec-r16-attach-verb-vouched-file.md 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. From 4e9e098593924567f99c341907027c24abf25829 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 22:28:34 +0000 Subject: [PATCH 05/21] test(objectql, service-storage): type the engine options the new pins pass Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../objectql/src/engine-unreadable-reference-target.test.ts | 2 +- .../service-storage/src/file-attachment-service.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/objectql/src/engine-unreadable-reference-target.test.ts b/packages/objectql/src/engine-unreadable-reference-target.test.ts index b19817939e9..7a2299eee80 100644 --- a/packages/objectql/src/engine-unreadable-reference-target.test.ts +++ b/packages/objectql/src/engine-unreadable-reference-target.test.ts @@ -512,7 +512,7 @@ describe('a reference a service verb vouches for on its own write: that one valu 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 } as any); + 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 () => { 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 5adb0a332b6..e57891523c1 100644 --- a/packages/services/service-storage/src/file-attachment-service.test.ts +++ b/packages/services/service-storage/src/file-attachment-service.test.ts @@ -561,7 +561,7 @@ describe('attachFileAs for a member who cannot read sys_file: the verb vouches f 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 } as any)); + const err = await refusalOf(ql.findOne('sys_file', { where: { id: 'file_alice' }, context: ALICE })); expect([err.code, err.status]).toEqual(['PERMISSION_DENIED', 403]); }); @@ -612,7 +612,7 @@ describe('attachFileAs for a member who cannot read sys_file: the verb vouches f ql.insert( 'sys_attachment', { parent_object: 'att_case', parent_id: 'case_a', file_id: fileId, file_name: 'direct.eml' }, - { context: ALICE } as any, + { context: ALICE }, ), ); const envelope = resolveThrownHttpError(err); From b905481c9664274e12e54bda1a1330053e57affa Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 23:29:14 +0000 Subject: [PATCH 06/21] wip: the upload-complete door declares an attachTo target Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- packages/spec/src/api/storage.zod.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/spec/src/api/storage.zod.ts b/packages/spec/src/api/storage.zod.ts index 8e3603c8edd..bbe8536daf8 100644 --- a/packages/spec/src/api/storage.zod.ts +++ b/packages/spec/src/api/storage.zod.ts @@ -82,6 +82,19 @@ export const CompleteUploadRequestSchema = lazySchema(() => z.object({ + 'that the storage backend holds bytes under the file\'s key and, when the upload declared a size, ' + 'that the held size equals it.', ), + // The attach verb of the record attachments surface: the commit and the + // `sys_attachment` insert run as one unit, as the caller, in this call — + // `FileAttachmentService.attachUploadAs` (`@objectstack/service-storage`). + attachTo: z.object({ + parentObject: z.string().min(1).describe('Object of the record the file is attached to (enable.files: true)'), + parentId: z.string().min(1).describe('Id of the record the file is attached to'), + }).optional().describe( + 'Attach the uploaded file to this record in the same call, as the signed-in caller: the commit and the ' + + 'attachment row are one unit, and the answer carries the new attachmentId. Only the uploader\'s own ' + + 'pending upload in the attachments scope can be attached this way. The attachment is judged as the ' + + 'caller\'s own create: the permission to create an attachment, edit access to the record, and the ' + + 'record object\'s enable.files opt-in. A refused attach leaves the upload pending and uncommitted.', + ), })); // ========================================== @@ -107,7 +120,11 @@ export const PresignedUrlResponseSchema = lazySchema(() => BaseResponseSchema.ex })); export const FileUploadResponseSchema = lazySchema(() => BaseResponseSchema.extend({ - data: FileMetadataSchema.describe('Uploaded file metadata'), + data: FileMetadataSchema.extend({ + attachmentId: z.string().optional().describe( + 'Id of the sys_attachment row the commit created, when the request carried attachTo', + ), + }).describe('Uploaded file metadata'), })); /** From 2d0bb75f5024db1a82b7b057deba0eeeb53a7d43 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 23:32:53 +0000 Subject: [PATCH 07/21] wip: attachUploadAs commits a member's own pending upload and attaches it as them Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- content/docs/references/api/storage.mdx | 9 + .../api.md | 2 +- .../src/file-attachment-service.ts | 211 ++++++++++++++++-- .../service-storage/src/storage-routes.ts | 6 +- packages/spec/authorable-surface/api.json | 1 + 5 files changed, 210 insertions(+), 19 deletions(-) diff --git a/content/docs/references/api/storage.mdx b/content/docs/references/api/storage.mdx index f34c17c4b44..e2916eb0d1a 100644 --- a/content/docs/references/api/storage.mdx +++ b/content/docs/references/api/storage.mdx @@ -103,6 +103,14 @@ const result = CompleteChunkedUploadRequestSchema.parse(data); | :--- | :--- | :--- | :--- | | **fileId** | `string` | ✅ | File ID returned from presigned request | | **eTag** | `string` | optional | ETag the client reports for the uploaded object. Stored on the file record as sent and not compared with anything: no storage adapter reports a stored object's ETag. Before committing, the door checks that the storage backend holds bytes under the file's key and, when the upload declared a size, that the held size equals it. | +| **attachTo** | `{ parentObject: string; parentId: string }` | optional | Attach the uploaded file to this record in the same call, as the signed-in caller: the commit and the attachment row are one unit, and the answer carries the new attachmentId. Only the uploader's own pending upload in the attachments scope can be attached this way. The attachment is judged as the caller's own create: the permission to create an attachment, edit access to the record, and the record object's enable.files opt-in. A refused attach leaves the upload pending and uncommitted. | + +### Nested Shape: `CompleteUploadRequest.attachTo` + +| Property | Type | Required | Description | +| :--- | :--- | :--- | :--- | +| **parentObject** | `string` | ✅ | Object of the record the file is attached to (enable.files: true) | +| **parentId** | `string` | ✅ | Id of the record the file is attached to | --- @@ -211,6 +219,7 @@ const result = CompleteChunkedUploadRequestSchema.parse(data); | **created** | `string` | ✅ | Creation timestamp | | **etag** | `string` | optional | Entity tag | | **fileId** | `string` | optional | Opaque sys_file id (ADR-0104 D3 file-as-reference) | +| **attachmentId** | `string` | optional | Id of the sys_attachment row the commit created, when the request carried attachTo | --- diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.counts/api.md b/docs/audits/2026-07-unknown-key-strictness-ledger.counts/api.md index 3ba6578f2de..12717707e6a 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.counts/api.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.counts/api.md @@ -19,4 +19,4 @@ hand-patch a number here** — fix the code or the verdict and regenerate. | Dir | Sites | |---|---| -| `api/` | 500 | +| `api/` | 501 | diff --git a/packages/services/service-storage/src/file-attachment-service.ts b/packages/services/service-storage/src/file-attachment-service.ts index c8ea9362d3a..90356480366 100644 --- a/packages/services/service-storage/src/file-attachment-service.ts +++ b/packages/services/service-storage/src/file-attachment-service.ts @@ -15,7 +15,10 @@ 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'; +import { StorageMetadataStore, organizationOutOfWriteReach, type StorageWriteContext } from './metadata-store.js'; +// The upload ownership rule, asked of the upload the attach-on-commit verb +// names — the commit door's own rule and its one refusal. +import { isFileUploader, NOT_UPLOADER_CODE, NOT_UPLOADER_MESSAGE, NOT_UPLOADER_STATUS } from './upload-ownership.js'; // The attach rule's one verdict (parent EDIT), asked here before a producer // spends work on a file it could not attach. import { @@ -35,8 +38,9 @@ import { UPLOAD_TOO_LARGE_STATUS, type StorageLimitsSnapshot, } from './storage-limits.js'; -// The upload doors' own storage-key layout. -import { buildKey } from './storage-routes.js'; +// The upload doors' own storage-key layout, and the commit door's question +// of whether the backend holds an upload's bytes. +import { buildKey, heldBytesRefusal } from './storage-routes.js'; /** * [#23306] The server-side attach verb — `IFileAttachmentService`, registered @@ -157,6 +161,42 @@ export interface ICallerFileAttachmentService { * `403`: the system has `attachFile`. */ attachFileAs(input: AttachFileInput, context: ExecutionContext): Promise; + /** + * Attach the caller's own browser upload to a record, and commit it, in one + * unit, as `context`'s user: the attach verb of the record attachments + * surface, asked by the upload-complete door when its request carries + * `attachTo` (`POST /api/v1/storage/upload/complete`). + * + * The file must be the upload the caller is completing: a `sys_file` row + * whose `owner_id` is the caller (the upload doors stamp it from the + * session), still `pending`, in the `attachments` scope, in the caller's + * organization, with its bytes held by the storage backend. Any other id — + * another user's upload, an id that names no file — is refused with the + * upload ownership rule's one answer (`PERMISSION_DENIED` / `403`, the same + * body for both); a committed file `RESOURCE_CONFLICT` / `409`; another + * scope `INVALID_REQUEST` / `400`. + * + * Then the attach authorization, before any write: {@link assertMayAttach} + * (the permission to create an attachment, and the parent-EDIT attach rule), + * and the parent read as the caller (`RECORD_NOT_FOUND` / `404`, as + * {@link attachFileAs} answers). The `sys_attachment` insert runs as the + * caller, so every gate on it judges the caller, and it vouches for this + * one file id and for nothing else (`vouchForReference`, `@objectstack/core`). + * A refusal leaves the upload `pending`, its bytes stored, and no attachment. + */ + attachUploadAs(input: AttachUploadInput, context: ExecutionContext): Promise; +} + +/** What {@link ICallerFileAttachmentService.attachUploadAs} attaches: an upload, and the record it is attached to. */ +export interface AttachUploadInput { + /** The `sys_file` id the upload-start door returned to the caller. */ + fileId: string; + /** The object of the record the file is attached to. */ + parentObject: string; + /** The id of the record the file is attached to. */ + parentId: string; + /** The ETag the client reports for the uploaded bytes, stored on the file as the commit door stores it. */ + eTag?: string; } /** The upload scope of every file this verb writes: the attachments surface. */ @@ -213,6 +253,17 @@ function validateAttachFileInput(input: AttachFileInput): AttachFileInput { return input; } +/** {@link validateAttachFileInput}'s counterpart for {@link AttachUploadInput}. */ +function validateAttachUploadInput(input: AttachUploadInput): AttachUploadInput { + const fail = (message: string): never => refuse('VALIDATION_ERROR', 400, `attachUploadAs: ${message}`); + if (!input || typeof input !== 'object') fail('the input must be an object'); + for (const field of ['fileId', 'parentObject', 'parentId'] as const) { + if (!nonEmptyString(input[field])) fail(`'${field}' must be a non-empty string`); + } + if (input.eTag !== undefined && typeof input.eTag !== 'string') fail("'eTag' must be a string when it is given"); + return input; +} + export class FileAttachmentService implements IFileAttachmentService, ICallerFileAttachmentService { constructor(private readonly deps: FileAttachmentServiceDeps) {} @@ -224,6 +275,119 @@ export class FileAttachmentService implements IFileAttachmentService, ICallerFil return this.attach(input, requireUserContext(context)); } + async attachUploadAs(input: AttachUploadInput, context: ExecutionContext): Promise { + const caller = requireUserContext(context); + const request = validateAttachUploadInput(input); + const engine = this.deps.getEngine(); + const storage = this.deps.getStorage(); + if (!engine || !storage) refuse('SERVICE_UNAVAILABLE', 503, NOT_READY_MESSAGE); + const store = new StorageMetadataStore(engine); + + // ① The upload — the caller's own, as the store stamped it. Read as the + // system, by id, as the commit door reads it; nothing about the row is + // answered before the ownership rule has passed, and a missing row gets + // the same answer as another user's. + const file = await store.getFile(request.fileId); + if (!file || !isFileUploader(caller.userId, file)) { + refuse(NOT_UPLOADER_CODE, NOT_UPLOADER_STATUS, NOT_UPLOADER_MESSAGE); + } + if (file.status !== 'pending') { + refuse( + 'RESOURCE_CONFLICT', + 409, + 'This upload is already completed, so it cannot be attached on completion. An upload is attached in the ' + + 'call that completes it: start a new upload and complete it with attachTo.', + ); + } + if (file.scope !== ATTACHMENTS_SCOPE) { + refuse( + 'INVALID_REQUEST', + 400, + `This upload was started in the '${String(file.scope ?? 'user')}' scope, and only an upload in the ` + + `'${ATTACHMENTS_SCOPE}' scope is attached to a record. Start the upload with scope '${ATTACHMENTS_SCOPE}'.`, + ); + } + const callerOrganization = nonEmptyString(caller.tenantId) ? caller.tenantId : null; + if (organizationOutOfWriteReach(store, file, { organizationId: callerOrganization }) !== null) { + refuse( + 'PERMISSION_DENIED', + 403, + 'attachUploadAs: this upload was started in another organization than the one the caller acts in; ' + + 'switch back to that organization to attach it.', + ); + } + const notHeld = await heldBytesRefusal(storage, file); + if (notHeld !== null) refuse('RESOURCE_CONFLICT', 409, notHeld); + + // ② The attach authorization, before any write: the permission to create + // an attachment and the attach rule (parent EDIT), then the parent read as + // the caller and the organization the attachment is written in. + await this.assertMayAttach(request.parentObject, request.parentId, caller); + const organizationId = await this.organizationOfParent( + engine, + { parentObject: request.parentObject, parentId: request.parentId, organizationId: callerOrganization }, + caller, + ); + const fileOrganization = nonEmptyString(file.organization_id) ? file.organization_id : null; + if (fileOrganization !== null && fileOrganization !== organizationId) { + refuse( + 'RECORD_NOT_FOUND', + 404, + `Cannot attach a file to ${request.parentObject}/${request.parentId}: the record does not exist in the ` + + 'organization the upload was started in.', + ); + } + const writeContext: StorageWriteContext = { organizationId }; + + // ③ The attachment and the commit — one unit, as the caller. + let attachmentId: string | undefined; + let ownedUnit = false; + try { + await this.inOneUnit(engine, organizationId, caller, async (unitContext, owned) => { + ownedUnit = owned; + const row = (await engine.insert( + 'sys_attachment', + { + parent_object: request.parentObject, + parent_id: request.parentId, + file_id: file.id, + file_name: file.name, + mime_type: file.mime_type, + size: file.size, + }, + // As in `attachFileAs`: the call vouches for the one file it judged + // in ① — the caller's own pending upload, which this call commits — + // and for no other value. The write-time reference check does not + // ask the caller's read door for that id, and still probes that it + // exists; every other gate on the insert judges the caller. + { context: vouchForReference(unitContext, { object: 'sys_attachment', field: 'file_id', id: file.id }) }, + )) as { id?: unknown } | null | undefined; + if (row?.id === undefined || row.id === null || row.id === '') { + throw new Error('attachUploadAs: the sys_attachment insert answered no row id, so the attachment cannot be named'); + } + attachmentId = String(row.id); + const committed = await store.updateFile( + file.id, + { status: 'committed', ...(request.eTag !== undefined ? { etag: request.eTag } : {}) }, + writeContext, + ); + if (!committed) { + throw new Error(`attachUploadAs: the sys_file row ${file.id} was gone before it could be committed`); + } + }); + } catch (err) { + // An owned transaction rolled the attachment back with the commit. A + // unit this call does not own keeps an inserted row, so it is removed + // here. The upload itself is the caller's and stays as it was — + // `pending`, its bytes stored — so completing again can still commit it. + if (!ownedUnit && attachmentId !== undefined) { + await this.removeAttachment(engine, attachmentId, file.id, writeContext); + } + throw err; + } + return { fileId: file.id, attachmentId: attachmentId!, size: Number(file.size ?? 0) }; + } + async assertMayAttach(parentObject: string, parentId: string, context: ExecutionContext): Promise { const caller = requireUserContext(context); const engine = this.deps.getEngine(); @@ -393,7 +557,7 @@ export class FileAttachmentService implements IFileAttachmentService, ICallerFil */ private async organizationOfParent( engine: FileAttachmentEngine, - request: AttachFileInput, + request: Pick, caller: ExecutionContext | null, ): Promise { let parent: Record | null; @@ -453,6 +617,31 @@ export class FileAttachmentService implements IFileAttachmentService, ICallerFil await engine.transaction((trxCtx, info) => work(trxCtx as Record, info?.owned === true), base); } + /** + * Remove the `sys_attachment` row a refused call inserted outside a + * transaction it owns. Best-effort: the call's refusal is what is thrown. + */ + private async removeAttachment( + engine: FileAttachmentEngine, + attachmentId: string, + fileId: string, + writeContext: StorageWriteContext, + ): Promise { + try { + const tenant = writeContext.organizationId; + await engine.delete('sys_attachment', { + where: { id: attachmentId }, + context: tenant ? { isSystem: true, tenantId: tenant } : { isSystem: true }, + }); + } catch (err) { + this.deps.logger.warn( + `[storage] attachFile: a refused call could not remove its sys_attachment row ${attachmentId} ` + + `(${(err as Error)?.message ?? err}). It points at the pending sys_file ${fileId}; delete the ` + + 'attachment row by hand.', + ); + } + } + /** * Undo a refused call: the attachment row it still holds (if any), then the * bytes, then the pending row. Best-effort, and the caller's refusal is what @@ -468,19 +657,7 @@ export class FileAttachmentService implements IFileAttachmentService, ICallerFil ): Promise { const { logger } = this.deps; if (call.attachmentId !== undefined) { - try { - const tenant = call.writeContext.organizationId; - await engine.delete('sys_attachment', { - where: { id: call.attachmentId }, - context: tenant ? { isSystem: true, tenantId: tenant } : { isSystem: true }, - }); - } catch (err) { - logger.warn( - `[storage] attachFile: a refused call could not remove its sys_attachment row ${call.attachmentId} ` + - `(${(err as Error)?.message ?? err}). It points at the pending sys_file ${call.fileId}; delete the ` + - 'attachment row by hand.', - ); - } + await this.removeAttachment(engine, call.attachmentId, call.fileId, call.writeContext); } try { await storage.delete(call.key); diff --git a/packages/services/service-storage/src/storage-routes.ts b/packages/services/service-storage/src/storage-routes.ts index 90cd9a7c352..c46c3eee627 100644 --- a/packages/services/service-storage/src/storage-routes.ts +++ b/packages/services/service-storage/src/storage-routes.ts @@ -1972,8 +1972,12 @@ function judgeCompletion(session: UploadSessionRecord, listed: ReadonlyArray { +export async function heldBytesRefusal(storage: IStorageService, file: FileRecord): Promise { const remedy = 'Send the file to its upload URL (start a new upload if that URL has expired), then complete again. ' + 'The file stays pending until then.'; diff --git a/packages/spec/authorable-surface/api.json b/packages/spec/authorable-surface/api.json index 966f6a767ec..1100abac1e8 100644 --- a/packages/spec/authorable-surface/api.json +++ b/packages/spec/authorable-surface/api.json @@ -351,6 +351,7 @@ "api/CompleteChunkedUploadResponse:error", "api/CompleteChunkedUploadResponse:meta", "api/CompleteChunkedUploadResponse:success", + "api/CompleteUploadRequest:attachTo", "api/CompleteUploadRequest:eTag", "api/CompleteUploadRequest:fileId", "api/ConceptListResponse:data", From 53eef2e793dcd91fdb06c56baf26eed831d8546f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 23:34:38 +0000 Subject: [PATCH 08/21] wip: the upload-complete door attaches the caller's own upload when the request names attachTo Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../src/storage-route-ledger.ts | 2 +- .../service-storage/src/storage-routes.ts | 168 ++++++++++++++++-- .../src/storage-service-plugin.ts | 85 ++++++++- 3 files changed, 237 insertions(+), 18 deletions(-) diff --git a/packages/services/service-storage/src/storage-route-ledger.ts b/packages/services/service-storage/src/storage-route-ledger.ts index 7857e81c281..91a5eeb1671 100644 --- a/packages/services/service-storage/src/storage-route-ledger.ts +++ b/packages/services/service-storage/src/storage-route-ledger.ts @@ -99,7 +99,7 @@ export const STORAGE_ROUTE_LEDGER: readonly StorageRouteLedgerEntry[] = [ { route: 'POST /api/v1/storage/upload/presigned', family: 'upload-presigned', disposition: 'sdk', client: 'storage.getPresignedUrl', note: 'also step 1 of the composite storage.upload' }, { route: 'POST /api/v1/storage/upload/complete', family: 'upload-presigned', disposition: 'sdk', client: 'storage.upload', - note: 'step 3 of the composite upload; no standalone client method mints the commit on its own' }, + note: 'step 3 of the composite upload; no standalone client method mints the commit on its own. With attachTo it also attaches the upload to a record as the caller (the console attachments panel); the SDK sends none' }, // ── chunked / resumable upload ──────────────────────────────────────────── { route: 'POST /api/v1/storage/upload/chunked', family: 'upload-chunked', disposition: 'sdk', client: 'storage.initChunkedUpload' }, diff --git a/packages/services/service-storage/src/storage-routes.ts b/packages/services/service-storage/src/storage-routes.ts index c46c3eee627..8bacdb79bf9 100644 --- a/packages/services/service-storage/src/storage-routes.ts +++ b/packages/services/service-storage/src/storage-routes.ts @@ -12,6 +12,9 @@ import { InitiateChunkedUploadRequestSchema, UploadScopeSchema } from '@objectst // [#23174] A catch-all 5xx answers through `sendServerFault`: the route's // generic text on the wire, the caught error with its stack in the fault log. import { sendOk, sendError, sendServerFault } from '@objectstack/types'; +// The commit door's attach arm relays the attach verb's declared refusals as +// the print door relays its service's: the producer's code and status. +import { declaredRefusalMessage, demotedDeclaredCode, resolveThrownHttpError } from '@objectstack/types'; // [#15999] The BRAND predicate, never `instanceof` — this error crosses package // boundaries and a monorepo resolves the same module through more than one path // (`src` under vitest aliases, `dist` under the published `exports`), so two @@ -435,7 +438,8 @@ export interface StorageRoutesOptions { * * Routes: * - POST /storage/upload/presigned → get presigned upload URL - * - POST /storage/upload/complete → mark upload as committed + * - POST /storage/upload/complete → mark upload as committed (and, with + * `attachTo`, attach it to a record as the caller) * - POST /storage/upload/chunked → initiate chunked upload * - PUT /storage/upload/chunked/:uploadId/chunk/:chunkIndex → upload a chunk * - POST /storage/upload/chunked/:uploadId/complete → complete chunked @@ -468,6 +472,60 @@ export function registerStorageRoutes( */ export type StorageGatedSessionReader = (req: IHttpRequest) => Promise; +/** What the commit door's `attachTo` names: the upload, and the record it is attached to. */ +export interface StorageUploadAttachment { + fileId: string; + parentObject: string; + parentId: string; + eTag?: string; +} + +/** + * Package-internal: attach the request's caller's own pending upload to a + * record and commit it, as that caller — the commit door's `attachTo` arm. + * Resolves the request's FULL execution context (the one request-to-context + * composition, as the download authorizer does) and hands it to the + * file-attachments service's caller-scoped `attachUploadAs`, which asks every + * check and vouches for that one file. It throws what that verb throws, with + * its declared `code` and `status`. + * + * Bound by `composeStorageRoutes` alone, like every other storage gate, and a + * parameter of {@link registerStorageFamily} rather than a + * `StorageRoutesOptions` key, so no exported declaration moves and no host can + * hand the door an attacher of its own. `undefined` (no `auth` service, or no + * data engine): the door refuses an `attachTo` with `501`, and a completion + * without one is unchanged. + */ +export type StorageUploadAttacher = ( + req: IHttpRequest, + upload: StorageUploadAttachment, +) => Promise<{ attachmentId: string }>; + +/** The body keys of `attachTo` — `CompleteUploadRequestSchema.attachTo` (`@objectstack/spec/api`). */ +const ATTACH_TO_FIELDS = ['parentObject', 'parentId'] as const; + +/** + * The commit door's `attachTo`: `null` when the request carries none, the + * target when it is well formed, or the sentence a `400` answers. + */ +function attachTargetOf(value: unknown): { parentObject: string; parentId: string } | null | string { + if (value === undefined) return null; + const shape = + `attachTo is an object with the fields ${ATTACH_TO_FIELDS.join(' and ')}, each a non-empty string: ` + + 'the record the upload is attached to.'; + if (value === null || typeof value !== 'object' || Array.isArray(value)) return shape; + const fields = value as Record; + const known = new Set(ATTACH_TO_FIELDS); + const unknown = Object.keys(fields).filter((key) => !known.has(key)); + if (unknown.length > 0) { + return `${unknown.map((k) => `'${k}'`).join(', ')} ${unknown.length === 1 ? 'is not a field' : 'are not fields'} of attachTo. ${shape}`; + } + const { parentObject, parentId } = fields; + if (typeof parentObject !== 'string' || parentObject.length === 0) return shape; + if (typeof parentId !== 'string' || parentId.length === 0) return shape; + return { parentObject, parentId }; +} + /** * Package-internal: register the storage family through ONE door, which asks * the ADR-0069 gate before any route runs (see `mountFamilyRoute` below). @@ -485,6 +543,7 @@ export function registerStorageFamily( store: StorageMetadataStore, opts: StorageRoutesOptions, readGatedSessionUser: StorageGatedSessionReader | undefined, + attachUpload: StorageUploadAttacher | undefined = undefined, ): void { const basePath = opts.basePath ?? '/api/v1/storage'; const downloadTtl = opts.downloadTtl ?? 300; @@ -1053,11 +1112,17 @@ export function registerStorageFamily( // statement unscoped and raising `[tenant-audit]`. const session = await requireUploadSession(req, res); if (session === false) return; - const { fileId, eTag } = req.body ?? {}; + const { fileId, eTag, attachTo } = req.body ?? {}; if (!fileId) { sendError(res, 400, 'INVALID_REQUEST', 'fileId is required'); return; } + // The attach arm's target, judged before anything is read. + const attach = attachTargetOf(attachTo); + if (typeof attach === 'string') { + sendError(res, 400, 'INVALID_REQUEST', attach); + return; + } const file = await store.getFile(fileId); if (!file) { @@ -1082,6 +1147,43 @@ export function registerStorageFamily( return; } + // The attach arm: the record attachments surface's attach verb. The + // commit and the `sys_attachment` insert are one unit, run as the caller + // by the file-attachments service (`attachUploadAs`), which asks the + // ownership rule again, the upload's state and scope, the permission to + // create an attachment, the parent-EDIT attach rule and the parent read, + // and vouches for this one file on its own insert — so a member who may + // attach need not read `sys_file`. ⛔ The generic data door gets no such + // mark: a `sys_attachment` create through it naming a file the caller + // cannot read is still refused `reference_not_found`. + if (attach) { + if (!attachUpload || session === null) { + sendError( + res, + 501, + 'NOT_IMPLEMENTED', + 'This storage mount cannot attach an upload as its caller: the kernel composes no auth service or no ' + + 'data engine. Complete the upload without attachTo.', + ); + return; + } + let attached: { attachmentId: string }; + try { + attached = await attachUpload(req, { + fileId: String(fileId), + parentObject: attach.parentObject, + parentId: attach.parentId, + ...(eTag !== undefined && eTag !== null ? { eTag } : {}), + }); + } catch (err) { + relayAttachFailure(res, err, `${basePath}/upload/complete`); + return; + } + const attachedFile = (await store.getFile(fileId)) ?? file; + sendOk(res, { ...committedFileMetadata(attachedFile, fileId), attachmentId: attached.attachmentId }); + return; + } + const updated = await store.updateFile( fileId, { @@ -1091,19 +1193,7 @@ export function registerStorageFamily( writeContext, ); - sendOk(res, { - // The opaque sys_file id — the value a file field stores as a - // reference (ADR-0104 D3). Previously omitted, so a caller could not - // learn the id to persist after committing an upload. - fileId: updated!.id ?? fileId, - path: updated!.key, - name: updated!.name, - size: updated!.size ?? 0, - mimeType: updated!.mime_type ?? 'application/octet-stream', - lastModified: updated!.updated_at ?? new Date().toISOString(), - created: updated!.created_at ?? new Date().toISOString(), - etag: updated!.etag, - }); + sendOk(res, committedFileMetadata(updated!, fileId)); } catch (err: any) { sendServerFault(res, 500, 'INTERNAL', 'Internal error', err); } @@ -1995,6 +2085,54 @@ export async function heldBytesRefusal(storage: IStorageService, file: FileRecor ); } +/** + * The commit door's answer for a committed file — `FileUploadResponseSchema`'s + * `data` (`@objectstack/spec/api`). + */ +function committedFileMetadata(file: FileRecord, fileId: string): Record { + return { + // The opaque sys_file id — the value a file field stores as a + // reference (ADR-0104 D3). Previously omitted, so a caller could not + // learn the id to persist after committing an upload. + fileId: file.id ?? fileId, + path: file.key, + name: file.name, + size: file.size ?? 0, + mimeType: file.mime_type ?? 'application/octet-stream', + lastModified: file.updated_at ?? new Date().toISOString(), + created: file.created_at ?? new Date().toISOString(), + etag: file.etag, + }; +} + +/** + * The commit door's relay for a refused or failed attach: a declared `4xx` — + * the attach verb's own refusals, the `sys_attachment` insert's gates (the + * permission to create, `ATTACHMENT_PARENT_ACCESS`, `FILES_DISABLED`, a + * `VALIDATION_FAILED` with its field details) — keeps its code, status, + * message and details; a declared `5xx` keeps its code and status with this + * door's generic text unless the producer declared the text for its caller; + * anything else is a fault. + */ +function relayAttachFailure(res: IHttpResponse, err: unknown, path: string): void { + const thrown = resolveThrownHttpError(err, 500); + if (thrown.status < 500) { + const declaredCode = demotedDeclaredCode(thrown); + sendError(res, thrown.status, thrown.code, thrown.message, { + ...(thrown.details ? { details: thrown.details } : {}), + ...(declaredCode !== undefined ? { declaredCode } : {}), + ...(thrown.userMessage !== undefined ? { userMessage: thrown.userMessage } : {}), + }); + return; + } + const refusal = declaredRefusalMessage(err); + if (refusal !== undefined) { + sendError(res, thrown.status, thrown.code, refusal); + return; + } + sendServerFault(res, thrown.status, thrown.code, 'Internal error', err, { method: 'POST', path }); +} + /** * [#24548] Whether a local adapter read failed because no file is stored under * the key: Node's `ENOENT`, which `LocalStorageAdapter.download` passes through diff --git a/packages/services/service-storage/src/storage-service-plugin.ts b/packages/services/service-storage/src/storage-service-plugin.ts index 59063b6c259..c01dbb103b0 100644 --- a/packages/services/service-storage/src/storage-service-plugin.ts +++ b/packages/services/service-storage/src/storage-service-plugin.ts @@ -60,7 +60,12 @@ import { StorageMetadataStore, releasingRecordOf } from './metadata-store.js'; import type { FileRecord } from './metadata-store.js'; import type { TenancyPosture } from '@objectstack/spec/security'; import { registerStorageFamily } from './storage-routes.js'; -import type { FileReadVerdict, StorageGatedSessionReader, StorageUploadSession } from './storage-routes.js'; +import type { + FileReadVerdict, + StorageGatedSessionReader, + StorageUploadAttacher, + StorageUploadSession, +} from './storage-routes.js'; import { installAttachmentLifecycleHooks, createSysFileReapGuard, createUploadSessionReapGuard, findFileHolder, findHeldFiles } from './attachment-lifecycle.js'; import { installFileReferenceHooks, type FileReferenceEngine } from './file-reference-lifecycle.js'; import { installSysFileStorageOwnedColumnsGuard } from './sys-file-storage-owned-columns.js'; @@ -71,7 +76,11 @@ import { installAttachmentReadVisibility, } from './attachment-access-hooks.js'; import { contributeAttachmentDeleteFloorAlternate } from './attachment-delete-floor-alternate.js'; -import { FileAttachmentService, type FileAttachmentEngine } from './file-attachment-service.js'; +import { + FileAttachmentService, + type FileAttachmentEngine, + type ICallerFileAttachmentService, +} from './file-attachment-service.js'; import { FieldFileService } from './field-file-service.js'; import { SystemFile, SystemUploadSession } from './objects/index.js'; // ADR-0052 §3 ownership: `sys_attachment` (a file↔record link) belongs with the @@ -1183,6 +1192,7 @@ export function composeStorageRoutes( const resolveSession = buildAuthSessionResolver(registry); const readGatedSessionUser = buildAuthGateSessionReader(registry, engine); const authorizeFileRead = buildFileReadAuthorizer(registry, engine); + const attachUpload = buildUploadAttacher(registry, engine); const resolveFileHolder = engine && typeof (engine as any).find === 'function' ? (file: FileRecord) => findFileHolder(engine as any, file.id, file as any) @@ -1215,6 +1225,8 @@ export function composeStorageRoutes( }, // ADR-0069 — bound here with the other gates, so both mounts carry it. readGatedSessionUser, + // The commit door's `attachTo` arm — bound here with the other gates. + attachUpload, ); return { @@ -1606,6 +1618,75 @@ function buildFileReadAuthorizer( }; } +/** + * The commit door's `attachTo` arm (`POST …/upload/complete`): attach the + * caller's own pending upload to a record and commit it, AS THE CALLER — the + * record attachments surface's attach verb, which the browser panel calls + * instead of a `sys_attachment` create through the generic data door. + * + * The caller is the request's FULL execution context, resolved through the one + * request-to-context composition with the download authorizer's parameters + * (the attach is a write judged by the caller's grants, RLS and sharing, so a + * bare `{ userId }` would not do), and the verb is the file-attachments + * service's caller-scoped `attachUploadAs`, looked up per request: it asks + * every check, and its `sys_attachment` insert vouches for that one file id + * and for nothing else. A request that resolves no user is refused + * `AUTH_REQUIRED` / `401`; a kernel whose file-attachments service is not + * registered, `SERVICE_UNAVAILABLE` / `503`. Everything else is the verb's own + * answer, thrown with its declared `code` and `status` for the door to relay. + * + * `undefined` — the door then refuses `attachTo` with `501` — when the kernel + * has no `auth` service or no data engine: there is no caller to attach as. + */ +function buildUploadAttacher( + registry: StorageGateRegistry, + engine: IDataEngine | null, +): StorageUploadAttacher | undefined { + const authService = resolveGateAuthService(registry); + if (!authService || !engine) return undefined; + // Declared refusals (`refusal: true`): their text is written for the caller. + const refusal = (status: number, code: string, message: string) => + Object.assign(new Error(message), { status, code, refusal: true }); + return async (req, upload) => { + const headers = toWebHeaders(req); + // The download authorizer's parameters (see `buildFileReadAuthorizer`): + // the family's door has already asked the ADR-0069 gate on this request. + const context = headers + ? await resolveRequestExecutionContext({ + headers, + ql: engine, + authService, + tenancy: tenancyServiceResolver(registry), + anonymous: 'fail-closed', + authGate: 'resolved', + credentials: 'api-key-and-session', + sessionApi: 'optional', + session: 'request', + localization: 'withhold', + requestLocale: undefined, + accessToken: 'carry', + oauth: undefined, + onAdmission: undefined, + }) + : undefined; + if (!context?.userId) throw refusal(401, 'AUTH_REQUIRED', 'Authentication required to attach a file'); + let service: Partial | undefined; + try { + service = registry.getService>('file-attachments'); + } catch { + service = undefined; + } + if (!service || typeof service.attachUploadAs !== 'function') { + throw refusal( + 503, + 'SERVICE_UNAVAILABLE', + 'The file-attachments service is not registered in this kernel, so an upload cannot be attached.', + ); + } + return service.attachUploadAs(upload, context); + }; +} + /** * The download door's verdict on ONE file for a caller it has already * resolved — every arm after the door's own `'unauthenticated'` answer. Read From 4f78f5422a9df7fa92face0e6a491d6dd1e6849f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 23:39:02 +0000 Subject: [PATCH 09/21] wip: the vouch is spent on the verb's own write, and attachUploadAs vouches only for an unheld upload Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- packages/core/src/security/index.ts | 1 + .../core/src/security/vouched-reference.ts | 28 +++++++++-- .../src/engine-defaults-and-autonumbers.ts | 7 ++- ...engine-unreadable-reference-target.test.ts | 49 +++++++++++++++++++ .../src/file-attachment-service.ts | 21 ++++++-- 5 files changed, 99 insertions(+), 7 deletions(-) diff --git a/packages/core/src/security/index.ts b/packages/core/src/security/index.ts index 5095f0d2da8..16dd237b8f2 100644 --- a/packages/core/src/security/index.ts +++ b/packages/core/src/security/index.ts @@ -544,6 +544,7 @@ export { VOUCHED_REFERENCES, vouchForReference, isVouchedReference, + withoutVouchedReferences, type VouchedReference, } from './vouched-reference.js'; diff --git a/packages/core/src/security/vouched-reference.ts b/packages/core/src/security/vouched-reference.ts index 65d934a2d80..66a21a5d099 100644 --- a/packages/core/src/security/vouched-reference.ts +++ b/packages/core/src/security/vouched-reference.ts @@ -46,9 +46,18 @@ * 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. + * `ExecutionContextSchema.parse`. + * + * ## It is spent on the verb's own write — no hook inherits it + * + * The mark rides a spread of the context as any own property does, and every + * context a hook writes through is derived from the write that fired it. So + * the engine strips it where it builds a hook's api from the write's context + * (`buildHookApi`, objectql, via {@link withoutVouchedReferences}): a hook — + * app-authored or platform, `runAs: 'user'` or inherited — that writes the same + * object, field and id under the vouched write is judged through its caller's + * read door like any other write. The mark admits the one write the verb + * makes, and no write that write sets off. */ /** The context key of the mark. See the module header. */ @@ -98,3 +107,16 @@ export function isVouchedReference(context: unknown, object: unknown, field: unk && (r as VouchedReference).id === value, ); } + +/** + * `context` without the mark: the same object when it carries none, else a + * fresh copy with every other own property. The engine calls it where it hands + * a hook the write's context (see the module header), so the vouch is spent + * on the verb's own write. + */ +export function withoutVouchedReferences(context: T): T { + if (context === null || typeof context !== 'object') return context; + if (!Object.prototype.hasOwnProperty.call(context, VOUCHED_REFERENCES)) return context; + const { [VOUCHED_REFERENCES]: _spent, ...rest } = context as T & { [VOUCHED_REFERENCES]?: unknown }; + return rest as T; +} diff --git a/packages/objectql/src/engine-defaults-and-autonumbers.ts b/packages/objectql/src/engine-defaults-and-autonumbers.ts index 28fa8c99bcf..db8fdee18c2 100644 --- a/packages/objectql/src/engine-defaults-and-autonumbers.ts +++ b/packages/objectql/src/engine-defaults-and-autonumbers.ts @@ -37,6 +37,7 @@ import { isCurrentUserDefaultToken, isNowDefaultToken, missingFieldValues, parseAutonumberFormat, renderAutonumber, resolveAutonumberFormat } from '@objectstack/spec/data'; import type { EngineQueryOptions } from '@objectstack/spec/data'; import type { IDataEngine } from '@objectstack/core'; +import { withoutVouchedReferences } from '@objectstack/core'; import { isUniqueViolationError, keysetWalk, uniqueViolationColumn } from '@objectstack/types'; import type { KeysetPageQuery } from '@objectstack/types'; import type { ExecutionContext } from '@objectstack/spec/kernel'; @@ -202,7 +203,11 @@ export class DefaultsAndAutonumbersFacet { * is supplied (e.g. system-triggered hooks). */ static buildHookApi(this: Pick, execCtx?: ExecutionContext): ScopedContext { - const safeCtx: ExecutionContext = execCtx ?? ({ isSystem: true } as any); + // A reference a service verb vouched for on the write that fired this hook + // is spent on that write: no context a hook writes through carries it + // (`withoutVouchedReferences`, @objectstack/core). A context with no mark + // is handed through as the same object. + const safeCtx: ExecutionContext = withoutVouchedReferences(execCtx ?? ({ isSystem: true } as any)); return new ScopedContext(safeCtx, this.facade as unknown as IDataEngine); } diff --git a/packages/objectql/src/engine-unreadable-reference-target.test.ts b/packages/objectql/src/engine-unreadable-reference-target.test.ts index 7a2299eee80..30d50c6094e 100644 --- a/packages/objectql/src/engine-unreadable-reference-target.test.ts +++ b/packages/objectql/src/engine-unreadable-reference-target.test.ts @@ -562,6 +562,55 @@ describe('a reference a service verb vouches for on its own write: that one valu expect(items()).toEqual([]); }); + describe('the mark is spent on the vouched write: a hook that runs as the user under it is judged as any write is', () => { + /** + * An afterInsert hook on the task that, under the vouched insert, inserts + * a second task naming the SAME vouched id in the SAME field — through the + * api it is handed (the triggering user's context), or through + * `withRunAs('user')`, as an app-authored `runAs: 'user'` hook does. Its + * refusal is captured, and the outer write is left to finish. + */ + const armEchoHook = (via: 'api' | 'runAs-user') => { + const refusals: unknown[] = []; + engine.registerHook('afterInsert', async (ctx: any) => { + if (ctx.input?.data?.title !== 'outer') return; + const api = via === 'api' ? ctx.api : ctx.api.withRunAs('user', { hook: 'echo', object: ITEM, event: 'afterInsert' }); + try { + await api.object(ITEM).insert({ title: 'echo', duty: UNREADABLE }); + } catch (err) { + refusals.push(err); + } + }, { object: ITEM }); + return refusals; + }; + + for (const via of ['api', 'runAs-user'] as const) { + it(`a hook writing through ${via === 'api' ? 'the api it is handed' : "withRunAs('user')"} that names the vouched id again is refused reference_not_found`, async () => { + const refusals = armEchoHook(via); + await insert(ITEM, { title: 'outer', duty: UNREADABLE }, vouched(UNREADABLE)); + expect(refusals).toHaveLength(1); + expect(envelopeOf(refusals[0])).toEqual(notFound('duty')); + // The verb's own write landed; the hook's did not. + expect(items().map((r) => r.title)).toEqual(['outer']); + }); + } + + it('control: the same hook under an unmarked system write lands its echo (the hook\'s refusal above is the mark being spent, not the hook failing)', async () => { + const refusals: unknown[] = []; + engine.registerHook('afterInsert', async (ctx: any) => { + if (ctx.input?.data?.title !== 'outer') return; + try { + await ctx.api.object(ITEM).insert({ title: 'echo', duty: UNREADABLE }); + } catch (err) { + refusals.push(err); + } + }, { object: ITEM }); + await engine.insert(ITEM, { title: 'outer', duty: UNREADABLE }, { context: { isSystem: true } }); + expect(refusals).toEqual([]); + expect(items().map((r) => r.title).sort()).toEqual(['echo', 'outer']); + }); + }); + 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); diff --git a/packages/services/service-storage/src/file-attachment-service.ts b/packages/services/service-storage/src/file-attachment-service.ts index 90356480366..4dc9d0df7ee 100644 --- a/packages/services/service-storage/src/file-attachment-service.ts +++ b/packages/services/service-storage/src/file-attachment-service.ts @@ -19,6 +19,9 @@ import { StorageMetadataStore, organizationOutOfWriteReach, type StorageWriteCon // The upload ownership rule, asked of the upload the attach-on-commit verb // names — the commit door's own rule and its one refusal. import { isFileUploader, NOT_UPLOADER_CODE, NOT_UPLOADER_MESSAGE, NOT_UPLOADER_STATUS } from './upload-ownership.js'; +// The one definition of "is anything holding this file?" — the reap guard's — +// asked so the attach-on-commit verb vouches only for an upload nothing holds. +import { findFileHolder } from './attachment-lifecycle.js'; // The attach rule's one verdict (parent EDIT), asked here before a producer // spends work on a file it could not attach. import { @@ -170,11 +173,15 @@ export interface ICallerFileAttachmentService { * The file must be the upload the caller is completing: a `sys_file` row * whose `owner_id` is the caller (the upload doors stamp it from the * session), still `pending`, in the `attachments` scope, in the caller's - * organization, with its bytes held by the storage backend. Any other id — + * organization, held by nothing yet (no `sys_attachment` names it and no + * record field owns it — `findFileHolder`), with its bytes held by the + * storage backend. That is the whole premise of the vouch: the one file this + * call turns from the caller's unattached upload into an attachment, and + * nothing else. Any other id — * another user's upload, an id that names no file — is refused with the * upload ownership rule's one answer (`PERMISSION_DENIED` / `403`, the same - * body for both); a committed file `RESOURCE_CONFLICT` / `409`; another - * scope `INVALID_REQUEST` / `400`. + * body for both); a committed or already-held file `RESOURCE_CONFLICT` / + * `409`; another scope `INVALID_REQUEST` / `400`. * * Then the attach authorization, before any write: {@link assertMayAttach} * (the permission to create an attachment, and the parent-EDIT attach rule), @@ -316,6 +323,14 @@ export class FileAttachmentService implements IFileAttachmentService, ICallerFil 'switch back to that organization to attach it.', ); } + if ((await findFileHolder(engine, file.id, file as unknown as Record)) !== null) { + refuse( + 'RESOURCE_CONFLICT', + 409, + 'This upload is already attached to a record or owned by a record field, so it cannot be attached on ' + + 'completion. Start a new upload and complete it with attachTo.', + ); + } const notHeld = await heldBytesRefusal(storage, file); if (notHeld !== null) refuse('RESOURCE_CONFLICT', 409, notHeld); From e0b4c21582f89db87e933a89639c5f5369e474e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 23:42:15 +0000 Subject: [PATCH 10/21] wip: a census gate pins the call sites that set the vouched-reference mark Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- scripts/check-vouch-for-reference-callers.mjs | 286 ++++++++++++++++++ 1 file changed, 286 insertions(+) create mode 100644 scripts/check-vouch-for-reference-callers.mjs diff --git a/scripts/check-vouch-for-reference-callers.mjs b/scripts/check-vouch-for-reference-callers.mjs new file mode 100644 index 00000000000..57cf0999fa5 --- /dev/null +++ b/scripts/check-vouch-for-reference-callers.mjs @@ -0,0 +1,286 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Call-site census of the vouched-reference mark. + * + * node scripts/check-vouch-for-reference-callers.mjs + * node scripts/check-vouch-for-reference-callers.mjs --self-test + * + * ## The rule it holds + * + * 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 + * (`assertReferencesResolve`, objectql). The one exception is a reference a + * service verb vouches for on its own write (`vouchForReference`, + * `@objectstack/core`): a record the same call authorized for the caller, set + * only after that verb's own authorization, and never on a context a request, + * an import, a flow or an agent tool hands the engine. Whether a call site + * honours that is a judgement a reviewer makes once, when the call site is + * written. This gate makes sure no call site arrives without that review: every + * production source file that sets the mark is named in {@link ALLOWED_CALLERS} + * with the exact number of call sites it holds, and the gate is red when a file + * outside the list sets it, when a listed file holds more or fewer call sites + * than its row, and when a row names a file that no longer sets it. + * + * "Sets the mark" is decided three ways, each a spelling that reaches the same + * context key: + * + * 1. a call `vouchForReference(` (not a mention in a comment or in backticks); + * 2. an import that renames it (`vouchForReference as …`), which would hide + * every later call from (1); + * 3. the key itself, outside the module that defines it — `VOUCHED_REFERENCES` + * used as a computed property, or `Symbol.for('objectstack.core.vouchedReferences')` + * spelled anywhere else — a context built with the key by hand. + * + * Test files (`*.test.*`, `*.spec.*`, `*.testkit.*`, `__tests__/`) are outside + * the census: they set the mark on purpose to pin what it admits. + * + * The scan surface is git's (`git ls-files --cached --others --exclude-standard` + * under `packages/`), so build output never reads as a call site. + * + * Adding a caller is an edit to {@link ALLOWED_CALLERS} in the same change, + * with the reason in the row: what the verb created or authorized for the + * caller, and which authorization it asked first. + */ + +import { execFileSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { isEntrypoint } from './invoked-as.mjs'; + +const REPO = join(dirname(fileURLToPath(import.meta.url)), '..'); + +/** The module that defines the mark; its own spellings are not call sites. */ +const DEFINING_MODULE = 'packages/core/src/security/vouched-reference.ts'; + +/** + * Every production file that sets the mark, with its exact call-site count and + * the reason it may. Equality, not a ceiling: one more call site is a new + * vouch nobody reviewed, one fewer is a row that no longer says what the file + * does. + */ +export const ALLOWED_CALLERS = Object.freeze({ + 'packages/services/service-storage/src/file-attachment-service.ts': { + count: 2, + reason: + 'the file-attachments writer: attachFileAs vouches for the sys_file it created in the same call, and ' + + "attachUploadAs for the caller's own unheld pending upload it commits in the same unit — each only for " + + 'sys_attachment.file_id, after that verb\'s own attach authorization', + }, +}); + +const SOURCE_FILE = /\.(?:[cm]?ts|tsx|[cm]?js|jsx)$/; +const TEST_FILE = /(?:\.test\.|\.spec\.|\.testkit\.|\/__tests__\/|\/__fixtures__\/)/; + +/** A line that is a comment, by its first non-blank characters. */ +const isCommentLine = (line) => /^\s*(?:\/\/|\/\*|\*)/.test(line); + +/** Strip backtick-quoted spans, so prose naming `vouchForReference(` is no call. */ +const withoutBacktickSpans = (line) => line.replace(/`[^`]*`/g, '``'); + +/** + * The census of one file's text: its call sites and the spellings that would + * hide or bypass one. + */ +export function censusOf(path, text) { + let calls = 0; + const findings = []; + const lines = String(text).split('\n'); + lines.forEach((raw, index) => { + if (isCommentLine(raw)) return; + const line = withoutBacktickSpans(raw.replace(/\/\/.*$/, '')); + const where = `${path}:${index + 1}`; + const callHits = line.match(/\bvouchForReference\s*\(/g); + if (callHits && path !== DEFINING_MODULE) calls += callHits.length; + if (/\bvouchForReference\s+as\s+\w/.test(line)) { + findings.push(`${where}: renames vouchForReference on import — every later call would hide from this census`); + } + if (path === DEFINING_MODULE) return; + if (/\[\s*VOUCHED_REFERENCES\s*\]/.test(line)) { + findings.push(`${where}: builds the mark's key by hand ([VOUCHED_REFERENCES]) — call vouchForReference instead`); + } + if (/Symbol\.for\(\s*['"]objectstack\.core\.vouchedReferences['"]\s*\)/.test(line)) { + findings.push(`${where}: spells the mark's symbol by hand — call vouchForReference instead`); + } + }); + return { calls, findings }; +} + +/** The verdict over a set of `{ path, text }` files against `allowed`. */ +export function check(files, allowed = ALLOWED_CALLERS) { + const problems = []; + if (files.length === 0) { + problems.push('the scan found no source files at all — an empty scan proves nothing, so it is refused'); + return { problems, counts: new Map() }; + } + const counts = new Map(); + for (const { path, text } of files) { + if (!SOURCE_FILE.test(path) || TEST_FILE.test(path)) continue; + const { calls, findings } = censusOf(path, text); + problems.push(...findings); + if (calls > 0) counts.set(path, calls); + } + for (const [path, calls] of counts) { + const row = allowed[path]; + if (!row) { + problems.push( + `${path}: sets the vouched-reference mark ${calls} time(s) and is not in ALLOWED_CALLERS. A vouch lets one ` + + "reference skip the caller's read door; add the row with its reason only after review, or write as the caller.", + ); + } else if (row.count !== calls) { + problems.push( + `${path}: sets the vouched-reference mark ${calls} time(s); its ALLOWED_CALLERS row pins ${row.count}. ` + + 'Every call site is reviewed on its own: update the row (and its reason) in the same change.', + ); + } + } + for (const path of Object.keys(allowed)) { + if (!counts.has(path)) { + problems.push(`${path}: is in ALLOWED_CALLERS and no longer sets the mark — remove the stale row.`); + } + } + return { problems, counts }; +} + +/** The files the census reads: what git sees under `packages/`, never build output. */ +function readTree() { + const out = execFileSync('git', ['ls-files', '--cached', '--others', '--exclude-standard', '-z', '--', 'packages'], { + cwd: REPO, + encoding: 'utf8', + maxBuffer: 256 * 1024 * 1024, + }); + const files = []; + for (const path of out.split('\0')) { + if (!path || !SOURCE_FILE.test(path) || TEST_FILE.test(path)) continue; + let text; + try { + text = readFileSync(join(REPO, path), 'utf8'); + } catch { + continue; // listed by git, deleted in the working tree + } + if (!text.includes('vouchForReference') && !text.includes('VOUCHED_REFERENCES') && !text.includes('vouchedReferences')) { + text = ''; + } + files.push({ path, text }); + } + return files; +} + +// ── Self-test ────────────────────────────────────────────────────────────── +// +// Table-driven, as `check-agent-model-declared.mjs`'s: each row LABEL is a +// declared battery with a floor of 1, registered as the first statement of the +// loop body; the roster's own size is pinned; and the verdict sets a flag the +// dispatch refuses without (AGENTS.md, "Writing a --self-test"). + +let selfTestReachedVerdict = false; + +const SELF_TEST_BATTERIES = Object.freeze({ + 'an allow-listed file with exactly its pinned call sites → GREEN': 1, + 'a call from a file outside the allow-list → RED': 1, + 'an allow-listed file with one more call site than its row → RED': 1, + 'an allow-listed file with one fewer call site than its row → RED': 1, + 'a row naming a file that no longer sets the mark → RED (stale row)': 1, + 'an import that renames vouchForReference → RED': 1, + 'the key built by hand as [VOUCHED_REFERENCES] → RED': 1, + "the symbol spelled by hand with Symbol.for → RED": 1, + 'a test file that sets the mark → GREEN (outside the census)': 1, + 'a comment or backtick mention is no call site → GREEN': 1, + 'the defining module’s own spellings → GREEN': 1, + 'an empty scan → RED, never a green skip': 1, +}); +const SELF_TEST_BATTERY_FLOOR = 12; + +function selfTest() { + const VERB = 'packages/services/demo/src/verb.ts'; + const OTHER = 'packages/rest/src/door.ts'; + const ALLOW = { [VERB]: { count: 1, reason: 'demo' } }; + const call = " const ctx = vouchForReference(context, { object: 'o', field: 'f', id });\n"; + const filler = { path: 'packages/core/src/other.ts', text: 'export const x = 1;\n' }; + + const cases = [ + { label: 'an allow-listed file with exactly its pinned call sites → GREEN', files: [{ path: VERB, text: call }], expect: 'green' }, + { label: 'a call from a file outside the allow-list → RED', files: [{ path: VERB, text: call }, { path: OTHER, text: call }], expect: 'red', names: OTHER }, + { label: 'an allow-listed file with one more call site than its row → RED', files: [{ path: VERB, text: call + call }], expect: 'red', names: VERB }, + { label: 'an allow-listed file with one fewer call site than its row → RED', files: [{ path: VERB, text: call }], allow: { [VERB]: { count: 2, reason: 'demo' } }, expect: 'red', names: VERB }, + { label: 'a row naming a file that no longer sets the mark → RED (stale row)', files: [filler], expect: 'red', names: VERB }, + { label: 'an import that renames vouchForReference → RED', files: [{ path: VERB, text: call }, { path: OTHER, text: "import { vouchForReference as vouch } from '@objectstack/core';\n" }], expect: 'red', names: OTHER }, + { label: 'the key built by hand as [VOUCHED_REFERENCES] → RED', files: [{ path: VERB, text: call }, { path: OTHER, text: ' const ctx = { ...context, [VOUCHED_REFERENCES]: [ref] };\n' }], expect: 'red', names: OTHER }, + { label: "the symbol spelled by hand with Symbol.for → RED", files: [{ path: VERB, text: call }, { path: OTHER, text: " const k = Symbol.for('objectstack.core.vouchedReferences');\n" }], expect: 'red', names: OTHER }, + { label: 'a test file that sets the mark → GREEN (outside the census)', files: [{ path: VERB, text: call }, { path: 'packages/rest/src/door.test.ts', text: call + ' ({ [VOUCHED_REFERENCES]: [] });\n' }], expect: 'green' }, + { label: 'a comment or backtick mention is no call site → GREEN', files: [{ path: VERB, text: call }, { path: OTHER, text: '// vouchForReference(context, ref) is the verb\'s\n * see vouchForReference( here\nconst s = `vouchForReference(x)`;\n' }], expect: 'green' }, + { label: 'the defining module’s own spellings → GREEN', files: [{ path: VERB, text: call }, { path: DEFINING_MODULE, text: "export const VOUCHED_REFERENCES = Symbol.for('objectstack.core.vouchedReferences');\nexport function vouchForReference(context, reference) {\n return { ...context, [VOUCHED_REFERENCES]: [] };\n}\n" }], expect: 'green' }, + { label: 'an empty scan → RED, never a green skip', files: [], expect: 'red', names: 'no source files' }, + ]; + + const batterySeen = new Map(); + let failed = 0; + for (const c of cases) { + batterySeen.set(c.label, (batterySeen.get(c.label) ?? 0) + 1); + const { problems } = check(c.files, c.allow ?? ALLOW); + const red = problems.length > 0; + const ok = + (c.expect === 'green' && !red) || + (c.expect === 'red' && red && (!c.names || problems.some((p) => p.includes(c.names)))); + if (!ok) { + failed += 1; + console.error(`✗ ${c.label}\n expected ${c.expect}${c.names ? ` naming ${c.names}` : ''}, got: ${red ? problems.join(' | ') : 'green'}`); + } + } + + const declared = Object.keys(SELF_TEST_BATTERIES); + const floorFailure = (message) => { + console.error(`✗ self-test floor: ${message}`); + failed += 1; + }; + if (declared.length < SELF_TEST_BATTERY_FLOOR) { + floorFailure(`SELF_TEST_BATTERIES declares ${declared.length} batteries, below the pinned ${SELF_TEST_BATTERY_FLOOR}.`); + } + const labels = cases.map((c) => c.label); + const duplicated = [...new Set(labels.filter((l, i) => labels.indexOf(l) !== i))]; + if (duplicated.length > 0) floorFailure(`row label(s) used more than once: ${duplicated.join(', ')}`); + for (const [name] of batterySeen) { + if (!declared.includes(name)) floorFailure(`battery "${name}" ran but is not declared in SELF_TEST_BATTERIES.`); + } + for (const name of declared) { + const seen = batterySeen.get(name) ?? 0; + if (seen < SELF_TEST_BATTERIES[name]) { + floorFailure(`battery "${name}" DID NOT RUN — ${seen} case(s) registered, ${SELF_TEST_BATTERIES[name]} pinned.`); + } + } + + if (failed > 0) { + console.error(`\n✗ check-vouch-for-reference-callers self-test: ${failed} failure(s) (cases and floor).`); + process.exit(1); + } + console.log(`✓ check-vouch-for-reference-callers self-test: ${cases.length} cases pass.`); + selfTestReachedVerdict = true; +} + +function main() { + if (process.argv.includes('--self-test')) { + selfTest(); + if (!selfTestReachedVerdict) { + console.error( + '✗ check-vouch-for-reference-callers self-test: selfTest() returned without reaching its verdict, so no ' + + 'success line was printed; exiting 0 would report a self-test that never finished as one that passed.', + ); + process.exit(1); + } + return; + } + const { problems, counts } = check(readTree()); + if (problems.length > 0) { + console.error(`✗ check-vouch-for-reference-callers: ${problems.length} finding(s)\n`); + for (const p of problems) console.error(` - ${p}`); + process.exit(1); + } + const listed = [...counts].map(([path, n]) => `${path} (${n})`).join(', '); + console.log(`✓ check-vouch-for-reference-callers: every call site that sets the vouched-reference mark is allow-listed — ${listed}.`); +} + +if (isEntrypoint(import.meta.url)) { + main(); +} From 27c35614cf037c813091ad65426be7a972fae22a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 23:45:25 +0000 Subject: [PATCH 11/21] wip: Test Core runs the vouched-reference census gate, red and green Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- package.json | 1 + .../vouched-reference-callers.pin.test.ts | 38 +++++++++++++++++++ packages/core/vitest.repo-tests.json | 1 + scripts/cross-package-test-inputs.mjs | 5 ++- turbo.json | 3 +- 5 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 packages/core/src/security/vouched-reference-callers.pin.test.ts diff --git a/package.json b/package.json index ceafb161b83..16fff232e34 100644 --- a/package.json +++ b/package.json @@ -206,6 +206,7 @@ "check:stall-guard-headroom": "node scripts/measure-stall-guard-headroom.mjs --self-test", "check:swallow-census-controls": "node scripts/measure-durability-swallow-family.mjs --self-test=gated", "check:system-context-census": "node scripts/check-system-context-census.mjs --self-test && node scripts/check-system-context-census.mjs", + "check:vouch-for-reference-callers": "node scripts/check-vouch-for-reference-callers.mjs --self-test && node scripts/check-vouch-for-reference-callers.mjs", "gen:platform-object-tenancy-census": "node scripts/platform-object-tenancy-census.mjs --write", "check:platform-object-tenancy-census": "node scripts/check-platform-object-tenancy-census.mjs --self-test && node scripts/check-platform-object-tenancy-census.mjs" }, diff --git a/packages/core/src/security/vouched-reference-callers.pin.test.ts b/packages/core/src/security/vouched-reference-callers.pin.test.ts new file mode 100644 index 00000000000..b6c3af72643 --- /dev/null +++ b/packages/core/src/security/vouched-reference-callers.pin.test.ts @@ -0,0 +1,38 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The call-site census of the vouched-reference mark (`vouchForReference`, + * `./vouched-reference.ts`), run where `Test Core` runs it: the gate's own + * self-test (its red and green cases, with the battery floor and the verdict + * handshake), then the scan of this tree, which must be green — every + * production file that sets the mark is in the gate's `ALLOWED_CALLERS` with + * its exact call-site count. The gate's header is the authority on the rule. + */ + +import { describe, expect, it } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = resolve(HERE, '../../../..'); +const GATE = resolve(HERE, '../../../../scripts/check-vouch-for-reference-callers.mjs'); + +const run = (...args: string[]) => + spawnSync(process.execPath, [GATE, ...args], { cwd: REPO_ROOT, encoding: 'utf8' }); + +describe('check:vouch-for-reference-callers', () => { + it('its self-test reaches its verdict: every red and green case holds, and no battery fell below its floor', () => { + const out = run('--self-test'); + expect(out.stderr).toBe(''); + expect(out.status).toBe(0); + expect(out.stdout).toMatch(/self-test: \d+ cases pass/); + }); + + it('this tree is green: the only production caller is the file-attachments writer, at its pinned count', () => { + const out = run(); + expect(out.stderr).toBe(''); + expect(out.status).toBe(0); + expect(out.stdout).toContain('packages/services/service-storage/src/file-attachment-service.ts (2)'); + }); +}); diff --git a/packages/core/vitest.repo-tests.json b/packages/core/vitest.repo-tests.json index 02fe9b15e9b..c06e908a326 100644 --- a/packages/core/vitest.repo-tests.json +++ b/packages/core/vitest.repo-tests.json @@ -4,5 +4,6 @@ "src/security/operation-private-keys.pin.test.ts", "src/security/row-serving-door-exposure.pin.test.ts", "src/security/second-object-read-exposure.pin.test.ts", + "src/security/vouched-reference-callers.pin.test.ts", "src/utils/import-runner-sandbox-fault-row.test.ts" ] diff --git a/scripts/cross-package-test-inputs.mjs b/scripts/cross-package-test-inputs.mjs index a05d602ff1e..1d6efc615c5 100644 --- a/scripts/cross-package-test-inputs.mjs +++ b/scripts/cross-package-test-inputs.mjs @@ -349,7 +349,10 @@ export const CROSS_PACKAGE_TEST_INPUTS = { // hints: the case flipped from true to false with the added glob itself as // the covering hint. ⇒ Extensions and glob widen together or not at all, // and that pin's header carries the measurement of what the boundary costs. - globs: ['packages/**/*.ts'], + // + // src/security/vouched-reference-callers.pin.test.ts runs the census gate + // of the vouched-reference mark, a root script, over the same `.ts` scan. + globs: ['packages/**/*.ts', 'scripts/check-vouch-for-reference-callers.mjs'], }, '@objectstack/types': { // src/driver-error-classification.callers.test.ts is the #13440 gate: it diff --git a/turbo.json b/turbo.json index a8caa1478a2..4f4982518ee 100644 --- a/turbo.json +++ b/turbo.json @@ -806,7 +806,8 @@ "!coverage/**", "!.turbo/**", "!**/node_modules/**", - "$TURBO_ROOT$/packages/**/*.ts" + "$TURBO_ROOT$/packages/**/*.ts", + "$TURBO_ROOT$/scripts/check-vouch-for-reference-callers.mjs" ] }, "@objectstack/types#test:repo": { From 64beb5258bebd01c2c3e18a9b073c48375795128 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 23:55:46 +0000 Subject: [PATCH 12/21] test(service-storage): attachUploadAs attaches a member's own pending upload, and refuses every other file Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../src/file-attachment-service.test.ts | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) 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 e57891523c1..a3929d388d3 100644 --- a/packages/services/service-storage/src/file-attachment-service.test.ts +++ b/packages/services/service-storage/src/file-attachment-service.test.ts @@ -638,3 +638,187 @@ describe('attachFileAs for a member who cannot read sys_file: the verb vouches f expect(await rows(ql, 'sys_attachment')).toHaveLength(1); }); }); + +describe('attachUploadAs: a member attaches their own pending browser upload, and the commit is the attach', () => { + // The same member posture as above: the create grant on `sys_attachment`, + // nothing on `sys_file` (a non-system read of it is refused outright), and + // Bob cannot read the parent object. The upload is seeded as the presigned + // door leaves one: a `pending` row owned by the uploader in the + // `attachments` scope, and its bytes at its key. + 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; security?: 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; + } + + const BYTES = Buffer.from('hello'); + /** A pending upload, as the presigned door and the raw PUT leave one. */ + async function seedUpload( + ql: ObjectQL, + storage: IStorageService, + over: { id?: string; owner?: string; scope?: string; status?: string; bytes?: boolean } = {}, + ) { + const id = over.id ?? 'upl_alice'; + const scope = over.scope ?? 'attachments'; + const key = `${scope}/${id}.txt`; + await ql.insert('sys_file', { + id, key, name: 'notes.txt', mime_type: 'text/plain', size: BYTES.byteLength, scope, acl: 'private', + status: over.status ?? 'pending', owner_id: over.owner ?? ALICE.userId, organization_id: 'org_a', + }, SYS); + if (over.bytes !== false) await storage.upload(key, BYTES, { contentType: 'text/plain' }); + return id; + } + const target = (fileId: string, over: Record = {}) => ({ fileId, parentObject: 'att_case', parentId: 'case_a', ...over }); + const answer = (err: { code?: string; status?: number; message?: string }) => ({ code: err.code, status: err.status }); + const fileRow = async (ql: ObjectQL, id: string) => (await rows(ql, 'sys_file')).find((f) => f.id === id); + + /** A refused attach leaves the upload as it was: pending, bytes stored, nothing attached. */ + async function expectUploadUntouched(ql: ObjectQL, storage: IStorageService, id: string) { + const file = await fileRow(ql, id); + expect(file?.status).toBe('pending'); + expect(await storedKeys(storage)).toContain(file!.key); + expect(await rows(ql, 'sys_attachment')).toEqual([]); + } + + it('attaches as the member: the upload is committed and attached in one call, uploaded_by them, and sys_file stays unreadable to them', async () => { + const { ql, attachments, storage } = await bootMember(); + const fileId = await seedUpload(ql, storage); + const out = await asCaller(attachments).attachUploadAs(target(fileId, { eTag: 'etag-1' }), ALICE as never); + expect(out).toEqual({ fileId, attachmentId: expect.any(String), size: BYTES.byteLength }); + expect(await fileRow(ql, fileId)).toMatchObject({ status: 'committed', etag: 'etag-1', owner_id: ALICE.userId }); + 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: fileId, + file_name: 'notes.txt', uploaded_by: ALICE.userId, organization_id: 'org_a', + }); + const err = await refusalOf(ql.findOne('sys_file', { where: { id: fileId }, context: ALICE })); + expect(answer(err)).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); + }); + + it('control: another user’s pending upload is refused with the upload ownership rule’s one answer, and is left as it was', async () => { + const { ql, attachments, storage } = await bootMember(); + const fileId = await seedUpload(ql, storage, { id: 'upl_bob', owner: BOB.userId }); + const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId), ALICE as never)); + expect(answer(err)).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); + await expectUploadUntouched(ql, storage, fileId); + }); + + it('control: an id that names no file gets exactly the answer another user’s upload gets', async () => { + const { ql, attachments, storage } = await bootMember(); + const others = await seedUpload(ql, storage, { id: 'upl_bob', owner: BOB.userId }); + const a = await refusalOf(asCaller(attachments).attachUploadAs(target(others), ALICE as never)); + const b = await refusalOf(asCaller(attachments).attachUploadAs(target('upl_nowhere'), ALICE as never)); + expect([b.code, b.status, b.message]).toEqual([a.code, a.status, a.message]); + expect(await rows(ql, 'sys_attachment')).toEqual([]); + }); + + it('control: a committed upload is refused RESOURCE_CONFLICT / 409 — only the upload this call commits is vouched for', async () => { + const { ql, attachments, storage } = await bootMember(); + const fileId = await seedUpload(ql, storage, { status: 'committed' }); + const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId), ALICE as never)); + expect(answer(err)).toEqual({ code: 'RESOURCE_CONFLICT', status: 409 }); + expect(await rows(ql, 'sys_attachment')).toEqual([]); + }); + + it('control: a pending upload something already holds is refused RESOURCE_CONFLICT / 409, and no second attachment is written', async () => { + const { ql, attachments, storage } = await bootMember(); + const fileId = await seedUpload(ql, storage); + await ql.insert('sys_attachment', { parent_object: 'att_case', parent_id: 'case_global', file_id: fileId, file_name: 'n.txt' }, SYS); + const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId), ALICE as never)); + expect(answer(err)).toEqual({ code: 'RESOURCE_CONFLICT', status: 409 }); + expect(await rows(ql, 'sys_attachment')).toHaveLength(1); + expect((await fileRow(ql, fileId))?.status).toBe('pending'); + }); + + it('control: an upload outside the attachments scope is refused INVALID_REQUEST / 400', async () => { + const { ql, attachments, storage } = await bootMember(); + const fileId = await seedUpload(ql, storage, { scope: 'user' }); + const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId), ALICE as never)); + expect(answer(err)).toEqual({ code: 'INVALID_REQUEST', status: 400 }); + expect(await rows(ql, 'sys_attachment')).toEqual([]); + }); + + it('control: an upload whose bytes the backend does not hold is refused RESOURCE_CONFLICT / 409 and stays pending', async () => { + const { ql, attachments, storage } = await bootMember(); + const fileId = await seedUpload(ql, storage, { bytes: false }); + const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId), ALICE as never)); + expect(answer(err)).toEqual({ code: 'RESOURCE_CONFLICT', status: 409 }); + expect((await fileRow(ql, fileId))?.status).toBe('pending'); + expect(await rows(ql, 'sys_attachment')).toEqual([]); + }); + + it('control: a member the attach rule refuses is refused ATTACHMENT_PARENT_ACCESS / 403, and the upload is left as it was', async () => { + const sharing = { checkEdit: vi.fn(async () => 'deny') }; + const { ql, attachments, storage } = await bootMember({ sharing }); + const fileId = await seedUpload(ql, storage); + const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId), ALICE as never)); + expect(answer(err)).toEqual({ code: 'ATTACHMENT_PARENT_ACCESS', status: 403 }); + await expectUploadUntouched(ql, storage, fileId); + }); + + it('control: a member without the permission to create an attachment is refused PERMISSION_DENIED / 403, and the upload is left as it was', async () => { + const security = { explain: vi.fn(async () => ({ allowed: false })) }; + const { ql, attachments, storage } = await bootMember({ security }); + const fileId = await seedUpload(ql, storage); + const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId), ALICE as never)); + expect(answer(err)).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); + expect(security.explain).toHaveBeenCalledWith({ object: 'sys_attachment', operation: 'create' }, ALICE); + await expectUploadUntouched(ql, storage, fileId); + }); + + it('control: a parent object without enable.files is refused FILES_DISABLED / 403 by the insert’s own gate, and the upload is left as it was', async () => { + const { ql, attachments, storage } = await bootMember(); + const fileId = await seedUpload(ql, storage); + const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId, { parentObject: 'att_memo', parentId: 'memo_a' }), ALICE as never)); + expect(answer(err)).toEqual({ code: 'FILES_DISABLED', status: 403 }); + await expectUploadUntouched(ql, storage, fileId); + }); + + it('control: a parent the member cannot read is refused, and the upload is left as it was', async () => { + const { ql, attachments, storage } = await bootMember(); + const fileId = await seedUpload(ql, storage, { id: 'upl_bob', owner: BOB.userId }); + const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId), BOB as never)); + expect(err.status).toBe(err.code === 'RECORD_NOT_FOUND' ? 404 : 403); + expect(['RECORD_NOT_FOUND', 'ATTACHMENT_PARENT_ACCESS']).toContain(err.code); + await expectUploadUntouched(ql, storage, fileId); + }); + + it('the vouch is the verb’s call alone: after attachUploadAs, the member’s own insert naming the same file is refused reference_not_found', async () => { + const { ql, attachments, storage } = await bootMember(); + const fileId = await seedUpload(ql, storage); + await asCaller(attachments).attachUploadAs(target(fileId), ALICE as never); + const err = await refusalOf( + ql.insert('sys_attachment', { parent_object: 'att_case', parent_id: 'case_a', file_id: fileId, file_name: 'direct.txt' }, { context: ALICE }), + ); + const envelope = resolveThrownHttpError(err); + expect([envelope.code, envelope.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(((err as any).fields ?? []).map((f: any) => [f.field, f.code])).toEqual([['file_id', 'reference_not_found']]); + expect(await rows(ql, 'sys_attachment')).toHaveLength(1); + }); + + it('control: the system context and a context without a user are refused PERMISSION_DENIED / 403', async () => { + const { ql, attachments, storage } = await bootMember(); + const fileId = await seedUpload(ql, storage); + for (const context of [{ isSystem: true }, { tenantId: 'org_a' }]) { + const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId), context as never)); + expect(answer(err)).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); + } + await expectUploadUntouched(ql, storage, fileId); + }); +}); From 6ff78acaff3aba59cfd25a670819de456cbcdb64 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 00:08:13 +0000 Subject: [PATCH 13/21] test(service-storage): the commit door hands attachTo to the attacher only after its own gates Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../src/file-attachment-service.test.ts | 4 +- .../src/file-attachment-service.ts | 2 +- .../storage-routes.complete-attach.test.ts | 228 ++++++++++++++++++ 3 files changed, 231 insertions(+), 3 deletions(-) create mode 100644 packages/services/service-storage/src/storage-routes.complete-attach.test.ts 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 a3929d388d3..86c6706fde5 100644 --- a/packages/services/service-storage/src/file-attachment-service.test.ts +++ b/packages/services/service-storage/src/file-attachment-service.test.ts @@ -724,7 +724,7 @@ describe('attachUploadAs: a member attaches their own pending browser upload, an const others = await seedUpload(ql, storage, { id: 'upl_bob', owner: BOB.userId }); const a = await refusalOf(asCaller(attachments).attachUploadAs(target(others), ALICE as never)); const b = await refusalOf(asCaller(attachments).attachUploadAs(target('upl_nowhere'), ALICE as never)); - expect([b.code, b.status, b.message]).toEqual([a.code, a.status, a.message]); + expect([b.code, b.status, (b as Error).message]).toEqual([a.code, a.status, (a as Error).message]); expect(await rows(ql, 'sys_attachment')).toEqual([]); }); @@ -739,7 +739,7 @@ describe('attachUploadAs: a member attaches their own pending browser upload, an it('control: a pending upload something already holds is refused RESOURCE_CONFLICT / 409, and no second attachment is written', async () => { const { ql, attachments, storage } = await bootMember(); const fileId = await seedUpload(ql, storage); - await ql.insert('sys_attachment', { parent_object: 'att_case', parent_id: 'case_global', file_id: fileId, file_name: 'n.txt' }, SYS); + await ql.insert('sys_attachment', { parent_object: 'att_case', parent_id: 'case_a', file_id: fileId, file_name: 'n.txt', organization_id: 'org_a' }, SYS); const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId), ALICE as never)); expect(answer(err)).toEqual({ code: 'RESOURCE_CONFLICT', status: 409 }); 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 4dc9d0df7ee..24e738f31e2 100644 --- a/packages/services/service-storage/src/file-attachment-service.ts +++ b/packages/services/service-storage/src/file-attachment-service.ts @@ -375,7 +375,7 @@ export class FileAttachmentService implements IFileAttachmentService, ICallerFil // and for no other value. The write-time reference check does not // ask the caller's read door for that id, and still probes that it // exists; every other gate on the insert judges the caller. - { context: vouchForReference(unitContext, { object: 'sys_attachment', field: 'file_id', id: file.id }) }, + { context: vouchForReference(unitContext as ExecutionContext, { object: 'sys_attachment', field: 'file_id', id: file.id }) }, )) as { id?: unknown } | null | undefined; if (row?.id === undefined || row.id === null || row.id === '') { throw new Error('attachUploadAs: the sys_attachment insert answered no row id, so the attachment cannot be named'); diff --git a/packages/services/service-storage/src/storage-routes.complete-attach.test.ts b/packages/services/service-storage/src/storage-routes.complete-attach.test.ts new file mode 100644 index 00000000000..04548f39bf5 --- /dev/null +++ b/packages/services/service-storage/src/storage-routes.complete-attach.test.ts @@ -0,0 +1,228 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The commit door's `attachTo` arm (`POST …/upload/complete`): the record +// attachments surface's attach verb. With `attachTo`, the door hands the +// caller's own upload to the attacher `composeStorageRoutes` binds (the +// file-attachments service's `attachUploadAs`, as the caller) instead of +// committing it itself, and answers the committed file with its new +// `attachmentId`. +// +// Pinned here at the door: every gate the door already asked runs FIRST — +// the session, a malformed `attachTo`, a file that does not exist, the upload +// ownership rule, the bytes — and a refused request never reaches the +// attacher; the attacher's declared refusals are relayed with their code, +// status and details; a mount with no attacher bound refuses `attachTo` with +// `501`; and a completion without `attachTo` is unchanged. The attach verb's +// own checks and its one-file vouch are pinned in +// `file-attachment-service.test.ts`, and the whole flow on a real boot in the +// `attachments-browser-attach` dogfood. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { promises as fs } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts'; +import { LocalStorageAdapter } from './local-storage-adapter.js'; +import { StorageMetadataStore } from './metadata-store.js'; +import { + registerStorageFamily, + registerStorageRoutes, + type StorageUploadAttacher, + type StorageUploadSession, +} from './storage-routes.js'; + +const BASE = '/api/v1/storage'; +const COMMIT = `${BASE}/upload/complete`; + +type Res = IHttpResponse & { _status: number; _json: any }; + +function createMockHttpServer() { + const routes = new Map(); + return { + get: vi.fn((path: string, handler: RouteHandler) => { routes.set(`GET:${path}`, handler); }), + post: vi.fn((path: string, handler: RouteHandler) => { routes.set(`POST:${path}`, handler); }), + put: vi.fn((path: string, handler: RouteHandler) => { routes.set(`PUT:${path}`, handler); }), + delete: vi.fn(), + patch: vi.fn(), + use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + handler(method: string, path: string): RouteHandler { + const h = routes.get(`${method}:${path}`); + if (!h) throw new Error(`no route ${method} ${path}`); + return h; + }, + }; +} + +function createMockRes(): Res { + const res: any = { + _status: 200, + _json: null, + json(data: any) { res._json = data; }, + send(data: any) { res._sent = data; }, + status(code: number) { res._status = code; return res; }, + header() { return res; }, + }; + return res; +} + +const ALICE: StorageUploadSession = { userId: 'usr_alice' }; +const BOB: StorageUploadSession = { userId: 'usr_bob' }; +const BYTES = Buffer.from('hello'); + +let rootDir: string; +let storage: LocalStorageAdapter; +let store: StorageMetadataStore; +let server: ReturnType; +let attacher: ReturnType; + +/** A session per request, read off a test header — the door's `resolveSession`. */ +const resolveSession = async (req: { headers?: unknown }) => { + const who = (req.headers as Record | undefined)?.['x-test-user']; + return who === 'alice' ? ALICE : who === 'bob' ? BOB : null; +}; + +function mount(attach: StorageUploadAttacher | undefined) { + server = createMockHttpServer(); + registerStorageFamily(server as any, storage, store, { basePath: BASE, resolveSession }, undefined, attach); +} + +/** A pending upload of Alice's, its bytes held — what the presigned door and the raw PUT leave. */ +async function seedUpload(over: { id?: string; owner?: string; bytes?: boolean } = {}) { + const id = over.id ?? 'upl_alice'; + const key = `attachments/${id}.txt`; + await store.createFile({ + id, key, name: 'notes.txt', mime_type: 'text/plain', size: BYTES.byteLength, scope: 'attachments', + acl: 'private', status: 'pending', owner_id: over.owner ?? ALICE.userId, + }); + if (over.bytes !== false) await storage.upload(key, BYTES, { contentType: 'text/plain' }); + return id; +} + +async function complete(who: 'alice' | 'bob' | null, body: Record): Promise { + const res = createMockRes(); + const req = { params: {}, query: {}, body, headers: who ? { 'x-test-user': who } : {}, method: 'POST', path: COMMIT } as unknown as IHttpRequest; + await server.handler('POST', COMMIT)(req, res); + return res; +} + +const ATTACH_TO = { parentObject: 'att_case', parentId: 'case_a' }; + +beforeEach(async () => { + rootDir = join(tmpdir(), `os-complete-attach-${Date.now()}-${Math.random().toString(36).slice(2)}`); + await fs.mkdir(rootDir, { recursive: true }); + storage = new LocalStorageAdapter({ rootDir, signingSecret: 'complete-attach' }); + store = new StorageMetadataStore(null); + attacher = vi.fn(async () => ({ attachmentId: 'att_1' })); + mount(attacher as unknown as StorageUploadAttacher); +}); + +afterEach(async () => { + vi.restoreAllMocks(); + await fs.rm(rootDir, { recursive: true, force: true }); +}); + +describe('POST /storage/upload/complete with attachTo — the attach verb at the door', () => { + it('hands the caller’s own upload to the attacher, does not commit it itself, and answers the file with its attachmentId', async () => { + const fileId = await seedUpload(); + const res = await complete('alice', { fileId, eTag: 'etag-1', attachTo: ATTACH_TO }); + expect(res._status, JSON.stringify(res._json)).toBe(200); + expect(attacher).toHaveBeenCalledTimes(1); + expect(attacher.mock.calls[0][1]).toEqual({ fileId, ...ATTACH_TO, eTag: 'etag-1' }); + expect(res._json).toMatchObject({ success: true, data: { fileId, name: 'notes.txt', attachmentId: 'att_1' } }); + // The commit is the attacher's (one unit with the insert): this stand-in + // committed nothing, and the door wrote nothing of its own. + expect((await store.getFile(fileId))?.status).toBe('pending'); + }); + + it('control: another user’s upload is refused by the upload ownership rule before the attacher is asked', async () => { + const fileId = await seedUpload({ owner: BOB.userId }); + const res = await complete('alice', { fileId, attachTo: ATTACH_TO }); + expect([res._status, res._json?.error?.code]).toEqual([403, 'PERMISSION_DENIED']); + expect(attacher).not.toHaveBeenCalled(); + expect((await store.getFile(fileId))?.status).toBe('pending'); + }); + + it('control: an id that names no file is refused FILE_NOT_FOUND / 404 before the attacher is asked', async () => { + const res = await complete('alice', { fileId: 'upl_nowhere', attachTo: ATTACH_TO }); + expect([res._status, res._json?.error?.code]).toEqual([404, 'FILE_NOT_FOUND']); + expect(attacher).not.toHaveBeenCalled(); + }); + + it('control: an anonymous request is refused AUTH_REQUIRED / 401 before anything is read', async () => { + const fileId = await seedUpload(); + const res = await complete(null, { fileId, attachTo: ATTACH_TO }); + expect([res._status, res._json?.error?.code]).toEqual([401, 'AUTH_REQUIRED']); + expect(attacher).not.toHaveBeenCalled(); + }); + + it('control: an upload whose bytes are not held is refused RESOURCE_CONFLICT / 409 before the attacher is asked', async () => { + const fileId = await seedUpload({ bytes: false }); + const res = await complete('alice', { fileId, attachTo: ATTACH_TO }); + expect([res._status, res._json?.error?.code]).toEqual([409, 'RESOURCE_CONFLICT']); + expect(attacher).not.toHaveBeenCalled(); + }); + + for (const [what, attachTo] of [ + ['a missing parentId', { parentObject: 'att_case' }], + ['an empty parentObject', { parentObject: '', parentId: 'case_a' }], + ['a key attachTo does not declare', { ...ATTACH_TO, file_id: 'other' }], + ['an array', [ATTACH_TO]], + ['null', null], + ] as const) { + it(`control: attachTo with ${what} is refused INVALID_REQUEST / 400, and nothing is read, attached or committed`, async () => { + const fileId = await seedUpload(); + const res = await complete('alice', { fileId, attachTo }); + expect([res._status, res._json?.error?.code]).toEqual([400, 'INVALID_REQUEST']); + expect(attacher).not.toHaveBeenCalled(); + expect((await store.getFile(fileId))?.status).toBe('pending'); + }); + } + + it('relays the attacher’s declared refusal with its code and status, and the upload is left pending', async () => { + attacher.mockRejectedValueOnce(Object.assign(new Error('Cannot attach to att_case/case_a'), { code: 'ATTACHMENT_PARENT_ACCESS', status: 403 })); + const fileId = await seedUpload(); + const res = await complete('alice', { fileId, attachTo: ATTACH_TO }); + expect([res._status, res._json?.error?.code]).toEqual([403, 'ATTACHMENT_PARENT_ACCESS']); + expect(res._json.error.message).toBe('Cannot attach to att_case/case_a'); + expect((await store.getFile(fileId))?.status).toBe('pending'); + }); + + it('relays a validation refusal from the insert as VALIDATION_FAILED / 400 with its field details', async () => { + const validation = Object.assign(new Error('file_id: reference not found'), { + name: 'ValidationError', + fields: [{ field: 'file_id', code: 'reference_not_found', message: 'reference not found' }], + }); + attacher.mockRejectedValueOnce(validation); + const fileId = await seedUpload(); + const res = await complete('alice', { fileId, attachTo: ATTACH_TO }); + expect([res._status, res._json?.error?.code]).toEqual([400, 'VALIDATION_FAILED']); + }); + + it('an undeclared fault is a 500 with the door’s generic text, never the thrown message', async () => { + attacher.mockRejectedValueOnce(new Error('engine said something about the host')); + const fileId = await seedUpload(); + const res = await complete('alice', { fileId, attachTo: ATTACH_TO }); + expect(res._status).toBe(500); + expect(res._json.error.message).not.toContain('engine said'); + }); + + it('a mount with no attacher bound (the public registrar, a bare kernel) refuses attachTo NOT_IMPLEMENTED / 501 and commits nothing', async () => { + server = createMockHttpServer(); + registerStorageRoutes(server as any, storage, store, { basePath: BASE, resolveSession }); + const fileId = await seedUpload(); + const res = await complete('alice', { fileId, attachTo: ATTACH_TO }); + expect([res._status, res._json?.error?.code]).toEqual([501, 'NOT_IMPLEMENTED']); + expect((await store.getFile(fileId))?.status).toBe('pending'); + }); + + it('control: a completion without attachTo commits as before and never asks the attacher', async () => { + const fileId = await seedUpload(); + const res = await complete('alice', { fileId }); + expect(res._status).toBe(200); + expect(res._json.data.attachmentId).toBeUndefined(); + expect((await store.getFile(fileId))?.status).toBe('committed'); + expect(attacher).not.toHaveBeenCalled(); + }); +}); From f646b60cb337040dfa75ccc2dbc3fe7c14297108 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 00:19:04 +0000 Subject: [PATCH 14/21] fix(console): the attachments panel attaches through the upload's own completion Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../src/views/RecordAttachmentsPanel.tsx | 56 ++++++------- ...hmentsPanel.affordanceGrant-12047.test.tsx | 37 ++++----- .../__tests__/RecordAttachmentsPanel.test.tsx | 78 +++++++++++++++++++ .../data-objectstack/src/upload-adapter.ts | 36 +++++++-- 4 files changed, 157 insertions(+), 50 deletions(-) diff --git a/ui/packages/app-shell/src/views/RecordAttachmentsPanel.tsx b/ui/packages/app-shell/src/views/RecordAttachmentsPanel.tsx index 357174d5f02..fdaad5c44a4 100644 --- a/ui/packages/app-shell/src/views/RecordAttachmentsPanel.tsx +++ b/ui/packages/app-shell/src/views/RecordAttachmentsPanel.tsx @@ -46,7 +46,11 @@ import { RECORD_PAGE_ATTACHMENT_ROWS, recordAttachmentsCountField } from './reco * * Storage model: one `sys_file` row per uploaded blob (three-step * presigned upload via @object-ui/data-objectstack's ObjectStack adapter), one - * `sys_attachment` join row linking it to `(parent_object, parent_id)`. + * `sys_attachment` join row linking it to `(parent_object, parent_id)`. The + * join row is written by the upload's own completion (`attachTo`), as the + * signed-in user: the server's attach verb, never a `sys_attachment` create + * through the generic data door, which a member who may attach files but not + * read `sys_file` is refused (`reference_not_found` on `file_id`). * Downloads fetch a short-lived signed URL from `/storage/files/:fileId/url` * with the console's Bearer token (the endpoint requires an authenticated * session for attachments-scope files, objectstack-ai/objectstack#2970), then open it. @@ -65,9 +69,9 @@ interface AttachmentRow { export interface RecordAttachmentsPanelProps { objectName: string; recordId: string; - /** ObjectUI DataSource (sys_attachment CRUD goes through the generic data path). */ + /** ObjectUI DataSource (the list read and the delete go through the generic data path; the attach does not). */ dataSource: any; - /** Current user id, stamped on `uploaded_by`. */ + /** Current user id. The server stamps `uploaded_by` from the session; the panel no longer sends it. */ currentUserId?: string | null; className?: string; } @@ -175,7 +179,6 @@ export const RecordAttachmentsPanel: React.FC = ({ objectName, recordId, dataSource, - currentUserId, className, }) => { const { t } = useObjectTranslation(); @@ -216,10 +219,10 @@ export const RecordAttachmentsPanel: React.FC = ({ * and a delete go through the storage route, not `sys_attachment`'s generic * data door, so that door's operation set says nothing about them. * - * Upload needs `create`. Without it the three-step presigned flow below - * commits a `sys_file` and only the final `sys_attachment` insert is - * refused: the user meets the error after the bytes are stored, and the file - * stays committed with nothing attached to it. + * Upload needs `create`. Without it the attach at the end of the upload + * below is refused after the bytes are stored (the server leaves the file + * pending, and its retention reclaims it); hiding Upload spares the user + * that round trip. * * Delete needs `delete`, and being the uploader does not change that. The * server asks the object grant first (plugin-security's CRUD check wraps the @@ -252,9 +255,17 @@ export const RecordAttachmentsPanel: React.FC = ({ // upload adapter and the download-URL fetch — the storage routes require a // session and there is no cookie for `credentials: 'include'` to carry. const authFetch = React.useMemo(() => createAuthenticatedFetch(), []); + // Every upload from this panel is attached to this record by the call that + // completes it (`attachTo`): the server's attach verb, run as the user. const adapter = React.useMemo( - () => createObjectStackUploadAdapter({ baseUrl, scope: 'attachments', fetchImpl: authFetch }), - [baseUrl, authFetch], + () => + createObjectStackUploadAdapter({ + baseUrl, + scope: 'attachments', + fetchImpl: authFetch, + ...(objectName && recordId ? { attachTo: { parentObject: objectName, parentId: recordId } } : {}), + }), + [baseUrl, authFetch, objectName, recordId], ); /** Map the server's fail-closed 40x codes (objectstack-ai/objectstack#2755, objectstack-ai/objectstack#2970) to friendly copy. */ @@ -390,27 +401,18 @@ export const RecordAttachmentsPanel: React.FC = ({ const handleFiles = React.useCallback( async (files: FileList | null) => { - if (!files || files.length === 0 || !dataSource) return; + if (!files || files.length === 0 || !dataSource || !objectName || !recordId) return; setUploading(true); setError(null); try { for (const file of Array.from(files)) { - // 1) blob → sys_file via the canonical presigned three-step flow + // blob → sys_file → sys_attachment: the presigned three-step flow, + // whose completion attaches the file to this record as the user + // (the server checks the create grant, edit access to the record, + // `enable.files`, and that the file is the user's own upload). const uploaded = await adapter.upload(file); - const fileId = (uploaded.meta as any)?.fileId as string | undefined; - if (!fileId) throw new Error('Upload did not return a fileId'); - // 2) join row → sys_attachment (server enforces enable.files) - await dataSource.create('sys_attachment', { - parent_object: objectName, - parent_id: recordId, - file_id: fileId, - file_name: uploaded.name ?? file.name, - mime_type: uploaded.mimeType ?? file.type, - size: uploaded.size ?? file.size, - // Back-compat with pre-objectstack-ai/objectstack#2755 servers; a current server stamps - // `uploaded_by` from the session and ignores this value. - ...(currentUserId ? { uploaded_by: currentUserId } : {}), - }); + const attachmentId = (uploaded.meta as any)?.attachmentId as string | undefined; + if (!attachmentId) throw new Error('The server completed the upload but did not attach it to this record'); } await refresh(); } catch (err: any) { @@ -420,7 +422,7 @@ export const RecordAttachmentsPanel: React.FC = ({ if (inputRef.current) inputRef.current.value = ''; } }, - [adapter, dataSource, objectName, recordId, currentUserId, refresh], + [adapter, dataSource, objectName, recordId, refresh], ); /** diff --git a/ui/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.affordanceGrant-12047.test.tsx b/ui/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.affordanceGrant-12047.test.tsx index 46bf4020a2a..f103a56c512 100644 --- a/ui/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.affordanceGrant-12047.test.tsx +++ b/ui/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.affordanceGrant-12047.test.tsx @@ -36,12 +36,17 @@ import { render, screen, waitFor, within, act } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { MePermissionsProvider, type MePermissionsAuthenticatedResponse } from '@object-ui/permissions'; -/** The fake presigned adapter: `upload` resolving means a `sys_file` was committed. */ +/** The fake presigned adapter: `upload` resolving means the file was committed and attached. */ const adapterUpload = vi.fn(); +/** The options the panel built its adapter with — `attachTo` names the record. */ +const adapterOptions = vi.fn(); vi.mock('@object-ui/data-objectstack', async (importOriginal) => ({ ...(await importOriginal>()), - createObjectStackUploadAdapter: () => ({ upload: adapterUpload }), + createObjectStackUploadAdapter: (opts: unknown) => { + adapterOptions(opts); + return { upload: adapterUpload }; + }, })); import { RecordAttachmentsPanel } from '../RecordAttachmentsPanel'; @@ -174,20 +179,15 @@ describe('RecordAttachmentsPanel — write affordances follow the sys_attachment }); it('CONTROL — with Upload shown and the server refusing the attach, the refusal message still appears', async () => { - adapterUpload.mockResolvedValue({ - name: 'quote.pdf', - mimeType: 'application/pdf', - size: 10, - meta: { fileId: 'f-new' }, - }); - const dataSource = makeDataSource({ - create: vi.fn(async () => { - throw Object.assign(new Error("[Security] Access denied: operation 'insert' on sys_attachment"), { - httpStatus: 403, - code: 'PERMISSION_DENIED', - }); + // The attach is the upload's own completion (`attachTo`), so the server's + // refusal is the upload's. + adapterUpload.mockRejectedValue( + Object.assign(new Error('Upload completion failed (403): {"success":false,"error":{"code":"PERMISSION_DENIED"}}'), { + status: 403, + code: 'PERMISSION_DENIED', }), - }); + ); + const dataSource = makeDataSource(); const { container } = renderWithGrant({ allowCreate: true }, dataSource); await rowsLoaded(); expect(screen.getByRole('button', { name: 'Upload' })).toBeInTheDocument(); @@ -200,10 +200,11 @@ describe('RecordAttachmentsPanel — write affordances follow the sys_attachment expect(screen.getByRole('alert')).toHaveTextContent("You don't have permission to do that."), ); expect(adapterUpload).toHaveBeenCalledTimes(1); - expect(dataSource.create).toHaveBeenCalledWith( - 'sys_attachment', - expect.objectContaining({ parent_object: 'att_case', parent_id: 'r1', file_id: 'f-new' }), + expect(adapterOptions).toHaveBeenLastCalledWith( + expect.objectContaining({ scope: 'attachments', attachTo: { parentObject: 'att_case', parentId: 'r1' } }), ); + // No `sys_attachment` create through the generic data door. + expect(dataSource.create).not.toHaveBeenCalled(); }); it('no flash while permissions load: a read-only answer never shows Upload, not even before it arrives', async () => { diff --git a/ui/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx b/ui/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx index b57e023de4c..9b8148587a3 100644 --- a/ui/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx +++ b/ui/packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx @@ -846,3 +846,81 @@ describe('RecordAttachmentsPanel — mobile shell openFile', () => { expect(host.openFile).not.toHaveBeenCalled(); }); }); + +/** + * The upload attaches through the server's attach verb: the presigned upload's + * own completion carries `attachTo` (`POST /api/v1/storage/upload/complete`), + * and the server commits the file and writes its `sys_attachment` row as the + * signed-in user. The panel never creates `sys_attachment` through the generic + * data door, which refuses a member who may attach but cannot read `sys_file`. + * Driven through the REAL ObjectStack upload adapter over a stubbed `fetch`. + */ +describe('RecordAttachmentsPanel — upload attaches through the upload’s own completion', () => { + const ok = (data: unknown) => + new Response(JSON.stringify({ success: true, data }), { status: 200, headers: { 'content-type': 'application/json' } }); + + function stubStorage(complete: () => Response) { + const calls: Array<{ url: string; method: string; body: unknown }> = []; + const spy = vi.spyOn(globalThis, 'fetch').mockImplementation(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url; + const method = init?.method ?? 'GET'; + const body = typeof init?.body === 'string' ? JSON.parse(init.body) : init?.body; + calls.push({ url, method, body }); + if (url.endsWith('/api/v1/storage/upload/presigned')) { + return ok({ uploadUrl: '/api/v1/storage/_local/raw/tok', method: 'PUT', fileId: 'f-new', headers: {} }); + } + if (url.endsWith('/api/v1/storage/_local/raw/tok')) return ok({ key: 'attachments/f-new.pdf' }); + if (url.endsWith('/api/v1/storage/upload/complete')) return complete(); + throw new Error(`unexpected fetch ${method} ${url}`); + }); + return { spy, calls }; + } + + async function uploadOne() { + const dataSource = makeDataSource(); + const { container } = render( + , + ); + await waitFor(() => expect(screen.getByText('report.pdf')).toBeInTheDocument()); + const input = container.querySelector('input[type="file"]') as HTMLInputElement; + await userEvent.setup().upload(input, new File(['%PDF'], 'quote.pdf', { type: 'application/pdf' })); + return dataSource; + } + + afterEach(() => vi.restoreAllMocks()); + + it('completes the upload with attachTo naming this record, and creates no sys_attachment through the data door', async () => { + const { calls } = stubStorage(() => ok({ fileId: 'f-new', name: 'quote.pdf', attachmentId: 'a-new' })); + const dataSource = await uploadOne(); + await waitFor(() => expect(dataSource.find).toHaveBeenCalledTimes(2)); // the list re-read after the attach + const complete = calls.find((c) => c.url.endsWith('/upload/complete')); + expect(complete).toMatchObject({ + method: 'POST', + body: { fileId: 'f-new', attachTo: { parentObject: 'att_case', parentId: 'r1' } }, + }); + expect(calls.find((c) => c.url.endsWith('/upload/presigned'))?.body).toMatchObject({ scope: 'attachments' }); + expect(dataSource.create).not.toHaveBeenCalled(); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); + + it('maps the attach verb’s ATTACHMENT_PARENT_ACCESS refusal to the friendly copy', async () => { + stubStorage( + () => + new Response( + JSON.stringify({ success: false, error: { code: 'ATTACHMENT_PARENT_ACCESS', message: 'Cannot attach to att_case/r1' } }), + { status: 403, headers: { 'content-type': 'application/json' } }, + ), + ); + const dataSource = await uploadOne(); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent("You don't have access to attach files to this record."), + ); + expect(dataSource.create).not.toHaveBeenCalled(); + }); + + it('a completion that attached nothing is an error, never a silent success', async () => { + stubStorage(() => ok({ fileId: 'f-new', name: 'quote.pdf' })); + await uploadOne(); + await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('did not attach it to this record')); + }); +}); diff --git a/ui/packages/data-objectstack/src/upload-adapter.ts b/ui/packages/data-objectstack/src/upload-adapter.ts index 4cb6f33668c..46bb834e63b 100644 --- a/ui/packages/data-objectstack/src/upload-adapter.ts +++ b/ui/packages/data-objectstack/src/upload-adapter.ts @@ -45,6 +45,18 @@ export interface ObjectStackUploadAdapterOptions { * declares (objectui#12055). */ scope?: API.UploadScope; + /** + * Attach every upload from this adapter to this record, in the call that + * completes it: the completion request carries `attachTo`, and the server + * commits the file and writes its `sys_attachment` row as one unit, as the + * signed-in caller (the record attachments surface's attach verb). The + * result's `meta.attachmentId` names the new attachment. The server attaches + * only the caller's own pending upload in the `attachments` scope, so set + * `scope: 'attachments'` with it. A refused attach fails the upload with the + * server's refusal and leaves the file pending, never committed and + * unattached. + */ + attachTo?: { parentObject: string; parentId: string }; /** Override `fetch` (tests, ServiceWorker queues). */ fetchImpl?: typeof fetch; /** @@ -64,7 +76,8 @@ export interface ObjectStackUploadAdapterOptions { * 1. POST `/upload/presigned` → `{ uploadUrl, fileId, downloadUrl }` * 2. PUT `uploadUrl` → uploads the raw bytes (HMAC token) * 3. POST `/upload/complete` → marks the file ready and registers - * the `sys_file` record + * the `sys_file` record — and, with + * `attachTo`, attaches it to that record * * Falls back to a canonical (non-signed) download URL so the returned * `UploadResult.url` is stable across signed-URL expiry — components @@ -136,14 +149,27 @@ export function createObjectStackUploadAdapter( credentials, headers: { 'Content-Type': 'application/json' }, signal: options.signal, - body: JSON.stringify({ fileId }), + body: JSON.stringify(opts.attachTo ? { fileId, attachTo: opts.attachTo } : { fileId }), }); if (!completeRes.ok) { const text = await completeRes.text().catch(() => ''); - throw new Error( - `Upload completion failed (${completeRes.status}): ${text || completeRes.statusText}`, + // The server's refusal code and status ride on the error, so a caller + // maps a refused attach (`ATTACHMENT_PARENT_ACCESS`, `FILES_DISABLED`, + // `PERMISSION_DENIED`…) without parsing the message. + let code: string | undefined; + try { + const parsed = JSON.parse(text); + if (typeof parsed?.error?.code === 'string') code = parsed.error.code; + } catch { + // Not the declared envelope — the message keeps the raw text. + } + throw Object.assign( + new Error(`Upload completion failed (${completeRes.status}): ${text || completeRes.statusText}`), + { status: completeRes.status, ...(code ? { code } : {}) }, ); } + const completeBody = opts.attachTo ? await completeRes.json().catch(() => null) : null; + const attachmentId = (completeBody?.data ?? completeBody)?.attachmentId as string | undefined; // Use the canonical (stable) download endpoint as the field value // so a saved record keeps working past signed-URL expiry. This @@ -156,7 +182,7 @@ export function createObjectStackUploadAdapter( name, size: file.size, mimeType, - meta: { fileId, scope: opts.scope }, + meta: { fileId, scope: opts.scope, ...(attachmentId ? { attachmentId } : {}) }, }; }, }; From d2a2cde040841e1811ccff02ad9a27be16595fb4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 00:19:35 +0000 Subject: [PATCH 15/21] docs: the attachments panel attaches in the call that completes its upload Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- content/docs/api/plugin-endpoints.mdx | 9 ++++- .../docs/permissions/attachments-access.mdx | 34 +++++++++++++++++++ packages/services/service-storage/README.md | 2 +- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/content/docs/api/plugin-endpoints.mdx b/content/docs/api/plugin-endpoints.mdx index 24befc39509..e4834dce92a 100644 --- a/content/docs/api/plugin-endpoints.mdx +++ b/content/docs/api/plugin-endpoints.mdx @@ -154,7 +154,7 @@ other adapter. | Method | Endpoint | SDK | Description | |:-------|:---------|:----|:------------| | POST | `/storage/upload/presigned` | `storage.getPresignedUrl` | Step 1 — mint an upload target for a new `sys_file` row | -| POST | `/storage/upload/complete` | `storage.upload` | Step 3 — commit the uploaded bytes | +| POST | `/storage/upload/complete` | `storage.upload` | Step 3 — commit the uploaded bytes; with `attachTo`, also attach the file to a record as the caller | | POST | `/storage/upload/chunked` | `storage.initChunkedUpload` | Open a chunked / resumable session | | PUT | `/storage/upload/chunked/:uploadId/chunk/:chunkIndex` | `storage.uploadPart` | Send one chunk | | POST | `/storage/upload/chunked/:uploadId/complete` | `storage.completeChunkedUpload` | Assemble the parts | @@ -167,6 +167,13 @@ other adapter. so for a file whose type the browser does not know (`file.type === ''`) the helper sends `application/octet-stream`; a typed file keeps its own type. +To attach the upload to a record's Attachments, send `attachTo: { parentObject, +parentId }` with step 3 (and start the upload with `scope: 'attachments'`): the +commit and the `sys_attachment` row are one unit, written as the signed-in +caller, and the answer carries the `attachmentId` +([attachment access](/docs/permissions/attachments-access)). A refused attach +leaves the upload pending. + A file field takes the `fileId` only after step 3. A signed-in record write that names a file whose upload was never completed is refused with `400 VALIDATION_FAILED`, one `invalid_value` entry naming the field and the file, diff --git a/content/docs/permissions/attachments-access.mdx b/content/docs/permissions/attachments-access.mdx index 6760ee3ea6b..1f72d8bf5ba 100644 --- a/content/docs/permissions/attachments-access.mdx +++ b/content/docs/permissions/attachments-access.mdx @@ -72,6 +72,39 @@ drops a non-system caller's `uploaded_by` and reports it in `droppedFields`, and the rest of the write lands. System-context writes keep the value they supply. +### Attaching an upload from the browser + +The Attachments panel attaches a file in the call that completes its upload: +the presigned upload's `POST /api/v1/storage/upload/complete` carries +`attachTo: { parentObject, parentId }`, and the server commits the file and +writes its `sys_attachment` row as one unit, **as the signed-in user**. Every +check above judges that user: the permission to create an attachment, edit +access to the record, the record object's `enable.files`, and the file +reference itself. The answer carries the new `attachmentId`. A refused attach +leaves the upload `pending` and attached to nothing, and the pending-upload +retention reclaims it. + +The file must be the caller's own upload, still `pending`, in the +`attachments` scope, attached to nothing and owned by no record field, with its +bytes stored. Any other file id — another user's upload, an id that names no +file — is refused before anything is written: + +| Code | Status | When | +| --- | --- | --- | +| `PERMISSION_DENIED` | 403 | The file is another user's upload (the commit door's uploader rule) | +| `FILE_NOT_FOUND` | 404 | The id names no file | +| `RESOURCE_CONFLICT` | 409 | The upload is already completed, already attached or owned by a field, or its bytes are not stored | +| `INVALID_REQUEST` | 400 | `attachTo` is malformed, or the upload was not started in the `attachments` scope | +| `NOT_IMPLEMENTED` | 501 | The deployment composes no auth service or no data engine, so there is no caller to attach as | + +This is how a member who may attach files but holds no read on `sys_file` +attaches one: a `sys_attachment` create through the generic data API +(`POST /api/v1/data/sys_attachment`) names its `file_id` as any other +`lookup` value, which the caller must be able to read, and a file they cannot +read is refused `400 VALIDATION_FAILED` (`reference_not_found` on `file_id`). +The attach on completion vouches for that one file, in that one call, and +grants no read of `sys_file` or of anything else. + | Code | Status | When | | --- | --- | --- | | `RECORD_NOT_ACCESSIBLE` | 403 | The session names no signed-in user to record as the uploader | @@ -224,6 +257,7 @@ can be shared across records). Reclamation is handled by the platform LifecycleS | Operation | Requirement | Deny code | | --- | --- | --- | | Attach (create) | parent object opts in (`enable.files`) | `FILES_DISABLED` (403) | +| Attach on upload completion (`attachTo`) | the caller's own pending, unattached `attachments`-scope upload, then every create check | `PERMISSION_DENIED` (403) / `RESOURCE_CONFLICT` (409), then the create codes | | Attach (create), re-point | a by-id update of the parent record by the caller would be admitted | `ATTACHMENT_PARENT_ACCESS` (403); `OBJECT_NOT_FOUND` (404) when the parent object is not registered | | List / read | inherits parent read visibility | *(filtered out)* | | Delete | uploader or parent editor (+ RBAC delete grant) | `ATTACHMENT_DELETE_DENIED` (403); `PERMISSION_DENIED` (403) when the parent is not readable or no delete grant is held | diff --git a/packages/services/service-storage/README.md b/packages/services/service-storage/README.md index b40cee4e39d..386ce4d1392 100644 --- a/packages/services/service-storage/README.md +++ b/packages/services/service-storage/README.md @@ -129,7 +129,7 @@ All routes are mounted at `/api/v1/storage` (configurable via `basePath`). | Method | Path | Description | |--------|------|-------------| | POST | `/upload/presigned` | Get presigned upload URL | -| POST | `/upload/complete` | Mark upload as committed | +| POST | `/upload/complete` | Mark upload as committed; with `attachTo: { parentObject, parentId }`, attach the caller's own pending upload to that record as the caller, in the same unit | | POST | `/upload/chunked` | Initiate chunked upload | | PUT | `/upload/chunked/:uploadId/chunk/:chunkIndex` | Upload a chunk | | POST | `/upload/chunked/:uploadId/complete` | Complete chunked upload | From 7730c2fe08eb9a44a35fc6f326805e03f9fdce1f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 00:20:47 +0000 Subject: [PATCH 16/21] test(dogfood): a member with no sys_file read attaches through the panel's flow on the showcase boot Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- ...attachments-browser-attach.dogfood.test.ts | 225 ++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100644 packages/qa/dogfood/test/attachments-browser-attach.dogfood.test.ts diff --git a/packages/qa/dogfood/test/attachments-browser-attach.dogfood.test.ts b/packages/qa/dogfood/test/attachments-browser-attach.dogfood.test.ts new file mode 100644 index 00000000000..b99805f625d --- /dev/null +++ b/packages/qa/dogfood/test/attachments-browser-attach.dogfood.test.ts @@ -0,0 +1,225 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The console Attachments panel's flow, at the HTTP level, on the showcase +// boot: presigned upload → raw PUT → the upload-complete door with `attachTo`, +// which commits the file and writes its `sys_attachment` row as one unit, as +// the signed-in member (`FileAttachmentService.attachUploadAs`). +// +// The member holds what a real app grants a member who may attach files: the +// app's own `showcase_contributor` (they read and edit their own invoices) and +// a set with `sys_attachment` read and create, and nothing on `sys_file`. A +// `sys_attachment` create through the generic data door names `file_id` as +// any `lookup` value, which the caller must be able to read, so that member's +// two-step attach (complete, then `POST /data/sys_attachment`) is refused +// `reference_not_found`. The attach on completion vouches for the one upload +// it commits, and is what the panel calls. +// +// Controls: another user's upload id, an id that names no file, a record the +// member may not attach to, the generic door's direct create (still refused), +// and a member whose set reads every object (`'*'`), for whom both flows work. +// +// `showcase_invoice` declares `enable.files`; its select RLS +// (`owner == current_user.email`) hides another user's invoice from a +// contributor. + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { VerifyStack } from '@objectstack/verify'; +import type { IDataEngine } from '@objectstack/spec/contracts'; +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'; + +const SYS = { isSystem: true } as const; +const BYTES = 'hello'; + +/** The grant an app ships to let members attach files: `sys_attachment` only, nothing on `sys_file`. */ +const attacherSet = PermissionSetSchema.parse({ + name: 'browser_attach_dogfood_attacher', + label: 'Browser attach dogfood: attach files', + objects: { sys_attachment: { allowRead: true, allowCreate: true } }, +}); +/** A set that reads every object, `sys_file` included — the posture the permission-matrix fixture grants. */ +const wildcardReaderSet = PermissionSetSchema.parse({ + name: 'browser_attach_dogfood_wildcard_reader', + label: 'Browser attach dogfood: read everything', + objects: { '*': { allowRead: true }, sys_attachment: { allowRead: true, allowCreate: true } }, +}); + +describe('the Attachments panel’s upload flow on a real showcase boot: the upload’s own completion attaches, as the member', () => { + let stack: VerifyStack; + let rootDir: string; + let ql: IDataEngine; + let attacherToken: string; + let otherToken: string; + let wildcardToken: string; + let attacherId: string; + let attacherInvoice: string; + let othersInvoice: string; + let wildcardInvoice: string; + + beforeAll(async () => { + rootDir = mkdtempSync(join(tmpdir(), 'browser-attach-dogfood-')); + stack = await bootShowcase({ + security: new SecurityPlugin({ + ...appSecurityPluginOptions(showcaseStack), + defaultPermissionSets: [...securityDefaultPermissionSets, attacherSet, wildcardReaderSet], + }), + extraPlugins: [ + new StorageServicePlugin({ adapter: 'local', local: { rootDir }, bindToSettings: false }), + new AuditPlugin(), + ], + }); + ql = stack.kernel.getService('objectql'); + const member = async (email: string, sets: string[]) => { + const token = await stack.signUp(email); + const user = await ql.findOne('sys_user', { where: { email } }, { context: SYS }); + for (const permission_set of sets) { + await ql.insert('sys_user_permission_set', { user_id: user!.id, permission_set }, { context: SYS }); + } + return { token, id: String(user!.id) }; + }; + const attacher = await member('browser-attacher@dogfood.test', ['showcase_contributor', attacherSet.name]); + attacherToken = attacher.token; + attacherId = attacher.id; + otherToken = (await member('browser-other@dogfood.test', ['showcase_contributor', attacherSet.name])).token; + wildcardToken = (await member('browser-wildcard@dogfood.test', ['showcase_contributor', wildcardReaderSet.name])).token; + const [seeded] = await ql.find('showcase_invoice', { limit: 1 }, { context: SYS }); + const invoice = async (name: string, owner: string) => + String((await ql.insert('showcase_invoice', { name, owner, account: seeded.account, status: 'draft' }, { context: SYS })).id); + attacherInvoice = await invoice('INV-BROWSER-ATTACHER', 'browser-attacher@dogfood.test'); + othersInvoice = await invoice('INV-BROWSER-OTHER', 'someone-else@dogfood.test'); + wildcardInvoice = await invoice('INV-BROWSER-WILDCARD', 'browser-wildcard@dogfood.test'); + }, 180_000); + + afterAll(async () => { + await stack?.stop(); + rmSync(rootDir, { recursive: true, force: true }); + }); + + const json = { 'Content-Type': 'application/json' }; + const bearer = (token: string) => ({ Authorization: `Bearer ${token}` }); + + /** Steps 1 and 2 of the panel's flow: presign in the attachments scope, then PUT the bytes. */ + async function presignAndPut(token: string): Promise { + const presign = await stack.api('/storage/upload/presigned', { + method: 'POST', + headers: { ...json, ...bearer(token) }, + body: JSON.stringify({ filename: 'notes.txt', mimeType: 'text/plain', size: BYTES.length, scope: 'attachments' }), + }); + expect(presign.status, 'presign').toBe(200); + const { data } = (await presign.json()) as any; + const put = await stack.raw(String(data.uploadUrl).replace(/^https?:\/\/[^/]+/, ''), { + method: 'PUT', + headers: data.headers ?? { 'content-type': 'text/plain' }, + body: BYTES, + }); + expect(put.status, 'raw PUT').toBeLessThan(300); + return String(data.fileId); + } + + /** Step 3: the upload-complete door, with or without `attachTo`. */ + async function complete(token: string, fileId: string, attachTo?: { parentObject: string; parentId: string }) { + const res = await stack.api('/storage/upload/complete', { + method: 'POST', + headers: { ...json, ...bearer(token) }, + body: JSON.stringify(attachTo ? { fileId, attachTo } : { fileId }), + }); + return { status: res.status, body: (await res.json()) as any }; + } + + const onInvoice = (parentId: string) => ({ parentObject: 'showcase_invoice', parentId }); + const fileRow = (id: string) => ql.findOne('sys_file', { where: { id } }, { context: SYS }) as Promise; + const linksOf = (fileId: string) => + ql.find('sys_attachment', { where: { file_id: fileId } }, { context: SYS }) as Promise; + + it('precondition: the member cannot read sys_file through the data door', async () => { + const res = await stack.apiAs(attacherToken, 'GET', '/data/sys_file'); + expect(res.status).toBe(403); + }); + + it('the member attaches end to end: presign 200, PUT, complete with attachTo 200 — one committed file, one attachment in their name', async () => { + const fileId = await presignAndPut(attacherToken); + const out = await complete(attacherToken, fileId, onInvoice(attacherInvoice)); + expect(out.status, JSON.stringify(out.body)).toBe(200); + expect(out.body.data).toMatchObject({ fileId, name: 'notes.txt', attachmentId: expect.any(String) }); + expect(await fileRow(fileId)).toMatchObject({ status: 'committed', scope: 'attachments', owner_id: attacherId }); + const links = await linksOf(fileId); + expect(links).toHaveLength(1); + expect(links[0]).toMatchObject({ + id: out.body.data.attachmentId, parent_object: 'showcase_invoice', parent_id: attacherInvoice, uploaded_by: attacherId, + }); + // The panel's list read sees it, and the member downloads it. + const list = await stack.apiAs(attacherToken, 'GET', `/data/sys_attachment?parent_id=${attacherInvoice}`); + expect(list.status).toBe(200); + expect(JSON.stringify(await list.json())).toContain(out.body.data.attachmentId); + const url = await stack.api(`/storage/files/${fileId}/url`, { headers: bearer(attacherToken) }); + expect(url.status).toBe(200); + // No wider read: sys_file stays refused to them. + expect((await stack.apiAs(attacherToken, 'GET', `/data/sys_file/${fileId}`)).status).toBe(403); + }); + + it('control: the generic data door keeps its rule — the member’s two-step attach naming their own completed upload is refused reference_not_found', async () => { + const fileId = await presignAndPut(attacherToken); + expect((await complete(attacherToken, fileId)).status).toBe(200); + const res = await stack.apiAs(attacherToken, 'POST', '/data/sys_attachment', { + parent_object: 'showcase_invoice', parent_id: attacherInvoice, file_id: fileId, file_name: 'notes.txt', + }); + expect(res.status).toBe(400); + const body = JSON.stringify(await res.json()); + expect(body).toContain('VALIDATION_FAILED'); + expect(body).toContain('reference_not_found'); + expect(await linksOf(fileId)).toEqual([]); + }); + + it('control: another user’s upload id is refused, and their upload stays pending and unattached', async () => { + const othersFile = await presignAndPut(otherToken); + const out = await complete(attacherToken, othersFile, onInvoice(attacherInvoice)); + expect([out.status, out.body.error?.code]).toEqual([403, 'PERMISSION_DENIED']); + expect((await fileRow(othersFile)).status).toBe('pending'); + expect(await linksOf(othersFile)).toEqual([]); + }); + + it('control: an id that names no file is refused FILE_NOT_FOUND / 404', async () => { + const out = await complete(attacherToken, 'no-such-upload', onInvoice(attacherInvoice)); + expect([out.status, out.body.error?.code]).toEqual([404, 'FILE_NOT_FOUND']); + }); + + it('control: a record the member may not attach to (another user’s invoice) is refused, and the upload stays pending and unattached', async () => { + const fileId = await presignAndPut(attacherToken); + const out = await complete(attacherToken, fileId, onInvoice(othersInvoice)); + expect(out.status).toBeGreaterThanOrEqual(403); + expect(out.status).toBeLessThan(500); + expect(['ATTACHMENT_PARENT_ACCESS', 'RECORD_NOT_FOUND']).toContain(out.body.error?.code); + expect((await fileRow(fileId)).status).toBe('pending'); + expect(await linksOf(fileId)).toEqual([]); + }); + + it('control: an upload already attached cannot be attached again on completion', async () => { + const fileId = await presignAndPut(attacherToken); + expect((await complete(attacherToken, fileId, onInvoice(attacherInvoice))).status).toBe(200); + const again = await complete(attacherToken, fileId, onInvoice(attacherInvoice)); + expect([again.status, again.body.error?.code]).toEqual([409, 'RESOURCE_CONFLICT']); + expect(await linksOf(fileId)).toHaveLength(1); + }); + + it('control: a member whose set reads every object keeps working — through the attach on completion and through the generic two-step', async () => { + const viaComplete = await presignAndPut(wildcardToken); + const out = await complete(wildcardToken, viaComplete, onInvoice(wildcardInvoice)); + expect(out.status, JSON.stringify(out.body)).toBe(200); + expect(await linksOf(viaComplete)).toHaveLength(1); + + const viaDataDoor = await presignAndPut(wildcardToken); + expect((await complete(wildcardToken, viaDataDoor)).status).toBe(200); + const res = await stack.apiAs(wildcardToken, 'POST', '/data/sys_attachment', { + parent_object: 'showcase_invoice', parent_id: wildcardInvoice, file_id: viaDataDoor, file_name: 'notes.txt', + }); + expect(res.status, JSON.stringify(await res.clone().json())).toBeLessThan(300); + expect(await linksOf(viaDataDoor)).toHaveLength(1); + }); +}); From ce0325138337ea78317945aa1e4637ef3abe6e3e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 00:24:37 +0000 Subject: [PATCH 17/21] chore(changeset): the upload's own completion attaches a member's file from the browser Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../sec-r17-attach-on-upload-complete.md | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 .changeset/sec-r17-attach-on-upload-complete.md diff --git a/.changeset/sec-r17-attach-on-upload-complete.md b/.changeset/sec-r17-attach-on-upload-complete.md new file mode 100644 index 00000000000..1e95cf3b55d --- /dev/null +++ b/.changeset/sec-r17-attach-on-upload-complete.md @@ -0,0 +1,23 @@ +--- +'@objectstack/spec': patch +'@objectstack/core': patch +'@objectstack/objectql': patch +'@objectstack/service-storage': patch +'@object-ui/data-objectstack': patch +'@object-ui/app-shell': patch +--- + +fix(service-storage): a member attaches a file from the browser again — the upload's own completion attaches it as them, restoring the Attachments panel after #31031 + +Clause-②: yes (widening) +Metadata-protocol: none + +A signed-in member who may attach files, and holds no read of `sys_file`, can attach a file from a record's Attachments panel again. Since the write-time reference check began judging a `lookup` value through the caller's own read door, the panel's second step — `POST /api/v1/data/sys_attachment` naming the file it had just uploaded — was refused `400 VALIDATION_FAILED` (`file_id`, `reference_not_found`) for such a member. + +The rule now: the upload-complete door (`POST /api/v1/storage/upload/complete`) takes an optional `attachTo: { parentObject, parentId }`. With it, the door runs every gate it already asked — the session, the upload ownership rule, the starting organization, the held bytes — and then hands the upload to the file-attachments service's `attachUploadAs`, which runs as the caller. That verb vouches for one `sys_file` only: a `pending` upload in the `attachments` scope that the caller uploaded, that no record holds yet, in the caller's write reach and in the parent's organization. It asks the attach authorization first (the permission to create an attachment, the parent-edit attach rule, `enable.files`), then writes the `sys_attachment` row and commits the file as one unit. The answer is the committed file with its new `attachmentId`. Another user's upload id, an id that names no file, a committed or already attached upload, and a record the caller may not attach to are refused, and the upload stays `pending`. + +The vouch is spent on that one write: a hook that runs under it — as the user or as the system — writes without the mark (`withoutVouchedReferences`, `@objectstack/core`), so a hook naming the same `file_id` is judged through the caller's read door. A repo gate (`check:vouch-for-reference-callers`) pins the code that sets the mark to an allowlist of the file-attachments service. + +Unchanged: no caller gains a read of `sys_file` or of anything else. The generic data door keeps its rule — a `sys_attachment` create through `POST /api/v1/data/sys_attachment` naming a `sys_file` the caller cannot read is still refused `reference_not_found`. A completion without `attachTo` commits exactly as before. A mount with no file-attachments service bound refuses `attachTo` with `501 NOT_IMPLEMENTED`. + +The console's Attachments panel (`@object-ui/app-shell`) now completes its upload with `attachTo` and makes no second call, keeping its refusal messages; the upload adapter (`@object-ui/data-objectstack`) gains an `attachTo` option and returns the `attachmentId` it receives. From c418d5c657f2975b07452f0350cbdfa7142e18d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 00:49:25 +0000 Subject: [PATCH 18/21] ci: the vouched-reference census runs beside the single authz resolver guard; the tenant audit census counts the attach verb's insert Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .github/workflows/lint.yml | 12 ++++++- .../docs/permissions/tenant-audit-census.mdx | 32 +++++++++---------- ...08-tenant-audit-write-call-sites.counts.md | 14 ++++---- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index aa786e07fca..700b0baebc3 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -2550,10 +2550,20 @@ jobs: # @objectstack/core). Guards against a duplicate resolver copy drifting on a # security path (the REST-vs-dispatcher sys_user_role drift) and against an # entry point silently dropping the delegation. + # + # The second command is the same kind of guard on the write-time reference + # check: the code that may set the vouched-reference mark (a service verb + # vouching, on its own write, for the one record it proved) is pinned to an + # allowlist, so a new caller is a reviewed edit and never a quiet one. Its + # alias runs the census's `--self-test` first, which is what + # `check:self-test-wired` requires. Folded into this step rather than a step + # of its own: no new check context and no new required-status entry. - name: Single authz resolver guard id: single-authz-resolver-guard continue-on-error: true - run: pnpm check:authz-resolver + run: | + pnpm check:authz-resolver + pnpm check:vouch-for-reference-callers # #11352. Product code may not read a test-RUNNER variable (`TEST`, # `VITEST*`, `JEST_WORKER_ID`). Runner variables are INHERITED by every diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 0302c17a734..499deba0a13 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -93,7 +93,7 @@ what moved this page's population from 225 to 227; nothing about the two sites changed, only whether this instrument could see them. **The expensive failure direction is a keyword.** Sites whose receiver the author -typed `any` have no type to read, and there are 62 of them — just over a fifth +typed `any` have no type to read, and there are 63 of them — just over a fifth of the population, concentrated in exactly the seed and bootstrap paths this control exists for. Scoring an unreadable receiver as "not an engine" would have dropped every one of them silently, with a clean exit and a smaller number that @@ -132,7 +132,7 @@ are reported as `undecidable` rather than assumed either way. The same holds twice over for the context. An options argument spelled as a literal can be read; one spelled `options`, `{ ...opts }`, or handed through a -forwarding shim cannot, and **55 of the 265 sites are spelled that way**. A +forwarding shim cannot, and **55 of the 266 sites are spelled that way**. A context resolved from an inline literal or a local `const` can be tested for `isSystem`; one arriving from a helper call cannot. @@ -197,10 +197,10 @@ reproduce them. Where it disagrees, it disagrees on the page: | carried figure | where it survives | this census | | :--- | :--- | ---: | -| 175 write call sites | quoted in the merged changeset | **265** | +| 175 write call sites | quoted in the merged changeset | **266** | | 24 carrying no tenant context | quoted in the merged changeset | **2** provable and tenancy-enabled; **29** more whose options argument is unreadable | -| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **188 of 265** decidable, **77** undecidable | -| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 148 decidably elevated, 0 decidably not, 109 undecidable | +| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **189 of 266** decidable, **77** undecidable | +| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 148 decidably elevated, 0 decidably not, 110 undecidable | | 141 and 132, two independent re-derivations | the card that filed this work | — | **The differences are not reconciled, and deliberately so.** The old census's @@ -210,18 +210,18 @@ be stated is what this instrument counts, which is written above and re-runnable at any commit. Two structural facts do plausibly widen this reading against any hand or regex -one, and both are counted in the generated tables below: the 62 sites reached +one, and both are counted in the generated tables below: the 63 sites reached through an erased (`any`) receiver, and the 69 that name their object through a `const` rather than inline. An instrument that read either the way a person does would report a smaller number and would not say so. The fourth row is the one worth flagging to anyone citing it. **The 135 / 77% figure has no surviving corroboration anywhere in the tree.** This census reads -148 of 265 (56%) as decidably elevated, with 109 more whose elevation is a +148 of 266 (56%) as decidably elevated, with 110 more whose elevation is a run-time fact — so the claim is neither confirmed nor refuted, and the honest answer is that a static reading cannot settle it. -⇒ **Cite `2 / 265`, and say what it is**: the sites whose options argument was +⇒ **Cite `2 / 266`, and say what it is**: the sites whose options argument was READ and holds no tenant context, against a decidably tenancy-enabled object. That is the control's provable yield surface. ⛔ Do not cite it as "the sites without tenant context" — **29 further sites** have an options argument this @@ -233,28 +233,28 @@ cannot read, and they are neither in nor out. | what | count | | :--- | ---: | -| write call sites on the application surface | **265** | -| …whose object name is statically decidable | 188 | +| write call sites on the application surface | **266** | +| …whose object name is statically decidable | 189 | | …whose object name is chosen at run time | 77 | -| …against an object with tenancy ENABLED | 182 | +| …against an object with tenancy ENABLED | 183 | | …against an object that declares tenancy off | 6 | -| threading a tenant context | 202 | +| threading a tenant context | 203 | | PROVABLY carrying none (options read, no context key) | **8** | | …of those, against a decidably tenancy-enabled object | **2** | | options argument UNREADABLE — may or may not carry one | 55 | | …of those, against a decidably tenancy-enabled object | 29 | | threading a decidably ELEVATED (`isSystem`) context | 148 | | threading a context that is decidably NOT elevated | 0 | -| threading a context whose elevation is a run-time fact | 109 | +| threading a context whose elevation is a run-time fact | 110 | | how the instrument reached the site | count | | :--- | ---: | | receiver carried a readable engine type | 203 | -| receiver erased, placed by the object NAME | 52 | +| receiver erased, placed by the object NAME | 53 | | receiver erased, placed by an `object: string` PARAMETER | 5 | | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 5 | -| object name spelled inline | 119 | +| object name spelled inline | 120 | | object name spelled through a `const` | 69 | | object name is an `object: string` parameter | 7 | | object name is some other run-time expression | 70 | @@ -307,7 +307,7 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-11-06 at `4addb47fd`. +Measured on 2026-11-07 at `ce0325138`. | corpus scale (not enforced) | count | | :--- | ---: | diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index 2f57b162912..6d952f354cc 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -33,19 +33,19 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution. | Measure | Value | |---|---:| -| Write call sites | 265 | -| Object name statically decidable | 188 | +| Write call sites | 266 | +| Object name statically decidable | 189 | | Object name chosen at run time | 77 | -| Against a tenancy-enabled object | 182 | +| Against a tenancy-enabled object | 183 | | Against an object declaring tenancy off | 6 | -| Threading a tenant context | 202 | +| Threading a tenant context | 203 | | Provably carrying none | 8 | | …and decidably tenancy-enabled | 2 | | Options argument unreadable | 55 | | …and decidably tenancy-enabled | 29 | | Threading a decidably elevated context | 148 | | Threading a decidably non-elevated context | 0 | -| Threading a context of undecidable elevation | 109 | +| Threading a context of undecidable elevation | 110 | ## Subtractions the census could NOT defend — enforced @@ -90,7 +90,7 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-11-06 at `4addb47fd`. +Measured on 2026-11-07 at `ce0325138`. | corpus scale (not enforced) | count | | :--- | ---: | @@ -270,7 +270,7 @@ Measured on 2026-11-06 at `4addb47fd`. | `packages/services/service-storage/src/backfill-sys-file-organizations.ts` | `update` | `sys_file` | enabled | context, elevation undecidable | 1 | | `packages/services/service-storage/src/backfill-sys-file-public-scope.ts` | `update` | `sys_file` | enabled | context, elevation undecidable | 1 | | `packages/services/service-storage/src/file-attachment-service.ts` | `delete` | `sys_attachment` | enabled | context, elevation undecidable | 1 | -| `packages/services/service-storage/src/file-attachment-service.ts` | `insert` | `sys_attachment` | enabled | context, elevation undecidable | 1 | +| `packages/services/service-storage/src/file-attachment-service.ts` | `insert` | `sys_attachment` | enabled | context, elevation undecidable | 2 | | `packages/services/service-storage/src/file-reference-lifecycle.ts` | `insert` | `sys_file` | enabled | context, elevation undecidable | 1 | | `packages/services/service-storage/src/file-reference-lifecycle.ts` | `update` | `sys_file` | enabled | elevated | 2 | | `packages/services/service-storage/src/metadata-store.ts` | `delete` | `sys_file` | enabled | options unreadable | 1 | From a9adf172a9415f0d8bfaa893099ab3cdb05d988c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 01:28:23 +0000 Subject: [PATCH 19/21] chore: the vouched-reference census gate leaves this change for its own The census script, its root alias, the core repo pin, the Test Core input wiring and the lint workflow step move to a separate change. The attach on upload completion, the one-shot vouch and their pins stay. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../sec-r17-attach-on-upload-complete.md | 2 +- .github/workflows/lint.yml | 12 +- package.json | 1 - .../vouched-reference-callers.pin.test.ts | 38 --- packages/core/vitest.repo-tests.json | 1 - scripts/check-vouch-for-reference-callers.mjs | 286 ------------------ scripts/cross-package-test-inputs.mjs | 5 +- turbo.json | 3 +- 8 files changed, 4 insertions(+), 344 deletions(-) delete mode 100644 packages/core/src/security/vouched-reference-callers.pin.test.ts delete mode 100644 scripts/check-vouch-for-reference-callers.mjs diff --git a/.changeset/sec-r17-attach-on-upload-complete.md b/.changeset/sec-r17-attach-on-upload-complete.md index 1e95cf3b55d..3076e369730 100644 --- a/.changeset/sec-r17-attach-on-upload-complete.md +++ b/.changeset/sec-r17-attach-on-upload-complete.md @@ -16,7 +16,7 @@ A signed-in member who may attach files, and holds no read of `sys_file`, can at The rule now: the upload-complete door (`POST /api/v1/storage/upload/complete`) takes an optional `attachTo: { parentObject, parentId }`. With it, the door runs every gate it already asked — the session, the upload ownership rule, the starting organization, the held bytes — and then hands the upload to the file-attachments service's `attachUploadAs`, which runs as the caller. That verb vouches for one `sys_file` only: a `pending` upload in the `attachments` scope that the caller uploaded, that no record holds yet, in the caller's write reach and in the parent's organization. It asks the attach authorization first (the permission to create an attachment, the parent-edit attach rule, `enable.files`), then writes the `sys_attachment` row and commits the file as one unit. The answer is the committed file with its new `attachmentId`. Another user's upload id, an id that names no file, a committed or already attached upload, and a record the caller may not attach to are refused, and the upload stays `pending`. -The vouch is spent on that one write: a hook that runs under it — as the user or as the system — writes without the mark (`withoutVouchedReferences`, `@objectstack/core`), so a hook naming the same `file_id` is judged through the caller's read door. A repo gate (`check:vouch-for-reference-callers`) pins the code that sets the mark to an allowlist of the file-attachments service. +The vouch is spent on that one write: a hook that runs under it — as the user or as the system — writes without the mark (`withoutVouchedReferences`, `@objectstack/core`), so a hook naming the same `file_id` is judged through the caller's read door. Unchanged: no caller gains a read of `sys_file` or of anything else. The generic data door keeps its rule — a `sys_attachment` create through `POST /api/v1/data/sys_attachment` naming a `sys_file` the caller cannot read is still refused `reference_not_found`. A completion without `attachTo` commits exactly as before. A mount with no file-attachments service bound refuses `attachTo` with `501 NOT_IMPLEMENTED`. diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 700b0baebc3..aa786e07fca 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -2550,20 +2550,10 @@ jobs: # @objectstack/core). Guards against a duplicate resolver copy drifting on a # security path (the REST-vs-dispatcher sys_user_role drift) and against an # entry point silently dropping the delegation. - # - # The second command is the same kind of guard on the write-time reference - # check: the code that may set the vouched-reference mark (a service verb - # vouching, on its own write, for the one record it proved) is pinned to an - # allowlist, so a new caller is a reviewed edit and never a quiet one. Its - # alias runs the census's `--self-test` first, which is what - # `check:self-test-wired` requires. Folded into this step rather than a step - # of its own: no new check context and no new required-status entry. - name: Single authz resolver guard id: single-authz-resolver-guard continue-on-error: true - run: | - pnpm check:authz-resolver - pnpm check:vouch-for-reference-callers + run: pnpm check:authz-resolver # #11352. Product code may not read a test-RUNNER variable (`TEST`, # `VITEST*`, `JEST_WORKER_ID`). Runner variables are INHERITED by every diff --git a/package.json b/package.json index 16fff232e34..ceafb161b83 100644 --- a/package.json +++ b/package.json @@ -206,7 +206,6 @@ "check:stall-guard-headroom": "node scripts/measure-stall-guard-headroom.mjs --self-test", "check:swallow-census-controls": "node scripts/measure-durability-swallow-family.mjs --self-test=gated", "check:system-context-census": "node scripts/check-system-context-census.mjs --self-test && node scripts/check-system-context-census.mjs", - "check:vouch-for-reference-callers": "node scripts/check-vouch-for-reference-callers.mjs --self-test && node scripts/check-vouch-for-reference-callers.mjs", "gen:platform-object-tenancy-census": "node scripts/platform-object-tenancy-census.mjs --write", "check:platform-object-tenancy-census": "node scripts/check-platform-object-tenancy-census.mjs --self-test && node scripts/check-platform-object-tenancy-census.mjs" }, diff --git a/packages/core/src/security/vouched-reference-callers.pin.test.ts b/packages/core/src/security/vouched-reference-callers.pin.test.ts deleted file mode 100644 index b6c3af72643..00000000000 --- a/packages/core/src/security/vouched-reference-callers.pin.test.ts +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * The call-site census of the vouched-reference mark (`vouchForReference`, - * `./vouched-reference.ts`), run where `Test Core` runs it: the gate's own - * self-test (its red and green cases, with the battery floor and the verdict - * handshake), then the scan of this tree, which must be green — every - * production file that sets the mark is in the gate's `ALLOWED_CALLERS` with - * its exact call-site count. The gate's header is the authority on the rule. - */ - -import { describe, expect, it } from 'vitest'; -import { spawnSync } from 'node:child_process'; -import { dirname, resolve } from 'node:path'; -import { fileURLToPath } from 'node:url'; - -const HERE = dirname(fileURLToPath(import.meta.url)); -const REPO_ROOT = resolve(HERE, '../../../..'); -const GATE = resolve(HERE, '../../../../scripts/check-vouch-for-reference-callers.mjs'); - -const run = (...args: string[]) => - spawnSync(process.execPath, [GATE, ...args], { cwd: REPO_ROOT, encoding: 'utf8' }); - -describe('check:vouch-for-reference-callers', () => { - it('its self-test reaches its verdict: every red and green case holds, and no battery fell below its floor', () => { - const out = run('--self-test'); - expect(out.stderr).toBe(''); - expect(out.status).toBe(0); - expect(out.stdout).toMatch(/self-test: \d+ cases pass/); - }); - - it('this tree is green: the only production caller is the file-attachments writer, at its pinned count', () => { - const out = run(); - expect(out.stderr).toBe(''); - expect(out.status).toBe(0); - expect(out.stdout).toContain('packages/services/service-storage/src/file-attachment-service.ts (2)'); - }); -}); diff --git a/packages/core/vitest.repo-tests.json b/packages/core/vitest.repo-tests.json index c06e908a326..02fe9b15e9b 100644 --- a/packages/core/vitest.repo-tests.json +++ b/packages/core/vitest.repo-tests.json @@ -4,6 +4,5 @@ "src/security/operation-private-keys.pin.test.ts", "src/security/row-serving-door-exposure.pin.test.ts", "src/security/second-object-read-exposure.pin.test.ts", - "src/security/vouched-reference-callers.pin.test.ts", "src/utils/import-runner-sandbox-fault-row.test.ts" ] diff --git a/scripts/check-vouch-for-reference-callers.mjs b/scripts/check-vouch-for-reference-callers.mjs deleted file mode 100644 index 57cf0999fa5..00000000000 --- a/scripts/check-vouch-for-reference-callers.mjs +++ /dev/null @@ -1,286 +0,0 @@ -#!/usr/bin/env node -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * Call-site census of the vouched-reference mark. - * - * node scripts/check-vouch-for-reference-callers.mjs - * node scripts/check-vouch-for-reference-callers.mjs --self-test - * - * ## The rule it holds - * - * 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 - * (`assertReferencesResolve`, objectql). The one exception is a reference a - * service verb vouches for on its own write (`vouchForReference`, - * `@objectstack/core`): a record the same call authorized for the caller, set - * only after that verb's own authorization, and never on a context a request, - * an import, a flow or an agent tool hands the engine. Whether a call site - * honours that is a judgement a reviewer makes once, when the call site is - * written. This gate makes sure no call site arrives without that review: every - * production source file that sets the mark is named in {@link ALLOWED_CALLERS} - * with the exact number of call sites it holds, and the gate is red when a file - * outside the list sets it, when a listed file holds more or fewer call sites - * than its row, and when a row names a file that no longer sets it. - * - * "Sets the mark" is decided three ways, each a spelling that reaches the same - * context key: - * - * 1. a call `vouchForReference(` (not a mention in a comment or in backticks); - * 2. an import that renames it (`vouchForReference as …`), which would hide - * every later call from (1); - * 3. the key itself, outside the module that defines it — `VOUCHED_REFERENCES` - * used as a computed property, or `Symbol.for('objectstack.core.vouchedReferences')` - * spelled anywhere else — a context built with the key by hand. - * - * Test files (`*.test.*`, `*.spec.*`, `*.testkit.*`, `__tests__/`) are outside - * the census: they set the mark on purpose to pin what it admits. - * - * The scan surface is git's (`git ls-files --cached --others --exclude-standard` - * under `packages/`), so build output never reads as a call site. - * - * Adding a caller is an edit to {@link ALLOWED_CALLERS} in the same change, - * with the reason in the row: what the verb created or authorized for the - * caller, and which authorization it asked first. - */ - -import { execFileSync } from 'node:child_process'; -import { readFileSync } from 'node:fs'; -import { dirname, join } from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { isEntrypoint } from './invoked-as.mjs'; - -const REPO = join(dirname(fileURLToPath(import.meta.url)), '..'); - -/** The module that defines the mark; its own spellings are not call sites. */ -const DEFINING_MODULE = 'packages/core/src/security/vouched-reference.ts'; - -/** - * Every production file that sets the mark, with its exact call-site count and - * the reason it may. Equality, not a ceiling: one more call site is a new - * vouch nobody reviewed, one fewer is a row that no longer says what the file - * does. - */ -export const ALLOWED_CALLERS = Object.freeze({ - 'packages/services/service-storage/src/file-attachment-service.ts': { - count: 2, - reason: - 'the file-attachments writer: attachFileAs vouches for the sys_file it created in the same call, and ' + - "attachUploadAs for the caller's own unheld pending upload it commits in the same unit — each only for " + - 'sys_attachment.file_id, after that verb\'s own attach authorization', - }, -}); - -const SOURCE_FILE = /\.(?:[cm]?ts|tsx|[cm]?js|jsx)$/; -const TEST_FILE = /(?:\.test\.|\.spec\.|\.testkit\.|\/__tests__\/|\/__fixtures__\/)/; - -/** A line that is a comment, by its first non-blank characters. */ -const isCommentLine = (line) => /^\s*(?:\/\/|\/\*|\*)/.test(line); - -/** Strip backtick-quoted spans, so prose naming `vouchForReference(` is no call. */ -const withoutBacktickSpans = (line) => line.replace(/`[^`]*`/g, '``'); - -/** - * The census of one file's text: its call sites and the spellings that would - * hide or bypass one. - */ -export function censusOf(path, text) { - let calls = 0; - const findings = []; - const lines = String(text).split('\n'); - lines.forEach((raw, index) => { - if (isCommentLine(raw)) return; - const line = withoutBacktickSpans(raw.replace(/\/\/.*$/, '')); - const where = `${path}:${index + 1}`; - const callHits = line.match(/\bvouchForReference\s*\(/g); - if (callHits && path !== DEFINING_MODULE) calls += callHits.length; - if (/\bvouchForReference\s+as\s+\w/.test(line)) { - findings.push(`${where}: renames vouchForReference on import — every later call would hide from this census`); - } - if (path === DEFINING_MODULE) return; - if (/\[\s*VOUCHED_REFERENCES\s*\]/.test(line)) { - findings.push(`${where}: builds the mark's key by hand ([VOUCHED_REFERENCES]) — call vouchForReference instead`); - } - if (/Symbol\.for\(\s*['"]objectstack\.core\.vouchedReferences['"]\s*\)/.test(line)) { - findings.push(`${where}: spells the mark's symbol by hand — call vouchForReference instead`); - } - }); - return { calls, findings }; -} - -/** The verdict over a set of `{ path, text }` files against `allowed`. */ -export function check(files, allowed = ALLOWED_CALLERS) { - const problems = []; - if (files.length === 0) { - problems.push('the scan found no source files at all — an empty scan proves nothing, so it is refused'); - return { problems, counts: new Map() }; - } - const counts = new Map(); - for (const { path, text } of files) { - if (!SOURCE_FILE.test(path) || TEST_FILE.test(path)) continue; - const { calls, findings } = censusOf(path, text); - problems.push(...findings); - if (calls > 0) counts.set(path, calls); - } - for (const [path, calls] of counts) { - const row = allowed[path]; - if (!row) { - problems.push( - `${path}: sets the vouched-reference mark ${calls} time(s) and is not in ALLOWED_CALLERS. A vouch lets one ` + - "reference skip the caller's read door; add the row with its reason only after review, or write as the caller.", - ); - } else if (row.count !== calls) { - problems.push( - `${path}: sets the vouched-reference mark ${calls} time(s); its ALLOWED_CALLERS row pins ${row.count}. ` + - 'Every call site is reviewed on its own: update the row (and its reason) in the same change.', - ); - } - } - for (const path of Object.keys(allowed)) { - if (!counts.has(path)) { - problems.push(`${path}: is in ALLOWED_CALLERS and no longer sets the mark — remove the stale row.`); - } - } - return { problems, counts }; -} - -/** The files the census reads: what git sees under `packages/`, never build output. */ -function readTree() { - const out = execFileSync('git', ['ls-files', '--cached', '--others', '--exclude-standard', '-z', '--', 'packages'], { - cwd: REPO, - encoding: 'utf8', - maxBuffer: 256 * 1024 * 1024, - }); - const files = []; - for (const path of out.split('\0')) { - if (!path || !SOURCE_FILE.test(path) || TEST_FILE.test(path)) continue; - let text; - try { - text = readFileSync(join(REPO, path), 'utf8'); - } catch { - continue; // listed by git, deleted in the working tree - } - if (!text.includes('vouchForReference') && !text.includes('VOUCHED_REFERENCES') && !text.includes('vouchedReferences')) { - text = ''; - } - files.push({ path, text }); - } - return files; -} - -// ── Self-test ────────────────────────────────────────────────────────────── -// -// Table-driven, as `check-agent-model-declared.mjs`'s: each row LABEL is a -// declared battery with a floor of 1, registered as the first statement of the -// loop body; the roster's own size is pinned; and the verdict sets a flag the -// dispatch refuses without (AGENTS.md, "Writing a --self-test"). - -let selfTestReachedVerdict = false; - -const SELF_TEST_BATTERIES = Object.freeze({ - 'an allow-listed file with exactly its pinned call sites → GREEN': 1, - 'a call from a file outside the allow-list → RED': 1, - 'an allow-listed file with one more call site than its row → RED': 1, - 'an allow-listed file with one fewer call site than its row → RED': 1, - 'a row naming a file that no longer sets the mark → RED (stale row)': 1, - 'an import that renames vouchForReference → RED': 1, - 'the key built by hand as [VOUCHED_REFERENCES] → RED': 1, - "the symbol spelled by hand with Symbol.for → RED": 1, - 'a test file that sets the mark → GREEN (outside the census)': 1, - 'a comment or backtick mention is no call site → GREEN': 1, - 'the defining module’s own spellings → GREEN': 1, - 'an empty scan → RED, never a green skip': 1, -}); -const SELF_TEST_BATTERY_FLOOR = 12; - -function selfTest() { - const VERB = 'packages/services/demo/src/verb.ts'; - const OTHER = 'packages/rest/src/door.ts'; - const ALLOW = { [VERB]: { count: 1, reason: 'demo' } }; - const call = " const ctx = vouchForReference(context, { object: 'o', field: 'f', id });\n"; - const filler = { path: 'packages/core/src/other.ts', text: 'export const x = 1;\n' }; - - const cases = [ - { label: 'an allow-listed file with exactly its pinned call sites → GREEN', files: [{ path: VERB, text: call }], expect: 'green' }, - { label: 'a call from a file outside the allow-list → RED', files: [{ path: VERB, text: call }, { path: OTHER, text: call }], expect: 'red', names: OTHER }, - { label: 'an allow-listed file with one more call site than its row → RED', files: [{ path: VERB, text: call + call }], expect: 'red', names: VERB }, - { label: 'an allow-listed file with one fewer call site than its row → RED', files: [{ path: VERB, text: call }], allow: { [VERB]: { count: 2, reason: 'demo' } }, expect: 'red', names: VERB }, - { label: 'a row naming a file that no longer sets the mark → RED (stale row)', files: [filler], expect: 'red', names: VERB }, - { label: 'an import that renames vouchForReference → RED', files: [{ path: VERB, text: call }, { path: OTHER, text: "import { vouchForReference as vouch } from '@objectstack/core';\n" }], expect: 'red', names: OTHER }, - { label: 'the key built by hand as [VOUCHED_REFERENCES] → RED', files: [{ path: VERB, text: call }, { path: OTHER, text: ' const ctx = { ...context, [VOUCHED_REFERENCES]: [ref] };\n' }], expect: 'red', names: OTHER }, - { label: "the symbol spelled by hand with Symbol.for → RED", files: [{ path: VERB, text: call }, { path: OTHER, text: " const k = Symbol.for('objectstack.core.vouchedReferences');\n" }], expect: 'red', names: OTHER }, - { label: 'a test file that sets the mark → GREEN (outside the census)', files: [{ path: VERB, text: call }, { path: 'packages/rest/src/door.test.ts', text: call + ' ({ [VOUCHED_REFERENCES]: [] });\n' }], expect: 'green' }, - { label: 'a comment or backtick mention is no call site → GREEN', files: [{ path: VERB, text: call }, { path: OTHER, text: '// vouchForReference(context, ref) is the verb\'s\n * see vouchForReference( here\nconst s = `vouchForReference(x)`;\n' }], expect: 'green' }, - { label: 'the defining module’s own spellings → GREEN', files: [{ path: VERB, text: call }, { path: DEFINING_MODULE, text: "export const VOUCHED_REFERENCES = Symbol.for('objectstack.core.vouchedReferences');\nexport function vouchForReference(context, reference) {\n return { ...context, [VOUCHED_REFERENCES]: [] };\n}\n" }], expect: 'green' }, - { label: 'an empty scan → RED, never a green skip', files: [], expect: 'red', names: 'no source files' }, - ]; - - const batterySeen = new Map(); - let failed = 0; - for (const c of cases) { - batterySeen.set(c.label, (batterySeen.get(c.label) ?? 0) + 1); - const { problems } = check(c.files, c.allow ?? ALLOW); - const red = problems.length > 0; - const ok = - (c.expect === 'green' && !red) || - (c.expect === 'red' && red && (!c.names || problems.some((p) => p.includes(c.names)))); - if (!ok) { - failed += 1; - console.error(`✗ ${c.label}\n expected ${c.expect}${c.names ? ` naming ${c.names}` : ''}, got: ${red ? problems.join(' | ') : 'green'}`); - } - } - - const declared = Object.keys(SELF_TEST_BATTERIES); - const floorFailure = (message) => { - console.error(`✗ self-test floor: ${message}`); - failed += 1; - }; - if (declared.length < SELF_TEST_BATTERY_FLOOR) { - floorFailure(`SELF_TEST_BATTERIES declares ${declared.length} batteries, below the pinned ${SELF_TEST_BATTERY_FLOOR}.`); - } - const labels = cases.map((c) => c.label); - const duplicated = [...new Set(labels.filter((l, i) => labels.indexOf(l) !== i))]; - if (duplicated.length > 0) floorFailure(`row label(s) used more than once: ${duplicated.join(', ')}`); - for (const [name] of batterySeen) { - if (!declared.includes(name)) floorFailure(`battery "${name}" ran but is not declared in SELF_TEST_BATTERIES.`); - } - for (const name of declared) { - const seen = batterySeen.get(name) ?? 0; - if (seen < SELF_TEST_BATTERIES[name]) { - floorFailure(`battery "${name}" DID NOT RUN — ${seen} case(s) registered, ${SELF_TEST_BATTERIES[name]} pinned.`); - } - } - - if (failed > 0) { - console.error(`\n✗ check-vouch-for-reference-callers self-test: ${failed} failure(s) (cases and floor).`); - process.exit(1); - } - console.log(`✓ check-vouch-for-reference-callers self-test: ${cases.length} cases pass.`); - selfTestReachedVerdict = true; -} - -function main() { - if (process.argv.includes('--self-test')) { - selfTest(); - if (!selfTestReachedVerdict) { - console.error( - '✗ check-vouch-for-reference-callers self-test: selfTest() returned without reaching its verdict, so no ' + - 'success line was printed; exiting 0 would report a self-test that never finished as one that passed.', - ); - process.exit(1); - } - return; - } - const { problems, counts } = check(readTree()); - if (problems.length > 0) { - console.error(`✗ check-vouch-for-reference-callers: ${problems.length} finding(s)\n`); - for (const p of problems) console.error(` - ${p}`); - process.exit(1); - } - const listed = [...counts].map(([path, n]) => `${path} (${n})`).join(', '); - console.log(`✓ check-vouch-for-reference-callers: every call site that sets the vouched-reference mark is allow-listed — ${listed}.`); -} - -if (isEntrypoint(import.meta.url)) { - main(); -} diff --git a/scripts/cross-package-test-inputs.mjs b/scripts/cross-package-test-inputs.mjs index 1d6efc615c5..a05d602ff1e 100644 --- a/scripts/cross-package-test-inputs.mjs +++ b/scripts/cross-package-test-inputs.mjs @@ -349,10 +349,7 @@ export const CROSS_PACKAGE_TEST_INPUTS = { // hints: the case flipped from true to false with the added glob itself as // the covering hint. ⇒ Extensions and glob widen together or not at all, // and that pin's header carries the measurement of what the boundary costs. - // - // src/security/vouched-reference-callers.pin.test.ts runs the census gate - // of the vouched-reference mark, a root script, over the same `.ts` scan. - globs: ['packages/**/*.ts', 'scripts/check-vouch-for-reference-callers.mjs'], + globs: ['packages/**/*.ts'], }, '@objectstack/types': { // src/driver-error-classification.callers.test.ts is the #13440 gate: it diff --git a/turbo.json b/turbo.json index 4f4982518ee..a8caa1478a2 100644 --- a/turbo.json +++ b/turbo.json @@ -806,8 +806,7 @@ "!coverage/**", "!.turbo/**", "!**/node_modules/**", - "$TURBO_ROOT$/packages/**/*.ts", - "$TURBO_ROOT$/scripts/check-vouch-for-reference-callers.mjs" + "$TURBO_ROOT$/packages/**/*.ts" ] }, "@objectstack/types#test:repo": { From ad3ea1cfa849fcd29264bc4bbad26fcac9c7fccd Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 01:38:28 +0000 Subject: [PATCH 20/21] chore(changeset): the upload-complete request gains attachTo, so the protocol line is added and spec takes minor Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .changeset/sec-r17-attach-on-upload-complete.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.changeset/sec-r17-attach-on-upload-complete.md b/.changeset/sec-r17-attach-on-upload-complete.md index 3076e369730..e12e45b2c1f 100644 --- a/.changeset/sec-r17-attach-on-upload-complete.md +++ b/.changeset/sec-r17-attach-on-upload-complete.md @@ -1,5 +1,5 @@ --- -'@objectstack/spec': patch +'@objectstack/spec': minor '@objectstack/core': patch '@objectstack/objectql': patch '@objectstack/service-storage': patch @@ -10,7 +10,8 @@ fix(service-storage): a member attaches a file from the browser again — the upload's own completion attaches it as them, restoring the Attachments panel after #31031 Clause-②: yes (widening) -Metadata-protocol: none +Metadata-protocol: added +Skills-docs: updated here — `content/docs/permissions/attachments-access.mdx` and `content/docs/api/plugin-endpoints.mdx` state the `attachTo` key of the upload-complete request and its refusals; no file under `skills/**` teaches the upload-complete request (a `git grep` of `skills/` finds no `upload/complete` and no `CompleteUploadRequest`). A signed-in member who may attach files, and holds no read of `sys_file`, can attach a file from a record's Attachments panel again. Since the write-time reference check began judging a `lookup` value through the caller's own read door, the panel's second step — `POST /api/v1/data/sys_attachment` naming the file it had just uploaded — was refused `400 VALIDATION_FAILED` (`file_id`, `reference_not_found`) for such a member. From cb31937dcc5e30cb340f9bb565236a87d0c5da1b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 02:33:11 +0000 Subject: [PATCH 21/21] =?UTF-8?q?test(console):=20the=20write=20census=20d?= =?UTF-8?q?rops=20the=20attachments=20panel's=20create=20=E2=80=94=20the?= =?UTF-8?q?=20upload's=20own=20completion=20attaches=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../plugin-form/src/affordanceGrantMap-12082.test.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ui/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx b/ui/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx index d162fd0130b..6f802569d4c 100644 --- a/ui/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx +++ b/ui/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx @@ -680,8 +680,10 @@ const WRITE_SITES: Record = { outOfFamily("the language switcher changes the session's language locally and persists the caller's OWN sys_user.locale best-effort, a refusal surfacing as a toast"), 'packages/app-shell/src/hooks/useUserLocale.ts :: saveSignInLanguagePick :: update': outOfFamily("the sign-in page saves the language picked on it as the caller's OWN sys_user.locale, only while that column is unset; a refusal leaves the pick on this device and does not hold the sign-in (no CRUD visibility door)"), - 'packages/app-shell/src/views/RecordAttachmentsPanel.tsx :: RecordAttachmentsPanel > handleFiles :: create': - mapped(['attachmentUpload'], 'packages/app-shell/src/views/RecordAttachmentsPanel.tsx'), + // The attachments panel's upload has no data-source write site: the upload's + // own completion attaches it (the upload adapter's `attachTo`), so the + // `attachmentUpload` row is read where the upload is offered and no + // `create` is called. Its delete is still a data-source write. 'packages/app-shell/src/views/RecordAttachmentsPanel.tsx :: RecordAttachmentsPanel > handleDelete :: delete': mapped(['attachmentDelete'], 'packages/app-shell/src/views/RecordAttachmentsPanel.tsx'), 'packages/app-shell/src/views/RecordDetailView.tsx :: RecordDetailView > apiHandler :: update':