boot_from_cdrom_device: avoid os/boot plus cdrom boot conflict#6865
Open
BulaYoungR wants to merge 1 commit into
Open
boot_from_cdrom_device: avoid os/boot plus cdrom boot conflict#6865BulaYoungR wants to merge 1 commit into
BulaYoungR wants to merge 1 commit into
Conversation
When os_attrs_boots is set, call remove_all_boots() before setup_attrs, and drop boot/loadparm from cdrom attr dicts so libvirt does not see per-device boot together with <os>/boot. Committer: Bolatbek Issakh <bissakh@redhat.com>
WalkthroughThe change updates a test utility function to enforce OS-level boot configuration precedence over device-level boot settings. When Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libvirt/tests/src/guest_os_booting/boot_order/boot_from_cdrom_device.py (1)
52-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace all
eval()calls withast.literal_eval()for secure parameter parsing.The file contains four unsafe
eval()calls (lines 25, 26, 52, and 92) that can execute arbitrary code. Replace each withast.literal_eval()to safely parse only Python literals:Proposed fix
+import ast import os import platform @@ - cdrom1_attrs = eval(params.get('cdrom1_attrs', '{}')) - cdrom2_attrs = eval(params.get('cdrom2_attrs', '{}')) + try: + cdrom1_attrs = ast.literal_eval(params.get('cdrom1_attrs', '{}')) + cdrom2_attrs = ast.literal_eval(params.get('cdrom2_attrs', '{}')) + except (ValueError, SyntaxError) as detail: + test.fail(f"Invalid cdrom attrs format: {detail}") @@ - os_attrs_boots = eval(params.get('os_attrs_boots', '[]')) + try: + os_attrs_boots = ast.literal_eval(params.get('os_attrs_boots', '[]')) + except (ValueError, SyntaxError) as detail: + test.fail(f"Invalid os_attrs_boots format: {detail}") @@ - bootable_patterns = eval(params.get('bootable_patterns', '[]')) + try: + bootable_patterns = ast.literal_eval(params.get('bootable_patterns', '[]')) + except (ValueError, SyntaxError) as detail: + test.fail(f"Invalid bootable_patterns format: {detail}")🤖 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 `@libvirt/tests/src/guest_os_booting/boot_order/boot_from_cdrom_device.py` at line 52, Replace all unsafe eval() usages used to parse params with ast.literal_eval to prevent arbitrary code execution: import ast at the top if missing, then change expressions like os_attrs_boots = eval(params.get('os_attrs_boots', '[]')) to os_attrs_boots = ast.literal_eval(params.get('os_attrs_boots', '[]')) (and do the same for the other three eval(params.get(...)) occurrences); ensure default strings remain valid Python literals (e.g., '[]' or '{}' as appropriate) so ast.literal_eval can parse them.
🤖 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.
Outside diff comments:
In `@libvirt/tests/src/guest_os_booting/boot_order/boot_from_cdrom_device.py`:
- Line 52: Replace all unsafe eval() usages used to parse params with
ast.literal_eval to prevent arbitrary code execution: import ast at the top if
missing, then change expressions like os_attrs_boots =
eval(params.get('os_attrs_boots', '[]')) to os_attrs_boots =
ast.literal_eval(params.get('os_attrs_boots', '[]')) (and do the same for the
other three eval(params.get(...)) occurrences); ensure default strings remain
valid Python literals (e.g., '[]' or '{}' as appropriate) so ast.literal_eval
can parse them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 41516faf-e896-4580-b7c9-02595e120f8b
📒 Files selected for processing (1)
libvirt/tests/src/guest_os_booting/boot_order/boot_from_cdrom_device.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When os_attrs_boots is set, call remove_all_boots() before setup_attrs, and drop boot/loadparm from cdrom attr dicts so libvirt does not see per-device boot together with /boot.
Committer: Bolatbek Issakh bissakh@redhat.com
Summary by CodeRabbit