feat(policy): add --policy flag for user defined policies#18500
feat(policy): add --policy flag for user defined policies#18500allenhutchison wants to merge 2 commits intomainfrom
Conversation
Summary of ChangesHello @allenhutchison, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the policy management capabilities of the Gemini CLI by introducing a Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable --policy flag, allowing users to specify custom policy files and directories. The implementation is well-structured, with updates to argument parsing, settings schemas, and the core policy loading logic. The ability to handle both individual files and directories in the toml-loader is a great enhancement.
I've found one critical issue in packages/core/src/policy/toml-loader.ts related to an uninitialized variable that could lead to a crash, along with an inconsistency in file handling. My review comment provides a detailed explanation and a suggested fix.
Once that is addressed, this will be a solid feature addition to the CLI.
| let filesToLoad: string[]; | ||
| let baseDir: string; | ||
|
|
||
| try { | ||
| const dirEntries = await fs.readdir(dir, { withFileTypes: true }); | ||
| filesToLoad = dirEntries | ||
| .filter((entry) => entry.isFile() && entry.name.endsWith('.toml')) | ||
| .map((entry) => entry.name); | ||
| const stats = await fs.stat(p); | ||
| if (stats.isDirectory()) { | ||
| baseDir = p; | ||
| const dirEntries = await fs.readdir(p, { withFileTypes: true }); | ||
| filesToLoad = dirEntries | ||
| .filter((entry) => entry.isFile() && entry.name.endsWith('.toml')) | ||
| .map((entry) => entry.name); | ||
| } else if (stats.isFile()) { | ||
| baseDir = path.dirname(p); | ||
| filesToLoad = [path.basename(p)]; | ||
| } else { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
There's a critical bug here that can cause the application to crash. The filesToLoad variable is declared but not initialized. If fs.stat(p) fails with an error (e.g., permission denied), or if the path p is not a file or directory (e.g., a symlink), one of the continue statements will be executed. This leaves filesToLoad as undefined, causing a TypeError on line 263 when the code attempts to iterate over it.
Additionally, there's an inconsistency in how files are handled. When scanning a directory, only files ending with .toml are processed, while others are silently ignored. However, when an individual file path is provided, it's processed regardless of its extension, which will lead to a parsing error if it's not a TOML file. For consistent behavior, individual files should also be filtered to only include .toml files.
Both issues can be resolved by initializing filesToLoad and adding a check for the .toml extension for individual files.
let filesToLoad: string[] = [];
let baseDir = '';
try {
const stats = await fs.stat(p);
if (stats.isDirectory()) {
baseDir = p;
const dirEntries = await fs.readdir(p, { withFileTypes: true });
filesToLoad = dirEntries
.filter((entry) => entry.isFile() && entry.name.endsWith('.toml'))
.map((entry) => entry.name);
} else if (stats.isFile() && p.endsWith('.toml')) {
baseDir = path.dirname(p);
filesToLoad = [path.basename(p)];
}
// Other file types or non-toml files are silently ignored for consistency.
|
Size Change: +2.04 kB (+0.01%) Total Size: 23.8 MB
ℹ️ View Unchanged
|
gemini-cli
bot
added
the
🔒 maintainer only
1 participant
Summary
Add a
--policyflag to the Gemini CLI to allow users to specify additional policy files or directories. These user-provided policies replace the standard user policy directory (~/.gemini/policies), giving users full control over the policy engine's rules for a specific session or setup.Details
--policyflag to CLI arguments.PolicySettingsand settings schema to includepolicyPaths.loadPoliciesFromTomlto support individual file paths in addition to directories.createPolicyEngineConfigto prioritizepolicyPathsand exclude the default user policy directory when they are present.createNonInteractiveUIto outputinfo,warning, anderrormessages to the console, enabling/policies listto work in headless mode.Related Issues
Closes #11301
How to Validate
ls-deny.toml:npm start -- --policy ls-deny.toml --prompt "/policies list".ls-deny.tomlis listed under user policies and standard policies from~/.gemini/policiesare NOT present.Pre-Merge Checklist