diff --git a/docs/spec.md b/docs/spec.md index ff9232d..62e89d9 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -605,10 +605,20 @@ The adapter contract is split into two focused interfaces that are composed into **`StackBlobAdapter`** — binary storage: `putAttachment`, `getAttachment`, `deleteAttachment`, an optional `listFiles()` capability, and optional lifecycle hooks. ```ts -type StackAdapter = StackRecordAdapter & StackBlobAdapter; +type StackAdapter = StackRecordAdapter & + StackBlobAdapter & { + // Optional: bytes + _attachment@1 record as one atomic operation (#106) + putAttachmentWithMetadata?( + data: Uint8Array, + mimeType: string, + filename?: string, + ): Promise; + }; ``` -**Optional capabilities**, present on some adapters and not others, follow one pattern throughout: an optional interface method, checked for truthiness at the call site, with a described fallback when absent. `StackRecordAdapter.deleteUnreferencedAttachmentRecords()` (atomic reference check, [Attachments](#attachments)) and `StackBlobAdapter.listFiles()` (blob enumeration, used by [`collectAttachmentGarbage()`](#garbage-collection) to find bare-bytes orphans) are both this shape — no boolean flag in `capabilities`, just an optional method a caller checks for before using. `combineAdapters()` (below) preserves this: it forwards an optional method only when the underlying part actually implements it, never as a wrapper around a missing one. +**Optional capabilities**, present on some adapters and not others, follow one pattern throughout: an optional interface method, checked for truthiness at the call site, with a described fallback when absent. `StackRecordAdapter.deleteUnreferencedAttachmentRecords()` (atomic reference check, [Attachments](#attachments)), `StackBlobAdapter.listFiles()` (blob enumeration, used by [`collectAttachmentGarbage()`](#garbage-collection) to find bare-bytes orphans), and `StackAdapter.putAttachmentWithMetadata()` (atomic upload, below) are all this shape — no boolean flag in `capabilities`, just an optional method a caller checks for before using. `combineAdapters()` (below) preserves this: it forwards an optional method only when the underlying part actually implements it, never as a wrapper around a missing one. + +`StackAdapter.putAttachmentWithMetadata(data, mimeType, filename?)` (#106) stores bytes and creates the accompanying `_attachment@1` record as **one atomic operation**, returning the created record. It is declared on the composed `StackAdapter` type rather than on either half, because neither half can ever have it: a blob adapter has no record store and a record adapter has no byte store — "bytes + record in one operation" is a property only a whole adapter can offer. Today exactly one does: the API adapter, backed by a single `POST /attachments` request the server fulfills atomically. Local storage adapters don't implement it, and `combineAdapters()` never synthesizes it from parts (a record backend and a blob backend glued together have no shared transaction). `Stack.putAttachment()` checks for it — present means delegate the whole operation and trust the returned record as backend-authoritative; absent means the bytes-then-`create()` sequence `Stack.putAttachment()` has always used. See [Attachments](#attachments) for why the atomic form is a correctness requirement (#106's anti-oracle fix), not an efficiency optimization. ### Package naming convention @@ -699,9 +709,6 @@ const meta = results.records[0]?.content as AttachmentContent | undefined; - `Stack.putAttachment(data, mimeType, filename?)` — owner-level upload. Creates an `_attachment@1` record with no `entityId`. No grant check. - `ScopedStack.putAttachment(data, mimeType, filename?)` — entity-scoped upload. Requires a `create` grant on `_attachment@1`. The created record's `entityId` is set to the uploading entity. -- `Stack.putAttachmentBytes(data)` — owner-level, bytes only. Stores the file and returns its `fileId` without creating an `_attachment@1` record. No grant check. -- `ScopedStack.putAttachmentBytes(data)` — entity-scoped, bytes only. Gated identically to `ScopedStack.putAttachment()` (a `create` grant on `_attachment@1`): a bytes upload is only meaningful as a precursor to metadata creation, so the two share one authorization check. Anonymous requesters are always denied. - - `Stack.getAttachment(fileId)` — no permission check; always succeeds if the bytes exist. - `ScopedStack.getAttachment(fileId)` — accessible if the requester is the owner, can read any record that references the file, or uploaded the file themselves and it hasn't been associated with a record yet. Throws `StackPermissionError` otherwise. @@ -716,6 +723,14 @@ const meta = results.records[0]?.content as AttachmentContent | undefined; **Once created, an `_attachment@1` record's `fileId`, `size`, and `mimeType` are immutable — `filename` is the only field `update()` may change.** `fileId` and `size` describe the bytes themselves, so any attempted change is rejected (`StackValidationError`, 422). `mimeType`'s value was already pinned to the `fileId`'s established type at create time, so `update()` rejects any patch that touches it at all, including one that restates the same value — there's nothing a legitimate `mimeType` edit could accomplish that isn't already covered by the create-time rule above. To correct a wrongly-declared type, delete the attachment and re-upload: identical bytes hash to the same `fileId`, and the fresh first record establishes the corrected type. +**Creating `_attachment@1` records directly (#106).** `_attachment@1` records are access-conveying: `getAttachment()` and reference-creation (above) both grant access to a `fileId` a requester merely names in a readable record. `putAttachment()` is safe to expose to non-owners because it never lets the caller name that `fileId` — it computes one from bytes it just hashed, so possession is proven by construction. Generic `create()` has no such proof: its `fileId` is a plain caller-supplied string. So `ScopedStack.create()` refuses to create an `_attachment@1` record for any non-owner requester — `StackPermissionError` — even with an otherwise-sufficient `create` grant on the type. Without this, a bare `create` grant on `_attachment@1` (held by every uploader, by design) would let a requester name an arbitrary guessed `fileId` and, via `getAttachment()`'s uploader clause, turn a correct guess into a read. + +One carve-out: a non-owner who can already read some record referencing `fileId` may create an additional `_attachment@1` record for it (e.g. to record their own `filename`) without re-uploading bytes — this conveys no access they didn't already have. The carve-out is satisfied only by a readable referencing record, never by the requester's own prior `_attachment@1` record for the same `fileId` (the uploader clause of `getAttachment()`'s access rule) — allowing that would let one successful guess unlock unlimited further metadata records for the same guessed `fileId`. + +The owner and unscoped `Stack` are unaffected by this restriction — it applies to `ScopedStack.create()` only. `ScopedStack.putAttachment()` is unaffected too: having already derived `fileId` from bytes it hashed itself, it creates its record directly, bypassing this gate rather than satisfying it. + +**Anti-oracle.** The `mimeType`-conflict message (above) never names the established `mimeType` — doing so would confirm an existing `fileId`'s content type to a caller who only guessed the `fileId`, exactly the confirmation oracle the create refusal above is designed to close. + ### Garbage collection Attachment bytes are only ever removed by an explicit `deleteAttachment(fileId)` call — normal app flows delete _records_, and nothing notices when the last record referencing a file goes away. `collectAttachmentGarbage()` is an explicit, owner-invoked sweep that finds and removes those orphans. It is **not** automatic reference-counting: auto-delete on last dissociate/record-delete would coupling every record write to blob lifecycle and would race without a transactional adapter. @@ -731,7 +746,7 @@ stack.collectAttachmentGarbage(opts?: { **`_attachment@1` metadata records never themselves count as references** — otherwise nothing would ever be garbage — but a file's _newest_ metadata record (or, for bare bytes with no metadata record at all, the blob's own storage timestamp) must be older than `graceMs` to be collected. This protects the legitimate upload-then-associate window: a file just uploaded and not yet attached to anything is not yet garbage, just new. -**Bare-bytes orphans** — bytes stored by `putAttachmentBytes()`/`putAttachment()` with no `_attachment@1` record at all (e.g. a crash between storing bytes and writing metadata) — are only discoverable by enumerating the blob store directly, via the optional `StackBlobAdapter.listFiles()` capability (see [Adapters](#adapters)). An adapter that doesn't implement it still gets full protection for the common case (metadata-tracked files with no remaining reference); it simply can't find this rarer orphan class. +**Bare-bytes orphans** — bytes with no `_attachment@1` record at all (a `putAttachment()` that stored bytes on a non-atomic adapter but crashed before writing metadata) — are only discoverable by enumerating the blob store directly, via the optional `StackBlobAdapter.listFiles()` capability (see [Adapters](#adapters)). An adapter that doesn't implement it still gets full protection for the common case (metadata-tracked files with no remaining reference); it simply can't find this rarer orphan class. Deletion goes through `deleteAttachment()` itself, so its usual conflict check runs once more per file at delete time. A file that turns out to be referenced again (or already gone) by then is skipped, not treated as a sweep failure — the sweep always completes and reports what it actually collected. @@ -1000,7 +1015,7 @@ If-Match: "5" When present, the server applies the mutation only if the record's current version equals the header's value; otherwise it returns **412** with a `version_conflict` wire error (see [Error responses](#error-responses)) and changes nothing. Omit the header to keep unconditional last-writer-wins behavior — this is opt-in, so callers that don't need it don't pay for it. See [Versions](#versions) for the corresponding `Stack`/`StackClient` API (`ifVersion`). -`POST /records` accepts a full record body, including an optional client-supplied `id` — see [Record IDs](#record-ids) for the validation and duplicate-conflict rules the server applies. +`POST /records` accepts a full record body, including an optional client-supplied `id` — see [Record IDs](#record-ids) for the validation and duplicate-conflict rules the server applies. For `typeId: "_attachment@1"`, a non-owner requester gets `403` regardless of grants (#106) — see [Attachments](#attachments) for the refusal, its carve-out, and `POST /attachments` as the non-owner-safe combined path. `POST /records/:id/migrate` is the only way a record's `typeId` changes after creation. Body: `{ "toTypeId": "...", "content": {...} }` — the full post-migration content, computed client-side by the type's owning app (migration functions are app code, not server code) and validated by the server against `toTypeId`'s schema before writing. This is what `stack.update()` uses to commit a pending lazy migration alongside a content patch (a content-only `PATCH` can't carry a `typeId` change), and what `stack.migrateAll()` uses for each record in a batch pass. @@ -1078,29 +1093,37 @@ POST /types — register a type, or evolve an existing one in place ### Attachments ``` -POST /attachments — store raw file bytes, returns { fileId } +POST /attachments — store raw file bytes and create the _attachment@1 record, returns the record GET /attachments/:fileId — download a file DELETE /attachments/:fileId — delete a file ``` -Attachments are uploaded first to get a `fileId`, then referenced in an Association when creating or updating a Record. This keeps all Record endpoints JSON-only. +Attachments are uploaded first, then referenced in an Association when creating or updating a Record. This keeps all other Record endpoints JSON-only — `POST /attachments` is the one endpoint that also accepts a binary body. File IDs are SHA-256 hashes of the content. Uploading identical bytes twice returns the same `fileId` without writing a second copy. -**Upload:** Send the raw binary as the request body. `Content-Type` and `Content-Disposition` headers are ignored — `POST /attachments` stores bytes only and does **not** create an `_attachment@1` record. To record metadata (MIME type, filename, size), create an `_attachment@1` record via `POST /records` after upload. +**Upload (#106):** Send the raw binary as the request body, with `Content-Type` set to the MIME type and, optionally, `Content-Disposition` carrying a filename. The server stores the bytes and creates the `_attachment@1` record in the same request — implemented as `scopedStack.putAttachment(bytes, mimeType, filename)`, the same combined primitive described in [Attachments](#attachments) above. `Content-Type` defaults to `application/octet-stream` if omitted. ``` POST /attachments Authorization: Bearer +Content-Type: image/png +Content-Disposition: attachment; filename*=UTF-8''photo.png ``` -`POST /attachments` requires the same authorization as creating an `_attachment@1` record: `401` for anonymous/missing tokens, `403` if the requester lacks a `create` grant on `_attachment@1`. A bytes upload is only meaningful as a precursor to metadata creation, so the two share one authorization requirement. +Response: the created `_attachment@1` record (`200`), not just `{ fileId }` — same shape as `POST /records`. + +This is not an efficiency shortcut — it's the security boundary itself (#106). The record's `fileId` must be established from bytes the server actually received in _this_ request, with no seam where a caller-supplied string could stand in for them. That's why generic `POST /records` for `_attachment@1` is no longer available to non-owners at all (see [Attachments](#attachments) above): a separate two-step "upload bytes, then claim a fileId in a record" is indistinguishable, server-side, from an attacker who never uploaded anything and only guessed the fileId. + +`POST /attachments` requires the same authorization as creating an `_attachment@1` record: `401` for anonymous/missing tokens, `403` if the requester lacks a `create` grant on `_attachment@1`. Returns `413 Request Entity Too Large` if the payload exceeds the server's configured limit (default 50 MB, controlled by `MAX_ATTACHMENT_BYTES`). The same limit is exposed ahead of time as `maxAttachmentBytes` in [discovery](#discovery), so clients can pre-check and surface it in UI rather than learning the ceiling only by uploading the whole payload and getting a 413 back. -The SDK's `Stack.putAttachment()` and `ScopedStack.putAttachment()` perform both steps automatically. Direct HTTP callers must create the `_attachment@1` record separately if metadata is needed. +**SDK usage.** `Stack.putAttachment()`, when backed by `@haverstack/adapter-api`'s `APIAdapter`, calls this endpoint directly — one request, carrying the real `mimeType`/`filename` — rather than a bytes call followed by a separate `POST /records`. `StackAdapter.putAttachmentWithMetadata()` is the optional atomic-upload capability this rides on ([Interface split](#interface-split)): the API adapter implements it as this single request; local storage adapters don't implement it — bytes and records are different backends there, with no shared transaction — so `Stack.putAttachment()` falls back to its own `create()` call, exactly as before this endpoint existed. + +One consequence: there is no longer a bytes-only upload anywhere on the wire — this endpoint always creates a record. Accordingly, **bytes-only upload has no public SDK surface either**: `putAttachment(data, mimeType, filename?)` is the upload operation, everywhere, for everyone. `StackBlobAdapter.putAttachment()` remains the required adapter-level primitive local storage needs (it's what `Stack.putAttachment()`'s fallback writes bytes through), but on `APIAdapter` it is **unsupported and throws** rather than mapping to this endpoint — implementing it anyway would silently create a record with a default `mimeType`, a bytes-only upload that isn't. `Stack.putAttachment()` never reaches it there (the atomic capability takes precedence), so the throw guards direct adapter-level callers only. **Download:** Two optional query parameters control the response metadata and, when both are supplied, allow the server to skip the `_attachment@1` database lookup entirely: diff --git a/packages/adapter-api/src/index.ts b/packages/adapter-api/src/index.ts index 08366a2..8d32030 100644 --- a/packages/adapter-api/src/index.ts +++ b/packages/adapter-api/src/index.ts @@ -326,12 +326,13 @@ export class APIAdapter implements StackAdapter { return new Uint8Array(await res.arrayBuffer()); } + /** POST /attachments always returns the created _attachment@1 record (#106) — see putAttachmentWithMetadata() below. */ private async uploadBinary( path: string, data: Uint8Array, mimeType: string, filename?: string, - ): Promise> { + ): Promise { const url = `${this.baseUrl}${path}`; const headers: Record = { 'Content-Type': mimeType }; if (this.token) headers['Authorization'] = `Bearer ${this.token}`; @@ -348,7 +349,7 @@ export class APIAdapter implements StackAdapter { if (res.status === 401) throw new APIAdapterAuthError(); if (!res.ok) throw await this.errorForResponse(res, 'POST', path); - return res.json() as Promise>; + return res.json() as Promise; } // ------------------------------------------------------- @@ -567,9 +568,43 @@ export class APIAdapter implements StackAdapter { // Attachments // ------------------------------------------------------- - async putAttachment(data: Uint8Array): Promise { - const result = await this.uploadBinary('/attachments', data, 'application/octet-stream'); - return result.fileId as string; + /** + * Unsupported over the wire — always throws. POST /attachments is the + * only upload endpoint, and it always creates the accompanying + * _attachment@1 record (#106): the whole point of the endpoint is that + * the record's fileId comes from bytes the server received in the same + * request, so there is no bytes-only wire mode for this method to map + * to. Implementing it anyway would silently mint a record with a default + * mimeType and no filename — a bytes-only upload that isn't. Bytes-only + * storage is a local-adapter primitive with no public Stack surface + * (spec §Attachments); Stack.putAttachment() never reaches this method + * on this adapter (it takes the putAttachmentWithMetadata() path), so + * this throw is a guard against direct adapter-level callers, not a + * reachable Stack code path. + */ + async putAttachment(_data: Uint8Array): Promise { + throw new APIAdapterError( + 'Bytes-only upload is not supported over the wire: POST /attachments always creates ' + + 'an _attachment@1 record (#106). Use Stack.putAttachment(data, mimeType, filename?).', + ); + } + + /** + * StackAdapter's optional atomic-upload capability: store bytes and + * create the _attachment@1 record in one POST /attachments request — the + * wire counterpart of Stack.putAttachment() (#106). This adapter is the + * one implementation that can offer it, because bytes and records live + * behind the same boundary here (the server). Not an efficiency + * shortcut: the record's fileId is established from bytes the server + * received in *this* request, which is what makes the operation safe for + * a non-owner requester — see StackAdapter.putAttachmentWithMetadata. + */ + async putAttachmentWithMetadata( + data: Uint8Array, + mimeType: string, + filename?: string, + ): Promise { + return parseRecord(await this.uploadBinary('/attachments', data, mimeType, filename)); } async getAttachment(fileId: FileId): Promise { diff --git a/packages/adapter-api/tests/api.test.ts b/packages/adapter-api/tests/api.test.ts index 3f8634d..0072b53 100644 --- a/packages/adapter-api/tests/api.test.ts +++ b/packages/adapter-api/tests/api.test.ts @@ -776,19 +776,65 @@ describe('listTypes', () => { // Attachments // ------------------------------------------------------- +// POST /attachments always creates the _attachment@1 record now (#106) — +// the response is a full WireRecord, not { fileId }. +const attachmentRecordResponse = (overrides: Partial> = {}) => ({ + id: 'rec-attachment-1', + typeId: '_attachment@1', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + content: { fileId: 'file-xyz', mimeType: 'application/octet-stream', size: 4 }, + version: 1, + ...overrides, +}); + describe('putAttachment', () => { - test('sends POST /attachments with binary body and Content-Type: application/octet-stream', async () => { + // Bytes-only upload has no wire mode: POST /attachments always creates + // the _attachment@1 record (#106), so implementing this method would + // silently mint a default-mimeType record while claiming "no record + // created". It must throw — without ever reaching the network — rather + // than approximately honor the contract. + test('throws APIAdapterError and never issues a request', async () => { const adapter = await openAdapter(); - mockFetch.mockResolvedValueOnce(jsonResponse({ fileId: 'file-xyz' })); + mockFetch.mockClear(); + const data = new Uint8Array([0x89, 0x50, 0x4e, 0x47]); + await expect(adapter.putAttachment(data)).rejects.toThrow(APIAdapterError); + await expect(adapter.putAttachment(data)).rejects.toThrow(/not supported over the wire/); + expect(mockFetch).not.toHaveBeenCalled(); + }); +}); + +describe('putAttachmentWithMetadata', () => { + test('sends POST /attachments with the given Content-Type and Content-Disposition, returns the parsed record', async () => { + const adapter = await openAdapter(); + mockFetch.mockResolvedValueOnce( + jsonResponse( + attachmentRecordResponse({ + content: { fileId: 'file-xyz', mimeType: 'image/png', size: 4, filename: 'photo.png' }, + }), + ), + ); const data = new Uint8Array([0x89, 0x50, 0x4e, 0x47]); - const fileId = await adapter.putAttachment(data); - expect(fileId).toBe('file-xyz'); + const record = await adapter.putAttachmentWithMetadata(data, 'image/png', 'photo.png'); + + expect(record.id).toBe('rec-attachment-1'); + expect(record.content).toMatchObject({ fileId: 'file-xyz', mimeType: 'image/png' }); + const [url, init] = mockFetch.mock.lastCall as [string, RequestInit]; expect(url).toBe(`${BASE_URL}/attachments`); expect(init.method).toBe('POST'); - expect((init.headers as Record)['Content-Type']).toBe( - 'application/octet-stream', - ); + const headers = init.headers as Record; + expect(headers['Content-Type']).toBe('image/png'); + expect(headers['Content-Disposition']).toBe("attachment; filename*=UTF-8''photo.png"); + }); + + test('omits Content-Disposition when no filename is given', async () => { + const adapter = await openAdapter(); + mockFetch.mockResolvedValueOnce(jsonResponse(attachmentRecordResponse())); + await adapter.putAttachmentWithMetadata(new Uint8Array([1]), 'application/octet-stream'); + + const [, init] = mockFetch.mock.lastCall as [string, RequestInit]; + expect((init.headers as Record)['Content-Disposition']).toBeUndefined(); }); }); diff --git a/packages/adapter-api/tests/conformance.test.ts b/packages/adapter-api/tests/conformance.test.ts index a67d86e..506b4bc 100644 --- a/packages/adapter-api/tests/conformance.test.ts +++ b/packages/adapter-api/tests/conformance.test.ts @@ -19,6 +19,7 @@ import { restoreVersionFixtures, commitMigrationFixtures, errorResponseFixtures, + attachmentUploadFixtures, } from '@haverstack/conformance-fixtures'; import type { Association } from '@haverstack/core'; import { @@ -284,3 +285,50 @@ describe('error response fixtures', () => { }); } }); + +// ------------------------------------------------------- +// Attachment upload fixtures (#106) — POST /attachments carries Content-Type +// and creates the record. Only fixtures with a Content-Type header are +// dispatched here: putAttachmentWithMetadata()'s mimeType is required, so +// there is no way to drive the "header omitted" fixture through it — that +// one documents the raw wire contract for non-SDK callers only, same as +// attachmentDownloadFixtures aren't all exercised via APIAdapter either. +// ------------------------------------------------------- + +describe('attachment upload fixtures', () => { + for (const fixture of attachmentUploadFixtures) { + const contentType = fixture.requestHeaders['Content-Type']; + if (!contentType) continue; + + test(fixture.name, async () => { + const adapter = await openAdapter(); + mockFetch.mockResolvedValueOnce(jsonResponse(fixture.responseBody, fixture.responseStatus)); + + const disposition = fixture.requestHeaders['Content-Disposition']; + const filenameMatch = disposition?.match(/filename\*=UTF-8''(.+)$/); + const filename = filenameMatch ? decodeURIComponent(filenameMatch[1]) : undefined; + const data = new Uint8Array(fixture.requestBodyBytes); + + const dispatch = () => adapter.putAttachmentWithMetadata(data, contentType, filename); + + if (fixture.responseStatus >= 400) { + const code = ( + fixture.responseBody as { error: { code: keyof typeof ERROR_CLASS_FOR_CODE } } + ).error.code; + await expect(dispatch()).rejects.toThrow(ERROR_CLASS_FOR_CODE[code]); + } else { + const record = await dispatch(); + const expectedFileId = (fixture.responseBody as unknown as { content: { fileId: string } }) + .content.fileId; + expect(record.content.fileId).toBe(expectedFileId); + } + + const [url, init] = mockFetch.mock.lastCall as [string, RequestInit]; + expect(url).toBe(`${BASE_URL}/attachments`); + expect(init.method).toBe('POST'); + const headers = init.headers as Record; + expect(headers['Content-Type']).toBe(contentType); + if (disposition) expect(headers['Content-Disposition']).toBe(disposition); + }); + } +}); diff --git a/packages/conformance-fixtures/src/index.ts b/packages/conformance-fixtures/src/index.ts index 3255231..6dc743b 100644 --- a/packages/conformance-fixtures/src/index.ts +++ b/packages/conformance-fixtures/src/index.ts @@ -77,8 +77,12 @@ export const createRecordFixtures: ConformanceFixture[] '(#65) mimeType is a property of the fileId, established by the first _attachment@1 ' + 'record ever created for it. A second upload of the same bytes that declares a matching ' + 'mimeType succeeds and gets its own record — its own id, entityId, and filename — rather ' + - 'than being deduplicated away. Assumes an _attachment@1 record already exists for ' + - '"fileId": "abc123..." with "mimeType": "image/png".', + 'than being deduplicated away. Assumes the requester is the owner (#106 restricts generic ' + + 'POST /records for _attachment@1 to owner-only — see ' + + 'error-permission-denied-attachment-non-owner-create and ' + + 'create-attachment-record-non-owner-carve-out-succeeds for the non-owner cases) and that ' + + 'an _attachment@1 record already exists for "fileId": "abc123..." with ' + + '"mimeType": "image/png".', method: 'POST', path: '/records', requestBody: { @@ -99,6 +103,36 @@ export const createRecordFixtures: ConformanceFixture[] version: 1, }, }, + { + name: 'create-attachment-record-non-owner-carve-out-succeeds', + description: + '(#106 residual decision 1) A non-owner who can already read some record referencing ' + + 'fileId "abc123..." may create an additional _attachment@1 record for it — e.g. their own ' + + 'filename — without re-uploading bytes, since this conveys no access they did not already ' + + 'have. Assumes a record readable by this requester already carries an attachment ' + + 'association or file-ref field for "fileId": "abc123...". The carve-out is satisfied only ' + + "by that readable reference, never by the requester's own prior _attachment@1 record for " + + 'the same fileId — see create-attachment-record-non-owner-without-carve-out-refused.', + method: 'POST', + path: '/records', + requestBody: { + id: 'rec-attachment-carveout', + typeId: '_attachment@1', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + content: { fileId: 'abc123', mimeType: 'image/png', size: 12345, filename: 'mine.png' }, + version: 1, + }, + responseStatus: 200, + responseBody: { + id: 'rec-attachment-carveout', + typeId: '_attachment@1', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + content: { fileId: 'abc123', mimeType: 'image/png', size: 12345, filename: 'mine.png' }, + version: 1, + }, + }, ]; // ------------------------------------------------------- @@ -317,6 +351,55 @@ export const errorResponseFixtures: ConformanceFixture[] = [ responseStatus: 403, responseBody: { error: { code: 'permission', message: 'Permission denied' } }, }, + { + name: 'error-permission-denied-attachment-non-owner-create', + description: + '(#106) POST /records creating an _attachment@1 record is refused for any non-owner ' + + 'requester with 403 / code "permission" — even one holding an otherwise-sufficient ' + + '"create" grant on the type, and even for a fileId nobody has ever uploaded or referenced. ' + + 'This is not the ordinary missing-grant case (see error-permission-denied): _attachment@1 ' + + 'is access-conveying, and generic create() accepts a caller-supplied fileId with no proof ' + + 'it was ever derived from real bytes, unlike POST /attachments (see Attachments), which ' + + 'computes fileId from bytes it just hashed. Non-owners must use POST /attachments instead. ' + + 'Owner requests for the same body succeed (see create-attachment-record-matching-mimetype-' + + 'succeeds, which assumes an owner requester).', + method: 'POST', + path: '/records', + requestBody: { + id: 'rec-attachment-guessed', + typeId: '_attachment@1', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + content: { fileId: 'guessed-sha256-hash', mimeType: 'image/png', size: 12345 }, + version: 1, + }, + responseStatus: 403, + responseBody: { error: { code: 'permission', message: 'Permission denied' } }, + }, + { + name: 'create-attachment-record-non-owner-without-carve-out-refused', + description: + '(#106 residual decision 1) The carve-out (see ' + + 'create-attachment-record-non-owner-carve-out-succeeds) is satisfied only by a readable ' + + "record referencing the fileId — never by the requester's own prior _attachment@1 record " + + 'for the same fileId (the "uploaded it themselves" clause of the getAttachment() access ' + + 'rule). Allowing that would let one successful guess bootstrap unlimited further metadata ' + + 'records for the same fileId, reintroducing the circularity #106 closes. Assumes the ' + + 'requester already holds an _attachment@1 record for "fileId": "file-mine" (e.g. from a ' + + 'prior putAttachment() upload) but no readable record references it.', + method: 'POST', + path: '/records', + requestBody: { + id: 'rec-attachment-second-name', + typeId: '_attachment@1', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + content: { fileId: 'file-mine', mimeType: 'image/png', size: 1, filename: 'second-name.png' }, + version: 1, + }, + responseStatus: 403, + responseBody: { error: { code: 'permission', message: 'Permission denied' } }, + }, { name: 'error-not-found', description: @@ -392,12 +475,16 @@ export const errorResponseFixtures: ConformanceFixture[] = [ { name: 'error-validation-attachment-mimetype-conflict-on-create', description: - '(#65) POST /records creating an _attachment@1 record whose mimeType conflicts with the ' + - 'mimeType already established (by the first-ever record) for the same fileId returns 422 ' + - 'with code "validation" — reconstructed as StackValidationError. A matching mimeType ' + - 'would instead succeed (see create-attachment-record-matching-mimetype-succeeds). Assumes ' + - 'an _attachment@1 record already exists for "fileId": "abc123..." with ' + - '"mimeType": "image/png".', + '(#65, message genericized by #106) POST /records creating an _attachment@1 record whose ' + + 'mimeType conflicts with the mimeType already established (by the first-ever record) for ' + + 'the same fileId returns 422 with code "validation" — reconstructed as ' + + 'StackValidationError. A matching mimeType would instead succeed (see ' + + 'create-attachment-record-matching-mimetype-succeeds). The message never names the ' + + "established mimeType (anti-oracle, #106): stating it would confirm an existing fileId's " + + 'content type to a caller who only guessed the fileId. Assumes the requester is the owner ' + + '(non-owner POST /records for _attachment@1 is refused outright — see ' + + 'error-permission-denied-attachment-non-owner-create) and that an _attachment@1 record ' + + 'already exists for "fileId": "abc123..." with "mimeType": "image/png".', method: 'POST', path: '/records', requestBody: { @@ -416,9 +503,7 @@ export const errorResponseFixtures: ConformanceFixture[] = [ details: [ { path: 'mimeType', - message: - 'mimeType "text/html" conflicts with the mimeType "image/png" already ' + - 'established for fileId "abc123" by an earlier upload', + message: 'mimeType conflicts with the mimeType already established for this fileId', }, ], }, @@ -573,14 +658,116 @@ export const attachmentDownloadFixtures: AttachmentDownloadFixture[] = [ }, ]; +// ------------------------------------------------------- +// Attachment upload: POST /attachments creates the record (#106) +// ------------------------------------------------------- +// +// Like attachmentDownloadFixtures, POST /attachments doesn't fit +// ConformanceFixture: the request body is raw bytes, not JSON. What's +// pinned here is the request Content-Type/Content-Disposition headers going +// in and the created _attachment@1 record coming out — the wire shape of +// the combined, non-owner-safe upload primitive described in the spec's +// Attachments section. This is not an efficiency shortcut: fileId must be +// established from bytes the server actually received in this request, so +// there is no seam where a caller-supplied string could substitute for +// them (see error-permission-denied-attachment-non-owner-create, which +// pins the generic-create path this closes). + +export type AttachmentUploadFixture = { + /** Unique, stable name — usable as a test-case id. */ + name: string; + /** What this fixture pins down, and why. */ + description: string; + /** Request headers this POST must send. Authorization is omitted — every fixture here assumes a valid bearer token for the described requester. */ + requestHeaders: Record; + /** Raw request body bytes, as an array of byte values (0-255), so the fixture stays plain data with no binary encoding. */ + requestBodyBytes: number[]; + /** Expected HTTP status code. */ + responseStatus: number; + /** Expected JSON response body: the created _attachment@1 record, or a WireError. */ + responseBody: WireRecord | WireError; +}; + +// SHA-256 of the byte sequence below (the ASCII string "hello"). +const HELLO_FILE_ID = '2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824'; +const HELLO_BYTES = [104, 101, 108, 108, 111]; + +export const attachmentUploadFixtures: AttachmentUploadFixture[] = [ + { + name: 'attachment-upload-creates-metadata-record', + description: + '(#106) POST /attachments carries Content-Type and Content-Disposition (filename) and ' + + 'stores the bytes and creates the _attachment@1 record in the same request — the wire ' + + 'counterpart of ScopedStack.putAttachment()/Stack.putAttachment(). The response is the ' + + 'created record (same shape as POST /records), not just { fileId }. fileId is the SHA-256 ' + + 'hex hash of the request body.', + requestHeaders: { + 'Content-Type': 'text/plain', + 'Content-Disposition': "attachment; filename*=UTF-8''hello.txt", + }, + requestBodyBytes: HELLO_BYTES, + responseStatus: 200, + responseBody: { + id: 'rec-attachment-upload-1', + typeId: '_attachment@1', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + content: { + fileId: HELLO_FILE_ID, + mimeType: 'text/plain', + size: HELLO_BYTES.length, + filename: 'hello.txt', + }, + version: 1, + }, + }, + { + name: 'attachment-upload-no-content-type-defaults-to-octet-stream', + description: + '(#106) Content-Type is optional on upload — when omitted, the server defaults the ' + + "created record's mimeType to application/octet-stream rather than rejecting the request, " + + 'matching the download-side default (see attachment-download-no-metadata-defaults-to-' + + 'octet-stream).', + requestHeaders: {}, + requestBodyBytes: HELLO_BYTES, + responseStatus: 200, + responseBody: { + id: 'rec-attachment-upload-2', + typeId: '_attachment@1', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + content: { + fileId: HELLO_FILE_ID, + mimeType: 'application/octet-stream', + size: HELLO_BYTES.length, + }, + version: 1, + }, + }, + { + name: 'attachment-upload-non-owner-without-create-grant-forbidden', + description: + 'POST /attachments requires the same authorization as creating an _attachment@1 record: ' + + '403 / code "permission" if the requester lacks a create grant on _attachment@1. Unlike ' + + 'generic POST /records (see error-permission-denied-attachment-non-owner-create), a ' + + 'non-owner *with* a create grant succeeds here — that grant is exactly what makes this the ' + + 'sanctioned non-owner path.', + requestHeaders: { 'Content-Type': 'text/plain' }, + requestBodyBytes: HELLO_BYTES, + responseStatus: 403, + responseBody: { error: { code: 'permission', message: 'Permission denied' } }, + }, +]; + // ------------------------------------------------------- // All fixtures // ------------------------------------------------------- /** * Every fixture across every endpoint, for consumers that want to iterate - * uniformly. Excludes attachmentDownloadFixtures — a different shape - * (response headers, not a JSON body), imported separately. + * uniformly. Excludes attachmentDownloadFixtures and attachmentUploadFixtures + * — both a different shape (binary body and/or header-focused, not a plain + * JSON request/response pair), imported separately. */ export const allConformanceFixtures: ConformanceFixture[] = [ ...createRecordFixtures, diff --git a/packages/core/src/combine.ts b/packages/core/src/combine.ts index 325b1a4..fca768a 100644 --- a/packages/core/src/combine.ts +++ b/packages/core/src/combine.ts @@ -51,6 +51,12 @@ export function combineAdapters(parts: { parts.record.deleteUnreferencedAttachmentRecords!(fileId, metadataTypeId), }), + // StackAdapter.putAttachmentWithMetadata is deliberately never + // synthesized here: it promises bytes + record as one atomic operation, + // and a record backend glued to a blob backend has no shared + // transaction to honor that with (#106). Stack.putAttachment() falls + // back to its own bytes-then-create() sequence when the method is + // absent. putAttachment: (data) => parts.blob.putAttachment(data), getAttachment: (id) => parts.blob.getAttachment(id), deleteAttachment: (id) => parts.blob.deleteAttachment(id), diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index 48e30b1..54c283f 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -330,7 +330,6 @@ export interface StackClient { getVersion(id: string, version: number): Promise; restoreVersion(id: string, version: number, opts?: IfVersionOptions): Promise; getAttachment(fileId: string): Promise; - putAttachmentBytes(data: Uint8Array): Promise; putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise; deleteAttachment(fileId: string): Promise; collectAttachmentGarbage( @@ -485,7 +484,7 @@ export class Stack implements StackClient { * multi-tenant API server). */ asEntity(entityId: EntityId | null): ScopedStack { - return new ScopedStack(this, entityId, this.idTimestampSkewMsValue); + return new ScopedStack(this, entityId, this.idTimestampSkewMsValue, this.adapter); } // ------------------------------------------------------- @@ -1170,12 +1169,15 @@ export class Stack implements StackClient { const first = existing.reduce((a, b) => (a.createdAt <= b.createdAt ? a : b)); const establishedMimeType = (first.content as AttachmentContent).mimeType; if (mimeType !== establishedMimeType) { + // Anti-oracle (#106): the established mimeType is deliberately not + // interpolated into the message. Naming it would confirm the fileId's + // existing content type to a caller who only guessed the fileId, + // reintroducing the exact confirmation-oracle #51's anti-oracle rule + // exists to prevent. throw new StackValidationError([ { path: 'mimeType', - message: - `mimeType "${mimeType}" conflicts with the mimeType "${establishedMimeType}" ` + - `already established for fileId "${fileId}" by an earlier upload`, + message: 'mimeType conflicts with the mimeType already established for this fileId', }, ]); } @@ -1237,22 +1239,31 @@ export class Stack implements StackClient { } } - /** - * Store raw bytes and return the content-addressed file ID. - * Does not create an _attachment@1 record — use putAttachment() or - * ScopedStack.putAttachment() for the full upload flow. - */ - async putAttachmentBytes(data: Uint8Array): Promise { - return this.adapter.putAttachment(data); - } - /** * Store bytes and create an _attachment@1 metadata record (owner-attributed, * no entityId). Use ScopedStack.putAttachment() when the uploader is a * specific entity rather than the stack owner. + * + * When the adapter implements the optional putAttachmentWithMetadata() + * capability (the API adapter, via one POST /attachments request the + * server fulfills atomically — #106), the whole operation is delegated to + * it and the separate create() call below is skipped — not for + * efficiency, but because a second, independent record-creation call + * would be indistinguishable, server-side, from a non-owner who never + * uploaded anything and only guessed the fileId. On that path the + * returned record is trusted as backend-authoritative: client-side schema + * validation and the mimeType-conflict check don't run here — the server + * runs both (a client-side conflict check against remote state would be + * both racy and itself a mini-oracle). Adapters without the capability + * (all local storage) fall back to the create() call that was always + * here. */ async putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise { - const fileId = await this.putAttachmentBytes(data); + if (this.adapter.putAttachmentWithMetadata) { + const record = await this.adapter.putAttachmentWithMetadata(data, mimeType, filename); + return (record.content as AttachmentContent).fileId; + } + const fileId = await this.adapter.putAttachment(data); await this.create(`${SYSTEM_TYPES.ATTACHMENT}@1`, { fileId, mimeType, @@ -1677,6 +1688,14 @@ export class ScopedStack implements StackClient { private readonly stack: Stack, private readonly requesterEntityId: EntityId | null, private readonly idTimestampSkewMs: number | null, + // The bytes-storage primitive for putAttachment()'s upload step. Held + // directly (passed by Stack.asEntity()) because Stack's adapter is + // private and the bytes-only upload is no longer part of Stack's + // public API (#106 follow-up): the record ScopedStack creates carries + // the requester's entityId, which the adapter-level atomic capability + // has no parameter for, so this class always composes bytes + its own + // create() rather than delegating to putAttachmentWithMetadata(). + private readonly adapter: StackAdapter, ) {} get features(): StackFeatures { @@ -1818,6 +1837,20 @@ export class ScopedStack implements StackClient { return this.canRead(record); } + /** + * Whether the requester can already read some record referencing `fileId` + * — the "possession via a readable referencing record" clause shared by + * canAccessFile() and the non-owner _attachment@1 create() carve-out + * (#106, residual decision 1). Deliberately excludes the uploader clause: + * using "I hold a metadata record for F" to justify creating *another* + * metadata record for F would let one successful guess bootstrap + * unlimited further ones — the exact circularity #106 closes. + */ + private async hasReadableReference(fileId: string): Promise { + const refResult = await this.query({ filter: { attachmentFileId: fileId }, limit: 1 }); + return refResult.records.length > 0; + } + /** * Whether the requester may create a reference (`attachment` association * or file-ref content field) to `fileId` — the dual of getAttachment()'s @@ -1829,8 +1862,7 @@ export class ScopedStack implements StackClient { private async canAccessFile(fileId: string): Promise { if (this.requesterEntityId === this.stack.ownerEntityId) return true; - const refResult = await this.query({ filter: { attachmentFileId: fileId }, limit: 1 }); - if (refResult.records.length > 0) return true; + if (await this.hasReadableReference(fileId)) return true; if (!this.requesterEntityId) return false; @@ -1928,6 +1960,21 @@ export class ScopedStack implements StackClient { * left unchecked here: `permissions` is create-time-consistent with * setPermissions() (owner/creator territory already), and `appId` is * self-reported, untrusted metadata everywhere, not a permission input. + * + * `_attachment@1` (matched by baseId, like `_group`) is refused outright + * for non-owners, with one carve-out (#106): a readable record already + * referencing `content.fileId` may get a second metadata record (e.g. a + * second filename) without re-uploading. Otherwise, a bare `create` grant + * — held by every uploader — would let a requester name an arbitrary + * guessed fileId and, via canAccessFile()'s uploader clause, turn that + * guess into a read: creating an access-conveying record without ever + * proving possession of the bytes. `putAttachment()` is the only + * non-owner path left: it derives fileId from bytes it just hashed, so + * possession is proven by construction rather than asserted by the + * caller. The carve-out deliberately excludes the uploader clause of + * canAccessFile() — using an existing metadata record to justify creating + * another would let one successful guess bootstrap unlimited further + * ones, the same circularity this guard exists to close. */ async create = Record>( typeId: TypeId, @@ -1939,6 +1986,13 @@ export class ScopedStack implements StackClient { if (!(await this.checkCreateGrant(typeId))) { throw new StackPermissionError(`No create grant for type "${typeId}"`); } + const isOwner = requester === this.stack.ownerEntityId; + if (!isOwner && baseIdOf(typeId) === SYSTEM_TYPES.ATTACHMENT) { + const fileId = (content as Record).fileId; + if (typeof fileId !== 'string' || !(await this.hasReadableReference(fileId))) { + throw new StackPermissionError(); + } + } if (opts.id !== undefined) { validateRecordId(opts.id); validateIdTimestampSkew(opts.id, this.idTimestampSkewMs); @@ -1954,7 +2008,6 @@ export class ScopedStack implements StackClient { baseIdOf(typeId) === SYSTEM_TYPES.GROUP ? { ...opts, associations: stampGroupAdmin(opts.associations, requester) } : opts; - const isOwner = requester === this.stack.ownerEntityId; return this.stack.create(typeId, content, { ...createOpts, entityId: isOwner ? undefined : requester, @@ -2107,13 +2160,15 @@ export class ScopedStack implements StackClient { } /** - * Store raw bytes only — no _attachment@1 record. Gated identically to - * putAttachment(): a bytes upload is only meaningful as a precursor to - * metadata creation, so the two share one authorization check. - * Requires a `create` grant on `_attachment@1`. Anonymous requesters are - * always denied. + * Store bytes and create an _attachment@1 metadata record. Requires a + * `create` grant on `_attachment@1`. Anonymous requesters are always + * denied. The record's entityId is set to the requester — unless the + * requester is the owner, in which case entityId is omitted, matching the + * normalization create() applies (#69, #106 E1): without it, the owner + * uploading through asEntity(ownerEntityId) would produce a differently- + * shaped record than Stack.putAttachment() for the exact same author. */ - async putAttachmentBytes(data: Uint8Array): Promise { + async putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise { const requester = this.requesterEntityId; if (!requester) { throw new StackPermissionError('Anonymous requesters cannot upload attachments'); @@ -2121,18 +2176,8 @@ export class ScopedStack implements StackClient { if (!(await this.checkCreateGrant(`${SYSTEM_TYPES.ATTACHMENT}@1`))) { throw new StackPermissionError(`No create grant for type "${SYSTEM_TYPES.ATTACHMENT}@1"`); } - return this.stack.putAttachmentBytes(data); - } - - /** - * Store bytes and create an _attachment@1 metadata record owned by the - * requester. Requires a `create` grant on `_attachment@1`. - * Anonymous requesters are always denied. - */ - async putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise { - const fileId = await this.putAttachmentBytes(data); - // putAttachmentBytes() above throws if requesterEntityId is null. - const requester = this.requesterEntityId as string; + const fileId = await this.adapter.putAttachment(data); + const isOwner = requester === this.stack.ownerEntityId; await this.stack.create( `${SYSTEM_TYPES.ATTACHMENT}@1`, { @@ -2141,7 +2186,7 @@ export class ScopedStack implements StackClient { size: data.byteLength, ...(filename && { filename }), } satisfies AttachmentContent, - { entityId: requester }, + { entityId: isOwner ? undefined : requester }, ); return fileId; } diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index dc7b81d..1185b92 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -539,7 +539,37 @@ export interface StackBlobAdapter { * Pass this to Stack.create(). Build one with combineAdapters() when you * want different backends for records and blobs (e.g. SQLite + S3). */ -export type StackAdapter = StackRecordAdapter & StackBlobAdapter; +export type StackAdapter = StackRecordAdapter & + StackBlobAdapter & { + /** + * Store bytes and create the accompanying _attachment@1 record as one + * atomic operation. Optional — capability-flagged, like + * StackRecordAdapter.deleteUnreferencedAttachmentRecords(): implement it + * only when bytes and records genuinely live behind a single boundary + * that can do both in one operation. Today that is APIAdapter alone, via + * one POST /attachments request the server fulfills atomically (#106). + * It lives here on the composed type, not on either half, because + * neither half can ever have it: a blob adapter has no record store and + * a record adapter has no byte store. Accordingly, combineAdapters() + * never synthesizes it from parts — a record backend and a blob backend + * glued together have no shared transaction. + * + * This is not an efficiency shortcut: the record's fileId must be + * established from bytes the backend actually received in *this* + * operation, or a separate record-creation call is indistinguishable, + * server-side, from a caller who never uploaded anything and only + * guessed the fileId (#106). Stack.putAttachment() uses this when + * present — trusting the returned record as backend-authoritative, + * without re-running client-side validation — and otherwise falls back + * to its own bytes-then-create() sequence, unchanged from before this + * method existed. + */ + putAttachmentWithMetadata?( + data: Uint8Array, + mimeType: string, + filename?: string, + ): Promise; + }; export type TokenInfo = { id: string; diff --git a/packages/core/tests/combine.test.ts b/packages/core/tests/combine.test.ts index aed09f9..3c8fc73 100644 --- a/packages/core/tests/combine.test.ts +++ b/packages/core/tests/combine.test.ts @@ -112,6 +112,20 @@ describe('combineAdapters', () => { expect(fileId).toBe('computed-id'); }); + // putAttachmentWithMetadata (#106) promises bytes + record as one atomic + // operation — something a record backend glued to a blob backend can + // never honor, so combineAdapters() must not synthesize it. Its absence + // is what routes Stack.putAttachment() to the bytes-then-create() + // fallback. + test('never synthesizes putAttachmentWithMetadata from parts', () => { + const adapter = combineAdapters({ + record: makeRecordAdapter(), + blob: makeBlobAdapter(), + }); + expect('putAttachmentWithMetadata' in adapter).toBe(false); + expect(adapter.putAttachmentWithMetadata).toBeUndefined(); + }); + // Optional capabilities must round-trip exactly: present when the // underlying part implements them, absent otherwise. A wrapper that // always defines the key (even forwarding to a missing method) would diff --git a/packages/core/tests/scoped-stack.test.ts b/packages/core/tests/scoped-stack.test.ts index 404fea4..553d891 100644 --- a/packages/core/tests/scoped-stack.test.ts +++ b/packages/core/tests/scoped-stack.test.ts @@ -782,57 +782,15 @@ describe('ScopedStack.putAttachment', () => { const fileId = await stack.asEntity(STRANGER).putAttachment(data, 'image/png'); expect(typeof fileId).toBe('string'); }); -}); - -// ------------------------------------------------------- -// ScopedStack.putAttachmentBytes — bytes-only upload, gated like putAttachment -// ------------------------------------------------------- - -describe('ScopedStack.putAttachmentBytes', () => { - const data = new Uint8Array([1, 2, 3]); - - test('owner can always upload without a grant', async () => { - const fileId = await stack.asEntity(OWNER).putAttachmentBytes(data); - expect(typeof fileId).toBe('string'); - }); - - test('anonymous requester cannot upload', async () => { - await expect(stack.asEntity(null).putAttachmentBytes(data)).rejects.toThrow( - StackPermissionError, - ); - }); - - test('authenticated entity without a grant cannot upload', async () => { - await expect(stack.asEntity(MEMBER).putAttachmentBytes(data)).rejects.toThrow( - StackPermissionError, - ); - }); - - test('entity with create grant on _attachment@1 can upload', async () => { - await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); - const fileId = await stack.asEntity(MEMBER).putAttachmentBytes(data); - expect(typeof fileId).toBe('string'); - }); - - test('default grant allows any authenticated entity to upload', async () => { - await stack.grant(null, [{ actions: ['create'], typeId: '_attachment@1' }]); - const fileId = await stack.asEntity(STRANGER).putAttachmentBytes(data); - expect(typeof fileId).toBe('string'); - }); - test('does not create an _attachment@1 record', async () => { - await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); - await stack.asEntity(MEMBER).putAttachmentBytes(data); + // #106 E1: owner uploads carry no entityId, matching create()'s existing + // normalization — same author, same shape, whether writing directly or + // through asEntity(ownerEntityId). + test('owner upload via asEntity(ownerEntityId) produces a record with no entityId', async () => { + await stack.asEntity(OWNER).putAttachment(data, 'image/png'); const result = await stack.query({ filter: { typeId: '_attachment@1' } }); - expect(result.records).toHaveLength(0); - }); - - test('putAttachment() still succeeds and creates its metadata record via the shared gate', async () => { - await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); - const fileId = await stack.asEntity(MEMBER).putAttachment(data, 'image/png', 'photo.png'); - const result = await stack.query({ filter: { typeId: '_attachment@1', entityId: MEMBER } }); expect(result.records).toHaveLength(1); - expect(result.records[0].content).toMatchObject({ fileId, mimeType: 'image/png' }); + expect(result.records[0].entityId).toBeUndefined(); }); }); @@ -1276,6 +1234,154 @@ describe('ScopedStack.create — attachment association gating (#51)', () => { }); }); +// ------------------------------------------------------- +// ScopedStack.create — non-owner _attachment@1 refusal (#106) +// ------------------------------------------------------- +// +// putAttachment() derives fileId from bytes it just hashed — possession is +// proven by construction. Generic create() accepts a caller-supplied +// fileId string with no such proof, so a non-owner holding nothing but a +// bare `create` grant on _attachment@1 could otherwise name an arbitrary +// guessed fileId and, via canAccessFile()'s uploader clause, turn a correct +// guess into a read. These tests pin the guard that closes this path. + +describe('ScopedStack.create — non-owner _attachment@1 refusal (#106)', () => { + beforeEach(async () => { + await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); + }); + + test('non-owner create() with a guessed fileId is refused even with a create grant', async () => { + await expect( + stack.asEntity(MEMBER).create('_attachment@1', { + fileId: 'guessed-sha256-hash', + mimeType: 'image/png', + size: 12345, + }), + ).rejects.toThrow(StackPermissionError); + }); + + test('owner is exempt from the refusal', async () => { + const record = await stack.asEntity(OWNER).create('_attachment@1', { + fileId: 'anything', + mimeType: 'image/png', + size: 1, + }); + expect(record.typeId).toBe('_attachment@1'); + }); + + test('unscoped Stack.create() is unaffected (full trust)', async () => { + const record = await stack.create('_attachment@1', { + fileId: 'anything', + mimeType: 'image/png', + size: 1, + }); + expect(record.typeId).toBe('_attachment@1'); + }); + + // The exploit this guard closes: a non-owner cannot turn a guessed fileId + // into a read by first failing to create a metadata record for it, then + // trying to download it anyway. + test('exploit regression: a refused create leaves getAttachment() denied too', async () => { + const guessedFileId = 'guessed-sha256-hash'; + await expect( + stack.asEntity(MEMBER).create('_attachment@1', { + fileId: guessedFileId, + mimeType: 'image/png', + size: 12345, + }), + ).rejects.toThrow(StackPermissionError); + + await expect(stack.asEntity(MEMBER).getAttachment(guessedFileId)).rejects.toThrow( + StackPermissionError, + ); + }); + + test('non-owner putAttachment(bytes, mime, filename) still works end-to-end', async () => { + const data = new Uint8Array([1, 2, 3]); + const fileId = await stack.asEntity(MEMBER).putAttachment(data, 'image/png', 'photo.png'); + + // The upload is now accessible to them... + const bytes = await stack.asEntity(MEMBER).getAttachment(fileId); + expect(bytes).toEqual(data); + + // ...and they can reference it from a new record. + await stack.defineType(COMMENT, 'Comment', { text: { kind: 'text', required: true } }); + await stack.grant(MEMBER, [{ actions: ['create'], typeId: COMMENT }]); + const record = await stack + .asEntity(MEMBER) + .create( + COMMENT, + { text: 'hi' }, + { associations: [{ kind: 'attachment', label: 'x', fileId, mimeType: 'image/png' }] }, + ); + expect(record.associations).toContainEqual({ + kind: 'attachment', + label: 'x', + fileId, + mimeType: 'image/png', + }); + }); + + // Residual decision 1: a non-owner who can already read a record + // referencing F may add their own _attachment@1 (e.g. a second filename) + // without re-uploading bytes — this conveys no access they didn't already + // have via the readable record. + test('carve-out: a non-owner with a readable referencing record can add a second metadata record', async () => { + const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png', 'owner.png'); + const owned = await stack.create(NOTE, { text: 'owner note' }); + await stack.associate(owned.id, { + kind: 'attachment', + label: 'cover', + fileId, + mimeType: 'image/png', + }); + await stack.grant(MEMBER, [{ actions: ['read-any'], typeId: NOTE }]); + + const record = await stack.asEntity(MEMBER).create('_attachment@1', { + fileId, + mimeType: 'image/png', + size: 1, + filename: 'members-name.png', + }); + expect(record.content).toMatchObject({ fileId, filename: 'members-name.png' }); + }); + + // The carve-out must never fall back to the uploader clause: a non-owner + // who has an *existing* _attachment@1 record for F (but no readable + // record referencing F) does not get to bootstrap a second one from it — + // that would let one successful guess unlock unlimited further records + // for the same fileId. + test('carve-out does not extend to the uploader clause', async () => { + await stack.create( + '_attachment@1', + { fileId: 'file-mine', mimeType: 'image/png', size: 1 }, + { entityId: MEMBER }, + ); + + await expect( + stack.asEntity(MEMBER).create('_attachment@1', { + fileId: 'file-mine', + mimeType: 'image/png', + size: 1, + filename: 'second-name.png', + }), + ).rejects.toThrow(StackPermissionError); + }); + + test('a non-owner without a readable referencing record is refused even for a real, existing fileId', async () => { + const fileId = await stack.putAttachment(new Uint8Array([9]), 'image/png'); + // fileId is real and exists, but MEMBER has no readable record referencing it. + await expect( + stack.asEntity(MEMBER).create('_attachment@1', { + fileId, + mimeType: 'image/png', + size: 1, + filename: 'sneaky.png', + }), + ).rejects.toThrow(StackPermissionError); + }); +}); + describe('ScopedStack.create — relationship association and parentId gating (#51)', () => { let readableNote: StackRecord; let unreadableNote: StackRecord; diff --git a/packages/core/tests/stack.test.ts b/packages/core/tests/stack.test.ts index 3c815a9..e82f549 100644 --- a/packages/core/tests/stack.test.ts +++ b/packages/core/tests/stack.test.ts @@ -10,7 +10,7 @@ import { } from '../src/stack.js'; import { generateId, crockford32Encode } from '../src/id.js'; import { MemoryAdapter } from '../src/testing.js'; -import type { BlobFileInfo } from '../src/types.js'; +import type { BlobFileInfo, StackAdapter, StackRecord } from '../src/types.js'; // ------------------------------------------------------- // Test setup @@ -1656,6 +1656,56 @@ describe('putAttachment', () => { }); }); +// ------------------------------------------------------- +// putAttachment — atomic path (#106): when the adapter implements the +// optional StackAdapter.putAttachmentWithMetadata() capability (the API +// adapter, over one POST /attachments request), Stack.putAttachment() +// delegates the whole operation to it and must not also make its own +// create() call — that would double-create (and, for a mismatched +// mimeType, conflict). Local adapters like MemoryAdapter don't implement +// the capability, so the pre-existing create() fallback is exercised by +// every other test in this describe block above. +// ------------------------------------------------------- + +describe('putAttachment — atomic adapter path (#106)', () => { + test('delegates to putAttachmentWithMetadata() when present, skipping its own create() call', async () => { + const data = new Uint8Array([1, 2, 3]); + const fabricatedRecord: StackRecord = { + id: generateId(), + typeId: '_attachment@1', + createdAt: new Date(), + updatedAt: new Date(), + content: { fileId: 'atomic-file-id', mimeType: 'image/png', size: 3, filename: 'photo.png' }, + version: 1, + }; + const atomicAdapter: StackAdapter = Object.assign( + new MemoryAdapter({ ownerEntityId: 'owner-123', timezone: 'UTC' }), + { putAttachmentWithMetadata: vi.fn().mockResolvedValue(fabricatedRecord) }, + ); + const atomicStack = await Stack.create(atomicAdapter); + const createSpy = vi.spyOn(atomicStack, 'create'); + + const fileId = await atomicStack.putAttachment(data, 'image/png', 'photo.png'); + + expect(fileId).toBe('atomic-file-id'); + expect(atomicAdapter.putAttachmentWithMetadata).toHaveBeenCalledWith( + data, + 'image/png', + 'photo.png', + ); + expect(createSpy).not.toHaveBeenCalled(); + }); + + test('falls back to its own create() call when the adapter lacks the capability', async () => { + const data = new Uint8Array([1, 2, 3]); + const createSpy = vi.spyOn(stack, 'create'); + + await stack.putAttachment(data, 'image/png', 'photo.png'); + + expect(createSpy).toHaveBeenCalledTimes(1); + }); +}); + // ------------------------------------------------------- // _attachment@1 mimeType invariant (#65): first-recorded wins for serving, // a conflicting later upload is rejected rather than silently coexisting. @@ -1683,6 +1733,26 @@ describe('_attachment@1 mimeType conflict on create', () => { expect(result.records).toHaveLength(1); }); + // Anti-oracle (#106): the established mimeType must never appear in the + // conflict message — naming it would confirm the fileId's existing + // content type to a caller who only guessed the fileId, reintroducing the + // confirmation-oracle #51's anti-oracle rule exists to prevent. + test('the conflict message never names the established mimeType', async () => { + const data = new Uint8Array([1, 2, 3]); + await stack.putAttachment(data, 'text/markdown'); + + let error: StackValidationError | undefined; + try { + await stack.putAttachment(data, 'text/plain'); + } catch (e) { + error = e as StackValidationError; + } + expect(error).toBeInstanceOf(StackValidationError); + const message = JSON.stringify(error?.errors); + expect(message).not.toContain('text/markdown'); + expect(message).not.toContain('text/plain'); + }); + test('conflict is detected even when the established record is beyond the first query page (>50 records, fallback path)', async () => { for (let i = 0; i < 55; i++) { await stack.create('_attachment@1', { @@ -1708,7 +1778,7 @@ describe('_attachment@1 mimeType conflict on create', () => { test('two different uploaders of identical bytes each get their own filename under a matching mimeType', async () => { const data = new Uint8Array([1, 2, 3]); - const fileId = await stack.putAttachmentBytes(data); + const fileId = await adapter.putAttachment(data); await stack.create( '_attachment@1', { fileId, mimeType: 'image/png', size: 3, filename: 'alice.png' }, @@ -2061,11 +2131,12 @@ describe('collectAttachmentGarbage', () => { expect(meta.records).toHaveLength(1); }); - // A putAttachmentBytes() upload that never gets a metadata record (e.g. a - // crash between storing bytes and creating _attachment@1) is only - // discoverable via StackBlobAdapter.listFiles(). + // Bytes with no metadata record (a putAttachment() that stored bytes but + // crashed before creating _attachment@1 — simulated here by writing + // through the adapter directly, since no Stack method produces this + // state on purpose) are only discoverable via StackBlobAdapter.listFiles(). test('collects a bare-bytes orphan discovered via listFiles()', async () => { - const fileId = await stack.putAttachmentBytes(new Uint8Array([9, 9, 9])); + const fileId = await adapter.putAttachment(new Uint8Array([9, 9, 9])); const result = await stack.collectAttachmentGarbage({ graceMs: 0 }); @@ -2091,10 +2162,12 @@ describe('collectAttachmentGarbage', () => { class NoListFilesAdapter extends MemoryAdapter { override listFiles: (() => Promise) | undefined = undefined; } - const noListFilesStack = await Stack.create( - new NoListFilesAdapter({ ownerEntityId: 'owner-123', timezone: 'UTC' }), - ); - await noListFilesStack.putAttachmentBytes(new Uint8Array([9, 9, 9])); + const noListFilesAdapter = new NoListFilesAdapter({ + ownerEntityId: 'owner-123', + timezone: 'UTC', + }); + const noListFilesStack = await Stack.create(noListFilesAdapter); + await noListFilesAdapter.putAttachment(new Uint8Array([9, 9, 9])); const result = await noListFilesStack.collectAttachmentGarbage({ graceMs: 0 });