Experimental Permissions: Checks from fs functions that accept Buffers fail with TypeError

[alistairjevans]

Version

v22.5.1

Platform

Linux e0c5ff99f631 6.9.8-orbstack-00170-g7b4100b7ced4 #1 SMP Thu Jul 11 03:32:20 UTC 2024 aarch64 GNU/Linux

Subsystem

File system

What steps will reproduce the bug?

I saw this directly when when attempting a recursive delete. E.g.

script.js:

const fs = require('fs');
const pathBuffer = Buffer.from('/tmp/.mydir');
await fs.promises.rm(pathBuffer, { recursive: true })

Execute this with something like node --experimental-permission --allow-fs-write /tmp* script.js

How often does it reproduce? Is there a required condition?

This always happens provided the experimental-permission flag is set.

What is the expected behavior? Why is that the expected behavior?

Expected behaviour is that the permissions checking code allows a Buffer to be provided containing the path.

What do you see instead?

Error:

TypeError: The "reference" argument must be of type string. Received an instance of Buffer
    at Object.has (node:internal/process/permission:25:7)
    at lstat (node:fs:1551:45)
    at _rimraf (node:internal/fs/rimraf:67:3)
    at rimraf (node:internal/fs/rimraf:46:3)
    at node:internal/fs/rimraf:145:7
    at Array.forEach (<anonymous>)
    at node:internal/fs/rimraf:142:5
    at FSReqCallback.oncomplete (node:fs:187:23) {
  code: 'ERR_INVALID_ARG_TYPE'

From the looks of it, the code in .has that calls validateString needs to permit buffers as well for the reference argument.

Additional information

No response

[kevinuehara]

So the problem happens only when pass the flag --experimental-permission? In previous versions is this happens normally?

[alistairjevans]

Yes, only with --experimental-permission; that triggers the code path that checks the reference value. I haven't tested older versions, but to be honest the code is pretty clear that it mandates a string argument, I guess it just wasn't tested with callers that provide the path as a Buffer.

[kevinuehara]

So a think this is an improvement... I can test and see if this error occurred in older versions. But anyway I will validate and see if I can improve and resolve with PR.

[alistairjevans]

Ah, sorry, I wasn't trying to state that this is a regression, I think this is just a part of the still-in-development experimental permissions work, thought I'd flag it.

[kevinuehara]

Aah ok! But thank you to sinalize! Someone can answer this question and if it is already under development, otherwise I can help with the contribution ❤️

[alistairjevans]

I think @RafaelGSS is the man to talk to, based on the commit history.

[RafaelGSS]

Apparently, this is caused by #52135. I will work on that.