fix: properly quote parameters for vip wp#2184
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
This pull request has been marked stale because it has been open for 60 days with no activity. If there is no activity within 7 days, it will be closed. This is an automation to keep pull requests manageable and actionable and is not a comment on the quality of this pull request nor on the work done so far. Closed PRs are still valuable to the project and their branches are preserved. |
|
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the way command arguments are quoted for the vip wp command to prevent parameter injection by unconditionally enclosing each argument in double quotes and escaping inner double quotes. Key changes include:
- Simplified the requoteArgs function in src/lib/cli/format.ts to always wrap arguments in double quotes.
- Removed the conditional logic and helper functions (isJsonObject and isJson) that previously handled selective quoting.
- Updated corresponding tests in tests/lib/cli/format.js to reflect the new quoting behavior.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/lib/cli/format.ts | Simplifies the quoting logic by unconditionally quoting each argument |
| tests/lib/cli/format.js | Updates test expectations to match the new quoting behavior |
Comments suppressed due to low confidence (1)
src/lib/cli/format.ts:142
- Consider adding an inline comment explaining why the function now unconditionally quotes all arguments and removes the JSON detection logic to improve code clarity for future maintainers.
export function requoteArgs( args: string[] ): string[] {
|
Description
The way the parameters are quoted now is insecure and vulnerable to parameter injection.
This PR tries to fix this.
Because we don't care about shell expansion or redirection (WP CLI commands will be executed directly, not via a shell), we can simplify quoting by enclosing the parameter in double quotes and escaping all inner double quotes with a slash.
Related: Automattic/cron-control-runner#32
Pull request checklist
New release checklist
Steps to Test
TBD.