Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/27598-presigned-size-whole-non-negative.md
Original file line number Diff line number Diff line change
@@ -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

<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped and no metadata type or stored shape moves: a REST request declaration no metadata type embeds stops admitting a negative or fractional `size` and an empty `filename` or `mimeType`. The door already refused all but the fraction, and a fractional size never named a byte count a stored file can hold. A caller sends the file's whole byte length, name and MIME type; there is nothing for a migration to rewrite and no tombstone. -->

**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`.
6 changes: 3 additions & 3 deletions content/docs/references/api/storage.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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) |

Expand Down
33 changes: 30 additions & 3 deletions packages/services/service-storage/src/storage-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');
Expand Down
18 changes: 10 additions & 8 deletions packages/services/service-storage/src/storage-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions packages/spec/src/api/storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
9 changes: 6 additions & 3 deletions packages/spec/src/api/storage.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)'),
}));
Expand Down
Loading