Skip to content

feat: add static analysis for execa() command injection detection#268

Merged
Mossaka merged 5 commits intomainfrom
copilot/add-static-analysis-execa
Jan 17, 2026
Merged

feat: add static analysis for execa() command injection detection#268
Mossaka merged 5 commits intomainfrom
copilot/add-static-analysis-execa

Conversation

Copy link
Contributor

Copilot AI commented Jan 17, 2026

The codebase uses execa() extensively for shell commands but lacks static analysis to detect unsafe usage patterns that could lead to command injection vulnerabilities.

Changes

  • Added eslint-plugin-security - General security linting for Node.js (detect-child-process, detect-non-literal-fs, etc.)
  • Created custom ESLint rule no-unsafe-execa - Detects:
    • Template literals with expressions in command/arguments
    • String concatenation in command/arguments
    • Variables as command names
    • shell: true option usage
  • Added security review comments to known-safe flagged usages with inline disable + justification

Example Detection

// Flagged as unsafe
execa('docker', [`--filter=name=${userInput}`]);
execa(cmd, ['arg']);
execa('ls', ['-la'], { shell: true });

// Safe patterns (not flagged)
execa('docker', ['ps', '-a']);
execa('docker', ['stop', containerId.toString()]);

Configuration

Rule is set to warn level since it may produce false positives requiring human review. The rule has a comprehensive test suite in eslint-rules/no-unsafe-execa.test.js.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Security] Large execa() attack surface without static analysis</issue_title>
<issue_description>## Priority
Low

Description

The codebase uses execa() extensively for running shell commands, which creates a large attack surface for command injection vulnerabilities. Currently, there's no static analysis to detect unsafe usage patterns.

Impact

  • Severity: Low (assuming current code is safe)
  • Attack Vector: Command injection if user input flows to execa() calls
  • Risk: Arbitrary command execution

Proposed Solution

  1. Add static analysis tool to detect unsafe execa() usage
  2. Consider using ESLint plugin for detecting command injection patterns
  3. Add unit tests for all execa() call sites with untrusted input

Tools to Consider

  • eslint-plugin-security
  • semgrep with custom rules

Effort Estimate

~4 hours

References

Comments on the Issue (you are @copilot in this section)


💡 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.

Copilot AI and others added 2 commits January 17, 2026 04:32
Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
Copilot AI changed the title [WIP] Add static analysis tool for execa() usage feat: add static analysis for execa() command injection detection Jan 17, 2026
Copilot AI requested a review from Mossaka January 17, 2026 04:35
@Mossaka Mossaka marked this pull request as ready for review January 17, 2026 09:13
@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 77.19% 77.19% ➡️ +0.00%
Statements 77.27% 77.27% ➡️ +0.00%
Functions 77.17% 77.17% ➡️ +0.00%
Branches 69.76% 69.76% ➡️ +0.00%

Coverage comparison generated by scripts/ci/compare-coverage.ts

/**
* Checks if a node is a "safe" literal value
*/
function isLiteralOrSafe(node) {

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused function isLiteralOrSafe.

Copilot Autofix

AI 20 days ago

In general, unused helper functions should be removed to improve readability and avoid confusion about intended behavior. Since isLiteralOrSafe is not referenced, the safest fix that does not alter existing functionality is simply to delete its definition block.

Concretely, in eslint-rules/no-unsafe-execa.js, inside the create(context) { ... } body, remove the entire isLiteralOrSafe function definition, from its JSDoc comment starting at line 67 through its closing brace and terminating semicolon at line 104–104/105, keeping the surrounding code intact. No additional imports, methods, or variables are needed; we only delete dead code.

Suggested changeset 1
eslint-rules/no-unsafe-execa.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/eslint-rules/no-unsafe-execa.js b/eslint-rules/no-unsafe-execa.js
--- a/eslint-rules/no-unsafe-execa.js
+++ b/eslint-rules/no-unsafe-execa.js
@@ -64,46 +64,46 @@
       return false;
     }
 
