Elevated Sandbox 4 #7889
Elevated Sandbox 4 #7889
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
iceweasel-oai
force-pushed
the
dev/iceweasel/elevated-sandbox-4
branch
from
1c881a3 to
ad7d2ec
Compare
iceweasel-oai
force-pushed
the
dev/iceweasel/elevated-sandbox-4
branch
from
899a383 to
ea28a37
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[cfg(target_os = "windows")] | ||
| static WINDOWS_SANDBOX_ENABLED: AtomicBool = AtomicBool::new(false); | ||
| #[cfg(target_os = "windows")] | ||
| static WINDOWS_ELEVATED_SANDBOX_ENABLED: AtomicBool = AtomicBool::new(false); |
There was a problem hiding this comment.
this is not a pattern that we want long-term, but it's how we persist the sandbox flag today, and until we solve it for real, we'll continue to use it for the elevated flag.
etraut-openai
left a comment
etraut-openai
left a comment
There was a problem hiding this comment.
I left a few comments / suggestions.
| #[cfg(target_os = "windows")] | ||
| { | ||
| crate::safety::set_windows_sandbox_enabled(features.enabled(Feature::WindowsSandbox)); | ||
| // Base flag controls sandbox on/off; elevated only applies when base is enabled. |
There was a problem hiding this comment.
If you think these flags will be left in permanently, then it would be good to add a warning for the case where WindowsSandbox is false but WindowsSandboxElevated is true (and therefore ignored). If these flags are temporary, then it's fine as is.
| use windows_sys::Win32::System::Threading::PROCESS_INFORMATION; | ||
| use windows_sys::Win32::System::Threading::STARTUPINFOW; | ||
|
|
||
| fn ensure_dir(p: &Path) -> Result<()> { |
There was a problem hiding this comment.
This function (and several others) are duplicated across the two implementations (elevated vs not). If we intend to keep both implementations around longer term, it would be good to move these to a common utils module. If the intent is to delete the non-elevated impl at some point, then the duplication is probably OK.
No description provided.