Skip to content

fix(deployment): resolve intermittent pre-deployment command failures #9165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andrasbacsai merged 2 commits into from
Mar 31, 2026
+223 −58
Merged

fix(deployment): resolve intermittent pre-deployment command failures #9165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
811ee5d
refactor(jobs): extract container resolution logic for deployment com…
andrasbacsai Mar 25, 2026
2abb073
Merge remote-tracking branch 'origin/next' into 9076-investigate-pre-…
andrasbacsai Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 107 additions & 58 deletions app/Jobs/ApplicationDeploymentJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4036,46 +4036,91 @@ private function validateContainerName(string $value): string
return $value;
}

/**
* Resolve which container to execute a deployment command in.
*
* For single-container apps, returns the sole container.
* For multi-container apps, matches by the user-specified container name.
* If no container name is specified for multi-container apps, logs available containers and returns null.
*/
private function resolveCommandContainer(Collection $containers, ?string $specifiedContainerName, string $commandType): ?array
{
if ($containers->count() === 0) {
return null;
}

if ($containers->count() === 1) {
return $containers->first();
}

// Multi-container: require a container name to be specified
if (empty($specifiedContainerName)) {
$available = $containers->map(fn ($c) => data_get($c, 'Names'))->implode(', ');
$this->application_deployment_queue->addLogEntry(
"{$commandType} command: Multiple containers found but no container name specified. Available: {$available}"
);

return null;
}

// Multi-container: match by specified name prefix
$prefix = $specifiedContainerName.'-'.$this->application->uuid;
foreach ($containers as $container) {
$containerName = data_get($container, 'Names');
if (str_starts_with($containerName, $prefix)) {
return $container;
}
}

// No match found — log available containers to help the user debug
$available = $containers->map(fn ($c) => data_get($c, 'Names'))->implode(', ');
$this->application_deployment_queue->addLogEntry(
"{$commandType} command: Container '{$specifiedContainerName}' not found. Available: {$available}"
);

return null;
}

private function run_pre_deployment_command()
{
if (empty($this->application->pre_deployment_command)) {
return;
}
$containers = getCurrentApplicationContainerStatus($this->server, $this->application->id, $this->pull_request_id);
if ($containers->count() == 0) {
$this->application_deployment_queue->addLogEntry('Pre-deployment command: No running containers found. Skipping.');

return;
}
$this->application_deployment_queue->addLogEntry('Executing pre-deployment command (see debug log for output/errors).');

foreach ($containers as $container) {
$containerName = data_get($container, 'Names');
if ($containerName) {
$this->validateContainerName($containerName);
}
if ($containers->count() == 1 || str_starts_with($containerName, $this->application->pre_deployment_command_container.'-'.$this->application->uuid)) {
// Security: pre_deployment_command is intentionally treated as arbitrary shell input.
// Users (team members with deployment access) need full shell flexibility to run commands
// like "php artisan migrate", "npm run build", etc. inside their own application containers.
// The trust boundary is at the application/team ownership level — only authenticated team
// members can set these commands, and execution is scoped to the application's own container.
// The single-quote escaping here prevents breaking out of the sh -c wrapper, but does not
// restrict the command itself. Container names are validated separately via validateContainerName().
// Newlines are normalized to spaces to prevent injection via SSH heredoc transport
// (matches the pattern used for health_check_command at line ~2824).
$preCommand = str_replace(["\r\n", "\r", "\n"], ' ', $this->application->pre_deployment_command);
$cmd = "sh -c '".str_replace("'", "'\''", $preCommand)."'";
$exec = "docker exec {$containerName} {$cmd}";
$this->execute_remote_command(
[
'command' => $exec,
'hidden' => true,
],
);

return;
}
}
throw new DeploymentException('Pre-deployment command: Could not find a valid container. Is the container name correct?');
$container = $this->resolveCommandContainer($containers, $this->application->pre_deployment_command_container, 'Pre-deployment');
if ($container === null) {
throw new DeploymentException('Pre-deployment command: Could not find a valid container. Is the container name correct?');
}

$containerName = data_get($container, 'Names');
if ($containerName) {
$this->validateContainerName($containerName);
}
// Security: pre_deployment_command is intentionally treated as arbitrary shell input.
// Users (team members with deployment access) need full shell flexibility to run commands
// like "php artisan migrate", "npm run build", etc. inside their own application containers.
// The trust boundary is at the application/team ownership level — only authenticated team
// members can set these commands, and execution is scoped to the application's own container.
// The single-quote escaping here prevents breaking out of the sh -c wrapper, but does not
// restrict the command itself. Container names are validated separately via validateContainerName().
// Newlines are normalized to spaces to prevent injection via SSH heredoc transport
// (matches the pattern used for health_check_command at line ~2824).
$preCommand = str_replace(["\r\n", "\r", "\n"], ' ', $this->application->pre_deployment_command);
$cmd = "sh -c '".str_replace("'", "'\''", $preCommand)."'";
$exec = "docker exec {$containerName} {$cmd}";
$this->execute_remote_command(
[
'command' => $exec,
'hidden' => true,
],
);
}

private function run_post_deployment_command()
Expand All @@ -4087,38 +4132,42 @@ private function run_post_deployment_command()
$this->application_deployment_queue->addLogEntry('Executing post-deployment command (see debug log for output).');

$containers = getCurrentApplicationContainerStatus($this->server, $this->application->id, $this->pull_request_id);
foreach ($containers as $container) {
$containerName = data_get($container, 'Names');
if ($containerName) {
$this->validateContainerName($containerName);
}
if ($containers->count() == 1 || str_starts_with($containerName, $this->application->post_deployment_command_container.'-'.$this->application->uuid)) {
// Security: post_deployment_command is intentionally treated as arbitrary shell input.
// See the equivalent comment in run_pre_deployment_command() for the full security rationale.
// Newlines are normalized to spaces to prevent injection via SSH heredoc transport.
$postCommand = str_replace(["\r\n", "\r", "\n"], ' ', $this->application->post_deployment_command);
$cmd = "sh -c '".str_replace("'", "'\''", $postCommand)."'";
$exec = "docker exec {$containerName} {$cmd}";
try {
$this->execute_remote_command(
[
'command' => $exec,
'hidden' => true,
'save' => 'post-deployment-command-output',
],
);
} catch (Exception $e) {
$post_deployment_command_output = $this->saved_outputs->get('post-deployment-command-output');
if ($post_deployment_command_output) {
$this->application_deployment_queue->addLogEntry('Post-deployment command failed.');
$this->application_deployment_queue->addLogEntry($post_deployment_command_output, 'stderr');
}
}
if ($containers->count() == 0) {
$this->application_deployment_queue->addLogEntry('Post-deployment command: No running containers found. Skipping.');

return;
return;
}

$container = $this->resolveCommandContainer($containers, $this->application->post_deployment_command_container, 'Post-deployment');
if ($container === null) {
throw new DeploymentException('Post-deployment command: Could not find a valid container. Is the container name correct?');
}

$containerName = data_get($container, 'Names');
if ($containerName) {
$this->validateContainerName($containerName);
}
// Security: post_deployment_command is intentionally treated as arbitrary shell input.
// See the equivalent comment in run_pre_deployment_command() for the full security rationale.
// Newlines are normalized to spaces to prevent injection via SSH heredoc transport.
$postCommand = str_replace(["\r\n", "\r", "\n"], ' ', $this->application->post_deployment_command);
$cmd = "sh -c '".str_replace("'", "'\''", $postCommand)."'";
$exec = "docker exec {$containerName} {$cmd}";
try {
$this->execute_remote_command(
[
'command' => $exec,
'hidden' => true,
'save' => 'post-deployment-command-output',
],
);
} catch (Exception $e) {
$post_deployment_command_output = $this->saved_outputs->get('post-deployment-command-output');
if ($post_deployment_command_output) {
$this->application_deployment_queue->addLogEntry('Post-deployment command failed.');
$this->application_deployment_queue->addLogEntry($post_deployment_command_output, 'stderr');
}
}
throw new DeploymentException('Post-deployment command: Could not find a valid container. Is the container name correct?');
}

/**
Expand Down
116 changes: 116 additions & 0 deletions tests/Feature/DeploymentCommandContainerResolutionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
<?php

use App\Jobs\ApplicationDeploymentJob;
use App\Models\Application;
use App\Models\ApplicationDeploymentQueue;

function createJobWithProperties(string $uuid): object
{
$ref = new ReflectionClass(ApplicationDeploymentJob::class);
$instance = $ref->newInstanceWithoutConstructor();

$app = Mockery::mock(Application::class)->makePartial();
$app->uuid = $uuid;

$queue = Mockery::mock(ApplicationDeploymentQueue::class)->makePartial();
$queue->shouldReceive('addLogEntry')->andReturnNull();

$appProp = $ref->getProperty('application');
$appProp->setAccessible(true);
$appProp->setValue($instance, $app);

$queueProp = $ref->getProperty('application_deployment_queue');
$queueProp->setAccessible(true);
$queueProp->setValue($instance, $queue);

return $instance;
}

function invokeResolve(object $instance, $containers, ?string $specifiedName, string $type): ?array
{
$ref = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $ref->getMethod('resolveCommandContainer');
$method->setAccessible(true);

return $method->invoke($instance, $containers, $specifiedName, $type);
}

describe('resolveCommandContainer', function () {
test('returns null when no containers exist', function () {
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, collect([]), 'web', 'Pre-deployment');

expect($result)->toBeNull();
});

test('returns the sole container when only one exists', function () {
$container = ['Names' => 'web-abc123', 'Labels' => ''];
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, collect([$container]), null, 'Pre-deployment');

expect($result)->toBe($container);
});

test('returns the sole container regardless of specified name when only one exists', function () {
$container = ['Names' => 'web-abc123', 'Labels' => ''];
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, collect([$container]), 'wrong-name', 'Pre-deployment');

expect($result)->toBe($container);
});

test('returns null when no container name specified for multi-container app', function () {
$containers = collect([
['Names' => 'web-abc123', 'Labels' => ''],
['Names' => 'worker-abc123', 'Labels' => ''],
]);
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, $containers, null, 'Pre-deployment');

expect($result)->toBeNull();
});

test('returns null when empty string container name for multi-container app', function () {
$containers = collect([
['Names' => 'web-abc123', 'Labels' => ''],
['Names' => 'worker-abc123', 'Labels' => ''],
]);
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, $containers, '', 'Pre-deployment');

expect($result)->toBeNull();
});

test('matches correct container by specified name in multi-container app', function () {
$containers = collect([
['Names' => 'web-abc123', 'Labels' => ''],
['Names' => 'worker-abc123', 'Labels' => ''],
]);
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, $containers, 'worker', 'Pre-deployment');

expect($result)->toBe(['Names' => 'worker-abc123', 'Labels' => '']);
});

test('returns null when specified container name does not match any container', function () {
$containers = collect([
['Names' => 'web-abc123', 'Labels' => ''],
['Names' => 'worker-abc123', 'Labels' => ''],
]);
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, $containers, 'nonexistent', 'Pre-deployment');

expect($result)->toBeNull();
});

test('matches container with PR suffix', function () {
$containers = collect([
['Names' => 'web-abc123-pr-42', 'Labels' => ''],
['Names' => 'worker-abc123-pr-42', 'Labels' => ''],
]);
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, $containers, 'web', 'Pre-deployment');

expect($result)->toBe(['Names' => 'web-abc123-pr-42', 'Labels' => '']);
});
});
Loading