Repository navigation
Fix project-mode Maven crawl listing all of ~/.m2 (#265) - #1361
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
Conversation
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>
|
BugBot review |
There was a problem hiding this comment.
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.
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.
|
[agent] 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 |
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
|
bugbot run Generated by Claude Code |
|
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 |
There was a problem hiding this comment.
✅ 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.
There was a problem hiding this comment.
🔒 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(); |
There was a problem hiding this comment.
[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.

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 byvex. This is the Maven child of #595, following the "crawler as locator" shape that #1183 used for NuGet.Root cause
MavenCrawler::crawl_allwalked the whole Maven local repository whenever the cwd held apom.xml(scan_maven_repo). Hosted mode then treated any GA absent frompom.xmlas 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.<dependency>of the reactor, any scope. That covers the rootpom.xml, its<modules>/<subprojects>(recursively), every profile, and the dependencies each pom inherits from its parents.compile/runtimedependencies, plus those its parents declare.<base>-socket.<hex8>pin keeps its base release in scope.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.pom.xmlleaves the crawl unscoped.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/ecosystems.md(new "A Maven project's scan is scoped to its dependency graph" caveat, with the remedy for a never-resolved project) andCLI_CONTRACT.md.pom.xmldeclared nothing now declare the artifacts they expect.mavenbench fixture now declares every cached artifact its direct dependencies don't reach, so base and head scan the same 1000 packages.Per-issue checklist
~/.m2) #265:project_mode_crawl_keeps_only_the_projects_graph. This is the issue's repro: the pom depends only onjunit:junit:4.13.2(test), and the cache also holdscommons-lang3:3.12.0. With scoping disabled, the test fails withleft: [junit, commons-lang3, hamcrest-core]. With it, only junit and hamcrest-core are crawled.--globalstill lists all 3.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 twoe2e_vendor_cargo_buildold-toolchain cases. Those depend on local cargo toolchains, are unrelated, and fail the same way on main.--test e2e_redirect_maven_build -- --ignored--test e2e_vendor_maven_build -- --ignored--test e2e_vendor_jvm_build -- --ignored maven_reactorcargo 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 untouchedupstream/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
~/.m2as this project's packages.maven_scopewalks 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.MavenCrawlerapplies 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/PomModelinmaven_reactorfor 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