fs: fix mkdirp return UV_EROFS on read-only fs by hiimjako · Pull Request #48105 · nodejs/node

hiimjako · 2023-05-21T16:39:38Z

I tested the changes manually by mounting a read-only fs and trying to create a new folder inside it.
I honestly wouldn't know how to write tests on these changes without mounting a RO file systems during test sets, and I don't know if this is the best solution. If anyone has advice on how to do this, it is welcome

VoltrexKeyva · 2023-05-21T21:56:34Z

Hey @hiimjako, thank you for considering to contribute.

Your commit (b4a60a3) is pointing at an invalid user, make sure you've configured Git to point to you so that you would be shown as a contributor after your changes are accepted and merged.

A commit pointing to an invalid user can generally be caused by it being committed by the incorrect email address, make sure the user.email config in Git is your primary GitHub email address. You can set the config globally to your primary email address by running:

$ git config --global user.email "<primary email>"

(Replace <primary email> with your primary GitHub email address.)

You can check the Git getting started guide for more information.

After all that, force-push the commit to make it point to you.

Fixes: nodejs#47098

hiimjako · 2023-05-21T22:44:58Z

Hi @VoltrexKeyva,

Thanks for the suggestion, I was using another email for other things and I forgot to change it :)

Now it should be right

nodejs-github-bot · 2023-05-22T08:33:53Z

CI: https://ci.nodejs.org/job/node-test-pull-request/51904/

anonrig · 2023-05-22T17:35:41Z

Can we simulate a readonly environment and add a test for this? I don't feel comfortable merging this without any tests, unless someone more knowledgable on this matter reviews and approves this PR.

RafaelGSS

Thanks for the PR. Please include a test

test: add test for mkdir running in read-only filesystem

hiimjako · 2025-03-01T00:19:11Z

Hi @RafaelGSS,
thanks to the work of @kwaszczuk, a test has been added.
There was a discussion in the issue about which platforms to test, and it was decided that supporting only Linux would be sufficient since uv already maps to the error on each platform.
Does this seem reasonable to you?
Thanks!

kwaszczuk · 2025-04-03T10:05:06Z

Hey @RafaelGSS, any chance you'll be able to take a look at this PR soon? 🤞

RafaelGSS · 2025-04-07T21:37:54Z

test/parallel/test-fs-mkdir.js

 }

+// mkdirpSync when interacting with read-only filesystem.
+if (common.isLinux && process.getuid() === 0) // Mounting filesystem requires root privilege.


Do we have a test environment that would enter into this clause? @nodejs/build

mkdirSync with { recursive: true } on a read-only filesystem throws ENOENT instead of the expected EROFS. This test demonstrates the bug by mounting a read-only tmpfs via sudo and verifying the error code. Refs: nodejs#47098 Refs: nodejs#48105 Signed-off-by: Kenny Yeo <kenny@yeoyou.net>

mkdirSync with { recursive: true } on a read-only filesystem incorrectly throws ENOENT instead of the expected EROFS error. This test demonstrates the bug by mounting a read-only tmpfs via sudo and verifying the error code. The test requires passwordless sudo to mount a read-only tmpfs. In environments without this capability (such as ci.nodejs.org), the test is skipped with a warning message explaining why. Refs: nodejs#47098 Refs: nodejs#48105 Signed-off-by: Kenny Yeo <kenny@yeoyou.net>

@hiimjako

Add UV_EROFS to the list of errors that should be returned immediately in MKDirpSync and MKDirpAsync, preventing it from being incorrectly converted to ENOENT when mkdir is called with the recursive option on a read-only filesystem. Based on the original fix by Alberto Moretti (@hiimjako) in nodejs#48105. Refs: nodejs#47098 Refs: nodejs#48105 Co-authored-by: Alberto Moretti <moretti919@gmail.com> Signed-off-by: Kenny Yeo <kenny@yeoyou.net>

@hiimjako

