From 9daecad9d5cae6d9b35e7063c4abdae8ab122bb6 Mon Sep 17 00:00:00 2001
From: Daniel Martins <29534+danielfm@users.noreply.github.com>
Date: Mon, 11 Dec 2023 23:48:23 -0300
Subject: [PATCH] fix: Enforce policy checks for overridden apply reqs (#3960)

* Enforce policy checks for overriden apply reqs

* Another take on the fix

This new version only includes the 'policies_passed' req back if
policy checks are enabled for the project.

* Fix test

* Fix comment

* Fix spelling

---------

Co-authored-by: PePe Amengual <jose.amengual@gmail.com>
---
 server/core/config/valid/global_cfg.go        |  5 ++
 server/core/config/valid/global_cfg_test.go   | 63 +++++++++++++++++++
 .../project_command_builder_internal_test.go  |  2 +-
 3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/server/core/config/valid/global_cfg.go b/server/core/config/valid/global_cfg.go
index baa4de7575..d2dabd2f89 100644
--- a/server/core/config/valid/global_cfg.go
+++ b/server/core/config/valid/global_cfg.go
@@ -321,6 +321,11 @@ func (g GlobalCfg) MergeProjectCfg(log logging.SimpleLogging, repoID string, pro
 			if proj.ApplyRequirements != nil {
 				log.Debug("overriding server-defined %s with repo settings: [%s]", ApplyRequirementsKey, strings.Join(proj.ApplyRequirements, ","))
 				applyReqs = proj.ApplyRequirements
+
+				// Preserve policies_passed req if policy check is enabled
+				if policyCheck {
+					applyReqs = append(applyReqs, PoliciesPassedCommandReq)
+				}
 			}
 		case ImportRequirementsKey:
 			if proj.ImportRequirements != nil {
diff --git a/server/core/config/valid/global_cfg_test.go b/server/core/config/valid/global_cfg_test.go
index 5c51893470..e0c7d1ec6b 100644
--- a/server/core/config/valid/global_cfg_test.go
+++ b/server/core/config/valid/global_cfg_test.go
@@ -922,6 +922,69 @@ repos:
 				CustomPolicyCheck:  false,
 			},
 		},
+		"repo-side apply reqs should include non-overrideable 'policies_passed' req when overridden and policies enabled": {
+			gCfg: `
+repos:
+- id: /.*/
+  allowed_overrides: [apply_requirements]
+  apply_requirements: [approved]
+  policy_check: true
+`,
+			repoID: "github.com/owner/repo",
+			proj: valid.Project{
+				Dir:                ".",
+				Workspace:          "default",
+				PlanRequirements:   []string{},
+				ApplyRequirements:  []string{"mergeable"},
+				ImportRequirements: []string{},
+			},
+			repoWorkflows: nil,
+			exp: valid.MergedProjectCfg{
+				PlanRequirements:   []string{},
+				ApplyRequirements:  []string{"mergeable", "policies_passed"},
+				ImportRequirements: []string{},
+				Workflow:           defaultWorkflow,
+				RepoRelDir:         ".",
+				Workspace:          "default",
+				Name:               "",
+				AutoplanEnabled:    false,
+				PolicySets:         emptyPolicySets,
+				RepoLocking:        true,
+				CustomPolicyCheck:  false,
+				PolicyCheck:        true,
+			},
+		},
+		"repo-side apply reqs should not include non-overrideable 'policies_passed' req when overridden and policies disabled": {
+			gCfg: `
+repos:
+- id: /.*/
+  allowed_overrides: [apply_requirements]
+  apply_requirements: [approved]
+`,
+			repoID: "github.com/owner/repo",
+			proj: valid.Project{
+				Dir:                ".",
+				Workspace:          "default",
+				PlanRequirements:   []string{},
+				ApplyRequirements:  []string{"mergeable"},
+				ImportRequirements: []string{},
+			},
+			repoWorkflows: nil,
+			exp: valid.MergedProjectCfg{
+				PlanRequirements:   []string{},
+				ApplyRequirements:  []string{"mergeable"},
+				ImportRequirements: []string{},
+				Workflow:           defaultWorkflow,
+				RepoRelDir:         ".",
+				Workspace:          "default",
+				Name:               "",
+				AutoplanEnabled:    false,
+				PolicySets:         emptyPolicySets,
+				RepoLocking:        true,
+				CustomPolicyCheck:  false,
+				PolicyCheck:        false,
+			},
+		},
 		"repo-side import reqs win out if allowed": {
 			gCfg: `
 repos:
diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go
index bd524dea90..0b5f6c62eb 100644
--- a/server/events/project_command_builder_internal_test.go
+++ b/server/events/project_command_builder_internal_test.go
@@ -1062,7 +1062,7 @@ workflows:
 				Pull:               pull,
 				ProjectName:        "",
 				PlanRequirements:   []string{"policies_passed"},
-				ApplyRequirements:  []string{},
+				ApplyRequirements:  []string{"policies_passed"},
 				ImportRequirements: []string{"policies_passed"},
 				RepoConfigVersion:  3,
 				RePlanCmd:          "atlantis plan -d project1 -w myworkspace -- flag",