Skip to content

fix: command arguments need to be split#169

Open
neuromantik33 wants to merge 1 commit intorundeck-plugins:masterfrom
42school:run-script-fix
Open

fix: command arguments need to be split#169
neuromantik33 wants to merge 1 commit intorundeck-plugins:masterfrom
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
Copy link
Copy Markdown
Contributor

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 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(" "):
Copy link

Copilot AI Oct 1, 2025

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
Copy link
Copy Markdown
Contributor

fdevans commented Mar 6, 2026

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