Skip to content

ACOMMONS-153 Add jabber.org and apache.org XML feature URLs as safe cleartext identifiers - #511

Merged
daniel-teuchert-sonarsource merged 6 commits into
masterfrom
add-jabber-apache-safe-cleartext-urls
Sep 8, 2026
Merged

daniel-teuchert-sonarsource merged 6 commits into
masterfrom
add-jabber-apache-safe-cleartext-urls

Conversation

@daniel-teuchert-sonarsource

Copy link
Copy Markdown
Contributor

Part of

What

CleartextProtocolFilter now recognises a new category of well-known identifier URLs: specific http:// URLs (or URL prefixes) that are opaque protocol/feature identifiers rather than real network endpoints, on hosts that also serve ordinary content:

  • http://jabber.org/protocol/* — XMPP protocol namespaces (XEPs)
  • http://apache.org/xml/features/nonvalidating/load-dtd-grammar
  • http://apache.org/xml/features/nonvalidating/load-external-dtd
    (Xerces/JAXP XML parser features to disable DTD loading)

Why

These are used as opaque literal identifiers (XML namespace URIs / XML parser feature-flag URIs) in source code, never as actual network requests, so flagging them as insecure cleartext protocol usage is a false positive.

How

Unlike the existing NAMESPACE_URI_AUTHORITIES list (matched on host only), jabber.org and apache.org also serve ordinary content outside these specific paths, so a new SAFE_URL_PATH_PREFIXES pattern matches on host + path instead. Added positive and boundary-negative test cases (bare host, near-miss suffix, lookalike host) to CleartextProtocolFilterTest.

Testing

mvn -pl commons -am test -Dtest=CleartextProtocolFilterTest → 214 tests, 0 failures.

…tifiers

CleartextProtocolFilter now recognises a new category of well-known
identifier URLs: specific http:// URLs (or URL prefixes) that are
opaque protocol/feature identifiers rather than real network
endpoints, on hosts that also serve ordinary content:

- http://jabber.org/protocol/* (XMPP protocol namespaces)
- http://apache.org/xml/features/nonvalidating/load-dtd-grammar
- http://apache.org/xml/features/nonvalidating/load-external-dtd
  (Xerces/JAXP XML parser features to disable DTD loading)

Unlike the existing namespace-URI-authority list, these are matched
on host + path (not host alone), since jabber.org and apache.org
serve ordinary content outside these specific paths.
@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod Bot changed the title Add jabber.org and apache.org XML feature URLs as safe cleartext identifiers ACOMMONS-153 Add jabber.org and apache.org XML feature URLs as safe cleartext identifiers Sep 7, 2026
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

ACOMMONS-153

@datadog-sonarsource

This comment has been minimized.

- SAFE_URL_PATH_PREFIXES now matches the whole apache.org/xml/features/
  prefix instead of only the two DTD flags, matching the javadoc and
  covering other real Xerces feature flags used in this repo (e.g.
  xml-parsing's dom/create-entity-ref-nodes).
- isSafeWithoutTls(String)'s lenient fallback (used when strict URI
  parsing fails on templated input like ${node}) now also checks
  isKnownIdentifierUrl, so templated jabber.org/apache.org identifier
  URLs are recognised as safe just like well-formed ones.
@gitar-bot

gitar-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Adds jabber.org/protocol/* and apache.org/xml/features/* as safe cleartext identifiers to prevent false positives on opaque protocol/feature URIs that are never used as actual network requests. Resolved overly broad Javadoc and lenient fallback path that bypassed the new host+path identifier check.

✅ 2 resolved
✅ Quality: Javadoc promises all apache.org/xml/features/ flags; regex allows 2

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:50-54 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:169-175
The new javadoc bullet (lines 50-54) tells callers that "Xerces/JAXP XML parser feature flags under apache.org/xml/features/" are treated as safe, but SAFE_URL_PATH_PREFIXES only whitelists the two exact paths .../nonvalidating/load-dtd-grammar and .../nonvalidating/load-external-dtd. Concrete inputs that the doc implies are safe but that still return false: "http://apache.org/xml/features/disallow-doctype-decl" and "http://apache.org/xml/features/dom/create-entity-ref-nodes" — the latter is used in this very repo (xml-parsing/src/main/java/org/sonarsource/analyzer/commons/xml/SafeDomParserFactory.java), i.e. exactly the opaque-feature-flag false positive the PR sets out to remove. Either broaden the pattern to the whole apache.org/xml/features/ prefix (the same reasoning applies to every flag under it, and none is a real endpoint) or narrow the javadoc to name only the two DTD-loading flags.

✅ Edge Case: Lenient fallback path ignores the new host+path identifier check

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:279-284 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:302 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/CleartextProtocolFilter.java:332-335
isSafeWithoutTls(String) only reaches isKnownIdentifierUrl through the strict-URI branch; when java.net.URI rejects the string it falls back to isSafeHost(rest), which does not know about SAFE_URL_PATH_PREFIXES. Concrete trigger: isSafeWithoutTls("http://jabber.org/protocol/${node}") throws URISyntaxException on '{', falls back, and returns false, while the well-formed "http://jabber.org/protocol/muc" returns true — so templated occurrences of these identifiers stay false positives. Extend the fallback to pass the leading path segment to isKnownIdentifierUrl (or document the gap, as line 230 of the test does for the single-label case).

Options

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

Comment with these commands to change the behavior for this request:

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

Was this helpful? React with 👍 / 👎 | Gitar

path is never null: URI.getRawPath() is non-null whenever getHost()
is non-null (both call sites only invoke this after a host != null
check), and the lenient-fallback caller derives path via
String.split(), which never returns null.
@sonarqube-next

sonarqube-next Bot commented Sep 8, 2026

Copy link
Copy Markdown

@daniel-teuchert-sonarsource
daniel-teuchert-sonarsource merged commit 9ceb861 into master Sep 8, 2026
8 checks passed
@daniel-teuchert-sonarsource
daniel-teuchert-sonarsource deleted the add-jabber-apache-safe-cleartext-urls branch September 8, 2026 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants