Skip to content

fix: strip raw OpenAI function-call wire format from user-facing text#30479

Closed
kevinWangSheng wants to merge 25 commits into
openclaw:mainfrom
kevinWangSheng:fix/30441-strip-function-call-wire-format
Closed

fix: strip raw OpenAI function-call wire format from user-facing text#30479
kevinWangSheng wants to merge 25 commits into
openclaw:mainfrom
kevinWangSheng:fix/30441-strip-function-call-wire-format

Conversation

@kevinWangSheng

Copy link
Copy Markdown
Contributor

When models like Codex fail to produce properly structured tool calls, they sometimes emit raw function-call wire format like 'assistant to=functions.xxx' mixed with hallucinated text. This change adds a sanitization function to strip these patterns from text before it's delivered to users.

This addresses issue #30441 where Codex subagent output was leaking raw function-call syntax to Discord channels.

Changes

  • Added new function in that removes raw function-call wire format patterns
  • Updated in to include the new stripping function in the sanitization chain
  • Added comprehensive tests for the new function

Test plan

  • All existing tests pass
  • New tests added for the sanitization function

Kevin Shenghui and others added 25 commits February 28, 2026 09:46
Add allowMissingTarget option to assertPathSafety for write, mkdirp,
and rename operations. This fixes the regression where creating new
files fails with 'cannot create directories' error, even when the
workspace has rw access.

The issue occurs because assertPathSafety was checking for file
existence before allowing directory creation, but for new files the
parent directory may not have the target file yet.

Fixes openclaw#28734
Add allowMissingTarget option to assertPathSafety for write, mkdirp,
and rename operations. This fixes the regression where creating new
files fails with 'cannot create directories' error, even when the
workspace has rw access.

The issue occurs because assertPathSafety was checking for file
existence before allowing directory creation, but for new files the
parent directory may not have the target file yet.

Fixes openclaw#28734
Export waitForAbortSignal from the plugin SDK so extensions can import it
from 'openclaw/plugin-sdk' instead of using a relative path that breaks
in the built project.

Fixes openclaw#30057
- Add defaultAccount field to FeishuConfigSchema
- Update resolveDefaultFeishuAccountId to check defaultAccount first
- Update resolveFeishuAccount to use configured default when accountId is not provided
- Exclude defaultAccount from base config during merge to prevent incorrect inheritance

Fixes openclaw#30045
The withRemoteHttpResponse function was not passing proxy: "env" to
fetchWithSsrFGuard, causing memory search to fail with ECONNREFUSED
on servers that require an HTTP proxy for outbound HTTPS access.

This fix ensures the Gemini embedding API call respects the system's
HTTPS_PROXY and HTTP_PROXY environment variables, consistent with
other code paths like Telegram channel and web_fetch tool.

Fixes openclaw#30075
This fix enables providers like kimi-coding that require specific headers
(e.g., User-Agent) for authentication. The kimi-coding provider already
had headers defined in its config, but they were not being passed to the
API calls.

This change adds a wrapper function that injects the provider's custom
headers into the streamFn options for every API request.

Fixes openclaw#30099
…tories

Docker COPY preserves original permissions from the host filesystem. When
building the Docker image, extensions directories may end up with 777
permissions (world-writable), which causes OpenClaw to block all plugins
with: 'blocked plugin candidate: world-writable path'

This change adds chmod commands after COPY to ensure:
- Directories are 755 (rwxr-xr-x)
- Files are 644 (rw-r--r--)

Fixes openclaw#30139
…(feishu:user:...)

Fixes openclaw#30324 - Feishu target normalization fails for provider-prefixed
targets like 'feishu:user:ou_xxx'. The normalizeFeishuTarget function
now strips the optional 'feishu:' prefix first, then applies existing
chat|user|open_id parsing. Also updates looksLikeFeishuId to accept
provider-prefixed forms.
…bility

Issue openclaw#30467: Control UI blank page on browsers without Array.prototype.toSorted

Replace toSorted() with slice().sort() to ensure compatibility with
older browsers (e.g., Windows 7 Chrome-based browsers) that don't
support the ES2023 toSorted method.
…tibility

Fixes openclaw#30467 - Control UI blank page on browsers without Array.prototype.toSorted

Changed .slice().toSorted() to .slice().sort() for cross-browser compatibility.
The toSorted() method is not available in older Chrome-based browsers (e.g.,
Windows 7), causing the Control UI to render a blank page with 'TypeError:
Array.from(...).toSorted is not a function' error.
…user responses

When 'steer' mode injects a user message into a heartbeat run, the agent
produces a combined response like 'HEARTBEAT_OK Sure! Here is the answer.'
The previous logic stripped HEARTBEAT_OK and then checked if the remaining
text was ≤ maxAckChars (300). Since many user responses are under 300 chars,
the entire response was treated as a brief heartbeat ack and suppressed.

This fix adds a heuristic: after stripping HEARTBEAT_OK, if the remaining
text contains multi-sentence structure (sentence-ending punctuation [.!?]
followed by more content), it is recognized as a substantive response and
NOT suppressed.

