refactor: harden remaining plugin manifest reads
This commit is contained in:
@@ -271,6 +271,42 @@ describe("discoverOpenClawPlugins", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("ignores package manifests that are hardlinked aliases", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const stateDir = makeTempDir();
|
||||
const globalExt = path.join(stateDir, "extensions", "pack");
|
||||
const outsideDir = path.join(stateDir, "outside");
|
||||
const outsideManifest = path.join(outsideDir, "package.json");
|
||||
const linkedManifest = path.join(globalExt, "package.json");
|
||||
fs.mkdirSync(globalExt, { recursive: true });
|
||||
fs.mkdirSync(outsideDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(globalExt, "entry.ts"), "export default {}", "utf-8");
|
||||
fs.writeFileSync(
|
||||
outsideManifest,
|
||||
JSON.stringify({
|
||||
name: "@openclaw/pack",
|
||||
openclaw: { extensions: ["./entry.ts"] },
|
||||
}),
|
||||
"utf-8",
|
||||
);
|
||||
try {
|
||||
fs.linkSync(outsideManifest, linkedManifest);
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code === "EXDEV") {
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
const { candidates } = await withStateDir(stateDir, async () => {
|
||||
return discoverOpenClawPlugins({});
|
||||
});
|
||||
|
||||
expect(candidates.some((candidate) => candidate.idHint === "pack")).toBe(false);
|
||||
});
|
||||
|
||||
it.runIf(process.platform !== "win32")("blocks world-writable plugin paths", async () => {
|
||||
const stateDir = makeTempDir();
|
||||
const globalExt = path.join(stateDir, "extensions");
|
||||
|
||||
@@ -225,14 +225,21 @@ function shouldIgnoreScannedDirectory(dirName: string): boolean {
|
||||
|
||||
function readPackageManifest(dir: string): PackageManifest | null {
|
||||
const manifestPath = path.join(dir, "package.json");
|
||||
if (!fs.existsSync(manifestPath)) {
|
||||
const opened = openBoundaryFileSync({
|
||||
absolutePath: manifestPath,
|
||||
rootPath: dir,
|
||||
boundaryLabel: "plugin package directory",
|
||||
});
|
||||
if (!opened.ok) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
const raw = fs.readFileSync(manifestPath, "utf-8");
|
||||
const raw = fs.readFileSync(opened.fd, "utf-8");
|
||||
return JSON.parse(raw) as PackageManifest;
|
||||
} catch {
|
||||
return null;
|
||||
} finally {
|
||||
fs.closeSync(opened.fd);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -167,4 +167,70 @@ describe("loadPluginManifestRegistry", () => {
|
||||
expect(registry.plugins.length).toBe(1);
|
||||
expect(registry.plugins[0]?.origin).toBe("config");
|
||||
});
|
||||
|
||||
it("rejects manifest paths that escape plugin root via symlink", () => {
|
||||
const rootDir = makeTempDir();
|
||||
const outsideDir = makeTempDir();
|
||||
const outsideManifest = path.join(outsideDir, "openclaw.plugin.json");
|
||||
const linkedManifest = path.join(rootDir, "openclaw.plugin.json");
|
||||
fs.writeFileSync(path.join(rootDir, "index.ts"), "export default function () {}", "utf-8");
|
||||
fs.writeFileSync(
|
||||
outsideManifest,
|
||||
JSON.stringify({ id: "unsafe-symlink", configSchema: { type: "object" } }),
|
||||
"utf-8",
|
||||
);
|
||||
try {
|
||||
fs.symlinkSync(outsideManifest, linkedManifest);
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
|
||||
const registry = loadRegistry([
|
||||
createPluginCandidate({
|
||||
idHint: "unsafe-symlink",
|
||||
rootDir,
|
||||
origin: "workspace",
|
||||
}),
|
||||
]);
|
||||
expect(registry.plugins).toHaveLength(0);
|
||||
expect(
|
||||
registry.diagnostics.some((diag) => diag.message.includes("unsafe plugin manifest path")),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects manifest paths that escape plugin root via hardlink", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const rootDir = makeTempDir();
|
||||
const outsideDir = makeTempDir();
|
||||
const outsideManifest = path.join(outsideDir, "openclaw.plugin.json");
|
||||
const linkedManifest = path.join(rootDir, "openclaw.plugin.json");
|
||||
fs.writeFileSync(path.join(rootDir, "index.ts"), "export default function () {}", "utf-8");
|
||||
fs.writeFileSync(
|
||||
outsideManifest,
|
||||
JSON.stringify({ id: "unsafe-hardlink", configSchema: { type: "object" } }),
|
||||
"utf-8",
|
||||
);
|
||||
try {
|
||||
fs.linkSync(outsideManifest, linkedManifest);
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code === "EXDEV") {
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
const registry = loadRegistry([
|
||||
createPluginCandidate({
|
||||
idHint: "unsafe-hardlink",
|
||||
rootDir,
|
||||
origin: "workspace",
|
||||
}),
|
||||
]);
|
||||
expect(registry.plugins).toHaveLength(0);
|
||||
expect(
|
||||
registry.diagnostics.some((diag) => diag.message.includes("unsafe plugin manifest path")),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { MANIFEST_KEY } from "../compat/legacy-names.js";
|
||||
import { openBoundaryFileSync } from "../infra/boundary-file-read.js";
|
||||
import { isRecord } from "../utils.js";
|
||||
import type { PluginConfigUiHint, PluginKind } from "./types.js";
|
||||
|
||||
@@ -43,18 +44,32 @@ export function resolvePluginManifestPath(rootDir: string): string {
|
||||
|
||||
export function loadPluginManifest(rootDir: string): PluginManifestLoadResult {
|
||||
const manifestPath = resolvePluginManifestPath(rootDir);
|
||||
if (!fs.existsSync(manifestPath)) {
|
||||
return { ok: false, error: `plugin manifest not found: ${manifestPath}`, manifestPath };
|
||||
const opened = openBoundaryFileSync({
|
||||
absolutePath: manifestPath,
|
||||
rootPath: rootDir,
|
||||
boundaryLabel: "plugin root",
|
||||
});
|
||||
if (!opened.ok) {
|
||||
if (opened.reason === "path") {
|
||||
return { ok: false, error: `plugin manifest not found: ${manifestPath}`, manifestPath };
|
||||
}
|
||||
return {
|
||||
ok: false,
|
||||
error: `unsafe plugin manifest path: ${manifestPath} (${opened.reason})`,
|
||||
manifestPath,
|
||||
};
|
||||
}
|
||||
let raw: unknown;
|
||||
try {
|
||||
raw = JSON.parse(fs.readFileSync(manifestPath, "utf-8")) as unknown;
|
||||
raw = JSON.parse(fs.readFileSync(opened.fd, "utf-8")) as unknown;
|
||||
} catch (err) {
|
||||
return {
|
||||
ok: false,
|
||||
error: `failed to parse plugin manifest: ${String(err)}`,
|
||||
manifestPath,
|
||||
};
|
||||
} finally {
|
||||
fs.closeSync(opened.fd);
|
||||
}
|
||||
if (!isRecord(raw)) {
|
||||
return { ok: false, error: "plugin manifest must be an object", manifestPath };
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import fs from "node:fs/promises";
|
||||
import fsSync from "node:fs";
|
||||
import path from "node:path";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import { openBoundaryFileSync } from "../infra/boundary-file-read.js";
|
||||
import type { UpdateChannel } from "../infra/update-channels.js";
|
||||
import { resolveUserPath } from "../utils.js";
|
||||
import { discoverOpenClawPlugins } from "./discovery.js";
|
||||
@@ -69,12 +71,23 @@ type InstallIntegrityDrift = {
|
||||
};
|
||||
|
||||
async function readInstalledPackageVersion(dir: string): Promise<string | undefined> {
|
||||
const manifestPath = path.join(dir, "package.json");
|
||||
const opened = openBoundaryFileSync({
|
||||
absolutePath: manifestPath,
|
||||
rootPath: dir,
|
||||
boundaryLabel: "installed plugin directory",
|
||||
});
|
||||
if (!opened.ok) {
|
||||
return undefined;
|
||||
}
|
||||
try {
|
||||
const raw = await fs.readFile(`${dir}/package.json`, "utf-8");
|
||||
const raw = fsSync.readFileSync(opened.fd, "utf-8");
|
||||
const parsed = JSON.parse(raw) as { version?: unknown };
|
||||
return typeof parsed.version === "string" ? parsed.version : undefined;
|
||||
} catch {
|
||||
return undefined;
|
||||
} finally {
|
||||
fsSync.closeSync(opened.fd);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user