From 7a8d304a651a38998985ffc0e702a037d1d1fe2d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 23 Apr 2026 18:08:55 +0100 Subject: [PATCH] refactor: share core helper logic --- src/agents/harness/hook-context.ts | 25 +++++++ src/agents/harness/lifecycle-hook-helpers.ts | 26 +------- .../harness/prompt-compaction-hook-helpers.ts | 26 +------- .../reply/commands-export-common.ts | 65 +++++++++++++++++++ .../reply/commands-export-session.ts | 55 ++++------------ .../reply/commands-export-trajectory.ts | 50 ++++---------- src/commands/test-runtime-config-helpers.ts | 17 +++++ src/security/audit-gateway-config.ts | 28 +------- src/security/core-dangerous-config-flags.ts | 28 ++++++++ src/security/dangerous-config-flags.ts | 25 +------ 10 files changed, 166 insertions(+), 179 deletions(-) create mode 100644 src/agents/harness/hook-context.ts create mode 100644 src/auto-reply/reply/commands-export-common.ts create mode 100644 src/security/core-dangerous-config-flags.ts diff --git a/src/agents/harness/hook-context.ts b/src/agents/harness/hook-context.ts new file mode 100644 index 00000000000..6eb6538b590 --- /dev/null +++ b/src/agents/harness/hook-context.ts @@ -0,0 +1,25 @@ +import type { PluginHookAgentContext } from "../../plugins/hook-types.js"; + +export type AgentHarnessHookContext = { + runId: string; + agentId?: string; + sessionKey?: string; + sessionId?: string; + workspaceDir?: string; + messageProvider?: string; + trigger?: string; + channelId?: string; +}; + +export function buildAgentHookContext(params: AgentHarnessHookContext): PluginHookAgentContext { + return { + runId: params.runId, + ...(params.agentId ? { agentId: params.agentId } : {}), + ...(params.sessionKey ? { sessionKey: params.sessionKey } : {}), + ...(params.sessionId ? { sessionId: params.sessionId } : {}), + ...(params.workspaceDir ? { workspaceDir: params.workspaceDir } : {}), + ...(params.messageProvider ? { messageProvider: params.messageProvider } : {}), + ...(params.trigger ? { trigger: params.trigger } : {}), + ...(params.channelId ? { channelId: params.channelId } : {}), + }; +} diff --git a/src/agents/harness/lifecycle-hook-helpers.ts b/src/agents/harness/lifecycle-hook-helpers.ts index 792c29151b7..8e5bba3303c 100644 --- a/src/agents/harness/lifecycle-hook-helpers.ts +++ b/src/agents/harness/lifecycle-hook-helpers.ts @@ -2,37 +2,13 @@ import { createSubsystemLogger } from "../../logging/subsystem.js"; import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js"; import type { PluginHookAgentEndEvent, - PluginHookAgentContext, PluginHookLlmInputEvent, PluginHookLlmOutputEvent, } from "../../plugins/hook-types.js"; +import { buildAgentHookContext, type AgentHarnessHookContext } from "./hook-context.js"; const log = createSubsystemLogger("agents/harness"); -type AgentHarnessHookContext = { - runId: string; - agentId?: string; - sessionKey?: string; - sessionId?: string; - workspaceDir?: string; - messageProvider?: string; - trigger?: string; - channelId?: string; -}; - -function buildAgentHookContext(params: AgentHarnessHookContext): PluginHookAgentContext { - return { - runId: params.runId, - ...(params.agentId ? { agentId: params.agentId } : {}), - ...(params.sessionKey ? { sessionKey: params.sessionKey } : {}), - ...(params.sessionId ? { sessionId: params.sessionId } : {}), - ...(params.workspaceDir ? { workspaceDir: params.workspaceDir } : {}), - ...(params.messageProvider ? { messageProvider: params.messageProvider } : {}), - ...(params.trigger ? { trigger: params.trigger } : {}), - ...(params.channelId ? { channelId: params.channelId } : {}), - }; -} - export function runAgentHarnessLlmInputHook(params: { event: PluginHookLlmInputEvent; ctx: AgentHarnessHookContext; diff --git a/src/agents/harness/prompt-compaction-hook-helpers.ts b/src/agents/harness/prompt-compaction-hook-helpers.ts index 53f35a3d65e..df556682985 100644 --- a/src/agents/harness/prompt-compaction-hook-helpers.ts +++ b/src/agents/harness/prompt-compaction-hook-helpers.ts @@ -2,43 +2,19 @@ import type { AgentMessage } from "@mariozechner/pi-agent-core"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js"; import type { - PluginHookAgentContext, PluginHookBeforeAgentStartResult, PluginHookBeforePromptBuildResult, } from "../../plugins/types.js"; import { joinPresentTextSegments } from "../../shared/text/join-segments.js"; +import { buildAgentHookContext, type AgentHarnessHookContext } from "./hook-context.js"; const log = createSubsystemLogger("agents/harness"); -type AgentHarnessHookContext = { - runId: string; - agentId?: string; - sessionKey?: string; - sessionId?: string; - workspaceDir?: string; - messageProvider?: string; - trigger?: string; - channelId?: string; -}; - export type AgentHarnessPromptBuildResult = { prompt: string; developerInstructions: string; }; -function buildAgentHookContext(params: AgentHarnessHookContext): PluginHookAgentContext { - return { - runId: params.runId, - ...(params.agentId ? { agentId: params.agentId } : {}), - ...(params.sessionKey ? { sessionKey: params.sessionKey } : {}), - ...(params.sessionId ? { sessionId: params.sessionId } : {}), - ...(params.workspaceDir ? { workspaceDir: params.workspaceDir } : {}), - ...(params.messageProvider ? { messageProvider: params.messageProvider } : {}), - ...(params.trigger ? { trigger: params.trigger } : {}), - ...(params.channelId ? { channelId: params.channelId } : {}), - }; -} - export async function resolveAgentHarnessBeforePromptBuildResult(params: { prompt: string; developerInstructions: string; diff --git a/src/auto-reply/reply/commands-export-common.ts b/src/auto-reply/reply/commands-export-common.ts new file mode 100644 index 00000000000..9e2d0a7c675 --- /dev/null +++ b/src/auto-reply/reply/commands-export-common.ts @@ -0,0 +1,65 @@ +import { + resolveDefaultSessionStorePath, + resolveSessionFilePath, + resolveSessionFilePathOptions, +} from "../../config/sessions/paths.js"; +import { loadSessionStore } from "../../config/sessions/store.js"; +import type { SessionEntry } from "../../config/sessions/types.js"; +import { formatErrorMessage } from "../../infra/errors.js"; +import { resolveAgentIdFromSessionKey } from "../../routing/session-key.js"; +import type { ReplyPayload } from "../types.js"; +import type { HandleCommandsParams } from "./commands-types.js"; + +export interface ExportCommandSessionTarget { + entry: SessionEntry; + sessionFile: string; +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +export function parseExportCommandOutputPath( + commandBodyNormalized: string, + aliases: readonly string[], +): { outputPath?: string } { + const normalized = commandBodyNormalized.trim(); + if (aliases.some((alias) => normalized === `/${alias}`)) { + return {}; + } + const aliasPattern = aliases.map(escapeRegExp).join("|"); + const args = normalized.replace(new RegExp(`^/(${aliasPattern})\\s*`), "").trim(); + const outputPath = args.split(/\s+/).find((part) => !part.startsWith("-")); + return { outputPath }; +} + +export function resolveExportCommandSessionTarget( + params: HandleCommandsParams, +): ExportCommandSessionTarget | ReplyPayload { + const targetAgentId = resolveAgentIdFromSessionKey(params.sessionKey) || params.agentId; + const storePath = params.storePath ?? resolveDefaultSessionStorePath(targetAgentId); + const store = loadSessionStore(storePath, { skipCache: true }); + const entry = store[params.sessionKey] as SessionEntry | undefined; + if (!entry?.sessionId) { + return { text: `❌ Session not found: ${params.sessionKey}` }; + } + + try { + const sessionFile = resolveSessionFilePath( + entry.sessionId, + entry, + resolveSessionFilePathOptions({ agentId: targetAgentId, storePath }), + ); + return { entry, sessionFile }; + } catch (err) { + return { + text: `❌ Failed to resolve session file: ${formatErrorMessage(err)}`, + }; + } +} + +export function isReplyPayload( + value: ExportCommandSessionTarget | ReplyPayload, +): value is ReplyPayload { + return "text" in value; +} diff --git a/src/auto-reply/reply/commands-export-session.ts b/src/auto-reply/reply/commands-export-session.ts index 3209f716df7..d60b5d7d3b8 100644 --- a/src/auto-reply/reply/commands-export-session.ts +++ b/src/auto-reply/reply/commands-export-session.ts @@ -3,16 +3,12 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import type { SessionEntry as PiSessionEntry, SessionHeader } from "@mariozechner/pi-coding-agent"; import { SessionManager } from "@mariozechner/pi-coding-agent"; -import { - resolveDefaultSessionStorePath, - resolveSessionFilePath, - resolveSessionFilePathOptions, -} from "../../config/sessions/paths.js"; -import { loadSessionStore } from "../../config/sessions/store.js"; -import type { SessionEntry } from "../../config/sessions/types.js"; -import { formatErrorMessage } from "../../infra/errors.js"; -import { resolveAgentIdFromSessionKey } from "../../routing/session-key.js"; import type { ReplyPayload } from "../types.js"; +import { + isReplyPayload, + parseExportCommandOutputPath, + resolveExportCommandSessionTarget, +} from "./commands-export-common.js"; import { resolveCommandsSystemPromptBundle } from "./commands-system-prompt.js"; import type { HandleCommandsParams } from "./commands-types.js"; @@ -100,41 +96,16 @@ function generateHtml(sessionData: SessionData): string { .replace("{{HIGHLIGHT_JS}}", hljsJs); } -function parseExportArgs(commandBodyNormalized: string): { outputPath?: string } { - const normalized = commandBodyNormalized.trim(); - if (normalized === "/export-session" || normalized === "/export") { - return {}; - } - const args = normalized.replace(/^\/(export-session|export)\s*/, "").trim(); - // First non-flag argument is the output path - const outputPath = args.split(/\s+/).find((part) => !part.startsWith("-")); - return { outputPath }; -} - export async function buildExportSessionReply(params: HandleCommandsParams): Promise { - const args = parseExportArgs(params.command.commandBodyNormalized); - - // 1. Resolve target session entry and session file from the canonical target store. - const targetAgentId = resolveAgentIdFromSessionKey(params.sessionKey) || params.agentId; - const storePath = params.storePath ?? resolveDefaultSessionStorePath(targetAgentId); - const store = loadSessionStore(storePath, { skipCache: true }); - const entry = store[params.sessionKey] as SessionEntry | undefined; - if (!entry?.sessionId) { - return { text: `❌ Session not found: ${params.sessionKey}` }; - } - - let sessionFile: string; - try { - sessionFile = resolveSessionFilePath( - entry.sessionId, - entry, - resolveSessionFilePathOptions({ agentId: targetAgentId, storePath }), - ); - } catch (err) { - return { - text: `❌ Failed to resolve session file: ${formatErrorMessage(err)}`, - }; + const args = parseExportCommandOutputPath(params.command.commandBodyNormalized, [ + "export-session", + "export", + ]); + const sessionTarget = resolveExportCommandSessionTarget(params); + if (isReplyPayload(sessionTarget)) { + return sessionTarget; } + const { entry, sessionFile } = sessionTarget; if (!fs.existsSync(sessionFile)) { return { text: `❌ Session file not found: ${sessionFile}` }; diff --git a/src/auto-reply/reply/commands-export-trajectory.ts b/src/auto-reply/reply/commands-export-trajectory.ts index 768c0dce642..a85703a88fa 100644 --- a/src/auto-reply/reply/commands-export-trajectory.ts +++ b/src/auto-reply/reply/commands-export-trajectory.ts @@ -1,31 +1,18 @@ import fs from "node:fs"; import path from "node:path"; -import { - resolveDefaultSessionStorePath, - resolveSessionFilePath, - resolveSessionFilePathOptions, -} from "../../config/sessions/paths.js"; -import { loadSessionStore } from "../../config/sessions/store.js"; -import type { SessionEntry } from "../../config/sessions/types.js"; import { formatErrorMessage } from "../../infra/errors.js"; -import { resolveAgentIdFromSessionKey } from "../../routing/session-key.js"; import { exportTrajectoryBundle, resolveDefaultTrajectoryExportDir, } from "../../trajectory/export.js"; import type { ReplyPayload } from "../types.js"; +import { + isReplyPayload, + parseExportCommandOutputPath, + resolveExportCommandSessionTarget, +} from "./commands-export-common.js"; import type { HandleCommandsParams } from "./commands-types.js"; -function parseExportTrajectoryArgs(commandBodyNormalized: string): { outputPath?: string } { - const normalized = commandBodyNormalized.trim(); - if (normalized === "/export-trajectory" || normalized === "/trajectory") { - return {}; - } - const args = normalized.replace(/^\/(export-trajectory|trajectory)\s*/, "").trim(); - const outputPath = args.split(/\s+/).find((part) => !part.startsWith("-")); - return { outputPath }; -} - function isPathInsideOrEqual(baseDir: string, candidate: string): boolean { const relative = path.relative(baseDir, candidate); return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); @@ -126,27 +113,16 @@ function resolveTrajectoryCommandOutputDir(params: { export async function buildExportTrajectoryReply( params: HandleCommandsParams, ): Promise { - const args = parseExportTrajectoryArgs(params.command.commandBodyNormalized); - const targetAgentId = resolveAgentIdFromSessionKey(params.sessionKey) || params.agentId; - const storePath = params.storePath ?? resolveDefaultSessionStorePath(targetAgentId); - const store = loadSessionStore(storePath, { skipCache: true }); - const entry = store[params.sessionKey] as SessionEntry | undefined; - if (!entry?.sessionId) { - return { text: `❌ Session not found: ${params.sessionKey}` }; + const args = parseExportCommandOutputPath(params.command.commandBodyNormalized, [ + "export-trajectory", + "trajectory", + ]); + const sessionTarget = resolveExportCommandSessionTarget(params); + if (isReplyPayload(sessionTarget)) { + return sessionTarget; } + const { entry, sessionFile } = sessionTarget; - let sessionFile: string; - try { - sessionFile = resolveSessionFilePath( - entry.sessionId, - entry, - resolveSessionFilePathOptions({ agentId: targetAgentId, storePath }), - ); - } catch (err) { - return { - text: `❌ Failed to resolve session file: ${formatErrorMessage(err)}`, - }; - } if (!fs.existsSync(sessionFile)) { return { text: "❌ Session file not found." }; } diff --git a/src/commands/test-runtime-config-helpers.ts b/src/commands/test-runtime-config-helpers.ts index 0ab88593e9b..5769925df04 100644 --- a/src/commands/test-runtime-config-helpers.ts +++ b/src/commands/test-runtime-config-helpers.ts @@ -19,6 +19,12 @@ export type TestRuntime = { exit: MockFn; }; +export type CapturingTestRuntime = { + runtime: RuntimeEnv; + logs: string[]; + errors: string[]; +}; + export function createTestRuntime(): TestRuntime { const log = vi.fn() as MockFn; const error = vi.fn() as MockFn; @@ -30,6 +36,17 @@ export function createTestRuntime(): TestRuntime { }; } +export function createCapturingTestRuntime(): CapturingTestRuntime { + const logs: string[] = []; + const errors: string[] = []; + const runtime = { + log: (message: unknown) => logs.push(String(message)), + error: (message: unknown) => errors.push(String(message)), + exit: (_code?: number) => undefined, + }; + return { runtime, logs, errors }; +} + export function createThrowingTestRuntime(): RuntimeEnv { return { log: vi.fn(), diff --git a/src/security/audit-gateway-config.ts b/src/security/audit-gateway-config.ts index ba77235266d..054d1f809b6 100644 --- a/src/security/audit-gateway-config.ts +++ b/src/security/audit-gateway-config.ts @@ -7,6 +7,7 @@ import { normalizeOptionalLowercaseString, } from "../shared/string-coerce.js"; import type { SecurityAuditFinding } from "./audit.types.js"; +import { collectCoreInsecureOrDangerousFlags } from "./core-dangerous-config-flags.js"; import { DEFAULT_GATEWAY_HTTP_TOOL_DENY } from "./dangerous-tools.js"; type CollectDangerousConfigFlags = (cfg: OpenClawConfig) => string[]; @@ -19,33 +20,6 @@ function hasNonEmptyString(value: unknown): value is string { return typeof value === "string" && value.trim().length > 0; } -function collectCoreInsecureOrDangerousFlags(cfg: OpenClawConfig): string[] { - const enabledFlags: string[] = []; - if (cfg.gateway?.controlUi?.allowInsecureAuth === true) { - enabledFlags.push("gateway.controlUi.allowInsecureAuth=true"); - } - if (cfg.gateway?.controlUi?.dangerouslyAllowHostHeaderOriginFallback === true) { - enabledFlags.push("gateway.controlUi.dangerouslyAllowHostHeaderOriginFallback=true"); - } - if (cfg.gateway?.controlUi?.dangerouslyDisableDeviceAuth === true) { - enabledFlags.push("gateway.controlUi.dangerouslyDisableDeviceAuth=true"); - } - if (cfg.hooks?.gmail?.allowUnsafeExternalContent === true) { - enabledFlags.push("hooks.gmail.allowUnsafeExternalContent=true"); - } - if (Array.isArray(cfg.hooks?.mappings)) { - for (const [index, mapping] of cfg.hooks.mappings.entries()) { - if (mapping?.allowUnsafeExternalContent === true) { - enabledFlags.push(`hooks.mappings[${index}].allowUnsafeExternalContent=true`); - } - } - } - if (cfg.tools?.exec?.applyPatch?.workspaceOnly === false) { - enabledFlags.push("tools.exec.applyPatch.workspaceOnly=false"); - } - return enabledFlags; -} - export function collectGatewayConfigFindings( cfg: OpenClawConfig, sourceConfig: OpenClawConfig, diff --git a/src/security/core-dangerous-config-flags.ts b/src/security/core-dangerous-config-flags.ts new file mode 100644 index 00000000000..4693a690141 --- /dev/null +++ b/src/security/core-dangerous-config-flags.ts @@ -0,0 +1,28 @@ +import type { OpenClawConfig } from "../config/types.openclaw.js"; + +export function collectCoreInsecureOrDangerousFlags(cfg: OpenClawConfig): string[] { + const enabledFlags: string[] = []; + if (cfg.gateway?.controlUi?.allowInsecureAuth === true) { + enabledFlags.push("gateway.controlUi.allowInsecureAuth=true"); + } + if (cfg.gateway?.controlUi?.dangerouslyAllowHostHeaderOriginFallback === true) { + enabledFlags.push("gateway.controlUi.dangerouslyAllowHostHeaderOriginFallback=true"); + } + if (cfg.gateway?.controlUi?.dangerouslyDisableDeviceAuth === true) { + enabledFlags.push("gateway.controlUi.dangerouslyDisableDeviceAuth=true"); + } + if (cfg.hooks?.gmail?.allowUnsafeExternalContent === true) { + enabledFlags.push("hooks.gmail.allowUnsafeExternalContent=true"); + } + if (Array.isArray(cfg.hooks?.mappings)) { + for (const [index, mapping] of cfg.hooks.mappings.entries()) { + if (mapping?.allowUnsafeExternalContent === true) { + enabledFlags.push(`hooks.mappings[${index}].allowUnsafeExternalContent=true`); + } + } + } + if (cfg.tools?.exec?.applyPatch?.workspaceOnly === false) { + enabledFlags.push("tools.exec.applyPatch.workspaceOnly=false"); + } + return enabledFlags; +} diff --git a/src/security/dangerous-config-flags.ts b/src/security/dangerous-config-flags.ts index 1d50a9abed0..ca4e90ea376 100644 --- a/src/security/dangerous-config-flags.ts +++ b/src/security/dangerous-config-flags.ts @@ -6,6 +6,7 @@ import { resolvePluginConfigContractsById, } from "../plugins/config-contracts.js"; import { isRecord } from "../utils.js"; +import { collectCoreInsecureOrDangerousFlags } from "./core-dangerous-config-flags.js"; function formatDangerousConfigFlagValue(value: string | number | boolean | null): string { return value === null ? "null" : String(value); @@ -24,7 +25,7 @@ function getAgentDangerousFlagPathSegment(agent: unknown, index: number): string } export function collectEnabledInsecureOrDangerousFlags(cfg: OpenClawConfig): string[] { - const enabledFlags: string[] = []; + const enabledFlags = collectCoreInsecureOrDangerousFlags(cfg); const collectSandboxDockerDangerousFlags = ( docker: Record | undefined, @@ -40,34 +41,12 @@ export function collectEnabledInsecureOrDangerousFlags(cfg: OpenClawConfig): str } }; - if (cfg.gateway?.controlUi?.allowInsecureAuth === true) { - enabledFlags.push("gateway.controlUi.allowInsecureAuth=true"); - } - if (cfg.gateway?.controlUi?.dangerouslyAllowHostHeaderOriginFallback === true) { - enabledFlags.push("gateway.controlUi.dangerouslyAllowHostHeaderOriginFallback=true"); - } - if (cfg.gateway?.controlUi?.dangerouslyDisableDeviceAuth === true) { - enabledFlags.push("gateway.controlUi.dangerouslyDisableDeviceAuth=true"); - } - if (cfg.hooks?.gmail?.allowUnsafeExternalContent === true) { - enabledFlags.push("hooks.gmail.allowUnsafeExternalContent=true"); - } - if (Array.isArray(cfg.hooks?.mappings)) { - for (const [index, mapping] of cfg.hooks.mappings.entries()) { - if (mapping?.allowUnsafeExternalContent === true) { - enabledFlags.push(`hooks.mappings[${index}].allowUnsafeExternalContent=true`); - } - } - } if (cfg.hooks?.allowRequestSessionKey === true) { enabledFlags.push("hooks.allowRequestSessionKey=true"); } if (cfg.browser?.ssrfPolicy?.dangerouslyAllowPrivateNetwork === true) { enabledFlags.push("browser.ssrfPolicy.dangerouslyAllowPrivateNetwork=true"); } - if (cfg.tools?.exec?.applyPatch?.workspaceOnly === false) { - enabledFlags.push("tools.exec.applyPatch.workspaceOnly=false"); - } if (cfg.tools?.fs?.workspaceOnly === false) { enabledFlags.push("tools.fs.workspaceOnly=false"); }