fix(automations): code-review hardening for feed fetch & URL validation
- inspectFeed fetches via SafeHttpFetcher::get (SSRF guard + timeout + redirect
cap + UA) instead of a raw, timeout-less Http::get; add a timeout to the node fetch.
- ResolvableUrl now requires an http(s) scheme, so it's no weaker than the plain
url rule it replaced (rejects file://, javascript://).
- Apply ResolvableUrl to the HTTP Request and Webhook node URLs too, so templated
{{ }} URLs validate consistently across every URL field.
- Clear a fetch_rss node's discovered_fields when its feed_url changes, so stale
fields from the previous feed stop showing up as autocomplete suggestions.
This commit is contained in:
parent
f55d35363f
commit
45da024fc6
6 changed files with 75 additions and 12 deletions
|
|
@ -60,7 +60,7 @@ public function __invoke(AutomationRun $run, array $config): NodeRunResult
|
|||
]);
|
||||
}
|
||||
|
||||
$response = Http::get($feedUrl);
|
||||
$response = Http::timeout(10)->get($feedUrl);
|
||||
|
||||
if (! $response->successful()) {
|
||||
return NodeRunResult::failed(__('automations.errors.fetch_rss_request_failed'), ['status' => $response->status()]);
|
||||
|
|
|
|||
|
|
@ -39,7 +39,6 @@
|
|||
use Illuminate\Http\JsonResponse;
|
||||
use Illuminate\Http\RedirectResponse;
|
||||
use Illuminate\Http\Response as HttpResponse;
|
||||
use Illuminate\Support\Facades\Http;
|
||||
use Inertia\Inertia;
|
||||
use Inertia\Response;
|
||||
use RuntimeException;
|
||||
|
|
@ -225,15 +224,12 @@ public function inspectFeed(
|
|||
['variables' => $automation->resolvedVariables()],
|
||||
);
|
||||
|
||||
// SafeHttpFetcher::get() bundles the SSRF guard, a request timeout, a
|
||||
// redirect cap and a branded user-agent — so a slow or hostile feed can't
|
||||
// hang this synchronous request. It throws on SSRF, timeout or non-2xx.
|
||||
try {
|
||||
$safeHttp->guardAgainstSsrf($feedUrl);
|
||||
$response = $safeHttp->get($feedUrl);
|
||||
} catch (RuntimeException) {
|
||||
return response()->json(['message' => __('automations.errors.url_not_allowed')], SymfonyResponse::HTTP_UNPROCESSABLE_ENTITY);
|
||||
}
|
||||
|
||||
$response = Http::get($feedUrl);
|
||||
|
||||
if (! $response->successful()) {
|
||||
return response()->json(['message' => __('automations.errors.fetch_rss_request_failed')], SymfonyResponse::HTTP_UNPROCESSABLE_ENTITY);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -136,7 +136,7 @@ private function dataRulesForNodeType(?string $type, int $i): array
|
|||
'discovered_fields.*.sample' => ['nullable', 'string'],
|
||||
],
|
||||
NodeType::HttpRequest->value => [
|
||||
'url' => ['required', 'url'],
|
||||
'url' => ['required', new ResolvableUrl],
|
||||
'method' => ['required', Rule::in(array_column(HttpMethod::cases(), 'value'))],
|
||||
'auth_type' => ['required', Rule::in(array_column(AuthType::cases(), 'value'))],
|
||||
'auth_token' => ['nullable', 'string'],
|
||||
|
|
@ -171,7 +171,7 @@ private function dataRulesForNodeType(?string $type, int $i): array
|
|||
'scheduled_offset' => ['required_if:nodes.'.$i.'.data.mode,'.PublishMode::Scheduled->value, 'integer', 'min:0'],
|
||||
],
|
||||
NodeType::Webhook->value => [
|
||||
'url' => ['required', 'url'],
|
||||
'url' => ['required', new ResolvableUrl],
|
||||
'method' => ['required', Rule::in(array_column(HttpMethod::cases(), 'value'))],
|
||||
'payload_template' => ['nullable', 'string'],
|
||||
'headers' => ['nullable', 'array'],
|
||||
|
|
|
|||
|
|
@ -28,8 +28,12 @@ public function validate(string $attribute, mixed $value, Closure $fail): void
|
|||
}
|
||||
|
||||
$candidate = preg_replace('/\{\{\s*[\w.]+\s*\}\}/', 'placeholder', $value);
|
||||
$scheme = strtolower((string) parse_url($candidate, PHP_URL_SCHEME));
|
||||
|
||||
if (filter_var($candidate, FILTER_VALIDATE_URL) === false) {
|
||||
// Require a real http(s) URL once expressions are substituted, so the rule
|
||||
// is no weaker than the plain `url` rule it replaces (rejects file://,
|
||||
// javascript://, etc.) while still allowing templated hosts/queries.
|
||||
if (filter_var($candidate, FILTER_VALIDATE_URL) === false || ! in_array($scheme, ['http', 'https'], true)) {
|
||||
$fail('validation.url')->translate();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -35,6 +35,15 @@ const local = ref<FetchRssConfig>({
|
|||
|
||||
watch(local, (val) => emit('update', val), { deep: true });
|
||||
|
||||
// Changing the feed invalidates the discovered fields — they belong to the old
|
||||
// feed and would otherwise keep being offered as stale autocomplete suggestions.
|
||||
watch(
|
||||
() => local.value.feed_url,
|
||||
() => {
|
||||
local.value.discovered_fields = [];
|
||||
},
|
||||
);
|
||||
|
||||
const inspectHttp = useHttp<{ feed_url: string }, { fields: DiscoveredField[] }>({ feed_url: '' });
|
||||
const isInspecting = ref(false);
|
||||
const inspectError = ref<string | null>(null);
|
||||
|
|
|
|||
|
|
@ -125,6 +125,60 @@
|
|||
->assertSessionHasErrors('nodes.0.data.feed_url');
|
||||
});
|
||||
|
||||
it('rejects a non-http(s) scheme in feed_url', function () {
|
||||
$automation = Automation::factory()->for($this->workspace)->create();
|
||||
|
||||
$this->actingAs($this->user)
|
||||
->put(route('app.automations.update', $automation->id), [
|
||||
'nodes' => [[
|
||||
'id' => 'fetch_1',
|
||||
'type' => 'fetch_rss',
|
||||
'position' => ['x' => 0, 'y' => 0],
|
||||
'data' => ['feed_url' => 'file:///etc/passwd'],
|
||||
]],
|
||||
])
|
||||
->assertSessionHasErrors('nodes.0.data.feed_url');
|
||||
});
|
||||
|
||||
it('accepts a templated url on an http_request node', function () {
|
||||
$automation = Automation::factory()->for($this->workspace)->create();
|
||||
|
||||
$this->actingAs($this->user)
|
||||
->put(route('app.automations.update', $automation->id), [
|
||||
'nodes' => [[
|
||||
'id' => 'http_1',
|
||||
'type' => 'http_request',
|
||||
'position' => ['x' => 0, 'y' => 0],
|
||||
'data' => [
|
||||
'url' => 'https://api.example.com/{{ variables.PATH }}/feed',
|
||||
'method' => 'GET',
|
||||
'auth_type' => 'none',
|
||||
],
|
||||
]],
|
||||
])
|
||||
->assertRedirect()
|
||||
->assertSessionHasNoErrors();
|
||||
});
|
||||
|
||||
it('accepts a templated url on a webhook node', function () {
|
||||
$automation = Automation::factory()->for($this->workspace)->create();
|
||||
|
||||
$this->actingAs($this->user)
|
||||
->put(route('app.automations.update', $automation->id), [
|
||||
'nodes' => [[
|
||||
'id' => 'webhook_1',
|
||||
'type' => 'webhook',
|
||||
'position' => ['x' => 0, 'y' => 0],
|
||||
'data' => [
|
||||
'url' => 'https://hooks.example.com/{{ variables.TOKEN }}',
|
||||
'method' => 'POST',
|
||||
],
|
||||
]],
|
||||
])
|
||||
->assertRedirect()
|
||||
->assertSessionHasNoErrors();
|
||||
});
|
||||
|
||||
it('forbids inspecting a feed for an automation in another workspace', function () {
|
||||
Http::fake(['1.1.1.1/*' => Http::response('<rss version="2.0"><channel></channel></rss>', 200)]);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue