SONARJAVA-6375 S3752: Allow explicit unsafe HTTP methods and change from hotspot to vulnerability

[daniel-teuchert-sonarsource]

We are converting this rule to a vulnerability. To make sure that this rule brings the most value, we decided to stop raising when HTTP verbs are explicitly allowed and only raise when implicitly all verbs are allowed for an endpoint.

[sonarqubecloud]

Agentic Analysis: Early Results

Agentic Analysis and Context Augmentation are available on your project. Here are some issues that could have been prevented. Follow the links to learn how to put them into action.

10 issue(s) found across 1 file(s):

Rule	File	Line	Message
java:S139	java-checks-test-sources/default/src/main/java/checks/spring/SpringRequestMappingMethodCheckSample.java	35	Move this trailing comment on the previous empty line.
java:S139	java-checks-test-sources/default/src/main/java/checks/spring/SpringRequestMappingMethodCheckSample.java	50	Move this trailing comment on the previous empty line.
java:S139	java-checks-test-sources/default/src/main/java/checks/spring/SpringRequestMappingMethodCheckSample.java	67	Move this trailing comment on the previous empty line.
java:S139	java-checks-test-sources/default/src/main/java/checks/spring/SpringRequestMappingMethodCheckSample.java	72	Move this trailing comment on the previous empty line.
java:S139	java-checks-test-sources/default/src/main/java/checks/spring/SpringRequestMappingMethodCheckSample.java	77	Move this trailing comment on the previous empty line.
java:S139	java-checks-test-sources/default/src/main/java/checks/spring/SpringRequestMappingMethodCheckSample.java	86	Move this trailing comment on the previous empty line.
java:S139	java-checks-test-sources/default/src/main/java/checks/spring/SpringRequestMappingMethodCheckSample.java	91	Move this trailing comment on the previous empty line.
java:S125	java-checks-test-sources/default/src/main/java/checks/spring/SpringRequestMappingMethodCheckSample.java	131	This block of commented-out lines of code should be removed.
java:S139	java-checks-test-sources/default/src/main/java/checks/spring/SpringRequestMappingMethodCheckSample.java	131	Move this trailing comment on the previous empty line.
java:S139	java-checks-test-sources/default/src/main/java/checks/spring/SpringRequestMappingMethodCheckSample.java	145	Move this trailing comment on the previous empty line.

_{Analyzed by SonarQube Agentic Analysis in 10.7 s}

[hashicorp-vault-sonar-prod]

SONARJAVA-6375

[sonar-review-alpha]

Summary

This PR converts rule S3752 from a security hotspot to a vulnerability and fundamentally changes what the rule detects.

What changed:

• Rule type: SECURITY_HOTSPOT → VULNERABILITY (in rule metadata)
• Detection logic: Now only flags @RequestMapping endpoints with no explicit HTTP methods, rather than flagging endpoints that mix safe and unsafe methods together
• Message: Changed from "Make sure allowing safe and unsafe HTTP methods is safe here" to "Explicitly specify the HTTP methods this endpoint accepts"
• Exclusion: ErrorController implementations are now exempt from the rule

Why it changed:
When HTTP methods are explicitly specified (even if mixing safe and unsafe ones), the developer has made an intentional choice. The vulnerability is when methods are implicitly unspecified—the endpoint accepts all HTTP methods by default, which can enable CSRF attacks. This narrower scope reduces false positives and focuses on the actual vulnerability.

Impact:

• The complex logic for detecting mixed safe/unsafe method sets was removed (deleted HttpMethodVisitor visitor class)
• False negatives reduced from 31 to 24, indicating better accuracy
• Documentation completely rewritten to focus on CSRF exploitation rather than safe/unsafe method mixing
• Several test cases that were previously flagged (with explicit mixed methods) are now compliant

What reviewers should know

Start by reviewing:

1. The logic change in SpringRequestMappingMethodCheck.java (lines 50-68) — the detection now simply checks if @RequestMapping lacks the method parameter
2. The updated error message and rule metadata in S3752.json and S3752.html

Key points for reviewers:

• The old logic tracked safe vs unsafe methods separately at class and method level; this is completely gone. Make sure you agree with the simpler approach.
• The findRequestMethods() method still exists and is used to check if methods are present, but the complex visitor-based analysis is deleted
• The inheritRequestMethod() check remains to handle inherited @RequestMapping annotations at the class level
• ErrorController is explicitly excluded (line 52-54) — verify this is the right exception
• Test coverage shows the expected behavior: explicit methods = compliant, no methods = noncompliant (see test file changes)

---

• Generate Walkthrough
• Generate Diagram

🗣️ Give feedback

[dorian-burihabwa-sonarsource]

(commenting to avoid blocking the work)The change looks good to me, however I think we should probably update the rule description as the scope of findings has changed now (regardless of whether the rule should be a hotspot or a vulnerability). I would like to avoid merging the code now and releasing with an inconsistent implementation and description.

[daniel-teuchert-sonarsource]

(commenting to avoid blocking the work)The change looks good to me, however I think we should probably update the rule description as the scope of findings has changed now (regardless of whether the rule should be a hotspot or a vulnerability). I would like to avoid merging the code now and releasing with an inconsistent implementation and description.

The RSPEC update has been merged: https://github.com/SonarSource/rspec/pull/6822

[daniel-teuchert-sonarsource]

The RSPEC changes are now also added to this PR.

[sonar-review-alpha]

The documentation update is well-structured and the metadata changes are appropriate. One documentation gap worth discussing: the HTML docs don't mention the ErrorController exemption or the class-level inheritance behaviour (inheritRequestMethod), which could confuse users who hit those edge cases.

🗣️ Give feedback

[sonar-review-alpha]

The documentation examples only cover the simplest case (no method attribute at all). Two real-world scenarios that users will encounter are not mentioned:

1. Class-level inheritance — if a @RequestMapping on the class specifies method, a method-level @RequestMapping without method is compliant because it inherits from the class. A user who sees the rule fire on their method-level annotation may be confused if they don't know this, or they may not realise they don't need to repeat the method on every handler.
2. ErrorController exemption — the rule silently skips any class implementing ErrorController. Users who happen to land on this documentation while investigating why their ErrorController is (or isn't) flagged will find no explanation.

Consider adding a short note after the compliant example, e.g.:

<p>Routes that inherit an explicit HTTP method from a class-level <code>@RequestMapping</code> are also compliant.
Endpoints in classes that implement <code>ErrorController</code> are excluded from this rule.</p>

[daniel-teuchert-sonarsource]

I think those examples are a bit too specific for the RSPEC, also this would need to be another RSPEC PR so I will keep it as it is for this PR.

[sonar-review-alpha]

The severity is Minor for a rule now typed as VULNERABILITY. CSRF is typically classified as at least Medium in most security frameworks (OWASP rates it as a meaningful risk). Is Minor intentional here, or should it be raised to Medium/Major to match the threat level described in the updated HTML documentation?

[daniel-teuchert-sonarsource]

It is intentional. This used to be a security hotspot so we should not have a too high impact.

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[dorian-burihabwa-sonarsource]

LGTM 👍🏿 Thanks for including the changes to the rule description

For merging, you could go 1 of 2 ways:

1. Rework the changes into 2 commits:
   a. SONARJAVA-6375 S3752: Allow explicit unsafe HTTP methods
   b. SONARJAVA-6375 S3752: Change from hotspot to vulnerability
2. Changing the title of the PR for something a little more explicit like SONARJAVA-6375 S3752: Allow explicit unsafe HTTP methods and change from hotspot to vulnerability