Repository navigation
Reject invalid Domain Paths in make-pot - #499
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe make-pot command now normalizes plugin ChangesDomain Path validation
Estimated code review effort: 2 (Simple) | ~10 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 i18n make-pot by validating the plugin/theme Domain Path header before using it to derive the POT output destination, and adds an acceptance test to ensure invalid paths are rejected.
Changes:
- Normalize and validate
Domain Pathbefore applying it to the computed destination path. - Fail fast with a clear WP-CLI error when an invalid
Domain Pathis detected. - Add a Behat scenario asserting
..traversal inDomain Pathis refused.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/MakePotCommand.php | Adds normalization + validation for Domain Path before using it to build the POT destination path. |
| features/makepot.feature | Adds a Behat scenario asserting invalid Domain Path values are rejected with an error. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
features/makepot.feature (1)
329-347: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the remaining rejection branches.
This scenario verifies traversal rejection only. Add scenarios for an absolute value such as
C:/tmp/languagesand a stream value such asphp://filter/resource=languages. Verify the same error contract and return code for both cases.🤖 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/makepot.feature` around lines 329 - 347, Add scenarios in features/makepot.feature covering Domain Path values “C:/tmp/languages” and “php://filter/resource=languages”; for each, invoke wp i18n make-pot foo-plugin and assert STDERR contains the corresponding “Error: Invalid Domain Path value: …” message and the return code is 1, matching the existing traversal-rejection scenario.
🤖 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.
Inline comments:
In `@src/MakePotCommand.php`:
- Around line 398-400: Update the domain-path validation condition in
MakePotCommand to detect parent traversal only when “..” is a complete path
component, not merely a substring within a directory name such as “foo..bar”.
Preserve the existing absolute-path, stream-path, and invalid-value error
handling checks.
---
Nitpick comments:
In `@features/makepot.feature`:
- Around line 329-347: Add scenarios in features/makepot.feature covering Domain
Path values “C:/tmp/languages” and “php://filter/resource=languages”; for each,
invoke wp i18n make-pot foo-plugin and assert STDERR contains the corresponding
“Error: Invalid Domain Path value: …” message and the return code is 1, matching
the existing traversal-rejection scenario.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c2396538-603d-4301-959a-98917f612c2f
📒 Files selected for processing (2)
features/makepot.featuresrc/MakePotCommand.php
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Applying some slight hardening
Summary by CodeRabbit
Domain Pathvalues.