fix(security): default standalone servers to loopback bind#12370
Closed
davidrudduck wants to merge 20 commits intoopenclaw:mainfrom
Closed
fix(security): default standalone servers to loopback bind#12370davidrudduck wants to merge 20 commits intoopenclaw:mainfrom
davidrudduck wants to merge 20 commits intoopenclaw:mainfrom
Conversation
The actual crash site: contextFiles array can contain entries with undefined path. buildAgentSystemPrompt calls file.path.trim() inside Array.some() to detect SOUL.md — crashes when path is undefined. Stack trace: at buildAgentSystemPrompt (system-prompt.ts) at buildEmbeddedSystemPrompt at runEmbeddedAttempt This was the real .trim() crash killing all sub-agent spawns at ~34ms. Refs: openclaw/openclaw#6535
…path Add null coalescing (?? '') before .trim() in: - src/gateway/server-methods/agent.ts (request.message, p.runId) - src/gateway/session-utils.ts (params.sessionKey) - src/agents/subagent-registry.ts (requesterSessionKey) - src/agents/subagent-announce.ts (requesterSessionKey) Fixes TypeError: Cannot read properties of undefined (reading 'trim') that crashes subagent spawns at ~34ms before the agent session starts. Refs: openclaw/openclaw#6535
runMessageSent was defined in hooks.ts but never called anywhere.
This wires it into deliverOutboundPayloads() in deliver.ts so plugins
listening via api.on('message_sent') actually receive events.
Fixes #6535, #5513, #4006
Discord uses deliverDiscordReply() which bypasses the generic deliverOutboundPayloads(). Added hook call in reply-delivery.ts so Discord outbound messages also fire message_sent. Refs #6535
Move message_sent hook from channel-specific delivery paths to the central createReplyDispatcher(). This fires for ALL channels (Discord, WhatsApp, Signal, Slack, etc.) without patching each one. Added optional hookContext to ReplyDispatcherOptions for callers to provide channel/routing context. Falls back to empty strings when not provided — plugin still gets the message content. Removed Discord-specific hook from reply-delivery.ts (now redundant). Kept deliver.ts hook for non-dispatcher delivery paths (cron, heartbeat). Refs #6535
- 5 tests for reply-dispatcher: hook fires on delivery, passes hookContext, skips on failure/empty text, errors don't break delivery - 2 tests added to deliver.test.ts: hook fires with combined text, errors don't propagate All 19 tests pass. Covers both centralised (dispatcher) and fallback (deliverOutboundPayloads) hook call sites. Refs #6535
When bestEffort is enabled, failed payloads are skipped via onError. Only include text from payloads that produced delivery results to avoid reporting undelivered content as success. Co-authored-by: CodeRabbit <support@coderabbit.ai>
Require hookContext.channelId to be present before firing the hook. Callers must opt in with valid context — prevents emitting empty/invalid fields that would break plugins expecting meaningful channel metadata. Co-authored-by: CodeRabbit <support@coderabbit.ai>
- reply-dispatcher: use empty string fallback for `to` instead of channelId (channelId is a channel kind like "telegram", not a destination) - deliver: remove broken .slice(0, results.length) — results count sends not payloads, so chunking and bestEffort failures misreport text
The actual crash site: contextFiles array can contain entries with undefined path. buildAgentSystemPrompt calls file.path.trim() inside Array.some() to detect SOUL.md — crashes when path is undefined. Stack trace: at buildAgentSystemPrompt (system-prompt.ts) at buildEmbeddedSystemPrompt at runEmbeddedAttempt This was the real .trim() crash killing all sub-agent spawns at ~34ms. Refs: openclaw/openclaw#6535
…path Add null coalescing (?? '') before .trim() in: - src/gateway/server-methods/agent.ts (request.message, p.runId) - src/gateway/session-utils.ts (params.sessionKey) - src/agents/subagent-registry.ts (requesterSessionKey) - src/agents/subagent-announce.ts (requesterSessionKey) Fixes TypeError: Cannot read properties of undefined (reading 'trim') that crashes subagent spawns at ~34ms before the agent session starts. Refs: openclaw/openclaw#6535
Remove 23 tracked .omc/ files (session data, project memory, checkpoints) that should never have been committed. These are local Claude Code tooling artefacts.
Replace `"undefined"` literal with `"(unknown)"` to match the empty-string guard used in path normalization and avoid leaking TS artifacts into model context.
# Conflicts: # src/agents/system-prompt.ts
…nsistency When sandbox is enabled with read-only workspace access, effectiveWorkspace differs from resolvedWorkspace. All other call sites already use effectiveWorkspace — createAgentSession was the only outlier, causing the agent session cwd to point at a directory the sandbox may not allow access to.
Change canvas host and telegram webhook default bind from 0.0.0.0 (all interfaces) to 127.0.0.1 (loopback only) to prevent unintended network exposure when no explicit host is configured.
Comment on lines
+379
to
+384
| const hookRunner = getGlobalHookRunner(); | ||
| if (hookRunner && results.length > 0) { | ||
| const deliveredText = normalizedPayloads | ||
| .map((p) => p.text ?? "") | ||
| .filter(Boolean) | ||
| .join("\n"); |
Contributor
There was a problem hiding this comment.
Incorrect delivered text
deliverOutboundPayloads says it will “only report text from payloads that produced delivery results”, but deliveredText is built from all normalizedPayloads (normalizedPayloads.map(...)) as long as results.length > 0. With bestEffort: true, if an early payload fails and a later one succeeds, results.length is still >0 and the hook will incorrectly include text from the failed payload(s), reporting content that was never delivered.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/infra/outbound/deliver.ts
Line: 379:384
Comment:
**Incorrect delivered text**
`deliverOutboundPayloads` says it will “only report text from payloads that produced delivery results”, but `deliveredText` is built from *all* `normalizedPayloads` (`normalizedPayloads.map(...)`) as long as `results.length > 0`. With `bestEffort: true`, if an early payload fails and a later one succeeds, `results.length` is still >0 and the hook will incorrectly include text from the failed payload(s), reporting content that was never delivered.
How can I resolve this? If you propose a fix, please make it concise.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
0.0.0.0(all interfaces) to127.0.0.1(loopback only)Key Changes
src/canvas-host/server.ts: DefaultlistenHostfallback"0.0.0.0"→"127.0.0.1"src/telegram/webhook.ts: Defaulthostfallback"0.0.0.0"→"127.0.0.1"How It Works
listenHost/hostexplicitly in their configurationauto/loopback/lan/custom) — these standalone servers were bypassing itFiles Changed
src/canvas-host/server.ts:452"0.0.0.0"→"127.0.0.1"src/telegram/webhook.ts:36"0.0.0.0"→"127.0.0.1"Testing
pnpm tsgo): passes cleanGreptile Overview
Greptile Summary
This PR tightens default network exposure by switching the standalone Canvas host and Telegram webhook bind defaults from
0.0.0.0to127.0.0.1.Along the way, it also hardens several
.trim()call sites againstundefinedinputs, updates workspace resolution in the embedded runner to useresolveRunWorkspaceDir, and introduces a newmessage_senthook path in both the reply dispatcher and outbound delivery pipeline (with accompanying tests).Confidence Score: 4/5
Context used:
dashboard- CLAUDE.md (source)dashboard- AGENTS.md (source)