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.

Sign up for GitHub

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

Jump to bottom

Symlinks problem #3

Closed
missingdays opened this issue Aug 9, 2015 · 2 comments
Closed

Symlinks problem #3

missingdays opened this issue Aug 9, 2015 · 2 comments

Comments

@missingdays
Copy link

The same issue we had with chmodr.
Script walks on symlinks and tries to change permissions on every file there.
chmdod -R doesn't do that.
#2 closes this.
@missingdays missingdays mentioned this issue Aug 9, 2015
Closed
@simevo
Copy link

simevo commented Jan 10, 2018

AFAICT this is now addressed on master since somehow the PR #2 has been actually merged. So this bug could be closed.
But the fix introduced #14.
An alternative fix would have been to use lchown rather than chown. lchown does not dereference symbolic links and merely changes the owner of the link.
Would that be acceptable ?

@simevo simevo mentioned this issue Jan 10, 2018
Closed
simevo pushed a commit to simevo/chownr that referenced this issue Jan 11, 2018
use fs.lchown rather than fs.chown and thereby fix the symlinks problem
0307bb7 
isaacs#3 while not causing the TOCTOU vulnerability isaacs#14
simevo pushed a commit to simevo/chownr that referenced this issue Aug 10, 2018
use fs.lchown rather than fs.chown and thereby fix isaacs#14
3bd6861 
fixes the symlinks problem isaacs#3 while not causing the TOCTOU vulnerability isaacs#14

The [patch in libuv 1.21.0](https://github.com/libuv/libuv/releases/tag/v1.21.0) that undeprecates `fs.lchown` [has been incorporated in nodejs Version 10.6.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V10.md#2018-07-04-version-1060-current-targos).

So I specified the minimum nodejs version in `package.json` with the `engine` key: https://docs.npmjs.com/files/package.json#engines
@simevo simevo mentioned this issue Aug 10, 2018
Closed
@isaacs
Copy link
Owner

isaacs commented Sep 17, 2018

Fixed in v1.1.0. The fix for chmodr is a bit more complicated, because lchmod is implemented in a way that cannot work on directories.

@isaacs isaacs closed this as completed Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@isaacs @missingdays @simevo