Skip to content

Migrate Store v1.2.0 — WordPress 7 Readiness, Security Hardening & Shipping Method Coverage#8

Open
MahamadSayed wants to merge 18 commits into
masterfrom
migratestore-wp7-readiness
Open

Migrate Store v1.2.0 — WordPress 7 Readiness, Security Hardening & Shipping Method Coverage#8
MahamadSayed wants to merge 18 commits into
masterfrom
migratestore-wp7-readiness

Conversation

@MahamadSayed

Copy link
Copy Markdown
Collaborator

Description

Summary

Prepares Migrate Store for WordPress 7.0 and bumps the plugin to v1.2.0.
The branch lands three phases of work — compatibility/headers, security
hardening & data integrity, and full shipping-method export coverage.

What changed

Phase 1 — Plugin headers & PHP compatibility

  • Bumped version 1.1.91.2.0 (plugin header + MIGRATESTORE_VERSION).
  • Added Requires at least: 6.0, Tested up to: 7.0, Requires PHP: 7.4,
    and Requires Plugins: woocommerce headers.
  • Updated readme.txt (Tested up to, Stable tag, changelog).
  • PHP 8.2 / 8.3 compatible; minimum PHP 7.4.

Phase 2 — Security hardening & data integrity

  • Added manage_woocommerce capability checks to all export and import handlers.
  • Validated ZIP uploads (MIME type + size limit) with guaranteed temp-file
    cleanup on both success and failure.
  • Unified option field names (option_name / option_value) across all
    exporters and importers.
  • Fixed the AbstractImporter constructor signature.
  • Removed a duplicate entry in the email settings exporter.

Phase 3 — Shipping method coverage

  • Shipping method export is no longer limited to the three built-in types;
    all registered shipping methods are now exported.
  • Unrecognized methods are reported back to the user on import.
  • Added shipping method filtering on the admin import page.

Files touched (plugin)

