Skip to content

Conversation

@hubertdeng123
Copy link
Member

This allows us to easily ensure devservices does not bring up container for service in CI. Will be useful for snuba

if [ "${{ inputs.toggle }}" != "" ]; then
echo "Toggling off ${{ inputs.toggle }}"
devservices toggle ${{ inputs.toggle }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Unquoted shell variable allows command injection

The ${{ inputs.toggle }} value is directly interpolated into the shell command without quotes on line 132. If the toggle input contains special shell characters or malicious content (e.g., snuba; malicious-command), it could lead to command injection. While the input is quoted in the condition check on line 130, it's unquoted when passed to devservices toggle. Using an environment variable (like WORKDIR is handled) and quoting it would be safer.

Fix in Cursor Fix in Web

required: false
default: 'default'
toggle:
description: 'Dependency to toggle off '
Copy link
Member

@joshuarli joshuarli Dec 5, 2025

Choose a reason for hiding this comment

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

shouldn't this be dependency to toggle to local to be explicit?

and then devservices toggle foo local

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, it should

@hubertdeng123 hubertdeng123 marked this pull request as draft December 5, 2025 22:58
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.

3 participants