From 533e0449e2e258b42a23daf755633bb2221173d0 Mon Sep 17 00:00:00 2001
From: vitali87 <eheva87@gmail.com>
Date: Sun, 30 Aug 2026 19:43:59 +0100
Subject: [PATCH] fix: reject non-http webhook URLs for merge --notify up front

---
 README.md                  |  2 +-
 pr_split/cli.py            | 19 ++++++++++++++++++-
 tests/test_cli_coverage.py | 31 +++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index b254398..8355325 100644
--- a/README.md
+++ b/README.md
@@ -118,7 +118,7 @@ Use `--auto` to queue merges behind CI checks (uses `gh pr merge --auto`):
 pr-split merge --auto
 ```
 
-Use `--notify` to POST merge results to a webhook URL (e.g. Slack, Discord):
+Use `--notify` to POST merge results to an http(s) webhook URL (e.g. Slack, Discord); other schemes are rejected up front, and a delivery failure (timeout, non-2xx) is logged as a warning without changing the exit code:
 
 ```bash
 pr-split merge --notify https://hooks.slack.com/...
diff --git a/pr_split/cli.py b/pr_split/cli.py
index ec892db..cdccc5d 100644
--- a/pr_split/cli.py
+++ b/pr_split/cli.py
@@ -4,6 +4,7 @@
 import shutil
 import tempfile
 import time
+import urllib.parse
 import urllib.request
 from collections.abc import Generator
 from concurrent.futures import ThreadPoolExecutor, as_completed
@@ -1102,6 +1103,21 @@ def _poll_for_merged(
     return actually_merged
 
 
+def _validate_webhook_url(value: str | None) -> str | None:
+    """Accept only http(s) webhook URLs.
+
+    urllib happily opens file:// (and reports "Webhook notification sent"),
+    and a bare hostname fails only after the merges with an obscure
+    "unknown url type" warning.
+    """
+    if value is None:
+        return None
+    parts = urllib.parse.urlsplit(value)
+    if parts.scheme not in ("http", "https") or not parts.netloc:
+        raise typer.BadParameter(f"must be an http(s) URL, got '{value}'")
+    return value
+
+
 def _send_webhook(url: str, payload: dict[str, object]) -> None:
     try:
         data = json_mod.dumps(payload).encode("utf-8")
@@ -1125,8 +1141,9 @@ def merge_all(
         str | None,
         typer.Option(
             "--notify",
-            help="Webhook URL to POST merge results to",
+            help="Webhook URL (http/https) to POST merge results to",
             envvar="PR_SPLIT_WEBHOOK_URL",
+            callback=_validate_webhook_url,
         ),
     ] = None,
 ) -> None:
diff --git a/tests/test_cli_coverage.py b/tests/test_cli_coverage.py
index 8d05885..0190f1d 100644
--- a/tests/test_cli_coverage.py
+++ b/tests/test_cli_coverage.py
@@ -7,6 +7,7 @@
 
 from __future__ import annotations
 
+import re
 from pathlib import Path
 from unittest.mock import MagicMock, patch
 
@@ -537,3 +538,33 @@ def test_status_no_plan(self, mock_pe: MagicMock) -> None:
     def test_clean_no_plan(self, mock_pe: MagicMock) -> None:
         result = runner.invoke(app, ["clean"])
         assert result.exit_code == 0
+
+
+def _flat(output: str) -> str:
+    # Rich draws the error in a box that wraps text (and, on CI, colours it
+    # with ANSI codes); strip both and collapse whitespace so a phrase can be
+    # matched regardless of wrapping.
+    plain = re.sub(r"\x1b\[[0-9;]*m", "", output)
+    return " ".join(re.sub(r"[│╭╮╰╯─]", " ", plain).split())
+
+
+class TestNotifyUrlValidation:
+    @pytest.mark.parametrize("bad", ["file:///tmp/wh.txt", "not-a-url", "ftp://x/y", "https://"])
+    def test_non_http_urls_are_rejected_before_anything_runs(self, bad: str) -> None:
+        with patch("pr_split.cli.plan_exists") as mock_pe:
+            result = runner.invoke(app, ["merge", "--notify", bad])
+        assert result.exit_code == 2
+        assert "must be an http(s) URL" in _flat(result.output)
+        mock_pe.assert_not_called()
+
+    def test_env_var_is_validated_too(self) -> None:
+        with patch("pr_split.cli.plan_exists") as mock_pe:
+            result = runner.invoke(app, ["merge"], env={"PR_SPLIT_WEBHOOK_URL": "file:///x"})
+        assert result.exit_code == 2
+        mock_pe.assert_not_called()
+
+    @patch("pr_split.cli.plan_exists", return_value=False)
+    def test_http_urls_pass(self, mock_pe: MagicMock) -> None:
+        result = runner.invoke(app, ["merge", "--notify", "https://hooks.example/abc"])
+        assert result.exit_code == 0
+        mock_pe.assert_called_once()