Simple implementation for limiting shell commands.

[Penguin-Guru]

Overview

Allows for limiting the shell commands available as tools.

Additional information

This is still lacking some features that might be nice to have (see below), but it works reasonably well.

Features that should probably be added:

• Support for reading whitelist from a specified directory or config file (not just C.L.I. or environment variable).

Features that could be added:

• Support for escaping blacklisted symbols.
• Mechanism to specify shell at run-time or (ideally) per tool.
• Support for wider character sets.
• Better mechanism allowing A.I. models to discern which programs are actually available.
• Support for "always allow" per command/program (when user is prompted for tool use).

Limitations:

• Programs can call other programs not in the white list. User must plan accordingly.
• Preprocessor macros are used to set the shell type (pre-existing mechanism) and blacklisted symbols.
• Obviously subject to shell misdirection attacks, like PATH poisoning and swapping executables.

Warnings:

• I have not tested this on Windows.
• There may be some obscure shell delimiters I missed.

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: I used a fine-tuned Qwen 3.6 to help with the Python tests and tracking down a discrepancy with error handling in streaming mode.

[Penguin-Guru]

There are still some features I might add but I'm going to open this up for review to get some feedback. Is this something anyone else wants?

[ServeurpersoCom]

The problem is that this looks like a security feature but cannot be one as long as the string still goes to sh -c: a newline already bypasses the segment check entirely, and whitelisting a program name does not constrain its arguments.

On Linux the only boundary that actually holds is one enforced by the kernel, so the sandbox has to be a locked down container rather than a filter, and #26062 already gives you the clean way to get there: declare an MCP stdio server whose command runs inside that container, and llama.cpp then only ever passes structured JSON params to a process whose argv you fixed yourself, so no model controlled string ever reaches a shell outside the container. Inside it, a tool is free to call sh -c on model input, because the container is the boundary and the parsing no longer buys you anything.

[Penguin-Guru]

This was a stupid oversight on my part. That bug is fixed now. Thanks, @ServeurpersoCom! 🙂

I agree that MCP over stdio is generally better in a lot of ways, but I think this is also useful. With that said, I wrote this before MCP over stdio was added and I haven't actually tried MCP. I will some day soon.

I tried to highlight the risks in the --help output so users don't think this is fool-proof. It is fundamentally bound by the same security limitations as the shell. The advantage is that users don't have to tool up MCP and can simply use the tools they already have installed with an interface they may already be familiar with. Pros and cons.

[ServeurpersoCom]

Even if it can be handy in the moment, we won't be able to merge this. The blocker isn't the individual bugs, it's the framing: a feature that can be taken for a security option without actually being one leads to too much downstream misuse, people will rely on it as a boundary and build on that false assumption. And this is the part that makes it worse than a static bug: a model will naturally and easily start working around the filter exactly when you least expect it, not out of malice but because routing around an obstacle to complete the task is what they do, so the whitelist gets bypassed in normal operation, not just under a crafted attack. That's a line we can't cross regardless of how the implementation evolves. For the record, a non-exhaustive sample of the failure modes, this is a bottomless pit and patching each one just moves the next one into view:

• argv[0] only (POSIX): the whitelist checks the program name, never its arguments, and most of coreutils can launch another process. Whitelist any single wrapper and it's over, none of these contain a blacklisted symbol so they all pass and reach sh -c: nice id, env sh -c id, timeout 5 id, setsid id, nohup id, stdbuf -o0 id, flock /tmp/l id, xargs id, find . -exec id ;, awk 'BEGIN{system("id")}', perl -e 'exec"id"', git -c core.pager=id log.
• argv[0] only (Windows): same class, cmd /c command has no shortage of built-in launchers that carry no blacklisted char: powershell -c whoami, forfiles /p . /c "cmd /c whoami", wmic process call create "calc", start /b whoami, conhost whoami, schtasks /create /tr "..." /sc once.
• prefix match: a whitelist entry with a space does a raw prefix compare, so ls -l also allows ls -la /etc/shadow.
• the string still goes to sh -c / cmd /c: as long as a shell interprets the final command, no user-space filter in front of it is a boundary.
• windows path is also unblacklisted for ^ (cmd escape) and untested by your own note.

[Penguin-Guru]

@ServeurpersoCom

If you don't want to merge it, that's ok. I agree that it's not perfect. Nothing is-- especially in security. I feel like this is reasonably safe, barring any bugs, and it is certainly a lot safer than simply using the exec_shell_command tool with no protection at all. If you don't want to merge something like this, I'm afraid you'll probably want to remove the exec_shell_command tool entirely, which I think would be a shame. It's obviously up to you all though. I can't really judge what's best without having tried MCP myself yet. It's hard for me to imagine MCP resolving any fundamental problems with my approach here though. This logic needs to be implemented somewhere if shell commands are ever to be safely supported.

With regards to your list of concerns:

1. Yes, this specifically allows for whitelisting programs. Logic that parses arguments seems better included in wrappers specific to those programs, since there's no end to what people might want. This feature allows for safe access to such wrappers. It also allows for safe access to whatever command-line programs users consider safe. There are a lot of perfectly safe command-line programs and utilities that modern A.I. models know how to use effectively.
2. Same as above.
3. There is a length comparison that protects against matching partial prefixes for program names. It does not attempt to parse arguments for precisely the reason you described, since that would require changing the interface to accept an additional array of arguments. With that said, I believe it does still allow for matching whitelisted entries that include arguments, like your example. This is useful to ensure those commands can only be run with those arguments (unless they can be purposefully over-ridden by later arguments); it is not intended to prevent running with additional arguments. This is more of a side-feature and I could remove this functionality if you want. The main point is the program whitelisting. Also, ls -l in the whitelist should not allow ls -la ... because there must be a shell_delim symbol immediately after the whitelisted term. That doesn't make a difference in your example but it does protect against whitelisted arguments being evaluated as prefixes of longer arguments.
4. The string does not go to the shell when any one segment matches a whitelist entry. The programs in all segments must match. If you are experiencing different behaviour, could you tell me what string you used and whether you're on POSIX or Windows?
5. I'm not aware of any reason to blacklist '^' on Windows. My understanding is that it only escapes the next symbol, like a backslash, but it has been a long time since I last used Windows. I did google around to assemble the current lists. If they are inadequate, hopefully further review and testing will reveal that. I can only test so much myself. Am I mistaken about what '^' does in CMD?

[ServeurpersoCom]

Quick answer to your direct question: POSIX, single segment "nice id". id isn't a second segment to whitelist, it's an argument to nice, and nice execs it. Same for env sh -c id, timeout 5 id, setsid id, xargs id, find . -exec id ;. argv[0] is whitelisted, the argument is the payload. That's the whole point: whitelisting the program name can't constrain a program whose job is to run another one.

On ls -l vs ls -la: it does match. The code memcmp's s.length() bytes and never checks the following char, so ls -l accepts ls -la /etc/shadow. There's no "delim must follow" in the logic.

You're right about ^, drop that one, it's not a clean bypass against an exact argv[0] match.

And this isn't about removing exec_shell_command. It's about not shipping a filter under a security label it can't hold. The tool as raw is honestly unsafe and says so; a filter that looks safe but isn't is worse, because people build on the false assumption.

[pwilkin]

Yeah, what @ServeurpersoCom said, we've discussed this already: we are explicitly marking the tool as unsafe to let the people know it is such. What we really don't want is shipping an imperfect guard as something that "hardens" the solution, because it's the worst of both worlds: it doesn't really secure it against attacks (since an attacker will try every vector), while it gives uses a false sense of security.

[ngxson]

My POV is: if you can show us how other open-source harness do this, then let's investigate to see how they did better.

Otherwise, my honest take: from security perspective, you cannot do much with whitelisting commands, as you pointed out:

Programs can call other programs not in the white list. User must plan accordingly.

A lot of commands can run other commands, so mostly of time user will end up with a too restricted set of whilelist that turns the exec_shell tool to useless. The LLM may also waste token to find the useful commands that are non-blocked, or try to abuse whitelisted commands to escape. Sounds familiar?

From system perspective, it's better to just do a proper sandbox if you really care about security. I already planned this when I reworked the tools, and already had a draft locally that run tools directly inside a docker container. I'll need a bit of time to test it, but will push it soon.

[ServeurpersoCom]

We all agree.

For most setups, running llama.cpp itself inside a container makes shell exec safe by default: the boundary is already there and the tool inherits it for free, no filter needed. On macOS, App Store apps are likewise sandboxed into per-app containers by default, same idea at the OS level. Either way the kernel does the work, and we're not going to reimplement rbash inside llama.cpp lol.

And restricting a model to a command subset tends to hurt its performance quite a bit anyway. When you actually want to narrow what it can do, the better path is to declare a tool list rather than gate a shell: a plain bash or powershell script is enough (there's a minimal example in the stdio PR). That's a whitelist instead of a blacklist, and any security mistake lives in a user script, not in the project's C++.

[Penguin-Guru]

@ServeurpersoCom

Quick answer to your direct question: POSIX, single segment "nice id". id isn't a second segment to whitelist, it's an argument to nice, and nice execs it. Same for env sh -c id, timeout 5 id, setsid id, xargs id, find . -exec id ;. argv[0] is whitelisted, the argument is the payload. That's the whole point: whitelisting the program name can't constrain a program whose job is to run another one.

Oh, yes. I thought you were saying something else with that point.

On ls -l vs ls -la: it does match. The code memcmp's s.length() bytes and never checks the following char, so ls -l accepts ls -la /etc/shadow. There's no "delim must follow" in the logic.

You're right, I mis-read it when I replied. I must have dropped that logic in my iterations on the last commit. I'll push a fix in a couple hours.

You're right about ^, drop that one, it's not a clean bypass against an exact argv[0] match.

👍

And this isn't about removing exec_shell_command. It's about not shipping a filter under a security label it can't hold. The tool as raw is honestly unsafe and says so; a filter that looks safe but isn't is worse, because people build on the false assumption.

I agree with the principle but it seems a bit excessive in this case. I guess the issue is that we have no way of signalling the danger effectively enough to manage the risk. Users who want this (in my opinion critical) functionality will have to add it themself or build some other fork. That's a major nuisance but I guess it's an effective way of making sure users know what they are doing.

---

@pwilkin

Yeah, what ServeurpersoCom said, we've discussed this already: we are explicitly marking the tool as unsafe to let the people know it is such. What we really don't want is shipping an imperfect guard as something that "hardens" the solution, because it's the worst of both worlds: it doesn't really secure it against attacks (since an attacker will try every vector), while it gives uses a false sense of security.

I absolutely agree if the hardening is unreliable but I feel like there's a point where it is safe enough and the security concerns are detached enough from the project to be unrelated. I understand your decision though. I mostly made this for myself anyway. 👍

---

@ngxson

My POV is: if you can show us how other open-source harness do this, then let's investigate to see how they did better.

If you're open to merging this if it's reliable, I can look into that a bit. It seems like the consensus is that this is not something you want though. If you all change your mind, just let me know and I will do what I can.

A lot of commands can run other commands, so mostly of time user will end up with a too restricted set of whilelist that turns the exec_shell tool to useless. The LLM may also waste token to find the useful commands that are non-blocked, or try to abuse whitelisted commands to escape. Sounds familiar?

So far, I've had an easy time using this now that pipes are supported and the LLM can query the programs it has access to. It's not perfect but I've had no real issues since those features were added. Of course, I'm sure it depends on the model and the prompt.

From system perspective, it's better to just do a proper sandbox if you really care about security. I already planned this when I reworked the tools, and already had a draft locally that run tools directly inside a docker container. I'll need a bit of time to test it, but will push it soon.

Ok. Personally, I don't want to containerise programs on my desktop or personal server but I would likely use some sort of container on a production server. They feel like different use cases to me, but I know a lot of people do prefer containers in general.

---

We all agree.

For most setups, running llama.cpp itself inside a container makes shell exec safe by default: the boundary is already there and the tool inherits it for free, no filter needed. On macOS, App Store apps are likewise sandboxed into per-app containers by default, same idea at the OS level. Either way the kernel does the work, and we're not going to reimplement rbash inside llama.cpp lol.

And restricting a model to a command subset tends to hurt its performance quite a bit anyway. When you actually want to narrow what it can do, the better path is to declare a tool list rather than gate a shell: a plain bash or powershell script is enough (there's a minimal example in the stdio PR). That's a whitelist instead of a blacklist, and any security mistake lives in a user script, not in the project's C++.

Ok, I will close the P.R. for now. Let me know if you all change your mind. I'm happy to do what I can to make it safe and useful for others.

[ngxson]

Ok. Personally, I don't want to containerise programs on my desktop or personal server but I would likely use some sort of container on a production server.

take my advice here as a grant of salt, but this seems to be a false sense of security. you may want to containerize when it runs on personal env because it would be super bad if the agent accidentally mess up with your home dir (or worse, rm -rf it ; and this is exactly why whitelisting command is bad: if the model try to rm something when it's not allowed, it may try to abuse other commands, like find to do so, and one wrong move can be dangerous)

a production env should already run inside a container anyway. at worse, it deletes the database or the mounted data dir, not nuking the host's home dir or root dir

[Penguin-Guru]

@ngxson

Thank you. I do understand the risk, but I'm comfortable enough with this for now. I use system permissions for access control and none of the commands I have whitelisted can modify files or system state. They're mostly coreutils, so pretty unlikely to have intrinsic vulnerabilities or updates that introduce unexpected features. I can also use sandboxing. My aversion to containers specifically is just that they add a lot of complexity when really all I would want is sandboxing. The main benefit I see in containers is orchestration for scaling. Hopefully that makes sense.

If my system does get wiped somehow, I do also keep backups. It would be an inconvenience but I'm comfortable taking that risk. 🙂