diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index ff2c74d2a..bb48eac6f 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -167,6 +167,8 @@ The rewriter reads a fixed set of candidate files from the project root: the npm **Hosted sbt (v5.0, additive)**: an sbt build root (`project/build.properties` naming an `sbt.version`, 0.13.18 or later) is wired through ONE generated root file, `socket-patch.sbt` — no user file is edited. It pins every granted Maven patch build-wide (a `ThisBuild` `dependencyOverrides +=` of the Socket-only `-socket.` version plus a `file:` resolver over `.socket/sbt-hosted/maven2/`, moved ahead of the default repositories on sbt 0.13 / 1.x so an unreachable one never blocks it offline), downloads the pinned pom and jar there on the first sbt load (sha256-checked, gitignored by the file itself), and installs a load-time verifier that fails `update` when any project resolves another version or a pinned artifact whose bytes are not pinned. Edits: `redirect_sbt_pin` (added), `redirect_sbt_pin_updated` (an existing row replaced: same GA and base under a new uuid, or the same uuid with new served values; `original` names the previous uuid and version), `redirect_sbt_pin_rechecked` (an existing row re-verified after the build's dependencies changed: its dependency digest is recorded anew, `original`/`new` are `{deps}`). The load-time verifier also fails `update` when a project declares a pinned GA at a version newer than the pin's base (the build-wide override would otherwise force it back down). A new pin is gated on sbt's own resolution records under `target/` (never the machine-wide cache): run-level stops wire nothing, warn once and exit 0 — `redirect_sbt_no_resolution_evidence` (none; run `sbt update` first; always the in-memory engine's answer), `redirect_sbt_resolution_incomplete` (a declared project left no evidence, or the project definitions cannot be read statically), `redirect_sbt_resolution_stale` (a build source is newer than some project's evidence: each project is dated by its own newest record, so a partial `sbt /update` does not vouch for the others). Per-patch refusals (never confirmed): `redirect_sbt_missing_override` (no `maven2` override or no suffixed version), `redirect_sbt_integrity_missing` (jar or pom sha256 missing), `redirect_sbt_unsafe_value` (a value unsafe in a Scala literal, or an index URL not naming the uuid), `redirect_sbt_version_conflict` (some project resolves another version, or a build source declares the GA newer than the patch's base), `redirect_sbt_override_conflict` (two patches for one GA in a run, or another base already pinned), `redirect_sbt_vendored_conflict` (the GA is pinned by `socket-patch-vendor.sbt`, or that file cannot be parsed — then every Maven patch), `redirect_sbt_owned_file_modified` / `redirect_sbt_owned_file_foreign` (`socket-patch.sbt` edited, or not socket-patch's — every Maven patch), `redirect_sbt_owned_file_unreadable` (a whole-run refusal: `socket-patch.sbt` is on disk but cannot be read as UTF-8 text, so writing it would replace it; nothing is written), `redirect_sbt_unsupported_version`, `redirect_sbt_build_root_unknown` (sbt files but no versioned build root — every Maven patch), `redirect_sbt_overrides_assignment` / `redirect_sbt_resolvers_assignment` (a build source reassigns `dependencyOverrides` / `resolvers` with `:=`, `~=` or `--=`), `redirect_sbt_dependency_lock_present` (a `build.sbt.lock`), `redirect_sbt_scala_runtime_unsupported` (`org.scala-lang`), `redirect_sbt_classifier_unsupported`; a GA no library configuration resolves is skipped silently (`redirect_sbt_meta_build_only` when only the meta-build resolves it). Advisories: `redirect_sbt_version_untested` (sbt 2.1+, still wired), `redirect_sbt_override_build_repos` (`sbt.override.build.repos=true`), `redirect_maven_pom_ignored_sbt_build` (a `pom.xml` beside the sbt build, which sbt never reads; the Maven rewriter still edits it for the Maven build). A re-run keeps an existing row and re-checks it. When the build's dependency digest changed since the pin, evidence resolved after the change (fresh, newer than the generated file) re-verifies it and the row's digest is refreshed (`redirect_sbt_pin_rechecked`); the uuid is NOT confirmed on `redirect_sbt_pin_declared_newer` (a build source now declares the GA newer than the pin's base; the row stays, sbt's load-time verifier fails the build, and the remedy is `socket-patch rollback` or declaring the base again), `redirect_sbt_pin_unverifiable` (the digest changed and the evidence predates the change, or the digest cannot be computed: run `sbt update`, then re-run socket-patch), `redirect_sbt_override_shadowed` (the evidence still resolves the base version) or `redirect_sbt_resolved_elsewhere` (the pinned version resolves from outside the pin repository from a file whose sha256 is not the pinned jar's; a copy holding the pinned bytes, such as the Ivy cache a second checkout reads, is fine — at most 64 pinned artifact files of up to 256 MiB are hashed, anything else counts as elsewhere), and also when a build source now reassigns `dependencyOverrides` / `resolvers` or a `build.sbt.lock` appeared (the same `redirect_sbt_overrides_assignment` / `redirect_sbt_resolvers_assignment` / `redirect_sbt_dependency_lock_present` codes; the row stays and sbt's load-time verifier fails the build). For a pure sbt root (no `pom.xml` / Gradle script beside it), maven confirmation is decided only by the sbt rewriter's report; on a mixed root a uuid the sbt rewriter refused is still confirmed by the Maven rewriter's own `pom.xml` pin (the generated sbt files never prove a pin by substring). **Mill and scala-cli** are guidance only: per Maven patch `redirect_mill_manual_snippet` / `redirect_scala_cli_manual_snippet` carry a paste-able snippet (repository + forced suffixed version), nothing is written or confirmed, and a pure Mill / scala-cli root gets no `redirect_maven_no_pom`; there, a Maven patch the server sent without a `maven2` registry override gets `redirect_maven_missing_override` instead of a snippet (with a `pom.xml` beside the Mill / scala-cli files the pom rewriter reports it). `rollback` / `remove` restore `socket-patch.sbt` offline (the rows removed, the file deleted with its last pin; the gitignored downloads are left). Manifest-less VEX reads every strictly parsed pin as a hosted reference but grants it the lockfile basis only when the local evidence shows every recorded version of the GA is the pinned one and every recorded artifact hashes to a pinned sha256 (else `sbt_resolution_unverified`). +**Non-UTF-8 candidate files (#721)**: the rewriters edit UTF-8 text only. A candidate file that exists but is not UTF-8 (for example a UTF-16 `requirements.txt`, which is what Windows PowerShell 5.1's `pip freeze >` writes and which pip installs from) is never read as absent. When a candidate of its ecosystem could rewrite it, the run is refused with `candidate_file_unreadable` (exit 1, `--dry-run` included), the message names the file, and nothing is written; the remedy is to re-save the file as UTF-8. Two exceptions keep their own refusals. A Gradle build file the Gradle planner reaches gets that planner's per-build refusal (`redirect_gradle_build_file_unreadable`, exit 0) and the rest of the run goes ahead, and with no readable Gradle build a stray Gradle file (a lock, a nested script) is never rewritten, so it does not refuse the run, while a non-UTF-8 root `settings.gradle(.kts)` or `build.gradle(.kts)` still refuses it (it may be the build itself). An unreadable `socket-patch.sbt` is refused with `redirect_sbt_owned_file_unreadable`. A vendored→hosted takeover checks this before it reverts anything, so a refused run leaves the vendored wiring, ledger entry and artifact byte-identical. Vendored mode likewise refuses a non-UTF-8 `requirements.txt` or `-r` include by name (`pypi_no_requirements`) instead of wiring around it. Lock-only discovery reads `requirements.txt` and its in-root `-r` includes the way pip decodes them (a UTF-16 or UTF-32 byte-order mark selects that encoding), so such a project's pins are still found instead of reporting "No packages found". + **Gem stale-install guard (additive warning — the canonical narrative; other mentions point here)**: the gem hosted rewrite is pure Gemfile/lock text, so a gem ALREADY materialized under the project's bundle paths keeps its upstream bytes — the next `bundle install` prints `Using ` and never refetches, on **every** bundler major (live-verified 2026-08-19 on 1.17.3 / 2.7.2 / 4.0.18: bundler 4's CHECKSUMS verify at download time only, and nothing is downloaded; `bundle install --force`/`--redownload` re-install from the stale cached `.gem` instead of re-fetching — bundler 1 silently, bundler 4 with an exit-37 checksum refusal that still leaves the upstream bytes installed; the **verified** remedy is removing the installed dir + cache `.gem` + `specifications` entry, then `bundle install`). After the rewrite, a hosted run therefore probes the installed-gem discovery paths (the same ruby-crawler discovery `apply` uses, honoring `--global`/`--global-prefix` like scan's own discovery, plus — read-only — a `.bundle/config` bundle path refused as a write root because it resolves outside the project, which takes the project-local remedy; the `gem env` homes count only when Bundler uses system gems, i.e. no deployment store under `vendor/bundle`, and the first settings tier (app config, environment, global config) that sets `path`, `path.system` or `disable_shared_gems` doesn't set a non-empty `path` without `path.system: true` or `disable_shared_gems: false`, since with such a `path` `bundle install` fetches non-default gems into it and never reuses a system copy) for each confirmed gem redirect and judges the materialization against the patch record's `afterHash` file map. Judgment rules: records are found **by uuid** among this run's fetched records (v5.0: hosted mode persists no records, so a purl whose `/patches/view` fetch failed this run is not judged; the warning re-fires on every re-scan whose fetch succeeds, until the stale materialization is gone); a materialization with every file at `afterHash` is already patched and never warns (an agent→hosted migration stays quiet by construction), and when several confirmed variant purls resolve to one installed dir, ANY of them judging it patched keeps it quiet; staleness needs **positive evidence** — at least one record file whose bytes were actually read and hash to neither state's expectation — so missing or unreadable files never produce a warning. Warnings emit `redirect_gem_stale_install` (JSON `redirect.warnings[]` + a code-tagged stderr line) in three flavors: a PROJECT-LOCAL dir (under the project root, compared on absolute paths so the default `--cwd .` counts, or under the project's own refused `.bundle/config` path) gets the verified delete-list remedy (installed dir, cache `.gem`, `specifications` entry — plus the project's committed `/.gem` when present and not proven to be the patched artifact, since bundler installs from its cache dir in preference to fetching); a SHARED gem-env home gets a caveat that the home is shared machine-wide and prefers migrating the project to a local bundle path over deleting shared files; and a committed cache-dir archive whose sha256 differs from the patched artifact's warns standalone even with no installed dir at all (a fresh checkout with a committed stale cache re-materializes the upstream bytes forever). A stale-flagged purl is additionally **excluded from the same run's `--vex` `assume_applied` set** — the envelope must never attest a CVE its own warning says is live; the purl falls back to normal installed-tree verification (a patched install still attests, a stale one is omitted). The cache dir is bundler's `cache_path` setting (`Bundler.app_cache`), resolved in `Bundler::Settings` priority: `BUNDLE_CACHE_PATH:` in the bundler app config (`$BUNDLE_APP_CONFIG/config`, else `.bundle/config`) first, then the `BUNDLE_CACHE_PATH` environment variable, then `BUNDLE_CACHE_PATH:` in the global config (`bundle config set --global`: `$BUNDLE_CONFIG`, else `$BUNDLE_USER_CONFIG`, else `$BUNDLE_USER_HOME/config`, else `~/.bundle/config`), else `vendor/cache`; a relative value is read against the project root. The same global tier, below the app config and the environment, applies to `BUNDLE_GEMFILE:` and, for agent-mode install-root discovery, to `BUNDLE_PATH:`. A present local or environment `path`, `path.system`, or `disable_shared_gems` setting (including an empty string or false flag) shadows the global path tier, matching the tested Bundler 2.6/4 behavior; Bundler 1.x's legacy global-path shortcut is not modeled. An empty higher-tier `gemfile` setting also shadows the global value but leaves an existing nonempty `BUNDLE_GEMFILE` environment value in effect, or uses default manifest discovery when there is none. With `BUNDLE_IGNORE_CONFIG` set (any value) bundler reads no config file, so the app and global configs are skipped here too and only the environment and the default count — the same holds for the `BUNDLE_GEMFILE:` app-config setting. The probe is read-only (nothing is deleted) and skipped on `--dry-run` — deliberately explicit, since nothing was rewritten. Exit code and `status` are unchanged (warning-only, the hosted-refusal posture); a same-run `--vex` may still fail on "nothing to attest" per the embedded-VEX contract. **Pipenv hosted redirect (`Pipfile.lock`, pipfile-spec 6)**: every category other than `_meta` (`default`, `develop`, and Pipenv 2022+ named categories) that pins the package at the patched version is rewritten to the hosted reference — `{"file" | "path": "#sha256=", "hashes": ["sha256:"]}` with `markers`/`extras`/`index` kept exactly as Pipenv wrote them (present or absent: whether Pipenv records `index` depends on its release, the Pipfile spelling and the locking environment, so only the entry itself knows) and `version` dropped; `_meta` (the Pipfile content hash) and the Pipfile itself are never touched, so `pipenv install --deploy`/`sync`/`verify` keep passing. The reference KEY depends on the installing Pipenv: releases 7–11 only install `path` references, 2018 and later `file` ones (0–6 write pipfile-spec < 6 and are refused). The release is probed once per command with `pipenv --version`, resolved on ABSOLUTE `PATH` entries only (a relative entry would run a `pipenv` planted in the scanned repository; `.bat`/`.cmd` shims are found through `PATHEXT` on Windows), only when a pypi patch actually targets an entry of the lock, and `SOCKET_PIPENV_MAJOR=` pins the answer without spawning anything. An unknown installer selects `file` and warns `redirect_pipenv_installer_unknown` only when the lock was rewritten. **Refusal scope**: a pin/source CONFLICT (another version pinned, a foreign `file`/`path` source, a VCS/editable dependency) refuses the whole dependency atomically across categories as `redirect_pipenv_refused` AND vetoes the sibling Python rewriters (requirements.txt / uv.lock / pyproject) for that patch — the project's Pipenv install could not pick the patch up, so a half-redirected checkout is refused; anything else (no entry for the package, an old pipfile-spec, an unparseable lock, a digest-less patch) is `redirect_pipenv_skipped` and leaves the siblings alone (a stale Pipfile.lock in a uv/Poetry/requirements project must not block them). The veto applies to a LIVE lock only: a `Pipfile.lock` with no `Pipfile` beside it is abandoned, so its conflict refuses that file but never the siblings. Hash enforcement at install time is split by era — the `#sha256=` URL fragment is what Pipenv 2023+ verifies, the `hashes` list what 2018–2022 verify, Pipenv 11 either — so both are load-bearing. **Pipenv stale-install guard**: Pipenv never reinstalls a release that is already present (`pipenv install`, `install --deploy` and `sync` all exit 0 and keep the installed bytes — measured on 11.10.4, 2018.11.26 and 2026.8.0, hosted and vendored), so after the rewrite the run probes the Python crawler's site-packages (VIRTUAL_ENV, `./.venv`, `./venv`, Pipenv's out-of-tree `WORKON_HOME` venv; `--global`/`--global-prefix` honoured) for each confirmed Pipfile.lock redirect with the same rules as the gem guard (records by uuid from this run's fetch, PATCHED = `verify_patch_record` Ok, STALE needs positive evidence, read-only, skipped on `--dry-run`, stale purls excluded from the same-run `--vex` `assume_applied` set) and the Python stale-install guard (`redirect_pypi_stale_install`, see above) names the site-packages dir and the Pipenv-specific verified remedy: `pipenv run pip uninstall -y && pipenv sync` (or `pipenv --rm && pipenv sync`), with the `sync` arguments following the lock, since plain `pipenv sync` installs only `default`: the targeted form re-syncs the categories that pin the package (`--dev` for `develop`, `--categories ""` for a named category) and the `--rm` form re-syncs every non-empty category — NOT `pipenv uninstall`, which rewrites the Pipfile and re-locks the patch away. The vendored backend emits the twin `pypi_pipenv_stale_install` (`skipped` warning event). **Rollback** (v5.0, upstream restore): each hosted entry gets its registry shape back — `"version": "=="`, the entry's own `index` carried back unchanged (refused unless it — and the Pipfile's explicit `index`, if any — names a PyPI source in `_meta.sources`), and every release file's sha256 from PyPI's JSON API (`SOCKET_PYPI_JSON_API`), sorted by filename as Pipenv records them; an entry that pins another version beside the hosted reference is refused with the `git checkout` remedy (see "Hosted unwind coverage"). A Pipfile names no project, so a same-run `--vex` on a Pipenv project needs `--vex-product` (or a git remote) to detect a product purl. **Discovery**: `Pipfile.lock` is part of the lockfile inventory (every category's `==` pins, with the lock's digest set as `Sha256AnyOf` integrity so a lock-only checkout can be vendored by fetching the pure wheel through PyPI's JSON API — only when `_meta.sources` name the public index; a private-index lock stays discovery-only and never reaches pypi.org), and Socket's own hosted / vendored references stay discoverable as the package they replace, so a re-scan of an already-redirected or already-vendored lock-only checkout re-confirms it (`--vex` attests, vendored reports `already_vendored`) instead of finding nothing. diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index ed3af2307..571924be4 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -1952,6 +1952,26 @@ async fn vendored_takeover( .filter(|_| entry.is_some_and(vlt_entry)) }) }; + // NON-UTF-8 PRE-CHECK (#721) — the GUARD's undecodable-file rule + // (`engine::undecodable_guard`), checked BEFORE any revert dispatches + // (and under --dry-run too): a takeover that reverted first and was + // then refused by the guard would leave the reverted purls unpatched + // in both modes. + if takeover.iter().any(|(_, entry)| entry.is_some()) { + let view = socket_patch_core::vendor::lock_inventory::ProjectView::Disk(&common.cwd); + let read = socket_patch_core::hosted::engine::read_candidate_files( + &view, + &std::collections::BTreeSet::new(), + candidates, + ) + .await; + if let Some(refusal) = socket_patch_core::hosted::engine::undecodable_guard( + &read.undecodable_reads, + candidates, + ) { + return Err(refusal); + } + } // SYMLINK PRE-CHECK for the takeover reverts — the same rule as the // SYMLINK GUARD below, applied to each ledger entry's recorded wiring // (the revert backends also stage and rename over the file). Checked @@ -2682,7 +2702,7 @@ fn created_settings_over_existing( } /// [`created_settings_over_existing`]'s code for `socket-patch.sbt`. -const SBT_OWNED_FILE_UNREADABLE: &str = "redirect_sbt_owned_file_unreadable"; +use socket_patch_core::hosted::engine::SBT_OWNED_FILE_UNREADABLE; #[cfg(test)] mod tests { diff --git a/crates/socket-patch-cli/tests/in_process_get_hosted_ecosystems.rs b/crates/socket-patch-cli/tests/in_process_get_hosted_ecosystems.rs index 6ce32e653..fb02f79d3 100644 --- a/crates/socket-patch-cli/tests/in_process_get_hosted_ecosystems.rs +++ b/crates/socket-patch-cli/tests/in_process_get_hosted_ecosystems.rs @@ -315,6 +315,61 @@ async fn pypi_requirements_hosted_rewrites_pep440_equivalent_pin() { } } +/// #721: Windows PowerShell 5.1 writes `pip freeze > requirements.txt` as +/// UTF-16 with a BOM, and pip installs from it. The hosted grant must not +/// treat that file as absent and exit 0 with the project unpatched: it is +/// refused by name (`candidate_file_unreadable`, exit 1), nothing written. +#[tokio::test] +#[serial] +async fn pypi_requirements_hosted_refuses_a_utf16_file() { + const UUID: &str = "a1a1a1a1-a1a1-4a1a-8a1a-a1a1a1a1a1a3"; + const PURL: &str = "pkg:pypi/requests@2.31.0"; + const SHA256: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + let url = format!( + "http://patch.test/patch/pypi/requests/2.31.0/{TOKEN}/{UUID}/requests-2.31.0-py3-none-any.whl" + ); + + let text = "flask==2.0.1\r\nrequests==2.31.0\r\n"; + let le: Vec = [0xFF, 0xFE] + .into_iter() + .chain(text.encode_utf16().flat_map(u16::to_le_bytes)) + .collect(); + let be: Vec = [0xFE, 0xFF] + .into_iter() + .chain(text.encode_utf16().flat_map(u16::to_be_bytes)) + .collect(); + for (what, bytes) in [("utf-16le", le), ("utf-16be", be)] { + let server = MockServer::start().await; + mock_view(&server, UUID, PURL).await; + mock_reference( + &server, + UUID, + PURL, + &url, + serde_json::json!({ "sha256": SHA256 }), + serde_json::Value::Null, + ) + .await; + + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("requirements.txt"), &bytes).unwrap(); + + let code = + socket_patch_cli::commands::get::run(get_hosted_args(UUID, tmp.path(), server.uri())) + .await; + assert_eq!( + code, 1, + "{what}: a requirements.txt hosted mode cannot read must refuse, not exit 0 unpatched" + ); + assert_eq!( + std::fs::read(tmp.path().join("requirements.txt")).unwrap(), + bytes, + "{what}: the refused file must stay byte-identical" + ); + assert_no_manifest_no_blobs(tmp.path()); + } +} + // --------------------------------------------------------------------------- // maven — pom.xml fail-closed suffixed-version pin (rewrite_maven_pom) // --------------------------------------------------------------------------- diff --git a/crates/socket-patch-cli/tests/mode_migration_pypi.rs b/crates/socket-patch-cli/tests/mode_migration_pypi.rs index 4c0087c91..0283b1bf1 100644 --- a/crates/socket-patch-cli/tests/mode_migration_pypi.rs +++ b/crates/socket-patch-cli/tests/mode_migration_pypi.rs @@ -899,6 +899,64 @@ async fn uv_takeover_without_wheel_metadata_fails_loudly() { /// the revert (the artifact and ledger entry are kept). The takeover must /// then refuse — keeping the ledger — rather than drop the entry and leave /// the project half vendored with no record of it. +/// #721: a non-UTF-8 candidate file (here a UTF-16 `pip freeze` export +/// beside a vendored Poetry project) refuses the hosted run BEFORE the +/// takeover reverts anything, wet and `--dry-run` alike: refusing only at +/// the rewrite would leave the reverted poetry.lock unpatched in both modes. +#[tokio::test] +async fn undecodable_candidate_refuses_before_the_takeover_reverts() { + let (_tmp, root) = project(); + std::fs::write( + root.join("pyproject.toml"), + "[tool.poetry]\nname = \"demo\"\nversion = \"0.1.0\"\ndescription = \"\"\nauthors = [\"x \"]\npackage-mode = false\n\n[tool.poetry.dependencies]\npython = \">=3.9\"\nsix = \"1.16.0\"\n", + ) + .unwrap(); + std::fs::write( + root.join("poetry.lock"), + POETRY_LOCK + .replace("WHEEL_SHA", WHEEL_SHA) + .replace("SDIST_SHA", SDIST_SHA), + ) + .unwrap(); + vendor_project(&root, &["poetry.lock", "pyproject.toml"]); + let mut utf16 = vec![0xFF, 0xFE]; + for unit in "six==1.16.0\r\n".encode_utf16() { + utf16.extend(unit.to_le_bytes()); + } + std::fs::write(root.join("requirements.txt"), &utf16).unwrap(); + let lock = std::fs::read_to_string(root.join("poetry.lock")).unwrap(); + let state = root.join(".socket/vendor/state.json"); + + let server = MockServer::start().await; + mount_hosted_api(&server, true).await; + let uri = server.uri(); + for dry_run in [true, false] { + let mut args = hosted_scan_args(&uri); + if dry_run { + args.push("--dry-run"); + } + let (code, env) = run_cli(&root, &args, &[]); + assert_eq!(code, 1, "dry_run={dry_run}: {env:#}"); + let text = env.to_string(); + assert!( + text.contains("candidate_file_unreadable") && text.contains("requirements.txt"), + "dry_run={dry_run}: {env:#}" + ); + assert!( + !text.contains("redirect_takeover_reverted_vendored"), + "dry_run={dry_run}: nothing is reverted: {env:#}" + ); + assert_eq!( + std::fs::read_to_string(root.join("poetry.lock")).unwrap(), + lock, + "dry_run={dry_run}: the vendored lock is untouched" + ); + assert!(std::fs::read_to_string(&state).unwrap().contains(UUID)); + assert!(root.join(format!(".socket/vendor/pypi/{UUID}")).exists()); + assert_eq!(std::fs::read(root.join("requirements.txt")).unwrap(), utf16); + } +} + #[tokio::test] async fn drifted_vendored_line_refuses_takeover() { let (_tmp, root) = project(); diff --git a/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs b/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs index 5feeb532b..5eea83dbc 100644 --- a/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs +++ b/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs @@ -6,6 +6,8 @@ //! //! * #523: whitespace around `==` and the legacy `name (==X)` form; //! * #412: pins reached through in-root `-r` includes; +//! * #721: a UTF-16 file with a BOM (Windows PowerShell 5.1's +//! `pip freeze >` output), which pip decodes. //! * #994: include targets pip unquotes (`-r "dev reqs.txt"`, //! `--requirement="dev.txt"`, `-r dev\ reqs.txt`) or expands //! (`-r ${REQDIR}/dev.txt`). @@ -103,6 +105,19 @@ async fn assert_lock_only_discovers_with_env( files: &[(&str, &str)], envs: &[(&str, &str)], expected: &[&str], +) { + let files: Vec<(&str, &[u8])> = files.iter().map(|(r, c)| (*r, c.as_bytes())).collect(); + assert_lock_only_discovers_bytes_with_env(&files, envs, expected).await; +} + +async fn assert_lock_only_discovers_bytes(files: &[(&str, &[u8])], expected: &[&str]) { + assert_lock_only_discovers_bytes_with_env(files, &[], expected).await; +} + +async fn assert_lock_only_discovers_bytes_with_env( + files: &[(&str, &[u8])], + envs: &[(&str, &str)], + expected: &[&str], ) { for mode in [&[][..], &["--vendor"][..]] { let mock = MockServer::start().await; @@ -166,6 +181,32 @@ async fn lock_only_scan_discovers_included_pins() { .await; } +/// #721: pip decodes a requirements file by its BOM, so a UTF-16 file +/// (what Windows PowerShell 5.1's `pip freeze >` writes) is discovered, +/// in either byte order, instead of reading as "No packages found". +#[tokio::test] +async fn lock_only_scan_discovers_utf16_pins() { + let text = "sp-fixture-idna==3.7\r\nsp-fixture-six==1.16.0\r\n"; + let le: Vec = [0xFF, 0xFE] + .into_iter() + .chain(text.encode_utf16().flat_map(u16::to_le_bytes)) + .collect(); + let be: Vec = [0xFE, 0xFF] + .into_iter() + .chain(text.encode_utf16().flat_map(u16::to_be_bytes)) + .collect(); + for bytes in [le, be] { + assert_lock_only_discovers_bytes( + &[("requirements.txt", &bytes)], + &[ + "pkg:pypi/sp-fixture-idna@3.7", + "pkg:pypi/sp-fixture-six@1.16.0", + ], + ) + .await; + } +} + /// #994: pip `shlex`-splits an include line's options, so a quoted or /// backslash-escaped target names the file without its quotes, and a /// target with a space is one path, not two words. diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index f0c1440b7..dcb693a59 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -70,7 +70,9 @@ pub const SYMLINK_REFUSAL: &str = "redirect_symlinked_file_unsupported"; /// Refusal code for a candidate file that exists but whose content the /// in-memory host did not provide (oversize, an LFS pointer, -/// presence-only); disk would read and rewrite it. +/// presence-only), and, on disk and in memory alike, for one that is not +/// UTF-8 text (#721): no rewriter can edit it, and reading it as absent +/// would leave its pins unpatched behind an exit-0 run. pub const UNREADABLE_REFUSAL: &str = "candidate_file_unreadable"; /// Rush's repo-state file, whose `pnpmShrinkwrapHash` a lock edit @@ -156,6 +158,33 @@ fn unreadable_refusal(rel: &str) -> Refusal { } } +/// The refusal for an unreadable `socket-patch.sbt`, the file the sbt +/// planner owns and would otherwise create over the user's bytes. +pub const SBT_OWNED_FILE_UNREADABLE: &str = "redirect_sbt_owned_file_unreadable"; + +fn undecodable_refusal(rel: &str) -> Refusal { + // Keeps the sbt planner's own refusal code, and still refuses here, + // before any vendored->hosted takeover revert. + if rel == crate::formats::sbt::owned_file::HOSTED_FILE { + return Refusal { + code: SBT_OWNED_FILE_UNREADABLE.to_string(), + message: format!( + "{rel} is not UTF-8 text, so the hosted sbt wiring would replace it; \ + re-save it as UTF-8 and re-run; nothing was written" + ), + }; + } + Refusal { + code: UNREADABLE_REFUSAL.to_string(), + message: format!( + "{rel} is not UTF-8 text (for example UTF-16, which Windows PowerShell 5.1 \ + writes for `pip freeze > requirements.txt`), so it cannot be rewritten \ + alongside the other lockfiles; re-save it as UTF-8 and re-run; nothing was \ + written" + ), + } +} + /// Reference grants → candidates. A selection without a usable grant is /// recorded in `skipped` (`not_found`, the reference status, `bad_purl`, /// `no_url`). @@ -304,6 +333,11 @@ pub struct CandidateFiles { /// project whose candidates could rewrite (or whose rewrite depends on) /// one is refused, since the rewriters would treat it as absent. pub unreadable_reads: Vec, + /// Candidate files that exist but are not UTF-8 text (a UTF-16 + /// requirements.txt pip reads, #721), on disk and in memory alike. They + /// are left out of `files`; a project whose candidates could rewrite + /// one is refused rather than read as if the file were absent. + pub undecodable_reads: Vec, /// Gradle build files the script graph reached that exist but cannot be /// read as text (any view): the hosted Gradle planner refuses the build /// instead of taking them for absent (and creating a settings file over @@ -330,7 +364,14 @@ impl CandidateFiles { // (non-blocking open + fstat regular-file check), so a FIFO // under a candidate name is skipped like a missing file instead // of wedging the run in open(2). - ProjectView::Disk(_) | ProjectView::Snapshot(_) => view.read_text(rel).await.ok(), + ProjectView::Disk(_) | ProjectView::Snapshot(_) => match view.read_text(rel).await { + Ok(text) => Some(text), + Err(e) if e.kind() == std::io::ErrorKind::InvalidData => { + self.undecodable_reads.push(rel.to_string()); + None + } + Err(_) => None, + }, ProjectView::Memory(project) => { if project.is_symlink(rel) { self.symlinked_reads.push(rel.to_string()); @@ -340,13 +381,17 @@ impl CandidateFiles { self.unreadable_reads.push(rel.to_string()); return false; } - // Disk reads any UTF-8 regular file; a non-UTF-8 one is - // absent to it as well. + // Disk reads any UTF-8 regular file and records a non-UTF-8 + // one as undecodable; so does memory. match project.get(rel) { Some(MemoryEntry::Text(text)) => Some(text.to_string()), - Some(MemoryEntry::Binary(bytes)) => { - std::str::from_utf8(bytes).ok().map(str::to_string) - } + Some(MemoryEntry::Binary(bytes)) => match std::str::from_utf8(bytes) { + Ok(text) => Some(text.to_string()), + Err(_) => { + self.undecodable_reads.push(rel.to_string()); + None + } + }, _ => None, } } @@ -588,6 +633,22 @@ pub async fn read_candidate_files( && crate::patch::redirect::gradle::gradle_build_present(&out.files) { read_gradle_files(view, unreadable, &mut out).await; + // The Gradle planner refuses a build over a file it cannot read as + // text and the scan carries on, so such a file is not a reason to + // refuse the whole run (#721). + let gradle_unreadable = &out.gradle_unreadable; + out.undecodable_reads + .retain(|rel| !gradle_unreadable.contains(rel)); + } else { + // No readable Gradle build: the Gradle planner never runs, so a + // stray Gradle file it would own (a lock, a nested script) is never + // rewritten and must not refuse the rest of the run. A root build + // or settings script still refuses: it may be the build itself, + // unreadable, which the planner would otherwise skip silently. + out.undecodable_reads.retain(|rel| { + !is_gradle_owned_file(rel) + || crate::patch::redirect::gradle::GRADLE_ROOT_FILES.contains(&rel.as_str()) + }); } // An sbt build's resolution evidence rides a synthetic key (see // `patch::redirect::sbt::SBT_RESOLUTION_KEY`). @@ -605,6 +666,8 @@ pub async fn read_candidate_files( out.symlinked_reads.dedup(); out.unreadable_reads.sort(); out.unreadable_reads.dedup(); + out.undecodable_reads.sort(); + out.undecodable_reads.dedup(); out } @@ -768,6 +831,7 @@ async fn keep_bundler_loaded_gem_files( out.files.retain(|rel, _| !dropped(rel)); out.symlinked_reads.retain(|rel| !dropped(rel)); out.unreadable_reads.retain(|rel| !dropped(rel)); + out.undecodable_reads.retain(|rel| !dropped(rel)); out.gem_refusal = refusal; } @@ -933,6 +997,7 @@ pub struct Rewritten { pub files: BTreeMap, pub symlinked_reads: Vec, pub unreadable_reads: Vec, + pub undecodable_reads: Vec, /// The rewriters' override slice (the candidates' deps). pub overrides: Vec, pub rewrite: RewriteResult, @@ -1082,6 +1147,7 @@ pub async fn rewrite( rush_lock_keys, symlinked_reads, unreadable_reads, + undecodable_reads, gradle_unreadable, gem_refusal, } = read; @@ -1258,6 +1324,7 @@ pub async fn rewrite( files, symlinked_reads, unreadable_reads, + undecodable_reads, overrides, rewrite, rewritten, @@ -1904,6 +1971,42 @@ fn file_ecosystem(rel: &str) -> Option<&'static str> { .then_some("pypi") } +/// A file only the hosted Gradle planner reads or writes: a settings or +/// build script, a dependency lock, the verification metadata, the wrapper +/// properties, or the planner's own owned files. +fn is_gradle_owned_file(rel: &str) -> bool { + let base = rel.rsplit('/').next().unwrap_or(rel); + base.ends_with(".gradle") + || base.ends_with(".gradle.kts") + || base.ends_with(".lockfile") + || rel == "gradle/verification-metadata.xml" + || rel == "gradle/wrapper/gradle-wrapper.properties" + || rel.starts_with(".socket/gradle/") +} + +/// The [`guard`]'s non-UTF-8 rule on its own (#721): the first of +/// `undecodable` (a [`CandidateFiles::undecodable_reads`]) whose ecosystem +/// has a candidate refuses the run. The vendored→hosted takeover runs it +/// before reverting anything, so a refusal never strands a reverted purl. +pub fn undecodable_guard(undecodable: &[String], candidates: &[Candidate]) -> Option { + undecodable + .iter() + .find(|rel| { + // The root manifest is read strictly only as a yarn berry + // rewrite target (its `resolutions`); advisory reads never + // record it, so here it is always an npm rewrite target. A root + // Gradle script left here (no readable build beside it) may be + // the build itself, which only maven candidates could patch. + let eco = file_ecosystem(rel) + .or((rel.as_str() == "package.json").then_some("npm")) + .or(crate::patch::redirect::gradle::GRADLE_ROOT_FILES + .contains(&rel.as_str()) + .then_some("maven")); + eco.is_some_and(|eco| candidates.iter().any(|c| c.dep.ecosystem == eco)) + }) + .map(|rel| undecodable_refusal(rel)) +} + /// SYMLINK GUARD — fail-closed, whole rewrite, before any write (hosted /// rewrites are transactional). The writer stages next to /// the path and renames over it, which REPLACES a symbolic link with a @@ -1911,6 +2014,9 @@ fn file_ecosystem(rel: &str) -> Option<&'static str> { /// bytes but never the link. Applies to every ecosystem's files and to dry /// runs, so a dry run predicts the refusal. /// +/// On disk and in memory: a candidate file that is not UTF-8 text, when a +/// candidate of its ecosystem could rewrite it (#721). +/// /// In memory, additionally: a candidate file read through a link (its bytes /// are unknown) or present without content, when a candidate of its /// ecosystem could rewrite it. @@ -1932,13 +2038,16 @@ pub fn guard( if let Some(linked) = written().find(|k| view.is_symlink(k)) { return Some(symlink_refusal(linked)); } - let ProjectView::Memory(project) = view else { - return None; - }; + if let Some(refusal) = undecodable_guard(&done.undecodable_reads, candidates) { + return Some(refusal); + } let candidate_ecosystems: BTreeSet<&str> = candidates .iter() .map(|c| c.dep.ecosystem.as_str()) .collect(); + let ProjectView::Memory(project) = view else { + return None; + }; if let Some(linked) = done .symlinked_reads .iter() @@ -2096,6 +2205,145 @@ mod tests { assert!(read.unreadable_reads.is_empty()); } + /// #721: a candidate file that is not UTF-8 (a UTF-16 requirements.txt, + /// which pip reads) is refused by name, on disk and in memory alike, + /// when a candidate of its ecosystem could rewrite it, instead of being + /// treated as absent (exit 0, nothing pinned, no diagnostic). + #[tokio::test] + async fn an_undecodable_candidate_file_refuses_its_ecosystem() { + let purl = "pkg:pypi/six@1.16.0"; + let uuid = "u-721"; + let mut refs = HashMap::new(); + refs.insert( + uuid.to_string(), + reference(serde_json::json!({ + "status": "granted", + "url": format!("https://patch.example/patch/pypi/six/1.16.0/tok/{uuid}/six-1.16.0-py2.py3-none-any.whl"), + "purl": purl, + "artifacts": [{"kind": "tarball", "url": null, "integrity": {"sha256": "ab"}}], + "registryOverride": null + })), + ); + let selected = vec![(purl.to_string(), uuid.to_string())]; + let mut skipped = Vec::new(); + let candidates = build_candidates(&selected, &refs, &mut skipped); + assert_eq!(candidates.len(), 1, "{skipped:?}"); + let utf16: Vec = [0xFF, 0xFE] + .into_iter() + .chain( + "idna==3.7\r\nsix==1.16.0\r\n" + .encode_utf16() + .flat_map(u16::to_le_bytes), + ) + .collect(); + let outer = OuterAllowRemote::default; + let options = || RewriteOptions { + dry_run: false, + targets_pipenv_lock: false, + pipenv_major: None, + pipenv_unknown_detail: String::new(), + trust_lockfile_config: true, + npm_allow_remote_config: true, + npm_outer: &outer, + blocking: false, + }; + + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("requirements.txt"), &utf16).unwrap(); + let mut memory = MemoryProject::new(); + memory.insert( + "requirements.txt", + MemoryEntry::Binary(utf16.clone().into()), + ); + for view in [ProjectView::Disk(tmp.path()), ProjectView::Memory(&memory)] { + let read = read_candidate_files(&view, &BTreeSet::new(), &candidates).await; + assert_eq!(read.undecodable_reads, vec!["requirements.txt"]); + let done = rewrite( + &view, + read, + &candidates, + BTreeMap::new(), + &BTreeSet::new(), + &[], + options(), + ) + .await; + let refusal = guard(&view, &done, &candidates).expect("refused"); + assert_eq!(refusal.code, UNREADABLE_REFUSAL); + assert!( + refusal.message.contains("requirements.txt") && refusal.message.contains("UTF-8"), + "{}", + refusal.message + ); + + // Another ecosystem's run is not blocked by it. + let (cargo_selected, cargo_refs) = cargo_reference("u-2"); + let cargo = build_candidates(&cargo_selected, &cargo_refs, &mut Vec::new()); + let read = read_candidate_files(&view, &BTreeSet::new(), &cargo).await; + let done = rewrite( + &view, + read, + &cargo, + BTreeMap::new(), + &BTreeSet::new(), + &[], + options(), + ) + .await; + assert!(guard(&view, &done, &cargo).is_none()); + } + } + + /// #721 review: beside a yarn berry lock the root `package.json` is a + /// rewrite target (its `resolutions`), so a non-UTF-8 one refuses the + /// run instead of being taken for absent. Beside an npm lock it is + /// advisory only and never refuses. + #[tokio::test] + async fn a_non_utf8_berry_manifest_refuses_the_npm_run() { + use crate::patch::redirect::Integrity; + let candidates = vec![Candidate { + purl: "pkg:npm/left-pad@1.3.0".into(), + dep: DepOverride { + ecosystem: "npm".into(), + name: "left-pad".into(), + namespace: None, + version: "1.3.0".into(), + token: "tok".into(), + patch_uuid: "uuid".into(), + artifact_url: + "https://patch.socket.dev/patch/npm/left-pad/1.3.0/tok/uuid/left-pad-1.3.0.tgz" + .into(), + registry_override: None, + integrity: Integrity::default(), + }, + }]; + let berry = "__metadata:\n version: 8\n cacheKey: 10c0\n\n\"left-pad@npm:^1.3.0\":\n \ + version: 1.3.0\n resolution: \"left-pad@npm:1.3.0\"\n"; + let latin1: &[u8] = b"{\"name\": \"Andr\xe9\"}\n"; + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("yarn.lock"), berry).unwrap(); + std::fs::write(tmp.path().join("package.json"), latin1).unwrap(); + let view = ProjectView::Disk(tmp.path()); + let read = read_candidate_files(&view, &BTreeSet::new(), &candidates).await; + assert_eq!(read.undecodable_reads, vec!["package.json"]); + let refusal = undecodable_guard(&read.undecodable_reads, &candidates).expect("refused"); + assert_eq!(refusal.code, UNREADABLE_REFUSAL); + + // Beside an npm lock the manifest is advisory: never refused. + std::fs::remove_file(tmp.path().join("yarn.lock")).unwrap(); + std::fs::write( + tmp.path().join("package-lock.json"), + "{\"lockfileVersion\": 3, \"packages\": {}}\n", + ) + .unwrap(); + let read = read_candidate_files(&view, &BTreeSet::new(), &candidates).await; + assert!( + read.undecodable_reads.is_empty(), + "{:?}", + read.undecodable_reads + ); + } + /// A hosted URL left in a berry project's `package.json` `resolutions` /// while `yarn.lock` still resolves the registry entry confirms nothing: /// only the lock pin installs (#404). @@ -2650,6 +2898,14 @@ mod tests { "{:?}", read.gradle_unreadable ); + // Only the Gradle build is refused, not the whole run (#721). + assert!( + !read + .undecodable_reads + .contains(&"settings.gradle".to_string()), + "{:?}", + read.undecodable_reads + ); assert!(done.rewrite.refused_gradle_uuids.contains(GRADLE_UUID)); assert!( !done.rewrite.files.contains_key("settings.gradle"), @@ -2711,6 +2967,78 @@ mod tests { assert!(text.contains("include 'core'"), "{text}"); } + /// #721 review: with no readable Gradle build the Gradle planner never + /// runs, so a stray non-UTF-8 Gradle lock does not refuse the rest of a + /// Maven run. A non-UTF-8 root build or settings script does: it may be + /// the whole build, unreadable, which would otherwise be skipped + /// silently (a Gradle-only project exiting 0 unpatched). + #[tokio::test] + async fn non_utf8_gradle_files_without_a_readable_build() { + const POM: &str = "com.socketfixturevictim1.10.0\n"; + let latin1: &[u8] = b"rootProject.name = 'Andr\xe9'\n"; + let candidates = vec![gradle_candidate()]; + + // A stray lock beside a pom.xml: not refused. + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("pom.xml"), POM).unwrap(); + std::fs::write(tmp.path().join("gradle.lockfile"), latin1).unwrap(); + let mut memory = MemoryProject::new(); + memory.insert_text("pom.xml", POM); + memory.insert( + "gradle.lockfile", + MemoryEntry::Binary(latin1.to_vec().into()), + ); + for view in [ProjectView::Disk(tmp.path()), ProjectView::Memory(&memory)] { + let (read, done) = gradle_rewrite_in(&view).await; + assert!( + read.undecodable_reads.is_empty(), + "{:?}", + read.undecodable_reads + ); + assert!(guard(&view, &done, &candidates).is_none()); + } + + // A Gradle-only project whose root scripts are all non-UTF-8: + // refused, never skipped. + for root in ["settings.gradle", "build.gradle", "build.gradle.kts"] { + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join(root), latin1).unwrap(); + let mut memory = MemoryProject::new(); + memory.insert(root, MemoryEntry::Binary(latin1.to_vec().into())); + for view in [ProjectView::Disk(tmp.path()), ProjectView::Memory(&memory)] { + let (read, done) = gradle_rewrite_in(&view).await; + assert_eq!(read.undecodable_reads, vec![root.to_string()]); + let refusal = guard(&view, &done, &candidates).expect("refused"); + assert_eq!(refusal.code, UNREADABLE_REFUSAL); + } + } + } + + /// #721 review: an unreadable `socket-patch.sbt` is refused by the + /// run-wide check itself, which also runs before any vendored->hosted + /// takeover revert, and keeps the sbt planner's own refusal code. + #[tokio::test] + async fn an_unreadable_sbt_owned_file_refuses_early_with_the_sbt_code() { + let latin1: &[u8] = b"// Auteur: Andr\xe9\n"; + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("socket-patch.sbt"), latin1).unwrap(); + let candidates = vec![gradle_candidate()]; + let read = read_candidate_files( + &ProjectView::Disk(tmp.path()), + &BTreeSet::new(), + &candidates, + ) + .await; + assert_eq!(read.undecodable_reads, vec!["socket-patch.sbt"]); + let refusal = undecodable_guard(&read.undecodable_reads, &candidates).expect("refused"); + assert_eq!(refusal.code, SBT_OWNED_FILE_UNREADABLE); + assert!( + refusal.message.contains("socket-patch.sbt"), + "{}", + refusal.message + ); + } + /// A refused Gradle build is never confirmed by a snippet pasted into a /// build script, though it names the suffixed version and the index url. #[tokio::test] diff --git a/crates/socket-patch-core/src/hosted/memory/mod.rs b/crates/socket-patch-core/src/hosted/memory/mod.rs index 06d814f9e..cac4967b6 100644 --- a/crates/socket-patch-core/src/hosted/memory/mod.rs +++ b/crates/socket-patch-core/src/hosted/memory/mod.rs @@ -1371,6 +1371,7 @@ mod tests { files: BTreeMap::new(), symlinked_reads: Vec::new(), unreadable_reads: Vec::new(), + undecodable_reads: Vec::new(), overrides: Vec::new(), rewrite, rewritten: files.iter().map(|(rel, _)| (*rel).to_string()).collect(), diff --git a/crates/socket-patch-core/src/utils/requirements.rs b/crates/socket-patch-core/src/utils/requirements.rs index 650442c48..1b8229e1b 100644 --- a/crates/socket-patch-core/src/utils/requirements.rs +++ b/crates/socket-patch-core/src/utils/requirements.rs @@ -14,6 +14,42 @@ //! `--hash=sha256:ab#cd` are data. Exactly one leading BOM is encoding, not //! data (pip decodes with utf-8-sig; uv strips it too). +/// Decode a requirements file the way pip's `auto_decode` does: a UTF-16 +/// or UTF-32 byte-order mark selects that encoding and is dropped; anything +/// else is UTF-8, its one leading BOM kept for [`logical_lines`] to drop. +/// Windows PowerShell 5.1 writes `pip freeze > requirements.txt` as UTF-16 +/// LE with a BOM, and pip installs from it (#721). pip tries the UTF-16 +/// marks first, so a UTF-32 LE mark (`FF FE 00 00`) reads as UTF-16 LE, as +/// it does for pip. `None` when the bytes are not valid in that encoding. +/// (pip's last resort, the locale's encoding for a mark-less non-UTF-8 +/// file, is machine-dependent and not modelled.) +pub(crate) fn decode(bytes: &[u8]) -> Option { + fn utf16(body: &[u8], unit: fn([u8; 2]) -> u16) -> Option { + if !body.len().is_multiple_of(2) { + return None; + } + char::decode_utf16(body.chunks_exact(2).map(|c| unit([c[0], c[1]]))) + .collect::>() + .ok() + } + if let Some(body) = bytes.strip_prefix(&[0xFF, 0xFE]) { + return utf16(body, u16::from_le_bytes); + } + if let Some(body) = bytes.strip_prefix(&[0xFE, 0xFF]) { + return utf16(body, u16::from_be_bytes); + } + if let Some(body) = bytes.strip_prefix(&[0x00, 0x00, 0xFE, 0xFF]) { + if !body.len().is_multiple_of(4) { + return None; + } + return body + .chunks_exact(4) + .map(|c| char::from_u32(u32::from_be_bytes([c[0], c[1], c[2], c[3]]))) + .collect(); + } + String::from_utf8(bytes.to_vec()).ok() +} + /// One logical requirements line. pub(crate) struct LogicalLine { /// 0-based index of the first physical line. @@ -329,6 +365,34 @@ pub(crate) fn url_sha256_fragment(location: &str) -> Option { mod tests { use super::*; + /// #721: pip's `auto_decode` BOM table, in pip's order. + #[test] + fn decode_follows_pips_byte_order_marks() { + let text = "six==1.16.0\r\n"; + let le: Vec = text.encode_utf16().flat_map(u16::to_le_bytes).collect(); + let be: Vec = text.encode_utf16().flat_map(u16::to_be_bytes).collect(); + let be32: Vec = text + .chars() + .flat_map(|c| (c as u32).to_be_bytes()) + .collect(); + let with = |bom: &[u8], body: &[u8]| [bom, body].concat(); + assert_eq!(decode(text.as_bytes()).as_deref(), Some(text)); + // The UTF-8 mark is left for `logical_lines`. + let bom8 = with(&[0xEF, 0xBB, 0xBF], text.as_bytes()); + assert_eq!(decode(&bom8).as_deref(), Some("\u{feff}six==1.16.0\r\n")); + assert_eq!(decode(&with(&[0xFF, 0xFE], &le)).as_deref(), Some(text)); + assert_eq!(decode(&with(&[0xFE, 0xFF], &be)).as_deref(), Some(text)); + assert_eq!( + decode(&with(&[0x00, 0x00, 0xFE, 0xFF], &be32)).as_deref(), + Some(text) + ); + // Not valid in the encoding the mark selects (or mark-less and not + // UTF-8): unreadable, never guessed. + assert_eq!(decode(&with(&[0xFF, 0xFE], &le[1..])), None); + assert_eq!(decode(&with(&[0xFF, 0xFE], &[0x00, 0xD8])), None); + assert_eq!(decode(&[b's', 0xC3, 0x28]), None); + } + #[test] fn requires_hashes_reads_pip_hash_checking_mode() { for hashed in [ diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs index b1d154c9a..d21b6a87b 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs @@ -724,11 +724,17 @@ async fn inventory_requirements_txt(view: &ProjectView<'_>) -> Option) -> Option> { use crate::vendor::pypi_requirements::{is_in_root_rel, requirements_includes}; const ROOT: &str = "requirements.txt"; - let root = view.read_text(ROOT).await.ok()?; + let read = |rel: String| async move { + let bytes = view.read_bytes(&rel).await.ok()?; + crate::utils::requirements::decode(&bytes) + }; + let root = read(ROOT.to_string()).await?; let mut visited = std::collections::HashSet::from([ROOT.to_string()]); let mut stack: Vec = requirements_includes(ROOT, &root); stack.reverse(); @@ -737,7 +743,7 @@ async fn requirements_tree(view: &ProjectView<'_>) -> Option> { if !is_in_root_rel(&rel) || !visited.insert(rel.clone()) { continue; } - let Ok(text) = view.read_text(&rel).await else { + let Some(text) = read(rel.clone()).await else { continue; }; let mut includes = requirements_includes(&rel, &text); diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs index cef50b43a..6322aad77 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -3410,6 +3410,56 @@ async fn requirements_in_root_includes_are_inventoried() { assert_eq!(sorted_pairs(&in_memory), sorted_pairs(&entries)); } +/// #721: pip decodes a requirements file by its BOM, so a UTF-16 root +/// file and a UTF-16 include (what Windows PowerShell 5.1's `pip freeze >` +/// writes) are inventoried like their UTF-8 text, on disk and in memory. +#[tokio::test] +async fn requirements_utf16_files_are_inventoried() { + fn utf16(text: &str, le: bool) -> Vec { + let mut out = if le { + vec![0xFF, 0xFE] + } else { + vec![0xFE, 0xFF] + }; + for unit in text.encode_utf16() { + out.extend(if le { + unit.to_le_bytes() + } else { + unit.to_be_bytes() + }); + } + out + } + for le in [true, false] { + let root_bytes = utf16("-r requirements/base.txt\r\nidna==3.7\r\n", le); + let base_bytes = utf16("six==1.16.0\r\n", !le); + let tmp = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(tmp.path().join("requirements")).unwrap(); + std::fs::write(tmp.path().join("requirements.txt"), &root_bytes).unwrap(); + std::fs::write(tmp.path().join("requirements/base.txt"), &base_bytes).unwrap(); + let entries = inventory_pypi_locks(tmp.path()).await.unwrap(); + assert_eq!( + sorted_pairs(&entries), + vec![ + ("idna".to_string(), "3.7".to_string()), + ("six".to_string(), "1.16.0".to_string()), + ], + "le={le}: {entries:?}" + ); + + let mut project = MemoryProject::new(); + project.insert("requirements.txt", MemoryEntry::Binary(root_bytes.into())); + project.insert( + "requirements/base.txt", + MemoryEntry::Binary(base_bytes.into()), + ); + let in_memory = super::pypi::inventory_pypi_locks_in(&ProjectView::Memory(&project)) + .await + .unwrap(); + assert_eq!(sorted_pairs(&in_memory), sorted_pairs(&entries)); + } +} + /// pip applies an index option from ANY file of the tree globally, so an /// `--index-url` inside an include keeps the root file's hashed pins /// unverifiable too (the `public_index` rule spans the whole tree). diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 7733f6154..05693908f 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -849,6 +849,16 @@ async fn collect_requirements_files(root: &Path) -> Result, (&'stat }); Ok(true) } + // pip decodes a UTF-16 file by its BOM (#721), so a pin inside one + // is installed; wiring around it would leave that pin unpatched. + Err(e) if e.kind() == std::io::ErrorKind::InvalidData => Err(( + "pypi_no_requirements", + format!( + "{} is not UTF-8 text (for example UTF-16, which Windows PowerShell 5.1 \ + writes for `pip freeze > requirements.txt`); re-save it as UTF-8 and re-run", + path.display() + ), + )), Err(_) if out.is_empty() => Err(( "pypi_no_requirements", format!("cannot read {}", path.display()), @@ -1214,6 +1224,43 @@ mod tests { tmp } + /// #721: pip installs from a UTF-16 requirements file (what Windows + /// PowerShell 5.1's `pip freeze >` writes), so vendoring must refuse it + /// by name, as the root file or as an include, never wire around it. + #[tokio::test] + async fn a_utf16_requirements_file_is_refused_by_name() { + let utf16 = |text: &str| -> Vec { + let mut out = vec![0xFF, 0xFE]; + for unit in text.encode_utf16() { + out.extend(unit.to_le_bytes()); + } + out + }; + let tmp = tempfile::tempdir().unwrap(); + std::fs::write( + tmp.path().join("requirements.txt"), + utf16("six==1.16.0\r\n"), + ) + .unwrap(); + let err = wire_requirements(tmp.path(), "six", "1.16.0", REL_WHEEL, SHA) + .await + .unwrap_err(); + assert_eq!(err.0, "pypi_no_requirements"); + assert!( + err.1.contains("requirements.txt is not UTF-8 text"), + "{}", + err.1 + ); + + let tmp = write_root("-r inc.txt\nidna==3.7\n").await; + std::fs::write(tmp.path().join("inc.txt"), utf16("six==1.16.0\r\n")).unwrap(); + let err = wire_requirements(tmp.path(), "six", "1.16.0", REL_WHEEL, SHA) + .await + .unwrap_err(); + assert!(err.1.contains("inc.txt is not UTF-8 text"), "{}", err.1); + assert_eq!(read_root(tmp.path()).await, "-r inc.txt\nidna==3.7\n"); + } + async fn read_root(root: &Path) -> String { tokio::fs::read_to_string(root.join("requirements.txt")) .await