Skip to content

Commit

Permalink
minor #58710 [Process] On Windows, don't rely on the OS to find execu…
Browse files Browse the repository at this point in the history
…tables (nicolas-grekas)

This PR was merged into the 7.2 branch.

Discussion
----------

[Process] On Windows, don't rely on the OS to find executables

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Porting part of composer/composer#12180 here:

On Windows, when searching for an executable, the OS always looks at the current directory before using the PATH variable. This makes it easier than desired to hijack executables. Unix-like OSes don't have this issue.

This PR proposes to rely on ExecutableFinder instead.

Commits
-------

b35a7d42931 [Process] On Windows, don't rely on the OS to find executables
  • Loading branch information
nicolas-grekas committed Nov 4, 2024
2 parents 8b1744e + 18f50a7 commit 4b3cae7
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 5 deletions.
7 changes: 7 additions & 0 deletions Process.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class Process implements \IteratorAggregate
private ?int $cachedExitCode = null;

private static ?bool $sigchild = null;
private static array $executables = [];

/**
* Exit codes translation table.
Expand Down Expand Up @@ -1543,6 +1544,12 @@ private function buildShellCommandline(string|array $commandline): string
return $commandline;
}

if ('\\' === \DIRECTORY_SEPARATOR && isset($commandline[0][0]) && \strlen($commandline[0]) === strcspn($commandline[0], ':/\\')) {
// On Windows, we don't rely on the OS to find the executable if possible to avoid lookups
// in the current directory which could be untrusted. Instead we use the ExecutableFinder.
$commandline[0] = (self::$executables[$commandline[0]] ??= (new ExecutableFinder())->find($commandline[0])) ?? $commandline[0];
}

return implode(' ', array_map($this->escapeArgument(...), $commandline));
}

Expand Down
10 changes: 5 additions & 5 deletions Tests/ProcessFailedExceptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ public function testProcessFailedExceptionPopulatesInformationFromProcessOutput(

$exception = new ProcessFailedException($process);

$this->assertEquals(
"The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}\n\nOutput:\n================\n{$output}\n\nError Output:\n================\n{$errorOutput}",
$this->assertStringMatchesFormat(
"The command \"%s\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}\n\nOutput:\n================\n{$output}\n\nError Output:\n================\n{$errorOutput}",
str_replace("'php'", 'php', $exception->getMessage())
);
}
Expand Down Expand Up @@ -126,9 +126,9 @@ public function testDisabledOutputInFailedExceptionDoesNotPopulateOutput()

$exception = new ProcessFailedException($process);

$this->assertEquals(
"The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}",
str_replace("'php'", 'php', $exception->getMessage())
$this->assertStringMatchesFormat(
"The command \"%s\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}",
$exception->getMessage()
);
}
}
11 changes: 11 additions & 0 deletions Tests/ProcessTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1692,6 +1692,17 @@ public function testNotIgnoringSignal()
$this->assertSame(\SIGTERM, $process->getTermSignal());
}

public function testPathResolutionOnWindows()
{
if ('\\' !== \DIRECTORY_SEPARATOR) {
$this->markTestSkipped('This test is for Windows platform only');
}

$process = $this->getProcess(['where']);

$this->assertSame('C:\\Windows\\system32\\where.EXE', $process->getCommandLine());
}

private function getProcess(string|array $commandline, ?string $cwd = null, ?array $env = null, mixed $input = null, ?int $timeout = 60): Process
{
if (\is_string($commandline)) {
Expand Down

0 comments on commit 4b3cae7

Please sign in to comment.