Skip to content

Withhold Maven pins whose coordinates a profile redefines - #2026

Draft
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
fix/maven-discovery-property-overridesfrom
fix/maven-discovery-coordinate-overrides
Draft

Withhold Maven pins whose coordinates a profile redefines#2026
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
fix/maven-discovery-property-overridesfrom
fix/maven-discovery-coordinate-overrides

Conversation

@mikolalysenko

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

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.

The gap

Coordinates were only interpolated leniently, from the root <properties> with .mvn/maven.config user properties laid over them. 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: interpolate_each_keeping is interpolate_each with the lenient semantics. An undefined name, a cycle, a chain deeper than 16 levels, or an unterminated ${ stays as written, and may_name then reads it as any text. It returns None only when the work bounds are exceeded. PomDep::coordinate_alternatives holds every (groupId, artifactId) pair a declaration may name under any profile or combination of profiles. A profile's own declaration reads that profile's properties first. The pair the declaration reads with no profile active is excluded. User properties and project.* refs stay fixed. ScopedDep.group / artifact, which the hosted rewriter also reads, are unchanged.
  • vex/discover/maven: profile_override also runs coordinate_override. It reuses UnattestedKind::MavenProfileUnpatched, whose detail now names the coordinate redefinition. The CLI note and CLI_CONTRACT.md are updated.
  • vendor/jvm/maven_reactor: the v5 planner had the same gap. coordinates_elsewhere goes through inheritors and every profile on the chain, one at a time, like version_override.
    • (a) The existing coordinates_overridden check now covers profiles as well as inheritors.
    • (b) The new warn_moved_coordinates handles a declaration that names another artifact here but the GA under a profile or in an inheritor. It warns, and when the declaration has a version it also unpins the root and marks it uncertain, so attestable_checked no longer attests it.

Tests

Each of these 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

These cover the new API: formats::maven::tests::{keeping_interpolation_leaves_unresolved_references, profile_properties_give_coordinate_alternatives}.

The guard test is unrelated_coordinate_properties_do_not_withhold. The user-property variants (a chain through -D, and -D beating the profile) are inside each test. The golden corpus is unchanged.

Not done

  • The hosted rewriter (patch/redirect/maven_index.rs) still reads coordinates leniently, with no profile awareness. It can still pin a declaration that a profile moves, but discovery now withholds that pin.
  • The alternatives over-approximate on purpose. Each coordinate, and the version, is enumerated independently, so a profile that moves both coordinates and the version onto the pin together still withholds.

Generated by Claude Code

@mikolalysenko @claude
Resolve Maven pin properties through profiles and nesting
d7b4b40 
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 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 marks a live declaration that resolves the
   pin but has an alternative off it unattested with the existing
   UnattestedKind::MavenProfileUnpatched (detail names the redefinition;
   CLI note and CLI_CONTRACT.md updated), in hosted and the pre-v5
   vendored wiring alike. A <profiles> declaration's own ${v} now reads
   its own profile's <properties> first, and every version it may take
   is checked against the pin.

2. Property versions resolved one level deep. ${a} with <a>${b}</a> now
   resolves through every property it names, user properties first at
   each level, with the vendored reactor's limits: 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.

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)
- 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>
@mikolalysenko @claude
Withhold Maven pins whose coordinates a profile redefines
b9cad3c 
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 through the lenient interpolator, from the
root <properties> (with .mvn/maven.config user properties over them),
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.

formats/maven: interpolate_each_keeping is interpolate_each with the
lenient reading's semantics (an undefined name, cycle, chain past 16
levels or unterminated ${ stays as written, so may_name reads it as any
text; None only past the work bounds). PomDep now carries
coordinate_alternatives: every (groupId, artifactId) pair the
declaration may name under any profile or set of profiles (its own
profile's properties first for a profile declaration), minus what it
reads with no profile active. User properties and project.* model
references stay fixed, as for versions. ScopedDep.group / artifact
(the lenient base the hosted rewriter also reads) are unchanged; they
gain the raw texts.

vex/discover/maven: profile_override also runs coordinate_override,
which marks the ref unattested with the existing MavenProfileUnpatched
for (a) a live main-jar declaration at the pin with any alternative,
and (b) any declaration with an alternative that may name the GA (main
jar of a jar-ish type via may_name, or literally an executable
classifier copy) at a version other than the pin, or a classifier copy
with no version. A main jar without a version takes the live pin and
does not withhold. Applies to hosted and the pre-v5 vendored wiring.
CLI note and CLI_CONTRACT.md updated.

vendor/jvm/maven_reactor: the v5 vendored planner had the same gap.
coordinates_elsewhere enumerates inheritors and every profile on the
chain (one at a time, as version_override does). (a) The existing
coordinates_overridden check now weighs profiles too, not only
inheritors: a declaration a profile moves off the GA is left as is and
its root unpinned. (b) New warn_moved_coordinates: a declaration that
names another artifact in its pom but the GA in an inheritor or under a
profile 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), which was likewise unseen.

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
New API: formats::maven::tests::{keeping_interpolation_leaves_unresolved_references,
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) mentioned this pull request Nov 4, 2026
Draft
@mikolalysenko
Mikola Lysenko (mikolalysenko) force-pushed the fix/maven-discovery-property-overrides branch from d7b4b40 to ec89bd9 Compare November 4, 2026 21:39
@claude
Merge rewritten #2020 base into coordinate-override branch
9ce83ae 
The base branch was force-pushed (review fixes added; MavenConfigOverride
split out of profile overrides, build_version beside version, classifier
handling moved into unpatched_classifier_copy). Resolved against the old
base commit d7b4b40 so only this PR's changes are re-applied:

- coordinate_override now runs from bypasses after profile_override, and
  reads build_version (as the base's live-declaration checks do) and
  is_executable_classifier instead of the removed list.
- coordinate_alternatives reads the no-profile baseline with user
  properties (lookup's new build flag), matching lookup_any.
- CLI_CONTRACT and the profile note keep both sides' wording.

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

Mikola Lysenko (mikolalysenko) commented Nov 4, 2026

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mikolalysenko @claude