mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-06 08:16:02 +00:00
* fix(security): U11 log-injection, http-to-file-access, client-side-request-forgery U11.1: Add validateLLMBaseUrl() in llm-client.ts; called at the top of callLLM() to reject non-http/https schemes and http:// to non-loopback hosts before any fetch that writes LLM output to disk. U11.2: Strip CRLF from groupDir in bridge-db.ts openBridgeDbReadOnly before logging (defence-in-depth on top of pino's JSON escaping). U11.3: Replace console.log with logger.debug and sanitize normalizedName / job.id in api.ts resolveRepo to close js/log-injection alerts. U11.4: Add validateBackendUrl() in backend-client.ts; called inside setBackendUrl() to reject non-http/https schemes before the URL is stored as a fetch target, closing js/client-side-request-forgery alerts. U11.5: Tests added: - wiki-llm-client.test.ts: validateLLMBaseUrl happy/error paths - server-connection.test.ts: validateBackendUrl and setBackendUrl rejection paths All new tests pass (30/30 wiki-llm-client, 18/18 server-connection, 30/30 bridge-db). Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/0452a6ce-711f-4203-9ae6-5dd0b77fb157 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix: correct IPv6 loopback check in validateLLMBaseUrl Node's URL parser preserves brackets in hostname for IPv6 addresses (e.g. http://[::1]:11434 yields hostname '[::1]'), so strip them before comparing against '::1'. Add a test to cover this case. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/0452a6ce-711f-4203-9ae6-5dd0b77fb157 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix: also sanitize error message in bridge-db log call Sanitize lastErr.message (which may contain a file path from ENOENT errors) alongside groupDir to prevent CRLF injection from error message content. Addressed code review feedback. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/0452a6ce-711f-4203-9ae6-5dd0b77fb157 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix: address security review findings — credential hygiene and test coverage [LOW] Redact credentials from URL validation error messages: - validateLLMBaseUrl: malformed URL no longer echoes raw input; scheme error shows protocol only; http-non-loopback error uses parsed.origin (scheme+host+port) instead of full URL - validateBackendUrl: same treatment — no raw input in any error path [INFO] Add state-preservation test for setBackendUrl: - Proves _backendUrl is unchanged after a rejected call, covering the validation-before-assignment ordering. [INFO] Expand validateLLMBaseUrl adversarial test coverage: - LOCALHOST uppercase (case-fold path) - RFC 1918 / IMDS IPs (10.x, 169.254.x) - Hostname-spoofing (localhost.evil.com, 127.0.0.1.evil.com, localhost.) - Non-loopback IPv6 (fe80::1, ::ffff:127.0.0.1) - ftp:// scheme - Credential-hygiene assertion (sk-secret not in error message) Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/7bb18fa2-3e66-4fe0-949f-6d493fbd351b Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * style: prettier autoformat U11 security fix files Fixes the failing 'quality / format' check on PR #1456 by running 'prettier --write' over the 6 files touched by the security fix. Formatting only — no logic change. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: Gergo Magyar <gergomagyar@icloud.com>
231 lines
7.2 KiB
TypeScript
231 lines
7.2 KiB
TypeScript
import { afterEach, describe, expect, it, vi } from 'vitest';
|
|
import {
|
|
fetchGraph,
|
|
getBackendUrl,
|
|
normalizeServerUrl,
|
|
setBackendUrl,
|
|
validateBackendUrl,
|
|
} from '../../src/services/backend-client';
|
|
|
|
describe('normalizeServerUrl', () => {
|
|
it('adds http:// to localhost', () => {
|
|
expect(normalizeServerUrl('localhost:4747')).toBe('http://localhost:4747');
|
|
});
|
|
|
|
it('adds http:// to 127.0.0.1', () => {
|
|
expect(normalizeServerUrl('127.0.0.1:4747')).toBe('http://127.0.0.1:4747');
|
|
});
|
|
|
|
it('adds https:// to non-local hosts', () => {
|
|
expect(normalizeServerUrl('example.com')).toBe('https://example.com');
|
|
});
|
|
|
|
it('strips trailing slashes', () => {
|
|
expect(normalizeServerUrl('http://localhost:4747/')).toBe('http://localhost:4747');
|
|
expect(normalizeServerUrl('http://localhost:4747///')).toBe('http://localhost:4747');
|
|
});
|
|
|
|
it('strips /api suffix (base URL only)', () => {
|
|
expect(normalizeServerUrl('http://localhost:4747/api')).toBe('http://localhost:4747');
|
|
});
|
|
|
|
it('trims whitespace', () => {
|
|
expect(normalizeServerUrl(' localhost:4747 ')).toBe('http://localhost:4747');
|
|
});
|
|
|
|
it('preserves existing https://', () => {
|
|
expect(normalizeServerUrl('https://gitnexus.example.com')).toBe('https://gitnexus.example.com');
|
|
});
|
|
});
|
|
|
|
afterEach(() => {
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
describe('fetchGraph', () => {
|
|
it('requests streamed graph responses from the backend', async () => {
|
|
setBackendUrl('http://localhost:4747');
|
|
|
|
const fetchMock = vi.fn().mockResolvedValue(
|
|
new Response('{"nodes":[],"relationships":[]}', {
|
|
status: 200,
|
|
headers: {
|
|
'Content-Type': 'application/json',
|
|
},
|
|
}),
|
|
);
|
|
vi.stubGlobal('fetch', fetchMock);
|
|
|
|
await fetchGraph('big-repo');
|
|
|
|
expect(fetchMock).toHaveBeenCalledWith(
|
|
expect.stringContaining('/api/graph?repo=big-repo&stream=true'),
|
|
expect.any(Object),
|
|
);
|
|
});
|
|
|
|
it('parses NDJSON graph streams incrementally', async () => {
|
|
setBackendUrl('http://localhost:4747');
|
|
|
|
const encoder = new TextEncoder();
|
|
const stream = new ReadableStream<Uint8Array>({
|
|
start(controller) {
|
|
controller.enqueue(
|
|
encoder.encode(
|
|
[
|
|
'{"type":"node","data":{"id":"File:src/app.ts","label":"File","properties":{"name":"app.ts","filePath":"src/app.ts"}}}\n',
|
|
'{"type":"relationship","data":{"id":"File:src/app.ts_CONTAINS_Function:src/app.ts:main","type":"CONTAINS","sourceId":"File:src/app.ts","targetId":"Function:src/app.ts:main"}}\n',
|
|
].join(''),
|
|
),
|
|
);
|
|
controller.close();
|
|
},
|
|
});
|
|
|
|
vi.stubGlobal(
|
|
'fetch',
|
|
vi.fn().mockResolvedValue(
|
|
new Response(stream, {
|
|
status: 200,
|
|
headers: {
|
|
'Content-Type': 'application/x-ndjson',
|
|
},
|
|
}),
|
|
),
|
|
);
|
|
|
|
const progress = vi.fn();
|
|
const result = await fetchGraph('big-repo', { onProgress: progress });
|
|
|
|
expect(result.nodes).toHaveLength(1);
|
|
expect(result.relationships).toHaveLength(1);
|
|
expect(result.nodes[0].id).toBe('File:src/app.ts');
|
|
expect(result.relationships[0].type).toBe('CONTAINS');
|
|
expect(progress).toHaveBeenCalled();
|
|
});
|
|
|
|
it('parses NDJSON graph lines split across chunks', async () => {
|
|
setBackendUrl('http://localhost:4747');
|
|
|
|
const encoder = new TextEncoder();
|
|
const stream = new ReadableStream<Uint8Array>({
|
|
start(controller) {
|
|
controller.enqueue(
|
|
encoder.encode(
|
|
'{"type":"node","data":{"id":"File:src/app.ts","label":"File","properties":{"name":"app.ts"',
|
|
),
|
|
);
|
|
controller.enqueue(
|
|
encoder.encode(
|
|
',"filePath":"src/app.ts"}}}\n{"type":"relationship","data":{"id":"File:src/app.ts_CONTAINS_Function:src/app.ts:main","type":"CONTAINS","sourceId":"File:src/app.ts","targetId":"Function:src/app.ts:main"}}\n',
|
|
),
|
|
);
|
|
controller.close();
|
|
},
|
|
});
|
|
|
|
vi.stubGlobal(
|
|
'fetch',
|
|
vi.fn().mockResolvedValue(
|
|
new Response(stream, {
|
|
status: 200,
|
|
headers: {
|
|
'Content-Type': 'application/x-ndjson',
|
|
},
|
|
}),
|
|
),
|
|
);
|
|
|
|
const result = await fetchGraph('big-repo');
|
|
|
|
expect(result.nodes).toHaveLength(1);
|
|
expect(result.relationships).toHaveLength(1);
|
|
expect(result.nodes[0].properties.filePath).toBe('src/app.ts');
|
|
});
|
|
|
|
it('throws backend errors emitted in the NDJSON stream', async () => {
|
|
setBackendUrl('http://localhost:4747');
|
|
|
|
const encoder = new TextEncoder();
|
|
const stream = new ReadableStream<Uint8Array>({
|
|
start(controller) {
|
|
controller.enqueue(encoder.encode('{"type":"error","error":"stream failed"}\n'));
|
|
controller.close();
|
|
},
|
|
});
|
|
|
|
vi.stubGlobal(
|
|
'fetch',
|
|
vi.fn().mockResolvedValue(
|
|
new Response(stream, {
|
|
status: 200,
|
|
headers: {
|
|
'Content-Type': 'application/x-ndjson',
|
|
},
|
|
}),
|
|
),
|
|
);
|
|
|
|
await expect(fetchGraph('big-repo')).rejects.toMatchObject({
|
|
message: 'stream failed',
|
|
});
|
|
});
|
|
});
|
|
|
|
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');
|
|
});
|
|
});
|