Repository navigation
fix(security): repair runtimes for 64 GB Spark qualification - #12780
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. ℹ️ Recent review info
Walkthrough
Merge Risk:
|
|
🌿 Preview your docs: https://nvidia-preview-pr-12780.docs.buildwithfern.com/nemoclaw |
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Code coverage overviewLanguages: TypeScript TypeScript / code-coverage/plugin
View coverage of the 10 most impacted files.
TypeScript / code-coverage/cli
View coverage of the 10 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Merge b60c91f while retaining atomic Teams replacement, Node 24.22.1 admission, and the 64 GB profile. Verify and preserve only the approved native Discord UUID launcher link. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Decision: Accept promotion of the existing 64 GB DGX Spark Qwen profile to
This is the maintainer product-acceptance record for the existing profile's status promotion. It does not authorize merging PR #12780. |
|
PR Review Advisor finished for commit Request review only when Require no Advisor blockers is green. |
## Outcome E2E jobs that restore the exact-commit CLI artifact now run a restore helper that accepts Node 24.22.1. Before this change, 72 jobs in [E2E run 40255364836](https://github.com/NVIDIA/NemoClaw/actions/runs/40255364836) on `3c6d991640` downloaded the artifact successfully and then failed with `consumer must restore the CLI under the pinned Node 24.18.1 toolchain`. ## Reason #12780 moved E2E setup to Node 24.22.1 and updated `scripts/e2e/restore-cli-artifact.sh`. The workflows still call `restore-e2e-cli-artifact@b1494a0`, and that action runs its own helper copy through `GITHUB_ACTION_PATH`. That copy still requires Node 24.18.1. ### Related issues Refs #12780 ## Changes - Repin all 13 restore steps (12 in `.github/workflows/e2e.yaml`, 1 in `.github/workflows/e2e-standard-profile.yaml`) and `E2E_ACTION_PROVENANCE.restoreCliArtifact` in `tools/e2e/workflow-boundary-policy.mts` to `restore-e2e-cli-artifact@3c6d991`. Between `b1494a08` and `3c6d991640`, the action file is unchanged. The restore helper changes only its consumer-runtime and manifest-toolchain checks, both from 24.18.1 to 24.22.1. - Recompute `MCP_DEV_POST_INSTALL_TRANSITION_CONTENT_SHA256` in `tools/e2e/mcp-dev-workflow-boundary-digests.mts` because it hashes the `mcp-bridge-dev` restore step. ## Verification - `npx vitest run --project e2e-support` on the 30 restore, CLI-artifact, and workflow-boundary test files: 903 passed. - `npx vitest run test/mcp/mcp-openshell-workflow.test.ts`: passed. - Offline replay on artifact `12622272733` from the failing run: the `b1494a08` helper reproduces the CI error. The `3c6d991640` helper passes every provenance, digest, archive, and identity check and restores `dist/`. - The pre-commit and pre-push hooks passed. - Live E2E: pending. A manual PR E2E run takes `e2e.yaml` from `main`, which still has the old pin, so live evidence requires a run of this branch's workflow or a post-merge `main` run. - The diff contains no secrets, API keys, or credentials. ## Review notes This PR changes sensitive paths `.github/**` and `tools/e2e/**`. No pre-publication review exists; these paths await independent review. The Hermes base-image failure and the bootstrap sudo-environment failure from the same run are outside this change. --- Signed-off-by: San Dang <sdang@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Tests** * Updated the automated end-to-end test workflows to use a revised CLI artifact restoration step. * **Chores** * Updated workflow boundary validation data to reflect the current configuration. These changes affect automated testing and validation; no user-facing product behavior is noted. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: San Dang <sdang@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Outcome Update the reviewed Hermes base pin to the published image containing Node 24.22.1. Protected image builds currently select a base containing Node 24.18.1 and stop at the completed-image security check. ## Reason [PR #12780](#12780) raised the Node requirement without updating this base selector. Both protected architecture jobs in [E2E run 40268185522](https://github.com/NVIDIA/NemoClaw/actions/runs/40268185522) fail at that check. The compatible base was already published by [run 40255364854](https://github.com/NVIDIA/NemoClaw/actions/runs/40255364854). ## Changes Replace only `agents/hermes/Dockerfile`'s base index with `sha256:4574777698fa65e270f53ee4c6fca90640cc89867e7dcc726c2ca03429d4ed8b`. Keep Hermes 0.21.3, security requirements and test assertions unchanged. ## Verification Local validation used `639670dd3a73a0686a682426058d840d962d900b` plus this one-line patch on macOS arm64, Node 24.14.1. ```bash node node_modules/vitest/vitest.mjs run --project cli --project integration \ src/lib/agent/base-image-hermes.test.ts \ src/lib/agent/base-image-hermes-resolution.test.ts \ test/runtime/sandbox/sandbox-base-security-packages.test.ts \ --maxWorkers=1 --fileParallelism=false --coverage=false --reporter=verbose ``` All 160 tests passed across three files. The command ran with an isolated environment and live E2E disabled. Existing tests cover stale Node rejection, security inventory and pinned-base provenance; no new test is needed for the selector change. The replacement index and both native manifests match the checksum-verified publication contract from `3c6d991640414a2d78f95f629d67194d44829554`. Producer builds and the ACP/direct startup-hold checks passed on both architectures. That historical evidence does not establish a model turn or a new protected E2E pass for this repair. `git diff --check` passed. The diff contains no secrets, API keys or credentials. ## Review notes Source review covered NVIDIA/NemoClaw at `639670dd3a73` plus the one-line change to the sensitive path `agents/hermes/Dockerfile`, including both complete failed-job logs, image identities and downstream gates. Independent agent review found no actionable finding; maintainer approval remains pending. Keep this PR draft until its commit passes required CI and review. For Hermes image qualification, verify the recorded base references against the native manifests of the new pin; a source-built base or an unchecked `latest` alias alone does not verify this selector. --- Signed-off-by: Hung Le <hple@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Chores** * Updated the underlying sandbox image. No user-facing behavior changes are expected. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Hung Le <hple@nvidia.com>
Outcome
Promote the QA-verified 64 GB DGX Spark Qwen profile to supported and repair the admission, image-build and security-audit failures blocking its CI.
Reason
The original branch predates the admission helper and current dependency repairs. This update incorporates main
b60c91f7b762dbfc4c86385172783bc45efa0119, retains the profile and serving settings, and patches the managed Node runtime for the October security release.Changes
0755; private transaction directories stay private, and file modes, ownership and package bytes remain unchanged.7be38b490c3f83e1062ec0b7a110b41bc178a67471b786a39e6f0524caffe03a. Retain existing OpenShell artifact identities and the 0.1.3 registry record. Update the immutable E2E preparation action pin and content checksums for the same approved Node version.Verification
Publication candidate:
c9b2e3ba3c02fff51e8cd89afc099a155c17e16f.9e8d4a90c9265a7304f86ae49dfb850d8a772fa0; all 38 Pi image inputs match this publication.All triggered PR CI workflows and the refreshed Advisor blocker gate pass on this commit.
Review notes
Security-sensitive paths include managed/base images, CI runtime pins, archive remediation, audit inputs and base-image admission. Local self-review covered credential isolation, source/replacement integrity, native project containment, failure recovery and the approved launcher exception. The human-authorized commit-bound test-admission record is 6389075902; it does not authorize merging.
All nine Advisor specialists completed successfully on this commit. Every refreshed machine-readable ledger is clear, with zero P0/P1 findings and zero unresolved additional E2E recommendations. All nine artifact digests, source/base identities and complete specialist contexts were verified. The blocker gate and review-coordinator shadow evaluation both pass.
The maintainer product-acceptance record for the supported 64 GB profile is 6405822312, published as
ericksoawith MEMBER association. Its body matches the human-approved statement and remains unedited. It establishes the reason, placement, owner, lifecycle, compatibility, security and validation contract. The customer specialist's prose repeats the earlier missing-authority claim; this verified public decision resolves that claim. No profile behavior or policy gate was weakened.Current main
bbdd0bcf8913a537a1126a40a144bac4def5986fadds the Hermes credential-restore repair. It adds no newer vulnerability repair and has no merge conflicts with this branch; the only overlapping path, the E2E README, merges automatically. No security patch replacement or branch update was needed.CodeRabbit skipped its incremental review and preserved checkpoint
07f358da472edde5ec4d3cec81ec54e7d1c7e7ed. That historical documentation review does not cover the runtime repair. No merge has been performed.Accepted status decision
PR #12633 records physical 64 GB Spark startup, automatic onboarding, chat, file/exec tools, a bounded long-context request, and a restart on the same model and recipe settings. It identifies the tested source and hardware. Aaron's later QA confirmation is a human-provided status decision; this PR does not claim a new hardware run or an independently generated qualification receipt.
Signed-off-by: Aaron Erickson aerickson@nvidia.com
Signed-off-by: Miyoung Choi miyoungc@nvidia.com