Fixes openclaw#30449
When logging.file is set to a tilde-prefixed path (e.g. ~/.openclaw/logs/gateway.log),
the gateway crashes on startup with 'ENOENT: no such file or directory, mkdir ~/.openclaw/logs'.

This fix expands the tilde (~) to the user's home directory before creating
the log directory and writing log files.

Fixes: Issue openclaw#30401
When models like Codex fail to produce properly structured tool calls, they
sometimes emit raw function-call wire format like 'assistant to=functions.xxx'
mixed with hallucinated text. This change adds a sanitization function to
strip these patterns from text before it's delivered to users.

This addresses issue openclaw#30441 where Codex subagent output was leaking raw
function-call syntax to Discord channels.
@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord channel: nextcloud-talk Channel integration: nextcloud-talk channel: telegram Channel integration: telegram channel: whatsapp-web Channel integration: whatsapp-web app: web-ui App: web-ui labels Mar 1, 2026
@openclaw-barnacle openclaw-barnacle Bot added docker Docker and sandbox tooling agents Agent runtime and tooling channel: feishu Channel integration: feishu size: L trusted-contributor labels Mar 1, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f731b69350

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

* Response behavior when an unpaired (unauthorized) user sends a message.
* @default "silent"
*/
unpairedResponse?: UnpairedResponseMode;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add unpairedResponse to strict config schemas

unpairedResponse is introduced on channel defaults here, but the runtime config validators are still strict and do not accept it (src/config/zod-schema.providers.ts for channels.defaults and src/config/zod-schema.providers-core.ts for Telegram account objects). In practice, users who set channels.defaults.unpairedResponse (or Telegram overrides) will get validation errors, so the new behavior cannot be configured in production.

Useful? React with 👍 / 👎.

code,
mode: getUnpairedResponseMode(cfg),
})
: "Pairing already requested. Ask the bot owner to approve your code.";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect silent mode for repeated DM pairing replies

The created === false branch hardcodes a reply string, which bypasses buildUnpairedResponse and therefore bypasses unpairedResponse: "silent". In silent mode, an unpaired user who retries a Discord component interaction after their first pairing request will still get a DM response, which leaks bot behavior in the mode intended to suppress all unpaired replies.

Useful? React with 👍 / 👎.

@greptile-apps

greptile-apps Bot commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR primarily adds stripOpenAIWireFormatText to sanitize raw OpenAI function-call wire format (assistant to=functions.xxx) from user-facing text when models like Codex fail to produce properly structured tool calls. The sanitization function is correctly wired into the existing chain in sessions-helpers.ts and is well-tested. The PR also bundles several unrelated improvements: a configurable unpairedResponse mode for unauthorized-user pairing flows, a heartbeat multi-sentence suppression fix, Feishu defaultAccount support, tilde expansion in log paths, and Dockerfile permission hardening.

Key issues found:

  • Logic bug — Discord silent mode leak (src/discord/monitor/agent-components.ts): When a user has already submitted a pairing request (created === false), the string "Pairing already requested. Ask the bot owner to approve your code." is unconditionally assigned to responseText and is therefore always sent, even when unpairedResponse is "silent". The null-guard on line 516 only protects the buildUnpairedResponse path (created === true), leaving the repeat-request branch unguarded. This leaks system state to unauthenticated users, defeating the purpose of silent mode.
  • Dockerfile chmod 644 removes executable bits (Dockerfile): Applying chmod 644 to all files in /app/extensions, /app/.agent, and /app/.agents strips the executable bit from any shell scripts or compiled binaries that may exist there. Using go-w (remove world-write only) would achieve the security goal without affecting executability.
  • Duplicate test case name (src/agents/pi-embedded-utils.test.ts): Two cases are both named "function call with trailing content", which will make failure diagnostics ambiguous.

Confidence Score: 2/5

  • Not safe to merge as-is — the silent-mode information leak in the Discord pairing flow is a real security/privacy regression.
  • The core sanitization fix (stripOpenAIWireFormatText) is correct and well-tested. However, the unpairedResponse refactor in agent-components.ts contains a logic error where the "already requested" message is sent unconditionally even in silent mode, and the Dockerfile permission change risks breaking executable assets in extension directories. These issues need to be addressed before merging.
  • src/discord/monitor/agent-components.ts requires a fix to gate the created === false reply through the unpaired-response mode check. Dockerfile needs the file permission strategy revisited to avoid stripping executable bits.

Last reviewed commit: f731b69

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

29 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +507 to +514
const responseText = created
? buildUnpairedResponse({
channel: "discord",
idLine: `Your Discord user id: ${user.id}`,
code,
mode: getUnpairedResponseMode(cfg),
})
: "Pairing already requested. Ask the bot owner to approve your code.";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silent mode still sends "already requested" message

When created === false (the user has already made a pairing request), responseText is unconditionally set to "Pairing already requested. Ask the bot owner to approve your code." — a non-null string. The if (responseText !== null) guard on line 516 therefore never suppresses this branch.

In "silent" mode the intent is to reveal nothing to unauthorized users, but an attacker who sends a second message will receive this reply, leaking that their first pairing request was recorded.

The fix is to also gate the repeat-request message through the mode check:

const responseText = (() => {
  if (!created) {
    // Only inform the user if the mode is not silent
    const mode = getUnpairedResponseMode(cfg);
    return mode !== "silent"
      ? "Pairing already requested. Ask the bot owner to approve your code."
      : null;
  }
  return buildUnpairedResponse({
    channel: "discord",
    idLine: `Your Discord user id: ${user.id}`,
    code,
    mode: getUnpairedResponseMode(cfg),
  });
})();
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/discord/monitor/agent-components.ts
Line: 507-514

Comment:
**Silent mode still sends "already requested" message**

When `created === false` (the user has already made a pairing request), `responseText` is unconditionally set to `"Pairing already requested. Ask the bot owner to approve your code."` — a non-null string. The `if (responseText !== null)` guard on line 516 therefore never suppresses this branch.

In "silent" mode the intent is to reveal nothing to unauthorized users, but an attacker who sends a second message will receive this reply, leaking that their first pairing request was recorded.

The fix is to also gate the repeat-request message through the mode check:

```
const responseText = (() => {
  if (!created) {
    // Only inform the user if the mode is not silent
    const mode = getUnpairedResponseMode(cfg);
    return mode !== "silent"
      ? "Pairing already requested. Ask the bot owner to approve your code."
      : null;
  }
  return buildUnpairedResponse({
    channel: "discord",
    idLine: `Your Discord user id: ${user.id}`,
    code,
    mode: getUnpairedResponseMode(cfg),
  });
})();
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +598 to +601
{
name: "function call with trailing content",
text: "assistant to=functions.subagents.commentary some json here and then normal text after",
expected: "some json here and then normal text after",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate test case name

The name "function call with trailing content" is already used for the test case at line 562. When a test in this loop fails and the case name is printed, there will be two entries with the same label, making it hard to identify which one failed. Consider renaming this one to something more descriptive, e.g. "function call at start with trailing text".

Suggested change
{
name: "function call with trailing content",
text: "assistant to=functions.subagents.commentary some json here and then normal text after",
expected: "some json here and then normal text after",
{
name: "function call at start with trailing text",
text: "assistant to=functions.subagents.commentary some json here and then normal text after",
expected: "some json here and then normal text after",
},
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-embedded-utils.test.ts
Line: 598-601

Comment:
**Duplicate test case name**

The name `"function call with trailing content"` is already used for the test case at line 562. When a test in this loop fails and the case name is printed, there will be two entries with the same label, making it hard to identify which one failed. Consider renaming this one to something more descriptive, e.g. `"function call at start with trailing text"`.

```suggestion
      {
        name: "function call at start with trailing text",
        text: "assistant to=functions.subagents.commentary some json here and then normal text after",
        expected: "some json here and then normal text after",
      },
```

How can I resolve this? If you propose a fix, please make it concise.

@greptile-apps

greptile-apps Bot commented Mar 1, 2026

Copy link
Copy Markdown
Contributor
Additional Comments (1)

Dockerfile
chmod 644 strips executable bit from all files

The second find command applies 644 to every file in the extension and agent directories, removing the executable bit from any shell scripts or compiled binaries that live there. Node.js module files (.js, .ts) don't need the executable bit, but if any extension ships a helper script or pre-compiled binary (common in .agent/.agents directories), it will silently fail to execute at runtime.

Consider limiting the permission change to known non-executable types, or applying chmod 755 to files that already have an executable bit set:

# Only strip world-write permissions, leave executable bits intact
RUN find /app/extensions /app/.agent /app/.agents -type d -exec chmod go-w {} \; 2>/dev/null || true && \
    find /app/extensions /app/.agent /app/.agents -type f -exec chmod go-w {} \; 2>/dev/null || true
Prompt To Fix With AI
This is a comment left during a code review.
Path: Dockerfile
Line: 12-13

Comment:
**`chmod 644` strips executable bit from all files**

The second `find` command applies `644` to every file in the extension and agent directories, removing the executable bit from any shell scripts or compiled binaries that live there. Node.js module files (`.js`, `.ts`) don't need the executable bit, but if any extension ships a helper script or pre-compiled binary (common in `.agent`/`.agents` directories), it will silently fail to execute at runtime.

Consider limiting the permission change to known non-executable types, or applying `chmod 755` to files that already have an executable bit set:

```
# Only strip world-write permissions, leave executable bits intact
RUN find /app/extensions /app/.agent /app/.agents -type d -exec chmod go-w {} \; 2>/dev/null || true && \
    find /app/extensions /app/.agent /app/.agents -type f -exec chmod go-w {} \; 2>/dev/null || true
```

How can I resolve this? If you propose a fix, please make it concise.

@kevinWangSheng

Copy link
Copy Markdown
Contributor Author

Closing - issue #30441 has been closed as completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling app: web-ui App: web-ui channel: discord Channel integration: discord channel: feishu Channel integration: feishu channel: nextcloud-talk Channel integration: nextcloud-talk channel: telegram Channel integration: telegram channel: whatsapp-web Channel integration: whatsapp-web docker Docker and sandbox tooling size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant