Skip to content

Commit 2c683cf

Browse files
fix(spec)!: the presigned upload request declares a whole, non-negative size and a non-empty filename and mimeType (#28136)
domain:spec#1 · os-tesla · session_01L9qibwb6hBqszPTYJobitm <!-- claude-projects-attribution: {"github_login":"os-tesla"} --> _Requested by **Tesla** · [project thread](https://claude.ai/code/project/chan_01AXsQkLFKRdQCqQ3RTr6Jao?thread=cmsg_01AXsQkLFKRdQCqQ3RTr6Jao9ZLkgazHGbBtMatjZpT18B)_ Fixes #27598 Clause-②: no (narrowing) Metadata-protocol: none ## 维护者速读 - 改了什么:预签名上传请求的 schema(`GetPresignedUrlRequestSchema`)现在声明 `size` 必须是非负整数(`int().min(0)`),`filename`、`mimeType` 不能为空(`min(1)`);上传入口同时开始拒收小数字节数(400 INVALID_REQUEST)。 - 为什么改:公开 schema 原来接受负数、小数和空字符串,入口却拒收负数和空值,按 schema 写代码的调用方或 AI 会被误导;小数字节数更糟,入口会发出上传地址并写一条永远无法完成的待上传记录。 - 风险与回滚:负数、非数字、空文件名/类型的入口行为一字未改;唯一的行为变化是小数 `size` 从 200 变成 400,正常客户端发的 `File.size` 本来就是整数。回滚即 revert 本 PR。 - 先看:`packages/spec/src/api/storage.zod.ts`(三处收窄)、`packages/services/service-storage/src/storage-routes.ts`(`isDeclaredByteCount` 改用 `Number.isInteger`)、`.changeset/27598-presigned-size-whole-non-negative.md`(BREAKING 说明与 ADR-0087 处置)。 - 两分钟验证:`pnpm --filter @objectstack/spec exec vitest run src/api/storage.test.ts`,预期 73 个测试全过,其中包括 “refuses a fractional size, naming size”。 - 需要你做的:无,席位落地(等契约复审记录)。 ## What changes - `GetPresignedUrlRequestSchema.size` is `z.number().int().min(0)`, and its description reads "File size in bytes: a non-negative whole number (0 is an empty file)". The published JSON Schema now types it `integer` with `minimum: 0`. - **Declared increment (same file, same declared-versus-enforced class, the #27887 precedent):** `GetPresignedUrlRequestSchema.filename` and `.mimeType` declare `min(1)`, described "(non-empty)". The order allowed this only if the door refuses `''` today. Measured on `main` `2e955dde`: the door's presence check `!filename || !mimeType || size == null` (`storage-routes.ts:968`) answers `400 INVALID_REQUEST` for an empty value of either key. The new door test below pins that reading. - **Door (`POST /api/v1/storage/upload/presigned`):** `isDeclaredByteCount` asks `Number.isInteger(size)` in place of `Number.isFinite(size)`, so a fractional `size` is refused `400 INVALID_REQUEST` before any `sys_file` row is written. The refusal message now says "a non-negative whole number". Measured on `main` `2e955dde`: `size: 1.5` answered `200`, minted an upload URL and wrote a pending row. That row could never commit, because the complete door compares the held byte count (always whole) with the declared one (`heldBytesRefusal`, `storage-routes.ts`). Without this change the narrowed declaration would refuse a value the door still accepts, which is the same declared-versus-enforced gap in the other direction. - `content/docs/references/api/storage.mdx` is regenerated (3 lines). - Changeset `27598-presigned-size-whole-non-negative.md`: `@objectstack/spec` patch and `@objectstack/service-storage` patch (the declared kind is `none`: a REST request declaration that no metadata type embeds is not authorable metadata, the seat's claim correction 6524182832 on #27598), `fix(spec)!`, both declaration lines, and the ADR-0087 disposition `not-required (no-migration-prescription)`. The door still does not parse its body with the schema. Its presence check and byte-count check run before the scope gate, as #22470 requires, so status, code, message and ordering stay as they were for every value other than a fraction. ## Tests - `packages/spec/src/api/storage.test.ts`: - `refuses %s, naming size` covers a negative size, a fractional size and a negative fraction. Each asserts `success: false`, exactly one issue, and `path` equal to `['size']`. - `refuses an empty %s, naming that key` covers `filename` and `mimeType`, with `code` `too_small`. - `CONTROL: size %d parses (0 is an empty file)` covers `0`, `1` and `1048576`. - `packages/services/service-storage/src/storage-routes.test.ts`: - The #27413 refusal table gains `a positive fraction` (`1.5`). Every row now also asserts that `GetPresignedUrlRequestSchema.safeParse` refuses the same body. - `REFUSED 400 INVALID_REQUEST: an empty %s, which the declared schema refuses too, with no row written` covers `filename` and `mimeType` on this door. No earlier test sent an empty name or type here. - The `CONTROL` rows (`0`, `1024`) assert that the declaration accepts the body too. <details><summary>Ablations: each narrowing turns its own case red</summary> Each run used `node scripts/ablation-replace.mjs`. The anchor hit once, and the file was restored to its HEAD blob with `git diff HEAD` empty. - `size: z.number().int().min(0)` changed to `z.number().min(0)`: `× refuses a fractional size, naming size`, 1 failed / 72 passed (73). - `.min(0)` removed from `size`: `× refuses a negative size, naming size`, 1 failed / 72 passed (73). The negative fraction stays refused by `int()`. - `filename` `min(1)` removed: `× refuses an empty filename, naming that key`, 1 failed / 72 passed (73). - `mimeType` `min(1)` removed: `× refuses an empty mimeType, naming that key`, 1 failed / 72 passed (73). - Door `Number.isInteger(size)` changed back to `Number.isFinite(size)`: `× REFUSED 400 INVALID_REQUEST: size as a positive fraction, with no sys_file row written`, 1 failed / 74 passed (75). Before the door change, the same case failed with the door's `200` body, which is the measurement of `main` quoted above. </details> <details><summary>Local runs</summary> The package runs below were taken on `bbd24a5f` / `6e328c49` (before the merge with `origin/main`). The storage suites were re-run on the final head `902d10c0`: spec `storage.test.ts` Tests 73 passed (73); `@objectstack/service-storage` Test Files 71 passed (71), Tests 1314 passed (1314). - `pnpm --filter @objectstack/spec test`: Test Files 756 passed (756), Tests 22518 passed, 1 todo. `pnpm --filter @objectstack/spec typecheck`: exit 0. - `pnpm --filter @objectstack/service-storage test`: Test Files 71 passed (71), Tests 1314 passed (1314). `pnpm --filter @objectstack/service-storage typecheck`: exit 0, and `check:test-typecheck` OK. - `pnpm --filter @objectstack/spec check:generated`: 1 of 14 stale (`content/docs/references/**`) before `--fix`. After `--fix` all 14 are up to date. - `node scripts/check-adr-0087-registration.mjs`: 1 declared-breaking changeset, carrying `not-required (no-migration-prescription)`. </details> ## Acceptance notes - **Out of scope, reported to the seat and not filed:** the SDK's `storage.upload` (`packages/client/src/index.ts`) sends `mimeType: file.type`. A browser `File` of unknown type has `type === ''`, and the presigned door refuses that with `400 INVALID_REQUEST`, so such a file cannot be uploaded through the SDK helper. This PR now also declares that refusal. The fix belongs in the client (send `application/octet-stream`), not in the door. Not measured end to end in a browser. - A whitespace-only `filename` / `mimeType` is still admitted by both the schema and the door. The two agree, and this PR leaves that alone, as #27887 did. - The claim writes `Clause-②: no`. The order and triage `6500594085` write `no (narrowing)`, and `check-changeset-no-major` treats `narrowed` beside a line without the `(narrowing)` arm as a contradiction. This body and the changeset carry `no (narrowing)`. ## Verification All readings below are on head `902d10c0`, the branch merged with `origin/main` at `8cb8c7f461`, after a full `pnpm turbo run build` (116/116 tasks). - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 109 commands (the same 109 before and after the merge). All 109 were run with stdin from /dev/null and the exit code captured before any pipe: 109 exited 0. On the pre-merge head, 4 first refused with exit 3 (PREREQUISITE NOT MET: no workspace build, and one self-test that needs an unshallow clone); after the build and `git fetch --unshallow` they exited 0. - `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran FILE`: "109 derived famil(ies) accounted for — 109 run, 0 NOT-MEASURED". - `pnpm --filter @objectstack/spec check:generated` on the merged head: all 14 generated artefacts up to date. - Full dogfood (owed by the `(narrowing)` arm), `pnpm --filter @objectstack/dogfood test` under the verify lock on `902d10c0`: Test Files 1 failed | 357 passed | 3 skipped (361), Tests 1 failed | 2882 passed | 20 skipped (2903). The one failure is not this change. `test/api-action-door-forward.dogfood.test.ts` > "the switch holds: ban_user OFF answers 409 ACTION_DISABLED through the door and the user is NOT banned" expects the `ACTION_DISABLED` message to contain `sys_metadata_activation`, and the message is now the plain sentence "This action is turned off for this deployment. ..." (`packages/objectql/src/action-activation.ts`). Both the test and the new sentence are already on `origin/main`, and this PR touches neither file nor anything they import from storage. Reported to the seat as a main-red candidate. - CI on this PR: not yet read; the seat reads the check-runs. --- _Generated by [Claude Code](https://claude.ai/code/session_01L9qibvsQx1XxK9FrejoXno)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 10fc9bf commit 2c683cf

6 files changed

Lines changed: 101 additions & 17 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
'@objectstack/spec': patch
3+
'@objectstack/service-storage': patch
4+
---
5+
6+
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)
7+
8+
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.
9+
Metadata-protocol: none
10+
11+
<!-- 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. -->
12+
13+
**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.
14+
15+
- **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.
16+
- **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`.
17+
- **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`.
18+
- **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.
19+
20+
**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`.

‎content/docs/references/api/storage.mdx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,9 @@ const result = CompleteChunkedUploadRequestSchema.parse(data);
221221

222222
| Property | Type | Required | Description |
223223
| :--- | :--- | :--- | :--- |
224-
| **filename** | `string` | ✅ | Original filename |
225-
| **mimeType** | `string` | ✅ | File MIME type |
226-
| **size** | `number` | ✅ | File size in bytes |
224+
| **filename** | `string` | ✅ | Original filename (non-empty) |
225+
| **mimeType** | `string` | ✅ | File MIME type (non-empty) |
226+
| **size** | `integer` | ✅ | File size in bytes: a non-negative whole number (0 is an empty file) |
227227
| **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. |
228228
| **bucket** | `string` | optional | Specific bucket override (admin only) |
229229

‎packages/services/service-storage/src/storage-routes.test.ts‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
44
import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
5-
import { InitiateChunkedUploadRequestSchema } from '@objectstack/spec/api';
5+
import { GetPresignedUrlRequestSchema, InitiateChunkedUploadRequestSchema } from '@objectstack/spec/api';
66
import { LocalStorageAdapter } from './local-storage-adapter.js';
77
import { StorageMetadataStore } from './metadata-store.js';
88
import { registerStorageRoutes } from './storage-routes.js';
@@ -130,28 +130,55 @@ describe('Storage REST Routes', () => {
130130
['a numeric string', '100'],
131131
['a negative number', -5],
132132
['a negative fraction', -0.5],
133+
// [#27598] A byte count is whole: a fractional size minted a URL and a
134+
// pending row that no upload could ever complete (the held byte count
135+
// never equals it).
136+
['a positive fraction', 1.5],
133137
['NaN', Number.NaN],
134138
['Infinity', Number.POSITIVE_INFINITY],
135139
['a boolean', true],
136140
])('REFUSED 400 INVALID_REQUEST: size as %s, with no sys_file row written', async (_label, size) => {
137141
const handler = httpServer._getHandler('POST', '/api/v1/storage/upload/presigned')!;
138142
const createFile = vi.spyOn(store, 'createFile');
143+
const body = { filename: 'a.png', mimeType: 'image/png', size };
139144
const res = createMockRes();
140-
await handler(createMockReq({ body: { filename: 'a.png', mimeType: 'image/png', size } }), res);
145+
await handler(createMockReq({ body }), res);
141146

142147
expect(res._status, JSON.stringify(res._json)).toBe(400);
143148
expect(res._json.success).toBe(false);
144149
expect(res._json.error.code).toBe('INVALID_REQUEST');
145150
expect(res._json.error.message).toMatch(/^size must be the file size in bytes/);
151+
expect(GetPresignedUrlRequestSchema.safeParse(body).success, 'the declaration refuses it').toBe(false);
146152
expect(createFile, 'no pending sys_file row').not.toHaveBeenCalled();
147153
});
148154

155+
// [#27598] The door's presence check refuses an empty `filename` or
156+
// `mimeType`; the declaration says the same.
157+
it.each(['filename', 'mimeType'] as const)(
158+
'REFUSED 400 INVALID_REQUEST: an empty %s, which the declared schema refuses too, with no row written',
159+
async (key) => {
160+
const handler = httpServer._getHandler('POST', '/api/v1/storage/upload/presigned')!;
161+
const createFile = vi.spyOn(store, 'createFile');
162+
const body = { filename: 'a.png', mimeType: 'image/png', size: 1, [key]: '' };
163+
const res = createMockRes();
164+
await handler(createMockReq({ body }), res);
165+
166+
expect(res._status, JSON.stringify(res._json)).toBe(400);
167+
expect(res._json.success).toBe(false);
168+
expect(res._json.error.code).toBe('INVALID_REQUEST');
169+
expect(GetPresignedUrlRequestSchema.safeParse(body).success, 'the declaration refuses it').toBe(false);
170+
expect(createFile, 'no pending sys_file row').not.toHaveBeenCalled();
171+
},
172+
);
173+
149174
it.each([0, 1024])('CONTROL: size %d is accepted and stored on the pending row as sent', async (size) => {
150175
const handler = httpServer._getHandler('POST', '/api/v1/storage/upload/presigned')!;
176+
const body = { filename: 'a.png', mimeType: 'image/png', size };
151177
const res = createMockRes();
152-
await handler(createMockReq({ body: { filename: 'a.png', mimeType: 'image/png', size } }), res);
178+
await handler(createMockReq({ body }), res);
153179

154180
expect(res._status, JSON.stringify(res._json)).toBe(200);
181+
expect(GetPresignedUrlRequestSchema.safeParse(body).success, 'the declaration accepts it').toBe(true);
155182
const row = await store.getFile(res._json.data.fileId);
156183
expect(row!.size).toBe(size);
157184
expect(row!.status).toBe('pending');

‎packages/services/service-storage/src/storage-routes.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,19 +162,21 @@ function uploadScopeRefusalMessage(scope: unknown): string {
162162
}
163163

164164
/**
165-
* [#27413] The presigned door's declared `size`: a finite, non-negative JSON
166-
* number, the byte count `GetPresignedUrlRequestSchema.size` declares. A
167-
* string (even a numeric one), `NaN`, an infinity or a negative count is
168-
* refused `400` / `INVALID_REQUEST` before the pending `sys_file` row is
169-
* written — the code the door already answers a request missing its size
170-
* with. `0` is a legal empty file.
165+
* [#27413] The presigned door's declared `size`: a whole, non-negative JSON
166+
* number, the byte count `GetPresignedUrlRequestSchema.size` declares
167+
* (`int().min(0)`). A string (even a numeric one), `NaN`, an infinity, a
168+
* negative count or [#27598] a fraction is refused `400` / `INVALID_REQUEST`
169+
* before the pending `sys_file` row is written — the code the door already
170+
* answers a request missing its size with. A fraction used to mint a URL and
171+
* a pending row no upload could complete: the held byte count is whole, so
172+
* the complete door's size check never matched it. `0` is a legal empty file.
171173
*/
172174
function isDeclaredByteCount(size: unknown): size is number {
173-
return typeof size === 'number' && Number.isFinite(size) && size >= 0;
175+
return typeof size === 'number' && Number.isInteger(size) && size >= 0;
174176
}
175177

176178
const PRESIGNED_SIZE_REFUSAL_MESSAGE =
177-
'size must be the file size in bytes: a non-negative number, sent as a JSON number';
179+
'size must be the file size in bytes: a non-negative whole number, sent as a JSON number';
178180

179181
/**
180182
* [#27549] The chunked-upload init door's refusal for a body that does not

‎packages/spec/src/api/storage.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,38 @@ describe('GetPresignedUrlRequestSchema', () => {
7878
})
7979
).toThrow();
8080
});
81+
82+
// [#27598] `size` is a whole, non-negative byte count, and `filename` /
83+
// `mimeType` are non-empty: what the presigned door refuses, declared.
84+
it.each<[string, number]>([
85+
['a negative size', -1],
86+
['a fractional size', 1.5],
87+
['a negative fraction', -0.5],
88+
])('refuses %s, naming size', (_label, size) => {
89+
const result = GetPresignedUrlRequestSchema.safeParse({ filename: 'a.png', mimeType: 'image/png', size });
90+
expect(result.success).toBe(false);
91+
expect(result.error!.issues).toHaveLength(1);
92+
expect(result.error!.issues[0].path).toEqual(['size']);
93+
});
94+
95+
it.each(['filename', 'mimeType'] as const)('refuses an empty %s, naming that key', (key) => {
96+
const result = GetPresignedUrlRequestSchema.safeParse({
97+
filename: 'a.png',
98+
mimeType: 'image/png',
99+
size: 1,
100+
[key]: '',
101+
});
102+
expect(result.success).toBe(false);
103+
expect(result.error!.issues).toHaveLength(1);
104+
expect(result.error!.issues[0].path).toEqual([key]);
105+
expect(result.error!.issues[0].code).toBe('too_small');
106+
});
107+
108+
it.each([0, 1, 1048576])('CONTROL: size %d parses (0 is an empty file)', (size) => {
109+
const result = GetPresignedUrlRequestSchema.safeParse({ filename: 'a', mimeType: 'b', size });
110+
expect(result.success).toBe(true);
111+
expect(result.data!.size).toBe(size);
112+
});
81113
});
82114

83115
describe('CompleteUploadRequestSchema', () => {

‎packages/spec/src/api/storage.zod.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,12 @@ const UPLOAD_SCOPE_DESCRIPTION =
6464
+ 'carries no acl; every upload is stored private.';
6565

6666
export const GetPresignedUrlRequestSchema = lazySchema(() => z.object({
67-
filename: z.string().describe('Original filename'),
68-
mimeType: z.string().describe('File MIME type'),
69-
size: z.number().describe('File size in bytes'),
67+
// [#27598] What the presigned door refuses, declared: an empty `filename` or
68+
// `mimeType` (400 INVALID_REQUEST, as the chunked-init sibling declares
69+
// since #27801), and a `size` that is not a whole, non-negative byte count.
70+
filename: z.string().min(1).describe('Original filename (non-empty)'),
71+
mimeType: z.string().min(1).describe('File MIME type (non-empty)'),
72+
size: z.number().int().min(0).describe('File size in bytes: a non-negative whole number (0 is an empty file)'),
7073
scope: UploadScopeSchema.default('user').describe(UPLOAD_SCOPE_DESCRIPTION),
7174
bucket: z.string().optional().describe('Specific bucket override (admin only)'),
7275
}));

0 commit comments

Comments
 (0)