Security hardening + template safety (v1.4.2–v1.4.4)#1
Open
mt-alarcon wants to merge 9 commits into
Open
Conversation
…/rollback
Harden the write API against the main risks of letting an AI agent mutate a live
WordPress site:
- New Validator class:
- validate_tree(): structural + bounded depth (30) + size (5000) checks on any
element tree before it replaces _elementor_data. Rejects malformed nodes,
unknown elTypes, widgets without widgetType, non-array settings/children.
- resolve_media_path(): canonicalizes the import path and confirms it lives
inside wp_upload_dir() — closes a path-traversal / LFI vector in media import
(previously copy() ran on any server path). Rejects remote URLs and
non-image MIME types.
- is_valid_element_id(): validates element ids arriving in request BODIES
(URL routes were already constrained; bodies were not).
- Elementor_Data:
- save_page_data() now snapshots the prior _elementor_data to a backup meta
slot before overwriting; new restore_backup() reverts one level — a bad
write is now undoable.
- import_image() dedups by SHA-1 content hash (not title — different images
sharing a title no longer collapse) and uses wp_unique_filename() to avoid
clobbering existing uploads.
- REST_Controller: validate_tree on update_page/create_page/build_page/add_element,
id checks on patch-bulk, resolved media paths on import_media/build_page, plus
new POST /page/{id}/restore, GET /kit/globals, and a public GET /health probe.
Uniform error_response() carries HTTP status from WP_Error data.
- Bootstrap: load Validator; add str_starts_with() polyfill so the declared
"Requires PHP 7.4" floor is real (widget discovery used a PHP 8.0 function).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The MCP Abilities surface had drifted behind the REST surface (1.3.0 added bulk/ find/section to REST only). Bring them to parity and fix a latent bug: - Bug: the add-element ability inserted into $parent['element']['elements'] without ensuring the key existed — a parent container with no children array triggered an undefined-index error. The REST twin already guarded this; the ability now does too. - New abilities (parity with REST): find-elements, patch-elements-bulk, restore-page, get-kit-globals. - Reuse the new Validator across write abilities: validate_tree on save-page-data / create-page / build-page / add-element, valid id checks in patch-bulk, and resolve_media_path on build-page image import (same LFI guard as REST). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
41 assertions covering the plugin's pure-logic surface — tree validation (shape/depth/size), element-id checks, media-path traversal/LFI guard, and factory structure/id-reassignment. Runs on a bare PHP CLI (no composer, no PHPUnit) via `php tests/run.php`, exit 1 on any failure. WP stubs only cover the handful of functions the tested code reaches; runtime behaviour needing a live WordPress+Elementor stays out of scope and is flagged for on-install verification. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- README: "Fork of bvisible/elementor-mcp-api" section + highlights; new
endpoints (/page/{id}/restore, /kit/globals, /health); Security + Testing
sections; license attribution note; corrected abilities count.
- CHANGELOG.md: full 1.4.0 entry (security/added/fixed/needs-validation).
- IMPROVEMENTS.md: deep understanding, prioritized plan, what was implemented,
what needs a live WP to validate, deferred work.
- claude-skill/skill.md: __globals__ design-system-first guidance, restore-to-
undo note, new endpoints in the reference table.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MUST-FIX-1 from the independent security audit. SVG is XML that can carry <script>; media import does a raw copy() with no sanitization, so an imported SVG would execute on the site domain (stored XSS). WordPress blocks SVG uploads by default for exactly this reason. Remove image/svg+xml from the resolve_media_path() allowlist (now raster-only: jpg/png/gif/webp/avif) and update the rejection message. Do not re-enable without a dedicated safe-SVG sanitization library. Adds a test asserting an SVG staged in uploads is rejected as unsupported_media_type. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add Validator::check_body_size() — rejects a raw write-request body over ~4 MB (MAX_BODY_BYTES) with a 413 before it is JSON-decoded, so a runaway/malicious payload is never parsed into memory. Wired into the REST writes in a follow-up commit; this commit lands the pure-logic guard + 4 assertions (empty, 1KB, exact limit, over-limit). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…X-2) The write API was gated only on the blanket edit_posts, leaving an Author able to rewrite any page and to touch site-global config. Harden across BOTH the REST and MCP surfaces (closing the same REST↔MCP drift class as commit d90bb98): (a) Per-post REST checks — check_edit_permission()/check_read_permission() now do current_user_can('edit_post'/'read_post', $id) when the route carries an id; create routes (no id) require edit_pages. build_page enforces edit_post on the body-supplied page_id when updating an existing page. (b) MCP per-post checks INSIDE each execute_callback — an Ability's permission_callback never sees the input, so every write ability now calls guard_edit_post($post_id) (and reads call guard_read_post) after resolving the id, returning 403 otherwise. Without this, closing only REST left the MCP door open. (c) Site-global writes → manage_options — update_kit / create_template (REST routes moved to an $admin permission group; abilities call guard_admin()) and the register_post_meta auth_callback for _elementor_data. These are whole-site config, not per-page content. (d) Raw-HTML gate — the direct-meta fallback in save_page_data() (which bypasses Elementor's own save pipeline and can persist inline <script>) now requires unfiltered_html; lower roles get a no-op false return. The native Elementor path is unchanged. Also (SHOULD-3) list_pages / list-pages no longer leak drafts to callers who can't edit them; (SHOULD-4) build_page + import_media accept both `source_path` (canonical, matches the MCP ability) and legacy `path` so neither surface silently imports nothing; (SHOULD-6 wiring) reject_if_oversized() applied to all JSON-body writes; create_template now validates its element tree. Version → 1.4.1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- README Security section: Administrator-account requirement, tiered-permission model (per-post edit_post/read_post, manage_options for global config, MCP parity enforcement, unfiltered_html gate), SVG-rejection + payload-ceiling notes, one-level-undo snapshot semantics. - CHANGELOG 1.4.1 entry: MUST-FIX-1 (SVG), MUST-FIX-2 (capabilities), SHOULDs (draft leak, source_path consistency, payload ceiling, snapshot docs). - IMPROVEMENTS §3b (audit follow-up) + §4 expanded with the capability matrix and payload-ceiling checks that can only be validated on a live WordPress; flags SVG-XSS / capability / draft-leak as upstream-PR candidates. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
v1.4.2 — kses sanitization on all widget text fields; stored-XSS paths closed in create_element/update_element paths. v1.4.3 — add_section endpoint; ceiling DoS tightened (64 kB hard cap enforced at REST layer, not just MCP). v1.4.4 — create_template defaults to draft; publishing requires explicit status:"publish" or is rejected (REST 400 / Abilities WP_Error). Prevents theme-builder templates going live site-wide at creation time. delete_template added (trash default, --force for permanent). 4 new test suites (rest-security, abilities- security, kses-gate, template-safety); wp-stubs expanded. 115/115 assertions pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi! While hardening a vendored fork of this plugin for unsupervised, AI-driven writes to WordPress sites, we closed a set of security/robustness issues and are offering the fixes upstream. All issues require authenticated access (admin/editor) — none are anonymous/public — so this is safe to discuss openly.
What's in this PR
Stored XSS (was the main one)
unfiltered_htmlgate lived only in the direct-meta fallback ofsave_page_data(), which never runs when Elementor is active (production). Moved the gate to the top ofsave_page_data()so it covers all paths; added recursivewp_kses_postsanitization (kses_widget_html()/kses_settings()) of widget HTML settings when the caller lacksunfiltered_html. Admin callers get byte-identical content. Allowlist coverseditor/html/tab_content/alert_*/inner_textetc.DoS / payload ceiling
update_element,move_element,set_column_width,add_section) and aguard_input_size()across the MCP abilities (the MCP surface had no ceiling).Input validation
add_sectionnow runsValidator::validate_tree()+ size check before insert (was the only write skipping validation).Template safety
create_templatedefaulted topost_status: publishwith defaultinclude/generalconditions → aheader/footertemplate would go site-wide live at creation. Now defaults to draft; unknown status clamps to draft; invalid status → 400 without insert.DELETE /template/{id}(admin-only, trash by default,?force=truefor permanent + cleans the orphan entry inelementor_pro_theme_builder_conditions) — previously there was no template rollback.Tests
PHP test suite expanded 46 → 115 assertions (new suites: rest-security, abilities-security, kses-gate, template-safety). All green.
Happy to split this into smaller PRs or adjust framing if you prefer. Thanks for the plugin!