diff --git a/crates/nub-cli/src/cli.rs b/crates/nub-cli/src/cli.rs index 806ccd4fa..0e516e326 100644 --- a/crates/nub-cli/src/cli.rs +++ b/crates/nub-cli/src/cli.rs @@ -3894,21 +3894,21 @@ fn run_file_in_dir(args: &[String], compat_mode: bool, cwd: &Path, exec_ua: bool }) }) .flatten(); - check_schema_usable(env_owner.as_ref(), &runtime)?; + // A spawn that LAUNCHES the loader is nub re-entered through its own PATH + // shim, not the invocation the user typed — the loader's bin is a + // `#!/usr/bin/env node` script, so it arrives here on its way to running. The + // outer process already refused anything contradictory; raising the conflict + // again here would refuse a run over flags that live one process up, and + // `--no-env-file` in particular does not survive the spawn while the config + // snapshot does. Only the diagnostic is skipped: `runtime_child_env` still + // gets the unfiltered owner, so nub feeds the loader none of its own cascade. + check_schema_usable( + env_owner + .as_ref() + .filter(|owner| !owner.launches_loader(args)), + &runtime, + )?; let mut env_vars = runtime_child_env(&runtime, project_root, compat_mode, env_owner.as_ref())?; - if let Some((_, schema_dir)) = env_owner - .as_ref() - .and_then(crate::env_owner::EnvOwner::spawn_target) - { - // Reaches the loader process AND the Node it spawns, so neither re-enters - // nub and wraps a second time. Carries the schema dir rather than a bare - // flag: a nested nub in a DIFFERENT schema-owned project must still wrap - // its own, instead of inheriting this project's environment silently. - env_vars.insert( - crate::env_owner::WRAPPED_ENV.to_string(), - crate::env_owner::wrapped_marker(schema_dir), - ); - } // Bin-exec parity with `nub run`: when this spawn is nub LAUNCHING a resolved // node bin (a `nubx`/`nub exec` scaffolder — `exec_ua`), set the same role- @@ -3952,9 +3952,14 @@ fn run_file_in_dir(args: &[String], compat_mode: bool, cwd: &Path, exec_ua: bool // `!compat_mode`, so `--node` skips it regardless). let pnp_ctx = nub_core::pnp::detect(cwd); let config = nub_core::node::spawn::SpawnConfig { - // Put the loader in front of Node when one owns this project. + // Put the loader in front of Node when one owns this project — unless the + // loader is what this very spawn runs, in which case it is about to own + // the environment for everything below it and a wrap would only resolve + // the project twice (and, when the script passed its own `--path` or + // `--filter`, resolve something the caller did not ask for). env_owner: env_owner .as_ref() + .filter(|owner| !owner.launches_loader(args)) .and_then(crate::env_owner::EnvOwner::spawn_target), node: &node, user_args: args, @@ -6290,6 +6295,11 @@ fn run_watch(file: &str, args: &[String]) -> Result { // supervisor re-execs the child inside it. Values therefore freeze across // restarts, which is the trade-off this path already makes for every // expansion-dependent var it injects. + // No `launches_loader` guard here, unlike the file-run path, and the asymmetry + // is deliberate: watch never launches the loader. The loader's own shebang + // `node` re-enters nub as a plain FILE RUN carrying the loader's entry, so + // that path is where the guard belongs; the Node this line spawns is the + // loader's direct child, launched by absolute path, and never comes back here. let mut cmd = match env_owner .as_ref() .and_then(crate::env_owner::EnvOwner::spawn_target) @@ -6301,10 +6311,6 @@ fn run_watch(file: &str, args: &[String]) -> Result { .arg(schema_dir) .arg("--") .arg(node.path.as_str()); - cmd.env( - crate::env_owner::WRAPPED_ENV, - crate::env_owner::wrapped_marker(schema_dir), - ); cmd } None => std::process::Command::new(node.path.as_str()), diff --git a/crates/nub-cli/src/env_owner.rs b/crates/nub-cli/src/env_owner.rs index a13182da2..95b69e35f 100644 --- a/crates/nub-cli/src/env_owner.rs +++ b/crates/nub-cli/src/env_owner.rs @@ -24,10 +24,29 @@ //! So nub's whole involvement is: notice, stand down, and put the loader in the //! spawn chain. It resolves nothing, injects nothing, and redacts nothing. //! +//! ## When NOT to put it in the chain +//! +//! Two cases, and between them they replace the `__NUB_ENV_OWNER_WRAPPED` marker +//! this module used to stamp. A marker only ever covered a loader nub itself +//! spawned; these cover every launcher. +//! +//! - **The loader already ran** — [`LOADER_ENV_BLOB`] is in the environment and +//! names a resolution anchored at or below the schema nub found. Adding a +//! second resolution on top of it is what made nub fire `exec()` resolvers a +//! `--filter` had excluded, and what made a script's own `--path` run die on +//! the root schema's validation before it ever executed. +//! - **The loader is what nub is about to run** — see [`launches_loader`]. No +//! blob exists yet at that moment, by construction, so this one cannot be a +//! sentinel: nub has to recognize the program. It doubles as the recursion +//! guard, and a structural one beats a flag, because the loader's bin is a +//! `#!/usr/bin/env node` script whose interpreter re-enters nub through the +//! PATH shim. +//! //! ## Replaceability //! -//! nub is expected to grow its own schema-driven loader. The only -//! loader-specific knowledge here is [`LOADER_PACKAGE`] and the `run` verb. +//! nub is expected to grow its own schema-driven loader. The loader-specific +//! knowledge here is [`LOADER_PACKAGE`], the `run` verb, and the shape of +//! [`LOADER_ENV_BLOB`]. use std::path::{Path, PathBuf}; @@ -43,13 +62,24 @@ pub(crate) const SCHEMA_FILE: &str = ".env.schema"; /// so one lookup covers every install shape. const LOADER_PACKAGE: &str = "varlock"; -/// Set on the loader process so a nested nub does not wrap again. +/// The blob the loader publishes to every child it launches. +/// +/// This is how nub learns the environment is already resolved, and reading the +/// LOADER'S OWN surface rather than a nub marker is what makes it general: a +/// marker only covers a loader nub itself spawned, while this covers a Makefile, +/// a CI wrapper, a standalone binary, or a bare shell invocation — none of which +/// nub can observe. Measured shape: /// -/// The loader's bin is a `#!/usr/bin/env node` script, so its own interpreter -/// resolves through nub's PATH shim and re-enters nub. Without this marker that -/// nub would detect the same project and wrap once more, without bound. -/// Internal `__NUB_*` plumbing, not a user knob. -pub(crate) const WRAPPED_ENV: &str = "__NUB_ENV_OWNER_WRAPPED"; +/// ```json +/// {"basePath": "/abs/dir", +/// "sources": [{"type": "schema", "path": ""}, …]} +/// ``` +/// +/// `basePath` and `sources` stay plain JSON even when the loader encrypts the +/// injected values, which covers the envelope's contents and not the envelope. +/// Anything nub cannot parse is treated as absent, so an unrecognized future +/// shape degrades to wrapping rather than to a silently empty environment. +const LOADER_ENV_BLOB: &str = "__VARLOCK_ENV"; /// A schema nub cannot act on, and why. Always fatal. /// @@ -110,7 +140,7 @@ pub(crate) fn explicit_env_file_conflict(source: &str) -> String { pub(crate) struct EnvOwner { root: PathBuf, cli: Option, - wrapped: bool, + already_resolved: bool, } impl EnvOwner { @@ -141,10 +171,15 @@ impl EnvOwner { /// therefore keep loading. pub(crate) fn suppresses_env_files(&self) -> bool { // True in BOTH owned states. `cli` is Some when this process will put the - // loader in front of Node; it is None-but-wrapped when a parent nub - // already did, and the values are already in this environment. Loading - // `.env*` in either case would layer nub's answer over the loader's. - self.cli.is_some() || self.wrapped + // loader in front of Node; `already_resolved` is true when the loader has + // run somewhere above and its values are already here. Loading `.env*` in + // either case would layer nub's answer over the loader's. + // + // Deliberately NOT gated on whether this particular launch will wrap: + // when nub declines because the loader itself is what it is launching + // ([`launches_loader`]), the loader still owns the environment, so nub + // must not feed its own cascade to the loader's own process. + self.cli.is_some() || self.already_resolved } /// Whether this `.env.schema` is one nub should act on at all. @@ -177,7 +212,7 @@ impl EnvOwner { /// not applied" is false while that tool is applying it correctly, and /// recommends a package it never asked for. pub(crate) fn schema_problem(&self) -> Option { - if self.wrapped || self.cli.is_some() || !self.is_ours() { + if self.already_resolved || self.cli.is_some() || !self.is_ours() { return None; } Some(if self.loader_declared() { @@ -249,47 +284,136 @@ pub(crate) fn detect(project_root: &Path, workspace_root: Option<&Path>) -> Opti .flatten() .find(|dir| dir.join(SCHEMA_FILE).is_file())? .to_path_buf(); - // Already behind the loader: do NOT wrap again. Its bin is a - // `#!/usr/bin/env node` script, so its own interpreter resolves through nub's - // PATH shim and re-enters nub — which would otherwise detect this same - // project and wrap once more, without bound. - let wrapped = wrapped_for(&root); + // Already behind the loader: do NOT resolve on top of it. + let already_resolved = already_resolved_for(&root); let mut owner = EnvOwner { root, cli: None, - wrapped, + already_resolved, }; // `is_ours` gates the hand-over, not just the diagnostic: a project that // declares a rival claimant of this filename gets neither. - if !wrapped && owner.is_ours() { + if !already_resolved && owner.is_ours() { owner.cli = find_loader_cli(project_root, workspace_root); } Some(owner) } -/// Whether a parent nub already put the loader in front of THIS project. +/// Resolve a path as far as the filesystem allows, so two spellings of one +/// directory compare equal. +/// +/// The loader reports an already-canonical `basePath` (`/private/tmp/…` on macOS) +/// while nub's own roots routinely are not (`/tmp/…`), and either side can carry a +/// symlink or a `..`. A path that cannot be canonicalized is compared as written, +/// which is the best available answer and never worse than not comparing. +fn canonical(path: &Path) -> PathBuf { + path.canonicalize().unwrap_or_else(|_| path.to_path_buf()) +} + +/// Whether the loader has ALREADY resolved the schema nub found here. +/// +/// The test is CONTAINMENT against the schema directory, not equality, and the +/// asymmetry is deliberate — each direction is a different question: /// -/// Comparing the root is what makes the marker safe. A bare "something wrapped" -/// flag stands down for any project reached from inside the wrap — so a run in a -/// second, differently-configured schema project would silently inherit the outer -/// project's environment, with its own schema never resolved and no warning, -/// because the missing-loader diagnostic is gated on this too. -fn wrapped_for(root: &Path) -> bool { - let Some(marked) = std::env::var_os(WRAPPED_ENV) else { +/// - `basePath` at or BELOW the schema dir means someone pointed the loader +/// inside this project (`run --path ./config`). Their entry point is more +/// specific than nub's, they chose it, and it resolves the same project. Stand +/// down. +/// - `basePath` ABOVE it means the loader resolved an ancestor while nub found a +/// nearer schema — a workspace root's run reaching a member that ships its own. +/// The member's schema is the one that has NOT been resolved, and a member +/// schema wins over the root's, so nub must still resolve it. +/// - Unrelated means a different project entirely: the case where standing down +/// on a bare "the loader ran" flag would hand project B project A's values. +/// +/// The `sources` scan then adds back the one case containment misses: a root +/// schema that `@import`s the member's own file HAS resolved it, even though the +/// root sits above. The loader lists every schema it read, each path relative to +/// `basePath`. +fn already_resolved_for(schema_dir: &Path) -> bool { + let Some(raw) = std::env::var_os(LOADER_ENV_BLOB) else { return false; }; - // Canonicalize both sides: the marker travels through a spawn, and a symlinked - // or `..`-relative root would otherwise compare unequal to the same directory. - let same = |a: &Path, b: &Path| match (a.canonicalize(), b.canonicalize()) { - (Ok(a), Ok(b)) => a == b, - _ => a == b, + let Some(text) = raw.to_str() else { + return false; }; - same(Path::new(&marked), root) + let Ok(blob) = serde_json::from_str::(text) else { + // Not JSON — an opaque or future envelope nub has no claim to interpret. + // Wrapping costs a second resolution; standing down on a blob nub cannot + // read would hand the program whatever that envelope happened to hold. + return false; + }; + let Some(base) = blob.get("basePath").and_then(serde_json::Value::as_str) else { + return false; + }; + let base = canonical(Path::new(base)); + if base.starts_with(canonical(schema_dir)) { + return true; + } + let schema = canonical(&schema_dir.join(SCHEMA_FILE)); + blob.get("sources") + .and_then(serde_json::Value::as_array) + .is_some_and(|sources| { + sources + .iter() + .filter(|source| { + source.get("type").and_then(serde_json::Value::as_str) == Some("schema") + }) + .filter_map(|source| source.get("path").and_then(serde_json::Value::as_str)) + .any(|path| canonical(&base.join(path)) == schema) + }) } -/// The value to stamp so a nested nub can tell WHICH project is wrapped. -pub(crate) fn wrapped_marker(root: &Path) -> String { - root.to_string_lossy().into_owned() +// Kept beside `already_resolved_for` rather than up in the main block: the two +// are the pair of stand-down rules, and reading either one without the other +// invites putting the loader in front of itself. +impl EnvOwner { + /// Whether the command nub is about to launch IS the loader's own CLI. + /// + /// This is the one stand-down that cannot be a sentinel. At this moment the + /// loader has not run, so it has published nothing; nub must recognize the + /// program instead. It is also the recursion guard, replacing the marker nub + /// used to stamp on the loader it spawned — the loader's bin is a + /// `#!/usr/bin/env node` script, so its interpreter comes back through nub's + /// PATH shim, finds the same schema, and would wrap again without bound. + /// + /// Two clauses, because no single one covers every install shape: + /// + /// - The bin nub RESOLVED, canonicalized. `node_modules/.bin` normally holds + /// a symlink into the package, so this and the next clause agree — but an + /// install that copies the entry there instead leaves this as the only + /// match. + /// - Any path inside a `node_modules//` directory. This is what + /// catches Windows, where `.bin` holds a generated `.cmd` shim that + /// resolves to itself and hands Node the package's entry; it also catches a + /// global install's `/lib/node_modules//…`, a pnpm store + /// path, an unplugged PnP path, and someone running the entry by hand. + /// + /// Flags are skipped, so a command that merely MENTIONS the loader — a + /// `--require` of something inside it — does not lose its wrap. + pub(crate) fn launches_loader(&self, args: &[String]) -> bool { + let cli = self.cli.as_deref().map(canonical); + args.iter().filter(|arg| !arg.starts_with('-')).any(|arg| { + let path = canonical(Path::new(arg)); + cli.as_ref().is_some_and(|cli| *cli == path) || in_loader_package(&path) + }) + } +} + +/// Whether a path lies inside a `node_modules//` directory. +fn in_loader_package(path: &Path) -> bool { + let mut components = path.components(); + while let Some(component) = components.next() { + if component.as_os_str() == "node_modules" + && components + .clone() + .next() + .is_some_and(|next| next.as_os_str() == LOADER_PACKAGE) + { + return true; + } + } + false } /// The loader CLI: the project's `node_modules/.bin` first, then `PATH`. @@ -352,32 +476,48 @@ mod tests { dir } - /// `PATH` is process-global, so every test that reads or writes it takes this - /// lock rather than racing a sibling. - fn path_lock() -> std::sync::MutexGuard<'static, ()> { + /// The environment is process-global, so every test that reads or writes + /// `PATH` or the loader's blob takes this lock rather than racing a sibling. + /// One lock covers both, because `with_env` sets both and a second lock would + /// only invite a nested-acquisition deadlock. + fn env_lock() -> std::sync::MutexGuard<'static, ()> { static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); LOCK.lock().unwrap_or_else(|err| err.into_inner()) } - fn with_path(dir: Option<&Path>, f: impl FnOnce() -> T) -> T { - let _guard = path_lock(); - let saved = std::env::var_os("PATH"); - unsafe { - match dir { - Some(dir) => std::env::set_var("PATH", dir), - None => std::env::remove_var("PATH"), + fn with_env(dir: Option<&Path>, blob: Option<&str>, f: impl FnOnce() -> T) -> T { + let _guard = env_lock(); + let saved_path = std::env::var_os("PATH"); + let saved_blob = std::env::var_os(LOADER_ENV_BLOB); + let set = |key: &str, value: Option<&std::ffi::OsStr>| unsafe { + match value { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), } - } + }; + set("PATH", dir.map(Path::as_os_str)); + set(LOADER_ENV_BLOB, blob.map(std::ffi::OsStr::new)); let out = f(); - unsafe { - match saved { - Some(value) => std::env::set_var("PATH", value), - None => std::env::remove_var("PATH"), - } - } + set("PATH", saved_path.as_deref()); + set(LOADER_ENV_BLOB, saved_blob.as_deref()); out } + fn with_path(dir: Option<&Path>, f: impl FnOnce() -> T) -> T { + with_env(dir, None, f) + } + + /// A loader blob resolved at `base`, listing the schema files at `schemas` + /// (paths relative to `base`, as the loader reports them). + fn blob(base: &Path, schemas: &[&str]) -> String { + let sources: Vec<_> = schemas + .iter() + .map(|path| serde_json::json!({"type": "schema", "path": path})) + .collect(); + serde_json::json!({"basePath": base.to_str().expect("utf8"), "sources": sources}) + .to_string() + } + /// The loader bin name this platform's lookup actually probes. fn bin_name() -> &'static str { if cfg!(windows) { @@ -531,4 +671,152 @@ mod tests { "a package shipping its own schema must use it, not the root's" ); } + + /// The loader wrote the blob for THIS schema, so a second resolution on top + /// would be nub asking again for an answer it already has — and asking with + /// its own flags rather than the caller's. + #[test] + fn a_loader_run_over_this_schema_stands_nub_down() { + let dir = project(&[ + (".env.schema", "# ---\nA=1\n"), + (&format!("node_modules/.bin/{}", bin_name()), "#!/bin/sh\n"), + ]); + for base in [dir.path().to_path_buf(), dir.path().join("config")] { + std::fs::create_dir_all(&base).expect("mkdir"); + let owner = with_env(None, Some(&blob(&base, &[".env.schema"])), || { + detect(dir.path(), None) + }) + .expect("schema present"); + assert_eq!( + owner.spawn_target(), + None, + "a resolution anchored at {} must not be wrapped in a second one", + base.display() + ); + assert!( + owner.suppresses_env_files(), + "the loader owns the environment, so nub's own cascade stays off" + ); + } + } + + /// The sharp one: standing down here would hand the member the ROOT's + /// environment and never resolve its own schema, silently — which is what a + /// bare "the loader ran" flag would have done. + #[test] + fn a_loader_run_above_a_member_schema_does_not_stand_it_down() { + let dir = project(&[ + (".env.schema", "# ---\nROOT=1\n"), + (&format!("node_modules/.bin/{}", bin_name()), "#!/bin/sh\n"), + ("pkgs/web/package.json", r#"{"name":"web"}"#), + ("pkgs/web/.env.schema", "# ---\nMEMBER=1\n"), + ]); + let member = dir.path().join("pkgs/web"); + let owner = with_env(None, Some(&blob(dir.path(), &[".env.schema"])), || { + detect(&member, Some(dir.path())) + }) + .expect("member schema"); + assert!( + owner.spawn_target().is_some(), + "the member's own schema is the one that has NOT been resolved" + ); + } + + #[test] + fn a_loader_run_in_another_project_does_not_stand_nub_down() { + let other = project(&[]); + let dir = project(&[ + (".env.schema", "# ---\nA=1\n"), + (&format!("node_modules/.bin/{}", bin_name()), "#!/bin/sh\n"), + ]); + let owner = with_env(None, Some(&blob(other.path(), &[".env.schema"])), || { + detect(dir.path(), None) + }) + .expect("schema present"); + assert!( + owner.spawn_target().is_some(), + "another project's resolution says nothing about this one" + ); + } + + /// Containment alone would miss this: the root sits ABOVE the member, but it + /// `@import`ed the member's file, so that schema really has been resolved. + #[test] + fn an_imported_schema_counts_as_already_resolved() { + let dir = project(&[ + ( + ".env.schema", + "# @import(\"./pkgs/web/.env.schema\")\n# ---\n", + ), + (&format!("node_modules/.bin/{}", bin_name()), "#!/bin/sh\n"), + ("pkgs/web/package.json", r#"{"name":"web"}"#), + ("pkgs/web/.env.schema", "# ---\nMEMBER=1\n"), + ]); + let member = dir.path().join("pkgs/web"); + let sources = [".env.schema", "pkgs/web/.env.schema"]; + let owner = with_env(None, Some(&blob(dir.path(), &sources)), || { + detect(&member, Some(dir.path())) + }) + .expect("member schema"); + assert_eq!( + owner.spawn_target(), + None, + "the loader listed this member's schema among the files it read" + ); + } + + /// Degrade toward resolving, never toward an empty environment: a blob nub + /// cannot read may hold anything, including another project's values. + #[test] + fn a_blob_nub_cannot_read_is_treated_as_absent() { + let dir = project(&[ + (".env.schema", "# ---\nA=1\n"), + (&format!("node_modules/.bin/{}", bin_name()), "#!/bin/sh\n"), + ]); + for opaque in ["varlock:v1:ZW5jcnlwdGVk", "{}", "not json at all"] { + let owner = + with_env(None, Some(opaque), || detect(dir.path(), None)).expect("schema present"); + assert!( + owner.spawn_target().is_some(), + "an unreadable blob ({opaque}) must not be trusted to have resolved anything" + ); + } + } + + /// The other stand-down: nub is LAUNCHING the loader, so it must not put the + /// loader in front of it. Also the recursion guard — the loader's own + /// interpreter comes back through the PATH shim and finds this same schema. + #[test] + fn the_loaders_own_cli_is_recognized_wherever_it_lives() { + let dir = project(&[ + (".env.schema", "# ---\nA=1\n"), + (&format!("node_modules/.bin/{}", bin_name()), "#!/bin/sh\n"), + ]); + let owner = with_path(None, || detect(dir.path(), None)).expect("schema present"); + let bin = dir.path().join("node_modules/.bin").join(bin_name()); + + let entries = [ + // The bin nub resolved — the shape an install leaves when it copies + // the entry into `.bin` instead of symlinking it. + bin.to_string_lossy().into_owned(), + // Inside the package, wherever the package lives. + format!("/app/node_modules/{LOADER_PACKAGE}/bin/cli.js"), + format!("/usr/local/lib/node_modules/{LOADER_PACKAGE}/bin/cli.js"), + ]; + for entry in &entries { + assert!( + owner.launches_loader(&["--enable-source-maps".into(), entry.into(), "run".into()]), + "{entry} is the loader's own code, whoever invoked it" + ); + } + assert!( + !owner.launches_loader(&["/app/src/index.js".into(), "--path".into()]), + "an ordinary entry file must still be wrapped" + ); + assert!( + !owner.launches_loader(&[format!("--require=/app/node_modules/{LOADER_PACKAGE}/x.js")]), + "a flag is not an entry point — matching one would strip the wrap from \ + any command that merely mentions the loader" + ); + } } diff --git a/crates/nub-cli/tests/env_owner.rs b/crates/nub-cli/tests/env_owner.rs index 3be2a3ec9..5e35bd209 100644 --- a/crates/nub-cli/tests/env_owner.rs +++ b/crates/nub-cli/tests/env_owner.rs @@ -80,6 +80,13 @@ if (sep < 0) {{ const flags = argv.slice(1, sep); const cmd = argv.slice(sep + 1); const pathIdx = flags.indexOf("--path"); +// A real loader publishes its resolution to every child, and that blob is what +// tells a nub further down the chain to stand down instead of resolving again. +// `basePath` tracks `--path` and is absolute — both measured against varlock +// 1.16.1, and both load-bearing for the containment test nub applies to it. +const basePath = require("node:path").resolve( + pathIdx < 0 ? process.cwd() : flags[pathIdx + 1], +); const res = spawnSync(cmd[0], cmd.slice(1), {{ stdio: "inherit", env: {{ @@ -87,6 +94,10 @@ const res = spawnSync(cmd[0], cmd.slice(1), {{ FROM_LOADER: "yes", LOADER_SAW_NODE: /node/.test(cmd[0]) ? "yes" : "no", LOADER_PATH: pathIdx < 0 ? "" : flags[pathIdx + 1], + __VARLOCK_ENV: JSON.stringify({{ + basePath, + sources: [{{ type: "schema", path: ".env.schema" }}], + }}), }}, }}); process.exit(res.status ?? 1); @@ -150,6 +161,20 @@ fn which_node_dir() -> PathBuf { .to_path_buf() } +/// The blob a real loader publishes to every child, standing for "the schema at +/// `base` is already resolved". +/// +/// Nub reads the LOADER's variable rather than a marker of its own, so a test +/// that fakes being behind the loader has to speak the loader's shape. Built +/// through `serde_json` so a Windows path's backslashes survive. +fn resolved_blob(base: &Path) -> String { + serde_json::json!({ + "basePath": base, + "sources": [{"type": "schema", "path": ".env.schema"}], + }) + .to_string() +} + #[test] fn without_a_schema_nub_loads_env_files_as_before() { let dir = project(&[(".env", "FROM_DOTENV=yes\n")]); @@ -276,8 +301,8 @@ fn nub_watch_also_puts_the_loader_in_front_of_node() { fn a_watcher_inside_the_loader_does_not_load_config_sources() { // `run_watch` builds its own env instead of going through `runtime_child_env`, // so gating that function's `Sources` arm left this copy of the same hole open. - // Reachable because the wrap marker is INHERITED: any `nub watch` started by a - // program already running behind the loader arrives here owned, and used to + // Reachable because the loader's blob is INHERITED: any `nub watch` started by + // a program already running behind the loader arrives here owned, and used to // load `envFile` sources the outer refusal had no chance to see. let dir = project(&[ (".env.schema", "# ---\nA=1\n"), @@ -290,7 +315,7 @@ fn a_watcher_inside_the_loader_does_not_load_config_sources() { .args(["watch", "probe.mjs"]) .current_dir(dir.path()) .env("PATH", which_node_dir()) - .env("__NUB_ENV_OWNER_WRAPPED", &root) + .env("__VARLOCK_ENV", resolved_blob(&root)) .env_remove("NODE_OPTIONS") .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) @@ -356,7 +381,7 @@ fn the_node_command_reaches_the_loader_intact() { fn the_loader_is_invoked_exactly_once() { // The loader's real bin is a `#!/usr/bin/env node` script, so its own // interpreter resolves through nub's PATH shim and re-enters nub. Without the - // wrapped-marker that nub detects the same project and wraps again, forever. + // stand-down rules that nub detects the same project and wraps again, forever. let dir = project(&[(".env.schema", "# ---\nA=1\n")]); let tally = dir.path().join("tally"); install_stub_loader(dir.path(), &tally); @@ -388,9 +413,9 @@ fn a_nested_nub_behind_the_loader_does_not_load_env_files() { .arg("probe.mjs") .current_dir(dir.path()) .env("PATH", &node_dir) - // The marker carries the wrapped project's root, so a nested nub can tell + // The blob carries the resolved project's root, so a nested nub can tell // "this project is already behind the loader" from "some other one is". - .env("__NUB_ENV_OWNER_WRAPPED", dir.path()) + .env("__VARLOCK_ENV", resolved_blob(dir.path())) .env_remove("NODE_OPTIONS") .output() .expect("spawn nub"); @@ -578,8 +603,11 @@ fn a_different_project_inside_the_wrap_still_wraps_its_own() { let output = Command::new(nub_binary()) .arg("probe.mjs") .current_dir(dir.path()) - // A parent nub wrapped a DIFFERENT project. - .env("__NUB_ENV_OWNER_WRAPPED", "/somewhere/else/entirely") + // The loader ran for a DIFFERENT project. + .env( + "__VARLOCK_ENV", + resolved_blob(Path::new("/somewhere/else/entirely")), + ) .env("PATH", &node_dir) .env_remove("NODE_OPTIONS") .output() @@ -760,3 +788,75 @@ fn compat_mode_does_no_owner_handling_at_all() { "and must not invoke the loader at all in compat mode" ); } + +#[cfg(unix)] +#[test] +fn a_script_that_runs_the_loader_itself_gets_one_resolution() { + // The defect this guards, measured against real varlock 1.16.1 before the + // fix: an existing project that already wired the loader into its own scripts + // got a SECOND resolution inserted in front of the one it asked for. That + // second one carries nub's arguments, not the script's, so it re-ran `exec()` + // resolvers a `--filter` had excluded, and — where the script passed its own + // `--path` — died on the root schema's validation before the script's own + // invocation ever ran. `npm run` exited 0 on the same project; nub exited 1. + // + // Two rules have to hold together for the count to be 1. nub must not wrap + // the loader it is LAUNCHING, and the nub that the loader's own `node` child + // re-enters must read the loader's published blob and stand down. + let dir = project(&[ + (".env.schema", "# ---\nA=1\n"), + ( + "package.json", + r#"{"name":"fx","version":"1.0.0","scripts":{"go":"varlock run -- node probe.mjs"}}"#, + ), + ]); + let tally = dir.path().join("tally"); + install_stub_loader(dir.path(), &tally); + + // A script needs `sh` as well as `node`; nub prepends the project's own + // `node_modules/.bin`, which is where the script's `varlock` comes from. + let path = std::env::join_paths([which_node_dir(), PathBuf::from("/bin"), "/usr/bin".into()]) + .expect("join PATH"); + let output = Command::new(nub_binary()) + .args(["run", "go"]) + .current_dir(dir.path()) + .env("PATH", path) + .env_remove("NODE_OPTIONS") + .output() + .expect("spawn nub run"); + let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); + let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); + assert!( + output.status.success(), + "nub run exited {:?}\nstdout: {stdout}\nstderr: {stderr}", + output.status.code() + ); + + let runs = std::fs::read_to_string(&tally) + .unwrap_or_default() + .lines() + .count(); + assert_eq!( + runs, 1, + "the script asked for one resolution; {runs} means nub inserted its own \ + in front of it.\nstdout: {stdout}\nstderr: {stderr}" + ); + + // A positive control on the count: the script's own invocation really did + // reach Node, so `1` is one WORKING resolution rather than none plus a stray. + let probe = stdout + .lines() + .rev() + .find(|line| line.trim_start().starts_with('{')) + .unwrap_or_else(|| panic!("no probe JSON in stdout: {stdout}")); + let run = Run { + stdout: probe.to_string(), + stderr, + }; + assert_eq!( + run.var("FROM_LOADER").as_deref(), + Some("yes"), + "the script's own loader invocation must still feed Node. stderr: {}", + run.stderr + ); +} diff --git a/site/content/docs/runtime/varlock.mdx b/site/content/docs/runtime/varlock.mdx index ddf4689dd..b0ea487d8 100644 --- a/site/content/docs/runtime/varlock.mdx +++ b/site/content/docs/runtime/varlock.mdx @@ -32,6 +32,16 @@ nub index.ts nub run dev ``` +Varlock invocations you write yourself run exactly as written. A script that calls Varlock directly keeps its own flags, and Nub puts nothing in front of it. + +```json title="package.json" +{ + "scripts": { + "build": "varlock run --path ./config -- node build.js" + } +} +``` + If you're in a monorepo sub-package, Nub checks the package root for `.env.schema` first, then the workspace root. diff --git a/wiki/research/preload-ecosystem.md b/wiki/research/preload-ecosystem.md index 8525dcd02..02b538f7f 100644 --- a/wiki/research/preload-ecosystem.md +++ b/wiki/research/preload-ecosystem.md @@ -333,7 +333,7 @@ The survey's coexistence playbook, checked against the code: | Pattern found in the wild | Nub's existing implementation | |---|---| | `tsx` reads **both** `NODE_OPTIONS` and `execArgv` to pick its tier | `shouldAutoAsyncTierAtPreload()` = `nodeHookComposeBroken() && foreignAsyncLoaderFlagPresent()` ([`preload-common.cjs:280`](../../runtime/preload-common.cjs)), reading both channels, plus the launcher's predictive argv scan | -| Sentinel env var rather than stripping `NODE_OPTIONS` (dd-trace's forced retreat) | `__NUB_ENV_OWNER_WRAPPED`, and `is_reentrant_in` keying on Nub's own token ([`spawn.rs:809`](../../crates/nub-core/src/node/spawn.rs)) | +| Sentinel env var rather than stripping `NODE_OPTIONS` (dd-trace's forced retreat) | `is_reentrant_in`, keying on Nub's own token ([`spawn.rs:809`](../../crates/nub-core/src/node/spawn.rs)). The env-owner path minted a second sentinel of Nub's own until 2026-08-12 and now reads Varlock's `__VARLOCK_ENV` — a sentinel the *other* tool already publishes beats one you mint, because it also covers launchers you never see ([`varlock-integration.md`](varlock-integration.md)) | | Absolute paths only, never bare/relative specifiers | Nub emits absolute paths and `file://` URLs | | Append to a pre-existing `NODE_OPTIONS`, never assign | Nub appends ([`spawn.rs:1086`](../../crates/nub-core/src/node/spawn.rs)) | | Yarn PnP needs its token installed first | PnP token pushed before Nub's own ([`spawn.rs:1098`](../../crates/nub-core/src/node/spawn.rs)) | diff --git a/wiki/research/varlock-integration.md b/wiki/research/varlock-integration.md index 51e6f2349..efe6d2a13 100644 --- a/wiki/research/varlock-integration.md +++ b/wiki/research/varlock-integration.md @@ -30,7 +30,7 @@ Why this beats the in-process design it replaced: Three non-obvious points, each found by running it rather than reading it: 1. **The `nub run`, `nubx`, `nub watch` and lifecycle-script paths need no wrapping code.** Their `node` resolves through nub's PATH shim, re-enters Nub, and gets wrapped there. This deleted the marker-stamping-across-five-launch-paths problem wholesale. -2. **The guard marker `__NUB_ENV_OWNER_WRAPPED` must be nub's own, never `__VARLOCK_RUN`.** Keying it off varlock's variable would make a user's `varlock run -- nub` in a *different* directory silently serve the outer project's environment. Measured; see §"varlock's env vars". +2. **A bare "varlock ran" flag is not a usable guard.** A user's `varlock run -- nub` in a *different* directory would silently serve the outer project's environment. What the guard must compare is *which schema* was resolved. Nub carried its own `__NUB_ENV_OWNER_WRAPPED` marker for that until 2026-08-12 and now reads varlock's `__VARLOCK_ENV`; see §"varlock's env vars". 3. **The `--path` flag is load-bearing.** Without it a workspace member dies with `No .env files found in …/pkgs/web` against a schema Nub had just found at the root — Nub walked up to decide the loader owns the project, so it must say where it walked to. Also fixes `cd src && nub app.js`. ## Why nub cannot keep its own cascade @@ -574,32 +574,68 @@ Fixtures used: `/tmp/vlk` (single package), `/tmp/vlkmono` (workspace). Both are Reproducing the fork bomb needs care: bound it with a short `timeout` and clean up with `pkill -f "varlock load"`. Do **not** bound it with `ulimit -u` — that limit counts all of the user's existing processes, so on a busy host it fires before anything runs and produces a false positive. -## varlock's env vars, and why nub does not skip on them +## varlock's env vars, and how nub reads them -`varlock run` sets exactly **two** variables on its child (measured, not read from source): +`varlock run` sets exactly **two** variables on its child, alongside the resolved values (measured 2026-08-12 by diffing a child's whole environment against a bare `node`, not read from source): -- `__VARLOCK_ENV` — the serialized graph, which contains **`basePath`**, the directory it resolved from +- `__VARLOCK_ENV` — the serialized graph - `__VARLOCK_RUN=1` +`__VARLOCK_ENV` names the schema it resolved, which is the fact the whole integration now turns on: + +```json +{ "basePath": "/abs/dir", + "sources": [ { "type": "container", "label": "directory - /abs/dir" }, + { "type": "schema", "label": ".env.schema", "path": ".env.schema" } ] } +``` + +`join(basePath, path)` is the absolute schema path. Measured properties, each one load-bearing: + +| property | measured | +| --- | --- | +| `basePath` is absolute and cwd-independent | Yes. Running from a subdirectory does not move it. `label` IS cwd-relative — use `path` | +| `--path` moves `basePath` | Yes: `--path ./config` gives `…/config`. Several `-p` flags collapse to one `basePath` | +| `@import` is visible | Yes — each imported schema gets its own `sources` entry | +| Encryption hides it | No. With `encryptInjectedEnv: true` confirmed on, the envelope stayed plain JSON | +| In-process `auto-load` publishes it | Yes, into `process.env`, and children inherit it | + Every name in the source, for reference: `_VARLOCK_ENV_KEY`, `_VARLOCK_FILTER`, `_VARLOCK_REDACT_STDOUT`, `_VARLOCK_CACHE_KEY`, `__VARLOCK_INTEGRATION`, `__VARLOCK_EXECUTION_PHASE`, `_VARLOCK_USE_INJECTED_ENV`, `_VARLOCK_THROW_ON_LOAD_ERROR`, `_VARLOCK_FORCE_KILL_TIMEOUT_MS`, `__VARLOCK_SEA_BUILD__`, `__VARLOCK_BUILD_TYPE__`, `_VARLOCK_DYNAMIC_BUILD_ACCESS_MODE`, `_VARLOCK_FORCE_FILE_ENCRYPTION_FALLBACK`, and a proxy-session family. Convention (`auto-load.ts:124-127`): `__` = varlock-set, `_` = user-controllable. -**DECIDED (2026-08-02): nub does not use these to skip re-invoking varlock.** Measured the nested case — project A's `varlock run` wrapping a Nub that runs in project B: +**REVERSAL (2026-08-12): nub now skips on `__VARLOCK_ENV`, and its own `__NUB_ENV_OWNER_WRAPPED` marker is deleted.** The 2026-08-02 decision rested on three claimed failure modes of a `basePath` comparison. Re-measured against varlock 1.16.1, two of them are false: + +| 2026-08-02 claim | 2026-08-12 measurement | +| --- | --- | +| An encrypted blob is opaque `varlock:v1:…`, so `basePath` is unreadable | **False.** Encryption covers the values, not the envelope. The opaque form did not reproduce at all | +| `--path` gives the same `basePath` | **False.** It moves `basePath` to the path given | +| `_VARLOCK_FILTER` gives the same `basePath` with a subset injected | **True**, and now a deliberate choice — see below | + +The nested measurement that drove the original decision still stands and is what fixes its shape. Project A's `varlock run` wrapping a Nub in project B: | | value | | --- | --- | | control, nub in B alone | `GREETING=from-B`, `ONLY_A=null` | | A's `varlock run` → nub in B | `GREETING=from-B`, `ONLY_A=yes` | -Double resolution is **idempotent for schema-declared values** — B's schema wins. (`ONLY_A` rides along as ambient, which is varlock's own passthrough behaviour and identical to `varlock run -- varlock run -- node`.) Skipping on `__VARLOCK_RUN` alone would have given B **A's** `GREETING`. A `basePath` comparison would catch that, but it is necessary and not sufficient: +Skipping on a bare `__VARLOCK_RUN` would have given B **A's** `GREETING`. So the test is not "did the loader run" but "was THIS schema resolved" — containment of `basePath` against the schema directory Nub found, plus a scan of the blob's schema `sources`: + +| chain | `basePath` | Nub found | result | +| --- | --- | --- | --- | +| `varlock run -- nub app.js` | `/repo` | `/repo` | equal → stand down | +| `varlock run --path ./config -- nub app.js` | `/repo/config` | `/repo` | below → stand down; the caller's entry point is the more specific one | +| workspace-root run, Nub in a member with its own schema | `/repo` | `/repo/pkgs/web` | above → resolve; the member's schema is the unresolved one, and a member schema wins | +| project A's run, Nub in project B | A's dir | B's dir | unrelated → resolve | +| root schema `@import`s the member's | `/repo` | `/repo/pkgs/web` | above, but LISTED in `sources` → stand down | + +Two deliberate calls inside this: -1. An **encrypted blob** (`@encryptInjectedEnv`) is opaque `varlock:v1:…` — `basePath` is unreadable. -2. **`_VARLOCK_FILTER`** on the outer run ⇒ same `basePath`, a *subset* of variables injected. -3. **`--path`** or other outer flags ⇒ same `basePath`, different resolution. +- **An outer `--filter` is honored** — Nub stands down and the child gets the filtered subset. The 2026-08-02 note treated that as a failure mode; it is an explicit user flag, and the same principle already makes an explicit `--env-file` beat Nub's auto-discovery. Nub never parses the schema, so it could not distinguish "filtered" from "a small schema" in any case. +- **An unreadable blob degrades toward resolving**, never toward an empty environment. -Each failure mode is a silent wrong or incomplete environment, to save ~0.15 s on a deliberate and uncommon invocation. Not worth it. +What no sentinel can cover: the Node process that HOSTS an in-process `auto-load` (a Vite plugin). Nub decides before that process starts, so nothing has been published yet. Only a config opt-out reaches it, and none exists. ## Changelog +- 2026-08-12 — **REVERSAL: nub reads varlock's own `__VARLOCK_ENV` instead of stamping `__NUB_ENV_OWNER_WRAPPED`, which is deleted.** Prompted by the author, who asked whether varlock already publishes the schema path. It does — a typed `sources` entry, `join(basePath, path)` — and two of the three objections the 2026-08-02 decision raised against reading it do not reproduce: an encrypted blob keeps a plain-JSON envelope, and `--path` does move `basePath`. Reading the loader's surface rather than a nub marker covers every launcher, including a Makefile, a CI wrapper, and a standalone binary, none of which nub can observe. A second rule covers the one moment no sentinel can, when nub is LAUNCHING the loader and nothing has been published yet: nub recognizes the loader's own entry and does not put it in front of itself. Fixes two measured defects — a script's `varlock run --path ./config -- …` exited 1 under nub against 0 under npm, because nub's inserted root-anchored resolution failed validation first; and a `--filter`ed run fired an `exec()` resolver the filter had excluded (npm 0 fires, nub 1). Both rules verified by disabling each and watching the regression test go red. NOT fixed, and still open: the per-node-process cost (three nested `nub run` calls still boot varlock three times), and the absence of any opt-out short of `--node`. - 2026-08-07 — **REVERSAL: the `@env-spec` content sniff is removed.** Ownership is decided by whether varlock RESOLVES (`node_modules/.bin` up to the workspace root, then `PATH`), with a declared `dotenv-extended` as the sole carve-out. nub no longer reads the schema at all. Recorded in full under the contested-filename section above. - 2026-08-02 — **REVERSAL: the in-process design is replaced by `nub → varlock run → node`.** Maintainer's call, on parity grounds: varlock only reaches full capability when invoked as