π Duplicate Code Pattern: Guard Policy Env Var Reading β Mixed os.Getenv vs envutil
Part of duplicate code analysis: #5517
Summary
The same five guard-policy environment variables are read in two separate locations, and within guard_policy_parse.go the reads mix os.Getenv directly with envutil.GetEnvBool, which is inconsistent with the rest of the codebase.
Duplication Details
Pattern: Same env vars read in both flags_difc.go and guard_policy_parse.go
- Severity: Low
- Occurrences: 5 env vars Γ 2 read sites = 10 duplicated reads, plus inconsistent helper usage
- Locations:
internal/cmd/flags_difc.go (lines 31β35)
internal/config/guard_policy_parse.go (lines 227β245)
internal/cmd/flags_difc.go (lines 31β35) β uses envutil.GetEnvString/Bool:
cmd.Flags().StringVar(&guardPolicyJSON, "guard-policy-json",
envutil.GetEnvString("MCP_GATEWAY_GUARD_POLICY_JSON", ""), ...)
cmd.Flags().BoolVar(&allowOnlyPublic, "allowonly-scope-public",
envutil.GetEnvBool("MCP_GATEWAY_ALLOWONLY_SCOPE_PUBLIC", false), ...)
cmd.Flags().StringVar(&allowOnlyOwner, "allowonly-scope-owner",
envutil.GetEnvString("MCP_GATEWAY_ALLOWONLY_SCOPE_OWNER", ""), ...)
cmd.Flags().StringVar(&allowOnlyRepo, "allowonly-scope-repo",
envutil.GetEnvString("MCP_GATEWAY_ALLOWONLY_SCOPE_REPO", ""), ...)
cmd.Flags().StringVar(&allowOnlyMinInt, "allowonly-min-integrity",
envutil.GetEnvString("MCP_GATEWAY_ALLOWONLY_MIN_INTEGRITY", ""), ...)
internal/config/guard_policy_parse.go (lines 227β245) β mixes os.Getenv and envutil.GetEnvBool:
if envPolicyJSON := strings.TrimSpace(os.Getenv("MCP_GATEWAY_GUARD_POLICY_JSON")); ...
_, hasScopePublic := os.LookupEnv("MCP_GATEWAY_ALLOWONLY_SCOPE_PUBLIC")
// ...
policy, err := BuildAllowOnlyPolicy(
envutil.GetEnvBool("MCP_GATEWAY_ALLOWONLY_SCOPE_PUBLIC", false), // uses envutil
os.Getenv("MCP_GATEWAY_ALLOWONLY_SCOPE_OWNER"), // raw os.Getenv
os.Getenv("MCP_GATEWAY_ALLOWONLY_SCOPE_REPO"), // raw os.Getenv
os.Getenv("MCP_GATEWAY_ALLOWONLY_MIN_INTEGRITY"), // raw os.Getenv
)
Impact Analysis
- Maintainability: If a new AllowOnly env var is added, it must be added in both files. It's easy to forget one site.
- Bug Risk: Inconsistent
os.Getenv vs envutil.GetEnvString makes it harder to mock env vars in tests.
- Code Bloat: Minor (~10 lines), but the inconsistency is a subtle code smell.
Refactoring Recommendations
-
Replace raw os.Getenv calls in guard_policy_parse.go with envutil.GetEnvString to be consistent with the rest of the codebase. This is a one-line change per occurrence (lines 227, 243, 244, 245).
-
Consolidate env-var constants β define the five env-var name strings as package-level constants in internal/config (or internal/envutil) so both flags_difc.go and guard_policy_parse.go reference the same constant rather than repeating the string literal.
// internal/config/guard_policy_env.go (new file, or added to existing)
const (
EnvGuardPolicyJSON = "MCP_GATEWAY_GUARD_POLICY_JSON"
EnvAllowOnlyScopePublic = "MCP_GATEWAY_ALLOWONLY_SCOPE_PUBLIC"
EnvAllowOnlyScopeOwner = "MCP_GATEWAY_ALLOWONLY_SCOPE_OWNER"
EnvAllowOnlyScopeRepo = "MCP_GATEWAY_ALLOWONLY_SCOPE_REPO"
EnvAllowOnlyMinIntegrity = "MCP_GATEWAY_ALLOWONLY_MIN_INTEGRITY"
)
Implementation Checklist
Parent Issue
See parent analysis report: #5517
Related to #5517
Generated by Duplicate Code Detector Β· β 1.2M Β· β·
π Duplicate Code Pattern: Guard Policy Env Var Reading β Mixed
os.GetenvvsenvutilPart of duplicate code analysis: #5517
Summary
The same five guard-policy environment variables are read in two separate locations, and within
guard_policy_parse.gothe reads mixos.Getenvdirectly withenvutil.GetEnvBool, which is inconsistent with the rest of the codebase.Duplication Details
Pattern: Same env vars read in both
flags_difc.goandguard_policy_parse.gointernal/cmd/flags_difc.go(lines 31β35)internal/config/guard_policy_parse.go(lines 227β245)internal/cmd/flags_difc.go(lines 31β35) β usesenvutil.GetEnvString/Bool:internal/config/guard_policy_parse.go(lines 227β245) β mixesos.Getenvandenvutil.GetEnvBool:Impact Analysis
os.Getenvvsenvutil.GetEnvStringmakes it harder to mock env vars in tests.Refactoring Recommendations
Replace raw
os.Getenvcalls inguard_policy_parse.gowithenvutil.GetEnvStringto be consistent with the rest of the codebase. This is a one-line change per occurrence (lines 227, 243, 244, 245).Consolidate env-var constants β define the five env-var name strings as package-level constants in
internal/config(orinternal/envutil) so bothflags_difc.goandguard_policy_parse.goreference the same constant rather than repeating the string literal.Implementation Checklist
os.Getenvcalls inguard_policy_parse.go(lines 227, 243, 244, 245) withenvutil.GetEnvStringflags_difc.goandguard_policy_parse.goto reference the new constantsmake testto confirm no regressionsParent Issue
See parent analysis report: #5517
Related to #5517