feat: support the new SSH keys API

[camsmith]

Move the SSH key commands (ssh-keys, ssh-key:add, ssh-key:delete) from the old account-info API to the new SSH keys API at /users/{id}/ssh-keys.

Changes

• Add Api::getSshKeys(), getSshKey(), addSshKey() and deleteSshKey(), which call the new API directly. The key list follows pagination, with limits against circular or runaway next links, and is cached per session. Adding or deleting a key clears the cache.
• Add a Platformsh\Cli\Model\SshKey model for the new representation: the ID is an opaque string (a ULID) rather than an integer, the label replaces the title, and the key has an active flag.
• Fingerprints use the OpenSSH SHA-256 format (SHA256:…, as printed by ssh-keygen -l) instead of MD5.
• ssh-keys shows the label and whether each key is active, and the SHA-256 fingerprint is available as a column.
• Inactive keys are ignored when matching local keys to account keys.
• ssh-key:add checks for duplicates against a fresh key list and reports an existing inactive key. If the API returns 409 (the key is already registered by any user), the command shows a specific message.
• Fix a TypeError when a re-authentication challenge includes a max age.

[Copilot]

🟡 Changes recommended

The new SSH key API methods introduce inconsistent error handling (raw BadResponseException) compared to established ApiResponseException wrapping, which should be aligned before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR migrates the legacy CLI’s SSH key management from the older account/SSH-key representation to a newer SSH keys API, updating the CLI to use SHA-256 (OpenSSH-style) fingerprints and a new SSH key model.

Changes:

• Added a dedicated Platformsh\Cli\Model\SshKey model and updated commands/services to use new fields (id, label, sha256).
• Implemented new SSH key API methods in Api (list/get/add/delete) including pagination handling and cache invalidation.
• Updated fingerprint calculation to return OpenSSH-style SHA256: fingerprints and added PHPUnit coverage for the fingerprint behavior.

File summaries

File	Description
legacy/tests/Service/SshKeyTest.php	Adds tests for SHA-256/OpenSSH fingerprint generation and invalid key handling.
legacy/src/Service/SshKey.php	Switches account-key matching from MD5 to SHA-256/OpenSSH fingerprint format.
legacy/src/Service/Api.php	Adds SSH keys API endpoints (list/get/add/delete), pagination, and dedicated caching.
legacy/src/Model/SshKey.php	Introduces a CLI-level SSH key model for the new API representation.
legacy/src/Event/LoginRequiredEvent.php	Ensures login option values are consistently stringified for shell escaping.
legacy/src/Command/SshKey/SshKeyListCommand.php	Updates displayed/output columns to match new key fields (label, sha256).
legacy/src/Command/SshKey/SshKeyDeleteCommand.php	Updates deletion flow to use new key IDs and new API delete operation.
legacy/src/Command/SshKey/SshKeyAddCommand.php	Updates add flow to use new API and handles “already registered” (409) response.
legacy/phpstan-baseline.neon	Removes a baseline entry that is no longer applicable after key-ID typing changes.

Review details

Suppressed comments (2)

legacy/src/Service/Api.php:994

• In getSshKey(), non-404 failures are rethrown as BadResponseException. For consistency with the rest of the API layer (and to preserve the richer error details formatting), it should rethrow ApiResponseException::create(...) instead.

            if ($e->getResponse()->getStatusCode() === 404) {
                return null;
            }
            throw $e;
        }

legacy/src/Service/Api.php:1027

• deleteSshKey() currently lets BadResponseException bubble up directly. To keep error handling consistent with other direct HTTP calls, catch BadResponseException and rethrow ApiResponseException::create(...).

        $this->getHttpClient()->request('DELETE', $this->sshKeysUrl() . '/' . rawurlencode($id));
        $this->clearSshKeysCache();

• Files reviewed: 9/9 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[upsun-dispatch]

📋 PR Summary

This PR moves the SSH key commands (ssh-keys, ssh-key:add, ssh-key:delete) onto a new SSH key model with SHA-256 fingerprints and an active flag. It keeps a fallback to the legacy Accounts API. A new /ssh-key-source endpoint, cached per session, tells the CLI which backend to use. If it returns auth, the CLI uses the paginated /users/{id}/ssh-keys API. If it returns accounts or 404, the CLI uses /me and /ssh_keys, and converts legacy keys into the new model. The PR also fixes a TypeError in re-authentication challenges that include a max age.

