-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Improve error handling for Windows sandbox initialization #7992
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Thanks for the contribution. Please look at the code review feedback from codex. You'll also need to sign the CLA and fix the CI failures. |
|
I have read the CLA Document and I hereby sign the CLA |
|
@anant2526, there are still some lint failures. Looks like a simple formatting issue. |
|
@anant2526, there are still formatting issues. Run |
I analyzed the codex-rs/windows-sandbox-rs crate, specifically how it handles file permissions (ACLs) for the sandboxed environment.
The Bug: In src/lib.rs, the system was identifying which file paths strictly needed read or write access. It then attempted to grant these permissions using add_allow_ace. However, it was silently ignoring any errors during this process.
The Consequence: If the system failed to grant the necessary permissions (e.g., due to file locking or OS restrictions), it would proceed anyway. This meant the sandboxed process would start without the rights it needed, leading to "access denied" errors, retries, hangs, and the "sluggish" behavior users reported.
2. Implement the Fix
I modified codex-rs/windows-sandbox-rs/src/lib.rs to correctly handle these errors.
Stop Silent Failures: I replaced the if let Ok(...) checks with proper match statements.
Error Propagation: If granting permissions (add_allow_ace) or blocking access (add_deny_write_ace) fails, the function now immediately returns an error.
Improved Logging: I added log_failure calls to record exactly which file path caused the permission error.