-    /**
-     * Checks if a node is a "safe" literal value
-     */
-    function isLiteralOrSafe(node) {
-      if (!node) return true;
 
-      // Literal strings/numbers
-      if (node.type === 'Literal') {
-        return true;
-      }
 
-      // Simple template literal without expressions
-      if (node.type === 'TemplateLiteral' && node.expressions.length === 0) {
-        return true;
-      }
 
-      // Method calls that return safe values (e.g., toString() on a number)
-      if (
-        node.type === 'CallExpression' &&
-        node.callee.type === 'MemberExpression' &&
-        node.callee.property.name === 'toString' &&
-        node.arguments.length === 0
-      ) {
-        // Check if calling toString on a safe base
-        const obj = node.callee.object;
-        if (obj.type === 'Literal' && typeof obj.value === 'number') {
-          return true;
-        }
-        // Calling toString() on a variable is a common pattern for converting
-        // numeric values to strings (e.g., port.toString(), lineNum.toString())
-        // This is generally safe as toString() doesn't introduce shell metacharacters
-        if (obj.type === 'Identifier') {
-          // We'll allow identifiers calling toString() - common safe pattern
-          return true;
-        }
-      }
 
-      return false;
-    }
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
     /**
      * Checks if a node contains template literal expressions
      */
EOF
Copilot is powered by AI and may make mistakes. Always verify output.
@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2026

📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤

@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2026

🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation...

@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2026

🎬 THE ENDSmoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨

@github-actions
Copy link
Contributor

Smoke Test Results

Last 2 Merged PRs:

Test Results:

  • ✅ GitHub MCP: Retrieved 2 merged PRs successfully
  • ❌ Playwright: Failed (playwright-akamai.azureedge.net blocked by firewall)
  • ✅ File Writing: Created /tmp/gh-aw/agent/smoke-test-copilot-21092095149.txt
  • ✅ Bash Tool: Verified file contents successfully

Status: FAIL

cc @Mossaka @copilot

AI generated by Smoke Copilot

@github-actions
Copy link
Contributor

Smoke Test Results (Claude)

Last 2 merged PRs:

Test Results:
✅ GitHub MCP - PR retrieval successful
✅ Playwright - Page loaded with title "GitHub · Change is constant. GitHub keeps you ahead. · GitHub"
✅ File Writing - Test file created at /tmp/gh-aw/agent/smoke-test-claude-21092095160.txt
✅ Bash Tool - File content verified: Smoke test passed for Claude at Sat Jan 17 09:26:19 UTC 2026

Overall Status: PASS

AI generated by Smoke Claude

@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2026

📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤

@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2026

🎬 THE ENDSmoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨

@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2026

🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation...

@github-actions
Copy link
Contributor

Smoke Test Results - Claude Sonnet 4.5

Last 2 Merged PRs:

Test Results:

  • ✅ GitHub MCP - Fetched recent PRs
  • ✅ Playwright - Navigated to github.com, title verified "GitHub · Change is constant. GitHub keeps you ahead. · GitHub"
  • ✅ File Write - Created test file successfully
  • ✅ Bash - Verified file contents

Status: PASS

AI generated by Smoke Claude

@github-actions
Copy link
Contributor

Smoke Test Results

Last 2 Merged PRs:

Test Results:

  • ✅ GitHub MCP: Retrieved PR titles successfully
  • ❌ Playwright: Browser download failed in firewall environment
  • ✅ File Writing: Created test file at /tmp/gh-aw/agent/smoke-test-copilot-21092322744.txt
  • ✅ Bash Tool: Verified file contents successfully

Overall Status: PARTIAL PASS (3/4 tests passed)

Author: @Mossaka | Assignees: @Mossaka, @copilot

AI generated by Smoke Copilot

@Mossaka Mossaka merged commit f3581f0 into main Jan 17, 2026
41 of 42 checks passed
@Mossaka Mossaka deleted the copilot/add-static-analysis-execa branch January 17, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security] Large execa() attack surface without static analysis

2 participants