feat: add additional ESP-IDF tools - #44
Conversation
New MCP tools added: - clean_esp_project: Clean build artifacts (clean/fullclean) - get_esp_project_size: Analyze firmware RAM/Flash usage - get_esp_component_size: Per-component size breakdown - erase_esp_flash: Erase device flash memory - monitor_esp_device: Capture serial output with timeout - get_esp_app_info: Get project description and app info Updated README with tool reference table.
📝 WalkthroughWalkthroughIntroduces nine new MCP tools for ESP-IDF operations including cleaning build artifacts, analyzing firmware size, erasing flash memory, monitoring serial output, and retrieving project information. Updates README documentation with tool descriptions and usage examples. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@main.py`:
- Around line 215-245: In monitor_esp_device, validate and normalize
timeout_seconds before building the shell command: convert to int, ensure it's
at least 1 (or set to default 30) and then cap to 120 to prevent negative/zero
values from being used in the shell timeout; update the timeout_seconds
normalization logic near where timeout_seconds is set and before monitor_cmd is
constructed (function monitor_esp_device). Also sanitize or safely quote the
port value passed into monitor_cmd (the port parameter used in monitor_cmd) to
prevent shell injection—either validate against an allowlist/regex for expected
port names or apply proper shell quoting before interpolating into the command;
keep run_command_async usage unchanged. Ensure any log/write to mcp-monitor.log
still uses the validated values.
- Around line 186-212: The erase_esp_flash function currently interpolates the
port directly into a shell string (erase_cmd) causing command injection; update
erase_esp_flash (and similarly flash_esp_project) to validate/sanitize the port
and avoid using a shell string: either build the command as a list and call
run_command_async without shell=True or safely quote the port using shlex.quote
before insertion; add a small validator helper (e.g., is_valid_serial_port or
sanitize_port) to allow only expected characters/patterns (e.g., /^\/dev\/\w+$/
or COM port patterns) and reject/raise on invalid input; also change the
annotation from port: str = None to port: str | None = None (or Optional[str])
to match PEP 484.
🧹 Nitpick comments (2)
main.py (2)
248-268: Stderr suppression may hide useful diagnostic information.The command uses
2>/dev/nullfor bothreconfigureandcat, which suppresses all error output. This means if something goes wrong (e.g., permission errors, corrupted JSON), the user only sees "Project not built yet" regardless of the actual cause.Consider capturing stderr for logging purposes while still providing a user-friendly fallback message.
💡 Alternative approach
# Get project description from build returncode, stdout, stderr = await run_command_async( - f"bash -c 'source {export_script} && idf.py reconfigure 2>/dev/null; cat build/project_description.json 2>/dev/null || echo \"Project not built yet\"'" + f"bash -c 'source {export_script} && idf.py reconfigure && cat build/project_description.json || echo \"Project not built yet. Error: check mcp-app-info.log\"'" )
129-129: Note:os.chdir()is process-global state (pre-existing pattern).All tool functions use
os.chdir(project_path)which changes the working directory for the entire process. In an async context with concurrent tool invocations, this could cause race conditions where one tool'sos.chdir()affects another's execution.This is a pre-existing pattern in the codebase (seen in
build_esp_related_project,flash_esp_project, etc.), so not blocking this PR. However, for future robustness, consider usingsubprocess.run(..., cwd=project_path)or similar approaches that don't mutate global state.
| @mcp.tool() | ||
| async def erase_esp_flash(project_path: str, port: str = None) -> (str, str): | ||
| """Erase the entire flash memory of a connected ESP device. | ||
|
|
||
| WARNING: This will erase all data on the device including firmware, | ||
| NVS storage, and any other flash contents. | ||
|
|
||
| Args: | ||
| project_path: Path to any ESP-IDF project (used for idf.py context) | ||
| port: Serial port for the ESP device (optional, auto-detect if not provided) | ||
|
|
||
| Returns: | ||
| tuple: (stdout, stderr) - Erase operation logs | ||
| """ | ||
| os.chdir(project_path) | ||
| export_script = get_export_script() | ||
|
|
||
| if port: | ||
| erase_cmd = f"bash -c 'source {export_script} && idf.py -p {port} erase-flash'" | ||
| else: | ||
| erase_cmd = f"bash -c 'source {export_script} && idf.py erase-flash'" | ||
|
|
||
| returncode, stdout, stderr = await run_command_async(erase_cmd) | ||
|
|
||
| open('mcp-erase.log', 'w+').write(str((stdout, stderr))) | ||
| logging.warning(f"erase-flash result {stdout} {stderr}") | ||
| return stdout, stderr |
There was a problem hiding this comment.
Command injection risk via unsanitized port parameter.
The port parameter is directly interpolated into the shell command without validation. A malicious or malformed port value like "/dev/tty; rm -rf /" could execute arbitrary shell commands.
This issue also exists in the pre-existing flash_esp_project function (line 87), but new code should not propagate the vulnerability.
🛡️ Proposed fix: validate port parameter
Add validation at the start of the function (and consider extracting to a helper for reuse):
+import re
+
+def _validate_port(port: str | None) -> None:
+ """Validate serial port to prevent command injection."""
+ if port is not None and not re.match(r'^[/\w.-]+$', port):
+ raise ValueError(f"Invalid port format: {port}")
+
`@mcp.tool`()
-async def erase_esp_flash(project_path: str, port: str = None) -> (str, str):
+async def erase_esp_flash(project_path: str, port: str | None = None) -> (str, str):
"""Erase the entire flash memory of a connected ESP device.
...
"""
+ _validate_port(port)
os.chdir(project_path)Additionally, static analysis correctly notes that port: str = None should be port: str | None = None per PEP 484.
🧰 Tools
🪛 Ruff (0.14.14)
[warning] 187-187: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
[warning] 208-208: Unpacked variable returncode is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🤖 Prompt for AI Agents
In `@main.py` around lines 186 - 212, The erase_esp_flash function currently
interpolates the port directly into a shell string (erase_cmd) causing command
injection; update erase_esp_flash (and similarly flash_esp_project) to
validate/sanitize the port and avoid using a shell string: either build the
command as a list and call run_command_async without shell=True or safely quote
the port using shlex.quote before insertion; add a small validator helper (e.g.,
is_valid_serial_port or sanitize_port) to allow only expected
characters/patterns (e.g., /^\/dev\/\w+$/ or COM port patterns) and reject/raise
on invalid input; also change the annotation from port: str = None to port: str
| None = None (or Optional[str]) to match PEP 484.
| @mcp.tool() | ||
| async def monitor_esp_device(project_path: str, port: str = None, timeout_seconds: int = 30) -> (str, str): | ||
| """Monitor serial output from a connected ESP device. | ||
|
|
||
| Note: This captures output for a limited time since MCP tools can't run indefinitely. | ||
| For interactive monitoring, use idf.py monitor directly in a terminal. | ||
|
|
||
| Args: | ||
| project_path: Path to the ESP-IDF project | ||
| port: Serial port for the ESP device (optional, auto-detect if not provided) | ||
| timeout_seconds: How long to capture output (default: 30 seconds, max: 120) | ||
|
|
||
| Returns: | ||
| tuple: (stdout, stderr) - Captured serial output | ||
| """ | ||
| os.chdir(project_path) | ||
| export_script = get_export_script() | ||
|
|
||
| # Cap timeout to prevent runaway processes | ||
| timeout_seconds = min(timeout_seconds, 120) | ||
|
|
||
| if port: | ||
| monitor_cmd = f"bash -c 'source {export_script} && timeout {timeout_seconds} idf.py -p {port} monitor || true'" | ||
| else: | ||
| monitor_cmd = f"bash -c 'source {export_script} && timeout {timeout_seconds} idf.py monitor || true'" | ||
|
|
||
| returncode, stdout, stderr = await run_command_async(monitor_cmd) | ||
|
|
||
| open('mcp-monitor.log', 'w+').write(str((stdout, stderr))) | ||
| logging.warning(f"monitor result (captured {timeout_seconds}s) {stdout} {stderr}") | ||
| return stdout, stderr |
There was a problem hiding this comment.
Negative timeout_seconds values are not validated.
The code caps the maximum at 120, but negative or zero values would pass through. A negative timeout could cause timeout -5 idf.py monitor to behave unexpectedly (timeout command typically treats negative values as errors).
🛡️ Proposed fix
# Cap timeout to prevent runaway processes
- timeout_seconds = min(timeout_seconds, 120)
+ timeout_seconds = max(1, min(timeout_seconds, 120))Also note: the same command injection risk via port applies here as flagged for erase_esp_flash.
🧰 Tools
🪛 Ruff (0.14.14)
[warning] 216-216: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
[warning] 241-241: Unpacked variable returncode is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🤖 Prompt for AI Agents
In `@main.py` around lines 215 - 245, In monitor_esp_device, validate and
normalize timeout_seconds before building the shell command: convert to int,
ensure it's at least 1 (or set to default 30) and then cap to 120 to prevent
negative/zero values from being used in the shell timeout; update the
timeout_seconds normalization logic near where timeout_seconds is set and before
monitor_cmd is constructed (function monitor_esp_device). Also sanitize or
safely quote the port value passed into monitor_cmd (the port parameter used in
monitor_cmd) to prevent shell injection—either validate against an
allowlist/regex for expected port names or apply proper shell quoting before
interpolating into the command; keep run_command_async usage unchanged. Ensure
any log/write to mcp-monitor.log still uses the validated values.
1 participant
Summary
Adds 6 new MCP tools to expand ESP-IDF workflow coverage.
New Tools
clean_esp_projectidf.py clean/fullcleanget_esp_project_sizeidf.py sizeget_esp_component_sizeidf.py size-componentserase_esp_flashidf.py erase-flashmonitor_esp_deviceidf.py monitorget_esp_app_infoproject_description.jsonDetails
cleanandfullcleanmodes via boolean flagTesting
Tested tool registration and command generation. Actual ESP-IDF execution requires IDF_PATH environment setup.
README Updates
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.