From 30217543118dcb27178df926692c62a8f8eb5d40 Mon Sep 17 00:00:00 2001 From: Alex Price Date: Fri, 24 Jul 2026 18:54:55 +0100 Subject: [PATCH 1/2] Harden login honeypot against autofill false positives The hidden honeypot field was named company_website, which browser autofill and password managers populate for real users. A filled honeypot is treated as a bot: the request feigns success but sends no magic link, so genuine logins were silently dropped. Rename the field to referral_code, a name no autofill category matches, and add a shared Honeypot component that sets password-manager opt-out attributes (data-1p-ignore, data-lpignore, data-bwignore, data-form-type). Log honeypot trips so a false positive is no longer invisible. Also fix login.test.ts: the "redirects authenticated user" test mock.module()s the auth middleware and never restored it, leaking the stub into other test files when the full suite runs in one process. Ported from the-8020-pathway@c5a9d5171f9. Co-Authored-By: Claude Sonnet 5 --- src/server/components/honeypot.tsx | 33 +++++++++++++++++++++++ src/server/controllers/auth/login.test.ts | 30 ++++++++++++++++----- src/server/controllers/auth/login.tsx | 8 +++++- src/server/services/captcha.ts | 5 +++- src/server/templates/login.tsx | 19 ++----------- 5 files changed, 70 insertions(+), 25 deletions(-) create mode 100644 src/server/components/honeypot.tsx diff --git a/src/server/components/honeypot.tsx b/src/server/components/honeypot.tsx new file mode 100644 index 0000000..eecc5a9 --- /dev/null +++ b/src/server/components/honeypot.tsx @@ -0,0 +1,33 @@ +import { HONEYPOT_FIELD } from "../services/captcha"; + +/** + * Bot-trap field. A real text input, hidden from humans and — critically — + * flagged so password managers and browser autofill leave it empty. Bots that + * fill every field trip it; the controller then silently discards the request. + * + * The opt-out attributes matter: a filled honeypot is treated as a bot and the + * submission is dropped with no user and no email, so a false positive means a + * real person is turned away invisibly. The `data-*-ignore` hints below are + * honored by 1Password, LastPass, Bitwarden and Dashlane, and the neutral field + * name (no email/name/address/company/url token) keeps native autofill away. + */ +export const Honeypot = () => ( + +); diff --git a/src/server/controllers/auth/login.test.ts b/src/server/controllers/auth/login.test.ts index a0c811d..ef07dd6 100644 --- a/src/server/controllers/auth/login.test.ts +++ b/src/server/controllers/auth/login.test.ts @@ -1,13 +1,23 @@ import { afterAll, beforeEach, describe, expect, mock, test } from "bun:test"; import { createHash } from "node:crypto"; import { SQL } from "bun"; +import { redirectIfAuthenticated } from "../../middleware/auth"; import { clearRateLimitLog } from "../../middleware/rate-limit"; -import { clearUsedChallenges, issueChallenge } from "../../services/captcha"; +import { + clearUsedChallenges, + HONEYPOT_FIELD, + issueChallenge, +} from "../../services/captcha"; import type { LoginState } from "../../templates/login"; import { createBunRequest, findSetCookie } from "../../test-utils/bun-request"; import { cleanupTestData } from "../../test-utils/helpers"; import { stateHelpers } from "../../utils/state"; +// Snapshot the real function value now. `redirectIfAuthenticated` is a live +// binding, so once a test mock.module()s the auth middleware the import itself +// points at the stub — capturing it here keeps a handle on the genuine one. +const realRedirectIfAuthenticated = redirectIfAuthenticated; + // Solve a challenge the way the client would, for the captcha-enabled tests. const solveChallenge = ( challenge: ReturnType, @@ -134,11 +144,19 @@ describe("Login Controller", () => { }, }); - const response = await mockedLogin.index(request); + try { + const response = await mockedLogin.index(request); - expect(response.status).toBe(303); - expect(response.headers.get("location")).toBe("/"); - expect(mockRedirectIfAuthenticated).toHaveBeenCalled(); + expect(response.status).toBe(303); + expect(response.headers.get("location")).toBe("/"); + expect(mockRedirectIfAuthenticated).toHaveBeenCalled(); + } finally { + // Restore the real middleware so this module mock doesn't leak into + // other test files when the whole suite runs in one process. + mock.module("../../middleware/auth", () => ({ + redirectIfAuthenticated: realRedirectIfAuthenticated, + })); + } }); }); @@ -268,7 +286,7 @@ describe("Login Controller", () => { test("silently discards a submission with the honeypot filled", async () => { const formData = new FormData(); formData.append("email", "bot@example.com"); - formData.append("company_website", "http://spam.example"); + formData.append(HONEYPOT_FIELD, "http://spam.example"); const request = createBunRequest("http://localhost:3000/login", { method: "POST", diff --git a/src/server/controllers/auth/login.tsx b/src/server/controllers/auth/login.tsx index 81b17b1..a27b46a 100644 --- a/src/server/controllers/auth/login.tsx +++ b/src/server/controllers/auth/login.tsx @@ -9,6 +9,7 @@ import { verifyCaptcha, } from "../../services/captcha"; import { getEmailService } from "../../services/email"; +import { log } from "../../services/logger"; import type { LoginState } from "../../templates/login"; import { Login } from "../../templates/login"; import { redirect, render } from "../../utils/response"; @@ -37,8 +38,13 @@ export const login = { const formData = await req.formData(); // 2. Honeypot: a filled hidden field means a bot. Feign success — create - // nothing, send nothing — so the bot has no signal to adapt to. + // nothing, send nothing — so the bot has no signal to adapt to. Logged + // because a false positive drops a real sign-in with no other trace. if (formData.get(HONEYPOT_FIELD)) { + log.warn( + "login", + `honeypot tripped, dropping submission for ${formData.get("email") ?? "unknown"}`, + ); setFlash(req, { state: "email-sent" }); return redirect("/login"); } diff --git a/src/server/services/captcha.ts b/src/server/services/captcha.ts index fcf7c0a..e938132 100644 --- a/src/server/services/captcha.ts +++ b/src/server/services/captcha.ts @@ -10,7 +10,10 @@ import { computeHMAC, generateSecureToken, verifyHMAC } from "../utils/crypto"; // Shared field names — the login template, controller, and client solver must all // agree on these. The client bundle hardcodes the same literals (it cannot import // this module, which pulls in node:crypto), and a test cross-checks the two. -export const HONEYPOT_FIELD = "company_website"; +// Neutral name on purpose: no email/name/address/company/url token, so native +// browser autofill won't populate it (that produced real dropped signups). The +// Honeypot component also sets password-manager opt-out attributes. +export const HONEYPOT_FIELD = "referral_code"; export const CAPTCHA_SOLUTION_FIELD = "captcha_solution"; // How hard the client has to work: it brute-forces a number in [0, maxnumber]. diff --git a/src/server/templates/login.tsx b/src/server/templates/login.tsx index 98fbeb7..3619218 100644 --- a/src/server/templates/login.tsx +++ b/src/server/templates/login.tsx @@ -1,10 +1,10 @@ import { CaptchaWidget } from "../components/captcha-widget"; import { Flash } from "../components/flash"; import { FormField } from "../components/form-field"; +import { Honeypot } from "../components/honeypot"; import { BaseLayout } from "../components/layouts"; import { Logo } from "../components/logo"; import type { CaptchaChallenge } from "../services/captcha"; -import { HONEYPOT_FIELD } from "../services/captcha"; export interface LoginState { state?: "email-sent" | "validation-error"; @@ -65,22 +65,7 @@ export const Login = ({ state, challenge }: LoginProps) => { /> - {/* Honeypot — a real field hidden off-screen. Bots fill it; humans - never see it. Submissions with it set are silently discarded. */} - + From ffb14855470ea74c4f76fc9dbc1ad220b5764cf9 Mon Sep 17 00:00:00 2001 From: Alex Price Date: Fri, 24 Jul 2026 18:58:25 +0100 Subject: [PATCH 2/2] fix: deflake tampered-challenge captcha test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flipped the target hash's last hex digit to a fixed "0", a no-op 1-in-16 of the time when the digest already ended in "0" — the "tampered" payload was then identical to the original, still passed signature verification, and the test failed asserting false. Pick a digit guaranteed to differ from the original instead. Co-Authored-By: Claude Sonnet 5 --- src/server/services/captcha.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/server/services/captcha.test.ts b/src/server/services/captcha.test.ts index f4701a6..4a2a8c8 100644 --- a/src/server/services/captcha.test.ts +++ b/src/server/services/captcha.test.ts @@ -101,9 +101,13 @@ describe("Captcha Service", () => { test("rejects a tampered challenge (signature mismatch)", () => { const challenge = issueChallenge(); - // Flip the target hash without re-signing. + // Flip the target hash's last hex digit without re-signing. Pick a digit + // guaranteed to differ from the original, not a fixed "0" (that's a no-op + // 1-in-16 of the time, when the digest already ends in "0"). + const lastDigit = challenge.challenge.at(-1); + const flipped = lastDigit === "0" ? "1" : "0"; const tampered = solveChallenge(challenge, { - challenge: `${challenge.challenge.slice(0, -1)}0`, + challenge: `${challenge.challenge.slice(0, -1)}${flipped}`, }); expect(verifyCaptcha(tampered)).toBe(false); });