Add an option to prevent following symlinks

[nlf]

trafficstars

Currently, if a symlink is placed in the directory that inert shares files from and the user requests the symlink, the contents of the targeted file will be retrieved.

It is common practice to have an option to disable following symlinks and instead respond with a 404. We should add one.

[kanongil]

Example from nginx: http://nginx.org/en/docs/http/ngx_http_core_module.html#disable_symlinks

It is quite expensive to check, as each path component will need a stat lookup. Additionally, a safe implementation of such an option will require openat() and fstatat() support from the host. This means that it will never work on Windows, and will require native code (or node implementation of https://github.com/nodejs/node/issues/31110) to interface with the syscalls.

Given this, I vote that this feature is dropped.

[Nargonath]

Given what you stated I agree, also the initial problem stems in user land as it is his responsability not to create a symlink in the served directory to a private file AFAICT.

[Marsup]

If I understand the problem well enough, can't you use fs.realpath and compare the result with the provided path to make sure they're equal ? There's no mention in the docs about cross OS compatibility so I'm not sure about that part.

[kanongil]

fs.realpath can be used to implement the feature, but not in a safe way. Since it operates on the path, there will always be a window of opportunity where it can have returned a "wrong" value.

1. Get realpath.
2. Open file at resolved path.

Between 1 and 2, the filesystem can have been manipulated to include a symlink in the resolved path, causing the open to point to eg. /etc/passwd.

[Marsup]

The timing is highly unlikely though, I would consider it secure enough.

[nlf]

It being expensive is not a reason to not implement it. It's a reason to default it to off, but it doesn't mean we shouldn't have the option.

As for cross platform concerns, we support what node core supports. If node doesn't provide us an interface to support windows properly, then we note in the documentation that windows may not support the feature correctly.

I agree with @Marsup that the timing situation between comparing the result of fs.realpath to the provided path immediately before reading the file is probably acceptable. You'd have to tie up the event loop for quite a few cycles for that to introduce a gap big enough for it to be meaningful, and if someone can do that you've got bigger problems than them potentially following a symlink that you placed there.