From fce5f623691b854487a4d698b73784e4419308f9 Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Fri, 13 Feb 2026 22:42:49 -0800 Subject: [PATCH] update --- .../codex_app_server_protocol.schemas.json | 118 +++++ .../schema/json/v2/ConfigReadResponse.json | 118 +++++ .../schema/typescript/v2/AppDisabledReason.ts | 2 +- .../schema/typescript/v2/AppToolApproval.ts | 5 + .../schema/typescript/v2/AppToolDefaults.ts | 6 + .../schema/typescript/v2/AppToolsConfig.ts | 8 + .../schema/typescript/v2/AppsConfig.ts | 4 +- .../schema/typescript/v2/AppsDefaultConfig.ts | 5 + .../schema/typescript/v2/index.ts | 4 + .../app-server-protocol/src/protocol/v2.rs | 54 +++ codex-rs/app-server/README.md | 24 + .../app-server/tests/suite/v2/config_rpc.rs | 7 + codex-rs/core/config.schema.json | 105 ++++ codex-rs/core/src/app_tool_policy.rs | 452 ++++++++++++++++++ codex-rs/core/src/codex.rs | 75 ++- codex-rs/core/src/config/service.rs | 39 ++ codex-rs/core/src/config/types.rs | 169 +++++++ codex-rs/core/src/connectors.rs | 10 +- codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/mcp_tool_call.rs | 150 ++++-- .../src/tools/handlers/search_tool_bm25.rs | 74 ++- 21 files changed, 1358 insertions(+), 72 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/AppToolApproval.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/AppToolDefaults.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/AppToolsConfig.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/AppsDefaultConfig.ts create mode 100644 codex-rs/core/src/app_tool_policy.rs diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index d808219e40..d3a65af96e 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -10150,6 +10150,18 @@ }, "AppConfig": { "properties": { + "disable_destructive": { + "type": [ + "boolean", + "null" + ] + }, + "disable_open_world": { + "type": [ + "boolean", + "null" + ] + }, "disabled_reason": { "anyOf": [ { @@ -10163,12 +10175,23 @@ "enabled": { "default": true, "type": "boolean" + }, + "tools": { + "anyOf": [ + { + "$ref": "#/definitions/v2/AppToolsConfig" + }, + { + "type": "null" + } + ] } }, "type": "object" }, "AppDisabledReason": { "enum": [ + "admin_policy", "unknown", "user" ], @@ -10246,7 +10269,102 @@ "title": "AppListUpdatedNotification", "type": "object" }, + "AppToolApproval": { + "enum": [ + "auto", + "prompt", + "approve" + ], + "type": "string" + }, + "AppToolConfig": { + "properties": { + "approval": { + "anyOf": [ + { + "$ref": "#/definitions/v2/AppToolApproval" + }, + { + "type": "null" + } + ] + }, + "disabled_reason": { + "anyOf": [ + { + "$ref": "#/definitions/v2/AppDisabledReason" + }, + { + "type": "null" + } + ] + }, + "enabled": { + "type": [ + "boolean", + "null" + ] + } + }, + "type": "object" + }, + "AppToolDefaults": { + "properties": { + "approval": { + "anyOf": [ + { + "$ref": "#/definitions/v2/AppToolApproval" + }, + { + "type": "null" + } + ] + } + }, + "type": "object" + }, + "AppToolsConfig": { + "properties": { + "_default": { + "allOf": [ + { + "$ref": "#/definitions/v2/AppToolDefaults" + } + ], + "default": { + "approval": null + } + } + }, + "type": "object" + }, "AppsConfig": { + "properties": { + "_default": { + "allOf": [ + { + "$ref": "#/definitions/v2/AppsDefaultConfig" + } + ], + "default": { + "disable_destructive": false, + "disable_open_world": false + } + } + }, + "type": "object" + }, + "AppsDefaultConfig": { + "properties": { + "disable_destructive": { + "default": false, + "type": "boolean" + }, + "disable_open_world": { + "default": false, + "type": "boolean" + } + }, "type": "object" }, "AppsListParams": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json index d934df7f58..a20bb14d38 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json @@ -19,6 +19,18 @@ }, "AppConfig": { "properties": { + "disable_destructive": { + "type": [ + "boolean", + "null" + ] + }, + "disable_open_world": { + "type": [ + "boolean", + "null" + ] + }, "disabled_reason": { "anyOf": [ { @@ -32,18 +44,124 @@ "enabled": { "default": true, "type": "boolean" + }, + "tools": { + "anyOf": [ + { + "$ref": "#/definitions/AppToolsConfig" + }, + { + "type": "null" + } + ] } }, "type": "object" }, "AppDisabledReason": { "enum": [ + "admin_policy", "unknown", "user" ], "type": "string" }, + "AppToolApproval": { + "enum": [ + "auto", + "prompt", + "approve" + ], + "type": "string" + }, + "AppToolConfig": { + "properties": { + "approval": { + "anyOf": [ + { + "$ref": "#/definitions/AppToolApproval" + }, + { + "type": "null" + } + ] + }, + "disabled_reason": { + "anyOf": [ + { + "$ref": "#/definitions/AppDisabledReason" + }, + { + "type": "null" + } + ] + }, + "enabled": { + "type": [ + "boolean", + "null" + ] + } + }, + "type": "object" + }, + "AppToolDefaults": { + "properties": { + "approval": { + "anyOf": [ + { + "$ref": "#/definitions/AppToolApproval" + }, + { + "type": "null" + } + ] + } + }, + "type": "object" + }, + "AppToolsConfig": { + "properties": { + "_default": { + "allOf": [ + { + "$ref": "#/definitions/AppToolDefaults" + } + ], + "default": { + "approval": null + } + } + }, + "type": "object" + }, "AppsConfig": { + "properties": { + "_default": { + "allOf": [ + { + "$ref": "#/definitions/AppsDefaultConfig" + } + ], + "default": { + "disable_destructive": false, + "disable_open_world": false + } + } + }, + "type": "object" + }, + "AppsDefaultConfig": { + "properties": { + "disable_destructive": { + "default": false, + "type": "boolean" + }, + "disable_open_world": { + "default": false, + "type": "boolean" + } + }, "type": "object" }, "AskForApproval": { diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AppDisabledReason.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AppDisabledReason.ts index bc44835a76..9af9e16e70 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AppDisabledReason.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AppDisabledReason.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type AppDisabledReason = "unknown" | "user"; +export type AppDisabledReason = "admin_policy" | "unknown" | "user"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AppToolApproval.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AppToolApproval.ts new file mode 100644 index 0000000000..e92cd8e28b --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AppToolApproval.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type AppToolApproval = "auto" | "prompt" | "approve"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AppToolDefaults.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AppToolDefaults.ts new file mode 100644 index 0000000000..9205e91afd --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AppToolDefaults.ts @@ -0,0 +1,6 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AppToolApproval } from "./AppToolApproval"; + +export type AppToolDefaults = { approval: AppToolApproval | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AppToolsConfig.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AppToolsConfig.ts new file mode 100644 index 0000000000..81f529b5fa --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AppToolsConfig.ts @@ -0,0 +1,8 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AppDisabledReason } from "./AppDisabledReason"; +import type { AppToolApproval } from "./AppToolApproval"; +import type { AppToolDefaults } from "./AppToolDefaults"; + +export type AppToolsConfig = { _default: AppToolDefaults, } & ({ [key in string]?: { enabled: boolean | null, disabled_reason: AppDisabledReason | null, approval: AppToolApproval | null, } }); diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AppsConfig.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AppsConfig.ts index ae3e455201..22bded360b 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AppsConfig.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AppsConfig.ts @@ -2,5 +2,7 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AppDisabledReason } from "./AppDisabledReason"; +import type { AppToolsConfig } from "./AppToolsConfig"; +import type { AppsDefaultConfig } from "./AppsDefaultConfig"; -export type AppsConfig = { [key in string]?: { enabled: boolean, disabled_reason: AppDisabledReason | null, } }; +export type AppsConfig = { _default: AppsDefaultConfig, } & ({ [key in string]?: { enabled: boolean, disabled_reason: AppDisabledReason | null, disable_destructive: boolean | null, disable_open_world: boolean | null, tools: AppToolsConfig | null, } }); diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AppsDefaultConfig.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AppsDefaultConfig.ts new file mode 100644 index 0000000000..b5e4610f7e --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AppsDefaultConfig.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type AppsDefaultConfig = { disable_destructive: boolean, disable_open_world: boolean, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index 500b1d930f..54322bf839 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -9,7 +9,11 @@ export type { AnalyticsConfig } from "./AnalyticsConfig"; export type { AppDisabledReason } from "./AppDisabledReason"; export type { AppInfo } from "./AppInfo"; export type { AppListUpdatedNotification } from "./AppListUpdatedNotification"; +export type { AppToolApproval } from "./AppToolApproval"; +export type { AppToolDefaults } from "./AppToolDefaults"; +export type { AppToolsConfig } from "./AppToolsConfig"; export type { AppsConfig } from "./AppsConfig"; +export type { AppsDefaultConfig } from "./AppsDefaultConfig"; export type { AppsListParams } from "./AppsListParams"; export type { AppsListResponse } from "./AppsListResponse"; export type { AskForApproval } from "./AskForApproval"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 9bbd8fe6f6..4152f5da20 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -372,10 +372,59 @@ pub struct AnalyticsConfig { #[serde(rename_all = "snake_case")] #[ts(export_to = "v2/")] pub enum AppDisabledReason { + AdminPolicy, Unknown, User, } +#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +#[ts(export_to = "v2/")] +pub enum AppToolApproval { + Auto, + Prompt, + Approve, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +#[ts(export_to = "v2/")] +#[derive(Default)] +pub struct AppToolDefaults { + pub approval: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +#[ts(export_to = "v2/")] +pub struct AppToolConfig { + pub enabled: Option, + pub disabled_reason: Option, + pub approval: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +#[ts(export_to = "v2/")] +pub struct AppToolsConfig { + #[serde(default, rename = "_default")] + pub default: AppToolDefaults, + #[serde(default, flatten)] + #[schemars(with = "HashMap")] + pub tools: HashMap, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +#[ts(export_to = "v2/")] +#[derive(Default)] +pub struct AppsDefaultConfig { + #[serde(default)] + pub disable_destructive: bool, + #[serde(default)] + pub disable_open_world: bool, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] #[ts(export_to = "v2/")] @@ -383,12 +432,17 @@ pub struct AppConfig { #[serde(default = "default_enabled")] pub enabled: bool, pub disabled_reason: Option, + pub disable_destructive: Option, + pub disable_open_world: Option, + pub tools: Option, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] #[ts(export_to = "v2/")] pub struct AppsConfig { + #[serde(default, rename = "_default")] + pub default: AppsDefaultConfig, #[serde(default, flatten)] #[schemars(with = "HashMap")] pub apps: HashMap, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 39ddc73c65..c798073056 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -818,6 +818,30 @@ The server also emits `app/list/updated` notifications whenever either source (a } ``` +App behavior can be configured in `config.toml` under `[apps]`, including app-level and tool-level controls: + +```toml +[apps._default] +disable_destructive = false +disable_open_world = false + +[apps.connector_123] +enabled = false +disabled_reason = "admin_policy" +disable_destructive = true +disable_open_world = true + +[apps.connector_123.tools._default] +approval = "prompt" # "auto" | "prompt" | "approve" + +[apps.connector_123.tools."repos/list"] +approval = "approve" + +[apps.connector_123.tools."issues/create"] +enabled = false +disabled_reason = "admin_policy" +``` + Invoke an app by inserting `$` in the text input. The slug is derived from the app name and lowercased with non-alphanumeric characters replaced by `-` (for example, "Demo App" becomes `$demo-app`). Add a `mention` input item (recommended) so the server uses the exact `app://` path rather than guessing by name. Example: diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index de4c51cde4..37d6eb2949 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -185,11 +185,18 @@ disabled_reason = "user" assert_eq!( config.apps, Some(AppsConfig { + default: codex_app_server_protocol::AppsDefaultConfig { + disable_destructive: false, + disable_open_world: false, + }, apps: std::collections::HashMap::from([( "app1".to_string(), AppConfig { enabled: false, disabled_reason: Some(AppDisabledReason::User), + disable_destructive: None, + disable_open_world: None, + tools: None, }, )]), }) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 7a1c40a0a1..f95958830e 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -59,6 +59,14 @@ "additionalProperties": false, "description": "Config values for a single app/connector.", "properties": { + "disable_destructive": { + "description": "App-level override for disabling destructive tools.", + "type": "boolean" + }, + "disable_open_world": { + "description": "App-level override for disabling open-world tools.", + "type": "boolean" + }, "disabled_reason": { "allOf": [ { @@ -71,22 +79,119 @@ "default": true, "description": "When `false`, Codex does not surface this app.", "type": "boolean" + }, + "tools": { + "allOf": [ + { + "$ref": "#/definitions/AppToolsConfigToml" + } + ], + "description": "Per-tool policy for this app." } }, "type": "object" }, "AppDisabledReason": { "enum": [ + "admin_policy", "unknown", "user" ], "type": "string" }, + "AppToolApproval": { + "enum": [ + "auto", + "prompt", + "approve" + ], + "type": "string" + }, + "AppToolConfig": { + "additionalProperties": false, + "properties": { + "approval": { + "allOf": [ + { + "$ref": "#/definitions/AppToolApproval" + } + ], + "description": "Approval behavior for this specific tool." + }, + "disabled_reason": { + "allOf": [ + { + "$ref": "#/definitions/AppDisabledReason" + } + ], + "description": "Reason this tool was disabled." + }, + "enabled": { + "description": "When `false`, this individual tool is disabled.", + "type": "boolean" + } + }, + "type": "object" + }, + "AppToolDefaults": { + "additionalProperties": false, + "properties": { + "approval": { + "allOf": [ + { + "$ref": "#/definitions/AppToolApproval" + } + ], + "description": "Default approval mode for tools in this app." + } + }, + "type": "object" + }, + "AppToolsConfigToml": { + "additionalProperties": { + "$ref": "#/definitions/AppToolConfig" + }, + "properties": { + "_default": { + "allOf": [ + { + "$ref": "#/definitions/AppToolDefaults" + } + ], + "description": "Default tool settings under `[apps..tools._default]`." + } + }, + "type": "object" + }, "AppsConfigToml": { "additionalProperties": { "$ref": "#/definitions/AppConfig" }, "description": "App/connector settings loaded from `config.toml`.", + "properties": { + "_default": { + "allOf": [ + { + "$ref": "#/definitions/AppsDefaultConfig" + } + ], + "description": "Defaults shared by all apps unless overridden by app-specific config." + } + }, + "type": "object" + }, + "AppsDefaultConfig": { + "additionalProperties": false, + "properties": { + "disable_destructive": { + "description": "Disable tools with `destructive_hint = true` unless app-level override says otherwise.", + "type": "boolean" + }, + "disable_open_world": { + "description": "Disable tools with `open_world_hint = true` unless app-level override says otherwise.", + "type": "boolean" + } + }, "type": "object" }, "AskForApproval": { diff --git a/codex-rs/core/src/app_tool_policy.rs b/codex-rs/core/src/app_tool_policy.rs new file mode 100644 index 0000000000..da0f873d5e --- /dev/null +++ b/codex-rs/core/src/app_tool_policy.rs @@ -0,0 +1,452 @@ +use crate::config::Config; +use crate::config::types::AppConfig; +use crate::config::types::AppDisabledReason; +use crate::config::types::AppToolApproval; +use crate::config::types::AppToolConfig; +use crate::config::types::AppsConfigToml; +use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; +use crate::mcp_connection_manager::ToolInfo; +use serde::Deserialize; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub(crate) enum ResolvedToolApprovalMode { + #[default] + Auto, + Prompt, + Approve, +} + +impl From for ResolvedToolApprovalMode { + fn from(value: AppToolApproval) -> Self { + match value { + AppToolApproval::Auto => Self::Auto, + AppToolApproval::Prompt => Self::Prompt, + AppToolApproval::Approve => Self::Approve, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum AppToolBlockReason { + MissingConnectorId, + AppDisabled { + disabled_reason: Option, + }, + ToolDisabled { + disabled_reason: Option, + }, + DestructiveDisallowed, + OpenWorldDisallowed, +} + +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub(crate) struct ResolvedAppToolPolicy { + pub(crate) approval_mode: ResolvedToolApprovalMode, + pub(crate) block_reason: Option, +} + +impl ResolvedAppToolPolicy { + pub(crate) fn is_allowed(&self) -> bool { + self.block_reason.is_none() + } +} + +pub(crate) fn read_apps_config(config: &Config) -> Option { + let effective_config = config.config_layer_stack.effective_config(); + let apps_config = effective_config.as_table()?.get("apps")?.clone(); + AppsConfigToml::deserialize(apps_config).ok() +} + +pub(crate) fn resolve_app_tool_policy( + apps_config: Option<&AppsConfigToml>, + tool: &ToolInfo, +) -> ResolvedAppToolPolicy { + if tool.server_name != CODEX_APPS_MCP_SERVER_NAME { + return ResolvedAppToolPolicy::default(); + } + + let Some(connector_id) = tool.connector_id.as_deref() else { + return ResolvedAppToolPolicy { + approval_mode: ResolvedToolApprovalMode::Auto, + block_reason: Some(AppToolBlockReason::MissingConnectorId), + }; + }; + + let app_config = apps_config.and_then(|apps| apps.apps.get(connector_id)); + let tool_config = tool_config(app_config, &tool.tool_name); + let approval_mode = resolve_approval_mode(app_config, tool_config); + + if app_config.is_some_and(|app| !app.enabled) { + return ResolvedAppToolPolicy { + approval_mode, + block_reason: Some(AppToolBlockReason::AppDisabled { + disabled_reason: app_config.and_then(|app| app.disabled_reason.clone()), + }), + }; + } + + if tool_config.is_some_and(|tool_cfg| tool_cfg.enabled == Some(false)) { + return ResolvedAppToolPolicy { + approval_mode, + block_reason: Some(AppToolBlockReason::ToolDisabled { + disabled_reason: tool_config.and_then(|tool_cfg| tool_cfg.disabled_reason.clone()), + }), + }; + } + + if destructive_hint_is_blocked(apps_config, app_config, tool) { + return ResolvedAppToolPolicy { + approval_mode, + block_reason: Some(AppToolBlockReason::DestructiveDisallowed), + }; + } + + if open_world_hint_is_blocked(apps_config, app_config, tool) { + return ResolvedAppToolPolicy { + approval_mode, + block_reason: Some(AppToolBlockReason::OpenWorldDisallowed), + }; + } + + ResolvedAppToolPolicy { + approval_mode, + block_reason: None, + } +} + +pub(crate) fn blocked_message( + reason: &AppToolBlockReason, + tool_name: &str, + connector_name: Option<&str>, +) -> String { + let app_label = connector_name.unwrap_or("This app"); + match reason { + AppToolBlockReason::MissingConnectorId => { + format!("tool \"{tool_name}\" is missing connector metadata") + } + AppToolBlockReason::AppDisabled { disabled_reason } => match disabled_reason { + Some(disabled_reason) => { + format!( + "{app_label} is disabled ({disabled_reason}); tool \"{tool_name}\" is blocked" + ) + } + None => format!("{app_label} is disabled; tool \"{tool_name}\" is blocked"), + }, + AppToolBlockReason::ToolDisabled { disabled_reason } => match disabled_reason { + Some(disabled_reason) => { + format!("{app_label} tool \"{tool_name}\" is disabled ({disabled_reason})") + } + None => format!("{app_label} tool \"{tool_name}\" is disabled"), + }, + AppToolBlockReason::DestructiveDisallowed => { + format!("{app_label} tool \"{tool_name}\" is blocked by disable_destructive policy") + } + AppToolBlockReason::OpenWorldDisallowed => { + format!("{app_label} tool \"{tool_name}\" is blocked by disable_open_world policy") + } + } +} + +fn resolve_approval_mode( + app_config: Option<&AppConfig>, + tool_config: Option<&AppToolConfig>, +) -> ResolvedToolApprovalMode { + tool_config + .and_then(|tool_cfg| tool_cfg.approval) + .or_else(|| { + app_config + .and_then(|app| app.tools.as_ref()) + .and_then(|tools| tools.default.approval) + }) + .unwrap_or_default() + .into() +} + +fn tool_config<'a>( + app_config: Option<&'a AppConfig>, + tool_name: &str, +) -> Option<&'a AppToolConfig> { + app_config + .and_then(|app| app.tools.as_ref()) + .and_then(|tools| tools.tools.get(tool_name)) +} + +fn destructive_hint_is_blocked( + apps_config: Option<&AppsConfigToml>, + app_config: Option<&AppConfig>, + tool: &ToolInfo, +) -> bool { + let disable_destructive = app_config + .and_then(|app| app.disable_destructive) + .unwrap_or_else(|| { + apps_config + .map(|apps| apps.default.disable_destructive) + .unwrap_or(false) + }); + disable_destructive + && tool + .tool + .annotations + .as_ref() + .and_then(|a| a.destructive_hint) + == Some(true) +} + +fn open_world_hint_is_blocked( + apps_config: Option<&AppsConfigToml>, + app_config: Option<&AppConfig>, + tool: &ToolInfo, +) -> bool { + let disable_open_world = app_config + .and_then(|app| app.disable_open_world) + .unwrap_or_else(|| { + apps_config + .map(|apps| apps.default.disable_open_world) + .unwrap_or(false) + }); + disable_open_world + && tool + .tool + .annotations + .as_ref() + .and_then(|a| a.open_world_hint) + == Some(true) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::types::AppToolDefaults; + use crate::config::types::AppToolsConfigToml; + use pretty_assertions::assert_eq; + use rmcp::model::JsonObject; + use rmcp::model::Tool; + use rmcp::model::ToolAnnotations; + use std::collections::HashMap; + use std::sync::Arc; + + fn make_tool( + server_name: &str, + tool_name: &str, + connector_id: Option<&str>, + destructive_hint: Option, + open_world_hint: Option, + ) -> ToolInfo { + ToolInfo { + server_name: server_name.to_string(), + tool_name: tool_name.to_string(), + tool: Tool { + name: tool_name.to_string().into(), + title: None, + description: None, + input_schema: Arc::new(JsonObject::default()), + output_schema: None, + annotations: Some(ToolAnnotations { + destructive_hint, + idempotent_hint: None, + open_world_hint, + read_only_hint: None, + title: None, + }), + execution: None, + icons: None, + meta: None, + }, + connector_id: connector_id.map(str::to_string), + connector_name: connector_id.map(str::to_string), + } + } + + #[test] + fn non_codex_apps_tool_uses_default_policy() { + let tool = make_tool( + "other_server", + "issues/create", + Some("github"), + Some(true), + None, + ); + assert_eq!( + resolve_app_tool_policy(None, &tool), + ResolvedAppToolPolicy::default() + ); + } + + #[test] + fn app_disabled_blocks_tool() { + let tool = make_tool( + CODEX_APPS_MCP_SERVER_NAME, + "issues/create", + Some("connector_123"), + None, + None, + ); + let apps = AppsConfigToml { + default: Default::default(), + apps: HashMap::from([( + "connector_123".to_string(), + AppConfig { + enabled: false, + disabled_reason: Some(AppDisabledReason::AdminPolicy), + disable_destructive: None, + disable_open_world: None, + tools: None, + }, + )]), + }; + assert_eq!( + resolve_app_tool_policy(Some(&apps), &tool), + ResolvedAppToolPolicy { + approval_mode: ResolvedToolApprovalMode::Auto, + block_reason: Some(AppToolBlockReason::AppDisabled { + disabled_reason: Some(AppDisabledReason::AdminPolicy), + }), + } + ); + } + + #[test] + fn app_default_disables_destructive_tools() { + let tool = make_tool( + CODEX_APPS_MCP_SERVER_NAME, + "issues/create", + Some("connector_123"), + Some(true), + None, + ); + let apps = AppsConfigToml { + default: crate::config::types::AppsDefaultConfig { + disable_destructive: true, + disable_open_world: false, + }, + apps: HashMap::new(), + }; + assert_eq!( + resolve_app_tool_policy(Some(&apps), &tool), + ResolvedAppToolPolicy { + approval_mode: ResolvedToolApprovalMode::Auto, + block_reason: Some(AppToolBlockReason::DestructiveDisallowed), + } + ); + } + + #[test] + fn app_override_reenables_destructive_tools() { + let tool = make_tool( + CODEX_APPS_MCP_SERVER_NAME, + "issues/create", + Some("connector_123"), + Some(true), + None, + ); + let apps = AppsConfigToml { + default: crate::config::types::AppsDefaultConfig { + disable_destructive: true, + disable_open_world: false, + }, + apps: HashMap::from([( + "connector_123".to_string(), + AppConfig { + enabled: true, + disabled_reason: None, + disable_destructive: Some(false), + disable_open_world: None, + tools: None, + }, + )]), + }; + assert_eq!( + resolve_app_tool_policy(Some(&apps), &tool), + ResolvedAppToolPolicy { + approval_mode: ResolvedToolApprovalMode::Auto, + block_reason: None, + } + ); + } + + #[test] + fn per_tool_disable_blocks_tool() { + let tool = make_tool( + CODEX_APPS_MCP_SERVER_NAME, + "issues/create", + Some("connector_123"), + None, + None, + ); + let apps = AppsConfigToml { + default: Default::default(), + apps: HashMap::from([( + "connector_123".to_string(), + AppConfig { + enabled: true, + disabled_reason: None, + disable_destructive: None, + disable_open_world: None, + tools: Some(AppToolsConfigToml { + default: AppToolDefaults { approval: None }, + tools: HashMap::from([( + "issues/create".to_string(), + AppToolConfig { + enabled: Some(false), + disabled_reason: Some(AppDisabledReason::AdminPolicy), + approval: None, + }, + )]), + }), + }, + )]), + }; + assert_eq!( + resolve_app_tool_policy(Some(&apps), &tool), + ResolvedAppToolPolicy { + approval_mode: ResolvedToolApprovalMode::Auto, + block_reason: Some(AppToolBlockReason::ToolDisabled { + disabled_reason: Some(AppDisabledReason::AdminPolicy), + }), + } + ); + } + + #[test] + fn per_tool_approval_overrides_app_default() { + let tool = make_tool( + CODEX_APPS_MCP_SERVER_NAME, + "issues/create", + Some("connector_123"), + None, + None, + ); + let apps = AppsConfigToml { + default: Default::default(), + apps: HashMap::from([( + "connector_123".to_string(), + AppConfig { + enabled: true, + disabled_reason: None, + disable_destructive: None, + disable_open_world: None, + tools: Some(AppToolsConfigToml { + default: AppToolDefaults { + approval: Some(AppToolApproval::Prompt), + }, + tools: HashMap::from([( + "issues/create".to_string(), + AppToolConfig { + enabled: None, + disabled_reason: None, + approval: Some(AppToolApproval::Approve), + }, + )]), + }), + }, + )]), + }; + assert_eq!( + resolve_app_tool_policy(Some(&apps), &tool), + ResolvedAppToolPolicy { + approval_mode: ResolvedToolApprovalMode::Approve, + block_reason: None, + } + ); + } +} diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 59cbf57dd5..a09238a3e7 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -16,6 +16,7 @@ use crate::agent::agent_status_from_event; use crate::analytics_client::AnalyticsEventsClient; use crate::analytics_client::AppInvocation; use crate::analytics_client::build_track_events_context; +use crate::app_tool_policy; use crate::apps::render_apps_section; use crate::compact; use crate::compact::run_inline_auto_compact_task; @@ -4633,6 +4634,7 @@ fn connector_inserted_in_messages( fn filter_codex_apps_mcp_tools( mcp_tools: &HashMap, connectors: &[connectors::AppInfo], + apps_config: Option<&crate::config::types::AppsConfigToml>, ) -> HashMap { let allowed: HashSet<&str> = connectors .iter() @@ -4648,7 +4650,11 @@ fn filter_codex_apps_mcp_tools( let Some(connector_id) = codex_apps_connector_id(tool) else { return false; }; - allowed.contains(connector_id) + if !allowed.contains(connector_id) { + return false; + } + + app_tool_policy::resolve_app_tool_policy(apps_config, tool).is_allowed() }) .map(|(name, tool)| (name.clone(), tool.clone())) .collect() @@ -4809,6 +4815,7 @@ async fn built_tools( .list_all_tools() .or_cancel(cancellation_token) .await?; + let apps_config = app_tool_policy::read_apps_config(&turn_context.config); let mut effective_explicitly_enabled_connectors = explicitly_enabled_connectors.clone(); effective_explicitly_enabled_connectors.extend(sess.get_connector_selection().await); @@ -4845,12 +4852,13 @@ async fn built_tools( filter_codex_apps_mcp_tools_only(&mcp_tools, explicitly_enabled.as_ref()); selected_mcp_tools.extend(apps_mcp_tools); - mcp_tools = selected_mcp_tools; + mcp_tools = + filter_codex_apps_mcp_tools(&selected_mcp_tools, connectors, apps_config.as_ref()); } - let app_tools = connectors - .as_ref() - .map(|connectors| filter_codex_apps_mcp_tools(&mcp_tools, connectors)); + let app_tools = connectors.as_ref().map(|connectors| { + filter_codex_apps_mcp_tools(&mcp_tools, connectors, apps_config.as_ref()) + }); Ok(Arc::new(ToolRouter::from_config( &turn_context.tools_config, @@ -5638,6 +5646,7 @@ mod tests { use pretty_assertions::assert_eq; use rmcp::model::JsonObject; use rmcp::model::Tool; + use rmcp::model::ToolAnnotations; use serde::Deserialize; use serde_json::json; use std::path::PathBuf; @@ -5712,6 +5721,36 @@ mod tests { } } + fn make_mcp_tool_with_open_world_hint( + tool_name: &str, + connector_id: Option<&str>, + connector_name: Option<&str>, + ) -> ToolInfo { + ToolInfo { + server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), + tool_name: tool_name.to_string(), + tool: Tool { + name: tool_name.to_string().into(), + title: None, + description: Some(format!("Test tool: {tool_name}").into()), + input_schema: Arc::new(JsonObject::default()), + output_schema: None, + annotations: Some(ToolAnnotations { + destructive_hint: None, + idempotent_hint: None, + open_world_hint: Some(true), + read_only_hint: None, + title: None, + }), + execution: None, + icons: None, + meta: None, + }, + connector_id: connector_id.map(str::to_string), + connector_name: connector_name.map(str::to_string), + } + } + #[tokio::test] async fn get_base_instructions_no_user_content() { let prompt_with_apply_patch_instructions = @@ -5988,6 +6027,32 @@ mod tests { ); } + #[test] + fn filter_codex_apps_mcp_tools_applies_policy() { + let mcp_tools = HashMap::from([( + "mcp__codex_apps__calendar_search".to_string(), + make_mcp_tool_with_open_world_hint( + "calendar_search", + Some("calendar"), + Some("Calendar"), + ), + )]); + let connectors = vec![make_connector("calendar", "Calendar")]; + let apps_config = crate::config::types::AppsConfigToml { + default: crate::config::types::AppsDefaultConfig { + disable_destructive: false, + disable_open_world: true, + }, + apps: HashMap::new(), + }; + + let filtered = filter_codex_apps_mcp_tools(&mcp_tools, &connectors, Some(&apps_config)); + assert_eq!( + filtered.into_keys().collect::>(), + Vec::::new() + ); + } + #[tokio::test] async fn reconstruct_history_matches_live_compactions() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/core/src/config/service.rs index db6963a2f8..c7cef49d57 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/core/src/config/service.rs @@ -835,6 +835,28 @@ remote_models = true .await .expect("write apps.app1.disabled_reason succeeds"); + service + .write_value(ConfigValueWriteParams { + file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), + key_path: "apps.app1.tools.issues/create.enabled".to_string(), + value: serde_json::json!(false), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect("write apps.app1.tools.issues/create.enabled succeeds"); + + service + .write_value(ConfigValueWriteParams { + file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), + key_path: "apps.app1.tools.issues/create.disabled_reason".to_string(), + value: serde_json::json!("admin_policy"), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect("write apps.app1.tools.issues/create.disabled_reason succeeds"); + let read = service .read(ConfigReadParams { include_layers: false, @@ -846,11 +868,28 @@ remote_models = true assert_eq!( read.config.apps, Some(AppsConfig { + default: codex_app_server_protocol::AppsDefaultConfig { + disable_destructive: false, + disable_open_world: false, + }, apps: std::collections::HashMap::from([( "app1".to_string(), AppConfig { enabled: false, disabled_reason: Some(AppDisabledReason::User), + disable_destructive: None, + disable_open_world: None, + tools: Some(codex_app_server_protocol::AppToolsConfig { + default: codex_app_server_protocol::AppToolDefaults { approval: None }, + tools: std::collections::HashMap::from([( + "issues/create".to_string(), + codex_app_server_protocol::AppToolConfig { + enabled: Some(false), + disabled_reason: Some(AppDisabledReason::AdminPolicy), + approval: None, + }, + )]), + }), }, )]), }) diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 80c16281b0..7dfcfbfb21 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -428,6 +428,7 @@ impl From for MemoriesConfig { #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum AppDisabledReason { + AdminPolicy, Unknown, User, } @@ -435,12 +436,74 @@ pub enum AppDisabledReason { impl fmt::Display for AppDisabledReason { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + AppDisabledReason::AdminPolicy => write!(f, "admin_policy"), AppDisabledReason::Unknown => write!(f, "unknown"), AppDisabledReason::User => write!(f, "user"), } } } +#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, Default, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum AppToolApproval { + #[default] + Auto, + Prompt, + Approve, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct AppsDefaultConfig { + /// Disable tools with `destructive_hint = true` unless app-level override says otherwise. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub disable_destructive: bool, + + /// Disable tools with `open_world_hint = true` unless app-level override says otherwise. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub disable_open_world: bool, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct AppToolDefaults { + /// Default approval mode for tools in this app. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub approval: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct AppToolConfig { + /// When `false`, this individual tool is disabled. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub enabled: Option, + + /// Reason this tool was disabled. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub disabled_reason: Option, + + /// Approval behavior for this specific tool. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub approval: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct AppToolsConfigToml { + /// Default tool settings under `[apps..tools._default]`. + #[serde( + default, + rename = "_default", + skip_serializing_if = "app_tool_defaults_is_empty" + )] + pub default: AppToolDefaults, + + /// Per-tool overrides keyed by MCP tool name. + #[serde(default, flatten)] + pub tools: HashMap, +} + /// Config values for a single app/connector. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] #[schemars(deny_unknown_fields)] @@ -452,17 +515,45 @@ pub struct AppConfig { /// Reason this app was disabled. #[serde(default, skip_serializing_if = "Option::is_none")] pub disabled_reason: Option, + + /// App-level override for disabling destructive tools. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub disable_destructive: Option, + + /// App-level override for disabling open-world tools. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub disable_open_world: Option, + + /// Per-tool policy for this app. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub tools: Option, } /// App/connector settings loaded from `config.toml`. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct AppsConfigToml { + /// Defaults shared by all apps unless overridden by app-specific config. + #[serde( + default, + rename = "_default", + skip_serializing_if = "apps_default_config_is_empty" + )] + pub default: AppsDefaultConfig, + /// Per-app settings keyed by app ID (for example `[apps.google_drive]`). #[serde(default, flatten)] pub apps: HashMap, } +fn apps_default_config_is_empty(config: &AppsDefaultConfig) -> bool { + !config.disable_destructive && !config.disable_open_world +} + +fn app_tool_defaults_is_empty(config: &AppToolDefaults) -> bool { + config.approval.is_none() +} + // ===== OTEL configuration ===== #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema)] @@ -1093,4 +1184,82 @@ mod tests { "unexpected error: {err}" ); } + + #[test] + fn deserialize_apps_config_with_defaults_and_tool_overrides() { + let cfg: AppsConfigToml = toml::from_str( + r#" + [_default] + disable_destructive = true + disable_open_world = false + + [connector_123] + enabled = false + disabled_reason = "admin_policy" + disable_destructive = false + disable_open_world = true + + [connector_123.tools._default] + approval = "prompt" + + [connector_123.tools."repos/list"] + approval = "approve" + + [connector_123.tools."issues/create"] + enabled = false + disabled_reason = "admin_policy" + "#, + ) + .expect("deserialize apps config"); + + assert_eq!( + cfg, + AppsConfigToml { + default: AppsDefaultConfig { + disable_destructive: true, + disable_open_world: false, + }, + apps: HashMap::from([( + "connector_123".to_string(), + AppConfig { + enabled: false, + disabled_reason: Some(AppDisabledReason::AdminPolicy), + disable_destructive: Some(false), + disable_open_world: Some(true), + tools: Some(AppToolsConfigToml { + default: AppToolDefaults { + approval: Some(AppToolApproval::Prompt), + }, + tools: HashMap::from([ + ( + "repos/list".to_string(), + AppToolConfig { + enabled: None, + disabled_reason: None, + approval: Some(AppToolApproval::Approve), + }, + ), + ( + "issues/create".to_string(), + AppToolConfig { + enabled: Some(false), + disabled_reason: Some(AppDisabledReason::AdminPolicy), + approval: None, + }, + ), + ]), + }), + }, + )]), + } + ); + } + + #[test] + fn app_disabled_reason_display_admin_policy() { + assert_eq!( + AppDisabledReason::AdminPolicy.to_string(), + "admin_policy".to_string() + ); + } } diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 35c2dc5ceb..a2b1352092 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -9,7 +9,6 @@ use std::time::Instant; use async_channel::unbounded; pub use codex_app_server_protocol::AppInfo; use codex_protocol::protocol::SandboxPolicy; -use serde::Deserialize; use tokio_util::sync::CancellationToken; use tracing::warn; @@ -17,7 +16,6 @@ use crate::AuthManager; use crate::CodexAuth; use crate::SandboxState; use crate::config::Config; -use crate::config::types::AppsConfigToml; use crate::features::Feature; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::auth::compute_auth_statuses; @@ -281,7 +279,7 @@ pub fn merge_connectors( } pub fn with_app_enabled_state(mut connectors: Vec, config: &Config) -> Vec { - let apps = read_apps_config(config).map(|apps_config| apps_config.apps); + let apps = crate::app_tool_policy::read_apps_config(config).map(|apps_config| apps_config.apps); for connector in &mut connectors { if let Some(app) = apps.as_ref().and_then(|apps| apps.get(&connector.id)) { connector.is_enabled = app.enabled; @@ -290,12 +288,6 @@ pub fn with_app_enabled_state(mut connectors: Vec, config: &Config) -> connectors } -fn read_apps_config(config: &Config) -> Option { - let effective_config = config.config_layer_stack.effective_config(); - let apps_config = effective_config.as_table()?.get("apps")?.clone(); - AppsConfigToml::deserialize(apps_config).ok() -} - fn collect_accessible_connectors(tools: I) -> Vec where I: IntoIterator)>, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 8b396517e2..e4c64d0eef 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -7,6 +7,7 @@ mod analytics_client; pub mod api_bridge; +mod app_tool_policy; mod apply_patch; mod apps; pub mod auth; diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 745633b148..4c028b1cb3 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -5,6 +5,7 @@ use tracing::error; use crate::analytics_client::AppInvocation; use crate::analytics_client::build_track_events_context; +use crate::app_tool_policy; use crate::codex::Session; use crate::codex::TurnContext; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; @@ -129,6 +130,16 @@ pub(crate) async fn handle_mcp_tool_call( ) .await } + McpToolApprovalDecision::BlockedByPolicy(message) => { + notify_mcp_tool_call_skip( + sess.as_ref(), + turn_context, + &call_id, + invocation, + message, + ) + .await + } }; let status = if result.is_ok() { "ok" } else { "error" }; @@ -258,12 +269,13 @@ async fn maybe_track_codex_app_used( ); } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] enum McpToolApprovalDecision { Accept, AcceptAndRemember, Decline, Cancel, + BlockedByPolicy(String), } struct McpToolApprovalMetadata { @@ -293,31 +305,64 @@ async fn maybe_request_mcp_tool_approval( server: &str, tool_name: &str, ) -> Option { - if is_full_access_mode(turn_context) { - return None; - } if server != CODEX_APPS_MCP_SERVER_NAME { return None; } - let metadata = lookup_mcp_tool_metadata(sess, server, tool_name).await?; - if !requires_mcp_tool_approval(&metadata.annotations) { - return None; + let tool_info = lookup_mcp_tool_info(sess, server, tool_name).await?; + let apps_config = app_tool_policy::read_apps_config(&turn_context.config); + let resolved_policy = + app_tool_policy::resolve_app_tool_policy(apps_config.as_ref(), &tool_info); + if let Some(reason) = resolved_policy.block_reason.as_ref() { + let message = app_tool_policy::blocked_message( + reason, + tool_name, + tool_info.connector_name.as_deref(), + ); + return Some(McpToolApprovalDecision::BlockedByPolicy(message)); } - let approval_key = metadata - .connector_id - .as_deref() - .map(|connector_id| McpToolApprovalKey { - server: server.to_string(), - connector_id: connector_id.to_string(), - tool_name: tool_name.to_string(), - }); - if let Some(key) = approval_key.as_ref() + + let metadata = mcp_tool_approval_metadata_from_tool_info(tool_info); + if matches!( + resolved_policy.approval_mode, + app_tool_policy::ResolvedToolApprovalMode::Approve + ) { + return Some(McpToolApprovalDecision::Accept); + } + + let always_prompt = matches!( + resolved_policy.approval_mode, + app_tool_policy::ResolvedToolApprovalMode::Prompt + ); + if !always_prompt { + if is_full_access_mode(turn_context) { + return None; + } + if !requires_mcp_tool_approval(&metadata.annotations) { + return None; + } + } + + let approval_key = if always_prompt { + None + } else { + metadata + .connector_id + .as_deref() + .map(|connector_id| McpToolApprovalKey { + server: server.to_string(), + connector_id: connector_id.to_string(), + tool_name: tool_name.to_string(), + }) + }; + if !always_prompt + && let Some(key) = approval_key.as_ref() && mcp_tool_approval_is_remembered(sess, key).await { return Some(McpToolApprovalDecision::Accept); } + let allow_remember_option = !always_prompt && approval_key.is_some(); let question_id = format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_{call_id}"); let question = build_mcp_tool_approval_question( question_id.clone(), @@ -325,7 +370,7 @@ async fn maybe_request_mcp_tool_approval( metadata.tool_title.as_deref(), metadata.connector_name.as_deref(), &metadata.annotations, - approval_key.is_some(), + allow_remember_option, ); let args = RequestUserInputArgs { questions: vec![question], @@ -334,7 +379,8 @@ async fn maybe_request_mcp_tool_approval( .request_user_input(turn_context, call_id.to_string(), args) .await; let decision = parse_mcp_tool_approval_response(response, &question_id); - if matches!(decision, McpToolApprovalDecision::AcceptAndRemember) + if !always_prompt + && matches!(decision, McpToolApprovalDecision::AcceptAndRemember) && let Some(key) = approval_key { remember_mcp_tool_approval(sess, key).await; @@ -350,11 +396,35 @@ fn is_full_access_mode(turn_context: &TurnContext) -> bool { ) } -async fn lookup_mcp_tool_metadata( +fn mcp_tool_approval_metadata_from_tool_info( + tool_info: crate::mcp_connection_manager::ToolInfo, +) -> McpToolApprovalMetadata { + McpToolApprovalMetadata { + annotations: tool_info + .tool + .annotations + .unwrap_or_else(default_tool_annotations), + connector_id: tool_info.connector_id, + connector_name: tool_info.connector_name, + tool_title: tool_info.tool.title, + } +} + +fn default_tool_annotations() -> ToolAnnotations { + ToolAnnotations { + destructive_hint: None, + idempotent_hint: None, + open_world_hint: None, + read_only_hint: None, + title: None, + } +} + +async fn lookup_mcp_tool_info( sess: &Session, server: &str, tool_name: &str, -) -> Option { +) -> Option { let tools = sess .services .mcp_connection_manager @@ -363,21 +433,9 @@ async fn lookup_mcp_tool_metadata( .list_all_tools() .await; - tools.into_values().find_map(|tool_info| { - if tool_info.server_name == server && tool_info.tool_name == tool_name { - tool_info - .tool - .annotations - .map(|annotations| McpToolApprovalMetadata { - annotations, - connector_id: tool_info.connector_id, - connector_name: tool_info.connector_name, - tool_title: tool_info.tool.title, - }) - } else { - None - } - }) + tools + .into_values() + .find(|tool_info| tool_info.server_name == server && tool_info.tool_name == tool_name) } async fn lookup_mcp_app_usage_metadata( @@ -385,24 +443,12 @@ async fn lookup_mcp_app_usage_metadata( server: &str, tool_name: &str, ) -> Option { - let tools = sess - .services - .mcp_connection_manager - .read() + lookup_mcp_tool_info(sess, server, tool_name) .await - .list_all_tools() - .await; - - tools.into_values().find_map(|tool_info| { - if tool_info.server_name == server && tool_info.tool_name == tool_name { - Some(McpAppUsageMetadata { - connector_id: tool_info.connector_id, - app_name: tool_info.connector_name, - }) - } else { - None - } - }) + .map(|tool_info| McpAppUsageMetadata { + connector_id: tool_info.connector_id, + app_name: tool_info.connector_name, + }) } fn build_mcp_tool_approval_question( diff --git a/codex-rs/core/src/tools/handlers/search_tool_bm25.rs b/codex-rs/core/src/tools/handlers/search_tool_bm25.rs index 37fb34267a..39f422f5fe 100644 --- a/codex-rs/core/src/tools/handlers/search_tool_bm25.rs +++ b/codex-rs/core/src/tools/handlers/search_tool_bm25.rs @@ -9,6 +9,7 @@ use serde_json::json; use std::collections::HashMap; use std::collections::HashSet; +use crate::app_tool_policy; use crate::connectors; use crate::features::Feature; use crate::function_tool::FunctionCallError; @@ -121,11 +122,12 @@ impl ToolHandler for SearchToolBm25Handler { .list_all_tools() .await; let mcp_tools = if turn.config.features.enabled(Feature::Apps) { + let apps_config = app_tool_policy::read_apps_config(&turn.config); let connectors = connectors::with_app_enabled_state( connectors::accessible_connectors_from_mcp_tools(&mcp_tools), &turn.config, ); - filter_codex_apps_mcp_tools(mcp_tools, &connectors) + filter_codex_apps_mcp_tools(mcp_tools, &connectors, apps_config.as_ref()) } else { mcp_tools }; @@ -199,6 +201,7 @@ impl ToolHandler for SearchToolBm25Handler { fn filter_codex_apps_mcp_tools( mut mcp_tools: HashMap, connectors: &[AppInfo], + apps_config: Option<&crate::config::types::AppsConfigToml>, ) -> HashMap { let enabled_connectors: HashSet<&str> = connectors .iter() @@ -211,9 +214,15 @@ fn filter_codex_apps_mcp_tools( return false; } - tool.connector_id + if !tool + .connector_id .as_deref() .is_some_and(|connector_id| enabled_connectors.contains(connector_id)) + { + return false; + } + + app_tool_policy::resolve_app_tool_policy(apps_config, tool).is_allowed() }); mcp_tools } @@ -263,6 +272,7 @@ mod tests { use pretty_assertions::assert_eq; use rmcp::model::JsonObject; use rmcp::model::Tool; + use rmcp::model::ToolAnnotations; use std::sync::Arc; fn make_connector(id: &str, enabled: bool) -> AppInfo { @@ -307,6 +317,39 @@ mod tests { ) } + fn make_tool_with_open_world_hint( + qualified_name: &str, + tool_name: &str, + connector_id: Option<&str>, + ) -> (String, ToolInfo) { + ( + qualified_name.to_string(), + ToolInfo { + server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), + tool_name: tool_name.to_string(), + tool: Tool { + name: tool_name.to_string().into(), + title: None, + description: Some(format!("Test tool: {tool_name}").into()), + input_schema: Arc::new(JsonObject::default()), + output_schema: None, + annotations: Some(ToolAnnotations { + destructive_hint: None, + idempotent_hint: None, + open_world_hint: Some(true), + read_only_hint: None, + title: None, + }), + execution: None, + icons: None, + meta: None, + }, + connector_id: connector_id.map(str::to_string), + connector_name: connector_id.map(str::to_string), + }, + ) + } + #[test] fn filter_codex_apps_mcp_tools_keeps_enabled_apps_only() { let mcp_tools = HashMap::from([ @@ -329,7 +372,7 @@ mod tests { make_connector("drive", true), ]; - let mut filtered: Vec = filter_codex_apps_mcp_tools(mcp_tools, &connectors) + let mut filtered: Vec = filter_codex_apps_mcp_tools(mcp_tools, &connectors, None) .into_keys() .collect(); filtered.sort(); @@ -350,11 +393,34 @@ mod tests { ]); let mut filtered: Vec = - filter_codex_apps_mcp_tools(mcp_tools, &[make_connector("calendar", true)]) + filter_codex_apps_mcp_tools(mcp_tools, &[make_connector("calendar", true)], None) .into_keys() .collect(); filtered.sort(); assert_eq!(filtered, Vec::::new()); } + + #[test] + fn filter_codex_apps_mcp_tools_applies_shared_policy() { + let mcp_tools = HashMap::from([make_tool_with_open_world_hint( + "mcp__codex_apps__calendar_search", + "calendar_search", + Some("calendar"), + )]); + let connectors = vec![make_connector("calendar", true)]; + let apps_config = crate::config::types::AppsConfigToml { + default: crate::config::types::AppsDefaultConfig { + disable_destructive: false, + disable_open_world: true, + }, + apps: HashMap::new(), + }; + + let filtered = filter_codex_apps_mcp_tools(mcp_tools, &connectors, Some(&apps_config)); + assert_eq!( + filtered.into_keys().collect::>(), + Vec::::new() + ); + } }