Repository navigation
Provide clear guidance on avoiding security problems with template type package arguments #41
Description
Activity
I think there are (at least) two questions to consider
- Should the registry do any form of scanning/sanitizing of the input if we do choose to enable the
templatefield? - What should we document about the risks client developers face when constructing commands from the template value and through what documentation channel?
@connor4312 per the original exchange in #33, I agree about shell escaping being a client responsibility before running, but I worry more about malicious commands being included, like the example:
"template": “;rm -rf ~/Development”Are you thinking about this in a different way? Is there something I am missing in the design of the feature that makes this not a risk?
- Should the registry do any form of scanning/sanitizing of the input if we do choose to enable the
Should the registry do any form of scanning/sanitizing of the input if we do choose to enable the template field?
This is nice but not possible to do with complete accuracy -- this is the halting problem. And folks could always just have a command that gets a shell script from the web and install that way, which is a popular approach for some packages in general, though we could have some LM analysis step as part of submission to provide a rough safeguard (provided the false positive rate is acceptable.)
What should we document about the risks client developers face when constructing commands from the template value and through what documentation channel?
I'm not sure where such instructions should exist, but I think we can recommend that "Clients MUST display the command(s) use to execute an MCP server to users and obtain their consent before executing commands from new servers or new server versions."
Is there something I am missing in the design of the feature that makes this not a risk?
Notably, MCP server commands don't have to be executed in a shell. And I think spawning in a shell is actually pretty rare because clients need direct stdin/stdout access. If clients used
posix_spawnor equivalents then there is no risk of injection.To check my understanding, this is basically wanting to warn registry clients that running arbitrary commands can be dangerous, and runtime_arguments can be a vector for injecting harmful commands if not careful. So in general they should take precautions like escaping things, spawning things in a shell, and showing commands that might be run to the user.
If that's the case, is this good to move to implementation work for someone to write docs on?
@claude can you tell me where my understanding is wrong? Make sure to read this thread, plus PR #33, for context.
Reacted by Tadas AntanaviciusClaude finished @domdomegg's task —— View job
Your understanding is mostly correct with some important nuances.
Todo List
- Read PR feat: refine representation of commands in the API #33 for additional context on template arguments
- Analyze the security concerns and proposed solutions
- Clarify the user's understanding
- Provide recommendations on next steps
Summary
Your understanding is mostly correct but has one key inaccuracy about shell execution.
Where your understanding needs clarification
Correction: You mentioned "escaping things, spawning things in a shell" - but connor4312 actually emphasized that MCP servers don't have to be executed in a shell at all. The security risk is primarily when clients DO use shell execution.
Key points from the discussion:
-
The Risk: Template arguments (like
runtime_argumentswithtype: template) could contain malicious commands like;rm -rf ~/Development -
Why it's actually lower risk than it appears:
- MCP servers typically need direct stdin/stdout access
- Shell execution is "actually pretty rare" for MCP clients
- Using
posix_spawnor equivalents eliminates injection risk entirely
-
Connor's proposed mitigation strategy:
- Clients MUST display the command(s) to users
- Clients MUST obtain consent before executing commands from new servers/versions
- Use non-shell execution when possible
-
The template feature: From PR feat: refine representation of commands in the API #33, this allows complex argument templating like:
- type: template template: "--mount=type=bind,src={host_path},dst={container_path}" properties: host_path: { ... } container_path: { ... }
Ready for implementation: Yes, this appears ready for documentation work. The consensus is:
- Registry-side validation isn't practical (halting problem)
- Client-side responsibility with clear security guidance
- Focus on user consent and non-shell execution patterns
Recommended next steps: Create documentation that warns clients about:
- Command injection risks with shell execution
- Preference for non-shell execution methods
- Mandatory user consent for command execution
- Proper argument escaping when shell execution is necessary
Here:
registry/docs/server-registry-api/openapi.yaml
Lines 366 to 375 in e4002fd
- added a commit that references this issue
on Sep 8, 2025
@alexhancock brought this up #33 (comment):
@connor4312 said:
It'd be helpful to get some more thoughts on whether we have a potential security hole here, and what kind of formal guidance we can issue to clients (and/or SDK features we can implement) to mitigate any potential damage.