diff --git a/internal/purchase/errnotes.go b/internal/purchase/errnotes.go index 2a1657b17..561045750 100644 --- a/internal/purchase/errnotes.go +++ b/internal/purchase/errnotes.go @@ -12,24 +12,26 @@ import ( // notifications.go, plan_step_attach.go, stranded_redrive.go, step_cap.go, // due_age_cap.go, claim.go, ladder_execution_recheck.go and the api retry // handler's email reasons). Anything else in a stored error is treated as -// provider text. Each pattern is anchored and closed: no pattern matches free text or a newline. +// provider text. Each pattern is anchored and closed: every variable slot admits only +// its writer's alphabet, so no slot matches whitespace, a line break or a control +// or format character. // The em dash in the reaper note is written as \x{2014}. var serverNotePatterns = []string{ - `history_write_failed: commitment [^;\s]+ purchased but its history record failed to save`, + `history_write_failed: commitment ` + commitmentIDPattern + ` purchased but its history record failed to save`, `recovered after reap; provider purchase succeeded`, `reaped after \d+m in [a-z_]+ state \x{2014} executor did not complete(?:; safe to retry)?`, `execution was approved but its purchase run was interrupted before completing and never finalized; failed by the recovery sweep so it is not silently stuck \(issue #632\)\. (?:Verify on the cloud provider that no commitment was created, then Retry\.|Retry is refused for this purchase because a repeat could buy a second commitment\. Verify on the cloud provider whether the commitment was created; if it was not, start a new purchase\.)`, `refusing to execute retry attempt \d+: (?:` + redriveReasonPattern + `)`, `stale Azure recommendation pricing; refresh recommendations and submit a new purchase(?:: the latest collection state is missing, so Azure prices cannot be confirmed current)?`, - `(?:execution [^:\s]+: )?plan step has no recommendations attached, so it cannot buy anything(?: \(platform#631: plan steps cannot yet carry recommendations\))?`, + `(?:execution ` + uuidPattern + `: )?plan step has no recommendations attached, so it cannot buy anything(?: \(platform#631: plan steps cannot yet carry recommendations\))?`, `plan step refused by commitment cap(?:: plan has no max_step_commitment set \(auto-purchase of steps with recommendations is refused until one is set\)|: step total commitment \d+\.\d{2} exceeds max_step_commitment \d+\.\d{2})?`, `scheduled purchase skipped: it was due too long ago(?:: it was due at \d{4}-\d{2}-\d{2}T[0-9:.]+Z, [0-9a-z.]+ ago, which is more than the [0-9a-z.]+ limit \(SCHEDULED_PURCHASE_MAX_OVERDUE\); nothing was submitted, retry it if the purchase is still wanted)?`, `purchase refused after claim: the claimed execution no longer passes the authorization checks`, `refusing to execute ladder purchase: (?:global config unavailable|ladder planning is switched off|ladder execution is switched off|its tranche status is unknown|its tranche is [a-z_]+|its ladder config no longer exists|its ladder config is disabled|ladder config mode is "[a-z_]*", not auto_approve|its ladder run no longer exists|its ladder run was canceled)`, `scheduled plan step reached its scheduled date without recommendations; no purchase was submitted`, - `no current recommendation matched plan [^;\s]+ step \d+ by \d{4}-\d{2}-\d{2} \(\d+-day grace after the plan date\)(?:; ` + skippedNotePattern + `)?`, + `no current recommendation matched plan ` + uuidPattern + ` step \d+ by \d{4}-\d{2}-\d{2} \(\d+-day grace after the plan date\)(?:; ` + skippedNotePattern + `)?`, `target already met: target \d+, committed \d+(?:; ` + skippedNotePattern + `)?; \d+ failed row\(s\) of unknown outcome counted as committed`, - `selected recommendations do not resolve to a single cloud account: (?:\d+ selected recommendation\(s\) target \d+ cloud accounts \(` + idListPattern + `\); submit one purchase per account|\d+ selected recommendation\(s\) carry no cloud_account_id while \d+ target account ` + idPattern + `)`, + `selected recommendations do not resolve to a single cloud account: (?:\d+ selected recommendation\(s\) target \d+ cloud accounts \(` + uuidPattern + `(?:, ` + uuidPattern + `)*\); submit one purchase per account|\d+ selected recommendation\(s\) carry no cloud_account_id while \d+ target account ` + uuidPattern + `)`, `purchase was not successful`, regexp.QuoteMeta(skippedAccountRetryNote), regexp.QuoteMeta(skippedAccountNoRetryNote), @@ -37,7 +39,7 @@ var serverNotePatterns = []string{ regexp.QuoteMeta(redshiftHorizonRefusal), regexp.QuoteMeta(redshiftNoStampRefusal), regexp.QuoteMeta(redshiftRecentBuyRefusal), - `EC2 recommendation for \S+ has zonal scope but no AvailabilityZone: refusing to buy a zonal RI in an arbitrary AZ; re-fetch the recommendation`, + `EC2 recommendation for ` + resourceTypePattern + ` has zonal scope but no AvailabilityZone: refusing to buy a zonal RI in an arbitrary AZ; re-fetch the recommendation`, `zonal RI offering needs an availability zone; re-fetch the recommendation`, `email notifier not configured for this deployment`, `no notification email set in Settings \x{2192} General(?: and no account contact emails configured)?`, @@ -55,7 +57,7 @@ var bareNotePatterns = []string{ // The operator's stale-exchange resolve note (api/handler_ri_exchange_resolve.go // note()): fixed fields in a fixed order, then the admin's own reason as a Go // quoted string, which is text typed on purpose and closed by its quotes. - `operator:(?:completed|failed): account="\d{12}" region="[a-z0-9-]{1,32}" checked_at=\d{4}-\d{2}-\d{2}T[0-9:.]+(?:Z|[+-]\d{2}:\d{2})(?: ack=true)?(?: exchange_id="[A-Za-z0-9-]{1,100}")? reason="(?:[^"\\\n]|\\.)*"`, + `operator:(?:completed|failed): account="\d{12}" region="[a-z0-9-]{1,32}" checked_at=\d{4}-\d{2}-\d{2}T[0-9:.]+(?:Z|[+-]\d{2}:\d{2})(?: ack=true)?(?: exchange_id="[A-Za-z0-9-]{1,100}")? reason="` + quotedTextPattern + `"`, `config load failed`, `exchange record has no region; cannot execute safely`, `(?:daily|per-exchange) spending cap is not configured \(RIExchange(?:MaxDaily|MaxPerExchange)USD is 0\)`, @@ -81,10 +83,18 @@ func categoryNotePatterns() []string { const ( // amountPattern is a decimal amount; shortValuePattern is a short value with no // whitespace, quotes or newline. Anything wider falls back to the fixed head. - amountPattern = `\d+(?:\.\d+)?` - shortValuePattern = `[A-Za-z0-9._+-]{1,32}` - idPattern = `[^;\s,()]+` - idListPattern = idPattern + `(?:, ` + idPattern + `)*` + amountPattern = `\d+(?:\.\d+)?` + shortValuePattern = `[A-Za-z0-9._+-]{1,32}` + // uuidPattern is a Postgres UUID as it renders (plan, execution and cloud + // account IDs). commitmentIDPattern covers provider commitment IDs and ARNs; + // identityKeyPattern a RecIdentityKey with "/" separators (skippedNote). + uuidPattern = `[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}` + commitmentIDPattern = `[A-Za-z0-9._:/-]{1,256}` + identityKeyPattern = `[A-Za-z0-9._:/+-]{1,256}` + idListPattern = identityKeyPattern + `(?:, ` + identityKeyPattern + `)*` + // quotedTextPattern is the body of a Go %q string: printable characters + // other than '"' and '\\', ASCII space, or an escape. + quotedTextPattern = `(?:[^"\\\p{C}\p{Z}]| |\\[ -~])*` skippedNotePattern = `\d+ recommendation\(s\) without a frozen baseline skipped \(under-buy\): ` + idListPattern ) @@ -109,7 +119,7 @@ var exchangeNoteHeads = []string{ } var redriveReasonPattern = regexp.QuoteMeta(redriveReasonAWSNoGuard) + `|` + regexp.QuoteMeta(redriveReasonAzureSavingsPlan) + - `|provider "[^"]*" is not known to reject a duplicate purchase, so re-driving this could buy a second commitment` + `|provider "[A-Za-z0-9._+-]{0,32}" is not known to reject a duplicate purchase, so re-driving this could buy a second commitment` // resourceTypePattern is the ": " head the executor's per-rec join // adds (execution.go: fmt.Sprintf("%s: %v", rec.ResourceType, err)). The head is diff --git a/internal/purchase/errnotes_test.go b/internal/purchase/errnotes_test.go index 7898eddf2..562f9cf2e 100644 --- a/internal/purchase/errnotes_test.go +++ b/internal/purchase/errnotes_test.go @@ -44,11 +44,11 @@ func TestSanitizeStoredError_AllowlistedNotesPassThroughVerbatim(t *testing.T) { "refusing to execute ladder purchase: global config unreadable", "scheduled plan step reached its scheduled date without recommendations; no purchase was submitted", "no current recommendation matched plan 00000000-0000-0000-0000-000000000000 step 2 by 2026-01-02 (3-day grace after the plan date)", - "no current recommendation matched plan p step 2 by 2026-01-02 (3-day grace after the plan date); " + skippedNote([]string{"a\x1fb", "c"}), + "no current recommendation matched plan 00000000-0000-0000-0000-000000000000 step 2 by 2026-01-02 (3-day grace after the plan date); " + skippedNote([]string{"a\x1fb", "c"}), "target already met: target 10, committed 12; 0 failed row(s) of unknown outcome counted as committed", "target already met: target 10, committed 12; " + skippedNote([]string{"a"}) + "; 1 failed row(s) of unknown outcome counted as committed", - fmt.Errorf("%w: 2 selected recommendation(s) target 2 cloud accounts (a, b); submit one purchase per account", errAmbiguousAccountScope).Error(), - fmt.Errorf("%w: 1 selected recommendation(s) carry no cloud_account_id while 2 target account a", errAmbiguousAccountScope).Error(), + fmt.Errorf("%w: 2 selected recommendation(s) target 2 cloud accounts (00000000-0000-0000-0000-00000000000a, 00000000-0000-0000-0000-00000000000b); submit one purchase per account", errAmbiguousAccountScope).Error(), + fmt.Errorf("%w: 1 selected recommendation(s) carry no cloud_account_id while 2 target account 00000000-0000-0000-0000-00000000000a", errAmbiguousAccountScope).Error(), "purchase was not successful", skippedAccountRetryNote, skippedAccountNoRetryNote, @@ -219,3 +219,45 @@ func TestSanitizeStoredError_RedshiftGateRefusalsAreClosed(t *testing.T) { } } } + +// Every variable slot in a closed note admits only its writer's alphabet, so +// whitespace, a line break or a control or format character never passes +// verbatim, whichever writer later fills the slot. +func TestSanitizeStoredError_SlotsRejectWhitespaceAndControls(t *testing.T) { + const id = "00000000-0000-0000-0000-000000000000" + resolve := `operator:completed: account="` + "1234" + "56789012" + `" region="us-east-1" checked_at=2026-01-02T03:04:05Z reason="%s"` + slots := []string{ + "history_write_failed: commitment %s purchased but its history record failed to save", + "execution %s: " + ErrPlanStepNoRecommendations.Error(), + "no current recommendation matched plan %s step 2 by 2026-01-02 (3-day grace after the plan date)", + "no current recommendation matched plan " + id + " step 2 by 2026-01-02 (3-day grace after the plan date); 1 recommendation(s) without a frozen baseline skipped (under-buy): %s", + errAmbiguousAccountScope.Error() + ": 2 selected recommendation(s) target 2 cloud accounts (%s, " + id + "); submit one purchase per account", + errAmbiguousAccountScope.Error() + ": 1 selected recommendation(s) carry no cloud_account_id while 2 target account %s", + "EC2 recommendation for %s has zonal scope but no AvailabilityZone: refusing to buy a zonal RI in an arbitrary AZ; re-fetch the recommendation", + `refusing to execute retry attempt 1: provider "%s" is not known to reject a duplicate purchase, so re-driving this could buy a second commitment`, + resolve, + } + for _, bad := range []string{"a b", "a\nb", "a\rb", "a\tb", "a\vb", "a\fb", "a\u0085b", "a\u00a0b", "a\u2028b", "a\u200bb", "a\x00b"} { + for _, slot := range slots { + if slot == resolve && bad == "a b" { + continue // %q keeps an ASCII space; the admin's reason may have one + } + in := fmt.Sprintf(slot, bad) + assert.NotEqual(t, in, SanitizeStoredError(in), "%q", in) + } + } + // The real alphabets still pass verbatim. + for _, in := range []string{ + fmt.Sprintf(slots[0], "arn:aws:ec2:us-east-1:reserved/ri-0a1b.c_2"), + fmt.Sprintf(slots[1], id), + fmt.Sprintf(slots[2], id), + fmt.Sprintf(slots[3], id+"/ec2/us-east-1/m5.large//1/no-upfront//Linux/UNIX/default"), + fmt.Sprintf(slots[4], id), + fmt.Sprintf(slots[5], id), + fmt.Sprintf(slots[6], "m5.large"), + fmt.Sprintf(slots[7], "oracle-cloud"), + fmt.Sprintf(resolve, `done, see ticket \"7\"\n`), + } { + assert.Equal(t, in, SanitizeStoredError(in), in) + } +}