chore(purchase/server/analytics): low correctness findings from the 2026-09-02 audit [platform part] #339

Split from LeanerCloud/cloud-commitments-cli#2050 (cross-component issue, monorepo split)

Summary

Three low-severity correctness defects across internal/purchase, internal/server and internal/analytics, each confirmed against the audited commit: an execution with nothing selected still emails a purchase confirmation and lands "completed", the config_store health sub-check reports healthy over a closed pool, and every non-SavingsPlans commitment is labelled "RI" including GCP CUDs. One of these was downgraded from medium by the verifier; the reason is noted inline.

Each item is independently actionable; they are grouped so one sitting can clear them.

Findings

• An execution with nothing selected still sends a purchase-confirmation email and lands "completed" (A05-015)

  • Location: internal/purchase/execution.go:117 (sendPurchaseNotification call in executeSingleAccount); short-circuit at :562-566; buildPurchaseConfirmationData at :938-964 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • Failure scenario: When no recommendation is selected, processPurchaseRecommendations returns (0, 0, nil, nil), the len(purchaseErrors) > 0 guard is false, and sendPurchaseNotification fires unconditionally; finalizeExecution stamps "completed". The user receives a "purchase confirmed" email listing zero commitments and $0 savings for a run that never touched a cloud API, and History shows a completed purchase with no history rows. Reachable via the cron path: notification-created execution rows carry an empty Source, which executableByScheduler passes, and no recommendations at all.
  • Evidence:

    	// At least one rec committed (full or partial success): the confirmation
    	// must go out for the recs that purchased.
    	if notifyErr := m.sendPurchaseNotification(ctx, exec, plan, totalSavings, totalUpfront); notifyErr != nil {
    		logging.Errorf("Failed to send confirmation: %v", notifyErr)
    	}

  • Suggested fix: Gate the confirmation on anyRecPurchased(exec.Recommendations), which the function already computes for the failure branch.

• /health reports config_store healthy after a failed post-connect reinitialization (A06-009)

  • Location: internal/server/health.go:102-130 (checkConfigStore); app.Config = pgStore at internal/server/app.go:718, pool closed and app.DB = nil at :633-637 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • Note: Verifier downgraded from medium to low: aggregate status and the probe response are unaffected.
  • Failure scenario: reinitializeAfterConnect sets app.Config and can then fail (encryption-key load, analytics collector, LoadDefaultConfig). ensureDB closes the pool and nils app.DB but leaves app.Config pointing at a store over the closed pool. checkConfigStore then skips both branches and returns healthy. Scope correction from the verifier: checkAuthStore still fails in every one of those cases so the aggregate stays degraded, and /health returns HTTP 200 unconditionally, so no probe verdict changes; the mis-report is confined to the config_store sub-check.
  • Evidence:

    	if app.DB != nil {
    		if err := app.DB.HealthCheck(ctx); err != nil {
    			return CheckResult{Status: "unhealthy", ...}
    		}
    	}
    	return CheckResult{Status: "healthy"}

  • Suggested fix: Report on app.dbConnected (and the last ensureDB error) rather than on app.Config != nil, or clear app.Config on the reinit-failure path.

• Every non-SavingsPlans commitment is labelled "RI", including GCP committed-use discounts (A06-019)

  • Location: internal/analytics/collector.go:196-202 (commitmentTypeFor), provider available in the same loop at :160-172; CHECK enforced by validateCommitmentType at internal/analytics/postgres_analytics.go:146-151; typed values at pkg/common/types.go:176-183 (LeanerCloud/cloud-commitments-go) at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • Failure scenario: A GCP CUD purchase (Provider: "gcp") is bucketed and stored with commitment_type = "RI", so the Trends dashboard's commitment-type dimension reports GCP committed-use spend as reserved instances and any per-type filter mixes them. The four typed values in pkg/common use a different vocabulary that nothing on this path references, and the savings_snapshots CHECK admits only the two literals this function can emit, so the schema and the mapper are wrong together.
  • Evidence:

    func commitmentTypeFor(service string) string {
    	if service == "SavingsPlans" {
    		return "SavingsPlan"
    	}
    	return "RI"
    }

  • Suggested fix: Derive the type from (provider, service) using the pkg/common constants (module github.com/LeanerCloud/cloud-commitments-go) and widen the CHECK constraint to match.

Verification requirement

(per the sweep) each fix needs a regression test on the real failing path, not an isolated helper.

---

Found by the 2026-09-02 codebase audit, findings A05-015, A06-009, A06-019, reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.

Cross-references

• pkg/common/types.go referenced above lives in LeanerCloud/cloud-commitments-go, consumed here as a module dependency; no change needed there for A06-019, only in this repo's internal/analytics.
• The go-repo half of this same finding set (A06-022, pkg/reporter/reporter.go map-order summary) is filed separately as chore(reporter): filtered-reason summary emitted in Go map order (A06-022) [go part] cloud-commitments-go#110, since that file moved out of this repo (internal/reporter -> pkg/reporter) in the split.
• The cli-repo half (A10-010, cmd/multi_service_csv.go --output truncation) stays in chore(cli): --output silently truncates an existing CSV (A10-010) [cli part] cloud-commitments-cli#2050, since cmd/ is not part of this repo after the split.

Acceptance criteria

• internal/purchase/execution.go gates sendPurchaseNotification on anyRecPurchased(exec.Recommendations).
• internal/server/health.go checkConfigStore reports on app.dbConnected (or clears app.Config on reinit failure) instead of app.Config != nil.
• internal/analytics/collector.go commitmentTypeFor derives the commitment type from (provider, service) using pkg/common constants; internal/analytics/postgres_analytics.go CHECK constraint widened to match.
• Each fix ships a regression test exercising the real failing path (empty-selection execution, reinit-failure health check, GCP CUD collection).