From fb470a962def57e7ebbe5ab138ef88f684743eb4 Mon Sep 17 00:00:00 2001 From: naorpeled Date: Sat, 4 Jul 2026 17:26:19 +0300 Subject: [PATCH 1/2] fix(sendFile): surface resolved path in "No such file" error; clarify no auto-versioning (#257) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #257 reported that a `/:v/schema.json` route returned `{"error":"No such file"}` at `/v1/schema.json` and asked for an option to disable "versioning". Investigation showed lambda-api has no version routing/stripping — `/:v/schema.json` already matches `/v1/schema.json` and captures `req.params.v === "v1"`. The error actually came from `res.sendFile()` throwing ENOENT in the reporter's own handler; the terse, pathless message is what led to the misdiagnosis. - sendFile ENOENT errors now include the resolved path (e.g. `No such file: ./test-missing.txt`) so the real cause is obvious. - Add __tests__/versioning.unit.js locking in literal version-segment routing and documenting the `base: 'v1'` + leading `:param` collision caveat. - Update the two existing sendFile/download "Missing file" assertions. - Document in the README that lambda-api performs no automatic version handling, and how `res.sendFile(path, { root })` resolves paths. --- README.md | 2 + __tests__/download.unit.js | 2 +- __tests__/sendFile.unit.js | 2 +- __tests__/versioning.unit.js | 72 ++++++++++++++++++++++++++++++++++++ src/lib/response.js | 4 +- 5 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 __tests__/versioning.unit.js diff --git a/README.md b/README.md index 7e1bd30..23d26bf 100644 --- a/README.md +++ b/README.md @@ -878,6 +878,8 @@ Path parameters act as wildcards that capture the value into the `params` object A path can contain as many parameters as you want. E.g. `/users/:param1/:param2/:param3`. +Lambda API performs **no automatic version handling or stripping**. Version-like segments such as `v1` are treated as ordinary path values, so a route like `/:v/schema.json` matches `/v1/schema.json` and captures `req.params.v === 'v1'`. If you need versioned route groups, set them up explicitly with a `base` path or [Route Prefixing](#route-prefixing). Note that setting `base` to a version (e.g. `base: 'v1'`) makes that literal segment collide with a leading path parameter — prefer a non-version `base` like `/api` and use `register({ prefix: '/v1' })` for versioning. Remember too that `res.sendFile(path, { root })` resolves to `root + path`, so passing `req.path` (which includes any `base`/prefix segments) into `sendFile` looks for that full path on disk. + ## Wildcard Routes Wildcard routes are supported for matching arbitrary paths. Wildcards only work at the _end of a route definition_ such as `/*` or `/users/*`. Wildcards within a path, e.g. `/users/*/posts` are not supported. Wildcard routes do support parameters, however, so `/users/:id/*` would capture the `:id` parameter in your wildcard handler. diff --git a/__tests__/download.unit.js b/__tests__/download.unit.js index ddbba6d..c0648c8 100644 --- a/__tests__/download.unit.js +++ b/__tests__/download.unit.js @@ -155,7 +155,7 @@ describe('Download Tests:', function() { it('Missing file', async function() { let _event = Object.assign({},event,{ path: '/download' }) let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) })) - expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'], 'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file"}', isBase64Encoded: false }) + expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'], 'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file: ./test-missing.txt"}', isBase64Encoded: false }) }) // end it it('Missing file with custom catch', async function() { diff --git a/__tests__/sendFile.unit.js b/__tests__/sendFile.unit.js index b2c80f6..e6d00aa 100644 --- a/__tests__/sendFile.unit.js +++ b/__tests__/sendFile.unit.js @@ -201,7 +201,7 @@ describe('SendFile Tests:', function() { it('Missing file', async function() { let _event = Object.assign({},event,{ path: '/sendfile' }) let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) })) - expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'],'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file"}', isBase64Encoded: false }) + expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'],'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file: ./test-missing.txt"}', isBase64Encoded: false }) }) // end it it('Missing file with custom catch', async function() { diff --git a/__tests__/versioning.unit.js b/__tests__/versioning.unit.js new file mode 100644 index 0000000..f8502ae --- /dev/null +++ b/__tests__/versioning.unit.js @@ -0,0 +1,72 @@ +'use strict'; + +// Regression coverage for GitHub issue #257 ("Is it possible to disable versioning?"). +// +// lambda-api does NOT perform any automatic version detection or stripping. A +// version-like path segment (e.g. `v1`) is matched literally and exposed on +// `req.params` just like any other parameter value. These tests lock that in so +// the behavior can never silently regress. + +// Init API instance (no base -- matching the reporter's setup) +const api = require('../index')(); + +// NOTE: Set test to true +api._test = true; + +let event = { + httpMethod: 'get', + path: '/', + body: {}, + multiValueHeaders: { + 'Content-Type': ['application/json'] + } +} + +/******************************************************************************/ +/*** DEFINE TEST ROUTES ***/ +/******************************************************************************/ +api.get('/:v/schema.json', function(req,res) { + res.status(200).json({ method: 'get', status: 'ok', v: req.params.v }) +}) + +/******************************************************************************/ +/*** BEGIN TESTS ***/ +/******************************************************************************/ + +describe('Versioning (issue #257) Tests:', function() { + + it('Version-like segment is matched literally: /v1/schema.json', async function() { + let _event = Object.assign({},event,{ path: '/v1/schema.json' }) + let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) })) + expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'] }, statusCode: 200, body: '{"method":"get","status":"ok","v":"v1"}', isBase64Encoded: false }) + }) // end it + + it('Non-version first segment is matched the same way: /abc/schema.json', async function() { + let _event = Object.assign({},event,{ path: '/abc/schema.json' }) + let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) })) + expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'] }, statusCode: 200, body: '{"method":"get","status":"ok","v":"abc"}', isBase64Encoded: false }) + }) // end it + + it('No version segment is consumed -- extra segment still 404s: /v1/x/schema.json', async function() { + let _event = Object.assign({},event,{ path: '/v1/x/schema.json' }) + let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) })) + expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'] }, statusCode: 404, body: '{"error":"Route not found"}', isBase64Encoded: false }) + }) // end it + + it("Setting base to a version collides with a leading :param (documented caveat)", async function() { + // With `base: 'v1'`, the literal `v1` base segment consumes the first path + // segment, so `/v1/schema.json` matches `v1` + `:v` (schema.json) and never + // reaches the handler -- yielding a 405. Prefer a base like `/api` over a + // version, or use `register({ prefix })` for versioned route groups. + const basedApi = require('../index')({ base: 'v1' }) + basedApi._test = true; + basedApi.get('/:v/schema.json', function(req,res) { + res.status(200).json({ v: req.params.v }) + }) + + let _event = Object.assign({},event,{ path: '/v1/schema.json' }) + let result = await new Promise(r => basedApi.run(_event,{},(e,res) => { r(res) })) + expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'] }, statusCode: 405, body: '{"error":"Method not allowed"}', isBase64Encoded: false }) + }) // end it + +}) // end VERSIONING tests diff --git a/src/lib/response.js b/src/lib/response.js index fe1e26b..1f12862 100644 --- a/src/lib/response.js +++ b/src/lib/response.js @@ -397,7 +397,9 @@ class RESPONSE { // If missing file if (e.code === 'ENOENT') { - this.error(new FileError('No such file', e)); + this.error( + new FileError(`No such file${e.path ? `: ${e.path}` : ''}`, e) + ); } else { this.error(e); // Throw error if not done in callback } From ada8e4e6ee159f4dced253b1165d1a536cdda829 Mon Sep 17 00:00:00 2001 From: naorpeled Date: Sat, 4 Jul 2026 19:07:07 +0300 Subject: [PATCH 2/2] Address Copilot review: sanitize sendFile error path, doc concat pitfall, add v2/ALB versioning tests - response.js: strip opts.root prefix and reduce absolute paths to basename in the client-visible "No such file" error to avoid leaking server paths (e.g. /var/task/...); full e.path is still preserved on FileError for logs - README: note sendFile root+path is literal concatenation and needs a trailing/leading slash to avoid ./static + file.txt -> ./staticfile.txt - versioning.unit.js: assert version-like segments match literally for API Gateway v2 (rawPath) and ALB events, not just v1 - sendFile.unit.js: regression tests for absolute-root and absolute-path sanitization --- README.md | 2 +- __tests__/sendFile.unit.js | 24 ++++++++++++++++++++++++ __tests__/versioning.unit.js | 28 ++++++++++++++++++++++++++++ src/lib/response.js | 16 +++++++++++++++- 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 23d26bf..7e962d3 100644 --- a/README.md +++ b/README.md @@ -878,7 +878,7 @@ Path parameters act as wildcards that capture the value into the `params` object A path can contain as many parameters as you want. E.g. `/users/:param1/:param2/:param3`. -Lambda API performs **no automatic version handling or stripping**. Version-like segments such as `v1` are treated as ordinary path values, so a route like `/:v/schema.json` matches `/v1/schema.json` and captures `req.params.v === 'v1'`. If you need versioned route groups, set them up explicitly with a `base` path or [Route Prefixing](#route-prefixing). Note that setting `base` to a version (e.g. `base: 'v1'`) makes that literal segment collide with a leading path parameter — prefer a non-version `base` like `/api` and use `register({ prefix: '/v1' })` for versioning. Remember too that `res.sendFile(path, { root })` resolves to `root + path`, so passing `req.path` (which includes any `base`/prefix segments) into `sendFile` looks for that full path on disk. +Lambda API performs **no automatic version handling or stripping**. Version-like segments such as `v1` are treated as ordinary path values, so a route like `/:v/schema.json` matches `/v1/schema.json` and captures `req.params.v === 'v1'`. If you need versioned route groups, set them up explicitly with a `base` path or [Route Prefixing](#route-prefixing). Note that setting `base` to a version (e.g. `base: 'v1'`) makes that literal segment collide with a leading path parameter — prefer a non-version `base` like `/api` and use `register({ prefix: '/v1' })` for versioning. Remember too that `res.sendFile(path, { root })` resolves to `root + path` by plain string concatenation (not `path.join`), so passing `req.path` (which includes any `base`/prefix segments) into `sendFile` looks for that full path on disk. Because it's literal concatenation, make sure `root` ends with a trailing slash (or `path` begins with one) — otherwise `root: './static'` + `'file.txt'` resolves to `./staticfile.txt` rather than `./static/file.txt`. ## Wildcard Routes diff --git a/__tests__/sendFile.unit.js b/__tests__/sendFile.unit.js index e6d00aa..dc00eab 100644 --- a/__tests__/sendFile.unit.js +++ b/__tests__/sendFile.unit.js @@ -37,6 +37,18 @@ api.get('/sendfile/root', function(req,res) { res.sendFile('test.txt', { root: './__tests__/' }) }) +// Missing file under an absolute `root` -- the root prefix must be stripped +// from the client-visible error so internal server paths aren't disclosed. +api.get('/sendfile/root-missing', function(req,res) { + res.sendFile('missing.txt', { root: '/var/task/private/' }) +}) + +// Missing file passed as an absolute path (no root) -- reduced to its basename +// in the client-visible error for the same reason. +api.get('/sendfile/abs-missing', function(req,res) { + res.sendFile('/var/task/private/secret.txt') +}) + api.get('/sendfile/err', function(req,res) { res.sendFile('./test-missing.txt', err => { if (err) { @@ -204,6 +216,18 @@ describe('SendFile Tests:', function() { expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'],'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file: ./test-missing.txt"}', isBase64Encoded: false }) }) // end it + it('Missing file under absolute root strips the root prefix', async function() { + let _event = Object.assign({},event,{ path: '/sendfile/root-missing' }) + let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) })) + expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'],'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file: missing.txt"}', isBase64Encoded: false }) + }) // end it + + it('Missing file with absolute path is reduced to basename', async function() { + let _event = Object.assign({},event,{ path: '/sendfile/abs-missing' }) + let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) })) + expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'],'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file: secret.txt"}', isBase64Encoded: false }) + }) // end it + it('Missing file with custom catch', async function() { let _event = Object.assign({},event,{ path: '/sendfile/err' }) let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) })) diff --git a/__tests__/versioning.unit.js b/__tests__/versioning.unit.js index f8502ae..8681125 100644 --- a/__tests__/versioning.unit.js +++ b/__tests__/versioning.unit.js @@ -10,6 +10,14 @@ // Init API instance (no base -- matching the reporter's setup) const api = require('../index')(); +// Sample events for the other supported interfaces. Route matching must not +// diverge across API Gateway v1 (`path`), API Gateway v2 (`rawPath`) and ALB +// (`path` + `requestContext.elb`), so the same version-like segment is asserted +// against each shape below. Deep-cloned per test so overrides don't leak. +const sampleV2 = require('./sample-event-apigateway-v2.json'); +const sampleAlb = require('./sample-event-alb1.json'); +const clone = (obj) => JSON.parse(JSON.stringify(obj)); + // NOTE: Set test to true api._test = true; @@ -41,6 +49,26 @@ describe('Versioning (issue #257) Tests:', function() { expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'] }, statusCode: 200, body: '{"method":"get","status":"ok","v":"v1"}', isBase64Encoded: false }) }) // end it + it('API Gateway v2 (rawPath) matches the version-like segment literally: /v1/schema.json', async function() { + let _event = clone(sampleV2) + _event.rawPath = '/v1/schema.json' + _event.requestContext.http.method = 'GET' + _event.requestContext.http.path = '/v1/schema.json' + let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) })) + expect(result.statusCode).toBe(200) + expect(JSON.parse(result.body).v).toBe('v1') + }) // end it + + it('ALB event (path) matches the version-like segment literally: /v1/schema.json', async function() { + let _event = clone(sampleAlb) + _event.httpMethod = 'GET' + _event.path = '/v1/schema.json' + _event.isBase64Encoded = false + let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) })) + expect(result.statusCode).toBe(200) + expect(JSON.parse(result.body).v).toBe('v1') + }) // end it + it('Non-version first segment is matched the same way: /abc/schema.json', async function() { let _event = Object.assign({},event,{ path: '/abc/schema.json' }) let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) })) diff --git a/src/lib/response.js b/src/lib/response.js index 1f12862..90bf204 100644 --- a/src/lib/response.js +++ b/src/lib/response.js @@ -397,8 +397,22 @@ class RESPONSE { // If missing file if (e.code === 'ENOENT') { + // Surface the missing path so the cause is self-evident, but avoid + // leaking absolute server paths (e.g. /var/task/...) in the + // client-visible error body: strip any configured `root` prefix and + // reduce a still-absolute path to its basename. The full `e.path` is + // preserved on the FileError for server-side logging. + let display = e.path; + if (display) { + if (opts.root && display.startsWith(opts.root)) { + display = display.slice(opts.root.length); + } + if (path.isAbsolute(display)) { + display = path.basename(display); + } + } this.error( - new FileError(`No such file${e.path ? `: ${e.path}` : ''}`, e) + new FileError(`No such file${display ? `: ${display}` : ''}`, e) ); } else { this.error(e); // Throw error if not done in callback