diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 0ce0d94a9..0bbbc5acb 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1510,6 +1510,7 @@ command's integration tests check its output. | `redirect_yarn_classic_dep_manifest_rewritten` | `warnings[]` (warning) | scan/get `--mode hosted` (yarn classic): the served tarball's own `package.json` declares other dependencies than the pinned block's sub-maps, every descriptor already locked; the block's `dependencies:` / `optionalDependencies:` sub-maps are rewritten to match, in the shape yarn 1 writes for that manifest (#591, #1558). | | `vendor_artifact_gitignored` | `failed` | vendor (vlt, the npm-family tarball flavors — npm, pnpm, bun, yarn classic, yarn berry — NuGet, every PyPI flavor, and the JVM trees of Maven, Gradle, sbt and scala-cli): inside a git work tree, `git check-ignore --no-index` reports the new artifact's uuid directory (JVM: its tree root, such as `.socket/vendor/maven2`) as ignored by a rule its own `.gitignore` cannot override (such as a root `.socket/` or `vendor/` rule; the detail names the rule). Remedy: drop that rule for `.socket/vendor/`. Refused before any write, dry run included. A file rule such as `*.tgz`, `*.nupkg`, `*.whl` or `*.jar` is overridden by the `!*` `.gitignore` vendoring writes into the uuid dir or tree root (PyPI's uuid dir also gets the `.gitattributes` that unsets a `*.whl filter=lfs` rule); if a written tarball, directory payload, nupkg or wheel still reads as ignored, the run refuses and removes the uuid dir it created. | | `vendor_artifact_gitignore_unchecked` | warning | vendor (vlt, the npm-family tarball flavors, NuGet and every PyPI flavor): git is installed but could not answer the ignore check for the written vendored directory (it failed to start, ran past 30 s, or `rev-parse` / `check-ignore` exited with an error); the package is vendored and the detail names what failed. Remedy: make sure no ignore rule covers `.socket/` before committing. Git absent, or a project outside any work tree, raises nothing. | +| `vendor_ledger_gitignored` | top-level error (exit 1) | vendor / scan / get `--mode vendored` (every backend) and the hosted→vendored eject: inside a git work tree, `git check-ignore` reports the vendor ledger `.socket/vendor/state.json` as ignored by a file rule (such as `state.json`, `*.json` or `**/state.json`; the message names it), so the commit the run asks for would leave it out and no other checkout could `rollback`, `list` or `vendor --revert` the patches (#1638). A ledger git already tracks is never refused. Refused before any write, dry run included. Remedy: drop the rule or add `!.socket/vendor/state.json`. A rule that ignores `.socket/` or `.socket/vendor/` as a directory is `vendor_artifact_gitignored`: per package from the backends that check it themselves (the npm family, NuGet, PyPI, the JVM trees), and as this same run-level refusal when the run includes any other ecosystem. | | `vendor_ledger_entry_missing` | `failed` | vendor (vlt): the only installed copy is vlt's link to a committed vendored directory, but the vendor ledger has no entry for the package; restore `.socket/vendor/state.json` from version control (v5.0: `repair` no longer re-synthesizes it). Replaces the `package_not_installed` skip. | | `vendor_variant_ambiguous` | `failed` | vendor / scan / get `--mode vendored` (pypi, gem): the package is not installed and the manifest holds several release variants of it (`?artifact_id=` / `?platform=`), none of which the vendor ledger records, so nothing says which distribution to vendor; install the package or keep one release variant. A variant the ledger records (at any patch uuid) is taken as the wired one and its siblings are left out without an event. | | `vendor_artifact_missing` | reason | The recorded artifact is missing; repair requires an online exact redownload. | @@ -1700,7 +1701,7 @@ One command is **intentionally not** plain-envelope and will stay that way (not } ``` -**Events per mode.** Agent mode (`get`, `scan --mode agent` / `--sync`): a fetched patch is `downloaded` (new manifest record) or `updated` (+`oldUuid`: it replaced another uuid), one already recorded at the same uuid is `skipped` / `already_in_manifest` (it is still re-applied), a patch that could not be fetched or recorded is `failed` (`download_failed`, `patch_no_applicable_files`, `blob_write_failed`); then the nested apply adds `applied` for each recorded patch it patched (or found patched) and `failed` (apply's `apply_failed` / `package_not_installed` with its `error`, #424) for each it failed — a failing manifest patch the run did not select (the nested apply covers the whole `--ecosystems`-scoped manifest) gets its own `failed` event, and a failure no single patch explains (an unreadable manifest, the yarn PnP refusal, unavailable patch sources) is a purl-less `failed` event, so a failed apply is always `partialFailure` (exit 1). The apply's mismatch overwrites (#1004) are `content_mismatch_overwritten` warnings; a record whose purl the vendor ledger wires at another uuid adds a `vendored_uuid_drift` warning. Vendor-owned and lockfile-only selections are `skipped` / `vendored` and `skipped` / `package_not_installed` before any download. Hosted and vendored events are described under "Scan modes" above; every one carries `details.mode`. `get`'s narrowing adds `skipped` events (`package_not_installed`, `yarn_pnp_unsupported`, `pnpm_pnp_unsupported`) and `paidRequired` runs one `skipped` / `paid_required` event per patch found. +**Events per mode.** Agent mode (`get`, `scan --mode agent` / `--sync`): a fetched patch is `downloaded` (new manifest record) or `updated` (+`oldUuid`: it replaced another uuid), one already recorded at the same uuid is `skipped` / `already_in_manifest` (it is still re-applied), a patch that could not be fetched or recorded is `failed` (`download_failed`, `patch_no_applicable_files`, `blob_write_failed`); then the nested apply adds `applied` for each recorded patch it patched (or found patched) and `failed` (apply's `apply_failed` / `package_not_installed` with its `error`, #424) for each it failed — a failing manifest patch the run did not select (the nested apply covers the whole `--ecosystems`-scoped manifest) gets its own `failed` event, and a failure no single patch explains (an unreadable manifest, the yarn PnP refusal, unavailable patch sources) is a purl-less `failed` event, so a failed apply is always `partialFailure` (exit 1). The apply's mismatch overwrites (#1004) are `content_mismatch_overwritten` warnings; a record whose purl the vendor ledger wires at another uuid adds a `vendored_uuid_drift` warning. A run that leaves patches recorded in a manifest git would not commit (inside a git work tree, `git check-ignore` matches the manifest's project-relative path, e.g. a `manifest.json`, `*.json` or `.socket/` rule; a tracked manifest never warns) adds a `manifest_gitignored` warning naming the rule: a fresh checkout's `apply` would find no manifest and apply nothing (#1638). It stays a warning (exit unchanged) because a pipeline that scans and applies in one job never commits `.socket/`. Vendor-owned and lockfile-only selections are `skipped` / `vendored` and `skipped` / `package_not_installed` before any download. Hosted and vendored events are described under "Scan modes" above; every one carries `details.mode`. `get`'s narrowing adds `skipped` events (`package_not_installed`, `yarn_pnp_unsupported`, `pnpm_pnp_unsupported`) and `paidRequired` runs one `skipped` / `paid_required` event per patch found. **Patch metadata.** `downloaded` / `updated` events (agent and vendored) carry the patch metadata in `details`, the same keys for every command: diff --git a/crates/socket-patch-cli/src/commands/agent_download.rs b/crates/socket-patch-cli/src/commands/agent_download.rs index e23ef9658..06731252f 100644 --- a/crates/socket-patch-cli/src/commands/agent_download.rs +++ b/crates/socket-patch-cli/src/commands/agent_download.rs @@ -1320,6 +1320,45 @@ pub(crate) async fn download_patch_records_preflighted( } } +/// Warning code: git would not commit the manifest (see +/// [`warn_if_manifest_gitignored`]). +pub(crate) const MANIFEST_GITIGNORED: &str = "manifest_gitignored"; + +/// Warn (stderr `[warn]` + `warnings[]`) when git's ignore rules keep the +/// manifest this run recorded patches in out of the commit (#1638): the +/// blobs are committed, but a fresh checkout's `socket-patch apply` finds +/// no manifest, applies nothing and exits 0. Only a warning, because a +/// pipeline that scans and applies in one job never commits `.socket/`. +/// A manifest outside the project root is not checked. +pub(crate) async fn warn_if_manifest_gitignored( + project_root: &Path, + manifest_path: &Path, + quiet: bool, + warnings: &mut Vec, +) { + let Ok(rel) = manifest_path.strip_prefix(project_root) else { + return; + }; + let rel: Vec = rel + .components() + .map(|c| c.as_os_str().to_string_lossy().into_owned()) + .collect(); + let rel = rel.join("/"); + let Some(rules) = socket_patch_core::vendor::file_gitignored(project_root, &rel).await else { + return; + }; + let w = format!( + "git would not commit {rel} ({rules}), so `socket-patch apply` in other checkouts \ + (CI, a fresh clone) would find no manifest and apply nothing; remove or narrow that \ + rule (a `!{rel}` line re-includes the file only when no parent directory is \ + ignored)" + ); + if !quiet { + eprintln!(" [warn] {w}"); + } + warnings.push(RunWarning::new(MANIFEST_GITIGNORED, w)); +} + /// Warning code: the manifest now records a patch uuid the vendor ledger /// does not wire for that purl (see [`warn_on_vendored_uuid_drift`]). pub(crate) const VENDORED_UUID_DRIFT: &str = "vendored_uuid_drift"; @@ -1712,6 +1751,10 @@ pub async fn download_and_apply_patches_into( // download.) The nested apply below skips the vendored purl either way. let mut warnings = release_warnings(batch.warnings); warn_on_vendored_uuid_drift(¶ms.project_root(), quiet, &changed, &mut warnings).await; + if !recorded.is_empty() { + warn_if_manifest_gitignored(¶ms.project_root(), &manifest_path, quiet, &mut warnings) + .await; + } env.warnings.extend(warnings); if !quiet { diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index e84e63eed..b214cc0e8 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -31,8 +31,9 @@ use crate::commands::agent_download::{ decide_patch_action, download_patch_records_preflighted, download_patch_records_reusing, filter_to_installed_releases, max_vuln_severity, nested_apply_args, patch_event_metadata, record_apply_outcome, report_lock_failure, run_nested_apply, run_outcome, tag_mode, - unwind_new_blobs, warn_on_vendored_uuid_drift, write_all_patch_blobs, DetachedDownload, - PatchAction, VendorRefusals, ALREADY_IN_MANIFEST, BLOB_WRITE_FAILED, RELEASE_NARROWING, + unwind_new_blobs, warn_if_manifest_gitignored, warn_on_vendored_uuid_drift, + write_all_patch_blobs, DetachedDownload, PatchAction, VendorRefusals, ALREADY_IN_MANIFEST, + BLOB_WRITE_FAILED, RELEASE_NARROWING, }; pub use crate::commands::agent_download::{ download_and_apply_patches_into, download_and_apply_patches_with, DownloadParams, DownloadRun, @@ -2059,6 +2060,13 @@ async fn save_and_apply_patch( warn_on_vendored_uuid_drift(&args.common.project_root(), quiet, &recorded, &mut warnings) .await; } + warn_if_manifest_gitignored( + &args.common.project_root(), + &manifest_path, + quiet, + &mut warnings, + ) + .await; // Progress narration goes to stderr, like the search path's. if !quiet { diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 3516e7c49..9816d33fb 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1397,6 +1397,19 @@ async fn run_check(args: &VendorArgs) -> i32 { i32::from(env.summary.failed != 0) } +/// Whether every ecosystem in a vendored run has a backend that refuses a +/// `.socket/vendor/` directory ignore rule per package +/// (`vendor_artifact_gitignored`): the npm family (vlt included), NuGet, +/// PyPI and the JVM trees. The others rely on the run-level ledger check. +fn artifacts_gitignore_checked(mut ecosystems: impl Iterator) -> bool { + ecosystems.all(|eco| { + matches!( + eco, + Ecosystem::Npm | Ecosystem::Nuget | Ecosystem::Pypi | Ecosystem::Maven + ) + }) +} + /// A refused eject: the JSON error envelope (`status: error`) or an /// `Error:` line (printed even under `--silent`). Exit 1; nothing touched. fn emit_eject_refusal(common: &GlobalArgs, code: &str, message: &str) -> i32 { @@ -1728,6 +1741,15 @@ impl EjectSnapshot { /// the project to upstream, not to hosted). async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { let common = &args.common; + // A ledger git would not commit refuses before anything is fetched or + // restored, so the project stays hosted (#1638). + let purls: Vec = pins.iter().map(|pin| pin.purl.clone()).collect(); + let artifacts_checked = artifacts_gitignore_checked(partition_purls(&purls, None).into_keys()); + if let Some((code, detail)) = + vendor::ledger_gitignore_refusal(&common.cwd, artifacts_checked).await + { + return emit_eject_refusal(common, code, &detail); + } let (client, use_public_proxy) = get_api_client_with_overrides(common.api_client_overrides()).await; let telemetry = TelemetryAuth::for_client(&client); @@ -2761,6 +2783,24 @@ pub(crate) async fn vendor_records_reusing( return true; } }; + // The ledger must survive the commit the run asks for, or no other + // checkout can roll the patches back (#1638): refuse before anything + // is written, dry run included. + let artifacts_checked = artifacts_gitignore_checked( + vendorable_partition + .iter() + .filter(|(_, purls)| !purls.is_empty()) + .map(|(eco, _)| *eco), + ); + if let Some((code, detail)) = + vendor::ledger_gitignore_refusal(&common.cwd, artifacts_checked).await + { + if !common.json { + eprintln!("Error: {detail}"); + } + env.mark_error(EnvelopeError::new(code, detail)); + return true; + } // Pre-stage trees a previous run left behind (it crashed or was // interrupted between staging an archive and settling): nothing of // this run is staged yet, and the caller holds the apply lock, so every diff --git a/crates/socket-patch-cli/tests/get/get_failure_paths.rs b/crates/socket-patch-cli/tests/get/get_failure_paths.rs index 29b8e5b0d..3df8ac2ce 100644 --- a/crates/socket-patch-cli/tests/get/get_failure_paths.rs +++ b/crates/socket-patch-cli/tests/get/get_failure_paths.rs @@ -1699,3 +1699,121 @@ async fn get_uuid_json_mismatch_overwrite_is_reported() { assert!(!stderr.contains("did not match"), "stderr={stderr}"); assert_eq!(std::fs::read(&installed).unwrap(), AFTER_BYTES); } + +/// `git init` `root` with `gitignore`; `false` when git is not installed. +fn git_project_ignoring(root: &std::path::Path, gitignore: &str) -> bool { + if socket_patch_core::utils::process::resolve_tool("git").is_none() { + return false; + } + assert!(std::process::Command::new("git") + .arg("-C") + .arg(root) + .args(["init", "-q"]) + .status() + .unwrap() + .success()); + std::fs::write(root.join(".gitignore"), gitignore).unwrap(); + true +} + +/// Assert the envelope's `manifest_gitignored` warning names `rule`, or +/// that there is none. +fn assert_manifest_warning(json: &serde_json::Value, rule: Option<&str>, what: &str) { + let warning = json["warnings"] + .as_array() + .into_iter() + .flatten() + .find(|w| w["code"] == "manifest_gitignored"); + match rule { + Some(rule) => { + let detail = warning.unwrap_or_else(|| panic!("{what}: json={json}"))["detail"] + .as_str() + .unwrap(); + assert!( + detail.contains(".socket/manifest.json") && detail.contains(rule), + "{detail}" + ); + } + None => assert!(warning.is_none(), "{what}: json={json}"), + } +} + +/// An agent-mode run that records a patch in a manifest git would not +/// commit (`manifest.json`, `*.json` + `!package.json`, or a `.socket/` +/// directory rule) warns `manifest_gitignored`, naming the rule: otherwise +/// the blobs are committed without the manifest and a fresh checkout's +/// `apply` silently applies nothing (#1638). A re-included manifest gets +/// no warning. +#[tokio::test] +#[serial] +async fn agent_record_warns_when_git_ignores_the_manifest() { + let server = MockServer::start().await; + mount_view_files(&server, UUID, PURL, good_files()).await; + let selected = vec![search_result(UUID, PURL)]; + for (gitignore, rule) in [ + ("manifest.json\n", Some("manifest.json")), + ("*.json\n!package.json\n", Some("*.json")), + (".socket/\n", Some(".socket/")), + ("*.json\n!package.json\n!.socket/manifest.json\n", None), + ] { + let tmp = tempfile::tempdir().unwrap(); + if !git_project_ignoring(tmp.path(), gitignore) { + return; + } + let (code, json) = + download_and_apply_patches(&selected, &engine_params(tmp.path()), &server.uri()).await; + assert_eq!(code, 0, "{gitignore:?}: json={json}"); + assert!(tmp.path().join(".socket/manifest.json").is_file()); + assert_manifest_warning(&json, rule, gitignore); + } +} + +/// A manifest git already tracks keeps being committed whatever the ignore +/// rules say, so a re-run after a `*.json` rule lands does not warn. +#[tokio::test] +#[serial] +async fn agent_record_does_not_warn_for_a_tracked_manifest() { + let server = MockServer::start().await; + mount_view_files(&server, UUID, PURL, good_files()).await; + let selected = vec![search_result(UUID, PURL)]; + let tmp = tempfile::tempdir().unwrap(); + if !git_project_ignoring(tmp.path(), "") { + return; + } + let (code, json) = + download_and_apply_patches(&selected, &engine_params(tmp.path()), &server.uri()).await; + assert_eq!(code, 0, "json={json}"); + assert!(std::process::Command::new("git") + .arg("-C") + .arg(tmp.path()) + .args(["add", ".socket/manifest.json"]) + .status() + .unwrap() + .success()); + std::fs::write(tmp.path().join(".gitignore"), "*.json\n").unwrap(); + let (code, json) = + download_and_apply_patches(&selected, &engine_params(tmp.path()), &server.uri()).await; + assert_eq!(code, 0, "json={json}"); + assert_manifest_warning(&json, None, "tracked manifest"); +} + +/// `get ` records through its own save path, which warns the same +/// way. +#[tokio::test] +async fn get_uuid_warns_when_git_ignores_the_manifest() { + let server = MockServer::start().await; + mount_view_files(&server, UUID, PURL, good_files()).await; + let tmp = tempfile::tempdir().unwrap(); + if !git_project_ignoring(tmp.path(), "manifest.json\n") { + return; + } + let (code, stdout, stderr) = + run_get_bin(tmp.path(), &server.uri(), &[UUID, "--save-only", "--json"]); + assert_eq!(code, 0, "stdout={stdout}\nstderr={stderr}"); + assert!(tmp.path().join(".socket/manifest.json").is_file()); + assert_manifest_warning( + &parse_json_envelope(&stdout), + Some("manifest.json"), + "get ", + ); +} diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index bd3377f85..a1bf5e3c2 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -457,6 +457,95 @@ async fn dry_run_writes_nothing() { assert_eq!(std::fs::read(fx.installed_index()).unwrap(), ORIG_INDEX); } +/// `git init` `root` and write its `.gitignore`. `false` when git is not +/// installed (the probe has nothing to ask, so the test has nothing to +/// check). +fn git_project_ignoring(root: &Path, gitignore: &str) -> bool { + if socket_patch_core::utils::process::resolve_tool("git").is_none() { + return false; + } + assert!(std::process::Command::new("git") + .arg("-C") + .arg(root) + .args(["init", "-q"]) + .status() + .unwrap() + .success()); + std::fs::write(root.join(".gitignore"), gitignore).unwrap(); + true +} + +/// A `.gitignore` file rule that matches the vendor ledger (`state.json`, +/// `*.json`) would leave it out of the commit the run asks for, so every +/// other checkout has the lock wiring but no ledger and `rollback` fails +/// with `vendor_ledger_missing` (#1638). The run refuses before writing +/// anything, dry run included. +#[tokio::test] +async fn a_gitignored_ledger_refuses_before_any_write() { + for (gitignore, rule) in [ + ("node_modules/\nstate.json\n", "state.json"), + ("node_modules/\n*.json\n!package*.json\n", "*.json"), + ("node_modules/\n**/state.json\n", "**/state.json"), + ] { + for dry_run in [false, true] { + let fx = npm_fixture(); + if !git_project_ignoring(fx.root(), gitignore) { + return; + } + let extra: &[&str] = if dry_run { &["--dry-run"] } else { &[] }; + let (code, env) = vendor_cli(fx.root(), extra); + assert_eq!(code, 1, "{gitignore:?} dry_run={dry_run}: {env:#}"); + assert_eq!(env["error"]["code"], "vendor_ledger_gitignored", "{env:#}"); + let message = env["error"]["message"].as_str().unwrap(); + assert!( + message.contains(".socket/vendor/state.json") && message.contains(rule), + "the message names the ledger and the rule: {message}" + ); + assert!( + !fx.vendor_dir().exists(), + "nothing is vendored for {gitignore:?}" + ); + assert_eq!(fx.lock_bytes(), fx.original_lock, "lock byte-identical"); + } + } +} + +/// A rule that re-includes the ledger lets the run vendor as usual. +#[tokio::test] +async fn a_re_included_ledger_vendors() { + let fx = npm_fixture(); + if !git_project_ignoring( + fx.root(), + "node_modules/\n*.json\n!package*.json\n!.socket/vendor/state.json\n", + ) { + return; + } + assert_eq!(vendor_run(vendor_args(fx.root())).await, 0); + assert!(fx.state_path().is_file()); + assert!(fx.tgz_path().is_file()); +} + +/// A ledger git already tracks keeps being committed whatever the ignore +/// rules say, so a re-run after a `*.json` rule lands is not refused. +#[tokio::test] +async fn a_tracked_ledger_is_not_refused() { + let fx = npm_fixture(); + if !git_project_ignoring(fx.root(), "node_modules/\n") { + return; + } + assert_eq!(vendor_run(vendor_args(fx.root())).await, 0); + assert!(std::process::Command::new("git") + .arg("-C") + .arg(fx.root()) + .args(["add", ".socket/vendor/state.json"]) + .status() + .unwrap() + .success()); + std::fs::write(fx.root().join(".gitignore"), "node_modules/\n*.json\n").unwrap(); + let (code, env) = vendor_cli(fx.root(), &[]); + assert_eq!(code, 0, "{env:#}"); +} + // ───────────────────────────────────────────────────────────────────── // 4. vendor → revert round-trip // ───────────────────────────────────────────────────────────────────── @@ -3507,6 +3596,39 @@ fn run_scan_vendor(root: &Path, mock_uri: &str, extra: &[&str]) -> (i32, Value) (code, env) } +/// The ledger check sits in front of every vendored backend, not only the +/// npm family the report used (#1638): a gem `scan --mode vendored` whose +/// `.gitignore` matches `state.json` refuses `vendor_ledger_gitignored` +/// before anything is written. The gem backend has no per-package check of +/// its own for a `.socket/` directory rule, so that refuses here too, as +/// `vendor_artifact_gitignored`. +#[tokio::test] +async fn scan_vendor_gem_refuses_a_gitignored_ledger() { + for (gitignore, want) in [ + ("vendor/bundle/\nstate.json\n", "vendor_ledger_gitignored"), + ("vendor/bundle/\n.socket/\n", "vendor_artifact_gitignored"), + ] { + let mock = wiremock::MockServer::start().await; + mount_gem_patch_api(&mock, GEM_PURL).await; + let fx = gem_fixture(); + if !git_project_ignoring(fx.root(), gitignore) { + return; + } + let (code, env) = run_scan_vendor(fx.root(), &mock.uri(), &[]); + assert_eq!(code, 1, "{gitignore:?}: {env:#}"); + assert_eq!(env["error"]["code"], want, "{env:#}"); + assert!(!fx.root().join(".socket/vendor").exists(), "{env:#}"); + assert_eq!( + std::fs::read_to_string(fx.root().join("Gemfile.lock")).unwrap(), + GEM_LOCK + ); + assert_eq!( + std::fs::read_to_string(fx.root().join("Gemfile")).unwrap(), + GEM_GEMFILE + ); + } +} + /// `scan --mode vendored` end to end on a gem project: discover → download → /// vendor lands the gem pair edit (Gemfile pin + `path:`, lock PATH section /// + `(= …)!` DEPENDENCIES pin) and the patched artifact dir, keyed in the @@ -4618,6 +4740,52 @@ snapshots: assert!(!root.join(".npmrc").exists()); } + /// Hosted → vendored in a git project whose `.gitignore` matches the + /// vendor ledger (#1638): both the eject (no manifest) and the + /// manifest-driven takeover refuse `vendor_ledger_gitignored` before + /// restoring anything, so the project stays hosted. + #[tokio::test] + #[serial] + async fn hosted_then_vendor_with_a_gitignored_ledger_keeps_the_hosted_wiring() { + if socket_patch_core::utils::process::resolve_tool("git").is_none() { + return; + } + for with_manifest in [false, true] { + let server = MockServer::start().await; + mock_hosted_api(&server).await; + let registry = mock_registry(&server).await; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write_package_lock_project(root); + assert!(super::git_project_ignoring( + root, + "node_modules/\nstate.json\n" + )); + assert_eq!(scan_run(hosted_args(root, server.uri())).await, 0); + let hosted_lock = std::fs::read(root.join("package-lock.json")).unwrap(); + let hosted_npmrc = std::fs::read(root.join(".npmrc")).unwrap(); + assert!(String::from_utf8_lossy(&hosted_lock).contains(HOSTED_URL)); + + if with_manifest { + seed_manifest_and_blob(root); + } + let (code, env) = vendor_online_cli(root, ®istry, PATCH_ORIGIN, &[]); + assert_eq!(code, 1, "with_manifest={with_manifest}: {env:#}"); + assert_eq!(env["error"]["code"], "vendor_ledger_gitignored", "{env:#}"); + assert_eq!( + std::fs::read(root.join("package-lock.json")).unwrap(), + hosted_lock, + "the hosted lock must be untouched" + ); + assert_eq!( + std::fs::read(root.join(".npmrc")).unwrap(), + hosted_npmrc, + "the hosted .npmrc must be untouched" + ); + assert!(!root.join(".socket/vendor").exists(), "{env:#}"); + } + } + /// Hosted → vendored in a git project that ignores `.socket/` (#831): /// hosted mode writes nothing there, so the rule is common. The /// gitignore refusal comes BEFORE the takeover restores the upstream diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index ba29bb238..7287c9ff8 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -124,8 +124,9 @@ pub use source::PackageSource; pub(crate) use npm_common::is_safe_npm_name; pub use pypi_requirements::requirements_include_names; pub use state::{ - carry_forward_wiring, load_state, lookup_entry, lookup_entry_kv, purl_keys_cover, save_state, - save_state_shared, VendorEntry, VendorState, VENDOR_STATE_REL, + carry_forward_wiring, file_gitignored, ledger_gitignore_refusal, load_state, lookup_entry, + lookup_entry_kv, purl_keys_cover, save_state, save_state_shared, VendorEntry, VendorState, + LEDGER_GITIGNORED, VENDOR_STATE_REL, }; pub use verify::{ artifact_is_file_shaped, check_vendored_artifact, compute_dir_inventory, diff --git a/crates/socket-patch-core/src/vendor/npm_dir.rs b/crates/socket-patch-core/src/vendor/npm_dir.rs index b7a627d1e..dfeb7d688 100644 --- a/crates/socket-patch-core/src/vendor/npm_dir.rs +++ b/crates/socket-patch-core/src/vendor/npm_dir.rs @@ -462,6 +462,23 @@ pub(crate) async fn gitignore_probe( project_root: &Path, paths: &[String], ) -> Result, String> { + Ok(ignoring_rule_lines(project_root, paths, true) + .await? + .map(|lines| summarize_rules(&lines))) +} + +/// The ignoring rules `git check-ignore -v` reports for `paths`, one +/// `::\t` line each (see +/// [`ignoring_rules`]), or `Ok(None)` when none matches, git is absent or +/// the root is not a work tree. `no_index` asks about the paths as if +/// nothing were tracked (the right question for files this run creates); +/// without it a tracked file is never ignored, because git keeps +/// committing changes to it whatever the ignore rules say. +pub(crate) async fn ignoring_rule_lines( + project_root: &Path, + paths: &[String], + no_index: bool, +) -> Result>, String> { let Some(git) = crate::utils::process::resolve_tool("git") else { return Ok(None); }; @@ -480,27 +497,29 @@ pub(crate) async fn gitignore_probe( }; } let input: String = paths.iter().map(|p| format!("{p}\0")).collect(); - let (code, out) = git_output( - &git, - project_root, - &["check-ignore", "-v", "-z", "--no-index", "--stdin"], - Some(input), - ) - .await - .ok_or("`git check-ignore` did not run to completion")?; + let mut check = vec!["check-ignore", "-v", "-z", "--stdin"]; + if no_index { + check.push("--no-index"); + } + let (code, out) = git_output(&git, project_root, &check, Some(input)) + .await + .ok_or("`git check-ignore` did not run to completion")?; if code != 0 && code != 1 { return Err(format!("`git check-ignore` exited {code}")); } let lines = ignoring_rules(&out); - Ok((code == 0 && !lines.is_empty()).then(|| { - let shown: Vec<&str> = lines.iter().take(3).map(String::as_str).collect(); - let more = lines.len().saturating_sub(shown.len()); - let mut detail = shown.join("; "); - if more > 0 { - detail.push_str(&format!("; and {more} more")); - } - detail - })) + Ok((code == 0 && !lines.is_empty()).then_some(lines)) +} + +/// Up to three of `lines` joined for a message, with a count of the rest. +pub(crate) fn summarize_rules(lines: &[String]) -> String { + let shown: Vec<&str> = lines.iter().take(3).map(String::as_str).collect(); + let more = lines.len().saturating_sub(shown.len()); + let mut detail = shown.join("; "); + if more > 0 { + detail.push_str(&format!("; and {more} more")); + } + detail } /// The non-negated matches of `git check-ignore -v -z` output diff --git a/crates/socket-patch-core/src/vendor/state.rs b/crates/socket-patch-core/src/vendor/state.rs index 44b3237fa..0c13d0a3d 100644 --- a/crates/socket-patch-core/src/vendor/state.rs +++ b/crates/socket-patch-core/src/vendor/state.rs @@ -1003,6 +1003,74 @@ pub(crate) async fn write_marker_or_warn( } } +/// The vendor ledger is ignored by git, so the commit the vendored workflow +/// ends with would leave it out (#1638). +pub const LEDGER_GITIGNORED: &str = "vendor_ledger_gitignored"; + +/// Refuse a vendored run whose state git would not commit (#1638): every +/// other checkout would get the lock wiring and the artifacts but no +/// ledger, so `rollback`, `list` and `vendor --revert` there could not see +/// the patches. One `git check-ignore` over `.socket/vendor/` and the +/// ledger, honouring the index (a ledger already tracked keeps being +/// committed): +/// +/// - a rule that ignores `.socket/vendor/` as a directory (`.socket/`, +/// `vendor/`) is `vendor_artifact_gitignored`, unless +/// `artifacts_checked`: every backend in the run refuses that rule per +/// package itself (the npm family, NuGet, PyPI, the JVM trees), naming the +/// package; +/// - a file rule that matches only the ledger (`state.json`, `*.json`) is +/// `vendor_ledger_gitignored`. +/// +/// `None` when git would commit both, git is absent, the root is not a work +/// tree, or git cannot answer (the fail-open of +/// [`super::npm_dir::gitignored`]). +pub async fn ledger_gitignore_refusal( + project_root: &Path, + artifacts_checked: bool, +) -> Option<(&'static str, String)> { + let dir = format!("{VENDOR_DIR}/"); + let paths = [dir.clone(), VENDOR_STATE_REL.to_string()]; + let lines = super::npm_dir::ignoring_rule_lines(project_root, &paths, false) + .await + .ok()??; + let dir_lines: Vec = lines + .iter() + .filter(|line| line.ends_with(&format!("\t{dir}"))) + .cloned() + .collect(); + if !dir_lines.is_empty() { + return (!artifacts_checked).then(|| { + ( + super::npm_dir::GITIGNORED, + super::npm_dir::gitignored_detail( + VENDOR_DIR, + &super::npm_dir::summarize_rules(&dir_lines), + ), + ) + }); + } + Some(( + LEDGER_GITIGNORED, + format!( + "git would not commit the vendor ledger {VENDOR_STATE_REL} ({}), so other \ + checkouts could not roll back or list the vendored patches; remove that rule \ + or add `!{VENDOR_STATE_REL}` to .gitignore, then vendor again", + super::npm_dir::summarize_rules(&lines) + ), + )) +} + +/// The rules (summarized for a message) that would keep git from +/// committing the project-relative file `rel`. A tracked file is never +/// ignored. `None` also when git is absent or cannot answer. +pub async fn file_gitignored(project_root: &Path, rel: &str) -> Option { + super::npm_dir::ignoring_rule_lines(project_root, &[rel.to_string()], false) + .await + .ok()? + .map(|lines| super::npm_dir::summarize_rules(&lines)) +} + #[cfg(test)] mod tests { use super::*;