feat: add Rabbit R1 device pairing command#589
Open
kalub92 wants to merge 1 commit intoNVIDIA:mainfrom
Open
Conversation
Add `nemoclaw <name> r1-connect` command that configures the OpenClaw gateway inside a sandbox for Rabbit R1 device pairing over LAN or cloudflared tunnel. Also refactors the prompt() helper to use bash read instead of Node.js readline (fixes hanging after heavy spawnSync usage), and switches sandbox build context copy to rsync for efficiency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughModified the user input collection mechanism from Node.js readline to bash Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as nemoclaw CLI
participant Sandbox as NemoClaw Sandbox
participant Gateway as OpenClaw Gateway
participant Tunnel as cloudflared Tunnel
participant Device as Rabbit R1 Device
User->>CLI: nemoclaw sandbox-name r1-connect [--tunnel]
CLI->>Sandbox: bash r1-connect.sh --sandbox sandbox-name
Sandbox->>Sandbox: Validate sandbox & SSH access
Sandbox->>Gateway: Read/generate gateway.auth.token
Sandbox->>Gateway: Configure gateway (LAN, token, auth)
Sandbox->>Gateway: Restart gateway service
Sandbox->>Sandbox: Detect host LAN IPv4 addresses
Sandbox->>Sandbox: Setup port forward to 0.0.0.0:DASHBOARD_PORT
alt --tunnel flag enabled
Sandbox->>Tunnel: Start cloudflared tunnel to localhost:port
Tunnel->>Tunnel: Establish secure connection
Sandbox->>Sandbox: Retrieve tunnel host URL
else LAN mode
Sandbox->>Sandbox: Use detected LAN IPs
end
Sandbox->>Sandbox: Generate QR code with connection params
Sandbox->>User: Display QR code & connection details
User->>Device: Scan QR code on Rabbit R1
Device->>Gateway: Connect to gateway (via LAN or tunnel)
Gateway->>Sandbox: Device registration request
Sandbox->>Sandbox: Poll openclaw devices list
Sandbox->>Gateway: Auto-approve device
Sandbox->>User: Pairing complete
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment Tip CodeRabbit can use your project's `biome` configuration to improve the quality of JS/TS/CSS/JSON code reviews.Add a configuration file to your project to customize how CodeRabbit runs |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
scripts/r1-connect.sh (3)
290-293: Consider validatingPENDING(requestId) format before use in shell command.The
requestIdfromopenclaw devices listis used in a shell command at line 292. While it's likely a safe identifier from the openclaw system, validating it as alphanumeric/UUID would prevent potential injection if the format ever changes.🛡️ Proposed validation
if [ -n "$PENDING" ]; then + # Validate requestId is alphanumeric/UUID-like + if [[ ! "$PENDING" =~ ^[a-zA-Z0-9_-]+$ ]]; then + warn "Invalid requestId format, skipping approval" + continue + fi info "Found Rabbit R1 device, approving..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/r1-connect.sh` around lines 290 - 293, Validate the PENDING value before using it in the sandbox_exec call: ensure PENDING matches an expected safe pattern (e.g., UUID or strict alphanumeric with allowed -/_ characters) and if it fails validation, log an error and skip/exit instead of calling sandbox_exec "openclaw devices approve '$PENDING'"; update the validation immediately before the sandbox_exec invocation that uses PENDING and reference the PENDING variable and the sandbox_exec "openclaw devices approve" call when making the change.
258-259: Token is printed to stdout - ensure this is intentional.Line 258 echoes the raw token to stdout. This is likely intentional for the user to copy, but be aware this could appear in logs if output is captured.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/r1-connect.sh` around lines 258 - 259, The script currently prints the raw token variable via echo "Token: $TOKEN" which can leak secrets into stdout/logs; update r1-connect.sh to stop printing the raw TOKEN to stdout—either remove that echo, print a masked version, or send the token to stderr (e.g., write to >&2) and document the behavior so callers know it may appear in logs; target the echo that references TOKEN to implement this change.
69-72: Loose sandbox name matching may cause false positives.The
grep -q "$SANDBOX_NAME"check will match substrings. For example, a sandbox named"my"would match"my-assistant"in the list. Consider using word boundaries or exact matching.♻️ Proposed fix using word boundary
SANDBOX_LIST=$(openshell sandbox list 2>&1 || true) -if ! echo "$SANDBOX_LIST" | grep -q "$SANDBOX_NAME"; then +if ! echo "$SANDBOX_LIST" | grep -qw "$SANDBOX_NAME"; then error "Sandbox '$SANDBOX_NAME' not found. Run 'nemoclaw onboard' first." fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/r1-connect.sh` around lines 69 - 72, The current check uses a substring grep against SANDBOX_LIST which can match partial names; replace the substring match with an exact whole-line/fixed-string comparison when testing SANDBOX_NAME (the pipeline that echoes SANDBOX_LIST and the grep -q "$SANDBOX_NAME" check). In practice, change the grep invocation to perform a fixed-string whole-line match (or use awk/per-line equality) so only exact sandbox names match, preserving the surrounding logic that prints the error if no match is found.bin/lib/credentials.js (1)
6-7: Remove unusedreadlineimport.The
readlinemodule is imported but no longer used after refactoringprompt()to use bashread. Additionally,spawnSyncis required inline at line 39 but could be destructured here alongsideexecSync.♻️ Proposed fix
const fs = require("fs"); const path = require("path"); -const readline = require("readline"); -const { execSync } = require("child_process"); +const { execSync, spawnSync } = require("child_process");And at line 39:
- const { spawnSync } = require("child_process");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/lib/credentials.js` around lines 6 - 7, Remove the unused readline import and instead destructure spawnSync alongside execSync from child_process; update the top-level requires to remove the readline symbol and change const { execSync } = require("child_process") to include spawnSync so later inline use of spawnSync (used in prompt() at/near line where spawnSync is called) no longer requires a separate require.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/r1-connect.sh`:
- Around line 125-140: The existing token read into EXISTING_TOKEN is used
directly in sandbox_exec when setting gateway.auth.token (sandbox_exec "openclaw
config set gateway.auth.token '$TOKEN'") which allows shell injection if the
token contains quotes; validate EXISTING_TOKEN against a safe pattern (e.g.,
only [0-9a-fA-F]{64} for a 32-byte hex token) and if it fails, discard it and
generate a new secure token with openssl rand -hex 32, then assign to TOKEN;
additionally ensure the value passed to sandbox_exec is always a
validated/quoted hex-only TOKEN to prevent any chance of shell escaping when
calling sandbox_exec "openclaw config set gateway.auth.token '$TOKEN'".
---
Nitpick comments:
In `@bin/lib/credentials.js`:
- Around line 6-7: Remove the unused readline import and instead destructure
spawnSync alongside execSync from child_process; update the top-level requires
to remove the readline symbol and change const { execSync } =
require("child_process") to include spawnSync so later inline use of spawnSync
(used in prompt() at/near line where spawnSync is called) no longer requires a
separate require.
In `@scripts/r1-connect.sh`:
- Around line 290-293: Validate the PENDING value before using it in the
sandbox_exec call: ensure PENDING matches an expected safe pattern (e.g., UUID
or strict alphanumeric with allowed -/_ characters) and if it fails validation,
log an error and skip/exit instead of calling sandbox_exec "openclaw devices
approve '$PENDING'"; update the validation immediately before the sandbox_exec
invocation that uses PENDING and reference the PENDING variable and the
sandbox_exec "openclaw devices approve" call when making the change.
- Around line 258-259: The script currently prints the raw token variable via
echo "Token: $TOKEN" which can leak secrets into stdout/logs; update
r1-connect.sh to stop printing the raw TOKEN to stdout—either remove that echo,
print a masked version, or send the token to stderr (e.g., write to >&2) and
document the behavior so callers know it may appear in logs; target the echo
that references TOKEN to implement this change.
- Around line 69-72: The current check uses a substring grep against
SANDBOX_LIST which can match partial names; replace the substring match with an
exact whole-line/fixed-string comparison when testing SANDBOX_NAME (the pipeline
that echoes SANDBOX_LIST and the grep -q "$SANDBOX_NAME" check). In practice,
change the grep invocation to perform a fixed-string whole-line match (or use
awk/per-line equality) so only exact sandbox names match, preserving the
surrounding logic that prints the error if no match is found.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33076755-6887-43d0-9e47-721a67519433
📒 Files selected for processing (4)
bin/lib/credentials.jsbin/lib/onboard.jsbin/nemoclaw.jsscripts/r1-connect.sh
Comment on lines
+125
to
+140
| EXISTING_TOKEN=$(sandbox_exec "openclaw config get gateway.auth.token 2>/dev/null" || echo "") | ||
| EXISTING_TOKEN=$(echo "$EXISTING_TOKEN" | tr -d '[:space:]') | ||
|
|
||
| if [ -n "$EXISTING_TOKEN" ] && [ "$EXISTING_TOKEN" != "null" ] && [ "$EXISTING_TOKEN" != "undefined" ]; then | ||
| TOKEN="$EXISTING_TOKEN" | ||
| info "Reusing existing auth token" | ||
| else | ||
| TOKEN=$(openssl rand -hex 32) | ||
| info "Generated new auth token" | ||
| fi | ||
|
|
||
| # Apply gateway config inside the sandbox | ||
| sandbox_exec "openclaw config set gateway.bind lan" | ||
| sandbox_exec "openclaw config set gateway.auth.mode token" | ||
| sandbox_exec "openclaw config set gateway.auth.token '$TOKEN'" | ||
| sandbox_exec "openclaw config set gateway.controlUi.allowInsecureAuth true" |
There was a problem hiding this comment.
Potential shell injection via untrusted existing token.
The EXISTING_TOKEN is read from the sandbox and used in a shell command at line 139 with single quotes. If an attacker could set a malicious token containing ', they could escape the quotes and inject commands.
While the new token from openssl rand -hex 32 (line 132) is safe (hex-only), the existing token path should validate the token format.
🛡️ Proposed fix to validate token format
EXISTING_TOKEN=$(sandbox_exec "openclaw config get gateway.auth.token 2>/dev/null" || echo "")
EXISTING_TOKEN=$(echo "$EXISTING_TOKEN" | tr -d '[:space:]')
-if [ -n "$EXISTING_TOKEN" ] && [ "$EXISTING_TOKEN" != "null" ] && [ "$EXISTING_TOKEN" != "undefined" ]; then
+# Validate token is hex-only (safe for shell embedding)
+if [ -n "$EXISTING_TOKEN" ] && [ "$EXISTING_TOKEN" != "null" ] && [ "$EXISTING_TOKEN" != "undefined" ] && [[ "$EXISTING_TOKEN" =~ ^[a-fA-F0-9]+$ ]]; then
TOKEN="$EXISTING_TOKEN"
info "Reusing existing auth token"
else
TOKEN=$(openssl rand -hex 32)
info "Generated new auth token"
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| EXISTING_TOKEN=$(sandbox_exec "openclaw config get gateway.auth.token 2>/dev/null" || echo "") | |
| EXISTING_TOKEN=$(echo "$EXISTING_TOKEN" | tr -d '[:space:]') | |
| if [ -n "$EXISTING_TOKEN" ] && [ "$EXISTING_TOKEN" != "null" ] && [ "$EXISTING_TOKEN" != "undefined" ]; then | |
| TOKEN="$EXISTING_TOKEN" | |
| info "Reusing existing auth token" | |
| else | |
| TOKEN=$(openssl rand -hex 32) | |
| info "Generated new auth token" | |
| fi | |
| # Apply gateway config inside the sandbox | |
| sandbox_exec "openclaw config set gateway.bind lan" | |
| sandbox_exec "openclaw config set gateway.auth.mode token" | |
| sandbox_exec "openclaw config set gateway.auth.token '$TOKEN'" | |
| sandbox_exec "openclaw config set gateway.controlUi.allowInsecureAuth true" | |
| EXISTING_TOKEN=$(sandbox_exec "openclaw config get gateway.auth.token 2>/dev/null" || echo "") | |
| EXISTING_TOKEN=$(echo "$EXISTING_TOKEN" | tr -d '[:space:]') | |
| # Validate token is hex-only (safe for shell embedding) | |
| if [ -n "$EXISTING_TOKEN" ] && [ "$EXISTING_TOKEN" != "null" ] && [ "$EXISTING_TOKEN" != "undefined" ] && [[ "$EXISTING_TOKEN" =~ ^[a-fA-F0-9]+$ ]]; then | |
| TOKEN="$EXISTING_TOKEN" | |
| info "Reusing existing auth token" | |
| else | |
| TOKEN=$(openssl rand -hex 32) | |
| info "Generated new auth token" | |
| fi | |
| # Apply gateway config inside the sandbox | |
| sandbox_exec "openclaw config set gateway.bind lan" | |
| sandbox_exec "openclaw config set gateway.auth.mode token" | |
| sandbox_exec "openclaw config set gateway.auth.token '$TOKEN'" | |
| sandbox_exec "openclaw config set gateway.controlUi.allowInsecureAuth true" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/r1-connect.sh` around lines 125 - 140, The existing token read into
EXISTING_TOKEN is used directly in sandbox_exec when setting gateway.auth.token
(sandbox_exec "openclaw config set gateway.auth.token '$TOKEN'") which allows
shell injection if the token contains quotes; validate EXISTING_TOKEN against a
safe pattern (e.g., only [0-9a-fA-F]{64} for a 32-byte hex token) and if it
fails, discard it and generate a new secure token with openssl rand -hex 32,
then assign to TOKEN; additionally ensure the value passed to sandbox_exec is
always a validated/quoted hex-only TOKEN to prevent any chance of shell escaping
when calling sandbox_exec "openclaw config set gateway.auth.token '$TOKEN'".
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
1 participant
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
nemoclaw <name> r1-connectcommand that configures the OpenClaw gateway inside a sandbox for Rabbit R1 device pairing over LAN or cloudflared tunnelscripts/r1-connect.shwhich handles SSH into the sandbox, gateway configuration (bind to LAN, token auth), port forwarding rebind, QR code generation, and auto-approval of pending R1 devicesprompt()helper in credentials.js to use bashreadinstead of Node.js readline, fixing terminal hangs after heavy spawnSync usagecp -r+rm -rftorsync --excludefor efficiencyTest plan
nemoclaw <sandbox> r1-connectwith a running sandbox and verify gateway is configured for LAN accessnemoclaw <sandbox> r1-connect --tunneland verify cloudflared tunnel is establishednpm testto ensure no regressions🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
r1-connectcommand to configure Rabbit R1 device pairing within sandboxes over LAN.Improvements