Skip to content

Commit bc4d4bb

Browse files
superdav42vuckroclaude
authored
ci: fix PR #1382 E2E cleanup failure (#1413)
* fix(security): use a high-entropy random key for security-mode disable The unauthenticated ?wu_secure=KEY query string that turns the network-wide recovery "security mode" off used substr(md5(admin_email), 0, 6) as the key — only ~24 bits and derived from a commonly public/guessable value, so an attacker could compute or brute-force it and remotely disable the admin's safe-mode lockdown. Generate a 128-bit random key once (random_bytes, since this runs from sunrise before pluggable.php) and store it as a network option, and compare it with hash_equals(). The key is already displayed on the settings screen, so the documented "copy this URL to disable security mode" workflow is unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(security): preserve legacy security mode recovery URL * ci: guard e2e cleanup before checkout * fix: address security mode review feedback --------- Co-authored-by: vuckro <maribel_waters@howtocore.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 4c6ceed commit bc4d4bb

4 files changed

Lines changed: 104 additions & 12 deletions

File tree

.github/workflows/e2e.yml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ jobs:
1919

2020
services:
2121
mailpit:
22-
image: axllent/mailpit:latest
22+
image: ghcr.io/axllent/mailpit@sha256:3bd7c2f2696deb35a4780d152b404dceec99cb041b942c0877b3b22384714f85
2323
ports:
2424
- 1025:1025
2525
- 8025:8025
@@ -520,7 +520,12 @@ jobs:
520520
521521
- name: Fix permissions for Cypress output
522522
if: always()
523-
run: sudo chown -R $USER:$USER tests/e2e/cypress
523+
run: |
524+
if [ -d tests/e2e/cypress ]; then
525+
sudo chown -R $USER:$USER tests/e2e/cypress
526+
else
527+
echo "Cypress output directory does not exist; skipping permission fix."
528+
fi
524529
525530
- name: Upload Cypress screenshots
526531
if: always()
@@ -540,4 +545,9 @@ jobs:
540545

541546
- name: Stop WordPress Environment
542547
if: always()
543-
run: npm run env:stop
548+
run: |
549+
if [ -f package.json ]; then
550+
npm run env:stop
551+
else
552+
echo "package.json does not exist; checkout did not complete before cleanup."
553+
fi

inc/class-sunrise.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,9 @@ public static function load(): void {
335335
$security_mode = (bool) (int) wu_get_setting_early('security_mode');
336336

337337
if ($security_mode) {
338-
if (wu_get_isset($_GET, 'wu_secure') === wu_get_security_mode_key()) { // phpcs:ignore WordPress.Security.NonceVerification
338+
$provided_key = wu_get_isset($_GET, 'wu_secure'); // phpcs:ignore WordPress.Security.NonceVerification
339+
340+
if (is_string($provided_key) && hash_equals(wu_get_security_mode_key(false), $provided_key)) {
339341
wu_save_setting_early('security_mode', false);
340342
} else {
341343
/**

inc/functions/sunrise.php

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,56 @@ function wu_save_setting_early($key, $value) {
7272
}
7373

7474
/**
75-
* Get the security mode key used to disable security mode
75+
* Get the security mode key used to disable security mode.
76+
*
77+
* This key is exposed in an unauthenticated query string (?wu_secure=KEY) that
78+
* turns the network-wide recovery "security mode" off, so it must be
79+
* unpredictable. It used to be substr(md5(admin_email), 0, 6) — only ~24 bits
80+
* and derived from a frequently public/guessable value, which an attacker could
81+
* compute or brute-force. We now use a high-entropy random secret generated once
82+
* and stored as a network option when the key is displayed to admins.
83+
* random_bytes() is used (not wp_generate_password) because this runs from
84+
* sunrise, before pluggable.php is loaded. Sunrise validation does not generate
85+
* a new key while security mode is already active; if a random key has not been
86+
* persisted yet, the legacy derived key remains valid until an admin loads the
87+
* settings screen and sees the new random recovery URL.
7688
*
7789
* @since 2.0.20
90+
*
91+
* @param bool $generate Whether to generate and persist a random key when missing.
92+
* @return string
93+
*/
94+
function wu_get_security_mode_key($generate = true): string {
95+
96+
$key = (string) get_network_option(null, 'wu_security_mode_key', '');
97+
98+
if ('' === $key) {
99+
if (! $generate) {
100+
return wu_get_legacy_security_mode_key();
101+
}
102+
103+
$generated_key = bin2hex(random_bytes(16));
104+
105+
add_network_option(null, 'wu_security_mode_key', $generated_key);
106+
107+
$key = (string) get_network_option(null, 'wu_security_mode_key', '');
108+
109+
if ('' === $key) {
110+
return wu_get_legacy_security_mode_key();
111+
}
112+
}
113+
114+
return $key;
115+
}
116+
117+
/**
118+
* Get the legacy security mode key used before high-entropy keys were persisted.
119+
*
120+
* @since 2.0.20
121+
*
122+
* @return string
78123
*/
79-
function wu_get_security_mode_key(): string {
124+
function wu_get_legacy_security_mode_key(): string {
80125

81126
$hash = md5((string) get_network_option(null, 'admin_email'));
82127

tests/WP_Ultimo/Functions/Sunrise_Functions_Test.php

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,37 +101,72 @@ public function test_save_setting_early_stores_value(): void {
101101
}
102102

103103
/**
104-
* Test wu_get_security_mode_key returns a 6-character string.
104+
* Test wu_get_security_mode_key returns a high-entropy string.
105105
*/
106-
public function test_get_security_mode_key_returns_six_char_string(): void {
106+
public function test_get_security_mode_key_returns_high_entropy_string(): void {
107+
108+
delete_network_option(null, 'wu_security_mode_key');
107109

108110
$key = wu_get_security_mode_key();
109111

110112
$this->assertIsString($key);
111-
$this->assertSame(6, strlen($key));
113+
$this->assertSame(32, strlen($key));
112114
}
113115

114116
/**
115117
* Test wu_get_security_mode_key returns only hex characters.
116118
*/
117119
public function test_get_security_mode_key_returns_hex_characters(): void {
118120

121+
delete_network_option(null, 'wu_security_mode_key');
122+
119123
$key = wu_get_security_mode_key();
120124

121-
$this->assertMatchesRegularExpression('/^[0-9a-f]{6}$/', $key);
125+
$this->assertMatchesRegularExpression('/^[0-9a-f]{32}$/', $key);
122126
}
123127

124128
/**
125-
* Test wu_get_security_mode_key is deterministic for same admin email.
129+
* Test wu_get_security_mode_key is stable after generation.
126130
*/
127-
public function test_get_security_mode_key_is_deterministic(): void {
131+
public function test_get_security_mode_key_is_stable_after_generation(): void {
132+
133+
delete_network_option(null, 'wu_security_mode_key');
128134

129135
$key1 = wu_get_security_mode_key();
130136
$key2 = wu_get_security_mode_key();
131137

132138
$this->assertSame($key1, $key2);
133139
}
134140

141+
/**
142+
* Test wu_get_security_mode_key without generation returns a stored key.
143+
*/
144+
public function test_get_security_mode_key_without_generation_returns_persisted_key_when_available(): void {
145+
146+
delete_network_option(null, 'wu_security_mode_key');
147+
148+
$generated = wu_get_security_mode_key();
149+
$stored = get_network_option(null, 'wu_security_mode_key', '');
150+
151+
$this->assertSame($generated, $stored);
152+
$this->assertSame($stored, wu_get_security_mode_key(false));
153+
$this->assertMatchesRegularExpression('/^[0-9a-f]{32}$/', $stored);
154+
}
155+
156+
/**
157+
* Test wu_get_security_mode_key can preserve the legacy key without rotating.
158+
*/
159+
public function test_get_security_mode_key_without_generation_returns_legacy_key(): void {
160+
161+
delete_network_option(null, 'wu_security_mode_key');
162+
163+
$expected = substr(md5((string) get_network_option(null, 'admin_email')), 0, 6);
164+
$key = wu_get_security_mode_key(false);
165+
166+
$this->assertSame($expected, $key);
167+
$this->assertSame('', get_network_option(null, 'wu_security_mode_key', ''));
168+
}
169+
135170
/**
136171
* Test wu_kses_data returns string.
137172
*/

0 commit comments

Comments
 (0)