diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a0e6718..919bbd19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1183,8 +1183,10 @@ into the new version's section — see docs/releasing.md. a server-side archive build and count against quota, are exactly the one-at-a-time loop's (71 on a fresh depscan run, where an earlier draft of the look-ahead issued 74). What changes is only their timing: - up to four are in flight at once. `SOCKET_API_CONCURRENCY=1` turns the - look-ahead off entirely. + they are requested in one batch at the first planned package (see + "Fewer downloads in vendored runs"), and up to four archives are in + flight at once. `SOCKET_API_CONCURRENCY=1` turns the look-ahead off + entirely. - A token revoked *mid-run* now costs the authenticated batch endpoint the requests already in flight — up to the in-flight cap instead of one — before the run downgrades to the public proxy. Their answers are @@ -1194,6 +1196,21 @@ into the new version's section — see docs/releasing.md. ### Fixed +- **`apply` no longer half-applies a patch that creates a file from a + diff-only cache.** A diff archive has no delta for a file the patch + creates, but the source check counted a cached diff archive as covering + the whole patch: `apply --offline` passed it, patched the modified files, + then failed on the created file's missing blob; online `apply` never + fetched that blob. Coverage is now per file (a diff covers only files + with a `beforeHash`), so `apply --offline` reports the patch as having no + local source up front and changes nothing, online `apply` fetches just + the created files' blobs, and a default (diff-mode) `repair` downloads + them too. Such a repair's `--json` envelope carries a second + `downloaded` (dry-run `verified`) artifact event with `mode: "file"` for + those blobs. +- **Hosted `scan` resolves more than 500 patches.** The package-reference + request is sent in chunks of 500 uuids, the endpoint's limit; a larger + scan used to fail with a 400. - **`rollback` fetches a before-blob that only a store peer variant needs.** The before-blob gate now probes every pnpm and vlt store variant copy the rollback restores, so an online rollback no longer fails @@ -1817,6 +1834,15 @@ into the new version's section — see docs/releasing.md. ### Changed +- **Fewer downloads in vendored runs.** A vendored run now asks the patch + service for all of its planned packages' download references in one + request (in chunks of 500) from the first package it reaches, in place + of one request per package; an outage costs the same retries as before, + and a package the service reports still building is asked again at its + turn. A pypi patch the service serves as an sdist (every patch without a + file qualifier) is refused from its reference, before its bytes are + downloaded: `auto` still warns `vendor_prebuilt_unavailable` and builds + the wheel locally, `service` still refuses. - **One owner rule for the patch stores.** `list`, `vex`, `scan`'s `updates[]`, `rollback` and `remove` now read the manifest and the vendor ledger (plus, in `vex`, the hosted records) @@ -1927,22 +1953,25 @@ into the new version's section — see docs/releasing.md. covers the purl (its entry records the record's patch uuid and the committed artifact is on disk — a file artifact such as a wheel or tarball only while it still hashes to the ledger's `sha256`; `--force` - keeps the eager fetch), and for every lockfile-only cargo crate the - registry could fetch and verify (a crates.io `Cargo.lock` entry with a - checksum, or the pre-vendor resolution the ledger recovers) while the - patch service is enabled (the cargo backend reads the pristine source - only once `cargo_service_copy` falls back to the local build). A git, - path or custom-registry crate is never deferred: it keeps the eager - ladder's `vendor_fetch_unverifiable` + `package_not_installed` refusal - and is not vendored from the service's crates.io build, and a committed + keeps the eager fetch), and for every lockfile-only npm, cargo, golang + or composer package the registry would fetch and verify (a lock entry + with an integrity, or the pre-vendor resolution the ledger recovers, + that none of its fetcher's pre-download refusals applies to) while the + patch service is enabled (those backends read the pristine source only + once the service falls back to the local build; pypi and gem keep the + eager fetch, which their installed-variant probe reads). A git, path, + local-tarball or custom-registry package is never deferred: it keeps the + eager ladder's `vendor_fetch_unverifiable` + `package_not_installed` + refusal and is not vendored from the service's registry build, and a committed file artifact that no longer matches its pin keeps the eager ladder's outcome too. Visible effects: an idempotent re-run makes no registry requests and no longer reports `vendor_fetched_missing` for fetches it never needed; with no network (or under `--offline`) the re-run of an already-vendored pypi, cargo, go or lockfile-only gem project now SUCCEEDS (`already_vendored`, exit 0) instead of failing - `vendor_fetch_failed` / `package_not_installed`; a cargo crate the service - serves is never downloaded from the registry. When a deferred fetch does + `vendor_fetch_failed` / `package_not_installed`; an npm, cargo, golang or + composer package the service serves is never downloaded from the + registry. When a deferred fetch does happen (a drifted committed copy being rebuilt locally, a service miss), its `vendor_fetched_missing` warning is recorded just ahead of that package's own event instead of in the up-front fetch pass, and a failed, diff --git a/Cargo.lock b/Cargo.lock index 4475ec20..4a1aa3b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1769,6 +1769,7 @@ dependencies = [ "hex", "libc", "portable-pty", + "qbsdiff", "regex", "reqwest", "semver", diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 4ce5cb3a..b3a58ab1 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -127,7 +127,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc **Lockfile supplement (v3.4)**: `scan` discovery is no longer limited to installed trees. The project's lockfiles (`package-lock.json`/`npm-shrinkwrap.json`, `pnpm-lock.yaml` v9, `yarn.lock` classic + berry, `bun.lock`, `vlt-lock.json` (registry nodes, Socket-hosted pins included; vendored `file` nodes are left to the vendor ledger), `Cargo.lock`, `go.sum`, `composer.lock`, `Gemfile.lock`, `uv.lock`/`poetry.lock`/pinned `requirements.txt`) are inventoried and dependencies with NO installed copy join discovery — counts, the API lookup, the table (flagged ` [NOT INSTALLED]`, plus a stderr note), and the prune "scanned" set (a wiped node_modules no longer prunes lockfile-listed entries). JSON gains a top-level `lockfileOnlyPackages` count and an additive `notInstalled: true` on matching `packages[]` entries. `--apply` partitions lockfile-only patches out BEFORE download (calm `skipped`/`package_not_installed` records — never an error exit, never a manifest write); `--vendor` passes them through to the vendor engine's auto-fetch. Vendored-ledger entries likewise stay discoverable on a fresh clone (the committed artifact is the dependency). Global scans (`--global`) get no supplement. **Rush monorepos** (no root lockfile, `rush.json` present): the npm-lock inventory falls back to the Rush source-of-truth locks — `common/config/rush/pnpm-lock.yaml` plus every `common/config/subspaces/*/pnpm-lock.yaml` (`read_dir`-sorted, repo-relative paths preserved) — so a Rush repo's dependencies still join discovery. **Plug'n'Play layouts are an explicit refusal, not an empty inventory**: a `.pnp.*` loader means the npm packages are structurally unreachable in EVERY mode (under yarn PnP the installed-tree crawl is empty too — no `node_modules/`), so `scan` surfaces an additive top-level `warnings[]` array (`{code, detail}` objects, omitted when empty) carrying `yarn_pnp_unsupported` (same code as apply's refusal; remedy `yarn patch `) or `pnpm_pnp_unsupported` (pnpm's `node-linker=pnp` twin; pnpm remedies), plus a stderr `Warning: …` line on the human path. Exit code and `status` are deliberately unchanged (exit 0 / `success` — the same posture as hosted refusals, which exit 0 with `redirected: 0`); the warning is the machine-readable signal that nothing was checked. Pinned by `tests/e2e_safety_yarn_pnp.rs`. -**Vendor auto-fetch (v3.4)**: `vendor`/`scan --vendor` no longer fail on lockfile-resolved packages with no installed copy. Already-vendored purls stage from their committed artifact (sha256-verified against the vendor ledger — a vlt directory artifact against its file inventory, which leaves out the links vlt creates inside it; offline-safe) when the ledger entry is at the manifest record's patch uuid; a superseding uuid fetches the pristine package instead, since the older artifact holds the older patch's bytes. Otherwise the pristine artifact is fetched per the lockfile resolution and verified against the lock's recorded integrity FAIL-CLOSED before any write: npm SRI (or yarn classic's sha1 fragment; for vlt the registry node's slot [2]), yarn berry's cache-zip checksum (rebuilt from the fetched tarball; cacheKey 10c0 only), Cargo.lock sha256 over the .crate, go.sum `h1:` dirhash over the module zip, composer `dist.shasum` (sha1), Gemfile.lock `CHECKSUMS` sha256, uv.lock wheel sha256 (pure `py3-none-any` wheels only). Entries the lock cannot verify are NEVER fetched (`vendor_fetch_unverifiable` warning + the calm `package_not_installed` skip). Registry bases honor `SOCKET_NPM_REGISTRY`, `SOCKET_CRATES_REGISTRY`, `SOCKET_GOPROXY` (else `GOPROXY`, `GONOPROXY` and `GOPRIVATE` the way go reads them — see the env table); npm/yarn/composer/gem/uv lock-recorded URLs are used verbatim. `--offline` refuses the fetch with the calm skip (the detail names the lockfile resolution). The fetch stages into a private tempdir — the project tree is never touched. **Deferred fetch (v5.0):** a purl the vendor ledger already covers (its entry records the record's patch uuid and the committed artifact is on disk — a file artifact only while it hashes to the ledger's `sha256`; not under `--force`), and a lockfile-only cargo crate the registry could fetch and verify (a crates.io `Cargo.lock` entry with a checksum, or the pre-vendor resolution the ledger recovers) while the patch service is enabled, are NOT downloaded up front: the fetch runs only if the backend reaches a branch that reads the pristine tree (a drifted committed copy rebuilt locally, a service miss). An in-sync re-run therefore makes no registry request, reports no `vendor_fetched_missing`, and succeeds with no network or under `--offline`. A deferred fetch that does run records its `vendor_fetched_missing` just ahead of the package's own event; one that fails, is unverifiable, or is refused by `--offline` reports the same events the up-front fetch would have. A git, path or custom-registry cargo crate is never deferred, so it keeps `vendor_fetch_unverifiable` + `package_not_installed` and is never vendored from the service's crates.io build. **Gem, local build only** (`--vendor-source build`, or no service config): a not-installed gem the lock can verify (bundler >= 2.6 `CHECKSUMS`) and no ledger entry covers is refused `gem_spec_missing` (`failed`, the backend's own detail) BEFORE any download — a downloaded `.gem` carries no eval-able stub gemspec, so a local build can never vendor it; no `vendor_fetched_missing` precedes it, and a refusal the backend would have reached first on the fetched copy reports as `gem_spec_missing` too. Not under `--dry-run`, which still fetches and previews the gem (`vendor_fetched_missing` + `verified`). +**Vendor auto-fetch (v3.4)**: `vendor`/`scan --vendor` no longer fail on lockfile-resolved packages with no installed copy. Already-vendored purls stage from their committed artifact (sha256-verified against the vendor ledger — a vlt directory artifact against its file inventory, which leaves out the links vlt creates inside it; offline-safe) when the ledger entry is at the manifest record's patch uuid; a superseding uuid fetches the pristine package instead, since the older artifact holds the older patch's bytes. Otherwise the pristine artifact is fetched per the lockfile resolution and verified against the lock's recorded integrity FAIL-CLOSED before any write: npm SRI (or yarn classic's sha1 fragment; for vlt the registry node's slot [2]), yarn berry's cache-zip checksum (rebuilt from the fetched tarball; cacheKey 10c0 only), Cargo.lock sha256 over the .crate, go.sum `h1:` dirhash over the module zip, composer `dist.shasum` (sha1), Gemfile.lock `CHECKSUMS` sha256, uv.lock wheel sha256 (pure `py3-none-any` wheels only). Entries the lock cannot verify are NEVER fetched (`vendor_fetch_unverifiable` warning + the calm `package_not_installed` skip). Registry bases honor `SOCKET_NPM_REGISTRY`, `SOCKET_CRATES_REGISTRY`, `SOCKET_GOPROXY` (else `GOPROXY`, `GONOPROXY` and `GOPRIVATE` the way go reads them — see the env table); npm/yarn/composer/gem/uv lock-recorded URLs are used verbatim. `--offline` refuses the fetch with the calm skip (the detail names the lockfile resolution). The fetch stages into a private tempdir — the project tree is never touched. **Deferred fetch (v5.0):** a purl the vendor ledger already covers (its entry records the record's patch uuid and the committed artifact is on disk — a file artifact only while it hashes to the ledger's `sha256`; not under `--force`), and a lockfile-only npm, cargo, golang or composer package the registry would fetch and verify (a lock entry with an integrity, or the pre-vendor resolution the ledger recovers, that none of its fetcher's pre-download refusals applies to: a yarn berry cacheKey other than 10c0, a go module go fetches without a proxy, a composer entry with no dist URL) while the patch service is enabled, are NOT downloaded up front (those backends ask the service first and read the pristine tree only on a local-build fallback; pypi and gem keep the up-front fetch, which their installed-variant probe reads): the fetch runs only if the backend reaches a branch that reads the pristine tree (a drifted committed copy rebuilt locally, a service miss). An in-sync re-run therefore makes no registry request, reports no `vendor_fetched_missing`, and succeeds with no network or under `--offline`. A deferred fetch that does run records its `vendor_fetched_missing` just ahead of the package's own event; one that fails, is unverifiable, or is refused by `--offline` reports the same events the up-front fetch would have. A package whose fetch would be refused is never deferred (a git, path or custom-registry cargo crate, say), so it keeps `vendor_fetch_unverifiable` + `package_not_installed` and is never vendored from the service's registry build. **Gem, local build only** (`--vendor-source build`, or no service config): a not-installed gem the lock can verify (bundler >= 2.6 `CHECKSUMS`) and no ledger entry covers is refused `gem_spec_missing` (`failed`, the backend's own detail) BEFORE any download — a downloaded `.gem` carries no eval-able stub gemspec, so a local build can never vendor it; no `vendor_fetched_missing` precedes it, and a refusal the backend would have reached first on the fetched copy reports as `gem_spec_missing` too. Not under `--dry-run`, which still fetches and previews the gem (`vendor_fetched_missing` + `verified`). **Vendored write durability (v5.0)**: every write is atomic (stage + rename), but only the durable commit points — lockfiles, `go.mod`/`go.sum`, `pom.xml`, `nuget.config`, `package.json`, `pnpm-workspace.yaml`, `.cargo/config.toml`, the Python/Ruby manifests, `.socket/vendor/state.json` and `redirect-state.json` — are fsynced on write. The content-verified artifacts under `.socket/vendor///` (patched copies, packed/rebuilt archives and sidecars, markers) are written without an fsync and made durable by one barrier (file + directory fsync, one `F_FULLFSYNC` per device on macOS) ahead of the next commit point — and, for an artifact rebuilt in place that no commit point follows, at the end of the vendored run's commit and when the command releases the apply lock — so a crash can only lose an artifact that no durable commit point names yet, which the next run rebuilds. @@ -1162,7 +1162,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `vendor_override_conflict` | `failed` | vendor (pnpm/yarn-berry): a user-authored override/resolution for the package already exists. | | `vendor_integrity_unverified` | `skipped` (warning) | vendor (pipenv): the lockfile format does not hash-check file entries; the committed wheel bytes are the protection. | | `vendor_content_mismatch_overwritten` | `skipped` (warning) | vendor: a staged file matched NEITHER beforeHash nor afterHash (patch built against different bytes, or local edits); the stage was overwritten with the verified patched content and the vendor succeeded. | -| `vendor_fetched_missing` | `skipped` (warning) | vendor: the package was not installed; its pristine artifact was fetched per the lockfile resolution (or staged from the committed vendor artifact), integrity-verified, and vendored — the project tree was not touched. Not emitted when no fetch happened: an in-sync re-run of a ledger-covered purl, or a cargo crate the patch service served (see Vendor auto-fetch § Deferred fetch). For `poetry.lock` (which records hashes but no URLs) the pure-Python wheel's sha256 selects the file through PyPI's JSON API (`SOCKET_PYPI_JSON_API` overrides the endpoint); Poetry 0.12's bare `[metadata.hashes]` names no wheel, so those locks still need an installed copy (`vendor_fetch_unverifiable`). | +| `vendor_fetched_missing` | `skipped` (warning) | vendor: the package was not installed; its pristine artifact was fetched per the lockfile resolution (or staged from the committed vendor artifact), integrity-verified, and vendored — the project tree was not touched. Not emitted when no fetch happened: an in-sync re-run of a ledger-covered purl, or an npm, cargo, golang or composer package the patch service served (see Vendor auto-fetch § Deferred fetch). For `poetry.lock` (which records hashes but no URLs) the pure-Python wheel's sha256 selects the file through PyPI's JSON API (`SOCKET_PYPI_JSON_API` overrides the endpoint); Poetry 0.12's bare `[metadata.hashes]` names no wheel, so those locks still need an installed copy (`vendor_fetch_unverifiable`). | | `vendor_fetch_failed` | `failed` | vendor: the lockfile-resolved fetch was attempted and failed (HTTP error, size cap, integrity mismatch, or a PRESENT-but-corrupt committed artifact — pointed at `socket-patch repair`). A MISSING committed artifact no longer lands here: it falls through to the ledger-recovered registry fetch. Suppresses the duplicate `package_not_installed` skip. | | `vendor_fetch_unverifiable` | `skipped` (warning) | vendor: the lockfile records no usable integrity for the missing package; nothing was fetched (fail-closed) and the `package_not_installed` skip follows. Unchanged for gems by the build-mode `gem_spec_missing` refusal below, which fires only where a fetch WOULD have run. | | `vendor_vlt_transitive_unsupported` | `failed` | vendor (vlt): the target has an inbound edge from another package in `vlt-lock.json` (the detail names it); vendored mode rewires only direct dependencies of the root or a workspace member, because vlt silently reverts transitive lock surgery. Remedy: `--mode hosted`. Refused before any download or write, dry runs included (`would_refuse`). | @@ -1239,7 +1239,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `apply` | `Applied` · `Updated` · `Skipped` (already_patched / package_not_installed / vendored) · `Failed` · `Verified` (dry-run) | | `vendor` | `Applied` (= vendored; `command` routes) · `Skipped` (refusals, warnings, unsupported ecosystems) · `Failed` · `Removed` (reconcile + `--revert`) · `Verified` (dry-run) | | `list` | `Discovered` (with `details.vulnerabilities`, `details.tier`, `details.license`, `details.description`, `details.exportedAt`; hosted pins (v5.0: one per `(purl, uuid)` the lockfiles wire) additionally carry `details.mode: "hosted"` and `details.lockfiles: []` (no `details.ledger` — hosted mode keeps no ledger; the human listing labels them `Mode: hosted (wired in )`), both additive and absent on manifest entries; v5.0: vendor-ledger records carry `details.mode: "vendored"` + `details.ledger: ".socket/vendor/state.json"` the same way, and the human listing labels them `Mode: vendored (recorded in .socket/vendor/state.json)`; a `state.json` that cannot be read or parsed degrades to nothing-to-consult with the stderr line `Warning: unreadable vendor ledger (); its vendored patches are not listed` — muted by `--silent`, exit unchanged) | -| `repair`/`gc`| `Downloaded` (or `Verified` on dry-run) · `Rebuilt` (vendored artifacts; `Verified` previews on dry-run) · `Skipped` (vendor_uuid_mismatch) · `Removed` (or `Verified`) · `Failed` events | +| `repair`/`gc`| `Downloaded` (or `Verified` on dry-run; a diff-mode repair adds a second one, `mode: "file"`, for the blobs of files the patches create) · `Rebuilt` (vendored artifacts; `Verified` previews on dry-run) · `Skipped` (vendor_uuid_mismatch) · `Removed` (or `Verified`) · `Failed` events | | `remove` | `Removed` (per purl; `Verified` on dry-run) · artifact-level `Removed`/`Verified` event (with `details.blobsRemoved`, `details.rolledBack`) | | `--update` | `Downloaded` → `Updated` (success) · `Skipped` (already_latest) · `Verified` (dry-run check, reason update_check) — see the Self-update contract section for details fields and top-level error codes | diff --git a/crates/socket-patch-cli/Cargo.toml b/crates/socket-patch-cli/Cargo.toml index bd7262d0..ab434d5e 100644 --- a/crates/socket-patch-cli/Cargo.toml +++ b/crates/socket-patch-cli/Cargo.toml @@ -64,6 +64,8 @@ sha1 = { workspace = true } # scan_vendor_e2e builds pristine registry tarballs for the auto-fetch tests. tar = { workspace = true } flate2 = { workspace = true } +# diff_created_file_e2e builds a real diff archive. +qbsdiff = { workspace = true } # update_fixture builds the Windows-shaped release archive for self-update e2e. zip = { workspace = true } hex = { workspace = true } diff --git a/crates/socket-patch-cli/src/commands/fetch_stage.rs b/crates/socket-patch-cli/src/commands/fetch_stage.rs index e60bddb9..c3e11faa 100644 --- a/crates/socket-patch-cli/src/commands/fetch_stage.rs +++ b/crates/socket-patch-cli/src/commands/fetch_stage.rs @@ -201,10 +201,11 @@ fn format_blob_fallback(diff_failed: usize, blobs: usize) -> String { } /// The manifest PURLs with no usable local source. A patch is "locally -/// applicable" iff at least one of: -/// - every `after_hash` blob it references is on disk, OR -/// - its diff archive is on disk, OR -/// - its package archive is on disk. +/// applicable" iff its package archive is on disk, or every file it +/// touches has its `after_hash` blob on disk or is covered by the patch's +/// diff archive. A diff covers only files that exist before the patch: a +/// created file (empty `before_hash`) has nothing to diff against, so it +/// always needs its blob. /// /// The patch pipeline picks whichever is present per file. Shared by the /// offline gate (probed against `.socket/`) and the post-download gate @@ -219,13 +220,13 @@ fn patches_without_source<'m>( .patches .iter() .filter_map(|(purl, record)| { - let all_blobs_present = record - .files - .values() - .all(|f| !missing_blobs.contains(&f.after_hash)); let diff_present = !missing_diff_archives.contains(&record.uuid); let pkg_present = !missing_package_archives.contains(&record.uuid); - if all_blobs_present || diff_present || pkg_present { + let files_covered = record.files.values().all(|f| { + !missing_blobs.contains(&f.after_hash) + || (diff_present && !f.before_hash.is_empty()) + }); + if pkg_present || files_covered { None } else { Some(purl.as_str()) @@ -234,6 +235,33 @@ fn patches_without_source<'m>( .collect() } +/// `manifest` cut down to the files a diff archive cannot patch (created +/// files, whose `before_hash` is empty): the blobs a diff-mode fetch still +/// needs even when every diff archive is present. +pub(crate) fn files_diffs_cannot_cover(manifest: &PatchManifest) -> PatchManifest { + let patches = manifest + .patches + .iter() + .filter_map(|(purl, record)| { + let files: HashMap<_, _> = record + .files + .iter() + .filter(|(_, f)| f.before_hash.is_empty()) + .map(|(k, v)| (k.clone(), v.clone())) + .collect(); + (!files.is_empty()).then(|| { + let mut record = record.clone(); + record.files = files; + (purl.clone(), record) + }) + }) + .collect(); + PatchManifest { + patches, + setup: manifest.setup.clone(), + } +} + /// Mirror `src`'s files into `dst` by hardlink (copy fallback). Pre-seeds the /// overlay tempdir with everything already cached so only the gap downloads. async fn overlay_dir(src: &Path, dst: &Path) { @@ -311,12 +339,14 @@ pub(crate) async fn stage_patch_sources( // locally. We honor `--download-mode` for the primary fetch when there's // actually a gap to close. Skip the archive fetch entirely when all file // blobs are already present locally — the pipeline will succeed via the - // blob path, so an archive fetch would be wasted round-trips. + // blob path, so an archive fetch would be wasted round-trips. Cached + // diff archives can still leave a patch uncovered (a created file), and + // the blob top-up below closes that gap. let download_needed = !common.offline && match download_mode { DownloadMode::File => !missing_blobs.is_empty(), DownloadMode::Diff if missing_blobs.is_empty() => false, - DownloadMode::Diff => !missing_diff_archives.is_empty(), + DownloadMode::Diff => !missing_diff_archives.is_empty() || !no_source_purls.is_empty(), }; if !download_needed { @@ -374,15 +404,25 @@ pub(crate) async fn stage_patch_sources( // For non-file modes, automatically fetch any still-missing file blobs as // a fallback. Patches that lack the requested mode on the server will // still apply via the legacy blob path. + // + // With every diff archive already cached, only the files no diff can + // patch are fetched: that is the gap that triggered this download. let mut blob_fetch_failed = false; if download_mode != DownloadMode::File { - let still_missing_blobs = get_missing_blobs(manifest, &staged.blobs).await; + let created_only; + let blob_scope = if missing_diff_archives.is_empty() { + created_only = files_diffs_cannot_cover(manifest); + &created_only + } else { + manifest + }; + let still_missing_blobs = get_missing_blobs(blob_scope, &staged.blobs).await; if !still_missing_blobs.is_empty() { status.set(format_blob_fallback( fetch_result.failed, still_missing_blobs.len(), )); - let blob_result = fetch_missing_blobs(manifest, &staged.blobs, client, None).await; + let blob_result = fetch_missing_blobs(blob_scope, &staged.blobs, client, None).await; status.finish(); if !quiet { for line in format_fetch_summary(&blob_result, BLOB, true) { @@ -972,6 +1012,70 @@ mod tests { ); } + /// A diff archive cannot patch a file the patch creates (nothing to diff + /// against), so it covers such a patch only together with the created + /// file's blob: without it, offline staging is Unavailable up front + /// instead of passing the gate and failing mid-apply. + #[tokio::test] + async fn stage_offline_diff_archive_does_not_cover_a_created_file() { + let tmp = tempfile::tempdir().unwrap(); + let socket_dir = tmp.path().join(".socket"); + std::fs::create_dir_all(socket_dir.join("diffs")).unwrap(); + std::fs::write( + socket_dir.join("diffs").join(format!("{UUID}.tar.gz")), + b"x", + ) + .unwrap(); + let created = "c".repeat(64); + let mut manifest = manifest_with_one_patch(); + manifest + .patches + .get_mut("pkg:npm/left-pad@1.3.0") + .unwrap() + .files + .insert( + "new.js".to_string(), + PatchFileInfo { + before_hash: String::new(), + after_hash: created.clone(), + }, + ); + + let outcome = + stage_patch_sources(&offline_args(), &manifest, &socket_dir, &offline_client()) + .await + .expect("no hard failure"); + assert!(matches!(outcome, StageOutcome::Unavailable)); + + std::fs::create_dir_all(socket_dir.join("blobs")).unwrap(); + std::fs::write(socket_dir.join("blobs").join(&created), b"new").unwrap(); + let outcome = + stage_patch_sources(&offline_args(), &manifest, &socket_dir, &offline_client()) + .await + .expect("no hard failure"); + assert!( + matches!(outcome, StageOutcome::Ready(_)), + "diff for the modified file + blob for the created one covers the patch" + ); + } + + #[test] + fn files_diffs_cannot_cover_keeps_only_created_files() { + let mut manifest = manifest_with_one_patch(); + assert!(files_diffs_cannot_cover(&manifest).patches.is_empty()); + let record = manifest.patches.get_mut("pkg:npm/left-pad@1.3.0").unwrap(); + record.files.insert( + "new.js".to_string(), + PatchFileInfo { + before_hash: String::new(), + after_hash: "c".repeat(64), + }, + ); + let cut = files_diffs_cannot_cover(&manifest); + let files: Vec<&String> = cut.patches["pkg:npm/left-pad@1.3.0"].files.keys().collect(); + assert_eq!(files, ["new.js"]); + } + /// The vendor (in-memory) stager documents the opposite policy: a diff /// archive is NOT sufficient (auto-force can need the full after-blob), /// so the same fixture that satisfies the disk stager is Unavailable diff --git a/crates/socket-patch-cli/src/commands/repair.rs b/crates/socket-patch-cli/src/commands/repair.rs index 48fd4d0b..9444b0ac 100644 --- a/crates/socket-patch-cli/src/commands/repair.rs +++ b/crates/socket-patch-cli/src/commands/repair.rs @@ -14,6 +14,7 @@ use std::path::Path; use std::time::Duration; use crate::args::{apply_env_toggles, parse_bool_flag, GlobalArgs}; +use crate::commands::fetch_stage::files_diffs_cannot_cover; use crate::commands::lock_cli::{acquire_or_emit, error_envelope}; use crate::commands::rollback::{sweep_failure, sweep_unused_artifacts}; use crate::json_envelope::{Command, Envelope, PatchAction, PatchEvent, Status}; @@ -341,6 +342,100 @@ fn format_final_line( } } +/// The `.socket/` source directories a download pass writes into. +struct SourcePaths<'a> { + blobs: &'a Path, + packages: &'a Path, + diffs: &'a Path, +} + +/// What one download pass did: how many artifacts were missing, and how +/// many of them it downloaded or failed to. +#[derive(Default)] +struct DownloadPass { + missing: usize, + downloaded: usize, + failed: usize, +} + +/// Step 1's pass over `missing` (non-empty), the `mode` artifacts `m` +/// references: the `--offline` warning, the `--dry-run` preview, or the +/// download and its result lines. +async fn download_pass( + args: &RepairArgs, + client: &mut Option, + m: &socket_patch_core::manifest::schema::PatchManifest, + missing: &[String], + mode: DownloadMode, + paths: &SourcePaths<'_>, +) -> DownloadPass { + let quiet = args.common.json || args.common.silent; + let noun = mode.noun(); + let mut pass = DownloadPass { + missing: missing.len(), + ..DownloadPass::default() + }; + if args.common.offline { + if !quiet { + eprintln!("{}", format_offline_warning(missing, noun)); + } + return pass; + } + if !quiet { + println!("{}", format_found_missing(missing.len(), noun)); + } + if args.common.dry_run { + if !quiet { + println!(); + println!("Would download:"); + for line in format_id_list(missing, noun, DRY_RUN_LIST_CAP) { + println!("{line}"); + } + } + return pass; + } + let mut status = crate::ui::StatusLine::stderr(args.common.json, args.common.silent); + status.set(format!("Downloading {}...", noun.count(missing.len()))); + if client.is_none() { + *client = Some( + get_api_client_with_overrides(args.common.api_client_overrides()) + .await + .0, + ); + } + let client = client.as_ref().expect("client built just above"); + let sources = PatchSources { + blobs_path: paths.blobs, + packages_path: Some(paths.packages), + diffs_path: Some(paths.diffs), + mem_blobs: None, + }; + let fetch_result = fetch_missing_sources(m, &sources, mode, client, None).await; + status.finish(); + pass.downloaded = fetch_result.downloaded; + pass.failed = fetch_result.failed; + if !quiet { + for line in format_fetch_successes(&fetch_result, noun) { + println!("{line}"); + } + } + // Failures are error output: stderr, and not muted by `--silent` + // (`--json` runs carry them in the envelope). + if !args.common.json { + for (i, line) in format_fetch_failures(&fetch_result, noun) + .iter() + .enumerate() + { + if i == 0 { + eprintln!("Error: {line}"); + } else { + eprintln!("{line}"); + } + } + } + pass +} + /// Whether an API token will be found, mirroring the client's chain: the /// `--api-token` flag (clap also maps SOCKET_API_TOKEN into it), then — /// unless `SOCKET_NO_API_TOKEN` vetoes ambient tokens — the env var and the @@ -455,86 +550,48 @@ async fn repair_inner( .into_iter() .collect(), }; - let missing_count = missing_artifacts.len(); let noun = download_mode.noun(); + let paths = SourcePaths { + blobs: &blobs_path, + packages: &packages_path, + diffs: &diffs_path, + }; // Whether stdout already carries a line, so the blank separators // between sections never open the output (the offline warning goes // to stderr). - let mut stdout_started = true; - - if missing_artifacts.is_empty() { - if !quiet { - println!("{}", format_nothing_missing(manifest.as_ref(), noun)); + let mut stdout_started = !args.common.offline || missing_artifacts.is_empty(); + let primary = match scoped_manifest.as_ref() { + Some(m) if !missing_artifacts.is_empty() => { + download_pass(args, client, m, &missing_artifacts, download_mode, &paths).await } - } else if args.common.offline { - if !quiet { - eprintln!("{}", format_offline_warning(&missing_artifacts, noun)); - } - stdout_started = false; - } else { - if !quiet { - println!("{}", format_found_missing(missing_artifacts.len(), noun)); - } - - if args.common.dry_run { + _ => { if !quiet { - println!(); - println!("Would download:"); - for line in format_id_list(&missing_artifacts, noun, DRY_RUN_LIST_CAP) { - println!("{line}"); - } - } - } else { - let mut status = crate::ui::StatusLine::stderr(args.common.json, args.common.silent); - status.set(format!( - "Downloading {}...", - noun.count(missing_artifacts.len()) - )); - if client.is_none() { - *client = Some( - get_api_client_with_overrides(args.common.api_client_overrides()) - .await - .0, - ); + println!("{}", format_nothing_missing(manifest.as_ref(), noun)); } - let client = client.as_ref().expect("client built just above"); - let sources = PatchSources { - blobs_path: &blobs_path, - packages_path: Some(&packages_path), - diffs_path: Some(&diffs_path), - mem_blobs: None, - }; - // Step 1 only runs with a manifest (missing_artifacts is - // empty otherwise), so the expect is unreachable. - let m = scoped_manifest - .as_ref() - .expect("step 1 requires a manifest"); - let fetch_result = - fetch_missing_sources(m, &sources, download_mode, client, None).await; - status.finish(); - downloaded_count = fetch_result.downloaded; - download_failed_count = fetch_result.failed; - if !quiet { - for line in format_fetch_successes(&fetch_result, noun) { - println!("{line}"); - } - } - // Failures are error output: stderr, and not muted by - // `--silent` (`--json` runs carry them in the envelope). - if !args.common.json { - for (i, line) in format_fetch_failures(&fetch_result, noun) - .iter() - .enumerate() - { - if i == 0 { - eprintln!("Error: {line}"); - } else { - eprintln!("{line}"); - } - } + DownloadPass::default() + } + }; + // A diff archive has no delta for a file the patch creates, so in diff + // mode that file's blob is downloaded too: without it, a later + // `apply --offline` cannot apply the patch. + let created = match (&scoped_manifest, download_mode) { + (Some(m), DownloadMode::Diff) => { + let created = files_diffs_cannot_cover(m); + let missing: Vec = get_missing_blobs(&created, &blobs_path) + .await + .into_iter() + .collect(); + if missing.is_empty() { + DownloadPass::default() + } else { + download_pass(args, client, &created, &missing, DownloadMode::File, &paths).await } } - } + _ => DownloadPass::default(), + }; + let missing_count = primary.missing; + downloaded_count += primary.downloaded; + download_failed_count += primary.failed; // Step 1.5: vendored artifacts — health-check the ledger and re-vendor // missing/corrupt artifacts through the vendored backend (the patch @@ -620,13 +677,24 @@ async fn repair_inner( // so a piped stdout never ends in a stray blank line when the // line itself goes to stderr. let other_failure = matches!(env.status, Status::PartialFailure | Status::Error); - let line = format_final_line( - download_failed_count, - other_failure, - noun, - args.common.dry_run, - ); - if download_failed_count > 0 || other_failure { + let failed = download_failed_count + created.failed; + let line = if download_failed_count > 0 && created.failed > 0 { + format!( + "Repair finished with errors: {} and {} were not downloaded.", + noun.count(download_failed_count), + BLOB.count(created.failed) + ) + } else if download_failed_count > 0 { + format_final_line( + download_failed_count, + other_failure, + noun, + args.common.dry_run, + ) + } else { + format_final_line(created.failed, other_failure, BLOB, args.common.dry_run) + }; + if failed > 0 || other_failure { if stdout_started { eprintln!(); } @@ -667,6 +735,28 @@ async fn repair_inner( )); env.mark_partial_failure(); } + if created.downloaded > 0 + || (!args.common.offline && args.common.dry_run && created.missing > 0) + { + let (action, count) = if args.common.dry_run { + (PatchAction::Verified, created.missing) + } else { + (PatchAction::Downloaded, created.downloaded) + }; + env.record( + PatchEvent::artifact(action).with_details(serde_json::json!({ + "count": count, + "mode": DownloadMode::File.as_tag(), + })), + ); + } + if created.failed > 0 { + env.record(PatchEvent::artifact(PatchAction::Failed).with_error( + "download_failed", + format!("{} failed to download", BLOB.count(created.failed)), + )); + env.mark_partial_failure(); + } if blobs_cleaned > 0 { let cleanup_action = if args.common.dry_run { PatchAction::Verified @@ -683,7 +773,7 @@ async fn repair_inner( Ok(( env, RepairCounts { - downloaded: downloaded_count, + downloaded: downloaded_count + created.downloaded, cleaned: blobs_cleaned, bytes_freed, }, diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 38ab3964..093de219 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1686,6 +1686,45 @@ pub(crate) async fn pristine_fetch_is_verifiable( } } +/// Whether [`fetch_pristine_package`] would reach the download for this +/// purl: the same entry choice (see [`pristine_fetch_is_verifiable`]), and +/// none of the refusals its fetcher raises before the first request (a +/// foreign yarn berry cacheKey, a go module go would not fetch through a +/// proxy, a composer entry with no dist URL). Deferring a fetch that would +/// refuse `vendor_fetch_unverifiable` behind the patch service would +/// instead vendor the patch over a package it does not describe. +async fn pristine_fetch_reaches_download( + project_root: &Path, + inventory: &[lock_inventory::LockfileEntry], + purl: &str, + ledger_entry: Option<&VendorEntry>, +) -> bool { + let entry = match lock_inventory::lookup(inventory, purl) + .filter(|e| e.integrity != lock_inventory::LockIntegrity::None) + { + Some(e) => e.clone(), + None => match ledger_entry { + Some(le) => match lock_inventory::recover_lock_entry(project_root, le).await { + Ok(e) => e, + Err(_) => return false, + }, + None => return false, + }, + }; + registry_fetch::refusal_before_download(&entry).is_none() +} + +/// The ecosystems whose backend asks the patch service before it reads the +/// pristine tree, and reads it only on a local-build fallback. pypi and gem +/// read it earlier, in the loop's installed-variant probe; nuget and maven +/// have no registry fetch. +fn backend_reads_pristine_only_on_fallback(purl: &str) -> bool { + matches!( + Ecosystem::from_purl(purl), + Some(Ecosystem::Npm | Ecosystem::Cargo | Ecosystem::Golang | Ecosystem::Composer) + ) +} + /// The purls among `purls` with an installed copy, found exactly as the /// vendor loop finds them: the qualified-aware resolver /// ([`find_packages_for_rollback_reusing`]), then the npm `package.json` @@ -2365,11 +2404,12 @@ pub(crate) async fn vendor_records_reusing( // backend's in-sync hot path answers it from the committed // bytes alone, so a re-run needs no network. `--force` may // rebuild anyway, so it keeps the eager fetch. - // * a cargo crate the patch service can serve: the backend reads - // the pristine tree only if it falls back to the local build. - // Only a crate the registry ladder COULD fetch (see - // `pristine_fetch_is_verifiable`) — a git, path or - // custom-registry crate keeps the eager rung, whose + // * a package the patch service can serve, in an ecosystem whose + // backend reads the pristine tree only if it falls back to the + // local build (`backend_reads_pristine_only_on_fallback`). + // Only one the registry ladder would really download (see + // `pristine_fetch_reaches_download`) — a git, path or + // custom-registry crate, say, keeps the eager rung, whose // `vendor_fetch_unverifiable` refusal keeps a crates.io patch // off it. // @@ -2389,10 +2429,10 @@ pub(crate) async fn vendor_records_reusing( } None => false, }; - let cargo_via_service = service_enabled + let via_service = service_enabled && matches!(rung, MissingRung::Fetch) - && Ecosystem::from_purl(purl) == Some(Ecosystem::Cargo) - && pristine_fetch_is_verifiable( + && backend_reads_pristine_only_on_fallback(purl) + && pristine_fetch_reaches_download( &common.cwd, inventory .get_or_init(|| lock_inventory::inventory_project(&common.cwd)) @@ -2401,7 +2441,7 @@ pub(crate) async fn vendor_records_reusing( lookup_entry(&state.entries, purl), ) .await; - if covered || cargo_via_service { + if covered || via_service { *rung = MissingRung::Deferred; } } diff --git a/crates/socket-patch-cli/tests/diff_created_file_e2e.rs b/crates/socket-patch-cli/tests/diff_created_file_e2e.rs new file mode 100644 index 00000000..50b7b879 --- /dev/null +++ b/crates/socket-patch-cli/tests/diff_created_file_e2e.rs @@ -0,0 +1,282 @@ +//! A diff archive only carries deltas for files that exist before the +//! patch: a file the patch CREATES (empty `beforeHash`) has nothing to +//! diff against, so the patch service leaves it out and the pipeline can +//! only apply it from its after-blob (or a package archive). These tests +//! pin that the disk stager and `repair` both treat a diff archive as +//! covering only the files it can actually patch: +//! +//! - `apply --offline` with just the diff archive on disk fails closed +//! with the "no local source" report and leaves every file untouched, +//! instead of passing the gate and failing mid-apply; +//! - online `apply` with the diff archive cached still fetches the +//! created file's blob; +//! - a default (diff-mode) `repair` also downloads the created file's +//! blob, so a later `apply --offline` succeeds. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use flate2::write::GzEncoder; +use flate2::Compression; +use qbsdiff::Bsdiff; +use sha2::{Digest, Sha256}; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +const ORG_SLUG: &str = "test-org"; +const UUID: &str = "67676767-6767-4767-8767-676767676767"; +const PURL: &str = "pkg:npm/created-file-test@1.0.0"; +const BEFORE: &[u8] = b"module.exports = 'before, and long enough to diff';\n"; +const AFTER: &[u8] = b"module.exports = 'after!, and long enough to diff';\n"; +const CREATED: &[u8] = b"module.exports = 'a brand new file';\n"; + +fn binary() -> PathBuf { + env!("CARGO_BIN_EXE_socket-patch").into() +} + +fn git_sha256(content: &[u8]) -> String { + let header = format!("blob {}\0", content.len()); + let mut hasher = Sha256::new(); + hasher.update(header.as_bytes()); + hasher.update(content); + hex::encode(hasher.finalize()) +} + +fn run_cli(root: &Path, argv: &[&str], mock_uri: Option<&str>) -> (i32, String, String) { + let mut cmd = Command::new(binary()); + cmd.args(argv).current_dir(root); + for (key, _) in std::env::vars_os() { + if key.to_string_lossy().starts_with("SOCKET_") + && key.to_string_lossy() != "SOCKET_NO_CONFIG" + { + cmd.env_remove(&key); + } + } + cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); + // Offline runs get a dead endpoint: any request they make fails. + let api_url = mock_uri.unwrap_or("http://127.0.0.1:1"); + cmd.env("SOCKET_API_URL", api_url) + .env("SOCKET_API_TOKEN", "fake-token-for-test") + .env("SOCKET_ORG_SLUG", ORG_SLUG); + let out = cmd.output().expect("run socket-patch"); + ( + out.status.code().unwrap_or(-1), + String::from_utf8_lossy(&out.stdout).into_owned(), + String::from_utf8_lossy(&out.stderr).into_owned(), + ) +} + +/// The diff archive the service serves for this patch: a bsdiff delta for +/// the modified file and nothing for the created one. +fn diff_archive() -> Vec { + let mut delta = Vec::new(); + Bsdiff::new(BEFORE, AFTER) + .compare(std::io::Cursor::new(&mut delta)) + .unwrap(); + let mut builder = tar::Builder::new(GzEncoder::new(Vec::new(), Compression::default())); + let mut header = tar::Header::new_gnu(); + header.set_size(delta.len() as u64); + header.set_mode(0o644); + header.set_cksum(); + builder + .append_data(&mut header, "index.js", delta.as_slice()) + .unwrap(); + builder.into_inner().unwrap().finish().unwrap() +} + +/// An npm project with the unpatched package installed and a manifest +/// whose patch modifies `index.js` and creates `new.js` (sorted after +/// `index.js`, so a mid-apply failure would leave `index.js` patched). Returns the +/// installed package dir. +fn seed_project(root: &Path) -> PathBuf { + std::fs::write( + root.join("package.json"), + r#"{"name":"created-file-root","version":"0.0.0"}"#, + ) + .unwrap(); + let pkg = root.join("node_modules").join("created-file-test"); + std::fs::create_dir_all(&pkg).unwrap(); + std::fs::write( + pkg.join("package.json"), + r#"{"name":"created-file-test","version":"1.0.0"}"#, + ) + .unwrap(); + std::fs::write(pkg.join("index.js"), BEFORE).unwrap(); + + let socket = root.join(".socket"); + std::fs::create_dir_all(&socket).unwrap(); + std::fs::write( + socket.join("manifest.json"), + serde_json::to_vec_pretty(&serde_json::json!({ + "patches": { + PURL: { + "uuid": UUID, + "exportedAt": "2026-01-01T00:00:00Z", + "files": { + "package/index.js": { + "beforeHash": git_sha256(BEFORE), + "afterHash": git_sha256(AFTER), + }, + "package/new.js": { + "beforeHash": "", + "afterHash": git_sha256(CREATED), + } + }, + "vulnerabilities": {}, + "description": "creates a file", + "license": "MIT", + "tier": "free", + } + } + })) + .unwrap(), + ) + .unwrap(); + pkg +} + +fn seed_cached_diff_archive(root: &Path) { + let diffs = root.join(".socket").join("diffs"); + std::fs::create_dir_all(&diffs).unwrap(); + std::fs::write(diffs.join(format!("{UUID}.tar.gz")), diff_archive()).unwrap(); +} + +async fn mount_blob(mock: &MockServer, content: &'static [u8]) { + Mock::given(method("GET")) + .and(path(format!( + "/v0/orgs/{ORG_SLUG}/patches/blob/{}", + git_sha256(content) + ))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(content.to_vec())) + .mount(mock) + .await; +} + +fn assert_fully_patched(pkg: &Path) { + assert_eq!(std::fs::read(pkg.join("index.js")).unwrap(), AFTER); + assert_eq!(std::fs::read(pkg.join("new.js")).unwrap(), CREATED); +} + +#[test] +fn offline_apply_with_only_a_diff_archive_reports_the_created_file_gap() { + let tmp = tempfile::tempdir().unwrap(); + let pkg = seed_project(tmp.path()); + seed_cached_diff_archive(tmp.path()); + + let (code, stdout, stderr) = run_cli(tmp.path(), &["apply", "--offline"], None); + + assert_eq!(code, 1, "stdout={stdout}\nstderr={stderr}"); + assert_eq!( + std::fs::read(pkg.join("index.js")).unwrap(), + BEFORE, + "a patch that cannot fully apply must not be applied partway" + ); + assert!(!pkg.join("new.js").exists()); + assert!( + stderr.contains("1 patch has no local source and --offline is set:") + && stderr.contains(PURL) + && stderr.contains("socket-patch repair"), + "the offline gate names the patch and the remedy; stderr={stderr}" + ); +} + +#[tokio::test] +async fn online_apply_with_a_cached_diff_archive_fetches_the_created_files_blob() { + let mock = MockServer::start().await; + mount_blob(&mock, CREATED).await; + + let tmp = tempfile::tempdir().unwrap(); + let pkg = seed_project(tmp.path()); + seed_cached_diff_archive(tmp.path()); + + let (code, stdout, stderr) = run_cli(tmp.path(), &["apply"], Some(&mock.uri())); + + assert_eq!(code, 0, "stdout={stdout}\nstderr={stderr}"); + assert_fully_patched(&pkg); + let requested: Vec = mock + .received_requests() + .await + .unwrap_or_default() + .iter() + .map(|r| r.url.path().to_string()) + .collect(); + assert!( + !requested + .iter() + .any(|p| p.contains("/patches/blob/") && p.ends_with(&git_sha256(AFTER))), + "the modified file's blob is not needed: its delta applies; requested={requested:?}" + ); +} + +#[tokio::test] +async fn default_repair_downloads_the_created_files_blob_for_offline_apply() { + let mock = MockServer::start().await; + Mock::given(method("GET")) + .and(path(format!("/v0/orgs/{ORG_SLUG}/patches/diff/{UUID}"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(diff_archive())) + .mount(&mock) + .await; + mount_blob(&mock, CREATED).await; + + let tmp = tempfile::tempdir().unwrap(); + let pkg = seed_project(tmp.path()); + + let (code, stdout, stderr) = run_cli(tmp.path(), &["repair"], Some(&mock.uri())); + assert_eq!(code, 0, "repair: stdout={stdout}\nstderr={stderr}"); + let blobs = tmp.path().join(".socket").join("blobs"); + assert!( + blobs.join(git_sha256(CREATED)).exists(), + "repair must cache the created file's blob; stdout={stdout}\nstderr={stderr}" + ); + assert!( + !blobs.join(git_sha256(AFTER)).exists(), + "the modified file's delta covers it; its blob is not downloaded" + ); + + let (code, stdout, stderr) = run_cli(tmp.path(), &["apply", "--offline"], None); + assert_eq!(code, 0, "apply: stdout={stdout}\nstderr={stderr}"); + assert_fully_patched(&pkg); +} + +#[test] +fn offline_repair_names_the_created_files_missing_blob() { + let tmp = tempfile::tempdir().unwrap(); + seed_project(tmp.path()); + seed_cached_diff_archive(tmp.path()); + + let (code, stdout, stderr) = run_cli(tmp.path(), &["repair", "--offline"], None); + + assert_eq!(code, 0, "stdout={stdout}\nstderr={stderr}"); + assert!( + stdout.contains("All diff archives are present locally."), + "stdout={stdout}" + ); + let short: String = git_sha256(CREATED).chars().take(12).collect(); + assert!( + stderr.contains("Warning: 1 blob is missing (offline mode - not downloading):") + && stderr.contains(&short), + "the created file's blob is still missing; stderr={stderr}" + ); +} + +#[tokio::test] +async fn repair_json_reports_the_created_blob_download_once_as_file_mode() { + let mock = MockServer::start().await; + mount_blob(&mock, CREATED).await; + let tmp = tempfile::tempdir().unwrap(); + seed_project(tmp.path()); + seed_cached_diff_archive(tmp.path()); + + let (code, stdout, stderr) = run_cli(tmp.path(), &["repair", "--json"], Some(&mock.uri())); + assert_eq!(code, 0, "stdout={stdout}\nstderr={stderr}"); + let v: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap(); + let downloads: Vec<&serde_json::Value> = v["events"] + .as_array() + .unwrap() + .iter() + .filter(|e| e["action"] == "downloaded") + .collect(); + assert_eq!(downloads.len(), 1, "{v:#}"); + assert_eq!(downloads[0]["details"]["mode"], "file", "{v:#}"); + assert_eq!(downloads[0]["details"]["count"], 1, "{v:#}"); +} diff --git a/crates/socket-patch-cli/tests/scan_vendor_e2e.rs b/crates/socket-patch-cli/tests/scan_vendor_e2e.rs index 486b9657..53bc0949 100644 --- a/crates/socket-patch-cli/tests/scan_vendor_e2e.rs +++ b/crates/socket-patch-cli/tests/scan_vendor_e2e.rs @@ -1408,6 +1408,143 @@ async fn vendor_auto_fetches_missing_package_from_lockfile() { ); } +/// A lockfile-only npm package the patch service serves prebuilt: the +/// backend reads the pristine tarball only if the service falls back to a +/// local build, so the registry download is deferred until then — here, +/// never — and no `vendor_fetched_missing` is reported for it. +#[tokio::test] +async fn vendor_auto_takes_a_missing_package_from_the_service_without_the_registry() { + let registry = MockServer::start().await; + let tgz = pristine_tgz(); + let integrity = sri_of(&tgz); + mount_registry_tarball(®istry, tgz).await; + + let prebuilt = { + let mut builder = tar::Builder::new(flate2::write::GzEncoder::new( + Vec::new(), + flate2::Compression::default(), + )); + for (path, bytes) in [ + ( + "package/package.json", + br#"{"name":"left-pad","version":"1.3.0"}"#.as_slice(), + ), + ("package/index.js", AFTER), + ] { + let mut header = tar::Header::new_gnu(); + header.set_size(bytes.len() as u64); + header.set_mode(0o644); + header.set_cksum(); + builder.append_data(&mut header, path, bytes).unwrap(); + } + builder.into_inner().unwrap().finish().unwrap() + }; + let api = MockServer::start().await; + let serve_path = format!("/patch/npm/left-pad/1.3.0/tok/{UUID}/left-pad-1.3.0.tgz"); + let serve_url = format!("{}{serve_path}", api.uri()); + Mock::given(method("POST")) + .and(path("/v0/orgs/acme/patches/package")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "results": { UUID: { + "status": "granted", "url": serve_url, "purl": PURL, + "artifacts": [{ "kind": "tarball", "url": serve_url, + "integrity": { "sha512": sri_of(&prebuilt) } }] + }} + }))) + .mount(&api) + .await; + Mock::given(method("GET")) + .and(path(serve_path)) + .respond_with(ResponseTemplate::new(200).set_body_bytes(prebuilt)) + .mount(&api) + .await; + + let tmp = tempfile::tempdir().unwrap(); + write_lockfile_only_fixture( + tmp.path(), + &format!("{}/left-pad/-/left-pad-1.3.0.tgz", registry.uri()), + &integrity, + ); + seed_manifest_and_blob(tmp.path()); + let vendor = |extra: &[&str]| { + let mut cmd = Command::new(binary()); + cmd.args([ + "vendor", + "--json", + "--vendor-source", + "auto", + "--api-url", + &api.uri(), + "--api-token", + "sktsec_placeholder_value_for_tests_api", + "--org", + "acme", + ]) + .args(extra) + .current_dir(tmp.path()); + for (key, _) in std::env::vars() { + if key.starts_with("SOCKET_") && key != "SOCKET_NO_CONFIG" { + cmd.env_remove(key); + } + } + let out = cmd.env("SOCKET_TELEMETRY_DISABLED", "1").output().unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + let v: serde_json::Value = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("{e}: {stdout}\n{}", String::from_utf8_lossy(&out.stderr))); + (out.status.code(), v) + }; + + // Offline, nothing is deferred to a service it cannot reach: the + // not-installed skip (exit 1, as before), and no request to either + // server. + let (code, v) = vendor(&["--offline"]); + assert_eq!(code, Some(1), "{v:#}"); + assert!( + v["events"] + .as_array() + .unwrap() + .iter() + .any(|e| e["purl"] == PURL + && e["action"] == "skipped" + && e["errorCode"] == "package_not_installed"), + "{v:#}" + ); + assert!(registry + .received_requests() + .await + .unwrap_or_default() + .is_empty()); + assert!(api.received_requests().await.unwrap_or_default().is_empty()); + + let (code, v) = vendor(&[]); + assert_eq!(code, Some(0), "{v:#}"); + let events = v["events"].as_array().unwrap(); + assert!( + events + .iter() + .any(|e| e["action"] == "applied" && e["purl"] == PURL), + "{v:#}" + ); + assert!( + !events + .iter() + .any(|e| e["errorCode"] == "vendor_fetched_missing"), + "no pristine fetch happened, so none is reported: {v:#}" + ); + assert!( + registry + .received_requests() + .await + .unwrap_or_default() + .is_empty(), + "the pristine tarball is never downloaded" + ); + assert!(tmp + .path() + .join(format!(".socket/vendor/npm/{UUID}/left-pad-1.3.0.tgz")) + .is_file()); +} + /// Integrity mismatch between the lock and the served bytes is a distinct /// vendor_fetch_failed failure — and nothing is written. #[tokio::test] diff --git a/crates/socket-patch-core/src/api/client.rs b/crates/socket-patch-core/src/api/client.rs index 5f716c02..cf2677fc 100644 --- a/crates/socket-patch-core/src/api/client.rs +++ b/crates/socket-patch-core/src/api/client.rs @@ -332,6 +332,21 @@ fn retry_after_secs(headers: &HeaderMap) -> Option { /// without a hint). type VendorAttemptError = (ApiError, Option>); +/// Most UUIDs the package-reference endpoint takes in one request. +pub(crate) const MAX_REFERENCE_BATCH: usize = 500; + +/// Why a pypi reference is refused before its download: the served +/// artifact is not a wheel. +pub(crate) const PYPI_NOT_A_WHEEL: &str = + "the prebuilt artifact is not a .whl (pypi vendoring is wheel-based)"; + +/// The last path segment of a serve URL, when it names a `.whl`. +pub(crate) fn wheel_filename_from_url(url: &str) -> Option { + let path = url.split(['?', '#']).next().unwrap_or(url); + let name = path.rsplit('/').next().unwrap_or(""); + name.ends_with(".whl").then(|| name.to_string()) +} + /// Body payload for the batch search POST endpoint. #[derive(Serialize)] struct BatchSearchBody { @@ -740,8 +755,9 @@ impl ApiClient { /// Resolve hosted-patch references for a set of published-patch UUIDs /// (hosted-mode `scan`, the default for a bare `scan`). Uses the authenticated /// `POST /v0/orgs/{org}/patches/package` when a token+org are set, else the - /// public proxy `POST /patch/package` (free patches only). Returns a - /// UUID → reference map (missing/404 → empty). + /// public proxy `POST /patch/package` (free patches only), in requests + /// of at most [`MAX_REFERENCE_BATCH`] UUIDs (the endpoint rejects more). + /// Returns a UUID → reference map (missing/404 → empty). /// /// Uses the client's configured org slug; see /// [`Self::fetch_registry_references_for_org`] for a per-call override. @@ -765,14 +781,18 @@ impl ApiClient { return Ok(std::collections::HashMap::new()); } let path = self.patches_path(org_slug, "package"); - let body = PackageVendorRequest { - uuids: uuids.to_vec(), - free_only: None, - }; - let resp = self - .post_json::(&path, &body) - .await?; - Ok(resp.map(|r| r.results).unwrap_or_default()) + let mut results = std::collections::HashMap::new(); + for chunk in uuids.chunks(MAX_REFERENCE_BATCH) { + let body = PackageVendorRequest { + uuids: chunk.to_vec(), + free_only: None, + }; + let resp = self + .post_json::(&path, &body) + .await?; + results.extend(resp.map(|r| r.results).unwrap_or_default()); + } + Ok(results) } /// Internal: POST the batch search to the public proxy's @@ -1306,6 +1326,16 @@ impl ApiClient { }, None => download_url.to_string(), }; + // pypi vendoring is wheel-based, so the sdist a qualifier-less pypi + // patch is served can never be used: refuse it before downloading. + if result + .purl + .as_deref() + .is_some_and(|purl| purl.starts_with("pkg:pypi/")) + && wheel_filename_from_url(&download_url).is_none() + { + return done(VendorServiceOutcome::Unavailable(PYPI_NOT_A_WHEEL.into())); + } // Surface the OTHER served artifacts (e.g. the gem path-source stub // gemspec) — their host-rewritten URL + normalized sha512 — so a @@ -1398,21 +1428,54 @@ impl ApiClient { /// Step 1 of [`Self::fetch_vendor_package`], retried per the client's /// [`VendorRetryPolicy`]. `Err` carries whether the final failure was a - /// retryable (availability) one. + /// retryable (availability) one. A uuid the attached plan names is + /// answered from the plan's one reference batch (see + /// [`VendorPrefetch::reference`]). async fn request_vendor_package( &self, uuid: &str, free_only: bool, vendor_url: Option<&str>, ) -> Result { + let plan = self + .vendor_prefetch + .lock() + .ok() + .and_then(|slot| slot.clone()); + if let Some(plan) = plan { + if let Some(result) = plan.reference(self, uuid, free_only, vendor_url).await { + return result; + } + } + let mut results = self + .request_vendor_references(&[uuid.to_string()], free_only, vendor_url) + .await?; + results.remove(uuid).ok_or_else(|| { + ( + ApiError::Other(format!("package response missing a result for {uuid}")), + false, + ) + }) + } + + /// One package-reference request for `uuids` (at most + /// [`MAX_REFERENCE_BATCH`]), retried per the client's + /// [`VendorRetryPolicy`]: the per-uuid results. `Err` carries whether + /// the final failure was a retryable (availability) one. + pub(crate) async fn request_vendor_references( + &self, + uuids: &[String], + free_only: bool, + vendor_url: Option<&str>, + ) -> Result, (ApiError, bool)> { let attempts = self.vendor_retry.attempts.max(1); let mut attempt = 1; loop { match self - .request_vendor_package_once(uuid, free_only, vendor_url) + .request_vendor_references_once(uuids, free_only, vendor_url) .await { - Ok(result) => return Ok(result), + Ok(results) => return Ok(results), Err((e, Some(retry_after))) if attempt < attempts => { debug_log(&format!( "vendor package request attempt {attempt} failed: {e}" @@ -1425,15 +1488,15 @@ impl ApiClient { } } - /// One package-reference POST: the single requested UUID's result. - async fn request_vendor_package_once( + /// One package-reference POST: the requested UUIDs' results. + async fn request_vendor_references_once( &self, - uuid: &str, + uuids: &[String], free_only: bool, vendor_url: Option<&str>, - ) -> Result { + ) -> Result, VendorAttemptError> { let body = PackageVendorRequest { - uuids: vec![uuid.to_string()], + uuids: uuids.to_vec(), // Only send freeOnly when forcing it (the public-proxy contract); // the authenticated endpoint defaults to false. free_only: free_only.then_some(true), @@ -1481,12 +1544,7 @@ impl ApiClient { hint, ) })?; - return parsed.results.get(uuid).cloned().ok_or_else(|| { - ( - ApiError::Other(format!("package response missing a result for {uuid}")), - None, - ) - }); + return Ok(parsed.results); } // 429 classifies as RateLimited but is still retried (the hint); // 401/403 carry no hint. @@ -4583,6 +4641,48 @@ mod vendor_package_tests { assert_eq!(map[UUID].status, "granted"); } + /// The endpoint rejects more than [`MAX_REFERENCE_BATCH`] uuids per + /// request (400), so a larger scan goes out in capped chunks whose + /// results are merged. + #[tokio::test] + async fn fetch_registry_references_chunks_at_the_endpoint_cap() { + struct EchoGranted; + impl wiremock::Respond for EchoGranted { + fn respond(&self, request: &Request) -> ResponseTemplate { + let body: serde_json::Value = serde_json::from_slice(&request.body).unwrap(); + let uuids = body["uuids"].as_array().unwrap(); + if uuids.len() > MAX_REFERENCE_BATCH { + return ResponseTemplate::new(400); + } + let results: serde_json::Map = uuids + .iter() + .map(|u| { + ( + u.as_str().unwrap().to_string(), + json!({ "status": "granted", "url": null, "artifacts": [] }), + ) + }) + .collect(); + ResponseTemplate::new(200).set_body_json(json!({ "results": results })) + } + } + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/patch/package")) + .respond_with(EchoGranted) + .expect(2) + .mount(&server) + .await; + let uuids: Vec = (0..=MAX_REFERENCE_BATCH) + .map(|i| format!("{i:08x}-0000-4000-8000-{i:012x}")) + .collect(); + let map = proxy_client(server.uri()) + .fetch_registry_references(&uuids) + .await + .expect("chunked resolution succeeds"); + assert_eq!(map.len(), uuids.len()); + } + /// The package-reference route honors a per-call org override: /// `Some(slug)` beats the client's configured `acme`, and the one-arg /// wrapper keeps using `acme`. diff --git a/crates/socket-patch-core/src/api/vendor_prefetch.rs b/crates/socket-patch-core/src/api/vendor_prefetch.rs index ff57163c..212fdd29 100644 --- a/crates/socket-patch-core/src/api/vendor_prefetch.rs +++ b/crates/socket-patch-core/src/api/vendor_prefetch.rs @@ -86,6 +86,17 @@ //! download, removed as soon as their package is passed over or the loop //! ends) and by the [`crate::vendor::prestage`] pool, not by size. //! +//! The package-reference half is batched: the plan's first call sends +//! one request naming the planned uuids from its own position on (see +//! [`VendorPrefetch::reference`]) in place of its own, and later calls +//! take their granted reference from it. That request grants the rest of +//! the plan up front, which the plan's exactness makes safe: a position +//! the loop passes over is granted only if the loop passes it after the +//! batch was sent. The bounds +//! above then limit the archive downloads. A package the batch reports +//! still building, or leaves out, makes its own request at its turn, as +//! before. +//! //! A planned download may name a secondary artifact (the gem stub //! gemspec) its backend fetches right after a verified archive; the task //! fetches it along with the archive, under the backend's own conditions, @@ -107,8 +118,10 @@ use futures_util::StreamExt; use super::client::{ hold_back_debug, ApiClient, HeldBack, PlannedDownload, PrefetchedSecondary, - VendorServiceOutcome, VENDOR_BREAKER_THRESHOLD, + VendorServiceOutcome, MAX_REFERENCE_BATCH, VENDOR_BREAKER_THRESHOLD, }; +use super::types::PackageVendorResult; +use crate::api::client::ApiError; use crate::vendor::lock_inventory::LockIntegrity; use crate::vendor::prestage::PrestageRecipe; use crate::vendor::registry_fetch::{artifact_matches_integrity, verify_go_h1}; @@ -153,6 +166,9 @@ pub(crate) struct VendorPrefetch { /// What the task is allowed to request, shared with it. look: Arc, state: tokio::sync::Mutex, + /// The plan's package references, resolved in one batch by the first + /// planned call (see [`Self::reference`]) and taken uuid by uuid. + references: tokio::sync::OnceCell>>, } /// The window of plan positions the task may request: `[at, at + reach)`, @@ -354,9 +370,86 @@ impl VendorPrefetch { window: window.max(1), look: Arc::new(Lookahead::new(byte_budget)), state: tokio::sync::Mutex::new(PrefetchState::default()), + references: tokio::sync::OnceCell::new(), } } + /// Step 1 (the package-reference request) for a planned `uuid`, or + /// `None` to make the live request (not planned, other parameters, or + /// the batch did not answer it). + /// + /// The first planned call sends ONE request naming the plan from its + /// own position on, its own uuid first, in place of its single-uuid request and with the + /// same retry ladder: the endpoint takes up to [`MAX_REFERENCE_BATCH`] + /// uuids, and one request per package paid a round trip and a quota + /// unit each. Its failure is that call's own failure, so an outage + /// costs what the serial loop paid, and every later call makes its + /// live request. A package still building (`pending_build`) is asked + /// again at its own turn, as the serial loop did, since it may be + /// ready by then; every other answer is final and is reused. + pub(crate) async fn reference( + &self, + client: &ApiClient, + uuid: &str, + free_only: bool, + vendor_url: Option<&str>, + ) -> Option> { + if free_only != self.free_only || vendor_url != self.vendor_url.as_deref() { + return None; + } + let at = self.planned.iter().position(|planned| planned == uuid)?; + let mut own = None; + let cache = self + .references + .get_or_init(|| async { + // Positions before this call's were passed over: the loop + // never asks for them, so they are never granted. + let mut order = vec![uuid.to_string()]; + for planned in &self.planned[at..] { + if !order.contains(planned) { + order.push(planned.clone()); + } + } + let mut kept = HashMap::new(); + for (i, chunk) in order.chunks(MAX_REFERENCE_BATCH).enumerate() { + match client + .request_vendor_references(chunk, free_only, vendor_url) + .await + { + Ok(mut results) => { + if i == 0 { + own = Some(results.remove(uuid).ok_or_else(|| { + ( + ApiError::Other(format!( + "package response missing a result for {uuid}" + )), + false, + ) + })); + } + kept.extend( + results + .into_iter() + .filter(|(_, r)| r.status != "pending_build"), + ); + } + Err(e) if i == 0 => { + own = Some(Err(e)); + break; + } + // The chunk's uuids make their live requests. + Err(_) => break, + } + } + std::sync::Mutex::new(kept) + }) + .await; + if own.is_some() { + return own; + } + cache.lock().ok()?.remove(uuid).map(Ok) + } + /// The prefetched outcome of the loop's call for `uuid`, or `None` to /// make the live requests. Plan positions before `uuid`'s are passed /// over (their outcomes discarded unreleased). @@ -1353,4 +1446,201 @@ mod tests { planned_requests.sort(); assert_eq!(planned_requests, serial_requests); } + + /// A service that answers every uuid a package-reference request names, + /// as the real endpoint does: the request's first uuid decides a + /// whole-request failure (503 / 403), and the others it cannot grant + /// are left out of the results. + struct BatchService { + base: String, + scripts: HashMap, + } + + impl wiremock::Respond for BatchService { + fn respond(&self, request: &wiremock::Request) -> ResponseTemplate { + let body: serde_json::Value = serde_json::from_slice(&request.body).unwrap(); + let uuids: Vec = body["uuids"] + .as_array() + .unwrap() + .iter() + .map(|u| u.as_str().unwrap().to_string()) + .collect(); + match self.scripts.get(&uuids[0]) { + Some(Script::Down) => return ResponseTemplate::new(503), + Some(Script::Forbidden) => return ResponseTemplate::new(403), + _ => {} + } + let mut results = serde_json::Map::new(); + for u in uuids { + let status = match self.scripts.get(&u) { + Some(Script::Granted(_)) => "granted", + Some(Script::Pending) => "pending_build", + Some(Script::NotFound) => "not_found", + _ => continue, + }; + let url = format!("{}/serve/{u}.tgz", self.base); + let sri = format!( + "sha512-{}", + base64::engine::general_purpose::STANDARD.encode(Sha512::digest(u.as_bytes())) + ); + let artifacts = if status == "granted" { + serde_json::json!([{ "kind": "tarball", "url": url, + "integrity": { "sha512": sri } }]) + } else { + serde_json::json!([]) + }; + results.insert( + u.clone(), + serde_json::json!({ "status": status, "url": url, "artifacts": artifacts }), + ); + } + ResponseTemplate::new(200).set_body_json(serde_json::json!({ "results": results })) + } + } + + async fn serve_batches(scripts: &[Script]) -> MockServer { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path(POST_PATH)) + .respond_with(BatchService { + base: server.uri(), + scripts: scripts + .iter() + .enumerate() + .map(|(i, s)| (uuid(i), *s)) + .collect(), + }) + .mount(&server) + .await; + for i in 0..scripts.len() { + let u = uuid(i); + Mock::given(method("GET")) + .and(path(format!("/serve/{u}.tgz"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(u.into_bytes())) + .mount(&server) + .await; + } + server + } + + /// The `(POSTs, GETs)` the server has seen. + async fn request_counts(server: &MockServer) -> (usize, usize) { + let log = request_log(server).await; + let posts = log.iter().filter(|r| r.starts_with("POST")).count(); + (posts, log.len() - posts) + } + + /// One package-reference request resolves the whole plan: the serial + /// loop's outcomes, one POST instead of one per package, and the same + /// downloads. + #[tokio::test] + async fn one_reference_request_serves_the_whole_plan() { + let scripts: Vec