mirror of
https://github.com/openclaw/openclaw.git
synced 2026-10-03 01:29:56 +00:00
fix(ui): prevent Control UI hangs on malformed progress content (#147528)
* fix(ui): prevent progress card regex stalls * test(ui): prove adversarial progress rendering * fix(ui): preserve progress raw tag semantics
This commit is contained in:
parent
63e7363c89
commit
ad157baa20
4 changed files with 303 additions and 3 deletions
135
ui/src/components/markdown-raw-content.ts
Normal file
135
ui/src/components/markdown-raw-content.ts
Normal file
|
|
@ -0,0 +1,135 @@
|
|||
const PROGRESS_CARD_RAW_CONTENT_TAGS = [
|
||||
{ name: "script", pattern: /^script$/iu },
|
||||
{ name: "style", pattern: /^style$/iu },
|
||||
{ name: "iframe", pattern: /^iframe$/iu },
|
||||
{ name: "object", pattern: /^object$/iu },
|
||||
{ name: "template", pattern: /^template$/iu },
|
||||
];
|
||||
const PROGRESS_CARD_RAW_CONTENT_WORD_CHARACTER_RE = /^\w$/iu;
|
||||
const PROGRESS_CARD_RAW_CONTENT_WHITESPACE_RE = /^\s$/u;
|
||||
|
||||
interface ProgressCardRawContentTag {
|
||||
end: number;
|
||||
isClosing: boolean;
|
||||
name: string;
|
||||
start: number;
|
||||
}
|
||||
|
||||
function readProgressCardRawContentTag(
|
||||
input: string,
|
||||
start: number,
|
||||
close: number,
|
||||
): ProgressCardRawContentTag | null {
|
||||
let nameStart = start + 1;
|
||||
const isClosing = input[nameStart] === "/";
|
||||
if (isClosing) {
|
||||
nameStart += 1;
|
||||
}
|
||||
const candidate = PROGRESS_CARD_RAW_CONTENT_TAGS.find((entry) => {
|
||||
const nameEnd = nameStart + entry.name.length;
|
||||
return (
|
||||
entry.pattern.test(input.slice(nameStart, nameEnd)) &&
|
||||
!PROGRESS_CARD_RAW_CONTENT_WORD_CHARACTER_RE.test(input[nameEnd] ?? "")
|
||||
);
|
||||
});
|
||||
if (!candidate) {
|
||||
return null;
|
||||
}
|
||||
const name = candidate.name;
|
||||
const nameEnd = nameStart + name.length;
|
||||
if (isClosing) {
|
||||
for (let index = nameEnd; index < close; index += 1) {
|
||||
if (!PROGRESS_CARD_RAW_CONTENT_WHITESPACE_RE.test(input[index] ?? "")) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
return { start, end: close + 1, isClosing, name };
|
||||
}
|
||||
|
||||
export function stripProgressCardRawContentBlocks(input: string): string {
|
||||
const tags: ProgressCardRawContentTag[] = [];
|
||||
let searchFrom = 0;
|
||||
let nextClose = input.indexOf(">");
|
||||
while (searchFrom < input.length) {
|
||||
const start = input.indexOf("<", searchFrom);
|
||||
if (start === -1) {
|
||||
break;
|
||||
}
|
||||
while (nextClose !== -1 && nextClose < start) {
|
||||
nextClose = input.indexOf(">", nextClose + 1);
|
||||
}
|
||||
if (nextClose === -1) {
|
||||
break;
|
||||
}
|
||||
const tag = readProgressCardRawContentTag(input, start, nextClose);
|
||||
if (tag) {
|
||||
tags.push(tag);
|
||||
}
|
||||
// A candidate tag can contain another '<' before its closing '>'. Keep
|
||||
// inspecting those starts so an embedded raw-block closer remains visible
|
||||
// to the pairing pass, matching the previous regex's search semantics.
|
||||
searchFrom = start + 1;
|
||||
}
|
||||
|
||||
// Pair each opener with the first compatible close after the opener ends.
|
||||
// Closing-tag positions are monotonic, so binary search avoids rescanning the
|
||||
// remaining message for every unmatched opening tag.
|
||||
const closingTagsByName = new Map<string, number[]>();
|
||||
for (let index = 0; index < tags.length; index += 1) {
|
||||
const tag = tags[index];
|
||||
if (!tag?.isClosing) {
|
||||
continue;
|
||||
}
|
||||
const indices = closingTagsByName.get(tag.name) ?? [];
|
||||
indices.push(index);
|
||||
closingTagsByName.set(tag.name, indices);
|
||||
}
|
||||
const matchingClose = Array.from({ length: tags.length }, () => -1);
|
||||
for (let index = 0; index < tags.length; index += 1) {
|
||||
const tag = tags[index];
|
||||
if (!tag || tag.isClosing) {
|
||||
continue;
|
||||
}
|
||||
const closingIndices = closingTagsByName.get(tag.name);
|
||||
if (!closingIndices) {
|
||||
continue;
|
||||
}
|
||||
let low = 0;
|
||||
let high = closingIndices.length;
|
||||
while (low < high) {
|
||||
const middle = low + Math.floor((high - low) / 2);
|
||||
const closeIndex = closingIndices[middle];
|
||||
const close = closeIndex === undefined ? undefined : tags[closeIndex];
|
||||
if (close && close.start >= tag.end) {
|
||||
high = middle;
|
||||
} else {
|
||||
low = middle + 1;
|
||||
}
|
||||
}
|
||||
matchingClose[index] = closingIndices[low] ?? -1;
|
||||
}
|
||||
|
||||
let output = "";
|
||||
let cursor = 0;
|
||||
for (let index = 0; index < tags.length; index += 1) {
|
||||
const tag = tags[index];
|
||||
if (!tag) {
|
||||
continue;
|
||||
}
|
||||
const closeIndex = matchingClose[index] ?? -1;
|
||||
if (tag.isClosing || closeIndex < 0) {
|
||||
continue;
|
||||
}
|
||||
const close = tags[closeIndex];
|
||||
if (!close) {
|
||||
continue;
|
||||
}
|
||||
output += input.slice(cursor, tag.start);
|
||||
cursor = close.end;
|
||||
while ((tags[index + 1]?.start ?? Number.POSITIVE_INFINITY) < cursor) {
|
||||
index += 1;
|
||||
}
|
||||
}
|
||||
return cursor === 0 ? input : output + input.slice(cursor);
|
||||
}
|
||||
|
|
@ -1,5 +1,6 @@
|
|||
// @vitest-environment jsdom
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { stripProgressCardRawContentBlocks } from "./markdown-raw-content.ts";
|
||||
import { toSanitizedMarkdownHtml } from "./markdown.ts";
|
||||
|
||||
describe("progress-card markdown", () => {
|
||||
|
|
@ -16,4 +17,65 @@ describe("progress-card markdown", () => {
|
|||
expect(progressHtml).not.toContain("<script");
|
||||
expect(progressHtml).not.toContain("alert(2)");
|
||||
});
|
||||
|
||||
it("preserves raw-content block removal semantics", () => {
|
||||
const markdown =
|
||||
'before<SCRIPT data-test="true">alert(1)</script >' +
|
||||
"middle<style>body{display:none}</style><template>hidden</template>after";
|
||||
|
||||
expect(stripProgressCardRawContentBlocks(markdown)).toBe("beforemiddleafter");
|
||||
expect(stripProgressCardRawContentBlocks("before<script>unfinished")).toBe(
|
||||
"before<script>unfinished",
|
||||
);
|
||||
expect(stripProgressCardRawContentBlocks("<script><style </script>VISIBLE</style>")).toBe(
|
||||
"VISIBLE</style>",
|
||||
);
|
||||
expect(stripProgressCardRawContentBlocks("<scriptſ>visible</script>")).toBe(
|
||||
"<scriptſ>visible</script>",
|
||||
);
|
||||
expect(stripProgressCardRawContentBlocks("<ſcript>hidden</ſcript>")).toBe("");
|
||||
expect(stripProgressCardRawContentBlocks("<script>hidden</script\f>")).toBe("");
|
||||
expect(stripProgressCardRawContentBlocks("<script </script>VISIBLE</script>")).toBe("");
|
||||
|
||||
const unicodeBoundaryHtml = toSanitizedMarkdownHtml("`<scriptſ>visible</script>`", {
|
||||
progressBars: true,
|
||||
});
|
||||
const unicodeFoldHtml = toSanitizedMarkdownHtml("`<ſcript>hidden</ſcript>`", {
|
||||
progressBars: true,
|
||||
});
|
||||
const closingWhitespaceHtml = toSanitizedMarkdownHtml(
|
||||
"`before<script>hidden</script\f>after`",
|
||||
{ progressBars: true },
|
||||
);
|
||||
const embeddedCloserHtml = toSanitizedMarkdownHtml(
|
||||
"`before<script </script>VISIBLE</script>after`",
|
||||
{ progressBars: true },
|
||||
);
|
||||
|
||||
expect(unicodeBoundaryHtml).toContain("visible");
|
||||
expect(unicodeFoldHtml).not.toContain("hidden");
|
||||
expect(closingWhitespaceHtml).not.toContain("hidden");
|
||||
expect(closingWhitespaceHtml).toContain("beforeafter");
|
||||
expect(embeddedCloserHtml).not.toContain("VISIBLE");
|
||||
expect(embeddedCloserHtml).toContain("beforeafter");
|
||||
});
|
||||
|
||||
it("keeps raw-content preprocessing bounded for repeated unclosed tags", () => {
|
||||
const markdown = "<script>".repeat(17_500);
|
||||
const startedAt = performance.now();
|
||||
|
||||
const progressHtml = toSanitizedMarkdownHtml(markdown, { progressBars: true });
|
||||
|
||||
expect(progressHtml).not.toContain("<script");
|
||||
expect(performance.now() - startedAt).toBeLessThan(100);
|
||||
});
|
||||
|
||||
it("keeps malformed closing-tag validation bounded", () => {
|
||||
const markdown = "</script ".repeat(7_000) + " ".repeat(70_000) + ">";
|
||||
const startedAt = performance.now();
|
||||
|
||||
toSanitizedMarkdownHtml(markdown, { progressBars: true });
|
||||
|
||||
expect(performance.now() - startedAt).toBeLessThan(100);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import { renderAssistantTranscriptPlainTextFallback } from "./markdown-assistant
|
|||
import { renderMarkdownCodeBlock } from "./markdown-code-blocks.ts";
|
||||
import { isHostLocalMarkdownFileHref } from "./markdown-file-links.ts";
|
||||
import { createMarkdownParser } from "./markdown-parser.ts";
|
||||
import { stripProgressCardRawContentBlocks } from "./markdown-raw-content.ts";
|
||||
import {
|
||||
normalizeMarkdownRenderOptions,
|
||||
type MarkdownRenderEnv,
|
||||
|
|
@ -96,8 +97,6 @@ const progressSanitizeOptions = {
|
|||
ALLOWED_TAGS: [...allowedTags, "progress"],
|
||||
ALLOWED_ATTR: [...allowedAttrs, "value", "max"],
|
||||
};
|
||||
const PROGRESS_CARD_RAW_CONTENT_BLOCK_RE =
|
||||
/<(script|style|iframe|object|template)\b[^>]*>[\s\S]*?<\/\1\s*>/giu;
|
||||
|
||||
let hooksInstalled = false;
|
||||
const MARKDOWN_CHAR_LIMIT = 140_000;
|
||||
|
|
@ -553,7 +552,7 @@ function renderSanitizedMarkdown(renderInput: string, renderOptions: MarkdownRen
|
|||
? { text: renderInput, truncated: false, total: renderInput.length }
|
||||
: truncateText(renderInput, MARKDOWN_CHAR_LIMIT);
|
||||
const input = renderOptions.progressBars
|
||||
? appendMarkdownTruncationNotice(truncated).replace(PROGRESS_CARD_RAW_CONTENT_BLOCK_RE, "")
|
||||
? stripProgressCardRawContentBlocks(appendMarkdownTruncationNotice(truncated))
|
||||
: appendMarkdownTruncationNotice(truncated);
|
||||
if (isMarkdownBlockArtText(truncated.text)) {
|
||||
return DOMPurify.sanitize(
|
||||
|
|
|
|||
104
ui/src/e2e/markdown-progress-adversarial.e2e.test.ts
Normal file
104
ui/src/e2e/markdown-progress-adversarial.e2e.test.ts
Normal file
|
|
@ -0,0 +1,104 @@
|
|||
import type { Page } from "playwright";
|
||||
import { expect, it } from "vitest";
|
||||
import {
|
||||
captureUiProof,
|
||||
chatSessionListResponse,
|
||||
controlUiSessionUrl,
|
||||
createChatFlowE2eSuite,
|
||||
installMockGateway,
|
||||
} from "./chat-flow.test-support.ts";
|
||||
|
||||
const suite = createChatFlowE2eSuite();
|
||||
|
||||
async function captureProof(page: Page, fileName: string): Promise<void> {
|
||||
await captureUiProof(suite, page, "markdown-progress-adversarial", fileName);
|
||||
}
|
||||
|
||||
suite.define(() => {
|
||||
it("keeps an adversarial progress payload responsive through a Gateway refresh", async () => {
|
||||
const now = Date.now();
|
||||
const selectedSessionKey = "agent:main:adversarial-selected";
|
||||
const sessionKey = "agent:main:adversarial-progress";
|
||||
const cardResponse = (markdown: string, revision: number) => ({
|
||||
card: {
|
||||
markdown,
|
||||
revision,
|
||||
sessionKey,
|
||||
steps: [
|
||||
{ step: "Inspect", status: "completed" },
|
||||
{ step: "Render", status: revision === 1 ? "in_progress" : "completed" },
|
||||
{ step: "Refresh", status: revision === 1 ? "pending" : "in_progress" },
|
||||
],
|
||||
updatedAt: now,
|
||||
},
|
||||
});
|
||||
|
||||
await suite.withPage(
|
||||
{
|
||||
colorScheme: "dark",
|
||||
hasTouch: false,
|
||||
locale: "en-US",
|
||||
serviceWorkers: "block",
|
||||
viewport: { height: 900, width: 1280 },
|
||||
},
|
||||
async ({ page }) => {
|
||||
const gateway = await installMockGateway(page, {
|
||||
featureMethods: ["chat.metadata", "chat.startup", "progressCard.get"],
|
||||
methodResponses: {
|
||||
"progressCard.get": {
|
||||
cases: [
|
||||
{ match: { sessionKey: selectedSessionKey }, response: { card: null } },
|
||||
{
|
||||
match: { sessionKey },
|
||||
response: cardResponse(
|
||||
`Adversarial payload rendered\n\n${"<script>".repeat(17_500)}`,
|
||||
1,
|
||||
),
|
||||
},
|
||||
],
|
||||
},
|
||||
"sessions.list": chatSessionListResponse([
|
||||
{
|
||||
key: selectedSessionKey,
|
||||
kind: "direct",
|
||||
label: "Selected session",
|
||||
updatedAt: now,
|
||||
},
|
||||
{
|
||||
key: sessionKey,
|
||||
kind: "direct",
|
||||
label: "Adversarial progress",
|
||||
updatedAt: now - 1,
|
||||
},
|
||||
]),
|
||||
},
|
||||
sessionKey: selectedSessionKey,
|
||||
});
|
||||
|
||||
await page.goto(controlUiSessionUrl(suite.server.baseUrl, selectedSessionKey));
|
||||
const row = page.locator(`.sidebar-recent-session[data-session-key="${sessionKey}"]`);
|
||||
await row.waitFor({ state: "visible" });
|
||||
const renderStartedAt = performance.now();
|
||||
await row.hover();
|
||||
const card = page.locator(".session-progress-hovercard");
|
||||
await card.waitFor({ state: "visible" });
|
||||
await expect.poll(() => card.textContent()).toContain("Adversarial payload rendered");
|
||||
expect(performance.now() - renderStartedAt).toBeLessThan(2_000);
|
||||
await captureProof(page, "adversarial-payload-rendered.png");
|
||||
|
||||
await gateway.setMethodResponse(
|
||||
"progressCard.get",
|
||||
cardResponse(
|
||||
'**Responsive after refresh**\n\n<progress value="7" max="7"></progress>',
|
||||
2,
|
||||
),
|
||||
);
|
||||
const refreshStartedAt = performance.now();
|
||||
await gateway.emitGatewayEvent("progressCard.changed", { revision: 2, sessionKey });
|
||||
await expect.poll(() => card.textContent()).toContain("Responsive after refresh");
|
||||
expect(performance.now() - refreshStartedAt).toBeLessThan(1_000);
|
||||
await captureProof(page, "adversarial-payload-refreshed.png");
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue