Disable symlinks by default

README.md

@@ -58,6 +58,7 @@ You can use any of the following options:
 | [`unlisted`](#unlisted-array) | Exclude paths from the directory listing |
 | [`trailingSlash`](#trailingslash-boolean) | Remove or add trailing slashes to all paths |
 | [`renderSingle`](#rendersingle-boolean) | If a directory only contains one file, render it |
+| [`symlinks`](#symlinks-boolean) | Resolve symlinks instead of rendering a 404 error |
 
 ### public (String)
 
@@ -259,6 +260,20 @@ This is disabled by default and can be enabled like this:
 
 After that, if you access your directory `/test` (for example), you will see an image being rendered if the directory contains a single image file.
 
+### symlinks (Boolean)
+
+For security purposes, symlinks are disabled by default. If `serve-handler` encounters a symlink, it will treat it as if it doesn't exist in the first place. In turn, a 404 error is rendered for that path.
+
+However, this behavior can easily be adjusted:
+
+```js
+{
+ "symlinks": true
+}
+```
+
+Once this property is set as shown above, all symlinks will automatically be resolved to their targets.
+
 ## Error templates
 
 The handler will automatically determine the right error format if one occurs and then sends it to the client in that format.
@@ -275,7 +290,8 @@ These are the methods used by the package (they can all return a `Promise` or be
 
 ```js
 await handler(request, response, undefined, {
- stat(path) {},
+ lstat(path) {},
+ realpath(path) {},
  createReadStream(path, config) {}
  readdir(path) {},
  sendError(absolutePath, response, acceptsJSON, root, handlers, config, error) {}

src/index.js

@@ -1,7 +1,7 @@
 // Native
 const {promisify} = require('util');
 const path = require('path');
-const {stat, createReadStream, readdir} = require('fs');
+const {realpath, lstat, createReadStream, readdir} = require('fs');
 
 // Packages
 const url = require('fast-url-parser');
@@ -327,10 +327,10 @@ const renderDirectory = async (current, acceptsJSON, handlers, methods, config,
  // simulating those calls and needs to special-case this.
  let stats = null;
 
- if (methods.stat) {
- stats = await handlers.stat(filePath, true);
+ if (methods.lstat) {
+ stats = await handlers.lstat(filePath, true);
  } else {
- stats = await handlers.stat(filePath);
+ stats = await handlers.lstat(filePath);
  }
 
  details.relative = path.join(relativePath, details.base);
@@ -466,7 +466,7 @@ const sendError = async (absolutePath, response, acceptsJSON, current, handlers,
  const errorPage = path.join(current, `${statusCode}.html`);
 
  try {
- stats = await handlers.stat(errorPage);
+ stats = await handlers.lstat(errorPage);
  } catch (err) {
  if (err.code !== 'ENOENT') {
  console.error(err);
@@ -512,7 +512,8 @@ const internalError = async (...args) => {
 };
 
 const getHandlers = methods => Object.assign({
- stat: promisify(stat),
+ lstat: promisify(lstat),
+ realpath: promisify(realpath),
  createReadStream,
  readdir: promisify(readdir),
  sendError
@@ -580,7 +581,7 @@ module.exports = async (request, response, config = {}, methods = {}) => {
 
  if (path.extname(relativePath) !== '') {
  try {
- stats = await handlers.stat(absolutePath);
+ stats = await handlers.lstat(absolutePath);
  } catch (err) {
  if (err.code !== 'ENOENT') {
  return internalError(absolutePath, response, acceptsJSON, current, handlers, config, err);
@@ -592,7 +593,7 @@ module.exports = async (request, response, config = {}, methods = {}) => {
 
  if (!stats && (cleanUrl || rewrittenPath)) {
  try {
- const related = await findRelated(current, relativePath, rewrittenPath, handlers.stat);
+ const related = await findRelated(current, relativePath, rewrittenPath, handlers.lstat);
 
  if (related) {
  ({stats, absolutePath} = related);
@@ -606,7 +607,7 @@ module.exports = async (request, response, config = {}, methods = {}) => {
 
  if (!stats) {
  try {
- stats = await handlers.stat(absolutePath);
+ stats = await handlers.lstat(absolutePath);
  } catch (err) {
  if (err.code !== 'ENOENT') {
  return internalError(absolutePath, response, acceptsJSON, current, handlers, config, err);
@@ -652,7 +653,12 @@ module.exports = async (request, response, config = {}, methods = {}) => {
  }
  }
 
- if (!stats) {
+ const isSymLink = stats && stats.isSymbolicLink();
+
+ // There are two scenarios in which we want to reply with
+ // a 404 error: Either the path does not exist, or it is a
+ // symlink while the `symlinks` option is disabled (which it is by default).
+ if (!stats || (!config.symlinks && isSymLink)) {
  // allow for custom 404 handling
  return handlers.sendError(absolutePath, response, acceptsJSON, current, handlers, config, {
  statusCode: 404,
@@ -661,6 +667,14 @@ module.exports = async (request, response, config = {}, methods = {}) => {
  });
  }
 
+ // If we figured out that the target is a symlink, we need to
+ // resolve the symlink and run a new `stat` call just for the
+ // target of that symlink.
+ if (isSymLink) {
+ absolutePath = await handlers.realpath(absolutePath);
+ stats = await handlers.lstat(absolutePath);
+ }
+
  const streamOpts = {};
 
  // TODO ? if-range
@@ -669,6 +683,7 @@ module.exports = async (request, response, config = {}, methods = {}) => {
 
  if (typeof range === 'object' && range.type === 'bytes') {
  const {start, end} = range[0];
+
  streamOpts.start = start;
  streamOpts.end = end;
 

test/fixtures/symlinks/package.json

@@ -0,0 +1 @@
+../../../package.json

test/integration.js

@@ -129,14 +129,14 @@ test('render json sub directory listing with custom stat handler', async t => {
 
  // eslint-disable-next-line no-undefined
  const url = await getUrl(undefined, {
- stat: (location, isDirectoryListing) => {
+ lstat: (location, isDirectoryListing) => {
  if (contents.includes(path.basename(location))) {
  t.true(isDirectoryListing);
  } else {
  t.falsy(isDirectoryListing);
  }
 
- return fs.stat(location);
+ return fs.lstat(location);
  }
  });
 
@@ -207,13 +207,13 @@ test('try to render non-existing json file and `stat` errors', async t => {
 
  // eslint-disable-next-line no-undefined
  const url = await getUrl(undefined, {
- stat: location => {
+ lstat: location => {
  if (path.basename(location) === name && !done) {
  done = true;
  throw new Error(message);
  }
 
- return fs.stat(location);
+ return fs.lstat(location);
  }
  });
 
@@ -514,7 +514,7 @@ test('pass custom handlers', async t => {
 
  // eslint-disable-next-line no-undefined
  const url = await getUrl(undefined, {
- stat: fs.stat,
+ lstat: fs.lstat,
  createReadStream: fs.createReadStream
  });
 
@@ -617,12 +617,12 @@ test('receive custom `404.html` error page', async t => {
 test('error is still sent back even if reading `404.html` failed', async t => {
  // eslint-disable-next-line no-undefined
  const url = await getUrl(undefined, {
- stat: location => {
+ lstat: location => {
  if (path.basename(location) === '404.html') {
  throw new Error('Any error occured while checking the file');
  }
 
- return fs.stat(location);
+ return fs.lstat(location);
  }
  });
 
@@ -771,12 +771,12 @@ test('set `cleanUrls` config property to `true` and an error occurs', async t =>
  const url = await getUrl({
  cleanUrls: true
  }, {
- stat: location => {
+ lstat: location => {
  if (path.basename(location) === 'index.html') {
  throw new Error(message);
  }
 
- return fs.stat(location);
+ return fs.lstat(location);
  }
  });
 
@@ -798,7 +798,7 @@ test('error occurs while getting stat of path', async t => {
 
  // eslint-disable-next-line no-undefined
  const url = await getUrl(undefined, {
- stat: location => {
+ lstat: location => {
  if (path.basename(location) !== '500.html') {
  throw new Error(message);
  }
@@ -817,32 +817,32 @@ test('error occurs while getting stat of path', async t => {
  t.is(text, content);
 });
 
-test('the first `stat` call should be for a related file', async t => {
+test('the first `lstat` call should be for a related file', async t => {
  let done = null;
 
  // eslint-disable-next-line no-undefined
  const url = await getUrl(undefined, {
- stat: location => {
+ lstat: location => {
  if (!done) {
  t.is(path.basename(location), 'index.html');
  done = true;
  }
 
- return fs.stat(location);
+ return fs.lstat(location);
  }
  });
 
  await fetch(url);
 });
 
-test('the `stat` call should only be made for files and directories', async t => {
+test('the `lstat` call should only be made for files and directories', async t => {
  const locations = [];
 
  // eslint-disable-next-line no-undefined
  const url = await getUrl(undefined, {
- stat: location => {
+ lstat: location => {
  locations.push(location);
- return fs.stat(location);
+ return fs.lstat(location);
  }
  });
 
@@ -857,12 +857,12 @@ test('error occurs while getting stat of not-found path', async t => {
 
  // eslint-disable-next-line no-undefined
  const url = await getUrl(undefined, {
- stat: location => {
+ lstat: location => {
  if (path.basename(location) === base) {
  throw new Error(message);
  }
 
- return fs.stat(location);
+ return fs.lstat(location);
  }
  });
 
@@ -1124,8 +1124,8 @@ test('range request without size', async t => {
  };
 
  const url = await getUrl(config, {
- stat: async location => {
- const stats = await fs.stat(location);
+ lstat: async location => {
+ const stats = await fs.lstat(location);
 
  config.headers.unshift({
  source: '*',
@@ -1297,3 +1297,35 @@ test('prevent access to parent directory', async t => {
 
  t.is(text.trim(), '<span>Not Found</span>');
 });
+
+test('symlinks should not work by default', async t => {
+ const name = 'symlinks/package.json';
+ const url = await getUrl();
+
+ const response = await fetch(`${url}/${name}`);
+ const text = await response.text();
+
+ t.is(response.status, 404);
+ t.is(text.trim(), '<span>Not Found</span>');
+});
+
+test('allow symlinks by setting the option', async t => {
+ const name = 'symlinks/package.json';
+ const related = path.join(fixturesFull, name);
+ const content = await fs.readFile(related);
+
+ const url = await getUrl({
+ symlinks: true
+ });
+
+ const response = await fetch(`${url}/${name}`);
+ const length = Number(response.headers.get('content-length'));
+
+ t.is(length, content.length);
+ t.is(response.status, 200);
+
+ const text = await response.text();
+ const spec = content.toString();
+
+ t.is(text, spec);
+});