Skip to content

fix: command arguments need to be split#169

Open
neuromantik33 wants to merge 1 commit into
rundeck-plugins:mainfrom
42school:run-script-fix
Open

fix: command arguments need to be split#169
neuromantik33 wants to merge 1 commit into
rundeck-plugins:mainfrom
42school:run-script-fix

Conversation

@neuromantik33

Copy link
Copy Markdown

Command arguments need to be split when invoking inline scripts in pods.

@fdevans fdevans requested a review from Copilot October 1, 2025 23:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes a bug in the pod script execution where command arguments were being passed as a single string instead of being split into individual arguments. This ensures that inline scripts in pods receive properly formatted command-line arguments.

  • Splits the RD_CONFIG_ARGUMENTS string into individual arguments using spaces as delimiters
  • Changes from appending a single string to appending individual arguments to the exec_command list

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

if 'RD_CONFIG_ARGUMENTS' in os.environ:
arguments = os.environ.get('RD_CONFIG_ARGUMENTS')
exec_command.append(arguments)
for arg in arguments.split(" "):

Copilot AI Oct 1, 2025

Copy link

Choose a reason for hiding this comment

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

Using split(' ') will not handle arguments that contain spaces (even when quoted) or multiple consecutive spaces correctly. Consider using shlex.split() instead, which properly handles shell-style quoting and escaping.

Copilot uses AI. Check for mistakes.
@fdevans

fdevans commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

Thank you for identifying this bug! You're absolutely right that arguments need to be split - the current code incorrectly passes all arguments as a single string element instead of splitting them into individual arguments.

Your fix is correct and consistent with how the code already handles the invocation parameter (line 122).

One consideration: Simple space-splitting doesn't handle quoted arguments (e.g., --message "hello world"), but that's a pre-existing limitation that also affects the invocation parameter. Your change maintains consistency with the existing approach, which is the right call for this fix.

Before we can merge this, could you please:

  1. Rebase on the latest master branch - there have been several updates since this PR was opened
  2. Add a brief note in the PR description about what scenarios this fixes (e.g., "Fixes passing multiple arguments to inline scripts")

Thanks for the contribution!

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