express icon indicating copy to clipboard operation
express copied to clipboard

test: fix static file tests for supertest 7+ URL normalization

Open guimard opened this issue 2 months ago • 2 comments

Since superagent 9.0.2, the library uses new URL() instead of the deprecated url.parse() for URL handling. The URL class automatically normalizes paths containing /../ sequences, which prevented tests from verifying Express's security behavior for path traversal attempts.

This change modifies affected tests to use Node's http.request() directly instead of supertest, bypassing client-side URL normalization and allowing proper verification of server-side path traversal protection.

Changes:

  • Add http module import to test files
  • Modify 4 tests in express.static.js to use http.request()
  • Modify 1 test in acceptance/downloads.js to use http.request()
  • Add missing test fixture directory "snow ☃" for redirect encoding test
  • Update package.json to use supertest ^7.1.4 and superagent ^10.2.3

Tests modified:

  • express.static.js: "should fall-through when traversing past root"
  • express.static.js: "should 403 when traversing past root"
  • express.static.js: "should catch urlencoded ../"
  • express.static.js: "should not allow root path disclosure"
  • acceptance/downloads.js: "should respond with 403"

Fixes compatibility with supertest 7.x and superagent 10.x while maintaining proper security validation.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or

(b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or

(c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it.

(d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved.

guimard avatar Oct 25 '25 17:10 guimard

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatedsuperagent@​8.1.2 ⏵ 10.2.398 +110010087 +1100
Updatedsupertest@​6.3.4 ⏵ 7.1.499100100 +187 +1100

View full report

socket-security[bot] avatar Oct 25 '25 17:10 socket-security[bot]

To be clear, the changes are only relevant upon upgrading to supertest v7, which is where we get superagent 9.0.2

It is not clear from the PR that this is both a dep bump, and then a code change to address test breakage in said dep bump

elsewhere we have intentionally not taken supertest 7, in order to avoid specifically this issue ala https://github.com/pillarjs/send/pull/231

jonchurch avatar Oct 30 '25 00:10 jonchurch