Skip to content

improve APC UPS inputlines#6147

Open
rmorandell-pgum wants to merge 4 commits into
centreon:developfrom
i-Vertix:apc-ups-3phases
Open

improve APC UPS inputlines#6147
rmorandell-pgum wants to merge 4 commits into
centreon:developfrom
i-Vertix:apc-ups-3phases

Conversation

@rmorandell-pgum
Copy link
Copy Markdown
Contributor

@rmorandell-pgum rmorandell-pgum commented Apr 28, 2026

Community contributors

Description

The first problem is that there are APCs that do not provide input line voltage information. Since this SNMP query is set to nothing_quit => 1, we do not receive any information about the input phases, even though it is actually available.

The second problem is that there are, for example, the Galaxy series, which not only have the L-N but also the L-L

image

These are two different types of voltages:
• Phase to Neutral (L–N) → ~230 V
• Phase to Phase (L–L) → ~400 V
So you have:
• 3× L–N voltages (L1, L2, L3 → each about 230 V)
• 3× L–L voltages (L1–L2, L2–L3, L3–L1 → each about 400 V)

In the SNMP table

image

The index is one number longer as the normal one

image

so what I changed in the plugin?

• I've updated the (mode) class to meet the new standard. That is, counter constants, is_excluded, and include-exclude filters.
• This Galaxy doesn't have any InputLine data, so just the phases and the frequency. That's why I removed the “nothing_quit => 1”. It's not a problem because the code still runs afterward.
• This OID regex match I extend to possible 18 long OID

image

• The input number and the inputPhaseIndex I get from the SNMP table. Working for all APCs

image

• Filter by input_type (main, bypass)

image

• Filter by input phase

image

• help

image

So with the default exclude we exclude all L-L (3 numbers long instances).

without filter

image

using exclude filter

image

So you can see the plugin with this default should be work still in the same way as before but is working fine also for this “big” Galaxy” 3 Phase APCs.

Type of change

  • Patch fixing an issue (non-breaking change)
  • New functionality (non-breaking change)
  • Functionality enhancement or optimization (non-breaking change)
  • Breaking change (patch or feature) that might cause side effects breaking part of the Software

How this pull request can be tested ?

i will send a download link via private mail with 4 snmpwalks

Checklist

  • I have followed the coding style guidelines provided by Centreon
  • I have commented my code, especially hard-to-understand areas of the PR.
  • I have rebased my development branch on the base branch (develop).
  • I have provide data or shown output displaying the result of this code in the plugin area concerned.

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

  • Patch fixing an issue (non-breaking change)
  • New functionality (non-breaking change)
  • Functionality enhancement or optimization (non-breaking change)
  • Breaking change (patch or feature) that might cause side effects breaking part of the Software

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

  • I have followed the coding style guidelines provided by Centreon
  • I have commented my code, especially hard-to-understand areas of the PR.
  • I have rebased my development branch on the base branch (develop).
  • In case of a new plugin, I have created the new packaging directory accordingly.
  • I have implemented automated tests related to my commits.
    • Data used for automated tests are anonymized.
  • I have reviewed all the help messages in all the .pm files I have modified.
    • All sentences begin with a capital letter.
    • All sentences end with a period.
    • I am able to understand all the help messages, if not, exchange with the PO or TW to rewrite them.
  • After having created the PR, I will make sure that all the tests provided in this PR have run and passed.

Summary by Aikido

Security Issues: 0 🔍 Quality Issues: 2 Resolved Issues: 0

⚡ Enhancements

  • Converted counters to Centreon constant types and multi-instance handling.
  • Added imports for is_excluded and constants to support filtering.

🔧 Refactors

  • Renamed and adjusted output prefix functions to include instance context.

📚 Documentation

  • Updated copyright year from 2025 to 2026 in header.

More info

@rmorandell-pgum rmorandell-pgum requested a review from a team as a code owner April 28, 2026 12:24
@rmorandell-pgum rmorandell-pgum requested review from sdepassio and removed request for a team April 28, 2026 12:24
instance => $instance
);

next if is_excluded(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User-controlled include/exclude patterns (include_input_type/exclude_input_type) are passed directly to is_excluded. This can allow regex injection / ReDoS via crafted option values. Validate or safely escape/limit regex input before use.

Details

✨ AI Reasoning
​The code now calls is_excluded(...) with patterns coming directly from option_results (user input). These values are likely used as regular expressions or evaluated as patterns inside is_excluded. Supplying untrusted patterns to regex-matching utilities can allow regex injection or ReDoS (catastrophic backtracking) attacks or unintended behavior. The new calls were added in this PR and therefore represent an introduced risk.

🔧 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

my $instance = $phase_result->{inputNumber} . $phase_result->{inputPhaseIndex};

next if !defined($self->{inputs}->{$input_num});# input has just be excluded on inputs
next if is_excluded(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User-controlled include/exclude patterns (include_input_phase/exclude_input_phase) are passed directly to is_excluded. This can allow regex injection / ReDoS via crafted option values. Validate or safely escape/limit regex input before use.

Details

✨ AI Reasoning
​The diff adds another is_excluded(...) call that uses option_results include_input_phase/exclude_input_phase directly. As above, these options are user-controlled and likely interpreted as regex patterns by is_excluded, enabling regex injection or ReDoS if maliciously crafted. This call was introduced in the PR and therefore is a new risk.

🔧 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants