Skip to content

fix(cli): include IPv6 loopback in NO_PROXY - #3486

Merged
cv merged 1 commit into
NVIDIA:mainfrom
1PoPTRoN:fix/no-proxy-ipv6-loopback
May 16, 2026
Merged

fix(cli): include IPv6 loopback in NO_PROXY#3486
cv merged 1 commit into
NVIDIA:mainfrom
1PoPTRoN:fix/no-proxy-ipv6-loopback

Conversation

@1PoPTRoN

@1PoPTRoN 1PoPTRoN commented May 13, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR updates NemoClaw’s subprocess environment handling so local traffic is not accidentally routed through a forwarded proxy when tools target IPv6 loopback. The existing helper already protected localhost and 127.0.0.1; this extends the same behavior to ::1 and 0.0.0.0 in both mirrored subprocess environment helpers.

Related Issue

Fixes #3485

Changes

  • Added ::1 and 0.0.0.0 to the local host list injected into NO_PROXY and no_proxy.
  • Kept the CLI and plugin copies of subprocess-env.ts in sync.
  • Updated subprocess environment tests to cover the expanded local bypass list.
  • Added a parity regression test so the CLI and plugin helpers do not drift on this behavior again.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • make docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Focused verification run:

npm run build:cli
npx vitest run --project cli src/lib/subprocess-env.test.ts test/credential-exposure.test.ts
npm run typecheck:cli
cd nemoclaw && npm run build

The focused regression tests passed locally:

image(1)

Summary by CodeRabbit

  • Bug Fixes

    • Prevent local traffic from being routed through forwarded HTTP proxies by adding additional loopback/local addresses (::1, 0.0.0.0) to NO_PROXY/no_proxy when proxy variables are present.
  • Tests

    • Expanded tests to verify the broader local-address exclusion, augmentation/no-duplication behavior, and consistent results across implementations.
  • Documentation

    • Clarified wording to describe "local traffic" proxy exclusion.

Review Change Stack


Signed-off-by: 1PoPTRoN vrxn.arp1traj@gmail.com

Copilot AI review requested due to automatic review settings May 13, 2026 22:49
@copy-pr-bot

copy-pr-bot Bot commented May 13, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR extends withLocalNoProxy() to inject IPv6 loopback (::1) and wildcard (0.0.0.0) addresses alongside localhost and 127.0.0.1 into NO_PROXY/no_proxy when any HTTP proxy variables are forwarded; mirrored implementations and tests were updated and a cross-implementation consistency test was added.

Changes

Local proxy bypass expansion

Layer / File(s) Summary
Host list expansion and documentation
nemoclaw/src/lib/subprocess-env.ts, src/lib/subprocess-env.ts
withLocalNoProxy() now injects localhost,127.0.0.1,::1,0.0.0.0 when any HTTP proxy variable is present. Documentation updated to describe preventing local traffic routing through forwarded proxies.
Test expectations for expanded host list
src/lib/subprocess-env.test.ts
All withLocalNoProxy() and buildSubprocessEnv() test cases updated to assert the expanded local host allowlist (localhost,127.0.0.1,::1,0.0.0.0) in both NO_PROXY and no_proxy, including injection, augmentation, deduplication, and environment override scenarios.
Cross-implementation consistency verification
test/credential-exposure.test.ts
Added test comparing withLocalNoProxy() behavior between CLI and plugin implementations, verifying both produce identical expanded NO_PROXY and no_proxy values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Four little addresses, now reunited,
localhost, 127.0.0.1, ::1, and 0.0.0.0 invited,
No proxy shall intercept their local call,
IPv6 loopback gets love from all.
Tunnels flow true, no detours today!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(cli): include IPv6 loopback in NO_PROXY' directly describes the main change: adding IPv6 loopback (::1) to the NO_PROXY environment variable handling.
Linked Issues check ✅ Passed The PR fully addresses issue #3485 by expanding NO_PROXY/no_proxy to include ::1 and 0.0.0.0 in both mirrored subprocess-env helpers and adding comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes are scoped to the linked issue #3485: updating withLocalNoProxy() implementation in both helpers, updating corresponding tests, and adding a parity regression test.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

Copilot AI left a comment

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.

Pull request overview

This PR updates NemoClaw’s subprocess environment construction to ensure local traffic is not accidentally routed through forwarded HTTP(S) proxies when tools target IPv6 loopback or wildcard bind addresses.

Changes:

  • Extended withLocalNoProxy() to append ::1 and 0.0.0.0 (in addition to localhost and 127.0.0.1) to both NO_PROXY and no_proxy when a proxy is present.
  • Kept the mirrored CLI and plugin subprocess-env.ts implementations in sync.
  • Updated and added tests to cover the expanded bypass list and enforce CLI/plugin parity.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/lib/subprocess-env.ts Expands local-host entries injected into NO_PROXY/no_proxy when proxy vars are present.
nemoclaw/src/lib/subprocess-env.ts Mirrors the same withLocalNoProxy() behavior for the plugin package.
src/lib/subprocess-env.test.ts Updates expectations to include ::1 and 0.0.0.0 for both direct helper and builder injection tests.
test/credential-exposure.test.ts Adds a regression test asserting CLI/plugin withLocalNoProxy() produce identical NO_PROXY/no_proxy results.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@1PoPTRoN
1PoPTRoN force-pushed the fix/no-proxy-ipv6-loopback branch from d6261e0 to d74cf48 Compare May 13, 2026 23:26
@jyaunches
jyaunches self-requested a review May 14, 2026 01:10
@1PoPTRoN
1PoPTRoN force-pushed the fix/no-proxy-ipv6-loopback branch from bd40181 to 9c77fbd Compare May 14, 2026 01:12
@wscurran

Copy link
Copy Markdown
Contributor

✨ Thanks for submitting this detailed PR to fix the subprocess environment handling issue with IPv6 loopback in NO_PROXY. This change aims to improve the reliability of the NemoClaw CLI by ensuring local traffic is not accidentally routed through a forwarded proxy when tools target IPv6 loopback.


Related open issues:

@1PoPTRoN
1PoPTRoN force-pushed the fix/no-proxy-ipv6-loopback branch 2 times, most recently from 5e204b4 to 6cb54d2 Compare May 15, 2026 11:31
@1PoPTRoN

Copy link
Copy Markdown
Contributor Author

@ericksoa @jyaunches Would love i review on it!

@1PoPTRoN
1PoPTRoN force-pushed the fix/no-proxy-ipv6-loopback branch from 6cb54d2 to 042ea4a Compare May 16, 2026 01:45
@1PoPTRoN

Copy link
Copy Markdown
Contributor Author

@cv need a review!🙌🏽

@cv cv added the v0.0.45 label May 16, 2026
Signed-off-by: 1PoPTRoN <vrxn.arp1traj@gmail.com>
@1PoPTRoN
1PoPTRoN force-pushed the fix/no-proxy-ipv6-loopback branch from 042ea4a to a33e69c Compare May 16, 2026 14:06
@cv
cv self-requested a review May 16, 2026 17:23
@cv

cv commented May 16, 2026

Copy link
Copy Markdown
Collaborator

Thanks, this looks good to me.

On the added 0.0.0.0 entry: I think it is defensible here, but it is worth calling out why it is in the bypass list. 0.0.0.0 is not loopback in the strict networking sense; it is the IPv4 unspecified address, most commonly used by servers to bind all interfaces. In practice, though, developer tools often print or pass around URLs like http://0.0.0.0:PORT for a locally bound service. If a subprocess inherits HTTP_PROXY/HTTPS_PROXY and NO_PROXY does not include that literal, those local-dev requests can be sent to the forwarded proxy and fail confusingly.

The tradeoff is that this slightly expands the proxy-bypass surface: requests to the literal 0.0.0.0 will now connect directly rather than being mediated by the proxy. That is a pragmatic compatibility choice rather than a semantic claim that 0.0.0.0 is loopback. Given this helper already bypasses local host-bound targets such as localhost, 127.0.0.1, and host.docker.internal, the marginal risk seems acceptable for the developer-experience fix here.

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved. The change is small, mirrored between CLI/plugin helpers, covered by focused tests plus a parity regression test, and CI is green.

@cv
cv enabled auto-merge (squash) May 16, 2026 18:14
@cv
cv merged commit b67a8e4 into NVIDIA:main May 16, 2026
18 checks passed
@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression and removed NemoClaw CLI labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

withLocalNoProxy() misses ::1 and 0.0.0.0 in NO_PROXY injection

4 participants