From e34e8860cb9703c511e904c6c49845cb73005446 Mon Sep 17 00:00:00 2001 From: Crewlet Date: Sat, 11 Jul 2026 22:34:21 +0000 Subject: [PATCH] feat(api): manage workspace webhooks via the public token API Add GET/POST/PATCH/DELETE /api/v1/workspaces/{slug}/webhooks/ to the public (X-Api-Key) token API, mirroring the internal app-API webhook endpoint so webhooks can be provisioned programmatically instead of only through the UI. - New WebhookAPIEndpoint (workspace-admin only, via WorkspaceOwnerPermission) reusing the shared Webhook model. - New public WebhookSerializer: generates secret_key server-side (returned only on create, withheld on list/retrieve), accepts url + the entity toggles (issue, issue_comment, cycle, module, project), and enforces the schema/domain validators plus the SSRF / WEBHOOK_ALLOWED_IPS guard. - Extract the SSRF/disallowed-domain webhook-URL validation into a shared plane.utils.webhook.validate_webhook_url helper used by BOTH the app and public serializers, so the guard can never drift between the two surfaces. The app serializer keeps its _validate_webhook_url adapter (behaviour unchanged) and delegates to the shared helper. - Generic HMAC delivery (bgtasks/webhook_task.py) is unchanged: X-Plane-Signature stays HMAC-SHA256 over the JSON payload with secret_key. - Contract tests: create returns a usable server-generated secret, the secret is withheld on reads, the SSRF/non-http guards reject loopback/private/scheme-invalid targets, duplicate URLs 409, management is admin-only, and a real delivery signs the payload correctly with the API-issued secret. Signed-off-by: Seena Fallah --- apps/api/plane/api/serializers/__init__.py | 1 + apps/api/plane/api/serializers/webhook.py | 67 ++++ apps/api/plane/api/urls/__init__.py | 2 + apps/api/plane/api/urls/webhook.py | 20 ++ apps/api/plane/api/views/__init__.py | 2 + apps/api/plane/api/views/webhook.py | 217 ++++++++++++ apps/api/plane/app/serializers/webhook.py | 44 +-- .../plane/tests/contract/api/test_webhooks.py | 319 ++++++++++++++++++ apps/api/plane/utils/webhook.py | 69 ++++ 9 files changed, 704 insertions(+), 37 deletions(-) create mode 100644 apps/api/plane/api/serializers/webhook.py create mode 100644 apps/api/plane/api/urls/webhook.py create mode 100644 apps/api/plane/api/views/webhook.py create mode 100644 apps/api/plane/tests/contract/api/test_webhooks.py create mode 100644 apps/api/plane/utils/webhook.py diff --git a/apps/api/plane/api/serializers/__init__.py b/apps/api/plane/api/serializers/__init__.py index 2ab639d5466..caf050c7c76 100644 --- a/apps/api/plane/api/serializers/__init__.py +++ b/apps/api/plane/api/serializers/__init__.py @@ -64,3 +64,4 @@ from .invite import WorkspaceInviteSerializer from .member import ProjectMemberSerializer from .sticky import StickySerializer +from .webhook import WebhookSerializer diff --git a/apps/api/plane/api/serializers/webhook.py b/apps/api/plane/api/serializers/webhook.py new file mode 100644 index 00000000000..15c3c425c49 --- /dev/null +++ b/apps/api/plane/api/serializers/webhook.py @@ -0,0 +1,67 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +# Third party imports +from rest_framework import serializers + +# Module imports +from .base import BaseSerializer +from plane.db.models import Webhook +from plane.db.models.webhook import validate_domain, validate_schema +from plane.utils.webhook import validate_webhook_url + + +class WebhookSerializer(BaseSerializer): + """Public token-API serializer for workspace webhooks. + + Mirrors the internal app-API webhook serializer: enforces the schema/domain + validators plus the shared SSRF/disallowed-domain guard, and keeps + ``secret_key`` server-generated (read-only) so it is returned on create but + never accepted as input. + """ + + url = serializers.URLField(validators=[validate_schema, validate_domain]) + + def _validate_webhook_url(self, url): + """Validate a webhook URL against SSRF and disallowed-domain rules. + + Thin adapter binding the serializer's request context to the shared + ``validate_webhook_url`` guard, mirroring the internal app-API + serializer so the SSRF/URL checks cannot drift between the two. + """ + validate_webhook_url(url, self.context.get("request")) + + def create(self, validated_data): + url = validated_data.get("url", None) + self._validate_webhook_url(url) + return Webhook.objects.create(**validated_data) + + def update(self, instance, validated_data): + url = validated_data.get("url", None) + if url: + self._validate_webhook_url(url) + return super().update(instance, validated_data) + + class Meta: + model = Webhook + fields = [ + "id", + "url", + "is_active", + "secret_key", + "project", + "issue", + "module", + "cycle", + "issue_comment", + "created_at", + "updated_at", + ] + read_only_fields = [ + "id", + "workspace", + "secret_key", + "created_at", + "updated_at", + ] diff --git a/apps/api/plane/api/urls/__init__.py b/apps/api/plane/api/urls/__init__.py index 4a202431bc7..6556faa191c 100644 --- a/apps/api/plane/api/urls/__init__.py +++ b/apps/api/plane/api/urls/__init__.py @@ -14,6 +14,7 @@ from .work_item import urlpatterns as work_item_patterns from .invite import urlpatterns as invite_patterns from .sticky import urlpatterns as sticky_patterns +from .webhook import urlpatterns as webhook_patterns urlpatterns = [ *asset_patterns, @@ -28,4 +29,5 @@ *work_item_patterns, *invite_patterns, *sticky_patterns, + *webhook_patterns, ] diff --git a/apps/api/plane/api/urls/webhook.py b/apps/api/plane/api/urls/webhook.py new file mode 100644 index 00000000000..09cd77e7114 --- /dev/null +++ b/apps/api/plane/api/urls/webhook.py @@ -0,0 +1,20 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +from django.urls import path + +from plane.api.views import WebhookAPIEndpoint, WebhookDetailAPIEndpoint + +urlpatterns = [ + path( + "workspaces//webhooks/", + WebhookAPIEndpoint.as_view(http_method_names=["get", "post"]), + name="webhooks", + ), + path( + "workspaces//webhooks//", + WebhookDetailAPIEndpoint.as_view(http_method_names=["get", "patch", "delete"]), + name="webhook-detail", + ), +] diff --git a/apps/api/plane/api/views/__init__.py b/apps/api/plane/api/views/__init__.py index e8549afb437..6a5534899b9 100644 --- a/apps/api/plane/api/views/__init__.py +++ b/apps/api/plane/api/views/__init__.py @@ -63,3 +63,5 @@ from .invite import WorkspaceInvitationsViewset from .sticky import StickyViewSet + +from .webhook import WebhookAPIEndpoint, WebhookDetailAPIEndpoint diff --git a/apps/api/plane/api/views/webhook.py b/apps/api/plane/api/views/webhook.py new file mode 100644 index 00000000000..f6ee8ea6fcc --- /dev/null +++ b/apps/api/plane/api/views/webhook.py @@ -0,0 +1,217 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +# Django imports +from django.db import IntegrityError + +# Third party imports +from rest_framework import status +from rest_framework.response import Response +from drf_spectacular.utils import ( + extend_schema, + OpenApiResponse, + OpenApiRequest, + OpenApiParameter, + OpenApiTypes, +) + +# Module imports +from .base import BaseAPIView +from plane.api.serializers import WebhookSerializer +from plane.db.models import Webhook, Workspace +from plane.utils.permissions import WorkspaceOwnerPermission +from plane.utils.openapi import ( + WORKSPACE_SLUG_PARAMETER, + UNAUTHORIZED_RESPONSE, + FORBIDDEN_RESPONSE, + WORKSPACE_NOT_FOUND_RESPONSE, + CONFLICT_RESPONSE, +) + +# Fields returned to clients everywhere except the create response — the +# server-generated ``secret_key`` is intentionally withheld here and only ever +# surfaced once, in the create response, so it is not leaked on every read. +WEBHOOK_READ_FIELDS = ( + "id", + "url", + "is_active", + "created_at", + "updated_at", + "project", + "issue", + "cycle", + "module", + "issue_comment", +) + +WEBHOOK_PK_PARAMETER = OpenApiParameter( + name="pk", + description="Webhook ID", + required=True, + type=OpenApiTypes.UUID, + location=OpenApiParameter.PATH, +) + + +class WebhookAPIEndpoint(BaseAPIView): + """Manage workspace webhooks via the token API. + + Mirrors the internal app-API webhook endpoint: workspace-admin only, + enforces the shared URL schema/domain validators and the SSRF / + ``WEBHOOK_ALLOWED_IPS`` guard, and generates the signing ``secret_key`` + server-side (returned only in the create response). + """ + + permission_classes = [WorkspaceOwnerPermission] + serializer_class = WebhookSerializer + + @extend_schema( + operation_id="create_webhook", + summary="Create webhook", + description=( + "Register a webhook for the workspace. The signing `secret_key` is " + "generated server-side and returned only in this response. The target " + "`url` is validated against the SSRF/URL guards (localhost, private " + "networks and non-http(s) schemes are rejected)." + ), + tags=["Webhooks"], + parameters=[WORKSPACE_SLUG_PARAMETER], + request=OpenApiRequest(request=WebhookSerializer), + responses={ + 201: OpenApiResponse(description="Webhook created", response=WebhookSerializer), + 400: OpenApiResponse(description="Invalid or disallowed webhook URL"), + 401: UNAUTHORIZED_RESPONSE, + 403: FORBIDDEN_RESPONSE, + 404: WORKSPACE_NOT_FOUND_RESPONSE, + 409: CONFLICT_RESPONSE, + }, + ) + def post(self, request, slug): + workspace = Workspace.objects.get(slug=slug) + try: + serializer = WebhookSerializer(data=request.data, context={"request": request}) + if serializer.is_valid(): + serializer.save(workspace_id=workspace.id) + return Response(serializer.data, status=status.HTTP_201_CREATED) + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + except IntegrityError as e: + # Only the unique webhook-URL violation maps to 409; any other + # integrity error (FK / NOT NULL / ...) is re-raised so it is not + # masked as a duplicate-URL conflict. + if "already exists" in str(e): + return Response( + {"error": "URL already exists for the workspace"}, + status=status.HTTP_409_CONFLICT, + ) + raise + + @extend_schema( + operation_id="list_webhooks", + summary="List webhooks", + description=("List all webhooks for the workspace. The `secret_key` is never included in these responses."), + tags=["Webhooks"], + parameters=[WORKSPACE_SLUG_PARAMETER], + responses={ + 200: OpenApiResponse(description="Webhooks", response=WebhookSerializer(many=True)), + 401: UNAUTHORIZED_RESPONSE, + 403: FORBIDDEN_RESPONSE, + 404: WORKSPACE_NOT_FOUND_RESPONSE, + }, + ) + def get(self, request, slug): + webhooks = Webhook.objects.filter(workspace__slug=slug) + serializer = WebhookSerializer(webhooks, fields=WEBHOOK_READ_FIELDS, many=True) + return Response(serializer.data, status=status.HTTP_200_OK) + + +class WebhookDetailAPIEndpoint(BaseAPIView): + """Retrieve, update or delete a single workspace webhook via the token API. + + Split from ``WebhookAPIEndpoint`` so the collection ``GET`` (list) and the + detail ``GET`` (retrieve) get distinct, semantically correct OpenAPI + ``operationId``s instead of colliding on one shared handler. + """ + + permission_classes = [WorkspaceOwnerPermission] + serializer_class = WebhookSerializer + + @extend_schema( + operation_id="retrieve_webhook", + summary="Retrieve webhook", + description="Retrieve a single webhook by ID. The `secret_key` is never included in this response.", + tags=["Webhooks"], + parameters=[WORKSPACE_SLUG_PARAMETER, WEBHOOK_PK_PARAMETER], + responses={ + 200: OpenApiResponse(description="Webhook", response=WebhookSerializer), + 401: UNAUTHORIZED_RESPONSE, + 403: FORBIDDEN_RESPONSE, + 404: WORKSPACE_NOT_FOUND_RESPONSE, + }, + ) + def get(self, request, slug, pk): + webhook = Webhook.objects.get(workspace__slug=slug, pk=pk) + serializer = WebhookSerializer(webhook, fields=WEBHOOK_READ_FIELDS) + return Response(serializer.data, status=status.HTTP_200_OK) + + @extend_schema( + operation_id="update_webhook", + summary="Update webhook", + description=( + "Update a webhook's target `url`, active state or entity toggles. A " + "changed `url` is re-validated against the SSRF/URL guards. The " + "`secret_key` cannot be changed here." + ), + tags=["Webhooks"], + parameters=[WORKSPACE_SLUG_PARAMETER, WEBHOOK_PK_PARAMETER], + request=OpenApiRequest(request=WebhookSerializer), + responses={ + 200: OpenApiResponse(description="Webhook updated", response=WebhookSerializer), + 400: OpenApiResponse(description="Invalid or disallowed webhook URL"), + 401: UNAUTHORIZED_RESPONSE, + 403: FORBIDDEN_RESPONSE, + 404: WORKSPACE_NOT_FOUND_RESPONSE, + 409: CONFLICT_RESPONSE, + }, + ) + def patch(self, request, slug, pk): + webhook = Webhook.objects.get(workspace__slug=slug, pk=pk) + try: + serializer = WebhookSerializer( + webhook, + data=request.data, + context={"request": request}, + partial=True, + fields=WEBHOOK_READ_FIELDS, + ) + if serializer.is_valid(): + serializer.save() + return Response(serializer.data, status=status.HTTP_200_OK) + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + except IntegrityError as e: + # Only the unique webhook-URL violation maps to 409; any other + # integrity error is re-raised rather than masked as a conflict. + if "already exists" in str(e): + return Response( + {"error": "URL already exists for the workspace"}, + status=status.HTTP_409_CONFLICT, + ) + raise + + @extend_schema( + operation_id="delete_webhook", + summary="Delete webhook", + description="Delete a workspace webhook by ID.", + tags=["Webhooks"], + parameters=[WORKSPACE_SLUG_PARAMETER, WEBHOOK_PK_PARAMETER], + responses={ + 204: OpenApiResponse(description="Webhook deleted"), + 401: UNAUTHORIZED_RESPONSE, + 403: FORBIDDEN_RESPONSE, + 404: WORKSPACE_NOT_FOUND_RESPONSE, + }, + ) + def delete(self, request, slug, pk): + webhook = Webhook.objects.get(workspace__slug=slug, pk=pk) + webhook.delete() + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/apps/api/plane/app/serializers/webhook.py b/apps/api/plane/app/serializers/webhook.py index e08726f0d4f..d2e0a8a63e7 100644 --- a/apps/api/plane/app/serializers/webhook.py +++ b/apps/api/plane/app/serializers/webhook.py @@ -2,57 +2,27 @@ # SPDX-License-Identifier: AGPL-3.0-only # See the LICENSE file for details. -# Python imports -import logging -from urllib.parse import urlparse - # Third party imports from rest_framework import serializers -# Django imports -from django.conf import settings - # Module imports from .base import DynamicBaseSerializer from plane.db.models import Webhook, WebhookLog from plane.db.models.webhook import validate_domain, validate_schema -from plane.utils.ip_address import validate_url - -logger = logging.getLogger(__name__) +from plane.utils.webhook import validate_webhook_url class WebhookSerializer(DynamicBaseSerializer): url = serializers.URLField(validators=[validate_schema, validate_domain]) def _validate_webhook_url(self, url): - """Validate a webhook URL against SSRF and disallowed domain rules.""" - try: - validate_url( - url, - allowed_ips=settings.WEBHOOK_ALLOWED_IPS, - allowed_hosts=settings.WEBHOOK_ALLOWED_HOSTS, - ) - except ValueError as e: - logger.warning("Webhook URL validation failed for %s: %s", url, e) - raise serializers.ValidationError({"url": "Invalid or disallowed webhook URL."}) - - hostname = (urlparse(url).hostname or "").rstrip(".").lower() - - # Hosts explicitly trusted via WEBHOOK_ALLOWED_HOSTS bypass the - # disallowed-domain check — they're already trusted for SSRF, so - # the loop-back guard would only get in the way of legitimate - # sibling services that share a parent domain with Plane. - if hostname in settings.WEBHOOK_ALLOWED_HOSTS: - return - - request = self.context.get("request") - disallowed_domains = list(settings.WEBHOOK_DISALLOWED_DOMAINS) - if request: - request_host = request.get_host().split(":")[0].rstrip(".").lower() - disallowed_domains.append(request_host) + """Validate a webhook URL against SSRF and disallowed-domain rules. - if any(hostname == domain or hostname.endswith("." + domain) for domain in disallowed_domains): - raise serializers.ValidationError({"url": "URL domain or its subdomain is not allowed."}) + Thin adapter binding the serializer's request context to the shared + ``validate_webhook_url`` guard so the SSRF/URL checks live in a single + place shared with the public token API. + """ + validate_webhook_url(url, self.context.get("request")) def create(self, validated_data): url = validated_data.get("url", None) diff --git a/apps/api/plane/tests/contract/api/test_webhooks.py b/apps/api/plane/tests/contract/api/test_webhooks.py new file mode 100644 index 00000000000..defcd7b8c0e --- /dev/null +++ b/apps/api/plane/tests/contract/api/test_webhooks.py @@ -0,0 +1,319 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +""" +Contract tests for the public token-API workspace webhook endpoints. + +Covers CRUD via ``/api/v1/workspaces/{slug}/webhooks/`` and proves the two +security-critical guarantees: + +* create returns a usable, server-generated ``secret_key`` (and it is not + leaked on subsequent reads), and +* a delivery built from that secret signs the payload correctly with + ``X-Plane-Signature`` (HMAC-SHA256). +""" + +import hashlib +import hmac +import json +from unittest import mock +from uuid import uuid4 + +import pytest +from django.db import IntegrityError +from rest_framework import serializers, status +from rest_framework.test import APIClient + +from plane.db.models import Webhook, WorkspaceMember +from plane.db.models.api import APIToken + + +def _webhooks_url(slug): + return f"/api/v1/workspaces/{slug}/webhooks/" + + +def _webhook_detail_url(slug, pk): + return f"/api/v1/workspaces/{slug}/webhooks/{pk}/" + + +@pytest.fixture +def webhook_data(): + """A valid webhook payload targeting a public IP literal. + + Using a public IP literal keeps ``validate_url`` (which resolves the host) + deterministic and offline — no DNS lookup happens for a numeric address — + while still exercising the real SSRF guard. + """ + return { + "url": "https://8.8.8.8/webhook", + "issue": True, + "issue_comment": True, + "cycle": True, + "module": True, + "project": True, + } + + +@pytest.fixture +def create_webhook(db, workspace): + """An existing active webhook for the workspace.""" + return Webhook.objects.create( + workspace=workspace, + url="https://8.8.8.8/existing", + issue=True, + ) + + +@pytest.fixture +def member_api_key_client(db, workspace): + """An API-key client whose user is a workspace *member* (role 15). + + Used to prove webhook management is admin-only, mirroring the app API. + """ + from plane.db.models import User + + member = User.objects.create( + email="member@plane.so", + username="member@plane.so", + first_name="Member", + ) + member.set_password("member-password") + member.save() + WorkspaceMember.objects.create(workspace=workspace, member=member, role=15) + token = APIToken.objects.create(user=member, label="Member Token") + + client = APIClient() + client.credentials(HTTP_X_API_KEY=token.token) + return client + + +@pytest.mark.contract +class TestWebhookCreateAPIEndpoint: + @pytest.mark.django_db + def test_create_returns_usable_secret(self, api_key_client, workspace, webhook_data): + """Create generates a server-side secret and returns it once.""" + response = api_key_client.post(_webhooks_url(workspace.slug), webhook_data, format="json") + + assert response.status_code == status.HTTP_201_CREATED + + secret = response.data["secret_key"] + assert secret + assert secret.startswith("plane_wh_") + + # The secret is not client-supplied — it matches the persisted value. + webhook = Webhook.objects.get(id=response.data["id"]) + assert webhook.secret_key == secret + assert webhook.workspace_id == workspace.id + + # Entity toggles round-trip. + assert webhook.issue is True + assert webhook.issue_comment is True + assert webhook.cycle is True + assert webhook.module is True + assert webhook.project is True + + @pytest.mark.django_db + def test_create_ignores_client_supplied_secret(self, api_key_client, workspace, webhook_data): + """secret_key is read-only: a client-supplied value is discarded.""" + payload = {**webhook_data, "secret_key": "attacker-controlled"} + response = api_key_client.post(_webhooks_url(workspace.slug), payload, format="json") + + assert response.status_code == status.HTTP_201_CREATED + assert response.data["secret_key"] != "attacker-controlled" + assert response.data["secret_key"].startswith("plane_wh_") + + @pytest.mark.django_db + def test_create_ignores_client_supplied_internal_and_version(self, api_key_client, workspace, webhook_data): + """is_internal and version are server-controlled: client values are ignored.""" + payload = {**webhook_data, "is_internal": True, "version": "v99"} + response = api_key_client.post(_webhooks_url(workspace.slug), payload, format="json") + + assert response.status_code == status.HTTP_201_CREATED + + # Neither field is exposed on the public serializer, and the persisted + # webhook keeps the server-controlled model defaults. + webhook = Webhook.objects.get(id=response.data["id"]) + assert webhook.is_internal is False + assert webhook.version == "v1" + + @pytest.mark.django_db + def test_create_rejects_ssrf_target(self, api_key_client, workspace): + """The SSRF guard blocks loopback/private targets (not weakened).""" + response = api_key_client.post( + _webhooks_url(workspace.slug), + {"url": "http://127.0.0.1:9000/hook"}, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @pytest.mark.django_db + def test_create_rejects_non_http_scheme(self, api_key_client, workspace): + """Only http(s) schemes are accepted.""" + response = api_key_client.post( + _webhooks_url(workspace.slug), + {"url": "ftp://8.8.8.8/hook"}, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @pytest.mark.django_db + def test_create_duplicate_url_conflict(self, api_key_client, workspace, create_webhook): + """A duplicate URL for the workspace returns 409.""" + response = api_key_client.post( + _webhooks_url(workspace.slug), + {"url": create_webhook.url}, + format="json", + ) + assert response.status_code == status.HTTP_409_CONFLICT + + @pytest.mark.django_db + def test_create_non_unique_integrity_error_not_masked(self, api_key_client, workspace, webhook_data): + """A non-unique IntegrityError must not be misreported as a 409 duplicate-URL conflict.""" + with mock.patch( + "plane.api.views.webhook.WebhookSerializer.save", + side_effect=IntegrityError("null value in column violates not-null constraint"), + ): + response = api_key_client.post(_webhooks_url(workspace.slug), webhook_data, format="json") + + assert response.status_code != status.HTTP_409_CONFLICT + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "already exists" not in str(response.data) + + @pytest.mark.django_db + def test_create_requires_workspace_admin(self, member_api_key_client, workspace, webhook_data): + """Webhook management is workspace-admin only, mirroring the app API.""" + response = member_api_key_client.post(_webhooks_url(workspace.slug), webhook_data, format="json") + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.contract +class TestWebhookReadUpdateDeleteAPIEndpoint: + @pytest.mark.django_db + def test_list_hides_secret(self, api_key_client, workspace, create_webhook): + response = api_key_client.get(_webhooks_url(workspace.slug)) + + assert response.status_code == status.HTTP_200_OK + assert len(response.data) == 1 + assert "secret_key" not in response.data[0] + assert response.data[0]["url"] == create_webhook.url + + @pytest.mark.django_db + def test_retrieve_hides_secret(self, api_key_client, workspace, create_webhook): + response = api_key_client.get(_webhook_detail_url(workspace.slug, create_webhook.id)) + + assert response.status_code == status.HTTP_200_OK + assert response.data["id"] == create_webhook.id + assert "secret_key" not in response.data + + @pytest.mark.django_db + def test_retrieve_not_found(self, api_key_client, workspace): + response = api_key_client.get(_webhook_detail_url(workspace.slug, uuid4())) + assert response.status_code == status.HTTP_404_NOT_FOUND + + @pytest.mark.django_db + def test_update_toggle_and_active(self, api_key_client, workspace, create_webhook): + response = api_key_client.patch( + _webhook_detail_url(workspace.slug, create_webhook.id), + {"is_active": False, "cycle": True}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + create_webhook.refresh_from_db() + assert create_webhook.is_active is False + assert create_webhook.cycle is True + + @pytest.mark.django_db + def test_update_rejects_ssrf_target(self, api_key_client, workspace, create_webhook): + response = api_key_client.patch( + _webhook_detail_url(workspace.slug, create_webhook.id), + {"url": "http://169.254.169.254/latest/meta-data/"}, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @pytest.mark.django_db + def test_delete_webhook(self, api_key_client, workspace, create_webhook): + response = api_key_client.delete(_webhook_detail_url(workspace.slug, create_webhook.id)) + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not Webhook.objects.filter(id=create_webhook.id).exists() + + +@pytest.mark.contract +class TestWebhookDeliverySigning: + @pytest.mark.django_db + def test_delivery_signs_with_created_secret(self, api_key_client, workspace, webhook_data): + """A delivery built from the API-issued secret signs the payload with + an HMAC-SHA256 ``X-Plane-Signature`` the receiver can verify.""" + from plane.bgtasks.webhook_task import webhook_send_task + + # 1. Provision the webhook through the public API and capture its secret. + create_response = api_key_client.post(_webhooks_url(workspace.slug), webhook_data, format="json") + assert create_response.status_code == status.HTTP_201_CREATED + secret = create_response.data["secret_key"] + webhook_id = create_response.data["id"] + + captured = {} + + class _FakeResponse: + status_code = 200 + headers = {} + text = "ok" + + def _fake_pinned_fetch(method, url, **kwargs): + captured["headers"] = kwargs["headers"] + captured["json"] = kwargs["json"] + return _FakeResponse() + + # 2. Run the real delivery task with the network pinned-fetch stubbed out. + with mock.patch("plane.bgtasks.webhook_task.pinned_fetch", side_effect=_fake_pinned_fetch): + webhook_send_task.apply( + kwargs=dict( + webhook_id=str(webhook_id), + slug=workspace.slug, + event="issue", + event_data={"id": str(uuid4())}, + action="POST", + current_site="http://example.com", + activity=None, + ) + ) + + # 3. The receiver recomputes the signature from the shared secret. + headers = captured["headers"] + payload = captured["json"] + expected_signature = hmac.new( + secret.encode("utf-8"), + json.dumps(payload).encode("utf-8"), + hashlib.sha256, + ).hexdigest() + + assert headers["X-Plane-Signature"] == expected_signature + assert headers["X-Plane-Event"] == "issue" + assert payload["event"] == "issue" + assert payload["webhook_id"] == str(webhook_id) + + +@pytest.mark.contract +class TestWebhookLoopbackGuard: + """The request-host loop-back guard must handle bracketed IPv6 hosts.""" + + def test_ipv6_request_host_is_parsed_correctly(self): + from plane.utils.webhook import validate_webhook_url + + request = mock.MagicMock() + # Plane served on an IPv6 host with a port — get_host() returns it bracketed. + request.get_host.return_value = "[2001:db8::5]:8000" + + # Bypass the SSRF resolver so the test targets only the request-host + # loop-back guard (the piece that must parse "[::1]:8000"-style hosts). + with mock.patch("plane.utils.webhook.validate_url"): + # A webhook pointed at the instance's own IPv6 host is rejected — + # a naive split(":")[0] would yield "[2001" and fail to match. + with pytest.raises(serializers.ValidationError): + validate_webhook_url("http://[2001:db8::5]/hook", request) + + # An unrelated public host still passes. + validate_webhook_url("https://8.8.8.8/hook", request) diff --git a/apps/api/plane/utils/webhook.py b/apps/api/plane/utils/webhook.py new file mode 100644 index 00000000000..10626fbafd4 --- /dev/null +++ b/apps/api/plane/utils/webhook.py @@ -0,0 +1,69 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +# Python imports +import logging +from urllib.parse import urlparse + +# Third party imports +from rest_framework import serializers + +# Django imports +from django.conf import settings + +# Module imports +from plane.utils.ip_address import validate_url + +logger = logging.getLogger(__name__) + + +def validate_webhook_url(url, request=None): + """Validate a webhook URL against SSRF and disallowed-domain rules. + + Shared by the internal app API and the public token API webhook + serializers so the SSRF/URL guards can never drift between the two + surfaces. Resolves the host and rejects private/internal targets (unless + explicitly allow-listed via ``WEBHOOK_ALLOWED_IPS``/``WEBHOOK_ALLOWED_HOSTS``), + then rejects hosts matching ``WEBHOOK_DISALLOWED_DOMAINS`` (and the request + host, as a loop-back guard). + + Args: + url: The webhook target URL to validate. + request: The active request, used to append the request host to the + disallowed-domain list (loop-back guard). Optional. + + Raises: + rest_framework.serializers.ValidationError: If the URL resolves to a + blocked/internal target or matches a disallowed domain. + """ + try: + validate_url( + url, + allowed_ips=settings.WEBHOOK_ALLOWED_IPS, + allowed_hosts=settings.WEBHOOK_ALLOWED_HOSTS, + ) + except ValueError as e: + logger.warning("Webhook URL validation failed for %s: %s", url, e) + raise serializers.ValidationError({"url": "Invalid or disallowed webhook URL."}) + + hostname = (urlparse(url).hostname or "").rstrip(".").lower() + + # Hosts explicitly trusted via WEBHOOK_ALLOWED_HOSTS bypass the + # disallowed-domain check — they're already trusted for SSRF, so the + # loop-back guard would only get in the way of legitimate sibling services + # that share a parent domain with Plane. + if hostname in settings.WEBHOOK_ALLOWED_HOSTS: + return + + disallowed_domains = list(settings.WEBHOOK_DISALLOWED_DOMAINS) + if request: + # Parse via urlparse (prefixing "//" so the host is read as a netloc) + # so a bracketed IPv6 literal with a port (e.g. "[::1]:8000") yields the + # bare host "::1" instead of the "[" that a naive split(":")[0] returns. + request_host = (urlparse("//" + request.get_host()).hostname or "").rstrip(".").lower() + if request_host: + disallowed_domains.append(request_host) + + if any(hostname == domain or hostname.endswith("." + domain) for domain in disallowed_domains): + raise serializers.ValidationError({"url": "URL domain or its subdomain is not allowed."})