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.
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.
The previous MediaAttacher had ~5 responsibilities crammed in one class
(URL validation, HTTP fetch, streaming, MIME check, Storage write,
post.media merge) plus an `if ($app->runningUnitTests()) return true`
inside the SSRF guard — production code that knew about test mode.
Split into three:
- UrlSafetyGuard (interface) + DnsUrlSafetyGuard (default impl). The
full SSRF check is now isolated and unit-testable in
tests/Unit/Services/Post/DnsUrlSafetyGuardTest. Tests bind a permissive
guard in tests/TestCase::setUp so feature tests using Http::fake() with
synthetic hosts (cdn.example.com) still work — the runningUnitTests()
check inside production code is gone.
- MediaDownloader: takes a URL, returns a temp file + MIME or null. Uses
Http::sink with a Guzzle progress callback that throws once maxBytes
is exceeded, so we abort mid-stream without buffering the body in PHP
memory (the previous chunk loop was append-to-string which defeated
the whole point of streaming).
- MediaAttacher: pure orchestrator. Calls MediaDownloader, validates the
MIME against the post's enabled platforms, persists via Storage::putFileAs,
appends the Media record under a row lock. Drops from 275 to ~190 lines
with zero responsibility overlap.
The IO contract (`null` on failure, `['path','mime','bytes']` on success)
keeps MediaAttacher free of the temp-file lifecycle on the happy path —
ownership is documented in MediaDownloader's PHPDoc and the orchestrator
unlinks via try/finally in processOne().
Bind the interface in AppServiceProvider so production resolves
DnsUrlSafetyGuard automatically; tests override it.
7 new unit tests cover the SSRF cases (loopback, RFC1918, link-local,
zero/broadcast, IPv6 loopback/ULA/link-local, plus a public-IP positive
test). The 14 existing AttachMedia feature tests stay green.
