Fix replace-registry-host ${VAR} warning (#1233)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -161,7 +161,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc
 
 **npm hosted-mode `allow-remote` contract**: npm >=12 defaults `allow-remote=none` and refuses (EALLOWREMOTE) every lockfile entry whose `resolved` tarball is not served by the configured registry — exactly what a hosted redirect writes into `package-lock.json` / `npm-shrinkwrap.json`. Whenever a run leaves a ROOT npm lock carrying a granted hosted artifact URL (spliced this run, or already redirected by an earlier one — a missed config heals on re-run), the CLI ensures `allow-remote=all` in the project-root `.npmrc`: the file is created holding exactly `allow-remote=all\n` when absent, otherwise one `allow-remote=all` line is spliced in after the last non-empty top-level line (before any ini `[section]` header), in the file's own line ending, with the BOM, CRLF and trailing-newline shape preserved. The write lands in `redirect.rewrittenFiles` (edit kind `redirect_npmrc_allow_remote`), respects `--dry-run` (nothing written; the warning says what would be — including for a vendored → hosted takeover the dry run only previews), and is disabled by `--no-npm-allow-remote-config` / `SOCKET_NO_NPM_ALLOW_REMOTE_CONFIG`. The `.npmrc` grammar is npm's own `ini` parser's (cross-checked against it): lines split on any run of `\r` / `\n` (a bare `\r` ends a line), only the exact key `allow-remote` counts after ini unquoting (npm ignores `allow_remote` / `ALLOW-REMOTE` in a `.npmrc`; such a line is left alone and the real key appended), comment lines are ignored, a `[section]` header is recognized only as npm does — on the UNTRIMMED line (an indented or BOM-prefixed `[sec]` is a plain top-level key) — and ends the top-level scope, quotes and inline comments are stripped, the LAST top-level assignment wins, and the value is case-sensitive. An explicit other value (`allow-remote=none` / `root` / anything but `all`) is RESPECTED and never rewritten — the pnpm `trustLockfile: false` precedent — in the project `.npmrc` AND in every other npm config layer npm would consult: an `npm_config_allow_remote` environment variable (any spelling npm normalizes; it beats every `.npmrc`, so a project write could not take effect), and — when the project file sets nothing — the user (`npm_config_userconfig` / `~/.npmrc`), global (`npm_config_globalconfig` / `<prefix>/etc/npmrc`, prefix from `npm_config_prefix`, the user/builtin config, `PREFIX` or the `node` binary's install root) and builtin (npm's own `npmrc` beside the `node` binary: `<dir>/lib/node_modules/npm/npmrc`, `<dir>\node_modules\npm\npmrc` on Windows) config files — path values `${VAR}`-expanded and `~`-expanded like npm, env names case-insensitive on Windows, where a committed project line would silently override a machine / org policy. A symlinked, non-regular or unreadable `.npmrc`, or one with bare-`\r` line endings (npm splits on them, the line splice does not), is left untouched. Every variant emits the `redirect_npm_allow_remote` warning (written / would write / already set / explicit value respected — naming the project file, the env var, or the user/global/builtin config path — / opted out / unreadable or unsupported), always with the tradeoff: `allow-remote=all` lets npm install ANY url-resolved dependency, not just Socket's patched ones, while the per-entry sha512 integrity pins stay enforced; the remedy for the non-writing variants is `allow-remote=all` in `.npmrc` or `npm ci --allow-remote=all`. npm <=11 is unaffected (11 defaults to `all`, <=10 has no such setting). **Unwind (v5.0: no ledger)**: once `rollback`, `remove` or a hosted → vendored takeover has restored the last hosted entry of the root `package-lock.json` / `npm-shrinkwrap.json` to its upstream registry entry (see "Hosted unwind coverage"), a project `.npmrc` holding exactly `allow-remote=all\n` (the file hosted mode creates) is deleted; any other `.npmrc` that still has a top-level `allow-remote=all` line is left untouched and the `npm_allow_remote_left` warning says the line may be removed if nothing else needs it (v5 keeps no record of whether hosted mode added it, so it is never removed behind the user's back). The rewrite's stage file is created with the `.npmrc`'s own permission bits (a 0600 token-bearing file is never staged world-readable). **Vendored mode is unaffected by `allow-remote`**: its `file:.socket/vendor/…` resolutions are npm `file` specs, which npm >= 11.14 gates by `allow-file` (default `all`), never `allow-remote` — verified by the real npm 12 vendored matrix. **npm vendored `allow-file` contract (#969)**: `allow-file=none` refuses every `file:` dependency, and any other non-`all` value (`root`) admits one only when the project root or a workspace declares it and resolves to that very lock node (arborist's `_isRoot`); otherwise every `npm ci` / `npm install` fails EALLOWFILE. The effective value is read from the same npm config layers as `allow-remote` (an `npm_config_allow_file` environment variable, then the project `.npmrc`, then the user / global / builtin config files). When it refuses a `package-lock.json` / `npm-shrinkwrap.json` instance of a vendored `name@version` (the set vendoring rewires), the vendored run (`vendor`, `scan` / `get --mode vendored`, dry runs and in-sync re-runs included) still vendors, leaves the setting untouched, and records the `vendor_npm_allow_file` advisory naming the setting's source, the refused lock entries and the remedy (`allow-file=all` in the project `.npmrc`, unsetting the env var, or `npm ci --allow-file=all`); `vendor --check` fails that entry `vendor_check_failed` with the same reason. Other npm-family flavors (yarn, pnpm, bun, vlt) do not read `allow-file` and are not checked.
 
-**npm hosted-mode `replace-registry-host` contract**: npm >=8 rewrites the origin of a lock's `resolved` URL to the configured registry when its `replace-registry-host` setting is `always`, or equals the URL's hostname (`npmjs`, the default, stands for `registry.npmjs.org`; `never` matches nothing). A hosted pin rewritten that way is fetched from `<registry>/patch/npm/…` and every `npm ci` / `npm install` fails E404 (closed: never unpatched bytes; a warm npm cache can hide it locally). Whenever a run leaves a root npm lock carrying a granted hosted artifact URL (the same trigger as the `allow-remote` contract, so an "already on hosted patches" re-run reports it too), the CLI resolves `replace-registry-host` across npm's layers — an `npm_config_replace_registry_host` environment variable (any spelling npm normalizes), then the project `.npmrc`, then the user / global / builtin config files (located exactly like the `allow-remote` layers) — and, when the effective value rewrites a pinned host, emits the `redirect_npm_replace_registry_host` warning naming the layer (env var / project file / config path) and the remedies: `replace-registry-host=npmjs` in the project `.npmrc` (or unsetting the env var), or vendored mode, whose `file:` resolutions npm never rewrites. The setting is never rewritten; the redirect itself still lands and the exit status is unchanged.
+**npm hosted-mode `replace-registry-host` contract**: npm >=8 rewrites the origin of a lock's `resolved` URL to the configured registry when its `replace-registry-host` setting is `always`, or equals the URL's hostname (`npmjs`, the default, stands for `registry.npmjs.org`; `never` matches nothing). A hosted pin rewritten that way is fetched from `<registry>/patch/npm/…` and every `npm ci` / `npm install` fails E404 (closed: never unpatched bytes; a warm npm cache can hide it locally). Whenever a run leaves a root npm lock carrying a granted hosted artifact URL (the same trigger as the `allow-remote` contract, so an "already on hosted patches" re-run reports it too), the CLI resolves `replace-registry-host` across npm's layers — an `npm_config_replace_registry_host` environment variable (any spelling npm normalizes), then the project `.npmrc`, then the user / global / builtin config files (located exactly like the `allow-remote` layers), its value env-replaced the way npm reads it (`${VAR}` / `${VAR?}`: `replace-registry-host=${RRH}` with `RRH=always` counts as `always`, and the warning quotes both spellings; an unset `${VAR}` stays literal, as in npm) — and, when the effective value rewrites a pinned host, emits the `redirect_npm_replace_registry_host` warning naming the layer (env var / project file / config path) and the remedies: `replace-registry-host=npmjs` in the project `.npmrc` (or unsetting the env var), or vendored mode, whose `file:` resolutions npm never rewrites. The setting is never rewritten; the redirect itself still lands and the exit status is unchanged.
 
 **pnpm per-member locks (#492)**: a workspace whose root `pnpm-workspace.yaml` sets `sharedWorkspaceLockfile: false` (or, when the YAML leaves the key unset, whose root `.npmrc` sets `shared-workspace-lockfile=false`; on disk, when neither file sets it, `pnpm_config_shared_workspace_lockfile` / `npm_config_shared_workspace_lockfile`; booleans in any letter case, as pnpm's YAML reader takes them) installs each member from the member's own `pnpm-lock.yaml`. Run from the workspace root, hosted mode reads every member lock the `packages:` globs name (quoted `!` negations applied; `node_modules`, `bower_components`, `test` and `tests` trees and symlinked directories skipped) beside the root lock, which may be absent (pnpm 7): each is rewritten like the root lock, a 9.0 member lock engages the `trustLockfile: true` auto-config in the ROOT `pnpm-workspace.yaml` (heal-on-rerun included), and discovery (`list`, `vex`, `rollback`, `remove`) reads the same member locks. A root lock whose `importers:` lists a member is a shared lock, so member locks beside it are stale and never read or attested. When the members' locks cannot be listed or read (no `packages:` key, whose pnpm <= 8 default finds projects in every directory; a `packages:` value that is not a plain list of globs; no member lock and no root lock, or a member lock that exists but is not readable text), every pnpm lock (the root's included) is left untouched and the run warns `redirect_pnpm_member_locks_unresolved` instead of `redirect_pnpm_no_lockfile`; no pnpm pin is confirmed. The lockfile inventory reads the same member locks, so their dependencies are discovered without an installed tree. The in-memory engine gives the same result as a disk run from the workspace root: a directory whose `pnpm-lock.yaml` has no `pnpm-workspace.yaml` beside it is a workspace member, never a root of its own, when (with `npm` among the `ecosystems`) the nearest ancestor `pnpm-workspace.yaml` is readable and its `packages:` globs (read as above, no braces, classes or extglobs) list it, it holds a `package.json` / `package.yaml` / `package.json5`, the workspace root installs it from its own lock (`sharedWorkspaceLockfile: false` with the member list resolved) or from a root lock beside which its lock is a stale leftover, and socket.yml admits the workspace root. Its lock is then demoted into that workspace root, which becomes a root even with no lock of its own (pnpm 7), the way a Cargo member's lock is demoted; the member stays a root only when it holds another lock marker, without its pnpm lock. The workspace root then pins (or refuses) its member locks as above, trusts them in its own file, refuses a member's branch lock (#556) and leaves a member lock beside a shared root lock unread. A directory that fails any of those tests keeps its lock as a root of its own (pinned like any root, or refused with `redirect_pnpm_settings_elsewhere` while the trust auto-config is on and the ancestor file may list it). Path selection adds the workspace root above every candidate to `roots`, fetches it, and fetches the member locks, branch locks and manifests below it in every directory holding a package manifest; given those `roots` as `projectRoots`, the session drops a confirmed member named beside its workspace root (unless it holds another lock) and a lockless workspace root that pins no member, so it runs the roots it would detect. A confirmed member named in `projectRoots` without its workspace root is not pinned: its lock is dropped and the run-level `pnpm_member_lock_ignored` warning names the workspace root to scan.
 

crates/socket-patch-cli/tests/vendor/redirect_npm_allow_remote.rs

@@ -909,6 +909,66 @@ async fn replace_registry_host_rewriting_the_pin_is_warned() {
     );
 }
 
+/// #1233: npm env-replaces every config value, so
+/// `replace-registry-host=${RRH}` with `RRH=always` rewrites every hosted
+/// pin exactly like `=always`. The scan read the literal `${RRH}`, matched
+/// no host and stayed silent; it now warns in the project and user layers,
+/// naming the spelling the user wrote and what npm expands it to.
+#[tokio::test]
+async fn replace_registry_host_via_env_interpolation_is_warned() {
+    let server = MockServer::start().await;
+    mock_api(&server).await;
+
+    // Project `.npmrc`.
+    let tmp = tempfile::tempdir().unwrap();
+    write_npm_project(tmp.path(), "package-lock.json");
+    std::fs::write(tmp.path().join(".npmrc"), "replace-registry-host=${RRH}\n").unwrap();
+    let (code, doc, _) =
+        scan_hosted_env(tmp.path(), &server.uri(), &["--json"], &[("RRH", "always")]);
+    assert_eq!(code, 0, "{doc:#}");
+    let detail = replace_host_warning(&doc).unwrap_or_else(|| panic!("no {REPLACE_HOST}: {doc:#}"));
+    assert!(
+        detail.contains(
+            "the project .npmrc sets `replace-registry-host=${RRH}` (npm expands it to `always`)"
+        ),
+        "{detail}"
+    );
+
+    // User config.
+    let tmp = tempfile::tempdir().unwrap();
+    let cfg = tempfile::tempdir().unwrap();
+    write_npm_project(tmp.path(), "package-lock.json");
+    let user = cfg.path().join("user.npmrc");
+    std::fs::write(&user, "replace-registry-host=${RRH}\n").unwrap();
+    let user_s = user.to_str().unwrap();
+    let (code, doc, _) = scan_hosted_env(
+        tmp.path(),
+        &server.uri(),
+        &["--json"],
+        &[
+            ("NPM_CONFIG_USERCONFIG", user_s),
+            ("npm_config_userconfig", user_s),
+            ("RRH", "always"),
+        ],
+    );
+    assert_eq!(code, 0, "{doc:#}");
+    let detail = replace_host_warning(&doc).unwrap_or_else(|| panic!("no {REPLACE_HOST}: {doc:#}"));
+    assert!(
+        detail.contains("the user npm config")
+            && detail.contains(user_s)
+            && detail.contains("(npm expands it to `always`)"),
+        "{detail}"
+    );
+
+    // An unset `${RRH}` stays literal for npm too, and matches no host.
+    let tmp = tempfile::tempdir().unwrap();
+    write_npm_project(tmp.path(), "package-lock.json");
+    std::fs::write(tmp.path().join(".npmrc"), "replace-registry-host=${RRH}\n").unwrap();
+    let (code, doc, _) = scan_hosted_env(tmp.path(), &server.uri(), &["--json"], &[]);
+    assert_eq!(code, 0, "{doc:#}");
+    assert_eq!(replace_host_warning(&doc), None, "{doc:#}");
+}
+
 /// Review finding: `remove` dropped the hosted leg's warnings. A
 /// redirect-created `.npmrc` the user has since added to is no longer the
 /// scaffold, so the restore keeps it byte-for-byte and warns that the

crates/socket-patch-core/src/hosted/engine.rs

@@ -2607,16 +2607,16 @@ fn npm_allow_remote(
     // so every install 404s. The setting is the user's, so it is reported
     // (from whichever layer sets it), never overridden.
     let project = npmrc.as_ref().ok().and_then(|t| t.as_deref());
-    if let Some((value, source)) = effective_replace_registry_host(project, &outer) {
+    if let Some(setting) = effective_replace_registry_host(project, &outer) {
         let blocked: Vec<&str> = npm_hosts
             .iter()
             .copied()
-            .filter(|host| replace_registry_host_rewrites(&value, host))
+            .filter(|host| replace_registry_host_rewrites(&setting.value, host))
             .collect();
         if !blocked.is_empty() {
             npm_warnings.push(warning(
                 NPM_REPLACE_REGISTRY_HOST_CODE,
-                npm_replace_registry_host_detail(&blocked, &value, &source),
+                npm_replace_registry_host_detail(&blocked, &setting),
             ));
         }
     }

crates/socket-patch-core/src/hosted/guidance.rs

@@ -560,11 +560,21 @@ pub const NPM_REPLACE_REGISTRY_HOST_CODE: &str = "redirect_npm_replace_registry_
 /// user's — so the warning names where it is set and both remedies.
 pub fn npm_replace_registry_host_detail(
     hosts: &[&str],
-    value: &str,
-    source: &crate::patch::redirect::npmrc::SettingSource,
+    setting: &crate::patch::redirect::npmrc::EffectiveSetting,
 ) -> String {
     use crate::patch::redirect::npmrc::SettingSource;
-    let (where_, fix) = match source {
+    // Name the spelling the user can find (`${RRH}`), and what npm makes
+    // of it.
+    let value = if setting.raw.trim() == setting.value {
+        format!("`replace-registry-host={}`", setting.value)
+    } else {
+        format!(
+            "`replace-registry-host={}` (npm expands it to `{}`)",
+            setting.raw.trim(),
+            setting.value
+        )
+    };
+    let (where_, fix) = match &setting.source {
         SettingSource::Env(var) => (
             format!("the environment variable {var} sets"),
             format!(
@@ -586,7 +596,7 @@ pub fn npm_replace_registry_host_detail(
     };
     format!(
         "the npm lockfile now resolves patched dependencies from the hosted patch server ({}), \
-         but {where_} `replace-registry-host={value}`, which makes npm >=8 rewrite those \
+         but {where_} {value}, which makes npm >=8 rewrite those \
          `resolved` URLs to the configured registry: every `npm ci` / `npm install` then \
          fails E404 (a warm npm cache can hide this locally; a fresh checkout or CI runner \
          fails). To install the hosted patches, {fix}; or switch this project to vendored \