* [WEB-8103] fix: stop leaking webhook HMAC secret_key on reads (GHSA-83rj)
WebhookEndpoint list/retrieve/patch pass a fields= allowlist that excludes
secret_key, but DynamicBaseSerializer.__init__ discards the caller
allowlist (fields = self.expand). With WebhookSerializer using
fields="__all__" and secret_key only in read_only_fields (read-only is
still serialized), the HMAC signing secret leaked on every webhook read
(GHSA-83rj-4282-x39v; admin-only).
Rather than secret_key = CharField(write_only=True) — which would let a
client inject their own secret on create/patch and break the intended
one-time reveal — hide it by default and reveal only where intended:
- WebhookSerializer.to_representation drops secret_key unless the
show_secret_key context flag is set (secure by default). secret_key
stays server-generated (default=generate_token) and non-writable.
- POST create and WebhookSecretRegenerateEndpoint pass show_secret_key so
the secret is still returned once for the caller to configure their
receiver; list/retrieve/patch no longer emit it.
Add contract regression tests (fail-before verified). Follow-up: the
DynamicBaseSerializer.__init__ allowlist bug affects other serializers —
tracked separately.
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8103] test: address review — patch network boundary + pin to_representation
Per review (@sriramveeraghanta):
- Patch the network boundary (validate_url) instead of the whole private
_validate_webhook_url method, so the domain/schema checks still run and
the test survives a rename of the private method.
- Add an assertion that an explicit fields=("secret_key",) request still
hides the key, pinning to_representation as the enforcement point so a
future DynamicBaseSerializer._filter_fields fix can't silently re-open
the leak.
Co-authored-by: Plane AI <noreply@plane.so>
* [WEB-8103] docs: spell out both levels of the dead fields= allowlist
The previous comment named only DynamicBaseSerializer.__init__ discarding the
caller's fields=, which is half the root cause. _filter_fields never removes
anything either: it builds `allowed` purely to attach expansion serializers for
names not already on the serializer, then returns self.fields unfiltered
(serializers/base.py:45-119).
So the fields= kwargs in views/webhook/base.py are no-ops on two independent
levels. Documented so a future `fields = fields or self.expand` fix isn't
assumed to re-activate the allowlists for confidentiality — _filter_fields has
to be made restrictive first. The show_secret_key context flag remains the sole
enforcement point.
Addresses @sriramveeraghanta's review on #9382. 6/6 contract tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(security): drop advisory identifiers from code comments
Explanations kept unchanged; only the IDs are removed.
Co-authored-by: Plane AI <noreply@plane.so>
---------
Co-authored-by: Plane AI <noreply@plane.so>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
129 lines
4.7 KiB
Python
129 lines
4.7 KiB
Python
# 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
|
|
|
|
# Module imports
|
|
from plane.db.models import Webhook, WebhookLog, Workspace
|
|
from plane.db.models.webhook import generate_token
|
|
from ..base import BaseAPIView
|
|
from plane.app.permissions import allow_permission, ROLE
|
|
from plane.app.serializers import WebhookSerializer, WebhookLogSerializer
|
|
|
|
|
|
class WebhookEndpoint(BaseAPIView):
|
|
@allow_permission(allowed_roles=[ROLE.ADMIN], level="WORKSPACE")
|
|
def post(self, request, slug):
|
|
workspace = Workspace.objects.get(slug=slug)
|
|
try:
|
|
serializer = WebhookSerializer(
|
|
data=request.data,
|
|
context={"request": request, "show_secret_key": True},
|
|
)
|
|
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:
|
|
if "already exists" in str(e):
|
|
return Response(
|
|
{"error": "URL already exists for the workspace"},
|
|
status=status.HTTP_409_CONFLICT,
|
|
)
|
|
raise IntegrityError
|
|
|
|
@allow_permission(allowed_roles=[ROLE.ADMIN], level="WORKSPACE")
|
|
def get(self, request, slug, pk=None):
|
|
if pk is None:
|
|
webhooks = Webhook.objects.filter(workspace__slug=slug)
|
|
serializer = WebhookSerializer(
|
|
webhooks,
|
|
fields=(
|
|
"id",
|
|
"url",
|
|
"is_active",
|
|
"created_at",
|
|
"updated_at",
|
|
"project",
|
|
"issue",
|
|
"cycle",
|
|
"module",
|
|
"issue_comment",
|
|
),
|
|
many=True,
|
|
)
|
|
return Response(serializer.data, status=status.HTTP_200_OK)
|
|
else:
|
|
webhook = Webhook.objects.get(workspace__slug=slug, pk=pk)
|
|
serializer = WebhookSerializer(
|
|
webhook,
|
|
fields=(
|
|
"id",
|
|
"url",
|
|
"is_active",
|
|
"created_at",
|
|
"updated_at",
|
|
"project",
|
|
"issue",
|
|
"cycle",
|
|
"module",
|
|
"issue_comment",
|
|
),
|
|
)
|
|
return Response(serializer.data, status=status.HTTP_200_OK)
|
|
|
|
@allow_permission(allowed_roles=[ROLE.ADMIN], level="WORKSPACE")
|
|
def patch(self, request, slug, pk):
|
|
webhook = Webhook.objects.get(workspace__slug=slug, pk=pk)
|
|
serializer = WebhookSerializer(
|
|
webhook,
|
|
data=request.data,
|
|
context={"request": request},
|
|
partial=True,
|
|
fields=(
|
|
"id",
|
|
"url",
|
|
"is_active",
|
|
"created_at",
|
|
"updated_at",
|
|
"project",
|
|
"issue",
|
|
"cycle",
|
|
"module",
|
|
"issue_comment",
|
|
),
|
|
)
|
|
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)
|
|
|
|
@allow_permission(allowed_roles=[ROLE.ADMIN], level="WORKSPACE")
|
|
def delete(self, request, slug, pk):
|
|
webhook = Webhook.objects.get(pk=pk, workspace__slug=slug)
|
|
webhook.delete()
|
|
return Response(status=status.HTTP_204_NO_CONTENT)
|
|
|
|
|
|
class WebhookSecretRegenerateEndpoint(BaseAPIView):
|
|
@allow_permission(allowed_roles=[ROLE.ADMIN], level="WORKSPACE")
|
|
def post(self, request, slug, pk):
|
|
webhook = Webhook.objects.get(workspace__slug=slug, pk=pk)
|
|
webhook.secret_key = generate_token()
|
|
webhook.save()
|
|
serializer = WebhookSerializer(webhook, context={"show_secret_key": True})
|
|
return Response(serializer.data, status=status.HTTP_200_OK)
|
|
|
|
|
|
class WebhookLogsEndpoint(BaseAPIView):
|
|
@allow_permission(allowed_roles=[ROLE.ADMIN], level="WORKSPACE")
|
|
def get(self, request, slug, webhook_id):
|
|
webhook_logs = WebhookLog.objects.filter(workspace__slug=slug, webhook=webhook_id)
|
|
serializer = WebhookLogSerializer(webhook_logs, many=True)
|
|
return Response(serializer.data, status=status.HTTP_200_OK)
|