Skip to content

fix(config): allow OpenShell bridge URLs in config set - #7681

Merged
senthilr-nv merged 7 commits into
mainfrom
fix/7453-config-set-sandbox-bridge
Jul 28, 2026
Merged

fix(config): allow OpenShell bridge URLs in config set#7681
senthilr-nv merged 7 commits into
mainfrom
fix/7453-config-set-sandbox-bridge

Conversation

@rluo8

@rluo8 rluo8 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes config set so an OpenClaw memorySearch embedding provider can save an OpenShell host bridge URL through host.openshell.internal.
The bridge exception is limited to the intended provider baseUrl shape and retains the surrounding SSRF boundary.

Related Issue

Fixes #7453

Product Scope

Approved by the repository-member design decision in issue #7453, which requires both the documented memory-search setup and the narrowly scoped provider.baseUrl bridge allowance.
This extends the existing supported local Ollama and OpenShell bridge surfaces rather than introducing a new integration.

Changes

  • Allow the exact http://host.openshell.internal:<unprivileged-port> shape only for models.providers.<provider>.baseUrl in OpenClaw and model.base_url in Hermes.
  • Reject adjacent schemes, default or privileged ports, credentials, queries, fragments, subdomains, unsafe key segments, and unrelated config paths.
  • Share the bridge predicate through the existing private-network validation boundary and keep policy behavior aligned.
  • Add positive, negative, nested-config, Hermes onboarding, and write-path regression coverage.
  • Document the OpenClaw memorySearch setup with a host-side Ollama embedding provider.
  • Resolve CodeRabbit's exact-head security-testing finding by covering embedded credentials and an implicit privileged default port.

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)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification:
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Codex Desktop automated security review, not a human review, completed at head 9b184822a8eafdedc69b05e2e54089f3e808ea47; all nine categories passed with no findings.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: docs-updated
  • Evidence: docs/inference/custom-endpoint-security.mdx; docs/inference/set-up-ollama.mdx; no additional documentation required.
  • Agent: Codex Desktop documentation writer subagent

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable; scripts/prepare-dgx-station-host.sh is unchanged.
  • Station profile/scenario: Not applicable.
  • Result: No hardware-review claim.
  • Supporting evidence: Not applicable.

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — vitest exact-head focused set: 5 files and 147 tests passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — not applicable; this is a focused config-validation change and the repository checks passed.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only) — the command passed with 0 errors; Fern reported two unrelated advisories: unauthenticated redirect checking and the existing light-theme accent contrast.
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only) — not applicable; no new documentation page.

Exact reviewed state:

  • Head: 9b184822a8eafdedc69b05e2e54089f3e808ea47
  • Base: c3ab0526eba41d1c681e92bbad3d174925aeaa6a
  • Focused tests: 5 files, 147 tests passed.
  • Diff-aware hooks: pre-commit, commit-message, and pre-push passed against upstream/main.
  • Documentation: npm run docs passed with 0 errors and the two unrelated advisories described above.
  • Security: nine-category automated review passed with no findings; this is not a human sensitive-path review.
  • Product scope: approved by the linked repository-member design decision, independently of GitHub merge state.
  • Human approval: required for this exact head before merge.

Signed-off-by: Rui Luo ruluo@nvidia.com

Signed-off-by: Rui Luo <ruluo@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds strict OpenShell sandbox bridge URL validation, applies it to inference routes and selected config set paths, enables Hermes onboarding for the bridge, adds regression coverage, and documents host-side Ollama memory-search configuration.

Changes

OpenShell bridge URL support

