Opened 8 weeks ago
Last modified 3 weeks ago
#47985 new enhancement
Site Health: log errors to public file
Reported by: | afragen | Owned by: | |
---|---|---|---|
Milestone: | 5.4 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Site Health | Keywords: | has-patch dev-feedback |
Focuses: | Cc: | ||
PR Number: |
Description
As mentioned in #47046 just because WP_DEBUG_LOG
is true doesn't mean the file is publicly viewable.
Obviously viewable files are in the ABSPATH
usually in wp-content
somewhere. A simple test against ABSPATH
could be used to move the error from critical to recommended. It still shows as a potential problem but possibly not as severe as an average user might infer.
Attachments (4)
Change History (23)
This ticket was mentioned in Slack in #core by afragen. View the logs.
7 weeks ago
This ticket was mentioned in Slack in #core-site-health by afragen. View the logs.
7 weeks ago
This ticket was mentioned in Slack in #core by afragen. View the logs.
6 weeks ago
#5
@
6 weeks ago
- Keywords dev-feedback added
The patch is pretty straightforward. What it really needs is a #committer to see if the logic is sound.
#6
@
6 weeks ago
There's no way to reliably know if a path is accessible to others in any way, so I stand by my previous opinion that if logging is enabled, it should alert the user, they are the only ones that can know if it's available or not.
#7
@
6 weeks ago
@Clorith my thought was that if the log is not in the normal flow of the WordPress structure then the notification should be less severe.
If the log isn’t in the normal WordPress flow it would be much more difficult to randomly find it.
This ticket was mentioned in Slack in #core-site-health by afragen. View the logs.
6 weeks ago
This ticket was mentioned in Slack in #hosting-community by miss_jwo. View the logs.
6 weeks ago
This ticket was mentioned in Slack in #core by afragen. View the logs.
6 weeks ago
#11
@
6 weeks ago
@miss_jwo reached out in #hosting_community for feedback -- putting that in here as well, so it's easier to find:
I think it’s a good idea to alert folks if they have logging enabled, whether or not the log is public, since a log filling up disk is a real concern, if they’re not handling rotation and such.
It’s possible that it’ll slow down the site due to writes, too.
#12
@
6 weeks ago
@mikeschroder it does register an alert. It simply changes the alert from critical to recommended.
This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.
5 weeks ago
This ticket was mentioned in Slack in #hosting-community by mike. View the logs.
5 weeks ago
#16
@
5 weeks ago
@afragen Is there a place where I could see what the severity change would look like in practice? What would it mean for users to change it?
#17
@
5 weeks ago
@mikeschroder I discovered that the previous patch didn't work as expected. I've updated the patch to be as specific as possible. The screenshot is what happens when the patch is applied and WP_DEBUG and WP_DEBUG_LOG are both true if the log is outside of ABSPATH.
Otherwise the error shows as critical.
To test you would need to add something like the following line above the patch.
ini_set( 'error_log', '/tmp/debug.log' );
test error_log file path to ABSPATH