fix: re-sync SSH deploy keys when updating site source control#1083
fix: re-sync SSH deploy keys when updating site source control#1083urufudev wants to merge 4 commits into
Conversation
|
@urufudev thanks for the PR. I'd appreceate test coverage for this |
There was a problem hiding this comment.
Pull request overview
Fixes deploy-key drift when a site's Source Control provider/token is changed: the old provider's deploy key is removed and the existing site SSH key is re-registered with the new provider, with the resulting deploy_key_id persisted in type_data so DeleteSite can clean it up later.
Changes:
- In
UpdateSourceControl::update, attempt to delete the previous deploy key, swap source control (and unset the cached relation), then re-register the SSH key on the new provider and store the newdeploy_key_id. - Failures in the old-key deletion and new-key registration are caught and logged via
Log::warningrather than aborting the update. - Adds three feature tests covering old-key deletion, new-key registration, and resilience when the old-key deletion fails; also makes minor formatting-only edits to existing tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/Actions/Site/UpdateSourceControl.php | Adds delete-old / re-register-new deploy key logic around the existing source control swap, with try/catch + logging. |
| tests/Feature/SitesTest.php | Adds three tests for the new behavior; also reformats Http::fake calls and arrow-function spacing in existing tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| if ($site->sourceControl && isset($site->type_data['deploy_key_id'])) { | ||
| $site->sourceControl->provider()->deleteDeployKey( | ||
| $site->type_data['deploy_key_id'], | ||
| $site->repository, | ||
| ); | ||
| } | ||
| } catch (Throwable $e) { | ||
| Log::warning('Failed to delete previous deploy key during source control update', [ | ||
| 'site' => $site->id, | ||
| 'error' => $e->getMessage(), | ||
| ]); | ||
| } |
There was a problem hiding this comment.
Good catch! Reordered the logic to validate the new source control first before deleting the old deploy key, preventing orphaned state if validation fails.
When a user updates the Source Control provider/token for an existing site (via Settings → Source Controls), the SSH deploy key is not automatically re-registered in the repository. This leads to Permission denied (publickey) errors during subsequent deployments.
This PR updates UpdateSourceControl.php to:
Changes:
Modified app/Actions/Site/UpdateSourceControl.php to include the logic for deleting the old key and creating the new one using the existing Site and SourceControl providers.
Verification: