Skip to content

fix: add ASAR support for Node.js fs.cp methods#49013

Closed
arnav-54 wants to merge 2 commits intoelectron:mainfrom
arnav-54:fix/asar-cp-support
Closed

fix: add ASAR support for Node.js fs.cp methods#49013
arnav-54 wants to merge 2 commits intoelectron:mainfrom
arnav-54:fix/asar-cp-support

Conversation

@arnav-54
Copy link
Copy Markdown

@arnav-54 arnav-54 commented Nov 18, 2025 •
edited by nikwen
Loading

Fixes ENOENT errors when using fs.cp(), fs.cpSync(), and fs.promises.cp() on ASAR paths

This PR adds ASAR support for the newer Node.js copy APIs (fs.cp, fs.cpSync, and fs.promises.cp).
These methods were not previously wrapped by Electron’s ASAR filesystem layer, which caused copy operations targeting paths inside app.asar to fail with ENOENT.
The change brings these APIs in line with the existing ASAR-aware wrappers such as readFile, copyFile, and readdir.

Description of Change

Fixes #49012

Electron’s ASAR implementation overrides a subset of Node’s fs APIs so that operations on paths inside .asar archives resolve correctly.
However, the newer fs.cp family of methods was not included in this wrapper, resulting in unhandled real-filesystem lookups when attempting to copy files or directories from within an ASAR archive.

This PR implements ASAR-aware versions of:

  • fs.cp
  • fs.cpSync
  • fs.promises.cp

and adds tests to ensure these methods behave consistently with the rest of the ASAR virtual filesystem.

Checklist

  • PR description included
  • npm test passes
  • Tests are added or updated where appropriate
  • Documentation updates (not required unless a public API changes)
  • Release notes added following Electron’s style guide

Release Notes

Notes: Added ASAR support for fs.cp(), fs.cpSync(), and fs.promises.cp().

@arnav-54
fix: add ASAR support for Node.js fs.cp methods
8f35786 
Fixes ENOENT errors when using fs.cp, fs.cpSync, and fs.promises.cp
to copy files/folders from ASAR archives.
@welcome
Copy link
Copy Markdown

welcome bot commented Nov 18, 2025

💖 Thanks for opening this pull request! 💖

Semantic PR titles

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add app.isPackaged() method
  • docs: app.isDefaultProtocolClient is now available on Linux

Commit signing

This repo enforces commit signatures for all incoming PRs.
To sign your commits, see GitHub's documentation on Telling Git about your signing key.

PR tips

Things that will help get your PR across the finish line:

  • Follow the JavaScript, C++, and Python coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made following the documentation styleguide.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Nov 18, 2025
@bendemboski bendemboski mentioned this pull request Nov 18, 2025
Open
3 tasks
nikwen
nikwen previously requested changes Nov 18, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@nikwen nikwen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Could you please confirm whether you have built and tested these changes?
  2. Could you please add tests?

@arnav-54
Copy link
Copy Markdown
Author

arnav-54 commented Nov 19, 2025

  1. Could you please confirm whether you have built and tested these changes?
  2. Could you please add tests?

1.I've added tests and verified the code follows existing patterns. The fix uses the same overrideAPI pattern as copyFile methods. Tests are included to verify functionality.
2.Done! Added tests for fs.cp, fs.cpSync, and fs.promises.cp in the latest commit.

@arnav-54
Copy link
Copy Markdown
Author

arnav-54 commented Nov 19, 2025

The requested changes have been made. Tests were added, and release notes have been updated. Please let me know if anything else is needed.

@nikwen
Copy link
Copy Markdown
Member

nikwen commented Nov 22, 2025

@arnav-54 Could you please sign your commits? (See the automated comment above.)

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/38-x-y PR should also be added to the "38-x-y" branch. target/39-x-y PR should also be added to the "39-x-y" branch. target/40-x-y PR should also be added to the "40-x-y" branch. labels Nov 24, 2025
@electron-cation electron-cation bot added new-pr 🌱 PR opened recently and removed new-pr 🌱 PR opened recently labels Nov 24, 2025
@github-actions github-actions bot added the target/41-x-y PR should also be added to the "41-x-y" branch. label Jan 19, 2026
@nikwen
Copy link
Copy Markdown
Member

nikwen commented Feb 26, 2026

@arnav-54 Could you please sign your commits? We can't merge this PR unless your commits are signed.

@mlaurencin mlaurencin added the needs-signed-commits Currently some or all of the commits in this PR are not signed label Mar 9, 2026
@nikwen
Copy link
Copy Markdown
Member

nikwen commented Mar 12, 2026 •
edited
Loading

Would anyone like to resubmit this PR (or a similar one) with signed commits? We haven't received a response here in a few months. CC @nmggithub

@nmggithub nmggithub mentioned this pull request Mar 12, 2026
Merged
5 tasks
@github-actions github-actions bot added the target/42-x-y PR should also be added to the "42-x-y" branch. label Mar 13, 2026
@mlaurencin
Copy link
Copy Markdown
Member

mlaurencin commented Mar 16, 2026

Given that the above listed PR was merged with the same changes in it, I am going to go ahead and close this PR.

@mlaurencin mlaurencin closed this Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@codebytere codebytere codebytere approved these changes

@nikwen nikwen nikwen left review comments

@MarshallOfSound MarshallOfSound Awaiting requested review from MarshallOfSound

Assignees

No one assigned

Labels

needs-signed-commits Currently some or all of the commits in this PR are not signed semver/patch backwards-compatible bug fixes target/38-x-y PR should also be added to the "38-x-y" branch. target/39-x-y PR should also be added to the "39-x-y" branch. target/40-x-y PR should also be added to the "40-x-y" branch. target/41-x-y PR should also be added to the "41-x-y" branch. target/42-x-y PR should also be added to the "42-x-y" branch.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Node fs cp methods are not wrapped, don't work to copy out of an ASAR archive

5 participants

@arnav-54 @nikwen @mlaurencin @codebytere @ckerr