Add UV_EROFS to the list of errors that should be returned immediately in MKDirpSync and MKDirpAsync, preventing it from being incorrectly converted to ENOENT when mkdir is called with the recursive option on a read-only filesystem. Based on the original fix by Alberto Moretti (@hiimjako) in nodejs#48105. Refs: nodejs#47098 Refs: nodejs#48105 Co-authored-by: Alberto Moretti <moretti919@gmail.com> Signed-off-by: Kenny Yeo <kenny@yeoyou.net>

@hiimjako

Add UV_EROFS to the list of errors that should be returned immediately in MKDirpSync and MKDirpAsync, preventing it from being incorrectly converted to ENOENT when mkdir is called with the recursive option on a read-only filesystem. Based on the original fix by Alberto Moretti (@hiimjako) in nodejs#48105. Refs: nodejs#47098 Refs: nodejs#48105 Co-authored-by: Alberto Moretti <moretti919@gmail.com> Signed-off-by: Kenny Yeo <kenny@yeoyou.net>

mkdirSync with { recursive: true } on a read-only filesystem incorrectly throws ENOENT instead of the expected EROFS error. This test demonstrates the bug by mounting a read-only tmpfs via sudo and verifying the error code. The test requires passwordless sudo to mount a read-only tmpfs. In environments without this capability (such as ci.nodejs.org), the test is skipped with a warning message explaining why. Refs: nodejs#47098 Refs: nodejs#48105 Signed-off-by: Kenny Yeo <kenny@yeoyou.net>

@hiimjako

Add UV_EROFS to the list of errors that should be returned immediately in MKDirpSync and MKDirpAsync, preventing it from being incorrectly converted to ENOENT when mkdir is called with the recursive option on a read-only filesystem. Based on the original fix by Alberto Moretti (@hiimjako) in nodejs#48105. Refs: nodejs#47098 Refs: nodejs#48105 Co-authored-by: Alberto Moretti <moretti919@gmail.com> Signed-off-by: Kenny Yeo <kenny@yeoyou.net>

kennyhyun · 2026-04-10T00:58:20Z

Hi, I have tried the same fix with a test validated on Github Actions. one commit for failure and the other is fix (from @hiimjako).

My approach for testing is to just skip testing when sudo access failed. (maybe same approach with @kwaszczuk)

@RafaelGSS @hiimjako Can I open a new PR? at least there is no conflict.

hiimjako · 2026-04-11T19:54:26Z

Hi @kennyhyun!
On my side there is no problem!
Go for that :)

mkdirSync with { recursive: true } on a read-only filesystem incorrectly throws ENOENT instead of the expected EROFS error. Two test cases are included: - On Linux with passwordless sudo, mounts a real read-only tmpfs to verify the error code against the actual kernel behavior. - Uses internalBinding to mock binding.mkdir and simulate EROFS, ensuring the error propagates correctly without requiring root. Refs: nodejs#47098 Refs: nodejs#48105 Signed-off-by: Kenny Yeo <kenny@yeoyou.net>

@hiimjako

Add UV_EROFS to the list of errors that should be returned immediately in MKDirpSync and MKDirpAsync, preventing it from being incorrectly converted to ENOENT when mkdir is called with the recursive option on a read-only filesystem. Based on the original fix by Alberto Moretti (@hiimjako) in nodejs#48105. Refs: nodejs#47098 Refs: nodejs#48105 Co-authored-by: Alberto Moretti <moretti919@gmail.com> Signed-off-by: Kenny Yeo <kenny@yeoyou.net>

mkdirSync with { recursive: true } on a read-only filesystem incorrectly throws ENOENT instead of the expected EROFS error. Two test cases are included: - On Linux with passwordless sudo, mounts a real read-only tmpfs to verify the error code against the actual kernel behavior. - Uses internalBinding to mock binding.mkdir and simulate EROFS, ensuring the error propagates correctly without requiring root. Refs: nodejs#47098 Refs: nodejs#48105 Signed-off-by: Kenny Yeo <kenny@yeoyou.net>

