Port normalization and null-safety check

Author: steven10a

src/__tests__/unit/checks/keywords-urls.test.ts

@@ -384,6 +384,44 @@ describe('urls guardrail', () => {
     expect(result.info?.blocked).toContain('help-suntropy.es');
     expect(result.info?.blocked).toContain('help.suntropy.es');
   });
+
+  it('treats explicit default ports as equivalent to no port', async () => {
+    // URLs with explicit default ports should match allow list entries without ports
+    const config = {
+      url_allow_list: ['example.com'],
+      allowed_schemes: new Set(['https', 'http']),
+      allow_subdomains: false,
+      block_userinfo: true,
+    };
+
+    const text = 'Visit https://example.com:443 and http://example.com:80';
+    const result = await urls({}, text, config);
+
+    // Both should be allowed (443 is default for https, 80 is default for http)
+    expect(result.tripwireTriggered).toBe(false);
+    expect(result.info?.allowed).toContain('https://example.com:443');
+    expect(result.info?.allowed).toContain('http://example.com:80');
+    expect(result.info?.blocked).toEqual([]);
+  });
+
+  it('blocks non-default ports when allow list has no port', async () => {
+    // URLs with non-default ports should be blocked when allow list doesn't specify a port
+    const config = {
+      url_allow_list: ['example.com'],
+      allowed_schemes: new Set(['https']),
+      allow_subdomains: false,
+      block_userinfo: true,
+    };
+
+    const text = 'Visit https://example.com:8443 and https://example.com:9000';
+    const result = await urls({}, text, config);
+
+    // Both should be allowed - when allow list has no port, any port is OK
+    // (This matches the behavior: no port restriction when not specified)
+    expect(result.tripwireTriggered).toBe(false);
+    expect(result.info?.allowed).toContain('https://example.com:8443');
+    expect(result.info?.allowed).toContain('https://example.com:9000');
+  });
 });
 
 describe('competitors guardrail', () => {

src/checks/urls.ts

@@ -390,7 +390,7 @@ function isUrlAllowed(parsedUrl: URL, allowList: string[], allowSubdomains: bool
 
     const allowedScheme = hasExplicitScheme ? parsedAllowed.protocol.replace(/:$/, '').toLowerCase() : '';
     const allowedPort = safeGetPort(parsedAllowed, allowedScheme);
-    const allowIndicatesPort = parsedAllowed.host.includes(':') && !parsedAllowed.host.startsWith('[');
+    const allowIndicatesPort = Boolean(parsedAllowed.host) && parsedAllowed.host.includes(':') && !parsedAllowed.host.startsWith('[');
     if (allowedPort === null && allowIndicatesPort) {
       continue;
     }
@@ -410,10 +410,17 @@ function isUrlAllowed(parsedUrl: URL, allowList: string[], allowSubdomains: bool
         continue;
       }
 
-      // Port matching: only enforce when allow list entry explicitly specifies a port
-      // Check parsedAllowed.port (empty string when no port specified) not allowedPort (always has default)
-      if (parsedAllowed.port) {
-        if (urlPort === null || allowedPort !== urlPort) {
+      // Port matching: only enforce when allow list entry explicitly specifies a non-default port
+      // Explicit default ports (e.g., :443 for https) should be treated as no port specified
+      const allowedHasNonDefaultPort = parsedAllowed.port && 
+        (allowedPort !== DEFAULT_PORTS[allowedScheme as keyof typeof DEFAULT_PORTS]);
+      
+      if (allowedHasNonDefaultPort) {
+        // Allow list has explicit non-default port, so URL must match exactly
+        const urlHasNonDefaultPort = parsedUrl.port && 
+          (urlPort !== DEFAULT_PORTS[schemeLower as keyof typeof DEFAULT_PORTS]);
+        
+        if (!urlHasNonDefaultPort || allowedPort !== urlPort) {
           continue;
         }
       }
@@ -448,10 +455,17 @@ function isUrlAllowed(parsedUrl: URL, allowList: string[], allowSubdomains: bool
 
     const allowedDomain = allowedHost.replace(/^www\./, '');
 
-    // Port matching: only enforce when allow list entry explicitly specifies a port
-    // Check parsedAllowed.port (empty string when no port specified) not allowedPort (always has default)
-    if (parsedAllowed.port) {
-      if (urlPort === null || allowedPort !== urlPort) {
+    // Port matching: only enforce when allow list entry explicitly specifies a non-default port
+    // Explicit default ports (e.g., :443 for https) should be treated as no port specified
+    const allowedHasNonDefaultPort = parsedAllowed.port && 
+      (allowedPort !== DEFAULT_PORTS[allowedScheme as keyof typeof DEFAULT_PORTS]);
+    
+    if (allowedHasNonDefaultPort) {
+      // Allow list has explicit non-default port, so URL must match exactly
+      const urlHasNonDefaultPort = parsedUrl.port && 
+        (urlPort !== DEFAULT_PORTS[schemeLower as keyof typeof DEFAULT_PORTS]);
+      
+      if (!urlHasNonDefaultPort || allowedPort !== urlPort) {
         continue;
       }
     }