[Security] Sanitize user inputs to prevent OS command injection via SSM#469
Merged
[Security] Sanitize user inputs to prevent OS command injection via SSM#469
Conversation
gmarciani
reviewed
Apr 17, 2026
| region = request.args.get("region") | ||
| body = request.json | ||
|
|
||
| for k, v in body.items(): |
Collaborator
There was a problem hiding this comment.
What about using shlex.quote() to escape the content of sacct_args before the interpolation into the SSM command?
I guess in that case we do not even need to specify specific patterns to reject.
I thought about it here for the JSON, but I think it could be applied to whatever command interpolation, so also for the usernmae in the other SSM command.
Collaborator
There was a problem hiding this comment.
FYI we already use shlex for the same reasons in pcluster cli
…SM (P404791535) Add shell metacharacter validation for user-controlled inputs that are interpolated into shell commands executed via SSM SendCommand on cluster head nodes. Affected endpoints: /sacct, /scontrol_job, /cancel_job, /get_dcv_session, /queue_status. Changes: - Add has_no_shell_metacharacters validator that rejects ;|&$`()><'"\\\n\r\0 - Apply validator to user and job_id fields in Marshmallow schemas - Add inline validation for sacct POST JSON body keys and values - Add 24 test cases covering legitimate inputs and injection attempts
Wrap user-controlled inputs with shlex.quote before interpolating them into shell commands executed via SSM SendCommand on cluster head nodes. This is the same approach used in aws-parallelcluster CLI and is the POSIX-standard way to safely pass untrusted strings to a shell.
Verify that ssm_command safely escapes user and run_command arguments by asserting the constructed shell string parses back into the expected tokens via shlex.split. A malicious input containing shell metacharacters must round-trip as a single literal token; any regression that drops shlex.quote would cause the shell parser to split the input and fail the assertion.
hehe7318
force-pushed
the
wip/fix-command-injection
branch
from
8d797ef to
79a3ca8
Compare
gmarciani
reviewed
Apr 21, 2026
| ssm = boto3.client("ssm") | ||
|
|
||
| command = f"runuser -l {user} -c '{dcv_command} {session_directory}'" | ||
| inner_command = f"{dcv_command} {shlex.quote(session_directory)}" |
Collaborator
There was a problem hiding this comment.
why not shlexing the dcv_command as well?
Contributor
Author
There was a problem hiding this comment.
See
It's hardcoded, not user input.
gmarciani
approved these changes
Apr 21, 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
2 participants
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.
Description
Use
shlex.quoteto prevent OS command injection via SSM.Affected endpoints:
/sacct,/scontrol_job,/cancel_job,/get_dcv_session,/queue_status.Changes
user-controlledinputs with shlex.quote before interpolating them into shell commands executed via SSM SendCommand on cluster head nodes. This is the same approach used in aws-parallelcluster CLI and is the POSIX-standard way to safely pass untrusted strings to a shell.How Has This Been Tested?
PR Quality Checklist
react-i18nextlibrary (useTranslation hook and/or Trans component), see an example herenpm run buildbuilds without any errorIn order to increase the likelihood of your contribution being accepted, please make sure you have read both the Contributing Guidelines and the Project Guidelines
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.