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

Exec args always expanded #434

Open
hborham opened this issue Jul 7, 2022 · 2 comments · May be fixed by #505
Open

Exec args always expanded #434

hborham opened this issue Jul 7, 2022 · 2 comments · May be fixed by #505

Comments

@hborham
Copy link

hborham commented Jul 7, 2022

Within the sh package, args are being expanded in the case where normal sh command would not expand args within single quotes. Example command:

pact-broker create-webhook --user='admin:${user.bitbucketAppPassword}'

  1. Via normal shell the single quotes will not expand ${user.bitbucketAppPassword}
  2. Via mage the expansion occurs and the actual command becomes:
    pact-broker create-webhook --user='admin:'

mage/sh/cmd.go

Lines 92 to 115 in 26cdb5c

// Exec executes the command, piping its stderr to mage's stderr and
// piping its stdout to the given writer. If the command fails, it will return
// an error that, if returned from a target or mg.Deps call, will cause mage to
// exit with the same code as the command failed with. Env is a list of
// environment variables to set when running the command, these override the
// current environment variables set (which are also passed to the command). cmd
// and args may include references to environment variables in $FOO format, in
// which case these will be expanded before the command is run.
//
// Ran reports if the command ran (rather than was not found or not executable).
// Code reports the exit code the command returned if it ran. If err == nil, ran
// is always true and code is always 0.
func Exec(env map[string]string, stdout, stderr io.Writer, cmd string, args ...string) (ran bool, err error) {
expand := func(s string) string {
s2, ok := env[s]
if ok {
return s2
}
return os.Getenv(s)
}
cmd = os.Expand(cmd, expand)
for i := range args {
args[i] = os.Expand(args[i], expand)
}

@fergonco
Copy link

fergonco commented Aug 2, 2023

Similar problem here. I am calling sh.Exec with a list of arguments one of which contains $. I want the literal $ but mage tries to expand it and I found no way to prevent this.

rkennedy added a commit to rkennedy/mage that referenced this issue May 1, 2024
The `sh.Exec` family of functions used to use `os.Expand` to expand
environment variables in command arguments, but that function offers no
escape syntax -- no way to avoid expanding dollar signs that the caller
really wants to include on the command line. This commit implements a
new, mostly compatible `Expand` function to allow escaping the
environment-variable syntax, plus an accompanying `Escape` function to
producing escaped strings.

This technically breaks backward compatibility; any existing code that
has used the sequence "\\" or "\$" in its command arguments will need to
be updated. Although the compatibility argument prevents `os.Expand`
from changing, documentation for `sh.Exec` hasn't actually stated that
it follows all the same rules as `os.Expand`, so it's possible to view
this change as merely a bugfix, formalizing a new syntax that had not
previously been established.

Fixes magefile#434
@rkennedy rkennedy linked a pull request May 1, 2024 that will close this issue
@rkennedy
Copy link

rkennedy commented May 1, 2024

I've just submitted a PR that I think would solve this. But @hborham, note that for your example to work the way you want, you'd need to remove the apostrophes from your arguments. Apostrophes are one way to avoid having the shell expand environment variables. In doing so, the shell would strip the apostrophes from the argument prior to invoking the given command, so pact-broker would receive literally --user=admin:${user.bitbucketAppPassword} as its argument. Using my PR, you might write code like this:

sh.Run("pact-broker", "create-webhook", sh.Escape("--user=admin:${user.bitbucketAppPassword}"))

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 a pull request may close this issue.

3 participants