Commit 0fe325b80fe for woocommerce
commit 0fe325b80febfa4be2873848f8a5f74693631885
Author: Mike Jolley <mike.jolley@me.com>
Date: Wed Jul 8 12:58:51 2026 +0100
Fix: Shop Manager role cannot reorder Local Pickup locations (#59894) (#65286)
* Fix: Shop Manager role cannot save Local Pickup settings (issue #59894)
Shop Managers were getting 403 on the Local Pickup settings page because
the admin JS POSTed to /wp/v2/settings, which requires manage_options.
Shop Managers only have manage_woocommerce.
- Add PickupLocationsRestController registering POST /wc/v3/pickup-locations
with wc_rest_check_manager_permissions('settings','edit') as the guard
- Update ShippingController to register the new route and extend the
track_local_pickup Tracks hook to also fire on the new endpoint
- Update JS save path from /wp/v2/settings to /wc/v3/pickup-locations
- Add PHPUnit tests covering shop_manager allowed, admin allowed (regression),
editor denied, unauthenticated denied, and save/response correctness
The existing /wp/v2/settings registration is preserved so administrators
and third-party tooling that depend on it are unaffected.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Sanitize nested fields in pickup-locations REST payload
Defense-in-depth: nested object/array fields were passed straight to
update_option() with only top-level type validation. Apply per-field
sanitization (sanitize_text_field for labels, wp_kses_post for details
to match rendering, wp_strip_all_tags for cost to preserve formulas).
* Simplify pickup-locations REST controller and consolidate tracking
* Add changelog entry for Local Pickup Shop Manager permissions fix
* Fix local pickup CI checks
* Fix incorrect XSS assertion in pickup-locations details test
wp_kses_post() strips the <script> tag but keeps its inner text, so the
literal "alert(1)" survives as inert plain text. Assert the executable
<script>alert(1)</script> element is gone instead of the (harmless) text.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Fix Local pickup e2e setup waiting on the old settings endpoint
The pickup-location admin UI now saves via POST /wc/v3/pickup-locations
instead of /wp/v2/settings, so the checkout shopper test's beforeEach
timed out waiting for a /wp/v2/settings response that never fires.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Default missing address sub-keys when sanitizing pickup locations
A partial address (e.g. only country) was persisted as-is, so downstream
readers like ShippingController::filter_taxable_address() — which access
state/postcode/city unconditionally once country is set — would raise
undefined-index notices. Always emit every address sub-key with an empty
string default when an address object is provided.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Mike Jolley <mike.jolley@a8c.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
diff --git a/plugins/woocommerce/changelog/59894-fix-local-pickup-shop-manager-perms b/plugins/woocommerce/changelog/59894-fix-local-pickup-shop-manager-perms
new file mode 100644
index 00000000000..8bd2484a297
--- /dev/null
+++ b/plugins/woocommerce/changelog/59894-fix-local-pickup-shop-manager-perms
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Allow Shop Managers to save Local Pickup settings by adding a dedicated /wc/v3/pickup-locations REST route guarded by the manage_woocommerce capability, instead of posting to /wp/v2/settings which requires manage_options.
diff --git a/plugins/woocommerce/client/blocks/assets/js/extensions/shipping-methods/pickup-location/settings-context.tsx b/plugins/woocommerce/client/blocks/assets/js/extensions/shipping-methods/pickup-location/settings-context.tsx
index 9c7e7bfb187..a90aa308e8c 100644
--- a/plugins/woocommerce/client/blocks/assets/js/extensions/shipping-methods/pickup-location/settings-context.tsx
+++ b/plugins/woocommerce/client/blocks/assets/js/extensions/shipping-methods/pickup-location/settings-context.tsx
@@ -148,7 +148,7 @@ export const SettingsProvider = ( {
// @todo This should be improved to include error handling in case of API failure, or invalid data being sent that
// does not match the schema. This would fail silently on the API side.
apiFetch( {
- path: '/wp/v2/settings',
+ path: '/wc/v3/pickup-locations',
method: 'POST',
data,
} ).then( ( response ) => {
diff --git a/plugins/woocommerce/phpstan-baseline.neon b/plugins/woocommerce/phpstan-baseline.neon
index 5233cf3d74f..fb11252b780 100644
--- a/plugins/woocommerce/phpstan-baseline.neon
+++ b/plugins/woocommerce/phpstan-baseline.neon
@@ -54567,12 +54567,6 @@ parameters:
count: 1
path: src/Blocks/Shipping/ShippingController.php
- -
- message: '#^Action callback returns bool but should not return anything\.$#'
- identifier: return.void
- count: 1
- path: src/Blocks/Shipping/ShippingController.php
-
-
message: '#^Cannot call method get_data\(\) on bool\|WC_Shipping_Zone\.$#'
identifier: method.nonObject
diff --git a/plugins/woocommerce/src/Blocks/Shipping/PickupLocationsRestController.php b/plugins/woocommerce/src/Blocks/Shipping/PickupLocationsRestController.php
new file mode 100644
index 00000000000..64438d77995
--- /dev/null
+++ b/plugins/woocommerce/src/Blocks/Shipping/PickupLocationsRestController.php
@@ -0,0 +1,235 @@
+<?php
+declare( strict_types = 1 );
+
+namespace Automattic\WooCommerce\Blocks\Shipping;
+
+/**
+ * REST controller for Local Pickup location settings.
+ *
+ * Exposes /wc/v3/pickup-locations so users with the manage_woocommerce
+ * capability (e.g. Shop Managers) can save Local Pickup settings without
+ * requiring the manage_options capability needed by /wp/v2/settings.
+ *
+ * @since 11.0.0
+ */
+class PickupLocationsRestController extends \WP_REST_Controller {
+
+ /**
+ * REST API namespace.
+ *
+ * @var string
+ */
+ protected $namespace = 'wc/v3';
+
+ /**
+ * REST API resource base.
+ *
+ * @var string
+ */
+ protected $rest_base = 'pickup-locations';
+
+ /**
+ * Register routes.
+ *
+ * @return void
+ */
+ public function register_routes() {
+ register_rest_route(
+ $this->namespace,
+ '/' . $this->rest_base,
+ array(
+ array(
+ 'methods' => \WP_REST_Server::EDITABLE,
+ 'callback' => array( $this, 'update_settings' ),
+ 'permission_callback' => array( $this, 'update_settings_permissions_check' ),
+ 'args' => array(
+ 'pickup_location_settings' => array(
+ 'description' => __( 'Local pickup method settings.', 'woocommerce' ),
+ 'type' => 'object',
+ ),
+ 'pickup_locations' => array(
+ 'description' => __( 'List of local pickup locations.', 'woocommerce' ),
+ 'type' => 'array',
+ ),
+ ),
+ ),
+ )
+ );
+ }
+
+ /**
+ * Check whether the current user can update pickup location settings.
+ *
+ * @param \WP_REST_Request<array<string, mixed>> $request Request object.
+ * @return true|\WP_Error
+ */
+ public function update_settings_permissions_check( $request ) {
+ if ( ! wc_rest_check_manager_permissions( 'settings', 'edit' ) ) {
+ return new \WP_Error(
+ 'woocommerce_rest_cannot_edit',
+ __( 'Sorry, you cannot edit this resource.', 'woocommerce' ),
+ array( 'status' => rest_authorization_required_code() )
+ );
+ }
+
+ return true;
+ }
+
+ /**
+ * Save pickup location settings and return the saved values.
+ *
+ * @param \WP_REST_Request<array<string, mixed>> $request Request object.
+ * @return \WP_REST_Response|\WP_Error
+ */
+ public function update_settings( $request ) {
+ $settings = $request->get_param( 'pickup_location_settings' );
+ $locations = $request->get_param( 'pickup_locations' );
+
+ if ( is_array( $settings ) ) {
+ $settings = $this->sanitize_pickup_location_settings( $settings );
+ update_option( 'woocommerce_pickup_location_settings', $settings );
+ }
+
+ if ( is_array( $locations ) ) {
+ $locations = $this->sanitize_pickup_locations( $locations );
+ update_option( 'pickup_location_pickup_locations', $locations );
+ }
+
+ // The settings UI always saves both arrays together; a Tracks snapshot is
+ // only meaningful with both present, so skip partial (non-UI) updates.
+ if ( is_array( $settings ) && is_array( $locations ) ) {
+ $this->record_save_event( $settings, $locations );
+ }
+
+ return rest_ensure_response(
+ array(
+ 'pickup_location_settings' => $settings,
+ 'pickup_locations' => $locations,
+ )
+ );
+ }
+
+ /**
+ * Record a Tracks event summarising a Local Pickup settings save.
+ *
+ * @param array $settings Sanitized method settings.
+ * @param array $locations Sanitized list of pickup locations.
+ * @return void
+ */
+ private function record_save_event( array $settings, array $locations ): void {
+ $cost = $settings['cost'] ?? '';
+
+ \WC_Tracks::record_event(
+ 'local_pickup_save_changes',
+ array(
+ 'local_pickup_enabled' => 'yes' === ( $settings['enabled'] ?? '' ),
+ 'title' => __( 'Pickup', 'woocommerce' ) === ( $settings['title'] ?? '' ),
+ 'price' => '' === $cost,
+ 'cost' => '' === $cost ? 0 : $cost,
+ 'taxes' => $settings['tax_status'] ?? '',
+ 'total_pickup_locations' => count( $locations ),
+ 'pickup_locations_enabled' => count(
+ array_filter(
+ $locations,
+ function ( $location ) {
+ return ! empty( $location['enabled'] );
+ }
+ )
+ ),
+ )
+ );
+ }
+
+ /**
+ * Sanitize the pickup_location_settings payload before persisting.
+ *
+ * The WP REST dispatcher only auto-sanitizes top-level args, so nested
+ * object properties need to be cleaned here as defense in depth against
+ * stored HTML/JS in admin surfaces.
+ *
+ * @param array $settings Raw settings payload.
+ * @return array Sanitized settings payload.
+ */
+ private function sanitize_pickup_location_settings( array $settings ): array {
+ $sanitized = array();
+
+ if ( isset( $settings['enabled'] ) ) {
+ $sanitized['enabled'] = in_array( $settings['enabled'], array( 'yes', 'no' ), true )
+ ? $settings['enabled']
+ : 'no';
+ }
+
+ if ( isset( $settings['title'] ) ) {
+ $sanitized['title'] = sanitize_text_field( (string) $settings['title'] );
+ }
+
+ if ( isset( $settings['tax_status'] ) ) {
+ $sanitized['tax_status'] = in_array( $settings['tax_status'], array( 'taxable', 'none' ), true )
+ ? $settings['tax_status']
+ : 'none';
+ }
+
+ if ( isset( $settings['cost'] ) ) {
+ // Cost may be a math expression (e.g. "5 + 1.50"), so strip HTML
+ // without coercing to float — floatval would break formula syntax.
+ $sanitized['cost'] = wp_strip_all_tags( (string) $settings['cost'] );
+ }
+
+ return $sanitized;
+ }
+
+ /**
+ * Sanitize the pickup_locations payload before persisting.
+ *
+ * @param array $locations Raw list of pickup locations.
+ * @return array Sanitized list of pickup locations.
+ */
+ private function sanitize_pickup_locations( array $locations ): array {
+ $sanitized = array();
+
+ foreach ( $locations as $location ) {
+ if ( ! is_array( $location ) ) {
+ continue;
+ }
+
+ $name = isset( $location['name'] ) ? sanitize_text_field( (string) $location['name'] ) : '';
+
+ // A pickup location with no name is unusable, and incomplete entries
+ // would later trigger undefined-index notices in
+ // ShippingController::hydrate_client_settings(), which reads these
+ // fields unconditionally. Skip nameless entries and always emit every
+ // key with a safe default for the ones we keep.
+ if ( '' === $name ) {
+ continue;
+ }
+
+ // Always emit every address key with a safe default. Downstream
+ // readers such as ShippingController::filter_taxable_address() access
+ // state/postcode/city unconditionally once country is set, so a
+ // partial address (e.g. only country) would trigger undefined-index
+ // notices.
+ $address = array();
+ if ( isset( $location['address'] ) && is_array( $location['address'] ) ) {
+ foreach ( array( 'address_1', 'city', 'state', 'postcode', 'country' ) as $field ) {
+ $address[ $field ] = isset( $location['address'][ $field ] )
+ ? sanitize_text_field( (string) $location['address'][ $field ] )
+ : '';
+ }
+ }
+
+ $enabled = isset( $location['enabled'] ) ? rest_sanitize_boolean( (string) $location['enabled'] ) : false;
+
+ $sanitized[] = array(
+ 'name' => $name,
+ 'address' => $address,
+ // Details may contain limited HTML. Match the rendering side
+ // in ShippingController::show_local_pickup_details() which uses
+ // wp_kses_post().
+ 'details' => isset( $location['details'] ) ? wp_kses_post( (string) $location['details'] ) : '',
+ 'enabled' => $enabled,
+ );
+ }
+
+ return $sanitized;
+ }
+}
diff --git a/plugins/woocommerce/src/Blocks/Shipping/ShippingController.php b/plugins/woocommerce/src/Blocks/Shipping/ShippingController.php
index a6a84761bdb..5ca6ce616cf 100644
--- a/plugins/woocommerce/src/Blocks/Shipping/ShippingController.php
+++ b/plugins/woocommerce/src/Blocks/Shipping/ShippingController.php
@@ -10,7 +10,6 @@ use Automattic\WooCommerce\StoreApi\Utilities\LocalPickupUtils;
use Automattic\WooCommerce\Utilities\ArrayUtil;
use WC_Customer;
use WC_Shipping_Rate;
-use WC_Tracks;
/**
* ShippingController class.
@@ -84,7 +83,6 @@ class ShippingController {
add_filter( 'pre_update_option_pickup_location_pickup_locations', array( $this, 'flush_cache' ) );
add_filter( 'woocommerce_shipping_packages', array( $this, 'remove_shipping_if_no_address' ), 11 );
add_filter( 'woocommerce_order_shipping_to_display', array( $this, 'show_local_pickup_details' ), 10, 2 );
- add_action( 'rest_pre_serve_request', array( $this, 'track_local_pickup' ), 10, 4 );
}
/**
@@ -311,6 +309,10 @@ class ShippingController {
),
)
);
+
+ // Save route guarded by manage_woocommerce (via wc_rest_check_manager_permissions)
+ // so Shop Managers can save Local Pickup settings without needing manage_options.
+ ( new PickupLocationsRestController() )->register_routes();
}
/**
@@ -607,45 +609,19 @@ class ShippingController {
}
/**
- * Track local pickup settings changes via Store API
+ * Track local pickup settings changes.
+ *
+ * @deprecated 11.0.0 Tracking now happens inside PickupLocationsRestController::update_settings().
*
- * @param bool $served Whether the request has already been served.
- * @param \WP_REST_Response $result The response object.
+ * @param bool $served Whether the request has already been served.
+ * @param \WP_REST_Response $result The response object.
* @param \WP_REST_Request $request The request object.
* @return bool
*/
public function track_local_pickup( $served, $result, $request ) {
- if ( '/wp/v2/settings' !== $request->get_route() ) {
- return $served;
- }
- // Param name here comes from the show_in_rest['name'] value when registering the setting.
- if ( ! $request->get_param( 'pickup_location_settings' ) && ! $request->get_param( 'pickup_locations' ) ) {
- return $served;
- }
-
- $event_name = 'local_pickup_save_changes';
-
- $settings = $request->get_param( 'pickup_location_settings' );
- $locations = $request->get_param( 'pickup_locations' );
-
- $data = array(
- 'local_pickup_enabled' => 'yes' === $settings['enabled'] ? true : false,
- 'title' => __( 'Pickup', 'woocommerce' ) === $settings['title'],
- 'price' => '' === $settings['cost'] ? true : false,
- 'cost' => '' === $settings['cost'] ? 0 : $settings['cost'],
- 'taxes' => $settings['tax_status'],
- 'total_pickup_locations' => count( $locations ),
- 'pickup_locations_enabled' => count(
- array_filter(
- $locations,
- function ( $location ) {
- return $location['enabled']; }
- )
- ),
- );
-
- WC_Tracks::record_event( $event_name, $data );
+ unset( $result, $request );
+ wc_deprecated_function( __METHOD__, '11.0.0' );
return $served;
}
diff --git a/plugins/woocommerce/tests/e2e/tests/blocks/checkout/checkout-block.shopper.block_theme.spec.ts b/plugins/woocommerce/tests/e2e/tests/blocks/checkout/checkout-block.shopper.block_theme.spec.ts
index 662223deaaa..44b3ffaef39 100644
--- a/plugins/woocommerce/tests/e2e/tests/blocks/checkout/checkout-block.shopper.block_theme.spec.ts
+++ b/plugins/woocommerce/tests/e2e/tests/blocks/checkout/checkout-block.shopper.block_theme.spec.ts
@@ -141,7 +141,7 @@ test.describe( 'Shopper → Local pickup', () => {
.getByRole( 'button', { name: 'Save changes' } )
.click();
await admin.page.waitForResponse( ( response ) => {
- return response.url().includes( 'wp-json/wp/v2/settings' );
+ return response.url().includes( 'wp-json/wc/v3/pickup-locations' );
} );
} );
diff --git a/plugins/woocommerce/tests/php/src/Blocks/Shipping/PickupLocationsRestControllerTest.php b/plugins/woocommerce/tests/php/src/Blocks/Shipping/PickupLocationsRestControllerTest.php
new file mode 100644
index 00000000000..fa682edda2d
--- /dev/null
+++ b/plugins/woocommerce/tests/php/src/Blocks/Shipping/PickupLocationsRestControllerTest.php
@@ -0,0 +1,381 @@
+<?php
+declare( strict_types = 1 );
+
+namespace Automattic\WooCommerce\Tests\Blocks\Shipping;
+
+use Automattic\WooCommerce\Blocks\Shipping\PickupLocationsRestController;
+use WC_Unit_Test_Case;
+
+/**
+ * Tests for the PickupLocationsRestController class.
+ */
+class PickupLocationsRestControllerTest extends WC_Unit_Test_Case {
+
+ /**
+ * The System Under Test.
+ *
+ * @var PickupLocationsRestController
+ */
+ private $sut;
+
+ /**
+ * Shop manager user ID.
+ *
+ * @var int
+ */
+ private $shop_manager_id;
+
+ /**
+ * Editor user ID (no WooCommerce caps).
+ *
+ * @var int
+ */
+ private $editor_id;
+
+ /**
+ * Set up test fixtures.
+ */
+ public function setUp(): void {
+ parent::setUp();
+ $this->sut = new PickupLocationsRestController();
+ $this->shop_manager_id = self::factory()->user->create( array( 'role' => 'shop_manager' ) );
+ $this->editor_id = self::factory()->user->create( array( 'role' => 'editor' ) );
+ }
+
+ /**
+ * Tear down test fixtures.
+ */
+ public function tearDown(): void {
+ wp_set_current_user( 0 );
+ delete_option( 'woocommerce_pickup_location_settings' );
+ delete_option( 'pickup_location_pickup_locations' );
+ parent::tearDown();
+ }
+
+ // -------------------------------------------------------------------------
+ // Permission check tests
+ // -------------------------------------------------------------------------
+
+ /**
+ * @testdox Should allow a shop manager to update pickup location settings.
+ */
+ public function test_shop_manager_can_update_settings(): void {
+ wp_set_current_user( $this->shop_manager_id );
+
+ $result = $this->sut->update_settings_permissions_check(
+ new \WP_REST_Request( 'POST', '/wc/v3/pickup-locations' )
+ );
+
+ $this->assertTrue( $result, 'A shop manager should be allowed to edit pickup location settings.' );
+ }
+
+ /**
+ * @testdox Should deny an editor from updating pickup location settings.
+ */
+ public function test_editor_cannot_update_settings(): void {
+ wp_set_current_user( $this->editor_id );
+
+ $result = $this->sut->update_settings_permissions_check(
+ new \WP_REST_Request( 'POST', '/wc/v3/pickup-locations' )
+ );
+
+ $this->assertWPError( $result, 'An editor should not be allowed to edit pickup location settings.' );
+ }
+
+ // -------------------------------------------------------------------------
+ // Save / response tests
+ // -------------------------------------------------------------------------
+
+ /**
+ * @testdox Should save pickup location method settings and echo them back in the response.
+ */
+ public function test_update_settings_saves_method_settings(): void {
+ wp_set_current_user( $this->shop_manager_id );
+
+ $settings = array(
+ 'enabled' => 'yes',
+ 'title' => 'Local Pickup',
+ 'tax_status' => 'taxable',
+ 'cost' => '',
+ );
+
+ $request = new \WP_REST_Request( 'POST', '/wc/v3/pickup-locations' );
+ $request->set_param( 'pickup_location_settings', $settings );
+
+ $response = $this->sut->update_settings( $request );
+ $data = $response->get_data();
+
+ $this->assertSame( $settings, $data['pickup_location_settings'], 'Response should echo back the saved method settings.' );
+ $this->assertSame( $settings, get_option( 'woocommerce_pickup_location_settings' ), 'Method settings should be persisted to the database.' );
+ }
+
+ /**
+ * @testdox Should save pickup locations list and echo it back in the response.
+ */
+ public function test_update_settings_saves_pickup_locations(): void {
+ wp_set_current_user( $this->shop_manager_id );
+
+ $locations = array(
+ array(
+ 'name' => 'Main Store',
+ 'address' => array(
+ 'address_1' => '123 Main St',
+ 'city' => 'Anytown',
+ 'state' => 'CA',
+ 'postcode' => '90210',
+ 'country' => 'US',
+ ),
+ 'details' => 'Open daily 9am-5pm',
+ 'enabled' => true,
+ ),
+ );
+
+ $request = new \WP_REST_Request( 'POST', '/wc/v3/pickup-locations' );
+ $request->set_param( 'pickup_locations', $locations );
+
+ $response = $this->sut->update_settings( $request );
+ $data = $response->get_data();
+
+ $this->assertSame( $locations, $data['pickup_locations'], 'Response should echo back the saved locations.' );
+ $this->assertSame( $locations, get_option( 'pickup_location_pickup_locations' ), 'Locations should be persisted to the database.' );
+ }
+
+ /**
+ * @testdox Should drop incomplete locations and default missing keys so admin hydration never hits undefined indexes.
+ */
+ public function test_update_settings_drops_incomplete_locations(): void {
+ wp_set_current_user( $this->shop_manager_id );
+
+ $request = new \WP_REST_Request( 'POST', '/wc/v3/pickup-locations' );
+ $request->set_param(
+ 'pickup_locations',
+ array(
+ // Empty object: dropped.
+ array(),
+ // Nameless: dropped.
+ array( 'name' => '' ),
+ // Name only: kept, other keys defaulted.
+ array( 'name' => 'Warehouse' ),
+ )
+ );
+
+ $this->sut->update_settings( $request );
+
+ $saved = get_option( 'pickup_location_pickup_locations' );
+
+ $this->assertCount( 1, $saved, 'Only the named location should be persisted.' );
+ $this->assertSame(
+ array(
+ 'name' => 'Warehouse',
+ 'address' => array(),
+ 'details' => '',
+ 'enabled' => false,
+ ),
+ $saved[0],
+ 'A kept location must always carry every key so hydrate_client_settings() never reads a missing index.'
+ );
+ }
+
+ /**
+ * @testdox Should default missing address sub-keys when a partial address is provided.
+ */
+ public function test_update_settings_defaults_partial_address_keys(): void {
+ wp_set_current_user( $this->shop_manager_id );
+
+ $request = new \WP_REST_Request( 'POST', '/wc/v3/pickup-locations' );
+ $request->set_param(
+ 'pickup_locations',
+ array(
+ array(
+ 'name' => 'Warehouse',
+ // Only country supplied; the rest must be defaulted so
+ // ShippingController::filter_taxable_address() never reads a
+ // missing index once country is set.
+ 'address' => array( 'country' => 'US' ),
+ ),
+ )
+ );
+
+ $this->sut->update_settings( $request );
+
+ $saved = get_option( 'pickup_location_pickup_locations' );
+
+ $this->assertSame(
+ array(
+ 'address_1' => '',
+ 'city' => '',
+ 'state' => '',
+ 'postcode' => '',
+ 'country' => 'US',
+ ),
+ $saved[0]['address'],
+ 'A provided address must carry every sub-key so downstream readers never hit undefined indexes.'
+ );
+ }
+
+ /**
+ * @testdox Should preserve existing settings when only one param is sent.
+ */
+ public function test_omitted_params_are_not_overwritten(): void {
+ wp_set_current_user( $this->shop_manager_id );
+
+ $original_settings = array(
+ 'enabled' => 'yes',
+ 'title' => 'Local Pickup',
+ 'tax_status' => 'taxable',
+ 'cost' => '5.00',
+ );
+ update_option( 'woocommerce_pickup_location_settings', $original_settings );
+
+ $request = new \WP_REST_Request( 'POST', '/wc/v3/pickup-locations' );
+ $request->set_param( 'pickup_locations', array() );
+
+ $this->sut->update_settings( $request );
+
+ $this->assertSame(
+ $original_settings,
+ get_option( 'woocommerce_pickup_location_settings' ),
+ 'Existing method settings should not be overwritten when only pickup_locations is sent.'
+ );
+ }
+
+ // -------------------------------------------------------------------------
+ // Sanitization tests (defense in depth against stored XSS)
+ // -------------------------------------------------------------------------
+
+ /**
+ * @testdox Should strip HTML/script from the method settings title before saving.
+ */
+ public function test_update_settings_sanitizes_html_in_title(): void {
+ wp_set_current_user( $this->shop_manager_id );
+
+ $request = new \WP_REST_Request( 'POST', '/wc/v3/pickup-locations' );
+ $request->set_param(
+ 'pickup_location_settings',
+ array(
+ 'enabled' => 'yes',
+ 'title' => '<script>alert(1)</script>Pickup',
+ 'tax_status' => 'taxable',
+ 'cost' => '',
+ )
+ );
+
+ $this->sut->update_settings( $request );
+
+ $saved = get_option( 'woocommerce_pickup_location_settings' );
+
+ $this->assertIsArray( $saved );
+ $this->assertArrayHasKey( 'title', $saved );
+ $this->assertStringNotContainsString( '<script', $saved['title'], 'Script tag must be stripped from saved title.' );
+ $this->assertStringNotContainsString( 'alert(1)', $saved['title'], 'Inline script payload must not survive sanitization.' );
+ $this->assertStringContainsString( 'Pickup', $saved['title'], 'Plain text portion of the title should be preserved.' );
+ }
+
+ /**
+ * @testdox Should preserve safe HTML in location details but strip scripts.
+ */
+ public function test_update_settings_preserves_safe_html_in_details(): void {
+ wp_set_current_user( $this->shop_manager_id );
+
+ $locations = array(
+ array(
+ 'name' => 'Main Store',
+ 'address' => array(
+ 'address_1' => '123 Main St',
+ 'city' => 'Anytown',
+ 'state' => 'CA',
+ 'postcode' => '90210',
+ 'country' => 'US',
+ ),
+ 'details' => '<strong>Hours:</strong> 9-5 <a href="https://example.com">Map</a><script>alert(1)</script>',
+ 'enabled' => true,
+ ),
+ );
+
+ $request = new \WP_REST_Request( 'POST', '/wc/v3/pickup-locations' );
+ $request->set_param( 'pickup_locations', $locations );
+
+ $this->sut->update_settings( $request );
+
+ $saved = get_option( 'pickup_location_pickup_locations' );
+
+ $this->assertIsArray( $saved );
+ $this->assertArrayHasKey( 0, $saved );
+ $this->assertArrayHasKey( 'details', $saved[0] );
+ $this->assertStringContainsString( '<strong>', $saved[0]['details'], 'wp_kses_post should keep <strong>.' );
+ $this->assertStringContainsString( '<a ', $saved[0]['details'], 'wp_kses_post should keep <a> tags.' );
+ $this->assertStringNotContainsString( '<script', $saved[0]['details'], '<script> must be stripped by wp_kses_post.' );
+ // wp_kses_post() removes the executable <script> tag but keeps its inner
+ // text as harmless plain text, so assert the executable element is gone
+ // rather than the (now inert) "alert(1)" string.
+ $this->assertStringNotContainsString( '<script>alert(1)</script>', $saved[0]['details'], 'Executable script element must not survive sanitization.' );
+ }
+
+ /**
+ * @testdox Should strip HTML/script from pickup location name and address fields.
+ */
+ public function test_update_settings_sanitizes_html_in_location_fields(): void {
+ wp_set_current_user( $this->shop_manager_id );
+
+ $locations = array(
+ array(
+ 'name' => '<script>alert(1)</script>Main Store',
+ 'address' => array(
+ 'address_1' => '<img src=x onerror=alert(1)>123 Main St',
+ 'city' => 'Anytown',
+ 'state' => 'CA',
+ 'postcode' => '90210',
+ 'country' => 'US',
+ ),
+ 'details' => '',
+ 'enabled' => true,
+ ),
+ );
+
+ $request = new \WP_REST_Request( 'POST', '/wc/v3/pickup-locations' );
+ $request->set_param( 'pickup_locations', $locations );
+
+ $this->sut->update_settings( $request );
+
+ $saved = get_option( 'pickup_location_pickup_locations' );
+
+ $this->assertIsArray( $saved );
+ $this->assertArrayHasKey( 0, $saved );
+ $this->assertStringNotContainsString( '<script', $saved[0]['name'], 'Script tag must be stripped from saved location name.' );
+ $this->assertStringNotContainsString( 'alert(1)', $saved[0]['name'], 'Inline script payload must not survive location name sanitization.' );
+ $this->assertStringContainsString( 'Main Store', $saved[0]['name'], 'Plain text portion of the location name should be preserved.' );
+
+ $this->assertArrayHasKey( 'address_1', $saved[0]['address'] );
+ $this->assertStringNotContainsString( '<img', $saved[0]['address']['address_1'], 'HTML tags must be stripped from address fields.' );
+ $this->assertStringNotContainsString( 'onerror', $saved[0]['address']['address_1'], 'Event handler payload must not survive address sanitization.' );
+ $this->assertStringContainsString( '123 Main St', $saved[0]['address']['address_1'], 'Plain text portion of the address should be preserved.' );
+ }
+
+ /**
+ * @testdox Should preserve math expressions in cost while stripping HTML.
+ */
+ public function test_update_settings_preserves_cost_formula(): void {
+ wp_set_current_user( $this->shop_manager_id );
+
+ $request = new \WP_REST_Request( 'POST', '/wc/v3/pickup-locations' );
+ $request->set_param(
+ 'pickup_location_settings',
+ array(
+ 'enabled' => 'yes',
+ 'title' => 'Local Pickup',
+ 'tax_status' => 'taxable',
+ 'cost' => '<script>alert(1)</script>5 + 1.50',
+ )
+ );
+
+ $this->sut->update_settings( $request );
+
+ $saved = get_option( 'woocommerce_pickup_location_settings' );
+
+ $this->assertIsArray( $saved );
+ $this->assertArrayHasKey( 'cost', $saved );
+ $this->assertStringNotContainsString( '<script', $saved['cost'], '<script> must be stripped from cost.' );
+ $this->assertStringNotContainsString( 'alert(1)', $saved['cost'], 'Inline script payload must not survive cost sanitization.' );
+ $this->assertStringContainsString( '5 + 1.50', $saved['cost'], 'Math formula syntax must be preserved in cost — must not be coerced to float.' );
+ }
+}