TL;DR
- An auth check that uses
===where both sides can beundefinedis fail-open — a missing config turns it from locked to open.- The one secret missing from
.env.examplewas the internal auth token. Unset in a deploy,undefined === undefinedlet every anonymous request through.- Default to deny: validate types, reject empty, compare in constant time, and write the test that sends no credentials with the env var unset.
The coding agent handed me an auth middleware. It passed review. I almost shipped it.
function protect(req, res, next) {
if (req.headers['x-internal-token'] === process.env.INTERNAL_API_TOKEN) return next();
return requireAuth()(req, res, next);
}
Four lines, and it reads fine. An internal service calls with a shared token and skips the login flow; everyone else falls through to real auth. Then I checked what happens when the token isn't set.
What the check actually did
INTERNAL_API_TOKEN was the one secret missing from .env.example. Every other key was there — Stripe, Clerk, Paddle — but not this one. So on a deploy where nobody thought to set it, process.env.INTERNAL_API_TOKEN is undefined.
Now a normal browser request comes in. It doesn't send an x-internal-token header, so req.headers['x-internal-token'] is also undefined.
undefined === undefined → true → return next().
Auth bypassed. Every anonymous request to /checkout is treated as a trusted internal call. The endpoint is safest when the token is set and wide open when it's missing — which is exactly backwards. A misconfigured auth check should get more restrictive, not less.
And there's a quieter problem even when the token is set: === on a secret isn't constant-time. It short-circuits on the first mismatched byte, so response timing leaks the token one character at a time.
The real check
Fail closed:
const crypto = require('crypto');
function hasInternalToken(req) {
const expected = process.env.INTERNAL_API_TOKEN;
if (typeof expected !== 'string' || expected.length === 0) return false; // no token configured → deny
const provided = req.headers['x-internal-token'];
if (typeof provided !== 'string' || provided.length === 0) return false;
const a = Buffer.from(provided), b = Buffer.from(expected);
if (a.length !== b.length) return false;
return crypto.timingSafeEqual(a, b);
}
function protect(req, res, next) {
if (hasInternalToken(req)) return next();
return requireAuth()(req, res, next);
}
No configured token means no bypass. An empty or missing header means no bypass. The compare is constant-time, and length is checked first (timingSafeEqual throws on length mismatch). Then add INTERNAL_API_TOKEN to .env.example so it can never be silently omitted again.
Why agents write it this way
The happy path looks correct. Token set, caller sends the right one, x === y is true — review sees a plausible auth check and moves on. The model optimizes for "works when configured," and === fails open by default. It never had to reason about the unset case, because the unset case isn't in the example the code was written against.
Humans do this too — the fail-open default, the === on a secret, the config that's "obviously" always set. Agents just do it faster and ship it greener.
Fail closed, and prove it
The tell isn't in the code. It's in the test you didn't write. Send a request with no credentials and the env var unset, and assert it's rejected:
delete process.env.INTERNAL_API_TOKEN;
const res = await request(app).post('/checkout').send({ seats: 5 });
expect(res.status).not.toBe(200); // must be 401/302 — not "welcome in"
If an unauthenticated request with no token configured gets a 200, your auth is open — and no amount of green on the happy path will tell you. The bug lives entirely in the case the tests never exercise.
What I changed in the habit
- Auth defaults to deny. Every branch that grants access starts from "no."
-
Validate both sides — type is string, length > 0.
undefined === undefinedis not authentication. -
Constant-time compare for any secret (
crypto.timingSafeEqual), never===. - Write the misconfigured-env test — unset the token, send nothing, assert rejected.
-
Every secret the code reads goes in
.env.example. A missing one isn't a config gap; here it was the whole lock.
I found this the way I find most of them: reproduced it on the real code — an anonymous request returned 200 — applied the fix, and confirmed it flipped to a 302, with FetchSandbox. The bug was never in the happy path. It was in the case the code never had to handle.
Which is the rule every auth check should start from: fail closed.
Top comments (0)