child_process: use module.exports.spawn in fork and execFile#62666
Open
robertsLando wants to merge 1 commit intonodejs:mainfrom
Open
child_process: use module.exports.spawn in fork and execFile#62666robertsLando wants to merge 1 commit intonodejs:mainfrom
robertsLando wants to merge 1 commit intonodejs:mainfrom
Conversation
`fork()` and `execFile()` call the locally-scoped `spawn()` function directly, bypassing any wrapping applied to `child_process.spawn` via module exports. This is inconsistent with `exec()`, which correctly uses `module.exports.execFile()`. APM tools, security monitors, and testing frameworks that wrap `child_process.spawn` to intercept child process creation silently miss processes spawned through `fork()` and `execFile()`. Fix both functions to use `module.exports.spawn()`, consistent with the existing `exec()` pattern. Signed-off-by: Daniel Lando <daniel.sorridi@gmail.com>
11 tasks
Member
|
For context:
This was a leftover from when the child_process methods existed as module.exports properties rather than as module-declared functions (aa00968). AFAIA, the decision to leave this here was a convenience thing to avoid branch conflicts, not a statement of intent for facilitating the monkeypatching of API internals. |
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.
fork()andexecFile()call the locally-scopedspawn()functiondirectly, bypassing any wrapping applied to
child_process.spawnviamodule exports.
This is inconsistent with
exec(), which correctly usesmodule.exports.execFile()(see lib/child_process.js:236).The problem
APM tools (New Relic, Datadog), security monitors, and testing
frameworks commonly wrap
child_process.spawnon exports to interceptchild process creation. With the current code, processes spawned
through
fork()andexecFile()silently bypass these wrappers, whileexec()correctly goes through them.The fix
Change
fork()andexecFile()to callmodule.exports.spawn()instead of the locally-scoped
spawn(), consistent with the patternalready used by
exec().Refs: yao-pkg/pkg#231