fix(rollback): escape commit SHA to prevent shell injection

Properly escape commit SHA using escapeshellarg() before passing it
to shell commands. Add comprehensive tests for git commit rollback
scenarios including shallow clone, fallback behavior, and HEAD handling.
This commit is contained in:
Andras Bacsai 2026-02-27 23:26:31 +01:00
parent 530037c213
commit a565fc3b36
2 changed files with 61 additions and 11 deletions

View file

@ -1096,12 +1096,13 @@ public function setGitImportSettings(string $deployment_uuid, string $git_clone_
$commitToUse = $commit ?? $this->git_commit_sha;
if ($commitToUse !== 'HEAD') {
$escapedCommit = escapeshellarg($commitToUse);
// If shallow clone is enabled and we need a specific commit,
// we need to fetch that specific commit with depth=1
if ($isShallowCloneEnabled) {
$git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && GIT_SSH_COMMAND=\"ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null\" git fetch --depth=1 origin {$commitToUse} && git -c advice.detachedHead=false checkout {$commitToUse} >/dev/null 2>&1";
$git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && GIT_SSH_COMMAND=\"ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null\" git fetch --depth=1 origin {$escapedCommit} && git -c advice.detachedHead=false checkout {$escapedCommit} >/dev/null 2>&1";
} else {
$git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && GIT_SSH_COMMAND=\"ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null\" git -c advice.detachedHead=false checkout {$commitToUse} >/dev/null 2>&1";
$git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && GIT_SSH_COMMAND=\"ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null\" git -c advice.detachedHead=false checkout {$escapedCommit} >/dev/null 2>&1";
}
}
if ($this->settings->is_git_submodules_enabled) {

View file

@ -11,7 +11,7 @@
uses(RefreshDatabase::class);
describe('Application Rollback', function () {
test('setGitImportSettings uses passed commit instead of application git_commit_sha', function () {
beforeEach(function () {
$team = Team::factory()->create();
$project = Project::create([
'team_id' => $team->id,
@ -25,31 +25,80 @@
]);
$server = Server::factory()->create(['team_id' => $team->id]);
// Create application with git_commit_sha = 'HEAD' (default - use latest)
$application = Application::factory()->create([
$this->application = Application::factory()->create([
'environment_id' => $environment->id,
'destination_id' => $server->id,
'git_commit_sha' => 'HEAD',
]);
});
// Create application settings
test('setGitImportSettings uses passed commit instead of application git_commit_sha', function () {
ApplicationSetting::create([
'application_id' => $application->id,
'application_id' => $this->application->id,
'is_git_shallow_clone_enabled' => false,
]);
// The rollback commit SHA we want to deploy
$rollbackCommit = 'abc123def456';
// This should use the passed commit, not the application's git_commit_sha
$result = $application->setGitImportSettings(
$result = $this->application->setGitImportSettings(
deployment_uuid: 'test-uuid',
git_clone_command: 'git clone',
public: true,
commit: $rollbackCommit
);
// Assert: The command should checkout the ROLLBACK commit
expect($result)->toContain($rollbackCommit);
});
test('setGitImportSettings with shallow clone fetches specific commit', function () {
ApplicationSetting::create([
'application_id' => $this->application->id,
'is_git_shallow_clone_enabled' => true,
]);
$rollbackCommit = 'abc123def456';
$result = $this->application->setGitImportSettings(
deployment_uuid: 'test-uuid',
git_clone_command: 'git clone',
public: true,
commit: $rollbackCommit
);
expect($result)
->toContain('git fetch --depth=1 origin')
->toContain($rollbackCommit);
});
test('setGitImportSettings falls back to git_commit_sha when no commit passed', function () {
$this->application->update(['git_commit_sha' => 'def789abc012']);
ApplicationSetting::create([
'application_id' => $this->application->id,
'is_git_shallow_clone_enabled' => false,
]);
$result = $this->application->setGitImportSettings(
deployment_uuid: 'test-uuid',
git_clone_command: 'git clone',
public: true,
);
expect($result)->toContain('def789abc012');
});
test('setGitImportSettings does not append checkout when commit is HEAD', function () {
ApplicationSetting::create([
'application_id' => $this->application->id,
'is_git_shallow_clone_enabled' => false,
]);
$result = $this->application->setGitImportSettings(
deployment_uuid: 'test-uuid',
git_clone_command: 'git clone',
public: true,
);
expect($result)->not->toContain('advice.detachedHead=false checkout');
});
});