Avoid stdin approval for runtime-only permission grants (#48073)

## Why

Runtime filesystem grants, such as those used for plugin metrics, could trigger unnecessary approval for `write_stdin` and prevent input when sandbox approval was disabled.

## What changed

Include retained runtime grants in the baseline permission comparison for stdin review. Continue requiring review for additional agent permissions, sandbox bypasses, and policy changes. Update the approval description to report current permissions when only runtime grants are present.

## Testing

Extend unit coverage to ensure runtime grants do not require review or mask agent filesystem and network grants, and that sandbox bypasses still require review. Add local and remote plugin regression cases that send stdin and verify metric emission with sandbox approval disabled.

GitOrigin-RevId: 989098bdc7fbf839786c01f11cbf02999bae1cc1
This commit is contained in:
jif
2026-09-25 11:43:35 +00:00
committed by copyberry
parent ca60d6fa72
commit 60713126ee
4 changed files with 181 additions and 25 deletions
@@ -2,4 +2,4 @@
source: core/src/unified_exec/stdin_approval_tests.rs
expression: reason
---
Send input to an existing terminal. This terminal retains additional permissions. It also has an internal filesystem grant. The cwd is its launch directory; the terminal's current directory and state may have changed.
Send input to an existing terminal. This terminal uses the current permissions. It also has an internal filesystem grant. The cwd is its launch directory; the terminal's current directory and state may have changed.
@@ -2,6 +2,7 @@
//! Permission snapshots stay host-owned; neither an earlier approval nor a policy
//! change alters the sandbox of an already-running process. Native launches keep
//! their configured Windows sandbox selection; executor launches use executor defaults.
//! Runtime-internal grants alone do not require review; agent grants and policy drift do.
use super::ProcessEntry;
use super::UnifiedExecContext;
@@ -126,11 +127,12 @@ impl TerminalPermissions {
"this terminal cannot enforce the current denied-read restrictions; start a new terminal",
);
}
// Once the retained settings match, only the baseline permissions can differ.
// Runtime-internal grants are part of an ordinary launch, so only permissions
// beyond the baseline plus those grants need a fresh stdin approval.
Ok(if bypassed || &self.policy != current {
SandboxPermissions::RequireEscalated
} else if self.policy.sandbox.permissions
== effective_permission_profile(baseline, /*additional_permissions*/ None)
== effective_permission_profile(baseline, self.internal_permissions.as_ref())
{
SandboxPermissions::UseDefault
} else {
@@ -161,7 +161,7 @@ async fn captured_network_changes_require_review() -> anyhow::Result<()> {
}
#[tokio::test]
async fn internal_grants_require_review_without_exposing_paths() -> anyhow::Result<()> {
async fn internal_grants_do_not_require_review_or_hide_agent_grants() -> anyhow::Result<()> {
let (_session, turn) = make_session_and_context().await;
let mut environment = turn
.initial_environments
@@ -176,27 +176,44 @@ async fn internal_grants_require_review_without_exposing_paths() -> anyhow::Resu
"write": [turn.config.cwd.join("private-metrics")]
}
}))?;
let permissions = TerminalPermissions::for_launch(
&environment,
&turn,
TerminalSandboxSource::Native,
SandboxPermissions::UseDefault,
/*additional_permissions*/ None,
Some(&grants),
);
let current = TerminalPolicy::capture(
&environment,
&turn,
TerminalSandboxSource::Native,
Some(grants),
);
let expected = SandboxPermissions::WithAdditionalPermissions;
assert_eq!(
permissions.review_requirement(&current, environment.permission_profile()),
Ok(expected)
);
assert_eq!(permissions.additional_permissions, None);
insta::assert_snapshot!("internal_grant", permissions.approval_reason(expected)?);
for (additional_permissions, expected) in [
(None, SandboxPermissions::UseDefault),
(
Some(serde_json::from_value(json!({
"file_system": {"write": [turn.config.cwd.join("agent-output")]}
}))?),
SandboxPermissions::WithAdditionalPermissions,
),
(
Some(serde_json::from_value(
json!({"network": {"enabled": true}}),
)?),
SandboxPermissions::WithAdditionalPermissions,
),
] {
let permissions = TerminalPermissions::for_launch(
&environment,
&turn,
TerminalSandboxSource::Native,
SandboxPermissions::UseDefault,
additional_permissions.as_ref(),
Some(&grants),
);
assert_eq!(
permissions.review_requirement(&permissions.policy, environment.permission_profile()),
Ok(expected)
);
if additional_permissions.is_none() {
// Strict review can still request this description for ordinary stdin.
insta::assert_snapshot!("internal_grant", permissions.approval_reason(expected)?);
}
let mut bypassed = permissions;
bypassed.launch_permissions = SandboxPermissions::RequireEscalated;
assert_eq!(
bypassed.review_requirement(&bypassed.policy, environment.permission_profile()),
Ok(SandboxPermissions::RequireEscalated)
);
}
Ok(())
}
+137
View File
@@ -27,6 +27,8 @@ use codex_protocol::config_types::TrustLevel;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
#[cfg(unix)]
use codex_protocol::protocol::GranularApprovalConfig;
use codex_protocol::protocol::ThreadSettingsOverrides;
use codex_protocol::user_input::UserInput;
use codex_skills_extension::HostSkillsLoadInput;
@@ -50,6 +52,8 @@ use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
use core_test_support::skip_if_remote;
#[cfg(unix)]
use core_test_support::skip_if_sandbox;
use core_test_support::skip_if_target_windows;
use core_test_support::stdio_server_bin;
use core_test_support::submit_thread_settings;
@@ -499,6 +503,139 @@ async fn persisted_remote_plugin_command_attribution_flows_through_turn_context(
Ok(())
}
#[cfg(unix)]
#[test_case(codex_exec_server::LOCAL_ENVIRONMENT_ID; "local")]
#[test_case(codex_exec_server::REMOTE_ENVIRONMENT_ID; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn plugin_metrics_stdin_works_with_sandbox_approval_disabled(
environment_id: &str,
) -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let home = Arc::new(TempDir::new()?);
let script = write_remote_plugin_script_and_config(home.as_ref());
std::fs::write(
&script,
r#"test -n "${CODEX_PLUGIN_METRICS_OUTPUT:-}" || exit 2
IFS= read -r input
test "$input" = continue || exit 3
printf '%s' '{"version":1,"measurements":[{"name":"files_scanned","value":7}]}' > "$CODEX_PLUGIN_METRICS_OUTPUT" || exit 4
printf 'STDIN_OK\n'
"#,
)?;
std::fs::write(
script
.parent()
.unwrap()
.parent()
.unwrap()
.join("analytics.yaml"),
"version: 1\noperations: {scan: {path: ./scripts/run.sh, measurements: {files_scanned: {}}}}\n",
)?;
let command = shlex::try_join(["/bin/sh", script.to_string_lossy().as_ref()])?;
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_function_call(
"open",
"exec_command",
&serde_json::json!({
"cmd": command,
"login": false,
"tty": true,
"yield_time_ms": 1000,
"environment_id": environment_id,
})
.to_string(),
),
ev_completed("opened"),
]),
sse(vec![
ev_function_call(
"input",
"write_stdin",
&serde_json::json!({
"session_id": 1000,
"chars": "continue\n",
"yield_time_ms": 1000,
})
.to_string(),
),
ev_completed("sent"),
]),
sse(vec![ev_completed("done")]),
],
)
.await;
let base_url = server.uri();
let mut builder = test_codex()
.with_home(home)
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
.with_model("gpt-5.2")
.with_config(move |config| {
config.chatgpt_base_url = base_url;
config.analytics_enabled = Some(true);
config
.features
.enable(Feature::SkipHostSkillDiscovery)
.unwrap();
});
// Exercise both native and executor-managed sandboxes with the same plugin fixture.
let _executor = if environment_id == codex_exec_server::REMOTE_ENVIRONMENT_ID {
let executor = super::multi_exec_server_sandbox::ExecServerProcess::start().await?;
builder = builder.with_exec_server_url(executor.websocket_url.clone());
Some(executor)
} else {
None
};
let test = builder.build(&server).await?;
let (sandbox_policy, permission_profile) =
turn_permission_fields(PermissionProfile::read_only(), test.config.cwd.as_path());
test.codex
.start_or_steer_turn(
TurnInputRequest::user_input(vec![UserInput::Text {
text: "send input to the plugin".into(),
text_elements: Vec::new(),
}])
.with_thread_settings(ThreadSettingsOverrides {
approval_policy: Some(AskForApproval::Granular(GranularApprovalConfig {
sandbox_approval: false,
rules: false,
skill_approval: false,
request_permissions: false,
mcp_elicitations: false,
})),
sandbox_policy: Some(sandbox_policy),
permission_profile,
..Default::default()
}),
)
.await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let output = responses.function_call_output_text("input").unwrap();
assert!(output.contains("Process exited with code 0"), "{output}");
assert!(output.contains("STDIN_OK"), "{output}");
let measurement = wait_for_analytics_event(&server, "codex_plugin_measurement_event").await;
assert_eq!(
serde_json::json!({
"plugin_id": measurement["event_params"]["plugin_id"],
"measurement_name": measurement["event_params"]["measurement_name"],
"number_value": measurement["event_params"]["number_value"],
}),
serde_json::json!({
"plugin_id": REMOTE_PLUGIN_CONFIG_NAME,
"measurement_name": "files_scanned",
"number_value": 7.0,
}),
);
Ok(())
}
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn remote_plugin_measurements_require_the_frontend_version() -> Result<()> {