From 03f6a78daa3f9d46f0d1c9227ed5ffef3bb72fea Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 02:54:01 +0000 Subject: [PATCH 1/5] 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/5] 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/5] 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/5] 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/5] 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, });