Skip to content

Fix doctor and backup checks for local gateway runtimes #4583

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Christoffer91 wants to merge 2 commits into from
Closed

Fix doctor and backup checks for local gateway runtimes #4583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8567021
Fix doctor and backup checks for local gateway runtimes
Christoffer91 May 31, 2026
ee96a76
Merge branch 'main' into codex/nemoclaw-doctor-backup-fixes
cv Jun 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/lib/actions/sandbox/doctor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,20 @@ function dockerInspectGateway(containerName: string): DoctorCheck[] {
5000,
);
if (inspect.status !== 0) {
const processCheck = captureHostCommand("pgrep", ["-af", "openshell-gateway"], 5000);

@coderabbitai coderabbitai Bot Jun 3, 2026

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Simulate a false positive scenario where a user is editing the gateway binary
# but the gateway itself is not running.

# Check if the bare substring pattern would match unrelated processes
echo "Testing bare substring pattern (current code):"
ps aux | grep -i openshell-gateway | grep -v grep || echo "No matches"

echo ""
echo "The anchored pattern from HOST_GATEWAY_PGREP_PATTERN should be used instead:"
cat src/lib/onboard/host-gateway-process.ts | grep -A2 "HOST_GATEWAY_PGREP_PATTERN"

Repository: NVIDIA/NemoClaw

Length of output: 618

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Inspect doctor.ts around the reported lines
echo "=== doctor.ts around lines 180-240 ==="
nl -ba src/lib/actions/sandbox/doctor.ts | sed -n '180,240p'

# 2) Inspect host-gateway-process.ts for the anchored pattern definition
echo
echo "=== host-gateway-process.ts HOST_GATEWAY_PGREP_PATTERN definition ==="
rg -n "HOST_GATEWAY_PGREP_PATTERN" src/lib/onboard/host-gateway-process.ts
nl -ba src/lib/onboard/host-gateway-process.ts | sed -n '1,120p'

# 3) Check what ss usage looks like elsewhere in the repo (to judge syntax expectations)
echo
echo "=== repo-wide 'ss -ltn' occurrences (trimmed) ==="
rg -n "ss -ltn" -S src | head -n 50

Repository: NVIDIA/NemoClaw

Length of output: 159

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Show doctor.ts around the target area (no nl available)
echo "=== src/lib/actions/sandbox/doctor.ts lines 180-240 (approx) ==="
sed -n '180,240p' src/lib/actions/sandbox/doctor.ts

# 2) Locate and show HOST_GATEWAY_PGREP_PATTERN definition
echo
echo "=== src/lib/onboard/host-gateway-process.ts: HOST_GATEWAY_PGREP_PATTERN ==="
rg -n "HOST_GATEWAY_PGREP_PATTERN" src/lib/onboard/host-gateway-process.ts
sed -n '1,120p' src/lib/onboard/host-gateway-process.ts

# 3) Find how doctor.ts is currently searching for the gateway process
echo
echo "=== doctor.ts: pgrep occurrences ==="
rg -n "pgrep" src/lib/actions/sandbox/doctor.ts

# 4) Find how 'ss -ltn' is used in the repo (syntax comparison)
echo
echo "=== repo: ss -ltn occurrences (first 80 matches total) ==="
rg -n "ss -ltn" src | head -n 80

Repository: NVIDIA/NemoClaw

Length of output: 7162

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== src/lib/actions/sandbox/doctor.ts lines 140-230 ==="
sed -n '140,230p' src/lib/actions/sandbox/doctor.ts

echo
echo "=== Search for other ss invocations / sport filters ==="
rg -n "ss\\b|sport\\s*=\\s*:\\$\\{?GATEWAY_PORT\\}?|sport\\s*=\\s*:" src | head -n 80

Repository: NVIDIA/NemoClaw

Length of output: 10194

🌐 Web query:

ss -ltn filter syntax "( sport = :PORT )" "sport = :PORT" examples

💡 Result:

The ss command uses a filter expression syntax that supports comparing source (sport) and destination (dport) ports against specific values [1][2]. Because these characters (like parentheses, greater-than/less-than signs, or pipes) are often interpreted by the shell, they must typically be quoted or escaped when used in a command line [3][4]. Filter Syntax The general syntax for port filtering is: {dport|sport} [OP] [FAMILY:]:PORT - [OP]: The operator, such as "=", "<", "<=", "!=", ">=", or ">" [1][2]. If no operator is provided, "=" is assumed [1][2]. - [FAMILY:]: Optional, typically inet or inet6. -:PORT: The port number or name (e.g.,:80 or:http) [1][2]. Key Usage Notes - Shell Escaping: When using operators like ">", "<", or groupings like "", you must wrap the expression in quotes or escape the characters to prevent the shell from interpreting them [3][4]. For example, use '( sport =:80)' or ( sport =:80 ). - Implicit AND: If multiple predicates are provided without a boolean operator, an implicit AND is assumed [1][2]. - Boolean Operators: You can use "or" (|/||), "and" (&/&&), and "not" (!) to combine filters [1][2]. Examples - Filter by a specific port: ss sport =:80 ss 'sport =:80' - Filter with operators (requires quoting/escaping): ss 'sport >:1024' ss 'dport <=:5000' - Complex filters (grouping and boolean logic): ss -ltn '( sport =:80 or sport =:443)' ss -ltn 'state established ( dport =:ssh or sport =:ssh)' ss -ltn 'sport >=:1024 and sport <=:5000' The syntax "( sport =:PORT)" serves to group subexpressions, which is essential when combining multiple conditions with "or" to ensure correct precedence [1][2].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find captureHostCommand implementation
rg -n "function captureHostCommand|captureHostCommand\\(" src/lib/actions/sandbox/doctor.ts

# Print the relevant section (top of file)
sed -n '1,140p' src/lib/actions/sandbox/doctor.ts

# Print captureHostCommand section if it's elsewhere in same file
# (fallback: print around first occurrence line)
line=$(rg -n "function captureHostCommand" src/lib/actions/sandbox/doctor.ts | cut -d: -f1 | head -n1 || true)
if [ -n "${line:-}" ]; then
  start=$((line-20))
  end=$((line+80))
  sed -n "${start},${end}p" src/lib/actions/sandbox/doctor.ts
fi

Repository: NVIDIA/NemoClaw

Length of output: 7856

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show imports at top of doctor.ts to see if it can/does use HOST_GATEWAY_PGREP_PATTERN
sed -n '1,80p' src/lib/actions/sandbox/doctor.ts

# Show any references to host-gateway-process.ts / HOST_GATEWAY_PGREP_PATTERN in doctor.ts
rg -n "host-gateway-process|HOST_GATEWAY_PGREP_PATTERN" src/lib/actions/sandbox/doctor.ts || true

Repository: NVIDIA/NemoClaw

Length of output: 2950

🌐 Web query:

ss filter whitespace "sport = :80" "sport =:80" syntax

💡 Result:

In the ss command filter syntax, both "sport =:80" and "sport =:80" are generally parsed correctly because the filter expression parser is designed to be flexible regarding whitespace around operators [1][2][3]. The underlying grammar for ss filters, implemented in the iproute2 source code (specifically in ssfilter.y), processes tokens like SPORT, operators (e.g., '='), and host conditions (e.g., ':80') as distinct elements regardless of whether they are separated by spaces [3][4]. Key points regarding the syntax: - Operator flexibility: The '=' operator (and its aliases like '==', 'eq') can be used with or without surrounding whitespace [5][3]. - Port notation: The colon prefix (e.g., ':80') is the standard way to specify a port in ss filter expressions [1][6][7]. - Expression grouping: It is best practice to enclose the entire filter expression in single quotes (e.g., 'sport =:80') to prevent the shell from interpreting characters like '>', '<', or '|' as shell operators [1][8][7]. While "sport =:80" works, including a space (i.e., "sport =:80") is the recommended convention for readability and consistency with common documentation examples [1][9][7].

Citations:

Use HOST_GATEWAY_PGREP_PATTERN in dockerInspectGateway’s fallback to avoid false positives.

src/lib/actions/sandbox/doctor.ts currently checks pgrep -af openshell-gateway, which can match any unrelated process whose command line merely contains the substring. Use the anchored HOST_GATEWAY_PGREP_PATTERN from src/lib/onboard/host-gateway-process.ts (via pgrep -f ...) instead. The ss -ltn ( sport = :${GATEWAY_PORT} ) filter is passed as an argv element (no shell parsing) and is consistent with valid ss filter grouping.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/actions/sandbox/doctor.ts` at line 204, The fallback in
dockerInspectGateway is using captureHostCommand("pgrep", ["-af",
"openshell-gateway"], 5000) which can produce false positives; replace the
literal "openshell-gateway" argv with the anchored HOST_GATEWAY_PGREP_PATTERN
imported from src/lib/onboard/host-gateway-process.ts and call
captureHostCommand("pgrep", ["-f", HOST_GATEWAY_PGREP_PATTERN], 5000) so pgrep
runs with the proper anchored pattern; keep the existing timeout and ensure any
variable name references (e.g., processCheck) are preserved.

const portCheck = captureHostCommand("ss", ["-ltn", `( sport = :${GATEWAY_PORT} )`], 5000);
const processRunning = processCheck.status === 0 && processCheck.stdout.trim().length > 0;
const portListening = portCheck.status === 0 && portCheck.stdout.includes(`:${GATEWAY_PORT}`);
if (processRunning && portListening) {
checks.push({
group: "Gateway",
label: "Local gateway process",
status: "ok",
detail: `openshell-gateway is running and listening on port ${GATEWAY_PORT}`,
});
return checks;
}
Comment on lines +205 to +217

checks.push({
group: "Gateway",
label: "Docker container",
Expand Down
2 changes: 2 additions & 0 deletions src/lib/state/sandbox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ export function validateTarEntries(tarBuffer: Buffer, targetDir: string): TarVal
encoding: "utf-8",
stdio: ["pipe", "pipe", "pipe"],
timeout: 60000,
maxBuffer: 256 * 1024 * 1024,
});

if (result.status !== 0) {
Expand Down Expand Up @@ -376,6 +377,7 @@ export function rejectHardLinks(tarBuffer: Buffer): string[] {
encoding: "utf-8",
stdio: ["pipe", "pipe", "pipe"],
timeout: 60000,
maxBuffer: 256 * 1024 * 1024,
});

if (result.status !== 0) {
Expand Down
12 changes: 12 additions & 0 deletions test/security-sandbox-tar-traversal.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,18 @@ describe("Fix: rejectHardLinks blocks hard-link entries at validation time", ()
expect(violations.length).toBe(0);
});

it("accepts a large archive whose verbose listing exceeds Node's default spawn buffer", async () => {
const { rejectHardLinks } = await loadSandboxState();
const entries = Array.from({ length: 20_000 }, (_, index) => ({
path: `workspace/file-${index.toString().padStart(5, "0")}.txt`,
content: "x",
}));

const violations = rejectHardLinks(buildTar(entries));

expect(violations).toEqual([]);
});
Comment on lines +586 to +596

it("safeTarExtract rejects archive containing hard links", async () => {
const { safeTarExtract } = await loadSandboxState();
const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-hardlink-"));
Expand Down
Loading