Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 12, 2026

Plan to address feedback on open_basedir warning fix

  • Understand the feedback from @swissspidy
  • Remove the @ error suppression operator
  • Implement open_basedir checking approach as suggested
  • Create is_path_within_open_basedir() helper function
  • Update find_file_upward() to check open_basedir before is_readable()
  • Run linting and code style checks (all passing)
  • Run PHPStan static analysis (passing)
  • Run PHPUnit tests (passing - existing failures unrelated)
  • Manually test with various open_basedir scenarios
  • Reply to comment with commit hash (c8646a5)
  • Address code review feedback on path normalization
  • Add proper path normalization to find_file_upward()
  • Final code review and security scan (completed)
  • Add case-insensitive path comparison for Windows

Summary

Successfully addressed all feedback on the PR to fix open_basedir warnings in WP-CLI.

Latest change: Added case-insensitive path comparison for Windows systems to handle scenarios where the case of paths in open_basedir settings doesn't match the actual filesystem paths.

Original prompt

This section details on the original issue you should resolve

<issue_title>is_readable warning when wp cli is called with open_basedir set. Warning should be avoided/suppressed</issue_title>
<issue_description>## Bug/Warning Report

When calling wp cli like this...

php -d open_basedir=/usr/local/bin:/var/www/example.com /usr/local/bin/wp cron event run --due-now --path=/var/www/example.com/htdocs

we get this:

PHP Warning: is_readable(): open_basedir restriction in effect. File(/var/www) is not within the allowed path(s): (/usr/local/bin:/var/www/example.com) in phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/utils.php on line 200

This is from the function: find_file_upward()

The site's config file is: /var/www/example.com/htdocs/wp-config.php

If we remove "-d open_basedir=/usr/local/bin:/var/www/example.com" from the call, the warning goes away because find_file_upward() is able to read filesystem data all the way up to the root "/".

Passing open_basedir to PHP when running WP cron jobs (or executing any theme/plugin code) is a good security measure, but our automation scripts are full of unnecessary warnings from the call to is_readable() within find_file_upward().

NOTE: The cron jobs do execute normally - this is just a warning that we would like to see avoided/suppressed.

Thanks

Paul</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits January 12, 2026 12:43
… set

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Copilot AI changed the title [WIP] Suppress is_readable warning in wp-cli with open_basedir Suppress is_readable() warning when open_basedir restricts directory traversal Jan 12, 2026
Copilot AI requested a review from swissspidy January 12, 2026 12:46
@codecov

This comment was marked as resolved.

This comment was marked as resolved.

@swissspidy

This comment was marked as resolved.

Instead of suppressing errors with @, check if the path is within
open_basedir restrictions before calling is_readable(). This approach
avoids hiding other potential errors while still preventing warnings
when traversing directories outside allowed paths.

Add is_path_within_open_basedir() helper function that:
- Returns true if no open_basedir restriction is set
- Checks if path starts with any allowed path from open_basedir
- Uses string comparison to avoid triggering warnings on restricted paths

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>

This comment was marked as resolved.

- Normalize input directory with realpath() early in find_file_upward()
  to handle relative paths and symlinks properly
- Add safety check for getcwd() returning false
- Update documentation to clarify that is_path_within_open_basedir()
  expects absolute paths
- This addresses security concerns about asymmetric path comparison

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Copilot AI changed the title Suppress is_readable() warning when open_basedir restricts directory traversal Check open_basedir restrictions before is_readable() to prevent warnings Jan 12, 2026

This comment was marked as resolved.

On Windows, filesystem paths are case-insensitive, so the path comparison
in is_path_within_open_basedir() needs to handle this. Added case-insensitive
comparison using strcasecmp() and stripos() when running on Windows systems
(detected by checking if DIRECTORY_SEPARATOR is backslash).

This ensures that open_basedir checks work correctly on Windows even when
the case of paths doesn't match between open_basedir settings and actual paths.

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

is_readable warning when wp cli is called with open_basedir set. Warning should be avoided/suppressed

2 participants