Skip to content

Commit 6ff78ac

Browse files
committed
test(service-storage): the commit door hands attachTo to the attacher only after its own gates
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
1 parent 64beb52 commit 6ff78ac

3 files changed

Lines changed: 231 additions & 3 deletions

File tree

‎packages/services/service-storage/src/file-attachment-service.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,7 @@ describe('attachUploadAs: a member attaches their own pending browser upload, an
724724
const others = await seedUpload(ql, storage, { id: 'upl_bob', owner: BOB.userId });
725725
const a = await refusalOf(asCaller(attachments).attachUploadAs(target(others), ALICE as never));
726726
const b = await refusalOf(asCaller(attachments).attachUploadAs(target('upl_nowhere'), ALICE as never));
727-
expect([b.code, b.status, b.message]).toEqual([a.code, a.status, a.message]);
727+
expect([b.code, b.status, (b as Error).message]).toEqual([a.code, a.status, (a as Error).message]);
728728
expect(await rows(ql, 'sys_attachment')).toEqual([]);
729729
});
730730

@@ -739,7 +739,7 @@ describe('attachUploadAs: a member attaches their own pending browser upload, an
739739
it('control: a pending upload something already holds is refused RESOURCE_CONFLICT / 409, and no second attachment is written', async () => {
740740
const { ql, attachments, storage } = await bootMember();
741741
const fileId = await seedUpload(ql, storage);
742-
await ql.insert('sys_attachment', { parent_object: 'att_case', parent_id: 'case_global', file_id: fileId, file_name: 'n.txt' }, SYS);
742+
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);
743743
const err = await refusalOf(asCaller(attachments).attachUploadAs(target(fileId), ALICE as never));
744744
expect(answer(err)).toEqual({ code: 'RESOURCE_CONFLICT', status: 409 });
745745
expect(await rows(ql, 'sys_attachment')).toHaveLength(1);

‎packages/services/service-storage/src/file-attachment-service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ export class FileAttachmentService implements IFileAttachmentService, ICallerFil
375375
// and for no other value. The write-time reference check does not
376376
// ask the caller's read door for that id, and still probes that it
377377
// exists; every other gate on the insert judges the caller.
378-
{ context: vouchForReference(unitContext, { object: 'sys_attachment', field: 'file_id', id: file.id }) },
378+
{ context: vouchForReference(unitContext as ExecutionContext, { object: 'sys_attachment', field: 'file_id', id: file.id }) },
379379
)) as { id?: unknown } | null | undefined;
380380
if (row?.id === undefined || row.id === null || row.id === '') {
381381
throw new Error('attachUploadAs: the sys_attachment insert answered no row id, so the attachment cannot be named');
Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// The commit door's `attachTo` arm (`POST …/upload/complete`): the record
4+
// attachments surface's attach verb. With `attachTo`, the door hands the
5+
// caller's own upload to the attacher `composeStorageRoutes` binds (the
6+
// file-attachments service's `attachUploadAs`, as the caller) instead of
7+
// committing it itself, and answers the committed file with its new
8+
// `attachmentId`.
9+
//
10+
// Pinned here at the door: every gate the door already asked runs FIRST —
11+
// the session, a malformed `attachTo`, a file that does not exist, the upload
12+
// ownership rule, the bytes — and a refused request never reaches the
13+
// attacher; the attacher's declared refusals are relayed with their code,
14+
// status and details; a mount with no attacher bound refuses `attachTo` with
15+
// `501`; and a completion without `attachTo` is unchanged. The attach verb's
16+
// own checks and its one-file vouch are pinned in
17+
// `file-attachment-service.test.ts`, and the whole flow on a real boot in the
18+
// `attachments-browser-attach` dogfood.
19+
20+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
21+
import { promises as fs } from 'node:fs';
22+
import { join } from 'node:path';
23+
import { tmpdir } from 'node:os';
24+
import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
25+
import { LocalStorageAdapter } from './local-storage-adapter.js';
26+
import { StorageMetadataStore } from './metadata-store.js';
27+
import {
28+
registerStorageFamily,
29+
registerStorageRoutes,
30+
type StorageUploadAttacher,
31+
type StorageUploadSession,
32+
} from './storage-routes.js';
33+
34+
const BASE = '/api/v1/storage';
35+
const COMMIT = `${BASE}/upload/complete`;
36+
37+
type Res = IHttpResponse & { _status: number; _json: any };
38+
39+
function createMockHttpServer() {
40+
const routes = new Map<string, RouteHandler>();
41+
return {
42+
get: vi.fn((path: string, handler: RouteHandler) => { routes.set(`GET:${path}`, handler); }),
43+
post: vi.fn((path: string, handler: RouteHandler) => { routes.set(`POST:${path}`, handler); }),
44+
put: vi.fn((path: string, handler: RouteHandler) => { routes.set(`PUT:${path}`, handler); }),
45+
delete: vi.fn(),
46+
patch: vi.fn(),
47+
use: vi.fn(),
48+
listen: vi.fn().mockResolvedValue(undefined),
49+
close: vi.fn().mockResolvedValue(undefined),
50+
handler(method: string, path: string): RouteHandler {
51+
const h = routes.get(`${method}:${path}`);
52+
if (!h) throw new Error(`no route ${method} ${path}`);
53+
return h;
54+
},
55+
};
56+
}
57+
58+
function createMockRes(): Res {
59+
const res: any = {
60+
_status: 200,
61+
_json: null,
62+
json(data: any) { res._json = data; },
63+
send(data: any) { res._sent = data; },
64+
status(code: number) { res._status = code; return res; },
65+
header() { return res; },
66+
};
67+
return res;
68+
}
69+
70+
const ALICE: StorageUploadSession = { userId: 'usr_alice' };
71+
const BOB: StorageUploadSession = { userId: 'usr_bob' };
72+
const BYTES = Buffer.from('hello');
73+
74+
let rootDir: string;
75+
let storage: LocalStorageAdapter;
76+
let store: StorageMetadataStore;
77+
let server: ReturnType<typeof createMockHttpServer>;
78+
let attacher: ReturnType<typeof vi.fn>;
79+
80+
/** A session per request, read off a test header — the door's `resolveSession`. */
81+
const resolveSession = async (req: { headers?: unknown }) => {
82+
const who = (req.headers as Record<string, string> | undefined)?.['x-test-user'];
83+
return who === 'alice' ? ALICE : who === 'bob' ? BOB : null;
84+
};
85+
86+
function mount(attach: StorageUploadAttacher | undefined) {
87+
server = createMockHttpServer();
88+
registerStorageFamily(server as any, storage, store, { basePath: BASE, resolveSession }, undefined, attach);
89+
}
90+
91+
/** A pending upload of Alice's, its bytes held — what the presigned door and the raw PUT leave. */
92+
async function seedUpload(over: { id?: string; owner?: string; bytes?: boolean } = {}) {
93+
const id = over.id ?? 'upl_alice';
94+
const key = `attachments/${id}.txt`;
95+
await store.createFile({
96+
id, key, name: 'notes.txt', mime_type: 'text/plain', size: BYTES.byteLength, scope: 'attachments',
97+
acl: 'private', status: 'pending', owner_id: over.owner ?? ALICE.userId,
98+
});
99+
if (over.bytes !== false) await storage.upload(key, BYTES, { contentType: 'text/plain' });
100+
return id;
101+
}
102+
103+
async function complete(who: 'alice' | 'bob' | null, body: Record<string, unknown>): Promise<Res> {
104+
const res = createMockRes();
105+
const req = { params: {}, query: {}, body, headers: who ? { 'x-test-user': who } : {}, method: 'POST', path: COMMIT } as unknown as IHttpRequest;
106+
await server.handler('POST', COMMIT)(req, res);
107+
return res;
108+
}
109+
110+
const ATTACH_TO = { parentObject: 'att_case', parentId: 'case_a' };
111+
112+
beforeEach(async () => {
113+
rootDir = join(tmpdir(), `os-complete-attach-${Date.now()}-${Math.random().toString(36).slice(2)}`);
114+
await fs.mkdir(rootDir, { recursive: true });
115+
storage = new LocalStorageAdapter({ rootDir, signingSecret: 'complete-attach' });
116+
store = new StorageMetadataStore(null);
117+
attacher = vi.fn(async () => ({ attachmentId: 'att_1' }));
118+
mount(attacher as unknown as StorageUploadAttacher);
119+
});
120+
121+
afterEach(async () => {
122+
vi.restoreAllMocks();
123+
await fs.rm(rootDir, { recursive: true, force: true });
124+
});
125+
126+
describe('POST /storage/upload/complete with attachTo — the attach verb at the door', () => {
127+
it('hands the caller’s own upload to the attacher, does not commit it itself, and answers the file with its attachmentId', async () => {
128+
const fileId = await seedUpload();
129+
const res = await complete('alice', { fileId, eTag: 'etag-1', attachTo: ATTACH_TO });
130+
expect(res._status, JSON.stringify(res._json)).toBe(200);
131+
expect(attacher).toHaveBeenCalledTimes(1);
132+
expect(attacher.mock.calls[0][1]).toEqual({ fileId, ...ATTACH_TO, eTag: 'etag-1' });
133+
expect(res._json).toMatchObject({ success: true, data: { fileId, name: 'notes.txt', attachmentId: 'att_1' } });
134+
// The commit is the attacher's (one unit with the insert): this stand-in
135+
// committed nothing, and the door wrote nothing of its own.
136+
expect((await store.getFile(fileId))?.status).toBe('pending');
137+
});
138+
139+
it('control: another user’s upload is refused by the upload ownership rule before the attacher is asked', async () => {
140+
const fileId = await seedUpload({ owner: BOB.userId });
141+
const res = await complete('alice', { fileId, attachTo: ATTACH_TO });
142+
expect([res._status, res._json?.error?.code]).toEqual([403, 'PERMISSION_DENIED']);
143+
expect(attacher).not.toHaveBeenCalled();
144+
expect((await store.getFile(fileId))?.status).toBe('pending');
145+
});
146+
147+
it('control: an id that names no file is refused FILE_NOT_FOUND / 404 before the attacher is asked', async () => {
148+
const res = await complete('alice', { fileId: 'upl_nowhere', attachTo: ATTACH_TO });
149+
expect([res._status, res._json?.error?.code]).toEqual([404, 'FILE_NOT_FOUND']);
150+
expect(attacher).not.toHaveBeenCalled();
151+
});
152+
153+
it('control: an anonymous request is refused AUTH_REQUIRED / 401 before anything is read', async () => {
154+
const fileId = await seedUpload();
155+
const res = await complete(null, { fileId, attachTo: ATTACH_TO });
156+
expect([res._status, res._json?.error?.code]).toEqual([401, 'AUTH_REQUIRED']);
157+
expect(attacher).not.toHaveBeenCalled();
158+
});
159+
160+
it('control: an upload whose bytes are not held is refused RESOURCE_CONFLICT / 409 before the attacher is asked', async () => {
161+
const fileId = await seedUpload({ bytes: false });
162+
const res = await complete('alice', { fileId, attachTo: ATTACH_TO });
163+
expect([res._status, res._json?.error?.code]).toEqual([409, 'RESOURCE_CONFLICT']);
164+
expect(attacher).not.toHaveBeenCalled();
165+
});
166+
167+
for (const [what, attachTo] of [
168+
['a missing parentId', { parentObject: 'att_case' }],
169+
['an empty parentObject', { parentObject: '', parentId: 'case_a' }],
170+
['a key attachTo does not declare', { ...ATTACH_TO, file_id: 'other' }],
171+
['an array', [ATTACH_TO]],
172+
['null', null],
173+
] as const) {
174+
it(`control: attachTo with ${what} is refused INVALID_REQUEST / 400, and nothing is read, attached or committed`, async () => {
175+
const fileId = await seedUpload();
176+
const res = await complete('alice', { fileId, attachTo });
177+
expect([res._status, res._json?.error?.code]).toEqual([400, 'INVALID_REQUEST']);
178+
expect(attacher).not.toHaveBeenCalled();
179+
expect((await store.getFile(fileId))?.status).toBe('pending');
180+
});
181+
}
182+
183+
it('relays the attacher’s declared refusal with its code and status, and the upload is left pending', async () => {
184+
attacher.mockRejectedValueOnce(Object.assign(new Error('Cannot attach to att_case/case_a'), { code: 'ATTACHMENT_PARENT_ACCESS', status: 403 }));
185+
const fileId = await seedUpload();
186+
const res = await complete('alice', { fileId, attachTo: ATTACH_TO });
187+
expect([res._status, res._json?.error?.code]).toEqual([403, 'ATTACHMENT_PARENT_ACCESS']);
188+
expect(res._json.error.message).toBe('Cannot attach to att_case/case_a');
189+
expect((await store.getFile(fileId))?.status).toBe('pending');
190+
});
191+
192+
it('relays a validation refusal from the insert as VALIDATION_FAILED / 400 with its field details', async () => {
193+
const validation = Object.assign(new Error('file_id: reference not found'), {
194+
name: 'ValidationError',
195+
fields: [{ field: 'file_id', code: 'reference_not_found', message: 'reference not found' }],
196+
});
197+
attacher.mockRejectedValueOnce(validation);
198+
const fileId = await seedUpload();
199+
const res = await complete('alice', { fileId, attachTo: ATTACH_TO });
200+
expect([res._status, res._json?.error?.code]).toEqual([400, 'VALIDATION_FAILED']);
201+
});
202+
203+
it('an undeclared fault is a 500 with the door’s generic text, never the thrown message', async () => {
204+
attacher.mockRejectedValueOnce(new Error('engine said something about the host'));
205+
const fileId = await seedUpload();
206+
const res = await complete('alice', { fileId, attachTo: ATTACH_TO });
207+
expect(res._status).toBe(500);
208+
expect(res._json.error.message).not.toContain('engine said');
209+
});
210+
211+
it('a mount with no attacher bound (the public registrar, a bare kernel) refuses attachTo NOT_IMPLEMENTED / 501 and commits nothing', async () => {
212+
server = createMockHttpServer();
213+
registerStorageRoutes(server as any, storage, store, { basePath: BASE, resolveSession });
214+
const fileId = await seedUpload();
215+
const res = await complete('alice', { fileId, attachTo: ATTACH_TO });
216+
expect([res._status, res._json?.error?.code]).toEqual([501, 'NOT_IMPLEMENTED']);
217+
expect((await store.getFile(fileId))?.status).toBe('pending');
218+
});
219+
220+
it('control: a completion without attachTo commits as before and never asks the attacher', async () => {
221+
const fileId = await seedUpload();
222+
const res = await complete('alice', { fileId });
223+
expect(res._status).toBe(200);
224+
expect(res._json.data.attachmentId).toBeUndefined();
225+
expect((await store.getFile(fileId))?.status).toBe('committed');
226+
expect(attacher).not.toHaveBeenCalled();
227+
});
228+
});

0 commit comments

Comments
 (0)