Utils: Add `make_temp_file()` and `make_temp_dir()` helper functions by swissspidy · Pull Request #6374 · wp-cli/wp-cli

swissspidy · 2026-08-03T14:32:41Z

Summary by CodeRabbit

Security
- Improved temporary file and directory handling with secure creation, restrictive permissions, and exclusive access.
- Added protections against unsafe temporary-file scenarios, including symlink attacks.
Bug Fixes
- Temporary extraction and update operations now use safer temporary storage.
- Improved handling of extracted paths with trailing slashes.
- Editor input is written securely with clearer failure reporting.
Tests
- Added coverage for secure temporary resources, cleanup, suffix handling, symlink rejection, and path normalization.

…ch_editor_for_input()

coderabbitai · 2026-08-03T14:32:50Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR adds secure temporary file and directory helpers with bounded retries. Extraction, editor input, and Phar update flows use these helpers instead of manually constructed paths. Tests cover validation, permissions, cleanup, symlink handling, formatting, and extracted-path normalization.

Changes

Temporary resource security

Layer / File(s)	Summary
Temporary resource helpers and validation `php/utils.php`, `tests/UtilsTest.php`	Added `make_temp_file()` and `make_temp_dir()` with exclusive creation, restricted permissions, bounded retries, validation, and failure errors. Updated editor input writes and added helper tests.
Extraction and Phar caller migration `php/WP_CLI/Extractor.php`, `php/commands/src/CLI_Command.php`, `php/utils.php`, `tests/ExtractorTest.php`	ZIP, tarball, Phar extraction, and CLI update flows now use the centralized helpers. `get_first_subfolder()` normalizes trailing separators, with test coverage.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

wp-cli/wp-cli#6369: Both changes modify temporary Phar path handling in CLI_Command::update().
wp-cli/wp-cli#6373: Both changes modify temporary directory handling in extraction flows.
wp-cli/wp-cli#6378: Both changes modify directory handling in Extractor.php.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 52.94% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely identifies the two main utility functions added by the pull request.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🛠️ Fix failing CI checks 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch add/temp-file

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

php/utils.php (1)
599-613: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider reusing make_temp_file() here for consistency.

launch_editor_for_input() still implements its own exclusive-creation retry loop instead of calling the new make_temp_file() helper this PR introduces. The rest of the codebase now centralizes secure temporary-file creation in make_temp_file(). Duplicating the same 'xb' exclusive-open retry pattern here increases maintenance cost and risks the two implementations drifting apart over time.

Consider passing a prefix derived from $tmpfile and a suffix of '.' . $ext to make_temp_file(), then writing $input to the returned path with an explicit write-and-check step.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@php/utils.php` around lines 599 - 613, Update launch_editor_for_input() to
use the existing make_temp_file() helper instead of its local exclusive-creation
retry loop. Pass a prefix derived from $tmpfile and the '.'.$ext suffix, then
write $input to the returned path with an explicit write-result check while
preserving the existing cleanup/error behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@php/utils.php`:
- Around line 609-612: Update the file-writing block around $fp to validate the
return value of fwrite() and call WP_CLI::error() when the write fails or is
incomplete, before closing the file or opening the editor. Preserve the existing
successful-write flow and cleanup behavior.

---

Nitpick comments:
In `@php/utils.php`:
- Around line 599-613: Update launch_editor_for_input() to use the existing
make_temp_file() helper instead of its local exclusive-creation retry loop. Pass
a prefix derived from $tmpfile and the '.'.$ext suffix, then write $input to the
returned path with an explicit write-result check while preserving the existing
cleanup/error behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2ac0db3b-1510-4469-a1a5-b4fc72855e2c

📥 Commits

Reviewing files that changed from the base of the PR and between b0058f2 and 66470e2.

📒 Files selected for processing (4)

php/WP_CLI/Extractor.php
php/commands/src/CLI_Command.php
php/utils.php
tests/UtilsTest.php

codecov · 2026-08-03T15:21:40Z

Codecov Report

❌ Patch coverage is 57.77778% with 19 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
php/utils.php	56.09%	18 Missing ⚠️
php/commands/src/CLI_Command.php	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

`fopen( ..., 'xb' )` applies the process umask, which typically leaves the file at 0644 and therefore readable by other local users. Both call sites can hold sensitive input - the editor buffer in `launch_editor_for_input()` and whatever callers write to a `make_temp_file()` path - so create them under a temporary 0177 umask, matching the 0700 used for temporary directories. Narrowing the umask around `fopen()` rather than chmod'ing afterwards also avoids the window in which the file exists with wider permissions.

`ReflectionMethod::invoke()` returns mixed, so narrow it before passing it to `assertStringNotContainsString()`. `assertInstanceOf()` inside a typed `catch` block is always true. Assert the exit code carried by the exception instead, which is what the calls were meant to establish.

swissspidy added 4 commits August 3, 2026 16:29

Utils: Add make_temp_file() and make_temp_dir() helper functions

bffa3dd

CLI: Safely create and escape temp PHAR path in cli update

79a62da

Utils: Secure temporary file creation in extract_from_phar() and laun…

6b835b2

…ch_editor_for_input()

Extractor: Use make_temp_dir() for secure archive extraction

66470e2

swissspidy added the scope:framework label Aug 3, 2026

swissspidy marked this pull request as ready for review August 3, 2026 14:52

swissspidy requested a review from a team as a code owner August 3, 2026 14:52

Copilot AI review requested due to automatic review settings August 3, 2026 14:52

Copilot started reviewing on behalf of swissspidy August 3, 2026 14:52 View session

This comment was marked as resolved.

Sign in to view

coderabbitai Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread php/utils.php

swissspidy added 3 commits August 3, 2026 22:36

Fix temp file creation and write validation in launch_editor_for_input()

0dc3067

Validate prefix and suffix in make_temp_file() and make_temp_dir()

db086a3

Normalize path in get_first_subfolder() to avoid double slashes

045c8dd

This comment was marked as resolved.

Sign in to view

claude added 2 commits August 3, 2026 21:58

swissspidy force-pushed the add/temp-file branch from 9ef297c to 51fa905 Compare August 3, 2026 21:58

This comment was marked as resolved.

Sign in to view

swissspidy mentioned this pull request Aug 4, 2026

Use 0755 instead of 0700 for cache and extraction directories #6378

Merged

swissspidy added 3 commits August 5, 2026 12:29

Merge branch 'main' into add/temp-file

21fbbd4

PHPStan fix

39f1ba4

Address code review feedback

af2cc70

swissspidy added this to the 3.0.0 milestone Aug 5, 2026

Navigation Menu

Uh oh!

Utils: Add `make_temp_file()` and `make_temp_dir()` helper functions - #6374

Utils: Add `make_temp_file()` and `make_temp_dir()` helper functions#6374
swissspidy wants to merge 12 commits into
mainfrom
add/temp-file

swissspidy commented Aug 3, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

This comment was marked as resolved.

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

codecov Bot commented Aug 3, 2026 •

edited

Loading

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

swissspidy commented Aug 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

This comment was marked as resolved.

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

swissspidy commented Aug 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading

codecov Bot commented Aug 3, 2026 •

edited

Loading