diff --git a/app/Http/Controllers/Api/Concerns/HandlesDatabaseImportsApi.php b/app/Http/Controllers/Api/Concerns/HandlesDatabaseImportsApi.php index 5c06f77885..07156a9d4e 100644 --- a/app/Http/Controllers/Api/Concerns/HandlesDatabaseImportsApi.php +++ b/app/Http/Controllers/Api/Concerns/HandlesDatabaseImportsApi.php @@ -121,7 +121,9 @@ trait HandlesDatabaseImportsApi 'id' => $activity->id, 'status' => $status, 'exit_code' => data_get($activity, 'properties.exitCode'), - 'output' => remove_iip(RunRemoteProcess::decodeOutput($activity)), + 'output' => request()->attributes->get('can_read_sensitive', false) === true && request()->user()->can('update', $resource) + ? remove_iip(RunRemoteProcess::decodeOutput($activity)) + : null, 'created_at' => $activity->created_at, 'updated_at' => $activity->updated_at, 'finished_at' => $terminal ? $activity->updated_at : null, diff --git a/app/Support/DatabaseImport/DatabaseImportCommandBuilder.php b/app/Support/DatabaseImport/DatabaseImportCommandBuilder.php index c8c32a80f8..7e7c05ae6f 100644 --- a/app/Support/DatabaseImport/DatabaseImportCommandBuilder.php +++ b/app/Support/DatabaseImport/DatabaseImportCommandBuilder.php @@ -187,18 +187,19 @@ SH; // current data as it was, also when --clean dropped objects first. $owners = $keepOwners ? '' : ' --no-owner --no-acl'; $clean = $replaceExisting ? ' --clean --if-exists' : ''; - $sqlRestore = $replaceExisting ? <<<'SH' + $suffix = bin2hex(random_bytes(16)); + $sqlRestore = $replaceExisting ? "\n new=coolify_restore_new_{$suffix}\n old=coolify_restore_old_{$suffix}\n".<<<'SH' echo 'SQL backups cannot replace single objects. The backup is restored into a new database first; the current database is replaced only when that restore succeeds.' - new=coolify_restore_new - old=coolify_restore_old - PGOPTIONS='-c client_min_messages=warning' dropdb --maintenance-db=template1 -U $POSTGRES_USER --if-exists "$new" || exit 1 + if ! existing=$(printf '%s\n' "SELECT 1 FROM pg_database WHERE datname IN (:'new', :'old');" | psql -v ON_ERROR_STOP=1 -At -v new="$new" -v old="$old" -U $POSTGRES_USER -d template1); then + fail 'The temporary database names could not be checked. Nothing was changed.' + fi + [ -z "$existing" ] || fail 'A temporary database name is already in use. Nothing was changed.' createdb -U $POSTGRES_USER "$new" || exit 1 if ! stream | psql -v ON_ERROR_STOP=1 --single-transaction -U $POSTGRES_USER -d "$new"; then PGOPTIONS='-c client_min_messages=warning' dropdb --maintenance-db=template1 -U $POSTGRES_USER --if-exists "$new" fail 'The SQL restore failed. The current database was not changed.' fi - PGOPTIONS='-c client_min_messages=warning' dropdb --maintenance-db=template1 -U $POSTGRES_USER --if-exists "$old" || exit 1 if ! printf '%s\n' "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = :'db' AND pid <> pg_backend_pid();" 'ALTER DATABASE :"db" RENAME TO :"old";' 'ALTER DATABASE :"new" RENAME TO :"db";' | psql -v ON_ERROR_STOP=1 -v db="$db" -v old="$old" -v new="$new" -U $POSTGRES_USER -d template1 >/dev/null; then # Undo a half-done swap: the current database keeps its name and data. printf '%s\n' 'ALTER DATABASE :"old" RENAME TO :"db";' | psql -v db="$db" -v old="$old" -U $POSTGRES_USER -d template1 >/dev/null 2>&1 diff --git a/tests/Feature/Api/DatabaseImportApiTest.php b/tests/Feature/Api/DatabaseImportApiTest.php index abd0626af4..f0b8c4f99a 100644 --- a/tests/Feature/Api/DatabaseImportApiTest.php +++ b/tests/Feature/Api/DatabaseImportApiTest.php @@ -8,6 +8,8 @@ use App\Models\Environment; use App\Models\InstanceSettings; use App\Models\Project; use App\Models\Server; +use App\Models\Service; +use App\Models\ServiceDatabase; use App\Models\StandaloneDocker; use App\Models\StandalonePostgresql; use App\Models\Team; @@ -122,6 +124,7 @@ test('rejects unknown fields on standalone import', function () { }); test('returns only a team and resource scoped import activity', function () { + $this->token->update(['abilities' => ['read', 'read:sensitive']]); $database = StandalonePostgresql::create(['uuid' => (string) Str::uuid(), 'name' => 'db', 'postgres_user' => 'postgres', 'postgres_password' => 'password', 'postgres_db' => 'db', 'image' => 'postgres:17', 'status' => 'running', 'environment_id' => $this->environment->id, 'destination_id' => $this->destination->id, 'destination_type' => $this->destination->getMorphClass()]); $activity = Activity::create(['log_name' => 'default', 'description' => json_encode([['order' => 1, 'output' => 'restored', 'type' => 'stdout']]), 'properties' => ['team_id' => $this->team->id, 'type_uuid' => $database->uuid, 'operation' => 'database_import', 'status' => 'finished', 'exitCode' => 0]]); @@ -162,3 +165,56 @@ test('returns invalid token when the access token team is not a member team', fu 'create' => ['postJson', '/api/v1/databases/%s/imports'], 'show' => ['getJson', '/api/v1/databases/%s/imports/1'], ]); + +test('restricts database import output to authorized sensitive readers', function (string $resourceType, string $role, array $abilities, bool $bypassAbilityMiddleware, int $expectedStatus, bool $canReadOutput) { + $this->team->members()->updateExistingPivot($this->user->id, ['role' => $role]); + $this->token->update(['abilities' => $abilities]); + if ($bypassAbilityMiddleware) { + $this->withoutMiddleware([ApiAbility::class, EnsureTokenBelongsToCurrentTeamMember::class]); + } + + if ($resourceType === 'service') { + $service = Service::factory()->create(['environment_id' => $this->environment->id, 'server_id' => $this->server->id, 'destination_id' => $this->destination->id, 'destination_type' => $this->destination->getMorphClass(), 'docker_compose_raw' => "services: {}\n"]); + $database = ServiceDatabase::create(['name' => 'postgres', 'service_id' => $service->id, 'image' => 'postgres:17']); + $url = "/api/v1/services/{$service->uuid}/databases/{$database->uuid}/imports"; + } else { + $database = StandalonePostgresql::create(['name' => 'db', 'postgres_user' => 'postgres', 'postgres_password' => 'password', 'postgres_db' => 'db', 'image' => 'postgres:17', 'status' => 'running', 'environment_id' => $this->environment->id, 'destination_id' => $this->destination->id, 'destination_type' => $this->destination->getMorphClass()]); + $url = "/api/v1/databases/{$database->uuid}/imports"; + } + + $privateOutput = 'ERROR: Key (token)=(private-api-token) already exists.'; + $activity = Activity::create([ + 'log_name' => 'default', + 'description' => json_encode([['order' => 1, 'output' => $privateOutput, 'type' => 'stdout']]), + 'properties' => ['team_id' => $this->team->id, 'type_uuid' => $database->uuid, 'operation' => 'database_import', 'status' => 'error', 'exitCode' => 1], + ]); + + $response = $this->withHeaders($this->headers)->getJson("{$url}/{$activity->id}") + ->assertStatus($expectedStatus); + + if ($expectedStatus !== 200) { + $response->assertDontSee($privateOutput); + + return; + } + + $response->assertJsonPath('status', 'error') + ->assertJsonPath('exit_code', 1) + ->assertJsonPath('output', $canReadOutput ? $privateOutput : null); + if (! $canReadOutput) { + $response->assertDontSee($privateOutput); + } + + $foreignTeam = Team::factory()->create(); + $activity->properties = $activity->properties->merge(['team_id' => $foreignTeam->id]); + $activity->save(); + $this->withHeaders($this->headers)->getJson("{$url}/{$activity->id}") + ->assertNotFound()->assertDontSee($privateOutput); +})->with(['standalone', 'service'])->with([ + 'admin read only' => ['admin', ['read'], false, 200, false], + 'admin sensitive' => ['admin', ['read', 'read:sensitive'], false, 200, true], + 'owner root' => ['owner', ['root'], false, 200, true], + 'member read only' => ['member', ['read'], false, 200, false], + 'member legacy sensitive token' => ['member', ['read', 'read:sensitive'], false, 403, false], + 'member sensitive without ability middleware' => ['member', ['read', 'read:sensitive'], true, 200, false], +]); diff --git a/tests/Unit/DatabaseImport/DatabaseImportRestoreScriptTest.php b/tests/Unit/DatabaseImport/DatabaseImportRestoreScriptTest.php index 9a32fbd24e..b0ceb807df 100644 --- a/tests/Unit/DatabaseImport/DatabaseImportRestoreScriptTest.php +++ b/tests/Unit/DatabaseImport/DatabaseImportRestoreScriptTest.php @@ -346,6 +346,120 @@ function restoreScriptNormalize(string $contents): array } } +/** + * Runs a SQL replacement against stateful PostgreSQL client stubs. Database values represent + * their data, so accidental drops and a failed swap are visible in the final database state. + * + * @return array{exit: int, stdout: string, stderr: string, databases: array} + */ +function postgresReplacementScriptRun(string $target = 'app', string $failure = '', bool $gzip = false): array +{ + $dir = restoreScriptTempDir(); + + try { + $backup = $dir.'/backup.sql'; + file_put_contents($backup, $gzip ? gzencode('SELECT 1;') : 'SELECT 1;'); + $script = (new DatabaseImportCommandBuilder)->buildRestoreCommand(restoreScriptResource('postgresql'), $backup, false, true); + preg_match('/new=(\w+)/', $script, $new); + preg_match('/old=(\w+)/', $script, $old); + $databases = ['coolify_restore_new' => 'unrelated-new', 'coolify_restore_old' => 'unrelated-old', $target => 'original']; + if (in_array($failure, ['new_collision', 'old_collision'], true)) { + $databases[$failure === 'new_collision' ? $new[1] : $old[1]] = 'collision'; + } + $state = $dir.'/databases.json'; + file_put_contents($state, json_encode($databases)); + mkdir($dir.'/bin'); + + $client = <<<'PHP' + $arg) { + if ($arg === '-v') { + [$key, $value] = explode('=', $args[$index + 1], 2); + $variables[$key] = $value; + } + } + if (str_contains($sql, 'SELECT 1 FROM pg_database')) { + if (isset($databases[$variables['new']]) || isset($databases[$variables['old']])) { echo "1\n"; } + } elseif (str_contains($sql, 'ALTER DATABASE')) { + preg_match_all('/ALTER DATABASE :"(\w+)" RENAME TO :"(\w+)";/', $sql, $renames, PREG_SET_ORDER); + foreach ($renames as $rename) { + $from = $variables[$rename[1]]; + $to = $variables[$rename[2]]; + if (! isset($databases[$from]) || isset($databases[$to]) || ($failure === 'swap' && $rename[1] === 'new')) { + $save(); + exit(1); + } + $databases[$to] = $databases[$from]; + unset($databases[$from]); + } + } else { + if ($failure === 'restore') { exit(1); } + $database = $args[array_search('-d', $args, true) + 1]; + if (! isset($databases[$database])) { exit(1); } + $databases[$database] = 'restored'; + } +} +$save(); +PHP; + + foreach (['psql', 'createdb', 'dropdb'] as $name) { + file_put_contents($dir.'/bin/'.$name, '#!'.PHP_BINARY."\n".$client); + chmod($dir.'/bin/'.$name, 0755); + } + + return restoreScriptProcess(['sh', '-c', $script], $dir, [ + 'PATH' => $dir.'/bin'.PATH_SEPARATOR.(getenv('PATH') ?: '/usr/bin:/bin'), + 'POSTGRES_USER' => 'postgres', + 'POSTGRES_DB' => $target, + 'PG_STATE' => $state, + 'PG_FAILURE' => $failure, + ]) + ['databases' => json_decode(file_get_contents($state), true)]; + } finally { + restoreScriptRemoveDir($dir); + } +} + +test('SQL replacement preserves unrelated PostgreSQL databases with the former scratch names', function (string $target, bool $gzip) { + $run = postgresReplacementScriptRun($target, gzip: $gzip); + $expected = ['coolify_restore_new' => 'unrelated-new', 'coolify_restore_old' => 'unrelated-old', $target => 'restored']; + + expect($run['exit'])->toBe(0, $run['stderr']) + ->and($run['databases'])->toEqual($expected); +})->with(['app', 'coolify_restore_new', 'coolify_restore_old'])->with([false, true]); + +test('failed SQL replacement preserves every existing PostgreSQL database and removes only its own scratch database', function (string $failure) { + $run = postgresReplacementScriptRun(failure: $failure); + + expect($run['exit'])->toBe(1) + ->and($run['databases'])->toEqual(['coolify_restore_new' => 'unrelated-new', 'coolify_restore_old' => 'unrelated-old', 'app' => 'original']); +})->with(['restore', 'swap']); + +test('SQL replacement refuses scratch database name collisions without changing existing data', function (string $failure) { + $run = postgresReplacementScriptRun(failure: $failure); + + expect($run['exit'])->toBe(1) + ->and($run['databases'])->toHaveCount(4) + ->and($run['databases']['app'])->toBe('original') + ->and($run['databases']['coolify_restore_new'])->toBe('unrelated-new') + ->and($run['databases']['coolify_restore_old'])->toBe('unrelated-old') + ->and(array_values($run['databases']))->toContain('collision'); +})->with(['new_collision', 'old_collision']); + test('restores single PostgreSQL archives with pg_restore and SQL with psql', function (string $fixture, bool $replaceExisting, array $expectedCalls) { $run = restoreScriptRun('postgresql', restoreScriptFixture($fixture), false, $replaceExisting); @@ -363,20 +477,18 @@ test('restores single PostgreSQL archives with pg_restore and SQL with psql', fu // SQL cannot replace single objects: it restores into a new database, and only a // successful restore replaces the current one, so a failure changes nothing. 'plain SQL replacing existing objects' => ['pg-sql', true, [ - ['dropdb', '', ['--maintenance-db=template1 -U postgres --if-exists coolify_restore_new']], - ['createdb', '', ['-U postgres coolify_restore_new']], - ['psql', '-- Po', ['-v ON_ERROR_STOP=1 --single-transaction -U postgres -d coolify_restore_new']], - ['dropdb', '', ['--maintenance-db=template1 -U postgres --if-exists coolify_restore_old']], - ['psql', 'SELEC', ['-v ON_ERROR_STOP=1 -v db=app -v old=coolify_restore_old -v new=coolify_restore_new -U postgres -d template1']], - ['dropdb', '', ['--maintenance-db=template1 -U postgres --if-exists coolify_restore_old']], + ['psql', 'SELEC', ['-v ON_ERROR_STOP=1 -At', '-v new=coolify_restore_new_', '-v old=coolify_restore_old_']], + ['createdb', '', ['-U postgres coolify_restore_new_']], + ['psql', '-- Po', ['-v ON_ERROR_STOP=1 --single-transaction -U postgres -d coolify_restore_new_']], + ['psql', 'SELEC', ['-v ON_ERROR_STOP=1 -v db=app -v old=coolify_restore_old_', '-v new=coolify_restore_new_', '-U postgres -d template1']], + ['dropdb', '', ['--maintenance-db=template1 -U postgres --if-exists coolify_restore_old_']], ]], 'gzip SQL replacing existing objects' => ['pg-sql-gz', true, [ - ['dropdb', '', ['--maintenance-db=template1 -U postgres --if-exists coolify_restore_new']], - ['createdb', '', ['-U postgres coolify_restore_new']], - ['psql', '-- Po', ['-v ON_ERROR_STOP=1 --single-transaction -U postgres -d coolify_restore_new']], - ['dropdb', '', ['--maintenance-db=template1 -U postgres --if-exists coolify_restore_old']], - ['psql', 'SELEC', ['-v ON_ERROR_STOP=1 -v db=app -v old=coolify_restore_old -v new=coolify_restore_new -U postgres -d template1']], - ['dropdb', '', ['--maintenance-db=template1 -U postgres --if-exists coolify_restore_old']], + ['psql', 'SELEC', ['-v ON_ERROR_STOP=1 -At', '-v new=coolify_restore_new_', '-v old=coolify_restore_old_']], + ['createdb', '', ['-U postgres coolify_restore_new_']], + ['psql', '-- Po', ['-v ON_ERROR_STOP=1 --single-transaction -U postgres -d coolify_restore_new_']], + ['psql', 'SELEC', ['-v ON_ERROR_STOP=1 -v db=app -v old=coolify_restore_old_', '-v new=coolify_restore_new_', '-U postgres -d template1']], + ['dropdb', '', ['--maintenance-db=template1 -U postgres --if-exists coolify_restore_old_']], ]], ]);