Skip to content

Site Health false positive: WP_DEBUG_LOG warning when debug.log is outside wp-content - Ticket #64071#10684

Open
hbhalodia wants to merge 54 commits into
WordPress:trunkfrom
hbhalodia:fix/issue-64071
Open

Site Health false positive: WP_DEBUG_LOG warning when debug.log is outside wp-content - Ticket #64071#10684
hbhalodia wants to merge 54 commits into
WordPress:trunkfrom
hbhalodia:fix/issue-64071

Conversation

@hbhalodia

Copy link
Copy Markdown

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

Screenshots

Publicly accessible Not Publicly Accessible
Screenshot 2026-01-06 at 11 41 28 AM Screenshot 2026-01-06 at 11 42 39 AM

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@github-actions

github-actions Bot commented Jan 6, 2026

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props hbhalodia, westonruter, audrasjb, wildworks, mukesh27.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@hbhalodia

Copy link
Copy Markdown
Author

Hi Team, Can you please help review and update the wordings if needed?

Thank You,

@github-actions

github-actions Bot commented Jan 6, 2026

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
@hbhalodia
hbhalodia requested a review from westonruter January 8, 2026 06:49

@westonruter westonruter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we wanted to go the extra mile, there could be a loopback request to try to actually request the file over HTTP to see if it returns anything. This may not be helpful in the end, however, as the file may not be present if nothing has been written to the log yet. Just an idea.

Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
@hbhalodia

Copy link
Copy Markdown
Author

If we wanted to go the extra mile, there could be a loopback request to try to actually request the file over HTTP to see if it returns anything. This may not be helpful in the end, however, as the file may not be present if nothing has been written to the log yet. Just an idea.

Thanks @westonruter, I thought of an edge case as well, what if there is no any log file present, then realpath will return false, and since we are checking a condition out there related if both the paths are present and if that is public path, then only to consider it as the debugging log is present in public directory, else we can show what we are showing.

@hbhalodia
hbhalodia requested a review from westonruter January 8, 2026 07:39
@westonruter

Copy link
Copy Markdown
Member

I thought of an edge case as well, what if there is no any log file present, then realpath will return false, and since we are checking a condition out there related if both the paths are present and if that is public path, then only to consider it as the debugging log is present in public directory, else we can show what we are showing.

Good point. Well, in that case you could always check to see if ini_get( 'error_log' ) === WP_CONTENT_DIR . '/debug.log' in case the realpath() returns false. This will catch the default case when someone uses the default when setting WP_DEBUG_LOG to true:

if ( in_array( strtolower( (string) WP_DEBUG_LOG ), array( 'true', '1' ), true ) ) {
$log_path = WP_CONTENT_DIR . '/debug.log';

If, however, they do something like:

define( 'WP_DEBUG_LOG`, ABSPATH . 'debug.log' );

Then checking WP_DEBUG_LOG could fail if no logs have been written yet. In that case, if realpath() returns false, then actually there isn't a need to check realpath() on the full error log. All we need to do is check the path to the directory because that's not something which would be automatically created when a new error log entry is written. So I think this can be simplified by turning $debug_log_path = realpath( $debug_log_path ); into $debug_log_path = realpath( dirname( $debug_log_path ) ) . DIRECTORY_SEPARATOR;

Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
@hbhalodia
hbhalodia requested a review from westonruter January 9, 2026 05:15
Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a false positive in WordPress Site Health where a warning about WP_DEBUG_LOG being set to a potentially public file is shown even when the debug log is configured to be outside the wp-content directory (and thus not publicly accessible).

Changes:

  • Modified the debug mode check to determine if error log files are within or outside the WordPress document root
  • Added different status levels (critical vs good) and messages based on whether the log file is publicly accessible
  • Differentiated messaging between WP_DEBUG_LOG constant usage and direct PHP error_log configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/wp-admin/includes/class-wp-site-health.php
Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
Comment thread src/wp-admin/includes/class-wp-site-health.php Outdated
@hbhalodia
hbhalodia requested a review from westonruter March 20, 2026 10:47
Copilot AI review requested due to automatic review settings July 25, 2026 03:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (5)

tests/phpunit/tests/admin/wpSiteHealth.php:792

  • The expected public-log result hardcodes translatable strings (label/description) instead of using __(). Since WP_Site_Health::get_test_is_in_debug_mode() builds these messages with __(), using the same in tests avoids coupling to the untranslated English strings.
		$result = array(
			'status'      => 'critical',
			'label'       => 'Your site is set to log errors to a potentially public file',
			'description' => 'The constant, <code>WP_DEBUG_LOG</code>, has been added to this website&#8217;s configuration file. This means any errors on the site will be written to a file which is likely publicly accessible.',
			'test'        => 'is_in_debug_mode',

tests/phpunit/tests/admin/wpSiteHealth.php:814

  • The expected private-log result hardcodes translatable strings (label/description). Align this with the rest of the suite by wrapping user-facing strings in __() so the test isn’t tied to the untranslated English output.
		$result = array(
			'status'      => 'good',
			'label'       => 'Your site is set to log errors to a file outside the document root',
			'description' => 'The configuration constant, <code>WP_DEBUG_LOG</code>, is enabled. In addition, your site is set to write errors to a file outside the WordPress directory, which is a good practice as the log file should not be publicly accessible.',
			'test'        => 'is_in_debug_mode',

tests/phpunit/tests/admin/wpSiteHealth.php:836

  • The expected “unable to determine” result hardcodes translatable strings (label/description). Using __() here keeps the expectations consistent with WP_Site_Health::get_test_is_in_debug_mode() and the rest of this test class.
		$result = array(
			'status'      => 'critical',
			'label'       => 'Unable to determine error log file location',
			'description' => 'The configuration constant, <code>WP_DEBUG_LOG</code>, is enabled, but the log file location could not be determined.',
			'test'        => 'is_in_debug_mode',

tests/phpunit/tests/admin/wpSiteHealth.php:1098

  • These assertions compare the label against a hardcoded English string. Use the translated string via __() (as other tests in this file do) to avoid making the test dependent on the default locale.
		$this->assertSame( 'recommended', $actual_result['status'], 'Status should be "recommended" when WP_DEBUG_DISPLAY is enabled in development.' );
		$this->assertSame( 'Your site is set to display errors to site visitors', $actual_result['label'], 'Label should indicate that errors are displayed to visitors.' );
		$this->assertStringContainsString( 'WP_DEBUG_DISPLAY', $actual_result['description'], 'Description should contain WP_DEBUG_DISPLAY.' );

tests/phpunit/tests/admin/wpSiteHealth.php:962

  • Same portability issue as the previous test: /var/log may not exist, which would make realpath() fail and change the code path under test. Use an existing directory like sys_get_temp_dir() to keep the test reliable across environments.
		$site_health      = $this->setup_site_health_with_debug_properties( true, false, null );
		$private_log_path = '/var/log/php-error.log';

		ini_set( 'error_log', $private_log_path );

Comment thread src/wp-admin/includes/class-wp-site-health.php
Comment thread tests/phpstan/bootstrap.php Outdated
Comment thread tests/phpunit/tests/admin/wpSiteHealth.php
Comment thread tests/phpunit/tests/admin/wpSiteHealth.php
Comment thread tests/phpunit/tests/admin/wpSiteHealth.php
The assignments in test_is_in_debug_mode_error_log_private() were not
aligned, causing the Coding standards / PHP checks job to fail with
Generic.Formatting.MultipleStatementAlignment.NotSameWarning.

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 25, 2026 03:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (7)

src/wp-admin/includes/class-wp-site-health.php:1493

  • The “outside the document root” label is misleading here because the code is only checking whether the resolved error_log directory is inside realpath( ABSPATH ) (the WordPress installation directory), not the web server document root. This could cause confusing guidance for users (and mismatches the wording used in the description).
				} elseif ( 'private' === $log_path_status ) {
					$result['label']  = __( 'Your site is set to log errors to a file outside the document root' );
					$result['status'] = 'good';

tests/phpstan/bootstrap.php:29

  • This conditional uses an undefined $_ variable and results in WP_DEBUG_LOG always being falsey (false or ''), which doesn’t help PHPStan explore code paths where WP_DEBUG_LOG is enabled. Consider defining it from an environment variable so PHPStan sees a string|false union that can be truthy.
if ( isset( $_ ) ) {
	define( 'WP_DEBUG_LOG', false );
} else {
	define( 'WP_DEBUG_LOG', '' );
}

tests/phpunit/tests/admin/wpSiteHealth.php:940

  • This test hard-codes a Unix-specific /var/log/... path. On Windows (and some restricted CI environments) realpath( dirname( ini_get( 'error_log' ) ) ) will return false, causing the code under test to take the “unable to determine” branch and making the test fail. Use an existing directory that’s reliably outside ABSPATH, such as sys_get_temp_dir().
	public function test_is_in_debug_mode_error_log_private(): void {
		$site_health      = $this->setup_site_health_with_debug_properties( true, true, null );
		$private_log_path = '/var/log/php-error.log';

		ini_set( 'error_log', $private_log_path );

tests/phpunit/tests/admin/wpSiteHealth.php:962

  • Same portability issue as the previous test: /var/log/... is not guaranteed to exist across platforms, and can make realpath() fail, changing the expected status. Use sys_get_temp_dir() to ensure the directory exists and is outside ABSPATH.
	public function test_is_in_debug_mode_error_log_private_without_wp_debug_log(): void {
		$site_health      = $this->setup_site_health_with_debug_properties( true, false, null );
		$private_log_path = '/var/log/php-error.log';

		ini_set( 'error_log', $private_log_path );

tests/phpunit/tests/admin/wpSiteHealth.php:773

  • These expected values are hard-coded English strings, while the implementation returns translated strings via __(). That makes the tests locale-dependent and inconsistent with other assertions in this file (which generally compare against __()). Use __() for the expected label/badge strings to keep the test stable under non-default locales.
		return array(
			'status' => 'good',
			'label'  => 'Your site is not set to output debug information',
			'test'   => 'is_in_debug_mode',
			'badge'  => array(

tests/phpunit/tests/admin/wpSiteHealth.php:793

  • This helper hard-codes expected label/description strings instead of using __(), which makes the tests locale-dependent. Align the expected values with the implementation (translated strings) by wrapping them in __().
		$result = array(
			'status'      => 'critical',
			'label'       => 'Your site is set to log errors to a potentially public file',
			'description' => 'The constant, <code>WP_DEBUG_LOG</code>, has been added to this website&#8217;s configuration file. This means any errors on the site will be written to a file which is likely publicly accessible.',
			'test'        => 'is_in_debug_mode',
		);

tests/phpunit/tests/admin/wpSiteHealth.php:837

  • This helper hard-codes expected label/description strings instead of using __(), which makes the tests locale-dependent. Wrap these expected values in __() to match the production code’s translated strings.
		$result = array(
			'status'      => 'critical',
			'label'       => 'Unable to determine error log file location',
			'description' => 'The configuration constant, <code>WP_DEBUG_LOG</code>, is enabled, but the log file location could not be determined.',
			'test'        => 'is_in_debug_mode',
		);

Comment thread tests/phpunit/tests/admin/wpSiteHealth.php
@t-hamano

Copy link
Copy Markdown
Contributor

Hi, @hbhalodia, can you review the Copilot suggestions and update this PR accordingly if they are reasonable?

@hbhalodia

Copy link
Copy Markdown
Author

Hi, @hbhalodia, can you review the Copilot suggestions and update this PR accordingly if they are reasonable?

I would check those and update the PR.

Thanks,

Copilot AI review requested due to automatic review settings July 27, 2026 12:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/wp-admin/includes/class-wp-site-health.php:1469

  • ini_get( 'error_log' ) can return relative paths or special values (e.g. syslog/stderr). Using realpath( dirname( ini_get( 'error_log' ) ) ) will resolve these to the current working directory and can incorrectly mark the log as being inside the WordPress directory ("public"), producing false positives/regressions. Consider treating non-absolute error_log values as "unable to determine" instead of resolving them.
			if ( ! empty( ini_get( 'error_log' ) ) ) {
				$debug_log_dir = realpath( dirname( ini_get( 'error_log' ) ) );
				$absolute_path = realpath( ABSPATH ) . DIRECTORY_SEPARATOR;

				if ( false === $debug_log_dir ) {
					$log_path_status = 'error';
				} elseif ( str_starts_with( $debug_log_dir . DIRECTORY_SEPARATOR, $absolute_path ) ) {
					$log_path_status = 'public';
				} else {
					$log_path_status = 'private';

src/wp-admin/includes/class-wp-site-health.php:1493

  • The "private" branch is determined by the log directory being outside ABSPATH, but the label says "outside the document root". Being outside the WordPress directory does not guarantee being outside the web server document root (e.g. WordPress in a subdirectory of the docroot), so this label is misleading. Consider aligning the label with what’s actually checked.
				} elseif ( 'private' === $log_path_status ) {
					$result['label']  = __( 'Your site is set to log errors to a file outside the document root' );
					$result['status'] = 'good';

tests/phpunit/tests/admin/wpSiteHealth.php:813

  • This expected label repeats the same mismatch as the production code: the logic checks for paths outside the WordPress directory (ABSPATH), not necessarily outside the server document root. If the label in WP_Site_Health::get_test_is_in_debug_mode() is updated to reflect what’s actually checked, the test helper should be updated to match.
		$result = array(
			'status'      => 'good',
			'label'       => 'Your site is set to log errors to a file outside the document root',
			'description' => 'The configuration constant, <code>WP_DEBUG_LOG</code>, is enabled. In addition, your site is set to write errors to a file outside the WordPress directory, which is a good practice as the log file should not be publicly accessible.',

tests/phpstan/bootstrap.php:29

  • This conditional uses an undefined variable ($_) and both branches define falsey values (false/''), which makes the intent hard to understand and doesn’t represent the fact that WP_DEBUG_LOG can also be a truthy string path. Unless there is a specific PHPStan issue requiring this, it would be clearer to define the constant directly (as before).
define( 'WP_DEBUG', false );
define( 'WP_DEBUG_DISPLAY', false );
if ( isset( $_ ) ) {
	define( 'WP_DEBUG_LOG', false );
} else {
	define( 'WP_DEBUG_LOG', '' );
}

Copilot AI review requested due to automatic review settings July 27, 2026 12:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

tests/phpstan/bootstrap.php:29

  • The conditional if ( isset( $_ ) ) is opaque here and (because the bootstrap is executed) will deterministically define WP_DEBUG_LOG as an empty string. That value is not a typical core configuration and makes the intent of this PHPStan bootstrap harder to understand. Prefer defining a single, explicit default value.
if ( isset( $_ ) ) {
	define( 'WP_DEBUG_LOG', false );
} else {
	define( 'WP_DEBUG_LOG', '' );
}

src/wp-admin/includes/class-wp-site-health.php:1494

  • This branch marks the log location as "private" and sets status to good based only on the log directory being outside ABSPATH. That does not necessarily mean it is outside the document root (e.g. WordPress installed in a subdirectory), so the label "outside the document root" and the "good" status can be misleading / a false negative for publicly accessible log files. Consider rewording the label to match what’s actually checked (outside the WordPress directory), and consider whether the status should remain recommended unless you can reliably determine it is outside the web root.
				} elseif ( 'private' === $log_path_status ) {
					$result['label']  = __( 'Your site is set to log errors to a file outside the document root' );
					$result['status'] = 'good';

Comment thread src/wp-admin/includes/class-wp-site-health.php
Comment thread tests/phpunit/tests/admin/wpSiteHealth.php
Copilot AI review requested due to automatic review settings July 27, 2026 12:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

src/wp-admin/includes/class-wp-site-health.php:84

  • The constructor’s handling of WP_DEBUG_LOG and WP_DEBUG_DISPLAY is stricter than core’s wp_debug_mode() behavior (wp-includes/load.php:613+). For example, WP_DEBUG_LOG set to 1 or '1' enables logging in wp_debug_mode(), but this code treats it as disabled, which can make Site Health report the wrong messaging/status.
		$this->wp_debug = defined( 'WP_DEBUG' ) && WP_DEBUG;
		if ( defined( 'WP_DEBUG_LOG' ) && ( is_bool( WP_DEBUG_LOG ) || is_string( WP_DEBUG_LOG ) ) ) {
			$this->wp_debug_log = WP_DEBUG_LOG;
		} else {
			$this->wp_debug_log = false;
		}
		if ( defined( 'WP_DEBUG_DISPLAY' ) && is_bool( WP_DEBUG_DISPLAY ) ) {
			$this->wp_debug_display = WP_DEBUG_DISPLAY;
		} else {
			$this->wp_debug_display = null;
		}

src/wp-admin/includes/class-wp-site-health.php:1469

  • ini_get('error_log') may legally be a non-filesystem target (e.g. 'syslog') or a relative path. Using realpath(dirname(...)) will resolve those to the current working directory (e.g. '.') and can incorrectly classify the log as “public” when WordPress is installed in that directory. Consider treating non-absolute paths as “unknown/error” before applying realpath().
			if ( ! empty( ini_get( 'error_log' ) ) ) {
				$debug_log_dir = realpath( dirname( ini_get( 'error_log' ) ) );
				$absolute_path = realpath( ABSPATH ) . DIRECTORY_SEPARATOR;

				if ( false === $debug_log_dir ) {
					$log_path_status = 'error';
				} elseif ( str_starts_with( $debug_log_dir . DIRECTORY_SEPARATOR, $absolute_path ) ) {
					$log_path_status = 'public';
				} else {
					$log_path_status = 'private';

src/wp-admin/includes/class-wp-site-health.php:1493

  • The “private” branch is determined by checking whether the error log directory is outside ABSPATH (the WordPress install directory), but the label says it’s outside the “document root”. That wording is inaccurate and could overstate safety when WordPress is installed in a subdirectory of the document root.
				} elseif ( 'private' === $log_path_status ) {
					$result['label']  = __( 'Your site is set to log errors to a file outside the document root' );
					$result['status'] = 'good';

tests/phpunit/tests/admin/wpSiteHealth.php:940

  • This test hard-codes a “private” error log path under /var/log, which won’t exist on Windows and can cause realpath(dirname(...)) to fail (making the Site Health result “error/critical” instead of “private/good”). Using sys_get_temp_dir() keeps the test portable while still being outside ABSPATH in typical CI setups.
		$site_health      = $this->setup_site_health_with_debug_properties( true, true, null );
		$private_log_path = '/var/log/php-error.log';

		ini_set( 'error_log', $private_log_path );

tests/phpunit/tests/admin/wpSiteHealth.php:962

  • This test hard-codes a “private” error log path under /var/log, which can break on platforms where that path doesn’t exist (e.g. Windows), causing realpath(dirname(...)) to return false and the Site Health result to become “error/critical”. Use sys_get_temp_dir() to keep the test portable.
		$site_health      = $this->setup_site_health_with_debug_properties( true, false, null );
		$private_log_path = '/var/log/php-error.log';

		ini_set( 'error_log', $private_log_path );

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants