fix: implement robust PR URL validation and sanitization

package.json

@@ -11,8 +11,9 @@
     "db:migrations:list": "wrangler d1 migrations list pr_tracker",
     "lint": "echo 'Linting not configured yet' && exit 0",
     "format:check": "echo 'Format checking not configured yet' && exit 0",
-    "test": "node test-data-display.js",
-    "test:data-display": "node test-data-display.js"
+    "test": "node test-data-display.js && node test-url-validation.js",
+    "test:data-display": "node test-data-display.js",
+    "test:url-validation": "node test-url-validation.js"
   },
   "keywords": [
     "github",

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" class="hidden text-sm text-red-700 dark:text-red-400"></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 = [];
@@ -1266,6 +1320,116 @@ <h3 class="text-lg font-semibold text-slate-900 dark:text-slate-100 mb-2">Error<
             }, FEEDBACK_DISMISS_DELAY);
         }
 
+        const PR_PATH_RE = /^\/([^/]+)\/([^/]+)\/pull\/(\d+)\/?$/i;
+        const REPO_PATH_RE = /^\/([^/]+)\/([^/]+)\/?$/i;
+        const ORGS_PATH_RE = /^\/orgs\/([^/]+)\/?$/i;
+        const ORG_PATH_RE = /^\/([^/]+)\/?$/i;
+        const RESERVED_OWNER_NAMES = new Set([
+            'about', 'codespaces', 'collections', 'discussions', 'enterprise',
+            'explore', 'features', 'issues', 'login', 'marketplace', 'new',
+            'notifications', 'organizations', 'orgs', 'pricing', 'pulls',
+            'security', 'settings', 'signup', 'sponsors', 'topics', 'trending'
+        ]);
+
+        function setUrlError(message) {
+            const el = document.getElementById('url-error');
+            if (!el) return;
+            if (message) {
+                el.textContent = message;
+                el.classList.remove('hidden');
+            } else {
+                el.textContent = '';
+                el.classList.add('hidden');
+            }
+        }
+
+        function parseGitHubTrackingUrl(rawUrl) {
+            const value = typeof rawUrl === 'string' ? rawUrl.trim() : '';
+            if (!value) {
+                return { valid: false, reason: 'Please enter a PR URL' };
+            }
+            if (value.length > 2048) {
+                return { valid: false, reason: 'URL is too long' };
+            }
+
+            let url;
+            try {
+                url = new URL(value);
+            } catch {
+                return { valid: false, reason: 'Please enter a valid URL' };
+            }
+
+            const protocol = url.protocol.toLowerCase();
+            if (protocol !== 'http:' && protocol !== 'https:') {
+                return { valid: false, reason: 'URL must start with http:// or https://' };
+            }
+
+            const hostname = url.hostname.toLowerCase();
+            if (hostname !== 'github.com' && hostname !== 'www.github.com') {
+                return { valid: false, reason: 'Only github.com URLs are allowed' };
+            }
+
+            const cleanPath = url.pathname.replace(/\/+$/, '') || '/';
+            let match = cleanPath.match(PR_PATH_RE);
+            if (match) {
+                const owner = match[1];
+                const repo = match[2];
+                const prNumber = parseInt(match[3], 10);
+                return {
+                    valid: true,
+                    type: 'pr',
+                    owner,
+                    repo,
+                    prNumber,
+                    canonicalUrl: `https://github.com/${owner}/${repo}/pull/${prNumber}`
+                };
+            }
+
+            match = cleanPath.match(REPO_PATH_RE);
+            if (match) {
+                const owner = match[1];
+                const repo = match[2];
+                if (owner.toLowerCase() !== 'orgs' && repo.toLowerCase() !== 'pull') {
+                    return {
+                        valid: true,
+                        type: 'repo',
+                        owner,
+                        repo,
+                        canonicalUrl: `https://github.com/${owner}/${repo}`
+                    };
+                }
+            }
+
+            match = cleanPath.match(ORGS_PATH_RE);
+            if (match) {
+                const owner = match[1];
+                return {
+                    valid: true,
+                    type: 'org',
+                    owner,
+                    canonicalUrl: `https://github.com/${owner}`
+                };
+            }
+
+            match = cleanPath.match(ORG_PATH_RE);
+            if (match) {
+                const owner = match[1];
+                if (!RESERVED_OWNER_NAMES.has(owner.toLowerCase())) {
+                    return {
+                        valid: true,
+                        type: 'org',
+                        owner,
+                        canonicalUrl: `https://github.com/${owner}`
+                    };
+                }
+            }
+
+            return {
+                valid: false,
+                reason: 'Use a GitHub PR, repository, or organization URL'
+            };
+        }
+
         function showSectionMessage(message, type) {
             // Show an inline feedback message in the Add PR section on the page
             const section = document.getElementById('prUrlInput')?.closest('section');
@@ -3164,16 +3328,28 @@ <h3 class="text-xl font-semibold text-slate-900 dark:text-slate-100">${emptyTitl
             const checkbox = document.getElementById('addAllPrsCheckbox');
             const prUrl = input.value.trim();
             const addAll = checkbox.checked;
+            const parsedUrl = parseGitHubTrackingUrl(prUrl);
 
-            if (!prUrl) {
-                showInputError('prUrlInput', 'Please enter a PR URL');
+            if (!parsedUrl.valid) {
+                setUrlError(parsedUrl.reason);
+                input.focus();
                 return;
             }
 
-            // Auto-detect if URL is an org or repo URL (not a PR URL)
-            const isOrgOrRepoUrl = prUrl.match(/^https?:\/\/github\.com\/[^/]+\/?$/) || 
-                                   (prUrl.match(/^https?:\/\/github\.com\/[^/]+\/[^/]+\/?$/) && !prUrl.includes('/pull/'));
-            const effectiveAddAll = addAll || !!isOrgOrRepoUrl;
+            const effectiveAddAll = addAll || parsedUrl.type !== 'pr';
+            if (!effectiveAddAll && parsedUrl.type !== 'pr') {
+                setUrlError('Use a pull request URL when bulk import is not enabled');
+                input.focus();
+                return;
+            }
+            if (addAll && parsedUrl.type === 'pr') {
+                setUrlError('For bulk import, enter a repository or organization URL');
+                input.focus();
+                return;
+            }
+
+            setUrlError('');
+            const canonicalInputUrl = parsedUrl.canonicalUrl;
 
             btn.disabled = true;
             btn.textContent = effectiveAddAll ? 'Importing...' : 'Adding...';
@@ -3187,16 +3363,18 @@ <h3 class="text-xl font-semibold text-slate-900 dark:text-slate-100">${emptyTitl
                         'Content-Type': 'application/json'
                     },
                     body: JSON.stringify({
-                        pr_url: prUrl,
-                        add_all: addAll
+                        pr_url: canonicalInputUrl,
+                        add_all: effectiveAddAll
                     })
                 });
 
                 const data = await response.json();
                 if (data.error) {
-                    showInputError('prUrlInput', data.error);
+                    setUrlError(data.error);
+                    input.focus();
                 } else {
                     input.value = '';
+                    setUrlError('');
                     // Show success message for bulk imports
                     if (data.imported_count !== undefined) {
                         if (data.truncated) {
@@ -3207,11 +3385,9 @@ <h3 class="text-xl font-semibold text-slate-900 dark:text-slate-100">${emptyTitl
                     }
                     invalidateApiCache();
                     // For single PR additions, select the org/repo and highlight the added PR
-                    const prUrlPattern = /^https?:\/\/github\.com\/([a-zA-Z0-9._-]+)\/([a-zA-Z0-9._-]+)\/pull\/\d+/;
-                    const prMatch = !effectiveAddAll && prUrl.match(prUrlPattern);
-                    if (prMatch) {
-                        const prOwner = prMatch[1];
-                        const prRepoKey = `${prMatch[1]}/${prMatch[2]}`;
+                    if (!effectiveAddAll && parsedUrl.type === 'pr') {
+                        const prOwner = parsedUrl.owner;
+                        const prRepoKey = `${parsedUrl.owner}/${parsedUrl.repo}`;
                         // Update org filter to highlight the PR's owner/org
                         currentOrg = prOwner;
                         localStorage.setItem('currentOrg', currentOrg);
@@ -3227,7 +3403,7 @@ <h3 class="text-xl font-semibold text-slate-900 dark:text-slate-100">${emptyTitl
                         await loadPrs(true);
                         await loadRateLimit(true);
                         // Highlight the newly added PR row
-                        const added = allPrs.find(pr => pr.pr_url === prUrl);
+                        const added = allPrs.find(pr => pr.pr_url === canonicalInputUrl);
                         if (added) highlightPrRow(added.id);
                     } else {
                         // Force bypass service worker cache after import
@@ -3238,7 +3414,8 @@ <h3 class="text-xl font-semibold text-slate-900 dark:text-slate-100">${emptyTitl
                 }
             } catch (error) {
                 console.error('Error adding PR:', error);
-                showInputError('prUrlInput', 'Failed to add PR');
+                setUrlError('Failed to add PR');
+                input.focus();
             } finally {
                 btn.disabled = false;
                 btn.textContent = 'Add PR';
@@ -3339,6 +3516,9 @@ <h3 class="text-xl font-semibold text-slate-900 dark:text-slate-100">${emptyTitl
         });
 
         document.getElementById('addPrBtn').addEventListener('click', addPr);
+        document.getElementById('prUrlInput').addEventListener('input', () => {
+            setUrlError('');
+        });
         document.getElementById('prUrlInput').addEventListener('keypress', (e) => {
             if (e.key === 'Enter') addPr();
         });

src/handlers.py

@@ -85,24 +85,32 @@ async def handle_add_pr(request, env):
 
         db = get_db(env)
 
+        normalized_pr_url = pr_url.strip()
+
         # Auto-detect URL type: if it's an org or repo URL (not a PR URL),
         # automatically treat as bulk import regardless of checkbox state
         if not add_all:
-            # Check if the URL is an org URL or a repo URL (not a PR URL)
-            if parse_org_url(pr_url) or (parse_repo_url(pr_url) and '/pull/' not in pr_url):
+            is_pr_url = False
+            try:
+                parse_pr_url(normalized_pr_url)
+                is_pr_url = True
+            except ValueError:
+                is_pr_url = False
+
+            if not is_pr_url and (parse_org_url(normalized_pr_url) or parse_repo_url(normalized_pr_url)):
                 add_all = True
 
         if add_all:
             org_owner = ''
             # Try parsing as a repo URL first (e.g. https://github.com/owner/repo)
-            parsed = parse_repo_url(pr_url)
+            parsed = parse_repo_url(normalized_pr_url)
             if parsed:
                 # Single repo import
                 repos_to_import = [{'owner': parsed['owner'], 'name': parsed['repo']}]
                 is_org_import = False
             else:
                 # Parse org url
-                org_parsed = parse_org_url(pr_url)
+                org_parsed = parse_org_url(normalized_pr_url)
                 if not org_parsed:
                     return Response.new(json.dumps({'error': 'Invalid GitHub Repository or Organization URL'}), 
                                       {'status': 400, 'headers': {'Content-Type': 'application/json'}})
@@ -236,7 +244,7 @@ async def handle_add_pr(request, env):
             # Add single pr
             # Catch ValueError from parse_pr_url
             try:
-                parsed = parse_pr_url(pr_url)
+                parsed = parse_pr_url(normalized_pr_url)
             except ValueError as e:
                 return Response.new(
                     json.dumps({'error': str(e)}),
@@ -259,27 +267,28 @@ async def handle_add_pr(request, env):
                 return Response.new(json.dumps({'error': 'Cannot add merged/closed PRs'}), 
                                   {'status': 400, 'headers': {'Content-Type': 'application/json'}})
 
-            await upsert_pr(db, pr_url, parsed['owner'], parsed['repo'], parsed['pr_number'], pr_data)
+            canonical_pr_url = parsed['canonical_url']
+            await upsert_pr(db, canonical_pr_url, parsed['owner'], parsed['repo'], parsed['pr_number'], pr_data)
 
             # Auto-run readiness analysis for the newly added PR
             readiness_data = None
             try:
                 pr_result = await db.prepare(
                     'SELECT * FROM prs WHERE pr_url = ?'
-                ).bind(pr_url).first()
+                ).bind(canonical_pr_url).first()
                 if pr_result:
                     pr_row = pr_result.to_py()
                     readiness_data = await _run_readiness_analysis(env, pr_row, pr_row['id'], user_token)
             except Exception as analysis_err:
-                print(f"Auto-analysis failed for PR {pr_url}: {type(analysis_err).__name__}: {str(analysis_err)}")
+                print(f"Auto-analysis failed for PR {canonical_pr_url}: {type(analysis_err).__name__}: {str(analysis_err)}")
 
             # Include repo_owner, repo_name, pr_number, and pr_url in the response for frontend display
             response_data = {
                 **pr_data,
                 'repo_owner': parsed['owner'],
                 'repo_name': parsed['repo'],
                 'pr_number': parsed['pr_number'],
-                'pr_url': pr_url
+                'pr_url': canonical_pr_url
             }
 
             result_obj = {'success': True, 'data': response_data}