Repository navigation
deps(storage): bump visulima storage to 2.0.33, storage-client to 1.0.8 - #963
Merged
Merged
Conversation
✅ Deploy Preview for lunorash ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Thank you for following the naming conventions! 🙏 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe upload handler applies protocol checks before authorization and checks chunked-REST PUT targets for conflicts. TUS policies validate metadata and checksum requests. R2 storage changes add create-only writes, retain coalesced chunk ranges, and provide an S3-compatible storage factory. ChangesUpload handling and storage
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Client
participant UploadHandler
participant RoutePolicy
participant Authorization
participant ProtocolHandler
Client->>UploadHandler: Send upload request
UploadHandler->>RoutePolicy: Check protocol request
RoutePolicy-->>UploadHandler: Return declaration or refusal
alt Request refused
UploadHandler-->>Client: Return refusal response
else Request accepted
UploadHandler->>Authorization: Authorize request
Authorization-->>UploadHandler: Return authorization result
UploadHandler->>ProtocolHandler: Handle authorized request
ProtocolHandler-->>UploadHandler: Return protocol response
UploadHandler->>RoutePolicy: Finish response
RoutePolicy-->>UploadHandler: Return final response
UploadHandler-->>Client: Return response
end
Merge Risk: 🟡 Moderate · up to The new create-only guarantee for client-named uploads can still be broken. Competing uploads can block one another or overwrite an object, and a transient lookup failure can let a PUT replace an in-progress upload. These paths should be fixed before merge unless the risk is explicitly accepted. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The upload authorization boundary is preserved and method overrides are explicitly refused. However, the new create-only safeguard can strand a named upload if its object is written successfully but its completion state cannot be saved. Production exposure and the newly enabled S3 chunked-upload behavior remain incompletely established. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Thank you for confirming the Contributor License Agreement! 🙏 |
Member
Author
|
@coderabbitai review |
Member
Author
|
@coderabbitai review |
1 similar comment
Member
Author
|
@coderabbitai review |
|
Member
Author
|
@coderabbitai review |
1 similar comment
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
|
|
Member
Author
|
@coderabbitai review |
|
Merging this PR will improve performance by 41.4%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | flat 3 primitives (the notify.send attribute shape) |
116.9 µs | 55.5 µs | ×2.1 |
| ⚡ | 1 after-insert no-op trigger |
338.6 µs | 289.2 µs | +17.08% |
| ⚡ | httpRoute plain (no validators) |
711.3 µs | 620.4 µs | +14.67% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing deps/visulima-storage-2.0.26 (814ddc1) with alpha (f7cfaa6)2
Footnotes
-
17 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
alpha(e7b1dc8) during the generation of this report, so f7cfaa6 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
prisis
changed the title
Member
Author
|
@coderabbitai review |
|
@visulima/storage 2.0.26's TUS handler honors X-HTTP-Method-Override by default (allowMethodOverride, default true) and swaps the method inside the handler. createUploadHandler checks its write-only method allow-list and runs authorize on request.method first, so a POST carrying X-HTTP-Method-Override: GET passed both as a write and was then served as a GET read: the hole the write-only route closed. Two layers: - every handler is built with allowMethodOverride: false (only TUS reads it); - before authorize, a request carrying X-HTTP-Method-Override, X-HTTP-Method or X-Method-Override gets the route's existing 405 METHOD_NOT_ALLOWED refusal, with Allow. The override tests now assert 405, no authorize call and no getMeta read, on every protocol and allowed method. With both layers removed they fail (the TUS POST overridden to GET answers 200); with only allowMethodOverride: false kept, that request is a 400 create, not a read. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@visulima/storage 2.0.26 buffers a checksummed TUS PATCH in memory to verify Upload-Checksum when the storage cannot, up to maxChecksumBufferSize, 64 MiB by default: half a Worker isolate's 128 MB, so a few concurrent checksummed PATCHes could exhaust it. createUploadHandler now passes maxChecksumBufferSize = min(16 MiB, maxFileSize) to every handler (only TUS reads it), from one MAX_CHECKSUM_BUFFER_BYTES constant. A larger checksummed chunk is refused with 413 before its body is read; 16 MiB clears the client's 5 MiB default chunk and the 10 MiB chunks apps use. Tests: a checksummed PATCH of 16 MiB + 1 gets 413 with at most the stream's first chunk pulled and the offset unmoved (460 after buffering it all without the cap), and a checksummed 5 MiB chunk is stored (204). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review findings on the 2.0.26 upload route. maxFileSizeFor bypass (high). @visulima/storage 2.0.26 trims each Upload-Metadata pair before splitting key and value; Lunora's parser did not, so after a ", " separator it dropped the pair upstream stored. `filename <a.png>, filetype <image/png>` read as application/octet-stream and a 40 MiB create passed a 1 MiB image cap. tusMetadata now mirrors upstream's parseMetadata (trimmed pairs, one-space split, 400 on a pair of more than two parts, a duplicate key, a reserved key or a non-base64 value; url-safe base64 is refused as upstream refuses it), and the route answers an invalid header with that 400 on POST and PATCH before authorize and maxFileSizeFor. A parity test feeds the same headers to upstream's Tus handler and asserts the same status and the same stored type that maxFileSizeFor saw. Checksum buffering (medium). Peak memory is about twice the buffer, so a few concurrent checksummed PATCHes could exhaust an isolate, public routes included. A TUS request carrying Upload-Checksum now gets 400 (UnsupportedChecksumAlgorithm, as the binding provider answered before 2.0.26) before authorize and without its body being read, whenever the provider's checksumTypes is empty; OPTIONS on such a route drops Tus-Checksum-Algorithm. The 16 MiB maxChecksumBufferSize stays as a second guard for providers with some native checksums. Also: a test pins the options each protocol handler is built with (allowMethodOverride: false, the checksum buffer, finished-upload termination off), independent of the header refusal; the 423 docs say it needs one handler instance to serve both requests; createR2UploadStorage is documented as broken upstream (visulima/visulima#905: never ready, 503 after 5 s) with createR2BindingUploadStorage recommended, in the docs and its JSDoc. Route refusals before the gate move into refuseBeforeGate. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Follow-ups from the verification pass on the 2.0.26 upload route. - Upload-Checksum is refused only on PATCH and POST, the requests that carry bytes: upstream verifies it on PATCH and would store a POST create-with-upload unchecked. HEAD, DELETE and OPTIONS carrying it pass through as upstream treats them; a test pins that. - OPTIONS on a route that refuses checksums now drops `checksum` from Tus-Extension as well, keeping every other extension: upstream's base storage always lists it, so the memory provider advertised it. - The PATCH metadata test asserts authorize is never called. Upstream answers the same 400, so the status alone did not catch the route skipping its own check on PATCH. - Rewrap a docs paragraph. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2.0.27 (visulima/visulima#906) fixes #905 (AwsLightStorage never became ready) and #902 (concurrent chunked-REST PATCHes lost each other's chunk records, so a parallel upload was never marked complete). storage-client stays at 1.0.6. The bundled-client test asserts `status: "completed"` again and that no GET is sent at all (20/20 runs green under coverage). The guard test for chunked REST over createR2UploadStorage moves to the S3 suite in the next commit. The `_chunks` workaround in r2-upload-state-store.ts stays: with it disabled on 2.0.27, "keeps the bytes of a chunk cut off mid-body" still answers 202 where 200 is expected. Release age: the user-authorized one-off entry for storage moves from 2.0.26 to 2.0.27 (published 2026-10-04 04:40 UTC, drop 2026-10-05 04:40 UTC). pagination@7.0.2 (2.0.27 still exact-pins it) and storage-client@1.0.6 stay until they age out. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
createR2UploadStorage passed aws-light the bare account endpoint, and aws-light resolves every key with `new URL(key, endpoint)`. So each request went to `<account>/<key>`, which R2 reads as a bucket named after the file. Its bucket probe still passed, because aws-light accepts a 404 there. The endpoint is now `<account endpoint>/<bucket>/` (path-style), with or without a trailing slash on a custom `endpoint`. BREAKING CHANGE: `endpoint` on createR2UploadStorage is the account endpoint without the bucket; drop the bucket from it if you added it there. New S3 suite over an in-memory path-style R2 S3 fake (r2-s3-upload-storage.test.ts): - every request names the bucket, for the default and a custom endpoint (4 of 5 tests fail with the old endpoint); - a two-part TUS upload completes intact; - canary: upstream 2.0.27's aws-light ListParts parser keeps only the last <Part>, so the third TUS chunk gets 409 and the upload never completes; - chunked REST stays refused at construction. 2.0.26 added the contiguous-write check, so out-of-order and repeated chunks are refused (409) and no longer misplaced, but the chunk that completes an upload answers 404 with every byte stored: the provider deletes the upload's metadata on completion, before the Rest handler records the chunk. A canary over upstream's own Rest handler pins that; the guard's message and comment now give this reason. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- #905: the "createR2UploadStorage is broken upstream (503)" notes are gone from the storage docs and the file-storage concepts page. They now say what still fails: uploads of more than two parts (aws-light's ListParts parser keeps only the last part, so the third chunk gets 409). createR2BindingUploadStorage stays the recommendation: no S3 credentials, runs under wrangler dev, no part-size limits. - `endpoint` is documented as the account endpoint without the bucket. The provider takes no upload limits of its own (5 TB default), so the handler's maxFileSize/maxFileSizeFor are the caps that apply. - The chunked-REST refusal over the S3 provider now gives its real reason (the completing chunk answers 404), not "does not check offsets". - #902: the note that a parallel bundled-client upload can stay at "part" is gone; it now completes over providers that accept parallel chunks. The note that over the R2 binding the bundled client only works as a single chunk stays (still 409, verified on 2.0.27). - Upgrading names 2.0.27 and adds the parallel-completion and endpoint changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
createR2UploadStorage now appends the bucket to `endpoint` only when the endpoint does not already end in it, so `https://<acct>.r2.cloudflarestorage.com/<bucket>` (with or without a trailing slash) is not doubled. An endpoint that named the bucket with a trailing slash already worked before, so this supersedes the BREAKING CHANGE note of 664697e: no existing working configuration changes. The S3 suite asserts the exact request URLs of a two-part TUS upload under the default endpoint, a custom one, and one that already names the bucket: the bucket probe, create, ListParts, both part uploads, complete, and the metadata object's HEAD/PUT/DELETE. The fake now logs full URLs. The ListParts canary and the docs and JSDoc cite visulima/visulima#907. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The chunked-REST refusal over createR2UploadStorage exists because the completing chunk answers 404 upstream (S3BaseStorage.internalOnComplete deletes the metadata during write, then the Rest PATCH reads it back). That is now filed as visulima/visulima#908, and the guard's comment and error message, the canary test and both docs pages link it, so the refusal can be found and dropped once it is fixed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
No behaviour change; the public exports of @lunora/storage/upload are unchanged (api:check
matches).
Source:
- createR2UploadStorage, its endpoint helper and R2UploadStorageOptions move to
src/r2-s3-upload-storage.ts; the TUS rules (Upload-Metadata parser, checksum refusal,
OPTIONS without checksums) move to src/tus-route-policy.ts.
- A route policy `{ check, finish }` is built once per handler: TUS captures whether the
provider verifies checksums, chunked REST and multipart declare their file. That removes
the `protocol === "tus"` branches from fetch and the boolean parameter.
- Upload-Metadata is parsed once, before the gate, and passed to maxFileSizeFor; the
unreachable re-parse fallback is gone.
- Docblocks trimmed to the reason and the issue link. Version history they carried:
method overrides are honoured by the TUS handler since @visulima/storage 2.0.26; the
metadata parser mirrors 2.0.26/2.0.27 (unchanged between them); the S3 provider refuses
out-of-order chunks since 2.0.26. allowMethodOverride: false stays as defence in depth
next to the header list, which only names the headers known today.
- Re-wrapped the over-long comment lines and fixed the broken parenthetical in
r2-upload-state-store.ts.
Tests:
- The checksum and Upload-Metadata blocks move to upload-handler-tus.test.ts
(upload-handler.test.ts: 1306 -> ~1000 lines); MiB is declared once.
- New __tests__/tus-driver.ts (create/head/offset/patch/delete, optional extra headers,
half-duplex stream bodies) replaces rawTus, the binding suite's tus, the S3 suite's
tusCreate/tusPatch and checksummedPatch. sameBytes joins pattern in upload-pattern.ts.
- New __tests__/r2-multipart-rules.ts (validParts, concatParts) is shared by both R2 fakes.
- fake-r2-s3.ts routes to serveMultipart / serveObject (no cognitive-complexity disable),
and its upload ids come from a counter, not uploads.size + 1, which repeated.
- The override and GET refusal loops are it.each cases (no no-await-in-loop disables); the
redundant lowercase override header case is dropped.
- The metadata parity test also pins the refusal's error code against upstream's.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
createR2UploadStorage's endpoint: - A virtual-hosted endpoint (`https://<bucket>.<account>.r2.cloudflarestorage.com`) already names the bucket in its host. The bucket was still appended to the path, so every object silently landed under `<bucket>/<id>`. An explicit endpoint whose host starts with `<bucket>.` is now used as is, and resolves to `https://<host>/`, not `https://<host>//`. - An endpoint that is not an absolute http(s) URL, such as the schemeless `<account>.eu.r2.cloudflarestorage.com` the docs once showed, threw a bare `TypeError: Invalid URL` from the constructor, which can take down a Worker that builds its storage at module scope. It now throws VALIDATION_ERROR naming the expected `https://<account>.r2.cloudflarestorage.com` form. The S3 fake also serves virtual-hosted requests; the URL test gains a virtual-hosted case, and three malformed endpoints are refused at construction. The TUS checksum buffer is a fixed 16 MiB. Lowering it to `maxFileSize` could never take effect: a chunk over `maxFileSize` is already refused (413) on its Content-Length before the handler runs. The test case and the docs sentence about it are gone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2.0.28 (visulima/visulima#911) fixes #907 (aws-light ListParts kept only the last part), #908 (the chunk completing a chunked-REST upload over S3 answered 404), #909 (an interrupted chunk left an upload that could never complete) and #910 (a dropped client crashed the process during file-type detection). storage-client stays at 1.0.6 (no new release). The TUS handler, method-override and checksum code are untouched upstream. Removed, now that upstream covers it: - The construction-time refusal of chunked REST over createR2UploadStorage. Its tests become positive: over upstream's Rest and over createUploadHandler, a 3-part upload refuses out-of-order and repeated chunks (409, nothing stored) and completes (200) with the bytes intact. The ListParts test becomes a 3-part TUS upload that completes. - The `_chunks` workaround in r2-upload-state-store.ts (CHUNKS_KEY, withStoredChunks, withoutChunks, the stripping in save) and its two tests. Instead the binding provider declares 2.0.28's `sequentialWrites`: it only appends and keeps a cut-off chunk's bytes, so upstream takes the offset and completion from its `bytesWritten`. Without the flag upstream answers HEAD offset 0 after a cut-off chunk, the client re-sends from 0, and the provider refuses that overlap (409): the interrupted-chunk test fails in both the unit and the workerd suites (verified by flipping it). The workerd suite gains that interrupted-chunk test over Miniflare's R2. Changed expectations (2.0.28 semantics over a sequential provider): - X-Received-Chunks is now upstream's recorded list. The chunk completing the upload is not in it, so the bundled client resuming a finished upload re-sends that chunk once (200) instead of probing /metadata (405). The X-Received-Chunks test now pins X-Upload-Offset / X-Upload-Complete instead. - Over createR2UploadStorage a HEAD on a finished upload is a 404 (its state is deleted on completion), so the 3-part TUS test reads the offsets from the PATCH answers. Release age: the one-off storage entry moves from 2.0.27 to 2.0.28 (published 2026-10-04 08:28 UTC, remove after 2026-10-05 08:28 UTC). pagination@7.0.2 and storage-client@1.0.6 stay until they age out (11:14 and 20:05 UTC today). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- createR2UploadStorage: the "fails past two parts" (#907) notes are gone from the storage docs and the concepts page. It is documented with its real limits: one part per chunk (equal chunks of at least 5 MiB), in order. createR2BindingUploadStorage stays the recommendation. - Chunked REST: no longer refused over createR2UploadStorage (#908); the refusal paragraph and the concepts bullet are rewritten for both R2 providers (one chunk at a time, in order; out of order is a 409). - X-Received-Chunks is described as upstream's recorded list now that the provider no longer derives it (#909), including that a resuming client re-sends the completing chunk once; the PATCH over-report caveat is gone (completion comes from stored bytes). - Upgrading names 2.0.28. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… rest createR2UploadStorage's endpoint carries the signed requests and the uploaded bytes, so it must now be https:, or http: to a loopback host (localhost, 127.0.0.1, [::1]) for a local S3 such as MinIO. Any other http: endpoint throws the same VALIDATION_ERROR as a malformed one. Tested both ways. The construction-time refusal of protocol "chunked-rest" over createR2UploadStorage is back, for a new reason: @visulima/storage-client's chunked-REST adapter always reads the upload back with a final HEAD, and the S3 provider has deleted the upload's state on completion, so that HEAD is a 404 and the client rejects an upload whose bytes are stored (a retry orphans the first object), even for one chunk (visulima/visulima#915). The upstream-Rest 3-part test stays (the raw protocol works); a new canary runs the bundled adapter over upstream's Rest and the S3 fake and pins POST 201, HEAD 200, PATCH 200, HEAD 404 with the bytes stored. When #915 is fixed it flips and the refusal can go. Docs and JSDoc: - Chunked REST is documented over createR2BindingUploadStorage only; the "over either R2 provider" claims and the S3 "second chunk while streaming is a 409" claim are gone (on S3 that is a 423 on a shared handler, and 202 + 202 across isolates). - The resume note says the route answers the re-sent chunk 200 and writes nothing, over the binding provider, and links visulima/visulima#913. - createR2UploadStorage: keep an upload to 1,000 chunks or fewer, since ListParts is not paged (visulima/visulima#916); and the endpoint rules above. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Upstream records one `_chunks` entry per chunked-REST PATCH, and its merge is O(n^2): 2,500 one-byte chunks left a 67 KB upload state and about 5 ms per PATCH. The binding provider's state store now coalesces `_chunks` into contiguous ranges when it saves; the provider only appends, so the list is one prefix and stays a single entry. A new test sends 199 one-byte chunks and finds one stored range (it fails without the coalesce). The resume tests (unit and workerd) re-sent identical bytes, so "stores nothing" was vacuous. They now re-send different bytes and assert that no `put` reaches the bucket (a spy in the unit suite, a counting proxy over Miniflare's R2 in the workerd one) and the stored object is unchanged. They link visulima/visulima#913 for why the chunk is re-sent. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…opies
The `_chunks` coalesce accepted any number, so NaN, Infinity, negative or fractional
values read from stored metadata could break the sort or shrink a merged range. A range
now has to be a non-negative safe-integer offset and a positive safe-integer length, the
same rule upstream applies when it records a chunk ("Chunk length must be greater than
0"); a list with any other entry is stored as it came. Eight cases pin it: a valid list
merges into one range, and lists with a NaN, infinite, negative or fractional offset, or a
negative, zero or fractional length, stay as they are (six of them fail on the old check).
The chunked-REST refusal over createR2UploadStorage tested `instanceof AwsLightStorage`,
which an aws-light provider from a second copy of @visulima/storage (a duplicate install)
fails. It now also matches the provider class's `static name` ("aws-light"), which
upstream declares explicitly and so survives bundling. A test builds a foreign class with
that name that is not an `instanceof`, and finds it refused; a memory provider is not.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2.0.30 / 1.0.7 (visulima/visulima#914, #917) fix #913 (the chunk completing a sequential chunked-REST upload was missing from X-Received-Chunks, so a resuming client re-sent it), upload) and #916 (ListParts read only its first page). 2.0.30 also answers a REST HEAD on a finished S3 upload from the stored object. The TUS, multipart and base handlers are unchanged from 2.0.28 apart from minified names; method-override and checksum code too. Removed, now that upstream covers it: - The construction-time refusal of chunked REST over createR2UploadStorage (#915), with its `static name` fallback. The bundled-client canary is now a positive test through upstream's Rest and through createUploadHandler: POST 201, HEAD 200, PATCH 200, no trailing HEAD, bytes intact, and a later HEAD reports the finished upload complete. - The #913 expectation in the resume tests (unit and workerd): a resumed finished upload sends no PATCH, HEAD's X-Received-Chunks covers the whole file, and no put reaches the bucket. - The 1,000-part limit (#916): the S3 fake now pages ListParts, and a five-part TUS upload completes with a page size of 2 (following part-number-marker) and of 1,000. Kept: the stored-chunk coalescing (upstream's trackChunk still appends one entry per chunk; without it the 199-chunk test stores 199 entries) and the isChunkRange hardening. Release age: one-off entries move to storage 2.0.30 and storage-client 1.0.7 (both published 2026-10-04 10:53 UTC, remove after 2026-10-05 10:53 UTC); pagination@7.0.2 has aged out and is dropped. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Chunked REST is documented over both R2 providers again (one chunk at a time, in order); the "createR2UploadStorage is TUS-only" note and its #915 refusal paragraph are gone. The binding provider's lease (409 for a second streaming chunk) is scoped to it; createR2UploadStorage has no lease, so the docs say to send one chunk at a time there. - A HEAD on an upload finished over createR2UploadStorage answers it complete from the stored object. - X-Received-Chunks covers every byte stored, so a resumed finished upload sends nothing (#913 note removed). - The 1,000-chunk limit is gone (#916). - Upgrading names 2.0.30 and storage-client 1.0.7. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@visulima/storage 2.0.30's REST HEAD falls back to the bucket object of the requested id once no upload state exists (visulima/visulima#918). Over createR2UploadStorage a HEAD on the upload route therefore answered 200 with the size, type and etag of any object in the bucket: `/upload/payroll-2026` and `/upload/payroll-2026.pdf` (one extension stripped), and nested keys through an encoded slash, `/upload/avatars%2Fceo`. The route now refuses, before `authorize` and with the same 404 an unknown upload gets, any HEAD or PATCH (chunked REST and TUS) and TUS DELETE whose raw last path segment is not an id @visulima/storage issues: a 21-character nanoid, or the 2-3 hex groups its File derives from a named file's name, size and date, plus at most one extension. The raw segment is checked, so no % escape reaches the provider. Lunora passes no id generator, so these are the only shapes it can issue. A 404 rather than upstream's 400 for a malformed id: it does not tell a prober which ids the route considers well-formed, and it is what any id the route did not create already gets. Collection POSTs and OPTIONS are unaffected; GET (and /metadata) is already refused. A chunked-REST PUT names its own file, so it is only refused when its name carries a % escape (which would reach a nested key). Tests over createR2UploadStorage and the S3 fake seed a flat key, a key with an extension and a nested key the route never created: HEAD over chunked REST and TUS (10 cases), TUS PATCH/DELETE and an escaped PUT all get 404 with authorize never called and no request for the key reaching the bucket; a real upload's HEAD still answers. 13 of the 14 fail without the check. The R2 binding suites are unchanged and pass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@visulima/storage's chunked-REST PUT /upload/<name> creates or replaces the file at a key the client names (visulima/visulima#919). Through an upload route any caller the gate admits could therefore overwrite an earlier upload or any flat-keyed object in the bucket: PUT /upload/payroll-2026.txt replaced `payroll-2026`, on both R2 providers. BEHAVIOUR CHANGE: a PUT now creates <name> and is refused with 409 (FileConflict) when a file already exists under that name; it no longer replaces anything. putFile in @lunora/client/upload keeps working for new names. - Route (both providers): after `authorize`, so a caller the gate refuses gets its 403 and cannot use the 409 as an existence oracle, a PUT whose id (last segment, one extension stripped, as upstream reads it) has upload state, or a finished file the provider still finds by key (getCompletedFile, answered from the object by the S3 providers), gets 409 before any byte is written. A failed lookup counts as taken. - R2 binding provider: a create with a client-named id is create-only. It refuses when state or an object already exists at the key, the object write is a conditional put (onlyIf etagDoesNotMatch "*") so the check and the write are atomic for a file under 5 MiB, and a larger one is checked again right before complete(), which R2 gives no precondition. A lost race for one name is a 409. Generated ids (TUS, chunked-REST POST, multipart) are unaffected. - createR2UploadStorage: the route's check is check-then-write; aws-light offers no If-None-Match on its PutObject or CompleteMultipartUpload without patching upstream, so two PUTs racing for one new name can still both write. Documented, linking #919. Tests (upload-put-create-only.test.ts, over both providers): a new name is stored (201); a second PUT to it is 409 and the first file kept; a PUT onto a seeded foreign object is 409 and it is unchanged; a refused caller gets 403, not 409; putFile stores a new name and is refused on a taken one; over the binding provider two concurrent PUTs to one new name give exactly one 201 and one 409. The workerd suite checks the same over Miniflare's R2. Revert proofs: without the route check 5 of 11 fail (S3 cases, and the binding's second PUT, which upstream treats as an update); without the provider's check the foreign-object case over the binding fails. The racing-create unit test now resumes a re-sent generated id, and a new one pins the 409 for a racing client-named create. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Follow-ups to the create-only PUT (visulima/visulima#919) and issued-id (#918) checks. - The S3 name check failed open. Upstream's getMeta reports every failure as not found and its S3 getCompletedFile swallows every HeadObject error, so a HEAD that errored let the PUT replace the object. createR2UploadStorage now builds a small AwsLightStorage subclass whose isNameTaken() HEADs the upload's state and object itself and counts a name as free only on a 404 for both; any other failure is taken (409). Other providers treat only a FileNotFound from getMeta as absent. Tested with a 403 on either HEAD (a 5xx is first retried by aws4fetch for up to a minute, then takes the same path); the object-HEAD case fails without the change. - The check ran before the size caps and upstream's own PUT checks, so a request that would be refused anyway still told 409 from 400/413. It now runs last, and only for a PUT upstream would carry out (a name of letters, digits, `_`, `-`; a positive Content-Length within the provider's maxUploadSize). Tested: Content-Length 0 keeps 400, a maxFileSizeFor refusal keeps 413, on a taken name. Documented that a caller who may upload can still learn whether a name is taken. - The issued-id shape allowed only `[0-9a-z]{1,16}` after the dot, but chunked REST appends the MIME type's extension, and seven carry `_` or `-` (x_t, x_b, fe_launch, sfd-hdstx, n-gage, vbox-extpack, disposition-notification): their PATCH and HEAD got 404. The extension is now `[\w-]{1,32}`; it is stripped before any key is read. Three such uploads are tested to complete (they fail on the old shape). - A create-only PUT over the R2 binding refused at finish() left its multipart upload, segments and state behind. The writer aborts the multipart upload and the provider deletes the segments and state before the 409. Tested for a single put and a multipart upload whose name another writer takes mid-stream (both leave orphans without the fix). - The binding provider refuses an empty client-named file (upstream's PUT already requires content), so an empty object is never published before its state is held. Docs: the own-bucket warning names the root keys a HEAD can still reach: 21-character names, and hex groups such as cafe-babe or a date like 2026-10-04. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2.0.31 fixes visulima/visulima#918, #919 and #921 upstream, so the route workarounds for them go: - #918: REST HEAD answers only from upload metadata, and TUS DELETE/PATCH too, so the issued-id shape check (ISSUED_ID / refuseUnissuedId) is gone. A foreign key now gets its 404 after authorize; an escaped PUT name gets upstream's 400 instead of 404. - #919: upstream refuses a PUT onto an object without upload state (409) and its aws-light lookups fail closed, so R2S3UploadStorage.isNameTaken and the getCompletedFile check are gone. The route still refuses a PUT onto an existing upload, which upstream would replace. - #921: ids are always random, so the binding provider no longer resumes a re-sent create; only a client-named file can find its id taken. Needed for 2.0.31: - The R2 state store throws FILE_NOT_FOUND for a missing upload. Upstream now reads any other error as a failing store, which made HEAD on a deleted upload a 500 and every new-name PUT a 409. - _writeClaim joins the reserved TUS metadata keys, as upstream's parser has. - Tests: finished S3 uploads keep their .META; the dropped-client test sends a chunked body, since upstream's Readable.fromWeb drops its read-ahead. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…1.0.8 visulima/visulima#923 lands the rest of the upstream fixes, so two more workarounds go: - The R2 state store no longer merges `_chunks` itself: upstream records merged byte ranges now (one per contiguous run), which keeps the record small on every provider. - The TUS route reads Upload-Metadata with upstream's exported parseTusMetadata instead of a copy of its parser; the parity test still pins the route's answers to upstream's handler. The chunked-REST client sends one chunk at a time since 1.0.8, so a multi-chunk upload now completes over both R2 providers (it used to fail on the lease's 409); the tests that pinned the failure now pin the upload, and the docs drop the single-chunk caveat. createR2UploadStorage reads its record with GET, as upstream keeps it in the object body. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017mmaDGoEhi6ge1XqVp6AXC
…e-only storage 2.0.33 (visulima/visulima#925) accepts empty files on every protocol. Its PUT replaces an existing upload, and the route's create-only name check skipped any PUT without a positive Content-Length, because upstream used to refuse those itself: over createR2UploadStorage an empty PUT to a taken name answered 200 and replaced the file. The check now covers an explicit Content-Length: 0 as well; a PUT with no length still keeps upstream's or the cap's own answer. The R2 binding provider accepts an empty client-named file instead of refusing it. Its state is taken before the empty object is written create-only (and dropped again if that write is refused), so a create that loses the race for the name never publishes an object. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017mmaDGoEhi6ge1XqVp6AXC
prisis
force-pushed
the
deps/visulima-storage-2.0.26
branch
from
October 5, 2026 22:39
ea201cd to
ff3b1e4
Compare
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Versions
@visulima/storage2.0.25 → 2.0.33@visulima/storage-client1.0.5 → 1.0.8Together these pick up every upstream fix from visulima/visulima#899 through #925. The lockfile has one copy of each.
Release-age exception
You authorized one-off
minimumReleaseAgeExcludeentries for these upstream fixes. Their comment gives the removal dates:@visulima/storage@2.0.33@visulima/storage-client@1.0.8What the upload route does
The route's own checks, all still in place:
GETand any method the protocol doesn't need get 405 beforeauthorize. So does a request carrying a method-override header (X-HTTP-Method-Override,X-HTTP-Method,X-Method-Override), and every handler is built withallowMethodOverride: false.POSTtoPATCH/DELETE, but our gate runs on the original method, so the refusal stays.maxFileSizeandmaxFileSizeForapply before the gate, and a malformed size counts as too large.Upload-Metadata. It is read with upstream's exportedparseTusMetadata, somaxFileSizeFordecides on the metadata that gets stored. An invalid header gets a 400 beforeauthorize. A parity test sends 15 headers to upstream's handler and to the route and checks they agree.Upload-Checksum. It gets a 400 on a provider that verifies no checksum,createR2BindingUploadStorageincluded. The checksum buffer is capped at 16 MiB.PUT. A chunked-RESTPUTonto an existing upload gets a 409; upstream alone would replace it. Upstream itself now refuses aPUTonto an object that has no upload state (chore(docs): temporary ssr timing diagnostics (do not merge) #919). The binding provider also writes create-only, with a conditional put.Content-Length: 0. Before that, an emptyPUTonto a taken name overcreateR2UploadStorageanswered 200 and replaced the file.Workarounds removed
Upstream now covers these, so Lunora no longer needs them:
HEAD/PATCH/DELETEonly from upload state, so a foreign object gets a 404.isNameTakensubclass and fail-closed lookup (chore(docs): temporary ssr timing diagnostics (do not merge) #919). Upstream's lookups now fail closed on their own.parseTusMetadata.R2 binding provider
FILE_NOT_FOUND. Upstream now treats any other error as a failing store, so without this aHEADon a deleted upload returned 500 and every new-namePUTreturned 409.sequentialWrites. Its lease turns a concurrent write into a 409, and the TUS handler's in-memory lock answers 423 when one handler instance serves both requests.createR2UploadStorage(S3 API)<account endpoint>/<bucket>/), and an endpoint that already names the bucket is not given it twice.https:is accepted, orhttp:to a loopback host.Gates
@lunora/storagetests@lunora/storagetests with workerd (LUNORA_WORKERD_TESTS=1)client,react,solid,svelte,angular)tsc --noEmitand ESLint on the changed filesbuild:packagesapi:checkbuild:packages:prod, thendist:checklint:package-jsonpnpm install --frozen-lockfile🤖 Generated with Claude Code
https://claude.ai/code/session_017mmaDGoEhi6ge1XqVp6AXC