fix(server): three env knobs silently fall back on an unparseable value #236

Summary

DEFAULT_TERM and DEFAULT_COVERAGE are read through helpers that log one warning and return the built-in default when the value does not parse, and both feed the purchase manager as system-wide money defaults. A typo such as DEFAULT_TERM=3y therefore commits every subsequent purchase at the built-in 3-year default rather than the intended value. ANALYTICS_COLLECTION_ENABLED is worse: it discards the parse error with no log at all, so setting it to no or off leaves the collector enabled. The two sibling knobs on the same struct are validated and fail startup, and the analytics integer knobs deliberately use a fail-fast sentinel, so these three are the outliers.

Location

• internal/server/app.go:947 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
• internal/server/analytics_collect.go:96 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (finding A06-017)

Failure scenario

LoadApplicationConfig reads DEFAULT_TERM and DEFAULT_COVERAGE through these helpers and both feed the purchase manager as system-wide money defaults (purchase.ManagerConfig.DefaultTerm / DefaultCoverage, app.go:441-443). A typo such as DEFAULT_TERM=3y logs one warning line and silently commits every subsequent purchase at 3 years instead of the intended value. The two sibling knobs on the same struct are validated and fail startup (validateAppConfigEnvDefaults at app.go:288, added for issue LeanerCloud/cloud-commitments-cli#1026), and the analytics knobs deliberately use a non-defaulting parser for the same reason (loadAnalyticsInt, analytics_collect.go:60), so the term/coverage pair are the outliers.

Also at internal/server/analytics_collect.go:96 (finding A06-017): strconv.ParseBool rejects no, off, False , disabled. An operator who sets ANALYTICS_COLLECTION_ENABLED=no to stop the collector (for example while a partition problem is being fixed) gets the default true with no warning at all, and the collector keeps writing. The immediately preceding helper in the same file, loadAnalyticsInt, deliberately returns an out-of-range sentinel so Validate fails startup rather than "running with a default the operator never asked for" - this function contradicts that rationale.

Evidence

		result, err := strconv.Atoi(val)
		if err != nil {
			log.Printf("WARNING: %s=%q is not a valid integer; using default %d", key, val, defaultVal)
			return defaultVal
		}

From internal/server/analytics_collect.go:96:

func getEnvBool(key string, defaultVal bool) bool {
	if val := os.Getenv(key); val != "" {
		if result, err := strconv.ParseBool(val); err == nil {
			return result
		}
	}
	return defaultVal
}

Suggested fix

Extend validateAppConfigEnvDefaults to reject a set-but-unparseable DEFAULT_TERM / DEFAULT_COVERAGE at startup, using the loadAnalyticsInt sentinel pattern.

At internal/server/analytics_collect.go:96: make a set-but-unparseable value a startup error via AnalyticsConfig.Validate, or at minimum log a warning as the sibling env parsers do.

Found by the 2026-09-02 codebase audit, finding A06-016, reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.