Ci/sudo install tests

.github/workflows/ci.yml

@@ -112,62 +112,88 @@ jobs:
   # Validates `sudo pkgm install` behaviour fixed in 2b33f20:
   #  - privilege drop so pkgx cache stays owned by $SUDO_USER, not root
   #  - HOME override so the cache lands under the invoking user's tree
-  #  - fallback to running pkgx as root when it lives under /root/.pkgx
+  #  - fallback to running pkgx as root when it lives under root's home
   #    and is therefore unreachable to $SUDO_USER (pkgxdev/pkgm#68)
-  # Linux-only: the /root/.pkgx scenario doesn't arise on macOS in practice.
+  # The root-home path differs by OS (/root on Linux, /var/root on macOS);
+  # we resolve it dynamically via `eval echo ~root` rather than hard-coding.
   sudo-install:
-    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        os:
+          - ubuntu-latest
+          - macos-latest
+    runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v4
       - uses: pkgxdev/setup@v4
 
       - name: sudo install drops privileges and overrides HOME
         run: |
           set -eux
-          # marker to scope ownership checks to files created by this install
+          # marker to scope checks to entries created by this install
           touch /tmp/pkgm-sudo-marker
+          # Plain `sudo` (no -H). This is what a typical user types. On
+          # macOS, sudoers keeps HOME in env_keep so the shebang's outer
+          # pkgx runs as root inside $SUDO_USER's tree and pollutes
+          # $HOME/.pkgx with root-owned dirs. pkgm.ts's
+          # reclaim_pkgx_cache_for() chowns those back to $SUDO_USER
+          # before dropping privileges so the inner pkgx can write.
           sudo ./pkgm.ts i hyperfine
           test -x /usr/local/bin/hyperfine
-          # HOME override: pkgx must not have created anything under /root/.pkgx
-          # during this install. We scope the check to paths newer than the
-          # marker so a pre-existing /root/.pkgx from the runner image or
-          # setup action does not cause a false failure.
-          created_under_root=$(sudo find /root/.pkgx -newer /tmp/pkgm-sudo-marker -print 2>/dev/null || true)
-          if [ -n "$created_under_root" ]; then
-            echo "::error::pkgx cached under /root/.pkgx — HOME override failed"
-            echo "$created_under_root"
+
+          # HOME override + privilege drop are validated via the pkg cache
+          # under $HOME/.pkgx. We deliberately do NOT assert that
+          # /root/.pkgx is empty: the shebang's `pkgx --quiet deno^2.1 run …`
+          # runs as root before any pkgm.ts code executes, and that outer
+          # pkgx caches under $HOME/.pkgx which resolves to /root/.pkgx
+          # under sudo. That cache is unavoidable and unrelated to whether
+          # pkgm's *inner* pkgx call dropped privileges.
+          #
+          # We check for newly created entries (directories specifically),
+          # not files: tar -x preserves the archive's original mtimes on
+          # extracted files, so file mtimes are typically *older* than the
+          # marker. Directories are created fresh by `mkdir` during
+          # extraction and reliably have a current mtime.
+
+          new_dirs=$(sudo find "$HOME/.pkgx" -newer /tmp/pkgm-sudo-marker -type d -print 2>/dev/null | head -1 || true)
+          if [ -z "$new_dirs" ]; then
+            echo "::error::no new directories under \$HOME/.pkgx — inner pkgx did not cache to invoking user's tree"
             exit 1
           fi
-          # Privilege drop: nothing newly created under ~/.pkgx should be
-          # owned by root. Any root-owned file here means pkgx ran as root
-          # despite SUDO_USER being set.
+
           owned_by_root=$(sudo find "$HOME/.pkgx" -newer /tmp/pkgm-sudo-marker -user root -print 2>/dev/null || true)
           if [ -n "$owned_by_root" ]; then
-            echo "::error::pkgx cache files created as root under \$HOME/.pkgx:"
+            echo "::error::pkgx cache entries created as root under \$HOME/.pkgx — privilege drop failed:"
             echo "$owned_by_root"
             exit 1
           fi
 
       - name: sudo install falls back when pkgx is unreachable as $SUDO_USER
         # Must be last — this step strips pkgx from every location the
-        # runner user can reach, leaving only /root/.pkgx, which the
-        # subsequent shebang resolution still needs to walk through sudo.
+        # runner user can reach, leaving only root's private pkgx, which
+        # the subsequent shebang resolution still needs to walk through sudo.
         run: |
           set -eux
