feat(core): implement native Windows sandboxing#21807
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
mattKorwel
force-pushed
the
mk-windows-sandboxing
branch
from
a3ae69b to
70f4658
Compare
mattKorwel
force-pushed
the
mk-windows-sandboxing
branch
from
70f4658 to
1cb703b
Compare
Note on native Windows Sandbox compilationI've implemented the Windows native sandbox using a C# helper (GeminiSandbox.cs) that leverages Restricted Tokens for robust isolation. Architectural Choice: Local Compilation vs. Distributed Binary
This approach ensures the sandbox is auditable, targets the host architecture (x64/ARM64), and avoids the common issues with NPM-distributed Windows binaries. |
mattKorwel
force-pushed
the
mk-windows-sandboxing
branch
2 times, most recently
from
9472927 to
f7a74c7
Compare
mattKorwel
force-pushed
the
mk-windows-sandboxing
branch
from
f7a74c7 to
5c0b0f9
Compare
Windows Native Sandbox: Progressive Elevation & AI AwarenessI've updated the implementation to include a Progressive Elevation strategy to handle the limitations of native Windows security tokens:
|
Clarification on Sandbox Boundaries: Native vs. DockerBased on further testing, I'm documenting the explicit security boundaries of the \windows-native\ implementation to help users choose the right provider: The 'Level 3' Limitation:
Summary of Sandbox Providers for Windows:
|
galz10
force-pushed
the
galzahavi/add/sandboxed-tool
branch
from
8f5d917 to
52fb019
Compare
mattKorwel
force-pushed
the
mk-windows-sandboxing
branch
from
13877f6 to
7814427
Compare
|
Size Change: +10.2 kB (+0.04%) Total Size: 26.3 MB
ℹ️ View Unchanged
|
Summary of ChangesHello, 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 delivers a comprehensive native Windows sandboxing solution for the Gemini CLI. It significantly enhances the security posture by providing robust isolation for both external shell commands and internal Node.js file operations, thereby removing the dependency on containerization tools like Docker or Podman for secure execution on Windows. The implementation offers fine-grained control over network and file system access, ensuring that operations are confined to explicitly permitted resources. 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 native Windows sandboxing capabilities by adding a C# helper (GeminiSandbox.exe) for restricted token sandboxing, along with a script to compile it. The changes integrate WindowsSandboxManager and SandboxedFileSystemService to handle sandboxed shell execution and file system operations, updating configuration schemas to support a windows-native sandbox command. Review comments highlight a critical security vulnerability where the SandboxedFileSystemService might not be activated consistently with the WindowsSandboxManager, potentially bypassing file system sandboxing. Another high-severity security concern is raised regarding the grantLowIntegrityAccess method, which degrades system security by making user workspace directories writable by any low-integrity process. Additionally, memory leaks were identified in the GeminiSandbox.cs Main method due to unmanaged SID and struct pointers not being freed.
| static int Main(string[] args) { | ||
| if (args.Length < 3) { | ||
| Console.WriteLine("Usage: GeminiSandbox.exe <network:0|1> <cwd> <command> [args...]"); | ||
| Console.WriteLine("Internal commands: __read <path>, __write <path>"); | ||
| return 1; | ||
| } | ||
|
|
||
| bool networkAccess = args[0] == "1"; | ||
| string cwd = args[1]; | ||
| string command = args[2]; | ||
|
|
||
| // 1. Setup Token | ||
| IntPtr hCurrentProcess = GetCurrentProcess(); | ||
| IntPtr hToken; | ||
| if (!OpenProcessToken(hCurrentProcess, TOKEN_DUPLICATE | TOKEN_QUERY | TOKEN_ASSIGN_PRIMARY | TOKEN_ADJUST_DEFAULT, out hToken)) { | ||
| Console.Error.WriteLine("Failed to open process token"); | ||
| return 1; | ||
| } | ||
|
|
||
| IntPtr hRestrictedToken; | ||
| IntPtr pSidsToDisable = IntPtr.Zero; | ||
| uint sidCount = 0; | ||
|
|
||
| IntPtr pSidsToRestrict = IntPtr.Zero; | ||
| uint restrictCount = 0; | ||
|
|
||
| // "networkAccess == false" implies Strict Sandbox Level 1. | ||
| // In Strict mode, we strip the Network SID and apply the Restricted Code SID. | ||
| // This blocks network access and restricts file reads, but requires cmd.exe. | ||
| if (!networkAccess) { | ||
| IntPtr networkSid; | ||
| if (ConvertStringSidToSid("S-1-5-2", out networkSid)) { | ||
| sidCount = 1; | ||
| int saaSize = Marshal.SizeOf(typeof(SID_AND_ATTRIBUTES)); | ||
| pSidsToDisable = Marshal.AllocHGlobal(saaSize); | ||
| SID_AND_ATTRIBUTES saa = new SID_AND_ATTRIBUTES(); | ||
| saa.Sid = networkSid; | ||
| saa.Attributes = 0; | ||
| Marshal.StructureToPtr(saa, pSidsToDisable, false); | ||
| } | ||
|
|
||
| IntPtr restrictedSid; | ||
| // S-1-5-12 is Restricted Code SID | ||
| if (ConvertStringSidToSid("S-1-5-12", out restrictedSid)) { | ||
| restrictCount = 1; | ||
| int saaSize = Marshal.SizeOf(typeof(SID_AND_ATTRIBUTES)); | ||
| pSidsToRestrict = Marshal.AllocHGlobal(saaSize); | ||
| SID_AND_ATTRIBUTES saa = new SID_AND_ATTRIBUTES(); | ||
| saa.Sid = restrictedSid; | ||
| saa.Attributes = 0; | ||
| Marshal.StructureToPtr(saa, pSidsToRestrict, false); | ||
| } | ||
| } | ||
| // If networkAccess == true, we are in Elevated mode (Level 2). | ||
| // We only strip privileges (DISABLE_MAX_PRIVILEGE), allowing network and powershell. | ||
|
|
||
| if (!CreateRestrictedToken(hToken, DISABLE_MAX_PRIVILEGE, sidCount, pSidsToDisable, 0, IntPtr.Zero, restrictCount, pSidsToRestrict, out hRestrictedToken)) { | ||
| Console.Error.WriteLine("Failed to create restricted token"); | ||
| return 1; | ||
| } | ||
|
|
||
| // 2. Set Integrity Level to Low | ||
| IntPtr lowIntegritySid; | ||
| if (ConvertStringSidToSid("S-1-16-4096", out lowIntegritySid)) { | ||
| TOKEN_MANDATORY_LABEL tml = new TOKEN_MANDATORY_LABEL(); | ||
| tml.Label.Sid = lowIntegritySid; | ||
| tml.Label.Attributes = SE_GROUP_INTEGRITY; | ||
| int tmlSize = Marshal.SizeOf(tml); | ||
| IntPtr pTml = Marshal.AllocHGlobal(tmlSize); | ||
| Marshal.StructureToPtr(tml, pTml, false); | ||
| SetTokenInformation(hRestrictedToken, TokenIntegrityLevel, pTml, (uint)tmlSize); | ||
| Marshal.FreeHGlobal(pTml); | ||
| } | ||
|
|
||
| // 3. Handle Internal Commands or External Process | ||
| if (command == "__read") { | ||
| string path = args[3]; | ||
| return RunInImpersonation(hRestrictedToken, () => { | ||
| try { | ||
| using (FileStream fs = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read)) | ||
| using (StreamReader sr = new StreamReader(fs)) { | ||
| char[] buffer = new char[4096]; | ||
| int bytesRead; | ||
| while ((bytesRead = sr.Read(buffer, 0, buffer.Length)) > 0) { | ||
| Console.Write(buffer, 0, bytesRead); | ||
| } | ||
| } | ||
| return 0; | ||
| } catch (Exception e) { | ||
| Console.Error.WriteLine(e.Message); | ||
| return 1; | ||
| } | ||
| }); | ||
| } else if (command == "__write") { | ||
| string path = args[3]; | ||
| return RunInImpersonation(hRestrictedToken, () => { | ||
| try { | ||
| using (StreamReader reader = new StreamReader(Console.OpenStandardInput())) | ||
| using (FileStream fs = new FileStream(path, FileMode.Create, FileAccess.Write, FileShare.None)) | ||
| using (StreamWriter writer = new StreamWriter(fs)) { | ||
| char[] buffer = new char[4096]; | ||
| int bytesRead; | ||
| while ((bytesRead = reader.Read(buffer, 0, buffer.Length)) > 0) { | ||
| writer.Write(buffer, 0, bytesRead); | ||
| } | ||
| } | ||
| return 0; | ||
| } catch (Exception e) { | ||
| Console.Error.WriteLine(e.Message); | ||
| return 1; | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| // 4. Setup Job Object for external process | ||
| IntPtr hJob = CreateJobObject(IntPtr.Zero, null); | ||
| if (hJob != IntPtr.Zero) { | ||
| JOBOBJECT_EXTENDED_LIMIT_INFORMATION limitInfo = new JOBOBJECT_EXTENDED_LIMIT_INFORMATION(); | ||
| limitInfo.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; | ||
| int limitSize = Marshal.SizeOf(limitInfo); | ||
| IntPtr pLimit = Marshal.AllocHGlobal(limitSize); | ||
| Marshal.StructureToPtr(limitInfo, pLimit, false); | ||
| SetInformationJobObject(hJob, JobObjectInfoClass.ExtendedLimitInformation, pLimit, (uint)limitSize); | ||
| Marshal.FreeHGlobal(pLimit); | ||
| } | ||
|
|
||
| // 5. Launch Process | ||
| STARTUPINFO si = new STARTUPINFO(); | ||
| si.cb = (uint)Marshal.SizeOf(si); | ||
| si.dwFlags = STARTF_USESTDHANDLES; | ||
| si.hStdInput = GetStdHandle(-10); | ||
| si.hStdOutput = GetStdHandle(-11); | ||
| si.hStdError = GetStdHandle(-12); | ||
|
|
||
| List<string> quotedArgs = new List<string>(); | ||
| for (int i = 2; i < args.Length; i++) { | ||
| quotedArgs.Add(QuoteArgument(args[i])); | ||
| } | ||
| string commandLine = string.Join(" ", quotedArgs.ToArray()); | ||
|
|
||
| PROCESS_INFORMATION pi; | ||
| if (!CreateProcessAsUser(hRestrictedToken, null, commandLine, IntPtr.Zero, IntPtr.Zero, true, CREATE_SUSPENDED | CREATE_UNICODE_ENVIRONMENT, IntPtr.Zero, cwd, ref si, out pi)) { | ||
| Console.Error.WriteLine("Failed to create process. Error: " + Marshal.GetLastWin32Error()); | ||
| return 1; | ||
| } | ||
|
|
||
| if (hJob != IntPtr.Zero) { | ||
| AssignProcessToJobObject(hJob, pi.hProcess); | ||
| } | ||
|
|
||
| ResumeThread(pi.hThread); | ||
| WaitForSingleObject(pi.hProcess, INFINITE); | ||
|
|
||
| uint exitCode = 0; | ||
| GetExitCodeProcess(pi.hProcess, out exitCode); | ||
|
|
||
| CloseHandle(pi.hProcess); | ||
| CloseHandle(pi.hThread); | ||
| CloseHandle(hRestrictedToken); | ||
| CloseHandle(hToken); | ||
| if (hJob != IntPtr.Zero) CloseHandle(hJob); | ||
|
|
||
| return (int)exitCode; | ||
| } |
There was a problem hiding this comment.
The Main method has several memory leaks from unmanaged resources not being freed.
- SID pointers: The
IntPtrhandles returned byConvertStringSidToSid(fornetworkSid,restrictedSid,lowIntegritySid) are not being freed. According to documentation, these must be freed usingLocalFree. - Struct pointers: The memory allocated with
Marshal.AllocHGlobalforpSidsToDisableandpSidsToRestrictis not being freed withMarshal.FreeHGlobal.
While this is a short-lived process, it's crucial to correctly manage unmanaged resources. Please ensure all allocated memory is freed. A try...finally block is a good pattern to guarantee cleanup.
You'll need to add the P/Invoke declaration for LocalFree:
[DllImport("kernel32.dll", SetLastError = true)]
public static extern IntPtr LocalFree(IntPtr hMem);Then, ensure you call LocalFree on the SID pointers and Marshal.FreeHGlobal on the allocated struct pointers in a finally block.
gemini-cli
bot
added
the
status/need-issue
PR Review: feat(core): implement native Windows sandboxing (#21807)SummaryThis PR introduces native Windows sandboxing using Restricted Tokens and Mandatory Integrity Control (MIC). It's a significant security enhancement for Windows users, providing a native alternative to Docker/Podman isolation. The implementation of `SandboxedFileSystemService` to bring native tools into the same security boundary is excellent. However, there is a critical implementation gap: while the infrastructure for sandboxing shell commands is present, it is not currently integrated into `ShellExecutionService`. Consequently, shell commands are not yet running inside the `GeminiSandbox.exe` wrapper. Findings🔴 Critical
🟡 Improvements
🟢 Value-Add
ConclusionStatus: Request Changes The sandboxing mechanism itself (the C# helper and the `SandboxManager` logic) looks solid, but it needs to be properly wired into `ShellExecutionService` to achieve its goal for shell tools. |
| if (ConvertStringSidToSid("S-1-5-2", out networkSid)) { | ||
| sidCount = 1; | ||
| int saaSize = Marshal.SizeOf(typeof(SID_AND_ATTRIBUTES)); | ||
| pSidsToDisable = Marshal.AllocHGlobal(saaSize); |
There was a problem hiding this comment.
This pointer is never freed, and will result in a memory leak.
| return "\"" + escaped + "\""; | ||
| } | ||
|
|
||
| private static int RunInImpersonation(IntPtr hToken, Func<int> action) { |
There was a problem hiding this comment.
When GeminiSandbox.exe processes internal __read or __write commands, it returns early via return RunInImpersonation(...). However, it fails to clean up the handles (hToken, hRestrictedToken) and any allocated memory (pSidsToDisable, pSidsToRestrict) before exiting. While Windows will reclaim these resources on process exit, relying on the OS cleanup for a security boundary tool is bad practice.
…, and harden security
🚀 PR Update: Addressing Review Comments & Async Assessment SummaryI've completed a round of manual fixes and an automated async review. Here's the current status of PR 21807: ✅ Resolved in latest push:
⏳ Remaining Items (Identified by Async Review):
Tests for |
✅ Final Update: All Async Review Findings AddressedI've pushed the final set of changes addressing the remaining feedback:
This PR is now ready for final approval. |
|
🏁 Final Polish: Architectural & Build HardeningI've pushed the final set of improvements addressing the deeper architectural feedback:
The PR is now robust, compliant, and ready for final review. |
…dboxed file system
3 participants
Summary
Implement native Windows sandboxing using Restricted Tokens and Mandatory Integrity Control (MIC).
Details
This PR adds a complete native Windows sandboxing implementation for Gemini CLI, providing robust isolation for both external shell tools and native Node.js tools. It avoids the need for Docker or Podman on Windows while providing stronger security than simple filesystem checks.
Key Features:
GeminiSandbox.exe) to create a Windows Restricted Token with Double SID Evaluation (S-1-5-12). This model ensures the process has zero-trust access by default, blocking read access to any file not explicitly whitelisted.networkAccessin the configuration.WindowsSandboxManagerusesicaclsto grant "Low" integrity access to theCWDand any user-definedallowedPaths.SandboxedFileSystemService. Native Gemini tools likeread_fileandwrite_filenow delegate their work to the restricted worker, ensuring they operate within the same security boundary as shell tools.networkAccessandallowedPathstoSandboxConfig.The C# helper source is included and automatically compiled on the first run using the standard Windows
csc.execompiler.Related Issues
Partially addresses sandboxing requirements on Windows.
How to Validate
.gemini/config.yaml:@shell echo test > C:\Windows\test.txt-> Should fail with Access Denied.@shell type %USERPROFILE%\.ssh\id_rsa-> Should fail with Access Denied.@shell curl http://google.com-> Should fail.read_fileon a file in the workspace -> Should succeed.Pre-Merge Checklist