Draft text for security advisory coverage messages

I had a stab at a mockup and added it to the Issue Summary and I just I took a guess at which page that notification should link to for "more information.

Rationale for placement.

This text is about the maintainers of the project - so it made sense to me to post it under the maintenance status

Another option here might simply be add a new category: "Maintainance status: Actively maintained by novice maintainers"

A third alternative could be in the sidebar. I'll have a go at that next.

Here's a screengrab with that hacked in via inspect element

Which location works best for this notification?

now - let's get a few more eyeballs here, shall we?

Needs review.

Alternative for the Project information section.

It may be easier to insert the notification above the UL rather than in the middle of it.

Why is this needed at all?

When it was discussed in #2453587: [policy, no patch] Changes to the project application review process as far as I recall it was only in the context of warning people that the project doesn't get security team support, e.g. from that issue summary:

The project page will have a visible indicator that the project is maintained by a non-vetted user, including text that the project is not subject to Security Advisories.

But then we realized that warning about every project that doesn't get security advisories would be better; there is no reason to limit it to non-vetted users.

So what's the reason for wanting to add an additional message?

I think it will be very hard to come up with wording here that doesn't come across like "this person probably writes bad code", which is definitely not the message that should be sent.

If we stick with this, I'd expect this to be in the main content area, not the sidebar.

Maybe this should be flipped around to add a positive indication that maintainer(s) are vetted?

Maybe this should be flipped around to add a positive indication that maintainer(s) are vetted?

That would be better if every vetted git user had actually been vetted, but most of us were grandfathered in from the old 'ask for cvs access in irc and get it' system.

Sitting in a room at NOLA with jthorson talking about this:

Here's some suggested text - with the goal of being more concise and not using too much coded language.

This project is not eligible for stable releases, because the maintainers have not yet been through Drupal.org’s volunteer peer review process.

Per @kattekrab, I think the sidebar under maintainers makes sense. The blue background seems good (although eventually it'd be great to have @DyanneNova take a pass at the whole page).

I'd also suggest that the link to "For more information...." should go on a page that the maintainers see, since that info is more valuable to them than to the user evaluating a project - so something like the create release page.

Added a new issue for providing a message to maintainers about how to become vetted to edit releases: #2721251: Update security advisory policy for project coverage option

@hestenet - yes to all of that! :)

Per the recent updates to the Project Application Revamp proposal, the kind of messaging we need can be, I think, much clearer.

Because we are now going to separate the ability to make releases from the ability to opt into security coverage, the signal we should send here can be much clearer.

Here are two mock-ups of a project with security coverage, and one without:

The shield icon strongly indicates coverage, and the text next to it can link off to information about the security team.

The shield icon in dotted red draws attention, and the text warns users of the risks of using a project without security coverage.

I am somewhat concerned about how strongly the signal will come through for the visually impaired, so we should take some time to think about these messages with respect to accessibility. Hopefully the text itself will be sufficient for users of a screen reader.

For #786702: Add information about releases covered by security advisories to project pages near download table, I over-thought this and coded up checking if there are full releases currently showing on the project page. Turns out that isn’t needed right now. In case it is in the future:

    // Get “Recommended” API compatibilities that can be shown on the project
    // page.
    $query = new EntityFieldQuery();
    $recommended_term_result = $query->entityCondition('entity_type', 'taxonomy_term')
      ->fieldCondition('field_release_recommended', 'value', 1)
      ->execute();
    // Get “Supported” release NIDs in supported branches.
    $recommended_releases = db_query('SELECT recommended_release FROM {project_release_supported_versions} WHERE nid = :nid AND supported = 1 AND tid IN (:tids)', [
      ':nid' => $node->nid,
      ':tids' => array_keys($recommended_term_result['taxonomy_term']),
    ])->fetchCol();
    // Check if those releases have extra versions, like alpha42 or dev.
    $query = new EntityFieldQuery();
    $with_extra_release_count = $query->entityCondition('entity_type', 'node')
      ->entityCondition('bundle', project_release_release_node_types())
      ->propertyCondition('status', 1)
      ->propertyCondition('nid', $recommended_releases)
      ->fieldCondition('field_release_version_extra', 'value', '', '<>')
      ->count()->execute();
    if (count($recommended_releases) - $with_extra_release_count > 0) {
      // If there are full releases, recommended without extra.
      $output['items']['security']['#markup'] = t('yep');
    }
    else {
      // No full releases.
      $output['items']['security']['#markup'] = t('nope');
    }

Here’s my take on the implementation for covered projects:

That text is:

Stable releases receive coverage from the Drupal Security Team.
Look for the shield icon below.

I rewrote the text a bit to be more concise, link to more information (too many links?), and removed mention of the downloads specifically being a table.

I would prefer a more conservative wording regarding what the sec team actually does. We don't really "provide coverage".

We get the maintainers to fix issues once somebody else finds them and release advisories.

"providing coverage" would IMO imply that we review all the releases. We don't do that.

I think we need to remove the stable release coverage message if no releases are stable. It should be replaced with a message.

The problem with saying "coverage" is that https://www.drupal.org/security-advisory-policy the page it links to does not have the word "coverage" at all. So no clear definition for what this word means.

I'm not sure if anybody missed this on purpose, but, need to put my 5 cents here

After having a conversation with Steve https://twitter.com/stevepurkiss/status/770920061192273921

The main concern is that you are creating a "Security coverage" for software. But software is not vulnerable. This is a main issue of shifting point of view. We see the same in Gun Control around the world. The guns are not violent, the people do.
The same rule works about a software. And I'm relying here not only on gun control, but on code review process from our company.
By blaming(or making it starred with some nice "It is secured" badge) software we are loosing the main point - bugs and security issues introduced by people.
And only by marking that, somehow, for example - just put the number of security issues to the profile of the guy - and all should be fine.
Otherwise we'll get a risk of corruption, lack of responsibility ( we have now security team, that cares about, why bother? )

I know, there are a lot of cultural differences, but playing with security and removing personal responsibility is wrong.
I'm from Ukraine, I know what I'm saying...

@25 "If a vulnerability is discovered by the Drupal Security Team it is included in our security advisory policy"

This is just the wording I would want to avoid, since it IMO implies that the team is actively searching for vulnerabilities. It is not and people need apparently to understand that better.

re #28

I apologise if my wording is not quite well, I'm looking mostly from marketing and big picture points of view....

> for a whole load of reasons
by hiding those reasons we can head to the community where authority means more than a real security and quality. In some cases this could be fine, but I'm worrying about long term...

> a big PITA!
thanks for that.

> When it comes to marketing speak, it is all nice to have comforting icons,

If this is topic is about marketing, than would be awesome if the resulting message and look is also reviewed by the technical guys. Because all these nice and pretty-kitty icons over 80+ modules that I'm maintaining right show totally wrong story.
Nobody contacted me about review process,
The modules are not secure, period. The only icon that could be set there is - reviewed by security team XX days ago.

re #29
> This is just the wording I would want to avoid, since it IMO implies that the team is actively searching for vulnerabilities. It is not and people need apparently to understand that better.

Agree with this ^^^

#33 like

Newcomers to this issue should also make sure to check out #786702: Add information about releases covered by security advisories to project pages near download table where initial discussions about the best way to word these messages happened.

@jthorson maybe my understanding of the finer points of the meaning of the word "coverage" is simply wrong. ;)

If that's so, a simpler word should be chosen *and/or* the meaning should be explained on the page.

To me, this issue and the one I linked to has pointed out a general misunderstanding by the wider Drupal community of what exactly the security team does for contrib. My anecdotal experience has been that only people that have worked with the sec team on an issue in one of their contrib modules really understand the whole process.

This text and the docs it links to are a great opportunity to help educate the rest of the community about what coverage means.

I linked that other issue to show that this has been long-discussed and debated, this issue makes it seem more like it fell from the sky one day without the customary bikeshed.

Ok, let's recall the facts.

The security process (in general):

1. Someone finds the security issue and reports it to the security team.
2. Security team resolves that issue and posts the advisory.

Conclusions.

The message should be obvious and clear to all-leveled users. Here's an example of the message that mirrors the actual security process:

Reported security issues in releases marked with the shield icon are reviewed and resolved by the Drupal security team.

If you find a security issue in one of these releases you are encouraged to report it directly to the Drupal security team [link].

If you find a security issue in any other release please submit a [public] issue in this project's issue queue [this is only if project has issues enabled].

P.S.: I'm not sure whether the security team reviews the whole project. I always thought that they review only the reported issue, no more. And in that case we cannot display "when" the project was last reviewed by the security team, because it's only one issue.

We don't actually resolve the issue. We try to get the maintainer to do that and review the proposed solution.

And yes, we do *not* review the whole project.

We may check whether an issue is applicable to several branches/versions (D7, D8, ...) of a projects.

If you find a security issue in any other release please submit a [public] issue in this project's issue queue [this is only if project has issues enabled].

If there is any full release, it is best to report to security.drupal.org. Issues in a non-full release are likely enough to also be in the full releases. Being overly-cautious here is good.

This bit of text doesn’t need to cover reporting issues. There's the “Report a security vulnerability” link in the sidebar and the create issue form also mentions security. This is targeted toward general people building sites.

Adding “security advisory”, like jthorson reminded us of, makes sense to me:

Stable releases receive security advisory coverage from the Drupal Security Team.
Look for the shield icon below.

Sure, “coverage” is still there, but it applies to advisory instead of Team. I probably edited it down a bit too ruthlessly in trying to be more concise.

Indeed, security advisory coverage aligns much better with what is actually happening vs. security team coverage which is not a term defined outside of this message. At least while coverage is still not defined elsewhere, security advisory is :)

I committed that change to the dev branch, which will be deployed later today.

I suggest tweaking it to something like:

Stable releases are covered by the security advisory policy.
Look for the shield icon below.

or:

Stable releases get security advisories if a vulnerability is found.
Look for the shield icon below.

(These are similar to some of the text we discussed originally at #786702: Add information about releases covered by security advisories to project pages near download table a few years ago.)

I don't see a need to mention "Drupal Security Team" here (and especially not to link to it, since there should really only be one link for the user to focus on). But the page that is linked to (https://www.drupal.org/security-advisory-policy) would definitely benefit from a "What is a security advisory?" paragraph at the top that explains in simple language what a security advisory is and how that relates to the Drupal Security Team.

Also (and as others noted above) the current project page text is unclear in the case where the module has no stable releases at all. It tells you to look for the shield icon below, but there is none below; however there is still a shield icon next to the text you're reading. So overall it's possible to get confused about whether that project is covered by the security policy or not. To improve that (and assuming the big red icon thing from #20 isn't happening), it would be nice if "Look for the shield icon below" were to be replaced with "This project does not have any stable releases" in that scenario. (This could be a separate issue if necessary, since it's more than just a simple text change.)

I had a similar thought after I posted my last comment. I committed the first option and will be deploying shortly.

Merging this current draft from the issue summary, with an added link, into #2787165: Add security advisory coverage field to projects

This project has not opted into coverage from the Security Team.
It may have publicly disclosed vulnerabilities. Use at your own risk!

Since the first half of the messaging has been deployed, any further changes can be discussed there.

I took a stab at editing https://www.drupal.org/security-advisory-policy to explain what a security advisory is (as I suggested above in #48).

Here is the change I made: https://www.drupal.org/node/475848/revisions/view/9776727/10218686