fix(ui-backend-pi): run the bash tool without a login shell

.changeset/pi-bash-no-login-shell.md

@@ -0,0 +1,7 @@
+---
+"@schlessera/brain-backend-pi": patch
+---
+
+The pi backend's `bash` tool no longer runs commands in a login shell (#2960). It used `bash -lc`, so `/etc/profile` and the first of `~/.bash_profile`, `~/.bash_login` or `~/.profile` under the `HOME` the command received ran before every command. Their output reached the model in the tool result, and every variable or `PATH` entry they exported overrode the allowlisted environment, including tokens the allowlist withholds on purpose. Commands now run through `bash --noprofile --norc -c`, and `BASH_ENV` and `ENV` are removed from the command's environment even when admitted through `BRAIN_UI_SUBPROCESS_ENV_EXTRA`, so neither the command nor a shell it starts reads a startup file.
+
+A command's `PATH` and variables are now exactly the filtered environment. If a host relied on a login profile to put a tool on `PATH`, put that directory on the server's own `PATH`; set a variable a profile used to export in the server's environment and admit it with `BRAIN_UI_SUBPROCESS_ENV_EXTRA`.

packages/ui-backend-pi/README.md

@@ -399,13 +399,23 @@ a real `SessionManager` session file).
 ## The exec wrapper, and what it does not cover
 
 `BRAIN_UI_EXEC_WRAPPER` (below) is an absolute path to an executable that the
-`bash` and `grep` tools are launched through: `<wrapper> bash -lc <cmd>`. It is
-an argv[0], never a command line — no shell parses it — so a host can run the
+`bash` and `grep` tools are launched through:
+`<wrapper> bash --noprofile --norc -c <cmd>` and `<wrapper> grep …`. It is an
+argv[0], never a command line — no shell parses it — so a host can run the
 agent's children as another user without this package knowing how. Unset,
 spawns are exactly what they were. When it is set, the child leads its own
 process group and an abort signals the group, because a uid drop otherwise
 makes `kill(2)` fail with EPERM and leaves an aborted turn running.
 
