vultr/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-05-06 15:00:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: keep hook block reasons internal

Browse Source
This commit is contained in:
jesse-merhi
2026-05-06 14:38:39 +10:00
committed by clawsweeper
parent 5f10007b7a
commit 04d4c5caff
13 changed files with 130 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
extensions/diagnostics-otel/src/service.test.ts
View File

@@ -646,6 +646,34 @@ describe("diagnostics-otel service", () => {
await service.stop?.(ctx);
});
test("records hook-blocked run metrics with safe blocker originator", async () => {
const service = createDiagnosticsOtelService();
const ctx = createOtelContext(OTEL_TEST_ENDPOINT, { traces: true, metrics: true });
await service.start(ctx);
emitDiagnosticEvent({
type: "run.completed",
runId: "run-1",
provider: "openai",
model: "gpt-5.4",
outcome: "blocked",
blockedBy: "policy-plugin",
durationMs: 100,
});
await flushDiagnosticEvents();
expect(telemetryState.histograms.get("openclaw.run.duration_ms")?.record).toHaveBeenCalledWith(
100,
expect.objectContaining({
"openclaw.outcome": "blocked",
"openclaw.blocked_by": "policy-plugin",
}),
);
expect(JSON.stringify(telemetryState)).not.toContain("matched secret prompt");
await service.stop?.(ctx);
});
test("honors disabled traces when an OpenTelemetry SDK is preloaded", async () => {
process.env.OPENCLAW_OTEL_PRELOADED = "1";
const service = createDiagnosticsOtelService();

6
extensions/diagnostics-otel/src/service.ts
View File

@@ -1665,6 +1665,9 @@ export function createDiagnosticsOtelService(): OpenClawPluginService {
if (evt.channel) {
attrs["openclaw.channel"] = evt.channel;
}
if (evt.blockedBy) {
attrs["openclaw.blocked_by"] = lowCardinalityAttr(evt.blockedBy, "unknown");
}
durationHistogram.record(evt.durationMs, attrs);
if (!tracesEnabled) {
return;
@@ -1673,6 +1676,9 @@ export function createDiagnosticsOtelService(): OpenClawPluginService {
"openclaw.outcome": evt.outcome,
};
addRunAttrs(spanAttrs, evt);
if (evt.blockedBy) {
spanAttrs["openclaw.blocked_by"] = lowCardinalityAttr(evt.blockedBy, "unknown");
}
if (evt.errorCategory) {
spanAttrs["openclaw.errorCategory"] = lowCardinalityAttr(evt.errorCategory, "other");
}

31
extensions/diagnostics-prometheus/src/service.test.ts
View File

@@ -43,6 +43,37 @@ describe("diagnostics-prometheus service", () => {
expect(rendered).not.toContain("session-should-not-export");
});
it("records hook-blocked run metrics with safe blocker originator only", () => {
const store = __test__.createPrometheusMetricStore();
__test__.recordDiagnosticEvent(
store,
{
...baseEvent(),
type: "run.completed",
runId: "run-should-not-export",
sessionKey: "session-should-not-export",
provider: "openai",
model: "gpt-5.4",
channel: "slack",
trigger: "message",
durationMs: 250,
outcome: "blocked",
blockedBy: "policy-plugin",
},
trusted,
);
const rendered = __test__.renderPrometheusMetrics(store);
expect(rendered).toContain(
'openclaw_run_completed_total{blocked_by="policy-plugin",channel="slack",model="gpt-5.4",outcome="blocked",provider="openai",trigger="message"} 1',
);
expect(rendered).not.toContain("run-should-not-export");
expect(rendered).not.toContain("session-should-not-export");
expect(rendered).not.toContain("matched secret prompt");
});
it("drops untrusted plugin-emitted diagnostic events", () => {
const store = __test__.createPrometheusMetricStore();

2
extensions/diagnostics-prometheus/src/service.ts
View File

@@ -276,6 +276,7 @@ function renderPrometheusMetrics(store: PrometheusMetricStore): string {
}
function runLabels(evt: {
blockedBy?: string;
channel?: string;
model?: string;
outcome?: string;
@@ -283,6 +284,7 @@ function runLabels(evt: {
trigger?: string;
}): LabelSet {
return {
...(evt.blockedBy ? { blocked_by: lowCardinalityLabel(evt.blockedBy) } : {}),
channel: lowCardinalityLabel(evt.channel),
model: lowCardinalityLabel(evt.model),
outcome: lowCardinalityLabel(evt.outcome, "unknown"),

5
src/agents/cli-runner.reliability.test.ts
View File

@@ -642,7 +642,7 @@ describe("runCliAgent reliability", () => {
pluginId: "policy-plugin",
decision: {
outcome: "block" as const,
reason: "contains protected content",
reason: "matched secret prompt: secret prompt",
message: "The agent cannot read this message.",
},
})),
@@ -708,10 +708,11 @@ describe("runCliAgent reliability", () => {
const blockedLine = JSON.parse(lines[lines.length - 1]);
expect(blockedLine.message.content[0].text).toBe("The agent cannot read this message.");
expect(JSON.stringify(blockedLine)).not.toContain("secret prompt");
expect(JSON.stringify(blockedLine)).not.toContain("matched secret prompt");
expect(blockedLine.message.__openclaw.beforeAgentRunBlocked).toMatchObject({
blockedBy: "policy-plugin",
reason: "contains protected content",
});
expect(blockedLine.message.__openclaw.beforeAgentRunBlocked).not.toHaveProperty("reason");
expect(Object.hasOwn(blockedLine.message.__openclaw, "beforeAgentRunBlocked")).toBe(true);
} finally {
fs.rmSync(dir, { recursive: true, force: true });

12
src/agents/cli-runner.ts
View File

@@ -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 { resolveBlockMessage } from "../plugins/hook-decision-types.js";
import { DEFAULT_BLOCK_MESSAGE, 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";
@@ -242,7 +242,6 @@ export async function runPreparedCliAgent(
const persistBlockedBeforeAgentRun = async (block: {
message: string;
pluginId: string;
reason: string;
}): Promise<void> => {
try {
const nowMs = Date.now();
@@ -255,7 +254,6 @@ export async function runPreparedCliAgent(
__openclaw: {
beforeAgentRunBlocked: {
blockedBy: block.pluginId,
reason: block.reason,
blockedAt: nowMs,
},
},
@@ -419,11 +417,10 @@ export async function runPreparedCliAgent(
buildAgentHookContext(hookContext),
);
} catch (err) {
const blockMessage = "Request blocked by before_agent_run policy.";
const blockMessage = `${DEFAULT_BLOCK_MESSAGE} by before_agent_run`;
await persistBlockedBeforeAgentRun({
message: blockMessage,
pluginId: "before_agent_run",
reason: `before_agent_run hook failed closed: ${formatErrorMessage(err)}`,
});
runAgentHarnessAgentEndHook({
event: buildBlockedAgentEndEvent(blockMessage),
@@ -435,11 +432,12 @@ export async function runPreparedCliAgent(
const beforeRunDecision = beforeRunResult?.decision;
if (beforeRunDecision?.outcome === "block") {
const blockMessage = resolveBlockMessage(beforeRunDecision);
const blockMessage = resolveBlockMessage(beforeRunDecision, {
blockedBy: beforeRunResult?.pluginId ?? "unknown",
});
await persistBlockedBeforeAgentRun({
message: blockMessage,
pluginId: beforeRunResult?.pluginId ?? "unknown",
reason: beforeRunDecision.reason,
});
runAgentHarnessAgentEndHook({
event: buildBlockedAgentEndEvent(blockMessage),

3
src/agents/pi-embedded-runner/run/attempt.test.ts
View File

@@ -160,7 +160,6 @@ describe("normalizeMessagesForLlmBoundary", () => {
__openclaw: {
beforeAgentRunBlocked: {
blockedBy: "policy-plugin",
reason: "contains protected content",
blockedAt: 1,
},
},
@@ -175,7 +174,9 @@ describe("normalizeMessagesForLlmBoundary", () => {
{ type: "text", text: "The agent cannot read this message." },
]);
expect(output[0]).toHaveProperty("__openclaw.beforeAgentRunBlocked");
expect(output[0]).not.toHaveProperty("__openclaw.beforeAgentRunBlocked.reason");
expect(JSON.stringify(output)).not.toContain("secret prompt");
expect(JSON.stringify(output)).not.toContain("matched secret prompt");
expect(input[0]).toHaveProperty("__openclaw");
});
});

49
src/agents/pi-embedded-runner/run/attempt.ts
View File

@@ -25,7 +25,10 @@ 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 { resolveBlockMessage } from "../../../plugins/hook-decision-types.js";
import {
DEFAULT_BLOCK_MESSAGE,
resolveBlockMessage,
} from "../../../plugins/hook-decision-types.js";
import { getGlobalHookRunner } from "../../../plugins/hook-runner-global.js";
import {
extractModelCompat,
@@ -736,8 +739,14 @@ export async function runEmbeddedAttempt(
let timedOutDuringToolExecution = false;
let promptError: unknown = null;
let emitDiagnosticRunCompleted:
| ((outcome: "completed" | "aborted" | "error", err?: unknown) => void)
| ((
outcome: "completed" | "aborted" | "blocked" | "error",
err?: unknown,
extra?: { blockedBy?: string },
) => void)
| undefined;
let beforeAgentRunBlocked = false;
let beforeAgentRunBlockedBy: string | undefined;
try {
const skillsSnapshotForRun =
sandbox?.enabled && sandbox.workspaceAccess !== "rw" ? undefined : params.skillsSnapshot;
@@ -800,7 +809,7 @@ export async function runEmbeddedAttempt(
});
const diagnosticRunStartedAt = Date.now();
let diagnosticRunCompleted = false;
emitDiagnosticRunCompleted = (outcome, err) => {
emitDiagnosticRunCompleted = (outcome, err, extra) => {
if (diagnosticRunCompleted) {
return;
}
@@ -810,7 +819,8 @@ export async function runEmbeddedAttempt(
...diagnosticRunBase,
durationMs: Date.now() - diagnosticRunStartedAt,
outcome,
...(err ? { errorCategory: diagnosticErrorCategory(err) } : {}),
...(extra?.blockedBy ? { blockedBy: extra.blockedBy } : {}),
...(err && outcome !== "blocked" ? { errorCategory: diagnosticErrorCategory(err) } : {}),
});
};
const corePluginToolStages = createEmbeddedRunStageTracker();
@@ -2791,7 +2801,6 @@ export async function runEmbeddedAttempt(
const persistBlockedBeforeAgentRun = async (block: {
message: string;
pluginId: string;
reason: string;
}): Promise<boolean> => {
const idempotencyKey = `hook-block:before_agent_run:user:${params.runId}`;
if (sessionMessagesContainIdempotencyKey(activeSession.messages, idempotencyKey)) {
@@ -2806,7 +2815,6 @@ export async function runEmbeddedAttempt(
__openclaw: {
beforeAgentRunBlocked: {
blockedBy: block.pluginId,
reason: block.reason,
blockedAt: nowMs,
},
},
@@ -2851,26 +2859,30 @@ export async function runEmbeddedAttempt(
);
} catch (err) {
log.warn(`before_agent_run hook failed: ${formatErrorMessage(err)}`);
beforeAgentRunBlocked = true;
beforeAgentRunBlockedBy = "before_agent_run";
await persistBlockedBeforeAgentRun({
message: "Request blocked by before_agent_run policy.",
message: `${DEFAULT_BLOCK_MESSAGE} by before_agent_run`,
pluginId: "before_agent_run",
reason: "before_agent_run hook failed closed",
});
promptError = new Error("Request blocked by before_agent_run policy.");
promptError = new Error(`${DEFAULT_BLOCK_MESSAGE} by before_agent_run`);
promptErrorSource = "hook:before_agent_run";
skipPromptSubmission = true;
}
const beforeRunDecision = beforeRunResult?.decision;
const beforeRunPluginId = beforeRunResult?.pluginId ?? "unknown";
if (beforeRunDecision?.outcome === "block") {
const blockReplacementMsg = resolveBlockMessage(beforeRunDecision);
beforeAgentRunBlocked = true;
beforeAgentRunBlockedBy = beforeRunPluginId;
const blockReplacementMsg = resolveBlockMessage(beforeRunDecision, {
blockedBy: beforeRunPluginId,
});
log.warn(
`before_agent_run hook blocked by ${beforeRunPluginId}: ${beforeRunDecision.reason}`,
);
await persistBlockedBeforeAgentRun({
message: blockReplacementMsg,
pluginId: beforeRunPluginId,
reason: beforeRunDecision.reason,
});
promptError = new Error(blockReplacementMsg);
promptErrorSource = "hook:before_agent_run";
@@ -3783,12 +3795,19 @@ export async function runEmbeddedAttempt(
cleanupError = err;
}
emitDiagnosticRunCompleted?.(
cleanupError || promptError
cleanupError
? "error"
: aborted || timedOut || idleTimedOut || timedOutDuringCompaction
? "aborted"
: "completed",
: beforeAgentRunBlocked
? "blocked"
: promptError
? "error"
: aborted || timedOut || idleTimedOut || timedOutDuringCompaction
? "aborted"
: "completed",
cleanupError ?? promptError,
beforeAgentRunBlocked
? { blockedBy: beforeAgentRunBlockedBy ?? "before_agent_run" }
: undefined,
);
if (cleanupError) {
await Promise.reject(cleanupError);

5
src/gateway/session-message-events.test.ts
View File

@@ -314,7 +314,7 @@ describe("session.message websocket events", () => {
role: "user",
content: [{ type: "text", text: "The agent cannot read this message." }],
__openclaw: {
beforeAgentRunBlocked: { blockedBy: "policy-plugin", reason: "blocked", blockedAt: 1 },
beforeAgentRunBlocked: { blockedBy: "policy-plugin", blockedAt: 1 },
},
},
});
@@ -326,6 +326,7 @@ describe("session.message websocket events", () => {
{ type: "text", text: "The agent cannot read this message." },
]);
expect(JSON.stringify(payload.message)).not.toContain("secret blocked prompt");
expect(JSON.stringify(payload.message)).not.toContain("contains protected content");
});
});
@@ -353,7 +354,6 @@ describe("session.message websocket events", () => {
__openclaw: {
beforeAgentRunBlocked: {
blockedBy: "policy-plugin",
reason: "contains protected content",
blockedAt: Date.now(),
},
},
@@ -373,6 +373,7 @@ describe("session.message websocket events", () => {
{ type: "text", text: "The agent cannot read this message." },
]);
expect(JSON.stringify(payload.message)).not.toContain("secret blocked prompt");
expect(JSON.stringify(payload.message)).not.toContain("contains protected content");
});
});

7
src/gateway/session-utils.fs.test.ts
View File

@@ -82,7 +82,6 @@ function appendBlockedUserMessageWithSessionManager(params: {
originalText?: string;
redactedText: string;
pluginId: string;
reason: string;
idempotencyKey?: string;
}): string {
const sessionManager = SessionManager.open(params.sessionFile, path.dirname(params.sessionFile));
@@ -94,7 +93,6 @@ function appendBlockedUserMessageWithSessionManager(params: {
__openclaw: {
beforeAgentRunBlocked: {
blockedBy: params.pluginId,
reason: params.reason,
blockedAt: Date.now(),
},
},
@@ -1300,7 +1298,6 @@ describe("readSessionMessages", () => {
originalText: "[hitl:block] hello",
redactedText: "Blocked by HITL test hook.",
pluginId: "hitl-test-hooks",
reason: "blocked by test policy",
});
expect(messageId).toBeTruthy();
@@ -1316,6 +1313,7 @@ describe("readSessionMessages", () => {
{ role: "user", text: [{ type: "text", text: "Blocked by HITL test hook." }] },
]);
expect(JSON.stringify(out)).not.toContain("[hitl:block] hello");
expect(JSON.stringify(out)).not.toContain("matched original");
});
test("keeps repeated blocked hook messages together in a new session", async () => {
@@ -1343,14 +1341,12 @@ describe("readSessionMessages", () => {
originalText: "[hitl:block] first",
redactedText: "Blocked by HITL test hook.",
pluginId: "hitl-test-hooks",
reason: "blocked by test policy",
});
appendBlockedUserMessageWithSessionManager({
sessionFile,
originalText: "[hitl:block] second",
redactedText: "Blocked again by HITL test hook.",
pluginId: "hitl-test-hooks",
reason: "blocked by test policy",
});
const out = readSessionMessages(sessionId, storePath, sessionFile);
@@ -1365,6 +1361,7 @@ describe("readSessionMessages", () => {
]);
expect(JSON.stringify(out)).not.toContain("[hitl:block] first");
expect(JSON.stringify(out)).not.toContain("[hitl:block] second");
expect(JSON.stringify(out)).not.toContain("matched original");
});
});

3
src/infra/diagnostic-events.ts
View File

@@ -388,8 +388,9 @@ export type DiagnosticRunStartedEvent = DiagnosticRunBaseEvent & {
export type DiagnosticRunCompletedEvent = DiagnosticRunBaseEvent & {
type: "run.completed";
durationMs: number;
outcome: "completed" | "aborted" | "error";
outcome: "completed" | "aborted" | "blocked" | "error";
errorCategory?: string;
blockedBy?: string;
};
export type DiagnosticHarnessRunPhase = "prepare" | "start" | "send" | "resolve" | "cleanup";

3
src/plugins/hook-decision-types.test.ts
View File

@@ -65,6 +65,9 @@ describe("HookDecision helpers", () => {
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, message: " " })).toBe(DEFAULT_BLOCK_MESSAGE);
});
});

13
src/plugins/hook-decision-types.ts
View File

@@ -28,10 +28,15 @@ export type HookDecisionBlock = {
metadata?: Record<string, unknown>;
};
export function resolveBlockMessage(decision: HookDecisionBlock): string {
return typeof decision.message === "string" && decision.message.trim()
? decision.message
: DEFAULT_BLOCK_MESSAGE;
export function resolveBlockMessage(
decision: HookDecisionBlock,
params: { blockedBy?: string } = {},
): string {
if (typeof decision.message === "string" && decision.message.trim()) {
return decision.message;
}
const blockedBy = params.blockedBy?.trim();
return blockedBy ? `${DEFAULT_BLOCK_MESSAGE} by ${blockedBy}` : DEFAULT_BLOCK_MESSAGE;
}
/** Outcome severity for most-restrictive-wins merging. Higher = more restrictive. */