Repository: runatlantis/atlantis (language: go)

Issue:
Branch matcher does not work in atlantis v0.17.0
I saw that in v0.17.0 atlantis has added branch matcher feature https://github.com/runatlantis/atlantis/pull/1383, but when I tried it, its seem does not work.

1. update repo.yaml
```
repos:
  - id: /.*/
    branch: /(main|master)/
```
2. restart atlantis
3. create PR from branch test1 to branch test2

but atlantis still can be execute from that PR



Discussion / hints:
I think even if you specify a single `repo` object, atlantis actually has your repo (which is limited to master/main) and another global one that will match regardless. So yeah, it doesn't seem to be functional. I'm unfortunately no longer using this.

https://github.com/runatlantis/atlantis/blob/master/server/events/yaml/valid/global_cfg.go#L173
> I think even if you specify a single `repo` object, atlantis actually has your repo (which is limited to master/main) and another global one that will match regardless. So yeah, it doesn't seem to be functional. I'm unfortunately no longer using this.
> 
> https://github.com/runatlantis/atlantis/blob/master/server/events/yaml/valid/global_cfg.go#L173

is there any workaround for this ? i'm currently using bitbucket server (stash) and need this feature so atlantis cannot be abused using other branch 
Hi, @ericsonrumuy7 and all.
I also hit this problem and wrote a custom shell script  in `pre_workflow_hooks` as a workaround. It checks a base branch of pull request and generates an invalid `atlantis.yaml` to suppress plan if the base branch doesn't match intended.

The following example restricts the base branch to default branch:


```repos.yaml
# Tested in atlantis v0.17.0

repos:
- id: /.*/
  apply_requirements: [approved, mergeable]
  pre_workflow_hooks:
    - run: pre_workflow_hooks.sh
```

```pre_workflow_hooks.sh
#!/bin/bash

set -euo pipefail

# pre_workflow_hooks.sh

# Get the default branch
DEFAULT_BRANCH=$(git remote show origin | grep 'HEAD branch' | sed 's/.*: //')
echo "BASE_BRANCH_NAME: $BASE_BRANCH_NAME"
echo "DEFAULT_BRANCH: $DEFAULT_BRANCH"

if [ "$BASE_BRANCH_NAME" != "$DEFAULT_BRANCH" ] ; then
  echo "generate an invalid atlantis.yaml to restrict executions of plan / apply on the pull request that are merged into the non-default branch for security reasons."
  cat << EOF > atlantis.yaml
version: invalid
EOF
  # Note that exit with non-zero actually doesn't stop the following plan phase. It only writes stdout to log with ERROR level.
  # Generating the invalid atlantis.yaml will cause an error in plan.
  exit 1
fi
```

It works for me, but it's not the best solution by all means.
@minamijoyo - so does the plan still run and put a comment on the PR?

I've got three atlantis instances running in three different AWS accounts, all tied to a single terragrunt repo.  So the repo has three hooks, one for each atlantis instance behind an ALB.

I was toying with the idea of generating an atlantis config that was "valid", but sets the `branch` or `id` to a non-existent branch / repo name.  The idea being my `prod` atlantis instance shouldn't comment on a PR in the `non-prod` folder in my terragrunt repo.


@jasonrberk (Aside from the original issue that the `branch` keyword in `repo.yaml` doesn't work,) yes, with `pre_workflow_hooks`, I think it's possible to generate a valid `atlantis.yml` based on the branch.

Fix patch:
diff --git a/server/events/command_runner.go b/server/events/command_runner.go
index 24c717a01d..cb15e75eb7 100644
--- a/server/events/command_runner.go
+++ b/server/events/command_runner.go
@@ -22,6 +22,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/runatlantis/atlantis/server/events/models"
 	"github.com/runatlantis/atlantis/server/events/vcs"
+	"github.com/runatlantis/atlantis/server/events/yaml/valid"
 	"github.com/runatlantis/atlantis/server/logging"
 	"github.com/runatlantis/atlantis/server/recovery"
 	gitlab "github.com/xanzy/go-gitlab"
@@ -95,6 +96,7 @@ type DefaultCommandRunner struct {
 	DisableAutoplan          bool
 	EventParser              EventParsing
 	Logger                   logging.SimpleLogging
+	GlobalCfg                valid.GlobalCfg
 	// AllowForkPRs controls whether we operate on pull requests from forks.
 	AllowForkPRs bool
 	// ParallelPoolSize controls the size of the wait group used to run
@@ -320,6 +322,13 @@ func (c *DefaultCommandRunner) validateCtxAndComment(ctx *CommandContext) bool {
 		}
 		return false
 	}
+
+	repo := c.GlobalCfg.MatchingRepo(ctx.Pull.BaseRepo.ID())
+	if !repo.BranchMatches(ctx.Pull.BaseBranch) {
+		ctx.Log.Info("command was run on a pull request which doesn't match base branches")
+		// just ignore it to allow us to use any git workflows without malicious intentions.
+		return false
+	}
 	return true
 }
 
