Repository navigation
Commit 85663a5
fix(service-storage): refuse an out-of-range chunkIndex and a non-byte-count presigned size at the upload doors (#27537)
domain:devx#1 · sunhaolin · session_017xRBcScm27Q3R5mkYLPWdB
Fixes #27412
Fixes #27413
Clause-②: no (narrowing)
Metadata-protocol: none
Narrowed: a negative `size`, which the spec number
(`GetPresignedUrlRequestSchema.size`, a bare `z.number()`) admits, is
now refused `400` at the presigned door.
## 维护者速读
- 改了什么:存储上传的两个入口在门口拒收坏参数:分片上传的 chunkIndex 必须是 [0, totalChunks)
内的整数,预签名上传的 size 必须是非负有限数字;其余一律 400 INVALID_REQUEST,不落任何数据。
- 为什么改:之前 chunkIndex=-1 返回 500,越界分片被存下并计入进度,之后 complete 永远 409
且无法修复;size="abc" 返回 500,size=-5 会写入一条负大小的 sys_file 记录。
- 风险与回滚:收窄了这两个入口接受的请求(例如数字字符串 "100" 作为 size 现在被拒);SDK 与 Console
上传客户端都发送合法值,不受影响。回滚即 revert 本 PR。
-
先看:packages/services/service-storage/src/storage-routes.ts(分片门的范围判断位置:在
resume token
校验之后、读取字节之前),chunked-upload-integrity.test.ts,storage-limits.test.ts(夹具改为声明两个分片)。
- 两分钟验证:pnpm --filter @objectstack/service-storage exec vitest run
--maxWorkers=2 src/chunked-upload-integrity.test.ts
src/storage-routes.test.ts ,预期 2 个文件全部通过。
- 需要你做的:无,席位落地(落地前席位做一次 contract-review 档复核)。
## What changes
Two upload doors in `@objectstack/service-storage` now refuse malformed
request input at the door with `400 INVALID_REQUEST`, the code these
doors already answer for a request they cannot take.
**Chunk door** `PUT
/api/v1/storage/upload/chunked/UPLOAD_ID/chunk/CHUNK_INDEX` (#27412)
- The index is read strictly: decimal digits only, a safe integer.
`parseInt` read `0.5` as `0`, and `1e0` as `1`.
- It is then judged against the session's `total_chunks`: it must be in
`[0, totalChunks)`. The check runs after the resume-token, organization
and expiry checks (a caller without the token still gets `403
INVALID_RESUME_TOKEN` and learns nothing of the range) and before a byte
is read, stored or counted.
- The refusal names the range (`chunkIndex must be 0: this upload
declared 1 chunk, indexed from zero.` / `chunkIndex must be a whole
number from 0 to N: ...`) and carries `details: { chunkIndex,
totalChunks }`.
**Presigned door** `POST /api/v1/storage/upload/presigned` (#27413)
- `size` must be a finite, non-negative JSON number
(`GetPresignedUrlRequestSchema.size` is `z.number()`). It is judged
after the required-field check and before the scope gate, the size limit
and the pending `sys_file` insert.
- The refusal message starts `size must be the file size in bytes`.
**Exactly what is now refused** (the narrowing the contract review
reads)
- Chunk door: any `chunkIndex` path segment that is not all decimal
digits (`-1`, `0.5`, `1e0`, `0x0`, `abc`), and any whole number at or
past the session's `total_chunks`. Before: `-1` answered `500 INTERNAL`;
`0.5` was stored as chunk `0`; an out-of-range whole number answered
`200`, was stored and counted, and then blocked `complete` with `409`
for good.
- Presigned door: a `size` that is a string (numeric strings such as
`"100"` included), `NaN`, an infinity, a negative number, or a boolean.
Before: `"abc"` answered `500 INTERNAL` (the `sys_file` insert's number
validation); `-5` answered `200` and stored a pending row with `size:
-5`. A numeric string's previous answer depended on the engine's
number-field handling at insert and was not measured here.
- Unchanged: chunk indexes `0 .. totalChunks - 1`; sizes `0` and any
positive number. The SDK (`storage.upload`, `resumeUpload`) and the
Console `UploadProvider` send `File.size` and indexes in range.
The completion door's own `unexpectedChunks` refusal stays: a session
recorded before this change can still hold a stray chunk.
## Tests
- `chunked-upload-integrity.test.ts`: new block for #27412. Seven
refused indexes (`-1`, `1` = totalChunks, `7`, `0.5`, `1e0`, `0x0`,
`abc`), each asserting status `400`, `error.code` `INVALID_REQUEST`, the
range message, the details, that the backend `uploadChunk` was never
called, that the session's parts and progress are unchanged, and that a
following in-range upload completes `200` with the right bytes.
Controls: the multi-chunk range message, the last declared index
accepted, and the `403` ordering before the index is judged. The
existing stray-chunk completion case now plants its stray part on the
session row (the shape a pre-fix session carries), since the chunk door
no longer stores one.
- `storage-routes.test.ts`: new presign cases for #27413. Eight refused
sizes, each asserting status `400`, `error.code` `INVALID_REQUEST`, the
message naming `size`, and that `store.createFile` was never called.
Controls: `0` and `1024` answer `200` and store a pending row with that
size.
- `storage-limits.test.ts`: two upload-limit cases started a 1 MiB
upload (one declared chunk under the 5 MiB chunk floor) and then sent
chunk index `1`. They now widen the session row to two declared chunks
first; what they pin, bytes received across chunks against the limit, is
unchanged.
## Acceptance notes
- `GetPresignedUrlRequestSchema.size` in `packages/spec` is `z.number()`
with no `.int()` or `.min(0)`, so the spec still declares negative and
fractional sizes legal while the door now refuses negatives. The door
follows the dispatch's ruling (finite, non-negative); a fractional size
is still accepted. Not filed: no producer sends either.
- History shape: three commits, not two. The `storage-limits.test.ts`
fixture update is a consequence of the #27412 refusal and sits in its
own commit after the two card commits, because folding it into the first
commit needed a history rewrite that was not permitted in this session.
## Verification
<details><summary>Package tests and typecheck at dbfaf81 (before the
merge of origin/main; the merged commit touches no service-storage
file)</summary>
```
pnpm --filter @objectstack/service-storage test && pnpm --filter @objectstack/service-storage typecheck
Test Files 70 passed (70)
Tests 1278 passed (1278)
check:test-typecheck: OK, 0 file(s) / 0 error(s)
os-verify-lock: VERDICT command-exit 0
```
The first run, before the `storage-limits.test.ts` fixture update,
failed the two limit cases named above (`Tests 2 failed | 1276 passed`).
</details>
<details><summary>Ablation, one per guard (source mutation through
scripts/ablation-replace.mjs; the suites import the routes from src, so
no dist build is involved)</summary>
```
range check removed (anchor x1 -> x0, blob 6c89ee5 -> 14e5b17d7623)
chunked-upload-integrity.test.ts: Tests 3 failed | 20 passed (index 1, index 7, multi-chunk range)
restored: blob == HEAD (6c89ee5), git diff HEAD empty
size check removed (anchor x1 -> x0, blob 6c89ee5 -> ed2276f74506)
storage-routes.test.ts: Tests 8 failed | 52 passed (all eight refused sizes)
restored: blob == HEAD (6c89ee5), git diff HEAD empty
```
With the range check removed, `-1`, `0.5`, `1e0`, `0x0` and `abc` stay
refused by the strict parse, as expected.
</details>
<details><summary>Derived gate families (dispatch-gates --commands at
3a10b29)</summary>
```
dispatch-gates --ran: 64 derived famil(ies) accounted for: 61 run, 3 NOT-MEASURED (3 DERIVED from a recorded exit 3)
61 x exit 0
exit 3 PREREQUISITE NOT MET: pnpm check:dual-build-cjs-loads (workspace dists not built), pnpm check:i18n (CLI not built),
node scripts/check-plugin-teardown-shape.mjs --self-test (shallow clone, pinned fixture commit unreachable)
check-nul-bytes: OK
```
Those three are NOT MEASURED locally and are left to CI.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_017xRBcScm27Q3R5mkYLPWdB)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 79bf748 commit 85663a5
5 files changed
Lines changed: 199 additions & 5 deletions
File tree
- .changeset
- packages/services/service-storage/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
222 | 222 | | |
223 | 223 | | |
224 | 224 | | |
225 | | - | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
226 | 230 | | |
227 | 231 | | |
228 | 232 | | |
| |||
319 | 323 | | |
320 | 324 | | |
321 | 325 | | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
322 | 391 | | |
323 | 392 | | |
324 | 393 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
232 | 232 | | |
233 | 233 | | |
234 | 234 | | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
235 | 246 | | |
236 | 247 | | |
237 | 248 | | |
238 | 249 | | |
239 | 250 | | |
240 | 251 | | |
241 | 252 | | |
| 253 | + | |
242 | 254 | | |
243 | 255 | | |
244 | 256 | | |
| |||
263 | 275 | | |
264 | 276 | | |
265 | 277 | | |
266 | | - | |
| 278 | + | |
267 | 279 | | |
268 | 280 | | |
269 | 281 | | |
270 | 282 | | |
| 283 | + | |
271 | 284 | | |
272 | 285 | | |
273 | 286 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
118 | 118 | | |
119 | 119 | | |
120 | 120 | | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
121 | 158 | | |
122 | 159 | | |
123 | 160 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
159 | 159 | | |
160 | 160 | | |
161 | 161 | | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
162 | 199 | | |
163 | 200 | | |
164 | 201 | | |
| |||
918 | 955 | | |
919 | 956 | | |
920 | 957 | | |
| 958 | + | |
| 959 | + | |
| 960 | + | |
| 961 | + | |
| 962 | + | |
| 963 | + | |
| 964 | + | |
921 | 965 | | |
922 | 966 | | |
923 | 967 | | |
| |||
926 | 970 | | |
927 | 971 | | |
928 | 972 | | |
929 | | - | |
| 973 | + | |
930 | 974 | | |
931 | 975 | | |
932 | 976 | | |
| |||
1160 | 1204 | | |
1161 | 1205 | | |
1162 | 1206 | | |
1163 | | - | |
1164 | | - | |
| 1207 | + | |
1165 | 1208 | | |
1166 | 1209 | | |
1167 | 1210 | | |
| |||
1199 | 1242 | | |
1200 | 1243 | | |
1201 | 1244 | | |
| 1245 | + | |
| 1246 | + | |
| 1247 | + | |
| 1248 | + | |
| 1249 | + | |
| 1250 | + | |
| 1251 | + | |
| 1252 | + | |
| 1253 | + | |
| 1254 | + | |
| 1255 | + | |
| 1256 | + | |
| 1257 | + | |
| 1258 | + | |
| 1259 | + | |
| 1260 | + | |
1202 | 1261 | | |
1203 | 1262 | | |
1204 | 1263 | | |
| |||
0 commit comments