mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 02:26:12 +02:00
**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>
133 lines
5.2 KiB
TypeScript
133 lines
5.2 KiB
TypeScript
/**
|
|
* @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);
|
|
}
|