Skip to content

pkg/envutil: GetIntFromEnv bypasses structured logger for warning messages #29185

@github-actions

Description

@github-actions

Problem

GetIntFromEnv in pkg/envutil/envutil.go accepts a *logger.Logger parameter for success logging but uses fmt.Fprintln(os.Stderr, ...) for warning/error messages (invalid value format, out-of-bounds value). This is inconsistent: callers pass a logger for observability, but warnings bypass it entirely.

// pkg/envutil/envutil.go:29
func GetIntFromEnv(envVar string, defaultValue, minValue, maxValue int, log *logger.Logger) int {
    // ...
    if err != nil {
        fmt.Fprintln(os.Stderr, console.FormatWarningMessage(   // ← bypasses logger
            fmt.Sprintf("Invalid %s value '%s' ...", envVar, envValue, defaultValue),
        ))
        return defaultValue
    }

    if val < minValue || val > maxValue {
        fmt.Fprintln(os.Stderr, console.FormatWarningMessage(   // ← bypasses logger
            fmt.Sprintf("%s value %d is out of bounds ...", envVar, val, minValue, maxValue, defaultValue),
        ))
        return defaultValue
    }

    if log != nil {
        log.Printf("Using %s=%d", envVar, val)   // ← uses logger only for success
    }
}

Impact

  • Severity: Medium
  • Affected Files: pkg/envutil/envutil.go (single file, but called across the codebase wherever env vars are parsed with range validation)
  • Risk: Warning messages cannot be suppressed or redirected by callers who control logging; wasted parameter — callers pass log expecting all output to flow through it

Recommendation

Route all messages through the log parameter when it is non-nil, falling back to fmt.Fprintln only when no logger is provided:

After:

func GetIntFromEnv(envVar string, defaultValue, minValue, maxValue int, log *logger.Logger) int {
    warn := func(msg string) {
        if log != nil {
            log.Printf("WARNING: %s", msg)
        } else {
            fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg))
        }
    }
    // ...
    if err != nil {
        warn(fmt.Sprintf("Invalid %s value '%s' (must be a number), using default %d", envVar, envValue, defaultValue))
        return defaultValue
    }
    if val < minValue || val > maxValue {
        warn(fmt.Sprintf("%s value %d is out of bounds (must be %d-%d), using default %d", envVar, val, minValue, maxValue, defaultValue))
        return defaultValue
    }
    if log != nil {
        log.Printf("Using %s=%d", envVar, val)
    }
    return val
}

Validation

  • Run go test ./pkg/envutil/...
  • Verify warning messages still appear in CLI output when no logger provided
  • Confirm logger-based warnings appear in structured logs when logger is provided
  • Check all callers of GetIntFromEnv to confirm behavior is as expected

Estimated Effort: Small (single function, 2 warning sites)

Generated by Sergo - Serena Go Expert · ● 536.8K ·

  • expires on May 6, 2026, 8:44 PM UTC

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions