diff --git a/src/flows/doctor-startup-channel-maintenance.test.ts b/src/flows/doctor-startup-channel-maintenance.test.ts index a2073f28564..74932e07fe4 100644 --- a/src/flows/doctor-startup-channel-maintenance.test.ts +++ b/src/flows/doctor-startup-channel-maintenance.test.ts @@ -1,17 +1,7 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import { maybeRunDoctorStartupChannelMaintenance } from "./doctor-startup-channel-maintenance.js"; -const runChannelPluginStartupMaintenance = vi.hoisted(() => vi.fn()); - -vi.mock("../channels/plugins/lifecycle-startup.js", () => ({ - runChannelPluginStartupMaintenance, -})); - describe("doctor startup channel maintenance", () => { - beforeEach(() => { - runChannelPluginStartupMaintenance.mockClear(); - }); - it("runs Matrix startup migration during repair flows", async () => { const cfg = { channels: { @@ -22,17 +12,21 @@ describe("doctor startup channel maintenance", () => { }, }, }; - const runtime = { log: vi.fn(), error: vi.fn() }; + const calls: unknown[] = []; + const runtime = { log() {}, error() {} }; await maybeRunDoctorStartupChannelMaintenance({ cfg, env: { OPENCLAW_TEST: "1" }, + runChannelPluginStartupMaintenance: async (input) => { + calls.push(input); + }, runtime, shouldRepair: true, }); - expect(runChannelPluginStartupMaintenance).toHaveBeenCalledTimes(1); - expect(runChannelPluginStartupMaintenance).toHaveBeenCalledWith( + expect(calls).toHaveLength(1); + expect(calls[0]).toEqual( expect.objectContaining({ cfg, env: { OPENCLAW_TEST: "1" }, @@ -47,12 +41,17 @@ describe("doctor startup channel maintenance", () => { }); it("skips startup migration outside repair flows", async () => { + const calls: unknown[] = []; + await maybeRunDoctorStartupChannelMaintenance({ cfg: { channels: { matrix: {} } }, - runtime: { log: vi.fn(), error: vi.fn() }, + runChannelPluginStartupMaintenance: async (input) => { + calls.push(input); + }, + runtime: { log() {}, error() {} }, shouldRepair: false, }); - expect(runChannelPluginStartupMaintenance).not.toHaveBeenCalled(); + expect(calls).toEqual([]); }); }); diff --git a/src/flows/doctor-startup-channel-maintenance.ts b/src/flows/doctor-startup-channel-maintenance.ts index e9c52c55d5d..d9945d6e6f0 100644 --- a/src/flows/doctor-startup-channel-maintenance.ts +++ b/src/flows/doctor-startup-channel-maintenance.ts @@ -6,16 +6,21 @@ type DoctorStartupMaintenanceRuntime = { log: (message: string) => void; }; +type ChannelPluginStartupMaintenanceRunner = typeof runChannelPluginStartupMaintenance; + export async function maybeRunDoctorStartupChannelMaintenance(params: { cfg: OpenClawConfig; env?: NodeJS.ProcessEnv; + runChannelPluginStartupMaintenance?: ChannelPluginStartupMaintenanceRunner; runtime: DoctorStartupMaintenanceRuntime; shouldRepair: boolean; }): Promise { if (!params.shouldRepair) { return; } - await runChannelPluginStartupMaintenance({ + const runStartupMaintenance = + params.runChannelPluginStartupMaintenance ?? runChannelPluginStartupMaintenance; + await runStartupMaintenance({ cfg: params.cfg, env: params.env ?? process.env, log: { diff --git a/src/security/dangerous-config-flags-core.ts b/src/security/dangerous-config-flags-core.ts new file mode 100644 index 00000000000..1bd78a446d2 --- /dev/null +++ b/src/security/dangerous-config-flags-core.ts @@ -0,0 +1,143 @@ +import { DANGEROUS_SANDBOX_DOCKER_BOOLEAN_KEYS } from "../agents/sandbox/config.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { isRecord } from "../utils.js"; +import { collectCoreInsecureOrDangerousFlags } from "./core-dangerous-config-flags.js"; + +type DangerousFlagValue = string | number | boolean | null; + +type DangerousFlagContract = { + path: string; + equals: DangerousFlagValue; +}; + +type PluginConfigContractMetadata = { + configContracts: { + dangerousFlags?: DangerousFlagContract[]; + }; +}; + +type PluginConfigContractMatch = { + path: string; + value: unknown; +}; + +type CollectPluginConfigContractMatches = (input: { + pathPattern: string; + root: Record; +}) => Iterable; + +export type DangerousConfigFlagContractInputs = { + configContractsById?: ReadonlyMap; + collectPluginConfigContractMatches?: CollectPluginConfigContractMatches; +}; + +function formatDangerousConfigFlagValue(value: DangerousFlagValue): string { + return value === null ? "null" : String(value); +} + +function getAgentDangerousFlagPathSegment(agent: unknown, index: number): string { + const id = + agent && + typeof agent === "object" && + !Array.isArray(agent) && + typeof (agent as { id?: unknown }).id === "string" && + (agent as { id: string }).id.length > 0 + ? (agent as { id: string }).id + : undefined; + return id ? `agents.list[id=${JSON.stringify(id)}]` : `agents.list[${index}]`; +} + +function collectExactPluginConfigContractMatches({ + pathPattern, + root, +}: { + pathPattern: string; + root: Record; +}): PluginConfigContractMatch[] { + return Object.hasOwn(root, pathPattern) ? [{ path: pathPattern, value: root[pathPattern] }] : []; +} + +export function collectEnabledInsecureOrDangerousFlagsFromContracts( + cfg: OpenClawConfig, + inputs: DangerousConfigFlagContractInputs = {}, +): string[] { + const enabledFlags = collectCoreInsecureOrDangerousFlags(cfg); + + const collectSandboxDockerDangerousFlags = ( + docker: Record | undefined, + pathPrefix: string, + ): void => { + if (!isRecord(docker)) { + return; + } + for (const key of DANGEROUS_SANDBOX_DOCKER_BOOLEAN_KEYS) { + if (docker[key] === true) { + enabledFlags.push(`${pathPrefix}.${key}=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?.fs?.workspaceOnly === false) { + enabledFlags.push("tools.fs.workspaceOnly=false"); + } + collectSandboxDockerDangerousFlags( + isRecord(cfg.agents?.defaults?.sandbox?.docker) + ? cfg.agents?.defaults?.sandbox?.docker + : undefined, + "agents.defaults.sandbox.docker", + ); + if (Array.isArray(cfg.agents?.list)) { + for (const [index, agent] of cfg.agents.list.entries()) { + collectSandboxDockerDangerousFlags( + isRecord(agent?.sandbox?.docker) ? agent.sandbox.docker : undefined, + `${getAgentDangerousFlagPathSegment(agent, index)}.sandbox.docker`, + ); + } + } + + const pluginEntries = cfg.plugins?.entries; + if (!isRecord(pluginEntries)) { + return enabledFlags; + } + + const configContracts = inputs.configContractsById ?? new Map(); + const collectPluginConfigContractMatches = + inputs.collectPluginConfigContractMatches ?? collectExactPluginConfigContractMatches; + const seenFlags = new Set(); + for (const [pluginId, metadata] of configContracts.entries()) { + const dangerousFlags = metadata.configContracts.dangerousFlags; + if (!dangerousFlags?.length) { + continue; + } + const pluginEntry = pluginEntries[pluginId]; + if (!isRecord(pluginEntry) || !isRecord(pluginEntry.config)) { + continue; + } + for (const flag of dangerousFlags) { + for (const match of collectPluginConfigContractMatches({ + root: pluginEntry.config, + pathPattern: flag.path, + })) { + if (!Object.is(match.value, flag.equals)) { + continue; + } + const rendered = + `plugins.entries.${pluginId}.config.${match.path}` + + `=${formatDangerousConfigFlagValue(flag.equals)}`; + if (seenFlags.has(rendered)) { + continue; + } + seenFlags.add(rendered); + enabledFlags.push(rendered); + } + } + } + + return enabledFlags; +} diff --git a/src/security/dangerous-config-flags.test.ts b/src/security/dangerous-config-flags.test.ts index ae1bdb96902..bd14bf2ba13 100644 --- a/src/security/dangerous-config-flags.test.ts +++ b/src/security/dangerous-config-flags.test.ts @@ -1,48 +1,15 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; -import { collectEnabledInsecureOrDangerousFlags } from "./dangerous-config-flags.js"; - -const { resolvePluginConfigContractsByIdMock } = vi.hoisted(() => ({ - resolvePluginConfigContractsByIdMock: vi.fn(), -})); - -vi.mock("../plugins/config-contracts.js", () => ({ - collectPluginConfigContractMatches: ({ - pathPattern, - root, - }: { - pathPattern: string; - root: Record; - }) => (Object.hasOwn(root, pathPattern) ? [{ path: pathPattern, value: root[pathPattern] }] : []), - resolvePluginConfigContractsById: resolvePluginConfigContractsByIdMock, -})); +import { collectEnabledInsecureOrDangerousFlagsFromContracts } from "./dangerous-config-flags-core.js"; function asConfig(value: unknown): OpenClawConfig { return value as OpenClawConfig; } describe("collectEnabledInsecureOrDangerousFlags", () => { - beforeEach(() => { - resolvePluginConfigContractsByIdMock.mockReset(); - resolvePluginConfigContractsByIdMock.mockReturnValue(new Map()); - }); - it("collects manifest-declared dangerous plugin config values", () => { - resolvePluginConfigContractsByIdMock.mockReturnValue( - new Map([ - [ - "acpx", - { - configContracts: { - dangerousFlags: [{ path: "permissionMode", equals: "approve-all" }], - }, - }, - ], - ]), - ); - expect( - collectEnabledInsecureOrDangerousFlags( + collectEnabledInsecureOrDangerousFlagsFromContracts( asConfig({ plugins: { entries: { @@ -54,26 +21,25 @@ describe("collectEnabledInsecureOrDangerousFlags", () => { }, }, }), + { + configContractsById: new Map([ + [ + "acpx", + { + configContracts: { + dangerousFlags: [{ path: "permissionMode", equals: "approve-all" }], + }, + }, + ], + ]), + }, ), ).toContain("plugins.entries.acpx.config.permissionMode=approve-all"); }); it("ignores plugin config values that are not declared as dangerous", () => { - resolvePluginConfigContractsByIdMock.mockReturnValue( - new Map([ - [ - "other", - { - configContracts: { - dangerousFlags: [{ path: "mode", equals: "danger" }], - }, - }, - ], - ]), - ); - expect( - collectEnabledInsecureOrDangerousFlags( + collectEnabledInsecureOrDangerousFlagsFromContracts( asConfig({ plugins: { entries: { @@ -85,13 +51,25 @@ describe("collectEnabledInsecureOrDangerousFlags", () => { }, }, }), + { + configContractsById: new Map([ + [ + "other", + { + configContracts: { + dangerousFlags: [{ path: "mode", equals: "danger" }], + }, + }, + ], + ]), + }, ), ).toEqual([]); }); it("collects dangerous sandbox, hook, browser, and fs flags", () => { expect( - collectEnabledInsecureOrDangerousFlags( + collectEnabledInsecureOrDangerousFlagsFromContracts( asConfig({ agents: { defaults: { @@ -142,7 +120,7 @@ describe("collectEnabledInsecureOrDangerousFlags", () => { it("uses stable agent ids for per-agent dangerous sandbox flags", () => { expect( - collectEnabledInsecureOrDangerousFlags( + collectEnabledInsecureOrDangerousFlagsFromContracts( asConfig({ agents: { list: [ @@ -166,7 +144,7 @@ describe("collectEnabledInsecureOrDangerousFlags", () => { ); expect( - collectEnabledInsecureOrDangerousFlags( + collectEnabledInsecureOrDangerousFlagsFromContracts( asConfig({ agents: { list: [ diff --git a/src/security/dangerous-config-flags.ts b/src/security/dangerous-config-flags.ts index ca4e90ea376..e14e46bae98 100644 --- a/src/security/dangerous-config-flags.ts +++ b/src/security/dangerous-config-flags.ts @@ -1,73 +1,16 @@ import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../agents/agent-scope.js"; -import { DANGEROUS_SANDBOX_DOCKER_BOOLEAN_KEYS } from "../agents/sandbox/config.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { collectPluginConfigContractMatches, 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); -} - -function getAgentDangerousFlagPathSegment(agent: unknown, index: number): string { - const id = - agent && - typeof agent === "object" && - !Array.isArray(agent) && - typeof (agent as { id?: unknown }).id === "string" && - (agent as { id: string }).id.length > 0 - ? (agent as { id: string }).id - : undefined; - return id ? `agents.list[id=${JSON.stringify(id)}]` : `agents.list[${index}]`; -} +import { collectEnabledInsecureOrDangerousFlagsFromContracts } from "./dangerous-config-flags-core.js"; export function collectEnabledInsecureOrDangerousFlags(cfg: OpenClawConfig): string[] { - const enabledFlags = collectCoreInsecureOrDangerousFlags(cfg); - - const collectSandboxDockerDangerousFlags = ( - docker: Record | undefined, - pathPrefix: string, - ): void => { - if (!isRecord(docker)) { - return; - } - for (const key of DANGEROUS_SANDBOX_DOCKER_BOOLEAN_KEYS) { - if (docker[key] === true) { - enabledFlags.push(`${pathPrefix}.${key}=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?.fs?.workspaceOnly === false) { - enabledFlags.push("tools.fs.workspaceOnly=false"); - } - collectSandboxDockerDangerousFlags( - isRecord(cfg.agents?.defaults?.sandbox?.docker) - ? cfg.agents?.defaults?.sandbox?.docker - : undefined, - "agents.defaults.sandbox.docker", - ); - if (Array.isArray(cfg.agents?.list)) { - for (const [index, agent] of cfg.agents.list.entries()) { - collectSandboxDockerDangerousFlags( - isRecord(agent?.sandbox?.docker) ? agent.sandbox.docker : undefined, - `${getAgentDangerousFlagPathSegment(agent, index)}.sandbox.docker`, - ); - } - } - const pluginEntries = cfg.plugins?.entries; if (!isRecord(pluginEntries)) { - return enabledFlags; + return collectEnabledInsecureOrDangerousFlagsFromContracts(cfg); } const configContracts = resolvePluginConfigContractsById({ @@ -77,35 +20,8 @@ export function collectEnabledInsecureOrDangerousFlags(cfg: OpenClawConfig): str cache: true, pluginIds: Object.keys(pluginEntries), }); - const seenFlags = new Set(); - for (const [pluginId, metadata] of configContracts.entries()) { - const dangerousFlags = metadata.configContracts.dangerousFlags; - if (!dangerousFlags?.length) { - continue; - } - const pluginEntry = pluginEntries[pluginId]; - if (!isRecord(pluginEntry) || !isRecord(pluginEntry.config)) { - continue; - } - for (const flag of dangerousFlags) { - for (const match of collectPluginConfigContractMatches({ - root: pluginEntry.config, - pathPattern: flag.path, - })) { - if (!Object.is(match.value, flag.equals)) { - continue; - } - const rendered = - `plugins.entries.${pluginId}.config.${match.path}` + - `=${formatDangerousConfigFlagValue(flag.equals)}`; - if (seenFlags.has(rendered)) { - continue; - } - seenFlags.add(rendered); - enabledFlags.push(rendered); - } - } - } - - return enabledFlags; + return collectEnabledInsecureOrDangerousFlagsFromContracts(cfg, { + collectPluginConfigContractMatches, + configContractsById: configContracts, + }); } diff --git a/test/vitest-unit-fast-config.test.ts b/test/vitest-unit-fast-config.test.ts index 4073c185979..bf70e2a01be 100644 --- a/test/vitest-unit-fast-config.test.ts +++ b/test/vitest-unit-fast-config.test.ts @@ -26,8 +26,11 @@ describe("unit-fast vitest lane", () => { expect(config.test?.include).toContain("src/acp/control-plane/runtime-cache.test.ts"); expect(config.test?.include).toContain("src/acp/runtime/registry.test.ts"); expect(config.test?.include).toContain("src/commands/status-overview-values.test.ts"); + expect(config.test?.include).toContain("src/flows/doctor-startup-channel-maintenance.test.ts"); expect(config.test?.include).toContain("src/plugins/config-policy.test.ts"); expect(config.test?.include).toContain("src/plugin-sdk/provider-entry.test.ts"); + expect(config.test?.include).toContain("src/security/dangerous-config-flags.test.ts"); + expect(config.test?.include).toContain("src/security/safe-regex.test.ts"); }); it("does not treat moved config paths as CLI include filters", () => { diff --git a/test/vitest/vitest.unit-fast-paths.mjs b/test/vitest/vitest.unit-fast-paths.mjs index a348a56bb9c..772cc9205cb 100644 --- a/test/vitest/vitest.unit-fast-paths.mjs +++ b/test/vitest/vitest.unit-fast-paths.mjs @@ -92,6 +92,7 @@ export const forcedUnitFastTestFiles = [ "src/dockerfile.test.ts", "src/entry.compile-cache.test.ts", "src/entry.test.ts", + "src/flows/doctor-startup-channel-maintenance.test.ts", "src/i18n/registry.test.ts", "src/image-generation/openai-compatible-image-provider.test.ts", "src/install-sh-version.test.ts", @@ -124,10 +125,12 @@ export const forcedUnitFastTestFiles = [ "src/security/audit-channel-readonly-resolution.test.ts", "src/security/audit-exec-surface.test.ts", "src/security/audit-exec-safe-bins.test.ts", + "src/security/dangerous-config-flags.test.ts", "src/security/audit-extra.sync.test.ts", "src/security/audit-filesystem-windows.test.ts", "src/security/audit-gateway-exposure.test.ts", "src/security/audit-sandbox-docker-config.test.ts", + "src/security/safe-regex.test.ts", "src/security/audit-small-model-risk.test.ts", "src/security/audit-node-command-findings.test.ts", "src/security/audit-extra.async.test.ts",