mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(terminal): sanitize skills JSON and fallback on legacy Windows (#43520)
* Terminal: use ASCII borders on legacy Windows consoles * Skills: sanitize JSON output for control bytes * Changelog: credit terminal follow-up fixes * Update CHANGELOG.md * Update CHANGELOG.md Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Skills: strip remaining escape sequences from JSON output --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
This commit is contained in:
@@ -99,6 +99,8 @@ Docs: https://docs.openclaw.ai
|
||||
- Telegram/final preview delivery followup: keep ambiguous missing-`message_id` finals only when a preview was already visible, while first-preview/no-id cases still fall back so Telegram users do not lose the final reply. (#41932) thanks @hougangdev.
|
||||
- Agents/Azure OpenAI Responses: include the `azure-openai` provider in the Responses API store override so Azure OpenAI multi-turn cron jobs and embedded agent runs no longer fail with HTTP 400 "store is set to false". (#42934, fixes #42800) Thanks @ademczuk.
|
||||
- Agents/context pruning: prune image-only tool results during soft-trim, align context-pruning coverage with the new tool-result contract, and extend historical image cleanup to the same screenshot-heavy session path. (#43045) Thanks @MoerAI.
|
||||
- CLI/skills JSON: strip ANSI and C1 control bytes from `skills list --json`, `skills info --json`, and `skills check --json` so machine-readable output stays valid for terminals and skill metadata with embedded control characters. Fixes #27530. Related #27557. Thanks @Jimmy-xuzimo and @vincentkoc.
|
||||
- CLI/tables: default shared tables to ASCII borders on legacy Windows consoles while keeping Unicode borders on modern Windows terminals, so commands like `openclaw skills` stop rendering mojibake under GBK/936 consoles. Fixes #40853. Related #41015. Thanks @ApacheBin and @vincentkoc.
|
||||
- fix(models): guard optional model.input capability checks (#42096) thanks @andyliu
|
||||
- Security/plugin runtime: stop unauthenticated plugin HTTP routes from inheriting synthetic admin gateway scopes when they call `runtime.subagent.*`, so admin-only methods like `sessions.delete` stay blocked without gateway auth.
|
||||
- Security/session_status: enforce sandbox session-tree visibility and shared agent-to-agent access guards before reading or mutating target session state, so sandboxed subagents can no longer inspect parent session metadata or write parent model overrides via `session_status`.
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { SkillStatusEntry, SkillStatusReport } from "../agents/skills-status.js";
|
||||
import { stripAnsi } from "../terminal/ansi.js";
|
||||
import { getTerminalTableWidth, renderTable } from "../terminal/table.js";
|
||||
import { theme } from "../terminal/theme.js";
|
||||
import { shortenHomePath } from "../utils.js";
|
||||
@@ -42,6 +43,33 @@ function normalizeSkillEmoji(emoji?: string): string {
|
||||
return (emoji ?? "📦").replaceAll("\uFE0E", "\uFE0F");
|
||||
}
|
||||
|
||||
const REMAINING_ESC_SEQUENCE_REGEX = new RegExp(
|
||||
String.raw`\u001b(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])`,
|
||||
"g",
|
||||
);
|
||||
const JSON_CONTROL_CHAR_REGEX = new RegExp(String.raw`[\u0000-\u001f\u007f-\u009f]`, "g");
|
||||
|
||||
function sanitizeJsonString(value: string): string {
|
||||
return stripAnsi(value)
|
||||
.replace(REMAINING_ESC_SEQUENCE_REGEX, "")
|
||||
.replace(JSON_CONTROL_CHAR_REGEX, "");
|
||||
}
|
||||
|
||||
function sanitizeJsonValue(value: unknown): unknown {
|
||||
if (typeof value === "string") {
|
||||
return sanitizeJsonString(value);
|
||||
}
|
||||
if (Array.isArray(value)) {
|
||||
return value.map((item) => sanitizeJsonValue(item));
|
||||
}
|
||||
if (value && typeof value === "object") {
|
||||
return Object.fromEntries(
|
||||
Object.entries(value).map(([key, entryValue]) => [key, sanitizeJsonValue(entryValue)]),
|
||||
);
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
function formatSkillName(skill: SkillStatusEntry): string {
|
||||
const emoji = normalizeSkillEmoji(skill.emoji);
|
||||
return `${emoji} ${theme.command(skill.name)}`;
|
||||
@@ -71,7 +99,7 @@ export function formatSkillsList(report: SkillStatusReport, opts: SkillsListOpti
|
||||
const skills = opts.eligible ? report.skills.filter((s) => s.eligible) : report.skills;
|
||||
|
||||
if (opts.json) {
|
||||
const jsonReport = {
|
||||
const jsonReport = sanitizeJsonValue({
|
||||
workspaceDir: report.workspaceDir,
|
||||
managedSkillsDir: report.managedSkillsDir,
|
||||
skills: skills.map((s) => ({
|
||||
@@ -87,7 +115,7 @@ export function formatSkillsList(report: SkillStatusReport, opts: SkillsListOpti
|
||||
homepage: s.homepage,
|
||||
missing: s.missing,
|
||||
})),
|
||||
};
|
||||
});
|
||||
return JSON.stringify(jsonReport, null, 2);
|
||||
}
|
||||
|
||||
@@ -154,7 +182,7 @@ export function formatSkillInfo(
|
||||
}
|
||||
|
||||
if (opts.json) {
|
||||
return JSON.stringify(skill, null, 2);
|
||||
return JSON.stringify(sanitizeJsonValue(skill), null, 2);
|
||||
}
|
||||
|
||||
const lines: string[] = [];
|
||||
@@ -251,7 +279,7 @@ export function formatSkillsCheck(report: SkillStatusReport, opts: SkillsCheckOp
|
||||
|
||||
if (opts.json) {
|
||||
return JSON.stringify(
|
||||
{
|
||||
sanitizeJsonValue({
|
||||
summary: {
|
||||
total: report.skills.length,
|
||||
eligible: eligible.length,
|
||||
@@ -267,7 +295,7 @@ export function formatSkillsCheck(report: SkillStatusReport, opts: SkillsCheckOp
|
||||
missing: s.missing,
|
||||
install: s.install,
|
||||
})),
|
||||
},
|
||||
}),
|
||||
null,
|
||||
2,
|
||||
);
|
||||
|
||||
@@ -243,5 +243,46 @@ describe("skills-cli", () => {
|
||||
const parsed = JSON.parse(output) as Record<string, unknown>;
|
||||
assert(parsed);
|
||||
});
|
||||
|
||||
it("sanitizes ANSI and C1 controls in skills list JSON output", () => {
|
||||
const report = createMockReport([
|
||||
createMockSkill({
|
||||
name: "json-skill",
|
||||
emoji: "\u001b[31m📧\u001b[0m\u009f",
|
||||
description: "desc\u0093\u001b[2J\u001b[33m colored\u001b[0m",
|
||||
}),
|
||||
]);
|
||||
|
||||
const output = formatSkillsList(report, { json: true });
|
||||
const parsed = JSON.parse(output) as {
|
||||
skills: Array<{ emoji: string; description: string }>;
|
||||
};
|
||||
|
||||
expect(parsed.skills[0]?.emoji).toBe("📧");
|
||||
expect(parsed.skills[0]?.description).toBe("desc colored");
|
||||
expect(output).not.toContain("\\u001b");
|
||||
});
|
||||
|
||||
it("sanitizes skills info JSON output", () => {
|
||||
const report = createMockReport([
|
||||
createMockSkill({
|
||||
name: "info-json",
|
||||
emoji: "\u001b[31m🎙\u001b[0m\u009f",
|
||||
description: "hi\u0091",
|
||||
homepage: "https://example.com/\u0092docs",
|
||||
}),
|
||||
]);
|
||||
|
||||
const output = formatSkillInfo(report, "info-json", { json: true });
|
||||
const parsed = JSON.parse(output) as {
|
||||
emoji: string;
|
||||
description: string;
|
||||
homepage: string;
|
||||
};
|
||||
|
||||
expect(parsed.emoji).toBe("🎙");
|
||||
expect(parsed.description).toBe("hi");
|
||||
expect(parsed.homepage).toBe("https://example.com/docs");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,9 +1,18 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { visibleWidth } from "./ansi.js";
|
||||
import { wrapNoteMessage } from "./note.js";
|
||||
import { renderTable } from "./table.js";
|
||||
|
||||
describe("renderTable", () => {
|
||||
const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform");
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
if (originalPlatformDescriptor) {
|
||||
Object.defineProperty(process, "platform", originalPlatformDescriptor);
|
||||
}
|
||||
});
|
||||
|
||||
it("prefers shrinking flex columns to avoid wrapping non-flex labels", () => {
|
||||
const out = renderTable({
|
||||
width: 40,
|
||||
@@ -170,6 +179,42 @@ describe("renderTable", () => {
|
||||
expect(out).toContain("before");
|
||||
expect(out).toContain("after");
|
||||
});
|
||||
|
||||
it("falls back to ASCII borders on legacy Windows consoles", () => {
|
||||
Object.defineProperty(process, "platform", { value: "win32", configurable: true });
|
||||
vi.stubEnv("WT_SESSION", "");
|
||||
vi.stubEnv("TERM_PROGRAM", "");
|
||||
vi.stubEnv("TERM", "vt100");
|
||||
|
||||
const out = renderTable({
|
||||
columns: [
|
||||
{ key: "A", header: "A", minWidth: 6 },
|
||||
{ key: "B", header: "B", minWidth: 10, flex: true },
|
||||
],
|
||||
rows: [{ A: "row", B: "value" }],
|
||||
});
|
||||
|
||||
expect(out).toContain("+");
|
||||
expect(out).not.toContain("┌");
|
||||
});
|
||||
|
||||
it("keeps unicode borders on modern Windows terminals", () => {
|
||||
Object.defineProperty(process, "platform", { value: "win32", configurable: true });
|
||||
vi.stubEnv("WT_SESSION", "1");
|
||||
vi.stubEnv("TERM", "");
|
||||
vi.stubEnv("TERM_PROGRAM", "");
|
||||
|
||||
const out = renderTable({
|
||||
columns: [
|
||||
{ key: "A", header: "A", minWidth: 6 },
|
||||
{ key: "B", header: "B", minWidth: 10, flex: true },
|
||||
],
|
||||
rows: [{ A: "row", B: "value" }],
|
||||
});
|
||||
|
||||
expect(out).toContain("┌");
|
||||
expect(out).not.toContain("+");
|
||||
});
|
||||
});
|
||||
|
||||
describe("wrapNoteMessage", () => {
|
||||
|
||||
@@ -20,6 +20,26 @@ export type RenderTableOptions = {
|
||||
border?: "unicode" | "ascii" | "none";
|
||||
};
|
||||
|
||||
function resolveDefaultBorder(
|
||||
platform: NodeJS.Platform,
|
||||
env: NodeJS.ProcessEnv,
|
||||
): "unicode" | "ascii" {
|
||||
if (platform !== "win32") {
|
||||
return "unicode";
|
||||
}
|
||||
|
||||
const term = env.TERM ?? "";
|
||||
const termProgram = env.TERM_PROGRAM ?? "";
|
||||
const isModernTerminal =
|
||||
Boolean(env.WT_SESSION) ||
|
||||
term.includes("xterm") ||
|
||||
term.includes("cygwin") ||
|
||||
term.includes("msys") ||
|
||||
termProgram === "vscode";
|
||||
|
||||
return isModernTerminal ? "unicode" : "ascii";
|
||||
}
|
||||
|
||||
function repeat(ch: string, n: number): string {
|
||||
if (n <= 0) {
|
||||
return "";
|
||||
@@ -267,7 +287,7 @@ export function renderTable(opts: RenderTableOptions): string {
|
||||
}
|
||||
return next;
|
||||
});
|
||||
const border = opts.border ?? "unicode";
|
||||
const border = opts.border ?? resolveDefaultBorder(process.platform, process.env);
|
||||
if (border === "none") {
|
||||
const columns = opts.columns;
|
||||
const header = columns.map((c) => c.header).join(" | ");
|
||||
|
||||
Reference in New Issue
Block a user