mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-16 20:36:55 +00:00
fix(audit): show valid values for rejected filters (#124336)
* fix(audit): validate filter flags before gateway call * refactor(audit): keep filter constants owner-local * fix(audit): keep cursor errors operator-facing
This commit is contained in:
parent
4be10d44d5
commit
2585edba2e
4 changed files with 126 additions and 38 deletions
|
|
@ -5,26 +5,27 @@ import { NonEmptyString } from "./primitives.js";
|
|||
|
||||
const AuditActivitySchemaVersionV1Schema = Type.Integer({ minimum: 1, maximum: 1 });
|
||||
|
||||
const AuditActivityStatusV1Schema: TSchema = Type.Union([
|
||||
Type.Literal("started"),
|
||||
Type.Literal("succeeded"),
|
||||
Type.Literal("failed"),
|
||||
Type.Literal("cancelled"),
|
||||
Type.Literal("timed_out"),
|
||||
Type.Literal("blocked"),
|
||||
Type.Literal("unknown"),
|
||||
]);
|
||||
export const AUDIT_ACTIVITY_STATUSES = [
|
||||
"started",
|
||||
"succeeded",
|
||||
"failed",
|
||||
"cancelled",
|
||||
"timed_out",
|
||||
"blocked",
|
||||
"unknown",
|
||||
] as const;
|
||||
export const AUDIT_ACTIVITY_KINDS = ["agent_run", "tool_action", "message"] as const;
|
||||
export const AUDIT_ACTIVITY_DIRECTIONS = ["inbound", "outbound"] as const;
|
||||
|
||||
const AuditActivityKindV1Schema: TSchema = Type.Union([
|
||||
Type.Literal("agent_run"),
|
||||
Type.Literal("tool_action"),
|
||||
Type.Literal("message"),
|
||||
]);
|
||||
|
||||
const AuditActivityDirectionV1Schema: TSchema = Type.Union([
|
||||
Type.Literal("inbound"),
|
||||
Type.Literal("outbound"),
|
||||
]);
|
||||
const AuditActivityStatusV1Schema: TSchema = Type.Union(
|
||||
AUDIT_ACTIVITY_STATUSES.map((value) => Type.Literal(value)),
|
||||
);
|
||||
const AuditActivityKindV1Schema: TSchema = Type.Union(
|
||||
AUDIT_ACTIVITY_KINDS.map((value) => Type.Literal(value)),
|
||||
);
|
||||
const AuditActivityDirectionV1Schema: TSchema = Type.Union(
|
||||
AUDIT_ACTIVITY_DIRECTIONS.map((value) => Type.Literal(value)),
|
||||
);
|
||||
|
||||
const AuditActivityConversationKindV1Schema = Type.Union([
|
||||
Type.Literal("direct"),
|
||||
|
|
@ -607,9 +608,9 @@ export type AuditActivityListParams = {
|
|||
agentId?: string;
|
||||
sessionKey?: string;
|
||||
runId?: string;
|
||||
kind?: "agent_run" | "tool_action" | "message";
|
||||
status?: "started" | "succeeded" | "failed" | "cancelled" | "timed_out" | "blocked" | "unknown";
|
||||
direction?: "inbound" | "outbound";
|
||||
kind?: (typeof AUDIT_ACTIVITY_KINDS)[number];
|
||||
status?: (typeof AUDIT_ACTIVITY_STATUSES)[number];
|
||||
direction?: (typeof AUDIT_ACTIVITY_DIRECTIONS)[number];
|
||||
channel?: string;
|
||||
after?: number;
|
||||
before?: number;
|
||||
|
|
|
|||
|
|
@ -1,9 +1,15 @@
|
|||
// Audit command registration for privacy-preserving activity history.
|
||||
import type { Command } from "commander";
|
||||
import {
|
||||
AUDIT_ACTIVITY_DIRECTIONS,
|
||||
AUDIT_ACTIVITY_KINDS,
|
||||
AUDIT_ACTIVITY_STATUSES,
|
||||
} from "../../../packages/gateway-protocol/src/schema/audit-activity.js";
|
||||
import { formatDocsLink } from "../../../packages/terminal-core/src/links.js";
|
||||
import { theme } from "../../../packages/terminal-core/src/theme.js";
|
||||
import { auditListCommand, type AuditListCommandOptions } from "../../commands/audit.js";
|
||||
import { defaultRuntime } from "../../runtime.js";
|
||||
import { formatHumanList } from "../../shared/human-list.js";
|
||||
import { runCommandWithRuntime } from "../cli-utils.js";
|
||||
|
||||
/** Register the bounded operator audit query command. */
|
||||
|
|
@ -15,12 +21,12 @@ export function registerAuditCommand(program: Command): void {
|
|||
.option("--session <key>", "Filter by exact session key")
|
||||
.option("--run <id>", "Filter by run id")
|
||||
.option("--execution <id>", "Inspect one exact execution id")
|
||||
.option("--kind <kind>", "Filter by kind (agent_run, tool_action, or message)")
|
||||
.option("--kind <kind>", `Filter by kind (${formatHumanList(AUDIT_ACTIVITY_KINDS)})`)
|
||||
.option("--status <status>", `Filter by status (${formatHumanList(AUDIT_ACTIVITY_STATUSES)})`)
|
||||
.option(
|
||||
"--status <status>",
|
||||
"Filter by status (started, succeeded, failed, cancelled, timed_out, blocked, unknown)",
|
||||
"--direction <direction>",
|
||||
`Filter message direction (${formatHumanList(AUDIT_ACTIVITY_DIRECTIONS)})`,
|
||||
)
|
||||
.option("--direction <direction>", "Filter message direction (inbound or outbound)")
|
||||
.option("--channel <channel>", "Filter message channel")
|
||||
.option("--after <timestamp>", "Include records at/after ISO time or Unix milliseconds")
|
||||
.option("--before <timestamp>", "Include records at/before ISO time or Unix milliseconds")
|
||||
|
|
|
|||
|
|
@ -1,4 +1,10 @@
|
|||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
AUDIT_ACTIVITY_DIRECTIONS,
|
||||
AUDIT_ACTIVITY_KINDS,
|
||||
AUDIT_ACTIVITY_STATUSES,
|
||||
} from "../../packages/gateway-protocol/src/schema/audit-activity.js";
|
||||
import { runCommandWithRuntime } from "../cli/cli-utils.js";
|
||||
import type { RuntimeEnv } from "../runtime.js";
|
||||
import { auditListCommand } from "./audit.js";
|
||||
|
||||
|
|
@ -107,13 +113,39 @@ describe("audit command parsing", () => {
|
|||
expect(callGateway).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects unknown event kinds before querying the Gateway", async () => {
|
||||
await expect(
|
||||
auditListCommand({ kind: "bogus" as never, limit: "10" }, runtime),
|
||||
).rejects.toThrow("--kind must be agent_run, tool_action, or message");
|
||||
it.each([
|
||||
{
|
||||
options: { kind: "bogus" as never },
|
||||
message: "--kind must be agent_run, tool_action, or message.",
|
||||
},
|
||||
{
|
||||
options: { status: "bogus" as never },
|
||||
message:
|
||||
"--status must be started, succeeded, failed, cancelled, timed_out, blocked, or unknown.",
|
||||
},
|
||||
{
|
||||
options: { direction: "sideways" as never },
|
||||
message: "--direction must be inbound or outbound.",
|
||||
},
|
||||
])("rejects invalid audit filters before querying the Gateway", async ({ options, message }) => {
|
||||
await expect(auditListCommand({ ...options, limit: "10" }, runtime)).rejects.toThrow(message);
|
||||
expect(callGateway).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["kind", AUDIT_ACTIVITY_KINDS],
|
||||
["status", AUDIT_ACTIVITY_STATUSES],
|
||||
["direction", AUDIT_ACTIVITY_DIRECTIONS],
|
||||
] as const)("forwards every canonical %s value unchanged", async (filter, values) => {
|
||||
for (const value of values) {
|
||||
await auditListCommand({ [filter]: value }, runtime);
|
||||
expect(callGateway).toHaveBeenLastCalledWith({
|
||||
method: "audit.activity.list",
|
||||
params: { limit: 100, [filter]: value },
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it("renders activity safely without inventing message provenance", async () => {
|
||||
callGateway.mockResolvedValue({
|
||||
events: [
|
||||
|
|
@ -164,6 +196,8 @@ describe("audit command gateway compatibility", () => {
|
|||
beforeEach(() => {
|
||||
callGateway.mockReset();
|
||||
callGateway.mockResolvedValue({ events: [] });
|
||||
vi.mocked(runtime.error).mockClear();
|
||||
vi.mocked(runtime.exit).mockClear();
|
||||
});
|
||||
|
||||
it("forwards all filters to audit.activity.list", async () => {
|
||||
|
|
@ -278,16 +312,41 @@ describe("audit command gateway compatibility", () => {
|
|||
expect(callGateway).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not fall back for other request errors", async () => {
|
||||
it("renders other request errors without the Gateway error class name", async () => {
|
||||
const error = Object.assign(new Error("invalid audit activity params"), {
|
||||
name: "GatewayClientRequestError",
|
||||
gatewayCode: "INVALID_REQUEST",
|
||||
});
|
||||
callGateway.mockRejectedValueOnce(error);
|
||||
|
||||
await expect(auditListCommand({ limit: "10" }, runtime)).rejects.toBe(error);
|
||||
await runCommandWithRuntime(runtime, () => auditListCommand({ limit: "10" }, runtime));
|
||||
|
||||
expect(runtime.error).toHaveBeenCalledWith("Error: invalid audit activity params");
|
||||
expect(String(vi.mocked(runtime.error).mock.calls[0]?.[0])).not.toContain(
|
||||
"GatewayClientRequestError",
|
||||
);
|
||||
expect(runtime.exit).toHaveBeenCalledWith(1);
|
||||
expect(callGateway).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("turns an opaque cursor rejection into an operator recovery step", async () => {
|
||||
callGateway.mockRejectedValueOnce(
|
||||
Object.assign(new Error("invalid audit.activity.list range or cursor"), {
|
||||
name: "GatewayClientRequestError",
|
||||
gatewayCode: "INVALID_REQUEST",
|
||||
}),
|
||||
);
|
||||
|
||||
await runCommandWithRuntime(runtime, () => auditListCommand({ cursor: "abc" }, runtime));
|
||||
|
||||
expect(runtime.error).toHaveBeenCalledWith(
|
||||
"Error: --cursor must be a continuation token returned by a previous audit result.",
|
||||
);
|
||||
expect(String(vi.mocked(runtime.error).mock.calls[0]?.[0])).not.toContain(
|
||||
"audit.activity.list",
|
||||
);
|
||||
expect(runtime.exit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("audit run explanation", () => {
|
||||
|
|
|
|||
|
|
@ -15,11 +15,18 @@ import type {
|
|||
ExecutionIdentityContextV1,
|
||||
PrincipalRefV1,
|
||||
} from "../../packages/gateway-protocol/src/index.js";
|
||||
import {
|
||||
AUDIT_ACTIVITY_DIRECTIONS,
|
||||
AUDIT_ACTIVITY_KINDS,
|
||||
AUDIT_ACTIVITY_STATUSES,
|
||||
} from "../../packages/gateway-protocol/src/schema/audit-activity.js";
|
||||
import { sanitizeTerminalText } from "../../packages/terminal-core/src/safe-text.js";
|
||||
import { parsePositiveAuditCursor } from "../audit/audit-cursor.js";
|
||||
import { parseAbsoluteTimeMs } from "../cron/parse.js";
|
||||
import { callGateway } from "../gateway/call.js";
|
||||
import { formatErrorMessage } from "../infra/errors.js";
|
||||
import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js";
|
||||
import { formatHumanList } from "../shared/human-list.js";
|
||||
|
||||
const DEFAULT_AUDIT_LIMIT = 100;
|
||||
const MAX_AUDIT_LIMIT = 500;
|
||||
|
|
@ -161,12 +168,25 @@ function hasMessageSpecificFilters(options: AuditListCommandOptions): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
function validateAuditKind(kind: AuditListCommandOptions["kind"]): void {
|
||||
if (kind !== undefined && kind !== "agent_run" && kind !== "tool_action" && kind !== "message") {
|
||||
throw new Error("--kind must be agent_run, tool_action, or message.");
|
||||
function validateAuditFilter(
|
||||
value: string | undefined,
|
||||
flag: string,
|
||||
allowed: readonly string[],
|
||||
): void {
|
||||
if (value !== undefined && !allowed.includes(value)) {
|
||||
throw new Error(`${flag} must be ${formatHumanList(allowed)}.`);
|
||||
}
|
||||
}
|
||||
|
||||
function formatAuditGatewayError(error: unknown): Error {
|
||||
const message = formatErrorMessage(error);
|
||||
const operatorMessage =
|
||||
message === "invalid audit.activity.list range or cursor"
|
||||
? "--cursor must be a continuation token returned by a previous audit result."
|
||||
: message;
|
||||
return new Error(operatorMessage);
|
||||
}
|
||||
|
||||
function toLegacyAuditListParams(params: AuditActivityListParams): AuditListParams {
|
||||
return {
|
||||
...(params.agentId ? { agentId: params.agentId } : {}),
|
||||
|
|
@ -192,7 +212,7 @@ async function queryAuditActivity(
|
|||
});
|
||||
} catch (error) {
|
||||
if (!isUnsupportedActivityMethodError(error)) {
|
||||
throw error;
|
||||
throw formatAuditGatewayError(error);
|
||||
}
|
||||
if (hasMessageSpecificFilters(options)) {
|
||||
throw new Error(
|
||||
|
|
@ -237,7 +257,7 @@ async function queryAuditRunInspection(
|
|||
return await callGateway<AuditRunInspectResult>({ method: "audit.run.inspect", params });
|
||||
} catch (error) {
|
||||
if (!isUnsupportedRunInspectMethodError(error)) {
|
||||
throw error;
|
||||
throw formatAuditGatewayError(error);
|
||||
}
|
||||
return unsupportedRunInspection(
|
||||
typeof params.runId === "string"
|
||||
|
|
@ -503,7 +523,9 @@ export async function auditListCommand(
|
|||
if (options.executionId) {
|
||||
throw new Error("--execution requires --explain.");
|
||||
}
|
||||
validateAuditKind(options.kind);
|
||||
validateAuditFilter(options.kind, "--kind", AUDIT_ACTIVITY_KINDS);
|
||||
validateAuditFilter(options.status, "--status", AUDIT_ACTIVITY_STATUSES);
|
||||
validateAuditFilter(options.direction, "--direction", AUDIT_ACTIVITY_DIRECTIONS);
|
||||
const after = parseAuditTimestamp(options.after, "--after");
|
||||
const before = parseAuditTimestamp(options.before, "--before");
|
||||
if (after !== undefined && before !== undefined && after > before) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue