-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(install): bump OpenShell max version to 0.0.29 for Landlock enforcement #2141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
2314c51
fix(install): bump OpenShell max version to 0.0.29 for Landlock enfor…
prekshivyas ed2dee5
fix(install): enforce openshell 0.0.29 as hard minimum for Landlock fix
prekshivyas 1b9da8a
Merge branch 'main' into fix/1739-openshell-0.0.29-landlock
prekshivyas fbc2cb0
Merge branch 'main' into fix/1739-openshell-0.0.29-landlock
prekshivyas 863ed47
fix(install): handle m-dev version string and sync min-version constants
prekshivyas 57fb502
fix(install): address CodeRabbit review — sidecar guard and tmp cleanup
prekshivyas bb00e3b
Merge branch 'main' into fix/1739-openshell-0.0.29-landlock
prekshivyas 778cd43
Merge branch 'main' into fix/1739-openshell-0.0.29-landlock
prekshivyas 08a5b00
Merge branch 'main' into fix/1739-openshell-0.0.29-landlock
prekshivyas 9fe6edb
fix(install): address CodeRabbit review on #2141
prekshivyas f720a83
Merge branch 'main' into prekshi/openshell-0.0.29-landlock
prekshivyas d956ff9
Merge branch 'main' into prekshi/openshell-0.0.29-landlock
prekshivyas 771ee87
Merge branch 'main' into prekshi/openshell-0.0.29-landlock
prekshivyas adcb80e
Merge branch 'main' into prekshi/openshell-0.0.29-landlock
prekshivyas 972c0e0
Merge branch 'main' into prekshi/openshell-0.0.29-landlock
prekshivyas 519b43e
fix(install): invalidate stale .openshell-installed-version sidecar
prekshivyas 28699ce
refactor(onboard): consolidate min-version gate + test stale sidecar
prekshivyas 553c2d0
Merge branch 'main' into prekshi/openshell-0.0.29-landlock
prekshivyas 5034333
Merge branch 'main' into prekshi/openshell-0.0.29-landlock
prekshivyas eccb325
Merge branch 'main' into prekshi/openshell-0.0.29-landlock
prekshivyas 9e3360c
Merge branch 'main' into prekshi/openshell-0.0.29-landlock
prekshivyas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -489,8 +489,31 @@ function getInstalledOpenshellVersion(versionOutput = null) { | |
| versionOutput ?? runCapture([openshellBin, "-V"], { ignoreError: true }), | ||
| ).trim(); | ||
| const match = output.match(/openshell\s+([0-9]+\.[0-9]+\.[0-9]+)/i); | ||
| if (!match) return null; | ||
| return match[1]; | ||
| if (match) return match[1]; | ||
| // Fallback: read the sidecar version file written by install-openshell.sh for | ||
| // binaries that self-report an unparseable string (e.g. openshell 0.0.29 → "m-dev"). | ||
| // Also applies when versionOutput is provided but unparseable (e.g. passed from | ||
| // runCaptureOpenshell at the blueprint version gate). | ||
| if (openshellBin) { | ||
| try { | ||
| const sidecar = path.join(path.dirname(openshellBin), ".openshell-installed-version"); | ||
| // Invalidate the sidecar if the binary has been replaced since the sidecar | ||
| // was written (manual `cp openshell /usr/local/bin/openshell` without | ||
| // re-running install-openshell.sh). Stale sidecar + unsupported binary would | ||
| // otherwise bypass the min/max version gate silently. | ||
| const binaryMtime = fs.statSync(openshellBin).mtimeMs; | ||
| const sidecarMtime = fs.statSync(sidecar).mtimeMs; | ||
| if (binaryMtime > sidecarMtime) return null; | ||
| const sidecarMatch = fs | ||
| .readFileSync(sidecar, "utf-8") | ||
| .trim() | ||
| .match(/^([0-9]+\.[0-9]+\.[0-9]+)$/); | ||
| if (sidecarMatch) return sidecarMatch[1]; | ||
| } catch { | ||
| // sidecar absent or unreadable — fall through | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth a log? |
||
| } | ||
| } | ||
| return null; | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -2557,12 +2580,11 @@ async function preflight() { | |
| process.exit(1); | ||
| } | ||
| } else { | ||
| const parts = currentVersion.split(".").map(Number); | ||
| const minParts = [0, 0, 24]; // must match MIN_VERSION in scripts/install-openshell.sh | ||
| const needsUpgrade = | ||
| parts[0] < minParts[0] || | ||
| (parts[0] === minParts[0] && parts[1] < minParts[1]) || | ||
| (parts[0] === minParts[0] && parts[1] === minParts[1] && parts[2] < minParts[2]); | ||
| // Source of truth: min_openshell_version in nemoclaw-blueprint/blueprint.yaml. | ||
| // Fall back to the Landlock-enforcement floor (also MIN_VERSION in | ||
| // scripts/install-openshell.sh) if the blueprint cannot be read. | ||
| const minOpenshellVersion = getBlueprintMinOpenshellVersion() ?? "0.0.29"; | ||
| const needsUpgrade = !versionGte(currentVersion, minOpenshellVersion); | ||
| if (needsUpgrade) { | ||
| console.log( | ||
| ` openshell ${currentVersion} is below minimum required version. Upgrading...`, | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| // @ts-nocheck | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { describe, it, expect } from "vitest"; | ||
| import fs from "node:fs"; | ||
| import os from "node:os"; | ||
| import path from "node:path"; | ||
| import { spawnSync } from "node:child_process"; | ||
|
|
||
| const SCRIPT = path.join(import.meta.dirname, "..", "scripts", "install-openshell.sh"); | ||
|
|
||
| function writeExecutable(target: string, contents: string) { | ||
| fs.writeFileSync(target, contents, { mode: 0o755 }); | ||
| } | ||
|
|
||
| /** | ||
| * Run install-openshell.sh with a fake `openshell` binary that reports the | ||
| * given version. The download/install code path is never reached because we | ||
| * either exit early (version ok / too high) or hit the upgrade warn and then | ||
| * the script tries to download — so we stub curl and gh to fail fast. | ||
| */ | ||
| function runWithInstalledVersion(version: string) { | ||
| const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-openshell-ver-")); | ||
| try { | ||
| const fakeBin = path.join(tmp, "bin"); | ||
| fs.mkdirSync(fakeBin); | ||
|
|
||
| // Fake openshell that reports the given version | ||
| writeExecutable( | ||
| path.join(fakeBin, "openshell"), | ||
| `#!/usr/bin/env bash | ||
| if [ "\${1:-}" = "--version" ]; then echo "openshell ${version}"; exit 0; fi | ||
| exit 99`, | ||
| ); | ||
|
|
||
| // Stub curl to fail so the install path exits without doing real network I/O | ||
| writeExecutable( | ||
| path.join(fakeBin, "curl"), | ||
| `#!/usr/bin/env bash | ||
| echo "curl stub: not available in test" >&2 | ||
| exit 1`, | ||
| ); | ||
|
|
||
| // Stub gh CLI similarly | ||
| writeExecutable( | ||
| path.join(fakeBin, "gh"), | ||
| `#!/usr/bin/env bash | ||
| exit 1`, | ||
| ); | ||
|
|
||
| return spawnSync("bash", [SCRIPT], { | ||
| env: { ...process.env, PATH: `${fakeBin}:/usr/bin:/bin` }, | ||
| encoding: "utf8", | ||
| }); | ||
| } finally { | ||
| fs.rmSync(tmp, { recursive: true, force: true }); | ||
| } | ||
| } | ||
|
|
||
| describe("install-openshell.sh version check", () => { | ||
| it("exits cleanly when openshell 0.0.29 is already installed", () => { | ||
| const result = runWithInstalledVersion("0.0.29"); | ||
| expect(result.status).toBe(0); | ||
| expect(result.stdout).toMatch(/already installed.*0\.0\.29/); | ||
| }); | ||
|
|
||
| it("triggers upgrade when openshell 0.0.28 is installed (below MIN_VERSION)", () => { | ||
| const result = runWithInstalledVersion("0.0.28"); | ||
| // Script should warn about upgrade then fail at the download step (curl stub fails) | ||
| expect(result.status).not.toBe(0); | ||
| expect(result.stdout).toMatch(/below minimum.*upgrading/); | ||
| }); | ||
|
|
||
| it("triggers upgrade when openshell 0.0.26 is installed (Landlock-vulnerable version)", () => { | ||
| const result = runWithInstalledVersion("0.0.26"); | ||
| expect(result.status).not.toBe(0); | ||
| expect(result.stdout).toMatch(/below minimum.*upgrading/); | ||
| }); | ||
|
|
||
| it("triggers upgrade when openshell 0.0.24 is installed (old minimum)", () => { | ||
| const result = runWithInstalledVersion("0.0.24"); | ||
| expect(result.status).not.toBe(0); | ||
| expect(result.stdout).toMatch(/below minimum.*upgrading/); | ||
| }); | ||
|
|
||
| it("fails with a clear error when openshell is above MAX_VERSION", () => { | ||
| const result = runWithInstalledVersion("0.0.30"); | ||
| expect(result.status).toBe(1); | ||
| expect(result.stdout).toMatch(/above the maximum/); | ||
| }); | ||
|
|
||
| it("fails with a clear error when openshell is at a much newer version", () => { | ||
| const result = runWithInstalledVersion("0.1.0"); | ||
| expect(result.status).toBe(1); | ||
| expect(result.stdout).toMatch(/above the maximum/); | ||
| }); | ||
|
|
||
| it("exits cleanly when openshell reports m-dev but sidecar records 0.0.29", () => { | ||
| const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-openshell-mdev-")); | ||
| try { | ||
| const fakeBin = path.join(tmp, "bin"); | ||
| fs.mkdirSync(fakeBin); | ||
|
|
||
| // Fake openshell that reports "m-dev" (as openshell 0.0.29 does in practice) | ||
| writeExecutable( | ||
| path.join(fakeBin, "openshell"), | ||
| `#!/usr/bin/env bash | ||
| if [ "\${1:-}" = "--version" ]; then echo "openshell m-dev"; exit 0; fi | ||
| exit 99`, | ||
| ); | ||
|
|
||
| // Sidecar file written by a previous install | ||
| fs.writeFileSync(path.join(fakeBin, ".openshell-installed-version"), "0.0.29\n"); | ||
|
|
||
| writeExecutable(path.join(fakeBin, "curl"), `#!/usr/bin/env bash\nexit 1`); | ||
| writeExecutable(path.join(fakeBin, "gh"), `#!/usr/bin/env bash\nexit 1`); | ||
|
|
||
| const result = spawnSync("bash", [SCRIPT], { | ||
| env: { ...process.env, PATH: `${fakeBin}:/usr/bin:/bin` }, | ||
| encoding: "utf8", | ||
| }); | ||
| expect(result.status).toBe(0); | ||
| expect(result.stdout).toMatch(/already installed.*0\.0\.29/); | ||
| } finally { | ||
| fs.rmSync(tmp, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| it("triggers upgrade when openshell reports m-dev and sidecar records 0.0.26", () => { | ||
| const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-openshell-mdev-old-")); | ||
| try { | ||
| const fakeBin = path.join(tmp, "bin"); | ||
| fs.mkdirSync(fakeBin); | ||
|
|
||
| writeExecutable( | ||
| path.join(fakeBin, "openshell"), | ||
| `#!/usr/bin/env bash | ||
| if [ "\${1:-}" = "--version" ]; then echo "openshell m-dev"; exit 0; fi | ||
| exit 99`, | ||
| ); | ||
|
|
||
| // Sidecar from an older install that pre-dates the Landlock fix | ||
| fs.writeFileSync(path.join(fakeBin, ".openshell-installed-version"), "0.0.26\n"); | ||
|
|
||
| writeExecutable( | ||
| path.join(fakeBin, "curl"), | ||
| `#!/usr/bin/env bash\necho "curl stub" >&2\nexit 1`, | ||
| ); | ||
| writeExecutable(path.join(fakeBin, "gh"), `#!/usr/bin/env bash\nexit 1`); | ||
|
|
||
| const result = spawnSync("bash", [SCRIPT], { | ||
| env: { ...process.env, PATH: `${fakeBin}:/usr/bin:/bin` }, | ||
| encoding: "utf8", | ||
| }); | ||
| expect(result.status).not.toBe(0); | ||
| expect(result.stdout).toMatch(/below minimum.*upgrading/); | ||
| } finally { | ||
| fs.rmSync(tmp, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| it("proceeds to install when openshell is not present", () => { | ||
| const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-openshell-noop-")); | ||
| try { | ||
| const fakeBin = path.join(tmp, "bin"); | ||
| fs.mkdirSync(fakeBin); | ||
|
|
||
| // No openshell binary — just stub curl/gh to fail fast | ||
| writeExecutable( | ||
| path.join(fakeBin, "curl"), | ||
| `#!/usr/bin/env bash | ||
| echo "curl stub: not available in test" >&2 | ||
| exit 1`, | ||
| ); | ||
| writeExecutable( | ||
| path.join(fakeBin, "gh"), | ||
| `#!/usr/bin/env bash | ||
| exit 1`, | ||
| ); | ||
|
|
||
| const result = spawnSync("bash", [SCRIPT], { | ||
| env: { ...process.env, PATH: `${fakeBin}:/usr/bin:/bin` }, | ||
| encoding: "utf8", | ||
| }); | ||
|
|
||
| // Should attempt install (not exit 0 early) and fail at the download step | ||
| expect(result.stdout).toMatch(/Installing openshell CLI/); | ||
| expect(result.status).not.toBe(0); | ||
| } finally { | ||
| fs.rmSync(tmp, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.