Refactor: make whitelist validation a bit more robust (#4757) * refactor: extract IP whitelist validation into helper function - Added isIPInWhitelist helper with error handling for individual whitelist entry checks - Replaced inline whitelist matching logic with reusable function calls - Added JSDoc type annotations and error logging for failed IP matching attempts * refactor: simplify whitelist validation with upfront filtering - Moved IP validation to startup time instead of per-request checking - Extracted validateWhitelist function to filter invalid entries once at initialization - Simplified isIPInWhitelist by removing redundant error handling after validation * fix: correct IP whitelist matching to use parsed CIDR notation
Signed| @@ -25,6 +25,29 @@ if (fs.existsSync(whitelistPath)) { | ||
| 25 | 25 | } |
| 26 | 26 | |
| 27 | 27 | /** |
| 28 | + * Validates and filters the whitelist, removing any invalid entries. | |
| 29 | + * @param {string[]} entries - The whitelist entries to validate | |
| 30 | + * @returns {string[]} The filtered list of valid whitelist entries | |
| 31 | + */ | |
| 32 | +function validateWhitelist(entries) { | |
| 33 | + const validEntries = []; | |
| 34 | + | |
| 35 | + for (const entry of entries) { | |
| 36 | + try { | |
| 37 | + // This will throw if the entry is not a valid IP or CIDR | |
| 38 | + ipMatching.getMatch(entry); | |
| 39 | + validEntries.push(entry); | |
| 40 | + } catch (e) { | |
| 41 | + console.warn(`Whitelist ${color.red('Warning')}: Ignoring invalid entry ${color.yellow(entry)} - ${e.message}`); | |
| 42 | + } | |
| 43 | + } | |
| 44 | + | |
| 45 | + return validEntries; | |
| 46 | +} | |
| 47 | + | |
| 48 | +whitelist = validateWhitelist(whitelist); | |
| 49 | + | |
| 50 | +/** | |
| 28 | 51 | * Get the client IP address from the request headers. |
| 29 | 52 | * @param {import('express').Request} req Express request object |
| 30 | 53 | * @returns {string|undefined} The client IP address |
| @@ -91,9 +114,19 @@ export default async function getWhitelistMiddleware() { | ||
| 91 | 114 | const forwardedIp = getForwardedIp(req); |
| 92 | 115 | const userAgent = req.headers['user-agent']; |
| 93 | 116 | |
| 117 | + /** | |
| 118 | + * Checks if an IP address matches any entry in the whitelist. | |
| 119 | + * @param {string[]} whitelist - The list of whitelisted IPs/CIDRs | |
| 120 | + * @param {string} ip - The IP address to check | |
| 121 | + * @returns {boolean} True if the IP matches any whitelist entry | |
| 122 | + */ | |
| 123 | + function isIPInWhitelist(whitelist, ip) { | |
| 124 | + return whitelist.some(x => ipMatching.matches(ip, ipMatching.getMatch(x))); | |
| 125 | + } | |
| 126 | + | |
| 94 | 127 | //clientIp = req.connection.remoteAddress.split(':').pop(); |
| 95 | - if (!whitelist.some(x => ipMatching.matches(clientIp, ipMatching.getMatch(x))) | |
| 128 | + if (!isIPInWhitelist(whitelist, clientIp) | |
| 96 | 129 | || forwardedIp && !whitelist.some(x => ipMatching.matchesisIPInWhitelist(forwardedIpwhitelist, ipMatching.getMatch(x))forwardedIp) |
| 97 | 130 | ) { |
| 98 | 131 | // Log the connection attempt with real IP address |
| 99 | 132 | const ipDetails = forwardedIp |