Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jul 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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, "@")
Expand Down
11 changes: 6 additions & 5 deletions cmd/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions runatlantis.io/docs/server-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
19 changes: 19 additions & 0 deletions server/events/command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
76 changes: 76 additions & 0 deletions server/events/var_file_allowlist_checker.go
Original file line number Diff line number Diff line change
@@ -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
}
128 changes: 128 additions & 0 deletions server/events/var_file_allowlist_checker_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
5 changes: 5 additions & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions server/user_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down