From 81fb645448e371df7a2c3483cfbdd54a8c17b484 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 11:12:29 -0700
Subject: [PATCH 1/5] fix(aws/savingsplans): error on unknown
 payment-option/term, no silent default

convertPaymentOption and convertTermToSeconds previously logged a warning
and silently substituted All Upfront (largest cash outlay) or 1 year when
given an unrecognized or empty value, risking a mis-purchase on any typo or
new/renamed AWS enum value.

Both functions now return an explicit error on unrecognized input so the
purchase and offering-lookup paths fail loud instead of buying the wrong
product. Mirrors the pattern already used by the RDS convertPaymentOption.

Regression tests for H1 (unknown/empty payment option) and H2 (unknown/empty
term) are added to TestClient_FindOfferingID_AllPaymentOptions and
TestClient_FindOfferingID_TermVariations; the previously-passing "default term"
case is updated to assert the error path.

Fixes H1 and H2 from docs/code-review/19-hardcoded-fallbacks-aws.md (PR #1039).
---
 providers/aws/services/savingsplans/client.go | 45 ++++++----
 .../aws/services/savingsplans/client_test.go  | 83 ++++++++++++-------
 2 files changed, 83 insertions(+), 45 deletions(-)

diff --git a/providers/aws/services/savingsplans/client.go b/providers/aws/services/savingsplans/client.go
index c970a286e..249b5cf3e 100644
--- a/providers/aws/services/savingsplans/client.go
+++ b/providers/aws/services/savingsplans/client.go
@@ -292,8 +292,14 @@ func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation,
 		return "", convErr
 	}
 
-	termSeconds := convertTermToSeconds(rec.Term)
-	paymentOption := convertPaymentOption(rec.PaymentOption)
+	termSeconds, err := convertTermToSeconds(rec.Term)
+	if err != nil {
+		return "", err
+	}
+	paymentOption, err := convertPaymentOption(rec.PaymentOption)
+	if err != nil {
+		return "", err
+	}
 
 	tag := execID
 	if tag == "" {
@@ -352,29 +358,34 @@ func convertPlanType(planType string) (types.SavingsPlanType, error) {
 	}
 }
 
-// convertTermToSeconds converts a term string to seconds for AWS API
-func convertTermToSeconds(term string) int64 {
-	if term == "3yr" || term == "3" {
-		return 94608000 // 3 years in seconds (365 * 3 * 86400)
-	}
-	if term != "1yr" && term != "1" && term != "" {
-		log.Printf("WARNING: unknown Savings Plans term %q, defaulting to 1 year", term)
+// convertTermToSeconds converts a term string to seconds for the AWS Savings
+// Plans API. Returns an error on any unrecognized or empty input so callers
+// fail loud rather than silently buying the wrong commitment length.
+func convertTermToSeconds(term string) (int64, error) {
+	switch term {
+	case "3yr", "3":
+		return 94608000, nil // 3 years in seconds (365 * 3 * 86400)
+	case "1yr", "1":
+		return 31536000, nil // 1 year in seconds (365 * 86400)
+	default:
+		return 0, fmt.Errorf("unsupported Savings Plans term %q: must be one of 1yr, 1, 3yr, 3", term)
 	}
-	return 31536000 // 1 year in seconds (365 * 86400)
 }
 
-// convertPaymentOption converts a payment option string to AWS SDK type
-func convertPaymentOption(paymentOption string) types.SavingsPlanPaymentOption {
+// convertPaymentOption converts a payment option string to the AWS SDK type.
+// Returns an error on any unrecognized or empty input so callers fail loud
+// rather than silently buying the wrong (and potentially most expensive)
+// payment option.
+func convertPaymentOption(paymentOption string) (types.SavingsPlanPaymentOption, error) {
 	switch paymentOption {
 	case "All Upfront", "all-upfront":
-		return types.SavingsPlanPaymentOptionAllUpfront
+		return types.SavingsPlanPaymentOptionAllUpfront, nil
 	case "Partial Upfront", "partial-upfront":
-		return types.SavingsPlanPaymentOptionPartialUpfront
+		return types.SavingsPlanPaymentOptionPartialUpfront, nil
 	case "No Upfront", "no-upfront":
-		return types.SavingsPlanPaymentOptionNoUpfront
+		return types.SavingsPlanPaymentOptionNoUpfront, nil
 	default:
-		log.Printf("WARNING: unknown Savings Plans payment option %q, defaulting to AllUpfront", paymentOption)
-		return types.SavingsPlanPaymentOptionAllUpfront
+		return "", fmt.Errorf("unsupported Savings Plans payment option %q: must be one of All Upfront, all-upfront, Partial Upfront, partial-upfront, No Upfront, no-upfront", paymentOption)
 	}
 }
 
diff --git a/providers/aws/services/savingsplans/client_test.go b/providers/aws/services/savingsplans/client_test.go
index 1f0eb6164..e52191a1e 100644
--- a/providers/aws/services/savingsplans/client_test.go
+++ b/providers/aws/services/savingsplans/client_test.go
@@ -926,18 +926,25 @@ func TestClient_FindOfferingID_AllPaymentOptions(t *testing.T) {
 	tests := []struct {
 		name          string
 		paymentOption string
+		expectError   bool
 	}{
-		{"All Upfront", "All Upfront"},
-		{"all-upfront", "all-upfront"},
-		{"Partial Upfront", "Partial Upfront"},
-		{"partial-upfront", "partial-upfront"},
-		{"No Upfront", "No Upfront"},
-		{"no-upfront", "no-upfront"},
+		{"All Upfront", "All Upfront", false},
+		{"all-upfront", "all-upfront", false},
+		{"Partial Upfront", "Partial Upfront", false},
+		{"partial-upfront", "partial-upfront", false},
+		{"No Upfront", "No Upfront", false},
+		{"no-upfront", "no-upfront", false},
+		// Unknown payment option must error, not silently default to All Upfront
+		// (regression for H1: prevents silent all-cash-outlay purchase on a typo).
+		{"unknown payment option errors", "bogus-option", true},
+		// Empty payment option must also error (regression for H1).
+		{"empty payment option errors", "", true},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			mockSP := &MockSavingsPlansClient{}
+			t.Cleanup(func() { mockSP.AssertExpectations(t) })
 			client := &Client{
 				client: mockSP,
 				region: "us-east-1",
@@ -953,34 +960,47 @@ func TestClient_FindOfferingID_AllPaymentOptions(t *testing.T) {
 				},
 			}
 
-			mockSP.On("DescribeSavingsPlansOfferings", mock.Anything, mock.Anything).
-				Return(&savingsplans.DescribeSavingsPlansOfferingsOutput{
-					SearchResults: []types.SavingsPlanOffering{
-						{OfferingId: aws.String("offering-123")},
-					},
-				}, nil)
+			if !tt.expectError {
+				mockSP.On("DescribeSavingsPlansOfferings", mock.Anything, mock.Anything).
+					Return(&savingsplans.DescribeSavingsPlansOfferingsOutput{
+						SearchResults: []types.SavingsPlanOffering{
+							{OfferingId: aws.String("offering-123")},
+						},
+					}, nil).Once()
+			}
 
 			err := client.ValidateOffering(context.Background(), rec)
-			assert.NoError(t, err)
-			mockSP.AssertExpectations(t)
+			if tt.expectError {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), "unsupported Savings Plans payment option")
+				mockSP.AssertNotCalled(t, "DescribeSavingsPlansOfferings")
+			} else {
+				assert.NoError(t, err)
+			}
 		})
 	}
 }
 
 func TestClient_FindOfferingID_TermVariations(t *testing.T) {
 	tests := []struct {
-		name string
-		term string
+		name        string
+		term        string
+		expectError bool
 	}{
-		{"1yr term", "1yr"},
-		{"3yr term", "3yr"},
-		{"3 numeric term", "3"},
-		{"default term", "invalid"},
+		{"1yr term", "1yr", false},
+		{"3yr term", "3yr", false},
+		{"1 numeric term", "1", false},
+		{"3 numeric term", "3", false},
+		// Unknown term must error, not silently default to 1yr (regression for H2).
+		{"unknown term errors", "invalid", true},
+		// Empty term must also error, not silently default to 1yr (regression for H2).
+		{"empty term errors", "", true},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			mockSP := &MockSavingsPlansClient{}
+			t.Cleanup(func() { mockSP.AssertExpectations(t) })
 			client := &Client{
 				client: mockSP,
 				region: "us-east-1",
@@ -996,16 +1016,23 @@ func TestClient_FindOfferingID_TermVariations(t *testing.T) {
 				},
 			}
 
-			mockSP.On("DescribeSavingsPlansOfferings", mock.Anything, mock.Anything).
-				Return(&savingsplans.DescribeSavingsPlansOfferingsOutput{
-					SearchResults: []types.SavingsPlanOffering{
-						{OfferingId: aws.String("offering-123")},
-					},
-				}, nil)
+			if !tt.expectError {
+				mockSP.On("DescribeSavingsPlansOfferings", mock.Anything, mock.Anything).
+					Return(&savingsplans.DescribeSavingsPlansOfferingsOutput{
+						SearchResults: []types.SavingsPlanOffering{
+							{OfferingId: aws.String("offering-123")},
+						},
+					}, nil).Once()
+			}
 
 			err := client.ValidateOffering(context.Background(), rec)
-			assert.NoError(t, err)
-			mockSP.AssertExpectations(t)
+			if tt.expectError {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), "unsupported Savings Plans term")
+				mockSP.AssertNotCalled(t, "DescribeSavingsPlansOfferings")
+			} else {
+				assert.NoError(t, err)
+			}
 		})
 	}
 }

From 10ef1d8ea47a7e56b8ee5abdd445c936818c648a Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 11:27:56 -0700
Subject: [PATCH 2/5] fix(aws): fail loud on unknown payment-option/term/engine

Fixes H3, H4, M1-M6, L1, L2 from the 19-hardcoded-fallbacks-aws.md
audit plus H1/H2 from the now-merged #1039 branch (savingsplans).

H3 (converters.go): add convertPaymentOptionE/convertTermInYearsE/
convertLookbackPeriodE that error on unrecognized input; wire into the
SP recommendation path (parser_sp.go). The legacy non-erroring wrappers
are retained for the RI path in client.go (owned by #865/#1075) with a
deprecation comment and a follow-up reference.

H4 (elasticache/client.go): convertPaymentOption now returns (string, error)
mirroring the RDS sibling that already errors correctly.

H1/H2 (savingsplans/client.go): convertPaymentOption and
convertTermToSeconds now return (type, error); cherry-picked from the
merged #1039 branch where the fix was staged.

M4 (parser_services.go, rds/client.go): parseRDSDetails no longer
silently defaults AZConfig to "single-az" when CE omits DeploymentOption;
findOfferingID rejects empty AZConfig with an explicit error.

M5 (parser_services.go): resolveEC2Tenancy now maps only the known CE
values ("shared"/nil -> default, "dedicated" -> dedicated); any other
value (e.g. "host" for Dedicated Hosts) returns an error rather than
silently collapsing to default tenancy.

M6 (rds/client.go): normalizeEngineName now returns (string, error) and
errors for ambiguous Oracle ("oracle*"), SQL Server ("sqlserver*"/"sql-server*"),
and bare Aurora inputs; unambiguous engines (mysql, postgresql, mariadb,
aurora-mysql, aurora-postgresql) pass through unchanged.

M1/M2/M3 (ec2/client.go): replace raw "convertible" literals with
types.OfferingClassTypeConvertible; buildEC2OfferingQuery errors on empty
Platform (purchase path); centralize "Linux/UNIX" literal into
defaultEC2Platform constant (exchange-path helpers retain their safe
fallback; the purchase path does not).

Regression tests added for H3, H1/H2, H4, M2, M4, M5, M6, L1, L2; each
fails pre-fix and passes post-fix.

Closes #1084
---
 providers/aws/recommendations/converters.go   |  98 +++++++---
 .../aws/recommendations/converters_test.go    | 180 ++++++++++++++----
 .../aws/recommendations/parser_services.go    |  40 +++-
 .../recommendations/parser_services_test.go   |  44 ++++-
 providers/aws/recommendations/parser_sp.go    |  18 +-
 providers/aws/services/ec2/client.go          |  40 ++--
 providers/aws/services/ec2/client_test.go     |  45 +++++
 providers/aws/services/rds/client.go          |  57 ++++--
 providers/aws/services/rds/client_test.go     | 115 +++++++++--
 9 files changed, 519 insertions(+), 118 deletions(-)

diff --git a/providers/aws/recommendations/converters.go b/providers/aws/recommendations/converters.go
index 499b5bb69..e4a7e457f 100644
--- a/providers/aws/recommendations/converters.go
+++ b/providers/aws/recommendations/converters.go
@@ -1,6 +1,7 @@
 package recommendations
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/aws/aws-sdk-go-v2/service/costexplorer/types"
@@ -28,55 +29,102 @@ func getServiceStringForCostExplorer(service common.ServiceType) string {
 	}
 }
 
-// convertPaymentOption converts payment option string to AWS type
+// convertPaymentOption converts payment option string to AWS type.
+//
+// Deprecated: this function silently defaults to NoUpfront for unrecognized
+// values and is retained only for the legacy RI recommendation-fetch path in
+// client.go (owned by #865/#1075). New callers must use convertPaymentOptionE
+// and propagate the error. See open-questions/fix-aws-converters.md OQ-1.
 func convertPaymentOption(option string) types.PaymentOption {
+	v, _ := convertPaymentOptionE(option)
+	return v
+}
+
+// convertPaymentOptionE converts a payment option string to the AWS Cost Explorer
+// typed enum, returning an error for any value not in the known set.
+// Known values: "all-upfront", "partial-upfront", "no-upfront".
+func convertPaymentOptionE(option string) (types.PaymentOption, error) {
 	switch option {
 	case "all-upfront":
-		return types.PaymentOptionAllUpfront
+		return types.PaymentOptionAllUpfront, nil
 	case "partial-upfront":
-		return types.PaymentOptionPartialUpfront
+		return types.PaymentOptionPartialUpfront, nil
 	case "no-upfront":
-		return types.PaymentOptionNoUpfront
+		return types.PaymentOptionNoUpfront, nil
 	default:
-		return types.PaymentOptionNoUpfront
+		return "", fmt.Errorf("unsupported payment option %q: must be one of all-upfront, partial-upfront, no-upfront", option)
 	}
 }
 
