Conversation
iceweasel-oai
changed the title
iceweasel-oai
force-pushed
the
dev/iceweasel/promote-sandbox-nux
branch
from
e1fd473 to
ccdf8bc
Compare
dylan-hurd-oai
left a comment
dylan-hurd-oai
left a comment
There was a problem hiding this comment.
Some light things to discuss but nothing hugely blocking
codex-rs/core/src/features.rs
Outdated
| apply_windows_sandbox_mode( | ||
| &mut features, | ||
| resolve_windows_sandbox_mode(cfg, config_profile), | ||
| ); |
There was a problem hiding this comment.
should we really be potentially mutating the config every time we load it? I see the reasoning for resolving to a healthy config but curious if this is the right way to do so
There was a problem hiding this comment.
gonna clean this up so we don't ever rely on the old features. This doesn't write the config to disk, but it's still a bit wonky
|
|
||
| /// Begin the non-elevated Windows sandbox setup flow. | ||
| #[cfg_attr(not(target_os = "windows"), allow(dead_code))] | ||
| BeginWindowsSandboxLegacySetup { |
There was a problem hiding this comment.
We refer to this as non-admin in the product but Legacy in the code - should we standardize? wasn't obvious to me
There was a problem hiding this comment.
we should. I just wanna do it in a follow-up since Legacy probably appears in a bunch of places (including metrics tags, etc.)
| }, | ||
| SelectionItem { | ||
| name: stay_label, | ||
| name: "Use non-admin sandbox (higher risk if prompt injected)".to_string(), |
There was a problem hiding this comment.
Will we have already shown the user the warning in this case? Do we need the parentheses here? Also Use the
There was a problem hiding this comment.
this is the first time they are seeing really anything about sandbox modes
| lines.push(line![""]); | ||
| lines.push(line
| let windows_sandbox_level = WindowsSandboxLevel::from_features(&features); | ||
| let windows_sandbox_level = match windows_sandbox_mode { | ||
| Some(WindowsSandboxModeToml::Elevated) => WindowsSandboxLevel::Elevated, | ||
| Some(WindowsSandboxModeToml::Unelevated) => WindowsSandboxLevel::RestrictedToken, |
There was a problem hiding this comment.
WindowsSandboxModeToml and WindowsSandboxLevel feel mismatched - should we rename to WindowsSandboxLevelToml?
2 participants
[windows] sandbox="elevated|unelevatd"