Changes

Layer / File(s)	Summary
API and model
legacy/src/Service/Api.php	Adds getSshKeySource(), which is cached and rejects unknown values. It also adds getSshKeys(), getSshKey(), addSshKey() and deleteSshKey(), which pick the URL, payload field (title vs label) and data conversion based on the source. Legacy keys are listed from /me; Auth keys are listed with pagination guards. Cache keys include the source, and clearing the cache removes both.
legacy/src/Model/SshKey.php	New read-only SshKey model with a string ID, label, active flag and SHA-256 fingerprint. fromLegacyData() converts Accounts API keys, which are always treated as active and get a computed fingerprint. A shared fingerprint() helper produces the OpenSSH format.
legacy/src/Service/SshKey.php	Uses the model's fingerprint helper when matching local keys and ignores inactive keys.
Commands
legacy/src/Command/SshKey/SshKeyAddCommand.php	Checks for duplicates against a fresh key list, reports an existing inactive key, and shows a specific message when the API returns 409.
legacy/src/Command/SshKey/SshKeyDeleteCommand.php	Deletes keys through the new API methods, using string IDs.
legacy/src/Command/SshKey/SshKeyListCommand.php	Shows the label and active status, with the SHA-256 fingerprint available as a column.
Auth fix
legacy/src/Event/LoginRequiredEvent.php	Fixes a TypeError when a re-authentication challenge includes a max age.
Tooling
legacy/phpstan-baseline.neon	Removes baseline entries that are no longer needed.
Tests
legacy/tests/Service/ApiSshKeyTest.php	Covers source detection and caching, the 404 fallback, rejection of unknown sources, error propagation, the legacy list and mutation endpoints, pagination and cache clearing.
legacy/tests/Service/SshKeyTest.php	Tests fingerprinting and matching local keys to account keys.
legacy/tests/Command/SshKey/SshKeyAddCommandTest.php	Tests duplicate and conflict handling in ssh-key:add.
legacy/tests/Command/SshKey/SshKeyListCommandTest.php	Tests the output of ssh-keys.

Flow Diagram

flowchart TD
A[SSH key call] --> B{getSshKeySource}
B -->|auth| C[/users/id/ssh-keys paginated/]
B -->|accounts or 404| D[/me and /ssh_keys/]
C --> E[SshKey::fromData]
D --> F[SshKey::fromLegacyData]

Loading

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 3 minor points

🔍 Full review · 9 files reviewed

🔍 What this review checked

• The test vector's expected fingerprint SHA256:Zc8rf0C3ZFAVs8mnWl4r6jKmJN8kutsoBK2h4UkXPp8 is the correct unpadded base64 SHA-256 of that key blob (recomputed independently).
• No references to the old fields ($key->title, ->fingerprint, ->key_id) or to Platformsh\Client\Model\SshKey remain, and getLegacyAccountInfo has no leftover callers.
• addSshKey() and deleteSshKey() both invalidate the new session-scoped 'ssh-keys' cache key, and both commands re-warm it with getSshKeys(true).
• The removed phpstan-baseline entry is the only baseline entry referencing SshKey code, so no baseline entry is left unmatched.
• The is_array() cache check makes a cached empty key list a hit rather than a miss, unlike the old truthiness check.

Verification. The diff adds legacy/tests/Service/SshKeyTest.php covering only getPublicKeyFingerprint (valid and invalid key); nothing covers the new Api::getSshKeys pagination/caching, getSshKey 404 handling, addSshKey 409 handling or deleteSshKey. The legacy-php job in .github/workflows/ci.yml runs php-cs-fixer, phpstan (level 8 + baseline) and ./scripts/test/unit.sh over these files.

Review details

• Commit: 8672834
• Model: claude-opus-5

🔵 Minor points

