From ff84e9ea0f53a6fc295d649453fa1d56a31b6120 Mon Sep 17 00:00:00 2001 From: robbond Date: Mon, 9 Nov 2026 14:04:31 +0000 Subject: [PATCH] fix(security): use server-configured PET notification recipient --- actions/services/notifyDirectService.js | 3 +- pages/api/email/contactus-pet-notification.js | 15 ++-- pages/contactus.js | 20 +---- pages/myportal/contactus.js | 20 +---- tests/phase21/email-handler-contract.test.cjs | 79 +++++++++++++++++-- tests/phase7/service-behaviour.test.cjs | 29 +++---- 6 files changed, 99 insertions(+), 67 deletions(-) diff --git a/actions/services/notifyDirectService.js b/actions/services/notifyDirectService.js index ff7f6663..8f1f70ea 100644 --- a/actions/services/notifyDirectService.js +++ b/actions/services/notifyDirectService.js @@ -47,9 +47,8 @@ export const sendContactUsConfirmation = async (formValues, language) => { } }; -export const sendContactUsPETNotification = async (formValues, petEmail) => { +export const sendContactUsPETNotification = async (formValues) => { const mailData = { - "petEmail": petEmail, "contactEmail": formValues.contactEmail, "contactSubject": formValues.contactSubject, "contactbody": formValues.contactbody, diff --git a/pages/api/email/contactus-pet-notification.js b/pages/api/email/contactus-pet-notification.js index 510a874c..79b765d1 100644 --- a/pages/api/email/contactus-pet-notification.js +++ b/pages/api/email/contactus-pet-notification.js @@ -15,7 +15,6 @@ const allowedSubjects = [ ]; const allowedFields = new Set([ - "petEmail", "contactEmail", "contactSubject", "contactbody", @@ -120,22 +119,22 @@ export default async function ApiProxy(req, res) { ); } - const petEmail = sanitizeString(requestBody.petEmail); + const petEmail = sanitizeString(process.env.PETEMAIL); const contactEmail = sanitizeString(requestBody.contactEmail); const contactSubject = sanitizeString(requestBody.contactSubject); const contactBody = requestBody.contactbody; if ( - typeof requestBody.petEmail !== "string" || + typeof process.env.PETEMAIL !== "string" || !isNonEmptyString(petEmail) || petEmail.length > 200 || !emailRegex.test(petEmail) ) { - return validationError( - res, - "INVALID_PET_EMAIL", - "petEmail must be a valid email address of 200 characters or fewer" - ); + return respondError(res, { + status: 500, + code: "PET_RECIPIENT_NOT_CONFIGURED", + message: "PET notification recipient is not configured" + }); } if ( diff --git a/pages/contactus.js b/pages/contactus.js index 4339ce3c..7c91a7ab 100644 --- a/pages/contactus.js +++ b/pages/contactus.js @@ -21,7 +21,6 @@ import { sendContactUsConfirmation, sendContactUsPETNotification } from "../actions/services/notifyDirectService"; -import { wrapper } from "../store/store"; import { useState } from "react"; @@ -322,7 +321,7 @@ let ContactUs = (props) => { sendContactEmail(formValues); createCRMTask(formValues); formValues.typeOfContact == "plq" && - sendPETEmail(formValues, props.petEmail); + sendPETEmail(formValues); console.log("Form submitted:", formValues); }; @@ -350,11 +349,9 @@ let ContactUs = (props) => { } }; - const sendPETEmail = async (formValues, petEmail) => { - console.log(petEmail); - + const sendPETEmail = async (formValues) => { try { - const res = await sendContactUsPETNotification(formValues, petEmail); + const res = await sendContactUsPETNotification(formValues); console.log("Email sent successfully."); setFormSubmitted(true); @@ -519,17 +516,6 @@ const mapStateToProps = (state) => { }; }; -export const getServerSideProps = wrapper.getServerSideProps( - (store) => async (ctx) => { - const { req, locale } = ctx; - return { - props: { - petEmail: process.env.PETEMAIL - } - }; - } -); - export default connect( mapStateToProps, mapDispatchToProps diff --git a/pages/myportal/contactus.js b/pages/myportal/contactus.js index c7f47266..4334fc25 100644 --- a/pages/myportal/contactus.js +++ b/pages/myportal/contactus.js @@ -21,7 +21,6 @@ import { sendContactUsConfirmation, sendContactUsPETNotification } from "../../actions/services/notifyDirectService"; -import { wrapper } from "../../store/store"; import { useState } from "react"; @@ -322,7 +321,7 @@ let ContactUs = (props) => { sendContactEmail(formValues); createCRMTask(formValues); formValues.typeOfContact == "plq" && - sendPETEmail(formValues, props.petEmail); + sendPETEmail(formValues); console.log("Form submitted:", formValues); }; @@ -350,11 +349,9 @@ let ContactUs = (props) => { } }; - const sendPETEmail = async (formValues, petEmail) => { - console.log(petEmail); - + const sendPETEmail = async (formValues) => { try { - const res = await sendContactUsPETNotification(formValues, petEmail); + const res = await sendContactUsPETNotification(formValues); console.log("Email sent successfully."); setFormSubmitted(true); @@ -520,17 +517,6 @@ const mapStateToProps = (state) => { }; }; -export const getServerSideProps = wrapper.getServerSideProps( - (store) => async (ctx) => { - const { req, locale } = ctx; - return { - props: { - petEmail: process.env.PETEMAIL - } - }; - } -); - export default connect( mapStateToProps, mapDispatchToProps diff --git a/tests/phase21/email-handler-contract.test.cjs b/tests/phase21/email-handler-contract.test.cjs index 45323bc7..da6d4537 100644 --- a/tests/phase21/email-handler-contract.test.cjs +++ b/tests/phase21/email-handler-contract.test.cjs @@ -436,8 +436,10 @@ const loadContactConfirmationEndpoint = ({ sendEmail } = {}) => { const withContactOrigins = async (fn) => { const previousNextAuthUrl = process.env.NEXTAUTH_URL; const previousCyApiRoot = process.env.CY_API_ROOT; + const previousPetEmail = process.env.PETEMAIL; process.env.NEXTAUTH_URL = trustedContactOrigin; process.env.CY_API_ROOT = trustedWelshContactOrigin; + process.env.PETEMAIL = "pet@example.com"; try { await fn(); @@ -447,6 +449,9 @@ const withContactOrigins = async (fn) => { if (previousCyApiRoot === undefined) delete process.env.CY_API_ROOT; else process.env.CY_API_ROOT = previousCyApiRoot; + + if (previousPetEmail === undefined) delete process.env.PETEMAIL; + else process.env.PETEMAIL = previousPetEmail; } }; @@ -775,7 +780,6 @@ const validContactPetRequest = (overrides = {}) => ({ "content-type": "application/json; charset=utf-8" }, body: { - petEmail: "pet@example.com", contactEmail: "applicant@example.com", contactSubject: contactSubjects.plq.en, contactbody: "Please help with my enquiry.", @@ -931,18 +935,83 @@ test("Contact Us PET notification enforces its exact field set", async () => { }); }); -for (const petEmail of [ +test("Contact Us PET notification rejects obsolete caller-supplied recipient", async () => { + await assertContactPetRejected({ + req: validContactPetRequest({ + body: { + ...validContactPetRequest().body, + petEmail: "pet@example.com" + } + }), + status: 400, + code: "INVALID_REQUEST_FIELDS" + }); +}); + +for (const configuredRecipient of [undefined, "invalid"]) { + test(`Contact Us PET notification fails safely for configured recipient ${String(configuredRecipient)}`, async () => { + await withContactOrigins(async () => { + if (configuredRecipient === undefined) delete process.env.PETEMAIL; + else process.env.PETEMAIL = configuredRecipient; + + const loaded = loadContactPetEndpoint(); + const res = createRes(); + + await loaded.mod.default(validContactPetRequest(), res); + + assert.strictEqual(res.state.statusCode, 500); + assert.strictEqual( + res.state.jsonBody.error.code, + "PET_RECIPIENT_NOT_CONFIGURED" + ); + assert.strictEqual(loaded.notifyCalls.length, 0); + assert.strictEqual( + JSON.stringify(res.state.jsonBody).includes( + String(configuredRecipient) + ), + false + ); + }); + }); +} + +test("Contact Us PET notification rejects caller recipient before configured recipient validation", async () => { + await withContactOrigins(async () => { + delete process.env.PETEMAIL; + const loaded = loadContactPetEndpoint(); + const res = createRes(); + + await loaded.mod.default( + validContactPetRequest({ + body: { + ...validContactPetRequest().body, + petEmail: "attacker@example.com" + } + }), + res + ); + + assert.strictEqual(res.state.statusCode, 400); + assert.strictEqual( + res.state.jsonBody.error.code, + "INVALID_REQUEST_FIELDS" + ); + assert.strictEqual(loaded.notifyCalls.length, 0); + }); +}); + +for (const contactEmail of [ undefined, "invalid", `${"a".repeat(190)}@example.com` ]) { - test(`Contact Us PET notification rejects recipient ${String(petEmail).slice(0, 30)}`, async () => { + test(`Contact Us PET notification rejects contact email ${String(contactEmail).slice(0, 30)}`, async () => { await assertContactPetRejected({ req: validContactPetRequest({ - body: { ...validContactPetRequest().body, petEmail } + body: { ...validContactPetRequest().body, contactEmail } }), status: 400, - code: "INVALID_PET_EMAIL" + code: "INVALID_CONTACT_EMAIL" }); }); } diff --git a/tests/phase7/service-behaviour.test.cjs b/tests/phase7/service-behaviour.test.cjs index d27cb60a..286ebcb9 100644 --- a/tests/phase7/service-behaviour.test.cjs +++ b/tests/phase7/service-behaviour.test.cjs @@ -480,10 +480,7 @@ test("notify/sendContactUsPETNotification posts only the dedicated PET payload", ignored: "not sent" }; - const result = await notify.sendContactUsPETNotification( - formValues, - "pet@example.com" - ); + const result = await notify.sendContactUsPETNotification(formValues); assert.deepStrictEqual(normalize(result), { sent: true }); assert.strictEqual( @@ -492,7 +489,6 @@ test("notify/sendContactUsPETNotification posts only the dedicated PET payload", ); assert.strictEqual(axios.calls[0].config.method, "post"); assert.deepStrictEqual(normalize(axios.calls[0].config.data), { - petEmail: "pet@example.com", contactEmail: "person@example.com", contactSubject: "Questions about Planning legislation or policy", contactbody: "Please review https://example.invalid", @@ -513,15 +509,12 @@ test("notify/sendContactUsPETNotification logs and rethrows on failure", async ( }); await assert.rejects( - notify.sendContactUsPETNotification( - { - contactEmail: "person@example.com", - contactSubject: "Questions about Planning legislation or policy", - contactbody: "Message", - typeOfContact: "plq" - }, - "pet@example.com" - ), + notify.sendContactUsPETNotification({ + contactEmail: "person@example.com", + contactSubject: "Questions about Planning legislation or policy", + contactbody: "Message", + typeOfContact: "plq" + }), (thrown) => thrown === error ); assert.strictEqual(logger.calls.length, 1); @@ -538,7 +531,7 @@ test("public and portal Contact Us pages preserve PET gating and submission orde 'formValues.typeOfContact == "plq" &&' ); const petCall = source.indexOf( - "sendPETEmail(formValues, props.petEmail);" + "sendPETEmail(formValues);" ); assert.notStrictEqual(confirmationCall, -1); @@ -552,9 +545,7 @@ test("public and portal Contact Us pages preserve PET gating and submission orde true ); assert.strictEqual( - source.includes( - "sendContactUsPETNotification(formValues, petEmail)" - ), + source.includes("sendContactUsPETNotification(formValues)"), true ); assert.strictEqual( @@ -562,6 +553,8 @@ test("public and portal Contact Us pages preserve PET gating and submission orde true ); assert.strictEqual(source.includes("sendEmail(templateId,"), false); + assert.strictEqual(source.includes("props.petEmail"), false); + assert.strictEqual(source.includes("process.env.PETEMAIL"), false); } });