From 85ab7475f8cf30e9b28b021d4f7c15df7c32da0e Mon Sep 17 00:00:00 2001 From: Luke Kysow Date: Mon, 19 Mar 2018 20:27:02 -0700 Subject: [PATCH] Validate workspace the same way as Terraform. Saw in the TF code how they were validating workspace names and decided to do the same. '..' is actually a valid TF workspace but we don't support that. --- server/events/comment_parser.go | 10 ++++++---- server/events/comment_parser_test.go | 6 ++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/server/events/comment_parser.go b/server/events/comment_parser.go index 7445cda96d..198439e71f 100644 --- a/server/events/comment_parser.go +++ b/server/events/comment_parser.go @@ -16,6 +16,7 @@ package events import ( "fmt" "io/ioutil" + "net/url" "path/filepath" "strings" @@ -189,10 +190,11 @@ func (e *CommentParser) Parse(comment string, vcsHost vcs.Host) CommentParseResu return CommentParseResult{CommentResponse: e.errMarkdown(err.Error(), command, flagSet)} } - // Because we use the workspace name as a file, need to make sure it's - // not doing something weird like being a relative dir. - if strings.Contains(workspace, "..") { - return CommentParseResult{CommentResponse: e.errMarkdown(fmt.Sprintf("value for -%s/--%s can't contain '..'", WorkspaceFlagShort, WorkspaceFlagLong), command, flagSet)} + // Use the same validation that Terraform uses: https://git.io/vxGhU. Plus + // we also don't allow '..'. We don't want the workspace to contain a path + // since we create files based on the name. + if workspace != url.PathEscape(workspace) || strings.Contains(workspace, "..") { + return CommentParseResult{CommentResponse: e.errMarkdown(fmt.Sprintf("invalid workspace: %q", workspace), command, flagSet)} } return CommentParseResult{ diff --git a/server/events/comment_parser_test.go b/server/events/comment_parser_test.go index 41b556048a..8edbc1417b 100644 --- a/server/events/comment_parser_test.go +++ b/server/events/comment_parser_test.go @@ -240,10 +240,12 @@ func TestParse_RelativeDirPath(t *testing.T) { } func TestParse_InvalidWorkspace(t *testing.T) { - t.Log("if -w is used with '..', should return an error") + t.Log("if -w is used with '..' or '/', should return an error") comments := []string{ "atlantis plan -w ..", "atlantis apply -w ..", + "atlantis plan -w /", + "atlantis apply -w /", "atlantis plan -w ..abc", "atlantis apply -w abc..", "atlantis plan -w abc..abc", @@ -251,7 +253,7 @@ func TestParse_InvalidWorkspace(t *testing.T) { } for _, c := range comments { r := commentParser.Parse(c, vcs.Github) - exp := "Error: value for -w/--workspace can't contain '..'" + exp := "Error: invalid workspace" Assert(t, strings.Contains(r.CommentResponse, exp), "For comment %q expected CommentResponse %q to contain %q", c, r.CommentResponse, exp) }