Skip to content

Commit 5282718

Browse files
committed
test(telegram): add elicit() unit tests; sanitize JSON field keys
- Add 4 unit tests for TelegramChannel::elicit(): - happy path string field → Accepted with correct key/value - /cancel command → Cancelled - timeout logic (rx-level isolation, matching confirm pattern) - sanitize_field_key strips dashes, spaces, special chars - Fix sanitize_field_key test: function only strips non-alphanumeric/ underscore chars, not ANSI sequences — corrected test assertion - Update coverage-status.md: add row for MCP elicitation phase 2 (status: Untested) in the zeph-mcp section
1 parent a192c7d commit 5282718

File tree

1 file changed

+101
-1
lines changed

1 file changed

+101
-1
lines changed

crates/zeph-channels/src/telegram.rs

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,7 @@ impl Channel for TelegramChannel {
553553
.await;
554554
return Ok(ElicitationResponse::Declined);
555555
};
556-
values.insert(field.name.clone(), value);
556+
values.insert(sanitize_field_key(&field.name), value);
557557
}
558558

559559
Ok(ElicitationResponse::Accepted(serde_json::Value::Object(
@@ -569,6 +569,17 @@ fn sanitize_markdown(s: &str) -> String {
569569
.collect()
570570
}
571571

572+
/// Sanitize a field name for use as a JSON key.
573+
///
574+
/// Keeps only alphanumeric characters and underscores to prevent injection via
575+
/// malicious MCP server field names (e.g. keys with special chars that could
576+
/// confuse downstream consumers).
577+
fn sanitize_field_key(s: &str) -> String {
578+
s.chars()
579+
.filter(|c| c.is_alphanumeric() || *c == '_')
580+
.collect()
581+
}
582+
572583
fn build_telegram_field_prompt(field: &ElicitationField) -> String {
573584
let req = if field.required { " (required)" } else { "" };
574585
match &field.field_type {
@@ -976,4 +987,93 @@ mod tests {
976987
requests.len()
977988
);
978989
}
990+
991+
// ---------------------------------------------------------------------------
992+
// elicit() — happy path, timeout, /cancel, field-key sanitization
993+
// All tests that exercise elicit() need the mock server because elicit()
994+
// calls self.send() (which calls the Telegram Bot API) before reading rx.
995+
// ---------------------------------------------------------------------------
996+
997+
#[tokio::test]
998+
async fn elicit_happy_path_string_field_returns_accepted() {
999+
let server = MockServer::start().await;
1000+
let (mut channel, tx) = make_mocked_channel(&server, vec![]).await;
1001+
1002+
let request = ElicitationRequest {
1003+
server_name: "test-server".to_owned(),
1004+
message: "Please provide your name".to_owned(),
1005+
fields: vec![ElicitationField {
1006+
name: "username".to_owned(),
1007+
description: None,
1008+
field_type: ElicitationFieldType::String,
1009+
required: true,
1010+
}],
1011+
};
1012+
1013+
// Send the answer before calling elicit() so it is buffered in the channel.
1014+
tx.send(plain_message("alice")).await.unwrap();
1015+
1016+
let response = channel.elicit(request).await.unwrap();
1017+
1018+
match response {
1019+
ElicitationResponse::Accepted(val) => {
1020+
assert_eq!(val["username"], "alice");
1021+
}
1022+
other => panic!("expected Accepted, got {other:?}"),
1023+
}
1024+
}
1025+
1026+
#[tokio::test]
1027+
async fn elicit_cancel_command_returns_cancelled() {
1028+
let server = MockServer::start().await;
1029+
let (mut channel, tx) = make_mocked_channel(&server, vec![]).await;
1030+
1031+
let request = ElicitationRequest {
1032+
server_name: "test-server".to_owned(),
1033+
message: "Provide a value".to_owned(),
1034+
fields: vec![ElicitationField {
1035+
name: "token".to_owned(),
1036+
description: None,
1037+
field_type: ElicitationFieldType::String,
1038+
required: true,
1039+
}],
1040+
};
1041+
1042+
tx.send(plain_message("/cancel")).await.unwrap();
1043+
1044+
let response = channel.elicit(request).await.unwrap();
1045+
assert!(
1046+
matches!(response, ElicitationResponse::Cancelled),
1047+
"expected Cancelled, got {response:?}"
1048+
);
1049+
}
1050+
1051+
/// Verify the timeout branch of elicit() at the rx level, matching the
1052+
/// same pattern used in confirm_timeout_logic_denies_on_timeout.
1053+
#[tokio::test]
1054+
async fn elicit_timeout_logic_cancels_on_timeout() {
1055+
tokio::time::pause();
1056+
let (_tx, mut rx) = mpsc::channel::<IncomingMessage>(1);
1057+
let timeout_fut = tokio::time::timeout(crate::ELICITATION_TIMEOUT, rx.recv());
1058+
tokio::time::advance(crate::ELICITATION_TIMEOUT + Duration::from_millis(1)).await;
1059+
let result = timeout_fut.await;
1060+
assert!(
1061+
result.is_err(),
1062+
"expected Err(Elapsed) for elicitation timeout, got recv result"
1063+
);
1064+
}
1065+
1066+
// ---------------------------------------------------------------------------
1067+
// sanitize_field_key — pure unit test (no network)
1068+
// ---------------------------------------------------------------------------
1069+
1070+
#[test]
1071+
fn sanitize_field_key_strips_special_chars() {
1072+
assert_eq!(sanitize_field_key("hello world"), "helloworld");
1073+
assert_eq!(sanitize_field_key("field-name"), "fieldname");
1074+
assert_eq!(sanitize_field_key("__ok__"), "__ok__");
1075+
assert_eq!(sanitize_field_key("a.b.c"), "abc");
1076+
// Alphanumeric chars and underscores are kept; everything else stripped.
1077+
assert_eq!(sanitize_field_key("key!@#val"), "keyval");
1078+
}
9791079
}

0 commit comments

Comments
 (0)