migratestore.php, readme.txt, includes/MigrateStore.php,
includes/Exporters/**, includes/Importers/**, includes/admin/admin-import-page.php
(+255 / −42 across 9 files).

Testing

  • Manual export/import round-trip verified across all WooCommerce setting groups.
  • Capability checks and ZIP validation exercised; temp files confirmed cleaned up.
  • Shipping export verified against built-in and custom registered methods.

MahamadSayed and others added 13 commits June 10, 2026 22:54
Ratify Migrate Store constitution v1.0.0 for the WordPress 7.0 readiness
effort, plus Spec Kit scaffolding (.specify/, .claude/, CLAUDE.md).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 1 spec: Plugin Headers & PHP Compatibility (US-1, US-2, US-3) at
specs/001-plugin-headers-php-compat. Includes requirements quality checklist.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 1 plan + design artifacts (research, data-model, plugin-metadata
contract, quickstart) for Plugin Headers & PHP Compatibility. Grounded in a
static audit: source is clean of PHP 8.x deprecations; only header/version
edits required. Updated CLAUDE.md SPECKIT markers to reference the plan.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 1 tasks.md: 18 LLM-executable tasks (Setup, audit, US1 headers/version,
US2 PHP 8.x hygiene, US3 PHP 7.4 floor, lint+QA gate) with exact before/after
edits, file paths, and verification commands for an external implementer.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…grity

Add spec, requirements quality checklist, and point feature.json at the
new feature directory for Phase 2 (authorization, archive validation,
field-naming integrity, importer routing, constructor consistency,
debug-artifact removal).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rity

Add plan.md, research.md (code-grounded audit), data-model.md, the
handler-security and option-payload contracts, quickstart.md (lint + QA
matrix), and tasks.md (27 Kimi-ready tasks). Point CLAUDE.md agent context
at the Phase 2 plan.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…se QA

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…se QA

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Update plugin header: Version 1.2.0, Tested up to 7.0, Requires PHP 7.4, Requires Plugins: woocommerce
- Update MIGRATESTORE_VERSION constant to 1.2.0
- Update readme.txt: Tested up to 7.0, Stable tag 1.2.0, add 1.2.0 changelog
- Static audit: zero PHP 8.x deprecation patterns, zero PHP 8.0+ functions, zero dynamic properties
- No polyfill needed (no PHP 8.0+ functions in use)
- Behavior preservation: no export/import logic modified
- Mark all tasks as completed in tasks.md

Refs: SC-001..SC-006, FR-001..FR-017, US-1/US-2/US-3
User Story 1 (Authorization):
- Add current_user_can('manage_woocommerce') check to both export and import
  handlers before any data is read or written
- Reject unauthorized users with translated wp_die(403)

User Story 2 (Safe Upload Handling):
- Add filterable max upload size (migratestore_max_upload_size, default 10 MB)
- Add MIME validation via wp_check_filetype_and_ext() for .zip files
- Add ZipArchive traversal scan rejecting .. and absolute paths before extraction
- Add centralized cleanup_import_artifacts() helper
- Wire cleanup into every exit path: success, exception, and all early bails
- Remove moved upload file before ->cleanup() on success/catch

User Story 3 (Data Integrity):
- Standardize option entries on option_name/option_value in AbstractExporter
- Add legacy option/value fallback in AbstractImporter and ShippingZonesImporter
- Remove duplicate woocommerce_customer_completed_order_settings in
  EmailsOptionsExporter

User Story 4 (Exact Routing):
- Replace buggy strpos substring routing with array_key_exists exact match

User Story 5 & 6 (Constructor Consistency + Hygiene):
- Document constructor contract on AbstractImporter
- Resolve ShippingZonesImporter TODO with intentional-design comment
- Zero TODO/FIXME markers across all plugin source

Lint: php -l clean on PHP 8.3 for all 26 plugin files
Branch: migratestore-wp7-readiness
Apply shipping method filter support and WordPress 7 readiness updates to
the WooCommerce shipping zones exporter/importer, core plugin, and admin
import page.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Untrack the .specify/.claude tooling, specs/, and project docs (CLAUDE.md,
README.md, constitution.md, migratestore-plan.md) so the branch tracks only
the WordPress plugin source and build configuration. Files remain on disk.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@amElnagdy amElnagdy left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed with the guard skills + Plugin Check. The headline feature work is good — the "refuse to merge into existing zones" guard, the skip-unregistered-methods handling, and the zip-slip path check are all solid additions. A few things before merge:

🔴 Blocking — data corruption on import (see inline on ShippingZonesImporter.php:118): serialized shipping-method settings get mangled because sanitization runs before is_serialized(). Your new per-instance settings export round-trips exactly this kind of data.

🟠 Plugin Check — new code in this PR:

  • cleanup_import_artifacts() uses unlink() / rmdir() (MigrateStore.php:296,313,322,326) → wp_delete_file() / WP_Filesystem. It also re-implements the existing recursiveRemoveDirectory() — reuse that instead.
  • ShippingZonesExporter.php:83 — interpolated SQL not prepared → wrap in $wpdb->prepare().
  • MigrateStore.php:173$_POST / upload input not validated.

🟡 Non-blocking:

  • Capability mismatch: menu pages use manage_options, action handlers use manage_woocommerce (MigrateStore.php:34-49 vs 125/162) — pick one.
  • Changelog mentions a constructor-signature fix that isn't in this diff (readme.txt:73).

Separately, wp plugin check migratestore surfaces 23 pre-existing .org-blocking errors in the plugin's own code (legacy exporters/importers) unrelated to this PR — tracked in #12 so this PR stays focused.

$option_value = sanitize_text_field( $data['option_value'] );
// Canonical keys with legacy 'option'/'value' fallback for v1.1.9 archives.
$option_name = sanitize_key( $data['option_name'] ?? $data['option'] );
$option_value = sanitize_text_field( $data['option_value'] ?? $data['value'] );

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This corrupts serialized shipping-method settings on import. import_option() runs sanitize_text_field() on the raw value before is_serialized(). WooCommerce stores per-instance method settings as a serialized array; if any value contains a newline, double-space, or <, the serialized string's length prefixes get mangled → is_serialized() returns false → the settings array is silently stored corrupted.

The ordering is pre-existing, but this PR edits this line and adds the per-instance settings export that round-trips serialized data through here — so it's worth fixing here. The parent AbstractImporter::import_option() already does it correctly (unserialize raw → sanitize → enforce the option-name allowlist). Simplest fix: drop this override and rely on the parent; or mirror it:

protected function import_option( $data ) {
	$option_name  = sanitize_key( $data['option_name'] ?? $data['option'] );
	$option_value = $data['option_value'] ?? $data['value'];

	// Unserialize the RAW value first, THEN sanitize — sanitizing a serialized
	// string corrupts its length prefixes and silently destroys the settings.
	if ( is_serialized( $option_value ) ) {
		$option_value = maybe_unserialize( $option_value );
	}
	if ( is_array( $option_value ) ) {
		array_walk_recursive( $option_value, fn( &$v ) => $v = sanitize_text_field( $v ) );
	} else {
		$option_value = sanitize_text_field( $option_value );
	}

	update_option( $option_name, $option_value );
}

⚠️ This override also drops the parent's option-name allowlist guard — restoring it (or removing the override) closes that gap too.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amElnagdy All issues raised in review have been addressed. Each fix was manually tested before commit.

🔴 Blocking

  1. Serialized data corruption + dropped allowlist in ShippingZonesImporter::import_option() — c251d03
  • Sanitization ran before unserialization, corrupting serialized per-instance method settings (sanitize_text_field() altered byte lengths without updating PHP's serialization length prefixes → silent is_serialized() failure → mangled string stored).
  • Now maybe_unserialize() runs on the raw value first; sanitization only touches decoded leaves (recursive for arrays, is_string()-guarded for scalars).
  • Restored an option-name allowlist guard (^woocommerce_.+_\d+_settings$) so a crafted import can't write arbitrary options.

🟡 Plugin Check

  1. SQL injection flag in ShippingZonesExporter — 738f082
  • The dynamic IN () query (already bound via prepare(), but flagged by Plugin Check for the interpolated placeholder variable) was replaced with a get_option() loop over the known option-name list — removes the flagged pattern entirely (no phpcs:ignore) and clears the direct-query/no-caching warnings. Output is byte-identical (maybe_serialize() preserves the wire format the importer expects).
  1. Unvalidated upload input in handle_import_actio
  • Hardened the $_FILES['json_zip_file'] reads flagged under WordPress.Security.ValidatedSanitizedInput: int-cast the error code, isset-guard + cast the size, and add wp_unslash() before sanitize_file_name() on the name. No behavioral change — capability check, nonce, MIME allowlist, and zip-slip protection were already in place.
  1. Raw unlink()/rmdir() in import cleanup — 2f76d58
  • Replaced raw filesystem calls with WordPress APIs in both cleanup paths: wp_delete_file() for single files and
    $wp_filesystem->delete($dir, true) for the temp trbstractImporter::cleanup()); removed the now-unused custom recursiveRemoveDirectory() helper.

⚪ Non-blocking

  1. Capability mismatch + stale changelog — bdb506f
  • Menu pages used manage_options while handlers used manage_woocommerce, so Shop Managers could trigger actions but never see the menu. Aligned the four menu pages to manage_woocommerce (matching the handlers and the plugin's WooCommerce scope).
  • Removed the changelog line claiming the constructor-signature fix.

MahamadSayed and others added 5 commits June 22, 2026 14:36
…uard in ShippingZonesImporter

import_option() sanitized the option value before unserializing, which
corrupted serialized per-instance shipping method settings (sanitize_text_field
alters byte lengths without updating PHP's serialization length prefixes,
silently failing is_serialized() and storing a mangled string).

- Run maybe_unserialize() on the raw value first, then sanitize the decoded
  leaves (recursive for arrays, direct for strings; non-string scalars preserved).
- Restore an option-name allowlist guard: only woocommerce_{method_id}_{instance_id}_settings
  names are permitted; anything else throws RuntimeException, blocking writes to
  arbitrary options.
- Harden key access with null-coalescing fallbacks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Raw unlink()/rmdir() calls in the import cleanup paths silently fail on
hosts where PHP does not own the files (FTP/SSH-based filesystem access),
leaving stale artifacts behind. Plugin Check flags these too.

- MigrateStore.php: replace @Unlink() on the moved upload with wp_delete_file(),
  and rewrite cleanup_import_artifacts() to delete the temp extraction tree via
  $wp_filesystem->delete($dir, true) instead of a hand-rolled glob/recursion.
- AbstractImporter.php: rewrite cleanup() to use $wp_filesystem->delete($dir, true)
  and remove the now-unused custom recursiveRemoveDirectory() helper; escape the
  wp_die() path with esc_html().

Both rely on WP_Filesystem (loading wp-admin/includes/file.php where needed),
matching the pattern already used in get_json_data().

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tead of direct IN() query

Plugin Check flagged the dynamic IN () query as unprepared SQL because the
interpolated $placeholders variable can't be statically verified (the values
were already bound, so it was not actually injectable).

Replace the direct query with a get_option() loop over the known option-name
list. This removes the flagged pattern entirely (no phpcs:ignore needed) and
also clears the direct-query/no-caching warnings. Output is byte-identical:
get_option($name, null) preserves "existing rows only" semantics and
maybe_serialize() reproduces the raw DB wire format the importer expects.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…on()

Plugin Check flagged the upload-size read at line 173 under
WordPress.Security.ValidatedSanitizedInput (superglobal element accessed without
consistent existence check / unslash at the point of use).

- Cast the upload error code to int in the existence check.
- Read the upload size into an isset-guarded, int-cast local before comparing.
- Add wp_unslash() before sanitize_file_name() on the uploaded file name, and
  guard it with isset().

No behavioral change: the existing capability check, nonce verification, MIME
allowlist, and zip-slip protections already guarded this path; this only
hardens the raw $_FILES reads to satisfy the sniff.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…gelog entry

The admin menu/submenu pages required manage_options while the export/import
handlers required manage_woocommerce, so Shop Managers could trigger the action
but never see the menu. Align the four menu pages to manage_woocommerce to match
the handlers (the documented security boundary) and the plugin's WooCommerce focus.

Also remove the readme.txt changelog line claiming an AbstractImporter
constructor-signature fix; no such change exists in the release history.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants