Refactor duplicate environment variable getters into reusable envutil package

[Copilot]

Four getDefault*() functions in internal/cmd duplicated the same environment variable getter pattern with minor variations (string, int, bool).

Changes

• Created internal/envutil package with generic environment variable utilities:

  • GetEnvString(key, default) - string values
  • GetEnvInt(key, default) - integer values with positive validation
  • GetEnvBool(key, default) - boolean values (supports: 1/true/yes/on, 0/false/no/off)

• Refactored flag getters to use new utilities:

  • flags_logging.go: 3 getters (log dir, payload dir, payload size threshold)
  • flags_difc.go: 1 getter (DIFC enable flag)

• Added comprehensive test coverage (46 test cases covering edge cases and validation)

Before/After

// Before: 11 lines per getter
func getDefaultPayloadSizeThreshold() int {
    if envThreshold := os.Getenv("MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD"); envThreshold != "" {
        var threshold int
        if _, err := fmt.Sscanf(envThreshold, "%d", &threshold); err == nil && threshold > 0 {
            return threshold
        }
    }
    return defaultPayloadSizeThreshold
}

// After: 1 line
func getDefaultPayloadSizeThreshold() int {
    return envutil.GetEnvInt("MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD", defaultPayloadSizeThreshold)
}

Impact

• Reduced getter code by 75% (~49 lines → ~12 lines)
• Centralized validation logic (positive integers, case-insensitive booleans)
• Future flags can use these utilities without reimplementing patterns

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• example.com
  • Triggering command: /tmp/go-build542317655/b279/launcher.test /tmp/go-build542317655/b279/launcher.test -test.testlogfile=/tmp/go-build542317655/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/cgo (dns block)
• invalid-host-that-does-not-exist-12345.com
  • Triggering command: /tmp/go-build542317655/b264/config.test /tmp/go-build542317655/b264/config.test -test.testlogfile=/tmp/go-build542317655/b264/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/cgo 64/src/encoding/pem/pem.go x_amd64/vet (dns block)
• nonexistent.local
  • Triggering command: /tmp/go-build542317655/b279/launcher.test /tmp/go-build542317655/b279/launcher.test -test.testlogfile=/tmp/go-build542317655/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/cgo (dns block)
• slow.example.com
  • Triggering command: /tmp/go-build542317655/b279/launcher.test /tmp/go-build542317655/b279/launcher.test -test.testlogfile=/tmp/go-build542317655/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/cgo (dns block)
