Skip to content

Avoid passing ECS session token in plugin argv#10297

Open
NguyenCong2k wants to merge 1 commit intoaws:developfrom
NguyenCong2k:fix-ecs-session-token-argv
Open

Avoid passing ECS session token in plugin argv#10297
NguyenCong2k wants to merge 1 commit intoaws:developfrom
NguyenCong2k:fix-ecs-session-token-argv

Conversation

@NguyenCong2k
Copy link
Copy Markdown

Summary

  • pass the ECS Exec session response to session-manager-plugin through AWS_SSM_START_SESSION_RESPONSE when the installed plugin supports it
  • keep the existing argv fallback for older plugin versions
  • add unit coverage for the environment-variable path

Test

python -m pytest tests\unit\customizations\test_sessionmanager.py tests\unit\customizations\ecs\test_executecommand_startsession.py -q

Copilot AI review requested due to automatic review settings May 11, 2026 06:56
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the ECS execute-command integration with session-manager-plugin to avoid placing the StartSession token payload on the process command line when the installed plugin supports passing it via the AWS_SSM_START_SESSION_RESPONSE environment variable, while retaining the argv-based fallback for older plugin versions.

Changes:

  • Detect session-manager-plugin version and, for supported versions, pass the StartSession response via AWS_SSM_START_SESSION_RESPONSE instead of argv.
  • Preserve the existing argv-based behavior when the plugin version is too old or doesn’t meet the requirement.
  • Add unit test coverage for the new environment-variable path in the ECS execute-command flow.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
awscli/customizations/ecs/executecommand.py Adds plugin version probing and switches to env-var-based StartSession response passing when supported.
tests/unit/customizations/ecs/test_executecommand_startsession.py Updates existing tests for version probing and adds a new test validating the env-var path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 87 to +94
# Hence, making this empty session-manager-plugin call
# before calling execute-command to ensure that
# session-manager-plugin is installed
# before execute-command-command is made
check_call(["session-manager-plugin"])
plugin_version = check_output(
["session-manager-plugin", "--version"], text=True
)
Comment on lines 202 to 217
json.dumps(self.ssm_request_parameters),
self.endpoint_url],
self.endpoint_url],
)
@NguyenCong2k NguyenCong2k force-pushed the fix-ecs-session-token-argv branch from 84fad25 to 026f11a Compare May 11, 2026 07:14
@NguyenCong2k
Copy link
Copy Markdown
Author

Thanks, addressed both comments.

  • Removed the redundant initial session-manager-plugin check_call; the --version call now serves as the plugin existence check.
  • Moved the failing-path argv assertion outside the assertRaises block so it executes.

Tests:

python -m pytest tests\unit\customizations\test_sessionmanager.py tests\unit\customizations\ecs\test_executecommand_startsession.py -q

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants