Skip to content

Commit 63d4c9e

Browse files
author
Felix Arntz
committed
Revise implementation to use collection roots only as a denylist, failing for exact matches and parent directories.
1 parent c717430 commit 63d4c9e

File tree

2 files changed

+120
-39
lines changed

2 files changed

+120
-39
lines changed

‎src/wp-includes/class-wp-block-metadata-registry.php‎

Lines changed: 40 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class WP_Block_Metadata_Registry {
4343
* @since 6.8.0
4444
* @var string[]|null
4545
*/
46-
private static $default_allowed_collection_roots = null;
46+
private static $default_collection_roots = null;
4747

4848
/**
4949
* Registers a block metadata collection.
@@ -84,42 +84,48 @@ class WP_Block_Metadata_Registry {
8484
public static function register_collection( $path, $manifest ) {
8585
$path = wp_normalize_path( rtrim( $path, '/' ) );
8686

87-
$allowed_collection_roots = self::get_default_allowed_collection_roots();
87+
$collection_roots = self::get_default_collection_roots();
8888

8989
/**
90-
* Filters in which root directory paths block metadata collections are allowed.
90+
* Filters the root directory paths for block metadata collections.
9191
*
92-
* The default list encompasses the `wp-includes` directory, as well as the root directories for plugins,
93-
* must-use plugins, and themes. This filter can be used to expand the list, e.g. to custom directories with
94-
* symlinked plugins.
92+
* Any block metadata collection that is registered must not use any of these paths, or any parent directory
93+
* path of them. Most commonly, block metadata collections should reside within one of these paths, though in
94+
* some scenarios they may also reside in entirely different directories (e.g. in case of symlinked plugins).
95+
*
96+
* Example:
97+
* * It is allowed to register a collection with path `WP_PLUGIN_DIR . '/my-plugin'`.
98+
* * It is not allowed to register a collection with path `WP_PLUGIN_DIR`.
99+
* * It is not allowed to register a collection with path `dirname( WP_PLUGIN_DIR )`.
95100
*
96-
* Any block metadata collection that is registered must be within one of these directories. It must however
97-
* not match any of these directories exactly, as then the collection may conflict with another one within the
98-
* same root.
101+
* The default list encompasses the `wp-includes` directory, as well as the root directories for plugins,
102+
* must-use plugins, and themes. This filter can be used to expand the list, e.g. to custom directories that
103+
* contain symlinked plugins, so that these root directories cannot be used themselves for a block metadata
104+
* collection either.
99105
*
100106
* @since 6.8.0
101107
*
102-
* @param string[] $allowed_collection_roots List of allowed metadata collection root paths.
108+
* @param string[] $collection_roots List of allowed metadata collection root paths.
103109
*/
104-
$allowed_collection_roots = apply_filters( 'wp_allowed_block_metadata_collection_roots', $allowed_collection_roots );
110+
$collection_roots = apply_filters( 'wp_allowed_block_metadata_collection_roots', $collection_roots );
105111

106-
$allowed_collection_roots = array_unique(
112+
$collection_roots = array_unique(
107113
array_map(
108114
static function ( $allowed_root ) {
109115
return rtrim( $allowed_root, '/' );
110116
},
111-
$allowed_collection_roots
117+
$collection_roots
112118
)
113119
);
114120

115121
// Check if the path is valid:
116-
if ( ! self::is_valid_collection_path( $path, $allowed_collection_roots ) ) {
122+
if ( ! self::is_valid_collection_path( $path, $collection_roots ) ) {
117123
_doing_it_wrong(
118124
__METHOD__,
119125
sprintf(
120126
/* translators: %s: list of allowed collection roots */
121127
__( 'Block metadata collections can only be registered within one of the following directories: %s' ),
122-
esc_html( implode( wp_get_list_item_separator(), $allowed_collection_roots ) )
128+
esc_html( implode( wp_get_list_item_separator(), $collection_roots ) )
123129
),
124130
'6.8.0'
125131
);
@@ -251,46 +257,45 @@ private static function default_identifier_callback( $path ) {
251257
}
252258

253259
/**
254-
* Checks whether the given block metadata collection path is valid against the list of allowed collection roots.
260+
* Checks whether the given block metadata collection path is valid against the list of collection roots.
255261
*
256262
* @since 6.8.0
257263
*
258-
* @param string $path Block metadata collection path, without trailing slash.
259-
* @param string[] $allowed_collection_roots List of allowed collection root paths, without trailing slashes.
264+
* @param string $path Block metadata collection path, without trailing slash.
265+
* @param string[] $collection_roots List of collection root paths, without trailing slashes.
260266
* @return bool True if the path is allowed, false otherwise.
261267
*/
262-
private static function is_valid_collection_path( $path, $allowed_collection_roots ) {
263-
$matching_root_found = false;
264-
265-
foreach ( $allowed_collection_roots as $allowed_root ) {
266-
// If the path matches any allowed root exactly, it is invalid.
268+
private static function is_valid_collection_path( $path, $collection_roots ) {
269+
foreach ( $collection_roots as $allowed_root ) {
270+
// If the path matches any root exactly, it is invalid.
267271
if ( $allowed_root === $path ) {
268272
return false;
269273
}
270274

271-
// Otherwise, if the path is within any of the allowed roots, it is valid.
272-
if ( str_starts_with( $path, $allowed_root ) ) {
273-
$matching_root_found = true;
275+
// If the path is a parent path of any of the roots, it is invalid.
276+
if ( str_starts_with( $allowed_root, $path ) ) {
277+
return false;
274278
}
275279
}
276280

277-
return $matching_root_found;
281+
return true;
278282
}
279283

280284
/**
281-
* Gets the default allowed collection root paths.
285+
* Gets the default collection root directory paths.
282286
*
283287
* @since 6.8.0
284288
*
285289
* @return string[] List of directory paths within which metadata collections are allowed.
286290
*/
287-
private static function get_default_allowed_collection_roots() {
288-
if ( isset( self::$default_allowed_collection_roots ) ) {
289-
return self::$default_allowed_collection_roots;
291+
private static function get_default_collection_roots() {
292+
if ( isset( self::$default_collection_roots ) ) {
293+
return self::$default_collection_roots;
290294
}
291295

292-
$allowed_collection_roots = array(
296+
$collection_roots = array(
293297
wp_normalize_path( ABSPATH . WPINC ),
298+
wp_normalize_path( WP_CONTENT_DIR ),
294299
wp_normalize_path( WPMU_PLUGIN_DIR ),
295300
wp_normalize_path( WP_PLUGIN_DIR ),
296301
);
@@ -300,10 +305,10 @@ private static function get_default_allowed_collection_roots() {
300305
$theme_roots = array( $theme_roots );
301306
}
302307
foreach ( $theme_roots as $theme_root ) {
303-
$allowed_collection_roots[] = trailingslashit( wp_normalize_path( WP_CONTENT_DIR ) ) . ltrim( wp_normalize_path( $theme_root ), '/' );
308+
$collection_roots[] = trailingslashit( wp_normalize_path( WP_CONTENT_DIR ) ) . ltrim( wp_normalize_path( $theme_root ), '/' );
304309
}
305310

306-
self::$default_allowed_collection_roots = array_unique( $allowed_collection_roots );
307-
return self::$default_allowed_collection_roots;
311+
self::$default_collection_roots = array_unique( $collection_roots );
312+
return self::$default_collection_roots;
308313
}
309314
}

‎tests/phpunit/tests/blocks/wpBlockMetadataRegistry.php‎

Lines changed: 80 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,88 @@ public function test_register_collection_with_invalid_plugin_path() {
8080
$this->assertFalse( $result, 'Invalid plugin path should not be registered' );
8181
}
8282

83-
public function test_register_collection_with_non_existent_path() {
84-
$non_existent_path = '/path/that/does/not/exist';
83+
public function test_register_collection_with_valid_muplugin_path() {
84+
$plugin_path = WPMU_PLUGIN_DIR . '/my-plugin/blocks';
85+
$result = WP_Block_Metadata_Registry::register_collection( $plugin_path, $this->temp_manifest_file );
86+
$this->assertTrue( $result, 'Valid must-use plugin path should be registered successfully' );
87+
}
88+
89+
public function test_register_collection_with_invalid_muplugin_path() {
90+
$invalid_plugin_path = WPMU_PLUGIN_DIR;
91+
92+
$this->setExpectedIncorrectUsage( 'WP_Block_Metadata_Registry::register_collection' );
93+
94+
$result = WP_Block_Metadata_Registry::register_collection( $invalid_plugin_path, $this->temp_manifest_file );
95+
$this->assertFalse( $result, 'Invalid must-use plugin path should not be registered' );
96+
}
97+
98+
public function test_register_collection_with_valid_theme_path() {
99+
$theme_path = WP_CONTENT_DIR . '/themes/my-theme/blocks';
100+
$result = WP_Block_Metadata_Registry::register_collection( $theme_path, $this->temp_manifest_file );
101+
$this->assertTrue( $result, 'Valid theme path should be registered successfully' );
102+
}
103+
104+
public function test_register_collection_with_invalid_theme_path() {
105+
$invalid_theme_path = WP_CONTENT_DIR . '/themes';
106+
107+
$this->setExpectedIncorrectUsage( 'WP_Block_Metadata_Registry::register_collection' );
108+
109+
$result = WP_Block_Metadata_Registry::register_collection( $invalid_theme_path, $this->temp_manifest_file );
110+
$this->assertFalse( $result, 'Invalid theme path should not be registered' );
111+
}
112+
113+
public function test_register_collection_with_arbitrary_path() {
114+
$arbitrary_path = '/var/arbitrary/path';
115+
$result = WP_Block_Metadata_Registry::register_collection( $arbitrary_path, $this->temp_manifest_file );
116+
$this->assertTrue( $result, 'Arbitrary path should be registered successfully' );
117+
}
118+
119+
public function test_register_collection_with_arbitrary_path_and_collection_roots_filter() {
120+
$arbitrary_path = '/var/arbitrary/path';
121+
add_filter(
122+
'wp_allowed_block_metadata_collection_roots',
123+
static function ( $paths ) use ( $arbitrary_path ) {
124+
$paths[] = $arbitrary_path;
125+
return $paths;
126+
}
127+
);
128+
129+
$this->setExpectedIncorrectUsage( 'WP_Block_Metadata_Registry::register_collection' );
130+
131+
$result = WP_Block_Metadata_Registry::register_collection( $arbitrary_path, $this->temp_manifest_file );
132+
$this->assertFalse( $result, 'Arbitrary path should not be registered if it matches a collection root' );
133+
134+
$result = WP_Block_Metadata_Registry::register_collection( dirname( $arbitrary_path ), $this->temp_manifest_file );
135+
$this->assertFalse( $result, 'Arbitrary path should not be registered if it is a parent directory of a collection root' );
136+
137+
$result = WP_Block_Metadata_Registry::register_collection( $arbitrary_path . '/my-plugin/blocks', $this->temp_manifest_file );
138+
$this->assertTrue( $result, 'Arbitrary path should be registered successfully if it is within a collection root' );
139+
}
140+
141+
public function test_register_collection_with_wp_content_parent_directory_path() {
142+
$invalid_path = dirname( WP_CONTENT_DIR );
143+
144+
$this->setExpectedIncorrectUsage( 'WP_Block_Metadata_Registry::register_collection' );
145+
146+
$result = WP_Block_Metadata_Registry::register_collection( $invalid_path, $this->temp_manifest_file );
147+
$this->assertFalse( $result, 'Invalid path (parent directory of "wp-content") should not be registered' );
148+
}
149+
150+
public function test_register_collection_with_wp_includes_parent_directory_path() {
151+
$invalid_path = ABSPATH;
152+
153+
$this->setExpectedIncorrectUsage( 'WP_Block_Metadata_Registry::register_collection' );
154+
155+
$result = WP_Block_Metadata_Registry::register_collection( $invalid_path, $this->temp_manifest_file );
156+
$this->assertFalse( $result, 'Invalid path (parent directory of "wp-includes") should not be registered' );
157+
}
158+
159+
public function test_register_collection_with_non_existent_manifest() {
160+
$non_existent_manifest = '/path/that/does/not/exist/block-manifest.php';
85161

86162
$this->setExpectedIncorrectUsage( 'WP_Block_Metadata_Registry::register_collection' );
87163

88-
$result = WP_Block_Metadata_Registry::register_collection( $non_existent_path, $this->temp_manifest_file );
89-
$this->assertFalse( $result, 'Non-existent path should not be registered' );
164+
$result = WP_Block_Metadata_Registry::register_collection( '/var/arbitrary/path', $non_existent_manifest );
165+
$this->assertFalse( $result, 'Non-existent manifest should not be registered' );
90166
}
91167
}

0 commit comments

Comments
 (0)