fix(pxe): fully write out sparse OS images#2815
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. |
WalkthroughA single character flag, ChangesDisk Imaging Command Update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pxe/common_files/disk_imaging.sh (1)
534-534: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap unquoted shell variables to prevent word-splitting and globbing.
The variables
$file,$image_disk, and$log_outputlack double-quotes on line 534. While their expected values (filenames, device paths, log paths) are typically single tokens, unquoted variables create word-splitting and globbing vulnerabilities if these values ever contain spaces, glob characters, or special shell metacharacters. This violation of shell quoting discipline is flagged by ShellCheck (SC2086).🔧 Proposed fix with properly quoted variables
- qemu-img convert -p -O raw -S 0 $file $image_disk 2>&1 | tee $log_output + qemu-img convert -p -O raw -S 0 "$file" "$image_disk" 2>&1 | tee "$log_output"🤖 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 `@pxe/common_files/disk_imaging.sh` at line 534, The qemu-img convert command on line 534 uses unquoted shell variables $file, $image_disk, and $log_output which creates word-splitting and globbing vulnerabilities. Wrap each of these three variables in double quotes in the command: "$file", "$image_disk", and "$log_output" to properly protect against spaces and special characters that may appear in their values.Source: Linters/SAST tools
🤖 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 `@pxe/common_files/disk_imaging.sh`:
- Line 534: The qemu-img convert command on line 534 uses unquoted shell
variables $file, $image_disk, and $log_output which creates word-splitting and
globbing vulnerabilities. Wrap each of these three variables in double quotes in
the command: "$file", "$image_disk", and "$log_output" to properly protect
against spaces and special characters that may appear in their values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cb3c556f-0770-4f90-ba0f-16a3327199f3
📒 Files selected for processing (1)
pxe/common_files/disk_imaging.sh
iExalt
changed the title
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
2 participants
Description
Ensure OS image conversion writes zero-filled and unallocated source regions to the destination boot disk. Without
-S 0, sparse conversion can leave stale filesystem or volume metadata behind when reprovisioning a previously used disk.1s1s then a 9GB sparse region of0s-S 0flag will not overwrite the 2nd GB of1s with zeros as expected; the disk will still contain 2GB of1s.Related issues
Type of Change
Breaking Changes
Testing
Validated with:
git diff --check nvidia-upstream/main...personal-upstream/codex/pxe-overwrite-sparse-imagesbash -n pxe/common_files/disk_imaging.shAdditional Notes
qemu-img convert -S 0disables sparse-region detection so zero-filled regions represented by the source image are explicitly written. This is not a secure wipe of any destination capacity beyond the source image's virtual size.