fix: implement robust PR URL validation and sanitization

migrations/0004_add_canonical_url_and_composite_uniqueness.sql

@@ -0,0 +1,23 @@
+-- Migration: Add canonical_url metadata and composite uniqueness for PR identity
+-- Created: 2026-03-25
+-- Description:
+--   1) Adds canonical_url column for normalized PR URL storage
+--   2) Backfills canonical_url for existing rows
+--   3) Removes duplicate rows by (repo_owner, repo_name, pr_number), keeping latest id
+--   4) Enforces uniqueness on (repo_owner, repo_name, pr_number)
+
+ALTER TABLE prs ADD COLUMN canonical_url TEXT;
+
+UPDATE prs
+SET canonical_url = 'https://github.com/' || repo_owner || '/' || repo_name || '/pull/' || pr_number
+WHERE canonical_url IS NULL OR canonical_url = '';
+
+DELETE FROM prs
+WHERE id NOT IN (
+    SELECT MAX(id)
+    FROM prs
+    GROUP BY repo_owner, repo_name, pr_number
+);
+
+CREATE UNIQUE INDEX IF NOT EXISTS idx_prs_identity_unique
+ON prs(repo_owner, repo_name, pr_number);

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"
@@ -1266,6 +1267,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 +3275,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 +3310,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 +3332,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 +3350,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 +3361,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 +3463,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/database.py

@@ -261,14 +261,17 @@ async def delete_readiness_from_db(env, pr_id):
 async def upsert_pr(db, pr_url, owner, repo, pr_number, pr_data):
     """Helper to insert or update PR in database (Deduplicates logic)"""
     current_timestamp = datetime.now(timezone.utc).isoformat().replace('+00:00', 'Z')
+    canonical_url = f'https://github.com/{owner}/{repo}/pull/{pr_number}'
 
     stmt = db.prepare('''
-        INSERT INTO prs (pr_url, repo_owner, repo_name, pr_number, title, state, 
+        INSERT INTO prs (pr_url, canonical_url, repo_owner, repo_name, pr_number, title, state, 
                        is_merged, mergeable_state, files_changed, author_login,                        author_avatar, repo_owner_avatar, checks_passed, checks_failed, checks_skipped, 
                        commits_count, behind_by, review_status, last_updated_at, 
                        last_refreshed_at, updated_at, is_draft, open_conversations_count, reviewers_json, etag)
-        VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
-        ON CONFLICT(pr_url) DO UPDATE SET
+        VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+        ON CONFLICT(repo_owner, repo_name, pr_number) DO UPDATE SET
+            pr_url = excluded.pr_url,
+            canonical_url = excluded.canonical_url,
             title = excluded.title,
             state = excluded.state,
             is_merged = excluded.is_merged,
@@ -289,7 +292,7 @@ async def upsert_pr(db, pr_url, owner, repo, pr_number, pr_data):
             reviewers_json = excluded.reviewers_json,
             etag = excluded.etag
     ''').bind(
-        pr_url, owner, repo, pr_number,
+        pr_url, canonical_url, owner, repo, pr_number,
         pr_data.get('title') or '',
         pr_data.get('state') or '',
         1 if pr_data.get('is_merged') else 0,