fix(security): validate Hermes endpoint URL and reject allowed_ips in user presets - #6087
Merged
Conversation
… user presets Fixes two SSRF gaps identified in issues #6072 and #6073. 1. src/lib/onboard/inference-providers/hermes.ts: call isPrivateHostname() on the user-supplied endpointUrl before passing it downstream as OPENAI_BASE_URL. Previously the URL bypassed all SSRF checks until plugin-side validateEndpointUrl() fired at inference time — too late to prevent credential dispatch to an internal host. 2. src/lib/policy/index.ts: reject user-supplied preset files that declare allowed_ips in any network_policies endpoint. The merge was previously a blind structural pass-through that let callers expand the private-IP allowlist OpenShell enforces. Fixes #6072 Fixes #6073 Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codebase growth guardrail bans if statements in test files. Replace requireValue mock body with a single expression. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds endpoint URL validation for Hermes onboarding and rejects user-supplied policy presets that include ChangesHermes inference endpoint SSRF validation
Policy preset allowed_ips rejection
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
10 tasks
prekshivyas
added
security
area: security
Contributor
PR Review Advisor (Nemotron Ultra) — InformationalMerge posture: Informational / low confidence Action checklist
Findings index
Review findings by urgency: 0 required fixes, 1 item to resolve/justify, 0 in-scope improvements
|
Contributor
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/onboard/inference-providers/hermes.ts (1)
8-8: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider restricting
endpointUrltohttp/httpsschemes.
isPrivateHostnameonly inspectsparsedEndpoint.hostname. A scheme likefile://,unix://, or a custom scheme parses fine withnew URL()but yields an empty/irrelevant hostname, sidestepping the private-hostname check entirely. Given this value is later forwarded asbaseUrl/OPENAI_BASE_URL(Line 105, Line 110), an explicit protocol allowlist closes an easy bypass and is a natural complement to the hostname check being added here.🛡️ Proposed protocol check
try { parsedEndpoint = new URL(endpointUrl); } catch { throw new Error(`Invalid inference endpoint URL: ${endpointUrl}`); } + if (parsedEndpoint.protocol !== "http:" && parsedEndpoint.protocol !== "https:") { + throw new Error(`Inference endpoint URL must use http or https: ${endpointUrl}`); + } if (isPrivateHostname(parsedEndpoint.hostname)) {Also applies to: 32-44
🤖 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/onboard/inference-providers/hermes.ts` at line 8, Add an explicit protocol allowlist for endpointUrl in the Hermes inference provider flow, since isPrivateHostname only checks parsedEndpoint.hostname and can be bypassed by non-http schemes. Update the validation around the endpoint parsing logic in hermes.ts, using the existing Hermes setup paths that later pass endpointUrl into baseUrl/OPENAI_BASE_URL, so only http and https URLs are accepted before the hostname/private-network check runs.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/lib/onboard/inference-providers/hermes.test.ts`:
- Around line 9-21: The new `isPrivateHostname` mock in `hermes.test.ts` trips
the guardrail by adding two `if` statements for early returns. Refactor the
helper into a single boolean return expression that combines the private host
set, the `.internal`/`.local` suffix checks, and the `privatePatterns.some(...)`
match, preserving the same behavior while removing the explicit `if` branches.
---
Nitpick comments:
In `@src/lib/onboard/inference-providers/hermes.ts`:
- Line 8: Add an explicit protocol allowlist for endpointUrl in the Hermes
inference provider flow, since isPrivateHostname only checks
parsedEndpoint.hostname and can be bypassed by non-http schemes. Update the
validation around the endpoint parsing logic in hermes.ts, using the existing
Hermes setup paths that later pass endpointUrl into baseUrl/OPENAI_BASE_URL, so
only http and https URLs are accepted before the hostname/private-network check
runs.
🪄 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: Enterprise
Run ID: a348c8e4-95a5-42b1-883c-663a2a6dce6a
📒 Files selected for processing (4)
src/lib/onboard/inference-providers/hermes.test.tssrc/lib/onboard/inference-providers/hermes.tssrc/lib/policy/index.tssrc/lib/policy/preset-allowed-ips.test.ts
Drop the vi.mock factory — it contained if statements which the codebase-growth guardrail forbids in test files. The real implementation reads from nemoclaw-blueprint/private-networks.yaml and works correctly in test context. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace 'in' operator with Object.hasOwn to avoid matching inherited prototype properties (prototype pollution guard, per PRA-2). Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
Author
|
Addressing Nemotron advisor items: PRA-2 — Fixed. Replaced PRA-6 — Explained. #6085 was the original PR opened on the wrong base branch (carried unrelated #6047 commits in the diff). #6087 is the clean replacement rebased onto main. #6085 is closed. PRA-5 / PRA-9 — Justified. PRA-1 — Justified. PRA-3 / PRA-4 — Justified. The public-endpoint and null-endpointUrl tests verify that valid input doesn't throw and that downstream ( PRA-7 — Justified. Prototype pollution against PRA-8 — Justified. The 5 rejection tests cover the representative SSRF classes (loopback, link-local/metadata, RFC-1918, reserved hostname, |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tent bypass - hermes.ts: reject non-http/https schemes and URLs with embedded credentials; redact raw URL from parse-error messages - policy/index.ts: extract networkPoliciesHasAllowedIps helper and enforce it in applyPresetContent() when options.custom is set, closing the snapshot-replay bypass path (PRA-3 partial, PRA-4) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard/inference-providers/hermes.test.ts (1)
127-142: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTest title claims no raw-value leak but assertion doesn't verify it.
The test name says "without leaking the raw value" but only asserts
.rejects.toThrow(/valid URL/). This regex would still pass even if the implementation's error message included the raw"not-a-url"string (e.g.,"'not-a-url' is not a valid URL."), so the test doesn't actually exercise the no-leak claim it asserts in its title.As per path instructions for
**/*.test.{ts,js,mts,mjs,cts,cjs}: "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."✅ Suggested fix to actually assert no leak
- it("throws on malformed URL without leaking the raw value", async () => { - await expect( - setupHermesProviderInference( - { - sandboxName: "alpha", - model: "m", - provider: "p", - endpointUrl: "not-a-url", - credentialEnv: null, - hermesAuthMethod: null, - hermesToolGateways: [], - }, - makeDeps() as never, - ), - ).rejects.toThrow(/valid URL/); - }); + it("throws on malformed URL without leaking the raw value", async () => { + let caught: unknown; + try { + await setupHermesProviderInference( + { + sandboxName: "alpha", + model: "m", + provider: "p", + endpointUrl: "not-a-url", + credentialEnv: null, + hermesAuthMethod: null, + hermesToolGateways: [], + }, + makeDeps() as never, + ); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(Error); + expect((caught as Error).message).toMatch(/valid URL/); + expect((caught as Error).message).not.toContain("not-a-url"); + });🤖 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/onboard/inference-providers/hermes.test.ts` around lines 127 - 142, The hermes test title promises the malformed endpoint URL is not leaked, but the current assertion in setupHermesProviderInference only matches “valid URL” and would still pass if the raw value appears in the error. Update the test in hermes.test.ts so the rejection assertion also verifies the message does not contain the input endpointUrl (for example by checking the thrown error text via the setupHermesProviderInference call and asserting it excludes the raw “not-a-url” value), while still keeping the existing valid-URL expectation.Source: Path instructions
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@src/lib/onboard/inference-providers/hermes.test.ts`:
- Around line 127-142: The hermes test title promises the malformed endpoint URL
is not leaked, but the current assertion in setupHermesProviderInference only
matches “valid URL” and would still pass if the raw value appears in the error.
Update the test in hermes.test.ts so the rejection assertion also verifies the
message does not contain the input endpointUrl (for example by checking the
thrown error text via the setupHermesProviderInference call and asserting it
excludes the raw “not-a-url” value), while still keeping the existing valid-URL
expectation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3a438659-c549-4b0d-9a8f-0d2cf091d141
📒 Files selected for processing (4)
src/lib/onboard/inference-providers/hermes.test.tssrc/lib/onboard/inference-providers/hermes.tssrc/lib/policy/index.tssrc/lib/policy/preset-allowed-ips.test.ts
8 tasks
apurvvkumaria
approved these changes
Jul 1, 2026
… allowed_ips bypass
Address the required PR-advisor findings on the SSRF hardening:
- PRA-4/PRA-8 (DNS rebinding): the Hermes endpoint check used the string-only
isPrivateHostname, so a public hostname resolving to a private IP bypassed it.
Route the URL through rewriteConfigUrlsWithDnsPinning (the same DNS-resolving
validator the compatible-endpoint path uses): it rejects private-resolved
addresses, pins the IP for http (closing the config-time->runtime rebinding
window), and preserves the hostname for https (keeping TLS cert validation).
The scheme / embedded-credentials / valid-URL pre-checks are retained.
lookup is injectable via HermesDeps for deterministic tests.
- PRA-13: document why a null endpointUrl is intentionally accepted (managed/
OAuth path supplies the route later).
- PRA-5/PRA-11: networkPoliciesHasAllowedIps now also rejects allowed_ips
declared at the network-policy object level (not only inside endpoints), and
uses `in` rather than Object.hasOwn so an inherited/prototype-chain allowed_ips
cannot bypass the guard.
Tests: DNS-rebinding rejection (public host -> private IP), public-host accept,
resolves.toEqual({ ok: true }) on the public/null paths (PRA-9/PRA-10), a
parametrized sweep over private/reserved literals+names (PRA-7), and
object-level + prototype-chain allowed_ips rejection (PRA-11). The five original
literal/reserved rejection tests are unchanged (they short-circuit before DNS).
SKIP=test-cli: the full cli+integration hook trips on pre-existing macOS bash
3.2 shell-harness failures unrelated to this TS-only change; CI runs bash 5.x
green. hermes + policy suites, CLI typecheck, budget, and checks all pass.
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Contributor
Vitest E2E Target Results — ❌ Some jobs failedRun: 28544889595
|
Contributor
Vitest E2E Target Results — ❌ Some jobs failedRun: 28546136750
|
Collaborator
Author
|
Re PRA-3 (policy/index.ts monolith growth) — deferring with rationale, not fixing in this PR:
The two required security items are addressed in this PR: PRA-4/PRA-8 (DNS-rebinding — now routed through |
Collaborator
Author
|
CodeRabbit finding at |
… fix E2E coverage The two E2E failures on this PR were: - network-policy: the allowed_ips guard rejected the legitimate host-gateway preset (web_fetch to host.openshell.internal pins allowed_ips), because the guard couldn't tell a malicious user preset from the trusted sandbox->host bridge. Add a trust-boundary exemption: an endpoint may carry allowed_ips only when its host is host.openshell.internal (mirroring the ALLOWED_PRIVATE_CUSTOM_ENDPOINT_HOSTS exemption in inference-set.ts); any other host with allowed_ips is still rejected, and object-level allowed_ips is never exempt. - onboard-negative-paths: the rejection test used a fake sandbox name, so the CLI failed "sandbox does not exist" before ever reaching preset validation (the guard is dispatched after sandbox resolution). Move the end-to-end rejection to a real sandbox in network-policy.test.ts (tc-net-10: apply a non-bridge allowed_ips preset to the live sandbox, expect rejection), and remove the unreachable fake-sandbox case. The guard logic itself (reject non-bridge, accept the bridge, object-level, prototype-chain) is unit-covered in preset-allowed-ips.test.ts. SKIP=test-cli: the full cli+integration hook trips on pre-existing macOS bash 3.2 shell-harness failures unrelated to this change; CI runs bash 5.x green. policy unit suite, CLI typecheck, budget, and checks all pass. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28549501754
|
…ort, function or class' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
3 tasks
ericksoa
pushed a commit
that referenced
this pull request
Jul 2, 2026
## Summary - Add the `v0.0.72` release-note section with links to the deeper docs pages for installer recovery, command diagnostics, inference, policy, and sandbox repair changes. - Document the custom preset `allowed_ips` guard for user-authored policy files. ## Related Issue None. ## Source summary - #6132 -> `docs/about/release-notes.mdx`: Summarizes installer and upgrade recovery before generic onboarding, with links to quickstart and lifecycle docs. - #6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents that user-authored custom presets reject `allowed_ips` for ordinary endpoints; also summarized in release notes. - #5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based inference probes that keep API keys out of process arguments. - #6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels status` configuration reporting. - #6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2 metadata discovery disablement and links to security guidance. - #5980 and #5991 -> `docs/about/release-notes.mdx`: Summarizes `exec` multiline argument rejection and recovery guidance. - #6023 -> `docs/about/release-notes.mdx`: Summarizes registered-provider diagnostics for `inference set` failures. - #6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed NVIDIA Endpoints featured-model selection behavior. - #5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add` provider credential registration. - #6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw config permission restoration after `exec`. - #6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily access for managed Python workflows. - #6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime version-scheme comparison during upgrade checks. - #6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway watchdog recovery behavior. - #5976 and #5990 -> `docs/about/release-notes.mdx`: Summarizes prompt stdin EOF cancellation behavior during onboarding. - #5540 -> `docs/about/release-notes.mdx`: Summarizes clarified host-level and per-sandbox status command scope. - #5978 and #6018 -> `docs/about/release-notes.mdx`: Summarizes policy-denial log breadcrumbs in connect shells. ## Testing - `npm run docs:sync-agent-variants` - `npm run docs` - Commit hooks passed during `git commit`, including commitlint and gitleaks. - Pre-push hook passed during `git push`, including TypeScript CLI and package/tag version sync. ## Checklist - [x] Documentation updated. - [x] `npm run docs` completed with 0 errors and 1 existing Fern warning. - [x] No source code or generated build artifacts committed. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.72 covering improved installer recovery, clearer CLI diagnostics, safer inference setup and provider switching, better credential handling, stronger policy boundaries, and more robust runtime repair behavior. * Updated network policy guidance to clarify when `allowed_ips` can be used, including a specific exception for the sandbox-to-host bridge endpoint. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
… user presets (NVIDIA#6087) ## Summary Closes two SSRF gaps found during a security review of NemoClaw's policy and onboarding flows. Neither gap was exploitable through the sandboxed agent — both required user-level access to the CLI. ## Related Issue Fixes NVIDIA#6072 Fixes NVIDIA#6073 ## Changes - **`src/lib/onboard/inference-providers/hermes.ts`**: call `isPrivateHostname()` on the user-supplied `endpointUrl` before it is persisted and passed to OpenShell as `OPENAI_BASE_URL`. Previously the URL bypassed all SSRF checks until plugin-side `validateEndpointUrl()` fired at inference time — after credentials could already have been dispatched to an internal host. - **`src/lib/policy/index.ts`**: reject user-supplied preset files (`--from-file` / `--from-dir`) that declare `allowed_ips` in any `network_policies` endpoint. The merge was previously a blind structural pass-through that let callers expand the private-IP allowlist that OpenShell enforces. - **`src/lib/onboard/inference-providers/hermes.test.ts`**: 8 new unit tests covering loopback, link-local (169.254.x.x), RFC-1918, `.internal` TLD, malformed URLs, public endpoint acceptance, and null pass-through. - **`src/lib/policy/preset-allowed-ips.test.ts`**: 4 new unit tests covering single-policy rejection, multi-policy rejection, clean preset acceptance, and endpoint-without-allowed_ips acceptance. - **`test/e2e/live/onboard-negative-paths.test.ts`**: E2E test asserting `nemoclaw policy-add --from-file` exits non-zero and prints an `allowed_ips`/not-permitted error for a user-supplied preset that contains `allowed_ips`. Exits before any sandbox interaction so no live infra required beyond the compiled CLI. ## Type of Change - [x] Code change (feature, bug fix, or refactor) ## Quality Gates - [x] Tests added or updated for changed behavior - [x] Docs not applicable — justification: input validation only, no user-facing behavior change beyond error messages - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — self-review; security team tagged via NVIDIA#6072 and NVIDIA#6073 - [x] Non-success, skipped, or missing CI check accepted by maintainer — `tsc-cli` pre-push hook skipped locally due to missing optional dev deps (`@aws-sdk/client-bedrock-runtime`, `@earendil-works/pi-coding-agent`); pre-existing on `main` ## E2E Coverage Required runs (per E2E advisor): `hermes-e2e`, `network-policy` — dispatch against this branch. New E2E in this PR: `policy-add --from-file allowed_ips rejection` — lightweight, no sandbox required. Hermes onboarding SSRF rejection E2E (suggested by advisor): requires full Hermes sandbox setup (~70 min). Unit tests already cover the validation logic; follow-up issue to be filed. ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior - [x] No secrets, API keys, or credentials committed Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Tightened validation for network endpoints and preset content to block unsafe or internal addresses. * Rejected malformed URLs, unsupported schemes, embedded credentials, and private/internal hostnames. * Prevented presets with disallowed IP-based endpoint rules from being loaded or applied. * Added end-to-end coverage for failed preset imports and safer handling of valid public endpoints. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary - Add the `v0.0.72` release-note section with links to the deeper docs pages for installer recovery, command diagnostics, inference, policy, and sandbox repair changes. - Document the custom preset `allowed_ips` guard for user-authored policy files. ## Related Issue None. ## Source summary - NVIDIA#6132 -> `docs/about/release-notes.mdx`: Summarizes installer and upgrade recovery before generic onboarding, with links to quickstart and lifecycle docs. - NVIDIA#6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents that user-authored custom presets reject `allowed_ips` for ordinary endpoints; also summarized in release notes. - NVIDIA#5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based inference probes that keep API keys out of process arguments. - NVIDIA#6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels status` configuration reporting. - NVIDIA#6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2 metadata discovery disablement and links to security guidance. - NVIDIA#5980 and NVIDIA#5991 -> `docs/about/release-notes.mdx`: Summarizes `exec` multiline argument rejection and recovery guidance. - NVIDIA#6023 -> `docs/about/release-notes.mdx`: Summarizes registered-provider diagnostics for `inference set` failures. - NVIDIA#6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed NVIDIA Endpoints featured-model selection behavior. - NVIDIA#5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add` provider credential registration. - NVIDIA#6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw config permission restoration after `exec`. - NVIDIA#6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily access for managed Python workflows. - NVIDIA#6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime version-scheme comparison during upgrade checks. - NVIDIA#6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway watchdog recovery behavior. - NVIDIA#5976 and NVIDIA#5990 -> `docs/about/release-notes.mdx`: Summarizes prompt stdin EOF cancellation behavior during onboarding. - NVIDIA#5540 -> `docs/about/release-notes.mdx`: Summarizes clarified host-level and per-sandbox status command scope. - NVIDIA#5978 and NVIDIA#6018 -> `docs/about/release-notes.mdx`: Summarizes policy-denial log breadcrumbs in connect shells. ## Testing - `npm run docs:sync-agent-variants` - `npm run docs` - Commit hooks passed during `git commit`, including commitlint and gitleaks. - Pre-push hook passed during `git push`, including TypeScript CLI and package/tag version sync. ## Checklist - [x] Documentation updated. - [x] `npm run docs` completed with 0 errors and 1 existing Fern warning. - [x] No source code or generated build artifacts committed. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.72 covering improved installer recovery, clearer CLI diagnostics, safer inference setup and provider switching, better credential handling, stronger policy boundaries, and more robust runtime repair behavior. * Updated network policy guidance to clarify when `allowed_ips` can be used, including a specific exception for the sandbox-to-host bridge endpoint. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
21 tasks
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
5 participants
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
Closes two SSRF gaps found during a security review of NemoClaw's policy and onboarding flows. Neither gap was exploitable through the sandboxed agent — both required user-level access to the CLI.
Related Issue
Fixes #6072
Fixes #6073
Changes
src/lib/onboard/inference-providers/hermes.ts: callisPrivateHostname()on the user-suppliedendpointUrlbefore it is persisted and passed to OpenShell asOPENAI_BASE_URL. Previously the URL bypassed all SSRF checks until plugin-sidevalidateEndpointUrl()fired at inference time — after credentials could already have been dispatched to an internal host.src/lib/policy/index.ts: reject user-supplied preset files (--from-file/--from-dir) that declareallowed_ipsin anynetwork_policiesendpoint. The merge was previously a blind structural pass-through that let callers expand the private-IP allowlist that OpenShell enforces.src/lib/onboard/inference-providers/hermes.test.ts: 8 new unit tests covering loopback, link-local (169.254.x.x), RFC-1918,.internalTLD, malformed URLs, public endpoint acceptance, and null pass-through.src/lib/policy/preset-allowed-ips.test.ts: 4 new unit tests covering single-policy rejection, multi-policy rejection, clean preset acceptance, and endpoint-without-allowed_ips acceptance.test/e2e/live/onboard-negative-paths.test.ts: E2E test assertingnemoclaw policy-add --from-fileexits non-zero and prints anallowed_ips/not-permitted error for a user-supplied preset that containsallowed_ips. Exits before any sandbox interaction so no live infra required beyond the compiled CLI.Type of Change
Quality Gates
tsc-clipre-push hook skipped locally due to missing optional dev deps (@aws-sdk/client-bedrock-runtime,@earendil-works/pi-coding-agent); pre-existing onmainE2E Coverage
Required runs (per E2E advisor):
hermes-e2e,network-policy— dispatch against this branch.New E2E in this PR:
policy-add --from-file allowed_ips rejection— lightweight, no sandbox required.Hermes onboarding SSRF rejection E2E (suggested by advisor): requires full Hermes sandbox setup (~70 min). Unit tests already cover the validation logic; follow-up issue to be filed.
Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesSigned-off-by: Prekshi Vyas prekshiv@nvidia.com
Summary by CodeRabbit