Skip to content

Fix project-mode Maven crawl listing all of ~/.m2 (#265) - #1361

Open
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/v5-maven-crawl-scope
Open

Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/v5-maven-crawl-scope

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #265

Summary

A project-mode scan of a Maven project no longer reports everything another project left in the shared ~/.m2. The crawl now keeps only the coordinates the project's own poms reach. Unrelated cached artifacts are never looked up, so they are never pinned in hosted mode, patched in agent mode, or attested by vex. This is the Maven child of #595, following the "crawler as locator" shape that #1183 used for NuGet.

Root cause

MavenCrawler::crawl_all walked the whole Maven local repository whenever the cwd held a pom.xml (scan_maven_repo). Hosted mode then treated any GA absent from pom.xml as transitive and added a <dependencyManagement> pin, with the Socket repository and Trusted Checksums, for whatever some other build had cached.

Fix

  • crawlers/maven_scope.rs (new) computes the project's dependency graph from the poms the local repository already holds. Maven caches the pom of every artifact, parent and BOM it resolves.
    • Seeds: every <dependency> of the reactor, any scope. That covers the root pom.xml, its <modules>/<subprojects> (recursively), every profile, and the dependencies each pom inherits from its parents.
    • Edges: each artifact's own non-optional compile/runtime dependencies, plus those its parents declare.
    • Versions: literals are interpolated through properties and the parent chain. A version-less declaration takes the managed version: parent chain first, then imported BOMs. The reactor's management also applies to transitives, as it does in Maven. A hosted <base>-socket.<hex8> pin keeps its base release in scope.
    • Fallback: when a version can't be determined (an undefined property, a range, a pom missing from the repository), every cached version of that artifact is admitted. The scope over-approximates within artifacts the project names, never across them.
  • MavenCrawler::crawl_all_with: in project mode, a pure Maven build (no Gradle or sbt/Mill/scala-cli build beside it) filters the Maven local repository through the scope.
    • --global, --global-prefix, Gradle and Scala-tool roots are unchanged.
    • An unreadable root pom.xml leaves the crawl unscoped.
    • The scope's pom reads run alongside the scan's directory walk on the walk pool.
  • maven_reactor::pom_model (new, pub(crate)) exposes the reactor planner's real XML tree, so the scope reads poms the same way vendoring does: comments and CDATA are ignored, and plugin dependencies and exclusions are never declarations.
  • Docs: docs/ecosystems.md (new "A Maven project's scan is scoped to its dependency graph" caveat, with the remedy for a never-resolved project) and CLI_CONTRACT.md.
  • Tests and fixtures:
    • The two CLI scan tests whose pom.xml declared nothing now declare the artifacts they expect.
    • The maven bench fixture now declares every cached artifact its direct dependencies don't reach, so base and head scan the same 1000 packages.

Per-issue checklist

  • Maven hosted scan pins, and VEX attests, artifacts the project doesn't depend on (the crawler lists all of ~/.m2) #265: project_mode_crawl_keeps_only_the_projects_graph. This is the issue's repro: the pom depends only on junit:junit:4.13.2 (test), and the cache also holds commons-lang3:3.12.0. With scoping disabled, the test fails with left: [junit, commons-lang3, hamcrest-core]. With it, only junit and hamcrest-core are crawled. --global still lists all 3.
  • Scope unit tests:
    • the_scope_is_the_declared_graph_not_the_cache: test-scope and optional edges are excluded, and other cached versions are not admitted.
    • modules_parents_boms_and_management_are_followed.
    • an_undeterminable_version_admits_every_cached_version_of_that_artifact.
    • a_hosted_pin_keeps_its_base_release_in_scope.
    • no_readable_root_pom_is_unscoped.

Performance

socket-patch-bench compare --filter '^maven/' locally (macOS, base = main):

  • maven/hosted: +7.4% wall.
  • maven/rescan: +11.9% wall.

The cost is kernel time for one pom open per reachable artifact (about 40 µs each on this macOS machine). Locating the graph needs those reads. Overlapping them with the scan walk brought this down from +17–27%. Linux opens are much cheaper, so the CI bench may pass. If it flags the scenario anyway, that is the inherent price of scoping: a reviewer can weigh it and add performance-regression-accepted.

Commands run

  • cargo test -p socket-patch-core --no-fail-fast: green.
  • cargo test -p socket-patch-cli --no-fail-fast: green except the two e2e_vendor_cargo_build old-toolchain cases. Those depend on local cargo toolchains, are unrelated, and fail the same way on main.
  • Real-Maven capstones, all green:
    • --test e2e_redirect_maven_build -- --ignored
    • --test e2e_vendor_maven_build -- --ignored
    • --test e2e_vendor_jvm_build -- --ignored maven_reactor
  • cargo test -p socket-patch-bench: green.
  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo fmt --check: clean for the changed files. Local rustfmt flags only the untouched upstream/mod.rs:917.

🤖 Generated with Claude Code


Note

Medium Risk
Changes which Maven artifacts are discovered in project mode (hosted pins, patches, VEX), with intentional over-approximation when versions are ambiguous; global mode is unchanged.

Overview
Fixes #265: project-mode Maven scans no longer treat the entire shared ~/.m2 as this project's packages.

maven_scope walks the reactor and transitive graph from poms already in the local repo (modules, profiles, parents, BOM imports, dependency management, property interpolation) and admits only those GAVs—or all cached versions when a version cannot be resolved. MavenCrawler applies that filter for pure Maven project mode on Maven2 roots (not Coursier-spelled paths); --global, Gradle/Scala builds, and unreadable root poms stay unscoped. Scope POM reads run in parallel with the directory walk.

Adds pom_model / PomModel in maven_reactor for shared POM parsing. Docs (ecosystems.md, CLI_CONTRACT.md), CLI/e2e tests, and the Maven bench fixture are updated so declared dependencies match the scoped crawl.

Reviewed by Cursor Bugbot for commit 19a470b. Configure here.


Generated by Claude Code

WIP: tests and docs to follow.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A project-mode Maven crawl now keeps only what the project's poms
reach, so the scan tests' poms declare the artifacts they expect to
be discovered.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The scope walk opens one pom per reachable artifact while the scan
only walks directories, so the two now run side by side on the walk
pool. A hosted `<base>-socket.<hex8>` pin keeps its base release in
scope, so a rescan of an already-redirected project still sees the
packages it pinned. The maven bench fixture declares every cached
artifact its direct dependencies do not reach, so its whole cache
stays the project's under the scoped crawl.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 9, 2026 19:26
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Dependency coordinates skip property interpolation
    • Added interpolation of groupId and artifactId in the declare() function to match the behavior in decl_gav(), ensuring property-based coordinates like ${project.groupId} are resolved before use.

Create PR

Or push these changes by commenting:

@cursor push 21a5722153
Preview (21a5722153)
diff --git a/crates/socket-patch-core/src/crawlers/maven_scope.rs b/crates/socket-patch-core/src/crawlers/maven_scope.rs
--- a/crates/socket-patch-core/src/crawlers/maven_scope.rs
+++ b/crates/socket-patch-core/src/crawlers/maven_scope.rs
@@ -273,20 +273,26 @@
     /// Queue `decl` read in `chain`'s context. A transitive edge also takes
     /// any version the reactor's management assigns its artifact.
     fn declare(&mut self, decl: &PomDecl, chain: &Chain, direct: bool) {
+        let Some(group) = interpolate(&decl.group, chain) else {
+            return;
+        };
+        let Some(artifact) = interpolate(&decl.artifact, chain) else {
+            return;
+        };
         if !direct {
             let reactor = std::mem::take(&mut self.reactor_chains);
             for (i, rc) in reactor.iter().enumerate() {
-                let key = (i, decl.group.clone(), decl.artifact.clone());
+                let key = (i, group.clone(), artifact.clone());
                 let managed = match self.reactor_managed.get(&key) {
                     Some(found) => found.clone(),
                     None => {
-                        let found = self.managed_version(rc, &decl.group, &decl.artifact, 0);
+                        let found = self.managed_version(rc, &group, &artifact, 0);
                         self.reactor_managed.insert(key, found.clone());
                         found
                     }
                 };
                 if let Some(version) = managed {
-                    self.enqueue(&decl.group, &decl.artifact, version);
+                    self.enqueue(&group, &artifact, version);
                 }
             }
             self.reactor_chains = reactor;
@@ -294,10 +300,10 @@
         let version = match &decl.version {
             Some(raw) => Some(interpolate(raw, chain)),
             None => self
-                .managed_version(chain, &decl.group, &decl.artifact, 0)
+                .managed_version(chain, &group, &artifact, 0)
                 .or(Some(None)),
         };
-        self.enqueue(&decl.group, &decl.artifact, version.flatten());
+        self.enqueue(&group, &artifact, version.flatten());
     }
 
     /// The version `chain` manages `g:a` at: its parent chain's management,

diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs b/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs
--- a/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs
+++ b/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs
@@ -917,7 +917,10 @@
     fn bun_lock_remedies_name_the_forced_reinstall() {
         for file in ["bun.lockb", "bun.lock", "packages/app/bun.lockb"] {
             let remedy = checkout_remedy(&[file.to_string()]);
-            assert!(remedy.contains(&format!("`git checkout -- {file}`")), "{remedy}");
+            assert!(
+                remedy.contains(&format!("`git checkout -- {file}`")),
+                "{remedy}"
+            );
             assert!(remedy.ends_with(
                 ", then run `bun install --force` (a plain `bun install` keeps the patched copy)"
             ), "{remedy}");

You can send follow-ups to the cloud agent here.

Comment thread crates/socket-patch-core/src/crawlers/maven_scope.rs
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] scan performance flags maven/hosted / maven/rescan on CPU time only (+26% / +30%). Wall time is within noise: +0.3% [-0.2, +1.7] and +1.0% [+0.5, +2.0] (run 37980235397).

The extra CPU is inherent to the fix. Scoping the crawl to the project's graph means opening one cached pom per reachable artifact (1000 in this fixture). Before, the crawl only listed directories. Locally on macOS that is about +40 ms of kernel time and no measurable user time. The reads already run alongside the scan's directory walk, which is why wall time doesn't move.

Avoiding them would mean not scoping, which is #265 itself. If a maintainer agrees with the trade-off, the performance-regression-accepted label clears the gate. I have not added it myself.

declare interpolated a dependency's version but enqueued its raw groupId
and artifactId, so coordinates like ${project.groupId} stayed literal,
never resolved to a repository pom and never matched the path-derived
crawl names. Those artifacts and their transitives fell out of the
project-mode scan. Interpolate all three, as decl_gav already does for BOM
imports, and match managed entries the same way.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KoEk4Y9wedBsjtt9kTUPVY
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: disabled auto-merge before pushing a fix for the open Bugbot finding (19a470b: dependency groupId/artifactId are interpolated in the crawl scope). The final reviewer should re-review this head and re-arm. Awaiting CI and Bugbot.


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 19a470b. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Agentic Security Review — 2 finding(s).

  • [HIGH] crates/socket-patch-core/src/crawlers/maven_scope.rs:417 — Project-mode Maven scoping fail-opens when a reachable cached POM pushes the walk past MAX_NODES (50_000). visit() admits every queued GAV, including coordinates whose POM is not in the repo, and returns None once scope.gavs.len() exceeds the cap. project_scope() turns that into None, and crawl_all_with() applies admits() only inside if let Some(scope), so the full shared local-repository listing is kept. A single already-resolved compile/runtime POM can declare about 50_001 distinct GAVs (a few megabytes of XML; pom_model and read_regular_to_bytes_sync have no node or size cap) and force that path. There is no second graph filter: scan queries that list, and hosted mode pins every selected Maven package. rewrite_maven_pom inserts a pin, Socket repository, and trusted checksums even when the pom has no matching (maven_pom_fail_closed_transitive_dep_management). Those pins are what vex attests. The same unfiltered list is what agent mode patches in place in the shared cache. User docs only name an unreadable root pom.xml as unscoped; the cap comment says a bigger graph is left unscoped, which an untrusted POM can choose.

    • Impact: A publisher of a POM already cached and reachable from the reactor (direct or transitive compile/runtime dependency) can turn off the project-mode boundary this PR adds over the shared Maven local repository. A default hosted project scan (global false, no --global-prefix, pom.xml and no Gradle or Scala-tool build) then treats every other artifact in that repository as this project's package: it can write dependencyManagement pins and Socket repository entries for cached GAVs the project never declared, and vex will attest those pins. That is the cross-project contamination the scope was added to stop. Agent mode uses the same list to patch jars in the shared cache.
    • Remediation: When a project-mode scope walk hits MAX_NODES or otherwise cannot be computed, fail closed: return an error or a scope that admits nothing, so the shared cache is not kept. Do not reuse None, which crawl_all_with already treats as 'do not filter'. Keep the unreadable-root-pom case separate if that exception stays intentional, and document it; do not let a reachable cached POM select the unscoped path.
  • [HIGH] crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs:1911 — Project-mode Maven scope copies groupId, artifactId, version, and property text without decoding XML character references or the five predefined entities. Maven's model reader expands those references before it chooses the local-repo path, so crawl_all_with's exact retain() drops the coordinate Maven actually resolved and hosted pin / VEX never see it.

    • Impact: A dependency, parent, or BOM author (or a reactor pom) can spell a real coordinate with a numeric character reference such as 1.0.0 or com.example. Maven installs the decoded GAV; the new project-mode allowlist stores the undecoded text and retain() removes the path-scanned package. That artifact is not pinned or VEX-attested even though this build resolved it. The documented scope is supposed to be a superset of the resolved graph; this under-includes it. Custom DOCTYPE entities are not this bug: Maven rejects unresolved named entities instead of installing a decoded path.
    • Remediation: Decode numeric character references and the five predefined XML entities in the scope walk (the same bounded decoder as the NuGet and sbt readers) before interpolation and GAV admission, including property values. Do not treat undecoded '&#…;' / '&' text as a coordinate. Leave unresolved named entities undecoded only if you also refuse to admit them; Maven will not have installed a decoded path for those.

path,
local: false,
});
let deps: Vec<PomDecl> = chain.iter().flat_map(|n| n.model.deps.clone()).collect();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[agent] visit only follows <dependencies>; nothing in the new pom_model reads distributionManagement/relocation, so a relocation stub ends the walk. Scenario: project declares mysql:mysql-connector-java:8.0.33 (a relocation-only pom → com.mysql:mysql-connector-j). The scope admits the stub GAV (no jar), and the new filter in crawl_all_with (maven_crawler.rs ~981) drops the real com.mysql:mysql-connector-j jar and its transitive protobuf-java — both found and patchable before this PR. Same for any relocated coordinate. Fix: parse <relocation> (each missing part defaults to the stub's own g/a/v), enqueue the target here, and add a relocation-stub test.

This branch has not been deployed

No deployments
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.

Maven hosted scan pins, and VEX attests, artifacts the project doesn't depend on (the crawler lists all of ~/.m2)

3 participants