From 2e446207882487b0ceb8f288310aee98f9cfa6a9 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 12:47:29 -0700 Subject: [PATCH 01/50] chore(openshell): prepare 0.0.82 dependency upgrade Signed-off-by: Aaron Erickson From 225e154af534d199112674efbdb1a07a87a54f0a Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 13:01:26 -0700 Subject: [PATCH 02/50] fix(mcp): align credential diagnostics with OpenShell main Signed-off-by: Aaron Erickson --- agents/hermes/mcp-config-transaction.py | 4 +++- docs/deployment/set-up-mcp-bridge.mdx | 4 ++++ .../mcp-bridge-input-validation.test.ts | 22 +++++++++++++++++++ .../mcp-bridge-resolution-probe.test.ts | 14 ++++++++++++ .../sandbox/mcp-bridge-resolution-probe.ts | 3 +++ .../actions/sandbox/mcp-bridge-validation.ts | 7 ++++++ test/hermes-mcp-config-transaction.test.ts | 18 ++++++++++----- 7 files changed, 65 insertions(+), 7 deletions(-) diff --git a/agents/hermes/mcp-config-transaction.py b/agents/hermes/mcp-config-transaction.py index d848a26c6ac..8ac4bd97e73 100755 --- a/agents/hermes/mcp-config-transaction.py +++ b/agents/hermes/mcp-config-transaction.py @@ -57,6 +57,7 @@ ENV_PLACEHOLDER_RE = re.compile( r"^Bearer openshell:resolve:env:([A-Za-z_][A-Za-z0-9_]{0,127})$" ) +OPENSHELL_REVISIONED_CREDENTIAL_NAME_RE = re.compile(r"^v[0-9]+_[A-Za-z0-9_]+$") BOUNDARY_MANIFEST_NAME = "openshell-child-visible-credentials.v0.0.72.json" ANSI_ESCAPE_RE = re.compile( r"\x1b(?:\[[0-?]*[ -/]*[@-~]|\][^\x07]*(?:\x07|\x1b\\)|[@-_])" @@ -174,7 +175,8 @@ def _manifest_strings(manifest: dict[str, object], key: str) -> frozenset[str]: def _credential_name_is_reserved(name: str) -> bool: return ( - name in _RAW_CHILD_VALUE_KEYS + OPENSHELL_REVISIONED_CREDENTIAL_NAME_RE.fullmatch(name) is not None + or name in _RAW_CHILD_VALUE_KEYS or name in _REWRITTEN_CHILD_VALUE_KEYS or name in _RUNTIME_CONTROL_KEYS or any(name.startswith(prefix) for prefix in _RUNTIME_CONTROL_PREFIXES) diff --git a/docs/deployment/set-up-mcp-bridge.mdx b/docs/deployment/set-up-mcp-bridge.mdx index d064394bc54..7eb96528b5a 100644 --- a/docs/deployment/set-up-mcp-bridge.mdx +++ b/docs/deployment/set-up-mcp-bridge.mdx @@ -88,6 +88,8 @@ The child-visible compatibility list is pinned to OpenShell `v0.0.72` commit `8c Choose a dedicated name such as `MY_SERVICE_MCP_TOKEN`. NemoClaw also rejects host subprocess control names such as `PATH`, proxy/TLS variables, and `OPENSHELL_*`, `GRPC_*`, `LC_*`, or `XDG_*` keys so the selected credential cannot be inherited by unrelated OpenShell commands. Loader, shell, language, and agent runtime controls such as `LD_PRELOAD`, `BASH_ENV`, `NODE_OPTIONS`, `PYTHONHOME`, `NEMOCLAW_*`, and `OPENCLAW_*` are rejected as well because OpenShell attaches provider keys to fresh sandbox execs; use a dedicated service name such as `MY_SERVICE_MCP_TOKEN`. +OpenShell reserves credential names matching `v[0-9]+_[A-Za-z0-9_]+` for revisioned placeholders. +NemoClaw rejects names such as `v10_GITHUB_TOKEN` because OpenShell skips them instead of attaching a credential resolver. NemoClaw requires exactly one `--env` bearer credential per server. Every endpoint must use HTTPS. @@ -323,6 +325,8 @@ For an identical HTTP 401 or 403, a confirmed-valid credential settles it: the O An identical HTTP 400 stays inconclusive even with a valid credential, because the endpoint may reject the probe's `initialize` request itself; compare `mcp status` for the same server on a known-good host — if that host verifies, suspect this host's placeholder rewrite. That is a host-side OpenShell defect rather than a NemoClaw registration problem: verify the OpenShell installation on the host (tracked upstream as OpenShell issue 2161). A `credential resolution: unknown` verdict with an endpoint or policy detail means the probe could not reach a judgment; fix the reported endpoint or policy condition and rerun `mcp status `. +A detail containing `CONNECT 503` means OpenShell failed closed before TLS setup because gateway TLS termination or credential rewriting was unavailable. +Inspect the OpenShell gateway's ephemeral CA initialization and provider environment, repair the reported condition, and rerun `mcp status `. If `restart` reports a missing provider and the original credential is not registered in OpenShell, export the same variable name used during `add` and retry. diff --git a/src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts b/src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts index 1128356eb6e..250f84d5ed9 100644 --- a/src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts @@ -39,6 +39,28 @@ describe("MCP CLI input validation", () => { ).toThrow(/process arguments and shell history/); }); + it("rejects OpenShell revisioned placeholder names as MCP credentials (#6379)", () => { + for (const name of ["v1_TOKEN", "v999999_very_unlikely", "v0_1"]) { + expect(() => + parseMcpAddArgs(["github", "--url", "https://mcp.example.test/mcp", "--env", name]), + ).toThrow(/reserved for OpenShell credential revisions/); + expect(() => resolveCredentialEnv([{ name, value: "host-only-secret" }])).toThrow( + /would be skipped instead of attached/, + ); + expect(() => + buildMcpBridgeProviderArgs("create", "provider", [{ name }], { + [name]: "host-only-secret", + }), + ).toThrow(/reserved for OpenShell credential revisions/); + } + + for (const name of ["v_TOKEN", "v10_", "versioned_token", "V10_TOKEN"]) { + expect(() => + parseMcpAddArgs(["github", "--url", "https://mcp.example.test/mcp", "--env", name]), + ).not.toThrow(); + } + }); + // source-shape-contract: compatibility -- Pinned OpenShell child-visible keys must drive credential rejection through every MCP boundary it("rejects OpenShell child-environment compatibility keys as MCP credentials", () => { for (const name of childVisibleCredentialManifest.rawChildValueKeys) { diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts index 7592d7121ef..73cf334bea6 100644 --- a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts @@ -228,6 +228,20 @@ describe("MCP credential-resolution probe classification", () => { expect(probe.detail).toContain("CONNECT 403"); }); + it("classifies a CONNECT-level proxy 503 as unavailable credential rewriting (#6379)", () => { + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: probeStdout({ curlExit: 56 }), + stderr: "curl: (56) CONNECT tunnel failed, response 503", + }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(probe.detail).toContain("CONNECT 503"); + expect(probe.detail).toContain("credential-rewrite readiness"); + }); + it("classifies curl exit 28 as an indeterminate probe timeout (#6379)", () => { const probe = classifyCredentialResolutionProbe( { status: 0, stdout: probeStdout({ curlExit: 28 }), stderr: "" }, diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts index ccdedd26aa2..18b2c7d7b89 100644 --- a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts @@ -269,6 +269,9 @@ function transportDetail(curlExit: number, stderr: string): string | undefined { if (curlExit === 56 && /CONNECT tunnel failed,\s*response 403/i.test(stderr)) { return "OpenShell denied the probe connection (CONNECT 403); check the generated MCP policy"; } + if (curlExit === 56 && /CONNECT tunnel failed,\s*response 503/i.test(stderr)) { + return "OpenShell denied the probe before TLS setup (CONNECT 503); check gateway TLS termination and credential-rewrite readiness"; + } if (curlExit === 28) return `probe timed out after ${PROBE_CURL_MAX_TIME_SECONDS}s`; return undefined; } diff --git a/src/lib/actions/sandbox/mcp-bridge-validation.ts b/src/lib/actions/sandbox/mcp-bridge-validation.ts index 34d015da2d1..0de09d61ec1 100644 --- a/src/lib/actions/sandbox/mcp-bridge-validation.ts +++ b/src/lib/actions/sandbox/mcp-bridge-validation.ts @@ -28,6 +28,7 @@ export { const VALID_SERVER_RE = /^[A-Za-z][A-Za-z0-9_-]{0,63}$/; const VALID_ENV_RE = /^[A-Za-z_][A-Za-z0-9_]{0,127}$/; +const OPENSHELL_REVISIONED_CREDENTIAL_NAME_RE = /^v[0-9]+_[A-Za-z0-9_]+$/; const VALID_SANDBOX_RE = /^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/; const OPENSHELL_VERSION_OUTPUT_RE = /^openshell\s+([0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?)$/; @@ -178,6 +179,12 @@ export function validateMcpServerName(name: string): void { export function validateMcpCredentialEnvName(name: string): void { validatePersistedMcpCredentialEnvName(name); + if (OPENSHELL_REVISIONED_CREDENTIAL_NAME_RE.test(name)) { + throw new McpBridgeError( + `MCP credential environment name '${name}' is reserved for OpenShell credential revisions and would be skipped instead of attached. Use a dedicated secret name such as MY_SERVICE_MCP_TOKEN.`, + 2, + ); + } if (isSubprocessEnvNameAllowed(name)) { throw new McpBridgeError( `MCP credential environment name '${name}' is reserved for host subprocess control and could be forwarded outside the provider mutation. Use a dedicated secret name such as MY_SERVICE_MCP_TOKEN.`, diff --git a/test/hermes-mcp-config-transaction.test.ts b/test/hermes-mcp-config-transaction.test.ts index 34f76081bdb..8cacb71743b 100644 --- a/test/hermes-mcp-config-transaction.test.ts +++ b/test/hermes-mcp-config-transaction.test.ts @@ -185,12 +185,18 @@ print(json.dumps({"ok": True})) expect(JSON.parse(result.stdout)).toEqual({ ok: true }); }); - it("shares the host credential-name boundary while preserving exact cleanup", () => { - // One representative from each production category: OpenShell raw child - // value, OpenShell rewritten child value, exact process control, and - // process-control prefix. The exhaustive manifest-driven matrix lives at - // the shared TypeScript validator boundary. - const blockedNames = ["GCP_PROJECT_ID", "GCE_METADATA_HOST", "PATH", "NEMOCLAW_MCP_TOKEN"]; + it("shares the host credential-name boundary while preserving exact cleanup (#6379)", () => { + // Representatives cover OpenShell raw and rewritten child values, exact process control, + // process-control prefix, and revisioned placeholder namespace. The + // exhaustive manifest-driven matrix lives at the shared TypeScript + // validator boundary. + const blockedNames = [ + "GCP_PROJECT_ID", + "GCE_METADATA_HOST", + "PATH", + "NEMOCLAW_MCP_TOKEN", + "v10_GITHUB_TOKEN", + ]; const result = runPython( ` import importlib.util, json, sys From 2bd7c7c6bf086b1852fb0c298d9c38ed8d7f670b Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 15:39:19 -0700 Subject: [PATCH 03/50] docs(mcp): narrow CONNECT 503 diagnosis Signed-off-by: Aaron Erickson --- docs/deployment/set-up-mcp-bridge.mdx | 4 ++-- src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts | 4 ++-- src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/deployment/set-up-mcp-bridge.mdx b/docs/deployment/set-up-mcp-bridge.mdx index 7eb96528b5a..b8e1a437ef6 100644 --- a/docs/deployment/set-up-mcp-bridge.mdx +++ b/docs/deployment/set-up-mcp-bridge.mdx @@ -325,8 +325,8 @@ For an identical HTTP 401 or 403, a confirmed-valid credential settles it: the O An identical HTTP 400 stays inconclusive even with a valid credential, because the endpoint may reject the probe's `initialize` request itself; compare `mcp status` for the same server on a known-good host — if that host verifies, suspect this host's placeholder rewrite. That is a host-side OpenShell defect rather than a NemoClaw registration problem: verify the OpenShell installation on the host (tracked upstream as OpenShell issue 2161). A `credential resolution: unknown` verdict with an endpoint or policy detail means the probe could not reach a judgment; fix the reported endpoint or policy condition and rerun `mcp status `. -A detail containing `CONNECT 503` means OpenShell failed closed before TLS setup because gateway TLS termination or credential rewriting was unavailable. -Inspect the OpenShell gateway's ephemeral CA initialization and provider environment, repair the reported condition, and rerun `mcp status `. +A detail containing `CONNECT 503` means OpenShell failed closed before TLS setup because gateway TLS termination state was unavailable. +Inspect the OpenShell gateway's ephemeral CA generation and CA-file initialization, repair the reported condition, and rerun `mcp status `. If `restart` reports a missing provider and the original credential is not registered in OpenShell, export the same variable name used during `add` and retry. diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts index 73cf334bea6..dc461683db5 100644 --- a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts @@ -228,7 +228,7 @@ describe("MCP credential-resolution probe classification", () => { expect(probe.detail).toContain("CONNECT 403"); }); - it("classifies a CONNECT-level proxy 503 as unavailable credential rewriting (#6379)", () => { + it("classifies a CONNECT-level proxy 503 as unavailable TLS termination (#6379)", () => { const probe = classifyCredentialResolutionProbe( { status: 0, @@ -239,7 +239,7 @@ describe("MCP credential-resolution probe classification", () => { ); expect(probe.ok).toBeNull(); expect(probe.detail).toContain("CONNECT 503"); - expect(probe.detail).toContain("credential-rewrite readiness"); + expect(probe.detail).toContain("ephemeral CA initialization"); }); it("classifies curl exit 28 as an indeterminate probe timeout (#6379)", () => { diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts index 18b2c7d7b89..caf40f3f34a 100644 --- a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts @@ -270,7 +270,7 @@ function transportDetail(curlExit: number, stderr: string): string | undefined { return "OpenShell denied the probe connection (CONNECT 403); check the generated MCP policy"; } if (curlExit === 56 && /CONNECT tunnel failed,\s*response 503/i.test(stderr)) { - return "OpenShell denied the probe before TLS setup (CONNECT 503); check gateway TLS termination and credential-rewrite readiness"; + return "OpenShell denied the probe before TLS setup (CONNECT 503); check gateway ephemeral CA initialization and TLS termination readiness"; } if (curlExit === 28) return `probe timed out after ${PROBE_CURL_MAX_TIME_SECONDS}s`; return undefined; From 3a67bc3bd79ca43bccd835e07db7e42e5bf0c5bd Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 12:47:29 -0700 Subject: [PATCH 04/50] chore(openshell): prepare 0.0.82 dependency upgrade Signed-off-by: Aaron Erickson From 9213896ee9b5e670823cb4bc6c8ecf08534163a8 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 13:01:26 -0700 Subject: [PATCH 05/50] fix(mcp): align credential diagnostics with OpenShell main Signed-off-by: Aaron Erickson --- agents/hermes/mcp-config-transaction.py | 4 +++- docs/deployment/set-up-mcp-bridge.mdx | 4 ++++ .../mcp-bridge-input-validation.test.ts | 22 +++++++++++++++++++ .../mcp-bridge-resolution-probe.test.ts | 14 ++++++++++++ .../sandbox/mcp-bridge-resolution-probe.ts | 3 +++ .../actions/sandbox/mcp-bridge-validation.ts | 7 ++++++ test/hermes-mcp-config-transaction.test.ts | 18 ++++++++++----- 7 files changed, 65 insertions(+), 7 deletions(-) diff --git a/agents/hermes/mcp-config-transaction.py b/agents/hermes/mcp-config-transaction.py index d848a26c6ac..8ac4bd97e73 100755 --- a/agents/hermes/mcp-config-transaction.py +++ b/agents/hermes/mcp-config-transaction.py @@ -57,6 +57,7 @@ ENV_PLACEHOLDER_RE = re.compile( r"^Bearer openshell:resolve:env:([A-Za-z_][A-Za-z0-9_]{0,127})$" ) +OPENSHELL_REVISIONED_CREDENTIAL_NAME_RE = re.compile(r"^v[0-9]+_[A-Za-z0-9_]+$") BOUNDARY_MANIFEST_NAME = "openshell-child-visible-credentials.v0.0.72.json" ANSI_ESCAPE_RE = re.compile( r"\x1b(?:\[[0-?]*[ -/]*[@-~]|\][^\x07]*(?:\x07|\x1b\\)|[@-_])" @@ -174,7 +175,8 @@ def _manifest_strings(manifest: dict[str, object], key: str) -> frozenset[str]: def _credential_name_is_reserved(name: str) -> bool: return ( - name in _RAW_CHILD_VALUE_KEYS + OPENSHELL_REVISIONED_CREDENTIAL_NAME_RE.fullmatch(name) is not None + or name in _RAW_CHILD_VALUE_KEYS or name in _REWRITTEN_CHILD_VALUE_KEYS or name in _RUNTIME_CONTROL_KEYS or any(name.startswith(prefix) for prefix in _RUNTIME_CONTROL_PREFIXES) diff --git a/docs/deployment/set-up-mcp-bridge.mdx b/docs/deployment/set-up-mcp-bridge.mdx index d064394bc54..7eb96528b5a 100644 --- a/docs/deployment/set-up-mcp-bridge.mdx +++ b/docs/deployment/set-up-mcp-bridge.mdx @@ -88,6 +88,8 @@ The child-visible compatibility list is pinned to OpenShell `v0.0.72` commit `8c Choose a dedicated name such as `MY_SERVICE_MCP_TOKEN`. NemoClaw also rejects host subprocess control names such as `PATH`, proxy/TLS variables, and `OPENSHELL_*`, `GRPC_*`, `LC_*`, or `XDG_*` keys so the selected credential cannot be inherited by unrelated OpenShell commands. Loader, shell, language, and agent runtime controls such as `LD_PRELOAD`, `BASH_ENV`, `NODE_OPTIONS`, `PYTHONHOME`, `NEMOCLAW_*`, and `OPENCLAW_*` are rejected as well because OpenShell attaches provider keys to fresh sandbox execs; use a dedicated service name such as `MY_SERVICE_MCP_TOKEN`. +OpenShell reserves credential names matching `v[0-9]+_[A-Za-z0-9_]+` for revisioned placeholders. +NemoClaw rejects names such as `v10_GITHUB_TOKEN` because OpenShell skips them instead of attaching a credential resolver. NemoClaw requires exactly one `--env` bearer credential per server. Every endpoint must use HTTPS. @@ -323,6 +325,8 @@ For an identical HTTP 401 or 403, a confirmed-valid credential settles it: the O An identical HTTP 400 stays inconclusive even with a valid credential, because the endpoint may reject the probe's `initialize` request itself; compare `mcp status` for the same server on a known-good host — if that host verifies, suspect this host's placeholder rewrite. That is a host-side OpenShell defect rather than a NemoClaw registration problem: verify the OpenShell installation on the host (tracked upstream as OpenShell issue 2161). A `credential resolution: unknown` verdict with an endpoint or policy detail means the probe could not reach a judgment; fix the reported endpoint or policy condition and rerun `mcp status `. +A detail containing `CONNECT 503` means OpenShell failed closed before TLS setup because gateway TLS termination or credential rewriting was unavailable. +Inspect the OpenShell gateway's ephemeral CA initialization and provider environment, repair the reported condition, and rerun `mcp status `. If `restart` reports a missing provider and the original credential is not registered in OpenShell, export the same variable name used during `add` and retry. diff --git a/src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts b/src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts index 1128356eb6e..250f84d5ed9 100644 --- a/src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-input-validation.test.ts @@ -39,6 +39,28 @@ describe("MCP CLI input validation", () => { ).toThrow(/process arguments and shell history/); }); + it("rejects OpenShell revisioned placeholder names as MCP credentials (#6379)", () => { + for (const name of ["v1_TOKEN", "v999999_very_unlikely", "v0_1"]) { + expect(() => + parseMcpAddArgs(["github", "--url", "https://mcp.example.test/mcp", "--env", name]), + ).toThrow(/reserved for OpenShell credential revisions/); + expect(() => resolveCredentialEnv([{ name, value: "host-only-secret" }])).toThrow( + /would be skipped instead of attached/, + ); + expect(() => + buildMcpBridgeProviderArgs("create", "provider", [{ name }], { + [name]: "host-only-secret", + }), + ).toThrow(/reserved for OpenShell credential revisions/); + } + + for (const name of ["v_TOKEN", "v10_", "versioned_token", "V10_TOKEN"]) { + expect(() => + parseMcpAddArgs(["github", "--url", "https://mcp.example.test/mcp", "--env", name]), + ).not.toThrow(); + } + }); + // source-shape-contract: compatibility -- Pinned OpenShell child-visible keys must drive credential rejection through every MCP boundary it("rejects OpenShell child-environment compatibility keys as MCP credentials", () => { for (const name of childVisibleCredentialManifest.rawChildValueKeys) { diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts index 7592d7121ef..73cf334bea6 100644 --- a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts @@ -228,6 +228,20 @@ describe("MCP credential-resolution probe classification", () => { expect(probe.detail).toContain("CONNECT 403"); }); + it("classifies a CONNECT-level proxy 503 as unavailable credential rewriting (#6379)", () => { + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: probeStdout({ curlExit: 56 }), + stderr: "curl: (56) CONNECT tunnel failed, response 503", + }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(probe.detail).toContain("CONNECT 503"); + expect(probe.detail).toContain("credential-rewrite readiness"); + }); + it("classifies curl exit 28 as an indeterminate probe timeout (#6379)", () => { const probe = classifyCredentialResolutionProbe( { status: 0, stdout: probeStdout({ curlExit: 28 }), stderr: "" }, diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts index ccdedd26aa2..18b2c7d7b89 100644 --- a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts @@ -269,6 +269,9 @@ function transportDetail(curlExit: number, stderr: string): string | undefined { if (curlExit === 56 && /CONNECT tunnel failed,\s*response 403/i.test(stderr)) { return "OpenShell denied the probe connection (CONNECT 403); check the generated MCP policy"; } + if (curlExit === 56 && /CONNECT tunnel failed,\s*response 503/i.test(stderr)) { + return "OpenShell denied the probe before TLS setup (CONNECT 503); check gateway TLS termination and credential-rewrite readiness"; + } if (curlExit === 28) return `probe timed out after ${PROBE_CURL_MAX_TIME_SECONDS}s`; return undefined; } diff --git a/src/lib/actions/sandbox/mcp-bridge-validation.ts b/src/lib/actions/sandbox/mcp-bridge-validation.ts index 34d015da2d1..0de09d61ec1 100644 --- a/src/lib/actions/sandbox/mcp-bridge-validation.ts +++ b/src/lib/actions/sandbox/mcp-bridge-validation.ts @@ -28,6 +28,7 @@ export { const VALID_SERVER_RE = /^[A-Za-z][A-Za-z0-9_-]{0,63}$/; const VALID_ENV_RE = /^[A-Za-z_][A-Za-z0-9_]{0,127}$/; +const OPENSHELL_REVISIONED_CREDENTIAL_NAME_RE = /^v[0-9]+_[A-Za-z0-9_]+$/; const VALID_SANDBOX_RE = /^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/; const OPENSHELL_VERSION_OUTPUT_RE = /^openshell\s+([0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?)$/; @@ -178,6 +179,12 @@ export function validateMcpServerName(name: string): void { export function validateMcpCredentialEnvName(name: string): void { validatePersistedMcpCredentialEnvName(name); + if (OPENSHELL_REVISIONED_CREDENTIAL_NAME_RE.test(name)) { + throw new McpBridgeError( + `MCP credential environment name '${name}' is reserved for OpenShell credential revisions and would be skipped instead of attached. Use a dedicated secret name such as MY_SERVICE_MCP_TOKEN.`, + 2, + ); + } if (isSubprocessEnvNameAllowed(name)) { throw new McpBridgeError( `MCP credential environment name '${name}' is reserved for host subprocess control and could be forwarded outside the provider mutation. Use a dedicated secret name such as MY_SERVICE_MCP_TOKEN.`, diff --git a/test/hermes-mcp-config-transaction.test.ts b/test/hermes-mcp-config-transaction.test.ts index 34f76081bdb..8cacb71743b 100644 --- a/test/hermes-mcp-config-transaction.test.ts +++ b/test/hermes-mcp-config-transaction.test.ts @@ -185,12 +185,18 @@ print(json.dumps({"ok": True})) expect(JSON.parse(result.stdout)).toEqual({ ok: true }); }); - it("shares the host credential-name boundary while preserving exact cleanup", () => { - // One representative from each production category: OpenShell raw child - // value, OpenShell rewritten child value, exact process control, and - // process-control prefix. The exhaustive manifest-driven matrix lives at - // the shared TypeScript validator boundary. - const blockedNames = ["GCP_PROJECT_ID", "GCE_METADATA_HOST", "PATH", "NEMOCLAW_MCP_TOKEN"]; + it("shares the host credential-name boundary while preserving exact cleanup (#6379)", () => { + // Representatives cover OpenShell raw and rewritten child values, exact process control, + // process-control prefix, and revisioned placeholder namespace. The + // exhaustive manifest-driven matrix lives at the shared TypeScript + // validator boundary. + const blockedNames = [ + "GCP_PROJECT_ID", + "GCE_METADATA_HOST", + "PATH", + "NEMOCLAW_MCP_TOKEN", + "v10_GITHUB_TOKEN", + ]; const result = runPython( ` import importlib.util, json, sys From 1fc5e393bc19740ac7c6c771d142181798e8f07b Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 15:39:19 -0700 Subject: [PATCH 06/50] docs(mcp): narrow CONNECT 503 diagnosis Signed-off-by: Aaron Erickson --- docs/deployment/set-up-mcp-bridge.mdx | 4 ++-- src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts | 4 ++-- src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/deployment/set-up-mcp-bridge.mdx b/docs/deployment/set-up-mcp-bridge.mdx index 7eb96528b5a..b8e1a437ef6 100644 --- a/docs/deployment/set-up-mcp-bridge.mdx +++ b/docs/deployment/set-up-mcp-bridge.mdx @@ -325,8 +325,8 @@ For an identical HTTP 401 or 403, a confirmed-valid credential settles it: the O An identical HTTP 400 stays inconclusive even with a valid credential, because the endpoint may reject the probe's `initialize` request itself; compare `mcp status` for the same server on a known-good host — if that host verifies, suspect this host's placeholder rewrite. That is a host-side OpenShell defect rather than a NemoClaw registration problem: verify the OpenShell installation on the host (tracked upstream as OpenShell issue 2161). A `credential resolution: unknown` verdict with an endpoint or policy detail means the probe could not reach a judgment; fix the reported endpoint or policy condition and rerun `mcp status `. -A detail containing `CONNECT 503` means OpenShell failed closed before TLS setup because gateway TLS termination or credential rewriting was unavailable. -Inspect the OpenShell gateway's ephemeral CA initialization and provider environment, repair the reported condition, and rerun `mcp status `. +A detail containing `CONNECT 503` means OpenShell failed closed before TLS setup because gateway TLS termination state was unavailable. +Inspect the OpenShell gateway's ephemeral CA generation and CA-file initialization, repair the reported condition, and rerun `mcp status `. If `restart` reports a missing provider and the original credential is not registered in OpenShell, export the same variable name used during `add` and retry. diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts index 73cf334bea6..dc461683db5 100644 --- a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts @@ -228,7 +228,7 @@ describe("MCP credential-resolution probe classification", () => { expect(probe.detail).toContain("CONNECT 403"); }); - it("classifies a CONNECT-level proxy 503 as unavailable credential rewriting (#6379)", () => { + it("classifies a CONNECT-level proxy 503 as unavailable TLS termination (#6379)", () => { const probe = classifyCredentialResolutionProbe( { status: 0, @@ -239,7 +239,7 @@ describe("MCP credential-resolution probe classification", () => { ); expect(probe.ok).toBeNull(); expect(probe.detail).toContain("CONNECT 503"); - expect(probe.detail).toContain("credential-rewrite readiness"); + expect(probe.detail).toContain("ephemeral CA initialization"); }); it("classifies curl exit 28 as an indeterminate probe timeout (#6379)", () => { diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts index 18b2c7d7b89..caf40f3f34a 100644 --- a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts @@ -270,7 +270,7 @@ function transportDetail(curlExit: number, stderr: string): string | undefined { return "OpenShell denied the probe connection (CONNECT 403); check the generated MCP policy"; } if (curlExit === 56 && /CONNECT tunnel failed,\s*response 503/i.test(stderr)) { - return "OpenShell denied the probe before TLS setup (CONNECT 503); check gateway TLS termination and credential-rewrite readiness"; + return "OpenShell denied the probe before TLS setup (CONNECT 503); check gateway ephemeral CA initialization and TLS termination readiness"; } if (curlExit === 28) return `probe timed out after ${PROBE_CURL_MAX_TIME_SECONDS}s`; return undefined; From 6e904399fd2b8333c7aa2424f81b4702f28ccbd1 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 20:05:45 -0700 Subject: [PATCH 07/50] feat(exec): support multiline OpenShell commands Signed-off-by: Aaron Erickson --- docs/reference/commands.mdx | 30 +- src/commands/sandbox/exec.test.ts | 14 +- src/commands/sandbox/exec.ts | 2 +- .../sandbox/exec.multiline-argv.test.ts | 197 ++++++++++ .../sandbox/exec.multiline-guard.test.ts | 345 ------------------ src/lib/actions/sandbox/exec.test.ts | 6 +- src/lib/actions/sandbox/exec.ts | 90 +---- src/lib/actions/sandbox/runtime-env.test.ts | 23 ++ 8 files changed, 268 insertions(+), 439 deletions(-) create mode 100644 src/lib/actions/sandbox/exec.multiline-argv.test.ts delete mode 100644 src/lib/actions/sandbox/exec.multiline-guard.test.ts diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index ab912bc7a07..986b8c6569f 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -894,11 +894,16 @@ printf 'hello\n' | $$nemoclaw my-assistant exec --stdin -- cat ssh dgx-spark '$$nemoclaw my-assistant exec --no-stdin -- pwd' ``` -The OpenShell exec endpoint rejects any command argument (the values after `--`) that contains a newline or carriage return, so multi-line commands such as a `bash` heredoc cannot be passed through `exec`. -NemoClaw detects this before dispatch, names the offending argument position, and exits with status `2` instead of surfacing the lower-level OpenShell `InvalidArgument` error. -Join the statements with semicolons (`$$nemoclaw exec -- bash -lc "cmd1; cmd2"`). -Pipe the script into the sandbox shell over stdin (`printf 'cmd1\ncmd2\n' | $$nemoclaw exec --stdin -- bash`). -Or write the script to a file in the sandbox and run it (`$$nemoclaw exec -- bash `). +OpenShell preserves line endings and quote characters inside each command argument, so inline scripts and heredocs can be passed as one argument after `--`. +For example, a shell variable keeps the multi-line script in one argv element: + +```bash +script=$'cat <<\'EOF\'\nline one\nline two\nEOF' +$$nemoclaw exec -- bash -lc "$script" +``` + +NUL bytes are still rejected in command arguments. +Line breaks are accepted only in command argv: `--workdir` remains single-line, and NemoClaw does not expose OpenShell request-environment injection on this command. | Flag | Description | |------|-------------| @@ -1367,11 +1372,16 @@ By default, NemoClaw inherits caller stdin only when it is a terminal. Non-terminal or unavailable stdin is closed so SSH, CI, and other one-shot commands cannot wait on an inherited pipe. Pass `--stdin` to forward an intentional pipe, or `--no-stdin` to close terminal stdin explicitly. -The OpenShell exec endpoint rejects any command argument (the values after `--`) that contains a newline or carriage return, so multi-line commands such as a `bash` heredoc cannot be passed through `exec`. -NemoClaw detects this before dispatch, names the offending argument position, and exits with status `2` instead of surfacing the lower-level OpenShell `InvalidArgument` error. -Join the statements with semicolons (`$$nemoclaw exec -- bash -lc "cmd1; cmd2"`). -Pipe the script into the sandbox shell over stdin (`printf 'cmd1\ncmd2\n' | $$nemoclaw exec --stdin -- bash`). -Or write the script to a file in the sandbox and run it (`$$nemoclaw exec -- bash `). +OpenShell preserves line endings and quote characters inside each command argument, so inline scripts and heredocs can be passed as one argument after `--`. +For example, a shell variable keeps the multi-line script in one argv element: + +```bash +script=$'cat <<\'EOF\'\nline one\nline two\nEOF' +$$nemoclaw exec -- bash -lc "$script" +``` + +NUL bytes are still rejected in command arguments. +Line breaks are accepted only in command argv: `--workdir` remains single-line, and NemoClaw does not expose OpenShell request-environment injection on this command. | Flag | Description | |------|-------------| diff --git a/src/commands/sandbox/exec.test.ts b/src/commands/sandbox/exec.test.ts index 73b1a536d6b..05a0c8522c2 100644 --- a/src/commands/sandbox/exec.test.ts +++ b/src/commands/sandbox/exec.test.ts @@ -112,10 +112,9 @@ describe("SandboxExecCommand oclif parse path", () => { }); }); - it("forwards a multi-line heredoc command verbatim to the action guard (#5980)", async () => { - // The command layer forwards argv unchanged; execSandbox() applies the - // newline guard (exit 2 before dispatch), which is asserted directly in the - // action test. Here we pin that the heredoc reaches the action intact. + it("forwards a multi-line heredoc command verbatim to the action", async () => { + // The action dispatches this exact argument through OpenShell. Its + // byte-preserving boundary is asserted directly in the action test. const heredoc = "cat < { }); }); - it("forwards the semicolon workaround to dispatch (#5980)", async () => { - // Mirrors the action-layer "forwards the semicolon workaround to dispatch" - // test: the single-line semicolon-joined command carries no newline, so the - // command layer hands it to execSandbox() unchanged, which then dispatches. + it("forwards a semicolon-joined command unchanged", async () => { await SandboxExecCommand.run(["alpha", "--", "bash", "-lc", "echo line1; echo line2"], rootDir); expect(execSandboxMock).toHaveBeenCalledWith( "alpha", @@ -138,7 +134,7 @@ describe("SandboxExecCommand oclif parse path", () => { ); }); - it("preserves --workdir and forwards a single-line command unchanged (#5980)", async () => { + it("preserves --workdir and forwards a single-line command unchanged", async () => { await SandboxExecCommand.run( ["alpha", "--workdir", "/sandbox", "--", "bash", "-lc", "echo line1; echo line2"], rootDir, diff --git a/src/commands/sandbox/exec.ts b/src/commands/sandbox/exec.ts index f79eb5165ef..183062dfdb4 100644 --- a/src/commands/sandbox/exec.ts +++ b/src/commands/sandbox/exec.ts @@ -10,7 +10,7 @@ export default class SandboxExecCommand extends NemoClawCommand { static strict = false; static summary = "Run a command non-interactively in a running sandbox"; static description = - "Run a single command inside a running sandbox via the OpenShell exec endpoint. The command runs as the sandbox user (HOME=/sandbox) and exits with the remote command's exit code. Use `--` to separate exec options from the user command. Stdin is inherited by default only when it is a terminal; pass `--stdin` to forward an intentional pipe."; + "Run a single command inside a running sandbox via the OpenShell exec endpoint. The command runs as the sandbox user (HOME=/sandbox) and exits with the remote command's exit code. Use `--` to separate exec options from the user command; arguments after it preserve embedded line endings and quotes. NUL bytes are rejected, and `--workdir` must remain single-line. Stdin is inherited by default only when it is a terminal; pass `--stdin` to forward an intentional pipe."; static usage = [ " [--workdir ] [--tty|--no-tty] [--timeout ] [--stdin|--no-stdin] -- [args...]", ]; diff --git a/src/lib/actions/sandbox/exec.multiline-argv.test.ts b/src/lib/actions/sandbox/exec.multiline-argv.test.ts new file mode 100644 index 00000000000..8e020a0e95b --- /dev/null +++ b/src/lib/actions/sandbox/exec.multiline-argv.test.ts @@ -0,0 +1,197 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawn } from "node:child_process"; +import { EventEmitter } from "node:events"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +// The default exec runner shells out via spawn and chooses whether to inherit +// or ignore stdin. Mock node:child_process so the tests can assert that wiring +// at the execSandbox boundary without spawning a real process. Every other test +// injects a runner/probe seam. +vi.mock("node:child_process", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, spawn: vi.fn() }; +}); + +import { buildOpenshellExecArgs, execSandbox, wrapExecCommandWithRuntimeEnv } from "./exec"; + +function expectedExecArgs(sandboxName: string, command: readonly string[]): string[] { + return buildOpenshellExecArgs(sandboxName, wrapExecCommandWithRuntimeEnv(command)); +} + +function exitWithCode(): ReturnType { + return vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`exit:${code}`); + }) as never); +} + +describe("execSandbox multi-line argv", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it.each([ + { + label: "LF", + command: ["python3", "-c", "print('one')\nprint('two')"], + }, + { + label: "CRLF", + command: ["python3", "-c", "print('one')\r\nprint('two')"], + }, + { + label: "embedded single and double quotes", + command: ["python3", "-c", 'print("it\'s byte-exact")\nprint(\'a \\"quote\\"\')'], + }, + { + label: "heredoc", + command: ["bash", "-lc", "cat <<'EOF'\nline one\nline 'two'\nEOF"], + }, + ])("forwards $label bytes unchanged through the OpenShell argv boundary", async ({ command }) => { + const exitSpy = exitWithCode(); + vi.spyOn(console, "error").mockImplementation(() => {}); + const run = vi.fn((_binary: string, _args: readonly string[]) => ({ status: 0 })); + + await expect( + execSandbox("multiline-test", command, {}, { run, resolveBinary: () => "openshell" }), + ).rejects.toThrow("exit:0"); + + expect(run).toHaveBeenCalledOnce(); + expect(run).toHaveBeenCalledWith("openshell", expectedExecArgs("multiline-test", command)); + const forwarded = vi.mocked(run).mock.calls[0][1]; + expect(forwarded.slice(-command.length)).toEqual(command); + expect(exitSpy).toHaveBeenCalledWith(0); + }); + + it("still rejects a NUL-bearing command argument before dispatch", async () => { + const exitSpy = exitWithCode(); + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const run = vi.fn(() => ({ status: 0 })); + + await expect(execSandbox("multiline-test", ["printf", "a\0b"], {}, { run })).rejects.toThrow( + "exit:2", + ); + + expect(run).not.toHaveBeenCalled(); + expect(errSpy).toHaveBeenCalledWith( + "error: command argument 2 contains a NUL byte, which OpenShell exec does not accept", + ); + expect(exitSpy).toHaveBeenCalledWith(2); + }); + + it.each([ + "/sandbox/line-one\nline-two", + "/sandbox/line-one\rline-two", + "/sandbox/line-one\r\nline-two", + ])("still rejects a multi-line --workdir before probing or dispatch: %j", async (workdir) => { + const exitSpy = exitWithCode(); + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const probeWorkdir = vi.fn(() => ({ status: 0 })); + const run = vi.fn(() => ({ status: 0 })); + + await expect( + execSandbox( + "multiline-test", + ["pwd"], + { workdir }, + { run, resolveBinary: () => "openshell", probeWorkdir }, + ), + ).rejects.toThrow("exit:2"); + + expect(probeWorkdir).not.toHaveBeenCalled(); + expect(run).not.toHaveBeenCalled(); + expect(errSpy).toHaveBeenCalledWith( + "error: --workdir must not contain newlines or carriage returns", + ); + expect(exitSpy).toHaveBeenCalledWith(2); + }); + + it("still rejects a NUL-bearing --workdir before probing or dispatch", async () => { + const exitSpy = exitWithCode(); + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const probeWorkdir = vi.fn(() => ({ status: 0 })); + const run = vi.fn(() => ({ status: 0 })); + + await expect( + execSandbox( + "multiline-test", + ["pwd"], + { workdir: "/sandbox/a\0b" }, + { run, resolveBinary: () => "openshell", probeWorkdir }, + ), + ).rejects.toThrow("exit:2"); + + expect(probeWorkdir).not.toHaveBeenCalled(); + expect(run).not.toHaveBeenCalled(); + expect(errSpy).toHaveBeenCalledWith("error: --workdir must not contain NUL bytes"); + expect(exitSpy).toHaveBeenCalledWith(2); + }); + + it("does not populate OpenShell's separately validated request-environment field", () => { + const argv = buildOpenshellExecArgs("multiline-test", ["printf", "line one\nline two"]); + + // NemoClaw's public exec surface has no request-environment option. Runtime + // metadata is sourced inside the command wrapper, so allowing line breaks + // in command argv cannot broaden OpenShell's environment-value contract. + expect(argv).not.toContain("--env"); + expect(argv.slice(-2)).toEqual(["printf", "line one\nline two"]); + }); + + it("still validates a single-line --workdir before dispatch", async () => { + const exitSpy = exitWithCode(); + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const probeWorkdir = vi.fn(() => ({ status: 1 })); + const run = vi.fn(() => ({ status: 0 })); + + await expect( + execSandbox( + "multiline-test", + ["pwd"], + { workdir: "/no/such/dir" }, + { run, resolveBinary: () => "openshell", probeWorkdir }, + ), + ).rejects.toThrow("exit:1"); + + expect(probeWorkdir).toHaveBeenCalled(); + expect(run).not.toHaveBeenCalled(); + expect(errSpy).toHaveBeenCalledWith( + "error: --workdir: /no/such/dir does not exist inside the sandbox", + ); + expect(exitSpy).toHaveBeenCalledWith(1); + }); + + it.each([ + { label: "inherits stdin after explicit --stdin", stdin: true, expectedStdio: "inherit" }, + { + label: "closes stdin after explicit --no-stdin", + stdin: false, + expectedStdio: ["ignore", "inherit", "inherit"], + }, + ])("dispatches the default runner and $label", async ({ stdin, expectedStdio }) => { + const childEvents = new EventEmitter(); + const child = { + exitCode: null, + signalCode: null, + kill: vi.fn(), + once: ((event: string, listener: (...args: unknown[]) => void) => + childEvents.once(event, listener)) as never, + }; + vi.mocked(spawn).mockReset(); + vi.mocked(spawn).mockImplementation(((): never => { + queueMicrotask(() => childEvents.emit("close", 0, null)); + return child as never; + }) as never); + const exitSpy = exitWithCode(); + vi.spyOn(console, "error").mockImplementation(() => {}); + + await expect( + execSandbox("multiline-test", ["bash"], { stdin }, { resolveBinary: () => "openshell" }), + ).rejects.toThrow("exit:0"); + + expect(spawn).toHaveBeenCalledWith("openshell", expectedExecArgs("multiline-test", ["bash"]), { + stdio: expectedStdio, + }); + expect(exitSpy).toHaveBeenCalledWith(0); + }); +}); diff --git a/src/lib/actions/sandbox/exec.multiline-guard.test.ts b/src/lib/actions/sandbox/exec.multiline-guard.test.ts deleted file mode 100644 index fe7047da36c..00000000000 --- a/src/lib/actions/sandbox/exec.multiline-guard.test.ts +++ /dev/null @@ -1,345 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -import { spawn } from "node:child_process"; -import { EventEmitter } from "node:events"; -import { afterEach, describe, expect, it, vi } from "vitest"; - -// The default exec runner shells out via spawn and chooses whether to inherit -// or ignore stdin. Mock node:child_process so the tests can assert that wiring -// at the execSandbox boundary without spawning a real process. Every other test -// injects a runner/probe seam. -vi.mock("node:child_process", async (importOriginal) => { - const actual = await importOriginal(); - return { ...actual, spawn: vi.fn() }; -}); - -// execSandbox dynamically requires the OpenShell binary lookup, which exits the -// process when OpenShell is absent. The dispatch-path tests inject a -// resolveBinary seam (plus a runner and workdir probe) so they stay hermetic -// without spawning a real process or hitting that process-exiting lookup. -import { - buildOpenshellExecArgs, - execSandbox, - findMultilineExecArg, - multilineExecMessage, - wrapExecCommandWithRuntimeEnv, -} from "./exec"; - -function expectedExecArgs(sandboxName: string, command: readonly string[]): string[] { - return buildOpenshellExecArgs(sandboxName, wrapExecCommandWithRuntimeEnv(command)); -} - -describe("findMultilineExecArg", () => { - it("returns -1 when every argument is single-line", () => { - expect(findMultilineExecArg(["bash", "-lc", "echo line1; echo line2"])).toBe(-1); - }); - - it("returns the index of the first argument containing a newline", () => { - expect(findMultilineExecArg(["bash", "-lc", "cat < { - expect(findMultilineExecArg(["printf", "a\rb"])).toBe(1); - }); - - it("treats Unicode line separators (U+2028/U+2029) as single-line because OpenShell rejects only CR/LF", () => { - // The guard deliberately mirrors OpenShell's CR/LF-only rejection, so these - // code points are valid argv that dispatch unchanged. Broadening the guard - // to match them would reject commands OpenShell would otherwise run. - expect(findMultilineExecArg(["printf", "a\u2028b"])).toBe(-1); - expect(findMultilineExecArg(["printf", "a\u2029b"])).toBe(-1); - }); - - it("reports the earliest offending argument when several are multi-line", () => { - expect(findMultilineExecArg(["a", "b\nc", "d\ne"])).toBe(1); - }); -}); - -describe("multilineExecMessage", () => { - it("names the 1-based argument position and offers the semicolon, pipe, and script workarounds", () => { - const message = multilineExecMessage( - "nemoclaw", - "bug5980test", - ["bash", "-lc", "cat <"); - }); - - it("uses the active CLI name so the Hermes surface gets nemohermes guidance", () => { - const message = multilineExecMessage("nemohermes", "alpha", ["bash", "-lc", "a\nb"], 2); - expect(message).toContain("nemohermes alpha exec --stdin -- bash"); - expect(message).not.toContain("nemoclaw"); - }); - - it("describes the argument by size without echoing its contents (avoids leaking secrets)", () => { - // A multi-line value can carry pasted secrets; the message must never - // reproduce its contents. Use a neutral sentinel so the secret-scanner - // hooks do not flag the test fixture itself. - const sensitive = "SENSITIVE_LINE_ONE\nSENSITIVE_LINE_TWO\nSENSITIVE_LINE_THREE"; - const message = multilineExecMessage("nemoclaw", "alpha", ["bash", "-lc", sensitive], 2); - // The neutral size description appears... - expect(message).toContain(`${sensitive.length} characters spanning 3 lines`); - // ...but no fragment of the payload is ever printed. - expect(message).not.toContain("SENSITIVE_LINE"); - // Each line of the rendered message is itself free of stray carriage - // returns (the message is multi-line by design, joined with "\n"). - for (const line of message.split("\n")) { - expect(line).not.toMatch(/\r/); - } - }); - - it("uses singular units for a single-character single-line argument", () => { - const message = multilineExecMessage("nemoclaw", "alpha", ["printf", "\r"], 1); - expect(message).toContain("1 character spanning 2 lines"); - }); - - it("counts a trailing newline as a second (empty) line", () => { - // A single trailing "\n" splits into ["first", ""], so the size description - // reports 2 lines even though only one line carries text. This pins the - // documented bare-CR/trailing-break counting behavior. - const message = multilineExecMessage("nemoclaw", "alpha", ["bash", "-lc", "first\n"], 2); - expect(message).toContain("6 characters spanning 2 lines"); - }); -}); - -describe("execSandbox multi-line guard (#5980)", () => { - afterEach(() => { - vi.restoreAllMocks(); - }); - - it("rejects a multi-line command argument before dispatch with actionable guidance", async () => { - const exitSpy = vi.spyOn(process, "exit").mockImplementation(((_code?: number) => { - throw new Error(`exit:${_code}`); - }) as never); - const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const run = vi.fn(() => ({ status: 0 })); - - await expect( - execSandbox("bug5980test", ["bash", "-lc", "cat < String(call[0])).join("\n"); - expect(printed).toContain("contains a newline or carriage return"); - expect(printed).toContain('bash -lc "cmd1; cmd2"'); - }); - - it("forwards the semicolon workaround to dispatch and exits with the inner status", async () => { - // The reporter's confirmed workaround (`bash -lc "cmd1; cmd2"`) carries no - // newline/carriage return, so it passes the guard and dispatches. Injecting - // resolveBinary avoids the process-exiting OpenShell lookup, and the runner - // returns success so we can assert the argv forwarded and the exit code. - const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { - throw new Error(`exit:${code}`); - }) as never); - vi.spyOn(console, "error").mockImplementation(() => {}); - const run = vi.fn(() => ({ status: 0 })); - - await expect( - execSandbox( - "bug5980test", - ["bash", "-lc", "echo line1; echo line2"], - {}, - { run, resolveBinary: () => "openshell" }, - ), - ).rejects.toThrow("exit:0"); - - expect(run).toHaveBeenCalledWith( - "openshell", - expectedExecArgs("bug5980test", ["bash", "-lc", "echo line1; echo line2"]), - ); - expect(exitSpy).toHaveBeenCalledWith(0); - }); - - it("forwards a Unicode line-separator argument through dispatch (OpenShell accepts U+2028/U+2029; only CR/LF are guarded)", async () => { - // The guard mirrors OpenShell's CR/LF-only rejection, so an argument that - // carries U+2028 passes the guard and dispatches unchanged at the - // execSandbox boundary — confirming the documented assumption end-to-end on - // the dispatch path, not just in findMultilineExecArg. - const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { - throw new Error(`exit:${code}`); - }) as never); - vi.spyOn(console, "error").mockImplementation(() => {}); - const run = vi.fn(() => ({ status: 0 })); - - await expect( - execSandbox( - "bug5980test", - ["printf", "a\u2028b"], - {}, - { run, resolveBinary: () => "openshell" }, - ), - ).rejects.toThrow("exit:0"); - - expect(run).toHaveBeenCalledWith( - "openshell", - expectedExecArgs("bug5980test", ["printf", "a\u2028b"]), - ); - expect(exitSpy).toHaveBeenCalledWith(0); - }); - - it("still validates --workdir for a single-line command and fails with the workdir error, not the multi-line error", async () => { - // Guard ordering: the multi-line check runs before the workdir probe. A - // valid single-line command with a missing --workdir must surface the - // workdir error (exit 1), proving the workdir probe still runs after the - // guard and that the guard did not swallow the command. - const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { - throw new Error(`exit:${code}`); - }) as never); - const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const run = vi.fn(() => ({ status: 0 })); - const probeWorkdir = vi.fn(() => ({ status: 1 })); // `test -d` failure -> missing - - await expect( - execSandbox( - "alpha", - ["bash", "-lc", "echo ok"], - { workdir: "/no/such/dir" }, - { run, resolveBinary: () => "openshell", probeWorkdir }, - ), - ).rejects.toThrow("exit:1"); - - expect(probeWorkdir).toHaveBeenCalled(); - expect(exitSpy).toHaveBeenCalledWith(1); - const printed = errSpy.mock.calls.map((call) => String(call[0])).join("\n"); - expect(printed).toContain("does not exist inside the sandbox"); - expect(printed).not.toContain("newline or carriage return"); - // The workdir probe failed, so the command is never dispatched. - expect(run).not.toHaveBeenCalled(); - }); - - it("rejects a multi-line command before probing --workdir (guard runs first)", async () => { - // Ordering guarantee: when both a multi-line argv and --workdir are present, - // the multi-line guard must exit 2 *before* the workdir probe runs, so the - // probe is never reached and nothing is dispatched. - const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { - throw new Error(`exit:${code}`); - }) as never); - const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const run = vi.fn(() => ({ status: 0 })); - const probeWorkdir = vi.fn(() => ({ status: 0 })); - - await expect( - execSandbox( - "alpha", - ["bash", "-lc", "printf 'a\nb'"], - { workdir: "/workspace" }, - { run, resolveBinary: () => "openshell", probeWorkdir }, - ), - ).rejects.toThrow("exit:2"); - - expect(probeWorkdir).not.toHaveBeenCalled(); - expect(run).not.toHaveBeenCalled(); - expect(exitSpy).toHaveBeenCalledWith(2); - expect(errSpy.mock.calls.map((call) => String(call[0])).join("\n")).toContain( - "contains a newline or carriage return", - ); - }); - - it("forwards the stdin-pipe workaround argv to dispatch (script travels over stdin, not argv)", async () => { - // `printf 'cmd1\ncmd2\n' | nemoclaw exec --stdin -- bash` puts the - // multi-line script on stdin; the forwarded argv is just `bash` (no newline), - // so it passes the guard and dispatches. This test pins the argv shape only; - // the adjacent stdio test proves the runner forwards explicitly opted-in stdin. - const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { - throw new Error(`exit:${code}`); - }) as never); - vi.spyOn(console, "error").mockImplementation(() => {}); - const run = vi.fn(() => ({ status: 0 })); - - await expect( - execSandbox("bug5980test", ["bash"], {}, { run, resolveBinary: () => "openshell" }), - ).rejects.toThrow("exit:0"); - - expect(run).toHaveBeenCalledWith("openshell", expectedExecArgs("bug5980test", ["bash"])); - expect(exitSpy).toHaveBeenCalledWith(0); - }); - - it.each([ - { label: "inherits stdin after explicit --stdin", stdin: true, expectedStdio: "inherit" }, - { - label: "closes stdin after explicit --no-stdin", - stdin: false, - expectedStdio: ["ignore", "inherit", "inherit"], - }, - ])("dispatches the default runner and $label", async ({ stdin, expectedStdio }) => { - // Exercise the *default* runner (no injected `run`) so the assertion covers - // the production child-process wiring, not only the pure stdio selector. - const childEvents = new EventEmitter(); - const child = { - exitCode: null, - signalCode: null, - kill: vi.fn(), - once: ((event: string, listener: (...args: unknown[]) => void) => - childEvents.once(event, listener)) as never, - }; - vi.mocked(spawn).mockReset(); - vi.mocked(spawn).mockImplementation(((): never => { - // Resolve the runner once the close handler is registered. - queueMicrotask(() => childEvents.emit("close", 0, null)); - return child as never; - }) as never); - const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { - throw new Error(`exit:${code}`); - }) as never); - vi.spyOn(console, "error").mockImplementation(() => {}); - - await expect( - execSandbox("bug5980test", ["bash"], { stdin }, { resolveBinary: () => "openshell" }), - ).rejects.toThrow("exit:0"); - - expect(spawn).toHaveBeenCalledWith("openshell", expectedExecArgs("bug5980test", ["bash"]), { - stdio: expectedStdio, - }); - expect(exitSpy).toHaveBeenCalledWith(0); - }); - - it("forwards the script-file workaround to dispatch (bash )", async () => { - // `nemoclaw exec -- bash ` runs a script already written - // into the sandbox; the argv carries no newline and dispatches unchanged. - const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { - throw new Error(`exit:${code}`); - }) as never); - vi.spyOn(console, "error").mockImplementation(() => {}); - const run = vi.fn(() => ({ status: 0 })); - - await expect( - execSandbox( - "bug5980test", - ["bash", "/sandbox/run.sh"], - {}, - { run, resolveBinary: () => "openshell" }, - ), - ).rejects.toThrow("exit:0"); - - expect(run).toHaveBeenCalledWith( - "openshell", - expectedExecArgs("bug5980test", ["bash", "/sandbox/run.sh"]), - ); - expect(exitSpy).toHaveBeenCalledWith(0); - }); - - it("builds the forwarded argv unchanged for the single-line semicolon workaround", () => { - const command = ["bash", "-lc", "echo line1; echo line2"]; - expect(findMultilineExecArg(command)).toBe(-1); - expect(buildOpenshellExecArgs("bug5980test", command)).toEqual([ - "sandbox", - "exec", - "--name", - "bug5980test", - "--", - "bash", - "-lc", - "echo line1; echo line2", - ]); - }); -}); diff --git a/src/lib/actions/sandbox/exec.test.ts b/src/lib/actions/sandbox/exec.test.ts index 0ef2cb92203..3f1a38a2b83 100644 --- a/src/lib/actions/sandbox/exec.test.ts +++ b/src/lib/actions/sandbox/exec.test.ts @@ -3,9 +3,9 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -// The multi-line guard suites (findMultilineExecArg, multilineExecMessage, and -// the execSandbox dispatch guard for #5980) live in exec.multiline-guard.test.ts -// so this file stays focused on argv construction and the workdir probe. +// Multi-line command argv dispatch and field-specific rejection coverage lives +// in exec.multiline-argv.test.ts so this file stays focused on argv construction +// and the workdir probe. import { buildOpenshellExecArgs, buildWorkdirProbeArgs, diff --git a/src/lib/actions/sandbox/exec.ts b/src/lib/actions/sandbox/exec.ts index 680473d52d9..e578f853b2a 100644 --- a/src/lib/actions/sandbox/exec.ts +++ b/src/lib/actions/sandbox/exec.ts @@ -100,75 +100,23 @@ export function buildWorkdirProbeArgs(sandboxName: string, workdir: string): str return ["sandbox", "exec", "--name", sandboxName, "--", "test", "-d", workdir]; } -// OpenShell's `sandbox exec` rejects any argv element that contains a newline -// or carriage return ("command argument N contains newline or carriage return -// characters"). Multi-line commands such as heredocs therefore fail with a -// low-level InvalidArgument error that gives the reporter no NemoClaw-specific -// recovery path (#5980). We detect the offending argument before dispatch and -// fail with actionable guidance instead. -// -// Source-of-truth for this guard: -// - Invalid state: OpenShell's exec endpoint returns InvalidArgument for any -// argv element containing \r or \n. -// - Source boundary: the limitation lives in the external OpenShell -// `sandbox exec` argv contract, not in NemoClaw. We cannot fix it at the -// source from this repo, so the guard is a deliberately localized -// translation of that constraint into actionable NemoClaw guidance. -// - Regression coverage: `findMultilineExecArg`, `multilineExecMessage`, and -// the `execSandbox multi-line guard (#5980)` suite in exec.test.ts. -// - Removal condition: if a future OpenShell release accepts multi-line argv -// elements (tracked upstream in NVIDIA/OpenShell#2110), this guard and the -// matching docs notice in docs/reference/commands.mdx becomes unnecessary -// and should be removed with its generated variants. -// -// The pattern is intentionally limited to \r and \n: OpenShell rejects only -// "newline or carriage return characters", so Unicode line separators (U+2028 -// LINE SEPARATOR, U+2029 PARAGRAPH SEPARATOR) are valid argv that OpenShell -// accepts. Broadening the pattern to those code points would reject commands -// OpenShell would otherwise run, so the guard deliberately mirrors OpenShell's -// exact constraint rather than a general "line break" notion. -const MULTILINE_ARG_PATTERN = /[\r\n]/; - -/** @internal Exported for unit testing only; not part of the public API. */ -export function findMultilineExecArg(command: readonly string[]): number { - for (let index = 0; index < command.length; index += 1) { - if (MULTILINE_ARG_PATTERN.test(command[index])) return index; +// OpenShell accepts LF/CR in command argv while retaining field-specific +// rejection for NUL-bearing command args and NUL/LF/CR-bearing workdirs. Keep +// the downstream check narrow so inline scripts remain byte-exact. NemoClaw's +// public exec surface does not populate OpenShell's request-environment field, +// whose values remain subject to OpenShell's own NUL/LF/CR validation. +function execInputError(command: readonly string[], workdir: string | undefined): string | null { + const nulIndex = command.findIndex((arg) => arg.includes("\0")); + if (nulIndex !== -1) { + return `error: command argument ${nulIndex + 1} contains a NUL byte, which OpenShell exec does not accept`; } - return -1; -} - -// Describe the offending argument WITHOUT echoing its contents: a multi-line -// value can carry pasted secrets, env files, or private-key material, and -// printing even a truncated preview risks persisting it in terminal or CI logs. -// The 1-based position plus a neutral size description is enough for the user -// to find the argument they typed. -function describeMultilineArg(arg: string): string { - // Split on all three newline conventions (CRLF, bare CR, bare LF) so the - // count matches what a user sees regardless of platform. The alternation is - // ordered CRLF-first so a Windows "\r\n" counts as one break, not two. A - // single trailing break still yields a count of 2 (the empty final segment), - // which is correct: a lone "\r" argument spans two lines. - const lineCount = arg.split(/\r\n|\r|\n/).length; - const charLabel = arg.length === 1 ? "character" : "characters"; - const lineLabel = lineCount === 1 ? "line" : "lines"; - return `${arg.length} ${charLabel} spanning ${lineCount} ${lineLabel}`; -} - -export function multilineExecMessage( - cliName: string, - sandboxName: string, - command: readonly string[], - index: number, -): string { - // Report a 1-based position within the user command (the args after `--`). - const position = index + 1; - return [ - `error: command argument ${position} (${describeMultilineArg(command[index])}) contains a newline or carriage return, which OpenShell exec does not accept.`, - "Multi-line commands (for example heredocs) cannot be passed through exec argv. Instead:", - ` - join statements with semicolons: ${cliName} ${sandboxName} exec -- bash -lc "cmd1; cmd2"`, - ` - pipe the script into the sandbox shell over stdin: printf 'cmd1\\ncmd2\\n' | ${cliName} ${sandboxName} exec --stdin -- bash`, - ` - or write the script to a file in the sandbox and run it: ${cliName} ${sandboxName} exec -- bash `, - ].join("\n"); + if (workdir?.includes("\0")) { + return "error: --workdir must not contain NUL bytes"; + } + if (workdir && /[\r\n]/.test(workdir)) { + return "error: --workdir must not contain newlines or carriage returns"; + } + return null; } export function workdirMissingMessage(workdir: string): string { @@ -407,9 +355,9 @@ export async function execSandbox( ); process.exit(2); } - const multilineIndex = findMultilineExecArg(command); - if (multilineIndex !== -1) { - console.error(multilineExecMessage(CLI_NAME, sandboxName, command, multilineIndex)); + const inputError = execInputError(command, options.workdir); + if (inputError) { + console.error(inputError); process.exit(2); } const binary = (deps.resolveBinary ?? defaultResolveBinary)(); diff --git a/src/lib/actions/sandbox/runtime-env.test.ts b/src/lib/actions/sandbox/runtime-env.test.ts index a84729168a1..38ab1bb7640 100644 --- a/src/lib/actions/sandbox/runtime-env.test.ts +++ b/src/lib/actions/sandbox/runtime-env.test.ts @@ -27,6 +27,29 @@ describe("wrapExecCommandWithRuntimeEnv", () => { expect(wrapped[5]).not.toMatch(/[\r\n]/); }); + it("executes LF, CRLF, quote, and heredoc argv byte-exactly", () => { + const payloads = [ + "line one\nline two", + "line one\r\nline two", + `single ' and double " quotes`, + "cat <<'EOF'\nline one\nline 'two'\nEOF", + ]; + const wrapped = wrapExecCommandWithRuntimeEnv([ + process.execPath, + "-e", + "process.stdout.write(JSON.stringify(process.argv.slice(1)))", + ...payloads, + ]); + + const result = spawnSync(wrapped[0], wrapped.slice(1), { + encoding: "utf-8", + env: { ...process.env }, + }); + + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual(payloads); + }); + it("removes OPENCLAW_GATEWAY_TOKEN from the executed command environment (#6291)", () => { const wrapped = wrapExecCommandWithRuntimeEnv([ "/bin/sh", From a271c66c7bdee7594cdd8c504c707c6e211f319c Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 20:14:02 -0700 Subject: [PATCH 08/50] fix(install): validate OpenShell archives before extraction Signed-off-by: Aaron Erickson --- scripts/brev-launchable-ci-cpu.sh | 13 ++++ scripts/install-openshell.sh | 26 +++++++ test/brev-launchable-ci-cpu-checksum.test.ts | 53 +++++++++++++ test/install-openshell-version-check.test.ts | 81 ++++++++++++++++---- 4 files changed, 158 insertions(+), 15 deletions(-) diff --git a/scripts/brev-launchable-ci-cpu.sh b/scripts/brev-launchable-ci-cpu.sh index 67434e478eb..a63fae2101d 100755 --- a/scripts/brev-launchable-ci-cpu.sh +++ b/scripts/brev-launchable-ci-cpu.sh @@ -165,6 +165,18 @@ openshell_checksum_line() { awk -v asset="$asset" '$2 == asset { print; found=1; exit } END { if (!found) exit 1 }' "$checksum_file" } +validate_openshell_archive() { + local archive="$1" expected_member="$2" members verbose + members="$(LC_ALL=C tar -tzf "$archive")" \ + || fail "Unable to list OpenShell archive $(basename "$archive")" + [ "$members" = "$expected_member" ] \ + || fail "Unsafe OpenShell archive $(basename "$archive"): expected exactly one member named $expected_member" + verbose="$(LC_ALL=C tar -tvzf "$archive")" \ + || fail "Unable to inspect OpenShell archive $(basename "$archive")" + [[ "$verbose" != *$'\n'* && "${verbose:0:1}" = "-" && "${verbose##* }" = "$expected_member" ]] \ + || fail "Unsafe OpenShell archive $(basename "$archive"): $expected_member must be one regular file" +} + verify_openshell_cli_asset() { local tmpdir="$1" asset="$2" checksum_file="openshell-checksums-sha256.txt" local checksum_line expected_sha release_sha @@ -201,6 +213,7 @@ install_openshell_cli_release() { if [[ "$OPENSHELL_VERSION" != "dev" ]]; then verify_openshell_cli_asset "$tmpdir" "$asset" fi + validate_openshell_archive "$tmpdir/$asset" openshell tar xzf "$tmpdir/$asset" -C "$tmpdir" sudo install -m 755 "$tmpdir/openshell" /usr/local/bin/openshell rm -rf "$tmpdir" diff --git a/scripts/install-openshell.sh b/scripts/install-openshell.sh index 9d0c07669f2..ac1fc5b97bd 100755 --- a/scripts/install-openshell.sh +++ b/scripts/install-openshell.sh @@ -175,6 +175,22 @@ openshell_checksum_line() { awk -v asset="$asset" '$2 == asset { print; found=1; exit } END { if (!found) exit 1 }' "$checksum_file" } +# A pinned digest authenticates bytes, but it does not make extraction safe. +# Every consumed OpenShell archive must contain exactly the one regular binary +# selected by its asset name. This rejects absolute/parent paths, extra or +# duplicate members, and links/devices before tar can write anything. +validate_openshell_archive() { + local archive="$1" expected_member="$2" members verbose + members="$(LC_ALL=C tar -tzf "$archive")" \ + || fail "Unable to list OpenShell archive $(basename "$archive")" + [ "$members" = "$expected_member" ] \ + || fail "Unsafe OpenShell archive $(basename "$archive"): expected exactly one member named $expected_member" + verbose="$(LC_ALL=C tar -tvzf "$archive")" \ + || fail "Unable to inspect OpenShell archive $(basename "$archive")" + [[ "$verbose" != *$'\n'* && "${verbose:0:1}" = "-" && "${verbose##* }" = "$expected_member" ]] \ + || fail "Unsafe OpenShell archive $(basename "$archive"): $expected_member must be one regular file" +} + version_gte() { # Returns 0 (true) if $1 >= $2 — portable, no sort -V (BSD compat) local IFS=. @@ -748,6 +764,16 @@ for i in "${!ASSETS[@]}"; do || fail "SHA-256 checksum verification failed for $asset_name" done +for asset_name in "${ASSETS[@]}"; do + case "$asset_name" in + openshell-gateway-*) expected_member="openshell-gateway" ;; + openshell-sandbox-*) expected_member="openshell-sandbox" ;; + openshell-*) expected_member="openshell" ;; + *) fail "No expected archive member is defined for $asset_name" ;; + esac + validate_openshell_archive "$tmpdir/$asset_name" "$expected_member" +done + for asset_name in "${ASSETS[@]}"; do tar xzf "$tmpdir/$asset_name" -C "$tmpdir" done diff --git a/test/brev-launchable-ci-cpu-checksum.test.ts b/test/brev-launchable-ci-cpu-checksum.test.ts index e338ecd63f8..0508d77a3bf 100644 --- a/test/brev-launchable-ci-cpu-checksum.test.ts +++ b/test/brev-launchable-ci-cpu-checksum.test.ts @@ -14,6 +14,15 @@ const ASSET = "openshell-x86_64-unknown-linux-musl.tar.gz"; const PINNED_ASSET_SHA256 = "37836c3b50383e03249c5e16512c1806e591fba8451408a84fb2f628ddb318c4"; type FakeSystemOptions = { + archiveShape?: + | "absolute" + | "device" + | "duplicate" + | "extra" + | "hardlink" + | "safe" + | "symlink" + | "traversal"; checksum: "match" | "mismatch" | "unpinned"; nodeSourceChecksumTool?: boolean; openshellVersion?: string; @@ -133,6 +142,26 @@ exit 0 path.join(fakeBin, "tar"), `#!/usr/bin/env bash printf '%s\\n' "$*" >> ${JSON.stringify(tarLog)} +shape=${JSON.stringify(options.archiveShape ?? "safe")} +if [ "\${1:-}" = "-tzf" ] && [ "$shape" != "safe" ]; then + case "$shape" in + absolute) printf '/tmp/openshell\\n' ;; + traversal) printf '../openshell\\n' ;; + duplicate) printf 'openshell\\nopenshell\\n' ;; + extra) printf 'openshell\\nunexpected\\n' ;; + *) printf 'openshell\\n' ;; + esac + exit 0 +fi +if [ "\${1:-}" = "-tvzf" ] && [ "$shape" != "safe" ]; then + case "$shape" in + symlink) printf 'lrwxrwxrwx 0/0 0 2026-01-01 00:00 openshell -> target\\n' ;; + hardlink) printf 'hrwxr-xr-x 0/0 0 2026-01-01 00:00 openshell link to target\\n' ;; + device) printf 'crw-rw-rw- 0/0 1,3 2026-01-01 00:00 openshell\\n' ;; + *) printf '%s\\n' '-rwxr-xr-x 0/0 1 2026-01-01 00:00 openshell' ;; + esac + exit 0 +fi exec /usr/bin/tar "$@" `, ); @@ -349,4 +378,28 @@ describe("brev-launchable-ci-cpu.sh OpenShell checksum gate", { timeout: 30_000 fake.cleanup(); } }); + + it.each([ + "absolute", + "traversal", + "duplicate", + "extra", + "symlink", + "hardlink", + "device", + ] as const)("rejects an unsafe %s archive before extraction or install", (archiveShape) => { + const { fake, result } = runLaunchable({ archiveShape, checksum: "match" }); + try { + const out = combinedLaunchableOutput(result, fake.launchLog); + expect(result.status, out).toBe(1); + expect(out).toContain(`Unsafe OpenShell archive ${ASSET}`); + const tarCalls = fs.readFileSync(fake.tarLog, "utf-8"); + expect(tarCalls).not.toMatch(/^xzf /m); + expect(fs.existsSync(fake.sudoLog) ? fs.readFileSync(fake.sudoLog, "utf-8") : "").not.toMatch( + /^install -m 755 .*openshell/m, + ); + } finally { + fake.cleanup(); + } + }); }); diff --git a/test/install-openshell-version-check.test.ts b/test/install-openshell-version-check.test.ts index 161e140643b..9d9ff47fb97 100644 --- a/test/install-openshell-version-check.test.ts +++ b/test/install-openshell-version-check.test.ts @@ -422,11 +422,22 @@ describe("install-openshell.sh version check", { timeout: 15_000 }, () => { ); }); - it("downloads the macOS arm64 gateway asset during reinstall", () => { + it.each([ + "safe", + "absolute", + "traversal", + "duplicate", + "extra", + "symlink", + "hardlink", + "device", + "late-traversal", + ] as const)("%s macOS arm64 archives are checked before extraction", (archiveShape) => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-openshell-macos-assets-")); try { const fakeBin = path.join(tmp, "bin"); const downloadLog = path.join(tmp, "downloads.log"); + const tarLog = path.join(tmp, "tar.log"); fs.mkdirSync(fakeBin); writeExecutable( @@ -484,6 +495,35 @@ exit 0`, writeExecutable( path.join(fakeBin, "tar"), `#!/usr/bin/env bash +printf '%s\\n' "$*" >> ${JSON.stringify(tarLog)} +case "$*" in +*openshell-gateway*) name="openshell-gateway" ;; +*) name="openshell" ;; +esac +case "\${1:-}" in +-tzf) + case ${JSON.stringify(archiveShape)} in + absolute) printf '/tmp/%s\\n' "$name" ;; + traversal) printf '../%s\\n' "$name" ;; + duplicate) printf '%s\\n%s\\n' "$name" "$name" ;; + extra) printf '%s\\nunexpected\\n' "$name" ;; + late-traversal) + if [ "$name" = "openshell-gateway" ]; then printf '../%s\\n' "$name"; else printf '%s\\n' "$name"; fi + ;; + *) printf '%s\\n' "$name" ;; + esac + exit 0 + ;; +-tvzf) + case ${JSON.stringify(archiveShape)} in + symlink) printf 'lrwxrwxrwx 0/0 0 2026-01-01 00:00 %s -> target\\n' "$name" ;; + hardlink) printf 'hrwxr-xr-x 0/0 0 2026-01-01 00:00 %s link to target\\n' "$name" ;; + device) printf 'crw-rw-rw- 0/0 1,3 2026-01-01 00:00 %s\\n' "$name" ;; + *) printf '%s\\n' "-rwxr-xr-x 0/0 1 2026-01-01 00:00 $name" ;; + esac + exit 0 + ;; +esac outdir="" prev="" for arg in "$@"; do @@ -494,10 +534,6 @@ for arg in "$@"; do prev="$arg" done [ -n "$outdir" ] || exit 1 -case "$*" in -*openshell-gateway*) name="openshell-gateway" ;; -*) name="openshell" ;; -esac printf '#!/usr/bin/env bash\nexit 0\n' > "$outdir/$name" chmod 755 "$outdir/$name" exit 0`, @@ -528,6 +564,13 @@ exit 0`, encoding: "utf8", }); + if (archiveShape !== "safe") { + expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(1); + expect(result.stderr).toContain("Unsafe OpenShell archive"); + expect(fs.existsSync(path.join(tmp, "local-bin", "openshell"))).toBe(false); + expect(fs.readFileSync(tarLog, "utf8")).not.toMatch(/^xzf /m); + return; + } expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); const downloads = fs.readFileSync(downloadLog, "utf-8"); expect(downloads).toContain("openshell-aarch64-apple-darwin.tar.gz"); @@ -596,17 +639,21 @@ printf '%s\n' 'checksum OK'`, writeExecutable( path.join(fakeBin, "tar"), `#!/usr/bin/env bash +case "$*" in +*openshell-gateway*) name="openshell-gateway" ;; +*openshell-sandbox*) name="openshell-sandbox" ;; +*) name="openshell" ;; +esac +case "\${1:-}" in +-tzf) printf '%s\\n' "$name"; exit 0 ;; +-tvzf) printf '%s\\n' "-rwxr-xr-x 0/0 1 2026-01-01 00:00 $name"; exit 0 ;; +esac outdir="" prev="" for arg in "$@"; do if [ "$prev" = "-C" ]; then outdir="$arg"; break; fi prev="$arg" done -case "$*" in -*openshell-gateway*) name="openshell-gateway" ;; -*openshell-sandbox*) name="openshell-sandbox" ;; -*) name="openshell" ;; -esac printf '#!/usr/bin/env bash\nexit 0\n' > "$outdir/$name" chmod 755 "$outdir/$name"`, ); @@ -723,6 +770,15 @@ exit 0`, writeExecutable( path.join(fakeBin, "tar"), `#!/usr/bin/env bash +case "$*" in +*openshell-gateway*) name="openshell-gateway" ;; +*openshell-sandbox*) name="openshell-sandbox" ;; +*) name="openshell" ;; +esac +case "\${1:-}" in +-tzf) printf '%s\\n' "$name"; exit 0 ;; +-tvzf) printf '%s\\n' "-rwxr-xr-x 0/0 1 2026-01-01 00:00 $name"; exit 0 ;; +esac outdir="" prev="" for arg in "$@"; do @@ -733,11 +789,6 @@ for arg in "$@"; do prev="$arg" done [ -n "$outdir" ] || exit 1 -case "$*" in -*openshell-gateway*) name="openshell-gateway" ;; -*openshell-sandbox*) name="openshell-sandbox" ;; -*) name="openshell" ;; -esac printf '#!/usr/bin/env bash\\nexit 0\\n' > "$outdir/$name" chmod 755 "$outdir/$name" exit 0`, From 06ec576fab29dfa73b1a09137c7214cf72befbbd Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 20:14:47 -0700 Subject: [PATCH 09/50] docs(security): audit OpenShell 0.0.82 migration Signed-off-by: Aaron Erickson --- .../openshell-0.0.82-migration-review.md | 313 ++++++++++++++++++ .../openshell-0.0.82-migration-review.test.ts | 135 ++++++++ 2 files changed, 448 insertions(+) create mode 100644 docs/security/openshell-0.0.82-migration-review.md create mode 100644 test/openshell-0.0.82-migration-review.test.ts diff --git a/docs/security/openshell-0.0.82-migration-review.md b/docs/security/openshell-0.0.82-migration-review.md new file mode 100644 index 00000000000..3608e80f082 --- /dev/null +++ b/docs/security/openshell-0.0.82-migration-review.md @@ -0,0 +1,313 @@ + + + +# OpenShell 0.0.72 to 0.0.82 migration review + +## Status and decision + +This is a candidate migration review, not approval to ship OpenShell 0.0.82. +The reviewed upstream endpoint is OpenShell `main` at +[`bb72d0123c748ed7e209880f7bab593e10aae221`](https://github.com/NVIDIA/OpenShell/commit/bb72d0123c748ed7e209880f7bab593e10aae221). +OpenShell has not published a stable `v0.0.82` tag or release at this endpoint. +NemoClaw therefore remains pinned to `0.0.72` until the final release identity, +semantic migrations, artifact provenance, supported-platform proofs, and DGX Spark +credential-substitution proof below are complete. + +The source under review includes +[`40194f935ef6e29cb07500b9109314778ab6915c`](https://github.com/NVIDIA/OpenShell/commit/40194f935ef6e29cb07500b9109314778ab6915c), +which prevents a credential placeholder from leaving the proxy unresolved. That +change fails closed when the resolver or TLS-termination state is missing; it does +not prove that the affected DGX Spark host can initialize that state and complete a +real credential-bearing MCP call. NVIDIA/NemoClaw#6379 remains open until the +physical Docker 27 DGX Spark reproducer reports honest status and completes a real +MCP tool call with credential substitution. + +## Audit method and exact boundary + +The current identity is OpenShell `v0.0.72` at +`8cb16de9eae4c44d7d31e1493747d8c10abb5963`. The candidate identity is the exact +remote `main` SHA above, not a local branch name or the moving `dev` tag. + +The audit enumerated every stable adjacent tag, then read the release notes, +complete commit list, changed paths, source diffs, and upstream tests for each +range. Release notes were treated as leads rather than proof. The resulting boundary +contains 10 adjacent ranges, 46 commits, and 174 distinct changed paths in the +aggregate `v0.0.72..bb72d012` comparison. + +The ledger was produced with: + +```bash +collect-release-ledger.py \ + --repo \ + --from v0.0.72 \ + --to origin/main +``` + +| Range | Commits | Changed paths | Diff size | +|---|---:|---:|---:| +| `v0.0.72 -> v0.0.73` | 5 | 27 | +1,530 / -531 | +| `v0.0.73 -> v0.0.74` | 6 | 25 | +328 / -163 | +| `v0.0.74 -> v0.0.75` | 2 | 26 | +3,416 / -5 | +| `v0.0.75 -> v0.0.76` | 3 | 28 | +2,267 / -201 | +| `v0.0.76 -> v0.0.77` | 3 | 7 | +452 / -27 | +| `v0.0.77 -> v0.0.78` | 6 | 23 | +198 / -115 | +| `v0.0.78 -> v0.0.79` | 1 | 1 | +1 / -1 | +| `v0.0.79 -> v0.0.80` | 5 | 15 | +1,373 / -96 | +| `v0.0.80 -> v0.0.81` | 4 | 9 | +617 / -24 | +| `v0.0.81 -> bb72d012` | 11 | 75 | +7,850 / -982 | + +Release publication is a separate gate from source ancestry: + +- `v0.0.73` through `v0.0.80` have published GitHub releases. +- The `v0.0.79` release notes repeat the `v0.0.78` change list and add only the + `setup-uv` bump. The adjacent Git diff, not that cumulative note body, is the + source of truth for the `v0.0.78 -> v0.0.79` range. +- `v0.0.81` is a source tag at `420a855ddc21a20ac528f902bd2ed7f3fc133dc9`, + but it has no GitHub release. Release Tag run + [29101552146](https://github.com/NVIDIA/OpenShell/actions/runs/29101552146) + failed the Ubuntu 26.04 rootless-Podman E2E job and skipped publication. +- OpenShell `bb72d012` produced moving development build + `0.0.82.dev11+gbb72d0123` in successful Release Dev run + [29215426930](https://github.com/NVIDIA/OpenShell/actions/runs/29215426930). + A moving prerelease is useful for compatibility work, but is not a stable + selector or final provenance record. + +## Artifact baseline and provenance gap + +The currently shipped `0.0.72` supervisor index +`sha256:80ed9cda5bf672fefdb9dcd4604b40a8b09c0891b6eb9d03e10227c7e3dfb49d` +resolves to these exact platform identities: + +| Platform | Child manifest | Config | +|---|---|---| +| Linux amd64 | `sha256:e97174326ee25c896117e854c791945d0c458a26bc9d6eab004ccd6c19d86ee7` | `sha256:b34f500c495871bf92d8a04011a210167e95f3650927b3bd67dde3ddcc021ac2` | +| Linux arm64 | `sha256:0679e02da0bd480a3e2f119dc2d205269336c9c01d7d2c8f18d05400f89d160e` | `sha256:e53f2ac5b7b3667833271f62f053887d2be9f223d2699b7e39f88c78fd9df373` | + +Both child configs set `/openshell-sandbox` as the entrypoint but expose no OCI +source labels. A read-only registry audit on July 12, 2026 found that GHCR +returned no referrers index for the shipped manifest and that all 2,905 +supervisor tags contained no digest-derived signature, attestation, or SBOM tag +for that index. Therefore `0.0.72` has no verifiable source-to-image attestation; +matching release tags or timestamps cannot fill the gap. The immutable index +digest is the strongest enforceable runtime control for the baseline, while the +child manifest and config identities above are audit evidence rather than source +provenance. The final `0.0.82` artifact audit must verify any upstream OCI +attestation and source labels that are actually published, or explicitly retain +this provenance gap as an unresolved supply-chain gate. NemoClaw cannot +manufacture a missing upstream attestation. + +NemoClaw now validates the shape of every consumed OpenShell archive before any +extraction: the asset must contain exactly one regular file with the expected +CLI, gateway, or sandbox binary name. Absolute paths, parent traversal, extra or +duplicate members, links, and devices fail closed. This structural validation is +independent of release SHA-256 verification and also constrains the explicitly +unverified development-channel path. + +## Adjacent release findings + +### v0.0.72 to v0.0.73 + +Commits: `afc06dd2`, `a5161d0b`, `a2268060`, `f27ff150`, `474d2d4a`. + +- `afc06dd2` clears the full Linux capability bounding set for entrypoint, exec, + and connect children. Child launch fails when `CAP_SETPCAP` is unavailable and + the bounding set is nonempty; it succeeds without that capability only when the + runtime already supplied an empty bounding set. This directly intersects + NemoClaw's Docker Desktop, WSL, Colossus, cloud, and DGX capability workarounds. +- `a5161d0b` moves selected-driver configuration acquisition into a normalized + server path. NemoClaw renders an authenticated Docker TOML containing the + selected driver, TLS, mTLS, JWT, supervisor image, and supervisor binary; the + final OpenShell binary must parse that exact rendered file and preserve both + listener paths across restart and legacy-gateway upgrade. +- `f27ff150` reserves credential names matching `v_`, introduces + revision-scoped child placeholders, retains eight resolver generations, and + falls an evicted revision back to the current credential only when that key + still exists. NemoClaw must reject the reserved namespace and prove rotation, + removal, restart, and rebuild behavior rather than assuming the old unversioned + placeholder contract. +- `a2268060` changes only upstream GPU E2E fixture execution and `474d2d4a` + changes contributor documentation. They add no consumed runtime contract. + +### v0.0.73 to v0.0.74 + +Commits: `ed0026aa`, `0a25fdf5`, `5477e2f2`, `914da339`, `450685c7`, `45614a3f`. + +- `0a25fdf5` removes the unused gateway `extra_bind_addresses` configuration. + NemoClaw does not emit that field, but must parse its final Docker TOML and prove + the intended loopback and Docker-bridge reachability instead of relying on that + absence alone. +- `450685c7` rejects leading/trailing whitespace in mount fields. NemoClaw does + not configure production driver mounts; the test-only EXDEV tmpfs mount is the + downstream consumer and remains a required no-impact regression. +- The Helm SAN, MCP documentation, Kubernetes combined-topology, and removed raw + `SandboxTemplate.volume_claim_templates` changes are not consumed by NemoClaw's + Docker gateway or CLI integration. NemoClaw has no raw OpenShell protobuf client + for the removed field. + +### v0.0.74 to v0.0.75 + +Commits: `abcd15d1`, `45060f44`. + +Envoy Gateway TLS termination and the Gator agent manifest are native OpenShell +Kubernetes/agent surfaces. NemoClaw neither deploys that Helm topology nor selects +the Gator manifest, so these are evidence-backed exclusions from the Docker +dependency migration. + +### v0.0.75 to v0.0.76 + +Commits: `43bb0302`, `5f9bf9ce`, `6461677c`. + +- `43bb0302` changes Docker and Podman bind mounts to support SELinux relabeling, + explicit source checks, and Docker's legacy bind representation. Production + NemoClaw supplies no driver mounts; the EXDEV fixture remains the direct test. +- Numeric UID/GID policy identities are additive. NemoClaw continues to run the + named `sandbox` identity and must not silently change that identity during this + bump. +- The rootless-Podman host E2E change does not alter NemoClaw's Docker runtime. + +### v0.0.76 to v0.0.77 + +Commits: `f852d07b`, `6252aa17`, `31807d68`. + +This range contains Hermes support documentation, an unimplemented driver-config +passthrough RFC, and a workflow action bump. It adds no shipped contract consumed +by NemoClaw. + +### v0.0.77 to v0.0.78 + +Commits: `5656240c`, `290297ff`, `9c14de7b`, `eba5dd75`, `abe42fb5`, `a7271169`. + +The Podman sandbox-JWT secret delivery fix is outside NemoClaw's Docker path. +The remaining changes are documentation or removal of deprecated `--keep` +references; NemoClaw does not invoke `--keep`. + +### v0.0.78 to v0.0.79 + +Commit: `f7aa3aa3`. + +Only `astral-sh/setup-uv` changed. There is no runtime or packaging selector +consumed by NemoClaw in this adjacent range, despite the cumulative `v0.0.79` +release-note body listing older changes. + +### v0.0.79 to v0.0.80 + +Commits: `2e2b497f`, `ed8ce820`, `5207f118`, `ff9af8e3`, `709aa0fe`. + +- `ff9af8e3` acknowledges the exact initially loaded sandbox policy revision and + reconciles an initial mismatch instead of leaving version zero/pending state. + Status delivery uses an unbounded FIFO and retries retryable failures without a + terminal attempt limit. Policy enforcement continues, but an older unavailable + acknowledgement can head-of-line block later status delivery. NemoClaw must + prove initial load, failed load, hot update, outage/recovery ordering, restart, + and an exact policy re-read; `policy set --wait` alone is insufficient evidence. +- The Podman import fix, Docker-version typo, man-page date, and setup action bump + do not change the consumed Docker runtime contract. + +### v0.0.80 to v0.0.81 + +Commits: `83131d7e`, `88710225`, `49701088`, `420a855d`. + +NemoClaw does not call `provider refresh configure +--secret-material-env`. Telemetry documentation and Packit target changes are +not consumed. `420a855d` adds upstream supervisor proxy-hostname regression tests +without changing product source. The failed stable publication is nevertheless a +hard artifact gate for this source tag. + +### v0.0.81 to candidate main bb72d012 + +Commits: `5f38b7c4`, `ccdac9ce`, `caaa5165`, `8c0ecac8`, `233d207e`, +`10702133`, `bebf440b`, `8eacb477`, `614c8c16`, `40194f93`, `bb72d012`. + +- `40194f93` closes two placeholder leak paths: missing resolver state now rejects + reserved credential markers, and missing TLS-termination state returns a + pre-200 CONNECT 503 rather than creating a raw tunnel. The upstream PR explicitly + did not add a true connection-level test for the second branch. NemoClaw keeps + its wire-level status probe and requires the physical #6379 tool-call proof. +- `bb72d012` permits newline and carriage-return bytes in exec command arguments + while retaining strict NUL and non-command-field validation. NemoClaw's active + public guard and internal base64 workarounds must be removed or reclassified, + and byte-exact LF, CRLF, quotes, and heredoc cases must pass without weakening + workdir or environment validation. +- `8eacb477` changes the combined Docker supervisor even though its title names + Kubernetes. The supervisor image changes from `scratch` plus one mode-0550 + binary to Alpine 3.22 with `nftables`, `iptables`, `iptables-legacy`, and a + mode-0555 binary. This adds an OS package, SBOM, vulnerability, license, and + executable surface that must be reviewed and bound to the final OCI digest. +- The same commit changes generic Docker namespace nft installation from an + atomic batch to sequential commands. Required failures can occur after the + policy-accept chain and accept rules exist but before all IPv4/IPv6 TCP/UDP + rejects exist; the outer Docker setup records that failure as nonfatal. The + final runtime proof must inject failures, inspect the actual installed rules, + and verify that direct bypass remains unavailable through restart and teardown. +- `10702133` makes each driver's default supervisor tag follow the gateway + version. NemoClaw supplies an explicit image and supervisor binary, so the + downstream invariant remains exact CLI/gateway/sandbox/component equality plus + an immutable multi-architecture image digest. +- Shared child-process construction now strips `OPENSHELL_TLS_CA`, + `OPENSHELL_TLS_CERT`, and `OPENSHELL_TLS_KEY` from entrypoint, exec, and connect + children. Those values remain supervisor identity material; NemoClaw tests and + comments must assert absence rather than describing child injection. +- Network binary identity now hashes the live `/proc//exe` target. The + migration must prove that an already-running allowed process survives on-disk + replacement while a newly launched altered binary at the same path is denied. +- `ccdac9ce` adds sanitized MCP tool names to policy logs without logging + arguments. This is an additive observability/privacy change and NemoClaw has no + strict parser for the old format. +- Native Kubernetes sidecar/PVC/Helm changes, OpenShift documentation, and the TUI + warning destination are not consumed by NemoClaw's Docker integration. + +## Downstream concern ledger + +| ID | Severity | Downstream consumer and failure mode | Required disposition | Current state | +|---|---|---|---|---| +| `OS82-01` | Critical | All stable selectors, archives, checksums, binaries, and the supervisor image could identify different builds. | Pin one published tag; verify producer run, signatures/attestations, release hashes, extracted binaries, component versions, OCI index and child manifests; reject archive traversal, links, devices, duplicates, or unexpected members. | Blocked: no stable `v0.0.82` release. | +| `OS82-02` | Critical | `mcp status` can be honest while the affected Spark still cannot initialize resolver/CA state or perform a credential-bearing request. | Physical Docker 27 DGX Spark: register credential, require status success, load tools, complete a real MCP tool call, and prove the literal placeholder never reaches upstream. | Blocked on assigned hardware proof. | +| `OS82-03` | High | `src/lib/actions/sandbox/exec.ts`, command dispatch, docs, and internal wrappers encode the old newline rejection. | Remove the obsolete public rejection; prove byte-exact LF, CRLF, quotes, and heredoc argv; retain NUL plus multiline workdir/environment rejection. | Migration in progress. | +| `OS82-04` | High | OpenShell child launch now clears the complete capability bounding set. Hosts without `CAP_SETPCAP` may fail if their runtime does not pre-clear it. | Prove entrypoint, exec, and connect launch with `CapBnd=0` on Linux Docker, DGX Spark arm64, macOS Docker Desktop/Colima, WSL, and Colossus; update NemoClaw's #3280 caveat only from runtime evidence. | Open runtime gate. | +| `OS82-05` | High | Versioned credential placeholders and the eight-generation window change long-running MCP behavior. | Regenerate the exact-version child-visible manifest; reject reserved `v_` names; test more than eight rotations, removed keys, detach, restart/rebuild, fresh exec revision, expiry, and literal-placeholder scans. | Open migration and runtime gate. | +| `OS82-06` | High | Initial policy acknowledgement and ordered retry can make the active gateway status lag enforcement. | Test initial LOADED/FAILED, hot update, retry outage/recovery, restart, exact version/hash re-read, and ordered drain. | Open runtime gate. | +| `OS82-07` | High | Sequential nft setup can leave an incomplete policy-accept ruleset after a required command fails; Docker setup treats the error as nonfatal. | Inject each required failure; inspect IPv4/IPv6 TCP/UDP rules and direct-bypass negatives on Linux x86 and Spark arm64; verify restart and teardown. | Open security gate. | +| `OS82-08` | High | The supervisor image gains Alpine and three networking packages and changes binary mode. | Review SBOM, vulnerabilities, licenses, executables, modes, multiarch manifests, source labels, and OCI provenance; preserve an explicit digest. | Open supply-chain gate. | +| `OS82-09` | Medium-high | Normalized selected-driver config can change the effective Docker gateway even when the TOML text is unchanged. | Parse the final rendered TOML with the final binary; prove loopback/bridge listeners, JWT/mTLS, restart, persisted state, and legacy gateway upgrade. | Open runtime gate. | +| `OS82-10` | Medium-high | Supervisor TLS identity variables are no longer child environment. Stale tests/comments can normalize a credential leak. | Assert absence from entrypoint, exec, and connect children and update the source-of-truth rationale. | Open migration gate. | +| `OS82-11` | Medium-high | Live `/proc//exe` identity changes replacement-time policy behavior. | Prove old process survives replacement and a new altered process at the same path is denied. | Open runtime gate. | +| `OS82-12` | Medium | OpenShell declares Docker 28.0+ while #6379 is on Docker 27 and NemoClaw marks DGX Spark tested. | Either validate and document a precise downstream exception from physical proof or raise the supported floor and preflight it. | Open product/platform decision. | +| `OS82-13` | Low | Mount parsing/SELinux changes could affect the test-only tmpfs path. | Rerun the EXDEV tmpfs fixture and retain production no-mount evidence. | Open targeted test. | +| `OS82-14` | Low | Sanitized MCP tool names are newly present in logs. | Record the additive observability/privacy behavior; ensure no downstream parser assumes the old shape. | Source-reviewed; targeted log check pending. | + +An unresolved critical or high concern blocks the version selector change. A green +aggregate test suite does not override an open ledger row. + +## Test-selection and false-green audit + +The moving-development MCP workflow currently classifies an OpenShell version +different from the versioned child-visible credential manifest as an expected +compatibility rejection, records the classification as passed, and does not run +the full managed MCP lifecycle. That is correct fail-closed behavior for an +unreviewed development runtime, but it is not evidence that the candidate is +compatible. + +Before any `0.0.82` selector can be called green, the credential manifest and all +of its imports/image copies must identify the reviewed candidate, the workflow +must select `full-lifecycle`, and all three agents must complete registration, +credential rotation, DNS-rebinding denial, policy denial, real tool invocation, +restart/rebuild, and cleanup without a conditional skip or expected failure. + +## Final acceptance gates + +1. A stable OpenShell tag contains `bb72d012` or a reviewed descendant. Re-run + this entire adjacent-source audit for every commit between `bb72d012` and that + tag. +2. The tag has a successful release publication. Every consumed archive and OCI + child manifest is bound to that producer run and source identity. +3. Blueprint bounds, installer tables, Brev defaults, workflow pins, feature-gate + hashes, supervisor digest, credential manifest, tests, and active docs select + one coherent version. +4. Every concern-specific unit/integration proof above passes, followed by normal + repository checks and exact-head CI/advisor review. +5. The non-skipped live matrix passes on Linux x86 Docker, macOS Docker + Desktop/Colima, WSL, Colossus, and physical DGX Spark arm64. Legacy gateway + upgrade, restart, rollback, and teardown remain explicit phases. +6. The physical #6379 Spark run completes an authenticated real MCP tool call and + reports any failure honestly. Inclusion of `40194f93` alone cannot close the + issue. diff --git a/test/openshell-0.0.82-migration-review.test.ts b/test/openshell-0.0.82-migration-review.test.ts new file mode 100644 index 00000000000..a0d91d60244 --- /dev/null +++ b/test/openshell-0.0.82-migration-review.test.ts @@ -0,0 +1,135 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +const repoRoot = path.resolve(import.meta.dirname, ".."); +const review = fs.readFileSync( + path.join(repoRoot, "docs", "security", "openshell-0.0.82-migration-review.md"), + "utf8", +); + +const adjacentRanges = [ + { from: "v0.0.72", to: "v0.0.73", commits: 5, paths: 27 }, + { from: "v0.0.73", to: "v0.0.74", commits: 6, paths: 25 }, + { from: "v0.0.74", to: "v0.0.75", commits: 2, paths: 26 }, + { from: "v0.0.75", to: "v0.0.76", commits: 3, paths: 28 }, + { from: "v0.0.76", to: "v0.0.77", commits: 3, paths: 7 }, + { from: "v0.0.77", to: "v0.0.78", commits: 6, paths: 23 }, + { from: "v0.0.78", to: "v0.0.79", commits: 1, paths: 1 }, + { from: "v0.0.79", to: "v0.0.80", commits: 5, paths: 15 }, + { from: "v0.0.80", to: "v0.0.81", commits: 4, paths: 9 }, + { from: "v0.0.81", to: "bb72d012", commits: 11, paths: 75 }, +] as const; + +const auditedCommits = [ + "afc06dd2", + "a5161d0b", + "a2268060", + "f27ff150", + "474d2d4a", + "ed0026aa", + "0a25fdf5", + "5477e2f2", + "914da339", + "450685c7", + "45614a3f", + "abcd15d1", + "45060f44", + "43bb0302", + "5f9bf9ce", + "6461677c", + "f852d07b", + "6252aa17", + "31807d68", + "5656240c", + "290297ff", + "9c14de7b", + "eba5dd75", + "abe42fb5", + "a7271169", + "f7aa3aa3", + "2e2b497f", + "ed8ce820", + "5207f118", + "ff9af8e3", + "709aa0fe", + "83131d7e", + "88710225", + "49701088", + "420a855d", + "5f38b7c4", + "ccdac9ce", + "caaa5165", + "8c0ecac8", + "233d207e", + "10702133", + "bebf440b", + "8eacb477", + "614c8c16", + "40194f93", + "bb72d012", +] as const; + +describe("OpenShell 0.0.82 migration review", () => { + it("records every adjacent release range and all 46 audited commits", () => { + expect(adjacentRanges.reduce((total, range) => total + range.commits, 0)).toBe(46); + for (const range of adjacentRanges) { + expect(review).toContain( + `| \`${range.from} -> ${range.to}\` | ${range.commits} | ${range.paths} |`, + ); + } + for (const commit of auditedCommits) { + expect(review, `missing audited OpenShell commit ${commit}`).toContain(commit); + } + expect(review).toContain("174 distinct changed paths"); + }); + + it("keeps source ancestry, release publication, and artifact provenance as separate gates", () => { + expect(review).toContain("This is a candidate migration review, not approval to ship"); + expect(review).toContain("v0.0.81` is a source tag"); + expect(review).toContain("it has no GitHub release"); + expect(review).toContain("failed the Ubuntu 26.04 rootless-Podman E2E job"); + expect(review).toContain("no verifiable source-to-image attestation"); + expect(review).toContain("reject archive traversal, links, devices, duplicates"); + }); + + it("tracks every material migration concern and refuses false-green evidence", () => { + for (let number = 1; number <= 14; number += 1) { + const id = `OS82-${String(number).padStart(2, "0")}`; + expect(review.split(`| \`${id}\` |`), `${id} concern row`).toHaveLength(2); + } + expect(review).toContain("An unresolved critical or high concern blocks"); + expect(review).toContain("full managed MCP lifecycle"); + expect(review).toContain("without a conditional skip or expected failure"); + }); + + it("keeps the stable pin and physical Spark proof blocked until final evidence exists", () => { + const blueprint = fs.readFileSync( + path.join(repoRoot, "nemoclaw-blueprint", "blueprint.yaml"), + "utf8", + ); + const manifest = JSON.parse( + fs.readFileSync( + path.join( + repoRoot, + "src", + "lib", + "actions", + "sandbox", + "openshell-child-visible-credentials.v0.0.72.json", + ), + "utf8", + ), + ) as { openshellVersion: string }; + + expect(blueprint).toContain('min_openshell_version: "0.0.72"'); + expect(blueprint).toContain('max_openshell_version: "0.0.72"'); + expect(manifest.openshellVersion).toBe("0.0.72"); + expect(review).toContain("remains pinned to `0.0.72`"); + expect(review).toContain("physical Docker 27 DGX Spark"); + expect(review).toContain("Inclusion of `40194f93` alone cannot close"); + }); +}); From 18189eb1420574d52ebf4f6f35b79432ac4b5291 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 13:39:38 -0700 Subject: [PATCH 10/50] fix(ci): validate trusted OpenShell release upgrades Signed-off-by: Aaron Erickson --- scripts/check-installer-hash.sh | 187 ++++++++++++++-------- scripts/checks/dependency-pins.ts | 40 +++-- scripts/checks/extract-installer-pins.mts | 70 ++++---- test/dependency-pins-check.test.ts | 43 ++++- test/installer-hash-check.test.ts | 98 ++++++++++-- 5 files changed, 325 insertions(+), 113 deletions(-) diff --git a/scripts/check-installer-hash.sh b/scripts/check-installer-hash.sh index b1c6ab7d0e1..dedd4c4b1f9 100755 --- a/scripts/check-installer-hash.sh +++ b/scripts/check-installer-hash.sh @@ -6,8 +6,8 @@ # still match the immutable upstream checksum manifests. # # Checked artifacts: -# 1. OpenShell v0.0.72 — scripts/install-openshell.sh release-asset table -# 2. Brev OpenShell CLI — scripts/brev-launchable-ci-cpu.sh release-asset table +# 1. OpenShell archives — scripts/install-openshell.sh release-asset table +# 2. Brev OpenShell CLI — scripts/brev-launchable-ci-cpu.sh release-asset table # # Usage: # scripts/check-installer-hash.sh # exit 0 if current, 1 if stale @@ -23,7 +23,12 @@ else REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" fi CHECKER_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -OPENSHELL_RELEASE_VERSION="0.0.72" + +readonly -a OPENSHELL_RELEASE_MANIFEST_ALLOWLIST=( + "0.0.72|openshell-checksums-sha256.txt|0049181983eaf925ef9510382f75348229a9511d02e27196107782e7c3259ae1" + "0.0.72|openshell-gateway-checksums-sha256.txt|3c454dc15154b8c700ec820628559ea8964c6e552d9c5f8af78b6ee19cf34547" + "0.0.72|openshell-sandbox-checksums-sha256.txt|d38507501338576437cf3e554df71fefe927dc0d72758f88e260069527ed9ccc" +) case "${1:-}" in "") ;; @@ -57,7 +62,7 @@ sha256_file() { } # invalidState: CI reports trusted OpenShell pins without comparing every -# consumed archive with the immutable v0.0.72 checksum release assets. +# consumed archive with the selected immutable checksum release assets. # sourceBoundary: NVIDIA/OpenShell owns the release assets and their published # digests; NemoClaw owns this independent verification of its local pin table. # In pull-request CI, this checker and its pin parser execute only from the @@ -72,47 +77,20 @@ sha256_file() { check_openshell_release_assets() { local installer="${REPO_ROOT}/scripts/install-openshell.sh" local brev_installer="${REPO_ROOT}/scripts/brev-launchable-ci-cpu.sh" - local release_base="https://github.com/NVIDIA/OpenShell/releases/download/v${OPENSHELL_RELEASE_VERSION}" - local workspace manifests spec manifest expected actual source asset pinned upstream matches - local pin_records parser_error parser_errors + local release_base workspace manifests spec manifest expected actual source asset pinned upstream + local matches required_manifest required_matches + local pin_records parser_error parser_errors parsed_version release_version="" record_extra + local allowlist_entry allowlist_version allowlist_extra local count=0 brev_count=0 published_count=0 failures=0 - local -a manifest_specs=( - "openshell-checksums-sha256.txt:0049181983eaf925ef9510382f75348229a9511d02e27196107782e7c3259ae1" - "openshell-gateway-checksums-sha256.txt:3c454dc15154b8c700ec820628559ea8964c6e552d9c5f8af78b6ee19cf34547" - "openshell-sandbox-checksums-sha256.txt:d38507501338576437cf3e554df71fefe927dc0d72758f88e260069527ed9ccc" - ) + local -a manifest_specs=() workspace=$(mktemp -d) manifests="${workspace}/published-sha256.txt" : >"$manifests" trap 'rm -rf "$workspace"' RETURN - echo "Checking OpenShell v${OPENSHELL_RELEASE_VERSION} release assets..." - for spec in "${manifest_specs[@]}"; do - manifest="${spec%%:*}" - expected="${spec#*:}" - if ! fetch_file "${release_base}/${manifest}" "${workspace}/${manifest}"; then - echo " STALE: unable to download ${manifest}." - failures=$((failures + 1)) - continue - fi - if ! actual=$(sha256_file "${workspace}/${manifest}"); then - echo " STALE: unable to hash ${manifest}." - failures=$((failures + 1)) - continue - fi - if [[ "$actual" != "$expected" ]]; then - echo " STALE: ${manifest} digest does not match the pinned v${OPENSHELL_RELEASE_VERSION} release asset." - echo " pinned: ${expected}" - echo " upstream: ${actual}" - failures=$((failures + 1)) - continue - fi - echo " OK: ${manifest} (${actual})" - cat "${workspace}/${manifest}" >>"$manifests" - done - # invalidState: target-controlled shell formatting hides, duplicates, or - # changes a pin while the trusted release-asset check still reports success. + # mixes a release version while the trusted release-asset check still reports + # success. # sourceBoundary: this parser executes beside the checker only from the # base-trusted checkout or immutable bootstrap, never from the PR head. It # defines the accepted static shell subset; PR-head installers are input data @@ -126,7 +104,6 @@ check_openshell_release_assets() { parser_errors="${workspace}/pin-parser-errors.txt" if ! pin_records=$(node --experimental-strip-types \ "${CHECKER_ROOT}/checks/extract-installer-pins.mts" \ - --release-version "$OPENSHELL_RELEASE_VERSION" \ --installer "$installer" \ --brev-installer "$brev_installer" \ --format tsv 2>"$parser_errors"); then @@ -134,39 +111,123 @@ check_openshell_release_assets() { while IFS= read -r parser_error; do echo " ${parser_error}" done <"$parser_errors" - failures=$((failures + 1)) - else - while IFS=$'\t' read -r source asset pinned; do - if [[ "$source" == "installer" ]]; then - count=$((count + 1)) - else - brev_count=$((brev_count + 1)) - fi - matches=$(awk -v asset="$asset" '$2 == asset { count++ } END { print count + 0 }' "$manifests") - upstream=$(awk -v asset="$asset" '$2 == asset { print $1; exit }' "$manifests") - if [[ "$matches" -eq 1 && "$pinned" == "$upstream" ]]; then - published_count=$((published_count + 1)) - echo " OK: ${source} ${asset} (${pinned})" - else - echo " STALE: ${source} ${asset} does not match exactly one v${OPENSHELL_RELEASE_VERSION} checksum entry." - echo " pinned: ${pinned}" - echo " upstream: ${upstream:-missing}" - echo " matches: ${matches}" - failures=$((failures + 1)) - fi - done <<<"$pin_records" + return 1 fi + while IFS=$'\t' read -r parsed_version source asset pinned record_extra; do + if [[ ! "$parsed_version" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ || -z "$source" || -z "$asset" || -z "$pinned" || -n "$record_extra" ]]; then + echo " STALE: trusted parser returned an invalid installer pin record." + return 1 + fi + if [[ -z "$release_version" ]]; then + release_version="$parsed_version" + elif [[ "$parsed_version" != "$release_version" ]]; then + echo " STALE: trusted parser returned multiple OpenShell release versions." + return 1 + fi + case "$source" in + installer) count=$((count + 1)) ;; + "Brev launchable") brev_count=$((brev_count + 1)) ;; + *) + echo " STALE: trusted parser returned an unknown pin source." + return 1 + ;; + esac + done <<<"$pin_records" + if [[ "$count" -ne 8 ]]; then - echo " STALE: expected 8 pinned OpenShell v${OPENSHELL_RELEASE_VERSION} assets, found ${count}." + echo " STALE: expected 8 pinned OpenShell v${release_version:-unknown} assets, found ${count}." failures=$((failures + 1)) fi if [[ "$brev_count" -ne 2 ]]; then - echo " STALE: expected 2 pinned Brev OpenShell v${OPENSHELL_RELEASE_VERSION} CLI assets, found ${brev_count}." + echo " STALE: expected 2 pinned Brev OpenShell v${release_version:-unknown} CLI assets, found ${brev_count}." failures=$((failures + 1)) fi + if [[ "$failures" -ne 0 ]]; then + return "$failures" + fi + + for allowlist_entry in "${OPENSHELL_RELEASE_MANIFEST_ALLOWLIST[@]}"; do + IFS='|' read -r allowlist_version manifest expected allowlist_extra <<<"$allowlist_entry" + if [[ ! "$allowlist_version" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ || ! "$expected" =~ ^[a-f0-9]{64}$ || -z "$manifest" || -n "$allowlist_extra" ]]; then + echo " STALE: trusted OpenShell release-manifest allowlist is invalid." + return 1 + fi + if [[ "$allowlist_version" == "$release_version" ]]; then + manifest_specs+=("${manifest}:${expected}") + fi + done + + if [[ "${#manifest_specs[@]}" -eq 0 ]]; then + echo " STALE: OpenShell v${release_version} is not in the trusted release-manifest allowlist." + return 1 + fi + if [[ "${#manifest_specs[@]}" -ne 3 ]]; then + echo " STALE: OpenShell v${release_version} does not have exactly three trusted release-manifest digests." + return 1 + fi + for required_manifest in \ + openshell-checksums-sha256.txt \ + openshell-gateway-checksums-sha256.txt \ + openshell-sandbox-checksums-sha256.txt; do + required_matches=0 + for spec in "${manifest_specs[@]}"; do + if [[ "${spec%%:*}" == "$required_manifest" ]]; then + required_matches=$((required_matches + 1)) + fi + done + if [[ "$required_matches" -ne 1 ]]; then + echo " STALE: OpenShell v${release_version} does not have exactly one trusted ${required_manifest} digest." + failures=$((failures + 1)) + fi + done + if [[ "$failures" -ne 0 ]]; then + return "$failures" + fi + + release_base="https://github.com/NVIDIA/OpenShell/releases/download/v${release_version}" + echo "Checking OpenShell v${release_version} release assets..." + for spec in "${manifest_specs[@]}"; do + manifest="${spec%%:*}" + expected="${spec#*:}" + if ! fetch_file "${release_base}/${manifest}" "${workspace}/${manifest}"; then + echo " STALE: unable to download ${manifest}." + failures=$((failures + 1)) + continue + fi + if ! actual=$(sha256_file "${workspace}/${manifest}"); then + echo " STALE: unable to hash ${manifest}." + failures=$((failures + 1)) + continue + fi + if [[ "$actual" != "$expected" ]]; then + echo " STALE: ${manifest} digest does not match the pinned v${release_version} release asset." + echo " pinned: ${expected}" + echo " upstream: ${actual}" + failures=$((failures + 1)) + continue + fi + echo " OK: ${manifest} (${actual})" + cat "${workspace}/${manifest}" >>"$manifests" + done + + while IFS=$'\t' read -r parsed_version source asset pinned record_extra; do + matches=$(awk -v asset="$asset" '$2 == asset { count++ } END { print count + 0 }' "$manifests") + upstream=$(awk -v asset="$asset" '$2 == asset { print $1; exit }' "$manifests") + if [[ "$matches" -eq 1 && "$pinned" == "$upstream" ]]; then + published_count=$((published_count + 1)) + echo " OK: ${source} ${asset} (${pinned})" + else + echo " STALE: ${source} ${asset} does not match exactly one v${release_version} checksum entry." + echo " pinned: ${pinned}" + echo " upstream: ${upstream:-missing}" + echo " matches: ${matches}" + failures=$((failures + 1)) + fi + done <<<"$pin_records" + if [[ "$published_count" -ne 10 ]]; then - echo " STALE: expected all 10 pinned asset references in the v${OPENSHELL_RELEASE_VERSION} checksum manifests, matched ${published_count}." + echo " STALE: expected all 10 pinned asset references in the v${release_version} checksum manifests, matched ${published_count}." failures=$((failures + 1)) fi return "$failures" diff --git a/scripts/checks/dependency-pins.ts b/scripts/checks/dependency-pins.ts index 510755b0ac8..4e44a700203 100644 --- a/scripts/checks/dependency-pins.ts +++ b/scripts/checks/dependency-pins.ts @@ -31,6 +31,11 @@ type DependencyPins = Readonly<{ const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); const OPENCLAW_VERSION_ARG_SUFFIX_RE = /[.-]/g; const NUMERIC_VERSION_RE = /^[0-9]+\.[0-9]+\.[0-9]+$/; +const OPENSHELL_RELEASE_MANIFESTS = [ + "openshell-checksums-sha256.txt", + "openshell-gateway-checksums-sha256.txt", + "openshell-sandbox-checksums-sha256.txt", +] as const; function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); @@ -263,6 +268,29 @@ function requireVersionReference( } } +function requireOpenShellReleaseManifestAllowlist( + source: string, + expectedVersion: string, + failures: string[], +): void { + const entries = [ + ...source.matchAll(/^\s*"([0-9]+\.[0-9]+\.[0-9]+)\|([^|"\s]+)\|([a-f0-9]{64})"\s*$/gm), + ] + .filter((match) => match[1] === expectedVersion) + .map((match) => match[2]) + .filter((manifest): manifest is string => manifest !== undefined); + const complete = + entries.length === OPENSHELL_RELEASE_MANIFESTS.length && + OPENSHELL_RELEASE_MANIFESTS.every( + (manifest) => entries.filter((entry) => entry === manifest).length === 1, + ); + if (!complete) { + failures.push( + `OpenShell release-manifest allowlist: expected one complete entry for ${expectedVersion}`, + ); + } +} + function verifyOpenShellPins( pins: OpenShellPins, sources: { @@ -309,17 +337,7 @@ function verifyOpenShellPins( "OpenShell installer PIN_VERSION", failures, ); - compare( - extractSingle( - sources.installerHashCheck, - /^OPENSHELL_RELEASE_VERSION="([^"]+)"\s*$/gm, - "OpenShell installer hash release", - failures, - ), - pins.maxVersion, - "OpenShell installer hash release", - failures, - ); + requireOpenShellReleaseManifestAllowlist(sources.installerHashCheck, pins.maxVersion, failures); compare( extractSingle( sources.openshellVersion, diff --git a/scripts/checks/extract-installer-pins.mts b/scripts/checks/extract-installer-pins.mts index e726b085690..afc4f7f360e 100644 --- a/scripts/checks/extract-installer-pins.mts +++ b/scripts/checks/extract-installer-pins.mts @@ -12,13 +12,13 @@ type Token = { export type InstallerPin = { asset: string; + releaseVersion: string; sha256: string; source: string; }; type ExtractOptions = { functionName: string; - releaseVersion: string; sourceLabel: string; }; @@ -26,7 +26,6 @@ type CliOptions = { brevInstaller: string; format: "json" | "tsv"; installer: string; - releaseVersion: string; }; const FUNCTION_LOCAL_PATTERN = /^local release_tag\s*=\s*\$1 asset\s*=\s*\$2$/u; @@ -314,7 +313,12 @@ function staticPinFromArm(pattern: string, commandTokens: Token[]): InstallerPin if (!SHA256_PATTERN.test(sha256)) { fail(`case arm ${pattern} does not contain one literal lowercase SHA-256 digest`); } - return { asset: match[2] ?? "", sha256, source: "" }; + return { + asset: match[2] ?? "", + releaseVersion: match[1] ?? "", + sha256, + source: "", + }; } // invalidState: trusted CI accepts a pin table whose shell formatting hides, @@ -373,7 +377,7 @@ export function extractInstallerPins(source: string, options: ExtractOptions): I const pin = staticPinFromArm(pattern.value, body.slice(commandStart, cursor)); if (pattern.value === "*") { fallbackCount += 1; - } else if (pin && pattern.value.startsWith(`v${options.releaseVersion}:`)) { + } else if (pin) { pins.push({ ...pin, source: options.sourceLabel }); } cursor = skipSeparators(body, cursor + 1); @@ -386,6 +390,16 @@ export function extractInstallerPins(source: string, options: ExtractOptions): I fail(`${options.functionName} must contain exactly one fail-closed fallback arm`); } + if (pins.length === 0) { + fail(`${options.functionName} contains no versioned pins`); + } + const releaseVersions = [...new Set(pins.map((pin) => pin.releaseVersion))].sort(); + if (releaseVersions.length !== 1) { + fail( + `${options.functionName} must contain exactly one release version, found ${releaseVersions.join(", ")}`, + ); + } + const duplicateAssets = pins .map((pin) => pin.asset) .filter((asset, index, assets) => assets.indexOf(asset) !== index); @@ -394,9 +408,6 @@ export function extractInstallerPins(source: string, options: ExtractOptions): I `${options.functionName} contains duplicate assets: ${[...new Set(duplicateAssets)].join(", ")}`, ); } - if (pins.length === 0) { - fail(`${options.functionName} contains no v${options.releaseVersion} pins`); - } return pins; } @@ -407,7 +418,7 @@ function parseCliOptions(argv: string[]): CliOptions { const value = argv[index + 1] ?? ""; if (!option.startsWith("--") || !value) { fail( - "usage: extract-installer-pins.mts --release-version VERSION --installer PATH --brev-installer PATH [--format json|tsv]", + "usage: extract-installer-pins.mts --installer PATH --brev-installer PATH [--format json|tsv]", ); } if (values.has(option)) { @@ -415,48 +426,51 @@ function parseCliOptions(argv: string[]): CliOptions { } values.set(option, value); } - const releaseVersion = values.get("--release-version") ?? ""; const installer = values.get("--installer") ?? ""; const brevInstaller = values.get("--brev-installer") ?? ""; const format = values.get("--format") ?? "json"; - const allowedOptions = new Set([ - "--brev-installer", - "--format", - "--installer", - "--release-version", - ]); + const allowedOptions = new Set(["--brev-installer", "--format", "--installer"]); const unknownOptions = [...values.keys()].filter((option) => !allowedOptions.has(option)); if ( unknownOptions.length > 0 || - !/^[0-9]+\.[0-9]+\.[0-9]+$/u.test(releaseVersion) || !installer || !brevInstaller || (format !== "json" && format !== "tsv") ) { fail(`invalid CLI options${unknownOptions.length > 0 ? `: ${unknownOptions.join(", ")}` : ""}`); } - return { brevInstaller, format, installer, releaseVersion }; + return { brevInstaller, format, installer }; } function runCli(): void { const options = parseCliOptions(process.argv.slice(2)); - const pins = [ - ...extractInstallerPins(readInstallerInput(options.installer, "installer"), { - functionName: "openshell_pinned_sha256", - releaseVersion: options.releaseVersion, - sourceLabel: "installer", - }), - ...extractInstallerPins(readInstallerInput(options.brevInstaller, "Brev launchable"), { + const installerPins = extractInstallerPins(readInstallerInput(options.installer, "installer"), { + functionName: "openshell_pinned_sha256", + sourceLabel: "installer", + }); + const brevPins = extractInstallerPins( + readInstallerInput(options.brevInstaller, "Brev launchable"), + { functionName: "openshell_cli_pinned_sha256", - releaseVersion: options.releaseVersion, sourceLabel: "Brev launchable", - }), - ]; + }, + ); + const pins = [...installerPins, ...brevPins]; + const releaseVersions = [...new Set(pins.map((pin) => pin.releaseVersion))].sort(); + if (releaseVersions.length !== 1) { + fail( + `installer and Brev launchable pin tables must use the same release version, found ${releaseVersions.join(", ")}`, + ); + } if (options.format === "json") { process.stdout.write(`${JSON.stringify(pins)}\n`); return; } - process.stdout.write(pins.map((pin) => `${pin.source}\t${pin.asset}\t${pin.sha256}`).join("\n")); + process.stdout.write( + pins + .map((pin) => `${pin.releaseVersion}\t${pin.source}\t${pin.asset}\t${pin.sha256}`) + .join("\n"), + ); process.stdout.write("\n"); } diff --git a/test/dependency-pins-check.test.ts b/test/dependency-pins-check.test.ts index 2f28cfa00a8..9599a54d1f9 100644 --- a/test/dependency-pins-check.test.ts +++ b/test/dependency-pins-check.test.ts @@ -18,6 +18,12 @@ const ALTERNATE_INTEGRITY = "sha512-PzSJiYqmwpTudmakYs2oCJ57OW3VwEJYf8buTuKvuRvcYEUf/KOTu2dD6pLf2XYgDKErpvcDaoSAJ1nGCyvzAA=="; const HERMES_SEMVER = "7.8.9"; const MAP_SHA256 = "b".repeat(64); +const MANIFEST_SHA256 = "c".repeat(64); +const OPENSHELL_RELEASE_MANIFESTS = [ + "openshell-checksums-sha256.txt", + "openshell-gateway-checksums-sha256.txt", + "openshell-sandbox-checksums-sha256.txt", +] as const; type FixtureOverrides = Partial>; @@ -40,6 +46,17 @@ function writeFixture(root: string, overrides: FixtureOverrides = {}): void { `https://registry.npmjs.org/openclaw/-/openclaw-${openclawVersion}.tgz`; const openclawArg = `OPENCLAW_${openclawVersion.replace(/[.-]/g, "_")}`; const hermesSemver = overrides.hermesSemver ?? HERMES_SEMVER; + const installerHashVersions = [ + overrides.installerHashExtraVersion, + overrides.installerHashVersion ?? openshellMax, + ].filter((version): version is string => version !== undefined); + const installerHashAllowlist = installerHashVersions + .flatMap((version) => + OPENSHELL_RELEASE_MANIFESTS.filter( + (manifest) => manifest !== overrides.installerHashOmitManifest, + ).map((manifest) => ` "${version}|${manifest}|${MANIFEST_SHA256}"`), + ) + .join("\n"); const files: Record = { "nemoclaw-blueprint/blueprint.yaml": ` @@ -52,7 +69,9 @@ MAX_VERSION="${overrides.installerMax ?? openshellMax}" PIN_VERSION="${overrides.installerPinExpression ?? "$MAX_VERSION"}" `, "scripts/check-installer-hash.sh": ` -OPENSHELL_RELEASE_VERSION="${overrides.installerHashVersion ?? openshellMax}" +readonly -a OPENSHELL_RELEASE_MANIFEST_ALLOWLIST=( +${installerHashAllowlist} +) `, "scripts/brev-launchable-ci-cpu.sh": ` case "$NEMOCLAW_REF" in @@ -177,6 +196,14 @@ describe("dependency pin drift check", () => { ); }); + it("accepts the blueprint maximum in a multi-release manifest allowlist (#5242)", () => { + withFixture( + "nemoclaw-dependency-pins-multi-release-", + { installerHashExtraVersion: "1.2.3" }, + (root) => expect(verifyDependencyPins(root)).toEqual([]), + ); + }); + it("reports exact operational consumer drift (#5242)", () => { withFixture( "nemoclaw-dependency-pins-drift-", @@ -210,7 +237,7 @@ describe("dependency pin drift check", () => { "OpenShell installer MIN_VERSION: expected 1.2.3, found 1.2.2", "OpenShell installer MAX_VERSION: expected 1.2.4, found 1.2.3", "OpenShell installer PIN_VERSION: expected $MAX_VERSION, found 1.2.4", - "OpenShell installer hash release: expected 1.2.4, found 1.2.3", + "OpenShell release-manifest allowlist: expected one complete entry for 1.2.4", "OpenShell supported fallback version: expected 1.2.4, found 1.2.3", "OpenShell minimum fallback version: expected 1.2.3, found 1.2.2", "OpenShell supervisor manifest digest map: expected a reference to 1.2.4", @@ -258,6 +285,18 @@ describe("dependency pin drift check", () => { }); }); + it("rejects an incomplete manifest allowlist entry for the blueprint maximum (#5242)", () => { + withFixture( + "nemoclaw-dependency-pins-incomplete-openshell-allowlist-", + { installerHashOmitManifest: "openshell-sandbox-checksums-sha256.txt" }, + (root) => { + expect(verifyDependencyPins(root)).toEqual([ + "OpenShell release-manifest allowlist: expected one complete entry for 1.2.4", + ]); + }, + ); + }); + it("rejects an ambiguous operational authority (#5242)", () => { withFixture( "nemoclaw-dependency-pins-ambiguous-", diff --git a/test/installer-hash-check.test.ts b/test/installer-hash-check.test.ts index b46f040f2c0..fe415f21738 100644 --- a/test/installer-hash-check.test.ts +++ b/test/installer-hash-check.test.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -47,11 +48,15 @@ const ASSETS = [...ASSET_DIGESTS.keys()]; const UNPUBLISHED_ASSET = "openshell-sandbox-aarch64-unknown-linux-gnu-unpublished.tar.gz"; const SYMLINK_INPUT_MARKER = "LEAK565"; type FixtureMode = + | "allowlisted-alternate-version" | "brev-mismatch" | "complete" | "duplicate-brev-pin" | "failure" + | "incomplete-trusted-allowlist" + | "mismatched-table-versions" | "missing-brev-pin" + | "multiple-installer-versions" | "non-regular-brev-input" | "oversized-installer-input" | "partial" @@ -79,10 +84,13 @@ const BREV_MUTATIONS: Partial string>> = }, "missing-brev-pin": (source) => source.replace(ASSET_DIGESTS.get(ASSETS[1]) ?? "missing", "missing"), + "mismatched-table-versions": (source) => source.replaceAll("v0.0.72:", "v0.0.73:"), "pr-checker-bypass": corruptFirstBrevPin, "pr-parser-bypass": corruptFirstBrevPin, }; const INSTALLER_MUTATIONS: Partial string>> = { + "multiple-installer-versions": (source) => + source.replace(`v0.0.72:${ASSETS[0]}`, `v0.0.73:${ASSETS[0]}`), "partial-asset-missing": (source) => source.replace(ASSETS.at(-1) ?? "missing", UNPUBLISHED_ASSET), }; @@ -216,12 +224,10 @@ function createFixture( tempDirs.push(fixtureRoot); fs.mkdirSync(checksDir, { recursive: true }); fs.mkdirSync(binDir, { recursive: true }); - const checker = fs - .readFileSync(path.join(REPO_ROOT, "scripts", "check-installer-hash.sh"), "utf8") - .replace( - 'OPENSHELL_RELEASE_VERSION="0.0.72"', - `OPENSHELL_RELEASE_VERSION="${openshellVersion}"`, - ); + const checker = fs.readFileSync( + path.join(REPO_ROOT, "scripts", "check-installer-hash.sh"), + "utf8", + ); fs.writeFileSync(path.join(scriptsDir, "check-installer-hash.sh"), checker); fs.copyFileSync( path.join(REPO_ROOT, "scripts", "checks", "extract-installer-pins.mts"), @@ -318,6 +324,32 @@ function runFixture( : fs.readFileSync(targetChecker, "utf8"), ); const checker = trustedChecker ? trustedCheckerPath : targetChecker; + if (mode === "allowlisted-alternate-version") { + const alternateEntries = [...CHECKSUM_MANIFESTS.entries()] + .map( + ([manifest, contents]) => + ` "9.9.9|${manifest}|${createHash("sha256").update(contents).digest("hex")}"`, + ) + .join("\n"); + const checkerSource = fs.readFileSync(checker, "utf8"); + fs.writeFileSync( + checker, + checkerSource.replace( + "readonly -a OPENSHELL_RELEASE_MANIFEST_ALLOWLIST=(\n", + `readonly -a OPENSHELL_RELEASE_MANIFEST_ALLOWLIST=(\n${alternateEntries}\n`, + ), + ); + } + if (mode === "incomplete-trusted-allowlist") { + const checkerSource = fs.readFileSync(checker, "utf8"); + fs.writeFileSync( + checker, + checkerSource.replace( + /^\s*"0\.0\.72\|openshell-sandbox-checksums-sha256\.txt\|[a-f0-9]{64}"\s*$/m, + "", + ), + ); + } const installer = path.join(fixtureRoot, "scripts", "install-openshell.sh"); const installerSource = fs.readFileSync(installer, "utf8"); const mutateInstaller = INSTALLER_MUTATIONS[mode] ?? ((source: string) => source); @@ -357,14 +389,63 @@ describe("installer hash verification", () => { expect(result.stdout).toContain("All installer hashes are current"); }); - it("uses the single release-version constant for release URLs and pin selection", () => { - const result = runFixture("complete", "9.9.9"); + it("derives the release version from matching static installer pin tables", () => { + const result = runFixture("complete", undefined, true); + + expect(result.status).toBe(0); + expect(result.stdout).toContain("Checking OpenShell v0.0.72 release assets"); + expect(result.stdout).toContain("All installer hashes are current"); + }); + + it("selects a second complete trusted release from the allowlist", () => { + const result = runFixture("allowlisted-alternate-version", "9.9.9", true); expect(result.status).toBe(0); expect(result.stdout).toContain("Checking OpenShell v9.9.9 release assets"); expect(result.stdout).toContain("All installer hashes are current"); }); + it("fails closed when the derived release is not allowlisted", () => { + const result = runFixture("complete", "9.9.9", true); + + expect(result.status).toBe(1); + expect(result.stdout).toContain( + "OpenShell v9.9.9 is not in the trusted release-manifest allowlist", + ); + expect(result.stdout).not.toContain("Checking OpenShell v9.9.9 release assets"); + expect(result.stdout).not.toContain("All installer hashes are current"); + }); + + it("fails closed when an allowlisted release lacks all three manifest digests", () => { + const result = runFixture("incomplete-trusted-allowlist", undefined, true); + + expect(result.status).toBe(1); + expect(result.stdout).toContain( + "OpenShell v0.0.72 does not have exactly three trusted release-manifest digests", + ); + expect(result.stdout).not.toContain("Checking OpenShell v0.0.72 release assets"); + expect(result.stdout).not.toContain("All installer hashes are current"); + }); + + it.each([ + [ + "multiple-installer-versions", + "openshell_pinned_sha256 must contain exactly one release version, found 0.0.72, 0.0.73", + ], + [ + "mismatched-table-versions", + "installer and Brev launchable pin tables must use the same release version, found 0.0.72, 0.0.73", + ], + ] as const)("fails closed for %s", (mode, diagnostic) => { + const result = runFixture(mode, undefined, true); + + expect(result.status).toBe(1); + expect(result.stdout).toContain("unable to extract the OpenShell installer pin tables"); + expect(result.stdout).toContain(diagnostic); + expect(result.stdout).not.toContain("Checking OpenShell v0.0.72 release assets"); + expect(result.stdout).not.toContain("All installer hashes are current"); + }); + it.each([ "equals-whitespace", "comments", @@ -394,7 +475,6 @@ describe("installer hash verification", () => { expect(result.status).toBe(1); expect(result.stdout).toContain("unable to extract the OpenShell installer pin tables"); - expect(result.stdout).toContain("expected 2 pinned Brev OpenShell v0.0.72 CLI assets"); expect(result.stdout).not.toContain("All installer hashes are current"); }); From 12a6b1dc319f56529acc5b8b3becf81262c648c3 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 15:31:36 -0700 Subject: [PATCH 11/50] fix(ci): require exact OpenShell release asset sets Signed-off-by: Aaron Erickson --- .github/workflows/installer-hash-check.yaml | 2 +- scripts/checks/extract-installer-pins.mts | 43 ++++++++++++++++++ test/installer-hash-check.test.ts | 50 +++++++++++++++++++-- 3 files changed, 90 insertions(+), 5 deletions(-) diff --git a/.github/workflows/installer-hash-check.yaml b/.github/workflows/installer-hash-check.yaml index 4e137638c7b..30239c362eb 100644 --- a/.github/workflows/installer-hash-check.yaml +++ b/.github/workflows/installer-hash-check.yaml @@ -2,7 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 # # Verifies pinned installer SHA-256 hashes still match upstream scripts. -# Checked: OpenShell v0.0.72 installer and Brev release assets. +# Checked: allowlisted OpenShell installer and Brev release assets. # Reports the required network-backed drift check on every PR, every push to # main, and weekly. Pull requests execute checker code from their base commit; # the immutable bootstrap is used only for the PR that first adds that action. diff --git a/scripts/checks/extract-installer-pins.mts b/scripts/checks/extract-installer-pins.mts index afc4f7f360e..3cba0d89f99 100644 --- a/scripts/checks/extract-installer-pins.mts +++ b/scripts/checks/extract-installer-pins.mts @@ -33,6 +33,20 @@ const LITERAL_PIN_PATTERN = /^v([0-9]+\.[0-9]+\.[0-9]+):([A-Za-z0-9._+-]+)$/u; const SHA256_PATTERN = /^[a-f0-9]{64}$/u; const FUNCTION_SELECTOR_VALUES = new Set(["${release_tag}:${asset}", "$release_tag:$asset"]); const MAX_INSTALLER_INPUT_BYTES = 1024 * 1024; +const EXPECTED_INSTALLER_ASSETS = [ + "openshell-x86_64-unknown-linux-musl.tar.gz", + "openshell-aarch64-unknown-linux-musl.tar.gz", + "openshell-aarch64-apple-darwin.tar.gz", + "openshell-gateway-x86_64-unknown-linux-gnu.tar.gz", + "openshell-gateway-aarch64-unknown-linux-gnu.tar.gz", + "openshell-gateway-aarch64-apple-darwin.tar.gz", + "openshell-sandbox-x86_64-unknown-linux-gnu.tar.gz", + "openshell-sandbox-aarch64-unknown-linux-gnu.tar.gz", +] as const; +const EXPECTED_BREV_ASSETS = [ + "openshell-x86_64-unknown-linux-musl.tar.gz", + "openshell-aarch64-unknown-linux-musl.tar.gz", +] as const; function fail(message: string): never { throw new Error(`Installer pin extraction failed: ${message}`); @@ -105,6 +119,33 @@ function readInstallerInput(inputPath: string, sourceLabel: string): string { } } +// invalidState: base-trusted CI accepts the right number of valid published +// hashes while a pull request swaps in a different official release asset. +// sourceBoundary: these expected asset names live only in base-trusted parser +// code; the PR-head installer and Brev script remain inert input data. +// whyNotSourceFix: OpenShell can attest what it publishes but cannot determine +// which exact downstream assets NemoClaw consumes. +// regressionTest: test/installer-hash-check.test.ts substitutes official but +// unexpected assets while keeping valid upstream digests and record counts. +// removalCondition: remove this set check only when one base-trusted canonical +// dependency manifest directly drives both installer consumers. +function assertExactAssetSet( + pins: InstallerPin[], + expectedAssets: readonly string[], + label: string, +): void { + const actual = [...new Set(pins.map((pin) => pin.asset))].sort(); + const expected = [...expectedAssets].sort(); + const missing = expected.filter((asset) => !actual.includes(asset)); + const unexpected = actual.filter((asset) => !expected.includes(asset)); + if (missing.length > 0 || unexpected.length > 0) { + fail( + `${label} must contain the exact consumed asset set; ` + + `missing=[${missing.join(", ")}], unexpected=[${unexpected.join(", ")}]`, + ); + } +} + function isOperatorStart(character: string): boolean { return "(){};".includes(character); } @@ -455,6 +496,8 @@ function runCli(): void { sourceLabel: "Brev launchable", }, ); + assertExactAssetSet(installerPins, EXPECTED_INSTALLER_ASSETS, "installer pin table"); + assertExactAssetSet(brevPins, EXPECTED_BREV_ASSETS, "Brev pin table"); const pins = [...installerPins, ...brevPins]; const releaseVersions = [...new Set(pins.map((pin) => pin.releaseVersion))].sort(); if (releaseVersions.length !== 1) { diff --git a/test/installer-hash-check.test.ts b/test/installer-hash-check.test.ts index fe415f21738..14eecb21458 100644 --- a/test/installer-hash-check.test.ts +++ b/test/installer-hash-check.test.ts @@ -46,6 +46,12 @@ const ASSET_DIGESTS = new Map([ ]); const ASSETS = [...ASSET_DIGESTS.keys()]; const UNPUBLISHED_ASSET = "openshell-sandbox-aarch64-unknown-linux-gnu-unpublished.tar.gz"; +const OFFICIAL_UNEXPECTED_INSTALLER_ASSET = "openshell-driver-vm-x86_64-unknown-linux-gnu.tar.gz"; +const OFFICIAL_UNEXPECTED_INSTALLER_DIGEST = + "911dd804074c620b3ba353f17e39a8195222c0764072621a154164432d7906d0"; +const OFFICIAL_UNEXPECTED_BREV_ASSET = "openshell-driver-vm-aarch64-unknown-linux-gnu.tar.gz"; +const OFFICIAL_UNEXPECTED_BREV_DIGEST = + "5e6ba04030938e7be21b8b83af9a34b888deffb4c65e7e70dd6845c3bc7e264f"; const SYMLINK_INPUT_MARKER = "LEAK565"; type FixtureMode = | "allowlisted-alternate-version" @@ -58,6 +64,8 @@ type FixtureMode = | "missing-brev-pin" | "multiple-installer-versions" | "non-regular-brev-input" + | "official-but-unexpected-brev-asset" + | "official-but-unexpected-installer-asset" | "oversized-installer-input" | "partial" | "partial-asset-missing" @@ -85,12 +93,23 @@ const BREV_MUTATIONS: Partial string>> = "missing-brev-pin": (source) => source.replace(ASSET_DIGESTS.get(ASSETS[1]) ?? "missing", "missing"), "mismatched-table-versions": (source) => source.replaceAll("v0.0.72:", "v0.0.73:"), + "official-but-unexpected-brev-asset": (source) => + source + .replace(ASSETS[1] ?? "missing", OFFICIAL_UNEXPECTED_BREV_ASSET) + .replace(ASSET_DIGESTS.get(ASSETS[1] ?? "") ?? "missing", OFFICIAL_UNEXPECTED_BREV_DIGEST), "pr-checker-bypass": corruptFirstBrevPin, "pr-parser-bypass": corruptFirstBrevPin, }; const INSTALLER_MUTATIONS: Partial string>> = { "multiple-installer-versions": (source) => source.replace(`v0.0.72:${ASSETS[0]}`, `v0.0.73:${ASSETS[0]}`), + "official-but-unexpected-installer-asset": (source) => + source + .replace(ASSETS.at(-1) ?? "missing", OFFICIAL_UNEXPECTED_INSTALLER_ASSET) + .replace( + ASSET_DIGESTS.get(ASSETS.at(-1) ?? "") ?? "missing", + OFFICIAL_UNEXPECTED_INSTALLER_DIGEST, + ), "partial-asset-missing": (source) => source.replace(ASSETS.at(-1) ?? "missing", UNPUBLISHED_ASSET), }; @@ -446,6 +465,28 @@ describe("installer hash verification", () => { expect(result.stdout).not.toContain("All installer hashes are current"); }); + it.each([ + [ + "official-but-unexpected-installer-asset", + "installer pin table must contain the exact consumed asset set", + OFFICIAL_UNEXPECTED_INSTALLER_ASSET, + ], + [ + "official-but-unexpected-brev-asset", + "Brev pin table must contain the exact consumed asset set", + OFFICIAL_UNEXPECTED_BREV_ASSET, + ], + ] as const)("rejects %s despite a valid published digest", (mode, diagnostic, unexpected) => { + const result = runFixture(mode, undefined, true); + + expect(result.status).toBe(1); + expect(result.stdout).toContain("unable to extract the OpenShell installer pin tables"); + expect(result.stdout).toContain(diagnostic); + expect(result.stdout).toContain(`unexpected=[${unexpected}]`); + expect(result.stdout).not.toContain("Checking OpenShell v0.0.72 release assets"); + expect(result.stdout).not.toContain("All installer hashes are current"); + }); + it.each([ "equals-whitespace", "comments", @@ -553,15 +594,16 @@ describe("installer hash verification", () => { expect(result.stdout).not.toContain("All installer hashes are current"); }); - it("fails closed when a pinned installer asset is absent from every manifest", () => { + it("fails closed when a pinned installer asset is outside the exact consumed set", () => { const result = runFixture("partial-asset-missing"); expect(result.status).toBe(1); + expect(result.stdout).toContain("unable to extract the OpenShell installer pin tables"); expect(result.stdout).toContain( - `STALE: installer ${UNPUBLISHED_ASSET} does not match exactly one v0.0.72 checksum entry`, + "installer pin table must contain the exact consumed asset set", ); - expect(result.stdout).toContain("upstream: missing"); - expect(result.stdout).toContain("matches: 0"); + expect(result.stdout).toContain(`unexpected=[${UNPUBLISHED_ASSET}]`); + expect(result.stdout).not.toContain("Checking OpenShell v0.0.72 release assets"); expect(result.stdout).not.toContain("All installer hashes are current"); }); From ece87f21739aa6a53b58aec41dfa572e2d43a136 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 16:12:56 -0700 Subject: [PATCH 12/50] fix(ci): bind OpenShell hashes to runtime selectors Signed-off-by: Aaron Erickson --- scripts/check-installer-hash.sh | 1 + scripts/checks/extract-installer-pins.mts | 96 ++++++++++++++++++++--- test/installer-hash-check.test.ts | 75 +++++++++++++++++- 3 files changed, 157 insertions(+), 15 deletions(-) diff --git a/scripts/check-installer-hash.sh b/scripts/check-installer-hash.sh index dedd4c4b1f9..83ac495eeab 100755 --- a/scripts/check-installer-hash.sh +++ b/scripts/check-installer-hash.sh @@ -104,6 +104,7 @@ check_openshell_release_assets() { parser_errors="${workspace}/pin-parser-errors.txt" if ! pin_records=$(node --experimental-strip-types \ "${CHECKER_ROOT}/checks/extract-installer-pins.mts" \ + --blueprint "${REPO_ROOT}/nemoclaw-blueprint/blueprint.yaml" \ --installer "$installer" \ --brev-installer "$brev_installer" \ --format tsv 2>"$parser_errors"); then diff --git a/scripts/checks/extract-installer-pins.mts b/scripts/checks/extract-installer-pins.mts index 3cba0d89f99..b3a2fdcab02 100644 --- a/scripts/checks/extract-installer-pins.mts +++ b/scripts/checks/extract-installer-pins.mts @@ -23,6 +23,7 @@ type ExtractOptions = { }; type CliOptions = { + blueprint: string; brevInstaller: string; format: "json" | "tsv"; installer: string; @@ -146,6 +147,67 @@ function assertExactAssetSet( } } +// invalidState: the blueprint and stable runtime selectors request a newer +// OpenShell release while both embedded hash tables still name an older, +// independently valid release, so separate dependency and hash checks pass but +// installation cannot find a hash for the selected version. +// sourceBoundary: this base-trusted parser reads the PR blueprint and installer +// sources only as inert, bounded files and binds every stable selector to the +// single release extracted from the static hash tables. +// whyNotSourceFix: OpenShell can attest its release but cannot keep NemoClaw's +// blueprint, installer selector, Brev selector, and embedded tables coherent. +// regressionTest: test/installer-hash-check.test.ts moves all runtime consumers +// to 0.0.82 while leaving both valid pin tables at 0.0.72 and requires failure. +// removalCondition: remove these comparisons only when one base-trusted, +// machine-readable pin manifest directly drives every runtime consumer. +function extractSingleVersion( + source: string, + pattern: RegExp, + label: string, + captureIndex = 1, +): string { + const flags = pattern.flags.includes("g") ? pattern.flags : `${pattern.flags}g`; + const matches = [...source.matchAll(new RegExp(pattern.source, flags))]; + const version = matches[0]?.[captureIndex]; + if (matches.length !== 1 || !version) { + fail(`${label} must contain exactly one literal X.Y.Z version`); + } + return version; +} + +function extractBlueprintMaxVersion(source: string): string { + return extractSingleVersion( + source, + /^max_openshell_version:\s*(["'])([0-9]+\.[0-9]+\.[0-9]+)\1\s*$/gm, + "blueprint max_openshell_version", + 2, + ); +} + +function extractInstallerRuntimeVersion(source: string): string { + const maxVersion = extractSingleVersion( + source, + /^MAX_VERSION="([0-9]+\.[0-9]+\.[0-9]+)"\s*$/gm, + "installer MAX_VERSION", + ); + const pinVersionAssignments = [...source.matchAll(/^PIN_VERSION=(.*)\s*$/gm)]; + if ( + pinVersionAssignments.length !== 1 || + pinVersionAssignments[0]?.[1]?.trim() !== '"$MAX_VERSION"' + ) { + fail('installer PIN_VERSION must be exactly "$MAX_VERSION"'); + } + return maxVersion; +} + +function extractBrevStableRuntimeVersion(source: string): string { + return extractSingleVersion( + source, + /^\s*stable\s*\|\s*auto\)\s*OPENSHELL_VERSION="v([0-9]+\.[0-9]+\.[0-9]+)"\s*;;\s*$/gm, + "Brev stable OpenShell default", + ); +} + function isOperatorStart(character: string): boolean { return "(){};".includes(character); } @@ -459,7 +521,7 @@ function parseCliOptions(argv: string[]): CliOptions { const value = argv[index + 1] ?? ""; if (!option.startsWith("--") || !value) { fail( - "usage: extract-installer-pins.mts --installer PATH --brev-installer PATH [--format json|tsv]", + "usage: extract-installer-pins.mts --blueprint PATH --installer PATH --brev-installer PATH [--format json|tsv]", ); } if (values.has(option)) { @@ -467,35 +529,37 @@ function parseCliOptions(argv: string[]): CliOptions { } values.set(option, value); } + const blueprint = values.get("--blueprint") ?? ""; const installer = values.get("--installer") ?? ""; const brevInstaller = values.get("--brev-installer") ?? ""; const format = values.get("--format") ?? "json"; - const allowedOptions = new Set(["--brev-installer", "--format", "--installer"]); + const allowedOptions = new Set(["--blueprint", "--brev-installer", "--format", "--installer"]); const unknownOptions = [...values.keys()].filter((option) => !allowedOptions.has(option)); if ( unknownOptions.length > 0 || + !blueprint || !installer || !brevInstaller || (format !== "json" && format !== "tsv") ) { fail(`invalid CLI options${unknownOptions.length > 0 ? `: ${unknownOptions.join(", ")}` : ""}`); } - return { brevInstaller, format, installer }; + return { blueprint, brevInstaller, format, installer }; } function runCli(): void { const options = parseCliOptions(process.argv.slice(2)); - const installerPins = extractInstallerPins(readInstallerInput(options.installer, "installer"), { + const blueprintSource = readInstallerInput(options.blueprint, "blueprint"); + const installerSource = readInstallerInput(options.installer, "installer"); + const brevInstallerSource = readInstallerInput(options.brevInstaller, "Brev launchable"); + const installerPins = extractInstallerPins(installerSource, { functionName: "openshell_pinned_sha256", sourceLabel: "installer", }); - const brevPins = extractInstallerPins( - readInstallerInput(options.brevInstaller, "Brev launchable"), - { - functionName: "openshell_cli_pinned_sha256", - sourceLabel: "Brev launchable", - }, - ); + const brevPins = extractInstallerPins(brevInstallerSource, { + functionName: "openshell_cli_pinned_sha256", + sourceLabel: "Brev launchable", + }); assertExactAssetSet(installerPins, EXPECTED_INSTALLER_ASSETS, "installer pin table"); assertExactAssetSet(brevPins, EXPECTED_BREV_ASSETS, "Brev pin table"); const pins = [...installerPins, ...brevPins]; @@ -505,6 +569,16 @@ function runCli(): void { `installer and Brev launchable pin tables must use the same release version, found ${releaseVersions.join(", ")}`, ); } + const releaseVersion = releaseVersions[0] ?? fail("installer pin tables contain no release"); + for (const [label, runtimeVersion] of [ + ["blueprint max_openshell_version", extractBlueprintMaxVersion(blueprintSource)], + ["installer MAX_VERSION", extractInstallerRuntimeVersion(installerSource)], + ["Brev stable OpenShell default", extractBrevStableRuntimeVersion(brevInstallerSource)], + ] as const) { + if (runtimeVersion !== releaseVersion) { + fail(`installer pin-table release ${releaseVersion} must match ${label} ${runtimeVersion}`); + } + } if (options.format === "json") { process.stdout.write(`${JSON.stringify(pins)}\n`); return; diff --git a/test/installer-hash-check.test.ts b/test/installer-hash-check.test.ts index 14eecb21458..1870a937e08 100644 --- a/test/installer-hash-check.test.ts +++ b/test/installer-hash-check.test.ts @@ -60,6 +60,8 @@ type FixtureMode = | "duplicate-brev-pin" | "failure" | "incomplete-trusted-allowlist" + | "installer-max-version-drift" + | "installer-pin-selector-drift" | "mismatched-table-versions" | "missing-brev-pin" | "multiple-installer-versions" @@ -72,6 +74,8 @@ type FixtureMode = | "partial-manifest-missing" | "pr-checker-bypass" | "pr-parser-bypass" + | "brev-stable-version-drift" + | "runtime-consumers-newer-than-tables" | "symlink-installer-input" | "symlink-scripts-parent"; type PinFormatting = @@ -99,8 +103,22 @@ const BREV_MUTATIONS: Partial string>> = .replace(ASSET_DIGESTS.get(ASSETS[1] ?? "") ?? "missing", OFFICIAL_UNEXPECTED_BREV_DIGEST), "pr-checker-bypass": corruptFirstBrevPin, "pr-parser-bypass": corruptFirstBrevPin, + "brev-stable-version-drift": (source) => + source.replace( + 'stable | auto) OPENSHELL_VERSION="v0.0.72" ;;', + 'stable | auto) OPENSHELL_VERSION="v0.0.82" ;;', + ), + "runtime-consumers-newer-than-tables": (source) => + source.replace( + 'stable | auto) OPENSHELL_VERSION="v0.0.72" ;;', + 'stable | auto) OPENSHELL_VERSION="v0.0.82" ;;', + ), }; const INSTALLER_MUTATIONS: Partial string>> = { + "installer-max-version-drift": (source) => + source.replace('MAX_VERSION="0.0.72"', 'MAX_VERSION="0.0.82"'), + "installer-pin-selector-drift": (source) => + source.replace('PIN_VERSION="$MAX_VERSION"', 'PIN_VERSION="0.0.72"'), "multiple-installer-versions": (source) => source.replace(`v0.0.72:${ASSETS[0]}`, `v0.0.73:${ASSETS[0]}`), "official-but-unexpected-installer-asset": (source) => @@ -112,13 +130,20 @@ const INSTALLER_MUTATIONS: Partial strin ), "partial-asset-missing": (source) => source.replace(ASSETS.at(-1) ?? "missing", UNPUBLISHED_ASSET), + "runtime-consumers-newer-than-tables": (source) => + source.replace('MAX_VERSION="0.0.72"', 'MAX_VERSION="0.0.82"'), }; type InputMutationContext = { + blueprint: string; brevInstaller: string; fixtureRoot: string; installer: string; }; const INPUT_MUTATIONS: Partial void>> = { + "runtime-consumers-newer-than-tables": ({ blueprint }) => { + const source = fs.readFileSync(blueprint, "utf8"); + fs.writeFileSync(blueprint, source.replace('"0.0.72"', '"0.0.82"')); + }, "non-regular-brev-input": ({ brevInstaller }) => { fs.rmSync(brevInstaller); fs.mkdirSync(brevInstaller); @@ -243,6 +268,7 @@ function createFixture( tempDirs.push(fixtureRoot); fs.mkdirSync(checksDir, { recursive: true }); fs.mkdirSync(binDir, { recursive: true }); + fs.mkdirSync(path.join(fixtureRoot, "nemoclaw-blueprint"), { recursive: true }); const checker = fs.readFileSync( path.join(REPO_ROOT, "scripts", "check-installer-hash.sh"), "utf8", @@ -253,18 +279,27 @@ function createFixture( path.join(checksDir, "extract-installer-pins.mts"), ); + fs.writeFileSync( + path.join(fixtureRoot, "nemoclaw-blueprint", "blueprint.yaml"), + `max_openshell_version: "${openshellVersion}"\n`, + ); fs.writeFileSync( path.join(scriptsDir, "install-openshell.sh"), - renderPinFunction("openshell_pinned_sha256", ASSETS, openshellVersion, formatting), + `MAX_VERSION="${openshellVersion}"\nPIN_VERSION="$MAX_VERSION"\n${renderPinFunction( + "openshell_pinned_sha256", + ASSETS, + openshellVersion, + formatting, + )}`, ); fs.writeFileSync( path.join(scriptsDir, "brev-launchable-ci-cpu.sh"), - renderPinFunction( + `case "$NEMOCLAW_REF" in\n stable | auto) OPENSHELL_VERSION="v${openshellVersion}" ;;\nesac\n${renderPinFunction( "openshell_cli_pinned_sha256", ASSETS.slice(0, 2), openshellVersion, formatting, - ), + )}`, ); fs.writeFileSync( path.join(binDir, "curl"), @@ -370,6 +405,7 @@ function runFixture( ); } const installer = path.join(fixtureRoot, "scripts", "install-openshell.sh"); + const blueprint = path.join(fixtureRoot, "nemoclaw-blueprint", "blueprint.yaml"); const installerSource = fs.readFileSync(installer, "utf8"); const mutateInstaller = INSTALLER_MUTATIONS[mode] ?? ((source: string) => source); fs.writeFileSync(installer, mutateInstaller(installerSource)); @@ -384,7 +420,7 @@ function runFixture( ? 'process.stdout.write("PR_PARSER_EXECUTED\\n");\n' : fs.readFileSync(targetParser, "utf8"), ); - INPUT_MUTATIONS[mode]?.({ brevInstaller, fixtureRoot, installer }); + INPUT_MUTATIONS[mode]?.({ blueprint, brevInstaller, fixtureRoot, installer }); return spawnSync("bash", [checker], { cwd: fixtureRoot, encoding: "utf8", @@ -446,6 +482,37 @@ describe("installer hash verification", () => { expect(result.stdout).not.toContain("All installer hashes are current"); }); + it("rejects newer runtime consumers when both trusted pin tables stay on an older release", () => { + const result = runFixture("runtime-consumers-newer-than-tables", undefined, true); + + expect(result.status).toBe(1); + expect(result.stdout).toContain("unable to extract the OpenShell installer pin tables"); + expect(result.stdout).toContain( + "installer pin-table release 0.0.72 must match blueprint max_openshell_version 0.0.82", + ); + expect(result.stdout).not.toContain("Checking OpenShell v0.0.72 release assets"); + expect(result.stdout).not.toContain("All installer hashes are current"); + }); + + it.each([ + [ + "installer-max-version-drift", + "installer pin-table release 0.0.72 must match installer MAX_VERSION 0.0.82", + ], + [ + "brev-stable-version-drift", + "installer pin-table release 0.0.72 must match Brev stable OpenShell default 0.0.82", + ], + ["installer-pin-selector-drift", 'installer PIN_VERSION must be exactly "$MAX_VERSION"'], + ] as const)("rejects %s", (mode, diagnostic) => { + const result = runFixture(mode, undefined, true); + + expect(result.status).toBe(1); + expect(result.stdout).toContain("unable to extract the OpenShell installer pin tables"); + expect(result.stdout).toContain(diagnostic); + expect(result.stdout).not.toContain("All installer hashes are current"); + }); + it.each([ [ "multiple-installer-versions", From 36dda9a605f4020b966d4b923929515a26612b8e Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 20:22:12 -0700 Subject: [PATCH 13/50] docs(security): bind OpenShell dev artifact evidence Signed-off-by: Aaron Erickson --- .../openshell-0.0.82-migration-review.md | 51 ++++++++++++++++--- .../openshell-0.0.82-migration-review.test.ts | 16 ++++++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/docs/security/openshell-0.0.82-migration-review.md b/docs/security/openshell-0.0.82-migration-review.md index 3608e80f082..054725b4cd3 100644 --- a/docs/security/openshell-0.0.82-migration-review.md +++ b/docs/security/openshell-0.0.82-migration-review.md @@ -66,10 +66,15 @@ Release publication is a separate gate from source ancestry: but it has no GitHub release. Release Tag run [29101552146](https://github.com/NVIDIA/OpenShell/actions/runs/29101552146) failed the Ubuntu 26.04 rootless-Podman E2E job and skipped publication. -- OpenShell `bb72d012` produced moving development build - `0.0.82.dev11+gbb72d0123` in successful Release Dev run - [29215426930](https://github.com/NVIDIA/OpenShell/actions/runs/29215426930). - A moving prerelease is useful for compatibility work, but is not a stable +- OpenShell `bb72d012` produced a successful Release Dev run + [29215426930](https://github.com/NVIDIA/OpenShell/actions/runs/29215426930), + but it does not expose one interchangeable version string. The released CLI, + gateway, and standalone sandbox binaries report `0.0.82-dev.11+gbb72d012`; + the pipeline Cargo version and supervisor image report + `0.0.82-dev.11+gbb72d0123`; and Python wheel filenames use + `0.0.82.dev11+gbb72d0123`. Development compatibility manifests must record + the observed CLI output separately from producer and component versions. A + moving prerelease is useful for compatibility work, but is not a stable selector or final provenance record. ## Artifact baseline and provenance gap @@ -103,6 +108,40 @@ duplicate members, links, and devices fail closed. This structural validation is independent of release SHA-256 verification and also constrains the explicitly unverified development-channel path. +### Candidate development artifact evidence + +The successful `bb72d012` Release Dev run provides bounded compatibility inputs, +not release approval. The exact retained Linux amd64 artifacts inspected on July +12, 2026 are: + +| Role | Actions artifact | Actions ZIP SHA-256 | Inner archive SHA-256 | Extracted binary SHA-256 | +|---|---|---|---|---| +| CLI | `8266446648` (`cli-linux-amd64`) | `78923b27a492204b6e869d9f5f392e57b37d8ddcb9367d746f4ee46cfaf0e5a2` | `d1732c0b87801560afd1b06cfea31c60d6a357100d5b817b4a4fb181b0b71933` | `09083ef8087e5191fc3513a7239b08041b511fdeb7f2fe074bdf8820886cbea1` | +| Gateway | `8266452366` (`gateway-binary-linux-amd64`) | `39504758f07a8bac0a52d958ec56e380ac59824bde8db72a815a9b82c6bbcfd6` | `5e3728564b1f965cb5d320bab4f37d388303723f42a64c308227dbc1ef382043` | `39e75f7a2a96c220e3f2d645067f0623d922385ade07edb2037a27cc07ea81d1` | +| Standalone sandbox | `8266435047` (`supervisor-binary-linux-amd64`) | `7b2e47adbbfc644806b465a4f4c3c7bfaba7117e1f19ec9f151b37695b418bf4` | `6f7040e89ec249df7f3b36ddff609a87f096fcdf62cd5c28e86757f175e40a7a` | `58e5d99261d2b8ea06664d020995830fd3f153ea692f36622b92f9b827ea60c8` | + +Each Actions ZIP contains exactly the expected archive, and each nested archive +contains exactly one regular mode-0755 root-owned binary with no links or extra +paths. The artifacts expire around July 18, 2026; their IDs are evidence only +while retained and must not become long-lived dependency selectors. + +The development supervisor image resolves to immutable index +`sha256:fc441051102b1a16ffcabf59878fa464d3c548f29bfbfa6e4acb232ab67198b7`: + +| Platform | Child manifest | Config | Binary layer | `/openshell-sandbox` SHA-256 | +|---|---|---|---|---| +| Linux amd64 | `sha256:4a54b434decd007d2a966edb5db751adb3ca4cf8ab8ac0b248901f8efe614b71` | `sha256:5432194fa43840c333bc7b166bf6e7c0e15247e9dc195cb9a38c1a85b7415f44` | `sha256:d1baeaebaddef6291e0a94b697f28c3c319ac2ec1a83843026e89553cc7cd27e` | `8e89067afca2d1c02a25fb19906dd27fd8d524ee4eb3b2b36b1210338dae9235` | +| Linux arm64 | `sha256:fab8d5c551991648a19bf7876d2edf19fdcf4e95139ce5f75d638354c0820d51` | `sha256:66a1d121d6386e19297d05a950ba7409c5752f337bacfbc156c7c76513e40136` | `sha256:818c727cb5cbcdb78918a274ca6b9aa85be6a95fdb604e49f523cf2c87f2eba4` | `8ec9b88c49f001d070ada7bb5a98fb6f96498fc446b0f2f614056247d7300b85` | + +Those image binaries byte-match build artifacts `8266448422` (amd64) and +`8266451406` (arm64), respectively. The match binds registry content to retained +Actions output, but not cryptographically to source: GitHub returned no +attestation for the index or either child manifest, and the OCI configs expose no +source, revision, or version labels. Development proof must therefore record +`attestationStatus: absent`, preserve every digest above, and avoid claiming +source-to-image provenance. The final stable release must be audited anew rather +than inheriting this development evidence. + ## Adjacent release findings ### v0.0.72 to v0.0.73 @@ -262,12 +301,12 @@ Commits: `5f38b7c4`, `ccdac9ce`, `caaa5165`, `8c0ecac8`, `233d207e`, |---|---|---|---|---| | `OS82-01` | Critical | All stable selectors, archives, checksums, binaries, and the supervisor image could identify different builds. | Pin one published tag; verify producer run, signatures/attestations, release hashes, extracted binaries, component versions, OCI index and child manifests; reject archive traversal, links, devices, duplicates, or unexpected members. | Blocked: no stable `v0.0.82` release. | | `OS82-02` | Critical | `mcp status` can be honest while the affected Spark still cannot initialize resolver/CA state or perform a credential-bearing request. | Physical Docker 27 DGX Spark: register credential, require status success, load tools, complete a real MCP tool call, and prove the literal placeholder never reaches upstream. | Blocked on assigned hardware proof. | -| `OS82-03` | High | `src/lib/actions/sandbox/exec.ts`, command dispatch, docs, and internal wrappers encode the old newline rejection. | Remove the obsolete public rejection; prove byte-exact LF, CRLF, quotes, and heredoc argv; retain NUL plus multiline workdir/environment rejection. | Migration in progress. | +| `OS82-03` | High | `src/lib/actions/sandbox/exec.ts`, command dispatch, docs, and internal wrappers encode the old newline rejection. | Remove the obsolete public rejection; prove byte-exact LF, CRLF, quotes, and heredoc argv; retain NUL plus multiline workdir/environment rejection. | Source migration and focused tests complete; candidate runtime proof open. | | `OS82-04` | High | OpenShell child launch now clears the complete capability bounding set. Hosts without `CAP_SETPCAP` may fail if their runtime does not pre-clear it. | Prove entrypoint, exec, and connect launch with `CapBnd=0` on Linux Docker, DGX Spark arm64, macOS Docker Desktop/Colima, WSL, and Colossus; update NemoClaw's #3280 caveat only from runtime evidence. | Open runtime gate. | | `OS82-05` | High | Versioned credential placeholders and the eight-generation window change long-running MCP behavior. | Regenerate the exact-version child-visible manifest; reject reserved `v_` names; test more than eight rotations, removed keys, detach, restart/rebuild, fresh exec revision, expiry, and literal-placeholder scans. | Open migration and runtime gate. | | `OS82-06` | High | Initial policy acknowledgement and ordered retry can make the active gateway status lag enforcement. | Test initial LOADED/FAILED, hot update, retry outage/recovery, restart, exact version/hash re-read, and ordered drain. | Open runtime gate. | | `OS82-07` | High | Sequential nft setup can leave an incomplete policy-accept ruleset after a required command fails; Docker setup treats the error as nonfatal. | Inject each required failure; inspect IPv4/IPv6 TCP/UDP rules and direct-bypass negatives on Linux x86 and Spark arm64; verify restart and teardown. | Open security gate. | -| `OS82-08` | High | The supervisor image gains Alpine and three networking packages and changes binary mode. | Review SBOM, vulnerabilities, licenses, executables, modes, multiarch manifests, source labels, and OCI provenance; preserve an explicit digest. | Open supply-chain gate. | +| `OS82-08` | High | The supervisor image gains Alpine and three networking packages and changes binary mode. | Review SBOM, vulnerabilities, licenses, executables, modes, multiarch manifests, source labels, and OCI provenance; preserve an explicit digest. | Development image content audited; missing attestation and final stable image remain open. | | `OS82-09` | Medium-high | Normalized selected-driver config can change the effective Docker gateway even when the TOML text is unchanged. | Parse the final rendered TOML with the final binary; prove loopback/bridge listeners, JWT/mTLS, restart, persisted state, and legacy gateway upgrade. | Open runtime gate. | | `OS82-10` | Medium-high | Supervisor TLS identity variables are no longer child environment. Stale tests/comments can normalize a credential leak. | Assert absence from entrypoint, exec, and connect children and update the source-of-truth rationale. | Open migration gate. | | `OS82-11` | Medium-high | Live `/proc//exe` identity changes replacement-time policy behavior. | Prove old process survives replacement and a new altered process at the same path is denied. | Open runtime gate. | diff --git a/test/openshell-0.0.82-migration-review.test.ts b/test/openshell-0.0.82-migration-review.test.ts index a0d91d60244..be37b4654a1 100644 --- a/test/openshell-0.0.82-migration-review.test.ts +++ b/test/openshell-0.0.82-migration-review.test.ts @@ -96,6 +96,22 @@ describe("OpenShell 0.0.82 migration review", () => { expect(review).toContain("reject archive traversal, links, devices, duplicates"); }); + it("records exact development identities without treating them as a stable release", () => { + expect(review).toContain("0.0.82-dev.11+gbb72d012"); + expect(review).toContain("0.0.82-dev.11+gbb72d0123"); + expect(review).toContain("0.0.82.dev11+gbb72d0123"); + expect(review).toContain("8266446648"); + expect(review).toContain("8266452366"); + expect(review).toContain("8266435047"); + expect(review).toContain( + "sha256:fc441051102b1a16ffcabf59878fa464d3c548f29bfbfa6e4acb232ab67198b7", + ); + expect(review).toContain("8266448422"); + expect(review).toContain("8266451406"); + expect(review).toContain("attestationStatus: absent"); + expect(review).toContain("final stable release must be audited anew"); + }); + it("tracks every material migration concern and refuses false-green evidence", () => { for (let number = 1; number <= 14; number += 1) { const id = `OS82-${String(number).padStart(2, "0")}`; From 5e5fe4bba9436f67a0332c2ecec509f5b8f22227 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 20:41:24 -0700 Subject: [PATCH 14/50] test(exec): cover carriage-return command arguments Signed-off-by: Aaron Erickson --- src/lib/actions/sandbox/exec.multiline-argv.test.ts | 4 ++++ src/lib/actions/sandbox/runtime-env.test.ts | 1 + 2 files changed, 5 insertions(+) diff --git a/src/lib/actions/sandbox/exec.multiline-argv.test.ts b/src/lib/actions/sandbox/exec.multiline-argv.test.ts index 8e020a0e95b..c90772500c8 100644 --- a/src/lib/actions/sandbox/exec.multiline-argv.test.ts +++ b/src/lib/actions/sandbox/exec.multiline-argv.test.ts @@ -36,6 +36,10 @@ describe("execSandbox multi-line argv", () => { label: "LF", command: ["python3", "-c", "print('one')\nprint('two')"], }, + { + label: "CR", + command: ["python3", "-c", "one\rtwo"], + }, { label: "CRLF", command: ["python3", "-c", "print('one')\r\nprint('two')"], diff --git a/src/lib/actions/sandbox/runtime-env.test.ts b/src/lib/actions/sandbox/runtime-env.test.ts index 38ab1bb7640..6549c0fdece 100644 --- a/src/lib/actions/sandbox/runtime-env.test.ts +++ b/src/lib/actions/sandbox/runtime-env.test.ts @@ -30,6 +30,7 @@ describe("wrapExecCommandWithRuntimeEnv", () => { it("executes LF, CRLF, quote, and heredoc argv byte-exactly", () => { const payloads = [ "line one\nline two", + "line one\rline two", "line one\r\nline two", `single ' and double " quotes`, "cat <<'EOF'\nline one\nline 'two'\nEOF", From 441e0c8286092ac456ea90070cec0c2e049b901f Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 20:45:28 -0700 Subject: [PATCH 15/50] test(install): model archive validation in curl fallback Signed-off-by: Aaron Erickson --- test/runner.test.ts | 68 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 58 insertions(+), 10 deletions(-) diff --git a/test/runner.test.ts b/test/runner.test.ts index 992ef6f22cb..b711bd01a37 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -741,11 +741,35 @@ describe("regression guards", () => { strings() { echo "request-body-credential-rewrite websocket-credential-rewrite allow_all_known_mcp_methods"; } export -f strings tar() { - local destination="\${@: -1}" - printf '%s\n' '#!/bin/sh' 'echo "openshell 0.0.72"' > "$destination/openshell" - printf '%s\n' '#!/bin/sh' 'echo "openshell-gateway 0.0.72"' > "$destination/openshell-gateway" - printf '%s\n' '#!/bin/sh' 'echo "openshell-sandbox 0.0.72"' > "$destination/openshell-sandbox" - chmod +x "$destination/openshell" "$destination/openshell-gateway" "$destination/openshell-sandbox" + local mode="\${1:-}" archive="\${2:-}" expected="" destination="" + case "$(basename "$archive")" in + openshell-gateway-*) expected="openshell-gateway" ;; + openshell-sandbox-*) expected="openshell-sandbox" ;; + openshell-*) expected="openshell" ;; + *) return 2 ;; + esac + case "$mode" in + -tzf) + printf '%s\n' "$expected" + ;; + -tvzf) + printf '%s\n' "-rwxr-xr-x 0/0 0 2026-01-01 00:00 $expected" + ;; + xzf|-xzf) + shift 2 + while [ "$#" -gt 0 ]; do + if [ "$1" = "-C" ]; then + shift + destination="$1" + fi + shift || true + done + [ -n "$destination" ] || return 2 + printf '%s\n' '#!/bin/sh' 'echo "0.0.72"' > "$destination/$expected" + chmod +x "$destination/$expected" + ;; + *) return 2 ;; + esac }; export -f tar install() { /usr/bin/install "$@"; }; export -f install source "${scriptPath}" @@ -819,11 +843,35 @@ describe("regression guards", () => { strings() { echo "request-body-credential-rewrite websocket-credential-rewrite allow_all_known_mcp_methods"; } export -f strings tar() { - local destination="\${@: -1}" - printf '%s\n' '#!/bin/sh' 'echo "openshell 0.0.72"' > "$destination/openshell" - printf '%s\n' '#!/bin/sh' 'echo "openshell-gateway 0.0.72"' > "$destination/openshell-gateway" - printf '%s\n' '#!/bin/sh' 'echo "openshell-sandbox 0.0.72"' > "$destination/openshell-sandbox" - chmod +x "$destination/openshell" "$destination/openshell-gateway" "$destination/openshell-sandbox" + local mode="\${1:-}" archive="\${2:-}" expected="" destination="" + case "$(basename "$archive")" in + openshell-gateway-*) expected="openshell-gateway" ;; + openshell-sandbox-*) expected="openshell-sandbox" ;; + openshell-*) expected="openshell" ;; + *) return 2 ;; + esac + case "$mode" in + -tzf) + printf '%s\n' "$expected" + ;; + -tvzf) + printf '%s\n' "-rwxr-xr-x 0/0 0 2026-01-01 00:00 $expected" + ;; + xzf|-xzf) + shift 2 + while [ "$#" -gt 0 ]; do + if [ "$1" = "-C" ]; then + shift + destination="$1" + fi + shift || true + done + [ -n "$destination" ] || return 2 + printf '%s\n' '#!/bin/sh' 'echo "0.0.72"' > "$destination/$expected" + chmod +x "$destination/$expected" + ;; + *) return 2 ;; + esac }; export -f tar install() { /usr/bin/install "$@"; }; export -f install source "${scriptPath}" From 365855583e4f933b56ada2b88f14724dc024c257 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 20:48:19 -0700 Subject: [PATCH 16/50] docs(security): require trusted upgrade bootstrap Signed-off-by: Aaron Erickson --- .../openshell-0.0.82-migration-review.md | 16 ++++++++++------ test/openshell-0.0.82-migration-review.test.ts | 6 +++++- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/docs/security/openshell-0.0.82-migration-review.md b/docs/security/openshell-0.0.82-migration-review.md index 054725b4cd3..322b1b84cf7 100644 --- a/docs/security/openshell-0.0.82-migration-review.md +++ b/docs/security/openshell-0.0.82-migration-review.md @@ -313,6 +313,7 @@ Commits: `5f38b7c4`, `ccdac9ce`, `caaa5165`, `8c0ecac8`, `233d207e`, | `OS82-12` | Medium | OpenShell declares Docker 28.0+ while #6379 is on Docker 27 and NemoClaw marks DGX Spark tested. | Either validate and document a precise downstream exception from physical proof or raise the supported floor and preflight it. | Open product/platform decision. | | `OS82-13` | Low | Mount parsing/SELinux changes could affect the test-only tmpfs path. | Rerun the EXDEV tmpfs fixture and retain production no-mount evidence. | Open targeted test. | | `OS82-14` | Low | Sanitized MCP tool names are newly present in logs. | Record the additive observability/privacy behavior; ensure no downstream parser assumes the old shape. | Source-reviewed; targeted log check pending. | +| `OS82-15` | High | The installer-hash workflow executes its checker and parser from the PR base SHA. One PR cannot safely teach that trusted base about a new release and consume the release; using the head checker would let reviewed code define its own trust rules. | First land archive safety, normalized full-script template validation, and multi-release trust while selectors remain `0.0.72`; prove the old base rejects a new release and the new base permits only structured release-data changes; then submit the `0.0.82` pin. | NemoClaw-only prerequisite implementation in progress. | An unresolved critical or high concern blocks the version selector change. A green aggregate test suite does not override an open ledger row. @@ -334,19 +335,22 @@ restart/rebuild, and cleanup without a conditional skip or expected failure. ## Final acceptance gates -1. A stable OpenShell tag contains `bb72d012` or a reviewed descendant. Re-run +1. The trusted installer/hash prerequisite lands on NemoClaw `main` while all + runtime selectors remain `0.0.72`. Its base-owned parser rejects operational + installer drift and permits only validated release-data and selector changes. +2. A stable OpenShell tag contains `bb72d012` or a reviewed descendant. Re-run this entire adjacent-source audit for every commit between `bb72d012` and that tag. -2. The tag has a successful release publication. Every consumed archive and OCI +3. The tag has a successful release publication. Every consumed archive and OCI child manifest is bound to that producer run and source identity. -3. Blueprint bounds, installer tables, Brev defaults, workflow pins, feature-gate +4. Blueprint bounds, installer tables, Brev defaults, workflow pins, feature-gate hashes, supervisor digest, credential manifest, tests, and active docs select one coherent version. -4. Every concern-specific unit/integration proof above passes, followed by normal +5. Every concern-specific unit/integration proof above passes, followed by normal repository checks and exact-head CI/advisor review. -5. The non-skipped live matrix passes on Linux x86 Docker, macOS Docker +6. The non-skipped live matrix passes on Linux x86 Docker, macOS Docker Desktop/Colima, WSL, Colossus, and physical DGX Spark arm64. Legacy gateway upgrade, restart, rollback, and teardown remain explicit phases. -6. The physical #6379 Spark run completes an authenticated real MCP tool call and +7. The physical #6379 Spark run completes an authenticated real MCP tool call and reports any failure honestly. Inclusion of `40194f93` alone cannot close the issue. diff --git a/test/openshell-0.0.82-migration-review.test.ts b/test/openshell-0.0.82-migration-review.test.ts index be37b4654a1..007b750f87a 100644 --- a/test/openshell-0.0.82-migration-review.test.ts +++ b/test/openshell-0.0.82-migration-review.test.ts @@ -113,13 +113,17 @@ describe("OpenShell 0.0.82 migration review", () => { }); it("tracks every material migration concern and refuses false-green evidence", () => { - for (let number = 1; number <= 14; number += 1) { + for (let number = 1; number <= 15; number += 1) { const id = `OS82-${String(number).padStart(2, "0")}`; expect(review.split(`| \`${id}\` |`), `${id} concern row`).toHaveLength(2); } expect(review).toContain("An unresolved critical or high concern blocks"); expect(review).toContain("full managed MCP lifecycle"); expect(review).toContain("without a conditional skip or expected failure"); + expect(review).toContain("executes its checker and parser from the PR base SHA"); + expect(review).toContain( + "using the head checker would let reviewed code define its own trust rules", + ); }); it("keeps the stable pin and physical Spark proof blocked until final evidence exists", () => { From 14b7560ca5f28ec02cc7a625e4c9559346593fa8 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 12 Jul 2026 21:02:44 -0700 Subject: [PATCH 17/50] refactor(exec): remove obsolete newline transports Signed-off-by: Aaron Erickson --- .../openshell-0.0.82-migration-review.md | 2 +- .../sandbox/auto-pair-approval.test.ts | 19 ---------- src/lib/actions/sandbox/auto-pair-approval.ts | 23 +----------- .../actions/sandbox/auto-pair-warmup.test.ts | 29 ++++----------- src/lib/actions/sandbox/auto-pair-warmup.ts | 12 +----- src/lib/actions/sandbox/doctor.ts | 5 +-- .../sandbox/mcp-bridge-provider-readiness.ts | 15 ++------ .../sandbox/mcp-bridge-provider.test.ts | 28 ++++++-------- src/lib/actions/sandbox/runtime-env.test.ts | 2 +- .../sandbox/sessions/gateway-rpc-call.test.ts | 16 ++++---- .../actions/sandbox/sessions/gateway-rpc.ts | 4 +- .../06-deepagents-code-python-egress.sh | 8 ++-- .../07-deepagents-code-headless-inference.sh | 8 ++-- .../08-deepagents-code-secret-boundary.sh | 6 +-- .../09-deepagents-code-tavily-opt-in.sh | 4 +- test/e2e/live/skill-agent.test.ts | 4 +- ...platform-parity-cloud-experimental.test.ts | 18 ++++----- test/internal-commands-docs.test.ts | 13 +++---- test/langchain-deepagents-code-image.test.ts | 4 +- test/process-recovery-primitives.test.ts | 5 +-- .../auto-pair-approval.test.ts | 10 ++--- test/sandbox-connect-inference/helpers.ts | 37 +++---------------- 22 files changed, 82 insertions(+), 190 deletions(-) diff --git a/docs/security/openshell-0.0.82-migration-review.md b/docs/security/openshell-0.0.82-migration-review.md index 322b1b84cf7..a787af28cf1 100644 --- a/docs/security/openshell-0.0.82-migration-review.md +++ b/docs/security/openshell-0.0.82-migration-review.md @@ -301,7 +301,7 @@ Commits: `5f38b7c4`, `ccdac9ce`, `caaa5165`, `8c0ecac8`, `233d207e`, |---|---|---|---|---| | `OS82-01` | Critical | All stable selectors, archives, checksums, binaries, and the supervisor image could identify different builds. | Pin one published tag; verify producer run, signatures/attestations, release hashes, extracted binaries, component versions, OCI index and child manifests; reject archive traversal, links, devices, duplicates, or unexpected members. | Blocked: no stable `v0.0.82` release. | | `OS82-02` | Critical | `mcp status` can be honest while the affected Spark still cannot initialize resolver/CA state or perform a credential-bearing request. | Physical Docker 27 DGX Spark: register credential, require status success, load tools, complete a real MCP tool call, and prove the literal placeholder never reaches upstream. | Blocked on assigned hardware proof. | -| `OS82-03` | High | `src/lib/actions/sandbox/exec.ts`, command dispatch, docs, and internal wrappers encode the old newline rejection. | Remove the obsolete public rejection; prove byte-exact LF, CRLF, quotes, and heredoc argv; retain NUL plus multiline workdir/environment rejection. | Source migration and focused tests complete; candidate runtime proof open. | +| `OS82-03` | High | `src/lib/actions/sandbox/exec.ts`, command dispatch, docs, and internal wrappers encode the old newline rejection. | Remove the obsolete public rejection and newline-only wrappers; prove byte-exact LF, CR, CRLF, quotes, and heredoc argv; retain NUL plus multiline workdir/environment rejection. | Source and internal-wrapper migration complete; candidate runtime proof open. | | `OS82-04` | High | OpenShell child launch now clears the complete capability bounding set. Hosts without `CAP_SETPCAP` may fail if their runtime does not pre-clear it. | Prove entrypoint, exec, and connect launch with `CapBnd=0` on Linux Docker, DGX Spark arm64, macOS Docker Desktop/Colima, WSL, and Colossus; update NemoClaw's #3280 caveat only from runtime evidence. | Open runtime gate. | | `OS82-05` | High | Versioned credential placeholders and the eight-generation window change long-running MCP behavior. | Regenerate the exact-version child-visible manifest; reject reserved `v_` names; test more than eight rotations, removed keys, detach, restart/rebuild, fresh exec revision, expiry, and literal-placeholder scans. | Open migration and runtime gate. | | `OS82-06` | High | Initial policy acknowledgement and ordered retry can make the active gateway status lag enforcement. | Test initial LOADED/FAILED, hot update, retry outage/recovery, restart, exact version/hash re-read, and ordered drain. | Open runtime gate. | diff --git a/src/lib/actions/sandbox/auto-pair-approval.test.ts b/src/lib/actions/sandbox/auto-pair-approval.test.ts index a918ae3b590..2693cf9d434 100644 --- a/src/lib/actions/sandbox/auto-pair-approval.test.ts +++ b/src/lib/actions/sandbox/auto-pair-approval.test.ts @@ -11,7 +11,6 @@ import { AUTO_PAIR_MAX_APPROVALS, buildAutoPairApprovalScript, readAutoPairApprovalPolicyModule, - wrapSandboxShellScript, } from "./auto-pair-approval"; const SUMMARY_MARKER = "__NEMOCLAW_AUTO_PAIR_APPROVED__"; @@ -51,24 +50,6 @@ describe("buildAutoPairApprovalScript (#4263/#4616)", () => { }); }); -describe("wrapSandboxShellScript (#4616)", () => { - it("encodes a multi-line payload onto a single newline-free line", () => { - const wrapped = wrapSandboxShellScript("echo one\necho two\n"); - expect(wrapped).not.toMatch(/[\n\r]/); - expect(wrapped).toContain("base64 -d"); - expect(wrapped).toContain("mktemp"); - }); - - it("round-trips and preserves the inner exit status when run", () => { - const inner = "echo line-one\nprintf 'exit-then\\n'\nexit 3\n"; - const wrapped = wrapSandboxShellScript(inner); - const result = spawnSync("sh", ["-c", wrapped], { encoding: "utf-8", timeout: 10_000 }); - expect(result.stdout).toContain("line-one"); - expect(result.stdout).toContain("exit-then"); - expect(result.status).toBe(3); - }); -}); - describe("auto-pair approval pass behaviour (#4616)", () => { it("approves allowlisted upgrades, skips unknown clients, and reports the count", () => { if (spawnSync("sh", ["-c", "command -v python3"], { stdio: "ignore" }).status !== 0) { diff --git a/src/lib/actions/sandbox/auto-pair-approval.ts b/src/lib/actions/sandbox/auto-pair-approval.ts index f86c9073364..7c011b64b3a 100644 --- a/src/lib/actions/sandbox/auto-pair-approval.ts +++ b/src/lib/actions/sandbox/auto-pair-approval.ts @@ -82,27 +82,6 @@ export type AutoPairApprovalResult = { approved: number; }; -/** - * Wrap a multi-line shell payload so it survives `openshell sandbox exec`. - * - * OpenShell's exec RPC rejects any argument containing a newline or carriage - * return ("command argument N contains newline or carriage return characters"), - * so a multi-line `sh -c