fix(dns): probe iptables at /sbin, /usr/sbin when not on PATH (#557) by yimoj · Pull Request #1168 · NVIDIA/NemoClaw

yimoj · 2026-03-31T10:33:18Z

Summary

Fixes #557 — DNS resolution fails inside sandboxes because iptables is not on the sandbox image's PATH (/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin), even though it exists at /sbin/iptables.

Problem

The setup-dns-proxy.sh script runs bare iptables via ip netns exec, which searches PATH and fails silently. Without the iptables UDP:53 exception rule, DNS queries from the sandbox never reach the pod-side forwarder, causing EAI_AGAIN for all getaddrinfo calls — including WebSocket connections (Slack Socket Mode, Discord gateway) that bypass the HTTP proxy.

Fix

Probe well-known paths (iptables, /sbin/iptables, /usr/sbin/iptables) to find the binary before using it
Use the discovered path for both rule insertion and runtime verification
Skip the resolv.conf rewrite when no iptables binary is found, preserving fail-fast behavior instead of silently timing out
Warn clearly when iptables is not found at any path

Reproduction

Verified on macOS (Apple Silicon) with OpenShell 0.0.16 + Colima + ghcr.io/nvidia/openshell-community/sandboxes/base:latest:

Before: socket.gaierror: [Errno -3] Temporary failure in name resolution (EAI_AGAIN)
After: All 4 verification checks pass, wss-primary.slack.com resolves successfully

Test plan

3 new tests in test/dns-proxy.test.js covering path probing, discovered binary usage, and missing-iptables warning
Full suite: 683 passed, 3 skipped

Signed-off-by: Yimo Jiang yimoj@nvidia.com

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

Bug Fixes
- Enhanced DNS proxy setup to robustly detect required system utilities in multiple locations before attempting configuration.
- Improved error handling with clearer messaging when DNS setup encounters missing dependencies.
- Added automatic backup and restore mechanism for DNS configuration to prevent data loss during setup failures.
- Fixed DNS rule verification to use dynamically discovered system utilities instead of hardcoded paths.

coderabbitai · 2026-03-31T10:33:33Z

📝 Walkthrough

Walkthrough

The DNS proxy setup now probes for an iptables executable (iptables, /sbin/iptables, /usr/sbin/iptables) before inserting UDP:53 rules or rewriting /etc/resolv.conf; if none is found it warns, restores /etc/resolv.conf from /tmp/resolv.conf.orig, and uses the discovered IPTABLES_BIN for runtime checks. Tests updated accordingly.

Changes

Cohort / File(s)	Summary
DNS Proxy Setup Script `scripts/setup-dns-proxy.sh`	Probe for an `iptables` binary at `iptables`, `/sbin/iptables`, `/usr/sbin/iptables` and set `IPTABLES_BIN`; only run `"$IPTABLES_BIN" -C`/`-I` and rewrite `/etc/resolv.conf` when usable; create one-time backup `/tmp/resolv.conf.orig`; warn and restore when iptables missing; use `IPTABLES_BIN` for runtime verification; update final warning to reference `#626` and `#557`.
DNS Proxy Tests `test/dns-proxy.test.js`	Adjust assertions to match `-C OUTPUT` substring; add assertions for probing fallback `iptables` paths and recorded `IPTABLES_BIN`; assert use of `"$IPTABLES_BIN"` for `-C`/`-I` operations; assert failure messages when iptables absent and backup/restore of `/etc/resolv.conf` via `/tmp/resolv.conf.orig`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 I hopped through /sbin and /usr/sbin light,
sniffed for iptables morning and night.
If none I find, I warn and restore,
keep resolv safe in /tmp once more.
A tidy DNS, I thump with delight. 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: probing iptables at standard paths when not on PATH to fix DNS issues.
Linked Issues check	✅ Passed	The PR fully addresses issue `#557` by probing iptables at /sbin and /usr/sbin, using discovered paths for rule insertion, and providing clear warnings when iptables is unavailable.
Out of Scope Changes check	✅ Passed	All changes focus on DNS setup and iptables path resolution; no unrelated modifications were introduced beyond the stated objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

test/dns-proxy.test.js (1)
84-114: Add one behavioral test for the missing-iptables branch.

These cases still only readFileSync() the script and assert substrings, so a broken probe command or a bad no-iptables control path would still pass. Please add at least one spawnSync()-style test with stubbed docker/kubectl wrappers that actually runs scripts/setup-dns-proxy.sh and asserts the warning plus the no-/etc/resolv.conf-rewrite behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/dns-proxy.test.js` around lines 84 - 114, The current tests only inspect
the script text; add a behavioral test in test/dns-proxy.test.js that actually
executes scripts/setup-dns-proxy.sh via child_process.spawnSync (or a test
helper that wraps spawnSync) with environment stubs for docker/kubectl so the
probe path returns "not found", then assert the subprocess stdout/stderr
contains the iptables-missing warning ("iptables not found in pod" / "Cannot add
UDP DNS exception") and verify that the script did not attempt to rewrite
/etc/resolv.conf (e.g., by checking no writes or that the script exits before
writing that file). Stub the docker/kubectl wrappers used by the script (mock
the commands on PATH or set PATH to point to test shims) so the iptables probe
branch is exercised rather than just reading SETUP_DNS_PROXY content.

🤖 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/setup-dns-proxy.sh`:
- Around line 200-213: Persist the sandbox's original /etc/resolv.conf before
the first rewrite (e.g., when you run the kctl exec ... sh -c "printf
'nameserver ${VETH_GW}...' > /etc/resolv.conf" in the Step 5 block) by saving
its contents to a safe file in the sandbox namespace (use SANDBOX_NS and POD
context, e.g., /tmp/original_resolv.conf) and then, in the else branch where
iptables is not found, restore that saved file back to /etc/resolv.conf via kctl
exec (using the same POD and SANDBOX_NS) so the original resolver is
re-established when the UDP:53 iptables exception cannot be installed; ensure
the save happens only once (detect existing backup before overwriting) and use
the same kctl/ip netns invocation pattern as the existing rewrite to locate the
correct namespace and file.

---

Nitpick comments:
In `@test/dns-proxy.test.js`:
- Around line 84-114: The current tests only inspect the script text; add a
behavioral test in test/dns-proxy.test.js that actually executes
scripts/setup-dns-proxy.sh via child_process.spawnSync (or a test helper that
wraps spawnSync) with environment stubs for docker/kubectl so the probe path
returns "not found", then assert the subprocess stdout/stderr contains the
iptables-missing warning ("iptables not found in pod" / "Cannot add UDP DNS
exception") and verify that the script did not attempt to rewrite
/etc/resolv.conf (e.g., by checking no writes or that the script exits before
writing that file). Stub the docker/kubectl wrappers used by the script (mock
the commands on PATH or set PATH to point to test shims) so the iptables probe
branch is exercised rather than just reading SETUP_DNS_PROXY content.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: baaa7c8f-afa3-4af6-85bc-d5b2d6b721cf

📥 Commits

Reviewing files that changed from the base of the PR and between 36f4456 and da482b1.

📒 Files selected for processing (2)

scripts/setup-dns-proxy.sh
test/dns-proxy.test.js

coderabbitai

🧹 Nitpick comments (1)

scripts/setup-dns-proxy.sh (1)
185-190: Consider simplifying the absolute-path probes.

The current logic using command -v with a fallback works correctly, but for absolute paths like /sbin/iptables, a direct test -x is clearer since the path is already fully qualified.
♻️ Suggested simplification
   IPTABLES_BIN=""
   for candidate in iptables /sbin/iptables /usr/sbin/iptables; do
-    if kctl exec -n openshell "$POD" -- sh -c "test -x \"\$(command -v $candidate 2>/dev/null || echo $candidate)\"" 2>/dev/null; then
+    if kctl exec -n openshell "$POD" -- sh -c "command -v $candidate >/dev/null 2>&1 || test -x $candidate" 2>/dev/null; then
       IPTABLES_BIN="$candidate"
       break
     fi
   done
This separates concerns: command -v for PATH lookup (bare iptables), test -x for absolute paths — making the intent clearer while preserving the same behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/setup-dns-proxy.sh` around lines 185 - 190, Loop that sets
IPTABLES_BIN currently calls command -v even for absolute paths; change the
probe to call command -v only for bare candidates (e.g., "iptables") and use a
direct test -x for absolute candidates (candidates starting with "/") inside the
kctl exec check, preserving the kctl exec -n openshell "$POD" -- sh -c wrapper
and the assignment to IPTABLES_BIN when the check succeeds.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@scripts/setup-dns-proxy.sh`:
- Around line 185-190: Loop that sets IPTABLES_BIN currently calls command -v
even for absolute paths; change the probe to call command -v only for bare
candidates (e.g., "iptables") and use a direct test -x for absolute candidates
(candidates starting with "/") inside the kctl exec check, preserving the kctl
exec -n openshell "$POD" -- sh -c wrapper and the assignment to IPTABLES_BIN
when the check succeeds.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5c80259d-2143-4d28-86df-ce69380c7323

📥 Commits

Reviewing files that changed from the base of the PR and between da482b1 and cea448f.

📒 Files selected for processing (2)

scripts/setup-dns-proxy.sh
test/dns-proxy.test.js

🚧 Files skipped from review as they are similar to previous changes (1)

test/dns-proxy.test.js

…#557) DNS setup failed silently on sandbox images where iptables was installed at /sbin/iptables but not on PATH. The script now probes well-known locations before giving up, and skips the resolv.conf rewrite when no iptables binary is found to preserve fail-fast behavior.

yimoj · 2026-04-07T04:33:14Z

Last force push was to fix the commit signature

coderabbitai

🧹 Nitpick comments (1)

scripts/setup-dns-proxy.sh (1)

184-190: The probing logic is sound but consider a simpler check.

The current approach works correctly—command -v returns the executable path for both bare commands and absolute paths when they exist. However, for absolute path candidates, a direct test -x "$candidate" would be more straightforward:

♻️ Optional: Simplified probing logic

   IPTABLES_BIN=""
   for candidate in iptables /sbin/iptables /usr/sbin/iptables; do
-    if kctl exec -n openshell "$POD" -- sh -c "test -x \"\$(command -v $candidate 2>/dev/null || echo $candidate)\"" 2>/dev/null; then
+    if kctl exec -n openshell "$POD" -- sh -c "command -v $candidate >/dev/null 2>&1 && test -x \"\$(command -v $candidate)\"" 2>/dev/null; then
       IPTABLES_BIN="$candidate"
       break
     fi
   done

Either form is correct; the current version is functional.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/setup-dns-proxy.sh` around lines 184 - 190, The probing loop for
IPTABLES_BIN can be simplified: in the for-loop over candidate and variable
IPTABLES_BIN, change the kctl exec test so that if candidate is an absolute path
(starts with '/'), run remote 'test -x "$candidate"', otherwise run 'command -v
$candidate' (or 'test -x "$(command -v $candidate)"') via kctl exec -n openshell
"$POD" -- sh -c; set IPTABLES_BIN="$candidate" and break on success. Use the
existing symbols IPTABLES_BIN, candidate, kctl exec, and POD to locate and
update the check accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@scripts/setup-dns-proxy.sh`:
- Around line 184-190: The probing loop for IPTABLES_BIN can be simplified: in
the for-loop over candidate and variable IPTABLES_BIN, change the kctl exec test
so that if candidate is an absolute path (starts with '/'), run remote 'test -x
"$candidate"', otherwise run 'command -v $candidate' (or 'test -x "$(command -v
$candidate)"') via kctl exec -n openshell "$POD" -- sh -c; set
IPTABLES_BIN="$candidate" and break on success. Use the existing symbols
IPTABLES_BIN, candidate, kctl exec, and POD to locate and update the check
accordingly.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 938ff663-bcdc-4f47-8d20-7dbef1bddbdf

📥 Commits

Reviewing files that changed from the base of the PR and between a4a2d9c and 99305f9.

📒 Files selected for processing (2)

scripts/setup-dns-proxy.sh
test/dns-proxy.test.js

…#557) (NVIDIA#1168) ## Summary Fixes NVIDIA#557 — DNS resolution fails inside sandboxes because `iptables` is not on the sandbox image's `PATH` (`/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin`), even though it exists at `/sbin/iptables`. ## Problem The `setup-dns-proxy.sh` script runs bare `iptables` via `ip netns exec`, which searches PATH and fails silently. Without the iptables UDP:53 exception rule, DNS queries from the sandbox never reach the pod-side forwarder, causing `EAI_AGAIN` for all `getaddrinfo` calls — including WebSocket connections (Slack Socket Mode, Discord gateway) that bypass the HTTP proxy. ## Fix - Probe well-known paths (`iptables`, `/sbin/iptables`, `/usr/sbin/iptables`) to find the binary before using it - Use the discovered path for both rule insertion and runtime verification - Skip the `resolv.conf` rewrite when no iptables binary is found, preserving fail-fast behavior instead of silently timing out - Warn clearly when iptables is not found at any path ## Reproduction Verified on macOS (Apple Silicon) with OpenShell 0.0.16 + Colima + `ghcr.io/nvidia/openshell-community/sandboxes/base:latest`: **Before:** `socket.gaierror: [Errno -3] Temporary failure in name resolution` (EAI_AGAIN) **After:** All 4 verification checks pass, `wss-primary.slack.com` resolves successfully ## Test plan - 3 new tests in `test/dns-proxy.test.js` covering path probing, discovered binary usage, and missing-iptables warning - Full suite: 683 passed, 3 skipped Signed-off-by: Yimo Jiang <yimoj@nvidia.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code)  ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Enhanced DNS proxy setup to robustly detect required system utilities in multiple locations before attempting configuration. * Improved error handling with clearer messaging when DNS setup encounters missing dependencies. * Added automatic backup and restore mechanism for DNS configuration to prevent data loss during setup failures. * Fixed DNS rule verification to use dynamically discovered system utilities instead of hardcoded paths.

coderabbitai bot reviewed Mar 31, 2026

View reviewed changes

Comment thread scripts/setup-dns-proxy.sh

yimoj force-pushed the fix/dns-proxy-iptables-path-557 branch from da482b1 to cea448f Compare March 31, 2026 11:22

coderabbitai bot reviewed Mar 31, 2026

View reviewed changes

wscurran mentioned this pull request Apr 1, 2026

installation fails with default nodejs package path #569

Closed

2 tasks

cv approved these changes Apr 7, 2026

View reviewed changes

yimoj force-pushed the fix/dns-proxy-iptables-path-557 branch from a4a2d9c to 99305f9 Compare April 7, 2026 04:32

coderabbitai bot reviewed Apr 7, 2026

View reviewed changes

cv merged commit f07c788 into NVIDIA:main Apr 7, 2026
8 checks passed

yimoj deleted the fix/dns-proxy-iptables-path-557 branch April 17, 2026 05:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(dns): probe iptables at /sbin, /usr/sbin when not on PATH (#557)#1168

fix(dns): probe iptables at /sbin, /usr/sbin when not on PATH (#557)#1168
cv merged 1 commit intoNVIDIA:mainfrom
yimoj:fix/dns-proxy-iptables-path-557

yimoj commented Mar 31, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 31, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

yimoj commented Apr 7, 2026

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

yimoj commented Mar 31, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Fix

Reproduction

Test plan

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

yimoj commented Apr 7, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yimoj commented Mar 31, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 31, 2026 •

edited

Loading