-          # Stage pkgx exclusively under /root so that reachable_as() returns
-          # false for the runner user and no alternative is found.
+          # Resolve root's home portably: /root on Linux, /var/root on macOS.
+          # Hard-coding /root would fail on macOS because the system volume
+          # is read-only and `sudo mkdir /root` can't create a new top-level
+          # dir without /etc/synthetic.conf.
+          root_home=$(eval echo ~root)
+
+          # Stage pkgx exclusively under root's home so that reachable_as()
+          # returns false for the runner user and no alternative is found.
           pkgx_src=$(command -v pkgx)
-          sudo mkdir -p /root/.pkgx/bin
-          sudo cp "$pkgx_src" /root/.pkgx/bin/pkgx
+          sudo mkdir -p "$root_home/.pkgx/bin"
+          sudo cp "$pkgx_src" "$root_home/.pkgx/bin/pkgx"
           # Wipe every alternative the resolver looks for:
           #   ~/.pkgx/pkgx.sh/v*/bin/pkgx, ~/.local/bin/pkgx, /usr/local/bin/pkgx
           rm -rf "$HOME/.pkgx"
           sudo rm -f /usr/local/bin/pkgx "$HOME/.local/bin/pkgx"
-          # Invoke pkgm.ts with the /root pkgx on PATH. `sudo env PATH=...`
+          # Invoke pkgm.ts with the staged pkgx on PATH. `sudo env PATH=...`
           # is the canonical way around the default secure_path policy in
-          # Ubuntu's sudoers.
+          # Ubuntu's sudoers; macOS sudo respects the explicit env too.
           set +e
-          out=$(sudo env PATH="/root/.pkgx/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" ./pkgm.ts i gum 2>&1)
+          out=$(sudo env PATH="$root_home/.pkgx/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" ./pkgm.ts i gum 2>&1)
           rc=$?
           set -e
           echo "$out"

pkgm.ts

@@ -13,6 +13,13 @@ import { ensureDir, existsSync, walk } from "jsr:@std/fs@^1";
 import { parseArgs } from "jsr:@std/cli@^1";
 const { hydrate } = plumbing;
 
+// Module-scope SemVer literal: must be defined before any function that
+// reads it can be called from top-level code below. `const` declarations
+// are hoisted in name only (TDZ), so placing this further down the file
+// triggered "Cannot access 'PKGX_MIN_VERSION' before initialization" once
+// install()/get_pkgx() ran at module-init time.
+const PKGX_MIN_VERSION = new SemVer("2.4.0");
+
 function standardPath() {
   let path = "/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin";
 
@@ -297,25 +304,56 @@ async function query_pkgx(
   set("PKGX_DIST_URL");
   set("XDG_DATA_HOME");
 
-  const needs_sudo_backwards = install_prefix().string == "/usr/local";
-  let cmd = needs_sudo_backwards ? "/usr/bin/sudo" : pkgx;
-  if (needs_sudo_backwards) {
-    if (!Deno.env.get("SUDO_USER")) {
-      if (Deno.uid() == 0) {
+  const isRoot = Deno.uid() == 0;
+  const sudoUser = Deno.env.get("SUDO_USER");
+  const prefix = install_prefix().string;
+  const isSystemPrefix = prefix == "/usr/local";
+
+  let cmd = pkgx;
+  let cmd_args = args;
+
+  if (isSystemPrefix) {
+    if (isRoot && sudoUser) {
+      const sudo_user_home = user_home(sudoUser);
+
+      // If sudo preserved HOME (typical macOS — sudoers keeps HOME in
+      // env_keep by default; most Linux distros reset it via env_reset),
+      // the shebang's outer `pkgx --quiet deno^2.1 run …` ran as root
+      // with HOME pointing at SUDO_USER's tree and its self-cache left
+      // root-owned dirs under $SUDO_USER/.pkgx. The privilege-dropped
+      // inner pkgx below would then EACCES on those dirs and abort the
+      // install. Reclaim ownership for $SUDO_USER so the install can
+      // proceed without forcing the user to remember `sudo -H`.
+      if (sudo_user_home && Deno.env.get("HOME") === sudo_user_home) {
+        reclaim_pkgx_cache_for(sudo_user_home, sudoUser);
+      }
+
+      // Drop privileges so pkgx writes its cache as the invoking user, not root.
+      // But only if pkgx is reachable from sudoUser — otherwise the inner sudo
+      // aborts with "unable to execute …: Permission denied" (pkgxdev/pkgm#68).
+      const reachable = pkgx_reachable_as(pkgx, sudoUser);
+      if (reachable) {
+        cmd = "/usr/bin/sudo";
+        cmd_args = ["-u", sudoUser, "--", reachable, ...args];
+        // Override HOME, or pkgx will cache back under /root/ where sudoUser
+        // can't reach it on the next invocation.
+        if (sudo_user_home) env.HOME = sudo_user_home;
+      } else if (Deno.env.get("PKGM_DEBUG")) {
         console.error(
-          "%cwarning",
-          "color:yellow",
-          "installing as root; installing via `sudo` is preferred",
+          `pkgm: \`pkgx\` at ${pkgx} is not reachable as ${sudoUser}; running it as root`,
         );
       }
-      cmd = pkgx;
-    } else {
-      args.unshift("-u", Deno.env.get("SUDO_USER")!, pkgx);
+    } else if (isRoot) {
+      console.error(
+        "%cwarning",
+        "color:yellow",
+        "installing as root; installing via `sudo` is preferred",
+      );
     }
   }
 
   const proc = new Deno.Command(cmd, {
-    args: [...args, "--json=v1"],
+    args: [...cmd_args, "--json=v1"],
     stdout: "piped",
     env,
     clearEnv: true,
@@ -517,18 +555,26 @@ function symlink_with_overwrite(src: string, dst: string) {
   Deno.symlinkSync(src, dst);
 }
 
+function pkgx_meets_minimum(path: string): boolean {
+  try {
+    const out = new Deno.Command(path, { args: ["--version"] }).outputSync();
+    if (!out.success) return false;
+    const match = new TextDecoder().decode(out.stdout).match(
+      /^pkgx (\d+\.\d+\.\d+)/,
+    );
+    if (!match) return false;
+    return new SemVer(match[1]).gte(PKGX_MIN_VERSION);
+  } catch {
+    return false;
+  }
+}
+
 function get_pkgx() {
   for (const path of Deno.env.get("PATH")!.split(":")) {
     const pkgx = join(path, "pkgx");
-    if (existsSync(pkgx)) {
-      const out = new Deno.Command(pkgx, { args: ["--version"] }).outputSync();
-      const stdout = new TextDecoder().decode(out.stdout);
-      const match = stdout.match(/^pkgx (\d+\.\d+\.\d+)/);
-      if (!match || new SemVer(match[1]).lt(new SemVer("2.4.0"))) {
-        Deno.exit(1);
-      }
-      return pkgx;
-    }
+    if (!existsSync(pkgx)) continue;
+    if (!pkgx_meets_minimum(pkgx)) Deno.exit(1);
+    return pkgx;
   }
   throw new Error("no `pkgx` found in `$PATH`");
 }
@@ -766,6 +812,166 @@ function install_prefix() {
   }
 }
 
+function user_home_from_passwd(user: string): string | undefined {
+  try {
+    const passwd = Deno.readTextFileSync("/etc/passwd");
+    for (const line of passwd.split("\n")) {
+      if (!line || line.startsWith("#")) continue;
+      const fields = line.split(":");
+      if (fields[0] === user) return fields[5] || undefined;
+    }
+  } catch {
+    // Ignore unreadable or absent passwd database and fall back to other lookups.
+  }
+
+  return undefined;
+}
+
+function user_home_from_dscl(user: string): string | undefined {
+  if (!existsSync("/usr/bin/dscl")) return undefined;
+
+  try {
+    const out = new Deno.Command("/usr/bin/dscl", {
+      args: [".", "-read", `/Users/${user}`, "NFSHomeDirectory"],
+    }).outputSync();
+    if (!out.success) return undefined;
+
+    const line = new TextDecoder().decode(out.stdout).trim();
+    const prefix = "NFSHomeDirectory:";
+    if (!line.startsWith(prefix)) return undefined;
+
+    const home = line.slice(prefix.length).trim();
+    return home || undefined;
+  } catch {
+    return undefined;
+  }
+}
+
+function user_home(user: string): string | undefined {
+  // Prefer getent where available, but fall back to passwd parsing and macOS
+  // dscl so HOME can still be resolved when dropping privileges on systems
+  // without getent.
+  const getent = existsSync("/usr/bin/getent")
+    ? "/usr/bin/getent"
+    : existsSync("/bin/getent")
+    ? "/bin/getent"
+    : undefined;
+
+  if (getent) {
+    try {
+      const out = new Deno.Command(getent, {
+        args: ["passwd", user],
+      }).outputSync();
+      if (out.success) {
+        const fields = new TextDecoder().decode(out.stdout).trim().split(":");
+        if (fields[5]) return fields[5];
+      }
+    } catch {
+      // Ignore getent lookup failures and try portable fallbacks below.
+    }
+  }
+
+  return user_home_from_passwd(user) ?? user_home_from_dscl(user);
+}
+
+function reclaim_pkgx_cache_for(home: string, user: string): void {
+  // Targeted chown: only files currently owned by root, not user-owned
+  // entries the caller may have placed under .pkgx for their own reasons.
+  // Best-effort — if find/chown aren't reachable the inner pkgx may still
+  // EACCES, but most invocations succeed.
+  const cache = join(home, ".pkgx");
+  if (!existsSync(cache)) return;
+  const find = existsSync("/usr/bin/find") ? "/usr/bin/find" : "/bin/find";
+  try {
+    // `chown -h`: act on the symlink itself, not its target. Pkgx's
+    // versioned layout sprinkles v*, v<major>, v<major>.<minor> symlinks
+    // alongside the real version dirs; without -h chown would follow each
+    // link and chown the already-reclaimed target, leaving the link still
+    // root-owned (-h is in POSIX, present on both BSD and GNU chown).
+    new Deno.Command(find, {
+      args: [cache, "-uid", "0", "-exec", "chown", "-h", user, "{}", "+"],
+      stdout: "null",
+      stderr: "null",
+    }).outputSync();
+  } catch {
+    // best-effort
+  }
+}
+
+function pkgx_reachable_as(current: string, user: string): string | undefined {
+  // The caller has already enforced PKGX_MIN_VERSION for `current` via
+  // get_pkgx(); fallback candidates have not, so each return path below
+  // re-checks with pkgx_meets_minimum() to avoid handing back an
+  // unsupported binary (per #86 review).
+  if (reachable_as(current, user)) return current;
+
+  const home = user_home(user);
+  if (home) {
+    // Versioned pkgx.sh layout: ~/.pkgx/pkgx.sh/v<x.y.z>/bin/pkgx — pick the
+    // highest version that meets the minimum.
+    const root = join(home, ".pkgx/pkgx.sh");
+    if (existsSync(root)) {
+      let best: { v: SemVer; path: string } | undefined;
+      try {
+        if (Deno.statSync(root).isDirectory) {
+          for (const entry of Deno.readDirSync(root)) {
+            if (!entry.isDirectory || !entry.name.startsWith("v")) continue;
+            try {
+              const v = new SemVer(entry.name.slice(1));
+              if (v.lt(PKGX_MIN_VERSION)) continue;
+              const path = join(root, entry.name, "bin/pkgx");
+              if (!existsSync(path)) continue;
+              // Directory-name version is a cheap pre-filter; verify the
+              // actual binary too, matching the other fallback paths so a
+              // stale or non-executable `v*/bin/pkgx` can't be returned
+              // (per #86 review).
+              if (!pkgx_meets_minimum(path)) continue;
+              if (!best || v.gt(best.v)) best = { v, path };
+            } catch {
+              // skip malformed version dir
+            }
+          }
+        }
+      } catch {
+        // Ignore unreadable/non-directory pkgx.sh roots and fall back to other locations.
+      }
+      if (best) return best.path;
+    }
+    const local = join(home, ".local/bin/pkgx");
+    if (existsSync(local) && pkgx_meets_minimum(local)) return local;
+  }
+  if (
+    existsSync("/usr/local/bin/pkgx") &&
+    pkgx_meets_minimum("/usr/local/bin/pkgx")
+  ) {
+    return "/usr/local/bin/pkgx";
+  }
+  return undefined;
+}
+
+function reachable_as(p: string, user: string): boolean {
+  // Conservative heuristic: private home dirs are typically mode 700, so a
+  // path under another user's home is unreachable. System paths and the
+  // user's own home are assumed reachable.
+  const home = user_home(user);
+  if (home && (p === home || p.startsWith(`${home}/`))) return true;
+
+  // Shared Linuxbrew prefix lives under /home but is world-traversable and
+  // is treated as a system pkgx location by standardPath(). Without this
+  // exemption a pkgx installed via Linuxbrew would force the root-execution
+  // fallback, recreating the root-owned cache problem this code avoids
+  // (per #86 review). Honour $HOMEBREW_PREFIX in case it's elsewhere.
+  const brew = Deno.env.get("HOMEBREW_PREFIX") ?? "/home/linuxbrew/.linuxbrew";
+  if (p === brew || p.startsWith(`${brew}/`)) return true;
+
+  if (p === "/root" || p.startsWith("/root/")) return false;
+  if (p === "/var/root" || p.startsWith("/var/root/")) return false;
+
+  if (p.match(/^\/(home|Users)\/([^/]+)(?:\/|$)/)) return false;
+
+  return true;
+}
+
 function dev_stub_text(selfpath: string, bin_prefix: string, name: string) {
   if (selfpath.startsWith("/usr/local") && selfpath != "/usr/local/bin/dev") {
     return `