[Mikola Lysenko (mikolalysenko)]

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

Fixes #683

Summary

A Maven dependency declared through a property coordinate, such as
<groupId>${ct.group}</groupId> with ct.group defined in the pom's own
<properties>, was never recognised as the patched package.
scan --mode hosted then added a <dependencyManagement> pin. The direct
literal <version>1.10.0</version> overrides that pin, so Maven built
Central's unpatched jar. Scan still reported the dep as redirected, and both
the in-run and standalone vex attested not_affected.

The <exclusions>-first half of #683 was already fixed on main: the
hosted matcher reads through PomScope, which blanks <exclusions>. A
regression case for it is included here.

Root cause

formats::maven::PomScope::dependencies() read groupId / artifactId as
raw text. Every reader built on it missed the declaration: the hosted
rewriter (maven_index::PomIndex), the VEX reader (parse_pom, which
resolved ${prop} only in <version>), and the upstream restore.

Fix

• One shared resolution. PomScope now interpolates groupId /
  artifactId from the root <properties>. That covers properties defined
  through other properties and the project model refs (project.*,
  pom.*, project.parent.*, the parent's coordinates when inherited).
  Model refs win over same-named properties, as in Maven. The rewriter
  pins the direct literal, VEX reads the real GA, and rollback finds the
  pin it wrote.
• Fail closed on what this pom cannot resolve. A versioned main-jar
  declaration whose coordinate is a property the pom does not define (a
  parent's) may be the patched GA, and then its own version wins. The
  shared rule is formats::maven::may_shadow. An unresolved reference acts
  as a wildcard, and the literal text around it must still match, so
  ${project.artifactId}-core never shadows slf4j-api. pom-type and
  classifier declarations never count as the main jar.
  • Hosted: the dep is refused with nothing written, under the new warning
    redirect_maven_coordinate_unresolved (documented in CLI_CONTRACT.md
    and docs/ecosystems.md). A <profiles> declaration of that kind is
    reported as redirect_maven_profile_dependency_unpatched, the same as a
    literal one.
  • VEX: the declaration's version is folded into the GA's Maven precedence
    (ga_versions), so the existing "Maven resolves …" check refuses the
    hosted pin. The vendored conflict check still attests a declaration at
    the vendored version.
• Rollback of earlier wiring. A <dependencyManagement> pin that an
  earlier release added beside such a declaration (the Hosted Maven adds a dependencyManagement pin that the direct literal overrides when the dependency's groupId/artifactId is a ${property} or <exclusions> come first, so the build stays unpatched while scan reports redirected and VEX attests not_affected #683 bug itself) is
  still recognised as the rewriter's own and is removed. The authored
  check now counts suffixed declarations, not all versioned ones. Restore
  also reads the pom scope once per pin instead of twice.

Per-issue checklist

• Hosted Maven adds a dependencyManagement pin that the direct literal overrides when the dependency's groupId/artifactId is a ${property} or <exclusions> come first, so the build stays unpatched while scan reports redirected and VEX attests not_affected #683 variant A (property groupId) and variant B (property
  artifactId), hosted:
  maven_pom_property_coordinates_are_the_direct_declaration
• Hosted Maven adds a dependencyManagement pin that the direct literal overrides when the dependency's groupId/artifactId is a ${property} or <exclusions> come first, so the build stays unpatched while scan reports redirected and VEX attests not_affected #683 variant C (<exclusions> first), hosted: same test
  (already fixed on main; regression case)
• Hosted Maven adds a dependencyManagement pin that the direct literal overrides when the dependency's groupId/artifactId is a ${property} or <exclusions> come first, so the build stays unpatched while scan reports redirected and VEX attests not_affected #683 VEX attests the shadowed pin:
  property_coordinates_are_the_declared_ga,
  undefined_coordinate_property_is_not_attested_past, and the vendored
  case in vendored_ga_pinned_to_another_version_is_invalid
• Undefined coordinate property, hosted:
  maven_pom_undefined_coordinate_property_refuses_the_dep,
  maven_pom_undefined_coordinate_property_in_a_profile_is_reported
• Rollback round trip, plus a pre-Hosted Maven adds a dependencyManagement pin that the direct literal overrides when the dependency's groupId/artifactId is a ${property} or <exclusions> come first, so the build stays unpatched while scan reports redirected and VEX attests not_affected #683 pin:
  property_coordinate_pin_round_trips,
  pre_683_managed_pin_beside_a_property_declaration_is_removed
• Resolution rules: formats::maven::tests::coordinate_references

Test evidence

• Red→green: with the fix neutralised, all 5 initial regression tests fail
  (maven_pom_property_coordinates_are_the_direct_declaration,
  maven_pom_undefined_coordinate_property_refuses_the_dep,
  property_coordinate_pin_round_trips,
  property_coordinates_are_the_declared_ga,
  undefined_coordinate_property_is_not_attested_past), and with it they
  pass. pre_683_managed_pin_beside_a_property_declaration_is_removed
  fails with the old authored-entry count and passes with the new one.

• cargo fmt --all -- --check: clean.

• cargo clippy --workspace --all-features -- -D warnings: clean.

• cargo test -p socket-patch-core --all-features: 7523 passed, 4 failed.
  The 4 failures are permission-based tests that cannot fail as root
  (uid 0 in this container) and are unrelated to Maven:
  copy_tree::relax_loop_must_not_traverse_symlinked_root,
  vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,
  pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched
  and pypi_requirements::wire_failure_rolls_back_already_written_files.
  The core integration suites (redirect_golden and
  upstream_restore_golden among them) passed 655/655.

• socket-patch-cli, a subset (not the whole crate: a full
  cargo test --workspace filled the container's disk):

  • lib, contract_gradle_codes, e2e_vex_redirect, e2e_maven and
    cli_parse_vex: 1020 passed, 0 failed.
  • in_process_redirect, run on an earlier revision: 149 passed,
    3 failed. The 3 are the same kind of write-failure tests that need a
    non-root user.

• Real Maven 3.9 (--ignored): e2e_redirect_maven_build and
  e2e_vendor_maven_build both pass.

• /code-review high: 10 findings. Nine are fixed in 026c536. One is not
  changed:

  • A hosted pin shadowed by an undefined-coordinate declaration is
    dropped from discovery rather than kept as an unattested ref. This
    matches how VEX already treats a pin shadowed by a literal direct
    version (managed_pin_shadowed_by_a_direct_plain_version_is_invalid):
    DIAG_REF_INVALID, with the ledger claim recognised as dead.
    Changing that is a broader VEX policy question than Hosted Maven adds a dependencyManagement pin that the direct literal overrides when the dependency's groupId/artifactId is a ${property} or <exclusions> come first, so the build stays unpatched while scan reports redirected and VEX attests not_affected #683.

• Bugbot (026c536), both findings fixed in 453253e: a project coordinate
  written through a property is interpolated, not inherited from the
  parent, and `` is not read as the project's own
  coordinates; an unresolved `` may still be `jar`. CI was green on
  026c536 (82 checks).

• Review (Tanmay182003): `.mvn/maven.config` `-D` user properties override pom
  properties in Maven, but the coordinate lookup ignored them. Fixed in
  09ac691: hosted, `vex` and restore read them over the pom's, using the
  reader the vendored reactor already had (moved to `formats::maven`).
  Tests: `maven_pom_coordinate_properties_read_maven_config_overrides`,
  `coordinate_properties_read_maven_config_overrides`.

Follow-ups

None. The TypeScript twin in depscan cannot be reached from here. If
depscan's hosted Maven rewriter shares this matcher, it needs the same
property resolution.

https://claude.ai/code/session_011f9aeQkgxbNChxvpC4fAEF

---

Note

Medium Risk
Changes Maven hosted rewrite, VEX attestation, and rollback matching for property coordinates—build-critical lockfile/POM behavior—but stays fail-closed with broad regression tests rather than widening pins silently.

Overview
Fixes #683: hosted Maven treated dependencies whose groupId / artifactId used ${…} as missing matches, added a <dependencyManagement> pin, and still reported redirect/VEX success while Maven built the unpatched jar from the direct declaration.

Shared POM reading now interpolates coordinates from the pom’s <properties>, project model refs (project.*, parent inheritance), and .mvn/maven.config -D overrides (centralized in formats::maven, reused by vendored reactor). Defined properties pin the literal dependency line; undefined parent-style placeholders are handled with may_shadow / may_name so the tool fails closed instead of pinning through a shadowing declaration.

Hosted scan --mode hosted skips writes and emits redirect_maven_coordinate_unresolved (or profile redirect_maven_profile_dependency_unpatched) when an unresolved coordinate may be the GA. VEX discovery folds those shadow versions into GA precedence so pins are not attested past them. Rollback/upstream restore finds property-coordinate pins and removes legacy pre-fix <dependencyManagement> entries beside property literals.

Contract/docs (CLI_CONTRACT.md, docs/ecosystems.md) document the new refusal; extensive unit tests cover rewrite, VEX, rollback, and maven.config overrides.

^{Reviewed by Cursor Bugbot for commit 09ac691. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Ready for review at 453253e (burn-down agent).

• CI: 50/50 check runs on 453253e green or skipped (31 success, 19 skipped).
• Bugbot: reviewed 453253e, no new issues; both earlier threads on formats/maven/mod.rs resolved.
• Reviewer focus: PomScope::dependencies() now resolves ${prop} in groupId/artifactId, which feeds the hosted rewriter, the VEX reader and upstream restore at once.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final reviewer: pushed 27ca653, which caps Maven coordinate interpolation at 1 KB.

interpolate (formats/maven/mod.rs) re-expanded the whole string for up to 8 rounds with no size limit, so a pom whose properties each name the previous one many times grows exponentially (a "billion laughs"). A 1.2 KB pom took 32 s to build a 102 MB groupId; one more level (1.4 KB) was OOM-killed at about 10 GB. PomScope::new reaches this through model_refs, so vex, the hosted rewrite and rollback on any checked-out repo were exposed. A round that would exceed the cap is now not taken, so the reference stays unresolved, and may_shadow already treats that as fail-closed. New test: exponential_property_expansion_is_capped. cargo test -p socket-patch-core --all-features --lib maven: 307 passed.

Not fixed here: crawlers/maven_scope.rs interpolate_at on main has the same uncapped pattern (depth 16), so it's worth a follow-up.

Removed Ready for review; the burn-down re-labels this once CI is green.

bugbot run

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Ready for review at 27ca653 (burn-down agent).

• CI: all check runs on 27ca653 green or skipped (30 success, 19 skipped, plus Cursor Bugbot success).
• Bugbot: reviewed 27ca653 (the 1 KB interpolation cap commit), check passed, both review threads resolved.
• Reviewer focus: the ${property} interpolation and its length cap in formats/maven/mod.rs.
• Slack announcement not sent this run (Slack send unavailable to the agent).

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. Fixes #683. A Maven dependency written as <groupId>${prop}</groupId> (or artifactId) with the property defined in the pom was never matched, so hosted redirect added a <dependencyManagement> pin that the direct literal version overrode, and vex still attested not_affected. PomScope now interpolates coordinates from the root <properties> and the project model (project.*, parent.*), and every reader (hosted rewriter, VEX, rollback) shares that. A versioned declaration whose coordinate uses a property the pom does not define is treated as possibly the patched GA: hosted refuses the dep (redirect_maven_coordinate_unresolved) and VEX folds its version into Maven's precedence.

Risk: medium. It changes Maven matching in three readers, but the new rule fails closed (an unknown property can only block a pin or an attestation, never add one), and interpolation is capped at 8 rounds and 1 KB.

Look here

• formats/maven/mod.rs:309 interpolate and the size cap
• formats/maven/mod.rs:362 model_refs: which project and parent coordinates are bound, and when they are inherited
• formats/maven/mod.rs:422 may_name / may_shadow: the shared fail-closed rule
• redirect/mod.rs:8756 the hosted refusal and the profile warning
• upstream/maven.rs:193 rollback now counts suffixed declarations, so a pin an older release added beside a property declaration is removed

Verified

• Read the full diff (+840/−20, 7 files). CHANGELOG.md is untouched. The new warning code is documented in CLI_CONTRACT.md and docs/ecosystems.md.
• Locally: cargo test -p socket-patch-core --all-features --lib -- maven redirect vex gave 1824 passed, 1 failed. The failure is vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, a permission test that can't fail as root in this container and has nothing to do with Maven.
• CI on 27ca653: ci-ok and clippy green. Bugbot passed. No unresolved threads.
• An earlier final-review pass found and fixed exponential property expansion in 27ca653 ("billion laughs": a 1.4 KB pom OOM'd at ~10 GB).

Changes I made: none this pass.

Open questions (not blocking)

1. Fail-closed refusal: a versioned dep like ${parent.g}:slf4j-api with an undefined property now blocks the hosted pin for slf4j-api and reports redirect_maven_coordinate_unresolved. That's intended, but those projects will see "not redirected" where they used to get a (broken) pin.
2. Follow-up, not in this PR: crawlers/maven_scope.rs interpolate_at on main has the same uncapped expansion pattern.
3. Nit: in formats/maven/mod.rs tests, the /// #683: ... doc comment now sits on exponential_property_expansion_is_capped and not on coordinate_references. Cosmetic only, so I didn't push for it.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final reviewer: I turned auto-merge off. The review brief covers 27ca653, and 09ac691 ("Honour maven.config -D in Maven coordinates") changes code after it. That commit was pushed less than 45 minutes ago, so I'll review it in a later run and post a new brief. Auto-merge goes back on then if nothing blocks.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (updated for 09ac691)

What it does. Fixes #683. A Maven dependency written as <groupId>${prop}</groupId> (or artifactId) used to be matched literally, so hosted redirect pinned it through a <dependencyManagement> entry that the direct version overrode, and vex still attested it. PomScope now resolves coordinates from <properties> and the project model, and fails closed on a property the pom doesn't define. The new commit 09ac691 also reads .mvn/maven.config -D user properties over the pom's own, as Maven does, in the hosted rewriter, VEX and rollback.

Risk: medium. Maven matching changes in three readers. An undefined property can only block a pin or an attestation. A maven.config -D can now enable a match the pom alone doesn't define, which is how Maven resolves it.

Look here

• formats/maven/mod.rs:90 with_user_properties: user properties go over pom properties at L108
• formats/maven/mod.rs:481 maven_config_properties, moved unchanged from vendor/jvm/maven_reactor.rs so all readers share it
• redirect/mod.rs:8595 hosted rewriter reads root .mvn/maven.config
• upstream/maven.rs:310 rollback, and vex/discover/maven.rs:133 VEX
• Earlier parts covered by the 27ca653 brief: interpolate and its size cap, model_refs, may_name / may_shadow

Verified

• Re-read the full PR and the new commit 09ac691 (+209/−80, 6 files). The move of the maven.config parser is verbatim, and the only new logic is the merge order. CHANGELOG.md is untouched.
• Locally on 09ac691: cargo test -p socket-patch-core --lib -- maven: 309 passed, 0 failed. That includes the new rewriter and VEX tests, which cover both directions (an override that names another group, and one that defines a missing property).
• CI on 09ac691: ci-ok and clippy green, including hosted-e2e and the Maven e2e leg. All 3 review threads are resolved.
• Bugbot: no check run on 09ac691 yet, despite @bugbot review at 21:31. It passed on 27ca653.

Changes I made: none.

Open questions (not blocking)

1. Only .mvn/maven.config is read. A -D passed on the mvn command line in CI is invisible to socket-patch, so a coordinate overridden only that way still matches the pom's value. This was already true before this PR.
2. Still applies from the earlier brief: the fail-closed refusal (redirect_maven_coordinate_unresolved) makes some projects that used to get a (broken) pin show "not redirected".

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

✅ 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 09ac691. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: Ready for review confirmed at 09ac691. The previous ready comment covered 27ca653.

• CI: 50/50 check runs on 09ac691 green or skipped (31 success, 19 skipped). Mergeable, no conflicts, and no main-wide failures.
• Bugbot: reviewed 09ac691 and found no new issues. No unresolved review threads.
• For reviewers: 09ac691 adds support for .mvn/maven.config -D properties when resolving ${prop} coordinates. Auto-merge is still armed from the final reviewer.
• Slack: not announced this run, because posting to Slack wasn't available.

---

Generated by Claude Code