Skip to content

ACOMMONS-231 Find IP addresses throughout string literals - #605

Closed
erwan-leforestier-sonarsource wants to merge 6 commits into
masterfrom
dev/ai-experiment/ACOMMONS-231-implementation-codex-a01-p20-1
Closed

erwan-leforestier-sonarsource wants to merge 6 commits into
masterfrom
dev/ai-experiment/ACOMMONS-231-implementation-codex-a01-p20-1

Conversation

@erwan-leforestier-sonarsource

@erwan-leforestier-sonarsource erwan-leforestier-sonarsource commented Nov 6, 2026 •

Copy link
Copy Markdown

Add IpAddressCandidateExtractor.extractAll(String) to find validated IP address literals throughout text, paths, lists, and URL components. Results preserve source order, duplicate occurrences, address spelling, zones, and valid standalone CIDR suffixes; endpoint brackets and valid ports are removed. URL schemes and authority credentials are excluded, and malformed entries cannot yield shorter valid addresses.

The existing extract(String) operation and address classification remain unchanged. Tests cover the Kubernetes storage-path example, multiple addresses, URL components and credentials, endpoint validation, CIDR/path distinctions, recovery after malformed entries, bounded CIDR suffixes before later values, punctuation enclosing URL hosts, wrapped IPv6 URLs with paths/queries/fragments, bracketed zones in URLs and text, and malformed port rejection.

Validation: the pinned baseline passes mvn clean install (1,626 tests). The implementation passes the focused extractor/classifier suite (647 tests) and full mvn -Pcoverage clean install (1,854 tests), including the library size check. All Maven commands used the required build resource gate.

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

ACOMMONS-231

@gitar-bot

gitar-bot Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Code Review ✅ Approved 6 closed / 6 findings

🟡 Medium risk · The new extractAll scanner parses addresses across text and URLs; edge-case misparsing could omit literals or return incorrect candidates.

Adds IpAddressCandidateExtractor.extractAll(String) to find validated IP address literals throughout text, paths, lists, and URL components, preserving source order, duplicates, zones, and standalone CIDR suffixes. Resolved fixes include CIDR suffixes no longer swallowing later list entries, URL hosts followed by punctuation, bracketed IPv6 URL hosts with paths and zoned hosts, and bracket-bounded zones in URL hosts. No open issues found.

✅ 6 closed
✅ Edge Case: CIDR suffix swallows rest of list entry, dropping later addresses

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:179-184 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:189-200
In extractListEntry, once isStandalonePrefix passes, the code appends everything after the slash to the candidate (candidate += text.substring(tokens.end())) and then returns. isStandalonePrefix only checks that the suffix starts with a digit or sign. If a CIDR is followed by a character that ADDRESS_TOKEN treats as a separator but LIST_ENTRY/ENTRY do not (|, &, !, <, @, ...), the CIDR and every later address in that entry are lost.

Trace for 10.0.0.0/8|10.0.0.2: the token is 10.0.0.0. isStandalonePrefix sees suffix 8|10.0.0.2, which does not parse as an address and starts with 8, so it returns true. The candidate becomes 10.0.0.0/8|10.0.0.2, parsePrefix rejects 8|10.0.0.2 via CIDR_PREFIX_PATTERN, nothing is added, and the method returns. The result is empty, but 10.0.0.1|10.0.0.2 returns both addresses. The same happens with allow=10.0.0.0/8&gw=10.0.0.2 in non-URL text, and with 10.0.0.1/24! (yet 10.0.0.1! returns 10.0.0.1).

Fix: consume only the run of prefix-like characters after the slash (e.g. [\p{L}\p{N}_.+-]+, matching how other tokens end). Validate that run as the CIDR, then resume token matching after it instead of returning.

✅ Edge Case: URL host followed by punctuation is dropped, unlike bare addresses

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:142-154
In extractEntry, the URL authority runs until /, ? or #, and the host runs until , or ;. The host string is passed to stripBracketsAndPort/addValid as is, without the ADDRESS_TOKEN tokenization used for other text. So any trailing punctuation that ENTRY does not split on stays attached to the host and makes it invalid.

Examples: <http://10.0.0.1> (autolink or HTML-style text), http://10.0.0.1!, and [http://10.0.0.1] all return nothing. Meanwhile <10.0.0.1> and 10.0.0.1! (the latter is an explicit test case) each return 10.0.0.1. The scanner then sets start past the host, so that text is never searched again.

Fix: end the host at the first character outside the host/endpoint character set (ADDRESS_TOKEN plus % zones and @), or pass a host that fails validation to extractListEntry with inUrl=true.

