smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(direnv): honored direnv's allow list instead of auto-allowing .envrc

Browse Source 
Removed the direnv-allow preflight so an .envrc the user never allowed is
skipped silently (debug log) and never executed; only already-allowed files
export. Updated the setting description, changelog, and rewrote the tests to
allow explicitly per content change.
This commit is contained in:
can1357
2026-07-23 17:12:22 +02:00
parent 4838ec3686
commit 41fe3e1c31
4 changed files with 60 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+1
View File
@@ -1138,6 +1138,7 @@
### Fixed
- Extended the bash tool's direnv/devenv auto-loading to every backend: the ACP client terminal and the interactive PTY now receive the repo's direnv environment (variables set, and `unset -v` for variables the `.envrc` removes) — previously only the one-shot `executeBash` path did — via a shared preflight so all backends behave identically, with the caller's explicit env still winning. direnv loading also always re-runs `direnv export json` instead of serving a content-hashed cache, so a change to a `watch_file` target re-exports even when the `.envrc` text is unchanged (direnv's own watch invalidation is authoritative). ([#4455](https://github.com/can1357/oh-my-pi/issues/4455))
- The bash tool's direnv auto-load now honors direnv's own allow list: an `.envrc` the user has not `direnv allow`ed is skipped silently and never executed or auto-allowed, keeping OMP's trust boundary identical to the user's shell. ([#4455](https://github.com/can1357/oh-my-pi/issues/4455))
## [16.3.4] - 2026-07-03
packages/coding-agent/src/config/settings-schema.ts
+1 -1
View File
@@ -3356,7 +3356,7 @@ export const SETTINGS_SCHEMA = {
group: "Bash",
label: "direnv Auto-Load",
description:
"Auto-load (and auto-allow) a repo's direnv/devenv `.envrc` into the bash session so devenv tools and env vars are present without manual `direnv exec`",
"Auto-load a repo's direnv/devenv `.envrc` into the bash session so devenv tools and env vars are present without manual `direnv exec`. Honors direnv's allow list: an `.envrc` you haven't `direnv allow`ed is never executed",
},
},
"bash.direnvLoadTimeoutMs": {
packages/coding-agent/src/exec/direnv.ts
+20 -14
View File
@@ -73,7 +73,7 @@ async function runDirenv(
timeoutMs: number,
env: Record<string, string>,
signal?: AbortSignal,
): Promise<{ exitCode: number; stdout: string }> {
): Promise<{ exitCode: number; stdout: string; stderr: string }> {
// Bail on the caller's cancellation as well as the per-invocation cap so a
// cold `.envrc` load can't outlive an aborted / short-timeout bash call.
const abortSignal = signal
@@ -89,28 +89,29 @@ async function runDirenv(
// Drain stdout AND stderr concurrently: a cold `use devenv`/Nix load emits
// enough diagnostics to fill the stderr pipe and block the child forever if
// only stdout is read (it would then wait out `timeoutMs`).
const [stdout] = await Promise.all([
const [stdout, stderr] = await Promise.all([
new Response(proc.stdout as ReadableStream<Uint8Array>).text(),
new Response(proc.stderr as ReadableStream<Uint8Array>).text(),
]);
const exitCode = await proc.exited;
return { exitCode, stdout };
return { exitCode, stdout, stderr };
}
/**
* Resolve the nearest `.envrc` from `cwd`, auto-allow it, and return its
* `direnv export` diff (variables to set, and variables direnv removes).
* Returns `null` when there is no `.envrc`, `direnv` is not installed, or the
* export fails/times out.
* Resolve the nearest `.envrc` from `cwd` and return its `direnv export` diff
* (variables to set, and variables direnv removes). Returns `null` when there
* is no `.envrc`, `direnv` is not installed, the `.envrc` is not on direnv's
* allow list, or the export fails/times out.
*
* direnv's own allow list is honored — an `.envrc` the user has not
* `direnv allow`ed is NEVER executed or auto-allowed. This keeps OMP's trust
* boundary identical to the user's own shell: cloning a repo with a poisoned
* `.envrc` grants it nothing until the user explicitly allows it.
*
* Always re-invokes `direnv export json` rather than serving a cached diff:
* direnv is fast once warm, and its own watch/mtime invalidation is the
* authoritative freshness signal (a content-hash cache here would go stale when
* a `watch_file` target changes without the `.envrc` text changing).
*
* Auto-allow is deliberate: OMP already runs the repository's own code, so its
* `.envrc` is trusted under the same model rather than forcing a manual
* `direnv allow`.
*/
export async function loadDirenvEnv(
cwd: string,
@@ -125,10 +126,15 @@ export async function loadDirenvEnv(
const timeoutMs = opts?.timeoutMs ?? DEFAULT_DIRENV_TIMEOUT_MS;
const env = cleanSpawnEnv();
try {
await runDirenv(bin, ["allow"], dir, timeoutMs, env, opts?.signal);
const { exitCode, stdout } = await runDirenv(bin, ["export", "json"], dir, timeoutMs, env, opts?.signal);
const { exitCode, stdout, stderr } = await runDirenv(bin, ["export", "json"], dir, timeoutMs, env, opts?.signal);
if (exitCode !== 0) {
logger.warn("direnv export failed", { dir, exitCode });
// A not-yet-allowed .envrc is an expected steady state (the user opted
// out by never running `direnv allow`), not an error worth warning on.
if (stderr.includes("is blocked")) {
logger.debug("direnv .envrc not allowed; skipping", { dir });
} else {
logger.warn("direnv export failed", { dir, exitCode });
}
return null;
}
return parseDirenvExport(stdout);
packages/coding-agent/test/direnv.test.ts
+38 -2
View File
@@ -20,6 +20,20 @@ afterEach(async () => {
for (const dir of tmpDirs.splice(0)) await dir.remove();
});
/** Explicitly `direnv allow` an `.envrc` (against the test-isolated HOME/XDG
* dirs). Loading now honors the allow list, so tests that expect an export
* must opt the file in the same way a user would. Re-run after every content
* change — direnv's allow entry is content-hashed. */
async function allowEnvrc(dir: string): Promise<void> {
const env: Record<string, string> = {};
for (const key in Bun.env) {
const value = Bun.env[key];
if (value !== undefined && !key.startsWith("DIRENV_")) env[key] = value;
}
const proc = Bun.spawn(["direnv", "allow"], { cwd: dir, env, stdout: "ignore", stderr: "ignore" });
if ((await proc.exited) !== 0) throw new Error(`direnv allow failed in ${dir}`);
}
describe("findEnvrc", () => {
it("walks up to the nearest .envrc above the start dir", async () => {
const root = tmp();
@@ -62,7 +76,7 @@ describe("parseDirenvExport", () => {
});
});
describe.skipIf(!hasDirenv)("loadDirenvEnv (real direnv, auto-allow)", () => {
describe.skipIf(!hasDirenv)("loadDirenvEnv (real direnv, allow-list honored)", () => {
// `direnv allow` writes a trust entry into its data dir. Redirect HOME + the
// XDG dirs to a throwaway tmp so real-direnv cases never leak allow state
// into the dev/CI user's global direnv store.
@@ -81,10 +95,21 @@ describe.skipIf(!hasDirenv)("loadDirenvEnv (real direnv, auto-allow)", () => {
}
});
it("auto-allows an untrusted .envrc and returns its exported vars + PATH additions", async () => {
it("returns null for an .envrc the user has not allowed (never auto-allows)", async () => {
// The security contract: an untrusted `.envrc` is NEVER executed. A
// poisoned repo gets nothing until the user runs `direnv allow` — exactly
// the trust boundary direnv itself enforces in the user's shell.
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "export DIRENV_BLOCKED_TEST=leaked\n");
expect(await loadDirenvEnv(root)).toBeNull();
});
it("returns exported vars + PATH additions for an allowed .envrc", async () => {
const root = tmp();
await fs.mkdir(path.join(root, "bin"), { recursive: true });
await Bun.write(path.join(root, ".envrc"), "export DIRENV_FEATURE_TEST=loaded\nPATH_add bin\n");
await allowEnvrc(root);
const diff = await loadDirenvEnv(root);
@@ -95,6 +120,7 @@ describe.skipIf(!hasDirenv)("loadDirenvEnv (real direnv, auto-allow)", () => {
it("reports variables a .envrc unsets", async () => {
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "unset PI_DIRENV_UNSET_TEST\n");
await allowEnvrc(root);
// direnv emits a JSON null for a var only when it was present in the
// spawn env and the `.envrc` removes it, so seed it in the parent env.
// (Avoid a `DIRENV_`-prefixed name — the loader strips those before spawn.)
@@ -113,11 +139,15 @@ describe.skipIf(!hasDirenv)("loadDirenvEnv (real direnv, auto-allow)", () => {
});
it("re-exports when the .envrc content changes (no stale cache)", async () => {
// direnv's allow entry is content-hashed, so each rewrite is re-allowed —
// mirroring what a user does after editing their own .envrc.
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "export DIRENV_CACHE_TEST=one\n");
await allowEnvrc(root);
expect((await loadDirenvEnv(root))?.set.DIRENV_CACHE_TEST).toBe("one");
await Bun.write(path.join(root, ".envrc"), "export DIRENV_CACHE_TEST=two\n");
await allowEnvrc(root);
expect((await loadDirenvEnv(root))?.set.DIRENV_CACHE_TEST).toBe("two");
});
@@ -133,6 +163,7 @@ describe.skipIf(!hasDirenv)("loadDirenvEnv (real direnv, auto-allow)", () => {
path.join(root, ".envrc"),
'watch_file watched.env\nexport DIRENV_WATCH_TEST="$(cat watched.env)"\n',
);
await allowEnvrc(root);
expect((await loadDirenvEnv(root))?.set.DIRENV_WATCH_TEST).toBe("one");
await Bun.write(path.join(root, "watched.env"), "two\n");
@@ -159,6 +190,7 @@ describe.skipIf(!hasDirenv)("bash executor direnv wiring (end-to-end)", () => {
it("exposes direnv-loaded vars to the command while per-call env still wins", async () => {
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "export DIRENV_WIRE_TEST=fromdirenv\nexport OVERRIDE_ME=fromdirenv\n");
await allowEnvrc(root);
const result = await executeBash('printf "%s|%s" "$DIRENV_WIRE_TEST" "$OVERRIDE_ME"', {
cwd: root,
@@ -171,6 +203,7 @@ describe.skipIf(!hasDirenv)("bash executor direnv wiring (end-to-end)", () => {
it("removes variables the .envrc unsets from the command environment", async () => {
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "unset PI_DIRENV_UNSET_E2E\n");
await allowEnvrc(root);
// Inherited from the process env (as an OMP-provided var would be); the
// caller does NOT re-supply it, so direnv's unset must strip it. `printenv`
// exits non-zero and prints nothing when the name is genuinely absent. A
@@ -214,6 +247,7 @@ describe.skipIf(!hasDirenv)("applyDirenvPreflight (shared all-backends preflight
it("merges direnv set vars into env while the caller's env wins", async () => {
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "export DIRENV_PF_SET=fromdirenv\nexport OVERRIDE_ME=fromdirenv\n");
await allowEnvrc(root);
const { command, env } = await applyDirenvPreflight("echo hi", root, {
callerEnv: { OVERRIDE_ME: "fromcaller" },
@@ -229,6 +263,7 @@ describe.skipIf(!hasDirenv)("applyDirenvPreflight (shared all-backends preflight
it("prepends a regex-gated `unset -v` for vars the .envrc removes, skipping caller-resupplied names", async () => {
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "unset PI_PF_UNSET_A\nunset PI_PF_UNSET_B\n");
await allowEnvrc(root);
Bun.env.PI_PF_UNSET_A = "present";
Bun.env.PI_PF_UNSET_B = "present";
try {
@@ -249,6 +284,7 @@ describe.skipIf(!hasDirenv)("applyDirenvPreflight (shared all-backends preflight
it("applies the shell commandPrefix after the unset prefix", async () => {
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "unset PI_PF_ORDER\n");
await allowEnvrc(root);
Bun.env.PI_PF_ORDER = "present";
try {
const { command } = await applyDirenvPreflight("payload", root, {