Skip to content

Commit

Permalink
Disable symlinks by default (#84)
Browse files Browse the repository at this point in the history
* Disable symlinks by default

* Added comment
  • Loading branch information
leo committed Apr 3, 2019
1 parent d1368bf commit 360d3df
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 31 deletions.
18 changes: 17 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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.
Expand All @@ -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) {}
Expand Down
35 changes: 25 additions & 10 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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;

Expand Down
1 change: 1 addition & 0 deletions test/fixtures/symlinks/package.json
72 changes: 52 additions & 20 deletions test/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});

Expand Down Expand Up @@ -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);
}
});

Expand Down Expand Up @@ -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
});

Expand Down Expand Up @@ -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);
}
});

Expand Down Expand Up @@ -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);
}
});

Expand All @@ -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);
}
Expand All @@ -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);
}
});

Expand All @@ -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);
}
});

Expand Down Expand Up @@ -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: '*',
Expand Down Expand Up @@ -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);
});

0 comments on commit 360d3df

Please sign in to comment.