|
Honestly, someone should have time for that, it's part of the problem... I go out of my way to comment on as many PRs as I can, because I'll catch things that will become problems later far more than other peers who just click approve. The same can be said for security guys... they spend their day needing to work as well, and seeing a bunch of smaller things fly by is just as valid as a big audit periodically. It's easier to catch a lot of things before they become big as well... There are plenty of times I'll comment (Okay, letting this through, but in the future revise to do it this way), sometimes I'll push back, but not always, that's what the review process is for. I'm just suggesting multiple approvers for PR, where one is someone who is security minded. It's funny how many issues I'll see from other systems where someone does something per the spec, that has a flaw because they were completely compliant. Someone crafts an exploit, and I'm interested because I'd usually be more pragmatic in implementation. Last year there was a huff about JWT allowing cert overrides in some frameworks, as they don't ensure the origin cert matches a whitelist... when I'd implemented JWT, I only checked against our whitelist and ignored the property. Sometimes security guys will see things and think of things in a way others won't... for me, one thing I often catch that others don't are potential points for DDOS target viability. Some of that comes from using node, where you do NOT want to constrain your main event loop thread. Others don't think about putting limits on JSON size, or compute heavy tasks, etc. And, frankly, I'm tired of fixing related bugs to patterns that were broken from the start.... turtles all the way down, but the turtles are eating all the errors. |