diff --git a/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json b/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json index f6b92127a6e..e51a0ca0397 100644 --- a/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json +++ b/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json @@ -308,7 +308,7 @@ { "ruleKey": "S1168", "hasTruePositives": true, - "falseNegatives": 24, + "falseNegatives": 25, "falsePositives": 0 }, { @@ -2738,7 +2738,7 @@ { "ruleKey": "S6813", "hasTruePositives": true, - "falseNegatives": 66, + "falseNegatives": 67, "falsePositives": 0 }, { @@ -3190,5 +3190,11 @@ "hasTruePositives": true, "falseNegatives": 1, "falsePositives": 0 + }, + { + "ruleKey": "S8912", + "hasTruePositives": false, + "falseNegatives": 8, + "falsePositives": 0 } ] diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1168.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1168.json index 8887c268f44..f5aa4440d59 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1168.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1168.json @@ -1,6 +1,6 @@ { "ruleKey": "S1168", "hasTruePositives": true, - "falseNegatives": 24, + "falseNegatives": 25, "falsePositives": 0 } diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S6813.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6813.json index a9b3acaf13a..f32d08ed71d 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S6813.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6813.json @@ -1,6 +1,6 @@ { "ruleKey": "S6813", "hasTruePositives": true, - "falseNegatives": 66, + "falseNegatives": 67, "falsePositives": 0 } \ No newline at end of file diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S8912.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8912.json new file mode 100644 index 00000000000..57cd5afabdc --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8912.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S8912", + "hasTruePositives": false, + "falseNegatives": 8, + "falsePositives": 0 +} diff --git a/java-checks-test-sources/default/src/main/java/checks/CredentialsProviderUnremovableCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/CredentialsProviderUnremovableCheckSample.java new file mode 100644 index 00000000000..d89f848b895 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/CredentialsProviderUnremovableCheckSample.java @@ -0,0 +1,134 @@ +package checks; + +import io.quarkus.arc.Unremovable; +import io.quarkus.credentials.CredentialsProvider; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.context.Dependent; +import jakarta.enterprise.context.RequestScoped; +import jakarta.enterprise.context.SessionScoped; +import jakarta.inject.Inject; +import jakarta.inject.Singleton; +import java.util.Map; + +@ApplicationScoped +class MyCredentialsProvider implements CredentialsProvider { // Noncompliant {{Add the @Unremovable annotation to this CredentialsProvider implementation.}} +// ^^^^^^^^^^^^^^^^^^^^^ + + @Override + public Map getCredentials(String credentialsProviderName) { + return Map.of(USER_PROPERTY_NAME, "user", PASSWORD_PROPERTY_NAME, "password"); + } +} + +@ApplicationScoped +@Unremovable +class CompliantProvider implements CredentialsProvider { + + @Override + public Map getCredentials(String credentialsProviderName) { + return Map.of(USER_PROPERTY_NAME, "user", PASSWORD_PROPERTY_NAME, "password"); + } +} + +@RequestScoped +class RequestScopedProvider implements CredentialsProvider { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^ + + @Override + public Map getCredentials(String credentialsProviderName) { + return Map.of(USER_PROPERTY_NAME, "user", PASSWORD_PROPERTY_NAME, "pass"); + } +} + +@Singleton +class SingletonProvider implements CredentialsProvider { // Noncompliant +// ^^^^^^^^^^^^^^^^^ + + @Override + public Map getCredentials(String credentialsProviderName) { + return Map.of(USER_PROPERTY_NAME, "admin", PASSWORD_PROPERTY_NAME, "secret"); + } +} + +@Dependent +class DependentProvider implements CredentialsProvider { // Noncompliant +// ^^^^^^^^^^^^^^^^^ + + @Override + public Map getCredentials(String credentialsProviderName) { + return Map.of(USER_PROPERTY_NAME, "user", PASSWORD_PROPERTY_NAME, "pwd"); + } +} + +@SessionScoped +class SessionScopedProvider implements CredentialsProvider { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^ + + @Override + public Map getCredentials(String credentialsProviderName) { + return Map.of(USER_PROPERTY_NAME, "user", PASSWORD_PROPERTY_NAME, "pass"); + } +} + +class NoScopeProvider implements CredentialsProvider { + + @Override + public Map getCredentials(String credentialsProviderName) { + return Map.of(USER_PROPERTY_NAME, "user", PASSWORD_PROPERTY_NAME, "password"); + } +} + +@ApplicationScoped +class SomeOtherService { + public void doSomething() { + } +} + +@Singleton +@Unremovable +class CompliantSingleton implements CredentialsProvider { + + @Override + public Map getCredentials(String credentialsProviderName) { + return Map.of(USER_PROPERTY_NAME, "admin", PASSWORD_PROPERTY_NAME, "secret"); + } +} + +@ApplicationScoped +class ProviderWithDependencies implements CredentialsProvider { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^ + + @Inject + ConfigService configService; + + @Override + public Map getCredentials(String credentialsProviderName) { + String username = configService.getUsername(); + String password = configService.getPassword(); + return Map.of(USER_PROPERTY_NAME, username, PASSWORD_PROPERTY_NAME, password); + } +} + +@ApplicationScoped +class MultiInterfaceProvider implements CredentialsProvider, AutoCloseable { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^ + + @Override + public Map getCredentials(String credentialsProviderName) { + return Map.of(USER_PROPERTY_NAME, "user", PASSWORD_PROPERTY_NAME, "password"); + } + + @Override + public void close() { + } +} + +class ConfigService { + String getUsername() { + return "configUser"; + } + + String getPassword() { + return "configPass"; + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/CredentialsProviderUnremovableCheckSampleMinimal.java b/java-checks-test-sources/default/src/main/java/checks/CredentialsProviderUnremovableCheckSampleMinimal.java new file mode 100644 index 00000000000..2d2a630840f --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/CredentialsProviderUnremovableCheckSampleMinimal.java @@ -0,0 +1,15 @@ +package checks; + +import io.quarkus.credentials.CredentialsProvider; +import jakarta.enterprise.context.ApplicationScoped; +import java.util.Map; + +@ApplicationScoped +class MinimalProvider implements CredentialsProvider { // Noncompliant {{Add the @Unremovable annotation to this CredentialsProvider implementation.}} +// ^^^^^^^^^^^^^^^ + + @Override + public Map getCredentials(String credentialsProviderName) { + return null; + } +} diff --git a/java-checks-test-sources/default/src/main/java/io/quarkus/arc/Unremovable.java b/java-checks-test-sources/default/src/main/java/io/quarkus/arc/Unremovable.java new file mode 100644 index 00000000000..c00de7483af --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/io/quarkus/arc/Unremovable.java @@ -0,0 +1,11 @@ +package io.quarkus.arc; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ElementType.TYPE, ElementType.METHOD, ElementType.FIELD}) +@Retention(RetentionPolicy.RUNTIME) +public @interface Unremovable { +} \ No newline at end of file diff --git a/java-checks-test-sources/default/src/main/java/io/quarkus/credentials/CredentialsProvider.java b/java-checks-test-sources/default/src/main/java/io/quarkus/credentials/CredentialsProvider.java new file mode 100644 index 00000000000..8d912340f40 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/io/quarkus/credentials/CredentialsProvider.java @@ -0,0 +1,10 @@ +package io.quarkus.credentials; + +import java.util.Map; + +public interface CredentialsProvider { + String USER_PROPERTY_NAME = "user"; + String PASSWORD_PROPERTY_NAME = "password"; + + Map getCredentials(String credentialsProviderName); +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/CredentialsProviderUnremovableCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CredentialsProviderUnremovableCheck.java new file mode 100644 index 00000000000..06f9cde4f83 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/CredentialsProviderUnremovableCheck.java @@ -0,0 +1,85 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.List; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.semantic.SymbolMetadata; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S8912") +public class CredentialsProviderUnremovableCheck extends IssuableSubscriptionVisitor { + private static final String CREDENTIALS_PROVIDER_FQN = "io.quarkus.credentials.CredentialsProvider"; + private static final String UNREMOVABLE_FQN = "io.quarkus.arc.Unremovable"; + + private static final List CDI_SCOPE_ANNOTATIONS = List.of( + "jakarta.enterprise.context.ApplicationScoped", + "jakarta.enterprise.context.RequestScoped", + "jakarta.enterprise.context.SessionScoped", + "jakarta.enterprise.context.Dependent", + "jakarta.inject.Singleton", + "javax.enterprise.context.ApplicationScoped", + "javax.enterprise.context.RequestScoped", + "javax.enterprise.context.SessionScoped", + "javax.enterprise.context.Dependent", + "javax.inject.Singleton" + ); + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CLASS); + } + + @Override + public void visitNode(Tree tree) { + ClassTree classTree = (ClassTree) tree; + + if (classTree.simpleName() == null) { + return; + } + + if (!implementsCredentialsProvider(classTree)) { + return; + } + + SymbolMetadata metadata = classTree.symbol().metadata(); + + if (!hasCdiScopeAnnotation(metadata)) { + return; + } + + if (metadata.isAnnotatedWith(UNREMOVABLE_FQN)) { + return; + } + + reportIssue( + classTree.simpleName(), + "Add the @Unremovable annotation to this CredentialsProvider implementation." + ); + } + + private static boolean implementsCredentialsProvider(ClassTree classTree) { + return classTree.symbol().type().isSubtypeOf(CREDENTIALS_PROVIDER_FQN); + } + + private static boolean hasCdiScopeAnnotation(SymbolMetadata metadata) { + return CDI_SCOPE_ANNOTATIONS.stream() + .anyMatch(metadata::isAnnotatedWith); + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/CredentialsProviderUnremovableCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/CredentialsProviderUnremovableCheckTest.java new file mode 100644 index 00000000000..d83a0f71848 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/CredentialsProviderUnremovableCheckTest.java @@ -0,0 +1,53 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class CredentialsProviderUnremovableCheckTest { + private static final String SAMPLE_FILE = mainCodeSourcesPath("checks/CredentialsProviderUnremovableCheckSample.java"); + private static final String MINIMAL_FILE = mainCodeSourcesPath("checks/CredentialsProviderUnremovableCheckSampleMinimal.java"); + private static final CredentialsProviderUnremovableCheck CHECK = new CredentialsProviderUnremovableCheck(); + + @Test + void testMinimal() { + CheckVerifier.newVerifier() + .onFile(MINIMAL_FILE) + .withCheck(CHECK) + .verifyIssues(); + } + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(SAMPLE_FILE) + .withCheck(CHECK) + .verifyIssues(); + } + + @Test + void testWithoutSemantic() { + CheckVerifier.newVerifier() + .onFile(SAMPLE_FILE) + .withCheck(CHECK) + .withoutSemantic() + .verifyNoIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8912.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8912.html new file mode 100644 index 00000000000..35eff30d367 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8912.html @@ -0,0 +1,82 @@ +

This is an issue when a class implements a credential provider interface and is annotated with a dependency injection scope annotation (such as +application-scoped) but is missing the annotation that prevents removal during framework optimization.

+

In Java with CDI and Quarkus, this specifically refers to classes implementing the CredentialsProvider interface, annotated with +@ApplicationScoped (or other CDI scope annotations), and missing the @Unremovable annotation.

+

Why is this an issue?

+

In applications using frameworks with build-time optimization, the dependency injection container performs compile-time analysis to reduce the +runtime footprint. During this optimization phase, the container analyzes which components are actually used and may remove components that appear to +have no injection points or explicit references.

+

Custom implementations of service provider interfaces are discovered and used at runtime through the framework’s plugin or extension mechanism. +However, because these providers are typically not injected directly into other components, the dependency injection container cannot detect their +usage during build-time analysis.

+

Without explicit preservation annotations or configuration, the dependency injection container may incorrectly conclude that the custom service +provider component is unused and eliminate it during optimization. This leads to a situation where:

+
    +
  • The application builds successfully without warnings
    • +
  • The service provider component is not available at runtime
    • +
  • Authentication attempts fail when the application tries to retrieve credentials
    • +
  • The root cause is difficult to diagnose because the failure occurs at runtime rather than build time
    • +
+

Preservation annotations or configuration directives explicitly instruct the dependency injection container to retain the component during +optimization, ensuring it remains available for runtime discovery through the service provider interface mechanism.

+

What is the potential impact?

+

When a custom credentials provider implementation lacks annotations that prevent removal during build-time optimization, the dependency injection +framework may eliminate the implementation as apparently unused. This results in runtime authentication failures when the application attempts to +retrieve credentials from the provider.

+

The application will build successfully, making the issue particularly difficult to detect during development. The failure only manifests when the +credentials provider is actually invoked, which could be:

+
    +
  • During application startup when establishing database connections
    • +
  • When connecting to message brokers like RabbitMQ
    • +
  • When authenticating with OIDC or OAuth2 providers
    • +
  • At any point when the configured datasource or service attempts to authenticate
    • +
+

These runtime failures can cause:

+
    +
  • Service outages if the issue reaches production
    • +
  • Failed database connections preventing application functionality
    • +
  • Authentication errors that may be misattributed to credential validity rather than provider availability
    • +
  • Debugging complexity due to the indirect nature of the failure
    • +
+

How to fix it

+

Add the @Unremovable annotation to your custom CredentialsProvider implementation. This annotation must be imported from +io.quarkus.arc.Unremovable and placed alongside your CDI scope annotation (typically @ApplicationScoped).

+

Code examples

+

Noncompliant code example

+
+@ApplicationScoped
+public class MyCredentialsProvider implements CredentialsProvider { // Noncompliant
+
+    @Override
+    public Map<String, String> getCredentials(String credentialsProviderName) {
+        Map<String, String> properties = new HashMap<>();
+        properties.put(USER_PROPERTY_NAME, "user");
+        properties.put(PASSWORD_PROPERTY_NAME, "password");
+        return properties;
+    }
+}
+
+

Compliant solution

+
+@ApplicationScoped
+@Unremovable
+public class MyCredentialsProvider implements CredentialsProvider {
+
+    @Override
+    public Map<String, String> getCredentials(String credentialsProviderName) {
+        Map<String, String> properties = new HashMap<>();
+        properties.put(USER_PROPERTY_NAME, "user");
+        properties.put(PASSWORD_PROPERTY_NAME, "password");
+        return properties;
+    }
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8912.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8912.json new file mode 100644 index 00000000000..b2768b5ff85 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8912.json @@ -0,0 +1,26 @@ +{ + "title": "Custom CredentialsProvider implementations should be annotated with @Unremovable", + "type": "BUG", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5 min" + }, + "tags": [ + "quarkus", + "credentials", + "cdi" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-8912", + "sqKey": "S8912", + "scope": "All", + "quickfix": "targeted", + "code": { + "impacts": { + "SECURITY": "MEDIUM", + "RELIABILITY": "HIGH" + }, + "attribute": "COMPLETE" + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json index 27529004769..1d4f7e89006 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json @@ -534,6 +534,7 @@ "S8745", "S8786", "S8911", + "S8912", "S8924" ] }