Repository navigation
fix: hardening for tus signed - #1492
Conversation
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
Coverage Report for CI Build 37762210411Coverage increased (+0.07%) to 84.946%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the bug hunter's ruled-out note, I checked that onIncomingRequest (which now calls verifySignedUploadRequest) always runs before onCreate in the @ tus/server POST/create lifecycle, so the unguarded findBucket in onCreate is still preceded by signature verification for signed uploads — consistent with the ruled-out finding. I also confirmed the new reply.raw.once('close', ...) dispose hook and the explicit verifySignedUploadRequest call in maxSize match their test coverage (index.test.ts, src/test/tus.test.ts).
Extended reasoning...
The diff reworks TUS signed-upload detection from URL-string matching to an explicit route-registered boolean threaded through request context, extracts signature verification into an idempotent helper now also invoked from the PATCH/maxSize path, and relocates DB-connection disposal to a single reply-level 'close' listener. It touches authentication/signature-verification logic for uploads, a security-sensitive surface, but the change is well-covered by updated/new tests (including an integration test exercising encoded-URL bypass attempts against the old string-matching approach) and the one candidate gap (onCreate lacking direct verification) is moot given onIncomingRequest always runs first in the tus-server lifecycle.
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
Signed resumable uploads derive their request context from URL string matching, which can lead to inconsistent handling and generated upload locations. Some early TUS responses also skip database connection cleanup.
What is the new behavior?
Track signed-upload context explicitly through the registered routes and upload lifecycle. Use that context when generating upload URLs, and release database connections when TUS responses close.