Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hbhalodia wants to merge 8 commits into
base: trunk
Choose a base branch
from
+28 −10
Open

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

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f99e009
CoreTrac-64071 Show different warning if debug.log is publicly access…
hbhalodia Jan 6, 2026
bcebdf8
CoreTrac-64071 Update the wordings and code structure
hbhalodia Jan 6, 2026
3749fef
CoreTrac-64071 Add the directory separator to absolute path to preven…
hbhalodia Jan 8, 2026
8636002
CoreTrac-64071 Update wordings in message to show in site-health for …
hbhalodia Jan 8, 2026
ffda9d8
CoreTrac-64071 Check the directory of log instead of checking file
hbhalodia Jan 9, 2026
b98ece6
CoreTrac-64071 Fix phpcs error
hbhalodia Jan 9, 2026
2da8e22
CoreTrac-64071 Update the debug_log_path to use error_log config
hbhalodia Jan 9, 2026
97e37bd
Merge branch 'trunk' into fix/issue-64071
hbhalodia Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 29 additions & 10 deletions src/wp-admin/includes/class-wp-site-health.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1409,18 +1409,37 @@ public function get_test_is_in_debug_mode() {

if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
if ( defined( 'WP_DEBUG_LOG' ) && WP_DEBUG_LOG ) {
Copy link
Member

@westonruter westonruter Jan 9, 2026

Choose a reason for hiding this comment

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

Should this be changed?

Suggested change
if ( defined( 'WP_DEBUG_LOG' ) && WP_DEBUG_LOG ) {
if ( ! empty( ini_get( 'error_log' ) ) ) {

Copy link
Author

@hbhalodia hbhalodia Jan 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Amm, I thought so as well, but I have kept it as is. This is because the current condition will only provide us if WP_DEBUG_LOG is set or not, and that is enough here for a check. Ultimately we are using ini_get( 'error_log' ) to get the error_log config.

IMO, there is no harm to change this, and also there is no harm to keep as is, as ultimately the output that we need would be same.

Also if we need to consider a performance check, IMO reading from constant would be faster, then calling a function for the same and then toggling the condition via ! empty.

So for first thing, defined( 'WP_DEBUG_LOG' ) && WP_DEBUG_LOG a simple constant hashlookup, while for this, ! empty( ini_get( 'error_log' ) ), a function call to check for value, then check for empty and then toggle condition. Still it's very negligible difference.

I am okay to accomodate the above change, but there is no requirement in it unless you see something specific?

Copy link
Member

@westonruter westonruter Jan 9, 2026

Choose a reason for hiding this comment

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

The performance here doesn't matter since it's only when the Site Health tests runs, right? It's not running with request.

My concern is if a server is misconfigured to have an error_log wet to somewhere in the document root (ABSPATH), even if WP_DEBUG_LOG isn't being set.

Copy link
Author

@hbhalodia hbhalodia Jan 9, 2026

Choose a reason for hiding this comment

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

The performance here doesn't matter since it's only when the Site Health tests runs, right? It's not running with request.

Yes it won't have much impact related to performance, but thought to raise. We can neglect this.

My concern is if a server is misconfigured to have an error_log wet to somewhere in the document root (ABSPATH), even if WP_DEBUG_LOG isn't being set.

If I am understanding correctly, you are saying anyhow someone have set this debug file to use document root, that is, from somewhere they call this, ini_set( 'error_log', 'path-to-document-without-wp-debug-log-constant' ) without explicitly setting the constant value, So in this case, our condition would fail if ( defined( 'WP_DEBUG_LOG' ) && WP_DEBUG_LOG ) {, and it would show the false information.

But that could ever happen? Because let's say WP_DEBUG_LOG is not set and somehow the error_log with document root path is configured, then we are never outputing user that you have enabled debug_log as good practice, instead we would say the default message, Your site is not set to output debug information, though this is misleading as error_log is configured and we need to provide information that it is good practice or not based on the path.

Also, if we use ! empty( ini_get( 'error_log' ) ) it as a condition, the point would again be misleading because we would show whatever message need to show based on path, but we are saying to user that, 'The constant, WP_DEBUG_LOG .....' is set but that is not set as per the edge case we are assuming.

Copy link
Member

@westonruter westonruter Jan 9, 2026

Choose a reason for hiding this comment

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

Consider this case: someone may not define WP_DEBUG_LOG at all, but their wp-config.php may have this:

ini_set( 'error_log', __DIR__ . '/error-log.txt' );

This would not be detected as a problem currently in Site Health test, but it would be a similar problem, as if there are any error_log() calls being made, they'll go to that public file. The same issue would happen if the php.ini was set with an error_log set to a public location, right?

Copy link
Author

@hbhalodia hbhalodia Jan 9, 2026

Choose a reason for hiding this comment

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

Thanks @westonruter, Yes agreeing to the point, Concerning part here is below message to me,

$result['description'] .= sprintf(
	'<p>%s</p>',
	sprintf(
		/* translators: %s: WP_DEBUG_LOG */
		__( 'The constant, %s, 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.' ),
		'<code>WP_DEBUG_LOG</code>'
	)
);

It say's 'The constant, WP_DEBUG_LOG, has been added to this website’s configuration file', but as per the example shared, the WP_DEBUG_LOG is not actually being set, the error log is added by this statement, ini_set( 'error_log', __DIR__ . '/error-log.txt' );, So we need to change the statement here to more generic.

Ideally if I am a user and see's this message, I will first check this constant, WP_DEBUG_LOG, and I would not able to find how this is set (but actually that is not set as per example), hence I would not get an idea that it was set from server or somewhere else.

Copy link
Member

@westonruter westonruter Jan 9, 2026

Choose a reason for hiding this comment

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

Agreed. We could have a separate message when the WP_DEBUG_LOG constant is set, and then another general one about the error_log being misconfigured.

$result['label'] = __( 'Your site is set to log errors to a potentially public file' );
$debug_log_path = WP_DEBUG_LOG === true ? WP_CONTENT_DIR . '/debug.log' : WP_DEBUG_LOG;
$debug_log_path = realpath( $debug_log_path );
Copy link
Member

@westonruter westonruter Jan 8, 2026

Choose a reason for hiding this comment

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

Suggested change
$debug_log_path = realpath( $debug_log_path );
$debug_log_path = realpath( dirname( $debug_log_path ) ) . DIRECTORY_SEPARATOR;

Copy link
Author

@hbhalodia hbhalodia Jan 9, 2026

Choose a reason for hiding this comment

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

Yes, that could be done. Then I do not think so to check this ini_get( 'error_log' ) === WP_CONTENT_DIR, just check for directory what it is in and show the details. As because, we are always checking with WP_CONTENT_DIR in debug.log

I will update the PR with the requested change.

$absolute_path = realpath( ABSPATH );
Copy link
Member

@westonruter westonruter Jan 8, 2026

Choose a reason for hiding this comment

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

This should help ensure that there isn't a false positive of there being another sibling directory name with the same common prefix.

Suggested change
$absolute_path = realpath( ABSPATH );
$absolute_path = realpath( ABSPATH ) . DIRECTORY_SEPARATOR;

Copy link
Author

@hbhalodia hbhalodia Jan 8, 2026

Choose a reason for hiding this comment

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

Updated, Thanks.


$result['status'] = str_starts_with( ini_get( 'error_log' ), ABSPATH ) ? 'critical' : 'recommended';
if ( $debug_log_path && $absolute_path && str_starts_with( $debug_log_path, $absolute_path ) ) {
$result['label'] = __( 'Your site is set to log errors to a potentially public file' );

$result['description'] .= sprintf(
'<p>%s</p>',
sprintf(
/* translators: %s: WP_DEBUG_LOG */
__( 'The value, %s, 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 potentially available to all users.' ),
'<code>WP_DEBUG_LOG</code>'
)
);
$result['status'] = 'critical';

$result['description'] .= sprintf(
'<p>%s</p>',
sprintf(
/* translators: %s: WP_DEBUG_LOG */
__( 'The value, %s, 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 potentially available to all users.' ),
Copy link
Member

@westonruter westonruter Jan 8, 2026

Choose a reason for hiding this comment

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

Suggested change
__( 'The value, %s, 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 potentially available to all users.' ),
__( 'The constant, %s, 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.' ),

'<code>WP_DEBUG_LOG</code>'
)
);
} else {
$result['label'] = __( 'Your site is set to log errors to a file outside the public directory' );
Copy link
Member

@westonruter westonruter Jan 8, 2026

Choose a reason for hiding this comment

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

Suggested change
$result['label'] = __( 'Your site is set to log errors to a file outside the public directory' );
$result['label'] = __( 'Your site is set to log errors to a file outside the document root' );


$result['status'] = 'good';

$result['description'] .= sprintf(
'<p>%s</p>',
sprintf(
/* translators: %s: WP_DEBUG_LOG */
__( 'The value, %s, has been configured to write errors to a file outside the WordPress directory. This is a good practice as the log file is not publicly accessible.' ),
Copy link
Member

@westonruter westonruter Jan 8, 2026

Choose a reason for hiding this comment

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

Suggested change
__( 'The value, %s, has been configured to write errors to a file outside the WordPress directory. This is a good practice as the log file is not publicly accessible.' ),
__( 'The configuration constant, %s, has been set to write errors to a file outside the WordPress directory. This is a good practice as the log file should not be publicly accessible.' ),

'<code>WP_DEBUG_LOG</code>'
)
);
}
}

if ( defined( 'WP_DEBUG_DISPLAY' ) && WP_DEBUG_DISPLAY ) {
Expand Down
Loading