Fail-open shell deny rules, lossy streaming tool-call loop, and unbounded dict tool output in praisonaiagents

[MervinPraison]

Summary

An in-depth review of src/praisonai-agents/praisonaiagents found three gaps, each reproduced locally against the current main. They are ranked by impact. Docs, tests, coverage and file size are out of scope.

1. Permission DENY rules for shell commands are bypassed by trivial wrappers (security, fail-open).
2. The streaming tool loop silently drops chained tool calls and mis-assembles parallel calls (correctness).
3. Tool-output truncation for dict results never bounds the total size, so a single tool result can overflow the context (robustness/performance).

---

1. Shell DENY rules are bypassed by wrappers and path prefixes

Where: permissions/manager.py (PermissionManager.check, ~L322, and command decomposition ~L488-591) and permissions/command_parser.py.

Only the PowerShell/cmd wrappers are unwrapped. POSIX wrappers are not. has_unresolvable_expansion already escalates $VAR / ${IFS} to ASK, but the same fail-open class remains for the forms below. Unparseable input also returns None and falls through to the broad ALLOW.

Repro (rules bash:* ALLOW at priority 0 and bash:rm * DENY at priority 10):

import tempfile
from praisonaiagents.permissions.manager import PermissionManager
from praisonaiagents.permissions import PermissionRule, PermissionAction as A

pm = PermissionManager(storage_dir=tempfile.mkdtemp())
pm.add_rule(PermissionRule(pattern="bash:*",    action=A.ALLOW, priority=0))
pm.add_rule(PermissionRule(pattern="bash:rm *", action=A.DENY,  priority=10))

for c in ["rm -rf x", "ls; rm -rf x", "/bin/rm -rf x", "sudo rm -rf x",
          "env rm -rf x", "bash -c 'rm -rf x'", "timeout 5 rm -rf x",
          "xargs rm x", "find . -delete", "echo 'x; rm -rf x"]:
    print(f"{c!r:30}", pm.check("bash:" + c).action)

Observed output:

'rm -rf x'                     DENY      <- correct
'ls; rm -rf x'                 DENY      <- correct
'/bin/rm -rf x'                ALLOW     <- bypass
'sudo rm -rf x'                ALLOW     <- bypass
'env rm -rf x'                 ALLOW     <- bypass
"bash -c 'rm -rf x'"           ALLOW     <- bypass
'timeout 5 rm -rf x'           ALLOW     <- bypass
'xargs rm x'                   ALLOW     <- bypass
'find . -delete'               ALLOW     <- bypass
"echo 'x; rm -rf x"            ALLOW     <- unterminated quote: parse error treated as allow

An agent (or a prompt-injected agent) that has been denied rm can run it by prefixing sudo, env, /bin/, bash -c, and so on. Further untested forms reported by the reviewer: nohup, nice, command, exec, busybox, eval, { ...; }, if ...; then ...; fi, and command-position $(...) / backticks.

Suggested fix (keep it in the existing command parser; no new API surface):

# permissions/command_parser.py
_PREFIX_WRAPPERS = {"sudo", "env", "command", "nohup", "nice", "time", "timeout",
                    "exec", "busybox", "xargs"}
_SHELLS = {"sh", "bash", "zsh", "dash"}

def normalize_segment(argv: list[str]) -> list[list[str]]:
    """Return every effective command inside one parsed segment."""
    out = []
    while argv:
        exe = os.path.basename(argv[0])          # '/bin/rm' -> 'rm'
        if exe in _PREFIX_WRAPPERS:
            argv = _skip_wrapper_args(exe, argv[1:])   # drop flags / 'timeout 5' / VAR=val
            continue
        if exe in _SHELLS and "-c" in argv:
            i = argv.index("-c")
            out += decompose(argv[i + 1])        # recurse into the quoted script
            return out
        argv = [exe] + argv[1:]
        break
    if argv:
        out.append(argv)
    return out

And in PermissionManager.check: treat a shlex parse failure, and $(...) / backticks in command position, as ASK (the same treatment has_unresolvable_expansion already gives $VAR) instead of returning None.

---

2. Streaming tool loop drops chained tool calls and mis-assembles parallel calls

Where: llm/llm.py, get_response_stream (~L5028-5045) and _process_tool_calls_from_stream (~L7586-7604). The accumulator is shared by the sync stream path and the async stream path.