+The `bash` tool's shell is not a login shell and reads no startup file: not
+`/etc/profile`, not a profile or `.bashrc` under `HOME`, and not the file
+`BASH_ENV` or `ENV` names, which are removed from the command's environment
+even when an operator admits them. A command's `PATH` and variables are exactly
+the filtered environment ([Environment](#environment)). A tool that only a login
+profile put on `PATH` belongs on the server's own `PATH`; a variable a profile
+exported is set in the server's environment and admitted through
+`BRAIN_UI_SUBPROCESS_ENV_EXTRA`.
+
 **It does not cover `brain.config.ts`.** `createBrainAccess` calls
 `initContext`, which `await import`s the repository's config — executable
 TypeScript owned by the brain, evaluated in the server process, not in a child.

packages/ui-backend-pi/src/tools.ts

@@ -563,7 +563,11 @@ export function createBrainTools(deps: BrainToolDeps): ToolDefinition[] {
       // by now (and may have edited the command via updatedInput). The rtk
       // rewrite runs AFTER the gate, so confirm patterns see the command as
       // the model wrote it; when rtk is absent or declines, it is untouched.
-      const childEnv = { ...subprocessEnv([...resolveEnabledWebSearchEnvNames(), ...WORKER_EGRESS_TRANSPORT_ENV]), ...RTK_WORKER_ENV };
+      const childEnv: NodeJS.ProcessEnv = { ...subprocessEnv([...resolveEnabledWebSearchEnvNames(), ...WORKER_EGRESS_TRANSPORT_ENV]), ...RTK_WORKER_ENV };
+      // A non-interactive bash sources the file BASH_ENV names (sh reads ENV),
+      // here and in every shell the command starts (#2960).
+      delete childEnv.BASH_ENV;
+      delete childEnv.ENV;
       const cmd = await rtkRewriteCommand(params.command, childEnv);
       if (signal?.aborted) throw new DOMException("Tool cancelled before lock admission", "AbortError");
       // Only commands that touch git staging/history or the brain CLI's
@@ -573,7 +577,12 @@ export function createBrainTools(deps: BrainToolDeps): ToolDefinition[] {
       return lock.withKey(bashLockKey(params.command), async () => {
         if (signal?.aborted) throw new DOMException("Tool cancelled before subprocess execution", "AbortError");
         const exec = deps.exec ?? ambientExecConfig();
-        const proc = spawn(wrapCommand(["bash", "-lc", cmd], exec.wrapper), {
+        // Not a login shell (#2960): `-l` sourced /etc/profile and the first
+        // profile under HOME before every command, so their output reached
+        // the result and anything they exported, PATH included, overrode the
+        // allowlisted childEnv. --noprofile --norc reads no startup file; the
+        // command sees exactly childEnv.
+        const proc = spawn(wrapCommand(["bash", "--noprofile", "--norc", "-c", cmd], exec.wrapper), {
           cwd: deps.shellCwd ?? brain.root,
           env: childEnv,
           stdout: "pipe",

packages/ui-backend-pi/tests/exec-wrapper.test.ts

@@ -79,7 +79,9 @@ describe("the bash tool", () => {
         // Absolute, so the helper can execv it and PATH cannot choose which
         // bash runs.
         Bun.which("bash")!,
-        "-lc",
+        "--noprofile",
+        "--norc",
+        "-c",
         "echo brain-kit-ok",
       ]);
     } finally {
@@ -125,7 +127,9 @@ describe("the bash tool", () => {
       expect(resultText(res)).toContain("still-fine");
       expect(readFileSync(argvLog, "utf-8").split("\n").filter(Boolean)).toEqual([
         Bun.which("bash")!,
-        "-lc",
+        "--noprofile",
+        "--norc",
+        "-c",
         "echo still-fine",
       ]);
       expect(existsSync(join(brain.root, "pwned"))).toBe(false);
@@ -184,7 +188,7 @@ test("host A/C and explicit empty override ambient B for both Pi spawn tools (#1
       expect(existsSync(b.argvLog), "EXPLICIT_EMPTY_NOT_AMBIENT").toBe(false);
     }
     expect(readFileSync(a.argvLog, "utf8").split("\n").filter(Boolean)).toEqual([
-      Bun.which("bash")!, "-lc", "echo needle", Bun.which("grep")!, "-rInE", "--", "needle", ".",
+      Bun.which("bash")!, "--noprofile", "--norc", "-c", "echo needle", Bun.which("grep")!, "-rInE", "--", "needle", ".",
     ]);
     expect(readFileSync(c.argvLog, "utf8")).toBe(readFileSync(a.argvLog, "utf8"));
   } finally { brain.cleanup(); }

packages/ui-backend-pi/tests/tools-permission.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, test } from "bun:test";
-import { readFileSync, writeFileSync } from "fs";
-import { join } from "path";
+import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "fs";
+import { tmpdir } from "os";
+import { dirname, join } from "path";
 
 import type { ToolDefinition } from "@earendil-works/pi-coding-agent";
 import type { PermissionDecision } from "@schlessera/brain-ui-sdk/server";
@@ -489,6 +490,81 @@ describe("curated tools execute without in-tool gating", () => {
     }
   });
 
+  /**
+   * Runs `command` through the real bash tool with a fixture HOME (#2960):
+   * `files` are written under it, `env` is set on the host for the call and
+   * `{HOME}` in either is replaced by the fixture's path.
+   */
+  async function bashWithHome(command: string, files: Record<string, string>, env: Record<string, string> = {}) {
+    const brain = makeEmptyBrain();
+    const home = mkdtempSync(join(tmpdir(), "pi-bash-home-"));
+    const names = ["HOME", ...Object.keys(env)];
+    const saved = new Map(names.map((name) => [name, process.env[name]]));
+    try {
+      for (const [rel, content] of Object.entries(files)) {
+        mkdirSync(dirname(join(home, rel)), { recursive: true });
+        writeFileSync(join(home, rel), content.replaceAll("{HOME}", home), { mode: 0o755 });
+      }
+      process.env.HOME = home;
+      for (const [name, value] of Object.entries(env)) process.env[name] = value.replaceAll("{HOME}", home);
+      const tools = toolMap(createBrainTools({
+        brain: createBrainAccess(brain.root),
+        turn: createTurnContext(),
+        lock: toolLockFromKeyed(createKeyedLock()),
+      }));
+      return await tools.bash!.execute("bash-home", { command }, undefined, undefined, CTX);
+    } finally {
+      for (const [name, value] of saved) {
+        if (value === undefined) delete process.env[name];
+        else process.env[name] = value;
+      }
+      rmSync(home, { recursive: true, force: true });
+      brain.cleanup();
+    }
+  }
+
+  const NOISY = "echo profile-stdout\necho profile-stderr >&2\n";
+  const NOISY_HOME = { ".bash_profile": NOISY, ".bash_login": NOISY, ".profile": NOISY, ".bashrc": NOISY };
+
+  test.each([
+    ["echo hi", "hi"],
+    ["true", "(no output)"],
+  ])("bash output carries nothing a profile in HOME prints: %s", async (command, output) => {
+    const res = await bashWithHome(command, NOISY_HOME);
+    expect(resultText(res)).toBe(output);
+    expect(res.details).toEqual({ exitCode: 0 });
+  });
+
+  test("a profile in HOME sets no variable and no PATH entry the command sees", async () => {
+    const leak = [
+      "export GH_TOKEN=from-profile",
+      "export BRAIN_TEST_PROFILE_LEAK=1",
+      'export PATH="{HOME}/planted-bin:$PATH"',
+      "",
+    ].join("\n");
+    const res = await bashWithHome(
+      'printf "tok=[%s] leak=[%s] probe=[%s] path=[%s]" "${GH_TOKEN:-unset}" "${BRAIN_TEST_PROFILE_LEAK:-unset}" '
+        + '"$(command -v brain-profile-probe || echo absent)" "$(case ":$PATH:" in *planted-bin*) echo planted;; *) echo clean;; esac)"',
+      {
+        ".bash_profile": leak, ".bash_login": leak, ".profile": leak, ".bashrc": leak,
+        "planted-bin/brain-profile-probe": "#!/bin/sh\necho planted-probe\n",
+      },
+      // Not on the agent allowlist: the allowlist removes the host value.
+      { GH_TOKEN: "host-value" },
+    );
+    expect(resultText(res)).toBe("tok=[unset] leak=[unset] probe=[absent] path=[clean]");
+  });
+
+  test("an operator-admitted BASH_ENV or ENV names no startup file the command or its shells source", async () => {
+    const startup = "echo startup-file-ran\nexport BRAIN_TEST_PROFILE_LEAK=1\n";
+    const res = await bashWithHome(
+      `printf "env=[%s] bash_env=[%s] " "\${ENV:-unset}" "\${BASH_ENV:-unset}"; bash -c 'printf "nested=[%s]" "\${BRAIN_TEST_PROFILE_LEAK:-unset}"'`,
+      { "startup.sh": startup },
+      { BRAIN_UI_SUBPROCESS_ENV_EXTRA: "BASH_ENV,ENV", BASH_ENV: "{HOME}/startup.sh", ENV: "{HOME}/startup.sh" },
+    );
+    expect(resultText(res)).toBe("env=[unset] bash_env=[unset] nested=[unset]");
+  });
+
   test("ask_user degrades to an error when the host lacks askUser", async () => {
     const brain = makeEmptyBrain();
     try {

packages/ui-backend-pi/tests/worker-host-exec.test.ts

@@ -63,7 +63,7 @@ for (const profile of ["governed", "strict", "restricted"] as const) {
       expect(report.grep).toContain("voyage.md:1:The wine-dark sea");
       expect(report.bash).toBe("Ithaca");
       const wrapped = report.wrapperLog.split("\n");
-      expect(wrapped, "WORKER_TOOLS_USED_HOST_WRAPPER").toEqual(expect.arrayContaining(["-rInE", "wine-dark", "-lc", "echo Ithaca", "sleep 30"]));
+      expect(wrapped, "WORKER_TOOLS_USED_HOST_WRAPPER").toEqual(expect.arrayContaining(["-rInE", "wine-dark", "--noprofile", "--norc", "-c", "echo Ithaca", "sleep 30"]));
       expect(report.cancel).toBe("AbortError");
       expect(report.killerLog, "WORKER_KILLER_CANCELLED_GROUP").toMatch(/^\d+ TERM\n$/);
       expect(report.cancelMs).toBeLessThan(10_000);