fix(ext/node): fs.cp across allowed dirs with ignored read permissions#32927
Merged
bartlomieju merged 2 commits intodenoland:mainfrom Mar 24, 2026
Merged
Conversation
When `fs.cp` was moved from JS to Rust, the `check_parent_paths` function walks up the destination's parent directory chain calling `check_cp_path()` at each level. With `"ignore": true` permissions, denied reads become `ENOENT` errors. The old JS code handled this gracefully (safeStatFn caught NotFound and stopped walking), but the Rust code propagated the error via `?` before reaching the stat call's own NotFound handler. Fix: catch NotFound from the permission check and treat it the same as a non-existent directory — stop walking and return Ok(()). Closes denoland#32923 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a regression in the Rust-backed node:fs.cp implementation where ancestor-path validation could incorrectly treat ignored permission denials as fatal ENOENT, breaking copies between otherwise-allowed directories when permissions.default.read.ignore: true is set.
Changes:
- Update
check_parent_pathsancestor traversal to treat ignored-permissionNotFoundfrom permission checks as a stop signal (likestat’sNotFound). - Add a new node spec regression test covering
fs.cp(callback),fs.promises.cp, andfs.cpSyncunder ignored read permissions.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
ext/node/ops/fs.rs |
Stops parent traversal when permission checks return NotFound due to ignored read permissions, preventing erroneous hard failures. |
tests/specs/node/fs_cp_ignored_permissions/main.ts |
New regression script exercising fs.cp variants under a permissions config using ignore: true. |
tests/specs/node/fs_cp_ignored_permissions/main.out |
Expected output for the new regression test. |
tests/specs/node/fs_cp_ignored_permissions/__test__.jsonc |
Spec test definition running the new script with -P permissions config. |
tests/specs/node/fs_cp_ignored_permissions/deno.json |
Permissions configuration reproducing the ignored-read-permissions scenario. |
tests/specs/node/fs_cp_ignored_permissions/dir1/data.txt |
Source fixture file for the copy. |
tests/specs/node/fs_cp_ignored_permissions/dir2/data_cb.txt |
Destination fixture file used by callback copy variant. |
tests/specs/node/fs_cp_ignored_permissions/dir2/data_promise.txt |
Destination fixture file used by promise copy variant. |
tests/specs/node/fs_cp_ignored_permissions/dir2/data_sync.txt |
Destination fixture file used by sync copy variant. |
tests/specs/node/fs_cp_ignored_permissions/dir2/.gitkeep |
Keeps the destination directory tracked. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
node:fs.cpfailing withENOENTwhen copying between alloweddirectories while read permissions outside those directories are ignored
(
"ignore": truein deno.json permissions config)check_parent_pathswalk-up now handlesNotFoundfrom permissionchecks (ignored denials) the same way it handles
NotFoundfromstat—by stopping the ancestor traversal
Root cause: When
fs.cpwas moved from JS to Rust (#32580, #32687), theparent path validation started propagating ignored-permission
ENOENTas ahard error instead of treating it as a stop signal. The old JS code's
safeStatFncaughtNotFoundand returnedundefined, gracefully stoppingthe walk. The Rust code's
check_cp_path()?propagated it before reachingthe stat call's own
NotFoundhandler.Closes #32923
Test plan
fs_cp_ignored_permissions— copies a file usingfs.cp(callback),fs.promises.cp, andfs.cpSyncacross directorieswith limited read permissions and
ignore: truefs_cpspec tests pass🤖 Generated with Claude Code