Skip to content

Normalize temp dir in Given a directory step #329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
swissspidy merged 2 commits into from
Apr 14, 2026
Merged

Normalize temp dir in Given a directory step #329

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15141ea
Normalize temp dir in `Given a directory` step
swissspidy Apr 13, 2026
04796d6
more realpath
swissspidy Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/Context/GivenStepDefinitions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,13 @@ public function given_a_specific_directory( $empty_or_nonexistent, $dir ): void
// Mac OS X can prefix the `/var` folder to turn it into `/private/var`.
$dir = preg_replace( '|^/private/var/|', '/var/', $dir );

$temp_dir = Path::normalize( sys_get_temp_dir() );
$temp_dir = realpath( sys_get_temp_dir() );
$temp_dir = $temp_dir ? Path::normalize( $temp_dir ) : Path::normalize( sys_get_temp_dir() );
Comment thread
swissspidy marked this conversation as resolved.
$dir = Path::normalize( $dir );

// Also normalize temp dir for Mac OS X.
$temp_dir = preg_replace( '|^/private/var/|', '/var/', $temp_dir );
Comment thread
swissspidy marked this conversation as resolved.

Comment on lines +56 to +62
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

realpath( sys_get_temp_dir() ) can resolve macOS temp paths to /private/tmp (e.g. when sys_get_temp_dir() is /tmp), but the current normalization only rewrites /private/var. This can make the safety check reject directories under /tmp because $dir may start with /tmp while $temp_dir starts with /private/tmp. Consider normalizing a leading /private/ prefix (not just /private/var) for both $dir and $temp_dir, or allowing both the realpath and non-realpath variants when checking the prefix.

Copilot uses AI. Check for mistakes.
Comment on lines +56 to +62
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated temp-dir canonicalization is OS/filesystem-dependent (e.g. /tmp vs /private/tmp, /var vs /private/var). There are Behat scenarios covering the “Given a specific directory” step, but none appear to exercise these canonicalization differences; adding a scenario that derives both sys_get_temp_dir() and realpath(sys_get_temp_dir()) and verifies both path forms are accepted would help prevent regressions on macOS runners.

Copilot uses AI. Check for mistakes.
// Also check for temp dir prefixed with `/private` for Mac OS X.
if ( 0 !== strpos( $dir, $temp_dir ) && 0 !== strpos( $dir, "/private{$temp_dir}" ) ) {
Comment on lines 63 to 64
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The containment check uses strpos( $dir, $temp_dir ) === 0, which can incorrectly allow paths that merely share a prefix (e.g. temp dir /tmp and dir /tmp2/...). To avoid accidental deletion outside the intended temp dir, tighten this to require a path boundary (exact match or prefix followed by a directory separator after normalization).

Copilot uses AI. Check for mistakes.
throw new RuntimeException(
Expand Down
Loading