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

refactor: move URL validation to the request layer with active_url

Browse source 
The MediaAttacher used to roll its own SSRF guard with DNS resolution
and a static fakeUrlSafety() flag for tests. Validating URLs is a
request-layer concern, not a service-layer one. Laravel ships
'active_url' which does the same DNS resolvability check via
dns_get_record — applying it at the FormRequest / MCP validate() level
catches dead URLs upfront with a proper 422 instead of letting the
download silently fail.

- Replace the inline 'urls.*' => ['url:http,https'] rule with
  ['url:http,https', 'active_url'] in both Api/PostController::attachMedia
  and Mcp/Tools/Post/AttachMediaFromUrlTool.
- Drop isUrlSafe(), fakeUrlSafety(), resetUrlSafety(), $skipUrlSafety
  from MediaAttacher. The remaining defenses (Http::sink streaming +
  progress abort at MAX_BYTES, allow_redirects: false, MIME allowlist)
  cover the operational concerns.
- Restore tests/TestCase to the original setUp — no SSRF bypass needed
  anymore because active_url is satisfied by the test hosts.
- Swap synthetic test hosts (cdn.example.com / evil.example.com) for
  example.com / example.org. Both are RFC-reserved AND have stable A
  records, so active_url accepts them while Http::fake() still
  intercepts the actual request.

For SSRF defense beyond 'active_url' (which doesn't block private IPs),
trypost relies on production network egress controls. Open-source
self-hosters who run without a firewall accept the corresponding risk;
that's a deployment concern, not a request validation concern.
This commit is contained in:
Paulo Castellano 2026-05-04 14:35:28 -03:00
parent bc581f54f2
commit 4892ee75a5
6 changed files with 35 additions and 97 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/Http/Controllers/Api/PostController.php
View file

@ -88,7 +88,7 @@ public function attachMedia(Request $request, Post $post): PostMediaAttachResour
$validated = $request->validate([
'urls' => ['required', 'array', 'min:1', 'max:10'],
'urls.*' => ['url:http,https'],
'urls.*' => ['url:http,https', 'active_url'],
]);
$result = app(MediaAttacher::class)->attachFromUrls($post, $validated['urls']);

2
app/Mcp/Tools/Post/AttachMediaFromUrlTool.php
View file

@ -22,7 +22,7 @@ public function handle(Request $request): Response|ResponseFactory
$validated = $request->validate([
'post_id' => ['required', 'uuid'],
'urls' => ['required', 'array', 'min:1', 'max:10'],
'urls.*' => ['url:http,https'],
'urls.*' => ['url:http,https', 'active_url'],
]);
$post = Post::where('workspace_id', $request->user()->current_workspace_id)

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

@ -16,51 +16,33 @@
* both the MCP `AttachMediaFromUrlTool` and the REST `POST /api/posts/{post}/media`
* endpoint.
*
* URL syntax (`url:http,https`) and DNS resolvability (`active_url`) are
* enforced at the request validation layer. SSRF defense beyond that is
* the responsibility of network-level egress controls in production.
*
* 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)
* 1. Stream the body to a temp file via Http::sink + a progress
* callback that aborts mid-download once MAX_BYTES is exceeded.
* 2. Validate the Content-Type against the MediaType enum's allow-list
* 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
* 3. Hand off to `Workspace::addMediaFromPath()` so storage path,
* MIME re-detection, image normalization, and the Media row stay
* in one place (same path as the web upload flow).
* 4. 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
{
/**
* Cap on URL-fetched payloads. Smaller than the web upload cap (which
* can be 1 GB for direct uploads) because URL fetches have stricter
* server-side concerns: bandwidth, timeout, and unbounded user input.
* 50 MB covers a long photo or a short video; bigger files should be
* uploaded directly.
* Cap on URL-fetched payloads. Smaller than the web upload cap (1 GB)
* because URL fetches have different operational constraints:
* bandwidth, timeout, and unbounded user input. 50 MB covers a long
* photo or a short video; bigger files should be uploaded directly.
*/
private const MAX_BYTES = 50 * 1024 * 1024;
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
* @return array{attached: array<int, array<string, mixed>>, failed: array<int, string>}
@ -97,10 +79,6 @@ public function attachFromUrls(Post $post, array $urls): array
*/
private function processOne(Workspace $workspace, string $url, array $allowedTypes): ?array
{
if (! $this->isUrlSafe($url)) {
return null;
}
$temp = tempnam(sys_get_temp_dir(), 'media_');
try {
@ -145,41 +123,6 @@ private function processOne(Workspace $workspace, string $url, array $allowedTyp
}
}
/**
* 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 isUrlSafe(string $url): bool
{
if (self::$skipUrlSafety) {
return true;
}
$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 filter_var(
$host,
FILTER_VALIDATE_IP,
FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE,
) !== false;
}
return true;
}
/**
* Lock-then-merge so concurrent attach calls don't overwrite each
* other's appended items in the JSON `media` column.

12
tests/Feature/Api/PostMediaApiTest.php
View file

@ -38,7 +38,7 @@
it('attaches media from url', function () {
Http::fake([
'cdn.example.com/photo.png' => Http::response(
'example.com/photo.png' => Http::response(
file_get_contents(__DIR__.'/../../fixtures/1x1.png'),
200,
['Content-Type' => 'image/png'],
@ -47,7 +47,7 @@
$this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken])
->postJson(route('api.posts.attach-media', $this->post), [
'urls' => ['https://cdn.example.com/photo.png'],
'urls' => ['https://example.com/photo.png'],
])
->assertOk()
->assertJsonPath('attached_count', 1)
@ -59,16 +59,16 @@
it('reports failures for unreachable urls', function () {
Http::fake([
'cdn.example.com/missing.png' => Http::response(null, 404),
'example.com/missing.png' => Http::response(null, 404),
]);
$this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken])
->postJson(route('api.posts.attach-media', $this->post), [
'urls' => ['https://cdn.example.com/missing.png'],
'urls' => ['https://example.com/missing.png'],
])
->assertOk()
->assertJsonPath('attached_count', 0)
->assertJsonPath('failed_urls.0', 'https://cdn.example.com/missing.png');
->assertJsonPath('failed_urls.0', 'https://example.com/missing.png');
});
it('cannot attach media to a post from another workspace', function () {
@ -77,7 +77,7 @@
$this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken])
->postJson(route('api.posts.attach-media', $post), [
'urls' => ['https://cdn.example.com/photo.png'],
'urls' => ['https://example.com/photo.png'],
])
->assertNotFound();
});

24
tests/Feature/Mcp/AttachMediaFromUrlToolTest.php
View file

@ -28,7 +28,7 @@
test('attaches an image from url and creates a media row', function () {
Http::fake([
'cdn.example.com/photo.jpg' => Http::response(
'example.com/photo.jpg' => Http::response(
file_get_contents(__DIR__.'/../../fixtures/1x1.png'),
200,
['Content-Type' => 'image/png'],
@ -38,7 +38,7 @@
$response = TryPostServer::actingAs($this->user)
->tool(AttachMediaFromUrlTool::class, [
'post_id' => $this->post->id,
'urls' => ['https://cdn.example.com/photo.jpg'],
'urls' => ['https://example.com/photo.jpg'],
]);
$response->assertOk();
@ -49,13 +49,13 @@
test('rejects url that returns non-image content type', function () {
Http::fake([
'evil.example.com/payload' => Http::response('not an image', 200, ['Content-Type' => 'text/html']),
'example.org/payload' => Http::response('not an image', 200, ['Content-Type' => 'text/html']),
]);
$response = TryPostServer::actingAs($this->user)
->tool(AttachMediaFromUrlTool::class, [
'post_id' => $this->post->id,
'urls' => ['https://evil.example.com/payload'],
'urls' => ['https://example.org/payload'],
]);
$response->assertOk();
@ -66,25 +66,25 @@
test('reports failures and successes separately', function () {
Http::fake([
'cdn.example.com/ok.png' => Http::response(
'example.com/ok.png' => Http::response(
file_get_contents(__DIR__.'/../../fixtures/1x1.png'),
200,
['Content-Type' => 'image/png'],
),
'cdn.example.com/missing.png' => Http::response(null, 404),
'example.com/missing.png' => Http::response(null, 404),
]);
$response = TryPostServer::actingAs($this->user)
->tool(AttachMediaFromUrlTool::class, [
'post_id' => $this->post->id,
'urls' => [
'https://cdn.example.com/ok.png',
'https://cdn.example.com/missing.png',
'https://example.com/ok.png',
'https://example.com/missing.png',
],
]);
$response->assertOk()
->assertSee(['cdn.example.com/missing.png']);
->assertSee(['example.com/missing.png']);
expect(Media::where('mediable_id', $this->workspace->id)->count())->toBe(1);
expect($this->post->fresh()->media)->toHaveCount(1);
@ -97,7 +97,7 @@
$response = TryPostServer::actingAs($this->user)
->tool(AttachMediaFromUrlTool::class, [
'post_id' => $post->id,
'urls' => ['https://cdn.example.com/photo.jpg'],
'urls' => ['https://example.com/photo.jpg'],
]);
$response->assertHasErrors(['Post not found.']);
@ -107,7 +107,7 @@
$response = TryPostServer::actingAs($this->user)
->tool(AttachMediaFromUrlTool::class, [
'post_id' => $this->post->id,
'urls' => ['ftp://cdn.example.com/photo.jpg'],
'urls' => ['ftp://example.com/photo.jpg'],
]);
$response->assertHasErrors();
@ -126,7 +126,7 @@
});
test('rejects more than 10 urls per call', function () {
$urls = collect(range(1, 11))->map(fn ($i) => "https://cdn.example.com/photo-{$i}.jpg")->all();
$urls = collect(range(1, 11))->map(fn ($i) => "https://example.com/photo-{$i}.jpg")->all();
$response = TryPostServer::actingAs($this->user)
->tool(AttachMediaFromUrlTool::class, [

5
tests/TestCase.php
View file

@ -4,7 +4,6 @@
namespace Tests;
use App\Services\Post\MediaAttacher;
use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
abstract class TestCase extends BaseTestCase
@ -23,9 +22,5 @@ protected function setUp(): void
parent::setUp();
$this->withoutVite();
// Bypass the SSRF check during tests so Http::fake() with
// synthetic hosts like cdn.example.com isn't rejected.
MediaAttacher::fakeUrlSafety();
}
}