-
Notifications
You must be signed in to change notification settings - Fork 0
[#1] Implemented branch deployment feature. #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
5fbcb05
483d32f
db7a385
20b37c9
ae456ee
60d5734
4ab9d9d
23635bb
2a0bba4
d6fdd08
d80497e
35b2622
5692c6a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,9 @@ import { exec } from 'child_process'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { promisify } from 'util'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import chalk from 'chalk'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { logAction, logError } from './logger'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import fs from 'fs/promises'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import path from 'path'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import os from 'os'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const execAsync = promisify(exec); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -187,3 +190,103 @@ export async function clearDrupalCache(instance, project, environment) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`Failed to clear cache for environment ${environment}: ${error.message}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Helper function to create a temporary directory for git operations | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function createTempDir() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tempDir = path.join(os.tmpdir(), `lagoon-cli-wrapper-${Date.now()}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await fs.mkdir(tempDir, { recursive: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return tempDir; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Helper function to clone a git repository | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function cloneRepository(gitUrl, directory) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const command = `git clone --bare ${gitUrl} ${directory}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await execAsync(command); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logError('Clone Repository', command, error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`Failed to clone repository: ${error.message}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
richardgaunt marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Helper function to list branches in a git repository | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function listBranches(directory) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const command = `cd ${directory} && git branch -r`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { stdout } = await execAsync(command); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Parse the branches from the output | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return stdout | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .split('\n') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map(branch => branch.trim()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter(branch => branch && !branch.includes('HEAD')) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map(branch => branch.replace('origin/', '')); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logError('List Branches', command, error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`Failed to list branches: ${error.message}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
richardgaunt marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Get all branches from a git repository | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export async function getGitBranches(gitUrl) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!gitUrl) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error('Git URL not provided or invalid'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Create a temporary directory | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tempDir = await createTempDir(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Clone the repository | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await cloneRepository(gitUrl, tempDir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // List branches | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const branches = await listBranches(tempDir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Clean up temporary directory | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await fs.rm(tempDir, { recursive: true, force: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return branches; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Make sure to clean up even if an error occurs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await fs.rm(tempDir, { recursive: true, force: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (cleanupError) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error(`Failed to clean up temporary directory: ${cleanupError.message}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw error; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`Failed to get git branches: ${error.message}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Deploy a branch to Lagoon | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export async function deployBranch(instance, project, branch) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Validate branch name to prevent command injection | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!/^[a-zA-Z0-9_.-]+$/.test(branch)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error('Invalid branch name. Branch names must contain only alphanumeric characters, underscores, hyphens, and periods.'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const command = `lagoon -l ${instance} -p ${project} deploy branch --branch ${branch} --output-json`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { stdout } = await execLagoonCommand(command, `Deploy Branch ${branch} to ${project}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Parse the JSON response | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = JSON.parse(stdout); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (response.result === 'success') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| success: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| message: `Branch ${branch} is being deployed to ${project}` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`Failed to deploy branch ${branch}: ${JSON.stringify(response)}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`Failed to deploy branch ${branch}: ${error.message}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Deploy a branch to Lagoon | |
| export async function deployBranch(instance, project, branch) { | |
| try { | |
| // Validate branch name to prevent command injection | |
| if (!/^[a-zA-Z0-9_.-]+$/.test(branch)) { | |
| throw new Error('Invalid branch name. Branch names must contain only alphanumeric characters, underscores, hyphens, and periods.'); | |
| } | |
| const command = `lagoon -l ${instance} -p ${project} deploy branch --branch ${branch} --output-json`; | |
| const { stdout } = await execLagoonCommand(command, `Deploy Branch ${branch} to ${project}`); | |
| // Parse the JSON response | |
| const response = JSON.parse(stdout); | |
| if (response.result === 'success') { | |
| return { | |
| success: true, | |
| message: `Branch ${branch} is being deployed to ${project}` | |
| }; | |
| } else { | |
| throw new Error(`Failed to deploy branch ${branch}: ${JSON.stringify(response)}`); | |
| } | |
| } catch (error) { | |
| throw new Error(`Failed to deploy branch ${branch}: ${error.message}`); | |
| } | |
| } | |
| // Deploy a branch to Lagoon | |
| export async function deployBranch(instance, project, branch) { | |
| try { | |
| // Validate branch name to prevent command injection | |
| - if (!/^[a-zA-Z0-9_.-]+$/.test(branch)) { | |
| + if (!/^[A-Za-z0-9_.\/\-]+$/.test(branch)) { | |
| throw new Error('Invalid branch name. Branch names must contain only alphanumeric characters, underscores, hyphens, and periods.'); | |
| } | |
| const command = `lagoon -l ${instance} -p ${project} deploy branch --branch ${branch} --output-json`; | |
| const { stdout } = await execLagoonCommand(command, `Deploy Branch ${branch} to ${project}`); | |
| // Parse the JSON response | |
| const response = JSON.parse(stdout); | |
| if (response.result === 'success') { | |
| return { | |
| success: true, | |
| message: `Branch ${branch} is being deployed to ${project}` | |
| }; | |
| } else { | |
| throw new Error(`Failed to deploy branch ${branch}: ${JSON.stringify(response)}`); | |
| } | |
| } catch (error) { | |
| throw new Error(`Failed to deploy branch ${branch}: ${error.message}`); | |
| } | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create an issue for adding the long term fix - this will be need to implemented everywhere we call the lagoon command
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@richardgaunt I've created a GitHub issue for implementing the long-term security fix using spawn/execFile for all Lagoon CLI commands. The issue includes:
- Description of the current security vulnerability
- Proposed solution with code examples
- Implementation plan
- Benefits of the new approach
- References to this PR discussion
I've assigned the issue to you as requested. This fix will indeed need to be implemented everywhere the Lagoon command is called to fully eliminate the command injection vulnerability.
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the long-term fix
ISSUE_TITLE="Security Enhancement: Switch to spawn/execFile for Lagoon CLI command execution"
ISSUE_BODY="## Background
As identified in [PR #8](https://github.com/richardgaunt/lagoon-cli-wrapper/pull/8#discussion_r2077246796), 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:
\`\`\`javascript
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:
\`\`\`javascript
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:
\`\`\`javascript
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 #8: Implemented branch deployment feature
- Discussion: https://github.com/richardgaunt/lagoon-cli-wrapper/pull/8#discussion_r2077246796
cc @richardgaunt"
# Create the issue using GitHub CLI
gh issue create --repo richardgaunt/lagoon-cli-wrapper --title "$ISSUE_TITLE" --body "$ISSUE_BODY" --assignee richardgaunt
Length of output: 4613
@richardgaunt I've successfully created issue #11 titled "Security Enhancement: Switch to spawn/execFile for Lagoon CLI command execution" in your repository.
The issue includes detailed information about:
- The current security vulnerability
- Code examples showing the problem
- The proposed solution using
spawn/execFile - A clear implementation plan
- Benefits of making the security enhancement
You can view the issue here: #11
✏️ Learnings added
Learnt from: richardgaunt
PR: richardgaunt/lagoon-cli-wrapper#8
File: src/lagoon-api.js:0-0
Timestamp: 2025-05-07T10:29:17.468Z
Learning: Security improvement: Lagoon CLI commands should use Node.js spawn/execFile functions instead of string interpolation to prevent command injection vulnerabilities.
Learnt from: richardgaunt
PR: richardgaunt/lagoon-cli-wrapper#8
File: src/lagoon-api.js:0-0
Timestamp: 2025-05-07T10:29:17.468Z
Learning: Security improvement: Lagoon CLI commands should use Node.js spawn/execFile functions instead of string interpolation to prevent command injection vulnerabilities.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Uh oh!
There was an error while loading. Please reload this page.