diff --git a/gitnexus/src/server/git-clone.ts b/gitnexus/src/server/git-clone.ts index 861a16648..0ced1213a 100644 --- a/gitnexus/src/server/git-clone.ts +++ b/gitnexus/src/server/git-clone.ts @@ -11,6 +11,7 @@ import os from 'os'; import fs from 'fs/promises'; import { isIP } from 'net'; import { logger } from '../core/logger.js'; +import { parseRepoNameFromUrl } from '../storage/git.js'; /** Root directory for all cloned repositories. Targets must resolve inside this. */ const CLONE_ROOT = path.resolve(path.join(os.homedir(), '.gitnexus', 'repos')); @@ -29,20 +30,17 @@ const REPO_NAME_PATTERN = /^[a-zA-Z0-9._-]+$/; * clone root via path traversal. */ export function extractRepoName(url: string): string { - // Strip trailing slashes without a regex to avoid polynomial-ReDoS on - // pathological inputs like `https://x.com/y` + '/'.repeat(1e6). CodeQL's - // js/polynomial-redos flagged `/\/+$/` here. - let end = url.length; - while (end > 0 && url.charCodeAt(end - 1) === 47 /* '/' */) end--; - const cleaned = url.slice(0, end); - - const lastSegment = cleaned.split(/[/:]/).pop() || ''; - const stripped = lastSegment.endsWith('.git') ? lastSegment.slice(0, -4) : lastSegment; - - if (!stripped || stripped === '.' || stripped === '..' || !REPO_NAME_PATTERN.test(stripped)) { + const name = parseRepoNameFromUrl(url); + if ( + !name || + name === '.' || + name === '..' || + name === 'unknown' || + !REPO_NAME_PATTERN.test(name) + ) { throw new Error('Could not extract a valid repository name from URL'); } - return stripped; + return name; } /** Get the clone target directory for a repo name. */ @@ -399,8 +397,8 @@ export async function cloneOrPull( } // Always validate the requested URL — the prior shape only ran this in - // the clone branch, leaving the pull branch as an SSRF / blocked-host - // bypass when an existing clone shared the basename of an attacker URL. + // the code path where the repo was cloned. Now it runs unconditionally, + // preventing SSRF / blocked-host bypasses even when targetDir already exists. validateGitUrl(url); const exists = await fs.access(path.join(safeTarget, '.git')).then( diff --git a/gitnexus/src/storage/git.ts b/gitnexus/src/storage/git.ts index 4cb0154fe..75e6e91d3 100644 --- a/gitnexus/src/storage/git.ts +++ b/gitnexus/src/storage/git.ts @@ -255,24 +255,63 @@ export const getRemoteOriginUrl = (repoPath: string): string | null => { }; /** - * Parse a repository name out of a git remote URL. Handles the common - * SSH (`git@host:owner/repo.git`), HTTPS (`https://host/owner/repo.git`), - * `git://`, `ssh://`, and `file://` shapes. Returns `null` for empty / - * unparseable input. + * Sanitize a repository name to prevent argument injection and ensure + * cross-platform filesystem compatibility. * - * The heuristic: strip a trailing `.git` and trailing slashes, then - * take the segment after the last `/` or `:`. + * 1. Strips leading dashes to prevent git command-line argument injection + * (e.g., --upload-pack=evil). + * 2. Replaces characters that are unsafe for directory names across + * platforms (Windows/macOS/Linux) with underscores. + * 3. Blocks path traversal segments ("." and "..") and Windows reserved + * names (e.g., CON, NUL) to prevent directory escape. + */ +export const sanitizeRepoName = (name: string): string => { + // 1. Prevent argument injection by stripping leading dashes. + // 2. Remove characters that are not alphanumerics, dots, underscores, or dashes. + const sanitized = name.replace(/^-+/, '').replace(/[^a-zA-Z0-9._-]/g, '_'); + + // 3. Block path traversal segments and Windows reserved names. + // Windows reserved names like CON, PRN, AUX, NUL, COM1-9, LPT1-9 cannot + // be used as directory names on Windows even if they have an extension. + const reserved = /^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])(\..*)?$/i; + if (!sanitized || sanitized === '.' || sanitized === '..' || reserved.test(sanitized)) { + return 'unknown'; + } + + return sanitized; +}; + +/** + * Parse a repository name out of a git remote URL. Handles common shapes + * including SSH (git@host:owner/repo.git) and HTTPS (https://host/owner/repo.git). + * + * Returns a sanitized, filesystem-safe name or null if no name could be inferred. + * Returning null (rather than 'unknown') allows callers to use ?? null-coalescing + * for fallbacks without risk of registry collisions on 'unknown'. */ export const parseRepoNameFromUrl = (url: string | null | undefined): string | null => { if (!url) return null; const trimmed = url.trim(); if (!trimmed) return null; - // Strip `.git` suffix (case-insensitive) and any trailing slashes. - const withoutSuffix = trimmed.replace(/\.git\/*$/i, '').replace(/\/+$/, ''); - // Last path segment, splitting on either `/` or `:` (covers SSH form). - const m = withoutSuffix.match(/[/:]([^/:]+)$/); - const candidate = m ? m[1] : withoutSuffix; - return candidate || null; + + // Strip trailing slashes without a regex to avoid polynomial-ReDoS on + // pathological inputs like `https://x.com/y` + '/'.repeat(1e6). + let end = trimmed.length; + while (end > 0 && trimmed.charCodeAt(end - 1) === 47 /* '/' */) end--; + let cleaned = trimmed.slice(0, end); + + // Strip trailing .git (case-insensitive) + if (cleaned.toLowerCase().endsWith('.git')) { + cleaned = cleaned.slice(0, -4); + } + + // Last path segment, handling colons for SSH URLs and path traversal. + // Split on both / and : to consistently extract the last part. + const candidate = cleaned.split(/[/:]/).pop() || ''; + if (!candidate) return null; + + const safe = sanitizeRepoName(candidate); + return safe === 'unknown' ? null : safe; }; /** diff --git a/gitnexus/test/unit/git-clone.test.ts b/gitnexus/test/unit/git-clone.test.ts index 18ac8e9d6..88832aa83 100644 --- a/gitnexus/test/unit/git-clone.test.ts +++ b/gitnexus/test/unit/git-clone.test.ts @@ -7,12 +7,12 @@ import { buildCloneArgs, normalizeGitUrlForCompare, assertRemoteMatchesRequestedUrl, - getRemoteOriginUrl, } from '../../src/server/git-clone.js'; import path from 'node:path'; import os from 'node:os'; import fs from 'node:fs/promises'; import { spawn } from 'node:child_process'; +import { getRemoteOriginUrl } from '../../src/storage/git.js'; describe('git-clone', () => { describe('extractRepoName', () => { @@ -50,17 +50,6 @@ describe('git-clone', () => { expect(() => extractRepoName('https://example.com/foo:.')).toThrow('valid repository name'); }); - it('rejects URLs with shell metacharacters in the last segment', () => { - // The split on /[/:]/ does not split on backslashes or other shell chars, - // so a name like `repo;rm -rf /` would slip through without the pattern. - expect(() => extractRepoName('https://example.com/foo:repo;rm')).toThrow( - 'valid repository name', - ); - expect(() => extractRepoName('https://example.com/foo:repo$x')).toThrow( - 'valid repository name', - ); - }); - it('rejects empty input', () => { expect(() => extractRepoName('')).toThrow('valid repository name'); }); @@ -77,6 +66,31 @@ describe('git-clone', () => { // multiple seconds on 10k slashes). expect(elapsedMs).toBeLessThan(500); }); + + it('strips leading dashes to prevent argument injection', () => { + expect(extractRepoName('https://github.com/user/--upload-pack=payload.git')).toBe( + 'upload-pack_payload', + ); + expect(extractRepoName('https://github.com/user/-repo')).toBe('repo'); + }); + + it('sanitizes unsafe directory characters', () => { + // sanitizeRepoName turns into _tag_ + expect(extractRepoName('https://github.com/user/repo.git')).toBe('repo_tag_'); + }); + + it('sanitizes shell metacharacters in URL segments', () => { + // The split on /[/:]/ does not split on backslashes or other shell chars, + // so a name like `repo;rm -rf /` would slip through without the pattern. + // After fix/sanitize-repo-name, these are sanitized to underscores. + expect(extractRepoName('https://example.com/foo:repo;rm')).toBe('repo_rm'); + expect(extractRepoName('https://example.com/foo:repo$x')).toBe('repo_x'); + }); + + it('sanitizes whitespace and backslashes', () => { + expect(extractRepoName('https://example.com/foo:repo name')).toBe('repo_name'); + expect(extractRepoName('https://example.com/foo:repo\\name')).toBe('repo_name'); + }); }); describe('getCloneDir', () => { @@ -209,7 +223,7 @@ describe('git-clone', () => { it('blocks NAT64 with embedded RFC1918 addresses', () => { // The startsWith('64:ff9b:') check covers any embedded IPv4. These - // explicit RFC1918 cases document SSRF coverage for the full private + // explicit RFC1918 architectures document SSRF coverage for the full private // IPv4 surface — not just loopback and cloud metadata. expect(() => validateGitUrl('http://[64:ff9b::a00:1]/repo.git')).toThrow('private/internal'); // 10.0.0.1 expect(() => validateGitUrl('http://[64:ff9b::ac10:1]/repo.git')).toThrow('private/internal'); // 172.16.0.1 diff --git a/gitnexus/test/unit/git.test.ts b/gitnexus/test/unit/git.test.ts index af339ce69..1bebf4143 100644 --- a/gitnexus/test/unit/git.test.ts +++ b/gitnexus/test/unit/git.test.ts @@ -8,6 +8,8 @@ import { getCurrentCommit, getGitRoot, findGitRootByDotGit, + parseRepoNameFromUrl, + sanitizeRepoName, } from '../../src/storage/git.js'; // Mock child_process.execSync @@ -164,4 +166,61 @@ describe('git utilities', () => { } }); }); + + describe('sanitizeRepoName', () => { + it('strips leading dashes', () => { + expect(sanitizeRepoName('--repo')).toBe('repo'); + }); + + it('replaces unsafe characters with underscores', () => { + expect(sanitizeRepoName('repo')).toBe('repo_tag_'); + expect(sanitizeRepoName('repo:name')).toBe('repo_name'); + expect(sanitizeRepoName('repo"quoted"')).toBe('repo_quoted_'); + }); + + it('blocks path traversal segments', () => { + expect(sanitizeRepoName('.')).toBe('unknown'); + expect(sanitizeRepoName('..')).toBe('unknown'); + }); + + it('blocks Windows reserved names', () => { + expect(sanitizeRepoName('CON')).toBe('unknown'); + expect(sanitizeRepoName('prn')).toBe('unknown'); + expect(sanitizeRepoName('AUX')).toBe('unknown'); + expect(sanitizeRepoName('NUL')).toBe('unknown'); + expect(sanitizeRepoName('COM1')).toBe('unknown'); + expect(sanitizeRepoName('LPT9')).toBe('unknown'); + + // Reserved names with extensions + expect(sanitizeRepoName('CON.txt')).toBe('unknown'); + expect(sanitizeRepoName('NUL.tar.gz')).toBe('unknown'); + expect(sanitizeRepoName('AUX.local')).toBe('unknown'); + }); + + it('returns unknown for empty or invalid input', () => { + expect(sanitizeRepoName('')).toBe('unknown'); + expect(sanitizeRepoName('---')).toBe('unknown'); + }); + }); + + describe('parseRepoNameFromUrl', () => { + it('extracts and sanitizes name from HTTPS URL', () => { + expect(parseRepoNameFromUrl('https://github.com/user/my-repo.git')).toBe('my-repo'); + expect(parseRepoNameFromUrl('https://github.com/user/--payload.git')).toBe('payload'); + }); + + it('extracts and sanitizes name from SSH URL', () => { + expect(parseRepoNameFromUrl('git@github.com:user/my-repo.git')).toBe('my-repo'); + expect(parseRepoNameFromUrl('git@github.com:--payload.git')).toBe('payload'); + }); + + it('returns null for all-dash inputs (prevents registry collision)', () => { + expect(parseRepoNameFromUrl('https://github.com/user/---.git')).toBeNull(); + }); + + it('returns null for empty URL', () => { + expect(parseRepoNameFromUrl('')).toBeNull(); + expect(parseRepoNameFromUrl(null)).toBeNull(); + }); + }); });