fix(security): close SSRF bypass via IPv6, CGNAT, and IPv4-mapped ranges in config set

[Siddhartha-singh01]

Summary

Closes the SSRF validation gap in nemoclaw config set by replacing the incomplete prefix-based isPrivateIp() in sandbox-config.ts with a comprehensive CIDR-based implementation that matches the coverage of the canonical ssrf.ts in the plugin.

Related Issue

Changes

• Replaced the 7-line string-prefix isPrivateIp() with a proper CIDR-based implementation using node:net for IPv4/IPv6 detection
• Added coverage for RFC 6598 CGNAT (100.64.0.0/10) — previously allowed through
• Added coverage for RFC 2544 benchmark testing (198.18.0.0/15) — previously allowed through
• Added full IPv6 private range coverage: unique-local (fc00::/7), link-local (fe80::/10), multicast (ff00::/8)
• Added IPv4-mapped IPv6 detection (::ffff:x.x.x.x) — extracts embedded IPv4 and checks against IPv4 ranges
• Fixed IPv6 bracket stripping — URL.hostname wraps IPv6 in [], now properly stripped before matching
• Added 13 new test cases to test/config-set.test.ts covering all previously missing ranges plus boundary/edge cases
• All 54 existing + new tests pass (43 config-set + 11 config-rotate-token)

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)

---

Signed-off-by: Siddhartha Singh siddharthagithub0007@gmail.com

Summary by CodeRabbit

• Bug Fixes
  • Enhanced URL validation to more accurately detect and prevent connections to private and internal IP addresses through comprehensive RFC-compliant IPv4 and IPv6 range matching. Now properly handles loopback, link-local, unique-local, CGNAT, and other reserved address ranges to strengthen security and prevent accidental internal connections.

[copy-pr-bot]

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]

📝 Walkthrough

Walkthrough

The URL validation logic for detecting private/internal hosts is enhanced from simple prefix matching to comprehensive RFC-based IP address range validation. The implementation now parses IPv4 and IPv6 addresses into byte arrays, handles IPv6 compression and IPv4-mapped formats, and performs CIDR bitmask comparisons against an expanded set of reserved and private network ranges.

Changes

Cohort / File(s)	Summary
IP Address Range Validation src/lib/sandbox-config.ts	Replaces hostname prefix checking with RFC-compliant private/internal IP detection. Adds node:net helpers, IPv4/IPv6 parsing logic, support for :: compression and IPv4-mapped IPv6 (::ffff:...), and a comprehensive PRIVATE_NETWORKS list covering loopback, private, link-local, CGNAT, and benchmark ranges. Uses bitmask comparisons for CIDR range checking.
Test Coverage Expansion test/config-set.test.ts	Extends validateUrlValue test assertions to cover IPv4 CGNAT (100.64.0.0/10), IPv6 link-local/unique-local/multicast addresses, IPv4-mapped IPv6 variants, public IPv6, and boundary cases. Confirms rejection of private ranges and acceptance of public addresses.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 Hoppy hops through ranges wide,
RFC-blessed, nothing hides,
IPv6 gardens, IPv4 fields,
CIDR masks reveal what's sealed!
Private networks take their bow,
Validation reigns supreme right now! 🌐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main security fix: closing SSRF vulnerabilities by expanding private IP detection to include IPv6 ranges, CGNAT addresses, and IPv4-mapped formats in the config set validation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ 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)

src/lib/sandbox-config.ts (1)

223-226: Extract this classifier instead of copying it.

This reintroduces a second copy of the IP parser/CIDR table even though nemoclaw/src/blueprint/ssrf.ts:104-119 is called out as canonical. Given this PR is fixing drift between the two paths, sharing one helper would make the next range update much less likely to miss one side.

Also applies to: 287-357

🤖 Prompt for AI Agents

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

In `@src/lib/sandbox-config.ts` around lines 223 - 226, Duplicate IP classifier
logic should be extracted into a single shared helper (e.g., export a function
like isPrivateOrReservedIp and the CIDR table constant PRIVATE_RESERVED_CIDRS)
and the copies in sandbox-config (the block around lines 223-226 and 287-357)
replaced with imports from that new module; update both callers (including the
canonical consumer in nemoclaw/src/blueprint/ssrf.ts) to call the shared
isPrivateOrReservedIp so future CIDR range updates are made in one place.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@src/lib/sandbox-config.ts`:
- Around line 338-341: The current check only allows the literal "localhost" and
misses special-use names like "localhost." and "*.localhost", so update the
early-return logic (the block that checks hostname and the derived addr
variable) to treat any loopback name that is exactly "localhost", "localhost."
or any subdomain ending with ".localhost" as local — i.e., perform the check
after stripping IPv6 brackets on addr and return true when addr equals
"localhost" (with or without trailing dot) or when addr ends with ".localhost".

---

Nitpick comments:
In `@src/lib/sandbox-config.ts`:
- Around line 223-226: Duplicate IP classifier logic should be extracted into a
single shared helper (e.g., export a function like isPrivateOrReservedIp and the
CIDR table constant PRIVATE_RESERVED_CIDRS) and the copies in sandbox-config
(the block around lines 223-226 and 287-357) replaced with imports from that new
module; update both callers (including the canonical consumer in
nemoclaw/src/blueprint/ssrf.ts) to call the shared isPrivateOrReservedIp so
future CIDR range updates are made in one place.

🪄 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 Plus

Run ID: d4851812-ab36-48a0-a2a8-0960112a88ce

📥 Commits

Reviewing files that changed from the base of the PR and between 59d3115 and d02abf9.

📒 Files selected for processing (2)

• src/lib/sandbox-config.ts
• test/config-set.test.ts

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Block .localhost names, not just literal localhost.

localhost. and any *.localhost hostname are special-use loopback names. The exact-string check here leaves an SSRF bypass even after the CIDR expansion.

🔒 Suggested fix

 function isPrivateIp(hostname: string): boolean {
-  if (hostname === "localhost") return true;
-
-  // URL.hostname wraps IPv6 in brackets — strip them for matching
-  const addr = hostname.replace(/^\[|\]$/g, "");
+  // URL.hostname may include IPv6 brackets and a trailing dot on FQDNs.
+  const addr = hostname.replace(/^\[|\]$/g, "").replace(/\.$/, "").toLowerCase();
+  if (addr === "localhost" || addr.endsWith(".localhost")) return true;

📝 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

	if (hostname === "localhost") return true;
	// URL.hostname wraps IPv6 in brackets — strip them for matching
	const addr = hostname.replace(/^\[|\]$/g, "");
	// URL.hostname may include IPv6 brackets and a trailing dot on FQDNs.
	const addr = hostname.replace(/^\[|\]$/g, "").replace(/\.$/, "").toLowerCase();
	if (addr === "localhost" || addr.endsWith(".localhost")) return true;

🤖 Prompt for AI Agents

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

In `@src/lib/sandbox-config.ts` around lines 338 - 341, The current check only
allows the literal "localhost" and misses special-use names like "localhost."
and "*.localhost", so update the early-return logic (the block that checks
hostname and the derived addr variable) to treat any loopback name that is
exactly "localhost", "localhost." or any subdomain ending with ".localhost" as
local — i.e., perform the check after stripping IPv6 brackets on addr and return
true when addr equals "localhost" (with or without trailing dot) or when addr
ends with ".localhost".

[wscurran]

✨ Thanks for submitting this issue that identifies a bug with the SSRF validation and proposes a fix.

[wscurran]

✨ Thanks for submitting this issue that identifies a bug with the SSRF validation and proposes a fix.