Skip to content

Refactor config file handling in InitCommand #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
naheel0 merged 7 commits into from
Mar 23, 2026
Merged

Refactor config file handling in InitCommand #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
72d423b
Refactor config file handling in InitCommand
naheel0 Mar 23, 2026
9b8f8c9
Initial plan
Copilot Mar 23, 2026
a65b3a6
Fix fs.constants flags, add try/finally for fd, and use atomic writes…
Copilot Mar 23, 2026
1b0832f
Merge pull request #19 from BeyteFlow/copilot/sub-pr-18
naheel0 Mar 23, 2026
2c8ac73
Initial plan
Copilot Mar 23, 2026
ad25fa1
Fix tempPath leak: unlink on write/fsync failure in atomic write helpers
Copilot Mar 23, 2026
caf677c
Merge pull request #20 from BeyteFlow/copilot/sub-pr-18
naheel0 Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 63 additions & 32 deletions git-ai/src/commands/InitCommand.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { logger } from '../utils/logger.js';
async function readSecretInput(rl: readline.Interface, prompt: string): Promise<string> {
const rlAny = rl as any;
const originalWrite = rlAny._writeToOutput;
// Suppress character echoing: only allow the initial prompt to be written
let promptWritten = false;
rlAny._writeToOutput = function _writeToOutput(str: string) {
if (!promptWritten) {
Expand All @@ -29,6 +28,28 @@ async function readSecretInput(rl: readline.Interface, prompt: string): Promise<
}
}

/**
* Atomically writes content to filePath using a temp file + fsync + rename,
* ensuring the file has permissions 0o600.
*/
function atomicWriteFileSync(filePath: string, content: string): void {
const dir = path.dirname(filePath);
const tempPath = path.join(dir, `.tmp-${process.pid}-${Date.now()}`);
const fd = fs.openSync(tempPath, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY, 0o600);
try {
try {
fs.writeFileSync(fd, content);
fs.fsyncSync(fd);
} finally {
fs.closeSync(fd);
}
fs.renameSync(tempPath, filePath);
} catch (error) {
try { fs.unlinkSync(tempPath); } catch { /* best-effort cleanup */ }
throw error;
}
}

export async function initCommand() {
const rl = readline.createInterface({
input: process.stdin,
Expand All @@ -38,6 +59,7 @@ export async function initCommand() {
console.log('🚀 Welcome to AI-Git-Terminal Setup\n');

try {
// --- Step 1: Read API Key ---
let apiKey = '';
while (!apiKey) {
const apiKeyInput = await readSecretInput(rl, '🔑 Enter your Gemini API Key: ');
Expand All @@ -47,56 +69,65 @@ export async function initCommand() {
}
}

// --- Step 2: Read model name ---
const modelInput = await rl.question('🤖 Enter model name (default: gemini-1.5-flash): ');
const model = modelInput.trim() || 'gemini-1.5-flash';

// --- Step 3: Build config object ---
const newConfig: Config = {
ai: {
provider: 'gemini',
apiKey,
model: model,
},
git: {
autoStage: false,
},
ui: {
theme: 'dark',
showIcons: true,
},
ai: { provider: 'gemini', apiKey, model },
git: { autoStage: false },
ui: { theme: 'dark', showIcons: true },
};

// Validate with Zod and persist with restricted permissions (mode 0o600)
// Validate with Zod
ConfigSchema.parse(newConfig);

const configPath = path.join(os.homedir(), '.aigitrc');

if (fs.existsSync(configPath)) {
const overwriteChoice = (await rl.question(
'⚠️ Existing config found. Choose [o]verwrite, [b]ackup then replace, or [c]ancel: '
)).trim().toLowerCase();

if (overwriteChoice === 'b' || overwriteChoice === 'backup') {
const timestamp = new Date().toISOString().replace(/[:.]/g, '-');
const backupPath = `${configPath}.bak-${timestamp}`;
fs.renameSync(configPath, backupPath);
console.log(`📦 Existing config backed up to ${backupPath}`);
} else if (overwriteChoice === 'o' || overwriteChoice === 'overwrite') {
console.log('📝 Overwriting existing config file.');
} else {
console.log('🚫 Initialization canceled. Existing config left unchanged.');
return;
// --- Step 4: Attempt atomic creation ---
try {
const fd = fs.openSync(configPath, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_RDWR, 0o600);
try {
fs.writeFileSync(fd, JSON.stringify(newConfig, null, 2));
fs.fsyncSync(fd);
} finally {
fs.closeSync(fd);
}
console.log(`\n✅ Configuration saved to ${configPath}`);
console.log('Try running: ai-git commit');
return;
} catch (err: any) {
if (err.code !== 'EEXIST') throw err;
// File already exists, proceed to backup/overwrite prompt
}

fs.writeFileSync(configPath, JSON.stringify(newConfig, null, 2), { mode: 0o600 });
fs.chmodSync(configPath, 0o600);
// --- Step 5: Handle existing file ---
const overwriteChoice = (await rl.question(
'⚠️ Existing config found. Choose [o]verwrite, [b]ackup then replace, or [c]ancel: '
)).trim().toLowerCase();

if (overwriteChoice === 'b' || overwriteChoice === 'backup') {
const timestamp = new Date().toISOString().replace(/[:.]/g, '-');
const backupPath = `${configPath}.bak-${timestamp}`;
fs.renameSync(configPath, backupPath);
console.log(`📦 Existing config backed up to ${backupPath}`);
atomicWriteFileSync(configPath, JSON.stringify(newConfig, null, 2));
} else if (overwriteChoice === 'o' || overwriteChoice === 'overwrite') {
atomicWriteFileSync(configPath, JSON.stringify(newConfig, null, 2));
console.log('📝 Overwriting existing config file.');
Comment on lines +110 to +118
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect config write callsites to confirm replacement strategy.
rg -nP --type ts 'writeFileSync\(|renameSync\(|chmodSync\(' -C3

Repository: BeyteFlow/git-ai

Length of output: 2833

🏁 Script executed:

#!/bin/bash
# Check all fs.writeFileSync calls to understand the extent of the issue
rg -n 'fs\.writeFileSync' --type ts -B2 -A2

Repository: BeyteFlow/git-ai

Length of output: 1627

Use atomic writes (temp-file + rename) for config file updates to prevent corruption.

Lines 90 and 92 write directly to configPath using writeFileSync; an interruption during the write can leave a truncated or corrupted config file. Replace direct writes with atomic temp-file + rename pattern in both backup and overwrite branches.

Note: The same vulnerability exists in ConfigService.saveConfig() (line 58 of ConfigService.ts), which should also adopt atomic writes.

Suggested hardening approach 
+    const writeConfigAtomically = (targetPath: string, config: Config) => {
+      const tmpPath = `${targetPath}.tmp-${process.pid}`;
+      fs.writeFileSync(tmpPath, JSON.stringify(config, null, 2), { mode: 0o600 });
+      fs.renameSync(tmpPath, targetPath);
+      fs.chmodSync(targetPath, 0o600);
+    };

     if (overwriteChoice === 'b' || overwriteChoice === 'backup') {
       const timestamp = new Date().toISOString().replace(/[:.]/g, '-');
       const backupPath = `${configPath}.bak-${timestamp}`;
       fs.renameSync(configPath, backupPath);
       console.log(`📦 Existing config backed up to ${backupPath}`);
-      // Now write new config
-      fs.writeFileSync(configPath, JSON.stringify(newConfig, null, 2), { mode: 0o600 });
+      writeConfigAtomically(configPath, newConfig);
     } else if (overwriteChoice === 'o' || overwriteChoice === 'overwrite') {
-      fs.writeFileSync(configPath, JSON.stringify(newConfig, null, 2), { mode: 0o600 });
+      writeConfigAtomically(configPath, newConfig);
       console.log('📝 Overwriting existing config file.');
     } else {
       console.log('🚫 Initialization canceled. Existing config left unchanged.');
       return;
     }

-    fs.chmodSync(configPath, 0o600);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@git-ai/src/commands/InitCommand.ts` around lines 84 - 93, Both branches
(backup and overwrite) currently call fs.writeFileSync(configPath, ...) which
can corrupt the file if interrupted; instead perform an atomic write: create a
temp file in the same directory (e.g.,
`${configPath}.tmp-${process.pid}-${Date.now()}`), write the JSON to that temp
file, fsync and close it, set secure permissions, then fs.renameSync(tempPath,
configPath) to atomically replace the config; apply the same pattern in the
overwrite branch handling overwriteChoice and also update
ConfigService.saveConfig() to use the same temp-file + fsync + rename flow for
all config writes.

} else {
console.log('🚫 Initialization canceled. Existing config left unchanged.');
return;
}

console.log(`\n✅ Configuration saved to ${configPath}`);
console.log('Try running: ai-git commit');

} catch (error) {
logger.error('Failed to save configuration: ' + (error instanceof Error ? error.message : String(error)));
console.error('\n❌ Invalid input or failed to write config file.');
} finally {
rl.close();
}
}
}
16 changes: 15 additions & 1 deletion git-ai/src/services/ConfigService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,21 @@ export class ConfigService {

public saveConfig(newConfig: Config): void {
const validated = ConfigSchema.parse(newConfig);
fs.writeFileSync(ConfigService.CONFIG_PATH, JSON.stringify(validated, null, 2), { mode: 0o600 });
const configPath = ConfigService.CONFIG_PATH;
const tempPath = path.join(path.dirname(configPath), `.tmp-${process.pid}-${Date.now()}`);
const fd = fs.openSync(tempPath, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY, 0o600);
try {
try {
fs.writeFileSync(fd, JSON.stringify(validated, null, 2));
fs.fsyncSync(fd);
} finally {
fs.closeSync(fd);
}
fs.renameSync(tempPath, configPath);
} catch (error) {
try { fs.unlinkSync(tempPath); } catch { /* best-effort cleanup */ }
throw error;
}
this.config = validated;
}
}
Loading