key improvements

[khaliqgant]

Summary

Test Plan

• Tests added/updated
• Manual testing completed

Screenshots

---

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

In general, to fix log injection you should sanitize any untrusted input before interpolating it into log messages. For plain-text logs, a common minimal approach is to strip carriage return and newline characters so an attacker cannot break out onto new log lines, and optionally remove other control characters. It is also useful to clearly delimit or quote user-provided values in logs.

For this specific issue, the best minimally invasive fix is to sanitize requestId right before it is used in the log statement. Since we only see the problematic log at line 688 and the value is only used for display to the user, we can create a sanitized version (e.g., safeRequestId) by converting requestId to a string and removing \r and \n. Then we use safeRequestId in the console.error message. This preserves existing functionality (the user still sees the same identifier for normal values) while preventing forged log lines from maliciously crafted requestId values. No new imports or external libraries are needed; we can rely on String() and .replace with a suitable regular expression.

Concretely:

• In packages/openclaw/src/gateway.ts, inside the if (requestId) { ... } block near line 688, introduce a const safeRequestId = String(requestId).replace(/[\r\n]/g, '');.
• Replace the existing console.error line that logs requestId with one that logs safeRequestId instead.
• Optionally, this pattern can be reused elsewhere if you later decide to sanitize other logged values, but for this fix we constrain changes only to the provided snippet.

[devin-ai-integration]

Devin Review found 2 new potential issues.

View 12 additional findings in Devin Review.

[devin-ai-integration]

🟡 Fallback state (payloadVersionOverride, fallbackAttempted) never resets during auto-reconnect, locking retries to one payload version

When both v3 and v2 payload versions fail authentication, the connect() promise is rejected but the WebSocket close handler still calls scheduleReconnect() (gateway.ts:564-565). scheduleReconnect invokes doConnect() directly (gateway.ts:812,826) which does NOT reset payloadVersionOverride or fallbackAttempted — those are only reset in connect() (gateway.ts:474-476). This means all subsequent auto-reconnect attempts are locked to the last-tried fallback payload version and never cycle back to the original. If the first version was rejected due to a transient server issue (and the isSignatureInvalid regex at line 680 is fairly broad — device.signature matches any error containing "device signature" even without "invalid"), the client would be permanently stuck retrying only the wrong payload version until an explicit connect() call is made.

(Refers to lines 809-813)

Prompt for agents

In packages/openclaw/src/gateway.ts, the scheduleReconnect() method (around lines 797-828) calls doConnect() directly, which does not reset the fallback state (payloadVersionOverride, fallbackAttempted). After both v3 and v2 fail, auto-reconnects are stuck on one version. Consider resetting these fields before calling doConnect() in scheduleReconnect(), similar to how connect() resets them at lines 474-476. For example, in the two setTimeout callbacks that call this.doConnect() (around lines 811-812 and 825-826), add:

  this.payloadVersionOverride = null;
  this.fallbackAttempted = false;

This way each auto-reconnect cycle gets a fresh chance to try the primary version first and fall back if needed.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🟡 Missing this.ws !== ws guard on WebSocket message handler allows stale messages during v3↔v2 fallback

The close and error handlers in doConnect() were given if (this.ws !== ws) return; guards (lines 535, 571) to ignore events from superseded WebSocket instances during v3↔v2 fallback. However, the message handler at line 528 was not given the same guard. During the fallback flow, this.ws is set to null and the old WebSocket is closed (gateway.ts:713-714), but if a buffered message is delivered on the old socket before the close completes, handleMessage would execute against the current (potentially new) connection state. For example, a stale connect.challenge from the old socket could cause a duplicate connect request to be sent on the newly created socket.

Inconsistent guard pattern in doConnect()

The close handler at line 532-567 and error handler at line 569-581 both check if (this.ws !== ws) return; but the message handler at line 528-530 does not:

ws.on('message', (data) => {
  this.handleMessage(data.toString()); // no guard
});

Suggested change

	ws.on('message', (data) => {
	this.handleMessage(data.toString());
	});
	ws.on('message', (data) => {
	// Guard: ignore messages from superseded WebSocket instances.
	if (this.ws !== ws) return;
	this.handleMessage(data.toString());
	});

---

Was this helpful? React with 👍 or 👎 to provide feedback.