Skip to content

Commit

Permalink
fix: escape dollar sign $ properly in .env, for ddev#6406, fixes ddev…
Browse files Browse the repository at this point in the history
  • Loading branch information
stasadev authored Nov 9, 2024
1 parent 9dd929b commit da51ad4
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 8 deletions.
5 changes: 3 additions & 2 deletions cmd/ddev/cmd/addon-get.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,10 +359,11 @@ func getInjectedEnv(envFile string, verbose bool) string {
}
injectedEnv = "export"
for k, v := range envMap {
v = strings.Replace(v, " ", `\ `, -1)
// Escape all spaces and dollar signs
v = strings.ReplaceAll(strings.ReplaceAll(v, `$`, `\$`), ` `, `\ `)
injectedEnv = injectedEnv + fmt.Sprintf(" %s=%s ", k, v)
if verbose {
util.Warning(`%s="%s"`, k, v)
util.Warning(`%s=%s`, k, v)
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions cmd/ddev/cmd/addon-get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func TestCmdAddonGetWithDotEnv(t *testing.T) {
busyboxEnvFile := filepath.Join(site.Dir, ".ddev/.env.busybox")
require.NoFileExists(t, busyboxEnvFile, ".ddev/.env.busybox file should not exist at this point")

out, err = exec.RunHostCommand(DdevBin, "dotenv", "set", ".ddev/.env.busybox", "--busybox-tag=1.36.0", "--pre-install-variable=pre", "--pre-install-variable2=pre2", "--post-install-variable", "post", "--force-run")
out, err = exec.RunHostCommand(DdevBin, "dotenv", "set", ".ddev/.env.busybox", "--busybox-tag=1.36.0", "--pre-install-variable=pre", "--pre-install-variable2=pre2", "--post-install-variable", "post", "--force-run", "--dollar-sign", `$dollar_variable`)
require.NoError(t, err, "out=%s", out)
out, err = exec.RunHostCommand(DdevBin, "add-on", "get", filepath.Join(origDir, "testdata", t.Name(), "busybox"))
require.NoError(t, err, "out=%s", out)
Expand All @@ -202,6 +202,7 @@ func TestCmdAddonGetWithDotEnv(t *testing.T) {
require.Contains(t, string(busyboxEnvFileContents), `BUSYBOX_TAG="1.36.0"`)
require.Contains(t, string(busyboxEnvFileContents), `PRE_INSTALL_VARIABLE="pre"`)
require.Contains(t, string(busyboxEnvFileContents), `POST_INSTALL_VARIABLE="post"`)
require.Contains(t, string(busyboxEnvFileContents), `DOLLAR_SIGN="\$dollar_variable"`)
// --force-run is converted to empty string
require.Contains(t, string(busyboxEnvFileContents), `FORCE_RUN=""`)

Expand Down Expand Up @@ -231,6 +232,7 @@ func TestCmdAddonGetWithDotEnv(t *testing.T) {
require.Contains(t, out, "PRE_INSTALL_VARIABLE=pre")
require.Contains(t, out, "POST_INSTALL_VARIABLE=post")
require.Contains(t, out, "THIS_VARIABLE_CAN_BE_CHANGED_FROM_ENV=true")
require.Contains(t, out, `DOLLAR_SIGN=$dollar_variable`)
// Variables from *.example files should not be here
require.NotContains(t, out, "WEB_EXAMPLE_VARIABLE")
require.Contains(t, out, "BUSYBOX_EXAMPLE_VARIABLE=notset")
Expand All @@ -251,7 +253,7 @@ func TestCmdAddonGetWithDotEnv(t *testing.T) {

// Update the busybox image in .ddev/.env.busybox
// And update the value for THIS_VARIABLE_CAN_BE_CHANGED_FROM_ENV
out, err = exec.RunHostCommand(DdevBin, "dotenv", "set", ".ddev/.env.busybox", "--busybox-tag", "1.36.1", "--this-variable-can-be-changed-from-env=changed")
out, err = exec.RunHostCommand(DdevBin, "dotenv", "set", ".ddev/.env.busybox", "--busybox-tag", "1.36.1", "--this-variable-can-be-changed-from-env=changed", "--dollar-sign", `$dollar_variable_override`)
require.NoError(t, err, "out=%s", out)

out, err = exec.RunHostCommand(DdevBin, "restart")
Expand All @@ -272,6 +274,7 @@ func TestCmdAddonGetWithDotEnv(t *testing.T) {
require.Contains(t, out, "POST_INSTALL_VARIABLE=post")
// The variable below is already added to the busybox environment stanza.
require.Contains(t, out, "THIS_VARIABLE_CAN_BE_CHANGED_FROM_ENV=changed")
require.Contains(t, out, `DOLLAR_SIGN=$dollar_variable_override`)
require.NotContains(t, out, "THIS_VARIABLE_CAN_BE_CHANGED_FROM_ENV=true")
// Variables from *.example files should not be here
require.NotContains(t, out, "WEB_EXAMPLE_VARIABLE")
Expand Down
2 changes: 1 addition & 1 deletion cmd/ddev/cmd/dotenv-set.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ ddev dotenv set .ddev/.env.redis --redis-tag 7-bookworm`,
// Prepare the friendly message with formatted environment variables
var formattedVars []string
for _, k := range rawResultKeys {
formattedVars = append(formattedVars, fmt.Sprintf(`%s="%v"`, k, strings.ReplaceAll(changedEnvMap[k], `"`, `\"`)))
formattedVars = append(formattedVars, fmt.Sprintf(`%s=%v`, k, ddevapp.EscapeEnvFileValue(changedEnvMap[k])))
}
friendlyMsg := fmt.Sprintf("Updated %s with:\n\n%s", envFile, strings.Join(formattedVars, "\n"))
output.UserOut.WithField("raw", changedEnvMap).Print(friendlyMsg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ project_files:
pre_install_actions:
- echo "PRE_INSTALL_VARIABLE=${PRE_INSTALL_VARIABLE:-notset}"
- echo "PRE_INSTALL_VARIABLE2=${PRE_INSTALL_VARIABLE2:-notset}"
- echo "DOLLAR_SIGN=${DOLLAR_SIGN:-notset}"

post_install_actions:
- echo "POST_INSTALL_VARIABLE=${POST_INSTALL_VARIABLE:-notset}"
6 changes: 5 additions & 1 deletion pkg/ddevapp/compose_yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"gopkg.in/yaml.v3"
"os"
"path/filepath"
"strings"
//compose_cli "github.com/compose-spec/compose-go/cli"
//compose_types "github.com/compose-spec/compose-go/types"
)
Expand Down Expand Up @@ -166,7 +167,10 @@ func fixupComposeYaml(yamlStr string, app *DdevApp) (map[string]interface{}, err
}
if environmentMap, ok := serviceMap["environment"].(map[string]interface{}); ok {
for envKey, envValue := range envMap {
environmentMap[envKey] = envValue
// Escape $ characters in environment variables
// The same thing is done in `docker-compose config`
// See https://github.com/docker/compose/blob/361c0893a9e16d54f535cdb2e764362363d40702/cmd/compose/config.go#L405-L409
environmentMap[envKey] = strings.ReplaceAll(envValue, `$`, `$$`)
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/ddevapp/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -888,6 +888,9 @@ func TestPHPConfig(t *testing.T) {
assert.Contains(out, "IS_DDEV_PROJECT=true")
// Make sure the .ddev/.env file works
assert.Contains(out, "SOMEENV=someenv-value")
assert.Contains(out, "DOLLAR_SINGLE_QUOTES=$SOMEENV $ sign")
assert.Contains(out, "DOLLAR_DOUBLE_QUOTES=someenv-value $ sign")
assert.Contains(out, "DOLLAR_DOUBLE_QUOTES_ESCAPED=$SOMEENV $ sign")

// Remove the PHP84 exception when it has missing extensions
if v != nodeps.PHP84 {
Expand Down
16 changes: 14 additions & 2 deletions pkg/ddevapp/envfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ func ReadProjectEnvFile(envFilePath string) (envMap map[string]string, envText s
func WriteProjectEnvFile(envFilePath string, envMap map[string]string, envText string) error {
// envFilePath := filepath.Join(app.AppRoot, ".env")
for k, v := range envMap {
// Escape double quotes in the value, since we wrap it in double quotes
v = `"` + strings.ReplaceAll(v, `"`, `\"`) + `"`
v = EscapeEnvFileValue(v)
// If the item is already in envText, use regex to replace it
// otherwise, append it to the envText.
// (^|[\r\n]+) - first group $1 matches the start of a line or newline characters
Expand Down Expand Up @@ -55,3 +54,16 @@ func WriteProjectEnvFile(envFilePath string, envMap map[string]string, envText s
}
return nil
}

// EscapeEnvFileValue escapes the value so it can be used in an .env file
// The value is wrapped in double quotes for correct work with spaces.
func EscapeEnvFileValue(value string) string {
value = strings.NewReplacer(
// Escape all dollar signs so they are not interpreted as bash variables
`$`, `\$`,
// Escape all double quotes since we wrap the value in double quotes
`"`, `\"`,
).Replace(value)
// Wrap the value in double quotes
return `"` + value + `"`
}
3 changes: 3 additions & 0 deletions pkg/ddevapp/testdata/TestPHPConfig/.ddev/.env
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
SOMEENV=someenv-value
DOLLAR_SINGLE_QUOTES='$SOMEENV $ sign'
DOLLAR_DOUBLE_QUOTES="$SOMEENV $ sign"
DOLLAR_DOUBLE_QUOTES_ESCAPED="\$SOMEENV \$ sign"
3 changes: 3 additions & 0 deletions pkg/ddevapp/testdata/TestPHPConfig/phpinfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,8 @@

print "phpversion=" . phpversion() . "\n";
print "SOMEENV=" . getenv("SOMEENV"). "\n";
print "DOLLAR_SINGLE_QUOTES=" . getenv("DOLLAR_SINGLE_QUOTES"). "\n";
print "DOLLAR_DOUBLE_QUOTES=" . getenv("DOLLAR_DOUBLE_QUOTES"). "\n";
print "DOLLAR_DOUBLE_QUOTES_ESCAPED=" . getenv("DOLLAR_DOUBLE_QUOTES_ESCAPED"). "\n";
print "IS_DDEV_PROJECT=" . getenv("IS_DDEV_PROJECT"). "\n";
print "loaded_extensions=" . implode(",", get_loaded_extensions()) . "\n";

0 comments on commit da51ad4

Please sign in to comment.