Merge branch 'main' into feat/Desktop-app

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(

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;
 };
 
 /**

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 <tag> into _tag_
+      expect(extractRepoName('https://github.com/user/repo<tag>.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

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<tag>')).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();
+    });
+  });
 });