From 9d118325b36764867e94e16e43de39feeb1c04da Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 27 Mar 2017 16:16:09 -0400 Subject: [PATCH] Reject names that aren't url-safe Environment names can be used in a number of contexts, and should be properly escaped for safety. Since most state names are store in path structures, and often in a URL, use `url.PathEscape` to check for disallowed characters --- command/env_command.go | 17 ++++++++++++++++- command/env_command_test.go | 38 +++++++++++++++++++++++++++++++++++++ command/env_delete.go | 5 +++++ command/env_new.go | 5 +++++ command/env_select.go | 4 ++++ 5 files changed, 68 insertions(+), 1 deletion(-) diff --git a/command/env_command.go b/command/env_command.go index 481c0092c..9548122a1 100644 --- a/command/env_command.go +++ b/command/env_command.go @@ -1,6 +1,9 @@ package command -import "strings" +import ( + "net/url" + "strings" +) // EnvCommand is a Command Implementation that manipulates local state // environments. @@ -39,6 +42,13 @@ func (c *EnvCommand) Synopsis() string { return "Environment management" } +// validEnvName returns true is this name is valid to use as an environment name. +// Since most named states are accessed via a filesystem path or URL, check if +// escaping the name would be required. +func validEnvName(name string) bool { + return name == url.PathEscape(name) +} + const ( envNotSupported = `Backend does not support environments` @@ -81,5 +91,10 @@ Environment %[1]q is your active environment! You cannot delete the currently active environment. Please switch to another environment and try again. +` + + envInvalidName = ` +The environment name %q is not allowed. The name must contain only URL safe +characters, and no path separators. ` ) diff --git a/command/env_command_test.go b/command/env_command_test.go index 0b28beb01..5a04d8db0 100644 --- a/command/env_command_test.go +++ b/command/env_command_test.go @@ -103,6 +103,44 @@ func TestEnv_createAndList(t *testing.T) { } } +// Don't allow names that aren't URL safe +func TestEnv_createInvalid(t *testing.T) { + // Create a temporary working directory that is empty + td := tempDir(t) + os.MkdirAll(td, 0755) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + newCmd := &EnvNewCommand{} + + envs := []string{"test_a*", "test_b/foo", "../../../test_c", "好_d"} + + // create multiple envs + for _, env := range envs { + ui := new(cli.MockUi) + newCmd.Meta = Meta{Ui: ui} + if code := newCmd.Run([]string{env}); code == 0 { + t.Fatalf("expected failure: \n%s", ui.OutputWriter) + } + } + + // list envs to make sure none were created + listCmd := &EnvListCommand{} + ui := new(cli.MockUi) + listCmd.Meta = Meta{Ui: ui} + + if code := listCmd.Run(nil); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter) + } + + actual := strings.TrimSpace(ui.OutputWriter.String()) + expected := "* default" + + if actual != expected { + t.Fatalf("\nexpected: %q\nactual: %q", expected, actual) + } +} + func TestEnv_createWithState(t *testing.T) { td := tempDir(t) os.MkdirAll(td, 0755) diff --git a/command/env_delete.go b/command/env_delete.go index a9958f8aa..be21b76ed 100644 --- a/command/env_delete.go +++ b/command/env_delete.go @@ -32,6 +32,11 @@ func (c *EnvDeleteCommand) Run(args []string) int { delEnv := args[0] + if !validEnvName(delEnv) { + c.Ui.Error(fmt.Sprintf(envInvalidName, delEnv)) + return 1 + } + configPath, err := ModulePath(args[1:]) if err != nil { c.Ui.Error(err.Error()) diff --git a/command/env_new.go b/command/env_new.go index 5f4999e24..8b0e8fcdb 100644 --- a/command/env_new.go +++ b/command/env_new.go @@ -35,6 +35,11 @@ func (c *EnvNewCommand) Run(args []string) int { newEnv := args[0] + if !validEnvName(newEnv) { + c.Ui.Error(fmt.Sprintf(envInvalidName, newEnv)) + return 1 + } + configPath, err := ModulePath(args[1:]) if err != nil { c.Ui.Error(err.Error()) diff --git a/command/env_select.go b/command/env_select.go index 073f92ac5..e7bc8743e 100644 --- a/command/env_select.go +++ b/command/env_select.go @@ -39,6 +39,10 @@ func (c *EnvSelectCommand) Run(args []string) int { } name := args[0] + if !validEnvName(name) { + c.Ui.Error(fmt.Sprintf(envInvalidName, name)) + return 1 + } states, err := b.States() if err != nil {