fix: update for auditor, nss and fine tuning cves#617
Conversation
mckornfield
force-pushed
the
cves-0708-containers/mck
branch
from
8aa80e6 to
352f6d2
Compare
| /usr/local/lib/python3.12/dist-packages/tornado-*.dist-info \ | ||
| /usr/local/lib/python3.12/dist-packages/onnx \ | ||
| /usr/local/lib/python3.12/dist-packages/onnx-*.dist-info \ | ||
| /usr/local/lib/python3.12/dist-packages/flash_attn \ |
There was a problem hiding this comment.
hmmm feel like these are required, so this might be bad lol
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates auditor-tasks image bases and build inputs, Safe Synthesizer and CU129 dependency resolution, related constraints and package versions, and unsloth Hugging Face version pins. ChangesDocker image and dependency changes
Sequence Diagram(s)sequenceDiagram
participant SafeSynthesizerConfig
participant runtime_package_index_options
participant uv
SafeSynthesizerConfig->>runtime_package_index_options: select cu129 runtime
runtime_package_index_options->>uv: provide VLLM_CU129_INDEX_URL
uv->>uv: install Safe Synthesizer and vLLM dependencies
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| ARG FLASHINFER_CU129_INDEX_URL="https://flashinfer.ai/whl/cu129" | ||
| ARG PYTORCH_CU129_INDEX_URL="https://download.pytorch.org/whl/cu129" | ||
| ARG VLLM_CU129_WHEEL="vllm @ https://github.com/vllm-project/vllm/releases/download/v0.20.0/vllm-0.20.0%2Bcu129-cp38-abi3-manylinux_2_31_x86_64.whl" | ||
| ARG VLLM_CU129_WHEEL="vllm @ https://github.com/vllm-project/vllm/releases/download/v0.22.0/vllm-0.22.0%2Bcu129-cp38-abi3-manylinux_2_28_x86_64.whl" |
There was a problem hiding this comment.
I have a PR to bump to 0.24.0 in nss, but it's in limbo. This is probably fine for now though, since I'll have to come back and update this
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docker/Dockerfile.safe-synthesizer-tasks (1)
151-151: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale comment. This step now
--excludesvLLM, so "currently dominated by vLLM" is misleading. Update it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/Dockerfile.safe-synthesizer-tasks` at line 151, The comment on the runtime dependencies install step is stale because this stage now excludes vLLM, so the note about being “currently dominated by vLLM” is misleading. Update the inline Dockerfile comment near the remaining runtime dependencies install step to reflect the current behavior, keeping it aligned with the actual dependency set and the surrounding build logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docker/Dockerfile.safe-synthesizer-tasks`:
- Line 151: The comment on the runtime dependencies install step is stale
because this stage now excludes vLLM, so the note about being “currently
dominated by vLLM” is misleading. Update the inline Dockerfile comment near the
remaining runtime dependencies install step to reflect the current behavior,
keeping it aligned with the actual dependency set and the surrounding build
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 39d5a9b2-681c-4f52-97c5-27b87155512f
📒 Files selected for processing (5)
docker-bake.hcldocker/Dockerfile.auditor-tasksdocker/Dockerfile.nmp-unsloth-trainingdocker/Dockerfile.safe-synthesizer-tasksdocker/automodel/Dockerfile.nmp-automodel-base
💤 Files with no reviewable changes (1)
- docker-bake.hcl
|
| UV_CACHE_DIR=/root/.cache/uv uv sync --package nemo-safe-synthesizer-plugin --no-dev --no-editable --inexact \ | ||
| --no-install-package nemo-platform | ||
|
|
||
| RUN --mount=type=cache,target=/root/.cache/uv \ |
There was a problem hiding this comment.
can we just do this once?
There was a problem hiding this comment.
think it might be right to move to the constraints anyhow
There was a problem hiding this comment.
Done in 866c30d. The Safe Synthesizer Dockerfile now installs the patched cryptography/pyarrow versions once in the runtime dependency layer, then skips those packages during the later plugin uv sync so the workspace lock cannot downgrade them. I also backed vLLM back to the Safe Synthesizer 0.1.2 default and removed the separate vLLM install split; we can pick up the vLLM bump with the next NSS release.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docker/Dockerfile.safe-synthesizer-tasks (1)
151-160: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRemove the duplicate install step in
docker/Dockerfile.safe-synthesizer-tasks:151-160; it repeats the sameuv pip installas141-149, so it just re-runs the expensive resolution. The line 151 comment is stale.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/Dockerfile.safe-synthesizer-tasks` around lines 151 - 160, The RUN step in docker/Dockerfile.safe-synthesizer-tasks is a duplicate of the earlier uv pip install block and should be removed or replaced with the intended distinct action. Update the Dockerfile around the repeated runtime-install section so only one vLLM/runtime dependency installation via uv pip install remains, and ensure the stale comment near the duplicated block is deleted or rewritten to match the actual behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docker/Dockerfile.safe-synthesizer-tasks`:
- Around line 151-160: The RUN step in docker/Dockerfile.safe-synthesizer-tasks
is a duplicate of the earlier uv pip install block and should be removed or
replaced with the intended distinct action. Update the Dockerfile around the
repeated runtime-install section so only one vLLM/runtime dependency
installation via uv pip install remains, and ensure the stale comment near the
duplicated block is deleted or rewritten to match the actual behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 83370ce1-50a2-46ea-8c77-0e4316aaae9f
📒 Files selected for processing (2)
docker/Dockerfile.auditor-tasksdocker/Dockerfile.safe-synthesizer-tasks
🚧 Files skipped from review as they are similar to previous changes (1)
- docker/Dockerfile.auditor-tasks
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docker/Dockerfile.safe-synthesizer-tasks (2)
115-116: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse constraints instead of absolute overrides for these security floors.
This file is passed via
--overridesto everyuv pip install; uv 0.9.30 documents overrides as replacing package requirements even for invalid resolutions, whereas constraints preserve dependency compatibility. Split the existingwandboverride from thecryptography/pyarrowbounds and pass the latter with--constraints. (raw.githubusercontent.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/Dockerfile.safe-synthesizer-tasks` around lines 115 - 116, Update the Dockerfile’s uv installation arguments so the `wandb` requirement remains in the overrides file, while the `cryptography` and `pyarrow` security bounds are moved to a separate constraints file and supplied via `--constraints` for every `uv pip install` invocation.
177-183: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert the patched versions in the smoke test.
The current smoke test only checks imports, so it will pass even if a later sync reintroduces older
cryptographyorpyarrowversions. Add metadata/version assertions for both packages after the final sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/Dockerfile.safe-synthesizer-tasks` around lines 177 - 183, Add smoke-test assertions after the final uv sync to verify the installed metadata versions of cryptography and pyarrow are the expected patched versions, rather than only checking imports. Update the relevant smoke-test command in Dockerfile.safe-synthesizer-tasks, using package metadata lookups and failing when either version is older or unexpected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/Dockerfile.safe-synthesizer-tasks`:
- Line 17: Update the vLLM cu129 dependency from 0.20.0 to at least 0.22.0 in
the Dockerfile’s VLLM_CU129_WHEEL and the corresponding runtime configuration in
nemo_safe_synthesizer_plugin.runtime, ensuring both references use a compatible
released wheel and remain consistent.
---
Nitpick comments:
In `@docker/Dockerfile.safe-synthesizer-tasks`:
- Around line 115-116: Update the Dockerfile’s uv installation arguments so the
`wandb` requirement remains in the overrides file, while the `cryptography` and
`pyarrow` security bounds are moved to a separate constraints file and supplied
via `--constraints` for every `uv pip install` invocation.
- Around line 177-183: Add smoke-test assertions after the final uv sync to
verify the installed metadata versions of cryptography and pyarrow are the
expected patched versions, rather than only checking imports. Update the
relevant smoke-test command in Dockerfile.safe-synthesizer-tasks, using package
metadata lookups and failing when either version is older or unexpected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 27f950d8-5dcd-4473-aeb4-48cd23843495
📒 Files selected for processing (2)
docker/Dockerfile.safe-synthesizer-tasksplugins/nemo-safe-synthesizer/constraints.txt
✅ Files skipped from review due to trivial changes (1)
- plugins/nemo-safe-synthesizer/constraints.txt
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docker/Dockerfile.safe-synthesizer-tasks (2)
115-116: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse constraints instead of absolute overrides for these security floors.
This file is passed via
--overridesto everyuv pip install; uv 0.9.30 documents overrides as replacing package requirements even for invalid resolutions, whereas constraints preserve dependency compatibility. Split the existingwandboverride from thecryptography/pyarrowbounds and pass the latter with--constraints. (raw.githubusercontent.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/Dockerfile.safe-synthesizer-tasks` around lines 115 - 116, Update the Dockerfile’s uv installation arguments so the `wandb` requirement remains in the overrides file, while the `cryptography` and `pyarrow` security bounds are moved to a separate constraints file and supplied via `--constraints` for every `uv pip install` invocation.
177-183: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert the patched versions in the smoke test.
The current smoke test only checks imports, so it will pass even if a later sync reintroduces older
cryptographyorpyarrowversions. Add metadata/version assertions for both packages after the final sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/Dockerfile.safe-synthesizer-tasks` around lines 177 - 183, Add smoke-test assertions after the final uv sync to verify the installed metadata versions of cryptography and pyarrow are the expected patched versions, rather than only checking imports. Update the relevant smoke-test command in Dockerfile.safe-synthesizer-tasks, using package metadata lookups and failing when either version is older or unexpected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/Dockerfile.safe-synthesizer-tasks`:
- Line 17: Update the vLLM cu129 dependency from 0.20.0 to at least 0.22.0 in
the Dockerfile’s VLLM_CU129_WHEEL and the corresponding runtime configuration in
nemo_safe_synthesizer_plugin.runtime, ensuring both references use a compatible
released wheel and remain consistent.
---
Nitpick comments:
In `@docker/Dockerfile.safe-synthesizer-tasks`:
- Around line 115-116: Update the Dockerfile’s uv installation arguments so the
`wandb` requirement remains in the overrides file, while the `cryptography` and
`pyarrow` security bounds are moved to a separate constraints file and supplied
via `--constraints` for every `uv pip install` invocation.
- Around line 177-183: Add smoke-test assertions after the final uv sync to
verify the installed metadata versions of cryptography and pyarrow are the
expected patched versions, rather than only checking imports. Update the
relevant smoke-test command in Dockerfile.safe-synthesizer-tasks, using package
metadata lookups and failing when either version is older or unexpected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 27f950d8-5dcd-4473-aeb4-48cd23843495
📒 Files selected for processing (2)
docker/Dockerfile.safe-synthesizer-tasksplugins/nemo-safe-synthesizer/constraints.txt
✅ Files skipped from review due to trivial changes (1)
- plugins/nemo-safe-synthesizer/constraints.txt
🛑 Comments failed to post (1)
docker/Dockerfile.safe-synthesizer-tasks (1)
17-17: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the target Dockerfile around the referenced lines nl -ba docker/Dockerfile.safe-synthesizer-tasks | sed -n '1,240p' # Find related dependency/override files and references rg -n "vllm|NSS|override|constraints|uv 0.9.30|0.20.0|0.1.2" docker . --glob '!**/.git/**'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 203
🏁 Script executed:
#!/bin/bash set -euo pipefail # Read the Dockerfile with line numbers using POSIX tools cat -n docker/Dockerfile.safe-synthesizer-tasks | sed -n '1,240p' # Locate vLLM/NSS/override references in the repo rg -n "vllm|NSS|override|constraints|uv 0.9.30|0.20.0|0.1.2" docker . --glob '!**/.git/**'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 50382
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== Dockerfile slice ==" sed -n '1,220p' docker/Dockerfile.safe-synthesizer-tasks | cat -n echo "== Files mentioning nemo-safe-synthesizer or vllm ==" rg -n "nemo-safe-synthesizer|vllm|VLLM_CU129_WHEEL|safe-synthesizer" \ docker packages pyproject.toml .github --glob '!**/.git/**' --glob '!**/dist/**' --glob '!**/build/**' echo "== Candidate metadata files ==" git ls-files | rg '(^|/)(pyproject\.toml|requirements.*|uv\.lock|constraints.*|override.*|Dockerfile\.safe-synthesizer-tasks)$'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 22309
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== plugins/nemo-safe-synthesizer/pyproject.toml ==" sed -n '1,260p' plugins/nemo-safe-synthesizer/pyproject.toml | cat -n echo "== plugins/nemo-safe-synthesizer/constraints.txt ==" sed -n '1,220p' plugins/nemo-safe-synthesizer/constraints.txt | cat -n echo "== vllm references in safe-synthesizer plugin ==" rg -n "vllm|nemo-safe-synthesizer|cu129|0.20.0|0.1.2" plugins/nemo-safe-synthesizer sdk/python/overrides/nemo-platform packages/nemo_platform --glob '!**/.git/**'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 11839
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== plugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/runtime.py ==" sed -n '1,260p' plugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/runtime.py | cat -n echo "== Search for any version constraints tied to vllm ==" rg -n "VLLM_CU129_VERSION|vllm_cu129_wheel|vllm.*0\.20\.0|0\.22\.0|requires.*vllm|compatible.*vllm" plugins/nemo-safe-synthesizer packages/nemo_platform docker --glob '!**/.git/**'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 9895
🌐 Web query:
vLLM 0.20.0 advisory fixed in 0.22.0 vulnerability release notes💡 Result:
Multiple security vulnerabilities affecting vLLM versions prior to 0.22.0 were remediated in the 0.22.0 release [1][2]. Specifically, vLLM 0.22.0 introduced fixes for the following security issues: 1. Security Check Bypass via assert Statement (CVE-2026-41523): An assert-based security check in the activation function loading mechanism could be bypassed when vLLM was run in Python optimized mode (using python -O or PYTHONOPTIMIZE=1), potentially allowing arbitrary code execution when loading a malicious HuggingFace model [3][1]. 2. OpenAI API Authentication Bypass (CVE-2026-48746): An authentication bypass existed in the OpenAI API middleware due to inconsistent interpretation of HTTP requests (specifically involving the Host header), which allowed access without providing the required API key [4][2]. 3. Artifact Pin Decay (CVE-2026-47155): vLLM failed to consistently apply revision pinning to all secondary artifacts (such as GGUF files, image processors, or side weights), which could lead to deployments loading unpinned code or weights despite having an explicit revision pin [5]. Additionally, users should note that other vulnerabilities may affect specific version ranges, such as the SSRF vulnerability in download_bytes_from_url (CVE-2026-34753), which was fixed in 0.19.0 [6], and the Remote DoS via Invalid Recovered Token Reinjection (CVE-2026-54234), which was identified in versions up to 0.17.1 and requires later patching [7]. Always verify the specific CVEs and affected versions in the official GitHub security advisories for the most accurate security posture [3][5][6][7][4].
Citations:
- 1: https://nvd.nist.gov/vuln/detail/CVE-2026-41523
- 2: https://pypi.org/project/vllm/0.20.0/
- 3: GHSA-q8gq-377p-jq3r
- 4: GHSA-94f4-hr76-p5j6
- 5: GHSA-3ww4-5jv9-j5gm
- 6: GHSA-pf3h-qjgv-vcpr
- 7: GHSA-8wr5-jm2h-8r4f
Bump the vLLM cu129 pin
docker/Dockerfile.safe-synthesizer-tasksandplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/runtime.pyboth hardcode vLLM0.20.0. That’s below the0.22.0release that fixes multiple security issues, so this image still ships with the vulnerable line. Upgrade the cu129 wheel or document why those paths are unreachable here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/Dockerfile.safe-synthesizer-tasks` at line 17, Update the vLLM cu129 dependency from 0.20.0 to at least 0.22.0 in the Dockerfile’s VLLM_CU129_WHEEL and the corresponding runtime configuration in nemo_safe_synthesizer_plugin.runtime, ensuring both references use a compatible released wheel and remain consistent.
mckornfield
force-pushed
the
cves-0708-containers/mck
branch
from
856806b to
4331bf2
Compare
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
mckornfield
force-pushed
the
cves-0708-containers/mck
branch
from
4331bf2 to
2343e44
Compare
2 participants
Summary by CodeRabbit
Security
Compatibility
Performance