rosslh/coolify
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(security): harden model assignment and sensitive data handling

Browse source 
Restrict mass-assignable attributes across user/team/redis models and
switch privileged root/team creation paths to forceFill/forceCreate.

Encrypt legacy ClickHouse admin passwords via migration and cast the
correct ClickHouse password field as encrypted.

Tighten API and runtime exposure by removing sensitive team fields from
responses and sanitizing Git/compose error messages.

Expand security-focused feature coverage for command-injection and mass
assignment protections.
This commit is contained in:
Andras Bacsai 2026-03-29 20:56:04 +02:00
parent ad694275b0
commit b3256d4df1
12 changed files with 154 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
app/Actions/Fortify/CreateNewUser.php
View file

@ -37,12 +37,13 @@ public function create(array $input): User
if (User::count() == 0) {
// If this is the first user, make them the root user
// Team is already created in the database/seeders/ProductionSeeder.php
$user = User::create([
$user = (new User)->forceFill([
'id' => 0,
'name' => $input['name'],
'email' => $input['email'],
'password' => Hash::make($input['password']),
]);
$user->save();
$team = $user->teams()->first();
// Disable registration after first user is created

8
app/Http/Controllers/Api/TeamController.php
View file

@ -14,14 +14,6 @@ private function removeSensitiveData($team)
'custom_server_limit',
'pivot',
]);
if (request()->attributes->get('can_read_sensitive', false) === false) {
$team->makeHidden([
'smtp_username',
'smtp_password',
'resend_api_key',
'telegram_token',
]);
}
return serializeApiResponse($team);
}

4
app/Models/Application.php
View file

@ -1767,7 +1767,7 @@ public function loadComposeFile($isInit = false, ?string $restoreBaseDirectory =
$fileList = collect([".$workdir$composeFile"]);
$gitRemoteStatus = $this->getGitRemoteStatus(deployment_uuid: $uuid);
if (! $gitRemoteStatus['is_accessible']) {
throw new RuntimeException("Failed to read Git source:\n\n{$gitRemoteStatus['error']}");
throw new RuntimeException('Failed to read Git source. Please verify repository access and try again.');
}
$getGitVersion = instant_remote_process(['git --version'], $this->destination->server, false);
$gitVersion = str($getGitVersion)->explode(' ')->last();
@ -1825,7 +1825,7 @@ public function loadComposeFile($isInit = false, ?string $restoreBaseDirectory =
}
throw new RuntimeException('Repository does not exist. Please check your repository URL and try again.');
}
throw new RuntimeException($e->getMessage());
throw new RuntimeException('Failed to read the Docker Compose file from the repository.');
} finally {
// Cleanup only - restoration happens in catch block
$commands = collect([

1
app/Models/ServerSetting.php
View file

@ -56,6 +56,7 @@ class ServerSetting extends Model
protected $guarded = [];
protected $casts = [
'force_disabled' => 'boolean',
'force_docker_cleanup' => 'boolean',
'docker_cleanup_threshold' => 'integer',
'sentinel_token' => 'encrypted',

2
app/Models/StandaloneClickhouse.php
View file

@ -42,7 +42,7 @@ class StandaloneClickhouse extends BaseModel
protected $appends = ['internal_db_url', 'external_db_url', 'database_type', 'server_status'];
protected $casts = [
'clickhouse_password' => 'encrypted',
'clickhouse_admin_password' => 'encrypted',
'public_port_timeout' => 'integer',
'restart_count' => 'integer',
'last_restart_at' => 'datetime',

1
app/Models/StandaloneRedis.php
View file

@ -16,7 +16,6 @@ class StandaloneRedis extends BaseModel
protected $fillable = [
'name',
'description',
'redis_password',
'redis_conf',
'status',
'image',

3
app/Models/Team.php
View file

@ -43,10 +43,9 @@ class Team extends Model implements SendsDiscord, SendsEmail, SendsPushover, Sen
protected $fillable = [
'name',
'description',
'personal_team',
'show_boarding',
'custom_server_limit',
'use_instance_email_settings',
'resend_api_key',
];
protected $casts = [

11
app/Models/User.php
View file

@ -49,9 +49,6 @@ class User extends Authenticatable implements SendsEmail
'password',
'force_password_reset',
'marketing_emails',
'pending_email',
'email_change_code',
'email_change_code_expires_at',
];
protected $hidden = [
@ -98,7 +95,7 @@ protected static function boot()
$team['id'] = 0;
$team['name'] = 'Root Team';
}
$new_team = Team::create($team);
$new_team = Team::forceCreate($team);
$user->teams()->attach($new_team, ['role' => 'owner']);
});
@ -201,7 +198,7 @@ public function recreate_personal_team()
$team['id'] = 0;
$team['name'] = 'Root Team';
}
$new_team = Team::create($team);
$new_team = Team::forceCreate($team);
$this->teams()->attach($new_team, ['role' => 'owner']);
return $new_team;
@ -412,11 +409,11 @@ public function requestEmailChange(string $newEmail): void
$expiryMinutes = config('constants.email_change.verification_code_expiry_minutes', 10);
$expiresAt = Carbon::now()->addMinutes($expiryMinutes);
$this->update([
$this->forceFill([
'pending_email' => $newEmail,
'email_change_code' => $code,
'email_change_code_expires_at' => $expiresAt,
]);
])->save();
// Send verification email to new address
$this->notify(new EmailChangeVerification($this, $code, $newEmail, $expiresAt));

39
database/migrations/2026_03_29_000000_encrypt_existing_clickhouse_admin_passwords.php Normal file
View file

@ -0,0 +1,39 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Support\Facades\Crypt;
use Illuminate\Support\Facades\DB;
class EncryptExistingClickhouseAdminPasswords extends Migration
{
public function up(): void
{
try {
DB::table('standalone_clickhouses')->chunkById(100, function ($clickhouses) {
foreach ($clickhouses as $clickhouse) {
$password = $clickhouse->clickhouse_admin_password;
if (empty($password)) {
continue;
}
// Skip if already encrypted (idempotent)
try {
Crypt::decryptString($password);
continue;
} catch (Exception) {
// Not encrypted yet — encrypt it
}
DB::table('standalone_clickhouses')
->where('id', $clickhouse->id)
->update(['clickhouse_admin_password' => Crypt::encryptString($password)]);
}
});
} catch (Exception $e) {
echo 'Encrypting ClickHouse admin passwords failed.';
echo $e->getMessage();
}
}
}

3
database/seeders/RootUserSeeder.php
View file

@ -45,12 +45,13 @@ public function run(): void
}
try {
User::create([
$user = (new User)->forceFill([
'id' => 0,
'name' => env('ROOT_USERNAME', 'Root User'),
'email' => env('ROOT_USER_EMAIL'),
'password' => Hash::make(env('ROOT_USER_PASSWORD')),
]);
$user->save();
echo "\n SUCCESS Root user created successfully.\n\n";
} catch (\Exception $e) {
echo "\n ERROR Failed to create root user: {$e->getMessage()}\n\n";

120
tests/Feature/GetLogsCommandInjectionTest.php
View file

@ -1,8 +1,40 @@
<?php
use App\Livewire\Project\Shared\GetLogs;
use App\Models\Application;
use App\Models\Environment;
use App\Models\Project;
use App\Models\Server;
use App\Models\StandaloneDocker;
use App\Models\Team;
use App\Models\User;
use App\Support\ValidationPatterns;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Livewire\Attributes\Locked;
use Livewire\Livewire;
uses(RefreshDatabase::class);
beforeEach(function () {
$this->user = User::factory()->create();
$this->team = Team::factory()->create();
$this->user->teams()->attach($this->team, ['role' => 'owner']);
$this->server = Server::factory()->create(['team_id' => $this->team->id]);
// Server::created auto-creates a StandaloneDocker, reuse it
$this->destination = StandaloneDocker::where('server_id', $this->server->id)->first();
$this->project = Project::factory()->create(['team_id' => $this->team->id]);
$this->environment = Environment::factory()->create(['project_id' => $this->project->id]);
$this->application = Application::factory()->create([
'environment_id' => $this->environment->id,
'destination_id' => $this->destination->id,
'destination_type' => $this->destination->getMorphClass(),
]);
$this->actingAs($this->user);
session(['currentTeam' => $this->team]);
});
describe('GetLogs locked properties', function () {
test('container property has Locked attribute', function () {
@ -34,47 +66,67 @@
});
});
describe('GetLogs container name validation in getLogs', function () {
test('getLogs method validates container name with ValidationPatterns', function () {
$method = new ReflectionMethod(GetLogs::class, 'getLogs');
$startLine = $method->getStartLine();
$endLine = $method->getEndLine();
$lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1);
$methodBody = implode('', $lines);
describe('GetLogs Livewire action validation', function () {
test('getLogs rejects invalid container name', function () {
// Make server functional by setting settings directly
$this->server->settings->forceFill([
'is_reachable' => true,
'is_usable' => true,
'force_disabled' => false,
])->save();
// Reload server with fresh settings to ensure casted values
$server = Server::with('settings')->find($this->server->id);
expect($methodBody)->toContain('ValidationPatterns::isValidContainerName');
Livewire::test(GetLogs::class, [
'server' => $server,
'resource' => $this->application,
'container' => 'container;malicious-command',
])
->call('getLogs')
->assertSet('outputs', 'Invalid container name.');
});
test('downloadAllLogs method validates container name with ValidationPatterns', function () {
$method = new ReflectionMethod(GetLogs::class, 'downloadAllLogs');
$startLine = $method->getStartLine();
$endLine = $method->getEndLine();
$lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1);
$methodBody = implode('', $lines);
test('getLogs rejects unauthorized server access', function () {
$otherTeam = Team::factory()->create();
$otherServer = Server::factory()->create(['team_id' => $otherTeam->id]);
expect($methodBody)->toContain('ValidationPatterns::isValidContainerName');
});
});
describe('GetLogs authorization checks', function () {
test('getLogs method checks server ownership via ownedByCurrentTeam', function () {
$method = new ReflectionMethod(GetLogs::class, 'getLogs');
$startLine = $method->getStartLine();
$endLine = $method->getEndLine();
$lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1);
$methodBody = implode('', $lines);
expect($methodBody)->toContain('Server::ownedByCurrentTeam()');
Livewire::test(GetLogs::class, [
'server' => $otherServer,
'resource' => $this->application,
'container' => 'test-container',
])
->call('getLogs')
->assertSet('outputs', 'Unauthorized.');
});
test('downloadAllLogs method checks server ownership via ownedByCurrentTeam', function () {
$method = new ReflectionMethod(GetLogs::class, 'downloadAllLogs');
$startLine = $method->getStartLine();
$endLine = $method->getEndLine();
$lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1);
$methodBody = implode('', $lines);
test('downloadAllLogs returns empty for invalid container name', function () {
$this->server->settings->forceFill([
'is_reachable' => true,
'is_usable' => true,
'force_disabled' => false,
])->save();
$server = Server::with('settings')->find($this->server->id);
expect($methodBody)->toContain('Server::ownedByCurrentTeam()');
Livewire::test(GetLogs::class, [
'server' => $server,
'resource' => $this->application,
'container' => 'container$(whoami)',
])
->call('downloadAllLogs')
->assertReturned('');
});
test('downloadAllLogs returns empty for unauthorized server', function () {
$otherTeam = Team::factory()->create();
$otherServer = Server::factory()->create(['team_id' => $otherTeam->id]);
Livewire::test(GetLogs::class, [
'server' => $otherServer,
'resource' => $this->application,
'container' => 'test-container',
])
->call('downloadAllLogs')
->assertReturned('');
});
});

18
tests/Feature/MassAssignmentProtectionTest.php
View file

@ -96,6 +96,9 @@
expect($user->isFillable('remember_token'))->toBeFalse('remember_token should not be fillable');
expect($user->isFillable('two_factor_secret'))->toBeFalse('two_factor_secret should not be fillable');
expect($user->isFillable('two_factor_recovery_codes'))->toBeFalse('two_factor_recovery_codes should not be fillable');
expect($user->isFillable('pending_email'))->toBeFalse('pending_email should not be fillable');
expect($user->isFillable('email_change_code'))->toBeFalse('email_change_code should not be fillable');
expect($user->isFillable('email_change_code_expires_at'))->toBeFalse('email_change_code_expires_at should not be fillable');
});
test('User model allows mass assignment of profile fields', function () {
@ -110,7 +113,18 @@
$team = new Team;
expect($team->isFillable('id'))->toBeFalse();
expect($team->isFillable('personal_team'))->toBeFalse('personal_team should not be fillable');
expect($team->isFillable('use_instance_email_settings'))->toBeFalse('use_instance_email_settings should not be fillable (migrated to EmailNotificationSettings)');
expect($team->isFillable('resend_api_key'))->toBeFalse('resend_api_key should not be fillable (migrated to EmailNotificationSettings)');
});
test('Team model allows mass assignment of expected fields', function () {
$team = new Team;
expect($team->isFillable('name'))->toBeTrue();
expect($team->isFillable('description'))->toBeTrue();
expect($team->isFillable('personal_team'))->toBeTrue();
expect($team->isFillable('show_boarding'))->toBeTrue();
expect($team->isFillable('custom_server_limit'))->toBeTrue();
});
test('standalone database models block mass assignment of relationship IDs', function () {
@ -145,7 +159,7 @@
expect($model->isFillable('limits_memory'))->toBeTrue();
$model = new StandaloneRedis;
expect($model->isFillable('redis_password'))->toBeTrue();
expect($model->isFillable('redis_conf'))->toBeTrue();
$model = new StandaloneMysql;
expect($model->isFillable('mysql_root_password'))->toBeTrue();