xss audit by Gawuww · Pull Request #746 · Crocoblock/jetformbuilder

Gawuww · 2026-11-04T08:44:54Z

Issue - https://github.com/Crocoblock/issues-tracker/issues/20753

…tor data Crocoblock/issues-tracker#20753

…resets Crocoblock/issues-tracker#20764

github-actions · 2026-11-04T08:47:54Z

🤖 AI PR Review

Risk level: medium

Review

Summary
This PR applies a broad XSS-hardening sweep across the admin/editor/frontend code: it removes unsafe eval usage in calculated fields, tightens HTML macro handling and URL checks, ensures template/preview strings are HTML-escaped where appropriate, and escapes values used in editor innerHTML. Overall the changes look well scoped and follow WP security practices (escaping, sanitizing, forbidding dangerous attributes). I focused on security, backwards-compatibility and tests to add.

What I checked (important files)

assets/src/frontend/calculated.field/functions.js
- Removed eval() and added toMillis() to only treat numeric timestamps as dates. This eliminates executing arbitrary strings — very good.
assets/src/frontend/main/html.macro/CalculatedHtmlString.js
- Improved sanitization: trusted SafeHtml replaced with temporary placeholders, whole string sanitized, placeholders restored afterwards. This is the correct approach to preserve known-safe markup while sanitizing the rest.
assets/src/frontend/main/html.macro/macroUrl.js (new)
- Introduces URL attribute handling for macros in href/src-like attributes. Adds:
  - list of URL macro attributes (href, src, action, etc)
  - getUrlScheme/isAllowedMacroUrl using allowed protocols from window.JetFormBuilderSettings (fallback to DEFAULT_PROTOCOLS)
  - getStaticTemplate to compare template scheme vs resulting scheme
