Skip to content

Commit bf89bec

Browse files
shawnplatformclaudepjcdawkins
authored
fix(legacy): parse the SSH config instead of matching the exact snippet (#180)
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Patrick Dawkins <patrick@upsun.com>
1 parent 3aedeea commit bf89bec

4 files changed

Lines changed: 390 additions & 4 deletions

File tree

‎legacy/phpstan-baseline.neon‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6825,7 +6825,7 @@ parameters:
68256825
-
68266826
message: '#^Parameter \#2 \$array of function implode expects array, mixed given\.$#'
68276827
identifier: argument.type
6828-
count: 5
6828+
count: 4
68296829
path: src/Service/SshConfig.php
68306830

68316831
-

‎legacy/src/Service/SshConfig.php‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Platformsh\Cli\SshCert\Certifier;
88
use Platformsh\Cli\Util\OsUtil;
99
use Platformsh\Cli\Util\Snippeter;
10+
use Platformsh\Cli\Util\SshConfigInspector;
1011
use Symfony\Component\Console\Output\ConsoleOutputInterface;
1112
use Symfony\Component\Console\Output\OutputInterface;
1213
use Symfony\Component\Filesystem\Exception\IOException;
@@ -284,13 +285,16 @@ public function addUserSshConfig(QuestionHelper $questionHelper): bool
284285
$filename = $this->getUserSshConfigFilename();
285286

286287
$wildcards = $this->config->getWithDefault('ssh.domain_wildcards', []);
287-
if (!$wildcards) {
288+
if (!\is_array($wildcards) || !$wildcards) {
288289
return false;
289290
}
291+
$wildcards = \array_values(\array_filter($wildcards, 'is_string'));
292+
293+
$includePaths = $this->formattedPaths($this->getCliSshDir() . DIRECTORY_SEPARATOR . '*.config');
290294

291295
$lines = [];
292296
$lines[] = 'Host ' . \implode(' ', $wildcards);
293-
foreach ($this->formattedPaths($this->getCliSshDir() . DIRECTORY_SEPARATOR . '*.config') as $path) {
297+
foreach ($includePaths as $path) {
294298
$lines[] = ' Include ' . $path;
295299
}
296300
$lines[] = 'Host *';
@@ -312,7 +316,7 @@ public function addUserSshConfig(QuestionHelper $questionHelper): bool
312316
$this->stdErr->writeln('Failed to read file: <comment>' . $filename . '</comment>');
313317
return false;
314318
}
315-
if (str_contains($currentContents, $suggestedConfig)) {
319+
if (SshConfigInspector::includesPath($currentContents, $wildcards, $includePaths, $this->getSshHomeDirectory())) {
316320
$this->stdErr->writeln('Validated SSH configuration file: <info>' . $filename . '</info>', OutputInterface::VERBOSITY_VERBOSE);
317321
return true;
318322
}
@@ -433,6 +437,25 @@ private function getUserSshConfigChanges(string $currentConfig, string $newConfi
433437
return (new Snippeter())->updateSnippet($currentConfig, $newConfig, $begin, $end);
434438
}
435439

440+
/**
441+
* Returns the home directory that the OpenSSH client uses to expand "~".
442+
*
443+
* On Unix, OpenSSH reads the home directory from the passwd database,
444+
* not from HOME, which may differ.
445+
*/
446+
private function getSshHomeDirectory(): string
447+
{
448+
if (OsUtil::isWindows() || !\function_exists('posix_getpwuid') || !\function_exists('posix_geteuid')) {
449+
return $this->config->getHomeDirectory();
450+
}
451+
$entry = \posix_getpwuid(\posix_geteuid());
452+
if (!$entry || $entry['dir'] === '') {
453+
return '';
454+
}
455+
456+
return \realpath($entry['dir']) ?: $entry['dir'];
457+
}
458+
436459
/**
437460
* Returns the path to the user's global SSH config file.
438461
*
Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Platformsh\Cli\Util;
6+
7+
/**
8+
* Inspects the contents of an OpenSSH client configuration file.
9+
*
10+
* This is a deliberately small parser: it only needs to answer whether an
11+
* Include directive already applies to a set of host patterns. See the
12+
* ssh_config(5) man page for the file format.
13+
*/
14+
class SshConfigInspector
15+
{
16+
/**
17+
* Checks whether the config already includes all of the given paths for all of the given host patterns.
18+
*
19+
* The include is accepted if it appears:
20+
* - at the top level (before any Host or Match block), or in a "Match all" block, or
21+
* - in a Host block whose patterns contain every required pattern, or the catch-all "*".
22+
*
23+
* Host blocks containing negated patterns ("!example.com") are ignored, as
24+
* they may exclude hosts that the CLI needs to configure.
25+
*
26+
* @param string $contents
27+
* The contents of the SSH config file.
28+
* @param string[] $hostPatterns
29+
* The host patterns (wildcards) that the include must apply to, e.g. ['*.example.com'].
30+
* @param string[] $includePaths
31+
* Required Include paths. These may be quoted in the same way as an SSH
32+
* config value. Each one must be included, unless a single "~" path
33+
* matches: that resolves for every SSH client, via its own home directory.
34+
* @param string $homeDir
35+
* The user's home directory as SSH sees it, used to expand a leading "~"
36+
* in Include paths. If empty, "~" paths are never matched.
37+
*/
38+
public static function includesPath(string $contents, array $hostPatterns, array $includePaths, string $homeDir = ''): bool
39+
{
40+
if ($hostPatterns === [] || $includePaths === []) {
41+
return false;
42+
}
43+
44+
$expected = [];
45+
foreach ($includePaths as $path) {
46+
foreach (self::splitArgs($path) as $arg) {
47+
$expected[] = $arg;
48+
}
49+
}
50+
$expected = \array_values(\array_unique($expected));
51+
$canonicalExpected = \array_map([self::class, 'canonicalPath'], $expected);
52+
$found = [];
53+
$required = \array_map('strtolower', $hostPatterns);
54+
55+
// The scope is null at the top level, or otherwise the list of Host
56+
// patterns (lowercase) for the current block. A Match block, other
57+
// than "Match all", is represented as an empty list so that its
58+
// Include directives are not counted.
59+
$scope = null;
60+
61+
foreach (\preg_split('/\r\n|\r|\n/', $contents) ?: [] as $line) {
62+
$line = \trim($line);
63+
if ($line === '' || $line[0] === '#') {
64+
continue;
65+
}
66+
if (!\preg_match('/^([A-Za-z]+)(?:\s*=\s*|\s+)(.*)$/', $line, $matches)) {
67+
continue;
68+
}
69+
$keyword = \strtolower($matches[1]);
70+
$args = self::splitArgs($matches[2]);
71+
switch ($keyword) {
72+
case 'host':
73+
$scope = \array_map('strtolower', $args);
74+
break;
75+
76+
case 'match':
77+
$scope = \count($args) === 1 && \strtolower($args[0]) === 'all' ? null : [];
78+
break;
79+
80+
case 'include':
81+
if ($scope !== null && !self::patternsCover($scope, $required)) {
82+
break;
83+
}
84+
foreach ($args as $arg) {
85+
if ($arg === '~' || \str_starts_with($arg, '~/')) {
86+
if ($homeDir !== '' && \in_array(self::canonicalPath($homeDir . \substr($arg, 1)), $canonicalExpected, true)) {
87+
return true;
88+
}
89+
continue;
90+
}
91+
$index = \array_search($arg, $expected, true);
92+
if ($index !== false) {
93+
$found[$index] = true;
94+
if (\count($found) === \count($expected)) {
95+
return true;
96+
}
97+
}
98+
}
99+
break;
100+
}
101+
}
102+
103+
return false;
104+
}
105+
106+
/**
107+
* Checks whether a Host block's patterns cover all of the required patterns.
108+
*
109+
* @param string[] $patterns
110+
* @param string[] $required
111+
*/
112+
private static function patternsCover(array $patterns, array $required): bool
113+
{
114+
foreach ($patterns as $pattern) {
115+
if (\str_starts_with($pattern, '!')) {
116+
return false;
117+
}
118+
}
119+
if (\in_array('*', $patterns, true)) {
120+
return true;
121+
}
122+
foreach ($required as $pattern) {
123+
if (!\in_array($pattern, $patterns, true)) {
124+
return false;
125+
}
126+
}
127+
return true;
128+
}
129+
130+
/**
131+
* Splits a config value into arguments, honoring double quotes.
132+
*
133+
* @return string[]
134+
*/
135+
private static function splitArgs(string $value): array
136+
{
137+
$args = [];
138+
$current = '';
139+
$quoted = false;
140+
$started = false;
141+
$length = \strlen($value);
142+
for ($i = 0; $i < $length; $i++) {
143+
$char = $value[$i];
144+
// An unquoted "#" at the start of an argument begins a comment (OpenSSH 8.7+).
145+
if ($char === '#' && !$quoted && !$started) {
146+
break;
147+
}
148+
if ($char === '"') {
149+
$quoted = !$quoted;
150+
$started = true;
151+
continue;
152+
}
153+
if (!$quoted && ($char === ' ' || $char === "\t")) {
154+
if ($started) {
155+
$args[] = $current;
156+
$current = '';
157+
$started = false;
158+
}
159+
continue;
160+
}
161+
$current .= $char;
162+
$started = true;
163+
}
164+
if ($started) {
165+
$args[] = $current;
166+
}
167+
return $args;
168+
}
169+
170+
/**
171+
* Converts a path to a canonical form, for comparing Windows paths in their different formats.
172+
*
173+
* For example "C:\Users\me/.ssh" and "/c/Users/me/.ssh" are both converted to "/c/Users/me/.ssh".
174+
*/
175+
private static function canonicalPath(string $path): string
176+
{
177+
$path = \str_replace('\\', '/', $path);
178+
if (\preg_match('#^([A-Za-z]):/#', $path, $matches)) {
179+
$path = '/' . \strtolower($matches[1]) . \substr($path, 2);
180+
}
181+
return (string) \preg_replace('#/+#', '/', $path);
182+
}
183+
}

0 commit comments

Comments
 (0)