Harden `POST /plugins/install`: canonicalize `localPath` for all instances; bundled-only floor for managed instances (#10067)
**Builds on** #10058 — managed detection keys off the *presence* of the `PAPERCLIP_MANAGED_CONFIG` env var that PR introduces, deliberately never its parsed body. **Summary.** Two layered hardenings of the plugin install route. (1) For **all** instances: `localPath` installs previously skipped the package-name validation entirely; the path is now null-byte-checked, resolved absolute, `realpath`'d (collapsing `..` traversal and symlinks), and required to be an existing directory before the loader ever sees it. (2) For instances running under a managed hosting control plane (detected by the *presence* of `PAPERCLIP_MANAGED_CONFIG` — deliberately never its body, so a corrupted document cannot widen the surface): registry/npm installs return 403, and `localPath` installs must canonicalize to inside the bundled plugin catalog root (`packages/plugins`) — a positive allowlist enforced in code at the route, independent of any flag value. Self-hosted behavior is otherwise unchanged. ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The plugin system lets instance admins install plugins from a registry or from a local filesystem path, and plugin installation is code execution on the host > - The `localPath` branch of `POST /plugins/install` skips the validation applied to registry installs; the raw path reaches the plugin loader without canonicalization > - Separately, instances operated by a managed hosting control plane must constrain installs to the bundled plugin catalog, because there the host belongs to the operator, not the tenant > - This pull request canonicalizes and validates `localPath` for all instances, and adds a bundled-only install floor for managed instances > - The benefit is a smaller install-route attack surface everywhere, and a positive code-enforced allowlist where the operator owns the machine ## Linked Issues or Issue Description No public issue exists; `bug_report` template fields for the validation gap this PR fixes: - **What happened:** `POST /plugins/install` with `localPath` set bypasses the package-name validation entirely; the un-canonicalized path (relative segments, symlinks, no existence check) is handed straight to the plugin loader. - **Expected behavior:** path installs are validated like registry installs — null-byte-checked, resolved absolute, `realpath`'d, and required to be an existing directory before the loader sees them. - **Steps to reproduce:** as an instance admin, call `POST /plugins/install` with a `localPath` containing `..` traversal or a symlink pointing outside any plugin directory; observe the loader receives the raw path. Exploitability is bounded (the route already requires instance admin), so this is hardening of an admin-only surface rather than an open exploit. - **Version:** current `master`. The managed-instance bundled-only floor layered on top is new behavior (motivation: on managed hosting, arbitrary plugin install is arbitrary code execution on operator infrastructure), aligned with the in-progress "Cloud deployments" milestone in `ROADMAP.md`. ## What Changed - New `server/src/services/plugin-install-guard.ts` — three pure primitives: managed detection (presence-based), path canonicalization (null-byte check → absolute resolve → `realpath` → must be an existing directory), and segment-based containment in the bundled plugin catalog root. - Route enforcement in `server/src/routes/plugins.ts`: npm/registry installs return 403 on managed instances; `localPath` installs are canonicalized on every instance and, on managed instances, must land inside the bundled catalog root. - The plugin loader now receives the canonical path instead of the raw request string. ## Verification - 15 guard unit tests (`server/src/__tests__/plugin-install-guard.test.ts`): traversal, symlink escape, null byte, file-vs-directory, string-prefix sibling root. - 13 route security tests (`server/src/__tests__/plugin-install-route-security.test.ts`): 403 matrix on managed instances + self-hosted happy paths. - 36 existing plugin route authz tests green (`server/src/__tests__/plugin-routes-authz.test.ts`). - Server `tsc --noEmit` clean. ```bash cd server pnpm vitest run src/__tests__/plugin-install-guard.test.ts src/__tests__/plugin-install-route-security.test.ts src/__tests__/plugin-routes-authz.test.ts pnpm exec tsc --noEmit ``` ## Risks - Managed instances: npm/registry installs and out-of-catalog `localPath` installs now return 403 — intended new behavior, enforced in code rather than configuration. - All instances: `localPath` installs that previously pointed at nonexistent paths or non-directories now fail with 400 before reaching the loader (previously the loader failed later, less safely). Symlinked deployment layouts are handled by canonicalizing both sides of the containment check. - Self-hosted npm install path is unchanged. Low residual risk. ## Model Used Claude Fable 5 (`claude-fable-5`), extended thinking, agentic tool use; independently peer-reviewed by a second AI agent before push. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Devin Foley
GitHub
parent
04e070bf45
commit
0ef3b320c7
|
|
@ -0,0 +1,161 @@
|
|||
import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import {
|
||||
canonicalizeLocalPluginPath,
|
||||
isCloudManagedInstance,
|
||||
isWithinBundledPluginRoot,
|
||||
} from "../services/plugin-install-guard.js";
|
||||
|
||||
describe("isCloudManagedInstance", () => {
|
||||
it("returns false when PAPERCLIP_MANAGED_CONFIG is absent", () => {
|
||||
expect(isCloudManagedInstance({})).toBe(false);
|
||||
expect(isCloudManagedInstance({ OTHER_VAR: "x" })).toBe(false);
|
||||
});
|
||||
|
||||
it("returns true when PAPERCLIP_MANAGED_CONFIG is set to a cloud document", () => {
|
||||
const doc = JSON.stringify({ v: 1, mode: "cloud", catalogVersion: "1", features: {}, plugins: { autoInstall: [] } });
|
||||
expect(isCloudManagedInstance({ PAPERCLIP_MANAGED_CONFIG: doc })).toBe(true);
|
||||
});
|
||||
|
||||
it("fails closed: blank or corrupted documents still count as cloud-managed", () => {
|
||||
expect(isCloudManagedInstance({ PAPERCLIP_MANAGED_CONFIG: "" })).toBe(true);
|
||||
expect(isCloudManagedInstance({ PAPERCLIP_MANAGED_CONFIG: " " })).toBe(true);
|
||||
expect(isCloudManagedInstance({ PAPERCLIP_MANAGED_CONFIG: "{not json" })).toBe(true);
|
||||
expect(isCloudManagedInstance({ PAPERCLIP_MANAGED_CONFIG: JSON.stringify({ mode: "self-hosted" }) })).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("canonicalizeLocalPluginPath", () => {
|
||||
const cleanupPaths = new Set<string>();
|
||||
|
||||
afterEach(async () => {
|
||||
for (const cleanupPath of cleanupPaths) {
|
||||
await rm(cleanupPath, { recursive: true, force: true });
|
||||
}
|
||||
cleanupPaths.clear();
|
||||
});
|
||||
|
||||
async function makeTempDir(prefix: string): Promise<string> {
|
||||
const dir = await mkdtemp(path.join(os.tmpdir(), prefix));
|
||||
cleanupPaths.add(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
it("accepts an existing absolute directory path", async () => {
|
||||
const dir = await makeTempDir("guard-abs-");
|
||||
const result = await canonicalizeLocalPluginPath(dir);
|
||||
expect(result).toEqual({ ok: true, canonicalPath: await realCanonical(dir) });
|
||||
});
|
||||
|
||||
it("collapses traversal segments to the canonical path", async () => {
|
||||
const dir = await makeTempDir("guard-traversal-");
|
||||
const nested = path.join(dir, "a", "b");
|
||||
await mkdir(nested, { recursive: true });
|
||||
const traversal = path.join(dir, "a", "..", "a", "b", "..", "b");
|
||||
const result = await canonicalizeLocalPluginPath(traversal);
|
||||
expect(result).toEqual({ ok: true, canonicalPath: await realCanonical(nested) });
|
||||
});
|
||||
|
||||
it("resolves symlinks to their target", async () => {
|
||||
const dir = await makeTempDir("guard-symlink-");
|
||||
const target = path.join(dir, "target");
|
||||
await mkdir(target, { recursive: true });
|
||||
const link = path.join(dir, "link");
|
||||
await symlink(target, link, "dir");
|
||||
const result = await canonicalizeLocalPluginPath(link);
|
||||
expect(result).toEqual({ ok: true, canonicalPath: await realCanonical(target) });
|
||||
});
|
||||
|
||||
it("rejects paths containing a null byte", async () => {
|
||||
const result = await canonicalizeLocalPluginPath("/tmp/foo\0bar");
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) expect(result.reason).toContain("null byte");
|
||||
});
|
||||
|
||||
it("rejects nonexistent paths", async () => {
|
||||
const dir = await makeTempDir("guard-missing-");
|
||||
const result = await canonicalizeLocalPluginPath(path.join(dir, "does-not-exist"));
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) expect(result.reason).toContain("does not exist");
|
||||
});
|
||||
|
||||
it("rejects paths that resolve to a file rather than a directory", async () => {
|
||||
const dir = await makeTempDir("guard-file-");
|
||||
const file = path.join(dir, "plugin.txt");
|
||||
await writeFile(file, "not a directory", "utf8");
|
||||
const result = await canonicalizeLocalPluginPath(file);
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) expect(result.reason).toContain("not a directory");
|
||||
});
|
||||
});
|
||||
|
||||
describe("isWithinBundledPluginRoot", () => {
|
||||
const cleanupPaths = new Set<string>();
|
||||
|
||||
afterEach(async () => {
|
||||
for (const cleanupPath of cleanupPaths) {
|
||||
await rm(cleanupPath, { recursive: true, force: true });
|
||||
}
|
||||
cleanupPaths.clear();
|
||||
});
|
||||
|
||||
async function makeCatalogFixture(): Promise<{ root: string; inside: string; outside: string }> {
|
||||
const base = await mkdtemp(path.join(os.tmpdir(), "guard-catalog-"));
|
||||
cleanupPaths.add(base);
|
||||
const root = path.join(base, "packages", "plugins");
|
||||
const inside = path.join(root, "plugin-good");
|
||||
const outside = path.join(base, "elsewhere");
|
||||
await mkdir(inside, { recursive: true });
|
||||
await mkdir(outside, { recursive: true });
|
||||
return { root, inside, outside };
|
||||
}
|
||||
|
||||
it("accepts a directory inside the catalog root", async () => {
|
||||
const { root, inside } = await makeCatalogFixture();
|
||||
expect(await isWithinBundledPluginRoot(await realCanonical(inside), root)).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects a directory outside the catalog root", async () => {
|
||||
const { root, outside } = await makeCatalogFixture();
|
||||
expect(await isWithinBundledPluginRoot(await realCanonical(outside), root)).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects the catalog root itself", async () => {
|
||||
const { root } = await makeCatalogFixture();
|
||||
expect(await isWithinBundledPluginRoot(await realCanonical(root), root)).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects a sibling directory whose name shares the root as a string prefix", async () => {
|
||||
const { root } = await makeCatalogFixture();
|
||||
const sibling = `${root}-evil`;
|
||||
await mkdir(sibling, { recursive: true });
|
||||
expect(await isWithinBundledPluginRoot(await realCanonical(sibling), root)).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects a symlink target that escapes the catalog root once canonicalized", async () => {
|
||||
const { root, outside } = await makeCatalogFixture();
|
||||
const link = path.join(root, "sneaky");
|
||||
await symlink(outside, link, "dir");
|
||||
// The guard contract is that callers canonicalize first; the symlink's
|
||||
// real path lands outside the root and must be rejected.
|
||||
const canonical = await canonicalizeLocalPluginPath(link);
|
||||
expect(canonical.ok).toBe(true);
|
||||
if (canonical.ok) {
|
||||
expect(await isWithinBundledPluginRoot(canonical.canonicalPath, root)).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("fails closed when the catalog root does not exist", async () => {
|
||||
const { inside } = await makeCatalogFixture();
|
||||
expect(await isWithinBundledPluginRoot(await realCanonical(inside), "/nonexistent/catalog/root")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
/** realpath through the same lens the guard uses (macOS /tmp is a symlink). */
|
||||
async function realCanonical(target: string): Promise<string> {
|
||||
const result = await canonicalizeLocalPluginPath(target);
|
||||
if (!result.ok) throw new Error(`fixture path did not canonicalize: ${result.reason}`);
|
||||
return result.canonicalPath;
|
||||
}
|
||||
|
|
@ -0,0 +1,386 @@
|
|||
/**
|
||||
* Route-level coverage for the plugin install security floor:
|
||||
*
|
||||
* - Cloud-managed instances (PAPERCLIP_MANAGED_CONFIG present) may only
|
||||
* install plugins whose source canonicalizes into the bundled plugin
|
||||
* catalog root; npm installs and arbitrary local paths are rejected.
|
||||
* - Every instance canonicalizes and validates `localPath` (traversal,
|
||||
* symlink, and absolute-path handling) before the loader runs.
|
||||
* - Self-hosted non-localPath install validation is unchanged.
|
||||
*/
|
||||
import express from "express";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { mkdir, mkdtemp, realpath, rm, symlink, writeFile } from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import request from "supertest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { createDb, plugins } from "@paperclipai/db";
|
||||
import { pluginLoader, REPO_ROOT } from "../services/plugin-loader.js";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
|
||||
const mockLifecycle = vi.hoisted(() => ({
|
||||
load: vi.fn(),
|
||||
upgrade: vi.fn(),
|
||||
unload: vi.fn(),
|
||||
enable: vi.fn(),
|
||||
disable: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../services/plugin-lifecycle.js", () => ({
|
||||
pluginLifecycleManager: () => mockLifecycle,
|
||||
}));
|
||||
|
||||
vi.mock("../services/activity-log.js", () => ({
|
||||
logActivity: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../services/live-events.js", () => ({
|
||||
publishGlobalLiveEvent: vi.fn(),
|
||||
}));
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip;
|
||||
|
||||
if (!embeddedPostgresSupport.supported) {
|
||||
console.warn(
|
||||
`Skipping plugin install route security tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`,
|
||||
);
|
||||
}
|
||||
|
||||
const CLOUD_MANAGED_CONFIG = JSON.stringify({
|
||||
v: 1,
|
||||
mode: "cloud",
|
||||
catalogVersion: "2026.720.0",
|
||||
features: {},
|
||||
plugins: { autoInstall: [] },
|
||||
});
|
||||
|
||||
const repoPluginRoot = path.join(REPO_ROOT, "packages", "plugins");
|
||||
|
||||
type FixturePlugin = {
|
||||
packageName: string;
|
||||
pluginKey: string;
|
||||
packageRoot: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Create a valid, already-built plugin package (dist written directly, so no
|
||||
* pnpm auto-build runs during install) at an arbitrary directory.
|
||||
*/
|
||||
async function createBuiltPluginFixture(parentDir: string, nameSuffix: string): Promise<FixturePlugin> {
|
||||
const slug = `plugin-install-guard-${nameSuffix}-${randomUUID().slice(0, 8)}`;
|
||||
const packageName = `@paperclipai/${slug}`;
|
||||
const pluginKey = `paperclip.${slug.replace(/^plugin-/, "").replace(/-/g, "_")}`;
|
||||
const packageRoot = path.join(parentDir, slug);
|
||||
const distDir = path.join(packageRoot, "dist");
|
||||
|
||||
await mkdir(distDir, { recursive: true });
|
||||
await writeFile(
|
||||
path.join(packageRoot, "package.json"),
|
||||
JSON.stringify({
|
||||
name: packageName,
|
||||
version: "0.1.0",
|
||||
private: true,
|
||||
type: "module",
|
||||
paperclipPlugin: {
|
||||
manifest: "./dist/manifest.js",
|
||||
worker: "./dist/worker.js",
|
||||
},
|
||||
}, null, 2),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
const manifest = {
|
||||
id: pluginKey,
|
||||
apiVersion: 1,
|
||||
version: "0.1.0",
|
||||
displayName: "Install Guard Fixture",
|
||||
description: "Plugin fixture for install-route security floor coverage.",
|
||||
author: "Paperclip",
|
||||
categories: ["automation"],
|
||||
capabilities: ["companies.read"],
|
||||
entrypoints: {
|
||||
worker: "./dist/worker.js",
|
||||
},
|
||||
};
|
||||
await writeFile(
|
||||
path.join(distDir, "manifest.js"),
|
||||
`export default ${JSON.stringify(manifest, null, 2)};\n`,
|
||||
"utf8",
|
||||
);
|
||||
await writeFile(path.join(distDir, "worker.js"), "export {};\n", "utf8");
|
||||
|
||||
return { packageName, pluginKey, packageRoot };
|
||||
}
|
||||
|
||||
async function createInstallApp(db: ReturnType<typeof createDb>) {
|
||||
const [{ pluginRoutes }, { errorHandler }] = await Promise.all([
|
||||
import("../routes/plugins.js"),
|
||||
import("../middleware/index.js"),
|
||||
]);
|
||||
|
||||
const loader = pluginLoader(db, {
|
||||
enableLocalFilesystem: false,
|
||||
enableNpmDiscovery: false,
|
||||
});
|
||||
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => {
|
||||
req.actor = {
|
||||
type: "board",
|
||||
userId: "admin-1",
|
||||
source: "session",
|
||||
isInstanceAdmin: true,
|
||||
companyIds: [],
|
||||
} as typeof req.actor;
|
||||
next();
|
||||
});
|
||||
app.use("/api", pluginRoutes(db as never, loader as never, {} as never, undefined, {} as never, {} as never));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
|
||||
describeEmbeddedPostgres("plugin install route security floor", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
const cleanupPaths = new Set<string>();
|
||||
const originalManagedConfig = process.env["PAPERCLIP_MANAGED_CONFIG"];
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-plugin-install-guard-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
await db.delete(plugins);
|
||||
for (const cleanupPath of cleanupPaths) {
|
||||
await rm(cleanupPath, { recursive: true, force: true });
|
||||
}
|
||||
cleanupPaths.clear();
|
||||
if (originalManagedConfig === undefined) {
|
||||
delete process.env["PAPERCLIP_MANAGED_CONFIG"];
|
||||
} else {
|
||||
process.env["PAPERCLIP_MANAGED_CONFIG"] = originalManagedConfig;
|
||||
}
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await tempDb?.cleanup();
|
||||
}, 30_000);
|
||||
|
||||
describe("cloud-managed instances", () => {
|
||||
it("rejects npm installs outright", async () => {
|
||||
process.env["PAPERCLIP_MANAGED_CONFIG"] = CLOUD_MANAGED_CONFIG;
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: "paperclip-plugin-anything", version: "1.0.0" });
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body.error).toContain("npm installs are disabled on cloud-managed instances");
|
||||
expect(mockLifecycle.load).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects npm installs even when the managed config document is corrupted", async () => {
|
||||
// The fail-closed startup parser refuses to boot a managed instance
|
||||
// with a corrupted document, so boot with a valid one and corrupt it
|
||||
// afterwards: the install floor must still hold because it keys off
|
||||
// the variable's presence at request time, never its parsed content.
|
||||
process.env["PAPERCLIP_MANAGED_CONFIG"] = CLOUD_MANAGED_CONFIG;
|
||||
const app = await createInstallApp(db);
|
||||
process.env["PAPERCLIP_MANAGED_CONFIG"] = "{definitely not json";
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: "paperclip-plugin-anything" });
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(mockLifecycle.load).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects localPath installs from outside the bundled catalog root", async () => {
|
||||
process.env["PAPERCLIP_MANAGED_CONFIG"] = CLOUD_MANAGED_CONFIG;
|
||||
const outsideDir = await mkdtemp(path.join(os.tmpdir(), "guard-outside-"));
|
||||
cleanupPaths.add(outsideDir);
|
||||
const fixture = await createBuiltPluginFixture(outsideDir, "outside");
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: fixture.packageRoot, isLocalPath: true });
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body.error).toContain("bundled plugin catalog");
|
||||
expect(mockLifecycle.load).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects localPath traversal that escapes the bundled catalog root", async () => {
|
||||
process.env["PAPERCLIP_MANAGED_CONFIG"] = CLOUD_MANAGED_CONFIG;
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
// Starts under packages/plugins but canonicalizes to the repo's server
|
||||
// directory — a real, readable directory outside the catalog.
|
||||
const traversalPath = path.join(repoPluginRoot, "..", "..", "server");
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: traversalPath, isLocalPath: true });
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body.error).toContain("bundled plugin catalog");
|
||||
expect(mockLifecycle.load).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a symlink inside the catalog root that points outside it", async () => {
|
||||
process.env["PAPERCLIP_MANAGED_CONFIG"] = CLOUD_MANAGED_CONFIG;
|
||||
const outsideDir = await mkdtemp(path.join(os.tmpdir(), "guard-symlink-target-"));
|
||||
cleanupPaths.add(outsideDir);
|
||||
const fixture = await createBuiltPluginFixture(outsideDir, "symlink-target");
|
||||
|
||||
const linkPath = path.join(repoPluginRoot, `guard-sneaky-link-${randomUUID().slice(0, 8)}`);
|
||||
cleanupPaths.add(linkPath);
|
||||
await symlink(fixture.packageRoot, linkPath, "dir");
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: linkPath, isLocalPath: true });
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body.error).toContain("bundled plugin catalog");
|
||||
expect(mockLifecycle.load).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows installing a plugin from inside the bundled catalog root", async () => {
|
||||
process.env["PAPERCLIP_MANAGED_CONFIG"] = CLOUD_MANAGED_CONFIG;
|
||||
const fixture = await createBuiltPluginFixture(repoPluginRoot, "bundled");
|
||||
cleanupPaths.add(fixture.packageRoot);
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: fixture.packageRoot, isLocalPath: true });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.packageName).toBe(fixture.packageName);
|
||||
expect(res.body.pluginKey).toBe(fixture.pluginKey);
|
||||
expect(mockLifecycle.load).toHaveBeenCalledTimes(1);
|
||||
}, 30_000);
|
||||
});
|
||||
|
||||
describe("all instances: localPath canonicalization", () => {
|
||||
it("rejects a nonexistent localPath with a validation error", async () => {
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: "/nonexistent/plugin/dir", isLocalPath: true });
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("Invalid localPath");
|
||||
expect(res.body.error).toContain("does not exist");
|
||||
expect(mockLifecycle.load).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a localPath containing a null byte", async () => {
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: "/tmp/foo\u0000bar", isLocalPath: true });
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("Invalid localPath");
|
||||
expect(res.body.error).toContain("null byte");
|
||||
expect(mockLifecycle.load).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a localPath that resolves to a file", async () => {
|
||||
const outsideDir = await mkdtemp(path.join(os.tmpdir(), "guard-file-"));
|
||||
cleanupPaths.add(outsideDir);
|
||||
const filePath = path.join(outsideDir, "plugin.tgz");
|
||||
await writeFile(filePath, "not a directory", "utf8");
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: filePath, isLocalPath: true });
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("Invalid localPath");
|
||||
expect(res.body.error).toContain("not a directory");
|
||||
expect(mockLifecycle.load).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("canonicalizes traversal segments before installing (self-hosted)", async () => {
|
||||
const outsideDir = await mkdtemp(path.join(os.tmpdir(), "guard-traversal-"));
|
||||
cleanupPaths.add(outsideDir);
|
||||
const fixture = await createBuiltPluginFixture(outsideDir, "traversal");
|
||||
const slug = path.basename(fixture.packageRoot);
|
||||
const traversalPath = path.join(outsideDir, "..", path.basename(outsideDir), ".", slug);
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: traversalPath, isLocalPath: true });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.packageName).toBe(fixture.packageName);
|
||||
expect(res.body.packagePath).toBe(await realpath(fixture.packageRoot));
|
||||
expect(mockLifecycle.load).toHaveBeenCalledTimes(1);
|
||||
}, 30_000);
|
||||
|
||||
it("resolves symlinked localPath installs to the real target (self-hosted)", async () => {
|
||||
const outsideDir = await mkdtemp(path.join(os.tmpdir(), "guard-selfhosted-symlink-"));
|
||||
cleanupPaths.add(outsideDir);
|
||||
const fixture = await createBuiltPluginFixture(outsideDir, "symlinked");
|
||||
const linkPath = path.join(outsideDir, "linked-plugin");
|
||||
await symlink(fixture.packageRoot, linkPath, "dir");
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: linkPath, isLocalPath: true });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.packageName).toBe(fixture.packageName);
|
||||
expect(res.body.packagePath).toBe(await realpath(fixture.packageRoot));
|
||||
expect(mockLifecycle.load).toHaveBeenCalledTimes(1);
|
||||
}, 30_000);
|
||||
});
|
||||
|
||||
describe("self-hosted npm installs (behavior unchanged)", () => {
|
||||
it("still rejects package names with invalid characters", async () => {
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: "bad<name>" });
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toBe("packageName contains invalid characters");
|
||||
expect(mockLifecycle.load).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not apply the cloud floor when PAPERCLIP_MANAGED_CONFIG is absent", async () => {
|
||||
const app = await createInstallApp(db);
|
||||
|
||||
// A well-formed npm package name passes route validation and reaches
|
||||
// the loader (which fails here because npm cannot resolve the package
|
||||
// in the test environment) — proving the 403 floor did not trigger.
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/install")
|
||||
.send({ packageName: `paperclip-plugin-guard-missing-${randomUUID().slice(0, 8)}` });
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("npm install failed");
|
||||
expect(mockLifecycle.load).not.toHaveBeenCalled();
|
||||
}, 150_000);
|
||||
});
|
||||
});
|
||||
|
|
@ -84,6 +84,11 @@ import {
|
|||
import {
|
||||
extractSecretRefBindingsFromConfig,
|
||||
} from "../services/plugin-secrets-handler.js";
|
||||
import {
|
||||
canonicalizeLocalPluginPath,
|
||||
isCloudManagedInstance,
|
||||
isWithinBundledPluginRoot,
|
||||
} from "../services/plugin-install-guard.js";
|
||||
import { secretService } from "../services/secrets.js";
|
||||
import { badRequest, forbidden, notFound, unauthorized, unprocessable } from "../errors.js";
|
||||
|
||||
|
|
@ -1104,10 +1109,18 @@ export function pluginRoutes(
|
|||
* 3. Registers in the database
|
||||
* 4. Transitions to `ready` state if no new capability approval is needed
|
||||
*
|
||||
* Cloud-managed instances (identified by the harness-injected
|
||||
* `PAPERCLIP_MANAGED_CONFIG` environment variable) enforce a positive
|
||||
* allowlist: only local paths that canonicalize to a directory inside the
|
||||
* bundled plugin catalog root may be installed. npm/registry installs and
|
||||
* arbitrary local paths are rejected with `403`. Local paths are
|
||||
* canonicalized and validated on every instance.
|
||||
*
|
||||
* Response: `PluginRecord`
|
||||
*
|
||||
* Errors:
|
||||
* - `400` — validation failure or install error (package not found, bad manifest, etc.)
|
||||
* - `403` — install source not permitted on a cloud-managed instance
|
||||
* - `500` — installation succeeded but manifest is missing (indicates a loader bug)
|
||||
*/
|
||||
router.post("/plugins/install", async (req, res) => {
|
||||
|
|
@ -1143,9 +1156,39 @@ export function pluginRoutes(
|
|||
return;
|
||||
}
|
||||
|
||||
// Cloud install floor: on harness-managed instances only bundled-catalog
|
||||
// sources are installable, regardless of actor privileges or flag state.
|
||||
const cloudManaged = isCloudManagedInstance();
|
||||
if (cloudManaged && !isLocalPath) {
|
||||
res.status(403).json({
|
||||
error:
|
||||
"npm installs are disabled on cloud-managed instances; only plugins bundled with the application may be installed",
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// Canonicalize local install paths on every instance so traversal
|
||||
// segments and symlinks cannot smuggle an aliased path past validation.
|
||||
let canonicalLocalPath: string | undefined;
|
||||
if (isLocalPath) {
|
||||
const validated = await canonicalizeLocalPluginPath(trimmedPackage);
|
||||
if (!validated.ok) {
|
||||
res.status(400).json({ error: `Invalid localPath: ${validated.reason}` });
|
||||
return;
|
||||
}
|
||||
if (cloudManaged && !(await isWithinBundledPluginRoot(validated.canonicalPath))) {
|
||||
res.status(403).json({
|
||||
error:
|
||||
"cloud-managed instances may only install plugins from the bundled plugin catalog",
|
||||
});
|
||||
return;
|
||||
}
|
||||
canonicalLocalPath = validated.canonicalPath;
|
||||
}
|
||||
|
||||
try {
|
||||
const installOptions = isLocalPath
|
||||
? { localPath: trimmedPackage }
|
||||
const installOptions = canonicalLocalPath !== undefined
|
||||
? { localPath: canonicalLocalPath }
|
||||
: { packageName: trimmedPackage, version: version?.trim() };
|
||||
|
||||
const discovered = await loader.installPlugin(installOptions);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,132 @@
|
|||
/**
|
||||
* @fileoverview Security guard for plugin install sources.
|
||||
*
|
||||
* Two related protections for `POST /api/plugins/install`:
|
||||
*
|
||||
* 1. **Cloud install floor.** Instances managed by the Paperclip Cloud
|
||||
* harness receive a `PAPERCLIP_MANAGED_CONFIG` environment document that
|
||||
* only the harness can inject. On such instances, plugin installation is
|
||||
* a remote-code-execution surface on shared infrastructure, so the route
|
||||
* enforces a positive allowlist: only install sources that canonicalize
|
||||
* to a path inside the bundled plugin catalog root may be installed.
|
||||
* npm/registry installs and arbitrary `localPath` installs are rejected.
|
||||
*
|
||||
* 2. **`localPath` canonicalization for every instance.** The install route
|
||||
* historically skipped its package-name character check when
|
||||
* `isLocalPath` was set, passing the raw request string straight to the
|
||||
* loader. All local install paths are now canonicalized (absolute
|
||||
* resolution + symlink/`..` normalization via `realpath`) and validated
|
||||
* to be readable directories before the loader sees them.
|
||||
*
|
||||
* The floor is enforced in code at the route, independent of any feature
|
||||
* flag or of the managed-config document's *content*: a corrupted flag
|
||||
* document cannot widen the install surface because this module never reads
|
||||
* the document body at all (see `isCloudManagedInstance`).
|
||||
*/
|
||||
|
||||
import { realpath, stat } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { BUNDLED_LOCAL_PLUGIN_ROOT } from "./plugin-loader.js";
|
||||
|
||||
/**
|
||||
* Environment key carrying the harness-injected managed-instance document.
|
||||
*
|
||||
* The full document contract (`mode: "cloud"`, feature overlay, plugin
|
||||
* auto-install list) is parsed fail-closed elsewhere at startup; this module
|
||||
* only cares about the variable's presence.
|
||||
*/
|
||||
export const MANAGED_CONFIG_ENV_KEY = "PAPERCLIP_MANAGED_CONFIG";
|
||||
|
||||
/**
|
||||
* Whether this instance is managed by the Paperclip Cloud harness.
|
||||
*
|
||||
* Deliberately presence-based rather than content-based: the strict startup
|
||||
* parser refuses to boot a managed instance with a malformed document, and
|
||||
* absent env means self-hosted. Deciding the security floor on presence
|
||||
* alone means a corrupted, truncated, or attacker-influenced document can
|
||||
* never *disable* the floor — the failure mode is closed, not open.
|
||||
*
|
||||
* @param env - Raw environment map (injectable for tests; defaults to `process.env`)
|
||||
*/
|
||||
export function isCloudManagedInstance(
|
||||
env: Record<string, string | undefined> = process.env,
|
||||
): boolean {
|
||||
return env[MANAGED_CONFIG_ENV_KEY] !== undefined;
|
||||
}
|
||||
|
||||
/** Result of canonicalizing a requested local plugin install path. */
|
||||
export type LocalPluginPathValidation =
|
||||
| { ok: true; canonicalPath: string }
|
||||
| { ok: false; reason: string };
|
||||
|
||||
/**
|
||||
* Canonicalize and validate a raw `localPath` install source.
|
||||
*
|
||||
* Resolves the request string to an absolute path, then to its real path —
|
||||
* collapsing `..` traversal segments and resolving every symlink — and
|
||||
* requires the result to be an existing directory. Downstream checks (the
|
||||
* cloud catalog containment test, the loader itself) must only ever see the
|
||||
* canonical form so that no alias of a path can reach a different decision
|
||||
* than the path itself.
|
||||
*
|
||||
* @param rawPath - The unsanitized `packageName` value from the request body
|
||||
*/
|
||||
export async function canonicalizeLocalPluginPath(
|
||||
rawPath: string,
|
||||
): Promise<LocalPluginPathValidation> {
|
||||
if (rawPath.includes("\0")) {
|
||||
return { ok: false, reason: "path contains a null byte" };
|
||||
}
|
||||
|
||||
const absolutePath = path.resolve(rawPath);
|
||||
|
||||
let canonicalPath: string;
|
||||
try {
|
||||
canonicalPath = await realpath(absolutePath);
|
||||
} catch {
|
||||
return { ok: false, reason: `path does not exist: ${absolutePath}` };
|
||||
}
|
||||
|
||||
try {
|
||||
const stats = await stat(canonicalPath);
|
||||
if (!stats.isDirectory()) {
|
||||
return { ok: false, reason: `path is not a directory: ${canonicalPath}` };
|
||||
}
|
||||
} catch {
|
||||
return { ok: false, reason: `path is not readable: ${canonicalPath}` };
|
||||
}
|
||||
|
||||
return { ok: true, canonicalPath };
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a canonical path lies strictly inside the bundled plugin catalog
|
||||
* root (`packages/plugins` in the application bundle).
|
||||
*
|
||||
* The catalog root itself is also canonicalized before comparison so a
|
||||
* symlinked deployment layout cannot produce false negatives, and the
|
||||
* containment test is segment-based (`path.relative`), never a string-prefix
|
||||
* check. The root itself does not count as inside — an install source must
|
||||
* be a package directory *within* the catalog.
|
||||
*
|
||||
* @param canonicalPath - A path already canonicalized by {@link canonicalizeLocalPluginPath}
|
||||
* @param bundledRootOverride - Catalog root override for tests; defaults to
|
||||
* {@link BUNDLED_LOCAL_PLUGIN_ROOT}
|
||||
*/
|
||||
export async function isWithinBundledPluginRoot(
|
||||
canonicalPath: string,
|
||||
bundledRootOverride?: string,
|
||||
): Promise<boolean> {
|
||||
const bundledRoot = bundledRootOverride ?? BUNDLED_LOCAL_PLUGIN_ROOT;
|
||||
|
||||
let canonicalRoot: string;
|
||||
try {
|
||||
canonicalRoot = await realpath(bundledRoot);
|
||||
} catch {
|
||||
// No catalog root on disk means nothing is bundled; fail closed.
|
||||
return false;
|
||||
}
|
||||
|
||||
const relative = path.relative(canonicalRoot, canonicalPath);
|
||||
return relative.length > 0 && !relative.startsWith("..") && !path.isAbsolute(relative);
|
||||
}
|
||||
Loading…
Reference in New Issue