Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 95 additions & 5 deletions app/Rules/PhotoUrlRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,24 @@
namespace App\Rules;

use App\Repositories\ConfigManager;
use Closure;
use Illuminate\Contracts\Validation\ValidationRule;
use Safe\Exceptions\UrlException;
use function Safe\parse_url;

final class PhotoUrlRule implements ValidationRule
{
/**
*
* @param ConfigManager $config_manager
* @param Closure(string $hostname, int $type = ?, array &$authoritative_name_servers = ?, array &$additional_records = ?, bool $raw = ?): array|false $dns_get_record

Check failure on line 22 in app/Rules/PhotoUrlRule.php

View workflow job for this annotation

GitHub Actions / 1️⃣ PHP 8.4 - PHPStan

PHPDoc tag `@param` has invalid value (Closure(string $hostname, int $type = ?, array &$authoritative_name_servers = ?, array &$additional_records = ?, bool $raw = ?): array|false $dns_get_record): Unexpected token "(", expected variable at offset 67 on line 4
* @return void
*/
public function __construct(
private ConfigManager $config_manager,
private null|Closure $dns_get_record = null
) {
$this->dns_get_record = $dns_get_record ?? Closure::fromCallable('dns_get_record');
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/**
Expand Down Expand Up @@ -83,23 +92,104 @@
return;
}

$resolved_ips = $this->resolveHostToIPs($host);

if (
$this->config_manager->getValueAsBool('import_via_url_forbidden_local_ip') &&
filter_var($host, FILTER_VALIDATE_IP) !== false &&
filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) === false
$this->hasPrivateOrReservedIP($resolved_ips)
) {
$fail($attribute . ' must not be a private IP address.');
$fail($attribute . ' must not resolve to a private or reserved IP address.');

return;
}

if (
$this->config_manager->getValueAsBool('import_via_url_forbidden_localhost') &&
in_array(strtolower($host), ['localhost', '127.0.0.1', '::1'], true)
$this->hasLocalhostIP($host, $resolved_ips)
) {
$fail($attribute . ' must not be localhost.');
$fail($attribute . ' must not resolve to localhost.');

return;
}
}

/**
* Resolve a hostname to its IP addresses.
*
* If the host is already an IP address, return it directly.
*
* @param string $host
* @return string[]
*/
private function resolveHostToIPs(string $host): array
{
// If the host is already a valid IP, no resolution needed.
if (filter_var($host, FILTER_VALIDATE_IP) !== false) {
return [$host];
}

$ips = [];

try {
// Resolve A records (IPv4).
$a_records = call_user_func($this->dns_get_record, $host, DNS_A);
if ($a_records !== false) {
foreach ($a_records as $record) {
$ips[] = $record['ip'];
}
}

// Resolve AAAA records (IPv6).
$aaaa_records = call_user_func($this->dns_get_record, $host, DNS_AAAA);
if ($aaaa_records !== false) {
foreach ($aaaa_records as $record) {
$ips[] = $record['ipv6'];
}
}
} catch (\ErrorException) {
// DNS resolution failed — return empty array.
// The hostname checks (e.g. literal "localhost") still apply.
}

return $ips;
Comment thread
ildyria marked this conversation as resolved.
}

/**
* Check if any of the resolved IPs are private or reserved.
*
* @param string[] $ips
* @return bool
*/
private function hasPrivateOrReservedIP(array $ips): bool
{
foreach ($ips as $ip) {
if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) === false) {
return true;
}
}

return false;
}

