From 498f4b3f4c4c981bcc653a3f3ed51bee4d38538e Mon Sep 17 00:00:00 2001 From: Shawn Ogasawara Date: Fri, 25 Sep 2026 16:31:37 +0900 Subject: [PATCH 1/3] fix(legacy): parse the SSH config instead of matching the exact snippet On login, addUserSshConfig() checked whether ~/.ssh/config already contained the CLI's Include by searching for the exact three-line snippet as a substring. Any deviation, such as extra options in the Host block or a missing trailing "Host *", failed the check, so the CLI prompted to rewrite the file on every login. Add SshConfigInspector, a small ssh_config parser that accepts the Include when it appears at the top level, in a "Match all" block, or in a Host block whose patterns cover all of the configured domain wildcards. It handles "=" syntax, quoted values, tilde expansion, mixed case, and CRLF line endings, and ignores comments, Match blocks, and Host blocks with negated patterns. Every suggested Include path must be present, so on Windows both the converted and raw path formats are still required. A "~" path counts for all of them, as each SSH client expands it to its own home directory; it is compared in a canonical form so that Windows separators and drive letters match. On Unix, "~" is expanded using the passwd home directory, as OpenSSH does, rather than HOME. Co-Authored-By: Claude Opus 5.5 --- legacy/phpstan-baseline.neon | 2 +- legacy/src/Service/SshConfig.php | 29 ++- legacy/src/Util/SshConfigInspector.php | 179 +++++++++++++++++++ legacy/tests/Util/SshConfigInspectorTest.php | 164 +++++++++++++++++ 4 files changed, 370 insertions(+), 4 deletions(-) create mode 100644 legacy/src/Util/SshConfigInspector.php create mode 100644 legacy/tests/Util/SshConfigInspectorTest.php diff --git a/legacy/phpstan-baseline.neon b/legacy/phpstan-baseline.neon index 128f1d365..02fac6e02 100644 --- a/legacy/phpstan-baseline.neon +++ b/legacy/phpstan-baseline.neon @@ -7149,7 +7149,7 @@ parameters: - message: '#^Parameter \#2 \$array of function implode expects array, mixed given\.$#' identifier: argument.type - count: 5 + count: 4 path: src/Service/SshConfig.php - diff --git a/legacy/src/Service/SshConfig.php b/legacy/src/Service/SshConfig.php index 819074a47..898974c2b 100644 --- a/legacy/src/Service/SshConfig.php +++ b/legacy/src/Service/SshConfig.php @@ -7,6 +7,7 @@ use Platformsh\Cli\SshCert\Certifier; use Platformsh\Cli\Util\OsUtil; use Platformsh\Cli\Util\Snippeter; +use Platformsh\Cli\Util\SshConfigInspector; use Symfony\Component\Console\Output\ConsoleOutputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Filesystem\Exception\IOException; @@ -284,13 +285,16 @@ public function addUserSshConfig(QuestionHelper $questionHelper): bool $filename = $this->getUserSshConfigFilename(); $wildcards = $this->config->getWithDefault('ssh.domain_wildcards', []); - if (!$wildcards) { + if (!\is_array($wildcards) || !$wildcards) { return false; } + $wildcards = \array_values(\array_filter($wildcards, 'is_string')); + + $includePaths = $this->formattedPaths($this->getCliSshDir() . DIRECTORY_SEPARATOR . '*.config'); $lines = []; $lines[] = 'Host ' . \implode(' ', $wildcards); - foreach ($this->formattedPaths($this->getCliSshDir() . DIRECTORY_SEPARATOR . '*.config') as $path) { + foreach ($includePaths as $path) { $lines[] = ' Include ' . $path; } $lines[] = 'Host *'; @@ -312,7 +316,7 @@ public function addUserSshConfig(QuestionHelper $questionHelper): bool $this->stdErr->writeln('Failed to read file: ' . $filename . ''); return false; } - if (str_contains($currentContents, $suggestedConfig)) { + if (SshConfigInspector::includesPath($currentContents, $wildcards, $includePaths, $this->getSshHomeDirectory())) { $this->stdErr->writeln('Validated SSH configuration file: ' . $filename . '', OutputInterface::VERBOSITY_VERBOSE); return true; } @@ -438,6 +442,25 @@ private function getUserSshConfigChanges(string $currentConfig, string $newConfi * * @return string */ + /** + * Returns the home directory that the OpenSSH client uses to expand "~". + * + * On Unix, OpenSSH reads the home directory from the passwd database, + * not from HOME, which may differ (see quoteFilePath()). + */ + private function getSshHomeDirectory(): string + { + if (OsUtil::isWindows() || !\function_exists('posix_getpwuid') || !\function_exists('posix_geteuid')) { + return $this->config->getHomeDirectory(); + } + $entry = \posix_getpwuid(\posix_geteuid()); + if (!$entry || $entry['dir'] === '') { + return ''; + } + + return \realpath($entry['dir']) ?: $entry['dir']; + } + private function getUserSshConfigFilename(): string { return $this->config->getHomeDirectory() . DIRECTORY_SEPARATOR . '.ssh' . DIRECTORY_SEPARATOR . 'config'; diff --git a/legacy/src/Util/SshConfigInspector.php b/legacy/src/Util/SshConfigInspector.php new file mode 100644 index 000000000..8afd2a6b3 --- /dev/null +++ b/legacy/src/Util/SshConfigInspector.php @@ -0,0 +1,179 @@ + [ + "Host *.platform.sh *.upsun.com\n Include $include\nHost *\n", + true, + ], + 'extra options in the block' => [ + "# BEGIN: Upsun certificate configuration\n" + . "Host *.platform.sh *.upsun.com\n" + . " StrictHostKeyChecking no\n" + . " UserKnownHostsFile /dev/null\n" + . " LogLevel ERROR\n" + . " Include $include\n" + . "# END: Upsun certificate configuration\n", + true, + ], + 'different indentation, case, and pattern order' => [ + "host *.UPSUN.com *.platform.sh\n\tinclude $include\n", + true, + ], + 'keyword with equals sign' => [ + "Host=*.platform.sh *.upsun.com\nInclude=$include\n", + true, + ], + 'include among several paths' => [ + "Host *.platform.sh *.upsun.com\n Include ~/.ssh/other.config $include\n", + true, + ], + 'tilde expansion' => [ + "Host *.platform.sh *.upsun.com\n Include ~/.upsun-cli/ssh/*.config\n", + true, + ], + 'quoted path' => [ + "Host *.platform.sh *.upsun.com\n Include \"$include\"\n", + true, + ], + 'top-level include' => [ + "Include $include\n\nHost github.com\n User git\n", + true, + ], + 'match all block' => [ + "Match all\n Include $include\n", + true, + ], + 'catch-all host block' => [ + "Host *\n Include $include\n", + true, + ], + 'Windows line endings' => [ + "Host *.platform.sh *.upsun.com\r\n Include $include\r\n", + true, + ], + 'empty file' => [ + '', + false, + ], + 'commented out' => [ + "# Host *.platform.sh *.upsun.com\n# Include $include\n", + false, + ], + 'missing one wildcard' => [ + "Host *.platform.sh\n Include $include\n", + false, + ], + 'different include path' => [ + "Host *.platform.sh *.upsun.com\n Include /home/user/.platformsh/ssh/*.config\n", + false, + ], + 'include in an unrelated host block' => [ + "Host *.platform.sh *.upsun.com\n User foo\nHost github.com\n Include $include\n", + false, + ], + 'include in a match block' => [ + "Match host *.platform.sh\n Include $include\n", + false, + ], + 'negated pattern' => [ + "Host *.platform.sh *.upsun.com !foo.upsun.com\n Include $include\n", + false, + ], + 'include path as a prefix only' => [ + "Host *.platform.sh *.upsun.com\n Include {$include}.bak\n", + false, + ], + ]; + foreach ($cases as $name => [$contents, $expected]) { + $this->assertSame($expected, SshConfigInspector::includesPath($contents, $wildcards, [$include], '/home/user'), $name); + } + } + + public function testQuotedIncludePathWithSpaces(): void + { + $include = '"/home/some user/.upsun-cli/ssh/*.config"'; + $contents = "Host *.example.com\n Include \"/home/some user/.upsun-cli/ssh/*.config\"\n"; + $this->assertTrue(SshConfigInspector::includesPath($contents, ['*.example.com'], [$include])); + $this->assertFalse(SshConfigInspector::includesPath($contents, ['*.example.com'], ['/home/other/.upsun-cli/ssh/*.config'])); + } + + public function testTildeWithDifferentHomeDirectory(): void + { + $contents = "Host *.example.com\n Include ~/.upsun-cli/ssh/*.config\n"; + $include = '/home/user/.upsun-cli/ssh/*.config'; + $this->assertTrue(SshConfigInspector::includesPath($contents, ['*.example.com'], [$include], '/home/user/')); + $this->assertFalse(SshConfigInspector::includesPath($contents, ['*.example.com'], [$include], '/root')); + $this->assertFalse(SshConfigInspector::includesPath($contents, ['*.example.com'], [$include], '')); + } + + public function testWindowsPaths(): void + { + $wildcards = ['*.example.com']; + // The converted and raw formats, as produced by SshConfig::formattedPaths() with ssh.windows_paths: both. + $includes = ['/c/Users/me/.upsun-cli/ssh/*.config', 'C:\Users\me\.upsun-cli\ssh\*.config']; + $homeDir = 'C:\Users\me'; + $cases = [ + 'both formats' => [ + "Host *.example.com\n Include /c/Users/me/.upsun-cli/ssh/*.config\n Include C:\\Users\\me\\.upsun-cli\\ssh\\*.config\n", + true, + ], + 'both formats on one line' => [ + "Host *.example.com\n Include /c/Users/me/.upsun-cli/ssh/*.config C:\\Users\\me\\.upsun-cli\\ssh\\*.config\n", + true, + ], + 'converted format only' => [ + "Host *.example.com\n Include /c/Users/me/.upsun-cli/ssh/*.config\n", + false, + ], + 'raw format only' => [ + "Host *.example.com\n Include C:\\Users\\me\\.upsun-cli\\ssh\\*.config\n", + false, + ], + 'tilde' => [ + "Host *.example.com\n Include ~/.upsun-cli/ssh/*.config\n", + true, + ], + 'tilde with a different path' => [ + "Host *.example.com\n Include ~/.platformsh/ssh/*.config\n", + false, + ], + ]; + foreach ($cases as $name => [$contents, $expected]) { + $this->assertSame($expected, SshConfigInspector::includesPath($contents, $wildcards, $includes, $homeDir), $name); + } + } + + public function testEmptyInputs(): void + { + $this->assertFalse(SshConfigInspector::includesPath("Include /foo\n", [], ['/foo'])); + $this->assertFalse(SshConfigInspector::includesPath("Include /foo\n", ['*.example.com'], [])); + } +} From c1463e9d6dcab1fcf0a9d52612186c7d4908a5ea Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 14 Oct 2026 19:44:38 +0100 Subject: [PATCH 2/3] docs(legacy): fix docblocks around getSshHomeDirectory() Move the getUserSshConfigFilename() docblock back above its method, and drop a reference to quoteFilePath(), which does not describe the passwd home directory. Co-Authored-By: Claude Opus 5.5 --- legacy/src/Service/SshConfig.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/legacy/src/Service/SshConfig.php b/legacy/src/Service/SshConfig.php index 898974c2b..3e131a422 100644 --- a/legacy/src/Service/SshConfig.php +++ b/legacy/src/Service/SshConfig.php @@ -437,16 +437,11 @@ private function getUserSshConfigChanges(string $currentConfig, string $newConfi return (new Snippeter())->updateSnippet($currentConfig, $newConfig, $begin, $end); } - /** - * Returns the path to the user's global SSH config file. - * - * @return string - */ /** * Returns the home directory that the OpenSSH client uses to expand "~". * * On Unix, OpenSSH reads the home directory from the passwd database, - * not from HOME, which may differ (see quoteFilePath()). + * not from HOME, which may differ. */ private function getSshHomeDirectory(): string { @@ -461,6 +456,11 @@ private function getSshHomeDirectory(): string return \realpath($entry['dir']) ?: $entry['dir']; } + /** + * Returns the path to the user's global SSH config file. + * + * @return string + */ private function getUserSshConfigFilename(): string { return $this->config->getHomeDirectory() . DIRECTORY_SEPARATOR . '.ssh' . DIRECTORY_SEPARATOR . 'config'; From 3d35532fac1a5ecf0a41e9128f314e26ae0d485c Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 14 Oct 2026 23:01:08 +0100 Subject: [PATCH 3/3] fix(legacy): ignore trailing comments in the SSH config OpenSSH 8.7+ treats an unquoted "#" at the start of an argument as the start of a comment. Previously, comment text was parsed as arguments, so e.g. "Match all # global" or a commented "!host" pattern caused the CLI to prompt to rewrite the config. A "#" inside an argument is still literal, as in OpenSSH. Co-Authored-By: Claude Opus 5.5 --- legacy/src/Util/SshConfigInspector.php | 4 ++++ legacy/tests/Util/SshConfigInspectorTest.php | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/legacy/src/Util/SshConfigInspector.php b/legacy/src/Util/SshConfigInspector.php index 8afd2a6b3..99ca6acf7 100644 --- a/legacy/src/Util/SshConfigInspector.php +++ b/legacy/src/Util/SshConfigInspector.php @@ -141,6 +141,10 @@ private static function splitArgs(string $value): array $length = \strlen($value); for ($i = 0; $i < $length; $i++) { $char = $value[$i]; + // An unquoted "#" at the start of an argument begins a comment (OpenSSH 8.7+). + if ($char === '#' && !$quoted && !$started) { + break; + } if ($char === '"') { $quoted = !$quoted; $started = true; diff --git a/legacy/tests/Util/SshConfigInspectorTest.php b/legacy/tests/Util/SshConfigInspectorTest.php index 4d3763a89..f666dc880 100644 --- a/legacy/tests/Util/SshConfigInspectorTest.php +++ b/legacy/tests/Util/SshConfigInspectorTest.php @@ -64,6 +64,22 @@ public function testIncludesPath(): void "Host *.platform.sh *.upsun.com\r\n Include $include\r\n", true, ], + 'trailing comments' => [ + "Host *.platform.sh *.upsun.com # !old.upsun.com\n Include $include # CLI\n", + true, + ], + 'trailing comment on match all' => [ + "Match all # global\n Include $include\n", + true, + ], + 'hash inside an argument' => [ + "Host *.platform.sh *.upsun.com\n Include \"$include\"#x\n", + false, + ], + 'wildcards only in a trailing comment' => [ + "Host github.com # *.platform.sh *.upsun.com\n Include $include\n", + false, + ], 'empty file' => [ '', false,