Keep Composer redirects and rollback consistent

crates/socket-patch-core/src/formats/composer/hosted.rs

@@ -10,11 +10,11 @@ use std::sync::LazyLock;
 
 use regex::Regex;
 
-use crate::utils::composer_version::composer_versions_equivalent;
 use super::source as composer_source;
 use crate::patch::redirect::{
     artifact_url_present, full_name, DepOverride, RewriteResult, RewriteWarning,
 };
+use crate::utils::composer_version::composer_versions_equivalent;
 
 /// Byte offset of the `}` closing the JSON object that CONTAINS `from`, which
 /// must be a position inside that object. Brace counting skips string literals,
@@ -111,6 +111,17 @@ pub(crate) fn find_composer_entry(content: &str, pkg: &str, version: &str) -> Co
     }
 }
 
+pub(crate) fn find_composer_member(
+    content: &str,
+    entry: (usize, usize),
+    key: &str,
+) -> Option<composer_source::Member> {
+    let object_open = composer_source::entry_object_start(content, entry.0)?;
+    composer_source::top_level_members(content, object_open, entry.1)
+        .into_iter()
+        .find(|member| member.key == key)
+}
+
 /// Append `"shasum": "<sha1>"` as the last key of a `"dist": { … }` block,
 /// indented like the keys already in it. VCS/zipball dists omit `shasum`
 /// entirely; redirecting such a block without inserting the pin left the hosted
@@ -163,7 +174,6 @@ pub(crate) fn rewrite_composer_lock(
         });
         return;
     }
-    const DIST_KEY: &str = "\"dist\": {";
     let mut content = files["composer.lock"].clone();
     let type_re: &Regex = &COMPOSER_DIST_TYPE_RE;
     let url_re: &Regex = &COMPOSER_DIST_URL_RE;
@@ -202,28 +212,17 @@ pub(crate) fn rewrite_composer_lock(
                     continue;
                 }
             };
-        // The dist block MUST belong to the located entry. Scanning forward
-        // from the name for the next `"dist": {` walked into the FOLLOWING
-        // package whenever the target was installed from source, repointing a
-        // bystander's url + shasum — a checksum-clean install of the wrong
-        // code. A target with no dist of its own pins nothing: fail closed.
-        let Some(dist_start) = content[entry_start..=entry_end]
-            .find(DIST_KEY)
-            .map(|offset| entry_start + offset)
+        let Some(dist_member) = find_composer_member(&content, (entry_start, entry_end), "dist")
+            .filter(|member| content.as_bytes()[member.value_start] == b'{')
         else {
             result.warnings.push(RewriteWarning {
                 code: "redirect_composer_no_dist".into(),
                 detail: format!("{composer_name} has no dist block"),
             });
             continue;
         };
-        let Some(dist_end) = json_object_end_from(&content, dist_start + DIST_KEY.len()) else {
-            result.warnings.push(RewriteWarning {
-                code: "redirect_composer_lock_malformed".into(),
-                detail: format!("{composer_name}'s dist block is unterminated"),
-            });
-            continue;
-        };
+        let dist_start = dist_member.key_start;
+        let dist_end = dist_member.value_end;
         // The dist's own members only: a `mirrors` entry listed before the
         // dist `url` would otherwise take the redirected url and then be
         // dropped with the mirrors, leaving the upstream url pinned to the

crates/socket-patch-core/src/patch/redirect/upstream/composer.rs

@@ -27,15 +27,13 @@ use std::collections::BTreeMap;
 
 use serde_json::Value;
 
-use crate::formats::composer::hosted::{
-    find_composer_entry, json_object_end_from, json_string_field, ComposerEntry,
-};
 use super::{Ctx, FormatResult, HostedPin, View};
 use crate::crawlers::composer_crawler::normalize_version;
+use crate::formats::composer::hosted::{
+    find_composer_entry, find_composer_member, json_string_field, ComposerEntry,
+};
 
 const COMPOSER_LOCK: &str = "composer.lock";
-const DIST_KEY: &str = "\"dist\": {";
-const SOURCE_KEY: &str = "\"source\": {";
 /// The `notification-url` composer records for packagist packages.
 const PACKAGIST_NOTIFY: &str = "https://packagist.org/downloads/";
 
