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

Do we need node-fs-extra? #140

Open
arielshaqed opened this issue Nov 12, 2017 · 1 comment
Open

Do we need node-fs-extra? #140

arielshaqed opened this issue Nov 12, 2017 · 1 comment

Comments

@arielshaqed
Copy link
Contributor

In the #130 review @michaeladda and @rylandg discussed some rough edges in using node-fs-extra to copy files. The ncp function there is suspect: apart from verifying existence to decide whether to delete before copying instead of opening with O_WRONLY|O_TRUNC (a classic TOCTTOU error), it also copies file permissions across (https://github.com/jprichardson/node-fs-extra/blob/a37d7bbe1c62e5fc2c20212678f4bbd41fc9e202/lib/copy/ncp.js#L99). This is plain wrong: the source file might not even share user and groups, so copying mode won't work. And umask should anyway apply to the mode, so it most likely won't even do the wrong thing it wants to do.
Do we need to use this? It exhibits unPosixan behaviour, and is very surprising to the end-user.

@rylandg
Copy link
Contributor

rylandg commented Nov 13, 2017

I will explore this but it's very low priority for me.

That being said. If we do choose to omit using it we should probably enforce that org wise?

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

No branches or pull requests

2 participants