diff --git a/src/filesystem/__tests__/lib.test.ts b/src/filesystem/__tests__/lib.test.ts index f7e585af22..05388b6621 100644 --- a/src/filesystem/__tests__/lib.test.ts +++ b/src/filesystem/__tests__/lib.test.ts @@ -308,6 +308,22 @@ describe('Lib Functions', () => { expect(mockFs.writeFile).toHaveBeenCalledWith('/test/file.txt', 'new content', { encoding: "utf-8", flag: 'wx' }); }); + + it('preserves file permissions when overwriting existing file', async () => { + // First writeFile call with 'wx' flag fails because file exists + mockFs.writeFile.mockRejectedValueOnce(Object.assign(new Error('EEXIST'), { code: 'EEXIST' })); + // stat returns executable permissions + mockFs.stat.mockResolvedValueOnce({ mode: 0o100755 }); + // Second writeFile (to temp) succeeds + mockFs.writeFile.mockResolvedValueOnce(undefined); + mockFs.rename.mockResolvedValueOnce(undefined); + mockFs.chmod.mockResolvedValueOnce(undefined); + + await writeFileContent('/test/script.sh', 'new content'); + + expect(mockFs.stat).toHaveBeenCalledWith('/test/script.sh'); + expect(mockFs.chmod).toHaveBeenCalledWith('/test/script.sh', 0o100755); + }); }); }); @@ -416,6 +432,8 @@ describe('Lib Functions', () => { beforeEach(() => { mockFs.readFile.mockResolvedValue('line1\nline2\nline3\n'); mockFs.writeFile.mockResolvedValue(undefined); + mockFs.stat.mockResolvedValue({ mode: 0o100644 }); + mockFs.chmod.mockResolvedValue(undefined); }); it('applies simple text replacement', async () => { @@ -494,6 +512,30 @@ describe('Lib Functions', () => { ); }); + it('preserves file permissions after applying edits', async () => { + mockFs.stat.mockResolvedValue({ mode: 0o100755 }); + const edits = [ + { oldText: 'line2', newText: 'modified line2' } + ]; + + mockFs.rename.mockResolvedValueOnce(undefined); + + await applyFileEdits('/test/script.sh', edits, false); + + expect(mockFs.stat).toHaveBeenCalledWith('/test/script.sh'); + expect(mockFs.chmod).toHaveBeenCalledWith('/test/script.sh', 0o100755); + }); + + it('does not restore permissions in dry run mode', async () => { + const edits = [ + { oldText: 'line2', newText: 'modified line2' } + ]; + + await applyFileEdits('/test/file.txt', edits, true); + + expect(mockFs.chmod).not.toHaveBeenCalled(); + }); + it('throws error for non-matching edits', async () => { const edits = [ { oldText: 'nonexistent line', newText: 'replacement' } diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index 17e4654cd5..64f384346a 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -168,10 +168,14 @@ export async function writeFileContent(filePath: string, content: string): Promi // Security: Use atomic rename to prevent race conditions where symlinks // could be created between validation and write. Rename operations // replace the target file atomically and don't follow symlinks. + const origStats = await fs.stat(filePath); const tempPath = `${filePath}.${randomBytes(16).toString('hex')}.tmp`; try { await fs.writeFile(tempPath, content, 'utf-8'); await fs.rename(tempPath, filePath); + // Restore original file permissions since the atomic rename replaces + // the inode and the temp file has default (0644) permissions. + await fs.chmod(filePath, origStats.mode); } catch (renameError) { try { await fs.unlink(tempPath); @@ -266,10 +270,14 @@ export async function applyFileEdits( // Security: Use atomic rename to prevent race conditions where symlinks // could be created between validation and write. Rename operations // replace the target file atomically and don't follow symlinks. + const origStats = await fs.stat(filePath); const tempPath = `${filePath}.${randomBytes(16).toString('hex')}.tmp`; try { await fs.writeFile(tempPath, modifiedContent, 'utf-8'); await fs.rename(tempPath, filePath); + // Restore original file permissions since the atomic rename replaces + // the inode and the temp file has default (0644) permissions. + await fs.chmod(filePath, origStats.mode); } catch (error) { try { await fs.unlink(tempPath);