improve URL validation

test/backend-test/test-notification.js

@@ -110,40 +110,147 @@ test("Notification - Queue Management Test", async (t) => {
     assert.strictEqual(notification.queue.length, 2, "Queue should have 2 items");
 });
 
-test("Notification - URL Validation Test", async (t) => {
+test("Notification - URL Validation and Sanitization Test", async (t) => {
     const notification = new Notification();
 
-    // Test with various URL formats
+    // Test with various URL formats and edge cases
     const testCases = [
+        // Valid URLs
         {
             url: "https://test.mydomain.com",
             valid: true,
-            description: "Basic HTTPS URL"
+            description: "Basic HTTPS URL",
+            expectedOutput: "https://test.mydomain.com"
         },
         {
             url: "http://sub.test.mydomain.com",
             valid: true,
-            description: "Subdomain URL"
+            description: "Subdomain URL",
+            expectedOutput: "http://sub.test.mydomain.com"
         },
         {
             url: "https://test.mydomain.com/path",
             valid: true,
-            description: "URL with path"
+            description: "URL with path",
+            expectedOutput: "https://test.mydomain.com/path"
         },
         {
-            url: "https://malicious.test.mydomain.com/test.mydomain.com",
+            url: "https://test.mydomain.com:8080",
             valid: true,
-            description: "URL with misleading path"
+            description: "URL with port",
+            expectedOutput: "https://test.mydomain.com:8080"
         },
+        {
+            url: "https://test.mydomain.com/path?query=1",
+            valid: true,
+            description: "URL with query parameters",
+            expectedOutput: "https://test.mydomain.com/path?query=1"
+        },
+        {
+            url: "https://test.mydomain.com/path#fragment",
+            valid: true,
+            description: "URL with fragment",
+            expectedOutput: "https://test.mydomain.com/path#fragment"
+        },
+        {
+            url: "https://test.mydomain.com/special%20chars",
+            valid: true,
+            description: "URL with encoded characters",
+            expectedOutput: "https://test.mydomain.com/special%20chars"
+        },
+
+        // Potentially malicious URLs
         {
             url: "javascript:alert(1)",
             valid: false,
-            description: "JavaScript protocol"
+            description: "JavaScript protocol",
+            expectedOutput: ""
         },
         {
             url: "data:text/html,<script>alert(1)</script>",
             valid: false,
-            description: "Data URL"
+            description: "Data URL",
+            expectedOutput: ""
+        },
+        {
+            url: "file:///etc/passwd",
+            valid: false,
+            description: "File protocol",
+            expectedOutput: ""
+        },
+        {
+            url: "https://malicious.com?redirect=https://test.mydomain.com",
+            valid: true,
+            description: "URL with redirect parameter",
+            expectedOutput: "https://malicious.com?redirect=https://test.mydomain.com"
+        },
+        {
+            url: "https://malicious.com/https://test.mydomain.com",
+            valid: true,
+            description: "URL with embedded URL in path",
+            expectedOutput: "https://malicious.com/https://test.mydomain.com"
+        },
+        {
+            url: "https://test.mydomain.com@malicious.com",
+            valid: false,
+            description: "URL with @ character",
+            expectedOutput: ""
+        },
+        {
+            url: "https://malicious.com\\@test.mydomain.com",
+            valid: false,
+            description: "URL with escaped @ character",
+            expectedOutput: ""
+        },
+        {
+            url: "https:\\\\test.mydomain.com",
+            valid: false,
+            description: "URL with backslashes",
+            expectedOutput: ""
+        },
+        {
+            url: "https://test.mydomain.com/path/<script>alert(1)</script>",
+            valid: true,
+            description: "URL with XSS in path",
+            expectedOutput: "https://test.mydomain.com/path/<script>alert(1)</script>"
+        },
+
+        // Edge cases
+        {
+            url: "https://test.mydomain.com//double//slashes",
+            valid: true,
+            description: "URL with double slashes",
+            expectedOutput: "https://test.mydomain.com//double//slashes"
+        },
+        {
+            url: "https://test.mydomain.com/./path/../test",
+            valid: true,
+            description: "URL with dot segments",
+            expectedOutput: "https://test.mydomain.com/./path/../test"
+        },
+        {
+            url: "https://test.mydomain.com/%2e%2e%2f",
+            valid: true,
+            description: "URL with encoded dot segments",
+            expectedOutput: "https://test.mydomain.com/%2e%2e%2f"
+        },
+        {
+            url: "https://test.mydomain.com/\u0000",
+            valid: false,
+            description: "URL with null byte",
+            expectedOutput: ""
+        },
+        {
+            url: "https://test.mydomain.com/path with spaces",
+            valid: true,
+            description: "URL with unencoded spaces",
+            expectedOutput: "https://test.mydomain.com/path%20with%20spaces"
+        },
+        {
+            url: "https://xn--mnich-kva.example.com",
+            valid: true,
+            description: "Punycode URL",
+            expectedOutput: "https://xn--mnich-kva.example.com"
         }
     ];
 
@@ -159,11 +266,70 @@ test("Notification - URL Validation Test", async (t) => {
         };
 
         const formatted = notification.format(msg);
+
         if (testCase.valid) {
-            assert.ok(formatted.includes(testCase.url), `Should include ${testCase.description}`);
-        } else {
-            assert.ok(!formatted.includes(testCase.url), `Should not include ${testCase.description}`);
+            // For valid URLs, check if the URL is included exactly as expected
+            if (testCase.expectedOutput) {
+                assert.ok(
+                    formatted.includes(testCase.expectedOutput),
+                    `${testCase.description}: Should include exact URL ${testCase.expectedOutput}`
+                );
             }
+
+            // Check for potential URL substring issues
+            if (testCase.url.includes("test.mydomain.com")) {
+                const urlParts = testCase.url.split("test.mydomain.com");
+                if (urlParts.length > 1) {
+                    // Check that we don't have unintended URL substring matches
+                    const occurrences = formatted.split("test.mydomain.com").length - 1;
+                    assert.strictEqual(
+                        occurrences,
+                        1,
+                        `${testCase.description}: Should only include the URL once`
+                    );
+                }
+            }
+        } else {
+            // For invalid URLs, ensure they're not included at all
+            assert.ok(
+                !formatted.includes(testCase.url),
+                `${testCase.description}: Should not include invalid URL`
+            );
+
+            // For invalid URLs with potentially dangerous parts, ensure those parts are not present
+            if (testCase.url.includes("javascript:") ||
+                testCase.url.includes("data:") ||
+                testCase.url.includes("file:")) {
+                assert.ok(
+                    !formatted.includes("javascript:") &&
+                    !formatted.includes("data:") &&
+                    !formatted.includes("file:"),
+                    `${testCase.description}: Should not include dangerous protocols`
+                );
+            }
+        }
+
+        // Check for proper URL encoding
+        if (testCase.valid && testCase.url.includes(" ")) {
+            assert.ok(
+                !formatted.includes(" "),
+                `${testCase.description}: Spaces should be properly encoded`
+            );
+            assert.ok(
+                formatted.includes("%20"),
+                `${testCase.description}: Should use percent encoding for spaces`
+            );
+        }
+
+        // Additional security checks
+        assert.ok(
+            !formatted.includes("<script>"),
+            `${testCase.description}: Should not include unescaped script tags`
+        );
+        assert.ok(
+            !formatted.includes("javascript:"),
+            `${testCase.description}: Should not include javascript: protocol`
+        );
     }
 });