mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-06-13 12:57:12 -07:00
fix(core): mitigate TOCTOU vulnerability in plan directory creation
This change addresses a critical security review finding regarding a Time-of-Check to Time-of-Use (TOCTOU) vulnerability. Previously, plan directory paths were validated using `isSubpath` before creation. However, an attacker could potentially replace a path component with a symlink pointing outside the project root exactly between validation and creation. By resolving the physical path *after* `fs.mkdirSync` using `resolveToRealPath` and then verifying it with `isSubpath`, we ensure that the actual directory created on disk resides safely within the workspace. Any violation results in a warning, and the malicious path is prevented from being added to the agent's `workspaceContext`.
This commit is contained in:
@@ -3367,6 +3367,46 @@ describe('Plans Directory Initialization', () => {
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('should log a warning if the resolved plan directory is outside the project root (TOCTOU security violation)', async () => {
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
|
||||
let isMalicious = false;
|
||||
vi.spyOn(fs, 'mkdirSync').mockImplementation(() => {
|
||||
// Simulate an attacker creating a symlink right after the initial check but before/during creation
|
||||
isMalicious = true;
|
||||
return undefined;
|
||||
});
|
||||
|
||||
// When Config.getPlansDir calls resolveToRealPath AFTER creation, it resolves to an outside path.
|
||||
const realpathSpy = vi
|
||||
.spyOn(fs, 'realpathSync')
|
||||
.mockImplementation((p: fs.PathLike) => {
|
||||
const pStr = p.toString();
|
||||
if (isMalicious && pStr.includes('plans'))
|
||||
return '/outside/the/project/root/plans';
|
||||
return pStr;
|
||||
});
|
||||
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
plan: true,
|
||||
});
|
||||
|
||||
await config.initialize();
|
||||
try {
|
||||
config.getPlansDir();
|
||||
} finally {
|
||||
realpathSpy.mockRestore();
|
||||
}
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringMatching(
|
||||
/Security violation: Resolved plan directory.*is outside the project root/,
|
||||
),
|
||||
);
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('should log a warning if mkdirSync fails during getPlansDir (e.g. EACCES)', async () => {
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
vi.spyOn(fs, 'mkdirSync').mockImplementation(() => {
|
||||
|
||||
@@ -2269,6 +2269,14 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
fs.mkdirSync(plansDir, { recursive: true });
|
||||
|
||||
const realPlansDir = resolveToRealPath(plansDir);
|
||||
const realProjectRoot = resolveToRealPath(this.getTargetDir());
|
||||
|
||||
if (!isSubpath(realProjectRoot, realPlansDir)) {
|
||||
throw new Error(
|
||||
`Security violation: Resolved plan directory '${realPlansDir}' is outside the project root '${realProjectRoot}'.`,
|
||||
);
|
||||
}
|
||||
|
||||
this.workspaceContext.addDirectory(realPlansDir);
|
||||
this.initializedPlanDirs.set(plansDir, true);
|
||||
} catch (e: unknown) {
|
||||
|
||||
Reference in New Issue
Block a user