fix(install): bump OpenShell max version to 0.0.29 for Landlock enforcement#2141
Merged
Conversation
…cement OpenShell 0.0.26 does not enforce Landlock filesystem policy on the sandbox process because its Landlock setup ran after drop_privileges(), causing path fd opens to fail as the unprivileged user and the best_effort handler to silently return success. As a result, /sandbox remained writable on every platform even though NemoClaw delivered the correct policy. Upstream fixed the ordering in NVIDIA/OpenShell#810, released in openshell 0.0.29. Bump MAX_VERSION / PIN_VERSION in the installer and the max_openshell_version pin in the blueprint (plus its test fixture) so fresh installs pull 0.0.29 and existing installs on 0.0.26 are flagged as below the supported range. Closes #1739 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Raise MIN_VERSION from 0.0.24 to 0.0.29 so existing installs on 0.0.26–0.0.28 are forced to upgrade. Previously those versions passed the range check and skipped the upgrade despite carrying the drop_privileges/Landlock ordering bug (NVIDIA/OpenShell#810). Add unit tests covering the version-check branch: clean exit at 0.0.29, upgrade triggered for 0.0.24–0.0.28, and hard failure above MAX_VERSION. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
openshell 0.0.29 self-reports "m-dev" rather than a semver string, causing three independent failures: 1. install-openshell.sh parsed the version as 0.0.0 and re-downloaded the binary on every run even though 0.0.29 was already present. 2. onboard.ts getInstalledOpenshellVersion returned null, triggering an unnecessary reinstall and silently skipping the blueprint version gate. 3. onboard.ts minParts and blueprint.yaml min_openshell_version were both still set to 0.0.24 — not updated when MIN_VERSION was raised to 0.0.29. Fix: after a successful install write PIN_VERSION to a sidecar file (.openshell-installed-version alongside the binary). Both the bash version check and the TypeScript getInstalledOpenshellVersion fall back to that file when --version is unparseable. Update minParts to [0,0,29] and min_openshell_version to "0.0.29" to match MIN_VERSION. Add two unit tests covering m-dev + sidecar: clean exit when sidecar records 0.0.29, upgrade triggered when sidecar records 0.0.26. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
- onboard.ts: broaden sidecar fallback guard from (!versionOutput && openshellBin) to (openshellBin) so the fallback also fires when versionOutput is provided but unparseable (e.g. "openshell m-dev" passed from the blueprint version gate). - test: wrap all inline temp dirs in try/finally + fs.rmSync to prevent /tmp accumulation across repeated test runs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
6 tasks
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:
📝 WalkthroughWalkthroughOpenShell compatibility was pinned to 0.0.29. The installer and library now fall back to a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Installer as Installer Script
participant CLI as "openshell" Binary
participant Sidecar as ".openshell-installed-version"
participant Onboard as Onboard Library
User->>Installer: run install-openshell.sh
Installer->>CLI: `openshell --version`
CLI-->>Installer: versionOutput (parseable / unparseable / missing)
alt parseable
Installer->>Installer: use parsed X.Y.Z
else unparseable or missing
Installer->>Sidecar: read sidecar next to CLI
alt sidecar present and valid
Sidecar-->>Installer: X.Y.Z
else missing/invalid
Installer->>Installer: treat as 0.0.0 (fallback) and attempt install
end
end
alt install performed
Installer->>Sidecar: write pinned X.Y.Z next to installed binary
end
Installer->>Onboard: run preflight (uses blueprint min or default 0.0.29)
Onboard-->>User: proceed or request upgrade based on comparison
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/onboard.test.ts (1)
924-926: Optional: align fixture min version with current pinned range for clarity.This test validates max parsing only, so Line 924 works as-is. Consider changing fixture
min_openshell_versionto0.0.29too, to avoid future confusion when reading the fixture.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/onboard.test.ts` around lines 924 - 926, Update the test fixture so min_openshell_version matches the current pinned range to avoid confusion: change the fixture field min_openshell_version (in the test case that also contains max_openshell_version and min_openclaw_version) from "0.0.24" to "0.0.29" since the test only verifies max parsing but the min value should reflect the pinned range for clarity.src/lib/onboard.ts (1)
2369-2374: Avoid hardcoding the minimum OpenShell floor in another place.
0.0.29now lives here, inscripts/install-openshell.sh, and innemoclaw-blueprint/blueprint.yaml. Deriving this check from a shared source would make the next version bump much harder to drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard.ts` around lines 2369 - 2374, The hardcoded minParts = [0,0,29] in src/lib/onboard.ts should be removed and the minimum OpenShell version must be sourced from the single canonical place (the MIN_VERSION currently defined in scripts/install-openshell.sh / nemoclaw-blueprint/blueprint.yaml). Replace the constant with a single-source lookup: either export the MIN_VERSION into a shared module (e.g., add and import a constant like MIN_OPENSHELL_VERSION from a common config/const file) or parse the canonical file at runtime to produce the numeric array, then compute needsUpgrade using that derived value instead of minParts; update the variable names (minParts -> minVersionParts) and the comparison logic to use the imported/parsed MIN_OPENSHELL_VERSION.
🤖 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/install-openshell.sh`:
- Around line 36-42: Update the default OPENSHELL_VERSION used by the launchable
CI script so it matches the installer pin (MIN_VERSION/MAX_VERSION = "0.0.29");
specifically edit the variable OPENSHELL_VERSION in the launchable CI script
(scripts/brev-launchable-ci-cpu.sh) to change its default from v0.0.24 to
v0.0.29 so the CI cannot provision the older, vulnerable build.
- Around line 143-146: The script writes the sidecar file
".openshell-installed-version" as the unprivileged user (echo "$PIN_VERSION"
>"$target_dir/.openshell-installed-version" 2>/dev/null || true), so when the
binary was installed with sudo into a system directory the sidecar is not
persisted; update the write to detect non-writable target_dir and perform the
write with elevated privileges (e.g., use sudo sh -c 'echo "PIN_VERSION" >
"target_dir/.openshell-installed-version"' or sudo tee) so the sidecar is always
created alongside the installed binary (and remove the unconditional ignore of
failures so errors surface).
---
Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 2369-2374: The hardcoded minParts = [0,0,29] in src/lib/onboard.ts
should be removed and the minimum OpenShell version must be sourced from the
single canonical place (the MIN_VERSION currently defined in
scripts/install-openshell.sh / nemoclaw-blueprint/blueprint.yaml). Replace the
constant with a single-source lookup: either export the MIN_VERSION into a
shared module (e.g., add and import a constant like MIN_OPENSHELL_VERSION from a
common config/const file) or parse the canonical file at runtime to produce the
numeric array, then compute needsUpgrade using that derived value instead of
minParts; update the variable names (minParts -> minVersionParts) and the
comparison logic to use the imported/parsed MIN_OPENSHELL_VERSION.
In `@test/onboard.test.ts`:
- Around line 924-926: Update the test fixture so min_openshell_version matches
the current pinned range to avoid confusion: change the fixture field
min_openshell_version (in the test case that also contains max_openshell_version
and min_openclaw_version) from "0.0.24" to "0.0.29" since the test only verifies
max parsing but the min value should reflect the pinned range for clarity.
🪄 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: 5406ad03-7ec5-4917-8feb-8c4d36fab615
📒 Files selected for processing (5)
nemoclaw-blueprint/blueprint.yamlscripts/install-openshell.shsrc/lib/onboard.tstest/install-openshell-version-check.test.tstest/onboard.test.ts
- scripts/brev-launchable-ci-cpu.sh: bump OPENSHELL_VERSION default from v0.0.24 to v0.0.29 (and sync the comment) so the launchable CI path cannot provision the pre-Landlock build after this change. - scripts/install-openshell.sh: persist the .openshell-installed-version sidecar inside each install branch. The sudo path now uses `sudo tee` so system-wide installs always create the sidecar alongside the binary. Removed the blanket `2>/dev/null || true` so write failures surface under `set -e` instead of silently leaving m-dev builds unrecognized on the next run. - src/lib/onboard.ts: derive the minimum OpenShell version in the reinstall-gate branch from getBlueprintMinOpenshellVersion() instead of hardcoding [0, 0, 29], with a fallback to "0.0.29" (the Landlock floor matching MIN_VERSION in install-openshell.sh) if the blueprint is unreadable. Removes one of the three duplicates of the version constant. - test/onboard.test.ts: align the min_openshell_version fixture in the getBlueprintMaxOpenshellVersion test with the current pinned range (0.0.29) for readability. Test asserts max only, so no behavior change.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/onboard.ts`:
- Around line 2369-2374: The fallback minimum OpenShell version is set too high:
change the fallback in the minOpenshellVersion assignment (where
getBlueprintMinOpenshellVersion() is used) from "0.0.29" to the correct lower
floor (e.g., "0.0.24" or the Landlock-enforcement MIN_VERSION used by
scripts/install-openshell.sh) so unreadable/missing blueprints don't force
upgrades for supported 0.0.24–0.0.28 installs.
🪄 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: 69078618-d8e1-4ca9-afc2-656250a7091e
📒 Files selected for processing (4)
scripts/brev-launchable-ci-cpu.shscripts/install-openshell.shsrc/lib/onboard.tstest/onboard.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/onboard.test.ts
- scripts/install-openshell.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/onboard.ts`:
- Around line 474-487: The fallback currently trusts
.openshell-installed-version even if the openshell binary was replaced; update
the logic in the block that uses openshellBin/sidecar so the sidecar is only
accepted when it is newer-or-equal to the binary: stat both openshellBin and the
sidecar (fs.statSync(openshellBin).mtimeMs and fs.statSync(sidecar).mtimeMs) and
if sidecar.mtimeMs < binary.mtimeMs then treat the sidecar as stale and fall
through (do not return the matched version); otherwise parse and return the
version as before.
🪄 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: 2ca9092f-cff6-40ed-a562-d8249ed48d35
📒 Files selected for processing (2)
scripts/brev-launchable-ci-cpu.shsrc/lib/onboard.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/brev-launchable-ci-cpu.sh
Addresses the last CodeRabbit comment on #2141. Previously the m-dev fallback trusted the sidecar unconditionally, so a manual binary swap (cp /some/openshell /usr/local/bin/openshell) that reports an unparseable version string would silently pass the min/max version gate by re-reading the previous install's sidecar value. Now we compare the binary's mtime to the sidecar's mtime: if the binary is newer, the sidecar is stale and we return null instead of trusting it, forcing a reinstall / re-check. Also tightens the sidecar version regex with ^...$ anchors so junk-wrapped values can't partial-match. No new unit test: exercising this branch requires mocking the module-local resolveOpenshell() call + fs.statSync, which is more plumbing than the defensive one-liner warrants. Covered by: - Existing 138 tests still pass (fresh-install happy path unchanged). - The behavior matches CodeRabbit's suggested diff verbatim.
Addresses the two CodeRabbit nitpicks on #2141: 1. Reuse versionGte() for the min-version upgrade gate in preflight. The open-coded three-way numeric comparison duplicated logic already centralized in versionGte() (and used by the max-version check a few lines below). Collapses 6 lines to 1. 2. Add focused tests for the stale-sidecar branch introduced in the previous commit. Two cases: - binary newer than sidecar -> return null (stale, bypass prevented) - binary not newer than sidecar -> return parsed version (happy path) Both write a fake openshell binary + sidecar into a tmp dir, prepend it to PATH so resolveOpenshell's `command -v` lookup finds it, and control mtimes via fs.utimesSync. PATH is restored in a finally. No behavior change from nitpick 1 (versionGte produces the same result as the open-coded comparison — confirmed by existing tests that pass both before and after). Nitpick 2 locks in the stale-sidecar guard so a future regression can't silently bypass the min/max version gate.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
Author
tested on brev: Landlock now actually attaches (e.g. /dev/shm and Partially closes #1739; full close pending upstream. CI green except the 3 nightly jobs that also fail on main (pre-existing). |
| .match(/^([0-9]+\.[0-9]+\.[0-9]+)$/); | ||
| if (sidecarMatch) return sidecarMatch[1]; | ||
| } catch { | ||
| // sidecar absent or unreadable — fall through |
brandonpelfrey
approved these changes
Apr 21, 2026
1 task
miyoungc
added a commit
that referenced
this pull request
Apr 22, 2026
## Summary Bumps the published doc version to `0.0.22` and documents the user-visible CLI behavior changes to `nemoclaw <name> connect` that landed since v0.0.21. Drafted via the `nemoclaw-contributor-update-docs` skill against commits in `v0.0.21..origin/main`, filtered through `docs/.docs-skip`. ## Changes - **`docs/project.json`** and **`docs/versions1.json`**: bump the published version from `0.0.20` to `0.0.22`; insert a `0.0.21` entry into the version list so the history stays contiguous. - **`docs/reference/commands.md`** → `nemoclaw <name> connect`: document two new behaviors. - Readiness poll with `NEMOCLAW_CONNECT_TIMEOUT` (integer seconds; default `120`) that replaces the silent hang when the sandbox is not yet `Ready` — right after onboarding, while the 2.4 GB image is still pulling (#466). - Post-connect hint is now agent-aware, names the correct TUI command for the sandbox's agent, and tells you to use `/exit` to leave the chat before `exit` returns you to the host shell (#2080). Feature PRs that shipped their own docs in the same commit are intentionally not re-documented here: - `channels list/add/remove` (#2139) — command reference and the "`openclaw channels` blocked inside the sandbox" troubleshooting entry landed with the feature. - `nemoclaw gc` (#2176) — documented as part of the destroy/rebuild image cleanup PR. Skipped per `docs/.docs-skip`: - `e6bad533 fix(shields): verify config lock and fail hard on re-lock failure (#2066)` — matched `skip-features: src/lib/shields.ts`. Other commits in the range (#2141 OpenShell version bump, #1819 plugin banner live inference probe, #2085 / #2146 Slack Socket Mode fixes, #2110 axios proxy fix, #1818 NIM curl timeouts, #1824 onboard gateway bootstrap recovery, and assorted CI / test / install plumbing) are internal behavior refinements with no doc-relevant surface change. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes for the modified files via the pre-commit hook, including `Regenerate agent skills from docs` (source ↔ generated parity confirmed) - [ ] `npm test` passes — skipped; the one pre-existing `test/cli.test.ts > unknown command exits 1` failure on `origin/main` is unrelated to these markdown/JSON-only changes - [ ] Tests added or updated for new or changed behavior — n/a, doc-only - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) — not run locally - [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) — n/a, no new pages ## AI Disclosure - [x] AI-assisted — tool: Claude Code --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * `connect` now displays the sandbox phase while waiting for readiness and honors a configurable timeout via NEMOCLAW_CONNECT_TIMEOUT (default 120s). * TTY hints are agent-aware and instruct using `/exit` before returning to the host shell. * **Documentation** * Command docs updated to describe polling, timeout, and TTY guidance. * Project/docs metadata updated for versions 0.0.21 and 0.0.22 (package version bumped to 0.0.22). <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 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
4 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
Bumps the OpenShell version range from
0.0.26→0.0.29so fresh NemoClaw installs pull a build that actually enforces the Landlock filesystem policy on the sandbox process.Root cause recap
OpenShell 0.0.26's Landlock setup ran after
drop_privileges(), so path-fd opens failed as the unprivileged sandbox user and thebest_efforthandler silently returned success. The sandbox then ran unconfined and/sandboxwas writable on every platform, even though NemoClaw delivered the correct policy (nemoclaw <name> status --jsonshowedfilesystem_policy.read_only: [/sandbox, /sandbox/.openclaw],landlock.compatibility: best_effort).Upstream investigation: NVIDIA/OpenShell#803.
Upstream fix: NVIDIA/OpenShell#810 — two-phase Landlock that opens path fds before dropping privs. First release containing the fix:
openshell 0.0.29.Known limitation: proxy-mode enrichment overrides include_workdir: false
Testing on 0.0.29 confirmed Landlock is enforced — writes to
/dev/shmand/var/tmp(both DAC 1777) are correctly blocked. However,/sandboxremains writable despite being inread_onlywithinclude_workdir: false.Root cause:
enrich_proto_baseline_paths()in theopenshell-sandboxbinary hardcodes/sandboxinPROXY_BASELINE_READ_WRITE. This runs after the gateway delivers NemoClaw's strict policy but beforeprepare()builds the Landlock ruleset. Since Landlock grants the union of matching rules,/sandboxin bothread_onlyandread_write= writable.Filed upstream as NVIDIA/OpenShell#905. No NemoClaw-side workaround exists — the enrichment happens inside the OpenShell binary.
Changes
scripts/install-openshell.sh:MIN_VERSIONandMAX_VERSION(andPIN_VERSIONwhich followsMAX) pinned to0.0.29. Added a comment citing OpenShell#810.nemoclaw-blueprint/blueprint.yaml:max_openshell_version: "0.0.29".test/onboard.test.ts: updated fixture + assertion ingetBlueprintMaxOpenshellVersiontest.Type of Change
Testing
npx vitest run test/onboard.test.ts— 129 passedManual QA results (live sandbox, OpenShell 0.0.29)
touch /dev/shm/testPermission deniedPermission deniedtouch /var/tmp/testPermission deniedPermission deniedtouch /sandbox/testfilePermission deniedtouch /sandbox/.openclaw/testPermission deniedCONFIG:ENRICHEDro:9 rw:5 (was rw:4)Notes
MIN_VERSIONis now0.0.29(raised from0.0.24to matchMAX_VERSION). Operators on 0.0.24–0.0.28 will be flagged and advised to upgrade — those builds have the Landlock silent-pass bug this PR is closing. The blueprint-fallback path insrc/lib/onboard.tsalso defaults to0.0.29so a missing or unreadable blueprint does not silently re-allow the vulnerable versions./sandboxread-only protection requires an upstream fix for OpenShell#905.Relates to #1739
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Tests