Why should this issue be prioritized:
Amendments to the project applications process will unblock significant roadblocks to on-boarding new contributors, and remove a long-standing pain point for new entrants to the Drupal community.
Summary of Proposal (500 words or less):
After numerous false starts in the attempt to revamp the existing project applications process for obtaining permission to create ‘full projects’ on Drupal.org (largely due to a lack of clarity around governance and ownership of decision making surrounding the issue), the Technical Working Group coordinated with the Drupal.org Software Working Group and Drupal Security Team to come up with a proposed ‘next step’ for the project applications process. The proposal, outlined in #2453587: [policy, no patch] Changes to the project application review process, attempts to strike a balance between removing roadblocks to contribution for new contributors, and maintaining the community’s reputation for high standards of quality with regards to both code and security coverage for contributed projects listed on Drupal.org.
A proposed implementation plan is currently being circulated for comments amongst the TWG and Security team. Constructive comments and feedback welcome. https://docs.google.com/document/d/1eaI0Jq5CIVClCHv4uAxbJVHky5twPkWOqJ-A...
Constraints on the Solution
- We need to remove the gate to new contribution entirely - not just kick the can to a particular elevated role, or a specific limit on the # or kind of releases a new contributor is allowed.
- We need to continue to send strong signals about security coverage to users evaluating whether to use modules from Drupal.org.
- Follow-up: We need to find ways to preserve the value collaborative code review, through changes to Project Discovery to provide signals about code quality, and by providing incentives and credit for review.
After much discussion among many parties, including: @kattekrab, @dries, @mlhess and the security working group, myself, and users who have commented in these issues - we have found a solution. This is based on the initial proposal and work put together by @jthorson, adjusted to meet the above constraints more strongly.
Solution: High Level overview:
- The git user role (which any confirmed user can get by agreeing to the terms) will be able to:
- Create any number of projects (we will need a better policy for abundant namespaces)
- Create any releases for those projects
- All new projects will have a new option to opt in to security team coverage.
- Those that do not, will get a warning message on the project page and on the update status page for their sites. (this will require core updates for the update status)
- A role is required to opt in to security team coverage. We can repurpose the “git vetted user” role to “Opt into security team coverage”
- For now, to get the new “Opt into security team coverage” role, we will use basically the current project application process - however instead of being a gate on new project contribution, it is only a gate on participating in security team coverage.
- Because this opt-in process is now decoupled from the project creation/release process and is wholly in the domain of security coverage it can be replaced by a coding standards quiz or whatever the SecWG decides will best manage their work, sometime in the future.
Solution: User stories/issues:
Phase 1: Preserve the signals about project that give Drupal its security reputation:
- #2035277: Draft text for security advisory coverage messages
- #786702: Add information about releases covered by security advisories to project pages near download table
- #2457643: Only allow releases with security coverage to be recommended
- #2787165: Add security advisory coverage field to projects
- Mass update all existing full projects with stable releases to indicate they are 'covere'd ^^ in the new field
- Allow security team members to update the security advisory coverage field on projects
- #2853696: Add security advisory coverage to update status XML
- #2766491: Update status should indicate whether installed contributed projects receive security coverage, needs backport to D7
Phase 2: Set up the opt-in process
- #2825912: Rename “Git vetted user” role to “Opt into security advisory coverage” <- security team can set up their opt in process to receive that
- Allow node owners of projects with the 'May opt-in...' role to update the advisory coverage field to opt-in.
Phase 3: Open the gates!
- Allow git-users to create full projects and releases* <-- this opens the gates
- Update all appropriate documentation - Please help us with this!
- Followups
- #2859739: Make the “not covered” security warning more visible on project pages
- #2858261: Allow any vetted maintainer, not just project owner, to opt into security coverage helps organization-account owned projects
- #2860311: Remind maintainers what a full release is when creating one
- #2859696: Auto-delete projects without code to help with namespace spam
Community Team:
jthorson TWG liason
mlhess
kattekrab
Drupal Association Liaison(s):
Availability:
DA has prioritized time in Q1 2017
Long term support:
The DA and security team have agreed to support the process outlined in the high-level overview/user stories above
Related issues
Meta / background
'Value Add' tasks
Long term 'value add' tasks to consider
- Introduce Automated Scans for security and coding style #1266444: DrupalCI should return failed for poor code style - php
- Display scan results on project pages #2589853: Integrate PHP CodeSniffer test with drupal.org
Comments
Comment #2
hestenetComment #3
jthorson CreditAttribution: jthorson as a volunteer commentedComment #4
fuzzy76 CreditAttribution: fuzzy76 commentedAs one of the most vocal complainers on the project application process, I just want to say that I love this suggestion. :) It also has the added benefit of making existing / old projects more aware of their shortcomings when it comes to publishing stable releases and following the guidelines.
Comment #5
hestenetThis was communicated somewhat in IRC, but wanted to comment here for a more permanent record:
This proposal has been added to the initiatives under consideration:
https://www.drupal.org/drupalorg/roadmap/community-initiatives
And we're definitely looking forward to this one. We're looking forward to see who in the community is excited to help out with this as the plan gets circulated and starts coming together.
Comment #6
dasjoMuch like
Comment #7
pingwin4egAdded a comment/suggestion to the doc, and duplicating it here for consistency.
There's a project promotion process in the Desired Outcome section. And currently it is:
My comment:
Wouldn't it be better to move this item ["Choose a namespace for the project"] upper in a list? So maintainer would be able to "reserve" a name for the project before actually pushing the code to repo. This reservation can be temporary (say for 1 month) to avoid namesquatting. But this will ensure a maintainer won't rewrite the project code in case the project name is taken by someone else in the end of this process.
Comment #8
almaudoh CreditAttribution: almaudoh commented#7: +1
Comment #9
fuzzy76 CreditAttribution: fuzzy76 commentedI have a hard time imagining a "v1.0 - first take" of a module that can't be renamed in 10 minutes.
Today a project might spend 2 months in the application queue. Has it ever been a huge problem that the module name was taken by the time the module is approved?
Comment #10
gisleAs much as I would love to like this proposal, I still think the "automated scan" that is proposed as mandatory step for promoting a sandbox to a full project is problematic. (By all means provide this as an option - and if it is based upon something less brain-damaged than Coder I may even take advantage of such an option - but please do not make this mandatory.)
If you make it mandatory, here is what I think the process for promoting a sandbox to a full project status will look like after this mandatory measure has been introduced:
I.e. for somebody who wants to game the system, the "automated scan" is not much of a speed-bump. So we should not delude ourselves to think that this is some sort of a gatekeeper.
Automated scans (when done well) are great QA tools for honest developers (so please offer it as an option). But automated scans are useless as a gatekeeper to shut out dishonest developers. Pretending it can be used as a gatekeeper is simply misleading, so it should not be done.
IMHO the revamped process would be more transparent if the automated scan of the repo was made optional instead of mandatory.
Comment #11
klausi@gisle: ouch, "braindamaged coder" hurts my feelings. Any particular criticism you can share?
Comment #12
gisle@klausi,
let me first say that I understand the rationale behind the use of Coder in the PAreview workflow. The current PAreview process is a manual one. As a sometimes manual reviewer, I know that it is much harder to review non-standard code. When visually inspecting code for vulnerabilities, one scans for "patterns" that "look wrong" and that may need closer inspection. With non-standard code, all of it "look wrong", so it is much more time-consuming to do a manual review of non-standard code.
IMHO, Coder should be tolerated as a tool that ensures that code created by inexperienced coders is cast in a format suitable for manual review.
However, I've noticed that Coder is going to be used for something completely different in the PAReview process revamp - and this is the use I commented on here.
IMHO, it is simply not clever enough to be used as an automated gatekeeper for project promotion. When I used the word "brain-damaged" I was specifically addressing its usability outside the use-case it was created for (preparing code for manual review as part of the PAReview workflow).
Here, in five short points, is why I think Coder is unsuitable for use as a gatekeeper:
Comment #13
jthorson CreditAttribution: jthorson as a volunteer commentedRe: #7:
Point taken, in that namespace reservation would be a nice enhancement. This said, the proposed approach also considers ease of implementation given today's existing logic and code ... I'd have concerns that creation of a new namespace reservation process might be quite disruptive relative to simply adjusting the existing logic and associated code.
Re: #10:
A certain number of dishonest peeps will exist in any large community; but I think it's important to point out that this is not intended as a 'policing' system (which, unfortunately, the existing process started to look like). I'd suggest that those who are intentionally or maliciously bypassing the automated tools will continue to be dealt with reactively on a case-by-case basis, the same way they are in other areas of Drupal.org.
EDIT: Also, I should elaborate ... once they are built, I would anticipate the automated scans at project creation would also be run on every commit/push - thus eliminating any perceived value in gaming the system to avoid the initial scan.
Comment #14
gisleWell, if failing automated scans results in a sandbox project being barred from being promoted to full project status, it is a policing system, no matter what you call it.
While I haven't tested this, I would not be surprised if the majority of the non-trivial projects currently hosted on Drupal.org would fail automated scans by Coder. If this assumption is correct, the impact is going to be huge if you plan to do something as a result of an existing project failing automated scans.
What do you plan to do in the event of an existing project failing the automatic scan on a push? (Revoke status. Refuse to accept the push. Nothing. Something else.)
And if the answer to the previous question is not: "Nothing.": How do you plan to deal with the false positives? (I think there are going to be plenty!)
Comment #15
jthorson CreditAttribution: jthorson as a volunteer commentedThe intent is i) education, and ii) continuous improvement. The gate at initial project creation helps with the former, and scans per commit help with the latter.
If a project doesn't pass a scan on a push, we'll do the same thing we do if they don't pass tests. Publish the result, make the information available, and let the end users make their own decisions.
To be blunt ... attempts to improve this process have been derailed at least three times in the last five years due to a constant barrage of negativity and criticism interfering with and causing frustration for those trying to actually implement change. The next steps have been defined and agreed to ... let's keep this issue focused on making those happen, please.
Comment #16
fuzzy76 CreditAttribution: fuzzy76 commentedAnd I will state the same argument I did for the namespace reservations... We should not worry about bogus code unless we have clear indicators that this is a real problem.
The promoted project still can't have stable releases, and the difference between stable and non-stable releases are about to be made perfectly clear on the project pages in the future. So what would be the point? To have a project marked in red as unsecure?
Comment #17
catchI think a mandatory automated scan before project creation is a lot better than mandatory manual review before project creation, especially since the manual review, as @gisle points out, relies on a module more or less passing automated scanning anyway, since it's hard to review anything that doesn't.
For me personally, I'd be fine with the automated scan being informational rather than blocking (as is suggested for everywhere else it runs once a project is published), but that's a much smaller, easier change to make in the medium to long term. Additionally, the rules that DrupalCS shows notices vs. warnings for etc. is configurable and false positives are being worked on actively as part of the process of getting core to pass, in #1299710: [meta] Automate the coding-standards part of patch review and related issues.
So a firm +1 to going ahead with what's proposed here, and we can review it 3 or 6 months in to see how it's going and tweak things if necessary.
Comment #18
kattekrab CreditAttribution: kattekrab at Creative Contingencies commentedComment #19
kattekrab CreditAttribution: kattekrab at Creative Contingencies commentedComment #20
jibranI'd like to help as well but the problem is there are no patches in the issues. For core or contrib we don't have to go to sandbox to review the patch. Dreditor allows us to review the code from the issue and read all the context as well. So maybe consider adding a new requirement of adding a patch to the issues with snadbox links so that reviewers can easily review the new applications.
This will be a good addition imo because git vetted users should know how to create patches so they can support the issue queue of their module in the future.
Comment #21
kattekrab CreditAttribution: kattekrab at Creative Contingencies commentedComment #22
kattekrab CreditAttribution: kattekrab at Creative Contingencies commentedComment #23
kattekrab CreditAttribution: kattekrab at Creative Contingencies commented@jthorson wrote:
Indeed.
I've added myself to the community team list to offer PM support for this initiative.
As a first step, I have checked in with @jthorson and made sure all the issues listed in the proposal gdoc are linked here as related issues, and added them to the Issue Summary.
What we need
1. Triage those issues
2. More volunteers to join the community team to get this done.
Comment #24
jthorson CreditAttribution: jthorson as a volunteer commentedComment #25
kattekrab CreditAttribution: kattekrab at Creative Contingencies commentedComment #26
hestenetComment #27
drummAdding links to #1266444: DrupalCI should return failed for poor code style - php & #2589853: Integrate PHP CodeSniffer test with drupal.org for the PHP_CodeSniffer integration plans.
Comment #28
jthorson CreditAttribution: jthorson as a volunteer commentedI've completed a proposed implementation for #786702: Add information about releases covered by security advisories to project pages near download table, which is currently waiting for feedback on the resulting implementation (i.e. suggestions for improvement on the look/feel of the result?) ...
Comment #29
mrf CreditAttribution: mrf at Chapter Three commentedI think there is a need to open a new issue to change drupalorg_git_gateway_admin_user_form to be able to add / revoke access to the new permission being created in #2666576: Allow non-git vetted users to promote sandbox projects to full project status. Didn't see that covered in any of the existing linked issues, but I might be missing it.
Comment #30
kattekrab CreditAttribution: kattekrab as a volunteer commentedWhere are we at? There's lots of issues, and we've still got a huge backlog of people waiting to get their projects approved.
Comment #31
davidhernandezGiven the new post on the association website about staff reductions, including Josh, and the DAs focus moving from software delivery to adoption and marketing, I'm convinced this process won't change significantly in the next year (probably more). To remove this pain point we need to make non-software changes, and the justification for not opening the gates is getting harder to make.
We should make the Security Team policy change to not cover new/low usage modules, add the warning message to project pages, and then open the gate. All the automated, CodeSniffer stuff can come later, if at all.
Comment #32
DamienMcKennaWe have a number of smaller sub issues and related issues for this, lets try getting them all to RTBC?
Comment #33
jthorson CreditAttribution: jthorson as a volunteer commentedOf the three main short-term chunks, #2666576: Allow non-git vetted users to promote sandbox projects to full project status is the farthest away from completion. mrf sprinted on this in New Orleans, but it led him down a rabbit-hole of related complexities - this is where we could use the most attention.
#786702: Add information about releases covered by security advisories to project pages near download table is very close ... was just waiting on someone to review the proposed shield graphic against the drupal.org style guide to confirm whether the colors align with the d.o design guidelines. After this, I think it would be RTBC/ready for DA review.
chrissarusso ran with #2035235: Add a permission for creating stable releases, and grant to “git vetted” users in NOLA, and followed up after the sprint to post an initial patch. Neil has suggested a minor tweak on that patch, with reference to another code instance that shows how he would prefer the initialization be handled. This is the third 'big piece' that would be needed; and without looking at the current patch, my impression is that the remaining work isn't very large.
In my opinion, having these three in place would allow us to flush out the current queue in the short run, even without some of the longer term code review tools that we'd like to see in place.
Comment #34
jordanpagewhite CreditAttribution: jordanpagewhite as a volunteer commentedI think that it would be valuable to have multiple pathways to vetted status. There are many younger/newer users, such as myself, that contribute quite a bit to other projects but do not have git-access. Speaking from my own experience at least, I think that this method of contributing has made me into a more careful and knowledgable module developer than all of the custom modules that I've had to create. Contributing to existing projects, particularly projects with active maintainers, provides a newer contributor with thorough and frequent feedback on their patches from, usually, more experienced Drupal developers/engineers.
All of that being said, I admit that I cannot think of a reasonable way to implement this type of pathway to vetted status. There are many complications that would have to be considered. For example, the quality of patches should really be considered over the quantity of contributions.
Can't users current obtain vetted status simply by being adding as a co-maintainer to a project though? If so, how would, or wouldn't, this change in this proposed revamp? Would new contributors only be able to obtain vetted status through the process outline in the Google Doc, or could they obtain vetted status through being added as a co-maintainer to a module project.
This is a really exciting change though! I really appreciate the time that you have all spent to improve this process for newer contributors.
Comment #35
jthorson CreditAttribution: jthorson as a volunteer commentedThe initial (and most urgent, IMO) goal of this proposal is to remove the roadblock at the very first on-ramp into community contribution for new project maintainers ... so I consider this as a 'first step' in a longer evolution and re-evaluation of the entire 'git vetted' role - dealing with the most immediate and pressing problem, with the recognition that this is only part of the overall solution.
Being added as a co-maintainer is a way around the git vetted user process for contribution to that project ... but it only affects that particular project. It does not afford the user the ability to generate new projects of their own.
Other ideas which have been seriously considered throughout the long course of this discussion include the the automated review checks as described in this proposal, as well as a self-serve 'challenge quiz' that users would use to demonstrate knowledge of the Drupal APIs and best practices ... there are a number of ways that we could administer the 'git vetted' permission; but the first step is to open up certain capabilities on Drupal.org to not need that permission in the first place. (We do acknowledge that this simply 'moves' the main concern, that the manual review process is unsustainable and does not scale, to later in the development process; a problem which will still need to be addressed in the longer term.)
Comment #36
jordanpagewhite CreditAttribution: jordanpagewhite as a volunteer commented@jthorson Thank you for clearing that up. I was under the impression that being added as a co-maintainer of a project also granted that user the ability to create new projects. With that being clarified, I still think that it would be valuable to add in some incentive for new contributors to spend their volunteer time on existing projects. I think that there would be many benefits to this pathway, both to the individual contributor and the greater community. Hmm. I wonder if it is realistic to expect every new contributor to create a new project that doesn't duplicate the behavior of an existing project and also meets a practical need of a larger community. I have only looked through the project applications a few times, so I won't pretend to have enough information to have a strong opinion on this. I think that this page (https://www.drupal.org/node/23789) outlines a good case for collaboration vs. competition.
Just to clarify since this is an online forum and tone is sometimes difficult to convey/interpret, I am in no way complaining about the current direction. I think that shortening and simplifying the project application process is a fantastic idea, along with really anything that makes it easier for newer contributors to make, what they perceive to be, a meaningful contribution to the Drupal community. I find any reason to publicly thank all of the core/contrib mentors for the help they gave me as a new contributor, and the mentorship that they continue to provide me with as necessary, so thanks to all of you and anyone else who helps out newer contributors. Many Drupal developers/engineers seem to be isolated, which I think only raises the learning curve to contributing, so all of these little improvements are a huge help.
If there is an opportunity to consider some kind of pathway to git vetted status through contributions to existing projects, I would love to be a part of that discussion. I'm really don't have any experience with the current project application process though, so I'm not sure if I can be very helpful there.
Comment #37
mlncn CreditAttribution: mlncn at Agaric commentedAdding the list of projects needing review, oldest first for context (and so those frustrated by this issue can channel their energy into something temporarily useful).
Comment #38
mlncn CreditAttribution: mlncn at Agaric commentedLong-term fix: Anyone—anyone—who wants to promote a project to full project status needs one other person with two promoted projects to review and endorse the project.
If we let anyone with an approved project (or two) approve anyone else's project—knowing that their reputation (and permission to approve other projects) is on the line—then longtime contributors need just ping a friend and new contributors have a process without an artificially restricted number of gatekeepers. The approver stays on record on the project as the person who approved.
We tweak this as necessary but we're all in this together. The first thing that will go is any requirement that every automated review must pass perfectly or that every step of the review process is required, but naturally the review process will be followed more rigorously for people without a known track record.
Many of the people struggling through the review process (and probably a disproportionate number of those who stick with it) have in fact been engaged members of the community for years.
Project reviews should be done in the project's issue queue so the knowledge and things worked out there aren't disassociated from the project. Any project maintainer should be able to request a review at any time, and the list of requested reviews should be highlighted on drupal.org as one of the most useful and learning-rich things a person can do, and of course people who do a review should get credit. This is one case where a special permission for experienced project reviewers could be useful, to grant credit when a project maintainer doesn't know how (or doesn't want to because their module was torn apart by the reviewers).
Again: If a review system would be untenable for current contributors, it will be ten times as terrible to new contributors. No project should be listed on Drupal.org with only one set of eyes ever on it. Let's make review part of our community culture— acknowledging we will have to lower our standards of review to do so.
Comment #39
mrf CreditAttribution: mrf at Chapter Three commentedThanks everyone for the renewed attention to this issue.
I agree that with the re-prioritization of DA resources, if we ever want to see this process fixed it is going to have to be fully community-driven.
Along those lines I just wanted to chime in that contributing your own changes to drupal.org, while daunting at first, is not that much different than working on large complex Drupal 7 client sites (which I know many of us do for our day jobs). It just takes some time to wrap your head around it all. Right now my biggest limitation for actually completing anything is how time-consuming it is...
I posted a start of a patch at #2035235: Add a permission for creating stable releases, and grant to “git vetted” users hopefully someone with more time on their hands can pick it up before I get a chance to get back to it.
Comment #40
kattekrab CreditAttribution: kattekrab as a volunteer commented@mrf - thank you - for contributing a patch to one of the revamp issues. I agree we need to ask the community to step up here.
@mlncn - thank you - for revisiting the underlying challenge with me. I agree that a culture of review and valuing reviewers is a great goal. Thanks also for jumping in to review and approve projects in the backlog.
@DamienMcKenna - thank you for diving into the backlog and moving things along!
@jordanpagewhite - thank you for volunteering to help with this very long standing problem.
@jthorson @hestenet @davidhernandez @mlhess - should we regroup, reprioritise and get this thing done?
Comment #41
hestenetI'd like to summarize a few out-of-band conversations that have been held over the course of the last month or so on this topic.
In several combinations and permutations @kattekrab, @dries, @mlhess and the security working group, and myself have been working together to see if we can take the foundation laid out in @jthorson's revamp proposal a step further. Collectively, we've managed to articulate the two fundamental constraints on achieving a real solution.
Constraints
1) We need to remove the gate to new contribution entirely - not just kick the can to a particular elevated role, or a specific limit on the # or kind of releases a new contributor is allowed.
2) We need to continue to send strong signals about security coverage to users evaluating whether to use modules from Drupal.org.
Taking these two constraints as the basic requirements of our solution - we've finally hit on something that will put this to bed: decoupling the ability to make full projects and full releases from security coverage.
The implementation plan for this follows the general outline of @jthorson's revamp proposed above, and much of the work that has been started will be still be usable, with slight changes.
Furthermore, the Security Working Group has helped to draft this plan, and has already approved it.
High Level overview:
As we have sign-off from the Security Working Group already - I'll be allocating DA Staff time in the coming weeks to help get this solution across the finish line.
I'll work on updating the issue summary and child issues as appropriate shortly.
Comment #42
hestenetThere's also a third constraint that we need to start thinking about as a follow-up to this implementation. There is still the incredible value of peer code review - both for developers finding their feet with Drupal, and for the end-users trying to discover the right modules to use.
Better project discovery is critical, so that we provide signals to end-users about what modules are right to use, and we provide incentives to participating in collaborative code review.
I think this third constraint is absolutely critical as well, but I do think it's something we implement as a follow up to these initial changes.
Comment #43
gisleAbout #41 and #42: This is the most encouraging I've read about this thing. Kudos to everyone involved!
Yes!
We need to do more to motivate community members to participate in collaborative code review.
At the moment, I am not aware of any incentives for doing this.
On my Drupal.org profile page, there are already some "brownie points" indicators in place, such as how long I've been a community member, a rough count of the number of edits I've done to user contributed documentation, a list of projects I've committed to, and issues I've helped fixing in the last three months.
But I've nothing to show for the hours I've spent doing collaborative code review in the PAReview issue queue. A short term response to the missing incentives would be to get in place a simple counter (similar to the present one one for documentation edits), simply counting the total number of manual code reviews the user has done.
For the longer term, I've been thinking of how to "gamify" collaborative code review. This will obviously need careful design, but would as a minimum involve the basic gamification components points (e.g. for doing upvoted peer reviews), badges (for various achievements) and leaderboards (for being the most prolific, and/or most upvoted reviewer, and/or spotting most security issues, and/or some other metric).
Creating such a system would be a quite extensive undertaking, and would probably need to have its own community initiative project - but I believe that gamification would help a lot in providing some much needed motivation to get more people involved in collaborative code review.
Comment #44
kreynen CreditAttribution: kreynen at University of Colorado Boulder commentedIs there an open issue for the required core change to updates status?
Since the security team only reviews 1.0 and > releases, will all alpa and beta releases of projects also include a similar warning... since leaving a module in perpetual beta is basically opting out of the security reviews?
Will distributions using modules with this new security warning inherit the warning on their project page the way including a version of a module with a security releases in a .make does now?
Comment #45
hestenet@gisle - I'm glad this resonates with you! I think you are right about the scope of code review credit and incentives being fairly extensive - but hopefully we can identify some quicker short term improvements on the road to a longer term solution for that aspect of all this.
@kreynen - I was pulled away at the end of the day yesterday, so those child issues haven't yet been created (or updated where appropriate on the existing child issues). I'll try to get more of that posted out today. You raise some important questions, for sure.
Comment #46
hestenetComment #47
hestenetComment #48
fuzzy76 CreditAttribution: fuzzy76 commentedYes, I love this! Just hope that the automated code review isn't completely forgotten. While it does not need to be a part of the security coverage process, I still think it is important to have shown prominently on the project page for both users and developers to notice.
Comment #49
gisleAccording to this report, hacker Peiter Zatko and his mathematician wife Sarah Zatko has created a suite of algorithms for static analysis of computer programs that provides some metric that indicates how vulnerable the program is to an attack ("security hygiene").
Most of the algorithms seems to be applicable only to binary code (as they look at things like ASLR and steack/heap protection). However. it may be that some of the techniques they use (like "fuzzing") may also prove useful in an interpreted language environment.
AFAIK, except for klausi's PAReview.sh (which is mainly about coding standards), and the in-core regression testing framework, there is currently no community effort for creating software specifically for static analysis and security benchmarking of contributed projects.
There used to exist a free software static code analysis tool for PHP code known as RIPS, but it was abandoned (as free software) in March 2015 and the rewritten and updated version is AFAIK only available as SaaS (I haven't found any pricing information).
Is a static code analysis tool for Drupal something we should consider as a new community initiative?
Comment #50
mlncn CreditAttribution: mlncn at Agaric commentedHere's a tangential, possibly interim way to get more people involved in reviewing (the process remains in crisis, with hundreds of applications awaiting action at various phases): #2810485: Allow more maintainers to grant credit
Comment #51
DamienMcKennaAnyone wanting to fix this problem should collaborate on the child / related issues that hestenet and others have added.
Comment #52
kattekrab CreditAttribution: kattekrab as a volunteer commentedComment #53
kattekrab CreditAttribution: kattekrab as a volunteer commented@hestenet - are you able to provide an update on this initiative?
Comment #54
hestenet@kattekrab - Sure I'd be happy to update.
Per the 'High level overview' in the issue summary above - I've broken down the phases into more specific user stories. Here are those user stories we've generated internally/with the help of the community folks who've been involved in this initiative. Where appropriate we'll be creating or updating the relevant d.o issues as we go.
Phase 1: Preserve the signals about project that give Drupal its security reputation:
Phase 2: Set up the opt-in process
Phase 3: Open the gates!
Phase 4: Project quality and code review incentives <-- even after the gates are open these have value!
With some of our other initiatives wrapped up at the end of last year, we've prioritized this work for @Drumm in our next several internal sprints. So hopefully you'll see more frequent updates coming soon.
Comment #55
Crell CreditAttribution: Crell at Platform.sh commentedIs there a reason that #2809373: Support for Code Climate or similar wasn't included (in the issue summary as of when I post this)? Automated static analysis, whether it's an official gate or not, can certainly remove the bulk of the human labor portion of whatever process at whatever step it's being used.
Comment #56
mallezieIn the same light of code climate. There is also a possibility to use coverity for static code analysis. #2844865: Coverity scan for drupal core. Not sure of integrating that with CI is possible however.
Comment #57
drumm#2809373: Support for Code Climate is relatively new, it wasn’t discussed here because time travel is less possible than making the project application process better.
More practically, DrupalCI is running phpcs now. For example:
#2851916: Expose phpcs patches to comment stream is the issue for getting the good parts of that directly in Drupal.org’s UI.
Comment #58
drummUpdating the issue summary now that #2787165: Add security advisory coverage field to projects is done.
Comment #59
drummAdding #2825912: Rename “Git vetted user” role to “Opt into security advisory coverage” to replace text in the issue summary.
Comment #60
drummRemoving #2035235: Add a permission for creating stable releases, and grant to “git vetted” users from the issue summary, that need has been fulfilled by #2787165: Add security advisory coverage field to projects, which came with a permission for using that field.
Comment #61
drummThere’s been good progress on this recently. We’re planning on doing the big changes, #2825912: Rename “Git vetted user” role to “Opt into security advisory coverage” and #2666576: Allow non-git vetted users to promote sandbox projects to full project status later this week or early next week.
#2766491: Update status should indicate whether installed contributed projects receive security coverage is a core followup that’s needed to make the security coverage conveniently visible to site maintainers. Reviews are appreciated.
Comment #62
drummI fully expect to miss some of the documentation that might need an update as this change happens. A good way to help out is finding that and putting it in the issue summary.
Comment #63
Pere OrgaAdding [#251466] to the list of documentation pages to update
Comment #64
drummI’m deploying #2666576: Allow non-git vetted users to promote sandbox projects to full project status and #2825912: Rename “Git vetted user” role to “Opt into security advisory coverage” shortly.
Comment #65
drumm#2666576: Allow non-git vetted users to promote sandbox projects to full project status has been deployed. I decided against #2825912: Rename “Git vetted user” role to “Opt into security advisory coverage” since the role name change doesn’t make sense to me right now.
Comment #66
drummI made a pass through the documentation at https://www.drupal.org/node/1011698 and https://www.drupal.org/node/636570. A lot of the edits were straightforward removals of “full” in front of “application process”.
Comment #67
drummLooks like the issues mentioned in the issue summary are now mostly done.
Comment #68
davidhernandezdrumm++
I'm looking to see if I can find someone without vetted access to test.
Comment #69
mrf CreditAttribution: mrf at Chapter Three commentedGreat news!! Thanks everyone for your work on this.
Comment #70
Anonymous (not verified) CreditAttribution: Anonymous commentedGreat scott, it finally happened!
I promoted my project to full status here: https://www.drupal.org/project/webform_timestamp_field
I noticed I could NOT make a release from a branch, even though I pushed the 7.x-1.0 branch. I had to tag it and push tags. It says branches can be an option but that didn't seem to be the case.
Comment #71
DamienMcKenna@vilepickle: In order to release code from a branch you have to name it [core].x-[version]-x, e.g. "7.x-1.x" or "8.x-4.x", the two X's have to be X's, they can't be numbers.
Comment #72
Anonymous (not verified) CreditAttribution: Anonymous commentedHmm, the instructions on the release page, at least when I had no releases, said the branch itself could be something such as 7.x-1.0 (it said branches or tags). Seems the instructions may need to be updated.
Comment #73
davidhernandezI posted this in a couple places and tweeted it. I got feedback from people that are not vetted that it is working.
What shall we declare qualifies this as RTBC short of going through every bullet point and linked issue?
Comment #74
Peter MajmeskuI agree with Crell. No automated process can replace the human action in the project application process.
I remember my own project application regarding the Permissions by Term module. It took more or less a half year. But in this time I had the occasion to learn a lot in Drupal programming and programming in general. I was forced to code for quality. Even it was no pleasure sometimes. But that way I learned.
It's a pity that this human review process in project application will go away. Is this now an action to get more developers into the Drupal project, even if the quality of the module's software will get worse?
Comment #75
hestenet@jepSter - There are quite a few factors that went into this decision to change policy and functionality. One of them was indeed to lower the barrier to entry to making projects on Drupal.org. It's been a long term pain point and a constant source of feedback from developers outside of the community as to why they haven't contributed projects/built integrations/etc. Some of these folks then put their contributions on gitlab or github or another code host, but others often just don't contribute back at all.
At the same time, we absolutely do want to preserve Drupal's reputation for code quality and security. Both automated tools and human review by dedicated volunteers can and should be a part of that - but after review with many stakeholders we did not want that human review to be a hard gate on contribution. Hence this change.
Please do note that Phase 4 of this plan (the next phase) is to design and implement additional code quality signals for projects on Drupal.org - and in my mind that should include both automated tools and new incentives for peer code review. Ideally these will also factor in to searching for projects on Drupal.org.
Comment #76
mrf CreditAttribution: mrf at Chapter Three commented@Jepster there currently are no humans reviewing project applications, so any conversation that wishes for the good old days of application review should take that into account.
If there was a large team of people helping review applications none of these steps would have had to have been taken. This is a bandaid over a bullet wound so that there is at least some kind of process that allows newcomers to contribute modules.
If you look at item #3 on follow-up in the issue description, the community is still very much interested in solutions for module authors to either get human or automated reviews of their modules. Any help creating those systems would be greatly appreciated.
Now that it seems pretty clear that this new process is functional has anyone else signed up to go through https://www.drupal.org/project/projectapplications and let everyone know they are out of purgatory? I'll volunteer myself
We should also update that project page with details about the changes to the process.
Comment #77
hestenet@mrf - It's not quite fair to say no one is still reviewing project apps - there are definitely a few heroes in there still helping out every day - and they deserve our thanks.
I wouldn't sell them short at all, and I hope they'll continue to participate as we find new ways to incentivize quality peer review.
If you are going to make updates to the issue queue and that project page, please bear in mind that while application process is not a gate on full projects anymore, it is still a gate on being able to opt your projects into receiving security advisory coverage, at least for now. The security team may develop a new system for that over time.
Comment #78
drummRemember – https://www.drupal.org/project/projectapplications is still the process if you would like permission to mark your projects as covered by the security advisory process.
We do have some new tools, like DrupalCI being able to be set up on the dev releases of full projects, which includes coding standards messages. It would be good to see the best practices for that and other changes be documented.
Comment #79
mrf CreditAttribution: mrf at Chapter Three commented@hestenet I guess I should have been clearer and said there is no on APPROVING project applications, there are still a few brave souls and submitters looking for the review bonus reviewing the existing pile.
I am happy to see this process improving after several years of this being pointed out as a huge issue from dozens of sources, but I don't have a lot of patience for pretending that the existing process wasn't horribly broken.
It looks like some of the docs are still inconsistent. Where is the best place (besides this issue) to point the people in that queue to let them know about the changes? https://www.drupal.org/node/1011698 seems to be up to date in terms of opting in to the security coverage so I'll definitely include that.
Comment #80
mrf CreditAttribution: mrf at Chapter Three commentedCreated #2858911: Docs updates to reflect changes to project application process as a followup to keep track of documentation changes needed. Once there are some docs pages updated with this new information I can link to I'll start notifying people in the current review queue about the changes.
I do not have access to update the project page on https://www.drupal.org/project/projectapplications whose queue should that update go into?
Comment #81
drummI think the queue for https://www.drupal.org/project/projectapplications itself, since the maintainers there “own” the project page. However, I’m seeing none of the maintainers have “Edit project” access, so I guess it is up to https://www.drupal.org/project/webmasters. Especially now, it would be good for 2-3 people to get that access and lead the changes.
Comment #82
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedNow that the floodgates are open, what recourse we do have against super low quality contributions? Is there a process for addressing this other than to politely point out the issues in the queue? Speaking in reference to #2859086: Module is beyond insecure.
I've been keeping an eye on the new modules list after this announcement and this was the first project I've reviewed.
Comment #83
fuzzy76 CreditAttribution: fuzzy76 commented@jepSter You are jumping into a discussion that has been going on for years. You should probably research that, but it is far too long (and was too heated) to be repeated in full here.
The issue being fixed here is that people used to wait for months between each feedback on their application, and even longer for the actual permission grant. This process haven't been working as it should for many years, and have been scaring off a lot of very good developers. Voluntary work won't scale, and the Drupal community have grown too large.
If you keep to security supported modules, this issue won't change the code quality one bit. On the contrary, it will probably improve it since the security support status is more prominently displayed and will probably inspire more developers among those already vetted to keep their modules supported.
And quite frankly, who would use modules without security support? ;) For some reason it has become fairly common in the Drupal community to use alphas, betas and even dev releases. Hopefully this will help change that.
Comment #84
Rajab Natshah CreditAttribution: Rajab Natshah at Vardot commentedThank you :)
I got to this from this tweet by Klausi
https://twitter.com/_klausi_/status/839483666002739200
Comment #85
klausiProposal for updating the security advisory policy document: #2721251: Update security advisory policy for project coverage option
Comment #86
klausiI updated https://www.drupal.org/project/projectapplications to "Drupal.org security advisory coverage applications".
Not really super happy with the name, better suggestions welcome!
Comment #87
kattekrab CreditAttribution: kattekrab as a volunteer commentedWow.
Thank you everyone. This long long long running issue is closer to being resolved.
There will obviously be some adjustments as we learn to live with a different baseline, and I've no doubt we'll encounter new challenges.
@sam152 - We still care about security, but we no longer use security as a reason to keep people from contributing to our community.
Whilst this issue was created a year ago, the proposal here was the result of more than 5 years effort to fix a broken system. This pain point has been a festering wound in our community for too long.
Comment #88
drummAdding 3 followups to the issue summary:
Comment #89
gregglesI imagine this was an oversight in this statement, but... It was possible to contribute code as a project ever since the great git migration. It was possible to contribute code as a patch since forever. It was more difficult to create a downloadable version of your contribution until the many changes as part of this process revamp.
Comment #90
drummPulling in one more followup: #2860311: Remind maintainers what a full release is when creating one
Comment #91
drumm#2859739: Make the “not covered” security warning more visible on project pages and #2858261: Allow any vetted maintainer, not just project owner, to opt into security coverage have been deployed.
I have #2860311: Remind maintainers what a full release is when creating one in my sprint plan for the next 2 weeks.
Comment #92
hestenetUpdating the issue summary to reflect that I'm moving the 'Phase 4' tasks into their own plan issue: #2861341: [PAR Phase 4] Improve project discovery with strong project quality signals(automated) and incentivize peer code review(manual) . That will hopefully be a bit less unwieldy.