diff --git a/.changeset/27598-presigned-size-whole-non-negative.md b/.changeset/27598-presigned-size-whole-non-negative.md new file mode 100644 index 00000000000..89756a3d5f2 --- /dev/null +++ b/.changeset/27598-presigned-size-whole-non-negative.md @@ -0,0 +1,20 @@ +--- +'@objectstack/spec': patch +'@objectstack/service-storage': patch +--- + +fix(spec)!: `GetPresignedUrlRequestSchema.size` declares a whole, non-negative byte count, and `filename` / `mimeType` declare that they are non-empty, the accept set the presigned-upload door enforces (#27598) + +Clause-②: no (narrowing) — the published declaration of the presigned-upload request body refuses a negative or fractional `size` and an empty `filename` or `mimeType`, which it admitted before. Nothing is widened and no key is added, removed or renamed. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing of a published request declaration (`@objectstack/spec/api`), read by the `POST /api/v1/storage/upload/presigned` door, codegen, the reference docs and clients. + +- **Before.** `size` was any number, and `filename` / `mimeType` were bare strings, so `-1`, `1.5` and `''` parsed. The door refused a negative size and an empty name or type with a 400, but accepted a fractional size: it minted an upload URL and a pending file record that no upload could complete, because the complete door compares the held byte count, always whole, with the declared one. +- **Now.** `size` is `int().min(0)`, and `filename` / `mimeType` carry a minimum length of 1; their descriptions say so. A parse of any of those values fails on that key. The door (`@objectstack/service-storage`) also refuses a fractional `size` with `400 INVALID_REQUEST`, before any record is written, with the message `size must be the file size in bytes: a non-negative whole number, sent as a JSON number`. +- **Unchanged.** A negative, non-numeric, `NaN` or infinite size, and an empty or missing `filename` or `mimeType`, get the same 400 as before, in the same order as before. `0` is still a legal empty file. The SDK's `storage.getPresignedUrl` takes this schema's input type, which stays `number` / `string`. +- **Who sees it.** Code that parses a presigned-upload body with this schema and sent one of the refused values, a generated client that trusted the wider declaration, and a caller that sent a fractional size to the door. + +**The remedy.** Send the file's length in whole bytes (`File.size` already is), and its name and MIME type; an unknown type is sent as `application/octet-stream`. diff --git a/content/docs/references/api/storage.mdx b/content/docs/references/api/storage.mdx index 8259fd190cc..f34c17c4b44 100644 --- a/content/docs/references/api/storage.mdx +++ b/content/docs/references/api/storage.mdx @@ -221,9 +221,9 @@ const result = CompleteChunkedUploadRequestSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **filename** | `string` | ✅ | Original filename | -| **mimeType** | `string` | ✅ | File MIME type | -| **size** | `number` | ✅ | File size in bytes | +| **filename** | `string` | ✅ | Original filename (non-empty) | +| **mimeType** | `string` | ✅ | File MIME type (non-empty) | +| **size** | `integer` | ✅ | File size in bytes: a non-negative whole number (0 is an empty file) | | **scope** | `Enum<'user' \| 'tenant' \| 'private' \| 'temp' \| 'attachments'>` | optional (default: `"user"`) | Storage scope the new file is filed under (default user; attachments for the record attachments surface). Not an access setting: the upload doors refuse public, and a file is served without sign-in only when its stored file record carries acl 'public_read' (ADR-0104). The upload request carries no acl; every upload is stored private. | | **bucket** | `string` | optional | Specific bucket override (admin only) | diff --git a/packages/services/service-storage/src/storage-routes.test.ts b/packages/services/service-storage/src/storage-routes.test.ts index 02059604545..f6e26cc0afc 100644 --- a/packages/services/service-storage/src/storage-routes.test.ts +++ b/packages/services/service-storage/src/storage-routes.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts'; -import { InitiateChunkedUploadRequestSchema } from '@objectstack/spec/api'; +import { GetPresignedUrlRequestSchema, InitiateChunkedUploadRequestSchema } from '@objectstack/spec/api'; import { LocalStorageAdapter } from './local-storage-adapter.js'; import { StorageMetadataStore } from './metadata-store.js'; import { registerStorageRoutes } from './storage-routes.js'; @@ -130,28 +130,55 @@ describe('Storage REST Routes', () => { ['a numeric string', '100'], ['a negative number', -5], ['a negative fraction', -0.5], + // [#27598] A byte count is whole: a fractional size minted a URL and a + // pending row that no upload could ever complete (the held byte count + // never equals it). + ['a positive fraction', 1.5], ['NaN', Number.NaN], ['Infinity', Number.POSITIVE_INFINITY], ['a boolean', true], ])('REFUSED 400 INVALID_REQUEST: size as %s, with no sys_file row written', async (_label, size) => { const handler = httpServer._getHandler('POST', '/api/v1/storage/upload/presigned')!; const createFile = vi.spyOn(store, 'createFile'); + const body = { filename: 'a.png', mimeType: 'image/png', size }; const res = createMockRes(); - await handler(createMockReq({ body: { filename: 'a.png', mimeType: 'image/png', size } }), res); + await handler(createMockReq({ body }), res); expect(res._status, JSON.stringify(res._json)).toBe(400); expect(res._json.success).toBe(false); expect(res._json.error.code).toBe('INVALID_REQUEST'); expect(res._json.error.message).toMatch(/^size must be the file size in bytes/); + expect(GetPresignedUrlRequestSchema.safeParse(body).success, 'the declaration refuses it').toBe(false); expect(createFile, 'no pending sys_file row').not.toHaveBeenCalled(); }); + // [#27598] The door's presence check refuses an empty `filename` or + // `mimeType`; the declaration says the same. + it.each(['filename', 'mimeType'] as const)( + 'REFUSED 400 INVALID_REQUEST: an empty %s, which the declared schema refuses too, with no row written', + async (key) => { + const handler = httpServer._getHandler('POST', '/api/v1/storage/upload/presigned')!; + const createFile = vi.spyOn(store, 'createFile'); + const body = { filename: 'a.png', mimeType: 'image/png', size: 1, [key]: '' }; + const res = createMockRes(); + await handler(createMockReq({ body }), res); + + expect(res._status, JSON.stringify(res._json)).toBe(400); + expect(res._json.success).toBe(false); + expect(res._json.error.code).toBe('INVALID_REQUEST'); + expect(GetPresignedUrlRequestSchema.safeParse(body).success, 'the declaration refuses it').toBe(false); + expect(createFile, 'no pending sys_file row').not.toHaveBeenCalled(); + }, + ); + it.each([0, 1024])('CONTROL: size %d is accepted and stored on the pending row as sent', async (size) => { const handler = httpServer._getHandler('POST', '/api/v1/storage/upload/presigned')!; + const body = { filename: 'a.png', mimeType: 'image/png', size }; const res = createMockRes(); - await handler(createMockReq({ body: { filename: 'a.png', mimeType: 'image/png', size } }), res); + await handler(createMockReq({ body }), res); expect(res._status, JSON.stringify(res._json)).toBe(200); + expect(GetPresignedUrlRequestSchema.safeParse(body).success, 'the declaration accepts it').toBe(true); const row = await store.getFile(res._json.data.fileId); expect(row!.size).toBe(size); expect(row!.status).toBe('pending'); diff --git a/packages/services/service-storage/src/storage-routes.ts b/packages/services/service-storage/src/storage-routes.ts index 8fea7953ad7..90cd9a7c352 100644 --- a/packages/services/service-storage/src/storage-routes.ts +++ b/packages/services/service-storage/src/storage-routes.ts @@ -162,19 +162,21 @@ function uploadScopeRefusalMessage(scope: unknown): string { } /** - * [#27413] The presigned door's declared `size`: a finite, non-negative JSON - * number, the byte count `GetPresignedUrlRequestSchema.size` declares. A - * string (even a numeric one), `NaN`, an infinity or a negative count is - * refused `400` / `INVALID_REQUEST` before the pending `sys_file` row is - * written — the code the door already answers a request missing its size - * with. `0` is a legal empty file. + * [#27413] The presigned door's declared `size`: a whole, non-negative JSON + * number, the byte count `GetPresignedUrlRequestSchema.size` declares + * (`int().min(0)`). A string (even a numeric one), `NaN`, an infinity, a + * negative count or [#27598] a fraction is refused `400` / `INVALID_REQUEST` + * before the pending `sys_file` row is written — the code the door already + * answers a request missing its size with. A fraction used to mint a URL and + * a pending row no upload could complete: the held byte count is whole, so + * the complete door's size check never matched it. `0` is a legal empty file. */ function isDeclaredByteCount(size: unknown): size is number { - return typeof size === 'number' && Number.isFinite(size) && size >= 0; + return typeof size === 'number' && Number.isInteger(size) && size >= 0; } const PRESIGNED_SIZE_REFUSAL_MESSAGE = - 'size must be the file size in bytes: a non-negative number, sent as a JSON number'; + 'size must be the file size in bytes: a non-negative whole number, sent as a JSON number'; /** * [#27549] The chunked-upload init door's refusal for a body that does not diff --git a/packages/spec/src/api/storage.test.ts b/packages/spec/src/api/storage.test.ts index a28f2f68dc3..a76f7e7d455 100644 --- a/packages/spec/src/api/storage.test.ts +++ b/packages/spec/src/api/storage.test.ts @@ -78,6 +78,38 @@ describe('GetPresignedUrlRequestSchema', () => { }) ).toThrow(); }); + + // [#27598] `size` is a whole, non-negative byte count, and `filename` / + // `mimeType` are non-empty: what the presigned door refuses, declared. + it.each<[string, number]>([ + ['a negative size', -1], + ['a fractional size', 1.5], + ['a negative fraction', -0.5], + ])('refuses %s, naming size', (_label, size) => { + const result = GetPresignedUrlRequestSchema.safeParse({ filename: 'a.png', mimeType: 'image/png', size }); + expect(result.success).toBe(false); + expect(result.error!.issues).toHaveLength(1); + expect(result.error!.issues[0].path).toEqual(['size']); + }); + + it.each(['filename', 'mimeType'] as const)('refuses an empty %s, naming that key', (key) => { + const result = GetPresignedUrlRequestSchema.safeParse({ + filename: 'a.png', + mimeType: 'image/png', + size: 1, + [key]: '', + }); + expect(result.success).toBe(false); + expect(result.error!.issues).toHaveLength(1); + expect(result.error!.issues[0].path).toEqual([key]); + expect(result.error!.issues[0].code).toBe('too_small'); + }); + + it.each([0, 1, 1048576])('CONTROL: size %d parses (0 is an empty file)', (size) => { + const result = GetPresignedUrlRequestSchema.safeParse({ filename: 'a', mimeType: 'b', size }); + expect(result.success).toBe(true); + expect(result.data!.size).toBe(size); + }); }); describe('CompleteUploadRequestSchema', () => { diff --git a/packages/spec/src/api/storage.zod.ts b/packages/spec/src/api/storage.zod.ts index e4b1a731384..8e3603c8edd 100644 --- a/packages/spec/src/api/storage.zod.ts +++ b/packages/spec/src/api/storage.zod.ts @@ -64,9 +64,12 @@ const UPLOAD_SCOPE_DESCRIPTION = + 'carries no acl; every upload is stored private.'; export const GetPresignedUrlRequestSchema = lazySchema(() => z.object({ - filename: z.string().describe('Original filename'), - mimeType: z.string().describe('File MIME type'), - size: z.number().describe('File size in bytes'), + // [#27598] What the presigned door refuses, declared: an empty `filename` or + // `mimeType` (400 INVALID_REQUEST, as the chunked-init sibling declares + // since #27801), and a `size` that is not a whole, non-negative byte count. + filename: z.string().min(1).describe('Original filename (non-empty)'), + mimeType: z.string().min(1).describe('File MIME type (non-empty)'), + size: z.number().int().min(0).describe('File size in bytes: a non-negative whole number (0 is an empty file)'), scope: UploadScopeSchema.default('user').describe(UPLOAD_SCOPE_DESCRIPTION), bucket: z.string().optional().describe('Specific bucket override (admin only)'), }));