Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions codex-rs/config/src/loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1127,6 +1127,19 @@ fn sanitize_project_config(config: &mut TomlValue) -> Vec<String> {
{
ignored_keys.push("features.respect_system_proxy".to_string());
}
// Repository contents must not turn an ordinary key into a permission increase.
if let Some(chat) = table
.get_mut("tui")
.and_then(|tui| tui.get_mut("keymap"))
.and_then(|keymap| keymap.get_mut("chat"))
.and_then(TomlValue::as_table_mut)
{
for key in ["previous_permission_mode", "next_permission_mode"] {
if chat.remove(key).is_some() {
ignored_keys.push(format!("tui.keymap.chat.{key}"));
}
}
}

ignored_keys
}
Expand Down
13 changes: 13 additions & 0 deletions codex-rs/config/src/loader/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@ use tempfile::tempdir;

pub(super) struct TestFileSystem;

#[test]
fn project_config_cannot_bind_permission_shortcuts() {
let safe = "[tui.keymap.chat]\nincrease_reasoning_effort = 'f9'\n";
for key in ["previous_permission_mode", "next_permission_mode"] {
let mut config = toml::from_str(&format!("{safe}{key} = 'page-down'")).unwrap();
assert_eq!(
sanitize_project_config(&mut config),
[format!("tui.keymap.chat.{key}")]
);
assert_eq!(config, toml::from_str::<TomlValue>(safe).unwrap());
}
}

#[tokio::test]
async fn managed_browser_import_denial_survives_user_and_session_config() {
let tmp = tempdir().expect("tempdir");
Expand Down
4 changes: 4 additions & 0 deletions codex-rs/config/src/tui_keymap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ pub struct TuiChatKeymap {
pub decrease_reasoning_effort: Option<KeybindingsSpec>,
/// Increase the active reasoning effort.
pub increase_reasoning_effort: Option<KeybindingsSpec>,
/// Switch to the previous available permission mode.
pub previous_permission_mode: Option<KeybindingsSpec>,
/// Switch to the next available permission mode.
pub next_permission_mode: Option<KeybindingsSpec>,
/// Edit the most recently queued message.
pub edit_queued_message: Option<KeybindingsSpec>,
}
Expand Down
24 changes: 22 additions & 2 deletions codex-rs/core/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -3571,7 +3571,9 @@
"decrease_reasoning_effort": null,
"edit_queued_message": null,
"increase_reasoning_effort": null,
"interrupt_turn": null
"interrupt_turn": null,
"next_permission_mode": null,
"previous_permission_mode": null
},
"composer": {
"history_search_next": null,
Expand Down Expand Up @@ -3967,6 +3969,22 @@
}
],
"description": "Interrupt the active turn."
},
"next_permission_mode": {
"allOf": [
{
"$ref": "#/definitions/KeybindingsSpec"
}
],
"description": "Switch to the next available permission mode."
},
"previous_permission_mode": {
"allOf": [
{
"$ref": "#/definitions/KeybindingsSpec"
}
],
"description": "Switch to the previous available permission mode."
}
},
"type": "object"
Expand Down Expand Up @@ -4309,7 +4327,9 @@
"decrease_reasoning_effort": null,
"edit_queued_message": null,
"increase_reasoning_effort": null,
"interrupt_turn": null
"interrupt_turn": null,
"next_permission_mode": null,
"previous_permission_mode": null
}
},
"composer": {
Expand Down
1 change: 1 addition & 0 deletions codex-rs/tui/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ mod history_ui;
mod input;
mod loaded_threads;
mod pending_interactive_replay;
mod permission_shortcuts;
mod pets;
mod platform_actions;
mod plugin_mentions;
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/tui/src/app/event_dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1519,6 +1519,9 @@ impl App {
profile_selection,
);
}
AppEvent::ApplyPermissionShortcut { thread_id, selection } => {
self.apply_permission_shortcut(app_server, tui, thread_id, selection).await;
}
AppEvent::OpenWorldWritableWarningConfirmation {
preset,
profile_selection,
Expand Down
71 changes: 71 additions & 0 deletions codex-rs/tui/src/app/permission_shortcuts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
//! Confirm session-only permission changes before updating local state.

use super::*;
use codex_app_server_protocol::ThreadSettingsUpdateParams;

impl App {
pub(super) async fn apply_permission_shortcut(
&mut self,
app_server: &mut AppServerSession,
tui: &mut tui::Tui,
thread_id: ThreadId,
selection: PermissionProfileSelection,
) {
if self.current_displayed_thread_id() != Some(thread_id)
|| self.chat_widget.thread_id() != Some(thread_id)
{
self.chat_widget.complete_permission_shortcut(thread_id);
return;
}
let result: Result<()> = async {
let active_profile = ActivePermissionProfile::new(selection.profile_id.clone());
let profile = builtin_permission_profile_for_active_permission_profile(&active_profile)
.ok_or_else(|| color_eyre::eyre::eyre!("unknown built-in permission profile"))?;
let mut config = self.chat_widget.config_ref().clone();
if let Some(policy) = selection.approval_policy {
config.permissions.approval_policy.set(policy.to_core())?;
}
if let Some(reviewer) = selection.approvals_reviewer {
config.config_layer_stack.requirements().approvals_reviewer.can_set(&reviewer)?;
config.approvals_reviewer = reviewer;
}
config.permissions.set_permission_profile_from_session_snapshot(
PermissionProfileSnapshot::active(profile, active_profile),
)?;

if !app_server.thread_settings_update(ThreadSettingsUpdateParams {
thread_id: thread_id.to_string(),
permissions: Some(selection.profile_id),
approval_policy: selection.approval_policy,
approvals_reviewer: selection.approvals_reviewer.map(Into::into),
..Default::default()
}).await? {
color_eyre::eyre::bail!("this app server does not support confirmed permission changes; use /permissions");
}

// This exact widget configuration was validated before the request.
self.chat_widget.set_permission_profile_with_active_profile(
config.permissions.permission_profile().clone(),
config.permissions.active_permission_profile(),
)?;
self.chat_widget.set_approval_policy(AskForApproval::from(config.permissions.approval_policy.value()));
self.config.permissions = config.permissions.clone();
self.set_approvals_reviewer_in_app_and_widget(config.approvals_reviewer);
self.runtime_approval_policy_override = selection.approval_policy;
self.runtime_permission_profile_override = Some(RuntimePermissionProfileOverride::from_config(&config));
self.sync_active_thread_permission_settings_to_cached_session().await;
self.insert_history_cell(tui, Box::new(history_cell::new_info_event(format!("Permissions updated to {}", selection.display_label), /*hint*/ None)));
Ok(())
}.await;
self.chat_widget.complete_permission_shortcut(thread_id);
if let Err(err) = result {
let error = crate::config_update::format_config_error(&err);
self.insert_history_cell(
tui,
Box::new(history_cell::new_error_event(format!(
"Failed to update permissions: {error}"
))),
);
}
}
}
2 changes: 2 additions & 0 deletions codex-rs/tui/src/app/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ mod key_chords;
#[path = "tests/mcp_startup.rs"]
mod mcp_startup;
mod model_catalog;
#[path = "tests/permission_shortcuts_tests.rs"]
mod permission_shortcuts_tests;
mod plugin_catalog;
mod rate_limits;
mod safety_buffering;
Expand Down
138 changes: 138 additions & 0 deletions codex-rs/tui/src/app/tests/permission_shortcuts_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
use super::*;
use codex_arg0::Arg0DispatchPaths;
use pretty_assertions::assert_eq;

fn read_only_selection() -> PermissionProfileSelection {
PermissionProfileSelection {
profile_id: ":read-only".to_string(),
approval_policy: Some(AskForApproval::OnRequest),
approvals_reviewer: Some(ApprovalsReviewer::User),
display_label: "Read Only".to_string(),
}
}

#[tokio::test]
async fn permission_shortcut_rejections_leave_state_unchanged() -> Result<()> {
for experimental_api in [false, true] {
let (mut app, mut events, _op_rx) = make_test_app_with_channels().await;
let mut tui = crate::tui::test_support::make_test_tui()?;
let thread_id = ThreadId::new();
app.active_thread_id = Some(thread_id);
app.chat_widget
.handle_thread_session_quiet(test_thread_session(
thread_id,
app.config.cwd.to_path_buf(),
));
let original = RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref());
let original_reviewer = app.config.approvals_reviewer;
let client = crate::start_embedded_app_server_with(
Arg0DispatchPaths::default(),
app.config.clone(),
Vec::new(),
LoaderOverrides::without_managed_config_for_tests(),
/*strict_config*/ false,
CloudConfigBundleLoader::default(),
codex_feedback::CodexFeedback::new(),
/*log_db*/ None,
/*state_db*/ None,
Arc::clone(&app.environment_manager),
|mut args| {
args.experimental_api = experimental_api;
codex_app_server_client::InProcessAppServerClient::start(args)
},
)
.await?;
let mut app_server = AppServerSession::new(
codex_app_server_client::AppServerClient::InProcess(client),
crate::app_server_session::ThreadParamsMode::Embedded,
);
while events.try_recv().is_ok() {}
let transcript_len = app.transcript_cells.len();
app.apply_permission_shortcut(
&mut app_server,
&mut tui,
ThreadId::new(),
read_only_selection(),
)
.await;
assert_eq!(app.transcript_cells.len(), transcript_len);
assert!(events.try_recv().is_err());
app.apply_permission_shortcut(&mut app_server, &mut tui, thread_id, read_only_selection())
.await;
assert_eq!(
RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()),
original
);
assert_eq!(app.config.approvals_reviewer, original_reviewer);
let cell = app.transcript_cells.last().expect("rejection notice");
insta::assert_snapshot!(
if experimental_api {
"permission_shortcut_server_error"
} else {
"permission_shortcut_unsupported"
},
lines_to_single_string(&cell.display_lines(/*width*/ 120))
.replace(&thread_id.to_string(), "<THREAD_ID>")
);
assert!(events.try_recv().is_err());
app_server.shutdown().await?;
}
Ok(())
}

