mirror of
https://github.com/openai/codex.git
synced 2026-10-21 09:54:56 +00:00
Validate tool suggestion install URLs before showing the app link (#48015)
## Why Tool suggestions could pass unchecked `install_url` values to the app link view for opening in a browser. ## What changed Reuse the external URL validator to require HTTPS URLs with a host and no embedded credentials. Decline suggestions with invalid URLs on the requesting thread without showing a popup. ## Testing Add a regression test covering malformed URLs, unsupported schemes, embedded credentials, and a valid HTTPS URL. Verify that rejected suggestions leave the popup unchanged and resolve on the requesting thread, while the valid URL opens on confirmation. GitOrigin-RevId: e47bf09868ec81aeaf1ffed8330039b69c60e093
This commit is contained in:
@@ -203,7 +203,7 @@ impl AppLinkViewParams {
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_external_url(url: &str, require_chatgpt_host: bool) -> Option<Url> {
|
||||
pub(super) fn validate_external_url(url: &str, require_chatgpt_host: bool) -> Option<Url> {
|
||||
let parsed = Url::parse(url).ok()?;
|
||||
if parsed.scheme() != "https" || parsed.host_str().is_none() {
|
||||
return None;
|
||||
|
||||
@@ -1901,6 +1901,20 @@ impl BottomPane {
|
||||
if let Some(tool_suggestion) = request.tool_suggestion()
|
||||
&& let Some(install_url) = tool_suggestion.install_url.clone()
|
||||
{
|
||||
let Some(install_url) = app_link_view::validate_external_url(
|
||||
&install_url,
|
||||
/*require_chatgpt_host*/ false,
|
||||
) else {
|
||||
self.app_event_tx.resolve_elicitation(
|
||||
request.thread_id(),
|
||||
request.server_name().to_string(),
|
||||
request.request_id().clone(),
|
||||
codex_app_server_protocol::McpServerElicitationAction::Decline,
|
||||
/*content*/ None,
|
||||
/*meta*/ None,
|
||||
);
|
||||
return;
|
||||
};
|
||||
let suggestion_type = match tool_suggestion.suggest_type {
|
||||
mcp_server_elicitation::ToolSuggestionType::Install => {
|
||||
AppLinkSuggestionType::Install
|
||||
@@ -1930,7 +1944,7 @@ impl BottomPane {
|
||||
"external actions use URL mode elicitation, not tool suggestion forms"
|
||||
),
|
||||
},
|
||||
url: install_url,
|
||||
url: install_url.into(),
|
||||
is_installed,
|
||||
is_enabled: false,
|
||||
suggest_reason: Some(tool_suggestion.suggest_reason.clone()),
|
||||
|
||||
+8
@@ -0,0 +1,8 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/app_server.rs
|
||||
assertion_line: 572
|
||||
expression: popup
|
||||
---
|
||||
› Ask Codex to do anything
|
||||
|
||||
GPT-5.6-Sol default · /tmp/project
|
||||
@@ -506,6 +506,78 @@ async fn safety_buffering_ignores_hidden_stale_and_historical_updates() {
|
||||
assert!(!render_bottom_popup(&chat, /*width*/ 80).contains("Stop this attempt and retry?"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn tool_suggestion_install_url_is_validated_before_opening() {
|
||||
for install_url in [
|
||||
"file:///tmp/connector",
|
||||
"http://example.test/install",
|
||||
"custom://example.test/install",
|
||||
"not a URL",
|
||||
"https://user:password@example.test/install",
|
||||
"https://example.test/install",
|
||||
] {
|
||||
let (mut chat, _app_event_tx, mut rx, _op_rx) = make_chatwidget_manual_with_sender().await;
|
||||
let request_thread_id = ThreadId::new();
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
let initial_popup = render_bottom_popup(&chat, /*width*/ 80);
|
||||
|
||||
chat.handle_elicitation_request_now(
|
||||
codex_app_server_protocol::RequestId::Integer(9),
|
||||
codex_app_server_protocol::McpServerElicitationRequestParams {
|
||||
thread_id: request_thread_id.to_string(),
|
||||
turn_id: Some("turn-install".to_string()),
|
||||
server_name: "connector-server".to_string(),
|
||||
request: codex_app_server_protocol::McpServerElicitationRequest::Form {
|
||||
meta: Some(serde_json::json!({
|
||||
"codex_approval_kind": "tool_suggestion",
|
||||
"tool_type": "connector",
|
||||
"suggest_type": "install",
|
||||
"suggest_reason": "Install the connector to continue",
|
||||
"tool_id": "connector_test",
|
||||
"tool_name": "Test Connector",
|
||||
"install_url": install_url,
|
||||
})),
|
||||
message: "Install Test Connector".to_string(),
|
||||
requested_schema: serde_json::from_value(serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {},
|
||||
}))
|
||||
.expect("valid schema"),
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
if install_url == "https://example.test/install" {
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
assert_matches!(
|
||||
rx.try_recv(),
|
||||
Ok(AppEvent::OpenUrlInBrowser { url }) if url == install_url
|
||||
);
|
||||
} else {
|
||||
assert_matches!(
|
||||
rx.try_recv(),
|
||||
Ok(AppEvent::SubmitThreadOp {
|
||||
thread_id,
|
||||
op: Op::ResolveElicitation {
|
||||
server_name,
|
||||
request_id: codex_app_server_protocol::RequestId::Integer(9),
|
||||
decision: codex_app_server_protocol::McpServerElicitationAction::Decline,
|
||||
content: None,
|
||||
meta: None,
|
||||
},
|
||||
}) if thread_id == request_thread_id && server_name == "connector-server"
|
||||
);
|
||||
let popup = render_bottom_popup(&chat, /*width*/ 80);
|
||||
assert_eq!(popup, initial_popup);
|
||||
assert_chatwidget_snapshot!(
|
||||
"declined_tool_suggestion",
|
||||
normalize_snapshot_paths(popup)
|
||||
);
|
||||
}
|
||||
assert!(rx.try_recv().is_err());
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn invalid_url_elicitation_is_declined() {
|
||||
let (mut chat, _app_event_tx, mut rx, _op_rx) = make_chatwidget_manual_with_sender().await;
|
||||
|
||||
Reference in New Issue
Block a user