From c28fc108e37f333ea9343ee0903a7da4e1056a5e Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Sat, 11 Jul 2026 22:34:21 +0000 Subject: [PATCH 01/20] 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."}) From d9f7a74acb45b299c460c33f5e057646e4e00815 Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Thu, 23 Jul 2026 20:15:29 +0000 Subject: [PATCH 02/20] feat(webhooks): dev-only WEBHOOK_ALLOW_PRIVATE_URLS escape hatch for private targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Provisioning webhooks against a local docker-compose Plane is impossible today because the SSRF guard rejects private/loopback targets — a webhook pointing at http://host.docker.internal:/ (an engine on the docker host) is blocked at both registration and delivery. Add a deliberately narrow, default-off instance flag that permits such targets for local development only: - New WEBHOOK_ALLOW_PRIVATE_URLS setting (settings/common.py), parsed with the house `== "1"` idiom, default off, documented with a production warning next to the other webhook SSRF controls. - Thread an `allow_private` flag through the shared guard so BOTH the app and public token APIs pick it up in one place: - registration: validate_webhook_url -> validate_url (allow_private skips only the private-IP block via require_safe=not allow_private). - delivery: webhook_send_task -> pinned_fetch -> _fetch_validated_hop (require_safe = not (trusted or allow_private)). - The flag ONLY lifts the private-IP block: scheme (http/https) and URL well-formedness are still enforced, and delivery is still pinned to the resolved IP (no DNS-rebinding regression). - Default-off is byte-identical to prior behavior on every path. - Hardening: pinned_fetch_following_redirects (OAuth avatar / link unfurling) explicitly drops any allow_private so the redirect path can never inherit the escape hatch. - Docs: dev-only commented-out example + production warning in the aio and cli community variables.env; wire the var through the cli docker-compose x-app-env anchor so the documented flag actually reaches the container. - Tests: default rejects a private URL, flag permits the same URL, non-http and malformed URLs rejected regardless of the flag, delivery skips the block but still pins, and the redirect path never inherits the flag. Signed-off-by: Seena Fallah --- apps/api/plane/bgtasks/webhook_task.py | 1 + apps/api/plane/settings/common.py | 10 +++ .../plane/tests/contract/api/test_webhooks.py | 50 +++++++++++ .../tests/unit/bg_tasks/test_url_security.py | 85 ++++++++++++++++--- apps/api/plane/utils/ip_address.py | 8 +- apps/api/plane/utils/url_security.py | 30 +++++-- apps/api/plane/utils/webhook.py | 7 ++ deployments/aio/community/variables.env | 6 ++ deployments/cli/community/docker-compose.yml | 2 + deployments/cli/community/variables.env | 6 ++ 10 files changed, 184 insertions(+), 21 deletions(-) diff --git a/apps/api/plane/bgtasks/webhook_task.py b/apps/api/plane/bgtasks/webhook_task.py index 5184ae6162a..d1c6f8cbe21 100644 --- a/apps/api/plane/bgtasks/webhook_task.py +++ b/apps/api/plane/bgtasks/webhook_task.py @@ -319,6 +319,7 @@ def webhook_send_task( webhook.url, allowed_ips=settings.WEBHOOK_ALLOWED_IPS, allowed_hosts=settings.WEBHOOK_ALLOWED_HOSTS, + allow_private=settings.WEBHOOK_ALLOW_PRIVATE_URLS, headers=headers, json=payload, timeout=30, diff --git a/apps/api/plane/settings/common.py b/apps/api/plane/settings/common.py index be444794f24..67ea92702ba 100644 --- a/apps/api/plane/settings/common.py +++ b/apps/api/plane/settings/common.py @@ -90,6 +90,16 @@ if _d.strip() ] +# Dev-only escape hatch: when set, webhook targets that resolve to +# private/loopback/link-local addresses (e.g. http://host.docker.internal:/ +# for a service running on the docker host) are permitted instead of rejected by +# the SSRF guard. Scheme (http/https) and URL well-formedness are still enforced, +# and outbound delivery is still pinned to the resolved IP (no DNS rebinding). +# SECURITY WARNING: this disables the SSRF private-network protection for webhook +# registration AND delivery — never enable it in production. Intended only for +# local / docker-compose development against an engine on the host. +WEBHOOK_ALLOW_PRIVATE_URLS = os.environ.get("WEBHOOK_ALLOW_PRIVATE_URLS", "0") == "1" + # Allowed Hosts ALLOWED_HOSTS = os.environ.get("ALLOWED_HOSTS", "*").split(",") diff --git a/apps/api/plane/tests/contract/api/test_webhooks.py b/apps/api/plane/tests/contract/api/test_webhooks.py index defcd7b8c0e..210ee06b67f 100644 --- a/apps/api/plane/tests/contract/api/test_webhooks.py +++ b/apps/api/plane/tests/contract/api/test_webhooks.py @@ -22,6 +22,7 @@ import pytest from django.db import IntegrityError +from django.test import override_settings from rest_framework import serializers, status from rest_framework.test import APIClient @@ -296,6 +297,55 @@ def _fake_pinned_fetch(method, url, **kwargs): assert payload["webhook_id"] == str(webhook_id) +def _addr(ip): + """Build a getaddrinfo-style result tuple for a resolved IP (no real DNS).""" + family = 6 if ":" in ip else 2 + return (family, None, None, None, (ip, 0)) + + +@pytest.mark.contract +class TestWebhookAllowPrivateFlag: + """The dev-only WEBHOOK_ALLOW_PRIVATE_URLS escape hatch for local development. + + The flag only lifts the private/internal-IP block — scheme and URL + well-formedness are still enforced regardless of the flag. + """ + + PRIVATE_URL = "http://internal.example.com/hook" + + def _validate(self, url): + from plane.utils.webhook import validate_webhook_url + + # Resolve the host to a private IP without touching real DNS. + with mock.patch( + "plane.utils.ip_address.socket.getaddrinfo", + return_value=[_addr("10.0.0.5")], + ): + validate_webhook_url(url) + + def test_private_url_rejected_by_default(self): + # Guard active by default: a private/internal target is rejected. + with pytest.raises(serializers.ValidationError): + self._validate(self.PRIVATE_URL) + + @override_settings(WEBHOOK_ALLOW_PRIVATE_URLS=True) + def test_flag_permits_private_url(self): + # Flag on: the same private target is permitted (no exception). + self._validate(self.PRIVATE_URL) + + @override_settings(WEBHOOK_ALLOW_PRIVATE_URLS=True) + def test_non_http_scheme_rejected_regardless_of_flag(self): + # The flag must NOT bypass scheme validation. + with pytest.raises(serializers.ValidationError): + self._validate("ftp://internal.example.com/hook") + + @override_settings(WEBHOOK_ALLOW_PRIVATE_URLS=True) + def test_malformed_url_rejected_regardless_of_flag(self): + # The flag must NOT bypass URL well-formedness validation. + with pytest.raises(serializers.ValidationError): + self._validate("http:///nohost") + + @pytest.mark.contract class TestWebhookLoopbackGuard: """The request-host loop-back guard must handle bracketed IPv6 hosts.""" diff --git a/apps/api/plane/tests/unit/bg_tasks/test_url_security.py b/apps/api/plane/tests/unit/bg_tasks/test_url_security.py index 3a1e8d3dba1..46a15c58ec8 100644 --- a/apps/api/plane/tests/unit/bg_tasks/test_url_security.py +++ b/apps/api/plane/tests/unit/bg_tasks/test_url_security.py @@ -128,6 +128,75 @@ def test_unresolvable_raises(self): resolve_and_validate("nope.invalid") +# --------------------------------------------------------------------------- +# Dev-only allow_private escape hatch (WEBHOOK_ALLOW_PRIVATE_URLS) +# --------------------------------------------------------------------------- +@pytest.mark.unit +class TestValidateUrlAllowPrivate: + def test_allow_private_permits_private_ip_but_default_blocks(self): + with patch("plane.utils.ip_address.socket.getaddrinfo") as dns: + dns.return_value = [_addr("10.0.0.5")] + # Default (guard active): private target is blocked. + with pytest.raises(ValueError, match="private/internal"): + validate_url("http://internal.example.com/x") + # allow_private=True: the block is lifted (no raise). + validate_url("http://internal.example.com/x", allow_private=True) + + def test_allow_private_still_enforces_scheme(self): + with patch("plane.utils.ip_address.socket.getaddrinfo") as dns: + dns.return_value = [_addr("10.0.0.5")] + with pytest.raises(ValueError, match="scheme"): + validate_url("ftp://internal.example.com/x", allow_private=True) + + +@pytest.mark.unit +class TestPinnedFetchAllowPrivate: + """allow_private lifts the delivery-time block check but preserves pinning.""" + + @patch("plane.utils.url_security.requests.Session") + @patch("plane.utils.url_security.resolve_and_validate") + def test_allow_private_skips_block_but_still_pins(self, mock_resolve, mock_session_cls): + mock_resolve.return_value = ["10.0.0.5"] + session = mock_session_cls.return_value + session.request.return_value = _resp(200) + + pinned_fetch("POST", "http://host.docker.internal:8000/hook", allow_private=True, json={"a": 1}) + + # The private-IP block check is skipped ... + assert mock_resolve.call_args.kwargs["require_safe"] is False + # ... but the socket is STILL pinned to the resolved IP literal (no + # DNS-rebinding window), and the Host header keeps the real hostname. + _, url = session.request.call_args.args + assert url == "http://10.0.0.5:8000/hook" + assert session.request.call_args.kwargs["headers"]["Host"] == "host.docker.internal:8000" + + @patch("plane.utils.url_security.requests.Session") + @patch("plane.utils.url_security.resolve_and_validate") + def test_default_keeps_block_check_on(self, mock_resolve, mock_session_cls): + mock_resolve.return_value = ["93.184.216.34"] + session = mock_session_cls.return_value + session.request.return_value = _resp(200) + + pinned_fetch("POST", "https://example.com/hook") + + # Default (no allow_private, non-trusted host): block check stays on. + assert mock_resolve.call_args.kwargs["require_safe"] is True + + @patch("plane.utils.url_security.requests.Session") + @patch("plane.utils.url_security.resolve_and_validate") + def test_redirect_follower_never_inherits_allow_private(self, mock_resolve, mock_session_cls): + # The redirect-following path (OAuth avatar / link unfurl) must stay + # fully guarded — it must NOT inherit the webhook-only escape hatch even + # if a caller mistakenly threads allow_private=True via kwargs. + mock_resolve.return_value = ["93.184.216.34"] + session = mock_session_cls.return_value + session.request.return_value = _resp(200) + + pinned_fetch_following_redirects("GET", "https://example.com/x", allow_private=True) + + assert mock_resolve.call_args.kwargs["require_safe"] is True + + # --------------------------------------------------------------------------- # Cluster B — connection pinned to the validated IP (DNS-rebinding TOCTOU) # --------------------------------------------------------------------------- @@ -186,9 +255,7 @@ def test_ipv6_validated_ip_is_bracketed(self, mock_resolve, mock_session_cls): @patch("plane.utils.url_security.resolve_and_validate") def test_blocked_target_raises_before_any_request(self, mock_resolve): - mock_resolve.side_effect = ValueError( - "Access to private/internal networks is not allowed" - ) + mock_resolve.side_effect = ValueError("Access to private/internal networks is not allowed") with pytest.raises(ValueError, match="private/internal"): pinned_fetch("POST", "https://attacker.com/hook") @@ -275,9 +342,7 @@ def test_blocks_redirect_to_private_ip(self, mock_resolve, mock_session_cls): ValueError("Access to private/internal networks is not allowed"), ] session = mock_session_cls.return_value - session.request.return_value = _resp( - 302, headers={"Location": "http://169.254.169.254/latest/meta-data/"} - ) + session.request.return_value = _resp(302, headers={"Location": "http://169.254.169.254/latest/meta-data/"}) with pytest.raises(ValueError, match="private/internal"): pinned_fetch_following_redirects("GET", "https://evil.com/r") @@ -286,13 +351,9 @@ def test_blocks_redirect_to_private_ip(self, mock_resolve, mock_session_cls): def test_too_many_redirects(self, mock_resolve, mock_session_cls): mock_resolve.return_value = ["93.184.216.34"] session = mock_session_cls.return_value - session.request.return_value = _resp( - 302, headers={"Location": "https://example.com/loop"} - ) + session.request.return_value = _resp(302, headers={"Location": "https://example.com/loop"}) with pytest.raises(requests.TooManyRedirects): - pinned_fetch_following_redirects( - "GET", "https://example.com/start", max_redirects=3 - ) + pinned_fetch_following_redirects("GET", "https://example.com/start", max_redirects=3) # --------------------------------------------------------------------------- diff --git a/apps/api/plane/utils/ip_address.py b/apps/api/plane/utils/ip_address.py index c4b0d331287..bfc42adecc9 100644 --- a/apps/api/plane/utils/ip_address.py +++ b/apps/api/plane/utils/ip_address.py @@ -156,7 +156,7 @@ def resolve_and_validate(hostname, allowed_ips=None, require_safe=True): return validated -def validate_url(url, allowed_ips=None, allowed_hosts=None): +def validate_url(url, allowed_ips=None, allowed_hosts=None, allow_private=False): """ Validate that a URL doesn't resolve to a private/internal IP address (SSRF protection). @@ -174,6 +174,10 @@ def validate_url(url, allowed_ips=None, allowed_hosts=None): Typically sourced from the WEBHOOK_ALLOWED_HOSTS setting and used for trusted internal services (e.g. Silo) whose IPs are dynamic in containerised deployments. + allow_private: When True, skip the private/internal-IP block check entirely + (the host is still resolved). Scheme and hostname validation + are unaffected. Dev-only escape hatch — see + ``WEBHOOK_ALLOW_PRIVATE_URLS``; never enable in production. Raises: ValueError: If the URL is invalid or resolves to a blocked IP. @@ -193,7 +197,7 @@ def validate_url(url, allowed_ips=None, allowed_hosts=None): }: return - resolve_and_validate(hostname, allowed_ips=allowed_ips) + resolve_and_validate(hostname, allowed_ips=allowed_ips, require_safe=not allow_private) def get_client_ip(request): diff --git a/apps/api/plane/utils/url_security.py b/apps/api/plane/utils/url_security.py index 37f4d0636a1..fc69e1483da 100644 --- a/apps/api/plane/utils/url_security.py +++ b/apps/api/plane/utils/url_security.py @@ -157,7 +157,7 @@ def _close_all(_orig=_orig_close, _sess=session): return response -def _fetch_validated_hop(method, url, *, allowed_ips, allowed_hosts, headers, timeout, **kwargs): +def _fetch_validated_hop(method, url, *, allowed_ips, allowed_hosts, allow_private=False, headers, timeout, **kwargs): """ Resolve ``url``'s host, validate it, then issue a single (non-redirecting) request pinned to a resolved IP. Returns ``(response, normalized_host)``. @@ -166,6 +166,9 @@ def _fetch_validated_hop(method, url, *, allowed_ips, allowed_hosts, headers, ti whose IPs are dynamic): they skip the private-IP *block* check, but the connection is STILL pinned to the resolved IP so a trusted hostname cannot be rebound to a different internal target between validation and connect. + + ``allow_private`` applies that same "skip the block check but still pin" + treatment globally (dev-only escape hatch — see ``WEBHOOK_ALLOW_PRIVATE_URLS``). """ scheme, hostname, port, path, auth = _split_target(url) @@ -174,10 +177,12 @@ def _fetch_validated_hop(method, url, *, allowed_ips, allowed_hosts, headers, ti (h or "").rstrip(".").lower() for h in allowed_hosts if h } - # Resolve once (and validate unless the host is operator-trusted), then pin - # the connection to a resolved IP literal — urllib3 performs no second DNS - # lookup, so the address validated here is exactly the one reached. - ips = resolve_and_validate(hostname, allowed_ips=allowed_ips, require_safe=not trusted) + # Resolve once (and validate unless the host is operator-trusted or the + # dev-only allow_private escape hatch is on), then pin the connection to a + # resolved IP literal — urllib3 performs no second DNS lookup, so the address + # validated here is exactly the one reached (pinning is preserved even when + # the block check is skipped). + ips = resolve_and_validate(hostname, allowed_ips=allowed_ips, require_safe=not (trusted or allow_private)) last_exc = None for ip in ips: @@ -201,6 +206,7 @@ def pinned_fetch( *, allowed_ips=None, allowed_hosts=None, + allow_private=False, headers=None, timeout=30, **kwargs, @@ -210,13 +216,17 @@ def pinned_fetch( connection to a validated IP (defeating DNS rebinding). Does NOT follow redirects. + ``allow_private`` is a dev-only escape hatch (see ``WEBHOOK_ALLOW_PRIVATE_URLS``): + when True the private/internal-IP block is skipped, but the host is still + resolved and the connection is still pinned to the resolved IP. + Raises: ValueError: if the URL is invalid or resolves to a blocked address. requests.RequestException: on network/transport errors. """ response, _ = _fetch_validated_hop( method, url, - allowed_ips=allowed_ips, allowed_hosts=allowed_hosts, + allowed_ips=allowed_ips, allowed_hosts=allowed_hosts, allow_private=allow_private, headers=headers, timeout=timeout, **kwargs, ) return response @@ -243,12 +253,18 @@ def pinned_fetch_following_redirects( requests.TooManyRedirects: if the hop limit is exceeded. requests.RequestException: on network/transport errors. """ + # The redirect-following path is used only by fully-guarded, non-webhook + # callers (OAuth avatar, link unfurling). Pin allow_private=False explicitly + # so the dev-only private-network escape hatch can never reach this path — + # even accidentally via **kwargs (a duplicate key would raise, not silently + # enable the bypass). + kwargs.pop("allow_private", None) current_url = url redirects = 0 while True: response, _ = _fetch_validated_hop( method, current_url, - allowed_ips=allowed_ips, allowed_hosts=allowed_hosts, + allowed_ips=allowed_ips, allowed_hosts=allowed_hosts, allow_private=False, headers=headers, timeout=timeout, **kwargs, ) diff --git a/apps/api/plane/utils/webhook.py b/apps/api/plane/utils/webhook.py index 10626fbafd4..f7a438bd0da 100644 --- a/apps/api/plane/utils/webhook.py +++ b/apps/api/plane/utils/webhook.py @@ -28,6 +28,12 @@ def validate_webhook_url(url, request=None): then rejects hosts matching ``WEBHOOK_DISALLOWED_DOMAINS`` (and the request host, as a loop-back guard). + The ``WEBHOOK_ALLOW_PRIVATE_URLS`` setting is a dev-only escape hatch: when + enabled it permits private/loopback targets (e.g. ``host.docker.internal``) + for local development. It only lifts the private-IP block — the scheme + (http/https) and URL well-formedness are still enforced. Never enable it in + production (see ``settings/common.py``). + Args: url: The webhook target URL to validate. request: The active request, used to append the request host to the @@ -42,6 +48,7 @@ def validate_webhook_url(url, request=None): url, allowed_ips=settings.WEBHOOK_ALLOWED_IPS, allowed_hosts=settings.WEBHOOK_ALLOWED_HOSTS, + allow_private=settings.WEBHOOK_ALLOW_PRIVATE_URLS, ) except ValueError as e: logger.warning("Webhook URL validation failed for %s: %s", url, e) diff --git a/deployments/aio/community/variables.env b/deployments/aio/community/variables.env index 0a74aa2a007..01b2c046dd0 100644 --- a/deployments/aio/community/variables.env +++ b/deployments/aio/community/variables.env @@ -72,3 +72,9 @@ WEBHOOK_ALLOWED_IPS= # SSRF check. Useful for trusted internal services whose container/service IPs are # dynamic (e.g. "silo,silo.namespace.svc.cluster.local") WEBHOOK_ALLOWED_HOSTS= + +# DEV ONLY — set to 1 to allow webhooks that target private/loopback addresses +# (e.g. http://host.docker.internal:/ for a service on the docker host). +# This disables the SSRF private-network protection for webhook registration and +# delivery. NEVER enable this in production; leave it unset in real deployments. +# WEBHOOK_ALLOW_PRIVATE_URLS=1 diff --git a/deployments/cli/community/docker-compose.yml b/deployments/cli/community/docker-compose.yml index cd41be85f0f..c64a04d0cb4 100644 --- a/deployments/cli/community/docker-compose.yml +++ b/deployments/cli/community/docker-compose.yml @@ -60,6 +60,8 @@ x-app-env: &app-env LIVE_SERVER_SECRET_KEY: ${LIVE_SERVER_SECRET_KEY} WEBHOOK_ALLOWED_IPS: ${WEBHOOK_ALLOWED_IPS:-} WEBHOOK_ALLOWED_HOSTS: ${WEBHOOK_ALLOWED_HOSTS:-} + # DEV ONLY — see variables.env; never enable in production. + WEBHOOK_ALLOW_PRIVATE_URLS: ${WEBHOOK_ALLOW_PRIVATE_URLS:-0} services: web: diff --git a/deployments/cli/community/variables.env b/deployments/cli/community/variables.env index f141d01fb7d..ca14ce71d26 100644 --- a/deployments/cli/community/variables.env +++ b/deployments/cli/community/variables.env @@ -96,3 +96,9 @@ WEBHOOK_ALLOWED_IPS= # SSRF check. Useful for trusted internal services whose container/service IPs are # dynamic (e.g. "silo,silo.namespace.svc.cluster.local") WEBHOOK_ALLOWED_HOSTS= + +# DEV ONLY — set to 1 to allow webhooks that target private/loopback addresses +# (e.g. http://host.docker.internal:/ for a service on the docker host). +# This disables the SSRF private-network protection for webhook registration and +# delivery. NEVER enable this in production; leave it unset in real deployments. +# WEBHOOK_ALLOW_PRIVATE_URLS=1 From ca5c320f9bc41de9bbc5c80e4e4c626e173bdc99 Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Sat, 25 Jul 2026 17:51:09 +0000 Subject: [PATCH 03/20] fix(api): stop the webhook OpenAPI schema from declaring secret_key on reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The list/retrieve/update responses were annotated with WebhookSerializer, whose declared fields include the signing `secret_key`, while the handlers filtered it out at runtime via `fields=WEBHOOK_READ_FIELDS`. The published schema therefore contradicted the documented guarantee that the secret is returned only once, on create — all three read operations resolved to the `Webhook` component, which declares `secret_key`. Replace the runtime field filtering with a dedicated read serializer so the response and the schema are generated from the same class and cannot drift: - Add WebhookLiteSerializer, whose field list is derived from WebhookSerializer minus `secret_key`, making the exclusion structural rather than a hand-maintained parallel list. - Use it for the list, retrieve and update responses (runtime + schema) and drop the now-unused WEBHOOK_READ_FIELDS tuple. Create is unchanged and still returns the secret exactly once. - Update PATCH to validate/save with the full serializer (it carries the URL schema, domain and SSRF guards) and render the response with the lite one. - Tests: assert the update response hides the secret, that a client-supplied secret is ignored on update, and that the read shape is exactly the full shape minus `secret_key`. Generated schema now resolves list/retrieve/update to a WebhookLite component with no `secret_key`; create still resolves to Webhook. Signed-off-by: Seena Fallah --- apps/api/plane/api/serializers/__init__.py | 2 +- apps/api/plane/api/serializers/webhook.py | 18 ++++++++ apps/api/plane/api/views/webhook.py | 41 ++++++++----------- .../plane/tests/contract/api/test_webhooks.py | 37 +++++++++++++++++ 4 files changed, 73 insertions(+), 25 deletions(-) diff --git a/apps/api/plane/api/serializers/__init__.py b/apps/api/plane/api/serializers/__init__.py index caf050c7c76..0c2fec4ad46 100644 --- a/apps/api/plane/api/serializers/__init__.py +++ b/apps/api/plane/api/serializers/__init__.py @@ -64,4 +64,4 @@ from .invite import WorkspaceInviteSerializer from .member import ProjectMemberSerializer from .sticky import StickySerializer -from .webhook import WebhookSerializer +from .webhook import WebhookSerializer, WebhookLiteSerializer diff --git a/apps/api/plane/api/serializers/webhook.py b/apps/api/plane/api/serializers/webhook.py index 15c3c425c49..3a1e374443b 100644 --- a/apps/api/plane/api/serializers/webhook.py +++ b/apps/api/plane/api/serializers/webhook.py @@ -65,3 +65,21 @@ class Meta: "created_at", "updated_at", ] + + +class WebhookLiteSerializer(BaseSerializer): + """Read-side webhook shape — everything ``WebhookSerializer`` exposes except + the signing ``secret_key``. + + The secret is surfaced exactly once, in the create response; list, retrieve + and update responses use this serializer so it is never returned on a read. + Deriving the field list from ``WebhookSerializer`` (minus the secret) makes + that exclusion structural rather than a hand-maintained parallel list, and + because the runtime response and the published OpenAPI schema are both + generated from this one class they cannot contradict each other. + """ + + class Meta: + model = Webhook + fields = [field for field in WebhookSerializer.Meta.fields if field != "secret_key"] + read_only_fields = fields diff --git a/apps/api/plane/api/views/webhook.py b/apps/api/plane/api/views/webhook.py index f6ee8ea6fcc..68377c0b3a3 100644 --- a/apps/api/plane/api/views/webhook.py +++ b/apps/api/plane/api/views/webhook.py @@ -18,7 +18,7 @@ # Module imports from .base import BaseAPIView -from plane.api.serializers import WebhookSerializer +from plane.api.serializers import WebhookSerializer, WebhookLiteSerializer from plane.db.models import Webhook, Workspace from plane.utils.permissions import WorkspaceOwnerPermission from plane.utils.openapi import ( @@ -29,21 +29,11 @@ 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", -) +# Reads (list/retrieve/update) serialize through WebhookLiteSerializer, which +# omits the server-generated ``secret_key`` — the secret is surfaced exactly +# once, in the create response, so it is not leaked on every read. Using a +# dedicated serializer rather than runtime field filtering keeps the published +# OpenAPI schema identical to what the endpoint actually returns. WEBHOOK_PK_PARAMETER = OpenApiParameter( name="pk", @@ -113,7 +103,7 @@ def post(self, request, slug): tags=["Webhooks"], parameters=[WORKSPACE_SLUG_PARAMETER], responses={ - 200: OpenApiResponse(description="Webhooks", response=WebhookSerializer(many=True)), + 200: OpenApiResponse(description="Webhooks", response=WebhookLiteSerializer(many=True)), 401: UNAUTHORIZED_RESPONSE, 403: FORBIDDEN_RESPONSE, 404: WORKSPACE_NOT_FOUND_RESPONSE, @@ -121,7 +111,7 @@ def post(self, request, slug): ) def get(self, request, slug): webhooks = Webhook.objects.filter(workspace__slug=slug) - serializer = WebhookSerializer(webhooks, fields=WEBHOOK_READ_FIELDS, many=True) + serializer = WebhookLiteSerializer(webhooks, many=True) return Response(serializer.data, status=status.HTTP_200_OK) @@ -143,7 +133,7 @@ class WebhookDetailAPIEndpoint(BaseAPIView): tags=["Webhooks"], parameters=[WORKSPACE_SLUG_PARAMETER, WEBHOOK_PK_PARAMETER], responses={ - 200: OpenApiResponse(description="Webhook", response=WebhookSerializer), + 200: OpenApiResponse(description="Webhook", response=WebhookLiteSerializer), 401: UNAUTHORIZED_RESPONSE, 403: FORBIDDEN_RESPONSE, 404: WORKSPACE_NOT_FOUND_RESPONSE, @@ -151,7 +141,7 @@ class WebhookDetailAPIEndpoint(BaseAPIView): ) def get(self, request, slug, pk): webhook = Webhook.objects.get(workspace__slug=slug, pk=pk) - serializer = WebhookSerializer(webhook, fields=WEBHOOK_READ_FIELDS) + serializer = WebhookLiteSerializer(webhook) return Response(serializer.data, status=status.HTTP_200_OK) @extend_schema( @@ -166,7 +156,7 @@ def get(self, request, slug, pk): parameters=[WORKSPACE_SLUG_PARAMETER, WEBHOOK_PK_PARAMETER], request=OpenApiRequest(request=WebhookSerializer), responses={ - 200: OpenApiResponse(description="Webhook updated", response=WebhookSerializer), + 200: OpenApiResponse(description="Webhook updated", response=WebhookLiteSerializer), 400: OpenApiResponse(description="Invalid or disallowed webhook URL"), 401: UNAUTHORIZED_RESPONSE, 403: FORBIDDEN_RESPONSE, @@ -177,16 +167,19 @@ def get(self, request, slug, pk): def patch(self, request, slug, pk): webhook = Webhook.objects.get(workspace__slug=slug, pk=pk) try: + # Validate/save with the full serializer (it carries the URL schema, + # domain and SSRF guards); serialize the response with the lite one so + # the secret is not echoed back on update. secret_key is read-only on + # both, so it can never be written here either. 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) + updated = serializer.save() + return Response(WebhookLiteSerializer(updated).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 diff --git a/apps/api/plane/tests/contract/api/test_webhooks.py b/apps/api/plane/tests/contract/api/test_webhooks.py index 210ee06b67f..301d17c2a19 100644 --- a/apps/api/plane/tests/contract/api/test_webhooks.py +++ b/apps/api/plane/tests/contract/api/test_webhooks.py @@ -221,10 +221,26 @@ def test_update_toggle_and_active(self, api_key_client, workspace, create_webhoo ) assert response.status_code == status.HTTP_200_OK + assert "secret_key" not in response.data 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_ignores_client_supplied_secret(self, api_key_client, workspace, create_webhook): + """secret_key is read-only on update too — a supplied value is discarded.""" + original_secret = create_webhook.secret_key + + response = api_key_client.patch( + _webhook_detail_url(workspace.slug, create_webhook.id), + {"secret_key": "attacker-controlled", "is_active": False}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + create_webhook.refresh_from_db() + assert create_webhook.secret_key == original_secret + @pytest.mark.django_db def test_update_rejects_ssrf_target(self, api_key_client, workspace, create_webhook): response = api_key_client.patch( @@ -242,6 +258,27 @@ def test_delete_webhook(self, api_key_client, workspace, create_webhook): assert not Webhook.objects.filter(id=create_webhook.id).exists() +@pytest.mark.contract +class TestWebhookSerializerShapes: + """The read serializer is what the OpenAPI schema is generated from, so its + shape IS the published contract — these assertions keep the documented + response identical to the one the endpoints actually return.""" + + def test_read_shape_excludes_secret_but_create_shape_includes_it(self): + from plane.api.serializers import WebhookLiteSerializer, WebhookSerializer + + # The secret is surfaced exactly once, in the create response ... + assert "secret_key" in WebhookSerializer().fields + # ... and never on a read (list / retrieve / update). + assert "secret_key" not in WebhookLiteSerializer().fields + + def test_read_shape_is_the_full_shape_minus_the_secret(self): + """Adding a field to the webhook API must not silently drop it from reads.""" + from plane.api.serializers import WebhookLiteSerializer, WebhookSerializer + + assert set(WebhookLiteSerializer().fields) == set(WebhookSerializer().fields) - {"secret_key"} + + @pytest.mark.contract class TestWebhookDeliverySigning: @pytest.mark.django_db From a8e4155e54fbc1ee679d59f0c73a1fd12542d41f Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Sat, 11 Jul 2026 22:50:34 +0000 Subject: [PATCH 04/20] feat(webhooks): emit page created/deleted webhooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Workspace webhooks could subscribe to project, issue, cycle, module and issue_comment events but not pages. Add a `page` event so subscribers are notified when a page is created or deleted. - db: add `Webhook.page` BooleanField (default False) + migration 0122_webhook_page. - bgtasks/webhook_task: register `page` in SERIALIZER_MAPPER and MODEL_MAPPER and route `event="page"` to page-subscribed webhooks. Add a read-only api PageSerializer for the payload (excludes the Yjs `description_binary` BinaryField, which has no JSON representation). - app page viewset: dispatch `webhook_activity.delay(event="page", ...)` on create ("created"), destroy ("deleted") and duplicate ("created"), mirroring the project viewset pattern. Duplication is a real create path, so it fires a created event too. - app webhook endpoint: add `page` to the GET/PATCH field enumerations so the new flag is listed alongside its peers (the app WebhookSerializer already exposes it via fields="__all__"). Out of scope (deliberately): page "updated" webhooks and `page_comment` — page edits flow through the Yjs/live collab server (not a DRF save) and there is no PageComment model. Tests: unit coverage for the page routing + serializer in webhook_activity, and contract coverage for create/destroy/duplicate dispatch and for provisioning the `page` flag through the webhook API. Signed-off-by: Seena Fallah --- apps/api/plane/api/serializers/__init__.py | 1 + apps/api/plane/api/serializers/page.py | 42 +++++ apps/api/plane/app/views/page/base.py | 46 +++++ apps/api/plane/app/views/webhook/base.py | 3 + apps/api/plane/bgtasks/webhook_task.py | 9 +- .../plane/db/migrations/0122_webhook_page.py | 17 ++ apps/api/plane/db/models/webhook.py | 1 + .../plane/tests/contract/app/test_page_app.py | 172 ++++++++++++++++++ .../tests/contract/app/test_webhook_app.py | 62 +++++++ .../unit/bg_tasks/test_webhook_page_task.py | 159 ++++++++++++++++ 10 files changed, 511 insertions(+), 1 deletion(-) create mode 100644 apps/api/plane/api/serializers/page.py create mode 100644 apps/api/plane/db/migrations/0122_webhook_page.py create mode 100644 apps/api/plane/tests/contract/app/test_page_app.py create mode 100644 apps/api/plane/tests/contract/app/test_webhook_app.py create mode 100644 apps/api/plane/tests/unit/bg_tasks/test_webhook_page_task.py diff --git a/apps/api/plane/api/serializers/__init__.py b/apps/api/plane/api/serializers/__init__.py index 0c2fec4ad46..5d5dc897bc1 100644 --- a/apps/api/plane/api/serializers/__init__.py +++ b/apps/api/plane/api/serializers/__init__.py @@ -65,3 +65,4 @@ from .member import ProjectMemberSerializer from .sticky import StickySerializer from .webhook import WebhookSerializer, WebhookLiteSerializer +from .page import PageSerializer diff --git a/apps/api/plane/api/serializers/page.py b/apps/api/plane/api/serializers/page.py new file mode 100644 index 00000000000..694fdf88af4 --- /dev/null +++ b/apps/api/plane/api/serializers/page.py @@ -0,0 +1,42 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +# Module imports +from .base import BaseSerializer +from plane.db.models import Page + + +class PageSerializer(BaseSerializer): + """Serializer used to build the webhook payload for page events. + + Read-only projection of a :class:`~plane.db.models.Page`. ``description_binary`` + is intentionally excluded — it is a ``BinaryField`` (the Yjs document) that has + no JSON representation and is not part of the public webhook contract. + """ + + class Meta: + model = Page + fields = [ + "id", + "name", + "description_html", + "description_stripped", + "owned_by", + "access", + "color", + "parent", + "archived_at", + "is_locked", + "is_global", + "view_props", + "logo_props", + "external_id", + "external_source", + "workspace", + "created_at", + "updated_at", + "created_by", + "updated_by", + ] + read_only_fields = fields diff --git a/apps/api/plane/app/views/page/base.py b/apps/api/plane/app/views/page/base.py index ec391afc1aa..82dfa990cc8 100644 --- a/apps/api/plane/app/views/page/base.py +++ b/apps/api/plane/app/views/page/base.py @@ -53,7 +53,9 @@ from plane.bgtasks.page_version_task import track_page_version from plane.bgtasks.recent_visited_task import recent_visited_task from plane.bgtasks.copy_s3_object import copy_s3_objects_of_description_and_assets +from plane.bgtasks.webhook_task import webhook_activity from plane.app.permissions import ProjectPagePermission +from plane.utils.host import base_host def unarchive_archive_page_and_descendants(page_id, archived_at): @@ -146,6 +148,20 @@ def create(self, request, slug, project_id): old_description_html=None, page_id=serializer.data["id"], ) + # Dispatch the webhook for the page creation + webhook_activity.delay( + event="page", + verb="created", + field=None, + old_value=None, + new_value=None, + actor_id=request.user.id, + slug=slug, + current_site=base_host(request=request, is_app=True), + event_id=serializer.data["id"], + old_identifier=None, + new_identifier=None, + ) page = self.get_queryset().get(pk=serializer.data["id"]) serializer = PageDetailSerializer(page) return Response(serializer.data, status=status.HTTP_201_CREATED) @@ -402,6 +418,20 @@ def destroy(self, request, slug, project_id, page_id): ).update(parent=None) page.delete() + # Dispatch the webhook for the page deletion + webhook_activity.delay( + event="page", + verb="deleted", + field=None, + old_value=None, + new_value=None, + actor_id=request.user.id, + slug=slug, + current_site=base_host(request=request, is_app=True), + event_id=page.id, + old_identifier=None, + new_identifier=None, + ) # Delete the user favorite page UserFavorite.objects.filter( project=project_id, @@ -610,6 +640,22 @@ def post(self, request, slug, project_id, page_id): updated_by_id=page.updated_by_id, ) + # Duplicating a page creates a new page, so it fires a "created" webhook + # just like PageViewSet.create does. + webhook_activity.delay( + event="page", + verb="created", + field=None, + old_value=None, + new_value=None, + actor_id=request.user.id, + slug=slug, + current_site=base_host(request=request, is_app=True), + event_id=page.id, + old_identifier=None, + new_identifier=None, + ) + page_transaction.delay( new_description_html=page.description_html, old_description_html=None, diff --git a/apps/api/plane/app/views/webhook/base.py b/apps/api/plane/app/views/webhook/base.py index 24496c742bf..7887005ca7a 100644 --- a/apps/api/plane/app/views/webhook/base.py +++ b/apps/api/plane/app/views/webhook/base.py @@ -52,6 +52,7 @@ def get(self, request, slug, pk=None): "cycle", "module", "issue_comment", + "page", ), many=True, ) @@ -71,6 +72,7 @@ def get(self, request, slug, pk=None): "cycle", "module", "issue_comment", + "page", ), ) return Response(serializer.data, status=status.HTTP_200_OK) @@ -94,6 +96,7 @@ def patch(self, request, slug, pk): "cycle", "module", "issue_comment", + "page", ), ) if serializer.is_valid(): diff --git a/apps/api/plane/bgtasks/webhook_task.py b/apps/api/plane/bgtasks/webhook_task.py index d1c6f8cbe21..e42072c0b03 100644 --- a/apps/api/plane/bgtasks/webhook_task.py +++ b/apps/api/plane/bgtasks/webhook_task.py @@ -30,6 +30,7 @@ IssueExpandSerializer, ModuleIssueSerializer, ModuleSerializer, + PageSerializer, ProjectSerializer, UserLiteSerializer, IntakeIssueSerializer, @@ -41,6 +42,7 @@ IssueComment, Module, ModuleIssue, + Page, Project, User, Webhook, @@ -63,6 +65,7 @@ "cycle_issue": CycleIssueSerializer, "module_issue": ModuleIssueSerializer, "issue_comment": IssueCommentSerializer, + "page": PageSerializer, "user": UserLiteSerializer, "intake_issue": IntakeIssueSerializer, } @@ -75,6 +78,7 @@ "cycle_issue": CycleIssue, "module_issue": ModuleIssue, "issue_comment": IssueComment, + "page": Page, "user": User, "intake_issue": IntakeIssue, } @@ -411,7 +415,7 @@ def webhook_activity( to all active webhooks for the workspace. Args: - event (str): Type of event (project, issue, module, cycle, issue_comment) + event (str): Type of event (project, issue, module, cycle, issue_comment, page) verb (str): Action performed (created, updated, deleted) field (Optional[str]): Name of the field that was changed old_value (Any): Previous value of the field @@ -448,6 +452,9 @@ def webhook_activity( if event == "issue_comment": webhooks = webhooks.filter(issue_comment=True) + if event == "page": + webhooks = webhooks.filter(page=True) + for webhook in webhooks: webhook_send_task.delay( webhook_id=webhook.id, diff --git a/apps/api/plane/db/migrations/0122_webhook_page.py b/apps/api/plane/db/migrations/0122_webhook_page.py new file mode 100644 index 00000000000..3f9d94856da --- /dev/null +++ b/apps/api/plane/db/migrations/0122_webhook_page.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.30 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("db", "0121_alter_estimate_type"), + ] + + operations = [ + migrations.AddField( + model_name="webhook", + name="page", + field=models.BooleanField(default=False), + ), + ] diff --git a/apps/api/plane/db/models/webhook.py b/apps/api/plane/db/models/webhook.py index 99431ed4225..e19e1ab63f0 100644 --- a/apps/api/plane/db/models/webhook.py +++ b/apps/api/plane/db/models/webhook.py @@ -41,6 +41,7 @@ class Webhook(BaseModel): module = models.BooleanField(default=False) cycle = models.BooleanField(default=False) issue_comment = models.BooleanField(default=False) + page = models.BooleanField(default=False) is_internal = models.BooleanField(default=False) version = models.CharField(default="v1", max_length=50) diff --git a/apps/api/plane/tests/contract/app/test_page_app.py b/apps/api/plane/tests/contract/app/test_page_app.py new file mode 100644 index 00000000000..ddbbfa547ba --- /dev/null +++ b/apps/api/plane/tests/contract/app/test_page_app.py @@ -0,0 +1,172 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +from unittest import mock + +import pytest +from django.utils import timezone +from rest_framework import status + +from plane.db.models import Page, Project, ProjectMember, ProjectPage + + +@pytest.fixture +def project(db, workspace, create_user): + """A project in the test workspace with the test user as an active admin.""" + project = Project.objects.create(name="Docs", identifier="DOCS", workspace=workspace) + ProjectMember.objects.create(project=project, member=create_user, role=20, is_active=True) + return project + + +def pages_url(slug, project_id, page_id=None): + base = f"/api/workspaces/{slug}/projects/{project_id}/pages/" + return f"{base}{page_id}/" if page_id else base + + +@pytest.mark.contract +class TestPageWebhookDispatch: + """Creating or deleting a page through the app API fires a ``page`` webhook.""" + + @pytest.fixture(autouse=True) + def _web_url(self, settings): + # base_host() needs an origin to build current_site; the value is + # irrelevant to the assertions but must be a valid URL. + settings.WEB_URL = "http://localhost" + + @pytest.mark.django_db + def test_create_dispatches_page_webhook(self, session_client, workspace, project): + with ( + mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook, + mock.patch("plane.app.views.page.base.page_transaction"), + ): + response = session_client.post( + pages_url(workspace.slug, project.id), + {"name": "Runbook"}, + format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED, f"Got {response.status_code}: {response.data!r}" + page_id = response.data["id"] + assert Page.objects.filter(id=page_id).exists() + assert ProjectPage.objects.filter(page_id=page_id, project=project).exists() + + mocked_webhook.delay.assert_called_once() + kwargs = mocked_webhook.delay.call_args.kwargs + assert kwargs["event"] == "page" + assert kwargs["verb"] == "created" + assert str(kwargs["event_id"]) == str(page_id) + assert kwargs["slug"] == workspace.slug + + @pytest.mark.django_db + def test_create_does_not_dispatch_on_invalid_payload(self, session_client, workspace, project): + """A failed create (serializer invalid) must not fire a webhook.""" + with ( + mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook, + mock.patch("plane.app.views.page.base.page_transaction"), + ): + response = session_client.post( + pages_url(workspace.slug, project.id), + {"access": "not-an-integer"}, + format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert not Page.objects.exists() + mocked_webhook.delay.assert_not_called() + + @pytest.mark.django_db + def test_destroy_dispatches_page_webhook(self, session_client, workspace, project, create_user): + page = Page.objects.create( + workspace=workspace, + owned_by=create_user, + name="Old Page", + archived_at=timezone.now(), + created_by=create_user, + updated_by=create_user, + ) + ProjectPage.objects.create( + workspace=workspace, + project=project, + page=page, + created_by=create_user, + updated_by=create_user, + ) + + with ( + mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook, + # page.delete() soft-deletes and enqueues a cascade task; stub it so + # the test has no hidden broker dependency. + mock.patch("plane.db.mixins.soft_delete_related_objects"), + ): + response = session_client.delete(pages_url(workspace.slug, project.id, page.id)) + + assert response.status_code == status.HTTP_204_NO_CONTENT, f"Got {response.status_code}: {response.data!r}" + assert not Page.objects.filter(id=page.id).exists() + + mocked_webhook.delay.assert_called_once() + kwargs = mocked_webhook.delay.call_args.kwargs + assert kwargs["event"] == "page" + assert kwargs["verb"] == "deleted" + assert str(kwargs["event_id"]) == str(page.id) + + @pytest.mark.django_db + def test_duplicate_dispatches_page_webhook(self, session_client, workspace, project, create_user): + page = Page.objects.create( + workspace=workspace, + owned_by=create_user, + name="Template", + created_by=create_user, + updated_by=create_user, + ) + ProjectPage.objects.create( + workspace=workspace, + project=project, + page=page, + created_by=create_user, + updated_by=create_user, + ) + + url = pages_url(workspace.slug, project.id, page.id) + "duplicate/" + with ( + mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook, + mock.patch("plane.app.views.page.base.page_transaction"), + mock.patch("plane.app.views.page.base.copy_s3_objects_of_description_and_assets"), + ): + response = session_client.post(url, {}, format="json") + + assert response.status_code == status.HTTP_201_CREATED, f"Got {response.status_code}: {response.data!r}" + new_page_id = response.data["id"] + # The duplicate is a distinct new page. + assert str(new_page_id) != str(page.id) + + mocked_webhook.delay.assert_called_once() + kwargs = mocked_webhook.delay.call_args.kwargs + assert kwargs["event"] == "page" + assert kwargs["verb"] == "created" + assert str(kwargs["event_id"]) == str(new_page_id) + + @pytest.mark.django_db + def test_destroy_requires_archived_page(self, session_client, workspace, project, create_user): + """A non-archived page cannot be deleted, so no webhook fires.""" + page = Page.objects.create( + workspace=workspace, + owned_by=create_user, + name="Live Page", + created_by=create_user, + updated_by=create_user, + ) + ProjectPage.objects.create( + workspace=workspace, + project=project, + page=page, + created_by=create_user, + updated_by=create_user, + ) + + with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + response = session_client.delete(pages_url(workspace.slug, project.id, page.id)) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert Page.objects.filter(id=page.id).exists() + mocked_webhook.delay.assert_not_called() diff --git a/apps/api/plane/tests/contract/app/test_webhook_app.py b/apps/api/plane/tests/contract/app/test_webhook_app.py new file mode 100644 index 00000000000..4a5a6e8cc06 --- /dev/null +++ b/apps/api/plane/tests/contract/app/test_webhook_app.py @@ -0,0 +1,62 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +from unittest import mock + +import pytest +from rest_framework import status + +from plane.db.models import Webhook + + +def webhooks_url(slug, pk=None): + base = f"/api/workspaces/{slug}/webhooks/" + return f"{base}{pk}/" if pk else base + + +@pytest.mark.contract +class TestWebhookPageField: + """The workspace webhook API must let callers provision the ``page`` flag.""" + + @pytest.fixture(autouse=True) + def _no_ssrf(self): + # The create/update path runs an SSRF check that resolves DNS; stub it so + # the test stays hermetic and focused on the `page` flag wiring. + with mock.patch("plane.app.serializers.webhook.validate_url"): + yield + + @pytest.mark.django_db + def test_create_webhook_with_page_flag(self, session_client, workspace): + response = session_client.post( + webhooks_url(workspace.slug), + {"url": "https://example.com/hook", "page": True}, + format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED, f"Got {response.status_code}: {response.data!r}" + assert response.data["page"] is True + webhook = Webhook.objects.get(id=response.data["id"]) + assert webhook.page is True + + @pytest.mark.django_db + def test_get_and_patch_page_flag(self, session_client, workspace): + webhook = Webhook.objects.create( + workspace=workspace, + url="https://example.com/hook", + page=False, + ) + detail = webhooks_url(workspace.slug, webhook.id) + + # GET must expose the page flag. + get_resp = session_client.get(detail) + assert get_resp.status_code == status.HTTP_200_OK + assert get_resp.data["page"] is False + + # PATCH must be able to toggle it. + patch_resp = session_client.patch(detail, {"page": True}, format="json") + assert patch_resp.status_code == status.HTTP_200_OK + assert patch_resp.data["page"] is True + + webhook.refresh_from_db() + assert webhook.page is True diff --git a/apps/api/plane/tests/unit/bg_tasks/test_webhook_page_task.py b/apps/api/plane/tests/unit/bg_tasks/test_webhook_page_task.py new file mode 100644 index 00000000000..a8ab76faea5 --- /dev/null +++ b/apps/api/plane/tests/unit/bg_tasks/test_webhook_page_task.py @@ -0,0 +1,159 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +from unittest.mock import patch +from uuid import uuid4 + +import pytest + +from plane.bgtasks.webhook_task import ( + MODEL_MAPPER, + SERIALIZER_MAPPER, + get_model_data, + webhook_activity, +) +from plane.db.models import Page, Webhook + + +@pytest.fixture +def page(db, workspace, create_user): + """A minimal page owned by the test user.""" + return Page.objects.create( + workspace=workspace, + owned_by=create_user, + name="Release Notes", + created_by=create_user, + updated_by=create_user, + ) + + +@pytest.mark.unit +class TestPageWebhookMapping: + """The webhook fan-out must know how to serialize a ``page`` event.""" + + def test_page_registered_in_mappers(self): + assert "page" in MODEL_MAPPER + assert MODEL_MAPPER["page"] is Page + assert "page" in SERIALIZER_MAPPER + + def test_get_model_data_serializes_page(self, page): + data = get_model_data(event="page", event_id=page.id) + assert str(data["id"]) == str(page.id) + assert data["name"] == "Release Notes" + # Pin the payload contract: the identifying + metadata fields are present. + expected_keys = { + "id", + "name", + "owned_by", + "access", + "is_locked", + "workspace", + "created_at", + "updated_at", + } + assert expected_keys <= set(data), f"missing keys: {expected_keys - set(data)}" + # The Yjs binary blob is never part of the webhook contract. + assert "description_binary" not in data + assert "description_json" not in data + + +@pytest.mark.unit +class TestPageWebhookActivity: + """``webhook_activity`` must route ``page`` events to page-subscribed webhooks.""" + + @pytest.fixture + def page_webhook(self, workspace): + return Webhook.objects.create( + workspace=workspace, + url="https://example.com/page-hook", + page=True, + ) + + @pytest.fixture + def other_webhook(self, workspace): + """A webhook that subscribes to issues but not pages.""" + return Webhook.objects.create( + workspace=workspace, + url="https://example.com/issue-hook", + issue=True, + page=False, + ) + + @patch("plane.bgtasks.webhook_task.webhook_send_task") + def test_created_dispatches_only_to_page_webhooks( + self, mock_send_task, page_webhook, other_webhook, page, create_user, workspace + ): + webhook_activity( + event="page", + verb="created", + field=None, + old_value=None, + new_value=None, + actor_id=create_user.id, + slug=workspace.slug, + current_site="http://localhost", + event_id=page.id, + old_identifier=None, + new_identifier=None, + ) + + mock_send_task.delay.assert_called_once() + kwargs = mock_send_task.delay.call_args.kwargs + assert kwargs["webhook_id"] == page_webhook.id + assert kwargs["event"] == "page" + assert kwargs["action"] == "created" + # On create the full serialized page travels in the payload. + assert str(kwargs["event_data"]["id"]) == str(page.id) + assert kwargs["event_data"]["name"] == "Release Notes" + + @patch("plane.bgtasks.webhook_task.webhook_send_task") + def test_deleted_sends_only_the_id(self, mock_send_task, page_webhook, create_user, workspace): + # A delete fires after the row is gone, so the fan-out never reloads the + # page — any id stands in for the already-deleted page here. + deleted_page_id = uuid4() + + webhook_activity( + event="page", + verb="deleted", + field=None, + old_value=None, + new_value=None, + actor_id=create_user.id, + slug=workspace.slug, + current_site="http://localhost", + event_id=deleted_page_id, + old_identifier=None, + new_identifier=None, + ) + + mock_send_task.delay.assert_called_once() + kwargs = mock_send_task.delay.call_args.kwargs + assert kwargs["action"] == "deleted" + # A delete cannot serialize a gone row — only the id is sent. + assert kwargs["event_data"] == {"id": deleted_page_id} + + @patch("plane.bgtasks.webhook_task.webhook_send_task") + def test_inactive_webhook_is_skipped(self, mock_send_task, page, create_user, workspace): + Webhook.objects.create( + workspace=workspace, + url="https://example.com/inactive-hook", + page=True, + is_active=False, + ) + + webhook_activity( + event="page", + verb="created", + field=None, + old_value=None, + new_value=None, + actor_id=create_user.id, + slug=workspace.slug, + current_site="http://localhost", + event_id=page.id, + old_identifier=None, + new_identifier=None, + ) + + mock_send_task.delay.assert_not_called() From 9943f6d2d81dd3b72dae9e46e5c9760427224b56 Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Thu, 23 Jul 2026 20:11:06 +0000 Subject: [PATCH 05/20] feat(webhooks): debounce page update webhooks in webhook_activity Add an optional `debounce` flag to `webhook_activity` and a redis-backed rate limiter for `page` update events. Page *content* is persisted by the live (Yjs) collab server flushing to the API on a fixed 10s Hocuspocus debounce (apps/live/src/hocuspocus.ts), so without throttling a continuous editing session would emit a `page` update webhook roughly every 10s. - `_suppress_page_update_webhook` claims a per-page redis window with SET NX for content flushes (debounce=True) and refreshes it for discrete property edits (debounce=False, which always deliver). Fails open so a redis error never drops a webhook. - PAGE_UPDATE_WEBHOOK_DEBOUNCE = 60s (six live flush cycles) coalesces a live editing session into at most one delivery per page per minute; documented at the constant with the rationale tied to the live flush cadence. Unit tests cover: property edit fires once, two rapid content flushes inside the window fire once, a flush after the window fires again, a property edit refreshing the window suppresses a following flush, toggle-off = no delivery, and the update payload matching the create-event PageSerializer. Signed-off-by: Seena Fallah --- apps/api/plane/bgtasks/webhook_task.py | 67 ++++++++ .../unit/bg_tasks/test_webhook_page_task.py | 160 ++++++++++++++++++ 2 files changed, 227 insertions(+) diff --git a/apps/api/plane/bgtasks/webhook_task.py b/apps/api/plane/bgtasks/webhook_task.py index e42072c0b03..22b8230cea6 100644 --- a/apps/api/plane/bgtasks/webhook_task.py +++ b/apps/api/plane/bgtasks/webhook_task.py @@ -52,6 +52,7 @@ IssueAssignee, ) from plane.license.utils.instance_value import get_email_configuration +from plane.settings.redis import redis_instance from plane.utils.email import generate_plain_text_from_html from plane.utils.exception_logger import log_exception from plane.utils.url_security import pinned_fetch @@ -84,6 +85,61 @@ } +# --------------------------------------------------------------------------- +# Page "updated" webhook debounce +# --------------------------------------------------------------------------- +# Page *content* (the rich-text body) is never written through a normal DRF +# save. The live collaboration server (apps/live) streams Yjs updates and +# flushes the merged document back to the API through the page ``description`` +# endpoint on a fixed Hocuspocus ``debounce`` of 10s +# (``debounce: 10000`` in ``apps/live/src/hocuspocus.ts``). During a continuous +# editing session that flush therefore fires roughly every 10s, and every flush +# would otherwise emit its own ``page`` update webhook — ~6 deliveries a minute +# for a single page. +# +# To keep a live editing session from spamming subscribers we rate-limit +# ``page`` update webhooks to at most one per page per window. 60s == six live +# flush cycles (6 * 10s) collapsed into a single delivery: enough to tame a +# continuous editing session while still telling external systems at least once +# a minute that the page changed. Discrete property edits (rename, access, +# lock/unlock, archive/restore) are never suppressed — they pass +# ``debounce=False`` and simply refresh the window so a content flush landing +# immediately afterwards does not emit a duplicate. +PAGE_UPDATE_WEBHOOK_DEBOUNCE = 60 + + +def _page_update_webhook_debounce_key(page_id: str | uuid.UUID) -> str: + return f"page_update_webhook:{page_id}" + + +def _suppress_page_update_webhook(page_id: str | uuid.UUID, debounce: bool) -> bool: + """Decide whether a ``page`` update webhook should be suppressed. + + A single redis key per page marks that an update webhook fired recently. + + - ``debounce=True`` (live content flush): claim the window atomically with a + ``SET NX``. If the key already exists another update webhook fired within + ``PAGE_UPDATE_WEBHOOK_DEBOUNCE`` seconds, so this flush is suppressed. + - ``debounce=False`` (discrete property edit): never suppressed; just refresh + the window so an immediately following content flush is coalesced. + + Fails open — a redis error must never drop a webhook, so on any failure we + return ``False`` (deliver). + """ + try: + ri = redis_instance() + key = _page_update_webhook_debounce_key(page_id) + if debounce: + # ``set(nx=True)`` returns True only when the key did not exist. + claimed = ri.set(key, "1", nx=True, ex=PAGE_UPDATE_WEBHOOK_DEBOUNCE) + return not claimed + ri.set(key, "1", ex=PAGE_UPDATE_WEBHOOK_DEBOUNCE) + return False + except Exception as e: + log_exception(e, warning=True) + return False + + logger = logging.getLogger("plane.worker") @@ -407,6 +463,7 @@ def webhook_activity( event_id: str | uuid.UUID, old_identifier: Optional[str], new_identifier: Optional[str], + debounce: bool = False, ) -> None: """ Process and send webhook notifications for various activities in the system. @@ -426,6 +483,10 @@ def webhook_activity( event_id (str | uuid.UUID): ID of the event object old_identifier (Optional[str]): Previous identifier if any new_identifier (Optional[str]): New identifier if any + debounce (bool): Only meaningful for ``page`` update events. When True the + delivery is rate-limited per page (see PAGE_UPDATE_WEBHOOK_DEBOUNCE) so + the high-frequency live content-persist flushes do not emit a webhook + each. Discrete property edits leave this False and always deliver. Returns: None @@ -435,6 +496,12 @@ def webhook_activity( race conditions where objects might have been deleted. """ try: + # Collapse the live collab server's frequent content-persist flushes into + # at most one ``page`` update webhook per debounce window. Property edits + # (debounce=False) always deliver and just refresh that window. + if event == "page" and verb == "updated" and _suppress_page_update_webhook(event_id, debounce): + return + webhooks = Webhook.objects.filter(workspace__slug=slug, is_active=True) if event == "project": diff --git a/apps/api/plane/tests/unit/bg_tasks/test_webhook_page_task.py b/apps/api/plane/tests/unit/bg_tasks/test_webhook_page_task.py index a8ab76faea5..f759a4c60cd 100644 --- a/apps/api/plane/tests/unit/bg_tasks/test_webhook_page_task.py +++ b/apps/api/plane/tests/unit/bg_tasks/test_webhook_page_task.py @@ -12,10 +12,40 @@ SERIALIZER_MAPPER, get_model_data, webhook_activity, + _page_update_webhook_debounce_key, ) from plane.db.models import Page, Webhook +class _FakeRedis: + """In-memory stand-in for the subset of redis ``webhook_activity`` uses. + + Models ``set`` with ``nx``/``ex`` exactly like redis-py: a plain ``set`` + returns ``True``; ``set(nx=True)`` returns ``True`` only when the key is + absent and ``None`` when it already exists. TTL is not wall-clock driven — + tests simulate the debounce window elapsing by calling :meth:`delete`. + """ + + def __init__(self): + self.store = {} + + def set(self, key, value, nx=False, ex=None): + if nx and key in self.store: + return None + self.store[key] = value + return True + + def get(self, key): + return self.store.get(key) + + def delete(self, *keys): + removed = 0 + for key in keys: + if self.store.pop(key, None) is not None: + removed += 1 + return removed + + @pytest.fixture def page(db, workspace, create_user): """A minimal page owned by the test user.""" @@ -157,3 +187,133 @@ def test_inactive_webhook_is_skipped(self, mock_send_task, page, create_user, wo ) mock_send_task.delay.assert_not_called() + + +@pytest.mark.unit +class TestPageUpdateWebhookDebounce: + """``page`` update webhooks: property edits always fire, content flushes are + debounced per page so a live editing session does not emit a webhook each + time the collab server persists the document.""" + + @pytest.fixture + def page_webhook(self, workspace): + return Webhook.objects.create( + workspace=workspace, + url="https://example.com/page-hook", + page=True, + ) + + @pytest.fixture + def fake_redis(self): + return _FakeRedis() + + def _fire_update(self, page, create_user, workspace, *, debounce, field): + webhook_activity( + event="page", + verb="updated", + field=field, + old_value=None, + new_value=None, + actor_id=create_user.id, + slug=workspace.slug, + current_site="http://localhost", + event_id=page.id, + old_identifier=None, + new_identifier=None, + debounce=debounce, + ) + + @patch("plane.bgtasks.webhook_task.webhook_send_task") + def test_property_update_fires_once(self, mock_send_task, fake_redis, page_webhook, page, create_user, workspace): + """A discrete property edit (debounce=False) always delivers exactly one + update webhook.""" + with patch("plane.bgtasks.webhook_task.redis_instance", return_value=fake_redis): + self._fire_update(page, create_user, workspace, debounce=False, field="name") + + mock_send_task.delay.assert_called_once() + kwargs = mock_send_task.delay.call_args.kwargs + assert kwargs["event"] == "page" + assert kwargs["action"] == "updated" + assert str(kwargs["event_data"]["id"]) == str(page.id) + + @patch("plane.bgtasks.webhook_task.webhook_send_task") + def test_two_rapid_content_persists_fire_once( + self, mock_send_task, fake_redis, page_webhook, page, create_user, workspace + ): + """Two content flushes inside the debounce window collapse to one + delivery.""" + with patch("plane.bgtasks.webhook_task.redis_instance", return_value=fake_redis): + self._fire_update(page, create_user, workspace, debounce=True, field="description_html") + self._fire_update(page, create_user, workspace, debounce=True, field="description_html") + + mock_send_task.delay.assert_called_once() + + @patch("plane.bgtasks.webhook_task.webhook_send_task") + def test_content_persist_after_window_fires_again( + self, mock_send_task, fake_redis, page_webhook, page, create_user, workspace + ): + """Once the debounce window elapses (marker gone), the next flush fires + again.""" + with patch("plane.bgtasks.webhook_task.redis_instance", return_value=fake_redis): + self._fire_update(page, create_user, workspace, debounce=True, field="description_html") + # Simulate the debounce window expiring (redis TTL) by dropping the marker. + fake_redis.delete(_page_update_webhook_debounce_key(page.id)) + self._fire_update(page, create_user, workspace, debounce=True, field="description_html") + + assert mock_send_task.delay.call_count == 2 + + @patch("plane.bgtasks.webhook_task.webhook_send_task") + def test_property_edit_refreshes_window_and_suppresses_following_flush( + self, mock_send_task, fake_redis, page_webhook, page, create_user, workspace + ): + """A property edit refreshes the window, so a content flush landing right + after it is coalesced (no duplicate).""" + with patch("plane.bgtasks.webhook_task.redis_instance", return_value=fake_redis): + self._fire_update(page, create_user, workspace, debounce=False, field="name") + self._fire_update(page, create_user, workspace, debounce=True, field="description_html") + + mock_send_task.delay.assert_called_once() + + @patch("plane.bgtasks.webhook_task.webhook_send_task") + def test_toggle_off_no_delivery(self, mock_send_task, fake_redis, page, create_user, workspace): + """With no page-subscribed webhook, an update is never delivered.""" + Webhook.objects.create( + workspace=workspace, + url="https://example.com/issue-hook", + issue=True, + page=False, + ) + with patch("plane.bgtasks.webhook_task.redis_instance", return_value=fake_redis): + self._fire_update(page, create_user, workspace, debounce=False, field="name") + + mock_send_task.delay.assert_not_called() + + @patch("plane.bgtasks.webhook_task.webhook_send_task") + def test_update_payload_matches_create_serializer( + self, mock_send_task, fake_redis, page_webhook, page, create_user, workspace + ): + """The update event carries the same PageSerializer payload as create.""" + with patch("plane.bgtasks.webhook_task.redis_instance", return_value=fake_redis): + webhook_activity( + event="page", + verb="created", + field=None, + old_value=None, + new_value=None, + actor_id=create_user.id, + slug=workspace.slug, + current_site="http://localhost", + event_id=page.id, + old_identifier=None, + new_identifier=None, + ) + created_payload = mock_send_task.delay.call_args.kwargs["event_data"] + + mock_send_task.reset_mock() + + self._fire_update(page, create_user, workspace, debounce=True, field="description_html") + updated_payload = mock_send_task.delay.call_args.kwargs["event_data"] + + # Same read-only PageSerializer projection for both create and update. + assert updated_payload == created_payload + assert "description_binary" not in updated_payload From 4431bec5f87eb2fa8f17c50b4aa19e44cc0820da Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Thu, 23 Jul 2026 20:11:21 +0000 Subject: [PATCH 06/20] feat(webhooks): emit page update webhooks on property + content changes PR #9401 wired page create/destroy/duplicate webhooks but deliberately left out updates. Close that gap so external systems can react to page edits. - Route every direct page webhook dispatch through a new `dispatch_page_webhook` helper (DRY: create/destroy/duplicate now use it too). - Property edits fire a `page` update webhook via the existing activity path: - rename / access / parent etc. through `model_activity` (diffs a pre-save snapshot and fans out one update per changed field), mirroring the project viewset; - lock/unlock, access, archive/restore fire `webhook_activity` directly with the changed field (is_locked / access / archived_at). - Content persistence: the description endpoint (PagesDescriptionViewSet) is what the live collab server flushes Yjs state into, so it now fires a `page` update webhook with debounce=True, carrying the shared read-only PageSerializer payload (binary excluded). - Respect the existing Webhook.page toggle; no new model flag. - Incidental fix: page archive computed datetime.now() twice (SQL update vs response body), yielding two slightly different timestamps; capture once and reuse for the SQL, webhook and response. Contract tests assert each endpoint (rename, access, lock, unlock, archive, unarchive, content) dispatches the expected update, and that the content endpoint requests debounce. Signed-off-by: Seena Fallah --- apps/api/plane/app/views/page/base.py | 138 +++++++++++----- .../plane/tests/contract/app/test_page_app.py | 147 ++++++++++++++++++ 2 files changed, 243 insertions(+), 42 deletions(-) diff --git a/apps/api/plane/app/views/page/base.py b/apps/api/plane/app/views/page/base.py index 82dfa990cc8..461559f4abb 100644 --- a/apps/api/plane/app/views/page/base.py +++ b/apps/api/plane/app/views/page/base.py @@ -53,7 +53,7 @@ from plane.bgtasks.page_version_task import track_page_version from plane.bgtasks.recent_visited_task import recent_visited_task from plane.bgtasks.copy_s3_object import copy_s3_objects_of_description_and_assets -from plane.bgtasks.webhook_task import webhook_activity +from plane.bgtasks.webhook_task import model_activity, webhook_activity from plane.app.permissions import ProjectPagePermission from plane.utils.host import base_host @@ -74,6 +74,40 @@ def unarchive_archive_page_and_descendants(page_id, archived_at): cursor.execute(sql, [page_id, archived_at]) +def dispatch_page_webhook( + request, + slug, + page_id, + verb, + field=None, + old_value=None, + new_value=None, + debounce=False, +): + """Fire a ``page`` webhook through the shared webhook activity path. + + Every DRF-side page mutation (create, delete, duplicate, and the property / + content updates) funnels through here so the payload, actor and origin are + built identically. ``debounce`` is only set for the live content-persist + flush (see PagesDescriptionViewSet) to keep an editing session from emitting + a webhook per flush. + """ + webhook_activity.delay( + event="page", + verb=verb, + field=field, + old_value=old_value, + new_value=new_value, + actor_id=request.user.id, + slug=slug, + current_site=base_host(request=request, is_app=True), + event_id=page_id, + old_identifier=None, + new_identifier=None, + debounce=debounce, + ) + + class PageViewSet(BaseViewSet): serializer_class = PageSerializer model = Page @@ -149,19 +183,7 @@ def create(self, request, slug, project_id): page_id=serializer.data["id"], ) # Dispatch the webhook for the page creation - webhook_activity.delay( - event="page", - verb="created", - field=None, - old_value=None, - new_value=None, - actor_id=request.user.id, - slug=slug, - current_site=base_host(request=request, is_app=True), - event_id=serializer.data["id"], - old_identifier=None, - new_identifier=None, - ) + dispatch_page_webhook(request, slug, serializer.data["id"], verb="created") page = self.get_queryset().get(pk=serializer.data["id"]) serializer = PageDetailSerializer(page) return Response(serializer.data, status=status.HTTP_201_CREATED) @@ -197,6 +219,9 @@ def partial_update(self, request, slug, project_id, page_id): serializer = PageDetailSerializer(page, data=request.data, partial=True) page_description = page.description_html + # Snapshot the page before the write so the webhook fan-out can diff + # which DRF-side properties (name, access, …) actually changed. + current_instance = json.dumps(PageDetailSerializer(page).data, cls=DjangoJSONEncoder) if serializer.is_valid(): serializer.save() # capture the page transaction @@ -207,6 +232,19 @@ def partial_update(self, request, slug, project_id, page_id): page_id=page_id, ) + # Dispatch a "page" webhook (action=update) for every changed + # property. model_activity diffs request.data against the + # snapshot and fans out one update event per changed field. + model_activity.delay( + model_name="page", + model_id=str(page_id), + requested_data=request.data, + current_instance=current_instance, + actor_id=request.user.id, + slug=slug, + origin=base_host(request=request, is_app=True), + ) + return Response(serializer.data, status=status.HTTP_200_OK) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) except Page.DoesNotExist: @@ -269,6 +307,9 @@ def lock(self, request, slug, project_id, page_id): page.is_locked = True page.save() + dispatch_page_webhook( + request, slug, page_id, verb="updated", field="is_locked", old_value=False, new_value=True + ) return Response(status=status.HTTP_204_NO_CONTENT) def unlock(self, request, slug, project_id, page_id): @@ -281,6 +322,9 @@ def unlock(self, request, slug, project_id, page_id): page.is_locked = False page.save() + dispatch_page_webhook( + request, slug, page_id, verb="updated", field="is_locked", old_value=True, new_value=False + ) return Response(status=status.HTTP_204_NO_CONTENT) @@ -300,8 +344,12 @@ def access(self, request, slug, project_id, page_id): status=status.HTTP_400_BAD_REQUEST, ) + old_access = page.access page.access = access page.save() + dispatch_page_webhook( + request, slug, page_id, verb="updated", field="access", old_value=old_access, new_value=access + ) return Response(status=status.HTTP_204_NO_CONTENT) def list(self, request, slug, project_id): @@ -348,9 +396,16 @@ def archive(self, request, slug, project_id, page_id): workspace__slug=slug, ).delete() - unarchive_archive_page_and_descendants(page_id, datetime.now()) + # Capture a single timestamp for the SQL update, the webhook and the + # response so all three agree (previously datetime.now() was called + # twice, yielding two slightly different values). + archived_at = datetime.now() + unarchive_archive_page_and_descendants(page_id, archived_at) + dispatch_page_webhook( + request, slug, page_id, verb="updated", field="archived_at", old_value=None, new_value=str(archived_at) + ) - return Response({"archived_at": str(datetime.now())}, status=status.HTTP_200_OK) + return Response({"archived_at": str(archived_at)}, status=status.HTTP_200_OK) def unarchive(self, request, slug, project_id, page_id): page = Page.objects.get( @@ -377,7 +432,17 @@ def unarchive(self, request, slug, project_id, page_id): page.parent = None page.save(update_fields=["parent"]) + old_archived_at = page.archived_at unarchive_archive_page_and_descendants(page_id, None) + dispatch_page_webhook( + request, + slug, + page_id, + verb="updated", + field="archived_at", + old_value=str(old_archived_at) if old_archived_at else None, + new_value=None, + ) return Response(status=status.HTTP_204_NO_CONTENT) @@ -419,19 +484,7 @@ def destroy(self, request, slug, project_id, page_id): page.delete() # Dispatch the webhook for the page deletion - webhook_activity.delay( - event="page", - verb="deleted", - field=None, - old_value=None, - new_value=None, - actor_id=request.user.id, - slug=slug, - current_site=base_host(request=request, is_app=True), - event_id=page.id, - old_identifier=None, - new_identifier=None, - ) + dispatch_page_webhook(request, slug, page.id, verb="deleted") # Delete the user favorite page UserFavorite.objects.filter( project=project_id, @@ -600,6 +653,19 @@ def partial_update(self, request, slug, project_id, page_id): existing_instance=existing_instance, user_id=request.user.id, ) + + # Dispatch a "page" webhook (action=update) for the content change. + # This endpoint is what the live (Yjs) collab server flushes into on + # every store cycle (~10s), so the delivery is debounced per page to + # avoid a webhook per flush during an active editing session. + dispatch_page_webhook( + request, + slug, + page_id, + verb="updated", + field="description_html", + debounce=True, + ) return Response({"message": "Updated successfully"}) else: return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) @@ -642,19 +708,7 @@ def post(self, request, slug, project_id, page_id): # Duplicating a page creates a new page, so it fires a "created" webhook # just like PageViewSet.create does. - webhook_activity.delay( - event="page", - verb="created", - field=None, - old_value=None, - new_value=None, - actor_id=request.user.id, - slug=slug, - current_site=base_host(request=request, is_app=True), - event_id=page.id, - old_identifier=None, - new_identifier=None, - ) + dispatch_page_webhook(request, slug, page.id, verb="created") page_transaction.delay( new_description_html=page.description_html, diff --git a/apps/api/plane/tests/contract/app/test_page_app.py b/apps/api/plane/tests/contract/app/test_page_app.py index ddbbfa547ba..99fe0324487 100644 --- a/apps/api/plane/tests/contract/app/test_page_app.py +++ b/apps/api/plane/tests/contract/app/test_page_app.py @@ -24,6 +24,10 @@ def pages_url(slug, project_id, page_id=None): return f"{base}{page_id}/" if page_id else base +def page_action_url(slug, project_id, page_id, action): + return f"{pages_url(slug, project_id, page_id)}{action}/" + + @pytest.mark.contract class TestPageWebhookDispatch: """Creating or deleting a page through the app API fires a ``page`` webhook.""" @@ -170,3 +174,146 @@ def test_destroy_requires_archived_page(self, session_client, workspace, project assert response.status_code == status.HTTP_400_BAD_REQUEST assert Page.objects.filter(id=page.id).exists() mocked_webhook.delay.assert_not_called() + + +@pytest.mark.contract +class TestPageUpdateWebhookDispatch: + """DRF-side page property + content changes fire a ``page`` update webhook.""" + + @pytest.fixture(autouse=True) + def _web_url(self, settings): + settings.WEB_URL = "http://localhost" + + @pytest.fixture + def page(self, db, workspace, project, create_user): + page = Page.objects.create( + workspace=workspace, + owned_by=create_user, + name="Runbook", + created_by=create_user, + updated_by=create_user, + ) + ProjectPage.objects.create( + workspace=workspace, + project=project, + page=page, + created_by=create_user, + updated_by=create_user, + ) + return page + + @pytest.mark.django_db + def test_rename_dispatches_update_via_model_activity(self, session_client, workspace, project, page): + """A rename goes through model_activity, which diffs and fans out one + ``page`` update webhook per changed field.""" + with ( + mock.patch("plane.app.views.page.base.model_activity") as mocked_model_activity, + mock.patch("plane.app.views.page.base.page_transaction"), + ): + response = session_client.patch( + pages_url(workspace.slug, project.id, page.id), + {"name": "Renamed Runbook"}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK, f"Got {response.status_code}: {response.data!r}" + mocked_model_activity.delay.assert_called_once() + kwargs = mocked_model_activity.delay.call_args.kwargs + assert kwargs["model_name"] == "page" + assert str(kwargs["model_id"]) == str(page.id) + assert kwargs["requested_data"] == {"name": "Renamed Runbook"} + + @pytest.mark.django_db + def test_access_change_dispatches_update(self, session_client, workspace, project, page): + with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + response = session_client.post( + page_action_url(workspace.slug, project.id, page.id, "access"), + {"access": 1}, + format="json", + ) + + assert response.status_code == status.HTTP_204_NO_CONTENT + mocked_webhook.delay.assert_called_once() + kwargs = mocked_webhook.delay.call_args.kwargs + assert kwargs["event"] == "page" + assert kwargs["verb"] == "updated" + assert kwargs["field"] == "access" + assert kwargs["new_value"] == 1 + assert str(kwargs["event_id"]) == str(page.id) + + @pytest.mark.django_db + def test_lock_dispatches_update(self, session_client, workspace, project, page): + with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + response = session_client.post(page_action_url(workspace.slug, project.id, page.id, "lock")) + + assert response.status_code == status.HTTP_204_NO_CONTENT + mocked_webhook.delay.assert_called_once() + kwargs = mocked_webhook.delay.call_args.kwargs + assert kwargs["verb"] == "updated" + assert kwargs["field"] == "is_locked" + assert kwargs["new_value"] is True + + @pytest.mark.django_db + def test_unlock_dispatches_update(self, session_client, workspace, project, page): + page.is_locked = True + page.save() + with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + response = session_client.delete(page_action_url(workspace.slug, project.id, page.id, "lock")) + + assert response.status_code == status.HTTP_204_NO_CONTENT + mocked_webhook.delay.assert_called_once() + kwargs = mocked_webhook.delay.call_args.kwargs + assert kwargs["verb"] == "updated" + assert kwargs["field"] == "is_locked" + assert kwargs["new_value"] is False + + @pytest.mark.django_db + def test_archive_dispatches_update(self, session_client, workspace, project, page): + with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + response = session_client.post(page_action_url(workspace.slug, project.id, page.id, "archive")) + + assert response.status_code == status.HTTP_200_OK + mocked_webhook.delay.assert_called_once() + kwargs = mocked_webhook.delay.call_args.kwargs + assert kwargs["verb"] == "updated" + assert kwargs["field"] == "archived_at" + assert kwargs["new_value"] is not None + + @pytest.mark.django_db + def test_unarchive_dispatches_update(self, session_client, workspace, project, page): + page.archived_at = timezone.now() + page.save() + with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + response = session_client.delete(page_action_url(workspace.slug, project.id, page.id, "archive")) + + assert response.status_code == status.HTTP_204_NO_CONTENT + mocked_webhook.delay.assert_called_once() + kwargs = mocked_webhook.delay.call_args.kwargs + assert kwargs["verb"] == "updated" + assert kwargs["field"] == "archived_at" + assert kwargs["new_value"] is None + + @pytest.mark.django_db + def test_content_persist_dispatches_debounced_update(self, session_client, workspace, project, page): + """The description (live content-persist) endpoint fires a debounced + ``page`` update webhook.""" + with ( + mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook, + mock.patch("plane.app.views.page.base.page_transaction"), + mock.patch("plane.app.views.page.base.track_page_version"), + ): + response = session_client.patch( + page_action_url(workspace.slug, project.id, page.id, "description"), + {"description_html": "

hello

"}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK, f"Got {response.status_code}: {response.data!r}" + mocked_webhook.delay.assert_called_once() + kwargs = mocked_webhook.delay.call_args.kwargs + assert kwargs["event"] == "page" + assert kwargs["verb"] == "updated" + assert kwargs["field"] == "description_html" + # The live flush path is debounced so a session does not emit per flush. + assert kwargs["debounce"] is True + assert str(kwargs["event_id"]) == str(page.id) From 5a1b62bfbadbb9e930178e37dc3c20ebc4be680b Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Thu, 23 Jul 2026 21:12:05 +0000 Subject: [PATCH 07/20] refactor(webhooks): share dispatch_page_webhook from webhook_task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the page webhook dispatch helper out of the internal app viewset and into plane.bgtasks.webhook_task, next to the webhook_activity / model_activity tasks it wraps, so the public token API can fire the exact same `page` events without duplicating the event-firing logic. - webhook_task: add dispatch_page_webhook (builds actor/origin/payload and calls webhook_activity.delay with the shared debounce flag); import base_host. - app page viewset: import the helper instead of defining it locally. - app page contract tests: patch plane.bgtasks.webhook_task.webhook_activity (the helper's new home) instead of the app-view module name. No behaviour change — pure relocation for reuse. Signed-off-by: Seena Fallah --- apps/api/plane/app/views/page/base.py | 36 +------------------ apps/api/plane/bgtasks/webhook_task.py | 27 ++++++++++++++ .../plane/tests/contract/app/test_page_app.py | 22 ++++++------ 3 files changed, 39 insertions(+), 46 deletions(-) diff --git a/apps/api/plane/app/views/page/base.py b/apps/api/plane/app/views/page/base.py index 461559f4abb..049da1e73e8 100644 --- a/apps/api/plane/app/views/page/base.py +++ b/apps/api/plane/app/views/page/base.py @@ -53,7 +53,7 @@ from plane.bgtasks.page_version_task import track_page_version from plane.bgtasks.recent_visited_task import recent_visited_task from plane.bgtasks.copy_s3_object import copy_s3_objects_of_description_and_assets -from plane.bgtasks.webhook_task import model_activity, webhook_activity +from plane.bgtasks.webhook_task import dispatch_page_webhook, model_activity from plane.app.permissions import ProjectPagePermission from plane.utils.host import base_host @@ -74,40 +74,6 @@ def unarchive_archive_page_and_descendants(page_id, archived_at): cursor.execute(sql, [page_id, archived_at]) -def dispatch_page_webhook( - request, - slug, - page_id, - verb, - field=None, - old_value=None, - new_value=None, - debounce=False, -): - """Fire a ``page`` webhook through the shared webhook activity path. - - Every DRF-side page mutation (create, delete, duplicate, and the property / - content updates) funnels through here so the payload, actor and origin are - built identically. ``debounce`` is only set for the live content-persist - flush (see PagesDescriptionViewSet) to keep an editing session from emitting - a webhook per flush. - """ - webhook_activity.delay( - event="page", - verb=verb, - field=field, - old_value=old_value, - new_value=new_value, - actor_id=request.user.id, - slug=slug, - current_site=base_host(request=request, is_app=True), - event_id=page_id, - old_identifier=None, - new_identifier=None, - debounce=debounce, - ) - - class PageViewSet(BaseViewSet): serializer_class = PageSerializer model = Page diff --git a/apps/api/plane/bgtasks/webhook_task.py b/apps/api/plane/bgtasks/webhook_task.py index 22b8230cea6..ff5b69eb3e0 100644 --- a/apps/api/plane/bgtasks/webhook_task.py +++ b/apps/api/plane/bgtasks/webhook_task.py @@ -55,6 +55,7 @@ from plane.settings.redis import redis_instance from plane.utils.email import generate_plain_text_from_html from plane.utils.exception_logger import log_exception +from plane.utils.host import base_host from plane.utils.url_security import pinned_fetch @@ -594,3 +595,29 @@ def model_activity(model_name, model_id, requested_data, current_instance, actor ) return + + +def dispatch_page_webhook(request, slug, page_id, verb, field=None, old_value=None, new_value=None, debounce=False): + """Fire a ``page`` webhook through the shared webhook activity path. + + Every DRF-side page mutation — from both the internal app API and the public + token API (create, delete, duplicate, and the property / content updates) — + funnels through here so the payload, actor and origin are built identically. + ``debounce`` is only set for the live content-persist flush (see + ``PagesDescriptionViewSet`` / the public API content update) to keep an + editing session from emitting a webhook per flush. + """ + webhook_activity.delay( + event="page", + verb=verb, + field=field, + old_value=old_value, + new_value=new_value, + actor_id=request.user.id, + slug=slug, + current_site=base_host(request=request, is_app=True), + event_id=page_id, + old_identifier=None, + new_identifier=None, + debounce=debounce, + ) diff --git a/apps/api/plane/tests/contract/app/test_page_app.py b/apps/api/plane/tests/contract/app/test_page_app.py index 99fe0324487..4d828a27193 100644 --- a/apps/api/plane/tests/contract/app/test_page_app.py +++ b/apps/api/plane/tests/contract/app/test_page_app.py @@ -41,7 +41,7 @@ def _web_url(self, settings): @pytest.mark.django_db def test_create_dispatches_page_webhook(self, session_client, workspace, project): with ( - mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook, + mock.patch("plane.bgtasks.webhook_task.webhook_activity") as mocked_webhook, mock.patch("plane.app.views.page.base.page_transaction"), ): response = session_client.post( @@ -66,7 +66,7 @@ def test_create_dispatches_page_webhook(self, session_client, workspace, project def test_create_does_not_dispatch_on_invalid_payload(self, session_client, workspace, project): """A failed create (serializer invalid) must not fire a webhook.""" with ( - mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook, + mock.patch("plane.bgtasks.webhook_task.webhook_activity") as mocked_webhook, mock.patch("plane.app.views.page.base.page_transaction"), ): response = session_client.post( @@ -98,7 +98,7 @@ def test_destroy_dispatches_page_webhook(self, session_client, workspace, projec ) with ( - mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook, + mock.patch("plane.bgtasks.webhook_task.webhook_activity") as mocked_webhook, # page.delete() soft-deletes and enqueues a cascade task; stub it so # the test has no hidden broker dependency. mock.patch("plane.db.mixins.soft_delete_related_objects"), @@ -133,7 +133,7 @@ def test_duplicate_dispatches_page_webhook(self, session_client, workspace, proj url = pages_url(workspace.slug, project.id, page.id) + "duplicate/" with ( - mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook, + mock.patch("plane.bgtasks.webhook_task.webhook_activity") as mocked_webhook, mock.patch("plane.app.views.page.base.page_transaction"), mock.patch("plane.app.views.page.base.copy_s3_objects_of_description_and_assets"), ): @@ -168,7 +168,7 @@ def test_destroy_requires_archived_page(self, session_client, workspace, project updated_by=create_user, ) - with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + with mock.patch("plane.bgtasks.webhook_task.webhook_activity") as mocked_webhook: response = session_client.delete(pages_url(workspace.slug, project.id, page.id)) assert response.status_code == status.HTTP_400_BAD_REQUEST @@ -225,7 +225,7 @@ def test_rename_dispatches_update_via_model_activity(self, session_client, works @pytest.mark.django_db def test_access_change_dispatches_update(self, session_client, workspace, project, page): - with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + with mock.patch("plane.bgtasks.webhook_task.webhook_activity") as mocked_webhook: response = session_client.post( page_action_url(workspace.slug, project.id, page.id, "access"), {"access": 1}, @@ -243,7 +243,7 @@ def test_access_change_dispatches_update(self, session_client, workspace, projec @pytest.mark.django_db def test_lock_dispatches_update(self, session_client, workspace, project, page): - with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + with mock.patch("plane.bgtasks.webhook_task.webhook_activity") as mocked_webhook: response = session_client.post(page_action_url(workspace.slug, project.id, page.id, "lock")) assert response.status_code == status.HTTP_204_NO_CONTENT @@ -257,7 +257,7 @@ def test_lock_dispatches_update(self, session_client, workspace, project, page): def test_unlock_dispatches_update(self, session_client, workspace, project, page): page.is_locked = True page.save() - with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + with mock.patch("plane.bgtasks.webhook_task.webhook_activity") as mocked_webhook: response = session_client.delete(page_action_url(workspace.slug, project.id, page.id, "lock")) assert response.status_code == status.HTTP_204_NO_CONTENT @@ -269,7 +269,7 @@ def test_unlock_dispatches_update(self, session_client, workspace, project, page @pytest.mark.django_db def test_archive_dispatches_update(self, session_client, workspace, project, page): - with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + with mock.patch("plane.bgtasks.webhook_task.webhook_activity") as mocked_webhook: response = session_client.post(page_action_url(workspace.slug, project.id, page.id, "archive")) assert response.status_code == status.HTTP_200_OK @@ -283,7 +283,7 @@ def test_archive_dispatches_update(self, session_client, workspace, project, pag def test_unarchive_dispatches_update(self, session_client, workspace, project, page): page.archived_at = timezone.now() page.save() - with mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook: + with mock.patch("plane.bgtasks.webhook_task.webhook_activity") as mocked_webhook: response = session_client.delete(page_action_url(workspace.slug, project.id, page.id, "archive")) assert response.status_code == status.HTTP_204_NO_CONTENT @@ -298,7 +298,7 @@ def test_content_persist_dispatches_debounced_update(self, session_client, works """The description (live content-persist) endpoint fires a debounced ``page`` update webhook.""" with ( - mock.patch("plane.app.views.page.base.webhook_activity") as mocked_webhook, + mock.patch("plane.bgtasks.webhook_task.webhook_activity") as mocked_webhook, mock.patch("plane.app.views.page.base.page_transaction"), mock.patch("plane.app.views.page.base.track_page_version"), ): From 035cc988c4ec8db613979f0cb691fd0cf4dd8b04 Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Thu, 23 Jul 2026 21:12:31 +0000 Subject: [PATCH 08/20] feat(api): add project pages to the public token API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds page endpoints to the /api/v1 token API (X-Api-Key), building on community PR #8865 and closing its private-page visibility leak. Endpoints (BaseAPIView + APIKeyAuthentication + ProjectEntityPermission): - GET/POST .../projects//pages/ list (cursor paginated, with `search` and `type`=all|public|private|archived) + create - GET/PATCH/DELETE .../pages// retrieve, update, delete - POST/DELETE .../pages//archive/ archive / restore - POST/DELETE .../pages//lock/ lock / unlock Access control (the review blocker): a single access-scoped queryset on a shared PageAPIEndpoint base — Q(owned_by=user) | Q(access=PUBLIC) — is used by EVERY action, so a private page is only ever visible or mutable to its owner while public pages follow project membership. Per-action authz mirrors the internal PageViewSet: access change is owner-only, archive/restore and delete are owner-or-admin, lock/unlock need no ownership (any member who can see the page). Archive captures a single timestamp (fixes PR #8865's double datetime.now()). Content: exchanged as description_html, sanitized on write with the same validate_html_content sanitizer the internal API uses; the Yjs binary/description_json fields stay out of the serializer. Webhooks: every mutating action reuses the shared helpers so API-driven changes are externally observable identically to UI-driven ones — create/delete fire dispatch_page_webhook (created/deleted); property edits fan out through model_activity (updated, per field); content updates reuse the debounced content-persist webhook. No event-firing logic is duplicated. OpenAPI: page_docs decorator + PAGE_* parameters/examples wired into plane.utils.openapi. Tests: full private-page access matrix (owner / other member / non-project workspace member / guest x every verb), pagination search/type, HTML sanitization on create+update, and webhook assertions incl. a public-API update firing the page webhook with action=update. A cache-clearing fixture keeps the ApiKeyRateThrottle from leaking 429s across tests. Signed-off-by: Seena Fallah --- apps/api/plane/api/serializers/__init__.py | 2 +- apps/api/plane/api/serializers/page.py | 64 ++ apps/api/plane/api/urls/__init__.py | 2 + apps/api/plane/api/urls/page.py | 35 + apps/api/plane/api/views/__init__.py | 7 + apps/api/plane/api/views/page.py | 565 +++++++++++++++++ .../plane/tests/contract/api/test_pages.py | 596 ++++++++++++++++++ apps/api/plane/utils/openapi/__init__.py | 12 + apps/api/plane/utils/openapi/decorators.py | 19 +- apps/api/plane/utils/openapi/examples.py | 43 ++ apps/api/plane/utils/openapi/parameters.py | 29 + 11 files changed, 1372 insertions(+), 2 deletions(-) create mode 100644 apps/api/plane/api/urls/page.py create mode 100644 apps/api/plane/api/views/page.py create mode 100644 apps/api/plane/tests/contract/api/test_pages.py diff --git a/apps/api/plane/api/serializers/__init__.py b/apps/api/plane/api/serializers/__init__.py index 5d5dc897bc1..037bd75a342 100644 --- a/apps/api/plane/api/serializers/__init__.py +++ b/apps/api/plane/api/serializers/__init__.py @@ -65,4 +65,4 @@ from .member import ProjectMemberSerializer from .sticky import StickySerializer from .webhook import WebhookSerializer, WebhookLiteSerializer -from .page import PageSerializer +from .page import PageSerializer, PageAPISerializer diff --git a/apps/api/plane/api/serializers/page.py b/apps/api/plane/api/serializers/page.py index 694fdf88af4..799a1b8fc7a 100644 --- a/apps/api/plane/api/serializers/page.py +++ b/apps/api/plane/api/serializers/page.py @@ -2,9 +2,13 @@ # 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 Page +from plane.utils.content_validator import validate_html_content class PageSerializer(BaseSerializer): @@ -40,3 +44,63 @@ class Meta: "updated_by", ] read_only_fields = fields + + +class PageAPISerializer(BaseSerializer): + """Read/write serializer for pages in the public token (v1) API. + + Third-party integrations exchange page content as ``description_html``, + sanitized on write with the same ``validate_html_content`` sanitizer the + internal API uses. The Yjs ``description_binary`` / ``description_json`` + fields are intentionally excluded from the public contract — they are the + live-collaboration document state and have no stable JSON representation. + """ + + class Meta: + model = Page + fields = [ + "id", + "name", + "description_html", + "access", + "color", + "is_locked", + "archived_at", + "view_props", + "logo_props", + "external_id", + "external_source", + "owned_by", + "parent", + "sort_order", + "created_at", + "updated_at", + "created_by", + "updated_by", + ] + read_only_fields = [ + "id", + "is_locked", + "archived_at", + "owned_by", + "created_at", + "updated_at", + "created_by", + "updated_by", + ] + + def validate_description_html(self, value): + """Sanitize page HTML with the same sanitizer the internal API uses. + + Mirrors ``PageBinaryUpdateSerializer.validate_description_html`` so + API-authored content is held to the identical nh3 allow-list. + """ + if not value: + return value + + is_valid, error_message, sanitized_html = validate_html_content(value) + if not is_valid: + raise serializers.ValidationError(error_message) + + # Return sanitized HTML if available, otherwise return original + return sanitized_html if sanitized_html is not None else value diff --git a/apps/api/plane/api/urls/__init__.py b/apps/api/plane/api/urls/__init__.py index 6556faa191c..7c1c3def642 100644 --- a/apps/api/plane/api/urls/__init__.py +++ b/apps/api/plane/api/urls/__init__.py @@ -15,6 +15,7 @@ from .invite import urlpatterns as invite_patterns from .sticky import urlpatterns as sticky_patterns from .webhook import urlpatterns as webhook_patterns +from .page import urlpatterns as page_patterns urlpatterns = [ *asset_patterns, @@ -23,6 +24,7 @@ *label_patterns, *member_patterns, *module_patterns, + *page_patterns, *project_patterns, *state_patterns, *user_patterns, diff --git a/apps/api/plane/api/urls/page.py b/apps/api/plane/api/urls/page.py new file mode 100644 index 00000000000..41925f1c695 --- /dev/null +++ b/apps/api/plane/api/urls/page.py @@ -0,0 +1,35 @@ +# 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.page import ( + PageListCreateAPIEndpoint, + PageDetailAPIEndpoint, + PageArchiveAPIEndpoint, + PageLockAPIEndpoint, +) + +urlpatterns = [ + path( + "workspaces//projects//pages/", + PageListCreateAPIEndpoint.as_view(http_method_names=["get", "post"]), + name="page-list-create", + ), + path( + "workspaces//projects//pages//", + PageDetailAPIEndpoint.as_view(http_method_names=["get", "patch", "delete"]), + name="page-detail", + ), + path( + "workspaces//projects//pages//archive/", + PageArchiveAPIEndpoint.as_view(http_method_names=["post", "delete"]), + name="page-archive", + ), + path( + "workspaces//projects//pages//lock/", + PageLockAPIEndpoint.as_view(http_method_names=["post", "delete"]), + name="page-lock", + ), +] diff --git a/apps/api/plane/api/views/__init__.py b/apps/api/plane/api/views/__init__.py index 6a5534899b9..f35c66acdbe 100644 --- a/apps/api/plane/api/views/__init__.py +++ b/apps/api/plane/api/views/__init__.py @@ -65,3 +65,10 @@ from .sticky import StickyViewSet from .webhook import WebhookAPIEndpoint, WebhookDetailAPIEndpoint + +from .page import ( + PageListCreateAPIEndpoint, + PageDetailAPIEndpoint, + PageArchiveAPIEndpoint, + PageLockAPIEndpoint, +) diff --git a/apps/api/plane/api/views/page.py b/apps/api/plane/api/views/page.py new file mode 100644 index 00000000000..f5413db31d8 --- /dev/null +++ b/apps/api/plane/api/views/page.py @@ -0,0 +1,565 @@ +# 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 json +from datetime import datetime + +# Django imports +from django.core.serializers.json import DjangoJSONEncoder +from django.db import connection, transaction +from django.db.models import Q + +# Third party imports +from rest_framework import status +from rest_framework.response import Response +from drf_spectacular.utils import OpenApiResponse + +# Module imports +from plane.api.serializers import PageAPISerializer +from plane.app.permissions import ProjectEntityPermission +from plane.db.models import ( + Page, + Project, + ProjectMember, + ProjectPage, + UserFavorite, + UserRecentVisit, +) +from plane.bgtasks.page_transaction_task import page_transaction +from plane.bgtasks.webhook_task import dispatch_page_webhook, model_activity +from plane.utils.host import base_host +from plane.utils.openapi import ( + page_docs, + PAGE_ID_PARAMETER, + PAGE_TYPE_PARAMETER, + SEARCH_PARAMETER, + CURSOR_PARAMETER, + PER_PAGE_PARAMETER, + FIELDS_PARAMETER, + EXPAND_PARAMETER, + create_paginated_response, + PAGE_CREATE_EXAMPLE, + PAGE_UPDATE_EXAMPLE, + PAGE_EXAMPLE, + EXTERNAL_ID_EXISTS_RESPONSE, +) + +from .base import BaseAPIView + + +def unarchive_archive_page_and_descendants(page_id, archived_at): + """Archive or unarchive a page and all of its descendant pages.""" + sql = """ + WITH RECURSIVE descendants AS ( + SELECT id FROM pages WHERE id = %s + UNION ALL + SELECT pages.id FROM pages, descendants WHERE pages.parent_id = descendants.id + ) + UPDATE pages SET archived_at = %s WHERE id IN (SELECT id FROM descendants); + """ + with connection.cursor() as cursor: + cursor.execute(sql, [page_id, archived_at]) + + +class PageAPIEndpoint(BaseAPIView): + """Shared base for the public v1 page endpoints. + + Centralises the access-scoped queryset so every action — list, retrieve, + update, delete, archive and lock — honours page visibility identically: a + private page (``access=PRIVATE_ACCESS``) is only ever visible or mutable to + its owner, while public pages follow project membership (enforced by + ``ProjectEntityPermission``). This closes the private-page leak where any + project member could read or edit another member's private page. + """ + + serializer_class = PageAPISerializer + model = Page + permission_classes = [ProjectEntityPermission] + use_read_replica = True + + def get_queryset(self): + return ( + Page.objects.filter(workspace__slug=self.kwargs.get("slug")) + .filter( + projects__id=self.kwargs.get("project_id"), + projects__archived_at__isnull=True, + ) + .filter(project_pages__deleted_at__isnull=True) + # Visibility rule: the owner sees their own pages at any access + # level; everyone else only sees public pages. Private pages never + # leak to non-owners — used by every action, read or write. + .filter(Q(owned_by=self.request.user) | Q(access=Page.PUBLIC_ACCESS)) + .select_related("workspace", "owned_by") + .distinct() + ) + + def _is_owner_or_admin(self, page): + """True if the caller owns the page or is a project admin. + + Mirrors the internal ``PageViewSet`` authorization for archive/restore + and delete. + """ + if page.owned_by_id == self.request.user.id: + return True + return ProjectMember.objects.filter( + workspace__slug=self.kwargs.get("slug"), + project_id=self.kwargs.get("project_id"), + member=self.request.user, + role=20, + is_active=True, + ).exists() + + +class PageListCreateAPIEndpoint(PageAPIEndpoint): + """List pages in a project or create a new page (public v1 API).""" + + @page_docs( + operation_id="list_pages", + summary="List pages", + description=( + "Retrieve a paginated list of the pages the caller can access in a project. " + "Private pages are only returned to their owner. Filter with `type` " + "(all | public | private | archived) and `search` (page name)." + ), + parameters=[ + CURSOR_PARAMETER, + PER_PAGE_PARAMETER, + PAGE_TYPE_PARAMETER, + SEARCH_PARAMETER, + FIELDS_PARAMETER, + EXPAND_PARAMETER, + ], + responses={ + 200: create_paginated_response( + PageAPISerializer, + "PaginatedPageResponse", + "Paginated list of pages", + "Paginated Pages", + ), + }, + ) + def get(self, request, slug, project_id): + """List pages + + Retrieve a paginated list of pages the caller can access, filtered by + `type` and `search`. Archived pages are excluded unless `type=archived`. + """ + queryset = self.get_queryset() + + search = request.GET.get("search") + if search: + queryset = queryset.filter(name__icontains=search) + + page_type = request.GET.get("type", "all") + if page_type == "public": + queryset = queryset.filter(access=Page.PUBLIC_ACCESS, archived_at__isnull=True) + elif page_type == "private": + queryset = queryset.filter(access=Page.PRIVATE_ACCESS, archived_at__isnull=True) + elif page_type == "archived": + queryset = queryset.filter(archived_at__isnull=False) + else: + queryset = queryset.filter(archived_at__isnull=True) + + return self.paginate( + request=request, + queryset=queryset.order_by("-created_at"), + on_results=lambda pages: PageAPISerializer(pages, many=True, fields=self.fields, expand=self.expand).data, + ) + + @page_docs( + operation_id="create_page", + summary="Create page", + description=( + "Create a new page in a project. Content is provided as `description_html`, " + "sanitized on write. Supports `external_id`/`external_source` for integrations." + ), + request=PageAPISerializer, + examples=[PAGE_CREATE_EXAMPLE], + responses={ + 201: OpenApiResponse( + description="Page created", + response=PageAPISerializer, + examples=[PAGE_EXAMPLE], + ), + 409: EXTERNAL_ID_EXISTS_RESPONSE, + }, + ) + def post(self, request, slug, project_id): + """Create page + + Create a new page owned by the caller. `description_html` is sanitized; + the Yjs binary is left empty for the live service to derive. + """ + project = Project.objects.get(pk=project_id, workspace__slug=slug) + + serializer = PageAPISerializer(data=request.data) + if serializer.is_valid(): + # Reject a duplicate (external_source, external_id) pair. + if ( + request.data.get("external_id") + and request.data.get("external_source") + and Page.objects.filter( + projects__id=project_id, + workspace__slug=slug, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).exists() + ): + existing = Page.objects.filter( + workspace__slug=slug, + projects__id=project_id, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).first() + return Response( + { + "error": "Page with the same external id and external source already exists", + "id": str(existing.id), + }, + status=status.HTTP_409_CONFLICT, + ) + + with transaction.atomic(): + page = serializer.save( + owned_by=request.user, + workspace_id=project.workspace_id, + # description_html arrives already sanitized via the + # serializer; default to an empty doc when omitted. The + # binary starts empty — the live service derives it. + description_html=serializer.validated_data.get("description_html") or "

", + description_binary=None, + ) + ProjectPage.objects.create( + workspace_id=project.workspace_id, + project_id=project_id, + page_id=page.id, + created_by_id=request.user.id, + updated_by_id=request.user.id, + ) + + # Track the page transaction for version history. + page_transaction.delay( + new_description_html=page.description_html, + old_description_html=None, + page_id=page.id, + ) + # Fire the same `page` created webhook the internal app API fires. + dispatch_page_webhook(request, slug, page.id, verb="created") + + return Response(PageAPISerializer(page).data, status=status.HTTP_201_CREATED) + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + +class PageDetailAPIEndpoint(PageAPIEndpoint): + """Retrieve, update or delete a page (public v1 API).""" + + @page_docs( + operation_id="retrieve_page", + summary="Retrieve page", + description="Retrieve a page by ID. Private pages are only visible to their owner.", + parameters=[PAGE_ID_PARAMETER, FIELDS_PARAMETER, EXPAND_PARAMETER], + responses={ + 200: OpenApiResponse( + description="Page details", + response=PageAPISerializer, + examples=[PAGE_EXAMPLE], + ) + }, + ) + def get(self, request, slug, project_id, page_id): + """Retrieve page""" + page = self.get_queryset().get(pk=page_id) + return Response( + PageAPISerializer(page, fields=self.fields, expand=self.expand).data, + status=status.HTTP_200_OK, + ) + + @page_docs( + operation_id="update_page", + summary="Update page", + description=( + "Update a page's properties or content. Locked and archived pages cannot be " + "edited, and only the owner may change `access`. Content is sanitized " + "`description_html`." + ), + parameters=[PAGE_ID_PARAMETER], + request=PageAPISerializer, + examples=[PAGE_UPDATE_EXAMPLE], + responses={ + 200: OpenApiResponse( + description="Page updated", + response=PageAPISerializer, + examples=[PAGE_EXAMPLE], + ), + 400: OpenApiResponse(description="Page is locked or archived"), + 403: OpenApiResponse(description="Only the owner can change access"), + }, + ) + def patch(self, request, slug, project_id, page_id): + """Update page""" + page = self.get_queryset().get(pk=page_id) + + if page.is_locked: + return Response({"error": "Page is locked"}, status=status.HTTP_400_BAD_REQUEST) + + if page.archived_at: + return Response( + {"error": "Archived page cannot be edited"}, + status=status.HTTP_400_BAD_REQUEST, + ) + + # Only the owner can change the access level. + if page.access != request.data.get("access", page.access) and page.owned_by_id != request.user.id: + return Response( + {"error": "Access cannot be updated since this page is owned by someone else"}, + status=status.HTTP_403_FORBIDDEN, + ) + + old_description_html = page.description_html + # Snapshot BEFORE the write so the webhook fan-out can diff which + # properties actually changed. + current_instance = json.dumps(PageAPISerializer(page).data, cls=DjangoJSONEncoder) + + serializer = PageAPISerializer(page, data=request.data, partial=True) + if serializer.is_valid(): + if request.data.get("description_html"): + # A direct content write resets the Yjs binary; the live service + # re-derives it from the HTML on next open. + serializer.save(description_binary=None) + else: + serializer.save() + + if request.data.get("description_html"): + page_transaction.delay( + new_description_html=request.data.get("description_html", "

"), + old_description_html=old_description_html, + page_id=page_id, + ) + + # Property edits fan out one `page` update webhook per changed field + # through the shared model_activity path. description_html is excluded + # here — content is signalled separately (debounced) below. + property_data = {key: value for key, value in request.data.items() if key != "description_html"} + if property_data: + model_activity.delay( + model_name="page", + model_id=str(page_id), + requested_data=property_data, + current_instance=current_instance, + actor_id=request.user.id, + slug=slug, + origin=base_host(request=request, is_app=True), + ) + + # Content changes reuse the debounced content-persist webhook so an + # integration streaming edits does not emit a webhook per call. + if request.data.get("description_html"): + dispatch_page_webhook( + request, + slug, + page_id, + verb="updated", + field="description_html", + debounce=True, + ) + + return Response(serializer.data, status=status.HTTP_200_OK) + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + @page_docs( + operation_id="delete_page", + summary="Delete page", + description="Permanently delete an archived page. Only the owner or a project admin can delete.", + parameters=[PAGE_ID_PARAMETER], + responses={ + 204: OpenApiResponse(description="Page deleted"), + 400: OpenApiResponse(description="Page must be archived first"), + 403: OpenApiResponse(description="Only owner or admin can delete"), + }, + ) + def delete(self, request, slug, project_id, page_id): + """Delete page""" + page = self.get_queryset().get(pk=page_id) + + if page.archived_at is None: + return Response( + {"error": "The page should be archived before deleting"}, + status=status.HTTP_400_BAD_REQUEST, + ) + + if not self._is_owner_or_admin(page): + return Response( + {"error": "Only admin or owner can delete the page"}, + status=status.HTTP_403_FORBIDDEN, + ) + + # Detach children so the recursive delete does not orphan them. + Page.objects.filter( + parent_id=page_id, + projects__id=project_id, + workspace__slug=slug, + project_pages__deleted_at__isnull=True, + ).update(parent=None) + + page.delete() + # Fire the same `page` deleted webhook the internal app API fires. + dispatch_page_webhook(request, slug, page.id, verb="deleted") + + UserFavorite.objects.filter( + project=project_id, + workspace__slug=slug, + entity_identifier=page_id, + entity_type="page", + ).delete() + UserRecentVisit.objects.filter( + project_id=project_id, + workspace__slug=slug, + entity_identifier=page_id, + entity_name="page", + ).delete(soft=False) + + return Response(status=status.HTTP_204_NO_CONTENT) + + +class PageArchiveAPIEndpoint(PageAPIEndpoint): + """Archive or restore a page (public v1 API).""" + + @page_docs( + operation_id="archive_page", + summary="Archive page", + description="Archive a page and all its descendants. Only the owner or a project admin can archive.", + parameters=[PAGE_ID_PARAMETER], + request=None, + responses={ + 200: OpenApiResponse(description="Page archived"), + 403: OpenApiResponse(description="Only owner or admin can archive"), + }, + ) + def post(self, request, slug, project_id, page_id): + """Archive page""" + page = self.get_queryset().get(pk=page_id) + + if not self._is_owner_or_admin(page): + return Response( + {"error": "Only the owner or admin can archive the page"}, + status=status.HTTP_403_FORBIDDEN, + ) + + UserFavorite.objects.filter( + entity_type="page", + entity_identifier=page_id, + project_id=project_id, + workspace__slug=slug, + ).delete() + + # Single timestamp shared by the SQL update, the webhook and the + # response so all three agree. + archived_at = datetime.now() + unarchive_archive_page_and_descendants(page_id, archived_at) + dispatch_page_webhook( + request, + slug, + page_id, + verb="updated", + field="archived_at", + old_value=None, + new_value=str(archived_at), + ) + + return Response({"archived_at": str(archived_at)}, status=status.HTTP_200_OK) + + @page_docs( + operation_id="unarchive_page", + summary="Unarchive page", + description="Unarchive a page and all its descendants. Only the owner or a project admin can restore.", + parameters=[PAGE_ID_PARAMETER], + request=None, + responses={ + 204: OpenApiResponse(description="Page unarchived"), + 403: OpenApiResponse(description="Only owner or admin can unarchive"), + }, + ) + def delete(self, request, slug, project_id, page_id): + """Unarchive page""" + page = self.get_queryset().get(pk=page_id) + + if not self._is_owner_or_admin(page): + return Response( + {"error": "Only the owner or admin can unarchive the page"}, + status=status.HTTP_403_FORBIDDEN, + ) + + old_archived_at = page.archived_at + # If the parent is still archived, break the hierarchy. + if page.parent_id and page.parent.archived_at: + page.parent = None + page.save(update_fields=["parent"]) + + unarchive_archive_page_and_descendants(page_id, None) + dispatch_page_webhook( + request, + slug, + page_id, + verb="updated", + field="archived_at", + old_value=str(old_archived_at) if old_archived_at else None, + new_value=None, + ) + + return Response(status=status.HTTP_204_NO_CONTENT) + + +class PageLockAPIEndpoint(PageAPIEndpoint): + """Lock or unlock a page (public v1 API).""" + + @page_docs( + operation_id="lock_page", + summary="Lock page", + description="Lock a page to prevent editing. Any project member who can access the page may lock it.", + parameters=[PAGE_ID_PARAMETER], + request=None, + responses={200: OpenApiResponse(description="Page locked")}, + ) + def post(self, request, slug, project_id, page_id): + """Lock page""" + page = self.get_queryset().get(pk=page_id) + + page.is_locked = True + page.save() + dispatch_page_webhook( + request, + slug, + page_id, + verb="updated", + field="is_locked", + old_value=False, + new_value=True, + ) + return Response({"is_locked": True}, status=status.HTTP_200_OK) + + @page_docs( + operation_id="unlock_page", + summary="Unlock page", + description="Unlock a page to allow editing. Any project member who can access the page may unlock it.", + parameters=[PAGE_ID_PARAMETER], + request=None, + responses={200: OpenApiResponse(description="Page unlocked")}, + ) + def delete(self, request, slug, project_id, page_id): + """Unlock page""" + page = self.get_queryset().get(pk=page_id) + + page.is_locked = False + page.save() + dispatch_page_webhook( + request, + slug, + page_id, + verb="updated", + field="is_locked", + old_value=True, + new_value=False, + ) + return Response({"is_locked": False}, status=status.HTTP_200_OK) diff --git a/apps/api/plane/tests/contract/api/test_pages.py b/apps/api/plane/tests/contract/api/test_pages.py new file mode 100644 index 00000000000..f6118e8bcc0 --- /dev/null +++ b/apps/api/plane/tests/contract/api/test_pages.py @@ -0,0 +1,596 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +from unittest import mock + +import pytest +from django.core.cache import cache +from django.utils import timezone +from rest_framework import status +from rest_framework.test import APIClient + +from plane.db.models import ( + Page, + Project, + ProjectMember, + ProjectPage, + User, + WorkspaceMember, +) +from plane.db.models.api import APIToken + + +@pytest.fixture(autouse=True) +def _clear_throttle_cache(): + """Reset the ApiKeyRateThrottle counter (redis-backed cache) between tests. + + The throttle state lives in the shared cache, not the DB, so it is not + rolled back per test — without clearing it, a run's requests accumulate and + later tests spuriously hit HTTP 429. Mirrors the pattern in + ``test_authentication.py``. + """ + cache.clear() + yield + cache.clear() + + +# --------------------------------------------------------------------------- +# Fixtures & helpers +# --------------------------------------------------------------------------- + + +@pytest.fixture +def project(db, workspace, create_user): + """A project in the test workspace with the api-key user as an admin.""" + project = Project.objects.create( + name="Test Project", + identifier="TP", + workspace=workspace, + created_by=create_user, + ) + ProjectMember.objects.create(project=project, member=create_user, role=20, is_active=True) + return project + + +def _api_client_for(user): + """Build an X-Api-Key authenticated client for an arbitrary user.""" + token = APIToken.objects.create(user=user, label="test-token") + client = APIClient() + client.credentials(HTTP_X_API_KEY=token.token) + return client + + +@pytest.fixture +def make_project_user(db, workspace, project): + """Factory: a user provisioned at a given project role (or workspace-only).""" + + def _make(email, project_role): + # username is unique and defaults to "" — set it so multiple test users + # don't collide on the empty string. + user = User.objects.create(email=email, username=email, first_name=email.split("@")[0]) + user.set_password("password") + user.save() + WorkspaceMember.objects.create(workspace=workspace, member=user, role=15) + if project_role is not None: + ProjectMember.objects.create(project=project, member=user, role=project_role, is_active=True) + return user + + return _make + + +@pytest.fixture +def role_clients(api_key_client, make_project_user): + """A client for each role in the access matrix. + + - owner: the api-key user (project admin) who owns the pages under test + - member: a different project member (role 15) + - outsider: a workspace member who is NOT in the project + - guest: a project guest (role 5) + """ + return { + "owner": api_key_client, + "member": _api_client_for(make_project_user("member@plane.so", 15)), + "outsider": _api_client_for(make_project_user("outsider@plane.so", None)), + "guest": _api_client_for(make_project_user("guest@plane.so", 5)), + } + + +def make_page(project, owner, *, access=Page.PRIVATE_ACCESS, archived=False, locked=False, name="Secret Page"): + page = Page.objects.create( + name=name, + description_html="

content

", + owned_by=owner, + workspace=project.workspace, + access=access, + archived_at=timezone.now().date() if archived else None, + is_locked=locked, + ) + ProjectPage.objects.create( + workspace=project.workspace, + project=project, + page=page, + created_by_id=owner.id, + updated_by_id=owner.id, + ) + return page + + +def list_url(slug, project_id): + return f"/api/v1/workspaces/{slug}/projects/{project_id}/pages/" + + +def detail_url(slug, project_id, page_id): + return f"{list_url(slug, project_id)}{page_id}/" + + +def archive_url(slug, project_id, page_id): + return f"{detail_url(slug, project_id, page_id)}archive/" + + +def lock_url(slug, project_id, page_id): + return f"{detail_url(slug, project_id, page_id)}lock/" + + +# --------------------------------------------------------------------------- +# List / Create +# --------------------------------------------------------------------------- + + +@pytest.mark.contract +class TestPageListCreateAPIEndpoint: + @pytest.mark.django_db + def test_unauthenticated_request(self, api_client, workspace, project): + # No X-Api-Key -> APIKeyAuthentication returns 401 Unauthorized. + response = api_client.get(list_url(workspace.slug, project.id)) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + @pytest.mark.django_db + def test_list_pages_success(self, api_key_client, workspace, project, create_user): + make_page(project, create_user, access=Page.PUBLIC_ACCESS, name="A public page") + response = api_key_client.get(list_url(workspace.slug, project.id)) + assert response.status_code == status.HTTP_200_OK + assert "results" in response.data + assert len(response.data["results"]) >= 1 + + @pytest.mark.django_db + def test_list_excludes_archived_by_default(self, api_key_client, workspace, project, create_user): + live = make_page(project, create_user, access=Page.PUBLIC_ACCESS, name="Live") + archived = make_page(project, create_user, access=Page.PUBLIC_ACCESS, archived=True, name="Archived") + response = api_key_client.get(list_url(workspace.slug, project.id)) + ids = [str(p["id"]) for p in response.data["results"]] + assert str(live.id) in ids + assert str(archived.id) not in ids + + @pytest.mark.django_db + def test_list_search_filters_by_name(self, api_key_client, workspace, project, create_user): + make_page(project, create_user, access=Page.PUBLIC_ACCESS, name="Runbook") + make_page(project, create_user, access=Page.PUBLIC_ACCESS, name="Design Doc") + response = api_key_client.get(list_url(workspace.slug, project.id), {"search": "run"}) + names = [p["name"] for p in response.data["results"]] + assert names == ["Runbook"] + + @pytest.mark.django_db + def test_list_type_filter(self, api_key_client, workspace, project, create_user): + public = make_page(project, create_user, access=Page.PUBLIC_ACCESS, name="Public") + private = make_page(project, create_user, access=Page.PRIVATE_ACCESS, name="Private") + archived = make_page(project, create_user, access=Page.PUBLIC_ACCESS, archived=True, name="Archived") + + def ids(resp): + return {str(p["id"]) for p in resp.data["results"]} + + public_ids = ids(api_key_client.get(list_url(workspace.slug, project.id), {"type": "public"})) + assert str(public.id) in public_ids + assert str(private.id) not in public_ids + assert str(archived.id) not in public_ids + + private_ids = ids(api_key_client.get(list_url(workspace.slug, project.id), {"type": "private"})) + assert private_ids == {str(private.id)} + + archived_ids = ids(api_key_client.get(list_url(workspace.slug, project.id), {"type": "archived"})) + assert archived_ids == {str(archived.id)} + + @pytest.mark.django_db + def test_list_hides_other_users_private_pages(self, role_clients, workspace, project, create_user): + """The private-page leak fix: a member never sees another user's private page.""" + private = make_page(project, create_user, access=Page.PRIVATE_ACCESS, name="Owner Only") + public = make_page(project, create_user, access=Page.PUBLIC_ACCESS, name="Shared") + + response = role_clients["member"].get(list_url(workspace.slug, project.id)) + ids = [str(p["id"]) for p in response.data["results"]] + assert str(public.id) in ids + assert str(private.id) not in ids + + @pytest.mark.django_db + def test_create_page_success(self, api_key_client, workspace, project): + data = {"name": "New Page", "description_html": "

Hello world

"} + response = api_key_client.post(list_url(workspace.slug, project.id), data, format="json") + + assert response.status_code == status.HTTP_201_CREATED + assert response.data["name"] == "New Page" + page = Page.objects.get(pk=response.data["id"]) + assert page.description_binary is None + assert ProjectPage.objects.filter(page=page, project=project).exists() + + @pytest.mark.django_db + def test_create_sanitizes_description_html(self, api_key_client, workspace, project): + """HTML content is sanitized on write with the same nh3 sanitizer.""" + data = { + "name": "XSS", + "description_html": "

ok

", + } + response = api_key_client.post(list_url(workspace.slug, project.id), data, format="json") + assert response.status_code == status.HTTP_201_CREATED + page = Page.objects.get(pk=response.data["id"]) + assert ""}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + mocked_transaction.delay.assert_called_once() + recorded = mocked_transaction.delay.call_args.kwargs["new_description_html"] + assert ""}, + format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + recorded = mocked_transaction.delay.call_args.kwargs["new_description_html"] + assert "