vignesh/trypost
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor: collapse media-attach helpers back into MediaAttacher

Browse source 
The previous split (MediaAttacher + MediaDownloader + UrlSafetyGuard
interface + DnsUrlSafetyGuard impl + provider binding) was over-engineered
for one production implementation. Roll it all back into a single
MediaAttacher class and delegate to the existing `Workspace::addMediaFromPath`
helper for storage + Media row creation (the same path the web upload
flow uses).

What's gone:
- app/Services/Post/MediaDownloader.php
- app/Services/Post/UrlSafetyGuard.php (interface)
- app/Services/Post/DnsUrlSafetyGuard.php (impl)
- tests/Unit/Services/Post/DnsUrlSafetyGuardTest.php
- The provider bind in AppServiceProvider

What's still defended:
- Streaming download via Http::sink with a Guzzle progress callback that
  aborts mid-stream once MAX_BYTES is exceeded (no body buffered in PHP).
- Redirects disabled so a 200→302 trick can't pivot to internal hosts.
- IP-literal SSRF guard: rejects loopback / private / link-local /
  reserved ranges via FILTER_FLAG_NO_PRIV_RANGE | NO_RES_RANGE. DNS
  hostnames are accepted — finer SSRF (DNS rebinding etc.) is left to
  network-level egress controls. This is a deliberate simplification:
  the prior DNS resolution defense added a class + interface + provider
  binding for marginal gain when the realistic attack surface is
  hard-coded internal IPs.
