feat: allow private/loopback webhook URLs for self-hosted instances#8837
feat: allow private/loopback webhook URLs for self-hosted instances#8837prue-starfield wants to merge 1 commit intomakeplane:previewfrom
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduced environment-driven Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/api/plane/app/serializers/webhook.py (2)
14-15: Implementation is correct; ensure operators understand the security trade-off.The env var parsing is sound. However, since
webhook_task.pyperforms no independent IP validation at delivery time, enabling this flag completely removes SSRF protection for webhooks. This is the intended behavior for self-hosted instances, but consider:
- Adding a startup log warning when this flag is enabled to increase operator awareness
- Documenting this flag's security implications in deployment documentation
💡 Optional: Add startup warning when flag is enabled
# Allow private/loopback webhook URLs for self-hosted instances ALLOW_PRIVATE_WEBHOOKS = os.environ.get("PLANE_ALLOW_PRIVATE_WEBHOOKS", "0").lower() in ("1", "true", "yes") + +if ALLOW_PRIVATE_WEBHOOKS: + import logging + logging.getLogger(__name__).warning( + "PLANE_ALLOW_PRIVATE_WEBHOOKS is enabled - webhooks can target private/internal IPs. " + "Only enable this in trusted self-hosted environments." + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/plane/app/serializers/webhook.py` around lines 14 - 15, Add an explicit startup warning when ALLOW_PRIVATE_WEBHOOKS is enabled: detect the ALLOW_PRIVATE_WEBHOOKS flag at application startup (where configuration is initialized), and emit a clear, high-visibility log message (e.g., warning level) explaining that enabling ALLOW_PRIVATE_WEBHOOKS disables SSRF protection for webhooks and pointing to webhook_task.py for delivery behavior; also add/update deployment documentation to describe the security trade-offs and recommended usage for self-hosted instances.
79-83: Consider extracting duplicated URL validation logic.The IP validation logic (lines 79-83) is identical to lines 43-47 in
create(). The entire URL validation block (hostname extraction, DNS resolution, IP checks, domain checks) is duplicated between both methods.♻️ Proposed refactor to reduce duplication
+def _validate_webhook_url(url: str, request) -> None: + """Validate webhook URL for IP restrictions and disallowed domains.""" + hostname = urlparse(url).hostname + if not hostname: + raise serializers.ValidationError({"url": "Invalid URL: No hostname found."}) + + try: + ip_addresses = socket.getaddrinfo(hostname, None) + except socket.gaierror: + raise serializers.ValidationError({"url": "Hostname could not be resolved."}) + + if not ip_addresses: + raise serializers.ValidationError({"url": "No IP addresses found for the hostname."}) + + if not ALLOW_PRIVATE_WEBHOOKS: + for addr in ip_addresses: + ip = ipaddress.ip_address(addr[4][0]) + if ip.is_private or ip.is_loopback or ip.is_reserved or ip.is_link_local: + raise serializers.ValidationError({"url": "URL resolves to a blocked IP address."}) + + disallowed_domains = ["plane.so"] + if request: + request_host = request.get_host().split(":")[0] + 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."}) + class WebhookSerializer(DynamicBaseSerializer): url = serializers.URLField(validators=[validate_schema, validate_domain]) def create(self, validated_data): url = validated_data.get("url", None) - # ... duplicated validation code ... + _validate_webhook_url(url, self.context.get("request")) return Webhook.objects.create(**validated_data) def update(self, instance, validated_data): url = validated_data.get("url", None) if url: - # ... duplicated validation code ... + _validate_webhook_url(url, self.context.get("request")) return super().update(instance, validated_data)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/plane/app/serializers/webhook.py` around lines 79 - 83, Extract the duplicated URL validation block used in create() and the other webhook method into a single helper function (e.g., _validate_webhook_url or validate_webhook_url) inside the serializer module; move hostname extraction, DNS resolution, IP parsing, the ALLOW_PRIVATE_WEBHOOKS conditional, and the ip.is_private/is_loopback/is_reserved/is_link_local checks into that helper so it raises serializers.ValidationError on failure, then call this helper from both create() and the other method to preserve existing behavior and flags (including ALLOW_PRIVATE_WEBHOOKS) without changing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/api/plane/app/serializers/webhook.py`:
- Around line 14-15: Add an explicit startup warning when ALLOW_PRIVATE_WEBHOOKS
is enabled: detect the ALLOW_PRIVATE_WEBHOOKS flag at application startup (where
configuration is initialized), and emit a clear, high-visibility log message
(e.g., warning level) explaining that enabling ALLOW_PRIVATE_WEBHOOKS disables
SSRF protection for webhooks and pointing to webhook_task.py for delivery
behavior; also add/update deployment documentation to describe the security
trade-offs and recommended usage for self-hosted instances.
- Around line 79-83: Extract the duplicated URL validation block used in
create() and the other webhook method into a single helper function (e.g.,
_validate_webhook_url or validate_webhook_url) inside the serializer module;
move hostname extraction, DNS resolution, IP parsing, the ALLOW_PRIVATE_WEBHOOKS
conditional, and the ip.is_private/is_loopback/is_reserved/is_link_local checks
into that helper so it raises serializers.ValidationError on failure, then call
this helper from both create() and the other method to preserve existing
behavior and flags (including ALLOW_PRIVATE_WEBHOOKS) without changing logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f801a05-83d9-4f11-9900-a8039da25217
📒 Files selected for processing (1)
apps/api/plane/app/serializers/webhook.py
|
Changes looks good to me. Please sign the CLA. |
prue-starfield
force-pushed
the
fix/allow-private-webhook-urls
branch
from
131f88f to
2e3799e
Compare
|
Thanks @sriramveeraghanta! I've updated the PR:
Clean single-file diff now. Will sign the CLA shortly. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/plane/app/serializers/webhook.py`:
- Line 40: The hostname extraction at hostname = urlparse(url).hostname (and the
similar comparisons around the block handling lines 63-67) must normalize
hostnames before comparing to blocked domains: trim any trailing dots, convert
to lowercase, and apply IDNA/punycode normalization if relevant; then use the
normalized hostname for domain checks and comparisons to the blocked-domain list
(e.g., replace raw hostname usage in the webhook validation functions with the
normalized_hostname variable) so canonical variants cannot bypass the blocklist.
- Line 71: The URLField is always using validate_domain which overrides the
ALLOW_PRIVATE_WEBHOOKS opt-in; change the serializer to apply validate_domain
conditionally: either remove validate_domain from
serializers.URLField(validators=[...]) and in the serializer's __init__ (or a
classmethod like get_validators) append validate_domain only when
settings.ALLOW_PRIVATE_WEBHOOKS is False, or modify validate_domain itself to
early-return/skip loopback checks when settings.ALLOW_PRIVATE_WEBHOOKS is True;
target the URLField declaration and the serializer class initializer and
reference validate_schema, validate_domain, and the ALLOW_PRIVATE_WEBHOOKS
setting so private loopback hosts are allowed when the flag is enabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e51c716c-d710-4147-9692-c63d3654fa48
📒 Files selected for processing (1)
apps/api/plane/app/serializers/webhook.py
Add PLANE_ALLOW_PRIVATE_WEBHOOKS environment variable that bypasses IP validation for webhook URLs when set to '1', 'true', or 'yes'. Disabled by default. Self-hosted Plane instances commonly need webhooks pointing to local services (notification relays, CI runners, internal APIs) but the SSRF protection blocks all private/loopback/reserved/link-local IPs. Changes: - Gated private-IP bypass via env var (default: disabled) - Startup log warning when flag is enabled - Extracted shared URL validation into _validate_webhook_url() helper - Zero impact on existing behavior when not set Refs: makeplane#5248
prue-starfield
force-pushed
the
fix/allow-private-webhook-urls
branch
from
2e3799e to
bbea154
Compare
3 participants
Summary
Add
PLANE_ALLOW_PRIVATE_WEBHOOKSenvironment variable that bypasses IP validation for webhook URLs when set to1,true, oryes. Disabled by default.Problem
Self-hosted Plane instances commonly need webhooks pointing to local services (notification relays, CI runners, internal APIs) but the SSRF protection blocks all private/loopback/reserved/link-local IPs unconditionally.
This was raised in #5248 and affects anyone running Plane behind a reverse proxy with internal webhook receivers.
Solution
Gated bypass via environment variable
PLANE_ALLOW_PRIVATE_WEBHOOKS. When enabled, the private IP check inWebhookSerializeris skipped for both create and update paths.Changes
apps/api/plane/app/serializers/webhook.py: ReadPLANE_ALLOW_PRIVATE_WEBHOOKSenv var; skip IP validation when enabledTesting
Summary by CodeRabbit
New Features
Behavior
Quality