fix: replace create_subprocess_shell by deacon-mp · Pull Request #3265 · mitre/caldera

deacon-mp · 2026-03-15T18:51:02Z

Avoid shell injection risk by using create_subprocess_exec instead.

Description
start_vue_dev_server() in server.py previously launched the Magma Vue development server using asyncio.create_subprocess_shell, which passes the command string to /bin/sh -c. This introduces a shell injection surface: if MAGMA_PATH or any surrounding logic were ever influenced by user-controlled input, shell metacharacters could be interpreted by the shell.

Replaced with asyncio.create_subprocess_exec, which bypasses the shell entirely and invokes npm directly with its arguments as a proper argv list. This is the safer default for any subprocess call where shell features (pipes, redirects, glob expansion) are not required — and they are not required here.

No functional change to runtime behavior.

Type of change
Bug fix (non-breaking change which fixes an issue)
How Has This Been Tested?
tests/security/test_subprocess_exec.py — static analysis test that parses server.py, locates the start_vue_dev_server function body, and asserts:
create_subprocess_shell is not present
create_subprocess_exec is present
Verified the development server still launches correctly via python3 server.py --insecure

pytest tests/security/test_subprocess_exec.py -v

1 passed

Checklist
My code follows the style guidelines of this project
I have performed a self-review of my own code
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works

Copilot

Pull request overview

This PR hardens the Vue dev-server startup in server.py by avoiding shell invocation, and adds a regression test to ensure the safer subprocess API is used.

Changes:

Replace asyncio.create_subprocess_shell("npm run dev", ...) with asyncio.create_subprocess_exec("npm", "run", "dev", ...) in start_vue_dev_server.
Add a security-focused test to assert create_subprocess_shell is not used for starting the Vue dev server.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
`server.py`	Switches Vue dev-server startup from shell-based to exec-based subprocess invocation.
`tests/security/test_subprocess_exec.py`	Adds a regression test intended to prevent reintroducing `create_subprocess_shell` in `start_vue_dev_server`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

server.py

tests/security/test_subprocess_exec.py

Copilot

Pull request overview

This PR hardens the Vue dev-server startup path by removing shell-based subprocess execution and adds a regression test to ensure the safer subprocess API remains in use.

Changes:

Replace asyncio.create_subprocess_shell("npm run dev", ...) with asyncio.create_subprocess_exec("npm", "run", "dev", ...) when starting the Vue dev server.
Improve logging to include the spawned process PID and schedule a background wait() to avoid leaving the child process unreaped.
Add a security-focused test that parses server.py and asserts start_vue_dev_server does not use create_subprocess_shell.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`server.py`	Switches Vue dev-server startup to `create_subprocess_exec`, improving security by avoiding shell invocation.
`tests/security/test_subprocess_exec.py`	Adds an AST-based regression test guarding against reintroducing shell-based subprocess startup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Updates the Vue dev-server startup to avoid invoking a shell (reducing command-injection risk) and adds a regression test to ensure shell-based subprocess creation isn’t reintroduced.

Changes:

Replace asyncio.create_subprocess_shell("npm run dev", ...) with asyncio.create_subprocess_exec("npm", "run", "dev", ...) in start_vue_dev_server.
Log the spawned dev-server PID and schedule proc.wait() in the background.
Add an AST-based security test asserting start_vue_dev_server does not contain create_subprocess_shell.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`server.py`	Switches Vue dev-server startup to `create_subprocess_exec` and logs PID.
`tests/security/test_subprocess_exec.py`	Adds regression test to prevent reintroducing `create_subprocess_shell` in `start_vue_dev_server`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/security/test_subprocess_exec.py

+    func_content = ast.get_source_segment(content, func_node)
+    assert 'create_subprocess_shell' not in func_content
+    assert 'create_subprocess_exec' in func_content


…server Avoid shell injection risk by using create_subprocess_exec instead.

- Capture proc from create_subprocess_exec, log PID, schedule proc.wait() to avoid zombie processes on Vue dev server exit - Rewrite test to use pytest style, ast-based function extraction, and Path-relative server.py resolution Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

ast.get_source_segment() returns None when source offsets are unavailable; assert it is not None before using it in string checks.

Copilot

Pull request overview

Updates the Caldera startup path to launch the Vue dev server without using a shell, and adds a regression test to prevent reintroducing shell-based subprocess execution.

Changes:

Replace asyncio.create_subprocess_shell("npm run dev", ...) with asyncio.create_subprocess_exec("npm", "run", "dev", ...) in start_vue_dev_server.
Add a security-focused test that asserts start_vue_dev_server does not use create_subprocess_shell.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`server.py`	Switches Vue dev server startup from shell-based to exec-based subprocess creation; logs PID and schedules a background wait.
`tests/security/test_subprocess_exec.py`	Adds an AST-based test to guard against reintroducing `create_subprocess_shell` in `start_vue_dev_server`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/security/test_subprocess_exec.py

+
+def test_no_create_subprocess_shell_in_start_vue():
+    """Verify create_subprocess_shell is not used in start_vue_dev_server."""
+    content = SERVER_PY.read_text()


tests/security/test_subprocess_exec.py

+    func_content = ast.get_source_segment(content, func_node)
+    assert func_content is not None, "Could not extract source for start_vue_dev_server"
+    assert 'create_subprocess_shell' not in func_content
+    assert 'create_subprocess_exec' in func_content


server.py

    )
-    logging.info("VueJS development server is live.")
+    logging.info("Started VueJS development server with PID %s.", proc.pid)
+    asyncio.create_task(proc.wait())


…mbie subprocess

Copilot

Pull request overview

This PR hardens how the Vue dev server is launched by removing shell-based subprocess invocation and adds a regression test to prevent reintroducing shell execution in start_vue_dev_server.

Changes:

Replace asyncio.create_subprocess_shell("npm run dev", ...) with asyncio.create_subprocess_exec("npm", "run", "dev", ...) in server.py.
Add an AST-based security test to ensure start_vue_dev_server does not contain create_subprocess_shell and does contain create_subprocess_exec.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
tests/security/test_subprocess_exec.py	Adds a regression test intended to prevent shell-based subprocess usage in `start_vue_dev_server`.
server.py	Switches Vue dev server startup from shell invocation to exec-style invocation and logs PID.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/security/test_subprocess_exec.py

+    func_content = ast.get_source_segment(content, func_node)
+    assert func_content is not None, "Could not extract source for start_vue_dev_server"
+    assert 'create_subprocess_shell' not in func_content
+    assert 'create_subprocess_exec' in func_content


tests/security/test_subprocess_exec.py

+    tree = ast.parse(content, filename=str(SERVER_PY))
+    func_node = None
+    for node in ast.walk(tree):
+        if isinstance(node, ast.AsyncFunctionDef) and node.name == 'start_vue_dev_server':


… check

Copilot

Pull request overview

Updates how the Caldera server starts the Vue dev server to avoid shell invocation, and adds a regression test to enforce the safer subprocess API usage.

Changes:

Replace asyncio.create_subprocess_shell("npm run dev", ...) with asyncio.create_subprocess_exec("npm", "run", "dev", ...) in start_vue_dev_server.
Improve the startup log message to include the spawned process PID.
Add a security regression test that inspects server.py to ensure create_subprocess_shell is not used for starting the Vue dev server.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`server.py`	Switches Vue dev server startup to `create_subprocess_exec` and logs PID.
`tests/security/test_subprocess_exec.py`	Adds a test to prevent reintroducing `create_subprocess_shell` in `start_vue_dev_server`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

server.py

+    proc = await asyncio.create_subprocess_exec(
+        "npm", "run", "dev", stdout=sys.stdout, stderr=sys.stderr, cwd=MAGMA_PATH
    )
-    logging.info("VueJS development server is live.")
+    logging.info("VueJS development server started (PID %s).", proc.pid)


tests/security/test_subprocess_exec.py

+    assert 'create_subprocess_shell' not in func_content
+    assert 'create_subprocess_exec' in func_content


tests/security/test_subprocess_exec.py

+
+def test_no_create_subprocess_shell_in_start_vue():
+    """Verify create_subprocess_shell is not used in start_vue_dev_server."""
+    content = SERVER_PY.read_text()


sonarqubecloud · 2026-03-16T01:14:43Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot

Pull request overview

Updates the Vue dev-server startup path to avoid shell-based subprocess execution and adds a security regression test to prevent reintroducing create_subprocess_shell in start_vue_dev_server.

Changes:

Replace asyncio.create_subprocess_shell("npm run dev", ...) with asyncio.create_subprocess_exec("npm", "run", "dev", ...) in server.py.
Add a security test that inspects server.py to ensure start_vue_dev_server does not use create_subprocess_shell.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tests/security/test_subprocess_exec.py	Adds an AST-based regression test intended to enforce non-shell subprocess usage in `start_vue_dev_server`.
server.py	Switches Vue dev server startup from shell invocation to exec-style subprocess creation and logs the spawned PID.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/security/test_subprocess_exec.py

+    func_content = ast.get_source_segment(content, func_node)
+    assert func_content is not None, "Could not extract source for start_vue_dev_server"
+    assert 'create_subprocess_shell' not in func_content
+    assert 'create_subprocess_exec' in func_content


* fix: replace create_subprocess_shell with safe exec in start_vue_dev_server Avoid shell injection risk by using create_subprocess_exec instead. * fix: address Copilot review feedback on subprocess PR - Capture proc from create_subprocess_exec, log PID, schedule proc.wait() to avoid zombie processes on Vue dev server exit - Rewrite test to use pytest style, ast-based function extraction, and Path-relative server.py resolution Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: guard against None return from ast.get_source_segment ast.get_source_segment() returns None when source offsets are unavailable; assert it is not None before using it in string checks. * fix: remove untracked create_task in start_vue_dev_server to avoid zombie subprocess * test: accept FunctionDef and AsyncFunctionDef in start_vue_dev_server check * test: use explicit utf-8 encoding in read_text() --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

deacon-mp requested a review from Copilot March 15, 2026 18:51

Copilot started reviewing on behalf of deacon-mp March 15, 2026 18:51 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

deacon-mp requested a review from Copilot March 15, 2026 19:58

Copilot started reviewing on behalf of deacon-mp March 15, 2026 20:08 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

deacon-mp force-pushed the fix/subprocess-shell-to-exec branch 2 times, most recently from fabf4b9 to 647c11a Compare March 15, 2026 23:19

deacon-mp requested a review from Copilot March 15, 2026 23:45

Copilot started reviewing on behalf of deacon-mp March 15, 2026 23:46 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

tests/security/test_subprocess_exec.py

Comment on lines +18 to +20

func_content = ast.get_source_segment(content, func_node)

assert 'create_subprocess_shell' not in func_content

assert 'create_subprocess_exec' in func_content

deacon-mp and others added 3 commits March 15, 2026 19:51

fix: replace create_subprocess_shell with safe exec in start_vue_dev_…

3138ba0

…server Avoid shell injection risk by using create_subprocess_exec instead.

test: guard against None return from ast.get_source_segment

5daf105

ast.get_source_segment() returns None when source offsets are unavailable; assert it is not None before using it in string checks.

deacon-mp requested a review from Copilot March 15, 2026 23:52

deacon-mp force-pushed the fix/subprocess-shell-to-exec branch from 647c11a to 5daf105 Compare March 15, 2026 23:52

Copilot started reviewing on behalf of deacon-mp March 15, 2026 23:53 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

fix: remove untracked create_task in start_vue_dev_server to avoid zo…

fd1cb51

…mbie subprocess

deacon-mp requested a review from Copilot March 16, 2026 00:06

Copilot started reviewing on behalf of deacon-mp March 16, 2026 00:07 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

test: accept FunctionDef and AsyncFunctionDef in start_vue_dev_server…

9d0082e

… check

deacon-mp requested a review from Copilot March 16, 2026 00:43

Copilot started reviewing on behalf of deacon-mp March 16, 2026 00:46 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

test: use explicit utf-8 encoding in read_text()

d24d100

deacon-mp requested a review from Copilot March 16, 2026 01:14

Copilot started reviewing on behalf of deacon-mp March 16, 2026 01:16 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

deacon-mp merged commit d748c6b into master Mar 16, 2026
19 checks passed

deacon-mp deleted the fix/subprocess-shell-to-exec branch March 16, 2026 01:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: replace create_subprocess_shell #3265

fix: replace create_subprocess_shell #3265
deacon-mp merged 6 commits intomasterfrom
fix/subprocess-shell-to-exec

deacon-mp commented Mar 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

sonarqubecloud bot commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		assert 'create_subprocess_shell' not in func_content
		assert 'create_subprocess_exec' in func_content

Conversation

deacon-mp commented Mar 15, 2026

1 passed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

sonarqubecloud bot commented Mar 16, 2026

Quality Gate passed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants