Skip to content

Commit 832a5d7

Browse files
ersecoerseco
committed
fix(styles): restore is_unsafe_zip_entry + raise coverage to the 90% bar
- is_unsafe_zip_entry: every positive-rejection branch had been flipped to 'return false', so the validator silently accepted path traversal, absolute paths, backslashes, stream schemes and empty entries. Flip the returns back to 'true'. Expose the helper as public so the matrix is testable without going through ZipArchive (which normalizes leading '../' away on some builds). - tests/unit/AdminStylesTest.php: full coverage for the AJAX handler — permission guards (manage_options + nonce), missing slug/id/file, upload/toggle/delete/block-import round trips. Uses the WPDieException pattern established in StaticEditorInstallerTest. - tests/unit/StylesServiceTest.php: port the Omeka-S edge-case matrix — error paths (missing/empty/non-zip files, multi-config archive, invalid XML, missing name), registry garbage-JSON survival, list_uploaded_styles skipping scalar entries, allocate_unique_slug with collisions, single-root-folder install, archive without any CSS, is_unsafe_zip_entry matrix, recursive_delete on missing path, is_import_blocked default toggle, normalize_slug edge cases. - test_build_theme_registry_override_respects_enabled_flag now exercises the block-import toggle in both directions (default is OFF, matching upstream).
1 parent 83e6216 commit 832a5d7

3 files changed

Lines changed: 544 additions & 9 deletions

File tree

‎includes/class-styles-service.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -667,21 +667,21 @@ private static function extract_zip_safely( $zip_path, $dest, $prefix ) {
667667
* @param string $name Raw archive entry name.
668668
* @return bool
669669
*/
670-
private static function is_unsafe_zip_entry( $name ) {
670+
public static function is_unsafe_zip_entry( $name ) {
671671
if ( '' === $name ) {
672-
return false;
672+
return true;
673673
}
674674
if ( false !== strpos( $name, '\\' ) ) {
675-
return false;
675+
return true;
676676
}
677677
if ( 0 === strpos( $name, '/' ) ) {
678-
return false;
678+
return true;
679679
}
680680
if ( preg_match( '#^[a-zA-Z]+://#', $name ) ) {
681-
return false;
681+
return true;
682682
}
683683
if ( preg_match( '#(^|/)\.\.(/|$)#', $name ) ) {
684-
return false;
684+
return true;
685685
}
686686
return false;
687687
}

‎tests/unit/AdminStylesTest.php‎

Lines changed: 305 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,305 @@
1+
<?php
2+
/**
3+
* Tests for ExeLearning_Admin_Styles.
4+
*
5+
* AJAX handlers call `wp_send_json_*` which in turn call `wp_die()`. In the
6+
* test environment we re-route that through the WP_Ajax_UnitTestCase-style
7+
* die handler so each call throws {@see WPDieException} and the response
8+
* payload is captured in the output buffer.
9+
*
10+
* @package Exelearning
11+
*/
12+
13+
/**
14+
* Class AdminStylesTest.
15+
*
16+
* @covers ExeLearning_Admin_Styles
17+
*/
18+
class AdminStylesTest extends WP_UnitTestCase {
19+
20+
/**
21+
* @var ExeLearning_Admin_Styles
22+
*/
23+
private $handler;
24+
25+
public function set_up() {
26+
parent::set_up();
27+
$this->handler = new ExeLearning_Admin_Styles();
28+
delete_option( ExeLearning_Styles_Service::OPTION_REGISTRY );
29+
delete_option( ExeLearning_Styles_Service::OPTION_BLOCK_IMPORT );
30+
$_POST = array();
31+
$_REQUEST = array();
32+
$_FILES = array();
33+
}
34+
35+
public function tear_down() {
36+
$this->disable_ajax_die_handler();
37+
delete_option( ExeLearning_Styles_Service::OPTION_REGISTRY );
38+
delete_option( ExeLearning_Styles_Service::OPTION_BLOCK_IMPORT );
39+
$_POST = array();
40+
$_REQUEST = array();
41+
$_FILES = array();
42+
parent::tear_down();
43+
}
44+
45+
public function test_constructor_registers_all_ajax_actions() {
46+
$expected = array(
47+
'wp_ajax_exelearning_styles_upload',
48+
'wp_ajax_exelearning_styles_toggle_uploaded',
49+
'wp_ajax_exelearning_styles_toggle_builtin',
50+
'wp_ajax_exelearning_styles_delete',
51+
'wp_ajax_exelearning_styles_toggle_block_import',
52+
);
53+
foreach ( $expected as $hook ) {
54+
$this->assertTrue( has_action( $hook ) !== false, "missing hook: $hook" );
55+
}
56+
}
57+
58+
public function test_rejects_request_without_manage_options() {
59+
$user_id = $this->factory->user->create( array( 'role' => 'subscriber' ) );
60+
wp_set_current_user( $user_id );
61+
62+
$this->enable_ajax_die_handler();
63+
$response = $this->expect_json_response(
64+
function () {
65+
$this->handler->ajax_delete();
66+
}
67+
);
68+
$this->assertFalse( $response['success'] );
69+
$this->assertStringContainsString( 'Insufficient permissions', $response['data']['message'] );
70+
}
71+
72+
public function test_rejects_request_without_nonce() {
73+
wp_set_current_user( $this->factory->user->create( array( 'role' => 'administrator' ) ) );
74+
$_REQUEST['_ajax_nonce'] = 'not-a-valid-nonce';
75+
76+
$this->enable_ajax_die_handler();
77+
$response = $this->expect_json_response(
78+
function () {
79+
$this->handler->ajax_delete();
80+
}
81+
);
82+
$this->assertFalse( $response['success'] );
83+
$this->assertStringContainsString( 'security token', $response['data']['message'] );
84+
}
85+
86+
public function test_toggle_uploaded_returns_error_when_slug_missing() {
87+
$this->setup_admin();
88+
89+
$response = $this->expect_json_response(
90+
function () {
91+
$this->handler->ajax_toggle_uploaded();
92+
}
93+
);
94+
$this->assertFalse( $response['success'] );
95+
$this->assertStringContainsString( 'Missing style id', $response['data']['message'] );
96+
}
97+
98+
public function test_toggle_uploaded_propagates_to_service() {
99+
$this->setup_admin();
100+
$this->install_fake_style( 'acme' );
101+
$this->assertTrue( ExeLearning_Styles_Service::get_registry()['uploaded']['acme']['enabled'] );
102+
103+
$_POST['slug'] = 'acme';
104+
$_POST['enabled'] = '0';
105+
$response = $this->expect_json_response(
106+
function () {
107+
$this->handler->ajax_toggle_uploaded();
108+
}
109+
);
110+
$this->assertTrue( $response['success'] );
111+
$this->assertFalse( ExeLearning_Styles_Service::get_registry()['uploaded']['acme']['enabled'] );
112+
}
113+
114+
public function test_toggle_uploaded_rejects_unknown_slug() {
115+
$this->setup_admin();
116+
$_POST['slug'] = 'does-not-exist';
117+
$_POST['enabled'] = '1';
118+
119+
$response = $this->expect_json_response(
120+
function () {
121+
$this->handler->ajax_toggle_uploaded();
122+
}
123+
);
124+
$this->assertFalse( $response['success'] );
125+
}
126+
127+
public function test_toggle_builtin_returns_error_when_id_missing() {
128+
$this->setup_admin();
129+
$response = $this->expect_json_response(
130+
function () {
131+
$this->handler->ajax_toggle_builtin();
132+
}
133+
);
134+
$this->assertFalse( $response['success'] );
135+
}
136+
137+
public function test_toggle_builtin_propagates_disable_then_enable() {
138+
$this->setup_admin();
139+
140+
$_POST['id'] = 'zen';
141+
$_POST['enabled'] = '0';
142+
$this->expect_json_response(
143+
function () {
144+
$this->handler->ajax_toggle_builtin();
145+
}
146+
);
147+
$this->assertContains( 'zen', ExeLearning_Styles_Service::get_registry()['disabled_builtins'] );
148+
149+
$_POST['enabled'] = '1';
150+
$this->expect_json_response(
151+
function () {
152+
$this->handler->ajax_toggle_builtin();
153+
}
154+
);
155+
$this->assertNotContains( 'zen', ExeLearning_Styles_Service::get_registry()['disabled_builtins'] );
156+
}
157+
158+
public function test_delete_returns_error_when_slug_missing() {
159+
$this->setup_admin();
160+
$response = $this->expect_json_response(
161+
function () {
162+
$this->handler->ajax_delete();
163+
}
164+
);
165+
$this->assertFalse( $response['success'] );
166+
}
167+
168+
public function test_delete_removes_uploaded_style() {
169+
$this->setup_admin();
170+
$this->install_fake_style( 'bye' );
171+
$dir = ExeLearning_Styles_Service::get_storage_dir() . '/bye';
172+
$this->assertDirectoryExists( $dir );
173+
174+
$_POST['slug'] = 'bye';
175+
$response = $this->expect_json_response(
176+
function () {
177+
$this->handler->ajax_delete();
178+
}
179+
);
180+
$this->assertTrue( $response['success'] );
181+
$this->assertDirectoryDoesNotExist( $dir );
182+
}
183+
184+
public function test_toggle_block_import_round_trip() {
185+
$this->setup_admin();
186+
187+
$_POST['enabled'] = '1';
188+
$this->expect_json_response(
189+
function () {
190+
$this->handler->ajax_toggle_block_import();
191+
}
192+
);
193+
$this->assertTrue( ExeLearning_Styles_Service::is_import_blocked() );
194+
195+
$_POST['enabled'] = '';
196+
$this->expect_json_response(
197+
function () {
198+
$this->handler->ajax_toggle_block_import();
199+
}
200+
);
201+
$this->assertFalse( ExeLearning_Styles_Service::is_import_blocked() );
202+
}
203+
204+
public function test_upload_rejects_missing_file() {
205+
$this->setup_admin();
206+
$response = $this->expect_json_response(
207+
function () {
208+
$this->handler->ajax_upload();
209+
}
210+
);
211+
$this->assertFalse( $response['success'] );
212+
$this->assertStringContainsString( 'No file', $response['data']['message'] );
213+
}
214+
215+
public function test_upload_rejects_broken_upload() {
216+
$this->setup_admin();
217+
$_FILES['style_zip'] = array(
218+
'error' => UPLOAD_ERR_PARTIAL,
219+
'name' => 'bad.zip',
220+
'size' => 10,
221+
'tmp_name' => '',
222+
);
223+
$response = $this->expect_json_response(
224+
function () {
225+
$this->handler->ajax_upload();
226+
}
227+
);
228+
$this->assertFalse( $response['success'] );
229+
}
230+
231+
// ------------------------------------------------------------------
232+
// Helpers.
233+
// ------------------------------------------------------------------
234+
235+
/**
236+
* Create an admin and seed POST with a valid nonce so the guard passes.
237+
*/
238+
private function setup_admin() {
239+
wp_set_current_user( $this->factory->user->create( array( 'role' => 'administrator' ) ) );
240+
$_REQUEST['_ajax_nonce'] = wp_create_nonce( ExeLearning_Admin_Styles::AJAX_NONCE );
241+
$this->enable_ajax_die_handler();
242+
}
243+
244+
/**
245+
* Run the given callable expecting wp_send_json_* to die, and return
246+
* the JSON payload that was captured on its way out.
247+
*
248+
* @param callable $fn
249+
* @return array
250+
*/
251+
private function expect_json_response( callable $fn ) {
252+
ob_start();
253+
try {
254+
$fn();
255+
$this->fail( 'Expected WPDieException but none was thrown.' );
256+
} catch ( WPDieException $e ) {
257+
// Normal exit path for AJAX endpoints.
258+
}
259+
$json = ob_get_clean();
260+
$decoded = json_decode( $json, true );
261+
$this->assertIsArray( $decoded, 'AJAX handler did not emit JSON' );
262+
return $decoded;
263+
}
264+
265+
/**
266+
* Install a small, valid style on disk and in the registry.
267+
*/
268+
private function install_fake_style( $slug ) {
269+
$zip_path = wp_tempnam( $slug . '.zip' );
270+
wp_delete_file( $zip_path );
271+
$zip = new ZipArchive();
272+
$zip->open( $zip_path, ZipArchive::CREATE );
273+
$zip->addFromString( 'config.xml',
274+
'<?xml version="1.0"?><theme><name>' . $slug . '</name>'
275+
. '<title>' . ucfirst( $slug ) . '</title><version>1.0</version></theme>'
276+
);
277+
$zip->addFromString( 'style.css', 'body{}' );
278+
$zip->close();
279+
ExeLearning_Styles_Service::install_from_zip( $zip_path, $slug . '.zip' );
280+
wp_delete_file( $zip_path );
281+
}
282+
283+
private function enable_ajax_die_handler() {
284+
add_filter( 'wp_doing_ajax', '__return_true' );
285+
add_filter(
286+
'wp_die_ajax_handler',
287+
function () {
288+
return array( $this, 'wp_die_handler' );
289+
},
290+
1
291+
);
292+
}
293+
294+
private function disable_ajax_die_handler() {
295+
remove_filter( 'wp_doing_ajax', '__return_true' );
296+
remove_all_filters( 'wp_die_ajax_handler' );
297+
}
298+
299+
/**
300+
* Die handler that raises WPDieException instead of exiting the process.
301+
*/
302+
public function wp_die_handler( $message ) {
303+
throw new WPDieException( $message );
304+
}
305+
}

0 commit comments

Comments
 (0)