Skip to content

Commit a498fc5

Browse files
hugenshenNIO
hugenshen
and
NIO
authored
fix(file-transfer): keep bounded stderr tail UTF-16 safe end-to-end (#104151)
Co-authored-by: NIO <[email protected]>
1 parent ab9a1c9 commit a498fc5

6 files changed

Lines changed: 126 additions & 15 deletions

File tree

‎extensions/file-transfer/src/shared/append-bounded-text-tail.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// File Transfer tests cover bounded stderr tail UTF-16 safety.
2+
import { describe, expect, it } from "vitest";
3+
import { appendBoundedTextTail, projectBoundedTextTail } from "./append-bounded-text-tail.js";
4+
5+
const hasUnpairedUtf16Surrogate = (text: string): boolean =>
6+
/[\uD800-\uDBFF](?![\uDC00-\uDFFF])/.test(text);
7+
8+
describe("appendBoundedTextTail", () => {
9+
it("keeps stderr tail UTF-16 safe when the boundary bisects an emoji", () => {
10+
const stderr = appendBoundedTextTail("prefix", Buffer.from("🤖tail"), 5);
11+
12+
expect(stderr).toBe("tail");
13+
expect(hasUnpairedUtf16Surrogate(stderr)).toBe(false);
14+
});
15+
});
16+
17+
describe("projectBoundedTextTail", () => {
18+
it("keeps final error projection UTF-16 safe when the boundary bisects an emoji", () => {
19+
const stderr = "p".repeat(5) + "🤖fail";
20+
const projected = projectBoundedTextTail(stderr, 5);
21+
22+
expect(projected).toBe("fail");
23+
expect(hasUnpairedUtf16Surrogate(projected)).toBe(false);
24+
});
25+
});

‎extensions/file-transfer/src/shared/append-bounded-text-tail.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Bounded stderr tails must stay UTF-16 safe when tar/dir-fetch paths mention emoji filenames.
2+
import { sliceUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime";
3+
4+
export function appendBoundedTextTail(current: string, chunk: Buffer, maxChars: number): string {
5+
const next = current + chunk.toString();
6+
if (next.length <= maxChars) {
7+
return next;
8+
}
9+
return sliceUtf16Safe(next, Math.max(0, next.length - maxChars));
10+
}
11+
12+
// Final error projections reuse the ring-buffer tail; raw slice(-N) can still bisect emoji.
13+
export function projectBoundedTextTail(text: string, maxChars: number): string {
14+
return sliceUtf16Safe(text, Math.max(0, text.length - maxChars));
15+
}

‎extensions/file-transfer/src/shared/node-invoke-policy.stream-errors.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,32 @@ describe("dir.fetch archive policy output lifecycle", () => {
7070
}),
7171
).resolves.toEqual({ ok: true, entries: ["ok.txt"] });
7272
});
73+
74+
it("surfaces UTF-16 safe tar stderr tail when archive listing fails with emoji at projection boundary", async () => {
75+
const oldNoise = "n".repeat(250);
76+
// Length 201: raw slice(-200) would start on the low surrogate of 🤖.
77+
const recent = "🤖" + "f".repeat(199);
78+
const spawnMock = mockTarSpawn((child) => {
79+
child.stderr.emit("data", Buffer.from(oldNoise));
80+
child.stderr.emit("data", Buffer.from(recent));
81+
child.emit("close", 2);
82+
});
83+
const { testing } = await importWithSpawn(spawnMock);
84+
const { projectBoundedTextTail } = await import("./append-bounded-text-tail.js");
85+
86+
const result = await testing.listDirFetchArchiveEntries({
87+
tarBase64: Buffer.from("archive").toString("base64"),
88+
});
89+
expect(result.ok).toBe(false);
90+
if (!result.ok) {
91+
expect(result.reason).toContain(projectBoundedTextTail(recent, 200));
92+
expect(result.reason).toContain("f".repeat(199));
93+
expect(result.reason).not.toContain("🤖");
94+
expect(
95+
/[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]/.test(
96+
result.reason,
97+
),
98+
).toBe(false);
99+
}
100+
});
73101
});

‎extensions/file-transfer/src/shared/node-invoke-policy.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type {
66
OpenClawPluginNodeInvokePolicyContext,
77
OpenClawPluginNodeInvokePolicyResult,
88
} from "openclaw/plugin-sdk/plugin-entry";
9+
import { appendBoundedTextTail, projectBoundedTextTail } from "./append-bounded-text-tail.js";
910
import { appendFileTransferAudit, type FileTransferAuditOp } from "./audit.js";
1011
import { consumeChildOutput } from "./child-output.js";
1112
import {
@@ -22,6 +23,7 @@ const DIR_FETCH_MAX_ENTRIES = 5000;
2223
const DIR_FETCH_ARCHIVE_LIST_TIMEOUT_MS = 30_000;
2324
const DIR_FETCH_ARCHIVE_LIST_MAX_OUTPUT_BYTES = 32 * 1024 * 1024;
2425
const DIR_FETCH_ARCHIVE_LIST_STDERR_TAIL_CHARS = 4096;
26+
const DIR_FETCH_ARCHIVE_LIST_ERROR_STDERR_CHARS = 200;
2527

2628
type FileTransferCommand = FileTransferNodeInvokeCommand;
2729

@@ -31,11 +33,6 @@ function asRecord(value: unknown): Record<string, unknown> {
3133
: {};
3234
}
3335

34-
function appendBoundedTextTail(current: string, chunk: Buffer, maxChars: number): string {
35-
const next = current + chunk.toString();
36-
return next.length > maxChars ? next.slice(-maxChars) : next;
37-
}
38-
3936
function readPath(params: Record<string, unknown>): string {
4037
return typeof params.path === "string" ? params.path.trim() : "";
4138
}
@@ -424,7 +421,7 @@ async function listDirFetchArchiveEntries(
424421
finish({
425422
ok: false,
426423
code: "ARCHIVE_ENTRIES_UNREADABLE",
427-
reason: `tar -tzf exited ${code}: ${stderr.slice(-200)}`,
424+
reason: `tar -tzf exited ${code}: ${projectBoundedTextTail(stderr, DIR_FETCH_ARCHIVE_LIST_ERROR_STDERR_CHARS)}`,
428425
});
429426
return;
430427
}

‎extensions/file-transfer/src/tools/dir-fetch-tool.test.ts‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import fs from "node:fs/promises";
55
import os from "node:os";
66
import path from "node:path";
77
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
8+
import { projectBoundedTextTail } from "../shared/append-bounded-text-tail.js";
89
import { validateTarUncompressedBudget } from "./dir-fetch-tool.js";
910

1011
let tmpRoot: string;
@@ -276,12 +277,49 @@ describe("dir.fetch tar validation", () => {
276277
const result = await testing.preValidateTarball(Buffer.from("x"));
277278
expect(result.ok).toBe(false);
278279
if (!result.ok) {
279-
expect(result.reason).toContain(recent.slice(-200));
280+
expect(result.reason).toContain(projectBoundedTextTail(recent, 200));
280281
expect(result.reason).not.toContain(oldNoise.slice(0, 40));
281282
}
282283
} finally {
283284
vi.doUnmock("node:child_process");
284285
vi.resetModules();
285286
}
286287
});
288+
289+
it("surfaces UTF-16 safe tar stderr tail when listing fails with emoji at projection boundary", async () => {
290+
vi.resetModules();
291+
const oldNoise = "n".repeat(250);
292+
// Length 201: raw slice(-200) would start on the low surrogate of 🤖.
293+
const recent = "🤖" + "f".repeat(199);
294+
vi.doMock("node:child_process", async (importOriginal) => {
295+
const actual = await importOriginal<typeof import("node:child_process")>();
296+
return {
297+
...actual,
298+
spawn: mockTarSpawn((child) => {
299+
child.stderr.emit("data", Buffer.from(oldNoise));
300+
child.stderr.emit("data", Buffer.from(recent));
301+
child.emit("close", 2);
302+
}),
303+
};
304+
});
305+
306+
try {
307+
const { testing } = await import("./dir-fetch-tool.js");
308+
const result = await testing.preValidateTarball(Buffer.from("x"));
309+
expect(result.ok).toBe(false);
310+
if (!result.ok) {
311+
expect(result.reason).toContain(projectBoundedTextTail(recent, 200));
312+
expect(result.reason).toContain("f".repeat(199));
313+
expect(result.reason).not.toContain("🤖");
314+
expect(
315+
/[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]/.test(
316+
result.reason,
317+
),
318+
).toBe(false);
319+
}
320+
} finally {
321+
vi.doUnmock("node:child_process");
322+
vi.resetModules();
323+
}
324+
});
287325
});

