Conversation
… with excessive size value Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds strict validation for the EC2 RunInstances MinCount parameter to prevent excessive slice allocations by enforcing positive integer bounds and a maximum batch size before invoking RunInstances. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider making
maxRunInstancesBatchCounteither configurable or at least documenting why1000was chosen so future maintainers understand the rationale for this limit. - The new
InvalidParameterValueresponses forMinCountshould be checked against existing EC2 error conventions in this service (e.g., status code and message shape) to ensure they are consistent with how other parameter validation errors are reported. - You now strictly validate
MinCountbut still ignoreMaxCount; it may be worth validatingMaxCountin a similar way (or explicitly noting why it is not used) to avoid inconsistent behavior and future confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider making `maxRunInstancesBatchCount` either configurable or at least documenting why `1000` was chosen so future maintainers understand the rationale for this limit.
- The new `InvalidParameterValue` responses for `MinCount` should be checked against existing EC2 error conventions in this service (e.g., status code and message shape) to ensure they are consistent with how other parameter validation errors are reported.
- You now strictly validate `MinCount` but still ignore `MaxCount`; it may be worth validating `MaxCount` in a similar way (or explicitly noting why it is not used) to avoid inconsistent behavior and future confusion.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- Document maxRunInstancesBatchCount rationale (matches AWS EC2 default) - Add MaxCount validation with same checks as MinCount
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.
Potential fix for https://github.com/skyoo2003/devcloud/security/code-scanning/20
Add strict bounds validation for
MinCountininternal/services/ec2/provider.gowithinhandleRunInstances:count = 1.MinCountas today, but reject invalid/non-positive values with aBadRequest-style EC2 XML error.counttop.store.RunInstances(...).This is the best single fix because it addresses the tainted input at the trust boundary (request parsing), preserves existing functionality for valid requests, and prevents excessive allocations at the sink in
RunInstanceswithout requiring broader architectural changes.Suggested fixes powered by Copilot Autofix. Review carefully before merging.