From a5aac3c21ee4a405016b2fedef9627573ff3f07c Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Tue, 23 Jun 2026 00:55:42 +0200 Subject: [PATCH] Logic function handler name hardened validation (#21956) # Introduction Introduce centralized handlerName validation for the logic function handlerName inside the flat logic function validator Even if not safe by definition, avoid string interpolation inside the local driver executor when retrieving the handler name from the parent module Review in cubic --- .../drivers/lambda.driver.ts | 8 + .../local-child-process-runner.service.ts | 10 +- .../flat-logic-function-validator.service.ts | 23 +++ ...tion-handler-name.integration-spec.ts.snap | 141 ++++++++++++++++++ ...-function-handler-name.integration-spec.ts | 127 ++++++++++++++++ ...-function-handler-name.integration-spec.ts | 119 +++++++++++++++ 6 files changed, 424 insertions(+), 4 deletions(-) create mode 100644 packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-handler-name.integration-spec.ts.snap create mode 100644 packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-handler-name.integration-spec.ts create mode 100644 packages/twenty-server/test/integration/metadata/suites/logic-function/successful-logic-function-handler-name.integration-spec.ts diff --git a/packages/twenty-server/src/engine/core-modules/logic-function/logic-function-drivers/drivers/lambda.driver.ts b/packages/twenty-server/src/engine/core-modules/logic-function/logic-function-drivers/drivers/lambda.driver.ts index 4e0e7f663f..5f9493ca40 100644 --- a/packages/twenty-server/src/engine/core-modules/logic-function/logic-function-drivers/drivers/lambda.driver.ts +++ b/packages/twenty-server/src/engine/core-modules/logic-function/logic-function-drivers/drivers/lambda.driver.ts @@ -27,6 +27,7 @@ import { LambdaLayerManagerService } from 'src/engine/core-modules/logic-functio import { LambdaToolFunctionsService } from 'src/engine/core-modules/logic-function/logic-function-drivers/drivers/lambda/services/lambda-tool-functions.service'; import { buildLogicFunctionTimeoutResult } from 'src/engine/core-modules/logic-function/logic-function-drivers/drivers/lambda/utils/build-logic-function-timeout-result.util'; import { parseLambdaLogResult } from 'src/engine/core-modules/logic-function/logic-function-drivers/drivers/lambda/utils/parse-lambda-log-result.util'; +import { HANDLER_NAME_REGEX } from 'src/engine/metadata-modules/logic-function/constants/handler.contant'; import { LogicFunctionExecutionStatus } from 'src/engine/metadata-modules/logic-function/dtos/logic-function-execution-result.dto'; import { LogicFunctionExecutionMode } from 'src/engine/metadata-modules/logic-function/logic-function.entity'; import { @@ -158,6 +159,13 @@ export class LambdaDriver implements LogicFunctionDriver { currentPhase = LambdaExecutionPhase.FETCH_CODE; const invokeFlowStart = Date.now(); + if (!HANDLER_NAME_REGEX.test(flatLogicFunction.handlerName)) { + throw new LogicFunctionException( + `Invalid handlerName "${flatLogicFunction.handlerName}": must be a valid JavaScript identifier or dotted path`, + LogicFunctionExceptionCode.INVALID_LOGIC_FUNCTION_INPUT, + ); + } + const executorPayload: LambdaDriverExecutorPayload = { params: payload, env: env ?? {}, diff --git a/packages/twenty-server/src/engine/core-modules/logic-function/logic-function-drivers/drivers/local/services/local-child-process-runner.service.ts b/packages/twenty-server/src/engine/core-modules/logic-function/logic-function-drivers/drivers/local/services/local-child-process-runner.service.ts index a15a693ea1..5e4db53762 100644 --- a/packages/twenty-server/src/engine/core-modules/logic-function/logic-function-drivers/drivers/local/services/local-child-process-runner.service.ts +++ b/packages/twenty-server/src/engine/core-modules/logic-function/logic-function-drivers/drivers/local/services/local-child-process-runner.service.ts @@ -73,6 +73,7 @@ export class LocalChildProcessRunnerService { } const runnerPath = join(dir, '__runner.cjs'); + const handlerAccessor = `mod?.${handlerName.split('.').join('?.')}`; const code = ` // Auto-generated. Do not edit. const { pathToFileURL } = require('node:url'); @@ -81,8 +82,9 @@ export class LocalChildProcessRunnerService { try { const builtUrl = pathToFileURL(${JSON.stringify(builtFileAbsPath)}); const mod = await import(builtUrl.href); - if (typeof mod.${handlerName} !== 'function') { - throw new Error('Export "${handlerName}" not found in function bundle'); + const handlerFn = ${handlerAccessor}; + if (typeof handlerFn !== 'function') { + throw new Error('Export "' + ${JSON.stringify(handlerName)} + '" not found in function bundle'); } let payload = undefined; @@ -90,7 +92,7 @@ export class LocalChildProcessRunnerService { process.on('message', async (msg) => { if (!msg || msg.type !== 'run') return; try { - const out = await mod.${handlerName}(msg.payload); + const out = await handlerFn(msg.payload); process.send && process.send({ ok: true, result: out }); process.exit(0); } catch (err) { @@ -102,7 +104,7 @@ export class LocalChildProcessRunnerService { // Fallback: read payload from argv[2] (JSON) and print to stdout const json = process.argv[2]; payload = json ? JSON.parse(json) : undefined; - const out = await mod.${handlerName}(payload); + const out = await handlerFn(payload); process.stdout.write(JSON.stringify({ ok: true, result: out })); process.exit(0); } diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-logic-function-validator.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-logic-function-validator.service.ts index f1e8a1c534..ccf573544e 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-logic-function-validator.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-logic-function-validator.service.ts @@ -7,6 +7,7 @@ import { isDefined } from 'twenty-shared/utils'; import { validateFilePath } from 'src/engine/core-modules/file-storage/utils/validate-file-path.util'; import { findFlatEntityByUniversalIdentifier } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-universal-identifier.util'; +import { HANDLER_NAME_REGEX } from 'src/engine/metadata-modules/logic-function/constants/handler.contant'; import { LogicFunctionExecutionMode } from 'src/engine/metadata-modules/logic-function/logic-function.entity'; import { LogicFunctionExceptionCode } from 'src/engine/metadata-modules/logic-function/logic-function.exception'; import { isLogicFunctionReadyForPrebuiltInstall } from 'src/engine/metadata-modules/logic-function/utils/is-logic-function-ready-for-prebuilt-install.util'; @@ -81,6 +82,17 @@ export class FlatLogicFunctionValidatorService { } } + if ( + isDefined(flatEntityUpdate.handlerName) && + !HANDLER_NAME_REGEX.test(flatEntityUpdate.handlerName) + ) { + validationResult.errors.push({ + code: LogicFunctionExceptionCode.INVALID_LOGIC_FUNCTION_INPUT, + message: t`handlerName must be a valid JavaScript identifier or dotted path`, + userFriendlyMessage: msg`Handler name is invalid`, + }); + } + const mergedPrebuiltState = { executionMode: flatEntityUpdate.executionMode ?? @@ -202,6 +214,17 @@ export class FlatLogicFunctionValidatorService { } } + if ( + !isDefined(flatLogicFunctionToValidate.handlerName) || + !HANDLER_NAME_REGEX.test(flatLogicFunctionToValidate.handlerName) + ) { + validationResult.errors.push({ + code: LogicFunctionExceptionCode.INVALID_LOGIC_FUNCTION_INPUT, + message: t`handlerName must be a valid JavaScript identifier or dotted path`, + userFriendlyMessage: msg`Handler name is invalid`, + }); + } + if ( flatLogicFunctionToValidate.executionMode === LogicFunctionExecutionMode.PREBUILT && diff --git a/packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-handler-name.integration-spec.ts.snap b/packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-handler-name.integration-spec.ts.snap new file mode 100644 index 0000000000..959feb7b0a --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-handler-name.integration-spec.ts.snap @@ -0,0 +1,141 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Logic function creation via manifest sync should fail for invalid handlerName when handlerName contains an invalid identifier character 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "handlerName must be a valid JavaScript identifier or dotted path", + "userFriendlyMessage": "Handler name is invalid", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "create", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Handler name is invalid", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Logic function creation via manifest sync should fail for invalid handlerName when handlerName injects a statement via semicolon 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "handlerName must be a valid JavaScript identifier or dotted path", + "userFriendlyMessage": "Handler name is invalid", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "create", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Handler name is invalid", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Logic function creation via manifest sync should fail for invalid handlerName when handlerName is a malformed dotted path (consecutive dots) 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "handlerName must be a valid JavaScript identifier or dotted path", + "userFriendlyMessage": "Handler name is invalid", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "create", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Handler name is invalid", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Logic function creation via manifest sync should fail for invalid handlerName when handlerName is empty 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "handlerName must be a valid JavaScript identifier or dotted path", + "userFriendlyMessage": "Handler name is invalid", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "create", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Handler name is invalid", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; diff --git a/packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-handler-name.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-handler-name.integration-spec.ts new file mode 100644 index 0000000000..7fa047affe --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-handler-name.integration-spec.ts @@ -0,0 +1,127 @@ +import { expectOneNotInternalServerErrorSnapshot } from 'test/integration/graphql/utils/expect-one-not-internal-server-error-snapshot.util'; +import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util'; +import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util'; +import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util'; +import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util'; +import { uploadApplicationFile } from 'test/integration/metadata/suites/application/utils/upload-application-file.util'; +import { + type LogicFunctionManifest, + type Manifest, +} from 'twenty-shared/application'; +import { + type EachTestingContext, + eachTestingContextFilter, +} from 'twenty-shared/testing'; +import { v4 as uuidv4 } from 'uuid'; + +const TEST_APP_ID = uuidv4(); +const TEST_ROLE_ID = uuidv4(); + +const VALID_BUILT_PATH = 'src/logic-functions/handler.mjs'; + +const buildLogicFunction = ( + overrides: Partial = {}, +): LogicFunctionManifest => ({ + universalIdentifier: uuidv4(), + name: 'TestLogicFunction', + description: 'A test logic function', + sourceHandlerPath: 'src/logic-functions/handler.ts', + builtHandlerPath: VALID_BUILT_PATH, + builtHandlerChecksum: 'valid-checksum', + handlerName: 'handler', + ...overrides, +}); + +const buildManifest = ( + logicFunctionOverrides: Partial = {}, +): Manifest => + buildBaseManifest({ + appId: TEST_APP_ID, + roleId: TEST_ROLE_ID, + overrides: { + logicFunctions: [buildLogicFunction(logicFunctionOverrides)], + }, + }); + +type TestContext = { + manifest: Manifest; +}; + +const FAILING_HANDLER_NAME_TEST_CASES: EachTestingContext[] = [ + { + title: 'when handlerName injects a statement via semicolon', + context: { + manifest: buildManifest({ + handlerName: 'handler; process.exit(1)', + }), + }, + }, + { + title: 'when handlerName contains an invalid identifier character', + context: { + manifest: buildManifest({ + handlerName: 'my-handler', + }), + }, + }, + { + title: 'when handlerName is a malformed dotted path (consecutive dots)', + context: { + manifest: buildManifest({ + handlerName: 'default..handler', + }), + }, + }, + { + title: 'when handlerName is empty', + context: { + manifest: buildManifest({ + handlerName: '', + }), + }, + }, +]; + +describe('Logic function creation via manifest sync should fail for invalid handlerName', () => { + beforeAll(async () => { + await setupApplicationForSync({ + applicationUniversalIdentifier: TEST_APP_ID, + name: 'Test Handler Name Logic App', + description: 'App for testing handlerName validation on creation', + sourcePath: 'test-handler-name-logic', + }); + + jest.useRealTimers(); + + await uploadApplicationFile({ + applicationUniversalIdentifier: TEST_APP_ID, + fileFolder: 'BuiltLogicFunction', + filePath: VALID_BUILT_PATH, + fileBuffer: Buffer.from('dummy built handler content'), + filename: 'handler.mjs', + contentType: 'application/javascript', + expectToFail: false, + }); + + jest.useFakeTimers(); + }, 60000); + + afterAll(async () => { + await cleanupApplicationAndAppRegistration({ + applicationUniversalIdentifier: TEST_APP_ID, + }); + }); + + it.each(eachTestingContextFilter(FAILING_HANDLER_NAME_TEST_CASES))( + '$title', + async ({ context }) => { + const { errors } = await syncApplication({ + manifest: context.manifest, + expectToFail: true, + }); + + expectOneNotInternalServerErrorSnapshot({ errors }); + }, + 60000, + ); +}); diff --git a/packages/twenty-server/test/integration/metadata/suites/logic-function/successful-logic-function-handler-name.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/logic-function/successful-logic-function-handler-name.integration-spec.ts new file mode 100644 index 0000000000..7dca12b8b5 --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/logic-function/successful-logic-function-handler-name.integration-spec.ts @@ -0,0 +1,119 @@ +import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util'; +import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util'; +import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util'; +import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util'; +import { uploadApplicationFile } from 'test/integration/metadata/suites/application/utils/upload-application-file.util'; +import { + type LogicFunctionManifest, + type Manifest, +} from 'twenty-shared/application'; +import { + type EachTestingContext, + eachTestingContextFilter, +} from 'twenty-shared/testing'; +import { v4 as uuidv4 } from 'uuid'; + +const TEST_APP_ID = uuidv4(); +const TEST_ROLE_ID = uuidv4(); + +const VALID_BUILT_PATH = 'src/logic-functions/handler.mjs'; + +const buildLogicFunction = ( + overrides: Partial = {}, +): LogicFunctionManifest => ({ + universalIdentifier: uuidv4(), + name: 'TestLogicFunction', + description: 'A test logic function', + sourceHandlerPath: 'src/logic-functions/handler.ts', + builtHandlerPath: VALID_BUILT_PATH, + builtHandlerChecksum: 'valid-checksum', + handlerName: 'handler', + ...overrides, +}); + +const buildManifest = ( + logicFunctionOverrides: Partial = {}, +): Manifest => + buildBaseManifest({ + appId: TEST_APP_ID, + roleId: TEST_ROLE_ID, + overrides: { + logicFunctions: [buildLogicFunction(logicFunctionOverrides)], + }, + }); + +type TestContext = { + manifest: Manifest; +}; + +const SUCCESSFUL_HANDLER_NAME_TEST_CASES: EachTestingContext[] = [ + { + title: 'when handlerName is a simple identifier', + context: { + manifest: buildManifest({ + handlerName: 'main', + }), + }, + }, + { + title: 'when handlerName is the SDK-emitted dotted path', + context: { + manifest: buildManifest({ + handlerName: 'default.config.handler', + }), + }, + }, + { + title: 'when handlerName uses identifier-legal symbols ($ and _)', + context: { + manifest: buildManifest({ + handlerName: '$run_handler2', + }), + }, + }, +]; + +describe('Logic function creation via manifest sync should succeed for valid handlerName', () => { + beforeAll(async () => { + await setupApplicationForSync({ + applicationUniversalIdentifier: TEST_APP_ID, + name: 'Test Valid Handler Name Logic App', + description: 'App for testing valid handlerName values on creation', + sourcePath: 'test-valid-handler-name-logic', + }); + + jest.useRealTimers(); + + await uploadApplicationFile({ + applicationUniversalIdentifier: TEST_APP_ID, + fileFolder: 'BuiltLogicFunction', + filePath: VALID_BUILT_PATH, + fileBuffer: Buffer.from('dummy built handler content'), + filename: 'handler.mjs', + contentType: 'application/javascript', + expectToFail: false, + }); + + jest.useFakeTimers(); + }, 60000); + + afterAll(async () => { + await cleanupApplicationAndAppRegistration({ + applicationUniversalIdentifier: TEST_APP_ID, + }); + }); + + it.each(eachTestingContextFilter(SUCCESSFUL_HANDLER_NAME_TEST_CASES))( + '$title', + async ({ context }) => { + const { data, errors } = await syncApplication({ + manifest: context.manifest, + expectToFail: false, + }); + + expect(errors).toBeUndefined(); + expect(data?.syncApplication).toBeDefined(); + }, + 60000, + ); +});