feat: Add configurable SSH port for source control providers and SSH …#1028
feat: Add configurable SSH port for source control providers and SSH …#1028khmish wants to merge 3 commits intovitodeploy:3.xfrom
Conversation
…cloning. feat: Add configurable SSH port for source control providers and SSH cloning.
…odels and provider logic to use it.
There was a problem hiding this comment.
Pull request overview
Adds support for configuring a custom SSH port on source control connections (notably self-hosted providers) and wires that port into the SSH clone flow.
Changes:
- Add
portcolumn tosource_controlsand expose it on theSourceControlmodel. - Introduce
getSshPort()on source control providers and pass the port into the SSH clone script (includingssh-keyscan -p). - Add a GitLab UI field and validation rule for the SSH port.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| resources/views/ssh/git/clone.blade.php | Writes SSH config/known_hosts entries and now includes a configurable SSH port during cloning. |
| database/migrations/2026_02_12_074800_add_port_to_source_controls_table.php | Adds nullable port column to source_controls. |
| app/SourceControlProviders/SourceControlProvider.php | Extends provider interface with getSshPort(). |
| app/SourceControlProviders/Gitlab.php | Adds validation rule for a port input field. |
| app/SourceControlProviders/AbstractSourceControlProvider.php | Implements default getSshPort() logic for providers. |
| app/SSH/OS/Git.php | Passes computed SSH port into the clone view. |
| app/Providers/SourceControlServiceProvider.php | Adds GitLab dynamic form field for SSH port. |
| app/Models/SourceControl.php | Makes port fillable and casts it to integer. |
| app/Actions/SourceControl/ConnectSourceControl.php | Persists port when connecting a source control provider. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| public function getSshPort(): ?int | ||
| { | ||
| return (int) $this->sourceControl->port ?: ((int) ($this->sourceControl->provider_data['port'] ?? null) ?: null); |
There was a problem hiding this comment.
getSshPort() uses a nested cast/ternary that can return surprising results and doesn't enforce valid SSH port bounds. Consider normalizing with explicit null checks (prefer $this->sourceControl->port first) and returning null for out-of-range values so callers can safely default to 22.
| return (int) $this->sourceControl->port ?: ((int) ($this->sourceControl->provider_data['port'] ?? null) ?: null); | |
| $port = $this->sourceControl->port; | |
| if ($port === null && isset($this->sourceControl->provider_data['port'])) { | |
| $port = $this->sourceControl->provider_data['port']; | |
| } | |
| if ($port === null) { | |
| return null; | |
| } | |
| $port = (int) $port; | |
| if ($port < 1 || $port > 65535) { | |
| return null; | |
| } | |
| return $port; |
| IdentityFile=~/.ssh/{{ $key }}" >> ~/.ssh/config | ||
| Hostname {{ $host }} | ||
| Port {{ $port }} | ||
| IdentityFile=~/.ssh/{{ $key }}" >> ~/.ssh/config |
There was a problem hiding this comment.
In ~/.ssh/config, IdentityFile is normally specified as IdentityFile <path> (space-separated). Using IdentityFile=... risks being parsed as an unknown option and can make SSH ignore/fail the config, breaking cloning. Update the rendered config line to use the standard IdentityFile syntax.
| IdentityFile=~/.ssh/{{ $key }}" >> ~/.ssh/config | |
| IdentityFile ~/.ssh/{{ $key }}" >> ~/.ssh/config |
| public function up(): void | ||
| { | ||
| Schema::table('source_controls', function (Blueprint $table): void { | ||
| $table->unsignedInteger('port')->nullable()->after('url'); |
There was a problem hiding this comment.
This migration uses unsignedInteger for an SSH port, but valid ports only range up to 65535. Using unsignedSmallInteger better models the domain and prevents accidental storage of out-of-range values.
| $table->unsignedInteger('port')->nullable()->after('url'); | |
| $table->unsignedSmallInteger('port')->nullable()->after('url'); |
| 'path' => $path ?? $site->path, | ||
| 'branch' => $site->branch, | ||
| 'key' => $site->getSshKeyName(), | ||
| 'port' => $site->sourceControl?->provider()?->getSshPort() ?? 22, |
There was a problem hiding this comment.
New behavior (passing a configurable SSH port into the clone script) isn’t covered by existing SourceControl connection tests. Consider adding a test that connects/creates a Gitlab source control with a non-22 port and asserts it’s persisted and used (or at least surfaced via getSshPort()), with defaulting to 22 when unset.
|
|
||
| public function getRepos(bool $useCache = true): array; | ||
|
|
||
| public function getBranches(string $repo, bool $useCache = true): array; |
There was a problem hiding this comment.
Adding getSshPort() to the SourceControlProvider interface is a backwards-incompatible change for any external/custom source-control providers registered via the plugin system. If this project supports third-party providers, consider providing a non-breaking default (e.g. via an optional trait/base class contract or a separate interface) or clearly documenting this as a breaking change.
| public function getBranches(string $repo, bool $useCache = true): array; | |
| public function getBranches(string $repo, bool $useCache = true): array; | |
| } | |
| interface SshAwareSourceControlProvider extends SourceControlProvider | |
| { |
| 'profile' => $input['name'], | ||
| 'url' => isset($input['url']) && $input['url'] ? $input['url'] : null, | ||
| 'port' => isset($input['port']) && $input['port'] ? (int) $input['port'] : null, | ||
| 'project_id' => isset($input['global']) && $input['global'] ? null : $user->currentProject?->id, |
There was a problem hiding this comment.
port is persisted from user input but there is no base validation for it. For non-Gitlab providers, providerRules() won't include port, yet this code will still store the value (including negatives / >65535), which can later break SSH cloning. Add a shared validation rule for port (e.g. nullable|integer|min:1|max:65535) in validate() or only allow persisting it when the active provider explicitly supports it.
| 'url:http,https', | ||
| 'ends_with:/', | ||
| ], | ||
| 'port' => 'nullable|integer', |
There was a problem hiding this comment.
The new port field is optional but currently lacks bounds validation. SSH ports must be within 1-65535; allowing 0/negative/large values will cause connection failures (and can store invalid data in an unsignedInteger column). Consider changing the rule to include min/max (and keep it consistent with server port validation elsewhere in the codebase).
| 'port' => 'nullable|integer', | |
| 'port' => 'nullable|integer|min:1|max:65535', |
…cloning.
feat: Add configurable SSH port for source control providers and SSH cloning.