From e93764350d428401a004a1c0f90772f55d2b2e89 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 15 Feb 2026 04:27:30 +0000 Subject: [PATCH] refactor(install): share safe install path helpers --- src/hooks/install.ts | 54 ++++++++++------------------------ src/infra/install-safe-path.ts | 37 +++++++++++++++++++++++ src/plugins/install.ts | 53 ++++++++++----------------------- 3 files changed, 68 insertions(+), 76 deletions(-) create mode 100644 src/infra/install-safe-path.ts diff --git a/src/hooks/install.ts b/src/hooks/install.ts index 13edb40c13c..da25388d89b 100644 --- a/src/hooks/install.ts +++ b/src/hooks/install.ts @@ -10,6 +10,7 @@ import { resolvePackedRootDir, } from "../infra/archive.js"; import { installPackageDir } from "../infra/install-package-dir.js"; +import { resolveSafeInstallDir, unscopedPackageName } from "../infra/install-safe-path.js"; import { validateRegistryNpmSpec } from "../infra/npm-registry-spec.js"; import { runCommandWithTimeout } from "../process/exec.js"; import { CONFIG_DIR, resolveUserPath } from "../utils.js"; @@ -38,22 +39,6 @@ export type InstallHooksResult = const defaultLogger: HookInstallLogger = {}; -function unscopedPackageName(name: string): string { - const trimmed = name.trim(); - if (!trimmed) { - return trimmed; - } - return trimmed.includes("/") ? (trimmed.split("/").pop() ?? trimmed) : trimmed; -} - -function safeDirName(input: string): string { - const trimmed = input.trim(); - if (!trimmed) { - return trimmed; - } - return trimmed.replaceAll("/", "__").replaceAll("\\", "__"); -} - function validateHookId(hookId: string): string | null { if (!hookId) { return "invalid hook name: missing"; @@ -73,32 +58,17 @@ export function resolveHookInstallDir(hookId: string, hooksDir?: string): string if (hookIdError) { throw new Error(hookIdError); } - const targetDirResult = resolveSafeInstallDir(hooksBase, hookId); + const targetDirResult = resolveSafeInstallDir({ + baseDir: hooksBase, + id: hookId, + invalidNameMessage: "invalid hook name: path traversal detected", + }); if (!targetDirResult.ok) { throw new Error(targetDirResult.error); } return targetDirResult.path; } -function resolveSafeInstallDir( - hooksDir: string, - hookId: string, -): { ok: true; path: string } | { ok: false; error: string } { - const targetDir = path.join(hooksDir, safeDirName(hookId)); - const resolvedBase = path.resolve(hooksDir); - const resolvedTarget = path.resolve(targetDir); - const relative = path.relative(resolvedBase, resolvedTarget); - if ( - !relative || - relative === ".." || - relative.startsWith(`..${path.sep}`) || - path.isAbsolute(relative) - ) { - return { ok: false, error: "invalid hook name: path traversal detected" }; - } - return { ok: true, path: targetDir }; -} - async function ensureOpenClawHooks(manifest: HookPackageManifest) { const hooks = manifest[MANIFEST_KEY]?.hooks; if (!Array.isArray(hooks)) { @@ -188,7 +158,11 @@ async function installHookPackageFromDir(params: { : path.join(CONFIG_DIR, "hooks"); await fs.mkdir(hooksDir, { recursive: true }); - const targetDirResult = resolveSafeInstallDir(hooksDir, hookPackId); + const targetDirResult = resolveSafeInstallDir({ + baseDir: hooksDir, + id: hookPackId, + invalidNameMessage: "invalid hook name: path traversal detected", + }); if (!targetDirResult.ok) { return { ok: false, error: targetDirResult.error }; } @@ -271,7 +245,11 @@ async function installHookFromDir(params: { : path.join(CONFIG_DIR, "hooks"); await fs.mkdir(hooksDir, { recursive: true }); - const targetDirResult = resolveSafeInstallDir(hooksDir, hookName); + const targetDirResult = resolveSafeInstallDir({ + baseDir: hooksDir, + id: hookName, + invalidNameMessage: "invalid hook name: path traversal detected", + }); if (!targetDirResult.ok) { return { ok: false, error: targetDirResult.error }; } diff --git a/src/infra/install-safe-path.ts b/src/infra/install-safe-path.ts new file mode 100644 index 00000000000..012f633a88e --- /dev/null +++ b/src/infra/install-safe-path.ts @@ -0,0 +1,37 @@ +import path from "node:path"; + +export function unscopedPackageName(name: string): string { + const trimmed = name.trim(); + if (!trimmed) { + return trimmed; + } + return trimmed.includes("/") ? (trimmed.split("/").pop() ?? trimmed) : trimmed; +} + +export function safeDirName(input: string): string { + const trimmed = input.trim(); + if (!trimmed) { + return trimmed; + } + return trimmed.replaceAll("/", "__").replaceAll("\\", "__"); +} + +export function resolveSafeInstallDir(params: { + baseDir: string; + id: string; + invalidNameMessage: string; +}): { ok: true; path: string } | { ok: false; error: string } { + const targetDir = path.join(params.baseDir, safeDirName(params.id)); + const resolvedBase = path.resolve(params.baseDir); + const resolvedTarget = path.resolve(targetDir); + const relative = path.relative(resolvedBase, resolvedTarget); + if ( + !relative || + relative === ".." || + relative.startsWith(`..${path.sep}`) || + path.isAbsolute(relative) + ) { + return { ok: false, error: params.invalidNameMessage }; + } + return { ok: true, path: targetDir }; +} diff --git a/src/plugins/install.ts b/src/plugins/install.ts index 287450f5982..3ef4e4bb468 100644 --- a/src/plugins/install.ts +++ b/src/plugins/install.ts @@ -10,6 +10,11 @@ import { resolvePackedRootDir, } from "../infra/archive.js"; import { installPackageDir } from "../infra/install-package-dir.js"; +import { + resolveSafeInstallDir, + safeDirName, + unscopedPackageName, +} from "../infra/install-safe-path.js"; import { validateRegistryNpmSpec } from "../infra/npm-registry-spec.js"; import { runCommandWithTimeout } from "../process/exec.js"; import * as skillScanner from "../security/skill-scanner.js"; @@ -38,23 +43,6 @@ export type InstallPluginResult = | { ok: false; error: string }; const defaultLogger: PluginInstallLogger = {}; - -function unscopedPackageName(name: string): string { - const trimmed = name.trim(); - if (!trimmed) { - return trimmed; - } - return trimmed.includes("/") ? (trimmed.split("/").pop() ?? trimmed) : trimmed; -} - -function safeDirName(input: string): string { - const trimmed = input.trim(); - if (!trimmed) { - return trimmed; - } - return trimmed.replaceAll("/", "__").replaceAll("\\", "__"); -} - function safeFileName(input: string): string { return safeDirName(input); } @@ -108,32 +96,17 @@ export function resolvePluginInstallDir(pluginId: string, extensionsDir?: string if (pluginIdError) { throw new Error(pluginIdError); } - const targetDirResult = resolveSafeInstallDir(extensionsBase, pluginId); + const targetDirResult = resolveSafeInstallDir({ + baseDir: extensionsBase, + id: pluginId, + invalidNameMessage: "invalid plugin name: path traversal detected", + }); if (!targetDirResult.ok) { throw new Error(targetDirResult.error); } return targetDirResult.path; } -function resolveSafeInstallDir( - extensionsDir: string, - pluginId: string, -): { ok: true; path: string } | { ok: false; error: string } { - const targetDir = path.join(extensionsDir, safeDirName(pluginId)); - const resolvedBase = path.resolve(extensionsDir); - const resolvedTarget = path.resolve(targetDir); - const relative = path.relative(resolvedBase, resolvedTarget); - if ( - !relative || - relative === ".." || - relative.startsWith(`..${path.sep}`) || - path.isAbsolute(relative) - ) { - return { ok: false, error: "invalid plugin name: path traversal detected" }; - } - return { ok: true, path: targetDir }; -} - async function installPluginFromPackageDir(params: { packageDir: string; extensionsDir?: string; @@ -225,7 +198,11 @@ async function installPluginFromPackageDir(params: { : path.join(CONFIG_DIR, "extensions"); await fs.mkdir(extensionsDir, { recursive: true }); - const targetDirResult = resolveSafeInstallDir(extensionsDir, pluginId); + const targetDirResult = resolveSafeInstallDir({ + baseDir: extensionsDir, + id: pluginId, + invalidNameMessage: "invalid plugin name: path traversal detected", + }); if (!targetDirResult.ok) { return { ok: false, error: targetDirResult.error }; }