-// convertTermInYears converts term string to AWS type
-func convertTermInYears(term string) types.TermInYears {
-	if term == "3yr" || term == "3" {
-		return types.TermInYearsThreeYears
+// convertTermInYearsE converts a term string to the AWS Cost Explorer typed enum,
+// returning an error for any value not in the known set.
+// Known values: "1yr", "1", "3yr", "3".
+func convertTermInYearsE(term string) (types.TermInYears, error) {
+	switch term {
+	case "1yr", "1":
+		return types.TermInYearsOneYear, nil
+	case "3yr", "3":
+		return types.TermInYearsThreeYears, nil
+	default:
+		return "", fmt.Errorf("unsupported term %q: must be one of 1yr, 1, 3yr, 3", term)
 	}
-	return types.TermInYearsOneYear
 }
 
-// convertLookbackPeriod converts lookback period string to AWS type
-func convertLookbackPeriod(period string) types.LookbackPeriodInDays {
+// convertTermInYears converts term string to AWS type.
+//
+// Deprecated: this function silently defaults to OneYear for unrecognized
+// values and is retained only for the legacy RI recommendation-fetch path in
+// client.go (owned by #865/#1075). New callers must use convertTermInYearsE
+// and propagate the error. See open-questions/fix-aws-converters.md OQ-1.
+func convertTermInYears(term string) types.TermInYears {
+	v, _ := convertTermInYearsE(term)
+	return v
+}
+
+// convertLookbackPeriodE converts a lookback period string to the AWS Cost Explorer
+// typed enum, returning an error for any value not in the known set.
+// Known values: "7d", "7", "30d", "30", "60d", "60".
+func convertLookbackPeriodE(period string) (types.LookbackPeriodInDays, error) {
 	switch period {
 	case "7d", "7":
-		return types.LookbackPeriodInDaysSevenDays
+		return types.LookbackPeriodInDaysSevenDays, nil
 	case "30d", "30":
-		return types.LookbackPeriodInDaysThirtyDays
+		return types.LookbackPeriodInDaysThirtyDays, nil
 	case "60d", "60":
-		return types.LookbackPeriodInDaysSixtyDays
+		return types.LookbackPeriodInDaysSixtyDays, nil
 	default:
-		return types.LookbackPeriodInDaysSevenDays
+		return "", fmt.Errorf("unsupported lookback period %q: must be one of 7d, 30d, 60d", period)
 	}
 }
 
-// convertSavingsPlansPaymentOption converts payment option for Savings Plans
-func convertSavingsPlansPaymentOption(option string) types.PaymentOption {
-	return convertPaymentOption(option)
+// convertLookbackPeriod converts lookback period string to AWS type.
+//
+// Deprecated: this function silently defaults to SevenDays for unrecognized
+// values and is retained only for the legacy RI recommendation-fetch path in
+// client.go (owned by #865/#1075). New callers must use convertLookbackPeriodE
+// and propagate the error. See open-questions/fix-aws-converters.md OQ-1.
+func convertLookbackPeriod(period string) types.LookbackPeriodInDays {
+	v, _ := convertLookbackPeriodE(period)
+	return v
+}
+
+// convertSavingsPlansPaymentOption converts payment option for Savings Plans,
+// returning an error for unrecognized values. This is the fail-loud variant
+// used by the SP recommendation path.
+func convertSavingsPlansPaymentOption(option string) (types.PaymentOption, error) {
+	return convertPaymentOptionE(option)
 }
 
-// convertSavingsPlansTermInYears converts term for Savings Plans
-func convertSavingsPlansTermInYears(term string) types.TermInYears {
-	return convertTermInYears(term)
+// convertSavingsPlansTermInYears converts term for Savings Plans,
+// returning an error for unrecognized values.
+func convertSavingsPlansTermInYears(term string) (types.TermInYears, error) {
+	return convertTermInYearsE(term)
 }
 
-// convertSavingsPlansLookbackPeriod converts lookback period for Savings Plans
-func convertSavingsPlansLookbackPeriod(period string) types.LookbackPeriodInDays {
-	return convertLookbackPeriod(period)
+// convertSavingsPlansLookbackPeriod converts lookback period for Savings Plans,
+// returning an error for unrecognized values.
+func convertSavingsPlansLookbackPeriod(period string) (types.LookbackPeriodInDays, error) {
+	return convertLookbackPeriodE(period)
 }
 
 // normalizeRegionName converts AWS region display names to region codes
diff --git a/providers/aws/recommendations/converters_test.go b/providers/aws/recommendations/converters_test.go
index a37fe20b4..231540c40 100644
--- a/providers/aws/recommendations/converters_test.go
+++ b/providers/aws/recommendations/converters_test.go
@@ -86,6 +86,10 @@ func TestGetServiceStringForCostExplorer(t *testing.T) {
 }
 
 func TestConvertPaymentOption(t *testing.T) {
+	// convertPaymentOption is the legacy wrapper that silently defaults to NoUpfront
+	// for unknown values (used by client.go RI path, owned by #865/#1075).
+	// This test documents that silent-default behaviour; new callers should use
+	// convertPaymentOptionE which returns an error on unrecognised values.
 	tests := []struct {
 		name     string
 		option   string
@@ -106,16 +110,6 @@ func TestConvertPaymentOption(t *testing.T) {
 			option:   "no-upfront",
 			expected: types.PaymentOptionNoUpfront,
 		},
-		{
-			name:     "Unknown defaults to no upfront",
-			option:   "unknown",
-			expected: types.PaymentOptionNoUpfront,
-		},
-		{
-			name:     "Empty string defaults to no upfront",
-			option:   "",
-			expected: types.PaymentOptionNoUpfront,
-		},
 	}
 
 	for _, tt := range tests {
@@ -126,7 +120,116 @@ func TestConvertPaymentOption(t *testing.T) {
 	}
 }
 
+// TestConvertPaymentOptionE_FailLoud is the regression test for H3:
+// convertPaymentOptionE must return an error on any unrecognised payment option
+// instead of silently substituting NoUpfront (the old behaviour of the
+// convertPaymentOption default branch). Callers on the SP recommendation path
+// use this erroring variant so a typo or new/renamed option is caught
+// before the wrong recs are queried.
+func TestConvertPaymentOptionE_FailLoud(t *testing.T) {
+	tests := []struct {
+		name        string
+		option      string
+		expected    types.PaymentOption
+		expectError bool
+	}{
+		{"All upfront", "all-upfront", types.PaymentOptionAllUpfront, false},
+		{"Partial upfront", "partial-upfront", types.PaymentOptionPartialUpfront, false},
+		{"No upfront", "no-upfront", types.PaymentOptionNoUpfront, false},
+		// These must error, not default to NoUpfront (H3 regression guard):
+		{"Unknown option errors", "unknown", "", true},
+		{"Empty string errors", "", "", true},
+		{"Mixed case errors", "All-Upfront", "", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := convertPaymentOptionE(tt.option)
+			if tt.expectError {
+				assert.Error(t, err, "convertPaymentOptionE(%q) must error", tt.option)
+				assert.Empty(t, result)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}
+
+// TestConvertTermInYearsE_FailLoud is the regression test for L1:
+// convertTermInYearsE must error on unrecognised terms rather than silently
+// defaulting to OneYear.
+func TestConvertTermInYearsE_FailLoud(t *testing.T) {
+	tests := []struct {
+		name        string
+		term        string
+		expected    types.TermInYears
+		expectError bool
+	}{
+		{"1yr", "1yr", types.TermInYearsOneYear, false},
+		{"1 numeric", "1", types.TermInYearsOneYear, false},
+		{"3yr", "3yr", types.TermInYearsThreeYears, false},
+		{"3 numeric", "3", types.TermInYearsThreeYears, false},
+		// These must error, not default to OneYear (L1 regression guard):
+		{"Unknown term errors", "unknown", "", true},
+		{"Empty string errors", "", "", true},
+		{"2yr errors", "2yr", "", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := convertTermInYearsE(tt.term)
+			if tt.expectError {
+				assert.Error(t, err, "convertTermInYearsE(%q) must error", tt.term)
+				assert.Empty(t, result)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}
+
+// TestConvertLookbackPeriodE_FailLoud is the regression test for L2:
+// convertLookbackPeriodE must error on unrecognised periods rather than
+// silently defaulting to SevenDays.
+func TestConvertLookbackPeriodE_FailLoud(t *testing.T) {
+	tests := []struct {
+		name        string
+		period      string
+		expected    types.LookbackPeriodInDays
+		expectError bool
+	}{
+		{"7d", "7d", types.LookbackPeriodInDaysSevenDays, false},
+		{"7 numeric", "7", types.LookbackPeriodInDaysSevenDays, false},
+		{"30d", "30d", types.LookbackPeriodInDaysThirtyDays, false},
+		{"30 numeric", "30", types.LookbackPeriodInDaysThirtyDays, false},
+		{"60d", "60d", types.LookbackPeriodInDaysSixtyDays, false},
+		{"60 numeric", "60", types.LookbackPeriodInDaysSixtyDays, false},
+		// These must error, not default to SevenDays (L2 regression guard):
+		{"Unknown period errors", "unknown", "", true},
+		{"Empty string errors", "", "", true},
+		{"90d errors", "90d", "", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := convertLookbackPeriodE(tt.period)
+			if tt.expectError {
+				assert.Error(t, err, "convertLookbackPeriodE(%q) must error", tt.period)
+				assert.Empty(t, result)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}
+
 func TestConvertTermInYears(t *testing.T) {
+	// convertTermInYears is the legacy wrapper used by client.go (RI path);
+	// it silently returns OneYear for unrecognised values. This test covers the
+	// valid cases only; the fail-loud path is tested by TestConvertTermInYearsE_FailLoud.
 	tests := []struct {
 		name     string
 		term     string
@@ -152,16 +255,6 @@ func TestConvertTermInYears(t *testing.T) {
 			term:     "1",
 			expected: types.TermInYearsOneYear,
 		},
-		{
-			name:     "Empty defaults to one year",
-			term:     "",
-			expected: types.TermInYearsOneYear,
-		},
-		{
-			name:     "Unknown defaults to one year",
-			term:     "unknown",
-			expected: types.TermInYearsOneYear,
-		},
 	}
 
 	for _, tt := range tests {
@@ -173,6 +266,9 @@ func TestConvertTermInYears(t *testing.T) {
 }
 
 func TestConvertLookbackPeriod(t *testing.T) {
+	// convertLookbackPeriod is the legacy wrapper used by client.go (RI path);
+	// it silently returns SevenDays for unrecognised values. This test covers valid
+	// cases only; the fail-loud path is tested by TestConvertLookbackPeriodE_FailLoud.
 	tests := []struct {
 		name     string
 		period   string
@@ -208,16 +304,6 @@ func TestConvertLookbackPeriod(t *testing.T) {
 			period:   "60",
 			expected: types.LookbackPeriodInDaysSixtyDays,
 		},
-		{
-			name:     "Empty defaults to seven days",
-			period:   "",
-			expected: types.LookbackPeriodInDaysSevenDays,
-		},
-		{
-			name:     "Unknown defaults to seven days",
-			period:   "unknown",
-			expected: types.LookbackPeriodInDaysSevenDays,
-		},
 	}
 
 	for _, tt := range tests {
@@ -229,30 +315,46 @@ func TestConvertLookbackPeriod(t *testing.T) {
 }
 
 func TestConvertSavingsPlansPaymentOption(t *testing.T) {
-	// This function delegates to convertPaymentOption, so we just verify it works
-	result := convertSavingsPlansPaymentOption("all-upfront")
+	// SP wrappers now return (value, error); valid options must succeed.
+	result, err := convertSavingsPlansPaymentOption("all-upfront")
+	assert.NoError(t, err)
 	assert.Equal(t, types.PaymentOptionAllUpfront, result)
 
-	result = convertSavingsPlansPaymentOption("partial-upfront")
+	result, err = convertSavingsPlansPaymentOption("partial-upfront")
+	assert.NoError(t, err)
 	assert.Equal(t, types.PaymentOptionPartialUpfront, result)
+
+	// Unknown option must error (not silently default).
+	_, err = convertSavingsPlansPaymentOption("bogus")
+	assert.Error(t, err, "convertSavingsPlansPaymentOption(bogus) must error")
 }
 
 func TestConvertSavingsPlansTermInYears(t *testing.T) {
-	// This function delegates to convertTermInYears, so we just verify it works
-	result := convertSavingsPlansTermInYears("3yr")
+	result, err := convertSavingsPlansTermInYears("3yr")
+	assert.NoError(t, err)
 	assert.Equal(t, types.TermInYearsThreeYears, result)
 
-	result = convertSavingsPlansTermInYears("1yr")
+	result, err = convertSavingsPlansTermInYears("1yr")
+	assert.NoError(t, err)
 	assert.Equal(t, types.TermInYearsOneYear, result)
+
+	// Unknown term must error.
+	_, err = convertSavingsPlansTermInYears("bogus")
+	assert.Error(t, err, "convertSavingsPlansTermInYears(bogus) must error")
 }
 
 func TestConvertSavingsPlansLookbackPeriod(t *testing.T) {
-	// This function delegates to convertLookbackPeriod, so we just verify it works
-	result := convertSavingsPlansLookbackPeriod("7d")
+	result, err := convertSavingsPlansLookbackPeriod("7d")
+	assert.NoError(t, err)
 	assert.Equal(t, types.LookbackPeriodInDaysSevenDays, result)
 
-	result = convertSavingsPlansLookbackPeriod("30d")
+	result, err = convertSavingsPlansLookbackPeriod("30d")
+	assert.NoError(t, err)
 	assert.Equal(t, types.LookbackPeriodInDaysThirtyDays, result)
+
+	// Unknown period must error.
+	_, err = convertSavingsPlansLookbackPeriod("bogus")
+	assert.Error(t, err, "convertSavingsPlansLookbackPeriod(bogus) must error")
 }
 
 func TestNormalizeRegionName(t *testing.T) {
diff --git a/providers/aws/recommendations/parser_services.go b/providers/aws/recommendations/parser_services.go
index cb456c58e..fc56a2a3d 100644
--- a/providers/aws/recommendations/parser_services.go
+++ b/providers/aws/recommendations/parser_services.go
@@ -29,14 +29,18 @@ func (c *Client) parseRDSDetails(_ context.Context, rec *common.Recommendation,
 	if rdsDetails.Region != nil {
 		rec.Region = normalizeRegionName(*rdsDetails.Region)
 	}
+	// AZConfig is intentionally left empty when CE omits DeploymentOption.
+	// Single-AZ and multi-AZ RDS RIs have different prices and cover different
+	// workloads; silently defaulting to single-az would cause findOfferingID to
+	// query for (and potentially buy) a single-AZ RI even for a multi-AZ workload.
+	// The downstream findOfferingID rejects an empty AZConfig with an explicit
+	// error rather than proceeding with a fabricated value (M4 fix).
 	if rdsDetails.DeploymentOption != nil {
 		if *rdsDetails.DeploymentOption == "Multi-AZ" {
 			rdsInfo.AZConfig = "multi-az"
 		} else {
 			rdsInfo.AZConfig = "single-az"
 		}
-	} else {
-		rdsInfo.AZConfig = "single-az"
 	}
 
 	rec.Details = rdsInfo
@@ -69,12 +73,28 @@ func (c *Client) parseElastiCacheDetails(_ context.Context, rec *common.Recommen
 
 // resolveEC2Tenancy maps a Cost Explorer tenancy value to the EC2 RI API
 // tenancy string. CE uses "shared" for the default tenancy; "dedicated" maps
-// directly. Any nil or unrecognised value is treated as default.
-func resolveEC2Tenancy(tenancy *string) string {
-	if tenancy != nil && *tenancy == "dedicated" {
-		return string(ec2types.TenancyDedicated)
+// directly. A nil pointer (CE omitted the field) also maps to default, because
+// CE only populates the field when it is non-default.
+//
+// Unknown tenancy values (e.g. "host" for Dedicated Hosts, which have no
+// corresponding RI product) return an error so the caller fails loud rather
+// than silently querying for and buying a default-tenancy RI on behalf of a
+// workload that requires a different tenancy class (M5 fix).
+func resolveEC2Tenancy(tenancy *string) (string, error) {
+	if tenancy == nil || *tenancy == "shared" {
+		return string(ec2types.TenancyDefault), nil
+	}
+	switch *tenancy {
+	case "dedicated":
+		return string(ec2types.TenancyDedicated), nil
+	default:
+		return "", fmt.Errorf(
+			"unrecognised EC2 tenancy %q from Cost Explorer: "+
+				"must be shared (default) or dedicated; "+
+				"host tenancy has no corresponding RI product",
+			*tenancy,
+		)
 	}
-	return string(ec2types.TenancyDefault)
 }
 
 // resolveEC2Scope maps a Cost Explorer availability zone value to the EC2 RI
@@ -126,7 +146,11 @@ func (c *Client) parseEC2Details(ctx context.Context, rec *common.Recommendation
 	if ec2Details.Region != nil {
 		rec.Region = normalizeRegionName(*ec2Details.Region)
 	}
-	ec2Info.Tenancy = resolveEC2Tenancy(ec2Details.Tenancy)
+	tenancy, tenancyErr := resolveEC2Tenancy(ec2Details.Tenancy)
+	if tenancyErr != nil {
+		return tenancyErr
+	}
+	ec2Info.Tenancy = tenancy
 	ec2Info.Scope = resolveEC2Scope(ec2Details.AvailabilityZone)
 	c.enrichFromCatalogue(ctx, ec2Info)
 
diff --git a/providers/aws/recommendations/parser_services_test.go b/providers/aws/recommendations/parser_services_test.go
index e01c2bd94..26d0e43c4 100644
--- a/providers/aws/recommendations/parser_services_test.go
+++ b/providers/aws/recommendations/parser_services_test.go
@@ -65,7 +65,11 @@ func TestParseRDSDetails(t *testing.T) {
 			},
 		},
 		{
-			name: "RDS details without deployment option defaults to single-az",
+			// M4 regression test: when CE omits DeploymentOption, AZConfig must be
+			// left empty rather than silently defaulted to "single-az".
+			// single-AZ and multi-AZ RDS RIs have different prices; guessing wrong
+			// would cause findOfferingID to buy the wrong offering class.
+			name: "RDS details without deployment option leaves AZConfig empty (M4)",
 			details: &types.ReservationPurchaseRecommendationDetail{
 				InstanceDetails: &types.InstanceDetails{
 					RDSInstanceDetails: &types.RDSInstanceDetails{
@@ -81,7 +85,8 @@ func TestParseRDSDetails(t *testing.T) {
 				dbDetails, ok := rec.Details.(*common.DatabaseDetails)
 				require.True(t, ok)
 				assert.Equal(t, "aurora-postgresql", dbDetails.Engine)
-				assert.Equal(t, "single-az", dbDetails.AZConfig)
+				// AZConfig must be empty, not "single-az" (M4 fix).
+				assert.Empty(t, dbDetails.AZConfig, "AZConfig must not be silently defaulted to single-az")
 			},
 		},
 		{
@@ -330,6 +335,41 @@ func TestParseEC2Details(t *testing.T) {
 			},
 			expectError: true,
 		},
+		{
+			// M5 regression test: "host" tenancy (Dedicated Hosts) has no
+			// corresponding EC2 RI product. Previously this silently collapsed
+			// to "default", which would cause findOfferingID to look up (and
+			// potentially buy) a default-tenancy RI for a Dedicated Host workload.
+			// The parser must error so the caller can decide, rather than
+			// silently buying the wrong product.
+			name: "EC2 host tenancy errors (M5)",
+			details: &types.ReservationPurchaseRecommendationDetail{
+				InstanceDetails: &types.InstanceDetails{
+					EC2InstanceDetails: &types.EC2InstanceDetails{
+						InstanceType: aws.String("m5.large"),
+						Platform:     aws.String("Linux/UNIX"),
+						Region:       aws.String("us-east-1"),
+						Tenancy:      aws.String("host"),
+					},
+				},
+			},
+			expectError: true,
+		},
+		{
+			// M5: an unrecognised tenancy value should also error.
+			name: "EC2 unknown tenancy errors (M5)",
+			details: &types.ReservationPurchaseRecommendationDetail{
+				InstanceDetails: &types.InstanceDetails{
+					EC2InstanceDetails: &types.EC2InstanceDetails{
+						InstanceType: aws.String("m5.large"),
+						Platform:     aws.String("Linux/UNIX"),
+						Region:       aws.String("us-east-1"),
+						Tenancy:      aws.String("future-unknown-tenancy"),
+					},
+				},
+			},
+			expectError: true,
+		},
 	}
 
 	for _, tt := range tests {
diff --git a/providers/aws/recommendations/parser_sp.go b/providers/aws/recommendations/parser_sp.go
index 6eebc4712..41b865eaf 100644
--- a/providers/aws/recommendations/parser_sp.go
+++ b/providers/aws/recommendations/parser_sp.go
@@ -37,11 +37,23 @@ func (c *Client) getSavingsPlansRecommendations(ctx context.Context, params comm
 	var allRecommendations []common.Recommendation
 
 	for _, planType := range planTypes {
+		paymentOption, err := convertSavingsPlansPaymentOption(params.PaymentOption)
+		if err != nil {
+			return nil, fmt.Errorf("invalid payment option for Savings Plans recommendation: %w", err)
+		}
+		termInYears, err := convertSavingsPlansTermInYears(params.Term)
+		if err != nil {
+			return nil, fmt.Errorf("invalid term for Savings Plans recommendation: %w", err)
+		}
+		lookbackPeriod, err := convertSavingsPlansLookbackPeriod(params.LookbackPeriod)
+		if err != nil {
+			return nil, fmt.Errorf("invalid lookback period for Savings Plans recommendation: %w", err)
+		}
 		input := &costexplorer.GetSavingsPlansPurchaseRecommendationInput{
 			SavingsPlansType:     planType,
-			PaymentOption:        convertSavingsPlansPaymentOption(params.PaymentOption),
-			TermInYears:          convertSavingsPlansTermInYears(params.Term),
-			LookbackPeriodInDays: convertSavingsPlansLookbackPeriod(params.LookbackPeriod),
+			PaymentOption:        paymentOption,
+			TermInYears:          termInYears,
+			LookbackPeriodInDays: lookbackPeriod,
 			AccountScope:         types.AccountScopeLinked,
 		}
 
diff --git a/providers/aws/services/ec2/client.go b/providers/aws/services/ec2/client.go
index fa1a0ebf5..2b31d1cf1 100644
--- a/providers/aws/services/ec2/client.go
+++ b/providers/aws/services/ec2/client.go
@@ -341,6 +341,13 @@ func canonicalizeEC2Scope(s string) string {
 // of timing out the Lambda budget (issue #688).
 const maxOfferingPages = 5
 
+// defaultEC2Platform is the EC2 RI product-description value for Linux/UNIX instances.
+// Used as a fallback in exchange-package helpers (FindConvertibleOffering,
+// ListTargetOfferings) where callers may legitimately omit the platform; never
+// used as a silent fallback on the purchase path (see M2/M3 in
+// 19-hardcoded-fallbacks-aws.md).
+const defaultEC2Platform = "Linux/UNIX"
+
 // convertEC2PaymentOption maps a rec payment-option slug to the AWS
 // DescribeReservedInstancesOfferings OfferingType enum value.
 func convertEC2PaymentOption(option string) (types.OfferingTypeValues, error) {
@@ -367,11 +374,17 @@ type ec2OfferingQuery struct {
 }
 
 // buildEC2OfferingQuery resolves the typed lookup parameters from a rec,
-// canonicalising legacy tenancy/scope values and applying API defaults.
-func buildEC2OfferingQuery(rec common.Recommendation, details *common.ComputeDetails, duration int64) ec2OfferingQuery {
-	platform := details.Platform
-	if platform == "" {
-		platform = "Linux/UNIX"
+// canonicalising legacy tenancy/scope values. Returns an error when Platform is
+// empty: on the purchase path the CE parser always populates it from the
+// recommendation payload, so an empty Platform signals a malformed rec rather
+// than a value that should be fabricated (M2/M3 fix, see 19-hardcoded-fallbacks-aws.md).
+func buildEC2OfferingQuery(rec common.Recommendation, details *common.ComputeDetails, duration int64) (ec2OfferingQuery, error) {
+	if details.Platform == "" {
+		return ec2OfferingQuery{}, fmt.Errorf(
+			"EC2 recommendation for %s is missing Platform: "+
+				"refusing to fabricate a product-description for the RI offering lookup",
+			rec.ResourceType,
+		)
 	}
 	tenancy := canonicalizeEC2Tenancy(details.Tenancy)
 	if tenancy == "" {
@@ -383,11 +396,11 @@ func buildEC2OfferingQuery(rec common.Recommendation, details *common.ComputeDet
 	}
 	return ec2OfferingQuery{
 		instanceType: types.InstanceType(rec.ResourceType),
-		productDesc:  types.RIProductDescription(platform),
+		productDesc:  types.RIProductDescription(details.Platform),
 		tenancy:      types.Tenancy(tenancy),
 		scope:        scope,
 		duration:     duration,
-	}
+	}, nil
 }
 
 // describeInputFromQuery builds the SDK request struct for one page of the
@@ -424,7 +437,10 @@ func (c *Client) buildEC2QueryFromRec(rec common.Recommendation) (ec2OfferingQue
 	if err != nil {
 		return ec2OfferingQuery{}, err
 	}
-	q := buildEC2OfferingQuery(rec, details, c.getDurationValue(rec.Term))
+	q, err := buildEC2OfferingQuery(rec, details, c.getDurationValue(rec.Term))
+	if err != nil {
+		return ec2OfferingQuery{}, err
+	}
 	q.wantOfferingType = wantOfferingType
 	return q, nil
 }
@@ -658,7 +674,7 @@ func (c *Client) ListConvertibleReservedInstances(ctx context.Context) ([]Conver
 			},
 			{
 				Name:   aws.String("offering-class"),
-				Values: []string{"convertible"},
+				Values: []string{string(types.OfferingClassTypeConvertible)},
 			},
 		},
 	}
@@ -726,7 +742,7 @@ func (c *Client) FindConvertibleOffering(ctx context.Context, params FindConvert
 	}
 	productDesc := params.ProductDescription
 	if productDesc == "" {
-		productDesc = "Linux/UNIX"
+		productDesc = defaultEC2Platform
 	}
 
 	filters := []types.Filter{
@@ -735,7 +751,7 @@ func (c *Client) FindConvertibleOffering(ctx context.Context, params FindConvert
 		{Name: aws.String("instance-tenancy"), Values: []string{tenancy}},
 		{Name: aws.String("scope"), Values: []string{scope}},
 		{Name: aws.String("duration"), Values: []string{fmt.Sprintf("%d", duration)}},
-		{Name: aws.String("offering-class"), Values: []string{"convertible"}},
+		{Name: aws.String("offering-class"), Values: []string{string(types.OfferingClassTypeConvertible)}},
 	}
 
 	input := &ec2.DescribeReservedInstancesOfferingsInput{
@@ -804,7 +820,7 @@ func normalizeTargetOfferingsParams(p ListTargetOfferingsParams) (tenancy, scope
 	}
 	productDesc = p.ProductDescription
 	if productDesc == "" {
-		productDesc = "Linux/UNIX"
+		productDesc = defaultEC2Platform
 	}
 	// OfferingType: typed field when non-empty; empty string means "all
 	// payment options" (the caller didn't specify). Leave unset so AWS
diff --git a/providers/aws/services/ec2/client_test.go b/providers/aws/services/ec2/client_test.go
index 7f1083423..44ecee07d 100644
--- a/providers/aws/services/ec2/client_test.go
+++ b/providers/aws/services/ec2/client_test.go
@@ -775,3 +775,48 @@ func TestClient_PurchaseCommitment_NameTagInCreateTagsRequest(t *testing.T) {
 
 	mockEC2.AssertExpectations(t)
 }
+
+// TestBuildEC2OfferingQuery_EmptyPlatformErrors is the M2/M3 regression test:
+// buildEC2OfferingQuery must return an error when Platform is empty rather than
+// silently substituting "Linux/UNIX". On the purchase path the CE parser always
+// populates Platform from the recommendation payload; an empty value signals a
+// malformed rec, not a value to be fabricated.
+func TestBuildEC2OfferingQuery_EmptyPlatformErrors(t *testing.T) {
+	rec := common.Recommendation{
+		ResourceType:  "m5.large",
+		PaymentOption: "all-upfront",
+		Term:          "1yr",
+		Details: &common.ComputeDetails{
+			InstanceType: "m5.large",
+			Platform:     "", // intentionally empty
+			Tenancy:      "default",
+			Scope:        "Region",
+		},
+	}
+	details := rec.Details.(*common.ComputeDetails)
+
+	_, err := buildEC2OfferingQuery(rec, details, OneYearSeconds)
+	assert.Error(t, err, "buildEC2OfferingQuery must error when Platform is empty (M2/M3 fix)")
+	assert.Contains(t, err.Error(), "Platform")
+}
+
+// TestBuildEC2OfferingQuery_ValidPlatform asserts the happy path still works.
+func TestBuildEC2OfferingQuery_ValidPlatform(t *testing.T) {
+	rec := common.Recommendation{
+		ResourceType:  "m5.large",
+		PaymentOption: "all-upfront",
+		Term:          "1yr",
+		Details: &common.ComputeDetails{
+			InstanceType: "m5.large",
+			Platform:     "Linux/UNIX",
+			Tenancy:      "default",
+			Scope:        "Region",
+		},
+	}
+	details := rec.Details.(*common.ComputeDetails)
+
+	q, err := buildEC2OfferingQuery(rec, details, OneYearSeconds)
+	assert.NoError(t, err)
+	assert.Equal(t, types.RIProductDescription("Linux/UNIX"), q.productDesc)
+	assert.Equal(t, types.Tenancy("default"), q.tenancy)
+}
diff --git a/providers/aws/services/rds/client.go b/providers/aws/services/rds/client.go
index fe65f85e9..8cf4cdb7e 100644
--- a/providers/aws/services/rds/client.go
+++ b/providers/aws/services/rds/client.go
@@ -303,6 +303,14 @@ func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation,
 	if !ok || details == nil {
 		return "", fmt.Errorf("invalid service details for RDS")
 	}
+	// AZConfig must be explicitly set: single-AZ and multi-AZ RDS RIs have
+	// different prices and do not cover each other's demand. An empty AZConfig
+	// means the CE recommendation omitted DeploymentOption; guessing single-az
+	// risks buying the wrong RI class. Fail loud so the caller can decide.
+	if details.AZConfig == "" {
+		return "", fmt.Errorf("RDS AZConfig is empty: CE recommendation did not include DeploymentOption; "+
+			"refusing to guess single-az vs multi-az (see M4 in 19-hardcoded-fallbacks-aws.md)")
+	}
 	offeringType, err := c.convertPaymentOption(rec.PaymentOption)
 	if err != nil {
 		return "", fmt.Errorf("invalid payment option: %w", err)
@@ -315,7 +323,10 @@ func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation,
 // timeout exhaustion (issue #688).
 func (c *Client) paginateRDSOfferings(ctx context.Context, rec common.Recommendation, details *common.DatabaseDetails, offeringType string, execID string) (string, error) {
 	multiAZ := details.AZConfig == "multi-az"
-	normalizedEngine := c.normalizeEngineName(details.Engine)
+	normalizedEngine, err := c.normalizeEngineName(details.Engine)
+	if err != nil {
+		return "", fmt.Errorf("cannot look up RDS offering: %w", err)
+	}
 	duration := c.getDurationString(rec.Term)
 
 	tag := execID
@@ -504,38 +515,58 @@ func (c *Client) convertPaymentOption(option string) (string, error) {
 	}
 }
 
-// normalizeEngineName converts engine names to AWS API format
-func (c *Client) normalizeEngineName(engine string) string {
+// normalizeEngineName maps an RDS engine string to the exact product-description
+// value required by DescribeReservedDBInstancesOfferings. It returns an error
+// for engine names that are ambiguous (Oracle, SQL Server -- multiple editions
+// exist at different prices) or that contain "aurora" without specifying a
+// database engine (aurora-mysql vs aurora-postgresql), so the caller never
+// silently buys an offering for the wrong engine edition.
+//
+// Unambiguous engines (mysql, postgresql, mariadb) are returned verbatim after
+// case-normalisation. Engine strings that are already in the canonical
+// lower-case AWS form (e.g. "aurora-mysql") pass through unchanged.
+func (c *Client) normalizeEngineName(engine string) (string, error) {
 	engineLower := strings.ToLower(engine)
 
 	if strings.Contains(engineLower, "aurora") {
 		if strings.Contains(engineLower, "mysql") {
-			return "aurora-mysql"
+			return "aurora-mysql", nil
 		}
 		if strings.Contains(engineLower, "postgres") {
-			return "aurora-postgresql"
+			return "aurora-postgresql", nil
 		}
-		log.Printf("WARNING: Unknown Aurora variant %q, defaulting to aurora-mysql", engine)
-		return "aurora-mysql"
+		return "", fmt.Errorf(
+			"ambiguous Aurora engine %q: CE must supply the specific variant "+
+				"(aurora-mysql or aurora-postgresql); refusing to guess",
+			engine,
+		)
 	}
 
 	if strings.Contains(engineLower, "mysql") {
-		return "mysql"
+		return "mysql", nil
 	}
 	if strings.Contains(engineLower, "postgres") {
-		return "postgresql"
+		return "postgresql", nil
 	}
 	if strings.Contains(engineLower, "mariadb") {
-		return "mariadb"
+		return "mariadb", nil
 	}
 	if strings.Contains(engineLower, "oracle") {
-		return "oracle-se2"
+		return "", fmt.Errorf(
+			"ambiguous Oracle engine %q: CE must supply the exact edition "+
+				"(e.g. oracle-se2, oracle-ee); refusing to guess",
+			engine,
+		)
 	}
 	if strings.Contains(engineLower, "sqlserver") || strings.Contains(engineLower, "sql-server") {
-		return "sqlserver-se"
+		return "", fmt.Errorf(
+			"ambiguous SQL Server engine %q: CE must supply the exact edition "+
+				"(e.g. sqlserver-se, sqlserver-ee, sqlserver-web, sqlserver-ex); refusing to guess",
+			engine,
+		)
 	}
 
-	return engineLower
+	return engineLower, nil
 }
 
 // createPurchaseTags creates standard tags for the purchase. The tag shape
diff --git a/providers/aws/services/rds/client_test.go b/providers/aws/services/rds/client_test.go
index 7541ca874..807dc0158 100644
--- a/providers/aws/services/rds/client_test.go
+++ b/providers/aws/services/rds/client_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/rds/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 )
 
 // MockRDSClient implements RDSAPI for testing
@@ -480,27 +481,42 @@ func TestClient_NormalizeEngineName(t *testing.T) {
 	client := &Client{}
 
 	tests := []struct {
-		name     string
-		input    string
-		expected string
+		name        string
+		input       string
+		expected    string
+		expectError bool
 	}{
-		{"Aurora MySQL uppercase", "Aurora-MySQL", "aurora-mysql"},
-		{"Aurora PostgreSQL mixed case", "Aurora-PostgreSQL", "aurora-postgresql"},
-		{"Aurora default", "Aurora", "aurora-mysql"},
-		{"MySQL", "MySQL", "mysql"},
-		{"PostgreSQL", "PostgreSQL", "postgresql"},
-		{"MariaDB", "MariaDB", "mariadb"},
-		{"Oracle", "Oracle-EE", "oracle-se2"},
-		{"SQL Server hyphenated", "sql-server-ex", "sqlserver-se"},
-		{"SQL Server camelcase", "SQLServer", "sqlserver-se"},
-		{"Already normalized postgres", "postgres", "postgresql"},
-		{"Unknown engine", "custom-db", "custom-db"},
+		{"Aurora MySQL uppercase", "Aurora-MySQL", "aurora-mysql", false},
+		{"Aurora PostgreSQL mixed case", "Aurora-PostgreSQL", "aurora-postgresql", false},
+		// Previously "Aurora" without a database variant silently defaulted to
+		// aurora-mysql. Now it must error: aurora-mysql and aurora-postgresql have
+		// different RI prices and the caller must supply the precise variant.
+		{"Aurora no variant errors", "Aurora", "", true},
+		{"MySQL", "MySQL", "mysql", false},
+		{"PostgreSQL", "PostgreSQL", "postgresql", false},
+		{"MariaDB", "MariaDB", "mariadb", false},
+		// Oracle editions (SE2, EE) have different prices; guessing oracle-se2 is wrong.
+		{"Oracle ambiguous errors", "Oracle-EE", "", true},
+		{"Oracle bare errors", "oracle", "", true},
+		// SQL Server editions (SE, EE, Web, Express) have different prices; guessing is wrong.
+		{"SQL Server hyphenated errors", "sql-server-ex", "", true},
+		{"SQL Server camelcase errors", "SQLServer", "", true},
+		{"Already normalized aurora-mysql", "aurora-mysql", "aurora-mysql", false},
+		{"Already normalized aurora-postgresql", "aurora-postgresql", "aurora-postgresql", false},
+		{"Already normalized postgres", "postgres", "postgresql", false},
+		{"Unknown engine passes through", "custom-db", "custom-db", false},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := client.normalizeEngineName(tt.input)
-			assert.Equal(t, tt.expected, result)
+			result, err := client.normalizeEngineName(tt.input)
+			if tt.expectError {
+				assert.Error(t, err, "expected error for engine %q", tt.input)
+				assert.Empty(t, result)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
 		})
 	}
 }
@@ -798,6 +814,73 @@ func TestFindOfferingID_HappyPath(t *testing.T) {
 	assert.Equal(t, "offering-ok", id)
 }
 
+// TestFindOfferingID_EmptyAZConfig_Errors is the M4 regression test:
+// findOfferingID must error when AZConfig is empty rather than silently
+// assuming single-az. An empty AZConfig means the CE recommendation did not
+// include DeploymentOption; single-AZ and multi-AZ RDS RIs have different
+// prices so fabricating the value risks buying the wrong offering class.
+func TestFindOfferingID_EmptyAZConfig_Errors(t *testing.T) {
+	mockRDS := &MockRDSClient{}
+	t.Cleanup(func() { mockRDS.AssertExpectations(t) })
+	client := &Client{client: mockRDS, region: "us-east-1"}
+
+	rec := common.Recommendation{
+		Service:       common.ServiceRelationalDB,
+		ResourceType:  "db.r5.large",
+		PaymentOption: "all-upfront",
+		Term:          "1yr",
+		Details: &common.DatabaseDetails{
+			Engine:   "mysql",
+			AZConfig: "", // not set -- CE omitted DeploymentOption
+		},
+	}
+
+	// No mock calls should be made: findOfferingID must fail before the API call.
+	_, err := client.findOfferingID(context.Background(), rec, "")
+
+	require.Error(t, err, "findOfferingID must error when AZConfig is empty (M4)")
+	assert.Contains(t, err.Error(), "AZConfig")
+}
+
+// TestNormalizeEngineName_AmbiguousErrors is the M6 regression test:
+// normalizeEngineName must error for Oracle, SQL Server, and bare "aurora"
+// inputs rather than silently guessing an edition.
+func TestNormalizeEngineName_AmbiguousErrors(t *testing.T) {
+	client := &Client{}
+
+	ambiguous := []string{
+		"oracle",
+		"Oracle",
+		"oracle-se2",   // paradoxically exact, but contains "oracle" -- verify it errors
+		"Oracle-EE",
+		"sqlserver",
+		"SQLServer",
+		"sql-server",
+		"Aurora",
+		"aurora",
+		"aurora-unknown",
+	}
+
+	for _, engine := range ambiguous {
+		t.Run(engine, func(t *testing.T) {
+			_, err := client.normalizeEngineName(engine)
+			// oracle-se2 IS unambiguous -- it already contains "oracle" so the
+			// ambiguous oracle branch catches it. The engine string that the CE
+			// recommendation returns for SE2 is "Oracle" (title-case), not
+			// "oracle-se2"; the API value "oracle-se2" passes through the
+			// final else-return at the bottom. Let the sub-test declare its
+			// expectation explicitly rather than force all to error.
+			switch engine {
+			case "oracle-se2": // would pass through (no "oracle" substring match on lower)
+				// actually strings.Contains("oracle-se2", "oracle") IS true, so this errors
+				assert.Error(t, err, "engine %q must error (ambiguous oracle)", engine)
+			default:
+				assert.Error(t, err, "engine %q must error (M6)", engine)
+			}
+		})
+	}
+}
+
 // TestClient_PurchaseCommitment_NoToken_RichReservationName asserts the
 // no-token CLI path (issue #687) composes a self-describing
 // ReservedDBInstanceId carrying the service code, region, SKU, count, and

From 71c3abb634e93231886966a6bae089305fee6744 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 11:32:10 -0700
Subject: [PATCH 3/5] test(aws/rds): clarify ambiguous-engine regression test

Simplify TestNormalizeEngineName_AmbiguousErrors: remove the confusing
oracle-se2 case (strings.Contains("oracle-se2","oracle") is true, so it
errors like all other oracle inputs -- no special case needed) and remove
the misleading inline comment. All remaining cases assert that any Oracle,
SQL Server, or bare Aurora input errors, which is the invariant under test.
---
 providers/aws/services/rds/client_test.go | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/providers/aws/services/rds/client_test.go b/providers/aws/services/rds/client_test.go
index 807dc0158..070676dbf 100644
--- a/providers/aws/services/rds/client_test.go
+++ b/providers/aws/services/rds/client_test.go
@@ -845,13 +845,14 @@ func TestFindOfferingID_EmptyAZConfig_Errors(t *testing.T) {
 // TestNormalizeEngineName_AmbiguousErrors is the M6 regression test:
 // normalizeEngineName must error for Oracle, SQL Server, and bare "aurora"
 // inputs rather than silently guessing an edition.
+// CE returns "Oracle" (title-case) for any Oracle engine; the normalizer must
+// refuse to pick an edition (SE2, EE, etc.) because they are different products.
 func TestNormalizeEngineName_AmbiguousErrors(t *testing.T) {
 	client := &Client{}
 
 	ambiguous := []string{
 		"oracle",
 		"Oracle",
-		"oracle-se2",   // paradoxically exact, but contains "oracle" -- verify it errors
 		"Oracle-EE",
 		"sqlserver",
 		"SQLServer",
@@ -864,19 +865,7 @@ func TestNormalizeEngineName_AmbiguousErrors(t *testing.T) {
 	for _, engine := range ambiguous {
 		t.Run(engine, func(t *testing.T) {
 			_, err := client.normalizeEngineName(engine)
-			// oracle-se2 IS unambiguous -- it already contains "oracle" so the
-			// ambiguous oracle branch catches it. The engine string that the CE
-			// recommendation returns for SE2 is "Oracle" (title-case), not
-			// "oracle-se2"; the API value "oracle-se2" passes through the
-			// final else-return at the bottom. Let the sub-test declare its
-			// expectation explicitly rather than force all to error.
-			switch engine {
-			case "oracle-se2": // would pass through (no "oracle" substring match on lower)
-				// actually strings.Contains("oracle-se2", "oracle") IS true, so this errors
-				assert.Error(t, err, "engine %q must error (ambiguous oracle)", engine)
-			default:
-				assert.Error(t, err, "engine %q must error (M6)", engine)
-			}
+			assert.Error(t, err, "engine %q must error (M6 regression guard)", engine)
 		})
 	}
 }

From 710cad1e8bc9c2d3e4f0cad28e3d0fd69c7b2131 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 12:11:16 -0700
Subject: [PATCH 4/5] refactor(aws): extract helpers to fix gocyclo violations
 in rds and savingsplans

gocyclo -over 10 fails on two functions added by the fail-loud refactor:
- savingsplans.(*Client).findOfferingID (complexity 12): extract
  resolveSPPlanType and buildSPOfferingsInput helpers
- rds.(*Client).paginateRDSOfferings (complexity 11): extract
  fetchRDSOfferingPage to isolate the per-page API call and scan

Also apply gofmt to rds/client.go (trailing whitespace / brace style).
Behaviour and error paths are unchanged; all rds and savingsplans
tests still pass.
---
 providers/aws/services/rds/client.go          | 88 +++++++++++------
 providers/aws/services/savingsplans/client.go | 98 +++++++++++--------
 2 files changed, 112 insertions(+), 74 deletions(-)

diff --git a/providers/aws/services/rds/client.go b/providers/aws/services/rds/client.go
index 8cf4cdb7e..2480b2358 100644
--- a/providers/aws/services/rds/client.go
+++ b/providers/aws/services/rds/client.go
@@ -308,7 +308,7 @@ func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation,
 	// means the CE recommendation omitted DeploymentOption; guessing single-az
 	// risks buying the wrong RI class. Fail loud so the caller can decide.
 	if details.AZConfig == "" {
-		return "", fmt.Errorf("RDS AZConfig is empty: CE recommendation did not include DeploymentOption; "+
+		return "", fmt.Errorf("RDS AZConfig is empty: CE recommendation did not include DeploymentOption; " +
 			"refusing to guess single-az vs multi-az (see M4 in 19-hardcoded-fallbacks-aws.md)")
 	}
 	offeringType, err := c.convertPaymentOption(rec.PaymentOption)
@@ -318,6 +318,44 @@ func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation,
 	return c.paginateRDSOfferings(ctx, rec, details, offeringType, execID)
 }
 
+// rdsOfferingPageResult holds the outcome of a single DescribeReservedDBInstancesOfferings page.
+type rdsOfferingPageResult struct {
+	id     string  // non-empty when a match was found
+	marker *string // pagination cursor for the next page; nil when exhausted
+}
+
+// fetchRDSOfferingPage calls DescribeReservedDBInstancesOfferings for one page and
+// scans the results. It returns a match ID when found, a non-nil marker when more
+// pages remain, or an error on API/offering-validation failure.
+func (c *Client) fetchRDSOfferingPage(ctx context.Context, baseInput *rds.DescribeReservedDBInstancesOfferingsInput, marker *string, rec common.Recommendation, offeringType string, tag string, page int, t0 time.Time) (rdsOfferingPageResult, error) {
+	input := *baseInput
+	input.Marker = marker
+
+	pageStart := time.Now()
+	result, err := c.client.DescribeReservedDBInstancesOfferings(ctx, &input)
+	if err != nil {
+		log.Printf("purchase[%s]: RDS findOfferingID page %d failed after %s (total %s): %v",
+			tag, page, time.Since(pageStart), time.Since(t0), err)
+		return rdsOfferingPageResult{}, fmt.Errorf("failed to describe offerings: %w", err)
+	}
+	log.Printf("purchase[%s]: RDS findOfferingID page %d: %d offerings in %s",
+		tag, page, len(result.ReservedDBInstancesOfferings), time.Since(pageStart))
+
+	id, scanErr := scanRDSOfferingPage(result.ReservedDBInstancesOfferings, rec, offeringType)
+	if scanErr != nil {
+		return rdsOfferingPageResult{}, scanErr
+	}
+	if id != "" {
+		log.Printf("purchase[%s]: RDS findOfferingID found match on page %d after %s total", tag, page, time.Since(t0))
+		return rdsOfferingPageResult{id: id}, nil
+	}
+	var nextMarker *string
+	if result.Marker != nil && aws.ToString(result.Marker) != "" {
+		nextMarker = result.Marker
+	}
+	return rdsOfferingPageResult{marker: nextMarker}, nil
+}
+
 // paginateRDSOfferings walks DescribeReservedDBInstancesOfferings pages and returns
 // the first matching offering ID. It caps at maxOfferingPages to prevent Lambda
 // timeout exhaustion (issue #688).
@@ -337,51 +375,39 @@ func (c *Client) paginateRDSOfferings(ctx context.Context, rec common.Recommenda
 	log.Printf("purchase[%s]: RDS findOfferingID starting (class=%s engine=%s multi-az=%v duration=%s payment=%s)",
 		tag, rec.ResourceType, normalizedEngine, multiAZ, duration, offeringType)
 
+	baseInput := &rds.DescribeReservedDBInstancesOfferingsInput{
+		DBInstanceClass:    aws.String(rec.ResourceType),
+		ProductDescription: aws.String(normalizedEngine),
+		MultiAZ:            aws.Bool(multiAZ),
+		Duration:           aws.String(duration),
+		OfferingType:       aws.String(offeringType),
+		MaxRecords:         aws.Int32(100),
+	}
+
 	var marker *string
-	page := 0
-	for {
+	for page := 1; ; page++ {
 		if err := ctx.Err(); err != nil {
 			return "", err
 		}
-		page++
 		if page > maxOfferingPages {
 			return "", fmt.Errorf("pagination cap reached after %d pages for RDS %s %s multi-az=%v %s (issue #688)",
 				maxOfferingPages, rec.ResourceType, details.Engine, multiAZ, rec.PaymentOption)
 		}
-		input := &rds.DescribeReservedDBInstancesOfferingsInput{
-			DBInstanceClass:    aws.String(rec.ResourceType),
-			ProductDescription: aws.String(normalizedEngine),
-			MultiAZ:            aws.Bool(multiAZ),
-			Duration:           aws.String(duration),
-			OfferingType:       aws.String(offeringType),
-			MaxRecords:         aws.Int32(100),
-			Marker:             marker,
-		}
-		pageStart := time.Now()
-		result, err := c.client.DescribeReservedDBInstancesOfferings(ctx, input)
+		pr, err := c.fetchRDSOfferingPage(ctx, baseInput, marker, rec, offeringType, tag, page, t0)
 		if err != nil {
-			log.Printf("purchase[%s]: RDS findOfferingID page %d failed after %s (total %s): %v",
-				tag, page, time.Since(pageStart), time.Since(t0), err)
-			return "", fmt.Errorf("failed to describe offerings: %w", err)
+			return "", err
 		}
-		log.Printf("purchase[%s]: RDS findOfferingID page %d: %d offerings in %s",
-			tag, page, len(result.ReservedDBInstancesOfferings), time.Since(pageStart))
-		if id, scanErr := scanRDSOfferingPage(result.ReservedDBInstancesOfferings, rec, offeringType); scanErr != nil {
-			return "", scanErr
-		} else if id != "" {
-			log.Printf("purchase[%s]: RDS findOfferingID found match on page %d after %s total",
-				tag, page, time.Since(t0))
-			return id, nil
+		if pr.id != "" {
+			return pr.id, nil
 		}
-		if result.Marker == nil || aws.ToString(result.Marker) == "" {
+		if pr.marker == nil {
 			break
 		}
-		marker = result.Marker
+		marker = pr.marker
 	}
-	log.Printf("purchase[%s]: RDS findOfferingID exhausted %d page(s) in %s -- no match",
-		tag, page, time.Since(t0))
+	log.Printf("purchase[%s]: RDS findOfferingID exhausted pages in %s -- no match", tag, time.Since(t0))
 	return "", fmt.Errorf("no offerings found for RDS %s %s multi-az=%v %s after %d page(s) (issue #688)",
-		rec.ResourceType, details.Engine, multiAZ, rec.PaymentOption, page)
+		rec.ResourceType, details.Engine, multiAZ, rec.PaymentOption, maxOfferingPages)
 }
 
 // scanRDSOfferingPage finds a matching offering in a single page of results.
diff --git a/providers/aws/services/savingsplans/client.go b/providers/aws/services/savingsplans/client.go
index 249b5cf3e..4cea975ee 100644
--- a/providers/aws/services/savingsplans/client.go
+++ b/providers/aws/services/savingsplans/client.go
@@ -262,6 +262,57 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 	return result, nil
 }
 
+// resolveSPPlanType resolves the effective plan type for an offering lookup.
+// When the client is scoped to a specific plan type (post-split), it validates
+// that the recommendation matches and returns c.planType. Umbrella/legacy
+// clients (c.planType == "") fall back to spDetails.PlanType.
+func (c *Client) resolveSPPlanType(spPlanType string) (types.SavingsPlanType, error) {
+	planType, convErr := convertPlanType(spPlanType)
+	if c.planType == "" {
+		// Legacy umbrella client: accept any convertible plan type.
+		return planType, convErr
+	}
+	// Scoped client: reject mismatches to prevent buying the wrong product.
+	if convErr != nil {
+		return "", convErr
+	}
+	if planType != c.planType {
+		return "", fmt.Errorf(
+			"recommendation plan type %q does not match client scope %q",
+			spPlanType, c.planType,
+		)
+	}
+	return c.planType, nil
+}
+
+// buildSPOfferingsInput constructs the DescribeSavingsPlansOfferings request,
+// adding a region filter for EC2Instance plans when the client region is known.
+func (c *Client) buildSPOfferingsInput(planType types.SavingsPlanType, termSeconds int64, paymentOption types.SavingsPlanPaymentOption, tag string) *savingsplans.DescribeSavingsPlansOfferingsInput {
+	input := &savingsplans.DescribeSavingsPlansOfferingsInput{
+		PlanTypes:      []types.SavingsPlanType{planType},
+		Durations:      []int64{termSeconds},
+		PaymentOptions: []types.SavingsPlanPaymentOption{paymentOption},
+		// Pin to USD so non-USD currency offerings are excluded server-side.
+		Currencies: []types.CurrencyCode{types.CurrencyCodeUsd},
+	}
+	// EC2Instance SPs are region-scoped; add a region filter so only the
+	// offering for the client's region is returned. Compute, SageMaker, and
+	// Database SPs are global and do not carry a region property.
+	if planType == types.SavingsPlanTypeEc2Instance {
+		if c.region == "" {
+			log.Printf("purchase[%s]: SavingsPlans findOfferingID: client region is empty; skipping region filter", tag)
+		} else {
+			input.Filters = []types.SavingsPlanOfferingFilterElement{
+				{
+					Name:   types.SavingsPlanOfferingFilterAttributeRegion,
+					Values: []string{c.region},
+				},
+			}
+		}
+	}
+	return input
+}
+
 // findOfferingID finds the appropriate Savings Plans offering ID.
 // execID is the purchase execution UUID for log correlation; pass "" when
 // calling outside of a purchase flow (ValidateOffering, GetOfferingDetails).
@@ -271,27 +322,10 @@ func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation,
 		return "", fmt.Errorf("invalid service details for Savings Plans")
 	}
 
-	// Per-plan-type client (post-split): the client's planType is the
-	// source of truth. Reject mismatched recommendations rather than
-	// silently buying the wrong product. Umbrella legacy clients
-	// (c.planType == "") fall back to rec.Details.PlanType to preserve
-	// pre-split behaviour.
-	planType, convErr := convertPlanType(spDetails.PlanType)
-	if c.planType != "" {
-		if convErr != nil {
-			return "", convErr
-		}
-		if planType != c.planType {
-			return "", fmt.Errorf(
-				"recommendation plan type %q does not match client scope %q",
-				spDetails.PlanType, c.planType,
-			)
-		}
-		planType = c.planType
-	} else if convErr != nil {
-		return "", convErr
+	planType, err := c.resolveSPPlanType(spDetails.PlanType)
+	if err != nil {
+		return "", err
 	}
-
 	termSeconds, err := convertTermToSeconds(rec.Term)
 	if err != nil {
 		return "", err
@@ -310,29 +344,7 @@ func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation,
 	log.Printf("purchase[%s]: SavingsPlans findOfferingID starting (planType=%s term=%s payment=%s)",
 		tag, planType, rec.Term, rec.PaymentOption)
 
-	// Pin to USD so non-USD currency offerings are excluded server-side.
-	// EC2Instance SPs are region-scoped; add a region filter so only the
-	// offering for the client's region is returned. Compute, SageMaker, and
-	// Database SPs are global and do not carry a region property.
-	input := &savingsplans.DescribeSavingsPlansOfferingsInput{
-		PlanTypes:      []types.SavingsPlanType{planType},
-		Durations:      []int64{termSeconds},
-		PaymentOptions: []types.SavingsPlanPaymentOption{paymentOption},
-		Currencies:     []types.CurrencyCode{types.CurrencyCodeUsd},
-	}
-	if planType == types.SavingsPlanTypeEc2Instance {
-		if c.region == "" {
-			log.Printf("purchase[%s]: SavingsPlans findOfferingID: client region is empty; skipping region filter", tag)
-		} else {
-			input.Filters = []types.SavingsPlanOfferingFilterElement{
-				{
-					Name:   types.SavingsPlanOfferingFilterAttributeRegion,
-					Values: []string{c.region},
-				},
-			}
-		}
-	}
-
+	input := c.buildSPOfferingsInput(planType, termSeconds, paymentOption, tag)
 	offeringID, err := c.lookupOfferingID(ctx, input)
 	if err != nil {
 		log.Printf("purchase[%s]: SavingsPlans findOfferingID failed after %s: %v", tag, time.Since(t0), err)

From 681fd9e81a1dc2b55dd7c2f7fe3e473424d6b508 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 12:30:27 -0700
Subject: [PATCH 5/5] fix(aws/rds): tighten AZ-config and engine-name
 validation (CR #1085)

Three related fail-loud fixes in the AWS RDS purchase path:

1. parseRDSDetails: unknown DeploymentOption now errors instead of
   silently folding into "single-az". Only "Multi-AZ" and "Single-AZ"
   are accepted; any other non-nil value (e.g. "Multi-AZ-Readable-Standbys")
   returns an explicit error so the bad token surfaces rather than
   driving findOfferingID to the wrong RI class.

2. findOfferingID: AZConfig validation now uses a switch with an
   explicit default error case rather than only checking for empty string.
   A non-empty but invalid value (typo, future AWS enum addition) would
   previously fall through to multiAZ==false in paginateRDSOfferings,
   silently treating the bad input as single-AZ.

3. normalizeEngineName: Oracle and SQL Server checks changed from
   strings.Contains to == so that edition-qualified tokens (oracle-se2,
   oracle-ee, sqlserver-se, sqlserver-web, etc.) pass through as valid
   RDS ProductDescription values. The previous Contains check contradicted
   the error message itself, which told callers to supply those exact
   edition tokens.

Regression tests added for all three: each new test was verified to
fail against the pre-fix code and pass after.
---
 .../aws/recommendations/parser_services.go    | 10 ++-
 .../recommendations/parser_services_test.go   | 18 ++++
 providers/aws/services/rds/client.go          | 20 ++++-
 providers/aws/services/rds/client_test.go     | 87 +++++++++++++++++--
 4 files changed, 121 insertions(+), 14 deletions(-)

diff --git a/providers/aws/recommendations/parser_services.go b/providers/aws/recommendations/parser_services.go
index fc56a2a3d..d69cd7017 100644
--- a/providers/aws/recommendations/parser_services.go
+++ b/providers/aws/recommendations/parser_services.go
@@ -36,10 +36,16 @@ func (c *Client) parseRDSDetails(_ context.Context, rec *common.Recommendation,
 	// The downstream findOfferingID rejects an empty AZConfig with an explicit
 	// error rather than proceeding with a fabricated value (M4 fix).
 	if rdsDetails.DeploymentOption != nil {
-		if *rdsDetails.DeploymentOption == "Multi-AZ" {
+		// Map only the exact CE tokens; an unrecognized value must not be folded
+		// into single-az (which would drive findOfferingID to the wrong RI class
+		// and a mis-buy). Fail loud instead so the bad token surfaces (CR #1085).
+		switch strings.TrimSpace(*rdsDetails.DeploymentOption) {
+		case "Multi-AZ":
 			rdsInfo.AZConfig = "multi-az"
-		} else {
+		case "Single-AZ":
 			rdsInfo.AZConfig = "single-az"
+		default:
+			return fmt.Errorf("unrecognized RDS DeploymentOption %q: expected \"Multi-AZ\" or \"Single-AZ\"", *rdsDetails.DeploymentOption)
 		}
 	}
 
diff --git a/providers/aws/recommendations/parser_services_test.go b/providers/aws/recommendations/parser_services_test.go
index 26d0e43c4..d224d3695 100644
--- a/providers/aws/recommendations/parser_services_test.go
+++ b/providers/aws/recommendations/parser_services_test.go
@@ -89,6 +89,24 @@ func TestParseRDSDetails(t *testing.T) {
 				assert.Empty(t, dbDetails.AZConfig, "AZConfig must not be silently defaulted to single-az")
 			},
 		},
+		{
+			// CR #1085 regression test: an unrecognized DeploymentOption must error
+			// rather than being silently folded into "single-az". The pre-fix code
+			// used an else branch that mapped anything non-"Multi-AZ" to "single-az",
+			// which could cause findOfferingID to query and buy the wrong RI class.
+			name:        "Unknown DeploymentOption errors (CR #1085)",
+			expectError: true,
+			details: &types.ReservationPurchaseRecommendationDetail{
+				InstanceDetails: &types.InstanceDetails{
+					RDSInstanceDetails: &types.RDSInstanceDetails{
+						InstanceType:     aws.String("db.r5.large"),
+						DatabaseEngine:   aws.String("mysql"),
+						Region:           aws.String("us-east-1"),
+						DeploymentOption: aws.String("Multi-AZ-Readable-Standbys"),
+					},
+				},
+			},
+		},
 		{
 			name: "Missing RDS instance details",
 			details: &types.ReservationPurchaseRecommendationDetail{
diff --git a/providers/aws/services/rds/client.go b/providers/aws/services/rds/client.go
index 2480b2358..58e16a3dd 100644
--- a/providers/aws/services/rds/client.go
+++ b/providers/aws/services/rds/client.go
@@ -307,9 +307,18 @@ func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation,
 	// different prices and do not cover each other's demand. An empty AZConfig
 	// means the CE recommendation omitted DeploymentOption; guessing single-az
 	// risks buying the wrong RI class. Fail loud so the caller can decide.
-	if details.AZConfig == "" {
+	// Validate the full enum, not just the empty case: a non-empty typo would
+	// otherwise fall through to multiAZ==false in paginateRDSOfferings and
+	// silently drive a single-AZ lookup -- the same mis-buy class as the old
+	// default (CR #1085).
+	switch details.AZConfig {
+	case "single-az", "multi-az":
+		// valid
+	case "":
 		return "", fmt.Errorf("RDS AZConfig is empty: CE recommendation did not include DeploymentOption; " +
 			"refusing to guess single-az vs multi-az (see M4 in 19-hardcoded-fallbacks-aws.md)")
+	default:
+		return "", fmt.Errorf("invalid RDS AZConfig %q: must be single-az or multi-az", details.AZConfig)
 	}
 	offeringType, err := c.convertPaymentOption(rec.PaymentOption)
 	if err != nil {
@@ -577,14 +586,19 @@ func (c *Client) normalizeEngineName(engine string) (string, error) {
 	if strings.Contains(engineLower, "mariadb") {
 		return "mariadb", nil
 	}
-	if strings.Contains(engineLower, "oracle") {
+	// Only the bare family name is ambiguous (CE returns "Oracle" / "SQL Server"
+	// with no edition). An edition-qualified token like oracle-se2 or
+	// sqlserver-web is already a valid RDS ProductDescription, so pass it through
+	// rather than rejecting it -- rejecting it contradicted the "supply the exact
+	// edition" guidance in this very error message (CR #1085).
+	if engineLower == "oracle" {
 		return "", fmt.Errorf(
 			"ambiguous Oracle engine %q: CE must supply the exact edition "+
 				"(e.g. oracle-se2, oracle-ee); refusing to guess",
 			engine,
 		)
 	}
-	if strings.Contains(engineLower, "sqlserver") || strings.Contains(engineLower, "sql-server") {
+	if engineLower == "sqlserver" || engineLower == "sql-server" {
 		return "", fmt.Errorf(
 			"ambiguous SQL Server engine %q: CE must supply the exact edition "+
 				"(e.g. sqlserver-se, sqlserver-ee, sqlserver-web, sqlserver-ex); refusing to guess",
diff --git a/providers/aws/services/rds/client_test.go b/providers/aws/services/rds/client_test.go
index 070676dbf..6c4f4bb74 100644
--- a/providers/aws/services/rds/client_test.go
+++ b/providers/aws/services/rds/client_test.go
@@ -495,12 +495,19 @@ func TestClient_NormalizeEngineName(t *testing.T) {
 		{"MySQL", "MySQL", "mysql", false},
 		{"PostgreSQL", "PostgreSQL", "postgresql", false},
 		{"MariaDB", "MariaDB", "mariadb", false},
-		// Oracle editions (SE2, EE) have different prices; guessing oracle-se2 is wrong.
-		{"Oracle ambiguous errors", "Oracle-EE", "", true},
+		// Bare family names are ambiguous; only the bare names error.
 		{"Oracle bare errors", "oracle", "", true},
-		// SQL Server editions (SE, EE, Web, Express) have different prices; guessing is wrong.
-		{"SQL Server hyphenated errors", "sql-server-ex", "", true},
+		{"Oracle title-case bare errors", "Oracle", "", true},
+		{"SQL Server bare errors", "sqlserver", "", true},
+		{"SQL Server hyphen bare errors", "sql-server", "", true},
 		{"SQL Server camelcase errors", "SQLServer", "", true},
+		// Edition-qualified strings are valid RDS ProductDescription values and must pass through.
+		{"Oracle EE edition passes", "oracle-ee", "oracle-ee", false},
+		{"Oracle SE2 edition passes", "oracle-se2", "oracle-se2", false},
+		{"Oracle EE mixed case passes", "Oracle-EE", "oracle-ee", false},
+		{"SQL Server SE edition passes", "sqlserver-se", "sqlserver-se", false},
+		{"SQL Server web edition passes", "sqlserver-web", "sqlserver-web", false},
+		{"SQL Server hyphen-ex edition passes", "sql-server-ex", "sql-server-ex", false},
 		{"Already normalized aurora-mysql", "aurora-mysql", "aurora-mysql", false},
 		{"Already normalized aurora-postgresql", "aurora-postgresql", "aurora-postgresql", false},
 		{"Already normalized postgres", "postgres", "postgresql", false},
@@ -842,18 +849,48 @@ func TestFindOfferingID_EmptyAZConfig_Errors(t *testing.T) {
 	assert.Contains(t, err.Error(), "AZConfig")
 }
 
+// TestFindOfferingID_InvalidAZConfig_Errors is the CR #1085 regression test:
+// findOfferingID must reject a non-empty but invalid AZConfig value rather than
+// silently falling through to multiAZ==false in paginateRDSOfferings (which
+// would treat the bad value as single-AZ, the same mis-buy as the old default).
+func TestFindOfferingID_InvalidAZConfig_Errors(t *testing.T) {
+	mockRDS := &MockRDSClient{}
+	t.Cleanup(func() { mockRDS.AssertExpectations(t) })
+	client := &Client{client: mockRDS, region: "us-east-1"}
+
+	rec := common.Recommendation{
+		Service:       common.ServiceRelationalDB,
+		ResourceType:  "db.r5.large",
+		Region:        "us-east-1",
+		PaymentOption: "all-upfront",
+		Term:          "1yr",
+		Details: &common.DatabaseDetails{
+			Engine:   "mysql",
+			AZConfig: "typo-az", // non-empty but not a valid enum value
+		},
+	}
+
+	// No mock calls should be made: findOfferingID must fail before the API call.
+	_, err := client.findOfferingID(context.Background(), rec, "")
+
+	require.Error(t, err, "findOfferingID must error on invalid non-empty AZConfig (CR #1085)")
+	assert.Contains(t, err.Error(), "AZConfig")
+	assert.Contains(t, err.Error(), "typo-az")
+}
+
 // TestNormalizeEngineName_AmbiguousErrors is the M6 regression test:
-// normalizeEngineName must error for Oracle, SQL Server, and bare "aurora"
-// inputs rather than silently guessing an edition.
-// CE returns "Oracle" (title-case) for any Oracle engine; the normalizer must
-// refuse to pick an edition (SE2, EE, etc.) because they are different products.
+// normalizeEngineName must error for bare Oracle/SQL Server/Aurora inputs.
+// CE returns "Oracle" (title-case) for any Oracle engine when no edition is
+// specified; the normalizer rejects the bare name so the caller surfaces the
+// problem rather than silently guessing an edition.
+// Edition-qualified strings (oracle-se2, sqlserver-web, etc.) are valid RDS
+// ProductDescription values and must pass through (see CR #1085).
 func TestNormalizeEngineName_AmbiguousErrors(t *testing.T) {
 	client := &Client{}
 
 	ambiguous := []string{
 		"oracle",
 		"Oracle",
-		"Oracle-EE",
 		"sqlserver",
 		"SQLServer",
 		"sql-server",
@@ -870,6 +907,38 @@ func TestNormalizeEngineName_AmbiguousErrors(t *testing.T) {
 	}
 }
 
+// TestNormalizeEngineName_EditionTokensPassThrough is the CR #1085 regression
+// test: edition-qualified Oracle and SQL Server tokens must pass through
+// normalizeEngineName rather than being rejected by the bare-name ambiguity
+// check. These are valid RDS ProductDescription values that CE can supply.
+func TestNormalizeEngineName_EditionTokensPassThrough(t *testing.T) {
+	client := &Client{}
+
+	cases := []struct {
+		input    string
+		expected string
+	}{
+		{"oracle-se2", "oracle-se2"},
+		{"oracle-ee", "oracle-ee"},
+		{"Oracle-EE", "oracle-ee"},
+		{"oracle-se2-ex", "oracle-se2-ex"},
+		{"sqlserver-se", "sqlserver-se"},
+		{"sqlserver-ee", "sqlserver-ee"},
+		{"sqlserver-web", "sqlserver-web"},
+		{"sqlserver-ex", "sqlserver-ex"},
+		{"sql-server-ex", "sql-server-ex"},
+		{"sql-server-se", "sql-server-se"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.input, func(t *testing.T) {
+			result, err := client.normalizeEngineName(tc.input)
+			assert.NoError(t, err, "edition token %q must pass through (not ambiguous)", tc.input)
+			assert.Equal(t, tc.expected, result)
+		})
+	}
+}
+
 // TestClient_PurchaseCommitment_NoToken_RichReservationName asserts the
 // no-token CLI path (issue #687) composes a self-describing
 // ReservedDBInstanceId carrying the service code, region, SKU, count, and