fix(legacy): parse the SSH config instead of matching the exact snippet

[shawnplatform]

Summary

On login, addUserSshConfig() checked whether ~/.ssh/config already contained the CLI's Include by searching for the exact three-line snippet as a substring:

Host *.platform.sh *.upsun.com
  Include ~/.upsun-cli/ssh/*.config
Host *

Any deviation, such as extra options inside the Host block or a missing trailing Host *, failed the check. The CLI then printed "Checking SSH configuration file" and prompted to rewrite the file on every login, even though the configuration was functionally correct. Answering "yes" would replace the block and drop the user's custom options.

This PR replaces the substring match with a small ssh_config parser, SshConfigInspector, which accepts the Include when it appears:

• at the top level (before any Host or Match block), or in a Match all block, or
• in a Host block whose patterns contain every configured domain wildcard, or the catch-all *.

It handles Keyword=value syntax, quoted values, ~ expansion, mixed-case keywords and patterns, and CRLF line endings. It ignores comments, Match blocks, and Host blocks with negated patterns (which could exclude hosts the CLI needs to configure).

Typing the wildcards list properly also removed one baselined PHPStan error, so the baseline count is adjusted accordingly.

Test plan

• New SshConfigInspectorTest with table-driven cases covering the exact snippet, blocks with extra options, = syntax, quoting, tilde expansion, top-level and Match all includes, CRLF, comments, missing wildcards, unrelated blocks, Match blocks, negated patterns, and prefix-only paths
• Verified against a real ~/.ssh/config containing the marked block with extra StrictHostKeyChecking / UserKnownHostsFile / LogLevel options: now validated, no prompt
• phpstan level 10 clean, php-cs-fixer clean
• phpunit: the only failures are the two pre-existing DependenciesTest failures that also fail on main

🤖 Generated with Claude Code

[upsun-dispatch]

Warning

Changes suggested — 🟡 2 warnings · 🔵 1 minor point

🔍 Full review · 4 files reviewed

Verification

• splitArgs runs on both the expected paths and the file's Include arguments, so the """ quoting produced by quoteFilePath still compares consistently on both sides.
• Host patterns and required wildcards are both lowercased before comparison, while paths keep their case, which matches ssh's case-insensitive host matching.
• A Host or non-all Match line resets $scope, so an Include in a later unrelated block is not credited to an earlier matching block.
• The is_array guard and the is_string filter on ssh.domain_wildcards run before implode, which accounts for the lower PHPStan baseline count.

The new SshConfigInspectorTest covers the parser in isolation. No test covers SshConfig::addUserSshConfig(), the Windows formattedPaths() variants, or ~ expansion when HOME differs from the passwd home.

Review details

• Commit: 78efa92
• Model: claude-opus-5-5

---

Review 1 of 10 for this pull request · View the full run

[upsun-dispatch]

📋 PR Summary

When a user logs in, the CLI checks whether ~/.ssh/config already includes its SSH config. This PR swaps the exact three-line substring match for a small ssh_config parser, SshConfigInspector. The parser accepts the Include when it appears at the top level, in a Match all block, or in a Host block that covers every required domain wildcard (or *). That stops the repeated rewrite prompt for configs that already work. The newest changes have two parts. First, every configured Include path is now required, with Windows path formats compared in one canonical form. Second, a ~ in the file is expanded using the home directory OpenSSH itself uses: on Unix that comes from the passwd entry, not from HOME.

Changes

Layer / File(s)	Summary
SSH config detection
legacy/src/Util/SshConfigInspector.php	New parser that checks whether the required Include paths apply to the given host patterns. It handles = syntax, quoting, case, CRLF, comments, Match/negated Host blocks, and ~ expansion. Every expected path must be present unless a single ~ path matches. Windows paths are compared in canonical form.
legacy/src/Service/SshConfig.php	Replaces the snippet substring check with SshConfigInspector::includesPath(). Adds getSshHomeDirectory(), which uses the passwd home directory on Unix (falling back to the configured home directory) to expand ~.
Tests
legacy/tests/Util/SshConfigInspectorTest.php	Table-driven tests for the parser. These cover the syntax variants, where an Include is accepted, tilde expansion against different home directories, and Windows paths in both formats.
Tooling
legacy/phpstan-baseline.neon	Lowers the baselined PHPStan error count by one now that the wildcards list is properly typed.

[upsun-dispatch]

Note

Reviewed — No blocking findings · ⚪ 1 nitpick

🔁 Incremental (head + base moved) · 3 files reviewed

⚪ Nitpick

• legacy/src/Service/SshConfig.php:445 — getSshHomeDirectory() and its docblock were inserted between getUserSshConfigFilename() and that method's own docblock ("Returns the path to the user's global SSH config file. @return string"). The old docblock now sits as an orphan directly above the new one, and getUserSshConfigFilename() has no docblock at all. Nothing breaks at runtime.

Verification

• addUserSshConfig() now expands ~ with the passwd home from posix_getpwuid(posix_geteuid()). HOME is used only on Windows, or when the posix functions are unavailable.
• includesPath() records a match for each deduplicated expected path in $found. It returns true only once every path is found, so a Windows config that includes just one of the two windows_paths: both forms is rejected.
• canonicalPath() converts C:\Users\me/.upsun-cli/... and /c/Users/me/.upsun-cli/... to the same string. Because of that, a ~ Include on Windows now matches the paths formattedPaths() produces.
• OsUtil is already imported in SshConfig.php. The Go/PHP version updates on the base branch do not touch SshConfig or SshConfigInspector.

This change adds testTildeWithDifferentHomeDirectory and testWindowsPaths to SshConfigInspectorTest, which run with the legacy phpunit suite in CI. No test covers getSshHomeDirectory() or the passwd lookup in SshConfig.php.

Review details

• Commits: 78efa92..498f4b3
• Base moved since: 185da06
• Model: claude-opus-5-5

---

Review 2 of 10 for this pull request · View the full run