diff --git a/codex-rs/code-mode-runtime/src/runtime/value.rs b/codex-rs/code-mode-runtime/src/runtime/value.rs index 2e43d883f8..e5305a6979 100644 --- a/codex-rs/code-mode-runtime/src/runtime/value.rs +++ b/codex-rs/code-mode-runtime/src/runtime/value.rs @@ -313,7 +313,26 @@ pub(super) fn v8_value_to_json( } return Ok(None); }; - serde_json::from_str(&stringified.to_rust_string_lossy(&tc)) + let json = stringified.to_rust_string_lossy(&tc); + // V8 emits compact JSON with these ASCII key names unescaped. The object/comma + // prefix and colon distinguish keys from string contents. Reject these before + // Serde can reinterpret them as values and bypass its normal recursion limit. + // TODO(cconger): Remove this guard once our resolved serde_json dependency + // includes the upstream private-key provenance fix. Retain regression coverage + // for literal keys, depth limits, and session recovery. + // https://github.com/cconger/json/commit/63f6f14053ef3a173013d7e5ee5ac74140736929 + if [ + r#"{"$serde_json::private::RawValue":"#, + r#","$serde_json::private::RawValue":"#, + r#"{"$serde_json::private::Number":"#, + r#","$serde_json::private::Number":"#, + ] + .into_iter() + .any(|key| json.contains(key)) + { + return Err("failed to serialize JavaScript value: reserved JSON object key".to_string()); + } + serde_json::from_str(&json) .map(Some) .map_err(|err| format!("failed to serialize JavaScript value: {err}")) } diff --git a/codex-rs/code-mode-runtime/src/service_tests.rs b/codex-rs/code-mode-runtime/src/service_tests.rs index 62d15d6648..2f675249f4 100644 --- a/codex-rs/code-mode-runtime/src/service_tests.rs +++ b/codex-rs/code-mode-runtime/src/service_tests.rs @@ -686,6 +686,105 @@ text(load("key")); ); } +#[tokio::test] +async fn json_conversion_rejects_serde_markers_without_poisoning_sessions() { + let first = InProcessCodeModeSession::new(); + let second = InProcessCodeModeSession::new(); + let source = r#" +store("healthy", 42); +function rejects(value, message) { + let error; + try { store("healthy", value); } catch (e) { error = String(e); } + if (!message.test(error ?? "")) throw new Error(`expected ${message}; got ${error ?? "no error"}`); +} +const keys = ["$serde_json::private::RawValue", "$serde_json::private::Number"]; +for (const key of keys) { + for (const value of [ + {[key]: "123"}, + {first: 0, [key]: "null"}, + [{[key]: "123"}], + JSON.parse('{"\\u0024' + key.slice(1) + '":"123"}'), + ]) rejects(value, /reserved JSON object key/); + + // Marker text in strings and longer keys must remain valid. + const literal = ['{"' + key + '":"123"}', {['prefix"' + key]: key}]; + store("literal", literal); + if (JSON.stringify(load("literal")) !== JSON.stringify(literal)) throw "changed literal"; +} +for (const segments of [2, 5]) { + let encoded = "0"; + for (let i = 0; i < segments; i++) { + encoded = "[".repeat(80) + encoded + "]".repeat(80); + if (i < segments - 1) encoded = JSON.stringify({[keys[0]]: encoded}); + } + // Exercise Serde's depth reset without first requiring a deep V8 object. + rejects({[keys[0]]: encoded}, /reserved JSON object key/); +} +let value = 0; +for (let depth = 1; depth <= 160; depth++) { + value = [value]; + if (depth === 127) { + try { + store("boundary", value); + if (JSON.stringify(load("boundary")) !== JSON.stringify(value)) throw "changed boundary"; + } catch (e) { + // V8 can reach its stack limit before Serde's depth limit on Windows. + if (!String(e).includes("Maximum call stack size exceeded")) throw e; + } + } + if (depth === 128 || depth === 160) rejects(value, /recursion limit exceeded|Maximum call stack size exceeded/); +} +const normal = {toJSON() { return [undefined, NaN, Infinity, 1.25, 1e100]; }}; +store("normal", normal); +if (JSON.stringify(load("normal")) !== JSON.stringify(normal)) throw "changed normal JSON"; +text("ok"); +store("healthy", {[keys[0]]: "null"}); +"#; + + for (session, source, expected_text, expected_error) in [ + ( + &second, + r#"store("healthy", 42); text(load("healthy"));"#, + "42", + None, + ), + ( + &first, + source, + "ok", + Some("failed to serialize JavaScript value: reserved JSON object key"), + ), + (&first, r#"text(load("healthy"));"#, "42", None), + (&second, r#"text(load("healthy"));"#, "42", None), + ] { + let response = execute( + session, + ExecuteRequest { + yield_time_ms: None, + ..execute_request(source) + }, + ) + .await; + let RuntimeResponse::Result { + content_items, + error_text, + .. + } = response + else { + panic!("unexpected response: {response:?}"); + }; + assert_eq!( + (content_items, error_text), + ( + vec![FunctionCallOutputContentItem::InputText { + text: expected_text.to_string() + }], + expected_error.map(str::to_string) + ) + ); + } +} + #[tokio::test] async fn shutdown_interrupts_cpu_bound_cells() { let service = InProcessCodeModeSession::new(); diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index b2371a0c9e..f27581cd41 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -9075,14 +9075,18 @@ structuredContent=null" } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn code_mode_can_store_and_load_values_across_turns() -> Result<()> { +async fn code_mode_can_store_and_load_values_across_turns_after_rejection() -> Result<()> { skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; - let mut builder = test_codex().with_config(move |config| { - let _ = config.features.enable(Feature::CodeMode); - }); - let test = builder.build(&server).await?; + let mut builder = test_codex() + .with_code_mode_host_program(codex_utils_cargo_bin::cargo_bin("codex-code-mode-host")?) + .with_config(|config| { + let _ = config.features.enable(Feature::CodeMode); + let _ = config.features.enable(Feature::CodeModeHost); + config.code_mode.disable_in_process_fallback = true; + }); + let test = builder.build_with_auto_env(&server).await?; responses::mount_sse_once( &server, @@ -9121,6 +9125,43 @@ text("stored"); ); assert_eq!(first_output, "stored"); + let rejection = responses::mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-rejected"), + ev_custom_tool_call( + "call-rejected", + "exec", + r#" +// Each RawValue resets the parser's depth counter; the decoded arrays reach depth 400. +let encoded = "0"; +for (let i = 0; i < 5; i++) { + encoded = JSON.stringify({ + "$serde_json::private::RawValue": "[".repeat(80) + encoded + "]".repeat(80), + }); +} +store("nb", JSON.parse(encoded)); +"#, + ), + ev_completed("resp-rejected"), + ]), + sse(vec![ + ev_assistant_message("msg-rejected", "rejected"), + ev_completed("resp-rejected-complete"), + ]), + ], + ) + .await; + test.submit_turn("attempt to overwrite the stored value with a private Serde marker") + .await?; + let (rejected_output, _) = + custom_tool_output_body_and_success(&rejection.requests()[1], "call-rejected"); + assert_eq!( + rejected_output, + "Script error:\nfailed to serialize JavaScript value: reserved JSON object key", + ); + responses::mount_sse_once( &server, sse(vec![