Integrate with site_audit

@FluxSauce excellent, this looks pretty good! Some of the check results have specific array structures which aren't standardized so getResultFail() outputs some $values in the report as Array.

I'm working off your patch to see if I can leverage the check help functions for standardizing the structure so that Site Audit can use it consistently. My anonymous arrays are really becoming a pain in the ass here :(

  $items['audit_security'] = array(
    'description' => dt('Render a Security Review with Site Audit.'),
    'aliases' => array('as'),

The "as" alias is already used for the audit_status command. May I suggest "asec".

It would also be nice to have the details text (from /admin/reports/security-review/help/security_review/...) available in the report.

I would also suggest limiting the number of files listed for "File system permissions" unless the --details option is passed.

+1 for asec

Hello,

I want to test the patch but I have the following error Call to undefined function site_audit_common_options()

If I understand, site_audit is a drush extension ?

Is there a way to add something like

if (module_exists('site_audit')) {
  // The new drush command here.
}

but for drush. Is there a function like "module_exists" but for drush command ?

Even with site_audit downloaded in ~/.drush/site_audit, I can't use drush cc all and any other drush command so I commented the new drush command.

Hello,

I test the patch. You forgot to add the file security_review.site_audit.inc

So I use the one in patch in comment number 1. And I remove the class SiteAuditReportSecurity extends.

And It works fine !!!

Thank you very much.

I attached a new patch with the modifications.

I used drush aa --skip=insights --html --detail --bootstrap > site-audit-report.html and got the latest code from both side audit and security review.

When running via Drupal I got "Drupal installation files and directories (except required) are not writable by the server." but running via Drush I got "Some files and directories in your install are writable by the server." Then it listed every file in my Drupal install. This must be because I'm running drush as my user account, which owns all the files.

For text formats check I got "Untrusted users are allowed to input dangerous HTML tags. -Array"

For drupal permissions I got "Untrusted roles have been granted administrative or trusted Drupal permissions. -Array"

When running via Drupal I got "There are Views that do not provide any access checks." but via drush there was no report for views.

@FluxSauce this is looking great! You probably have a better solution in the works, but for now I just convert the nested arrays to nested unordered lists (or indented strings). The results aren't the most beautiful, but works consistently:

I've tested this and it works well. My environment:

• PHP 5.4.39
• Drush 7.0.0
• security_review-7.x-1.x at 3141b79
• site_audit-7.x-1.x at 5b8f832
• Run on a local copy of a Pantheon drops-7 site.

... I see the Menu Router, File system permissions, Text formats, Content, Error reporting, Private files, Allowed upload extensions, Drupal permissions, Executable PHP, Drupal base URL, and Temporary files checks, and the output from site_audit matches the output from the security_review UI on the same site.

***

There are a few coding standards errors and DrupalPractice warnings in the new file, security_review.site_audit.inc: you can see the output from these two commands in the attached text file, 2279283-16-coding-standards.txt. Note there are existing coding standards and DrupalPractice issues in the security_review.drush.inc file, BUT not in the code added in this patch (i.e.: this patch does not introduce problems in that file).

***

Once the coding standards issues are fixed, this is RTBC from me.

Great!

Looks like the patch in #18 meets all Drupal 7 coding standards and best practices.

This is RTBC from me now.

If you want some superficial documentation standards feedback:

/**
 * Given a nested array, generate a unordered list, or text-only equivalent.
 *
 * @param $array
 * @param bool $html
 * @param int $indentation
 * @return string
 */

- Verb in summary sentence should be third-person singular present tense (i.e., "generates").
- Missing param type "array" for $array. It is obvious given the variable name but I think it is supposed to be indicated.
- Supposed to be blank line (beyond asterisk) between @param and @return. ("Some tags also trigger paragraph breaks: @param, @return, @see, @var.")
- Grammar: "a unordered" -> "an unordered"

See: https://www.drupal.org/coding-standards/docs

Thanks, @FluxSauce! This works great in my testing! Committed. :-)

Thanks!

Actually, I just noticed that there's a D8 version of site_audit. We should probably port this to the 8.x branch of security_review! Updating issue status...

Help would be appreciated! :-) There's some other things in the queue I was planning to work on before circling back to digging into this.

Neither 7.x-dev or a stable release since the commit to D7. Is a release with this far away?

It's been a really long time since the last release, and there's some good improvements in 7.x-1.x-dev... I'll discuss with the other maintainers!

I've rolled a version of @FluxSauce's patch against the 8.x-1.x branch of Security Review. This enables integration between Site Audit 8.x-2.x-dev
and Security Review 8.x-1.x-dev.

The patch needs to be updated to Site Audit 8.x-3.x version. I also hide the previous patch.

any update on this for 3.x site audit version?

If this is still a valid feature request think we will need a reroll.

Reroll of #31 for 2.0.x.

Did you check to see if this integrates with site audit? They appear to use plugins now and don’t think this solution works. Also there are additional tests that were excluded.

Is this still a desired feature?

Closing out. If still a desired feature please reopen.