Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bundling fails without read permission to parent directories #938

Closed
mitschabaude opened this issue Mar 9, 2021 · 18 comments · Fixed by #942
Closed

Bundling fails without read permission to parent directories #938

mitschabaude opened this issue Mar 9, 2021 · 18 comments · Fixed by #942

Comments

@mitschabaude
Copy link

Hi, I am trying to use esbuild on an Ubuntu VPS where my $HOME directory is /usr/home/<USER>, but I don't have read permission to the parent folder /usr/home.

This causes bundling to fail with Cannot read directory ... permission denied

The error can be reproduced on any machine by mimicking the permissions layout:

sudo mkdir forbidden && sudo mkdir forbidden/repo
sudo chmod o-rw forbidden && sudo chmod a+w forbidden/repo
cd forbidden/repo
npm init -y
npm i esbuild react
echo 'import "react"' | npx esbuild --bundle

Output:

 > error: Cannot read directory "..": permission denied

 > <stdin>:1:7: error: Could not resolve "react" (mark it as external to exclude it from the bundle)

From my (ignorant) viewpoint it seems unnecessary to read anything above the package root when resolving node modules, so I thought esbuild could possibly prevent tripping up in this situation. (E.g. Webpack bundles without errors in the same situation.)

@kzc
Copy link
Contributor

kzc commented Mar 9, 2021

I did notice something similar for output file paths. Here's a contrived example for demonstration purposes:

$ esbuild --version
0.9.0
$ echo 'console.log(1 ? 2 : 3)' > 0.js
$ tty
/dev/ttys000
$ esbuild 0.js --minify --outfile=`tty`
console.log(2);

  ../../../../../dev/ttys000  16b 

⚡ Done in 2ms

Side note... not crazy about the need for the extra typing needed to disable the verbose output:

$ esbuild 0.js --minify --outfile=`tty` --log-level=error
console.log(2);

Although I'd prefer the way esbuild used to work with UNIX-style "silent by default except for errors", might you consider having --silent to be an alias for --log-level=error? Rollup is also verbose by default now (ugh) but it does feature a --silent option.

@nettybun
Copy link

nettybun commented Mar 10, 2021

@kzc Don't use --outfile=`tty` and it'll be silent.

$ esbuild --version
0.9.0
$ esbuild 0.js --minify
console.log(2);

@kzc
Copy link
Contributor

kzc commented Mar 10, 2021

@heyheyhello I prefaced my comment with:

Here's a contrived example for demonstration purposes:

@nettybun
Copy link

Yeah and your contrived example is great to show the ../../../../../ weirdness ✅ but if you care about esbuild's stdout for unix-pipe reasons then it should be good about that by default?

@kzc
Copy link
Contributor

kzc commented Mar 10, 2021

It was a side note based on the previous command. Here's a simpler example:

Compare the length of this command:

$ esbuild 0.js --outfile=out.js

  out.js  24b 

⚡ Done in 4ms

to this one:

$ esbuild 0.js --outfile=out.js --log-level=error
$ 

Once anyone has used esbuild once they never need to see the verbose output again and will have to type --log-level=error in all their build scripts for future projects to not clutter the console or their log files. Is it a big deal? No, it's not. It's just a minor annoyance. I just prefer more succinct commands and explicit opt-ins for nonessential information.

@nettybun
Copy link

I think the use case you're imagining is very different than a lot of developers... Personally I want to see the build size everytime I build. When esbuild used to not show it the first thing I'd do after was an ls -lh. I guess "essential" is subjective. I agree about adding a --silent flag though

@evanw
Copy link
Owner

evanw commented Mar 10, 2021

This causes bundling to fail with Cannot read directory ... permission denied

Thanks for reporting this issue. I'm aware of the issue and it's come up once before here: #803 (comment). I gave fixing it a quick try then but I was unable to reproduce the issue myself (didn't try too hard at the time) and gave up. Even your repro instructions fail to cause the issue for me. I can give it another shot though.

The underlying reason is that esbuild makes heavy use of caching to avoid huge performance issues with node's path resolution algorithm. The algorithm is inherently inefficient because it involves making potentially hundreds of file system queries to resolve each individual import path (check all implicit file extensions * check package.json and implicit index files * check node_modules in all parent directories).

My implementation caches file system syscalls in the resolver to try to make this as efficient as possible. The cache in the resolver currently works by ensuring the parent directory is cached before caching a child directory, but that doesn't work if a parent directory is off-limits. Without investigating this myself first, I'm not totally sure what would need to change about the caching strategy to fix this problem correctly. But I agree that it should be fixed.

@mitschabaude
Copy link
Author

Cool, thanks for the explanation!

@ Repro: For context, the commands above were run on Ubuntu 20.10, several times with the unreadable directory at different levels above the root, and always caused the issue for me.

@kzc
Copy link
Contributor

kzc commented Mar 10, 2021

repro:

$ mkdir -p beep/boop/bop
$ cd beep/boop/bop
$ chmod 000 ..
$ echo 'console.log(1 ? 2 : 3)' > 0.js
$ esbuild --version
0.9.0
$ cat 0.js
console.log(1 ? 2 : 3)
$ esbuild ./0.js --minify
 > error: Cannot read directory "..": permission denied

 > error: Cannot read directory ".": permission denied

 > error: Cannot read directory ".": permission denied

 > error: Could not resolve "./0.js"

4 errors
$ chmod 700 ..
$ esbuild ./0.js --minify
console.log(2);

@kzc
Copy link
Contributor

kzc commented Mar 10, 2021

TIL...

$ chmod 000 ..
$ cat 0.js
console.log(1 ? 2 : 3)
$ node ./0.js 
internal/bootstrap/switches/does_own_process_state.js:129
    cachedCwd = rawMethods.cwd();
                           ^

Error: EACCES: permission denied, uv_cwd

@evanw
Copy link
Owner

evanw commented Mar 10, 2021

I can reproduce the issue if I use chmod 000 ... However, nothing else works in that state either:

$ mkdir -p some/dir
$ cd some/dir
$ echo {} > package.json
$ npm i esbuild webpack webpack-cli parcel@next rollup
$ echo 'import "./1.mjs"' > 0.mjs
$ echo 'console.log("works")' > 1.mjs
$ node 0.mjs 
works

$ chmod 000 ..

$ node_modules/.bin/esbuild --bundle 0.mjs 
 > error: Cannot read directory "../..": permission denied

 > error: Cannot read directory ".": permission denied

 > error: Could not resolve "0.mjs"

3 errors

$ node 0.mjs 
internal/bootstrap/switches/does_own_process_state.js:129
    cachedCwd = rawMethods.cwd();
                           ^

Error: EACCES: permission denied, uv_cwd
    at process.wrappedCwd [as cwd] (internal/bootstrap/switches/does_own_process_state.js:129:28)
    at Object.resolve (path.js:978:47)
    at resolveMainPath (internal/modules/run_main.js:12:40)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:66:24)
    at internal/main/run_main_module.js:17:47 {
  errno: -13,
  code: 'EACCES',
  syscall: 'uv_cwd'
}

$ node_modules/.bin/webpack ./0.mjs 
internal/bootstrap/switches/does_own_process_state.js:129
    cachedCwd = rawMethods.cwd();
                           ^

Error: EACCES: permission denied, uv_cwd
    at process.wrappedCwd [as cwd] (internal/bootstrap/switches/does_own_process_state.js:129:28)
    at Object.resolve (path.js:978:47)
    at resolveMainPath (internal/modules/run_main.js:12:40)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:66:24)
    at internal/main/run_main_module.js:17:47 {
  errno: -13,
  code: 'EACCES',
  syscall: 'uv_cwd'
}

$ node_modules/.bin/parcel build 0.mjs
internal/bootstrap/switches/does_own_process_state.js:129
    cachedCwd = rawMethods.cwd();
                           ^

Error: EACCES: permission denied, uv_cwd
    at process.wrappedCwd [as cwd] (internal/bootstrap/switches/does_own_process_state.js:129:28)
    at Object.resolve (path.js:978:47)
    at resolveMainPath (internal/modules/run_main.js:12:40)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:66:24)
    at internal/main/run_main_module.js:17:47 {
  errno: -13,
  code: 'EACCES',
  syscall: 'uv_cwd'
}

$ node_modules/.bin/rollup 0.mjs 
internal/bootstrap/switches/does_own_process_state.js:129
    cachedCwd = rawMethods.cwd();
                           ^

Error: EACCES: permission denied, uv_cwd
    at process.wrappedCwd [as cwd] (internal/bootstrap/switches/does_own_process_state.js:129:28)
    at Object.resolve (path.js:978:47)
    at resolveMainPath (internal/modules/run_main.js:12:40)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:66:24)
    at internal/main/run_main_module.js:17:47 {
  errno: -13,
  code: 'EACCES',
  syscall: 'uv_cwd'
}

So it's not clear what esbuild's behavior should be in that case. It sort of seems like it's fine for it to not work in that scenario of node itself doesn't even work.

Note that this is on macOS.

@mitschabaude
Copy link
Author

mitschabaude commented Mar 10, 2021

Ha, I just tried the exact same steps, but with chmod 111 .. in the place of chmod 000 .., and then everything works except esbuild :)

This is fun, I'm learning some things about permissions (no expert here either). chmod 111 means the x (execute) permission is there but nothing else. If xis missing, you can't even cd into the folder and it seems to break a lot. Makes sense that a VPS like the one mentioned above would give x but not rw to the /home folder.

@kzc
Copy link
Contributor

kzc commented Mar 10, 2021

chmod 111 means the x (execute) permission is there but nothing else. If x is missing, you can't even cd into the folder and it seems to break a lot

TIL the getcwd libc call fails if any of its parent directories do not have execute permission...

$ cat cwd.c
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
int main() {
    char buf[512];
    char* cwd = getcwd(buf, sizeof(buf) - 1);
    int err = errno;
    printf("%s\n", cwd ? cwd : strerror(err));
    return err;
}
$ cc cwd.c -o a.out
$ chmod 700 ..
$ ./a.out 
/tmp/esbuild/esbuild/beep/boop/bop
$ chmod 000 ..
$ ./a.out 
Permission denied
$ chmod 600 ..
$ ./a.out 
Permission denied
$ chmod 100 ..
$ ./a.out 
/tmp/esbuild/esbuild/beep/boop/bop

@evanw
Copy link
Owner

evanw commented Mar 10, 2021

Thanks for all the help. Now I finally understand the problem. I think an appropriate fix should be to just treat this directory as empty, since node seems to let you still import packages declared in the parent directory of the inaccessible directory. Treating it as empty means esbuild will simply pass through it on its search for packages. Working on a fix now.

@evanw
Copy link
Owner

evanw commented Mar 10, 2021

Although I'd prefer the way esbuild used to work with UNIX-style "silent by default except for errors", might you consider having --silent to be an alias for --log-level=error? Rollup is also verbose by default now (ugh) but it does feature a --silent option.

@kzc regarding the tangent about the new summary logging after builds: I understand the desire to have clean terminal output. Here is my rationale for the change, from the most recent release notes:

I'm going to try this because I believe it will improve the UX. People have this problem with esbuild when they first try it where it runs so quickly that they think it must be broken, only to later discover that it actually worked fine. While this is funny, it seems like a good indication that the UX could be improved. So I'm going to try automatically printing a summary to see how that goes.

Some examples of people encountering this problem in the wild:

I'm turning it on by default because I believe that doing this improves the UX in the majority of cases, and it's easy to turn off if you don't want it. I expect most uses of esbuild to be done through code (npm scripts or API calls) instead of manually-typed commands, in which case adding a parameter to disable it is not a problem. And I expect that in the majority of cases printing the summary for manually-typed commands is the right thing to do, in which case adding a parameter to disable it is the minority use case.

I think adding --silent isn't great for a few reasons. Having it map to --log-level=error instead of --log-level=silent is pretty confusing IMO. I'm also not keen on having multiple ways of doing things from an API design perspective. And it sounds like you actually never want this to happen, in which case it is still sub-optimal to continually add --silent every time you manually type a command.

Given these concerns, I think a better way would be to support an environment variable such as ESBUILD_LOG_LEVEL=error. Then you could just set it once in your shell setup and never have this problem again from that point onward. It's not discoverable but I suspect it's something few people would need, so I'm ok with people having to ask about it to discover it. Normally I wouldn't consider adding extra preferences like this but I think this case could be a valid exception, especially because the change is purely cosmetic (doesn't affect behavior) and because it's a very small addition that's easy to maintain.

@kzc
Copy link
Contributor

kzc commented Mar 10, 2021

I think a better way would be to support an environment variable such as ESBUILD_LOG_LEVEL=error

@evanw thanks for the explanation and consideration. And thanks for esbuild and the long hours you put into it each week making it rock solid and thoughtfully responding to everyone. I wonder how you find the time.

No need to add an environment variable on my account. With the env var you'd have the opposite problem - remembering to unset it on the rare occasions you'd actually like to see a warning. As I mentioned, it's just a minor annoyance. For command line usage I can create an alias for esbuild --log-level=error in ~/.profile. In the worst case I can hack a local copy of esbuild to suit my needs. Love that open source.

Just curious... Regarding changing the absolute paths of inputs and outputs to relative paths from the current working directory - is that intentional and also related to the node resolve algorithm?

@evanw
Copy link
Owner

evanw commented Mar 10, 2021

Just curious... Regarding changing the absolute paths of inputs and outputs to relative paths from the current working directory - is that intentional and also related to the node resolve algorithm?

It's intentional but not related to path resolution. Paths are always resolved to absolute paths internally but are pretty-printed as relative paths in error messages. This is both because relative paths are usually shorter than absolute paths and because this makes error messages deterministic across platforms instead of encoding platform-specific differences such as the user's home directory and/or C:\ vs. / as a file system root.

This was referenced Mar 15, 2021
@cuzfrog
Copy link

cuzfrog commented Aug 14, 2023

@evanw this fix seems not including some cross build problem (Windows), somehow err does not match EACCES or EPERM in my case.

Reproduce it in a Windows10x64 VM:

folder access setup:
image

logs (I update the code to output err in the log, you can see the code is 5):

$ esbuild --log-level=verbose test.js
X [ERROR] Cannot read directory "..": Access is denied. , error code: 5

♦ [VERBOSE] Resolving import "./test.js" in directory "B:\\esbuild_test\\parent_not_readable\\myproject" of type "entry-point"

  Read 4 entries for directory "B:\\"
  Read 1 entry for directory "B:\\esbuild_test"
  Failed to read directory "B:\\esbuild_test\\parent_not_readable": open B:\esbuild_test\parent_not_readable: Access is denied.
  Failed to read directory "B:\\esbuild_test\\parent_not_readable"
  Failed to read directory "B:\\esbuild_test\\parent_not_readable\\myproject"
  Failed to read directory "B:\\esbuild_test\\parent_not_readable\\myproject"
  Failed to read directory "B:\\esbuild_test\\parent_not_readable\\myproject"
  Attempting to load "B:\\esbuild_test\\parent_not_readable\\myproject\\test.js" as a file
    Checking for file "test.js"
    Found file "test.js"
  Failed to read directory "B:\\esbuild_test\\parent_not_readable\\myproject"
  Primary path is "B:\\esbuild_test\\parent_not_readable\\myproject\\test.js" in namespace "file"

1 error

A locally built 0.18.20 version in a Windows10 VM can work in that VM, but a same version installed by npm cannot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants