This came up in some discussion, and I want to know what others think. Comments have been made that a lot of the problems the security team encounters are with newer modules/maintainers. If this is true, I would like to get more feedback on the reviews being done. I can only speak for myself, but I've never received feedback of any kind. If I missed something important, or the group as a whole keeps missing the same things, I think it would be helpful to know this. It would make it much easier to create documentation with specific examples of things to look for. The point really is how do you make something better if you don't know what it is being done wrong. Also, if people do desire feedback, where will it come from? Just the security team?
Comments
I've been trying to stay on
I've been trying to stay on top of the RTBC queue doing one last check (focused on security) prior to fixing. If you want feedback then you can leave an issue at RTBC and I'll do my best to provide an additional review which should act as a form of feedback on what you might have missed.
In terms of frequency of security issues, there's a little sub-discussion that talks about this. If you want to look for the most likely security issues then look for XSS.
I don't think feedback will come just from the security team but from other reviewers. That's the nature of do-ocracy is that the policies that are followed are the ones that people commenting in the application queue mention. This is both good and bad... I think historically we just looked for another reviewer, saw someone had reviewed, and left it there. Hopefully we can get to a point where more than 1 reviewer looks at the application prior to approval which will help us all to give each other feedback.
If I disagree with something someone said I'll usually state my argument in the issue or mail the person privately to discuss.
Thanks for bringing this up!
knaddison blog | Morris Animal Foundation
When I find a security issue
When I find a security issue I tag it as "PAReview: security" (others should do this as well, of course).
This helps us to do analysis after the fact - Issues tagged with pareview security.
knaddison blog | Morris Animal Foundation
Should we tag all issues we
Should we tag all issues we want checked? If we aren't certain there is a vulnerability, but we want it double-check by someone before approving.
The tag is for issues where
The tag is for issues where there is a vulnerability. I think if you want a specific security review then make that as a comment and leave it as "needs review" or "RTBC" pending that. You should also irc message or mail me and/or Dave Reid.
I also suggest folks try to do that security review themselves and if you get stuck/confused drop into #drupal-codereview - I'll gladly setup skype/phone/screenshare to help others learn to find security vulnerabilities in the review process.
knaddison blog | Morris Animal Foundation