Remove Redundant Path Resolution in Internal File System Utilities

[avivkeller]

Summary

This pull request eliminates redundant path resolution within the internal file system (fs) utilities. Previously, this resolution was implemented to prevent path traversal via relative paths. However, with the mitigation of this threat, the code responsible for converting absolute paths has become unnecessary.

Proof of Concept

process.cwd = () => '../'; // Sets CWD to relative
fs.readFileSync('unsafe/file.txt')

Executing this code with --experimental-permission --allow-fs-read=safe will result in Error: Access to this API has been restricted, even though path.resolve returns a relative path. Therefore, the conversion of absolute paths is no longer essential.

Related Issues

"Due to the PrefixRadixTree (fs_permission) lookup, relative paths are not supported. For this reason, the possiblyTransformPath was needed. I do have plans to create a pretty similar path.resolve on the C++ side so the possiblyTransformPath won't be needed, but I'll do it in a second iteration."

• src,process: initial permission model implementation #44004 introduced this functionality initially when relative paths posed a threat. However, with the current landscape, they are no longer necessary.

A discussion here mentioned this change, but did not end up implementing it. While this code is redundant, I can't locate the original pull-request that made it so.

Note

I am a JS dev, but not a C one. While I know that this all works from the JS side, I am no expert in identifying the C code that makes this redundant, and the commit that added it.

[marco-ippolito]

Thanks for your contribution, unfortunately this change breaks every single test on fs permission model.
Can you explain why the check is redundant, and the reasoning behind this change?

[avivkeller]

My bad, I realized a mistake, see the issue I posted.