diff --git a/server/events/pre_workflow_hooks_command_runner.go b/server/events/pre_workflow_hooks_command_runner.go
index 1f110db1d4..7619fd9a82 100644
--- a/server/events/pre_workflow_hooks_command_runner.go
+++ b/server/events/pre_workflow_hooks_command_runner.go
@@ -34,7 +34,7 @@ func (w *DefaultPreWorkflowHooksCommandRunner) RunPreHooks(
 
 	preWorkflowHooks := make([]*valid.PreWorkflowHook, 0)
 	for _, repo := range w.GlobalCfg.Repos {
-		if repo.IDMatches(baseRepo.ID()) && repo.BranchMatches(pull.BaseBranch) && len(repo.PreWorkflowHooks) > 0 {
+		if repo.IDMatches(baseRepo.ID()) && len(repo.PreWorkflowHooks) > 0 {
 			preWorkflowHooks = append(preWorkflowHooks, repo.PreWorkflowHooks...)
 		}
 	}
diff --git a/server/events/yaml/valid/global_cfg.go b/server/events/yaml/valid/global_cfg.go
index c2c4ced7db..81068d11ab 100644
--- a/server/events/yaml/valid/global_cfg.go
+++ b/server/events/yaml/valid/global_cfg.go
@@ -467,3 +467,15 @@ func (g GlobalCfg) getMatchingCfg(log logging.SimpleLogging, repoID string) (app
 	}
 	return
 }
+
+// MatchingRepo returns an instance of Repo which matches a given repoID.
+// If multiple repos match, return the last one for consistency with getMatchingCfg.
+func (g GlobalCfg) MatchingRepo(repoID string) *Repo {
+	for i := len(g.Repos) - 1; i >= 0; i-- {
+		repo := g.Repos[i]
+		if repo.IDMatches(repoID) {
+			return &repo
+		}
+	}
+	return nil
+}
diff --git a/server/server.go b/server/server.go
index fd90e15daa..1a05582843 100644
--- a/server/server.go
+++ b/server/server.go
@@ -599,6 +599,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
 		CommentCommandRunnerByCmd:     commentCommandRunnerByCmd,
 		EventParser:                   eventParser,
 		Logger:                        logger,
+		GlobalCfg:                     globalCfg,
 		AllowForkPRs:                  userConfig.AllowForkPRs,
 		AllowForkPRsFlag:              config.AllowForkPRsFlag,
 		SilenceForkPRErrors:           userConfig.SilenceForkPRErrors,


Test patch:
diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go
index 601e03bc15..fbffafee1d 100644
--- a/server/controllers/events/events_controller_e2e_test.go
+++ b/server/controllers/events/events_controller_e2e_test.go
@@ -883,6 +883,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl
 		GithubPullGetter:              e2eGithubGetter,
 		GitlabMergeRequestGetter:      e2eGitlabGetter,
 		Logger:                        logger,
+		GlobalCfg:                     globalCfg,
 		AllowForkPRs:                  allowForkPRs,
 		AllowForkPRsFlag:              "allow-fork-prs",
 		CommentCommandRunnerByCmd:     commentCommandRunnerByCmd,
diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go
index a236eb30d0..0184d226e2 100644
--- a/server/events/command_runner_test.go
+++ b/server/events/command_runner_test.go
@@ -16,6 +16,7 @@ package events_test
 import (
 	"errors"
 	"fmt"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -182,6 +183,8 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 
 	When(preWorkflowHooksCommandRunner.RunPreHooks(matchers.AnyPtrToEventsCommandContext())).ThenReturn(nil)
 
+	globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{})
+
 	ch = events.DefaultCommandRunner{
 		VCSClient:                     vcsClient,
 		CommentCommandRunnerByCmd:     commentCommandRunnerByCmd,
@@ -190,6 +193,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 		GitlabMergeRequestGetter:      gitlabGetter,
 		AzureDevopsPullGetter:         azuredevopsGetter,
 		Logger:                        logger,
+		GlobalCfg:                     globalCfg,
 		AllowForkPRs:                  false,
 		AllowForkPRsFlag:              "allow-fork-prs-flag",
 		Drainer:                       drainer,
@@ -404,6 +408,40 @@ func TestRunCommentCommand_ClosedPull(t *testing.T) {
 	vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, modelPull.Num, "Atlantis commands can't be run on closed pull requests", "")
 }
 
+func TestRunCommentCommand_MatchedBranch(t *testing.T) {
+	t.Log("if a command is run on a pull request which matches base branches run plan successfully")
+	vcsClient := setup(t)
+
+	ch.GlobalCfg.Repos = append(ch.GlobalCfg.Repos, valid.Repo{
+		IDRegex:     regexp.MustCompile(".*"),
+		BranchRegex: regexp.MustCompile("^main$"),
+	})
+	var pull github.PullRequest
+	modelPull := models.PullRequest{BaseRepo: fixtures.GithubRepo, BaseBranch: "main"}
+	When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenReturn(&pull, nil)
+	When(eventParsing.ParseGithubPull(&pull)).ThenReturn(modelPull, modelPull.BaseRepo, fixtures.GithubRepo, nil)
+
+	ch.RunCommentCommand(fixtures.GithubRepo, nil, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.PlanCommand})
+	vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, modelPull.Num, "Ran Plan for 0 projects:\n\n\n\n", "plan")
+}
+
+func TestRunCommentCommand_UnmatchedBranch(t *testing.T) {
+	t.Log("if a command is run on a pull request which doesn't match base branches do not comment with error")
+	vcsClient := setup(t)
+
+	ch.GlobalCfg.Repos = append(ch.GlobalCfg.Repos, valid.Repo{
+		IDRegex:     regexp.MustCompile(".*"),
+		BranchRegex: regexp.MustCompile("^main$"),
+	})
+	var pull github.PullRequest
+	modelPull := models.PullRequest{BaseRepo: fixtures.GithubRepo, BaseBranch: "foo"}
+	When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenReturn(&pull, nil)
+	When(eventParsing.ParseGithubPull(&pull)).ThenReturn(modelPull, modelPull.BaseRepo, fixtures.GithubRepo, nil)
+
+	ch.RunCommentCommand(fixtures.GithubRepo, nil, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.PlanCommand})
+	vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString())
+}
+
 func TestRunUnlockCommand_VCSComment(t *testing.T) {
 	t.Log("if unlock PR command is run, atlantis should" +
 		" invoke the delete command and comment on PR accordingly")
diff --git a/server/events/yaml/valid/global_cfg_test.go b/server/events/yaml/valid/global_cfg_test.go
index 475cf7dfa4..f317a81015 100644
--- a/server/events/yaml/valid/global_cfg_test.go
+++ b/server/events/yaml/valid/global_cfg_test.go
@@ -892,6 +892,69 @@ func TestRepo_BranchMatches(t *testing.T) {
 	Equals(t, false, (valid.Repo{BranchRegex: regexp.MustCompile("release")}).BranchMatches("main"))
 }
 
+func TestGlobalCfg_MatchingRepo(t *testing.T) {
+	defaultRepo := valid.Repo{
+		IDRegex:           regexp.MustCompile(".*"),
+		BranchRegex:       regexp.MustCompile(".*"),
+		ApplyRequirements: []string{},
+	}
+	repo1 := valid.Repo{
+		IDRegex:           regexp.MustCompile(".*"),
+		BranchRegex:       regexp.MustCompile("^main$"),
+		ApplyRequirements: []string{"approved"},
+	}
+	repo2 := valid.Repo{
+		ID:                "github.com/owner/repo",
+		BranchRegex:       regexp.MustCompile("^master$"),
+		ApplyRequirements: []string{"approved", "mergeable"},
+	}
+
+	cases := map[string]struct {
+		gCfg   valid.GlobalCfg
+		repoID string
+		exp    *valid.Repo
+	}{
+		"matches to default": {
+			gCfg: valid.GlobalCfg{
+				Repos: []valid.Repo{
+					defaultRepo,
+					repo2,
+				},
+			},
+			repoID: "foo",
+			exp:    &defaultRepo,
+		},
+		"matches to IDRegex": {
+			gCfg: valid.GlobalCfg{
+				Repos: []valid.Repo{
+					defaultRepo,
+					repo1,
+					repo2,
+				},
+			},
+			repoID: "foo",
+			exp:    &repo1,
+		},
+		"matches to ID": {
+			gCfg: valid.GlobalCfg{
+				Repos: []valid.Repo{
+					defaultRepo,
+					repo1,
+					repo2,
+				},
+			},
+			repoID: "github.com/owner/repo",
+			exp:    &repo2,
+		},
+	}
+
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			Equals(t, c.exp, c.gCfg.MatchingRepo(c.repoID))
+		})
+	}
+}
+
 // String is a helper routine that allocates a new string value
 // to store v and returns a pointer to it.
 func String(v string) *string { return &v }