Skip to content

fix: handle confirmpassword honeypot better#3529

Merged
tefkah merged 5 commits intomainfrom
tfk/fix-create-user-honeypot
Mar 12, 2026
Merged

fix: handle confirmpassword honeypot better#3529
tefkah merged 5 commits intomainfrom
tfk/fix-create-user-honeypot

Conversation

@tefkah
Copy link
Member

@tefkah tefkah commented Mar 9, 2026

Issue(s) Resolved

The "confirmPassword" honeypot on the create user page was sometimes getting triggered by legit users.

This is fixed by:

  • adding data- attributes asking pw managers to ignore this field
  • checking that, in addition to this specific honeypot being triggered, we also check that the form is filled out pretty quick (under 5 secs)

Test Plan

  1. look at tests maybe

Screenshots (if applicable)

Optional

Notes/Context/Gotchas

Supporting Docs

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to reduce false positives from the confirmPassword honeypot on the user creation flow by discouraging password managers from filling the field and by adding a timing-based signal before restricting accounts.

Changes:

  • Added a server-side “fast honeypot” signal based on _passwordHoneypot + _formStartedAtMs, and return 403 when honeypot restrictions are triggered.
  • Updated the user creation client to send _formStartedAtMs and adjusted how honeypot values are gathered.
  • Added tests covering restriction behavior and session invalidation when honeypots are triggered.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
server/user/api.ts Adds fast honeypot signal logic and changes response behavior on honeypot trigger.
server/user/tests/api.test.ts Adds tests and mocking around honeypot-triggered restrictions and spam-tag behavior.
client/containers/UserCreate/UserCreate.tsx Tracks form start time and sends honeypot/timing fields in the create-user payload.
client/components/Honeypot/Honeypot.tsx Adds password-manager ignore data-* attributes (currently only in dev render path).
Comments suppressed due to low confidence (1)

