fix(security): validate service URLs with URL constructor (CodeQL #196)

[POWERFULMOVES]

Summary

• Replace regex URL validation with new URL() constructor + strict http:/https: protocol allowlist in chrome-extension/options/options.js (CodeQL Codex/add cloudflared service with makefile targets #196)
• Add validateServiceUrl() helper applied to both fetch() call sites — individual test button (line 121) and test-all button (line 162) (CodeRabbit follow-up)
• Invalid URLs now show "No URL" or "Invalid URL" status instead of attempting fetch with potentially malicious schemes

Test plan

• Load chrome extension options page
• Verify test-individual button rejects javascript: URLs
• Verify test-all button shows "Invalid URL" for malformed entries
• Verify valid http://localhost:8080 URLs still work correctly
• CodeQL rescan confirms Codex/add cloudflared service with makefile targets #196 auto-closure

Supersedes security portion of PR #866.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Added robust URL validation for all service URLs to prevent invalid entries from being processed.
  • Per-service and "Test All" tests now properly display "Invalid URL" status for malformed URLs instead of attempting to proceed.
  • Diagnostics link generation now validates URLs before creating links.

[coderabbitai]

📝 Walkthrough

Walkthrough

This change adds URL validation to the Chrome extension's options page. A new validateServiceUrl() helper function validates that service URLs use http or https schemes. This validation is applied to per-service test flows, batch test operations, and diagnostics link generation to reject invalid URLs early.

Changes

Cohort / File(s)	Summary
URL Validation in Options pmoves/chrome-extension/options/options.js	Introduced validateServiceUrl() helper function to validate service URLs against http/https schemes. Applied validation to per-service test flows (showing "Invalid URL" status on failure), batch test operations (pre-validating all URLs), and diagnostics link generation (using URL constructor for validation).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A hop through the URLs we go,
Validating schemes from high to low,
Http and https pass the test,
Invalid ones left with the rest,
Now service links are truly blessed! 🔗

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers the summary of changes and test plan, but is missing key required sections: Testing commands/output, Required Checks, and Review Coordination sections from the template.	Add the Testing section with actual commands/output, complete all Required Checks items, and fill in Review Coordination section before merging.
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: validating service URLs using the URL constructor to address CodeQL security issue #196.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/codeql-196-url-validation

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

pmoves/chrome-extension/options/options.js (2)

274-284: Correct security fix, but consider reusing validateServiceUrl.

The inline validation correctly prevents XSS by only assigning href for http/https URLs. However, this duplicates the logic from validateServiceUrl().

Two suggestions:

1. Refactor to reuse the helper (the trailing slash difference is negligible for full URLs):

♻️ Suggested refactor

-      // Validate URL via constructor to prevent XSS (CodeQL `#194`, `#196`)
-      try {
-        const parsed = new URL(vizUrl);
-        if (parsed.protocol === 'http:' || parsed.protocol === 'https:') {
-          link.href = parsed.href;
-        }
-      } catch { /* invalid URL — skip href assignment */ }
+      // Validate URL via constructor to prevent XSS (CodeQL `#194`, `#196`)
+      const validatedVizUrl = validateServiceUrl(vizUrl);
+      if (validatedVizUrl) {
+        link.href = validatedVizUrl;
+      }

2. UX improvement: When the URL is invalid, the "View SVG" link renders but clicking it does nothing. Consider hiding or styling the link differently when href is not set.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pmoves/chrome-extension/options/options.js` around lines 274 - 284, Refactor
the inline URL validation to reuse the existing validateServiceUrl(vizUrl)
helper instead of duplicating constructor/protocol checks: call
validateServiceUrl(vizUrl) to determine validity and, if valid, set link.href =
normalizedUrl (or vizUrl as returned); otherwise do not set href and alter the
link (link.hidden = true or add a disabled CSS class) so the "View SVG" anchor
is not shown or appears disabled when invalid; update references to
vizUrl/link/div in that block accordingly.

---

209-218: Solid implementation of URL validation.

The use of the URL constructor with a strict protocol allowlist is the correct approach to address CodeQL #196. The implementation correctly:

• Rejects invalid URLs via try/catch
• Restricts to http/https protocols only
• Returns a normalized, canonical URL

Scope consideration: Per the context snippets, background.js loads URLs from storage via loadConfig() and passes them directly to setServiceUrls() without validation. This means previously-stored malicious URLs (or URLs modified directly in storage) could still bypass this validation. The fix here is valuable for the options page UI path, but consider adding validation in pmoves-api.js or background.js as defense-in-depth.

,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pmoves/chrome-extension/options/options.js` around lines 209 - 218, The
options-only validation leaves previously stored or externally-modified URLs
unvalidated; update the background loading and API call paths to validate again:
in background.js modify loadConfig() to run each stored URL through
validateServiceUrl() and only pass non-null results to setServiceUrls(), and add
the same guard in pmoves-api.js where service URLs are consumed (validate before
use and reject/ignore invalid entries), ensuring validateServiceUrl() is the
single source of truth for canonicalization and rejection.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pmoves/chrome-extension/options/options.js`:
- Around line 274-284: Refactor the inline URL validation to reuse the existing
validateServiceUrl(vizUrl) helper instead of duplicating constructor/protocol
checks: call validateServiceUrl(vizUrl) to determine validity and, if valid, set
link.href = normalizedUrl (or vizUrl as returned); otherwise do not set href and
alter the link (link.hidden = true or add a disabled CSS class) so the "View
SVG" anchor is not shown or appears disabled when invalid; update references to
vizUrl/link/div in that block accordingly.
- Around line 209-218: The options-only validation leaves previously stored or
externally-modified URLs unvalidated; update the background loading and API call
paths to validate again: in background.js modify loadConfig() to run each stored
URL through validateServiceUrl() and only pass non-null results to
setServiceUrls(), and add the same guard in pmoves-api.js where service URLs are
consumed (validate before use and reject/ignore invalid entries), ensuring
validateServiceUrl() is the single source of truth for canonicalization and
rejection.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 634a2096-7a02-4d52-b8c0-36c0b939d0ea

📥 Commits

Reviewing files that changed from the base of the PR and between 65c3efd and cfa30fb.

📒 Files selected for processing (1)

• pmoves/chrome-extension/options/options.js