Context Navigation

← Previous Ticket
Next Ticket →

#61834 closed defect (bug) (fixed)

class-wp-site-health-auto-updates.php triggers error when basedir restrictions in effect

Reported by:	Keffr3n	Owned by:	SergeyBiryukov
Priority:	normal	Milestone:	6.7
Component:	Filesystem API	Version:	6.6.1
Severity:	normal	Keywords:	has-patch needs-testing
Cc:		Focuses:

Description

Hello, for security reasons, the php pools serving my wordpress instances are isolated with open_basedir directive.
For site example.com, it's basedir is fixed to /www/example.com.

But in class-wp-site-health-auto-updates.php, paths outside this directory are tested: /.git, /www/.git and this throws errors.
instead of testing with is_dir, could you test also for access? like with is_readable()?

Attachments (1)

basedir error.webp (39.4 KB ) - added by Keffr3n 2 years ago.

Download all attachments as: .zip

Change History (7)

@Keffr3n
2 years ago

Attachment basedir error.webp added

This ticket was mentioned in PR #7153 on WordPress/wordpress-develop by @narenin.

2 years ago #1

Keywords has-patch added

Trac ticket: https://core.trac.wordpress.org/ticket/61834

#2 @jrf
2 years ago

Focuses php-compatibility removed
Keywords needs-testing added; changes-requested removed

#3 @SergeyBiryukov
2 years ago

Milestone Awaiting Review → 6.7

#4 in reply to: ↑ description @SergeyBiryukov
2 years ago

Hi there, thanks for the ticket! I was able to reproduce the issue. For anyone testing, note that the warning here may not be displayed on screen due to this test being performed via a REST API request, for which display_errors is off by default in wp_debug_mode(), so make sure to check the error log.

Replying to Keffr3n:

instead of testing with is_dir, could you test also for access? like with is_readable()?

In my testing, is_readable() appears to throw the same warning. However, we did resolve a similar issue previously in [55425] / #42619, so we can reuse WP_Automatic_Updater::is_allowed_dir() here.

#5 @SergeyBiryukov
2 years ago

Owner set to SergeyBiryukov
Resolution → fixed
Status new → closed

In 58921:

Site Health: Check if the directories are allowed when testing for a VCS checkout.

As part of determining whether to perform automatic updates, WordPress checks if it is running within a version-controlled environment, recursively looking up the filesystem to the top of the drive, looking for a Subversion, Git, Mercurial, or Bazaar directory, erring on the side of detecting a VCS checkout somewhere.

This commit reuses WP_Automatic_Updater::is_allowed_dir() in the Site Health test to avoid a PHP warning if the open_basedir directive is in use and any of the directories checked in the process are not allowed:

is_dir(): open_basedir restriction in effect. File(/.git) is not within the allowed path(s)

Follow-up to [44986], [55425].

Props Keffr3n, narenin.
Fixes #61834.

@SergeyBiryukov commented on PR #7153:

2 years ago #6

Thanks for the PR! Merged a slightly different approach in r58921.

Note: See TracTickets for help on using tickets.

Download in other formats:

Make WordPress Core

Context Navigation

#61834 closed defect (bug) (fixed)

class-wp-site-health-auto-updates.php triggers error when basedir restrictions in effect

Description

Attachments (1)

Change History (7)

@Keffr3n 2 years ago

This ticket was mentioned in ​PR #7153 on ​WordPress/wordpress-develop by ​@narenin. 2 years ago #1

#2 @jrf 2 years ago

#3 @SergeyBiryukov 2 years ago

#4 in reply to: ↑ description @SergeyBiryukov 2 years ago

#5 @SergeyBiryukov 2 years ago

​@SergeyBiryukov commented on ​PR #7153: 2 years ago #6