Add --var-file-allowlist in server configuration (#2362)

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, "@")

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,

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"

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

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
+}

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)
+			}
+		})
+	}
+}

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 {

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"`