• legacy/src/Service/Api.php:988 — The new SSH key methods let raw Guzzle exceptions escape, unlike every other direct HTTP call in this codebase (e.g. Api::getTasks() at line ~1914, TaskRunCommand, TeamUserAddCommand, which all do throw ApiResponseException::create($e->getRequest(), $e->getResponse(), $e)). getSshKey() maps only 404 to null; combined with the removal of the is_numeric($id) guard in SshKeyDeleteCommand, ssh-key:delete <malformed-id> now sends the ID to the API and a 400/422 response surfaces as an unhandled GuzzleHttp\Exception\ClientException with a truncated-body message instead of "SSH key not found" or a formatted API error. The same applies to getSshKeys(), addSshKey() (non-409 errors) and deleteSshKey().
• legacy/src/Service/Api.php:959 — The pagination loop in getSshKeys() has no page limit and no guard against a repeated URL: it follows _links.next.href unconditionally, so if the API returns a next link on the last page that resolves to the URL just requested (a common pattern for APIs that always emit next), the loop issues the same GET forever while appending duplicate items to $items, hanging the command and growing memory without bound.
• legacy/src/Model/SshKey.php:22 — SshKey::$active is parsed from the API but never read anywhere: Api::getSshKeys() returns inactive keys, SshKeyListCommand has no active column, and Service\SshKey::listAccountKeyFingerprints() feeds inactive keys' fingerprints into findIdentityMatchingPublicKeys(). A user whose only account key is inactive gets it auto-selected as the SSH IdentityFile by selectIdentity() and is told by WelcomeCommand/SshDiagnostics that a local key matching the account exists, while SSH authentication fails with no explanation.

---

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

[upsun-dispatch]

Warning

Changes suggested — 🟡 1 warning · 🔵 2 minor points

🔁 Incremental · 5 files reviewed

🔍 What this review checked

• ApiResponseException::wrapGuzzleException() exists in platformsh/client at the locked revision and recreates the exception with get_class($e), so SshKeyAddCommand's catch (BadResponseException) still matches wrapped client errors.
• addSshKey() deliberately rethrows the raw 409 so the command's duplicate-key message still fires, while all other statuses get API error details appended.
• The circular-pagination guard keys $visitedUrls on the resolved absolute URL, so a relative next href pointing back at the first page is detected.
• The anonymous test subclass matches Api::__construct(config, cache, output, io, ...) and the getHttpClient(): ClientInterface / getMyUserId(bool): string signatures it overrides.
• listAccountKeyFingerprints()' array_filter result keeps string values only, so in_array() in findIdentityMatchingPublicKeys() is unaffected by the non-list keys.

Verification. The diff adds legacy/tests/Service/ApiSshKeyTest.php (pagination, 404→null, error details, cache invalidation) and one SshKeyTest case, run by the legacy-php CI job via ./scripts/test/unit.sh alongside make lint-phpstan/make lint-php-cs-fixer; note the new SshKeyTest case does not actually exercise the inactive-key filter (see finding), and no test covers SshKeyListCommand's new active column.

Review details

• Commits: 8672834..488f1d4
• Model: claude-opus-5

---

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

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 2 minor points

🔁 Incremental · 4 files reviewed

🔍 What this review checked

• Removing the $key->active ? guard in SshKeyListCommand makes the local-path lookup run for inactive keys, so a matching ~/.ssh key is now shown instead of 'Not found'.
• active was added to both $tableHeader and $defaultColumns, so the new column renders and is selectable via --columns.
• The circular-pagination test's next href /api/users/user-id/ssh-keys resolves to exactly the first URL built by sshKeysUrl() from base https://api.example.test/api, so the RuntimeException is really triggered.
• PLATFORMSH_CLI_HOME/PLATFORMSH_CLI_API_URL/PLATFORMSH_CLI_SESSION_ID match application.env_prefix in legacy/config.yaml, and the added assertions fail loudly if the overrides stop being honoured.
• SshKeyListCommandTest can construct the command without a container: CommandBase::run() never touches the unset private $config, and readonly services like SshKey are already doubled elsewhere in the suite.

Verification. This increment adds legacy/tests/Command/SshKey/SshKeyListCommandTest.php covering the inactive-key row and path, and tightens the env overrides in ApiSshKeyTest/SshKeyTest; all run in the legacy-php CI job (./scripts/test/unit.sh, plus phpstan and php-cs-fixer lint steps) on ubuntu-latest only.

Review details

• Commits: 488f1d4..a9e4892
• Model: claude-opus-5

🔵 Minor points

• legacy/tests/Service/SshKeyTest.php:84 — assertSame($this->tempDir, $config->getHomeDirectory()) compares the raw tempnam() path against Config::getHomeDirectory(), which returns realpath($value). HasTempDirTrait::tempDirSetUp() builds the directory under sys_get_temp_dir(), which on macOS is /var/folders/... — a symlink to /private/var/folders/.... The assertion therefore fails on any platform where the temp dir path contains a symlink (macOS make test locally), even though the underlying behaviour is correct; the GitHub Actions legacy-php job runs on ubuntu-latest and does not exercise this. Comparing realpath($this->tempDir) would be portable.
• legacy/tests/Service/SshKeyTest.php:87 — testInactiveAccountKeysAreNotMatched depends on SshKey::listPublicKeys(), whose result is held in a method-level static $publicKeyList shared by every SshKey instance in the PHP process. The test only proves the active filter because no earlier test in the suite calls a real SshKey::findIdentityMatchingPublicKeys()/hasLocalKey(); as soon as one does (PHPUnit runs all test classes in one process, without process isolation), the glob of $tempDir/.ssh is skipped, hasLocalKey() returns false for the wrong reason and the test passes even if the array_filter(..., fn($k) => $k->active) is deleted.

---

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

[upsun-dispatch]

Warning

Changes suggested — 🟡 1 warning · 🔵 2 minor points · 2 still open

🔍 Full review · 11 files reviewed

Outstanding from earlier reviews:

• 🔵 legacy/tests/Service/SshKeyTest.php:84: New assertion fails for macOS developers despite correct behaviour. — Line 84 still asserts assertSame($this->tempDir, ...) against getHomeDirectory(), which returns realpath($value) (Config.php:216), so it fails where the temp dir contains a symlink.
• 🔵 legacy/tests/Service/SshKeyTest.php:87: A future test can silently turn this regression guard into a no-op. — listPublicKeys() still uses a method-level static $publicKeyList and the test calls hasLocalKey() without any reset, so an earlier glob in the same process masks the assertion.

Verification

