Skip to content

Commit 29481a1

Browse files
authored
fix: robust ripgrep path resolution and 1p hermetic execution support (google-gemini#27253)
1 parent 2c85f57 commit 29481a1

4 files changed

Lines changed: 132 additions & 4 deletions

File tree

packages/core/src/tools/ripGrep.test.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1813,7 +1813,18 @@ describe('resolveRipgrepPath', () => {
18131813
);
18141814
});
18151815

1816-
it('should resolve the SEA (flattened) path first', async () => {
1816+
it('should resolve the SEA (purely flattened) path first', async () => {
1817+
vi.mocked(fileExists).mockImplementation(async (checkPath) => {
1818+
const expectedTarget = path.resolve(__dirname, `rg-linux-x64`);
1819+
return checkPath.includes(expectedTarget);
1820+
});
1821+
1822+
const resolvedPath = await resolveRipgrepPath();
1823+
expect(resolvedPath).not.toBeNull();
1824+
expect(resolvedPath).toContain('rg-linux-x64');
1825+
});
1826+
1827+
it('should resolve the SEA (vendor subdirectory) path if purely flattened is missing', async () => {
18171828
vi.mocked(fileExists).mockImplementation(async (checkPath) =>
18181829
checkPath.includes(path.normalize('vendor/ripgrep')),
18191830
);
@@ -1823,6 +1834,39 @@ describe('resolveRipgrepPath', () => {
18231834
expect(resolvedPath).toContain(path.normalize('vendor/ripgrep'));
18241835
});
18251836

1837+
it('should resolve the Dev/Dist layout (actual output with src/) if SEA path is missing', async () => {
1838+
vi.mocked(fileExists).mockImplementation(async (checkPath) => {
1839+
// Normalize the expected check against the absolute resolved path logic
1840+
const expectedTarget = path.resolve(
1841+
__dirname,
1842+
'../../../vendor/ripgrep',
1843+
);
1844+
return checkPath.includes(expectedTarget);
1845+
});
1846+
1847+
const resolvedPath = await resolveRipgrepPath();
1848+
expect(resolvedPath).not.toBeNull();
1849+
expect(resolvedPath).toContain('vendor');
1850+
});
1851+
1852+
it('should resolve the Dev/Dist layout (assumed output without src/) if others are missing', async () => {
1853+
vi.mocked(fileExists).mockImplementation(async (checkPath) => {
1854+
const expectedTarget = path.resolve(
1855+
__dirname,
1856+
'../../vendor/ripgrep',
1857+
);
1858+
const skipTarget = path.resolve(__dirname, '../../../vendor/ripgrep');
1859+
return (
1860+
checkPath.includes(expectedTarget) &&
1861+
!checkPath.includes(skipTarget)
1862+
);
1863+
});
1864+
1865+
const resolvedPath = await resolveRipgrepPath();
1866+
expect(resolvedPath).not.toBeNull();
1867+
expect(resolvedPath).toContain('vendor');
1868+
});
1869+
18261870
it('should fall back to system PATH if both bundled paths are missing and system is trusted', async () => {
18271871
vi.mocked(fileExists).mockResolvedValue(false);
18281872
vi.mocked(resolveExecutable).mockReturnValue('/usr/bin/rg');
@@ -1862,6 +1906,13 @@ describe('resolveRipgrepPath', () => {
18621906
const resolvedPath = await resolveRipgrepPath();
18631907
expect(resolvedPath).toBeNull();
18641908
});
1909+
1910+
it('should handle errors gracefully and return null', async () => {
1911+
vi.mocked(fileExists).mockRejectedValue(new Error('File system error'));
1912+
1913+
const resolvedPath = await resolveRipgrepPath();
1914+
expect(resolvedPath).toBeNull();
1915+
});
18651916
});
18661917

18671918
describe('on Windows', () => {

packages/core/src/tools/ripGrep.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,13 @@ export async function resolveRipgrepPath(): Promise<string | null> {
5959
const binName = `rg-${platform}-${arch}${platform === 'win32' ? '.exe' : ''}`;
6060

6161
const candidatePaths = [
62-
// 1. SEA runtime layout: everything is flattened into the root dir
62+
// 1. SEA runtime layout (Flattened): everything is in the root dir
63+
path.resolve(__dirname, binName),
64+
// 2. SEA runtime layout (Subdirectory): bundled into a vendor/ripgrep dir
6365
path.resolve(__dirname, 'vendor/ripgrep', binName),
64-
// 2. Dev/Dist layout: packages/core/dist/tools/ripGrep.js -> packages/core/vendor/ripgrep
66+
// 3. Dev/Dist layout (Actual): dist/src/tools/ripGrep.js -> packages/core/vendor/ripgrep
67+
path.resolve(__dirname, '../../../vendor/ripgrep', binName),
68+
// 4. Dev/Dist layout (Assumed/Bundled): dist/tools/ripGrep.js -> packages/core/vendor/ripgrep
6569
path.resolve(__dirname, '../../vendor/ripgrep', binName),
6670
];
6771

packages/core/src/utils/paths.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,24 @@ describe('normalizePath', () => {
811811
expect(isTrustedSystemPath(cwd)).toBe(false);
812812
});
813813

814+
it('should not reject paths if the current working directory is the root directory', () => {
815+
mockPlatform('linux');
816+
const originalCwd = process.cwd;
817+
process.cwd = vi.fn().mockReturnValue('/');
818+
expect(isTrustedSystemPath('/usr/bin/rg')).toBe(true);
819+
process.cwd = originalCwd;
820+
});
821+
822+
it('should not reject paths if the current working directory is a Windows root directory', () => {
823+
mockPlatform('win32');
824+
vi.stubEnv('SystemRoot', 'C:\\Windows');
825+
const originalCwd = process.cwd;
826+
process.cwd = vi.fn().mockReturnValue('C:\\');
827+
expect(isTrustedSystemPath('C:\\Windows\\System32\\rg.exe')).toBe(true);
828+
process.cwd = originalCwd;
829+
vi.unstubAllEnvs();
830+
});
831+
814832
it('should allow trusted paths on Windows', () => {
815833
mockPlatform('win32');
816834
vi.stubEnv('SystemRoot', 'C:\\Windows');
@@ -854,5 +872,50 @@ describe('normalizePath', () => {
854872
expect(isTrustedSystemPath('/tmp/rg')).toBe(false);
855873
expect(isTrustedSystemPath('/Library/rg')).toBe(false);
856874
});
875+
876+
it('should allow 1P internal hermetic execution paths', () => {
877+
mockPlatform('linux');
878+
879+
expect(isTrustedSystemPath('/google/bin/rg')).toBe(true);
880+
expect(
881+
isTrustedSystemPath(
882+
'/google/src/cloud/user/workspace/bazel-out/k8-fastbuild/bin/rg',
883+
),
884+
).toBe(true);
885+
expect(
886+
isTrustedSystemPath(
887+
'/google/src/cloud/user/workspace/blaze-out/k8-opt/bin/rg',
888+
),
889+
).toBe(true);
890+
});
891+
892+
describe('in secure hermetic environments', () => {
893+
const originalCwd = process.cwd;
894+
const cwd = '/sandbox';
895+
896+
beforeEach(() => {
897+
mockPlatform('linux');
898+
process.cwd = vi.fn().mockReturnValue(cwd);
899+
});
900+
901+
afterEach(() => {
902+
process.cwd = originalCwd;
903+
vi.unstubAllEnvs();
904+
});
905+
906+
it('should reject paths in the CWD by default', () => {
907+
expect(isTrustedSystemPath(path.join(cwd, 'bin/rg'))).toBe(false);
908+
});
909+
910+
it.each([
911+
['TEST_SRCDIR', '/mock/runfiles'],
912+
['BAZEL_TEST', '1'],
913+
['TEST_WORKSPACE', 'my_workspace'],
914+
['RUNFILES_DIR', '/mock/runfiles'],
915+
])('should bypass CWD rejection when %s is set', (envVar, value) => {
916+
vi.stubEnv(envVar, value);
917+
expect(isTrustedSystemPath(path.join(cwd, 'bin/rg'))).toBe(true);
918+
});
919+
});
857920
});
858921
});

packages/core/src/utils/paths.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -527,10 +527,17 @@ export function isTrustedSystemPath(filePath: string): boolean {
527527

528528
// 1. Explicitly reject paths in current working directory to prevent RCE
529529
// Exclude root directories to avoid inadvertently rejecting all system paths.
530+
// Bypass this restriction in secure, hermetic environments (e.g., Bazel/Blaze).
531+
const isHermeticEnv =
532+
!!process.env['TEST_SRCDIR'] ||
533+
!!process.env['TEST_WORKSPACE'] ||
534+
!!process.env['BAZEL_TEST'] ||
535+
!!process.env['RUNFILES_DIR'];
536+
530537
const normCwd = normalizePath(process.cwd());
531538
const isRoot = normCwd === '/' || /^[a-zA-Z]:[\\/]?$/.test(normCwd);
532539
if (!isRoot && isSubpath(normCwd, normPath)) {
533-
return false;
540+
return isHermeticEnv;
534541
}
535542

536543
// 2. Allow standard system directories
@@ -555,6 +562,9 @@ export function isTrustedSystemPath(filePath: string): boolean {
555562
'/usr/local/Cellar',
556563
'/usr/sbin',
557564
'/sbin',
565+
// 1P internal hermetic execution paths
566+
'/google/bin',
567+
'/google/src/cloud',
558568
].map((p) => normalizePath(p));
559569

560570
return trustedPrefixes.some(

0 commit comments

Comments
 (0)