@hiimjako

Add UV_EROFS to the list of errors that should be returned immediately in MKDirpSync and MKDirpAsync, preventing it from being incorrectly converted to ENOENT when mkdir is called with the recursive option on a read-only filesystem. Based on the original fix by Alberto Moretti (@hiimjako) in nodejs#48105. Refs: nodejs#47098 Refs: nodejs#48105 Co-authored-by: Alberto Moretti <moretti919@gmail.com> Signed-off-by: Kenny Yeo <kenny@yeoyou.net>

kennyhyun · 2026-04-14T04:32:06Z

Thanks @hiimjako, I ended up with adding a mock test case as well. Let me create a new PR. Thanks!

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels May 21, 2023

hiimjako force-pushed the fix/mkdirp-EROFS branch from 3d14ad9 to b4a60a3 Compare May 21, 2023 16:53

fs: fix mkdirp return UV_EROFS on read-only fs

237b429

Fixes: nodejs#47098

hiimjako force-pushed the fix/mkdirp-EROFS branch from b4a60a3 to 237b429 Compare May 21, 2023 22:34

marco-ippolito added request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels May 22, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 22, 2023

marco-ippolito requested a review from anonrig May 22, 2023 17:19

marco-ippolito requested a review from RafaelGSS May 25, 2023 09:52

RafaelGSS requested changes May 25, 2023

View reviewed changes

kwaszczuk mentioned this pull request Feb 28, 2025

test: add test for mkdir running in read-only filesystem hiimjako/node#1

Merged

kwaszczuk and others added 3 commits March 1, 2025 00:29

test: add test for mkdir running in read-only filesystem

9c1c321

Merge pull request #1 from kwaszczuk/fix/mkdirp-EROFS

db9e65a

test: add test for mkdir running in read-only filesystem

Merge branch 'main' into fix/mkdirp-EROFS

fd1117d

RafaelGSS reviewed Apr 7, 2025

View reviewed changes

kennyhyun mentioned this pull request Apr 8, 2026

test: add failing test for recursive mkdir on read-only fs kennyhyun/node#1

Open

kennyhyun mentioned this pull request Apr 14, 2026

src,test: handle EROFS in mkdir recursive #62725

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fs: fix mkdirp return UV_EROFS on read-only fs#48105

fs: fix mkdirp return UV_EROFS on read-only fs#48105
hiimjako wants to merge 4 commits intonodejs:mainfrom
hiimjako:fix/mkdirp-EROFS

hiimjako commented May 21, 2023

Uh oh!

VoltrexKeyva commented May 21, 2023 •

edited

Loading

Uh oh!

hiimjako commented May 21, 2023

Uh oh!

nodejs-github-bot commented May 22, 2023

Uh oh!

anonrig commented May 22, 2023

Uh oh!

RafaelGSS left a comment

Uh oh!

hiimjako commented Mar 1, 2025

Uh oh!

kwaszczuk commented Apr 3, 2025

Uh oh!

RafaelGSS Apr 7, 2025

Uh oh!

richardlau Apr 7, 2025

Uh oh!

kennyhyun commented Apr 10, 2026

Uh oh!

hiimjako commented Apr 11, 2026

Uh oh!

kennyhyun commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Uh oh!

Conversation

hiimjako commented May 21, 2023

Uh oh!

VoltrexKeyva commented May 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hiimjako commented May 21, 2023

Uh oh!

nodejs-github-bot commented May 22, 2023

Uh oh!

anonrig commented May 22, 2023

Uh oh!

RafaelGSS left a comment

Choose a reason for hiding this comment

Uh oh!

hiimjako commented Mar 1, 2025

Uh oh!

kwaszczuk commented Apr 3, 2025

Uh oh!

RafaelGSS Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

richardlau Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

kennyhyun commented Apr 10, 2026

Uh oh!

hiimjako commented Apr 11, 2026

Uh oh!

kennyhyun commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

VoltrexKeyva commented May 21, 2023 •

edited

Loading