Skip to content

Commit 009dca3

Browse files
Fix hosted Maven pom edits outside live markup (#259, #262, #342) (#1327)
* Scope hosted Maven pom edits to live markup WIP: draft PR placeholder. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Scope hosted Maven pom edits to live markup Hosted Maven rewrites found their edit points with raw text anchors, so a scan could rewrite a commented-out or plugin <dependency>, put the Socket repository or pin inside an inactive profile or a comment, suffix sources/tests/native classifier jars that the patch server does not serve, and add a second <repositories> or <dependencyManagement> next to a self-closed or commented one. Each case exited 0 with redirected=1 while the build stayed unpatched or broke. The rewriter now reads the pom through formats::maven::PomScope, the same scope VEX attests from: comments, CDATA, <build>, <reporting>, <pluginRepositories>, <distributionManagement> and <profiles> are never matched or edited, self-closed sections are expanded in place, and only classifier-less declarations decide literal-vs-pin. Classifier variants and profile base literals are left alone with new warnings (redirect_maven_classifier_unsupported, redirect_maven_profile_dependency_unpatched); an unreadable pom is skipped with redirect_maven_pom_unreadable. Upstream restore and VEX discovery follow the same scope so these rewrites round-trip and attest. Fixes #259, #262, #342. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Skip GAs declared only in a Maven profile A GA whose only declaration sits in an inactive profile got a top-level dependencyManagement pin that is inert exactly when the profile pulls it in. Skip such a dep with redirect_maven_profile_dependency_unpatched instead of reporting it redirected, keeping the existing in-process contract. Self-closed sections restore to an equivalent empty element, so those goldens are excluded from the byte-exact restore round trip and covered by a unit test instead. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Index the pom once for hosted Maven rewrites Re-reading the pom through PomScope for every patched dependency made hosted Maven scans slower (scan-performance flagged maven/hosted +35%, maven/rescan +25%). The rewriter now reads the pom once into a PomIndex and keeps its offsets in step with its own edits (version rewrites, repository and dependencyManagement insertions, superseded repository removal, URL refresh), so a pom with many patched dependencies is scanned once instead of once per dependency. A 1000-dependency pom rewrites in ~25 ms, down from ~1.7 s before this PR series. A unit test checks the kept index against a fresh build after every kind of edit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Address hosted Maven pom-scope review threads - maven_index: on a pure insertion, an element end (and a self-closed element's inner offsets) exactly at the insertion point no longer shifts, so a <dependencyManagement> inserted before </project> is not overwritten when a self-closed <repositories/> ending there is expanded. - Profiles: any versioned declaration inside <profiles> (a ${property} or another literal, not just the base literal) is reported unpatched, and the dep is skipped when profiles hold its only versioned declarations. - Classifiers: a variant other than sources/javadoc at the patched release skips the dep instead of pinning the main jar beside it. - Restore: entry_is_users reads the same live scope as the versioned count, so a commented, plugin, profile or classifier versionless sibling no longer makes the authored pin look user-written. - Rollback: a commented-out socket-patch repository twin no longer counts as a second repository, nor as a leftover name. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Withhold Maven VEX when an executable classifier copy is unpatched A hosted Maven pin is a classifier-less coordinate: it does not retarget a <classifier> dependency of the same GA, and the grant serves only the main jar, so a tests or native classifier copy resolves from the next repository (the public release). VEX discovery grouped deps with classifiers dropped, so such a pom was still attested as patched. The pin now stays a ref (list/rollback/remove find it) but is marked unattested (vex_maven_classifier_unpatched) when a live, non-profile classifier copy other than sources/javadoc resolves any version but the pin: its own literal, a classifier-keyed dependencyManagement entry, or none in the root pom. sources/javadoc copies keep attesting; they never execute. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent ee14e7d commit 009dca3

85 files changed

Lines changed: 3214 additions & 210 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 3 additions & 2 deletions
Large diffs are not rendered by default.

‎crates/socket-patch-cli/src/commands/vex_sources.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,11 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) {
251251
"not attested while the root declares modules (roll it back and re-patch the \
252252
reactor with `scan --mode vendored`)",
253253
),
254+
UnattestedKind::MavenClassifierUnpatched => (
255+
NOTE_MAVEN_CLASSIFIER_UNPATCHED,
256+
"not attested while that classifier copy resolves an unpatched version; the \
257+
wiring itself is intact, so re-running `scan` does not change this",
258+
),
254259
}
255260
}
256261

@@ -261,6 +266,10 @@ pub(crate) const NOTE_NPM_SHRINKWRAP_ONLY: &str = "vex_npm_shrinkwrap_only";
261266
/// Omission tag and note: a hosted Maven pin sits in a reactor root, where a
262267
/// module's own `<version>` may override it (`vex::Unattested`, #261).
263268
pub(crate) const NOTE_MAVEN_REACTOR_ROOT: &str = "vex_maven_reactor_root";
269+
/// Omission tag and note: a hosted Maven pin's GA also has an executable
270+
/// classifier copy at another version, which the pin does not reach
271+
/// (`vex::Unattested`).
272+
pub(crate) const NOTE_MAVEN_CLASSIFIER_UNPATCHED: &str = "vex_maven_classifier_unpatched";
264273

265274
fn note(code: &'static str, detail: String) -> PlanNote {
266275
PlanNote { code, detail }

‎crates/socket-patch-core/src/formats/maven/mod.rs‎

Lines changed: 182 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -44,66 +44,168 @@ pub(crate) struct PomDep {
4444
/// Trimmed literal version, `${prop}` resolved from the root
4545
/// `<properties>` when possible; `None` when the declaration has none.
4646
pub(crate) version: Option<String>,
47+
/// The non-empty `<classifier>`: a sibling artifact of the GA (sources,
48+
/// tests, a native build), managed and resolved apart from the main jar.
49+
pub(crate) classifier: Option<String>,
4750
/// Inside `<dependencyManagement>`.
4851
pub(crate) managed: bool,
4952
pub(crate) in_profile: bool,
5053
}
5154

55+
/// Where Maven reads a root pom's model from: its text with comments and
56+
/// CDATA blanked (offsets kept), and the sections that are not the
57+
/// project's own resolution. `<build>` / `<reporting>` hold plugin
58+
/// classpaths, `<pluginRepositories>` / `<distributionManagement>` plugin
59+
/// and deploy repositories; `<profiles>` apply only when activated. Shared
60+
/// by the VEX reader ([`parse_pom`]) and the hosted rewriter, so the
61+
/// rewriter edits exactly the markup VEX later attests.
62+
#[derive(Debug)]
63+
pub(crate) struct PomScope {
64+
/// The pom with comments and CDATA blanked byte-for-byte.
65+
pub(crate) masked: String,
66+
ignored: Vec<Element>,
67+
profiles: Vec<Element>,
68+
}
69+
70+
impl PomScope {
71+
/// Fails (fail-closed) on a pom that is not well-formed where it
72+
/// matters: no `<project>`, or an unterminated element or CDATA.
73+
pub(crate) fn new(raw: &str) -> Result<PomScope, String> {
74+
let masked = blank_non_markup(raw)?;
75+
if open_tags(&masked, "project")?.is_empty() || !masked.contains("</project>") {
76+
return Err("no <project> element".to_string());
77+
}
78+
let ignored = [
79+
elements(&masked, "build")?,
80+
elements(&masked, "reporting")?,
81+
elements(&masked, "pluginRepositories")?,
82+
elements(&masked, "distributionManagement")?,
83+
]
84+
.concat();
85+
let profiles = elements(&masked, "profiles")?;
86+
Ok(PomScope {
87+
masked,
88+
ignored,
89+
profiles,
90+
})
91+
}
92+
93+
/// Inside a plugin / deploy section (never project resolution).
94+
pub(crate) fn is_ignored(&self, pos: usize) -> bool {
95+
inside(pos, &self.ignored)
96+
}
97+
98+
/// Inside `<profiles>` (read only when that profile is active).
99+
pub(crate) fn in_profile(&self, pos: usize) -> bool {
100+
inside(pos, &self.profiles)
101+
}
102+
103+
/// Every `<name>` element outside ignored sections (profiles included).
104+
pub(crate) fn elements(&self, name: &str) -> Result<Vec<Element>, String> {
105+
Ok(elements(&self.masked, name)?
106+
.into_iter()
107+
.filter(|e| !self.is_ignored(e.start))
108+
.collect())
109+
}
110+
111+
/// Every `<name>` element Maven always reads: outside ignored sections
112+
/// and profiles.
113+
pub(crate) fn live(&self, name: &str) -> Result<Vec<Element>, String> {
114+
Ok(self
115+
.elements(name)?
116+
.into_iter()
117+
.filter(|e| !self.in_profile(e.start))
118+
.collect())
119+
}
120+
121+
/// The `<` of the closing `</project>` tag.
122+
pub(crate) fn project_close(&self) -> Option<usize> {
123+
self.masked.rfind("</project>")
124+
}
125+
}
126+
127+
fn inside(pos: usize, set: &[Element]) -> bool {
128+
set.iter().any(|e| pos >= e.start && pos < e.end)
129+
}
130+
131+
/// One `<dependency>` element of a [`PomScope`] (outside ignored sections),
132+
/// with offsets into the pom.
133+
#[derive(Debug)]
134+
pub(crate) struct ScopedDep {
135+
pub(crate) element: Element,
136+
pub(crate) group: Option<String>,
137+
pub(crate) artifact: Option<String>,
138+
/// Inner-text byte range of the literal `<version>`, when present.
139+
pub(crate) version_inner: Option<(usize, usize)>,
140+
/// Trimmed `<version>` text as written (no property resolution).
141+
pub(crate) version_text: Option<String>,
142+
pub(crate) type_text: Option<String>,
143+
/// Trimmed non-empty `<classifier>` text.
144+
pub(crate) classifier: Option<String>,
145+
pub(crate) in_profile: bool,
146+
}
147+
148+
impl PomScope {
149+
/// Every `<dependency>` outside ignored sections, children read with
150+
/// `<exclusions>` blanked (they carry their own groupId/artifactId).
151+
pub(crate) fn dependencies(&self) -> Result<Vec<ScopedDep>, String> {
152+
let mut out = Vec::new();
153+
for dep in self.elements("dependency")? {
154+
let body = blank_elements(dep.inner(&self.masked), "exclusions")?;
155+
let version = elements(&body, "version")?.into_iter().next();
156+
out.push(ScopedDep {
157+
element: dep,
158+
group: child_text(&body, "groupId")?,
159+
artifact: child_text(&body, "artifactId")?,
160+
version_inner: version.map(|v| {
161+
(
162+
dep.inner_start + v.inner_start,
163+
dep.inner_start + v.inner_end,
164+
)
165+
}),
166+
version_text: version.map(|v| v.inner(&body).trim().to_string()),
167+
type_text: child_text(&body, "type")?,
168+
classifier: child_text(&body, "classifier")?.filter(|c| !c.is_empty()),
169+
in_profile: self.in_profile(dep.start),
170+
});
171+
}
172+
Ok(out)
173+
}
174+
}
175+
52176
/// A bounded, dependency-free scan of the pom's element structure — enough
53177
/// for the handful of elements the Socket wirings touch. Comments and CDATA
54178
/// sections are blanked first (Maven reads neither as elements); an
55179
/// unterminated element or CDATA section is an error (fail-closed: nothing
56180
/// is discovered from a pom that is not well-formed where it matters).
57181
pub(crate) fn parse_pom(raw: &str) -> Result<Pom, String> {
58-
let text = blank_non_markup(raw)?;
59-
if open_tags(&text, "project")?.is_empty() || !text.contains("</project>") {
60-
return Err("no <project> element".to_string());
61-
}
62-
let ignored = [
63-
elements(&text, "build")?,
64-
elements(&text, "reporting")?,
65-
elements(&text, "pluginRepositories")?,
66-
elements(&text, "distributionManagement")?,
67-
]
68-
.concat();
69-
let profiles = elements(&text, "profiles")?;
70-
let managed = elements(&text, "dependencyManagement")?;
71-
let inside = |pos: usize, set: &[Element]| set.iter().any(|e| pos >= e.start && pos < e.end);
72-
73-
let properties = properties(&text, &ignored, &profiles)?;
182+
let scope = PomScope::new(raw)?;
183+
let text = &scope.masked;
184+
let managed = elements(text, "dependencyManagement")?;
185+
186+
let properties = properties(text, &scope.ignored, &scope.profiles)?;
74187
let mut pom = Pom::default();
75188

76-
for repo in elements(&text, "repository")? {
77-
if inside(repo.start, &ignored) {
78-
continue;
79-
}
80-
let body = repo.inner(&text);
189+
for repo in scope.elements("repository")? {
190+
let body = repo.inner(text);
81191
pom.repos.push(PomRepo {
82192
id: child_text(body, "id")?.unwrap_or_default(),
83193
url: child_text(body, "url")?.unwrap_or_default(),
84-
in_profile: inside(repo.start, &profiles),
194+
in_profile: scope.in_profile(repo.start),
85195
});
86196
}
87197

88-
for dep in elements(&text, "dependency")? {
89-
if inside(dep.start, &ignored) {
90-
continue;
91-
}
92-
// `<exclusions>` carry their own groupId/artifactId children.
93-
let body = blank_elements(dep.inner(&text), "exclusions")?;
94-
let (Some(group), Some(artifact)) = (
95-
child_text(&body, "groupId")?,
96-
child_text(&body, "artifactId")?,
97-
) else {
198+
for dep in scope.dependencies()? {
199+
let (Some(group), Some(artifact)) = (dep.group, dep.artifact) else {
98200
continue;
99201
};
100-
let version = child_text(&body, "version")?.map(|v| resolve_property(&v, &properties));
101202
pom.deps.push(PomDep {
102203
group,
103204
artifact,
104-
version,
105-
managed: inside(dep.start, &managed),
106-
in_profile: inside(dep.start, &profiles),
205+
version: dep.version_text.map(|v| resolve_property(&v, &properties)),
206+
classifier: dep.classifier,
207+
managed: inside(dep.element.start, &managed),
208+
in_profile: dep.in_profile,
107209
});
108210
}
109211
Ok(pom)
@@ -195,6 +297,50 @@ fn blank_elements(text: &str, name: &str) -> Result<String, String> {
195297

196298
#[cfg(test)]
197299
mod tests {
300+
use super::PomScope;
301+
302+
#[test]
303+
fn scope_skips_comments_and_plugin_sections_and_flags_profiles() {
304+
let pom = "<project><dependencies>\
305+
<!-- <dependency><groupId>g</groupId><artifactId>a</artifactId><version>0</version></dependency> -->\
306+
<dependency><groupId>g</groupId><artifactId>a</artifactId><version> 1 </version>\
307+
<exclusions><exclusion><groupId>x</groupId><artifactId>y</artifactId></exclusion></exclusions></dependency>\
308+
<dependency><groupId>g</groupId><artifactId>a</artifactId><version>1</version><classifier>sources</classifier></dependency>\
309+
</dependencies>\
310+
<build><plugins><plugin><dependencies><dependency><groupId>g</groupId><artifactId>a</artifactId><version>2</version></dependency></dependencies></plugin></plugins></build>\
311+
<profiles><profile><repositories/><dependencies><dependency><groupId>g</groupId><artifactId>a</artifactId><version>3</version></dependency></dependencies></profile></profiles>\
312+
<repositories/></project>";
313+
let scope = PomScope::new(pom).unwrap();
314+
let deps = scope.dependencies().unwrap();
315+
let seen: Vec<_> = deps
316+
.iter()
317+
.map(|d| {
318+
(
319+
d.group.as_deref().unwrap(),
320+
d.version_text.as_deref().unwrap(),
321+
d.classifier.as_deref(),
322+
d.in_profile,
323+
)
324+
})
325+
.collect();
326+
assert_eq!(
327+
seen,
328+
[
329+
("g", "1", None, false),
330+
("g", "1", Some("sources"), false),
331+
("g", "3", None, true),
332+
]
333+
);
334+
let (s, e) = deps[0].version_inner.unwrap();
335+
assert_eq!(&pom[s..e], " 1 ", "offsets index the original pom");
336+
let repos = scope.live("repositories").unwrap();
337+
assert_eq!(repos.len(), 1, "the profile's section is not live");
338+
assert_eq!(repos[0].open_tag(pom), "<repositories/>");
339+
assert_eq!(scope.project_close(), pom.rfind("</project>"));
340+
assert!(PomScope::new("<project><![CDATA[ open</project>").is_err());
341+
assert!(PomScope::new("<!-- <project></project> -->").is_err());
342+
}
343+
198344
#[test]
199345
fn suffix_grammar_is_exact() {
200346
assert_eq!(

0 commit comments

Comments
 (0)