Skip to content

Commit ad713bc

Browse files
committed
fix(mcp): guarantee valid UTF-8 in tree-format output
Dry run 30206811293: macos-15-intel release smoke failed B3 with a visibly correct 'semantic: 50' table — BSD grep returns no-match for the ENTIRE output when any line carries a NUL, control byte, or invalid UTF-8 (verified: printf poisoned input -> grep -qE fails), so one bad byte in one row silently unmatches every anchored check. Close the class at the emission choke point: control bytes and invalid UTF-8 force the quoted form; quoting escapes controls as \u00XX and replaces invalid sequences with U+FFFD (RFC 3629 validation, NUL-safe). Tool output is valid UTF-8 by construction. Unit test proven RED-on-revert (raw bytes emitted) and green with the fix. The smoke's B3 failure path now also dumps od -c bytes so a future red names the exact byte. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 040c7f4 commit ad713bc

3 files changed

Lines changed: 102 additions & 5 deletions

File tree

scripts/smoke-test.sh

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,11 @@ fi
568568
if echo "$IM_ARR" | grep -qE '^semantic: [0-9]+'; then
569569
echo "OK B3: ARRAY flag (repeated --semantic-query) → semantic TOON table"
570570
else
571-
echo "FAIL B3: repeated --semantic-query did not produce a semantic table"; echo "$IM_ARR" | head -c 300; exit 1
571+
echo "FAIL B3: repeated --semantic-query did not produce a semantic table"; echo "$IM_ARR" | head -c 300
572+
# Byte-exact post-mortem: an invisible control/invalid-UTF8 byte anywhere in
573+
# the output makes BSD grep treat ALL of it as unmatchable binary, and the
574+
# rendered log cannot show which byte — od can.
575+
echo; echo "-- B3 first bytes (od) --"; echo "$IM_ARR" | od -c | head -6; exit 1
572576
fi
573577

574578
# B4: STDIN — piped JSON resolves; this path must NOT emit a deprecation warning.

src/mcp/compact_out.c

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,16 +97,61 @@ static bool looks_numeric(const char *s) {
9797
return digit;
9898
}
9999

100+
/* UTF-8 sequence length starting at p, validating continuation bytes and the
101+
* lead-byte ranges (RFC 3629: no overlongs, no surrogates, max U+10FFFF);
102+
* 0 = not a valid sequence start. Reads stop at the first invalid byte, so a
103+
* terminating NUL is never overrun. */
104+
static size_t utf8_sequence_length(const unsigned char *p) {
105+
unsigned char c = p[0];
106+
if (c < 0x80) {
107+
return 1;
108+
}
109+
if (c < 0xC2) {
110+
return 0; /* bare continuation byte or overlong lead */
111+
}
112+
if (c < 0xE0) {
113+
return (p[1] & 0xC0) == 0x80 ? 2 : 0;
114+
}
115+
if (c < 0xF0) {
116+
unsigned char lo = c == 0xE0 ? 0xA0 : 0x80;
117+
unsigned char hi = c == 0xED ? 0x9F : 0xBF;
118+
if (p[1] < lo || p[1] > hi || (p[2] & 0xC0) != 0x80) {
119+
return 0;
120+
}
121+
return 3;
122+
}
123+
if (c < 0xF5) {
124+
unsigned char lo = c == 0xF0 ? 0x90 : 0x80;
125+
unsigned char hi = c == 0xF4 ? 0x8F : 0xBF;
126+
if (p[1] < lo || p[1] > hi || (p[2] & 0xC0) != 0x80 || (p[3] & 0xC0) != 0x80) {
127+
return 0;
128+
}
129+
return 4;
130+
}
131+
return 0;
132+
}
133+
100134
static bool needs_quotes(const char *s) {
101135
if (!s || !*s) {
102136
return false; /* empty cells emit as the "-" placeholder, not quotes */
103137
}
104138
for (const char *p = s; *p; p++) {
139+
unsigned char c = (unsigned char)*p;
105140
/* Space-delimited rows: any internal whitespace or quote forces
106-
* quoting so column positions stay parseable. */
107-
if (isspace((unsigned char)*p) || *p == '"' || *p == '\r') {
141+
* quoting so column positions stay parseable. Control bytes and
142+
* invalid UTF-8 force the quoted path too, which sanitizes them —
143+
* one raw byte otherwise makes line-oriented consumers (BSD grep)
144+
* treat the ENTIRE tool output as unmatchable binary. */
145+
if (isspace(c) || *p == '"' || *p == '\r' || c < 0x20 || c == 0x7f) {
108146
return true;
109147
}
148+
if (c >= 0x80) {
149+
size_t len = utf8_sequence_length((const unsigned char *)p);
150+
if (!len) {
151+
return true;
152+
}
153+
p += len - 1;
154+
}
110155
}
111156
if (strcmp(s, "true") == 0 || strcmp(s, "false") == 0 || strcmp(s, "null") == 0 ||
112157
strcmp(s, "-") == 0) {
@@ -131,10 +176,29 @@ static void append_quoted(cbm_sb_t *sb, const char *s) {
131176
case '\r':
132177
cbm_sb_append_n(sb, "\\r", 2);
133178
break;
134-
default:
135-
cbm_sb_append_n(sb, p, 1);
179+
default: {
180+
unsigned char c = (unsigned char)*p;
181+
if (c >= 0x20 && c != 0x7f && c < 0x80) {
182+
cbm_sb_append_n(sb, p, 1);
183+
} else if (c < 0x20 || c == 0x7f) {
184+
/* JSON-style escape: the value stays one printable line. */
185+
char esc[8];
186+
snprintf(esc, sizeof(esc), "\\u%04x", (unsigned)c);
187+
cbm_sb_append(sb, esc);
188+
} else {
189+
size_t len = utf8_sequence_length((const unsigned char *)p);
190+
if (len) {
191+
cbm_sb_append_n(sb, p, len);
192+
p += len - 1;
193+
} else {
194+
/* Invalid byte → U+FFFD: output is valid UTF-8 by
195+
* construction, and the corruption stays visible. */
196+
cbm_sb_append_n(sb, "\xEF\xBF\xBD", 3);
197+
}
198+
}
136199
break;
137200
}
201+
}
138202
}
139203
cbm_sb_append_n(sb, "\"", 1);
140204
}

tests/test_mcp.c

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "../src/foundation/log.h"
1111
#include "../src/foundation/platform.h" /* cbm_file_size */
1212
#include "../src/foundation/subprocess.h"
13+
#include "../src/mcp/compact_out.h"
1314
#include "test_framework.h"
1415
#include "test_helpers.h"
1516
#include <cli/cli.h>
@@ -550,6 +551,33 @@ static void mcp_replacing_mutation_guard_end(void *context, const char *project)
550551
}
551552
}
552553

554+
TEST(tree_cell_sanitizes_control_and_invalid_utf8) {
555+
/* One raw control or invalid-UTF8 byte in a cell poisons LINE-ORIENTED
556+
* consumers of the ENTIRE output (BSD grep treats all of it as
557+
* unmatchable binary — the macos-15-intel release-smoke B3 class), so
558+
* cell emission guarantees valid UTF-8: control bytes escape as \u00XX,
559+
* invalid sequences become U+FFFD, and both force the quoted form. */
560+
cbm_sb_t sb;
561+
cbm_sb_init(&sb);
562+
cbm_tree_cell_str(&sb,
563+
"evil\x01name\xff"
564+
"end",
565+
true);
566+
char *out = cbm_sb_finish(&sb);
567+
ASSERT_NOT_NULL(out);
568+
ASSERT_STR_EQ(out, "\"evil\\u0001name\xEF\xBF\xBD"
569+
"end\"");
570+
free(out);
571+
572+
cbm_sb_init(&sb);
573+
cbm_tree_cell_str(&sb, "b\xC3\xA4r_ok", true);
574+
out = cbm_sb_finish(&sb);
575+
ASSERT_NOT_NULL(out);
576+
ASSERT_STR_EQ(out, "b\xC3\xA4r_ok"); /* valid UTF-8 stays raw + unquoted */
577+
free(out);
578+
PASS();
579+
}
580+
553581
/* ══════════════════════════════════════════════════════════════════
554582
* JSON-RPC PARSING
555583
* ══════════════════════════════════════════════════════════════════ */
@@ -9468,6 +9496,7 @@ SUITE(mcp) {
94689496
RUN_TEST(jsonrpc_parse_request);
94699497
RUN_TEST(jsonrpc_parse_notification);
94709498
RUN_TEST(jsonrpc_parse_invalid);
9499+
RUN_TEST(tree_cell_sanitizes_control_and_invalid_utf8);
94719500
RUN_TEST(jsonrpc_parse_tools_call);
94729501
RUN_TEST(jsonrpc_parse_string_id_issue253);
94739502
RUN_TEST(jsonrpc_format_response_string_id_issue253);

0 commit comments

Comments
 (0)