fix(service-storage): refuse an out-of-range chunkIndex and a non-byte-count presigned size at the upload doors

.changeset/27412-storage-route-input.md

@@ -0,0 +1,16 @@
+---
+'@objectstack/service-storage': patch
+---
+
+fix(service-storage): the chunk upload door refuses a `chunkIndex` the upload did not declare, and the presigned upload door refuses a `size` that is not a byte count
+
+Clause-②: no (narrowing)
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) A REST request value is refused at the presigned upload door (a negative size): no key of any metadata schema is removed, renamed or re-shaped, no export, type or error code is removed or renamed, and the refusal reuses the existing 400 INVALID_REQUEST envelope. There is nothing for `objectstack migrate meta` to rewrite and no tombstone. -->
+
+Narrowed: a negative `size`, which the spec number (`GetPresignedUrlRequestSchema.size`, a bare `z.number()`) admits, is now refused `400` at the presigned door.
+
+- **Chunk door** (`PUT /api/v1/storage/upload/chunked/:uploadId/chunk/:chunkIndex`). **Before:** the index was read with `parseInt`, so `0.5` was read as `0`, `-1` reached the storage backend as part number 0 and answered `500 INTERNAL`, and an index at or past the upload's `totalChunks` was stored, counted in the progress, and then made `POST .../complete` answer `409` for good, since no door removes a stored chunk. **Now:** the index must be a whole number (decimal digits only) in `[0, totalChunks)`; anything else answers `400 INVALID_REQUEST` naming the valid range, with `details: { chunkIndex, totalChunks }`. Nothing is stored and the progress is unchanged, so the upload can still be completed. The check runs after the resume-token check, so a caller without the token still gets `403 INVALID_RESUME_TOKEN` first.
+- **Presigned upload door** (`POST /api/v1/storage/upload/presigned`). **Before:** only a missing `size` was refused. A non-numeric `size` such as `"abc"` reached the `sys_file` insert and failed there as `500 INTERNAL`, and a negative `size` such as `-5` answered `200` and stored a pending `sys_file` row with that size. **Now:** `size` must be a finite, non-negative JSON number (the `GetPresignedUrlRequestSchema.size` number); a string (a numeric string such as `"100"` included), `NaN`, an infinity, a negative number or a boolean answers `400 INVALID_REQUEST` naming `size`, and no `sys_file` row is written. `0` and any positive number behave as before.
+- **Who sees it.** Only a client that sent an index outside the range it was given, or a size that is not a JSON number of bytes. The SDK (`storage.upload`, `resumeUpload`) and the Console upload client send `0 .. totalChunks - 1` and `File.size`, so they are not affected.

packages/services/service-storage/src/chunked-upload-integrity.test.ts

@@ -222,7 +222,11 @@ describe('[#22313] the chunked completion door assembles the parts the upload ho
   it('REFUSED: a chunk held beyond the declared chunk count', async () => {
     const { uploadId, resumeToken, fileId } = await init(100);
     const sent0 = await putChunk(uploadId, resumeToken, 0, fill('a', 100));
-    await putChunk(uploadId, resumeToken, 3, fill('z', 5));
+    // The chunk door refuses an index outside the declared range (#27412), so
+    // a stray chunk can only be one a session recorded before that refusal
+    // existed: planted on the session row, the shape such a session carries.
+    const held = JSON.parse((await store.getSession(uploadId))!.parts ?? '[]') as Array<Record<string, unknown>>;
+    await store.updateSession(uploadId, { parts: JSON.stringify([...held, { chunkIndex: 3, eTag: 'stray', size: 5 }]) });
 
     const res = await complete(uploadId, [{ chunkIndex: 0, eTag: etagOf(sent0) }]);
 
@@ -319,6 +323,71 @@ describe('[#22313] the chunked completion door assembles the parts the upload ho
     expect((await adapter.download(res._json.data.key)).equals(fill('a', 100))).toBe(true);
   });
 
+  // ── the chunk door refuses an index the upload did not declare (#27412) ──
+
+  describe('[#27412] the chunk door takes only a whole-number index in [0, totalChunks)', () => {
+    async function putRaw(uploadId: string, resumeToken: string, chunkIndex: string, bytes: Buffer): Promise<Res> {
+      const res = createMockRes();
+      await server.handler('PUT', CHUNK)(
+        createMockReq({ params: { uploadId, chunkIndex }, headers: { 'x-resume-token': resumeToken }, rawBody: async () => bytes }),
+        res,
+      );
+      return res;
+    }
+
+    // `1` is the declared count itself (one chunk declared, so index 0 alone).
+    it.each(['-1', '1', '7', '0.5', '1e0', '0x0', 'abc'])(
+      'REFUSED 400 INVALID_REQUEST: index %s, with nothing stored or counted, and the upload still completes',
+      async (index) => {
+        const { uploadId, resumeToken } = await init(100);
+        const stored = vi.spyOn(adapter, 'uploadChunk');
+        const before = await store.getSession(uploadId);
+
+        const res = await putRaw(uploadId, resumeToken, index, fill('z', 10));
+
+        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(/^chunkIndex must be 0: this upload declared 1 chunk/);
+        expect(res._json.error.details).toEqual({ chunkIndex: index, totalChunks: 1 });
+        expect(stored, 'no byte reached the backend').not.toHaveBeenCalled();
+        const after = await store.getSession(uploadId);
+        expect(after!.parts ?? null).toEqual(before!.parts ?? null);
+        expect(await progress(uploadId)).toMatchObject({ uploadedChunks: 0, uploadedSize: 0, percentComplete: 0 });
+
+        // The refusal left nothing behind that blocks the upload.
+        stored.mockRestore();
+        const sent0 = await putChunk(uploadId, resumeToken, 0, fill('a', 100));
+        const done = await complete(uploadId, [{ chunkIndex: 0, eTag: etagOf(sent0) }]);
+        expect(done._status, JSON.stringify(done._json)).toBe(200);
+        expect((await adapter.download(done._json.data.key)).equals(fill('a', 100))).toBe(true);
+      },
+    );
+
+    it('names the range of a multi-chunk upload', async () => {
+      const { uploadId, resumeToken, totalChunks } = await init(MIN_CHUNK + 10);
+      expect(totalChunks).toBe(2);
+      const res = await putRaw(uploadId, resumeToken, '2', fill('z', 10));
+      expect(res._status, JSON.stringify(res._json)).toBe(400);
+      expect(res._json.error.code).toBe('INVALID_REQUEST');
+      expect(res._json.error.message).toMatch(/^chunkIndex must be a whole number from 0 to 1: this upload declared 2 chunks/);
+    });
+
+    it('CONTROL: the last declared index is accepted', async () => {
+      const { uploadId, resumeToken } = await init(MIN_CHUNK + 10);
+      const res = await putRaw(uploadId, resumeToken, '1', fill('b', 10));
+      expect(res._status, JSON.stringify(res._json)).toBe(200);
+      expect(res._json.data.chunkIndex).toBe(1);
+    });
+
+    it('CONTROL: a caller without the resume token learns nothing of the range — 403 before the index is judged', async () => {
+      const { uploadId } = await init(100);
+      const res = await putRaw(uploadId, 'not-the-token', '7', fill('z', 10));
+      expect(res._status, JSON.stringify(res._json)).toBe(403);
+      expect(res._json.error.code).toBe('INVALID_RESUME_TOKEN');
+    });
+  });
+
   // ── sessions recorded before part sizes existed ───────────────────────
 
   describe('a session whose parts predate the recorded size is judged by its running total', () => {

packages/services/service-storage/src/storage-limits.test.ts

@@ -232,13 +232,25 @@ describe('[#22283] §1 · the upload doors refuse a file over max_upload_mb, bef
     expect(rawBody).not.toHaveBeenCalled();
   });
 
+  /**
+   * The init door floors a chunk at 5 MiB, so an upload under a 1 MiB limit
+   * declares ONE chunk, and the chunk door refuses an index past the declared
+   * count (#27412). The limit is judged on the bytes received across chunks,
+   * so these cases widen the session row to two declared chunks to send its
+   * bytes in two.
+   */
+  async function declareTwoChunks(store: { updateSession(id: string, patch: Record<string, unknown>): Promise<unknown> }, uploadId: string) {
+    await store.updateSession(uploadId, { total_chunks: 2 });
+  }
+
   it('chunk door: the chunk that takes the bytes received past the limit is 413 — not stored, progress unchanged', async () => {
     const { adapter, store, routes } = await mountDoors(SAVED_ONE_MB);
     const started = await call(routes, 'POST', `${BASE}/upload/chunked`, {
       body: { filename: 'c.bin', mimeType: 'application/octet-stream', totalSize: MIB },
     });
     expect(started.status).toBe(200);
     const { uploadId, resumeToken } = started.body.data;
+    await declareTwoChunks(store, uploadId);
     const uploadChunk = vi.spyOn(adapter, 'uploadChunk');
     const chunk = (index: number, bytes: number, headers: Record<string, string> = {}) =>
       call(routes, 'PUT', `${BASE}/upload/chunked/:uploadId/chunk/:chunkIndex`, {
@@ -263,11 +275,12 @@ describe('[#22283] §1 · the upload doors refuse a file over max_upload_mb, bef
   });
 
   it('chunk door: a RETRIED chunk index replaces its earlier bytes — a retry is not counted twice, a grown retry still is judged', async () => {
-    const { adapter, routes } = await mountDoors(SAVED_ONE_MB);
+    const { adapter, store, routes } = await mountDoors(SAVED_ONE_MB);
     const started = await call(routes, 'POST', `${BASE}/upload/chunked`, {
       body: { filename: 'r.bin', mimeType: 'application/octet-stream', totalSize: MIB },
     });
     const { uploadId, resumeToken } = started.body.data;
+    await declareTwoChunks(store, uploadId);
     const uploadChunk = vi.spyOn(adapter, 'uploadChunk');
     const chunk = (index: number, bytes: number) =>
       call(routes, 'PUT', `${BASE}/upload/chunked/:uploadId/chunk/:chunkIndex`, {

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

@@ -118,6 +118,43 @@ describe('Storage REST Routes', () => {
       await handler(req, res);
       expect(res._status).toBe(400);
     });
+
+    // [#27413] The declared size is a byte count: a finite, non-negative JSON
+    // number. Anything else is refused at the door, before the pending
+    // `sys_file` row — a non-numeric size used to fail that insert as a 500,
+    // and a negative one was stored.
+    it.each<[string, unknown]>([
+      ['a non-numeric string', 'abc'],
+      ['an empty string', ''],
+      ['a numeric string', '100'],
+      ['a negative number', -5],
+      ['a negative fraction', -0.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 res = createMockRes();
+      await handler(createMockReq({ body: { filename: 'a.png', mimeType: 'image/png', size } }), 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(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 res = createMockRes();
+      await handler(createMockReq({ body: { filename: 'a.png', mimeType: 'image/png', size } }), res);
+
+      expect(res._status, JSON.stringify(res._json)).toBe(200);
+      const row = await store.getFile(res._json.data.fileId);
+      expect(row!.size).toBe(size);
+      expect(row!.status).toBe('pending');
+    });
   });
 
   describe('POST /upload/complete', () => {

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

@@ -159,6 +159,43 @@ function uploadScopeRefusalMessage(scope: unknown): string {
   return `scope ${named} is not accepted: it is not an upload scope. ${allowed}`;
 }
 
+/**
+ * [#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.
+ */
+function isDeclaredByteCount(size: unknown): size is number {
+  return typeof size === 'number' && Number.isFinite(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';
+
+/**
+ * [#27412] The chunk door's `chunkIndex` path segment, read strictly: decimal
+ * digits only, so `-1`, `0.5`, `1e2` and `0x1` are not indexes. `null` ⇒ not
+ * a whole number. The range half — below the upload's declared
+ * `total_chunks` — is judged against the session row by the caller, because
+ * a chunk stored outside that range is counted in the progress and then
+ * blocks the completion door for good (no door removes a chunk).
+ */
+function parseChunkIndex(raw: string): number | null {
+  if (!/^\d+$/.test(raw)) return null;
+  const n = Number(raw);
+  return Number.isSafeInteger(n) ? n : null;
+}
+
+function chunkIndexRefusalMessage(totalChunks: number): string {
+  const range = totalChunks === 1 ? 'must be 0' : `must be a whole number from 0 to ${totalChunks - 1}`;
+  return (
+    `chunkIndex ${range}: this upload declared ${totalChunks} chunk${totalChunks === 1 ? '' : 's'}, ` +
+    'indexed from zero.'
+  );
+}
+
 /**
  * [#22332] How many times the chunk door tries to record a stored chunk in the
  * upload's progress before it refuses.
@@ -918,6 +955,13 @@ export function registerStorageFamily(
         sendError(res, 400, 'INVALID_REQUEST', 'filename, mimeType, and size are required');
         return;
       }
+      // [#27413] Not a byte count ⇒ refused before anything is stored: a
+      // non-numeric size otherwise failed the `sys_file` insert as a 500, and
+      // a negative one was stored on a pending row.
+      if (!isDeclaredByteCount(size)) {
+        sendError(res, 400, 'INVALID_REQUEST', PRESIGNED_SIZE_REFUSAL_MESSAGE);
+        return;
+      }
       // [#22470] A scope outside the upload vocabulary is refused before
       // anything is stored or minted.
       if (!requireAcceptedUploadScope(scope, res)) return;
@@ -926,7 +970,7 @@ export function registerStorageFamily(
       // bytes are judged again at `_local/raw`; an S3 URL takes them straight
       // to the bucket, so this is the platform's one look at that upload.
       const { presignedTtl, maxUploadBytes } = currentLimits();
-      if (!requireWithinUploadLimit(Number(size), maxUploadBytes, (n) => `The declared file size is ${n} bytes`, res)) return;
+      if (!requireWithinUploadLimit(size, maxUploadBytes, (n) => `The declared file size is ${n} bytes`, res)) return;
 
       const fileId = randomUUID();
       const key = buildKey(scope ?? 'user', fileId, filename);
@@ -1160,8 +1204,7 @@ export function registerStorageFamily(
       if (authSession === false) return;
       const writeContext: StorageWriteContext = { organizationId: authSession?.organizationId };
       const { uploadId, chunkIndex: chunkIndexStr } = req.params;
-      const chunkIndex = parseInt(chunkIndexStr, 10);
-      if (!uploadId || isNaN(chunkIndex)) {
+      if (!uploadId || !chunkIndexStr) {
         sendError(res, 400, 'INVALID_REQUEST', 'uploadId and chunkIndex are required');
         return;
       }
@@ -1199,6 +1242,22 @@ export function registerStorageFamily(
         return;
       }
 
+      // [#27412] The index must name a chunk this upload declared: a whole
+      // number in [0, total_chunks). Judged here — after the resume token
+      // (a caller who cannot prove it owns the session learns nothing of its
+      // chunk count) and before a byte is read, stored or counted. An index
+      // outside the range used to be stored and counted in the progress, then
+      // refused for good by the completion door; `-1` reached the backend as
+      // part number 0 and answered 500.
+      const totalChunks = Number(session.total_chunks);
+      const chunkIndex = parseChunkIndex(chunkIndexStr);
+      if (chunkIndex === null || !(chunkIndex < totalChunks)) {
+        sendError(res, 400, 'INVALID_REQUEST', chunkIndexRefusalMessage(totalChunks), {
+          details: { chunkIndex: chunkIndexStr, totalChunks },
+        });
+        return;
+      }
+
       // [#22283] The bytes received so far plus this chunk, against the saved
       // limit — the declared length first, so an oversized chunk is refused
       // before it is read, then the bytes actually read, before the backend