fix: implement robust PR URL validation and sanitization

public/index.html

@@ -568,6 +568,7 @@ <h2 class="mb-3 text-xs font-semibold uppercase tracking-[0.12em] text-slate-500
                         Add PR
                     </button>
                 </div>
+                <span id="url-error" style="color: #ff4d4d; font-size: 0.85em; display: none; margin-top: 5px;"></span>
 
                 <div class="mt-2 flex items-center gap-2">
                     <input type="checkbox" id="addAllPrsCheckbox"
@@ -630,6 +631,59 @@ <h2 class="text-xl font-semibold text-slate-900 dark:text-slate-100">Raw PR Data
             user: null
         };
         let repoSortByPrs = localStorage.getItem('repoSortByPrs') !== 'false';
+        /**
+ * Validates that a string is a well-formed GitHub PR URL.
+ * Accepts:
+ *   https://github.com/owner/repo/pull/123
+ *   https://github.com/owner/repo/pull/123/files  (common variant)
+ * Returns { valid: true, owner, repo, number } or { valid: false, reason }
+ */
+function parseGitHubPRUrl(raw) {
+  let url;
+  try {
+    url = new URL(raw.trim());
+  } catch {
+    return { valid: false, reason: 'That doesn\'t look like a URL. Please paste the full PR link.' };
+  }
+
+  if (url.hostname !== 'github.com') {
+    return { valid: false, reason: 'Only github.com PRs are supported.' };
+  }
+
+  // Path must be:  /owner/repo/pull/NUMBER  (optionally /files etc.)
+  const PR_PATH_RE = /^\/([^/]+)\/([^/]+)\/pull\/(\d+)(\/.*)?$/;
+  const match = url.pathname.match(PR_PATH_RE);
+
+  if (!match) {
+    return {
+      valid: false,
+      reason: 'URL must point to a Pull Request, e.g. https://github.com/owner/repo/pull/42',
+    };
+  }
+
+  const prNumber = parseInt(match[3], 10);
+  if (prNumber < 1 || prNumber > 999999) {
+    return { valid: false, reason: 'PR number looks invalid.' };
+  }
+
+  return { valid: true, owner: match[1], repo: match[2], number: prNumber };
+}
+
+// Attach to button
+document.getElementById('addPrBtn').addEventListener('click', () => {
+  const raw = document.getElementById('prUrlInput').value;
+  const errEl = document.getElementById('url-error');
+  errEl.textContent = '';
+
+  const result = parseGitHubPRUrl(raw);
+  if (!result.valid) {
+    errEl.textContent = result.reason;   // inline error, no alert()
+    document.getElementById('prUrlInput').focus();
+    return;
+  }
+
+  checkPR(raw);  // passes only after validation
+});
         // Support unlimited sort columns
         // Each entry in sortColumns array has: {column: 'column_name', direction: 'asc'|'desc'}
         let sortColumns = [];

src/handlers.py

@@ -237,9 +237,13 @@ async def handle_add_pr(request, env):
             # Catch ValueError from parse_pr_url
             try:
                 parsed = parse_pr_url(pr_url)
+                # Extract the validated data from the dictionary
+                owner = parsed['owner']
+                repo = parsed['repo']
+                pr_num = parsed['pr_number']
             except ValueError as e:
                 return Response.new(
-                    json.dumps({'error': str(e)}),
+                    json.dumps({'ok': False, 'error': str(e)}),
                     {'status': 400, 'headers': {'Content-Type': 'application/json'}}
                 )
 

src/utils.py

@@ -1,6 +1,7 @@
 """Utility functions for PR parsing and analysis"""
 
 import re
+import json
 from datetime import datetime, timezone
 
 # Score multiplier when changes are requested
@@ -12,36 +13,45 @@
 _MERGE_CONFLICTS_SCORE_MULTIPLIER = 0.67
 
 
-def parse_pr_url(pr_url):
+# 1. Place this regex at the top of the file (outside the function)
+_GITHUB_PR_RE = re.compile(
+    r'^https://github\.com/([a-zA-Z0-9_.-]+)/([a-zA-Z0-9_.-]+)/pull/(\d+)(/.*)?$'
+)
+
+# 2. Replace the old parse_pr_url with this one
+def parse_pr_url(pr_url: str):
     """
-    Parse GitHub PR URL to extract owner, repo, and PR number.
-
-    Security Hardening (Issue #45):
-    - Type validation to prevent type confusion attacks
-    - Anchored regex pattern to block malformed URLs with trailing junk
-    - Raises ValueError instead of returning None for better error handling
+    Validates and parses a GitHub PR URL.
+    Returns a dict with owner, repo, and pr_number or raises ValueError.
     """
-    # FIX Issue #45: Type validation
-    if not isinstance(pr_url, str):
-        raise ValueError("PR URL must be a string")
-
-    if not pr_url:
-        raise ValueError("PR URL is required")
-
-    pr_url = pr_url.strip().rstrip('/')
-
-    # FIX Issue #45: Anchored regex - must match EXACTLY, no trailing junk allowed
-    pattern = r'^https?://github\.com/([^/]+)/([^/]+)/pull/(\d+)$'
-    match = re.match(pattern, pr_url)
-
+    # Type and presence validation
+    if not pr_url or not isinstance(pr_url, str):
+        raise ValueError("PR URL is required and must be a string.")
+
+    raw_url = pr_url.strip()
+
+    # Security: Prevent ReDoS or overflow attacks with length limit
+    if len(raw_url) > 300:
+        raise ValueError("URL is too long.")
+
+    match = _GITHUB_PR_RE.match(raw_url)
     if not match:
-        # FIX Issue #45: Raise error instead of returning None
-        raise ValueError("Invalid GitHub PR URL. Format: https://github.com/OWNER/REPO/pull/NUMBER")
-
+        raise ValueError(
+            "Invalid GitHub PR URL. Expected format: "
+            "https://github.com/owner/repo/pull/123"
+        )
+
+    owner, repo, pr_num_str = match.group(1), match.group(2), match.group(3)
+    pr_number = int(pr_num_str)
+
+    if pr_number < 1:
+        raise ValueError("PR number must be a positive integer.")
+
+    # Return as a dictionary to match the original function's signature
     return {
-        'owner': match.group(1),
-        'repo': match.group(2),
-        'pr_number': int(match.group(3))
+        'owner': owner,
+        'repo': repo,
+        'pr_number': pr_number
     }