Cannot read files from /proc/self/fd since Deno 1.43

[felipecrs]

Version: Deno 1.43.1

https://github.com/felipecrs/deno-repro-fd-perm

$ git clone https://github.com/felipecrs/deno-repro-fd-perm

$ cd deno-repro-fd-perm

$ pkgx [email protected] run --allow-read main.ts <(echo felipe)
felipe

$ pkgx [email protected] run --allow-read main.ts <(echo felipe)
felipe

$ pkgx [email protected] run --allow-read main.ts <(echo felipe)
error: Uncaught (in promise) PermissionDenied: permission denied: readfile '/proc/self/fd/16'
const text = await Deno.readTextFile(file);
                        ^
    at Object.readTextFile (ext:deno_fs/30_fs.js:878:24)
    at file:///home/felipecrs/repos/deno-repro-fd-perm/main.ts:7:25

[mmastrac]

With some changes in 1.43 this unfortunately requires --allow-all now, as we don't currently have a way to discriminate between pipes and files in /dev/fd and /proc/self/fd. I think we may be able to improve this situation.

[NeKzor]

Looks like a lot more is now disallowed which is a big breaking change.

Here is an example from the docs:

$ deno run --allow-read=/etc https://deno.land/[email protected]/examples/cat.ts /etc/passwd
error: Uncaught (in promise) PermissionDenied: permission denied: open '/etc/passwd'
  const file = await Deno.open(filename);
                          ^
    at Object.open (ext:deno_fs/30_fs.js:633:21)
    at https://deno.land/[email protected]/examples/cat.ts:10:27

Moving from --allow-read to --allow-all is very questionable.

[felipecrs]

I personally don't mind adding more granularity to the --allow flags (supposing some new flag will come to allow these extra use cases).

I just don't think it's a good idea to do it in Deno 1.x.

[bartlomieju]

Looks like a lot more is now disallowed which is a big breaking change.

Here is an example from the docs:

$ deno run --allow-read=/etc https://deno.land/[email protected]/examples/cat.ts /etc/passwd
error: Uncaught (in promise) PermissionDenied: permission denied: open '/etc/passwd'
  const file = await Deno.open(filename);
                          ^
    at Object.open (ext:deno_fs/30_fs.js:633:21)
    at https://deno.land/[email protected]/examples/cat.ts:10:27

Moving from --allow-read to --allow-all is very questionable.

This has been relaxed by #23718 and will work again in v1.43.2.

I personally don't mind adding more granularity to the --allow flags (supposing some new flag will come to allow these extra use cases).

I just don't think it's a good idea to do it in Deno 1.x.

That's true, but we had to do it because of the security vulnerability that you can see at GHSA-23rx-c3g5-hv9w.

[felipecrs]

Maybe supporting --allow-read=/dev/fd or --allow-read=/proc/self/fd is enough? User would be being explicit about it.

[lucacasonato]

@felipecrs Read access to /dev/fd allows bypassing all kinds of --allow-* permissions - so we are unlikely to reconsider for /dev/fd or /dev/self/fd. For more info, see: GHSA-23rx-c3g5-hv9w

[felipecrs]

I see. -A then. Feel free to close this issue.

[mmastrac]

I think we might be able to make this work without relaxing the security sandbox -- we'll allow opening FD magic links on unix systems, but only if they are not stdio, and are pipes.