- Strict MIME allowlist (no SVG, no PDF, no application/*).
- Lock-then-merge into post.media[] to avoid lost-update races.

Tests bypass the SSRF check via the static `MediaAttacher::fakeUrlSafety()`
called once in tests/TestCase::setUp — same idiom as Mail::fake / Bus::fake.
This commit is contained in:
Paulo Castellano 2026-05-04 14:02:50 -03:00
parent c871edf144
commit c3b8af7189
7 changed files with 104 additions and 312 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
app/Providers/AppServiceProvider.php
View file

@ -37,8 +37,6 @@
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;
@ -83,7 +81,6 @@ 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);

60
app/Services/Post/DnsUrlSafetyGuard.php
View file

@ -1,60 +0,0 @@
<?php
declare(strict_types=1);
namespace App\Services\Post;
/**
* Default UrlSafetyGuard. Rejects:
* - non-http(s) schemes
* - missing or empty hosts
* - literal IPv4/IPv6 hosts in restricted ranges
* - DNS hostnames whose A/AAAA records resolve into restricted ranges
* (covers DNS-rebinding attempts where the first lookup is public and
* subsequent lookups resolve internally)
*/
class DnsUrlSafetyGuard implements UrlSafetyGuard
{
public function isSafe(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;
}
if (filter_var($host, FILTER_VALIDATE_IP) !== false) {
return $this->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;
}
}

172
app/Services/Post/MediaAttacher.php
View file

@ -5,22 +5,38 @@
namespace App\Services\Post;
use App\Enums\Media\Type as MediaType;
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\Storage;
use Illuminate\Support\Str;
use Illuminate\Support\Facades\Http;
use RuntimeException;
/**
* 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.
* Downloads public URLs and attaches them as media to a post. Used by
* both the MCP `AttachMediaFromUrlTool` and the REST `POST /api/posts/{post}/media`
* endpoint.
*
* Used by both the MCP `AttachMediaFromUrlTool` and the REST
* `POST /api/posts/{post}/media` endpoint so behaviour stays aligned.
* Flow per URL:
* 1. Reject the URL if its host is a literal IP in a restricted range
* (loopback / private / link-local / reserved). DNS hostnames go
* through; we trust the upstream firewall / egress controls for
* finer-grained SSRF defense.
* 2. Stream the body to a temp file via Http::sink + a progress
* callback that aborts mid-download once MAX_BYTES is exceeded —
* memory stays bounded.
* 3. Validate the Content-Type against an allowlist (no SVG, no PDF)
* AND the intersection of allowed media types across the post's
* enabled platforms.
* 4. Hand off to `Workspace::addMediaFromPath()` (the same helper the
* web upload flow uses) so storage path, MIME re-detection, image
* normalization, and the Media row stay in one place.
* 5. Append the resulting media item to the post's `media[]` JSON
* column under a row lock so concurrent attach calls don't clobber
* each other.
*
* Tests bypass the SSRF check via `MediaAttacher::fakeUrlSafety()`
* (called in tests/TestCase) so synthetic Http::fake hosts aren't
* rejected.
*/
class MediaAttacher
{
@ -30,9 +46,17 @@ class MediaAttacher
private const MAX_BYTES = 50 * 1024 * 1024; // 50 MB
public function __construct(
private readonly MediaDownloader $downloader,
) {}
private static bool $skipUrlSafety = false;
public static function fakeUrlSafety(): void
{
self::$skipUrlSafety = true;
}
public static function resetUrlSafety(): void
{
self::$skipUrlSafety = false;
}
/**
* @param array<int, string> $urls
@ -70,59 +94,87 @@ public function attachFromUrls(Post $post, array $urls): array
*/
private function processOne(Workspace $workspace, string $url, array $allowedTypes): ?array
{
$download = $this->downloader->download($url, self::MAX_BYTES);
if ($download === null) {
if (! $this->isUrlSafe($url)) {
return null;
}
$temp = tempnam(sys_get_temp_dir(), 'media_');
try {
$type = $this->resolveType(data_get($download, 'mime'));
$response = Http::timeout(20)
->sink($temp)
->withOptions([
'allow_redirects' => false,
'progress' => static function ($total, $downloaded): void {
if ($downloaded > self::MAX_BYTES) {
throw new RuntimeException('exceeded max bytes');
}
},
])
->get($url);
if (! $response->successful() || filesize($temp) === 0) {
return null;
}
$mime = trim(explode(';', (string) $response->header('Content-Type'))[0]);
$type = $this->resolveType($mime);
if ($type === null || ! in_array($type, $allowedTypes, true)) {
return null;
}
return $this->storeMedia($workspace, $download, $type, $url);
$originalFilename = basename(parse_url($url, PHP_URL_PATH) ?? '') ?: 'download.bin';
$media = $workspace->addMediaFromPath($temp, $originalFilename, 'assets');
return [
'id' => $media->id,
'path' => $media->path,
'url' => $media->url,
'type' => $media->type,
'mime_type' => $media->mime_type,
'original_filename' => $media->original_filename,
];
} catch (RuntimeException) {
return null;
} finally {
@unlink(data_get($download, 'path'));
@unlink($temp);
}
}
/**
* @param array{path: string, mime: ?string, bytes: int} $download
* @return array<string, mixed>
* Reject obvious SSRF targets: non-http(s) schemes, missing host,
* and IP-literal hosts in private / loopback / link-local / reserved
* ranges. DNS hostnames are accepted — finer-grained protection
* (DNS rebinding, etc.) is left to network-level controls.
*/
private function storeMedia(Workspace $workspace, array $download, MediaType $type, string $url): array
private function isUrlSafe(string $url): bool
{
$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;
if (self::$skipUrlSafety) {
return true;
}
Storage::putFileAs('', new File(data_get($download, 'path')), $filename);
$parts = parse_url($url);
$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();
if (! is_array($parts) || ! in_array(data_get($parts, 'scheme'), ['http', 'https'], true)) {
return false;
}
return [
'id' => $media->id,
'path' => $media->path,
'url' => $media->url,
'type' => $type->value,
'mime_type' => $media->mime_type,
'original_filename' => $media->original_filename,
];
$host = data_get($parts, 'host');
if (! is_string($host) || $host === '') {
return false;
}
if (filter_var($host, FILTER_VALIDATE_IP) !== false) {
return filter_var(
$host,
FILTER_VALIDATE_IP,
FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE,
) !== false;
}
return true;
}
/**
@ -172,7 +224,7 @@ private function allowedMediaTypesFor(Post $post): array
private function resolveType(?string $mime): ?MediaType
{
if ($mime === null) {
if ($mime === null || $mime === '') {
return null;
}
@ -186,28 +238,4 @@ private function resolveType(?string $mime): ?MediaType
return null;
}
private function extensionFor(?string $mime, string $url): string
{
$byMime = match ($mime) {
'image/jpeg' => 'jpg',
'image/png' => 'png',
'image/gif' => 'gif',
'image/webp' => 'webp',
'video/mp4' => 'mp4',
'video/quicktime' => 'mov',
'video/webm' => 'webm',
default => null,
};
if ($byMime) {
return $byMime;
}
$byUrl = strtolower(pathinfo(parse_url($url, PHP_URL_PATH) ?? '', PATHINFO_EXTENSION));
return in_array($byUrl, ['jpg', 'jpeg', 'png', 'gif', 'webp', 'mp4', 'mov', 'webm'], true)
? $byUrl
: 'bin';
}
}

78
app/Services/Post/MediaDownloader.php
View file

@ -1,78 +0,0 @@
<?php
declare(strict_types=1);
namespace App\Services\Post;
use Illuminate\Support\Facades\Http;
use RuntimeException;
/**
* Downloads a single media file from a public URL into a local temporary
* file, streaming the body to disk so we don't buffer it in PHP memory.
* Aborts mid-stream once `maxBytes` is exceeded.
*
* The caller owns the temp file lifecycle — receive `path`, do whatever
* (validate / upload to Storage), then delete it.
*/
class MediaDownloader
{
public function __construct(
private readonly UrlSafetyGuard $guard,
) {}
/**
* @return array{path: string, mime: ?string, bytes: int}|null
* null when the URL is unsafe, the response failed, or the
* download exceeded `maxBytes`.
*/
public function download(string $url, int $maxBytes): ?array
{
if (! $this->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,
];
}
}

16
app/Services/Post/UrlSafetyGuard.php
View file

@ -1,16 +0,0 @@
<?php
declare(strict_types=1);
namespace App\Services\Post;
/**
* Decides whether a user-supplied URL is safe to fetch from the server side.
* The default implementation (DnsUrlSafetyGuard) blocks loopback / private /
* link-local / reserved IP ranges to prevent SSRF; tests bind a permissive
* implementation so synthetic hosts under Http::fake() aren't rejected.
*/
interface UrlSafetyGuard
{
public function isSafe(string $url): bool;
}

16
tests/TestCase.php
View file

@ -4,7 +4,7 @@
namespace Tests;
use App\Services\Post\UrlSafetyGuard;
use App\Services\Post\MediaAttacher;
use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
abstract class TestCase extends BaseTestCase
@ -24,16 +24,8 @@ protected function setUp(): void
$this->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;
}
});
// Bypass the SSRF check during tests so Http::fake() with
// synthetic hosts like cdn.example.com isn't rejected.
MediaAttacher::fakeUrlSafety();
}
}

71
tests/Unit/Services/Post/DnsUrlSafetyGuardTest.php
View file

@ -1,71 +0,0 @@
<?php
declare(strict_types=1);
use App\Services\Post\DnsUrlSafetyGuard;
beforeEach(function () {
// The TestCase setUp binds a permissive UrlSafetyGuard for feature
// tests; this unit test exercises the real DnsUrlSafetyGuard directly,
// so we instantiate it instead of resolving from the container.
$this->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');