Three guards that looked right until I ran them against the real tool

A guard is only as good as what it actually compares. Mine kept comparing the wrong thing, and each time the review that caught it ran the real tool instead of reading the code.

The problem

Two unrelated jobs landed on the same day. One was a credential leak waiting to happen: a deploy tool read a hostname out of a config file and sent an admin password to it, with no check that the hostname was one I trusted. The other was a secret scanner whose allowlist silenced two filenames across a dozen repos, so a real key pasted into either file would have been reported clean.

Both fixes were small. Both took several rounds, because each "fix" had a hole I only found by running it.

What I tried

The password. The obvious fix is an allowlist of hostnames that fails closed when unset. That shipped. Then the review asked about redirects: the HTTP client follows a 307 by default and re-sends the request body, password included, to wherever the response points. An allowlist on the first hostname says nothing about the second. Forcing "error on redirect" in the one function that makes every request closed it, and a test that pretends to be a redirecting server pins it.

The scanner. The obvious fix was to allowlist the finding, not the path: hash the matched value and ignore only that exact value. I almost shipped it. Before writing the code I ran the scanner on a planted fake key and looked at the field I meant to hash. It held eight characters. The rules match only a prefix, so every key sharing that prefix hashes identically, and a real key would hide behind the reviewed placeholder.

The fix that works hashes the whole source line, read from the commit the scanner reported. A real key on a new line, or on the edited line, hashes differently and comes back.

The guard that trusted its tool's error message. To tell "not a git repository" from a real checkout, the filter read git's error text. A stale GIT_DIR inherited from a parent process makes git say exactly that inside a real checkout. The filter then read the working tree instead of the commit and suppressed a finding it was documented to report. I had deferred that as "needs control of the environment". It does not: a leftover variable is enough. The repair strips the variables that redirect git, and only believes "not a repository" when the filesystem agrees. The first version of that repair opened the mirror-image hole (a repository located only through those variables looked non-git), and a second review pass caught it.

What I'd do again

• Run the tool, look at the field. A harness that reports zero findings proves nothing until a planted secret makes it report one.
• Fail toward reporting. Every unclear state, a blank commit id, an unreadable file, a failing git, should produce a finding, never a quiet pass.
• Scan what you add. The test file that proved the fix tripped the gate on its own, and so did my first draft of the write-up, which spelled out a key prefix. Run the scanner over the change before the pull request.
• Check the run after merge, not just the checks before it. A release pipeline had been red for days for a harmless reason, and fixing that exposed the real failure behind it (a package manager's output format changing under a floating tool version). Repairing it would also have published three packages nobody had decided to publish, so publishing now sits behind a switch.
• Say plainly what was and wasn't reviewed. One fix commit built from a reviewer's own suggestions had not itself been reviewed. Saying so, then running that review, was cheaper than being asked.

The scanner and the deploy tool are better for it. The lesson I keep relearning is older: read the field before you hash it.