mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-12 09:41:11 +00:00
fix: suppress gateway control reply leakage
Regeneration-Prompt: | Prepare PR 51739 after review found two remaining user-facing leaks in gateway chat handling. Keep the original exact-token suppression for ANNOUNCE_SKIP and REPLY_SKIP, but also suppress streamed lead fragments of those internal control replies so partial deltas do not leak before the exact token is assembled. Preserve the pre-existing NO_REPLY behavior, including the case where a short reply like "No" is hidden during the delta stream but still appears in the final message. Reuse the same exact control-token check in chat history sanitization so assistant-only ANNOUNCE_SKIP and REPLY_SKIP transcript entries are removed on reload while mixed-content or text-precedence cases continue to work. Add regression coverage in the gateway agent-events tests for streamed prefixes and in the gateway chat-history tests for the new exact tokens, then add the required Unreleased changelog entry for the fix.
This commit is contained in:
@@ -46,7 +46,8 @@ Docs: https://docs.openclaw.ai
|
||||
- Security/dependency audit: bump Hono to `4.12.12` and `@hono/node-server` to `1.19.13` in production resolution paths.
|
||||
- Slack/partial streaming: keep the final fallback reply path active when preview finalization fails so stale preview text cannot suppress the actual final answer. (#62859) Thanks @gumadeiras.
|
||||
- Plugins/contracts: keep test-only helpers out of production contract barrels, load shared contract harnesses through bundled test surfaces, and harden guardrails so indirect re-exports and canonical `*.test.ts` files stay blocked. (#63311) Thanks @altaywtf.
|
||||
- Sessions/routing: preserve established external routes on inter-session announce traffic so `sessions_send` follow-ups do not steal delivery from Telegram, Discord, or other external channels. (#58013) thanks @duqaXxX.
|
||||
- Sessions/routing: preserve established external routes on inter-session announce traffic so `sessions_send` follow-ups do not steal delivery from Telegram, Discord, or other external channels. (#58013) Thanks @duqaXxX.
|
||||
- Gateway/chat: suppress exact and streamed `ANNOUNCE_SKIP` / `REPLY_SKIP` control replies across live chat updates and history sanitization so internal agent-to-agent control tokens no longer leak into user-facing gateway chat surfaces. (#51739) Thanks @Pinghuachiu.
|
||||
|
||||
## 2026.4.8
|
||||
|
||||
@@ -1481,7 +1482,6 @@ Docs: https://docs.openclaw.ai
|
||||
### Fixes
|
||||
|
||||
- Agents/edit tool: accept common path/text alias spellings, show current file contents on exact-match failures, and avoid false edit failures after successful writes. (#52516) thanks @mbelinky.
|
||||
|
||||
## 2026.3.13
|
||||
|
||||
### Changes
|
||||
|
||||
62
src/gateway/control-reply-text.ts
Normal file
62
src/gateway/control-reply-text.ts
Normal file
@@ -0,0 +1,62 @@
|
||||
import { isSilentReplyText, SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js";
|
||||
|
||||
const SUPPRESSED_CONTROL_REPLY_TOKENS = [
|
||||
SILENT_REPLY_TOKEN,
|
||||
"ANNOUNCE_SKIP",
|
||||
"REPLY_SKIP",
|
||||
] as const;
|
||||
|
||||
const MIN_BARE_PREFIX_LENGTH_BY_TOKEN: Readonly<
|
||||
Record<(typeof SUPPRESSED_CONTROL_REPLY_TOKENS)[number], number>
|
||||
> = {
|
||||
[SILENT_REPLY_TOKEN]: 2,
|
||||
ANNOUNCE_SKIP: 3,
|
||||
REPLY_SKIP: 3,
|
||||
};
|
||||
|
||||
function normalizeSuppressedControlReplyFragment(text: string): string {
|
||||
const trimmed = text.trim();
|
||||
if (!trimmed) {
|
||||
return "";
|
||||
}
|
||||
const normalized = trimmed.toUpperCase();
|
||||
if (/[^A-Z_]/.test(normalized)) {
|
||||
return "";
|
||||
}
|
||||
return normalized;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return true when a chat-visible reply is exactly an internal control token.
|
||||
*/
|
||||
export function isSuppressedControlReplyText(text: string): boolean {
|
||||
const normalized = text.trim();
|
||||
return SUPPRESSED_CONTROL_REPLY_TOKENS.some((token) => isSilentReplyText(normalized, token));
|
||||
}
|
||||
|
||||
/**
|
||||
* Return true when streamed assistant text looks like the leading fragment of a control token.
|
||||
*/
|
||||
export function isSuppressedControlReplyLeadFragment(text: string): boolean {
|
||||
const trimmed = text.trim();
|
||||
const normalized = normalizeSuppressedControlReplyFragment(text);
|
||||
if (!normalized) {
|
||||
return false;
|
||||
}
|
||||
return SUPPRESSED_CONTROL_REPLY_TOKENS.some((token) => {
|
||||
const tokenUpper = token.toUpperCase();
|
||||
if (normalized === tokenUpper) {
|
||||
return false;
|
||||
}
|
||||
if (!tokenUpper.startsWith(normalized)) {
|
||||
return false;
|
||||
}
|
||||
if (normalized.includes("_")) {
|
||||
return true;
|
||||
}
|
||||
if (token !== SILENT_REPLY_TOKEN && trimmed !== trimmed.toUpperCase()) {
|
||||
return false;
|
||||
}
|
||||
return normalized.length >= MIN_BARE_PREFIX_LENGTH_BY_TOKEN[token];
|
||||
});
|
||||
}
|
||||
@@ -275,6 +275,38 @@ describe("agent event handler", () => {
|
||||
nowSpy?.mockRestore();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["ANNOUNCE_SKIP", ["ANN", "ANNOUNCE_", "ANNOUNCE_SKIP"]],
|
||||
["REPLY_SKIP", ["REP", "REPLY_", "REPLY_SKIP"]],
|
||||
] as const)(
|
||||
"suppresses %s lead fragments and does not leak the streamed prefix in the final chat message",
|
||||
(_replyText, fragments) => {
|
||||
const { broadcast, nodeSendToSession, chatRunState, handler, nowSpy } = createHarness({
|
||||
now: 2_150,
|
||||
});
|
||||
chatRunState.registry.add("run-control", {
|
||||
sessionKey: "session-control",
|
||||
clientRunId: "client-control",
|
||||
});
|
||||
|
||||
for (const text of fragments) {
|
||||
handler({
|
||||
runId: "run-control",
|
||||
seq: 1,
|
||||
stream: "assistant",
|
||||
ts: Date.now(),
|
||||
data: { text },
|
||||
});
|
||||
}
|
||||
emitLifecycleEnd(handler, "run-control");
|
||||
|
||||
const payload = expectSingleFinalChatPayload(broadcast) as { message?: unknown };
|
||||
expect(payload.message).toBeUndefined();
|
||||
expect(sessionChatCalls(nodeSendToSession)).toHaveLength(1);
|
||||
nowSpy?.mockRestore();
|
||||
},
|
||||
);
|
||||
|
||||
it("keeps final short replies like 'No' even when lead-fragment deltas are suppressed", () => {
|
||||
const { broadcast, nodeSendToSession, chatRunState, handler, nowSpy } = createHarness({
|
||||
now: 2_200,
|
||||
|
||||
@@ -12,8 +12,8 @@ import {
|
||||
import { dispatchInboundMessage } from "../../auto-reply/dispatch.js";
|
||||
import { createReplyDispatcher } from "../../auto-reply/reply/reply-dispatcher.js";
|
||||
import type { MsgContext } from "../../auto-reply/templating.js";
|
||||
import { isSilentReplyText, SILENT_REPLY_TOKEN } from "../../auto-reply/tokens.js";
|
||||
import type { ReplyPayload } from "../../auto-reply/types.js";
|
||||
import { createReplyPrefixOptions } from "../../channels/reply-prefix.js";
|
||||
import { resolveSessionFilePath } from "../../config/sessions.js";
|
||||
import { formatErrorMessage } from "../../infra/errors.js";
|
||||
import { jsonUtf8Bytes } from "../../infra/json-utf8-bytes.js";
|
||||
@@ -54,6 +54,7 @@ import {
|
||||
} from "../chat-attachments.js";
|
||||
import { stripEnvelopeFromMessage, stripEnvelopeFromMessages } from "../chat-sanitize.js";
|
||||
import { augmentChatHistoryWithCliSessionImports } from "../cli-session-history.js";
|
||||
import { isSuppressedControlReplyText } from "../control-reply-text.js";
|
||||
import { ADMIN_SCOPE } from "../method-scopes.js";
|
||||
import {
|
||||
GATEWAY_CLIENT_CAPS,
|
||||
@@ -732,7 +733,7 @@ function shouldDropAssistantHistoryMessage(message: unknown): boolean {
|
||||
return true;
|
||||
}
|
||||
const text = extractAssistantTextForSilentCheck(message);
|
||||
if (text === undefined || !isSilentReplyText(text, SILENT_REPLY_TOKEN)) {
|
||||
if (text === undefined || !isSuppressedControlReplyText(text)) {
|
||||
return false;
|
||||
}
|
||||
return !hasAssistantNonTextContent(message);
|
||||
@@ -745,14 +746,14 @@ export function sanitizeChatHistoryMessages(messages: unknown[], maxChars: numbe
|
||||
let changed = false;
|
||||
const next: unknown[] = [];
|
||||
for (const message of messages) {
|
||||
// Drop assistant commentary-only entries and NO_REPLY-only entries, but
|
||||
const res = sanitizeChatHistoryMessage(message, maxChars);
|
||||
changed ||= res.changed;
|
||||
// Drop assistant commentary-only entries and exact control replies, but
|
||||
// keep mixed assistant entries that still carry non-text content.
|
||||
if (shouldDropAssistantHistoryMessage(message)) {
|
||||
if (shouldDropAssistantHistoryMessage(res.message)) {
|
||||
changed = true;
|
||||
continue;
|
||||
}
|
||||
const res = sanitizeChatHistoryMessage(message, maxChars);
|
||||
changed ||= res.changed;
|
||||
next.push(res.message);
|
||||
}
|
||||
return changed ? next : messages;
|
||||
|
||||
@@ -565,6 +565,50 @@ describe("gateway server chat", () => {
|
||||
expect(collectHistoryTextValues(historyMessages)).toEqual(["hello", "real reply"]);
|
||||
});
|
||||
|
||||
test("chat.history hides assistant announce/reply skip-only entries", async () => {
|
||||
const historyMessages = await loadChatHistoryWithMessages([
|
||||
{
|
||||
role: "assistant",
|
||||
content: [{ type: "text", text: "ANNOUNCE_SKIP" }],
|
||||
timestamp: 1,
|
||||
},
|
||||
{
|
||||
role: "assistant",
|
||||
content: [{ type: "text", text: "REPLY_SKIP" }],
|
||||
timestamp: 2,
|
||||
},
|
||||
{
|
||||
role: "assistant",
|
||||
text: "real text field reply",
|
||||
content: "ANNOUNCE_SKIP",
|
||||
timestamp: 3,
|
||||
},
|
||||
{
|
||||
role: "assistant",
|
||||
content: [{ type: "text", text: "real reply" }],
|
||||
timestamp: 4,
|
||||
},
|
||||
]);
|
||||
const roleAndText = historyMessages
|
||||
.map((message) => {
|
||||
const role =
|
||||
message &&
|
||||
typeof message === "object" &&
|
||||
typeof (message as { role?: unknown }).role === "string"
|
||||
? (message as { role: string }).role
|
||||
: "unknown";
|
||||
const text =
|
||||
message &&
|
||||
typeof message === "object" &&
|
||||
typeof (message as { text?: unknown }).text === "string"
|
||||
? (message as { text: string }).text
|
||||
: (extractFirstTextBlock(message) ?? "");
|
||||
return `${role}:${text}`;
|
||||
})
|
||||
.filter((entry) => entry !== "unknown:");
|
||||
|
||||
expect(roleAndText).toEqual(["assistant:real text field reply", "assistant:real reply"]);
|
||||
});
|
||||
test("routes chat.send slash commands without agent runs", async () => {
|
||||
await withMainSessionStore(async () => {
|
||||
const spy = vi.mocked(agentCommand);
|
||||
|
||||
Reference in New Issue
Block a user