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
This commit is contained in:
OpenClaw 2026-04-09 15:20:25 +05:30
parent a3066cab54
commit b65fb64bd9
5 changed files with 124 additions and 58 deletions

View file

@ -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;

View file

@ -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;

View file

@ -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();
});

View file

@ -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<void> {
// 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',
},
});

View file

@ -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');
});
});
});