diff --git a/packages/cli/src/config/extension.test.ts b/packages/cli/src/config/extension.test.ts index c1aa276aadf..b6decd99d5c 100644 --- a/packages/cli/src/config/extension.test.ts +++ b/packages/cli/src/config/extension.test.ts @@ -57,7 +57,9 @@ const mockGit = { fetch: vi.fn(), checkout: vi.fn(), listRemote: vi.fn(), - revparse: vi.fn(), + revparse: vi + .fn() + .mockResolvedValue('mock-sha-1234567890123456789012345678901234567890'), // Not a part of the actual API, but we need to use this to do the correct // file system interactions. path: vi.fn(), @@ -170,6 +172,9 @@ describe('extension tests', () => { beforeEach(() => { vi.clearAllMocks(); + mockGit.revparse.mockResolvedValue( + 'mock-sha-1234567890123456789012345678901234567890', + ); resetSettingsCacheForTesting(); keychainData = {}; mockKeychainStorage = { diff --git a/packages/cli/src/config/extensions/github.test.ts b/packages/cli/src/config/extensions/github.test.ts index 830506c0020..f9a508db690 100644 --- a/packages/cli/src/config/extensions/github.test.ts +++ b/packages/cli/src/config/extensions/github.test.ts @@ -91,6 +91,9 @@ describe('github.ts', () => { it('should clone, fetch and checkout a repo', async () => { mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); + mockGit.revparse.mockResolvedValue( + 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', + ); await cloneFromGit( { @@ -107,7 +110,93 @@ describe('github.ts', () => { ['--depth', '1'], ); expect(mockGit.fetch).toHaveBeenCalledWith('origin', 'v1.0.0'); - expect(mockGit.checkout).toHaveBeenCalledWith('FETCH_HEAD'); + expect(mockGit.revparse).toHaveBeenCalledWith(['FETCH_HEAD']); + expect(mockGit.checkout).toHaveBeenCalledWith( + 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', + ); + expect(mockGit.revparse).toHaveBeenCalledWith(['HEAD']); + }); + + it('should throw error if checked out SHA does not match target SHA', async () => { + mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); + // First call for FETCH_HEAD, second call for HEAD + mockGit.revparse + .mockResolvedValueOnce('a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2') + .mockResolvedValueOnce('different_malicious_sha'); + + await expect( + cloneFromGit( + { + type: 'git', + source: 'https://github.com/owner/repo.git', + ref: 'v1.0.0', + }, + '/dest', + ), + ).rejects.toThrow( + 'Security verification failed: checked out SHA (different_malicious_sha) does not match the target SHA (a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2)', + ); + }); + + it('should throw error if checked out SHA does not match pinned SHA', async () => { + mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); + mockGit.revparse.mockResolvedValue( + 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', + ); + + await expect( + cloneFromGit( + { + type: 'git', + source: 'https://github.com/owner/repo.git', + ref: 'e9f8d7c6b5a4e9f8d7c6b5a4e9f8d7c6b5a4e9f8', // Pinned SHA + }, + '/dest', + ), + ).rejects.toThrow( + 'Security verification failed: checked out SHA (a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2) does not match the requested pin (e9f8d7c6b5a4e9f8d7c6b5a4e9f8d7c6b5a4e9f8)', + ); + }); + + it('should succeed if checked out SHA matches pinned SHA', async () => { + mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); + mockGit.revparse.mockResolvedValue( + 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', + ); + + await cloneFromGit( + { + type: 'git', + source: 'https://github.com/owner/repo.git', + ref: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', // Pinned SHA matching + }, + '/dest', + ); + + expect(mockGit.checkout).toHaveBeenCalledWith( + 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', + ); + }); + + it('should succeed and skip SHA validation if ref is a hex-like branch/tag shorter than 40 characters', async () => { + mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); + mockGit.revparse.mockResolvedValue( + 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', + ); + + await cloneFromGit( + { + type: 'git', + source: 'https://github.com/owner/repo.git', + ref: 'deadbeef', // Looks like hex but is shorter than 40 chars (e.g. branch/tag) + }, + '/dest', + ); + + // Verify that checkout was still called with the resolved target SHA from FETCH_HEAD + expect(mockGit.checkout).toHaveBeenCalledWith( + 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', + ); }); it('should throw if no remotes found', async () => { diff --git a/packages/cli/src/config/extensions/github.ts b/packages/cli/src/config/extensions/github.ts index 06cf344a0d3..ba86e46362a 100644 --- a/packages/cli/src/config/extensions/github.ts +++ b/packages/cli/src/config/extensions/github.ts @@ -66,9 +66,35 @@ export async function cloneFromGit( await git.fetch(remotes[0].name, refToFetch); - // After fetching, checkout FETCH_HEAD to get the content of the fetched ref. + // Resolve FETCH_HEAD to its absolute commit SHA first to bypass any local branch named 'FETCH_HEAD'. + const targetSha = (await git.revparse(['FETCH_HEAD'])).trim(); + + // After fetching, checkout the resolved SHA to get the content of the fetched ref. // This results in a detached HEAD state, which is fine for this purpose. - await git.checkout('FETCH_HEAD'); + await git.checkout(targetSha); + + // Verify checkout integrity + const checkedOutSha = (await git.revparse(['HEAD'])).trim(); + if (checkedOutSha !== targetSha) { + throw new Error( + `Security verification failed: checked out SHA (${checkedOutSha}) does not match the target SHA (${targetSha}).`, + ); + } + + // If a specific ref was pinned and looks like a SHA, verify that the checked-out commit matches it. + if (installMetadata.ref) { + const refLower = installMetadata.ref.toLowerCase(); + // Match only full-length SHA-1 (40 characters) or SHA-256 (64 characters) hashes. + // This prevents short hex-only branch/tag names (e.g. ticket numbers or 'deadbeef') from triggering false-positive security errors. + const hexRegex = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/; + if (hexRegex.test(refLower)) { + if (checkedOutSha.toLowerCase() !== refLower) { + throw new Error( + `Security verification failed: checked out SHA (${checkedOutSha}) does not match the requested pin (${installMetadata.ref}).`, + ); + } + } + } } catch (error) { throw new Error( `Failed to clone Git repository from ${installMetadata.source} ${getErrorMessage(error)}`,