Skip to content

ACOMMONS-231 Add IpAddressCandidateExtractor.extractAll to find embedded IP address candidates - #596

Closed
erwan-leforestier-sonarsource wants to merge 6 commits into
masterfrom
dev/ai-experiment/ACOMMONS-231-ip-address-heuristics-claude-a01-1
Closed

erwan-leforestier-sonarsource wants to merge 6 commits into
masterfrom
dev/ai-experiment/ACOMMONS-231-ip-address-heuristics-claude-a01-1

Conversation

@erwan-leforestier-sonarsource

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

Copy link
Copy Markdown

ACOMMONS-231

Problem

IpAddressCandidateExtractor.extract interprets a whole literal as a single address, endpoint, URL, or list and returns one candidate from its beginning. Addresses later in the string are missed, e.g. "/registry/ipaddresses/192.168.2.3" (Kubernetes), "connect to 10.0.0.1", or every entry after the first in "10.0.0.1;10.0.0.2".

Contract decision

  • extract(String) is unchanged. It is public API for consumers that expect the whole literal to be one address (e.g. firewall-rule values) and rely on malformed content staying observable as an unparseable candidate. Changing it to search would hide that.
  • New extractAll(String): List<String> scans the whole content and returns all address-shaped candidates in order of appearance (duplicates retained, unmodifiable list).
    • Candidates are dotted quads of 1–3 ASCII digits, or IPv6 shapes (hex groups, at most one ::, 8 groups or fewer with ::, optional dotted IPv4 tail, optional zone). They are not range-validated (999.1.1.1, /33 are returned, like extract), so consumers still call IpAddressClassifier.parse.
    • Brackets and ports are removed. Bare 1–3 digit slash suffixes are kept as CIDRs, except after a port or on URL hosts (//host, user@host), where they are paths. This matches extract.
    • Precision heuristics: tokens adjacent to letters, digits, _ or -, or inside longer dotted or colon sequences, are ignored (v1.2.3.4, OIDs 1.3.6.1.4.1, host-1.2.3.4, times 12:30:45, MAC addresses, std::vector). Addresses in URL user information (http://192.0.2.8:pass@host/..., //user:1.2.3.4@host) or directly followed by @ are ignored. The URL host, path, query, and fragment are still searched. User-Agent product versions such as Chrome/120.0.0.0 are also ignored. A trailing full stop or single colon (punctuation) ends a candidate. Dotted names such as 1.2.0.192.in-addr.arpa are ignored. Boundaries are Unicode-aware, including supplementary-plane letters.
    • Scanning is linear in the content length: each hex/colon/dot run is processed once.
    • Regression test: when extract yields a parseable address, extractAll contains it.

Known limitations (documented)

  • Dotted four-part version numbers in free text (version 1.0.0.0) are returned. Consumers classify them like any literal.
  • Hyphenated ranges (10.0.0.1-10.0.0.9) yield no candidate, because hyphens join tokens. Ranges with spaces work.
  • A single-segment name/1.2.3.4 at the start of the content or after whitespace is treated as a product token and skipped.

Tests

mvn clean install -Pcoverage passes, including enforce-plugin-size. The Commons JAR is 186,408 bytes at base and 189,601 bytes on this branch (max 190,000). The extractor has 100% line coverage. A 100k-address regression test guards against quadratic scanning without asserting on timing.

🤖 Generated with Claude Code

…ded IP address candidates

`extract` interprets the whole literal as one address, endpoint, URL, or list and
returns only the first candidate from its beginning, so addresses in path segments
or surrounding text, such as "/registry/ipaddresses/192.168.2.3", are missed.

`extractAll` scans the whole content for address-shaped tokens and returns every
candidate in order of appearance. It strips brackets and ports, preserves zones and
bare CIDR suffixes, treats URL hosts' slash suffixes as paths, and skips tokens
embedded in words, dotted sequences (OIDs), hyphenated names, user information,
and User-Agent product versions such as "Chrome/120.0.0.0".

`extract` keeps its existing behavior.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@hashicorp-vault-sonar-prod

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

Copy link
Copy Markdown
Contributor

ACOMMONS-231

Addresses inside "//userinfo@" spans, such as "192.0.2.8" in
"http://192.0.2.8:password@example.org/path/10.0.0.1", are credentials rather
than hosts. The URL host, path, query, and fragment are still searched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the lookaround-heavy regular expression (complexity 46) with a scan of
hex/colon/dot runs whose boundaries and shapes are checked in small helpers,
and split complex methods. Reverse DNS names such as "1.2.0.192.in-addr.arpa"
remain excluded, and the jar stays within the enforced size limit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
addIpv4Candidates searched the rest of the content for the next colon for every
address-shaped run, making extraction quadratic for many colon-free addresses.
Scan each run's characters once instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…etters before runs

An IPv6 address followed by a single colon used as punctuation, as in
"Server ::1: connection refused", is now found, like its IPv4 counterpart.
The left boundary decodes the full code point before a run, so a
supplementary-plane letter, as in "𠀋192.0.2.1", embeds the address in a word.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tive complexity

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@gitar-bot

gitar-bot Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 2 closed / 3 findings

🟡 Medium risk · The new extractAll heuristic returns embedded address-shaped tokens without validation, so consumers could classify version numbers as IP candidates.

Adds IpAddressCandidateExtractor.extractAll to return all embedded IP address candidates in order, leaving extract unchanged. Two earlier findings are resolved: IPv6 addresses followed by a colon and supplementary-plane letters before a run. Open suggestion: the combined Commons JAR may exceed the 190,000-byte enforcer limit once this PR lands alongside #593, so the size should be verified after rebasing.

💡 Cross-PR: Combined Commons JAR likely exceeds the 190,000-byte enforcer limit · #593

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:153

Breaks when this PR and #593 (together with its base stack) both land on master, and mvn verify runs the enforce-plugin-size rule on the Commons JAR.

How

  1. commons/pom.xml fails the build when the Commons JAR exceeds requireFilesSize <maxsize>190000</maxsize>.
  2. #593 reports a 188,114-byte JAR with its identifier inventory in CleartextProtocolFilter.java:230, which leaves 1,886 bytes of headroom.
  3. This PR adds about 180 lines of compiled logic to IpAddressCandidateExtractor.java:153-326: extractAll, around 15 private helpers and 5 new Pattern constants. That is probably more than 1,886 bytes after compression. I have not measured it, so this is an estimate.

Fix

  • This PR: Report the Commons JAR size from mvn verify, rebased onto #593's stack. If the combined JAR goes over the limit, trim the implementation or raise maxsize on purpose.
  • #593: Re-run the size check after rebasing onto whichever change merges first, and coordinate any maxsize increase with this PR.

Compared d62aa60 with #593 at e549e46

✅ 2 closed
✅ Edge Case: IPv6 address followed by a colon yields no candidate

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:162-176 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:245-259
Trailing dots are trimmed from a run (lines 162-165), but trailing colons are not. In "Server 2001:db8::1: connection refused" the run is 2001:db8::1:. isIpv6Shaped splits the tail 1: into ["1", ""], countHexGroups returns -1, and the shape check fails. The IPv4 fallback finds nothing because the run has no dots. So the IPv6 address is missed, while the matching IPv4 case "Server 10.0.0.1: connection refused" is found, as one of the tests expects. The same happens to ::1: or fe80::1: used as sentence punctuation. Fix: when the IPv6 shape check fails, retry it with one trailing : removed, provided the remaining text does not end in ::, and add a test for it.

✅ Edge Case: Left boundary check ignores supplementary-plane letters

📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:170 📄 commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:311-321
isBoundary(content, start - 1) calls isWordCharacter, which uses content.codePointAt(index). If the character before the run is a supplementary letter, such as CJK Extension B 𠀋 or a mathematical alphanumeric, then start - 1 points at the low surrogate. codePointAt then returns the lone surrogate, and Character.isLetterOrDigit reports false for it. As a result "𠀋192.0.2.1" produces a candidate, while the BMP case "接口192.0.2.1" (tested) correctly produces none. The right-side check works because codePointAt on a high surrogate returns the full code point. Fix: use Character.codePointBefore(content, start) for the preceding-character check.

🤖 Prompt for agents
Code Review: Adds `IpAddressCandidateExtractor.extractAll` to return all embedded IP address candidates in order, leaving `extract` unchanged. Two earlier findings are resolved: IPv6 addresses followed by a colon and supplementary-plane letters before a run. Open suggestion: the combined Commons JAR may exceed the 190,000-byte enforcer limit once this PR lands alongside #593, so the size should be verified after rebasing.

1. 💡 Cross-PR: Combined Commons JAR likely exceeds the 190,000-byte enforcer limit · [#593](<https://github.com/SonarSource/sonar-analyzer-commons/pull/593>)
   Files: commons/src/main/java/org/sonarsource/analyzer/commons/appsec/IpAddressCandidateExtractor.java:153

   **Breaks when** this PR and [#593](<https://github.com/SonarSource/sonar-analyzer-commons/pull/593>) (together with its base stack) both land on master, and `mvn verify` runs the `enforce-plugin-size` rule on the Commons JAR.
   
   **How**
   
   1. `commons/pom.xml` fails the build when the Commons JAR exceeds `requireFilesSize` `<maxsize>190000</maxsize>`.
   2. [#593](<https://github.com/SonarSource/sonar-analyzer-commons/pull/593>) reports a 188,114-byte JAR with its identifier inventory in `CleartextProtocolFilter.java:230`, which leaves 1,886 bytes of headroom.
   3. This PR adds about 180 lines of compiled logic to `IpAddressCandidateExtractor.java:153-326`: `extractAll`, around 15 private helpers and 5 new `Pattern` constants. That is probably more than 1,886 bytes after compression. I have not measured it, so this is an estimate.
   
   **Fix**
   
   - **This PR:** Report the Commons JAR size from `mvn verify`, rebased onto [#593](<https://github.com/SonarSource/sonar-analyzer-commons/pull/593>)'s stack. If the combined JAR goes over the limit, trim the implementation or raise `maxsize` on purpose.
   - **[#593](<https://github.com/SonarSource/sonar-analyzer-commons/pull/593>):** Re-run the size check after rebasing onto whichever change merges first, and coordinate any `maxsize` increase with this PR.
   
   _Compared `d62aa60` with [#593](<https://github.com/SonarSource/sonar-analyzer-commons/pull/593>) at `e549e46`_

Verification Rules 2/2 met Repos 11 inspected · PRs 1 inspected

Functional validation No results

Rules 2 passed 1 not applicable

✅ Internal artifact registries
✅ Repository license compliance

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

SonarSource/sonarlint-omnisharp Inspected
SonarSource/sonar-java Inspected
SonarSource/sonar-dotnet Inspected
SonarSource/sonar-xml Inspected
SonarSource/sonar-php Inspected

6 more repositories

Cross-PR 1 PR inspected 1 open impact

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 5, 2026

Copy link
Copy Markdown

@erwan-leforestier-sonarsource
erwan-leforestier-sonarsource deleted the dev/ai-experiment/ACOMMONS-231-ip-address-heuristics-claude-a01-1 branch November 5, 2026 16:34
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