From b65fb64bd92be77cc52c5daaff78bcf62ff6f8fb Mon Sep 17 00:00:00 2001 From: OpenClaw Date: Thu, 9 Apr 2026 15:20:25 +0530 Subject: [PATCH] fix: address CI failures and review feedback - Fix IPv6 address normalization in SSRF validation using net.isIP() - Add tests for numeric IP encoding bypasses (decimal, hex, 0.0.0.0) - Fix singleton_class static detection regression in both parser paths - Move HTTP_CLIENT_RECEIVERS to module level to avoid hot-loop allocation - Fix GIT_ASKPASS for cross-platform support (Windows uses echo) - De-duplicate PNA header in OPTIONS handler - Run prettier on all modified files --- .../src/core/ingestion/parsing-processor.ts | 10 +- .../core/ingestion/workers/parse-worker.ts | 32 +++-- gitnexus/src/server/api.ts | 5 +- gitnexus/src/server/git-clone.ts | 115 +++++++++++++----- gitnexus/test/unit/git-clone.test.ts | 20 ++- 5 files changed, 124 insertions(+), 58 deletions(-) diff --git a/gitnexus/src/core/ingestion/parsing-processor.ts b/gitnexus/src/core/ingestion/parsing-processor.ts index 1cf2ef6dc..f37f1a52f 100644 --- a/gitnexus/src/core/ingestion/parsing-processor.ts +++ b/gitnexus/src/core/ingestion/parsing-processor.ts @@ -240,13 +240,9 @@ function seqFindEnclosingClassNode(node: SyntaxNode): SyntaxNode | null { let current = node.parent; while (current) { if (CLASS_CONTAINER_TYPES.has(current.type)) { - // Ruby singleton_class (class << self) has no name field u2014 skip it and - // continue walking up to find the actual class/module container. - // This matches the behavior in parse-worker.ts findEnclosingClassNode(). - if (current.type === 'singleton_class') { - current = current.parent; - continue; - } + // Return singleton_class directly so the method extractor sees it as + // the owner node and correctly marks methods as static. Name resolution + // for qualified names is handled separately by findEnclosingClassInfo. return current; } current = current.parent; diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index 103b0a5de..7abff42b4 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -378,12 +378,9 @@ function findEnclosingClassNode(node: SyntaxNode): SyntaxNode | null { let current = node.parent; while (current) { if (CLASS_CONTAINER_TYPES.has(current.type)) { - // Ruby singleton_class (class << self) has no name field — walk up to - // the enclosing class/module so the caller gets a node with a findable name. - if (current.type === 'singleton_class') { - current = current.parent; - continue; - } + // Return singleton_class directly so the method extractor sees it as + // the owner node and correctly marks methods as static. Name resolution + // for qualified names is handled separately by findEnclosingClassInfo. return current; } current = current.parent; @@ -841,6 +838,23 @@ const EXPRESS_ROUTE_METHODS = new Set([ // function is captured separately by the route.fetch query. const HTTP_CLIENT_ONLY_METHODS = new Set(['head', 'options', 'request', 'ajax']); +// Known HTTP client receivers u2014 skip these, they're API consumers not routes +const HTTP_CLIENT_RECEIVERS = new Set([ + 'axios', + 'request', + 'fetch', + 'http', + 'https', + 'got', + 'ky', + 'superagent', + 'needle', + 'undici', + 'apiclient', + 'client', + 'httpclient', +]); + // Decorator names that indicate HTTP route handlers (NestJS, Flask, FastAPI, Spring) const ROUTE_DECORATOR_NAMES = new Set([ 'Get', @@ -1577,12 +1591,6 @@ const processFileGroup = ( const receiverNode = funcNode?.childForFieldName?.('object') ?? funcNode?.children?.[0]; const receiverText = receiverNode?.text?.toLowerCase() ?? ''; - // Known HTTP client receivers u2014 skip these, they're API consumers not routes - const HTTP_CLIENT_RECEIVERS = new Set([ - 'axios', 'request', 'fetch', 'http', 'https', 'got', 'ky', - 'superagent', 'needle', 'undici', 'apiclient', 'client', 'httpclient', - ]); - if (HTTP_CLIENT_RECEIVERS.has(receiverText)) { // This is an HTTP client call, not a route definition u2014 skip it continue; diff --git a/gitnexus/src/server/api.ts b/gitnexus/src/server/api.ts index 9af070133..bb223696a 100644 --- a/gitnexus/src/server/api.ts +++ b/gitnexus/src/server/api.ts @@ -452,10 +452,9 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => // Handle PNA preflight: Chromium sends Access-Control-Request-Private-Network // on OPTIONS requests and expects the allow header in the response. + // Note: the actual Allow-Private-Network header is already set by the global + // middleware above, so we just need to call next() here. app.options('*', (_req, res, next) => { - if (_req.headers['access-control-request-private-network'] === 'true') { - res.setHeader('Access-Control-Allow-Private-Network', 'true'); - } next(); }); diff --git a/gitnexus/src/server/git-clone.ts b/gitnexus/src/server/git-clone.ts index f2ede5d98..557ca55d9 100644 --- a/gitnexus/src/server/git-clone.ts +++ b/gitnexus/src/server/git-clone.ts @@ -49,48 +49,103 @@ export function validateGitUrl(url: string): void { const host = parsed.hostname.toLowerCase(); - // Block well-known internal hostnames - if (host === 'localhost' || BLOCKED_HOSTNAMES.has(host)) { + // Block known dangerous hostnames (cloud metadata services) + const blockedHostnames = ['localhost', 'metadata.google.internal', 'metadata.azure.com']; + if (blockedHostnames.includes(host)) { throw new Error('Cloning from private/internal addresses is not allowed'); } - // IPv6 loopback — URL parser strips brackets, so hostname is "::1" not "[::1]" - if (host === '::1') { + // Check if this is an IPv6 address + if (isIP(host) === 6) { + assertNotPrivateIPv6(host); + return; + } + + // Check if this is an IPv4 address (including numeric encodings) + if (isIP(host) === 4) { + assertNotPrivateIPv4(host); + return; + } + + // For non-IP hostnames, check for numeric IP tricks + // Decimal encoding: 2130706433 = 127.0.0.1 + // Hex encoding: 0x7f000001 = 127.0.0.1 + if (/^\d+$/.test(host) || /^0x[0-9a-f]+$/i.test(host)) { throw new Error('Cloning from private/internal addresses is not allowed'); } - // IPv6 private ranges: ULA (fc00::/7), link-local (fe80::), IPv4-mapped (::ffff:) + // Standard IPv4 regex checks for dotted notation if ( - host.startsWith('fc') || - host.startsWith('fd') || - host.startsWith('fe80') || - host.startsWith('::ffff:') + /^127\./.test(host) || + /^10\./.test(host) || + /^172\.(1[6-9]|2\d|3[01])\./.test(host) || + /^192\.168\./.test(host) || + /^169\.254\./.test(host) || + /^0\./.test(host) || + host === '0.0.0.0' || + /^100\.(6[4-9]|[7-9]\d|1[01]\d|12[0-7])\./.test(host) || + /^198\.1[89]\./.test(host) + ) { + throw new Error('Cloning from private/internal addresses is not allowed'); + } +} + +function assertNotPrivateIPv6(ip: string): void { + // Expand common compressed forms for comparison + const lower = ip.toLowerCase(); + + // IPv6 loopback + if (lower === '::1' || lower === '0:0:0:0:0:0:0:1') { + throw new Error('Cloning from private/internal addresses is not allowed'); + } + + // Unspecified address + if (lower === '::' || lower === '0:0:0:0:0:0:0:0') { + throw new Error('Cloning from private/internal addresses is not allowed'); + } + + // IPv6 Unique Local Address (fc00::/7 = fc and fd prefixes) + if (lower.startsWith('fc') || lower.startsWith('fd')) { + throw new Error('Cloning from private/internal addresses is not allowed'); + } + + // IPv6 link-local (fe80::/10) + if ( + lower.startsWith('fe80') || + lower.startsWith('fe8') || + lower.startsWith('fe9') || + lower.startsWith('fea') || + lower.startsWith('feb') ) { throw new Error('Cloning from private/internal addresses is not allowed'); } - // IPv4 validation — use net.isIP() to catch decimal/hex encoding bypasses - // (e.g. 2130706433, 0x7f000001 both resolve to 127.0.0.1) - if (isIP(host) === 4) { - const octets = host.split('.').map(Number); - const [a, b] = octets; - if ( - a === 127 || // 127.0.0.0/8 loopback - a === 10 || // 10.0.0.0/8 private - (a === 172 && b >= 16 && b <= 31) || // 172.16.0.0/12 private - (a === 192 && b === 168) || // 192.168.0.0/16 private - (a === 169 && b === 254) || // 169.254.0.0/16 link-local - a === 0 || // 0.0.0.0/8 - (a === 100 && b >= 64 && b <= 127) || // 100.64.0.0/10 CGN (RFC 6598) - (a === 198 && (b === 18 || b === 19)) // 198.18.0.0/15 benchmarking - ) { - throw new Error('Cloning from private/internal addresses is not allowed'); - } + // IPv4-mapped IPv6 (::ffff:x.x.x.x or ::ffff:hex:hex) + // Node may normalize ::ffff:127.0.0.1 to ::ffff:7f00:1 + if (lower.startsWith('::ffff:')) { + throw new Error('Cloning from private/internal addresses is not allowed'); } - // Reject bare numeric IPs that aren't valid dotted-quad — could be decimal/hex encoding - if (/^\d+$/.test(host) || /^0x[0-9a-f]+$/i.test(host)) { - throw new Error('Numeric IP encoding is not allowed'); + // Also catch the expanded form: 0:0:0:0:0:ffff: + if (lower.includes(':ffff:')) { + throw new Error('Cloning from private/internal addresses is not allowed'); + } +} + +function assertNotPrivateIPv4(ip: string): void { + const parts = ip.split('.').map(Number); + const [a, b] = parts; + if ( + a === 127 || + a === 10 || + (a === 172 && b >= 16 && b <= 31) || + (a === 192 && b === 168) || + (a === 169 && b === 254) || + a === 0 || + (a === 100 && b >= 64 && b <= 127) || + (a === 198 && (b === 18 || b === 19)) + ) { + throw new Error('Cloning from private/internal addresses is not allowed'); } } @@ -137,7 +192,7 @@ function runGit(args: string[], cwd?: string): Promise { // Prevent git from prompting for credentials (hangs the process) GIT_TERMINAL_PROMPT: '0', // Ensure no credential helper tries to open a GUI prompt - GIT_ASKPASS: '/bin/true', + GIT_ASKPASS: process.platform === 'win32' ? 'echo' : '/bin/true', }, }); diff --git a/gitnexus/test/unit/git-clone.test.ts b/gitnexus/test/unit/git-clone.test.ts index 2df5a0a5e..832f95641 100644 --- a/gitnexus/test/unit/git-clone.test.ts +++ b/gitnexus/test/unit/git-clone.test.ts @@ -50,9 +50,7 @@ describe('git-clone', () => { }); it('blocks file:// protocol', () => { - expect(() => validateGitUrl('file:///etc/passwd')).toThrow( - 'Only https:// and http://', - ); + expect(() => validateGitUrl('file:///etc/passwd')).toThrow('Only https:// and http://'); }); it('blocks IPv4 loopback', () => { @@ -80,9 +78,7 @@ describe('git-clone', () => { expect(() => validateGitUrl('http://metadata.google.internal/repo')).toThrow( 'private/internal', ); - expect(() => validateGitUrl('http://metadata.azure.com/repo')).toThrow( - 'private/internal', - ); + expect(() => validateGitUrl('http://metadata.azure.com/repo')).toThrow('private/internal'); }); it('blocks IPv6 ULA (fc/fd)', () => { @@ -113,5 +109,17 @@ describe('git-clone', () => { expect(() => validateGitUrl('http://198.18.0.1/repo.git')).toThrow('private/internal'); expect(() => validateGitUrl('http://198.19.255.255/repo.git')).toThrow('private/internal'); }); + + it('blocks numeric decimal IP encoding', () => { + expect(() => validateGitUrl('http://2130706433/repo.git')).toThrow('private/internal'); + }); + + it('blocks hex IP encoding', () => { + expect(() => validateGitUrl('http://0x7f000001/repo.git')).toThrow('private/internal'); + }); + + it('blocks 0.0.0.0', () => { + expect(() => validateGitUrl('http://0.0.0.0/repo.git')).toThrow('private/internal'); + }); }); });