Skip to content

Withhold Maven pins whose coordinates a profile redefines - #2026

Merged
Mikola Lysenko (mikolalysenko) merged 14 commits into
fix/maven-discovery-false-attestationfrom
fix/maven-discovery-coordinate-overrides
Nov 9, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 14 commits into
fix/maven-discovery-false-attestationfrom
fix/maven-discovery-coordinate-overrides

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #2020. This closes the last Maven property false-attestation gap in VEX discovery: a <groupId> / <artifactId> written as ${prop} that a profile's <properties> redefine. Hosted scan gets the same refusal, and so does the v5 vendored planner.

The gap

Coordinates were resolved only with no profile active. Discovery therefore attested two shapes the build may not resolve:

  • (a) The declaration carrying the pin names another package whenever a profile that redefines 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 turns into the patched GA under a profile, at its own version, which isn't the pin. This covers both a live declaration and one inside a profile whose own properties retarget it.

Fix

  • formats/maven: coordinates now go through Resolve Maven pin properties through profiles and nesting #2020's shared helpers.

    • resolve_in_build is now a wrapper over resolve_all_in_build. That function reads a declaration's groupId, artifactId and version under the same profile combinations, with each text on its own interpolation (interpolate_keeping for coordinates, interpolate_strict for the version) and its own work budget. profile_combinations takes all three texts.
    • PomDep::coordinate_alternatives holds each reading whose coordinates differ from the no-profile reading, with its version and the profiles that cause it. A profile declaration's reading under its own profile counts too.
    • More than 8 relevant profiles, or profile <properties> that can't be parsed, fail closed: the text as written becomes the reading.
  • vex/discover/maven: bypasses() treats each such reading as a declaration of its own (retargeted()), active only under those profiles and at the version it resolves there. The existing checks then weigh it: unpatched_classifier_copy (with the Wiring) and profile_override.

    • Its <type> is read the way Maven maps it, consistent with may_shadow: test-jar is the tests classifier copy, and any other non-jar type isn't a jar on the classpath.
    • (a) is the one remaining dedicated check, pin_retargeted.
    • Details name the declaration as written and the profiles that retarget it.
  • vendor/jvm/maven_reactor: the v5 planner had the same gap.

    • read_elsewhere and profile_override read several texts under one combination, each on its own budget. version_override and coordinates_elsewhere both use them.
    • (a) The existing coordinates_overridden check now covers profiles as well as inheritors.
    • (b) warn_property_coordinates is one pass over the property-coordinate declarations. It first rules out text that can never name the GA (may_interpolate_to), then sorts each remaining declaration into unresolved (property_unresolved) or moved under a profile or inheritor (coordinates_overridden). Both share one warn-and-unpin tail. An executable classifier copy gets classifier_declared, as a literal one does.
  • Hosted (back-port): PomIndex now reads profile <properties> and each declaration's coordinate_alternatives. rewrite_maven_pom refuses the dep, writing nothing, in two cases:

    • the declaration the pin would go into moves off the GA under a profile;
    • another declaration becomes the GA's main jar at another version, or an executable classifier copy of it.

    The warning is redirect_maven_profile_coordinates_unpatched, documented in CLI_CONTRACT.md. scan therefore never writes a pin that vex would withhold for this reason. The vex_maven_profile_unpatched note no longer suggests re-patching such a pin with vendored mode.

  • Hosted, profile versions (back-port): hosted already refused a live ${property} version, but it still pinned a live declaration that had a versioned <profiles> declaration of the GA beside it, and discovery then withheld that pin forever. PomIndex now records each declaration's managed flag and every version the build may resolve for it, using resolve_in_build with interpolate_strict, the same data discovery reads. PomIndex::profile_bypass mirrors discovery's profile_override check.

    • When it finds a bypass, the dep is refused with nothing written, under the existing redirect_maven_profile_dependency_unpatched code.
    • Shapes covered: a profile literal at another version; a profile ${property} that is undefined, or that its own or any combination of profiles' <properties> move off the pin; a profile declaration through an undefined coordinate property; and a profile <dependencyManagement> entry where management governs the pin.
    • The pin still lands for a versionless profile declaration, one that resolves the pin, or profile management under a direct pin.
  • Hosted, classifier copies (back-port): hosted used to skip a dep only when an executable classifier copy named the patched release. PomIndex::unserved_classifier_copy now mirrors discovery's unpatched_classifier_copy, using the shared is_executable_classifier and each declaration's build_versions.

    • The dep is refused with nothing written, under the existing redirect_maven_classifier_unsupported code, when an executable copy may resolve anything but the suffixed pin.
    • That covers a copy live or in <profiles>, and versions that are another release, a ${property} a profile moves, or none in the pom once its classifier-keyed <dependencyManagement> (a profile's too) is consulted.
    • sources / javadoc copies still only warn. A test-jar typed declaration stays refused as an unsupported packaging, or as a profile bypass.

Tests

Each of these fails before this change, or with the fix disabled:

  • 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. New cases: test-jar is withheld; war, and coordinates plus version moved onto the pin together, are attested.
  • vex::discover::maven::tests::vendored_ref_under_a_retargeted_classifier_copy_is_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
  • 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
  • patch::redirect::tests::maven_executable_classifier_off_the_pin_blocks_the_redirect
  • patch::redirect::tests::maven_profile_versions_are_weighed_before_pinning. Also updated: maven_versioned_profile_declarations_are_never_pinned_over and maven_pom_undefined_coordinate_property_in_a_profile_is_reported, which asserted the old behavior of pinning beside a profile declaration.
  • formats::maven::tests::profile_properties_give_coordinate_alternatives, including an unterminated ${ in one coordinate.

The guard test is unrelated_coordinate_properties_do_not_withhold. The user-property variants are inside each test. The redirect goldens (tests/redirect_golden.rs) and the VEX golden corpus are unchanged.

Review findings (/code-review high)

  1. Confirmed, fixed. A classifier copy retargeted onto the GA is now weighed by unpatched_classifier_copy with the Wiring. Under the pre-v5 vendored repository it is withheld even at the vendored version.
  2. Confirmed, fixed. warn_property_coordinates filters with may_interpolate_to on the raw text before any inheritor or profile walk, so the >8-profile fail-closed path only applies to text that could name the GA. The reactor's profile_override still fails closed whatever moved says; the filter is the caller's job, as its doc now states.
  3. Confirmed, fixed. A retargeted executable classifier copy gets classifier_declared, without unpinning the root. That is the same treatment as a literal copy in rewrite_declarations. sources / javadoc copies are still not warned about.
  4. Confirmed, fixed by the same filter as (2), which runs before coordinates_elsewhere.
  5. Confirmed, fixed. The U+0001 joined text is gone. Each text has its own interpolation budget, an unterminated ${ stays in its own coordinate, and every reading names a non-empty profile set: combinations are non-empty, and an own-profile reading is added only when that profile exists.
  6. Done. coordinate_override is removed. Readings are expanded into per-combination declarations and go through the existing checks, which closes (1) and the "coordinates and version both moved onto the pin" gap. Only the "pin moves off the GA" case keeps its own small check, because no existing check looks for a pin disappearing.
  7. Done. The two warn passes are merged into warn_property_coordinates.
  8. Confirmed, fixed for retargeted readings in both discovery (retargeted()) and hosted (PomIndex::retargeted). A literal-coordinate declaration's type handling is Fix Maven VEX false attestation through profiles, properties and coordinates #2005's and is unchanged.
  9. Done. This is the hosted back-port described above. A follow-up commit closes the version shape as well: hosted no longer writes a pin that a profile declaration's version (literal, or moved by <properties>) bypasses.

Not done

  • In the vendored reactor, a declaration's coordinate readings and its version are not matched per combination. Like the existing undefined-coordinate rule, any versioned declaration that may become the GA unpins its root.
  • In vendored mode, an executable classifier copy (literal or retargeted) still only warns. attestable_checked does not withhold for it. This predates this PR (rewrite_declarations' classifier_declared).

Brought current with #2020, plus round-2 review findings (cloud routine)

Merged the updated #2020 (3389c34) in as c82973f. Conflicts:

#2020 round 2 made a profile declaration's coordinates read its own profile's <properties>. This PR had read them with no profile active instead. Round 3 below reverts that, so the declaration's own profile is read again.

Review findings, round 2 (/code-review high on c77d5d5..402fccf)

  1. FALSE ATTESTATION: a retargeted declaration with a type and an explicit classifier was dropped. Confirmed and fixed. test-jar + <classifier>tests</classifier> fell through to "any other type: not weighed" in both discovery's and the hosted index's retargeted. Hosted scan wrote the pin and VEX attested it, while the active profile put the unpatched tests jar on the classpath. An explicit classifier now names the copy whatever the type. Tests: profile_properties_turning_a_declaration_into_the_pinned_ga_are_not_attested and maven_pom_profile_moved_coordinates_refuse_the_dep, both extended. Both failed without the fix.
  2. Contract wording on the retargeted classifier copy in vendored mode: fixed. It now says this case is reported as classifier_declared with the root still pinned.

Declined:

  • Hosted retargeted ignores managed / pin_managed, so it refuses some pins VEX would attest. This over-refuses, which is the fail-safe direction. The proper fix is the single shared implementation the reviewer suggests, which is a refactor.
  • Version-only profiles now count toward the 8-profile cap for coordinates. This also over-refuses.
  • set_version leaves build_versions stale. No current caller reads them after an edit. Worth fixing in the refactor.
  • Clones, doc.coordinate reuse and the loosened kind assertion. These are performance and test style.

Tests (cloud run): cargo test -p socket-patch-core --lib -- formats::maven vex::discover:: patch::redirect:: vendor::jvm gives 1611 passed, golden included. The 1 failure is the chmod test, which root ignores. CLI --lib vex_sources passes. clippy and fmt are clean.

Review findings, round 3 (/code-review high after merging current #2020 / main)

Merged #2020 (ddf4808) in as 791ded0, with a doc follow-up in 237307e (its no-profile wording is superseded by finding 1). Fixed in 37d9fd9:

  1. Regression, confirmed and fixed (over-refusal). A <profiles> declaration's ${…} coordinate was read with no profile active, and its own profile's reading was only an alternative. A groupId that only its own <properties> define therefore stayed ${g}, which may_name reads as any text. VEX withheld, and hosted scan refused, a valid pin that sat beside a profile dependency naming another group. Coordinates now read the declaration's own profile again, as its version does, and other profiles stay alternatives.
    • Tests: a new case in unrelated_coordinate_properties_do_not_withhold and in maven_pom_profile_moved_coordinates_refuse_the_dep. Both failed without the fix.
    • The own-profile case in profile_properties_turning_a_declaration_into_the_pinned_ga_are_not_attested is still withheld (MavenProfileUnpatched). It is now asserted on its own, because the detail names the declaration as a profile declaration of the GA. The formats expectation is updated to match.
  2. Vendored parity, confirmed and fixed. warn_property_coordinates skipped a ${…} <type>, which discovery and the hosted index read as possibly jar. As a result, vendored mode pinned a root that VEX never attests, and nothing explained why. It is weighed now. Test: a_profile_turning_a_declaration_into_the_patched_artifact_is_weighed, which failed without the fix.

Declined:

  • An executable classifier copy at exactly the suffixed version is no longer refused by hosted. This was deliberate: maven_executable_classifier_off_the_pin_blocks_the_redirect lists it under pinned, mirroring how discovery's unpatched_classifier_copy treats served. It can't attest unpatched code either, because the hosted repo serves only the main jar, so such a copy fails resolution loudly. Any rewriter run hits the same failure, and that failure exists before the scan.
  • Hosted retargeted ignores managed / pin_managed. This over-refuses, and was declined in round 2.
  • The legacy same-GAV path passes served = None to retargeted. This over-refuses. Passing dep.version would also let through a retargeted executable classifier copy, which the legacy repository doesn't serve either.
  • may_interpolate_to rejects an unterminated ${. Maven reads an unterminated ${ literally, so "${g" can never be the GA. The reactor is right here; VEX and hosted only over-refuse.
  • The type→copy mapping is repeated in three places. This is a refactor. The one parity bug it caused (2) is fixed.
  • Eager PomIndex combination work and PomDep clones in bypasses(). These are performance.

Coherence check requested by the owner, read on the merged stack:

  • The rewriter writes and rolls back through the pom-spelled version.
  • Discovery compares the spelled and build-resolved versions, and a .mvn/maven.config -D that moves the pin off is MavenConfigOverride.
  • Hosted refuses a live ${property} version outright.
  • Profile moves (MavenProfileUnpatched) and classifier copies are weighed on build-resolved versions in both discovery and the hosted index.

No incoherence found.

Tests on 37d9fd9: cargo test -p socket-patch-core --lib -- maven vex:: patch::redirect:: passes 1964, redirect_golden passes, CLI --lib vex_sources passes, and the CLI vex, e2e_vex, cli_parse_vex and vendor_jvm_cli tests pass. Workspace clippy and fmt are clean.


Note

Medium Risk
Changes hosted Maven redirect and VEX attestation logic in lockfile-writing paths—fail-closed (fewer pins written) but affects build wiring decisions for profile-heavy POMs.

Overview
Maven POM resolution under profiles now reads groupId, artifactId, and version together per active profile combination, recording coordinate_alternatives when profile <properties> retarget coordinates.

Hosted scan (scan --mode hosted) stops writing pins that VEX would not attest:

  • redirect_maven_profile_coordinates_unpatched when a ${…} coordinate a profile redefines would move the pin off the GA or turn another declaration into the patched artifact (including executable classifier copies such as test-jar + explicit classifier).
  • Existing classifier/profile codes now refuse the dep (no pom.xml write) when an executable classifier or a profile declaration/management can resolve any version other than the suffixed pin—not only when they name the base release.

CLI contract and VEX notes document the stricter hosted behavior and extend vex_maven_profile_unpatched messaging for coordinate property redefinition (vendored mode no longer suggested as a fix when profiles bypass the pin).

Core parsing (formats/maven, PomIndex) and redirect tests back the new fail-closed checks; behavior stacks on the shared profile-resolution work from the parent branch.

Reviewed by Cursor Bugbot for commit a8541ab. Configure here.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: merged the rewritten base (#2020 was force-pushed to ec89bd9) into this branch at 9ce83ae. Conflicts resolved against the old base d7b4b40, so only this PR's change was re-applied on top of the base's new structure:

  • coordinate_override now runs from bypasses after profile_override (the base replaced the old declaration_override), and reads build_version and is_executable_classifier like the base's checks.
  • coordinate_alternatives' no-profile baseline calls lookup(name, &[], true) (user properties included), matching lookup_any.
  • CLI_CONTRACT.md and the profile note keep both sides' wording.

Local: cargo check core+cli clean; cargo test -p socket-patch-core --lib maven 361/361, including this PR's four regression tests. Still a draft, so it stays unlabeled.


Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) force-pushed the fix/maven-discovery-coordinate-overrides branch from 9ce83ae to 2b06722 Compare November 5, 2026 19:16
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Nov 6, 2026
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>
@mikolalysenko
Mikola Lysenko (mikolalysenko) force-pushed the fix/maven-discovery-coordinate-overrides branch from 2b06722 to e458517 Compare November 6, 2026 05:00
…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>
…rrides' into fix/maven-discovery-coordinate-overrides

# Conflicts:
#	crates/socket-patch-core/src/vex/discover/maven.rs
…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
Second review round on Maven coordinate overrides.

FALSE ATTESTATION: a profile-retargeted declaration with both a non-jar
`<type>` and an explicit `<classifier>` (`test-jar` + `tests`) was
dropped by both readers' `retargeted` (its match fell to "any other
type: not weighed"), so hosted scan wrote the pin and VEX attested it
while the active profile put the unpatched tests jar on the classpath.
An explicit classifier now names the copy whatever the type, as it does
for a literal declaration. The contract also states that the vendored
reactor reports a retargeted classifier copy as `classifier_declared`
with its root still pinned.

Co-Authored-By: Claude Opus 5.5 <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>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: merged the base fix/maven-discovery-property-overrides (84967b1) into this branch as 74fa073 to clear the merge conflicts.

The base had picked up main's Maven variant refactor (#2006), where a <type> such as test-jar counts like the classifier it implies. Conflicts were in redirect/mod.rs, maven_reactor.rs, vex/discover/maven.rs and CLI_CONTRACT.md, and both sides are kept. One semantic follow-up was needed: MavenIndex::unserved_classifier_copy now weighs every executable variant, typed ones included, with management keyed by classifier and type. Without that, the base's maven_executable_classifier_variant_blocks_the_redirect test (a test-jar copy) failed.

Local run: cargo test -p socket-patch-core --lib -- formats::maven vex::discover:: patch::redirect:: vendor::jvm gives 1666 passed. The 1 failure is the vlt_heal chmod test, which root ignores. CLI lib tests pass (865).

Worth a reviewer's look: retargeted() in discover still drops a retargeted declaration whose type is neither jar, test-jar nor explicitly classified (for example ejb), while the base's variant rule counts such types as executable.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


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.

Stale Bugbot comment from a previous run.

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
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


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.

Stale Bugbot comment from a previous run.

…-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>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


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 a8541ab. Configure here.

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>
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review November 9, 2026 20:52

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[agent] Review findings (Claude Opus + codex, independent passes):

  1. Retargeted executable variants are dropped (crates/socket-patch-core/src/vex/discover/maven.rs:898; same in patch/redirect/maven_index.rs:280 and vendor/jvm/maven_reactor.rs:2318; Opus). retargeted() discards a profile-moved reading for any <type> other than jar/test-jar/${…}/explicit classifier — including ejb, ejb-client, maven-plugin, bundle, which is_executable_variant (formats/maven/mod.rs:984-1000) treats as code. Input: hosted pin org.slf4j:slf4j-api:1.7.36-socket.<hex> plus ${g}:slf4j-api:1.7.30 with <type>ejb</type>, root g=org.other, profile p sets g=org.slf4j → scan pins and vex attests, while -Pp puts unpatched slf4j-api-1.7.30 on the classpath. The literal-coordinates version of the same pom is withheld. Suggested fix: map retargeted types through is_variant/is_executable_variant in all three places instead of a hand-written allowlist.
  2. Management precedence ignored in hosted retargeted() (maven_index.rs:298, codex; fail-safe). Direct slf4j-api:1.7.36 plus managed ${g}:slf4j-api:1.7.30 with a profile retargeting g makes hosted scan write nothing, though the direct version wins under every profile; discovery already excludes it.

Minor: hosted unserved_classifier_copy matches only literal coordinates while discovery uses may_name (hosted pins, VEX withholds forever).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[agent] Approving per maintainer decision; see the findings in the review above.

Base automatically changed from fix/maven-discovery-property-overrides to fix/maven-discovery-false-attestation November 9, 2026 21:15
@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit 37d9fd9 into fix/maven-discovery-false-attestation Nov 9, 2026
80 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the fix/maven-discovery-coordinate-overrides branch November 9, 2026 21:15
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Not merged to main. GitHub marked this merged because the stack was folded: fix/maven-discovery-false-attestation was fast-forwarded to this PR's head. The whole Maven stack is now reviewed and landed as #2005.

Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Nov 9, 2026
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>
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Nov 9, 2026
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>
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.

3 participants