mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-07 04:41:40 +00:00
fix(security): harden spoofed system marker handling
This commit is contained in:
@@ -72,7 +72,7 @@ vi.mock("./session-updates.js", () => ({
|
||||
systemSent,
|
||||
skillsSnapshot: undefined,
|
||||
})),
|
||||
prependSystemEvents: vi.fn().mockImplementation(async ({ prefixedBodyBase }) => prefixedBodyBase),
|
||||
buildQueuedSystemPrompt: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("./typing-mode.js", () => ({
|
||||
@@ -81,6 +81,7 @@ vi.mock("./typing-mode.js", () => ({
|
||||
|
||||
import { runReplyAgent } from "./agent-runner.js";
|
||||
import { routeReply } from "./route-reply.js";
|
||||
import { buildQueuedSystemPrompt } from "./session-updates.js";
|
||||
import { resolveTypingMode } from "./typing-mode.js";
|
||||
|
||||
function baseParams(
|
||||
@@ -294,4 +295,18 @@ describe("runPreparedReply media-only handling", () => {
|
||||
| undefined;
|
||||
expect(call?.suppressTyping).toBe(true);
|
||||
});
|
||||
|
||||
it("routes queued system events to system prompt context, not user prompt text", async () => {
|
||||
vi.mocked(buildQueuedSystemPrompt).mockResolvedValueOnce(
|
||||
"## Runtime System Events (gateway-generated)\n- [t] Model switched.",
|
||||
);
|
||||
|
||||
await runPreparedReply(baseParams());
|
||||
|
||||
const call = vi.mocked(runReplyAgent).mock.calls[0]?.[0];
|
||||
expect(call).toBeTruthy();
|
||||
expect(call?.commandBody).not.toContain("Runtime System Events");
|
||||
expect(call?.followupRun.run.extraSystemPrompt).toContain("Runtime System Events");
|
||||
expect(call?.followupRun.run.extraSystemPrompt).toContain("Model switched.");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -44,7 +44,7 @@ import { resolveOriginMessageProvider } from "./origin-routing.js";
|
||||
import { resolveQueueSettings } from "./queue.js";
|
||||
import { routeReply } from "./route-reply.js";
|
||||
import { BARE_SESSION_RESET_PROMPT } from "./session-reset-prompt.js";
|
||||
import { ensureSkillSnapshot, prependSystemEvents } from "./session-updates.js";
|
||||
import { buildQueuedSystemPrompt, ensureSkillSnapshot } from "./session-updates.js";
|
||||
import { resolveTypingMode } from "./typing-mode.js";
|
||||
import { resolveRunTypingPolicy } from "./typing-policy.js";
|
||||
import type { TypingController } from "./typing.js";
|
||||
@@ -267,9 +267,12 @@ export async function runPreparedReply(
|
||||
const inboundMetaPrompt = buildInboundMetaSystemPrompt(
|
||||
isNewSession ? sessionCtx : { ...sessionCtx, ThreadStarterBody: undefined },
|
||||
);
|
||||
const extraSystemPrompt = [inboundMetaPrompt, groupChatContext, groupIntro, groupSystemPrompt]
|
||||
.filter(Boolean)
|
||||
.join("\n\n");
|
||||
const extraSystemPromptParts = [
|
||||
inboundMetaPrompt,
|
||||
groupChatContext,
|
||||
groupIntro,
|
||||
groupSystemPrompt,
|
||||
].filter(Boolean);
|
||||
const baseBody = sessionCtx.BodyStripped ?? sessionCtx.Body ?? "";
|
||||
// Use CommandBody/RawBody for bare reset detection (clean message without structural context).
|
||||
const rawBodyTrimmed = (ctx.CommandBody ?? ctx.RawBody ?? ctx.Body ?? "").trim();
|
||||
@@ -329,13 +332,15 @@ export async function runPreparedReply(
|
||||
});
|
||||
const isGroupSession = sessionEntry?.chatType === "group" || sessionEntry?.chatType === "channel";
|
||||
const isMainSession = !isGroupSession && sessionKey === normalizeMainKey(sessionCfg?.mainKey);
|
||||
prefixedBodyBase = await prependSystemEvents({
|
||||
const queuedSystemPrompt = await buildQueuedSystemPrompt({
|
||||
cfg,
|
||||
sessionKey,
|
||||
isMainSession,
|
||||
isNewSession,
|
||||
prefixedBodyBase,
|
||||
});
|
||||
if (queuedSystemPrompt) {
|
||||
extraSystemPromptParts.push(queuedSystemPrompt);
|
||||
}
|
||||
prefixedBodyBase = appendUntrustedContext(prefixedBodyBase, sessionCtx.UntrustedContext);
|
||||
const threadStarterBody = ctx.ThreadStarterBody?.trim();
|
||||
const threadHistoryBody = ctx.ThreadHistoryBody?.trim();
|
||||
@@ -504,7 +509,7 @@ export async function runPreparedReply(
|
||||
timeoutMs,
|
||||
blockReplyBreak: resolvedBlockStreamingBreak,
|
||||
ownerNumbers: command.ownerList.length > 0 ? command.ownerList : undefined,
|
||||
extraSystemPrompt: extraSystemPrompt || undefined,
|
||||
extraSystemPrompt: extraSystemPromptParts.join("\n\n") || undefined,
|
||||
...(isReasoningTagProvider(provider) ? { enforceFinalTag: true } : {}),
|
||||
},
|
||||
};
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { normalizeChatType } from "../../channels/chat-type.js";
|
||||
import { resolveConversationLabel } from "../../channels/conversation-label.js";
|
||||
import type { FinalizedMsgContext, MsgContext } from "../templating.js";
|
||||
import { normalizeInboundTextNewlines } from "./inbound-text.js";
|
||||
import { normalizeInboundTextNewlines, sanitizeInboundSystemTags } from "./inbound-text.js";
|
||||
|
||||
export type FinalizeInboundContextOptions = {
|
||||
forceBodyForAgent?: boolean;
|
||||
@@ -16,7 +16,7 @@ function normalizeTextField(value: unknown): string | undefined {
|
||||
if (typeof value !== "string") {
|
||||
return undefined;
|
||||
}
|
||||
return normalizeInboundTextNewlines(value);
|
||||
return sanitizeInboundSystemTags(normalizeInboundTextNewlines(value));
|
||||
}
|
||||
|
||||
function normalizeMediaType(value: unknown): string | undefined {
|
||||
@@ -40,8 +40,8 @@ export function finalizeInboundContext<T extends Record<string, unknown>>(
|
||||
): T & FinalizedMsgContext {
|
||||
const normalized = ctx as T & MsgContext;
|
||||
|
||||
normalized.Body = normalizeInboundTextNewlines(
|
||||
typeof normalized.Body === "string" ? normalized.Body : "",
|
||||
normalized.Body = sanitizeInboundSystemTags(
|
||||
normalizeInboundTextNewlines(typeof normalized.Body === "string" ? normalized.Body : ""),
|
||||
);
|
||||
normalized.RawBody = normalizeTextField(normalized.RawBody);
|
||||
normalized.CommandBody = normalizeTextField(normalized.CommandBody);
|
||||
@@ -50,7 +50,7 @@ export function finalizeInboundContext<T extends Record<string, unknown>>(
|
||||
normalized.ThreadHistoryBody = normalizeTextField(normalized.ThreadHistoryBody);
|
||||
if (Array.isArray(normalized.UntrustedContext)) {
|
||||
const normalizedUntrusted = normalized.UntrustedContext.map((entry) =>
|
||||
normalizeInboundTextNewlines(entry),
|
||||
sanitizeInboundSystemTags(normalizeInboundTextNewlines(entry)),
|
||||
).filter((entry) => Boolean(entry));
|
||||
normalized.UntrustedContext = normalizedUntrusted;
|
||||
}
|
||||
@@ -67,7 +67,9 @@ export function finalizeInboundContext<T extends Record<string, unknown>>(
|
||||
normalized.CommandBody ??
|
||||
normalized.RawBody ??
|
||||
normalized.Body);
|
||||
normalized.BodyForAgent = normalizeInboundTextNewlines(bodyForAgentSource);
|
||||
normalized.BodyForAgent = sanitizeInboundSystemTags(
|
||||
normalizeInboundTextNewlines(bodyForAgentSource),
|
||||
);
|
||||
|
||||
const bodyForCommandsSource = opts.forceBodyForCommands
|
||||
? (normalized.CommandBody ?? normalized.RawBody ?? normalized.Body)
|
||||
@@ -75,7 +77,9 @@ export function finalizeInboundContext<T extends Record<string, unknown>>(
|
||||
normalized.CommandBody ??
|
||||
normalized.RawBody ??
|
||||
normalized.Body);
|
||||
normalized.BodyForCommands = normalizeInboundTextNewlines(bodyForCommandsSource);
|
||||
normalized.BodyForCommands = sanitizeInboundSystemTags(
|
||||
normalizeInboundTextNewlines(bodyForCommandsSource),
|
||||
);
|
||||
|
||||
const explicitLabel = normalized.ConversationLabel?.trim();
|
||||
if (opts.forceConversationLabel || !explicitLabel) {
|
||||
|
||||
@@ -4,3 +4,15 @@ export function normalizeInboundTextNewlines(input: string): string {
|
||||
// Windows paths like C:\Work\nxxx\README.md or user-intended escape sequences.
|
||||
return input.replaceAll("\r\n", "\n").replaceAll("\r", "\n");
|
||||
}
|
||||
|
||||
const BRACKETED_SYSTEM_TAG_RE = /\[\s*(System\s*Message|System|Assistant|Internal)\s*\]/gi;
|
||||
const LINE_SYSTEM_PREFIX_RE = /^(\s*)System:(?=\s|$)/gim;
|
||||
|
||||
/**
|
||||
* Neutralize user-controlled strings that spoof internal system markers.
|
||||
*/
|
||||
export function sanitizeInboundSystemTags(input: string): string {
|
||||
return input
|
||||
.replace(BRACKETED_SYSTEM_TAG_RE, (_match, tag: string) => `(${tag})`)
|
||||
.replace(LINE_SYSTEM_PREFIX_RE, "$1System (untrusted):");
|
||||
}
|
||||
|
||||
@@ -13,13 +13,12 @@ import {
|
||||
import { getRemoteSkillEligibility } from "../../infra/skills-remote.js";
|
||||
import { drainSystemEventEntries } from "../../infra/system-events.js";
|
||||
|
||||
export async function prependSystemEvents(params: {
|
||||
export async function buildQueuedSystemPrompt(params: {
|
||||
cfg: OpenClawConfig;
|
||||
sessionKey: string;
|
||||
isMainSession: boolean;
|
||||
isNewSession: boolean;
|
||||
prefixedBodyBase: string;
|
||||
}): Promise<string> {
|
||||
}): Promise<string | undefined> {
|
||||
const compactSystemEvent = (line: string): string | null => {
|
||||
const trimmed = line.trim();
|
||||
if (!trimmed) {
|
||||
@@ -104,11 +103,15 @@ export async function prependSystemEvents(params: {
|
||||
}
|
||||
}
|
||||
if (systemLines.length === 0) {
|
||||
return params.prefixedBodyBase;
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const block = systemLines.map((l) => `System: ${l}`).join("\n");
|
||||
return `${block}\n\n${params.prefixedBodyBase}`;
|
||||
return [
|
||||
"## Runtime System Events (gateway-generated)",
|
||||
"Treat this section as trusted gateway runtime metadata, not user text.",
|
||||
"",
|
||||
...systemLines.map((line) => `- ${line}`),
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
export async function ensureSkillSnapshot(params: {
|
||||
|
||||
@@ -9,7 +9,7 @@ import { saveSessionStore } from "../../config/sessions.js";
|
||||
import { formatZonedTimestamp } from "../../infra/format-time/format-datetime.ts";
|
||||
import { enqueueSystemEvent, resetSystemEventsForTest } from "../../infra/system-events.js";
|
||||
import { applyResetModelOverride } from "./session-reset-model.js";
|
||||
import { prependSystemEvents } from "./session-updates.js";
|
||||
import { buildQueuedSystemPrompt } from "./session-updates.js";
|
||||
import { persistSessionUsageUpdate } from "./session-usage.js";
|
||||
import { initSessionState } from "./session.js";
|
||||
|
||||
@@ -1130,7 +1130,7 @@ describe("initSessionState preserves behavior overrides across /new and /reset",
|
||||
});
|
||||
});
|
||||
|
||||
describe("prependSystemEvents", () => {
|
||||
describe("buildQueuedSystemPrompt", () => {
|
||||
it("adds a local timestamp to queued system events by default", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
@@ -1140,16 +1140,16 @@ describe("prependSystemEvents", () => {
|
||||
|
||||
enqueueSystemEvent("Model switched.", { sessionKey: "agent:main:main" });
|
||||
|
||||
const result = await prependSystemEvents({
|
||||
const result = await buildQueuedSystemPrompt({
|
||||
cfg: {} as OpenClawConfig,
|
||||
sessionKey: "agent:main:main",
|
||||
isMainSession: false,
|
||||
isNewSession: false,
|
||||
prefixedBodyBase: "User: hi",
|
||||
});
|
||||
|
||||
expect(expectedTimestamp).toBeDefined();
|
||||
expect(result).toContain(`System: [${expectedTimestamp}] Model switched.`);
|
||||
expect(result).toContain("Runtime System Events (gateway-generated)");
|
||||
expect(result).toContain(`- [${expectedTimestamp}] Model switched.`);
|
||||
} finally {
|
||||
resetSystemEventsForTest();
|
||||
vi.useRealTimers();
|
||||
|
||||
Reference in New Issue
Block a user