diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 4f72bd78..311c4cbe 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -37,6 +37,8 @@ use App\Models\WorkspaceInvite; use App\Models\WorkspaceLabel; use App\Models\WorkspaceSignature; +use App\Services\Post\DnsUrlSafetyGuard; +use App\Services\Post\UrlSafetyGuard; use App\Services\PostTemplate\Registry as PostTemplateRegistry; use App\Socialite\InstagramProvider; use App\Socialite\LinkedInPageExtendSocialite; @@ -81,6 +83,7 @@ class AppServiceProvider extends ServiceProvider public function register(): void { $this->app->singleton(PostTemplateRegistry::class); + $this->app->bind(UrlSafetyGuard::class, DnsUrlSafetyGuard::class); if ($this->app->environment('local') && class_exists(\Laravel\Telescope\TelescopeServiceProvider::class)) { $this->app->register(\Laravel\Telescope\TelescopeServiceProvider::class); diff --git a/app/Services/Post/DnsUrlSafetyGuard.php b/app/Services/Post/DnsUrlSafetyGuard.php new file mode 100644 index 00000000..0740fc3c --- /dev/null +++ b/app/Services/Post/DnsUrlSafetyGuard.php @@ -0,0 +1,60 @@ +ipIsPublic($host); + } + + $records = @dns_get_record($host, DNS_A | DNS_AAAA); + + if ($records === false || $records === []) { + return false; + } + + foreach ($records as $record) { + $ip = $record['ip'] ?? $record['ipv6'] ?? null; + if (! is_string($ip) || ! $this->ipIsPublic($ip)) { + return false; + } + } + + return true; + } + + private function ipIsPublic(string $ip): bool + { + return filter_var( + $ip, + FILTER_VALIDATE_IP, + FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE, + ) !== false; + } +} diff --git a/app/Services/Post/MediaAttacher.php b/app/Services/Post/MediaAttacher.php index b8db1a69..cc4e2ae9 100644 --- a/app/Services/Post/MediaAttacher.php +++ b/app/Services/Post/MediaAttacher.php @@ -8,15 +8,19 @@ use App\Models\Media; use App\Models\Post; use App\Models\Workspace; +use Illuminate\Http\File; use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Str; /** - * Downloads media from public URLs and attaches them to a post. Used by both - * the MCP `AttachMediaFromUrlTool` and the REST `POST /api/posts/{post}/media` - * endpoint so behaviour and validation stay aligned. + * Orchestrates "URL → media on a post" — given a list of public URLs, + * download each via `MediaDownloader`, validate the MIME against the + * post's enabled platforms, persist the file on the configured Storage + * disk, and append a Media record to the post. + * + * Used by both the MCP `AttachMediaFromUrlTool` and the REST + * `POST /api/posts/{post}/media` endpoint so behaviour stays aligned. */ class MediaAttacher { @@ -26,6 +30,10 @@ class MediaAttacher private const MAX_BYTES = 50 * 1024 * 1024; // 50 MB + public function __construct( + private readonly MediaDownloader $downloader, + ) {} + /** * @param array $urls * @return array{attached: array>, failed: array} @@ -38,7 +46,7 @@ public function attachFromUrls(Post $post, array $urls): array $failed = []; foreach ($urls as $url) { - $item = $this->downloadAndStore($post->workspace, $url, $allowedTypes); + $item = $this->processOne($post->workspace, $url, $allowedTypes); if ($item === null) { $failed[] = $url; @@ -50,23 +58,93 @@ public function attachFromUrls(Post $post, array $urls): array } if ($attached !== []) { - // Lock + reload before merging so concurrent attach calls don't - // overwrite each other's appended items (lost-update race). - DB::transaction(function () use ($post, $attached) { - $fresh = Post::whereKey($post->id)->lockForUpdate()->first(); - $fresh->update([ - 'media' => collect($fresh->media ?? [])->concat($attached)->all(), - ]); - $post->setRawAttributes($fresh->getAttributes(), true); - }); + $this->mergeIntoPostMedia($post, $attached); } return ['attached' => $attached, 'failed' => $failed]; } /** - * Intersection of allowed media types across platforms enabled on the - * post. If no platform is enabled, accept anything supported. + * @param array $allowedTypes + * @return array|null + */ + private function processOne(Workspace $workspace, string $url, array $allowedTypes): ?array + { + $download = $this->downloader->download($url, self::MAX_BYTES); + + if ($download === null) { + return null; + } + + try { + $type = $this->resolveType(data_get($download, 'mime')); + + if ($type === null || ! in_array($type, $allowedTypes, true)) { + return null; + } + + return $this->storeMedia($workspace, $download, $type, $url); + } finally { + @unlink(data_get($download, 'path')); + } + } + + /** + * @param array{path: string, mime: ?string, bytes: int} $download + * @return array + */ + private function storeMedia(Workspace $workspace, array $download, MediaType $type, string $url): array + { + $mime = data_get($download, 'mime'); + $extension = $this->extensionFor($mime, $url); + $filename = 'media/'.Str::uuid()->toString().'.'.$extension; + $originalFilename = basename(parse_url($url, PHP_URL_PATH) ?? '') ?: 'download.'.$extension; + + Storage::putFileAs('', new File(data_get($download, 'path')), $filename); + + $media = new Media([ + 'collection' => 'post-media', + 'type' => $type, + 'path' => $filename, + 'original_filename' => $originalFilename, + 'mime_type' => $mime ?? '', + 'size' => data_get($download, 'bytes'), + 'order' => 0, + ]); + $media->mediable_type = Workspace::class; + $media->mediable_id = $workspace->id; + $media->save(); + + return [ + 'id' => $media->id, + 'path' => $media->path, + 'url' => $media->url, + 'type' => $type->value, + 'mime_type' => $media->mime_type, + 'original_filename' => $media->original_filename, + ]; + } + + /** + * Lock-then-merge so concurrent attach calls don't overwrite each + * other's appended items in the JSON `media` column. + * + * @param array> $attached + */ + private function mergeIntoPostMedia(Post $post, array $attached): void + { + DB::transaction(function () use ($post, $attached): void { + $fresh = Post::whereKey($post->id)->lockForUpdate()->first(); + $fresh->update([ + 'media' => collect($fresh->media ?? [])->concat($attached)->all(), + ]); + $post->setRawAttributes($fresh->getAttributes(), true); + }); + } + + /** + * Intersection of allowed media types across platforms enabled on + * the post. With no enabled platform, accept anything we support. * * @return array */ @@ -92,146 +170,6 @@ private function allowedMediaTypesFor(Post $post): array return array_map(fn ($value) => MediaType::from($value), $intersection); } - /** - * @param array $allowedTypes - * @return array|null - */ - private function downloadAndStore(Workspace $workspace, string $url, array $allowedTypes): ?array - { - if (! $this->isPublicHttpUrl($url)) { - return null; - } - - // Disable redirects (a public URL could 302 to an internal target), - // stream the body, and abort once we exceed MAX_BYTES so a malicious - // host can't exhaust memory or our process timeout. - $response = Http::timeout(20) - ->withOptions([ - 'allow_redirects' => false, - 'stream' => true, - ]) - ->get($url); - - if (! $response->successful()) { - return null; - } - - $body = ''; - $bytes = 0; - $stream = $response->toPsrResponse()->getBody(); - - while (! $stream->eof()) { - $chunk = $stream->read(8192); - $bytes += strlen($chunk); - - if ($bytes > self::MAX_BYTES) { - return null; - } - - $body .= $chunk; - } - - if ($bytes === 0) { - return null; - } - - $mime = $response->header('Content-Type'); - $mime = $mime ? trim(explode(';', $mime)[0]) : null; - - $type = $this->resolveType($mime); - - if ($type === null || ! in_array($type, $allowedTypes, true)) { - return null; - } - - $extension = $this->extensionFor($mime, $url); - $filename = 'media/'.Str::uuid()->toString().'.'.$extension; - $originalFilename = basename(parse_url($url, PHP_URL_PATH) ?? '') ?: 'download.'.$extension; - - Storage::put($filename, $body); - - $media = new Media([ - 'collection' => 'post-media', - 'type' => $type, - 'path' => $filename, - 'original_filename' => $originalFilename, - 'mime_type' => $mime ?? '', - 'size' => $bytes, - 'order' => 0, - ]); - $media->mediable_type = Workspace::class; - $media->mediable_id = $workspace->id; - $media->save(); - - return [ - 'id' => $media->id, - 'path' => $media->path, - 'url' => $media->url, - 'type' => $type->value, - 'mime_type' => $media->mime_type, - 'original_filename' => $media->original_filename, - ]; - } - - /** - * Reject anything that isn't a plain http(s) URL targeting a public host. - * Blocks loopback, link-local, private, and reserved ranges so a caller - * can't pivot from us into the internal network (SSRF). - */ - private function isPublicHttpUrl(string $url): bool - { - $parts = parse_url($url); - - if (! is_array($parts) || ! in_array(data_get($parts, 'scheme'), ['http', 'https'], true)) { - return false; - } - - $host = data_get($parts, 'host'); - - if (! is_string($host) || $host === '') { - return false; - } - - // Under `Http::fake()` the HTTP facade short-circuits real network - // calls; skip DNS resolution so tests can stub responses for synthetic - // hosts without our SSRF guard rejecting them. - if (app()->runningUnitTests()) { - return true; - } - - // Reject literal IPv4/IPv6 host inputs that fall in restricted ranges. - if (filter_var($host, FILTER_VALIDATE_IP) !== false) { - return $this->ipIsPublic($host); - } - - // For DNS hostnames, resolve and check every record. Fail closed - // (no records / unresolvable / private) to prevent DNS-rebinding tricks - // where the first lookup is public and the second resolves internally. - $records = @dns_get_record($host, DNS_A | DNS_AAAA); - - if ($records === false || $records === []) { - return false; - } - - foreach ($records as $record) { - $ip = $record['ip'] ?? $record['ipv6'] ?? null; - if (! is_string($ip) || ! $this->ipIsPublic($ip)) { - return false; - } - } - - return true; - } - - private function ipIsPublic(string $ip): bool - { - return filter_var( - $ip, - FILTER_VALIDATE_IP, - FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE, - ) !== false; - } - private function resolveType(?string $mime): ?MediaType { if ($mime === null) { diff --git a/app/Services/Post/MediaDownloader.php b/app/Services/Post/MediaDownloader.php new file mode 100644 index 00000000..43632351 --- /dev/null +++ b/app/Services/Post/MediaDownloader.php @@ -0,0 +1,78 @@ +guard->isSafe($url)) { + return null; + } + + $temp = tempnam(sys_get_temp_dir(), 'media_'); + + try { + $response = Http::timeout(20) + ->sink($temp) + ->withOptions([ + 'allow_redirects' => false, + 'progress' => static function ($total, $downloaded) use ($maxBytes): void { + if ($downloaded > $maxBytes) { + throw new RuntimeException('exceeded max bytes'); + } + }, + ]) + ->get($url); + } catch (RuntimeException) { + @unlink($temp); + + return null; + } + + if (! $response->successful()) { + @unlink($temp); + + return null; + } + + $bytes = filesize($temp) ?: 0; + + if ($bytes === 0 || $bytes > $maxBytes) { + @unlink($temp); + + return null; + } + + $mime = $response->header('Content-Type'); + $mime = $mime ? trim(explode(';', $mime)[0]) : null; + + return [ + 'path' => $temp, + 'mime' => $mime, + 'bytes' => $bytes, + ]; + } +} diff --git a/app/Services/Post/UrlSafetyGuard.php b/app/Services/Post/UrlSafetyGuard.php new file mode 100644 index 00000000..5b757abb --- /dev/null +++ b/app/Services/Post/UrlSafetyGuard.php @@ -0,0 +1,16 @@ +withoutVite(); + + // Bypass the DNS-based SSRF guard during tests. Feature tests use + // Http::fake() with synthetic hosts (e.g. cdn.example.com) that + // wouldn't resolve to public IPs; the real guard is exercised by + // tests/Unit/Services/Post/DnsUrlSafetyGuardTest in isolation. + $this->app->bind(UrlSafetyGuard::class, fn () => new class implements UrlSafetyGuard + { + public function isSafe(string $url): bool + { + return true; + } + }); } } diff --git a/tests/Unit/Services/Post/DnsUrlSafetyGuardTest.php b/tests/Unit/Services/Post/DnsUrlSafetyGuardTest.php new file mode 100644 index 00000000..adef78f9 --- /dev/null +++ b/tests/Unit/Services/Post/DnsUrlSafetyGuardTest.php @@ -0,0 +1,71 @@ +guard = new DnsUrlSafetyGuard; +}); + +test('rejects non-http(s) schemes', function () { + expect($this->guard->isSafe('ftp://example.com/file.zip'))->toBeFalse(); + expect($this->guard->isSafe('file:///etc/passwd'))->toBeFalse(); + expect($this->guard->isSafe('javascript:alert(1)'))->toBeFalse(); + expect($this->guard->isSafe('gopher://example.com'))->toBeFalse(); +}); + +test('rejects malformed URLs', function () { + expect($this->guard->isSafe('not-a-url'))->toBeFalse(); + expect($this->guard->isSafe('http://'))->toBeFalse(); + expect($this->guard->isSafe(''))->toBeFalse(); +}); + +test('rejects IPv4 hosts in restricted ranges', function () { + // Loopback + expect($this->guard->isSafe('http://127.0.0.1/'))->toBeFalse(); + expect($this->guard->isSafe('http://127.255.255.254/'))->toBeFalse(); + + // RFC1918 private + expect($this->guard->isSafe('http://10.0.0.1/'))->toBeFalse(); + expect($this->guard->isSafe('http://172.16.0.1/'))->toBeFalse(); + expect($this->guard->isSafe('http://192.168.1.1/'))->toBeFalse(); + + // Link-local + AWS metadata endpoint + expect($this->guard->isSafe('http://169.254.169.254/latest/meta-data'))->toBeFalse(); + + // Reserved zero / broadcast + expect($this->guard->isSafe('http://0.0.0.0/'))->toBeFalse(); + expect($this->guard->isSafe('http://255.255.255.255/'))->toBeFalse(); +}); + +test('rejects IPv6 hosts in restricted ranges', function () { + // Loopback + expect($this->guard->isSafe('http://[::1]/'))->toBeFalse(); + + // Unique local (fc00::/7) + expect($this->guard->isSafe('http://[fd00::1]/'))->toBeFalse(); + + // Link-local (fe80::/10) + expect($this->guard->isSafe('http://[fe80::1]/'))->toBeFalse(); +}); + +test('accepts a literal public IPv4', function () { + // 1.1.1.1 (Cloudflare DNS) is a stable public IP we can hard-code. + expect($this->guard->isSafe('http://1.1.1.1/'))->toBeTrue(); + expect($this->guard->isSafe('https://8.8.8.8/'))->toBeTrue(); +}); + +test('accepts a hostname that resolves to public IPs', function () { + // example.com is reserved by IANA for documentation; resolves stably + // and lives at public IPs. + expect($this->guard->isSafe('https://example.com/'))->toBeTrue(); +})->skip(getenv('CI') === 'true', 'depends on outbound DNS, skipped on CI'); + +test('rejects a hostname with no DNS records', function () { + // .invalid is reserved (RFC 2606) — guaranteed not to resolve. + expect($this->guard->isSafe('http://does-not-exist.invalid/'))->toBeFalse(); +})->skip(getenv('CI') === 'true', 'depends on DNS resolution behavior, skipped on CI');