---
title: "Why a Second Reviewer Found What a Clean Security Review Missed"
canonical: https://dxdev.com/blog/internal-review-said-clean-second-pass-found-six/
datePublished: 2026-06-25
---
The internal review of a bulk-edit feature came back clean on every question it asked. Nothing was injectable, every write was scoped to the right account, and the one flagged risk (a set of database writes running as several separate calls instead of one transaction) was rated correctly and already had a fix proposed. We read it, agreed with all of it, and sent the same feature to a [second, independent reviewer](/everyday-ai/the-review-that-said-clean/) anyway, mostly out of habit.

That second review confirmed everything the first one found. Then it kept going, because "does the code do what the internal review checked for" and "does the code do what a user with a mouse can make it do" are different questions, and the first review had only asked the first one. It came back with six bugs.

## The fix that missed a second call site

The flagged transaction problem was about moving a row between two tables: delete from one, insert into the other, no wrapping transaction, so a failure partway through could leave the data half-moved. That was correct, and it was rated high severity.

What the internal review didn't ask was where else the same move already happens. A single-row edit path elsewhere in the same feature carried a code comment saying the field in question was "just a relabel," which implies the record stays in its current table. It doesn't. The backend routes that field through the identical two-table move the internal review had just flagged as unsafe in the bulk path. The comment was wrong, and the fix proposed for the bulk case never touched this second call site, because nobody had gone looking for one.

## The write that succeeds and is wrong

The one that worried us more produces no error at all. One of the bulk operations shifts a date field relative to its current value: math on the existing date and a target date. If the existing date on a given row is empty, that date arithmetic in SQL Server doesn't error. It resolves to nothing, and the update sets the field to nothing. There is no error page and no rejected write. A real value is replaced with an empty one, on whichever rows happened to have no date already.

The internal review's checklist covered the loud failures, the things that would blow up in an admin's face. Nothing on it was built to notice a write that succeeds and is simply wrong.

## A blank search box that means zero

A related one was almost funny. You can tell the tool to update the field only on rows matching a value you searched for. If you leave that search field blank, meaning "match everything," the backend reads the blank as the number zero and filters for rows whose value literally equals zero.

So you leave the search box empty expecting "apply to everything in the selection," and the change lands only on the small subset that already has a zero in that field. Everyone else in the selection is skipped with no warning. The success message still says how many rows were touched, but that count was taken before the filter ran, so it reports the full number regardless of how many rows the update actually reached.

## What the first review was shaped to see

None of the six findings needed exotic tooling. They needed someone reading the same code with a different question in mind: what happens on every input a real user, with no malicious intent, could plausibly produce. The internal review wasn't wrong about anything it checked. It checked one shape of failure (loud, structural, the kind that throws an error), and the code's weak points were in the other shape: a write that succeeds, reports success, and is wrong.

We don't run every feature through two independent reviewers as a permanent policy. That doesn't scale, and most changes don't need it. What changed is the trigger for the second pass. It used to be "does this touch security." Now it's "does this write to the database on behalf of a selection the user built by hand," because that is exactly the shape of input a checklist built from known failure modes won't anticipate.

When a feature works that way, ask for a second, independent review, and ask the reviewer what happens on ordinary inputs, not only on malicious ones.
