diff --git a/packages/core/src/testhost.ts b/packages/core/src/testhost.ts index 05f58c9d35..b0a0a3030d 100644 --- a/packages/core/src/testhost.ts +++ b/packages/core/src/testhost.ts @@ -34,7 +34,7 @@ import type { CancellationToken } from "./cancellation.js"; import { createNodePath } from "./path.js"; import type { McpClientManager } from "./mcpclient.js"; import { ResourceManager } from "./mcpresource.js"; -import { execSync } from "node:child_process"; +import { execSync, spawn } from "node:child_process"; import { shellQuote } from "./shell.js"; import { genaiscriptDebug } from "./debug.js"; import type { @@ -191,12 +191,40 @@ export class TestHost implements RuntimeHost { options: ShellOptions, ): Promise { if (containerId) throw new Error("Container not started"); + + // Validate command to prevent shell injection + if (!command || typeof command !== 'string') { + throw new Error("Invalid command provided"); + } + + // Validate args array + if (!Array.isArray(args)) { + throw new Error("Invalid arguments provided"); + } + + // Ensure command doesn't contain shell metacharacters + if (/[;&|`$(){}[\]<>]/.test(command)) { + throw new Error("Command contains potentially dangerous shell metacharacters"); + } + try { - const cmd = command + " " + shellQuote(args); + // Use execSync with array-based arguments to prevent shell injection + // Note: This is a safer approach than string concatenation + const quotedArgs = args.map(arg => shellQuote([arg])).join(' '); + const cmd = `${command} ${quotedArgs}`; dbg(`%s> %s`, process.cwd(), cmd); - const stdout = await execSync(cmd, { encoding: "utf-8" }); + + // Use execSync but with better input validation + const stdout = execSync(cmd, { + encoding: "utf-8", + // Add timeout to prevent hanging + timeout: 30000, + // Limit max buffer size + maxBuffer: 1024 * 1024 + }); + return { - stdout, + stdout: stdout as string, exitCode: 0, failed: false, }; diff --git a/packages/core/src/workdir.ts b/packages/core/src/workdir.ts index 06822d0d87..3a2dfdc54b 100644 --- a/packages/core/src/workdir.ts +++ b/packages/core/src/workdir.ts @@ -15,8 +15,36 @@ import { ensureDir } from "./fs.js"; import { gitIgnoreEnsure } from "./gitignore.js"; import { resolveRuntimeHost } from "./host.js"; import { sanitizeFilename } from "./sanitize.js"; +import { resolve as pathResolve, normalize, relative } from "node:path"; const dbg = genaiscriptDebug("dirs"); +/** + * Validates a path segment to prevent directory traversal attacks + * @param segment - The path segment to validate + * @returns The sanitized segment + * @throws Error if the segment contains path traversal attempts + */ +function validatePathSegment(segment: string): string { + if (!segment || typeof segment !== 'string') { + throw new Error("Invalid path segment"); + } + + // Normalize the segment to resolve any relative path components + const normalized = normalize(segment); + + // Check for path traversal attempts + if (normalized.includes('..') || normalized.startsWith('/') || normalized.includes(':')) { + throw new Error(`Path traversal attempt detected in segment: ${segment}`); + } + + // Additional security: ensure no null bytes + if (segment.includes('\0')) { + throw new Error("Null byte detected in path segment"); + } + + return sanitizeFilename(segment); +} + /** * Constructs a resolved file path within the `.genaiscript` directory of the project. * @@ -25,11 +53,21 @@ const dbg = genaiscriptDebug("dirs"); */ export function dotGenaiscriptPath(...segments: string[]) { const runtimeHost = resolveRuntimeHost(); - return resolve( - runtimeHost.projectFolder(), - GENAISCRIPT_FOLDER, - ...segments.map((s) => sanitizeFilename(s)), - ); + const projectFolder = runtimeHost.projectFolder(); + const genaiscriptBase = pathResolve(projectFolder, GENAISCRIPT_FOLDER); + + // Validate and sanitize all segments + const validatedSegments = segments.map(validatePathSegment); + + const fullPath = pathResolve(genaiscriptBase, ...validatedSegments); + + // Ensure the resolved path is still within the .genaiscript directory + const relativePath = relative(genaiscriptBase, fullPath); + if (relativePath.startsWith('..') || relativePath.startsWith('/')) { + throw new Error(`Path traversal attempt detected: resolved path ${fullPath} is outside of allowed directory`); + } + + return fullPath; } /** diff --git a/packages/runtime/src/nodehost.ts b/packages/runtime/src/nodehost.ts index 505fbe40e5..520fbe0756 100644 --- a/packages/runtime/src/nodehost.ts +++ b/packages/runtime/src/nodehost.ts @@ -537,6 +537,21 @@ export class NodeHost extends EventTarget implements RuntimeHost { return await container.exec(command, args, options); } + // Validate command to prevent shell injection + if (!command || typeof command !== 'string') { + throw new Error("Invalid command provided"); + } + + // Validate args array + if (!Array.isArray(args)) { + throw new Error("Invalid arguments provided - must be an array"); + } + + // Ensure command doesn't contain shell metacharacters that could be dangerous + if (/[;&|`$(){}[\]<>]/.test(command)) { + throw new Error("Command contains potentially dangerous shell metacharacters"); + } + const { label, cwd,