From 2170674ee5c4eb8165ce86932223ae8587812828 Mon Sep 17 00:00:00 2001 From: ThankNIXlater Date: Mon, 23 Mar 2026 21:52:54 +0100 Subject: [PATCH 1/2] fix(security): validate path characters in skill removal request (fixes #53) Adds regex validation for baseDir, workspaceDir, and managedSkillsDir in the skill removal endpoint. These values are passed as SSH positional arguments and should only contain safe filesystem characters. Uses the same safe-path pattern established in PR #39 for trashDir validation, ensuring consistency across all SSH-facing inputs. --- src/app/api/gateway/skills/remove/route.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/app/api/gateway/skills/remove/route.ts b/src/app/api/gateway/skills/remove/route.ts index 0fe047c7..2a5f474f 100644 --- a/src/app/api/gateway/skills/remove/route.ts +++ b/src/app/api/gateway/skills/remove/route.ts @@ -17,6 +17,8 @@ const REMOVABLE_SOURCES = new Set([ "openclaw-workspace", ]); +const SAFE_PATH_RE = /^[a-zA-Z0-9_.~\x2F-]+$/; + const normalizeRequired = (value: unknown, field: string): string => { if (typeof value !== "string") { throw new Error(`${field} is required.`); @@ -28,6 +30,14 @@ const normalizeRequired = (value: unknown, field: string): string => { return trimmed; }; +const normalizeRequiredPath = (value: unknown, field: string): string => { + const trimmed = normalizeRequired(value, field); + if (!SAFE_PATH_RE.test(trimmed)) { + throw new Error(`${field} contains invalid characters.`); + } + return trimmed; +}; + const resolveSkillRemovalSshTarget = (): string | null => { const configured = resolveConfiguredSshTarget(process.env); if (configured) return configured; @@ -51,9 +61,9 @@ const normalizeRemoveRequest = (body: unknown): SkillRemoveRequest => { return { skillKey: normalizeRequired(record.skillKey, "skillKey"), source: sourceRaw as RemovableSkillSource, - baseDir: normalizeRequired(record.baseDir, "baseDir"), - workspaceDir: normalizeRequired(record.workspaceDir, "workspaceDir"), - managedSkillsDir: normalizeRequired(record.managedSkillsDir, "managedSkillsDir"), + baseDir: normalizeRequiredPath(record.baseDir, "baseDir"), + workspaceDir: normalizeRequiredPath(record.workspaceDir, "workspaceDir"), + managedSkillsDir: normalizeRequiredPath(record.managedSkillsDir, "managedSkillsDir"), }; }; From b2f313e33e97907c7920de7f8ec0f2382b6719f0 Mon Sep 17 00:00:00 2001 From: earntoshi Date: Wed, 25 Mar 2026 13:14:29 +0100 Subject: [PATCH 2/2] fix: address review feedback on path validation - Map 'invalid characters' error to 400 (was falling through to 500) - Allow colon and backslash in SAFE_PATH_RE for Windows SSH paths (C:\ and C:/) --- src/app/api/gateway/skills/remove/route.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app/api/gateway/skills/remove/route.ts b/src/app/api/gateway/skills/remove/route.ts index 2a5f474f..9eafebde 100644 --- a/src/app/api/gateway/skills/remove/route.ts +++ b/src/app/api/gateway/skills/remove/route.ts @@ -17,7 +17,7 @@ const REMOVABLE_SOURCES = new Set([ "openclaw-workspace", ]); -const SAFE_PATH_RE = /^[a-zA-Z0-9_.~\x2F-]+$/; +const SAFE_PATH_RE = /^[a-zA-Z0-9_.~\x2F:\\-]+$/; const normalizeRequired = (value: unknown, field: string): string => { if (typeof value !== "string") { @@ -88,7 +88,8 @@ export async function POST(request: Request) { message.includes("not a directory") || message.includes("Gateway URL is missing") || message.includes("Invalid gateway URL") || - message.includes("require OPENCLAW_GATEWAY_SSH_TARGET") + message.includes("require OPENCLAW_GATEWAY_SSH_TARGET") || + message.includes("invalid characters") ? 400 : 500; if (status >= 500) {