From 6a59008cda228595f134a9f0d0a94a18817f0d80 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:23:51 +0200 Subject: [PATCH] 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. --- .ai/lessons.md | 1 + .../Concerns/InteractsWithDnsProviders.php | 70 ++++++++++++++---- app/Livewire/Project/Application/Domains.php | 3 + app/Livewire/Project/Service/Domains.php | 3 + resources/js/app.js | 4 + resources/js/modal-confirmation.js | 51 +++++++++++++ resources/js/modal-confirmation.test.js | 36 +++++++++ .../components/modal-confirmation.blade.php | 4 +- .../shared/dns-provider-management.blade.php | 2 +- tests/Feature/ApplicationDomainsTest.php | 69 +++++++---------- tests/Feature/DnsProviderManagementTest.php | 74 +++++++++++-------- 11 files changed, 225 insertions(+), 92 deletions(-) create mode 100644 resources/js/modal-confirmation.js create mode 100644 resources/js/modal-confirmation.test.js diff --git a/.ai/lessons.md b/.ai/lessons.md index 01d0d48671..cefe025782 100644 --- a/.ai/lessons.md +++ b/.ai/lessons.md @@ -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. diff --git a/app/Livewire/Concerns/InteractsWithDnsProviders.php b/app/Livewire/Concerns/InteractsWithDnsProviders.php index 2107e44984..a82a152818 100644 --- a/app/Livewire/Concerns/InteractsWithDnsProviders.php +++ b/app/Livewire/Concerns/InteractsWithDnsProviders.php @@ -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; diff --git a/app/Livewire/Project/Application/Domains.php b/app/Livewire/Project/Application/Domains.php index 6803e0fd97..fef335ade0 100644 --- a/app/Livewire/Project/Application/Domains.php +++ b/app/Livewire/Project/Application/Domains.php @@ -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 = [ diff --git a/app/Livewire/Project/Service/Domains.php b/app/Livewire/Project/Service/Domains.php index d51e5c6642..dfa7f3af60 100644 --- a/app/Livewire/Project/Service/Domains.php +++ b/app/Livewire/Project/Service/Domains.php @@ -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 */ diff --git a/resources/js/app.js b/resources/js/app.js index 13a2b6778d..4a4691e06f 100644 --- a/resources/js/app.js +++ b/resources/js/app.js @@ -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); diff --git a/resources/js/modal-confirmation.js b/resources/js/modal-confirmation.js new file mode 100644 index 0000000000..c2bad2b578 --- /dev/null +++ b/resources/js/modal-confirmation.js @@ -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 }; +} diff --git a/resources/js/modal-confirmation.test.js b/resources/js/modal-confirmation.test.js new file mode 100644 index 0000000000..7613da7029 --- /dev/null +++ b/resources/js/modal-confirmation.test.js @@ -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: [] }); +}); diff --git a/resources/views/components/modal-confirmation.blade.php b/resources/views/components/modal-confirmation.blade.php index 6b0624a3bb..427e309eb2 100644 --- a/resources/views/components/modal-confirmation.blade.php +++ b/resources/views/components/modal-confirmation.blade.php @@ -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 diff --git a/resources/views/livewire/project/shared/dns-provider-management.blade.php b/resources/views/livewire/project/shared/dns-provider-management.blade.php index b33d073e13..e41d0f3ab2 100644 --- a/resources/views/livewire/project/shared/dns-provider-management.blade.php +++ b/resources/views/livewire/project/shared/dns-provider-management.blade.php @@ -28,7 +28,7 @@ @php($conflict = $dnsProviderConflicts[$key])