From 2dd4dc2c30324b996fbfe5fe5dcdd9ad38782fa4 Mon Sep 17 00:00:00 2001 From: hzt <3061613175@qq.com> Date: Fri, 20 Feb 2026 13:18:05 +0800 Subject: [PATCH 1/2] fix: add user-friendly error messages for remote git operation failures Improve error handling for remote git operations (clone, fetch, ls-remote) by wrapping raw errors in descriptive RepomixError messages that distinguish between timeout, authentication, repository-not-found, and connection errors. This builds on the existing GIT_TERMINAL_PROMPT=0 and 30s timeout to provide clear, actionable error messages when accessing inaccessible repositories. Fixes #1077 --- src/core/git/gitCommand.ts | 65 +++++++++++- tests/core/git/gitCommand.test.ts | 168 +++++++++++++++++++++++++++++- 2 files changed, 225 insertions(+), 8 deletions(-) diff --git a/src/core/git/gitCommand.ts b/src/core/git/gitCommand.ts index cd3ebed7f..0abbc6615 100644 --- a/src/core/git/gitCommand.ts +++ b/src/core/git/gitCommand.ts @@ -11,6 +11,53 @@ const GIT_REMOTE_TIMEOUT = 30000; const gitRemoteEnv = { ...process.env, GIT_TERMINAL_PROMPT: '0' }; const gitRemoteOpts = { timeout: GIT_REMOTE_TIMEOUT, env: gitRemoteEnv }; +interface ExecFileError extends Error { + killed?: boolean; + signal?: string; + code?: number | string; + stderr?: string; +} + +export const createGitRemoteError = (error: unknown, url: string, operation: string): RepomixError => { + const err = error as ExecFileError; + const stderr = err.stderr || err.message || ''; + + // Timeout: process was killed by the timeout option + if (err.killed || err.signal === 'SIGTERM') { + return new RepomixError( + `Git ${operation} timed out after ${GIT_REMOTE_TIMEOUT / 1000} seconds for ${url}. The repository may be inaccessible, or the network connection is too slow.`, + ); + } + + // Authentication failure + if (stderr.includes('Authentication failed') || stderr.includes('could not read Username')) { + return new RepomixError( + `Git ${operation} failed for ${url}: Authentication required. The repository may be private or the URL may be incorrect.`, + ); + } + + // Repository not found + if (stderr.includes('not found') || stderr.includes('does not exist') || stderr.includes('Repository not found')) { + return new RepomixError( + `Git ${operation} failed for ${url}: Repository not found. Please verify the URL is correct.`, + ); + } + + // Connection errors + if ( + stderr.includes('Could not resolve host') || + stderr.includes('Failed to connect') || + stderr.includes('Connection refused') + ) { + return new RepomixError( + `Git ${operation} failed for ${url}: Unable to connect to the remote host. Please check your network connection and the URL.`, + ); + } + + // Generic fallback + return new RepomixError(`Git ${operation} failed for ${url}: ${err.message}`); +}; + export const execGitLogFilenames = async ( directory: string, maxCommits = 100, @@ -101,7 +148,7 @@ export const execLsRemote = async ( return result.stdout || ''; } catch (error) { logger.trace('Failed to execute git ls-remote:', (error as Error).message); - throw error; + throw createGitRemoteError(error, url, 'ls-remote'); } }; @@ -126,13 +173,19 @@ export const execGitShallowClone = async ( ); await deps.execFileAsync('git', ['-C', directory, 'checkout', 'FETCH_HEAD']); } catch (err: unknown) { + // Check for timeout first — no point retrying if the remote is unreachable + const execErr = err as ExecFileError; + if (execErr.killed || execErr.signal === 'SIGTERM') { + throw createGitRemoteError(err, url, 'fetch'); + } + // git fetch --depth 1 origin always throws "couldn't find remote ref" error const isRefNotfoundError = err instanceof Error && err.message.includes(`couldn't find remote ref ${remoteBranch}`); if (!isRefNotfoundError) { // Rethrow error as nothing else we can do - throw err; + throw createGitRemoteError(err, url, 'fetch'); } // Short SHA detection - matches a hexadecimal string of 4 to 39 characters @@ -142,7 +195,7 @@ export const execGitShallowClone = async ( if (isNotShortSHA) { // Rethrow error as nothing else we can do - throw err; + throw createGitRemoteError(err, url, 'fetch'); } // Maybe the error is due to a short SHA, let's try again @@ -151,7 +204,11 @@ export const execGitShallowClone = async ( await deps.execFileAsync('git', ['-C', directory, 'checkout', remoteBranch]); } } else { - await deps.execFileAsync('git', ['clone', '--depth', '1', '--', url, directory], gitRemoteOpts); + try { + await deps.execFileAsync('git', ['clone', '--depth', '1', '--', url, directory], gitRemoteOpts); + } catch (error) { + throw createGitRemoteError(error, url, 'clone'); + } } // Clean up .git directory diff --git a/tests/core/git/gitCommand.test.ts b/tests/core/git/gitCommand.test.ts index e4b2d5e04..ccf2036d5 100644 --- a/tests/core/git/gitCommand.test.ts +++ b/tests/core/git/gitCommand.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, test, vi } from 'vitest'; import { + createGitRemoteError, execGitDiff, execGitLog, execGitLogFilenames, @@ -8,6 +9,7 @@ import { execGitVersion, execLsRemote, } from '../../../src/core/git/gitCommand.js'; +import { RepomixError } from '../../../src/shared/errorHandle.js'; import { logger } from '../../../src/shared/logger.js'; vi.mock('../../../src/shared/logger'); @@ -142,7 +144,7 @@ file2.ts await expect( execGitShallowClone(url, directory, remoteBranch, { execFileAsync: mockFileExecAsync }), - ).rejects.toThrow('Authentication failed'); + ).rejects.toThrow('Authentication required'); expect(mockFileExecAsync).toHaveBeenCalledWith( 'git', @@ -193,7 +195,7 @@ file2.ts await expect( execGitShallowClone(url, directory, remoteBranch, { execFileAsync: mockFileExecAsync }), - ).rejects.toThrow('Authentication failed'); + ).rejects.toThrow('Authentication required'); expect(mockFileExecAsync).toHaveBeenCalledTimes(3); expect(mockFileExecAsync).toHaveBeenNthCalledWith(1, 'git', ['-C', directory, 'init']); expect(mockFileExecAsync).toHaveBeenNthCalledWith(2, 'git', [ @@ -268,7 +270,7 @@ file2.ts await expect( execGitShallowClone(url, directory, remoteBranch, { execFileAsync: mockFileExecAsync }), - ).rejects.toThrow(errMessage); + ).rejects.toThrow(RepomixError); expect(mockFileExecAsync).toHaveBeenCalledTimes(3); expect(mockFileExecAsync).toHaveBeenNthCalledWith(1, 'git', ['-C', directory, 'init']); expect(mockFileExecAsync).toHaveBeenNthCalledWith(2, 'git', [ @@ -408,8 +410,166 @@ c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8\trefs/tags/v1.0.0 await expect( execLsRemote('https://github.com/user/repo.git', { execFileAsync: mockFileExecAsync }), - ).rejects.toThrow('git command failed'); + ).rejects.toThrow(RepomixError); expect(logger.trace).toHaveBeenCalledWith('Failed to execute git ls-remote:', 'git command failed'); }); }); + + describe('createGitRemoteError', () => { + const url = 'https://github.com/user/repo.git'; + + test('should create timeout error when process was killed', () => { + const error = Object.assign(new Error('Command failed'), { killed: true, signal: 'SIGTERM' }); + const result = createGitRemoteError(error, url, 'clone'); + + expect(result).toBeInstanceOf(RepomixError); + expect(result.message).toContain('timed out after 30 seconds'); + expect(result.message).toContain(url); + }); + + test('should create timeout error when SIGTERM signal is present', () => { + const error = Object.assign(new Error('Command failed'), { signal: 'SIGTERM' }); + const result = createGitRemoteError(error, url, 'ls-remote'); + + expect(result).toBeInstanceOf(RepomixError); + expect(result.message).toContain('timed out'); + expect(result.message).toContain('ls-remote'); + }); + + test('should create authentication error for auth failures', () => { + const error = Object.assign(new Error('Command failed'), { + stderr: 'fatal: Authentication failed for https://github.com/user/repo.git', + }); + const result = createGitRemoteError(error, url, 'clone'); + + expect(result).toBeInstanceOf(RepomixError); + expect(result.message).toContain('Authentication required'); + expect(result.message).toContain('private'); + }); + + test('should create authentication error when username prompt fails', () => { + const error = Object.assign(new Error('Command failed'), { + stderr: 'fatal: could not read Username for', + }); + const result = createGitRemoteError(error, url, 'fetch'); + + expect(result).toBeInstanceOf(RepomixError); + expect(result.message).toContain('Authentication required'); + }); + + test('should create not-found error for missing repositories', () => { + const error = Object.assign(new Error('Command failed'), { + stderr: 'ERROR: Repository not found.', + }); + const result = createGitRemoteError(error, url, 'clone'); + + expect(result).toBeInstanceOf(RepomixError); + expect(result.message).toContain('Repository not found'); + expect(result.message).toContain('verify the URL'); + }); + + test('should create connection error for DNS failures', () => { + const error = Object.assign(new Error('Command failed'), { + stderr: 'fatal: Could not resolve host: github.com', + }); + const result = createGitRemoteError(error, url, 'ls-remote'); + + expect(result).toBeInstanceOf(RepomixError); + expect(result.message).toContain('Unable to connect'); + expect(result.message).toContain('network connection'); + }); + + test('should create connection error for refused connections', () => { + const error = Object.assign(new Error('Command failed'), { + stderr: 'fatal: Connection refused', + }); + const result = createGitRemoteError(error, url, 'clone'); + + expect(result).toBeInstanceOf(RepomixError); + expect(result.message).toContain('Unable to connect'); + }); + + test('should create generic error as fallback', () => { + const error = new Error('Some unknown git error'); + const result = createGitRemoteError(error, url, 'fetch'); + + expect(result).toBeInstanceOf(RepomixError); + expect(result.message).toContain('Some unknown git error'); + expect(result.message).toContain('fetch'); + expect(result.message).toContain(url); + }); + }); + + describe('timeout and error handling for remote operations', () => { + test('execLsRemote should throw timeout error when process is killed', async () => { + const timeoutError = Object.assign(new Error('Command failed: git ls-remote'), { + killed: true, + signal: 'SIGTERM', + }); + const mockFileExecAsync = vi.fn().mockRejectedValue(timeoutError); + + await expect( + execLsRemote('https://github.com/user/repo.git', { execFileAsync: mockFileExecAsync }), + ).rejects.toThrow('timed out after 30 seconds'); + }); + + test('execGitShallowClone should throw timeout error when clone times out', async () => { + const timeoutError = Object.assign(new Error('Command failed: git clone'), { + killed: true, + signal: 'SIGTERM', + }); + const mockFileExecAsync = vi.fn().mockRejectedValue(timeoutError); + + await expect( + execGitShallowClone('https://github.com/user/repo.git', '/tmp/repo', undefined, { + execFileAsync: mockFileExecAsync, + }), + ).rejects.toThrow('timed out after 30 seconds'); + }); + + test('execGitShallowClone should throw timeout error when fetch with branch times out', async () => { + const timeoutError = Object.assign(new Error('Command failed: git fetch'), { + killed: true, + signal: 'SIGTERM', + }); + const mockFileExecAsync = vi + .fn() + .mockResolvedValueOnce({ stdout: '', stderr: '' }) // git init + .mockResolvedValueOnce({ stdout: '', stderr: '' }) // git remote add + .mockRejectedValueOnce(timeoutError); // git fetch times out + + await expect( + execGitShallowClone('https://github.com/user/repo.git', '/tmp/repo', 'main', { + execFileAsync: mockFileExecAsync, + }), + ).rejects.toThrow('timed out after 30 seconds'); + + // Should not attempt short SHA fallback after timeout + expect(mockFileExecAsync).toHaveBeenCalledTimes(3); + }); + + test('execGitShallowClone should throw descriptive error for repo not found on clone', async () => { + const error = Object.assign(new Error('Command failed'), { + stderr: 'ERROR: Repository not found.', + }); + const mockFileExecAsync = vi.fn().mockRejectedValue(error); + + await expect( + execGitShallowClone('https://github.com/user/repo.git', '/tmp/repo', undefined, { + execFileAsync: mockFileExecAsync, + }), + ).rejects.toThrow('Repository not found'); + }); + + test('execLsRemote should throw descriptive error for authentication failures', async () => { + const error = Object.assign(new Error('Command failed'), { + stderr: "fatal: could not read Username for 'https://github.com': terminal prompts disabled", + }); + const mockFileExecAsync = vi.fn().mockRejectedValue(error); + + await expect( + execLsRemote('https://github.com/user/private-repo.git', { execFileAsync: mockFileExecAsync }), + ).rejects.toThrow('Authentication required'); + }); + }); }); From 17115dd9156861a106c037107314b9c309805f8b Mon Sep 17 00:00:00 2001 From: hzt <3061613175@qq.com> Date: Fri, 20 Feb 2026 13:51:14 +0800 Subject: [PATCH 2/2] fix(core): Harden createGitRemoteError against security and robustness issues Address PR review feedback from gemini-code-assist and coderabbitai: - Redact embedded credentials from URLs in error messages to prevent sensitive information leakage (e.g. https://user:pass@host -> https://***@host) - Use Partial with optional chaining for safe property access, preventing crashes when error is null or a primitive - Preserve original error context via { cause: error } in RepomixError - Tighten 'not found' substring check to avoid false positives from unrelated stderr messages (removed broad 'not found', kept specific 'Repository not found' and 'does not exist') - Wrap git init/remote add in try-catch with descriptive RepomixError - Wrap short-SHA retry path in try-catch with createGitRemoteError - Add tests for credential redaction, non-Error inputs, null inputs, cause preservation, short-SHA retry failure, and git init failure --- src/core/git/gitCommand.ts | 52 ++++++++++++++++++------- tests/core/git/gitCommand.test.ts | 65 +++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 15 deletions(-) diff --git a/src/core/git/gitCommand.ts b/src/core/git/gitCommand.ts index 0abbc6615..823e41430 100644 --- a/src/core/git/gitCommand.ts +++ b/src/core/git/gitCommand.ts @@ -18,28 +18,41 @@ interface ExecFileError extends Error { stderr?: string; } +/** + * Redacts embedded credentials from a URL to prevent sensitive information leakage. + * e.g., "https://user:password@github.com/repo.git" -> "https://***@github.com/repo.git" + */ +const redactUrl = (url: string): string => { + return url.replace(/^(https?:\/\/)([^@/]+)@/i, '$1***@'); +}; + export const createGitRemoteError = (error: unknown, url: string, operation: string): RepomixError => { - const err = error as ExecFileError; - const stderr = err.stderr || err.message || ''; + const err = error as Partial; + const message = err?.message || String(error); + const stderr = err?.stderr || message; + const safeUrl = redactUrl(url); // Timeout: process was killed by the timeout option - if (err.killed || err.signal === 'SIGTERM') { + if (err?.killed || err?.signal === 'SIGTERM') { return new RepomixError( - `Git ${operation} timed out after ${GIT_REMOTE_TIMEOUT / 1000} seconds for ${url}. The repository may be inaccessible, or the network connection is too slow.`, + `Git ${operation} timed out after ${GIT_REMOTE_TIMEOUT / 1000} seconds for ${safeUrl}. The repository may be inaccessible, or the network connection is too slow.`, + { cause: error }, ); } // Authentication failure if (stderr.includes('Authentication failed') || stderr.includes('could not read Username')) { return new RepomixError( - `Git ${operation} failed for ${url}: Authentication required. The repository may be private or the URL may be incorrect.`, + `Git ${operation} failed for ${safeUrl}: Authentication required. The repository may be private or the URL may be incorrect.`, + { cause: error }, ); } // Repository not found - if (stderr.includes('not found') || stderr.includes('does not exist') || stderr.includes('Repository not found')) { + if (stderr.includes('does not exist') || stderr.includes('Repository not found')) { return new RepomixError( - `Git ${operation} failed for ${url}: Repository not found. Please verify the URL is correct.`, + `Git ${operation} failed for ${safeUrl}: Repository not found. Please verify the URL is correct.`, + { cause: error }, ); } @@ -50,12 +63,13 @@ export const createGitRemoteError = (error: unknown, url: string, operation: str stderr.includes('Connection refused') ) { return new RepomixError( - `Git ${operation} failed for ${url}: Unable to connect to the remote host. Please check your network connection and the URL.`, + `Git ${operation} failed for ${safeUrl}: Unable to connect to the remote host. Please check your network connection and the URL.`, + { cause: error }, ); } // Generic fallback - return new RepomixError(`Git ${operation} failed for ${url}: ${err.message}`); + return new RepomixError(`Git ${operation} failed for ${safeUrl}: ${message}`, { cause: error }); }; export const execGitLogFilenames = async ( @@ -163,8 +177,12 @@ export const execGitShallowClone = async ( validateGitUrl(url); if (remoteBranch) { - await deps.execFileAsync('git', ['-C', directory, 'init']); - await deps.execFileAsync('git', ['-C', directory, 'remote', 'add', '--', 'origin', url]); + try { + await deps.execFileAsync('git', ['-C', directory, 'init']); + await deps.execFileAsync('git', ['-C', directory, 'remote', 'add', '--', 'origin', url]); + } catch (initErr) { + throw new RepomixError(`Failed to initialize local git repository for ${redactUrl(url)}`, { cause: initErr }); + } try { await deps.execFileAsync( 'git', @@ -174,8 +192,8 @@ export const execGitShallowClone = async ( await deps.execFileAsync('git', ['-C', directory, 'checkout', 'FETCH_HEAD']); } catch (err: unknown) { // Check for timeout first — no point retrying if the remote is unreachable - const execErr = err as ExecFileError; - if (execErr.killed || execErr.signal === 'SIGTERM') { + const execErr = err as Partial; + if (execErr?.killed || execErr?.signal === 'SIGTERM') { throw createGitRemoteError(err, url, 'fetch'); } @@ -200,8 +218,12 @@ export const execGitShallowClone = async ( // Maybe the error is due to a short SHA, let's try again // Can't use --depth 1 here as we need to fetch the specific commit - await deps.execFileAsync('git', ['-C', directory, 'fetch', 'origin'], gitRemoteOpts); - await deps.execFileAsync('git', ['-C', directory, 'checkout', remoteBranch]); + try { + await deps.execFileAsync('git', ['-C', directory, 'fetch', 'origin'], gitRemoteOpts); + await deps.execFileAsync('git', ['-C', directory, 'checkout', remoteBranch]); + } catch (retryErr) { + throw createGitRemoteError(retryErr, url, 'fetch'); + } } } else { try { diff --git a/tests/core/git/gitCommand.test.ts b/tests/core/git/gitCommand.test.ts index ccf2036d5..ff6257b1d 100644 --- a/tests/core/git/gitCommand.test.ts +++ b/tests/core/git/gitCommand.test.ts @@ -425,6 +425,7 @@ c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8\trefs/tags/v1.0.0 expect(result).toBeInstanceOf(RepomixError); expect(result.message).toContain('timed out after 30 seconds'); expect(result.message).toContain(url); + expect(result.cause).toBe(error); }); test('should create timeout error when SIGTERM signal is present', () => { @@ -497,6 +498,33 @@ c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8\trefs/tags/v1.0.0 expect(result.message).toContain('Some unknown git error'); expect(result.message).toContain('fetch'); expect(result.message).toContain(url); + expect(result.cause).toBe(error); + }); + + test('should redact credentials from URL in error messages', () => { + const urlWithCreds = 'https://user:password123@github.com/user/repo.git'; + const error = new Error('Some git error'); + const result = createGitRemoteError(error, urlWithCreds, 'clone'); + + expect(result).toBeInstanceOf(RepomixError); + expect(result.message).not.toContain('password123'); + expect(result.message).not.toContain('user:password123'); + expect(result.message).toContain('***@github.com'); + }); + + test('should handle non-Error inputs safely', () => { + const result = createGitRemoteError('string error', url, 'fetch'); + + expect(result).toBeInstanceOf(RepomixError); + expect(result.message).toContain('string error'); + expect(result.message).not.toContain('undefined'); + }); + + test('should handle null error input safely', () => { + const result = createGitRemoteError(null, url, 'clone'); + + expect(result).toBeInstanceOf(RepomixError); + expect(result.message).not.toContain('undefined'); }); }); @@ -571,5 +599,42 @@ c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8\trefs/tags/v1.0.0 execLsRemote('https://github.com/user/private-repo.git', { execFileAsync: mockFileExecAsync }), ).rejects.toThrow('Authentication required'); }); + + test('execGitShallowClone should throw RepomixError when short-SHA retry fails', async () => { + const shortSha = 'ce9b621'; + const refNotFoundError = new Error( + `Command failed: git fetch --depth 1 origin ${shortSha}\nfatal: couldn't find remote ref ${shortSha}`, + ); + const retryTimeoutError = Object.assign(new Error('Command failed: git fetch'), { + killed: true, + signal: 'SIGTERM', + }); + const mockFileExecAsync = vi + .fn() + .mockResolvedValueOnce({ stdout: '', stderr: '' }) // git init + .mockResolvedValueOnce({ stdout: '', stderr: '' }) // git remote add + .mockRejectedValueOnce(refNotFoundError) // git fetch --depth 1 (triggers short-SHA fallback) + .mockRejectedValueOnce(retryTimeoutError); // git fetch origin (retry fails with timeout) + + await expect( + execGitShallowClone('https://github.com/user/repo.git', '/tmp/repo', shortSha, { + execFileAsync: mockFileExecAsync, + }), + ).rejects.toThrow('timed out after 30 seconds'); + + expect(mockFileExecAsync).toHaveBeenCalledTimes(4); + }); + + test('execGitShallowClone should throw RepomixError when git init fails', async () => { + const mockFileExecAsync = vi.fn().mockRejectedValueOnce(new Error('permission denied')); + + await expect( + execGitShallowClone('https://github.com/user/repo.git', '/tmp/repo', 'main', { + execFileAsync: mockFileExecAsync, + }), + ).rejects.toThrow('Failed to initialize local git repository'); + + expect(mockFileExecAsync).toHaveBeenCalledTimes(1); + }); }); });