qickfix for "false" route handling#2979
Conversation
…aned up afterward
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the network routing logic in lib/cuckoo/core/analysis_manager.py to include 'false' as a valid value for disabling routing in both route_network and unroute_network methods. The reviewer suggests refactoring these checks to be case-insensitive and to handle potential boolean types more robustly, which would also reduce duplication across the file.
| self.rooter_response = rooter("libvirt_fwo_enable", self.machine.interface, self.machine.ip) | ||
|
|
||
| elif self.route in ("none", "None", "drop"): | ||
| elif self.route in ("none", "None", "drop", "false"): |
There was a problem hiding this comment.
The tuple of routing values ("none", "None", "drop", "false") is now duplicated in three places within this file (lines 540, 609, and 746). To improve maintainability and robustness, consider using a case-insensitive check. This approach also handles variations like "False" or "DROP" and correctly processes boolean False values that might be passed via API calls.
| elif self.route in ("none", "None", "drop", "false"): | |
| elif str(self.route).lower() in ("none", "drop", "false"): |
| self.rooter_response = rooter("libvirt_fwo_disable", self.machine.interface, self.machine.ip) | ||
|
|
||
| elif self.route in ("none", "None", "drop"): | ||
| elif self.route in ("none", "None", "drop", "false"): |
There was a problem hiding this comment.
For consistency with the logic in route_network, consider using a case-insensitive check here as well. This ensures that the cleanup process correctly identifies all variations of the 'no routing' state.
| elif self.route in ("none", "None", "drop", "false"): | |
| elif str(self.route).lower() in ("none", "drop", "false"): |
1 participant
If a user or an API call set the routing option to "false":
1.Initial identification (line 540) worked: The system use four different values as indicators to disable routing: "none", "None", "drop", and "false"
2.The "Drop" command was skipped: The code failed to call rooter("drop_enable", ...) because "false" was missing from the second check. This meant the network traffic might not have been explicitly blocked by the rooter as intended.
3.Cleanup was skipped: During task teardown, rooter("drop_disable", ...) was not called, potentially leaving the network state inconsistent for future tasks.
this fix ensure that rooter correctly enables and disables traffic dropping when "false" is used.