Ci/sudo install tests by tannevaled · Pull Request #86 · pkgxdev/pkgm

tannevaled · 2026-05-18T11:13:59Z

No description provided.

Restore the privilege drop when running as root via sudo (so pkgx caches stay user-owned), but only when the resolved pkgx binary is actually reachable from $SUDO_USER. Falls back to running pkgx as root rather than crashing the inner sudo with "Permission denied" when pkgx lives under /root/.pkgx/ (pkgxdev#68). - Restore drop-privilege behaviour for `sudo pkgm` (fixes the regression flagged by jhheider on pkgxdev#83). - Resolve an alternative pkgx under $SUDO_USER's home / /usr/local when the current path is unreachable to $SUDO_USER. - Override HOME so pkgx caches under the invoking user's tree. - Stop mutating `args` so the args[0] lookup at line 341 keeps working. - Avoid the `Deno.env.get("USER")!` non-null assertion crash. - Call install_prefix() once (it has filesystem side effects). - Keep the visible log surface unchanged: only the pre-existing "installing as root" warning fires by default; the new "pkgx unreachable" diagnostic is gated behind PKGM_DEBUG. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR updates sudo pkgm install handling so pkgx can run as the invoking sudo user when installing to /usr/local, while adding CI coverage for sudo install scenarios.

Changes:

Adds sudo privilege-dropping logic with HOME resolution and fallback behavior when pkgx is root-only.
Adds helper functions for resolving user homes and selecting reachable pkgx paths.
Adds a Linux CI job covering sudo install cache ownership and fallback behavior.

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

File	Description
`pkgm.ts`	Updates sudo/root `pkgx` invocation behavior and adds user-home/reachability helpers.
`.github/workflows/ci.yml`	Adds sudo install regression tests on Ubuntu.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+              const path = join(root, entry.name, "bin/pkgx");
+              if (!existsSync(path)) continue;
+              if (!best || v.gt(best.v)) best = { v, path };
+            } catch { /* skip malformed version dir */ }


+    const local = join(home, ".local/bin/pkgx");
+    if (existsSync(local)) return local;
+  }
+  if (existsSync("/usr/local/bin/pkgx")) return "/usr/local/bin/pkgx";


+
+  if (p === "/root" || p.startsWith("/root/")) return false;
+  if (p === "/var/root" || p.startsWith("/var/root/")) return false;
+


- expand single-line `} catch { /* … */ }` to the multi-line form `deno fmt` expects, so the existing `deno fmt --check .` CI job stops failing on it. - extract the `pkgx >= 2.4.0` floor from `get_pkgx()` into a shared `pkgx_meets_minimum()` helper and apply it to every fallback path in `pkgx_reachable_as()` (versioned pkgx.sh dir, ~/.local/bin/pkgx, /usr/local/bin/pkgx). Previously a stale older pkgx could be returned as the fallback when the original-root pkgx was new enough, breaking the JSON query. - exempt the shared Linuxbrew prefix (default /home/linuxbrew/.linuxbrew, or \$HOMEBREW_PREFIX when set) from `reachable_as()`'s /home/<user>/ rejection. standardPath() already treats it as a system pkgx location; without this exemption a Linuxbrew-installed pkgx would force the root-execution fallback, recreating the root-owned cache problem this whole change exists to prevent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The previous commit placed `const PKGX_MIN_VERSION = new SemVer("2.4.0")` above pkgx_meets_minimum() near line ~540, but the top-level dispatch block (parsedArgs switch at lines 60–126) calls install() → get_pkgx() → pkgx_meets_minimum() at module-init time, which is BEFORE execution reaches line 540. `const` declarations are hoisted in name only (TDZ), unlike `function` declarations whose bodies are hoisted — so the reference threw `Cannot access 'PKGX_MIN_VERSION' before initialization` at runtime, causing every `pkgm install` to fail silently with exit 1. Neither `deno fmt`, `deno lint`, nor `deno check` detect this — it's a module-init ordering issue invisible to static analysis. Pushed CI on pkgxdev#86 caught it via the existing `./pkgm.ts i git` test. Move the constant up to module-scope right after the `hydrate` import, above any function that's reachable from top-level code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

+              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;