✅ Bug: Enclosing '[' check drops bracketed IPv6 hosts in URLs with a path

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:158-162
The new check at L159 removes a trailing ] from the host whenever the URL comes right after [. It doesn't check whether that ] actually closes the outer bracket. If a bracket-enclosed URL has a path, query, or fragment, the authority ends at /, ? or #. The host is then just [::1], its only ] gets stripped, and stripBracketsAndPort("[::1") returns empty. I ran the extractor to confirm: [http://[::1]/], [http://[2001:db8::1]/path] and [http://[::1]?q=1] all return [], while [http://[::1]:80/x] and the unbracketed http://[2001:db8::1]/path both find the address. Before this commit the host was found in these inputs, so this is a regression. Fix: only strip the trailing ] when the host contains more ] than [.

✅ Edge Case: Zoned bracketed IPv6 URL hosts still dropped when followed by punctuation

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:38 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:153-156 📄 commons/src/test/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractorAllTest.java:198-211
The new ADDRESS_TOKEN host bound fixes trailing punctuation for http://[2001:db8::1]:80!, but not for zoned hosts. The zone part %[^,;/?#=&]* keeps matching through ], :, ! and >. So in http://[fe80::1%eth0]! the host token becomes [fe80::1%eth0]!. stripBracketsAndPort then sees ! where a port separator should follow the closing bracket, and rejects the host. I ran it to confirm: http://[fe80::1%eth0]!, <http://[fe80::1%eth0]> and <http://[fe80::1%eth0]:80> all return [], while http://[fe80::1%eth0]:80 returns fe80::1%eth0. The only zoned case the new test covers is [http://[fe80::1%eth0]]. Inside brackets, the closing ] already delimits the zone, so this doesn't conflict with the opaque zone contract for unbracketed zones. Fix: when the host starts with [, end the host token at the closing ] plus an optional :digits port.

✅ Edge Case: Closing bracket bounds zones in URL hosts only, not in bare text

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:38 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:185-199 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:215-224
This commit makes urlHostEnd treat the endpoint's closing ] as the end of an opaque zone. That is why http://[fe80::1%eth0]!, <http://[fe80::1%eth0]> and http://[fe80::1%eth0]:80! now return fe80::1%eth0. Text outside a URL still goes through extractListEntry, which uses ADDRESS_TOKEN, and that zone part %[^,;/?#=&]* keeps going past ].

So <[fe80::1%eth0]> gives the token [fe80::1%eth0]>, and [fe80::1%eth0]:80! gives [fe80::1%eth0]:80! with the port 80!. In both cases stripBracketsAndPort rejects the token and the address is dropped. The non-zoned versions <[2001:db8::1]> and [2001:db8::1]:80! are still found, because ] and ! stop the token there. Bracketed zoned endpoints written outside a URL are therefore silently missed, while the same endpoint inside a URL is found.

Fix: apply the same closing-bracket bound when scanning tokens in extractListEntry. When a token starts with [, end the zone at the first ] and then match only the port or suffix after it. Unbracketed zones stay opaque, so cases like fe80::1%eth>0 keep their spelling.

...and 1 more closed from earlier reviews

Verification Rules 2/2 met Knowledge 🔎 1 Repos 10 inspected · PRs 1 inspected

Functional validation No results

Rules 2 passed 1 not applicable

✅ Internal artifact registries
✅ Repository license compliance

Knowledge 🔎 1 informed a finding

🔎 IPv6 zone suffixes remain opaque informed a finding, resolved -> Zoned bracketed IPv6 URL hosts still dropped when followed by punctuation

Broader impact Repos 10 inspected PRs 1 inspected
Cross-repo 10 repositories inspected 0 open impacts

SonarSource/sonar-html Inspected
SonarSource/SonarJS Inspected
SonarSource/sonar-flex Inspected
SonarSource/sonar-xml Inspected
SonarSource/sonar-python Inspected

5 more repositories

Cross-PR 1 PR inspected 0 open impacts

PR 593: ACOMMONS-241 Add the approved identifier URL inventory and matcher Inspected

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Options

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

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

@sonarqube-next

sonarqube-next Bot commented Nov 6, 2026

Copy link
Copy Markdown

@datadog-sonarsource

datadog-sonarsource Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Pipelines

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 19608cd | Docs | View more details | Give us feedback!

@erwan-leforestier-sonarsource
erwan-leforestier-sonarsource deleted the dev/ai-experiment/ACOMMONS-231-implementation-codex-a01-p20-1 branch November 6, 2026 12:26
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.

1 participant