fix(core): add linux bubblewrap sandbox tag (#11767)
## Summary - add a distinct `linux_bubblewrap` sandbox tag when the Linux bubblewrap pipeline feature is enabled - thread the bubblewrap feature flag into sandbox tag generation for: - turn metadata header emission - tool telemetry metric tags and after-tool-use hooks - add focused unit tests for `sandbox_tag` precedence and Linux bubblewrap behavior ## Validation - `just fmt` - `cargo clippy -p codex-core --all-targets` - `cargo test -p codex-core sandbox_tags::tests` - started `cargo test -p codex-core` and stopped it per request Co-authored-by: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
This commit is contained in:
parent
ebceb71db6
commit
db6aa80195
4 changed files with 128 additions and 3 deletions
|
|
@ -945,6 +945,9 @@ impl Session {
|
|||
cwd.clone(),
|
||||
session_configuration.sandbox_policy.get(),
|
||||
session_configuration.windows_sandbox_level,
|
||||
per_turn_config
|
||||
.features
|
||||
.enabled(Feature::UseLinuxSandboxBwrap),
|
||||
));
|
||||
TurnContext {
|
||||
sub_id,
|
||||
|
|
@ -4072,6 +4075,9 @@ async fn spawn_review_thread(
|
|||
parent_turn_context.cwd.clone(),
|
||||
&parent_turn_context.sandbox_policy,
|
||||
parent_turn_context.windows_sandbox_level,
|
||||
parent_turn_context
|
||||
.features
|
||||
.enabled(Feature::UseLinuxSandboxBwrap),
|
||||
));
|
||||
|
||||
let review_turn_context = TurnContext {
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ use codex_protocol::config_types::WindowsSandboxLevel;
|
|||
pub(crate) fn sandbox_tag(
|
||||
policy: &SandboxPolicy,
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
use_linux_sandbox_bwrap: bool,
|
||||
) -> &'static str {
|
||||
if matches!(policy, SandboxPolicy::DangerFullAccess) {
|
||||
return "none";
|
||||
|
|
@ -17,8 +18,61 @@ pub(crate) fn sandbox_tag(
|
|||
{
|
||||
return "windows_elevated";
|
||||
}
|
||||
if cfg!(target_os = "linux") && use_linux_sandbox_bwrap {
|
||||
return "linux_bubblewrap";
|
||||
}
|
||||
|
||||
get_platform_sandbox(windows_sandbox_level != WindowsSandboxLevel::Disabled)
|
||||
.map(SandboxType::as_metric_tag)
|
||||
.unwrap_or("none")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::sandbox_tag;
|
||||
use crate::exec::SandboxType;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use crate::safety::get_platform_sandbox;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::protocol::NetworkAccess;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn danger_full_access_is_untagged_even_when_bubblewrap_is_enabled() {
|
||||
let actual = sandbox_tag(
|
||||
&SandboxPolicy::DangerFullAccess,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
true,
|
||||
);
|
||||
assert_eq!(actual, "none");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn external_sandbox_keeps_external_tag_when_bubblewrap_is_enabled() {
|
||||
let actual = sandbox_tag(
|
||||
&SandboxPolicy::ExternalSandbox {
|
||||
network_access: NetworkAccess::Enabled,
|
||||
},
|
||||
WindowsSandboxLevel::Disabled,
|
||||
true,
|
||||
);
|
||||
assert_eq!(actual, "external");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bubblewrap_feature_sets_distinct_linux_tag() {
|
||||
let actual = sandbox_tag(
|
||||
&SandboxPolicy::new_read_only_policy(),
|
||||
WindowsSandboxLevel::Disabled,
|
||||
true,
|
||||
);
|
||||
let expected = if cfg!(target_os = "linux") {
|
||||
"linux_bubblewrap"
|
||||
} else {
|
||||
get_platform_sandbox(false)
|
||||
.map(SandboxType::as_metric_tag)
|
||||
.unwrap_or("none")
|
||||
};
|
||||
assert_eq!(actual, expected);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ use std::time::Duration;
|
|||
use std::time::Instant;
|
||||
|
||||
use crate::client_common::tools::ToolSpec;
|
||||
use crate::features::Feature;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use crate::sandbox_tags::sandbox_tag;
|
||||
|
|
@ -88,6 +89,10 @@ impl ToolRegistry {
|
|||
sandbox_tag(
|
||||
&invocation.turn.sandbox_policy,
|
||||
invocation.turn.windows_sandbox_level,
|
||||
invocation
|
||||
.turn
|
||||
.features
|
||||
.enabled(Feature::UseLinuxSandboxBwrap),
|
||||
),
|
||||
),
|
||||
(
|
||||
|
|
@ -356,8 +361,12 @@ async fn dispatch_after_tool_use_hook(dispatch: AfterToolUseHookDispatch<'_>) {
|
|||
success: dispatch.success,
|
||||
duration_ms: u64::try_from(dispatch.duration.as_millis()).unwrap_or(u64::MAX),
|
||||
mutating: dispatch.mutating,
|
||||
sandbox: sandbox_tag(&turn.sandbox_policy, turn.windows_sandbox_level)
|
||||
.to_string(),
|
||||
sandbox: sandbox_tag(
|
||||
&turn.sandbox_policy,
|
||||
turn.windows_sandbox_level,
|
||||
turn.features.enabled(Feature::UseLinuxSandboxBwrap),
|
||||
)
|
||||
.to_string(),
|
||||
sandbox_policy: sandbox_policy_tag(&turn.sandbox_policy).to_string(),
|
||||
output_preview: dispatch.output_preview.clone(),
|
||||
},
|
||||
|
|
|
|||
|
|
@ -132,9 +132,17 @@ impl TurnMetadataState {
|
|||
cwd: PathBuf,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
use_linux_sandbox_bwrap: bool,
|
||||
) -> Self {
|
||||
let repo_root = get_git_repo_root(&cwd).map(|root| root.to_string_lossy().into_owned());
|
||||
let sandbox = Some(sandbox_tag(sandbox_policy, windows_sandbox_level).to_string());
|
||||
let sandbox = Some(
|
||||
sandbox_tag(
|
||||
sandbox_policy,
|
||||
windows_sandbox_level,
|
||||
use_linux_sandbox_bwrap,
|
||||
)
|
||||
.to_string(),
|
||||
);
|
||||
let base_metadata = build_turn_metadata_bag(Some(turn_id), sandbox, None, None);
|
||||
let base_header = base_metadata
|
||||
.to_header_value()
|
||||
|
|
@ -290,4 +298,52 @@ mod tests {
|
|||
Some(false)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_metadata_state_respects_linux_bubblewrap_toggle() {
|
||||
let temp_dir = TempDir::new().expect("temp dir");
|
||||
let cwd = temp_dir.path().to_path_buf();
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
|
||||
let without_bubblewrap = TurnMetadataState::new(
|
||||
"turn-a".to_string(),
|
||||
cwd.clone(),
|
||||
&sandbox_policy,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
false,
|
||||
);
|
||||
let with_bubblewrap = TurnMetadataState::new(
|
||||
"turn-b".to_string(),
|
||||
cwd,
|
||||
&sandbox_policy,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
true,
|
||||
);
|
||||
|
||||
let without_bubblewrap_header = without_bubblewrap
|
||||
.current_header_value()
|
||||
.expect("without_bubblewrap_header");
|
||||
let with_bubblewrap_header = with_bubblewrap
|
||||
.current_header_value()
|
||||
.expect("with_bubblewrap_header");
|
||||
|
||||
let without_bubblewrap_json: Value =
|
||||
serde_json::from_str(&without_bubblewrap_header).expect("without_bubblewrap_json");
|
||||
let with_bubblewrap_json: Value =
|
||||
serde_json::from_str(&with_bubblewrap_header).expect("with_bubblewrap_json");
|
||||
|
||||
let without_bubblewrap_sandbox = without_bubblewrap_json
|
||||
.get("sandbox")
|
||||
.and_then(Value::as_str);
|
||||
let with_bubblewrap_sandbox = with_bubblewrap_json.get("sandbox").and_then(Value::as_str);
|
||||
|
||||
let expected_with_bubblewrap =
|
||||
sandbox_tag(&sandbox_policy, WindowsSandboxLevel::Disabled, true);
|
||||
assert_eq!(with_bubblewrap_sandbox, Some(expected_with_bubblewrap));
|
||||
|
||||
if cfg!(target_os = "linux") {
|
||||
assert_eq!(with_bubblewrap_sandbox, Some("linux_bubblewrap"));
|
||||
assert_ne!(with_bubblewrap_sandbox, without_bubblewrap_sandbox);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue