denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
94.48k stars 5.24k forks source link

Using fs on a network drive requires more permissions on Windows than Mac, and it's unclear what the extra permission is. #24703

Open ZYinMD opened 2 months ago

ZYinMD commented 2 months ago

Version: Deno 1.45.3

I have a NAS, say the server name is MY_NAS, and it shares a folder foo over SMB.

On mac, to visit foo in Finder, I should press ⌘K and input smb://MY_NAS/foo. To visit in CLI, I need to use the path /Volumes/foo.

On Windows, \\MY_NAS\foo can be used in both File Explorer and CLI.

The problem is when I want to operate on it using fs with deno, the permission required seems to differ on Windows and Mac.

// test.ts
if (Deno.build.os === 'windows') {
  Deno.writeTextFileSync('\\\\MY_NAS\\foo\\hello.txt', 'Hello World');
}
if (Deno.build.os === 'darwin') {
  Deno.writeTextFileSync('/Volumes/foo/hello.txt', 'Hello World');
}

deno run -A test.ts will succeed on both Windows and Mac. deno run --allow-write test.ts will fail on Windows, succeed on Mac. deno run test.ts will ask for --allow-write, pressing y will fail on Windows, succeed on Mac.

Error message on Windows:

Uncaught (in promise) PermissionDenied: permission denied: writefile '\\MY_NAS\foo\hello.txt' at writeFileSync (ext:deno_fs/30_fs.js:899:3) at Object.writeTextFileSync (ext:deno_fs/30_fs.js:962:10)

yazan-abdalrahman commented 1 month ago

@lucacasonato @ZYinMD

I think the issue is with the permissions of the sheared folder on Windows. For example, if the permissions are denied,deno run -A test.ts would fail.

image image

if permissions allowed will work image image

ZYinMD commented 1 month ago

Hi @yazan-abdalrahman , in OP, -A always had no issues, so "Windows doesn't allow it" isn't the problem. See bottom half of OP.

KarolBajkowski commented 1 month ago

@yazan-abdalrahman You have an excellent remark but I think something weird is happening that needs to be discussed. Windows Permissions tab works at the OS/filesystem level. That means if the file has Denay all permissions then it's not possible to read/write to that file because OS won't allow reading/writing from it (for any user process in the system). What I mean by that the behaviour should be consistent (the same) regardless of the programming language/platform. Recently I made some quick trivial scripts (I can share the sources if you like) using Python and Ruby and it's possible to read/write any file I like without any issue. Why does the Deno runtime require --allow-all, but --allow-read/--allow-write is not enough? It looks like Deno has some additional logic that uses Windows Permissions and that prevents --allow-read/--allow-write from working properly. Why does the runtime when it sees --allow-read/--allow-write doesn't trust the OS regarding the File Permissions and do some additional (I think wrong) logic?

yazan-abdalrahman commented 1 month ago

You have an excellent remark but I think something weird is happening that needs to be discussed. Windows Permissions tab works at the OS/filesystem level. That means if the file has Denay all permissions then it's not possible to read/write to that file because OS won't allow reading/writing from it (for any user process in the system). What I mean by that the behaviour should be consistent (the same) regardless of the programming language/platform. Recently I made some quick trivial scripts (I can share the sources if you like) using Python and Ruby and it's possible to read/write any file I like without any issue. Why does the Deno runtime require --allow-all, but --allow-read/--allow-write is not enough? It looks like Deno has some additional logic that uses Windows Permissions and that prevents --allow-read/--allow-write from working properly. Why does the runtime when it sees --allow-read/--allow-write doesn't trust the OS regarding the File Permissions and do some additional (I think wrong) logic?

@KarolBajkowski

I pushed this PR https://github.com/denoland/deno/pull/25132 to fix it and it was fixed but it's still needs confirm they thinks on windows it's needs all permission to access network files for security reasons so I told my opinion on PR comments and still needs confirm or what should I do

KarolBajkowski commented 1 month ago

@yazan-abdalrahman I looked at the PR and I think I should put more context into that issue because the described issue is fully reproducible on my machine. If I understand the PR correctly it changes the way how Deno validates the UNC paths on Windows. IMHO the problem might be deeper than that.

  1. The issue doesn't appear only using UNC paths (like in OP \\MY_NAS\foo) but also when the network drive is mapped to Windows letter (for example it could be mapped as d:\ drive). On my machine I have the same behavior OP described but using network mapped drive (in my case it's z:\)
  2. The issue is not only about writing (as in OP) but also about reading files.

So based on the above it might be only the UNC path validation issue, but to me, it doesn't necessarily have to be only there.

yazan-abdalrahman commented 1 month ago

@yazan-abdalrahman I looked at the PR and I think I should put more context into that issue because the described issue is fully reproducible on my machine. If I understand the PR correctly it changes the way how Deno validates the UNC paths on Windows. IMHO the problem might be deeper than that.

  1. The issue doesn't appear only using UNC paths (like in OP \\MY_NAS\foo) but also when the network drive is mapped to Windows letter (for example it could be mapped as d:\ drive). On my machine I have the same behavior OP described but using network mapped drive (in my case it's z:\)
  2. The issue is not only about writing (as in OP) but also about reading files.

So based on the above it might be only the UNC path validation issue, but to me, it doesn't necessarily have to be only there.

@KarolBajkowski However, this solution applies to all permission types—not just write permissions. It might also need to be excluded if the network drive is mapped to a Windows letter using that method. The real problem, though, is that this behavior checks all permissions to see if they are granted for security reasons on Windows; if this isn't possible, then it's a bug, so it should be closed and a message to use should be printed.