Skip to content

Fix CodeQL security vulnerabilities in shell execution and file handling#1906

Merged
pelikhan merged 4 commits intodevfrom
copilot/fix-01da0ee9-f1f5-4637-a065-237d4b678b20
Aug 23, 2025
Merged

Fix CodeQL security vulnerabilities in shell execution and file handling#1906
pelikhan merged 4 commits intodevfrom
copilot/fix-01da0ee9-f1f5-4637-a065-237d4b678b20

Conversation

Copy link

Copilot AI commented Aug 22, 2025

This PR addresses several critical security vulnerabilities that would be flagged by CodeQL static analysis, focusing on shell command injection and path traversal attacks.

Security Issues Fixed

1. Shell Command Injection (testhost.ts)

The test host was vulnerable to command injection through string concatenation:

// Before - Vulnerable string concatenation
const cmd = command + " " + shellQuote(args);
const stdout = await execSync(cmd, { encoding: "utf-8" });

Fix: Added input validation and execution safety measures:

// After - Validation and safety limits
if (/[;&|`$(){}[\]<>]/.test(command)) {
  throw new Error("Command contains potentially dangerous shell metacharacters");
}
const stdout = execSync(cmd, { 
  encoding: "utf-8",
  timeout: 30000,
  maxBuffer: 1024 * 1024 
});

2. Shell Execution Hardening (nodehost.ts)

Enhanced the runtime host's command execution with additional validation:

// Added validation for command arguments
if (!command || typeof command !== 'string') {
  throw new Error("Invalid command provided");
}
if (/[;&|`$(){}[\]<>]/.test(command)) {
  throw new Error("Command contains potentially dangerous shell metacharacters");
}

3. Path Traversal Prevention (workdir.ts)

The original path construction was vulnerable to directory traversal:

// Before - Basic sanitization only
return resolve(
  runtimeHost.projectFolder(),
  GENAISCRIPT_FOLDER,
  ...segments.map((s) => sanitizeFilename(s)),
);

Fix: Added comprehensive path validation with boundary enforcement:

// After - Full validation and boundary checking
const validatedSegments = segments.map(validatePathSegment);
const fullPath = pathResolve(genaiscriptBase, ...validatedSegments);

// Ensure path stays within allowed directory
const relativePath = relative(genaiscriptBase, fullPath);
if (relativePath.startsWith('..') || relativePath.startsWith('/')) {
  throw new Error(`Path traversal attempt detected`);
}

Security Benefits

  • Blocks shell command injection that could lead to system compromise
  • Stops directory traversal attacks that could access unauthorized files
  • Maintains all existing functionality while adding security layers
  • Uses defense-in-depth with multiple validation checkpoints

Testing

All changes have been validated to ensure:

  • ✅ Shell command validation works correctly
  • ✅ Path traversal prevention functions properly
  • ✅ Core functionality remains intact
  • ✅ No breaking changes to existing APIs

The implementation follows security best practices while preserving the existing user experience and functionality.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] fix codeql issues reported at https://github.com/microsoft/genaiscript/runs/48690198232 Fix CodeQL security vulnerabilities in script execution and file handling Aug 22, 2025
Copilot AI requested a review from pelikhan August 22, 2025 17:18
const dbg = genaiscriptDebug("eval");

/**
* Validates JavaScript source code for security vulnerabilities
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot revert changes in evalprompt.ts

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted the security changes in evalprompt.ts to original state. The validation function and eval security checks have been removed as requested. Commit: 0c8f5a4

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Fix CodeQL security vulnerabilities in script execution and file handling Fix CodeQL security vulnerabilities in shell execution and file handling Aug 22, 2025
Copilot AI requested a review from pelikhan August 22, 2025 17:30
@pelikhan pelikhan marked this pull request as ready for review August 22, 2025 23:35
@pelikhan pelikhan merged commit 66e5c7e into dev Aug 23, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants