Security Enhancement: Switch to spawn/execFile for Lagoon CLI command execution

[coderabbitai]

Background

As identified in PR #8, the current approach of using string interpolation when executing Lagoon CLI commands creates potential command injection vulnerabilities.

Problem

Currently, command arguments (like instance names, project names, branch names, etc.) are interpolated directly into shell command strings:

const command = `lagoon -l ${instance} -p ${project} deploy branch --branch ${branch} --output-json`;
const { stdout } = await execLagoonCommand(command, `Deploy Branch ${branch} to ${project}`);

While we've implemented regex validation in some places, this approach is still vulnerable to command injection if an argument contains shell metacharacters.

Proposed Solution

Replace string interpolation with Node.js spawn or execFile functions to pass arguments securely:

const { stdout } = await util.promisify(child_process.execFile)('lagoon', [
  '-l', instance,
  '-p', project,
  'deploy', 'branch',
  '--branch', branch,
  '--output-json'
]);

Or using spawn with proper output handling:

const lagoonProcess = child_process.spawn('lagoon', [
  '-l', instance,
  '-p', project,
  'deploy', 'branch',
  '--branch', branch,
  '--output-json'
]);

// Handle output streams appropriately

Implementation Plan

1. Create a new secure wrapper function for Lagoon CLI calls
2. Refactor all instances of execLagoonCommand to use the new wrapper
3. Update the logging mechanism to work with the new approach
4. Add tests to verify the security of the implementation

Benefits

• Eliminates command injection vulnerabilities
• More maintainable and secure code
• No need for complex regex validation of input parameters
• Explicit separation of command and arguments improves readability

Related

• PR [#1] Implemented branch deployment feature. #8: Implemented branch deployment feature
• Discussion: [#1] Implemented branch deployment feature. #8 (comment)

cc @richardgaunt

[richardgaunt]

Implementation Plan for Security Enhancement

1. Analysis of Current Implementation

The current implementation in lagoon-api.mjs has these key characteristics:

• Uses exec from Node.js child_process
• Commands are constructed using string interpolation (e.g., lagoon -l ${instance} -p ${project})
• Some manual security measures are implemented (e.g., regex validation and escaping in deployBranch)
• All commands follow a similar pattern but with different arguments

2. Proposed Data Structure and Class Design

A. Command Structure Class

class LagoonCommand {
  constructor() {
    this.baseCommand = 'lagoon';
    this.args = [];
  }

  withInstance(instance) {
    if (instance) {
      this.args.push('-l', instance);
    }
    return this;
  }

  withProject(project) {
    if (project) {
      this.args.push('-p', project);
    }
    return this;
  }

  withEnvironment(environment) {
    if (environment) {
      this.args.push('-e', environment);
    }
    return this;
  }

  withJsonOutput() {
    this.args.push('--output-json');
    return this;
  }

  withForce() {
    this.args.push('--force');
    return this;
  }

  // Add command-specific methods
  listProjects() {
    this.args.push('list', 'projects');
    return this;
  }

  listEnvironments() {
    this.args.push('list', 'environments');
    return this;
  }

  listUsers() {
    this.args.push('list', 'all-users');
    return this;
  }

  deleteEnvironment(environmentName) {
    this.args.push('delete', 'environment', '--environment', environmentName);
    return this;
  }

  deployBranch(branchName) {
    this.args.push('deploy', 'branch', '--branch', branchName);
    return this;
  }

  ssh(command) {
    this.args.push('ssh', '-C', command);
    return this;
  }

  getArgs() {
    return this.args;
  }
}

B. Command Executor Class

class LagoonExecutor {
  constructor(logger) {
    this.logger = logger;
  }

  async execute(command, action = 'Unknown Action') {
    const args = command.getArgs();
    console.log(chalk.blue(`Executing: lagoon ${args.join(' ')}`));

    try {
      const result = await this._executeCommand('lagoon', args);
      this.logger.logAction(action, `lagoon ${args.join(' ')}`, 'Success');
      return result;
    } catch (error) {
      this.logger.logError(action, `lagoon ${args.join(' ')}`, error);
      throw error;
    }
  }

  async _executeCommand(command, args) {
    // Use execFile for security
    return util.promisify(child_process.execFile)(command, args);
  }
}

3. Implementation Plan

Phase 1: Create Core Command Structure

1. Create LagoonCommand class that builds commands using an array of arguments
2. Create LagoonExecutor class to execute commands securely using execFile
3. Add unit tests for both classes to ensure they work as expected

Phase 2: Refactor API Functions

1. Update execLagoonCommand to use the new classes:

export async function execLagoonCommand(commandArgs, action = 'Unknown Action') {
  const executor = new LagoonExecutor({ logAction, logError });
  return executor.execute(commandArgs, action);
}

2. Refactor all API functions to use the new command structure. For example:

// Current implementation
const command = `lagoon -l ${instance} -p ${project} list environments --output-json`;
const { stdout } = await execLagoonCommand(command, `List Environments for ${project}`);

// New implementation
const command = new LagoonCommand()
  .withInstance(instance)
  .withProject(project)
  .listEnvironments()
  .withJsonOutput();
const { stdout } = await execLagoonCommand(command, `List Environments for ${project}`);

Phase 3: Special Case Handling

1. Special handling for SSH commands which need shell commands as arguments:

// For SSH commands like drush commands
const sshCommand = new LagoonCommand()
  .withInstance(instance)
  .withProject(project)
  .withEnvironment(environment)
  .ssh("drush cr");

2. Special handling for git ls-remote which isn't a Lagoon command:

// Create a separate GitCommand class for git commands
class GitCommand {
  constructor() {
    this.baseCommand = 'git';
    this.args = [];
  }

  lsRemote(gitUrl) {
    this.args = ['ls-remote', '--heads', gitUrl];
    return this;
  }

  getArgs() {
    return this.args;
  }
}

4. Benefits and Considerations

Benefits:

1. Security: Eliminates command injection vulnerabilities by using execFile which doesn't use a shell
2. Code Organization: Commands are structured and self-documenting
3. Maintainability: Easier to add new commands and modify existing ones
4. Type Safety: Better IDE support and type checking
5. Testability: Easier to mock and test command execution

Considerations:

1. Migration Effort: Need to refactor all command executions across the codebase
2. Special Cases: SSH commands that execute shell commands need careful handling
3. Error Handling: Ensure errors are properly propagated and logged
4. Performance: Consider the slight overhead of object creation for each command

5. Future Extensibility

This architecture allows for:

1. Easy addition of new Lagoon commands
2. Implementation of command caching if needed
3. Adding validation rules for different command types
4. Supporting dry-run mode for testing
5. Adding metrics/telemetry around command execution

6. Implementation Steps

1. Create new files for the command structure and executor classes
2. Write tests for the core functionality
3. Update execLagoonCommand to use the new approach
4. Refactor one API endpoint at a time, testing thoroughly
5. Add specific implementation for special cases (SSH, git)
6. Update documentation and add examples

The class-based approach provides a more structured way to build and execute commands compared to a simple object-based approach, offering better encapsulation, extension points, and a fluent interface for command building.