Add SFP port mode to brocade plugin#6062
Conversation
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| if (defined($self->{option_results}->{display_transform_src})) { | ||
| $self->{option_results}->{display_transform_dst} = '' if (!defined($self->{option_results}->{display_transform_dst})); | ||
|
|
||
| $self->{safe}->reval("\$assign_var =~ s{$self->{option_results}->{display_transform_src}}{$self->{option_results}->{display_transform_dst}}", |
There was a problem hiding this comment.
User-provided values display_transform_src/display_transform_dst are interpolated into Safe->reval code (dynamic eval). Avoid evaluating user-controlled expressions directly; use a safe regex compilation API or validate/escape inputs before use.
Details
✨ AI Reasoning
The code constructs and executes a Perl substitution expression via Safe->reval using values coming from option_results (display_transform_src and display_transform_dst). Those options are controllable by the user at runtime. Executing dynamically-built code based on user-provided strings can lead to code injection or unexpected execution even when using Safe, because the substituted expression may contain metacharacters or constructs that alter the evaluated code. This is a high-risk pattern for injection vulnerabilities and was introduced in the new sfpport mode. The problematic call is the dynamic reval invocation that embeds untrusted option values into code passed to the evaluator. The issue harms security and should be addressed in this PR rather than left for a larger refactor.
🔧 How do I fix it?
Use parameterized queries with placeholders, array-based command execution (no shell interpretation), or properly escaped arguments using vetted libraries. Avoid dynamic queries/commands built with user input concatenation.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
| " RX: %s, TX: %s", | ||
| $self->{result_values}->{tx_power_status}, | ||
| $self->{result_values}->{rx_power_status} |
There was a problem hiding this comment.
custom_status_output swaps RX/TX values: 'RX' prints tx_power_status and 'TX' prints rx_power_status, causing incorrect status reporting.
Show fix
| " RX: %s, TX: %s", | |
| $self->{result_values}->{tx_power_status}, | |
| $self->{result_values}->{rx_power_status} | |
| " RX: %s, TX: %s", | |
| $self->{result_values}->{rx_power_status}, | |
| $self->{result_values}->{tx_power_status} |
Details
✨ AI Reasoning
1. The code is building a human-readable status message for SFP power state.
2. The message text says the first value is RX and the second is TX.
3. The first interpolated value comes from tx_power_status and the second from rx_power_status.
4. This creates a guaranteed mismatch between reported labels and actual values, leading to incorrect monitoring output interpretation.
5. This is a concrete correctness issue, not a style concern, and is fully within the scope of the introduced changes.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
|
|
||
| Filter ports by interface name (can be a regexp). Can be used only together with --add-interface-name. | ||
|
|
||
| =item B<--include-interface> |
There was a problem hiding this comment.
Help text is contradictory: '--include-interface' is documented as excluding ports by interface name, which conflicts with the option name and expected behavior.
| =item B<--include-interface> | |
| =item B<--exclude-interface> |
Details
✨ AI Reasoning
The help entry declares an option named include-interface but describes exclusion semantics. This creates contradictory behavior expectations for users and can lead to wrong command usage because the documented intent cannot match the option name shown.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
2 participants
Community contributors
Description
The plugin has been extended to include a --list-sfp-ports mode and a --sfp-port option.
In addition, the standard SNMP modes --tcp-con, --udp-con, and --uptime have been added.
Type of change
How this pull request can be tested ?
The walk is from a ExtremeSLX9640 switch but is based on the brocade MIBs .1.3.6.1.4.1.1588
BROCADE-OPTICAL-MONITORING-MIB.mib.txt.
extremeSLX.-brocadesnmpwalk.txt.zip
Checklist
Centreon team (internal PR)
Description
PLEASE MAKE SURE THAT THE BRANCH PR INCLUDES JIRA TICKET ID
Please include a short resume of the changes and what is the purpose of this pull request.
Any relevant information should be added to help reviewers to understand what are the stakes
of the pull request.
Fixes # (issue)
If you are fixing a github Issue already existing, mention it here.
If you are fixing one or more JIRA ticket, mention it here too.
Type of change
How this pull request can be tested ?
Please describe the procedure to verify that the goal of the PR is matched.
Provide clear instructions so that it can be correctly tested.
Mention the automated tests included in this FOR (what they test like mode/option combinations).
Checklist
Summary by Aikido
🚀 New Features
⚡ Enhancements
🔧 Refactors
More info