From 03f6a78daa3f9d46f0d1c9227ed5ffef3bb72fea Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 02:54:01 +0000 Subject: [PATCH 1/8] fix(core): close the _attachment@1 hash-oracle in ScopedStack.create() (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ScopedStack.create() applied no special check for _attachment@1, so a non-owner holding nothing but a bare create grant on the type — every uploader, by design — could name an arbitrary guessed fileId and, via canAccessFile()'s uploader clause, turn a correct guess into a read. putAttachment() was always the safe primitive (it derives fileId from bytes it just hashed, so possession is proven by construction); this closes the other path. - ScopedStack.create() refuses _attachment@1 for non-owners, with a carve-out: a readable record already referencing the fileId may get a second metadata record (e.g. a second filename) without re-uploading — never via the uploader clause, which would reintroduce the same circularity. - ScopedStack.putAttachment() now omits entityId for owner uploads, matching the normalization create() already applies (E1). - The mimeType-conflict validation error no longer names the established mimeType, closing a secondary anti-oracle leak. - Updates docs/spec.md (§Attachments, §API Adapter Wire Format) and @haverstack/conformance-fixtures to match: POST /attachments is documented as creating the _attachment@1 record atomically (the non-owner-safe combined primitive, not an efficiency optimization), and generic POST /records for _attachment@1 is owner-only. Scope note: the atomicity portion of #106 (folding bytes+metadata into one local-adapter transaction, closing the bare-bytes orphan window and the F3 mimeType race) depends on #112 and is deliberately not included here, per the issue's own sequencing. @haverstack/adapter-api is unchanged in this PR. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01LSNFMXRWseS34w8U2rt1u5 --- docs/spec.md | 26 ++- packages/conformance-fixtures/src/index.ts | 213 +++++++++++++++++++-- packages/core/src/stack.ts | 62 +++++- packages/core/tests/scoped-stack.test.ts | 158 +++++++++++++++ packages/core/tests/stack.test.ts | 20 ++ 5 files changed, 449 insertions(+), 30 deletions(-) diff --git a/docs/spec.md b/docs/spec.md index ff9232d..e108462 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -716,6 +716,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. @@ -1000,7 +1008,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 +1086,33 @@ 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`. -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. +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`. -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. +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. **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/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/stack.ts b/packages/core/src/stack.ts index 48e30b1..4a73729 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -1170,12 +1170,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', }, ]); } @@ -1818,6 +1821,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 +1846,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 +1944,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 +1970,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 +1992,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, @@ -2125,14 +2162,19 @@ export class ScopedStack implements StackClient { } /** - * 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. + * 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 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 isOwner = requester === this.stack.ownerEntityId; await this.stack.create( `${SYSTEM_TYPES.ATTACHMENT}@1`, { @@ -2141,7 +2183,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/tests/scoped-stack.test.ts b/packages/core/tests/scoped-stack.test.ts index 404fea4..c9f8077 100644 --- a/packages/core/tests/scoped-stack.test.ts +++ b/packages/core/tests/scoped-stack.test.ts @@ -782,6 +782,16 @@ describe('ScopedStack.putAttachment', () => { const fileId = await stack.asEntity(STRANGER).putAttachment(data, 'image/png'); expect(typeof fileId).toBe('string'); }); + + // #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(1); + expect(result.records[0].entityId).toBeUndefined(); + }); }); // ------------------------------------------------------- @@ -1276,6 +1286,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..5ed903f 100644 --- a/packages/core/tests/stack.test.ts +++ b/packages/core/tests/stack.test.ts @@ -1683,6 +1683,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', { From 0c6f8388783bdeec0e498535b2ab20282ac77625 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Jul 2026 18:55:28 +0000 Subject: [PATCH 2/8] feat(adapter-api): route Stack.putAttachment() through the atomic POST /attachments endpoint (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the ScopedStack.create() fix: POST /attachments now creates the _attachment@1 record atomically, but Stack.putAttachment() (client-side, wrapping APIAdapter) still made two separate wire calls under the hood — a bytes-only POST /attachments followed by a generic POST /records, which is now refused for non-owners. That left non-owner grantees using the SDK remotely with no working upload-with-metadata path. - StackBlobAdapter gains a required (not optional/capability-flagged) putAttachmentWithMetadata(data, mimeType, filename?) returning { fileId, record? }. Local storage adapters (disk, sqljs, memory) can't create a record themselves — a different backend — so they return no record; Stack.putAttachment() falls back to its existing create() call, unchanged. APIAdapter is the one implementation that populates record, via a single POST /attachments request. - Stack.putAttachment() calls the new method first and skips its own create() call whenever a record comes back, avoiding a redundant, potentially conflicting second write. - APIAdapter.putAttachment() (bytes-only) and putAttachmentWithMetadata() both parse the record POST /attachments now returns. - Wires @haverstack/conformance-fixtures' attachmentUploadFixtures into adapter-api's conformance test suite. - Updates docs/spec.md to describe the SDK's use of the endpoint and the resulting putAttachmentBytes()-over-HTTP quirk. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01LSNFMXRWseS34w8U2rt1u5 --- docs/spec.md | 8 ++- packages/adapter-api/src/index.ts | 41 +++++++++++++-- packages/adapter-api/tests/api.test.ts | 51 ++++++++++++++++++- .../adapter-api/tests/conformance.test.ts | 48 +++++++++++++++++ packages/adapter-local/src/index.ts | 8 +++ packages/adapter-local/tests/local.test.ts | 10 ++++ packages/blob-adapter-disk/src/index.ts | 5 ++ packages/blob-adapter-disk/tests/blob.test.ts | 11 ++++ packages/core/src/combine.ts | 2 + packages/core/src/stack.ts | 19 ++++++- packages/core/src/testing.ts | 5 ++ packages/core/src/types.ts | 27 ++++++++++ packages/core/tests/combine.test.ts | 19 +++++++ packages/core/tests/stack.test.ts | 45 +++++++++++++++- 14 files changed, 290 insertions(+), 9 deletions(-) diff --git a/docs/spec.md b/docs/spec.md index e108462..29e0a16 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -602,7 +602,7 @@ The adapter contract is split into two focused interfaces that are composed into **`StackRecordAdapter`** — structured storage: capabilities, stack identity (`ownerEntityId`, `timezone`), all record/association/version/type methods, and optional lifecycle hooks (`flush`, `close`). -**`StackBlobAdapter`** — binary storage: `putAttachment`, `getAttachment`, `deleteAttachment`, an optional `listFiles()` capability, and optional lifecycle hooks. +**`StackBlobAdapter`** — binary storage: `putAttachment`, `putAttachmentWithMetadata`, `getAttachment`, `deleteAttachment`, an optional `listFiles()` capability, and optional lifecycle hooks. ```ts type StackAdapter = StackRecordAdapter & StackBlobAdapter; @@ -610,6 +610,8 @@ type StackAdapter = StackRecordAdapter & StackBlobAdapter; **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. +`StackBlobAdapter.putAttachmentWithMetadata(data, mimeType, filename?)` (#106) is **not** part of that optional-capability pattern — it's required on every adapter, because there's exactly one correct answer for each: local storage adapters store bytes only and return `{ fileId }` with no `record`, since creating one is the record adapter's job, a different backend reached through a different object; the API adapter, backed by a single `POST /attachments` request the server fulfills in one call, returns `{ fileId, record }`. `Stack.putAttachment()` branches on whether `record` came back — present means skip its own `create()` call, absent means make it, exactly as `Stack.putAttachment()` always has. See [Attachments](#attachments) for why this is a correctness requirement (#106's anti-oracle fix), not an efficiency optimization. + ### Package naming convention Packages follow a naming convention that makes the adapter type discoverable: @@ -1114,6 +1116,10 @@ This is not an efficiency shortcut — it's the security boundary itself (#106). 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. +**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`. `StackBlobAdapter.putAttachmentWithMetadata()` is the adapter-level primitive this relies on: local storage adapters (disk, sqljs, memory) can't create a record themselves — that's the record adapter's job, a different backend — so they store bytes only and `Stack.putAttachment()` falls back to its own `create()` call, exactly as before this endpoint existed. Only the API adapter, backed by a single atomic request, returns the created record directly. + +One consequence: `StackBlobAdapter.putAttachment()` (the bytes-only primitive behind `Stack.putAttachmentBytes()`) still maps to this same endpoint over the wire, and this endpoint always creates a record now — so `Stack.putAttachmentBytes()`'s documented "no record created" contract holds for local storage but is only approximate over `APIAdapter` (a record with a default `mimeType` is created as a side effect). `Stack.putAttachmentBytes()` remains intended for owner/server-internal use against local storage; remote, non-owner callers should use `Stack.putAttachment()`. + **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: | Parameter | Effect | diff --git a/packages/adapter-api/src/index.ts b/packages/adapter-api/src/index.ts index 08366a2..7f8a443 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 putAttachment()/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,41 @@ export class APIAdapter implements StackAdapter { // Attachments // ------------------------------------------------------- + /** + * Bytes only, per the StackBlobAdapter contract — but POST /attachments + * always creates the accompanying _attachment@1 record now (#106): there + * is no bytes-only wire mode, since the whole point of this endpoint is + * that the record's fileId must come from bytes the server received in + * the same request. This call still ends up creating a record, with a + * default mimeType (application/octet-stream) and no filename, as a side + * effect. Stack.putAttachmentBytes()'s documented "no record created" + * contract holds for local storage adapters; over this adapter it's + * approximate. Stack.putAttachmentBytes() remains intended for owner/ + * server-internal use (spec §Attachments) — remote, non-owner callers + * should use putAttachmentWithMetadata() (Stack.putAttachment()) instead. + */ async putAttachment(data: Uint8Array): Promise { - const result = await this.uploadBinary('/attachments', data, 'application/octet-stream'); - return result.fileId as string; + const record = parseRecord( + await this.uploadBinary('/attachments', data, 'application/octet-stream'), + ); + return record.content.fileId as string; + } + + /** + * Store bytes and create the _attachment@1 record in one POST + * /attachments request — the wire counterpart of Stack.putAttachment() + * (#106). 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 + * StackBlobAdapter.putAttachmentWithMetadata(). + */ + async putAttachmentWithMetadata( + data: Uint8Array, + mimeType: string, + filename?: string, + ): Promise<{ fileId: FileId; record: StackRecord }> { + const record = parseRecord(await this.uploadBinary('/attachments', data, mimeType, filename)); + return { fileId: record.content.fileId as string, record }; } 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..38b147e 100644 --- a/packages/adapter-api/tests/api.test.ts +++ b/packages/adapter-api/tests/api.test.ts @@ -776,10 +776,22 @@ 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 () => { + test('sends POST /attachments with binary body and Content-Type: application/octet-stream, returns fileId from the created record', async () => { const adapter = await openAdapter(); - mockFetch.mockResolvedValueOnce(jsonResponse({ fileId: 'file-xyz' })); + mockFetch.mockResolvedValueOnce(jsonResponse(attachmentRecordResponse())); const data = new Uint8Array([0x89, 0x50, 0x4e, 0x47]); const fileId = await adapter.putAttachment(data); expect(fileId).toBe('file-xyz'); @@ -792,6 +804,41 @@ describe('putAttachment', () => { }); }); +describe('putAttachmentWithMetadata', () => { + test('sends POST /attachments with the given Content-Type and Content-Disposition, returns fileId and 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 result = await adapter.putAttachmentWithMetadata(data, 'image/png', 'photo.png'); + + expect(result.fileId).toBe('file-xyz'); + expect(result.record.id).toBe('rec-attachment-1'); + expect(result.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'); + 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(); + }); +}); + describe('getAttachment', () => { test('sends GET /attachments/:fileId and returns Uint8Array', async () => { const adapter = await openAdapter(); diff --git a/packages/adapter-api/tests/conformance.test.ts b/packages/adapter-api/tests/conformance.test.ts index a67d86e..8d565f8 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 result = await dispatch(); + const expectedFileId = (fixture.responseBody as { content: { fileId: string } }).content + .fileId; + expect(result.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/adapter-local/src/index.ts b/packages/adapter-local/src/index.ts index 854a5bd..ffc6ef5 100644 --- a/packages/adapter-local/src/index.ts +++ b/packages/adapter-local/src/index.ts @@ -260,6 +260,14 @@ export class LocalAdapter implements StackAdapter { return this.blob.putAttachment(data); } + async putAttachmentWithMetadata( + data: Uint8Array, + mimeType: string, + filename?: string, + ): Promise<{ fileId: FileId }> { + return this.blob.putAttachmentWithMetadata(data, mimeType, filename); + } + async getAttachment(fileId: FileId): Promise { return this.blob.getAttachment(fileId); } diff --git a/packages/adapter-local/tests/local.test.ts b/packages/adapter-local/tests/local.test.ts index bb811d0..0ad973a 100644 --- a/packages/adapter-local/tests/local.test.ts +++ b/packages/adapter-local/tests/local.test.ts @@ -140,6 +140,16 @@ describe('attachments', () => { expect(files[0].size).toBe(Buffer.from('hello attachment').byteLength); expect(files[0].modifiedAt).toBeInstanceOf(Date); }); + + // LocalAdapter delegates to its DiskBlobAdapter, which can't create a + // record itself (a different backend) — Stack.putAttachment() relies on + // `record` being absent here to fall back to its own create() call. + test('putAttachmentWithMetadata stores bytes and returns fileId only, no record', async () => { + const adapter = await initAdapter(); + const result = await adapter.putAttachmentWithMetadata(Buffer.from('hello'), 'text/plain'); + expect(result.fileId).toMatch(/^[0-9a-f]{64}$/); + expect(result.record).toBeUndefined(); + }); }); // ------------------------------------------------------- diff --git a/packages/blob-adapter-disk/src/index.ts b/packages/blob-adapter-disk/src/index.ts index c32de2c..5b07523 100644 --- a/packages/blob-adapter-disk/src/index.ts +++ b/packages/blob-adapter-disk/src/index.ts @@ -34,6 +34,11 @@ export class DiskBlobAdapter implements StackBlobAdapter { return fileId; } + /** Bytes storage only — this adapter has no access to record creation, a different backend. */ + async putAttachmentWithMetadata(data: Uint8Array): Promise<{ fileId: FileId }> { + return { fileId: await this.putAttachment(data) }; + } + async getAttachment(fileId: FileId): Promise { assertFileId(fileId); if (!existsSync(join(this.dir, fileId))) throw new Error(`Attachment not found: "${fileId}"`); diff --git a/packages/blob-adapter-disk/tests/blob.test.ts b/packages/blob-adapter-disk/tests/blob.test.ts index 8831524..c7d16e9 100644 --- a/packages/blob-adapter-disk/tests/blob.test.ts +++ b/packages/blob-adapter-disk/tests/blob.test.ts @@ -42,6 +42,17 @@ describe('DiskBlobAdapter', () => { expect(files).toContain(fileId); }); + // Bytes storage only — this adapter has no access to record creation + // (that's the record adapter's job, a different backend). Stack.putAttachment() + // relies on `record` being absent here to fall back to its own create() call. + test('putAttachmentWithMetadata stores bytes and returns fileId only, no record', async () => { + const result = await adapter.putAttachmentWithMetadata(Buffer.from('hello'), 'text/plain'); + expect(result.fileId).toMatch(/^[0-9a-f]{64}$/); + expect(result.record).toBeUndefined(); + const retrieved = await adapter.getAttachment(result.fileId); + expect((retrieved as Buffer).toString()).toBe('hello'); + }); + test('getAttachment throws for unknown fileId', async () => { const validHash = 'a'.repeat(64); await expect(adapter.getAttachment(validHash)).rejects.toThrow(); diff --git a/packages/core/src/combine.ts b/packages/core/src/combine.ts index 325b1a4..6e507e6 100644 --- a/packages/core/src/combine.ts +++ b/packages/core/src/combine.ts @@ -52,6 +52,8 @@ export function combineAdapters(parts: { }), putAttachment: (data) => parts.blob.putAttachment(data), + putAttachmentWithMetadata: (data, mimeType, filename) => + parts.blob.putAttachmentWithMetadata(data, mimeType, filename), getAttachment: (id) => parts.blob.getAttachment(id), deleteAttachment: (id) => parts.blob.deleteAttachment(id), ...(parts.blob.listFiles && { diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index 4a73729..87779ca 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -1253,9 +1253,26 @@ export class Stack implements StackClient { * 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. + * + * Delegates to the adapter's putAttachmentWithMetadata() first. Adapters + * that can create the record as part of the same operation (the API + * adapter, via one POST /attachments request the server fulfills + * atomically — #106) return it directly here, skipping the separate + * create() call below entirely — 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. Local storage adapters can't create a record + * themselves (a different backend, reached through a different object), + * so they return no record and this falls back to the create() call that + * was always here. */ async putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise { - const fileId = await this.putAttachmentBytes(data); + const { fileId, record } = await this.adapter.putAttachmentWithMetadata( + data, + mimeType, + filename, + ); + if (record) return fileId; await this.create(`${SYSTEM_TYPES.ATTACHMENT}@1`, { fileId, mimeType, diff --git a/packages/core/src/testing.ts b/packages/core/src/testing.ts index 1978386..d73db14 100644 --- a/packages/core/src/testing.ts +++ b/packages/core/src/testing.ts @@ -297,6 +297,11 @@ export class MemoryAdapter implements StackAdapter { } return fileId; } + + /** Local storage: bytes only, no record — Stack.putAttachment() supplies the create() step. */ + async putAttachmentWithMetadata(data: Uint8Array): Promise<{ fileId: string }> { + return { fileId: await this.putAttachment(data) }; + } // Deliberately lenient for fileIds never actually put (returns empty bytes, // not an error) — a lot of existing tests exercise permission logic with // synthetic fileIds that were never uploaded. Subclass and override this diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index dc7b81d..50cf949 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -517,6 +517,33 @@ export interface StackBlobAdapter { getAttachment(fileId: FileId): Promise; deleteAttachment(fileId: FileId): Promise; + /** + * Store bytes and, when the adapter can, create the accompanying + * _attachment@1 record in the same operation. Required on every adapter — + * not capability-flagged — because there is exactly one correct answer + * for each: local storage adapters (disk, sqljs, memory) cannot create a + * record themselves (that's the record adapter's job, a different + * backend reached through a different object), so they always store + * bytes only and return `record: undefined`; `Stack.putAttachment()` + * falls back to its own create() call in that case, unchanged from + * before this method existed. + * + * The API adapter is the one implementation that can genuinely do both in + * one operation — a single POST /attachments request the server fulfills + * atomically — and populates `record`. This is not an efficiency + * shortcut: fileId must be established from bytes the server actually + * received in *this* request, 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()` + * skips its own create() call whenever `record` is present, trusting it + * as authoritative rather than re-validating client-side. + */ + putAttachmentWithMetadata( + data: Uint8Array, + mimeType: string, + filename?: string, + ): Promise<{ fileId: FileId; record?: StackRecord }>; + /** * Enumerate every blob currently in storage. Optional — capability-flagged, * like StackRecordAdapter.deleteUnreferencedAttachmentRecords(). Without it, diff --git a/packages/core/tests/combine.test.ts b/packages/core/tests/combine.test.ts index aed09f9..d386e63 100644 --- a/packages/core/tests/combine.test.ts +++ b/packages/core/tests/combine.test.ts @@ -60,6 +60,7 @@ function makeRecordAdapter(overrides: Partial = {}): StackRe function makeBlobAdapter(overrides: Partial = {}): StackBlobAdapter { return { putAttachment: async () => 'file-id', + putAttachmentWithMetadata: async () => ({ fileId: 'file-id-with-metadata' }), getAttachment: async () => new Uint8Array(), deleteAttachment: async () => {}, ...overrides, @@ -112,6 +113,24 @@ describe('combineAdapters', () => { expect(fileId).toBe('computed-id'); }); + test('forwards putAttachmentWithMetadata to the blob part with all arguments', async () => { + let calledWith: [Uint8Array, string, string | undefined] | undefined; + const adapter = combineAdapters({ + record: makeRecordAdapter(), + blob: makeBlobAdapter({ + putAttachmentWithMetadata: async (data, mimeType, filename) => { + calledWith = [data, mimeType, filename]; + return { fileId: 'computed-id' }; + }, + }), + }); + + const bytes = new Uint8Array([1, 2, 3]); + const result = await adapter.putAttachmentWithMetadata(bytes, 'image/png', 'photo.png'); + expect(calledWith).toEqual([bytes, 'image/png', 'photo.png']); + expect(result).toEqual({ fileId: 'computed-id' }); + }); + // 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/stack.test.ts b/packages/core/tests/stack.test.ts index 5ed903f..6e0ad96 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, StackRecord } from '../src/types.js'; // ------------------------------------------------------- // Test setup @@ -1656,6 +1656,49 @@ describe('putAttachment', () => { }); }); +// ------------------------------------------------------- +// putAttachment — atomic path (#106): when the adapter creates the record +// as part of putAttachmentWithMetadata() (the API adapter, over one POST +// /attachments request), Stack.putAttachment() must not also make its own +// create() call — that would double-create (and, for a mismatched +// mimeType, conflict). Local adapters like MemoryAdapter return no record, +// so the pre-existing create() fallback is exercised by every other test +// in this describe block above. +// ------------------------------------------------------- + +describe('putAttachment — atomic adapter path (#106)', () => { + test('skips the separate create() call when the adapter already created the record', 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, + }; + vi.spyOn(adapter, 'putAttachmentWithMetadata').mockResolvedValue({ + fileId: 'atomic-file-id', + record: fabricatedRecord, + }); + const createSpy = vi.spyOn(stack, 'create'); + + const fileId = await stack.putAttachment(data, 'image/png', 'photo.png'); + + expect(fileId).toBe('atomic-file-id'); + expect(createSpy).not.toHaveBeenCalled(); + }); + + test('falls back to its own create() call when the adapter returns no record', 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. From a6fa676a4d3519738ebe56d762a1a9d04714d739 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 19:48:51 +0000 Subject: [PATCH 3/8] fix(core): correct putAttachmentWithMetadata signatures across local adapters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trivial local-adapter implementations from the previous commit declared narrower types than the StackBlobAdapter interface: return types omitted the optional `record` field, and mimeType/filename params were dropped entirely from the disk and in-memory adapters. Both were structurally assignable to the interface (fewer required params, a return subtype), so they built and passed vitest — which transforms test files without full type-checking — but failed `tsc --noEmit` on the concrete class types: mocking a record in a test, or calling with the full argument list, doesn't type-check against a narrower concrete signature. Also fixes an adapter-api test-only cast where WireRecord | WireError didn't overlap enough with the narrower shape being asserted. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01LSNFMXRWseS34w8U2rt1u5 --- packages/adapter-api/tests/conformance.test.ts | 4 ++-- packages/adapter-local/src/index.ts | 2 +- packages/blob-adapter-disk/src/index.ts | 8 ++++++-- packages/core/src/testing.ts | 6 +++++- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/adapter-api/tests/conformance.test.ts b/packages/adapter-api/tests/conformance.test.ts index 8d565f8..b05a71b 100644 --- a/packages/adapter-api/tests/conformance.test.ts +++ b/packages/adapter-api/tests/conformance.test.ts @@ -318,8 +318,8 @@ describe('attachment upload fixtures', () => { await expect(dispatch()).rejects.toThrow(ERROR_CLASS_FOR_CODE[code]); } else { const result = await dispatch(); - const expectedFileId = (fixture.responseBody as { content: { fileId: string } }).content - .fileId; + const expectedFileId = (fixture.responseBody as unknown as { content: { fileId: string } }) + .content.fileId; expect(result.fileId).toBe(expectedFileId); } diff --git a/packages/adapter-local/src/index.ts b/packages/adapter-local/src/index.ts index ffc6ef5..8b06f68 100644 --- a/packages/adapter-local/src/index.ts +++ b/packages/adapter-local/src/index.ts @@ -264,7 +264,7 @@ export class LocalAdapter implements StackAdapter { data: Uint8Array, mimeType: string, filename?: string, - ): Promise<{ fileId: FileId }> { + ): Promise<{ fileId: FileId; record?: StackRecord }> { return this.blob.putAttachmentWithMetadata(data, mimeType, filename); } diff --git a/packages/blob-adapter-disk/src/index.ts b/packages/blob-adapter-disk/src/index.ts index 5b07523..3a4d1a5 100644 --- a/packages/blob-adapter-disk/src/index.ts +++ b/packages/blob-adapter-disk/src/index.ts @@ -11,7 +11,7 @@ import { createHash } from 'node:crypto'; import { mkdirSync, existsSync } from 'fs'; import { readFile, writeFile, unlink, readdir, stat } from 'fs/promises'; import { join } from 'path'; -import type { StackBlobAdapter, BlobFileInfo, FileId } from '@haverstack/core'; +import type { StackBlobAdapter, BlobFileInfo, FileId, StackRecord } from '@haverstack/core'; const SHA256_HEX_RE = /^[0-9a-f]{64}$/; @@ -35,7 +35,11 @@ export class DiskBlobAdapter implements StackBlobAdapter { } /** Bytes storage only — this adapter has no access to record creation, a different backend. */ - async putAttachmentWithMetadata(data: Uint8Array): Promise<{ fileId: FileId }> { + async putAttachmentWithMetadata( + data: Uint8Array, + _mimeType: string, + _filename?: string, + ): Promise<{ fileId: FileId; record?: StackRecord }> { return { fileId: await this.putAttachment(data) }; } diff --git a/packages/core/src/testing.ts b/packages/core/src/testing.ts index d73db14..e737213 100644 --- a/packages/core/src/testing.ts +++ b/packages/core/src/testing.ts @@ -299,7 +299,11 @@ export class MemoryAdapter implements StackAdapter { } /** Local storage: bytes only, no record — Stack.putAttachment() supplies the create() step. */ - async putAttachmentWithMetadata(data: Uint8Array): Promise<{ fileId: string }> { + async putAttachmentWithMetadata( + data: Uint8Array, + _mimeType: string, + _filename?: string, + ): Promise<{ fileId: string; record?: StackRecord }> { return { fileId: await this.putAttachment(data) }; } // Deliberately lenient for fileIds never actually put (returns empty bytes, From beacce6e72753becfd033cb0762633a9f2506926 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 19:57:01 +0000 Subject: [PATCH 4/8] refactor(adapter-api): APIAdapter.putAttachment() delegates to putAttachmentWithMetadata() Both methods hit the same POST /attachments endpoint and got back a full record; putAttachment() had its own separate uploadBinary()+ parseRecord() call instead of reusing putAttachmentWithMetadata()'s, discarding the record for no reason other than that it was written before the second method existed. Now it's a two-line wrapper: call the richer method with the default mimeType, keep the fileId. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01LSNFMXRWseS34w8U2rt1u5 --- packages/adapter-api/src/index.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/adapter-api/src/index.ts b/packages/adapter-api/src/index.ts index 7f8a443..310b56b 100644 --- a/packages/adapter-api/src/index.ts +++ b/packages/adapter-api/src/index.ts @@ -575,17 +575,17 @@ export class APIAdapter implements StackAdapter { * that the record's fileId must come from bytes the server received in * the same request. This call still ends up creating a record, with a * default mimeType (application/octet-stream) and no filename, as a side - * effect. Stack.putAttachmentBytes()'s documented "no record created" - * contract holds for local storage adapters; over this adapter it's - * approximate. Stack.putAttachmentBytes() remains intended for owner/ - * server-internal use (spec §Attachments) — remote, non-owner callers - * should use putAttachmentWithMetadata() (Stack.putAttachment()) instead. + * effect — the same request putAttachmentWithMetadata() makes, just with + * the record discarded to satisfy this method's narrower return type. + * Stack.putAttachmentBytes()'s documented "no record created" contract + * holds for local storage adapters; over this adapter it's approximate. + * Stack.putAttachmentBytes() remains intended for owner/server-internal + * use (spec §Attachments) — remote, non-owner callers should use + * putAttachmentWithMetadata() (Stack.putAttachment()) instead. */ async putAttachment(data: Uint8Array): Promise { - const record = parseRecord( - await this.uploadBinary('/attachments', data, 'application/octet-stream'), - ); - return record.content.fileId as string; + const { fileId } = await this.putAttachmentWithMetadata(data, 'application/octet-stream'); + return fileId; } /** From d42824488e255ed2035e4e3df35dd20d58a7e236 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 21:08:51 +0000 Subject: [PATCH 5/8] refactor(core): rename putAttachmentWithMetadata to tryPutAttachmentWithMetadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The method's name promised metadata gets applied, but that's only ever true for the API adapter — every local storage adapter (disk, sqljs, memory) silently ignores mimeType/filename, since it has no access to record creation (a different backend). The `try` prefix makes that explicit: callers must check `record` in the result, not the method name, to know whether metadata was actually used. Pure rename, no behavior change. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01LSNFMXRWseS34w8U2rt1u5 --- docs/spec.md | 6 ++--- packages/adapter-api/src/index.ts | 12 +++++----- packages/adapter-api/tests/api.test.ts | 6 ++--- .../adapter-api/tests/conformance.test.ts | 4 ++-- packages/adapter-local/src/index.ts | 4 ++-- packages/adapter-local/tests/local.test.ts | 4 ++-- packages/blob-adapter-disk/src/index.ts | 2 +- packages/blob-adapter-disk/tests/blob.test.ts | 4 ++-- packages/core/src/combine.ts | 4 ++-- packages/core/src/stack.ts | 4 ++-- packages/core/src/testing.ts | 2 +- packages/core/src/types.ts | 22 ++++++++++--------- packages/core/tests/combine.test.ts | 8 +++---- packages/core/tests/stack.test.ts | 4 ++-- 14 files changed, 44 insertions(+), 42 deletions(-) diff --git a/docs/spec.md b/docs/spec.md index 29e0a16..5cc8923 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -602,7 +602,7 @@ The adapter contract is split into two focused interfaces that are composed into **`StackRecordAdapter`** — structured storage: capabilities, stack identity (`ownerEntityId`, `timezone`), all record/association/version/type methods, and optional lifecycle hooks (`flush`, `close`). -**`StackBlobAdapter`** — binary storage: `putAttachment`, `putAttachmentWithMetadata`, `getAttachment`, `deleteAttachment`, an optional `listFiles()` capability, and optional lifecycle hooks. +**`StackBlobAdapter`** — binary storage: `putAttachment`, `tryPutAttachmentWithMetadata`, `getAttachment`, `deleteAttachment`, an optional `listFiles()` capability, and optional lifecycle hooks. ```ts type StackAdapter = StackRecordAdapter & StackBlobAdapter; @@ -610,7 +610,7 @@ type StackAdapter = StackRecordAdapter & StackBlobAdapter; **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. -`StackBlobAdapter.putAttachmentWithMetadata(data, mimeType, filename?)` (#106) is **not** part of that optional-capability pattern — it's required on every adapter, because there's exactly one correct answer for each: local storage adapters store bytes only and return `{ fileId }` with no `record`, since creating one is the record adapter's job, a different backend reached through a different object; the API adapter, backed by a single `POST /attachments` request the server fulfills in one call, returns `{ fileId, record }`. `Stack.putAttachment()` branches on whether `record` came back — present means skip its own `create()` call, absent means make it, exactly as `Stack.putAttachment()` always has. See [Attachments](#attachments) for why this is a correctness requirement (#106's anti-oracle fix), not an efficiency optimization. +`StackBlobAdapter.tryPutAttachmentWithMetadata(data, mimeType, filename?)` (#106) is **not** part of that optional-capability pattern — it's required on every adapter, because there's exactly one correct answer for each: local storage adapters store bytes only and return `{ fileId }` with no `record`, since creating one is the record adapter's job, a different backend reached through a different object; the API adapter, backed by a single `POST /attachments` request the server fulfills in one call, returns `{ fileId, record }`. The `try` prefix is deliberate: `mimeType`/`filename` are accepted by every implementation but only ever _acted on_ by the one that can — callers must check `record` in the result, not the method name, to know whether metadata was actually applied. `Stack.putAttachment()` branches on it — present means skip its own `create()` call, absent means make it, exactly as `Stack.putAttachment()` always has. See [Attachments](#attachments) for why this is a correctness requirement (#106's anti-oracle fix), not an efficiency optimization. ### Package naming convention @@ -1116,7 +1116,7 @@ This is not an efficiency shortcut — it's the security boundary itself (#106). 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. -**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`. `StackBlobAdapter.putAttachmentWithMetadata()` is the adapter-level primitive this relies on: local storage adapters (disk, sqljs, memory) can't create a record themselves — that's the record adapter's job, a different backend — so they store bytes only and `Stack.putAttachment()` falls back to its own `create()` call, exactly as before this endpoint existed. Only the API adapter, backed by a single atomic request, returns the created record directly. +**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`. `StackBlobAdapter.tryPutAttachmentWithMetadata()` is the adapter-level primitive this relies on: local storage adapters (disk, sqljs, memory) can't create a record themselves — that's the record adapter's job, a different backend — so they store bytes only and `Stack.putAttachment()` falls back to its own `create()` call, exactly as before this endpoint existed. Only the API adapter, backed by a single atomic request, returns the created record directly. One consequence: `StackBlobAdapter.putAttachment()` (the bytes-only primitive behind `Stack.putAttachmentBytes()`) still maps to this same endpoint over the wire, and this endpoint always creates a record now — so `Stack.putAttachmentBytes()`'s documented "no record created" contract holds for local storage but is only approximate over `APIAdapter` (a record with a default `mimeType` is created as a side effect). `Stack.putAttachmentBytes()` remains intended for owner/server-internal use against local storage; remote, non-owner callers should use `Stack.putAttachment()`. diff --git a/packages/adapter-api/src/index.ts b/packages/adapter-api/src/index.ts index 310b56b..9e42429 100644 --- a/packages/adapter-api/src/index.ts +++ b/packages/adapter-api/src/index.ts @@ -326,7 +326,7 @@ export class APIAdapter implements StackAdapter { return new Uint8Array(await res.arrayBuffer()); } - /** POST /attachments always returns the created _attachment@1 record (#106) — see putAttachment()/putAttachmentWithMetadata() below. */ + /** POST /attachments always returns the created _attachment@1 record (#106) — see putAttachment()/tryPutAttachmentWithMetadata() below. */ private async uploadBinary( path: string, data: Uint8Array, @@ -575,16 +575,16 @@ export class APIAdapter implements StackAdapter { * that the record's fileId must come from bytes the server received in * the same request. This call still ends up creating a record, with a * default mimeType (application/octet-stream) and no filename, as a side - * effect — the same request putAttachmentWithMetadata() makes, just with + * effect — the same request tryPutAttachmentWithMetadata() makes, just with * the record discarded to satisfy this method's narrower return type. * Stack.putAttachmentBytes()'s documented "no record created" contract * holds for local storage adapters; over this adapter it's approximate. * Stack.putAttachmentBytes() remains intended for owner/server-internal * use (spec §Attachments) — remote, non-owner callers should use - * putAttachmentWithMetadata() (Stack.putAttachment()) instead. + * tryPutAttachmentWithMetadata() (Stack.putAttachment()) instead. */ async putAttachment(data: Uint8Array): Promise { - const { fileId } = await this.putAttachmentWithMetadata(data, 'application/octet-stream'); + const { fileId } = await this.tryPutAttachmentWithMetadata(data, 'application/octet-stream'); return fileId; } @@ -594,9 +594,9 @@ export class APIAdapter implements StackAdapter { * (#106). 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 - * StackBlobAdapter.putAttachmentWithMetadata(). + * StackBlobAdapter.tryPutAttachmentWithMetadata(). */ - async putAttachmentWithMetadata( + async tryPutAttachmentWithMetadata( data: Uint8Array, mimeType: string, filename?: string, diff --git a/packages/adapter-api/tests/api.test.ts b/packages/adapter-api/tests/api.test.ts index 38b147e..7bbdbea 100644 --- a/packages/adapter-api/tests/api.test.ts +++ b/packages/adapter-api/tests/api.test.ts @@ -804,7 +804,7 @@ describe('putAttachment', () => { }); }); -describe('putAttachmentWithMetadata', () => { +describe('tryPutAttachmentWithMetadata', () => { test('sends POST /attachments with the given Content-Type and Content-Disposition, returns fileId and the parsed record', async () => { const adapter = await openAdapter(); mockFetch.mockResolvedValueOnce( @@ -815,7 +815,7 @@ describe('putAttachmentWithMetadata', () => { ), ); const data = new Uint8Array([0x89, 0x50, 0x4e, 0x47]); - const result = await adapter.putAttachmentWithMetadata(data, 'image/png', 'photo.png'); + const result = await adapter.tryPutAttachmentWithMetadata(data, 'image/png', 'photo.png'); expect(result.fileId).toBe('file-xyz'); expect(result.record.id).toBe('rec-attachment-1'); @@ -832,7 +832,7 @@ describe('putAttachmentWithMetadata', () => { 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'); + await adapter.tryPutAttachmentWithMetadata(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 b05a71b..441be89 100644 --- a/packages/adapter-api/tests/conformance.test.ts +++ b/packages/adapter-api/tests/conformance.test.ts @@ -289,7 +289,7 @@ 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 +// dispatched here: tryPutAttachmentWithMetadata()'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. @@ -309,7 +309,7 @@ describe('attachment upload fixtures', () => { const filename = filenameMatch ? decodeURIComponent(filenameMatch[1]) : undefined; const data = new Uint8Array(fixture.requestBodyBytes); - const dispatch = () => adapter.putAttachmentWithMetadata(data, contentType, filename); + const dispatch = () => adapter.tryPutAttachmentWithMetadata(data, contentType, filename); if (fixture.responseStatus >= 400) { const code = ( diff --git a/packages/adapter-local/src/index.ts b/packages/adapter-local/src/index.ts index 8b06f68..278505c 100644 --- a/packages/adapter-local/src/index.ts +++ b/packages/adapter-local/src/index.ts @@ -260,12 +260,12 @@ export class LocalAdapter implements StackAdapter { return this.blob.putAttachment(data); } - async putAttachmentWithMetadata( + async tryPutAttachmentWithMetadata( data: Uint8Array, mimeType: string, filename?: string, ): Promise<{ fileId: FileId; record?: StackRecord }> { - return this.blob.putAttachmentWithMetadata(data, mimeType, filename); + return this.blob.tryPutAttachmentWithMetadata(data, mimeType, filename); } async getAttachment(fileId: FileId): Promise { diff --git a/packages/adapter-local/tests/local.test.ts b/packages/adapter-local/tests/local.test.ts index 0ad973a..756e9ac 100644 --- a/packages/adapter-local/tests/local.test.ts +++ b/packages/adapter-local/tests/local.test.ts @@ -144,9 +144,9 @@ describe('attachments', () => { // LocalAdapter delegates to its DiskBlobAdapter, which can't create a // record itself (a different backend) — Stack.putAttachment() relies on // `record` being absent here to fall back to its own create() call. - test('putAttachmentWithMetadata stores bytes and returns fileId only, no record', async () => { + test('tryPutAttachmentWithMetadata stores bytes and returns fileId only, no record', async () => { const adapter = await initAdapter(); - const result = await adapter.putAttachmentWithMetadata(Buffer.from('hello'), 'text/plain'); + const result = await adapter.tryPutAttachmentWithMetadata(Buffer.from('hello'), 'text/plain'); expect(result.fileId).toMatch(/^[0-9a-f]{64}$/); expect(result.record).toBeUndefined(); }); diff --git a/packages/blob-adapter-disk/src/index.ts b/packages/blob-adapter-disk/src/index.ts index 3a4d1a5..8039218 100644 --- a/packages/blob-adapter-disk/src/index.ts +++ b/packages/blob-adapter-disk/src/index.ts @@ -35,7 +35,7 @@ export class DiskBlobAdapter implements StackBlobAdapter { } /** Bytes storage only — this adapter has no access to record creation, a different backend. */ - async putAttachmentWithMetadata( + async tryPutAttachmentWithMetadata( data: Uint8Array, _mimeType: string, _filename?: string, diff --git a/packages/blob-adapter-disk/tests/blob.test.ts b/packages/blob-adapter-disk/tests/blob.test.ts index c7d16e9..6e59467 100644 --- a/packages/blob-adapter-disk/tests/blob.test.ts +++ b/packages/blob-adapter-disk/tests/blob.test.ts @@ -45,8 +45,8 @@ describe('DiskBlobAdapter', () => { // Bytes storage only — this adapter has no access to record creation // (that's the record adapter's job, a different backend). Stack.putAttachment() // relies on `record` being absent here to fall back to its own create() call. - test('putAttachmentWithMetadata stores bytes and returns fileId only, no record', async () => { - const result = await adapter.putAttachmentWithMetadata(Buffer.from('hello'), 'text/plain'); + test('tryPutAttachmentWithMetadata stores bytes and returns fileId only, no record', async () => { + const result = await adapter.tryPutAttachmentWithMetadata(Buffer.from('hello'), 'text/plain'); expect(result.fileId).toMatch(/^[0-9a-f]{64}$/); expect(result.record).toBeUndefined(); const retrieved = await adapter.getAttachment(result.fileId); diff --git a/packages/core/src/combine.ts b/packages/core/src/combine.ts index 6e507e6..c122ddf 100644 --- a/packages/core/src/combine.ts +++ b/packages/core/src/combine.ts @@ -52,8 +52,8 @@ export function combineAdapters(parts: { }), putAttachment: (data) => parts.blob.putAttachment(data), - putAttachmentWithMetadata: (data, mimeType, filename) => - parts.blob.putAttachmentWithMetadata(data, mimeType, filename), + tryPutAttachmentWithMetadata: (data, mimeType, filename) => + parts.blob.tryPutAttachmentWithMetadata(data, mimeType, filename), getAttachment: (id) => parts.blob.getAttachment(id), deleteAttachment: (id) => parts.blob.deleteAttachment(id), ...(parts.blob.listFiles && { diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index 87779ca..87041ac 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -1254,7 +1254,7 @@ export class Stack implements StackClient { * no entityId). Use ScopedStack.putAttachment() when the uploader is a * specific entity rather than the stack owner. * - * Delegates to the adapter's putAttachmentWithMetadata() first. Adapters + * Delegates to the adapter's tryPutAttachmentWithMetadata() first. Adapters * that can create the record as part of the same operation (the API * adapter, via one POST /attachments request the server fulfills * atomically — #106) return it directly here, skipping the separate @@ -1267,7 +1267,7 @@ export class Stack implements StackClient { * was always here. */ async putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise { - const { fileId, record } = await this.adapter.putAttachmentWithMetadata( + const { fileId, record } = await this.adapter.tryPutAttachmentWithMetadata( data, mimeType, filename, diff --git a/packages/core/src/testing.ts b/packages/core/src/testing.ts index e737213..23fd646 100644 --- a/packages/core/src/testing.ts +++ b/packages/core/src/testing.ts @@ -299,7 +299,7 @@ export class MemoryAdapter implements StackAdapter { } /** Local storage: bytes only, no record — Stack.putAttachment() supplies the create() step. */ - async putAttachmentWithMetadata( + async tryPutAttachmentWithMetadata( data: Uint8Array, _mimeType: string, _filename?: string, diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 50cf949..2bbddf5 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -518,15 +518,17 @@ export interface StackBlobAdapter { deleteAttachment(fileId: FileId): Promise; /** - * Store bytes and, when the adapter can, create the accompanying - * _attachment@1 record in the same operation. Required on every adapter — - * not capability-flagged — because there is exactly one correct answer - * for each: local storage adapters (disk, sqljs, memory) cannot create a - * record themselves (that's the record adapter's job, a different - * backend reached through a different object), so they always store - * bytes only and return `record: undefined`; `Stack.putAttachment()` - * falls back to its own create() call in that case, unchanged from - * before this method existed. + * *Try* to store bytes and create the accompanying _attachment@1 record + * in the same operation — `mimeType`/`filename` are not guaranteed to be + * used. Required on every adapter, but only ever fully honored by one: + * local storage adapters (disk, sqljs, memory) cannot create a record + * themselves (that's the record adapter's job, a different backend + * reached through a different object), so they always store bytes only, + * silently ignore `mimeType`/`filename`, and return `record: undefined`; + * `Stack.putAttachment()` falls back to its own create() call in that + * case, unchanged from before this method existed. Check `record` in the + * result, not the method name, to know whether metadata was actually + * applied. * * The API adapter is the one implementation that can genuinely do both in * one operation — a single POST /attachments request the server fulfills @@ -538,7 +540,7 @@ export interface StackBlobAdapter { * skips its own create() call whenever `record` is present, trusting it * as authoritative rather than re-validating client-side. */ - putAttachmentWithMetadata( + tryPutAttachmentWithMetadata( data: Uint8Array, mimeType: string, filename?: string, diff --git a/packages/core/tests/combine.test.ts b/packages/core/tests/combine.test.ts index d386e63..c224dba 100644 --- a/packages/core/tests/combine.test.ts +++ b/packages/core/tests/combine.test.ts @@ -60,7 +60,7 @@ function makeRecordAdapter(overrides: Partial = {}): StackRe function makeBlobAdapter(overrides: Partial = {}): StackBlobAdapter { return { putAttachment: async () => 'file-id', - putAttachmentWithMetadata: async () => ({ fileId: 'file-id-with-metadata' }), + tryPutAttachmentWithMetadata: async () => ({ fileId: 'file-id-with-metadata' }), getAttachment: async () => new Uint8Array(), deleteAttachment: async () => {}, ...overrides, @@ -113,12 +113,12 @@ describe('combineAdapters', () => { expect(fileId).toBe('computed-id'); }); - test('forwards putAttachmentWithMetadata to the blob part with all arguments', async () => { + test('forwards tryPutAttachmentWithMetadata to the blob part with all arguments', async () => { let calledWith: [Uint8Array, string, string | undefined] | undefined; const adapter = combineAdapters({ record: makeRecordAdapter(), blob: makeBlobAdapter({ - putAttachmentWithMetadata: async (data, mimeType, filename) => { + tryPutAttachmentWithMetadata: async (data, mimeType, filename) => { calledWith = [data, mimeType, filename]; return { fileId: 'computed-id' }; }, @@ -126,7 +126,7 @@ describe('combineAdapters', () => { }); const bytes = new Uint8Array([1, 2, 3]); - const result = await adapter.putAttachmentWithMetadata(bytes, 'image/png', 'photo.png'); + const result = await adapter.tryPutAttachmentWithMetadata(bytes, 'image/png', 'photo.png'); expect(calledWith).toEqual([bytes, 'image/png', 'photo.png']); expect(result).toEqual({ fileId: 'computed-id' }); }); diff --git a/packages/core/tests/stack.test.ts b/packages/core/tests/stack.test.ts index 6e0ad96..3a8c4ed 100644 --- a/packages/core/tests/stack.test.ts +++ b/packages/core/tests/stack.test.ts @@ -1658,7 +1658,7 @@ describe('putAttachment', () => { // ------------------------------------------------------- // putAttachment — atomic path (#106): when the adapter creates the record -// as part of putAttachmentWithMetadata() (the API adapter, over one POST +// as part of tryPutAttachmentWithMetadata() (the API adapter, over one POST // /attachments request), Stack.putAttachment() must not also make its own // create() call — that would double-create (and, for a mismatched // mimeType, conflict). Local adapters like MemoryAdapter return no record, @@ -1677,7 +1677,7 @@ describe('putAttachment — atomic adapter path (#106)', () => { content: { fileId: 'atomic-file-id', mimeType: 'image/png', size: 3, filename: 'photo.png' }, version: 1, }; - vi.spyOn(adapter, 'putAttachmentWithMetadata').mockResolvedValue({ + vi.spyOn(adapter, 'tryPutAttachmentWithMetadata').mockResolvedValue({ fileId: 'atomic-file-id', record: fabricatedRecord, }); From d56c22594710ddb42f4b562540d741b1c9d7babb Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 14:46:47 +0000 Subject: [PATCH 6/8] refactor(core): make atomic attachment upload an optional StackAdapter capability (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The combined bytes+record upload was a required method on StackBlobAdapter, an interface whose contract is bytes-only — so three of four implementations were stubs that ignored two of their three parameters, the method needed a 'try' prefix and a { fileId, record? } result to model half-support, and the spec needed a paragraph explaining why this wasn't the repo's optional-capability pattern. Move it to where the capability actually lives: putAttachmentWithMetadata? is now an optional method on the composed StackAdapter type, following the deleteUnreferencedAttachmentRecords()/listFiles() idiom — implemented only by APIAdapter (one atomic POST /attachments), never synthesized by combineAdapters() (a record backend glued to a blob backend has no shared transaction), and truthiness-checked by Stack.putAttachment(), which otherwise falls back to its pre-existing bytes-then-create() sequence. Declaring it on the intersection type rather than either half encodes the structural fact driving #106: 'bytes + record in one operation' is a property only a whole adapter can have — a blob adapter cannot even declare it. Deletes all local-adapter stubs, the try prefix, the optional- record result type, and the spec's pattern-deviation paragraph; the ScopedStack create() guard, carve-out, anti-oracle message, and E1 fix are unchanged. On the atomic path the returned record is server-authoritative: client-side validation and the mimeType-conflict check are documented as deliberately skipped (the server runs both; a client-side conflict check against remote state would be racy and itself a mini-oracle). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QQBTuf3CnEwdvH76nvazJF --- docs/spec.md | 18 ++++-- packages/adapter-api/src/index.ts | 31 +++++----- packages/adapter-api/tests/api.test.ts | 13 ++-- .../adapter-api/tests/conformance.test.ts | 8 +-- packages/adapter-local/src/index.ts | 8 --- packages/adapter-local/tests/local.test.ts | 10 --- packages/blob-adapter-disk/src/index.ts | 11 +--- packages/blob-adapter-disk/tests/blob.test.ts | 11 ---- packages/core/src/combine.ts | 8 ++- packages/core/src/stack.ts | 35 +++++------ packages/core/src/testing.ts | 9 --- packages/core/src/types.ts | 61 ++++++++++--------- packages/core/tests/combine.test.ts | 23 +++---- packages/core/tests/stack.test.ts | 37 ++++++----- 14 files changed, 126 insertions(+), 157 deletions(-) diff --git a/docs/spec.md b/docs/spec.md index 5cc8923..5222e6a 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -602,15 +602,23 @@ The adapter contract is split into two focused interfaces that are composed into **`StackRecordAdapter`** — structured storage: capabilities, stack identity (`ownerEntityId`, `timezone`), all record/association/version/type methods, and optional lifecycle hooks (`flush`, `close`). -**`StackBlobAdapter`** — binary storage: `putAttachment`, `tryPutAttachmentWithMetadata`, `getAttachment`, `deleteAttachment`, an optional `listFiles()` capability, and optional lifecycle hooks. +**`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. -`StackBlobAdapter.tryPutAttachmentWithMetadata(data, mimeType, filename?)` (#106) is **not** part of that optional-capability pattern — it's required on every adapter, because there's exactly one correct answer for each: local storage adapters store bytes only and return `{ fileId }` with no `record`, since creating one is the record adapter's job, a different backend reached through a different object; the API adapter, backed by a single `POST /attachments` request the server fulfills in one call, returns `{ fileId, record }`. The `try` prefix is deliberate: `mimeType`/`filename` are accepted by every implementation but only ever _acted on_ by the one that can — callers must check `record` in the result, not the method name, to know whether metadata was actually applied. `Stack.putAttachment()` branches on it — present means skip its own `create()` call, absent means make it, exactly as `Stack.putAttachment()` always has. See [Attachments](#attachments) for why this is a correctness requirement (#106's anti-oracle fix), not an efficiency optimization. +`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 @@ -1116,7 +1124,7 @@ This is not an efficiency shortcut — it's the security boundary itself (#106). 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. -**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`. `StackBlobAdapter.tryPutAttachmentWithMetadata()` is the adapter-level primitive this relies on: local storage adapters (disk, sqljs, memory) can't create a record themselves — that's the record adapter's job, a different backend — so they store bytes only and `Stack.putAttachment()` falls back to its own `create()` call, exactly as before this endpoint existed. Only the API adapter, backed by a single atomic request, returns the created record directly. +**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: `StackBlobAdapter.putAttachment()` (the bytes-only primitive behind `Stack.putAttachmentBytes()`) still maps to this same endpoint over the wire, and this endpoint always creates a record now — so `Stack.putAttachmentBytes()`'s documented "no record created" contract holds for local storage but is only approximate over `APIAdapter` (a record with a default `mimeType` is created as a side effect). `Stack.putAttachmentBytes()` remains intended for owner/server-internal use against local storage; remote, non-owner callers should use `Stack.putAttachment()`. diff --git a/packages/adapter-api/src/index.ts b/packages/adapter-api/src/index.ts index 9e42429..683ca25 100644 --- a/packages/adapter-api/src/index.ts +++ b/packages/adapter-api/src/index.ts @@ -326,7 +326,7 @@ export class APIAdapter implements StackAdapter { return new Uint8Array(await res.arrayBuffer()); } - /** POST /attachments always returns the created _attachment@1 record (#106) — see putAttachment()/tryPutAttachmentWithMetadata() below. */ + /** POST /attachments always returns the created _attachment@1 record (#106) — see putAttachment()/putAttachmentWithMetadata() below. */ private async uploadBinary( path: string, data: Uint8Array, @@ -575,34 +575,35 @@ export class APIAdapter implements StackAdapter { * that the record's fileId must come from bytes the server received in * the same request. This call still ends up creating a record, with a * default mimeType (application/octet-stream) and no filename, as a side - * effect — the same request tryPutAttachmentWithMetadata() makes, just with + * effect — the same request putAttachmentWithMetadata() makes, just with * the record discarded to satisfy this method's narrower return type. * Stack.putAttachmentBytes()'s documented "no record created" contract * holds for local storage adapters; over this adapter it's approximate. * Stack.putAttachmentBytes() remains intended for owner/server-internal * use (spec §Attachments) — remote, non-owner callers should use - * tryPutAttachmentWithMetadata() (Stack.putAttachment()) instead. + * putAttachmentWithMetadata() (Stack.putAttachment()) instead. */ async putAttachment(data: Uint8Array): Promise { - const { fileId } = await this.tryPutAttachmentWithMetadata(data, 'application/octet-stream'); - return fileId; + const record = await this.putAttachmentWithMetadata(data, 'application/octet-stream'); + return record.content.fileId as string; } /** - * Store bytes and create the _attachment@1 record in one POST - * /attachments request — the wire counterpart of Stack.putAttachment() - * (#106). 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 - * StackBlobAdapter.tryPutAttachmentWithMetadata(). + * 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 tryPutAttachmentWithMetadata( + async putAttachmentWithMetadata( data: Uint8Array, mimeType: string, filename?: string, - ): Promise<{ fileId: FileId; record: StackRecord }> { - const record = parseRecord(await this.uploadBinary('/attachments', data, mimeType, filename)); - return { fileId: record.content.fileId as string, record }; + ): 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 7bbdbea..c10cebc 100644 --- a/packages/adapter-api/tests/api.test.ts +++ b/packages/adapter-api/tests/api.test.ts @@ -804,8 +804,8 @@ describe('putAttachment', () => { }); }); -describe('tryPutAttachmentWithMetadata', () => { - test('sends POST /attachments with the given Content-Type and Content-Disposition, returns fileId and the parsed record', async () => { +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( @@ -815,11 +815,10 @@ describe('tryPutAttachmentWithMetadata', () => { ), ); const data = new Uint8Array([0x89, 0x50, 0x4e, 0x47]); - const result = await adapter.tryPutAttachmentWithMetadata(data, 'image/png', 'photo.png'); + const record = await adapter.putAttachmentWithMetadata(data, 'image/png', 'photo.png'); - expect(result.fileId).toBe('file-xyz'); - expect(result.record.id).toBe('rec-attachment-1'); - expect(result.record.content).toMatchObject({ fileId: 'file-xyz', mimeType: 'image/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`); @@ -832,7 +831,7 @@ describe('tryPutAttachmentWithMetadata', () => { test('omits Content-Disposition when no filename is given', async () => { const adapter = await openAdapter(); mockFetch.mockResolvedValueOnce(jsonResponse(attachmentRecordResponse())); - await adapter.tryPutAttachmentWithMetadata(new Uint8Array([1]), 'application/octet-stream'); + 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 441be89..506b4bc 100644 --- a/packages/adapter-api/tests/conformance.test.ts +++ b/packages/adapter-api/tests/conformance.test.ts @@ -289,7 +289,7 @@ 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: tryPutAttachmentWithMetadata()'s mimeType is required, so +// 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. @@ -309,7 +309,7 @@ describe('attachment upload fixtures', () => { const filename = filenameMatch ? decodeURIComponent(filenameMatch[1]) : undefined; const data = new Uint8Array(fixture.requestBodyBytes); - const dispatch = () => adapter.tryPutAttachmentWithMetadata(data, contentType, filename); + const dispatch = () => adapter.putAttachmentWithMetadata(data, contentType, filename); if (fixture.responseStatus >= 400) { const code = ( @@ -317,10 +317,10 @@ describe('attachment upload fixtures', () => { ).error.code; await expect(dispatch()).rejects.toThrow(ERROR_CLASS_FOR_CODE[code]); } else { - const result = await dispatch(); + const record = await dispatch(); const expectedFileId = (fixture.responseBody as unknown as { content: { fileId: string } }) .content.fileId; - expect(result.fileId).toBe(expectedFileId); + expect(record.content.fileId).toBe(expectedFileId); } const [url, init] = mockFetch.mock.lastCall as [string, RequestInit]; diff --git a/packages/adapter-local/src/index.ts b/packages/adapter-local/src/index.ts index 278505c..854a5bd 100644 --- a/packages/adapter-local/src/index.ts +++ b/packages/adapter-local/src/index.ts @@ -260,14 +260,6 @@ export class LocalAdapter implements StackAdapter { return this.blob.putAttachment(data); } - async tryPutAttachmentWithMetadata( - data: Uint8Array, - mimeType: string, - filename?: string, - ): Promise<{ fileId: FileId; record?: StackRecord }> { - return this.blob.tryPutAttachmentWithMetadata(data, mimeType, filename); - } - async getAttachment(fileId: FileId): Promise { return this.blob.getAttachment(fileId); } diff --git a/packages/adapter-local/tests/local.test.ts b/packages/adapter-local/tests/local.test.ts index 756e9ac..bb811d0 100644 --- a/packages/adapter-local/tests/local.test.ts +++ b/packages/adapter-local/tests/local.test.ts @@ -140,16 +140,6 @@ describe('attachments', () => { expect(files[0].size).toBe(Buffer.from('hello attachment').byteLength); expect(files[0].modifiedAt).toBeInstanceOf(Date); }); - - // LocalAdapter delegates to its DiskBlobAdapter, which can't create a - // record itself (a different backend) — Stack.putAttachment() relies on - // `record` being absent here to fall back to its own create() call. - test('tryPutAttachmentWithMetadata stores bytes and returns fileId only, no record', async () => { - const adapter = await initAdapter(); - const result = await adapter.tryPutAttachmentWithMetadata(Buffer.from('hello'), 'text/plain'); - expect(result.fileId).toMatch(/^[0-9a-f]{64}$/); - expect(result.record).toBeUndefined(); - }); }); // ------------------------------------------------------- diff --git a/packages/blob-adapter-disk/src/index.ts b/packages/blob-adapter-disk/src/index.ts index 8039218..c32de2c 100644 --- a/packages/blob-adapter-disk/src/index.ts +++ b/packages/blob-adapter-disk/src/index.ts @@ -11,7 +11,7 @@ import { createHash } from 'node:crypto'; import { mkdirSync, existsSync } from 'fs'; import { readFile, writeFile, unlink, readdir, stat } from 'fs/promises'; import { join } from 'path'; -import type { StackBlobAdapter, BlobFileInfo, FileId, StackRecord } from '@haverstack/core'; +import type { StackBlobAdapter, BlobFileInfo, FileId } from '@haverstack/core'; const SHA256_HEX_RE = /^[0-9a-f]{64}$/; @@ -34,15 +34,6 @@ export class DiskBlobAdapter implements StackBlobAdapter { return fileId; } - /** Bytes storage only — this adapter has no access to record creation, a different backend. */ - async tryPutAttachmentWithMetadata( - data: Uint8Array, - _mimeType: string, - _filename?: string, - ): Promise<{ fileId: FileId; record?: StackRecord }> { - return { fileId: await this.putAttachment(data) }; - } - async getAttachment(fileId: FileId): Promise { assertFileId(fileId); if (!existsSync(join(this.dir, fileId))) throw new Error(`Attachment not found: "${fileId}"`); diff --git a/packages/blob-adapter-disk/tests/blob.test.ts b/packages/blob-adapter-disk/tests/blob.test.ts index 6e59467..8831524 100644 --- a/packages/blob-adapter-disk/tests/blob.test.ts +++ b/packages/blob-adapter-disk/tests/blob.test.ts @@ -42,17 +42,6 @@ describe('DiskBlobAdapter', () => { expect(files).toContain(fileId); }); - // Bytes storage only — this adapter has no access to record creation - // (that's the record adapter's job, a different backend). Stack.putAttachment() - // relies on `record` being absent here to fall back to its own create() call. - test('tryPutAttachmentWithMetadata stores bytes and returns fileId only, no record', async () => { - const result = await adapter.tryPutAttachmentWithMetadata(Buffer.from('hello'), 'text/plain'); - expect(result.fileId).toMatch(/^[0-9a-f]{64}$/); - expect(result.record).toBeUndefined(); - const retrieved = await adapter.getAttachment(result.fileId); - expect((retrieved as Buffer).toString()).toBe('hello'); - }); - test('getAttachment throws for unknown fileId', async () => { const validHash = 'a'.repeat(64); await expect(adapter.getAttachment(validHash)).rejects.toThrow(); diff --git a/packages/core/src/combine.ts b/packages/core/src/combine.ts index c122ddf..fca768a 100644 --- a/packages/core/src/combine.ts +++ b/packages/core/src/combine.ts @@ -51,9 +51,13 @@ 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), - tryPutAttachmentWithMetadata: (data, mimeType, filename) => - parts.blob.tryPutAttachmentWithMetadata(data, mimeType, filename), getAttachment: (id) => parts.blob.getAttachment(id), deleteAttachment: (id) => parts.blob.deleteAttachment(id), ...(parts.blob.listFiles && { diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index 87041ac..96fb574 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -1254,25 +1254,26 @@ export class Stack implements StackClient { * no entityId). Use ScopedStack.putAttachment() when the uploader is a * specific entity rather than the stack owner. * - * Delegates to the adapter's tryPutAttachmentWithMetadata() first. Adapters - * that can create the record as part of the same operation (the API - * adapter, via one POST /attachments request the server fulfills - * atomically — #106) return it directly here, skipping the separate - * create() call below entirely — 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. Local storage adapters can't create a record - * themselves (a different backend, reached through a different object), - * so they return no record and this falls back to the create() call that - * was always here. + * 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, record } = await this.adapter.tryPutAttachmentWithMetadata( - data, - mimeType, - filename, - ); - if (record) return fileId; + if (this.adapter.putAttachmentWithMetadata) { + const record = await this.adapter.putAttachmentWithMetadata(data, mimeType, filename); + return (record.content as AttachmentContent).fileId; + } + const fileId = await this.putAttachmentBytes(data); await this.create(`${SYSTEM_TYPES.ATTACHMENT}@1`, { fileId, mimeType, diff --git a/packages/core/src/testing.ts b/packages/core/src/testing.ts index 23fd646..1978386 100644 --- a/packages/core/src/testing.ts +++ b/packages/core/src/testing.ts @@ -297,15 +297,6 @@ export class MemoryAdapter implements StackAdapter { } return fileId; } - - /** Local storage: bytes only, no record — Stack.putAttachment() supplies the create() step. */ - async tryPutAttachmentWithMetadata( - data: Uint8Array, - _mimeType: string, - _filename?: string, - ): Promise<{ fileId: string; record?: StackRecord }> { - return { fileId: await this.putAttachment(data) }; - } // Deliberately lenient for fileIds never actually put (returns empty bytes, // not an error) — a lot of existing tests exercise permission logic with // synthetic fileIds that were never uploaded. Subclass and override this diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 2bbddf5..1185b92 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -517,35 +517,6 @@ export interface StackBlobAdapter { getAttachment(fileId: FileId): Promise; deleteAttachment(fileId: FileId): Promise; - /** - * *Try* to store bytes and create the accompanying _attachment@1 record - * in the same operation — `mimeType`/`filename` are not guaranteed to be - * used. Required on every adapter, but only ever fully honored by one: - * local storage adapters (disk, sqljs, memory) cannot create a record - * themselves (that's the record adapter's job, a different backend - * reached through a different object), so they always store bytes only, - * silently ignore `mimeType`/`filename`, and return `record: undefined`; - * `Stack.putAttachment()` falls back to its own create() call in that - * case, unchanged from before this method existed. Check `record` in the - * result, not the method name, to know whether metadata was actually - * applied. - * - * The API adapter is the one implementation that can genuinely do both in - * one operation — a single POST /attachments request the server fulfills - * atomically — and populates `record`. This is not an efficiency - * shortcut: fileId must be established from bytes the server actually - * received in *this* request, 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()` - * skips its own create() call whenever `record` is present, trusting it - * as authoritative rather than re-validating client-side. - */ - tryPutAttachmentWithMetadata( - data: Uint8Array, - mimeType: string, - filename?: string, - ): Promise<{ fileId: FileId; record?: StackRecord }>; - /** * Enumerate every blob currently in storage. Optional — capability-flagged, * like StackRecordAdapter.deleteUnreferencedAttachmentRecords(). Without it, @@ -568,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 c224dba..3c8fc73 100644 --- a/packages/core/tests/combine.test.ts +++ b/packages/core/tests/combine.test.ts @@ -60,7 +60,6 @@ function makeRecordAdapter(overrides: Partial = {}): StackRe function makeBlobAdapter(overrides: Partial = {}): StackBlobAdapter { return { putAttachment: async () => 'file-id', - tryPutAttachmentWithMetadata: async () => ({ fileId: 'file-id-with-metadata' }), getAttachment: async () => new Uint8Array(), deleteAttachment: async () => {}, ...overrides, @@ -113,22 +112,18 @@ describe('combineAdapters', () => { expect(fileId).toBe('computed-id'); }); - test('forwards tryPutAttachmentWithMetadata to the blob part with all arguments', async () => { - let calledWith: [Uint8Array, string, string | undefined] | undefined; + // 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({ - tryPutAttachmentWithMetadata: async (data, mimeType, filename) => { - calledWith = [data, mimeType, filename]; - return { fileId: 'computed-id' }; - }, - }), + blob: makeBlobAdapter(), }); - - const bytes = new Uint8Array([1, 2, 3]); - const result = await adapter.tryPutAttachmentWithMetadata(bytes, 'image/png', 'photo.png'); - expect(calledWith).toEqual([bytes, 'image/png', 'photo.png']); - expect(result).toEqual({ fileId: 'computed-id' }); + expect('putAttachmentWithMetadata' in adapter).toBe(false); + expect(adapter.putAttachmentWithMetadata).toBeUndefined(); }); // Optional capabilities must round-trip exactly: present when the diff --git a/packages/core/tests/stack.test.ts b/packages/core/tests/stack.test.ts index 3a8c4ed..16ed0cc 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, StackRecord } from '../src/types.js'; +import type { BlobFileInfo, StackAdapter, StackRecord } from '../src/types.js'; // ------------------------------------------------------- // Test setup @@ -1657,17 +1657,18 @@ describe('putAttachment', () => { }); // ------------------------------------------------------- -// putAttachment — atomic path (#106): when the adapter creates the record -// as part of tryPutAttachmentWithMetadata() (the API adapter, over one POST -// /attachments request), Stack.putAttachment() must not also make its own +// 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 return no record, -// so the pre-existing create() fallback is exercised by every other test -// in this describe block above. +// 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('skips the separate create() call when the adapter already created the record', async () => { + test('delegates to putAttachmentWithMetadata() when present, skipping its own create() call', async () => { const data = new Uint8Array([1, 2, 3]); const fabricatedRecord: StackRecord = { id: generateId(), @@ -1677,19 +1678,25 @@ describe('putAttachment — atomic adapter path (#106)', () => { content: { fileId: 'atomic-file-id', mimeType: 'image/png', size: 3, filename: 'photo.png' }, version: 1, }; - vi.spyOn(adapter, 'tryPutAttachmentWithMetadata').mockResolvedValue({ - fileId: 'atomic-file-id', - record: fabricatedRecord, - }); - const createSpy = vi.spyOn(stack, 'create'); + 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 stack.putAttachment(data, 'image/png', 'photo.png'); + 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 returns no record', async () => { + 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'); From 8c2e9128aca89c364721146fcce5cac8e69c44e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 15:00:06 +0000 Subject: [PATCH 7/8] fix(adapter-api): make bytes-only putAttachment() throw instead of minting a default record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /attachments always creates the _attachment@1 record now (#106), so there is no bytes-only wire mode for APIAdapter.putAttachment() to map to. It previously approximated the contract by issuing the combined request and discarding the record — silently creating one with a default application/octet-stream mimeType and no filename, while Stack.putAttachmentBytes() documented 'no record created'. Throw APIAdapterError up front (no request issued) instead: bytes-only upload is a local-storage primitive, and remote callers use Stack.putAttachment(). Spec updated to state the contract plainly in both the Stack-methods list and the wire section, replacing the 'approximate over APIAdapter' caveat. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QQBTuf3CnEwdvH76nvazJF --- docs/spec.md | 4 ++-- packages/adapter-api/src/index.ts | 33 +++++++++++++------------- packages/adapter-api/tests/api.test.ts | 22 ++++++++--------- packages/core/src/stack.ts | 4 ++++ 4 files changed, 34 insertions(+), 29 deletions(-) diff --git a/docs/spec.md b/docs/spec.md index 5222e6a..73644bc 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -709,7 +709,7 @@ 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. +- `Stack.putAttachmentBytes(data)` — owner-level, bytes only. Stores the file and returns its `fileId` without creating an `_attachment@1` record. No grant check. A **local-storage primitive**: over `APIAdapter` it throws, because the wire has no bytes-only upload — `POST /attachments` always creates the record (#106; see [Attachments wire format](#attachments-1)). - `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. @@ -1126,7 +1126,7 @@ Returns `413 Request Entity Too Large` if the payload exceeds the server's confi **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: `StackBlobAdapter.putAttachment()` (the bytes-only primitive behind `Stack.putAttachmentBytes()`) still maps to this same endpoint over the wire, and this endpoint always creates a record now — so `Stack.putAttachmentBytes()`'s documented "no record created" contract holds for local storage but is only approximate over `APIAdapter` (a record with a default `mimeType` is created as a side effect). `Stack.putAttachmentBytes()` remains intended for owner/server-internal use against local storage; remote, non-owner callers should use `Stack.putAttachment()`. +One consequence: there is no longer a bytes-only upload anywhere on the wire — this endpoint always creates a record. So `StackBlobAdapter.putAttachment()` (the bytes-only primitive behind `Stack.putAttachmentBytes()`) is **unsupported over `APIAdapter` and throws** rather than mapping to this endpoint: implementing it anyway would silently create a record with a default `mimeType`, violating `Stack.putAttachmentBytes()`'s documented "no record created" contract while appearing to honor it. Bytes-only upload is a local-storage primitive for owner/server-internal use; remote callers use `Stack.putAttachment()`. **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 683ca25..735d7f5 100644 --- a/packages/adapter-api/src/index.ts +++ b/packages/adapter-api/src/index.ts @@ -326,7 +326,7 @@ export class APIAdapter implements StackAdapter { return new Uint8Array(await res.arrayBuffer()); } - /** POST /attachments always returns the created _attachment@1 record (#106) — see putAttachment()/putAttachmentWithMetadata() below. */ + /** POST /attachments always returns the created _attachment@1 record (#106) — see putAttachmentWithMetadata() below. */ private async uploadBinary( path: string, data: Uint8Array, @@ -569,23 +569,24 @@ export class APIAdapter implements StackAdapter { // ------------------------------------------------------- /** - * Bytes only, per the StackBlobAdapter contract — but POST /attachments - * always creates the accompanying _attachment@1 record now (#106): there - * is no bytes-only wire mode, since the whole point of this endpoint is - * that the record's fileId must come from bytes the server received in - * the same request. This call still ends up creating a record, with a - * default mimeType (application/octet-stream) and no filename, as a side - * effect — the same request putAttachmentWithMetadata() makes, just with - * the record discarded to satisfy this method's narrower return type. - * Stack.putAttachmentBytes()'s documented "no record created" contract - * holds for local storage adapters; over this adapter it's approximate. - * Stack.putAttachmentBytes() remains intended for owner/server-internal - * use (spec §Attachments) — remote, non-owner callers should use + * 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 — violating Stack.putAttachmentBytes()'s + * documented "no record created" contract while appearing to honor it. + * Throwing keeps the contract honest: bytes-only upload is a local + * storage primitive (spec §Attachments), and remote callers use * putAttachmentWithMetadata() (Stack.putAttachment()) instead. */ - async putAttachment(data: Uint8Array): Promise { - const record = await this.putAttachmentWithMetadata(data, 'application/octet-stream'); - return record.content.fileId as string; + 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?) ' + + 'instead of Stack.putAttachmentBytes(data).', + ); } /** diff --git a/packages/adapter-api/tests/api.test.ts b/packages/adapter-api/tests/api.test.ts index c10cebc..0072b53 100644 --- a/packages/adapter-api/tests/api.test.ts +++ b/packages/adapter-api/tests/api.test.ts @@ -789,18 +789,18 @@ const attachmentRecordResponse = (overrides: Partial> = }); describe('putAttachment', () => { - test('sends POST /attachments with binary body and Content-Type: application/octet-stream, returns fileId from the created record', async () => { - const adapter = await openAdapter(); - mockFetch.mockResolvedValueOnce(jsonResponse(attachmentRecordResponse())); + // 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.mockClear(); const data = new Uint8Array([0x89, 0x50, 0x4e, 0x47]); - const fileId = await adapter.putAttachment(data); - expect(fileId).toBe('file-xyz'); - 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', - ); + await expect(adapter.putAttachment(data)).rejects.toThrow(APIAdapterError); + await expect(adapter.putAttachment(data)).rejects.toThrow(/not supported over the wire/); + expect(mockFetch).not.toHaveBeenCalled(); }); }); diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index 96fb574..7b6ca25 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -1244,6 +1244,10 @@ 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. + * + * A local-storage primitive: over the API adapter this throws, because + * the wire has no bytes-only upload — POST /attachments always creates + * the record (#106). Remote callers use putAttachment(). */ async putAttachmentBytes(data: Uint8Array): Promise { return this.adapter.putAttachment(data); From 428224e29804d6ab50f81fbbba262e662430969f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 13:17:22 +0000 Subject: [PATCH 8/8] refactor(core): remove putAttachmentBytes() from the public API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wire lost its bytes-only upload mode in #106 — POST /attachments always creates the _attachment@1 record — which emptied the public bytes-only method's constituency: its only wire consumer was the old bytes-only endpoint, non-owner bytes-only upload was already a documented dead end, and owner bytes-only upload just manufactures the bare-bytes orphans the GC sweep exists to clean up. A public method whose docs warn most callers away and which throws on the most common remote deployment is a landmine, not an API. Remove Stack.putAttachmentBytes(), ScopedStack.putAttachmentBytes(), and the StackClient entry. The upload story is now one sentence: putAttachment(data, mimeType, filename?) is the upload operation, everywhere, for everyone. StackBlobAdapter.putAttachment() stays required at the adapter layer — it's the storage primitive Stack.putAttachment()'s fallback writes through, and within blob storage every implementation can store bytes. APIAdapter's throwing implementation is now unreachable from Stack's public surface (the atomic capability always takes precedence there), so it guards direct adapter-level callers only. ScopedStack now receives the adapter from Stack.asEntity() and composes bytes + its own entityId-stamped create() directly; the orphan- manufacturing GC tests write through the adapter, since no Stack method produces bare bytes on purpose; the bytes-only gating tests are dropped as exact duplicates of the putAttachment() suite's coverage. This goes one step past #106's residual decision 2 ("keep putAttachmentBytes for owner/server-internal use") — that resolution predates the wire losing bytes-only upload entirely, which is what left the method without a legitimate public consumer. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QQBTuf3CnEwdvH76nvazJF --- docs/spec.md | 7 +-- packages/adapter-api/src/index.ts | 14 +++--- packages/core/src/stack.ts | 55 ++++++++---------------- packages/core/tests/scoped-stack.test.ts | 52 ---------------------- packages/core/tests/stack.test.ts | 21 +++++---- 5 files changed, 39 insertions(+), 110 deletions(-) diff --git a/docs/spec.md b/docs/spec.md index 73644bc..62e89d9 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -709,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. A **local-storage primitive**: over `APIAdapter` it throws, because the wire has no bytes-only upload — `POST /attachments` always creates the record (#106; see [Attachments wire format](#attachments-1)). -- `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. @@ -749,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. @@ -1126,7 +1123,7 @@ Returns `413 Request Entity Too Large` if the payload exceeds the server's confi **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. So `StackBlobAdapter.putAttachment()` (the bytes-only primitive behind `Stack.putAttachmentBytes()`) is **unsupported over `APIAdapter` and throws** rather than mapping to this endpoint: implementing it anyway would silently create a record with a default `mimeType`, violating `Stack.putAttachmentBytes()`'s documented "no record created" contract while appearing to honor it. Bytes-only upload is a local-storage primitive for owner/server-internal use; remote callers use `Stack.putAttachment()`. +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 735d7f5..8d32030 100644 --- a/packages/adapter-api/src/index.ts +++ b/packages/adapter-api/src/index.ts @@ -575,17 +575,17 @@ export class APIAdapter implements StackAdapter { * 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 — violating Stack.putAttachmentBytes()'s - * documented "no record created" contract while appearing to honor it. - * Throwing keeps the contract honest: bytes-only upload is a local - * storage primitive (spec §Attachments), and remote callers use - * putAttachmentWithMetadata() (Stack.putAttachment()) instead. + * 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?) ' + - 'instead of Stack.putAttachmentBytes(data).', + 'an _attachment@1 record (#106). Use Stack.putAttachment(data, mimeType, filename?).', ); } diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index 7b6ca25..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); } // ------------------------------------------------------- @@ -1240,19 +1239,6 @@ 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. - * - * A local-storage primitive: over the API adapter this throws, because - * the wire has no bytes-only upload — POST /attachments always creates - * the record (#106). Remote callers use putAttachment(). - */ - 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 @@ -1277,7 +1263,7 @@ export class Stack implements StackClient { const record = await this.adapter.putAttachmentWithMetadata(data, mimeType, filename); return (record.content as AttachmentContent).fileId; } - const fileId = await this.putAttachmentBytes(data); + const fileId = await this.adapter.putAttachment(data); await this.create(`${SYSTEM_TYPES.ATTACHMENT}@1`, { fileId, mimeType, @@ -1702,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 { @@ -2165,24 +2159,6 @@ export class ScopedStack implements StackClient { return this.stack.restoreVersion(id, version, opts); } - /** - * 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. - */ - async putAttachmentBytes(data: Uint8Array): Promise { - const requester = this.requesterEntityId; - if (!requester) { - throw new StackPermissionError('Anonymous requesters cannot upload attachments'); - } - 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. Requires a * `create` grant on `_attachment@1`. Anonymous requesters are always @@ -2193,9 +2169,14 @@ export class ScopedStack implements StackClient { * shaped record than Stack.putAttachment() for the exact same author. */ 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 requester = this.requesterEntityId; + if (!requester) { + throw new StackPermissionError('Anonymous requesters cannot upload attachments'); + } + if (!(await this.checkCreateGrant(`${SYSTEM_TYPES.ATTACHMENT}@1`))) { + throw new StackPermissionError(`No create grant for type "${SYSTEM_TYPES.ATTACHMENT}@1"`); + } + const fileId = await this.adapter.putAttachment(data); const isOwner = requester === this.stack.ownerEntityId; await this.stack.create( `${SYSTEM_TYPES.ATTACHMENT}@1`, diff --git a/packages/core/tests/scoped-stack.test.ts b/packages/core/tests/scoped-stack.test.ts index c9f8077..553d891 100644 --- a/packages/core/tests/scoped-stack.test.ts +++ b/packages/core/tests/scoped-stack.test.ts @@ -794,58 +794,6 @@ describe('ScopedStack.putAttachment', () => { }); }); -// ------------------------------------------------------- -// 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); - 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' }); - }); -}); - // ------------------------------------------------------- // ScopedStack.getAttachment — owner, referencing-record, and uploader access // ------------------------------------------------------- diff --git a/packages/core/tests/stack.test.ts b/packages/core/tests/stack.test.ts index 16ed0cc..e82f549 100644 --- a/packages/core/tests/stack.test.ts +++ b/packages/core/tests/stack.test.ts @@ -1778,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' }, @@ -2131,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 }); @@ -2161,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 });