Skip to content

Commit b1d7b3e

Browse files
magnusvirimagnusviri
authored
Merge branch 'pkgxdev:main' into main
2 parents dab7263 + 4ffae15 commit b1d7b3e

2 files changed

Lines changed: 121 additions & 39 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,70 @@ jobs:
108108
- run: |
109109
./pkgm.ts i spotify_player
110110
spotify_player --version
111+
112+
# Validates `sudo pkgm install` behaviour fixed in 2b33f20:
113+
# - privilege drop so pkgx cache stays owned by $SUDO_USER, not root
114+
# - HOME override so the cache lands under the invoking user's tree
115+
# - fallback to running pkgx as root when it lives under /root/.pkgx
116+
# and is therefore unreachable to $SUDO_USER (pkgxdev/pkgm#68)
117+
# Linux-only: the /root/.pkgx scenario doesn't arise on macOS in practice.
118+
sudo-install:
119+
runs-on: ubuntu-latest
120+
steps:
121+
- uses: actions/checkout@v4
122+
- uses: pkgxdev/setup@v4
123+
124+
- name: sudo install drops privileges and overrides HOME
125+
run: |
126+
set -eux
127+
# marker to scope ownership checks to files created by this install
128+
touch /tmp/pkgm-sudo-marker
129+
sudo ./pkgm.ts i hyperfine
130+
test -x /usr/local/bin/hyperfine
131+
# HOME override: pkgx must not have created anything under /root/.pkgx
132+
# during this install. We scope the check to paths newer than the
133+
# marker so a pre-existing /root/.pkgx from the runner image or
134+
# setup action does not cause a false failure.
135+
created_under_root=$(sudo find /root/.pkgx -newer /tmp/pkgm-sudo-marker -print 2>/dev/null || true)
136+
if [ -n "$created_under_root" ]; then
137+
echo "::error::pkgx cached under /root/.pkgx — HOME override failed"
138+
echo "$created_under_root"
139+
exit 1
140+
fi
141+
# Privilege drop: nothing newly created under ~/.pkgx should be
142+
# owned by root. Any root-owned file here means pkgx ran as root
143+
# despite SUDO_USER being set.
144+
owned_by_root=$(sudo find "$HOME/.pkgx" -newer /tmp/pkgm-sudo-marker -user root -print 2>/dev/null || true)
145+
if [ -n "$owned_by_root" ]; then
146+
echo "::error::pkgx cache files created as root under \$HOME/.pkgx:"
147+
echo "$owned_by_root"
148+
exit 1
149+
fi
150+
151+
- name: sudo install falls back when pkgx is unreachable as $SUDO_USER
152+
# Must be last — this step strips pkgx from every location the
153+
# runner user can reach, leaving only /root/.pkgx, which the
154+
# subsequent shebang resolution still needs to walk through sudo.
155+
run: |
156+
set -eux
157+
# Stage pkgx exclusively under /root so that reachable_as() returns
158+
# false for the runner user and no alternative is found.
159+
pkgx_src=$(command -v pkgx)
160+
sudo mkdir -p /root/.pkgx/bin
161+
sudo cp "$pkgx_src" /root/.pkgx/bin/pkgx
162+
# Wipe every alternative the resolver looks for:
163+
# ~/.pkgx/pkgx.sh/v*/bin/pkgx, ~/.local/bin/pkgx, /usr/local/bin/pkgx
164+
rm -rf "$HOME/.pkgx"
165+
sudo rm -f /usr/local/bin/pkgx "$HOME/.local/bin/pkgx"
166+
# Invoke pkgm.ts with the /root pkgx on PATH. `sudo env PATH=...`
167+
# is the canonical way around the default secure_path policy in
168+
# Ubuntu's sudoers.
169+
set +e
170+
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)
171+
rc=$?
172+
set -e
173+
echo "$out"
174+
# Regression check for pkgxdev/pkgm#68: the install succeeds without
175+
# crashing when only the sudo-only pkgx remains reachable.
176+
test $rc -eq 0
177+
test -x /usr/local/bin/gum

‎pkgm.ts‎

Lines changed: 54 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ function standardPath() {
2424
break;
2525
case "linux":
2626
homebrewPrefix = `/home/linuxbrew/.linuxbrew:${
27-
Deno.env.get("HOME")
27+
Deno.env.get(
28+
"HOME",
29+
)
2830
}/.linuxbrew`;
2931
break;
3032
}
@@ -87,7 +89,7 @@ if (parsedArgs.help || parsedArgs._[0] == "help") {
8789
{
8890
let all_success = true;
8991
for (const arg of args) {
90-
if (!await uninstall(arg)) {
92+
if (!(await uninstall(arg))) {
9193
all_success = false;
9294
}
9395
}
@@ -132,8 +134,8 @@ async function install(args: string[], basePath: string) {
132134
const pkgx = get_pkgx();
133135

134136
const [json] = await query_pkgx(pkgx, args);
135-
const pkg_prefixes = json.pkgs.map((x) =>
136-
`${x.pkg.project}/v${x.pkg.version}`
137+
const pkg_prefixes = json.pkgs.map(
138+
(x) => `${x.pkg.project}/v${x.pkg.version}`,
137139
);
138140

139141
// get the pkgx_dir this way as it is a) more reliable and b) the only way if
@@ -200,9 +202,10 @@ async function install(args: string[], basePath: string) {
200202
}
201203

202204
if (
203-
!Deno.env.get("PATH")?.split(":")?.includes(
204-
new Path(basePath).join("bin").string,
205-
)
205+
!Deno.env
206+
.get("PATH")
207+
?.split(":")
208+
?.includes(new Path(basePath).join("bin").string)
206209
) {
207210
console.error(
208211
"%c! warning:",
@@ -282,7 +285,7 @@ async function query_pkgx(
282285
args = args.map((x) => `+${x}`);
283286

284287
const env: Record<string, string> = {
285-
"PATH": standardPath(),
288+
PATH: standardPath(),
286289
};
287290
const set = (key: string) => {
288291
const x = Deno.env.get(key);
@@ -316,8 +319,7 @@ async function query_pkgx(
316319
stdout: "piped",
317320
env,
318321
clearEnv: true,
319-
})
320-
.spawn();
322+
}).spawn();
321323

322324
const status = await proc.status;
323325

@@ -328,22 +330,24 @@ async function query_pkgx(
328330
const out = await proc.output();
329331
const json = JSON.parse(new TextDecoder().decode(out.stdout));
330332

331-
const pkgs =
332-
(json.pkgs as { path: string; project: string; version: string }[]).map(
333-
(x) => {
334-
return {
335-
path: new Path(x.path),
336-
pkg: { project: x.project, version: new SemVer(x.version) },
337-
};
338-
},
339-
);
333+
const pkgs = (
334+
json.pkgs as { path: string; project: string; version: string }[]
335+
).map((x) => {
336+
return {
337+
path: new Path(x.path),
338+
pkg: { project: x.project, version: new SemVer(x.version) },
339+
};
340+
});
340341
const pkg = pkgs.find((x) => `+${x.pkg.project}` == args[0])!;
341-
return [{
342-
pkg,
343-
pkgs,
344-
env: json.env,
345-
runtime_env: json.runtime_env,
346-
}, env];
342+
return [
343+
{
344+
pkg,
345+
pkgs,
346+
env: json.env,
347+
runtime_env: json.runtime_env,
348+
},
349+
env,
350+
];
347351
}
348352

349353
async function mirror_directory(dst: string, src: string, prefix: string) {
@@ -465,10 +469,7 @@ async function create_v_symlinks(prefix: string) {
465469
}
466470
}
467471

468-
function expand_runtime_env(
469-
json: JsonResponse,
470-
basePath: string,
471-
) {
472+
function expand_runtime_env(json: JsonResponse, basePath: string) {
472473
const { runtime_env, pkgs } = json;
473474

474475
//FIXME this combines all runtime env which is strictly overkill
@@ -498,7 +499,11 @@ function expand_runtime_env(
498499

499500
// DUMB but easiest way to fix a bug
500501
const rv2: Record<string, Record<string, string>> = {};
501-
for (const { pkg: { project } } of json.pkgs) {
502+
for (
503+
const {
504+
pkg: { project },
505+
} of json.pkgs
506+
) {
502507
rv2[project] = rv;
503508
}
504509

@@ -529,8 +534,8 @@ function get_pkgx() {
529534
if (existsSync(pkgx)) {
530535
const out = new Deno.Command(pkgx, { args: ["--version"] }).outputSync();
531536
const stdout = new TextDecoder().decode(out.stdout);
532-
const match = stdout.match(/^pkgx (\d+.\d+)/);
533-
if (!match || parseFloat(match[1]) < 2.4) {
537+
const match = stdout.match(/^pkgx (\d+\.\d+\.\d+)/);
538+
if (!match || new SemVer(match[1]).lt(new SemVer("2.4.0"))) {
534539
Deno.exit(1);
535540
}
536541
return pkgx;
@@ -541,7 +546,10 @@ function get_pkgx() {
541546

542547
async function* ls() {
543548
for (
544-
const path of [new Path("/usr/local/pkgs"), Path.home().join(".local/pkgs")]
549+
const path of [
550+
new Path("/usr/local/pkgs"),
551+
Path.home().join(".local/pkgs"),
552+
]
545553
) {
546554
if (!path.isDirectory()) continue;
547555
const dirs = [path];
@@ -560,8 +568,15 @@ async function* ls() {
560568
}
561569

562570
async function uninstall(arg: string) {
563-
let found: { project: string } | undefined =
564-
(await hooks.usePantry().find(arg))?.[0];
571+
let found: { project: string } | undefined;
572+
try {
573+
found = (await hooks.usePantry().find(arg))?.[0];
574+
} catch {
575+
console.error(
576+
"%c pantry not found, trying to find package another way",
577+
"color:blue",
578+
);
579+
}
565580
if (!found) {
566581
found = await plumbing.which(arg);
567582
}
@@ -675,8 +690,8 @@ async function outdated() {
675690
for (const { path, pkg } of pkgs) {
676691
const versions = await hooks.useInventory().get(pkg);
677692
// console.log(pkg, graph[pkg.project]);
678-
const constrained_versions = versions.filter((x) =>
679-
graph[pkg.project].satisfies(x) && x.gt(pkg.version)
693+
const constrained_versions = versions.filter(
694+
(x) => graph[pkg.project].satisfies(x) && x.gt(pkg.version),
680695
);
681696
if (constrained_versions.length) {
682697
console.log(
@@ -730,8 +745,8 @@ async function update() {
730745

731746
for (const { pkg } of pkgs) {
732747
const versions = await hooks.useInventory().get(pkg);
733-
const constrained_versions = versions.filter((x) =>
734-
graph[pkg.project].satisfies(x) && x.gt(pkg.version)
748+
const constrained_versions = versions.filter(
749+
(x) => graph[pkg.project].satisfies(x) && x.gt(pkg.version),
735750
);
736751

737752
if (constrained_versions.length) {

0 commit comments

Comments
 (0)