Repository navigation
Fix Maven VEX false attestation through profiles, properties and coordinates - #2005
Open
Mikola Lysenko (mikolalysenko) wants to merge 40 commits into
Open
Mikola Lysenko (mikolalysenko) wants to merge 40 commits into
Mikola Lysenko (mikolalysenko) wants to merge 40 commits into
Conversation
VEX discovery attested two Maven pins the build may not resolve:
1. Profile declarations (J1). Discovery dropped every dependency inside
<profiles> from the version grouping and diagnosed only suffixed
profile versions. A profile declaring the pinned GA at the base
version (or any other version) left the hosted ref attested, yet
whenever that profile is active Maven resolves the unpatched jar. The
hosted rewriter leaves profile declarations as written
(redirect_maven_profile_dependency_unpatched) and vendored mode treats
every profile as live, so discovery now agrees with the writers: a
profile declaration of the pinned GA (main jar in <dependencies> or
<dependencyManagement>, a declaration an undefined coordinate property
may make that GA, or an executable classifier copy) at any version
other than the pin keeps the ref but marks it unattested with the new
UnattestedKind::MavenProfileUnpatched (CLI note
vex_maven_profile_unpatched). A profile declaration with no <version>
takes the live pin and does not withhold it. The pre-v5 single-pom
vendored wiring gets the same check against its vendored version.
2. .mvn/maven.config user properties (J4). A ${prop} <version> was
resolved from the pom's <properties> only, though coordinates already
read maven.config's -D user properties. Maven looks user properties up
first, so <version>${v}</version> with <v>1.0-socket.x</v> in the pom
and -Dv=1.0 in maven.config builds the base release while discovery
attested the pin. PomScope now lays the user properties over the root
<properties> once, and both version resolution and coordinate
interpolation read that map. The vendored reactor's interpolator is a
method on the multi-pom Reactor, so it is not reused here; this is the
minimal change in formats/maven.
Tests (each fails on origin/main):
- formats::maven::tests::version_property_reads_user_properties_first
- vex::discover::maven::tests::version_property_reads_maven_config_overrides
- vex::discover::maven::tests::profile_declaration_off_the_pin_is_not_attested
- vex::discover::maven::tests::vendored_ref_under_a_profile_version_is_not_attested
No existing test asserted the old behavior; the golden corpus is
unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 4, 2026
Review findings on the profile / user-property attestation fix:
- A profile <dependencyManagement> entry was flagged even when the live
pin is a direct literal <version>, which beats management; likewise a
profile-managed classifier entry no declaration takes. Profile
management now withholds the pin only when management governs it (no
live direct version). The old test case managed(Some("1.7.36")) beside
a direct pin asserted the wrong outcome and now expects an attested
pin; the governing case (managed live pin) expects the opposite.
- A version-less profile classifier copy was always flagged. One shared
classifier check now covers live and profile copies and resolves a
version-less copy from the classifier-keyed management (live, with a
profile's entry overriding it; none in this pom fails closed). Profile
classifier copies report MavenClassifierUnpatched, like live ones.
- The same check now runs for the pre-v5 vendored wiring too, where any
executable classifier copy comes from the next repository (the vendored
tree holds only the main jar). formats::maven gains
NON_EXECUTABLE_CLASSIFIERS / is_executable_classifier, worded as in
#2006 so the two merge cleanly.
- A pom-typed (or java-source / javadoc) profile declaration carries no
code and no longer counts against the pin. Other types (test-jar, ejb)
still do: they put a jar on a classpath.
- PomDep now keeps both the version as the pom spells it (`version`, what
hosted writes and rolls back) and as the build resolves it
(`build_version`, -D user properties first). Pins are found from the
former, so a pin .mvn/maven.config moves stays a ref (list / rollback /
remove and liveness keep the wiring) but is marked unattested with the
new UnattestedKind::MavenConfigOverride (CLI note
vex_maven_config_override). A Socket version only maven.config
supplies is diagnosed (DIAG_REF_UNATTRIBUTABLE naming
.mvn/maven.config), never a pom.xml ref, so nothing edits pom.xml for
it.
- The build view leaves project.* / pom.* to the model: -Dproject.version
no longer resolves a ${project.version} dependency version.
- Cleanup: main_jar_declarations / classifier_declarations take a scope
predicate and back ga_versions, the classifier check and the profile
check; group_by_ga is gone (live_gas lists the pin candidates);
trusted_checksums takes the maven.config text extract already read.
Tests: version_property_reads_maven_config_overrides,
profile_declaration_off_the_pin_is_not_attested and
vendored_ref_bypassed_by_a_profile_or_classifier_is_not_attested
(renamed from vendored_ref_under_a_profile_version_is_not_attested) were
extended with a case per finding; each new case failed on the previous
commit. formats::maven gains model_reference_version_ignores_user_properties.
The golden corpus is unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes the two Maven false-attestation gaps #2005 left open in VEX discovery. 1. Profile <properties> redefining a pin's property. A live <version>${v}</version> that the root <properties> resolve to the Socket pin resolves something else whenever an active profile's <properties> redefine v (or a property v names): Maven merges active profile properties over the pom's own before interpolating. Vendored mode refuses to pin over such a property (version_override); discovery attested it. PomScope now reads each profile's <properties>, and every PomDep carries version_alternatives: the other versions its build_version takes under any profile or set of profiles active together (user properties and project.* model references stay fixed; a value some set leaves unresolved, cyclic or too deep counts as the text as written). profile_override now walks live main-jar declarations too (same main_jar_declarations scope and management-governs filter): one whose build version is the pin but has an alternative off it marks the ref unattested with the existing UnattestedKind::MavenProfileUnpatched (CLI note and CLI_CONTRACT.md updated), in hosted and the pre-v5 vendored wiring alike. Profile declarations and the shared classifier check (unpatched_classifier_copy) count every alternative as a version the declaration may resolve. A <profiles> declaration's own ${v} reads its own profile's <properties> first. 2. Property versions resolved one level deep. ${a} with <a>${b}</a> now resolves through every property it names, in both views PomDep keeps: `version` (as the pom spells it: model, own profile, root properties) and `build_version` (.mvn/maven.config user properties first at every level; project.* stays the model's). Limits are the vendored reactor's: at most 16 levels, and a cycle, undefined name or unterminated ${ never resolves (the version stays as written, which is never a pin). A partly literal version (${a}.${b}-x) resolves too, and ${project.version} resolves from the model in both views. The vendored reactor's interpolator (interpolate_by and the duplicate body of Reactor::interpolate_with) moves to formats/maven as the pure interpolate_strict, built on interpolate_each, which enumerates the values a name may take (capped at 16 results). Both now detect cycles explicitly and bound work (1024 lookups, 4096 bytes), so a pom whose properties each name the next many times over fails closed instead of expanding exponentially. Reactor::interpolate drops its always-zero depth argument. Vendored behavior is otherwise unchanged. Tests (each fails before this change): - vex::discover::maven::tests::profile_properties_redefining_a_pin_property_are_not_attested - vex::discover::maven::tests::profile_declaration_reads_its_own_profile_properties - vex::discover::maven::tests::version_property_resolves_through_nested_properties (16-deep chain resolves, 17 does not; two cycles; nested user overrides) - vex::discover::maven::tests::vendored_ref_under_profile_properties_is_not_attested - formats::maven::tests::{version_properties_resolve_recursively_and_fail_closed, profile_properties_give_version_alternatives, interpolation_is_bounded} The golden corpus is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review findings on the profile / nested property resolution (#2020): 1. Model references moved by properties (false attestation). The model (${project.version} from <version>${revision}</version>) was bound already resolved from the root properties, and profiles were assumed unable to change it. Model references are now bound raw and resolved wherever they are read, with that read's user properties and active profiles: a profile <revision> moves ${project.version} (unattested), and a -Drevision=<pin> only .mvn/maven.config supplies makes the spelled and built versions differ, so it is diagnosed, never a ref. 2. Attributed / spaced property tags (false attestation). read_properties took the whole tag as the name, so <v combine.self="override"> and <v > were skipped. The name now ends at whitespace or '/', closing tags may carry whitespace, and <v/> defines an empty value. 3. Nested references in a profile value resolved against the root. Each combination of profiles is now resolved as one property view (the profiles merged over the pom's, a later one winning), so a profile's ${w} reads its own <w> first. 4. Profile <properties> parse errors failed the whole scope, including the hosted rewriter's. PomScope::with_user_properties no longer reads profile properties; parse_pom_with calls read_profiles(), and an unreadable one withholds every ${...} pin (MovedBy::Unreadable) while the rest of the pom is read. 5. The shared cross-product budget collapsed ordinary multi-profile poms. interpolate_each and its 16-result / 1024-lookup product are gone: profile_combinations walks the names reachable from the version, keeps only the profiles that define one (at most 8, else fail closed), and every combination of those is resolved on its own budget. 6. The detail named the lexicographically first alternative. Alternatives are listed smallest combination first and carry the profile ids (MovedBy), and the detail names them and the version they give. 7. Two precedence implementations. formats::maven::properties now holds lookup_property (Maven's order: model, user properties, each pom's properties with its active profiles merged in, bare ${version}), PomLayer and profile_combinations; PomScope and the vendored Reactor::lookup_with / version_override both use them, so vendoring now weighs the same combinations VEX does (it previously tried one profile at a time) and fails closed past the same bound. 8. Two interpolators. The round-based coordinate interpolate is gone: coordinates and model references go through the same expansion as versions, in a keep-unresolved mode (interpolate_lenient). Coordinate alternatives stay for #2026; PomScope::resolve_in_build takes the interpolation so it can reuse it. 9. Cleanups: resolutions() drops its from-profile flag (the classifier check derives it), PomScope::fixed and the profile-set guard are gone, and readers that never weigh profiles do not scan them. Tests (each failed before this commit): - vex::discover::maven::tests::model_references_move_with_the_properties_they_are_spelled_through - vex::discover::maven::tests::profile_properties_with_attributes_are_read - vex::discover::maven::tests::profile_values_resolve_through_their_own_profile - vex::discover::maven::tests::unreadable_profile_properties_withhold_only_property_pins - vex::discover::maven::tests::many_profiles_that_keep_the_pin_are_attested - vex::discover::maven::tests::profile_property_detail_names_the_profile_and_version - vendor::jvm::maven_reactor::tests::profiles_that_move_the_version_only_together_are_weighed plus formats::maven::properties unit tests and extended formats::maven::tests::profile_properties_give_version_alternatives. The golden corpus is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes the last Maven property false-attestation gap in VEX discovery after #2020: a <groupId> / <artifactId> written as ${prop} that a profile's <properties> redefine. Coordinates were read only with no profile active, so discovery attested two shapes the build may not resolve: (a) The declaration carrying the pin names another package whenever a profile redefining its coordinate property is active; the pin then no longer applies to the patched GA, which resolves from its other declarations or transitively, unpinned. (b) Another declaration (live, or inside a profile whose own properties retarget it) becomes the patched GA under a profile, at its own version off the pin. Both readers now resolve coordinates through #2020's shared helpers (formats::maven::properties): the groupId and artifactId are joined with COORDINATE_SEPARATOR (U+0001, which XML 1.0 forbids) so one profile_combinations walk and one interpolation per combination read both under the same active profiles, through interpolate_keeping (the keep-unresolved interpolate_lenient as an Interpolate), and split_coordinates splits the result. formats/maven: PomScope::coordinate_alternatives runs the joined text through resolve_in_build; PomDep carries group_text / artifact_text and coordinate_alternatives (CoordinateAlternative: group, artifact, MovedBy), every pair some combination gives other than the no-profile reading (ScopedDep.group / artifact, unchanged), plus a profile declaration's own-profile reading. More than 8 relevant profiles or an unreadable profile fail closed (the text as written, which may_name reads as any text). vex/discover/maven: bypasses() runs coordinate_override after profile_override, reported as MavenProfileUnpatched (detail names the profiles), for (a) the live main-jar declaration carrying the pin (build_version == pin) with any alternative, and (b) any declaration with an alternative that may be the GA's main jar (literal or may_shadow; pom / sources / javadoc types skipped) at a version other than the pin among its resolutions(), or literally an executable classifier copy off the pin or with no version of its own. As in profile_override, a managed declaration counts only where management governs the pin, and a versionless main jar takes the live pin. CLI note and CLI_CONTRACT.md updated. vendor/jvm/maven_reactor: the v5 planner had the same gap. interpolate_with and profile_override take the Interpolate to use (interpolate_strict for versions, as before), and coordinates_elsewhere reads the joined coordinates in each inheritor and under each profile combination on the chain through them. (a) The existing coordinates_overridden check now weighs profiles, not only inheritors. (b) New warn_moved_coordinates: a declaration that names another artifact in its pom but may be the GA in an inheritor or under a profile combination is warned (coordinates_overridden) and, when versioned, unpins its root and is uncertain, so attestable_checked no longer attests it. This also covers the inheritor form of (b). Tests (each fails before this change): - vex::discover::maven::tests::profile_properties_moving_the_pinned_coordinates_are_not_attested - vex::discover::maven::tests::profile_properties_turning_a_declaration_into_the_pinned_ga_are_not_attested - vendor::jvm::maven_reactor::tests::a_profile_moving_the_declared_coordinates_is_left_alone - vendor::jvm::maven_reactor::tests::a_profile_turning_a_declaration_into_the_patched_artifact_is_weighed - formats::maven::tests::profile_properties_give_coordinate_alternatives Guard: vex::discover::maven::tests::unrelated_coordinate_properties_do_not_withhold. User-property variants (a chain through -D, -D beating the profile) are in each. The golden corpus is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ecks /code-review high findings on #2026. Discovery (vex/discover/maven.rs): coordinate_override is gone. Each declaration's coordinate_alternatives now carry the version it resolves under the same profile combination, and bypasses() weighs every such reading as a declaration of its own (retargeted(): read only under those profiles, marked retargeted_by) through the existing unpatched_classifier_copy and profile_override checks; only the pinned declaration moving off the GA keeps its own small check (pin_retargeted). So: - (1) a classifier copy retargeted onto the GA is weighed with the Wiring: under the pre-v5 vendored repository (main jar only) it withholds even at the vendored version; - (6) a profile that moves coordinates and version onto the pin together no longer withholds; - (8) the reading's <type> maps as Maven's does: test-jar is the tests classifier copy, any other non-jar type (war, pom, ...) is not the GA's jar. formats/maven: resolve_in_build is a wrapper over resolve_all_in_build, which reads several texts of one declaration under the same profile combinations but each through its own interpolation and work budget; profile_combinations takes all of them. (5) The U+0001 joined text is gone: an unterminated ${ in one coordinate stays in that one, and a reading always names a non-empty profile set. Vendored (maven_reactor.rs): profile_override / read_elsewhere read several texts under one combination, each on its own budget. (7) warn_unresolved_coordinates and warn_moved_coordinates are one pass, warn_property_coordinates, classifying each declaration as unresolved or moved with a shared warn-and-unpin tail. (2)(4) Text that can never read the patched GA (may_interpolate_to) is ruled out before any inheritor or profile is read, so a >8-profile fail-closed walk no longer unpins roots for ${g}:guava. (3) An executable classifier copy an inheritor or profile retargets onto the GA warns classifier_declared, as a literal one does. Hosted (9, back-port): PomIndex reads profile <properties> and each declaration's coordinate_alternatives, and rewrite_maven_pom refuses a dep whose pinned declaration a profile moves off the GA, or that another declaration becomes at another version (or as an executable classifier copy), with nothing written: redirect_maven_profile_coordinates_unpatched (CLI_CONTRACT.md). The vex_maven_profile_unpatched note no longer suggests re-patching such a pin with vendored mode. Tests (each fails with the fix disabled): - vex::discover::maven::tests::vendored_ref_under_a_retargeted_classifier_copy_is_not_attested - vendor::jvm::maven_reactor::tests::many_profiles_on_an_unrelated_coordinate_are_not_weighed - vendor::jvm::maven_reactor::tests::a_profile_retargeting_a_classifier_copy_warns - patch::redirect::tests::maven_pom_profile_moved_coordinates_refuse_the_dep - new cases in profile_properties_turning_a_declaration_into_the_pinned_ga_are_not_attested (test-jar withheld; war and coordinates+version onto the pin attested) and formats::maven::tests::profile_properties_give_coordinate_alternatives (an unterminated ${ in one coordinate). Redirect and VEX goldens unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Back-port rule: hosted must not write a pin VEX can never attest.
Hosted scan already refuses a live ${property} version, so the shape
left was a versioned <profiles> declaration of the GA beside a live
one: hosted warned redirect_maven_profile_dependency_unpatched but
still pinned the live declaration, and discovery's profile_override
withheld that pin forever. That covers a profile literal at the base or
another release, a profile ${property} (undefined, or one its own or
another profile's <properties> move off the pin, alone or combined),
a profile declaration through an undefined coordinate property, and a
profile <dependencyManagement> entry where management governs the pin
(the <dependencyManagement> pin hosted adds when no live direct
declaration carries a version).
PomIndex now records each declaration's managed flag and every version
the build may resolve for it (PomScope::resolve_in_build with
interpolate_strict, the same data discovery reads), and
PomIndex::profile_bypass mirrors discovery's profile check. When it
finds one, rewrite_maven_pom refuses the dep with nothing written,
under the existing redirect_maven_profile_dependency_unpatched code
(detail: the declaration, the version it resolves, the suffixed pin).
A versionless profile declaration, one resolving the pin, or profile
management under a direct pin still lets the pin land; the old
informational warning now fires only when nothing live is pinned or on
the legacy same-GAV path. CLI_CONTRACT.md and the
vex_maven_profile_unpatched note say so.
Tests (each fails with the check disabled):
- patch::redirect::tests::maven_profile_versions_are_weighed_before_pinning (new)
- patch::redirect::tests::maven_versioned_profile_declarations_are_never_pinned_over
and maven_pom_undefined_coordinate_property_in_a_profile_is_reported,
which asserted the old pin-beside-a-profile behavior.
Redirect golden (profile-literal keeps its warning code) and the VEX
golden corpus are unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Back-port rule: hosted must not write a pin VEX can never attest.
Hosted scan skipped a dep only when an executable classifier copy
(not sources / javadoc) named the patched release; one at another
release, through a ${property}, or with no version in the pom let the
main jar's pin land, and discovery's unpatched_classifier_copy then
withheld it forever (vex_maven_classifier_unpatched).
PomIndex::unserved_classifier_copy mirrors that check with the shared
is_executable_classifier and the per-declaration build_versions (every
version the build may resolve, profile <properties> included): a
non-managed executable copy of the GA, live or inside <profiles>, whose
own versions, or else its classifier-keyed <dependencyManagement>
entries' (live; a profile's too; none live = no version in this pom),
include anything but the suffixed pin. rewrite_maven_pom then refuses
the dep with nothing written under the existing
redirect_maven_classifier_unsupported code (detail: the copy, the
version it resolves, the suffixed pin). sources / javadoc copies still
only warn, and the legacy same-GAV path keeps its release check. A
test-jar typed declaration has no classifier and stays refused as an
unsupported packaging (live) or a profile bypass. CLI_CONTRACT.md says
so in both the hosted and the vex_maven_classifier_unpatched sections.
Test (fails with the check disabled):
- patch::redirect::tests::maven_executable_classifier_off_the_pin_blocks_the_redirect
Redirect golden and the VEX golden corpus are unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
bugbot run Generated by Claude Code |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…classifiers
The "only through .mvn/maven.config" diagnostic fired whenever the build
resolved a different Socket version than the pom spelled, even when the
pom spelling was itself a Socket pin. That pin is pom.xml wiring: pin
detection keeps its ref and reports MavenConfigOverride, so the
diagnostic contradicted it. Skip it when the pom spelling is a pin.
The executable-classifier scan matched only a literal groupId/artifactId,
so a classifier copy declared through an undefined ${property}
coordinate was ignored and the main-jar pin could still be attested
while Maven might put that unpatched classifier jar on the classpath.
Match coordinates with may_name, as main-jar grouping already does.
Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
Author
|
bugbot run Generated by Claude Code |
Mikola Lysenko (mikolalysenko)
marked this pull request as ready for review
November 6, 2026 12:35
A version-less executable classifier copy took its version from any managed classifier entry `may_name` matched, including one whose groupId or artifactId is an undefined property. That entry may manage another GA, so when it was the only one and sat at the pin, discovery attested while Maven could resolve the copy from outside this pom, unpatched. Such entries still count as possible versions, but without a live entry that names the GA literally the copy's version is treated as unknown. Addresses Bugbot's classifier-management finding on this PR. Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
Author
|
bugbot run Generated by Claude Code |
Mikola Lysenko (mikolalysenko)
added
the
Ready for review
Collaborator
Author
|
Ready for review at
Generated by Claude Code |
…property-overrides Resolve the classifier-copy conflict by keeping both sides: this branch's per-resolution profile flags (flagged/resolutions) and the base's rule that a managed entry naming the GA only through an undefined property never stands in for the copy's version alone. Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
Author
Final review briefWhat it does: Maven VEX discovery stops attesting pins the build may not use. (1) If a Risk: medium. This is the attestation path, and it is about 800 lines of rule logic. But every change withholds or narrows an attestation, never adds one, and the golden corpus output is unchanged. Look here:
Verified:
Changes I made: none. Open questions (none block merging):
Approving now doesn't merge it yet: auto-merge is not armed because Generated by Claude Code |
…rrides' into fix/maven-discovery-coordinate-overrides # Conflicts: # crates/socket-patch-core/src/vex/discover/maven.rs
Second review round on Maven discovery false attestation.
- `MavenConfigOverride` compared the build-resolved versions with the
pom-spelled ones, so a property that only `.mvn/maven.config` defines
AT the pinned version (a pre-v5 vendored `${commons.version}`) was
reported as an override. It is one only when the build resolves
something other than the pin; the detail lists versions plainly
instead of a debug-printed set.
- A version-less profile `<dependencyManagement>` entry for a classifier
copy (scope or exclusions only) counted as an unpatched copy; it
overrides no version and is skipped, as `profile_override` already
does for the main jar.
- The config-only Socket-version diagnostic is emitted once per GA and
version, not once per declaration.
- The hosted rewriter uses the shared `is_executable_classifier`; the
contract rows, `UnattestedKind` doc and CLI notes say pre-v5 vendored
refs get these kinds too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ation' into fix/maven-discovery-property-overrides # Conflicts: # crates/socket-patch-cli/CLI_CONTRACT.md # crates/socket-patch-core/src/vex/discover/maven.rs
Second review round on Maven property resolution.
- `read_properties` kept the FIRST definition of a duplicated property;
Maven's model reader keeps the last (`Properties.put`). A pom defining
`slf4j.version` as the Socket pin and then as the base release was
attested while Maven built the base. Last wins now, for the root, the
profiles and the reactor alike.
- A profile declaration's `<groupId>` / `<artifactId>` were interpolated
with no profile active while its version read the profile's own
`<properties>`, so a profile dependency naming the pinned GA only
through a profile property was missed by `profile_override`.
Coordinates now read the declaration's own profile first.
- A profile was named by the first `<id>` anywhere under it (a nested
`<repository>`'s); it is named by its own direct `<id>` child.
- An undetermined profile alternative (unreadable profile properties,
too many profiles) no longer reads as "nothing it can resolve
(${v})" but as a version this reader cannot work out.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rrides' into fix/maven-discovery-coordinate-overrides # Conflicts: # crates/socket-patch-cli/CLI_CONTRACT.md # crates/socket-patch-core/src/formats/maven/mod.rs # crates/socket-patch-core/src/patch/redirect/mod.rs
Collaborator
Author
|
bugbot run 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 b6d0178. Configure here.
Collaborator
Author
|
Burn-down agent: not labeled Ready for review. Added At Question for a maintainer: should #2020 be folded into this PR before review, or should #2005 land as it is, with #2020 following right after? (Until #2020 lands, main would carry this false-attestation gap.) Generated by Claude Code |
…property-overrides The base merged main, which brought in #2006 (shared Maven variant helpers, classifier-and-type keyed management) and #2008/#2007 (new unattested kinds in CLI_CONTRACT). Kept both sides: - CLI_CONTRACT: base's eight-case list plus this branch's property-interpolation paragraph for profile declarations. - formats/maven: keep base's is_variant / is_main_jar / is_executable_variant helpers; is_model_reference already lives in maven/properties.rs on this branch. - vex/discover/maven: unpatched_classifier_copy keys management by classifier and type (base) and still checks every profile-property resolution (flagged/resolutions, this branch); profile_override walks live and profile main-jar declarations (this branch) through base's is_main_jar filter. - vendor/jvm/maven_reactor: base's new variant checks call this branch's two-argument interpolate. Co-Authored-By: Claude <noreply@anthropic.com>
…-coordinate-overrides The base took main's variant refactor (#2006: a <type> such as test-jar counts like the classifier it implies). Conflicts resolved keeping both: - redirect/mod.rs: the variant loop labels and weighs copies through variant_label / is_executable_variant, and still defers suffixed pins to unserved_classifier_copy. - maven_index.rs: unserved_classifier_copy now weighs every executable variant (typed ones too), with management keyed by classifier and type, so a test-jar copy off the pin still blocks the redirect. - maven_reactor.rs: version_moved kept as a wrapper over this PR's read_elsewhere (main's callers use it); the property-coordinate filter keeps this PR's executable-classifier rule. - discover/maven.rs: this PR's retargeted helpers with the base's variant doc and imports. - CLI_CONTRACT.md: both sides' additions (invalid_hash checksum, NuGet and sbt cases, typed variants, Go workspaces) plus this PR's text. Co-Authored-By: Claude <noreply@anthropic.com>
Two readings of a classifier copy that a profile's <properties> move disagreed with VEX discovery: - Hosted: `retargeted` counted a moved classifier copy as the patched GA only when its interpolated coordinates equal it exactly. Discovery reads them through `may_name`, so a coordinate the profile leaves on a property the pom does not define may still be the GA. Scan could write a pin that vex then withholds (`vex_maven_classifier_unpatched`). Use `may_name` here too. - Vendored: `warn_property_coordinates` dropped every declaration whose <type> is not `jar`, test-jar included. Discovery and hosted map test-jar to the `tests` classifier copy, so a profile that turns one into the patched GA now gets `classifier_declared` like a <classifier>tests</classifier> copy does. Addresses Bugbot 4501092184 and 4501092187 on #2026. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011f9aePC6sLFkqRM2XPWq4P
Main's #2010 added the pnpm branch-lock and self-patched npm/Bun unattested kinds in the same places this PR adds the Maven profile and .mvn/maven.config kinds (UnattestedKind, vex_sources.rs notes, and the CLI_CONTRACT.md bullet and table). Kept both sides, kept this PR's reworded Maven classifier sentence and row, and counted the cases as ten (six before, two from each side). Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
Author
|
Burn-down agent: merged main ( #2010 added the pnpm branch-lock and self-patched npm/Bun unattested kinds in the same places this PR adds the Maven profile and Local checks: Generated by Claude Code |
…property-overrides
Resolve CLI_CONTRACT.md: the base grew the unattested-references list
to ten cases (pnpm branch locks, self-patched npm/Bun packages); keep
that text and re-insert this PR's sentence on profile-redefined
${property} pin versions in the Maven profile declarations case.
Co-Authored-By: Claude <noreply@anthropic.com>
…-coordinate-overrides Resolve CLI_CONTRACT.md's Unattested references paragraph with a sentence-level three-way merge: keep the base's ten-case text (pnpm branch locks, self-patched npm/Bun) and this PR's coordinate-override wording. Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
Author
|
Owner decision on the Generated by Claude Code |
…iscovery-property-overrides
…discovery-coordinate-overrides
/code-review high on #2020: - A live `${prop}` this pom leaves unresolved (a parent pom defines it) dropped every profile alternative, so a pre-v5 vendored ref spelled that way stayed attested while a profile's `<properties>` moved the build to a public release. The profile values are now kept, and the live check weighs an unresolved declaration as one that may be the pin. - A profile without `<id>` is named `default`, as Maven and the vendored reactor name it (it was numbered by `<properties>` block). - Docs: read_properties is last-wins; ScopedDep coordinates read a profile declaration's own `<properties>` only once read_profiles ran, which the hosted rewriter does not do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…discovery-coordinate-overrides
ScopedDep coordinates here are the no-profile value, with every profile reading carried by coordinate_alternatives, and the hosted index reads profile <properties> too: restate both docs #2020's round-3 merge brought in for its own branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
/code-review high on #2026: - A <profiles> declaration's ${...} groupId / artifactId was read with no profile active, its own profile's reading only an alternative. A coordinate only its own <properties> define stayed "${g}", which may_name reads as any text, so VEX withheld and hosted scan refused a valid pin beside a profile dependency its own profile names as another group (a regression from #2020). Coordinates read the declaration's own profile again, as its version does; other profiles stay alternatives. - The vendored reactor's warn_property_coordinates skipped a ${...} <type>, which discovery and the hosted index read as possibly a jar: it is weighed now, so vendored mode warns instead of pinning a root VEX then never attests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Wenxin Jiang (Wenxin-Jiang)
approved these changes
Nov 9, 2026
Wenxin Jiang (Wenxin-Jiang)
left a comment
Wenxin Jiang (Wenxin-Jiang)
left a comment
Contributor
There was a problem hiding this comment.
[agent] Approving: no regressions found by two independent passes (Claude Opus, codex); all Bugbot threads are fixed at head.
Non-blocking:
- Profile
<properties>changing a${…}pin is still attested here (pre-existing on main); #2020 closes it, so land it soon after. - Fail-safe over-refusals codex noted: a profile-local
${v}equal to the pin (maven.rs:955), a profile's direct version vs root management (maven.rs:845), and a classifierless<type>ejb</type>vendored dep treated as an unserved variant (maven.rs:870). - PR body is stale: #2006 has landed and the deferred
test-jaritem is effectively closed.
Behavior-preserving cleanup of the folded #2005/#2020/#2026 diff: - Share the copies the three PRs each grew: `classpath_copy` (which copy a profile-moved declaration reads as) for discovery's and hosted's `retargeted` and the reactor's coordinate warning; `management_key` for both classifier-copy checks; `Definitions::fixed` for the model-reference / user-property prefix PomScope and the reactor built by hand; one `named` helper for "(which may be g:a)" details. - Drop dead code: resolve_in_build's always-strict `Interpolate` parameter (hosted reads `resolve_version` instead), the reactor's literal re-binding of already-literal coordinates, a redundant filter in the -D diagnostic, a re-checked `suffixed_version`. - Cheaper: literal coordinates skip the profile scan, `bypasses` borrows the pom's declarations unless a profile moves one, hosted computes build versions only for the profile / classifier declarations that are read, and its detail text is built only when returned. - Docs: no review-history narration; the UnattestedKind and note docs reflowed and pointed at one another; the discovery module doc defers to `bypasses`. - Tests: shared builders and assertion helpers (decl / mgmt / assert_withheld / vendored_project_with, rewrite_pom / assert_refused / assert_pinned, reactor / with_corp_profile, user_props) replace per-test copies; no case removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Hosted build_versions are computed for every declaration again, so an empty list only ever means "no <version>"; the index reads a new PomScope::build_versions (the build half of resolve_version) instead of resolving the pom-spelled version it discarded. - assert_withheld takes the case it checks, so a table-driven failure names the failing case again. - The discovery module doc lists the bypasses it withholds for (classifier copies, profile declarations and management, profile properties moving a version or coordinates, -D overrides) and the interpolation bounds. - The reactor's classifier comment says where its type rule is narrower. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A `${…}` groupId / artifactId declaration with an empty `<type></type>`
gave no reading once a profile's <properties> moved it: classpath_copy
treated the empty type as "no jar on the classpath", while Maven (and
is_main_jar / is_variant everywhere else) reads it as the default jar.
So a declaration a profile turned into the pinned GA at another version
went unweighed: hosted scan wrote the pin and VEX attested it (a false
attestation, carried over from the folded #2026). An empty type now maps
like `jar`. Tests: a withheld case in
profile_properties_turning_a_declaration_into_the_pinned_ga_are_not_attested
and a refused case in maven_pom_profile_moved_coordinates_refuse_the_dep,
both failing before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
changed the title
…alse-attestation # Conflicts: # crates/socket-patch-cli/CLI_CONTRACT.md
Collaborator
Author
|
Merged main ( One conflict, in
Local runs on
Generated by Claude Code |
…alse-attestation # Conflicts: # crates/socket-patch-cli/CLI_CONTRACT.md # crates/socket-patch-cli/src/commands/vex_sources.rs # crates/socket-patch-core/src/vex/discover/mod.rs
Collaborator
Author
|
Merged main ( Conflicts. Every one is a both-sides addition at the same spot, and both sides are kept:
#2009's pnpm and origin changes merged without conflicts, and the Maven code compiles against them. Local runs on
Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
VEX attested hosted (and pre-v5 vendored) Maven pins that the build may not resolve: a profile declaring the GA at another version, an executable classifier copy, a
.mvn/maven.config-Dthat moves the pin's${property}, profile<properties>that move it (alone or combined, through nested properties), or ones that retarget a${…}groupId / artifactId. Hostedscanwrote some of those pins too.Fix
properties.rs): Maven's lookup order, nested interpolation (≤16), profile combinations (≤8, else fail closed).PomDephasversion(as spelled) andbuild_version(-Dfirst), plusversion_alternativesandcoordinate_alternatives.bypasses()keeps the ref but withholds it:MavenClassifierUnpatched, newMavenProfileUnpatchedandMavenConfigOverride. A Socket version onlymaven.configsupplies is a diagnostic, not a ref.redirect_maven_profile_coordinates_unpatched; the classifier and profile codes now refuse whenever a version may be off the pin.coordinates_overriddencovers profiles.vex_maven_profile_unpatched/vex_maven_config_override; CLI_CONTRACT rows.Owner decisions
version/build_versionboth stay: the rewriter writes and rolls back viaversion, discovery checksbuild_version, andmaven.configmoves stayMavenConfigOverride.Where to look
formats/maven/properties.rs:lookup_property,profile_combinations, the interpolators.formats/maven/mod.rs:PomScope::resolve_all_in_build/coordinate_alternatives/build_versions.vex/discover/maven.rs:bypasses,profile_override,unpatched_classifier_copy,retargeted.patch/redirect/maven_index.rs(retargeted,profile_bypass,unserved_classifier_copy) and their order inrewrite_maven_pom.vendor/jvm/maven_reactor.rs:profile_override,warn_property_coordinates.Testing
Each behavior fix has a test that failed before it. Locally: core
maven/vex::/patch::redirect::(1965),redirect_golden, CLIvex_sources,vex,e2e_vex,cli_parse_vex,vendor_jvm_cli; workspace clippy and fmt clean.Folds #2020, #2026.
Review history
Fixed:
maven.configoverride keeps the ref unattested;maven.config-only pin is a diagnostic;-Dproject.*ignored.<properties>, nested and combined, move versions; model refs move with them.<id>,defaultwhen absent.test-jar, type + classifier,${…}type, empty type.Declined:
retargetedignores management, legacy same-GAVserved, 8-profile cap, unterminated${.${parent}classifier,${…}type) need a behavior PR first.direct_child_text>in attributes: detail text only.🤖 Generated with Claude Code