mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 23:00:43 +00:00
fix: wrap hook block messages with originator
This commit is contained in:
@@ -668,7 +668,10 @@ describe("runCliAgent reliability", () => {
|
||||
});
|
||||
|
||||
expect(result.payloads).toEqual([
|
||||
{ text: "The agent cannot read this message.", isError: true },
|
||||
{
|
||||
text: "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)",
|
||||
isError: true,
|
||||
},
|
||||
]);
|
||||
expect(result.meta.livenessState).toBe("blocked");
|
||||
expect(supervisorSpawnMock).not.toHaveBeenCalled();
|
||||
@@ -692,11 +695,13 @@ describe("runCliAgent reliability", () => {
|
||||
expect(hookRunner.runAgentEnd).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
success: false,
|
||||
error: "The agent cannot read this message.",
|
||||
error:
|
||||
"Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)",
|
||||
messages: expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
role: "user",
|
||||
content: "The agent cannot read this message.",
|
||||
content:
|
||||
"Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)",
|
||||
}),
|
||||
]),
|
||||
}),
|
||||
@@ -706,7 +711,9 @@ describe("runCliAgent reliability", () => {
|
||||
|
||||
const lines = fs.readFileSync(sessionFile, "utf-8").trim().split("\n");
|
||||
const blockedLine = JSON.parse(lines[lines.length - 1]);
|
||||
expect(blockedLine.message.content[0].text).toBe("The agent cannot read this message.");
|
||||
expect(blockedLine.message.content[0].text).toBe(
|
||||
"Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)",
|
||||
);
|
||||
expect(JSON.stringify(blockedLine)).not.toContain("secret prompt");
|
||||
expect(JSON.stringify(blockedLine)).not.toContain("matched secret prompt");
|
||||
expect(blockedLine.message.__openclaw.beforeAgentRunBlocked).toMatchObject({
|
||||
|
||||
@@ -4,7 +4,7 @@ import { SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js";
|
||||
import { formatErrorMessage } from "../infra/errors.js";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { buildAgentHookContextChannelFields } from "../plugins/hook-agent-context.js";
|
||||
import { DEFAULT_BLOCK_MESSAGE, resolveBlockMessage } from "../plugins/hook-decision-types.js";
|
||||
import { resolveBlockMessage } from "../plugins/hook-decision-types.js";
|
||||
import { getGlobalHookRunner } from "../plugins/hook-runner-global.js";
|
||||
import { loadCliSessionHistoryMessages } from "./cli-runner/session-history.js";
|
||||
import type { PreparedCliRunContext, RunCliAgentParams } from "./cli-runner/types.js";
|
||||
@@ -417,7 +417,10 @@ export async function runPreparedCliAgent(
|
||||
buildAgentHookContext(hookContext),
|
||||
);
|
||||
} catch (err) {
|
||||
const blockMessage = `${DEFAULT_BLOCK_MESSAGE} by before_agent_run`;
|
||||
const blockMessage = resolveBlockMessage(
|
||||
{ outcome: "block", reason: "before_agent_run hook failed" },
|
||||
{ blockedBy: "before_agent_run" },
|
||||
);
|
||||
await persistBlockedBeforeAgentRun({
|
||||
message: blockMessage,
|
||||
pluginId: "before_agent_run",
|
||||
|
||||
@@ -155,7 +155,12 @@ describe("normalizeMessagesForLlmBoundary", () => {
|
||||
const input = [
|
||||
{
|
||||
role: "user",
|
||||
content: [{ type: "text", text: "The agent cannot read this message." }],
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)",
|
||||
},
|
||||
],
|
||||
timestamp: 1,
|
||||
__openclaw: {
|
||||
beforeAgentRunBlocked: {
|
||||
@@ -171,7 +176,10 @@ describe("normalizeMessagesForLlmBoundary", () => {
|
||||
) as Array<Record<string, unknown>>;
|
||||
|
||||
expect(output[0]?.content).toEqual([
|
||||
{ type: "text", text: "The agent cannot read this message." },
|
||||
{
|
||||
type: "text",
|
||||
text: "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)",
|
||||
},
|
||||
]);
|
||||
expect(output[0]).toHaveProperty("__openclaw.beforeAgentRunBlocked");
|
||||
expect(output[0]).not.toHaveProperty("__openclaw.beforeAgentRunBlocked.reason");
|
||||
|
||||
@@ -25,10 +25,7 @@ import { MAX_IMAGE_BYTES } from "../../../media/constants.js";
|
||||
import { listRegisteredPluginAgentPromptGuidance } from "../../../plugins/command-registry-state.js";
|
||||
import { getCurrentPluginMetadataSnapshot } from "../../../plugins/current-plugin-metadata-snapshot.js";
|
||||
import { buildAgentHookContextChannelFields } from "../../../plugins/hook-agent-context.js";
|
||||
import {
|
||||
DEFAULT_BLOCK_MESSAGE,
|
||||
resolveBlockMessage,
|
||||
} from "../../../plugins/hook-decision-types.js";
|
||||
import { resolveBlockMessage } from "../../../plugins/hook-decision-types.js";
|
||||
import { getGlobalHookRunner } from "../../../plugins/hook-runner-global.js";
|
||||
import {
|
||||
extractModelCompat,
|
||||
@@ -2862,10 +2859,18 @@ export async function runEmbeddedAttempt(
|
||||
beforeAgentRunBlocked = true;
|
||||
beforeAgentRunBlockedBy = "before_agent_run";
|
||||
await persistBlockedBeforeAgentRun({
|
||||
message: `${DEFAULT_BLOCK_MESSAGE} by before_agent_run`,
|
||||
message: resolveBlockMessage(
|
||||
{ outcome: "block", reason: "before_agent_run hook failed" },
|
||||
{ blockedBy: "before_agent_run" },
|
||||
),
|
||||
pluginId: "before_agent_run",
|
||||
});
|
||||
promptError = new Error(`${DEFAULT_BLOCK_MESSAGE} by before_agent_run`);
|
||||
promptError = new Error(
|
||||
resolveBlockMessage(
|
||||
{ outcome: "block", reason: "before_agent_run hook failed" },
|
||||
{ blockedBy: "before_agent_run" },
|
||||
),
|
||||
);
|
||||
promptErrorSource = "hook:before_agent_run";
|
||||
skipPromptSubmission = true;
|
||||
}
|
||||
|
||||
@@ -1692,9 +1692,18 @@ describe("runReplyAgent claude-cli routing", () => {
|
||||
|
||||
it("does not leak hook-blocked CLI input in raw trace payloads", async () => {
|
||||
runCliAgentMock.mockResolvedValueOnce({
|
||||
payloads: [{ text: "The agent cannot read this message.", isError: true }],
|
||||
payloads: [
|
||||
{
|
||||
text: "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)",
|
||||
isError: true,
|
||||
},
|
||||
],
|
||||
meta: {
|
||||
error: { kind: "hook_block", message: "The agent cannot read this message." },
|
||||
error: {
|
||||
kind: "hook_block",
|
||||
message:
|
||||
"Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)",
|
||||
},
|
||||
agentMeta: {
|
||||
provider: "claude-cli",
|
||||
model: "opus-4.5",
|
||||
@@ -1773,7 +1782,9 @@ describe("runReplyAgent claude-cli routing", () => {
|
||||
const texts = Array.isArray(result)
|
||||
? result.map((payload) => payload.text ?? "").join("\n")
|
||||
: (result?.text ?? "");
|
||||
expect(texts).toContain("The agent cannot read this message.");
|
||||
expect(texts).toContain(
|
||||
"Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)",
|
||||
);
|
||||
expect(texts).not.toContain("secret hitl prompt");
|
||||
});
|
||||
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
BLOCK_MESSAGE_PREFIX,
|
||||
type HookDecision,
|
||||
type HookDecisionBlock,
|
||||
mergeHookDecisions,
|
||||
isHookDecision,
|
||||
DEFAULT_BLOCK_MESSAGE,
|
||||
resolveBlockMessage,
|
||||
} from "./hook-decision-types.js";
|
||||
|
||||
@@ -63,12 +63,19 @@ describe("HookDecision helpers", () => {
|
||||
reason: "policy",
|
||||
};
|
||||
|
||||
expect(resolveBlockMessage(explicit)).toBe("Please rephrase your request.");
|
||||
expect(resolveBlockMessage(fallback)).toBe(DEFAULT_BLOCK_MESSAGE);
|
||||
expect(resolveBlockMessage(fallback, { blockedBy: "policy-plugin" })).toBe(
|
||||
`${DEFAULT_BLOCK_MESSAGE} by policy-plugin`,
|
||||
expect(resolveBlockMessage(explicit)).toBe(
|
||||
`${BLOCK_MESSAGE_PREFIX}: Please rephrase your request.`,
|
||||
);
|
||||
expect(resolveBlockMessage(fallback)).toBe(`${BLOCK_MESSAGE_PREFIX}: blocked`);
|
||||
expect(resolveBlockMessage(fallback, { blockedBy: "policy-plugin" })).toBe(
|
||||
`${BLOCK_MESSAGE_PREFIX}: blocked by policy-plugin`,
|
||||
);
|
||||
expect(resolveBlockMessage(explicit, { blockedBy: "policy-plugin" })).toBe(
|
||||
`${BLOCK_MESSAGE_PREFIX}: Please rephrase your request. (blocked by policy-plugin)`,
|
||||
);
|
||||
expect(resolveBlockMessage({ ...explicit, message: " " })).toBe(
|
||||
`${BLOCK_MESSAGE_PREFIX}: blocked`,
|
||||
);
|
||||
expect(resolveBlockMessage({ ...explicit, message: " " })).toBe(DEFAULT_BLOCK_MESSAGE);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,8 +10,8 @@ export type HookDecisionPass = {
|
||||
outcome: "pass";
|
||||
};
|
||||
|
||||
/** Default user-facing replacement message when a `block` decision omits one. */
|
||||
export const DEFAULT_BLOCK_MESSAGE = "This request was blocked by policy";
|
||||
/** Prefix for user-facing replacement messages when a `block` decision stops a request. */
|
||||
export const BLOCK_MESSAGE_PREFIX = "Your message could not be sent";
|
||||
|
||||
/**
|
||||
* Content is blocked. `reason` is internal; `message` is user-facing.
|
||||
@@ -20,7 +20,7 @@ export type HookDecisionBlock = {
|
||||
outcome: "block";
|
||||
/** Internal reason for logging/observability. Never shown to user. */
|
||||
reason: string;
|
||||
/** Optional user-facing replacement text. Defaults to `DEFAULT_BLOCK_MESSAGE`. */
|
||||
/** Optional user-facing detail included in the block response envelope. */
|
||||
message?: string;
|
||||
/** Plugin-defined category for analytics (e.g. "violence", "pii", "cost_limit"). */
|
||||
category?: string;
|
||||
@@ -32,11 +32,16 @@ export function resolveBlockMessage(
|
||||
decision: HookDecisionBlock,
|
||||
params: { blockedBy?: string } = {},
|
||||
): string {
|
||||
if (typeof decision.message === "string" && decision.message.trim()) {
|
||||
return decision.message;
|
||||
}
|
||||
const message = typeof decision.message === "string" ? decision.message.trim() : "";
|
||||
const blockedBy = params.blockedBy?.trim();
|
||||
return blockedBy ? `${DEFAULT_BLOCK_MESSAGE} by ${blockedBy}` : DEFAULT_BLOCK_MESSAGE;
|
||||
if (message) {
|
||||
return blockedBy
|
||||
? `${BLOCK_MESSAGE_PREFIX}: ${message} (blocked by ${blockedBy})`
|
||||
: `${BLOCK_MESSAGE_PREFIX}: ${message}`;
|
||||
}
|
||||
return blockedBy
|
||||
? `${BLOCK_MESSAGE_PREFIX}: blocked by ${blockedBy}`
|
||||
: `${BLOCK_MESSAGE_PREFIX}: blocked`;
|
||||
}
|
||||
|
||||
/** Outcome severity for most-restrictive-wins merging. Higher = more restrictive. */
|
||||
|
||||
Reference in New Issue
Block a user