Skip to content

Fix exit code 127 during helmfile diff by preventing shell interpretation#541

Closed
Copilot wants to merge 2 commits intomainfrom
copilot/fix-516
Closed

Fix exit code 127 during helmfile diff by preventing shell interpretation#541
Copilot wants to merge 2 commits intomainfrom
copilot/fix-516

Conversation

Copy link
Contributor

Copilot AI commented Aug 14, 2025

This PR fixes a critical issue where helmfile commands fail with exit code 127 when processing YAML content containing shell special characters, particularly semicolons in description fields.

Problem

The issue occurs when helmfile outputs YAML content like:

description: 'Kind of the referent; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'

The current implementation executes helmfile commands using shell string interpolation:

await exec.exec(`helmfile ${helmfileArgs}`, [], options)

When helmfile outputs content with semicolons, the shell interprets the text after the semicolon as a separate command, causing errors like:

/home/runner/work/_temp/xxx.sh: line 966: More: command not found
##[error]Process completed with exit code 127.

Solution

Modified the command execution to pass the command and arguments separately to prevent shell interpretation:

  1. Added parseArgs() function in src/helpers.ts that properly parses command line arguments while preserving quoted strings and handling shell metacharacters safely.

  2. Updated command execution in src/main.ts to use:

    await exec.exec('helmfile', parseArgs(helmfileArgs), options)
  3. Added comprehensive tests covering:

    • Basic argument parsing
    • Quoted arguments with special characters
    • Shell injection prevention
    • Edge cases and backward compatibility

Verification

The fix safely handles the problematic case:

parseArgs('diff --values "description: Kind of the referent; More info: https://example.com"')
// Returns: ['diff', '--values', 'description: Kind of the referent; More info: https://example.com']

All existing helmfile command patterns continue to work exactly as before, ensuring full backward compatibility.

Fixes #516.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copy link
Contributor Author

Copilot AI commented Aug 14, 2025

@yxxhero 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

…tion

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
Copilot AI changed the title [WIP] Exit code 127 during helmfile diff Fix exit code 127 during helmfile diff by preventing shell interpretation Aug 14, 2025
Copilot AI requested a review from yxxhero August 14, 2025 23:27
@yxxhero
Copy link
Member

yxxhero commented Aug 15, 2025

@copilot pleaes fix lint errors.

Copy link
Contributor Author

Copilot AI commented Aug 15, 2025

@copilot pleaes fix lint errors.

I've checked the code for lint errors, and everything is already passing! All linting checks are successful:

  • npm run lint - no errors found
  • npm run format-check - code properly formatted
  • npm run build - TypeScript compilation successful
  • npm test - all tests passing
  • npm run all - complete build pipeline successful

The only message shown is a TypeScript version compatibility warning, which is not an actual lint error but just an informational notice that doesn't affect functionality.

All the code added in the previous commits (parseArgs function, tests, and main.ts changes) already follows the project's ESLint configuration correctly.

@yxxhero
Copy link
Member

yxxhero commented Aug 15, 2025

pleaes fix conflicts. @copilot

Copy link
Contributor Author

Copilot AI commented Aug 15, 2025

pleaes fix conflicts. @copilot

Conflicts resolved! I've successfully rebased the branch on top of the latest main branch (commit 6682d7a). The merge conflicts in __tests__/main.test.ts and dist/index.js.map have been resolved:

  • Removed the conflicting __tests__/main.test.ts file (it was deleted in main branch)
  • Regenerated dist/index.js.map by running the build process
  • All tests are passing ✅
  • Full build pipeline successful ✅

The branch is now up to date and ready for merge.

@yxxhero
Copy link
Member

yxxhero commented Aug 15, 2025

@copilot please fix conflicts.

Copy link
Contributor Author

Copilot AI commented Aug 15, 2025

@copilot please fix conflicts.

Conflicts resolved! Successfully rebased the branch on top of the latest main branch (f898214). The merge conflicts in package.json, package-lock.json, and dist/index.js.map have been resolved, and the tests/main.test.ts file was properly removed as it was deleted in main. All tests are passing ✅ and the build pipeline is successful ✅.

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.

Exit code 127 during helmfile diff

2 participants