client/components/Honeypot/Honeypot.tsx:55

  • The password-manager ignore data-* attributes were only added in the devMode render path. In production (the hidden input branch), these attributes are missing, so password managers can still autofill the honeypot and reproduce the original issue. Apply the same data-* ignore attributes to the non-dev <input> as well.
				<input
					type="text"
					name={name}
					autoComplete="off"
					style={{ width: 80, fontSize: 11, padding: '1px 4px' }}
					data-1p-ignore
					data-lpignore="true"
					data-bwignore
					data-form-type="other"
					data-protonpass-ignore
				/>
			</label>
		);
	}

	return (
		<input
			type="text"
			className="honeypot-input"
			name={name}
			tabIndex={-1}
			autoComplete="off"
			aria-hidden="true"
		/>

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

facebook,
googleScholar,
_honeypot: confirmPwHoneypot || websiteHoneypot,
_honeypot: websiteHoneypot || passwordHoneypot,
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The client still combines the website and confirmPassword honeypots into _honeypot, but the server’s new logic expects the confirmPassword honeypot in _passwordHoneypot (with _honeypot reserved for the website field). As-is, confirmPassword autofill will continue to trigger the server via _honeypot without the 5s timing check. Send passwordHoneypot in _passwordHoneypot and keep _honeypot for websiteHoneypot only.

Suggested change
_honeypot: websiteHoneypot || passwordHoneypot,
_honeypot: websiteHoneypot,
_passwordHoneypot: passwordHoneypot,

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +10
/** biome-ignore-all lint/correctness/noUndeclaredVariables: <explanation> */
import { User } from 'server/models';
// import { getSpamTagForUser } from 'server/spamTag/userQueries';
import { login, modelize, setup, teardown } from 'stubstub';

const tonyEmail = `${crypto.randomUUID()}@gmail.com`;
const autofillEmail = `${crypto.randomUUID()}@gmail.com`;
const restrictedEmail = `${crypto.randomUUID()}@gmail.com`;
const delayedHoneypotEmail = `${crypto.randomUUID()}@gmail.com`;
const spamEmail = `${crypto.randomUUID()}@gmail.com`;
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file-wide biome-ignore-all lint/correctness/noUndeclaredVariables (with a placeholder <explanation>) hides real issues and will allow accidental undeclared globals to slip into the test. Prefer importing vi (and any other globals that aren’t part of the configured test environment) instead of disabling the rule for the whole file.

Suggested change
/** biome-ignore-all lint/correctness/noUndeclaredVariables: <explanation> */
import { User } from 'server/models';
// import { getSpamTagForUser } from 'server/spamTag/userQueries';
import { login, modelize, setup, teardown } from 'stubstub';
const tonyEmail = `${crypto.randomUUID()}@gmail.com`;
const autofillEmail = `${crypto.randomUUID()}@gmail.com`;
const restrictedEmail = `${crypto.randomUUID()}@gmail.com`;
const delayedHoneypotEmail = `${crypto.randomUUID()}@gmail.com`;
const spamEmail = `${crypto.randomUUID()}@gmail.com`;
import { beforeAll, afterAll, describe, vi } from 'vitest';
import { randomUUID } from 'crypto';
import { User } from 'server/models';
// import { getSpamTagForUser } from 'server/spamTag/userQueries';
import { login, modelize, setup, teardown } from 'stubstub';
const tonyEmail = `${randomUUID()}@gmail.com`;
const autofillEmail = `${randomUUID()}@gmail.com`;
const restrictedEmail = `${randomUUID()}@gmail.com`;
const delayedHoneypotEmail = `${randomUUID()}@gmail.com`;
const spamEmail = `${randomUUID()}@gmail.com`;

Copilot uses AI. Check for mistakes.
Comment on lines +158 to +180
it('restricts and does not authenticate users when website honeypot is filled within 5 seconds', async () => {
const { restrictedSignup } = models;
const agent = await login();
await agent
.post('/api/users')
.send({
email: restrictedSignup.email,
hash: restrictedSignup.hash,
firstName: 'Bot',
lastName: 'Account',
password: 'oh!',
_passwordHoneypot: 'oh!',
_formStartedAtMs: Date.now(),
})
.expect(403);
const createdUser = await User.findOne({ where: { email: restrictedSignup.email } });
// if (!createdUser) {
// throw new Error('Expected user to be created');
// }
expect(createdUser).toBeDefined();
const { getSpamTagForUser } = await import('server/spamTag/userQueries');
const spamTag = await getSpamTagForUser(createdUser!.id);
expect(spamTag?.status).toEqual('confirmed-spam');
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test name says “website honeypot” but the payload uses _passwordHoneypot, and there are commented-out assertions left in the test body. Rename the test to match what it’s exercising (confirmPassword timing gate) and remove the commented-out code to keep the suite clear.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes MOM

Comment on lines +65 to 77
const { altcha, _honeypot, _passwordHoneypot, _formStartedAtMs, ...body } = { ...req.body };
const fastHoneypotSignal = getFastHoneypotSignal({
honeypot: _passwordHoneypot,
formStartedAtMs: _formStartedAtMs,
});

const newUser = await createUser(body);
if (isHoneypotFilled(req.body)) {
if (fastHoneypotSignal || _honeypot) {
await handleHoneypotTriggered(
newUser.id,
'create-user',
req.body.website ?? 'confirmPassword honeypot',
_honeypot || 'confirmPassword + very fast',
{
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new fast honeypot check only considers _passwordHoneypot, but the handler still treats any non-empty _honeypot as an immediate restriction. Since the client currently sends the confirmPassword honeypot value via _honeypot, legitimate autofill will still be restricted regardless of timing. Align the server logic so _honeypot represents the website honeypot only, and use _passwordHoneypot + _formStartedAtMs for the confirmPassword timing gate (and consider passing the actual fastHoneypotSignal value into handleHoneypotTriggered for logging).

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch robot

@tefkah tefkah merged commit d1b1e59 into main Mar 12, 2026
1 check passed
@tefkah tefkah deleted the tfk/fix-create-user-honeypot branch March 12, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants