Replaced eslint-disable-next-line comments with explicit type guarding (checking instanceof Error and 'code') and used process.stderr.write instead of console.warn to comply with project linting rules natively.
Moved the isSubpath and resolveToRealPath imports to the top of the file to satisfy the static analysis bot, which failed to detect the existing import statement further down in the file.
- Reverted the 'isIdle' guard in AppContainer.tsx to ensure slash commands entered while the agent is busy are correctly processed or queued, preventing them from falling through as regular chat text.
- Enhanced the physical path validation in config.ts to gracefully handle 'mkdirSync' failures (e.g. EACCES). The CLI will now log a warning and return the lexically-validated path instead of throwing a misleading 'Security violation' via 'resolveToRealPath'.
If the user's home directory or global `.gemini` config directory has
restrictive permissions that prevent `realpathSync` from succeeding, the
CLI should not crash. Instead, it now gracefully degrades by omitting
the global config directory from the valid security boundaries for plan
directories, allowing the CLI to continue operating securely within the
project root.
Resolves security review findings:
- Reordered resolveToRealPath before mkdirSync to fully eliminate TOCTOU risks with symlink injection.
- Fail closed by re-throwing 'Security violation' errors instead of swallowing them.
- Replaced lint-disabler with process.stderr.write for legitimate fallback warnings.
- Used direct context string as LRUCache key to avoid collision with an extension potentially named 'default'.
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 addresses the final performance and usability review comments:
- **Performance:** Introduced `LRUCache` for `plansDirCache` and `initializedPlanDirs` to prevent redundant, synchronous filesystem calls to `Storage.getPlansDir` on every turn.
- **Performance:** Cached the resolved `realProjectRoot` in the `Storage` constructor, eliminating expensive synchronous symlink resolution calls during active command routing.
- **Usability:** Replaced hard `throw` with `console.warn` when `fs.mkdirSync` fails (e.g., `EACCES`, `EEXIST`), allowing the CLI to gracefully degrade and continue functioning rather than crashing the entire process.
- **Validation:** Updated `config.test.ts` to verify the exact warning messages emitted during filesystem failures.
This commit addresses several critical findings from the review bot:
- **Security:** Implemented defense-in-depth symlink resolution. Removed insecure string-based fallbacks in `Storage.getPlansDir` and added a mandatory `isSubpath` validation AFTER directory creation in `Config.getPlansDir` to prevent TOCTOU traversal attacks.
- **Architecture:** Fixed a race condition where active extension context was mutated synchronously in `AppContainer`, potentially corrupting concurrent background tasks. Mutation now occurs within the command execution pipeline.
- **Robustness:** Switched to canonical path checking for `plan` command detection to support aliases and subcommands.
- **Regressions:** Added a `planEnabled` guard to prevent unwanted directory creation when the planning feature is disabled.
- **Validation:** Added exhaustive unit tests covering sequential context switching, shared directory deduplication, and symlink security edge cases.
This removes the insecure ENOENT fallback in `Storage.getPlansDir` that could be exploited to bypass the `isSubpath` check via symlinks. The fallback was unnecessary because the underlying `resolveToRealPath` function (via `robustRealpath`) was recently updated to gracefully handle and resolve symlinks for non-existent target paths.
This addresses a potential TOCTOU vulnerability and edge case identified during review. The redundant `fs.existsSync` check in `getPlansDir` has been removed, allowing `fs.mkdirSync(..., { recursive: true })` to safely handle directory idempotency.
By relying directly on `mkdirSync`, we ensure that if a non-directory file already exists at the target path, the system will correctly throw an `EEXIST` error rather than silently treating the file as a directory and crashing later during workspace registration.
This commit addresses two bugs identified during review:
1. Cleared the sticky `activeExtensionContext` when the standard `/plan` command is executed, ensuring subsequent prompts correctly target the default global plan directory.
2. Fixed a path resolution regression in `Storage.getPlansDir()` by constructing the fallback ENOENT path directly against the real project root. This prevents `isSubpath` validation failures and potential traversal vulnerabilities when the project root is a symlink.
Updates prompts and tool implementations (edit, write-file, enter/exit plan mode) to route through Config.getPlansDir() instead of Storage.getPlansDir(). This ensures the plan directory is lazily created exactly when these features attempt to use it, preventing ENOENT failures.
Introduces active extension context tracking in config to support dynamic switching of plan directories. Resolves circular dependency in storage by deferring plan directory creation until on-demand use, preventing ENOENT errors on non-existent paths.
Mahima Shanware