From 6f7acdfdf515e7b120425b6f672402a9ed0b3ed2 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 4 Sep 2026 11:26:39 +0100 Subject: [PATCH] fix(ai): avoid shell execution for fixture tags The AI test-fixture fetcher interpolated the latest git tag from the external vertexai-sdk-test-data repository directly into an execSync shell string. A tag containing shell metacharacters (e.g. v17.0;id) would have its non-tag portion executed by the shell. Switch both git invocations to execFileSync with argument arrays and add a -- separator before positional repo/path args, so an attacker-controlled tag can never be parsed as shell syntax. Reported by Oskar Eichler, who also supplied the fix. --- .../ai/__tests__/fetchAiMockResponses.test.ts | 104 ++++++++++++++++++ scripts/fetch_ai_mock_responses.ts | 81 ++++++++------ 2 files changed, 152 insertions(+), 33 deletions(-) create mode 100644 packages/ai/__tests__/fetchAiMockResponses.test.ts diff --git a/packages/ai/__tests__/fetchAiMockResponses.test.ts b/packages/ai/__tests__/fetchAiMockResponses.test.ts new file mode 100644 index 0000000000..abe56aaede --- /dev/null +++ b/packages/ai/__tests__/fetchAiMockResponses.test.ts @@ -0,0 +1,104 @@ +import { execFileSync } from 'child_process'; +import { existsSync, readdirSync } from 'fs'; +import { rimrafSync } from 'rimraf'; +import { afterEach, describe, expect, it, jest } from '@jest/globals'; + +import { fetchAiMockResponses } from '../../../scripts/fetch_ai_mock_responses'; + +jest.mock('child_process', () => ({ + execFileSync: jest.fn(), +})); +jest.mock('fs', () => ({ + existsSync: jest.fn(), + readdirSync: jest.fn(), +})); +jest.mock('rimraf', () => ({ + rimrafSync: jest.fn(), +})); + +describe('fetchAiMockResponses', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('passes a repository-controlled tag to git without shell interpretation', () => { + const maliciousTag = 'v17.0;id'; + jest.mocked(existsSync).mockReturnValue(false); + jest + .mocked(execFileSync) + .mockReturnValueOnce(`hash refs/tags/${maliciousTag}`) + .mockReturnValueOnce(''); + + fetchAiMockResponses(); + + expect(execFileSync).toHaveBeenNthCalledWith( + 1, + 'git', + ['ls-remote', '--tags', '--sort=version:refname', expect.any(String)], + { encoding: 'utf8' }, + ); + expect(execFileSync).toHaveBeenNthCalledWith(2, 'git', [ + '-c', + 'advice.detachedHead=false', + 'clone', + '--branch', + maliciousTag, + '--', + expect.any(String), + expect.any(String), + ]); + expect(rimrafSync).toHaveBeenCalledTimes(1); + }); + + it('exits early without touching git when a matching clone already exists locally', () => { + const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit called'); + }); + jest.mocked(existsSync).mockReturnValue(true); + jest + .mocked(readdirSync) + .mockReturnValue([ + { name: 'vertexai-sdk-test-data_v17.0', isDirectory: () => true }, + ] as unknown as ReturnType); + + expect(() => fetchAiMockResponses()).toThrow('process.exit called'); + + expect(exitSpy).toHaveBeenCalledWith(0); + expect(execFileSync).not.toHaveBeenCalled(); + exitSpy.mockRestore(); + }); + + it('exits early without cloning when the target directory for the latest tag already exists', () => { + const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit called'); + }); + jest + .mocked(existsSync) + .mockReturnValueOnce(false) // no existing clone dir found yet + .mockReturnValueOnce(true); // target dir for the latest tag already exists + jest.mocked(execFileSync).mockReturnValueOnce('hash refs/tags/v18.0'); + + expect(() => fetchAiMockResponses()).toThrow('process.exit called'); + + expect(exitSpy).toHaveBeenCalledWith(0); + expect(execFileSync).toHaveBeenCalledTimes(1); // ls-remote only, never clone + exitSpy.mockRestore(); + }); + + it('cleans stale fixture data while preserving TypeScript sources and the latest tag directory', () => { + jest.mocked(existsSync).mockReturnValue(false); + jest.mocked(execFileSync).mockReturnValueOnce('hash refs/tags/v19.0').mockReturnValueOnce(''); + + fetchAiMockResponses(); + + const [, rimrafOptions] = jest.mocked(rimrafSync).mock.calls[0]; + const { filter } = rimrafOptions as unknown as { + filter: (path: string, stat: unknown) => boolean; + }; + + // filter returns true for entries rimraf should delete, false to keep them. + expect(filter('/root/keep.ts', {})).toBe(false); + expect(filter('/root/vertexai-sdk-test-data_v19.0/fixture.json', {})).toBe(false); + expect(filter('/root/vertexai-sdk-test-data_v18.0/stale.json', {})).toBe(true); + }); +}); diff --git a/scripts/fetch_ai_mock_responses.ts b/scripts/fetch_ai_mock_responses.ts index 8ed5bb8eaf..16ad0c56c6 100644 --- a/scripts/fetch_ai_mock_responses.ts +++ b/scripts/fetch_ai_mock_responses.ts @@ -16,7 +16,7 @@ // clone of the shared repository of Vertex AI test data. // eslint-disable-next-line @typescript-eslint/no-require-imports -import { execSync } from 'child_process'; +import { execFileSync } from 'child_process'; import { join } from 'path'; import { existsSync, readdirSync } from 'fs'; import { rimrafSync } from 'rimraf'; @@ -37,42 +37,57 @@ function findExistingCloneDirName(): string | undefined { .at(0); } -const existingCloneDirName = findExistingCloneDirName(); -if (existingCloneDirName !== undefined) { - console.log('AI mock responses data exists locally already. Exiting fetch script.'); - process.exit(0); -} +export function fetchAiMockResponses(): void { + const existingCloneDirName = findExistingCloneDirName(); + if (existingCloneDirName !== undefined) { + console.log('AI mock responses data exists locally already. Exiting fetch script.'); + process.exit(0); + } -// Get tags from repository, sorted by tag name, and coerce result to a string, then trim it -const repoTags = ( - execSync(`git ls-remote --tags --sort=version:refname "${REPO_LINK}"`) + '' -).trim(); + // Get tags from repository, sorted by tag name, and coerce result to a string, then trim it + const repoTags = execFileSync( + 'git', + ['ls-remote', '--tags', '--sort=version:refname', REPO_LINK], + { encoding: 'utf8' }, + ).trim(); -// Fish out just the tag name from the last line (since they are sorted already) -const latestTag = repoTags.split('/').at(-1); -if (latestTag === undefined) { - console.error('Unable to determine latest test data tag.'); - process.exit(1); -} + // Fish out just the tag name from the last line (since they are sorted already) + const latestTag = repoTags.split('/').at(-1); + if (latestTag === undefined) { + console.error('Unable to determine latest test data tag.'); + process.exit(1); + } -// Create the test data directory based on the latest tag -const cloneDirName = `${REPO_NAME}_${latestTag}`; -const cloneDirPath = join(TEST_DATA_ROOT, cloneDirName); + // Create the test data directory based on the latest tag + const cloneDirName = `${REPO_NAME}_${latestTag}`; + const cloneDirPath = join(TEST_DATA_ROOT, cloneDirName); -// Clean out any test data that isn't the latest test data -rimrafSync(TEST_DATA_ROOT, { - preserveRoot: false, - filter: (path: string, _) => !path.endsWith('.ts') && !path.includes(cloneDirName), -}); + // Clean out any test data that isn't the latest test data + rimrafSync(TEST_DATA_ROOT, { + preserveRoot: false, + filter: (path: string, _) => !path.endsWith('.ts') && !path.includes(cloneDirName), + }); -// Exit if our intended latest data clone target already exists -if (existsSync(cloneDirPath)) { - console.log('AI mock responses data exists locally already. Exiting fetch script.'); - process.exit(0); + // Exit if our intended latest data clone target already exists + if (existsSync(cloneDirPath)) { + console.log('AI mock responses data exists locally already. Exiting fetch script.'); + process.exit(0); + } + + // Clone the latest test data + console.log(`Fetching AI mock responses data...`); + execFileSync('git', [ + '-c', + 'advice.detachedHead=false', + 'clone', + '--branch', + latestTag, + '--', + REPO_LINK, + cloneDirPath, + ]); } -// Clone the latest test data -console.log(`Fetching AI mock responses data...`); -execSync( - `git -c advice.detachedHead=false clone --branch ${latestTag} ${REPO_LINK} ${cloneDirPath}`, -); +if (require.main === module) { + fetchAiMockResponses(); +}