mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-23 03:24:42 -07:00
fix: enforce folder trust for workspace settings, skills, and context (#17596)
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This commit is contained in:
@@ -918,6 +918,7 @@ export class Config {
|
||||
await this.getSkillManager().discoverSkills(
|
||||
this.storage,
|
||||
this.getExtensions(),
|
||||
this.isTrustedFolder(),
|
||||
);
|
||||
this.getSkillManager().setDisabledSkills(this.disabledSkills);
|
||||
|
||||
@@ -1924,6 +1925,7 @@ export class Config {
|
||||
await this.getSkillManager().discoverSkills(
|
||||
this.storage,
|
||||
this.getExtensions(),
|
||||
this.isTrustedFolder(),
|
||||
);
|
||||
this.getSkillManager().setDisabledSkills(this.disabledSkills);
|
||||
|
||||
|
||||
@@ -40,6 +40,7 @@ describe('ContextManager', () => {
|
||||
getMcpClientManager: vi.fn().mockReturnValue({
|
||||
getMcpInstructions: vi.fn().mockReturnValue('MCP Instructions'),
|
||||
}),
|
||||
isTrustedFolder: vi.fn().mockReturnValue(true),
|
||||
} as unknown as Config;
|
||||
|
||||
contextManager = new ContextManager(mockConfig);
|
||||
@@ -112,6 +113,24 @@ describe('ContextManager', () => {
|
||||
fileCount: 2,
|
||||
});
|
||||
});
|
||||
|
||||
it('should not load environment memory if folder is not trusted', async () => {
|
||||
vi.mocked(mockConfig.isTrustedFolder).mockReturnValue(false);
|
||||
const mockGlobalResult = {
|
||||
files: [
|
||||
{ path: '/home/user/.gemini/GEMINI.md', content: 'Global Content' },
|
||||
],
|
||||
};
|
||||
vi.mocked(memoryDiscovery.loadGlobalMemory).mockResolvedValue(
|
||||
mockGlobalResult,
|
||||
);
|
||||
|
||||
await contextManager.refresh();
|
||||
|
||||
expect(memoryDiscovery.loadEnvironmentMemory).not.toHaveBeenCalled();
|
||||
expect(contextManager.getEnvironmentMemory()).toBe('');
|
||||
expect(contextManager.getGlobalMemory()).toContain('Global Content');
|
||||
});
|
||||
});
|
||||
|
||||
describe('discoverContext', () => {
|
||||
@@ -150,5 +169,16 @@ describe('ContextManager', () => {
|
||||
|
||||
expect(result).toBe('');
|
||||
});
|
||||
|
||||
it('should return empty string if folder is not trusted', async () => {
|
||||
vi.mocked(mockConfig.isTrustedFolder).mockReturnValue(false);
|
||||
|
||||
const result = await contextManager.discoverContext('/app/src/file.ts', [
|
||||
'/app',
|
||||
]);
|
||||
|
||||
expect(memoryDiscovery.loadJitSubdirectoryMemory).not.toHaveBeenCalled();
|
||||
expect(result).toBe('');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -43,6 +43,10 @@ export class ContextManager {
|
||||
}
|
||||
|
||||
private async loadEnvironmentMemory(): Promise<void> {
|
||||
if (!this.config.isTrustedFolder()) {
|
||||
this.environmentMemory = '';
|
||||
return;
|
||||
}
|
||||
const result = await loadEnvironmentMemory(
|
||||
[...this.config.getWorkspaceContext().getDirectories()],
|
||||
this.config.getExtensionLoader(),
|
||||
@@ -68,6 +72,9 @@ export class ContextManager {
|
||||
accessedPath: string,
|
||||
trustedRoots: string[],
|
||||
): Promise<string> {
|
||||
if (!this.config.isTrustedFolder()) {
|
||||
return '';
|
||||
}
|
||||
const result = await loadJitSubdirectoryMemory(
|
||||
accessedPath,
|
||||
trustedRoots,
|
||||
@@ -101,9 +108,7 @@ export class ContextManager {
|
||||
}
|
||||
|
||||
private markAsLoaded(paths: string[]): void {
|
||||
for (const p of paths) {
|
||||
this.loadedPaths.add(p);
|
||||
}
|
||||
paths.forEach((p) => this.loadedPaths.add(p));
|
||||
}
|
||||
|
||||
getLoadedPaths(): ReadonlySet<string> {
|
||||
|
||||
@@ -78,13 +78,19 @@ description: project-desc
|
||||
};
|
||||
|
||||
vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue(userDir);
|
||||
vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-user-agent',
|
||||
);
|
||||
const storage = new Storage('/dummy');
|
||||
vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(projectDir);
|
||||
vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-project-agent',
|
||||
);
|
||||
|
||||
const service = new SkillManager();
|
||||
// @ts-expect-error accessing private method for testing
|
||||
vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined);
|
||||
await service.discoverSkills(storage, [mockExtension]);
|
||||
await service.discoverSkills(storage, [mockExtension], true);
|
||||
|
||||
const skills = service.getSkills();
|
||||
expect(skills).toHaveLength(3);
|
||||
@@ -135,13 +141,19 @@ description: project-desc
|
||||
};
|
||||
|
||||
vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue(userDir);
|
||||
vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-user-agent',
|
||||
);
|
||||
const storage = new Storage('/dummy');
|
||||
vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(projectDir);
|
||||
vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-project-agent',
|
||||
);
|
||||
|
||||
const service = new SkillManager();
|
||||
// @ts-expect-error accessing private method for testing
|
||||
vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined);
|
||||
await service.discoverSkills(storage, [mockExtension]);
|
||||
await service.discoverSkills(storage, [mockExtension], true);
|
||||
|
||||
const skills = service.getSkills();
|
||||
expect(skills).toHaveLength(1);
|
||||
@@ -149,7 +161,7 @@ description: project-desc
|
||||
|
||||
// Test User > Extension
|
||||
vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue('/non-existent');
|
||||
await service.discoverSkills(storage, [mockExtension]);
|
||||
await service.discoverSkills(storage, [mockExtension], true);
|
||||
expect(service.getSkills()[0].description).toBe('user-desc');
|
||||
});
|
||||
|
||||
@@ -173,7 +185,7 @@ description: project-desc
|
||||
vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue('/non-existent');
|
||||
vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue('/non-existent');
|
||||
|
||||
await service.discoverSkills(storage);
|
||||
await service.discoverSkills(storage, [], true);
|
||||
|
||||
const skills = service.getSkills();
|
||||
expect(skills).toHaveLength(1);
|
||||
@@ -196,12 +208,18 @@ body1`,
|
||||
|
||||
const storage = new Storage('/dummy');
|
||||
vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(testRootDir);
|
||||
vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-project-agent',
|
||||
);
|
||||
vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue('/non-existent');
|
||||
vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-user-agent',
|
||||
);
|
||||
|
||||
const service = new SkillManager();
|
||||
// @ts-expect-error accessing private method for testing
|
||||
vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined);
|
||||
await service.discoverSkills(storage);
|
||||
await service.discoverSkills(storage, [], true);
|
||||
service.setDisabledSkills(['skill1']);
|
||||
|
||||
expect(service.getSkills()).toHaveLength(0);
|
||||
@@ -209,6 +227,40 @@ body1`,
|
||||
expect(service.getAllSkills()[0].disabled).toBe(true);
|
||||
});
|
||||
|
||||
it('should skip workspace skills if folder is not trusted', async () => {
|
||||
const projectDir = path.join(testRootDir, 'workspace');
|
||||
await fs.mkdir(path.join(projectDir, 'skill-project'), { recursive: true });
|
||||
|
||||
await fs.writeFile(
|
||||
path.join(projectDir, 'skill-project', 'SKILL.md'),
|
||||
`---
|
||||
name: skill-project
|
||||
description: project-desc
|
||||
---
|
||||
`,
|
||||
);
|
||||
|
||||
const storage = new Storage('/dummy');
|
||||
vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(projectDir);
|
||||
vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-project-agent',
|
||||
);
|
||||
vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue('/non-existent');
|
||||
vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-user-agent',
|
||||
);
|
||||
|
||||
const service = new SkillManager();
|
||||
// @ts-expect-error accessing private method for testing
|
||||
vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined);
|
||||
|
||||
// Call with isTrusted = false
|
||||
await service.discoverSkills(storage, [], false);
|
||||
|
||||
const skills = service.getSkills();
|
||||
expect(skills).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should filter built-in skills in getDisplayableSkills', async () => {
|
||||
const service = new SkillManager();
|
||||
|
||||
@@ -303,14 +355,20 @@ body1`,
|
||||
});
|
||||
|
||||
vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue(userDir);
|
||||
vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-user-agent',
|
||||
);
|
||||
const storage = new Storage('/dummy');
|
||||
vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(projectDir);
|
||||
vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-project-agent',
|
||||
);
|
||||
|
||||
const service = new SkillManager();
|
||||
// @ts-expect-error accessing private method for testing
|
||||
vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined);
|
||||
|
||||
await service.discoverSkills(storage, []);
|
||||
await service.discoverSkills(storage, [], true);
|
||||
|
||||
expect(emitFeedbackSpy).toHaveBeenCalledWith(
|
||||
'warning',
|
||||
@@ -356,12 +414,18 @@ body1`,
|
||||
});
|
||||
|
||||
vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue(userDir);
|
||||
vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-user-agent',
|
||||
);
|
||||
const storage = new Storage('/dummy');
|
||||
vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue('/non-existent');
|
||||
vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue(
|
||||
'/non-existent-project-agent',
|
||||
);
|
||||
|
||||
const service = new SkillManager();
|
||||
|
||||
await service.discoverSkills(storage, []);
|
||||
await service.discoverSkills(storage, [], true);
|
||||
|
||||
// UI warning should not be called
|
||||
expect(emitFeedbackSpy).not.toHaveBeenCalled();
|
||||
|
||||
@@ -47,6 +47,7 @@ export class SkillManager {
|
||||
async discoverSkills(
|
||||
storage: Storage,
|
||||
extensions: GeminiCLIExtension[] = [],
|
||||
isTrusted: boolean = false,
|
||||
): Promise<void> {
|
||||
this.clearSkills();
|
||||
|
||||
@@ -71,6 +72,13 @@ export class SkillManager {
|
||||
this.addSkillsWithPrecedence(userAgentSkills);
|
||||
|
||||
// 4. Workspace skills (highest precedence)
|
||||
if (!isTrusted) {
|
||||
debugLogger.debug(
|
||||
'Workspace skills disabled because folder is not trusted.',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const projectSkills = await loadSkillsFromDir(
|
||||
storage.getProjectSkillsDir(),
|
||||
);
|
||||
|
||||
@@ -112,7 +112,7 @@ describe('SkillManager Alias', () => {
|
||||
// @ts-expect-error accessing private method for testing
|
||||
vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined);
|
||||
|
||||
await service.discoverSkills(storage, []);
|
||||
await service.discoverSkills(storage, [], true);
|
||||
|
||||
const skills = service.getSkills();
|
||||
expect(skills).toHaveLength(4);
|
||||
@@ -169,7 +169,7 @@ describe('SkillManager Alias', () => {
|
||||
// @ts-expect-error accessing private method for testing
|
||||
vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined);
|
||||
|
||||
await service.discoverSkills(storage, []);
|
||||
await service.discoverSkills(storage, [], true);
|
||||
|
||||
const skills = service.getSkills();
|
||||
expect(skills).toHaveLength(1);
|
||||
|
||||
Reference in New Issue
Block a user