@@ -61,9 +59,9 @@ fn is_dev_version(version: &str) -> bool {
 
 /// The `[start, end]` byte range of the entry's `"dist": {…}` object.
 fn dist_range(content: &str, entry: (usize, usize)) -> Option<(usize, usize)> {
-    let start = entry.0 + content[entry.0..=entry.1].find(DIST_KEY)?;
-    let end = json_object_end_from(content, start + DIST_KEY.len())?;
-    Some((start, end))
+    let member = find_composer_member(content, entry, "dist")
+        .filter(|member| content.as_bytes()[member.value_start] == b'{')?;
+    Some((member.key_start, member.value_end))
 }
 
 /// A JSON string literal in the lock's style: `\/` when the lock escapes
@@ -337,7 +335,7 @@ pub(crate) async fn restore(
         // Re-insert the source the rewriter dropped — unless the entry
         // still carries one (a lock redirected before the drop, or a
         // source it could not remove).
-        let has_source = content[start..d_start].contains(SOURCE_KEY);
+        let has_source = find_composer_member(&content, (start, end), "source").is_some();
         let source_text = match upstream.get("source").and_then(Value::as_object) {
             Some(source) if !has_source => {
                 match render_block(

crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/dist-before-name/expected-edits.json

@@ -1 +1,10 @@
-[]
+[
+  {
+    "path": "composer.lock",
+    "kind": "redirect_composer_dist",
+    "action": "rewritten",
+    "key": "monolog/monolog",
+    "original": "\"dist\": {\n                \"type\": \"zip\",\n                \"url\": \"https://api.github.com/repos/Seldaek/monolog/zipball/abc123\",\n                \"reference\": \"abc123def456\",\n                \"shasum\": \"\"\n            },\n            \"name\": \"monolog/monolog\",\n            \"version\": \"2.0.0\",\n            \"source\": {\n                \"type\": \"git\",\n                \"url\": \"https://github.com/Seldaek/monolog.git\",\n                \"reference\": \"abc123def456\"\n            }",
+    "new": "\"dist\": {\n                \"type\": \"zip\",\n                \"url\": \"https://patch.socket.dev/patch/composer/monolog/monolog/2.0.0/11111111-1111-1111-1111-111111111111/44444444-4444-4444-4444-444444444444/monolog-2.0.0.zip\",\n                \"reference\": \"abc123def456\",\n                \"shasum\": \"abcdef0123456789abcdef0123456789abcdef01\"\n            },\n            \"name\": \"monolog/monolog\",\n            \"version\": \"2.0.0\""
+  }
+]

crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/dist-before-name/expected-warnings.json

@@ -1,3 +1 @@
-[
-  "redirect_composer_no_dist"
-]
+[]

crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/nested-dist-before-real/expected-edits.json

@@ -0,0 +1,10 @@
+[
+  {
+    "path": "composer.lock",
+    "kind": "redirect_composer_dist",
+    "action": "rewritten",
+    "key": "monolog/monolog",
+    "original": "\"source\": {\n                \"type\": \"git\",\n                \"url\": \"https://github.com/Seldaek/monolog.git\",\n                \"reference\": \"abc123def456\"\n            },\n            \"extra\": {\n                \"dist\": {\n                    \"type\": \"zip\",\n                    \"url\": \"https://example.com/plugin-metadata.zip\",\n                    \"shasum\": \"\"\n                }\n            },\n            \"dist\": {\n                \"type\": \"zip\",\n                \"url\": \"https://api.github.com/repos/Seldaek/monolog/zipball/abc123\",\n                \"reference\": \"abc123def456\",\n                \"shasum\": \"\"\n            }",
+    "new": "\"extra\": {\n                \"dist\": {\n                    \"type\": \"zip\",\n                    \"url\": \"https://example.com/plugin-metadata.zip\",\n                    \"shasum\": \"\"\n                }\n            },\n            \"dist\": {\n                \"type\": \"zip\",\n                \"url\": \"https://patch.socket.dev/patch/composer/monolog/monolog/2.0.0/11111111-1111-1111-1111-111111111111/44444444-4444-4444-4444-444444444444/monolog-2.0.0.zip\",\n                \"reference\": \"abc123def456\",\n                \"shasum\": \"abcdef0123456789abcdef0123456789abcdef01\"\n            }"
+  }
+]

crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/nested-dist-before-real/expected-warnings.json

@@ -0,0 +1 @@
+[]

crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/nested-dist-before-real/overrides.json

@@ -0,0 +1,14 @@
+[
+  {
+    "ecosystem": "composer",
+    "name": "monolog",
+    "namespace": "monolog",
+    "version": "2.0.0",
+    "token": "11111111-1111-1111-1111-111111111111",
+    "patchUuid": "44444444-4444-4444-4444-444444444444",
+    "artifactUrl": "https://patch.socket.dev/patch/composer/monolog/monolog/2.0.0/11111111-1111-1111-1111-111111111111/44444444-4444-4444-4444-444444444444/monolog-2.0.0.zip",
+    "integrity": {
+      "sha1": "abcdef0123456789abcdef0123456789abcdef01"
+    }
+  }
+]

crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-nested-dist/expected-edits.json

@@ -0,0 +1 @@
+[]

crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-nested-dist/expected-warnings.json

@@ -0,0 +1,3 @@
+[
+  "redirect_composer_no_dist"
+]

crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-nested-dist/overrides.json

@@ -0,0 +1,14 @@
+[
+  {
+    "ecosystem": "composer",
+    "name": "monolog",
+    "namespace": "monolog",
+    "version": "2.0.0",
+    "token": "11111111-1111-1111-1111-111111111111",
+    "patchUuid": "44444444-4444-4444-4444-444444444444",
+    "artifactUrl": "https://patch.socket.dev/patch/composer/monolog/monolog/2.0.0/11111111-1111-1111-1111-111111111111/44444444-4444-4444-4444-444444444444/monolog-2.0.0.zip",
+    "integrity": {
+      "sha1": "abcdef0123456789abcdef0123456789abcdef01"
+    }
+  }
+]