From 0ef3b320c70d79dbb5f73f8339bef70788f23d9d Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 22 Jul 2026 20:48:41 -0700 Subject: [PATCH] Harden `POST /plugins/install`: canonicalize `localPath` for all instances; bundled-only floor for managed instances (#10067) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **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 --- .../__tests__/plugin-install-guard.test.ts | 161 ++++++++ .../plugin-install-route-security.test.ts | 386 ++++++++++++++++++ server/src/routes/plugins.ts | 47 ++- server/src/services/plugin-install-guard.ts | 132 ++++++ 4 files changed, 724 insertions(+), 2 deletions(-) create mode 100644 server/src/__tests__/plugin-install-guard.test.ts create mode 100644 server/src/__tests__/plugin-install-route-security.test.ts create mode 100644 server/src/services/plugin-install-guard.ts diff --git a/server/src/__tests__/plugin-install-guard.test.ts b/server/src/__tests__/plugin-install-guard.test.ts new file mode 100644 index 0000000000..db14e6b3d6 --- /dev/null +++ b/server/src/__tests__/plugin-install-guard.test.ts @@ -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(); + + afterEach(async () => { + for (const cleanupPath of cleanupPaths) { + await rm(cleanupPath, { recursive: true, force: true }); + } + cleanupPaths.clear(); + }); + + async function makeTempDir(prefix: string): Promise { + 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(); + + 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 { + const result = await canonicalizeLocalPluginPath(target); + if (!result.ok) throw new Error(`fixture path did not canonicalize: ${result.reason}`); + return result.canonicalPath; +} diff --git a/server/src/__tests__/plugin-install-route-security.test.ts b/server/src/__tests__/plugin-install-route-security.test.ts new file mode 100644 index 0000000000..ff8ad17be4 --- /dev/null +++ b/server/src/__tests__/plugin-install-route-security.test.ts @@ -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 { + 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) { + 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; + let tempDb: Awaited> | null = null; + const cleanupPaths = new Set(); + 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" }); + + 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); + }); +}); diff --git a/server/src/routes/plugins.ts b/server/src/routes/plugins.ts index 566a200129..8db0353236 100644 --- a/server/src/routes/plugins.ts +++ b/server/src/routes/plugins.ts @@ -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); diff --git a/server/src/services/plugin-install-guard.ts b/server/src/services/plugin-install-guard.ts new file mode 100644 index 0000000000..63497e3b4f --- /dev/null +++ b/server/src/services/plugin-install-guard.ts @@ -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 = 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 { + 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 { + 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); +}