Skip to content

Commit

Permalink
Restrict possible project names.
Browse files Browse the repository at this point in the history
We should only allow project names (which are specified in an
atlantis.yaml file) that don't need to be url escaped. The one
exceptional character is '/' which we allow because users like to name
their projects to match the directory they're in. We use the same rule
that Terraform uses for workspace names.

I've also changed the characters that are replaced when writing out the
plan filename to only remove invalid filename characters instead of just
allowing alphanumeric. This is the smallest amount of change required to
ensure the filename is valid.
  • Loading branch information
lkysow committed Oct 30, 2018
1 parent b652568 commit 2d214b2
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 13 deletions.
5 changes: 4 additions & 1 deletion server/events/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ func MustConstraint(constraint string) version.Constraints {
return c
}

var invalidFilenameChars = regexp.MustCompile(`[^a-zA-Z0-9-_\.]`)
// invalidFilenameChars matches chars that are invalid for linux and windows
// filenames.
// From https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch08s25.html
var invalidFilenameChars = regexp.MustCompile(`[\\/:"*?<>|]`)

// GetPlanFilename returns the filename (not the path) of the generated tf plan
// given a workspace and maybe a project's config.
Expand Down
21 changes: 9 additions & 12 deletions server/events/runtime/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,11 @@ func TestGetPlanFilename(t *testing.T) {
nil,
"workspace.tfplan",
},
{
"workspace with space",
nil,
"workspace-with-space.tfplan",
},
{
"workspace",
&valid.Project{},
"workspace.tfplan",
},
{
"workspace with space",
&valid.Project{},
"workspace-with-space.tfplan",
},
{
"workspace",
&valid.Project{
Expand All @@ -54,14 +44,21 @@ func TestGetPlanFilename(t *testing.T) {
&valid.Project{
Name: String("project with space"),
},
"project-with-space-workspace.tfplan",
"project with space-workspace.tfplan",
},
{
"workspace😀",
&valid.Project{
Name: String("project😀"),
},
"project--workspace-.tfplan",
"project😀-workspace😀.tfplan",
},
{
"default",
&valid.Project{
Name: String(`all.invalid.chars \/"*?<>`),
},
"all.invalid.chars --------default.tfplan",
},
}

Expand Down
13 changes: 13 additions & 0 deletions server/events/yaml/raw/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package raw

import (
"fmt"
"net/url"
"path/filepath"
"strings"

Expand Down Expand Up @@ -58,6 +59,9 @@ func (p Project) Validate() error {
if *strPtr == "" {
return errors.New("if set cannot be empty")
}
if !validProjectName(*strPtr) {
return fmt.Errorf("%q is not allowed: must contain only URL safe characters", *strPtr)
}
return nil
}
return validation.ValidateStruct(&p,
Expand Down Expand Up @@ -99,3 +103,12 @@ func (p Project) ToValid() valid.Project {

return v
}

// validProjectName returns true if the project name is valid.
// Since the name might be used in URLs and definitely in files we don't
// support any characters that must be url escaped *except* for '/' because
// users like to name their projects to match the directory it's in.
func validProjectName(name string) bool {
nameWithoutSlashes := strings.Replace(name, "/", "-", -1)
return nameWithoutSlashes == url.QueryEscape(nameWithoutSlashes)
}
40 changes: 40 additions & 0 deletions server/events/yaml/raw/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,46 @@ func TestProject_Validate(t *testing.T) {
},
expErr: "name: if set cannot be empty.",
},
{
description: "project name with slashes",
input: raw.Project{
Dir: String("."),
Name: String("my/name"),
},
expErr: "",
},
{
description: "project name with emoji",
input: raw.Project{
Dir: String("."),
Name: String("😀"),
},
expErr: "name: \"😀\" is not allowed: must contain only URL safe characters.",
},
{
description: "project name with spaces",
input: raw.Project{
Dir: String("."),
Name: String("name with spaces"),
},
expErr: "name: \"name with spaces\" is not allowed: must contain only URL safe characters.",
},
{
description: "project name with +",
input: raw.Project{
Dir: String("."),
Name: String("namewith+"),
},
expErr: "name: \"namewith+\" is not allowed: must contain only URL safe characters.",
},
{
description: `project name with \`,
input: raw.Project{
Dir: String("."),
Name: String(`namewith\`),
},
expErr: `name: "namewith\\" is not allowed: must contain only URL safe characters.`,
},
}
validation.ErrorTag = "yaml"
for _, c := range cases {
Expand Down

0 comments on commit 2d214b2

Please sign in to comment.