zx icon indicating copy to clipboard operation
zx copied to clipboard

Implement the quota function for cmd.exe

Open antonmedv opened this issue 2 years ago • 12 comments

For better Windows support.

antonmedv avatar Jul 04 '22 09:07 antonmedv

To those who want to overcome this issue on win32 before an official solution is provided, you can monkey-patch the quote function. Example in a zx.ts:

import { $ as _zx } from "zx"

_zx.quote = (arg = "") => {
  if (/^[a-z0-9/_.\-@:=]+$/i.test(arg) || arg === "") {
    return arg
  }
  const isWindows = process.platform === "win32"

  const sanitizedArg = arg
    .replace(/\\/g, "\\\\")
    .replace(/'/g, "\\'")
    .replace(/\f/g, "\\f")
    .replace(/\n/g, "\\n")
    .replace(/\r/g, "\\r")
    .replace(/\t/g, "\\t")
    .replace(/\v/g, "\\v")
    .replace(/\0/g, "\\0")

  return isWindows ? sanitizedArg : `$'${sanitizedArg}'`
}

export const zx = _zx

Then, import { zx } from that and use it instead.

louisgv avatar Sep 18 '22 20:09 louisgv

@louisgv this is a bad solution. You should not do it like this. Also, what you did, can be easily achieved with in one line:

$.quota = x => x

antonmedv avatar Sep 18 '22 20:09 antonmedv

@louisgv this is a bad solution. You should not do it like this. Also, what you did, can be easily achieved with in one line:

$.quota = x => x

Have you tested it? ;d

louisgv avatar Sep 18 '22 21:09 louisgv

Test this: echo ${'; rm -rf /'}

antonmedv avatar Sep 18 '22 21:09 antonmedv

Test this: echo ${'; rm -rf /'}

I did that massively on a cluster of 50 CentOS boxes back in high school for fun yeah, took about 5 mins or so for it to start taking effect on x ᕕ( ᐛ )ᕗ (the real fun starts later with PXE ofc xD)

On the other hand, win32 does not use /, and rm is actually not recognized in pwsh/cmd unless you alias it with Remove-Item, which also requires one to specify the drive's root (C:\\ etc...). And even then, there's no sudo so I would need to run it on an elevated pwsh instance ;d

For WSL, / would point to a virtual drive and not the host's system, so it won't produce the desired effect. And I think git bash points / to its local root drive too, so I'm not sure it would work as well...

Anyhow, to folks who wanna use my working but "bad" hack: just make sure to sanitize input and cross your fingers, and it'll be fine 🤞

louisgv avatar Sep 19 '22 01:09 louisgv

I'm having similar issues using Windows.
I'm dynamically building a command to be executed by yarn/npm, and when finally I do this:

await $`${removeCommandString}`;

where removeCommandString has been populated as yarn remove @somedomain/somepackage

I get this on Windows:

$ $'yarn remove @somedomain/somepackage'
'$'yarn' is not recognized as an internal or external command,
operable program or batch file.

So apparently the whole command is still wrapped by single quotes, and it obviously does not work.

I also cannot use the above hack because our zx script is running directly from a "scripts" section on a package.json, we are not importing zx into any source file manually.

Maikel-Nait avatar Sep 19 '22 02:09 Maikel-Nait

@Maikel-Nait you can try this before invoking your yarn/templated stuffs:

$.shell = "bash"

Since you're not dealing with path, it should work afaik on win32.

Also, I think you can def hack the $.quote in your script - just do it before your main script logic like this:

$.quote = (arg = "") => {
  if (/^[a-z0-9/_.\-@:=]+$/i.test(arg) || arg === "") {
    return arg
  }
  const isWindows = process.platform === "win32"

  const sanitizedArg = arg
    .replace(/\\/g, "\\\\")
    .replace(/'/g, "\\'")
    .replace(/\f/g, "\\f")
    .replace(/\n/g, "\\n")
    .replace(/\r/g, "\\r")
    .replace(/\t/g, "\\t")
    .replace(/\v/g, "\\v")
    .replace(/\0/g, "\\0")

  return isWindows ? sanitizedArg : `$'${sanitizedArg}'`
}

...

await $`${removeCommandString}`;

It's nodejs, we should be able to bend it to our will ;d

louisgv avatar Sep 19 '22 02:09 louisgv

Oh I see... going to give it a try... thanks

[UPDATE] doing the hack before the actual logic works like a charm... you just saved my life !

Let's hope the official fix comes soon...

Maikel-Nait avatar Sep 19 '22 02:09 Maikel-Nait

@louisgv your quota function is unsafe). It just deletes quotes on windows - this is not a proper way to sanitize your inputs. We should thing about proper and secure way of "escaping" on windows.

@Maikel-Nait zx not designed to take commands inputs. In your case just using spawn will be easier.

antonmedv avatar Sep 19 '22 07:09 antonmedv

@antonmedv what do you mean exactly by using spawn ? I'm just doing something similar to the example given here: https://github.com/google/zx#command- but instead of doing mkdir I'm calling yarn

Maikel-Nait avatar Sep 19 '22 07:09 Maikel-Nait

@louisgv your quota function is unsafe). It just deletes quotes on windows - this is not a proper way to sanitize your inputs. We should thing about proper and secure way of "escaping" on windows.

Unsafe to whom?

When I said "sanitize input" above, the idea is people just do it themselves before passing it down to zx. To me, zx is just a convenient wrapper around spawn that has 30k stars and actively maintained by cool people. I can just write bash but then it won't run on win32, which is the whole point of this issue xD And bash is just holy unmaintainable ;d

Also why would zx cares if people shoot themselves in the foot? We're not highschool kid anymore. Just do one thing and do it well imo, otherwise as you said in many other PR - zx is getting too fat lol

louisgv avatar Sep 19 '22 14:09 louisgv

Actually I have an idea for windows: replace with env vars.

antonmedv avatar Sep 19 '22 15:09 antonmedv

https://github.com/google/zx/pull/522

antonmedv avatar Oct 04 '22 21:10 antonmedv

The new quote function has the same issue in pwsh. It "worked" now because zx automatically finds a bash env and switch shell, thus obfuscating the core issue.

FYI To those who has WSL, zx might actually point your shell to WSL instead of your host, leading to wrong mount points.

louisgv avatar Oct 09 '22 22:10 louisgv

Now zx has two quote functions: quote for bash, quotePowerShell for pwsh.

Logic for choosing a shell: lookup for bash, otherwise (if it’s windows) for pwsh.

antonmedv avatar Oct 10 '22 07:10 antonmedv