Skip to content

Commit f104e30

Browse files
swissspidyclaude
andauthored
Utils: Add make_temp_file() and make_temp_dir() helper functions (#6374)
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 97e0ac2 commit f104e30

5 files changed

Lines changed: 229 additions & 20 deletions

File tree

‎php/WP_CLI/Extractor.php‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,7 @@ private static function extract_zip( $zipfile, $dest ) {
6363
$res = $zip->open( $zipfile );
6464

6565
if ( true === $res ) {
66-
$name = Path::basename( $zipfile );
67-
$tempdir = Utils\get_temp_dir()
68-
. uniqid( 'wp-cli-extract-zipfile-', true )
69-
. "-{$name}";
66+
$tempdir = Utils\make_temp_dir( 'wp-cli-extract-zipfile-' );
7067

7168
$zip->extractTo( $tempdir );
7269
$zip->close();
@@ -147,10 +144,7 @@ private static function extract_tarball( $tarball, $dest ) {
147144
$phar_error = null;
148145

149146
if ( class_exists( 'PharData' ) ) {
150-
$name = Path::basename( $tarball );
151-
$tempdir = Utils\get_temp_dir()
152-
. uniqid( 'wp-cli-extract-tarball-', true )
153-
. "-{$name}";
147+
$tempdir = Utils\make_temp_dir( 'wp-cli-extract-tarball-' );
154148

155149
try {
156150
$phar = new PharData( $tarball );
@@ -357,6 +351,7 @@ public static function tar_error_msg( $process_run ) {
357351
*/
358352
private static function get_first_subfolder( $path ) {
359353
$iterator = new DirectoryIterator( $path );
354+
$path = rtrim( $path, '/\\' );
360355

361356
foreach ( $iterator as $fileinfo ) {
362357
if ( $fileinfo->isDir() && ! $fileinfo->isDot() ) {

‎php/commands/src/CLI_Command.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ public function update( $args, $assoc_args ) {
498498

499499
WP_CLI::log( sprintf( 'Downloading from %s...', $download_url ) );
500500

501-
$temp = Utils\get_temp_dir() . uniqid( 'wp_', true ) . '.phar';
501+
$temp = Utils\make_temp_file( 'wp_', '.phar' );
502502

503503
$headers = [];
504504
$options = [

‎php/utils.php‎

Lines changed: 106 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ function extract_from_phar( $path ) {
7979

8080
$fname = Path::basename( $path );
8181

82-
$tmp_path = get_temp_dir() . uniqid( 'wp-cli-extract-from-phar-', true ) . "-$fname";
82+
$tmp_path = make_temp_file( 'wp-cli-extract-from-phar-', "-$fname" );
8383

8484
copy( $path, $tmp_path );
8585

@@ -599,28 +599,39 @@ function launch_editor_for_input( $input, $title = 'WP-CLI', $ext = 'tmp' ) {
599599

600600
$tmpdir = get_temp_dir();
601601

602+
$created = false;
603+
$attempts = 0;
604+
602605
do {
603606
$tmpfile = Path::basename( $title );
604607
$tmpfile = preg_replace( '|\.[^.]*$|', '', $tmpfile );
605608
$tmpfile .= '-' . substr( md5( (string) mt_rand() ), 0, 6 ); // phpcs:ignore WordPress.WP.AlternativeFunctions.rand_mt_rand -- no crypto and WP not loaded.
606609
$tmpfile = $tmpdir . $tmpfile . '.' . $ext;
607-
$fp = fopen( $tmpfile, 'xb' );
608-
if ( ! $fp && is_writable( $tmpdir ) && file_exists( $tmpfile ) ) {
609-
$tmpfile = '';
610-
continue;
611-
}
610+
611+
// Restrict the file to its owner, as it can hold sensitive input.
612+
$old_umask = umask( 0177 );
613+
$fp = @fopen( $tmpfile, 'xb' );
614+
umask( $old_umask );
615+
612616
if ( $fp ) {
617+
$created = true;
618+
// A short write (e.g. on a full disk) must not leave a truncated file for the editor to open.
619+
if ( strlen( $input ) !== fwrite( $fp, $input ) ) {
620+
fclose( $fp );
621+
@unlink( $tmpfile );
622+
WP_CLI::error( 'Error writing to temporary file.' );
623+
}
613624
fclose( $fp );
625+
break;
614626
}
615-
} while ( ! $tmpfile );
627+
$tmpfile = '';
628+
++$attempts;
629+
} while ( $attempts < 100 );
616630

617-
// @phpstan-ignore booleanNot.alwaysFalse
618-
if ( ! $tmpfile ) {
631+
if ( ! $created ) {
619632
WP_CLI::error( 'Error creating temporary file.' );
620633
}
621634

622-
file_put_contents( $tmpfile, $input );
623-
624635
$editor = getenv( 'EDITOR' );
625636
if ( ! $editor ) {
626637
$editor = is_windows() ? 'notepad' : 'vi';
@@ -1337,6 +1348,90 @@ function get_temp_dir() {
13371348
return $temp;
13381349
}
13391350

1351+
/**
1352+
* Create a unique temporary file safely without following symlinks.
1353+
*
1354+
* The file is only readable and writable by its owner on non-Windows systems.
1355+
*
1356+
* The file is created in the directory returned by WP-CLI's `get_temp_dir()`
1357+
* (based on `sys_get_temp_dir()`), not WordPress's `get_temp_dir()`, and is
1358+
* not removed automatically; callers are responsible for cleaning it up.
1359+
*
1360+
* On failure, this function exits via `WP_CLI::error()` rather than throwing.
1361+
*
1362+
* @access public
1363+
* @category System
1364+
*
1365+
* @param string $prefix Optional. Prefix for the temporary file name. Default 'wp-cli-'.
1366+
* @param string $suffix Optional. Suffix for the temporary file name. Default ''.
1367+
* @return string Path to the created temporary file.
1368+
*/
1369+
function make_temp_file( $prefix = 'wp-cli-', $suffix = '' ) {
1370+
if ( false !== strpbrk( $prefix, "/\\\0" ) || false !== strpbrk( $suffix, "/\\\0" ) ) {
1371+
WP_CLI::error( 'Invalid temporary file prefix or suffix.' );
1372+
}
1373+
1374+
$temp_dir = get_temp_dir();
1375+
$attempts = 0;
1376+
1377+
do {
1378+
$path = $temp_dir . uniqid( $prefix, true ) . $suffix;
1379+
1380+
// Restrict the file to its owner, as callers may write sensitive data to it.
1381+
$old_umask = umask( 0177 );
1382+
$handle = @fopen( $path, 'xb' );
1383+
umask( $old_umask );
1384+
1385+
++$attempts;
1386+
} while ( ! $handle && $attempts < 100 );
1387+
1388+
if ( ! $handle ) {
1389+
WP_CLI::error( 'Failed to create a temporary file.' );
1390+
}
1391+
1392+
fclose( $handle );
1393+
1394+
return $path;
1395+
}
1396+
1397+
/**
1398+
* Create a unique temporary directory safely without following symlinks.
1399+
*
1400+
* The directory is only accessible by its owner on non-Windows systems.
1401+
*
1402+
* The directory is created in the directory returned by WP-CLI's `get_temp_dir()`
1403+
* (based on `sys_get_temp_dir()`), not WordPress's `get_temp_dir()`, and is
1404+
* not removed automatically; callers are responsible for cleaning it up.
1405+
*
1406+
* On failure, this function exits via `WP_CLI::error()` rather than throwing.
1407+
*
1408+
* @access public
1409+
* @category System
1410+
*
1411+
* @param string $prefix Optional. Prefix for the temporary directory name. Default 'wp-cli-'.
1412+
* @return string Path to the created temporary directory with a trailing slash.
1413+
*/
1414+
function make_temp_dir( $prefix = 'wp-cli-' ) {
1415+
if ( false !== strpbrk( $prefix, "/\\\0" ) ) {
1416+
WP_CLI::error( 'Invalid temporary directory prefix.' );
1417+
}
1418+
1419+
$temp_dir = get_temp_dir();
1420+
$attempts = 0;
1421+
1422+
do {
1423+
$path = $temp_dir . uniqid( $prefix, true );
1424+
$success = @mkdir( $path, 0700 );
1425+
++$attempts;
1426+
} while ( ! $success && $attempts < 100 );
1427+
1428+
if ( ! $success ) {
1429+
WP_CLI::error( 'Failed to create a temporary directory.' );
1430+
}
1431+
1432+
return Path::trailingslashit( $path );
1433+
}
1434+
13401435
/**
13411436
* Parse a SSH url for its host, port, and path.
13421437
*

‎tests/ExtractorTest.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,27 @@ public function test_ensure_dir_exists(): void {
378378
rmdir( $dir );
379379
}
380380

381+
public function test_get_first_subfolder(): void {
382+
list( $temp_dir, $src_dir, $wp_dir ) = self::create_test_directory_structure();
383+
384+
$test_class = new ReflectionClass( Extractor::class );
385+
$method = $test_class->getMethod( 'get_first_subfolder' );
386+
if ( PHP_VERSION_ID < 80100 ) {
387+
// @phpstan-ignore method.deprecated
388+
$method->setAccessible( true );
389+
}
390+
391+
$result_with_slash = $method->invoke( null, rtrim( $src_dir, '/\\' ) . '/' );
392+
$result_without_slash = $method->invoke( null, rtrim( $src_dir, '/\\' ) );
393+
394+
$this->assertSame( $wp_dir, $result_with_slash );
395+
$this->assertSame( $wp_dir, $result_without_slash );
396+
$this->assertIsString( $result_with_slash );
397+
$this->assertStringNotContainsString( '//', $result_with_slash );
398+
399+
Extractor::rmdir( $temp_dir );
400+
}
401+
381402
/**
382403
* @return array{0: string, 1: string, 2: string}
383404
*/

‎tests/UtilsTest.php‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,104 @@ public function testGetTempDir(): void {
487487
$this->assertTrue( '/' === substr( Utils\get_temp_dir(), -1 ) );
488488
}
489489

490+
public function testMakeTempFile(): void {
491+
$file = Utils\make_temp_file();
492+
$this->assertFileExists( $file );
493+
$this->assertFalse( is_link( $file ) );
494+
if ( ! Utils\is_windows() ) {
495+
$this->assertSame( '0600', substr( sprintf( '%o', (int) fileperms( $file ) ), -4 ) );
496+
}
497+
unlink( $file );
498+
499+
$file_with_suffix = Utils\make_temp_file( 'wp-test-', '.phar' );
500+
$this->assertFileExists( $file_with_suffix );
501+
$this->assertStringEndsWith( '.phar', $file_with_suffix );
502+
unlink( $file_with_suffix );
503+
}
504+
505+
public function testMakeTempFileInvalidPrefixOrSuffix(): void {
506+
$class_wp_cli_capture_exit = new \ReflectionProperty( 'WP_CLI', 'capture_exit' );
507+
if ( PHP_VERSION_ID < 80100 ) {
508+
// @phpstan-ignore method.deprecated
509+
$class_wp_cli_capture_exit->setAccessible( true );
510+
}
511+
$prev_capture_exit = $class_wp_cli_capture_exit->getValue();
512+
$prev_logger = WP_CLI::get_logger();
513+
514+
$class_wp_cli_capture_exit->setValue( null, true );
515+
WP_CLI::set_logger( new Loggers\Execution() );
516+
517+
$invalid_inputs = [
518+
[ '../prefix', '' ],
519+
[ 'prefix/', '' ],
520+
[ 'prefix\\', '' ],
521+
[ "prefix\0", '' ],
522+
[ 'prefix', '../suffix' ],
523+
[ 'prefix', '/suffix' ],
524+
[ 'prefix', '\\suffix' ],
525+
[ 'prefix', "suffix\0" ],
526+
];
527+
528+
try {
529+
foreach ( $invalid_inputs as [ $prefix, $suffix ] ) {
530+
try {
531+
Utils\make_temp_file( $prefix, $suffix );
532+
$this->fail( 'Expected ExitException for invalid prefix/suffix.' );
533+
} catch ( ExitException $ex ) {
534+
$this->assertSame( 1, $ex->getCode() );
535+
}
536+
}
537+
} finally {
538+
$class_wp_cli_capture_exit->setValue( null, $prev_capture_exit );
539+
WP_CLI::set_logger( $prev_logger );
540+
}
541+
}
542+
543+
public function testMakeTempDir(): void {
544+
$dir = Utils\make_temp_dir();
545+
$this->assertDirectoryExists( $dir );
546+
$this->assertFalse( is_link( rtrim( $dir, '/\\' ) ) );
547+
$this->assertTrue( '/' === substr( $dir, -1 ) );
548+
if ( ! Utils\is_windows() ) {
549+
$this->assertSame( '0700', substr( sprintf( '%o', (int) fileperms( $dir ) ), -4 ) );
550+
}
551+
rmdir( $dir );
552+
}
553+
554+
public function testMakeTempDirInvalidPrefix(): void {
555+
$class_wp_cli_capture_exit = new \ReflectionProperty( 'WP_CLI', 'capture_exit' );
556+
if ( PHP_VERSION_ID < 80100 ) {
557+
// @phpstan-ignore method.deprecated
558+
$class_wp_cli_capture_exit->setAccessible( true );
559+
}
560+
$prev_capture_exit = $class_wp_cli_capture_exit->getValue();
561+
$prev_logger = WP_CLI::get_logger();
562+
563+
$class_wp_cli_capture_exit->setValue( null, true );
564+
WP_CLI::set_logger( new Loggers\Execution() );
565+
566+
$invalid_prefixes = [
567+
'../prefix',
568+
'prefix/',
569+
'prefix\\',
570+
"prefix\0",
571+
];
572+
573+
try {
574+
foreach ( $invalid_prefixes as $prefix ) {
575+
try {
576+
Utils\make_temp_dir( $prefix );
577+
$this->fail( 'Expected ExitException for invalid prefix.' );
578+
} catch ( ExitException $ex ) {
579+
$this->assertSame( 1, $ex->getCode() );
580+
}
581+
}
582+
} finally {
583+
$class_wp_cli_capture_exit->setValue( null, $prev_capture_exit );
584+
WP_CLI::set_logger( $prev_logger );
585+
}
586+
}
587+
490588
public function testHttpRequestBadAddress(): void {
491589
// Save WP_CLI state.
492590
$class_wp_cli_capture_exit = new \ReflectionProperty( 'WP_CLI', 'capture_exit' );

0 commit comments

Comments
 (0)