Posted by klausi on June 6, 2013 at 9:03pm
kscheirer has done an amazing job in the project application issue queue by reviewing a lot of projects. Big thanks for that! Now if somebody shows that kind of motivation we should consider that person a candidate for being a git administrator, i.e. someone that is able to approve applicants themselves.
So let's verify all the RTBC applications of kscheirer and help him become a code review administrator (if he wants that :-)! Your help with that would be highly appreciated (by the applicants too, of course).
Comments
I like being empowered :) I'd
I like being empowered :)
I'd love to become a code review admin! So far I've noticed I missed an XSS vulnerability that klausi pointed out in Dummy Permissions (gotta check strings even on the admin side). Any other areas I need to work on?
Most of what I saw was modules getting dinged for very minor problems (typos, minor code and comment style problems, small bugs) instead of security issues.
You did a nice round of
You did a nice round of reviews again recently, so I think it is time to promote you to a code review admin: https://drupal.org/node/2031683
Please leave a comment there that you want this position.
All others: please weigh in with support for kscheirer or post any concerns you might have. Thanks!
I'm all for, even though my
I'm all for, even though my only reason is that people who actually want to contribute should be rewarded and given power to do so! :)
Congratulations Karl and
Congratulations Karl and welcome on board! I added you to the list of code review administrators at https://groups.drupal.org/node/142454 .
Feel free to start approving applications now, docs: https://drupal.org/node/1125818
Small security hole I've noticed...
When I grant a user "git vetted status" at https://drupal.org/user/[uid]/edit/git, on the page refresh I see the following status message
I suppose it's not a very large problem, since by giving me Git Administrator access I already have a lot of permissions and can be trusted. But it's not information I have access to anywhere else. Theoretically this means I could uncover any d.o user's private email address.
That's a good point, though
That's a good point, though it's not the end of the world.
You could post to http://drupal.org/project/issues/drupalorg and suggest that the drupal_set_message be wrapped in a test for either the user saving is the uid being affected or that the user has "administer users" permission.
knaddison blog | Morris Animal Foundation
Thanks greggles, this has
Thanks greggles, this has been fixed: https://drupal.org/node/2039173.
my next task...
I've been working mostly in the "needs review" portion of the queue, but that's now down to a single page and the oldest is 8 weeks (ok a couple more snuck in there since yesterday).
Next up is the RTBC portion. There's about 100, the oldest getting close to 1.5 years. Many of the very old ones are issues that I RTBC'd from needs review. From https://drupal.org/node/1125818 I know
So I'm going to apply that policy (they are all over a month old in this case). My goal is to reduce the RTBC issues to nothing over 8 weeks and under 1 page. I won't get to this until next week at the earliest, so please let me know if I'm off track or my initial reviews were not good enough.
A couple issues are very difficult to review simply because of the size of the project:
Excellent - and don't get
Excellent - and don't get discouraged by larger projects, you don't have to review every line of code. If I don't find severe issues after 10-15 minutes of reviewing I stop and call it ready. Remember: this is a sanity check and not a professional grade full code audit. Of course we try to be as helpful and detailed as possible, but our time is limited and the queue is long :-)