From 543aa067bf1f1e46465a6c557eb214442e9c1e57 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Mon, 31 Aug 2026 10:28:27 +0200 Subject: [PATCH 1/4] fix(TaskProcessing): Harden task scheduling with webhooks fix(TaskProcessing): Harden task scheduling with webhooks Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr [skip ci] --- lib/private/TaskProcessing/Manager.php | 77 ++++++++++++- lib/public/TaskProcessing/IManager.php | 4 +- .../lib/TaskProcessing/TaskProcessingTest.php | 106 ++++++++++++++++++ 3 files changed, 184 insertions(+), 3 deletions(-) diff --git a/lib/private/TaskProcessing/Manager.php b/lib/private/TaskProcessing/Manager.php index 83270db41a73a..0962b44b87563 100644 --- a/lib/private/TaskProcessing/Manager.php +++ b/lib/private/TaskProcessing/Manager.php @@ -1389,7 +1389,7 @@ public function setTaskStatus(Task $task, int $status): void { } /** - * Validate input, fill input default values, set completionExpectedAt, set scheduledAt + * Validate input and webhook, fill input default values, set completionExpectedAt, set scheduledAt * * @param Task $task * @return void @@ -1426,6 +1426,8 @@ private function prepareTask(Task $task): void { $this->validateFileId($fileId); $this->validateUserAccessToFile($fileId, $task->getUserId()); } + // validate the webhook configuration + $this->validateWebhook($task); // remove superfluous keys and set input $input = $this->removeSuperfluousArrayKeys($task->getInput(), $inputShape, $optionalInputShape); $inputWithDefaults = $this->fillInputDefaults($input, $inputShapeDefaults, $optionalInputShapeDefaults); @@ -1438,6 +1440,79 @@ private function prepareTask(Task $task): void { $task->setCompletionExpectedAt($completionExpectedAt); } + /** + * Validate the webhook URI and webhook method of a task + * + * Both values are optional, but if one is set, the other one has to be set as well. + * Supported methods are `HTTP:`, which require an absolute + * http(s) URI pointing at a non-local host, and `AppAPI::`, + * which requires an absolute path as URI. + * + * @param Task $task + * @return void + * @throws ValidationException + */ + private function validateWebhook(Task $task): void { + $uri = $task->getWebhookUri(); + $method = $task->getWebhookMethod(); + + if (($uri === null || $uri === '') && ($method === null || $method === '')) { + return; + } + if ($uri === null || $uri === '') { + throw new ValidationException('Webhook URI is required when a webhook method is set'); + } + if ($method === null || $method === '') { + throw new ValidationException('Webhook method is required when a webhook URI is set'); + } + if (mb_strlen($uri) > 4000) { + throw new ValidationException('Webhook URI is too long, maximum length is 4000 characters'); + } + if (mb_strlen($method) > 64) { + throw new ValidationException('Webhook method is too long, maximum length is 64 characters'); + } + + if (str_starts_with($method, 'HTTP:')) { + if (!in_array($method, ['HTTP:GET', 'HTTP:POST', 'HTTP:PUT', 'HTTP:DELETE'], true)) { + throw new ValidationException('Invalid webhook method: ' . $method); + } + if (filter_var($uri, FILTER_VALIDATE_URL) === false) { + throw new ValidationException('Invalid webhook URI: ' . $uri); + } + $parsedUri = parse_url($uri); + if ($parsedUri === false || !isset($parsedUri['scheme']) || !isset($parsedUri['host']) || $parsedUri['host'] === '') { + throw new ValidationException('Invalid webhook URI: ' . $uri); + } + if (!in_array(strtolower($parsedUri['scheme']), ['http', 'https'], true)) { + throw new ValidationException('Invalid webhook URI scheme, only http and https are supported: ' . $uri); + } + if (!$this->remoteHostValidator->isValid($parsedUri['host'])) { + throw new ValidationException('Invalid webhook URI, the host is not allowed to be connected to: ' . $uri); + } + return; + } + + if (str_starts_with($method, 'AppAPI:')) { + $parsedMethod = explode(':', $method); + if (count($parsedMethod) !== 3) { + throw new ValidationException('Invalid webhook method: ' . $method); + } + [, $exAppId, $httpMethod] = $parsedMethod; + if (preg_match('/^[a-z][a-z0-9_-]*$/', $exAppId) !== 1) { + throw new ValidationException('Invalid ExApp ID in webhook method: ' . $method); + } + if (!in_array($httpMethod, ['GET', 'POST', 'PUT', 'DELETE'], true)) { + throw new ValidationException('Invalid webhook method: ' . $method); + } + if (!str_starts_with($uri, '/')) { + throw new ValidationException('Invalid webhook URI, an absolute path is required for AppAPI webhooks: ' . $uri); + } + return; + } + + throw new ValidationException('Invalid webhook method: ' . $method); + } + /** * Store the task in the DB and set its ID in the \OCP\TaskProcessing\Task input param * diff --git a/lib/public/TaskProcessing/IManager.php b/lib/public/TaskProcessing/IManager.php index 28beeafc88474..73bd70b820ed7 100644 --- a/lib/public/TaskProcessing/IManager.php +++ b/lib/public/TaskProcessing/IManager.php @@ -67,7 +67,7 @@ public function getAvailableTaskTypeIds(bool $showDisabled = false, ?string $use /** * @param Task $task The task to run * @throws PreConditionNotMetException If no or not the requested provider was registered but this method was still called - * @throws ValidationException the given task input didn't pass validation against the task type's input shape and/or the providers optional input shape specs + * @throws ValidationException the given task input didn't pass validation against the task type's input shape and/or the providers optional input shape specs, or the specified webhook didn't pass validation * @throws Exception storing the task in the database failed * @throws UnauthorizedException the user scheduling the task does not have access to the files used in the input * @since 30.0.0 @@ -80,7 +80,7 @@ public function scheduleTask(Task $task): void; * @param Task $task The task to run * @return Task The result task * @throws PreConditionNotMetException If no or not the requested provider was registered but this method was still called - * @throws ValidationException the given task input didn't pass validation against the task type's input shape and/or the providers optional input shape specs + * @throws ValidationException the given task input didn't pass validation against the task type's input shape and/or the providers optional input shape specs, or the specified webhook didn't pass validation * @throws Exception storing the task in the database failed * @throws UnauthorizedException the user scheduling the task does not have access to the files used in the input * @since 30.0.0 diff --git a/tests/lib/TaskProcessing/TaskProcessingTest.php b/tests/lib/TaskProcessing/TaskProcessingTest.php index 81c34d4e9c3d4..1366847af5d03 100644 --- a/tests/lib/TaskProcessing/TaskProcessingTest.php +++ b/tests/lib/TaskProcessing/TaskProcessingTest.php @@ -33,6 +33,7 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\L10N\IFactory; +use OCP\Security\IRemoteHostValidator; use OCP\Server; use OCP\TaskProcessing\EShapeType; use OCP\TaskProcessing\Events\GetTaskProcessingProvidersEvent; @@ -602,6 +603,11 @@ protected function setUp(): void { ); $this->userMountCache = $this->createMock(IUserMountCache::class); + $this->invalidRemoteHosts = []; + $this->rejectAllRemoteHosts = false; + $this->remoteHostValidator = $this->createMock(IRemoteHostValidator::class); + $this->remoteHostValidator->expects($this->any())->method('isValid') + ->willReturnCallback(fn (string $host): bool => !$this->rejectAllRemoteHosts && !in_array($host, $this->invalidRemoteHosts, true)); $this->config = Server::get(IConfig::class); $this->appConfig = Server::get(IAppConfig::class); $this->manager = new Manager( @@ -672,6 +678,106 @@ public function testProviderShouldBeRegisteredAndTaskFailValidation(): void { $this->manager->scheduleTask($task); } + public static function invalidWebhookDataProvider(): array { + return [ + 'uri without method' => ['https://example.com/hook', null], + 'method without uri' => [null, 'HTTP:POST'], + 'empty uri with method' => ['', 'HTTP:POST'], + 'uri with empty method' => ['https://example.com/hook', ''], + 'unknown method prefix' => ['https://example.com/hook', 'FTP:GET'], + 'unknown http verb' => ['https://example.com/hook', 'HTTP:PATCH'], + 'lowercase http verb' => ['https://example.com/hook', 'HTTP:post'], + 'unsupported uri scheme' => ['file:///etc/passwd', 'HTTP:GET'], + 'relative uri for http method' => ['/some/path', 'HTTP:POST'], + 'malformed uri' => ['https://', 'HTTP:POST'], + 'appapi method without exapp id' => ['/some/path', 'AppAPI:POST'], + 'appapi method with too many parts' => ['/some/path', 'AppAPI:my_app:POST:extra'], + 'appapi method with invalid exapp id' => ['/some/path', 'AppAPI:My App:POST'], + 'appapi method with unknown http verb' => ['/some/path', 'AppAPI:my_app:PATCH'], + 'absolute uri for appapi method' => ['https://example.com/hook', 'AppAPI:my_app:POST'], + 'uri too long' => ['https://example.com/' . str_repeat('a', 4000), 'HTTP:POST'], + 'method too long' => ['/some/path', 'AppAPI:' . str_repeat('a', 64) . ':POST'], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('invalidWebhookDataProvider')] + public function testProviderShouldBeRegisteredAndWebhookFailValidation(?string $webhookUri, ?string $webhookMethod): void { + $this->registrationContext->expects($this->any())->method('getTaskProcessingProviders')->willReturn([ + new ServiceRegistration('test', SuccessfulSyncProvider::class) + ]); + $task = new Task(TextToText::ID, ['input' => 'Hello'], 'test', null); + $task->setWebhookUri($webhookUri); + $task->setWebhookMethod($webhookMethod); + self::expectException(ValidationException::class); + $this->manager->scheduleTask($task); + } + + public static function validWebhookDataProvider(): array { + return [ + 'no webhook' => [null, null], + 'empty webhook' => ['', ''], + 'http get' => ['http://example.com/hook', 'HTTP:GET'], + 'https post' => ['https://example.com/hook?foo=bar', 'HTTP:POST'], + 'https put' => ['https://example.com/hook', 'HTTP:PUT'], + 'https delete' => ['https://example.com/hook', 'HTTP:DELETE'], + 'appapi post' => ['/some/path', 'AppAPI:my_app:POST'], + 'appapi get' => ['/', 'AppAPI:my-app2:GET'], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('validWebhookDataProvider')] + public function testProviderShouldBeRegisteredAndWebhookPassValidation(?string $webhookUri, ?string $webhookMethod): void { + $this->registrationContext->expects($this->any())->method('getTaskProcessingProviders')->willReturn([ + new ServiceRegistration('test', SuccessfulSyncProvider::class) + ]); + $task = new Task(TextToText::ID, ['input' => 'Hello'], 'test', null); + $task->setWebhookUri($webhookUri); + $task->setWebhookMethod($webhookMethod); + $this->manager->scheduleTask($task); + self::assertNotNull($task->getId()); + self::assertEquals(Task::STATUS_SCHEDULED, $task->getStatus()); + // clean up so the scheduled task does not interfere with other tests + $this->manager->deleteTask($task); + } + + public static function localWebhookHostDataProvider(): array { + return [ + 'localhost' => ['http://localhost/hook', 'localhost'], + 'ipv4 loopback' => ['http://127.0.0.1:8080/hook', '127.0.0.1'], + 'ipv6 loopback' => ['http://[::1]/hook', '[::1]'], + 'private network' => ['https://192.168.1.1/hook', '192.168.1.1'], + 'local hostname' => ['https://server.local/hook', 'server.local'], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('localWebhookHostDataProvider')] + public function testProviderShouldBeRegisteredAndLocalWebhookHostFailValidation(string $webhookUri, string $host): void { + $this->registrationContext->expects($this->any())->method('getTaskProcessingProviders')->willReturn([ + new ServiceRegistration('test', SuccessfulSyncProvider::class) + ]); + $this->invalidRemoteHosts = [$host]; + $task = new Task(TextToText::ID, ['input' => 'Hello'], 'test', null); + $task->setWebhookUri($webhookUri); + $task->setWebhookMethod('HTTP:POST'); + self::expectException(ValidationException::class); + $this->manager->scheduleTask($task); + } + + public function testProviderShouldBeRegisteredAndAppApiWebhookSkipsHostValidation(): void { + $this->registrationContext->expects($this->any())->method('getTaskProcessingProviders')->willReturn([ + new ServiceRegistration('test', SuccessfulSyncProvider::class) + ]); + // AppAPI webhooks use an absolute path, so no remote host is involved + $this->rejectAllRemoteHosts = true; + $task = new Task(TextToText::ID, ['input' => 'Hello'], 'test', null); + $task->setWebhookUri('/some/path'); + $task->setWebhookMethod('AppAPI:my_app:POST'); + $this->manager->scheduleTask($task); + self::assertEquals(Task::STATUS_SCHEDULED, $task->getStatus()); + // clean up so the scheduled task does not interfere with other tests + $this->manager->deleteTask($task); + } + public function testProviderShouldBeRegisteredAndTaskWithFilesFailValidation(): void { $this->registrationContext->expects($this->any())->method('getTaskProcessingTaskTypes')->willReturn([ new ServiceRegistration('test', AudioToImage::class) From 7cd59a4340afd76d8e484bf2d64c62d070eda187 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 1 Sep 2026 09:12:09 +0200 Subject: [PATCH 2/4] fix(TaskProcessing): Add IRemoteHostValidator dependency to Manager Signed-off-by: Marcel Klehr --- lib/private/TaskProcessing/Manager.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/private/TaskProcessing/Manager.php b/lib/private/TaskProcessing/Manager.php index 0962b44b87563..0087ac35d79f2 100644 --- a/lib/private/TaskProcessing/Manager.php +++ b/lib/private/TaskProcessing/Manager.php @@ -41,6 +41,7 @@ use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Lock\LockedException; +use OCP\Security\IRemoteHostValidator; use OCP\SpeechToText\ISpeechToTextProvider; use OCP\SpeechToText\ISpeechToTextProviderWithId; use OCP\TaskProcessing\EShapeType; @@ -123,6 +124,7 @@ public function __construct( private IUserSession $userSession, ICacheFactory $cacheFactory, private IFactory $l10nFactory, + private IRemoteHostValidator $remoteHostValidator, ) { $this->appData = $appDataFactory->get('core'); $this->distributedCache = $cacheFactory->createDistributed('task_processing::'); From a55284ca9d2bfca74222ae282d9be22dc67acdc9 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 1 Sep 2026 09:37:49 +0200 Subject: [PATCH 3/4] fix: Add IRemoteHostValidator to TaskProcessingTest dependencies Signed-off-by: Marcel Klehr --- tests/lib/TaskProcessing/TaskProcessingTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/lib/TaskProcessing/TaskProcessingTest.php b/tests/lib/TaskProcessing/TaskProcessingTest.php index 1366847af5d03..95a5b13f5ca72 100644 --- a/tests/lib/TaskProcessing/TaskProcessingTest.php +++ b/tests/lib/TaskProcessing/TaskProcessingTest.php @@ -628,6 +628,7 @@ protected function setUp(): void { Server::get(IUserSession::class), Server::get(ICacheFactory::class), Server::get(IFactory::class), + Server::get(IRemoteHostValidator::class), ); } From 484f8e9dce6b48b33fc17482b47e98a5976844f1 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 1 Sep 2026 13:15:16 +0200 Subject: [PATCH 4/4] fix: Add IRemoteHostValidator to TaskProcessingTest dependencies Signed-off-by: Marcel Klehr --- tests/lib/TaskProcessing/TaskProcessingTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/lib/TaskProcessing/TaskProcessingTest.php b/tests/lib/TaskProcessing/TaskProcessingTest.php index 95a5b13f5ca72..25342a67197f1 100644 --- a/tests/lib/TaskProcessing/TaskProcessingTest.php +++ b/tests/lib/TaskProcessing/TaskProcessingTest.php @@ -1403,6 +1403,7 @@ private function createManagerInstance(): Manager { Server::get(IUserSession::class), Server::get(ICacheFactory::class), Server::get(IFactory::class), + Server::get(IRemoteHostValidator::class), ); }