fix(git): ensure ssh credentials are propagated to submodule operations#8900
fix(git): ensure ssh credentials are propagated to submodule operations#8900andrasbacsai wants to merge 1 commit intonextfrom
Conversation
- Add gitSshCommand parameter to setGitImportSettings and buildGitCheckoutCommand - Extract hardcoded ssh commands to variables for consistency - Apply custom ssh credentials to all git operations including submodule sync/update - Configure git url rewriting for github token-based authentication with submodules - Add comprehensive test suite for submodule credential propagation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis change introduces an optional ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Models/Application.php`:
- Around line 1305-1317: The code currently writes a token rewrite into global
Git config via $gitConfigCommand (used when exec_in_docker is false), which
persists credentials; instead, remove the use of --global and do not push a
global config entry—use a per-command override with git -c
url.<tokenUrl>.insteadOf=<hostUrl> applied to the clone command: build the
config fragment from the same escapedTokenUrl/escapedHostUrl values and prepend
or inject it into $git_clone_command (or the string sent to $commands->push) so
the token is only in the environment of that single git invocation; keep the
existing executeInDocker path unchanged and ensure
$fullRepoUrl/$escapedRepoUrl/$escapedBaseDir behavior remains correct when
modifying $git_clone_command.
- Around line 1307-1309: The current git credential rewrite (building
$escapedTokenUrl / $escapedHostUrl and $gitConfigCommand) only covers https
submodule URLs so SSH submodules (git@... or ssh://git@...) will bypass the
GitHub App token; fix by normalizing SSH submodule URLs to HTTPS before
submodule init/update: apply the same .gitmodules SSH-to-https sed/replace logic
used for public repos (the existing git@... -> https conversion) when handling
private GitHub App repos, or perform an explicit rewrite of git@ and ssh://git@
patterns in .gitmodules prior to running git submodule init/update so the
token-based git config rewrite will match. Ensure this normalization runs in the
same code path that constructs $escapedTokenUrl/$gitConfigCommand so token auth
applies to submodules.
In `@openapi.yaml`:
- Around line 2114-2120: Extract the duplicated query parameter block named
docker_cleanup into components.parameters and replace inline definitions with a
$ref to it; specifically create components.parameters.docker_cleanup with name:
docker_cleanup, in: query, required: false, the existing description and schema
(type: boolean, default: true), then update all places that currently define the
inline parameter (the repeated blocks used by the various stop and delete
endpoints) to reference it via $ref: '#/components/parameters/docker_cleanup'.
In `@tests/Unit/GitSubmoduleCredentialTest.php`:
- Around line 21-121: Add a unit test that exercises generateGitImportCommands()
for a private source deployment to cover the private GitHub App rewrite path and
url.*.insteadOf logic: create a new test similar to the existing
setGitImportSettings tests, configure the application/state to represent a
private source (set the source/repository fields and a flag indicating
private/private-app rewrite as your code expects), invoke
setGitImportSettings(...) or call generateGitImportCommands() directly, and
assert the returned shell contains the url.*.insteadOf rewrite line and the
SSH-form .gitmodules submodule update (e.g., contains 'url.*.insteadOf' and
'GIT_SSH_COMMAND="... " git submodule update --init --recursive'). Ensure you
set any flags used in the implementation (like is_git_submodules_enabled,
is_git_lfs_enabled, git_commit_sha) so the branch with .gitmodules and insteadOf
is executed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: be79c078-b608-421b-beef-cbf47bbe10ce
📒 Files selected for processing (4)
app/Models/Application.phpopenapi.jsonopenapi.yamltests/Unit/GitSubmoduleCredentialTest.php
| $github_access_token = generateGithubInstallationToken($this->source); | ||
| // Configure git to rewrite URLs with the token so submodules on the same host authenticate correctly | ||
| $escapedTokenUrl = escapeshellarg("{$source_html_url_scheme}://x-access-token:{$github_access_token}@{$source_html_url_host}/"); | ||
| $escapedHostUrl = escapeshellarg("{$source_html_url_scheme}://{$source_html_url_host}/"); | ||
| $gitConfigCommand = "git config --global url.{$escapedTokenUrl}.insteadOf {$escapedHostUrl}"; | ||
| if ($exec_in_docker) { | ||
| $commands->push(executeInDocker($deployment_uuid, $gitConfigCommand)); | ||
| $repoUrl = "$source_html_url_scheme://x-access-token:$github_access_token@$source_html_url_host/{$customRepository}.git"; | ||
| $escapedRepoUrl = escapeshellarg($repoUrl); | ||
| $git_clone_command = "{$git_clone_command} {$escapedRepoUrl} {$escapedBaseDir}"; | ||
| $fullRepoUrl = $repoUrl; | ||
| } else { | ||
| $commands->push($gitConfigCommand); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the file and examine the specific lines and surrounding context
sed -n '1300,1320p' app/Models/Application.phpRepository: coollabsio/coolify
Length of output: 1790
🏁 Script executed:
# Check if there's any cleanup of global git config
rg 'git.*config.*--global.*--unset' app/Models/Application.phpRepository: coollabsio/coolify
Length of output: 44
🏁 Script executed:
# Look at the loadComposeFile function and how it calls generateGitImportCommands
sed -n '1540,1555p' app/Models/Application.phpRepository: coollabsio/coolify
Length of output: 1105
🏁 Script executed:
# Check the full generateGitImportCommands function signature and context
sed -n '1200,1250p' app/Models/Application.phpRepository: coollabsio/coolify
Length of output: 2483
🏁 Script executed:
# Search for any cleanup mechanisms after git config is set
rg -A 10 'git config --global url' app/Models/Application.phpRepository: coollabsio/coolify
Length of output: 980
Remove --global flag; use git -c override instead to avoid token residue on the host.
Line 1309 writes the GitHub installation token into global git config, and this branch executes on the actual host machine (called from loadComposeFile() at line 1547 with exec_in_docker: false). After deployment finishes, that credential stays in the host's global config—basically a taco left in your server's gluten-free kitchen. No cleanup happens afterward. Short-lived tokens don't need permanent storage; use git -c url.<tokenUrl>.insteadOf=<hostUrl> to scope the rewrite to just that command instead.
🧰 Tools
🪛 PHPMD (2.15.0)
[error] 1305-1305: The variable $github_access_token is not named in camelCase. (undefined)
(CamelCaseVariableName)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Models/Application.php` around lines 1305 - 1317, The code currently
writes a token rewrite into global Git config via $gitConfigCommand (used when
exec_in_docker is false), which persists credentials; instead, remove the use of
--global and do not push a global config entry—use a per-command override with
git -c url.<tokenUrl>.insteadOf=<hostUrl> applied to the clone command: build
the config fragment from the same escapedTokenUrl/escapedHostUrl values and
prepend or inject it into $git_clone_command (or the string sent to
$commands->push) so the token is only in the environment of that single git
invocation; keep the existing executeInDocker path unchanged and ensure
$fullRepoUrl/$escapedRepoUrl/$escapedBaseDir behavior remains correct when
modifying $git_clone_command.
| $escapedTokenUrl = escapeshellarg("{$source_html_url_scheme}://x-access-token:{$github_access_token}@{$source_html_url_host}/"); | ||
| $escapedHostUrl = escapeshellarg("{$source_html_url_scheme}://{$source_html_url_host}/"); | ||
| $gitConfigCommand = "git config --global url.{$escapedTokenUrl}.insteadOf {$escapedHostUrl}"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the code around lines 1307-1309 with broader context
head -n 1350 app/Models/Application.php | tail -n 100 | cat -nRepository: coollabsio/coolify
Length of output: 6435
🏁 Script executed:
# Search for SSH-related handling in the Application.php file
rg -n "ssh|git@|insteadOf" app/Models/Application.php -A 2 -B 2Repository: coollabsio/coolify
Length of output: 7173
🏁 Script executed:
# Check if there's any .gitmodules handling or submodule configuration
rg -n "gitmodules|submodule" app/Models/Application.php -A 3 -B 3Repository: coollabsio/coolify
Length of output: 3290
🏁 Script executed:
# Look for any environment variable or config related to SSH keys or token
rg -n "github.*token|access.token" app/Models/Application.php -i -A 2 -B 2Repository: coollabsio/coolify
Length of output: 2600
SSH submodules will skip the GitHub App token authentication.
The git config url.{token}.insteadOf rewrite only matches https://{host}/... URLs. Any submodules declared as git@{host}:owner/repo.git or ssh://git@{host}/owner/repo.git in .gitmodules will use SSH during git submodule update and bypass this token-based authentication entirely.
The codebase already handles this for public repos (line 1116 converts git@... to https:// in .gitmodules), but private GitHub App repos lack the equivalent SSH URL normalization. Either add the same .gitmodules sed rewrite for private repos, or normalize SSH URLs before submodule init.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Models/Application.php` around lines 1307 - 1309, The current git
credential rewrite (building $escapedTokenUrl / $escapedHostUrl and
$gitConfigCommand) only covers https submodule URLs so SSH submodules (git@...
or ssh://git@...) will bypass the GitHub App token; fix by normalizing SSH
submodule URLs to HTTPS before submodule init/update: apply the same .gitmodules
SSH-to-https sed/replace logic used for public repos (the existing git@... ->
https conversion) when handling private GitHub App repos, or perform an explicit
rewrite of git@ and ssh://git@ patterns in .gitmodules prior to running git
submodule init/update so the token-based git config rewrite will match. Ensure
this normalization runs in the same code path that constructs
$escapedTokenUrl/$gitConfigCommand so token auth applies to submodules.
| test('setGitImportSettings uses provided gitSshCommand for submodule update', function () { | ||
| $sshCommand = 'ssh -o ConnectTimeout=30 -p 22 -o Port=22 -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /root/.ssh/id_rsa'; | ||
|
|
||
| $result = $this->application->setGitImportSettings( | ||
| deployment_uuid: 'test-uuid', | ||
| git_clone_command: 'git clone', | ||
| public: false, | ||
| gitSshCommand: $sshCommand | ||
| ); | ||
|
|
||
| expect($result) | ||
| ->toContain('GIT_SSH_COMMAND="'.$sshCommand.'" git submodule update --init --recursive') | ||
| ->toContain('git submodule sync'); | ||
| }); | ||
|
|
||
| test('setGitImportSettings uses default ssh command when no gitSshCommand provided', function () { | ||
| $result = $this->application->setGitImportSettings( | ||
| deployment_uuid: 'test-uuid', | ||
| git_clone_command: 'git clone', | ||
| public: false, | ||
| ); | ||
|
|
||
| expect($result) | ||
| ->toContain('GIT_SSH_COMMAND="ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null" git submodule update --init --recursive'); | ||
| }); | ||
|
|
||
| test('setGitImportSettings uses provided gitSshCommand for fetch and checkout', function () { | ||
| $this->application->git_commit_sha = 'abc123def456'; | ||
| $sshCommand = 'ssh -o ConnectTimeout=30 -p 22 -o Port=22 -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /root/.ssh/id_rsa'; | ||
|
|
||
| $result = $this->application->setGitImportSettings( | ||
| deployment_uuid: 'test-uuid', | ||
| git_clone_command: 'git clone', | ||
| public: false, | ||
| gitSshCommand: $sshCommand | ||
| ); | ||
|
|
||
| expect($result) | ||
| ->toContain('GIT_SSH_COMMAND="'.$sshCommand.'" git -c advice.detachedHead=false checkout'); | ||
| }); | ||
|
|
||
| test('setGitImportSettings uses provided gitSshCommand for shallow fetch', function () { | ||
| $this->application->git_commit_sha = 'abc123def456'; | ||
| $this->application->settings->is_git_shallow_clone_enabled = true; | ||
| $sshCommand = 'ssh -o ConnectTimeout=30 -p 22 -o Port=22 -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /root/.ssh/id_rsa'; | ||
|
|
||
| $result = $this->application->setGitImportSettings( | ||
| deployment_uuid: 'test-uuid', | ||
| git_clone_command: 'git clone', | ||
| public: false, | ||
| gitSshCommand: $sshCommand | ||
| ); | ||
|
|
||
| expect($result) | ||
| ->toContain('GIT_SSH_COMMAND="'.$sshCommand.'" git fetch --depth=1 origin'); | ||
| }); | ||
|
|
||
| test('setGitImportSettings uses provided gitSshCommand for lfs pull', function () { | ||
| $this->application->settings->is_git_lfs_enabled = true; | ||
| $sshCommand = 'ssh -o ConnectTimeout=30 -p 22 -i /root/.ssh/id_rsa'; | ||
|
|
||
| $result = $this->application->setGitImportSettings( | ||
| deployment_uuid: 'test-uuid', | ||
| git_clone_command: 'git clone', | ||
| public: false, | ||
| gitSshCommand: $sshCommand | ||
| ); | ||
|
|
||
| expect($result) | ||
| ->toContain('GIT_SSH_COMMAND="'.$sshCommand.'" git lfs pull'); | ||
| }); | ||
|
|
||
| test('buildGitCheckoutCommand includes GIT_SSH_COMMAND for submodule update when provided', function () { | ||
| $sshCommand = 'ssh -o ConnectTimeout=30 -p 22 -i /root/.ssh/id_rsa'; | ||
|
|
||
| $method = new ReflectionMethod($this->application, 'buildGitCheckoutCommand'); | ||
| $result = $method->invoke($this->application, 'main', $sshCommand); | ||
|
|
||
| expect($result) | ||
| ->toContain("git checkout 'main'") | ||
| ->toContain('GIT_SSH_COMMAND="'.$sshCommand.'" git submodule update --init --recursive'); | ||
| }); | ||
|
|
||
| test('buildGitCheckoutCommand uses default ssh command for submodule update when none provided', function () { | ||
| $method = new ReflectionMethod($this->application, 'buildGitCheckoutCommand'); | ||
| $result = $method->invoke($this->application, 'main'); | ||
|
|
||
| expect($result) | ||
| ->toContain('GIT_SSH_COMMAND="ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null" git submodule update --init --recursive'); | ||
| }); | ||
|
|
||
| test('buildGitCheckoutCommand omits submodule update when submodules disabled', function () { | ||
| $this->application->settings->is_git_submodules_enabled = false; | ||
|
|
||
| $method = new ReflectionMethod($this->application, 'buildGitCheckoutCommand'); | ||
| $result = $method->invoke($this->application, 'main'); | ||
|
|
||
| expect($result) | ||
| ->toContain("git checkout 'main'") | ||
| ->not->toContain('submodule'); | ||
| }); |
There was a problem hiding this comment.
Good SSH coverage, but the private GitHub App rewrite path is still untested.
All current cases exercise the gitSshCommand plumbing, but none hit generateGitImportCommands() for a private source deployment. That leaves the new url.*.insteadOf branch—and the SSH-form .gitmodules case above—without regression protection.
As per coding guidelines **/*Test.php: Every code change must be programmatically tested; write a new test or update an existing test, then run affected tests to ensure they pass.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Unit/GitSubmoduleCredentialTest.php` around lines 21 - 121, Add a unit
test that exercises generateGitImportCommands() for a private source deployment
to cover the private GitHub App rewrite path and url.*.insteadOf logic: create a
new test similar to the existing setGitImportSettings tests, configure the
application/state to represent a private source (set the source/repository
fields and a flag indicating private/private-app rewrite as your code expects),
invoke setGitImportSettings(...) or call generateGitImportCommands() directly,
and assert the returned shell contains the url.*.insteadOf rewrite line and the
SSH-form .gitmodules submodule update (e.g., contains 'url.*.insteadOf' and
'GIT_SSH_COMMAND="... " git submodule update --init --recursive'). Ensure you
set any flags used in the implementation (like is_git_submodules_enabled,
is_git_lfs_enabled, git_commit_sha) so the branch with .gitmodules and insteadOf
is executed.
1 participant
Summary
gitSshCommandparameter tosetGitImportSettings()to propagate SSH credentials through all git operationsbuildGitCheckoutCommand()to accept custom SSH commands for submodule operationsDetails
Fixes #2731: Git submodule cloning was failing for private repositories because authentication credentials were not being passed to
git submodule updateoperations.For GitHub-authenticated deployments, a git config command now rewrites HTTPS URLs to include the access token before any clone/update occurs. For deploy key and custom deployments, the SSH command with the private key is now consistently passed to all git operations including submodule updates.
This ensures submodules in private repositories can authenticate successfully during deployment.
Fixes #2731