Skip to content

Commit 34e7432

Browse files
authored
Merge pull request #1798 from yamadashy/security/pin-remote-clone-git-removal
test(security): Pin that a remote clone's .git is always removed
2 parents ebed8e7 + 729f712 commit 34e7432

2 files changed

Lines changed: 32 additions & 1 deletion

File tree

src/core/git/gitCommand.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,13 @@ export const execGitShallowClone = async (
185185
await deps.execFileAsync('git', ['clone', '--depth', '1', '--', url, directory], gitRemoteOpts);
186186
}
187187

188-
// Clean up .git directory
188+
// Drop the clone's .git. This keeps git internals out of the packed output, but
189+
// it is also load-bearing for safety, so keep it unconditional. Packing runs
190+
// `git -C <dir> log` by default (output.git.sortByChanges), and git honors the
191+
// repository's own .git/config — so a retained .git would let a cloned repo
192+
// execute commands on this host through keys such as gpg.program (reached via
193+
// log.showSignature). Making the removal conditional, for example to support
194+
// diffs on a remote repo, would reopen that path.
189195
await fs.rm(path.join(directory, '.git'), { recursive: true, force: true });
190196
};
191197

tests/core/git/gitCommand.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import fs from 'node:fs/promises';
2+
import os from 'node:os';
3+
import path from 'node:path';
14
import { beforeEach, describe, expect, test, vi } from 'vitest';
25
import {
36
execGitDiff,
@@ -126,6 +129,28 @@ file2.ts
126129
});
127130

128131
describe('execGitShallowClone', () => {
132+
// The .git removal is load-bearing for safety, not just tidiness: packing runs
133+
// `git log` inside the clone by default and git honors that repository's own
134+
// .git/config, so a retained .git would be a host command-execution vector.
135+
// Unlike the sibling tests this one touches a real directory, because the
136+
// guarantee is about what is left on disk rather than which git args ran.
137+
test('removes the clone .git so a cloned repository cannot supply git config', async () => {
138+
const mockFileExecAsync = vi.fn().mockResolvedValue({ stdout: '', stderr: '' });
139+
const directory = await fs.mkdtemp(path.join(os.tmpdir(), 'repomix-clone-git-'));
140+
await fs.mkdir(path.join(directory, '.git'), { recursive: true });
141+
await fs.writeFile(path.join(directory, '.git', 'config'), '[log]\n\tshowSignature = true\n');
142+
143+
try {
144+
await execGitShallowClone('https://github.com/user/repo.git', directory, undefined, {
145+
execFileAsync: mockFileExecAsync,
146+
});
147+
148+
await expect(fs.access(path.join(directory, '.git'))).rejects.toThrow();
149+
} finally {
150+
await fs.rm(directory, { recursive: true, force: true });
151+
}
152+
});
153+
129154
test('should execute without branch option if not specified by user', async () => {
130155
const mockFileExecAsync = vi.fn().mockResolvedValue({ stdout: '', stderr: '' });
131156
const url = 'https://github.com/user/repo.git';

0 commit comments

Comments
 (0)