From aba94216475077fde64701db082ca5d6dfd0546f Mon Sep 17 00:00:00 2001 From: Jon Lauridsen Date: Thu, 7 May 2026 02:25:49 +0200 Subject: [PATCH 1/4] Fix #51: pass dir to dev when activating from subdir The `chpwd` hook walks up from `$PWD` looking for an activation marker, but when it found one in a parent dir it invoked `dev` with no arguments, making `dev` sniff `$PWD` (the subdir, with no keyfiles) instead of the activated dir. `dev` exited with "no devenv detected" and empty stdout, and the dir intended for `dev` got passed to eval as a positional arg instead, which the shell tried to execute, producing "permission denied: " on `cd`. This change moves "$dir" inside the command substitution, so `dev` sniffs the activated directory. This also adds an integration test that drives `zsh` end-to-end with a fake `dev` on PATH to verify the hook invokes `dev` with the activated dir as argv[1] and produces no permission-denied error. --- src/shellcode().test.ts | 134 ++++++++++++++++++++++++++++++++++++++++ src/shellcode().ts | 2 +- 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 src/shellcode().test.ts diff --git a/src/shellcode().test.ts b/src/shellcode().test.ts new file mode 100644 index 0000000..a0a65be --- /dev/null +++ b/src/shellcode().test.ts @@ -0,0 +1,134 @@ +import { assert, assertEquals } from "jsr:@std/assert"; +import { Path } from "libpkgx"; +import shellcode, { datadir } from "./shellcode().ts"; + +// Exercises `_pkgx_chpwd_hook` end-to-end via a zsh subprocess with a fake +// `dev` binary on PATH. Reproduces the bug from issue #51 where cd-ing +// directly into a subdir of an activated devenv fails to activate and emits +// `permission denied`. + +async function run_hook_in_subdir(): Promise< + { stdout: string; stderr: string; recorded_args: string[] } +> { + const tmp = Path.mktemp(); + const proj = tmp.join("proj").mkdir(); + const sub = proj.join("sub").mkdir(); + + // Override XDG_DATA_HOME so datadir() lives in the temp tree. + const xdg = tmp.join("xdg").mkdir(); + + // Pre-create the activation marker for `proj`. + const marker_dir = xdg.join( + "pkgx", + "dev", + proj.string.slice(1), + ).mkdir("p"); + marker_dir.join("dev.pkgx.activated").touch(); + + // Fake `dev` that records its argv and emits a sentinel shell command we + // can detect in stdout. Crucially: the real `dev` (with no args) would + // sniff $PWD, find nothing, and exit 1 — the bug we're testing. + const fake_bin = tmp.join("bin").mkdir(); + const fake_dev = fake_bin.join("dev"); + const log = tmp.join("dev-args.log"); + Deno.writeTextFileSync( + fake_dev.string, + `#!/bin/sh\nprintf '%s\\n' "$@" >> "${log.string}"\necho "echo HOOK_OK"\n`, + ); + Deno.chmodSync(fake_dev.string, 0o755); + + const env = { + ...Deno.env.toObject(), + XDG_DATA_HOME: xdg.string, + PATH: `${fake_bin.string}:${Deno.env.get("PATH") ?? ""}`, + }; + + // Generate shellcode using a PATH that resolves `dev` to our fake and an + // XDG_DATA_HOME that points at our temp datadir (both are baked in at + // codegen time). + const original_path = Deno.env.get("PATH"); + const original_xdg = Deno.env.get("XDG_DATA_HOME"); + Deno.env.set("PATH", env.PATH); + Deno.env.set("XDG_DATA_HOME", xdg.string); + let code: string; + try { + code = shellcode(); + } finally { + if (original_path !== undefined) Deno.env.set("PATH", original_path); + if (original_xdg === undefined) Deno.env.delete("XDG_DATA_HOME"); + else Deno.env.set("XDG_DATA_HOME", original_xdg); + } + + // Simulate the user: shell starts in HOME, then cd's directly into the + // subdirectory of the already-activated project. + const script = ` +${code} +cd "${sub.string}" +`; + + const script_path = tmp.join("script.zsh"); + Deno.writeTextFileSync(script_path.string, script); + + const proc = await new Deno.Command("zsh", { + args: [script_path.string], + env, + stdout: "piped", + stderr: "piped", + }).output(); + + const recorded_args = log.isFile() + ? Deno.readTextFileSync(log.string).split("\n").filter((x) => x.length > 0) + : []; + + return { + stdout: new TextDecoder().decode(proc.stdout), + stderr: new TextDecoder().decode(proc.stderr), + recorded_args, + }; +} + +Deno.test("chpwd hook activates when cd-ing directly into subdir of devenv", async () => { + const { stdout, stderr, recorded_args } = await run_hook_in_subdir(); + + // The hook must invoke our fake dev and run its emitted shellcode. + assert( + stdout.includes("HOOK_OK"), + `expected hook to eval dev's stdout (HOOK_OK), got stdout=${ + JSON.stringify(stdout) + } stderr=${JSON.stringify(stderr)}`, + ); + + // Crucially: no "permission denied" from the shell trying to execute a + // directory path as a command (the bug from issue #51). + assert( + !/permission denied/i.test(stderr), + `unexpected 'permission denied' in stderr: ${stderr}`, + ); + + // dev must have been invoked with the activated dir so it sniffs the + // right place — not invoked bare while $PWD points at the subdir. + assertEquals( + recorded_args.length, + 1, + `expected dev to be called with exactly one argument, got: ${ + JSON.stringify(recorded_args) + }`, + ); + assert( + recorded_args[0].endsWith("/proj"), + `expected dev to be called with the activated dir, got: ${ + recorded_args[0] + }`, + ); +}); + +Deno.test("datadir respects XDG_DATA_HOME", () => { + const original = Deno.env.get("XDG_DATA_HOME"); + try { + Deno.env.set("XDG_DATA_HOME", "/tmp/xdg-test"); + assertEquals(datadir().string, "/tmp/xdg-test/pkgx/dev"); + } finally { + if (original === undefined) Deno.env.delete("XDG_DATA_HOME"); + else Deno.env.set("XDG_DATA_HOME", original); + } +}); diff --git a/src/shellcode().ts b/src/shellcode().ts index 0fb82e0..6515769 100644 --- a/src/shellcode().ts +++ b/src/shellcode().ts @@ -15,7 +15,7 @@ _pkgx_chpwd_hook() { dir="$PWD" while [ "$dir" != / -a "$dir" != . ]; do if [ -f "${datadir()}/$dir/dev.pkgx.activated" ]; then - eval "$(${dev_cmd})" "$dir" + eval "$(${dev_cmd} "$dir")" break fi dir="$(dirname "$dir")" From 9ca511fa25c0666d001810f69ebae3061328a93a Mon Sep 17 00:00:00 2001 From: Jon Lauridsen Date: Thu, 7 May 2026 02:48:45 +0200 Subject: [PATCH 2/4] Refactor: thread env explicitly through shellcode/datadir shellcode() and datadir() read PATH and XDG_DATA_HOME from Deno.env implicitly at codegen time, which made the integration test for issue #51 noisy: it had to mutate process env, save originals, and restore them so the generated shellcode would point at the test's temp dirs. This change makes env an explicit parameter (defaulting to Deno.env.toObject()) so production callers in app.ts stay unchanged, but tests can pass a clean env per-invocation. Test drops from 130 to 68 lines, without all the save/restore ceremony. --- src/shellcode().test.ts | 125 +++++++++------------------------------- src/shellcode().ts | 26 +++++---- 2 files changed, 43 insertions(+), 108 deletions(-) diff --git a/src/shellcode().test.ts b/src/shellcode().test.ts index a0a65be..791ff6d 100644 --- a/src/shellcode().test.ts +++ b/src/shellcode().test.ts @@ -2,133 +2,64 @@ import { assert, assertEquals } from "jsr:@std/assert"; import { Path } from "libpkgx"; import shellcode, { datadir } from "./shellcode().ts"; -// Exercises `_pkgx_chpwd_hook` end-to-end via a zsh subprocess with a fake -// `dev` binary on PATH. Reproduces the bug from issue #51 where cd-ing -// directly into a subdir of an activated devenv fails to activate and emits -// `permission denied`. - -async function run_hook_in_subdir(): Promise< - { stdout: string; stderr: string; recorded_args: string[] } -> { +// Issue #51: cd-ing directly into a subdir of an already-activated devenv +// must activate the devenv (and not emit `permission denied`). This drives +// the generated shellcode through a real zsh subprocess with a fake `dev` +// on PATH so we can assert how the chpwd hook invokes it. +Deno.test("chpwd hook activates when cd-ing into subdir of devenv (#51)", async () => { const tmp = Path.mktemp(); const proj = tmp.join("proj").mkdir(); const sub = proj.join("sub").mkdir(); - - // Override XDG_DATA_HOME so datadir() lives in the temp tree. const xdg = tmp.join("xdg").mkdir(); + const bin = tmp.join("bin").mkdir(); + const log = tmp.join("dev-args.log"); - // Pre-create the activation marker for `proj`. - const marker_dir = xdg.join( - "pkgx", - "dev", - proj.string.slice(1), - ).mkdir("p"); - marker_dir.join("dev.pkgx.activated").touch(); + // pre-activate `proj` (not `sub`) — that's the case from the bug report + xdg.join("pkgx", "dev").join(proj.string.slice(1)).mkdir("p") + .join("dev.pkgx.activated").touch(); - // Fake `dev` that records its argv and emits a sentinel shell command we - // can detect in stdout. Crucially: the real `dev` (with no args) would - // sniff $PWD, find nothing, and exit 1 — the bug we're testing. - const fake_bin = tmp.join("bin").mkdir(); - const fake_dev = fake_bin.join("dev"); - const log = tmp.join("dev-args.log"); + // fake `dev` records its argv and emits a sentinel for eval to run + const fake_dev = bin.join("dev"); Deno.writeTextFileSync( fake_dev.string, - `#!/bin/sh\nprintf '%s\\n' "$@" >> "${log.string}"\necho "echo HOOK_OK"\n`, + `#!/bin/sh\nprintf '%s\\n' "$@" >> "${log}"\necho 'echo HOOK_OK'\n`, ); Deno.chmodSync(fake_dev.string, 0o755); const env = { ...Deno.env.toObject(), + PATH: `${bin}:${Deno.env.get("PATH") ?? ""}`, XDG_DATA_HOME: xdg.string, - PATH: `${fake_bin.string}:${Deno.env.get("PATH") ?? ""}`, }; - // Generate shellcode using a PATH that resolves `dev` to our fake and an - // XDG_DATA_HOME that points at our temp datadir (both are baked in at - // codegen time). - const original_path = Deno.env.get("PATH"); - const original_xdg = Deno.env.get("XDG_DATA_HOME"); - Deno.env.set("PATH", env.PATH); - Deno.env.set("XDG_DATA_HOME", xdg.string); - let code: string; - try { - code = shellcode(); - } finally { - if (original_path !== undefined) Deno.env.set("PATH", original_path); - if (original_xdg === undefined) Deno.env.delete("XDG_DATA_HOME"); - else Deno.env.set("XDG_DATA_HOME", original_xdg); - } - - // Simulate the user: shell starts in HOME, then cd's directly into the - // subdirectory of the already-activated project. - const script = ` -${code} -cd "${sub.string}" -`; - - const script_path = tmp.join("script.zsh"); - Deno.writeTextFileSync(script_path.string, script); - const proc = await new Deno.Command("zsh", { - args: [script_path.string], + args: ["-c", `${shellcode(env)}\ncd "${sub}"`], env, stdout: "piped", stderr: "piped", }).output(); - const recorded_args = log.isFile() - ? Deno.readTextFileSync(log.string).split("\n").filter((x) => x.length > 0) + const stdout = new TextDecoder().decode(proc.stdout); + const stderr = new TextDecoder().decode(proc.stderr); + const dev_args = log.isFile() + ? Deno.readTextFileSync(log.string).split("\n").filter(Boolean) : []; - return { - stdout: new TextDecoder().decode(proc.stdout), - stderr: new TextDecoder().decode(proc.stderr), - recorded_args, - }; -} - -Deno.test("chpwd hook activates when cd-ing directly into subdir of devenv", async () => { - const { stdout, stderr, recorded_args } = await run_hook_in_subdir(); - - // The hook must invoke our fake dev and run its emitted shellcode. assert( stdout.includes("HOOK_OK"), - `expected hook to eval dev's stdout (HOOK_OK), got stdout=${ - JSON.stringify(stdout) - } stderr=${JSON.stringify(stderr)}`, + `hook should eval dev's stdout. stdout=${stdout} stderr=${stderr}`, ); - - // Crucially: no "permission denied" from the shell trying to execute a - // directory path as a command (the bug from issue #51). - assert( - !/permission denied/i.test(stderr), - `unexpected 'permission denied' in stderr: ${stderr}`, - ); - - // dev must have been invoked with the activated dir so it sniffs the - // right place — not invoked bare while $PWD points at the subdir. + assertEquals(stderr, "", "hook should produce no stderr"); assertEquals( - recorded_args.length, - 1, - `expected dev to be called with exactly one argument, got: ${ - JSON.stringify(recorded_args) - }`, - ); - assert( - recorded_args[0].endsWith("/proj"), - `expected dev to be called with the activated dir, got: ${ - recorded_args[0] - }`, + dev_args, + [proj.string], + "dev must be invoked with the activated dir, not bare", ); }); Deno.test("datadir respects XDG_DATA_HOME", () => { - const original = Deno.env.get("XDG_DATA_HOME"); - try { - Deno.env.set("XDG_DATA_HOME", "/tmp/xdg-test"); - assertEquals(datadir().string, "/tmp/xdg-test/pkgx/dev"); - } finally { - if (original === undefined) Deno.env.delete("XDG_DATA_HOME"); - else Deno.env.set("XDG_DATA_HOME", original); - } + assertEquals( + datadir({ XDG_DATA_HOME: "/tmp/xdg-test" }).string, + "/tmp/xdg-test/pkgx/dev", + ); }); diff --git a/src/shellcode().ts b/src/shellcode().ts index 6515769..6848fb7 100644 --- a/src/shellcode().ts +++ b/src/shellcode().ts @@ -1,20 +1,24 @@ import { Path } from "libpkgx"; -export default function shellcode() { +type Env = Record; + +export default function shellcode(env: Env = Deno.env.toObject()) { // find self - const dev_cmd = Deno.env.get("PATH")?.split(":").map((path) => + const dev_cmd = env.PATH?.split(":").map((path) => Path.abs(path)?.join("dev") ) .filter((x) => x?.isExecutableFile())[0]; if (!dev_cmd) throw new Error("couldn’t find `dev`"); + const dd = datadir(env); + return ` _pkgx_chpwd_hook() { if ! type _pkgx_dev_try_bye >/dev/null 2>&1 || _pkgx_dev_try_bye; then dir="$PWD" while [ "$dir" != / -a "$dir" != . ]; do - if [ -f "${datadir()}/$dir/dev.pkgx.activated" ]; then + if [ -f "${dd}/$dir/dev.pkgx.activated" ]; then eval "$(${dev_cmd} "$dir")" break fi @@ -29,8 +33,8 @@ dev() { if type -f _pkgx_dev_try_bye >/dev/null 2>&1; then dir="$PWD" while [ "$dir" != / -a "$dir" != . ]; do - if [ -f "${datadir()}/$dir/dev.pkgx.activated" ]; then - rm "${datadir()}/$dir/dev.pkgx.activated" + if [ -f "${dd}/$dir/dev.pkgx.activated" ]; then + rm "${dd}/$dir/dev.pkgx.activated" break fi dir="$(dirname "$dir")" @@ -43,8 +47,8 @@ dev() { if [ "$2" ]; then "${dev_cmd}" "$@" elif ! type -f _pkgx_dev_try_bye >/dev/null 2>&1; then - mkdir -p "${datadir()}$PWD" - touch "${datadir()}$PWD/dev.pkgx.activated" + mkdir -p "${dd}$PWD" + touch "${dd}$PWD/dev.pkgx.activated" eval "$(${dev_cmd})" else echo "devenv already active" >&2 @@ -77,19 +81,19 @@ fi `.trim(); } -export function datadir() { +export function datadir(env: Env = Deno.env.toObject()) { return new Path( - Deno.env.get("XDG_DATA_HOME")?.trim() || platform_data_home_default(), + env.XDG_DATA_HOME?.trim() || platform_data_home_default(env), ).join("pkgx", "dev"); } -function platform_data_home_default() { +function platform_data_home_default(env: Env) { const home = Path.home(); switch (Deno.build.os) { case "darwin": return home.join("Library/Application Support"); case "windows": { - const LOCALAPPDATA = Deno.env.get("LOCALAPPDATA"); + const LOCALAPPDATA = env.LOCALAPPDATA; if (LOCALAPPDATA) { return new Path(LOCALAPPDATA); } else { From afb758f5642f01e22d8b8c8dc2ed0cea49816b96 Mon Sep 17 00:00:00 2001 From: Jon Lauridsen Date: Thu, 7 May 2026 03:14:08 +0200 Subject: [PATCH 3/4] Change test to drive chpwd hook with bash, not zsh CI Ubuntu runners ship without zsh, so the integration test failed with `NotFound: Failed to spawn 'zsh'`. This changes the new test to rely on bash. --- src/shellcode().test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shellcode().test.ts b/src/shellcode().test.ts index 791ff6d..f57e048 100644 --- a/src/shellcode().test.ts +++ b/src/shellcode().test.ts @@ -4,7 +4,7 @@ import shellcode, { datadir } from "./shellcode().ts"; // Issue #51: cd-ing directly into a subdir of an already-activated devenv // must activate the devenv (and not emit `permission denied`). This drives -// the generated shellcode through a real zsh subprocess with a fake `dev` +// the generated shellcode through a real bash subprocess with a fake `dev` // on PATH so we can assert how the chpwd hook invokes it. Deno.test("chpwd hook activates when cd-ing into subdir of devenv (#51)", async () => { const tmp = Path.mktemp(); @@ -32,7 +32,7 @@ Deno.test("chpwd hook activates when cd-ing into subdir of devenv (#51)", async XDG_DATA_HOME: xdg.string, }; - const proc = await new Deno.Command("zsh", { + const proc = await new Deno.Command("bash", { args: ["-c", `${shellcode(env)}\ncd "${sub}"`], env, stdout: "piped", From 0b959fd7b98f7ae5dd3be51fda8dcba9eb712d67 Mon Sep 17 00:00:00 2001 From: Jon Lauridsen Date: Thu, 7 May 2026 03:17:58 +0200 Subject: [PATCH 4/4] Use `--allow-run=bash` in CI + specify in AGENTS.md The new chpwd-hook integration test spawns a bash subprocess to drive the generated shellcode end-to-end, so --allow-run` is required. This change also updates AGENTS.md's documented test command to match the change in ci.yml. Scoped to bash so the test suite cannot spawn arbitrary executables. --- .github/workflows/ci.yml | 2 +- AGENTS.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8cf0d77..5ea120d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,4 +40,4 @@ jobs: - uses: denolib/setup-deno@v2 with: deno-version: v2.x - - run: deno test --allow-read --allow-write --allow-env + - run: deno test --allow-read --allow-write --allow-env --allow-run=bash diff --git a/AGENTS.md b/AGENTS.md index 6b7eaa4..229b42b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,7 +7,7 @@ Public repository for developer environment activation tooling. - `deno fmt --check .` - `deno lint .` - `deno check ./app.ts` -- `deno test --allow-read --allow-write --allow-env` +- `deno test --allow-read --allow-write --allow-env --allow-run=bash` ## Always Do