fix(inference): reject wrong-platform NEMOCLAW_VLLM_MODEL overrides before downloads#7359
Conversation
…efore downloads The managed vLLM install's platform gate only fired for models carrying a runtime override, so platform-restricted models without one passed the gate on hardware they cannot serve: qwen3.6-35b-a3b-nvfp4 (Spark-only, NVFP4 sm_121a) and deepseek-v4-flash (Station-only, 352 GB) proceeded through the image pull and full model download before failing at vllm serve load time. The env override bypasses the picker's modelsForPlatform filter, so this gate is the only platform enforcement on that path. Drop the model.runtime qualifier so every wrong-platform override fails before side effects, and update the platforms field doc to match. Closes NVIDIA#7358 Signed-off-by: Andrew White <andrewh@cdw.com>
📝 WalkthroughWalkthroughvLLM installation now validates platform compatibility for all selected models, including ChangesvLLM platform compatibility
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
|
✨ Thanks for the fix. This closes the platform gate gap for wrong-platform env overrides. Maintainers will review the vLLM platform enforcement changes. Related open issues: |
Approval was submitted while attempting to approve the protected E2E-skip environment. PR approval was premature and referenced a stale SHA/base pair; exact-head required CI and E2E gates remain pending.
prekshivyas
left a comment
There was a problem hiding this comment.
Sensitive-path security review — PASS
Exact pair reviewed: head b835f60490e0a8fd3b7b6863099a0bb37502c3bf against base f0848373b6866c7a4071491a6d054767a7a5968b.
Verdict: No security findings. This is safe from a code-security perspective. It strengthens the existing managed-vLLM platform boundary by applying the registry allowlist to every selected model before callbacks, prerequisite probes, Docker inspection, image pulls, or model downloads. Product scope is established by #7358 and the linked maintainer acknowledgment that #7359 addresses the reported existing-surface defect. This security PASS does not waive CI or required exact-head E2E.
Findings: none.
- Secrets and credentials — PASS: no secret-handling code or credential material changed.
- Input validation and sanitization — PASS: the user-controlled model override must resolve through the existing registry and now must match
profile.platformbefore side effects. - Authentication and authorization — PASS: no auth boundary changes; the new check restricts rather than expands behavior.
- Dependencies and third-party libraries — PASS: no dependency changes.
- Error handling and logging — PASS: rejection reports only the model label and profile name and exposes no sensitive data.
- Cryptography and data protection — PASS: no cryptographic or stored-data behavior changes.
- Configuration and security headers — PASS: no container flags, ports, image references, network policy, or HTTP headers change.
- Security testing — PASS: regressions cover Spark-only and Station-only overrides on generic Linux before callbacks/Docker work, plus the valid Spark path.
- System security — PASS: the check is fail-closed, uses the registry's explicit platform allowlist, runs before all relevant side effects, and introduces no TOCTOU or privilege expansion.
Files reviewed in full: src/lib/inference/vllm-models.ts, src/lib/inference/vllm.ts, and src/lib/inference/vllm.test.ts.
|
@prekshivyas do you prefer I use the update branch button when the branch gets behind main or leave it alone and let a maintainer do it? |
prekshivyas
left a comment
There was a problem hiding this comment.
Sensitive-path maintainer review refreshed for exact head 95498cf89caecee6b88cd3c9d80c3e5c93b75c59 on base 0670d20352298dd4686d6efe6abc5118ab8f27d2: PASS.
The three product blobs reviewed previously (src/lib/inference/vllm-models.ts, src/lib/inference/vllm.ts, and src/lib/inference/vllm.test.ts) are byte-identical to reviewed head b835f60490e0a8fd3b7b6863099a0bb37502c3bf; the new commit is a GitHub-Verified merge-only refresh from main. The prior nine-category security review therefore remains exact-content applicable: the platform gate still fails before callbacks, Docker probing, or other side effects on unsupported hosts; tests cover both unsupported platform paths and the supported path; no credentials, policy broadening, dependency, logging, or cryptographic changes are introduced. git diff --check 0670d2035...95498cf89 passes.
Approval still requires fresh exact-head CI/advisors and non-skipped exact-head E2E evidence.
@andrewwhitecdw generally if there are no conflicts it should not matter, but if we change ci and e2e tests on main it can affect the PR, so if the contributor frequently can, that is well and good or we take care of it! |
|
Exact-head gate status for |
prekshivyas
left a comment
There was a problem hiding this comment.
Approved at exact head 95498cf. Security review passed with no blockers; focused platform-allowlist validation fails before side effects, targeted tests pass, and all CI checks are satisfied. Credentialed E2E was skipped through the audited fork-PR gate as required by repository policy.
<!-- markdownlint-disable MD041 --> ## Summary This PR adds the canonical dated release entry for NemoClaw v0.0.94 before the tag is cut. The entry reconciles all 26 commits since v0.0.93 and links each user-visible change to its owning documentation. ## Changes - Add `docs/changelog/2026-07-24.mdx` with the exact `## v0.0.94` heading, parser-safe SPDX comment, release summary, and detailed bullets. - Record sandbox restore and update behavior, onboarding and inference changes, network policy behavior, security evidence, Hermes build performance, DGX Station guidance, and E2E validation changes. - Preserve `docs/` as the source of truth without changing the AI-agent documentation routing skill. - Use [E2E run 30075443016](https://github.com/NVIDIA/NemoClaw/actions/runs/30075443016) for release QA evidence at exact tested SHA `04e6dfa883071dda9df429c66e73168e1a995cba`. ### Source summary - [#7461](#7461) -> `docs/changelog/2026-07-24.mdx`: Record the ownership-preserving Hermes image layer reduction and hosted timing comparison. - [#7460](#7460) -> `docs/changelog/2026-07-24.mdx`: Record removal of candidate Hermes swap setup from E2E validation. - [#7458](#7458) -> `docs/security/fern-5.80.1-dependency-review.md`, `docs/changelog/2026-07-24.mdx`: Record the reviewed Fern CLI update. - [#7457](#7457) -> `docs/changelog/2026-07-24.mdx`: Record periodic runner-pressure telemetry. - [#7455](#7455) -> `docs/changelog/2026-07-24.mdx`: Record non-blocking absent Fern previews. - [#7450](#7450) -> `docs/changelog/2026-07-24.mdx`: Record stable cancellation handling for live-test child processes. - [#7449](#7449) -> `docs/changelog/2026-07-24.mdx`: Record parallel plugin EXDEV coverage. - [#7448](#7448) -> `docs/changelog/2026-07-24.mdx`: Record isolated long-running E2E lanes. - [#7444](#7444) -> `docs/changelog/2026-07-24.mdx`: Record exact-head Hermes swap validation. - [#7437](#7437) -> `docs/manage-sandboxes/backup-restore.mdx`, `docs/changelog/2026-07-24.mdx`: Record gateway pairing and authenticated verification after cross-sandbox restore. - [#7436](#7436) -> `docs/manage-sandboxes/backup-restore.mdx`, `docs/reference/commands.mdx`, `docs/changelog/2026-07-24.mdx`: Record selected stale-state cleanup and Hermes virtual-environment access repair. - [#7385](#7385) -> `docs/network-policy/customize-network-policy.mdx`, `docs/changelog/2026-07-24.mdx`: Record the read-only agent-variant route check. - [#7371](#7371) -> `docs/changelog/2026-07-24.mdx`: Record host-artifact verification for session exports. - [#7359](#7359) -> `docs/changelog/2026-07-24.mdx`: Record platform validation for managed vLLM model overrides. - [#7356](#7356) -> `docs/changelog/2026-07-24.mdx`: Record token-shaped value redaction for `sandbox doctor --json`. - [#7354](#7354) -> `docs/security/advisory-early-warning.md`, `docs/changelog/2026-07-24.mdx`: Record advisory correlation and retained audit provenance. - [#7352](#7352) -> `docs/network-policy/customize-network-policy.mdx`, `docs/network-policy/integration-policy-examples.mdx`, `docs/reference/commands.mdx`, `docs/changelog/2026-07-24.mdx`: Record preset reapplication and bounded `tls: skip` guidance. - [#7345](#7345) -> `docs/security/openclaw-2026.6.10-dependency-review.md`, `docs/security/openclaw-2026.7.1-dependency-review.md`, `docs/changelog/2026-07-24.mdx`: Record reviewed npm audit exception enforcement. - [#7340](#7340) -> `docs/network-policy/customize-network-policy.mdx`, `docs/changelog/2026-07-24.mdx`: Record the repaired CLI-reference route. - [#7334](#7334) -> `docs/get-started/dgx-station-preparation.mdx`, `docs/changelog/2026-07-24.mdx`: Record the qualified OTA metadata fallback and narrowed override wording. - [#7322](#7322) -> `docs/changelog/2026-07-24.mdx`: Reconcile the gateway source tag added to plugin registration banners. - [#7284](#7284) -> `docs/manage-sandboxes/update-sandboxes.mdx`, `docs/changelog/2026-07-24.mdx`: Record read-only `upgrade-sandboxes --check` behavior and recorded-gateway selection. - [#7277](#7277) -> `docs/changelog/2026-07-24.mdx`: Reconcile deterministic gateway TCP refusal coverage. - [#7234](#7234) -> `docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-24.mdx`: Record preserved DGX Spark managed vLLM Express intent on resume. - [#7185](#7185) -> `docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-24.mdx`: Record IPv4 fallback DNS selection and exact resolver probing. - [#6820](#6820) -> `docs/reference/commands.mdx`, `docs/changelog/2026-07-24.mdx`: Record the versioned, redacted `--events=jsonl` onboarding stream. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `npx vitest run test/changelog-docs.test.ts` passed 6/6 tests. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/changelog/2026-07-24.mdx`; the writing rules, documentation style, exact release range, skip terms, published routes, and product scope were reviewed; the changelog test passed 6/6; `npm run docs` passed with route checking OK, zero errors, and two existing warnings. - Agent: Codex Desktop <!-- docs-review-head-sha: 65368f9 --> <!-- docs-review-agents-blob-sha: 9c9b36d --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable - Result: Not applicable - Supporting evidence: Not applicable. This PR does not change `scripts/prepare-dgx-station-host.sh`. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts` passed 6/6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to the dated changelog entry. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only). The build passed with zero errors and two existing Fern warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only). Native dated changelog entries use the required parser-safe MDX SPDX comment and no frontmatter. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added the v0.0.94 release changelog. * Documented improvements to sandbox snapshot and restore behavior. * Added updates for gateway selection, policy comparisons, onboarding event output, and DGX recovery workflows. * Documented enhanced diagnostics redaction, npm audit provenance, image assembly performance, and validation stability improvements. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Summary
NEMOCLAW_VLLM_MODELoverrides for platform-restricted models without a runtime field could start image and model downloads on unsupported hardware. The installer now rejects every model whose platform list excludes the detected profile before callbacks or Docker side effects. Product scope verdict: PASS — #7358 fixes existing managed-vLLM platform enforcement and does not add a supported integration or product surface; this verdict is independent of GitHubmergeStateStatus.Related Issue
Closes #7358
Changes
runVllmInstall.runVllmInstallenforces every model's platform list before downloads.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededsrc/lib/inference/vllm-models.ts,src/lib/inference/vllm.test.ts, andsrc/lib/inference/vllm.ts. All PR product paths are unchanged from reviewed headb835f6049; the new commit is a merge-only update frommain. The fix enforces documented platform support for managed-vLLM model overrides and adds no command, environment variable, or workflow.git diff --check 0670d2035...95498cf89passed.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project cli src/lib/inference/vllm.test.ts src/lib/inference/vllm-models.test.ts(100/100 passed)npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Andrew White andrewh@cdw.com