Better CIDR handling

Author: steven10a

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

@@ -422,6 +422,58 @@ describe('urls guardrail', () => {
     expect(result.info?.allowed).toContain('https://example.com:8443');
     expect(result.info?.allowed).toContain('https://example.com:9000');
   });
+
+  it('accepts CIDR /0 with 0.0.0.0 network address', async () => {
+    // 0.0.0.0/0 should match all IPs
+    const config = {
+      url_allow_list: ['0.0.0.0/0'],
+      allowed_schemes: new Set(['https']),
+      allow_subdomains: false,
+      block_userinfo: true,
+    };
+
+    const text = 'Visit https://1.2.3.4 and https://192.168.1.1';
+    const result = await urls({}, text, config);
+
+    expect(result.tripwireTriggered).toBe(false);
+    expect(result.info?.allowed).toContain('https://1.2.3.4');
+    expect(result.info?.allowed).toContain('https://192.168.1.1');
+  });
+
+  it('rejects CIDR /0 with non-zero network address', async () => {
+    // 10.0.0.0/0 is ambiguous - /0 should only use 0.0.0.0
+    const config = {
+      url_allow_list: ['10.0.0.0/0'],
+      allowed_schemes: new Set(['https']),
+      allow_subdomains: false,
+      block_userinfo: true,
+    };
+
+    const text = 'Visit https://10.5.5.5 and https://192.168.1.1';
+    const result = await urls({}, text, config);
+
+    // Should block both because 10.0.0.0/0 is invalid (emits warning)
+    expect(result.tripwireTriggered).toBe(true);
+    expect(result.info?.blocked).toContain('https://10.5.5.5');
+    expect(result.info?.blocked).toContain('https://192.168.1.1');
+  });
+
+  it('rejects invalid CIDR prefix values', async () => {
+    // Test various invalid CIDR prefixes
+    const config = {
+      url_allow_list: ['10.0.0.0/33', '192.168.0.0/-1', '172.16.0.0/abc'],
+      allowed_schemes: new Set(['https']),
+      allow_subdomains: false,
+      block_userinfo: true,
+    };
+
+    const text = 'Visit https://10.5.5.5 and https://192.168.1.1 and https://172.16.1.1';
+    const result = await urls({}, text, config);
+
+    // All should be blocked due to invalid CIDR configurations
+    expect(result.tripwireTriggered).toBe(true);
+    expect(result.info?.blocked).toHaveLength(3);
+  });
 });
 
 describe('competitors guardrail', () => {

src/checks/urls.ts

@@ -407,6 +407,47 @@ function isUrlAllowed(parsedUrl: URL, allowList: string[], allowSubdomains: bool
       continue;
     }
 
+    // Handle CIDR notation before URL parsing
+    // CIDR blocks like "10.0.0.0/8" should not be parsed as URLs
+    const cidrMatch = normalizedEntry.match(/^(\d+\.\d+\.\d+\.\d+)\/(\d+)$/);
+    if (cidrMatch) {
+      // Only match against IP URLs
+      if (!urlIsIp || urlIpInt === null) {
+        continue;
+      }
+
+      const [, network, prefixStr] = cidrMatch;
+      const prefix = Number(prefixStr);
+
+      if (!Number.isInteger(prefix) || prefix < 0 || prefix > 32) {
+        console.warn(`Warning: Invalid CIDR prefix in allow list: "${normalizedEntry}"`);
+        continue;
+      }
+
+      // Validate /0 must use 0.0.0.0 for clarity
+      // Any other network address with /0 is ambiguous and likely a configuration error
+      if (prefix === 0 && network !== '0.0.0.0') {
+        console.warn(
+          `Warning: CIDR /0 prefix must use 0.0.0.0, not "${network}". Entry: "${normalizedEntry}"`
+        );
+        continue;
+      }
+
+      try {
+        const networkInt = ipToInt(network);
+        const mask = prefix === 0 ? 0 : (0xffffffff << (32 - prefix)) >>> 0;
+        if ((networkInt & mask) === (urlIpInt & mask)) {
+          return true;
+        }
+      } catch (error) {
+        console.warn(
+          `Warning: Invalid CIDR network address in allow list: "${normalizedEntry}" - ${error instanceof Error ? error.message : error}`
+        );
+      }
+
+      continue; // Skip URL parsing for CIDR entries
+    }
+
     const hasExplicitScheme = SCHEME_PREFIX_RE.test(normalizedEntry);
 
     let parsedAllowed: URL;
@@ -453,31 +494,11 @@ function isUrlAllowed(parsedUrl: URL, allowList: string[], allowSubdomains: bool
         continue;
       }
 
+      // Exact IP match
       if (ipToInt(allowedHost) === urlIpInt) {
         return true;
       }
 
-      let networkSpec = allowedHost;
-      if (allowedPath && allowedPath !== '/') {
-        networkSpec = `${networkSpec}${allowedPath}`;
-      }
-
-      if (networkSpec.includes('/')) {
-        const [network, prefixStr] = networkSpec.split('/');
-        const prefix = Number(prefixStr);
-        if (Number.isInteger(prefix) && prefix >= 0 && prefix <= 32) {
-          try {
-            const networkInt = ipToInt(network);
-            const mask = prefix === 0 ? 0 : (0xffffffff << (32 - prefix)) >>> 0;
-            if ((networkInt & mask) === (urlIpInt & mask)) {
-              return true;
-            }
-          } catch {
-            // Invalid CIDR entry; ignore.
-          }
-        }
-      }
-
       continue;
     }