The previous assertion `find /root/.pkgx -newer marker` is fundamentally incompatible with the shebang resolution: when `sudo ./pkgm.ts …` runs, the kernel reads `#!/usr/bin/env -S pkgx --quiet deno^2.1 run …` and execs `pkgx` as root *before any pkgm.ts code runs*. That outer pkgx caches deno under $HOME/.pkgx, which under sudo as root resolves to /root/.pkgx. Those cache files are unavoidable and unrelated to whether pkgm's *inner* pkgx invocation drops privileges and overrides HOME. As a consequence the sudo-install job has been failing on main ever since pkgxdev#85 merged (run 26006991059) — the test caught the shebang's deno cache and reported it as a HOME-override failure regardless of which pkgm.ts version it ran against. The Copilot autofix on pkgxdev#85 had moved the assertion from `test -e` to `find -newer marker` but didn't remove it, so the false positive persisted. Replace the negative check with a positive one against $HOME/.pkgx: - assert new files appeared under $HOME/.pkgx after the install (proves inner pkgx pointed HOME at the invoking user) - assert none of those files are root-owned (proves the privilege drop in query_pkgx took effect) If either property fails, one of HOME override or privilege drop is broken in pkgm.ts — which is what the job is meant to guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The positive cache check landed but is still failing on the runner: "no new files under \$HOME/.pkgx". To diagnose without log access I need to surface the relevant state via the annotations API. Add four warnings that capture, on every run: - which pkgx the sudo session resolves (the path get_pkgx() returns) - what new files appeared under /root/.pkgx (shebang's deno cache plus any cache the inner pkgx made there) - what new files appeared under \$HOME/.pkgx (the privilege-drop target) - the first 500 bytes of pkgm.ts's stderr, captured with PKGM_DEBUG=1 so the "is not reachable as <user>" fallback diagnostic surfaces To be reverted once the failure mode is identified. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The diagnostic ::warning:: annotations from ea4dab6 confirmed the install actually works: hyperfine landed under /home/runner/.pkgx/crates.io/hyperfine/v1.20.0/ with the privilege drop and HOME override both firing as intended (no "is not reachable as <user>" diagnostic on pkgm's stderr, only deno download lines). The remaining failure was the assertion itself: `find -type f -newer marker` returns empty because tar -x preserves the archive's original mtimes on extracted files, so the binary's mtime predates the marker. Only the directories pkgx mkdirs during extraction have a current mtime. Switch the positive check to `-type d` and drop the diagnostic warnings now that the failure mode is identified. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

+          # 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


+              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;


Copilot's review on 18e0471 flagged that the versioned-layout fallback (`~/.pkgx/pkgx.sh/v<x.y.z>/bin/pkgx`) picked candidates based only on the directory-name semver and existence of the binary, while the other fallback paths (~/.local/bin/pkgx, /usr/local/bin/pkgx) verify candidates with pkgx_meets_minimum(). A stale, non-executable, or version-mismatched v*/bin/pkgx could therefore be returned and break the inner sudo invocation. Keep the directory-name semver as a cheap pre-filter — it lets us skip old-version dirs without spawning --version per entry — but verify the actual binary with pkgx_meets_minimum() before accepting it as `best`. The other two Copilot remarks on this commit either don't apply (Linuxbrew exemption already landed in e1104ea, sits before the /home|Users/ rejection) or are stale (the ci.yml comment refers to `set -ux` / `rc=$?` from ea4dab6 which I already removed in 18e0471; the current step uses `set -eux`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The job was pinned to ubuntu-latest because the previous /root/.pkgx assertion was Linux-specific. Now that the assertion targets \$HOME/.pkgx instead and pkgm.ts's user_home() has a dscl fallback for macOS, both tests are portable — with one platform difference: root's home is /root on Linux but /var/root on macOS. `sudo mkdir /root` on modern macOS fails because the system volume is read-only and new top-level directories can't be created without /etc/synthetic.conf. - Add a {ubuntu-latest, macos-latest} matrix. - Resolve root's home via `eval echo ~root` in the fallback step so the same script stages pkgx under /root on Linux and /var/root on macOS. Test 1 (privilege drop + HOME override) already used only \$HOME/.pkgx and POSIX find flags, so it ports without changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

macOS sudoers keeps HOME in env_keep by default; plain `sudo ./pkgm.ts` on macOS preserves HOME=\$HOME for the shebang's outer pkgx invocation, which runs as ROOT and writes root-owned dirs into \$HOME/.pkgx for the deno self-cache. The inner pkgx (dropped back to \$SUDO_USER by pkgm's fix) then EACCES on those dirs, killing the install with Error: Os { code: 13, kind: PermissionDenied, message: "Permission denied" } (captured live via the diagnostic ::warning:: annotations). `sudo -H` forces HOME to root's home, matching Linux sudoers' env_reset behaviour. Harmless on Linux (HOME wasn't preserved there to begin with), required for hermetic test runs on macOS. Also drop the temporary diagnostic warnings; they served their purpose.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

.github/workflows/ci.yml:151

These diagnostics use ::warning:: for normal execution data and will add warning annotations to every passing run. Please emit them as plain logs or only enable them when a debug flag is set, otherwise routine CI runs will be noisy and real warnings become harder to spot.


          # 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

          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
-          sudo ./pkgm.ts i hyperfine
+
+          # Why `sudo -H`: macOS sudoers keeps HOME in env_keep by default,
+          # so plain `sudo ./pkgm.ts` would preserve HOME=$HOME for the
+          # shebang's outer `pkgx --quiet deno^2.1 run …` invocation —


macOS sudoers keeps HOME in env_keep by default, so plain \`sudo pkgm install …\` preserves the caller's HOME. The shebang's outer \`pkgx --quiet deno^2.1 run …\` then runs as root with HOME pointing at \$SUDO_USER's tree, and its self-cache creates root-owned dirs under \$SUDO_USER/.pkgx. The privilege-dropped inner pkgx that this PR introduces (sudo -u \$SUDO_USER -- …) then EACCES's on those same dirs and the install dies with Error: Os { code: 13, kind: PermissionDenied, message: "Permission denied" } The CI matrix's macOS leg hit this exact failure mode (see the diagnostic ::warning:: annotations on c700b49/cd3133b) until we switched the test invocation to \`sudo -H\`. A real macOS user typing \`sudo pkgm install …\` will hit the same wall. Detect this state (\`isRoot && SUDO_USER set && HOME == \$SUDO_USER's home\`) and emit a one-line yellow warning pointing the user at \`sudo -H\`. The check is platform-agnostic — any sudoers config that preserves HOME triggers it — but in practice it's a macOS-only path, so the message names that. Also factor the user_home(sudoUser) lookup so the warning and the HOME override share the same call rather than spawning getent/dscl twice. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Previous commit warned users to retype \`sudo -H pkgm …\` on macOS, where sudoers preserves HOME and the shebang's outer pkgx pollutes \$SUDO_USER/.pkgx with root-owned dirs that the privilege-dropped inner pkgx can't write to. Asking the user to remember a flag breaks the "works the same everywhere" property pkgm should have. Self-heal instead: function reclaim_pkgx_cache_for(home, user) { find \$home/.pkgx -uid 0 -exec chown \$user {} + } Targeted by `-uid 0` so we only touch entries the shebang's outer pkgx left behind — user-owned entries (legitimate cache) are not disturbed. Best-effort: if find/chown aren't reachable the install may still EACCES but in that case `sudo -H` is the manual escape hatch. Wire it into query_pkgx alongside the existing HOME override, gated on the same `HOME === SUDO_USER home` condition that previously emitted the warning. Drop the warning — there's nothing for the user to do now. CI: remove `sudo -H` from the sudo-install test on both legs of the matrix. The macOS leg now exercises the realistic user invocation (plain `sudo pkgm`) and validates that pkgm's self-heal makes it work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

+  const find = existsSync("/usr/bin/find") ? "/usr/bin/find" : "/bin/find";
+  try {
+    new Deno.Command(find, {
+      args: [cache, "-uid", "0", "-exec", "chown", user, "{}", "+"],


-          # 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)


…ME/.pkgx

The first cut of reclaim_pkgx_cache_for() invoked \`find -uid 0 -exec chown user {} +\`, which followed symlinks. Pkgx's versioned-layout sprinkles v*, v<major>, v<major>.<minor> symlinks beside the real version directories; without -h, chown walked each link to its (already-reclaimed) target, leaving the link itself still root-owned. Diagnostic ::warning:: annotations on 77b78c9 confirmed: 6 entries remained owned by root after the chown — all of them version-alias symlinks under deno.land/* and info-zip.org/unzip/*. Add -h. It's in POSIX, present on both BSD (macOS) and GNU (Linux) chown, and a no-op for regular files. Also drop the diagnostic warnings from ci.yml now that the failure mode is identified. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

+      } 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`,
        );
      }


+              // 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;


+    const local = join(home, ".local/bin/pkgx");
+    if (existsSync(local) && pkgx_meets_minimum(local)) return local;


Copilot AI review requested due to automatic review settings May 18, 2026 11:14

Copilot started reviewing on behalf of tannevaled May 18, 2026 11:14 View session

tannevaled and others added 8 commits May 18, 2026 13:17

Potential fix for pull request finding

3d8f6a1

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

3c85e6d

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

373b100

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

bfcf4a7

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

3470f52

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

069770d

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

e3530f1

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

tannevaled force-pushed the ci/sudo-install-tests branch from 3afc99c to e3530f1 Compare May 18, 2026 11:17