Closed (fixed)
Project:
Security Review
Version:
2.0.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
9 Aug 2011 at 19:08 UTC
Updated:
28 Nov 2022 at 15:04 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
Justin_KleinKeane commentedAttaching a patch that will check for cron running in the last 24 hours.
Comment #2
gregglesBetter status.
I didn't test or look at the code much.
One nitpick is that comments should end with a period.
Comment #3
smustgrave commentedAs Drupal6 has been EOL https://www.drupal.org/about/drupal6-eol closing as outdated
Comment #4
gregglesThis one also feels nice to keep in modern versions as cron is still important, I believe, for security.
Comment #5
smustgrave commentedNot sure what to check though. Each project runs cron at different intervals. Or if you use ultimate cron some cron jobs run at different times.
Comment #6
gregglesIf the dblog module is enabled and if there are messages in the table about cron that seems like enough to me?
I guess a more robust solution is this module could have its own cron task that sets a variable to the last time it ran and then during review phase if that variable is too old it would alert that cron is not running?
Comment #7
smustgrave commentedwhat do we consider too old though? That could be different per site. 24hours?
Comment #8
smustgrave commentedComment #9
gregglesFor purposes of the module I think the really bad situation would be if cron hadn't run in a long time (days, weeks, etc.). Seems good to check it has run in the last ~72 hours?
If we use 24 and a site has cron running once a day then there's a risk if cron doesn't run for 1 day or is late that it would get flagged as a problem when that's not really a problem. It seems good to balance the security and not sending too many error messages.
Comment #10
smustgrave commentedTry this.
Comment #11
gregglesThe quotes (double vs single) are a little inconsistent on the user-facing strings here:
Otherwise LGTM!
Comment #12
smustgrave commentedUpdated.
Comment #13
gregglesLGTM. Thanks.
Comment #16
smustgrave commented