#[tokio::test]
async fn permission_shortcut_confirms_without_persisting() -> Result<()> {
let (mut app, mut events, _op_rx) = make_test_app_with_channels().await;
let mut tui = crate::tui::test_support::make_test_tui()?;
let codex_home = tempdir()?;
app.config.codex_home = codex_home.path().to_path_buf().abs();
app.config
.permissions
.set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active(
PermissionProfile::workspace_write(),
ActivePermissionProfile::new(":workspace"),
))?;
let config_path = codex_home.path().join("config.toml");
let contents = "approvals_reviewer = \"auto_review\"\n";
std::fs::write(&config_path, contents)?;
let mut app_server = start_config_write_test_app_server(&app).await?;
let started = app_server.start_thread(&app.config).await?;
let thread_id = started.session.thread_id;
app.chat_widget
.handle_thread_session_quiet(started.session.clone());
app.enqueue_primary_thread_session(started.session, started.turns)
.await?;
let contents = std::fs::read_to_string(&config_path)?;
while events.try_recv().is_ok() {}

app.apply_permission_shortcut(&mut app_server, &mut tui, thread_id, read_only_selection())
.await;

let cell = app.transcript_cells.last().expect("confirmed notice");
insta::assert_snapshot!(
lines_to_single_string(&cell.display_lines(/*width*/ 80)),
@"• Permissions updated to Read Only"
);

let settings = next_thread_settings_updated(&mut app_server, thread_id)
.await
.thread_settings;
let profile = app
.chat_widget
.config_ref()
.permissions
.active_permission_profile();
assert_eq!(
settings.active_permission_profile,
profile.clone().map(Into::into)
);
assert_eq!(profile, Some(ActivePermissionProfile::new(":read-only")));
assert_eq!(app.config.approvals_reviewer, ApprovalsReviewer::User);
assert_eq!(std::fs::read_to_string(config_path)?, contents);
assert!(
events.try_recv().is_err(),
"must not queue another update or config write"
);
app_server.shutdown().await?;
Ok(())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: tui/src/app/tests/permission_shortcuts_tests.rs
assertion_line: 63
expression: "lines_to_single_string(&cell.display_lines(120)).replace(&missing_thread.to_string(),\n\"<THREAD_ID>\")"
---
■ Failed to update permissions: thread/settings/update failed in TUI: thread/settings/update failed: thread not found: <THREAD_ID> (code -32600)
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: tui/src/app/tests/permission_shortcuts_tests.rs
assertion_line: 33
expression: lines_to_single_string(&cell.display_lines(120))
---
■ Failed to update permissions: this app server does not support confirmed permission changes; use /permissions
6 changes: 6 additions & 0 deletions codex-rs/tui/src/app_event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,12 @@ pub(crate) enum AppEvent {
profile_selection: Option<PermissionProfileSelection>,
},

/// Apply a permission shortcut only while its originating thread is displayed.
ApplyPermissionShortcut {
thread_id: ThreadId,
selection: PermissionProfileSelection,
},

/// Open the Windows world-writable directories warning.
/// If `preset` is `Some`, the confirmation will apply the provided
/// approval/sandbox configuration on Continue; if `None`, it performs no
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/tui/src/chatwidget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ mod model_popups;
mod notifications;
use self::notifications::Notification;
mod permission_popups;
mod permission_shortcuts;
mod permissions_menu;
pub(crate) use self::permissions_menu::auto_review_available;
pub(crate) use self::permissions_menu::cyber_model_approval_reviewer;
Expand Down Expand Up @@ -696,6 +697,7 @@ pub(crate) struct ChatWidget {
safety_buffering_prompt: Option<UserMessage>,
/// Main chat-surface bindings resolved from `tui.keymap.chat`.
chat_keymap: ChatKeymap,
permission_shortcut_pending: bool,
/// Keybinding to show for popping the most-recently queued message back
/// into the composer. This may differ from the first configured binding
/// when the default set includes a terminal-specific fallback.
Expand Down
1 change: 1 addition & 0 deletions codex-rs/tui/src/chatwidget/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ impl ChatWidget {
input_queue: InputQueueState::default(),
safety_buffering_prompt: None,
chat_keymap,
permission_shortcut_pending: false,
queued_message_edit_hint_binding,
show_welcome_banner: is_first_run,
startup_tooltip_override,
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/tui/src/chatwidget/interaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl ChatWidget {
return;
}

if self.handle_reasoning_shortcut(key_event) {
if self.handle_reasoning_shortcut(key_event) || self.handle_permission_shortcut(key_event) {
self.bottom_pane.clear_quit_shortcut_hint();
self.quit_shortcut_expires_at = None;
self.quit_shortcut_key = None;
Expand Down
Loading
Loading