-
Notifications
You must be signed in to change notification settings - Fork 744
Closed
Labels
execIssues specific to the shell.exec() APIIssues specific to the shell.exec() APIfeaturehigh prioritysecurity
Milestone
Description
It would be great if exec had an interface, perhaps like the following:
var fileName = 'foo.txt';
exec('git', 'add', fileName);This is great because of the following:
- We can declare that each argument is its own non-splittable word (so
fileName = 'file1.txt file2.txt'means the git call will search for one file namedfile1.txt file2.txt, not two separate files) - We can implement this fairly easily with
child_process.execFile - This would allow us to do our own glob expansion, which would be good for Windows (Windows
exec()won't glob-expand) - This would be useful in
shx(Executing paths should be supported shx#65 & Globbing arguments to an external command shx#68) - Variables would have to be referenced as JavaScript variables just like in the rest of ShellJS, not as shell variables (
exec('echo', env.PATH)vs.exec('ls $PATH')), which is the safer approach - Doing our own globbing means that
execwill respect a call toset('-f') - This would give more security to shelljs-exec-proxy for the same reasons
The issue with this interface is what to do when we have only 1 argument (do we use the old behavior or the new execFile implementation?). An alternative approach is exec([arg1, arg2, arg3]), which uses a list to remove ambiguity. This is easier to parse, but perhaps not as nice to work with.
Edit: to clarify, this is a feature request, not a security vulnerability. This new API would be easier to use safely than shell.exec(), but shell.exec() is not itself vulnerable and can in fact be used safely without much difficulty (see security guidelines).
CAD97, runk, ylg, wmertens and voldikssaxelson
Metadata
Metadata
Assignees
Labels
execIssues specific to the shell.exec() APIIssues specific to the shell.exec() APIfeaturehigh prioritysecurity