Skip to content

Commit 7ce869e

Browse files
Fix pip rollback refusing all-hosted requirements (#410) (#827)
* Start fix for #410 Assisted-by: Claude Code:claude-opus-5-5 * Fix pip rollback refusing all-hosted requirements Hosted rollback, remove and the hosted-to-vendored takeover refused a requirements.txt in which every requirement was a hosted pin (a lone `six==1.16.0`, or one beside `-e .`). They couldn't tell whether the original line used pip's hash-checking mode, so the only way back was version control. The restore now counts an editable line as unhashed evidence (pip refuses editables in hash-checking mode). When no other line settles the mode, it reads the hosted line itself: the rewriter writes `--hash` only into an already hashed file and otherwise pins by the url's `#sha256=` fragment. With nothing else in the file to conflict with, either restored form installs. Fixes #410 Assisted-by: Claude Code:claude-opus-5-5 * Update restore golden for sole-pin requirements The golden test asserted that a requirements.txt holding only the hosted pin is refused as ambiguous, which is the #410 bug. It now asserts that both the unhashed and hashed sole-pin files round-trip, and keeps the mixed hashed/unhashed refusal. Refs #410 Assisted-by: Claude Code:claude-opus-5-5 * Fix vex alias tests broken by store-copy merge #605 taught the name-keyed npm resolver to probe bundled store trees, so it now finds aliased copies (node_modules/lp) and a nested host's store peers itself. Two vex_consumed tests from #738 assumed that set never held aliases, so main's CI went red after both merged. The tests now feed the alias-free set explicitly to keep covering alias expansion, and also check the resolver's own set reaches the same copies with no duplicates. No production code changes. Assisted-by: Claude Code:claude-opus-5-5 (cherry picked from commit 40dac07) * Route Gradle digests through utils::digest main has failed socket-patch-core's lib tests since Gradle support (#646) and the digest helpers (#865) both landed. The guard test production_digests_go_through_the_helpers flags three files #646 added that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. That breaks test, test-release and coverage on every open PR. Each inline sha1/sha256 call now goes through sha1_hex_of or sha256_hex_of, which compute the same lowercase hex. Behaviour is unchanged. Assisted-by: Claude Code:claude-opus-5-5 (cherry picked from commit 659ac2c) --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8a02f08 commit 7ce869e

4 files changed

Lines changed: 263 additions & 7 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -917,7 +917,7 @@ v5.0 replaces v4's per-purl reverts and whole-ledger reverse replay (`revert_rem
917917
* **vlt** — `vlt-lock.json`: slot [2] from the registry's `dist.integrity`, slot [3] per the lock's own convention (see the vlt hosted-mode contract); every hosted instance of the pin together.
918918
* **cargo** — `Cargo.lock` back on crates.io (source + the sparse index's checksum, `SOCKET_CRATES_INDEX`); every `Cargo.toml` declaration loses its `registry = "socket-patch-<uuid>"` pin (the shorthand the rewriter produced collapses back); the unreferenced `[registries.socket-patch-<uuid>]` block leaves the project cargo config. A declaration it cannot unpin refuses.
919919
* **golang** — the hosted `replace` and the socket module's go.sum lines go; the upstream module's two go.sum lines come back, hashed from the module proxy (`SOCKET_GOPROXY`, else `GOPROXY` / `GONOPROXY` / `GOPRIVATE` as go reads them) and cross-checked against the checksum database (`SOCKET_GOSUMDB_URL`, else `sum.golang.org` unless `GOSUMDB=off` / `GONOSUMDB` / `GOPRIVATE` say go would not ask it). A `replace` the user had before the hosted run is not recorded anywhere, so the restore lands on the plain upstream module.
920-
* **pypi** — `Pipfile.lock`, `requirements.txt` (+ in-root `-r` includes), Hatch PEP 508 direct references (`pyproject.toml` / `hatch.toml`), `poetry.lock`, `pdm.lock`, `uv.lock`, PEP 723 script locks and PEP 751 `pylock*.toml` (+ the paired `pyproject.toml` / script metadata): hashes re-derived from PyPI's JSON API (`SOCKET_PYPI_JSON_API`). Refused: a `pdm.lock` without `cross_platform`, or a uv / script / pylock lock, whose release has a wheel that is not pure Python 3 (which files the lock keeps is not re-derivable); a uv lock whose options filter files (`exclude-newer`, `no-binary`, `no-build`), or whose other registry packages name no registry, several, or one other than PyPI's simple index; a pylock whose other registry packages show neither an `index` nor (as `uv pip compile` writes them) only PyPI files with none, which restores the entry without an `index` too; uv 0.2 `[[distribution]]` locks. Restored artifact fields keep the spelling the lock's other entries show, including the `upload_time` that uv 0.6.15–0.6.17 write. A restored pylock entry's `upload-time`s are whole seconds, as uv writes them, unless the lock's other entries show fractions. Its artifacts come back in the TOML spelling the other entries use: uv's inline `wheels = [{ … }]`, or the standard tables `pip lock` writes (`[[packages.wheels]]` with a `[packages.wheels.hashes]` sub-table, `[packages.sdist]`). A `pip lock` file (`created-by = "pip"`) records only the artifact pip selected, so the entry is restored with only the release's wheel (its sdist when it has none), and a release with several wheels is refused. A transitive `override-dependencies` entry hosted mode added is removed (`upstream_uv_override_removed`).
920+
* **pypi** — `Pipfile.lock`, `requirements.txt` (+ in-root `-r` includes), Hatch PEP 508 direct references (`pyproject.toml` / `hatch.toml`), `poetry.lock`, `pdm.lock`, `uv.lock`, PEP 723 script locks and PEP 751 `pylock*.toml` (+ the paired `pyproject.toml` / script metadata): hashes re-derived from PyPI's JSON API (`SOCKET_PYPI_JSON_API`). A restored `requirements.txt` line gets `--hash` options only when the file is in pip's hash-checking mode. The mode is read off the file's other requirement lines (an `-e` / `--editable` line means unhashed). When every requirement is a hosted pin, it is read off the hosted line itself (`--hash` vs a `#sha256=` url fragment) (#410). Refused: a `pdm.lock` without `cross_platform`, or a uv / script / pylock lock, whose release has a wheel that is not pure Python 3 (which files the lock keeps is not re-derivable); a uv lock whose options filter files (`exclude-newer`, `no-binary`, `no-build`), or whose other registry packages name no registry, several, or one other than PyPI's simple index; a pylock whose other registry packages show neither an `index` nor (as `uv pip compile` writes them) only PyPI files with none, which restores the entry without an `index` too; uv 0.2 `[[distribution]]` locks. Restored artifact fields keep the spelling the lock's other entries show, including the `upload_time` that uv 0.6.15–0.6.17 write. A restored pylock entry's `upload-time`s are whole seconds, as uv writes them, unless the lock's other entries show fractions. Its artifacts come back in the TOML spelling the other entries use: uv's inline `wheels = [{ … }]`, or the standard tables `pip lock` writes (`[[packages.wheels]]` with a `[packages.wheels.hashes]` sub-table, `[packages.sdist]`). A `pip lock` file (`created-by = "pip"`) records only the artifact pip selected, so the entry is restored with only the release's wheel (its sdist when it has none), and a release with several wheels is refused. A transitive `override-dependencies` entry hosted mode added is removed (`upstream_uv_override_removed`).
921921
* **gem** — `Gemfile.lock` / `gems.locked` + `Gemfile` / `gems.rb`: the spec moves back into the upstream `GEM` section (or the Socket remote leaves a merged section), the `source "<patch registry>" do … end` block is undone, the `CHECKSUMS` entry is re-pinned from the rubygems.org compact index (`SOCKET_RUBYGEMS_URL`) and the `DEPENDENCIES` pin loses its `!`. The declaration's original constraint is not recorded, so it comes back as the exact pin `gem "<name>", "<version>"`. A transitive gem (one the manifest never declared) gets an appended block with a blank line before it; the restore removes that block, its blank line and the `DEPENDENCIES` entry, so the pair comes back byte for byte. An appended block with no blank line before it (written by a release before this one) can't be told apart from an in-place rewrite, so it still comes back as the exact pin. Refused: an ambiguous upstream section, an upstream remote other than rubygems.org.
922922
* **composer** — `composer.lock`: `dist` and the deleted `source` block from packagist's composer v2 metadata (`SOCKET_PACKAGIST_URL`). Refused unless the entry is packagist-sourced and packagist still serves the lock's `dist.reference` for the version.
923923
* **maven** — `pom.xml` (the `-socket.<hex8>` version suffix, the added `<repository>` / `<dependencyManagement>` entry) and the `.mvn/maven.config` / `.mvn/checksums/checksums.sha256` lines hosted mode writes: **no network**, so it restores under `--offline` too. `.mvn` files holding anything else keep the resolver lines (`maven_trusted_checksums_left`).

‎crates/socket-patch-cli/tests/mode_migration_pypi.rs‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,62 @@ async fn requirements_sole_pin_vendored_to_hosted() {
397397
assert_vendored_to_hosted(&root, &["requirements.txt"]).await;
398398
}
399399

400+
/// #410: a requirements.txt in which every requirement is the hosted pin
401+
/// (a lone `six==1.16.0`, or one beside `-e .`) can be unwound again.
402+
/// Hosted `rollback`, `remove` and the hosted → vendored takeover restore
403+
/// the pin to its unhashed registry spelling. Before the fix they all
404+
/// refused: no other line said whether the original used `--hash`.
405+
async fn assert_all_hosted_requirements_unwind(pristine: &str) {
406+
let server = MockServer::start().await;
407+
let hosted_url = mount_hosted_api(&server, true).await;
408+
let uri = server.uri();
409+
for unwind in [
410+
vec!["rollback", "--yes", "--offline"],
411+
vec!["remove", PURL, "--yes", "--offline"],
412+
// The fixture server builds the vendored wheel.
413+
vec!["vendor"],
414+
] {
415+
let (_tmp, root) = project();
416+
std::fs::write(root.join("requirements.txt"), pristine).unwrap();
417+
let (code, env) = hosted_scan(&root, &server);
418+
assert_eq!(code, 0, "hosted scan: {env:#}");
419+
assert_eq!(env["redirect"]["redirected"], 1, "{env:#}");
420+
let wired = std::fs::read_to_string(root.join("requirements.txt")).unwrap();
421+
assert!(wired.contains(&hosted_url), "hosted first:\n{wired}");
422+
423+
if unwind[0] == "vendor" {
424+
stage_manifest(&root);
425+
// `--patch-server-url` (which names the hosted origin to take
426+
// over) also moves the vendored download onto this server.
427+
prebuilt_common::mount_project(&server, &root).await;
428+
}
429+
let mut args = unwind.clone();
430+
args.extend(["--patch-server-url", uri.as_str()]);
431+
let (code, env) = run_cli(&root, &args, &[]);
432+
assert_eq!(code, 0, "{unwind:?} over {pristine:?}: {env:#}");
433+
let after = std::fs::read_to_string(root.join("requirements.txt")).unwrap();
434+
if unwind[0] == "vendor" {
435+
assert!(
436+
after.contains(&format!(".socket/vendor/pypi/{UUID}/"))
437+
&& !after.contains(&hosted_url),
438+
"the takeover leaves the project vendored:\n{after}"
439+
);
440+
} else {
441+
assert_eq!(after, pristine, "{unwind:?} restores the pristine file");
442+
}
443+
}
444+
}
445+
446+
#[tokio::test]
447+
async fn requirements_sole_hosted_pin_unwinds() {
448+
assert_all_hosted_requirements_unwind("six==1.16.0\n").await;
449+
}
450+
451+
#[tokio::test]
452+
async fn requirements_editable_beside_hosted_pin_unwinds() {
453+
assert_all_hosted_requirements_unwind("-e .\nsix==1.16.0\n").await;
454+
}
455+
400456
/// A Poetry project; returns its wiring files.
401457
fn stage_poetry(root: &Path) -> &'static [&'static str] {
402458
std::fs::write(

‎crates/socket-patch-core/src/patch/redirect/upstream/pypi.rs‎

Lines changed: 198 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,17 @@ fn has_hash_option(tokens: &[&str]) -> bool {
443443
.any(|t| *t == "--hash" || t.starts_with("--hash="))
444444
}
445445

446+
/// Whether a requirements line is an editable (`-e <path>`, `-e<path>`,
447+
/// `--editable <path>`, `--editable=<path>`), which pip refuses in
448+
/// hash-checking mode.
449+
fn is_editable(tokens: &[&str]) -> bool {
450+
tokens.first().is_some_and(|t| {
451+
(t.starts_with("-e") && !t.starts_with("--"))
452+
|| *t == "--editable"
453+
|| t.starts_with("--editable=")
454+
})
455+
}
456+
446457
/// A hosted requirement line, cut into what its registry spelling keeps.
447458
struct HostedLine {
448459
uuid: String,
@@ -454,6 +465,10 @@ struct HostedLine {
454465
marker: String,
455466
options: String,
456467
comment: String,
468+
/// The hosted line carries `--hash`: the requirements rewriter writes
469+
/// it only into a file already in pip's hash-checking mode (#376),
470+
/// else it pins by the url's `#sha256=` fragment.
471+
hashed: bool,
457472
}
458473

459474
/// The in-scope hosted line `requirement` is, if any: `Err((uuid, why))`
@@ -489,6 +504,7 @@ fn hosted_line(
489504
let tokens = requirement_tokens(body);
490505
let marker_len = tokens.iter().take_while(|t| !t.starts_with("--")).count();
491506
let marker = tokens[..marker_len].join(" ");
507+
let hashed = has_hash_option(&tokens[marker_len..]);
492508
let mut options = Vec::new();
493509
let mut rest_tokens = tokens[marker_len..].iter();
494510
while let Some(token) = rest_tokens.next() {
@@ -517,6 +533,7 @@ fn hosted_line(
517533
marker,
518534
options: options.join(" "),
519535
comment: comment.trim().to_string(),
536+
hashed,
520537
}))
521538
}
522539

@@ -562,6 +579,12 @@ pub(crate) async fn restore_requirements(
562579
if tokens.contains(&"--require-hashes") {
563580
require_hashes = true;
564581
}
582+
// pip refuses an editable in hash-checking mode, so one settles
583+
// the file as unhashed (#410).
584+
if is_editable(&tokens) {
585+
unhashed += 1;
586+
continue;
587+
}
565588
if code.starts_with('-') {
566589
continue;
567590
}
@@ -597,10 +620,10 @@ pub(crate) async fn restore_requirements(
597620
"{rel} mixes hashed and unhashed requirements, so whether the original line \
598621
carried `--hash` options is not derivable"
599622
)),
600-
(false, false) => Err(format!(
601-
"every requirement in {rel} is a hosted pin, so whether the original used pip's \
602-
hash-checking mode (`--hash`) is not derivable"
603-
)),
623+
// Every requirement is a hosted pin (#410): nothing else in the
624+
// file can conflict with either form, so follow the hosted lines'
625+
// own shape, which records the mode the rewrite found.
626+
(false, false) => Ok(hits.iter().any(|(_, line)| line.hashed)),
604627
};
605628
let hash_mode = match hash_mode {
606629
Ok(mode) => mode,
@@ -993,4 +1016,175 @@ mod tests {
9931016
assert_eq!(multiline_toml_array(&[]), "[]");
9941017
assert_eq!(toml_quote("a\"b\\"), "\"a\\\"b\\\\\"");
9951018
}
1019+
1020+
// ── requirements.txt: pip's hash-checking mode (#410) ──────────────────
1021+
1022+
use super::super::{restore_upstream, HostedPin, PinStatus, RestoreOptions, RestoreOutcome};
1023+
use wiremock::matchers::{method, path};
1024+
use wiremock::{Mock, MockServer, ResponseTemplate};
1025+
1026+
const SIX_UUID: &str = "41041041-0410-4410-8410-410410410410";
1027+
const SIX_PATCHED: &str = "1111111111111111111111111111111111111111111111111111111111111111";
1028+
const SIX_WHEEL: &str = "2222222222222222222222222222222222222222222222222222222222222222";
1029+
const SIX_SDIST: &str = "3333333333333333333333333333333333333333333333333333333333333333";
1030+
1031+
fn six_url() -> String {
1032+
format!(
1033+
"https://patch.socket.dev/patch-registry/pypi/11111111-1111-1111-1111-111111111111/{SIX_UUID}/six-1.16.0-py2.py3-none-any.whl"
1034+
)
1035+
}
1036+
1037+
/// The hosted line the requirements rewriter writes for six into an
1038+
/// unhashed file (url `#sha256=` fragment, no `--hash` option).
1039+
fn fragment_line() -> String {
1040+
format!("six @ {}#sha256={SIX_PATCHED}", six_url())
1041+
}
1042+
1043+
/// The hosted line it writes into a hashed file (and that every pre-#383
1044+
/// v5 rewrite wrote, whatever the file's mode).
1045+
fn hashed_line() -> String {
1046+
format!("six @ {} --hash=sha256:{SIX_PATCHED}", six_url())
1047+
}
1048+
1049+
async fn pypi_json() -> MockServer {
1050+
let server = MockServer::start().await;
1051+
Mock::given(method("GET"))
1052+
.and(path("/pypi/six/1.16.0/json"))
1053+
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
1054+
"urls": [
1055+
{ "filename": "six-1.16.0-py2.py3-none-any.whl",
1056+
"url": "https://files.example/six-1.16.0-py2.py3-none-any.whl",
1057+
"digests": { "sha256": SIX_WHEEL } },
1058+
{ "filename": "six-1.16.0.tar.gz",
1059+
"url": "https://files.example/six-1.16.0.tar.gz",
1060+
"digests": { "sha256": SIX_SDIST } },
1061+
]
1062+
})))
1063+
.mount(&server)
1064+
.await;
1065+
server
1066+
}
1067+
1068+
/// Restore six's hosted pin in `requirements` (online against a mocked
1069+
/// PyPI JSON API); the outcome and the file afterwards.
1070+
async fn restore_six(requirements: &str) -> (RestoreOutcome, String) {
1071+
let server = pypi_json().await;
1072+
std::env::set_var("SOCKET_PYPI_JSON_API", format!("{}/pypi", server.uri()));
1073+
let tmp = tempfile::tempdir().unwrap();
1074+
std::fs::write(tmp.path().join("requirements.txt"), requirements).unwrap();
1075+
let pins = [HostedPin {
1076+
purl: "pkg:pypi/six@1.16.0".into(),
1077+
uuid: SIX_UUID.into(),
1078+
files: vec!["requirements.txt".into()],
1079+
}];
1080+
let outcome = restore_upstream(tmp.path(), &pins, &RestoreOptions::default()).await;
1081+
std::env::remove_var("SOCKET_PYPI_JSON_API");
1082+
let after = std::fs::read_to_string(tmp.path().join("requirements.txt")).unwrap();
1083+
(outcome, after)
1084+
}
1085+
1086+
fn assert_restored(outcome: &RestoreOutcome) {
1087+
assert_eq!(
1088+
outcome.pins[0].status,
1089+
PinStatus::Restored,
1090+
"{:?}",
1091+
outcome.pins
1092+
);
1093+
}
1094+
1095+
/// #410: a file whose only requirement is the hosted pin restores. The
1096+
/// hosted line's own shape records the original mode: a `#sha256=`
1097+
/// fragment means the file was unhashed.
1098+
#[tokio::test]
1099+
#[serial_test::serial]
1100+
async fn requirements_with_only_a_hosted_pin_restores_unhashed() {
1101+
for eol in ["\n", "\r\n"] {
1102+
let (outcome, after) = restore_six(&format!("{}{eol}", fragment_line())).await;
1103+
assert_restored(&outcome);
1104+
assert_eq!(after, format!("six==1.16.0{eol}"));
1105+
}
1106+
// Comments, options and blank lines don't settle the mode either.
1107+
let (outcome, after) = restore_six(&format!(
1108+
"# pinned\n--index-url https://pypi.org/simple\n\n{} # via app\n",
1109+
fragment_line()
1110+
))
1111+
.await;
1112+
assert_restored(&outcome);
1113+
assert_eq!(
1114+
after,
1115+
"# pinned\n--index-url https://pypi.org/simple\n\nsix==1.16.0 # via app\n"
1116+
);
1117+
}
1118+
1119+
/// #410: an all-hosted file whose hosted line carries `--hash` restores
1120+
/// in hash-checking mode, with every upstream release file's hash. With
1121+
/// no other requirement in the file there is nothing for it to conflict
1122+
/// with.
1123+
#[tokio::test]
1124+
#[serial_test::serial]
1125+
async fn requirements_with_only_a_hashed_hosted_pin_restores_hashed() {
1126+
let (outcome, after) = restore_six(&format!("{}\n", hashed_line())).await;
1127+
assert_restored(&outcome);
1128+
assert_eq!(
1129+
after,
1130+
format!("six==1.16.0 --hash=sha256:{SIX_WHEEL} --hash=sha256:{SIX_SDIST}\n")
1131+
);
1132+
}
1133+
1134+
/// #410: pip refuses an editable requirement in hash-checking mode, so an
1135+
/// `-e` line settles the file as unhashed, even beside a hosted line
1136+
/// that carries `--hash` (the pre-#383 shape).
1137+
#[tokio::test]
1138+
#[serial_test::serial]
1139+
async fn an_editable_line_settles_requirements_as_unhashed() {
1140+
for editable in ["-e .", "-e ./lib", "--editable .", "--editable=.", "-e."] {
1141+
for hosted in [fragment_line(), hashed_line()] {
1142+
let (outcome, after) = restore_six(&format!("{editable}\n{hosted}\n")).await;
1143+
assert_restored(&outcome);
1144+
assert_eq!(after, format!("{editable}\nsix==1.16.0\n"), "{hosted}");
1145+
}
1146+
}
1147+
}
1148+
1149+
/// Other requirement lines still decide, and genuinely mixed files are
1150+
/// still refused rather than guessed.
1151+
#[tokio::test]
1152+
#[serial_test::serial]
1153+
async fn other_requirement_lines_still_settle_the_mode() {
1154+
let (outcome, after) = restore_six(&format!("idna==3.7\n{}\n", hashed_line())).await;
1155+
assert_restored(&outcome);
1156+
assert_eq!(after, "idna==3.7\nsix==1.16.0\n");
1157+
1158+
let hashed_idna = "idna==3.7 --hash=sha256:aaaa\n";
1159+
let (outcome, after) = restore_six(&format!("{hashed_idna}{}\n", fragment_line())).await;
1160+
assert_restored(&outcome);
1161+
assert_eq!(
1162+
after,
1163+
format!(
1164+
"{hashed_idna}six==1.16.0 --hash=sha256:{SIX_WHEEL} --hash=sha256:{SIX_SDIST}\n"
1165+
)
1166+
);
1167+
1168+
let mixed = format!(
1169+
"idna==3.7 --hash=sha256:aaaa\ncertifi==2024.2.2\n{}\n",
1170+
fragment_line()
1171+
);
1172+
let (outcome, after) = restore_six(&mixed).await;
1173+
assert!(
1174+
matches!(&outcome.pins[0].status, PinStatus::Refused(why) if why.contains("mixes hashed and unhashed")),
1175+
"{:?}",
1176+
outcome.pins
1177+
);
1178+
assert_eq!(after, mixed);
1179+
1180+
// `-e` beside `--require-hashes` is a file pip can't install at all.
1181+
let broken = format!("--require-hashes\n-e .\n{}\n", hashed_line());
1182+
let (outcome, after) = restore_six(&broken).await;
1183+
assert!(
1184+
matches!(&outcome.pins[0].status, PinStatus::Refused(_)),
1185+
"{:?}",
1186+
outcome.pins
1187+
);
1188+
assert_eq!(after, broken);
1189+
}
9961190
}

0 commit comments

Comments
 (0)