Skip to content

Commit a68430a

Browse files
fix(spec)!: the chunked-upload init request declares filename and mimeType non-empty (#27887)
<!-- claude-projects-attribution: {"github_login":"marchtian"} --> _Requested by **田騏** · [project thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSiEosMC3zDfHaauURHFNq1GJ)_ Fixes #27801 Clause-②: no (narrowing) Metadata-protocol: none ## 维护者速读 - 改了什么:分块上传初始化请求的 schema(`InitiateChunkedUploadRequestSchema`)现在声明 `filename`、`mimeType` 不能为空(`min(1)`),参考文档同步标注“non-empty”。 - 为什么改:上传入口一直拒收空文件名、空 MIME 类型(400 INVALID_REQUEST),但公开 schema 写的是“任意字符串”,按 schema 写代码的调用方或 AI 会被误导;现在声明与执行一致。 - 风险与回滚:上传入口的行为一字未改(状态码、错误码、文案、与 scope 检查的先后都不变),只有直接用这个 schema 解析空值的代码会从通过变成失败;回滚即 revert 本 PR。 - 先看:`packages/spec/src/api/storage.zod.ts`(两个 `min(1)`)、`packages/services/service-storage/src/storage-routes.ts`(入口的存在性检查为何保留的注释)、`.changeset/27801-chunked-init-filename-min1.md`(BREAKING 说明与 ADR-0087 处置)。 - 两分钟验证:`pnpm --filter @objectstack/spec exec vitest run src/api/storage.test.ts`,预期 65 个测试全过,其中包括“refuses an empty filename / mimeType”两条。 - 需要你做的:无,席位落地(等契约复审记录)。 Independent review: none run (quota); the PR waits for the seat's contract-review hand-off. ## What changes - `InitiateChunkedUploadRequestSchema.filename` and `.mimeType` declare `min(1)`, and their descriptions read "Original filename (non-empty)" and "File MIME type (non-empty)". The published JSON Schema now carries `minLength: 1` on both keys, and `content/docs/references/api/storage.mdx` is regenerated (2 lines). - The door (`POST /api/v1/storage/upload/chunked`) is unchanged in behaviour. Its presence check `!body.filename || !body.mimeType || body.totalSize == null` stays where it is. A comment now says that it refuses nothing the schema parse accepts. - Changeset `27801-chunked-init-filename-min1.md`, `@objectstack/spec` patch, `fix(spec)!`, with both declaration lines and the ADR-0087 disposition `not-required (no-migration-prescription)`. This follows the precedent of the other request-declaration narrowings (`26817-http-find-query-params-count-select.md`, `27683-search-fields-names-a-field.md`). **What happens to the door's truthiness check.** I kept it, for three measured reasons: 1. With `min(1)` the check is a strict subset of the schema's refusal set. A missing, `''`, `null`, `0` or `false` value fails `z.string().min(1)` as well, so the two never disagree about what is refused. 2. It runs **before** the scope gate. The scope gate runs before the parse, and #22470 asked for that order. If the check were folded into the parse, an empty `filename` sent with an off-vocabulary `scope` would get the scope refusal instead of the presence refusal. That is the ordering change the order forbids. 3. Folding it in would also change the message for an empty or missing `filename` / `mimeType` from `filename, mimeType, and totalSize are required` to `chunkedInitRefusalMessage`'s text. A missing `totalSize` would still need its own check ahead of the scope gate. Keeping the check means status, code, message and ordering stay byte-identical, which is what the ruling's "no behaviour change at the door" asks for. ## Tests - `packages/spec/src/api/storage.test.ts`: `refuses an empty %s, naming that key` covers `filename` and `mimeType`. Each asserts `success: false`, exactly one issue, `path` equal to the key, and `code` equal to `too_small`. `CONTROL: a one-character filename and mimeType parse` asserts that `'a'` / `'b'` parse. - `packages/services/service-storage/src/storage-routes.test.ts`: `REFUSED 400 INVALID_REQUEST: an empty %s, which the declared schema refuses too, with no row written` covers `filename` and `mimeType`. Each asserts status `400`, `success: false` and `error.code` `INVALID_REQUEST`, that `InitiateChunkedUploadRequestSchema.safeParse` refuses the same body, and that neither `createFile` nor `createSession` was called. No earlier test sent an empty `filename` or `mimeType` to this door. The existing chunked-init cases cover `totalSize`, `chunkSize` and `metadata` only. <details><summary>Ablation (spec): removing each min(1) turns its own case red</summary> Run with `node scripts/ablation-replace.mjs`. The anchor hit once each time and the file was restored to its HEAD blob (`e4b1a7313842`), with `git diff HEAD` empty. - `filename: z.string().min(1)` replaced by `filename: z.string()`: `× refuses an empty filename, naming that key`, Tests 1 failed / 64 passed (65). - `mimeType: z.string().min(1)` replaced by `mimeType: z.string()`: `× refuses an empty mimeType, naming that key`, Tests 1 failed / 64 passed (65). The door test is not ablated, because the door's behaviour did not change. Its schema-agreement assertion reads `@objectstack/spec/api` through the built `dist/`. </details> <details><summary>Local runs</summary> - `pnpm --filter @objectstack/spec exec vitest run src/api/storage.test.ts`: Test Files 1 passed, Tests 65 passed. - `pnpm --filter @objectstack/service-storage test`: Test Files 71 passed (71), Tests 1311 passed (1311). `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`. `--fix` regenerated `gen:docs` and re-checked green. After merging `origin/main`, the spec was rebuilt and all 14 generated artefacts are up to date. - Probe on the merged head, driving both doors and both schemas with `''`: - presigned `filename ''`: schema `success: true`, door `400 INVALID_REQUEST`. - chunked `filename ''`: schema `success: false`, door `400 INVALID_REQUEST`. - The same holds for `mimeType`. </details> ## Acceptance notes - **Out of scope, reported to the seat and not filed:** `GetPresignedUrlRequestSchema.filename` and `.mimeType` (`storage.zod.ts:67-68`) are still bare `z.string()`. `POST /api/v1/storage/upload/presigned` refuses `''` for either with `400 INVALID_REQUEST` (`storage-routes.ts:968-969`), so this is the same declared-versus-enforced gap on the sibling door. The probe above shows it. That door does not parse its body with the schema at all. This belongs with the card split from #27549 part (b). - A whitespace-only `filename` / `mimeType` is still admitted by both the schema and the door. The two agree, and this PR leaves that alone. ## Verification All readings below are on head `f5bb94f94e`, which is the branch merged with `origin/main` at `755e28500`. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 109 commands, and all 109 were run with each exit code captured before any pipe. - 106 exited 0 on the first pass. - 3 exited 3 (PREREQUISITE NOT MET, unbuilt `dist/`): `check:skill-examples`, `check:dual-build-cjs-loads` and `check:i18n`. All 3 exited 0 after a workspace build. - `--ran` reconciliation: "109 derived, 109 run, 0 NOT-MEASURED, 0 UNRUN (a DERIVED zero — all 109 recorded an exit code and none of them is 3)". - `node scripts/check-adr-0087-registration.mjs --base origin/main` exited 0 and reads the changeset as `[BREAKING+bang+clause-②-narrowing] not-required (no-migration-prescription)`. - Full dogfood (`pnpm --filter @objectstack/dogfood test`, under the lock, closure built) gave VERDICT command-exit 1: Test Files 3 failed / 352 passed / 3 skipped (358), Tests 4 failed / 2864 passed / 20 skipped (2888). All 4 failures still fail when their files are run alone, and none touches storage: - 2 in `error-sentences-live.dogfood.test.ts` and 1 in `object-hook-lifecycle-scratch.dogfood.test.ts` assert that the `/data` door answers an undeclared hook abort with no `code`. Commit `21c4bdd804` on `main` made that answer `HOOK_ABORTED`. - 1 in `platform-app-object-entry-views.test.ts` names `sys_exchange_rate` as undeclared. That object arrived on `main` with `194e66c028`. - This is main-red on the push-only battery, reported to the seat and not caused by this diff. - CI check-runs: not yet read. The PR opens as a draft, and the seat reads convergence. --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_017ihL2fi9zfJhUTubyJYb6V --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2a3dec2 commit a68430a

6 files changed

Lines changed: 74 additions & 4 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
fix(spec)!: `InitiateChunkedUploadRequestSchema.filename` and `.mimeType` declare that they are non-empty, as the chunked-upload init door has always required (#27801)
6+
7+
Clause-②: no (narrowing) — the published declaration of the chunked-upload init body refuses an empty `filename` or `mimeType`, which it admitted before. The door already refused both with a 400; nothing is widened and no key is added, removed or renamed.
8+
Metadata-protocol: none
9+
10+
<!-- 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 an empty `filename` or `mimeType`, values the chunked-upload init door already refused. A caller sends the file's name and MIME type; there is nothing for a migration to rewrite and no tombstone. -->
11+
12+
**BREAKING** accept-set narrowing of a published request declaration (`@objectstack/spec/api`), read by the `POST /api/v1/storage/upload/chunked` door, codegen, the reference docs and clients.
13+
14+
- **Before.** `filename` and `mimeType` were bare strings, so `''` parsed, and the published JSON Schema admitted an empty value that the door refuses.
15+
- **Now.** Both carry a minimum length of 1, and their descriptions say non-empty. A parse of an empty `filename` or `mimeType` fails on that key.
16+
- **Unchanged.** The door answers an empty or missing `filename` or `mimeType` with `400 INVALID_REQUEST` and the same message as before, ahead of the scope check, and writes no row. A whitespace-only value is still admitted by both.
17+
- **Who sees it.** Code that parses an init body with this schema and sent an empty name or type, or a generated client that trusted the wider declaration. The SDK's `storage.initChunkedUpload` takes this schema's input type, which stays `string` for both keys.
18+
19+
**The remedy.** Send the file's name and MIME type; an unknown type is sent as `application/octet-stream`.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,8 @@ const result = CompleteChunkedUploadRequestSchema.parse(data);
236236

237237
| Property | Type | Required | Description |
238238
| :--- | :--- | :--- | :--- |
239-
| **filename** | `string` | ✅ | Original filename |
240-
| **mimeType** | `string` | ✅ | File MIME type |
239+
| **filename** | `string` | ✅ | Original filename (non-empty) |
240+
| **mimeType** | `string` | ✅ | File MIME type (non-empty) |
241241
| **totalSize** | `integer` | ✅ | Total file size in bytes |
242242
| **chunkSize** | `integer` | optional (default: `5242880`) | Size of each chunk in bytes (minimum 5MB per S3 spec) |
243243
| **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. |

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +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';
56
import { LocalStorageAdapter } from './local-storage-adapter.js';
67
import { StorageMetadataStore } from './metadata-store.js';
78
import { registerStorageRoutes } from './storage-routes.js';
@@ -284,6 +285,28 @@ describe('Storage REST Routes', () => {
284285
expect(createSession, 'no upload-session row').not.toHaveBeenCalled();
285286
});
286287

288+
// [#27801] An empty `filename` or `mimeType` is refused at the door, and
289+
// the declared request schema refuses the same body: declared and
290+
// enforced agree on `''`.
291+
it.each(['filename', 'mimeType'] as const)(
292+
'REFUSED 400 INVALID_REQUEST: an empty %s, which the declared schema refuses too, with no row written',
293+
async (key) => {
294+
const initHandler = httpServer._getHandler('POST', '/api/v1/storage/upload/chunked')!;
295+
const createFile = vi.spyOn(store, 'createFile');
296+
const createSession = vi.spyOn(store, 'createSession');
297+
const body = { filename: 'b.bin', mimeType: 'application/octet-stream', totalSize: 100, [key]: '' };
298+
const res = createMockRes();
299+
await initHandler(createMockReq({ body }), res);
300+
301+
expect(res._status, JSON.stringify(res._json)).toBe(400);
302+
expect(res._json.success).toBe(false);
303+
expect(res._json.error.code).toBe('INVALID_REQUEST');
304+
expect(InitiateChunkedUploadRequestSchema.safeParse(body).success, 'the declaration refuses it').toBe(false);
305+
expect(createFile, 'no pending sys_file row').not.toHaveBeenCalled();
306+
expect(createSession, 'no upload-session row').not.toHaveBeenCalled();
307+
},
308+
);
309+
287310
it.each<[string, number | undefined, number, number]>([
288311
['an omitted chunkSize takes the declared default', undefined, 5242880, 3],
289312
['a chunkSize at the declared minimum', 5242880, 5242880, 3],

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,6 +1115,10 @@ export function registerStorageFamily(
11151115
const session = await requireUploadSession(req, res);
11161116
if (session === false) return;
11171117
const body = req.body ?? {};
1118+
// [#27801] The presence check refuses nothing the schema parse below
1119+
// accepts: `InitiateChunkedUploadRequestSchema` declares `filename` and
1120+
// `mimeType` non-empty. It stays ahead of the scope gate so a missing
1121+
// field keeps this answer whatever `scope` says.
11181122
if (!body.filename || !body.mimeType || body.totalSize == null) {
11191123
sendError(res, 400, 'INVALID_REQUEST', 'filename, mimeType, and totalSize are required');
11201124
return;

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,28 @@ describe('InitiateChunkedUploadRequestSchema', () => {
338338
});
339339
expect(req.metadata?.source).toBe('upload-form');
340340
});
341+
342+
// [#27801] `filename` and `mimeType` are non-empty: the init door has always
343+
// refused `''` for either, and the declaration now says the same.
344+
it.each(['filename', 'mimeType'] as const)('refuses an empty %s, naming that key', (key) => {
345+
const result = InitiateChunkedUploadRequestSchema.safeParse({
346+
filename: 'report.pdf',
347+
mimeType: 'application/pdf',
348+
totalSize: 1,
349+
[key]: '',
350+
});
351+
expect(result.success).toBe(false);
352+
expect(result.error!.issues).toHaveLength(1);
353+
expect(result.error!.issues[0].path).toEqual([key]);
354+
expect(result.error!.issues[0].code).toBe('too_small');
355+
});
356+
357+
it('CONTROL: a one-character filename and mimeType parse', () => {
358+
const result = InitiateChunkedUploadRequestSchema.safeParse({ filename: 'a', mimeType: 'b', totalSize: 1 });
359+
expect(result.success).toBe(true);
360+
expect(result.data!.filename).toBe('a');
361+
expect(result.data!.mimeType).toBe('b');
362+
});
341363
});
342364

343365
describe('InitiateChunkedUploadResponseSchema', () => {

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,10 @@ export type FileTypeValidation = z.input<typeof FileTypeValidationSchema>;
194194
* { filename: 'large-video.mp4', mimeType: 'video/mp4', totalSize: 1073741824, chunkSize: 5242880 }
195195
*/
196196
export const InitiateChunkedUploadRequestSchema = lazySchema(() => z.object({
197-
filename: z.string().describe('Original filename'),
198-
mimeType: z.string().describe('File MIME type'),
197+
// [#27801] Non-empty: the init door has always refused an empty `filename`
198+
// or `mimeType` with 400 INVALID_REQUEST, so the declaration says so too.
199+
filename: z.string().min(1).describe('Original filename (non-empty)'),
200+
mimeType: z.string().min(1).describe('File MIME type (non-empty)'),
199201
totalSize: z.number().int().min(1).describe('Total file size in bytes'),
200202
chunkSize: z.number().int().min(5242880).default(5242880)
201203
.describe('Size of each chunk in bytes (minimum 5MB per S3 spec)'),

0 commit comments

Comments
 (0)