feat: apply remote admin settings (no-op) #16106
Conversation
…file-based settings. This commit introduces the ability to fetch and apply remote administration settings, ensuring they take precedence over any conflicting settings defined in local configuration files. This enhances centralized control and consistency across deployments. Changes include: - Added method to to update configuration based on remote inputs. - Modified to prioritize remote admin settings. - Moved authentication flow in to occur before sandbox entry to fetch remote settings. - Updated related tests in , , and to cover new remote admin functionality.
Summary of ChangesHello @skeshive, 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 lays the groundwork for centralized administrative control by enabling the application to consume and prioritize remote admin settings. It establishes a clear hierarchy where remote configurations override local file-based settings, ensuring consistent policy enforcement. The authentication flow has been strategically repositioned to facilitate the retrieval of these settings early in the application lifecycle, preparing the system for future integration with a central configuration management service, even though the actual fetching is currently a no-op. Highlights
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
|
|
Size Change: +1.12 kB (+0.01%) Total Size: 22.3 MB
ℹ️ View Unchanged
|
![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 logic to apply remote admin settings, ensuring local settings are ignored in favor of schema defaults and remote configurations. While this enhances centralized management, the current implementation has a high-severity vulnerability: the application blindly trusts remote settings without integrity checks, which could allow a Man-in-the-Middle attacker to disable security features. Furthermore, a critical issue in gemini.tsx causes fetched remote admin settings to be lost when the application relaunches into a sandbox, defeating the intended purpose of applying these settings.
| const remoteAdminSettings = partialConfig.getRemoteAdminSettings(); | ||
| // Set remote admin settings if returned from CCPA. | ||
| if (remoteAdminSettings) { | ||
| settings.setRemoteAdminSettings(remoteAdminSettings); | ||
| } | ||
|
|
||
| // hop into sandbox if we are outside and sandboxing is enabled | ||
| if (!process.env['SANDBOX']) { |
There was a problem hiding this comment.
There appears to be a critical issue with how remote admin settings are handled when sandboxing is enabled.
The remote admin settings are fetched and applied to the in-memory settings object at lines 417-421. However, immediately after, the application may be relaunched to enter a sandbox (inside the if (!process.env['SANDBOX']) block).
When the application is relaunched, it's a new process that starts execution from the main() function again. This new process calls loadSettings() (line 302), which re-initializes the settings from files. The in-memory changes made to the settings object in the parent process are lost.
The authentication logic that fetches the remote settings has been moved out of the sandboxed execution path, so the new process will not re-fetch them. Consequently, the sandboxed process will operate without the remote admin settings, which contradicts the goal of this pull request.
To fix this, the fetched remote settings need to be persisted across the process relaunch. This could be done by:
- Serializing the remote settings and passing them to the child process via an environment variable.
- Saving the remote settings to a temporary file that the child process can read on startup.
Given the importance of enforcing admin settings, this should be addressed before merging.
There was a problem hiding this comment.
Agree this is an important consideration.
There was a problem hiding this comment.
hmm I might be missing something but this comment seems incorrect.
After we enter the sandbox, we would still hit lines 385 to 521 where the sandbox in-memory settings would be overwritten
gemini-cli
bot
added
the
status/need-issue
2 participants
This commit introduces the ability to apply remote admin settings, ensuring any admin settings in local files do not get used. This PR is still a no-op since we don't fetch admin settings from CCPA yet but defaults will be enforced.
Changes include:
Fixes: https://github.com/google-gemini/maintainers-gemini-cli/issues/1167