Commit 189ebb81ced for woocommerce
commit 189ebb81ceda5e0f5789c98f73663ad28429e592
Author: Vlad Olaru <vlad.olaru@automattic.com>
Date: Mon Mar 23 15:13:12 2026 +0200
Fix REST API settings schemas corrupting password fields with percent-encoded characters (#63597)
* fix(settings): Preserve percent-encoded characters in REST API password fields
Both EmailsSettingsSchema and AbstractPaymentGatewaySettingsSchema used
sanitize_text_field() for password fields, which decodes percent-encoded
sequences. A password like 'NlP4%EcCx}Na' would silently become
'NlP4Cx}Na' — the API returns 200 but stores a corrupted value, making
the gateway unusable with no visible error.
Replace sanitize_text_field() with wp_strip_all_tags( trim() ) for the
password case in both schemas, matching the fix applied to
WC_Admin_Settings::save_fields() in WOOPLUG-6157.
Refs WOOPLUG-6395
* chore: Add changelog entry for REST API password percent-encoding fix
Refs WOOPLUG-6395
* test(settings): Add trim behavior tests for REST API password sanitization
The companion admin settings fix (WOOPLUG-6157) included a whitespace-
trim test, but the REST API tests did not. Since the fix explicitly uses
trim() in wp_strip_all_tags(trim($value)), this behavior should be
verified: leading/trailing whitespace is stripped while percent-encoded
sequences are preserved.
Refs WOOPLUG-6395
* test: use mock gateway for password field sanitization tests
The password field tests relied on the PayPal Standard gateway, which is
conditionally loaded based on store history (enabled status or existing
PayPal orders). In CI, those conditions are never met, so the gateway is
absent and the REST endpoint returns 404.
Replace with a dedicated WCGatewayMockPassword that is always registered
via the woocommerce_payment_gateways filter, making the tests environment-
independent.
Refs WOOPLUG-6395
* fix: use minimal sanitization for REST API password fields to avoid truncation
The password cases in EmailsSettingsSchema and
AbstractPaymentGatewaySettingsSchema used wp_strip_all_tags(), which
calls PHP's strip_tags() internally. strip_tags() treats a lone '<' as
the start of a malformed HTML tag and drops everything from the '<'
onward (e.g. "pass<word" becomes "pass"), silently corrupting stored
API keys and SMTP passwords.
Replace wp_strip_all_tags( trim() ) with just trim() to align with
WC_Settings_API::validate_password_field(), which deliberately applies
no input sanitization to avoid corrupting passwords. Unlike the admin
settings path, the REST API does not add magic quotes, so no
stripslashes() is needed either.
Refs WOOPLUG-6395
* fix: guard REST API password fields against non-string values
trim() throws a TypeError for arrays/objects and emits a deprecation
for null in PHP 8.1+. The REST API does not enforce string type on
individual field values within the settings payload, so malformed
requests could send null or an array for a password field.
Add is_string() guard to both EmailsSettingsSchema and
AbstractPaymentGatewaySettingsSchema, returning '' for non-string
values. This matches the type guard pattern used in
WC_Admin_Settings::save_fields() and is consistent with how other
cases in the same switch handle type mismatches.
Refs WOOPLUG-6395
* fix: coerce scalar JSON password values to string in REST API sanitization
The previous is_string() guard returned '' for any non-string input,
including integers. A REST client sending a bare JSON number for a
password field (e.g. a 6-digit PIN: {"api_password": 123456}) would have
its secret silently cleared instead of stored as "123456".
Widen the guard to is_scalar() and cast to string, which restores the
coercion behaviour of the prior sanitize_text_field() path while still
rejecting arrays and objects from malformed requests.
Adds a regression test for the numeric-password case.
Refs WOOPLUG-6395
* test: inject mock payment gateway directly instead of using a filter
The anonymous woocommerce_payment_gateways filter added in setUp() was
never removed, so WCGatewayMockPassword would accumulate in $wp_filter
across the test process and appear in any later test class that called
WC()->payment_gateways()->init().
WC_REST_Unit_Test_Case::setUp() already resets the gateway array and
calls init(). Directly appending the mock instance to the initialized
array avoids filter pollution and the redundant second init() call.
Refs WOOPLUG-6395
* docs: clarify why stripslashes() is omitted from REST password sanitization
The comments in both schemas claimed the code "matches
WC_Settings_API::validate_password_field()", but that method calls
trim(stripslashes($value)) — and ours does not.
The divergence is intentional: validate_password_field() strips slashes
to undo WordPress's add_magic_quotes() on form POST data. REST JSON
input is never magic-quote-escaped, so calling stripslashes() there
would corrupt passwords containing literal backslashes.
Also aligns the EmailsSettingsSchema password case with the updated
AbstractPaymentGatewaySettingsSchema: replaces is_string() with
is_scalar() + (string) cast so numeric PINs/API keys are preserved
instead of blanked.
Refs WOOPLUG-6395
* chore: update changelog to cover all password field corruption cases
The original entry only mentioned percent-encoded sequences. Expand to
cover '<' truncation and numeric-only PIN blanking, which are also fixed
in this branch.
Refs WOOPLUG-6395
* chore: specify v4 in changelog message
Refs WOOPLUG-6395
* test: add DB persistence assertions to password field tests
The password sanitization tests verified the in-memory gateway settings
but did not confirm the values reached the database. Since the bug is
about silent data corruption during save, assert via get_option() that
the stored value matches the expected password in all five scenarios.
Refs WOOPLUG-6395
* refactor: shorten password field comments per review feedback
Reduce verbosity of password sanitization comments in both
EmailsSettingsSchema and AbstractPaymentGatewaySettingsSchema
while keeping all key context: why stripslashes is omitted for
REST JSON, why aggressive sanitization is skipped, and the
non-scalar/scalar handling rationale.
Refs WOOPLUG-6395
diff --git a/plugins/woocommerce/changelog/fix-WOOPLUG-6395-rest-api-password-percent-encoding b/plugins/woocommerce/changelog/fix-WOOPLUG-6395-rest-api-password-percent-encoding
new file mode 100644
index 00000000000..de5a16eac43
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-WOOPLUG-6395-rest-api-password-percent-encoding
@@ -0,0 +1,5 @@
+Significance: patch
+Type: fix
+Comment: Fixes silent corruption of password and API key fields saved via the V4 REST API settings endpoints for payment gateways and emails. Values containing percent-encoded sequences (e.g. %Ec), a literal '<', or a numeric-only PIN could be truncated or blanked without any error.
+
+Fix REST v4 API silently corrupting password and API key fields with special characters
diff --git a/plugins/woocommerce/src/Internal/RestApi/Routes/V4/Settings/Emails/Schema/EmailsSettingsSchema.php b/plugins/woocommerce/src/Internal/RestApi/Routes/V4/Settings/Emails/Schema/EmailsSettingsSchema.php
index b3ca15b55c8..b774aa9899d 100644
--- a/plugins/woocommerce/src/Internal/RestApi/Routes/V4/Settings/Emails/Schema/EmailsSettingsSchema.php
+++ b/plugins/woocommerce/src/Internal/RestApi/Routes/V4/Settings/Emails/Schema/EmailsSettingsSchema.php
@@ -551,8 +551,15 @@ class EmailsSettingsSchema extends AbstractSchema {
}
return is_string( $value ) ? array( sanitize_text_field( $value ) ) : array();
- case 'color':
case 'password':
+ // Only trim — no stripslashes() (REST JSON is not magic-quote-escaped),
+ // no wp_strip_all_tags() or wc_clean() which would corrupt passwords
+ // containing '<', backslashes, or percent-like sequences.
+ // Non-scalar values (arrays, objects, null) from malformed requests → empty string.
+ // Scalars coerced to string to preserve numeric PINs/API keys.
+ return is_scalar( $value ) ? trim( (string) $value ) : '';
+
+ case 'color':
case 'text':
case 'select':
default:
diff --git a/plugins/woocommerce/src/Internal/RestApi/Routes/V4/Settings/PaymentGateways/Schema/AbstractPaymentGatewaySettingsSchema.php b/plugins/woocommerce/src/Internal/RestApi/Routes/V4/Settings/PaymentGateways/Schema/AbstractPaymentGatewaySettingsSchema.php
index 1e0ab7eed08..c4457fec7b0 100644
--- a/plugins/woocommerce/src/Internal/RestApi/Routes/V4/Settings/PaymentGateways/Schema/AbstractPaymentGatewaySettingsSchema.php
+++ b/plugins/woocommerce/src/Internal/RestApi/Routes/V4/Settings/PaymentGateways/Schema/AbstractPaymentGatewaySettingsSchema.php
@@ -615,6 +615,13 @@ abstract class AbstractPaymentGatewaySettingsSchema extends AbstractSchema {
return sanitize_email( $value );
case 'password':
+ // Only trim — no stripslashes() (REST JSON is not magic-quote-escaped),
+ // no wp_strip_all_tags() or wc_clean() which would corrupt passwords
+ // containing '<', backslashes, or percent-like sequences.
+ // Non-scalar values (arrays, objects, null) from malformed requests → empty string.
+ // Scalars coerced to string to preserve numeric PINs/API keys.
+ return is_scalar( $value ) ? trim( (string) $value ) : '';
+
case 'color':
return sanitize_text_field( $value );
diff --git a/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Settings/Emails/EmailsSettingsControllerTest.php b/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Settings/Emails/EmailsSettingsControllerTest.php
index 7825edb1a43..d3e0e6d1834 100644
--- a/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Settings/Emails/EmailsSettingsControllerTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Settings/Emails/EmailsSettingsControllerTest.php
@@ -706,6 +706,84 @@ class EmailsSettingsControllerTest extends WC_REST_Unit_Test_Case {
}
}
+ /**
+ * @testdox Should preserve percent-encoded characters in password field sanitization.
+ */
+ public function test_sanitize_field_value_preserves_percent_encoded_chars_in_password_fields() {
+ // Arrange — use reflection since sanitize_field_value is private.
+ $schema = new EmailsSettingsSchema();
+ $reflection = new \ReflectionClass( $schema );
+ $method = $reflection->getMethod( 'sanitize_field_value' );
+ $method->setAccessible( true );
+
+ $password = 'NlP4%EcCx}Na';
+
+ // Act.
+ $result = $method->invoke( $schema, 'password', $password );
+
+ // Assert.
+ $this->assertSame( $password, $result, 'Password with %Ec sequence should be preserved' );
+ }
+
+ /**
+ * @testdox Should preserve HTML-like characters in password field values.
+ *
+ * Password fields use minimal sanitization (trim only) to avoid corrupting
+ * passwords and API keys, matching WC_Settings_API::validate_password_field().
+ * Characters like '<' and '>' are valid in secrets and must not be stripped.
+ */
+ public function test_sanitize_field_value_preserves_html_like_chars_in_password_fields() {
+ // Arrange.
+ $schema = new EmailsSettingsSchema();
+ $reflection = new \ReflectionClass( $schema );
+ $method = $reflection->getMethod( 'sanitize_field_value' );
+ $method->setAccessible( true );
+
+ // Act.
+ $result = $method->invoke( $schema, 'password', '<b>bold</b>secret%E0pass' );
+
+ // Assert.
+ $this->assertSame( '<b>bold</b>secret%E0pass', $result, 'HTML-like characters should be preserved in password fields' );
+ }
+
+ /**
+ * @testdox Should preserve a lone '<' in password field values without truncation.
+ *
+ * PHP's strip_tags() treats a lone '<' as the start of a malformed HTML tag and drops
+ * everything from the '<' onward (e.g. "abc<def" becomes "abc"). Password fields must
+ * not use strip_tags() or wp_strip_all_tags() for this reason.
+ */
+ public function test_sanitize_field_value_preserves_lone_less_than_in_password_fields() {
+ // Arrange.
+ $schema = new EmailsSettingsSchema();
+ $reflection = new \ReflectionClass( $schema );
+ $method = $reflection->getMethod( 'sanitize_field_value' );
+ $method->setAccessible( true );
+
+ // Act.
+ $result = $method->invoke( $schema, 'password', 'pass<word123' );
+
+ // Assert.
+ $this->assertSame( 'pass<word123', $result, 'A lone < must not truncate the password' );
+ }
+
+ /**
+ * @testdox Should trim whitespace from password field values.
+ */
+ public function test_sanitize_field_value_trims_whitespace_from_password_fields() {
+ // Arrange.
+ $schema = new EmailsSettingsSchema();
+ $reflection = new \ReflectionClass( $schema );
+ $method = $reflection->getMethod( 'sanitize_field_value' );
+ $method->setAccessible( true );
+
+ // Act.
+ $result = $method->invoke( $schema, 'password', ' my%20password ' );
+
+ // Assert.
+ $this->assertSame( 'my%20password', $result, 'Password should be trimmed but percent sequences preserved' );
+ }
+
/**
* Helper: Get stored subject from database.
*
diff --git a/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Settings/PaymentGateways/PaymentGatewaysSettingsControllerTest.php b/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Settings/PaymentGateways/PaymentGatewaysSettingsControllerTest.php
index 70e91e9cd7b..b1ea340a787 100644
--- a/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Settings/PaymentGateways/PaymentGatewaysSettingsControllerTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Settings/PaymentGateways/PaymentGatewaysSettingsControllerTest.php
@@ -45,6 +45,9 @@ class PaymentGatewaysSettingsControllerTest extends WC_REST_Unit_Test_Case {
$this->store_admin_id = $this->factory->user->create( array( 'role' => 'administrator' ) );
wp_set_current_user( $this->store_admin_id );
+ // Inject the mock gateway directly — avoids filter pollution and a second init() call.
+ WC()->payment_gateways()->payment_gateways[] = new WCGatewayMockPassword();
+
$this->sut = new Controller();
$this->sut->register_routes();
}
@@ -693,6 +696,153 @@ class PaymentGatewaysSettingsControllerTest extends WC_REST_Unit_Test_Case {
$this->assertArrayHasKey( 'instructions', $data['values'] );
}
+ /**
+ * @testdox Should preserve percent-encoded characters in password fields.
+ */
+ public function test_update_payment_gateway_preserves_percent_encoded_chars_in_password_fields() {
+ // Arrange — mock gateway has a password-type field (api_password).
+ $password = 'NlP4%EcCx}Na';
+
+ // Act.
+ $request = new WP_REST_Request( 'PUT', self::ENDPOINT . '/mock_password' );
+ $request->set_param(
+ 'values',
+ array(
+ 'api_password' => $password,
+ )
+ );
+ $response = $this->server->dispatch( $request );
+
+ // Assert.
+ $this->assertSame( 200, $response->get_status() );
+
+ $gateway = WC()->payment_gateways->payment_gateways()['mock_password'];
+ $this->assertSame( $password, $gateway->settings['api_password'], 'Password with %Ec sequence should be preserved' );
+
+ // Verify DB persistence — the value that reached the database must also be intact.
+ $stored = get_option( 'woocommerce_mock_password_settings', array() );
+ $this->assertSame( $password, $stored['api_password'] ?? null, 'Password should be persisted to database without corruption' );
+ }
+
+ /**
+ * @testdox Should preserve HTML-like characters in password fields.
+ *
+ * Password fields use minimal sanitization (trim only) to avoid corrupting
+ * passwords and API keys, matching WC_Settings_API::validate_password_field().
+ * Characters like '<' and '>' are valid in secrets and must not be stripped.
+ */
+ public function test_update_payment_gateway_preserves_html_like_chars_in_password_fields() {
+ // Arrange.
+ $request = new WP_REST_Request( 'PUT', self::ENDPOINT . '/mock_password' );
+ $request->set_param(
+ 'values',
+ array(
+ 'api_password' => '<b>bold</b>secret%E0pass',
+ )
+ );
+
+ // Act.
+ $response = $this->server->dispatch( $request );
+
+ // Assert.
+ $this->assertSame( 200, $response->get_status() );
+
+ $gateway = WC()->payment_gateways->payment_gateways()['mock_password'];
+ $this->assertSame( '<b>bold</b>secret%E0pass', $gateway->settings['api_password'], 'HTML-like characters should be preserved in password fields' );
+
+ // Verify DB persistence.
+ $stored = get_option( 'woocommerce_mock_password_settings', array() );
+ $this->assertSame( '<b>bold</b>secret%E0pass', $stored['api_password'] ?? null, 'Password should be persisted to database without corruption' );
+ }
+
+ /**
+ * @testdox Should preserve a lone '<' in password field values without truncation.
+ *
+ * PHP's strip_tags() treats a lone '<' as the start of a malformed HTML tag and drops
+ * everything from the '<' onward (e.g. "abc<def" becomes "abc"). Password fields must
+ * not use strip_tags() or wp_strip_all_tags() for this reason.
+ */
+ public function test_update_payment_gateway_preserves_lone_less_than_in_password_fields() {
+ // Arrange.
+ $request = new WP_REST_Request( 'PUT', self::ENDPOINT . '/mock_password' );
+ $request->set_param(
+ 'values',
+ array(
+ 'api_password' => 'pass<word123',
+ )
+ );
+
+ // Act.
+ $response = $this->server->dispatch( $request );
+
+ // Assert.
+ $this->assertSame( 200, $response->get_status() );
+
+ $gateway = WC()->payment_gateways->payment_gateways()['mock_password'];
+ $this->assertSame( 'pass<word123', $gateway->settings['api_password'], 'A lone < must not truncate the password' );
+
+ // Verify DB persistence.
+ $stored = get_option( 'woocommerce_mock_password_settings', array() );
+ $this->assertSame( 'pass<word123', $stored['api_password'] ?? null, 'Password should be persisted to database without corruption' );
+ }
+
+ /**
+ * @testdox Should trim whitespace from password fields while preserving percent-encoded characters.
+ */
+ public function test_update_payment_gateway_trims_whitespace_from_password_fields() {
+ // Arrange.
+ $request = new WP_REST_Request( 'PUT', self::ENDPOINT . '/mock_password' );
+ $request->set_param(
+ 'values',
+ array(
+ 'api_password' => ' my%20password ',
+ )
+ );
+
+ // Act.
+ $response = $this->server->dispatch( $request );
+
+ // Assert.
+ $this->assertSame( 200, $response->get_status() );
+
+ $gateway = WC()->payment_gateways->payment_gateways()['mock_password'];
+ $this->assertSame( 'my%20password', $gateway->settings['api_password'], 'Password should be trimmed but percent sequences preserved' );
+
+ // Verify DB persistence.
+ $stored = get_option( 'woocommerce_mock_password_settings', array() );
+ $this->assertSame( 'my%20password', $stored['api_password'] ?? null, 'Password should be persisted to database without corruption' );
+ }
+
+ /**
+ * @testdox Should coerce a numeric JSON value for a password field to string instead of blanking it.
+ *
+ * json_decode() returns an int when the client sends a bare number (e.g. a 6-digit PIN).
+ * The sanitizer must not silently clear numeric-only secrets.
+ */
+ public function test_update_payment_gateway_coerces_numeric_password_to_string() {
+ // Arrange — WP REST API delivers the value as an int after JSON decoding.
+ $request = new WP_REST_Request( 'PUT', self::ENDPOINT . '/mock_password' );
+ $request->set_param(
+ 'values',
+ array(
+ 'api_password' => 123456,
+ )
+ );
+
+ // Act.
+ $response = $this->server->dispatch( $request );
+
+ // Assert.
+ $this->assertSame( 200, $response->get_status() );
+
+ $gateway = WC()->payment_gateways->payment_gateways()['mock_password'];
+ $this->assertSame( '123456', $gateway->settings['api_password'], 'Numeric password value should be coerced to string, not blanked' );
+
+ // Verify DB persistence.
+ $stored = get_option( 'woocommerce_mock_password_settings', array() );
+ $this->assertSame( '123456', $stored['api_password'] ?? null, 'Password should be persisted to database without corruption' );
+ }
+
/**
* Test that COD gateway enable_for_methods field has options populated.
*/
diff --git a/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Settings/PaymentGateways/WCGatewayMockPassword.php b/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Settings/PaymentGateways/WCGatewayMockPassword.php
new file mode 100644
index 00000000000..98b1c8136b5
--- /dev/null
+++ b/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Settings/PaymentGateways/WCGatewayMockPassword.php
@@ -0,0 +1,56 @@
+<?php
+declare( strict_types=1 );
+
+namespace Automattic\WooCommerce\Tests\Internal\RestApi\Routes\V4\Settings\PaymentGateways;
+
+use WC_Payment_Gateway;
+
+/**
+ * Mock payment gateway with password-type form fields for testing.
+ */
+class WCGatewayMockPassword extends WC_Payment_Gateway {
+ /**
+ * Constructor.
+ */
+ public function __construct() {
+ $this->id = 'mock_password';
+ $this->method_title = 'Mock Password Gateway';
+ $this->method_description = 'A mock gateway for testing password field sanitization.';
+ $this->has_fields = false;
+
+ $this->init_form_fields();
+ $this->init_settings();
+ }
+
+ /**
+ * Initialize form fields with password-type fields.
+ */
+ public function init_form_fields() {
+ $this->form_fields = array(
+ 'enabled' => array(
+ 'title' => 'Enable/Disable',
+ 'type' => 'checkbox',
+ 'label' => 'Enable Mock Password Gateway',
+ 'default' => 'no',
+ ),
+ 'title' => array(
+ 'title' => 'Title',
+ 'type' => 'text',
+ 'default' => 'Mock Password Gateway',
+ ),
+ 'api_password' => array(
+ 'title' => 'API Password',
+ 'type' => 'password',
+ ),
+ );
+ }
+
+ /**
+ * Process payment — not used in tests.
+ *
+ * @param int $order_id Order ID.
+ */
+ public function process_payment( $order_id ) {
+ return array( 'result' => 'success' );
+ }
+}