[SONARJAVA-6484] Implement new rule S8899

[romainbrenguier]

This rule detects a critical security vulnerability where HTTP request paths are used in security checks without normalizing multiple consecutive slashes. Attackers can bypass authentication and authorization by inserting extra slashes in URLs (e.g., //admin instead of /admin). The rule flags path-based security checks that lack proper normalization via replaceAll("/+", "/") and applies to both javax.servlet and jakarta.servlet frameworks, as well as JAX-RS UriInfo.

Part of

---

Summary by Gitar

• New Sonar Rule:
  • Implemented S8899 to detect unnormalized HTTP request paths in security checks.
  • Added full rule specification, including metadata, HTML documentation, and test samples covering various frameworks.
• Project Configuration:
  • Added .cursor/rules/sonar_code_context.mdc and .sonar-code-context/settings.json to manage development quality standards.
  • Updated .gitignore to track critical Sonar configuration while excluding transient files.
  • Added java-S8899.json tracking files to multiple test projects in its/ruling/.
• Refactoring:
  • Extracted logic into updatePathStateFromMethodInvocation helper within RequestPathNormalizationCheck.java.
• Environment Updates:
  • Updated mise.toml to lock Java to version 21.
  • Integrated rule into Sonar_agentic_AI_profile.json and Sonar_way_profile.json and adjusted AutoScanTest expectations.

_{This will update automatically on new commits.}

[hashicorp-vault-sonar-prod]

SONARJAVA-6484

[romainbrenguier]

This should be a separate PR

[alex-meseldzija-sonarsource]

This rule is useful, but it is a security rule.
I think this means we need to get sign off/reviews from the security domain.
WDYT?

[romainbrenguier]

This rule is useful, but it is a security rule. I think this means we need to get sign off/reviews from the security domain. WDYT?

This makes sense. I'll #ask-security-domain

[gitar-bot]

Code Review ✅ Approved 11 resolved / 11 findings

Implements Sonar rule S8899 to detect unnormalized HTTP request paths in security checks, resolving all identified implementation gaps including regex mismatches, dead code, and inaccurate path detection logic.

✅ 11 resolved

✅ Quality: Unrelated local tooling/config files committed in this PR

📄 .cursor/rules/sonar_code_context.mdc:1-15 📄 .sonar-code-context/settings.json:1-15 📄 .mvn/.gradle-enterprise/gradle-enterprise-workspace-id:1 📄 .gitignore:42-46
This feature PR also adds files that appear unrelated to rule S8899 and look like local developer/tooling artifacts: .cursor/rules/sonar_code_context.mdc, .sonar-code-context/settings.json, and .mvn/.gradle-enterprise/gradle-enterprise-workspace-id (a machine-specific workspace identifier). The .gitignore change is also crafted specifically to force-track settings.json while ignoring the rest of .sonar-code-context/. Committing per-developer tooling configuration and workspace IDs into a shared repository is usually undesirable and pollutes the history. Recommend removing these from the PR (or moving them to a separate, intentional change) so the PR contains only the new rule and its resources.

✅ Edge Case: Over-broad security-context detection causes many false positives

📄 java-checks/src/main/java/org/sonar/java/checks/security/RequestPathNormalizationCheck.java:69-73 📄 java-checks/src/main/java/org/sonar/java/checks/security/RequestPathNormalizationCheck.java:147-161
isInSecurityContext treats any enclosing if, ternary, return, throw, &&, or || as a security check, and SECURITY_CHECK_METHODS includes very common String operations (contains, indexOf, equals, endsWith, startsWith, matches). Combined, this flags virtually every conditional that inspects a request path, even for legitimate non-security purposes such as routing, feature toggles, content negotiation, redirect handling, or logging guards (e.g. if (request.getRequestURI().endsWith(".css")) ...). For a Critical VULNERABILITY rule this high false-positive rate will erode trust and bury real issues.

Consider narrowing the heuristic: require correlation with an actual authorization/authentication action in the guarded block (e.g. a throw of a Forbidden/Unauthorized exception, a checkPermission-style call, or returning a 401/403), and/or restrict the matched String methods to comparison-style operations most associated with access decisions. At minimum, document the precision trade-off and add negative test cases (e.g. an endsWith(".css") static-resource check) to the sample file.

✅ Edge Case: Normalization detection too narrow; misses common forms

📄 java-checks/src/main/java/org/sonar/java/checks/security/RequestPathNormalizationCheck.java:63-67 📄 java-checks/src/main/java/org/sonar/java/checks/security/RequestPathNormalizationCheck.java:185-195
Normalization is only recognized when String.replaceAll is called with a string literal containing the substring /+ or //+. This produces false positives for equally valid normalizations the rule will not recognize, e.g. replaceAll("/{2,}", "/"), a pre-compiled Pattern.matcher(path).replaceAll("/"), URI.create(path).normalize(), or framework canonicalization helpers. A user who normalizes correctly via any of these will still be flagged. Conversely, matching merely on the literal containing /+ (e.g. replaceAll("foo/+bar", "x")) can be mistaken for path normalization. Consider broadening the recognized normalization patterns (or at least documenting the supported form in S8899.html) and adding sample cases for the unsupported variants.

✅ Quality: Dead state: normalizedVariables set is written but never read

📄 java-checks/src/main/java/org/sonar/java/checks/security/RequestPathNormalizationCheck.java:21 📄 java-checks/src/main/java/org/sonar/java/checks/security/RequestPathNormalizationCheck.java:76 📄 java-checks/src/main/java/org/sonar/java/checks/security/RequestPathNormalizationCheck.java:86 📄 java-checks/src/main/java/org/sonar/java/checks/security/RequestPathNormalizationCheck.java:122 📄 java-checks/src/main/java/org/sonar/java/checks/security/RequestPathNormalizationCheck.java:140
The normalizedVariables Set<Symbol> (and its HashSet/Set imports) is populated in handleVariableDeclaration/handleAssignment and cleared in setContext, but it is never queried anywhere. All normalization tracking is actually done through pathVariables with PathState.NORMALIZED. The set is redundant dead state and should be removed to avoid confusion.

✅ Quality: Internal AI analysis doc committed to repo

📄 .agents/RIS_PREPROCESSING.md:1-15
This commit adds .agents/RIS_PREPROCESSING.md, a 479-line internal analysis of the rule engine ('SonarQube Java Analyzer - Existing Rule Engine Analysis'). This is preprocessing/scratch documentation, not part of the S8899 rule feature, and likely should not be committed to the repository. It mirrors the previously resolved finding about unrelated local tooling/config files (.cursor/rules/sonar_code_context.mdc) being committed in this PR. Consider removing it from the PR (or adding .agents/ to .gitignore if these files are meant to stay local).

...and 6 more resolved from earlier reviews

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

[romainbrenguier]

I'm withdrawing this rule for now, since it falls under the security domain.