zx: $'s template strings broken with zx version 2

Seems like zx only struggles when the expression contains dashes.

$`${'some string'}` // some string
$`${'some-string'}` // $'some-string'

After going through the source code the issue IMO is a regex inside of quote function.

Instead of:

/^[a-z0-9_.-/]+$/i

it should be

/^[a-z0-9_.\-/]+$/i

Because JS otherwise considers the dash to be range indicator (it matches all characters between . and / = between 46 and 47 = only these two characters) and the dash is being ignored.

You can easily test this:

/^[a-z0-9_.-/]+$/i.test('some-string')  // false
/^[a-z0-9_.\-/]+$/i.test('some-string') // true

I was able to get around this using:

const flag = `--help`
const command = `git ${flag}`

$([command]) // Is executed as `git --help`

As suggested by @YDSS, what about creting two commands to let the developper choose depending on its use case? Using the safe/unsafe keywords sometimes used in the web?

Use case 1: command’s parameters are not safe, and adding quotes could prevent a security vulnerability

$.unsafe`touch ${unsafe_file_name}`

Use case 2: command’s parameters are controlled by the developper, we can (and want to) accurately plan the template string’s output, no unexpected quotes are needed

const safe_file_name = "foo"
$`touch ${safe_file_name}`

I have the same issue with @smeijer and I understand the concern about security, but I think you can provide a way to disable the enhancement of “Quotes” feature, in my case I only want to it behave as a normal template string.

Is there a way to bypass the quoting/escaping? It makes it impossible to create function arguments dynamically.

For example:

const tarExcludes = ['.coverage', '.env.*', '.git', '.next', 'node_modules']
  .map((x) => `--exclude='${x}'`)
  .join(' ');

await $`tar -zcp ${tarExcludes} -f ../bundle.tgz .`;

I expect that to output:

$ tar -zcp --exclude='.coverage' --exclude='.env.*' --exclude='.git' --exclude='.next' --exclude='node_modules' -f ../bundle.tgz .

But instead, the output is:

$ tar -zcp $'--exclude=\'.coverage\' --exclude=\'.env.*\' --exclude=\'.git\' --exclude=\'.next\' --exclude=\'node_modules\'' -f ../bundle.tgz .

Which doesn’t work. Needless to say, that doesn’t have the expected result.

I suggest writing tests for those scenarios and fix these instances with pre/post processing to avoid the community hammering you with the same issues

Main reason why I’ve switched from Makefile / bash scripts is to use the power of a programming language to make my commands building easier to manage.

From a DX perspective, this behavior is really surprising and quite frankly a blocker for adoption IMO, all the expected benefits of command building disappear

const dcpProd = 'docker-compose -f /app/docker-compose.yml -f /app/docker-compose.prod.yml'
await $`${dcpProd + ' build'}` // doesn't work

await $`docker-compose -f /app/docker-compose.yml -f /app/docker-compose.prod.yml build` // works

I understand the security concerns but is there any solution that could make the above example works while still being secure ?

In the meantime, a workaround solution:

const quoteEscaping = $.quote
$.quote = (command) => command

// run your unescaped commands

// reenable quote escaping
$.quote = quoteEscaping

really? that should not happen…and why do you decide what context we consider unsafe? please don’t anticipate how users use it but instead add layered approach separating concerns, and add something like $.safe instead of the other way around

We already were thinking about the same API: https://github.com/google/zx/pull/151

But decided not to introduce it. It is dangerous and usually can be avoided altogether.

If you really need a command from a string there is a solution: use child_process directly and provide safety on your own.

Maybe sometimes it is desired to print quoted strings? Those should just be encoded correctly imo…

Definitely. I support your decision to not support unsafe operations. It’s your project after all. I just simply don’t get why my message sharing a workaround for those that do need it, got removed. But you explained it, and as I mentioned, your project, your issue tracker. So all fine now. 👍

Rejecting a feature/or saying no to additional configuration is also a part of an API design.

Sorry for hesitation. I don’t want show users not save way of using library. Didn’t mean any disrespect though.

I’ve been thinking about the security concern. Maybe it can detect a --dry-run arg (or process.env var), and instead of executing the commands, only echo it? That way the user could dry run their script, and read if the commands line up with what they were expecting.

As a bonus, that would also make it quite easy to wrap our scripts with tests.

Omit quotes in echo.

but what if I want to

$`echo 'some "${quote}"'`

??

Omit quotes in echo.

Sure.

@antonmedv can you please create a new release with this fix? 😃