From 5dc50b8a3f80089b41525bd43e878da1ab8c4cfb Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 19 Feb 2026 15:10:57 +0100 Subject: [PATCH] fix(security): harden npm plugin and hook install integrity flow --- CHANGELOG.md | 1 + docs/cli/hooks.md | 6 + docs/cli/plugins.md | 8 + docs/cli/security.md | 1 + docs/tools/plugin.md | 2 + src/cli/hooks-cli.ts | 58 ++- src/cli/plugins-cli.ts | 39 +- src/commands/onboarding/plugin-install.ts | 12 +- src/config/schema.help.ts | 11 + src/config/schema.labels.ts | 6 + src/config/types.hooks.ts | 6 + src/config/types.plugins.ts | 6 + src/config/zod-schema.installs.ts | 6 + src/hooks/install.test.ts | 53 ++- src/hooks/install.ts | 61 ++- src/infra/install-source-utils.test.ts | 55 ++- src/infra/install-source-utils.ts | 143 ++++++- src/plugins/install.e2e.test.ts | 60 ++- src/plugins/install.ts | 61 ++- src/plugins/update.ts | 61 +++ src/security/audit-extra.async.ts | 433 +++++++++++++++------- src/security/audit.test.ts | 139 ++++++- src/test-utils/exec-assertions.ts | 2 +- 23 files changed, 1047 insertions(+), 183 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be92e53993..acf0e90833 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Security/ACP: harden ACP bridge session management with duplicate-session refresh, idle-session reaping, oldest-idle soft-cap eviction, and burst rate limiting on session creation to reduce local DoS risk without disrupting normal IDE usage. +- Security/Plugins/Hooks: add optional `--pin` for npm plugin/hook installs, persist resolved npm metadata (`name`, `version`, `spec`, integrity, shasum, timestamp), warn/confirm on integrity drift during updates, and extend `openclaw security audit` to flag unpinned specs, missing integrity metadata, and install-record version drift. - Security/Gateway: rate-limit control-plane write RPCs (`config.apply`, `config.patch`, `update.run`) to 3 requests per minute per `deviceId+clientIp`, add restart single-flight coalescing plus a 30-second restart cooldown, and log actor/device/ip with changed-path audit details for config/update-triggered restarts. - Commands/Doctor: skip embedding-provider warnings when `memory.backend` is `qmd`, because QMD manages embeddings internally and does not require `memorySearch` providers. (#17263) Thanks @miloudbelarebia. - Security/Webhooks: harden Feishu and Zalo webhook ingress with webhook-mode token preconditions, loopback-default Feishu bind host, JSON content-type enforcement, per-path rate limiting, replay dedupe for Zalo events, constant-time Zalo secret comparison, and anomaly status counters. diff --git a/docs/cli/hooks.md b/docs/cli/hooks.md index a676a709ac..6dadb26970 100644 --- a/docs/cli/hooks.md +++ b/docs/cli/hooks.md @@ -188,6 +188,7 @@ openclaw hooks disable command-logger ```bash openclaw hooks install +openclaw hooks install --pin ``` Install a hook pack from a local folder/archive or npm. @@ -204,6 +205,7 @@ specs are rejected. Dependency installs run with `--ignore-scripts` for safety. **Options:** - `-l, --link`: Link a local directory instead of copying (adds it to `hooks.internal.load.extraDirs`) +- `--pin`: Record npm installs as exact resolved `name@version` in `hooks.internal.installs` **Supported archives:** `.zip`, `.tgz`, `.tar.gz`, `.tar` @@ -237,6 +239,10 @@ Update installed hook packs (npm installs only). - `--all`: Update all tracked hook packs - `--dry-run`: Show what would change without writing +When a stored integrity hash exists and the fetched artifact hash changes, +OpenClaw prints a warning and asks for confirmation before proceeding. Use +global `--yes` to bypass prompts in CI/non-interactive runs. + ## Bundled Hooks ### session-memory diff --git a/docs/cli/plugins.md b/docs/cli/plugins.md index cc7eeb18f9..6f3cb103cf 100644 --- a/docs/cli/plugins.md +++ b/docs/cli/plugins.md @@ -40,6 +40,7 @@ the plugin from loading and fail config validation. ```bash openclaw plugins install +openclaw plugins install --pin ``` Security note: treat plugin installs like running code. Prefer pinned versions. @@ -55,6 +56,9 @@ Use `--link` to avoid copying a local directory (adds to `plugins.load.paths`): openclaw plugins install -l ./my-plugin ``` +Use `--pin` on npm installs to save the resolved exact spec (`name@version`) in +`plugins.installs` while keeping the default behavior unpinned. + ### Uninstall ```bash @@ -82,3 +86,7 @@ openclaw plugins update --dry-run ``` Updates only apply to plugins installed from npm (tracked in `plugins.installs`). + +When a stored integrity hash exists and the fetched artifact hash changes, +OpenClaw prints a warning and asks for confirmation before proceeding. Use +global `--yes` to bypass prompts in CI/non-interactive runs. diff --git a/docs/cli/security.md b/docs/cli/security.md index aed7d3c190..fee2023006 100644 --- a/docs/cli/security.md +++ b/docs/cli/security.md @@ -27,6 +27,7 @@ The audit warns when multiple DM senders share the main session and recommends * It also warns when small models (`<=300B`) are used without sandboxing and with web/browser tools enabled. For webhook ingress, it warns when `hooks.defaultSessionKey` is unset, when request `sessionKey` overrides are enabled, and when overrides are enabled without `hooks.allowedSessionKeyPrefixes`. It also warns when sandbox Docker settings are configured while sandbox mode is off, when `gateway.nodes.denyCommands` uses ineffective pattern-like/unknown entries, when global `tools.profile="minimal"` is overridden by agent tool profiles, and when installed extension plugin tools may be reachable under permissive tool policy. +It also warns when npm-based plugin/hook install records are unpinned, missing integrity metadata, or drift from currently installed package versions. It warns when `gateway.auth.mode="none"` leaves Gateway HTTP APIs reachable without a shared secret (`/tools/invoke` plus any enabled `/v1/*` endpoint). ## JSON output diff --git a/docs/tools/plugin.md b/docs/tools/plugin.md index 2135ce8e88..ab031c389b 100644 --- a/docs/tools/plugin.md +++ b/docs/tools/plugin.md @@ -295,6 +295,7 @@ openclaw plugins install ./plugin.tgz # install from a local tarball openclaw plugins install ./plugin.zip # install from a local zip openclaw plugins install -l ./extensions/voice-call # link (no copy) for dev openclaw plugins install @openclaw/voice-call # install from npm +openclaw plugins install @openclaw/voice-call --pin # store exact resolved name@version openclaw plugins update openclaw plugins update --all openclaw plugins enable @@ -303,6 +304,7 @@ openclaw plugins doctor ``` `plugins update` only works for npm installs tracked under `plugins.installs`. +If stored integrity metadata changes between updates, OpenClaw warns and asks for confirmation (use global `--yes` to bypass prompts). Plugins may also register their own top‑level commands (example: `openclaw voicecall`). diff --git a/src/cli/hooks-cli.ts b/src/cli/hooks-cli.ts index ffa7383c5e..dfa578cc9b 100644 --- a/src/cli/hooks-cli.ts +++ b/src/cli/hooks-cli.ts @@ -1,9 +1,10 @@ +import type { Command } from "commander"; import fs from "node:fs"; import fsp from "node:fs/promises"; import path from "node:path"; -import type { Command } from "commander"; -import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../agents/agent-scope.js"; import type { OpenClawConfig } from "../config/config.js"; +import type { HookEntry } from "../hooks/types.js"; +import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../agents/agent-scope.js"; import { loadConfig, writeConfigFile } from "../config/io.js"; import { buildWorkspaceHookStatus, @@ -16,7 +17,6 @@ import { resolveHookInstallDir, } from "../hooks/install.js"; import { recordHookInstall } from "../hooks/installs.js"; -import type { HookEntry } from "../hooks/types.js"; import { loadWorkspaceHookEntries } from "../hooks/workspace.js"; import { resolveArchiveKind } from "../infra/archive.js"; import { buildPluginStatusReport } from "../plugins/status.js"; @@ -26,6 +26,7 @@ import { renderTable } from "../terminal/table.js"; import { theme } from "../terminal/theme.js"; import { resolveUserPath, shortenHomePath } from "../utils.js"; import { formatCliCommand } from "./command-format.js"; +import { promptYesNo } from "./prompt.js"; export type HooksListOptions = { json?: boolean; @@ -550,7 +551,8 @@ export function registerHooksCli(program: Command): void { .description("Install a hook pack (path, archive, or npm spec)") .argument("", "Path to a hook pack or npm package spec") .option("-l, --link", "Link a local path instead of copying", false) - .action(async (raw: string, opts: { link?: boolean }) => { + .option("--pin", "Record npm installs as exact resolved @", false) + .action(async (raw: string, opts: { link?: boolean; pin?: boolean }) => { const resolved = resolveUserPath(raw); const cfg = loadConfig(); @@ -658,13 +660,29 @@ export function registerHooksCli(program: Command): void { } let next = enableInternalHookEntries(cfg, result.hooks); + const resolvedSpec = result.npmResolution?.resolvedSpec; + const recordSpec = opts.pin && resolvedSpec ? resolvedSpec : raw; + if (opts.pin && !resolvedSpec) { + defaultRuntime.log( + theme.warn("Could not resolve exact npm version for --pin; storing original npm spec."), + ); + } + if (opts.pin && resolvedSpec) { + defaultRuntime.log(`Pinned npm install record to ${resolvedSpec}.`); + } next = recordHookInstall(next, { hookId: result.hookPackId, source: "npm", - spec: raw, + spec: recordSpec, installPath: result.targetDir, version: result.version, + resolvedName: result.npmResolution?.name, + resolvedVersion: result.npmResolution?.version, + resolvedSpec: result.npmResolution?.resolvedSpec, + integrity: result.npmResolution?.integrity, + shasum: result.npmResolution?.shasum, + resolvedAt: result.npmResolution?.resolvedAt, hooks: result.hooks, }); await writeConfigFile(next); @@ -721,6 +739,18 @@ export function registerHooksCli(program: Command): void { mode: "update", dryRun: true, expectedHookPackId: hookId, + expectedIntegrity: record.integrity, + onIntegrityDrift: async (drift) => { + const specLabel = drift.resolution.resolvedSpec ?? drift.spec; + defaultRuntime.log( + theme.warn( + `Integrity drift detected for "${hookId}" (${specLabel})` + + `\nExpected: ${drift.expectedIntegrity}` + + `\nActual: ${drift.actualIntegrity}`, + ), + ); + return true; + }, logger: createInstallLogger(), }); if (!probe.ok) { @@ -742,6 +772,18 @@ export function registerHooksCli(program: Command): void { spec: record.spec, mode: "update", expectedHookPackId: hookId, + expectedIntegrity: record.integrity, + onIntegrityDrift: async (drift) => { + const specLabel = drift.resolution.resolvedSpec ?? drift.spec; + defaultRuntime.log( + theme.warn( + `Integrity drift detected for "${hookId}" (${specLabel})` + + `\nExpected: ${drift.expectedIntegrity}` + + `\nActual: ${drift.actualIntegrity}`, + ), + ); + return await promptYesNo(`Continue updating "${hookId}" with this artifact?`); + }, logger: createInstallLogger(), }); if (!result.ok) { @@ -756,6 +798,12 @@ export function registerHooksCli(program: Command): void { spec: record.spec, installPath: result.targetDir, version: nextVersion, + resolvedName: result.npmResolution?.name, + resolvedVersion: result.npmResolution?.version, + resolvedSpec: result.npmResolution?.resolvedSpec, + integrity: result.npmResolution?.integrity, + shasum: result.npmResolution?.shasum, + resolvedAt: result.npmResolution?.resolvedAt, hooks: result.hooks, }); updatedCount += 1; diff --git a/src/cli/plugins-cli.ts b/src/cli/plugins-cli.ts index bca81b40af..dba4b58b82 100644 --- a/src/cli/plugins-cli.ts +++ b/src/cli/plugins-cli.ts @@ -1,15 +1,15 @@ +import type { Command } from "commander"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import type { Command } from "commander"; import type { OpenClawConfig } from "../config/config.js"; +import type { PluginRecord } from "../plugins/registry.js"; import { loadConfig, writeConfigFile } from "../config/config.js"; import { resolveStateDir } from "../config/paths.js"; import { resolveArchiveKind } from "../infra/archive.js"; import { installPluginFromNpmSpec, installPluginFromPath } from "../plugins/install.js"; import { recordPluginInstall } from "../plugins/installs.js"; import { clearPluginManifestRegistryCache } from "../plugins/manifest-registry.js"; -import type { PluginRecord } from "../plugins/registry.js"; import { applyExclusiveSlotSelection } from "../plugins/slots.js"; import { resolvePluginSourceRoots, formatPluginSourceForTable } from "../plugins/source-display.js"; import { buildPluginStatusReport } from "../plugins/status.js"; @@ -535,7 +535,8 @@ export function registerPluginsCli(program: Command) { .description("Install a plugin (path, archive, or npm spec)") .argument("", "Path (.ts/.js/.zip/.tgz/.tar.gz) or an npm package spec") .option("-l, --link", "Link a local path instead of copying", false) - .action(async (raw: string, opts: { link?: boolean }) => { + .option("--pin", "Record npm installs as exact resolved @", false) + .action(async (raw: string, opts: { link?: boolean; pin?: boolean }) => { const fileSpec = resolveFileNpmSpecToLocalPath(raw); if (fileSpec && !fileSpec.ok) { defaultRuntime.error(fileSpec.error); @@ -648,12 +649,28 @@ export function registerPluginsCli(program: Command) { clearPluginManifestRegistryCache(); let next = enablePluginInConfig(cfg, result.pluginId); + const resolvedSpec = result.npmResolution?.resolvedSpec; + const recordSpec = opts.pin && resolvedSpec ? resolvedSpec : raw; + if (opts.pin && !resolvedSpec) { + defaultRuntime.log( + theme.warn("Could not resolve exact npm version for --pin; storing original npm spec."), + ); + } + if (opts.pin && resolvedSpec) { + defaultRuntime.log(`Pinned npm install record to ${resolvedSpec}.`); + } next = recordPluginInstall(next, { pluginId: result.pluginId, source: "npm", - spec: raw, + spec: recordSpec, installPath: result.targetDir, version: result.version, + resolvedName: result.npmResolution?.name, + resolvedVersion: result.npmResolution?.version, + resolvedSpec: result.npmResolution?.resolvedSpec, + integrity: result.npmResolution?.integrity, + shasum: result.npmResolution?.shasum, + resolvedAt: result.npmResolution?.resolvedAt, }); const slotResult = applySlotSelectionForPlugin(next, result.pluginId); next = slotResult.config; @@ -691,6 +708,20 @@ export function registerPluginsCli(program: Command) { info: (msg) => defaultRuntime.log(msg), warn: (msg) => defaultRuntime.log(theme.warn(msg)), }, + onIntegrityDrift: async (drift) => { + const specLabel = drift.resolvedSpec ?? drift.spec; + defaultRuntime.log( + theme.warn( + `Integrity drift detected for "${drift.pluginId}" (${specLabel})` + + `\nExpected: ${drift.expectedIntegrity}` + + `\nActual: ${drift.actualIntegrity}`, + ), + ); + if (drift.dryRun) { + return true; + } + return await promptYesNo(`Continue updating "${drift.pluginId}" with this artifact?`); + }, }); for (const outcome of result.outcomes) { diff --git a/src/commands/onboarding/plugin-install.ts b/src/commands/onboarding/plugin-install.ts index 6f5f6b9e56..9714d18550 100644 --- a/src/commands/onboarding/plugin-install.ts +++ b/src/commands/onboarding/plugin-install.ts @@ -1,16 +1,16 @@ import fs from "node:fs"; import path from "node:path"; -import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../../agents/agent-scope.js"; import type { ChannelPluginCatalogEntry } from "../../channels/plugins/catalog.js"; import type { OpenClawConfig } from "../../config/config.js"; +import type { RuntimeEnv } from "../../runtime.js"; +import type { WizardPrompter } from "../../wizard/prompts.js"; +import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../../agents/agent-scope.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { enablePluginInConfig } from "../../plugins/enable.js"; import { installPluginFromNpmSpec } from "../../plugins/install.js"; import { recordPluginInstall } from "../../plugins/installs.js"; import { loadOpenClawPlugins } from "../../plugins/loader.js"; import { createPluginLoaderLogger } from "../../plugins/logger.js"; -import type { RuntimeEnv } from "../../runtime.js"; -import type { WizardPrompter } from "../../wizard/prompts.js"; type InstallChoice = "npm" | "local" | "skip"; @@ -175,6 +175,12 @@ export async function ensureOnboardingPluginInstalled(params: { spec: entry.install.npmSpec, installPath: result.targetDir, version: result.version, + resolvedName: result.npmResolution?.name, + resolvedVersion: result.npmResolution?.version, + resolvedSpec: result.npmResolution?.resolvedSpec, + integrity: result.npmResolution?.integrity, + shasum: result.npmResolution?.shasum, + resolvedAt: result.npmResolution?.resolvedAt, }); return { cfg: next, installed: true }; } diff --git a/src/config/schema.help.ts b/src/config/schema.help.ts index 2a59c153b0..48d54abd1e 100644 --- a/src/config/schema.help.ts +++ b/src/config/schema.help.ts @@ -283,6 +283,17 @@ export const FIELD_HELP: Record = { "plugins.installs.*.installPath": "Resolved install directory (usually ~/.openclaw/extensions/).", "plugins.installs.*.version": "Version recorded at install time (if available).", + "plugins.installs.*.resolvedName": "Resolved npm package name from the fetched artifact.", + "plugins.installs.*.resolvedVersion": + "Resolved npm package version from the fetched artifact (useful for non-pinned specs).", + "plugins.installs.*.resolvedSpec": + "Resolved exact npm spec (@) from the fetched artifact.", + "plugins.installs.*.integrity": + "Resolved npm dist integrity hash for the fetched artifact (if reported by npm).", + "plugins.installs.*.shasum": + "Resolved npm dist shasum for the fetched artifact (if reported by npm).", + "plugins.installs.*.resolvedAt": + "ISO timestamp when npm package metadata was last resolved for this install record.", "plugins.installs.*.installedAt": "ISO timestamp of last install/update.", "agents.list.*.identity.avatar": "Agent avatar (workspace-relative path, http(s) URL, or data URI).", diff --git a/src/config/schema.labels.ts b/src/config/schema.labels.ts index e84abca057..277cecfcfd 100644 --- a/src/config/schema.labels.ts +++ b/src/config/schema.labels.ts @@ -322,5 +322,11 @@ export const FIELD_LABELS: Record = { "plugins.installs.*.sourcePath": "Plugin Install Source Path", "plugins.installs.*.installPath": "Plugin Install Path", "plugins.installs.*.version": "Plugin Install Version", + "plugins.installs.*.resolvedName": "Plugin Resolved Package Name", + "plugins.installs.*.resolvedVersion": "Plugin Resolved Package Version", + "plugins.installs.*.resolvedSpec": "Plugin Resolved Package Spec", + "plugins.installs.*.integrity": "Plugin Resolved Integrity", + "plugins.installs.*.shasum": "Plugin Resolved Shasum", + "plugins.installs.*.resolvedAt": "Plugin Resolution Time", "plugins.installs.*.installedAt": "Plugin Install Time", }; diff --git a/src/config/types.hooks.ts b/src/config/types.hooks.ts index 3e13931cd6..2345c9ba7d 100644 --- a/src/config/types.hooks.ts +++ b/src/config/types.hooks.ts @@ -93,6 +93,12 @@ export type HookInstallRecord = { sourcePath?: string; installPath?: string; version?: string; + resolvedName?: string; + resolvedVersion?: string; + resolvedSpec?: string; + integrity?: string; + shasum?: string; + resolvedAt?: string; installedAt?: string; hooks?: string[]; }; diff --git a/src/config/types.plugins.ts b/src/config/types.plugins.ts index 68268683e1..f1e4211b1f 100644 --- a/src/config/types.plugins.ts +++ b/src/config/types.plugins.ts @@ -27,6 +27,12 @@ export type PluginInstallRecord = { sourcePath?: string; installPath?: string; version?: string; + resolvedName?: string; + resolvedVersion?: string; + resolvedSpec?: string; + integrity?: string; + shasum?: string; + resolvedAt?: string; installedAt?: string; }; diff --git a/src/config/zod-schema.installs.ts b/src/config/zod-schema.installs.ts index 3c2710096f..7853948a10 100644 --- a/src/config/zod-schema.installs.ts +++ b/src/config/zod-schema.installs.ts @@ -12,5 +12,11 @@ export const InstallRecordShape = { sourcePath: z.string().optional(), installPath: z.string().optional(), version: z.string().optional(), + resolvedName: z.string().optional(), + resolvedVersion: z.string().optional(), + resolvedSpec: z.string().optional(), + integrity: z.string().optional(), + shasum: z.string().optional(), + resolvedAt: z.string().optional(), installedAt: z.string().optional(), } as const; diff --git a/src/hooks/install.test.ts b/src/hooks/install.test.ts index 20e26a7fe8..b8ef3f761e 100644 --- a/src/hooks/install.test.ts +++ b/src/hooks/install.test.ts @@ -253,7 +253,16 @@ describe("installHooksFromNpmSpec", () => { fs.writeFileSync(path.join(packTmpDir, packedName), npmPackHooksBuffer); return { code: 0, - stdout: `${packedName}\n`, + stdout: JSON.stringify([ + { + id: "@openclaw/test-hooks@0.0.1", + name: "@openclaw/test-hooks", + version: "0.0.1", + filename: packedName, + integrity: "sha512-hook-test", + shasum: "hookshasum", + }, + ]), stderr: "", signal: null, killed: false, @@ -274,6 +283,8 @@ describe("installHooksFromNpmSpec", () => { return; } expect(result.hookPackId).toBe("test-hooks"); + expect(result.npmResolution?.resolvedSpec).toBe("@openclaw/test-hooks@0.0.1"); + expect(result.npmResolution?.integrity).toBe("sha512-hook-test"); expect(fs.existsSync(path.join(result.targetDir, "hooks", "one-hook", "HOOK.md"))).toBe(true); expectSingleNpmPackIgnoreScriptsCall({ @@ -293,6 +304,46 @@ describe("installHooksFromNpmSpec", () => { } expect(result.error).toContain("unsupported npm spec"); }); + + it("aborts when integrity drift callback rejects the fetched artifact", async () => { + const run = vi.mocked(runCommandWithTimeout); + run.mockResolvedValue({ + code: 0, + stdout: JSON.stringify([ + { + id: "@openclaw/test-hooks@0.0.1", + name: "@openclaw/test-hooks", + version: "0.0.1", + filename: "test-hooks-0.0.1.tgz", + integrity: "sha512-new", + shasum: "newshasum", + }, + ]), + stderr: "", + signal: null, + killed: false, + termination: "exit", + }); + + const onIntegrityDrift = vi.fn(async () => false); + const result = await installHooksFromNpmSpec({ + spec: "@openclaw/test-hooks@0.0.1", + expectedIntegrity: "sha512-old", + onIntegrityDrift, + }); + + expect(onIntegrityDrift).toHaveBeenCalledWith( + expect.objectContaining({ + expectedIntegrity: "sha512-old", + actualIntegrity: "sha512-new", + }), + ); + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + expect(result.error).toContain("integrity drift"); + }); }); describe("gmail watcher", () => { diff --git a/src/hooks/install.ts b/src/hooks/install.ts index 2a1daac746..6bd0614640 100644 --- a/src/hooks/install.ts +++ b/src/hooks/install.ts @@ -11,6 +11,8 @@ import { import { installPackageDir } from "../infra/install-package-dir.js"; import { resolveSafeInstallDir, unscopedPackageName } from "../infra/install-safe-path.js"; import { + type NpmIntegrityDrift, + type NpmSpecResolution, packNpmSpecToArchive, resolveArchiveSourcePath, withTempDir, @@ -37,9 +39,18 @@ export type InstallHooksResult = hooks: string[]; targetDir: string; version?: string; + npmResolution?: NpmSpecResolution; + integrityDrift?: NpmIntegrityDrift; } | { ok: false; error: string }; +export type HookNpmIntegrityDriftParams = { + spec: string; + expectedIntegrity: string; + actualIntegrity: string; + resolution: NpmSpecResolution; +}; + const defaultLogger: HookInstallLogger = {}; function validateHookId(hookId: string): string | null { @@ -375,6 +386,8 @@ export async function installHooksFromNpmSpec(params: { mode?: "install" | "update"; dryRun?: boolean; expectedHookPackId?: string; + expectedIntegrity?: string; + onIntegrityDrift?: (params: HookNpmIntegrityDriftParams) => boolean | Promise; }): Promise { const { logger, timeoutMs, mode, dryRun } = resolveTimedHookInstallModeOptions(params); const expectedHookPackId = params.expectedHookPackId; @@ -395,7 +408,44 @@ export async function installHooksFromNpmSpec(params: { return packedResult; } - return await installHooksFromArchive({ + const npmResolution: NpmSpecResolution = { + ...packedResult.metadata, + resolvedAt: new Date().toISOString(), + }; + + let integrityDrift: NpmIntegrityDrift | undefined; + if ( + params.expectedIntegrity && + npmResolution.integrity && + params.expectedIntegrity !== npmResolution.integrity + ) { + integrityDrift = { + expectedIntegrity: params.expectedIntegrity, + actualIntegrity: npmResolution.integrity, + }; + const driftPayload: HookNpmIntegrityDriftParams = { + spec, + expectedIntegrity: integrityDrift.expectedIntegrity, + actualIntegrity: integrityDrift.actualIntegrity, + resolution: npmResolution, + }; + let proceed = true; + if (params.onIntegrityDrift) { + proceed = await params.onIntegrityDrift(driftPayload); + } else { + logger.warn?.( + `Integrity drift detected for ${driftPayload.resolution.resolvedSpec ?? driftPayload.spec}: expected ${driftPayload.expectedIntegrity}, got ${driftPayload.actualIntegrity}`, + ); + } + if (!proceed) { + return { + ok: false, + error: `aborted: npm package integrity drift detected for ${driftPayload.resolution.resolvedSpec ?? driftPayload.spec}`, + }; + } + } + + const installResult = await installHooksFromArchive({ archivePath: packedResult.archivePath, hooksDir: params.hooksDir, timeoutMs, @@ -404,6 +454,15 @@ export async function installHooksFromNpmSpec(params: { dryRun, expectedHookPackId, }); + if (!installResult.ok) { + return installResult; + } + + return { + ...installResult, + npmResolution, + integrityDrift, + }; }); } diff --git a/src/infra/install-source-utils.test.ts b/src/infra/install-source-utils.test.ts index 143572479a..a46c6c9aab 100644 --- a/src/infra/install-source-utils.test.ts +++ b/src/infra/install-source-utils.test.ts @@ -85,7 +85,52 @@ describe("resolveArchiveSourcePath", () => { }); describe("packNpmSpecToArchive", () => { - it("packs spec and returns archive path using the final non-empty stdout line", async () => { + it("packs spec and returns archive path using JSON output metadata", async () => { + const cwd = await createTempDir("openclaw-install-source-utils-"); + runCommandWithTimeoutMock.mockResolvedValue({ + stdout: JSON.stringify([ + { + id: "openclaw-plugin@1.2.3", + name: "openclaw-plugin", + version: "1.2.3", + filename: "openclaw-plugin-1.2.3.tgz", + integrity: "sha512-test-integrity", + shasum: "abc123", + }, + ]), + stderr: "", + code: 0, + signal: null, + killed: false, + }); + + const result = await packNpmSpecToArchive({ + spec: "openclaw-plugin@1.2.3", + timeoutMs: 1000, + cwd, + }); + + expect(result).toEqual({ + ok: true, + archivePath: path.join(cwd, "openclaw-plugin-1.2.3.tgz"), + metadata: { + name: "openclaw-plugin", + version: "1.2.3", + resolvedSpec: "openclaw-plugin@1.2.3", + integrity: "sha512-test-integrity", + shasum: "abc123", + }, + }); + expect(runCommandWithTimeoutMock).toHaveBeenCalledWith( + ["npm", "pack", "openclaw-plugin@1.2.3", "--ignore-scripts", "--json"], + expect.objectContaining({ + cwd, + timeoutMs: 300_000, + }), + ); + }); + + it("falls back to parsing final stdout line when npm json output is unavailable", async () => { const cwd = await createTempDir("openclaw-install-source-utils-"); runCommandWithTimeoutMock.mockResolvedValue({ stdout: "npm notice created package\nopenclaw-plugin-1.2.3.tgz\n", @@ -104,14 +149,8 @@ describe("packNpmSpecToArchive", () => { expect(result).toEqual({ ok: true, archivePath: path.join(cwd, "openclaw-plugin-1.2.3.tgz"), + metadata: {}, }); - expect(runCommandWithTimeoutMock).toHaveBeenCalledWith( - ["npm", "pack", "openclaw-plugin@1.2.3", "--ignore-scripts"], - expect.objectContaining({ - cwd, - timeoutMs: 300_000, - }), - ); }); it("returns npm pack error details when command fails", async () => { diff --git a/src/infra/install-source-utils.ts b/src/infra/install-source-utils.ts index f51be1e7e4..d4a2ac025d 100644 --- a/src/infra/install-source-utils.ts +++ b/src/infra/install-source-utils.ts @@ -5,6 +5,20 @@ import { runCommandWithTimeout } from "../process/exec.js"; import { resolveUserPath } from "../utils.js"; import { fileExists, resolveArchiveKind } from "./archive.js"; +export type NpmSpecResolution = { + name?: string; + version?: string; + resolvedSpec?: string; + integrity?: string; + shasum?: string; + resolvedAt?: string; +}; + +export type NpmIntegrityDrift = { + expectedIntegrity: string; + actualIntegrity: string; +}; + export async function withTempDir( prefix: string, fn: (tmpDir: string) => Promise, @@ -39,6 +53,97 @@ export async function resolveArchiveSourcePath(archivePath: string): Promise< return { ok: true, path: resolved }; } +function toOptionalString(value: unknown): string | undefined { + if (typeof value !== "string") { + return undefined; + } + const trimmed = value.trim(); + return trimmed.length > 0 ? trimmed : undefined; +} + +function parseResolvedSpecFromId(id: string): string | undefined { + const at = id.lastIndexOf("@"); + if (at <= 0 || at >= id.length - 1) { + return undefined; + } + const name = id.slice(0, at).trim(); + const version = id.slice(at + 1).trim(); + if (!name || !version) { + return undefined; + } + return `${name}@${version}`; +} + +function normalizeNpmPackEntry( + entry: unknown, +): { filename?: string; metadata: NpmSpecResolution } | null { + if (!entry || typeof entry !== "object") { + return null; + } + const rec = entry as Record; + const name = toOptionalString(rec.name); + const version = toOptionalString(rec.version); + const id = toOptionalString(rec.id); + const resolvedSpec = + (name && version ? `${name}@${version}` : undefined) ?? + (id ? parseResolvedSpecFromId(id) : undefined); + + return { + filename: toOptionalString(rec.filename), + metadata: { + name, + version, + resolvedSpec, + integrity: toOptionalString(rec.integrity), + shasum: toOptionalString(rec.shasum), + }, + }; +} + +function parseNpmPackJsonOutput( + raw: string, +): { filename?: string; metadata: NpmSpecResolution } | null { + const trimmed = raw.trim(); + if (!trimmed) { + return null; + } + + const candidates = [trimmed]; + const arrayStart = trimmed.indexOf("["); + if (arrayStart > 0) { + candidates.push(trimmed.slice(arrayStart)); + } + + for (const candidate of candidates) { + let parsed: unknown; + try { + parsed = JSON.parse(candidate); + } catch { + continue; + } + + const entries = Array.isArray(parsed) ? parsed : [parsed]; + let fallback: { filename?: string; metadata: NpmSpecResolution } | null = null; + for (let i = entries.length - 1; i >= 0; i -= 1) { + const normalized = normalizeNpmPackEntry(entries[i]); + if (!normalized) { + continue; + } + if (!fallback) { + fallback = normalized; + } + if (normalized.filename) { + return normalized; + } + } + if (fallback) { + return fallback; + } + } + + return null; +} + export async function packNpmSpecToArchive(params: { spec: string; timeoutMs: number; @@ -47,32 +152,44 @@ export async function packNpmSpecToArchive(params: { | { ok: true; archivePath: string; + metadata: NpmSpecResolution; } | { ok: false; error: string; } > { - const res = await runCommandWithTimeout(["npm", "pack", params.spec, "--ignore-scripts"], { - timeoutMs: Math.max(params.timeoutMs, 300_000), - cwd: params.cwd, - env: { - COREPACK_ENABLE_DOWNLOAD_PROMPT: "0", - NPM_CONFIG_IGNORE_SCRIPTS: "true", + const res = await runCommandWithTimeout( + ["npm", "pack", params.spec, "--ignore-scripts", "--json"], + { + timeoutMs: Math.max(params.timeoutMs, 300_000), + cwd: params.cwd, + env: { + COREPACK_ENABLE_DOWNLOAD_PROMPT: "0", + NPM_CONFIG_IGNORE_SCRIPTS: "true", + }, }, - }); + ); if (res.code !== 0) { return { ok: false, error: `npm pack failed: ${res.stderr.trim() || res.stdout.trim()}` }; } - const packed = (res.stdout || "") - .split("\n") - .map((line) => line.trim()) - .filter(Boolean) - .pop(); + const parsedJson = parseNpmPackJsonOutput(res.stdout || ""); + + const packed = + parsedJson?.filename ?? + (res.stdout || "") + .split("\n") + .map((line) => line.trim()) + .filter(Boolean) + .pop(); if (!packed) { return { ok: false, error: "npm pack produced no archive" }; } - return { ok: true, archivePath: path.join(params.cwd, packed) }; + return { + ok: true, + archivePath: path.join(params.cwd, packed), + metadata: parsedJson?.metadata ?? {}, + }; } diff --git a/src/plugins/install.e2e.test.ts b/src/plugins/install.e2e.test.ts index cf870b7925..ca36983491 100644 --- a/src/plugins/install.e2e.test.ts +++ b/src/plugins/install.e2e.test.ts @@ -1,8 +1,8 @@ +import JSZip from "jszip"; import { randomUUID } from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import JSZip from "jszip"; import * as tar from "tar"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import * as skillScanner from "../security/skill-scanner.js"; @@ -491,7 +491,16 @@ describe("installPluginFromNpmSpec", () => { await packToArchive({ pkgDir, outDir: packTmpDir, outName: packedName }); return { code: 0, - stdout: `${packedName}\n`, + stdout: JSON.stringify([ + { + id: "@openclaw/voice-call@0.0.1", + name: "@openclaw/voice-call", + version: "0.0.1", + filename: packedName, + integrity: "sha512-plugin-test", + shasum: "pluginshasum", + }, + ]), stderr: "", signal: null, killed: false, @@ -508,6 +517,11 @@ describe("installPluginFromNpmSpec", () => { logger: { info: () => {}, warn: () => {} }, }); expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + expect(result.npmResolution?.resolvedSpec).toBe("@openclaw/voice-call@0.0.1"); + expect(result.npmResolution?.integrity).toBe("sha512-plugin-test"); expectSingleNpmPackIgnoreScriptsCall({ calls: run.mock.calls, @@ -527,4 +541,46 @@ describe("installPluginFromNpmSpec", () => { } expect(result.error).toContain("unsupported npm spec"); }); + + it("aborts when integrity drift callback rejects the fetched artifact", async () => { + const { runCommandWithTimeout } = await import("../process/exec.js"); + const run = vi.mocked(runCommandWithTimeout); + run.mockResolvedValue({ + code: 0, + stdout: JSON.stringify([ + { + id: "@openclaw/voice-call@0.0.1", + name: "@openclaw/voice-call", + version: "0.0.1", + filename: "voice-call-0.0.1.tgz", + integrity: "sha512-new", + shasum: "newshasum", + }, + ]), + stderr: "", + signal: null, + killed: false, + termination: "exit", + }); + + const onIntegrityDrift = vi.fn(async () => false); + const { installPluginFromNpmSpec } = await import("./install.js"); + const result = await installPluginFromNpmSpec({ + spec: "@openclaw/voice-call@0.0.1", + expectedIntegrity: "sha512-old", + onIntegrityDrift, + }); + + expect(onIntegrityDrift).toHaveBeenCalledWith( + expect.objectContaining({ + expectedIntegrity: "sha512-old", + actualIntegrity: "sha512-new", + }), + ); + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + expect(result.error).toContain("integrity drift"); + }); }); diff --git a/src/plugins/install.ts b/src/plugins/install.ts index 537ece54a2..32e33bb9f3 100644 --- a/src/plugins/install.ts +++ b/src/plugins/install.ts @@ -15,6 +15,8 @@ import { unscopedPackageName, } from "../infra/install-safe-path.js"; import { + type NpmIntegrityDrift, + type NpmSpecResolution, packNpmSpecToArchive, resolveArchiveSourcePath, withTempDir, @@ -43,9 +45,18 @@ export type InstallPluginResult = manifestName?: string; version?: string; extensions: string[]; + npmResolution?: NpmSpecResolution; + integrityDrift?: NpmIntegrityDrift; } | { ok: false; error: string }; +export type PluginNpmIntegrityDriftParams = { + spec: string; + expectedIntegrity: string; + actualIntegrity: string; + resolution: NpmSpecResolution; +}; + const defaultLogger: PluginInstallLogger = {}; function safeFileName(input: string): string { return safeDirName(input); @@ -420,6 +431,8 @@ export async function installPluginFromNpmSpec(params: { mode?: "install" | "update"; dryRun?: boolean; expectedPluginId?: string; + expectedIntegrity?: string; + onIntegrityDrift?: (params: PluginNpmIntegrityDriftParams) => boolean | Promise; }): Promise { const { logger, timeoutMs, mode, dryRun } = resolveTimedPluginInstallModeOptions(params); const expectedPluginId = params.expectedPluginId; @@ -440,7 +453,44 @@ export async function installPluginFromNpmSpec(params: { return packedResult; } - return await installPluginFromArchive({ + const npmResolution: NpmSpecResolution = { + ...packedResult.metadata, + resolvedAt: new Date().toISOString(), + }; + + let integrityDrift: NpmIntegrityDrift | undefined; + if ( + params.expectedIntegrity && + npmResolution.integrity && + params.expectedIntegrity !== npmResolution.integrity + ) { + integrityDrift = { + expectedIntegrity: params.expectedIntegrity, + actualIntegrity: npmResolution.integrity, + }; + const driftPayload: PluginNpmIntegrityDriftParams = { + spec, + expectedIntegrity: integrityDrift.expectedIntegrity, + actualIntegrity: integrityDrift.actualIntegrity, + resolution: npmResolution, + }; + let proceed = true; + if (params.onIntegrityDrift) { + proceed = await params.onIntegrityDrift(driftPayload); + } else { + logger.warn?.( + `Integrity drift detected for ${driftPayload.resolution.resolvedSpec ?? driftPayload.spec}: expected ${driftPayload.expectedIntegrity}, got ${driftPayload.actualIntegrity}`, + ); + } + if (!proceed) { + return { + ok: false, + error: `aborted: npm package integrity drift detected for ${driftPayload.resolution.resolvedSpec ?? driftPayload.spec}`, + }; + } + } + + const installResult = await installPluginFromArchive({ archivePath: packedResult.archivePath, extensionsDir: params.extensionsDir, timeoutMs, @@ -449,6 +499,15 @@ export async function installPluginFromNpmSpec(params: { dryRun, expectedPluginId, }); + if (!installResult.ok) { + return installResult; + } + + return { + ...installResult, + npmResolution, + integrityDrift, + }; }); } diff --git a/src/plugins/update.ts b/src/plugins/update.ts index 46b8d4abed..628ff84995 100644 --- a/src/plugins/update.ts +++ b/src/plugins/update.ts @@ -29,6 +29,16 @@ export type PluginUpdateSummary = { outcomes: PluginUpdateOutcome[]; }; +export type PluginUpdateIntegrityDriftParams = { + pluginId: string; + spec: string; + expectedIntegrity: string; + actualIntegrity: string; + resolvedSpec?: string; + resolvedVersion?: string; + dryRun: boolean; +}; + export type PluginChannelSyncSummary = { switchedToBundled: string[]; switchedToNpm: string[]; @@ -143,6 +153,7 @@ export async function updateNpmInstalledPlugins(params: { pluginIds?: string[]; skipIds?: Set; dryRun?: boolean; + onIntegrityDrift?: (params: PluginUpdateIntegrityDriftParams) => boolean | Promise; }): Promise { const logger = params.logger ?? {}; const installs = params.config.plugins?.installs ?? {}; @@ -210,6 +221,25 @@ export async function updateNpmInstalledPlugins(params: { mode: "update", dryRun: true, expectedPluginId: pluginId, + expectedIntegrity: record.integrity, + onIntegrityDrift: async (drift) => { + const payload: PluginUpdateIntegrityDriftParams = { + pluginId, + spec: drift.spec, + expectedIntegrity: drift.expectedIntegrity, + actualIntegrity: drift.actualIntegrity, + resolvedSpec: drift.resolution.resolvedSpec, + resolvedVersion: drift.resolution.version, + dryRun: true, + }; + if (params.onIntegrityDrift) { + return await params.onIntegrityDrift(payload); + } + logger.warn?.( + `Integrity drift for "${pluginId}" (${payload.resolvedSpec ?? payload.spec}): expected ${payload.expectedIntegrity}, got ${payload.actualIntegrity}`, + ); + return true; + }, logger, }); } catch (err) { @@ -257,6 +287,25 @@ export async function updateNpmInstalledPlugins(params: { spec: record.spec, mode: "update", expectedPluginId: pluginId, + expectedIntegrity: record.integrity, + onIntegrityDrift: async (drift) => { + const payload: PluginUpdateIntegrityDriftParams = { + pluginId, + spec: drift.spec, + expectedIntegrity: drift.expectedIntegrity, + actualIntegrity: drift.actualIntegrity, + resolvedSpec: drift.resolution.resolvedSpec, + resolvedVersion: drift.resolution.version, + dryRun: false, + }; + if (params.onIntegrityDrift) { + return await params.onIntegrityDrift(payload); + } + logger.warn?.( + `Integrity drift for "${pluginId}" (${payload.resolvedSpec ?? payload.spec}): expected ${payload.expectedIntegrity}, got ${payload.actualIntegrity}`, + ); + return true; + }, logger, }); } catch (err) { @@ -283,6 +332,12 @@ export async function updateNpmInstalledPlugins(params: { spec: record.spec, installPath: result.targetDir, version: nextVersion, + resolvedName: result.npmResolution?.name, + resolvedVersion: result.npmResolution?.version, + resolvedSpec: result.npmResolution?.resolvedSpec, + integrity: result.npmResolution?.integrity, + shasum: result.npmResolution?.shasum, + resolvedAt: result.npmResolution?.resolvedAt, }); changed = true; @@ -406,6 +461,12 @@ export async function syncPluginsForUpdateChannel(params: { spec, installPath: result.targetDir, version: result.version, + resolvedName: result.npmResolution?.name, + resolvedVersion: result.npmResolution?.version, + resolvedSpec: result.npmResolution?.resolvedSpec, + integrity: result.npmResolution?.integrity, + shasum: result.npmResolution?.shasum, + resolvedAt: result.npmResolution?.resolvedAt, sourcePath: undefined, }); summary.switchedToNpm.push(pluginId); diff --git a/src/security/audit-extra.async.ts b/src/security/audit-extra.async.ts index 520159cfdc..aa70b6575c 100644 --- a/src/security/audit-extra.async.ts +++ b/src/security/audit-extra.async.ts @@ -5,23 +5,25 @@ */ import fs from "node:fs/promises"; import path from "node:path"; +import type { SandboxToolPolicy } from "../agents/sandbox/types.js"; +import type { OpenClawConfig, ConfigFileSnapshot } from "../config/config.js"; +import type { AgentToolsConfig } from "../config/types.tools.js"; +import type { SkillScanFinding } from "./skill-scanner.js"; +import type { ExecFn } from "./windows-acl.js"; import { resolveDefaultAgentId } from "../agents/agent-scope.js"; import { isToolAllowedByPolicies } from "../agents/pi-tools.policy.js"; import { resolveSandboxConfigForAgent, resolveSandboxToolPolicyForAgent, } from "../agents/sandbox.js"; -import type { SandboxToolPolicy } from "../agents/sandbox/types.js"; import { loadWorkspaceSkillEntries } from "../agents/skills.js"; import { resolveToolProfilePolicy } from "../agents/tool-policy.js"; import { listAgentWorkspaceDirs } from "../agents/workspace-dirs.js"; import { MANIFEST_KEY } from "../compat/legacy-names.js"; import { resolveNativeSkillsEnabled } from "../config/commands.js"; -import type { OpenClawConfig, ConfigFileSnapshot } from "../config/config.js"; import { createConfigIO } from "../config/config.js"; import { collectIncludePathsRecursive } from "../config/includes-scan.js"; import { resolveOAuthDir } from "../config/paths.js"; -import type { AgentToolsConfig } from "../config/types.tools.js"; import { normalizePluginsConfig } from "../plugins/config-state.js"; import { normalizeAgentId } from "../routing/session-key.js"; import { @@ -32,9 +34,7 @@ import { } from "./audit-fs.js"; import { pickSandboxToolPolicy } from "./audit-tool-policy.js"; import { extensionUsesSkippedScannerPath, isPathInside } from "./scan-paths.js"; -import type { SkillScanFinding } from "./skill-scanner.js"; import * as skillScanner from "./skill-scanner.js"; -import type { ExecFn } from "./windows-acl.js"; export type SecurityAuditFinding = { checkId: string; @@ -215,6 +215,29 @@ function hasProviderPluginAllow(params: { return false; } +function isPinnedRegistrySpec(spec: string): boolean { + const value = spec.trim(); + if (!value) { + return false; + } + const at = value.lastIndexOf("@"); + if (at <= 0 || at >= value.length - 1) { + return false; + } + const version = value.slice(at + 1).trim(); + return /^v?\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/.test(version); +} + +async function readInstalledPackageVersion(dir: string): Promise { + try { + const raw = await fs.readFile(path.join(dir, "package.json"), "utf-8"); + const parsed = JSON.parse(raw) as { version?: unknown }; + return typeof parsed.version === "string" ? parsed.version : undefined; + } catch { + return undefined; + } +} + // -------------------------------------------------------------------------- // Exported collectors // -------------------------------------------------------------------------- @@ -227,155 +250,279 @@ export async function collectPluginsTrustFindings(params: { const { extensionsDir, pluginDirs } = await listInstalledPluginDirs({ stateDir: params.stateDir, }); - if (pluginDirs.length === 0) { - return findings; - } + if (pluginDirs.length > 0) { + const allow = params.cfg.plugins?.allow; + const allowConfigured = Array.isArray(allow) && allow.length > 0; + if (!allowConfigured) { + const hasString = (value: unknown) => typeof value === "string" && value.trim().length > 0; + const hasAccountStringKey = (account: unknown, key: string) => + Boolean( + account && + typeof account === "object" && + hasString((account as Record)[key]), + ); - const allow = params.cfg.plugins?.allow; - const allowConfigured = Array.isArray(allow) && allow.length > 0; - if (!allowConfigured) { - const hasString = (value: unknown) => typeof value === "string" && value.trim().length > 0; - const hasAccountStringKey = (account: unknown, key: string) => - Boolean( - account && - typeof account === "object" && - hasString((account as Record)[key]), - ); + const discordConfigured = + hasString(params.cfg.channels?.discord?.token) || + Boolean( + params.cfg.channels?.discord?.accounts && + Object.values(params.cfg.channels.discord.accounts).some((a) => + hasAccountStringKey(a, "token"), + ), + ) || + hasString(process.env.DISCORD_BOT_TOKEN); - const discordConfigured = - hasString(params.cfg.channels?.discord?.token) || - Boolean( - params.cfg.channels?.discord?.accounts && - Object.values(params.cfg.channels.discord.accounts).some((a) => - hasAccountStringKey(a, "token"), - ), - ) || - hasString(process.env.DISCORD_BOT_TOKEN); + const telegramConfigured = + hasString(params.cfg.channels?.telegram?.botToken) || + hasString(params.cfg.channels?.telegram?.tokenFile) || + Boolean( + params.cfg.channels?.telegram?.accounts && + Object.values(params.cfg.channels.telegram.accounts).some( + (a) => hasAccountStringKey(a, "botToken") || hasAccountStringKey(a, "tokenFile"), + ), + ) || + hasString(process.env.TELEGRAM_BOT_TOKEN); - const telegramConfigured = - hasString(params.cfg.channels?.telegram?.botToken) || - hasString(params.cfg.channels?.telegram?.tokenFile) || - Boolean( - params.cfg.channels?.telegram?.accounts && - Object.values(params.cfg.channels.telegram.accounts).some( - (a) => hasAccountStringKey(a, "botToken") || hasAccountStringKey(a, "tokenFile"), - ), - ) || - hasString(process.env.TELEGRAM_BOT_TOKEN); + const slackConfigured = + hasString(params.cfg.channels?.slack?.botToken) || + hasString(params.cfg.channels?.slack?.appToken) || + Boolean( + params.cfg.channels?.slack?.accounts && + Object.values(params.cfg.channels.slack.accounts).some( + (a) => hasAccountStringKey(a, "botToken") || hasAccountStringKey(a, "appToken"), + ), + ) || + hasString(process.env.SLACK_BOT_TOKEN) || + hasString(process.env.SLACK_APP_TOKEN); - const slackConfigured = - hasString(params.cfg.channels?.slack?.botToken) || - hasString(params.cfg.channels?.slack?.appToken) || - Boolean( - params.cfg.channels?.slack?.accounts && - Object.values(params.cfg.channels.slack.accounts).some( - (a) => hasAccountStringKey(a, "botToken") || hasAccountStringKey(a, "appToken"), - ), - ) || - hasString(process.env.SLACK_BOT_TOKEN) || - hasString(process.env.SLACK_APP_TOKEN); - - const skillCommandsLikelyExposed = - (discordConfigured && - resolveNativeSkillsEnabled({ - providerId: "discord", - providerSetting: params.cfg.channels?.discord?.commands?.nativeSkills, - globalSetting: params.cfg.commands?.nativeSkills, - })) || - (telegramConfigured && - resolveNativeSkillsEnabled({ - providerId: "telegram", - providerSetting: params.cfg.channels?.telegram?.commands?.nativeSkills, - globalSetting: params.cfg.commands?.nativeSkills, - })) || - (slackConfigured && - resolveNativeSkillsEnabled({ - providerId: "slack", - providerSetting: params.cfg.channels?.slack?.commands?.nativeSkills, - globalSetting: params.cfg.commands?.nativeSkills, - })); - - findings.push({ - checkId: "plugins.extensions_no_allowlist", - severity: skillCommandsLikelyExposed ? "critical" : "warn", - title: "Extensions exist but plugins.allow is not set", - detail: - `Found ${pluginDirs.length} extension(s) under ${extensionsDir}. Without plugins.allow, any discovered plugin id may load (depending on config and plugin behavior).` + - (skillCommandsLikelyExposed - ? "\nNative skill commands are enabled on at least one configured chat surface; treat unpinned/unallowlisted extensions as high risk." - : ""), - remediation: "Set plugins.allow to an explicit list of plugin ids you trust.", - }); - } - - const enabledExtensionPluginIds = resolveEnabledExtensionPluginIds({ - cfg: params.cfg, - pluginDirs, - }); - if (enabledExtensionPluginIds.length > 0) { - const enabledPluginSet = new Set(enabledExtensionPluginIds); - const contexts: Array<{ - label: string; - agentId?: string; - tools?: AgentToolsConfig; - }> = [{ label: "default" }]; - for (const entry of params.cfg.agents?.list ?? []) { - if (!entry || typeof entry !== "object" || typeof entry.id !== "string") { - continue; - } - contexts.push({ - label: `agents.list.${entry.id}`, - agentId: entry.id, - tools: entry.tools, - }); - } - - const permissiveContexts: string[] = []; - for (const context of contexts) { - const profile = context.tools?.profile ?? params.cfg.tools?.profile; - const restrictiveProfile = Boolean(resolveToolProfilePolicy(profile)); - const sandboxMode = resolveSandboxConfigForAgent(params.cfg, context.agentId).mode; - const policies = resolveToolPolicies({ - cfg: params.cfg, - agentTools: context.tools, - sandboxMode, - agentId: context.agentId, - }); - const broadPolicy = isToolAllowedByPolicies("__openclaw_plugin_probe__", policies); - const explicitPluginAllow = - !restrictiveProfile && - (hasExplicitPluginAllow({ - allowEntries: collectAllowEntries(params.cfg.tools), - enabledPluginIds: enabledPluginSet, - }) || - hasProviderPluginAllow({ - byProvider: params.cfg.tools?.byProvider, - enabledPluginIds: enabledPluginSet, - }) || - hasExplicitPluginAllow({ - allowEntries: collectAllowEntries(context.tools), - enabledPluginIds: enabledPluginSet, - }) || - hasProviderPluginAllow({ - byProvider: context.tools?.byProvider, - enabledPluginIds: enabledPluginSet, + const skillCommandsLikelyExposed = + (discordConfigured && + resolveNativeSkillsEnabled({ + providerId: "discord", + providerSetting: params.cfg.channels?.discord?.commands?.nativeSkills, + globalSetting: params.cfg.commands?.nativeSkills, + })) || + (telegramConfigured && + resolveNativeSkillsEnabled({ + providerId: "telegram", + providerSetting: params.cfg.channels?.telegram?.commands?.nativeSkills, + globalSetting: params.cfg.commands?.nativeSkills, + })) || + (slackConfigured && + resolveNativeSkillsEnabled({ + providerId: "slack", + providerSetting: params.cfg.channels?.slack?.commands?.nativeSkills, + globalSetting: params.cfg.commands?.nativeSkills, })); - if (broadPolicy || explicitPluginAllow) { - permissiveContexts.push(context.label); - } + findings.push({ + checkId: "plugins.extensions_no_allowlist", + severity: skillCommandsLikelyExposed ? "critical" : "warn", + title: "Extensions exist but plugins.allow is not set", + detail: + `Found ${pluginDirs.length} extension(s) under ${extensionsDir}. Without plugins.allow, any discovered plugin id may load (depending on config and plugin behavior).` + + (skillCommandsLikelyExposed + ? "\nNative skill commands are enabled on at least one configured chat surface; treat unpinned/unallowlisted extensions as high risk." + : ""), + remediation: "Set plugins.allow to an explicit list of plugin ids you trust.", + }); } - if (permissiveContexts.length > 0) { + const enabledExtensionPluginIds = resolveEnabledExtensionPluginIds({ + cfg: params.cfg, + pluginDirs, + }); + if (enabledExtensionPluginIds.length > 0) { + const enabledPluginSet = new Set(enabledExtensionPluginIds); + const contexts: Array<{ + label: string; + agentId?: string; + tools?: AgentToolsConfig; + }> = [{ label: "default" }]; + for (const entry of params.cfg.agents?.list ?? []) { + if (!entry || typeof entry !== "object" || typeof entry.id !== "string") { + continue; + } + contexts.push({ + label: `agents.list.${entry.id}`, + agentId: entry.id, + tools: entry.tools, + }); + } + + const permissiveContexts: string[] = []; + for (const context of contexts) { + const profile = context.tools?.profile ?? params.cfg.tools?.profile; + const restrictiveProfile = Boolean(resolveToolProfilePolicy(profile)); + const sandboxMode = resolveSandboxConfigForAgent(params.cfg, context.agentId).mode; + const policies = resolveToolPolicies({ + cfg: params.cfg, + agentTools: context.tools, + sandboxMode, + agentId: context.agentId, + }); + const broadPolicy = isToolAllowedByPolicies("__openclaw_plugin_probe__", policies); + const explicitPluginAllow = + !restrictiveProfile && + (hasExplicitPluginAllow({ + allowEntries: collectAllowEntries(params.cfg.tools), + enabledPluginIds: enabledPluginSet, + }) || + hasProviderPluginAllow({ + byProvider: params.cfg.tools?.byProvider, + enabledPluginIds: enabledPluginSet, + }) || + hasExplicitPluginAllow({ + allowEntries: collectAllowEntries(context.tools), + enabledPluginIds: enabledPluginSet, + }) || + hasProviderPluginAllow({ + byProvider: context.tools?.byProvider, + enabledPluginIds: enabledPluginSet, + })); + + if (broadPolicy || explicitPluginAllow) { + permissiveContexts.push(context.label); + } + } + + if (permissiveContexts.length > 0) { + findings.push({ + checkId: "plugins.tools_reachable_permissive_policy", + severity: "warn", + title: "Extension plugin tools may be reachable under permissive tool policy", + detail: + `Enabled extension plugins: ${enabledExtensionPluginIds.join(", ")}.\n` + + `Permissive tool policy contexts:\n${permissiveContexts.map((entry) => `- ${entry}`).join("\n")}`, + remediation: + "Use restrictive profiles (`minimal`/`coding`) or explicit tool allowlists that exclude plugin tools for agents handling untrusted input.", + }); + } + } + } + + const pluginInstalls = params.cfg.plugins?.installs ?? {}; + const npmPluginInstalls = Object.entries(pluginInstalls).filter( + ([, record]) => record?.source === "npm", + ); + if (npmPluginInstalls.length > 0) { + const unpinned = npmPluginInstalls + .filter(([, record]) => typeof record.spec === "string" && !isPinnedRegistrySpec(record.spec)) + .map(([pluginId, record]) => `${pluginId} (${record.spec})`); + if (unpinned.length > 0) { findings.push({ - checkId: "plugins.tools_reachable_permissive_policy", + checkId: "plugins.installs_unpinned_npm_specs", severity: "warn", - title: "Extension plugin tools may be reachable under permissive tool policy", - detail: - `Enabled extension plugins: ${enabledExtensionPluginIds.join(", ")}.\n` + - `Permissive tool policy contexts:\n${permissiveContexts.map((entry) => `- ${entry}`).join("\n")}`, + title: "Plugin installs include unpinned npm specs", + detail: `Unpinned plugin install records:\n${unpinned.map((entry) => `- ${entry}`).join("\n")}`, remediation: - "Use restrictive profiles (`minimal`/`coding`) or explicit tool allowlists that exclude plugin tools for agents handling untrusted input.", + "Pin install specs to exact versions (for example, `@scope/pkg@1.2.3`) for higher supply-chain stability.", + }); + } + + const missingIntegrity = npmPluginInstalls + .filter( + ([, record]) => typeof record.integrity !== "string" || record.integrity.trim() === "", + ) + .map(([pluginId]) => pluginId); + if (missingIntegrity.length > 0) { + findings.push({ + checkId: "plugins.installs_missing_integrity", + severity: "warn", + title: "Plugin installs are missing integrity metadata", + detail: `Plugin install records missing integrity:\n${missingIntegrity.map((entry) => `- ${entry}`).join("\n")}`, + remediation: + "Reinstall or update plugins to refresh install metadata with resolved integrity hashes.", + }); + } + + const pluginVersionDrift: string[] = []; + for (const [pluginId, record] of npmPluginInstalls) { + const recordedVersion = record.resolvedVersion ?? record.version; + if (!recordedVersion) { + continue; + } + const installPath = record.installPath ?? path.join(params.stateDir, "extensions", pluginId); + // eslint-disable-next-line no-await-in-loop + const installedVersion = await readInstalledPackageVersion(installPath); + if (!installedVersion || installedVersion === recordedVersion) { + continue; + } + pluginVersionDrift.push( + `${pluginId} (recorded ${recordedVersion}, installed ${installedVersion})`, + ); + } + if (pluginVersionDrift.length > 0) { + findings.push({ + checkId: "plugins.installs_version_drift", + severity: "warn", + title: "Plugin install records drift from installed package versions", + detail: `Detected plugin install metadata drift:\n${pluginVersionDrift.map((entry) => `- ${entry}`).join("\n")}`, + remediation: + "Run `openclaw plugins update --all` (or reinstall affected plugins) to refresh install metadata.", + }); + } + } + + const hookInstalls = params.cfg.hooks?.internal?.installs ?? {}; + const npmHookInstalls = Object.entries(hookInstalls).filter( + ([, record]) => record?.source === "npm", + ); + if (npmHookInstalls.length > 0) { + const unpinned = npmHookInstalls + .filter(([, record]) => typeof record.spec === "string" && !isPinnedRegistrySpec(record.spec)) + .map(([hookId, record]) => `${hookId} (${record.spec})`); + if (unpinned.length > 0) { + findings.push({ + checkId: "hooks.installs_unpinned_npm_specs", + severity: "warn", + title: "Hook installs include unpinned npm specs", + detail: `Unpinned hook install records:\n${unpinned.map((entry) => `- ${entry}`).join("\n")}`, + remediation: + "Pin hook install specs to exact versions (for example, `@scope/pkg@1.2.3`) for higher supply-chain stability.", + }); + } + + const missingIntegrity = npmHookInstalls + .filter( + ([, record]) => typeof record.integrity !== "string" || record.integrity.trim() === "", + ) + .map(([hookId]) => hookId); + if (missingIntegrity.length > 0) { + findings.push({ + checkId: "hooks.installs_missing_integrity", + severity: "warn", + title: "Hook installs are missing integrity metadata", + detail: `Hook install records missing integrity:\n${missingIntegrity.map((entry) => `- ${entry}`).join("\n")}`, + remediation: + "Reinstall or update hooks to refresh install metadata with resolved integrity hashes.", + }); + } + + const hookVersionDrift: string[] = []; + for (const [hookId, record] of npmHookInstalls) { + const recordedVersion = record.resolvedVersion ?? record.version; + if (!recordedVersion) { + continue; + } + const installPath = record.installPath ?? path.join(params.stateDir, "hooks", hookId); + // eslint-disable-next-line no-await-in-loop + const installedVersion = await readInstalledPackageVersion(installPath); + if (!installedVersion || installedVersion === recordedVersion) { + continue; + } + hookVersionDrift.push( + `${hookId} (recorded ${recordedVersion}, installed ${installedVersion})`, + ); + } + if (hookVersionDrift.length > 0) { + findings.push({ + checkId: "hooks.installs_version_drift", + severity: "warn", + title: "Hook install records drift from installed package versions", + detail: `Detected hook install metadata drift:\n${hookVersionDrift.map((entry) => `- ${entry}`).join("\n")}`, + remediation: + "Run `openclaw hooks update --all` (or reinstall affected hooks) to refresh install metadata.", }); } } diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index ba5490a180..d85a37fe8a 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -4,9 +4,9 @@ import path from "node:path"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import type { ChannelPlugin } from "../channels/plugins/types.js"; import type { OpenClawConfig } from "../config/config.js"; +import type { SecurityAuditOptions, SecurityAuditReport } from "./audit.js"; import { collectPluginsCodeSafetyFindings } from "./audit-extra.js"; import { runSecurityAudit } from "./audit.js"; -import type { SecurityAuditOptions, SecurityAuditReport } from "./audit.js"; import * as skillScanner from "./skill-scanner.js"; const isWindows = process.platform === "win32"; @@ -1502,6 +1502,143 @@ describe("security audit", () => { } }); + it("warns on unpinned npm install specs and missing integrity metadata", async () => { + const tmp = await makeTmpDir("install-metadata-warns"); + const stateDir = path.join(tmp, "state"); + await fs.mkdir(stateDir, { recursive: true }); + + const cfg: OpenClawConfig = { + plugins: { + installs: { + "voice-call": { + source: "npm", + spec: "@openclaw/voice-call", + }, + }, + }, + hooks: { + internal: { + installs: { + "test-hooks": { + source: "npm", + spec: "@openclaw/test-hooks", + }, + }, + }, + }, + }; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: true, + includeChannelSecurity: false, + stateDir, + configPath: path.join(stateDir, "openclaw.json"), + }); + + expect(hasFinding(res, "plugins.installs_unpinned_npm_specs", "warn")).toBe(true); + expect(hasFinding(res, "plugins.installs_missing_integrity", "warn")).toBe(true); + expect(hasFinding(res, "hooks.installs_unpinned_npm_specs", "warn")).toBe(true); + expect(hasFinding(res, "hooks.installs_missing_integrity", "warn")).toBe(true); + }); + + it("does not warn on pinned npm install specs with integrity metadata", async () => { + const tmp = await makeTmpDir("install-metadata-clean"); + const stateDir = path.join(tmp, "state"); + await fs.mkdir(stateDir, { recursive: true }); + + const cfg: OpenClawConfig = { + plugins: { + installs: { + "voice-call": { + source: "npm", + spec: "@openclaw/voice-call@1.2.3", + integrity: "sha512-plugin", + }, + }, + }, + hooks: { + internal: { + installs: { + "test-hooks": { + source: "npm", + spec: "@openclaw/test-hooks@1.2.3", + integrity: "sha512-hook", + }, + }, + }, + }, + }; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: true, + includeChannelSecurity: false, + stateDir, + configPath: path.join(stateDir, "openclaw.json"), + }); + + expect(hasFinding(res, "plugins.installs_unpinned_npm_specs")).toBe(false); + expect(hasFinding(res, "plugins.installs_missing_integrity")).toBe(false); + expect(hasFinding(res, "hooks.installs_unpinned_npm_specs")).toBe(false); + expect(hasFinding(res, "hooks.installs_missing_integrity")).toBe(false); + }); + + it("warns when install records drift from installed package versions", async () => { + const tmp = await makeTmpDir("install-version-drift"); + const stateDir = path.join(tmp, "state"); + const pluginDir = path.join(stateDir, "extensions", "voice-call"); + const hookDir = path.join(stateDir, "hooks", "test-hooks"); + await fs.mkdir(pluginDir, { recursive: true }); + await fs.mkdir(hookDir, { recursive: true }); + await fs.writeFile( + path.join(pluginDir, "package.json"), + JSON.stringify({ name: "@openclaw/voice-call", version: "9.9.9" }), + "utf-8", + ); + await fs.writeFile( + path.join(hookDir, "package.json"), + JSON.stringify({ name: "@openclaw/test-hooks", version: "8.8.8" }), + "utf-8", + ); + + const cfg: OpenClawConfig = { + plugins: { + installs: { + "voice-call": { + source: "npm", + spec: "@openclaw/voice-call@1.2.3", + integrity: "sha512-plugin", + resolvedVersion: "1.2.3", + }, + }, + }, + hooks: { + internal: { + installs: { + "test-hooks": { + source: "npm", + spec: "@openclaw/test-hooks@1.2.3", + integrity: "sha512-hook", + resolvedVersion: "1.2.3", + }, + }, + }, + }, + }; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: true, + includeChannelSecurity: false, + stateDir, + configPath: path.join(stateDir, "openclaw.json"), + }); + + expect(hasFinding(res, "plugins.installs_version_drift", "warn")).toBe(true); + expect(hasFinding(res, "hooks.installs_version_drift", "warn")).toBe(true); + }); + it("flags enabled extensions when tool policy can expose plugin tools", async () => { const tmp = await makeTmpDir("plugins-reachable"); const stateDir = path.join(tmp, "state"); diff --git a/src/test-utils/exec-assertions.ts b/src/test-utils/exec-assertions.ts index f26166bc23..50bf54f61e 100644 --- a/src/test-utils/exec-assertions.ts +++ b/src/test-utils/exec-assertions.ts @@ -28,7 +28,7 @@ export function expectSingleNpmPackIgnoreScriptsCall(params: { throw new Error("expected npm pack call"); } const [argv, options] = packCall; - expect(argv).toEqual(["npm", "pack", params.expectedSpec, "--ignore-scripts"]); + expect(argv).toEqual(["npm", "pack", params.expectedSpec, "--ignore-scripts", "--json"]); const commandOptions = typeof options === "number" ? undefined : options; expect(commandOptions).toMatchObject({ env: { NPM_CONFIG_IGNORE_SCRIPTS: "true" } }); }