fix(security): harden plugin/hook npm installs

This commit is contained in:
Peter Steinberger
2026-02-14 14:07:07 +01:00
parent d69b32a073
commit 6f7d31c426
10 changed files with 391 additions and 119 deletions

View File

@@ -4,7 +4,7 @@ import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import * as tar from "tar";
import { afterAll, describe, expect, it, vi } from "vitest";
import { afterAll, beforeEach, describe, expect, it, vi } from "vitest";
const fixtureRoot = path.join(os.tmpdir(), `openclaw-hook-install-${randomUUID()}`);
let tempDirIndex = 0;
@@ -13,6 +13,28 @@ vi.mock("../process/exec.js", () => ({
runCommandWithTimeout: vi.fn(),
}));
async function packToArchive({
pkgDir,
outDir,
outName,
}: {
pkgDir: string;
outDir: string;
outName: string;
}) {
const dest = path.join(outDir, outName);
fs.rmSync(dest, { force: true });
await tar.c(
{
gzip: true,
file: dest,
cwd: path.dirname(pkgDir),
},
[path.basename(pkgDir)],
);
return dest;
}
function makeTempDir() {
const dir = path.join(fixtureRoot, `case-${tempDirIndex++}`);
fs.mkdirSync(dir, { recursive: true });
@@ -20,7 +42,8 @@ function makeTempDir() {
}
const { runCommandWithTimeout } = await import("../process/exec.js");
const { installHooksFromArchive, installHooksFromPath } = await import("./install.js");
const { installHooksFromArchive, installHooksFromNpmSpec, installHooksFromPath } =
await import("./install.js");
afterAll(() => {
try {
@@ -30,6 +53,10 @@ afterAll(() => {
}
});
beforeEach(() => {
vi.clearAllMocks();
});
describe("installHooksFromArchive", () => {
it("installs hook packs from zip archives", async () => {
const stateDir = makeTempDir();
@@ -308,3 +335,88 @@ describe("installHooksFromPath", () => {
expect(fs.existsSync(path.join(result.targetDir, "HOOK.md"))).toBe(true);
});
});
describe("installHooksFromNpmSpec", () => {
it("uses --ignore-scripts for npm pack and cleans up temp dir", async () => {
const workDir = makeTempDir();
const stateDir = makeTempDir();
const pkgDir = path.join(workDir, "package");
fs.mkdirSync(path.join(pkgDir, "hooks", "one-hook"), { recursive: true });
fs.writeFileSync(
path.join(pkgDir, "package.json"),
JSON.stringify({
name: "@openclaw/test-hooks",
version: "0.0.1",
openclaw: { hooks: ["./hooks/one-hook"] },
}),
"utf-8",
);
fs.writeFileSync(
path.join(pkgDir, "hooks", "one-hook", "HOOK.md"),
[
"---",
"name: one-hook",
"description: One hook",
'metadata: {"openclaw":{"events":["command:new"]}}',
"---",
"",
"# One Hook",
].join("\n"),
"utf-8",
);
fs.writeFileSync(
path.join(pkgDir, "hooks", "one-hook", "handler.ts"),
"export default async () => {};\n",
"utf-8",
);
const run = vi.mocked(runCommandWithTimeout);
let packTmpDir = "";
const packedName = "test-hooks-0.0.1.tgz";
run.mockImplementation(async (argv, opts) => {
if (argv[0] === "npm" && argv[1] === "pack") {
packTmpDir = String(opts?.cwd ?? "");
await packToArchive({ pkgDir, outDir: packTmpDir, outName: packedName });
return { code: 0, stdout: `${packedName}\n`, stderr: "", signal: null, killed: false };
}
throw new Error(`unexpected command: ${argv.join(" ")}`);
});
const hooksDir = path.join(stateDir, "hooks");
const result = await installHooksFromNpmSpec({
spec: "@openclaw/test-hooks@0.0.1",
hooksDir,
logger: { info: () => {}, warn: () => {} },
});
expect(result.ok).toBe(true);
if (!result.ok) {
return;
}
expect(result.hookPackId).toBe("test-hooks");
expect(fs.existsSync(path.join(result.targetDir, "hooks", "one-hook", "HOOK.md"))).toBe(true);
const packCalls = run.mock.calls.filter(
(c) => Array.isArray(c[0]) && c[0][0] === "npm" && c[0][1] === "pack",
);
expect(packCalls.length).toBe(1);
const packCall = packCalls[0];
if (!packCall) {
throw new Error("expected npm pack call");
}
const [argv, options] = packCall;
expect(argv).toEqual(["npm", "pack", "@openclaw/test-hooks@0.0.1", "--ignore-scripts"]);
expect(options?.env).toMatchObject({ NPM_CONFIG_IGNORE_SCRIPTS: "true" });
expect(packTmpDir).not.toBe("");
expect(fs.existsSync(packTmpDir)).toBe(false);
});
it("rejects non-registry npm specs", async () => {
const result = await installHooksFromNpmSpec({ spec: "github:evil/evil" });
expect(result.ok).toBe(false);
if (result.ok) {
return;
}
expect(result.error).toContain("unsupported npm spec");
});
});

View File

@@ -9,6 +9,7 @@ import {
resolveArchiveKind,
resolvePackedRootDir,
} from "../infra/archive.js";
import { validateRegistryNpmSpec } from "../infra/npm-registry-spec.js";
import { runCommandWithTimeout } from "../process/exec.js";
import { CONFIG_DIR, resolveUserPath } from "../utils.js";
import { parseFrontmatter } from "./frontmatter.js";
@@ -356,44 +357,48 @@ export async function installHooksFromArchive(params: {
}
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-hook-"));
const extractDir = path.join(tmpDir, "extract");
await fs.mkdir(extractDir, { recursive: true });
logger.info?.(`Extracting ${archivePath}`);
try {
await extractArchive({ archivePath, destDir: extractDir, timeoutMs, logger });
} catch (err) {
return { ok: false, error: `failed to extract archive: ${String(err)}` };
}
const extractDir = path.join(tmpDir, "extract");
await fs.mkdir(extractDir, { recursive: true });
let rootDir = "";
try {
rootDir = await resolvePackedRootDir(extractDir);
} catch (err) {
return { ok: false, error: String(err) };
}
logger.info?.(`Extracting ${archivePath}`);
try {
await extractArchive({ archivePath, destDir: extractDir, timeoutMs, logger });
} catch (err) {
return { ok: false, error: `failed to extract archive: ${String(err)}` };
}
const manifestPath = path.join(rootDir, "package.json");
if (await fileExists(manifestPath)) {
return await installHookPackageFromDir({
packageDir: rootDir,
let rootDir = "";
try {
rootDir = await resolvePackedRootDir(extractDir);
} catch (err) {
return { ok: false, error: String(err) };
}
const manifestPath = path.join(rootDir, "package.json");
if (await fileExists(manifestPath)) {
return await installHookPackageFromDir({
packageDir: rootDir,
hooksDir: params.hooksDir,
timeoutMs,
logger,
mode: params.mode,
dryRun: params.dryRun,
expectedHookPackId: params.expectedHookPackId,
});
}
return await installHookFromDir({
hookDir: rootDir,
hooksDir: params.hooksDir,
timeoutMs,
logger,
mode: params.mode,
dryRun: params.dryRun,
expectedHookPackId: params.expectedHookPackId,
});
} finally {
await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => undefined);
}
return await installHookFromDir({
hookDir: rootDir,
hooksDir: params.hooksDir,
logger,
mode: params.mode,
dryRun: params.dryRun,
expectedHookPackId: params.expectedHookPackId,
});
}
export async function installHooksFromNpmSpec(params: {
@@ -411,40 +416,48 @@ export async function installHooksFromNpmSpec(params: {
const dryRun = params.dryRun ?? false;
const expectedHookPackId = params.expectedHookPackId;
const spec = params.spec.trim();
if (!spec) {
return { ok: false, error: "missing npm spec" };
const specError = validateRegistryNpmSpec(spec);
if (specError) {
return { ok: false, error: specError };
}
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-hook-pack-"));
logger.info?.(`Downloading ${spec}`);
const res = await runCommandWithTimeout(["npm", "pack", spec], {
timeoutMs: Math.max(timeoutMs, 300_000),
cwd: tmpDir,
env: { COREPACK_ENABLE_DOWNLOAD_PROMPT: "0" },
});
if (res.code !== 0) {
return { ok: false, error: `npm pack failed: ${res.stderr.trim() || res.stdout.trim()}` };
}
try {
logger.info?.(`Downloading ${spec}`);
const res = await runCommandWithTimeout(["npm", "pack", spec, "--ignore-scripts"], {
timeoutMs: Math.max(timeoutMs, 300_000),
cwd: tmpDir,
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((l) => l.trim())
.filter(Boolean)
.pop();
if (!packed) {
return { ok: false, error: "npm pack produced no archive" };
}
const packed = (res.stdout || "")
.split("\n")
.map((l) => l.trim())
.filter(Boolean)
.pop();
if (!packed) {
return { ok: false, error: "npm pack produced no archive" };
}
const archivePath = path.join(tmpDir, packed);
return await installHooksFromArchive({
archivePath,
hooksDir: params.hooksDir,
timeoutMs,
logger,
mode,
dryRun,
expectedHookPackId,
});
const archivePath = path.join(tmpDir, packed);
return await installHooksFromArchive({
archivePath,
hooksDir: params.hooksDir,
timeoutMs,
logger,
mode,
dryRun,
expectedHookPackId,
});
} finally {
await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => undefined);
}
}
export async function installHooksFromPath(params: {

View File

@@ -0,0 +1,41 @@
export function validateRegistryNpmSpec(rawSpec: string): string | null {
const spec = rawSpec.trim();
if (!spec) {
return "missing npm spec";
}
if (/\s/.test(spec)) {
return "unsupported npm spec: whitespace is not allowed";
}
// Registry-only: no URLs, git, file, or alias protocols.
// Keep strict: this runs on the gateway host.
if (spec.includes("://")) {
return "unsupported npm spec: URLs are not allowed";
}
if (spec.includes("#")) {
return "unsupported npm spec: git refs are not allowed";
}
if (spec.includes(":")) {
return "unsupported npm spec: protocol specs are not allowed";
}
const at = spec.lastIndexOf("@");
const hasVersion = at > 0;
const name = hasVersion ? spec.slice(0, at) : spec;
const version = hasVersion ? spec.slice(at + 1) : "";
const unscopedName = /^[a-z0-9][a-z0-9-._~]*$/;
const scopedName = /^@[a-z0-9][a-z0-9-._~]*\/[a-z0-9][a-z0-9-._~]*$/;
const isValidName = name.startsWith("@") ? scopedName.test(name) : unscopedName.test(name);
if (!isValidName) {
return "unsupported npm spec: expected <name> or <name>@<version> from the npm registry";
}
if (hasVersion) {
if (!version) {
return "unsupported npm spec: missing version/tag after @";
}
if (/[\\/]/.test(version)) {
return "unsupported npm spec: invalid version/tag";
}
}
return null;
}

View File

@@ -4,7 +4,7 @@ import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import * as tar from "tar";
import { afterEach, describe, expect, it, vi } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import * as skillScanner from "../security/skill-scanner.js";
vi.mock("../process/exec.js", () => ({
@@ -52,6 +52,10 @@ afterEach(() => {
}
});
beforeEach(() => {
vi.clearAllMocks();
});
describe("installPluginFromArchive", () => {
it("installs into ~/.openclaw/extensions and uses unscoped id", async () => {
const stateDir = makeTempDir();
@@ -487,3 +491,72 @@ describe("installPluginFromDir", () => {
expect(opts?.cwd).toBe(res.targetDir);
});
});
describe("installPluginFromNpmSpec", () => {
it("uses --ignore-scripts for npm pack and cleans up temp dir", async () => {
const workDir = makeTempDir();
const stateDir = makeTempDir();
const pkgDir = path.join(workDir, "package");
fs.mkdirSync(path.join(pkgDir, "dist"), { recursive: true });
fs.writeFileSync(
path.join(pkgDir, "package.json"),
JSON.stringify({
name: "@openclaw/voice-call",
version: "0.0.1",
openclaw: { extensions: ["./dist/index.js"] },
}),
"utf-8",
);
fs.writeFileSync(path.join(pkgDir, "dist", "index.js"), "export {};", "utf-8");
const extensionsDir = path.join(stateDir, "extensions");
fs.mkdirSync(extensionsDir, { recursive: true });
const { runCommandWithTimeout } = await import("../process/exec.js");
const run = vi.mocked(runCommandWithTimeout);
let packTmpDir = "";
const packedName = "voice-call-0.0.1.tgz";
run.mockImplementation(async (argv, opts) => {
if (argv[0] === "npm" && argv[1] === "pack") {
packTmpDir = String(opts?.cwd ?? "");
await packToArchive({ pkgDir, outDir: packTmpDir, outName: packedName });
return { code: 0, stdout: `${packedName}\n`, stderr: "", signal: null, killed: false };
}
throw new Error(`unexpected command: ${argv.join(" ")}`);
});
const { installPluginFromNpmSpec } = await import("./install.js");
const result = await installPluginFromNpmSpec({
spec: "@openclaw/voice-call@0.0.1",
extensionsDir,
logger: { info: () => {}, warn: () => {} },
});
expect(result.ok).toBe(true);
const packCalls = run.mock.calls.filter(
(c) => Array.isArray(c[0]) && c[0][0] === "npm" && c[0][1] === "pack",
);
expect(packCalls.length).toBe(1);
const packCall = packCalls[0];
if (!packCall) {
throw new Error("expected npm pack call");
}
const [argv, options] = packCall;
expect(argv).toEqual(["npm", "pack", "@openclaw/voice-call@0.0.1", "--ignore-scripts"]);
expect(options?.env).toMatchObject({ NPM_CONFIG_IGNORE_SCRIPTS: "true" });
expect(packTmpDir).not.toBe("");
expect(fs.existsSync(packTmpDir)).toBe(false);
});
it("rejects non-registry npm specs", async () => {
const { installPluginFromNpmSpec } = await import("./install.js");
const result = await installPluginFromNpmSpec({ spec: "github:evil/evil" });
expect(result.ok).toBe(false);
if (result.ok) {
return;
}
expect(result.error).toContain("unsupported npm spec");
});
});

View File

@@ -9,6 +9,7 @@ import {
resolveArchiveKind,
resolvePackedRootDir,
} from "../infra/archive.js";
import { validateRegistryNpmSpec } from "../infra/npm-registry-spec.js";
import { runCommandWithTimeout } from "../process/exec.js";
import * as skillScanner from "../security/skill-scanner.js";
import { CONFIG_DIR, resolveUserPath } from "../utils.js";
@@ -334,37 +335,41 @@ export async function installPluginFromArchive(params: {
}
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-plugin-"));
const extractDir = path.join(tmpDir, "extract");
await fs.mkdir(extractDir, { recursive: true });
logger.info?.(`Extracting ${archivePath}`);
try {
await extractArchive({
archivePath,
destDir: extractDir,
const extractDir = path.join(tmpDir, "extract");
await fs.mkdir(extractDir, { recursive: true });
logger.info?.(`Extracting ${archivePath}`);
try {
await extractArchive({
archivePath,
destDir: extractDir,
timeoutMs,
logger,
});
} catch (err) {
return { ok: false, error: `failed to extract archive: ${String(err)}` };
}
let packageDir = "";
try {
packageDir = await resolvePackedRootDir(extractDir);
} catch (err) {
return { ok: false, error: String(err) };
}
return await installPluginFromPackageDir({
packageDir,
extensionsDir: params.extensionsDir,
timeoutMs,
logger,
mode,
dryRun: params.dryRun,
expectedPluginId: params.expectedPluginId,
});
} catch (err) {
return { ok: false, error: `failed to extract archive: ${String(err)}` };
} finally {
await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => undefined);
}
let packageDir = "";
try {
packageDir = await resolvePackedRootDir(extractDir);
} catch (err) {
return { ok: false, error: String(err) };
}
return await installPluginFromPackageDir({
packageDir,
extensionsDir: params.extensionsDir,
timeoutMs,
logger,
mode,
dryRun: params.dryRun,
expectedPluginId: params.expectedPluginId,
});
}
export async function installPluginFromDir(params: {
@@ -468,43 +473,51 @@ export async function installPluginFromNpmSpec(params: {
const dryRun = params.dryRun ?? false;
const expectedPluginId = params.expectedPluginId;
const spec = params.spec.trim();
if (!spec) {
return { ok: false, error: "missing npm spec" };
const specError = validateRegistryNpmSpec(spec);
if (specError) {
return { ok: false, error: specError };
}
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-npm-pack-"));
logger.info?.(`Downloading ${spec}`);
const res = await runCommandWithTimeout(["npm", "pack", spec], {
timeoutMs: Math.max(timeoutMs, 300_000),
cwd: tmpDir,
env: { COREPACK_ENABLE_DOWNLOAD_PROMPT: "0" },
});
if (res.code !== 0) {
return {
ok: false,
error: `npm pack failed: ${res.stderr.trim() || res.stdout.trim()}`,
};
}
try {
logger.info?.(`Downloading ${spec}`);
const res = await runCommandWithTimeout(["npm", "pack", spec, "--ignore-scripts"], {
timeoutMs: Math.max(timeoutMs, 300_000),
cwd: tmpDir,
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((l) => l.trim())
.filter(Boolean)
.pop();
if (!packed) {
return { ok: false, error: "npm pack produced no archive" };
}
const packed = (res.stdout || "")
.split("\n")
.map((l) => l.trim())
.filter(Boolean)
.pop();
if (!packed) {
return { ok: false, error: "npm pack produced no archive" };
}
const archivePath = path.join(tmpDir, packed);
return await installPluginFromArchive({
archivePath,
extensionsDir: params.extensionsDir,
timeoutMs,
logger,
mode,
dryRun,
expectedPluginId,
});
const archivePath = path.join(tmpDir, packed);
return await installPluginFromArchive({
archivePath,
extensionsDir: params.extensionsDir,
timeoutMs,
logger,
mode,
dryRun,
expectedPluginId,
});
} finally {
await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => undefined);
}
}
export async function installPluginFromPath(params: {