Repository navigation
fix(billing-cost-management-mcp-server): let botocore reject a non-string billing period start - #4992
Merged
Merged
Conversation
…ring billing period start get-billing-group-cost-report checks InclusiveStartBillingPeriod for a future month before calling the service. A non-empty non-string value parsed from billing_period_range (for example 202701, an object or true) reached re.fullmatch and raised TypeError, so the caller got unknown_typeerror / do_not_retry and no hint about the bad field. _is_future_billing_period now returns False for any non-string value, as its docstring already promised for malformed input. botocore then rejects the request before sending it with a ParamValidationError that names BillingPeriodRange.InclusiveStartBillingPeriod and its type, returned as validation_error / modify_request. Tests: the period check returns False for int, dict, list and bytes, and get_billing_group_cost_report with an int, object or boolean start returns the ParamValidationError without sending a request.
…e for procurement preference list tests The list tests used the Get-shaped sample_preference fixture, which carries EinvoiceDeliveryPreference, a field ListProcurementPortalPreferences summaries never return. Add a sample_preference_summary fixture built from ProcurementPortalPreferenceSummary members (plus the shared secret, to keep exercising the defensive list redaction), switch the list and list-routing tests to it, and check the fixture against the botocore model. Get tests keep sample_preference.
finklen-amazon
requested review from
a team,
Comusus,
Dinkie,
Rahul-1404,
anniewn,
bhatia-di,
callnmm,
chittev,
johnwangwyx,
junaidgirkar,
mattkelsey,
shsrams,
somsubhro,
wangyuhere and
yasmeenh11
as code owners
November 2, 2026 19:51
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4992 +/- ##
=======================================
Coverage 93.79% 93.79%
=======================================
Files 1080 1080
Lines 94712 94714 +2
Branches 15442 15443 +1
=======================================
+ Hits 88833 88835 +2
Misses 3505 3505
Partials 2374 2374 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
yasmeenh11
approved these changes
Nov 3, 2026
junaidgirkar
approved these changes
Nov 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a regression from #4939.
Summary
Changes
get-billing-group-cost-report: the future-month check added in fix(billing-cost-management-mcp-server): reject or flag future Billing Conductor billing periods #4939 ranre.fullmatchonInclusiveStartBillingPeriodfrom the caller'sbilling_period_rangeJSON. A non-empty non-string value (for example202701, an object, ortrue) raisedTypeErrorbefore any request; empty ones such as0or{}already fell through to botocore._is_future_billing_periodnow returnsFalsefor any non-string value, as its docstring already said for malformed input, so botocore rejects the request with its usualParamValidationError.sample_preferencefixture, which carriesEinvoiceDeliveryPreference, a field list summaries never return. They now use asample_preference_summaryfixture built fromProcurementPortalPreferenceSummarymembers, checked against the botocore model. Get tests are unchanged. Test-only.No tool parameter, description or success response changes.
User experience
get-billing-group-cost-reportwithbilling_period_range={"InclusiveStartBillingPeriod": 202701, "ExclusiveEndBillingPeriod": "2026-11"}:error_type: unknown_typeerror,retry_strategy: do_not_retry,message: "expected string or bytes-like object".error_type: validation_error,boto_error_type: ParamValidationError,retry_strategy: modify_request, and a message that namesBillingPeriodRange.InclusiveStartBillingPeriodand its type. No request is sent.String starts behave as before: a future month is still refused locally, and current, past and malformed strings are sent to the service.
Testing
_is_future_billing_periodreturnsFalsefor int, dict, list and bytes, andget_billing_group_cost_reportwith an int, object or boolean start returns theParamValidationErrorfrom a real botocore client, with abefore-sendhook proving that nothing is sent. 7 of the 15 tests selected fail onmain(the 7 new cases); the other 8 are existing regression guards. The fixture check is a regression guard and passes onmain.ruff check,ruff format --checkandpyrightare clean.main. The 4 failures onmainare the int and object start cases. The controls (a string start reaches the service; a future string start is still refused locally) and the procurement-preferences list shape checks pass on both.Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change? (Y/N) N
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.