From 5fb9b5c4dcf0607d2234cb40c8068c4f9f5942fc Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Tue, 28 Apr 2026 14:39:00 -0600 Subject: [PATCH 1/3] FOUR-30819 sanitize screen template config --- ProcessMaker/Helpers/ScreenTemplateHelper.php | 87 ++++++++++++++ ProcessMaker/Templates/ScreenTemplate.php | 12 +- .../Templates/Api/ScreenTemplateTest.php | 106 ++++++++++++++++++ 3 files changed, 202 insertions(+), 3 deletions(-) diff --git a/ProcessMaker/Helpers/ScreenTemplateHelper.php b/ProcessMaker/Helpers/ScreenTemplateHelper.php index 67a3684c3f..b6af576270 100644 --- a/ProcessMaker/Helpers/ScreenTemplateHelper.php +++ b/ProcessMaker/Helpers/ScreenTemplateHelper.php @@ -4,6 +4,32 @@ class ScreenTemplateHelper { + private const RENDERABLE_STRING_FIELDS = [ + 'ariaLabel', + 'content', + 'fieldValue', + 'helper', + 'label', + 'loadingLabel', + 'placeholder', + ]; + + /** + * Remove serialized Vue component definitions from screen config. + * + * Screen templates can contain old inspector metadata where inspector.type + * is a serialized Vue component object. That data is not needed at runtime + * and can reach renderer paths that expect Mustache templates to be strings. + */ + public static function sanitizeScreenConfig($config): array + { + if (!is_array($config)) { + return []; + } + + return self::sanitizeConfigValue($config); + } + /** * Remove screen components from the configuration based on the provided components. * @@ -402,4 +428,65 @@ public static function generateCss($cssArray) return $cssString; } + + private static function sanitizeConfigValue($value, ?string $key = null) + { + if ($key === 'validation' && is_array($value) && $value === []) { + return null; + } + + if (in_array($key, self::RENDERABLE_STRING_FIELDS, true)) { + return self::sanitizeRenderableString($value); + } + + if (!is_array($value)) { + return $value; + } + + if (array_is_list($value)) { + return array_map(fn ($item) => self::sanitizeConfigValue($item), $value); + } + + $sanitized = []; + foreach ($value as $childKey => $childValue) { + if ($childKey === 'inspector' && is_array($childValue)) { + $sanitized[$childKey] = array_map( + fn ($item) => self::sanitizeInspectorItem($item), + $childValue + ); + continue; + } + + $sanitized[$childKey] = self::sanitizeConfigValue($childValue, (string) $childKey); + } + + return $sanitized; + } + + private static function sanitizeInspectorItem($item) + { + if (!is_array($item)) { + return $item; + } + + $sanitized = []; + foreach ($item as $key => $value) { + if ($key === 'type' && is_array($value)) { + continue; + } + + $sanitized[$key] = self::sanitizeConfigValue($value, (string) $key); + } + + return $sanitized; + } + + private static function sanitizeRenderableString($value): ?string + { + if ($value === null || is_array($value)) { + return null; + } + + return is_string($value) ? $value : (string) $value; + } } diff --git a/ProcessMaker/Templates/ScreenTemplate.php b/ProcessMaker/Templates/ScreenTemplate.php index 742cd5048d..61a1b76339 100644 --- a/ProcessMaker/Templates/ScreenTemplate.php +++ b/ProcessMaker/Templates/ScreenTemplate.php @@ -725,6 +725,8 @@ public function applyTemplate(Request $request) $newTemplateScreen, $currentScreen, $templateOptions, $currentScreenPage); } + $currentScreen->config = ScreenTemplateHelper::sanitizeScreenConfig($currentScreen->config); + // Copy the updated config from screenVersion to screen // This is needed to save the changes to the current screen when applying a template to the screen version if ($hasVersionsPackage) { @@ -854,8 +856,10 @@ private function mergeFields($screen, $currentScreenPage, $newTemplateScreen, $t throw new MissingScreenPageException(); } - $templateComponents = ScreenTemplateHelper::getScreenComponents($newTemplateScreen->config, - $supportedComponents, false)[0]['items']; + $templateComponents = ScreenTemplateHelper::sanitizeScreenConfig( + ScreenTemplateHelper::getScreenComponents($newTemplateScreen->config, + $supportedComponents, false)[0]['items'] ?? [] + ); $screenConfig[$currentScreenPage]['items'] = array_merge($screenConfig[$currentScreenPage]['items'], $templateComponents); @@ -866,11 +870,13 @@ private function mergeFields($screen, $currentScreenPage, $newTemplateScreen, $t private function getTemplateComponents($newTemplateScreen, $templateOptions, $supportedComponents) { - return !in_array('Fields', $templateOptions) + $templateComponents = !in_array('Fields', $templateOptions) ? ScreenTemplateHelper::getScreenComponents($newTemplateScreen->config, $supportedComponents, false)[0]['items'] ?? [] : $newTemplateScreen->config[0]['items'] ?? []; + + return ScreenTemplateHelper::sanitizeScreenConfig($templateComponents); } private function setScreenConfig($screen) diff --git a/tests/Feature/Templates/Api/ScreenTemplateTest.php b/tests/Feature/Templates/Api/ScreenTemplateTest.php index 36e684888d..7745c2e0f9 100644 --- a/tests/Feature/Templates/Api/ScreenTemplateTest.php +++ b/tests/Feature/Templates/Api/ScreenTemplateTest.php @@ -477,6 +477,42 @@ public function testApplyFieldsToExistingScreen() $this->assertNotEmpty($updatedScreen->config[1]['items']); } + public function testApplyTemplateSanitizesSerializedInspectorComponents() + { + $templatePath = base_path(self::SCREEN_TEMPLATE_PATH); + $screenTemplateData = File::get($templatePath); + + $screenTemplate = $this->createScreenTemplateFromManifest($screenTemplateData); + + $screenPath = base_path(self::SCREEN_PATH); + $screenData = json_decode(File::get($screenPath), true); + + $newScreen = Screen::factory()->create([ + 'config' => $screenData, + ]); + + $route = route('api.template.applyTemplate', [ + 'type' => 'screen', + 'id' => $screenTemplate->id, + ]); + + $response = $this->apiCall('POST', $route, [ + 'screenId' => $newScreen->id, + 'templateOptions' => ['Fields', 'Layout'], + 'currentScreenPage' => 1, + ]); + + $response->assertStatus(200); + + if (class_exists(VersionHistory::class)) { + $updatedScreen = \ProcessMaker\Models\ScreenVersion::select('config')->where('screen_id', $newScreen->id)->latest()->firstOrFail(); + } else { + $updatedScreen = Screen::select('config')->where('id', $newScreen->id)->firstOrFail(); + } + + $this->assertScreenConfigSanitized($updatedScreen->config); + } + public function testApplyLayoutToExistingScreen() { // Create screen from template-manifest-with-css-fields-layout.json @@ -574,4 +610,74 @@ public function testApplyFullTemplateToExistingScreen() // Check that the screen has the custom_css $this->assertNotNull($updatedScreen->custom_css); } + + private function assertScreenConfigSanitized(array $config): void + { + foreach ($config as $page) { + $this->assertScreenConfigValueSanitized($page); + } + } + + private function createScreenTemplateFromManifest(string $manifest): ScreenTemplates + { + $screenTemplate = ScreenTemplates::make([ + 'unique_template_id' => 'serialized-inspector-regression', + 'name' => 'Serialized Inspector Regression', + 'description' => 'Template with serialized Vue component inspector metadata.', + 'version' => '1.0.0', + 'user_id' => null, + 'editing_screen_uuid' => null, + 'screen_category_id' => ScreenCategory::factory()->create()->getKey(), + 'screen_type' => 'FORM', + 'media_collection' => 'serialized-inspector-regression-media', + 'manifest' => $manifest, + 'screen_custom_css' => null, + 'is_public' => true, + 'is_default_template' => false, + 'is_system' => false, + 'asset_type' => null, + ]); + $screenTemplate->saveOrFail(); + + return $screenTemplate; + } + + private function assertScreenConfigValueSanitized($value): void + { + if (!is_array($value)) { + return; + } + + if (array_key_exists('inspector', $value) && is_array($value['inspector'])) { + foreach ($value['inspector'] as $inspector) { + if (is_array($inspector) && array_key_exists('type', $inspector)) { + $this->assertFalse(is_array($inspector['type'])); + } + } + } + + foreach (['content', 'label'] as $field) { + if (!array_key_exists($field, $value)) { + continue; + } + + $this->assertFalse(is_array($value[$field])); + } + + if ( + array_key_exists('tooltip', $value) + && is_array($value['tooltip']) + && array_key_exists('content', $value['tooltip']) + ) { + $this->assertFalse(is_array($value['tooltip']['content'])); + } + + if (array_key_exists('validation', $value) && is_array($value['validation'])) { + $this->assertNotEmpty($value['validation']); + } + + foreach ($value as $childValue) { + $this->assertScreenConfigValueSanitized($childValue); + } + } } From cf0339b963020f3e5e9a84319c654ef1056a7a20 Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Wed, 29 Apr 2026 09:47:12 -0600 Subject: [PATCH 2/3] FOUR-30819 Scope template sanitizer to imported content --- ProcessMaker/Templates/ScreenTemplate.php | 4 ++-- tests/Feature/Templates/Api/ScreenTemplateTest.php | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ProcessMaker/Templates/ScreenTemplate.php b/ProcessMaker/Templates/ScreenTemplate.php index 61a1b76339..964eb303eb 100644 --- a/ProcessMaker/Templates/ScreenTemplate.php +++ b/ProcessMaker/Templates/ScreenTemplate.php @@ -725,8 +725,6 @@ public function applyTemplate(Request $request) $newTemplateScreen, $currentScreen, $templateOptions, $currentScreenPage); } - $currentScreen->config = ScreenTemplateHelper::sanitizeScreenConfig($currentScreen->config); - // Copy the updated config from screenVersion to screen // This is needed to save the changes to the current screen when applying a template to the screen version if ($hasVersionsPackage) { @@ -856,6 +854,7 @@ private function mergeFields($screen, $currentScreenPage, $newTemplateScreen, $t throw new MissingScreenPageException(); } + // Sanitize only imported template components so existing screen config is not altered. $templateComponents = ScreenTemplateHelper::sanitizeScreenConfig( ScreenTemplateHelper::getScreenComponents($newTemplateScreen->config, $supportedComponents, false)[0]['items'] ?? [] @@ -876,6 +875,7 @@ private function getTemplateComponents($newTemplateScreen, $templateOptions, $su ?? [] : $newTemplateScreen->config[0]['items'] ?? []; + // Sanitize only imported template components so existing screen config is not altered. return ScreenTemplateHelper::sanitizeScreenConfig($templateComponents); } diff --git a/tests/Feature/Templates/Api/ScreenTemplateTest.php b/tests/Feature/Templates/Api/ScreenTemplateTest.php index 7745c2e0f9..08cc543e05 100644 --- a/tests/Feature/Templates/Api/ScreenTemplateTest.php +++ b/tests/Feature/Templates/Api/ScreenTemplateTest.php @@ -392,13 +392,11 @@ public function testSharedTemplateAuthorization() public function testApplyCssToExistingScreen() { - // Create a new screen with two pages and no custom_css - $screenPath = base_path(self::SCREEN_PATH); - $screenData = json_decode(File::get($screenPath), true); - + // Create a new screen with no config and no custom_css $screen = Screen::factory()->create([ - 'config' => $screenData, + 'config' => null, ]); + $this->assertNull($screen->config); $this->assertNull($screen->custom_css); // Create a screen template with custom_css @@ -420,13 +418,14 @@ public function testApplyCssToExistingScreen() $response->assertStatus(200); if (class_exists(VersionHistory::class)) { - $updatedScreen = \ProcessMaker\Models\ScreenVersion::select('custom_css')->where('screen_id', $screen->id)->latest()->firstOrFail(); + $updatedScreen = \ProcessMaker\Models\ScreenVersion::select('config', 'custom_css')->where('screen_id', $screen->id)->latest()->firstOrFail(); } else { - $updatedScreen = Screen::select('custom_css')->where('id', $screen->id)->firstOrFail(); + $updatedScreen = Screen::select('config', 'custom_css')->where('id', $screen->id)->firstOrFail(); } // Check that the screen has the custom_css $this->assertNotNull($updatedScreen->custom_css); + $this->assertNull($updatedScreen->config); } public function testApplyFieldsToExistingScreen() @@ -475,6 +474,7 @@ public function testApplyFieldsToExistingScreen() // Check that the screen config is not empty $this->assertNotEmpty($updatedScreen->config[1]['items']); + $this->assertScreenConfigSanitized($updatedScreen->config); } public function testApplyTemplateSanitizesSerializedInspectorComponents() From 809d56648f8a6fdd8b897a8485c217e964659edd Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Wed, 6 May 2026 12:15:59 -0600 Subject: [PATCH 3/3] FOUR-30819 Sanitize screen template renderable strings --- ProcessMaker/Helpers/ScreenTemplateHelper.php | 52 +++-- .../Templates/Api/ScreenTemplateTest.php | 194 +++++++++++++++++- 2 files changed, 206 insertions(+), 40 deletions(-) diff --git a/ProcessMaker/Helpers/ScreenTemplateHelper.php b/ProcessMaker/Helpers/ScreenTemplateHelper.php index b6af576270..3659eb71de 100644 --- a/ProcessMaker/Helpers/ScreenTemplateHelper.php +++ b/ProcessMaker/Helpers/ScreenTemplateHelper.php @@ -21,7 +21,7 @@ class ScreenTemplateHelper * is a serialized Vue component object. That data is not needed at runtime * and can reach renderer paths that expect Mustache templates to be strings. */ - public static function sanitizeScreenConfig($config): array + public static function sanitizeScreenConfig(mixed $config): array { if (!is_array($config)) { return []; @@ -429,41 +429,35 @@ public static function generateCss($cssArray) return $cssString; } - private static function sanitizeConfigValue($value, ?string $key = null) + private static function sanitizeConfigValue(mixed $value, ?string $key = null): mixed { if ($key === 'validation' && is_array($value) && $value === []) { - return null; - } - - if (in_array($key, self::RENDERABLE_STRING_FIELDS, true)) { - return self::sanitizeRenderableString($value); - } - - if (!is_array($value)) { - return $value; - } - - if (array_is_list($value)) { - return array_map(fn ($item) => self::sanitizeConfigValue($item), $value); - } + $sanitized = null; + } elseif (in_array($key, self::RENDERABLE_STRING_FIELDS, true)) { + $sanitized = self::sanitizeRenderableString($value); + } elseif (!is_array($value)) { + $sanitized = $value; + } elseif (array_is_list($value)) { + $sanitized = array_map(fn ($item) => self::sanitizeConfigValue($item), $value); + } else { + $sanitized = []; + foreach ($value as $childKey => $childValue) { + if ($childKey === 'inspector' && is_array($childValue)) { + $sanitized[$childKey] = array_map( + fn ($item) => self::sanitizeInspectorItem($item), + $childValue + ); + continue; + } - $sanitized = []; - foreach ($value as $childKey => $childValue) { - if ($childKey === 'inspector' && is_array($childValue)) { - $sanitized[$childKey] = array_map( - fn ($item) => self::sanitizeInspectorItem($item), - $childValue - ); - continue; + $sanitized[$childKey] = self::sanitizeConfigValue($childValue, (string) $childKey); } - - $sanitized[$childKey] = self::sanitizeConfigValue($childValue, (string) $childKey); } return $sanitized; } - private static function sanitizeInspectorItem($item) + private static function sanitizeInspectorItem(mixed $item): mixed { if (!is_array($item)) { return $item; @@ -481,10 +475,10 @@ private static function sanitizeInspectorItem($item) return $sanitized; } - private static function sanitizeRenderableString($value): ?string + private static function sanitizeRenderableString(mixed $value): string { if ($value === null || is_array($value)) { - return null; + return ''; } return is_string($value) ? $value : (string) $value; diff --git a/tests/Feature/Templates/Api/ScreenTemplateTest.php b/tests/Feature/Templates/Api/ScreenTemplateTest.php index 08cc543e05..aa2283e5fc 100644 --- a/tests/Feature/Templates/Api/ScreenTemplateTest.php +++ b/tests/Feature/Templates/Api/ScreenTemplateTest.php @@ -27,6 +27,16 @@ class ScreenTemplateTest extends TestCase private const SCREEN_PATH = 'tests/Feature/Templates/fixtures/screen-config-with-two-pages.json'; + private const RENDERABLE_STRING_FIELDS = [ + 'ariaLabel', + 'content', + 'fieldValue', + 'helper', + 'label', + 'loadingLabel', + 'placeholder', + ]; + public function testCreateScreenTemplate() { $screenCategoryId = ScreenCategory::factory()->create()->id; @@ -513,6 +523,54 @@ public function testApplyTemplateSanitizesSerializedInspectorComponents() $this->assertScreenConfigSanitized($updatedScreen->config); } + public function testApplyTemplateSanitizesDatePickerRenderableStrings() + { + $templatePath = base_path(self::SCREEN_TEMPLATE_PATH); + $screenTemplateData = $this->addDatePickerWithInvalidRenderableFields(File::get($templatePath)); + + $screenTemplate = $this->createScreenTemplateFromManifest($screenTemplateData); + + $screenPath = base_path(self::SCREEN_PATH); + $screenData = json_decode(File::get($screenPath), true); + + $newScreen = Screen::factory()->create([ + 'config' => $screenData, + ]); + + $route = route('api.template.applyTemplate', [ + 'type' => 'screen', + 'id' => $screenTemplate->id, + ]); + + $response = $this->apiCall('POST', $route, [ + 'screenId' => $newScreen->id, + 'templateOptions' => ['Fields'], + 'currentScreenPage' => 1, + ]); + + $response->assertStatus(200); + + if (class_exists(VersionHistory::class)) { + $updatedScreen = \ProcessMaker\Models\ScreenVersion::select('config')->where('screen_id', $newScreen->id)->latest()->firstOrFail(); + } else { + $updatedScreen = Screen::select('config')->where('id', $newScreen->id)->firstOrFail(); + } + + $datePicker = $this->findScreenConfigItem( + $updatedScreen->config[1]['items'], + 'FormDatePicker', + 'date' + ); + + $this->assertNotNull($datePicker); + $this->assertSame('', $datePicker['config']['label']); + $this->assertSame('', $datePicker['config']['placeholder']); + $this->assertSame('', $datePicker['config']['helper']); + $this->assertSame('', $datePicker['config']['ariaLabel']); + $this->assertNull($datePicker['config']['validation']); + $this->assertScreenConfigSanitized($updatedScreen->config); + } + public function testApplyLayoutToExistingScreen() { // Create screen from template-manifest-with-css-fields-layout.json @@ -642,12 +700,109 @@ private function createScreenTemplateFromManifest(string $manifest): ScreenTempl return $screenTemplate; } - private function assertScreenConfigValueSanitized($value): void + private function addDatePickerWithInvalidRenderableFields(string $manifest): string + { + $payload = json_decode($manifest, true); + $root = $payload['root']; + $config = json_decode($payload['export'][$root]['attributes']['config'], true); + + $config[0]['items'][] = [ + 'label' => 'Date Picker', + 'config' => [ + 'icon' => 'far fa-calendar-alt', + 'name' => 'date', + 'type' => 'datetime', + 'label' => null, + 'helper' => null, + 'placeholder' => null, + 'ariaLabel' => null, + 'minDate' => null, + 'maxDate' => null, + 'dataFormat' => 'date', + 'validation' => [], + ], + 'component' => 'FormDatePicker', + 'inspector' => [ + [ + 'type' => 'FormInput', + 'field' => 'label', + 'config' => [ + 'label' => 'Label', + 'helper' => 'The label describes the field\'s name', + ], + ], + [ + 'type' => 'ValidationSelect', + 'field' => 'validation', + 'config' => [ + 'label' => 'Validation Rules', + 'helper' => 'The validation rules needed for this field', + ], + ], + ], + 'editor-control' => 'FormDatePicker', + 'editor-component' => 'FormDatePicker', + ]; + + $payload['export'][$root]['attributes']['config'] = json_encode($config); + + return json_encode($payload); + } + + private function findScreenConfigItem(array $items, string $component, string $name): ?array + { + $matchingItem = null; + + foreach ($items as $item) { + if (!is_array($item)) { + continue; + } + + if (($item['component'] ?? null) === $component && ($item['config']['name'] ?? null) === $name) { + $matchingItem = $item; + break; + } + + $nestedItem = $this->findScreenConfigItemInChildren($item, $component, $name); + if ($nestedItem !== null) { + $matchingItem = $nestedItem; + break; + } + } + + return $matchingItem; + } + + private function findScreenConfigItemInChildren(array $item, string $component, string $name): ?array + { + foreach ($this->screenConfigChildCollections($item) as $childItems) { + $nestedItem = $this->findScreenConfigItem($childItems, $component, $name); + if ($nestedItem !== null) { + return $nestedItem; + } + } + + return null; + } + + private function assertScreenConfigValueSanitized(mixed $value): void { if (!is_array($value)) { return; } + $this->assertInspectorItemsSanitized($value); + $this->assertRenderableFieldsSanitized($value); + $this->assertTooltipSanitized($value); + $this->assertValidationSanitized($value); + + foreach ($value as $childValue) { + $this->assertScreenConfigValueSanitized($childValue); + } + } + + private function assertInspectorItemsSanitized(array $value): void + { if (array_key_exists('inspector', $value) && is_array($value['inspector'])) { foreach ($value['inspector'] as $inspector) { if (is_array($inspector) && array_key_exists('type', $inspector)) { @@ -655,29 +810,46 @@ private function assertScreenConfigValueSanitized($value): void } } } + } - foreach (['content', 'label'] as $field) { + private function assertRenderableFieldsSanitized(array $value): void + { + foreach (self::RENDERABLE_STRING_FIELDS as $field) { if (!array_key_exists($field, $value)) { continue; } - $this->assertFalse(is_array($value[$field])); + $this->assertIsString($value[$field]); } + } - if ( - array_key_exists('tooltip', $value) - && is_array($value['tooltip']) - && array_key_exists('content', $value['tooltip']) - ) { - $this->assertFalse(is_array($value['tooltip']['content'])); + private function assertTooltipSanitized(array $value): void + { + $tooltip = $value['tooltip'] ?? null; + if (is_array($tooltip) && array_key_exists('content', $tooltip)) { + $this->assertIsString($value['tooltip']['content']); } + } + private function assertValidationSanitized(array $value): void + { if (array_key_exists('validation', $value) && is_array($value['validation'])) { $this->assertNotEmpty($value['validation']); } + } - foreach ($value as $childValue) { - $this->assertScreenConfigValueSanitized($childValue); + private function screenConfigChildCollections(array $item): array + { + $collections = []; + + if (array_key_exists('items', $item) && is_array($item['items'])) { + $collections[] = $item['items']; } + + if (array_is_list($item)) { + $collections[] = $item; + } + + return $collections; } }