From 63702e844c7c2479cd080171b9371333dc00eeff Mon Sep 17 00:00:00 2001 From: vastsa Date: Mon, 14 Sep 2026 04:13:52 +0800 Subject: [PATCH 1/2] fix(plugins): harden imported dependency installation --- .../desktop/electron/main/agent-extensions.ts | 445 +++++------------- apps/desktop/electron/main/npm-installer.ts | 408 ++++++++++++++++ .../electron/main/npm-registry-proxy.ts | 112 +++++ apps/desktop/test/agent-extensions.test.mjs | 129 ++++- 4 files changed, 770 insertions(+), 324 deletions(-) create mode 100644 apps/desktop/electron/main/npm-installer.ts create mode 100644 apps/desktop/electron/main/npm-registry-proxy.ts diff --git a/apps/desktop/electron/main/agent-extensions.ts b/apps/desktop/electron/main/agent-extensions.ts index f60ebccbe..b5b62a746 100644 --- a/apps/desktop/electron/main/agent-extensions.ts +++ b/apps/desktop/electron/main/agent-extensions.ts @@ -6,15 +6,17 @@ * and run inside the agent sidecar. This bridge holds what the sidecar reports * back per session (registered commands, load reports, diagnostics) and brokers * the modal prompts between the sidecar and the renderer. Discovery and - * enablement are the plugin system's job; nothing here touches the filesystem. + * enablement are the plugin system's job. Import generation stays here, while + * dependency installation lives in the dedicated npm installer so its trust + * boundary and lifecycle can be tested independently. */ -import { spawn } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; import { copyFileSync, cpSync, existsSync, lstatSync, mkdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs"; import { basename, dirname, extname, isAbsolute, join, relative, resolve, sep } from "node:path"; -import { tmpdir } from "node:os"; import { assertImportedPackagePath, discoverImportedPackageSkills } from "./imported-package-skills"; import { discoverManualPath } from "@pi-desktop/agent-runtime"; +export { defaultDependencyRunner, installExtensionDependencies } from "./npm-installer"; +export type { DependencyCommandRunner, ExtensionDependencyInstallResult } from "./npm-installer"; import { ErrorCodes, TRUSTED_EXTENSION_PROMPT_TIMEOUT_MS, @@ -239,249 +241,40 @@ export class AgentExtensionBridge { } } -export type ExtensionDependencyInstallResult = - | { state: "skipped"; reason: "no-package-json" | "no-dependencies" } - | { state: "installed" } - | { state: "failed"; error: string }; -/** Injectable so tests never run npm. Resolves with the exit code and captured stderr. */ -export type DependencyCommandRunner = ( - command: string, - args: string[], - cwd: string, - timeoutMs: number, -) => Promise<{ code: number; stderr: string }>; - -const NPM_INSTALL_TIMEOUT_MS = 120_000; -/** npm output is not toast-shaped; the tail carries the actual failure. */ -const DEPENDENCY_ERROR_TAIL_CHARS = 200; -/** Rolling cap so a chatty npm cannot balloon the main process's memory. */ -const DEPENDENCY_STDERR_KEEP_CHARS = 8192; - -function dependencyErrorTail(text: string): string { - return text.length > DEPENDENCY_ERROR_TAIL_CHARS - ? `…${text.slice(-DEPENDENCY_ERROR_TAIL_CHARS)}` - : text; -} - -const PUBLIC_NPM_REGISTRY = "https://registry.npmjs.org/"; - -function isRegistryDependencySpec(value: unknown): boolean { - if (typeof value !== "string") return false; - const spec = value.trim(); - // Registry versions and ranges contain no URL/path separators or scheme - // delimiter. This also rejects npm aliases and workspace/local specs. - return spec.length > 0 && !/[\\/:]/.test(spec); -} - -function dependencyMapError(field: string, value: unknown): string | undefined { - if (value === undefined) return undefined; - if (!value || typeof value !== "object" || Array.isArray(value)) { - return `${field} must be a JSON object`; - } - for (const [name, spec] of Object.entries(value as Record)) { - if (!isRegistryDependencySpec(spec)) { - return `dependency ${name} in ${field} uses a non-registry spec (${String(spec)}); only registry versions are installable`; - } - } - return undefined; -} - -function overrideSpecError(value: unknown, path = "overrides"): string | undefined { - if (value === undefined) return undefined; - if (typeof value === "string") { - // npm's $name references reuse a dependency spec already declared above. - if (value.startsWith("$")) return undefined; - if (!isRegistryDependencySpec(value)) { - return `override ${path} uses a non-registry spec (${value}); only registry versions are installable`; - } - return undefined; - } - if (!value || typeof value !== "object" || Array.isArray(value)) { - return `override ${path} must be a JSON object or registry version`; - } - for (const [name, nested] of Object.entries(value as Record)) { - const error = overrideSpecError(nested, `${path}.${name}`); - if (error) return error; - } - return undefined; -} - -function lockfileHasUnsafeSource(value: unknown): boolean { - if (Array.isArray(value)) return value.some(lockfileHasUnsafeSource); - if (!value || typeof value !== "object") return false; - for (const [key, nested] of Object.entries(value as Record)) { - if (key === "resolved") { - if ( - typeof nested !== "string" || - (nested.length > 0 && !nested.startsWith(PUBLIC_NPM_REGISTRY)) - ) { - return true; - } - } - if (key === "link" && nested === true) return true; - if ((key === "version" || key === "from") && typeof nested === "string" && nested.length > 0) { - if (!isRegistryDependencySpec(nested)) return true; - } - if (lockfileHasUnsafeSource(nested)) return true; - } - return false; -} - -export function defaultDependencyRunner( - command: string, - args: string[], - cwd: string, - timeoutMs: number, -): Promise<{ code: number; stderr: string }> { - return new Promise((resolve, reject) => { - // Shell only where npm is a .cmd shim (Windows); every arg is a literal. - // A shell kill on Windows terminates the shim, possibly leaving npm - // itself running — accepted for v1, the timeout result still resolves. - // Explicit minimal environment: npm must not see npm auth tokens, - // proxy/SSH configuration or anything else from the desktop process. - const isolatedUserConfig = join(tmpdir(), `.pi-desktop-npm-user-${randomUUID()}.npmrc`); - const isolatedGlobalConfig = join(tmpdir(), `.pi-desktop-npm-global-${randomUUID()}.npmrc`); - const child = spawn(command, args, { - cwd, - shell: process.platform === "win32", - windowsHide: true, - stdio: ["ignore", "ignore", "pipe"], - env: { - PATH: process.env.PATH ?? "", - HOME: process.env.HOME ?? process.env.USERPROFILE ?? "", - TMPDIR: process.env.TMPDIR ?? process.env.TEMP ?? "", - LANG: process.env.LANG ?? "en_US.UTF-8", - npm_config_userconfig: isolatedUserConfig, - npm_config_globalconfig: isolatedGlobalConfig, - npm_config_registry: PUBLIC_NPM_REGISTRY, - npm_config_proxy: "", - npm_config_https_proxy: "", - npm_config_noproxy: "*", - npm_config_ignore_scripts: "true", - npm_config_audit: "false", - npm_config_fund: "false", - npm_config_update_notifier: "false", - }, - }); - let stderr = ""; - child.stderr?.on("data", (chunk: Buffer) => { - stderr += chunk.toString("utf8"); - if (stderr.length > DEPENDENCY_STDERR_KEEP_CHARS * 2) { - stderr = stderr.slice(-DEPENDENCY_STDERR_KEEP_CHARS); - } - }); - let killTimer: ReturnType | undefined; - const timer = setTimeout(() => { - stderr += `\nnpm install exceeded ${timeoutMs}ms and was terminated`; - child.kill("SIGTERM"); - killTimer = setTimeout(() => child.kill("SIGKILL"), 5_000); - }, timeoutMs); - const settle = (fn: () => void) => { - clearTimeout(timer); - if (killTimer) clearTimeout(killTimer); - fn(); - }; - child.on("error", (err) => { - settle(() => reject(err)); - }); - child.on("close", (code) => { - settle(() => resolve({ code: code ?? 1, stderr })); - }); - }); -} - -/** - * Install an imported extension's npm dependencies inside the generated plugin - * directory (spec 07-plugins/16 §3): the sidecar's jiti resolves bare imports - * from the plugin root's `node_modules`, and the kernel packages (`pi-ai`, - * `pi-coding-agent`, `pi-tui`) keep winning through virtual modules, so - * installing them is harmless. `--legacy-peer-deps` keeps `pi-coding-agent` - * peers out of the tree; `--ignore-scripts` means no third-party install - * script ever runs here — a native module that needs one fails to load with a - * diagnostic instead (documented workaround: rebuild against Electron - * headers). A failure never blocks the import; the extension reports its own - * load error and the renderer surfaces this result. - */ -export async function installExtensionDependencies( - pluginDir: string, - options?: { runner?: DependencyCommandRunner; timeoutMs?: number }, -): Promise { - const packageJsonPath = join(pluginDir, "package.json"); - if (!existsSync(packageJsonPath)) { - return { state: "skipped", reason: "no-package-json" }; - } - let manifest: { - dependencies?: unknown; - optionalDependencies?: unknown; - overrides?: unknown; - }; - try { - manifest = JSON.parse(readFileSync(packageJsonPath, "utf8")); - } catch (err) { - return { - state: "failed", - error: `package.json is not valid JSON: ${err instanceof Error ? err.message : String(err)}`, - }; - } - // JSON.parse("null") succeeds; reading .dependencies below would then throw - // outside this guard and block the whole import. - if (manifest === null || typeof manifest !== "object" || Array.isArray(manifest)) { - return { state: "failed", error: "package.json is not a JSON object" }; - } - for (const field of ["dependencies", "optionalDependencies"] as const) { - const error = dependencyMapError(field, manifest[field]); - if (error) return { state: "failed", error }; - } - const overrideError = overrideSpecError(manifest.overrides); - if (overrideError) return { state: "failed", error: overrideError }; - const hasDependencies = [manifest.dependencies, manifest.optionalDependencies].some( - (value) => value && typeof value === "object" && !Array.isArray(value) && Object.keys(value).length > 0, +const PLUGIN_ID_PREFIX = "imported."; +const NPM_LOCKFILE_NAMES = ["package-lock.json", "npm-shrinkwrap.json"] as const; + +const IMPORT_SENSITIVE_FILE_NAMES = new Set([ + ".npmrc", + ".netrc", + ".pypirc", + ".git-credentials", +]); +const IMPORT_SENSITIVE_DIRECTORY_NAMES = new Set([ + ".git", + ".ssh", + ".aws", + ".gnupg", + ".kube", + ".docker", +]); + +function isSensitiveImportedPath(relativePath: string): boolean { + const parts = relativePath.split(sep).filter(Boolean); + if (parts.some((part) => IMPORT_SENSITIVE_DIRECTORY_NAMES.has(part.toLowerCase()))) return true; + const name = parts.at(-1)?.toLowerCase() ?? ""; + return ( + IMPORT_SENSITIVE_FILE_NAMES.has(name) || + name.startsWith(".env") || + name.startsWith("id_rsa") || + /\.(pem|p12|pfx|keystore)$/.test(name) ); - if (!hasDependencies) { - return { state: "skipped", reason: "no-dependencies" }; - } - // A copied lockfile pins resolved URLs; keep it only when every entry - // resolves from the public registry, otherwise drop it so npm resolves - // from package.json against the default registry. - const lockfileCandidates = ["package-lock.json", "npm-shrinkwrap.json", "yarn.lock", "pnpm-lock.yaml"]; - for (const name of lockfileCandidates) { - const lockPath = join(pluginDir, name); - if (!existsSync(lockPath)) continue; - if (name === "package-lock.json" || name === "npm-shrinkwrap.json") { - try { - const lock = JSON.parse(readFileSync(lockPath, "utf8")) as unknown; - if (lockfileHasUnsafeSource(lock)) rmSync(lockPath); - } catch { - rmSync(lockPath); - } - } else { - rmSync(lockPath); - } - } - try { - const result = await (options?.runner ?? defaultDependencyRunner)( - "npm", - ["install", "--omit=dev", "--legacy-peer-deps", "--no-audit", "--no-fund", "--ignore-scripts"], - pluginDir, - options?.timeoutMs ?? NPM_INSTALL_TIMEOUT_MS, - ); - if (result.code !== 0) { - return { - state: "failed", - error: dependencyErrorTail(`npm install exited ${result.code}: ${result.stderr.trim()}`), - }; - } - return { state: "installed" }; - } catch (err) { - return { - state: "failed", - error: dependencyErrorTail(err instanceof Error ? err.message : String(err)), - }; - } } -const PLUGIN_ID_PREFIX = "imported."; +function importedPathError(message: string): Error & { errorCode?: string } { + return Object.assign(new Error(message), { errorCode: ErrorCodes.INVALID_ARGUMENT }); +} function slugFor(path: string): string { const stem = basename(path, extname(path)) @@ -512,92 +305,114 @@ export function generateImportedExtensionPlugin( // index.js or other incidental script to executable agent extensions. const specs = skillsOnly ? [] : discoverManualPath(resolved); if (specs.length === 0 && skills.length === 0) { - throw Object.assign(new Error("no extension entry found at that path"), { - errorCode: ErrorCodes.INVALID_ARGUMENT, - }); + throw importedPathError("no extension entry found at that path"); + } + for (const spec of specs) { + assertImportedPackagePath(isDirectory ? resolved : dirname(resolved), spec.entry); + if (isSensitiveImportedPath(relative(isDirectory ? resolved : dirname(resolved), spec.entry))) { + throw importedPathError("imported packages cannot contain credential files"); + } } - for (const spec of specs) assertImportedPackagePath(isDirectory ? resolved : dirname(resolved), spec.entry); const slug = slugFor(resolved); - const id = `${PLUGIN_ID_PREFIX}${slug}`; - let dir = join(importRoot, slug); + mkdirSync(importRoot, { recursive: true }); + if (lstatSync(importRoot).isSymbolicLink()) { + throw importedPathError("import destination root cannot be a symbolic link"); + } + let dir: string; let suffix = 2; - while (existsSync(dir)) dir = join(importRoot, `${slug}-${suffix++}`); - const destinationRelative = relative(resolved, resolve(dir)); - if (isDirectory && !isAbsolute(destinationRelative) && !destinationRelative.split(sep).includes("..")) { - throw Object.assign(new Error("import destination cannot be inside the selected package"), { - errorCode: ErrorCodes.INVALID_ARGUMENT, - }); + while (true) { + const candidate = join(importRoot, suffix === 2 ? slug : `${slug}-${suffix}`); + const destinationRelative = relative(resolved, resolve(candidate)); + if (isDirectory && !isAbsolute(destinationRelative) && !destinationRelative.split(sep).includes("..")) { + throw importedPathError("import destination cannot be inside the selected package"); + } + try { + // Atomic creation prevents a pre-existing or racing symlink from being followed. + mkdirSync(candidate); + dir = candidate; + break; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "EEXIST") { + suffix += 1; + continue; + } + throw error; + } } + const id = `${PLUGIN_ID_PREFIX}${basename(dir)}`; const srcDir = join(dir, "src"); try { - mkdirSync(srcDir, { recursive: true }); + mkdirSync(srcDir); if (isDirectory) { - cpSync(resolved, srcDir, { recursive: true, filter: (path) => { - // Exclude dependencies within the selection, not its npm installation ancestors. - if (relative(resolved, path).split(sep).includes("node_modules")) return false; - if (lstatSync(path).isSymbolicLink()) { - throw Object.assign(new Error("imported packages cannot contain a symbolic link"), { - errorCode: ErrorCodes.INVALID_ARGUMENT, - }); - } - return true; - } }); + cpSync(resolved, srcDir, { + recursive: true, + filter: (path) => { + const relativePath = relative(resolved, path); + // Exclude dependencies within the selection, not its npm installation ancestors. + if (relativePath.split(sep).includes("node_modules")) return false; + if (relativePath && isSensitiveImportedPath(relativePath)) return false; + if (lstatSync(path).isSymbolicLink()) { + throw importedPathError("imported packages cannot contain a symbolic link"); + } + return true; + }, + }); } else { cpSync(resolved, join(srcDir, basename(resolved))); } - } catch (error) { - rmSync(dir, { recursive: true, force: true }); - throw error; - } - const entries = specs.map((spec) => - isDirectory - ? `src/${relative(resolved, spec.entry).split("\\").join("/")}` - : `src/${basename(resolved)}`, - ); - const manifest = { - schemaVersion: 1, - id, - name: slug, - version: "0.0.0", - description: `Imported pi extension from ${resolved}`, - main: "main.js", - permissions: [...(entries.length ? ["agent.extension"] : []), ...(skills.length ? ["agent.prompt.inject"] : [])], - contributes: { - ...(entries.length ? { agentExtensions: entries } : {}), - ...(skills.length ? { skills: skills.map((path) => ({ - path: `src/${path}`, - // Distinct directories often use the same SKILL.md basename. - id: `skill-${createHash("sha256").update(path).digest("hex").slice(0, 16)}`, - })) } : {}), - }, - }; - writeFileSync(join(dir, "manifest.json"), JSON.stringify(manifest, null, 2) + "\n", "utf8"); - writeFileSync( - join(dir, "main.js"), - "// Generated by PI-Desktop: declarative skills and/or agent extensions.\nmodule.exports = {};\n", - "utf8", - ); - if (isDirectory) { - const rootPackageJson = join(resolved, "package.json"); - if (existsSync(rootPackageJson)) { - // A `workspaces` field would send npm into the copied sources under - // src/; strip it so the install sees only the declared dependencies. - try { - const pkg = JSON.parse(readFileSync(rootPackageJson, "utf8")) as Record; - if (pkg && typeof pkg === "object" && "workspaces" in pkg) { - delete pkg.workspaces; - writeFileSync(join(dir, "package.json"), JSON.stringify(pkg, null, 2) + "\n", "utf8"); - } else { + const entries = specs.map((spec) => + isDirectory + ? `src/${relative(resolved, spec.entry).split("\\").join("/")}` + : `src/${basename(resolved)}`, + ); + const manifest = { + schemaVersion: 1, + id, + name: slug, + version: "0.0.0", + description: `Imported pi extension from ${resolved}`, + main: "main.js", + permissions: [...(entries.length ? ["agent.extension"] : []), ...(skills.length ? ["agent.prompt.inject"] : [])], + contributes: { + ...(entries.length ? { agentExtensions: entries } : {}), + ...(skills.length ? { skills: skills.map((path) => ({ + path: `src/${path}`, + // Distinct directories often use the same SKILL.md basename. + id: `skill-${createHash("sha256").update(path).digest("hex").slice(0, 16)}`, + })) } : {}), + }, + }; + writeFileSync(join(dir, "manifest.json"), JSON.stringify(manifest, null, 2) + "\n", "utf8"); + writeFileSync( + join(dir, "main.js"), + "// Generated by PI-Desktop: declarative skills and/or agent extensions.\nmodule.exports = {};\n", + "utf8", + ); + if (isDirectory) { + const rootPackageJson = join(resolved, "package.json"); + if (existsSync(rootPackageJson)) { + // A `workspaces` field would send npm into the copied sources under + // src/; strip it so the install sees only the declared dependencies. + try { + const pkg = JSON.parse(readFileSync(rootPackageJson, "utf8")) as Record; + if (pkg && typeof pkg === "object" && "workspaces" in pkg) { + delete pkg.workspaces; + writeFileSync(join(dir, "package.json"), JSON.stringify(pkg, null, 2) + "\n", "utf8"); + } else { + copyFileSync(rootPackageJson, join(dir, "package.json")); + } + } catch { + // Not valid JSON: copy verbatim; the install step reports the failure. copyFileSync(rootPackageJson, join(dir, "package.json")); } - } catch { - // Not valid JSON: copy verbatim; the install step reports the failure. - copyFileSync(rootPackageJson, join(dir, "package.json")); - } - for (const file of ["package-lock.json", "npm-shrinkwrap.json"]) { - if (existsSync(join(resolved, file))) copyFileSync(join(resolved, file), join(dir, file)); + for (const file of NPM_LOCKFILE_NAMES) { + if (existsSync(join(resolved, file))) copyFileSync(join(resolved, file), join(dir, file)); + } } } + return { path: dir, id, entries }; + } catch (error) { + rmSync(dir, { recursive: true, force: true }); + throw error; } - return { path: dir, id, entries }; } diff --git a/apps/desktop/electron/main/npm-installer.ts b/apps/desktop/electron/main/npm-installer.ts new file mode 100644 index 000000000..5e6233c20 --- /dev/null +++ b/apps/desktop/electron/main/npm-installer.ts @@ -0,0 +1,408 @@ +import { spawn } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { existsSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { withRegistryOnlyProxy } from "./npm-registry-proxy"; + +export type ExtensionDependencyInstallResult = + | { state: "skipped"; reason: "no-package-json" | "no-dependencies" } + | { state: "installed" } + | { state: "failed"; error: string }; + +/** Injectable so tests never run npm. Resolves with the exit code and captured stderr. */ +export type DependencyCommandRunner = ( + command: string, + args: string[], + cwd: string, + timeoutMs: number, + envOverrides?: Record, +) => Promise<{ code: number; stderr: string }>; + +const NPM_INSTALL_TIMEOUT_MS = 120_000; +/** npm output is not toast-shaped; the tail carries the actual failure. */ +const DEPENDENCY_ERROR_TAIL_CHARS = 200; +/** Rolling cap so a chatty npm cannot balloon the main process's memory. */ +const DEPENDENCY_STDERR_KEEP_CHARS = 8192; +const PUBLIC_NPM_REGISTRY = "https://registry.npmjs.org/"; +const NPM_LOCKFILE_NAMES = ["package-lock.json", "npm-shrinkwrap.json"] as const; +const NON_NPM_LOCKFILE_NAMES = ["yarn.lock", "pnpm-lock.yaml"] as const; +const LOCKFILE_DEPENDENCY_FIELDS = new Set([ + "dependencies", + "optionalDependencies", + "devDependencies", + "peerDependencies", +]); + +function dependencyErrorTail(text: string): string { + return text.length > DEPENDENCY_ERROR_TAIL_CHARS + ? `…${text.slice(-DEPENDENCY_ERROR_TAIL_CHARS)}` + : text; +} + +function dependencyRunnerError(error: unknown): string { + if (error && typeof error === "object" && "code" in error && error.code === "ENOENT") { + return "npm is not available on PATH; install Node.js/npm before importing dependencies"; + } + return error instanceof Error ? error.message : String(error); +} + +function isRegistryDependencySpec(value: unknown): boolean { + if (typeof value !== "string") return false; + const spec = value.trim(); + // Registry versions and ranges contain no URL/path separators or scheme + // delimiter. This also rejects npm aliases and workspace/local specs. + return spec.length > 0 && !/[\\/:]/.test(spec); +} + +function dependencyMapError(field: string, value: unknown): string | undefined { + if (value === undefined) return undefined; + if (!value || typeof value !== "object" || Array.isArray(value)) { + return `${field} must be a JSON object`; + } + for (const [name, spec] of Object.entries(value as Record)) { + if (!isRegistryDependencySpec(spec)) { + return dependencyErrorTail(`dependency ${name} in ${field} uses a non-registry spec (${String(spec)}); only registry versions are installable`); + } + } + return undefined; +} + +function overrideSpecError(value: unknown, path = "overrides"): string | undefined { + if (value === undefined) return undefined; + const pending: Array<{ value: unknown; path: string }> = [{ value, path }]; + while (pending.length) { + const current = pending.pop(); + if (!current) continue; + if (typeof current.value === "string") { + // npm's $name references reuse a dependency spec already declared above. + if (current.value.startsWith("$")) continue; + if (!isRegistryDependencySpec(current.value)) { + return dependencyErrorTail(`override ${current.path} uses a non-registry spec (${current.value}); only registry versions are installable`); + } + continue; + } + if (!current.value || typeof current.value !== "object" || Array.isArray(current.value)) { + return `override ${current.path} must be a JSON object or registry version`; + } + for (const [name, nested] of Object.entries(current.value as Record)) { + pending.push({ value: nested, path: `${current.path}.${name}` }); + } + } + return undefined; +} + +function unsafeLockfilePackagePath(location: string): boolean { + if (location === "") return false; + const parts = location.split("/"); + return ( + !location.startsWith("node_modules/") || + location.includes("\\") || + parts.includes("..") || + parts.some((part) => part.length === 0) + ); +} + +function lockfileHasUnsafeSource(value: unknown): boolean { + const pending: unknown[] = [value]; + while (pending.length) { + const current = pending.pop(); + if (Array.isArray(current)) { + pending.push(...current); + continue; + } + if (!current || typeof current !== "object") continue; + for (const [key, nested] of Object.entries(current as Record)) { + if (key === "packages" && nested && typeof nested === "object" && !Array.isArray(nested)) { + if (Object.keys(nested).some(unsafeLockfilePackagePath)) return true; + } + if (LOCKFILE_DEPENDENCY_FIELDS.has(key) && nested && typeof nested === "object" && !Array.isArray(nested)) { + for (const spec of Object.values(nested as Record)) { + if (typeof spec === "string" && !isRegistryDependencySpec(spec)) return true; + } + } + if (key === "resolved") { + if ( + typeof nested !== "string" || + (nested.length > 0 && !nested.startsWith(PUBLIC_NPM_REGISTRY)) + ) { + return true; + } + } + if (key === "link" && nested === true) return true; + if ((key === "version" || key === "from") && typeof nested === "string" && nested.length > 0) { + if (!isRegistryDependencySpec(nested)) return true; + } + pending.push(nested); + } + } + return false; +} + +export function defaultDependencyRunner( + command: string, + args: string[], + cwd: string, + timeoutMs: number, + envOverrides?: Record, +): Promise<{ code: number; stderr: string }> { + return new Promise((resolve, reject) => { + // Shell only where npm is a .cmd shim (Windows); every arg is a literal. + // A shell kill on Windows terminates the shim, possibly leaving npm + // itself running — the process-tree kill below handles both. + // Explicit minimal environment: npm must not see npm auth tokens, + // proxy/SSH configuration or anything else from the desktop process. + const isolatedUserConfig = join(tmpdir(), `.pi-desktop-npm-user-${randomUUID()}.npmrc`); + const isolatedGlobalConfig = join(tmpdir(), `.pi-desktop-npm-global-${randomUUID()}.npmrc`); + const isolatedGit = join(tmpdir(), `.pi-desktop-npm-git-${randomUUID()}`); + const isolatedCache = join(cwd, ".npm-cache"); + const child = spawn(command, args, { + cwd, + shell: process.platform === "win32", + detached: process.platform !== "win32", + windowsHide: true, + stdio: ["ignore", "ignore", "pipe"], + env: { + PATH: process.env.PATH ?? "", + HOME: process.env.HOME ?? process.env.USERPROFILE ?? "", + TMPDIR: process.env.TMPDIR ?? process.env.TEMP ?? "", + LANG: process.env.LANG ?? "en_US.UTF-8", + npm_config_userconfig: isolatedUserConfig, + npm_config_globalconfig: isolatedGlobalConfig, + npm_config_registry: PUBLIC_NPM_REGISTRY, + npm_config_proxy: "", + npm_config_https_proxy: "", + npm_config_noproxy: "*", + npm_config_git: isolatedGit, + npm_config_cache: isolatedCache, + npm_config_ignore_scripts: "true", + npm_config_audit: "false", + npm_config_fund: "false", + npm_config_update_notifier: "false", + ...envOverrides, + }, + }); + let stderr = ""; + child.stderr?.on("data", (chunk: Buffer) => { + stderr += chunk.toString("utf8"); + if (stderr.length > DEPENDENCY_STDERR_KEEP_CHARS * 2) { + stderr = stderr.slice(-DEPENDENCY_STDERR_KEEP_CHARS); + } + }); + const killProcessTree = (signal: NodeJS.Signals) => { + const pid = child.pid; + if (!pid) return; + if (process.platform === "win32") { + const killer = spawn("taskkill", ["/PID", String(pid), "/T", "/F"], { + windowsHide: true, + stdio: "ignore", + }); + killer.once("error", () => { + // Best effort: the child close handler still settles the runner. + }); + killer.unref(); + return; + } + try { + process.kill(-pid, signal); + } catch { + child.kill(signal); + } + }; + let killTimer: ReturnType | undefined; + const timer = setTimeout(() => { + stderr += `\nnpm dependency install exceeded ${timeoutMs}ms and was terminated`; + killProcessTree("SIGTERM"); + killTimer = setTimeout(() => killProcessTree("SIGKILL"), 5_000); + }, timeoutMs); + const settle = (fn: () => void) => { + clearTimeout(timer); + if (killTimer) clearTimeout(killTimer); + fn(); + }; + child.on("error", (err) => { + settle(() => reject(err)); + }); + child.on("close", (code) => { + settle(() => resolve({ code: code ?? 1, stderr })); + }); + }); +} + +function sanitizeDependencyLockfiles(pluginDir: string): { + snapshots: Map; + removedUnsafe: boolean; +} { + const snapshots = new Map(); + let removedUnsafe = false; + for (const name of NPM_LOCKFILE_NAMES) { + const lockPath = join(pluginDir, name); + if (!existsSync(lockPath)) continue; + const content = readFileSync(lockPath, "utf8"); + try { + if (lockfileHasUnsafeSource(JSON.parse(content))) { + rmSync(lockPath, { force: true }); + removedUnsafe = true; + } else { + snapshots.set(name, content); + } + } catch { + rmSync(lockPath, { force: true }); + removedUnsafe = true; + } + } + for (const name of NON_NPM_LOCKFILE_NAMES) { + rmSync(join(pluginDir, name), { force: true }); + } + return { snapshots, removedUnsafe }; +} + +function cleanupFailedDependencyInstall( + pluginDir: string, + snapshots: ReadonlyMap, +): void { + try { + rmSync(join(pluginDir, "node_modules"), { recursive: true, force: true }); + } catch { + // Best effort: the install result must remain reportable. + } + try { + rmSync(join(pluginDir, ".npm-cache"), { recursive: true, force: true }); + } catch { + // Best effort: the install result must remain reportable. + } + for (const name of NPM_LOCKFILE_NAMES) { + const lockPath = join(pluginDir, name); + const original = snapshots.get(name); + try { + if (original === undefined) rmSync(lockPath, { force: true }); + else writeFileSync(lockPath, original, "utf8"); + } catch { + // Best effort: the install result must remain reportable. + } + } +} + +function cleanupDependencyCache(pluginDir: string): void { + try { + rmSync(join(pluginDir, ".npm-cache"), { recursive: true, force: true }); + } catch { + // Best effort cleanup after npm exits. + } +} + +/** + * Install an imported extension's npm dependencies inside the generated plugin + * directory. Resolution is bounded and lockfile-checked before `npm ci`; all + * third-party lifecycle scripts remain disabled and failures never block import. + */ +export async function installExtensionDependencies( + pluginDir: string, + options?: { runner?: DependencyCommandRunner; timeoutMs?: number }, +): Promise { + const packageJsonPath = join(pluginDir, "package.json"); + if (!existsSync(packageJsonPath)) { + return { state: "skipped", reason: "no-package-json" }; + } + let manifest: { + dependencies?: unknown; + optionalDependencies?: unknown; + devDependencies?: unknown; + peerDependencies?: unknown; + overrides?: unknown; + }; + try { + manifest = JSON.parse(readFileSync(packageJsonPath, "utf8")); + } catch (err) { + return { + state: "failed", + error: `package.json is not valid JSON: ${err instanceof Error ? err.message : String(err)}`, + }; + } + if (manifest === null || typeof manifest !== "object" || Array.isArray(manifest)) { + return { state: "failed", error: "package.json is not a JSON object" }; + } + for (const field of [ + "dependencies", + "optionalDependencies", + "devDependencies", + "peerDependencies", + ] as const) { + const error = dependencyMapError(field, manifest[field]); + if (error) return { state: "failed", error }; + } + const overrideError = overrideSpecError(manifest.overrides); + if (overrideError) return { state: "failed", error: overrideError }; + let lockfiles: { snapshots: Map; removedUnsafe: boolean }; + try { + lockfiles = sanitizeDependencyLockfiles(pluginDir); + } catch (err) { + return { + state: "failed", + error: `could not inspect dependency lockfile: ${err instanceof Error ? err.message : String(err)}`, + }; + } + const hasDependencies = [manifest.dependencies, manifest.optionalDependencies].some( + (value) => value && typeof value === "object" && !Array.isArray(value) && Object.keys(value).length > 0, + ); + if (!hasDependencies) { + return { state: "skipped", reason: "no-dependencies" }; + } + const runner = options?.runner ?? defaultDependencyRunner; + const timeoutMs = options?.timeoutMs ?? NPM_INSTALL_TIMEOUT_MS; + const deadline = Date.now() + timeoutMs; + const runNpm = (args: string[], envOverrides?: Record) => + runner("npm", args, pluginDir, Math.max(1, deadline - Date.now()), envOverrides); + const failed = (message: string): ExtensionDependencyInstallResult => { + cleanupFailedDependencyInstall(pluginDir, lockfiles.snapshots); + return { state: "failed", error: dependencyErrorTail(message) }; + }; + const execute = async (envOverrides?: Record): Promise => { + try { + const resolution = await runNpm([ + "install", + "--package-lock-only", + "--omit=dev", + "--legacy-peer-deps", + "--no-audit", + "--no-fund", + "--ignore-scripts", + ], envOverrides); + if (resolution.code !== 0) { + return failed(`npm dependency resolution exited ${resolution.code}: ${resolution.stderr.trim()}`); + } + const resolvedLockfiles = sanitizeDependencyLockfiles(pluginDir); + if (resolvedLockfiles.removedUnsafe) { + return failed("npm dependency resolution produced a non-registry lockfile source"); + } + const install = await runNpm([ + "ci", + "--omit=dev", + "--legacy-peer-deps", + "--no-audit", + "--no-fund", + "--ignore-scripts", + ], envOverrides); + if (install.code !== 0) { + return failed(`npm dependency install exited ${install.code}: ${install.stderr.trim()}`); + } + return { state: "installed" }; + } catch (err) { + return failed(dependencyRunnerError(err)); + } + }; + try { + if (options?.runner) return await execute(); + return await withRegistryOnlyProxy((proxyUrl) => + execute({ + npm_config_proxy: proxyUrl, + npm_config_https_proxy: proxyUrl, + npm_config_noproxy: "", + }), + ); + } catch (err) { + return failed(dependencyRunnerError(err)); + } finally { + cleanupDependencyCache(pluginDir); + } +} diff --git a/apps/desktop/electron/main/npm-registry-proxy.ts b/apps/desktop/electron/main/npm-registry-proxy.ts new file mode 100644 index 000000000..416f410be --- /dev/null +++ b/apps/desktop/electron/main/npm-registry-proxy.ts @@ -0,0 +1,112 @@ +import { request as httpRequest, createServer, type IncomingMessage, type ServerResponse } from "node:http"; +import { request as httpsRequest } from "node:https"; +import { connect, type Socket } from "node:net"; + +type ProxyHandler = (proxyUrl: string) => Promise; + +const REGISTRY_HOST = "registry.npmjs.org"; + +function allowedAuthority(host: string, port: string): boolean { + return host.toLowerCase() === REGISTRY_HOST && (port === "80" || port === "443"); +} + +function parseAuthority(authority: string): { host: string; port: string } | undefined { + const match = authority.trim().match(/^([^:]+):(\d+)$/); + if (!match) return undefined; + return { host: match[1], port: match[2] }; +} + +function rejectResponse(response: ServerResponse): void { + response.writeHead(403, { "content-type": "text/plain", connection: "close" }); + response.end("Only registry.npmjs.org is allowed during dependency installation.\n"); +} + +function forwardHttpRequest(request: IncomingMessage, response: ServerResponse): void { + let target: URL; + try { + target = new URL(request.url ?? ""); + } catch { + response.writeHead(400); + response.end("Invalid proxy request.\n"); + return; + } + const port = target.port || (target.protocol === "https:" ? "443" : "80"); + if ((target.protocol !== "http:" && target.protocol !== "https:") || !allowedAuthority(target.hostname, port)) { + rejectResponse(response); + return; + } + const headers = { ...request.headers }; + delete headers["proxy-authorization"]; + delete headers["proxy-connection"]; + headers.host = target.host; + headers.connection = "close"; + const requestOptions = { + hostname: target.hostname, + port, + path: `${target.pathname}${target.search}`, + method: request.method, + headers, + }; + const forward = target.protocol === "https:" ? httpsRequest : httpRequest; + const upstream = forward(requestOptions, (upstreamResponse) => { + response.writeHead(upstreamResponse.statusCode ?? 502, upstreamResponse.headers); + upstreamResponse.pipe(response); + }); + upstream.on("error", () => { + if (!response.headersSent) response.writeHead(502); + response.end("Registry proxy upstream request failed.\n"); + }); + request.pipe(upstream); +} + +function handleConnect(request: IncomingMessage, client: Socket, head: Buffer): void { + const authority = parseAuthority(request.url ?? ""); + if (!authority || !allowedAuthority(authority.host, authority.port)) { + client.end("HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n"); + return; + } + const upstream = connect(Number(authority.port), authority.host); + upstream.once("connect", () => { + client.write("HTTP/1.1 200 Connection Established\r\n\r\n"); + if (head.length) upstream.write(head); + client.pipe(upstream).pipe(client); + }); + upstream.on("error", () => client.destroy()); +} + +async function closeServer(server: ReturnType, sockets: Set): Promise { + for (const socket of sockets) socket.destroy(); + if (!server.listening) return; + await new Promise((resolve) => server.close(() => resolve())); +} + +/** Run an operation with a loopback proxy that permits only the public npm registry. */ +export async function withRegistryOnlyProxy(handler: ProxyHandler): Promise { + const sockets = new Set(); + const server = createServer((request, response) => forwardHttpRequest(request, response)); + server.on("connect", handleConnect); + server.on("connection", (socket) => { + sockets.add(socket); + socket.once("close", () => sockets.delete(socket)); + }); + await new Promise((resolve, reject) => { + const onError = (error: Error) => { + server.off("listening", onListening); + reject(error); + }; + const onListening = () => { + server.off("error", onError); + resolve(); + }; + server.once("error", onError); + server.once("listening", onListening); + server.listen(0, "127.0.0.1"); + }); + try { + const address = server.address(); + if (!address || typeof address === "string") throw new Error("registry proxy did not bind to a TCP port"); + return await handler(`http://127.0.0.1:${address.port}`); + } finally { + await closeServer(server, sockets); + } +} diff --git a/apps/desktop/test/agent-extensions.test.mjs b/apps/desktop/test/agent-extensions.test.mjs index caf492f41..d3efb4ed4 100644 --- a/apps/desktop/test/agent-extensions.test.mjs +++ b/apps/desktop/test/agent-extensions.test.mjs @@ -1,7 +1,8 @@ import assert from "node:assert/strict"; import test from "node:test"; import { register } from "node:module"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { request as httpRequest } from "node:http"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -11,6 +12,7 @@ const { generateImportedExtensionPlugin, installExtensionDependencies, } = await import("../electron/main/agent-extensions.ts"); +const { withRegistryOnlyProxy } = await import("../electron/main/npm-registry-proxy.ts"); function bridge(overrides = {}) { const events = { changed: 0, prompts: [], toasts: [], statuses: [] }; @@ -28,6 +30,29 @@ function bridge(overrides = {}) { return { b, events }; } +test("registry-only proxy rejects non-registry HTTP targets", async () => { + const status = await withRegistryOnlyProxy(async (proxyUrl) => { + const proxy = new URL(proxyUrl); + return new Promise((resolve, reject) => { + const request = httpRequest( + { + hostname: proxy.hostname, + port: Number(proxy.port), + path: "http://127.0.0.1:9/not-allowed", + method: "GET", + }, + (response) => { + response.resume(); + response.once("end", () => resolve(response.statusCode)); + }, + ); + request.once("error", reject); + request.end(); + }); + }); + assert.equal(status, 403); +}); + test("installing a package.json whose JSON body is null or an array fails without throwing", async () => { for (const body of ["null", "[]"]) { const root = mkdtempSync(join(tmpdir(), "ext-deps-null-")); @@ -63,6 +88,10 @@ test("optional dependencies and overrides cannot escape the registry before npm dependencies: { "left-pad": "^1.3.0" }, overrides: { "left-pad": "https://evil.example/left-pad.tgz" }, }, + { + dependencies: { "left-pad": "^1.3.0" }, + devDependencies: { evil: "git+ssh://git@evil.example/evil.git" }, + }, ]; for (const packageJson of cases) { const root = mkdtempSync(join(tmpdir(), "ext-deps-source-")); @@ -78,6 +107,51 @@ test("optional dependencies and overrides cannot escape the registry before npm assert.match(String(result.error), /non-registry spec/); assert.equal(npmRan, false); } + let deepOverrides = {}; + let cursor = deepOverrides; + for (let index = 0; index < 2000; index += 1) { + cursor[`package-${index}`] = {}; + cursor = cursor[`package-${index}`]; + } + cursor.evil = "git+ssh://git@evil.example/evil.git"; + const deepRoot = mkdtempSync(join(tmpdir(), "ext-deps-deep-overrides-")); + writeFileSync(join(deepRoot, "package.json"), JSON.stringify({ dependencies: { ok: "^1" }, overrides: deepOverrides })); + const deepResult = await installExtensionDependencies(deepRoot, { runner: async () => ({ code: 0, stderr: "" }) }); + assert.equal(deepResult.state, "failed"); + assert.match(String(deepResult.error), /non-registry spec/); + +}); + +test("lockfiles reject nested dependency sources and package paths", async () => { + const cases = [ + { + packages: { + "node_modules/root": { + dependencies: { evil: "git+ssh://git@evil.example/evil.git" }, + }, + }, + }, + { + packages: { + "../../outside": { resolved: "https://registry.npmjs.org/evil/-/evil.tgz" }, + }, + }, + ]; + for (const lock of cases) { + const root = mkdtempSync(join(tmpdir(), "ext-deps-lock-nested-")); + writeFileSync(join(root, "package.json"), JSON.stringify({ dependencies: { "left-pad": "^1.3.0" } })); + writeFileSync(join(root, "package-lock.json"), JSON.stringify(lock)); + let npmRan = false; + const result = await installExtensionDependencies(root, { + runner: async () => { + npmRan = true; + return { code: 0, stderr: "" }; + }, + }); + assert.deepEqual(result, { state: "installed" }); + assert.equal(npmRan, true); + assert.equal(existsSync(join(root, "package-lock.json")), false); + } }); test("a lockfile with non-registry resolved urls is dropped before install", async () => { @@ -193,12 +267,13 @@ test("importing a pi extension directory or file generates a plugin holding agen const file = join(root, "solo.ts"); writeFileSync(file, "export default function () {}\n"); const single = generateImportedExtensionPlugin(file, importRoot); - assert.equal(single.id, "imported.solo"); + assert.equal(single.id, `imported.${single.path.split(/[\\/]/).pop()}`); assert.deepEqual(single.entries, ["src/solo.ts"]); // A second import of the same name gets its own directory. const again = generateImportedExtensionPlugin(file, importRoot); - assert.notEqual(again.path, single.path); + assert.notEqual(again.id, single.id); + assert.equal(again.id, `imported.${again.path.split(/[\\/]/).pop()}`); writeFileSync(join(root, "notes.md"), "# no"); assert.throws(() => generateImportedExtensionPlugin(join(root, "notes.md"), importRoot), /no extension entry/); @@ -209,6 +284,9 @@ test("importing a directory keeps its package.json at the plugin root and never const extDir = join(root, "memory-ext"); mkdirSync(join(extDir, "node_modules", "some-dep"), { recursive: true }); writeFileSync(join(extDir, "node_modules", "some-dep", "index.js"), "module.exports = {};"); + mkdirSync(join(extDir, ".git"), { recursive: true }); + writeFileSync(join(extDir, ".env"), "SECRET=do-not-copy\n"); + writeFileSync(join(extDir, ".npmrc"), "//registry.example/:_authToken=secret\n"); writeFileSync(join(extDir, "index.ts"), "export default function () {}\n"); writeFileSync( join(extDir, "package.json"), @@ -219,7 +297,10 @@ test("importing a directory keeps its package.json at the plugin root and never const generated = generateImportedExtensionPlugin(extDir, join(root, "imported")); assert.equal(readFileSync(join(generated.path, "package.json"), "utf8"), readFileSync(join(extDir, "package.json"), "utf8"), "package.json lands at the plugin root for dependency install"); assert.ok(existsSync(join(generated.path, "package-lock.json"))); - assert.ok(!existsSync(join(generated.path, "src", "node_modules")), "node_modules is reinstalled, never copied"); + assert.equal(existsSync(join(generated.path, "src", ".env")), false, "credential files are not copied"); + assert.equal(existsSync(join(generated.path, "src", ".npmrc")), false, "npm config is not copied"); + assert.equal(existsSync(join(generated.path, "src", ".git")), false, "repository metadata is not copied"); + assert.equal(existsSync(join(generated.path, "src", "node_modules")), false, "node_modules is reinstalled, never copied"); const file = join(root, "solo.ts"); writeFileSync(file, "export default function () {}\n"); @@ -274,7 +355,7 @@ test("default runner caps captured stderr and escalates the timeout kill", async 300, ); assert.notEqual(stalled.code, 0, "a stalled install is killed"); - assert.match(stalled.stderr, /exceeded 300ms and was terminated/); + assert.match(stalled.stderr, /dependency install exceeded 300ms and was terminated/); }); test("the default dependency runner isolates npm config sources and proxies", async () => { const { defaultDependencyRunner } = await import("../electron/main/agent-extensions.ts"); @@ -293,6 +374,8 @@ test("the default dependency runner isolates npm config sources and proxies", as assert.equal(childEnv.npm_config_proxy, ""); assert.equal(childEnv.npm_config_https_proxy, ""); assert.equal(childEnv.npm_config_noproxy, "*"); + assert.ok(childEnv.npm_config_git.startsWith(tmpdir())); + assert.equal(childEnv.npm_config_cache, join(process.cwd(), ".npm-cache")); assert.equal(childEnv.NPM_TOKEN, undefined); assert.equal(childEnv.NODE_AUTH_TOKEN, undefined); }); @@ -324,12 +407,14 @@ test("dependency install: skips without a manifest or dependencies, runs npm wit return { code: 0, stderr: "" }; }; assert.deepEqual(await installExtensionDependencies(installed, { runner, timeoutMs: 1234 }), { state: "installed" }); - assert.equal(calls.length, 1); + assert.equal(calls.length, 2); assert.equal(calls[0].command, "npm"); - assert.deepEqual(calls[0].args, ["install", "--omit=dev", "--legacy-peer-deps", "--no-audit", "--no-fund", "--ignore-scripts"]); + assert.deepEqual(calls[0].args, ["install", "--package-lock-only", "--omit=dev", "--legacy-peer-deps", "--no-audit", "--no-fund", "--ignore-scripts"]); + assert.deepEqual(calls[1].args, ["ci", "--omit=dev", "--legacy-peer-deps", "--no-audit", "--no-fund", "--ignore-scripts"]); assert.equal(calls[0].cwd, installed, "npm runs inside the plugin directory"); - assert.equal(calls[0].timeoutMs, 1234); - + assert.equal(calls[1].cwd, installed, "npm ci runs inside the plugin directory"); + assert.ok(calls[0].timeoutMs > 0 && calls[0].timeoutMs <= 1234); + assert.ok(calls[1].timeoutMs > 0 && calls[1].timeoutMs <= 1234); const failing = write("failing", JSON.stringify({ name: "x", dependencies: { "some-dep": "^1" } })); const result = await installExtensionDependencies(failing, { runner: async () => ({ code: 1, stderr: "npm error code ENOTFOUND\nnpm error network unreachable" }), @@ -338,6 +423,21 @@ test("dependency install: skips without a manifest or dependencies, runs npm wit assert.match(result.error, /exited 1/); assert.match(result.error, /ENOTFOUND/); + const partial = write("partial", JSON.stringify({ name: "x", dependencies: { "some-dep": "^1" } })); + const partialResult = await installExtensionDependencies(partial, { + runner: async () => { + mkdirSync(join(partial, "node_modules", "partial"), { recursive: true }); + mkdirSync(join(partial, ".npm-cache"), { recursive: true }); + writeFileSync(join(partial, "node_modules", "partial", "index.js"), "module.exports = {};"); + writeFileSync(join(partial, "package-lock.json"), "{}"); + return { code: 1, stderr: "partial install" }; + }, + }); + assert.equal(partialResult.state, "failed"); + assert.equal(existsSync(join(partial, "node_modules")), false); + assert.equal(existsSync(join(partial, ".npm-cache")), false); + assert.equal(existsSync(join(partial, "package-lock.json")), false); + const throwing = write("throwing", JSON.stringify({ name: "x", dependencies: { "some-dep": "^1" } })); const thrown = await installExtensionDependencies(throwing, { runner: async () => { @@ -345,4 +445,15 @@ test("dependency install: skips without a manifest or dependencies, runs npm wit }, }); assert.deepEqual(thrown, { state: "failed", error: "npm not found" }); + const missing = write("missing-npm", JSON.stringify({ name: "x", dependencies: { "some-dep": "^1" } })); + const missingResult = await installExtensionDependencies(missing, { + runner: async () => { + throw Object.assign(new Error("spawn npm"), { code: "ENOENT" }); + }, + }); + assert.deepEqual(missingResult, { + state: "failed", + error: "npm is not available on PATH; install Node.js/npm before importing dependencies", + }); + }); From 0bb09fa893ade70450ac158658248328e4e49457 Mon Sep 17 00:00:00 2001 From: vastsa Date: Mon, 14 Sep 2026 04:14:03 +0800 Subject: [PATCH 2/2] docs(plugins): document dependency security boundary --- README.md | 2 +- README.zh-CN.md | 2 +- ...agent-extensions-as-plugin-contribution.md | 2 +- ...-imported-extension-dependency-boundary.md | 53 +++++++++++++++++++ docs/adr/README.md | 1 + docs/plugin-development.md | 21 ++++---- docs/spec/06-delivery/04-e2e-test-plan.md | 19 ++++--- docs/spec/07-plugins/06-plugin-packaging.md | 2 +- docs/spec/07-plugins/16-trusted-extensions.md | 53 +++++++++++-------- docs/zh-CN/plugin-development.md | 15 +++--- .../spec/06-delivery/04-e2e-test-plan.md | 13 ++--- .../spec/07-plugins/06-plugin-packaging.md | 2 +- .../spec/07-plugins/16-trusted-extensions.md | 31 ++++++----- 13 files changed, 146 insertions(+), 70 deletions(-) create mode 100644 docs/adr/0243-imported-extension-dependency-boundary.md diff --git a/README.md b/README.md index 016fdead2..1d6597652 100644 --- a/README.md +++ b/README.md @@ -231,7 +231,7 @@ The control endpoint supports project, session, and Agent workflows plus a revie Extensions written for the [pi](https://github.com/badlogic/pi-mono) CLI can run inside PI-Desktop's agent unchanged. -A plugin can list them under `contributes.agentExtensions`, or **Plugins → Import pi extension** can wrap an existing extension file or directory in a plugin. If the directory declares npm `dependencies`, they are installed into the generated plugin before its first load (`--ignore-scripts`, so no third-party install script ever runs). +A plugin can list them under `contributes.agentExtensions`, or **Plugins → Import pi extension** can wrap an existing extension file or directory in a plugin. If the directory declares production or optional npm dependencies, a system `npm` on `PATH` performs a bounded registry-only install before first load (`--ignore-scripts`, so no third-party install script ever runs); release builds do not include standalone Node/npm. They can register tools, slash commands, and hooks on every turn, tool call, and provider request. They run with the same access as the agent's own tools, gated by the `agent.extension` permission. diff --git a/README.zh-CN.md b/README.zh-CN.md index 1feb3b6d9..7c14840fc 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -291,7 +291,7 @@ PI_DESKTOP_MCP_CONTROL=1 **Plugins → Import pi extension** -直接把现有 extension 文件或目录包装成 PI-Desktop 插件。若目录声明了 npm `dependencies`，会在首次加载前安装到生成的插件中（`--ignore-scripts`，绝不运行第三方安装脚本）。 +直接把现有 extension 文件或目录包装成 PI-Desktop 插件。若目录声明了生产或可选 npm `dependencies`，`PATH` 中的系统 `npm` 会在首次加载前执行有界的 registry-only 安装（`--ignore-scripts`，绝不运行第三方安装脚本）；发布版不包含独立 Node/npm。 这些扩展可以注册： diff --git a/docs/adr/0215-agent-extensions-as-plugin-contribution.md b/docs/adr/0215-agent-extensions-as-plugin-contribution.md index 3a1ac72f8..ed1cdc107 100644 --- a/docs/adr/0215-agent-extensions-as-plugin-contribution.md +++ b/docs/adr/0215-agent-extensions-as-plugin-contribution.md @@ -47,7 +47,7 @@ used as one. untouched; only the ownership layer moved. - Sandbox level is unchanged and now explicit as a permission. A sandboxed variant that runs modules in the plugin host process remains a v2 option. -- The v1 E2E harness moved to plugin-form fixtures and passes end to end. +- The v1 harness covers plugin-form fixtures and the importer/runtime contracts; the native picker, npm install, and provider-turn journey remain separate environment-dependent validation. ## Alternatives considered diff --git a/docs/adr/0243-imported-extension-dependency-boundary.md b/docs/adr/0243-imported-extension-dependency-boundary.md new file mode 100644 index 000000000..fa2e38858 --- /dev/null +++ b/docs/adr/0243-imported-extension-dependency-boundary.md @@ -0,0 +1,53 @@ +# ADR 0243: Bound dependency installation for imported extensions + +- Status: Accepted +- Date: 2026-09-14 +- Decision: Imported pi extension dependencies use a registry-only, non-lifecycle npm boundary +- Related: ADR 0214, ADR 0215, `07-plugins/16-trusted-extensions.md` §3.2 + +## Context + +The explicit `Plugins → Import pi extension` flow copies a local pi package and +may install its declared dependencies before the first sidecar load. A normal +`npm install` inherits user configuration and can resolve local paths, git +repositories, private registries, proxies, or HTTP tarballs. `--ignore-scripts` +alone does not provide a sufficient boundary, and a packaged PI-Desktop build +does not ship a standalone Node/npm executable. + +## Decision + +1. The importer validates registry-only specs in `dependencies`, + `optionalDependencies`, `devDependencies`, and `peerDependencies`, plus + recursive `overrides`. Invalid values fail before npm starts. +2. npm lockfiles are accepted only when package locations, nested dependency + specs, and every `resolved` URL are registry-safe. Unsupported lockfile + formats are removed; unsafe lockfiles are not reused. +3. Real npm runs with a minimal environment: user/global config files are + isolated, the registry is fixed to `https://registry.npmjs.org/`, git + resolution is disabled, lifecycle scripts/audit/fund/update notifications + are disabled, and npm uses a per-import cache removed after completion. +4. Real npm traffic goes through a loopback proxy that permits only + `registry.npmjs.org` on ports 80/443. This blocks non-registry transitive + HTTP(S) tarballs and redirects. Git sources are rejected by validation or + the disabled git resolver. +5. Dependency resolution and installation are separate bounded commands: + `npm install --package-lock-only` validates the complete resolved lockfile, + then `npm ci` installs it. A failure removes partial `node_modules`, cache, + and generated lockfiles while preserving a safe source lockfile when possible. +6. Imported copies omit credential files and repository metadata, create the + destination directory atomically, and derive the plugin id from the final + unique directory name. Missing system npm is reported as a clear warning; + the import remains non-blocking but its dependencies do not load. + +## Consequences + +- Private registries, private dependency URLs, git dependencies, local paths, + npm aliases, and lifecycle-build dependencies are intentionally unsupported + by this import path. +- The explicit import flow still requires a user-trusted local extension and a + system npm on `PATH` when dependencies are present. +- The main process owns the proxy and child-process lifecycle; dependency + installation remains outside the renderer and sidecar. +- The dependency install and full picker/provider journey remain separate E2E + validation surfaces; deterministic boundary and cleanup tests cover the + security contract. diff --git a/docs/adr/README.md b/docs/adr/README.md index 54d363252..ae965d03c 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -261,4 +261,5 @@ Each ADR includes: | 0240 | Independent session discovery and navigable collaboration projections | Accepted (amends ADR 0239) | | 0241 | Ship the file view as a vendored, updatable plugin | Accepted (supersedes ADR 0105; issue #304) | | 0242 | Delta-only coalesced streaming updates | Accepted (amends 0127 / 0130 / 0149 / 0153; issue #299) | +| 0243 | Bound dependency installation for imported extensions | Accepted | | active-turn-steering | Bind Composer steering to the active durable turn | Accepted (active-turn-steering; issue #164) | diff --git a/docs/plugin-development.md b/docs/plugin-development.md index c6e4051a2..56f9cf3aa 100644 --- a/docs/plugin-development.md +++ b/docs/plugin-development.md @@ -39,6 +39,7 @@ For the recommended app-first path, you need: - a running PI-Desktop build; - an empty folder for the plugin; and - a text editor. +- To import a pi extension directory with npm dependencies, a system `npm` executable must be on `PATH`. Release builds ship no standalone Node/npm; without it, PI-Desktop reports a warning and the imported dependency cannot load. For the repository CLI path, you also need Node.js 22.19 or newer, pnpm 10 or newer, and a checkout of this repository. The devkit and SDK are currently @@ -725,15 +726,17 @@ What to know before you use it: details, never thrown. - **Existing pi extensions** need no changes: Plugins → overflow menu → "Import pi extension" wraps a file or directory in a generated plugin. If - the directory ships a `package.json` with `dependencies`, they are - installed into the plugin root before the first load - (`npm install --omit=dev --legacy-peer-deps --no-audit --no-fund - --ignore-scripts`): no third-party install script ever runs, so a native - module that needs one fails to load with a diagnostic — rebuild it against - Electron headers (`npx @electron/rebuild -v `) inside - the plugin directory to fix it. A failed install never blocks the import; - you get a warning toast with the npm error and the row shows the load - error. + the directory declares production or optional dependencies, PI-Desktop first + runs `npm install --package-lock-only --omit=dev --legacy-peer-deps --no-audit + --no-fund --ignore-scripts`, validates the generated registry-only lockfile, + then runs `npm ci` with the same safety flags. Direct dependency specs are + checked across production, optional, dev, and peer fields; git resolution is + disabled, and no third-party lifecycle script runs. A native module that + needs a build script fails with a diagnostic — rebuild it against Electron + headers (`npx @electron/rebuild -v `) inside the plugin + directory to fix it. Failed installs clean partial dependencies and report a + warning toast without blocking the import; the row shows a load error only if + the extension actually fails to load. ## 7. Permission design diff --git a/docs/spec/06-delivery/04-e2e-test-plan.md b/docs/spec/06-delivery/04-e2e-test-plan.md index cbc2c8447..9de790f37 100644 --- a/docs/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/spec/06-delivery/04-e2e-test-plan.md @@ -10440,7 +10440,8 @@ sample extensions under `apps/desktop/test/fixtures/pi-extensions/`. `agent.prompt.inject`, and Host reports the `skills` capability while retaining both skill documents. Invalid declarations fail without importing outside data or retaining a partial copied plugin. Nothing automatically - imports `~/.pi` or runs npm/package lifecycle scripts. + imports `~/.pi` or runs npm lifecycle scripts; the explicit dependency path + may run the bounded npm installer described in E2E-PLUGIN-import-extension-installs-dependencies. - **Specs linked**: `07-plugins/16-trusted-extensions.md` §3.2; `07-plugins/02-plugin-manifest-schema.md`; D007 - **Acceptance**: E (tools & permissions), G (plugins), Quality @@ -10546,12 +10547,14 @@ sample extensions under `apps/desktop/test/fixtures/pi-extensions/`. Inspect `plugins/imported//`. 3) Send a prompt that exercises the extension. - **Expected**: The plugin root holds the copied `package.json` with any - `workspaces` field stripped and a `node_modules` directory created by - `npm install --omit=dev --legacy-peer-deps --no-audit --no-fund - --ignore-scripts` (no install script ran); the extension row reaches - `loaded` with its tools, commands, and hooks registered, and they take - effect in the turn. -- **Specs linked**: `07-plugins/16-trusted-extensions.md` §3.2, §10.2 + `workspaces` field stripped. Dependency resolution first runs + `npm install --package-lock-only --omit=dev --legacy-peer-deps --no-audit + --no-fund --ignore-scripts`, validates registry-only sources, and then + creates `node_modules` with `npm ci --omit=dev --legacy-peer-deps --no-audit + --no-fund --ignore-scripts` (no install script ran); the extension row reaches + `loaded` with its tools, commands, and hooks registered, and they take effect + in the turn. +- **Specs linked**: `07-plugins/16-trusted-extensions.md` §3.2, §10.2; ADR 0243 - **Acceptance**: Security, Quality - **Milestone**: Post-MVP (R7 v1) - **Status**: Unit-covered by `apps/desktop/test/agent-extensions.test.mjs` @@ -10570,7 +10573,7 @@ sample extensions under `apps/desktop/test/fixtures/pi-extensions/`. tail; the plugin still registers; the row reports the extension `error` state with a `load_error` diagnostic; the session and all other extensions keep working. -- **Specs linked**: `07-plugins/16-trusted-extensions.md` §3.2, §4.4, §10.2 +- **Specs linked**: `07-plugins/16-trusted-extensions.md` §3.2, §4.4, §10.2; ADR 0243 - **Acceptance**: Security, Quality - **Milestone**: Post-MVP (R7 v1) - **Status**: Unit-covered by `apps/desktop/test/agent-extensions.test.mjs` diff --git a/docs/spec/07-plugins/06-plugin-packaging.md b/docs/spec/07-plugins/06-plugin-packaging.md index f266cd224..00c13db37 100644 --- a/docs/spec/07-plugins/06-plugin-packaging.md +++ b/docs/spec/07-plugins/06-plugin-packaging.md @@ -97,7 +97,7 @@ Minimal spec: - Source can be TypeScript - Compile to directly loadable js/html/css before distribution -- Do not rely on the host to run `npm install` on the spot (MVP does not support pulling dependencies at install time) +- Do not rely on the host to run `npm install` for ordinary `.piplug` or development plugins (MVP does not pull dependencies at install time). The explicit Plugins → Import pi extension flow is the documented exception; its bounded npm behavior is defined in `16-trusted-extensions.md` §3.2. If a plugin needs third-party libraries: - Bundle them into the plugin directory yourself diff --git a/docs/spec/07-plugins/16-trusted-extensions.md b/docs/spec/07-plugins/16-trusted-extensions.md index 3ced6dbaa..e89fa7483 100644 --- a/docs/spec/07-plugins/16-trusted-extensions.md +++ b/docs/spec/07-plugins/16-trusted-extensions.md @@ -1,6 +1,6 @@ # 16. Trusted Extensions -> Status: Implemented v1.1 (D387 / D388, ADR 0214 / ADR 0215); implementation notes are marked "v1 note" +> Status: Implemented v1.1 (D387 / D388, ADR 0214 / ADR 0215 / ADR 0243); implementation notes are marked "v1 note" > Scope: v1.1. v2 and v3 items are listed in §12 and are not committed. ## 1. Purpose and terminology @@ -72,10 +72,11 @@ manifest that lists entries without the permission is invalid Plugins → "Import pi extension" opens a native picker (main owns the path, D344) for an explicit local file or directory. Main copies the selected source under `/plugins/imported//src/`, writes a generated -no-op `main.js` and a manifest with id `imported.`, and registers the -directory through the existing local-plugin flow. The confirmation before -the picker remains the trust decision; the generated manifest declares the -permissions needed by its actual contributions. +no-op `main.js` and a manifest with id `imported.` (a unique suffix is +added for repeated imports), and registers the directory through the existing +local-plugin flow. The confirmation before the picker remains the trust +decision; the generated manifest declares the permissions needed by its actual +contributions. For extension files and packages without `pi.skills`, entry discovery keeps the existing `pi-coding-agent` rules: `package.json` `pi.extensions`, otherwise @@ -84,15 +85,17 @@ A package that explicitly declares `pi.skills` and has no `pi.extensions` (or an empty array) is skill-only: incidental scripts, including `index.js`, are copied as resources but never promoted to executable agent extensions. -A directory that ships a `package.json` also has it (plus its lockfile) copied -to the plugin root with any `workspaces` field stripped; if it declares -`dependencies`, main installs them into the plugin root before the first load -with `npm install --omit=dev --legacy-peer-deps --no-audit --no-fund ---ignore-scripts` (bounded time, no third-party install script ever runs, -kernel packages keep resolving through virtual modules). A failed install is -reported to the renderer and never blocks the import — the extension then -reports its own load error. The confirm discloses the npm step alongside the -skills disclosure. +A directory that ships a `package.json` also has it (plus its npm lockfile) +copied to the plugin root with any `workspaces` field stripped. When it declares +production or optional dependencies, main performs a bounded two-step install: +it first resolves `npm install --package-lock-only --omit=dev --legacy-peer-deps +--no-audit --no-fund --ignore-scripts`, validates the complete generated lockfile, +then runs `npm ci` with the same safety flags. Direct specs in `dependencies`, +`optionalDependencies`, `devDependencies`, and `peerDependencies` must be +registry-only because npm may inspect all four; the git resolver is disabled. +No lifecycle script runs. Failed installs remove partial dependencies/cache and +are reported to the renderer without blocking the import. The confirm discloses +the npm step alongside the skills disclosure. | Source | Becomes | |---|---| @@ -120,19 +123,25 @@ Copying uses paths relative to the selected package. A package installed under an ancestor `node_modules` directory is copied normally; only its own `node_modules` directory segments are excluded. References, assets, helper scripts, and other ordinary source files remain under `src/`, preserving -skill-relative resource paths. The selected root is resolved to its real -path. Contribution paths must stay inside that root, cannot traverse `..`, -and cannot point into its dependency directories. Absolute `pi.skills` -paths and descendant symbolic links are rejected. Copying also rejects -symbolic links among retained resources and removes a partial copy on -failure. The generated destination must not be inside the selected source. +skill-relative resource paths. Credential files (`.env*`, `.npmrc`, `.netrc`, +`.pypirc`, private-key and certificate files) and repository metadata directories +are not copied. The selected root is resolved to its real path. Contribution +paths must stay inside that root, cannot traverse `..`, and cannot point into +its dependency directories. Absolute `pi.skills` paths and descendant symbolic +links are rejected. Copying also rejects symbolic links among retained resources +and removes a partial copy on failure. The generated destination is created +atomically and must not be inside the selected source. This is an explicit local import, not a pi CLI package manager. It never automatically scans or imports `~/.pi`, does not read the CLI's installed package registry, and does not run npm lifecycle scripts. When dependencies are declared, the bounded installer accepts only registry version specs and -registry-resolved npm lockfiles; importing a package does not promise that -every third-party extension dependency can execute. +registry-resolved npm lockfiles, rejects unsafe package locations and nested +dependency specs, disables git resolution, and isolates npm's config/cache from +the user's credentials and proxy settings. Importing a package does not promise +that every third-party extension dependency can execute. + +## 4. Loading and runtime ## 4. Loading and runtime diff --git a/docs/zh-CN/plugin-development.md b/docs/zh-CN/plugin-development.md index 5a710e990..09544613e 100644 --- a/docs/zh-CN/plugin-development.md +++ b/docs/zh-CN/plugin-development.md @@ -40,6 +40,7 @@ - 正在运行的 PI-Desktop 版本； - 插件的空文件夹；和 - 文本编辑器。 +- 若要导入带 npm 依赖的 pi 扩展目录，`PATH` 中必须有系统 `npm` 可执行文件。发布版不附带独立 Node/npm；缺少 npm 时，PI-Desktop 会报告警告，导入扩展的依赖无法加载。 对于存储库 CLI 路径，您还需要 Node.js 22.19 或更高版本、pnpm 10 或 较新，并签出此存储库。 devkit 和 SDK 目前已 @@ -633,14 +634,14 @@ export default function (pi) { `ctx.ui.input` / `select` / `confirm` 打开原生对话框；`ui.notify` 是 toast。 - **受支持的成员**见规格 07-plugins/16 §5。不支持的成员（`setWidget`、 `registerMessageRenderer`、`navigateTree` 及其他仅终端可用的界面）是空操作，在插件行 - 的详情里报告，绝不抛出。 - **已有的 pi 扩展**无需修改：插件页 → 溢出菜单 →“导入 pi 扩展”会把文件或目录包成 - 生成的插件。若目录自带声明了 `dependencies` 的 `package.json`，会在首次加载前把 - 依赖安装到插件根（`npm install --omit=dev --legacy-peer-deps --no-audit --no-fund - --ignore-scripts`）：绝不运行第三方安装脚本，因此需要构建脚本的原生模块会以诊断的 - 形式加载失败——在该插件目录内用 Electron 头重建（`npx @electron/rebuild -v - `）即可修复。安装失败绝不阻塞导入；你会收到带 npm 错误的警告 - toast，插件行显示加载错误。 + 生成的插件。若目录声明了生产或可选依赖，PI-Desktop 会先运行 + `npm install --package-lock-only --omit=dev --legacy-peer-deps --no-audit --no-fund + --ignore-scripts`，校验生成的 registry-only lockfile，再以相同安全参数运行 `npm ci`。 + 生产、可选、开发和 peer 字段中的直接依赖 spec 都会校验，git 解析会被禁用，绝不运行 + 第三方生命周期脚本。需要构建脚本的原生模块会以诊断形式加载失败——在插件目录内用 + Electron 头重建（`npx @electron/rebuild -v `）即可修复。安装失败会清理 + 部分依赖并显示警告 toast，不会阻塞导入；只有扩展实际加载失败时插件行才显示 load error。 ## 7.权限设计 diff --git a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md index 1ad9a6e2b..9a881a0d7 100644 --- a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md @@ -6565,11 +6565,12 @@ IPC 请求无法关闭。 无 `node_modules` 的 pi 扩展目录；npm 可达；导入确认已接受。 - **步骤**：1）插件页 → 导入 pi 扩展，选择该目录。2）检查 `plugins/imported//`。 3）发送一个会用到该扩展的提示。 -- **预期**：插件根有复制来的 `package.json`（`workspaces` 字段已被剥离）和由 - `npm install --omit=dev --legacy-peer-deps --no-audit --no-fund --ignore-scripts` - 创建的 `node_modules`（没有运行任何安装脚本）；扩展行达到 `loaded`，工具、命令与 - hooks 均已注册，并在回合中生效。 -- **链接规格**：`07-plugins/16-trusted-extensions.md` §3.2、§10.2 +- **预期**：插件根有复制来的 `package.json`（`workspaces` 字段已被剥离）。依赖解析先运行 + `npm install --package-lock-only --omit=dev --legacy-peer-deps --no-audit --no-fund + --ignore-scripts`，校验 registry-only 来源，再通过 `npm ci --omit=dev --legacy-peer-deps + --no-audit --no-fund --ignore-scripts` 创建 `node_modules`（没有运行任何安装脚本）； + 扩展行达到 `loaded`，工具、命令与 hooks 均已注册，并在回合中生效。 +- **链接规格**：`07-plugins/16-trusted-extensions.md` §3.2、§10.2；ADR 0243 - **验收**：安全、质量 - **里程碑**：MVP 后（R7 v1） - **状态**：由 `apps/desktop/test/agent-extensions.test.mjs` 单元覆盖，并已用 @@ -6584,7 +6585,7 @@ IPC 请求无法关闭。 第二个目录并开始回合。 - **预期**：渲染层出现携带 npm stderr 尾部的警告 toast；插件仍然注册；该行显示扩展 `error` 状态与 `load_error` 诊断；会话与其他扩展均不受影响。 -- **链接规格**：`07-plugins/16-trusted-extensions.md` §3.2、§4.4、§10.2 +- **链接规格**：`07-plugins/16-trusted-extensions.md` §3.2、§4.4、§10.2；ADR 0243 - **验收**：安全、质量 - **里程碑**：MVP 后（R7 v1） - **状态**：由 `apps/desktop/test/agent-extensions.test.mjs`（跳过、失败与无效 manifest diff --git a/docs/zh-CN/spec/07-plugins/06-plugin-packaging.md b/docs/zh-CN/spec/07-plugins/06-plugin-packaging.md index ddcae8723..57d1811f9 100644 --- a/docs/zh-CN/spec/07-plugins/06-plugin-packaging.md +++ b/docs/zh-CN/spec/07-plugins/06-plugin-packaging.md @@ -100,7 +100,7 @@ Load Development Plugin → choose directory → validate → register(source=de - 来源可以是 TypeScript - 分发前编译为可直接加载的 js/html/css -- 不要依赖主机当场运行 `npm install`（MVP 不支持在安装时拉取依赖项） +- 不要依赖主机为普通 `.piplug` 或开发插件当场运行 `npm install`（MVP 不会在安装时拉取依赖项）。明确的“插件 → 导入 pi 扩展”流程是文档规定的例外；其有界 npm 行为见 `16-trusted-extensions.md` §3.2。 如果插件需要第三方库： - 自己将它们捆绑到插件目录中 diff --git a/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md b/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md index 5fe3efecc..eaea0105e 100644 --- a/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md +++ b/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md @@ -63,8 +63,9 @@ agent 循环上注册工具、命令和事件处理器。`ExtensionAPI` 契约 插件页 →“导入 pi 扩展”打开原生选择器（main 拥有路径，D344），由用户明确选择本地 文件或目录。main 把所选源码复制到 `/plugins/imported//src/`，生成空操作 -`main.js` 和 id 为 `imported.` 的 manifest，再通过既有本地插件流程注册。 -选择器之前的确认仍是信任决定；生成的 manifest 只声明实际贡献所需的权限。 +`main.js` 和 id 为 `imported.` 的 manifest（重复导入时追加唯一后缀），再通过 +既有本地插件流程注册。选择器之前的确认仍是信任决定；生成的 manifest 只声明实际贡献 +所需的权限。 扩展文件及未声明 `pi.skills` 的包保持既有 `pi-coding-agent` 入口发现规则：先取 `package.json` 的 `pi.extensions`，否则取 `index.ts` / `index.js`，再否则取一层深度内 @@ -72,12 +73,14 @@ agent 循环上注册工具、命令和事件处理器。`ExtensionAPI` 契约 的包视为仅技能包：包括 `index.js` 在内的附带脚本作为资源复制，不会被提升为可执行的 Agent 扩展。 -目录若自带 `package.json`，会（连同其 lockfile）一并复制到插件根并剥离 `workspaces` 字段； -若声明了 `dependencies`，main 会在首次加载前把依赖安装到插件根，命令为 -`npm install --omit=dev --legacy-peer-deps --no-audit --no-fund --ignore-scripts` -（限时执行、绝不运行第三方安装脚本、内核包继续经 virtual modules 解析）。安装失败会上报 -渲染层且绝不阻塞导入——扩展随后上报自身的 load error。确认对话框会与技能披露一并说明 -npm 安装步骤。 +目录若自带 `package.json`，会（连同其 npm lockfile）一并复制到插件根并剥离 `workspaces` 字段。 +若声明了生产或可选依赖，main 会在首次加载前执行有界的两阶段安装：先运行 +`npm install --package-lock-only --omit=dev --legacy-peer-deps --no-audit --no-fund +--ignore-scripts` 并校验完整生成的 lockfile，再使用相同安全参数运行 `npm ci`。 +`dependencies`、`optionalDependencies`、`devDependencies` 和 `peerDependencies` 中的 +直接 spec 都必须来自 registry，因为 npm 可能检查全部四者；git resolver 会被禁用。 +不会运行生命周期脚本。安装失败会清理部分依赖/cache、上报渲染层且不阻塞导入。确认对话框 +会与技能披露一并说明 npm 安装步骤。 | 来源 | 结果 | |---|---| @@ -98,14 +101,16 @@ npm 安装步骤。 复制时按所选包的相对路径判断排除项。包的祖先路径含 `node_modules` 不影响复制， 只排除包自身依赖目录中的 `node_modules` 路径段。引用文档、素材、辅助脚本及其他 -普通源码文件保留在 `src/` 下，使技能的相对资源引用仍然成立。所选根目录先解析为真实 -路径；贡献路径必须位于根目录内，不能包含 `..` 穿越，也不能指向包内依赖目录。 -绝对 `pi.skills` 路径与后代符号链接会被拒绝；复制保留资源时也拒绝符号链接，复制失败 -会清理部分生成的目录。生成目标不能位于所选源目录内。 +普通源码文件保留在 `src/` 下，使技能的相对资源引用仍然成立。凭据文件（`.env*`、 +`.npmrc`、`.netrc`、`.pypirc`、私钥和证书文件）及仓库元数据目录不会被复制。所选 +根目录先解析为真实路径；贡献路径必须位于根目录内，不能包含 `..` 穿越，也不能指向 +包内依赖目录。绝对 `pi.skills` 路径与后代符号链接会被拒绝；复制保留资源时也拒绝 +符号链接，复制失败会清理部分生成的目录。生成目标以原子方式创建，不能位于所选源目录内。 这是显式本地导入，不是 pi CLI 包管理器：不会自动扫描或导入 `~/.pi`，不会读取 CLI 已安装包注册表，也不会执行 npm 生命周期脚本。声明依赖时，有界安装器只接受 registry -版本说明和 registry 来源的 npm lockfile；导入包不代表其所有第三方扩展依赖都能执行。 +版本说明和 registry 来源的 npm lockfile，拒绝不安全的包路径和嵌套依赖 spec，禁用 git +解析，并隔离 npm 的配置/cache 与用户凭据和代理设置。导入包不代表其所有第三方扩展依赖都能执行。 ## 4. 加载与运行时