• this-host-does-not-exist-12345.com
  • Triggering command: /tmp/go-build542317655/b288/mcp.test /tmp/go-build542317655/b288/mcp.test -test.testlogfile=/tmp/go-build542317655/b288/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true ache/go/1.25.6/x64/src/runtime/c-errorsas .cfg tnet/tools/as (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[duplicate-code] Duplicate Code Pattern: Flag Environment Variable Getters</issue_title>
<issue_description># 🔍 Duplicate Code Pattern: Flag Environment Variable Getters

Part of duplicate code analysis: #797

Summary

Multiple getDefault*() functions in the internal/cmd package follow an identical pattern for retrieving configuration values from environment variables with fallback to hardcoded defaults. This pattern is repeated 4 times across flag files with only minor variations.

Duplication Details

Pattern: Environment Variable Getter Functions

• Severity: Medium
• Occurrences: 4 instances (3 in flags_logging.go, 1 in flags_difc.go)
• Locations:
  • internal/cmd/flags_logging.go:
    • getDefaultLogDir() (lines 36-41)
    • getDefaultPayloadDir() (lines 45-50)
    • getDefaultPayloadSizeThreshold() (lines 54-64)
  • internal/cmd/flags_difc.go:
    • getDefaultEnableDIFC() (lines 30-38)

Duplicated Structure

All functions follow the same pattern:

// Pattern 1: String environment variable
func getDefault(Name)() string {
    if env(Name) := os.Getenv("MCP_GATEWAY_(NAME)"); env(Name) != "" {
        return env(Name)
    }
    return default(Name)
}

// Pattern 2: Boolean environment variable
func getDefault(Name)() bool {
    if env(Name) := os.Getenv("MCP_GATEWAY_(NAME)"); env(Name) != "" {
        switch strings.ToLower(env(Name)) {
        case "1", "true", "yes", "on":
            return true
        }
    }
    return default(Name)
}

// Pattern 3: Integer environment variable with validation
func getDefault(Name)() int {
    if env(Name) := os.Getenv("MCP_GATEWAY_(NAME)"); env(Name) != "" {
        var value int
        if _, err := fmt.Sscanf(env(Name), "%d", &value); err == nil && value > 0 {
            return value
        }
    }
    return default(Name)
}

Code Examples

String Getter (getDefaultLogDir):

func getDefaultLogDir() string {
    if envLogDir := os.Getenv("MCP_GATEWAY_LOG_DIR"); envLogDir != "" {
        return envLogDir
    }
    return defaultLogDir
}

String Getter (getDefaultPayloadDir) - Nearly identical:

func getDefaultPayloadDir() string {
    if envPayloadDir := os.Getenv("MCP_GATEWAY_PAYLOAD_DIR"); envPayloadDir != "" {
        return envPayloadDir
    }
    return defaultPayloadDir
}

Integer Getter (getDefaultPayloadSizeThreshold):

func getDefaultPayloadSizeThreshold() int {
    if envThreshold := os.Getenv("MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD"); envThreshold != "" {
        var threshold int
        if _, err := fmt.Sscanf(envThreshold, "%d", &threshold); err == nil && threshold > 0 {
            return threshold
        }
    }
    return defaultPayloadSizeThreshold
}

Boolean Getter (getDefaultEnableDIFC):

func getDefaultEnableDIFC() bool {
    if envDIFC := os.Getenv("MCP_GATEWAY_ENABLE_DIFC"); envDIFC != "" {
        switch strings.ToLower(envDIFC) {
        case "1", "true", "yes", "on":
            return true
        }
    }
    return defaultEnableDIFC
}

Impact Analysis

Maintainability

• Medium Risk: New flags require copy-paste of getter pattern
• Boilerplate Heavy: Each new configuration value needs 6-11 lines of getter code
• Validation Inconsistency: Integer validation inline, boolean validation via switch, no validation for strings

Bug Risk

• Low-Medium Risk: Inconsistent validation approaches could lead to bugs
• Example: Integer validation checks > 0, but what about negative values elsewhere?
• Missing Features: No support for default value overrides, validation error messages

Code Bloat

• Total Lines: ~40 lines of similar getter functions
• Future Growth: Every new flag adds 6-11 lines of duplicated code

Refactoring Recommendations

1. Create Generic Environment Getter Utility

Effort: Low-Medium (2-4 hours)

Extract common pattern into type-safe generic utility:

// Package envutil provides utilities for reading configuration from environment variables
package envutil

import (
    "fmt"
    "os"
    "strconv"
    "strings"
)

// GetEnvString returns string value from env var or default
func GetEnvString(envKey, defaultValue string) string {
    if value := os.Getenv(envKey); value != "" {
        return value
    }
    return defaultValue
}

// GetEnvInt returns integer value from env var or default
// Validates that value is positive (> 0)
func GetEnvInt(envKey string, defaultValue int) int {
    if envValue := os.Getenv(envKey); envValue != "" {
        if value, err := strconv.Atoi(envValue); err == nil && value > 0 {
            return value
        }
    }
    return defaultValue
}

// GetEnvBool returns boolean value from env var or default
// Accepts: "1", "true", "yes", "on" (case-insensitive)
func GetEnvBool(envKey string, defaultValue bool) bool {
    if envV...

</details>


> **Custom agent used: agentic-workflows**
> GitHub Agentic Workflows (gh-aw) - Create, debug, and upgrade AI-powered workflows with intelligent prompt routing



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes github/gh-aw-mcpg#799

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs.

[Copilot]

Pull request overview

Refactors duplicated “env var + default fallback” logic used by CLI flag default getters into a shared internal/envutil package, so environment parsing/validation is centralized and easier to reuse across internal/cmd.

Changes:

• Added internal/envutil helpers: GetEnvString, GetEnvInt (positive int), GetEnvBool (truthy/falsy set).
• Updated internal/cmd flag default getters (logging + DIFC) to call envutil instead of duplicating logic.
• Added unit tests for the new envutil helpers.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
internal/envutil/envutil.go	Introduces shared env var getter utilities (string/int/bool).
internal/envutil/envutil_test.go	Adds test coverage for env var parsing and defaults.
internal/cmd/flags_logging.go	Refactors logging-related default getters to use envutil.
internal/cmd/flags_difc.go	Refactors DIFC default getter to use envutil.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

GetEnvInt previously used fmt.Sscanf("%d") in the flag getter, which accepts leading/trailing whitespace (and newlines). Switching to strconv.Atoi changes behavior so values like " 4096 " will now be treated as invalid and fall back to default. To avoid a subtle regression for env var parsing, trim surrounding whitespace before Atoi (and keep the >0 validation).

Suggested change

	if value, err := strconv.Atoi(envValue); err == nil && value > 0 {
	return value
	envValue = strings.TrimSpace(envValue)
	if envValue != "" {
	if value, err := strconv.Atoi(envValue); err == nil && value > 0 {
	return value
	}

[Copilot]

The GetEnvInt tests don't cover whitespace-padded values (e.g. " 2048 ", "2048\n"), which were accepted by the previous implementation using fmt.Sscanf("%d"). Add a couple of cases to lock in the intended behavior (either accept via TrimSpace or explicitly document/reject) so future refactors don't reintroduce parsing regressions.