From 8d7230390cbabaa7a92ddcffafc9d989644ae0ac Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 5 Jul 2025 01:45:10 +0000 Subject: [PATCH 1/2] Fix git status parsing, model name, and prevent command injection Co-authored-by: arthurmedeiros32 --- BUGS_FOUND_AND_FIXED.md | 91 +++++++++++++++++++++++++++++++++++++++++ src/commands/init.ts | 14 ++++--- src/config/config.ts | 2 +- src/core/ai.ts | 4 +- src/core/git.ts | 3 +- 5 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 BUGS_FOUND_AND_FIXED.md diff --git a/BUGS_FOUND_AND_FIXED.md b/BUGS_FOUND_AND_FIXED.md new file mode 100644 index 0000000..f8ed288 --- /dev/null +++ b/BUGS_FOUND_AND_FIXED.md @@ -0,0 +1,91 @@ +# GitLift Codebase Bug Analysis and Fixes + +## Bug #1: Logic Error in Git Status Parsing + +**Location**: `src/core/git.ts` - `getUnstagedChanges()` function (lines 220-236) + +**Bug Type**: Logic Error + +**Description**: +The function incorrectly parses git status output for unstaged changes. The current logic checks for both `"M "` (staged but not modified in working tree) and `" M"` (modified in working tree but not staged) when looking for unstaged modified files. This creates confusion about what constitutes "unstaged" changes. + +**Impact**: +- Incorrectly identifies staged files as unstaged +- May cause the application to prompt users about files that are already staged +- Could lead to confusion in the commit workflow + +**Original Code**: +```typescript +if (trimmedLine.startsWith("M ") || trimmedLine.startsWith(" M")) { + unstagedModifiedFiles.push(trimmedLine.substring(2).trim()); +} +``` + +**Fix**: +Only check for `" M"` (modified in working tree but not staged) for unstaged changes. + +--- + +## Bug #2: Performance Issue - Incorrect AI Model Name + +**Location**: `src/core/ai.ts` - Default model parameter (lines 44, 89) + +**Bug Type**: Performance Issue / API Error + +**Description**: +The default model name `"gpt-4.1-mini"` is incorrect. This model doesn't exist in OpenAI's API, causing all AI generation calls to fail with a "model not found" error. + +**Impact**: +- Complete failure of AI functionality +- Users cannot generate PR content or commit messages +- API calls will consistently fail with model not found errors + +**Original Code**: +```typescript +modelName = "gpt-4.1-mini" +``` + +**Fix**: +Change to the correct model name `"gpt-4o-mini"` which is a valid OpenAI model. + +--- + +## Bug #3: Security Vulnerability - Command Injection in API Key Setup + +**Location**: `src/commands/init.ts` - `setupOpenAIKey()` function (lines 60-61) and `setupEditor()` function (lines 140-141) + +**Bug Type**: Security Vulnerability + +**Description**: +The functions directly interpolate user-provided input (API key and editor command) into shell commands without proper escaping or validation. If these inputs contain shell metacharacters (like backticks, semicolons, or pipes), it could lead to command injection vulnerabilities. + +**Impact**: +- Potential command injection if malicious input is provided +- Shell profile files could be corrupted +- Security risk if attackers can control the API key or editor command input + +**Original Code**: +```typescript +const exportLine = `\nexport OPENAI_API_KEY="${apiKey}"\n`; +const exportLine = `\nexport EDITOR="${editorCommand}"\n`; +``` + +**Fix**: +Properly escape the user input to prevent shell injection by using single quotes instead of double quotes and escaping any single quotes in the input values. + +--- + +## Summary + +All three bugs have been fixed: +1. **Logic Error**: Fixed git status parsing to correctly identify unstaged files +2. **Performance Issue**: Corrected the AI model name to a valid OpenAI model in multiple locations +3. **Security Vulnerability**: Added proper escaping to prevent command injection for both API key and editor command inputs + +## Additional Fixes Applied + +- Updated `src/config/config.ts` to use the correct default model name +- Updated `src/commands/init.ts` to show correct model options in the setup wizard +- Applied the same security fix to the editor setup function + +These fixes improve the reliability, functionality, and security of the GitLift application. \ No newline at end of file diff --git a/src/commands/init.ts b/src/commands/init.ts index 2e82a66..1a0d02e 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -67,7 +67,9 @@ async function setupOpenAIKey() { const profile = shell.includes("zsh") ? ".zshrc" : ".bashrc"; const profilePath = join(process.env.HOME || "", profile); - const exportLine = `\nexport OPENAI_API_KEY="${apiKey}"\n`; + // Properly escape the API key to prevent shell injection + const escapedApiKey = apiKey.replace(/'/g, "'\"'\"'"); + const exportLine = `\nexport OPENAI_API_KEY='${escapedApiKey}'\n`; await writeFile(profilePath, exportLine, { flag: "a" }); console.log(theme.success(`✓ Added OPENAI_API_KEY to ${profile}`)); @@ -173,7 +175,9 @@ async function setupEditor() { const profile = shell.includes("zsh") ? ".zshrc" : ".bashrc"; const profilePath = join(process.env.HOME || "", profile); - const exportLine = `\nexport EDITOR="${editorCommand}"\n`; + // Properly escape the editor command to prevent shell injection + const escapedEditorCommand = editorCommand.replace(/'/g, "'\"'\"'"); + const exportLine = `\nexport EDITOR='${escapedEditorCommand}'\n`; await writeFile(profilePath, exportLine, { flag: "a" }); console.log(theme.success(`✓ Added EDITOR to ${profile}`)); @@ -287,11 +291,11 @@ async function handleInit(options: InitOptions) { name: "model", message: "Preferred OpenAI model:", choices: [ - { name: "GPT-4 Turbo (Recommended)", value: "gpt-4.1-mini" }, + { name: "GPT-4o Mini (Recommended)", value: "gpt-4o-mini" }, { name: "GPT-4o", value: "gpt-4o" }, - { name: "GPT-4o Mini (Faster/Cheaper)", value: "gpt-4o-mini" }, + { name: "GPT-4 Turbo", value: "gpt-4-turbo" }, ], - default: "gpt-4.1-mini", + default: "gpt-4o-mini", }, { type: "list", diff --git a/src/config/config.ts b/src/config/config.ts index 93e278e..a75b88c 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -18,7 +18,7 @@ export type AppConfig = z.infer; // Define default configuration values (make this partial, required applied later) export const defaultConfig: Partial = { baseBranch: "main", - model: "gpt-4.1-mini", + model: "gpt-4o-mini", skipConfirmations: false, language: "english", // Default language is English }; diff --git a/src/core/ai.ts b/src/core/ai.ts index 0e61aca..3011e3e 100644 --- a/src/core/ai.ts +++ b/src/core/ai.ts @@ -49,7 +49,7 @@ export const CommitMessageSchema = z.object({ export async function generatePrContent( diff: string, commits?: string, - modelName = "gpt-4.1-mini", + modelName = "gpt-4o-mini", language = "english", ): Promise> { const spinner = ora( @@ -95,7 +95,7 @@ export async function generatePrContent( */ export async function generateCommitMessageContent( stagedDiff: string, - modelName = "gpt-4.1-mini", + modelName = "gpt-4o-mini", language = "english", ): Promise> { const spinner = ora( diff --git a/src/core/git.ts b/src/core/git.ts index cfa799b..cbfe490 100644 --- a/src/core/git.ts +++ b/src/core/git.ts @@ -252,7 +252,8 @@ export async function getUnstagedChanges(): Promise<{ const lines = statusOutput.trim().split("\n"); for (const line of lines) { const trimmedLine = line.trim(); - if (trimmedLine.startsWith("M ") || trimmedLine.startsWith(" M")) { + if (trimmedLine.startsWith(" M")) { + // Only files modified in working tree but not staged unstagedModifiedFiles.push(trimmedLine.substring(2).trim()); } else if (trimmedLine.startsWith("??")) { untrackedFiles.push(trimmedLine.substring(2).trim()); From 50d5ccbfb921527b31d21888fed4b4c7ebf82fc7 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 5 Jul 2025 01:49:35 +0000 Subject: [PATCH 2/2] Fix bugs in init process, improve config validation, and enhance setup experience Co-authored-by: arthurmedeiros32 --- BUGS_FOUND_AND_FIXED.md | 91 ------------------------- src/commands/init.ts | 143 ++++++++++++++++++++++++++++++++++++---- src/config/config.ts | 2 +- src/core/ai.ts | 4 +- 4 files changed, 132 insertions(+), 108 deletions(-) delete mode 100644 BUGS_FOUND_AND_FIXED.md diff --git a/BUGS_FOUND_AND_FIXED.md b/BUGS_FOUND_AND_FIXED.md deleted file mode 100644 index f8ed288..0000000 --- a/BUGS_FOUND_AND_FIXED.md +++ /dev/null @@ -1,91 +0,0 @@ -# GitLift Codebase Bug Analysis and Fixes - -## Bug #1: Logic Error in Git Status Parsing - -**Location**: `src/core/git.ts` - `getUnstagedChanges()` function (lines 220-236) - -**Bug Type**: Logic Error - -**Description**: -The function incorrectly parses git status output for unstaged changes. The current logic checks for both `"M "` (staged but not modified in working tree) and `" M"` (modified in working tree but not staged) when looking for unstaged modified files. This creates confusion about what constitutes "unstaged" changes. - -**Impact**: -- Incorrectly identifies staged files as unstaged -- May cause the application to prompt users about files that are already staged -- Could lead to confusion in the commit workflow - -**Original Code**: -```typescript -if (trimmedLine.startsWith("M ") || trimmedLine.startsWith(" M")) { - unstagedModifiedFiles.push(trimmedLine.substring(2).trim()); -} -``` - -**Fix**: -Only check for `" M"` (modified in working tree but not staged) for unstaged changes. - ---- - -## Bug #2: Performance Issue - Incorrect AI Model Name - -**Location**: `src/core/ai.ts` - Default model parameter (lines 44, 89) - -**Bug Type**: Performance Issue / API Error - -**Description**: -The default model name `"gpt-4.1-mini"` is incorrect. This model doesn't exist in OpenAI's API, causing all AI generation calls to fail with a "model not found" error. - -**Impact**: -- Complete failure of AI functionality -- Users cannot generate PR content or commit messages -- API calls will consistently fail with model not found errors - -**Original Code**: -```typescript -modelName = "gpt-4.1-mini" -``` - -**Fix**: -Change to the correct model name `"gpt-4o-mini"` which is a valid OpenAI model. - ---- - -## Bug #3: Security Vulnerability - Command Injection in API Key Setup - -**Location**: `src/commands/init.ts` - `setupOpenAIKey()` function (lines 60-61) and `setupEditor()` function (lines 140-141) - -**Bug Type**: Security Vulnerability - -**Description**: -The functions directly interpolate user-provided input (API key and editor command) into shell commands without proper escaping or validation. If these inputs contain shell metacharacters (like backticks, semicolons, or pipes), it could lead to command injection vulnerabilities. - -**Impact**: -- Potential command injection if malicious input is provided -- Shell profile files could be corrupted -- Security risk if attackers can control the API key or editor command input - -**Original Code**: -```typescript -const exportLine = `\nexport OPENAI_API_KEY="${apiKey}"\n`; -const exportLine = `\nexport EDITOR="${editorCommand}"\n`; -``` - -**Fix**: -Properly escape the user input to prevent shell injection by using single quotes instead of double quotes and escaping any single quotes in the input values. - ---- - -## Summary - -All three bugs have been fixed: -1. **Logic Error**: Fixed git status parsing to correctly identify unstaged files -2. **Performance Issue**: Corrected the AI model name to a valid OpenAI model in multiple locations -3. **Security Vulnerability**: Added proper escaping to prevent command injection for both API key and editor command inputs - -## Additional Fixes Applied - -- Updated `src/config/config.ts` to use the correct default model name -- Updated `src/commands/init.ts` to show correct model options in the setup wizard -- Applied the same security fix to the editor setup function - -These fixes improve the reliability, functionality, and security of the GitLift application. \ No newline at end of file diff --git a/src/commands/init.ts b/src/commands/init.ts index 1a0d02e..8cd71b4 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -41,6 +41,9 @@ async function setupOpenAIKey() { console.log( theme.info("Get your API key at: https://platform.openai.com/api-keys"), ); + console.log( + theme.dim("Your API key should start with 'sk-' followed by additional characters"), + ); const { apiKey } = await inquirer.prompt([ { @@ -48,7 +51,18 @@ async function setupOpenAIKey() { name: "apiKey", message: "Enter your OpenAI API Key:", mask: "*", - validate: (input) => input.length > 0 || "API Key is required", + validate: (input) => { + if (!input || input.length === 0) { + return "API Key is required"; + } + if (!input.startsWith("sk-")) { + return "API Key should start with 'sk-'"; + } + if (input.length < 20) { + return "API Key appears to be too short"; + } + return true; + }, }, ]); @@ -154,7 +168,16 @@ async function setupEditor() { type: "input", name: "customEditor", message: "Enter editor command:", - validate: (input) => input.length > 0 || "Editor command is required", + validate: (input) => { + if (!input || input.length === 0) { + return "Editor command is required"; + } + // Basic validation - check if it looks like a valid command + if (input.includes("&&") || input.includes("||") || input.includes(";")) { + return "Editor command should not contain shell operators"; + } + return true; + }, }, ]); editorCommand = customEditor; @@ -212,26 +235,68 @@ async function createConfigFile(config: InitConfig, isGlobal: boolean) { async function testConfiguration(config: InitConfig) { const spinner = ora("Testing configuration...").start(); + const testResults = { + openai: false, + github: false, + git: false, + }; try { // Test OpenAI API spinner.text = "Testing OpenAI API connection..."; - const testPrompt = "test"; - // Note: This would be a minimal API call to test connectivity - // For now, we'll just check if the key exists if (!process.env.OPENAI_API_KEY) { throw new Error("OpenAI API Key not found"); } + try { + // Make a simple API call to test the key + const { openai } = await import("@ai-sdk/openai"); + const { generateText } = await import("ai"); + + await generateText({ + model: openai(config.model), + prompt: "Hello", + maxTokens: 5, + }); + + testResults.openai = true; + spinner.text = theme.success("✓ OpenAI API connection successful"); + } catch (apiError) { + console.log(theme.warning("\n⚠️ OpenAI API test failed")); + console.log(theme.dim("This might be due to invalid API key or network issues")); + console.log(theme.dim("You can continue, but AI features may not work")); + } + // Test GitHub CLI - spinner.text = "Testing GitHub CLI..."; - await $`gh auth status`.quiet(); + spinner.text = "Testing GitHub CLI authentication..."; + try { + await $`gh auth status`.quiet(); + testResults.github = true; + spinner.text = theme.success("✓ GitHub CLI authenticated"); + } catch (ghError) { + console.log(theme.warning("\n⚠️ GitHub CLI test failed")); + console.log(theme.dim("You may need to run 'gh auth login' later")); + } // Test Git - spinner.text = "Testing Git..."; - await $`git status`.quiet(); + spinner.text = "Testing Git repository..."; + try { + await $`git status`.quiet(); + testResults.git = true; + spinner.text = theme.success("✓ Git repository detected"); + } catch (gitError) { + console.log(theme.warning("\n⚠️ Git test failed")); + console.log(theme.dim("Make sure you're in a Git repository")); + } + + const successCount = Object.values(testResults).filter(Boolean).length; + const totalTests = Object.keys(testResults).length; - spinner.succeed(theme.success("✓ All configurations working correctly!")); + if (successCount === totalTests) { + spinner.succeed(theme.success("✓ All configurations working correctly!")); + } else { + spinner.succeed(theme.warning(`✓ Setup completed (${successCount}/${totalTests} tests passed)`)); + } } catch (error) { spinner.fail(theme.error("⚠️ Configuration test failed")); console.log( @@ -250,12 +315,28 @@ async function handleInit(options: InitOptions) { "This will guide you through setting up GitLift for your project.", ), ); + console.log( + theme.dim( + "You can exit at any time with Ctrl+C and run 'gitlift init' again.", + ), + ); + + const setupProgress = { + prerequisites: false, + openai: false, + github: false, + editor: false, + config: false, + test: false, + }; try { // Step 1: Check basic prerequisites console.log(theme.info("\n📋 Step 1: Checking prerequisites...")); + console.log(theme.dim("Verifying Git and GitHub CLI are installed")); try { await checkPrerequisites(); + setupProgress.prerequisites = true; } catch (error) { console.log( theme.warning("⚠️ Some prerequisites are missing. Let's set them up!"), @@ -264,18 +345,25 @@ async function handleInit(options: InitOptions) { // Step 2: Setup OpenAI API Key console.log(theme.info("\n🔑 Step 2: OpenAI API Key setup...")); + console.log(theme.dim("This is required for AI-powered content generation")); await setupOpenAIKey(); + setupProgress.openai = true; // Step 3: Setup GitHub Authentication console.log(theme.info("\n🐙 Step 3: GitHub CLI authentication...")); + console.log(theme.dim("This is needed to create pull requests")); await setupGitHubAuth(); + setupProgress.github = true; // Step 4: Setup Editor console.log(theme.info("\n✏️ Step 4: Editor configuration...")); + console.log(theme.dim("Choose your preferred editor for reviewing generated content")); await setupEditor(); + setupProgress.editor = true; // Step 5: Configuration wizard console.log(theme.info("\n⚙️ Step 5: GitLift configuration...")); + console.log(theme.dim("Customize GitLift settings for your workflow")); const detectedBranch = await detectGitInfo(); @@ -285,17 +373,18 @@ async function handleInit(options: InitOptions) { name: "baseBranch", message: "Default base branch for PRs:", default: detectedBranch, + validate: (input) => input.length > 0 || "Base branch is required", }, { type: "list", name: "model", message: "Preferred OpenAI model:", choices: [ - { name: "GPT-4o Mini (Recommended)", value: "gpt-4o-mini" }, + { name: "GPT-4 Turbo (Recommended)", value: "gpt-4.1-mini" }, { name: "GPT-4o", value: "gpt-4o" }, - { name: "GPT-4 Turbo", value: "gpt-4-turbo" }, + { name: "GPT-4o Mini (Faster/Cheaper)", value: "gpt-4o-mini" }, ], - default: "gpt-4o-mini", + default: "gpt-4.1-mini", }, { type: "list", @@ -305,6 +394,8 @@ async function handleInit(options: InitOptions) { { name: "English", value: "english" }, { name: "Português", value: "portuguese" }, { name: "Español", value: "spanish" }, + { name: "Français", value: "french" }, + { name: "Deutsch", value: "german" }, { name: "Other (specify)", value: "other" }, ], default: "english", @@ -329,20 +420,35 @@ async function handleInit(options: InitOptions) { config.language = customLanguage; } + setupProgress.config = true; + // Step 6: Save configuration console.log(theme.info("\n💾 Step 6: Saving configuration...")); await createConfigFile(config as InitConfig, options.global); // Step 7: Test configuration console.log(theme.info("\n🧪 Step 7: Testing configuration...")); + console.log(theme.dim("Verifying all components are working correctly")); await testConfiguration(config as InitConfig); + setupProgress.test = true; // Success message console.log(theme.success("\n✨ GitLift setup completed successfully!")); - console.log(theme.info("\nNext steps:")); + + // Setup summary + const completedSteps = Object.values(setupProgress).filter(Boolean).length; + const totalSteps = Object.keys(setupProgress).length; + console.log(theme.info(`\n📊 Setup Summary: ${completedSteps}/${totalSteps} steps completed`)); + + console.log(theme.info("\n🚀 Next steps:")); console.log(theme.dim("• Try: gitlift generate pr")); console.log(theme.dim("• Try: gitlift generate commit")); console.log(theme.dim("• Docs: https://github.com/arthurbm/gitlift")); + + console.log(theme.info("\n💡 Tips:")); + console.log(theme.dim("• Use --help flag to see all available options")); + console.log(theme.dim("• Config file location: " + (options.global ? "~/.gitliftrc.json" : "./.gitliftrc.json"))); + console.log(theme.dim("• Re-run 'gitlift init' anytime to update settings")); } catch (error: unknown) { if (error instanceof Error) { console.error(theme.error(`\n❌ Setup failed: ${error.message}`)); @@ -352,6 +458,15 @@ async function handleInit(options: InitOptions) { error, ); } + + // Show troubleshooting tips + console.log(theme.info("\n🔧 Troubleshooting tips:")); + console.log(theme.dim("• Make sure you're in a Git repository")); + console.log(theme.dim("• Check your internet connection")); + console.log(theme.dim("• Verify your OpenAI API key is valid")); + console.log(theme.dim("• Run 'gh auth login' if GitHub CLI auth fails")); + console.log(theme.dim("• Try running 'gitlift init' again")); + process.exit(1); } } diff --git a/src/config/config.ts b/src/config/config.ts index a75b88c..93e278e 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -18,7 +18,7 @@ export type AppConfig = z.infer; // Define default configuration values (make this partial, required applied later) export const defaultConfig: Partial = { baseBranch: "main", - model: "gpt-4o-mini", + model: "gpt-4.1-mini", skipConfirmations: false, language: "english", // Default language is English }; diff --git a/src/core/ai.ts b/src/core/ai.ts index 3011e3e..0e61aca 100644 --- a/src/core/ai.ts +++ b/src/core/ai.ts @@ -49,7 +49,7 @@ export const CommitMessageSchema = z.object({ export async function generatePrContent( diff: string, commits?: string, - modelName = "gpt-4o-mini", + modelName = "gpt-4.1-mini", language = "english", ): Promise> { const spinner = ora( @@ -95,7 +95,7 @@ export async function generatePrContent( */ export async function generateCommitMessageContent( stagedDiff: string, - modelName = "gpt-4o-mini", + modelName = "gpt-4.1-mini", language = "english", ): Promise> { const spinner = ora(