What this is
In April 2026 our founder reviewed TermBeam, a Node CLI that shares a local terminal to a phone browser over a QR code, as part of the Review Bomb series on his personal blog. The full review is published at wshoffner.dev. TermBeam belongs to its maintainer and is not our work, not our client, and not a portfolio piece.
We are reproducing it here because of what happened between the first audit of that codebase and the second. An outside reporter had already filed a correct, specific security issue naming two vulnerable sites in the code. A second reader working from the same issue found a third instance of the same bug that the report had not covered. The fix for all three was merged by the maintainer on April 23, 2026, the same day the review was published.
The starting point
Issue #177 had been filed a couple of weeks earlier by an outside reporter running a code audit. It was a good report. It identified the bug class correctly, it named the exact file and line for two occurrences, and it recommended the right remedy.
The finding was a timing side channel in password comparison. TermBeam generates a password at startup unless you supply one, and it checks that password on every authentication attempt, with two of those checks using JavaScript's === operator on the submitted string. String equality in JavaScript short-circuits at the first byte that differs, so the time a comparison takes carries information about how much of the password the attacker got right. Over enough attempts against a network service, that timing difference can be measured and used to recover the secret one byte at a time. The two sites the report named were in src/server/websocket.js and src/server/routes.js. The recommended fix was crypto.timingSafeEqual(), which is exactly right.
Reproducing somebody else's finding before acting on it is standard practice for us, so the review started by confirming both reported sites in the current code rather than taking the report's word for them. Both were real, both were still present, and both were reachable from an authentication path.
The third instance
The report described a class of bug, not a pair of lines, so the next step was to look for every other place the same class could live. A grep across the authentication surface turned up a third comparison that the report had not covered, at src/server/auth.js:376, where the HTTP Authorization: Bearer <password> header is compared against the expected Bearer ${password} string with ===.
It is the same bug with the same exploitability, reached through a different entry point. The two reported sites sit on the WebSocket upgrade path and the HTTP route handler, both of which are what you find when you follow the login flow through the code. The Bearer header path is a separate way into the same authentication check, and it is the one you find by asking where else a password could possibly be compared rather than by following the flow you already know about.
That distinction is the whole argument for a second reading, and it is not a comment on the reporter's work. A report that lands two out of three real vulnerabilities in a stranger's codebase, unpaid and unasked, is a good report. But the coverage of a finding is a separate question from its correctness, and a fix that closes the two known doors on a house with three of them leaves the security posture where it started. Anybody remediating from that issue alone would have shipped a patch, closed the ticket, and still had a byte-by-byte timing oracle sitting on their Authorization header.
All three went into the same pull request, because splitting them would have meant shipping a partial fix for a vulnerability class and then arguing about the rest.
The remediation, and why it looks the way it does
The fix is a single helper used at all three call sites.
function safeCompare(a, b) {
if (typeof a !== 'string' || typeof b !== 'string') return false;
const ah = crypto.createHash('sha256').update(a).digest();
const bh = crypto.createHash('sha256').update(b).digest();
return crypto.timingSafeEqual(ah, bh);
}
The design decision worth explaining is the hash, because crypto.timingSafeEqual is the recommended primitive and the obvious implementation would call it on the two strings directly. That implementation has two problems. The first is operational: timingSafeEqual throws when the two buffers differ in length, so a caller that passes a submitted password of the wrong length gets an exception rather than a rejection, and the exception handling then becomes a new place for timing and behavior to diverge. The second is that even a correct length check leaks the length itself, which narrows the search space for an attacker who is going to be measuring anyway.
Hashing both sides with SHA-256 before the comparison resolves both. Two digests are always 32 bytes, so timingSafeEqual never throws and always does its full constant-time comparison, and the length of the original password no longer influences anything observable. Non-string inputs return false before reaching the hash at all.
Eight test cases went in with the fix, covering identical strings, different strings, a length mismatch, each of the non-string input types (null, undefined, a number, an object), unicode input, and the empty-string edge case. The helper is exposed both on the object returned by createAuth and at module level, so the WebSocket test mock can exercise the real implementation rather than reimplementing a constant-time comparison inside a test helper, which is a good way to end up testing the mock.
Reporting what was not ours
Running the test suite surfaced two failures in test/server/routes.test.js, where requests for the single-page app's index.html returned 404.
Both reproduce on a clean checkout of main with no changes applied, because TermBeam builds its frontend through a separate command that npm test does not invoke. Decoupling the frontend build from the test run is a reasonable speed decision, and the consequence is that anybody who runs the tests before building gets two red results that have nothing to do with their work.
The pull request said so explicitly, with the reproduction steps that show the failures predate the change. Stating the boundary of your own work is part of a clean handoff. A maintainer reading an unfamiliar contributor's pull request should not have to spend their first ten minutes working out whether the red test run is the contributor's fault, and a report that quietly ignores failures it did not cause teaches the reader to distrust the parts they cannot check.
The outcome
PR #204 was merged by the maintainer on April 23, 2026, the same day the review was published, closing all three timing side channels together.
What we take from it
The reason this one is on our site rather than only on the blog is that it is the cleanest available demonstration of something that is otherwise hard to argue for without sounding self-serving.
Two competent readings of the same authentication code, working weeks apart and from the same starting information, produced different coverage. The first found the two comparisons that sit on the path you walk when you trace a login. The second found those two plus one more, because it went looking for the bug class rather than the bug, and the third instance was on a code path nobody had traced. Neither reading was careless, so the difference lies in how the search was framed, and that framing is most of what you are buying when you pay someone to audit a system rather than waiting for a volunteer to file an issue.
The corollary is the part clients tend to find useful: an existing security report is a reason to look harder, not a reason to consider the area covered. When a finding arrives from outside, the productive response is to treat the reported instances as a sample of a class and go looking for the rest of it.
If you want a second reading of something that has already been reviewed once, the services page describes how we scope that work.