Layer / File(s) Summary
Bridge URL contract and routing
src/lib/private-networks.ts, src/lib/actions/inference-set-route-containment.ts, src/lib/policy/index.ts
Defines the canonical bridge hostname and validates exact HTTP URLs on unprivileged ports without credentials, queries, or fragments. Inference routing and policy checks reuse the shared contract.
Path-aware config validation
src/lib/sandbox/config.ts, test/config-set.test.ts, test/config-set-nested-ssrf.test.ts
Adds explicit bridge-validation options, recursive path tracking, allowlisted agent/provider paths, prototype-pollution protections, and config-write integration tests.
Inference integration and validation
src/lib/onboard/inference-providers/hermes.ts, src/lib/onboard/inference-providers/hermes.test.ts
Allows Hermes onboarding to preserve the supported bridge endpoint without DNS lookup and verifies the resulting setup arguments and smoke-test input.
Bridge configuration documentation
docs/inference/custom-endpoint-security.mdx, docs/inference/set-up-ollama.mdx
Documents the bridge URL restrictions and the OpenClaw host-Ollama memory-search setup procedure.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant ConfigSet
  participant RewriteConfigUrls
  participant OpenShellBridgeValidator
  participant ConfigGuard
  Operator->>ConfigSet: set provider baseUrl
  ConfigSet->>RewriteConfigUrls: validate nested config with path
  RewriteConfigUrls->>OpenShellBridgeValidator: check bridge URL shape
  OpenShellBridgeValidator-->>RewriteConfigUrls: allow eligible path or reject
  RewriteConfigUrls->>ConfigGuard: write validated configuration
Loading

Suggested labels: area: security, bug-fix, feature, area: onboarding, area: inference

Suggested reviewers: cv, brandonpelfrey, dnandakumar-nv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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
Linked Issues check ✅ Passed The PR adds a supported bridge-URL path for config set, regression coverage, and Ollama memory-search documentation matching issue #7453.
Out of Scope Changes check ✅ Passed The changes stay focused on OpenShell bridge URL handling, related validation, tests, and documentation; no clear unrelated work stands out.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: enabling OpenShell bridge URLs in config set.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/7453-config-set-sandbox-bridge

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

@github-actions

Copy link
Copy Markdown
Contributor

@github-code-quality

github-code-quality Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 9b18482 in the fix/7453-config-set-... branch remains at 96%, unchanged from commit c3ab052 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 9b18482 in the fix/7453-config-set-... branch remains at 81%, unchanged from commit c3ab052 in the main branch.

Show a code coverage summary of the most impacted files.
File main c3ab052 fix/7453-config-set-... 9b18482 +/-
src/lib/domain/.../connect-env.ts 97% 89% -8%
src/lib/onboard...iders/hermes.ts 89% 87% -2%
src/lib/policy/index.ts 61% 61% 0%
src/lib/actions...-containment.ts 95% 95% 0%
src/lib/sandbox...rce-identity.ts 88% 88% 0%
src/lib/sandbox/config.ts 72% 74% +2%
src/lib/private-networks.ts 88% 93% +5%

Updated July 28, 2026 16:46 UTC

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 1 suggestion
  • Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, the same number of warnings, 1 more suggestion.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-inference, security-posture, inference-routing, network-policy, onboard-repair, onboard-resume, cloud-onboard

1 optional E2E recommendation
  • hermes-inference-switch

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

Signed-off-by: Rui Luo <ruluo@nvidia.com>
@rluo8
rluo8 marked this pull request as ready for review July 28, 2026 04:34

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (1)
test/config-set.test.ts (1)

463-477: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider covering the two remaining rejected bridge shapes.

isAllowedOpenShellSandboxBridgeUrl also rejects embedded credentials and the implicit (privileged) default port, but neither shape has a negative test. This is the natural place for them.

As per path instructions for src/lib/sandbox/{config,privileged-exec}.ts: "Require negative-path tests that prove the boundary rejects bypasses and does not leak secrets in errors, logs, state, or process arguments."

🧪 Additional negative cases
       expect(() =>
         validateUrlValue("http://host.openshell.internal:11434/v1?token=secret", options),
       ).toThrow(/private/i);
+      expect(() =>
+        validateUrlValue("http://user:pass@host.openshell.internal:11434/v1", options),
+      ).toThrow(/private/i);
+      expect(() => validateUrlValue("http://host.openshell.internal/v1", options)).toThrow(
+        /private/i,
+      );
     });
🤖 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 `@test/config-set.test.ts` around lines 463 - 477, Add negative assertions to
the “rejects adjacent OpenShell host bridge bypass shapes” test covering URLs
with embedded credentials and URLs using the implicit privileged default port.
Invoke validateUrlValue with the existing allowOpenShellBridge options and
assert both cases throw a private-related error, while preserving the current
bypass cases.

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.

Nitpick comments:
In `@test/config-set.test.ts`:
- Around line 463-477: Add negative assertions to the “rejects adjacent
OpenShell host bridge bypass shapes” test covering URLs with embedded
credentials and URLs using the implicit privileged default port. Invoke
validateUrlValue with the existing allowOpenShellBridge options and assert both
cases throw a private-related error, while preserving the current bypass cases.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 90de4e34-c9b7-4773-9d45-19b9c562018e

📥 Commits

Reviewing files that changed from the base of the PR and between b92ef67 and b1d5b37.

📒 Files selected for processing (10)
  • docs/inference/custom-endpoint-security.mdx
  • docs/inference/set-up-ollama.mdx
  • src/lib/actions/inference-set-route-containment.ts
  • src/lib/onboard/inference-providers/hermes.test.ts
  • src/lib/onboard/inference-providers/hermes.ts
  • src/lib/policy/index.ts
  • src/lib/sandbox/config.ts
  • src/lib/sandbox/openshell-bridge-url.ts
  • test/config-set-nested-ssrf.test.ts
  • test/config-set.test.ts

@rluo8 rluo8 added the v0.0.97 label Jul 28, 2026
@senthilr-nv senthilr-nv self-assigned this Jul 28, 2026
@senthilr-nv

Copy link
Copy Markdown
Collaborator

Automated-review disposition for exact head 39bf3fed697e27ed0dd3becea096eafbd6013699: CodeRabbit’s bridge-boundary test finding was valid and is resolved in e6e6a0e53b32ca05b60af2498493e6970b86c9ed. The negative test now covers both embedded URL credentials and the implicit privileged default HTTP port. The refreshed focused suite passes 147 tests. No other CodeRabbit finding from the prior head remains undispositioned.

@senthilr-nv

Copy link
Copy Markdown
Collaborator

Automated-review disposition for CodeRabbit’s exact-head docstring-coverage warning on 39bf3fed697e27ed0dd3becea096eafbd6013699: non-actionable. NemoClaw does not require exported-function docstrings as a merge gate, the changed bridge predicate and path guard are small direct functions with behavior-oriented tests, and the repository lint, architecture, focused test, and docs gates pass. Adding quota-driven comments would not clarify a supported contract or fix a correctness, security, or test-coverage defect. No code change is warranted.

@senthilr-nv senthilr-nv 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.

I reviewed the Files changed for exact head 39bf3fe against base f90c792. I approve this exact revision for merge, subject to the required checks and exact-pair E2E gates passing and upstream/main remaining unchanged.

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
@senthilr-nv

Copy link
Copy Markdown
Collaborator

Automated-review disposition for Nemotron's exact-head suggestion on 9b184822a8eafdedc69b05e2e54089f3e808ea47: already covered, so no code change is warranted. src/lib/actions/inference-set-endpoint-security.test.ts directly verifies normalizeCustomEndpointUrl accepts the exact bridge at ports 1024 and 65535, rejects no-port, privileged-port, and HTTPS shapes, and rejects userinfo, query, and fragment shapes. src/lib/actions/inference-set-compatible-provider.test.ts additionally rejects the bridge subdomain shape. The refreshed focused suite passes all 147 tests. The suggestion's premise that inference-set lacks a direct regression test is therefore false.

@senthilr-nv senthilr-nv 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.

I reviewed the Files changed for exact head 9b18482 against base c3ab052. I approve this exact revision for merge, subject to the required checks and exact-pair E2E gates passing and upstream/main remaining unchanged.

@senthilr-nv
senthilr-nv merged commit 7325a80 into main Jul 28, 2026
74 of 78 checks passed
@senthilr-nv
senthilr-nv deleted the fix/7453-config-set-sandbox-bridge branch July 28, 2026 17:08
@cjagwani cjagwani added v0.0.98 and removed v0.0.97 labels Jul 28, 2026
@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression integration: openclaw OpenClaw integration behavior labels Jul 28, 2026
@cjagwani cjagwani mentioned this pull request Jul 28, 2026
23 tasks
cv pushed a commit that referenced this pull request Jul 28, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical dated changelog entry for NemoClaw v0.0.97 before the
release plan captures `origin/main`.
The entry groups the user-visible and maintainer-facing changes since
v0.0.96 while preserving the Deferred dual-Station status, experimental
runtime-identity boundary, and pending physical IGX validation.

## Changes

- Add `docs/changelog/2026-07-28.mdx` with the parser-safe MDX SPDX
comment and exact `## v0.0.97` heading.
- Summarize the 43 merged PRs in the release range, omitting
internal-only changes from the public entry and linking each grouped
change to its most specific published documentation.
- Keep the experimental Okta reference explicitly opt-in and outside
normal onboarding, keep the two-Station path Deferred, and state that
physical IGX Orin validation remains pending.

### Source summary

- [#7440](#7440),
[#7443](#7443), and
[#7445](#7445) ->
`docs/changelog/2026-07-28.mdx`: Document read-only host readiness
reports and fail-closed platform qualification.
- [#7030](#7030) ->
`docs/changelog/2026-07-28.mdx`: Document the Deferred trusted
two-Station vLLM evaluation.
- [#7265](#7265) ->
`docs/changelog/2026-07-28.mdx`: Document the bounded experimental
direct-runner Okta runtime-identity reference.
- [#7711](#7711) and
[#7648](#7648) ->
`docs/changelog/2026-07-28.mdx`: Document compatible-endpoint reasoning
effort and retired NVIDIA Build model paths.
- [#7746](#7746),
[#7763](#7763), and
[#7681](#7681) ->
`docs/changelog/2026-07-28.mdx`: Document safe compatible-provider
creation, replacement refusal, and narrow OpenShell bridge URL handling.
- [#7641](#7641),
[#7690](#7690),
[#7631](#7631), and
[#7710](#7710) ->
`docs/changelog/2026-07-28.mdx`: Document paused-container recovery,
recreation journaling, pre-mutation uninstall checks, and
source-checkout OpenShell selection.
- [#7624](#7624) and
[#7762](#7762) ->
`docs/changelog/2026-07-28.mdx`: Document Jetson release diagnostics and
bounded render-device group propagation.
- [#7639](#7639),
[#7760](#7760),
[#7721](#7721), and
[#7761](#7761) ->
`docs/changelog/2026-07-28.mdx`: Document Telegram, MCP media-type,
Hermes image-mode, and locked-restart fixes.
- [#7653](#7653) and
[#7680](#7680) ->
`docs/changelog/2026-07-28.mdx`: Document Deep Agents policy tasks and
the bounded Claude Code OAuth path.
- [#7679](#7679) ->
`docs/changelog/2026-07-28.mdx`: Document the checksum-bound libssh2 and
Python HTMLParser backports.
- [#7655](#7655),
[#7651](#7651),
[#7664](#7664),
[#7666](#7666),
[#7670](#7670),
[#7719](#7719), and
[#7741](#7741) ->
`docs/changelog/2026-07-28.mdx`: Document exact candidate E2E evidence,
Launchable selection, diagnostic consolidation, and trusted WSL
validation.

## Type of Change

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

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification:
`test/changelog-docs.test.ts` validates the dated changelog contract,
MDX header, heading uniqueness, and release-entry structure.
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Documentation Writer Review

- [x] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated`
- Evidence: The committed `docs/changelog/2026-07-28.mdx` blob exactly
matches the reviewed file. Completeness, factual accuracy, link shape,
parser-safe MDX header, one-sentence-per-line style, `.docs-skip`
compliance, and bounded product claims passed.
- Agent: Codex Desktop documentation writer subagent
<!-- docs-review-head-sha: da6aa27 -->
<!-- docs-review-agents-blob-sha: be20a09 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable; this PR changes only the dated
changelog.
- Station profile/scenario: Not applicable.
- Result: Not applicable.
- Supporting evidence: Not applicable.

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run
test/changelog-docs.test.ts` passed 6/6.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not applicable to this doc-only release
entry.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) —
completed with 0 errors and 2 pre-existing Fern warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)
— native changelog entries use the required parser-safe MDX SPDX comment
and intentionally have no frontmatter.

---
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added improved host readiness reporting and Jetson onboarding
guidance.
* Added controls for reasoning effort with compatible endpoints and
enhanced managed MCP discovery.
  * Improved Deep Agents task publication and preset support.
* **Bug Fixes**
* Hardened provider switching, sandbox recovery, uninstall behavior, and
Telegram connectivity.
* Improved container image integrity checks, media-type handling, and
checksum validation.
  * Enhanced vLLM evaluation behavior and release diagnostics.
* **Documentation**
  * Added the NemoClaw v0.0.97 changelog.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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 integration: openclaw OpenClaw integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

How to configure a memory-search embedding provider pointing at local Ollama?

4 participants