Skip to content

212 sonarqube - security blockers and some reliability maintainability blockers#2394

Merged
crivetimihai merged 3 commits intomainfrom
212_sonarqube
Jan 25, 2026
Merged

212 sonarqube - security blockers and some reliability maintainability blockers#2394
crivetimihai merged 3 commits intomainfrom
212_sonarqube

Conversation

@brian-hussey
Copy link
Copy Markdown
Member

@brian-hussey brian-hussey commented Jan 25, 2026

Remove last security blockers and 5 reliability/maintainability blockers

This is in order to reduce Sonarqube findings on the maintainability of the code base as per #212

Key Changes

Refactor certain functions to not always return the same values.
Changed to use a positive piece of work and a default return instead. Increases readability and maintainability.

Note: tls_utils changes were primarily for sonarqube to ignore security issues we purposefully allow for via user configuration.

Benefits

The key benefits of these changes are to get towards net 0 blockers from Sonarqube's point of view so that we can activate a gate not allowing code with Blocker level problems in the CI pipeline.

Files Changed

.gitignore (ignore sonar scanner directory)
mcpgateway/config.py
mcpgateway/plugins/framework/external/mcp/tls_utils.py

@crivetimihai crivetimihai self-assigned this Jan 25, 2026
@crivetimihai
Copy link
Copy Markdown
Member

Awesome, can you check if this closes any of these https://github.com/IBM/mcp-context-forge/issues?q=is%3Aissue%20state%3Aopen%20label%3Asonar please?

@brian-hussey
Copy link
Copy Markdown
Member Author

Awesome, can you check if this closes any of these https://github.com/IBM/mcp-context-forge/issues?q=is%3Aissue%20state%3Aopen%20label%3Asonar please?

No they don't, I'm targeting blockers at the moment to be able to cut off the top.
Are the tickets or the gate higher priority at the moment and I'll dig in accordingly next.

@brian-hussey brian-hussey changed the title 212 sonarqube 212 sonarqube - security blockers and some reliability maintainability blockers Jan 25, 2026
Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com>
Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com>
Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com>
@crivetimihai crivetimihai added the sonar SonarQube code quality findings label Jan 25, 2026
@crivetimihai
Copy link
Copy Markdown
Member

Review & Rebase Complete

Changes made:

  • Rebased onto main (no conflicts)
  • Fixed minor typo in tls_utils.py: extra space in comment (# Disable# Disable)

Review findings:

  • ✅ All logic preserved correctly - refactoring from early-return to positive-condition blocks
  • ✅ NOSONAR comments appropriately placed on intentional security exceptions
  • ✅ Design difference between Create/Update validators is intentional (Update delegates feature flag/allowlist checks to service layer for "grandfather clause" logic)
  • ✅ No new environment variables or configuration needed
  • ✅ All 96 tests for changed modules pass

Note on client_mode skip behavior: The security warnings in config.py were already skipped in client mode before this PR. The refactoring preserved identical behavior - only the control flow structure changed (early return → positive condition block).

Signed-off-by: Mihai Criveti crivetimihai@gmail.com

@crivetimihai crivetimihai merged commit 9adec7d into main Jan 25, 2026
53 checks passed
@crivetimihai crivetimihai deleted the 212_sonarqube branch January 25, 2026 15:17
kcostell06 pushed a commit to kcostell06/mcp-context-forge that referenced this pull request Feb 24, 2026
…y blockers (IBM#2394)

* Remove last 2 security issues from Sonarqube

Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com>

* Remove 5 of 8 blocker maintainability issues

Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com>

* Correct linting errors

Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com>

---------

Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sonar SonarQube code quality findings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants