Skip to content

ci: validate sudo pkgm install behaviour#85

Merged
jhheider merged 2 commits into
pkgxdev:mainfrom
tannevaled:ci/sudo-install-tests
May 18, 2026
Merged

ci: validate sudo pkgm install behaviour#85
jhheider merged 2 commits into
pkgxdev:mainfrom
tannevaled:ci/sudo-install-tests

Conversation

@tannevaled
Copy link
Copy Markdown
Contributor

first try

Add a Linux-only sudo-install job that exercises the three behaviours fixed in 2b33f20 and that the existing test matrix doesn't cover:

  • privilege drop: nothing freshly created under $HOME/.pkgx after a sudo pkgm install may be owned by root
  • HOME override: pkgx must not cache under /root/.pkgx (checked via sudo test since /root/ is mode 700)
  • sudo pkgm install doesn't work #68 fallback: with pkgx only reachable under /root/.pkgx and every alternative removed, sudo PKGM_DEBUG=1 pkgm install must exit 0, install the package, and emit the "is not reachable as …" diagnostic instead of crashing with "Permission denied"

The fallback step is destructive (wipes ~/.pkgx, /usr/local/bin/pkgx, ~/.local/bin/pkgx) and runs last for that reason.

Add a Linux-only `sudo-install` job that exercises the three behaviours
fixed in 2b33f20 and that the existing test matrix doesn't cover:

- privilege drop: nothing freshly created under $HOME/.pkgx after a
  `sudo pkgm install` may be owned by root
- HOME override: pkgx must not cache under /root/.pkgx (checked via
  `sudo test` since /root/ is mode 700)
- pkgxdev#68 fallback: with pkgx only reachable under /root/.pkgx
  and every alternative removed, `sudo PKGM_DEBUG=1 pkgm install` must
  exit 0, install the package, and emit the "is not reachable as …"
  diagnostic instead of crashing with "Permission denied"

The fallback step is destructive (wipes ~/.pkgx, /usr/local/bin/pkgx,
~/.local/bin/pkgx) and runs last for that reason.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 17, 2026 17:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a Linux-only CI job that validates sudo pkgm install behaviors that aren’t covered by the existing matrix, focusing on correct cache ownership/HOME handling and a regression scenario where pkgx is only available under /root.

Changes:

  • Introduces a new sudo-install GitHub Actions job on ubuntu-latest.
  • Adds checks for HOME override (no caching under /root/.pkgx) and privilege dropping (no root-owned newly-created files under $HOME/.pkgx).
  • Adds a destructive final step that stages pkgx only under /root/.pkgx and verifies sudo pkgm install succeeds in that scenario.

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

Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 17, 2026 18:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread .github/workflows/ci.yml
Comment on lines +169 to +176
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)
rc=$?
set -e
echo "$out"
# Regression check for pkgxdev/pkgm#68: the install succeeds without
# crashing when only the sudo-only pkgx remains reachable.
test $rc -eq 0
@jhheider
Copy link
Copy Markdown
Contributor

looks good. let's push it in, so we can get #84

@jhheider jhheider merged commit 4ffae15 into pkgxdev:main May 18, 2026
6 of 8 checks passed
tannevaled added a commit to tannevaled/pkgm that referenced this pull request May 18, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants