fix(dns): lock DNS modal state and unquote modal confirmation args

Mark DNS proposals, conflicts and server IP properties as #[Locked] so
clients cannot tamper with them. Re-resolve the zone per hostname,
lowercase hostnames, validate record content server-side and use the
resource's team id for DNS lookups and jobs.

Move modal-confirmation submitAction parsing into a tested JS helper
that strips quotes and keeps commas, parentheses and escaped quotes
inside quoted arguments. Pass the hostname to replaceManagedDnsRecord
through it instead of Js::from.

Rework member-permission tests to open the real DNS dialog rather than
setting locked properties directly.
This commit is contained in:
Andras Bacsai
2026-09-30 10:23:51 +02:00
parent 2b5bffe37d
commit 6a59008cda
11 changed files with 225 additions and 92 deletions
+1
View File
@@ -39,6 +39,7 @@
- Put destructive actions on the footer's left and primary actions last on the right.
- Use shared section, helper, tooltip, and icon-button components instead of local variants.
- Keep validation, preview, and save controls in a fixed footer when the body is large.
- `x-modal-confirmation` parses `submitAction` with `resources/js/modal-confirmation.js`: quoted arguments lose their quotes, unquoted ones arrive as text. Quote text that can contain commas or parentheses.
## Verify layered UI behavior visually
- Inspect the real layout with all conditional elements visible, especially compound status badges.
@@ -11,13 +11,17 @@ use App\Services\Dns\CloudflareDnsProvider;
use App\Services\Dns\ManagedDnsRecordCleanup;
use Illuminate\Contracts\Cache\LockTimeoutException;
use Illuminate\Database\Eloquent\Model;
use Livewire\Attributes\Locked;
use RuntimeException;
trait InteractsWithDnsProviders
{
public bool $showDnsProviderModal = false;
#[Locked]
public array $dnsProviderProposals = [];
#[Locked]
public array $dnsProviderConflicts = [];
public bool $deleteManagedDns = true;
@@ -42,10 +46,11 @@ trait InteractsWithDnsProviders
public function createManagedDnsRecord(string $hostname, int $zoneId, ?string $content = null): void
{
$this->authorizeDnsProviderChange();
$hostname = strtolower($hostname);
$cloudflare = app(CloudflareDnsProvider::class);
$zone = $this->findTeamZone($zoneId);
$content ??= $this->serverIp;
if ($zone === null || blank($content) || filter_var($content, FILTER_VALIDATE_IP) === false) {
$zone = $this->findTeamZone($zoneId, $hostname);
$content = $this->dnsRecordContent($content);
if ($zone === null || $content === null) {
$this->dispatch('error', 'No connected DNS provider or public server IP is available for this domain.');
return;
@@ -80,7 +85,7 @@ trait InteractsWithDnsProviders
return collect($urls)->contains(function (string $url) use ($provider): bool {
$hostname = parse_url($url, PHP_URL_HOST);
return is_string($hostname) && $provider->findZones(currentTeam()->id, $hostname)->isNotEmpty();
return is_string($hostname) && $provider->findZones($this->dnsTeamId(), $hostname)->isNotEmpty();
});
}
@@ -124,7 +129,7 @@ trait InteractsWithDnsProviders
$resource = $this->dnsResourceForHostname($proposal['hostname']);
ConfigureDnsRecordJob::dispatch(
currentTeam()->id,
$this->dnsTeamId(),
$zone->id,
$resource?->getMorphClass(),
$resource?->getKey(),
@@ -147,11 +152,12 @@ trait InteractsWithDnsProviders
public function replaceManagedDnsRecord(string $hostname, int $zoneId, string $password = ''): void
{
$this->authorizeDnsProviderChange();
$hostname = strtolower($hostname);
$key = $hostname.'|'.$zoneId;
$conflict = $this->dnsProviderConflicts[$key] ?? null;
$zone = $this->findTeamZone($zoneId);
$content = $this->serverIp;
if ($conflict === null || $zone === null || blank($content) || filter_var($content, FILTER_VALIDATE_IP) === false) {
$zone = $this->findTeamZone($zoneId, $hostname);
$content = $this->dnsRecordContent($conflict['proposed'] ?? null);
if ($conflict === null || $zone === null || $content === null) {
$this->dispatch('error', 'The DNS conflict is no longer available. Check the record again.');
return;
@@ -181,8 +187,9 @@ trait InteractsWithDnsProviders
{
$provider = app(CloudflareDnsProvider::class);
$hostnames ??= $this->allDomainHostnames();
$managed = ManagedDnsRecord::query()->where('team_id', currentTeam()->id)->whereIn('name', $hostnames)->pluck('id', 'name');
$this->dnsProviderProposals = collect($hostnames)->flatMap(fn (string $hostname) => $provider->findZones(currentTeam()->id, $hostname)
$teamId = $this->dnsTeamId();
$managed = ManagedDnsRecord::query()->where('team_id', $teamId)->whereIn('name', $hostnames)->pluck('id', 'name');
$this->dnsProviderProposals = collect($hostnames)->flatMap(fn (string $hostname) => $provider->findZones($teamId, $hostname)
->map(fn (DnsProviderZone $zone) => [
'hostname' => $hostname, 'zone_id' => $zone->id, 'zone' => $zone->name,
'credential' => $zone->integrationToken->name, 'target' => (string) $this->serverIp,
@@ -250,7 +257,7 @@ trait InteractsWithDnsProviders
return;
}
$result = app(ManagedDnsRecordCleanup::class)->releaseHostname($resource, $hostname, currentTeam()->id, $deleteRecord);
$result = app(ManagedDnsRecordCleanup::class)->releaseHostname($resource, $hostname, $this->dnsTeamId(), $deleteRecord);
$this->reportManagedDnsRelease($result, 'removed');
}
@@ -273,7 +280,7 @@ trait InteractsWithDnsProviders
*/
protected function releaseManagedDnsForEditedDomains(Model $resource, array $previousHostnames): void
{
$result = app(ManagedDnsRecordCleanup::class)->releaseRemovedHostnames($resource, $previousHostnames, currentTeam()->id);
$result = app(ManagedDnsRecordCleanup::class)->releaseRemovedHostnames($resource, $previousHostnames, $this->dnsTeamId());
$this->reportManagedDnsRelease($result, 'updated');
}
@@ -314,13 +321,46 @@ trait InteractsWithDnsProviders
protected function authorizeDnsProviderChange(): void
{
$this->authorize('update', property_exists($this, 'application') ? $this->application : $this->service);
$this->authorize('update', $this->dnsResource());
}
protected function findTeamZone(int $zoneId): ?DnsProviderZone
/** The application or service whose domains this component manages. */
protected function dnsResource(): Model
{
return property_exists($this, 'application') ? $this->application : $this->service;
}
/**
* Team of the resource. The session team can differ: a user can switch teams in another tab
* while this component is still open.
*/
protected function dnsTeamId(): int
{
return $this->dnsResource()->team()?->id ?? throw new RuntimeException('The resource has no team.');
}
/**
* Zone of the resource team. With a hostname, only a zone that manages that hostname.
*/
protected function findTeamZone(int $zoneId, ?string $hostname = null): ?DnsProviderZone
{
if ($hostname !== null) {
return app(CloudflareDnsProvider::class)->findZones($this->dnsTeamId(), $hostname)->firstWhere('id', $zoneId);
}
return DnsProviderZone::query()->whereKey($zoneId)
->whereHas('integrationToken', fn ($query) => $query->where('team_id', currentTeam()->id))->first();
->whereHas('integrationToken', fn ($query) => $query->where('team_id', $this->dnsTeamId()))->first();
}
/**
* Content of a DNS record for this resource: one of its server addresses (IPv4 or IPv6),
* the first one when none is requested. Any other address is rejected.
*/
protected function dnsRecordContent(?string $requested = null): ?string
{
$addresses = array_values(array_filter($this->serverIpsForDnsHints()));
return $requested === null ? ($addresses[0] ?? null) : (in_array($requested, $addresses, true) ? $requested : null);
}
abstract protected function persistDomainDnsStatuses(): void;
@@ -17,6 +17,7 @@ use Illuminate\Database\Eloquent\Model;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\DB;
use Livewire\Attributes\Locked;
use Livewire\Component;
class Domains extends Component
@@ -122,9 +123,11 @@ class Domains extends Component
public bool $dnsValidationEnabled = true;
/** Resolved or literal IP users should point DNS at. */
#[Locked]
public ?string $serverIp = null;
/** Raw server IP/hostname as configured (may be a hostname). */
#[Locked]
public ?string $serverIpConfigured = null;
protected $listeners = [
+3
View File
@@ -18,6 +18,7 @@ use Illuminate\Database\Eloquent\Model;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\DB;
use Livewire\Attributes\Locked;
use Livewire\Component;
class Domains extends Component
@@ -110,8 +111,10 @@ class Domains extends Component
public bool $dnsValidationEnabled = true;
#[Locked]
public ?string $serverIp = null;
#[Locked]
public ?string $serverIpConfigured = null;
/** Pending save payload after conflict/port confirmation */
+4
View File
@@ -4,6 +4,10 @@ import { initializeSettingsSidebarAccordionComponent } from './settings-sidebar-
import { initializeTerminalComponent } from './terminal.js';
import './traffic-globe.js';
import { registerLivewireRequestFailureHandler } from './livewire-request-failure.js';
import { parseSubmitAction } from './modal-confirmation.js';
// Used by the modal-confirmation Blade component to call its submitAction.
window.parseModalSubmitAction = parseSubmitAction;
document.addEventListener('livewire:init', () => {
registerLivewireRequestFailureHandler(window.Livewire);
+51
View File
@@ -0,0 +1,51 @@
/**
* Splits the submitAction of the modal-confirmation component, such as
* `replaceManagedDnsRecord('app.example.com', 4)`, into the Livewire method and its arguments.
* Quoted arguments ('…' or "…") lose their quotes; other arguments stay text, as before.
*/
export function parseSubmitAction(submitAction) {
const open = submitAction.indexOf('(');
if (open === -1) {
return { method: submitAction.trim(), params: [] };
}
const close = submitAction.lastIndexOf(')');
const argumentText = submitAction.slice(open + 1, close > open ? close : undefined);
const params = [];
let current = '';
let quote = null;
let quoted = false;
const finish = () => {
const value = quoted ? current : current.trim();
if (value !== '' || quoted) {
params.push(value);
}
current = '';
quoted = false;
};
for (let index = 0; index < argumentText.length; index++) {
const character = argumentText[index];
if (quote) {
if (character === '\\' && index + 1 < argumentText.length) {
current += argumentText[++index];
} else if (character === quote) {
quote = null;
} else {
current += character;
}
} else if ((character === "'" || character === '"') && current.trim() === '') {
quote = character;
quoted = true;
current = '';
} else if (character === ',') {
finish();
} else if (!quoted) {
current += character;
}
}
finish();
return { method: submitAction.slice(0, open).trim(), params };
}
+36
View File
@@ -0,0 +1,36 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import { parseSubmitAction } from './modal-confirmation.js';
test('keeps unquoted arguments as text, as the modal always passed them', () => {
assert.deepEqual(parseSubmitAction('delete(5)'), { method: 'delete', params: ['5'] });
assert.deepEqual(parseSubmitAction('changeSource(3, App\\Models\\GithubApp)'), {
method: 'changeSource',
params: ['3', 'App\\Models\\GithubApp'],
});
assert.deepEqual(parseSubmitAction('resetDefaultLabels(true)'), { method: 'resetDefaultLabels', params: ['true'] });
});
test('removes the quotes of quoted arguments', () => {
assert.deepEqual(parseSubmitAction("generateNginxConfiguration('spa')"), {
method: 'generateNginxConfiguration',
params: ['spa'],
});
assert.deepEqual(parseSubmitAction("replaceManagedDnsRecord('app.example.com', 4)"), {
method: 'replaceManagedDnsRecord',
params: ['app.example.com', '4'],
});
assert.deepEqual(parseSubmitAction('rename("staging")'), { method: 'rename', params: ['staging'] });
});
test('keeps commas, parentheses and escaped quotes inside quoted arguments', () => {
assert.deepEqual(parseSubmitAction("save('a, b', 'c)d', 'it\\'s')"), {
method: 'save',
params: ['a, b', 'c)d', "it's"],
});
});
test('passes no arguments for empty parentheses or a bare method name', () => {
assert.deepEqual(parseSubmitAction('cleanupDeleted()'), { method: 'cleanupDeleted', params: [] });
assert.deepEqual(parseSubmitAction('delete'), { method: 'delete', params: [] });
});
@@ -102,9 +102,7 @@
return Promise.resolve(true);
}
const methodName = this.submitAction.split('(')[0];
const paramsMatch = this.submitAction.match(/\((.*?)\)/);
const params = paramsMatch ? paramsMatch[1].split(',').map(param => param.trim()) : [];
const { method: methodName, params } = window.parseModalSubmitAction(this.submitAction);
// Always pass password parameter (empty string if password confirmation is skipped)
// This ensures consistent method signature for backend Livewire methods
@@ -28,7 +28,7 @@
@php($conflict = $dnsProviderConflicts[$key])
<div class="flex flex-col items-end gap-2 text-xs"><span class="text-red-600 dark:text-red-400">Currently {{ $conflict['current'] }}</span>
<x-modal-confirmation title="Replace conflicting DNS record?" isErrorButton buttonTitle="Replace record"
submitAction="replaceManagedDnsRecord({{ \Illuminate\Support\Js::from($proposal['hostname']) }}, {{ $proposal['zone_id'] }})"
submitAction="replaceManagedDnsRecord({{ $proposal['hostname'] }}, {{ $proposal['zone_id'] }})"
:actions="['Replace '.$proposal['hostname'].' value '.$conflict['current'].' with '.$conflict['proposed'], 'Coolify will manage the replaced record.']"
:confirmWithPassword="false" :confirmWithText="false" step2ButtonText="Replace record"
canGate="update" :canResource="$dnsAuthResource" />
+28 -41
View File
@@ -1358,55 +1358,42 @@ it('hides dns check buttons from members', function () {
->assertDontSee('Check DNS');
});
function openDnsProviderModalWithZone(Team $team, Application $application): mixed
{
$token = IntegrationToken::factory()->for($team)->create(['provider' => 'cloudflare', 'name' => 'Cloudflare', 'token' => 'secret']);
DnsProviderZone::factory()->for($token)->create(['provider_zone_id' => 'zone-1', 'name' => 'example.com']);
$application->update(['fqdn' => 'https://app.example.com']);
return Livewire::test(Domains::class, ['application' => $application->fresh()])->call('openDnsProviderModal');
}
it('disables create dns record for members and hides replace confirmation', function () {
$this->team->members()->updateExistingPivot($this->user->id, ['role' => 'member']);
$this->actingAs($this->user->fresh());
Http::fake(['https://api.cloudflare.com/client/v4/zones/zone-1/dns_records?*' => Http::response([
'success' => true,
'result' => [['id' => 'rec-1', 'type' => 'A', 'name' => 'app.example.com', 'content' => '198.51.100.10']],
])]);
$setRole = function (string $role): void {
$this->team->members()->updateExistingPivot($this->user->id, ['role' => $role]);
$this->actingAs($this->user->fresh());
};
$proposal = [
'hostname' => 'app.example.com',
'zone_id' => 1,
'zone' => 'example.com',
'credential' => 'Cloudflare',
'target' => '203.0.113.10',
'managed' => false,
];
$createHtml = Livewire::test(Domains::class, ['application' => $this->application->fresh()])
->set('showDnsProviderModal', true)
->set('dnsProviderProposals', [$proposal])
->html();
expect($createHtml)->toContain('Create DNS record')
// The owner opens the DNS dialog, then becomes a member while the page is still open.
$createPage = openDnsProviderModalWithZone($this->team, $this->application);
$setRole('member');
expect($createPage->call('$refresh')->html())->toContain('Create DNS record')
->toMatch('/<button[^>]*\sdisabled(?:[=\s>])[^>]*>.*?Create DNS record/s');
$replaceHtml = Livewire::test(Domains::class, ['application' => $this->application->fresh()])
->set('showDnsProviderModal', true)
->set('dnsProviderProposals', [$proposal])
->set('dnsProviderConflicts', [
'app.example.com|1' => [
'record_id' => 'rec-1',
'current' => '198.51.100.10',
'proposed' => '203.0.113.10',
],
])
->html();
expect($replaceHtml)->toContain('Currently 198.51.100.10')
$setRole('owner');
$replacePage = Livewire::test(Domains::class, ['application' => $this->application->fresh()])
->call('openDnsProviderModal')
->call('createManagedDnsRecord', 'app.example.com', DnsProviderZone::query()->value('id'));
$setRole('member');
expect($replacePage->call('$refresh')->html())->toContain('Currently 198.51.100.10')
->toMatch('/<button[^>]*\sdisabled(?:[=\s>])[^>]*>.*?Replace record/s');
});
it('shows create dns record enabled for owners', function () {
$html = Livewire::test(Domains::class, ['application' => $this->application->fresh()])
->set('showDnsProviderModal', true)
->set('dnsProviderProposals', [[
'hostname' => 'app.example.com',
'zone_id' => 1,
'zone' => 'example.com',
'credential' => 'Cloudflare',
'target' => '203.0.113.10',
'managed' => false,
]])
->html();
$html = openDnsProviderModalWithZone($this->team, $this->application)->html();
expect($html)->toContain('Create DNS record')
->not->toMatch('/<button[^>]*\sdisabled(?:[=\s>])[^>]*>.*?Create DNS record/s');
+42 -32
View File
@@ -28,6 +28,7 @@ use Illuminate\Support\Facades\Event;
use Illuminate\Support\Facades\Http;
use Illuminate\Support\Facades\Queue;
use Illuminate\Support\Str;
use Livewire\Features\SupportLockedProperties\CannotUpdateLockedPropertyException;
use Livewire\Livewire;
uses(RefreshDatabase::class);
@@ -604,13 +605,10 @@ test('replacing a managed dns record uses the server ip and live cloudflare reco
]);
Livewire::test(Domains::class, ['application' => $application->fresh()])
->set('dnsProviderConflicts', [
'app.example.com|'.$zone->id => [
'record_id' => 'record-1',
'current' => '198.51.100.50',
'proposed' => '198.51.100.1',
],
])
->call('createManagedDnsRecord', 'app.example.com', $zone->id)
->assertSet('dnsProviderConflicts', ['app.example.com|'.$zone->id => [
'record_id' => 'record-1', 'current' => '198.51.100.50', 'proposed' => '203.0.113.10',
]])
->call('replaceManagedDnsRecord', 'app.example.com', $zone->id)
->assertDispatched('success', 'DNS record replaced for app.example.com.');
@@ -620,34 +618,46 @@ test('replacing a managed dns record uses the server ip and live cloudflare reco
expect(ManagedDnsRecord::query()->where('name', 'app.example.com')->where('content', '203.0.113.10')->where('owned', false)->exists())->toBeTrue();
});
test('replacing a managed dns record ignores a tampered conflict record id', function () {
['application' => $application, 'zone' => $zone] = prepareManagedDnsApplication();
test('dns provider state cannot be changed from the browser', function (string $property, mixed $value) {
['application' => $application] = prepareManagedDnsApplication();
Http::fake([
'https://api.cloudflare.com/client/v4/zones/zone-1/dns_records?*' => Http::response([
'success' => true,
'result' => [['id' => 'record-1', 'type' => 'A', 'name' => 'app.example.com', 'content' => '198.51.100.50']],
]),
'https://api.cloudflare.com/client/v4/zones/zone-1/dns_records/*' => Http::response([
'success' => true, 'result' => ['id' => 'record-other'],
]),
]);
Livewire::test(Domains::class, ['application' => $application->fresh()])->set($property, $value);
})->with([
'server ip' => ['serverIp', '6.6.6.6'],
'configured server ip' => ['serverIpConfigured', '6.6.6.6'],
'conflicts' => ['dnsProviderConflicts', ['app.example.com|1' => ['record_id' => 'record-other', 'current' => 'x', 'proposed' => '6.6.6.6']]],
'proposals' => ['dnsProviderProposals', [['hostname' => 'www.victim.com', 'zone_id' => 1]]],
])->throws(CannotUpdateLockedPropertyException::class);
test('dns records use the zones of the resource team, not the session team', function () {
['application' => $application] = prepareManagedDnsApplication();
$otherTeam = Team::factory()->create();
$otherTeam->members()->attach(auth()->id(), ['role' => 'member']);
$otherToken = IntegrationToken::factory()->for($otherTeam)->create(['provider' => 'cloudflare', 'token' => 'other-secret']);
$otherZone = DnsProviderZone::factory()->for($otherToken)->create(['provider_zone_id' => 'zone-other', 'name' => 'victim.com']);
Http::fake();
$component = Livewire::test(Domains::class, ['application' => $application->fresh()]);
// The user switches to the other team in another browser tab.
session(['currentTeam' => $otherTeam]);
$component->call('createManagedDnsRecord', 'www.victim.com', $otherZone->id)
->assertDispatched('error', 'No connected DNS provider or public server IP is available for this domain.');
Http::assertNothingSent();
});
test('dns records point only at an address of the resource server in a zone of the hostname', function (string $hostname, ?string $content) {
['application' => $application, 'zone' => $zone] = prepareManagedDnsApplication();
Http::fake();
Livewire::test(Domains::class, ['application' => $application->fresh()])
->set('dnsProviderConflicts', [
'app.example.com|'.$zone->id => [
'record_id' => 'record-other',
'current' => '198.51.100.50',
'proposed' => '198.51.100.1',
],
])
->call('replaceManagedDnsRecord', 'app.example.com', $zone->id)
->assertDispatched('error', 'The DNS conflict is no longer available. Check the record again.')
->assertSet('dnsProviderConflicts', []);
Http::assertNotSent(fn ($request) => in_array($request->method(), ['PUT', 'PATCH'], true));
expect(ManagedDnsRecord::query()->where('name', 'app.example.com')->exists())->toBeFalse();
});
->call('createManagedDnsRecord', $hostname, $zone->id, $content)
->assertDispatched('error', 'No connected DNS provider or public server IP is available for this domain.');
Http::assertNothingSent();
})->with([
'another ip' => ['app.example.com', '6.6.6.6'],
'hostname outside the zone' => ['www.other.org', null],
]);
/**
* @return array{application: Application, zone: DnsProviderZone}