Skip to content

Commit e8a6f23

Browse files
Copilotswissspidy
andcommitted
Address code review feedback - improve security and code quality
- Properly escape null device path in shell commands to prevent injection - Remove error suppression operator and add explicit error logging - Use more precise phpcs exclude patterns instead of wildcards Co-authored-by: swissspidy <[email protected]>
1 parent 61d08c9 commit e8a6f23

File tree

2 files changed

+19
-8
lines changed

2 files changed

+19
-8
lines changed

phpcs.xml.dist

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,12 @@
5959
<rule ref="WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedNamespaceFound">
6060
<exclude-pattern>*/src/WP_CLI/Fetchers/(Plugin|Theme)\.php$</exclude-pattern>
6161
<exclude-pattern>*/src/WP_CLI/CommandWithUpgrade\.php$</exclude-pattern>
62-
<exclude-pattern>*/src/WP_CLI/(CommandWith|DestructivePlugin|DestructiveTheme|ValidatingPlugin|ValidatingTheme|PackageValidator|UpgraderWithValidation).*\.php$</exclude-pattern>
62+
<exclude-pattern>*/src/WP_CLI/DestructivePluginUpgrader\.php$</exclude-pattern>
63+
<exclude-pattern>*/src/WP_CLI/DestructiveThemeUpgrader\.php$</exclude-pattern>
64+
<exclude-pattern>*/src/WP_CLI/ValidatingPluginUpgrader\.php$</exclude-pattern>
65+
<exclude-pattern>*/src/WP_CLI/ValidatingThemeUpgrader\.php$</exclude-pattern>
66+
<exclude-pattern>*/src/WP_CLI/PackageValidator\.php$</exclude-pattern>
67+
<exclude-pattern>*/src/WP_CLI/UpgraderWithValidation\.php$</exclude-pattern>
6368
<exclude-pattern>*/src/WP_CLI/Parse(Plugin|Theme)NameInput\.php$</exclude-pattern>
6469
</rule>
6570

src/WP_CLI/PackageValidator.php

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ private static function is_unzip_available() {
7878
// Suppress output to avoid cluttering the console.
7979
$null_device = '\\' === DIRECTORY_SEPARATOR ? 'NUL' : '/dev/null';
8080
$result = WP_CLI::launch(
81-
sprintf( 'unzip -v > %s 2>&1', $null_device ),
81+
'unzip -v > ' . escapeshellarg( $null_device ) . ' 2>&1',
8282
false,
8383
true
8484
);
@@ -97,11 +97,7 @@ private static function is_unzip_available() {
9797
private static function validate_with_unzip( $file_path ) {
9898
// Suppress output - use platform-appropriate null device.
9999
$null_device = '\\' === DIRECTORY_SEPARATOR ? 'NUL' : '/dev/null';
100-
$command = sprintf(
101-
'unzip -t %s > %s 2>&1',
102-
escapeshellarg( $file_path ),
103-
$null_device
104-
);
100+
$command = 'unzip -t ' . escapeshellarg( $file_path ) . ' > ' . escapeshellarg( $null_device ) . ' 2>&1';
105101

106102
$result = WP_CLI::launch(
107103
$command,
@@ -130,6 +126,16 @@ public static function delete_corrupted_file( $file_path ) {
130126
return true;
131127
}
132128

133-
return @unlink( $file_path );
129+
$result = unlink( $file_path );
130+
131+
// Log if deletion failed, but don't throw an error.
132+
if ( ! $result ) {
133+
WP_CLI::debug(
134+
sprintf( 'Failed to delete corrupted file: %s', $file_path ),
135+
'extension-command'
136+
);
137+
}
138+
139+
return $result;
134140
}
135141
}

0 commit comments

Comments
 (0)