Skip to content

Commit cc681f1

Browse files
committed
Merge remote-tracking branch 'origin/main' into agent/fix-ci-hermetic-vex-commons
# Conflicts: # crates/socket-patch-cli/tests/spawn_env_hygiene.rs
2 parents 9078ed4 + 49847ef commit cc681f1

12 files changed

Lines changed: 149 additions & 77 deletions

File tree

‎crates/socket-patch-cli/tests/common/cache_env.rs‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,8 +318,9 @@ fn scrub_parent_vars(cmd: &mut Command, parent: Vec<OsString>, ambient: fn(&str)
318318

319319
// ── Ambient-env scrub for the vlt suites ──────────────────────────────
320320

321-
/// True for an ambient variable the real-vlt suites strip from every child
322-
/// (vlt and the CLI under test) before [`isolate`]:
321+
/// True for an ambient variable the real-vlt suites strip from every vlt
322+
/// child before [`isolate`] (a `socket-patch` child built with
323+
/// `hermetic::command` takes [`scrub_ambient_vlt_tool_env`] instead):
323324
///
324325
/// * every `VLT_*` — vlt reads each config option from `VLT_<OPTION>`
325326
/// (`VLT_REGISTRY`, `VLT_REGISTRIES`, `VLT_CACHE`, `VLT_STORE_LINKER`);
@@ -343,6 +344,18 @@ pub fn scrub_ambient_vlt_env(cmd: &mut Command) {
343344
scrub_parent_vars(cmd, parent_var_names(), is_ambient_vlt_var);
344345
}
345346

347+
/// [`is_ambient_vlt_var`] minus `SOCKET_*`.
348+
fn is_ambient_vlt_tool_var(name: &str) -> bool {
349+
!name.starts_with("SOCKET_") && is_ambient_vlt_var(name)
350+
}
351+
352+
/// [`scrub_ambient_vlt_env`] minus `SOCKET_*`, for a `socket-patch` child
353+
/// built with `hermetic::command`: the builder already handled `SOCKET_*`,
354+
/// and removing them again would undo the opt-outs it forces on.
355+
pub fn scrub_ambient_vlt_tool_env(cmd: &mut Command) {
356+
scrub_parent_vars(cmd, parent_var_names(), is_ambient_vlt_tool_var);
357+
}
358+
346359
// ── Self-tests ────────────────────────────────────────────────────────
347360
//
348361
// Integration-test crates do not get `cfg(test)`, so — exactly as in
@@ -454,6 +467,24 @@ mod cache_env_selftests {
454467
}
455468
}
456469

470+
/// The CLI-child variant keeps every `SOCKET_*`, so it never undoes
471+
/// the opt-outs `hermetic::command` forces on, and still strips the
472+
/// vlt, npm and Python config.
473+
#[test]
474+
fn ambient_vlt_tool_scrub_leaves_socket_vars_to_the_hermetic_builder() {
475+
for name in [
476+
"SOCKET_TELEMETRY_DISABLED",
477+
"SOCKET_NO_UPDATE_CHECK",
478+
"SOCKET_NO_CONFIG",
479+
"SOCKET_API_TOKEN",
480+
] {
481+
assert!(!is_ambient_vlt_tool_var(name), "{name} must survive");
482+
}
483+
for name in ["VLT_REGISTRY", "npm_config_registry", "VIRTUAL_ENV"] {
484+
assert!(is_ambient_vlt_tool_var(name), "{name} must be scrubbed");
485+
}
486+
}
487+
457488
#[test]
458489
fn ambient_vlt_scrub_removes_only_matching_parent_vars() {
459490
let parent = parent_var_names();

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,6 @@ const PENDING_RAW_SPAWNS: &[&str] = &[
5555
"vendor/in_process_vendor_bun.rs",
5656
"vendor/vendor_gem_lockfile_only_e2e.rs",
5757
"vendor/vendor_rerun_no_network_e2e.rs",
58-
"vlt_e2e_common/mod.rs",
59-
"vlt_hosted_common/mod.rs",
60-
"vlt_vendor_common/mod.rs",
6158
];
6259

6360
/// The environment `cmd`'s child would see: the parent environment with

‎crates/socket-patch-cli/tests/vlt_e2e_common/mod.rs‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1758,25 +1758,22 @@ pub fn urlencode(s: &str) -> String {
17581758

17591759
// ── socket-patch ──────────────────────────────────────────────────────────
17601760

1761-
/// The socket-patch binary with the ambient `SOCKET_*`, `VLT_*`, npm and
1762-
/// proxy environment scrubbed.
1761+
/// The socket-patch binary with the hermetic `SOCKET_*` environment (see
1762+
/// `common::hermetic::command`) and the ambient `VLT_*`, npm and proxy
1763+
/// environment scrubbed.
17631764
pub fn socket_cmd() -> Command {
1764-
let mut cmd = Command::new(socket_bin());
1765-
cache_env::scrub_ambient_vlt_env(&mut cmd);
1765+
let mut cmd = common::hermetic::command(&socket_bin());
1766+
cache_env::scrub_ambient_vlt_tool_env(&mut cmd);
17661767
for (key, _) in std::env::vars_os() {
17671768
let name = key.to_string_lossy();
17681769
if ["http_proxy", "https_proxy", "all_proxy", "no_proxy"]
17691770
.iter()
17701771
.any(|p| name.eq_ignore_ascii_case(p))
1771-
|| name.starts_with("SOCKET_PATCH_VLT_E2E_")
17721772
{
17731773
cmd.env_remove(&key);
17741774
}
17751775
}
1776-
cmd.env("SOCKET_NO_CONFIG", "1")
1777-
.env("SOCKET_NO_UPDATE_CHECK", "1")
1778-
.env("SOCKET_TELEMETRY_DISABLED", "1")
1779-
.env("VLT_TELEMETRY", "0")
1776+
cmd.env("VLT_TELEMETRY", "0")
17801777
.env("LANG", "C")
17811778
.env("LC_ALL", "C");
17821779
cmd

‎crates/socket-patch-cli/tests/vlt_hosted_common/mod.rs‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ pub fn with_flags(line: &str, flags: u8) -> String {
9191
line.replacen("[0,", &format!("[{flags},"), 1)
9292
}
9393

94+
#[path = "../common/hermetic.rs"]
95+
mod hermetic;
9496
#[path = "../common/oracle.rs"]
9597
mod oracle;
9698

@@ -418,32 +420,19 @@ pub fn ledger_path(root: &Path) -> PathBuf {
418420
root.join(".socket/vendor/redirect-state.json")
419421
}
420422

421-
/// The `socket-patch` binary with the ambient `SOCKET_*` and proxy
422-
/// environment scrubbed (telemetry opt-outs kept).
423+
/// The `socket-patch` binary with the hermetic `SOCKET_*` environment (see
424+
/// `hermetic::command`) and the ambient proxy environment scrubbed.
423425
pub fn scrubbed_cli() -> std::process::Command {
424-
let mut cmd = std::process::Command::new(env!("CARGO_BIN_EXE_socket-patch"));
425-
cmd.env("SOCKET_DRY_RUN", "true")
426-
.env("SOCKET_OFFLINE", "true")
427-
.env("SOCKET_NO_VLT_INSTALL_CLEANUP", "true")
428-
.env_remove("SOCKET_DRY_RUN")
429-
.env_remove("SOCKET_OFFLINE")
430-
.env_remove("SOCKET_NO_VLT_INSTALL_CLEANUP");
426+
let mut cmd = hermetic::binary_command();
431427
for (key, _) in std::env::vars_os() {
432428
let name = key.to_string_lossy();
433-
let proxy = name.eq_ignore_ascii_case("http_proxy")
434-
|| name.eq_ignore_ascii_case("https_proxy")
435-
|| name.eq_ignore_ascii_case("all_proxy")
436-
|| name.eq_ignore_ascii_case("no_proxy");
437-
if proxy
438-
|| (name.starts_with("SOCKET_")
439-
&& !name.contains("TELEMETRY")
440-
&& name != "SOCKET_NO_CONFIG")
429+
if ["http_proxy", "https_proxy", "all_proxy", "no_proxy"]
430+
.iter()
431+
.any(|p| name.eq_ignore_ascii_case(p))
441432
{
442433
cmd.env_remove(&key);
443434
}
444435
}
445-
cmd.env("SOCKET_NO_CONFIG", "1")
446-
.env("SOCKET_NO_UPDATE_CHECK", "1");
447436
cmd
448437
}
449438

‎crates/socket-patch-cli/tests/vlt_vendor_common/mod.rs‎

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
1919
#![allow(dead_code)]
2020

21+
#[path = "../common/hermetic.rs"]
22+
mod hermetic;
2123
#[path = "../prebuilt_common/mod.rs"]
2224
mod prebuilt_common;
2325

@@ -347,34 +349,20 @@ pub fn capture_project(root: &Path, case: &Case) {
347349

348350
// ── CLI ──────────────────────────────────────────────────────────────────
349351

350-
/// The `socket-patch` binary with the ambient `SOCKET_*`, `VLT_*` and proxy
351-
/// environment scrubbed (telemetry disabled).
352+
/// The `socket-patch` binary with the hermetic `SOCKET_*` environment (see
353+
/// `hermetic::command`) and the ambient `VLT_*` and proxy environment
354+
/// scrubbed.
352355
pub fn cli() -> Command {
353-
let mut cmd = Command::new(env!("CARGO_BIN_EXE_socket-patch"));
356+
let mut cmd = hermetic::binary_command();
354357
for (key, _) in std::env::vars_os() {
355-
let name = key.to_string_lossy().to_string();
358+
let name = key.to_string_lossy();
356359
let proxy = ["http_proxy", "https_proxy", "all_proxy", "no_proxy"]
357360
.iter()
358361
.any(|p| name.eq_ignore_ascii_case(p));
359-
if proxy
360-
|| name.starts_with("VLT_")
361-
|| (name.starts_with("SOCKET_") && name != "SOCKET_NO_CONFIG")
362-
{
362+
if proxy || name.starts_with("VLT_") {
363363
cmd.env_remove(&key);
364364
}
365365
}
366-
for key in [
367-
"SOCKET_OFFLINE",
368-
"SOCKET_DRY_RUN",
369-
"SOCKET_API_URL",
370-
"SOCKET_PROXY_URL",
371-
"SOCKET_NO_VLT_INSTALL_CLEANUP",
372-
] {
373-
cmd.env_remove(key);
374-
}
375-
cmd.env("SOCKET_TELEMETRY_DISABLED", "1")
376-
.env("SOCKET_NO_CONFIG", "1")
377-
.env("SOCKET_NO_UPDATE_CHECK", "1");
378366
cmd
379367
}
380368

‎crates/socket-patch-core/src/utils/poetry_lock.rs‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pub(crate) fn package_files(package: &Table) -> Vec<&dyn TableLike> {
3333

3434
/// The lock-wide `[metadata.files]` entry for package `name` (lock
3535
/// 1.0/1.1; keys compared PEP 503-canonical).
36-
pub(crate) fn metadata_files<'d>(lock: &'d DocumentMut, name: &str) -> Vec<&'d dyn TableLike> {
36+
pub(crate) fn metadata_files<'d>(lock: &'d Table, name: &str) -> Vec<&'d dyn TableLike> {
3737
let canon = canonicalize_pypi_name(name);
3838
let entry = lock
3939
.get("metadata")
@@ -52,6 +52,10 @@ pub(crate) fn metadata_files<'d>(lock: &'d DocumentMut, name: &str) -> Vec<&'d d
5252
/// Whether a `[metadata.files]` entry is laid out one file per line, which is
5353
/// how Poetry 1.0/1.1 write every NON-empty entry (tomlkit `multiline(True)`);
5454
/// an empty one is `[]`.
55+
///
56+
/// Reads the array's layout, so `item` must come from a `DocumentMut`: a
57+
/// read-only `toml_edit::Document` keeps layout as spans into its source
58+
/// and this would answer `false` for every entry.
5559
pub(crate) fn is_multiline_array(item: &Item) -> bool {
5660
item.as_array().is_some_and(|array| {
5761
let has_break = |raw: Option<&str>| raw.is_some_and(|s| s.contains('\n'));
@@ -135,13 +139,10 @@ fn multiline_files(files: Array) -> Item {
135139
/// bumps the minor additively — 2.0 → 2.1 kept every shape we rewrite, and the
136140
/// vendored loader already accepts newer minors with an advisory; the hosted
137141
/// path must not refuse what the vendored path accepts).
138-
pub fn lock_version(lock: &DocumentMut) -> Result<&str, String> {
139-
lock_version_of(lock)
140-
}
141-
142-
/// [`lock_version`] of a parsed lock's root table (a `DocumentMut` or a
143-
/// spanned `Document`).
144-
fn lock_version_of(lock: &Table) -> Result<&str, String> {
142+
///
143+
/// Takes the root table of either a `DocumentMut` or a read-only spanned
144+
/// `Document`.
145+
pub fn lock_version(lock: &Table) -> Result<&str, String> {
145146
let metadata = lock
146147
.get("metadata")
147148
.filter(|item| item.is_table_like())
@@ -616,7 +617,7 @@ fn rewrite_poetry_lock_batch(
616617
&names,
617618
|index, lock| {
618619
if version.is_none() {
619-
*version = lock_version_of(lock).ok().map(str::to_string);
620+
*version = lock_version(lock).ok().map(str::to_string);
620621
}
621622
let dep = &deps[index];
622623
let sha256 = checked_sha256(
@@ -690,7 +691,7 @@ fn plan_poetry_rewrite(
690691
source_url: &str,
691692
sha256: &str,
692693
) -> Result<Option<PoetryLockPlan>, String> {
693-
let format = lock_version_of(lock)?.to_string();
694+
let format = lock_version(lock)?.to_string();
694695
if format == "0" && source_type == "url" {
695696
return Err("Poetry 0.x ignores URL sources; hosted patches require Poetry >= 1.0".into());
696697
}

‎crates/socket-patch-core/src/utils/python_lock.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ impl ArtifactSource<'_> {
3434
/// (uv 0.2.x) or `package` (uv ≥ 0.2.35 and PEP 723 script locks). The
3535
/// lock inventory, lockfile discovery and `lock_inventory::
3636
/// wired_vendor_integrity` all pick the array with this one rule.
37-
pub(crate) fn lock_package_collection(doc: &DocumentMut) -> (&'static str, bool) {
37+
pub(crate) fn lock_package_collection(doc: &Table) -> (&'static str, bool) {
3838
if doc.contains_key("lock-version") {
3939
("packages", true)
4040
} else if doc.contains_key("distribution") {

‎crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use std::path::Path;
77

88
use serde_json::Value;
9-
use toml_edit::{DocumentMut, Item, TableLike};
9+
use toml_edit::{Item, Table, TableLike};
1010

1111
use crate::crawlers::python_crawler::canonicalize_pypi_name;
1212
use crate::formats::text::strip_bom;
@@ -326,7 +326,8 @@ pub(super) fn portable_wheel_artifact(
326326
}
327327

328328
pub(super) fn python_lock_inventory(text: &str) -> Option<Vec<LockfileEntry>> {
329-
let document: DocumentMut = text.parse().ok()?;
329+
// Read-only parse, as in lockfile discovery (`toml_or_diag`, #1473).
330+
let document = toml_edit::Document::parse(text).ok()?;
330331
let (collection, pep751) = lock_package_collection(&document);
331332
// Only the lock formats the fetch layer understands.
332333
let supported = if pep751 {
@@ -395,7 +396,7 @@ pub(super) fn python_lock_inventory(text: &str) -> Option<Vec<LockfileEntry>> {
395396
/// can be chosen there). A lock that is not TOML contributes nothing.
396397
async fn inventory_poetry_lock(view: &ProjectView<'_>) -> Option<Vec<LockfileEntry>> {
397398
let text = view.read_text("poetry.lock").await.ok()?;
398-
let document: DocumentMut = text.parse().ok()?;
399+
let document = toml_edit::Document::parse(text.as_str()).ok()?;
399400
let pure_wheel_sha = |files: Vec<&dyn TableLike>| {
400401
files.into_iter().find_map(|entry| {
401402
let file = entry.get("file")?.as_str()?;
@@ -429,7 +430,7 @@ async fn inventory_poetry_lock(view: &ProjectView<'_>) -> Option<Vec<LockfileEnt
429430

430431
/// `(canonical name, version, purl, table)` of every path-safe `[[package]]`
431432
/// of a poetry.lock / pdm.lock document.
432-
fn toml_package_coords(document: &DocumentMut) -> Vec<(String, String, String, &toml_edit::Table)> {
433+
fn toml_package_coords(document: &Table) -> Vec<(String, String, String, &Table)> {
433434
let Some(packages) = document.get("package").and_then(Item::as_array_of_tables) else {
434435
return Vec::new();
435436
};
@@ -588,7 +589,7 @@ async fn inventory_pipfile_lock(view: &ProjectView<'_>) -> Option<Vec<LockfileEn
588589
/// does not probe, so a lock-only vendored path would not survive a re-scan.
589590
async fn inventory_pdm_lock(view: &ProjectView<'_>) -> Option<Vec<LockfileEntry>> {
590591
let text = view.read_text("pdm.lock").await.ok()?;
591-
let document: DocumentMut = text.parse().ok()?;
592+
let document = toml_edit::Document::parse(text.as_str()).ok()?;
592593
let out: Vec<LockfileEntry> = toml_package_coords(&document)
593594
.into_iter()
594595
.map(|(name, version, purl, _)| LockfileEntry {

‎crates/socket-patch-core/src/vex/discover/cargo.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ async fn read_toml(ctx: &DiscoverCtx<'_>, file: &str, out: &mut Discovery) -> Op
312312
}
313313

314314
fn parse_toml(file: &str, text: &str, out: &mut Discovery) -> Option<DocumentMut> {
315-
toml_or_diag(file, text, TomlDiag::TrimEnd, out)
315+
toml_or_diag(file, text, TomlDiag::TrimEnd, out).map(toml_edit::Document::into_mut)
316316
}
317317

318318
/// The config file cargo actually reads ([`effective_config_rel_in`]), parsed,

0 commit comments

Comments
 (0)