/**
* Check if the host or any resolved IP is localhost.
*
* @param string $host
* @param string[] $ips
* @return bool
*/
private function hasLocalhostIP(string $host, array $ips): bool
{
if (strtolower($host) === 'localhost') {
return true;
}

foreach ($ips as $ip) {
if (str_starts_with($ip, '127.') || $ip === '::1') {
return true;
}
}

return false;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
52 changes: 52 additions & 0 deletions database/migrations/2026_03_22_094636_bump_version070502.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

/**
* SPDX-License-Identifier: MIT
* Copyright (c) 2017-2018 Tobias Reich
* Copyright (c) 2018-2026 LycheeOrg.
*/

use Illuminate\Database\Migrations\Migration;
use Illuminate\Support\Facades\Artisan;
use Illuminate\Support\Facades\DB;
use Symfony\Component\Console\Output\ConsoleOutput;
use Symfony\Component\Console\Output\ConsoleSectionOutput;

return new class() extends Migration {
private ConsoleOutput $output;
private ConsoleSectionOutput $msg_section;

public function __construct()
{
$this->output = new ConsoleOutput();
$this->msg_section = $this->output->section();
}

/**
* Run the migrations.
*
* @return void
*/
public function up(): void
{
DB::table('configs')->where('key', 'version')->update(['value' => '070502']);
try {
Artisan::call('cache:clear');
} catch (\Throwable $e) {
$this->msg_section->writeln('<error>Warning:</error> Failed to clear cache for version 7.5.2');

return;
}
$this->msg_section->writeln('<info>Info:</info> Cleared cache for version 7.5.2');
}

/**
* Reverse the migrations.
*
* @return void
*/
public function down(): void
{
DB::table('configs')->where('key', 'version')->update(['value' => '070501']);
}
};
65 changes: 60 additions & 5 deletions tests/Unit/Rules/PhotoUrlRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
namespace Tests\Unit\Rules;

use App\Models\Configs;
use App\Repositories\ConfigManager;
use App\Rules\PhotoUrlRule;
use Closure;
use Tests\AbstractTestCase;

class PhotoUrlRuleTest extends AbstractTestCase
Expand All @@ -31,7 +33,7 @@ class PhotoUrlRuleTest extends AbstractTestCase
public function setUp(): void
{
parent::setUp();
$this->rule = resolve(PhotoUrlRule::class);
$this->rule = $this->makeRule();
$this->failCalled = false;
$this->failMessage = '';

Expand All @@ -41,6 +43,19 @@ public function setUp(): void
Configs::set('import_via_url_forbidden_localhost', '1');
}

/**
* Create a PhotoUrlRule with an optional mock DNS resolver.
*
* @param Closure|null $dns_get_record
*/
private function makeRule(?Closure $dns_get_record = null): PhotoUrlRule
{
return new PhotoUrlRule(
resolve(ConfigManager::class),
$dns_get_record ?? fn (string $hostname, int $type = DNS_A) => [],
);
}

public function tearDown(): void
{
Configs::set('import_via_url_require_https', '1');
Expand Down Expand Up @@ -102,6 +117,10 @@ public function testHttpsRequiredButNotProvided(): void
*/
public function testHttpsRequiredAndProvided(): void
{
$this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) {
DNS_A => [['ip' => '93.184.216.34']],
default => [],
});
$this->rule->validate('photo_url', 'https://example.com', fn ($m) => $this->m($m));
self::assertFalse($this->failCalled);
self::assertEquals('', $this->failMessage);
Expand Down Expand Up @@ -134,9 +153,11 @@ public function testForbiddenPort(): void
*/
public function testAllowedPort(): void
{
$this->rule->validate('photo_url', 'https://example.com:80', function ($message) {
$this->failCalled = true;
$this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) {
DNS_A => [['ip' => '93.184.216.34']],
default => [],
});
$this->rule->validate('photo_url', 'https://example.com:80', fn ($m) => $this->m($m));

self::assertFalse($this->failCalled);
self::assertEquals('', $this->failMessage);
Expand All @@ -149,17 +170,51 @@ public function testForbiddenPrivateIp(): void
{
$this->rule->validate('photo_url', 'https://192.168.1.1', fn ($m) => $this->m($m));
self::assertTrue($this->failCalled);
self::assertEquals('photo_url must not be a private IP address.', $this->failMessage);
self::assertEquals('photo_url must not resolve to a private or reserved IP address.', $this->failMessage);
}

/**
* Test validation when localhost is forbidden and localhost is provided.
*/
public function testForbiddenLocalhost(): void
{
// Disable private IP check so we specifically test the localhost check.
Configs::set('import_via_url_forbidden_local_ip', '0');

$this->rule->validate('photo_url', 'https://localhost', fn ($m) => $this->m($m));
self::assertTrue($this->failCalled);
self::assertEquals('photo_url must not be localhost.', $this->failMessage);
self::assertEquals('photo_url must not resolve to localhost.', $this->failMessage);
}

/**
* Test that hostnames resolving to private IPs are blocked (DNS rebinding protection).
*/
public function testForbiddenPrivateIpViaHostname(): void
{
$this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) {
DNS_A => [['ip' => '127.0.0.1']],
Comment thread
ildyria marked this conversation as resolved.
Outdated
default => [],
});
$this->rule->validate('photo_url', 'https://evil.example.com/test.jpg', fn ($m) => $this->m($m));
self::assertTrue($this->failCalled);
self::assertEquals('photo_url must not resolve to a private or reserved IP address.', $this->failMessage);
}

/**
* Test that hostnames resolving to localhost are blocked.
*/
public function testForbiddenLocalhostViaHostname(): void
{
$this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) {
DNS_A => [['ip' => '127.0.0.1']],
default => [],
});

Configs::set('import_via_url_forbidden_local_ip', '0');

$this->rule->validate('photo_url', 'https://evil.example.com/test.jpg', fn ($m) => $this->m($m));
self::assertTrue($this->failCalled);
self::assertEquals('photo_url must not resolve to localhost.', $this->failMessage);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion version.md
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7.5.1
7.5.2
Loading