Use fs.chmod when target is not a symlink

[mbargiel]

This PR fixes EACCES error when attempting to use chmodr on a non-writable file. (#19)

There weren't any chmod tests on files (aside from symlinks), so I wasn't sure how/where to write them. I'll gladly add them if you can provide me with a suggestion on new test file names, or whether I should modify existing tests, etc.

[isaacs]

Unfortunately, this reintroduces a TOCTOU vulnerability, since the file can change from a regular file to a symbolic link in the time between the stat and the chmod. (This exists for the directory check as well, but it's fundamentally unavoidable there.)

Another option would be to detect and catch the lchmod error and then fall back to chmod instead.

Really, though, this is a bug in node core. There is no need to open the file in a writable mode to do fchmod on it. The flags there should be O_SYMLINK only, not O_WRONLY | O_SYMLINK.

[mbargiel]

(Thanks for the super fast response!)

I understand. But even if we catch the lchmod error and fallback to chmod, the race condition is still present, no? I mean, between the failure to open the file, and the use of chmod...

Another option might be to reimplement a lchmod that does not use the O_WRONLY flag (I mean, barring fixing node and expecting this to be backported to old enough versions in time for this to be of use to the few number of people who are affected by this...)

In any case, a fix would be useful, because if you use chmodr on a readonly file (or a directory containing a readonly file at any level), it will always fail, basically defeating one of the main use cases for chmodr.

[mbargiel]

8a1b4c0 reverts the first commit and implements your suggestion:

Another option would be to detect and catch the lchmod error and then fall back to chmod instead.

(No worries, I'll squash once approved.)

[isaacs]

Bug reported to node core: nodejs/node#23736

[mbargiel]

Do you prefer to wait for the actual fix in node or is this fix acceptable in the meantime? If you prefer to reject the PR, I can live with my fork in the meantime, the risk is small enough for my use case.