Skip to content

Commit

Permalink
feat: add validation for ddev dotenv flags, for ddev#6406 (ddev#6615)
Browse files Browse the repository at this point in the history
  • Loading branch information
stasadev authored Oct 16, 2024
1 parent 96aaa50 commit b6c3c7b
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 15 deletions.
6 changes: 1 addition & 5 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", "-A", "short_flag_VALUE", "--force-run", "-yes", "--force-UPPERCASE=true")
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")
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 @@ -204,10 +204,6 @@ func TestCmdAddonGetWithDotEnv(t *testing.T) {
require.Contains(t, string(busyboxEnvFileContents), `POST_INSTALL_VARIABLE="post"`)
// --force-run is converted to empty string
require.Contains(t, string(busyboxEnvFileContents), `FORCE_RUN=""`)
// Some flags are ignored because they should not contain uppercase, or be a wrong flag:
require.NotContains(t, string(busyboxEnvFileContents), `A="short_flag_VALUE"`)
require.NotContains(t, string(busyboxEnvFileContents), "YES")
require.NotContains(t, string(busyboxEnvFileContents), "FORCE_UPPERCASE")

out, err = exec.RunHostCommand(DdevBin, "add-on", "get", filepath.Join(origDir, "testdata", t.Name(), "bare-busybox"))
require.NoError(t, err, "out=%s", out)
Expand Down
7 changes: 5 additions & 2 deletions cmd/ddev/cmd/dotenv-get.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ ddev dotenv get .ddev/.env.redis --redis-tag`,
}

// Get unknown flags and ensure only one flag is passed
envFlags := GetUnknownFlags(cmd)
envFlags, err := GetUnknownFlags(cmd)
if err != nil {
util.Failed("Error reading command flags: %v", err)
}
if len(envFlags) < 1 {
_ = cmd.Help()
return
Expand All @@ -69,7 +72,7 @@ ddev dotenv get .ddev/.env.redis --redis-tag`,
}

if !strings.HasPrefix(flag, "--") {
util.Failed("The flag must be in a long format.")
util.Failed("The flag must be in long format, but received %s", flag)
}

// Extract the environment variable name
Expand Down
7 changes: 6 additions & 1 deletion cmd/ddev/cmd/dotenv-set.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ ddev dotenv set .ddev/.env.redis --redis-tag 7-bookworm`,
}

// Get unknown flags and convert them to env variables
envSlice := GetUnknownFlags(cmd)
envSlice, err := GetUnknownFlags(cmd)
if err != nil {
util.Failed("Error reading command flags: %v", err)
}
hasUnknownFlags := false
changedEnvMap := make(map[string]string)
for flag, value := range envSlice {
Expand All @@ -72,6 +75,8 @@ ddev dotenv set .ddev/.env.redis --redis-tag 7-bookworm`,
envMap[envName] = value
changedEnvMap[envName] = value
hasUnknownFlags = true
} else {
util.Failed("The flag must be in long format, but received %s", flag)
}
}

Expand Down
14 changes: 13 additions & 1 deletion cmd/ddev/cmd/dotenv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,18 @@ func TestCmdDotEnvGetAndSet(t *testing.T) {
require.Error(t, err, "out=%s", out)
require.Contains(t, out, "The file should have .env prefix")

out, err = exec.RunHostCommand(DdevBin, "dotenv", "set", envFile, "-a", "custom value")
require.Error(t, err, "out=%s", out)
require.Contains(t, out, "flag must be in long format")

out, err = exec.RunHostCommand(DdevBin, "dotenv", "set", envFile, "--TEST", "custom value")
require.Error(t, err, "out=%s", out)
require.Contains(t, out, "the flag must be lowercase and start with a letter")

out, err = exec.RunHostCommand(DdevBin, "dotenv", "set", envFile, "--1test", "custom value")
require.Error(t, err, "out=%s", out)
require.Contains(t, out, "the flag must be lowercase and start with a letter")

out, err = exec.RunHostCommand(DdevBin, "dotenv", "get", envFile, "--test-value", "--test-value-2")
require.Error(t, err, "out=%s", out)
require.Contains(t, out, "one environment variable can be retrieved at a time")
Expand All @@ -85,7 +97,7 @@ func TestCmdDotEnvGetAndSet(t *testing.T) {

out, err = exec.RunHostCommand(DdevBin, "dotenv", "get", envFile, "-a")
require.Error(t, err, "out=%s", out)
require.Contains(t, out, "flag must be in a long format")
require.Contains(t, out, "flag must be in long format")

out, err = exec.RunHostCommand(DdevBin, "dotenv", "set", envFile, "--test-value-with-special-characters", `Test$variable\nwith\n_new_lines`)
require.NoError(t, err, "out=%s", out)
Expand Down
19 changes: 14 additions & 5 deletions cmd/ddev/cmd/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ func getRequestedProjectsExtended(names []string, all bool, withNonExisting bool
// Works only with this config in Cobra: FParseErrWhitelist: cobra.FParseErrWhitelist{UnknownFlags: true}
// If Cobra implements handling of unknown flags, this function can be removed/refactored.
// See https://github.com/spf13/cobra/issues/739
func GetUnknownFlags(cmd *cobra.Command) map[string]string {
func GetUnknownFlags(cmd *cobra.Command) (map[string]string, error) {
unknownFlags := make(map[string]string)
if len(os.Args) < 1 {
return unknownFlags
return unknownFlags, nil
}

// Known flags tracking
Expand All @@ -116,8 +116,17 @@ func GetUnknownFlags(cmd *cobra.Command) map[string]string {
args := os.Args[1:]
for i := 0; i < len(args); i++ {
arg := args[i]
// Skip if the argument is not a valid flag or a known flag
if !flagRegex.MatchString(arg) || knownFlags[arg] {
// Skip if the value is a known flag
if knownFlags[arg] {
continue
}

if !flagRegex.MatchString(arg) {
// Fail if the value is not a valid flag
if strings.HasPrefix(arg, "-") {
return unknownFlags, fmt.Errorf("the flag must be lowercase and start with a letter, but received %s", arg)
}
// Skip if the value is not a flag, but an argument
continue
}

Expand All @@ -134,5 +143,5 @@ func GetUnknownFlags(cmd *cobra.Command) map[string]string {
unknownFlags[arg] = ""
}
}
return unknownFlags
return unknownFlags, nil
}
2 changes: 1 addition & 1 deletion pkg/appimport/appimport.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func ValidateAsset(unexpandedAssetPath string, assetType string) (string, bool,
}

for _, ext := range extensions {
if strings.HasSuffix(assetPath, ext) {
if strings.HasSuffix(assetPath, "."+ext) {
return assetPath, true, nil
}
}
Expand Down

0 comments on commit b6c3c7b

Please sign in to comment.