diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8f37c5c3..42379944 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -642,6 +642,12 @@ jobs: suite: e2e_maven - os: ubuntu-latest suite: e2e_composer + # composer is a shipped ecosystem, so e2e_composer's tests are + # NOT `#[ignore]`-gated the way the experimental-ecosystem suites + # (maven, nuget) are — the matrix default `--ignored` filter + # selected zero tests here and the leg passed vacuously. + # `--include-ignored` runs them, plus any capstone added later. + test_filter: --include-ignored - os: ubuntu-latest suite: e2e_nuget # Host vendor build-proof capstones: fresh-checkout install + @@ -799,7 +805,10 @@ jobs: tools: composer:2 - name: Run e2e tests - run: cargo test -p socket-patch-cli --all-features --test ${{ matrix.suite }} -- --ignored + # Suites are `#[ignore]`-gated out of the unpinned `test` job by + # default, hence `--ignored`; an entry that sets `test_filter` + # overrides the selector for itself only. + run: cargo test -p socket-patch-cli --all-features --test ${{ matrix.suite }} -- ${{ matrix.test_filter || '--ignored' }} # ---------------------------------------------------------------------- # Docker-driven real-package e2e suite. diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index ab0b9f68..a7a29236 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -496,7 +496,14 @@ pub(super) async fn run_redirect( } let encoded = socket_patch_core::utils::uri::encode_uri_component(artifact_url); final_texts.iter().any(|text| { - text.contains(artifact_url.as_str()) + // The rewriters' own predicate — raw, or the `\/`-escaped + // slashes an old composer.lock spells them with — so a + // writer's spelling can never be one this probe misses. It + // was: the composer rewriter emitted `\/`-escaped urls this + // probe never looked for, so a fully successful composer + // redirect reported `redirected: 0`, fetched no patch record + // into the ledger, and left the patch unattestable by `vex`. + socket_patch_core::patch::redirect::artifact_url_present(text, artifact_url) // The berry rewriter writes the URL percent-encoded into the // lock's `::__archiveUrl=` binding, so the raw form is absent. || text.contains(encoded.as_str()) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 62d69114..43a68d6f 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -198,33 +198,68 @@ async fn dispatch_in_use_one(entry: &VendorEntry, project_root: &Path) -> Option } } +/// What the orphan sweep did with the uuid dirs no ledger entry owns. +#[derive(Default)] +struct OrphanSweep { + /// Un-ledgered AND unreferenced — deleted (unless `dry_run`). + removed: Vec, + /// Un-ledgered but a project lockfile still points into them — kept. + still_wired: Vec, +} + /// Uuid dirs under `.socket/vendor//` with no owning `(eco, uuid)` /// ledger entry (a hand-edited state file, or artifacts left by an -/// interrupted run). The lockfile wiring for these is already gone or -/// owned by a recorded entry, so removal is safe; removed unless -/// `dry_run`. Unparseable dirs are never returned (and never deleted). -/// Returns the orphans so callers can emit events / counts. -async fn sweep_orphan_vendor_dirs( - cwd: &Path, - state: &VendorState, - dry_run: bool, -) -> Vec { +/// interrupted run). Unparseable dirs are never returned (and never +/// deleted). Returns the orphans so callers can emit events / counts. +/// +/// A missing ledger entry does NOT prove missing wiring: `repair` +/// reconstructs entries from lockfiles that still point into +/// `.socket/vendor/` precisely because that state occurs (a deleted +/// state.json, a partial commit). Deleting such a dir would break the next +/// install, so every candidate is checked against the wiring-bearing files +/// first — the same lockfile scan `repair` reconstructs from — and a +/// referenced dir is kept for the caller to warn about. +async fn sweep_orphan_vendor_dirs(cwd: &Path, state: &VendorState, dry_run: bool) -> OrphanSweep { let recorded_units: HashSet<(&str, &str)> = state .entries .values() .map(|e| (e.ecosystem.as_str(), e.uuid.as_str())) .collect(); - let mut orphans = Vec::new(); - for unit in vendor::path::sweep_vendor_dirs(cwd).await { - if recorded_units.contains(&(unit.eco.as_str(), unit.uuid.as_str())) { + let candidates: Vec = vendor::path::sweep_vendor_dirs(cwd) + .await + .into_iter() + .filter(|unit| !recorded_units.contains(&(unit.eco.as_str(), unit.uuid.as_str()))) + .collect(); + let mut out = OrphanSweep::default(); + if candidates.is_empty() { + return out; + } + let wired: HashSet<(String, String)> = + crate::commands::repair_vendor::scan_vendor_references(cwd) + .await + .into_iter() + .map(|(eco, uuid, _path)| (eco, uuid)) + .collect(); + for unit in candidates { + if wired.contains(&(unit.eco.clone(), unit.uuid.clone())) { + out.still_wired.push(unit); continue; } if !dry_run { let _ = remove_tree(&unit.dir).await; } - orphans.push(unit); + out.removed.push(unit); } - orphans + out +} + +/// How an orphan uuid dir is named in events: the PURL recovered from its +/// leaf when the layout is recognizable, else `/`. +fn orphan_label(unit: &vendor::path::SweptVendorDir) -> String { + unit.purls + .first() + .cloned() + .unwrap_or_else(|| format!("{}/{}", unit.eco, unit.uuid)) } /// Does `eco` fall inside this run's `--ecosystems` scope? @@ -1228,17 +1263,30 @@ async fn run_revert(args: &VendorArgs, env: &mut Envelope) -> i32 { } // Orphan sweep: uuid dirs on disk with no ledger entry (a hand-edited - // state file, or artifacts left by an interrupted run). The lockfile - // wiring for these is already gone or owned by a recorded entry, so - // removal is safe; unparseable dirs are reported, never deleted. - for unit in sweep_orphan_vendor_dirs(&common.cwd, &state, common.dry_run).await { - let label = unit - .purls - .first() - .cloned() - .unwrap_or_else(|| format!("{}/{}", unit.eco, unit.uuid)); + // state file, or artifacts left by an interrupted run). Unparseable dirs + // are reported, never deleted — and neither are dirs a lockfile still + // points at (their wiring outlived the ledger). + let sweep = sweep_orphan_vendor_dirs(&common.cwd, &state, common.dry_run).await; + for unit in &sweep.still_wired { + let label = orphan_label(unit); + record_warning( + env, + &label, + &VendorWarning::new( + "vendor_orphan_still_wired", + format!( + "a project lockfile still points at .socket/vendor/{}/{}, which no ledger \ + entry owns; the artifacts were kept (run `socket-patch repair` to re-adopt \ + them into the ledger, then revert again)", + unit.eco, unit.uuid + ), + ), + common, + ); + } + for unit in &sweep.removed { env.record( - PatchEvent::new(PatchAction::Removed, label) + PatchEvent::new(PatchAction::Removed, orphan_label(unit)) .with_reason("vendor_orphan_removed", "vendored dir had no ledger entry"), ); } @@ -1419,9 +1467,11 @@ pub(crate) async fn run_vendor_gc( } } - // (c) orphan uuid dirs, against the post-removal ledger. + // (c) orphan uuid dirs, against the post-removal ledger. Dirs a lockfile + // still points at are kept, so they are not counted as reclaimed. out.orphan_dirs = sweep_orphan_vendor_dirs(&common.cwd, &state, dry_run) .await + .removed .len(); out } diff --git a/crates/socket-patch-cli/tests/docker_e2e_vendor_composer.rs b/crates/socket-patch-cli/tests/docker_e2e_vendor_composer.rs index 78fcfa00..fd344306 100644 --- a/crates/socket-patch-cli/tests/docker_e2e_vendor_composer.rs +++ b/crates/socket-patch-cli/tests/docker_e2e_vendor_composer.rs @@ -84,8 +84,15 @@ cat > composer.json <<'EOF' EOF # 1. REAL fixture: composer update resolves + installs psr/log from packagist. -composer update --no-interaction > /tmp/install.log 2>&1 || { - cat /tmp/install.log >&2; fail "composer update (fixture install) failed"; } +# psr/log arrives over the real network (packagist metadata + the GitHub +# zipball); transient stream/connection errors are the dominant flake in this +# suite — retry with backoff before declaring the fixture broken. +for attempt in 1 2 3; do + composer update --no-interaction > /tmp/install.log 2>&1 && break + if [ "$attempt" = 3 ]; then cat /tmp/install.log >&2; fail "composer update (fixture install) failed"; fi + echo "composer update attempt $attempt failed; retrying" >&2 + sleep $((attempt * 5)) +done PSR_VER=$(php -r ' $l = json_decode(file_get_contents("composer.lock"), true); diff --git a/crates/socket-patch-cli/tests/e2e_vendor_composer_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_composer_build.rs index 2840a5d2..85e5a288 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_composer_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_composer_build.rs @@ -47,6 +47,8 @@ const GHSA: &str = "GHSA-vend-composer-host"; /// The dependency under test — dep-free, tiny, and the same fixture the /// docker twin uses. const DEP: &str = "psr/log"; +/// Version the hand-written (composer-free) revert fixtures below pin. +const FIXTURE_VERSION: &str = "3.0.2"; // ── self-contained helpers ──────────────────────────────────────────── @@ -476,3 +478,234 @@ fn composer_vendor_fresh_checkout_install_and_revert() { ".socket/vendor must be fully removed after revert" ); } + +// ── revert against ledger state the capstone above never produces ───── +// +// Both regressions below are about `.socket/vendor/` state that outlived (or +// was rebuilt without) its wiring record, which the capstone's clean +// vendor→revert round trip cannot reach. They hand-write the wired lock + +// artifact instead of driving composer, so they need neither the toolchain +// nor the network and run in the normal `test` job (no `#[ignore]`). + +/// `repair`-reconstructed ledger entry: recovered from the lockfile path, so +/// it owns the artifact but records NO pre-vendor wiring (see +/// `repair_vendor.rs`'s `synth_entry`). +const UUID_RECONSTRUCTED: &str = "1a2b3c4d-5e6f-4a7b-8c9d-0e1f2a3b4c5d"; +/// Un-ledgered artifact dir that composer.lock still points at. +const UUID_ORPHAN_WIRED: &str = "2b3c4d5e-6f7a-4b8c-9d0e-1f2a3b4c5d6e"; +/// Un-ledgered artifact dir nothing references. +const UUID_ORPHAN_DEAD: &str = "3c4d5e6f-7a8b-4c9d-8e0f-2a3b4c5d6e7f"; + +const FIXTURE_PHP: &[u8] = + b" String { + let copy_rel = format!(".socket/vendor/composer/{uuid}/{DEP}@{FIXTURE_VERSION}"); + let src = proj.join(©_rel).join("src"); + std::fs::create_dir_all(&src).unwrap(); + std::fs::write(src.join("LoggerInterface.php"), FIXTURE_PHP).unwrap(); + copy_rel +} + +/// composer.json + a composer.lock ALREADY wired to `uuid`'s copy — the +/// exact surgery `vendor` writes (path dist, uuid `reference`, `symlink: +/// false`, `source` gone) and what a fresh clone of a vendored project has. +fn write_wired_project(proj: &Path, uuid: &str) -> String { + let copy_rel = write_vendored_copy(proj, uuid); + std::fs::write( + proj.join("composer.json"), + r#"{ + "name": "socket/vendor-revert-fixture", + "require": { + "psr/log": "3.0.*" + } +} +"#, + ) + .unwrap(); + let lock = serde_json::json!({ + "_readme": ["This file locks the dependencies of your project to a known state"], + "content-hash": "7a59d114f58e9b02546b21d7e57430d3", + "packages": [{ + "name": DEP, + "version": FIXTURE_VERSION, + "dist": { "type": "path", "url": copy_rel, "reference": uuid }, + "transport-options": { "symlink": false }, + "type": "library", + }], + "packages-dev": [], + "minimum-stability": "stable", + "plugin-api-version": "2.6.0", + }); + std::fs::write( + proj.join("composer.lock"), + format!("{}\n", serde_json::to_string_pretty(&lock).unwrap()), + ) + .unwrap(); + copy_rel +} + +/// The `.socket/vendor/state.json` a `repair` reconstruction leaves: artifact +/// + uuid recovered from the lock path, `wiring` empty. +fn write_reconstructed_ledger(proj: &Path, uuid: &str, copy_rel: &str) { + let purl = format!("pkg:composer/{DEP}@{FIXTURE_VERSION}"); + let state = serde_json::json!({ + "version": 1, + "entries": { purl.clone(): { + "ecosystem": "composer", + "basePurl": purl, + "uuid": uuid, + "artifact": { "path": copy_rel }, + "wiring": [], + }} + }); + std::fs::write( + proj.join(".socket/vendor/state.json"), + serde_json::to_string_pretty(&state).unwrap(), + ) + .unwrap(); +} + +fn events(env: &serde_json::Value) -> &Vec { + env["events"].as_array().expect("events[]") +} + +/// REGRESSION: reverting a `repair`-reconstructed entry must not strand +/// composer.lock. There is no recorded registry `dist` to put back, so the +/// revert has to REFUSE and keep the artifacts — deleting them while the lock +/// still points at them made the next `composer install` fail with "Source +/// path … is not found", and the run reported success. +#[test] +fn revert_of_reconstructed_entry_refuses_and_keeps_artifacts() { + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + let copy_rel = write_wired_project(&proj, UUID_RECONSTRUCTED); + write_reconstructed_ledger(&proj, UUID_RECONSTRUCTED, ©_rel); + + let lock_path = proj.join("composer.lock"); + let lock_before = std::fs::read(&lock_path).unwrap(); + let state_before = std::fs::read(proj.join(".socket/vendor/state.json")).unwrap(); + + let (code, stdout, stderr) = run_socket( + &proj, + &[ + "vendor", + "--revert", + "--json", + "--cwd", + proj.to_str().unwrap(), + ], + ); + assert_eq!( + code, 1, + "an unrestorable entry must fail the revert.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = parse_envelope(&stdout); + assert_eq!(env["status"], "partialFailure", "envelope: {env}"); + assert_eq!(env["summary"]["removed"], 0, "nothing reverted: {env}"); + let failed = events(&env) + .iter() + .find(|e| e["action"] == "failed") + .unwrap_or_else(|| panic!("expected a failed event: {env}")); + assert_eq!(failed["errorCode"], "revert_failed", "{failed}"); + let detail = failed["error"].as_str().expect("error detail"); + assert!( + detail.contains(DEP) && detail.contains("composer update"), + "the refusal must name the package and the re-resolve escape hatch: {detail}" + ); + + assert!( + proj.join(©_rel) + .join("src/LoggerInterface.php") + .exists(), + "a refused revert must NOT delete the artifacts the lock still consumes" + ); + assert_eq!( + std::fs::read(&lock_path).unwrap(), + lock_before, + "composer.lock must be left exactly as it was" + ); + assert_eq!( + std::fs::read(proj.join(".socket/vendor/state.json")).unwrap(), + state_before, + "the entry must stay in the ledger so a later repair/revert can retry" + ); +} + +/// REGRESSION: with state.json gone, the orphan sweep must not delete a uuid +/// dir composer.lock still points at — un-ledgered does not mean un-wired +/// (that is exactly the state `repair` reconstructs from). A genuinely +/// unreferenced dir in the same run must still be swept. +#[test] +fn orphan_sweep_keeps_lock_referenced_dir_when_ledger_is_gone() { + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + let wired_rel = write_wired_project(&proj, UUID_ORPHAN_WIRED); + let dead_rel = write_vendored_copy(&proj, UUID_ORPHAN_DEAD); + assert!( + !proj.join(".socket/vendor/state.json").exists(), + "fixture models a project whose ledger was deleted" + ); + + let lock_before = std::fs::read(proj.join("composer.lock")).unwrap(); + let (code, stdout, stderr) = run_socket( + &proj, + &[ + "vendor", + "--revert", + "--json", + "--cwd", + proj.to_str().unwrap(), + ], + ); + assert_eq!( + code, 0, + "sweeping is not a failure.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = parse_envelope(&stdout); + + assert!( + proj.join(&wired_rel) + .join("src/LoggerInterface.php") + .exists(), + "the lock-referenced artifact must survive the sweep: {env}" + ); + assert!( + !proj + .join(format!(".socket/vendor/composer/{UUID_ORPHAN_DEAD}")) + .exists(), + "the unreferenced orphan must still be swept: {env}" + ); + assert_eq!( + std::fs::read(proj.join("composer.lock")).unwrap(), + lock_before, + "the sweep must not touch composer.lock" + ); + assert!( + events(&env) + .iter() + .any(|e| e["errorCode"] == "vendor_orphan_still_wired" + && e["reason"] + .as_str() + .is_some_and(|r| r.contains(UUID_ORPHAN_WIRED))), + "the kept dir must be surfaced as an advisory: {env}" + ); + assert!( + events(&env).iter().any(|e| e["action"] == "removed" + && e["errorCode"] == "vendor_orphan_removed" + && e["purl"] + .as_str() + .is_some_and(|p| p.contains(&format!("{DEP}@{FIXTURE_VERSION}")) + || p.contains(UUID_ORPHAN_DEAD))), + "the swept dir must be reported: {env}" + ); + assert_eq!( + dead_rel, + format!(".socket/vendor/composer/{UUID_ORPHAN_DEAD}/{DEP}@{FIXTURE_VERSION}"), + "fixture path convention" + ); +} diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index bd4a414e..52c2d61c 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -2407,6 +2407,195 @@ async fn scan_updates_reports_superseding_patch_for_ledger_only_project() { assert_eq!(updates[0]["newUuid"], UUID); } +// ── composer ───────────────────────────────────────────────────────────── + +const COMPOSER_PURL: &str = "pkg:composer/monolog/monolog@2.0.0"; +const COMPOSER_UUID: &str = "66666666-6666-4666-8666-666666666666"; +const COMPOSER_URL: &str = "http://patch.test/patch/composer/monolog/monolog/2.0.0/\ + 77777777-7777-4777-8777-777777777777/\ + 66666666-6666-4666-8666-666666666666/monolog-2.0.0.zip"; +const COMPOSER_SHA1: &str = "abcdef0123456789abcdef0123456789abcdef01"; + +/// A composer project discovered through `vendor/composer/installed.json`, +/// with the lock the redirect rewriter edits. The lock is composer-native: +/// `JSON_UNESCAPED_SLASHES`, 4-space indent. +fn write_composer_project(root: &Path) { + std::fs::write( + root.join("composer.json"), + "{ \"require\": { \"monolog/monolog\": \"2.0.0\" } }\n", + ) + .unwrap(); + let installed = root.join("vendor").join("composer"); + std::fs::create_dir_all(&installed).unwrap(); + std::fs::write( + installed.join("installed.json"), + r#"{ "packages": [ { "name": "monolog/monolog", "version": "2.0.0" } ] } +"#, + ) + .unwrap(); + std::fs::create_dir_all(root.join("vendor/monolog/monolog")).unwrap(); + std::fs::write( + root.join("composer.lock"), + r#"{ + "content-hash": "abc123def456abc123def456abc1", + "packages": [ + { + "name": "monolog/monolog", + "version": "2.0.0", + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/Seldaek/monolog/zipball/abc123", + "reference": "abc123def456", + "shasum": "" + } + } + ], + "packages-dev": [] +} +"#, + ) + .unwrap(); +} + +async fn mock_composer_api(server: &MockServer) { + Mock::given(method("POST")) + .and(path(format!("/v0/orgs/{ORG}/patches/batch"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "packages": [{ + "purl": COMPOSER_PURL, + "patches": [{ + "uuid": COMPOSER_UUID, "purl": COMPOSER_PURL, "tier": "free", + "cveIds": [], "ghsaIds": [], "severity": "high", + "title": "composer redirect fixture" + }] + }], + "canAccessPaidPatches": false, + }))) + .mount(server) + .await; + Mock::given(method("GET")) + .and(path_regex(format!( + "^/v0/orgs/{ORG}/patches/by-package/.+$" + ))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "patches": [{ + "uuid": COMPOSER_UUID, "purl": COMPOSER_PURL, + "publishedAt": "2024-01-01T00:00:00Z", + "description": "x", "license": "MIT", "tier": "free", + "vulnerabilities": {} + }], + "canAccessPaidPatches": false, + }))) + .mount(server) + .await; + // composer pins `dist.shasum` — a sha1, not npm's sha512. + Mock::given(method("POST")) + .and(path(format!("/v0/orgs/{ORG}/patches/package"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "results": { + COMPOSER_UUID: { + "status": "granted", + "url": COMPOSER_URL, + "purl": COMPOSER_PURL, + "artifacts": [{ + "kind": "tarball", + "url": COMPOSER_URL, + "integrity": { "sha1": COMPOSER_SHA1 } + }], + "registryOverride": null + } + } + }))) + .mount(server) + .await; + Mock::given(method("GET")) + .and(path(format!("/v0/orgs/{ORG}/patches/view/{COMPOSER_UUID}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "uuid": COMPOSER_UUID, + "purl": COMPOSER_PURL, + "publishedAt": "2024-01-01T00:00:00Z", + "files": { + "src/Logger.php": { + "beforeHash": "a".repeat(64), + "afterHash": "b".repeat(64), + } + }, + "vulnerabilities": { + GHSA: { + "cves": ["CVE-2024-9"], + "summary": "composer redirect fixture", + "severity": "high", + "description": "d" + } + }, + "description": "x", "license": "MIT", "tier": "free" + }))) + .mount(server) + .await; +} + +/// End-to-end regression for the composer redirect that reported itself as +/// having done nothing: the rewriter wrote the hosted url with `\/`-escaped +/// slashes while the post-rewrite confirmation probe searched only the raw and +/// percent-encoded spellings, so a fully successful rewrite yielded +/// `redirected: 0`, no patch record in the ledger, and nothing for `vex` to +/// attest. The rewriter now emits composer-native raw slashes and the probe +/// asks the rewriter's own predicate, so the lock edit and the confirmation +/// cannot disagree. Subprocess so the `--json` envelope can be read back. +#[tokio::test] +#[serial] +async fn composer_redirect_is_confirmed_and_recorded() { + let server = MockServer::start().await; + mock_composer_api(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + write_composer_project(tmp.path()); + + let env = run_redirect_subprocess(tmp.path(), &server.uri()); + assert_eq!(env["status"], "success", "envelope: {env}"); + assert_eq!( + env["redirect"]["redirected"], 1, + "the composer redirect must be CONFIRMED, not silently unconfirmed: {env}" + ); + assert_eq!( + env["redirect"]["rewrittenFiles"][0], "composer.lock", + "composer.lock must be the rewritten file: {env}" + ); + assert!( + warning_codes(&env).is_empty(), + "a clean composer redirect emits no warnings; got {:?}", + warning_codes(&env) + ); + + let lock = std::fs::read_to_string(tmp.path().join("composer.lock")).unwrap(); + assert!( + lock.contains(&format!("\"url\": \"{COMPOSER_URL}\"")), + "dist.url must be the hosted patch with composer-native raw slashes; got:\n{lock}" + ); + assert!( + !lock.contains("\\/"), + "composer writes lock JSON with JSON_UNESCAPED_SLASHES — no escaped slashes may \ + be introduced; got:\n{lock}" + ); + assert!( + lock.contains(&format!("\"shasum\": \"{COMPOSER_SHA1}\"")), + "dist.shasum must pin the patched artifact's sha1; got:\n{lock}" + ); + + // The confirmation is what drives the record fetch: no confirmation, no + // record, and `socket-patch vex` can never attest the patch. + let ledger = + std::fs::read_to_string(tmp.path().join(".socket/vendor/redirect-state.json")).unwrap(); + assert!( + ledger.contains(COMPOSER_PURL) && ledger.contains(GHSA), + "the ledger must carry the fetched patch record for the redirected purl: {ledger}" + ); + assert!( + ledger.contains("redirect_composer_dist"), + "the ledger must carry the revert edit for the lock rewrite: {ledger}" + ); +} + /// Mount the full cargo hosted-mock set (discovery + reference + view) for /// one patch over `purl`. async fn mock_cargo_patch( diff --git a/crates/socket-patch-cli/tests/setup_matrix_composer.rs b/crates/socket-patch-cli/tests/setup_matrix_composer.rs index c6eeabb3..79aa2257 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_composer.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_composer.rs @@ -48,8 +48,11 @@ mod host_guard { /// A realistic composer-only project: a PHP manifest requiring the /// same package the matrix targets, and nothing the npm/Python/Cargo - /// detectors would recognise. - const COMPOSER_JSON: &str = "{\n \"name\": \"acme/widget\",\n \"require\": {\n \"monolog/monolog\": \"3.5.0\"\n }\n}\n"; + /// detectors would recognise. Indented with 4 spaces, the way composer + /// itself writes the file (PHP `JSON_PRETTY_PRINT`) — so the + /// byte-for-byte restore below also pins that `setup` does not reformat + /// a composer-authored manifest to serde's 2-space default. + const COMPOSER_JSON: &str = "{\n \"name\": \"acme/widget\",\n \"require\": {\n \"monolog/monolog\": \"3.5.0\"\n }\n}\n"; /// Run the CLI with `args` in `cwd`; returns `(exit_code, stdout, stderr)`. /// Delegates to the shared `common::run_with_env`, which seeds-then-scrubs diff --git a/crates/socket-patch-core/src/crawlers/composer_crawler.rs b/crates/socket-patch-core/src/crawlers/composer_crawler.rs index 63904a9f..f684b613 100644 --- a/crates/socket-patch-core/src/crawlers/composer_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/composer_crawler.rs @@ -10,12 +10,18 @@ use crate::utils::process::{CommandRunner, SystemCommandRunner}; /// vendor directories. pub struct ComposerCrawler; -/// A single package entry distilled from installed.json. Only the two +/// A single package entry distilled from installed.json. Only the three /// fields the crawler needs are retained; everything else (source, /// dist, autoload, ...) is ignored. struct ComposerPackageEntry { name: String, version: String, + /// `install-path` as recorded, relative to the `vendor/composer/` + /// directory that holds installed.json (`../monolog/monolog` for a + /// conventional install, `../../web/app/plugins/x` for a + /// composer/installers target). `None` for Composer 1 entries and any + /// entry that omits it. + install_path: Option, } impl ComposerCrawler { @@ -29,9 +35,11 @@ impl ComposerCrawler { /// In global mode, checks `$COMPOSER_HOME/vendor/` (env var, command /// fallback, or platform defaults). /// - /// In local mode, checks `/vendor/` but only if the directory - /// contains `composer/installed.json` and the cwd looks like a PHP - /// project (`composer.json` or `composer.lock` present). + /// In local mode, checks the project's vendor directory + /// (`COMPOSER_VENDOR_DIR` / composer.json `config.vendor-dir` / + /// `vendor`, see [`resolve_local_vendor_dir`]) but only if the + /// directory contains `composer/installed.json` and the cwd looks like + /// a PHP project (`composer.json` or `composer.lock` present). pub async fn get_vendor_paths( &self, options: &CrawlerOptions, @@ -51,7 +59,9 @@ impl ComposerCrawler { } // Local mode - let vendor_dir = options.cwd.join("vendor"); + let Some(vendor_dir) = resolve_local_vendor_dir(&options.cwd).await else { + return Ok(Vec::new()); + }; let installed_json = vendor_dir.join("composer").join("installed.json"); if !is_dir(&vendor_dir).await || !is_file(&installed_json).await { @@ -77,15 +87,18 @@ impl ComposerCrawler { let vendor_paths = self.get_vendor_paths(options).await.unwrap_or_default(); for vendor_path in &vendor_paths { + let project_root = resolve_project_root(vendor_path).await; let entries = read_installed_json(vendor_path).await; for entry in entries { if let Some((namespace, name)) = entry.name.split_once('/') { // Skip packages that installed.json lists but that are - // not actually on disk (stale metadata, custom install - // paths). This keeps crawl_all consistent with - // find_by_purls, which only returns packages whose - // vendor directory exists. - let pkg_path = vendor_path.join(namespace).join(name); + // not actually on disk (stale metadata, a metapackage). + // This keeps crawl_all consistent with find_by_purls, + // which only returns packages whose directory exists. + let Some(pkg_path) = resolve_package_dir(vendor_path, &project_root, &entry) + else { + continue; + }; if !is_dir(&pkg_path).await { continue; } @@ -138,14 +151,15 @@ impl ComposerCrawler { // package names are case-insensitive and the canonical PURL is // lowercase, but installed.json records the *pretty* (case-preserved) // name and Composer writes the vendor directory with that same - // casing. Key the map by the lowercased name and carry the original - // name so the real on-disk path can be reconstructed even on + // casing. Key the map by the lowercased name and carry the whole + // entry so the real on-disk path can be reconstructed even on // case-sensitive filesystems. let entries = read_installed_json(vendor_path).await; - let installed: HashMap = entries + let installed: HashMap = entries .into_iter() - .map(|e| (e.name.to_ascii_lowercase(), (e.name, e.version))) + .map(|e| (e.name.to_ascii_lowercase(), e)) .collect(); + let project_root = resolve_project_root(vendor_path).await; for purl in purls { if let Some(((namespace, name), version)) = @@ -153,7 +167,7 @@ impl ComposerCrawler { { let full_name = format!("{namespace}/{name}").to_ascii_lowercase(); - let Some((installed_name, installed_version)) = installed.get(&full_name) else { + let Some(entry) = installed.get(&full_name) else { continue; }; @@ -161,17 +175,17 @@ impl ComposerCrawler { // normalized version so a `v`-prefixed installed.json // version (`v6.4.1`) matches a bare PURL version (`6.4.1`) // and vice versa. - if normalize_version(installed_version) != normalize_version(version) { + if normalize_version(&entry.version) != normalize_version(version) { continue; } - // Resolve the on-disk directory using the original casing - // recorded in installed.json, which is what Composer wrote to - // disk — the canonical (lowercase) PURL name would miss it on - // a case-sensitive filesystem. - let pkg_dir = match installed_name.split_once('/') { - Some((ns, n)) => vendor_path.join(ns).join(n), - None => continue, + // Resolve the on-disk directory from installed.json's own + // record — its `install-path` when present, else the + // conventional layout under the original (case-preserved) + // casing Composer wrote to disk; the canonical (lowercase) + // PURL name would miss it on a case-sensitive filesystem. + let Some(pkg_dir) = resolve_package_dir(vendor_path, &project_root, entry) else { + continue; }; if !is_dir(&pkg_dir).await { @@ -282,6 +296,222 @@ pub(crate) fn normalize_version(version: &str) -> &str { version } +/// How far above the vendor directory [`resolve_project_root`] looks for +/// the composer manifest. `config.vendor-dir` may nest the vendor tree +/// (`lib/deps`), so the project root is not always the immediate parent; +/// the walk is bounded so an unrelated `composer.json` far up the +/// filesystem can't widen the write boundary. +const PROJECT_ROOT_SEARCH_DEPTH: usize = 3; + +/// Read `config.vendor-dir` out of a composer.json body, mirroring the +/// slice of Composer's `Config::get('vendor-dir')` that matters here: +/// trailing separators are trimmed (`"vendor/"` is legal) and an empty +/// value counts as unset. +/// +/// Composer additionally expands `$HOME`/`~`/`%VAR%` placeholders +/// (`Platform::expandPath`); that is deliberately NOT implemented — an +/// unexpanded value simply resolves to a directory that does not exist, +/// so discovery reports nothing rather than guessing at a path. +fn parse_config_vendor_dir(composer_json: &str) -> Option { + let doc: serde_json::Value = serde_json::from_str(composer_json).ok()?; + let raw = doc.get("config")?.get("vendor-dir")?.as_str()?; + let trimmed = raw.trim_end_matches(['/', '\\']); + (!trimmed.is_empty()).then(|| trimmed.to_string()) +} + +/// Resolve the vendor directory of a local project the way Composer does: +/// `COMPOSER_VENDOR_DIR` wins, else composer.json `config.vendor-dir` +/// (relative to the manifest directory), else `vendor`. +/// +/// Composer relocates the WHOLE vendor tree — `composer/installed.json` +/// included — so assuming `/vendor` makes every installed package +/// invisible: scan reports them as lockfile-only ("not yet installed") +/// and apply resolves them as `package_not_found`. +/// +/// Returns `None` when composer.json configures a value that is not a +/// plain relative subpath. That value comes from the project being +/// scanned and names the directory apply later WRITES patch content into, +/// so `../../elsewhere` or `/etc` is refused outright rather than +/// silently downgraded to `vendor/` (which would patch an unrelated +/// tree). Absolute paths are legal in Composer but are refused for the +/// same reason; a project using one discovers nothing, exactly as today. +/// `COMPOSER_VENDOR_DIR` is not gated — it comes from the invoking +/// environment, the same trust level as `CARGO_HOME` / `NUGET_PACKAGES` / +/// `MAVEN_REPO_LOCAL`, which are all honored verbatim. +async fn resolve_local_vendor_dir(cwd: &Path) -> Option { + // A set-but-empty value counts as unset (twin of the MAVEN_REPO_LOCAL + // and NUGET_PACKAGES rules): honoring `""` would resolve the vendor + // tree to the project root itself. + if let Some(from_env) = std::env::var("COMPOSER_VENDOR_DIR") + .ok() + .map(|v| v.trim_end_matches(['/', '\\']).to_string()) + .filter(|v| !v.is_empty()) + { + // `join` substitutes an absolute value for the base, matching + // Composer's own relative-to-the-manifest-dir resolution. + return Some(cwd.join(from_env)); + } + + match read_config_vendor_dir(&cwd.join("composer.json")).await { + Some(configured) => normalize_config_vendor_dir(&configured) + .filter(|normalized| path_safety::is_safe_multi_segment(normalized)) + .map(|normalized| cwd.join(normalized)), + None => Some(cwd.join("vendor")), + } +} + +/// Reduce a `config.vendor-dir` value to plain `a/b` segments before the +/// safety gate. Composer accepts `./`-prefixed and `.`-interleaved values +/// (`./vendor`, `lib/./deps`) and either separator; refusing those shapes +/// outright regressed projects that previously resolved fine at the +/// hardcoded `vendor/`. `..` is resolved lexically the way Composer's own +/// path resolution does; a value that climbs above the project root (or +/// reduces to it) fails closed as `None`. +fn normalize_config_vendor_dir(raw: &str) -> Option { + if raw.starts_with(['/', '\\']) { + return None; + } + let mut segments: Vec<&str> = Vec::new(); + for segment in raw.split(['/', '\\']) { + match segment { + "" | "." => {} + ".." => { + segments.pop()?; + } + other => segments.push(other), + } + } + (!segments.is_empty()).then(|| segments.join("/")) +} + +/// Read `config.vendor-dir` from a composer.json on disk. Opened with +/// [`crate::utils::fs::open_regular_file`] for the same reason +/// installed.json is: the manifest belongs to the untrusted project, and +/// a FIFO planted at that path would wedge a plain read forever. +async fn read_config_vendor_dir(manifest_path: &Path) -> Option { + use tokio::io::AsyncReadExt; + + let (mut file, metadata) = crate::utils::fs::open_regular_file(manifest_path) + .await + .ok()?; + let mut content = String::with_capacity(metadata.len() as usize); + file.read_to_string(&mut content).await.ok()?; + parse_config_vendor_dir(&content) +} + +/// The directory an installed.json `install-path` may not escape. +/// +/// `install-path` legitimately points OUTSIDE the vendor tree — that is +/// the entire point of composer/installers (`extra.installer-paths`, +/// `type: wordpress-plugin`) — so the boundary cannot be the vendor root. +/// But installed.json is untrusted, tamperable input and the resolved +/// directory is a patch WRITE target, so it must stay inside the project: +/// the nearest ancestor of the vendor directory carrying a composer +/// manifest, else the vendor directory's immediate parent. +/// +/// Derived from the vendor path alone so scan (`crawl_all`) and apply +/// (`find_by_purls`, which is only ever handed the vendor directory) +/// agree on the boundary; disagreeing would surface packages in scan that +/// apply then refuses to resolve. +async fn resolve_project_root(vendor_path: &Path) -> PathBuf { + let mut fallback = None; + for ancestor in vendor_path + .ancestors() + .skip(1) + .take(PROJECT_ROOT_SEARCH_DEPTH) + { + if fallback.is_none() { + fallback = Some(ancestor.to_path_buf()); + } + if is_file(&ancestor.join("composer.json")).await + || is_file(&ancestor.join("composer.lock")).await + { + return normalize_lexically(ancestor).unwrap_or_else(|| ancestor.to_path_buf()); + } + } + let root = fallback.unwrap_or_else(|| vendor_path.to_path_buf()); + normalize_lexically(&root).unwrap_or(root) +} + +/// Resolve `.`/`..` without touching the filesystem, so a path can be +/// containment-checked BEFORE it is opened (a canonicalizing check would +/// have to stat the very path being validated, and would fail on +/// not-yet-existing directories). Returns `None` when `..` pops above the +/// path's own root — nothing legitimate does that, so it fails closed. +/// +/// Symlinks are not resolved: a symlink INSIDE the project pointing out +/// of it is a pre-existing trust decision of the project's own tree, the +/// same assumption the rest of the crawler layer makes. +fn normalize_lexically(path: &Path) -> Option { + use std::path::Component; + + let mut out = PathBuf::new(); + let mut depth = 0usize; + for component in path.components() { + match component { + Component::Prefix(_) | Component::RootDir => out.push(component.as_os_str()), + Component::CurDir => {} + Component::ParentDir => { + if depth == 0 { + return None; + } + out.pop(); + depth -= 1; + } + Component::Normal(segment) => { + out.push(segment); + depth += 1; + } + } + } + Some(out) +} + +/// Resolve an installed.json `install-path` against the vendor tree. +/// +/// Composer records it relative to `vendor/composer/` (the directory +/// holding installed.json), not to the vendor root. Returns `None` when +/// the resolved directory leaves `project_root` +/// ([`resolve_project_root`]) — fail closed, no fallback to +/// `vendor//`: an entry that claims to live somewhere out of +/// tree must not redirect the patch onto an unrelated directory. +fn resolve_install_path( + vendor_path: &Path, + project_root: &Path, + install_path: &str, +) -> Option { + if install_path.contains('\0') { + return None; + } + // `join` substitutes an absolute `install-path` (Composer writes one + // for some path repositories) for the base; the containment check + // below is what keeps it in bounds either way. + let joined = vendor_path.join("composer").join(install_path); + let resolved = normalize_lexically(&joined)?; + resolved.starts_with(project_root).then_some(resolved) +} + +/// The on-disk directory of an installed.json entry. +/// +/// `install-path` is authoritative when recorded: Composer 2 writes it for +/// every package, and for composer/installers targets (WordPress plugins, +/// Drupal modules, `extra.installer-paths`) it is the ONLY record of where +/// the package really lives. Entries without one (Composer 1, hand-written +/// metadata) keep the conventional `vendor//` layout. +fn resolve_package_dir( + vendor_path: &Path, + project_root: &Path, + entry: &ComposerPackageEntry, +) -> Option { + match entry.install_path.as_deref() { + Some(install_path) => resolve_install_path(vendor_path, project_root, install_path), + None => { + let (namespace, name) = entry.name.split_once('/')?; + Some(vendor_path.join(namespace).join(name)) + } + } +} + /// Whether an installed.json package name is safe to join onto the /// vendor root. Both `crawl_all` and `find_by_purls` split the recorded /// name at `/` and join the pieces onto the vendor directory, and the @@ -351,9 +581,19 @@ async fn read_installed_json(vendor_path: &Path) -> Vec { if name.is_empty() || version.is_empty() || !is_safe_composer_name(name) { return None; } + // `install-path` is NOT gated here: it is legitimately a + // `..`-prefixed path out of `vendor/composer/`, so the + // coordinate gate cannot be a per-segment one. It is validated + // when resolved instead ([`resolve_install_path`]). + let install_path = entry + .get("install-path") + .and_then(|p| p.as_str()) + .filter(|p| !p.is_empty()) + .map(str::to_string); Some(ComposerPackageEntry { name: name.to_string(), version: version.to_string(), + install_path, }) }) .collect() @@ -1085,6 +1325,143 @@ mod tests { ); } + #[test] + fn test_parse_config_vendor_dir() { + assert_eq!( + parse_config_vendor_dir(r#"{"config":{"vendor-dir":"lib/deps"}}"#).as_deref(), + Some("lib/deps") + ); + // Composer rtrims trailing separators before using the value. + assert_eq!( + parse_config_vendor_dir(r#"{"config":{"vendor-dir":"lib/deps/"}}"#).as_deref(), + Some("lib/deps") + ); + // No config block, no key, wrong type, empty value, malformed JSON — + // all "unset", so the caller falls back to `vendor`. + assert_eq!(parse_config_vendor_dir("{}"), None); + assert_eq!(parse_config_vendor_dir(r#"{"config":{}}"#), None); + assert_eq!( + parse_config_vendor_dir(r#"{"config":{"vendor-dir":7}}"#), + None + ); + assert_eq!( + parse_config_vendor_dir(r#"{"config":{"vendor-dir":""}}"#), + None + ); + assert_eq!( + parse_config_vendor_dir(r#"{"config":{"vendor-dir":"/"}}"#), + None + ); + assert_eq!(parse_config_vendor_dir("{ not json"), None); + } + + #[test] + fn test_normalize_config_vendor_dir() { + let n = normalize_config_vendor_dir; + // Composer-legal `./` prefixes and `.` segments reduce to the + // plain path; either separator is accepted. + assert_eq!(n("./vendor").as_deref(), Some("vendor")); + assert_eq!(n("./lib/deps").as_deref(), Some("lib/deps")); + assert_eq!(n("lib/./deps").as_deref(), Some("lib/deps")); + assert_eq!(n("lib\\deps").as_deref(), Some("lib/deps")); + assert_eq!(n("lib/../deps").as_deref(), Some("deps")); + assert_eq!(n("vendor").as_deref(), Some("vendor")); + // Escaping the project, reducing to it, or absolute — fail closed. + assert_eq!(n(".."), None); + assert_eq!(n("../elsewhere"), None); + assert_eq!(n("lib/../.."), None); + assert_eq!(n("."), None); + assert_eq!(n("a/.."), None); + assert_eq!(n("/etc/vendor"), None); + assert_eq!(n("\\\\share\\vendor"), None); + // A drive-letter segment survives normalization; the + // `is_safe_multi_segment` gate downstream rejects the colon. + assert_eq!( + n("C:\\Users\\x\\vendor").as_deref(), + Some("C:/Users/x/vendor") + ); + assert!(!crate::patch::path_safety::is_safe_multi_segment( + "C:/Users/x/vendor" + )); + } + + #[test] + fn test_normalize_lexically() { + let n = |p: &str| normalize_lexically(Path::new(p)); + // `.` drops out, `..` pops the previous segment. + assert_eq!( + n("/a/b/composer/../monolog/monolog").unwrap(), + PathBuf::from("/a/b/monolog/monolog") + ); + assert_eq!( + n("/a/b/composer/./installers").unwrap(), + PathBuf::from("/a/b/composer/installers") + ); + assert_eq!(n("/a/b/c/../../../web/x").unwrap(), PathBuf::from("/web/x")); + // Popping above the path's own root fails closed. + assert_eq!(n("/a/../.."), None); + assert_eq!(n("../x"), None); + // Relative paths stay relative. + assert_eq!(n("a/b/../c").unwrap(), PathBuf::from("a/c")); + } + + #[tokio::test] + async fn test_resolve_project_root_finds_manifest_above_nested_vendor() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("proj"); + let vendor = root.join("lib").join("deps"); + tokio::fs::create_dir_all(&vendor).await.unwrap(); + tokio::fs::write(root.join("composer.json"), "{}") + .await + .unwrap(); + + // A `config.vendor-dir` can nest the vendor tree, so the project root + // is not the vendor dir's parent — it is the nearest ancestor holding + // the manifest. + assert_eq!(resolve_project_root(&vendor).await, root); + + // With no manifest anywhere above, the immediate parent is the + // boundary rather than an unbounded walk up the filesystem. + let orphan = dir.path().join("orphan").join("vendor"); + tokio::fs::create_dir_all(&orphan).await.unwrap(); + assert_eq!( + resolve_project_root(&orphan).await, + dir.path().join("orphan") + ); + } + + #[tokio::test] + async fn test_crawl_all_without_install_path_uses_conventional_layout() { + // Composer 1 (and hand-written metadata) records no install-path; + // those entries must keep resolving to vendor//. + let dir = tempfile::tempdir().unwrap(); + let vendor_dir = dir.path().join("vendor"); + let composer_dir = vendor_dir.join("composer"); + tokio::fs::create_dir_all(&composer_dir).await.unwrap(); + tokio::fs::write( + composer_dir.join("installed.json"), + r#"[{"name": "monolog/monolog", "version": "3.5.0"}]"#, + ) + .await + .unwrap(); + tokio::fs::create_dir_all(vendor_dir.join("monolog").join("monolog")) + .await + .unwrap(); + tokio::fs::write(dir.path().join("composer.json"), "{}") + .await + .unwrap(); + + let crawler = ComposerCrawler::new(); + let options = CrawlerOptions { + cwd: dir.path().to_path_buf(), + global: false, + global_prefix: None, + }; + let packages = crawler.crawl_all(&options).await; + assert_eq!(packages.len(), 1); + assert_eq!(packages[0].path, vendor_dir.join("monolog").join("monolog")); + } + #[tokio::test] async fn test_find_by_purls_version_mismatch() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 556b5091..ddf3f9ec 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -20,6 +20,7 @@ use regex::Regex; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; +use crate::crawlers::composer_crawler::normalize_version; use crate::crawlers::python_crawler::canonicalize_pypi_name; use crate::vendor::yarn_berry_lock::yarnrc_compression_level; @@ -2217,6 +2218,125 @@ fn rewrite_uv_lock( } // ── composer.lock ──────────────────────────────────────────────────────────── +/// Whether `text` points at `artifact_url` in any spelling a rewritten file may +/// carry: the raw url every rewriter emits — composer.lock included, since +/// composer writes its lock through PHP's `JSON_UNESCAPED_SLASHES` — or the +/// `\/`-escaped slashes an older composer wrote, which redirect the install just +/// as well. Shared by the composer rewriter's already-redirected check and the +/// CLI's post-rewrite confirmation probe so the writer's spelling and the +/// probe's cannot drift: the probe searched only raw and percent-encoded urls +/// while the composer rewriter emitted `\/`, so a fully successful composer +/// redirect reported nothing redirected — no patch record reached the ledger and +/// `vex` had nothing to attest. +pub fn artifact_url_present(text: &str, artifact_url: &str) -> bool { + text.contains(artifact_url) || text.contains(&artifact_url.replace('/', "\\/")) +} + +/// Byte offset of the `}` closing the JSON object that CONTAINS `from`, which +/// must be a position inside that object. Brace counting skips string literals, +/// so a brace inside a description or URL cannot move the boundary. +fn json_object_end_from(text: &str, from: usize) -> Option { + let mut depth = 0usize; + let mut in_string = false; + let mut escaped = false; + for (offset, ch) in text[from..].char_indices() { + if in_string { + match ch { + _ if escaped => escaped = false, + '\\' => escaped = true, + '"' => in_string = false, + _ => {} + } + continue; + } + match ch { + '"' => in_string = true, + '{' => depth += 1, + '}' if depth == 0 => return Some(from + offset), + '}' => depth -= 1, + _ => {} + } + } + None +} + +/// Value of the first `"": ""` pair in `text` (composer writes its +/// lock with exactly one space after the colon, the same shape the surgical +/// `dist` regexes below assume). +fn json_string_field<'a>(text: &'a str, key: &str) -> Option<&'a str> { + let pattern = format!("\"{key}\": \""); + let start = text.find(&pattern)? + pattern.len(); + let end = text[start..].find('"')? + start; + Some(&text[start..end]) +} + +/// Outcome of locating a package entry in a composer.lock. +enum ComposerEntry { + /// Inclusive byte range from the entry's `"name"` key to the `}` closing + /// the entry — composer writes `name` first, so this covers every key the + /// rewriter edits. + Found(usize, usize), + /// The name matched but the lock pins this OTHER version. + VersionMismatch(String), + NotFound, +} + +/// Locate `pkg`'s entry in a composer.lock (either `packages[]` or +/// `packages-dev[]` — the scan is over the whole file). +/// +/// Names match CASE-INSENSITIVELY, the way the composer crawler and the vendor +/// backend already match them: packagist canonicalizes to lowercase, but +/// hand-written mixed-case locks install fine and would otherwise silently miss +/// the redirect. The locked version must match the patched one through +/// composer's leading-`v` normalization (locks carry the pretty `v6.4.1`, PURLs +/// the bare `6.4.1`); matching on name alone repointed whatever version the +/// lock happened to hold at a patch built for a different one. +fn find_composer_entry(content: &str, pkg: &str, version: &str) -> ComposerEntry { + let mut mismatched: Option = None; + for (name_idx, _) in content.match_indices("\"name\": \"") { + let Some(end) = json_object_end_from(content, name_idx) else { + continue; + }; + let entry = &content[name_idx..=end]; + if !json_string_field(entry, "name").is_some_and(|n| n.eq_ignore_ascii_case(pkg)) { + continue; + } + // Every package entry carries `version`; an `authors[]`/`support` + // object that happens to have a matching `name` does not. + let Some(locked) = json_string_field(entry, "version") else { + continue; + }; + if normalize_version(locked) == normalize_version(version) { + return ComposerEntry::Found(name_idx, end); + } + mismatched = Some(locked.to_string()); + } + match mismatched { + Some(locked) => ComposerEntry::VersionMismatch(locked), + None => ComposerEntry::NotFound, + } +} + +/// Append `"shasum": ""` 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 +/// artifact unverified, so composer would install whatever the URL returned. +/// `block` is the whole dist object and already holds at least a `url`. +fn append_composer_shasum(block: &str, sha1: &str) -> String { + let Some(close) = block.rfind('}') else { + return block.to_string(); + }; + let head = block[..close].trim_end(); + let indent: String = head[head.rfind('\n').map_or(0, |i| i + 1)..] + .chars() + .take_while(|c| c.is_whitespace()) + .collect(); + format!( + "{head},\n{indent}\"shasum\": \"{sha1}\"{}", + &block[head.len()..] + ) +} + fn rewrite_composer_lock( files: &BTreeMap, overrides: &[DepOverride], @@ -2229,6 +2349,7 @@ fn rewrite_composer_lock( if composer.is_empty() || !files.contains_key("composer.lock") { return; } + const DIST_KEY: &str = "\"dist\": {"; let mut content = files["composer.lock"].clone(); let type_re = Regex::new(r#"("type": ")[^"]*(")"#).unwrap(); let url_re = Regex::new(r#"("url": ")[^"]*(")"#).unwrap(); @@ -2243,16 +2364,38 @@ fn rewrite_composer_lock( }); continue; }; - let Some(name_idx) = content.find(&format!("\"name\": \"{composer_name}\"")) else { - result.warnings.push(RewriteWarning { - code: "redirect_composer_pkg_not_found".into(), - detail: format!("no composer.lock package named {composer_name}"), - }); - continue; - }; - let Some(dist_start) = content[name_idx..] - .find("\"dist\": {") - .map(|r| name_idx + r) + let (entry_start, entry_end) = + match find_composer_entry(&content, &composer_name, &dep.version) { + ComposerEntry::Found(start, end) => (start, end), + ComposerEntry::VersionMismatch(locked) => { + result.warnings.push(RewriteWarning { + code: "redirect_composer_version_mismatch".into(), + detail: format!( + "composer.lock pins {composer_name}@{locked}, not the patched {}", + dep.version + ), + }); + continue; + } + ComposerEntry::NotFound => { + result.warnings.push(RewriteWarning { + code: "redirect_composer_pkg_not_found".into(), + detail: format!( + "no composer.lock package named {composer_name}@{}", + dep.version + ), + }); + 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) else { result.warnings.push(RewriteWarning { code: "redirect_composer_no_dist".into(), @@ -2260,20 +2403,41 @@ fn rewrite_composer_lock( }); continue; }; - let Some(dist_end) = content[dist_start..].find('}').map(|r| dist_start + r) else { + 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 block = content[dist_start..=dist_end].to_string(); - let escaped_url = dep.artifact_url.replace('/', "\\/"); + // Already redirected (either slash spelling): recording an edit whose + // `original` IS the hosted url would grow the ledger on every re-run + // and poison a future revert. + if artifact_url_present(&block, &dep.artifact_url) && block.contains(&sha1) { + continue; + } + if !block.contains("\"url\": \"") { + result.warnings.push(RewriteWarning { + code: "redirect_composer_no_dist_url".into(), + detail: format!("{composer_name}'s dist block has no url to redirect"), + }); + continue; + } let mut rewritten = type_re.replace(&block, "${1}zip${2}").to_string(); rewritten = url_re - .replace(&rewritten, format!("${{1}}{escaped_url}${{2}}").as_str()) + .replace( + &rewritten, + format!("${{1}}{}${{2}}", dep.artifact_url).as_str(), + ) .to_string(); - if rewritten.contains("\"shasum\": \"") { - rewritten = shasum_re + rewritten = if rewritten.contains("\"shasum\": \"") { + shasum_re .replace(&rewritten, format!("${{1}}{sha1}${{2}}").as_str()) - .to_string(); - } + .to_string() + } else { + append_composer_shasum(&rewritten, &sha1) + }; if rewritten != block { content = format!( "{}{}{}", @@ -6770,6 +6934,186 @@ snapshots: ); } + const COMPOSER_ARTIFACT_URL: &str = + "https://patch.socket.dev/patch/composer/acme/target/1.0.0/\ + 11111111-1111-1111-1111-111111111111/\ + 44444444-4444-4444-4444-444444444444/target-1.0.0.zip"; + const COMPOSER_SHA1: &str = "abcdef0123456789abcdef0123456789abcdef01"; + + fn composer_override(version: &str) -> DepOverride { + DepOverride { + ecosystem: "composer".into(), + name: "target".into(), + namespace: Some("acme".into()), + version: version.into(), + token: String::new(), + patch_uuid: "44444444-4444-4444-4444-444444444444".into(), + artifact_url: COMPOSER_ARTIFACT_URL.into(), + berry_zip_url: None, + registry_override: None, + integrity: Integrity { + sha1: Some(COMPOSER_SHA1.into()), + ..Default::default() + }, + } + } + + /// A composer.lock holding `acme/target` (dist shaped by `target_dist`) + /// followed by an untouchable bystander that DOES have a dist. + fn composer_lock_with(target_dist: &str) -> String { + format!( + "{{ + \"packages\": [ + {{ + \"name\": \"acme/target\", + \"version\": \"1.0.0\",{target_dist} + }}, + {{ + \"name\": \"innocent/bystander\", + \"version\": \"2.0.0\", + \"dist\": {{ + \"type\": \"zip\", + \"url\": \"https://api.github.com/repos/innocent/bystander/zipball/beef\", + \"reference\": \"beef\", + \"shasum\": \"\" + }} + }} + ], + \"packages-dev\": [] +}} +" + ) + } + + fn composer_result(lock: &str, version: &str) -> RewriteResult { + let mut files = BTreeMap::new(); + files.insert("composer.lock".to_string(), lock.to_string()); + rewrite_registry_redirect(&files, &[composer_override(version)]) + } + + /// A source-only target (composer.lock records `source`, no `dist` — a VCS + /// install) must fail closed. The rewriter used to find the package by name + /// and then scan FORWARD for the next `"dist": {` with no package boundary, + /// so it repointed the FOLLOWING package's url AND shasum at the target's + /// patch: a checksum-clean install of the wrong code. + #[test] + fn composer_source_only_target_never_touches_the_next_package() { + let lock = composer_lock_with( + " + \"source\": { + \"type\": \"git\", + \"url\": \"https://github.com/acme/target.git\", + \"reference\": \"cafe\" + }", + ); + let r = composer_result(&lock, "1.0.0"); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "no dist belongs to acme/target, so nothing may be rewritten: files={:?} edits={:?}", + r.files.keys(), + r.edits + ); + assert_eq!(warning_codes(&r), vec!["redirect_composer_no_dist"]); + } + + /// A dist block with NO `shasum` key (VCS/zipball dists omit it) must get + /// the pin inserted, not redirected unpinned: composer would otherwise + /// install whatever the hosted url returned with nothing verifying it. + #[test] + fn composer_dist_without_shasum_key_gets_the_pin_inserted() { + let lock = composer_lock_with( + " + \"dist\": { + \"type\": \"zip\", + \"url\": \"https://example.test/vcs/acme/target/zipball/cafe\", + \"reference\": \"cafe\" + }", + ); + let r = composer_result(&lock, "1.0.0"); + let out = r + .files + .get("composer.lock") + .unwrap_or_else(|| panic!("the dist must be redirected; warnings={:?}", r.warnings)); + assert!( + out.contains(&format!( + "\"reference\": \"cafe\",\n \"shasum\": \"{COMPOSER_SHA1}\"" + )), + "the sha1 must be pinned as the dist's last key, at the block's own indent: {out}" + ); + assert!( + serde_json::from_str::(out).is_ok(), + "the surgical insertion must leave valid JSON: {out}" + ); + assert!( + out.contains("zipball/beef") && !out.contains("bystander/zipball/cafe"), + "the bystander's dist must be untouched: {out}" + ); + assert!(r.warnings.is_empty(), "no warnings: {:?}", r.warnings); + + // Re-run over the pinned output: nothing left to change. + let mut again = BTreeMap::new(); + again.insert("composer.lock".to_string(), out.clone()); + let second = rewrite_registry_redirect(&again, &[composer_override("1.0.0")]); + assert!( + second.files.is_empty() && second.edits.is_empty(), + "re-run must be a no-op: files={:?} edits={:?}", + second.files.keys(), + second.edits + ); + } + + /// The locked version must match the patched one. Matching on name alone + /// repointed whichever version the lock happened to hold at a patch built + /// for a different one. + #[test] + fn composer_version_mismatch_fails_closed() { + let lock = composer_lock_with( + " + \"dist\": { + \"type\": \"zip\", + \"url\": \"https://example.test/acme/target/zipball/cafe\", + \"reference\": \"cafe\", + \"shasum\": \"\" + }", + ); + let r = composer_result(&lock, "9.9.9"); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "a lock pinning another version must not be rewritten: {:?}", + r.files.keys() + ); + assert_eq!( + warning_codes(&r), + vec!["redirect_composer_version_mismatch"] + ); + } + + /// An already-redirected lock is left alone whichever way it spells the + /// hosted url — a lock written by older composer carries `\/`-escaped + /// slashes. Re-recording an edit whose `original` IS the hosted url would + /// grow the committed ledger on every run and poison a future revert. + #[test] + fn composer_rerun_over_an_escaped_slash_redirect_is_a_noop() { + let escaped = COMPOSER_ARTIFACT_URL.replace('/', "\\/"); + let lock = composer_lock_with(&format!( + " + \"dist\": {{ + \"type\": \"zip\", + \"url\": \"{escaped}\", + \"reference\": \"cafe\", + \"shasum\": \"{COMPOSER_SHA1}\" + }}" + )); + let r = composer_result(&lock, "1.0.0"); + assert!( + r.files.is_empty() && r.edits.is_empty() && r.warnings.is_empty(), + "an already-redirected lock must be a no-op: files={:?} edits={:?} warnings={:?}", + r.files.keys(), + r.edits, + r.warnings + ); + } + /// pnpm lockfileVersion 6 embeds resolved peers in the `packages:` key /// itself, so one name@version can appear as BOTH `/pkg@1.0.0:` and /// `/pkg@1.0.0(peer@2.0.0):`. Rewriting only the plain entry is silent @@ -6828,6 +7172,26 @@ packages: ); } + /// A dist block with no `url` has nothing to redirect: pinning a shasum + /// onto it would claim a redirect that cannot happen. + #[test] + fn composer_dist_without_url_fails_closed() { + let lock = composer_lock_with( + " + \"dist\": { + \"type\": \"path\", + \"reference\": \"cafe\" + }", + ); + let r = composer_result(&lock, "1.0.0"); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "nothing may be rewritten: {:?}", + r.files.keys() + ); + assert_eq!(warning_codes(&r), vec!["redirect_composer_no_dist_url"]); + } + /// A v6 dep resolved ONLY through peer-suffixed keys previously degraded /// to a bare `entry_not_found`; the refusal must instead name the exact /// key the grammar cannot repoint so the operator knows the lock (not the diff --git a/crates/socket-patch-core/src/setup/composer/mod.rs b/crates/socket-patch-core/src/setup/composer/mod.rs index 3e2f8d69..991eefae 100644 --- a/crates/socket-patch-core/src/setup/composer/mod.rs +++ b/crates/socket-patch-core/src/setup/composer/mod.rs @@ -11,16 +11,18 @@ //! //! `composer.json` is JSON, so — like the npm `package_json` backend — edits go //! through `serde_json` (with the workspace's `preserve_order` feature, so the -//! user's key order survives) and are written back with -//! `to_string_pretty(..) + "\n"`. The contract mirrors the other backends: -//! idempotent, `dry_run`-aware, `Updated`/`AlreadyConfigured`/`Error`, and a -//! `--remove` that strips exactly what `setup` added. +//! user's key order survives) and are written back in the file's own +//! formatting (see [`serialize_like_input`]). The contract mirrors the other +//! backends: idempotent, `dry_run`-aware, `Updated`/`AlreadyConfigured`/ +//! `Error`, and a `--remove` that strips exactly what `setup` added. use std::path::{Path, PathBuf}; use serde_json::{Map, Value}; use tokio::fs; +use crate::vendor::common::{detect_indent, serialize_json}; + /// The command `setup` appends to each composer script event. The socket-patch /// CLI is invoked from `PATH` (composer has no `npx`-style fetch), offline (the /// patches are committed under `.socket/`) and silent (so it doesn't clutter @@ -113,6 +115,29 @@ fn parse_checked(content: &str) -> Result { Ok(doc) } +/// Re-serialize the edited document in the formatting the file already used. +/// +/// Composer writes `composer.json` through PHP's `JSON_PRETTY_PRINT`, which +/// indents with 4 spaces, while serde's `to_string_pretty` is hard-wired to 2 — +/// so re-serializing turned a two-key edit into a whole-file diff and left +/// `--remove` unable to restore the original bytes. `detect_indent` / +/// `serialize_json` are the same helpers the vendor backends use when they +/// rewrite composer.json and the lockfiles. A file saved without a trailing +/// newline keeps that too, since `serialize_json` always appends one. +fn serialize_like_input(doc: &Value, original: &str) -> String { + let indent = detect_indent(original); + let mut text = match serialize_json(doc, &indent) { + // Always valid UTF-8: serde_json emits escaped ASCII/UTF-8 only. + Ok(bytes) => String::from_utf8_lossy(&bytes).into_owned(), + // Serializing a `Value` cannot fail; fall back to the 2-space form. + Err(_) => serde_json::to_string_pretty(doc).unwrap_or_default() + "\n", + }; + if !original.ends_with('\n') { + text.pop(); + } + text +} + /// Append [`APPLY_COMMAND`] to both hook events, normalising each to an array. /// `None` if already present in every event (idempotent no-op). fn composer_add(content: &str) -> Result, String> { @@ -141,7 +166,7 @@ fn composer_add(content: &str) -> Result, String> { } return Ok(None); } - Ok(Some(serde_json::to_string_pretty(&doc).unwrap() + "\n")) + Ok(Some(serialize_like_input(&doc, content))) } /// Strip [`APPLY_COMMAND`] from both hook events, pruning emptied events and an @@ -167,7 +192,7 @@ fn composer_remove(content: &str) -> Result, String> { // would teleport the last root key into this slot. root.shift_remove("scripts"); } - Ok(Some(serde_json::to_string_pretty(&doc).unwrap() + "\n")) + Ok(Some(serialize_like_input(&doc, content))) } /// Add [`APPLY_COMMAND`] to one event, normalising string → array. Returns @@ -257,12 +282,17 @@ async fn edit( None => Ok(false), Some(new) => { if !dry_run { - // The crate-wide atomic writer (stage+fsync+rename): the - // user's committed composer.json must never be left torn - // by a crash mid-write. - crate::utils::fs::atomic_write_bytes(composer_json, new.as_bytes()) - .await - .map_err(|e| e.to_string())?; + // Atomic (stage+fsync+rename), so the user's committed + // composer.json is never left torn by a crash mid-write, + // and mode-preserving, because the rename swaps in a fresh + // inode that would otherwise take umask defaults — the + // same writer every sibling manifest editor uses. + crate::utils::fs::atomic_write_bytes_preserving_mode( + composer_json, + new.as_bytes(), + ) + .await + .map_err(|e| e.to_string())?; } Ok(true) } @@ -382,6 +412,70 @@ mod tests { assert_eq!(removed, BASIC, "add→remove restores the original bytes"); } + /// Composer writes `composer.json` with PHP's `JSON_PRETTY_PRINT`, i.e. + /// 4-space indent — the shape virtually every real-world manifest has. + const COMPOSER_AUTHORED: &str = + "{\n \"name\": \"acme/app\",\n \"require\": {\n \"php\": \">=8.1\"\n }\n}\n"; + + #[test] + fn test_add_preserves_composer_four_space_indent() { + // Regression: re-serializing at serde's fixed 2-space indent reformatted + // a composer-authored manifest top to bottom, turning a one-key edit + // into a whole-file diff. + let out = composer_add(COMPOSER_AUTHORED).unwrap().unwrap(); + assert!( + out.contains("\n \"name\": \"acme/app\","), + "depth-1 indent not preserved:\n{out}" + ); + assert!( + out.contains("\n \"php\": \">=8.1\""), + "depth-2 indent not preserved:\n{out}" + ); + assert!( + out.contains("\n \"scripts\": {"), + "our own added key must use the file's indent:\n{out}" + ); + assert!(is_hook_present(&out)); + } + + #[test] + fn test_round_trip_restores_composer_authored_manifest() { + // The whole point of preserving the indent: `--remove` must give the + // user back the exact bytes composer wrote, not a reformatted file. + let added = composer_add(COMPOSER_AUTHORED).unwrap().unwrap(); + let removed = composer_remove(&added).unwrap().unwrap(); + assert_eq!( + removed, COMPOSER_AUTHORED, + "add→remove must restore composer's own formatting" + ); + } + + #[test] + fn test_round_trip_preserves_tab_indent() { + let inp = + "{\n\t\"name\": \"acme/app\",\n\t\"require\": {\n\t\t\"php\": \">=8.1\"\n\t}\n}\n"; + let added = composer_add(inp).unwrap().unwrap(); + assert!( + added.contains("\n\t\"scripts\": {"), + "tab indent not preserved:\n{added}" + ); + assert_eq!(composer_remove(&added).unwrap().unwrap(), inp); + } + + #[test] + fn test_round_trip_preserves_absent_trailing_newline() { + // `serialize_json` always appends a trailing newline, so a manifest + // saved without one would gain a phantom last-line diff that `--remove` + // could never take back. + let inp = COMPOSER_AUTHORED.trim_end_matches('\n'); + let added = composer_add(inp).unwrap().unwrap(); + assert!( + !added.ends_with('\n'), + "must not add a trailing newline the file did not have:\n{added:?}" + ); + assert_eq!(composer_remove(&added).unwrap().unwrap(), inp); + } + #[test] fn test_user_string_event_already_ours_is_noop() { // An event whose string value is exactly our command counts as present. @@ -606,11 +700,6 @@ mod tests { #[cfg(unix)] #[tokio::test] - #[ignore = "RED: `edit()` uses the plain `atomic_write_bytes`, so the stage \ - inode is created with umask defaults and the rename resets the \ - user's composer.json mode (0o744 -> 0o644). Every sibling manifest \ - editor uses `atomic_write_bytes_preserving_mode`; switching this \ - call over is the one-line fix, which was not part of this change."] async fn test_edit_preserves_manifest_permissions() { use std::os::unix::fs::PermissionsExt; // Regression: `composer.json` is a file the *user* owns and we merely diff --git a/crates/socket-patch-core/src/vendor/composer_lock.rs b/crates/socket-patch-core/src/vendor/composer_lock.rs index 0f7b393f..969e5444 100644 --- a/crates/socket-patch-core/src/vendor/composer_lock.rs +++ b/crates/socket-patch-core/src/vendor/composer_lock.rs @@ -27,6 +27,7 @@ //! (`JSON_PRETTY_PRINT`) + trailing newline; serde_json does not escape `/` //! (matching `JSON_UNESCAPED_SLASHES`). +use std::collections::HashSet; use std::path::Path; use serde_json::{json, Map, Value}; @@ -357,6 +358,15 @@ pub async fn vendor_composer( /// update`, a hand edit, or a newer vendor run — is left alone with a /// `vendor_lock_entry_drifted` warning. /// +/// Refused fail-closed when composer.lock still wires a package to our uuid +/// dir that NO wiring record can restore (a `repair`-reconstructed entry +/// carries no pre-vendor fragment): the registry `dist` the surgery replaced +/// exists nowhere else — not in the lock (we overwrote it), not in the +/// artifact — so an un-rewrite is impossible offline, and deleting the +/// artifacts anyway would leave the lock pointing at a gone path (`composer +/// install` then dies with "Source path … is not found"). Nothing is deleted +/// in that case; the error names the re-resolve escape hatch. +/// /// Note: the *installed* `vendor//` keeps the patched bytes until the /// next `composer install` re-mirrors from the registry; revert surfaces that /// as the `vendor_installed_copy_stale` advisory. @@ -378,6 +388,24 @@ pub async fn revert_composer( let lock_path = project_root.join(COMPOSER_LOCK); let mut warnings = Vec::new(); + // Nothing may be deleted while composer.lock still consumes it. Checked + // BEFORE the restore loop (and before any write) so the answer is the + // same for `--dry-run` and a wet run. + let stranded = stranded_wired_packages(&lock_path, &entry.uuid, &restorable_keys(entry)).await; + if !stranded.is_empty() { + let listed = stranded.join(", "); + let args = stranded.join(" "); + return RevertOutcome::failed(format!( + "refusing revert: composer.lock still points {listed} at {uuid_dir_rel}, but the \ + ledger entry records no pre-vendor lock fragment to restore (an entry \ + reconstructed by `socket-patch repair` recovers the artifact, never the \ + registry dist the surgery replaced). The vendored artifacts were LEFT IN \ + PLACE so the project still installs. To undo the vendoring, re-resolve the \ + package from the registry first (`composer update --no-install {args}`), then \ + re-run `socket-patch vendor --revert`" + )); + } + // Wiring is restored in reverse application order (one record today). for w in entry.wiring.iter().rev() { if w.kind != WIRING_KIND { @@ -679,6 +707,69 @@ fn composer_json_bytes(value: &Value) -> std::io::Result> { serialize_json(value, " ") } +/// The `
:` keys this entry can actually put back: +/// a recognized wiring kind, a well-formed key, and a recorded `original`. +fn restorable_keys(entry: &VendorEntry) -> HashSet { + entry + .wiring + .iter() + .filter(|w| w.kind == WIRING_KIND && w.original.is_some()) + .filter_map(|w| w.key.as_deref()) + .filter_map(|k| k.split_once(':')) + .filter(|(section, _)| *section == "packages" || *section == "packages-dev") + .map(|(section, pkg)| format!("{section}:{}", pkg.to_lowercase())) + .collect() +} + +/// Lock packages still wired to `uuid` that `restorable` cannot un-rewrite — +/// the set that a delete-the-artifacts revert would strand. Names are +/// returned lowercase and deduped (composer package names are canonically +/// lowercase; the lock's own casing is display-only). +/// +/// A missing or unparseable composer.lock yields none: no install can be +/// consuming a lock nothing can read, and the restore loop already degrades +/// to `vendor_lock_entry_drifted` for it. +async fn stranded_wired_packages( + lock_path: &Path, + uuid: &str, + restorable: &HashSet, +) -> Vec { + let Ok(text) = tokio::fs::read_to_string(lock_path).await else { + return Vec::new(); + }; + let Ok(lock) = serde_json::from_str::(&text) else { + return Vec::new(); + }; + let mut out: Vec = Vec::new(); + for section in ["packages", "packages-dev"] { + let Some(arr) = lock.get(section).and_then(Value::as_array) else { + continue; + }; + for e in arr { + let wired_to_us = e + .get("dist") + .and_then(|d| d.get("url")) + .and_then(Value::as_str) + .and_then(parse_vendor_path) + .is_some_and(|p| p.eco == "composer" && p.uuid == uuid); + if !wired_to_us { + continue; + } + let Some(name) = e.get("name").and_then(Value::as_str) else { + continue; + }; + let name = name.to_lowercase(); + // Section-qualified: `restore_lock_entry` only searches the + // section the wiring recorded, so an entry that moved between + // packages[] and packages-dev[] is unrestorable too. + if !restorable.contains(&format!("{section}:{name}")) && !out.contains(&name) { + out.push(name); + } + } + } + out +} + /// Restore one `composer_lock_package` wiring record. `Ok(true)` = restored /// (or would be, on dry run); `Ok(false)` = drifted, left alone; `Err` = a /// real I/O / serialization failure. diff --git a/crates/socket-patch-core/src/vendor/lock_inventory.rs b/crates/socket-patch-core/src/vendor/lock_inventory.rs index 914d9fee..8843ff13 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory.rs @@ -22,6 +22,7 @@ use std::path::Path; use serde_json::Value; +use crate::crawlers::composer_crawler::normalize_version; use crate::crawlers::python_crawler::canonicalize_pypi_name; use crate::patch::path_safety; use crate::utils::purl::{percent_decode_purl_component, strip_purl_qualifiers}; @@ -637,7 +638,8 @@ async fn inventory_bun(root: &Path) -> Option> { /// Inventory `composer.lock` `packages`/`packages-dev`. The `dist.shasum` /// (sha1 of the dist zip) is frequently empty — such entries stay /// discovery-only. Names lowercase to the canonical packagist form; -/// versions drop the pretty leading `v`. +/// versions drop the pretty leading `v`/`V` through the crawler's +/// [`normalize_version`], so installed and lockfile rows agree. async fn inventory_composer_lock(project_root: &Path) -> Option> { let bytes = tokio::fs::read(project_root.join("composer.lock")) .await @@ -656,11 +658,12 @@ async fn inventory_composer_lock(project_root: &Path) -> Option/composer/installed.json` with an +/// `install-path` relative to that `composer/` directory, and the package +/// itself at `//`. `config_vendor_dir` is written +/// into composer.json's `config` block when supplied. +async fn stage_relocated_project( + root: &Path, + vendor_rel: &str, + config_vendor_dir: Option<&str>, +) -> std::path::PathBuf { + let vendor = root.join(vendor_rel); + tokio::fs::create_dir_all(vendor.join("monolog").join("monolog")) + .await + .unwrap(); + let composer_dir = vendor.join("composer"); + tokio::fs::create_dir_all(&composer_dir).await.unwrap(); + tokio::fs::write( + composer_dir.join("installed.json"), + br#"{"packages":[{"name":"monolog/monolog","version":"3.5.0","install-path":"../monolog/monolog"}]}"#, + ) + .await + .unwrap(); + + let manifest = match config_vendor_dir { + Some(dir) => format!(r#"{{"config":{{"vendor-dir":"{dir}"}}}}"#), + None => "{}".to_string(), + }; + tokio::fs::write(root.join("composer.json"), manifest) + .await + .unwrap(); + vendor +} + +/// Composer relocates the ENTIRE vendor tree — `composer/installed.json` +/// included — when composer.json sets `config.vendor-dir`, so assuming +/// `/vendor` finds nothing: scan then reports every installed package +/// as lockfile-only ("not yet installed") and apply resolves each one as +/// `package_not_found`. Verified against composer 2.10.2: `"vendor-dir": +/// "lib/deps"` puts installed.json at `lib/deps/composer/installed.json`. +#[tokio::test] +#[serial_test::parallel] +async fn config_vendor_dir_relocates_discovery() { + let tmp = tempfile::tempdir().unwrap(); + // Nested (`lib/deps`), which composer allows and which also proves the + // project root is still found for the install-path boundary check. + let vendor = stage_relocated_project(tmp.path(), "lib/deps", Some("lib/deps")).await; + // No `vendor/` anywhere: the only discoverable tree is the relocated one. + assert!(!tmp.path().join("vendor").exists()); + + let crawler = ComposerCrawler; + let paths = crawler + .get_vendor_paths(&options_at(tmp.path())) + .await + .unwrap(); + assert_eq!(paths, vec![vendor.clone()]); + + let packages = crawler.crawl_all(&options_at(tmp.path())).await; + assert_eq!( + packages.len(), + 1, + "relocated vendor tree must be crawled; got {packages:?}" + ); + assert_eq!(packages[0].purl, ORG_PURL); + assert_eq!(packages[0].path, vendor.join("monolog").join("monolog")); +} + +/// Composer accepts `./`-prefixed vendor-dir values (`./lib/deps`); refusing +/// the `.` segment outright regressed such projects to zero discovery, worse +/// than the old hardcoded `vendor/`. The normalizer reduces the value before +/// the safety gate. +#[tokio::test] +#[serial_test::parallel] +async fn config_vendor_dir_dot_prefix_is_normalized() { + let tmp = tempfile::tempdir().unwrap(); + let vendor = stage_relocated_project(tmp.path(), "lib/deps", Some("./lib/deps")).await; + assert!(!tmp.path().join("vendor").exists()); + + let crawler = ComposerCrawler; + let paths = crawler + .get_vendor_paths(&options_at(tmp.path())) + .await + .unwrap(); + assert_eq!(paths, vec![vendor.clone()]); + + let packages = crawler.crawl_all(&options_at(tmp.path())).await; + assert_eq!( + packages.len(), + 1, + "./-prefixed vendor-dir must be crawled; got {packages:?}" + ); + assert_eq!(packages[0].purl, ORG_PURL); +} + +/// A trailing separator is legal in `config.vendor-dir` (Composer rtrims it +/// before use), so `"vendor-dir": "lib/deps/"` must resolve the same as +/// `"lib/deps"` — a naive join would produce an empty final segment and +/// fail the coordinate gate. +#[tokio::test] +#[serial_test::parallel] +async fn config_vendor_dir_trailing_slash_is_trimmed() { + let tmp = tempfile::tempdir().unwrap(); + let vendor = stage_relocated_project(tmp.path(), "lib/deps", Some("lib/deps/")).await; + + let crawler = ComposerCrawler; + let paths = crawler + .get_vendor_paths(&options_at(tmp.path())) + .await + .unwrap(); + assert_eq!(paths, vec![vendor]); +} + +/// `COMPOSER_VENDOR_DIR` outranks composer.json's `config.vendor-dir` in +/// Composer's own `Config::get`, so it must outrank it here too. Verified +/// against composer 2.10.2: `COMPOSER_VENDOR_DIR=third_party composer +/// install` writes `third_party/composer/installed.json`. +#[tokio::test] +#[serial_test::serial] +async fn composer_vendor_dir_env_outranks_config_and_default() { + let tmp = tempfile::tempdir().unwrap(); + // composer.json points at `lib/deps` and a decoy `vendor/` tree exists; + // the env var names a third directory, which must win over both. + let vendor = stage_relocated_project(tmp.path(), "third_party", Some("lib/deps")).await; + let decoy = tmp.path().join("vendor").join("composer"); + tokio::fs::create_dir_all(&decoy).await.unwrap(); + tokio::fs::write(decoy.join("installed.json"), b"{\"packages\":[]}") + .await + .unwrap(); + + let prev = std::env::var("COMPOSER_VENDOR_DIR").ok(); + std::env::set_var("COMPOSER_VENDOR_DIR", "third_party"); + + let crawler = ComposerCrawler; + let paths = crawler + .get_vendor_paths(&options_at(tmp.path())) + .await + .unwrap(); + let packages = crawler.crawl_all(&options_at(tmp.path())).await; + + match prev { + Some(v) => std::env::set_var("COMPOSER_VENDOR_DIR", v), + None => std::env::remove_var("COMPOSER_VENDOR_DIR"), + } + + assert_eq!(paths, vec![vendor.clone()], "env var must win"); + assert_eq!(packages.len(), 1, "got {packages:?}"); + assert_eq!(packages[0].path, vendor.join("monolog").join("monolog")); +} + +/// A set-but-empty `COMPOSER_VENDOR_DIR` counts as unset (twin of the +/// MAVEN_REPO_LOCAL / NUGET_PACKAGES rules): honoring `""` would resolve +/// the vendor tree to the project root itself, so `vendor/composer/` would +/// be looked for at `/composer/`. +#[tokio::test] +#[serial_test::serial] +async fn empty_composer_vendor_dir_env_falls_back_to_default() { + let tmp = tempfile::tempdir().unwrap(); + let vendor = stage_relocated_project(tmp.path(), "vendor", None).await; + + let prev = std::env::var("COMPOSER_VENDOR_DIR").ok(); + std::env::set_var("COMPOSER_VENDOR_DIR", ""); + + let crawler = ComposerCrawler; + let paths = crawler + .get_vendor_paths(&options_at(tmp.path())) + .await + .unwrap(); + + match prev { + Some(v) => std::env::set_var("COMPOSER_VENDOR_DIR", v), + None => std::env::remove_var("COMPOSER_VENDOR_DIR"), + } + + assert_eq!(paths, vec![vendor], "empty env var must not shadow vendor/"); +} + +/// composer.json belongs to the project being SCANNED and the vendor +/// directory it names is where apply later WRITES patch content, so a +/// `config.vendor-dir` that escapes the project is refused outright — and +/// refused fail-closed, NOT downgraded to `vendor/`, which would patch an +/// unrelated tree that Composer never installed into. +#[tokio::test] +#[serial_test::parallel] +async fn config_vendor_dir_escaping_project_is_refused() { + let outer = tempfile::tempdir().unwrap(); + let root = outer.path().join("proj"); + tokio::fs::create_dir_all(&root).await.unwrap(); + // A fully staged vendor tree OUTSIDE the project, so the refusal is the + // coordinate gate and not a missing directory. + stage_relocated_project(outer.path(), "escaped", None).await; + tokio::fs::write( + root.join("composer.json"), + br#"{"config":{"vendor-dir":"../escaped"}}"#, + ) + .await + .unwrap(); + // A conventional vendor/ tree also exists: the refusal must not silently + // fall back to it either. + let decoy = root.join("vendor").join("composer"); + tokio::fs::create_dir_all(&decoy).await.unwrap(); + tokio::fs::write( + decoy.join("installed.json"), + br#"{"packages":[{"name":"monolog/monolog","version":"3.5.0"}]}"#, + ) + .await + .unwrap(); + tokio::fs::create_dir_all(root.join("vendor").join("monolog").join("monolog")) + .await + .unwrap(); + + let crawler = ComposerCrawler; + let paths = crawler.get_vendor_paths(&options_at(&root)).await.unwrap(); + assert!( + paths.is_empty(), + "escaping config.vendor-dir must fail closed; got {paths:?}" + ); + let packages = crawler.crawl_all(&options_at(&root)).await; + assert!( + packages.is_empty(), + "escaping config.vendor-dir must not fall back to vendor/; got {packages:?}" + ); +} + +/// An ABSOLUTE `config.vendor-dir` is legal in Composer but refused here +/// for the same reason: it names an apply write target and composer.json is +/// tamperable. Discovery reports nothing, exactly as it did before custom +/// vendor directories were understood at all — no silent redirect. +#[tokio::test] +#[serial_test::parallel] +async fn absolute_config_vendor_dir_is_refused() { + let outer = tempfile::tempdir().unwrap(); + let root = outer.path().join("proj"); + tokio::fs::create_dir_all(&root).await.unwrap(); + let absolute = stage_relocated_project(outer.path(), "abs-vendor", None).await; + tokio::fs::write( + root.join("composer.json"), + // Backslashes JSON-escaped so the manifest stays parseable on + // Windows — an unparseable manifest would fall back to `vendor/` + // and pass this refusal test vacuously. + format!( + r#"{{"config":{{"vendor-dir":"{}"}}}}"#, + absolute.display().to_string().replace('\\', "\\\\") + ), + ) + .await + .unwrap(); + + let crawler = ComposerCrawler; + let paths = crawler.get_vendor_paths(&options_at(&root)).await.unwrap(); + assert!(paths.is_empty(), "got {paths:?}"); +} + +// ── installed.json install-path ──────────────────────────────── + +const PLUGIN_PURL: &str = "pkg:composer/socket/probe-plugin@1.2.3"; +const INSTALLERS_PURL: &str = "pkg:composer/composer/installers@2.3.0"; + +/// composer/installers (`type: wordpress-plugin`, `extra.installer-paths`) +/// installs packages OUTSIDE `vendor//`, and installed.json's +/// `install-path` is the only record of where they landed. Reconstructing +/// the conventional layout makes them invisible to scan and unpatchable by +/// apply, even though the metadata says exactly where they are. +/// +/// The fixture mirrors composer 2.10.2 byte-for-byte: with +/// `"web/app/plugins/{$name}/"` mapped to `type:wordpress-plugin`, it wrote +/// `"install-path": "../../../web/app/plugins/probe-plugin"` (three levels +/// up from `lib/deps/composer/`) — and, for `composer/installers` itself, +/// the `./`-relative `"./installers"`. +#[tokio::test] +#[serial_test::parallel] +async fn install_path_resolves_package_outside_vendor() { + let tmp = tempfile::tempdir().unwrap(); + let vendor = tmp.path().join("vendor"); + let composer_dir = vendor.join("composer"); + tokio::fs::create_dir_all(&composer_dir).await.unwrap(); + tokio::fs::write( + composer_dir.join("installed.json"), + br#"{"packages":[ + {"name":"composer/installers","version":"v2.3.0","install-path":"./installers"}, + {"name":"socket/probe-plugin","version":"1.2.3","install-path":"../../web/app/plugins/probe-plugin"} + ]}"#, + ) + .await + .unwrap(); + tokio::fs::write(tmp.path().join("composer.json"), b"{}") + .await + .unwrap(); + + let plugin_dir = tmp + .path() + .join("web") + .join("app") + .join("plugins") + .join("probe-plugin"); + tokio::fs::create_dir_all(&plugin_dir).await.unwrap(); + let installers_dir = composer_dir.join("installers"); + tokio::fs::create_dir_all(&installers_dir).await.unwrap(); + // Control: NEITHER package sits at the conventional location, so a + // reconstructed `vendor//` finds nothing to corroborate. + assert!(!vendor.join("socket").join("probe-plugin").exists()); + + let crawler = ComposerCrawler; + let packages = crawler.crawl_all(&options_at(tmp.path())).await; + assert_eq!(packages.len(), 2, "got {packages:?}"); + let plugin = packages.iter().find(|p| p.purl == PLUGIN_PURL).unwrap(); + assert_eq!(plugin.path, plugin_dir); + // `./installers` (a CurDir component) resolves inside vendor/composer/. + let installers = packages.iter().find(|p| p.purl == INSTALLERS_PURL).unwrap(); + assert_eq!(installers.path, installers_dir); + + // apply resolves through find_by_purls and must agree with the crawl — + // otherwise the patch button offers a package apply can't locate. + let found = crawler + .find_by_purls(&vendor, &[PLUGIN_PURL.to_string()]) + .await + .unwrap(); + assert_eq!(found.len(), 1, "got {found:?}"); + assert_eq!(found.get(PLUGIN_PURL).unwrap().path, plugin_dir); +} + +/// installed.json is untrusted, tamperable input and the directory it names +/// is a patch WRITE target, so an `install-path` that leaves the project +/// must be dropped — by both the scan path and apply's resolver. The +/// boundary is the project, not the vendor root: a legitimate +/// composer/installers target lives outside `vendor/` (see the test above), +/// so `..` alone cannot be the signal. +#[tokio::test] +#[serial_test::parallel] +async fn install_path_escaping_project_root_is_rejected() { + let outer = tempfile::tempdir().unwrap(); + let root = outer.path().join("proj"); + let vendor = root.join("vendor"); + let composer_dir = vendor.join("composer"); + tokio::fs::create_dir_all(&composer_dir).await.unwrap(); + tokio::fs::write(root.join("composer.json"), b"{}") + .await + .unwrap(); + + // Both escape targets EXIST on disk, so the on-disk corroboration alone + // does not stop them; only the containment gate does. + let outside = outer.path().join("evil").join("pkg"); + tokio::fs::create_dir_all(&outside).await.unwrap(); + tokio::fs::create_dir_all(vendor.join("monolog").join("monolog")) + .await + .unwrap(); + tokio::fs::write( + composer_dir.join("installed.json"), + format!( + r#"{{"packages":[ + {{"name":"monolog/monolog","version":"3.5.0","install-path":"../monolog/monolog"}}, + {{"name":"relative/evil","version":"1.0.0","install-path":"../../../evil/pkg"}}, + {{"name":"absolute/evil","version":"1.0.0","install-path":"{}"}} + ]}}"#, + // JSON-escape Windows path separators: raw backslashes made the + // whole installed.json unparseable there, so even the in-project + // package vanished and the test failed for the wrong reason. + outside.display().to_string().replace('\\', "\\\\") + ), + ) + .await + .unwrap(); + + let crawler = ComposerCrawler; + let packages = crawler.crawl_all(&options_at(&root)).await; + assert_eq!( + packages.len(), + 1, + "only the in-project package may survive; got {:?}", + packages.iter().map(|p| &p.path).collect::>() + ); + assert_eq!(packages[0].purl, ORG_PURL); + + let found = crawler + .find_by_purls( + &vendor, + &[ + "pkg:composer/relative/evil@1.0.0".to_string(), + "pkg:composer/absolute/evil@1.0.0".to_string(), + ], + ) + .await + .unwrap(); + assert!( + found.is_empty(), + "install-path escaped the project root: {:?}", + found.values().map(|p| &p.path).collect::>() + ); +} + +/// A rejected `install-path` must NOT fall back to `vendor//`: +/// installed.json says the package lives elsewhere, so patching whatever +/// happens to sit at the conventional path would edit the wrong tree. +#[tokio::test] +#[serial_test::parallel] +async fn rejected_install_path_does_not_fall_back_to_conventional_dir() { + let outer = tempfile::tempdir().unwrap(); + let root = outer.path().join("proj"); + let vendor = root.join("vendor"); + let composer_dir = vendor.join("composer"); + tokio::fs::create_dir_all(&composer_dir).await.unwrap(); + tokio::fs::write(root.join("composer.json"), b"{}") + .await + .unwrap(); + // The conventional directory exists and would otherwise be accepted. + tokio::fs::create_dir_all(vendor.join("monolog").join("monolog")) + .await + .unwrap(); + tokio::fs::create_dir_all(outer.path().join("elsewhere")) + .await + .unwrap(); + tokio::fs::write( + composer_dir.join("installed.json"), + br#"{"packages":[{"name":"monolog/monolog","version":"3.5.0","install-path":"../../../elsewhere"}]}"#, + ) + .await + .unwrap(); + + let crawler = ComposerCrawler; + assert!( + crawler.crawl_all(&options_at(&root)).await.is_empty(), + "a rejected install-path must not resolve to vendor/monolog/monolog" + ); + assert!(crawler + .find_by_purls(&vendor, &[ORG_PURL.to_string()]) + .await + .unwrap() + .is_empty()); +} + +// ── version normalization parity with the lockfile inventory ──── + +/// `V1.2.3` is a legal Composer tag. The crawler strips `v` AND `V` from +/// installed.json versions, so if the lockfile inventory strips only the +/// lowercase form the same package yields TWO different PURLs — one +/// installed `@1.2.3` row plus a phantom lockfile-only `@V1.2.3` row, both +/// POSTed to the API and both shown to the user. Pin the two normalizations +/// to the same output. +#[tokio::test] +#[serial_test::parallel] +async fn uppercase_v_version_normalizes_identically_in_crawl_and_lock_inventory() { + let tmp = tempfile::tempdir().unwrap(); + let vendor = tmp.path().join("vendor"); + let composer_dir = vendor.join("composer"); + tokio::fs::create_dir_all(&composer_dir).await.unwrap(); + tokio::fs::create_dir_all(vendor.join("monolog").join("monolog")) + .await + .unwrap(); + tokio::fs::write( + composer_dir.join("installed.json"), + br#"{"packages":[{"name":"monolog/monolog","version":"V3.5.0","install-path":"../monolog/monolog"}]}"#, + ) + .await + .unwrap(); + tokio::fs::write(tmp.path().join("composer.json"), b"{}") + .await + .unwrap(); + tokio::fs::write( + tmp.path().join("composer.lock"), + br#"{"packages":[{"name":"monolog/monolog","version":"V3.5.0","dist":{"type":"zip","url":"https://example.com/m.zip","shasum":""}}]}"#, + ) + .await + .unwrap(); + + let crawled = ComposerCrawler.crawl_all(&options_at(tmp.path())).await; + assert_eq!(crawled.len(), 1, "got {crawled:?}"); + assert_eq!(crawled[0].purl, ORG_PURL); + + let inventoried = + socket_patch_core::vendor::lock_inventory::inventory_project(tmp.path()).await; + let composer_rows: Vec<_> = inventoried + .iter() + .filter(|e| e.ecosystem == "composer") + .collect(); + assert_eq!(composer_rows.len(), 1, "got {composer_rows:?}"); + assert_eq!( + composer_rows[0].purl, crawled[0].purl, + "lockfile and installed rows must normalize to ONE purl, else the \ + package double-counts as installed + lockfile-only" + ); +} diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/expected-edits.json b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/expected-edits.json index f8ef3714..538817b4 100644 --- a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/expected-edits.json +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/expected-edits.json @@ -4,7 +4,7 @@ "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 }", - "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 }" + "original": "\"dist\": {\n \"type\": \"zip\",\n \"url\": \"https://api.github.com/repos/Seldaek/monolog/zipball/abc123\",\n \"reference\": \"abc123def456\",\n \"shasum\": \"\"\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 }" } ] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/expected/composer.lock b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/expected/composer.lock index 79d7686c..8937ef5e 100644 --- a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/expected/composer.lock +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/expected/composer.lock @@ -9,10 +9,20 @@ "version": "2.0.0", "dist": { "type": "zip", - "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", + "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", "reference": "abc123def456", "shasum": "abcdef0123456789abcdef0123456789abcdef01" } + }, + { + "name": "psr/log", + "version": "1.1.4", + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/log/zipball/d49695b909c3b7628b6289db5479a1c204601f11", + "reference": "d49695b909c3b7628b6289db5479a1c204601f11", + "shasum": "" + } } ], "packages-dev": [] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/input/composer.lock b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/input/composer.lock index 24fb942d..333e00f4 100644 --- a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/input/composer.lock +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/basic/input/composer.lock @@ -9,10 +9,20 @@ "version": "2.0.0", "dist": { "type": "zip", - "url": "https:\/\/api.github.com\/repos\/Seldaek\/monolog\/zipball\/abc123", + "url": "https://api.github.com/repos/Seldaek/monolog/zipball/abc123", "reference": "abc123def456", "shasum": "" } + }, + { + "name": "psr/log", + "version": "1.1.4", + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/log/zipball/d49695b909c3b7628b6289db5479a1c204601f11", + "reference": "d49695b909c3b7628b6289db5479a1c204601f11", + "shasum": "" + } } ], "packages-dev": [] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/expected-edits.json b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/expected-edits.json new file mode 100644 index 00000000..3b0043c4 --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/expected-edits.json @@ -0,0 +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 }", + "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 }" + } +] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/expected/composer.lock b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/expected/composer.lock new file mode 100644 index 00000000..cf8e4363 --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/expected/composer.lock @@ -0,0 +1,29 @@ +{ + "_readme": [ + "This file locks the dependencies of your project to a known state" + ], + "content-hash": "abc123def456abc123def456abc1", + "packages": [ + { + "name": "monolog/monolog", + "version": "2.0.0", + "dist": { + "type": "zip", + "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", + "reference": "abc123def456", + "shasum": "abcdef0123456789abcdef0123456789abcdef01" + } + }, + { + "name": "psr/log", + "version": "1.1.4", + "dist": { + "type": "zip", + "url": "https:\/\/api.github.com\/repos\/php-fig\/log\/zipball\/d49695b909c3b7628b6289db5479a1c204601f11", + "reference": "d49695b909c3b7628b6289db5479a1c204601f11", + "shasum": "" + } + } + ], + "packages-dev": [] +} diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/input/composer.lock b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/input/composer.lock new file mode 100644 index 00000000..2669e33b --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/input/composer.lock @@ -0,0 +1,29 @@ +{ + "_readme": [ + "This file locks the dependencies of your project to a known state" + ], + "content-hash": "abc123def456abc123def456abc1", + "packages": [ + { + "name": "monolog/monolog", + "version": "2.0.0", + "dist": { + "type": "zip", + "url": "https:\/\/api.github.com\/repos\/Seldaek\/monolog\/zipball\/abc123", + "reference": "abc123def456", + "shasum": "" + } + }, + { + "name": "psr/log", + "version": "1.1.4", + "dist": { + "type": "zip", + "url": "https:\/\/api.github.com\/repos\/php-fig\/log\/zipball\/d49695b909c3b7628b6289db5479a1c204601f11", + "reference": "d49695b909c3b7628b6289db5479a1c204601f11", + "shasum": "" + } + } + ], + "packages-dev": [] +} diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/overrides.json b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/overrides.json new file mode 100644 index 00000000..a83719b2 --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/escaped-slash-lock/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" + } + } +] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/expected-edits.json b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/expected-edits.json new file mode 100644 index 00000000..538817b4 --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/expected-edits.json @@ -0,0 +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 }", + "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 }" + } +] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/expected/composer.lock b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/expected/composer.lock new file mode 100644 index 00000000..4b6479d3 --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/expected/composer.lock @@ -0,0 +1,19 @@ +{ + "_readme": [ + "This file locks the dependencies of your project to a known state" + ], + "content-hash": "abc123def456abc123def456abc1", + "packages": [ + { + "name": "Monolog/MonoLog", + "version": "v2.0.0", + "dist": { + "type": "zip", + "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", + "reference": "abc123def456", + "shasum": "abcdef0123456789abcdef0123456789abcdef01" + } + } + ], + "packages-dev": [] +} diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/input/composer.lock b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/input/composer.lock new file mode 100644 index 00000000..d87b6340 --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/input/composer.lock @@ -0,0 +1,19 @@ +{ + "_readme": [ + "This file locks the dependencies of your project to a known state" + ], + "content-hash": "abc123def456abc123def456abc1", + "packages": [ + { + "name": "Monolog/MonoLog", + "version": "v2.0.0", + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/Seldaek/monolog/zipball/abc123", + "reference": "abc123def456", + "shasum": "" + } + } + ], + "packages-dev": [] +} diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/overrides.json b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/overrides.json new file mode 100644 index 00000000..a83719b2 --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/mixed-case-v-prefix/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" + } + } +] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/expected-edits.json b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/expected-edits.json new file mode 100644 index 00000000..adce35c5 --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/expected-edits.json @@ -0,0 +1,10 @@ +[ + { + "path": "composer.lock", + "kind": "redirect_composer_dist", + "action": "rewritten", + "key": "monolog/monolog", + "original": "\"dist\": {\n \"type\": \"zip\",\n \"url\": \"https://example.test/vcs/monolog/monolog/zipball/abc123\",\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 }" + } +] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/expected/composer.lock b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/expected/composer.lock new file mode 100644 index 00000000..8937ef5e --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/expected/composer.lock @@ -0,0 +1,29 @@ +{ + "_readme": [ + "This file locks the dependencies of your project to a known state" + ], + "content-hash": "abc123def456abc123def456abc1", + "packages": [ + { + "name": "monolog/monolog", + "version": "2.0.0", + "dist": { + "type": "zip", + "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", + "reference": "abc123def456", + "shasum": "abcdef0123456789abcdef0123456789abcdef01" + } + }, + { + "name": "psr/log", + "version": "1.1.4", + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/log/zipball/d49695b909c3b7628b6289db5479a1c204601f11", + "reference": "d49695b909c3b7628b6289db5479a1c204601f11", + "shasum": "" + } + } + ], + "packages-dev": [] +} diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/input/composer.lock b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/input/composer.lock new file mode 100644 index 00000000..6513111e --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/input/composer.lock @@ -0,0 +1,28 @@ +{ + "_readme": [ + "This file locks the dependencies of your project to a known state" + ], + "content-hash": "abc123def456abc123def456abc1", + "packages": [ + { + "name": "monolog/monolog", + "version": "2.0.0", + "dist": { + "type": "zip", + "url": "https://example.test/vcs/monolog/monolog/zipball/abc123", + "reference": "abc123def456" + } + }, + { + "name": "psr/log", + "version": "1.1.4", + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/log/zipball/d49695b909c3b7628b6289db5479a1c204601f11", + "reference": "d49695b909c3b7628b6289db5479a1c204601f11", + "shasum": "" + } + } + ], + "packages-dev": [] +} diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/overrides.json b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/overrides.json new file mode 100644 index 00000000..a83719b2 --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/no-shasum-key/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" + } + } +] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-bystander/expected-edits.json b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-bystander/expected-edits.json new file mode 100644 index 00000000..fe51488c --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-bystander/expected-edits.json @@ -0,0 +1 @@ +[] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-bystander/input/composer.lock b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-bystander/input/composer.lock new file mode 100644 index 00000000..f708f9fb --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-bystander/input/composer.lock @@ -0,0 +1,28 @@ +{ + "_readme": [ + "This file locks the dependencies of your project to a known state" + ], + "content-hash": "abc123def456abc123def456abc1", + "packages": [ + { + "name": "monolog/monolog", + "version": "2.0.0", + "source": { + "type": "git", + "url": "https://github.com/Seldaek/monolog.git", + "reference": "abc123def456" + } + }, + { + "name": "psr/log", + "version": "1.1.4", + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/log/zipball/d49695b909c3b7628b6289db5479a1c204601f11", + "reference": "d49695b909c3b7628b6289db5479a1c204601f11", + "shasum": "" + } + } + ], + "packages-dev": [] +} diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-bystander/overrides.json b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-bystander/overrides.json new file mode 100644 index 00000000..a83719b2 --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/source-only-bystander/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" + } + } +] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/version-mismatch/expected-edits.json b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/version-mismatch/expected-edits.json new file mode 100644 index 00000000..fe51488c --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/version-mismatch/expected-edits.json @@ -0,0 +1 @@ +[] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/version-mismatch/input/composer.lock b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/version-mismatch/input/composer.lock new file mode 100644 index 00000000..782a450b --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/version-mismatch/input/composer.lock @@ -0,0 +1,19 @@ +{ + "_readme": [ + "This file locks the dependencies of your project to a known state" + ], + "content-hash": "abc123def456abc123def456abc1", + "packages": [ + { + "name": "monolog/monolog", + "version": "1.9.0", + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/Seldaek/monolog/zipball/999999", + "reference": "999999aaaaaa", + "shasum": "" + } + } + ], + "packages-dev": [] +} diff --git a/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/version-mismatch/overrides.json b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/version-mismatch/overrides.json new file mode 100644 index 00000000..a83719b2 --- /dev/null +++ b/crates/socket-patch-core/tests/fixtures/redirect/composer/composer-lock/version-mismatch/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" + } + } +] diff --git a/tests/setup_matrix/matrix.json b/tests/setup_matrix/matrix.json index c5b4c308..f7ddeae3 100644 --- a/tests/setup_matrix/matrix.json +++ b/tests/setup_matrix/matrix.json @@ -179,7 +179,7 @@ { "ecosystem": "composer", "pm": "composer", "image": "composer", "hook_family": "composer-event", - "baseline_supported": false, + "baseline_supported": true, "package": "monolog/monolog", "version": "3.5.0", "purl": "pkg:composer/monolog/monolog@3.5.0", "manifest_key": "package/src/Monolog/Logger.php", "apply_ecosystems": "composer" }, diff --git a/tests/setup_matrix/run-case.sh b/tests/setup_matrix/run-case.sh index c399b675..15a34991 100755 --- a/tests/setup_matrix/run-case.sh +++ b/tests/setup_matrix/run-case.sh @@ -306,7 +306,23 @@ EOF ;; go) printf 'module sm-proj\n\ngo 1.21\n' > go.mod ;; - mvn|composer|dotnet) : ;; + composer) + # `setup` wires its hook into composer.json's script events, so the + # manifest must exist BEFORE setup runs — without it setup reports + # `no_files`, the hook is never written, and `composer require` below + # can't re-apply the patch. The dependency is left to `composer require` + # (same division of labour as npm/yarn/bun above). 4-space indent is + # what composer itself writes, so a reformat by setup would show up as + # a diff here too. + cat > composer.json <