ARCH-04 followup: EC2 client silent term + tenancy fallbacks not covered by PR #1207 #23

Problem

PR LeanerCloud/cloud-commitments-cli#1207 (closes LeanerCloud/cloud-commitments-cli#1192 / ARCH-04) made the term-string converters in five AWS RI clients (ElastiCache, MemoryDB, OpenSearch, RDS, Redshift) fail loud on unrecognized or empty input, removing a silent 1-year fallback that would have caused mis-purchases on 0/NULL Term DB rows from the scheduler purchase path.

The EC2 client (providers/aws/services/ec2/client.go) was not in scope and still carries the same money-affecting silent-default pattern in two places, even though EC2 RIs are CUDly's largest commitment-cost surface. Both functions sit on the findOfferingID -> buildEC2QueryFromRec -> buildEC2OfferingQuery path, so a malformed term or missing tenancy slipping past the boundary validators (for example the 0/NULL Term DB row scenario from LeanerCloud/cloud-commitments-cli#1192, executed by the scheduler purchase path which does not re-validate) silently purchases the wrong (and most likely the most expensive) commitment.

Bug 1 - silent 1-year fallback in getDurationValue

// providers/aws/services/ec2/client.go:629-635
func (c *Client) getDurationValue(term string) int64 {
    if term == "3yr" || term == "3" {
        return ThreeYearSeconds
    }
    return OneYearSeconds // silent default on "", "0", "2yr", typo, future AWS variant, ...
}

Called at line 440 inside buildEC2QueryFromRec. Identical bug shape to the five clients fixed in LeanerCloud/cloud-commitments-cli#1207.

Bug 2 - silent default tenancy fallback in buildEC2OfferingQuery

// providers/aws/services/ec2/client.go:389-392
tenancy := canonicalizeEC2Tenancy(details.Tenancy)
if tenancy == "" {
    tenancy = string(types.TenancyDefault) // silent default on empty / unknown
}

A missing or unrecognized details.Tenancy silently selects shared (default) tenancy. A dedicated-tenancy purchase intended for a Dedicated-Instances workload would land on the cheaper shared tenancy and not actually cover the running instances - the buyer pays for an RI that covers nothing.

canonicalizeEC2Tenancy itself returns the unrecognized input unchanged (line 312-314), so an unknown tenancy string like "host" (a real third AWS value) falls through and would be sent to DescribeReservedInstancesOfferings as-is, but the boundary case (empty input) silently defaults to default.

Fix

Mirror the PR LeanerCloud/cloud-commitments-cli#1207 pattern in EC2:

• getDurationValue returns (int64, error) and errors on anything outside 1yr, 1, 3yr, 3; buildEC2QueryFromRec propagates the error before the offering query is built.
• canonicalizeEC2Tenancy returns (string, error) and errors on unknown / empty input (or split into a strict parseEC2Tenancy for new code path, keep the legacy canonicalize shim for parser-fixup callers only). buildEC2OfferingQuery propagates the error before the API call. Decide explicitly whether "host" should be accepted; current AWS RI offerings only support default and dedicated, so host should error.

Test evidence required

• Term: regression test in client_test.go that asserts findOfferingID errors before DescribeReservedInstancesOfferings is called for "", "0", "2yr", "invalid". Mirrors the TestFindOfferingID_InvalidTerm_ErrorsBeforeAPICall tests added in PR fix(providers/aws): fail loud on unrecognized RI term strings cloud-commitments-cli#1207.
• Tenancy: regression test that asserts a Recommendation with details.Tenancy == "" errors at the boundary instead of falling through to a default-tenancy purchase.

Why this matters

EC2 RIs are the largest-spend commitment surface; a silent term or tenancy mis-purchase here is a higher-blast-radius bug than the five fixed by LeanerCloud/cloud-commitments-cli#1207. This issue should be priority/p1 once a real-world repro from prod data is available, but lands as p2 to match the parent ARCH-04 priority until then.

Refs: LeanerCloud/cloud-commitments-cli#1192 (ARCH-04 parent), LeanerCloud/cloud-commitments-cli#1207 (the five-client fix this extends).

Findings from the 2026-09-02 codebase audit

Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.

A07-032 (low)

There is a third silent default on this exact path, worth taking in the same pass. describeInputFromQuery (providers/aws/services/ec2/client.go:430-433) does oc := q.offeringClass; if oc == "" { oc = types.OfferingClassTypeConvertible }. It is unreachable today because its only production caller, findOfferingID, sets q.offeringClass from resolveOfferingClassType on the line above (:495-499), and that function returns either a non-empty enum or an error (:385-394) -- the same fail-loud shape this issue asks for on term and tenancy. The risk is the mirror of Bug 1: a future caller that forgets to set the field silently buys convertible instead of erroring, and the duplicated default means resolveOfferingClassType is no longer the single place the default is decided. Deleting the branch is the whole fix. (audit finding A07-032)