Merge branch 'main' into fix/sanitize-repo-name-rebased-v2

This commit is contained in:
Gergő Magyar 2026-05-09 17:26:46 +01:00 committed by GitHub
commit 484a1368c8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 242 additions and 11 deletions

View file

@ -205,8 +205,32 @@ export function streamSSE<T = unknown>(url: string, handlers: SSEHandlers<T>): A
let _backendUrl = 'http://localhost:4747';
/**
* Validate that a backend URL is a safe http:// or https:// origin before
* storing it as the fetch target base (CodeQL js/client-side-request-forgery).
*
* Throws if the URL uses a non-HTTP scheme (e.g. javascript:, data:, file://).
* All other well-formed http/https URLs are accepted the client intentionally
* supports connecting to remote GitNexus servers, not just localhost.
*/
export function validateBackendUrl(url: string): void {
let parsed: URL;
try {
parsed = new URL(url);
} catch {
// Do not echo raw input — it may contain credentials.
throw new Error('Invalid backend URL: must be a well-formed http:// or https:// URL');
}
if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
// Use parsed.protocol only (scheme), not the full URL, to avoid leaking credentials.
throw new Error(`Backend URL must use http:// or https:// (got ${parsed.protocol})`);
}
}
export const setBackendUrl = (url: string): void => {
_backendUrl = url.replace(/\/$/, '');
const trimmed = url.replace(/\/$/, '');
validateBackendUrl(trimmed);
_backendUrl = trimmed;
};
export const getBackendUrl = (): string => _backendUrl;

View file

@ -1,5 +1,11 @@
import { afterEach, describe, expect, it, vi } from 'vitest';
import { fetchGraph, normalizeServerUrl, setBackendUrl } from '../../src/services/backend-client';
import {
fetchGraph,
getBackendUrl,
normalizeServerUrl,
setBackendUrl,
validateBackendUrl,
} from '../../src/services/backend-client';
describe('normalizeServerUrl', () => {
it('adds http:// to localhost', () => {
@ -165,3 +171,61 @@ describe('fetchGraph', () => {
});
});
});
describe('validateBackendUrl', () => {
it('allows http:// URLs', () => {
expect(() => validateBackendUrl('http://localhost:4747')).not.toThrow();
expect(() => validateBackendUrl('http://127.0.0.1:4747')).not.toThrow();
});
it('allows https:// URLs', () => {
expect(() => validateBackendUrl('https://gitnexus.example.com')).not.toThrow();
expect(() => validateBackendUrl('https://my-server.internal:4747')).not.toThrow();
});
it('rejects non-http schemes', () => {
expect(() => validateBackendUrl('javascript:alert(1)')).toThrow('must use http:// or https://');
expect(() => validateBackendUrl('file:///etc/passwd')).toThrow('must use http:// or https://');
expect(() => validateBackendUrl('data:text/plain,evil')).toThrow(
'must use http:// or https://',
);
});
it('rejects malformed URLs', () => {
expect(() => validateBackendUrl('not-a-url')).toThrow('Invalid backend URL');
});
it('does not include the raw URL in error messages (credential hygiene)', () => {
const urlWithCreds = 'javascript:alert("sk-secret")';
let msg = '';
try {
validateBackendUrl(urlWithCreds);
} catch (e) {
msg = (e as Error).message;
}
expect(msg).not.toContain('sk-secret');
expect(msg).not.toContain(urlWithCreds);
});
});
describe('setBackendUrl', () => {
it('accepts valid http URLs', () => {
expect(() => setBackendUrl('http://localhost:4747')).not.toThrow();
});
it('accepts valid https URLs', () => {
expect(() => setBackendUrl('https://my-server.example.com')).not.toThrow();
});
it('rejects non-http/https schemes', () => {
expect(() => setBackendUrl('javascript:alert(1)')).toThrow('must use http:// or https://');
expect(() => setBackendUrl('file:///etc/passwd')).toThrow('must use http:// or https://');
});
it('does not mutate _backendUrl when validation fails', () => {
setBackendUrl('http://localhost:4747');
expect(() => setBackendUrl('javascript:alert(1)')).toThrow();
// State must be preserved — validation must happen before the assignment
expect(getBackendUrl()).toBe('http://localhost:4747');
});
});

View file

@ -722,13 +722,15 @@ export async function openBridgeDbReadOnly(groupDir: string): Promise<BridgeHand
await new Promise((r) => setTimeout(r, delay));
}
}
// Pino's NDJSON serialization is structurally injection-resistant
// (CodeQL js/log-injection): groupDir and err.message are JSON-escaped
// by the serializer, so no manual CRLF / U+2028 / ANSI sanitization is
// needed. Demoted to debug — only fires when the bridge truly gave up
// after retries, and operators only need it at debug verbosity.
// Strip CRLF from user-controlled strings before logging to close
// CodeQL js/log-injection. Pino's NDJSON serialization already
// JSON-escapes all values, but we sanitize here as a defence-in-depth
// measure so CodeQL can see the taint flow is broken.
const safeGroupDir = String(groupDir).replace(/[\r\n]/g, ' ');
const safeErrMsg =
lastErr instanceof Error ? String(lastErr.message).replace(/[\r\n]/g, ' ') : undefined;
bridgeLogger.debug(
{ groupDir, err: lastErr, attempts: LBUG_OPEN_RETRY_ATTEMPTS },
{ groupDir: safeGroupDir, errMsg: safeErrMsg, attempts: LBUG_OPEN_RETRY_ATTEMPTS },
'openBridgeDbReadOnly gave up',
);
return null;

View file

@ -77,6 +77,49 @@ export function estimateTokens(text: string): number {
return Math.ceil(text.length / 4);
}
/**
* Validate that a base URL supplied for LLM API calls is a safe HTTP/HTTPS
* endpoint (CWE-918 / CodeQL js/http-to-file-access).
*
* Allowed:
* - https:// with any hostname (public LLM APIs, Azure, OpenRouter, …)
* - http:// restricted to localhost / 127.0.0.1 (local servers: Ollama, LiteLLM, …)
*
* Rejected:
* - file://, data:, javascript:, and any other non-HTTP scheme
* - http:// aimed at non-loopback hosts (avoids SSRF against internal networks)
*
* Throws with a descriptive message on validation failure so callers surface a
* clear error rather than an opaque network error.
*/
export function validateLLMBaseUrl(baseUrl: string): void {
let parsed: URL;
try {
parsed = new URL(baseUrl);
} catch {
// Do not include the raw input in the message — it may contain credentials.
throw new Error('Invalid LLM base URL: must be a well-formed http:// or https:// URL');
}
if (!['https:', 'http:'].includes(parsed.protocol)) {
// Use parsed.protocol only (scheme), not the full URL, to avoid leaking credentials.
throw new Error(`LLM base URL must use http:// or https:// (got ${parsed.protocol})`);
}
if (parsed.protocol === 'http:') {
// Node's URL parser preserves IPv6 brackets in hostname (e.g. "[::1]"),
// so strip them before comparing to bare address literals.
const host = parsed.hostname.toLowerCase().replace(/^\[|\]$/g, '');
if (host !== 'localhost' && host !== '127.0.0.1' && host !== '::1') {
// Use parsed.origin (scheme+host+port, no credentials) instead of the full URL.
throw new Error(
`Insecure http:// LLM base URLs are only allowed for localhost/127.0.0.1. ` +
`Use https:// for remote endpoints (got ${parsed.origin})`,
);
}
}
}
/**
* Returns true if the given base URL is an Azure OpenAI endpoint.
* Uses proper hostname matching to avoid spoofed URLs like
@ -128,6 +171,9 @@ export async function callLLM(
systemPrompt?: string,
options?: CallLLMOptions,
): Promise<LLMResponse> {
// Validate base URL before any fetch (CodeQL js/http-to-file-access)
validateLLMBaseUrl(config.baseUrl);
const messages: Array<{ role: string; content: string }> = [];
if (systemPrompt) {
messages.push({ role: 'system', content: systemPrompt });

View file

@ -744,8 +744,13 @@ export const createServer = async (port: number, host: string = '127.0.0.1') =>
if (isMatch && ['queued', 'cloning', 'analyzing'].includes(job.status)) {
if (process.env.DEBUG) {
console.log(
`[debug] resolveRepo waiting for active job ${job.id} (${normalizedName})...`,
// Sanitize user-controlled values to prevent log injection (CodeQL js/log-injection).
logger.debug(
{
jobId: String(job.id).replace(/[\r\n]/g, ' '),
repoName: String(normalizedName).replace(/[\r\n]/g, ' '),
},
'[debug] resolveRepo waiting for active job',
);
}
for (let wait = 0; wait < HOLD_QUEUE_TIMEOUT_SECS; wait++) {
@ -769,7 +774,11 @@ export const createServer = async (port: number, host: string = '127.0.0.1') =>
// (e.g. registry file not yet flushed after clone completes).
if (!found && normalizedName && !isRetry) {
if (process.env.DEBUG) {
console.log(`[debug] resolveRepo 404 for "${normalizedName}". Triggering deep init...`);
// Sanitize user-controlled values to prevent log injection (CodeQL js/log-injection).
logger.debug(
{ repoName: String(normalizedName).replace(/[\r\n]/g, ' ') },
'[debug] resolveRepo 404, triggering deep init',
);
}
await backend.init();
return await resolveRepo(normalizedName, true, req);

View file

@ -5,6 +5,7 @@ import {
isAzureProvider,
isReasoningModel,
buildRequestUrl,
validateLLMBaseUrl,
} from '../../src/core/wiki/llm-client.js';
describe('isAzureProvider', () => {
@ -330,3 +331,88 @@ describe('readSSEStream — content_filter handling', () => {
).rejects.toThrow('content filter');
});
});
describe('validateLLMBaseUrl', () => {
it('allows https:// for any public host', () => {
expect(() => validateLLMBaseUrl('https://api.openai.com/v1')).not.toThrow();
expect(() => validateLLMBaseUrl('https://openrouter.ai/api/v1')).not.toThrow();
expect(() => validateLLMBaseUrl('https://myres.openai.azure.com/openai/v1')).not.toThrow();
});
it('allows http:// for localhost', () => {
expect(() => validateLLMBaseUrl('http://localhost:11434/v1')).not.toThrow();
expect(() => validateLLMBaseUrl('http://127.0.0.1:11434/v1')).not.toThrow();
// IPv6 loopback — Node's URL parser preserves brackets in hostname: "[::1]"
expect(() => validateLLMBaseUrl('http://[::1]:11434/v1')).not.toThrow();
});
it('allows http:// for LOCALHOST (uppercase) — lowercased before comparison', () => {
expect(() => validateLLMBaseUrl('http://LOCALHOST:11434/v1')).not.toThrow();
});
it('rejects http:// for non-loopback hosts', () => {
expect(() => validateLLMBaseUrl('http://evil.example.com/v1')).toThrow('Insecure http://');
expect(() => validateLLMBaseUrl('http://192.168.1.1/v1')).toThrow('Insecure http://');
// Private IP ranges
expect(() => validateLLMBaseUrl('http://10.0.0.1/v1')).toThrow('Insecure http://');
// AWS/GCP IMDS — should be blocked
expect(() => validateLLMBaseUrl('http://169.254.169.254/latest/meta-data')).toThrow(
'Insecure http://',
);
});
it('rejects http:// hostname-spoofing attempts', () => {
// Full-hostname comparison prevents prefix/suffix attacks
expect(() => validateLLMBaseUrl('http://localhost.evil.com/v1')).toThrow('Insecure http://');
expect(() => validateLLMBaseUrl('http://127.0.0.1.evil.com/v1')).toThrow('Insecure http://');
// Trailing dot — hostname 'localhost.' ≠ 'localhost'
expect(() => validateLLMBaseUrl('http://localhost./v1')).toThrow('Insecure http://');
});
it('rejects http:// non-loopback IPv6 addresses', () => {
// Link-local IPv6
expect(() => validateLLMBaseUrl('http://[fe80::1]/v1')).toThrow('Insecure http://');
// IPv4-mapped IPv6 loopback — bracket-stripped to '::ffff:127.0.0.1' ≠ '::1'
expect(() => validateLLMBaseUrl('http://[::ffff:127.0.0.1]/v1')).toThrow('Insecure http://');
});
it('rejects non-http schemes', () => {
expect(() => validateLLMBaseUrl('file:///etc/passwd')).toThrow('must use http:// or https://');
expect(() => validateLLMBaseUrl('javascript:alert(1)')).toThrow('must use http:// or https://');
expect(() => validateLLMBaseUrl('data:text/plain,evil')).toThrow(
'must use http:// or https://',
);
expect(() => validateLLMBaseUrl('ftp://example.com')).toThrow('must use http:// or https://');
});
it('rejects malformed URLs', () => {
expect(() => validateLLMBaseUrl('not-a-url')).toThrow('Invalid LLM base URL');
expect(() => validateLLMBaseUrl('')).toThrow('Invalid LLM base URL');
});
it('does not include the raw URL in error messages (credential hygiene)', () => {
// Simulates a URL with an embedded API key
const urlWithCreds = 'http://192.168.1.1/v1?apikey=sk-secret';
let msg = '';
try {
validateLLMBaseUrl(urlWithCreds);
} catch (e) {
msg = (e as Error).message;
}
expect(msg).not.toContain('sk-secret');
expect(msg).not.toContain(urlWithCreds);
});
it('callLLM rejects an invalid base URL before fetching', async () => {
const { callLLM } = await import('../../src/core/wiki/llm-client.js');
await expect(
callLLM('prompt', {
apiKey: 'key',
baseUrl: 'file:///etc/passwd',
model: 'gpt-4o',
maxTokens: 100,
temperature: 0,
}),
).rejects.toThrow('must use http:// or https://');
});
});