- observeMacroAttr now forbids dangerous attributes (on*, innerHTML, srcdoc) and removes attribute when a macro result yields a disallowed URL scheme.
assets/src/frontend/media.field/signal.js
- escapeAttr now escapes single quotes too (prevents issues when preview template uses single-quoted attributes). Good fix.
assets/src/package/preset/helpers/escapeText.js and usage
- Adds escapeText and uses it where HTML was previously injected into editor innerHTML for presets/conditions/validation. This closes potential editor XSS of form config strings.
assets/src/package/block-conditions/store/reducer.js, validation reducer, etc.
- Use escapeText when constructing HTML strings for editor display.
compatibility/* widgets
- Clarified PHPCS ignore comments, making explicit that renderer output is escaped.

Positive notes

Replacing eval() with numeric parsing for date conversion removes a major attack vector.
Blocking dangerous macro attributes (on*, innerHTML, srcdoc) is a strong protection.
Using placeholders to preserve trusted HTML while sanitizing the rest is the right pattern.
Editor-side escaping of configuration strings prevents injections through imported/generated form settings.

Potential regressions / behaviour changes (callouts)

Calculated field date handling (assets/src/frontend/calculated.field/functions.js)
- Before: convertMillisToDateString used eval() on the formula result. Authors could rely on JS expressions (e.g. a formula that returns an expression to eval). After the change only numeric values (numbers or numeric strings) are treated as timestamps. This is almost certainly correct for security, but it is a behavioural change that could break edge-case formulas that relied on eval. Please review whether any internal or third-party formulas relied on the ability to eval non-numeric strings and consider a deprecation note.
Macro URL handling (assets/src/frontend/main/html.macro/macroUrl.js + observeMacroAttr)
- The new code blocks attribute values whose resulting URL scheme is not in allowed protocols (window.JetFormBuilderSettings.allowedProtocols or DEFAULT_PROTOCOLS). DEFAULT_PROTOCOLS does not include the data: scheme in this PR — that means data: URIs produced by macros will be stripped by the frontend logic unless allowed via JetFormBuilderSettings.allowedProtocols.
- This may break existing templates that intentionally use data: URIs (for inline images) or custom schemes. You should:
  - Document the change and how to allow custom protocols via the JetFormBuilderSettings filter/value.
  - Consider whether data: should be included in the default list or intentionally excluded (security trade-off). If data: must be allowed in common use-cases, add it to DEFAULT_PROTOCOLS or make a note.
observeMacroAttr: node[ attrName ] assignment vs setAttribute
- The code sets node[ attrName ] = result and removes the attribute when invalid. This is usually OK for standard properties, but ensure it works for all attribute types in all browsers (e.g. svg attributes). The new logic already early-returns for non-string node values; still recommend an additional test sweep across common cases (img/src, a/href, form/action, video/poster, etc.).
sanitizeHtml usage
- CalculatedHtmlString relies on sanitizeHtml(). Confirm that sanitizeHtml is available on the frontend build (was used before) and that its configuration (allowed tags/attributes) is consistent with intended HTML output and localization.

Tests / QA recommended

Add JS unit tests and integration tests covering:
- Calculated field: ensure numeric, numeric-string, and non-numeric inputs behave as expected (and that no code executes). Verify date formatting result correctness.
- Macro URL behavior: test macros in href/src for cases where (a) result is relative URL, (b) result has same scheme as template, (c) result has new allowed scheme, (d) result has disallowed scheme — check attribute present/removed.
- Editor HTML escapes: reducers that build innerHTML (block-conditions, validation) should have snapshot tests to ensure strings are properly escaped and no double-escaping occurs.
- Media preview template escaping: preview template rendering includes escaped file names/URLs, check that quotes and single quotes are safe.
- Regressions: test any 3rd-party add-ons that might have relied on previous eval behavior.

Other small suggestions

Add a short inline comment near DEFAULT_PROTOCOLS explaining why data: is omitted (security) and how a site owner can allow it if desired.
Consider logging (or a debug event) when an attribute is removed due to disallowed URL scheme — useful for debugging why a link disappeared.
Update changelog and release notes to mention potential behavioral change for calculated fields (eval -> numeric only) so integrators are aware.

Missing tests / documentation

No new unit/integration tests were added in this PR. Given security-critical changes (removal of eval, macro URL scheme blocking, escaping in editor), please add tests and a short developer note documenting the change and any available filters (JetFormBuilderSettings.allowedProtocols) to recover prior behaviour.

Conclusion
This is a necessary and well-scoped hardening PR. The implementation follows WP security practices and addresses multiple XSS vectors. Address the potential regressions (notably data: URIs and calc-field eval semantics) with tests and documentation before merging to avoid surprising breakages for integrators.

Suggested changelog entry

- FIX: Hardened frontend and editor against XSS — removed eval from calculated fields, sanitized HTML macros and media previews, blocked dangerous macro attributes and validated URL schemes (affects calculated fields, HTML macros, media previews and editor displays)

Copilot

🟡 Changes recommended

Unsafe attribute stripping creates a stored-XSS path, while incomplete protection of encoded brackets allows shortcode execution during later processing.

4 open issues

4 files not reviewed

assets/build/admin/package.js: Generated file
assets/build/frontend/calculated.field.js: Generated file
assets/build/frontend/media.field.js: Generated file
compatibility/jet-engine/assets/build/editor.js: Generated file

What changed in this PR

This PR hardens JetFormBuilder against cross-site scripting across form rendering, dynamic presets, HTML notifications, and admin interfaces.

Changes:

Adds context-specific escaping, URL-scheme checks, and protections against executing visitor-controlled shortcodes or formula content.
Sanitizes external data and HTML previews while preserving supported formatting.
Adds security regression tests and updates built assets and cache versions.

File	Description
tests/wpunit/ToolsSanitizeInvariantsTest.php	Documents input sanitization boundaries.
tests/wpunit/StatusQueryParamMarkupTest.php	Tests status-query markup filtering.
tests/wpunit/RestErrorMessageSanitizationTest.php	Guards against raw exception responses.
tests/wpunit/RepeaterPresetRenderTest.php	Tests safe repeater preset rendering.
tests/wpunit/RedirectUrlSchemeTest.php	Tests redirect scheme restrictions.
tests/wpunit/PresetShortcodeNeutralizationTest.php	Tests preset shortcode neutralization.
tests/wpunit/PresetRichPipelineShortcodeTest.php	Tests shortcode protection in rich-content processing.
tests/wpunit/PresetRichAttributeShortcodeTest.php	Tests preset protection in labels and descriptions.
tests/wpunit/PresetRenderMatrixTest.php	Checks presets across field types.
tests/wpunit/PresetAccessControlTest.php	Enables compatibility behavior for callback-order testing.
tests/wpunit/PaypalPayerDataSanitizationTest.php	Tests payer and shipping sanitization.
tests/wpunit/MacroAttributeInjectionTest.php	Tests removal of injected macro attributes.
tests/wpunit/InlineCssEscapingTest.php	Tests style-element breakout protection.
tests/wpunit/GeneratedOptionLabelShortcodeTest.php	Tests generated-label shortcode protection.
tests/wpunit/FormArgumentsEscapingTest.php	Tests form-level argument escaping.
tests/wpunit/FieldRenderEscapingTest.php	Adds broad field-rendering injection checks.
tests/wpunit/EscapeOutputSuppressionsTest.php	Checks escaping-suppression explanations and counts.
tests/wpunit/CalculatedMetaFormulaTest.php	Tests safe metadata insertion into formulas.
tests/wpunit/AdminColumnsFuzzTest.php	Checks admin columns for active markup.
tests/wpunit/AdminColumnCellHtmlTest.php	Tests cell sanitization and formatting compatibility.
tests/wpunit/AddonsDescriptionSanitizationTest.php	Tests remote add-on description sanitization.
tests/_data/url-scheme-corpus.json	Shares URL cases between PHP and JavaScript checks.
templates/fields/textarea-field.php	Neutralizes shortcodes in preset textarea content.
templates/common/start-form-row.php	Documents existing output-escaping safeguards.
templates/common/field-description.php	Clarifies the escaping-suppression comment.
templates/common/end-form.php	Documents form-ID integer conversion.
modules/wysiwyg/blocks/wysiwyg/block-template.php	Neutralizes preset brackets after editor escaping.
modules/validation/ssr/ssr-registry-migration-notice.php	Clarifies existing escaping comments.
modules/validation/rules-controller.php	Escapes fatal error messages.
modules/validation/module.php	Casts inline-script form IDs to integers.
modules/shortcode/assets/build/block.editor.asset.php	Updates editor asset cache version.
modules/rich-content/module.php	Adds HTML-aware processing and preset protections.
modules/rich-content/macros-parser.php	Adds field-value callbacks for email escaping.
modules/rich-content/macro-runtime-values.php	Tracks per-pass escaping and neutralization flags.
modules/rest-api/endpoints/install-migrations-endpoint.php	Sanitizes migration error responses.
modules/post-type/actions/traits/import-form-trait.php	Escapes import failure messages.
modules/post-type/actions/import-action.php	Explains an existing output suppression.
modules/option-field/blocks/select/block-template.php	Neutralizes generated select-label shortcodes.
modules/option-field/blocks/radio/block-render.php	Protects generated radio labels.
modules/option-field/blocks/checkbox/block-render.php	Protects generated checkbox labels.
modules/option-field/assets/build/editor.asset.php	Updates editor asset cache version.
modules/onboarding/use-form/rest-api/use-form-route/use-form-endpoint.php	Sanitizes onboarding error responses.
modules/onboarding/use-form/assets/build/index.asset.php	Updates onboarding asset cache version.
modules/media-cleanup/media-ownership-migration-notice.php	Clarifies an existing escaping comment.
modules/html-parser/assets/src/parseHTMLtoBlocks.js	Parses HTML in an inert template.
modules/html-parser/assets/src/getFormInnerFields.js	Restricts attributes and uses inert parsing.
modules/html-parser/assets/build/parser.asset.php	Updates parser asset cache version.
modules/gateways/paypal/scenarios-logic/pay-now.php	Cleans payer and shipping data before storage.
modules/gateways/paypal/rest-endpoints/fetch-pay-now-editor.php	Sanitizes PayPal error responses.
modules/gateways/pages/export-page.php	Escapes payment export errors.
modules/framework/blocks-style-manager/style-inserter.php	Protects inline style wrappers.
modules/form-record/rest-endpoints/mark-view-record-base-endpoint.php	Sanitizes record-update error responses.
modules/form-record/admin/pages/export-page.php	Escapes form-record export errors.
modules/components/assets/build/index.asset.php	Updates component asset cache version.
modules/captcha/module.php	Casts inline-script form IDs to integers.
modules/blocks-v2/text-field/assets/build/editor.asset.php	Updates text-field editor cache version.
modules/blocks-v2/repeater-field/block-type.php	Inserts metadata into formulas as data.
modules/blocks-v2/phone-field/block-template.php	Escapes phone-field attributes.
modules/block-parsers/fields/hidden-field-parser.php	Tracks hidden-value provenance for email escaping.
modules/ai/rest-api/endpoints/generate-form-endpoint.php	Sanitizes AI-service error responses.
modules/advanced-choices/block-renders/choice-control-render.php	Sanitizes choice-control label markup.
modules/admin/module.php	Escapes translated deactivation-modal content.
modules/active-campaign/rest-api/active-campaign/active-campaign-endpoint.php	Sanitizes ActiveCampaign error responses.
modules/actions-v2/send-email/send-email-hooks.php	Uses HTML-aware processing for HTML emails.
modules/actions-v2/send-email/assets/build/editor.asset.php	Updates email editor cache version.
modules/actions-v2/redirect-to-page/redirect-to-page-action.php	Validates resolved redirect schemes.
modules/actions-v2/redirect-to-page/assets/build/editor.asset.php	Updates redirect editor cache version.
modules/actions-v2/mailchimp/rest-api/mailchimp/mailchimp-endpoint.php	Sanitizes Mailchimp error responses.
modules/actions-v2/mailchimp/assets/build/editor.asset.php	Updates Mailchimp editor cache version.
modules/actions-v2/get-response/rest-api/get-response/get-response-endpoint.php	Sanitizes GetResponse error responses.
modules/actions-v2/get-response/assets/build/editor.asset.php	Updates GetResponse editor cache version.
modules/actions-v2/assets/build/editor.asset.php	Updates actions editor cache version.
includes/form-break.php	Escapes progress-wrapper attributes.
includes/db-queries/query-conditions-builder.php	Explains an existing diagnostic-output suppression.
includes/db-queries/execution-builder.php	Explains an existing diagnostic-output suppression.
includes/classes/tools.php	Adds CSS, formula, and macro protection helpers.
includes/classes/gallery.php	Uses URL escaping for gallery links and images.
includes/classes/base-attributes-trait.php	Documents existing attribute escaping.
includes/classes/arrayable/array-convert-once.php	Explains an existing error-output suppression.
includes/blocks/types/base.php	Uses HTML-aware processing for rich attributes.
includes/blocks/render/media-field-render.php	Neutralizes shortcodes in media previews.
includes/blocks/render/form-builder.php	Protects inline CSS and its form-ID attribute.
includes/blocks/render/calculated-field-render.php	Safely inserts metadata into calculated formulas.
includes/blocks/render/base.php	Adds generated-option label protection.
includes/blocks/module.php	Supplies allowed URL protocols to the frontend.
includes/admin/table-views/column-base.php	Sanitizes HTML in string-type table cells.
includes/admin/single-pages/base-page-updater.php	Sanitizes admin update error responses.
includes/admin/pages/single-pages-manager.php	Escapes standalone-page errors.
includes/admin/pages/action-pages-manager.php	Escapes action-page errors and access messages.
includes/addons/manager.php	Sanitizes remote descriptions and activation errors.
components/rest-api/rest-api-controller-base.php	Explains an existing diagnostic-output suppression.
components/admin/buttons/base-vui-button.php	Explains an existing error-output suppression.
compatibility/jet-popup/jet-popup.php	Protects popup inline CSS.
compatibility/jet-engine/jet-engine.php	Unslashes and cleans scalar relation inputs.
compatibility/jet-engine/blocks/map-field/assets/src/frontend/map.field/signal.js	Builds map previews with DOM text.
compatibility/jet-engine/blocks/map-field/assets/build/frontend/map.js	Includes the map-preview protection.
compatibility/jet-engine/blocks/map-field/assets/build/frontend/map.asset.php	Updates map frontend cache version.
compatibility/jet-engine/blocks/check-mark/assets/build/editor.asset.php	Updates check-mark editor cache version.
compatibility/jet-engine/assets/build/editor.asset.php	Updates JetEngine editor cache version.
compatibility/elementor/widgets/form.php	Documents existing renderer escaping.
compatibility/bricks/widgets/form.php	Documents existing wrapper and renderer escaping.
assets/src/package/validation/store/reducer.js	Escapes callback names in editor summaries.
assets/src/package/preset/helpers/humanReadablePreset.js	Escapes values in preset-summary HTML.
assets/src/package/preset/helpers/escapeText.js	Adds a shared editor text-escaping helper.
assets/src/package/block-conditions/store/reducer.js	Escapes condition fields and states.
assets/src/frontend/media.field/signal.js	Escapes apostrophes in preview placeholders.
assets/src/frontend/main/html.macro/observeMacroAttr.js	Guards macro attribute assignments and URLs.
assets/src/frontend/main/html.macro/macroUrl.js	Adds URL-scheme and forbidden-attribute checks.
assets/src/frontend/main/html.macro/CalculatedHtmlString.js	Sanitizes assembled HTML while preserving trusted fragments.
assets/src/frontend/main/functions.js	Documents safe use of the HTML-parsing API.
assets/src/frontend/calculated.field/functions.js	Replaces timestamp `eval()` with numeric conversion.
assets/src/admin-vuex-package/components/TableColumns/link-type/LinkType.vue	Blocks script-like schemes in admin links.
assets/build/frontend/media.field.js	Includes media-preview escaping changes.
assets/build/frontend/media.field.asset.php	Updates media-field cache version.
assets/build/frontend/main.asset.php	Updates main frontend cache version.
assets/build/frontend/calculated.field.js	Includes numeric timestamp conversion.
assets/build/frontend/calculated.field.asset.php	Updates calculated-field cache version.
assets/build/editor/package.asset.php	Updates editor package cache version.
assets/build/editor/form.builder.asset.php	Updates form-builder editor cache version.
assets/build/admin/vuex.package.asset.php	Updates admin Vue package cache version.
assets/build/admin/package.asset.php	Updates admin package cache version.
.gitignore	Allows tracking the security tests and URL corpus.

_{Review effort: Balanced}

+	public static function neutralize_shortcodes( string $value ): string {
+		return str_replace( array( '[', ']' ), array( '&#x5B;', '&#x5D;' ), $value );


…t/Update Post Crocoblock/issues-tracker#20750

github-actions · 2026-11-05T13:52:15Z

🤖 AI PR Review

Risk level: medium

Review

Summary

This PR applies an XSS audit/fix across frontend/editor/admin JS and a few PHP compatibility renderers. Primary improvements: removal/avoidance of eval for calculated dates, stronger escaping for media preview templates, sanitization and safe handling of HTML macros (including href/src attribute checks), escaping of editor strings shown with innerHTML, and a number of built asset updates.

What I reviewed (high level)

JS source changes (assets/src):
- assets/src/frontend/calculated.field/functions.js: replace eval() with toMillis() numeric-only parsing.
- assets/src/frontend/main/functions.js: added toHTML() and doc comment about not passing unescaped user input.
- assets/src/frontend/main/html.macro/CalculatedHtmlString.js: stronger sanitization of calculated HTML; trusted SafeHtml parts are preserved via random markers then restored after sanitization.
- assets/src/frontend/main/html.macro/macroUrl.js (new): URL/macro attribute validation + helpers to detect forbidden attrs and allowed protocols.
- assets/src/frontend/main/html.macro/observeMacroAttr.js: skips forbidden attributes, integrates macroUrl checks and removes attribute if value not allowed.
- assets/src/frontend/media.field/signal.js: escapeAttr now escapes single quotes too.
- assets/src/package/preset/helpers/escapeText.js: new helper used to escape values that are inserted into editor HTML strings.
- assets/src/package/*: use escapeText across several editor reducers that produce HTML strings.
- Added/updated many built assets in assets/build/** (bundled output) to reflect source changes.
PHP compatibility files: small phpcs comments clarifying that output is escaped earlier (Elementor/Bricks compatibility files).

Good security improvements

eval() removal: The calculated field no longer evals the formula result for date conversion. Replaced with toMillis() which only accepts numeric input (with a strict regex) — removes a major vector for executing arbitrary code on visitor-provided data.
Macro sanitization and SafeHtml handling: CalculatedHtmlString now sanitizes all parts, preserves trusted markup (SafeHtml) with unpredictable markers during sanitization, and restores it. This reduces risk of attackers injecting markup via macros while still allowing intentionally-safe macro output.
Macro attribute URL checks: observeMacroAttr now refuses dangerous attribute names (on*, innerHTML, srcdoc) and validates scheme of computed href/src-like attributes against allowed protocols (window.JetFormBuilderSettings.allowedProtocols or a safe default). If a field value introduces a disallowed scheme, the attribute is removed instead of written.
Escaping in media previews: preview template values are escaped (single quotes included) so a crafted file name or URL cannot break a template and cause XSS.
Editor escaping: values used in innerHTML dangerouslySetInnerHTML are now escaped (escapeText) before being inserted.

Files/areas I inspected specifically

assets/src/frontend/calculated.field/functions.js (toMillis + use)
assets/src/frontend/main/html.macro/CalculatedHtmlString.js (sanitize+SafeHtml marker handling)
assets/src/frontend/main/html.macro/macroUrl.js (new; protocol checks, forbidden attrs)
assets/src/frontend/main/html.macro/observeMacroAttr.js (url attr removal logic)
assets/src/frontend/media.field/signal.js (escape single quote)
assets/src/package/preset/helpers/escapeText.js and usages (block conditions, validation reducer)
Built output under assets/build/** (matches source changes)

Issues, questions & recommendations

sanitizeHtml: configuration and allowlist
- The code relies on sanitizeHtml(...) but I did not see where sanitizeHtml is defined or what configuration/options are used. Please confirm sanitizeHtml is the trusted library (e.g. npm sanitize-html) and that it is configured with a restrictive allowlist appropriate for the plugin. Tests should verify that the sanitize options do not allow inline event handlers, script/style tags, javascript: URLs, data: urls (unless explicitly allowed), etc.
- In CalculatedHtmlString.calculateHtml you call sanitizeHtml twice (once per part and again on the whole joined string). That is okay since you need to sanitize both parts and the surrounding template, but please confirm this is intentional and comment the rationale in code for future reviewers.
Allowed protocol configuration
- macroUrl.js uses window.JetFormBuilderSettings.allowedProtocols or a default list (which matches wp_allowed_protocols). Ensure that the server-side code (plugin settings) can set allowedProtocols reliably for front-end consumers. If you rely only on client-side settings, an attacker could tamper with window object in compromised environments — however the main point is to protect regular visitors.
- Consider caching getAllowedProtocols() result (currently reads window each call). For pages with many macro attributes this function may be called repeatedly; it's cheap but caching would be trivial.
Removing attribute vs setting safe fallback
- observeMacroAttr will remove the attribute if the computed value uses a disallowed scheme. This is a conservative and secure behavior, but it can break legitimate use-cases where a nonstandard scheme is expected (e.g. a plugin using a custom scheme). Document this behavior in changelog and release notes. If needed, add a filter/hook (frontend-settings) to allow plugins to whitelist additional protocols.
toMillis() and date handling
- The toMillis() implementation only treats numeric strings as timestamps and rejects other input. This is correct and safe. However if the previous behavior accepted a variety of date-like strings, this change might alter behavior in subtle ways (e.g. form that relied on parsed "2020-01-01" being used). Confirm that CalculatedFormula outputs timestamps when appropriate, and add tests asserting expected behavior for common date inputs.
observeMacroAttr: return value for forbidden attrs
- The function returns null early for forbidden attrs. Ensure callers handle null gracefully (the caller code in the repo appears to accept that but add a short comment to clarify for maintainers).
escapeText usage consistency
- escapeText is applied to some editor strings; ensure all places using dangerouslySetInnerHTML or building HTML strings from untrusted settings now use escapeText. Search for other usages of dangerouslySetInnerHTML or manual HTML assembly and add escaping where necessary.
Tests
- There are several high-risk behavioral changes that should be covered by automated tests (unit/JS integration):
  - Calculated field date conversion: ensure numeric strings are accepted and others are not evaluated; verify no code execution possible.
  - Macro attribute URL handling: test that javascript: or data: URLs are rejected when not allowed and that author-provided scheme is preserved when allowed.
  - CalculatedHtmlString combined sanitization: make sure SafeHtml placeholders are restored correctly and no escape-break occurs.
  - Media preview escaping: file names/URLs containing quotes, angle brackets, or data URIs do not produce executable markup.
  - Editor UI: strings produced by humanReadablePreset and other reducers rendered via innerHTML are escaped
Performance/scalability
- The changes add sanitization steps and URL scheme checks for each macro attribute. On very large forms containing many macro attributes this could add CPU cost while rendering macros. This is necessary for security; consider micro-optimizations (caching allowed protocols, avoid repeated compilation of regexes) if profiling shows issues.
Built assets
- The PR includes updated built assets under assets/build/**. Ensure the build pipeline and source maps are correct and that no accidental leftover eval or insecure code remains in any other bundled module. I recommend a quick grep for "eval(" across JS sources and built files to verify no remaining eval usage.
Backward compatibility / change in behavior

The attribute removal for disallowed URL schemes and the tightened date parsing may change front-end behavior for some forms. Mark this in the release notes and provide guidance to integrators (how to whitelist protocols if necessary).

Minor style / code suggestions

In macroUrl.getAllowedProtocols you may cache the computed array in module scope for a single-page lifecycle to avoid repeated allocation.
Add brief inline comments to the toMillis() function to describe acceptable input formats.

Conclusion

This PR substantially reduces XSS vectors across several places and is a positive security hardening.
Before merging: add unit/integration tests for the critical guards (macro URL schemes, calculated date handling, media preview escaping, and sanitized macro HTML), confirm sanitizeHtml configuration, and document the attribute removal behavior for consumers.

Files I recommend adding tests for (at minimum)

assets/src/frontend/calculated.field/functions.js (toMillis and date formatting)
assets/src/frontend/main/html.macro/CalculatedHtmlString.js
assets/src/frontend/main/html.macro/macroUrl.js + observeMacroAttr behavior for href/src
assets/src/frontend/media.field/signal.js (preview HTML escaping)

If you'd like, I can draft Jest test cases for the above behaviors.

Suggested changelog entry

- FIX: Hardened frontend & editor XSS surface — removed eval in calculated fields, sanitized HTML macros and macro attributes (href/src), escaped media preview/template data and editor preset strings to prevent XSS.

github-actions · 2026-11-05T14:13:38Z

🤖 AI PR Review

Risk level: medium

Review

Summary

This PR contains many frontend/editor/admin asset changes focused on XSS hardening: replacing eval in calculated fields, escaping attribute values and admin UI strings, sanitizing calculated HTML, forbidding unsafe macro attributes (on* / innerHTML / srcdoc), and added URL-scheme validation for href/src-like macro attributes.

What I inspected (high level)

assets/src/frontend/calculated.field/functions.js — replaced eval() with a toMillis() parser and safer numeric checks (good).
assets/src/frontend/main/html.macro/CalculatedHtmlString.js — improved sanitization: preserves SafeHtml placeholders and sanitizes template + values with sanitizeHtml (good approach to avoid untrusted markup injection).
assets/src/frontend/main/html.macro/macroUrl.js (new) + observeMacroAttr.js — detects URL-like attributes, computes static template, blocks attributes when macro result introduces disallowed schemes, and forbids dangerous attributes (on*, innerHTML, srcdoc) (good mitigation for attribute-based XSS).
assets/src/frontend/media.field/signal.js — improved escapeAttr to include single-quote escaping (good for templates that use single quotes).
assets/src/package/preset/helpers/escapeText.js + uses across editor reducers — escapes admin/editor strings rendered via dangerouslySetInnerHTML (prevents admin UI XSS from malformed form settings).
Several compiled assets were regenerated (assets/build/**) — expected after source changes.
A few compatibility templates (Elementor, Bricks) updated PHPCS comments to explain why echo is used (acceptable if renderer output is escaped already).

Security review / issues found

Good improvements
- eval() removal in calculated fields (assets/src/frontend/calculated.field/functions.js) eliminates direct code execution of formula results. The new toMillis() accepts only numeric strings or numbers — correct and much safer.
- Blocking on* / innerHTML / srcdoc macro attrs and validating href/src schemes prevents common attribute-based XSS attacks where a user-supplied value becomes a javascript: URL or an attribute containing executable HTML.
- Escaping of single quotes in media preview attributes and escapeText in editor UI addresses multiple XSS vectors.
- CalculatedHtmlString change keeps trusted “SafeHtml” parts while sanitizing everything else — good defense-in-depth.
Things to verify / improve
- sanitizeHtml availability: CalculatedHtmlString uses sanitizeHtml(). Ensure sanitizeHtml is bundled and available in all contexts (frontend/editor). If it is already used elsewhere, OK; otherwise please confirm it is imported/included in the bundle. If a custom sanitizer is used, document it and verify configuration (allowed tags/attributes).
- observeMacroAttr removes the attribute entirely when an URL is disallowed. This is a safe behavior, but it is a functional change: links/images may be removed if a value has a disallowed scheme. Consider whether this should fall back to a safe placeholder (or keep the attribute but set it to empty) for better UX. Also document the behavior in changelog.
- toHTML() was added to assets/src/frontend/main/functions.js but in the patch snippet I don’t see an explicit export (or the place where JetFormBuilderFunctions.toHTML is assigned). If this function is meant to be a public API, ensure it is exported and documented. If it's not meant to be public, remove the comment claiming it is.
- replaceAll usage: several new helpers use String.prototype.replaceAll(). This is well supported in modern browsers, but not on very old ones (IE11). If you must support older environments, replace with a regex-based .replace or add a polyfill. Confirm supported browser baseline for JetFormBuilder.
- getAllowedProtocols() reads window.JetFormBuilderSettings.allowedProtocols. If sites rely on server-side whitelist, ensure that setting is present in all contexts; otherwise DEFAULT_PROTOCOLS is used (which is fine). Document or test behaviour when server list is empty.
- Performance: CalculatedHtmlString now does per-part sanitizeHtml() then sanitizes the whole string again. This is safer but slightly more CPU-heavy on large forms. Consider a benchmark for very large forms or many macros. Not a blocker.

Backward compatibility concerns

Attribute removal: observeMacroAttr will remove attributes (href/src) when the final result introduces a disallowed scheme. That can be visible to site admins who expected a macro-derived link to work even if it contains a custom scheme. This is a security-motivated breaking behavior; call it out in the changelog.
Editor UI: escaping human-readable preset strings will change how some previously unescaped HTML in admin UI appeared. This is correct and desired, but it changes presentation if users relied on HTML in those settings.

Multisite / integrations

The changes are primarily frontend and editor-side. Ensure that JetEngine / Elementor / Bricks integration code paths still receive the same sanitized HTML from the form renderer. The PR updated comments in compatibility templates to assert the renderer outputs escaped HTML — confirm via tests.

Tests

I did not find new tests in the PR. For security-sensitive changes like these, please add tests (unit/integration) that cover:
- convertMillisToDateString: numeric string, numeric, invalid string -> returns NaN/0; ensure no eval is used.
- CalculatedHtmlString: ensure SafeHtml parts preserved and surrounding/sourced content sanitized; test complex templates where macros are surrounded by tag boundaries.
- macroUrl: attribute combinations where the template has a scheme vs where a macro adds a scheme; ensure attribute is blocked only when appropriate; test allowed vs disallowed protocols.
- observeMacroAttr forbidden attrs: confirm on* and srcdoc are ignored/not observed.
- escapeText: ensure various inputs (quotes, HTML tags) are escaped before being used in dangerouslySetInnerHTML.
- Media preview escaping: verify that both single and double quotes are escaped in preview templates.

Other small notes

In assets/build compiled files many whitespace/CSS ordering changes exist — these are expected due to rebuild.
Confirm that the field renderer (server-side) still sanitizes/escapes values on rendering and on server-side macro expansion where applicable. Frontend sanitization is important but server-side is also required to avoid injection paths in contexts where JS could be circumvented.

Suggested action items before merge

Add/confirm unit tests for the macro URL logic, CalculatedHtmlString behavior and toMillis().
Ensure sanitizeHtml is present and configured in webpack bundles (and documented).
Confirm export or registration of toHTML() if intended to be part of JetFormBuilderFunctions public API.
Consider replacing replaceAll in critical code paths if older browser support is required or include a polyfill.
Document the attribute-removal behavior (links/imgs removed when macro result has disallowed scheme) in the changelog and user-facing docs so integrators are aware.

Overall assessment

This PR makes solid, necessary XSS mitigations across multiple front-end surfaces. The fixes address real attack vectors (eval usage, unsafe macro attributes, unescaped editor strings). The changes are generally correct and improve security. A few follow-ups (sanitizer availability, tests, and small compatibility docs) would make this safer to ship.

Suggested changelog entry

- FIX: XSS hardening — remove eval in calculated fields, sanitize calculated HTML and media previews, block unsafe macro attributes and disallowed URL schemes, and escape editor preset/validation strings

…ead of formulas Crocoblock/issues-tracker#20772

github-actions · 2026-11-05T16:50:34Z

🤖 AI PR Review

Risk level: medium

Review

Summary

This PR hardens a number of XSS vectors across frontend and admin JS: removes use of eval for calculated dates, sanitizes HTML produced by macros, adds URL-scheme checks for attributes like href/src, escapes admin/editor strings used with dangerouslySetInnerHTML, and strengthens attribute escaping in media previews. Built assets are updated accordingly.

What I checked

Security: replaced eval in assets/src/frontend/calculated.field/functions.js; added toMillis() to accept numeric input only.
Macro/HTML output: CalculatedHtmlString now sanitizes non-trusted parts and preserves trusted SafeHtml via placeholders (assets/src/frontend/main/html.macro/CalculatedHtmlString.js). Added URL policy checks (new assets/src/frontend/main/html.macro/macroUrl.js) and observeMacroAttr uses it.
Attribute escaping: media preview attribute escaping extended to single quotes (assets/src/frontend/media.field/signal.js). Admin/editor strings now escaped before producing innerHTML in editor reducers (assets/src/package/preset/helpers/escapeText.js and usages in reducers).
Dynamic value handling: added explicit handling for literal preset values (fromPreset/to_set_from_preset) to avoid treating visitor data as formulas (assets/src/frontend/dynamic.value/*).
Built assets: many assets/build/* files updated to reflect source changes.

Files of interest (high-level)

assets/src/frontend/calculated.field/functions.js — removed eval(), added toMillis() and safer date conversion.
assets/src/frontend/main/html.macro/CalculatedHtmlString.js — sanitize per-part and re-inject trusted SafeHtml placeholders safely.
assets/src/frontend/main/html.macro/macroUrl.js (new) & assets/src/frontend/main/html.macro/observeMacroAttr.js — add allowed-protocol checks for URL attributes and forbid dangerous attribute names (on*, innerHTML, srcdoc).
assets/src/frontend/media.field/signal.js — escape single quotes in preview URLs to prevent template injection.
assets/src/frontend/dynamic.value/ValueItem.js and MultipleValueItem.js — support for preset literal values and safer handling.
assets/src/package/preset/helpers/escapeText.js and usages in store reducers — escape strings shown with dangerouslySetInnerHTML in the editor.
assets/build/* — compiled bundles changed (ensure they match source changes and were produced by the build system).

Security review/comments

Good: Removing eval() from calculated date handling is an important security fix. The new toMillis() only accepts numbers or numeric strings.
Good: CalculatedHtmlString now sanitizes each piece and preserves trusted SafeHtml via placeholders; this reduces risk of combining user values with template HTML that could enable XSS.
Good: URL macro checking (isAllowedMacroUrl) prevents user-provided values from introducing dangerous schemes (javascript:). Using a configured allowed protocol list (window.JetFormBuilderSettings.allowedProtocols) with sensible defaults is appropriate.
Good: Escaping editor strings prevents the inspector/code view in the editor from rendering untrusted configuration as HTML.

Points to verify / suggestions

sanitizeHtml existence & configuration
- CalculatedHtmlString and observeMacroAttr call sanitizeHtml(). Ensure sanitizeHtml is reliably available in the contexts where these modules run and that its configuration (allowed tags/attributes) is appropriate. Document or centralize sanitizeHtml options to avoid accidental over/under-sanitization.
Allowed protocols source parity
- The client uses window.JetFormBuilderSettings.allowedProtocols or defaults. Confirm server-side validation or sanitization uses the same allowed scheme list to avoid mismatches where server accepts a protocol but the client strips the attribute (or vice versa).
Behavioral change: eval -> numeric-only
- Previously formulas returned values that were passed to eval(). If any existing forms relied on textual formulas that produced JavaScript expressions (unlikely but possible), they will now be treated as non-numeric and result in “no date” behavior (0). Please note this as a breaking behaviour for any user code relying on eval. Consider documenting in upgrade notes.
Macro URL handling
- When isAllowedMacroUrl() returns false the attribute is removed (node.removeAttribute(attrName)). This is a safe choice but can change visible behavior (link vanishes). Consider whether showing a placeholder or keeping a safe fallback is preferable. At minimum, ensure authors can configure allowed protocols via JetFormBuilderSettings so legitimate custom schemes are preserved.
Trusted SafeHtml placeholder approach
- The approach of replacing trusted chunks with markers then sanitizing the whole HTML and restoring trusted chunks is correct in principle. Verify there are no edge-cases where sanitizer would alter surrounding markup in a way that makes replaced placeholders produce a different DOM (e.g. sanitizer removing an unclosed tag that was intentionally closed by trusted HTML). The code already sanitizes then restores trusted HTML — still recommend unit tests for mixed trusted/untrusted scenarios.
New keys in dynamic value API
- dynamic.value now expects to_set_from_preset in observe() payloads for preset values. Code that calls observe() should be reviewed (and existing serialized config) to ensure they still pass compatible payloads; default behavior remains unchanged (fromPreset default false), so should be backward compatible.
Built assets
- Many assets/build/* files changed. Confirm the build was run with the repo’s build config and node/tooling; avoid accidental hand-edited bundles. CI should run a build and ensure assets match source.

Performance

Most changes are small transformations and checks (regex, replace). No heavy operations were introduced in hot paths; fine for large forms.
watch for any extra passes over big form HTML (e.g. sanitize over large templates). If forms can be very large, consider profiling sanitizeHtml cost.

Backward compatibility

The code keeps defaults so existing behavior should generally be preserved. Two notable behaviour changes that may affect users:
- Calculated date handling no longer evaluates JS strings — it only accepts numbers / numeric strings. This is intended for security but could change results if any form relied on eval.
- Macro URL attributes get removed when a visitor-provided value introduces a scheme outside the allowed list. Authors using custom protocols should be able to re-enable them via JetFormBuilderSettings.allowedProtocols.

Tests missing / recommended

Unit/integration tests for calculated field date handling:
- Numeric input variants (number, numeric string, scientific notation) should format correctly.
- Non-numeric or malicious strings should not be executed and should return 0 behavior.
Tests for HTML macro sanitization with mixed SafeHtml and untrusted values to ensure restoration works and no XSS bypass.
Tests for macro URL handling: values with javascript:, data:, and allowed schemes; verify attribute removal/kept.
Tests for admin/editor HTML escaping — ensure no regressions in UI rendering and that strings show correctly in editor.
Tests for dynamic preset path handling (to_set_from_preset) to ensure preset values are applied and conditions still evaluate.

Minor code suggestions

Add a small comment / link to server-side spot where allowed protocols list is defined so it’s clear how to sync client/server.
Consider logging a dev-only console.warn when an attribute is removed because of disallowed protocol, to help authors debug.

Summary recommendation

This is a clear security-focused PR that fixes multiple XSS vectors. Merge after the following are done:
1. Ensure sanitizeHtml is present and its options are correct in all contexts (editor and frontend).
2. Add/adjust unit tests for the areas mentioned above.
3. Verify CI build produced the assets/build/* files and they match the source builds.

Overall: good work — addresses important XSS issues and improves sanitization throughout the JS codebase.

Suggested changelog entry

- FIX: Prevent XSS in calculated fields, macro HTML and admin presets (removed unsafe eval, added HTML sanitization, URL scheme checks and escaping)

…rocoblock/issues-tracker#20753

github-actions · 2026-11-06T10:15:26Z

🤖 AI PR Review

Risk level: medium

Review

Summary

This PR implements an XSS audit / mitigation across frontend and admin code. Key changes:
- Removes use of eval() in calculated field code and replaces it with a safe numeric-only parser (assets/src/frontend/calculated.field/functions.js).
- Introduces sanitization for HTML macros and attribute macros, with a URL-scheme check for href/src-like attributes (assets/src/frontend/main/html.macro/* and new macroUrl.js).
- Ensures that macro attributes that would produce disallowed URLs are not applied (observeMacroAttr).
- Preserves trusted SafeHtml instances while sanitizing surrounding content (CalculatedHtmlString changes).
- Makes dynamic value handling safer (treats preset literal values separately from formula expressions) — prevents executing preset strings as formulas (assets/src/frontend/dynamic.value/* and ValueItem changes).
- Escapes editor strings shown via dangerouslySetInnerHTML (escapeText + usage in block-conditions and validation UI) to avoid admin XSS (assets/src/package/preset/helpers/escapeText.js, assets/src/package/* reducers).
- Tightens media preview escaping (single quote escaping and attribute sanitization) to avoid HTML injection in custom templates (assets/src/frontend/media.field/signal.js).
- Built/compiled assets updated accordingly (many files under assets/build/**).

Files / functions to pay attention to

assets/src/frontend/calculated.field/functions.js
- Replaces eval() with toMillis() which only accepts numeric timestamps; good fix. Ensure this covers all valid numeric input forms expected by existing sites.
assets/src/frontend/main/html.macro/CalculatedHtmlString.js
- Sanitizes macro output and preserves SafeHtml placeholders. This relies on sanitizeHtml being available and configured correctly — see notes below.
assets/src/frontend/main/html.macro/macroUrl.js (+ observeMacroAttr.js)
- New URL scheme checks and disallowed attribute handling. Uses a DEFAULT_PROTOCOLS fallback and window.JetFormBuilderSettings.allowedProtocols if present.
- observeMacroAttr now rejects "forbidden" attributes (on*, innerHTML, srcdoc, etc.) and removes attributes when the resulting URL is disallowed.
assets/src/frontend/dynamic.value/ValueItem.js and MultipleValueItem.js
- Introduces fromPreset flag and ensures preset values are treated as literal strings (not formulas). Good fix to prevent executing untrusted preset text.
assets/src/package/preset/helpers/escapeText.js and usages
- Escapes values shown in the editor (block-conditions & validation reducers). This prevents admin UI XSS vectors when showing imported/generated form settings.
assets/src/frontend/media.field/signal.js
- escapeAttr now also replaces single quotes ('). Template-based insertion uses sanitized placeholders. Ensure custom preview templates are still applied correctly.
assets/build/**
- Many compiled assets were regenerated; please ensure build was run in the same environment and that no unintended changes to bundler config occurred.

Security review / notes

Good: removal of eval() in calculated field is the highest-risk fix here — excellent.
Good: treating preset values as literal for dynamic values (fromPreset) prevents executing user-controlled strings.
Good: URL macro validation (macroUrl.js) prevents field values from inserting arbitrary schemes (script:, data:, etc.) into href/src attributes. The logic keeps an author-specified scheme in the attribute but disallows schemes introduced by field values.

Important items to verify (action items for author / release)

sanitizeHtml availability & configuration:
- CalculatedHtmlString now uses sanitizeHtml() extensively. Ensure sanitizeHtml is available in all contexts where this code runs (webpack build includes it or it's provided globally). Also confirm the sanitizeHtml configuration (allowed tags/attributes) is exactly what we want — too permissive config could undermine the changes; too strict may unexpectedly remove author markup. Add documentation or a unit test to assert sanitizeHtml behavior.
Allowed protocols list & filter:
- macroUrl uses window.JetFormBuilderSettings.allowedProtocols or a static DEFAULT_PROTOCOLS. Confirm the server-side and client-side lists are kept in sync and that site owners can extend allowed protocols if needed. Consider adding a PHP-side setting or docs pointing to how to configure allowed protocols.
Behavioural change when attribute is removed:
- observeMacroAttr removes the attribute when the resulting value is disallowed. This is secure, but may be a breaking behaviour for some advanced templates that expected the attribute to be present regardless. Recommend documenting this change in the changelog and release notes, and consider emitting a console warning when an attribute is blocked in dev mode to help debugging.
Tests / QA:
- Add unit tests for convertMillisToDateString / toMillis covering numbers, numeric strings, exponential notation, and non-numeric strings.
- Add unit tests for macroUrl.isAllowedMacroUrl() for same-scheme templates, relative URLs, allowed and disallowed schemes.
- Add integration/e2e tests for macros used in href/src attributes (ensure allowed schemes kept, disallowed ones removed), and for media preview templates with special chars and single quotes.
Backward compatibility:
- There is a potential backward-compatibility impact: previously some attributes might have been set even if they contained a nonstandard scheme introduced by macro output. Now the attribute will be removed. Document this change.
- Check internationalization: escapeText() returns plain strings; ensure it doesn't break places that expected HTML (this change specifically targets editor HTML where previously dangerous values were injected into innerHTML UI). The change is appropriate for security.

Performance / bundle notes

sanitizeHtml is called per-calculated macro output; for very large forms with many macros it could add CPU cost. However sanitization for HTML macros is necessary. If performance problems arise, consider throttling or caching sanitized results per formula parts.
The implementation uses a small placeholder mechanism to preserve trusted SafeHtml pieces; that looks efficient.

Other suggestions

Add a small dev-mode console log when observeMacroAttr blocks a URL attribute (only when window.JetFormBuilderSettings.debug or similar) so theme authors can find why attributes disappeared.
Add a comment or README note about the expectation that sanitizeHtml is configured and included by the build.
Consider adding a small feature flag or toggle (for advanced users) to change the behavior from attribute removal to setting a safe fallback URL — but removal is safest.

Build / release checklist

Run npm build / yarn build in the repo and verify assets/build files match the committed ones.
Run unit tests and integration tests covering the changed areas.
Verify admin editor strings still render correctly and that conditional UI (block conditions) shows expected values.

Conclusion

This PR substantially improves XSS posture across both frontend and admin areas by removing eval, sanitizing macro HTML, validating link schemes, and escaping admin strings. The fixes are appropriate and necessary.
Before merging: add/verify tests for the critical areas (toMillis, macro URL checks, HTML sanitization), confirm sanitizeHtml presence/config, and document the semantic change where attributes can be removed when disallowed.

Minor: I noticed modern APIs like String.prototype.replaceAll are used (fine for supported browsers). If you support older browsers this may require polyfills.

Suggested changelog entry

- FIX: Harden XSS vectors in frontend and admin – sanitize macro HTML, validate href/src macro URLs, avoid eval() in calculated fields, treat preset dynamic values as literals and escape editor strings

Copilot

🟡 Changes recommended

Formula scanning misinterprets quotes in regex literals and changes calculation results, while the shortcode matrix test can pass despite callbacks executing inside input attributes.

3 open issues

3 resolved since last review

4 files not reviewed

assets/build/admin/package.js: Generated file
assets/build/frontend/calculated.field.js: Generated file
assets/build/frontend/dynamic.value.js: Generated file
assets/build/frontend/media.field.js: Generated file

_{Review effort: Balanced}

 			return '';
 		}

+		$in_string = Tools::formula_macro_string_contexts( (string) $args['calc_formula'] );


+	 * @dataProvider fieldProvider
+	 */
+	public function testPresetValueStaysData( string $block, array $attrs, bool $use_preset = true ): void {
+		$html = $this->render( $block, $attrs, $use_preset );


github-actions · 2026-11-06T11:13:38Z

🤖 AI PR Review

Risk level: medium

Review

Summary

This PR addresses XSS attack vectors in frontend/editor code by removing dangerous eval usage, sanitizing HTML inserted via macros, validating/whitelisting URL schemes in attributes (href/src/etc.), escaping configuration values rendered into the editor, and tightening media preview escaping. Many compiled assets are updated accordingly.

What I checked / highlights

Removed eval() usage in convertMillisToDateString and replaced with a strict numeric parser (assets/src/frontend/calculated.field/functions.js + built assets). This prevents executing arbitrary code from formula results.
Introduced sanitize logic for HTML macros so only trusted SafeHtml is preserved and the rest is sanitized; uses placeholders for trusted parts (assets/src/frontend/main/html.macro/CalculatedHtmlString.js and observeMacroAttr.js).
Added macroUrl.js which validates attribute URLs (href/src/action/formaction/poster) against allowed protocols; forbids dangerous attributes (on*, innerHTML, srcdoc) being used as macro sinks (assets/src/frontend/main/html.macro/macroUrl.js + observeMacroAttr.js changes).
Escape single quotes in media preview attribute escaping to avoid injection when templates are single-quoted (assets/src/frontend/media.field/signal.js).
Dynamic value handling: supports literal preset values vs. formulas (assets/src/frontend/dynamic.value.*). This prevents treating preset data as formulas to be evaluated.
Editor-side escaping: added escapeText helper (assets/src/package/preset/helpers/escapeText.js) and used it where editor builds HTML with dangerouslySetInnerHTML (block-conditions reducer, validation reducer, humanReadablePreset). This prevents config values from injecting markup in the editor UI.
Many built assets (assets/build/**) are updated to match the above source changes.

Files called out (key changes)

assets/src/frontend/calculated.field/functions.js
- Removed eval() and added toMillis() which only accepts numeric input (string or number). Good security improvement.
assets/src/frontend/main/html.macro/CalculatedHtmlString.js
- Sanitizes non-trusted parts and preserves SafeHtml placeholders.
assets/src/frontend/main/html.macro/macroUrl.js (new)
- URL scheme checks, default protocol list, forbidden attrs list, helpers used by observeMacroAttr.
assets/src/frontend/main/html.macro/observeMacroAttr.js
- Uses macroUrl helpers; forbids macros in dangerous attributes; drops attribute when produced URL is not allowed.
assets/src/frontend/dynamic.value/* and assets/src/frontend/calculated.field/* built assets
- Adds support for literal preset values (fromPreset) and avoids executing preset data as formula.
assets/src/frontend/media.field/signal.js
- Escapes single quotes in addition to other entities.
assets/src/package/preset/helpers/escapeText.js + callers
- Escapes strings used in editor HTML (reduces editor-side XSS vectors).

Security aspects

Nonces/capabilities: changes are all frontend / rendering-related, not REST/admin endpoints; nonces are unchanged and out of scope here.
Entrypoints that previously executed or injected untrusted content no longer use eval and now sanitize/validate the data before setting innerHTML or attribute values. Good.

Backward compatibility / behavior changes to watch for

Calculated fields: previously convertMillisToDateString used eval on the formula result. Now only pure numeric timestamps (or numeric strings) are treated as dates. If any workflows relied on eval to parse non-numeric expressions into numbers, behavior will change. This is expected and desirable for security, but note as a functional change.
Macro URL handling: If a form author used a macro to produce a URL with a non-whitelisted scheme (or produced the scheme via the macro), observeMacroAttr will remove the attribute until it becomes valid. This is a breaking-but-secure change for unsafe schemes. Document this behavior for authors who may rely on non-standard protocols.
Dynamic values / presets: the code now distinguishes literal preset values from formulas. If any site was storing formulas in presets and expected them to be evaluated, that behavior will change. Again, this is safer but needs communication.
Editor UI: escapeText changes mean the editor will render previously-unescaped config values as escaped HTML. This may change how some form labels/presets appear in the editor (they will no longer be interpreted as HTML). This is correct for security but should be noted.

Potential issues / suggestions

sanitizeHtml dependency: The PR assumes a sanitizeHtml() function is available. Please confirm that sanitizeHtml is provided in runtime (it looks used elsewhere in the codebase). If it's a third-party lib, ensure it's loaded and its configuration is secure. Add a unit/integration test to assert sanitizeHtml exists and behaves as expected in this context.
Tests: These are security-sensitive changes. Please add unit tests (or integration tests) covering:
- Calculated field: ensure non-numeric formula results are not evaluated and dates are only produced from numeric timestamps.
- Macro URL: macros inside href/src produce attributes only for allowed schemes; attributes are removed for disallowed schemes; attributes starting with on* are ignored.
- CalculatedHtmlString: trusted SafeHtml pieces are preserved while everything else is sanitized.
- Editor escaping: ensure humanReadablePreset and reducers don't allow HTML injection into editor previews.
Logging / Warning: For the macro URL behavior, consider logging (console.warn) or developer-mode notice when an attribute is removed due to an invalid scheme — this will help authors debug if their macros stop producing hrefs.
Documentation: update developer docs to mention that macros should not output scripts and that allowedProtocols are enforceable via JetFormBuilderSettings.allowedProtocols.
Backward-compatibility migration note: If there are customers relying on previous (less safe) behaviors, include a changelog and potentially a developer note.
Performance: sanitizeHtml is called on concatenated strings; with very large forms it could add CPU cost. I think the tradeoff is acceptable but consider caching results if performance issues show up.
Edge cases: observeMacroAttr removes attribute if URL invalid — if template contains both static scheme and macro adds path, that is allowed (the implementation checks the scheme on the whole result but compares against the static template scheme and allows if they match). That's good; ensure all attribute templates are covered.

Missing tests / CI

No new tests included. For high-risk XSS fixes, please add tests and a short security checklist in the PR.

Conclusion

Overall the changes look well scoped and address multiple XSS attack surfaces. The main items to validate before merging are presence/configuration of sanitizeHtml, and to add tests for the new behavior. Also document the behavioral changes so integrators know about the stricter URL/HTML handling.

Minor / style notes

Follow-up: after tests are added, consider adding a simple e2e test that tries to inject a script via a macro attribute and asserts it doesn't execute.
Code quality: changes follow the repo patterns; built assets updated correctly.

Recommendation: Approve after adding (or pointing to) tests and confirming sanitizeHtml availability and behavior.

Suggested changelog entry

- FIX: Improve frontend XSS protections — sanitize HTML macros, validate/whitelist URL schemes for href/src-like attributes, escape editor preset/condition text, and remove eval() usage in calculated fields (frontend).

Gawuww added 2 commits November 2, 2026 15:39

FIX: Security hardening against XSS and shortcode injection from visi…

179280a

…tor data Crocoblock/issues-tracker#20753

FIX: Prevent XSS through HTML macro attributes in option labels and p…

3a5c389

…resets Crocoblock/issues-tracker#20764

MjHead requested a balanced review from Copilot November 4, 2026 14:28

Copilot started reviewing on behalf of MjHead November 4, 2026 14:28

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

FIX: Prevent term creation from tampered choice field values in Inser…

a4122ca

…t/Update Post Crocoblock/issues-tracker#20750

FIX: after review Crocoblock/issues-tracker#20753

64bcf53

FIX: Prevent XSS by treating dynamic preset values as plain data inst…

96c97ac

…ead of formulas Crocoblock/issues-tracker#20772

FIX: Harden form rendering and actions against untrusted visitor data C…

54d6cdc

…rocoblock/issues-tracker#20753

MjHead requested a balanced review from Copilot November 6, 2026 10:18

Copilot started reviewing on behalf of MjHead November 6, 2026 10:19

View session

Copilot AI reviewed Nov 6, 2026

View reviewed changes

FIX: after review Crocoblock/issues-tracker#20753

89250a9

Gawuww merged commit c789405 into release/3.6.8.1 Nov 6, 2026
1 check passed

Navigation Menu

Repository navigation

xss audit - #746

xss audit#746
Gawuww merged 7 commits into
release/3.6.8.1from
dev/xss-audit

Gawuww commented Nov 4, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 5, 2026

Uh oh!

github-actions Bot commented Nov 5, 2026

Uh oh!

github-actions Bot commented Nov 5, 2026

Uh oh!

github-actions Bot commented Nov 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Nov 6, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		public static function neutralize_shortcodes( string $value ): string {
		return str_replace( array( '[', ']' ), array( '[', ']' ), $value );

Conversation

Gawuww commented Nov 4, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

🤖 AI PR Review

Review

Suggested changelog entry

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 5, 2026

🤖 AI PR Review

Review

Suggested changelog entry

Uh oh!

github-actions Bot commented Nov 5, 2026

🤖 AI PR Review

Review

Suggested changelog entry

Uh oh!

github-actions Bot commented Nov 5, 2026

🤖 AI PR Review

Review

Suggested changelog entry

Uh oh!

github-actions Bot commented Nov 6, 2026

🤖 AI PR Review

Review

Suggested changelog entry

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

github-actions Bot commented Nov 6, 2026

🤖 AI PR Review

Review

Suggested changelog entry

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants