Skip to content

Commit cc96100

Browse files
zw-xysksteipete
zw-xysk
and
steipete
authored
fix(security): keep channel-metadata and install-policy truncation surrogate-safe (#102266)
* fix(security): keep channel-metadata and install-policy truncation surrogate-safe channel-metadata.ts and install-policy.ts both had private truncateText functions using String.prototype.slice(0, N) on user-visible text. channel-metadata is explicitly user-controlled untrusted content that gets injected into LLM prompt context -- a dangling surrogate from unsafe truncation would corrupt the prompt with invalid Unicode, affecting token counting and potentially LLM response quality. Replace .slice(0, ...) with truncateUtf16Safe(...) in both files so the truncation point always falls on a complete code-point boundary. Add test coverage for buildUntrustedChannelMetadata with emoji at both the entry-level (400-char) and outer (800-char) truncation boundaries. * test(security): prove UTF-16 truncation boundaries --------- Co-authored-by: Peter Steinberger <[email protected]>
1 parent 5533d97 commit cc96100

4 files changed

Lines changed: 69 additions & 2 deletions

File tree

‎src/security/channel-metadata.test.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Covers bounded, untrusted channel metadata construction.
2+
import { describe, expect, it } from "vitest";
3+
import { buildUntrustedChannelMetadata } from "./channel-metadata.js";
4+
5+
function normalizeMarkerIds(value: string): string {
6+
return value.replace(/id="[a-f0-9]{16}"/g, 'id="<id>"');
7+
}
8+
9+
function wrapExpected(content: string): string {
10+
return [
11+
"",
12+
'<<<EXTERNAL_UNTRUSTED_CONTENT id="<id>">>>',
13+
"Source: Channel metadata",
14+
"---",
15+
content,
16+
'<<<END_EXTERNAL_UNTRUSTED_CONTENT id="<id>">>>',
17+
].join("\n");
18+
}
19+
20+
describe("buildUntrustedChannelMetadata", () => {
21+
it("keeps per-entry truncation UTF-16 safe", () => {
22+
const entryPrefix = "a".repeat(396);
23+
const result = buildUntrustedChannelMetadata({
24+
source: "test",
25+
label: "Test channel",
26+
entries: [`${entryPrefix}🎉tail`],
27+
});
28+
29+
expect(normalizeMarkerIds(result ?? "")).toBe(
30+
wrapExpected(`UNTRUSTED channel metadata (test)\nTest channel:\n${entryPrefix}...`),
31+
);
32+
});
33+
34+
it("keeps the combined metadata limit UTF-16 safe", () => {
35+
const header = "UNTRUSTED channel metadata (test)\nTest channel:\n";
36+
const entryPrefix = "short";
37+
const result = buildUntrustedChannelMetadata({
38+
source: "test",
39+
label: "Test channel",
40+
entries: [`${entryPrefix}🎉tail`],
41+
maxChars: header.length + entryPrefix.length + 4,
42+
});
43+
44+
expect(normalizeMarkerIds(result ?? "")).toBe(wrapExpected(`${header}${entryPrefix}...`));
45+
});
46+
});

‎src/security/channel-metadata.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Extracts channel metadata used by security audit findings.
22
import { uniqueStrings } from "@openclaw/normalization-core/string-normalization";
3+
import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice";
34
import { wrapExternalContent } from "./external-content.js";
45

56
const DEFAULT_MAX_CHARS = 800;
@@ -16,7 +17,7 @@ function truncateText(value: string, maxChars: number): string {
1617
if (value.length <= maxChars) {
1718
return value;
1819
}
19-
const trimmed = value.slice(0, Math.max(0, maxChars - 3)).trimEnd();
20+
const trimmed = truncateUtf16Safe(value, Math.max(0, maxChars - 3)).trimEnd();
2021
return `${trimmed}...`;
2122
}
2223

‎src/security/install-policy.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,25 @@ describe("runInstallPolicy", () => {
355355
expect(warnings.join("\n")).toContain("blocked by install policy");
356356
});
357357

358+
it("keeps truncated operator block reasons UTF-16 safe", async () => {
359+
const reasonPrefix = "r".repeat(999);
360+
const result = await runInstallPolicy({
361+
config: configWithPolicy(scriptPath, {
362+
POLICY_RESPONSE: JSON.stringify({
363+
protocolVersion: 1,
364+
decision: "block",
365+
reason: `${reasonPrefix}🎉tail`,
366+
}),
367+
}),
368+
request: baseRequest(sourceDir),
369+
});
370+
371+
expect(result?.blocked).toEqual({
372+
code: "security_scan_blocked",
373+
reason: `blocked by install policy: ${reasonPrefix}...`,
374+
});
375+
});
376+
358377
it("preserves allow findings without file or line", async () => {
359378
const result = await runInstallPolicy({
360379
config: configWithPolicy(scriptPath, {

‎src/security/install-policy.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import { spawn } from "node:child_process";
33
import fs from "node:fs/promises";
44
import path from "node:path";
5+
import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice";
56
import type { OpenClawConfig, SecurityConfig } from "../config/types.openclaw.js";
67
import { formatErrorMessage } from "../infra/errors.js";
78
import {
@@ -378,7 +379,7 @@ async function assertSecurePolicyScriptArg(params: {
378379
}
379380

380381
function truncateText(value: string, maxChars: number): string {
381-
return value.length <= maxChars ? value : `${value.slice(0, maxChars)}...`;
382+
return value.length <= maxChars ? value : `${truncateUtf16Safe(value, maxChars)}...`;
382383
}
383384

384385
function createPolicyChildEnv(sourceEnv: NodeJS.ProcessEnv): NodeJS.ProcessEnv {

0 commit comments

Comments
 (0)