2a. Follow-up turn is a single non-streaming call whose tool calls are discarded

# llm/llm.py ~L5028
follow_up_response = self._completion_with_retry(
    **self._build_completion_params(messages=messages, tools=formatted_tools,
                                    temperature=temperature, stream=False, **kwargs))
if follow_up_response and follow_up_response.choices:
    follow_up_content = follow_up_response.choices[0].message.content
    if follow_up_content:
        yield follow_up_content        # tool_calls on this message are never looked at

The follow-up still advertises tools=formatted_tools. If the model answers it with another tool_calls (any multi-step task), content is None, the second tool is never executed, nothing more is yielded, and the user gets no final answer. The sync and async non-streaming loops iterate until the model stops calling tools; the stream loop does not.

Reviewer's mock-based repro (stream chunk requesting step1, then a follow-up requesting step2):

yielded: ['Let me check. ']
tools executed: ['step1']
completion calls (stream flags): [True, False]     # step2 never ran

Also, lines ~4664-5286 contain no token-usage tracking, so streamed turns are absent from cost/usage accounting.

2b. Accumulator corrupts parallel calls and can raise IndexError

# llm/llm.py ~L7586
for tc in delta.tool_calls:
    if tc.index >= len(tool_calls):
        tool_calls.append({"id": tc.id, "type": "function",
                           "function": {"name": "", "arguments": ""}})
    if tc.function.name:
        tool_calls[tc.index]["function"]["name"] = tc.function.name        # overwrites
    if tc.function.arguments:
        tool_calls[tc.index]["function"]["arguments"] += tc.function.arguments

Repro I ran:

from types import SimpleNamespace as N
from praisonaiagents.llm.llm import LLM
l = LLM.__new__(LLM)
d = lambda i, id, n, a: N(tool_calls=[N(index=i, id=id, function=N(name=n, arguments=a))])

tc = []
for x in [d(0, "a", "get_time", '{"c":"P"}'), d(0, "b", "get_time", '{"tz":"U"}')]:
    l._process_tool_calls_from_stream(x, tc)
print(tc)
# [{'id': 'a', ..., 'arguments': '{"c":"P"}{"tz":"U"}'}]   <- two calls merged, invalid JSON

l._process_tool_calls_from_stream(d(1, "z", "f", "{}"), [])
# IndexError: list index out of range

Caveat: this is simulated chunks, not a captured provider stream. Providers that reuse index=0 for distinct parallel calls are the ones affected. The IndexError case is deterministic whenever the first-seen index is greater than the current list length. The id is also never updated if it arrives after the first chunk.

Suggested fix:

def _process_tool_calls_from_stream(self, delta, tool_calls):
    for tc in getattr(delta, "tool_calls", None) or []:
        # key by index, but start a new slot when a *different* non-empty id appears at a used index
        idx = tc.index
        while len(tool_calls) <= idx:
            tool_calls.append({"id": None, "type": "function",
                               "function": {"name": "", "arguments": ""}})
        slot = tool_calls[idx]
        if tc.id and slot["id"] and tc.id != slot["id"]:
            tool_calls.append({"id": tc.id, "type": "function",
                               "function": {"name": "", "arguments": ""}})
            slot = tool_calls[-1]
        if tc.id and not slot["id"]:
            slot["id"] = tc.id
        if tc.function and tc.function.name:
            slot["function"]["name"] = tc.function.name
        if tc.function and tc.function.arguments:
            slot["function"]["arguments"] += tc.function.arguments
    return tool_calls

For 2a, replace the single stream=False follow-up with a loop: stream the follow-up, accumulate with the fixed accumulator, execute any tool calls, and repeat until the model returns plain content. Reuse the same tool-execution block as the first turn, and call the existing _track_token_usage / _extract_token_usage helpers used by the non-streaming loops.

---

3. Dict tool results are never bounded in total size

Where: agent/tool_execution.py (~L1695-1706, post-truncation block) and _truncate_dict_fields (~L2129-2172).

When a dict result is over the limit, the combined truncated string is discarded and _truncate_dict_fields is called instead. That helper only shortens an individual string field when that field is longer than max_field_chars (the same 16,000-char limit). Many fields just under the limit pass through untouched, so the total is unbounded.

Repro:

from praisonaiagents.agent.tool_execution import ToolExecutionMixin as T

class X(T):
    context_manager = None
    tool_output_limit = 16000

res = {"results": [{"content": "a" * 15990} for _ in range(50)]}
out = X()._truncate_dict_fields(res, "search", 16000)
print(len(str(res)), len(str(out)))
# 800363 800363    <- "truncated" output is identical, ~200k tokens against a 16k-char limit

A search/crawl tool returning many medium-sized snippets therefore bypasses the limit and overflows the context window. It also means the _artifact_ref spill path (#2792) is only used when a single field is oversized.

Suggested fix: after the per-field pass, enforce a total budget. If len(str(result)) > limit, spill the full serialized result to the tool-output store and return the already-built truncated head/tail string, together with the artifact reference, rather than the barely-changed dict.

if isinstance(result, dict):
    result = self._truncate_dict_fields(result, function_name, max_field_chars, tool_call_id)
    if len(str(result)) > limit:                    # NEW: total-size guard
        result = truncated                          # head/tail string already computed above
        # artifact_ref (full payload) is already attached below
    elif artifact_ref:
        result["_artifact_ref"] = artifact_ref.to_dict()

---

Notes for triage

• Scope check against the project principles: none of these add API surface. Github actions fix #1 and Main #3 tighten existing behaviour, and Merge pull request #1 from MervinPraison/develop #2 makes the stream path match the sync/async loops.
• Lower-severity items found during the review but not included above, for follow-up if wanted: RateLimiter.acquire_tokens() spins forever when the request exceeds the bucket cap (llm/rate_limiter.py; currently has no in-tree caller), DefaultSessionStore._cache has no eviction (session/store.py:652), urllib.request.urlopen without a timeout in agent/context_agent.py:1780 and :2353, and _parse_retry_delay ignoring units (llm/llm.py:849-882).

🤖 Generated with Claude Code

https://claude.ai/code/session_015Eh9NTQ8MAwxDxpsdSkHig

[praisonai-triage-agent]

Claude finished @MervinPraison's task —— View job • claude/issue-5399-20261001-0720 • [Create PR ➔](https://github.com/MervinPraison/PraisonAI/compare/main...claude/issue-5399-20261001-0720?quick_pull=1&title=Issue%20%235399%3A%20Changes%20from%20Claude&body=This%20PR%20addresses%20issue%20%235399%0A%0AGenerated%20with%20%5BClaude%20Code%5D(https%3A%2F%2Fclaude.ai%2Fcode)

---

Issue #5399 — Fixed all three gaps

• Read AGENTS.md (all three fixes are in core SDK, tighten existing behaviour, no new API surface)
• Fix Github actions fix #1: Shell DENY bypass — permissions/command_parser.py now resolves POSIX wrappers (sudo, env, command, nohup, nice, timeout, exec, busybox, xargs, doas, …), standard-bindir path prefixes (/bin/rm), and sh/bash -c '<script>' to the effective inner command so a specific deny fires. manager.py escalates unparseable commands (unbalanced quotes) to ASK.
• Fix Merge pull request #1 from MervinPraison/develop #2: Streaming tool loop — hardened _process_tool_calls_from_stream (sparse index → no more IndexError; parallel calls reusing index=0 no longer merged; late id/name filled in) and replaced the single follow-up with a loop that executes chained tool calls until the model returns plain content.
• Fix Main #3: Unbounded dict output — agent/tool_execution.py now enforces a total-size budget after the per-field pass, falling back to the bounded head/tail string (with artifact ref).
• Verified each issue repro locally
• Tests: 256 permission + 43 llm-stream + 1023 (permissions/llm/streaming/agent) passing
• Commit, push, PR created

Repro results after fix

Case	Before	After
/bin/rm, sudo rm, env rm, bash -c 'rm', timeout 5 rm, xargs rm, nohup/nice/command/exec/busybox rm, sudo -u root rm	ALLOW (bypass)	DENY
echo 'x; rm -rf x (unbalanced quote)	ALLOW	ASK
parallel calls reusing index=0	args merged (invalid JSON)	two distinct calls
first-seen index=1	IndexError	handled
chained follow-up tool (step2)	dropped, no answer	executed, final answer yielded
dict of 50×~16k fields (16k limit)	~800k chars	~16k chars

PR: #5400

Note: a few unrelated tests fail in this environment due to missing optional deps (fastapi) and offline model/embedding downloads (context/compaction/search suites); none touch the modified modules.

--- • Branch: claude/issue-5399-20261001-0720