From 09eaae4f0eff8097e62d307495fb0634da49b22b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 16:53:44 +0000 Subject: [PATCH 1/3] Start fix for #824 (slice 6) Assisted-by: Claude Code:claude-opus-5-5 From c836c12231702e21502d9f3f17ef63368c9028d7 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 17:13:11 +0000 Subject: [PATCH 2/3] Share binary/git_sha256 in the last test modules Four shared test modules (vex_e2e_common, vlt_e2e_common, vlt_hosted_common, vlt_vendor_common) still carried their own copy of the git-blob hash, and vex_e2e_common its own binary(). Its copy called production's hasher, so a test comparing the two could pass against a broken implementation. Both helpers now live in tests/common/oracle.rs. common re-exports them, and the shared modules pull the file in with #[path] and re-export it, the way common/hermetic.rs is shared. The pending list in cli/shared_helper_copies.rs is gone, so the check now covers every test file outside common/. Closes #824. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/cli/shared_helper_copies.rs | 33 ++++++------------- crates/socket-patch-cli/tests/common/mod.rs | 25 ++++---------- .../socket-patch-cli/tests/common/oracle.rs | 33 +++++++++++++++++++ .../tests/vex_e2e_common/mod.rs | 18 +++++----- .../tests/vlt_e2e_common/mod.rs | 9 ++--- .../tests/vlt_hosted_common/mod.rs | 15 +++++---- .../tests/vlt_vendor_common/mod.rs | 14 ++++---- 7 files changed, 76 insertions(+), 71 deletions(-) create mode 100644 crates/socket-patch-cli/tests/common/oracle.rs diff --git a/crates/socket-patch-cli/tests/cli/shared_helper_copies.rs b/crates/socket-patch-cli/tests/cli/shared_helper_copies.rs index 424de23ea..b5ba49f87 100644 --- a/crates/socket-patch-cli/tests/cli/shared_helper_copies.rs +++ b/crates/socket-patch-cli/tests/cli/shared_helper_copies.rs @@ -1,24 +1,14 @@ //! One `binary()` and one `git_sha256` for every CLI test target (#824). //! -//! `common/mod.rs` owns both helpers, and its own tests pin `git_sha256` -//! to the canonical Git-blob hash and to production's -//! `compute_git_sha256_from_bytes` (the two shapes the per-file copies -//! took). This ratchet keeps new private copies out: a file may define its -//! own only while it is listed below. +//! `common/oracle.rs` owns both helpers and `common/mod.rs` re-exports +//! them; `common`'s own tests pin `git_sha256` to the canonical Git-blob +//! hash and to production's `compute_git_sha256_from_bytes` (the two +//! shapes the per-file copies took). Shared modules whose includers don't +//! all declare `mod common` pull `common/oracle.rs` in on its own and +//! re-export it. This check keeps new private copies out. use std::path::Path; -/// Files that still define a private `binary()` or `git_sha256`. Each is -/// a shared module whose includers don't all declare `mod common`. -/// Migrate a file onto `common` and delete its entry; a stale entry is -/// not an error, so a PR that migrates one never turns another red. -const PENDING_PRIVATE_HELPERS: &[&str] = &[ - "vex_e2e_common/mod.rs", - "vlt_e2e_common/mod.rs", - "vlt_hosted_common/mod.rs", - "vlt_vendor_common/mod.rs", -]; - /// Every `.rs` file under `tests/`, as (`/`-joined relative path, text). fn test_sources() -> Vec<(String, String)> { fn walk(dir: &Path, root: &Path, out: &mut Vec<(String, String)>) { @@ -66,19 +56,16 @@ fn defines_private_helper(text: &str) -> bool { fn no_new_private_binary_or_git_sha256() { let unexpected: Vec = test_sources() .into_iter() - .filter(|(rel, text)| { - !rel.starts_with("common/") - && !PENDING_PRIVATE_HELPERS.contains(&rel.as_str()) - && defines_private_helper(text) - }) + .filter(|(rel, text)| !rel.starts_with("common/") && defines_private_helper(text)) .map(|(rel, _)| rel) .collect(); assert!( unexpected.is_empty(), "these test files define their own binary() or git_sha256: {unexpected:?}. \ Use `common::binary` / `common::git_sha256` instead (declare \ - `#[path = \"common/mod.rs\"] mod common;`, or `use crate::common::..` in a \ - directory test binary). Do not add them to PENDING_PRIVATE_HELPERS." + `#[path = \"common/mod.rs\"] mod common;`, `use crate::common::..` in a \ + directory test binary, or re-export them from `common/oracle.rs` in a \ + shared module)." ); } diff --git a/crates/socket-patch-cli/tests/common/mod.rs b/crates/socket-patch-cli/tests/common/mod.rs index 05fabad6c..11c6b4110 100644 --- a/crates/socket-patch-cli/tests/common/mod.rs +++ b/crates/socket-patch-cli/tests/common/mod.rs @@ -44,14 +44,14 @@ pub mod envelope; pub use envelope::{envelope_error_code, envelope_error_message, json_string, parse_json_envelope}; -// ── Binary discovery + invocation ───────────────────────────────────── +/// The binary under test and the Git-blob hash oracle. Shared modules +/// whose includers don't all declare `mod common` pull it in on its own +/// with `#[path = "../common/oracle.rs"] mod oracle;`. +pub mod oracle; -/// Absolute path to the built `socket-patch` binary that cargo -/// provides via the `CARGO_BIN_EXE_*` env var. Available because -/// these tests live in the same crate that produces the binary. -pub fn binary() -> PathBuf { - env!("CARGO_BIN_EXE_socket-patch").into() -} +pub use oracle::{binary, git_sha256}; + +// ── Binary discovery + invocation ───────────────────────────────────── /// Quick check whether `cmd` is on PATH. Used to soft-skip /// toolchain-dependent tests when the toolchain isn't installed @@ -126,17 +126,6 @@ pub fn assert_run_ok(cwd: &Path, args: &[&str], context: &str) -> (String, Strin // ── Hashing ─────────────────────────────────────────────────────────── -/// Compute Git-flavored SHA-256: `SHA256("blob \0" ++ content)`. -/// This is the hash socket-patch records in manifests under -/// `before_hash` / `after_hash`. -pub 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()) -} - /// Git-SHA-256 of the file at `path`. Panics if the file can't be /// read — tests use this on paths they know exist. pub fn git_sha256_file(path: &Path) -> String { diff --git a/crates/socket-patch-cli/tests/common/oracle.rs b/crates/socket-patch-cli/tests/common/oracle.rs new file mode 100644 index 000000000..86197c2c6 --- /dev/null +++ b/crates/socket-patch-cli/tests/common/oracle.rs @@ -0,0 +1,33 @@ +//! The binary under test and the Git-blob hash oracle (#824). +//! +//! `common/mod.rs` re-exports both. Shared modules whose includers don't +//! all declare `mod common` pull this file in on its own with +//! `#[path = "../common/oracle.rs"] mod oracle;`, so every test target +//! uses the one `binary()` and the one `git_sha256` that +//! `common::oracle_selftests` pins. + +#![allow(dead_code)] + +use std::path::PathBuf; + +use sha2::{Digest, Sha256}; + +/// Absolute path to the built `socket-patch` binary that cargo +/// provides via the `CARGO_BIN_EXE_*` env var. Available because +/// these tests live in the same crate that produces the binary. +pub fn binary() -> PathBuf { + env!("CARGO_BIN_EXE_socket-patch").into() +} + +/// Compute Git-flavored SHA-256: `SHA256("blob \0" ++ content)`. +/// This is the hash socket-patch records in manifests under +/// `before_hash` / `after_hash`. It is computed here, independently of +/// production's hasher, so a round trip through both can't agree on a +/// broken implementation. +pub 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()) +} diff --git a/crates/socket-patch-cli/tests/vex_e2e_common/mod.rs b/crates/socket-patch-cli/tests/vex_e2e_common/mod.rs index bc219bc99..13c61564a 100644 --- a/crates/socket-patch-cli/tests/vex_e2e_common/mod.rs +++ b/crates/socket-patch-cli/tests/vex_e2e_common/mod.rs @@ -99,16 +99,14 @@ pub fn installed_pnpm_pre_10_5(proj: &Path) -> bool { } } -/// Absolute path of the `socket-patch` binary under test. -pub fn binary() -> PathBuf { - env!("CARGO_BIN_EXE_socket-patch").into() -} - -/// Git-style sha256 of `bytes` — the `afterHash` a patch view carries for a -/// patched file (what installed-tree verification recomputes). -pub fn git_sha256(bytes: &[u8]) -> String { - socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes(bytes) -} +/// The binary under test and the Git-blob hash (the `afterHash` a patch +/// view carries for a patched file), shared with `common` (#824). +#[path = "../common/oracle.rs"] +mod oracle; + +// Not every includer uses these. +#[allow(unused_imports)] +pub use oracle::{binary, git_sha256}; // ── Project shaping ─────────────────────────────────────────────────── diff --git a/crates/socket-patch-cli/tests/vlt_e2e_common/mod.rs b/crates/socket-patch-cli/tests/vlt_e2e_common/mod.rs index 0784c4ea5..b746664d5 100644 --- a/crates/socket-patch-cli/tests/vlt_e2e_common/mod.rs +++ b/crates/socket-patch-cli/tests/vlt_e2e_common/mod.rs @@ -1280,12 +1280,9 @@ pub fn sha256_hex(bytes: &[u8]) -> String { hex::encode(Sha256::digest(bytes)) } -pub fn git_sha256(bytes: &[u8]) -> String { - let mut h = Sha256::new(); - h.update(format!("blob {}\0", bytes.len()).as_bytes()); - h.update(bytes); - hex::encode(h.finalize()) -} +// Not every includer uses it. +#[allow(unused_imports)] +pub use common::git_sha256; /// A dir vlt (rc.14) renames an old tree to and deletes in the background. pub fn is_vlt_delete_staging(name: &std::ffi::OsStr) -> bool { diff --git a/crates/socket-patch-cli/tests/vlt_hosted_common/mod.rs b/crates/socket-patch-cli/tests/vlt_hosted_common/mod.rs index 9ae436209..3693bb401 100644 --- a/crates/socket-patch-cli/tests/vlt_hosted_common/mod.rs +++ b/crates/socket-patch-cli/tests/vlt_hosted_common/mod.rs @@ -14,7 +14,7 @@ use std::path::{Path, PathBuf}; use base64::Engine as _; use serde_json::{json, Value}; -use sha2::{Digest, Sha256, Sha512}; +use sha2::{Digest, Sha512}; use wiremock::matchers::{method, path, path_regex}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -91,12 +91,13 @@ pub fn with_flags(line: &str, flags: u8) -> String { line.replacen("[0,", &format!("[{flags},"), 1) } -pub fn git_sha256(bytes: &[u8]) -> String { - let mut hasher = Sha256::new(); - hasher.update(format!("blob {}\0", bytes.len()).as_bytes()); - hasher.update(bytes); - hex::encode(hasher.finalize()) -} +#[path = "../common/oracle.rs"] +mod oracle; + +/// The Git-blob hash oracle shared with `common` (#824). +// Not every includer uses it. +#[allow(unused_imports)] +pub use oracle::git_sha256; pub fn sha512_sri(bytes: &[u8]) -> String { format!( diff --git a/crates/socket-patch-cli/tests/vlt_vendor_common/mod.rs b/crates/socket-patch-cli/tests/vlt_vendor_common/mod.rs index 30f61c744..4de6e0add 100644 --- a/crates/socket-patch-cli/tests/vlt_vendor_common/mod.rs +++ b/crates/socket-patch-cli/tests/vlt_vendor_common/mod.rs @@ -25,7 +25,6 @@ use std::path::{Path, PathBuf}; use std::process::Command; use serde_json::{json, Value}; -use sha2::{Digest, Sha256}; pub const UUID: &str = "9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f"; pub const UUID2: &str = "0a1b2c3d-4e5f-4a7b-8c9d-0e1f2a3b4c5d"; @@ -43,12 +42,13 @@ pub const VLT_LOCK: &str = "vlt-lock.json"; /// The real-vlt versions whose captures are committed. pub const CAPTURED: &[&str] = &["1.2.0", "1.0.10", "1.0.4", "1.0.0-rc.32", "1.0.0-rc.14"]; -pub fn git_sha256(bytes: &[u8]) -> String { - let mut hasher = Sha256::new(); - hasher.update(format!("blob {}\0", bytes.len()).as_bytes()); - hasher.update(bytes); - hex::encode(hasher.finalize()) -} +#[path = "../common/oracle.rs"] +mod oracle; + +/// The Git-blob hash oracle shared with `common` (#824). +// Not every includer uses it. +#[allow(unused_imports)] +pub use oracle::git_sha256; pub fn fixtures() -> PathBuf { Path::new(env!("CARGO_MANIFEST_DIR")).join("../socket-patch-core/tests/fixtures/vendor/npm/vlt") From 288319d716bcac0feebcc36dd55d302491869d70 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 17:23:40 +0000 Subject: [PATCH 3/3] Update the common test module's doc The doc still said test files carry their own helper copies and could migrate later. That migration is now done. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/tests/common/mod.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/crates/socket-patch-cli/tests/common/mod.rs b/crates/socket-patch-cli/tests/common/mod.rs index 11c6b4110..1cf239401 100644 --- a/crates/socket-patch-cli/tests/common/mod.rs +++ b/crates/socket-patch-cli/tests/common/mod.rs @@ -1,12 +1,9 @@ -//! Helpers shared across the e2e-safety test suites. -//! -//! The original e2e files (`e2e_npm.rs`, `e2e_pypi.rs`, `e2e_gem.rs`) -//! each carry their own copy of the same `binary` / `run` / -//! `assert_run_ok` / `git_sha256` helpers. Rather than refactor those -//! files in this PR, this module is an additive landing place for the -//! same surface plus the new helpers the safety suites need -//! (synthetic manifest writers, pnpm runners, cargo runners). Existing -//! suites can migrate in a follow-up. +//! Helpers shared across the CLI test suites: the binary under test and +//! the Git-blob hash oracle (`oracle`), `run` / `assert_run_ok`, and the +//! synthetic manifest writers and package-manager runners the e2e suites +//! need. Test files use these instead of private copies; +//! `cli/shared_helper_copies.rs` enforces that for `binary` and +//! `git_sha256` (#824). //! //! Each test file pulls this in with `#[path = "common/mod.rs"] mod common;`. //!