From 879e827a7ef5a2e0bf962ea7c052fc80226b329a Mon Sep 17 00:00:00 2001 From: Ray Zhu Date: Wed, 29 Apr 2026 16:38:19 +1000 Subject: [PATCH] fix: reject malformed jsonrpc envelopes --- .../schema/json/JSONRPCError.json | 4 +- .../schema/json/JSONRPCErrorError.json | 3 +- .../schema/json/JSONRPCMessage.json | 7 +- .../schema/json/JSONRPCNotification.json | 3 +- .../schema/json/JSONRPCRequest.json | 3 +- .../schema/json/JSONRPCResponse.json | 3 +- .../codex_app_server_protocol.schemas.json | 7 +- .../app-server-protocol/src/jsonrpc_lite.rs | 164 ++++++++++++++++++ 8 files changed, 187 insertions(+), 7 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/JSONRPCError.json b/codex-rs/app-server-protocol/schema/json/JSONRPCError.json index 6db5d1a7fa55..b65376e0100c 100644 --- a/codex-rs/app-server-protocol/schema/json/JSONRPCError.json +++ b/codex-rs/app-server-protocol/schema/json/JSONRPCError.json @@ -1,7 +1,9 @@ { "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, "definitions": { "JSONRPCErrorError": { + "additionalProperties": false, "properties": { "code": { "format": "int64", @@ -45,4 +47,4 @@ ], "title": "JSONRPCError", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/JSONRPCErrorError.json b/codex-rs/app-server-protocol/schema/json/JSONRPCErrorError.json index 932ef33c9a7b..01c26a90859f 100644 --- a/codex-rs/app-server-protocol/schema/json/JSONRPCErrorError.json +++ b/codex-rs/app-server-protocol/schema/json/JSONRPCErrorError.json @@ -1,5 +1,6 @@ { "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, "properties": { "code": { "format": "int64", @@ -16,4 +17,4 @@ ], "title": "JSONRPCErrorError", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/JSONRPCMessage.json b/codex-rs/app-server-protocol/schema/json/JSONRPCMessage.json index 27b78b90c929..e4c250c9c90f 100644 --- a/codex-rs/app-server-protocol/schema/json/JSONRPCMessage.json +++ b/codex-rs/app-server-protocol/schema/json/JSONRPCMessage.json @@ -16,6 +16,7 @@ ], "definitions": { "JSONRPCError": { + "additionalProperties": false, "description": "A response to a request that indicates an error occurred.", "properties": { "error": { @@ -32,6 +33,7 @@ "type": "object" }, "JSONRPCErrorError": { + "additionalProperties": false, "properties": { "code": { "format": "int64", @@ -49,6 +51,7 @@ "type": "object" }, "JSONRPCNotification": { + "additionalProperties": false, "description": "A notification which does not expect a response.", "properties": { "method": { @@ -62,6 +65,7 @@ "type": "object" }, "JSONRPCRequest": { + "additionalProperties": false, "description": "A request that expects a response.", "properties": { "id": { @@ -90,6 +94,7 @@ "type": "object" }, "JSONRPCResponse": { + "additionalProperties": false, "description": "A successful (non-error) response to a request.", "properties": { "id": { @@ -134,4 +139,4 @@ }, "description": "Refers to any valid JSON-RPC object that can be decoded off the wire, or encoded to be sent.", "title": "JSONRPCMessage" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/JSONRPCNotification.json b/codex-rs/app-server-protocol/schema/json/JSONRPCNotification.json index 2ddd61a8ca7b..8a8052e69cbb 100644 --- a/codex-rs/app-server-protocol/schema/json/JSONRPCNotification.json +++ b/codex-rs/app-server-protocol/schema/json/JSONRPCNotification.json @@ -1,5 +1,6 @@ { "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, "description": "A notification which does not expect a response.", "properties": { "method": { @@ -12,4 +13,4 @@ ], "title": "JSONRPCNotification", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/JSONRPCRequest.json b/codex-rs/app-server-protocol/schema/json/JSONRPCRequest.json index e4ea7c206b07..3c764f347eae 100644 --- a/codex-rs/app-server-protocol/schema/json/JSONRPCRequest.json +++ b/codex-rs/app-server-protocol/schema/json/JSONRPCRequest.json @@ -1,5 +1,6 @@ { "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, "definitions": { "RequestId": { "anyOf": [ @@ -57,4 +58,4 @@ ], "title": "JSONRPCRequest", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/JSONRPCResponse.json b/codex-rs/app-server-protocol/schema/json/JSONRPCResponse.json index 9f1ec2954852..3b0a954436a7 100644 --- a/codex-rs/app-server-protocol/schema/json/JSONRPCResponse.json +++ b/codex-rs/app-server-protocol/schema/json/JSONRPCResponse.json @@ -1,5 +1,6 @@ { "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, "definitions": { "RequestId": { "anyOf": [ @@ -26,4 +27,4 @@ ], "title": "JSONRPCResponse", "type": "object" -} \ No newline at end of file +} 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 baf4f786d997..4b6ac1da4317 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 @@ -2594,6 +2594,7 @@ }, "JSONRPCError": { "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, "description": "A response to a request that indicates an error occurred.", "properties": { "error": { @@ -2612,6 +2613,7 @@ }, "JSONRPCErrorError": { "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, "properties": { "code": { "format": "int64", @@ -2650,6 +2652,7 @@ }, "JSONRPCNotification": { "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, "description": "A notification which does not expect a response.", "properties": { "method": { @@ -2665,6 +2668,7 @@ }, "JSONRPCRequest": { "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, "description": "A request that expects a response.", "properties": { "id": { @@ -2695,6 +2699,7 @@ }, "JSONRPCResponse": { "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, "description": "A successful (non-error) response to a request.", "properties": { "id": { @@ -17831,4 +17836,4 @@ }, "title": "CodexAppServerProtocol", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/src/jsonrpc_lite.rs b/codex-rs/app-server-protocol/src/jsonrpc_lite.rs index 4e8858ce00a0..c769f719a0fe 100644 --- a/codex-rs/app-server-protocol/src/jsonrpc_lite.rs +++ b/codex-rs/app-server-protocol/src/jsonrpc_lite.rs @@ -43,6 +43,7 @@ pub enum JSONRPCMessage { /// A request that expects a response. #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)] +#[serde(deny_unknown_fields)] pub struct JSONRPCRequest { pub id: RequestId, pub method: String, @@ -57,6 +58,7 @@ pub struct JSONRPCRequest { /// A notification which does not expect a response. #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)] +#[serde(deny_unknown_fields)] pub struct JSONRPCNotification { pub method: String, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -66,6 +68,7 @@ pub struct JSONRPCNotification { /// A successful (non-error) response to a request. #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)] +#[serde(deny_unknown_fields)] pub struct JSONRPCResponse { pub id: RequestId, pub result: Result, @@ -73,12 +76,14 @@ pub struct JSONRPCResponse { /// A response to a request that indicates an error occurred. #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)] +#[serde(deny_unknown_fields)] pub struct JSONRPCError { pub error: JSONRPCErrorError, pub id: RequestId, } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)] +#[serde(deny_unknown_fields)] pub struct JSONRPCErrorError { pub code: i64, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -86,3 +91,162 @@ pub struct JSONRPCErrorError { pub data: Option, pub message: String, } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use serde_json::json; + + fn trace_context() -> W3cTraceContext { + W3cTraceContext { + traceparent: Some( + "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01".to_string(), + ), + tracestate: Some("vendor=value".to_string()), + } + } + + #[test] + fn request_round_trips_with_integer_id_params_and_trace() { + let request = JSONRPCRequest { + id: RequestId::Integer(7), + method: "thread/read".to_string(), + params: Some(json!({ "threadId": "thread-1" })), + trace: Some(trace_context()), + }; + + let value = serde_json::to_value(&request).expect("serialize request"); + assert_eq!( + value, + json!({ + "id": 7, + "method": "thread/read", + "params": { "threadId": "thread-1" }, + "trace": { + "traceparent": "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + "tracestate": "vendor=value", + }, + }) + ); + + let message: JSONRPCMessage = serde_json::from_value(value).expect("deserialize message"); + assert_eq!(message, JSONRPCMessage::Request(request)); + } + + #[test] + fn request_omits_absent_optional_fields() { + let request = JSONRPCRequest { + id: RequestId::String("req-1".to_string()), + method: "thread/list".to_string(), + params: None, + trace: None, + }; + + let value = serde_json::to_value(&request).expect("serialize request"); + assert_eq!( + value, + json!({ + "id": "req-1", + "method": "thread/list", + }) + ); + + let decoded: JSONRPCRequest = serde_json::from_value(value).expect("deserialize request"); + assert_eq!(decoded, request); + } + + #[test] + fn message_deserializes_notification_without_id() { + let value = json!({ + "method": "thread/subscribe", + "params": { "threadId": "thread-1" }, + }); + + let message: JSONRPCMessage = serde_json::from_value(value).expect("deserialize message"); + assert_eq!( + message, + JSONRPCMessage::Notification(JSONRPCNotification { + method: "thread/subscribe".to_string(), + params: Some(json!({ "threadId": "thread-1" })), + }) + ); + } + + #[test] + fn message_deserializes_success_response() { + let value = json!({ + "id": "req-1", + "result": { "ok": true }, + }); + + let message: JSONRPCMessage = serde_json::from_value(value).expect("deserialize message"); + assert_eq!( + message, + JSONRPCMessage::Response(JSONRPCResponse { + id: RequestId::String("req-1".to_string()), + result: json!({ "ok": true }), + }) + ); + } + + #[test] + fn message_deserializes_error_response_with_data() { + let value = json!({ + "id": 9, + "error": { + "code": -32602, + "message": "invalid params", + "data": { "field": "threadId" }, + }, + }); + + let message: JSONRPCMessage = serde_json::from_value(value).expect("deserialize message"); + assert_eq!( + message, + JSONRPCMessage::Error(JSONRPCError { + id: RequestId::Integer(9), + error: JSONRPCErrorError { + code: -32602, + message: "invalid params".to_string(), + data: Some(json!({ "field": "threadId" })), + }, + }) + ); + } + + #[test] + fn request_id_display_matches_wire_value() { + assert_eq!(RequestId::String("req-1".to_string()).to_string(), "req-1"); + assert_eq!(RequestId::Integer(42).to_string(), "42"); + } + + #[test] + fn message_rejects_request_with_response_fields() { + let value = json!({ + "id": 7, + "method": "thread/read", + "params": { "threadId": "thread-1" }, + "result": { "ok": true }, + }); + + serde_json::from_value::(value) + .expect_err("request with response result should be invalid"); + } + + #[test] + fn message_rejects_request_with_error_fields() { + let value = json!({ + "id": 7, + "method": "thread/read", + "params": { "threadId": "thread-1" }, + "error": { + "code": -32603, + "message": "internal error", + }, + }); + + serde_json::from_value::(value) + .expect_err("request with response error should be invalid"); + } +}