Add honeypot field validation to newsletter form

[localjo]

This is related to mozilla/bedrock#16231

• Implemented a honeypot field check for the 'office_fax' input to prevent spam submissions.
• Added error handling to display a generic error message when the honeypot is filled.

[Copilot]

Pull Request Overview

This PR implements a honeypot field check on the newsletter form to block spam and shows a generic error when the honeypot is filled.

• Adds lookup and validation for the office_fax honeypot field
• Calls handleFormError with a generic error code if the honeypot is non-empty
• Returns early to stop form submission on spam detection

Comments suppressed due to low confidence (2)

assets/js/protocol/newsletter.js:223

• Using ERROR_LIST.NOT_FOUND for a filled honeypot may be misleading; consider defining a dedicated HONEYPOT_ERROR code and updating handleFormError to display a clearer message to distinguish spam rejections.

                ERROR_LIST.NOT_FOUND

assets/js/protocol/newsletter.js:220

• Consider adding unit or integration tests to verify that submissions with a filled honeypot field are correctly rejected, ensuring this new validation remains enforced.

        const honeypotField = form.querySelector('input[name="office_fax"]');

[wen-2018]

r+

[janbrasna]

Newsletter component docs have example code and even list a few variants/states: https://protocol.mozilla.org/components/detail/newsletter--default

Is this feature important enough to be actually covered either in the code samples, or perhaps as its own variant page to document itself?

[janbrasna]

Given this may in some cases prevent submission, I think it could be worth not only mentioning this honeypot addition in the changelog; but even finding a good place in the component docs or example descriptions to note there's now this special cased office_fax value being used for honeypot validation.

[localjo]

@janbrasna After thinking through frontend validation options a bit more, I'm leaning towards not adding client-side honeypot validation to Protocol. The backend is already the security boundary and handles the real validation. I don't think frontend honeypot validation adds any real benefit since it's easily bypassed by bots and would theoretically never be seen by human users anyway.

Protocol should probably stay focused on UI concerns rather than security validation, and I don't think Protocol should be coupled to honeypot names that are specific to backend implementations. Also, publicly documenting the names and attributes of honeypot fields partially defeats the purpose of a honeypot by giving it away.

What do you think?

[janbrasna]

🤦 … and an error state designed to be served to rogue bots is not definitely a UI component feature. (Not mentioning it's always a matter of the backends or clients implementing the submissions; we don't have to go the extra mile and make the UX better for spambots really 🙈)

That's a very good point, yes.

(I wasn't familiar with the ask, reading up the ticket details now.)

So I think the original idea Alex mentioned was that in case there is something in that field for a real customer, and they would get rejected in the next step, the validation was supposed to warn them… however, if there's something populated in an invisible field (think: autofill plugins etc. for legitimate reasons and false positives), I doubt they'd be able to rectify the situation reasonably, so it's an error for error anyway.

I've added some more notes to the original issue to put it into a bit more context, but basically just JS shouldn't be the only validation, even though bots often script on top of user agents really so they might get stopped by form validation, but… the original form action post payload submission without relying on UA is still not protected by this.

[stephaniehobson]

Sorry for the slow review.

Could you please add a line to change log about this and update the newsletter unit tests to include a test for this?

• documented in the design system.
• recorded this change in CHANGELOG.md.

[localjo]

@stephaniehobson I think we're still waiting for more information about the situation from @stevejalim about whether or not this is the right approach: mozilla/bedrock#16231 (comment)

[stevejalim]

EDIT: thought this was a different issue 🤦

Still waiting to hear what we can do with a WAF in front of Basket