🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

1. 🔵 Unscoped parallel_tool_calls request-body injection into all provider payloads may break strict APIs and trigger retry-loop DoS

Description

applyExtraParamsToAgent() installs a stream wrapper that unconditionally mutates the outgoing request payload object by adding a top-level parallel_tool_calls field whenever it is configured in model params.

Key issues:

• Provider/API not gated: the wrapper runs regardless of model.api/provider (Anthropic, Google, Bedrock, Ollama native /api/chat, OpenAI Responses WebSocket, etc.). Many non-OpenAI APIs use strict request-body schemas and may reject unknown top-level fields with a 4xx.
• Mutation affects the real upstream request (not just logging): e.g. in openai-ws-stream.ts, options.onPayload(payload) is called before session.manager.send(payload). Since this wrapper mutates payload inside onPayload, the injected field is included in the WebSocket response.create message sent to OpenAI.
• Availability risk (DoS via retry loops / repeated failures): When a provider rejects the unknown field, the run loop can re-attempt/failover across profiles/models (up to high retry limits). A misconfigured parallel_tool_calls on an incompatible provider/model can therefore cause repeated failing requests, resource consumption, and degraded service.

Vulnerable code (payload injection):

onPayload: (payload) => {
  if (payload && typeof payload === "object") {
    (payload as Record<string, unknown>).parallel_tool_calls = parallelToolCalls;
  }
  originalOnPayload?.(payload);
}

Provider-specific compatibility risks (examples):

• google-generative-ai: request bodies are structured differently (often nested config objects); unknown top-level keys are likely invalid.
• anthropic-messages / Bedrock non-OpenAI adapters: generally strict schemas; unknown fields can cause 400s.
• ollama native /api/chat: request schema is model/messages/stream/tools/options; an extra root key may be rejected by some versions.
• OpenAI Responses WS (response.create): if parallel_tool_calls is not accepted for this event type/version, this breaks the WS transport entirely.

Recommendation

Gate injection to APIs/providers that are known to support this field, and avoid mutating payloads for strict non-OpenAI request schemas.

Suggested fix (example allowlist by model.api):

const PARALLEL_TOOL_CALLS_APIS = new Set(["openai-completions", "openai-responses"]);

onPayload: (payload) => {
  if (
    PARALLEL_TOOL_CALLS_APIS.has(model.api) &&
    payload &&
    typeof payload === "object"
  ) {
    (payload as Record<string, unknown>).parallel_tool_calls = parallelToolCalls;
  }
  originalOnPayload?.(payload);
}

Additionally:

• Consider an explicit provider allowlist (e.g., openai, openrouter, other confirmed OpenAI-compatible backends) since some openai-completions adapters are strict.
• Add tests ensuring this field is not injected for anthropic-messages, google-generative-ai, ollama, etc.
• If unsupported-field errors occur, ensure they are not retried excessively (fail fast / classify as non-retryable configuration error).

Analyzed PR: #37201 at commit 055753b

_{Last updated on: 2026-03-06T05:24:26Z}

Greptile Summary

This PR adds a createParallelToolCallsWrapper that lets users set parallel_tool_calls: false (or true) per model in their OpenClaw config, fixing a 400 error from providers like NVIDIA NIM that don't support parallel tool calls. The wrapper injects the boolean into the LLM request payload via onPayload, following the same pattern already used by createZaiToolStreamWrapper. The change is strictly opt-in and has no effect on default behavior.

• Implementation is clean and consistent with existing wrapper patterns in the file.
• The typeof merged?.parallel_tool_calls === "boolean" guard correctly prevents the wrapper from being applied when the param is absent, preserving backward compatibility.
• The test suite covers the true and false injection cases, but is missing a test for the "no config → no injection" path — the most important backward-compatibility guarantee. It would be worth adding that test case.

Confidence Score: 4/5

• This PR is safe to merge; it is a small, opt-in change with no risk to existing behavior.
• The implementation is correct, follows established patterns, and is gated behind an explicit boolean config check that preserves all current default behavior. The only gap is a missing automated test for the "no config → no injection" case, which is the primary backward-compatibility guarantee. Everything else is well-covered.
• No files require special attention beyond adding the missing backward-compatibility test in src/agents/pi-embedded-runner-extraparams.test.ts.

_{Last reviewed commit: 055753b}