• Relative next hrefs are resolved against the request URL via UriResolver, so ?page=2 correctly becomes the collection URL plus the query.
• addSshKey() and deleteSshKey() clear exactly the cache key getSshKeys() reads ({sessionId}:ssh-keys), so the delete command's follow-up getSshKeys(true) sees the mutation.
• No reference to the old Platformsh\Client\Model\SshKey properties (key_id, title, fingerprint) remains anywhere under legacy/src.
• getPublicKeyFingerprint() emits SHA256: + unpadded base64, the same shape the model's sha256 field carries, and both comparison sites use that field.
• The removed phpstan-baseline entries match the code the diff actually typed (LoginRequiredEvent's (string) $value, choose() receiving array<string,string>).

The diff adds three PHPUnit test files (ApiSshKeyTest covering pagination, caching, 404-to-null and cache invalidation; SshKeyTest covering fingerprint format and the active filter; SshKeyListCommandTest covering the new row shape), and the legacy-php CI job runs php-cs-fixer, phpstan (level 9 with the updated baseline) and the PHPUnit suite over them. Nothing covers the stale-cache duplicate check in ssh-key:add or the 409 branch of that command.

Review details

• Commit: ec2d1ea
• Model: claude-opus-5

---

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

[upsun-dispatch]

Warning

Changes suggested — 🟡 1 warning · 5 still open

🔍 Full review · 11 files reviewed

Outstanding from earlier reviews:

• 🟡 #4098231556 — legacy/src/Command/SshKey/SshKeyAddCommand.php:173: ssh-key:add silently no-ops for up to 10 minutes after an out-of-band deletion. — keyExistsByFingerprint() still calls $this->api->getSshKeys() without reset, so it reads the list cached for 600s.
• 🔵 #4098231566 — legacy/src/Service/Api.php:964: A benign always-present next link turns key listing into a hard error. — On a revisited URL, getSshKeys() still throws 'circular pagination link' and discards the items already collected.
• 🔵 #4098231576 — legacy/src/Command/SshKey/SshKeyAddCommand.php:174: User sees contradictory messages and cannot tell why SSH auth still fails. — keyExistsByFingerprint() still matches on sha256 only and ignores active.
• 🔵 legacy/tests/Service/SshKeyTest.php:84: New assertion fails for macOS developers despite correct behaviour. — SshKeyTest line 84 still compares the raw tempDir against getHomeDirectory(), which returns realpath($value). (first raised)
• 🔵 legacy/tests/Service/SshKeyTest.php:87: A future test can silently turn this regression guard into a no-op. — listPublicKeys() still keeps a method-level static $publicKeyList, and hasLocalKey() calls it without reset. (first raised)

Verification

• Config::getHomeDirectory() and getEnv() read the injected env array under the PLATFORMSH_CLI_ prefix, so the new tests' PLATFORMSH_CLI_HOME/API_URL/SESSION_ID overrides take effect.
• SshKeyListCommand now calls findIdentityMatchingPublicKeys for inactive keys too, and active is one of the default columns.
• After the migration, no code in legacy/src reads the old fingerprint, key_id, title or ssh_keys SshKey fields or the removed Client\Model\SshKey.
• addSshKey and deleteSshKey call clearSshKeysCache() only after a successful request, and logout() still runs flushAll(), which also clears the new {session}:ssh-keys entry.

The new PHPUnit tests (ApiSshKeyTest, SshKeyTest, SshKeyListCommandTest) cover pagination, caching, cache clearing after add/delete, 404 handling, SHA-256 fingerprints and inactive-key filtering. They run in the legacy-php job in .github/workflows/ci.yml (unit.sh, alongside phpstan). No test covers the 409 path in SshKeyAddCommand or SshKeyDeleteCommand.

Review details

• Commit: 8b4bccf
• Model: claude-opus-5-5

---

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

[upsun-dispatch]

Note

Reviewed — No new issues found · 2 still open

🔁 Incremental · 4 files reviewed

Outstanding from earlier reviews:

• 🔵 legacy/tests/Service/SshKeyTest.php:84: New assertion fails for macOS developers despite correct behaviour. — SshKeyTest still asserts assertSame($this->tempDir, $config->getHomeDirectory()), comparing the raw path with the realpath'd one. (first raised)
• 🔵 legacy/tests/Service/SshKeyTest.php:87: A future test can silently turn this regression guard into a no-op. — SshKey::listPublicKeys() still keeps its list in a method-level static $publicKeyList, so the test depends on test order. (first raised)

Verification

• findKeyByFingerprint() now calls getSshKeys(true), so the duplicate check always reads from the API and never from the 600s cache.
• When the matching account key is inactive, ssh-key:add now prints an 'inactive' error and returns 1 without calling addSshKey.
• getSshKeys() now stops when the resolved next URL equals the page just fetched. It still throws only when a link revisits an earlier page.
• In testRejectsCircularPagination, the second page links back to the first page's URL, so the test exercises the revisit branch and not the self-link branch.

This increment adds SshKeyAddCommandTest (cache bypass, inactive-key and active-key paths) and ApiSshKeyTest::testStopsAtANextLinkToTheCurrentPage, and updates testRejectsCircularPagination. These tests run in the repository's legacy PHPUnit job. No test covers resolving a next link against a base URL whose path differs from the request path.

Review details

• Commits: 8b4bccf..a67cef3
• Model: claude-opus-5-5

---

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

[upsun-dispatch]

Note

Reviewed — No new issues found · 2 still open

🔁 Incremental · 4 files reviewed

Outstanding from earlier reviews:

• 🔵 legacy/tests/Service/SshKeyTest.php:84: New assertion fails for macOS developers despite correct behaviour. — SshKeyTest line 84 still asserts assertSame($this->tempDir, $config->getHomeDirectory()) without realpath(), so it fails where the temp dir is behind a symlink. (first raised)
• 🔵 legacy/tests/Service/SshKeyTest.php:87: A future test can silently turn this regression guard into a no-op. — SshKey::listPublicKeys() still caches the glob in a method-level static, so the test depends on no earlier test having populated it. (first raised)

Verification

• Legacy-source URLs (/me, /ssh_keys, /ssh_keys/{id}) use getApiUrl(), matching PlatformClient::apiUrl() whenever api_url is configured.
• clearSshKeysCache() deletes both per-source cache keys, so a source switch cannot serve a stale list from the other source's key.
• Numeric legacy key IDs survive the interactive delete prompt: QuestionHelper::choose() casts the chosen key back to string before getSshKey(string).
• Non-404 errors from getSshKeySource() are wrapped as ApiResponseException (a RequestException/BadResponseException), so ssh-key:add's 409 handling and the propagation test hold.

ApiSshKeyTest gains Guzzle-mock tests with request-history assertions for source fetch/caching, 404→accounts fallback, unknown-source rejection, legacy list/add/delete/get URLs and payloads, and source refetch on reset. The command tests don't cover the legacy source, and nothing covers a 200 non-JSON /ssh-key-source response.

Review details

• Commits: a67cef3..90fbc96
• Model: claude-opus-5-5

---

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