diff --git a/cmd/server.go b/cmd/server.go index 9343220326..b1876c05d7 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -101,6 +101,7 @@ const ( SSLCertFileFlag = "ssl-cert-file" SSLKeyFileFlag = "ssl-key-file" TFDownloadURLFlag = "tf-download-url" + VarFileAllowlistFlag = "var-file-allowlist" VCSStatusName = "vcs-status-name" TFEHostnameFlag = "tfe-hostname" TFETokenFlag = "tfe-token" @@ -307,6 +308,10 @@ var stringFlags = map[string]stringFlag{ description: "Terraform version to default to (ex. v0.12.0). Will download if not yet on disk." + " If not set, Atlantis uses the terraform binary in its PATH.", }, + VarFileAllowlistFlag: { + description: "Comma-separated list of additional paths where variable definition files can be read from." + + " If this argument is not provided, it defaults to Atlantis' data directory, determined by the --data-dir argument.", + }, VCSStatusName: { description: "Name used to identify Atlantis for pull request statuses.", defaultValue: DefaultVCSStatusName, @@ -609,6 +614,7 @@ func (s *ServerCmd) run() error { if err := s.setDataDir(&userConfig); err != nil { return err } + s.setVarFileAllowlist(&userConfig) if err := s.deprecationWarnings(&userConfig); err != nil { return err } @@ -814,6 +820,14 @@ func (s *ServerCmd) setDataDir(userConfig *server.UserConfig) error { return nil } +// setVarFileAllowlist checks if var-file-allowlist is unassigned and makes it default to data-dir for better backward +// compatibility. +func (s *ServerCmd) setVarFileAllowlist(userConfig *server.UserConfig) { + if userConfig.VarFileAllowlist == "" { + userConfig.VarFileAllowlist = userConfig.DataDir + } +} + // trimAtSymbolFromUsers trims @ from the front of the github and gitlab usernames func (s *ServerCmd) trimAtSymbolFromUsers(userConfig *server.UserConfig) { userConfig.GithubUser = strings.TrimPrefix(userConfig.GithubUser, "@") diff --git a/cmd/server_test.go b/cmd/server_test.go index 19b1f8e1ea..456be9cdd1 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -131,11 +131,12 @@ func TestExecute_Defaults(t *testing.T) { Ok(t, err) strExceptions := map[string]string{ - GHUserFlag: "user", - GHTokenFlag: "token", - DataDirFlag: dataDir, - AtlantisURLFlag: "http://" + hostname + ":4141", - RepoAllowlistFlag: "*", + GHUserFlag: "user", + GHTokenFlag: "token", + DataDirFlag: dataDir, + AtlantisURLFlag: "http://" + hostname + ":4141", + RepoAllowlistFlag: "*", + VarFileAllowlistFlag: dataDir, } strIgnore := map[string]bool{ "config": true, diff --git a/runatlantis.io/docs/server-configuration.md b/runatlantis.io/docs/server-configuration.md index 3deea59bfb..ca3a44b5c6 100644 --- a/runatlantis.io/docs/server-configuration.md +++ b/runatlantis.io/docs/server-configuration.md @@ -634,6 +634,14 @@ Values are chosen in this order: ``` A token for Terraform Cloud/Terraform Enterprise integration. See [Terraform Cloud](terraform-cloud.html) for more details. +* ### `--var-file-allowlist` + ```bash + atlantis server --var-file-allowlist='/path/to/tfvars/dir' + ``` + Comma-separated list of additional directory paths where [variable definition files](https://www.terraform.io/language/values/variables#variable-definitions-tfvars-files) can be read from. + The paths in this argument should be absolute paths. Relative paths and globbing are currently not supported. + If this argument is not provided, it defaults to Atlantis' data directory, determined by the `--data-dir` argument. + * ### `--vcs-status-name` ```bash atlantis server --vcs-status-name="atlantis-dev" diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 00d1aa932d..0bdf0ba88d 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -122,6 +122,7 @@ type DefaultCommandRunner struct { PostWorkflowHooksCommandRunner PostWorkflowHooksCommandRunner PullStatusFetcher PullStatusFetcher TeamAllowlistChecker *TeamAllowlistChecker + VarFileAllowlistChecker *VarFileAllowlistChecker } // RunAutoplanCommand runs plan and policy_checks when a pull request is opened or updated. @@ -205,6 +206,15 @@ func (c *DefaultCommandRunner) checkUserPermissions(repo models.Repo, user model return true, nil } +// checkVarFilesInPlanCommandAllowlisted checks if paths in a 'plan' command are allowlisted. +func (c *DefaultCommandRunner) checkVarFilesInPlanCommandAllowlisted(cmd *CommentCommand) error { + if cmd == nil || cmd.CommandName() != command.Plan { + return nil + } + + return c.VarFileAllowlistChecker.Check(cmd.Flags) +} + // RunCommentCommand executes the command. // We take in a pointer for maybeHeadRepo because for some events there isn't // enough data to construct the Repo model and callers might want to wait until @@ -241,6 +251,15 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead return } + // Check if the provided var files in a 'plan' command are allowlisted + if err := c.checkVarFilesInPlanCommandAllowlisted(cmd); err != nil { + errMsg := fmt.Sprintf("```\n%s\n```", err.Error()) + if commentErr := c.VCSClient.CreateComment(baseRepo, pullNum, errMsg, ""); commentErr != nil { + c.Logger.Err("unable to comment on pull request: %s", commentErr) + } + return + } + headRepo, pull, err := c.ensureValidRepoMetadata(baseRepo, maybeHeadRepo, maybePull, user, pullNum, log) if err != nil { return diff --git a/server/events/var_file_allowlist_checker.go b/server/events/var_file_allowlist_checker.go new file mode 100644 index 0000000000..fde37aafa7 --- /dev/null +++ b/server/events/var_file_allowlist_checker.go @@ -0,0 +1,76 @@ +package events + +import ( + "fmt" + "github.com/pkg/errors" + "path/filepath" + "strings" +) + +// VarFileAllowlistChecker implements checking if paths are allowlisted to be used with +// this Atlantis. +type VarFileAllowlistChecker struct { + rules []string +} + +// NewVarFileAllowlistChecker constructs a new checker and validates that the +// allowlist isn't malformed. +func NewVarFileAllowlistChecker(allowlist string) (*VarFileAllowlistChecker, error) { + var rules []string + paths := strings.Split(allowlist, ",") + if paths[0] != "" { + for _, path := range paths { + absPath, err := filepath.Abs(path) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("converting allowlist %q to absolute path", path)) + } + rules = append(rules, absPath) + } + } + return &VarFileAllowlistChecker{ + rules: rules, + }, nil +} + +func (p *VarFileAllowlistChecker) Check(flags []string) error { + for i, flag := range flags { + var path string + if i < len(flags)-1 && flag == "-var-file" { + // Flags are in the format of []{"-var-file", "my-file.tfvars"} + path = flags[i+1] + } else { + flagSplit := strings.Split(flag, "=") + // Flags are in the format of []{"-var-file=my-file.tfvars"} + if len(flagSplit) == 2 && flagSplit[0] == "-var-file" { + path = flagSplit[1] + } + } + + if path != "" && !p.isAllowedPath(path) { + return fmt.Errorf("var file path %s is not allowed by the current allowlist: [%s]", + path, strings.Join(p.rules, ", ")) + } + } + return nil +} + +func (p *VarFileAllowlistChecker) isAllowedPath(path string) bool { + path = filepath.Clean(path) + + // If the path is within the repo directory, return true without checking the rules. + if !filepath.IsAbs(path) { + if !strings.HasPrefix(path, "..") && !strings.HasPrefix(path, "~") { + return true + } + } + + // Check the path against the rules. + for _, rule := range p.rules { + rel, err := filepath.Rel(rule, path) + if err == nil && !strings.HasPrefix(rel, "..") { + return true + } + } + + return false +} diff --git a/server/events/var_file_allowlist_checker_test.go b/server/events/var_file_allowlist_checker_test.go new file mode 100644 index 0000000000..10eb38b360 --- /dev/null +++ b/server/events/var_file_allowlist_checker_test.go @@ -0,0 +1,128 @@ +package events_test + +import ( + "testing" + + "github.com/runatlantis/atlantis/server/events" + . "github.com/runatlantis/atlantis/testing" +) + +func TestVarFileAllowlistChecker_IsAllowlisted(t *testing.T) { + cases := []struct { + Description string + Allowlist string + Flags []string + ExpErr string + }{ + { + "Empty Allowlist, no var file", + "", + []string{""}, + "", + }, + { + "Empty Allowlist, single var file under the repo directory", + "", + []string{"-var-file=test.tfvars"}, + "", + }, + { + "Empty Allowlist, single var file under the repo directory, specified in separate flags", + "", + []string{"-var-file", "test.tfvars"}, + "", + }, + { + "Empty Allowlist, single var file under the subdirectory of the repo directory", + "", + []string{"-var-file=sub/test.tfvars"}, + "", + }, + { + "Empty Allowlist, single var file outside the repo directory", + "", + []string{"-var-file=/path/to/file"}, + "var file path /path/to/file is not allowed by the current allowlist: []", + }, + { + "Empty Allowlist, single var file under the parent directory of the repo directory", + "", + []string{"-var-file=../test.tfvars"}, + "var file path ../test.tfvars is not allowed by the current allowlist: []", + }, + { + "Empty Allowlist, single var file under the home directory", + "", + []string{"-var-file=~/test.tfvars"}, + "var file path ~/test.tfvars is not allowed by the current allowlist: []", + }, + { + "Single path in allowlist, no var file", + "/path", + []string{""}, + "", + }, + { + "Single path in allowlist, single var file under the repo directory", + "/path", + []string{"-var-file=test.tfvars"}, + "", + }, + { + "Single path in allowlist, single var file under the allowlisted directory", + "/path", + []string{"-var-file=/path/test.tfvars"}, + "", + }, + { + "Single path with ending slash in allowlist, single var file under the allowlisted directory", + "/path/", + []string{"-var-file=/path/test.tfvars"}, + "", + }, + { + "Single path in allowlist, single var file in the parent directory of the repo directory", + "/path", + []string{"-var-file=../test.tfvars"}, + "var file path ../test.tfvars is not allowed by the current allowlist: [/path]", + }, + { + "Single path in allowlist, single var file outside the allowlisted directory", + "/path", + []string{"-var-file=/path_not_allowed/test.tfvars"}, + "var file path /path_not_allowed/test.tfvars is not allowed by the current allowlist: [/path]", + }, + { + "Single path in allowlist, single var file in the parent directory of the allowlisted directory", + "/path", + []string{"-var-file=/test.tfvars"}, + "var file path /test.tfvars is not allowed by the current allowlist: [/path]", + }, + { + "Root path in allowlist, with multiple var files", + "/", + []string{"-var-file=test.tfvars", "-var-file=/path/test.tfvars", "-var-file=/test.tfvars"}, + "", + }, + { + "Multiple paths in allowlist, with multiple var files under allowlisted directories", + "/path,/another/path", + []string{"-var-file=test.tfvars", "-var-file", "/path/test.tfvars", "unused-flag", "-var-file=/another/path/sub/test.tfvars"}, + "", + }, + } + + for _, c := range cases { + t.Run(c.Description, func(t *testing.T) { + v, err := events.NewVarFileAllowlistChecker(c.Allowlist) + Ok(t, err) + + err = v.Check(c.Flags) + if c.ExpErr != "" { + ErrEquals(t, c.ExpErr, err) + } else { + Ok(t, err) + } + }) + } +} diff --git a/server/server.go b/server/server.go index 047f840eff..738530cf12 100644 --- a/server/server.go +++ b/server/server.go @@ -673,6 +673,10 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { if err != nil { return nil, err } + varFileAllowlistChecker, err := events.NewVarFileAllowlistChecker(userConfig.VarFileAllowlist) + if err != nil { + return nil, err + } commandRunner := &events.DefaultCommandRunner{ VCSClient: vcsClient, @@ -694,6 +698,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { PostWorkflowHooksCommandRunner: postWorkflowHooksCommandRunner, PullStatusFetcher: boltdb, TeamAllowlistChecker: githubTeamAllowlistChecker, + VarFileAllowlistChecker: varFileAllowlistChecker, } repoAllowlist, err := events.NewRepoAllowlistChecker(userConfig.RepoAllowlist) if err != nil { diff --git a/server/user_config.go b/server/user_config.go index cab3d8d9f5..0dc858db8c 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -86,6 +86,7 @@ type UserConfig struct { TFDownloadURL string `mapstructure:"tf-download-url"` TFEHostname string `mapstructure:"tfe-hostname"` TFEToken string `mapstructure:"tfe-token"` + VarFileAllowlist string `mapstructure:"var-file-allowlist"` VCSStatusName string `mapstructure:"vcs-status-name"` DefaultTFVersion string `mapstructure:"default-tf-version"` Webhooks []WebhookConfig `mapstructure:"webhooks"`