From a068005e0786b425ba0505486cfeefd4d02b9583 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]"
 <41898282+github-actions[bot]@users.noreply.github.com>
Date: Fri, 15 May 2026 13:23:33 +0000
Subject: [PATCH 1/2] refactor(config): add envutil.HasEnvVar and
 hasMapKeyVariants helpers

- Add HasEnvVar(key string) bool to internal/envutil to wrap os.LookupEnv
  for presence-only checks, consistent with the envutil abstraction layer
- Replace 4x os.LookupEnv presence checks in guard_policy_parse.go with
  envutil.HasEnvVar calls, removing the direct os import from that file
- Extract hasMapKeyVariants helper to collapse two identical 5-line
  dual-form key check blocks (allow-only/allowonly, write-sink/writesink)
  into single 1-line calls

Closes #5733

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 internal/config/guard_policy_parse.go | 34 +++++++++++++--------------
 internal/envutil/envutil.go           |  7 ++++++
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/internal/config/guard_policy_parse.go b/internal/config/guard_policy_parse.go
index 9ba663698..30d3a4fc0 100644
--- a/internal/config/guard_policy_parse.go
+++ b/internal/config/guard_policy_parse.go
@@ -3,7 +3,6 @@ package config
 import (
 	"encoding/json"
 	"fmt"
-	"os"
 	"strings"
 
 	"github.com/github/gh-aw-mcpg/internal/envutil"
@@ -70,19 +69,8 @@ func ParsePolicyMap(raw map[string]interface{}) (*GuardPolicy, error) {
 		return nil, nil
 	}
 
-	hasAllowOnly := false
-	if _, ok := raw["allow-only"]; ok {
-		hasAllowOnly = true
-	} else if _, ok := raw["allowonly"]; ok { // Accept legacy "allowonly" form for backward compatibility
-		hasAllowOnly = true
-	}
-
-	hasWriteSink := false
-	if _, ok := raw["write-sink"]; ok {
-		hasWriteSink = true
-	} else if _, ok := raw["writesink"]; ok {
-		hasWriteSink = true
-	}
+	hasAllowOnly := hasMapKeyVariants(raw, "allow-only", "allowonly") // Accept legacy "allowonly" form for backward compatibility
+	hasWriteSink := hasMapKeyVariants(raw, "write-sink", "writesink")
 
 	logGuardPolicy.Printf("ParsePolicyMap: hasAllowOnly=%v, hasWriteSink=%v, keyCount=%d", hasAllowOnly, hasWriteSink, len(raw))
 
@@ -249,10 +237,10 @@ func ResolveGuardPolicyOverride(
 		return policy, "env", nil
 	}
 
-	_, hasScopePublic := os.LookupEnv(EnvAllowOnlyScopePublic)
-	_, hasScopeOwner := os.LookupEnv(EnvAllowOnlyScopeOwner)
-	_, hasScopeRepo := os.LookupEnv(EnvAllowOnlyScopeRepo)
-	_, hasMinIntegrity := os.LookupEnv(EnvAllowOnlyMinIntegrity)
+	hasScopePublic := envutil.HasEnvVar(EnvAllowOnlyScopePublic)
+	hasScopeOwner := envutil.HasEnvVar(EnvAllowOnlyScopeOwner)
+	hasScopeRepo := envutil.HasEnvVar(EnvAllowOnlyScopeRepo)
+	hasMinIntegrity := envutil.HasEnvVar(EnvAllowOnlyMinIntegrity)
 
 	if hasScopePublic || hasScopeOwner || hasScopeRepo || hasMinIntegrity {
 		logGuardPolicy.Printf("ResolveGuardPolicyOverride: using env vars for AllowOnly scope: hasScopePublic=%v, hasScopeOwner=%v, hasScopeRepo=%v, hasMinIntegrity=%v",
@@ -291,3 +279,13 @@ func NormalizeScopeKind(policy map[string]interface{}) map[string]interface{} {
 
 	return normalized
 }
+
+// hasMapKeyVariants reports whether any of the given key variants is present in m.
+func hasMapKeyVariants(m map[string]interface{}, keys ...string) bool {
+	for _, k := range keys {
+		if _, ok := m[k]; ok {
+			return true
+		}
+	}
+	return false
+}
diff --git a/internal/envutil/envutil.go b/internal/envutil/envutil.go
index 609ba67a7..9a85dedd1 100644
--- a/internal/envutil/envutil.go
+++ b/internal/envutil/envutil.go
@@ -12,6 +12,13 @@ import (
 
 var logEnvUtil = logger.New("envutil:envutil")
 
+// HasEnvVar reports whether the named environment variable is present in the
+// process environment, regardless of whether its value is empty.
+func HasEnvVar(key string) bool {
+	_, ok := os.LookupEnv(key)
+	return ok
+}
+
 // GetEnvString returns the value of the environment variable specified by envKey.
 // If the environment variable is not set or is empty, it returns the defaultValue.
 func GetEnvString(envKey, defaultValue string) string {

From 126dfd252a4773b6fa1ade50f2626d453fa3f076 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Fri, 15 May 2026 13:33:58 +0000
Subject: [PATCH 2/2] test(envutil): cover HasEnvVar unset and empty cases

---
 internal/envutil/envutil_test.go | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/internal/envutil/envutil_test.go b/internal/envutil/envutil_test.go
index 9caac5039..4131eb529 100644
--- a/internal/envutil/envutil_test.go
+++ b/internal/envutil/envutil_test.go
@@ -8,6 +8,18 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestHasEnvVar(t *testing.T) {
+	t.Run("unset returns false", func(t *testing.T) {
+		os.Unsetenv("TEST_HAS_ENV_VAR")
+		assert.False(t, HasEnvVar("TEST_HAS_ENV_VAR"))
+	})
+
+	t.Run("set to empty string returns true", func(t *testing.T) {
+		t.Setenv("TEST_HAS_ENV_VAR", "")
+		assert.True(t, HasEnvVar("TEST_HAS_ENV_VAR"))
+	})
+}
+
 func TestGetEnvDuration(t *testing.T) {
 	tests := []struct {
 		name         string