4

One of my team members committed a huge mistake; a clear SQL injection-vulnerability. It obviously didn't pass my peer review and I made very clear that this is unacceptable. I never saw this programmer make this mistake before and I'm unaware of any other security holes this particular programmer committed. The problem is fixed and my explanation is understood.

My considerations:

  1. The programmer is now aware of this mistake and probably won't do it again.
  2. The problem however was huge and could have had major consequences when brought to production.
  3. It's not a junior programmer (> 5 years experience).

Should I tell the manager?

Improve this question
10
  • 5
    That's really a different subject. Commented Jan 20, 2015 at 14:47
  • 2
    How do you know the other programmer wasn't testing your review process with an obvious error? Commented Jan 20, 2015 at 14:58
  • 17
    You should have mentioned it was a hotfix. Under severe time-constraints and pressure people make mistakes they normally would not. So you can report and blame your coworker which is easy, or blame the process that got a normally fine programmer to make such a mistake in the first place. Commented Jan 20, 2015 at 15:30
  • 4
    What is your worse fear if you do not report it? What's the point? Do you think he'll do it again? Does he have a history of poor code that makes your job harder? What are your boss's expectations on receiving bad news? Does he like to micro-manage and be in the loop on everything? Commented Jan 20, 2015 at 15:32
  • 3
    How is this question more opinion based than 70% of other questions on PSE? PSE seems the right platform for questions like this one. This matter can be a real dilemma for a programmer / team leader, and an external opinion like Wheeler's or Enderland's really helps making the right decision. I think this question deserves a place on PSE. Commented Jan 20, 2015 at 18:43

2 Answers 2

Reset to default
39

There are only two good reasons to report something like this to management: is if you believe that the coder who did this was malicious and attempting to sneak something through, or if you believe that the coder who did this is incompetent, which can be just as harmful as a malicious coder.

From the way you describe him, it sounds like you believe he's just a programmer who made a mistake. I bet you've done that a time or two, even as an experienced developer. It got caught in review before it ever made it to production, and you've talked to the guy and made him aware of the issue.

Personally, at this point, I'd file the whole incident under "no harm done, lesson learned" and let it go. But keep an eye on this guy's commits for a while, just to be sure, and if you see a pattern emerge, then go to management. But for the moment, there's no reason to go potentially screwing up someone's career over one mistake.

Improve this answer
2
  • Point taken. The reason why I'd go to the management with it, is because SQL injection is such a big deal in our world and so easy to prevent, that it's almost always a sign of incompetence to literally copy URL-passed variables into SQL. But I guess you're right and I should leave it and keep a close eye on the commits. Commented Jan 20, 2015 at 14:50
  • 19
    @Sherlock: Yeah, or a sign of someone who made a mistake. When the design of the language is such that the obvious, intuitive way to do something opens a security hole, security holes are going to happen, even for experienced developers. Why do you think we're still getting patches for buffer overflow vulnerabilities every Patch Tuesday, a quarter-century after the Morris Worm put them on everyone's radar? Because some mistakes are literally hard not to make in certain languages. Commented Jan 20, 2015 at 14:59
35

This is the whole point of code review.

Mistake is written. Mistake is found and corrected in code review.

Do you want to tattle on your senior team member? This team member according to you has never made a similar mistake before.

Unless the code was intentionally written that way (and you can prove it) this is a really dumb idea to go to management with.

Improve this answer
0

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.