Escape URL parameter in update-db and check-update-db network commands - #338
Conversation
swissspidy
added
command:core-update-db
📝 WalkthroughWalkthroughNetwork database check and update commands now escape site URLs before building nested WP-CLI commands. A multisite feature test covers site paths containing command-substitution characters. ChangesNetwork database command safety
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR hardens wp core check-update-db --network and wp core update-db --network by safely escaping the per-site --url argument when invoking subcommands, and adds an acceptance test intended to catch command-injection regressions.
Changes:
- Escape the
--urlargument viaUtils\esc_cmd()when runningcore check-update-dbfor each network site. - Escape the
--urlargument viaUtils\esc_cmd()when runningcore update-dbfor each network site (including--dry-runhandling). - Add a Behat scenario attempting to ensure special characters in site domain/path don’t lead to shell execution during network
check-update-db.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Core_Command.php |
Escapes --url when spawning network per-site check-update-db / update-db subcommands. |
features/core-check-update-db.feature |
Adds a Behat scenario aimed at preventing command-injection regressions in check-update-db --network. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| And I run `wp db query "INSERT INTO wp_blogs (site_id, domain, path, registered, last_updated) VALUES (1, 'example.com', '/x\\\$(touch /tmp/wpcli_test_check_db_marker)/', NOW(), NOW());"` | ||
|
|
| ++$total; | ||
| $url = $blog->domain . $blog->path; | ||
| $cmd = "--url={$url} core update-db"; | ||
| $cmd = Utils\esc_cmd( '--url=%s core update-db', $url ); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
features/core-check-update-db.feature (1)
102-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise
update-dbin the regression test.The scenario runs only
wp core check-update-db --networkat Line [106]. It does not execute the newupdate_db()path at Line [1631]. Addwp core update-db --network --dry-runand repeat the marker assertion.Suggested test extension
+ When I run `wp core update-db --network --dry-run` + Then the return code should be 0 + And the /tmp/wpcli_test_check_db_marker file should not exist🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/core-check-update-db.feature` around lines 102 - 109, Extend the special-character multisite scenario after the existing check-update-db command by running `wp core update-db --network --dry-run`, then repeat the assertion that `/tmp/wpcli_test_check_db_marker` does not exist. Keep the current return-code and marker checks for `check-update-db` unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@features/core-check-update-db.feature`:
- Around line 102-109: Extend the special-character multisite scenario after the
existing check-update-db command by running `wp core update-db --network
--dry-run`, then repeat the assertion that `/tmp/wpcli_test_check_db_marker`
does not exist. Keep the current return-code and marker checks for
`check-update-db` unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c8b8ad60-a760-4105-984f-7e5d18103682
📒 Files selected for processing (2)
features/core-check-update-db.featuresrc/Core_Command.php
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Apply some slight hardening
Summary by CodeRabbit