mirror of
https://github.com/tiennm99/coolify.git
synced 2026-10-15 12:14:07 +00:00
fix(models): replace forceFill/forceCreate with fill/create and add fillable guards
Replace all uses of `forceFill`, `forceCreate`, and `forceFill` with their non-force equivalents across models, actions, controllers, and Livewire components. Add explicit `$fillable` arrays to all affected Eloquent models to enforce mass assignment protection. Add ModelFillableCreationTest and ModelFillableRegressionTest to verify that model creation respects fillable constraints and prevent regressions.
This commit is contained in:
1 parent
047aff1c82
commit
1a603a10ed
79 files changed
+1411
-142
No files matched your search
@@ -31,7 +31,7 @@ beforeEach(function () {
|
||||
);
|
||||
});
|
||||
|
||||
$this->project = Project::forceCreate([
|
||||
$this->project = Project::create([
|
||||
'uuid' => (string) new Cuid2,
|
||||
'name' => 'test-project',
|
||||
'team_id' => $this->team->id,
|
||||
|
||||
@@ -6,7 +6,7 @@ use App\Models\ApplicationSetting;
|
||||
describe('Application Rollback', function () {
|
||||
beforeEach(function () {
|
||||
$this->application = new Application;
|
||||
$this->application->forceFill([
|
||||
$this->application->fill([
|
||||
'uuid' => 'test-app-uuid',
|
||||
'git_commit_sha' => 'HEAD',
|
||||
]);
|
||||
|
||||
@@ -31,7 +31,7 @@ beforeEach(function () {
|
||||
'redirect' => 'both',
|
||||
]);
|
||||
|
||||
$this->application->settings->forceFill([
|
||||
$this->application->settings->fill([
|
||||
'is_container_label_readonly_enabled' => false,
|
||||
])->save();
|
||||
|
||||
@@ -92,7 +92,7 @@ test('cloning application with multiple persistent volumes generates unique uuid
|
||||
});
|
||||
|
||||
test('cloning application reassigns settings to the cloned application', function () {
|
||||
$this->application->settings->forceFill([
|
||||
$this->application->settings->fill([
|
||||
'is_static' => true,
|
||||
'is_spa' => true,
|
||||
'is_build_server_enabled' => true,
|
||||
@@ -118,7 +118,7 @@ test('cloning application reassigns settings to the cloned application', functio
|
||||
});
|
||||
|
||||
test('cloning application reassigns scheduled tasks and previews to the cloned application', function () {
|
||||
$scheduledTask = ScheduledTask::forceCreate([
|
||||
$scheduledTask = ScheduledTask::create([
|
||||
'uuid' => 'scheduled-task-original',
|
||||
'application_id' => $this->application->id,
|
||||
'team_id' => $this->team->id,
|
||||
@@ -129,7 +129,7 @@ test('cloning application reassigns scheduled tasks and previews to the cloned a
|
||||
'timeout' => 120,
|
||||
]);
|
||||
|
||||
$preview = ApplicationPreview::forceCreate([
|
||||
$preview = ApplicationPreview::create([
|
||||
'uuid' => 'preview-original',
|
||||
'application_id' => $this->application->id,
|
||||
'pull_request_id' => 123,
|
||||
|
||||
@@ -14,7 +14,7 @@ it('populates fqdn from docker_compose_domains after generate_preview_fqdn_compo
|
||||
]),
|
||||
]);
|
||||
|
||||
$preview = ApplicationPreview::forceCreate([
|
||||
$preview = ApplicationPreview::create([
|
||||
'application_id' => $application->id,
|
||||
'pull_request_id' => 42,
|
||||
'pull_request_html_url' => 'https://github.com/example/repo/pull/42',
|
||||
@@ -39,7 +39,7 @@ it('populates fqdn with multiple domains from multiple services', function () {
|
||||
]),
|
||||
]);
|
||||
|
||||
$preview = ApplicationPreview::forceCreate([
|
||||
$preview = ApplicationPreview::create([
|
||||
'application_id' => $application->id,
|
||||
'pull_request_id' => 7,
|
||||
'pull_request_html_url' => 'https://github.com/example/repo/pull/7',
|
||||
@@ -65,7 +65,7 @@ it('sets fqdn to null when no domains are configured', function () {
|
||||
]),
|
||||
]);
|
||||
|
||||
$preview = ApplicationPreview::forceCreate([
|
||||
$preview = ApplicationPreview::create([
|
||||
'application_id' => $application->id,
|
||||
'pull_request_id' => 99,
|
||||
'pull_request_html_url' => 'https://github.com/example/repo/pull/99',
|
||||
|
||||
@@ -33,7 +33,7 @@ beforeEach(function () {
|
||||
|
||||
function createDatabase($context): StandalonePostgresql
|
||||
{
|
||||
return StandalonePostgresql::forceCreate([
|
||||
return StandalonePostgresql::create([
|
||||
'name' => 'test-postgres',
|
||||
'image' => 'postgres:15-alpine',
|
||||
'postgres_user' => 'postgres',
|
||||
|
||||
@@ -33,7 +33,7 @@ beforeEach(function () {
|
||||
|
||||
describe('PATCH /api/v1/databases', function () {
|
||||
test('updates public_port_timeout on a postgresql database', function () {
|
||||
$database = StandalonePostgresql::forceCreate([
|
||||
$database = StandalonePostgresql::create([
|
||||
'name' => 'test-postgres',
|
||||
'image' => 'postgres:15-alpine',
|
||||
'postgres_user' => 'postgres',
|
||||
@@ -57,7 +57,7 @@ describe('PATCH /api/v1/databases', function () {
|
||||
});
|
||||
|
||||
test('updates public_port_timeout on a redis database', function () {
|
||||
$database = StandaloneRedis::forceCreate([
|
||||
$database = StandaloneRedis::create([
|
||||
'name' => 'test-redis',
|
||||
'image' => 'redis:7',
|
||||
'redis_password' => 'password',
|
||||
@@ -79,7 +79,7 @@ describe('PATCH /api/v1/databases', function () {
|
||||
});
|
||||
|
||||
test('rejects invalid public_port_timeout value', function () {
|
||||
$database = StandalonePostgresql::forceCreate([
|
||||
$database = StandalonePostgresql::create([
|
||||
'name' => 'test-postgres',
|
||||
'image' => 'postgres:15-alpine',
|
||||
'postgres_user' => 'postgres',
|
||||
@@ -101,7 +101,7 @@ describe('PATCH /api/v1/databases', function () {
|
||||
});
|
||||
|
||||
test('accepts null public_port_timeout', function () {
|
||||
$database = StandalonePostgresql::forceCreate([
|
||||
$database = StandalonePostgresql::create([
|
||||
'name' => 'test-postgres',
|
||||
'image' => 'postgres:15-alpine',
|
||||
'postgres_user' => 'postgres',
|
||||
|
||||
@@ -52,7 +52,7 @@ it('reloads the mysql database model when refreshing so ssl controls follow the
|
||||
$project = Project::factory()->create(['team_id' => $this->team->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
|
||||
$database = StandaloneMysql::forceCreate([
|
||||
$database = StandaloneMysql::create([
|
||||
'name' => 'test-mysql',
|
||||
'image' => 'mysql:8',
|
||||
'mysql_root_password' => 'password',
|
||||
@@ -70,7 +70,7 @@ it('reloads the mysql database model when refreshing so ssl controls follow the
|
||||
$component = Livewire::test(MysqlGeneral::class, ['database' => $database])
|
||||
->assertDontSee('Database should be stopped to change this settings.');
|
||||
|
||||
$database->forceFill(['status' => 'running:healthy'])->save();
|
||||
$database->fill(['status' => 'running:healthy'])->save();
|
||||
|
||||
$component->call('refresh')
|
||||
->assertSee('Database should be stopped to change this settings.');
|
||||
|
||||
@@ -69,7 +69,7 @@ describe('GetLogs locked properties', function () {
|
||||
describe('GetLogs Livewire action validation', function () {
|
||||
test('getLogs rejects invalid container name', function () {
|
||||
// Make server functional by setting settings directly
|
||||
$this->server->settings->forceFill([
|
||||
$this->server->settings->fill([
|
||||
'is_reachable' => true,
|
||||
'is_usable' => true,
|
||||
'force_disabled' => false,
|
||||
@@ -100,7 +100,7 @@ describe('GetLogs Livewire action validation', function () {
|
||||
});
|
||||
|
||||
test('downloadAllLogs returns empty for invalid container name', function () {
|
||||
$this->server->settings->forceFill([
|
||||
$this->server->settings->fill([
|
||||
'is_reachable' => true,
|
||||
'is_usable' => true,
|
||||
'force_disabled' => false,
|
||||
|
||||
@@ -31,7 +31,7 @@ beforeEach(function () {
|
||||
'team_id' => $this->team->id,
|
||||
]);
|
||||
|
||||
$this->githubApp = GithubApp::forceCreate([
|
||||
$this->githubApp = GithubApp::create([
|
||||
'name' => 'Test GitHub App',
|
||||
'api_url' => 'https://api.github.com',
|
||||
'html_url' => 'https://github.com',
|
||||
|
||||
@@ -24,7 +24,7 @@ it('creates application settings for internally created applications', function
|
||||
]);
|
||||
$destination = $server->standaloneDockers()->firstOrFail();
|
||||
|
||||
$application = Application::forceCreate([
|
||||
$application = Application::create([
|
||||
'name' => 'internal-app',
|
||||
'git_repository' => 'https://github.com/coollabsio/coolify',
|
||||
'git_branch' => 'main',
|
||||
@@ -57,7 +57,7 @@ it('creates services with protected relationship ids in trusted internal paths',
|
||||
]);
|
||||
$destination = $server->standaloneDockers()->firstOrFail();
|
||||
|
||||
$service = Service::forceCreate([
|
||||
$service = Service::create([
|
||||
'docker_compose_raw' => 'services: {}',
|
||||
'environment_id' => $environment->id,
|
||||
'server_id' => $server->id,
|
||||
|
||||
File diff suppressed because it is too large.
Load diff
@@ -14,18 +14,18 @@ uses(RefreshDatabase::class);
|
||||
it('returns the correct team through the service relationship chain', function () {
|
||||
$team = Team::factory()->create();
|
||||
|
||||
$project = Project::forceCreate([
|
||||
$project = Project::create([
|
||||
'uuid' => (string) Str::uuid(),
|
||||
'name' => 'Test Project',
|
||||
'team_id' => $team->id,
|
||||
]);
|
||||
|
||||
$environment = Environment::forceCreate([
|
||||
$environment = Environment::create([
|
||||
'name' => 'test-env-'.Str::random(8),
|
||||
'project_id' => $project->id,
|
||||
]);
|
||||
|
||||
$service = Service::forceCreate([
|
||||
$service = Service::create([
|
||||
'uuid' => (string) Str::uuid(),
|
||||
'name' => 'supabase',
|
||||
'environment_id' => $environment->id,
|
||||
@@ -34,7 +34,7 @@ it('returns the correct team through the service relationship chain', function (
|
||||
'docker_compose_raw' => 'version: "3"',
|
||||
]);
|
||||
|
||||
$serviceDatabase = ServiceDatabase::forceCreate([
|
||||
$serviceDatabase = ServiceDatabase::create([
|
||||
'uuid' => (string) Str::uuid(),
|
||||
'name' => 'supabase-db',
|
||||
'service_id' => $service->id,
|
||||
@@ -47,18 +47,18 @@ it('returns the correct team through the service relationship chain', function (
|
||||
it('returns the correct team for ServiceApplication through the service relationship chain', function () {
|
||||
$team = Team::factory()->create();
|
||||
|
||||
$project = Project::forceCreate([
|
||||
$project = Project::create([
|
||||
'uuid' => (string) Str::uuid(),
|
||||
'name' => 'Test Project',
|
||||
'team_id' => $team->id,
|
||||
]);
|
||||
|
||||
$environment = Environment::forceCreate([
|
||||
$environment = Environment::create([
|
||||
'name' => 'test-env-'.Str::random(8),
|
||||
'project_id' => $project->id,
|
||||
]);
|
||||
|
||||
$service = Service::forceCreate([
|
||||
$service = Service::create([
|
||||
'uuid' => (string) Str::uuid(),
|
||||
'name' => 'supabase',
|
||||
'environment_id' => $environment->id,
|
||||
@@ -67,7 +67,7 @@ it('returns the correct team for ServiceApplication through the service relation
|
||||
'docker_compose_raw' => 'version: "3"',
|
||||
]);
|
||||
|
||||
$serviceApplication = ServiceApplication::forceCreate([
|
||||
$serviceApplication = ServiceApplication::create([
|
||||
'uuid' => (string) Str::uuid(),
|
||||
'name' => 'supabase-studio',
|
||||
'service_id' => $service->id,
|
||||
|
||||
@@ -49,7 +49,7 @@ function createTestApplication($context): Application
|
||||
|
||||
function createTestDatabase($context): StandalonePostgresql
|
||||
{
|
||||
return StandalonePostgresql::forceCreate([
|
||||
return StandalonePostgresql::create([
|
||||
'name' => 'test-postgres',
|
||||
'image' => 'postgres:15-alpine',
|
||||
'postgres_user' => 'postgres',
|
||||
|
||||
@@ -1,12 +1,14 @@
|
||||
<?php
|
||||
|
||||
use App\Models\Application;
|
||||
use App\Models\ApplicationSetting;
|
||||
|
||||
/**
|
||||
* Security tests for git ref validation (GHSA-mw5w-2vvh-mgf4).
|
||||
*
|
||||
* Ensures that git_commit_sha and related inputs are validated
|
||||
* to prevent OS command injection via shell metacharacters.
|
||||
*/
|
||||
|
||||
describe('validateGitRef', function () {
|
||||
test('accepts valid hex commit SHAs', function () {
|
||||
expect(validateGitRef('abc123def456'))->toBe('abc123def456');
|
||||
@@ -93,31 +95,31 @@ describe('validateGitRef', function () {
|
||||
describe('executeInDocker git log escaping', function () {
|
||||
test('git log command escapes commit SHA to prevent injection', function () {
|
||||
$maliciousCommit = "HEAD'; id; #";
|
||||
$command = "cd /workdir && git log -1 ".escapeshellarg($maliciousCommit).' --pretty=%B';
|
||||
$command = 'cd /workdir && git log -1 '.escapeshellarg($maliciousCommit).' --pretty=%B';
|
||||
$result = executeInDocker('test-container', $command);
|
||||
|
||||
// The malicious payload must not be able to break out of quoting
|
||||
expect($result)->not->toContain("id;");
|
||||
expect($result)->not->toContain('id;');
|
||||
expect($result)->toContain("'HEAD'\\''");
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildGitCheckoutCommand escaping', function () {
|
||||
test('checkout command escapes target to prevent injection', function () {
|
||||
$app = new \App\Models\Application;
|
||||
$app->forceFill(['uuid' => 'test-uuid']);
|
||||
$app = new Application;
|
||||
$app->fill(['uuid' => 'test-uuid']);
|
||||
|
||||
$settings = new \App\Models\ApplicationSetting;
|
||||
$settings = new ApplicationSetting;
|
||||
$settings->is_git_submodules_enabled = false;
|
||||
$app->setRelation('settings', $settings);
|
||||
|
||||
$method = new \ReflectionMethod($app, 'buildGitCheckoutCommand');
|
||||
$method = new ReflectionMethod($app, 'buildGitCheckoutCommand');
|
||||
|
||||
$result = $method->invoke($app, 'abc123');
|
||||
expect($result)->toContain("git checkout 'abc123'");
|
||||
|
||||
$result = $method->invoke($app, "abc'; id; #");
|
||||
expect($result)->not->toContain("id;");
|
||||
expect($result)->not->toContain('id;');
|
||||
expect($result)->toContain("git checkout 'abc'");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,76 @@
|
||||
<?php
|
||||
|
||||
use App\Models\Application;
|
||||
use App\Models\ApplicationDeploymentQueue;
|
||||
use App\Models\ApplicationPreview;
|
||||
use App\Models\ApplicationSetting;
|
||||
use App\Models\CloudProviderToken;
|
||||
use App\Models\Environment;
|
||||
use App\Models\GithubApp;
|
||||
use App\Models\Project;
|
||||
use App\Models\ProjectSetting;
|
||||
use App\Models\ScheduledDatabaseBackup;
|
||||
use App\Models\ScheduledDatabaseBackupExecution;
|
||||
use App\Models\ScheduledTask;
|
||||
use App\Models\ScheduledTaskExecution;
|
||||
use App\Models\Server;
|
||||
use App\Models\ServerSetting;
|
||||
use App\Models\Service;
|
||||
use App\Models\ServiceApplication;
|
||||
use App\Models\ServiceDatabase;
|
||||
use App\Models\StandaloneClickhouse;
|
||||
use App\Models\StandaloneDocker;
|
||||
use App\Models\StandaloneDragonfly;
|
||||
use App\Models\StandaloneKeydb;
|
||||
use App\Models\StandaloneMariadb;
|
||||
use App\Models\StandaloneMongodb;
|
||||
use App\Models\StandaloneMysql;
|
||||
use App\Models\StandalonePostgresql;
|
||||
use App\Models\StandaloneRedis;
|
||||
use App\Models\Subscription;
|
||||
use App\Models\SwarmDocker;
|
||||
use App\Models\Tag;
|
||||
use App\Models\User;
|
||||
|
||||
it('keeps required mass-assignment attributes fillable for internal create flows', function (string $modelClass, array $expectedAttributes) {
|
||||
$model = new $modelClass;
|
||||
|
||||
expect($model->getFillable())->toContain(...$expectedAttributes);
|
||||
})->with([
|
||||
// Relationship/ownership keys
|
||||
[CloudProviderToken::class, ['team_id']],
|
||||
[Tag::class, ['team_id']],
|
||||
[Subscription::class, ['team_id']],
|
||||
[ScheduledTaskExecution::class, ['scheduled_task_id']],
|
||||
[ScheduledDatabaseBackupExecution::class, ['uuid', 'scheduled_database_backup_id']],
|
||||
[ScheduledDatabaseBackup::class, ['uuid', 'team_id']],
|
||||
[ScheduledTask::class, ['uuid', 'team_id', 'application_id', 'service_id']],
|
||||
[ServiceDatabase::class, ['service_id']],
|
||||
[ServiceApplication::class, ['service_id']],
|
||||
[ApplicationDeploymentQueue::class, ['docker_registry_image_tag']],
|
||||
[Project::class, ['team_id', 'uuid']],
|
||||
[Environment::class, ['project_id', 'uuid']],
|
||||
[ProjectSetting::class, ['project_id']],
|
||||
[ApplicationSetting::class, ['application_id']],
|
||||
[ServerSetting::class, ['server_id']],
|
||||
[SwarmDocker::class, ['server_id']],
|
||||
[StandaloneDocker::class, ['server_id']],
|
||||
[User::class, ['pending_email', 'email_change_code', 'email_change_code_expires_at']],
|
||||
[Server::class, ['ip_previous']],
|
||||
[GithubApp::class, ['team_id', 'private_key_id']],
|
||||
|
||||
// Application/Service resource keys (including uuid for clone flows)
|
||||
[Application::class, ['uuid', 'environment_id', 'destination_id', 'destination_type', 'source_id', 'source_type', 'repository_project_id', 'private_key_id']],
|
||||
[ApplicationPreview::class, ['uuid', 'application_id']],
|
||||
[Service::class, ['uuid', 'environment_id', 'server_id', 'destination_id', 'destination_type']],
|
||||
|
||||
// Standalone database resource keys (including uuid for clone flows)
|
||||
[StandalonePostgresql::class, ['uuid', 'destination_type', 'destination_id', 'environment_id']],
|
||||
[StandaloneMysql::class, ['uuid', 'destination_type', 'destination_id', 'environment_id']],
|
||||
[StandaloneMariadb::class, ['uuid', 'destination_type', 'destination_id', 'environment_id']],
|
||||
[StandaloneMongodb::class, ['uuid', 'destination_type', 'destination_id', 'environment_id']],
|
||||
[StandaloneRedis::class, ['uuid', 'destination_type', 'destination_id', 'environment_id']],
|
||||
[StandaloneKeydb::class, ['uuid', 'destination_type', 'destination_id', 'environment_id']],
|
||||
[StandaloneDragonfly::class, ['uuid', 'destination_type', 'destination_id', 'environment_id']],
|
||||
[StandaloneClickhouse::class, ['uuid', 'destination_type', 'destination_id', 'environment_id']],
|
||||
]);
|
||||
@@ -16,8 +16,8 @@ it('ensures service parser does not include image in trusted service creation qu
|
||||
expect($parsersFile)
|
||||
->toContain("\$databaseFound = ServiceDatabase::where('name', \$serviceName)->where('service_id', \$resource->id)->first();")
|
||||
->toContain("\$applicationFound = ServiceApplication::where('name', \$serviceName)->where('service_id', \$resource->id)->first();")
|
||||
->toContain("forceCreate([\n 'name' => \$serviceName,\n 'service_id' => \$resource->id,\n ]);")
|
||||
->not->toContain("forceCreate([\n 'name' => \$serviceName,\n 'image' => \$image,\n 'service_id' => \$resource->id,\n ]);");
|
||||
->toContain("create([\n 'name' => \$serviceName,\n 'service_id' => \$resource->id,\n ]);")
|
||||
->not->toContain("create([\n 'name' => \$serviceName,\n 'image' => \$image,\n 'service_id' => \$resource->id,\n ]);");
|
||||
});
|
||||
|
||||
it('ensures service parser updates image after finding or creating service', function () {
|
||||
@@ -41,8 +41,8 @@ it('ensures parseDockerComposeFile does not create duplicates on null savedServi
|
||||
// The new code checks for null within the else block and creates only if needed
|
||||
expect($sharedFile)
|
||||
->toContain('if (is_null($savedService)) {')
|
||||
->toContain('$savedService = ServiceDatabase::forceCreate([')
|
||||
->toContain('$savedService = ServiceApplication::forceCreate([');
|
||||
->toContain('$savedService = ServiceDatabase::create([')
|
||||
->toContain('$savedService = ServiceApplication::create([');
|
||||
});
|
||||
|
||||
it('verifies image update logic is present in parseDockerComposeFile', function () {
|
||||
|
||||
@@ -77,21 +77,21 @@ uZx9iFkCELtxrh31QJ68AAAAEXNhaWxANzZmZjY2ZDJlMmRkAQIDBA==
|
||||
],
|
||||
]);
|
||||
|
||||
Project::forceCreate([
|
||||
Project::create([
|
||||
'uuid' => 'project-1',
|
||||
'name' => 'My first project',
|
||||
'description' => 'This is a test project in development',
|
||||
'team_id' => 0,
|
||||
]);
|
||||
|
||||
Project::forceCreate([
|
||||
Project::create([
|
||||
'uuid' => 'project-2',
|
||||
'name' => 'Production API',
|
||||
'description' => 'Backend services for production',
|
||||
'team_id' => 0,
|
||||
]);
|
||||
|
||||
Project::forceCreate([
|
||||
Project::create([
|
||||
'uuid' => 'project-3',
|
||||
'name' => 'Staging Environment',
|
||||
'description' => 'Staging and QA testing',
|
||||
|
||||
Reference in new issue
Block a user