fix(agents): strip external-content nonce so loop detection blocks repeated wrapped failures (#130261)

* fix(agents): strip external-content nonce before loop-detection resultHash

External-content wrappers carry a fresh anti-forgery nonce per result
(createExternalContentMarkerId). The loop detector hashed the wrapper text
including the nonce, so identical failing wrapped tool calls never reached
the no-progress block threshold — the detector warned but never blocked,
leaving the agent to loop until it gave up (#130210).

Strip the wrapper nonce before hashing in extractTextContent, mirroring
the existing stripVolatileSendIds treatment of per-call send ids (#89090).
Only the hash is normalized; the delivered wrapper keeps its nonce, so
anti-forgery is unaffected.

Closes #130210

Co-Authored-By: Claude <noreply@anthropic.com>

* test(agents): validate protected error marker capture

Narrow the generated marker capture before recording it so the real network-error loop regression also passes its owning test-type graph.

---------

Co-authored-by: ruel225 <ruel225@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
ruel225
2026-08-27 12:22:39 +08:00
committed by GitHub
parent 5e193d50ad
commit cd321938f6
5 changed files with 306 additions and 20 deletions
+1
View File
@@ -315,6 +315,7 @@ Docs: https://docs.openclaw.ai
- **Control UI agent model labels:** show each selected agent's effective model in the Default picker option instead of the global model. (#100719, #77690, #77440) Thanks @hyspacex.
- **Control UI inbound image previews:** render canonical inbound media references through the authenticated ticket route after chat-history reloads. (#100725, #90172, #89591) Thanks @sweetcornna.
- **Small-context compaction:** cap the effective reserve against the known model context window so small local models do not enter compaction from the first token. (#100621) Thanks @vincentkoc.
- **Tool-loop detection:** recognize repeated browser and network failures despite fresh external-content wrapper nonces, while preserving meaningful progress and model-visible security markers. Fixes #130210. (#130261) Thanks @ruel225.
- **Detail-less provider failures:** keep opaque upstream failures from cooling API-key auth profiles while preserving WHAM-backed OpenAI OAuth health checks and configured model fallback. (#100600, #100617) Thanks @fengjikui.
- **Plugin install diagnostics:** suppress the misleading hook-pack fallback after plugin install failures only when the hook manifest is absent, while preserving actionable malformed hook-pack errors. (#100554) Thanks @vincentkoc.
- **Config validation diagnostics:** emit each unchanged sanitized validation-warning payload once per config path, reset deduplication after a clean validation, and preserve the warning fingerprint across transient invalid reads and failed refreshes. (#100569, #25574) Thanks @vincentkoc.
+12 -5
View File
@@ -76,15 +76,22 @@ For `exec`, no-progress hashing compares stable command outcomes (status,
exit code, timed-out flag, output) and ignores volatile runtime metadata such
as duration, PID, session ID, and working directory. Outbound message-send
results are hashed with volatile per-call ids (message id, file id, timestamp)
stripped, so a "sent" result does not look identical to a different "sent"
result. When a run id is available, history is evaluated only within that run,
stripped, so delivery IDs alone do not make repeated equivalent sends look like
progress. When a run id is available, history is evaluated only within that run,
so scheduled heartbeat cycles and fresh runs do not inherit stale loop counts
from earlier runs.
Outcome comparisons also ignore fresh external-content wrapper nonces, including
wrapped errors and JSON results. Delivered security markers remain unchanged;
payload text, status, timestamps, and durations still distinguish network results.
This is a syntactic comparison: literal or copied text matching the complete
wrapper format also ignores nonce-only changes. It does not authenticate content,
change authorization, or modify delivered tool results.
## Recommended setup
- For smaller models, set `enabled: true`. Flagship models rarely need rolling-history detection and can
leave the master switch `false` while still benefiting from the
leave the master switch unset while still benefiting from the
post-compaction guard.
- To disable everything, including the post-compaction guard, set
`tools.loopDetection.enabled: false` explicitly.
@@ -113,8 +120,8 @@ so a no-config user still gets the protection.
}
```
- The guard never aborts while results are changing; only byte-identical
results across the window trigger it.
- The guard compares normalized outcome hashes, not raw result bytes. Meaningful
changes keep it from aborting; fresh wrapper nonces alone do not count as progress.
- It only arms in the immediate aftermath of a compaction-retry, not at other
points in a run.
@@ -2,6 +2,7 @@ import { createRequire } from "node:module";
import { describe, expect, it, vi } from "vitest";
import { SecretSurfaceUnavailableError } from "../secrets/runtime-degraded-state.js";
import { wrapToolWithBeforeToolCallHook } from "./agent-tools.before-tool-call.wrapper.js";
import { createPostCompactionLoopGuard } from "./embedded-agent-runner/post-compaction-loop-guard.js";
import { resolveToolExecutionErrorKind } from "./tool-result-error.js";
import { ToolInputError, type AnyAgentTool } from "./tools/common.js";
@@ -32,6 +33,68 @@ function createFailingTool(params: {
}
describe("before-tool-call network execution error boundary", () => {
it.each([undefined, false])(
"preserves post-compaction protection when loop detection is %s",
async (enabled) => {
const guard = createPostCompactionLoopGuard({ enabled: enabled !== false });
guard.armPostCompaction();
const verdicts: boolean[] = [];
const source = createFailingTool({ error: new Error("same network failure"), network: true });
const tool = wrapToolWithBeforeToolCallHook(
source,
{
sessionKey: `network-compaction-${enabled}`,
runId: `network-compaction-${enabled}`,
...(enabled === undefined ? {} : { loopDetection: { enabled } }),
onToolOutcome: (outcome) => verdicts.push(guard.observe(outcome).shouldAbort),
},
{ emitDiagnostics: false },
);
for (let index = 0; index < 3; index += 1) {
await expect(tool.execute(`compaction-${enabled}-${index}`, {})).rejects.toBeInstanceOf(
Error,
);
}
expect(verdicts).toEqual([false, false, enabled !== false]);
},
);
it("blocks the next call after twenty identical protected network failures", async () => {
const original = new Error('keyboard.press: Unknown key: "NotAKey"');
const source = createFailingTool({ error: original, network: true });
const tool = wrapToolWithBeforeToolCallHook(
source,
{
sessionKey: "network-error-loop",
runId: "network-error-loop",
loopDetection: { enabled: true },
},
{ emitDiagnostics: false },
);
const markerIds = new Set<string>();
for (let index = 0; index < 20; index += 1) {
const failure = await tool
.execute(`network-loop-${index}`, {})
.catch((error: unknown) => error);
expect(failure).toBeInstanceOf(Error);
const matches = [
...(failure as Error).message.matchAll(/EXTERNAL_UNTRUSTED_CONTENT id="([a-f0-9]{16})"/g),
];
expect(matches).toHaveLength(2);
const markerId = matches[0]?.[1];
if (!markerId) {
throw new Error("Expected a generated external-content marker");
}
expect(markerId).toBe(matches[1]?.[1]);
markerIds.add(markerId);
}
expect(markerIds.size).toBe(20);
const result = await tool.execute("network-loop-blocked", {});
expect(result.details).toMatchObject({ status: "blocked", deniedReason: "tool-loop" });
expect(source.execute).toHaveBeenCalledTimes(20);
expect(original.message).toBe('keyboard.press: Unknown key: "NotAKey"');
});
it.each([
["host input", () => new ToolInputError("query required")],
[
+199
View File
@@ -3,6 +3,7 @@
import { describe, expect, it, vi } from "vitest";
import type { ToolLoopDetectionConfig } from "../config/types.tools.js";
import type { SessionState } from "../logging/diagnostic-session-state.js";
import { wrapExternalContent } from "../security/external-content.js";
// Recognize a provider-docked send tool by name (only "telegram" here) so the
// volatility strip applies to it without pulling in the channel-plugin registry; the
@@ -20,6 +21,8 @@ import {
recordToolCall,
recordToolCallOutcome,
} from "./tool-loop-detection.js";
import { protectNetworkToolExecutionError } from "./tool-result-error.js";
import { jsonResult } from "./tools/common.js";
const TOOL_CALL_HISTORY_SIZE = 30;
const WARNING_THRESHOLD = 10;
@@ -446,6 +449,202 @@ describe("tool-loop-detection", () => {
}
});
it.each(["thrown", "returned", 0, 1, 2, 3, 4] as const)(
"blocks repeated external outcomes with fresh nonces (%s)",
(shape) => {
const state = createState();
const params = { action: "act", request: { kind: "press", key: "NotAKey" } };
const delivered = new Set<string>();
for (let index = 0; index < CRITICAL_THRESHOLD; index += 1) {
const payload = 'keyboard.press: Unknown key: "NotAKey"';
if (shape === "thrown") {
const error = protectNetworkToolExecutionError(new Error(payload), "Failed");
delivered.add((error as Error).message);
recordFailedCall(state, "browser", params, error, index);
} else {
let text = wrapExternalContent(payload, { source: "browser" });
delivered.add(text);
if (typeof shape === "number") {
for (let depth = 0; depth < shape; depth += 1) {
text = JSON.stringify({ text });
}
}
const result =
shape === "returned"
? { content: [{ type: "text", text }], details: { ok: false } }
: jsonResult({ text, fetchedAt: "2026-08-26T00:00:00Z", tookMs: 10 });
recordSuccessfulCall(state, "browser", params, result, index);
}
}
expect(delivered.size).toBe(CRITICAL_THRESHOLD);
expect(
detectToolCallLoop(state, "browser", params, enabledLoopDetectionConfig),
).toMatchObject({
stuck: true,
level: "critical",
detector: "generic_repeat",
});
const [first, second] = [...delivered];
expect(recordArgsHash("browser", { text: first })).not.toBe(
recordArgsHash("browser", { text: second }),
);
},
);
it.each([
["payload", (index: number) => ({ text: `page ${index}` })],
["status", (index: number) => ({ status: index === 0 ? "loading" : "ready" })],
["timestamp", (index: number) => ({ fetchedAt: `2026-08-26T00:00:0${index}Z` })],
["duration", (index: number) => ({ tookMs: index })],
["ordinary id", (index: number) => ({ id: `${index}`.repeat(16) })],
[
"malformed marker",
(index: number) => ({ text: `<<<EXTERNAL_UNTRUSTED_CONTENT id="${index}">>>` }),
],
[
"uppercase nonce",
(index: number) => ({
text: `<<<EXTERNAL_UNTRUSTED_CONTENT id="${index}ABCDEF012345678">>>`,
}),
],
["unrelated id text", (index: number) => ({ text: `id="${index.toString().repeat(16)}"` })],
[
"unpaired start marker",
(index: number) => ({
text: `<<<EXTERNAL_UNTRUSTED_CONTENT id="${index.toString().repeat(16)}">>>`,
}),
],
[
"unpaired end marker",
(index: number) => ({
text: `<<<END_EXTERNAL_UNTRUSTED_CONTENT id="${index.toString().repeat(16)}">>>`,
}),
],
[
"mismatched marker pair",
(index: number) => ({
text: `<<<EXTERNAL_UNTRUSTED_CONTENT id="${index.toString().repeat(16)}">>>payload<<<END_EXTERNAL_UNTRUSTED_CONTENT id="abcdef0123456789">>>`,
}),
],
[
"intervening marker",
(index: number) => ({
text: `<<<EXTERNAL_UNTRUSTED_CONTENT id="${index.toString().repeat(16)}">>>before<<<EXTERNAL_UNTRUSTED_CONTENT>>>after<<<END_EXTERNAL_UNTRUSTED_CONTENT id="${index.toString().repeat(16)}">>>`,
}),
],
[
"markers split across fields",
(index: number) => ({
start: `<<<EXTERNAL_UNTRUSTED_CONTENT id="${index.toString().repeat(16)}">>>`,
end: `<<<END_EXTERNAL_UNTRUSTED_CONTENT id="${index.toString().repeat(16)}">>>`,
}),
],
] as const)("retains changing %s in outcome hashes", (_label, details) => {
const hashes = [0, 1].map((index) => {
const state = createState();
return recordToolCallOutcome(state, {
toolName: "web_fetch",
toolParams: { url: "https://example.test" },
result: jsonResult({
content: wrapExternalContent("same page", { source: "web_fetch" }),
...details(index),
}),
})?.resultHash;
});
expect(hashes[0]).not.toBe(hashes[1]);
});
it.each([0, 1, 2, 3, 4])(
"preserves marker pairs split across encoded JSON fields (depth %s)",
(depth) => {
const hashes = [0, 1].map((index) => {
const id = index.toString().repeat(16);
let text = JSON.stringify({
start: `<<<EXTERNAL_UNTRUSTED_CONTENT id="${id}">>>\nSource: Browser\n---\npayload`,
end: `\n<<<END_EXTERNAL_UNTRUSTED_CONTENT id="${id}">>>`,
});
for (let level = 0; level < depth; level += 1) {
text = JSON.stringify({ text });
}
return recordToolCallOutcome(createState(), {
toolName: "browser",
toolParams: {},
result: jsonResult({ text }),
})?.resultHash;
});
expect(hashes[0]).not.toBe(hashes[1]);
},
);
it("preserves long backslash payloads while normalizing paired wrapper nonces", () => {
const hashes = ["same", "same", "changed"].map((suffix) => {
const text = wrapExternalContent(`${"\\".repeat(20_000)}"${suffix}"`, {
source: "browser",
});
return recordToolCallOutcome(createState(), {
toolName: "browser",
toolParams: {},
result: jsonResult({ text: JSON.stringify({ text }) }),
})?.resultHash;
});
expect(hashes[0]).toBe(hashes[1]);
expect(hashes[0]).not.toBe(hashes[2]);
});
it.each([0, 1, 2, 3, 4])("distinguishes malformed empty-ID envelopes (depth %s)", (depth) => {
const valid = wrapExternalContent("same page", { source: "browser" });
const malformed = valid.replace(/id="[a-f0-9]{16}"/g, 'id=""');
const zero = valid.replace(/id="[a-f0-9]{16}"/g, 'id="0000000000000000"');
const hashes = [valid, malformed, zero].map((sourceText) => {
let text = sourceText;
for (let level = 0; level < depth; level += 1) {
text = JSON.stringify({ text });
}
return recordToolCallOutcome(createState(), {
toolName: "read",
toolParams: {},
result: jsonResult({ text }),
})?.resultHash;
});
expect(hashes[0]).not.toBe(hashes[1]);
expect(hashes[0]).toBe(hashes[2]);
});
it.each([2, 4, 5, 6, 8, 14])(
"preserves malformed marker quote escaping (%s slashes)",
(count) => {
const hashes = [0, 1].map((index) => {
const id = index.toString().repeat(16);
const quote = "\\".repeat(count) + '"';
const text = `<<<EXTERNAL_UNTRUSTED_CONTENT id=${quote}${id}${quote}>>>payload<<<END_EXTERNAL_UNTRUSTED_CONTENT id=${quote}${id}${quote}>>>`;
return recordToolCallOutcome(createState(), {
toolName: "read",
toolParams: {},
result: jsonResult({ text }),
})?.resultHash;
});
expect(hashes[0]).not.toBe(hashes[1]);
},
);
it.each([1, 3, 7])(
"preserves shallower quotes after encoded backslashes (%s marker slashes)",
(escapeCount) => {
const hashes = [0, 1].map((index) => {
const id = index.toString().repeat(16);
const quote = "\\".repeat(escapeCount) + '"';
const boundary = "\\".repeat(escapeCount + 1) + '"';
const text = `<<<EXTERNAL_UNTRUSTED_CONTENT id=${quote}${id}${quote}>>>before${boundary}after<<<END_EXTERNAL_UNTRUSTED_CONTENT id=${quote}${id}${quote}>>>`;
return recordToolCallOutcome(createState(), {
toolName: "read",
toolParams: {},
result: jsonResult({ text }),
})?.resultHash;
});
expect(hashes[0]).not.toBe(hashes[1]);
},
);
it("warns for known polling no-progress loops", () => {
const { params, result } = createNoProgressPollFixture("sess-1");
const loopResult = detectLoopAfterRepeatedCalls({
+31 -15
View File
@@ -3,13 +3,13 @@
*
* Watches recent tool history for repeated no-progress patterns and circuit-breaker thresholds.
*/
import { createHash } from "node:crypto";
import { stableStringify } from "@openclaw/normalization-core";
import {
normalizeNullableString as nonEmptyStringField,
normalizeOptionalString as normalizeRunId,
} from "@openclaw/normalization-core/string-coerce";
import type { ToolLoopDetectionConfig } from "../config/types.tools.js";
import { sha256Hex } from "../infra/crypto-digest.js";
import type { SessionState, ToolCallRecord } from "../logging/diagnostic-session-state.js";
import { createSubsystemLogger } from "../logging/subsystem.js";
import { isPlainObject } from "../utils.js";
@@ -107,12 +107,28 @@ function resolveLoopDetectionConfig(config?: ToolLoopDetectionConfig): ResolvedL
* Uses tool name + deterministic JSON serialization digest of params.
*/
export function hashToolCall(toolName: string, params: unknown): string {
return `${toolName}:${digestStable(params)}`;
return `${toolName}:${sha256Hex(stableStringify(params))}`;
}
function digestStable(value: unknown): string {
const serialized = stableStringify(value);
return createHash("sha256").update(serialized).digest("hex");
function digestToolOutcome(value: unknown): string {
// Canonical IDs retain valid envelope syntax; malformed markers and JSON field
// boundaries remain meaningful. Literal/copied envelopes share this syntax rule;
// it grants no trust and never changes arguments or delivered content.
const canonicalMarkerId = "0000000000000000";
const serialized = stableStringify(value, (text) =>
text.replace(
/(<<<EXTERNAL_UNTRUSTED_CONTENT id=(\\*)")([a-f0-9]{16})(\2">>>(?:(?!<<<(?:END_)?EXTERNAL_UNTRUSTED_CONTENT)[\s\S])*<<<END_EXTERNAL_UNTRUSTED_CONTENT id=\2")\3(\2">>>)/g,
// Repeated JSON encoding produces 2^n - 1 backslashes before marker quotes.
(match, start: string, escapes: string, _id: string, middle: string, end: string) =>
(escapes.length & (escapes.length + 1)) !== 0 ||
[...middle.matchAll(/(?<!\\)\\*"/g)].some(
(quote) => quote[0].length % (escapes.length + 1) !== 0,
)
? match
: start + canonicalMarkerId + middle + canonicalMarkerId + end,
),
);
return sha256Hex(serialized);
}
function extractTextContent(result: unknown): string {
@@ -165,14 +181,14 @@ function hashExecToolOutcome(details: Record<string, unknown>, text: string): st
}
if (status === "running") {
return digestStable({
return digestToolOutcome({
status,
tail: stringField(details.tail) ?? "",
});
}
if (status === "completed" || status === "failed") {
return digestStable({
return digestToolOutcome({
status,
exitCode: typeof details.exitCode === "number" ? details.exitCode : null,
timedOut: details.timedOut === true,
@@ -181,7 +197,7 @@ function hashExecToolOutcome(details: Record<string, unknown>, text: string): st
}
if (status === "approval-pending" || status === "approval-unavailable") {
return digestStable({
return digestToolOutcome({
status,
reason: stringField(details.reason),
host: stringField(details.host),
@@ -291,13 +307,13 @@ function hashToolOutcome(
if (error !== undefined) {
const unknownToolName = extractUnknownToolName(error);
return {
resultHash: `error:${digestStable(formatErrorForHash(error))}`,
resultHash: `error:${digestToolOutcome(formatErrorForHash(error))}`,
noProgress: true,
unknownToolName,
};
}
if (!isPlainObject(result)) {
return { resultHash: result === undefined ? undefined : digestStable(result) };
return { resultHash: result === undefined ? undefined : digestToolOutcome(result) };
}
const details = isPlainObject(result.details) ? result.details : {};
@@ -327,13 +343,13 @@ function hashToolOutcome(
}
}
if (toolName === "write" && isWriteNoProgressOutcome(details)) {
return { resultHash: digestStable({ status: "unchanged" }), noProgress: true };
return { resultHash: digestToolOutcome({ status: "unchanged" }), noProgress: true };
}
if (isKnownPollToolCall(toolName, params) && toolName === "process" && isPlainObject(params)) {
const action = params.action;
if (action === "poll") {
return {
resultHash: digestStable({
resultHash: digestToolOutcome({
action,
status: details.status,
exitCode: details.exitCode ?? null,
@@ -345,7 +361,7 @@ function hashToolOutcome(
}
if (action === "log") {
return {
resultHash: digestStable({
resultHash: digestToolOutcome({
action,
status: details.status,
totalLines: details.totalLines ?? null,
@@ -360,11 +376,11 @@ function hashToolOutcome(
}
if (isVolatileSendResult(toolName, params)) {
return { resultHash: digestStable(stripVolatileSendIds(details)) };
return { resultHash: digestToolOutcome(stripVolatileSendIds(details)) };
}
return {
resultHash: digestStable({
resultHash: digestToolOutcome({
details,
text,
}),