Skip to content

Approval flow#487

Merged
khaliqgant merged 4 commits intomainfrom
approval-flow
Mar 5, 2026
Merged

Approval flow#487
khaliqgant merged 4 commits intomainfrom
approval-flow

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

@khaliqgant khaliqgant commented Mar 5, 2026

Summary

Test Plan

  • Tests added/updated
  • Manual testing completed

Screenshots


Open with Devin

@khaliqgant khaliqgant requested a review from willwashburn as a code owner March 5, 2026 00:40
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

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

View in Devin Review to see 5 additional findings.

Open in Devin Review

Resolved conflicts in packages/openclaw/skill/SKILL.md, keeping the more
detailed troubleshooting documentation from the approval-flow branch
including:
- Device pairing explanation with Ed25519 keypair details
- Step-by-step pairing approval instructions
- Full recovery runbook with jq and python3 alternatives
- Extended diagnostic matrix with more symptoms
- Safety improvements (explicit PIDs instead of broad pkill patterns)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
const requestId = errObj?.requestId ?? errObj?.request_id ?? '';
console.error('[openclaw-ws] Pairing rejected — device is not paired with the OpenClaw gateway.');
if (requestId) {
console.error(`[openclaw-ws] Approve this device: openclaw devices approve ${requestId}`);

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
.

Copilot Autofix

AI about 2 months ago

In general, to fix log injection, sanitize any user‑controlled data before interpolating it into log messages. For plain‑text logs, a minimal and compatible approach is to strip newline (\n) and carriage‑return (\r) characters so an attacker cannot break or prepend/append forged log lines. Optionally, you can also constrain logs to visible characters.

For this specific case in packages/openclaw/src/gateway.ts, we should sanitize requestId once before it is logged. Currently:

const requestId = errObj?.requestId ?? errObj?.request_id ?? '';
console.error('[openclaw-ws] Pairing rejected — device is not paired with the OpenClaw gateway.');
if (requestId) {
  console.error(`[openclaw-ws] Approve this device:  openclaw devices approve ${requestId}`);
}

We can modify the requestId assignment so that any \n or \r are removed, and optionally tag it clearly as user‑derived. This requires no new imports. The best minimal change is:

const rawRequestId = errObj?.requestId ?? errObj?.request_id ?? '';
const requestId = typeof rawRequestId === 'string'
  ? rawRequestId.replace(/[\r\n]/g, '')
  : String(rawRequestId).replace(/[\r\n]/g, '');

Then keep the existing conditional and logging logic, using the sanitized requestId. This ensures the log contents remain functionally the same, but control characters are removed so the log line cannot be split or injected.

Suggested changeset 1
packages/openclaw/src/gateway.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/openclaw/src/gateway.ts b/packages/openclaw/src/gateway.ts
--- a/packages/openclaw/src/gateway.ts
+++ b/packages/openclaw/src/gateway.ts
@@ -417,7 +417,11 @@
 
         if (isPairing) {
           const errObj = msg.error as Record<string, unknown> | undefined;
-          const requestId = errObj?.requestId ?? errObj?.request_id ?? '';
+          const rawRequestId = errObj?.requestId ?? errObj?.request_id ?? '';
+          const requestId =
+            typeof rawRequestId === 'string'
+              ? rawRequestId.replace(/[\r\n]/g, '')
+              : String(rawRequestId).replace(/[\r\n]/g, '');
           console.error('[openclaw-ws] Pairing rejected — device is not paired with the OpenClaw gateway.');
           if (requestId) {
             console.error(`[openclaw-ws] Approve this device:  openclaw devices approve ${requestId}`);
EOF
@@ -417,7 +417,11 @@

if (isPairing) {
const errObj = msg.error as Record<string, unknown> | undefined;
const requestId = errObj?.requestId ?? errObj?.request_id ?? '';
const rawRequestId = errObj?.requestId ?? errObj?.request_id ?? '';
const requestId =
typeof rawRequestId === 'string'
? rawRequestId.replace(/[\r\n]/g, '')
: String(rawRequestId).replace(/[\r\n]/g, '');
console.error('[openclaw-ws] Pairing rejected — device is not paired with the OpenClaw gateway.');
if (requestId) {
console.error(`[openclaw-ws] Approve this device: openclaw devices approve ${requestId}`);
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 8 additional findings in Devin Review.

Open in Devin Review

---
name: openclaw-relay
version: 3.1.5
version: 3.1.7
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.

🟡 SKILL.md version (3.1.7) mismatches package.json version (3.1.5)

The SKILL.md frontmatter was bumped from 3.1.5 to 3.1.7, but packages/openclaw/package.json:3 still declares "version": "3.1.5". The SKILL.md also references "3.1.6+" as the version introducing auto-retry after pairing rejection (the feature added in this PR), but the package hasn't been bumped to reflect this. Consumers or agents that read the skill metadata will see version 3.1.7 while the actual published package is 3.1.5, creating a confusing inconsistency.

Prompt for agents
Either bump packages/openclaw/package.json version to 3.1.7 (or 3.1.6) to match SKILL.md, or keep SKILL.md at the current package.json version (3.1.5) until the actual version bump is performed. The version in SKILL.md line 3 and packages/openclaw/package.json line 3 should stay in sync.
Open in Devin Review

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

@khaliqgant khaliqgant merged commit b799737 into main Mar 5, 2026
33 checks passed
@khaliqgant khaliqgant deleted the approval-flow branch March 5, 2026 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants