Node: fs.lchown should be undeprecated

[simevo]

• Version: v8.9.3
• Platform: Linux xxx 4.15.0-2-amd64 #1 SMP Debian 4.15.11-1 (2018-03-20) x86_64 GNU/Linux
• Subsystem: fs

fs.lchown is currently deprecated

But fs.lchown can be helpful to address Time of check to time of use (TOCTOU) bugs, like this one,
as it does not dereference symbolic links and merely changes the owner of the link.

Please undeprecate fs.lchown and implement it on the linux platform.

[ryzokuken]

@nodejs/fs thoughts on this?

@simevo you sure there's no alternative?

[addaleax]

I’m surprised to hear it’s deprecated and only implemented on some platforms, tbh.

[ryzokuken]

Strange. I'm equally surprised to hear that it's implemented on Windows and not on Linux. I mean, it's chown. How do you even not implement chown on linux? 😛

@addaleax that said, if we decide to go ahead and add this functionality, I'd love to take a shot at it.

[anliting]

@ryzokuken

I don't think @simevo means it is implemented on Windows. As https://github.com/nodejs/node-v0.x-archive/issues/7382, it is only implemented on macOS.

[ryzokuken]

@anliting I just found that out myself, and needless to say, it makes a lot of sense.

Could we somehow implement this functionality on Windows too? I'd love if we could make this platform-independent somehow. Let me look into it.

[bnoordhuis]

Related: #16695

I’m surprised to hear it’s deprecated and only implemented on some platforms, tbh.

There is no way to implement it with old Linux kernels: fchmodat() was added in 2.6.16 and it was until recently that we still still supported 2.6.9.

2.6.18 is the baseline now so that's no longer a blocker.

[ryzokuken]

@bnoordhuis Sounds great!

Now that the baseline has moved, should someone start working on making it work on Linux? We'd probably have to un-deprecate the function before that though, wouldn't we?

Also, does that mean it'd be semver-major? Or would it be semver-minor because we're not really removing anything, just adding new functionality?

[bnoordhuis]

Now that the baseline has moved, should someone start working on making it work on Linux?

Yes. It should be added to libuv first.

We'd probably have to un-deprecate the function before that though, wouldn't we?

No, the other way around. It was deprecated because it's platform-specific. It should be un-deprecated only when that's no longer true.

would it be semver-minor because we're not really removing anything, just adding new functionality?

Yep, I'd say so.

[ryzokuken]

Great! I think @chris--young had been working on adding it to libuv? If there's no open PR regarding this in there, I could try looking into it, or submit an issue. That way, we could go with whichever way helps us ship this quicker.

[bnoordhuis]

I don't remember any libuv PRs. A quick search doesn't turn up anything either.

[ryzokuken]

@bnoordhuis in that case, I'd make an issue on libuv and start looking into it myself as well.

[simevo]

Hi please note that libuv now has lchown, see: https://github.com/libuv/libuv/pull/1826#issuecomment-398416155