Open
Conversation
Coverage Report for CI Build 24742179298Coverage increased (+0.1%) to 70.064%Details
Uncovered Changes
Coverage Regressions2 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
This PR closes a remaining path traversal gap for TUS uploads when using the file backend by centralizing secure filesystem path resolution and applying it to TUS upload IDs, plus adding integration/unit coverage for the new behavior.
Changes:
- Extracted path traversal protection into a shared
resolveSecureFilesystemPath()helper and reused it in the file backend. - Updated the TUS file store to resolve upload paths securely (and added new unit tests around traversal protection / expiry cleanup).
- Expanded TUS integration coverage to run against both S3 and file backends, including a traversal regression test.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/tus.test.ts | Refactors/extends integration tests to cover both backends and adds a file-backed traversal regression test. |
| src/storage/protocols/tus/file-store.ts | Adds secure path resolution for file-backed TUS uploads and expands datastore functionality (create/read/write/remove/getUpload/deleteExpired). |
| src/storage/protocols/tus/file-store.test.ts | New unit tests validating traversal protection and deletion/expiry behavior for the file-backed TUS store. |
| src/storage/backend/secure-path.ts | New shared helper that validates and safely resolves user-controlled relative paths under a fixed root. |
| src/storage/backend/secure-path.test.ts | Unit tests for the new secure path helper. |
| src/storage/backend/index.ts | Re-exports the secure path helper from the backend package. |
| src/storage/backend/file.ts | Replaces inlined traversal logic with the shared secure path helper. |
| src/storage/backend/file.test.ts | Removes the old resolveSecurePath unit suite now covered by secure-path.test.ts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ferhatelmas
force-pushed
the
ferhat/tus-path-traversal
branch
from
17639e7 to
3249174
Compare
There was a problem hiding this comment.
Pull request overview
This PR closes a path traversal gap in the TUS implementation when using the file backend by centralizing secure filesystem path resolution and adding regression coverage for both file and S3 TUS flows.
Changes:
- Extracted a shared
resolveSecureFilesystemPath()helper and reused it in the file backend. - Hardened the TUS file store implementation (file backend) and expanded TUS integration tests to run against both S3 and file backends.
- Added targeted unit/integration tests covering traversal rejection and TUS file-store expiration behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/tus.test.ts | Refactors TUS integration tests to run for both backends and adds a file-backed traversal regression test. |
| src/storage/protocols/tus/file-store.ts | Adds secure path resolution for file-backed TUS uploads plus custom implementations for read/write/remove/getUpload/deleteExpired. |
| src/storage/protocols/tus/file-store.test.ts | Adds unit tests for traversal protection and deleteExpired behavior/concurrency. |
| src/storage/backend/secure-path.ts | Introduces shared secure path resolver to prevent traversal. |
| src/storage/backend/secure-path.test.ts | Adds unit tests for secure path resolver edge cases. |
| src/storage/backend/index.ts | Re-exports the new secure-path helper. |
| src/storage/backend/file.ts | Replaces inline secure-path logic with the shared helper. |
| src/storage/backend/file.test.ts | Removes the old resolveSecurePath unit suite (covered by secure-path tests now). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ferhatelmas
force-pushed
the
ferhat/tus-path-traversal
branch
from
3249174 to
128cab1
Compare
There was a problem hiding this comment.
Pull request overview
This PR closes a remaining path traversal vector in the TUS implementation when using the file backend by centralizing secure path resolution and applying it to TUS upload IDs, with expanded test coverage for both S3 and file-backed TUS flows.
Changes:
- Extracts a shared
resolveSecureFilesystemPath()helper and reuses it inFileBackendand the TUSFileStore. - Hardens file-backed TUS datastore operations (create/read/write/remove/getUpload/deleteExpired) against traversal and adds concurrency limiting for expired-upload cleanup.
- Expands/modernizes TUS integration tests to cover both backends, resumable behavior, deletion, and a traversal regression test.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/tus.test.ts | Refactors TUS integration tests to run against S3 + file backends; adds resumable/delete + traversal coverage. |
| src/storage/protocols/tus/file-store.ts | Applies secure path resolution to file-backed TUS IDs; adds read/write/remove/getUpload/deleteExpired implementations and concurrency limiting. |
| src/storage/protocols/tus/file-store.test.ts | Adds focused unit tests for traversal protection and expiration cleanup behavior in TUS FileStore. |
| src/storage/backend/secure-path.ts | Introduces shared secure path resolver for user-controlled filesystem paths. |
| src/storage/backend/secure-path.test.ts | Adds unit tests for secure path resolver behavior and rejection cases. |
| src/storage/backend/index.ts | Re-exports the new secure path helper. |
| src/storage/backend/file.ts | Replaces inline secure path logic with the shared helper. |
| src/storage/backend/file.test.ts | Removes the old FileBackend-only resolveSecurePath unit tests (now covered by secure-path tests). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ferhatelmas
force-pushed
the
ferhat/tus-path-traversal
branch
from
128cab1 to
8aef1c1
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses a remaining path traversal vulnerability in TUS uploads when using the file backend by extracting the existing secure-path logic into a shared helper and applying it to TUS upload IDs, alongside expanded integration/unit test coverage for both S3 and file-backed TUS flows.
Changes:
- Extracted filesystem path traversal prevention into
resolveSecureFilesystemPath()and reused it in the file backend and TUS file store. - Hardened file-backed TUS datastore operations (create/read/write/remove/getUpload/deleteExpired) to validate upload IDs and added concurrency limiting in expiration GC.
- Expanded integration/unit tests to cover file backend TUS flows (upload/resume/delete) and explicit traversal attempts.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/tus.test.ts | Refactors/expands TUS integration tests to run against both S3 and file backends; adds traversal regression test. |
| src/storage/protocols/tus/file-store.ts | Adds secure-path resolution for upload IDs and implements/remaps several datastore methods + concurrent deleteExpired. |
| src/storage/protocols/tus/file-store.test.ts | Adds focused unit tests for traversal protection and deleteExpired behaviors. |
| src/storage/backend/secure-path.ts | New shared helper for secure path resolution under a fixed root. |
| src/storage/backend/secure-path.test.ts | Unit tests for the new secure path helper. |
| src/storage/backend/index.ts | Re-exports the new secure-path helper. |
| src/storage/backend/file.ts | Replaces duplicated secure-path logic with the shared helper. |
| src/storage/backend/file.test.ts | Removes old resolveSecurePath unit tests (now covered by secure-path.test.ts). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ferhatelmas
force-pushed
the
ferhat/tus-path-traversal
branch
from
8aef1c1 to
a64ffe1
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses a remaining path traversal risk when using TUS uploads with the file backend by centralizing secure filesystem path resolution and applying it consistently to TUS upload IDs.
Changes:
- Extracted a shared
resolveSecureFilesystemPath()helper and reused it in the file backend and TUS file store. - Hardened the TUS file store to validate/resolve upload paths safely and added concurrency control + safer cleanup behavior for expired uploads.
- Expanded test coverage: TUS integration now runs against both S3 and file backends, plus new focused unit tests for secure-path and the TUS file store.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/test/tus.test.ts | Refactors TUS integration tests to run for both S3 and file backends; adds file-backed traversal regression coverage. |
| src/storage/protocols/tus/file-store.ts | Applies secure path resolution to TUS file operations; adds safer deleteExpired behavior with bounded concurrency. |
| src/storage/protocols/tus/file-store.test.ts | Adds unit tests covering traversal rejection, remove semantics, and deleteExpired behavior/concurrency. |
| src/storage/backend/secure-path.ts | Introduces shared secure path resolver to prevent traversal and invalid path inputs. |
| src/storage/backend/secure-path.test.ts | Adds unit tests for secure path resolver behavior and error shaping. |
| src/storage/backend/index.ts | Re-exports the new secure path helper. |
| src/storage/backend/file.ts | Replaces inline secure-path logic with the shared helper. |
| src/storage/backend/file.test.ts | Removes old resolveSecurePath unit tests now covered by secure-path tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ferhatelmas
force-pushed
the
ferhat/tus-path-traversal
branch
from
a64ffe1 to
483cca1
Compare
There was a problem hiding this comment.
Pull request overview
This PR hardens the TUS implementation (especially for the file backend) against path traversal by reusing the same secure path resolution logic already applied in the file backend, and expands test coverage to cover both file and S3 backends plus traversal scenarios.
Changes:
- Extracts a shared
resolveSecureFilesystemPath()helper and reuses it in the file backend and TUS FileStore. - Updates TUS FileStore to resolve upload paths securely and adds safer/controlled expired-upload cleanup behavior.
- Expands TUS integration/unit tests to cover file backend behavior, resumability, deletion, and traversal rejection.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/test/tus.test.ts | Refactors TUS integration tests to run against both S3 and file backends; adds resume/delete and traversal test coverage. |
| src/storage/protocols/tus/file-store.ts | Secures upload path resolution for file-backed TUS and adds concurrency-limited expired upload cleanup. |
| src/storage/protocols/tus/file-store.test.ts | Adds focused unit tests for FileStore traversal protection and GC behavior. |
| src/storage/backend/secure-path.ts | Introduces shared secure filesystem path resolver used for user-controlled relative paths. |
| src/storage/backend/secure-path.test.ts | Adds unit tests validating traversal/absolute-path/null-byte rejection semantics. |
| src/storage/backend/index.ts | Re-exports the new secure path helper from the backend module. |
| src/storage/backend/file.ts | Replaces inline secure-path logic with the extracted helper. |
| src/storage/backend/file.test.ts | Removes now-redundant unit tests that directly targeted the prior inline secure-path implementation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
itslenny
reviewed
Apr 21, 2026
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
ferhatelmas
force-pushed
the
ferhat/tus-path-traversal
branch
from
483cca1 to
3bde24c
Compare
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
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.
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
File backend prevents path traversal via #818 and #883 but TUS with file backend has still the same problem.
What is the new behavior?
Extract secure path helper from file backend and protect upload ids for TUS uploads.
Additional context
Add file backend to integration tests.
Add concurrency control into delete expired and guard for zero offset where it would delete complete uploads if expiration is given and gc was called.
Remove exposes more info about error now.