diff --git a/server/events/comment_parser.go b/server/events/comment_parser.go index 7445cda96..198439e71 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 41b556048..8edbc1417 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) }