Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use execFileSync to launch child process #790

Merged
merged 1 commit into from Oct 27, 2017
Merged

Conversation

nfischer
Copy link
Member

This uses child_process.execFileSync instead of execSync to launch the child
process. This further reduces the attack surface, removing a possible point for
command injection in the ShellJS implementation.

This does not affect backwards compatibility for the shell.exec API (the
behavior is determined by the call to child_process.exec within
src/exec-child.js).

Issue #782

This uses `child_process.execFileSync` instead of `execSync` to launch the child
process. This further reduces the attack surface, removing a possible point for
command injection in the ShellJS implementation.

This does not affect backwards compatibility for the `shell.exec` API (the
behavior is determined by the call to `child_process.exec` within
`src/exec-child.js`).

Issue #782
@nfischer nfischer added exec Issues specific to the shell.exec() API refactor security labels Oct 20, 2017
@codecov-io
Copy link

codecov-io commented Oct 20, 2017

Codecov Report

Merging #790 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #790      +/-   ##
==========================================
+ Coverage    95.5%   95.51%   +<.01%     
==========================================
  Files          34       34              
  Lines        1269     1270       +1     
==========================================
+ Hits         1212     1213       +1     
  Misses         57       57
Impacted Files Coverage Δ
src/exec-child.js 100% <ø> (ø) ⬆️
src/exec.js 97.4% <100%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e3f9ab...499d2ad. Read the comment docs.

@freitagbr
Copy link
Contributor

@nfischer LGTM

@nfischer nfischer merged commit b885590 into master Oct 27, 2017
@nfischer nfischer deleted the use-exec-file-api branch July 4, 2018 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exec Issues specific to the shell.exec() API refactor security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants