bug fixes and perf improvements for elevated sandbox setup #8094
Conversation
iceweasel-oai
changed the title
dylan-hurd-oai
left a comment
dylan-hurd-oai
left a comment
There was a problem hiding this comment.
Couple of non-blocking comments, but generally curious if it would be worth it to make more of this async!
| let cap_sid_path = cap_sid_file(codex_home); | ||
| let is_workspace_write = matches!(&policy, SandboxPolicy::WorkspaceWrite { .. }); | ||
|
|
||
| if matches!(&policy, SandboxPolicy::DangerFullAccess) { |
There was a problem hiding this comment.
It looks like we're adding this to fail earlier - is there a reason not to add this check immediately after parse_policy?
| } | ||
|
|
||
| let (tx, rx) = mpsc::channel::<(PathBuf, Result<bool>)>(); | ||
| std::thread::scope(|scope| { |
There was a problem hiding this comment.
run_setup is getting pretty long here - worth splitting up at all? particularly since we're splitting out threads
| } | ||
| } | ||
|
|
||
| let res = unsafe { ensure_allow_write_aces(&root, &psids) }; |
There was a problem hiding this comment.
Could we potentially encapsulate the psid *mut c_void to reduce the proliferation of unsafe calls It looks like ensure_allow_write_aces is only called here - maybe we could update it to take in a str/String and call convert_string_sid_to_sid inside there?
a few fixes based on testing feedback: