Security audit

[Gawuww]

issue - https://github.com/Crocoblock/issues-tracker/issues/20122

[github-actions]

🤖 AI PR Review

Risk level: medium

Review

Summary

This PR implements a security-focused audit / hardening across several modules: CSRF handling and rotation, captcha validation tightening (hCaptcha and reCAPTCHA v3), multi-gateway selection sanitization, plus a frontend token refresh in AJAX submit flow. It also adds unit tests covering these behaviors and updates build asset versions. Overall the changes improve security and add tests; a few things to verify/consider before merging are noted below.

What I reviewed (notable files / changes)

• modules/security/csrf/module.php

  • Now consumes CSRF tokens at verification time and rotates (issues replacement token) for AJAX continuations (handle_after_send receives handler and success flag). Added client_id property rename and small refactors.
  • Added remove_action invocation to prevent duplicate handlers.

• modules/security/csrf/csrf-tools.php

  • Added consume() wrapper which deletes and returns bool (helps ensure one-time tokens).

• assets/src/frontend/main/submit/AjaxSubmit.js

  • Reads response._jfb_csrf_token and updates input[name="_jfb_csrf_token"] for all forms with the same data-form-id (refreshCsrfToken). This enables AJAX continuations after a consumed token.

• modules/captcha/hcaptcha/*

  • hcaptcha.php: introduced get_captcha_args() that applies the existing filter and sanitizes sitekey (sanitize_text_field, scalar check). verify() now sets site_key on Verify_Token_Action.
  • verify-token-action.php: added site key checks (before_make_request throws if site key empty), set_site_key(), and action_body() that includes sitekey in provider request.

• modules/captcha/re-captcha-v3/*

  • Added sanitize_threshold() to validate stored threshold (fallback to default if invalid). Comparison in Verify_Token_Action changed from > to >= so score exactly equal to threshold is accepted.

• modules/multi-gateway/module.php

  • Tightened handling of submitted multi_gateway: sanitizes selected gateway, resolves real form id (handles revisions), computes allowed gateways for frontend (manual vs single mode), and only permits allowed gateway ids. Uses sanitize_key(). Also sets gateways_module->set_form_id($handler_form_id) and saves gateways data at runtime.

• Tests

  • Added unit tests for hCaptcha sitekey binding, reCAPTCHA threshold behavior, CSRF token lifecycle, multi-gateway submission rules.

Security assessment

• CSRF

  • Positive: Tokens are now consumed on verification (Csrf_Tools::consume), preventing replay. After an AJAX submission the module emits a fresh token in response data so the frontend can replace the used token and continue (AjaxSubmit refreshes inputs). This closes a replay/one-time-token gap.
  • Suggestion: Ensure Csrf_Token_Model::clear() (called in handle_request) is performant/doesn't cause large deletes on high-traffic sites — it's unchanged here but worth auditing separately for high-volume sites (add limits / TTL cleanups if necessary).

• Captchas

  • hCaptcha: Verification is bound to the configured/filtered sitekey and the code sanitizes the sitekey before use. Verify_Token_Action refuses to proceed if sitekey missing. This mitigates misuse where attacker-provided sitekey could be used to bypass checks.
  • reCAPTCHA v3: threshold sanitization avoids malformed config being used; comparison change (>=) is reasonable and documented by tests.

• Multi-gateway

  • User-submitted gateway id is now sanitized and validated against a computed allow-list for the form (manual vs single mode). This avoids arbitrary gateway ids being injected at runtime.

Correctness / compatibility notes

• reCAPTCHA change to >= (in verify) changes behaviour for scores exactly equal to the configured threshold (previously rejected). This is a behavior change but seems intentional and covered by tests. Consider if any callers relied on strict > semantics.

• modules/multi-gateway/module.php now calls Gateways_Module::save_gateways_form_data() after set_form_id() with the selected gateway (including potentially setting gateway => 'none'). That will persist runtime gateway selection into the gateways module state for that form_id (revision form id). The tests indicate this is intentional; double-check that saving here won't unintentionally persist into DB in undesired context. If save_gateways_form_data writes post meta, confirm this is safe for revisions and aligns with existing expectations.

• AjaxSubmit.js uses modern JS optional chaining (window?.JetFBPageConfigPackage?.nonce). Ensure the build toolchain/transpilation supports the target browsers for the plugin; this file is in assets/src and presumably built, but CI / build should transpile accordingly. The refreshCsrfToken DOM traversal is O(N) on number of forms — acceptable in practice but note in pages with many forms it loops all matching forms to update inputs for the matching form id.

• CSRF handle_after_send signature changed (now accepts $handler and bool $is_success). add_action was updated to register 2 args — good. remove_action is called inside handle_after_send to avoid duplicate runs; ensure priority matches in all cases (they use priority 10 for add/remove). remove_action is used with priority param which matches add_action usage.

Testing

• Good: New unit tests were added for the major security flows (CSRF lifecycle, captcha behaviors, multi-gateway). This is a strong plus.

Suggestions / minor issues to address before merge

1. Performance / DB load: review Csrf_Token_Model::clear() implementation to ensure it doesn’t cause heavy DB operations on busy sites; consider limiting clears or using expiration-based cleanup.
2. Confirm persistence behavior in multi-gateway: ensure save_gateways_form_data() invoked at runtime does not unintentionally write permanent changes to a parent form that should remain unchanged. If the intention is purely to set runtime gateway, ensure this function does not persist into DB, or document the behaviour. Tests expect set_form_id() + save to be safe, but a short comment clarifying intent (runtime vs persistent) would help future maintainers.
3. AjaxSubmit token update: consider adding unit/integration test that simulates an AJAX response carrying _jfb_csrf_token and asserts that input fields are updated. Current CSRF lifecycle tests exercise server-side rotation but not frontend JS. Tests for the JS behavior would further reduce regressions.
4. Ensure that any new exceptions or thrown Gateway_Exception or Spam_Exception are localized/handled by UI paths as expected and won’t leak internal messages to end-users. Current code throws Module::SPAM_EXCEPTION which seems to be handled elsewhere — just ensure admin UX is okay.
5. Consider documenting the change to reCAPTCHA comparison (>=) in changelog (it is included here) and any implications for sites that relied on strict > behavior.
6. Minor: ensure code style follows WPCS across modified PHP files (most looks fine).

Conclusion

This PR meaningfully improves security around CSRF and captcha usage, tightens gateway selection sanitization, and adds good unit test coverage. The changes are reasonable and the tests cover most flows, but please double-check the multi-gateway save behavior and consider adding a small frontend test for the AJAX CSRF refresh.

Suggested changelog entry

- FIX: Security: consume CSRF tokens on submission and rotate token for AJAX continuations; enforce hCaptcha sitekey binding and sanitize reCAPTCHA thresholds; restrict multi-gateway frontend selection to allowed gateways

[github-actions]

🤖 AI PR Review

Risk level: medium

Review

Summary
This PR contains multiple security hardening and correctness fixes across frontend JS and several backend modules (captcha, CSRF, multi-gateway, meta-box processing, validation). It also contains build asset version bumps and new unit tests (captcha). Overall changes increase security (CSRF consume/refresh, captcha sitekey binding, reCAPTCHA threshold sanitization), fix file upload limit comparisons, and prevent accidental clearing of JetEngine meta when mapped fields are absent.

What I checked

• Security: CSRF flow, captcha verification, sanitization of inputs, gateway selection sanitization.
• Backward compatibility: changes touching stored gateway/meta behaviour and responses.
• Performance: nothing expensive added; small JS changes and additional checks are fine.

Files / areas of interest (notes & recommendations)

• assets/src/frontend/main/submit/AjaxSubmit.js

  • Added handling of a CSRF token in the AJAX response and refreshCsrfToken(formId) which updates all matching form inputs named _jfb_csrf_token. Good improvement to rotate tokens on successful submissions.
  • Suggestion: add a short unit or integration test to assert refreshCsrfToken updates tokens on pages with multiple forms. Also consider guarding dataset.formId conversion (currently using unary +) with explicit parseInt for clarity and to avoid unexpected NaN handling on non-numeric dataset values.

• modules/security/csrf/module.php and modules/security/csrf/csrf-tools.php

  • Introduced consume() to atomically delete the token and return result; server now consumes token during handle_request and issues a new token in handle_after_send for AJAX handlers. Good improvement: prevents reuse of tokens.
  • handle_after_send now receives ($handler, bool $is_success) and only returns a new token for AJAX handlers. It removes the action on first call. Good.
  • Suggestion: ensure the new token is created only for successful submissions (current code issues new token regardless of handler->is_success state — but the code now checks handler->is_ajax() only; verify that only successful submissions will call this code path or consider checking $is_success or $handler->is_success before issuing a new token to avoid leaking tokens on failed requests).

• modules/captcha/hcaptcha/* and modules/captcha/re-captcha-v3/*

  • Hcaptcha: site key passed to Verify_Token_Action; get_captcha_args() centralizes and sanitizes sitekey. Verify action now throws if sitekey is empty (anti-spam measure). Good.
  • ReCaptcha: threshold sanitization added (sanitize_threshold) and verification changed to >= threshold. Good fix to avoid off-by-one where equals should pass.
  • Unit tests for both captcha modules added. Great.
  • Suggestion: double-check any filters that may return non-scalar sitekey values (get_captcha_args does sanitize to scalar). Good use of sanitize_text_field.

• modules/actions-v2/insert-post/traits/process-meta-boxes-trait.php

  • New logic filters $modifier->fields_map to only the current request field names (via get_current_fields_map) and then further filters meta-box fields by $this->value to avoid clearing mapped meta fields that are absent from the form. This prevents unwanted deletion of JetEngine meta when fields are omitted.
  • Security/perf: get_current_fields_map collects field names from modifier->get_request() and parsers list; good to avoid iterating huge maps when not necessary.
  • Suggestion: ensure keys used for array_intersect_key are sanitized (they come from form input names) — this is low-risk because you only intersect keys against the known $fields_map; still, consider casting keys to strings to avoid weird behaviour.

• modules/multi-gateway/module.php

  • Selection normalization: the submitted multi_gateway value is now sanitized and validated with get_allowed_gateway_for_submission(), which maps revisions to parent forms when reading stored gateways. Only allowed gateway values are accepted. The code also persists runtime selection by setting module form_id to the handler's form id and calling save_gateways_form_data().
  • Concerns: saving gateways_form_data() on submission has side effects (it writes to post meta). The comments explain the reason (Default_With_Gateway_Executor behaviour). This is acceptable but high-impact: we must ensure this does not unintentionally mutate form configuration on user submissions. Please add/investigate tests that assert expected postmeta changes only occur when intended. Also confirm appropriate capability checks are in place for any codepaths that write permanent data during a frontend submission (I assume save_gateways_form_data writes post meta — this must not be exploitable to overwrite config). If save_gateways_form_data writes to transient or temporary data, document that.

• modules/form-record/rest-endpoints/fetch-records-page-endpoint.php

  • Endpoint no longer returns 404 when no records found; instead returns list => [] and total => count. This is a more RESTful uniform response. Backward compatibility: callers expecting 404 should be updated; but returning 200 is safer for JS integrations.

• modules/post-type/meta/messages-meta.php

  • Moved filter application into get_filtered_messages() and messages() now refreshes stored messages before returning. Also defaults merged as array_merge(defaults, messages) to preserve defaults when no saved values exist. Good.
  • Potential bug (existing, not introduced here): get_by_key() loops messages() and breaks if the key is not present in the first message entry. That break looks suspicious and probably should be continue; please re-review this function — it's unrelated to this PR but worth addressing.

• assets/src/frontend/media.field.restrictions/* and FileSizeRestriction.js

  • File size comparison changed to <= (inclusive). JS likewise mirrors <=. This removes an off-by-one restriction. Good.

• assets/src/frontend/main/reporting/BrowserReporting.js

  • validateOnChange now uses this.getErrors() and sets validityState.current, and clears report when there are no errors. This makes validation state consistent.

• Tests

  • New unit tests for captcha modules are included. Good coverage for the new logic.
  • Missing tests: add unit/integration tests for CSRF token consumption/refresh (AjaxSubmit.refreshCsrfToken and module token lifecycle), and for multi-gateway saving behaviour (ensure form config isn't changed in unexpected ways without proper privileges).

Other small notes

• Many build asset files updated (hashed versions). No manual review needed for compiled/minified bundles.
• JS formatting changes (added newline at EOF in some modules) are harmless.

Required follow-ups / recommendations

1. Add tests for CSRF lifecycle: ensure token consumed on submit and refreshed only for successful AJAX submissions; ensure token is not regenerated on failed submissions. Consider asserting new token presence in response only when submission succeeded.
2. Add tests for multi-gateway persistence side effects: assert when and how save_gateways_form_data() modifies post meta and that frontend submissions cannot be used to overwrite gateway config (or require appropriate capabilities where applicable).
3. Review messages-meta::get_by_key() for the break that likely should be a continue.
4. Small JS robustness: in AjaxSubmit.refreshCsrfToken, consider parseInt for dataset value comparison and skip non-numeric dataset values explicitly (for clarity). Add a unit test for refreshCsrfToken.
5. Consider a short developer note documenting the reason save_gateways_form_data is invoked during submission (to avoid regressions in future changes) and any expected side-effects.

Overall: The changes improve security and correctness with reasonable care taken to sanitize inputs and add tests for captchas. The multi-gateway persistence change is the highest risk/confusing part because it writes configuration during a submit request — please add tests and an explicit comment documenting expectations and permission model.

Suggested changelog entry

- FIX: security & validation hardening — consume and refresh CSRF tokens on AJAX submit, bind hCaptcha verification to configured site key and sanitize it, sanitize and validate reCAPTCHA threshold (>=), fix file upload size/ext checks, avoid clearing JetEngine meta values when mapped fields are absent, and return empty list (200) for records endpoint when no records found.

[github-actions]

🤖 AI PR Review

Risk level: medium

Review

Summary

• This PR updates several built frontend/admin assets and makes targeted changes to the frontend source:
  • assets/src/frontend/main/submit/AjaxSubmit.js — adds handling to refresh/propagate a CSRF token returned from the server and adds refreshCsrfToken().
  • assets/src/frontend/main/reporting/BrowserReporting.js — change to validateOnChange to use getErrors() and clear report when no errors.
  • assets/src/frontend/media.field.restrictions/FileSizeRestriction.js — change file size validation from < to <= (fix off-by-one).
  • assets/src/frontend/media.field.restrictions.js — refactor/rewrites (mostly equivalent behaviour) and registers filters/restrictions.
  • Many built/minified asset files (assets/build/* and compatibility/*) were updated to reflect builds.

Security, capability checks and CSRF

• AjaxSubmit.js: Good to see the client refresh of _jfb_csrf_token when the server returns one. This improves security by rotating tokens after actions.
  • Recommendation: Ensure the server-side action that returns _jfb_csrf_token treats it as a secure token (signed/rotated) and that the server verifies it on subsequent requests. The client assumes the server returns a valid token — but server enforcement is required to actually improve security.
  • The client uses document.querySelectorAll('input[name="_jfb_csrf_token"]') and assigns field.value = csrfToken. That is OK, but ensure all forms actually include an input with this name. Consider supporting a configurable field name or updating all forms when token field name differs.
  • Consider validating token format before writing it into DOM (e.g. ensure it is a non-empty string / expected length) to avoid writing arbitrary content into hidden inputs from a compromised response.

XSS / DOM sanitization

• In media.field.restrictions.js validatePromise() the code sets element titles and displays markers using messages from restriction objects (i.title = r?.rejected?.length ? r?.rejected[0].getMessage() : ""). The message comes from internal restriction objects, but if any message can be influenced by remote data or untrusted input, it should be sanitized or escaped before insertion. Title attribute injection is lower risk but avoid it if messages could contain HTML that may be interpreted.

Behavioral / backward compatibility

• FileSizeRestriction: changed from < to <=. This is an intentional fix for the edge-case where file.size equal to limit should be allowed. This changes behaviour by one byte compared to previous releases. This is a bug fix but is a behavioural change — ensure changelog notes it.
• BrowserReporting: validateOnChange now uses getErrors() and clearReport() when there are no errors. This should not break existing flows, but please test with multi-step and conditional logic (watching mechanisms) to confirm no regressions in clearing error display.

Performance & large sites

• AjaxSubmit.refreshCsrfToken iterates over all forms with data-form-id and then all input fields named _jfb_csrf_token. On pages with many forms this is linear and acceptable in most cases, but if sites render thousands of forms this could be a concern. Probably fine; no heavy DOM queries beyond querySelectorAll.

Multisite / integrations

• Many compatibility files changed (bricks, jet-appointment, jet-booking, jet-engine, etc.). They appear to be code-style updates and small logic tweaks (mostly arrow function wrapping). Please run integration tests for: Bricks popup-initialisation, Jet Appointment, Booking, JetEngine blocks & listing options to ensure no regressions.

Other remarks

• A number of built asset files changed (assets/build/...); ensure the committed builds are the result of running the repo's canonical build process and that source changes are the intended ones. If this PR includes only built files but not source changes (except a few src files), verify that build artifacts are consistent with current build config.
• Optional chaining is used across modified files. Confirm supported browsers/targets or that the build/transpile pipeline produces compatible output for supported WP target browsers.

Tests missing / recommended

• Add an automated test (unit or integration) that verifies AjaxSubmit.refreshCsrfToken updates all matching hidden fields when server returns a new token. Tests for multi-form pages and for forms inside modals/popups (Bricks compatibility) would be useful.
• Add a test for FileSizeRestriction to ensure file with size equal to limit passes validation.
• Add an integration test for BrowserReporting.validateOnChange to ensure errors are cleared and UI updated appropriately.

Files of interest (high level)

• Modified source files to review closely:
  • assets/src/frontend/main/submit/AjaxSubmit.js
  • assets/src/frontend/main/reporting/BrowserReporting.js
  • assets/src/frontend/media.field.restrictions/FileSizeRestriction.js
  • assets/src/frontend/media.field.restrictions.js
• Many generated/minified files updated under assets/build/ and compatibility/* — review that they correspond to the source changes and do not introduce accidental changes in strings/translations.

Conclusion

• Overall the changes are sensible: they address CSRF token propagation, clear validation state more reliably, and fix a file size boundary bug. The security posture is improved but depends on server-side enforcement of CSRF as well. I recommend the small additions below before merging.

Required/Recommended follow-ups

1. Add server-side verification and rotation for _jfb_csrf_token (if not already present) and document the contract: which endpoint returns it and how long it is valid.
2. Sanitize token before writing to DOM (basic validation: typeof string && length > 0) to reduce the chance of unexpected values.
3. Sanitize/escape messages when assigning into title attributes (or use textContent on a sanitized node) if messages can contain untrusted content.
4. Add the tests mentioned above (csrf refresh, file-size equality case, reporting clear).
5. Run integration tests for compatibility modules: Bricks popups, Jet-Appointment, Jet-Booking, Jet-Engine blocks.

If you want I can propose a small patch to validate the CSRF token before applying it and to escape messages placed into title attributes.

Suggested changelog entry

- FIX: Refresh front-end CSRF token from AJAX responses and fix file size restriction edge-case (<=) in media field validation

[github-actions]

🤖 AI PR Review

Risk level: medium

Review

Summary

• This PR updates many built assets and a few source JS modules related to frontend submission, reporting and file size restrictions. Key source changes are in:
  • assets/src/frontend/main/submit/AjaxSubmit.js
  • assets/src/frontend/main/reporting/BrowserReporting.js
  • assets/src/frontend/media.field.restrictions/FileSizeRestriction.js
  • assets/src/frontend/media.field.restrictions.js (built)

What I looked for

• Security (CSRF/nonces/capabilities, trusting server input)
• Correctness / obvious bugs
• Backward compatibility and potential regression areas
• Performance concerns

Findings & recommendations

1. AjaxSubmit: CSRF token refresh (assets/src/frontend/main/submit/AjaxSubmit.js)

• What changed: After an AJAX response, code reads response._jfb_csrf_token and, if present, calls refreshCsrfToken which finds forms with data-form-id equal to the current form and sets input[name="_jfb_csrf_token"] value to the new token.
• Positive: Rotating/updating CSRF token client-side after server response is reasonable and can improve security posture when tokens are single-use or rotated.
• Concerns / suggestions:
  • Type checking / sanitization: response._jfb_csrf_token comes from the server, but treat it defensively before writing into the DOM. Add an explicit check (e.g. typeof csrfToken === 'string') and perhaps limit token length to a sane max before assigning to input.value.
  • Form id type consistency: refreshCsrfToken compares +formNode.dataset.formId !== formId. Ensure this.form.getFormId() returns Number (or coerce both sides consistently). Prefer explicit conversions for readability: if (String(formNode.dataset.formId) !== String(formId)) continue;
  • Query selector: document.querySelectorAll('form.jet-form-builder[data-form-id]') is fine; consider scoping to document (ok) and avoid repeated queries if this runs frequently. The operation is inexpensive unless there are many forms.
  • Tests: add an integration/unit test that simulates a response containing _jfb_csrf_token and verifies inputs are updated for matching forms only.

2. BrowserReporting.validateOnChange (assets/src/frontend/main/reporting/BrowserReporting.js)

• What changed: Instead of simply calling validate().then(...), the code now calls this.getErrors().then(errors => {...}) and uses results to update this.validityState.current and clear the report if there are no errors.
• Concerns / suggestions:
  • Ensure getErrors() always exists and returns an array of errors. If some environments only have validate(), this could break. If getErrors is the canonical API, this is fine.
  • Defensive .catch(() => {}) remains in place. Consider logging or central handling for unexpected promise rejections during development.

3. File size restriction change (assets/src/frontend/media.field.restrictions/FileSizeRestriction.js)

• What changed: validate previously used < max_size, now uses <= max_size. This is a correct fix (files equal to max_size should be allowed) and is a low-risk bugfix.
• Suggestion: Add unit tests covering boundary values (size === max, size === max+1).

4. media.field.restrictions (built) and other built compatibility JS

• The PR includes many changes to compiled/asset files (versions or minified sources). That is expected when rebuilding bundles after source change. Reviewers should verify that the committed built files correspond to source changes and that no unrelated logic changed.
• Suggestion: Confirm the build was produced with the repository’s canonical build toolchain and that no tool/config drift created unexpected syntax (optional chaining is used widely — ensure browser support/targets intended by your build process).

5. General security notes

• No new server-side PHP changes were introduced in this PR, so capability checks and nonce verification on the server are unchanged.
• The client-side update of CSRF token is fine, but remember server-side must accept the refreshed token scheme and the token rotation must be secure. Consider a short comment documenting the server-side behavior expected (when server returns _jfb_csrf_token and when). This helps future maintainers.

6. Backward compatibility and integrations

• The change to file size equality (<=) is backward compatible and arguably a bugfix.
• BrowserReporting change is internally focused; verify other code relying on previous validate() behaviour (e.g. code expecting validate() promise resolution) is still compatible.
• The CSRF refresh should be backwards compatible but ensure third-party integrations embedding forms and manipulating data-form-id still function; the code relies on data-form-id presence on forms.

7. Tests & QA

• Missing tests: add tests for
  • CSRF token refresh flow (client side) — ensures only matching forms updated and token sanitized.
  • File size boundary conditions.
  • BrowserReporting.getErrors integration (ensure validityState behaves as expected).
• Manual QA: test multi-form pages, forms in popups, and compatibility with Elementor/Bricks and the compatibility modules included in the build.

Files of interest (callouts)

• src: assets/src/frontend/main/submit/AjaxSubmit.js — add type-checking and consistent coercion for formId
• src: assets/src/frontend/main/reporting/BrowserReporting.js — ensure getErrors() exists and is stable
• src: assets/src/frontend/media.field.restrictions/FileSizeRestriction.js — good bugfix, add tests
• many built assets (assets/build/** and compatibility/**) — ensure they match intended source changes and were produced by the CI build process

Overall assessment

• Changes are small and targeted. The CSRF handling introduces new behavior that improves token rotation but should be made slightly more defensive (type-check token, coerce IDs consistently). The file-size <= change is a correct bugfix. Built asset diffs should be verified to be the product of a trusted build. Add tests for CSRF refresh and file-size boundaries.

Suggested code additions (examples)

• In AjaxSubmit.refreshCsrfToken:
  const csrfToken = response?._jfb_csrf_token;
  if ( typeof csrfToken !== 'string' || csrfToken.length > 4096 ) return; // defensive
  // coerce IDs consistently
  const formId = String(this.form.getFormId());
  ...

If you want I can propose a small patch to add the token type/length checks and to normalize formId comparisons.

Suggested changelog entry

- FIX: refresh CSRF token in frontend after AJAX form submission and allow files equal to max size for media field restrictions (frontend submission/media field)

[github-actions]

🤖 AI PR Review

Risk level: medium

Review

Summary

• This PR appears to be a security/validation audit and contains multiple frontend fixes and rebuilt assets. Key functional changes in source files:
  • assets/src/frontend/main/submit/AjaxSubmit.js: adds reading response._jfb_csrf_token and a refreshCsrfToken(...) method that updates _jfb_csrf_token input values for the matching form id. (Security improvement: client-side CSRF token rotation.)
  • assets/src/frontend/media.field.restrictions/FileSizeRestriction.js: file size comparison changed from < to <= (fix off-by-one rejection of allowed-size files).
  • assets/src/frontend/media.field.restrictions.js: reworked/cleaned-up restriction code and ordering; still registers restrictions and filters.
  • assets/src/frontend/main/reporting/BrowserReporting.js: validateOnChange now uses getErrors() and clears the report when there are no errors.
  • Many built/compiled asset files (assets/build/** and compatibility/**) were regenerated (versions changed).

Security, correctness and compatibility review

1. AjaxSubmit CSRF token refresh (assets/src/frontend/main/submit/AjaxSubmit.js)

   • Positive: rotating/refreshing client-side CSRF token when server provides a new _jfb_csrf_token improves security for long-running pages and multiple submissions.
   • Concerns / recommendations:
     • Server-side validation must still be authoritative. Client-side token replacement is only UX; ensure the server emits the rotated token only after verifying the submission and that server-side code validates incoming tokens on every submit. Add or reference server-side test coverage for nonce/token rotation endpoints.
     • The refreshCsrfToken() method searches all forms with selector 'form.jet-form-builder[data-form-id]' and compares +formNode.dataset.formId !== formId. This uses numeric coercion (+). If formNode.dataset.formId is missing or non-numeric, +formNode.dataset.formId becomes NaN and the check will be true (skips). That is fine as it avoids accidentally updating unrelated forms, but please confirm all JetFB forms consistently include a numeric data-form-id attribute. If there may be non-numeric ids, consider a strict string compare to avoid edge cases (e.g. formId could be string in some contexts). Example defensive change: if (String(formNode.dataset.formId) !== String(formId)) continue;
     • The code trusts response._jfb_csrf_token from the server and writes that value into input.value. That's acceptable (server-provided token), but ensure value is treated strictly as token string (not used elsewhere as HTML). Using value property is safe; avoid innerHTML/text insertion.
     • Performance: on pages with many forms this loops all forms on each response; acceptable but note cost if thousands of forms exist (edge case). This is low impact in typical usage.

2. File size / file restriction changes (assets/src/frontend/media.field.restrictions/FileSizeRestriction.js and media.field.restrictions.js)

   • Change from < to <= in FileSizeRestriction fixes an off-by-one bug and is appropriate — please confirm server-side file-size enforcement matches this <= behavior. Client-side checks are convenience only and must not be relied on for security. Add tests for file sizes exactly equal to limit.
   • The validatePromise logic manipulates DOM elements for invalid markers and titles (i.title = ...). This only sets title attribute and style.display which is low risk; ensure the message strings used are sanitized or plain strings. They come from restriction objects (getMessage()) — confirm those are safe strings (should be defaults or translations), not user content.
   • The registration of restrictions via addFilter remains unchanged in intent; ensure any new restriction classes are backward-compatible with existing plugin filters.

3. BrowserReporting.validateOnChange (assets/src/frontend/main/reporting/BrowserReporting.js)

   • The new behavior awaits getErrors(), sets validityState.current, and clears report when there are no errors. This avoids redundant validate().then(() => {}).catch(() => {}) calls and makes state explicit. Check for side effects: previously validate() might have produced different internal state; ensure getErrors() returns the same error list and that clearReport() is always safe to call here.

4. Built assets and compatibility files

   • Many autogenerated (minified/compiled) files were changed — this looks like a normal rebuild after source edits. Please ensure no unintended source changes were introduced into built files (they should reflect source changes only). CI should verify that built files match build from sources.

Missing tests and QA

• There are no new unit/integration tests in this PR. For these security-sensitive changes please add tests or QA steps for:
  • CSRF token rotation: server returns _jfb_csrf_token, client updates fields; test multiple forms on page, forms in popups, and forms without data-form-id.
  • File upload validations: test file.size exactly equal to limit, file count equals max_files limit, file extension checks, and multiple-file uploads.
  • Ensure server-side validation remains in place for file size, MIME type, file count and action capability checks.
  • Test multi-step forms and popups (Bricks/compatibility), since compatibility compiled JS changed.

Other suggestions

• Consider using String comparison when matching dataset.formId to avoid coercion surprises across various contexts.
• Add a short comment in AjaxSubmit.refreshCsrfToken to explain why client-side token refresh is safe and remind future reviewers that server-side validation is authoritative.
• Ensure messages used as title attributes are safe (they appear to be translation strings/objects but double-check they cannot contain raw HTML).

Conclusion

• Overall changes are small and appropriate: a security UX improvement (CSRF refresh), a bugfix (<= file size), and some frontend reporting behavior adjustment. The biggest risks are missing server-side validation or unexpected assumptions about data-form-id typing. Please add tests as suggested and consider the small defensive coding improvements above.

Suggested changelog entry

- IMPROVE: Refresh CSRF token after AJAX submit and fix media file validation (file size and count) on the frontend; improve browser reporting validate-on-change behavior.

[Gawuww]

Merged to release/3.6.5