Potential fix for code scanning alert no. 9: Client-side cross-site scripting

[EthanThePhoenix38]

Potential fix for https://github.com/ThePhoenixAgency/AI-Pulse/security/code-scanning/9

In general, to fix this kind of issue you must not use raw user-controlled URLs directly in navigation-related attributes (href, src) without strong validation. Beyond checking that the scheme is http or https, you should restrict the destination to a safe allowlist (known domains or patterns) or otherwise strictly validate its structure and origin before using it. This avoids malicious links being injected by simply sharing a crafted URL to your reader page.

For this specific code, the minimal change that improves security without altering existing behavior for valid uses is:

1. Introduce an isAllowedArticleUrl function that:

   • Reuses the existing isValidHttpUrl check.
   • Parses the URL and enforces one or more constraints, such as:
     • Only allow URLs whose hostname is in an allowlist (e.g. same origin as the reader, or a known set of domains), or
     • At least restrict extremely suspicious values (e.g. blank host).
   • Returns false if the URL does not meet these constraints.

2. Use isAllowedArticleUrl(articleUrl) in place of isValidHttpUrl(articleUrl) in the if condition on line 460. If the URL is not allowed, skip setting href/src and do not open the article view.

3. Keep using DOMPurify for the other parameters as already done; no change needed there.

Since we must not assume project-wide context, we’ll implement a conservative allowlist of hostnames using window.location.hostname as the only allowed host. This ensures the reader only opens content from the same host by default, which is a safe and common pattern. If your real app needs more domains, you can extend the allowlist array later without further code restructuring.

All changes are confined to reader.html in the shown script section: we will add the new helper function just after isValidHttpUrl and adjust the condition that gates use of articleUrl.

---

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

[Copilot]

Pull request overview

This pull request addresses a code scanning alert for client-side cross-site scripting (XSS) by implementing stricter URL validation for article loading. The fix adds a new validation function that restricts article URLs to same-host only, replacing the previous validation that only checked for HTTP/HTTPS protocols.

Key changes:

• Introduces isAllowedArticleUrl() function that validates URLs against an allowlist of hostnames
• Restricts article URLs to same hostname as the reader page (window.location.hostname)
• Updates the article loading condition to use the new stricter validation

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The parameter name "string" is not descriptive. Consider using a more meaningful name like "url" or "urlString" to improve code readability and consistency with the existing isValidHttpUrl function.

[Copilot]

Restricting article URLs to only same-host (window.location.hostname) will break the core functionality of this reader page, which appears designed to load articles from external sources via URL parameters. This is evidenced by the iframe loading external content (line 511), external link buttons (lines 503-504), and the general purpose of a reader page. The security fix is too restrictive and will prevent legitimate use cases. Consider either: 1) implementing a configurable allowlist of trusted domains, 2) using a more permissive validation that still prevents XSS attacks (e.g., ensuring proper URL structure and protocol while allowing external domains), or 3) documenting that this change fundamentally alters the application's functionality.

Suggested change

	// Allow only same-host articles by default. Extend this list if needed.
	const allowedHosts = [window.location.hostname];
	return allowedHosts.includes(url.hostname);
	// Optional: define window.ALLOWED_ARTICLE_HOSTS = ['example.com', 'another.com'] elsewhere
	// If an allowlist is provided and non-empty, enforce it; otherwise allow any valid HTTP(S) URL.
	const globalAllowlist = Array.isArray(window.ALLOWED_ARTICLE_HOSTS) ? window.ALLOWED_ARTICLE_HOSTS : null;
	if (globalAllowlist && globalAllowlist.length > 0) {
	const hostname = url.hostname.toLowerCase();
	return globalAllowlist
	.map(function (h) { return String(h).toLowerCase(); })
	.includes(hostname);
	}
	return true;

[Copilot]

The URL is being parsed twice - once in isValidHttpUrl (line 452) and again in isAllowedArticleUrl (line 465). While this is safe, it's inefficient. Since isAllowedArticleUrl already calls isValidHttpUrl, and then immediately parses the URL again in a try-catch block, the second try-catch is redundant. The URL has already been validated as parseable by isValidHttpUrl, so the try-catch at line 464 will never catch an error from the URL constructor. Consider restructuring to parse the URL once and perform all validations on that parsed object.