From c2a0dc3838c1d70fdeb3ce058bf99fcf6a8c2581 Mon Sep 17 00:00:00 2001 From: kevinkim-ogp Date: Wed, 12 Nov 2025 21:39:55 +0800 Subject: [PATCH 1/4] fix: allow workflow update before sending webhook --- .../gathersg/__tests__/auth/schema.test.ts | 39 +++++++++++++++++++ .../backend/src/apps/gathersg/auth/schema.ts | 29 ++++++++++---- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts b/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts index edfa3d9d2..dca30af64 100644 --- a/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts +++ b/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts @@ -64,6 +64,19 @@ describe('gathersg auth schema', () => { }) expect(result.success).toBe(true) }) + + it('accepts Workflow updatedBy with createdBy', () => { + const result = schema.safeParse({ + updatedBy: { + name: 'Workflow', + }, + createdBy: { + email: 'creator@example.com', + name: 'Creator', + }, + }) + expect(result.success).toBe(true) + }) }) describe('invalid cases - missing email', () => { @@ -240,4 +253,30 @@ describe('gathersg auth schema', () => { expect(result.success).toBe(false) }) }) + + describe('Workflow invalid case', () => { + it('rejects updatedBy with name "Workflow" when email is present', () => { + const result = schema.safeParse({ + updatedBy: { + name: 'Workflow', + email: 'workflow@example.com', + }, + }) + expect(result.success).toBe(false) + }) + + it('rejects Workflow updatedBy when email is present even with valid createdBy', () => { + const result = schema.safeParse({ + updatedBy: { + name: 'Workflow', + email: 'workflow@example.com', + }, + createdBy: { + email: 'creator@example.com', + name: 'Creator', + }, + }) + expect(result.success).toBe(false) + }) + }) }) diff --git a/packages/backend/src/apps/gathersg/auth/schema.ts b/packages/backend/src/apps/gathersg/auth/schema.ts index fad0e7a6e..fbc799197 100644 --- a/packages/backend/src/apps/gathersg/auth/schema.ts +++ b/packages/backend/src/apps/gathersg/auth/schema.ts @@ -1,5 +1,22 @@ import { z } from 'zod' +function validateUpdatedBy(updatedBy: { email?: string; name?: string }) { + /** + * SPECIAL CASE + * if the workflow had an update step before sending the webhook, + * the webhook will return with: + * { + * updatedBy: { + * name: 'Workflow', + * }, + * } + */ + if (updatedBy.name === 'Workflow') { + return !updatedBy.email + } + return !!updatedBy.email +} + /** * check for potential infinite loop * if the webhook is not triggered by a user, it will not contain @@ -9,7 +26,8 @@ const schema = z .object({ updatedBy: z .object({ - email: z.string().min(1), + // does not have email when update is done by an instant workflow + email: z.string().min(1).nullish(), name: z.string().min(1), }) .nullish(), @@ -30,14 +48,9 @@ const schema = z (data) => { const { updatedBy, createdBy, formsg } = data || {} - // if both updatedBy and createdBy exist, only check updatedBy.email - if (updatedBy && createdBy) { - return !!updatedBy.email - } - - // if updatedBy exists, check for updatedBy.email + // if updatedBy exists, check updatedBy first if (updatedBy) { - return !!updatedBy.email + return validateUpdatedBy(updatedBy) } // if only createdBy exists, check for createdBy.email From 0c4aeec5d54a8e760775f44135c778f68ec8586d Mon Sep 17 00:00:00 2001 From: kevinkim-ogp Date: Wed, 12 Nov 2025 21:44:41 +0800 Subject: [PATCH 2/4] refactor, dont need extra function --- .../backend/src/apps/gathersg/auth/schema.ts | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/packages/backend/src/apps/gathersg/auth/schema.ts b/packages/backend/src/apps/gathersg/auth/schema.ts index fbc799197..cd7bac2d2 100644 --- a/packages/backend/src/apps/gathersg/auth/schema.ts +++ b/packages/backend/src/apps/gathersg/auth/schema.ts @@ -1,22 +1,5 @@ import { z } from 'zod' -function validateUpdatedBy(updatedBy: { email?: string; name?: string }) { - /** - * SPECIAL CASE - * if the workflow had an update step before sending the webhook, - * the webhook will return with: - * { - * updatedBy: { - * name: 'Workflow', - * }, - * } - */ - if (updatedBy.name === 'Workflow') { - return !updatedBy.email - } - return !!updatedBy.email -} - /** * check for potential infinite loop * if the webhook is not triggered by a user, it will not contain @@ -50,7 +33,20 @@ const schema = z // if updatedBy exists, check updatedBy first if (updatedBy) { - return validateUpdatedBy(updatedBy) + if (updatedBy.name === 'Workflow') { + /** + * SPECIAL CASE + * if the workflow had an update step before sending the webhook, + * the webhook will return with: + * { + * updatedBy: { + * name: 'Workflow', + * }, + * } + */ + return !updatedBy.email + } + return !!updatedBy.email } // if only createdBy exists, check for createdBy.email From 2d153ae7389ee77e8a92c08194357a8f130818ed Mon Sep 17 00:00:00 2001 From: kevinkim-ogp Date: Wed, 12 Nov 2025 21:50:45 +0800 Subject: [PATCH 3/4] chore: change checks to nullish just in case --- .../gathersg/__tests__/auth/schema.test.ts | 44 ++++++++++++++++--- .../backend/src/apps/gathersg/auth/schema.ts | 4 +- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts b/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts index dca30af64..cb1bb21e5 100644 --- a/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts +++ b/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts @@ -1,5 +1,4 @@ import { describe, expect, it } from 'vitest' -import type { SafeParseError } from 'zod' import schema from '../../auth/schema' @@ -96,12 +95,16 @@ describe('gathersg auth schema', () => { }, }) expect(result.success).toBe(false) - if (!result.success) { - const { issues } = (result as SafeParseError).error - expect( - issues.some((i) => i.message.includes('createdBy.email is required')), - ).toBe(true) - } + }) + + it('rejects createdBy with email null when not FormSG', () => { + const result = schema.safeParse({ + createdBy: { + name: 'Regular User', + email: null, + }, + }) + expect(result.success).toBe(false) }) it('rejects when both updatedBy and createdBy exist but updatedBy has no email', () => { @@ -117,6 +120,20 @@ describe('gathersg auth schema', () => { expect(result.success).toBe(false) }) + it('rejects when both updatedBy and createdBy exist but updatedBy email is null', () => { + const result = schema.safeParse({ + updatedBy: { + name: 'Updater', + email: null, + }, + createdBy: { + email: 'creator@example.com', + name: 'Creator', + }, + }) + expect(result.success).toBe(false) + }) + it('rejects empty object (neither updatedBy nor createdBy)', () => { const result = schema.safeParse({}) expect(result.success).toBe(false) @@ -156,6 +173,19 @@ describe('gathersg auth schema', () => { expect(result.success).toBe(false) }) + it('rejects FormSG createdBy with submissionId null', () => { + const result = schema.safeParse({ + createdBy: { + name: 'FormSG', + }, + formsg: { + formId: 'form-123', + submissionId: null, + }, + }) + expect(result.success).toBe(false) + }) + it('rejects FormSG createdBy with empty formId', () => { const result = schema.safeParse({ createdBy: { diff --git a/packages/backend/src/apps/gathersg/auth/schema.ts b/packages/backend/src/apps/gathersg/auth/schema.ts index cd7bac2d2..c160f469d 100644 --- a/packages/backend/src/apps/gathersg/auth/schema.ts +++ b/packages/backend/src/apps/gathersg/auth/schema.ts @@ -16,8 +16,8 @@ const schema = z .nullish(), createdBy: z .object({ - email: z.string().min(1).optional(), - name: z.string().min(1).optional(), + email: z.string().min(1).nullish(), + name: z.string().min(1).nullish(), }) .nullish(), formsg: z From 656a11ddffb82361123c380e0adc905f9ddb5d42 Mon Sep 17 00:00:00 2001 From: kevinkim-ogp Date: Thu, 13 Nov 2025 07:37:59 +0800 Subject: [PATCH 4/4] fix: check for createdBy if updatedBy workflow --- .../gathersg/__tests__/auth/schema.test.ts | 34 +++++++++++++++++++ .../backend/src/apps/gathersg/auth/schema.ts | 8 +++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts b/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts index cb1bb21e5..10b429a3c 100644 --- a/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts +++ b/packages/backend/src/apps/gathersg/__tests__/auth/schema.test.ts @@ -308,5 +308,39 @@ describe('gathersg auth schema', () => { }) expect(result.success).toBe(false) }) + + it('rejects Workflow updatedBy without createdBy', () => { + const result = schema.safeParse({ + updatedBy: { + name: 'Workflow', + }, + }) + expect(result.success).toBe(false) + }) + + it('rejects Workflow updatedBy when createdBy is missing email', () => { + const result = schema.safeParse({ + updatedBy: { + name: 'Workflow', + }, + createdBy: { + name: 'Creator', + }, + }) + expect(result.success).toBe(false) + }) + + it('rejects Workflow updatedBy when createdBy email is null', () => { + const result = schema.safeParse({ + updatedBy: { + name: 'Workflow', + }, + createdBy: { + email: null, + name: 'Creator', + }, + }) + expect(result.success).toBe(false) + }) }) }) diff --git a/packages/backend/src/apps/gathersg/auth/schema.ts b/packages/backend/src/apps/gathersg/auth/schema.ts index c160f469d..f537ae3e1 100644 --- a/packages/backend/src/apps/gathersg/auth/schema.ts +++ b/packages/backend/src/apps/gathersg/auth/schema.ts @@ -36,15 +36,19 @@ const schema = z if (updatedBy.name === 'Workflow') { /** * SPECIAL CASE - * if the workflow had an update step before sending the webhook, + * if the workflow has an update step before calling the webhook, * the webhook will return with: * { * updatedBy: { * name: 'Workflow', * }, * } + * + * Best effort check that createdBy must contain both email and name + * to know that its not an API creating the case and an instant workflow + * calling Plumber again. */ - return !updatedBy.email + return !updatedBy.email && !!createdBy?.email && !!createdBy?.name } return !!updatedBy.email }