‎extensions/file-transfer/src/tools/dir-fetch-tool.ts‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import fs from "node:fs/promises";
55
import path from "node:path";
66
import type { AnyAgentTool } from "openclaw/plugin-sdk/agent-harness-runtime";
77
import { saveMediaBuffer } from "openclaw/plugin-sdk/media-store";
8+
import {
9+
appendBoundedTextTail,
10+
projectBoundedTextTail,
11+
} from "../shared/append-bounded-text-tail.js";
812
import { appendFileTransferAudit } from "../shared/audit.js";
913
import { consumeChildOutput } from "../shared/child-output.js";
1014
import { IMAGE_MIME_INLINE_SET, mimeFromExtension } from "../shared/mime.js";
@@ -32,6 +36,8 @@ const TAR_UNPACK_TIMEOUT_MS = 60_000;
3236
const TAR_UNPACK_MAX_ENTRIES = 5000;
3337
const TAR_LIST_OUTPUT_MAX_CHARS = 32 * 1024 * 1024;
3438
const TAR_STDERR_TAIL_CHARS = 4096;
39+
const TAR_ERROR_REASON_STDERR_CHARS = 200;
40+
const TAR_UNPACK_ERROR_STDERR_CHARS = 300;
3541

3642
// Hard caps on uncompressed extraction. Defends against decompression-bomb
3743
// archives that compress to <16MB but expand to gigabytes. Both caps are
@@ -40,11 +46,6 @@ const TAR_STDERR_TAIL_CHARS = 4096;
4046
const DIR_FETCH_MAX_UNCOMPRESSED_BYTES = 64 * 1024 * 1024;
4147
const DIR_FETCH_MAX_SINGLE_FILE_BYTES = 16 * 1024 * 1024;
4248

43-
function appendBoundedTextTail(current: string, chunk: Buffer, maxChars: number): string {
44-
const next = current + chunk.toString();
45-
return next.length > maxChars ? next.slice(-maxChars) : next;
46-
}
47-
4849
async function listTarOutputLines<T>(input: {
4950
args: string[];
5051
label: string;
@@ -137,7 +138,10 @@ async function listTarOutputLines<T>(input: {
137138
return;
138139
}
139140
if (code !== 0) {
140-
finish({ ok: false, reason: `${input.label} exited ${code}: ${stderr.slice(-200)}` });
141+
finish({
142+
ok: false,
143+
reason: `${input.label} exited ${code}: ${projectBoundedTextTail(stderr, TAR_ERROR_REASON_STDERR_CHARS)}`,
144+
});
141145
return;
142146
}
143147
if (pending) {
@@ -350,7 +354,7 @@ export async function validateTarUncompressedBudget(
350354
if (code !== 0) {
351355
finish({
352356
ok: false,
353-
reason: `tar uncompressed budget validation exited ${code}: ${stderr.slice(-200)}`,
357+
reason: `tar uncompressed budget validation exited ${code}: ${projectBoundedTextTail(stderr, TAR_ERROR_REASON_STDERR_CHARS)}`,
354358
});
355359
return;
356360
}
@@ -455,7 +459,11 @@ async function unpackTar(tarBuffer: Buffer, destDir: string): Promise<void> {
455459
});
456460
child.on("close", (code) => {
457461
if (code !== 0) {
458-
fail(new Error(`tar unpack exited ${code}: ${stderrOut.slice(-300)}`));
462+
fail(
463+
new Error(
464+
`tar unpack exited ${code}: ${projectBoundedTextTail(stderrOut, TAR_UNPACK_ERROR_STDERR_CHARS)}`,
465+
),
466+
);
459467
return;
460468
}
461469
succeed();

0 commit comments

Comments
 (0)