Use 0755 instead of 0700 for the packages directory - #244
Conversation
0700 breaks installations that point `WP_CLI_PACKAGES_DIR` at a location shared between users: whichever user first runs `wp package install` owns the directory, and every other user then silently loses their packages, because `IncludePackageAutoloader` only checks `is_readable()` and otherwise falls through to a debug message. The risk being guarded against is another local user *writing* into the packages directory, whose `vendor/autoload.php` is required on every WP-CLI run. Read access was never part of that, so 0755 closes the hole while leaving shared-read setups working. A umask can only clear further bits, so the result is never group- or world-writable. The unit test asserted a literal `0700`, which is umask-dependent. Assert that the group and other write bits are unset instead, which is the invariant that actually matters. Follow-up to #243. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014qPfx7yCg9xCTys3XXXRkU
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 default Composer packages directory now uses mode ChangesPackages directory permissions
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Adjusts WP-CLI package directory creation to use 0755 (instead of 0700) so installations that share WP_CLI_PACKAGES_DIR across multiple users remain readable, while still preventing group/world writes that could enable local code execution via the package autoloader.
Changes:
- Change packages directory creation mode from
0700to0755inPackage_Command::create_default_composer_json(). - Update the PHPUnit assertion to check the invariant “not group/world writable” rather than asserting a literal mode string that varies with
umask.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Package_Command.php |
Creates the packages composer directory with 0755 to keep shared directories readable while still preventing group/world writes. |
tests/phpunit/ComposerJsonTest.php |
Updates the permissions assertion to be umask-robust by checking write bits instead of a fixed mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $perms = fileperms( dirname( $actual ) ) & 0777; | ||
| $this->assertSame( | ||
| 0, | ||
| $perms & 0022, | ||
| sprintf( 'Packages directory must not be group- or world-writable, got %o.', $perms ) | ||
| ); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/Package_Command.php`:
- Around line 1286-1290: Update the package setup flow around mkdir in
Package_Command to protect Composer-generated PHP files, including
vendor/autoload.php, from group or world write access. Ensure the Composer root
remains private or enforce restrictive permissions on generated PHP files after
Composer runs, while preserving support for shared WP_CLI_PACKAGES_DIR
locations.
🪄 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: e9ede94f-0657-405e-b64e-e7bf8f93b883
📒 Files selected for processing (2)
src/Package_Command.phptests/phpunit/ComposerJsonTest.php
| // 0755 rather than 0700: the risk being guarded against is another local user | ||
| // *writing* into the packages directory, whose `vendor/autoload.php` is required on | ||
| // every WP-CLI run. Read access was never the problem, and revoking it breaks setups | ||
| // that point `WP_CLI_PACKAGES_DIR` at a location shared between users. | ||
| if ( ! @mkdir( $composer_dir, 0755, true ) ) { // @codingStandardsIgnoreLine |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Composer 2.10.2 src Composer Autoload AutoloadGenerator generate filePutContentsIfModified vendor/autoload.php chmod
💡 Result:
In Composer 2.10.2, the AutoloadGenerator class is responsible for generating autoloader files within the vendor directory [1][2][3]. The process involves generating several files (e.g., autoload.php, autoload_static.php, autoload_real.php) using the filePutContentsIfModified method from the Filesystem utility class [1][3][4][5]. The filePutContentsIfModified method is implemented in src/Composer/Util/Filesystem.php as follows: public function filePutContentsIfModified(string $path, string $content) { $currentContent = Silencer::call('file_get_contents', $path); if (false === $currentContent || $currentContent!== $content) { return file_put_contents($path, $content); } return 0; } This method only performs a file write operation if the target file does not exist or if the content has changed, which helps avoid unnecessary disk I/O and maintains original file timestamps when no modification is required [4]. It does not explicitly call chmod to modify file permissions during the generation of these files; standard file permissions are determined by the PHP file_put_contents function and the system's default umask settings [4].
Citations:
- 1: https://github.com/composer/composer/blob/main/src/Composer/Autoload/AutoloadGenerator.php
- 2: https://github.com/composer/composer/releases/tag/2.10.2
- 3: https://github.com/composer/composer/blob/d0aac44ed210e13ec4a4370908a5b36553a2f16c/src/Composer/Autoload/AutoloadGenerator.php
- 4: https://github.com/composer/composer/blob/main/src/Composer/Util/Filesystem.php
- 5: https://deepwiki.com/composer/composer/5.2-autoloader-generation
🌐 Web query:
Composer 2.10.2 Package_Command require composer version autoload vendor autoload.php include chmod
💡 Result:
Composer 2.10.2 is a version of the Composer dependency manager released on July 1, 2026 [1][2]. The require command is used to add new packages to your composer.json file and install them into your project [3]. When you run composer require <package-name>, Composer updates your dependencies and regenerates the autoloading files, specifically the vendor/autoload.php entry point [4][5][6]. Regarding vendor/autoload.php and chmod permissions: 1. Purpose of vendor/autoload.php: This file is the central entry point for PHP's autoloading mechanism in Composer-managed projects [4][6]. By including it at the start of your application (e.g., require DIR . '/vendor/autoload.php';), PHP will automatically load any classes from your installed packages when needed, eliminating the need for manual require or include statements for individual class files [4][7][5][6]. 2. Permission Issues and chmod: If you encounter "Permission denied" or "Failed opening required" errors when attempting to include this file, it is typically due to incorrect file system permissions or ownership [8][9][10]. - Ownership: The vendor directory and its contents should be owned by the user or service account that executes your PHP scripts (such as www-data, apache, or your own user) [8][9][10]. You can fix ownership issues using chown (e.g., sudo chown -R <user>:<group> vendor) [10]. - Permissions: Ensure the directories and files are readable by the PHP process [8]. While chmod can be used to set permissions (e.g., chmod 755 -R vendor), it is generally better practice to manage ownership correctly rather than granting broad, insecure permissions like o+w (world-writable) [11][10]. If the vendor/autoload.php file is missing, ensure you have successfully run composer install or composer update in the project root [4][6]. If you change your project's autoload configuration in composer.json, you must run composer dump-autoload to regenerate the autoloader files [4][6].
Citations:
- 1: https://github.com/composer/composer/releases/tag/2.10.2
- 2: https://getcomposer.org/changelog/2.10.2
- 3: https://getcomposer.org/doc/03-cli.md
- 4: https://getcomposer.org/doc/01-basic-usage.md
- 5: https://github.com/mochilang/mochi/blob/main/website/docs/research/0075/09-psr-autoloading.md
- 6: https://thecodeforge.io/php/composer-autoloading-php/
- 7: https://www.php.net/manual/en/language.oop5.autoload.php
- 8: https://stackoverflow.com/questions/35215723/composer-on-amazon-server-ec2-autoload-not-working
- 9: https://stackoverflow.com/questions/58897583/error-after-installing-package-from-composer
- 10: https://stackoverflow.com/questions/57333356/runtimeexception-on-a-composer-update
- 11: https://stackoverflow.com/questions/56247262/php-docker-vendor-autoload-php-not-found
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package composer constraints =="
git ls-files | rg '(^|/)composer\.(json|lock)$' || true
for f in $(git ls-files 'composer.*'); do
echo "--- $f"
sed -n '1,220p' "$f" | rg -n '"name"|"require"|"php"|"composer-plugin-api"|"composer-runtime-api"|WP_CLI' || true
done
echo "== autoload.php require paths =="
rg -n "vendor/autoload\.php|autoload\.php" src tests --glob '*.php' || true
echo "== package install/update autoloader generation hints =="
rg -n "dump-autoload|autoload|vendor/autoload|createDefaultComposer|create_default_composer|install\(|update\(" src tests/phpunit/ComposerJsonTest.php tests --glob '*.php' | head -n 200 || trueRepository: wp-cli/package-command
Length of output: 2955
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== composer.json =="
cat -n composer.json
echo "== Package_Command relevant sections =="
sed -n '1,90p' src/Package_Command.php
sed -n '220,265p' src/Package_Command.php
sed -n '680,720p' src/Package_Command.php
sed -n '800,850p' src/Package_Command.php
sed -n '1240,1340p' src/Package_Command.php
sed -n '1340,1390p' src/Package_Command.phpRepository: wp-cli/package-command
Length of output: 16208
Security Misconfiguration (CWE-732): Incorrect Permission Assignment for Critical Resource
Reachability: Internal
Protect generated autoload files, not only the packages directory.
0755 clears group/other write from the directory, but Composer 2.10.2 still writes generated PHP files through file_put_contents() without clearing file write bits. On a shared packages directory with a permissive umask, a local peer with file access can overwrite vendor/autoload.php even though they cannot create files there. Enforce that Composer-created PHP files under WP-CLI packages are not group- or world-writable, or keep the Composer root private until Composer applies stricter default permissions.
🤖 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 `@src/Package_Command.php` around lines 1286 - 1290, Update the package setup
flow around mkdir in Package_Command to protect Composer-generated PHP files,
including vendor/autoload.php, from group or world write access. Ensure the
Composer root remains private or enforce restrictive permissions on generated
PHP files after Composer runs, while preserving support for shared
WP_CLI_PACKAGES_DIR locations.
Source: MCP tools
Follow-up to #243, which set the packages directory to
0700.The problem
0700breaks installations that pointWP_CLI_PACKAGES_DIRat a location shared between users — e.g. a host settingWP_CLI_PACKAGES_DIR: /opt/wp-cli/packagesin/etc/wp-cli/config.yml. Whichever user first runswp package installcreates and owns the directory, and every other user then silently loses their packages, becauseIncludePackageAutoloaderonly checksis_readable():No error, no warning — the packages just stop existing unless you run
--debug.Why 0755 is enough
The risk #243 guards against is another local user writing into the packages directory, whose
vendor/autoload.phpis required on every single WP-CLI run. Read access was never part of that — these are downloaded public packages, not secrets.0755removes group and world write outright, and a umask can only clear further bits, so the result is never group- or world-writable:mkdir(0777)mkdir(0755)The two rows in bold are the configurations that made this exploitable in the first place, and
0755closes both.Note this only affects directories WP-CLI creates — the
is_dir()guard leaves a pre-provisioned directory untouched, so an admin who sets up a shared packages directory with a group and the setgid bit keeps full control either way.Test change
The unit test asserted the literal string
0700, which is umask-dependent. It now asserts that the group and other write bits are unset, which is the invariant that actually matters and holds under any umask.Testing
php -lpasses on both files. I could not run PHPUnit in my environment —composer installcannot authenticate to github.com for the dev dependencies — so CI is the real check here. I did verify the permission behaviour and both assertion styles directly across the four umasks in the table above.🤖 Generated with Claude Code
https://claude.ai/code/session_014qPfx7yCg9xCTys3XXXRkU
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
Tests