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

fix(security): guard automation node requests against redirect-SSRF

Browse source 
RunFetchRssNode, RunWebhookNode and RunHttpRequestNode guarded the initial URL but then followed redirects unguarded, so a public URL could 302 to an internal address. RSS now fetches through SafeHttpFetcher::get() (re-guards every hop); webhooks no longer follow redirects; the generic HTTP request node re-runs the SSRF guard on each hop via a new SafeHttpFetcher::redirectGuardOptions().
This commit is contained in:
Paulo Castellano 2026-07-17 14:55:56 -03:00
parent 9bb1f266e9
commit 11d6bddf9c
7 changed files with 131 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
app/Actions/Automation/Node/RunFetchRssNode.php
View file

@ -13,7 +13,6 @@
use App\Services\Automation\FeedParser;
use App\Services\Brand\SafeHttpFetcher;
use Carbon\CarbonImmutable;
use Illuminate\Support\Facades\Http;
use RuntimeException;
use Throwable;
@ -51,21 +50,19 @@ public function __invoke(AutomationRun $run, array $config): NodeRunResult
return NodeRunResult::failed(__('automations.errors.fetch_rss_missing_url'));
}
// SafeHttpFetcher::get() re-validates every redirect hop against the SSRF
// guard (not just the initial URL), so a public feed that 302s to an
// internal host is never followed. It throws on a blocked hop, connection
// failure, non-2xx status, or an excessive redirect chain — all of which
// are legitimate "this feed couldn't be fetched" failures for this node.
try {
$this->safeHttp->guardAgainstSsrf($feedUrl);
} catch (RuntimeException) {
return NodeRunResult::failed(__('automations.errors.url_not_allowed'), [
'reason' => 'url_not_allowed',
'url' => $feedUrl,
$response = $this->safeHttp->get($feedUrl);
} catch (RuntimeException $e) {
return NodeRunResult::failed(__('automations.errors.fetch_rss_request_failed'), [
'message' => $e->getMessage(),
]);
}
$response = Http::timeout(10)->get($feedUrl);
if (! $response->successful()) {
return NodeRunResult::failed(__('automations.errors.fetch_rss_request_failed'), ['status' => $response->status()]);
}
$items = $this->parser->parse($response->body());
if ($items === null) {

4
app/Actions/Automation/Node/RunHttpRequestNode.php
View file

@ -364,7 +364,9 @@ private function buildRequest(array $config, array $context): PendingRequest
$request = $request->withHeaders($headers);
}
return $request->withUserAgent(config('trypost.user_agent'));
return $request
->withUserAgent(config('trypost.user_agent'))
->withOptions($this->safeHttp->redirectGuardOptions());
}
/**

1
app/Actions/Automation/Node/RunWebhookNode.php
View file

@ -75,6 +75,7 @@ public function __invoke(AutomationRun $run, array $config): NodeRunResult
try {
$response = Http::withHeaders($headers)
->withUserAgent(config('trypost.user_agent'))
->withOptions(['allow_redirects' => false])
->send($method, $url, ['json' => $payload]);
} catch (Throwable $e) {
return NodeRunResult::failed(__('automations.errors.webhook_request_failed'), [

21
app/Services/Brand/SafeHttpFetcher.php
View file

@ -91,6 +91,27 @@ public function tryGet(string $url): ?Response
}
}
/**
* Guzzle allow_redirects options that re-run the SSRF guard on every hop.
* For callers that follow redirects on user-supplied URLs with methods/bodies
* that SafeHttpFetcher::get() cannot express.
*
* @return array<string, mixed>
*/
public function redirectGuardOptions(int $max = self::MAX_REDIRECTS): array
{
return [
'allow_redirects' => [
'max' => $max,
'strict' => true,
'protocols' => ['http', 'https'],
'on_redirect' => function ($request, $response, $uri): void {
$this->guardAgainstSsrf((string) $uri);
},
],
];
}
public function guardAgainstSsrf(string $url): void
{
$parts = parse_url($url);

36
tests/Feature/Automation/Node/FetchRssNodeTest.php
View file

@ -280,7 +280,7 @@
$result = app(RunFetchRssNode::class)($run, ['feed_url' => 'https://1.1.1.1/feed.xml']);
expect($result->status)->toBe(NodeRunStatus::Failed);
expect($result->error['status'])->toBe(500);
expect($result->error['message'])->toContain('500');
});
it('fails on a malformed RSS feed', function () {
@ -313,6 +313,40 @@
expect($result->output['fetched']['key'])->toBe('d');
});
it('never follows a feed redirect that targets a private or internal host', function () {
Http::fake([
'https://93.184.216.34/feed' => Http::response('', 302, ['Location' => 'http://127.0.0.1/internal']),
'http://127.0.0.1/*' => Http::response(feedFixture('rss_old'), 200),
]);
$automation = Automation::factory()->active()->create();
$run = AutomationRun::factory()->for($automation)->create(['current_node_id' => 'fetch_1']);
$result = app(RunFetchRssNode::class)($run, ['feed_url' => 'https://93.184.216.34/feed']);
expect($result->status)->toBe(NodeRunStatus::Failed);
Http::assertNotSent(fn ($request) => str_contains($request->url(), '127.0.0.1'));
});
it('still follows a legitimate public-to-public feed redirect', function () {
Carbon::setTestNow('2026-01-15 10:00:00');
Http::fake([
'https://93.184.216.34/feed' => Http::response('', 301, ['Location' => 'https://1.1.1.1/feed']),
'https://1.1.1.1/feed' => Http::response(feedFixture('rss_old'), 200),
]);
$automation = Automation::factory()->active()->create();
$run = AutomationRun::factory()->for($automation)->create(['current_node_id' => 'fetch_1']);
$result = app(RunFetchRssNode::class)($run, ['feed_url' => 'https://93.184.216.34/feed']);
expect($result->status)->toBe(NodeRunStatus::Completed);
Http::assertSentInOrder([
fn ($request) => str_contains($request->url(), '93.184.216.34'),
fn ($request) => str_contains($request->url(), '1.1.1.1'),
]);
});
it('falls back to the link as the dedup key when an item has no guid', function () {
Carbon::setTestNow('2026-01-15 10:00:00');
Http::fake(['1.1.1.1/*' => Http::response(feedFixture('rss_no_guid'), 200)]);

41
tests/Feature/Automation/Node/HttpRequestNodeTest.php
View file

@ -53,6 +53,47 @@
Http::assertNothingSent();
});
it('never follows a redirect that targets a private or internal host', function () {
Http::fake([
'https://93.184.216.34/*' => Http::response('', 302, ['Location' => 'http://127.0.0.1/internal']),
'http://127.0.0.1/*' => Http::response('internal secret', 200),
]);
$automation = Automation::factory()->active()->create();
$run = AutomationRun::factory()->for($automation)->create(['current_node_id' => 'http_1']);
$result = app(RunHttpRequestNode::class)($run, [
'url' => 'https://93.184.216.34/start',
'method' => 'GET',
'auth_type' => 'none',
]);
expect($result->status)->toBe(NodeRunStatus::Failed);
Http::assertNotSent(fn ($request) => str_contains($request->url(), '127.0.0.1'));
});
it('still follows a legitimate public-to-public redirect', function () {
Http::fake([
'https://93.184.216.34/*' => Http::response('', 301, ['Location' => 'https://1.1.1.1/final']),
'https://1.1.1.1/final' => Http::response(['ok' => true], 200),
]);
$automation = Automation::factory()->active()->create();
$run = AutomationRun::factory()->for($automation)->create(['current_node_id' => 'http_1']);
$result = app(RunHttpRequestNode::class)($run, [
'url' => 'https://93.184.216.34/start',
'method' => 'GET',
'auth_type' => 'none',
]);
expect($result->status)->toBe(NodeRunStatus::Completed);
Http::assertSentInOrder([
fn ($request) => str_contains($request->url(), '93.184.216.34'),
fn ($request) => str_contains($request->url(), '1.1.1.1'),
]);
});
it('processes first new item and spawns siblings when items_path is set', function () {
Carbon::setTestNow('2026-01-15 10:00:00');
Http::fake([

21
tests/Feature/Automation/Node/WebhookNodeTest.php
View file

@ -200,6 +200,27 @@
Http::assertSent(fn ($request) => $request->hasHeader('X-Token', 'tok-123'));
});
it('never follows a redirect to a private or internal host', function () {
Http::fake([
'https://93.184.216.34/*' => Http::response('', 302, ['Location' => 'http://127.0.0.1/internal']),
'http://127.0.0.1/*' => Http::response('internal secret', 200),
]);
$run = AutomationRun::factory()->create();
$result = app(RunWebhookNode::class)($run, [
'url' => 'https://93.184.216.34/hook',
'method' => 'POST',
'payload_template' => '{}',
]);
// The 3xx is returned as-is (not followed), so the node completes with the
// redirect status rather than the internal host's response.
expect($result->status)->toBe(Status::Completed);
expect($result->output['webhook']['status'])->toBe(302);
Http::assertNotSent(fn ($request) => str_contains($request->url(), '127.0.0.1'));
});
it('fails cleanly when the request throws a connection exception', function () {
Http::fake(fn () => throw new ConnectionException('connection timed out'));