From d7b4b40657222ccb74a3bbf4b90a122842c2659a Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 11:58:11 -0600 Subject: [PATCH 1/2] Resolve Maven pin properties through profiles and nesting Closes the two Maven false-attestation gaps #2005 left open in VEX discovery. 1. Profile redefining a pin's property. A live ${v} that the root resolve to the Socket pin resolves something else whenever an active profile's 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 , 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 declaration's own ${v} now reads its own profile's first, and every version it may take is checked against the pin. 2. Property versions resolved one level deep. ${a} with ${b} 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 --- crates/socket-patch-cli/CLI_CONTRACT.md | 4 +- .../src/commands/vex_sources.rs | 10 +- .../src/formats/maven/mod.rs | 495 ++++++++++++++++-- .../src/vendor/jvm/maven_reactor.rs | 75 +-- .../src/vex/discover/maven.rs | 353 +++++++++++-- .../socket-patch-core/src/vex/discover/mod.rs | 7 +- 6 files changed, 797 insertions(+), 147 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 62afd8b7a..7458c44df 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -547,7 +547,7 @@ Recognition rules that hold for every ecosystem: * **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above. * **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget ``, pom ``, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `` is diagnosed, never a reference. * **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. The root `requirements.txt` and its in-root `-r` includes count as one lock here: pip reads them as one requirement set, where a direct reference wins over a compatible `==` pin of the same version in another file of the set (#1086). A PyPI version matches under PEP 440 equality, so a requirements line pinned in any spelling that can only install the wired release (`==1.2.3.0`, `==v1.2.3`, `===1.2.3`) contests it like `==1.2.3` does; a range or wildcard (`~=1.2.3`, `==1.2.*`) does not (#1602). A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`, beneath a dependency that bundles it; the root project's own bundles install from the lock, #1618) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. yarn classic's `yarn.lock` records nothing about bundled copies either (#758), so for a lock with a Socket wiring they are found in the installed `node_modules`: each package a parent's `bundleDependencies` / `bundledDependencies` names inside the parent's own `node_modules`, the dependencies of those beside them, and everything nested under them. Such a copy contests the reference in `yarn.lock` (the diagnostic names the parent's lock entry and the copy's directory); a lock-only `vex` with no `node_modules` can't see it. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`, and a yarn classic hosted scan names one with `redirect_copy_unpatched`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. A reference withheld only because such an unreachable copy installs beside it (a bundled npm / Bun / vlt copy, an npm copy beneath a `hasShrinkwrap` package (#753) or one npm installs from a git / url / `file:` spec, or a yarn classic git or `file:` directory block of the same `name@version`) is still the rewriter's own wiring of exactly one package version: it is never attested, but `rollback`, `remove`, `list` and the hosted → vendored takeover treat it as an ordinary hosted pin and restore its upstream registry entry (#828), rather than refusing it with `hosted_wiring_contested`. Another entry of the **same** npm, Bun or yarn lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. -* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Five cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **Maven profile declarations** (`vex_maven_profile_unpatched`): a `` declaration in the root `pom.xml` of a pinned GA — its main jar in `` or ``, or an executable classifier copy — at any version other than the pin (the base release, another release, a `${property}` the pom does not resolve, an executable classifier copy with no version of its own) overrides the pin whenever that profile is active, so the reference is omitted. Activation is not evaluated: every profile counts, as in vendored mode, which rewrites profile declarations too; hosted scan leaves them as written (`redirect_maven_profile_dependency_unpatched`). A profile declaration without a `` takes the live pin and does not withhold the statement. The same holds for a pre-v5 single-pom vendored repository, which serves only the original version. +* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Five cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **Maven profile declarations** (`vex_maven_profile_unpatched`): a `` declaration in the root `pom.xml` of a pinned GA — its main jar in `` or ``, or an executable classifier copy — at any version other than the pin (the base release, another release, a `${property}` the pom does not resolve, an executable classifier copy with no version of its own) overrides the pin whenever that profile is active, so the reference is omitted. Activation is not evaluated: every profile counts, as in vendored mode, which rewrites profile declarations too; hosted scan leaves them as written (`redirect_maven_profile_dependency_unpatched`). A profile declaration without a `` takes the live pin and does not withhold the statement. A pin spelled through a property counts the same way: a `${property}` `` is interpolated through every property it names (user properties from `.mvn/maven.config` first, then the root ``; at most 16 levels deep, and a cycle or an undefined name never resolves to a pin), and when any profile's `` — or several profiles' together — redefine one of those properties so that it resolves anything other than the pin (or nothing), the reference is omitted too; a user property wins over every profile's. A profile declaration reads its own profile's `` first. The same holds for a pre-v5 single-pom vendored repository, which serves only the original version. * **Shrinkwrap without a twin (#899).** npm >= 12 never reads `npm-shrinkwrap.json`: on a project whose only npm lock is the shrinkwrap it resolves the tree from the registry and writes a fresh `package-lock.json`, so a Socket reference there reaches npm <= 11 only. The reference is still discovered, so `list`, `rollback` and `remove` manage it, but `vex` omits the patch (`vex_npm_shrinkwrap_only`, as a run warning and as the `failed[].reason`). A `package-lock.json` beside it (wired the same way) makes it attestable again. Hosted and vendored runs still rewire the shrinkwrap and warn `redirect_npm_shrinkwrap_only` / `vendor_npm_shrinkwrap_only`. * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). @@ -2678,7 +2678,7 @@ match it, withholds the statement: | `vex_deno_lock_copy` | run warning and `failed[].reason` | An npm pin is wired, but `deno.lock` locks the same `name@version`, which `deno install` installs without the wiring; no statement while it does. `vendor --check` and `scan` still treat the wiring as live. | | `vex_maven_reactor_root` | run warning and `failed[].reason` | A hosted Maven pin sits in a root `pom.xml` that declares `` / `` (a profile's included), where a module's own literal `` overrides it; only the root pom is read, so no statement (#261). `list`, `rollback` and `remove` still find the pin. | | `vex_maven_classifier_unpatched` | run warning and `failed[].reason` | A hosted Maven pin's GA also has a live executable `` copy (not `sources` / `javadoc`) at a version other than the pin, which the classifier-less pin does not retarget and the grant does not serve; no statement while it does. `list`, `rollback` and `remove` still find the pin. | -| `vex_maven_profile_unpatched` | run warning and `failed[].reason` | A Maven pin's GA is also declared inside `` in the root `pom.xml` at a version other than the pin (or as an executable classifier copy off it), which overrides the pin whenever that profile is active; no statement while it does. `list`, `rollback` and `remove` still find the pin. | +| `vex_maven_profile_unpatched` | run warning and `failed[].reason` | A Maven pin's GA is also declared inside `` in the root `pom.xml` at a version other than the pin (or as an executable classifier copy off it), or a profile's `` redefine a property the pin's `${property}` version names so it resolves another version, which overrides the pin whenever that profile is active; no statement while it does. `list`, `rollback` and `remove` still find the pin. | | `vex_gradle_derived_cache_unchecked` | run warning | The derived-cache walk was cut short (a very large transforms cache); the statement is still emitted, the copies the walk did reach were checked. | A vendored Gradle entry attests only while its wiring is live (apply line, index rows, diff --git a/crates/socket-patch-cli/src/commands/vex_sources.rs b/crates/socket-patch-cli/src/commands/vex_sources.rs index b1de1f94b..5bc2cca93 100644 --- a/crates/socket-patch-cli/src/commands/vex_sources.rs +++ b/crates/socket-patch-cli/src/commands/vex_sources.rs @@ -233,7 +233,8 @@ const NOTE_MAVEN_REACTOR_ROOT: &str = "vex_maven_reactor_root"; /// (`vex::Unattested`). const NOTE_MAVEN_CLASSIFIER_UNPATCHED: &str = "vex_maven_classifier_unpatched"; /// Omission tag and note: a hosted Maven pin's GA is also declared inside -/// `` at another version, which overrides the pin whenever that +/// `` at another version, or a profile's `` move +/// the pin's `${property}` version, which overrides the pin whenever that /// profile is active (`vex::Unattested`). const NOTE_MAVEN_PROFILE_UNPATCHED: &str = "vex_maven_profile_unpatched"; @@ -274,9 +275,10 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) { ), UnattestedKind::MavenProfileUnpatched => ( NOTE_MAVEN_PROFILE_UNPATCHED, - "not attested while a profile declares it at another version; the wiring itself is \ - intact, so re-running `scan` does not change this (drop that profile or \ - set it to the pin, or re-patch with `scan --mode vendored`)", + "not attested while a profile declares it at, or its move it to, \ + another version; the wiring itself is intact, so re-running `scan` does not change \ + this (drop that profile or property or set it to the pin, or re-patch \ + with `scan --mode vendored`)", ), } } diff --git a/crates/socket-patch-core/src/formats/maven/mod.rs b/crates/socket-patch-core/src/formats/maven/mod.rs index 7b63b0179..f994c63c3 100644 --- a/crates/socket-patch-core/src/formats/maven/mod.rs +++ b/crates/socket-patch-core/src/formats/maven/mod.rs @@ -2,7 +2,7 @@ //! [`parse_pom`]), the Maven coordinate grammars and the hosted //! `-socket.` version split. Pure: text in, structure out. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use super::xml::{blank_non_markup, child_text, elements, open_tags, Element}; @@ -41,10 +41,21 @@ pub(crate) struct PomRepo { pub(crate) struct PomDep { pub(crate) group: String, pub(crate) artifact: String, - /// Trimmed literal version, `${prop}` resolved (one level) from the - /// build's user properties, else the root ``, when - /// possible; `None` when the declaration has none. + /// Trimmed version, every `${prop}` interpolated + /// ([`interpolate_strict`]) from the build's user properties, the + /// project model, the `` of the profile holding the + /// declaration (if any), then the root ``; as written when + /// that fails (undefined, cyclic, too deep); `None` when the + /// declaration has none. pub(crate) version: Option, + /// The trimmed `` text as written. + pub(crate) version_text: Option, + /// The other versions [`Self::version`] becomes when some set of + /// profiles is active whose `` redefine a property it + /// names (transitively): a superset ([`interpolate_each`]), plus the + /// text as written when some such set leaves it unresolved. Empty when + /// no profile can move it. + pub(crate) version_alternatives: Vec, /// The non-empty ``: a sibling artifact of the GA (sources, /// tests, a native build), managed and resolved apart from the main jar. pub(crate) classifier: Option, @@ -55,6 +66,10 @@ pub(crate) struct PomDep { pub(crate) in_profile: bool, } +/// One `` inside `` (`name -> trimmed value`), with +/// the `` element that holds it when the scan finds one. +type ProfileProperties = (Option, BTreeMap); + /// Where Maven reads a root pom's model from: its text with comments and /// CDATA blanked (offsets kept), and the sections that are not the /// project's own resolution. `` / `` hold plugin @@ -69,11 +84,18 @@ pub(crate) struct PomScope { ignored: Vec, profiles: Vec, /// The root `` (see [`properties`]) with the build's user - /// properties laid over them: what a `${name}` resolves to. - properties: BTreeMap, - /// [`Self::properties`] plus the `${project.*}` model references a - /// coordinate may name (see [`model_refs`]). + /// properties laid over them, plus the `${project.*}` model references + /// (see [`model_refs`]): what a `${name}` resolves to while no profile + /// is active. coordinate_refs: BTreeMap, + /// The names no profile's `` can redefine: the build's user + /// properties and the `project.*` / `pom.*` model references, which + /// Maven looks up first. + fixed: BTreeSet, + /// Each `` inside ``, with the `` that + /// holds it (when the scan finds one): merged over the root's while + /// that profile is active. + profile_properties: Vec, } impl PomScope { @@ -105,27 +127,95 @@ impl PomScope { ] .concat(); let profiles = elements(&masked, "profiles")?; - let mut properties = properties(&masked, &ignored, &profiles)?; - properties.extend(user.iter().map(|(k, v)| (k.clone(), v.clone()))); - let mut coordinate_refs = properties.clone(); + let mut coordinate_refs = properties(&masked, &ignored, &profiles)?; + coordinate_refs.extend(user.iter().map(|(k, v)| (k.clone(), v.clone()))); + let mut fixed: BTreeSet = user.keys().cloned().collect(); // A `project.*` / `pom.*` reference names the model, whatever a // same-named property says; the deprecated bare form yields to one. for (name, value) in model_refs(&masked, &coordinate_refs.clone())? { if name.starts_with("project.") || name.starts_with("pom.") { + fixed.insert(name.clone()); coordinate_refs.insert(name, value); } else { coordinate_refs.entry(name).or_insert(value); } } + let profile_properties = profile_properties(&masked, &ignored, &profiles)?; Ok(PomScope { masked, ignored, profiles, - properties, coordinate_refs, + fixed, + profile_properties, }) } + /// The indices into [`Self::profile_properties`] of the profile that + /// holds `pos` (none outside one). + fn own_profile(&self, pos: usize) -> Vec { + (0..self.profile_properties.len()) + .filter(|&i| { + self.profile_properties[i] + .0 + .is_some_and(|p| pos >= p.start && pos < p.end) + }) + .collect() + } + + /// What `${name}` reads while only the profile `own` is active (if + /// any): a user property or model reference, else that profile's + /// ``, else the root's. + fn lookup(&self, name: &str, own: &[usize]) -> Option { + if !self.fixed.contains(name) { + if let Some(v) = own + .iter() + .find_map(|&i| self.profile_properties[i].1.get(name)) + { + return Some(v.clone()); + } + } + self.coordinate_refs.get(name).cloned() + } + + /// Every value `${name}` may read while `own` and any other profiles + /// are active: [`Self::lookup`]'s, then every other profile's + /// definition. Empty when [`Self::lookup`] finds none: some set of + /// profiles leaves it undefined. + fn lookup_any(&self, name: &str, own: &[usize]) -> Vec { + let Some(base) = self.lookup(name, own) else { + return Vec::new(); + }; + let mut values = vec![base]; + if !self.fixed.contains(name) { + values.extend( + self.profile_properties + .iter() + .enumerate() + .filter(|(i, _)| !own.contains(i)) + .filter_map(|(_, (_, props))| props.get(name).cloned()), + ); + } + values + } + + /// The `` text of the declaration at `pos`, resolved as + /// [`PomDep::version`] and [`PomDep::version_alternatives`] describe. + fn resolve_version(&self, text: &str, pos: usize) -> (String, Vec) { + if !text.contains("${") { + return (text.to_string(), Vec::new()); + } + let own = self.own_profile(pos); + let Some(base) = interpolate_strict(text, &|name| self.lookup(name, &own)) else { + return (text.to_string(), Vec::new()); + }; + let alternatives = match interpolate_each(text, &|name| self.lookup_any(name, &own)) { + Some(all) => all.into_iter().filter(|v| *v != base).collect(), + None => vec![text.to_string()], + }; + (base, alternatives) + } + /// Inside a plugin / deploy section (never project resolution). pub(crate) fn is_ignored(&self, pos: usize) -> bool { inside(pos, &self.ignored) @@ -245,12 +335,19 @@ pub(crate) fn parse_pom_with(raw: &str, user: &BTreeMap) -> Resu let (Some(group), Some(artifact)) = (dep.group, dep.artifact) else { continue; }; + let (version, version_alternatives) = match &dep.version_text { + Some(text) => { + let (version, alternatives) = scope.resolve_version(text, dep.element.start); + (Some(version), alternatives) + } + None => (None, Vec::new()), + }; pom.deps.push(PomDep { group, artifact, - version: dep - .version_text - .map(|v| resolve_property(&v, &scope.properties)), + version, + version_text: dep.version_text, + version_alternatives, classifier: dep.classifier, type_text: dep.type_text, managed: inside(dep.element.start, &managed), @@ -277,40 +374,175 @@ fn properties( { continue; } - let body = props.inner(text); - let mut from = 0; - while let Some(rel) = body[from..].find('<') { - let lt = from + rel; - let Some(gt) = body[lt..].find('>').map(|r| lt + r) else { - return Err("unterminated tag in ".to_string()); - }; - let name = &body[lt + 1..gt]; - if !is_property_name(name) { - from = gt + 1; - continue; - } - let close = format!(""); - let Some(end) = body[gt + 1..].find(&close).map(|r| gt + 1 + r) else { - return Err(format!("unterminated <{name}> in ")); - }; - map.entry(name.to_string()) - .or_insert_with(|| body[gt + 1..end].trim().to_string()); - from = end + close.len(); - } + read_properties(props.inner(text), &mut map)?; } Ok(map) } -/// `${name}` → the property's value (one level, no recursion); anything -/// else, or an undefined property, is returned unchanged. -fn resolve_property(version: &str, properties: &BTreeMap) -> String { - version - .strip_prefix("${") - .and_then(|rest| rest.strip_suffix('}')) - .filter(|name| is_property_name(name)) - .and_then(|name| properties.get(name)) - .cloned() - .unwrap_or_else(|| version.to_string()) +/// Each `` inside `` (outside ignored sections) as +/// `name -> trimmed value`, with the `` element that holds it. +fn profile_properties( + text: &str, + ignored: &[Element], + profiles: &[Element], +) -> Result, String> { + let live = |pos: usize| inside(pos, profiles) && !inside(pos, ignored); + // A `` no `` is found around (a scan this simple + // cannot place) belongs to no profile: it is then only ever an + // alternative, never a declaration's own, which reads more versions + // into [`PomDep::version_alternatives`], not fewer. + let holders: Vec = elements(text, "profile") + .unwrap_or_default() + .into_iter() + .filter(|p| live(p.start)) + .collect(); + let mut out = Vec::new(); + for props in elements(text, "properties")? { + if !live(props.start) { + continue; + } + let holder = holders + .iter() + .find(|p| props.start >= p.start && props.start < p.end) + .copied(); + let mut map = BTreeMap::new(); + read_properties(props.inner(text), &mut map)?; + out.push((holder, map)); + } + Ok(out) +} + +/// The `name -> trimmed value` pairs of a `` body, into `map` +/// (a name's first definition wins). +fn read_properties(body: &str, map: &mut BTreeMap) -> Result<(), String> { + let mut from = 0; + while let Some(rel) = body[from..].find('<') { + let lt = from + rel; + let Some(gt) = body[lt..].find('>').map(|r| lt + r) else { + return Err("unterminated tag in ".to_string()); + }; + let name = &body[lt + 1..gt]; + if !is_property_name(name) { + from = gt + 1; + continue; + } + let close = format!(""); + let Some(end) = body[gt + 1..].find(&close).map(|r| gt + 1 + r) else { + return Err(format!("unterminated <{name}> in ")); + }; + map.entry(name.to_string()) + .or_insert_with(|| body[gt + 1..end].trim().to_string()); + from = end + close.len(); + } + Ok(()) +} + +/// Nesting depth past which [`interpolate_strict`] gives up: a property +/// named through more than this many others (Maven itself fails on a +/// cycle, which this bounds too). +pub(crate) const MAX_INTERPOLATION_DEPTH: usize = 16; + +/// Most distinct results [`interpolate_each`] enumerates before giving up. +const MAX_RESOLUTIONS: usize = 16; + +/// Most lookups one interpolation makes before giving up: a value naming +/// the next property many times over would otherwise cost exponentially +/// many with every level ("billion laughs"). +const MAX_LOOKUPS: usize = 1024; + +/// Longest text [`interpolate_each`] builds before giving up. +const MAX_RESOLVED_LEN: usize = 4096; + +/// `value` with every `${name}` replaced by `lookup(name)`, itself +/// interpolated the same way, as Maven's model interpolator resolves a +/// property defined through another. `None` when a name is undefined, a +/// `${` is unterminated, or the nesting is cyclic or deeper than +/// [`MAX_INTERPOLATION_DEPTH`] (or the work bounds are hit): an unresolved +/// value never stands for a version. Shared by the vendored reactor +/// (its `lookup` walks the parent chain) and the VEX reader +/// ([`PomScope`]). +pub(crate) fn interpolate_strict( + value: &str, + lookup: &dyn Fn(&str) -> Option, +) -> Option { + interpolate_each(value, &|name| lookup(name).into_iter().collect())? + .into_iter() + .next() +} + +/// Every text `value` may interpolate to when each `${name}` may take any +/// of the values `lookup(name)` gives (an empty list: undefined), each +/// interpolated in turn ([`interpolate_strict`] with a single value per +/// name). A name that occurs twice may take a different value each time, +/// so the set may hold texts no single assignment builds: a superset, +/// which only ever withholds an attestation. `None` when any choice leaves +/// a name undefined, a `${` is unterminated, a name is reached through +/// itself (a cycle) or through more than [`MAX_INTERPOLATION_DEPTH`] +/// others, or the result or the work exceeds its bound. +pub(crate) fn interpolate_each( + value: &str, + lookup: &dyn Fn(&str) -> Vec, +) -> Option> { + let lookups = std::cell::Cell::new(MAX_LOOKUPS); + expand(value, lookup, &mut Vec::new(), &lookups) +} + +fn expand( + value: &str, + lookup: &dyn Fn(&str) -> Vec, + stack: &mut Vec, + lookups: &std::cell::Cell, +) -> Option> { + if stack.len() > MAX_INTERPOLATION_DEPTH { + return None; + } + let mut texts = BTreeSet::from([String::new()]); + let mut rest = value; + loop { + let (literal, reference) = match rest.find("${") { + Some(at) => (&rest[..at], Some(&rest[at + 2..])), + None => (rest, None), + }; + let Some(after) = reference else { + return concat(&texts, &BTreeSet::from([literal.to_string()])); + }; + let close = after.find('}')?; + let name = &after[..close]; + if stack.iter().any(|n| n == name) { + return None; + } + lookups.set(lookups.get().checked_sub(1)?); + let raws = lookup(name); + if raws.is_empty() { + return None; + } + stack.push(name.to_string()); + let mut values = BTreeSet::new(); + for raw in &raws { + values.extend(expand(raw, lookup, stack, lookups)?); + } + stack.pop(); + let prefixed = concat(&texts, &BTreeSet::from([literal.to_string()]))?; + texts = concat(&prefixed, &values)?; + rest = &after[close + 1..]; + } +} + +/// Every `a + b`, or `None` past [`MAX_RESOLUTIONS`] / [`MAX_RESOLVED_LEN`]. +fn concat(heads: &BTreeSet, tails: &BTreeSet) -> Option> { + let mut out = BTreeSet::new(); + for head in heads { + for tail in tails { + if head.len() + tail.len() > MAX_RESOLVED_LEN { + return None; + } + out.insert(format!("{head}{tail}")); + if out.len() > MAX_RESOLUTIONS { + return None; + } + } + } + Some(out) } /// Interpolation rounds before a still-unresolved reference is given up on @@ -767,6 +999,179 @@ mod tests { ); } + /// A `${property}` version is interpolated through every property it + /// names, user properties first at each level; a cycle, an undefined + /// name or a chain deeper than [`super::MAX_INTERPOLATION_DEPTH`] + /// leaves it as written. + #[test] + fn version_properties_resolve_recursively_and_fail_closed() { + let version = |props: &str, user: &[(&str, &str)], text: &str| { + let pom = format!( + "{props}\ + ga\ + {text}" + ); + let user = user + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + let dep = super::parse_pom_with(&pom, &user).unwrap().deps.remove(0); + assert!(dep.version_alternatives.is_empty(), "{props}"); + dep.version.unwrap() + }; + // `${p1}` ... `1.0`. + let chain = |n: usize| -> String { + (0..n) + .map(|i| match i + 1 == n { + true => format!("1.0"), + false => format!("${{p{}}}", i + 1), + }) + .collect() + }; + assert_eq!( + version("${b}${c}1.0", &[], "${a}"), + "1.0" + ); + assert_eq!( + version("${b}1.0", &[("b", "2.0")], "${a}"), + "2.0" + ); + assert_eq!(version("10", &[], "${a}.${b}-x"), "1.0-x"); + assert_eq!( + version("", &[], "${project.version}"), + "${project.version}", + "no project version here" + ); + assert_eq!(version(&chain(16), &[], "${p0}"), "1.0"); + assert_eq!(version(&chain(17), &[], "${p0}"), "${p0}", "too deep"); + for cyclic in [ + "${a}", + "${b}${a}", + "x${b}${a}y", + ] { + assert_eq!(version(cyclic, &[], "${a}"), "${a}", "{cyclic}"); + } + assert_eq!(version("${b}", &[], "${a}"), "${a}", "undefined"); + assert_eq!(version("1", &[], "${a"), "${a", "unterminated"); + } + + /// The versions a profile's `` may move a declaration to: + /// every profile, and every set of profiles active together, counts; a + /// declaration inside a profile reads that profile's own first; user + /// properties and model references no profile can redefine. + #[test] + fn profile_properties_give_version_alternatives() { + let resolve = |props: &str, profiles: &[&str], user: &[(&str, &str)], deps: &str| { + let profiles: String = profiles + .iter() + .map(|p| format!("{p}")) + .collect(); + let pom = format!( + "9{props}\ + {deps}{profiles}" + ); + let user = user + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + super::parse_pom_with(&pom, &user) + .unwrap() + .deps + .into_iter() + .map(|d| (d.version.unwrap(), d.version_alternatives)) + .collect::>() + }; + let dep = |v: &str| { + format!( + "ga\ + {v}" + ) + }; + let one = |props, profiles: &[&str], user: &[(&str, &str)]| { + resolve(props, profiles, user, &dep("${v}")).remove(0) + }; + let s = |v: &[&str]| v.iter().map(|v| v.to_string()).collect::>(); + assert_eq!(one("1", &["2"], &[]), ("1".into(), s(&["2"]))); + assert_eq!(one("1", &["1"], &[]), ("1".into(), s(&[]))); + assert_eq!(one("1", &["2"], &[]), ("1".into(), s(&[]))); + assert_eq!( + one("${w}1", &["2", "3"], &[]), + ("1".into(), s(&["2", "3"])) + ); + // Two profiles that move it only together. + assert_eq!( + one("11", &["${w}", "2"], &[]), + ("1".into(), s(&["2"])) + ); + // A profile value this pom cannot resolve, or a cycle. + assert_eq!( + one("1", &["${nope}"], &[]), + ("1".into(), s(&["${v}"])) + ); + assert_eq!( + one("1", &["${v}"], &[]), + ("1".into(), s(&["${v}"])) + ); + // User properties and model references win over every profile's. + assert_eq!( + one("1", &["2"], &[("v", "3")]), + ("3".into(), s(&[])) + ); + assert_eq!( + resolve( + "", + &["2"], + &[], + &dep("${project.version}") + ) + .remove(0), + ("9".into(), s(&[])) + ); + // More alternatives than are enumerated: unresolved. + let many: Vec = (0..17).map(|i| format!("{i}")).collect(); + let many: Vec<&str> = many.iter().map(String::as_str).collect(); + assert_eq!(one("x", &many, &[]), ("x".into(), s(&["${v}"]))); + + // A profile's own declaration reads its own properties first. + let pom = format!( + "1{}\ + a2\ + {}\ + b3", + dep("${v}"), + dep("${v}") + ); + let deps: Vec<_> = super::parse_pom(&pom) + .unwrap() + .deps + .into_iter() + .map(|d| (d.in_profile, d.version.unwrap(), d.version_alternatives)) + .collect(); + assert_eq!( + deps, + [ + (false, "1".to_string(), s(&["2", "3"])), + (true, "2".to_string(), s(&["3"])), + ] + ); + } + + /// The shared interpolator's work bounds: a value naming the next + /// property many times over fails instead of growing exponentially. + #[test] + fn interpolation_is_bounded() { + let props: std::collections::BTreeMap = (0..12) + .map(|i| (format!("p{i}"), format!("${{p{}}}", i + 1).repeat(8))) + .chain([("p12".to_string(), "x".to_string())]) + .collect(); + let lookup = |name: &str| props.get(name).cloned(); + assert_eq!(super::interpolate_strict("${p0}", &lookup), None); + assert_eq!( + super::interpolate_strict("${p11}", &lookup).as_deref(), + Some("xxxxxxxx") + ); + } + #[test] fn suffix_grammar_is_exact() { assert_eq!( diff --git a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs index 42a853fff..8b8e9b6cc 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -11,7 +11,9 @@ use std::ops::Range; use serde_json::{json, Value}; -use crate::formats::maven::maven_config_properties as cli_properties; +use crate::formats::maven::{ + interpolate_strict, maven_config_properties as cli_properties, MAX_INTERPOLATION_DEPTH, +}; use super::super::state::{WiringAction, WiringRecord}; use super::layout::{self, safe_coordinates}; @@ -51,8 +53,6 @@ const NESTED_MVN_FILES: &[&str] = &[ const SHAPE_UNSUPPORTED: &str = "vendor_jvm_shape_unsupported"; const UPSTREAM_UNAVAILABLE: &str = "vendor_jvm_upstream_unavailable"; const DEGRADED: &str = "vendor_jvm_degraded"; -/// Property interpolation depth cap (Maven itself fails on cycles). -const MAX_INTERPOLATION_DEPTH: usize = 16; /// Whether the pom declares a `` or `` of the project /// or of a profile (plugin configuration, comments and CDATA do not count). @@ -737,7 +737,7 @@ fn restore_versions( let resolve = |v: &str| { reactor .filter(|r| r.poms.contains_key(rel)) - .and_then(|r| r.interpolate(rel, v, 0)) + .and_then(|r| r.interpolate(rel, v)) }; let mut edits = Vec::new(); for (dep, key) in doc.keyed_declarations(c.group_id, c.artifact_id, &resolve) { @@ -1055,30 +1055,6 @@ struct ExternalPom { pom: Pom, } -/// `${name}` interpolation through `lookup`; `None` when a name is -/// undefined or the nesting is too deep. -fn interpolate_by( - value: &str, - lookup: &dyn Fn(&str) -> Option, - depth: usize, -) -> Option { - if depth > MAX_INTERPOLATION_DEPTH { - return None; - } - let mut out = String::new(); - let mut rest = value; - while let Some(at) = rest.find("${") { - out.push_str(&rest[..at]); - let after = &rest[at + 2..]; - let close = after.find('}')?; - let raw = lookup(&after[..close])?; - out.push_str(&interpolate_by(&raw, lookup, depth + 1)?); - rest = &after[close + 1..]; - } - out.push_str(rest); - Some(out) -} - impl Reactor { /// Management of `patch`'s g:a that a pin in local root `root` would /// override and that does not manage it at the patch's base version: @@ -1120,7 +1096,7 @@ impl Reactor { }) }; let resolve = |value: &str, what: &str| -> Result { - interpolate_by(value, &props, 0).ok_or_else(|| { + interpolate_strict(value, &props).ok_or_else(|| { Missing::Unresolved(format!("{rel}: {what} {value} is not defined")) }) }; @@ -1207,8 +1183,7 @@ impl Reactor { ) -> Result, Missing> { let mut chain: Vec = Vec::new(); let mut parent = self.poms[root].parent.as_ref().map(|p| { - let local = - |v: &Option| v.as_deref().and_then(|v| self.interpolate(root, v, 0)); + let local = |v: &Option| v.as_deref().and_then(|v| self.interpolate(root, v)); (local(&p.group), local(&p.artifact), local(&p.version)) }); while let Some((pg, pa, pv)) = parent { @@ -1374,7 +1349,7 @@ fn bom_manages<'b>( chain.iter().find_map(|e| e.pom.props.get(name).cloned()) }; let resolve = |value: &str| -> Result { - interpolate_by(value, &props, 0).ok_or_else(|| { + interpolate_strict(value, &props).ok_or_else(|| { Missing::Unresolved(format!( "the BOM {}:{}:{} manages {g}:{a} at {value}, which it does not define", bom.0, bom.1, bom.2 @@ -1729,13 +1704,13 @@ impl Reactor { fn keyed_declarations(&self, rel: &str, g: &str, a: &str) -> Vec<(usize, String)> { self.poms[rel] .doc - .keyed_declarations(g, a, &|v| self.interpolate(rel, v, 0)) + .keyed_declarations(g, a, &|v| self.interpolate(rel, v)) } /// Interpolate every `${name}` in `value` in the context of `rel`; /// `None` when any name is undefined in the checkout. - fn interpolate(&self, rel: &str, value: &str, depth: usize) -> Option { - self.interpolate_with(rel, value, depth, None) + fn interpolate(&self, rel: &str, value: &str) -> Option { + self.interpolate_with(rel, value, None) } /// [`Self::interpolate`] with `overlay`'s properties (a profile's) @@ -1744,24 +1719,9 @@ impl Reactor { &self, rel: &str, value: &str, - depth: usize, overlay: Option>, ) -> Option { - if depth > MAX_INTERPOLATION_DEPTH { - return None; - } - let mut out = String::new(); - let mut rest = value; - while let Some(at) = rest.find("${") { - out.push_str(&rest[..at]); - let after = &rest[at + 2..]; - let close = after.find('}')?; - let raw = self.lookup_with(rel, &after[..close], overlay)?; - out.push_str(&self.interpolate_with(rel, &raw, depth + 1, overlay)?); - rest = &after[close + 1..]; - } - out.push_str(rest); - Some(out) + interpolate_strict(value, &|name| self.lookup_with(rel, name, overlay)) } /// User properties win over model properties, as in Maven; then the @@ -1827,17 +1787,14 @@ impl Reactor { // has redefined something it names: that moves it too. let undefined = || "an undefined value".to_string(); let plain = (reader != rel) - .then(|| { - self.interpolate(reader, declared, 0) - .unwrap_or_else(undefined) - }) + .then(|| self.interpolate(reader, declared).unwrap_or_else(undefined)) .filter(|v| moved(v)) .map(|v| (v, reader.to_string())); plain.or_else(|| { self.chain(reader).find_map(|at| { self.poms[at].profile_props.iter().find_map(|(id, props)| { let v = self - .interpolate_with(reader, declared, 0, Some((at, props))) + .interpolate_with(reader, declared, Some((at, props))) .unwrap_or_else(undefined); moved(&v).then(|| (v, format!("{reader} under profile {id} of {at}"))) }) @@ -1897,7 +1854,7 @@ impl Reactor { // literal SV must not reach. if doc.has_property_coordinate(dep) { let other = self.inheritors(rel).find(|p| { - let resolve = |v: &str| self.interpolate(p, v, 0); + let resolve = |v: &str| self.interpolate(p, v); doc.coordinate(dep, "groupId", &resolve).as_deref() != Some(patch.group_id) || doc.coordinate(dep, "artifactId", &resolve).as_deref() != Some(patch.artifact_id) @@ -1924,7 +1881,7 @@ impl Reactor { }; let declared = doc.text_of(version); let resolved = if declared.contains("${") { - match self.interpolate(rel, &declared, 0) { + match self.interpolate(rel, &declared) { Some(v) => v, None => { warnings.push(degraded( @@ -2012,7 +1969,7 @@ impl Reactor { uncertain: &mut BTreeSet, ) { let doc = &self.poms[rel].doc; - let resolve = |v: &str| self.interpolate(rel, v, 0); + let resolve = |v: &str| self.interpolate(rel, v); for dep in doc.declarations() { let (Some(g), Some(a)) = ( doc.child_text(dep, "groupId"), diff --git a/crates/socket-patch-core/src/vex/discover/maven.rs b/crates/socket-patch-core/src/vex/discover/maven.rs index 8500bbc98..2b32bd5a8 100644 --- a/crates/socket-patch-core/src/vex/discover/maven.rs +++ b/crates/socket-patch-core/src/vex/discover/maven.rs @@ -54,9 +54,12 @@ //! any version other than the pin: the rewriter leaves it as written, and //! whenever that profile is active it overrides the pin (vendored mode //! treats every profile as live and rewrites it too). A `${property}` -//! version is resolved one level from `.mvn/maven.config`'s `-D` user -//! properties, else the root `` (Maven reads user properties -//! first). +//! version is interpolated through every property it names (the vendored +//! reactor's interpolator: at most 16 deep, a cycle never resolves), each +//! read from `.mvn/maven.config`'s `-D` user properties, else the root +//! `` (Maven reads user properties first). One that resolves +//! the pin but that any profile's (or set of profiles') `` +//! make resolve anything else keeps the ref unattested the same way. //! //! Integrity: when the pom sha256 was known the rewriter also writes Maven //! Trusted Checksums — `.mvn/checksums/checksums.sha256` lines @@ -99,7 +102,8 @@ //! (plugin classpaths) are not project resolution and are skipped silently; //! `` are only read when activated, so a Socket repository or pin //! there is diagnosed, never a ref, and a profile declaration of a pinned -//! GA at another version withholds the pin's attestation. Submodule poms +//! GA at another version, or a profile's `` that move the +//! pin's `${property}`, withholds the pin's attestation. Submodule poms //! and parent poms are not read (root-only). Gradle builds are discovered //! by `gradle.rs` (the hosted index) and the vendored JVM backend's own //! probes. @@ -775,39 +779,25 @@ fn unpatched_executable_classifier<'a>( }) } -/// Why a `` declaration of `group:artifact` moves it off -/// `pinned` whenever that profile is active, for the first such one: a -/// main-jar declaration (or one an undefined coordinate property may make -/// the main jar, [`may_shadow`]) at another version, or an executable -/// classifier copy that does not name `pinned` itself. A profile's -/// `` and `` merge over the pom's own, -/// so either overrides the pin; activation is not evaluated, so every -/// profile counts, as in vendored mode. A main-jar declaration without a -/// version takes the live one and moves nothing. +/// Why a profile moves `group:artifact` off `pinned` whenever it is +/// active, for the first such declaration (activation is not evaluated, so +/// every profile counts, as in vendored mode, and so does every set of +/// profiles active together): +/// +/// * a `` declaration: a main-jar one (or one an undefined +/// coordinate property may make the main jar, [`may_shadow`]) at another +/// version, or an executable classifier copy that does not name `pinned` +/// itself. A profile's `` and `` +/// merge over the pom's own, so either overrides the pin. A main-jar +/// declaration without a version takes the live one and moves nothing. +/// * a live declaration that resolves `pinned` through a property some +/// profile's `` redefine (or one it names), which then +/// resolves another version ([`PomDep::version_alternatives`]): Maven +/// merges an active profile's properties over the pom's own before it +/// interpolates. Vendored mode refuses to pin over such a property. fn profile_override(deps: &[PomDep], group: &str, artifact: &str, pinned: &str) -> Option { - deps.iter().filter(|d| d.in_profile).find_map(|d| { + deps.iter().find_map(|d| { let literal = d.group == group && d.artifact == artifact; - let what = match d.classifier.as_deref() { - None if literal - || may_shadow( - (&d.group, &d.artifact), - d.type_text.as_deref(), - None, - (group, artifact), - ) => - { - let version = d.version.as_deref().filter(|v| *v != pinned)?; - format!("{version}") - } - Some(c) if literal && !NON_EXECUTABLE_CLASSIFIERS.contains(&c) => { - match d.version.as_deref() { - Some(v) if v == pinned => return None, - Some(v) => format!("classifier {c} at {v}"), - None => format!("classifier {c} with no of its own"), - } - } - _ => return None, - }; let section = if d.managed { " " } else { @@ -821,6 +811,52 @@ fn profile_override(deps: &[PomDep], group: &str, artifact: &str, pinned: &str) d.group, d.artifact ) }; + // Every version the declaration may resolve, the base one first. + let mut versions = d.version.iter().chain(&d.version_alternatives); + let main = d.classifier.is_none() + && (literal + || may_shadow( + (&d.group, &d.artifact), + d.type_text.as_deref(), + None, + (group, artifact), + )); + let classifier = d + .classifier + .as_deref() + .filter(|c| literal && !NON_EXECUTABLE_CLASSIFIERS.contains(c)); + if !main && classifier.is_none() { + return None; + } + if !d.in_profile { + // The base version is the live one: a declaration off the pin + // there is diagnosed by the version grouping, not here. + if d.version.as_deref() != Some(pinned) { + return None; + } + let moved = d.version_alternatives.first()?; + let declared = d.version_text.as_deref().unwrap_or_default(); + let copy = classifier.map_or(String::new(), |c| format!(" classifier {c}")); + return Some(format!( + "{POM}: {section}{named}{copy} {declared} resolves the \ + patched {pinned} from the root , but a \ + redefinition of a property it names makes it {moved}; whenever that profile \ + is active Maven resolves it instead of the pin" + )); + } + let what = match classifier { + None => { + let version = versions.find(|v| *v != pinned)?; + format!("{version}") + } + Some(c) => match d.version { + None => format!("classifier {c} with no of its own"), + Some(_) => { + let v = versions.find(|v| *v != pinned)?; + format!("classifier {c} at {v}") + } + }, + }; Some(format!( "{POM}: a {section}declaration of {named} carries {what}, not the \ patched {pinned}; whenever that profile is active Maven resolves it instead of \ @@ -1693,6 +1729,223 @@ mod tests { } } + /// Whether `out` attests `purl`: a ref no unattested entry withholds. + fn attests(out: &Discovery, purl: &str) -> bool { + out.refs.iter().any(|r| { + r.purl == purl + && !out + .unattested + .iter() + .any(|u| u.purl == r.purl && u.uuid == r.uuid) + }) + } + + /// A hosted project pinning slf4j-api through `decl` (a + /// `` / `` block), with the root + /// `` `props` and one `` per entry of `profiles` + /// (each a profile body). + fn property_project(props: &str, decl: &str, profiles: &[&str]) -> Project { + let repo = hosted_repo(&format!("socket-patch-{UUID_A}"), ®istry_url(UUID_A)); + let profiles: String = profiles + .iter() + .enumerate() + .map(|(i, body)| format!("p{i}\n{body}\n")) + .collect(); + let p = Project::new(); + p.write( + "pom.xml", + pom(&format!( + "{props}\n{decl}\n{repo}\n\ + \n{profiles}\n" + )), + ); + p + } + + fn direct_decl(version: &str) -> String { + format!( + "\n{}\n", + dep("org.slf4j", "slf4j-api", Some(version)) + ) + } + + /// A pin spelled through a property is what Maven resolves only while + /// no active profile's `` redefines that property (or one + /// it names) to anything else: Maven merges an active profile's + /// properties over the pom's own before it interpolates the + /// ``. Vendored mode refuses to pin over such a property + /// (`version_override`), so the ref stays (rollback / remove / list + /// find it) but is never attested. User properties + /// (`.mvn/maven.config`) win over every profile's. + #[tokio::test] + async fn profile_properties_redefining_a_pin_property_are_not_attested() { + let pin = suffixed(UUID_A); + let v = |value: &str| format!("{value}"); + let w = |value: &str| format!("{value}"); + let in_props = |s: String| format!("{s}"); + let direct = direct_decl("${slf4j.version}"); + let managed = format!( + "\n{}\n\ + \n{}\n", + dep("org.slf4j", "slf4j-api", Some("${slf4j.version}")), + dep("org.slf4j", "slf4j-api", None), + ); + let withheld = [ + // The base release. + (v(&pin), &direct, vec![in_props(v("1.7.36"))]), + (v(&pin), &managed, vec![in_props(v("1.7.36"))]), + // Through a property the pin's property names. + (v("${w}") + &w(&pin), &direct, vec![in_props(w("1.7.30"))]), + // A value this pom cannot resolve, or a cycle. + (v(&pin), &direct, vec![in_props(v("${undefined}"))]), + (v(&pin), &direct, vec![in_props(v("${slf4j.version}"))]), + // Two profiles that move the pin only together. + ( + v(&pin) + &w(&pin), + &direct, + vec![in_props(v("${w}")), in_props(w("1.7.36"))], + ), + ]; + for (root, decl, profiles) in &withheld { + let profiles: Vec<&str> = profiles.iter().map(String::as_str).collect(); + let out = run(&property_project(root, decl, &profiles)).await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + assert_eq!( + out.unattested.len(), + 1, + "{root} {profiles:?}: {:?}", + out.unattested + ); + let u = &out.unattested[0]; + assert_eq!( + u.kind, + UnattestedKind::MavenProfileUnpatched, + "{profiles:?}" + ); + assert!( + u.detail.contains("org.slf4j:slf4j-api") && u.detail.contains(""), + "{}", + u.detail + ); + } + + for (root, profile, config) in [ + // The pin itself, or an unrelated property. + (v(&pin), in_props(v(&pin)), None), + (v(&pin), in_props("1".to_string()), None), + (v("${w}") + &w(&pin), in_props(w(&pin)), None), + // A user property wins over every profile's. + ( + v(&pin), + in_props(v("1.7.36")), + Some(format!("-Dslf4j.version={pin}\n")), + ), + ] { + let p = property_project(&root, &direct, &[&profile]); + if let Some(config) = &config { + p.write(".mvn/maven.config", config); + } + let out = run(&p).await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + assert!( + attests(&out, FX_PURL), + "{root} {profile}: {:?}", + out.unattested + ); + } + } + + /// A `` declaration's `${property}` version reads its own + /// profile's `` first: that profile is active whenever the + /// declaration applies. + #[tokio::test] + async fn profile_declaration_reads_its_own_profile_properties() { + let pin = suffixed(UUID_A); + let profile = |value: &str| { + format!( + "{value}\n\ + \n{}\n", + dep("org.slf4j", "slf4j-api", Some("${slf4j.version}")) + ) + }; + let direct = direct_decl(&pin); + for (root, own, attested) in [ + // The root resolves the pin, the profile the base release. + (pin.as_str(), "1.7.36", false), + // The root resolves the base release, the profile the pin. + ("1.7.36", pin.as_str(), true), + ] { + let p = property_project( + &format!("{root}"), + &direct, + &[&profile(own)], + ); + let out = run(&p).await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + assert_eq!( + attests(&out, FX_PURL), + attested, + "{own}: {:?}", + out.unattested + ); + } + } + + /// A `${property}` version resolves through every property it names, + /// with user properties first at each level, as Maven interpolates; a + /// cycle, or a chain deeper than the vendored reactor follows, is + /// never a pin. + #[tokio::test] + async fn version_property_resolves_through_nested_properties() { + let pin = suffixed(UUID_A); + // `${p1}` ... `{end}`. + let chain = |links: usize, end: &str| -> String { + (0..links) + .map(|i| { + let value = if i + 1 == links { + end.to_string() + } else { + format!("${{p{}}}", i + 1) + }; + format!("{value}") + }) + .collect() + }; + let split = format!( + "${{p1}}-socket.${{p2}}1.7.36{}", + &UUID_A[..8] + ); + let user_pin = format!("-Dp1={pin}\n"); + for (props, config, attested) in [ + (chain(2, &pin), None, true), + (chain(3, &pin), None, true), + (split, None, true), + // The deepest chain the vendored reactor resolves, and one more. + (chain(16, &pin), None, true), + (chain(17, &pin), None, false), + // A user property redefines a nested one. + (chain(2, &pin), Some("-Dp1=1.7.36\n"), false), + (chain(2, "1.7.36"), Some(user_pin.as_str()), true), + // A cycle. + ("${p1}${p0}".to_string(), None, false), + ("${p0}".to_string(), None, false), + ] { + let p = property_project(&props, &direct_decl("${p0}"), &[]); + if let Some(config) = config { + p.write(".mvn/maven.config", config); + } + let out = run(&p).await; + assert_eq!( + attests(&out, FX_PURL), + attested, + "{props} {config:?}: {:?} {:?} {:?}", + out.refs, + out.unattested, + out.diagnostics + ); + } + } + /// A versioned declaration whose coordinate is a property this pom /// does not define may be the pinned GA, so the pin is not attested /// past it (#683). @@ -1942,6 +2195,36 @@ mod tests { } } + /// The same for a profile's `` that move a `${property}` + /// version of the vendored GA off the vendored version. + #[tokio::test] + async fn vendored_ref_under_profile_properties_is_not_attested() { + for (version, attested) in [("1.10.1", false), ("1.10.0", true)] { + let p = vendored_project(UUID_A); + let wired = std::fs::read_to_string(p.root().join("pom.xml")) + .expect("read pom") + .replace("1.10.0", "${ct}"); + let at = wired.rfind("").expect("project close"); + p.write( + "pom.xml", + format!( + "{}1.10.0\np\ + {version}\n{}", + &wired[..at], + &wired[at..] + ), + ); + let out = run(&p).await; + assert_refs(&out, &[(V_PURL, UUID_A, WiringMode::Vendored)]); + assert_eq!( + attests(&out, V_PURL), + attested, + "{version}: {:?}", + out.unattested + ); + } + } + #[tokio::test] async fn vendored_url_spellings() { for spelled in [ diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 1e663b93a..efb377513 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -609,8 +609,11 @@ pub enum UnattestedKind { /// A Maven pin (hosted, or the pre-v5 single-pom vendored repository) /// whose GA a `` declaration of the root pom gives another /// version (the base release, another release, an - /// unresolved `${property}`): whenever that profile is active it - /// overrides the pin, so the build resolves the unpatched jar. + /// unresolved `${property}`), or whose `${property}` version a + /// profile's `` redefine (directly or through a property + /// it names) to another or an unresolvable version: whenever that + /// profile is active it overrides the pin, so the build resolves the + /// unpatched jar. MavenProfileUnpatched, } From b9cad3ccc92d97411c96ee571417b2c2c4cbbb83 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 14:23:15 -0600 Subject: [PATCH 2/2] Withhold Maven pins whose coordinates a profile redefines Closes the last Maven property false-attestation gap in VEX discovery after #2020: a / written as ${prop} that a profile's redefine. Coordinates were read only through the lenient interpolator, from the root (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 --- crates/socket-patch-cli/CLI_CONTRACT.md | 4 +- .../src/commands/vex_sources.rs | 8 +- .../src/formats/maven/mod.rs | 334 ++++++++++- .../src/vendor/jvm/maven_reactor.rs | 312 ++++++++++- .../src/vex/discover/maven.rs | 525 ++++++++++++++++-- 5 files changed, 1087 insertions(+), 96 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 7458c44df..b38e96b01 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -547,7 +547,7 @@ Recognition rules that hold for every ecosystem: * **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above. * **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget ``, pom ``, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `` is diagnosed, never a reference. * **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. The root `requirements.txt` and its in-root `-r` includes count as one lock here: pip reads them as one requirement set, where a direct reference wins over a compatible `==` pin of the same version in another file of the set (#1086). A PyPI version matches under PEP 440 equality, so a requirements line pinned in any spelling that can only install the wired release (`==1.2.3.0`, `==v1.2.3`, `===1.2.3`) contests it like `==1.2.3` does; a range or wildcard (`~=1.2.3`, `==1.2.*`) does not (#1602). A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`, beneath a dependency that bundles it; the root project's own bundles install from the lock, #1618) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. yarn classic's `yarn.lock` records nothing about bundled copies either (#758), so for a lock with a Socket wiring they are found in the installed `node_modules`: each package a parent's `bundleDependencies` / `bundledDependencies` names inside the parent's own `node_modules`, the dependencies of those beside them, and everything nested under them. Such a copy contests the reference in `yarn.lock` (the diagnostic names the parent's lock entry and the copy's directory); a lock-only `vex` with no `node_modules` can't see it. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`, and a yarn classic hosted scan names one with `redirect_copy_unpatched`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. A reference withheld only because such an unreachable copy installs beside it (a bundled npm / Bun / vlt copy, an npm copy beneath a `hasShrinkwrap` package (#753) or one npm installs from a git / url / `file:` spec, or a yarn classic git or `file:` directory block of the same `name@version`) is still the rewriter's own wiring of exactly one package version: it is never attested, but `rollback`, `remove`, `list` and the hosted → vendored takeover treat it as an ordinary hosted pin and restore its upstream registry entry (#828), rather than refusing it with `hosted_wiring_contested`. Another entry of the **same** npm, Bun or yarn lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. -* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Five cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **Maven profile declarations** (`vex_maven_profile_unpatched`): a `` declaration in the root `pom.xml` of a pinned GA — its main jar in `` or ``, or an executable classifier copy — at any version other than the pin (the base release, another release, a `${property}` the pom does not resolve, an executable classifier copy with no version of its own) overrides the pin whenever that profile is active, so the reference is omitted. Activation is not evaluated: every profile counts, as in vendored mode, which rewrites profile declarations too; hosted scan leaves them as written (`redirect_maven_profile_dependency_unpatched`). A profile declaration without a `` takes the live pin and does not withhold the statement. A pin spelled through a property counts the same way: a `${property}` `` is interpolated through every property it names (user properties from `.mvn/maven.config` first, then the root ``; at most 16 levels deep, and a cycle or an undefined name never resolves to a pin), and when any profile's `` — or several profiles' together — redefine one of those properties so that it resolves anything other than the pin (or nothing), the reference is omitted too; a user property wins over every profile's. A profile declaration reads its own profile's `` first. The same holds for a pre-v5 single-pom vendored repository, which serves only the original version. +* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Five cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **Maven profile declarations** (`vex_maven_profile_unpatched`): a `` declaration in the root `pom.xml` of a pinned GA — its main jar in `` or ``, or an executable classifier copy — at any version other than the pin (the base release, another release, a `${property}` the pom does not resolve, an executable classifier copy with no version of its own) overrides the pin whenever that profile is active, so the reference is omitted. Activation is not evaluated: every profile counts, as in vendored mode, which rewrites profile declarations too; hosted scan leaves them as written (`redirect_maven_profile_dependency_unpatched`). A profile declaration without a `` takes the live pin and does not withhold the statement. A pin spelled through a property counts the same way: a `${property}` `` is interpolated through every property it names (user properties from `.mvn/maven.config` first, then the root ``; at most 16 levels deep, and a cycle or an undefined name never resolves to a pin), and when any profile's `` — or several profiles' together — redefine one of those properties so that it resolves anything other than the pin (or nothing), the reference is omitted too; a user property wins over every profile's. A profile declaration reads its own profile's `` first. Coordinates count the same way: when a profile's `` (or several profiles' together, or the profile holding the declaration) redefine a property a `${property}` `` / `` names, so that the declaration carrying the pin names another package (the pin then no longer applies to the patched GA) or another declaration names the patched GA (its main jar, or an executable classifier copy) at a version other than the pin, or as a classifier copy with no version of its own, the reference is omitted; a profile value the pom cannot resolve may name any package, and a user property wins over every profile's. The same holds for a pre-v5 single-pom vendored repository, which serves only the original version. * **Shrinkwrap without a twin (#899).** npm >= 12 never reads `npm-shrinkwrap.json`: on a project whose only npm lock is the shrinkwrap it resolves the tree from the registry and writes a fresh `package-lock.json`, so a Socket reference there reaches npm <= 11 only. The reference is still discovered, so `list`, `rollback` and `remove` manage it, but `vex` omits the patch (`vex_npm_shrinkwrap_only`, as a run warning and as the `failed[].reason`). A `package-lock.json` beside it (wired the same way) makes it attestable again. Hosted and vendored runs still rewire the shrinkwrap and warn `redirect_npm_shrinkwrap_only` / `vendor_npm_shrinkwrap_only`. * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). @@ -2678,7 +2678,7 @@ match it, withholds the statement: | `vex_deno_lock_copy` | run warning and `failed[].reason` | An npm pin is wired, but `deno.lock` locks the same `name@version`, which `deno install` installs without the wiring; no statement while it does. `vendor --check` and `scan` still treat the wiring as live. | | `vex_maven_reactor_root` | run warning and `failed[].reason` | A hosted Maven pin sits in a root `pom.xml` that declares `` / `` (a profile's included), where a module's own literal `` overrides it; only the root pom is read, so no statement (#261). `list`, `rollback` and `remove` still find the pin. | | `vex_maven_classifier_unpatched` | run warning and `failed[].reason` | A hosted Maven pin's GA also has a live executable `` copy (not `sources` / `javadoc`) at a version other than the pin, which the classifier-less pin does not retarget and the grant does not serve; no statement while it does. `list`, `rollback` and `remove` still find the pin. | -| `vex_maven_profile_unpatched` | run warning and `failed[].reason` | A Maven pin's GA is also declared inside `` in the root `pom.xml` at a version other than the pin (or as an executable classifier copy off it), or a profile's `` redefine a property the pin's `${property}` version names so it resolves another version, which overrides the pin whenever that profile is active; no statement while it does. `list`, `rollback` and `remove` still find the pin. | +| `vex_maven_profile_unpatched` | run warning and `failed[].reason` | A Maven pin's GA is also declared inside `` in the root `pom.xml` at a version other than the pin (or as an executable classifier copy off it), or a profile's `` redefine a property the pin's `${property}` version names so it resolves another version, or a property a `${property}` groupId / artifactId names so the pin names another package or another declaration becomes the pinned GA off the pin, which overrides the pin whenever that profile is active; no statement while it does. `list`, `rollback` and `remove` still find the pin. | | `vex_gradle_derived_cache_unchecked` | run warning | The derived-cache walk was cut short (a very large transforms cache); the statement is still emitted, the copies the walk did reach were checked. | A vendored Gradle entry attests only while its wiring is live (apply line, index rows, diff --git a/crates/socket-patch-cli/src/commands/vex_sources.rs b/crates/socket-patch-cli/src/commands/vex_sources.rs index 5bc2cca93..485f726bd 100644 --- a/crates/socket-patch-cli/src/commands/vex_sources.rs +++ b/crates/socket-patch-cli/src/commands/vex_sources.rs @@ -276,9 +276,11 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) { UnattestedKind::MavenProfileUnpatched => ( NOTE_MAVEN_PROFILE_UNPATCHED, "not attested while a profile declares it at, or its move it to, \ - another version; the wiring itself is intact, so re-running `scan` does not change \ - this (drop that profile or property or set it to the pin, or re-patch \ - with `scan --mode vendored`)", + another version, or its redefine a groupId / artifactId property so \ + the pin names another package or another declaration names this one; the wiring \ + itself is intact, so re-running `scan` does not change this (drop that profile \ + or property or set it to the pin, or re-patch with \ + `scan --mode vendored`)", ), } } diff --git a/crates/socket-patch-core/src/formats/maven/mod.rs b/crates/socket-patch-core/src/formats/maven/mod.rs index f994c63c3..d5f4a753d 100644 --- a/crates/socket-patch-core/src/formats/maven/mod.rs +++ b/crates/socket-patch-core/src/formats/maven/mod.rs @@ -39,8 +39,22 @@ pub(crate) struct PomRepo { #[derive(Debug)] pub(crate) struct PomDep { + /// `` / `` as [`ScopedDep::group`] reads them: + /// while no profile is active, an undefined `${…}` kept as written. pub(crate) group: String, pub(crate) artifact: String, + /// The trimmed `` / `` text as written. + pub(crate) group_text: String, + pub(crate) artifact_text: String, + /// The other `(groupId, artifactId)` pairs the declaration names when + /// some set of profiles is active whose `` redefine a + /// property its coordinates name (transitively), or, for a declaration + /// inside a profile, while its own profile is: a superset + /// ([`interpolate_each_keeping`], each coordinate's values paired with + /// each of the other's), in which a reference some set leaves + /// unresolved stays as written ([`may_name`] reads it as any text). + /// Empty when no profile can move it. + pub(crate) coordinate_alternatives: Vec<(String, String)>, /// Trimmed version, every `${prop}` interpolated /// ([`interpolate_strict`]) from the build's user properties, the /// project model, the `` of the profile holding the @@ -216,6 +230,55 @@ impl PomScope { (base, alternatives) } + /// [`PomDep::coordinate_alternatives`] of the declaration at `pos` + /// whose coordinates read `base` ([`ScopedDep::group`] / + /// [`ScopedDep::artifact`]) and are written `text`. + fn coordinate_alternatives( + &self, + text: (&str, &str), + base: (&str, &str), + pos: usize, + ) -> Vec<(String, String)> { + if !text.0.contains("${") && !text.1.contains("${") { + return Vec::new(); + } + let own = self.own_profile(pos); + // Every value a coordinate may read, `base` included. What it reads + // with no profile active is `base`, whatever the two interpolators' + // bounds make of an odd property (a chain deeper than the lenient + // one follows, a cycle), so that reading is no alternative. + let values = |text: &str, base: &str| -> BTreeSet { + let mut all = BTreeSet::from([base.to_string()]); + if !text.contains("${") { + return all; + } + match interpolate_each_keeping(text, &|name| self.lookup_any(name, &own)) { + Some(each) => all.extend(each), + None => { + all.insert(text.to_string()); + } + } + let plain = |name: &str| self.lookup(name, &[]).into_iter().collect(); + for v in interpolate_each_keeping(text, &plain).unwrap_or_default() { + if v != base { + all.remove(&v); + } + } + all + }; + let groups = values(text.0, base.0); + let artifacts = values(text.1, base.1); + let mut out = Vec::new(); + for g in &groups { + for a in &artifacts { + if (g.as_str(), a.as_str()) != base { + out.push((g.clone(), a.clone())); + } + } + } + out + } + /// Inside a plugin / deploy section (never project resolution). pub(crate) fn is_ignored(&self, pos: usize) -> bool { inside(pos, &self.ignored) @@ -263,8 +326,13 @@ pub(crate) struct ScopedDep { /// `` or the project's own coordinates define /// interpolated (see [`interpolate`]); an /// undefined one stays as written, so [`may_name`] can tell it apart. + /// No profile's `` are read, not even those of the profile + /// holding the declaration ([`PomDep::coordinate_alternatives`] does). pub(crate) group: Option, pub(crate) artifact: Option, + /// The trimmed `` / `` text as written. + pub(crate) group_text: Option, + pub(crate) artifact_text: Option, /// Inner-text byte range of the literal ``, when present. pub(crate) version_inner: Option<(usize, usize)>, /// Trimmed `` text as written (no property resolution). @@ -283,12 +351,18 @@ impl PomScope { for dep in self.elements("dependency")? { let body = blank_elements(dep.inner(&self.masked), "exclusions")?; let version = elements(&body, "version")?.into_iter().next(); + let group_text = child_text(&body, "groupId")?; + let artifact_text = child_text(&body, "artifactId")?; out.push(ScopedDep { element: dep, - group: child_text(&body, "groupId")? - .map(|g| interpolate(&g, &self.coordinate_refs)), - artifact: child_text(&body, "artifactId")? - .map(|a| interpolate(&a, &self.coordinate_refs)), + group: group_text + .as_deref() + .map(|g| interpolate(g, &self.coordinate_refs)), + artifact: artifact_text + .as_deref() + .map(|a| interpolate(a, &self.coordinate_refs)), + group_text, + artifact_text, version_inner: version.map(|v| { ( dep.inner_start + v.inner_start, @@ -332,9 +406,16 @@ pub(crate) fn parse_pom_with(raw: &str, user: &BTreeMap) -> Resu } for dep in scope.dependencies()? { - let (Some(group), Some(artifact)) = (dep.group, dep.artifact) else { + let (Some(group), Some(artifact), Some(group_text), Some(artifact_text)) = + (dep.group, dep.artifact, dep.group_text, dep.artifact_text) + else { continue; }; + let coordinate_alternatives = scope.coordinate_alternatives( + (&group_text, &artifact_text), + (&group, &artifact), + dep.element.start, + ); let (version, version_alternatives) = match &dep.version_text { Some(text) => { let (version, alternatives) = scope.resolve_version(text, dep.element.start); @@ -345,6 +426,9 @@ pub(crate) fn parse_pom_with(raw: &str, user: &BTreeMap) -> Resu pom.deps.push(PomDep { group, artifact, + group_text, + artifact_text, + coordinate_alternatives, version, version_text: dep.version_text, version_alternatives, @@ -484,18 +568,31 @@ pub(crate) fn interpolate_each( lookup: &dyn Fn(&str) -> Vec, ) -> Option> { let lookups = std::cell::Cell::new(MAX_LOOKUPS); - expand(value, lookup, &mut Vec::new(), &lookups) + expand(value, lookup, &mut Vec::new(), &lookups, false) +} + +/// [`interpolate_each`], except that a reference it cannot resolve (an +/// undefined name, a cycle, a chain deeper than +/// [`MAX_INTERPOLATION_DEPTH`], an unterminated `${`) is kept as written +/// instead of failing the whole text, the way coordinates are read +/// ([`ScopedDep::group`]): each kept `${name}` then stands for any text +/// ([`may_name`]). `None` only past the work or result bounds. +pub(crate) fn interpolate_each_keeping( + value: &str, + lookup: &dyn Fn(&str) -> Vec, +) -> Option> { + let lookups = std::cell::Cell::new(MAX_LOOKUPS); + expand(value, lookup, &mut Vec::new(), &lookups, true) } +/// [`interpolate_each`] (`keep` false) or [`interpolate_each_keeping`]. fn expand( value: &str, lookup: &dyn Fn(&str) -> Vec, stack: &mut Vec, lookups: &std::cell::Cell, + keep: bool, ) -> Option> { - if stack.len() > MAX_INTERPOLATION_DEPTH { - return None; - } let mut texts = BTreeSet::from([String::new()]); let mut rest = value; loop { @@ -506,22 +603,37 @@ fn expand( let Some(after) = reference else { return concat(&texts, &BTreeSet::from([literal.to_string()])); }; - let close = after.find('}')?; + let Some(close) = after.find('}') else { + return match keep { + true => concat(&texts, &BTreeSet::from([rest.to_string()])), + false => None, + }; + }; let name = &after[..close]; - if stack.iter().any(|n| n == name) { - return None; - } - lookups.set(lookups.get().checked_sub(1)?); - let raws = lookup(name); - if raws.is_empty() { - return None; - } - stack.push(name.to_string()); - let mut values = BTreeSet::new(); - for raw in &raws { - values.extend(expand(raw, lookup, stack, lookups)?); - } - stack.pop(); + let written = || BTreeSet::from([format!("${{{name}}}")]); + let values = if stack.iter().any(|n| n == name) || stack.len() >= MAX_INTERPOLATION_DEPTH { + if !keep { + return None; + } + written() + } else { + lookups.set(lookups.get().checked_sub(1)?); + let raws = lookup(name); + if raws.is_empty() { + if !keep { + return None; + } + written() + } else { + stack.push(name.to_string()); + let mut values = BTreeSet::new(); + for raw in &raws { + values.extend(expand(raw, lookup, stack, lookups, keep)?); + } + stack.pop(); + values + } + }; let prefixed = concat(&texts, &BTreeSet::from([literal.to_string()]))?; texts = concat(&prefixed, &values)?; rest = &after[close + 1..]; @@ -1172,6 +1284,180 @@ mod tests { ); } + /// The keeping interpolator leaves what it cannot resolve as written + /// (an undefined name, a cycle, too deep, an unterminated `${`) and + /// fails only past its work bounds. + #[test] + fn keeping_interpolation_leaves_unresolved_references() { + let props: std::collections::BTreeMap<&str, &str> = [ + ("a", "org"), + ("b", "${a}.${nope}"), + ("c", "x${c}"), + ("d", "${a}"), + ] + .into(); + let lookup = |name: &str| props.get(name).map(|v| v.to_string()).into_iter().collect(); + let each = |text: &str| { + super::interpolate_each_keeping(text, &lookup) + .map(|set| set.into_iter().collect::>()) + }; + let one = |v: &str| Some(vec![v.to_string()]); + assert_eq!(each("${b}-x"), one("org.${nope}-x")); + assert_eq!(each("${c}"), one("x${c}")); + assert_eq!(each("${d}.${a"), one("org.${a")); + assert_eq!(each("plain"), one("plain")); + let many = |name: &str| match name { + "v" => vec!["1".to_string(), "2".to_string()], + _ => Vec::new(), + }; + assert_eq!( + super::interpolate_each_keeping("${v}.${w}", &many) + .map(|set| set.into_iter().collect::>()), + Some(vec!["1.${w}".to_string(), "2.${w}".to_string()]) + ); + let deep: std::collections::BTreeMap = (0..20) + .map(|i| (format!("p{i}"), format!("${{p{}}}", i + 1))) + .collect(); + let deep_lookup = |name: &str| deep.get(name).cloned().into_iter().collect(); + assert_eq!( + super::interpolate_each_keeping("${p0}", &deep_lookup), + Some(std::collections::BTreeSet::from(["${p16}".to_string()])) + ); + } + + /// The `(groupId, artifactId)` pairs a profile's `` may move + /// a `${property}` coordinate to: every profile (and set of them) + /// counts, a declaration inside a profile reads its own first, a value + /// left unresolved stays as written, and user properties and model + /// references no profile can redefine. + #[test] + fn profile_properties_give_coordinate_alternatives() { + let alternatives = |props: &str, profiles: &[&str], user: &[(&str, &str)], deps: &str| { + let profiles: String = profiles + .iter() + .map(|p| format!("{p}")) + .collect(); + let pom = format!( + "org.self{props}\ + {deps}{profiles}" + ); + let user = user + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + super::parse_pom_with(&pom, &user) + .unwrap() + .deps + .into_iter() + .map(|d| ((d.group, d.artifact), d.coordinate_alternatives)) + .collect::>() + }; + let dep = |g: &str, a: &str| { + format!( + "{g}{a}\ + 1" + ) + }; + let pair = |g: &str, a: &str| (g.to_string(), a.to_string()); + let one = |props, profiles: &[&str], user: &[(&str, &str)], g: &str, a: &str| { + alternatives(props, profiles, user, &dep(g, a)).remove(0) + }; + assert_eq!( + one("org.a", &["org.b"], &[], "${g}", "x"), + (pair("org.a", "x"), vec![pair("org.b", "x")]) + ); + assert_eq!( + one("org.a", &["org.a"], &[], "${g}", "x"), + (pair("org.a", "x"), vec![]) + ); + assert_eq!( + one("org.a", &["org.b"], &[], "${g}", "x"), + (pair("org.a", "x"), vec![]) + ); + // Both coordinates, each value paired with each of the other's. + assert_eq!( + one("a", &["b"], &[], "org.${v}", "${v}-api").1, + [ + pair("org.a", "b-api"), + pair("org.b", "a-api"), + pair("org.b", "b-api") + ] + ); + // Nested, and two profiles together. + assert_eq!( + one( + "${w}org.a", + &["org.b", "${w}.c"], + &[], + "${g}", + "x" + ) + .1, + [ + pair("org.a.c", "x"), + pair("org.b", "x"), + pair("org.b.c", "x") + ] + ); + // Unresolved under a profile, or a cycle: kept as written. + assert_eq!( + one("org.a", &["${nope}"], &[], "${g}", "x").1, + [pair("${nope}", "x")] + ); + assert_eq!( + one("org.a", &["${g}"], &[], "${g}", "x").1, + [pair("${g}", "x")] + ); + // Undefined with no profile involved: no alternative. + assert_eq!( + one("org.a", &[], &[], "${g}.${nope}", "x"), + (pair("org.a.${nope}", "x"), vec![]) + ); + // User properties and model references win over every profile's. + assert_eq!( + one( + "org.a", + &["org.b"], + &[("g", "org.c")], + "${g}", + "x" + ), + (pair("org.c", "x"), vec![]) + ); + assert_eq!( + one( + "", + &["org.b"], + &[], + "${project.groupId}", + "x" + ), + (pair("org.self", "x"), vec![]) + ); + + // A profile's own declaration reads its own properties first. + let pom = format!( + "org.a{}\ + porg.b\ + {}", + dep("${g}", "x"), + dep("${g}", "x") + ); + let deps: Vec<_> = super::parse_pom(&pom) + .unwrap() + .deps + .into_iter() + .map(|d| (d.in_profile, d.group, d.coordinate_alternatives)) + .collect(); + assert_eq!( + deps, + [ + (false, "org.a".to_string(), vec![pair("org.b", "x")]), + (true, "org.a".to_string(), vec![pair("org.b", "x")]), + ] + ); + } + #[test] fn suffix_grammar_is_exact() { assert_eq!( diff --git a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs index 8b8e9b6cc..acf585cfb 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -1803,6 +1803,44 @@ impl Reactor { }) } + /// Where `dep` of `rel`, a declaration named through `${...}` + /// coordinates, reads a groupId and artifactId (each `None` when + /// undefined there) that `hit` accepts, other than `rel` with no + /// profile active: an inheritor of `rel`, or `rel` or an inheritor with + /// a profile on its chain active (which covers the profile holding + /// `dep`). Maven interpolates after inheritance and with the active + /// profiles' `` merged in, then matches management and + /// pins. Profile activation is not evaluated, so every profile counts + /// (one at a time). + fn coordinates_elsewhere( + &self, + rel: &str, + dep: usize, + hit: &dyn Fn(Option, Option) -> bool, + ) -> Option { + let doc = &self.poms[rel].doc; + let read = |resolve: &dyn Fn(&str) -> Option| { + hit( + doc.coordinate(dep, "groupId", resolve), + doc.coordinate(dep, "artifactId", resolve), + ) + }; + std::iter::once(rel) + .chain(self.inheritors(rel)) + .find_map(|reader| { + let plain = (reader != rel && read(&|v| self.interpolate(reader, v))) + .then(|| reader.to_string()); + plain.or_else(|| { + self.chain(reader).find_map(|at| { + self.poms[at].profile_props.iter().find_map(|(id, props)| { + read(&|v| self.interpolate_with(reader, v, Some((at, props)))) + .then(|| format!("{reader} under profile {id} of {at}")) + }) + }) + }) + }) + } + /// Every pom in scope that inherits from `rel` (has it on its chain). fn inheritors<'s>(&'s self, rel: &'s str) -> impl Iterator + 's { self.scope @@ -1830,6 +1868,7 @@ impl Reactor { let doc = &self.poms[rel].doc; let ga = format!("{}:{}", patch.group_id, patch.artifact_id); self.warn_unresolved_coordinates(rel, patch, warnings, unpinned, uncertain); + self.warn_moved_coordinates(rel, patch, warnings, unpinned, uncertain); for (dep, key) in self.keyed_declarations(rel, patch.group_id, patch.artifact_id) { if doc .child_text(dep, "type") @@ -1849,15 +1888,13 @@ impl Reactor { continue; } let root = self.local_root(rel); - // Maven interpolates after inheritance: a pom that inherits this - // declaration may name another artifact through it, which the - // literal SV must not reach. + // Maven interpolates after inheritance and with the active + // profiles: a pom that inherits this declaration, or a profile + // redefining a property it names, may make it another artifact, + // which the literal SV must not reach. if doc.has_property_coordinate(dep) { - let other = self.inheritors(rel).find(|p| { - let resolve = |v: &str| self.interpolate(p, v); - doc.coordinate(dep, "groupId", &resolve).as_deref() != Some(patch.group_id) - || doc.coordinate(dep, "artifactId", &resolve).as_deref() - != Some(patch.artifact_id) + let other = self.coordinates_elsewhere(rel, dep, &|g, a| { + g.as_deref() != Some(patch.group_id) || a.as_deref() != Some(patch.artifact_id) }); if let Some(p) = other { warnings.push(degraded( @@ -2027,6 +2064,86 @@ impl Reactor { } } +impl Reactor { + /// Warn for each declaration of `rel` whose `${...}` groupId or + /// artifactId reads another artifact in `rel` itself but may read + /// `patch`'s g:a in an inheritor or with a profile active + /// ([`Self::coordinates_elsewhere`]): it is left as is, and there it + /// declares the g:a at its own version. One with a version beats any + /// pin, so its local root is not pinned. + fn warn_moved_coordinates( + &self, + rel: &str, + patch: &JvmPatch<'_>, + warnings: &mut Vec, + unpinned: &mut BTreeSet, + uncertain: &mut BTreeSet, + ) { + let doc = &self.poms[rel].doc; + let resolve = |v: &str| self.interpolate(rel, v); + for dep in doc.declarations() { + let (Some(g), Some(a)) = ( + doc.child_text(dep, "groupId"), + doc.child_text(dep, "artifactId"), + ) else { + continue; + }; + let other_variant = doc + .child_text(dep, "type") + .is_some_and(|t| !t.is_empty() && t != "jar") + || doc + .child_text(dep, "classifier") + .is_some_and(|c| !c.is_empty()); + // Undefined in `rel`: `warn_unresolved_coordinates` weighs it; + // `patch`'s g:a in `rel`: `rewrite_declarations` does. + let (Some(here_g), Some(here_a)) = ( + doc.coordinate(dep, "groupId", &resolve), + doc.coordinate(dep, "artifactId", &resolve), + ) else { + continue; + }; + if other_variant + || !doc.has_property_coordinate(dep) + || doc.pin_at(dep).is_some() + || (here_g == patch.group_id && here_a == patch.artifact_id) + { + continue; + } + let could_be = |raw: &str, read: Option, want: &str| match read { + Some(v) => v == want, + None => may_interpolate_to(raw, want), + }; + let Some(there) = self.coordinates_elsewhere(rel, dep, &|rg, ra| { + could_be(&g, rg, patch.group_id) && could_be(&a, ra, patch.artifact_id) + }) else { + continue; + }; + let at = format!("{rel}:{}", doc.line_of(doc.nodes[dep].start)); + let versioned = doc + .child_text(dep, "version") + .is_some_and(|v| !v.is_empty()); + let root = self.local_root(rel); + let unpin = if versioned { + format!(", and {root} is not pinned") + } else { + String::new() + }; + warnings.push(degraded( + "coordinates_overridden", + format!( + "{at}: {g}:{a} is {here_g}:{here_a} here but may be {}:{} in {there}; \ + left as is, probably unpatched there{unpin}", + patch.group_id, patch.artifact_id, + ), + )); + if versioned { + unpinned.insert(root); + uncertain.insert(at); + } + } + } +} + /// Whether `raw`, with each `${...}` standing for any text, can read /// `want`. fn may_interpolate_to(raw: &str, want: &str) -> bool { @@ -5815,6 +5932,185 @@ mod tests { assert!(unwired(&applied(&files, &plan))); } + /// `root` with `{props}` and a profile `corp` + /// whose `` are `profile` (none when empty), before its + /// ``. + fn root_with(props: &str, profile: &str) -> String { + let root = ROOT.replace( + " ", + &format!(" {props}\n "), + ); + match profile { + "" => root, + p => root.replace( + " ", + &format!( + " corp{p}\ + \n " + ), + ), + } + } + + /// A declaration whose `${...}` groupId a profile redefines names + /// another artifact whenever that profile is active, which the literal + /// SV must not reach: it is left as is and its root unpinned, as for + /// an inheritor that resolves it differently. A `.mvn/maven.config` + /// user property wins over the profile's. + #[test] + fn a_profile_moving_the_declared_coordinates_is_left_alone() { + let decl = "${ct.group}\ + commons-text1.10.0"; + let a = module("a", decl); + let props = "org.apache.commons"; + for profile in [ + "org.other", + "${nope}", + ] { + let files = fs(&[ + ("pom.xml", &root_with(props, profile)), + ("a/pom.xml", &a), + ("b/pom.xml", &module("b", "")), + ]); + let plan = run(&files).unwrap(); + assert_eq!(reasons(&plan), ["coordinates_overridden"], "{profile}"); + assert!( + plan.warnings[0].detail.contains("profile corp"), + "{:?}", + plan.warnings + ); + let after = applied(&files, &plan); + assert_eq!(text(&after, "a/pom.xml"), a); + assert!(!root_pinned(&plan, &files)); + assert!(unwired(&after), "{profile}"); + } + + // A vendored reactor a profile is added to later is not attestable. + let files = fs(&[ + ("pom.xml", &root_with(props, "")), + ("a/pom.xml", &a), + ("b/pom.xml", &module("b", "")), + ]); + let (_, mut after) = vendor(&files); + assert_eq!(attestable(&after), (true, true)); + let root = text(&after, "pom.xml").replace( + " ", + " corporg.other\ + \n ", + ); + after.insert("pom.xml".to_string(), root.into_bytes()); + assert_eq!(attestable(&after), (true, false)); + + // The same profile, beaten by a user property, or agreeing. + for (profile, config) in [ + ( + "org.other", + "-Dct.group=org.apache.commons\n", + ), + ("org.apache.commons", ""), + ] { + let files = fs(&[ + ("pom.xml", &root_with(props, profile)), + ("a/pom.xml", &a), + ("b/pom.xml", &module("b", "")), + (".mvn/maven.config", config), + ]); + let (plan, after) = vendor(&files); + assert!(plan.warnings.is_empty(), "{:?}", plan.warnings); + assert_eq!(text(&after, "a/pom.xml"), a.replace("1.10.0", SV)); + assert_eq!(attestable(&after), (true, true), "{profile}"); + } + } + + /// A declaration whose `${...}` groupId names another artifact with no + /// profile active, but the patched one under a profile, declares the + /// patched artifact at its own version there, beating the root pin: + /// it is left as is, warned about, and its root unpinned, so a reactor + /// vendored before that profile was added is not attestable. Unrelated + /// coordinate properties change nothing. + #[test] + fn a_profile_turning_a_declaration_into_the_patched_artifact_is_weighed() { + let other = |v: &str| { + format!( + "${{lib.group}}\ + commons-text{v}" + ) + }; + let props = "org.other"; + let a = module( + "a", + &format!("{}{}", dep("1.10.0"), other("1.9")), + ); + for (profile, warned) in [ + ("org.apache.commons", true), + ("${nope}", true), + ("org.third", false), + ("org.apache.commons", false), + ] { + let files = fs(&[ + ("pom.xml", &root_with(props, profile)), + ("a/pom.xml", &a), + ("b/pom.xml", &module("b", "")), + ]); + let plan = run(&files).unwrap(); + if warned { + assert_eq!(reasons(&plan), ["coordinates_overridden"], "{profile}"); + assert!( + plan.warnings[0].detail.contains("profile corp") + && plan.warnings[0] + .detail + .contains("org.apache.commons:commons-text"), + "{:?}", + plan.warnings + ); + assert!(!root_pinned(&plan, &files), "{profile}"); + } else { + assert!(plan.warnings.is_empty(), "{profile}: {:?}", plan.warnings); + assert!(root_pinned(&plan, &files), "{profile}"); + } + } + + // No version of its own: the pin applies, so the root stays pinned. + let files = fs(&[ + ( + "pom.xml", + &root_with(props, "org.apache.commons"), + ), + ( + "a/pom.xml", + &module("a", &format!("{}{}", dep("1.10.0"), other(""))), + ), + ("b/pom.xml", &module("b", "")), + ]); + let plan = run(&files).unwrap(); + assert_eq!(reasons(&plan), ["coordinates_overridden"]); + assert!(root_pinned(&plan, &files)); + + // Vendored first, the profile added later: not attestable; a user + // property fixing the group keeps it attestable. + let files = fs(&[ + ("pom.xml", &root_with(props, "")), + ("a/pom.xml", &a), + ("b/pom.xml", &module("b", "")), + ]); + let (_, after) = vendor(&files); + assert_eq!(attestable(&after), (true, true)); + let profiled = text(&after, "pom.xml").replace( + " ", + " corp\ + org.apache.commons\n \ + ", + ); + let mut later = after.clone(); + later.insert("pom.xml".to_string(), profiled.into_bytes()); + assert_eq!(attestable(&later), (true, false)); + later.insert( + ".mvn/maven.config".to_string(), + b"-Dlib.group=org.other\n".to_vec(), + ); + assert_eq!(attestable(&later), (true, true)); + } + fn attestable(files: &Fs) -> (bool, bool) { let read = |p: &str| files.get(p).cloned(); let c = Coords { diff --git a/crates/socket-patch-core/src/vex/discover/maven.rs b/crates/socket-patch-core/src/vex/discover/maven.rs index 2b32bd5a8..b3d38e4b5 100644 --- a/crates/socket-patch-core/src/vex/discover/maven.rs +++ b/crates/socket-patch-core/src/vex/discover/maven.rs @@ -59,7 +59,13 @@ //! read from `.mvn/maven.config`'s `-D` user properties, else the root //! `` (Maven reads user properties first). One that resolves //! the pin but that any profile's (or set of profiles') `` -//! make resolve anything else keeps the ref unattested the same way. +//! make resolve anything else keeps the ref unattested the same way. So +//! do `${property}` coordinates a profile's `` redefine +//! (Maven merges them in before it interpolates a groupId / artifactId, +//! then matches pins): the declaration carrying the pin then names +//! another package, or another declaration becomes the pinned GA at a +//! version off the pin. Coordinates keep an undefined reference as +//! written, so a profile value this pom cannot resolve may name anything. //! //! Integrity: when the pom sha256 was known the rewriter also writes Maven //! Trusted Checksums — `.mvn/checksums/checksums.sha256` lines @@ -116,7 +122,7 @@ use super::{ DIAG_REF_UNATTRIBUTABLE, }; use crate::formats::maven::{ - is_maven_coordinate, is_maven_version_text, maven_config_properties, may_shadow, + is_maven_coordinate, is_maven_version_text, maven_config_properties, may_name, may_shadow, parse_pom_with, split_socket_version, Pom, PomDep, PomRepo, }; use crate::patch::redirect::{MVN_CHECKSUMS, MVN_CONFIG, TRUSTED_CHECKSUMS_ON}; @@ -795,74 +801,158 @@ fn unpatched_executable_classifier<'a>( /// resolves another version ([`PomDep::version_alternatives`]): Maven /// merges an active profile's properties over the pom's own before it /// interpolates. Vendored mode refuses to pin over such a property. +/// +/// and, through [`coordinate_override`], a declaration whose coordinates +/// a profile's `` redefine. fn profile_override(deps: &[PomDep], group: &str, artifact: &str, pinned: &str) -> Option { deps.iter().find_map(|d| { - let literal = d.group == group && d.artifact == artifact; - let section = if d.managed { - " " - } else { - "" - }; - let named = if literal { - format!("{group}:{artifact}") - } else { - format!( - "{}:{} (which may be {group}:{artifact})", - d.group, d.artifact - ) - }; - // Every version the declaration may resolve, the base one first. - let mut versions = d.version.iter().chain(&d.version_alternatives); - let main = d.classifier.is_none() - && (literal - || may_shadow( - (&d.group, &d.artifact), - d.type_text.as_deref(), - None, - (group, artifact), - )); - let classifier = d - .classifier - .as_deref() - .filter(|c| literal && !NON_EXECUTABLE_CLASSIFIERS.contains(c)); - if !main && classifier.is_none() { + declaration_override(d, group, artifact, pinned) + .or_else(|| coordinate_override(d, group, artifact, pinned)) + }) +} + +/// [`profile_override`] for one declaration's version, read under the +/// coordinates it names while no profile is active. +fn declaration_override(d: &PomDep, group: &str, artifact: &str, pinned: &str) -> Option { + let literal = d.group == group && d.artifact == artifact; + let section = if d.managed { + " " + } else { + "" + }; + let named = if literal { + format!("{group}:{artifact}") + } else { + format!( + "{}:{} (which may be {group}:{artifact})", + d.group, d.artifact + ) + }; + // Every version the declaration may resolve, the base one first. + let mut versions = d.version.iter().chain(&d.version_alternatives); + let main = d.classifier.is_none() + && (literal + || may_shadow( + (&d.group, &d.artifact), + d.type_text.as_deref(), + None, + (group, artifact), + )); + let classifier = d + .classifier + .as_deref() + .filter(|c| literal && !NON_EXECUTABLE_CLASSIFIERS.contains(c)); + if !main && classifier.is_none() { + return None; + } + if !d.in_profile { + // The base version is the live one: a declaration off the pin + // there is diagnosed by the version grouping, not here. + if d.version.as_deref() != Some(pinned) { return None; } - if !d.in_profile { - // The base version is the live one: a declaration off the pin - // there is diagnosed by the version grouping, not here. - if d.version.as_deref() != Some(pinned) { - return None; - } - let moved = d.version_alternatives.first()?; - let declared = d.version_text.as_deref().unwrap_or_default(); - let copy = classifier.map_or(String::new(), |c| format!(" classifier {c}")); - return Some(format!( - "{POM}: {section}{named}{copy} {declared} resolves the \ + let moved = d.version_alternatives.first()?; + let declared = d.version_text.as_deref().unwrap_or_default(); + let copy = classifier.map_or(String::new(), |c| format!(" classifier {c}")); + return Some(format!( + "{POM}: {section}{named}{copy} {declared} resolves the \ patched {pinned} from the root , but a \ redefinition of a property it names makes it {moved}; whenever that profile \ is active Maven resolves it instead of the pin" - )); + )); + } + let what = match classifier { + None => { + let version = versions.find(|v| *v != pinned)?; + format!("{version}") } - let what = match classifier { - None => { - let version = versions.find(|v| *v != pinned)?; - format!("{version}") + Some(c) => match d.version { + None => format!("classifier {c} with no of its own"), + Some(_) => { + let v = versions.find(|v| *v != pinned)?; + format!("classifier {c} at {v}") } - Some(c) => match d.version { - None => format!("classifier {c} with no of its own"), - Some(_) => { - let v = versions.find(|v| *v != pinned)?; - format!("classifier {c} at {v}") - } - }, - }; - Some(format!( - "{POM}: a {section}declaration of {named} carries {what}, not the \ + }, + }; + Some(format!( + "{POM}: a {section}declaration of {named} carries {what}, not the \ patched {pinned}; whenever that profile is active Maven resolves it instead of \ the pin" - )) - }) + )) +} + +/// [`profile_override`] for a declaration whose `${…}` groupId or +/// artifactId some set of active profiles' `` (or the profile +/// holding it) redefine ([`PomDep::coordinate_alternatives`]); Maven merges +/// them over the pom's own before it interpolates the coordinates, and +/// matches management and pins after: +/// +/// * the live declaration carrying the pin (a main jar at `pinned`) then +/// names another package, so the pin no longer applies to +/// `group:artifact`, which resolves from wherever its other declarations +/// (or a transitive path) say; +/// * any other declaration then becomes `group:artifact` (its main jar, or +/// literally an executable classifier copy of it) at a version other +/// than `pinned`, or, for a classifier copy, with no version of its own. +/// A main jar without a version takes the live pin and moves nothing. +fn coordinate_override(d: &PomDep, group: &str, artifact: &str, pinned: &str) -> Option { + if d.coordinate_alternatives.is_empty() { + return None; + } + let section = if d.managed { + " " + } else { + "" + }; + let place = if d.in_profile { "a " } else { "a " }; + let written = format!("{}:{}", d.group_text, d.artifact_text); + let redefined = "a redefinition of a property its coordinates name"; + if d.group == group && d.artifact == artifact { + if d.in_profile || d.classifier.is_some() || d.version.as_deref() != Some(pinned) { + return None; + } + let (g, a) = &d.coordinate_alternatives[0]; + return Some(format!( + "{POM}: {place}{section}declaration of {written} names {group}:{artifact} at the \ + patched {pinned} from the root , but {redefined} makes it {g}:{a}; \ + whenever that profile is active the pin no longer applies to {group}:{artifact}, \ + which Maven resolves from its other declarations instead" + )); + } + let main_type = d + .type_text + .as_deref() + .is_none_or(|t| t == "jar" || t.contains("${")); + let (g, a) = d + .coordinate_alternatives + .iter() + .find(|(g, a)| match d.classifier.as_deref() { + None => main_type && may_name(g, group) && may_name(a, artifact), + Some(c) => !NON_EXECUTABLE_CLASSIFIERS.contains(&c) && g == group && a == artifact, + })?; + let mut versions = d.version.iter().chain(&d.version_alternatives); + let what = match (d.classifier.as_deref(), &d.version) { + (None, None) => return None, + (Some(c), None) => format!("classifier {c} with no of its own"), + (classifier, Some(_)) => { + let v = versions.find(|v| *v != pinned)?; + match classifier { + None => format!("{v}"), + Some(c) => format!("classifier {c} at {v}"), + } + } + }; + let which = if g == group && a == artifact { + String::new() + } else { + format!(", which may be {group}:{artifact},") + }; + Some(format!( + "{POM}: {place}{section}declaration of {written} names {}:{} with no profile active, \ + but {redefined} makes it {g}:{a}{which} carrying {what}, not the patched {pinned}; \ + whenever that profile is active Maven resolves it instead of the pin", + d.group, d.artifact + )) } #[cfg(test)] @@ -1891,6 +1981,323 @@ mod tests { } } + /// One `property_project` case: root ``, the declarations, + /// each profile's body, `.mvn/maven.config`. + type CoordinateCase = (String, String, Vec, Option); + + /// Runs `case` and checks the slf4j-api ref is found, and attested + /// exactly when `attested` (else withheld as a profile override naming + /// the GA and the profile ``). + async fn assert_coordinate_case(case: &CoordinateCase, attested: bool) { + let (props, decl, profiles, config) = case; + let profiles: Vec<&str> = profiles.iter().map(String::as_str).collect(); + let p = property_project(props, decl, &profiles); + if let Some(config) = config { + p.write(".mvn/maven.config", config); + } + let out = run(&p).await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + if attested { + assert!( + attests(&out, FX_PURL), + "{props} {decl} {profiles:?}: {:?}", + out.unattested + ); + return; + } + assert_eq!( + out.unattested.len(), + 1, + "{props} {decl} {profiles:?}: {:?}", + out.unattested + ); + let u = &out.unattested[0]; + assert_eq!(u.kind, UnattestedKind::MavenProfileUnpatched, "{decl}"); + assert_eq!((u.purl.as_str(), u.uuid.as_str()), (FX_PURL, UUID_A)); + assert!( + u.detail.contains("org.slf4j:slf4j-api") + && u.detail.contains("") + && u.detail.contains("coordinates"), + "{}", + u.detail + ); + } + + fn in_props(body: &str) -> String { + format!("{body}\n") + } + + /// A pin declared through a `${property}` groupId or artifactId is the + /// pin of that GA only while no active profile's `` + /// redefine the property (or one it names): Maven merges them over the + /// pom's own before it interpolates the coordinates, so with such a + /// profile active the pin names another package and slf4j-api resolves + /// from wherever else it is declared (here: transitively, unpinned). + #[tokio::test] + async fn profile_properties_moving_the_pinned_coordinates_are_not_attested() { + let pin = suffixed(UUID_A); + let direct = |g: &str, a: &str| { + format!("\n{}\n", dep(g, a, Some(&pin))) + }; + let managed = format!( + "\n{}\n", + dep("${g}", "slf4j-api", Some(&pin)) + ); + let case = |props: &str, decl: &str, profiles: &[&str], config: Option<&str>| { + ( + props.to_string(), + decl.to_string(), + profiles.iter().map(|p| in_props(p)).collect::>(), + config.map(str::to_string), + ) + }; + let g = "org.slf4j"; + let by_group = direct("${g}", "slf4j-api"); + let withheld = [ + // The groupId, the artifactId, or a managed pin. + case(g, &by_group, &["org.other"], None), + case( + "slf4j-api", + &direct("org.slf4j", "${a}"), + &["slf4j-simple"], + None, + ), + case(g, &managed, &["org.other"], None), + // Through a property the coordinate's names, or partly literal. + case( + "${vendor}org.slf4j", + &by_group, + &["org.other"], + None, + ), + case( + "slf4j", + &direct("org.${v}", "${v}-api"), + &["log4j"], + None, + ), + // A value this pom cannot resolve, or a cycle. + case(g, &by_group, &["${undefined}"], None), + case(g, &by_group, &["${g}"], None), + // Two profiles that move it only together. + case( + "org.slf4jorg.slf4j", + &by_group, + &["${w}", "org.other"], + None, + ), + // A user property in the chain does not fix a name above it. + case( + "${vendor}", + &by_group, + &["org.other"], + Some("-Dvendor=org.slf4j\n"), + ), + ]; + for c in &withheld { + assert_coordinate_case(c, false).await; + } + let attested = [ + // The same GA, or an unrelated property. + case(g, &by_group, &["org.slf4j"], None), + case(g, &by_group, &["org.other"], None), + // A user property wins over every profile's. + case(g, &by_group, &["org.other"], Some("-Dg=org.slf4j\n")), + case( + "${vendor}org.other", + &by_group, + &["org.another"], + Some("-Dvendor=org.slf4j\n"), + ), + ]; + for c in &attested { + assert_coordinate_case(c, true).await; + } + } + + /// Another declaration whose `${property}` coordinates name another + /// package while no profile is active but slf4j-api once a profile's + /// `` redefine them overrides the pin with its own version + /// whenever that profile is active. So does one inside a profile whose + /// own `` make it slf4j-api. + #[tokio::test] + async fn profile_properties_turning_a_declaration_into_the_pinned_ga_are_not_attested() { + let pin = suffixed(UUID_A); + let pinned = dep("org.slf4j", "slf4j-api", Some(&pin)); + let with = |other: &str| format!("\n{pinned}{other}\n"); + let other = |v: Option<&str>| dep("${h}", "slf4j-api", v); + let classified = |c: &str, v: Option<&str>| { + let v = v.map_or(String::new(), |v| format!("{v}")); + format!( + "${{h}}slf4j-api{v}\ + {c}\n" + ) + }; + let h = "org.other"; + let to_slf4j = in_props("org.slf4j"); + let case = |props: &str, decl: String, profiles: Vec, config: Option<&str>| { + ( + props.to_string(), + decl, + profiles, + config.map(str::to_string), + ) + }; + let withheld = [ + // The base release, or another one. + case( + h, + with(&other(Some("1.7.36"))), + vec![to_slf4j.clone()], + None, + ), + case( + h, + with(&other(Some("1.7.30"))), + vec![to_slf4j.clone()], + None, + ), + // Managed. + case( + h, + format!( + "\n{pinned}\n\ + \n{}\n", + other(Some("1.7.36")) + ), + vec![to_slf4j.clone()], + None, + ), + // A version the same profile moves too. + case( + &format!("{h}2.0.0"), + with(&other(Some("${v}"))), + vec![in_props("org.slf4j1.7.36")], + None, + ), + // A value the pom cannot resolve may be slf4j-api. + case( + h, + with(&other(Some("1.7.36"))), + vec![in_props("${undefined}")], + None, + ), + // An executable classifier copy, with or without a version. + case( + h, + with(&classified("tests", Some("1.7.36"))), + vec![to_slf4j.clone()], + None, + ), + case( + h, + with(&classified("tests", None)), + vec![to_slf4j.clone()], + None, + ), + // A profile declaration its own profile's properties retarget. + case( + h, + with(""), + vec![format!( + "{to_slf4j}\n{}\n", + other(Some("1.7.36")) + )], + None, + ), + // Through a user property lower in the chain. + case( + "${corp}", + with(&other(Some("1.7.36"))), + vec![to_slf4j.clone()], + Some("-Dcorp=org.other\n"), + ), + ]; + for c in &withheld { + assert_coordinate_case(c, false).await; + } + let attested = [ + // No version of its own: the live pin applies. + case(h, with(&other(None)), vec![to_slf4j.clone()], None), + // A non-executable classifier copy, or another type. + case( + h, + with(&classified("sources", Some("1.7.36"))), + vec![to_slf4j.clone()], + None, + ), + case( + h, + with( + "${h}slf4j-api\ + 1.7.36pom\n", + ), + vec![to_slf4j.clone()], + None, + ), + // A user property wins over every profile's. + case( + h, + with(&other(Some("1.7.36"))), + vec![to_slf4j.clone()], + Some("-Dh=org.other\n"), + ), + ]; + for c in &attested { + assert_coordinate_case(c, true).await; + } + } + + /// Coordinate properties that cannot make any declaration the pinned + /// GA, or move the pin off it, withhold nothing: a profile moving an + /// unrelated package to another unrelated one, a partly undefined + /// coordinate no profile touches, and a profile property no coordinate + /// names. + #[tokio::test] + async fn unrelated_coordinate_properties_do_not_withhold() { + let pin = suffixed(UUID_A); + let pinned = dep("org.slf4j", "slf4j-api", Some(&pin)); + let with = |other: &str| format!("\n{pinned}{other}\n"); + let none: Option = None; + for case in [ + ( + "org.other".to_string(), + with(&dep("${h}", "slf4j-api", Some("1.7.36"))), + vec![in_props("org.third")], + none.clone(), + ), + ( + "org.other".to_string(), + with(&dep("org.slf4j", "${h}", Some("1.7.36"))), + vec![in_props("slf4j-simple")], + none.clone(), + ), + ( + "com.corp".to_string(), + with(&dep("${h}.${undefined}", "slf4j-api", Some("1.7.36"))), + vec![in_props("1")], + none.clone(), + ), + ( + "org.other1".to_string(), + with(&dep("${h}", "slf4j-api", Some("1.7.36"))), + vec![in_props("2")], + none.clone(), + ), + ( + "org.slf4j".to_string(), + format!( + "\n{}\n", + dep("${g}", "slf4j-api", Some(&pin)) + ), + vec![in_props("org.other")], + none.clone(), + ), + ] { + assert_coordinate_case(&case, true).await; + } + } + /// A `${property}` version resolves through every property it names, /// with user properties first at each level, as Maven interpolates; a /// cycle, or a chain deeper than the vendored reactor follows, is