Skip to content

calls external commands#64

Closed
wbehrens-on-gh wants to merge 1 commit into
wren-lang:mainfrom
wbehrens-on-gh:externalCommands
Closed

calls external commands#64
wbehrens-on-gh wants to merge 1 commit into
wren-lang:mainfrom
wbehrens-on-gh:externalCommands

Conversation

@wbehrens-on-gh

@wbehrens-on-gh wbehrens-on-gh commented Mar 6, 2021

Copy link
Copy Markdown

Added an exec command to the Process class that basically wraps C's system() function so now Wren-cli can call external programs.

Example:

import  "os" for Process

Process.exec("echo hello") // Should print hello to the console
Process.exec("not a command") // Errors out saying command not found
Process.exec(12) // Aborts current fiber saying that exec requires a string

Don't think this is a perfect implementation but it's minimal.

@ChayimFriedman2

Copy link
Copy Markdown

I think the C system() API is not good. It does not allow passing arguments separately, for example, which can lead to attacks.

As the CLI is based on libuv, I think we should provide an API based on libuv processes.

@wbehrens-on-gh

Copy link
Copy Markdown
Author

I'm not familiar with working with libuv but I'll see if I can throw something up.

@PureFox48

PureFox48 commented Apr 11, 2021

Copy link
Copy Markdown

Even if it's only done as a temporary expedient, I think it would make sense to merge this relatively simple PR into the forthcoming v0.4.0 release as it would make a significant difference to what can be done with Wren-cli.

An API based on libuv processes can be added later.

@ChayimFriedman2

ChayimFriedman2 commented Apr 12, 2021

Copy link
Copy Markdown

If so, then at least have arguments passed separately using a list.

@wbehrens-on-gh

Copy link
Copy Markdown
Author

Could you explain why there is a need to pass arguments as a list? most users will just pass a single string with the arguments in it anyway.

@ChayimFriedman2

Copy link
Copy Markdown

Security.

@wbehrens-on-gh

Copy link
Copy Markdown
Author

Could you explain how it would help security?

@ChayimFriedman2

Copy link
Copy Markdown

If I do,

Process.exec("cat \"%(fileName)\"")

When fileName is user-provided, the user can inject for example \"; some_malicious_code \" executing arbitrary code. This can be solved by santizing the input, but sanitizing input is hard. For this reason, it's prefered to pass arguments as a separate list, where Process.exec("cat", [fileName]) can never execute arbitrary code.

@joshgoebel

Copy link
Copy Markdown
Contributor

uv_spawn looks pretty dead simple though at first glance (compared to networking). http://docs.libuv.org/en/v1.x/guide/processes.html#spawning-child-processes

Of course this quickly gets into:

  • pipes, input/output redirection
  • process control / signaling
  • process monitoring

But I think for starters just a run it and let you know the error code would be trivial. And if UV gets us cross-platform for free, even better. My first ideas for packages require being able to call git, so I'd need this too.

@joshgoebel

Copy link
Copy Markdown
Contributor

See #94 for a quick pass at this based on UV.

@joshgoebel

Copy link
Copy Markdown
Contributor

I believe this will be closed now that we are moving forward with #94.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants