Repository navigation
fix(ui-backend-pi): run the bash tool without a login shell - #2985
Merged
Merged
Conversation
The bash tool spawned `bash -lc`, so /etc/profile and the first of ~/.bash_profile, ~/.bash_login or ~/.profile under the child's HOME ran before every command. Their output reached the model-visible result and anything they exported, PATH included, overrode the allowlisted subprocess environment (a profile `export GH_TOKEN=...` reached the command although the allowlist removed the host value). Run `bash --noprofile --norc -c` instead and remove BASH_ENV and ENV from the command's environment even when an operator admits them, so neither the command nor a shell it starts sources a startup file. The command now sees exactly the filtered environment. Tests run the real tool with a fixture HOME whose profiles print, export GH_TOKEN/BRAIN_TEST_PROFILE_LEAK and prepend a planted PATH entry, and with an admitted BASH_ENV/ENV. Co-Authored-By: Claude Opus 5.5 <[email protected]>
schlessera
marked this pull request as ready for review
October 26, 2026 06:26
This was referenced Oct 26, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2960
What changed
The pi backend's
bashtool no longer runs commands in a login shell. Before this change it spawnedbash -lc, so/etc/profileand the first of~/.bash_profile,~/.bash_loginor~/.profileunder the child'sHOMEran before every command. Their output landed in the model-visible result, and every variable orPATHentry they exported overrode the allowlistedsubprocessEnv. Commands now run asbash --noprofile --norc -c <cmd>(packages/ui-backend-pi/src/tools.ts, at the spawn site, with a comment explaining why).BASH_ENVandENVare removed from the command's environment even when an operator admits them, so neither the command nor any shell it starts sources a startup file. The command sees exactly the filtered environment. The tool's result shape (textplus{ exitCode }) is unchanged.Why this shape
--noprofile --norc -crather than a bare-c. Non-interactivebash -calready skips profiles and.bashrc, with one exception: Debian/Ubuntu builds source~/.bashrcwhen they detect an ssh parent. The flags make "reads no startup file" explicit and independent of how bash was built. They are belt and braces, so no test goes red if only they are removed (receipts below).BASH_ENV/ENVscrubbing: a non-interactive bash sources$BASH_ENVin this command and in every nestedbashit starts.$ENVis the sh/posix equivalent. Neither name is on the agent allowlist, so they could only arrive throughBRAIN_UI_SUBPROCESS_ENV_EXTRA. They name startup files, not values a command uses. The README and changeset say so.HOMEon the four existing "bash returns output without a command echo" cases. Those cases now pass whatever the developer's profile does, which is the acceptance criterion; the new tests carry the fixtureHOME.Open questions from the issue, answered
HOMEthat outlives the turn? No. Every production pi turn goes through the worker launcher (session-runtime.ts; only the internalsessionFactorytest double runs tools in-process). Every non-envelope worker binds the host root read-only (--ro-bind / /,worker-launcher.ts). A probe of that mount shape answeredtouch: cannot touch '$HOME/…': Read-only file system. Profile (governed/strict) workers also mask/homeand/rootwith empty tmpfs and never bind the home back. With the restricted envelope,HOMEisWORKER_SCRATCH, which is writable. It is that worker's private turn-temp tmpfs and is gone when the per-turn worker is killed, so a planted file could only have affected later commands in the same turn. That is now moot, because nothing is sourced. Residual edge: before this change, a governed worker whose hostHOMEsat inside the writable brain could have persisted one. Removing-lcloses that edge too.-lload-bearing? No evidence that it is. It dates from the initial public release (ee888dec3) with no rationale. The only doc mention was the argv shape inpackages/ui-backend-pi/README.md, updated here. No doc, decision record or hosting-template file relies on a profile forPATH: the hosting template's container-privilege decision synthesizes a fixedPATH, and the restricted envelope setsPATH=/usr/bin:/bin. With-l, distro/etc/profilecould replace the allowlistedPATH. Ubuntu's appends/snap/bin(observed here); Debian's resets it outright (distro default, not checked here). A changeset covers hosts that relied on a profile forPATH.<shell> -c -lsourcing~/.bashrc. It then runs each command as<shell> -c "source <snapshot> && … eval <cmd>"and drops-lonly when a snapshot exists. Profile output and exported variables do not reach commands, and the snapshot'sPATHis the CLI's own. Functions and aliases from the host rc do reach commands. Without a usable snapshot, the CLI falls back to-c -lper command, which is the exact pi shape.Proof
Base:
origin/main@c670449d3. Head: see the PR head SHA.Failing-first. New tests (
packages/ui-backend-pi/tests/tools-permission.test.ts) run against the unchanged tree:bash output carries nothing a profile in HOME prints: echo hi: received"profile-stdout\nhi\n\nprofile-stderr"instead of"hi".… : true: received"profile-stdout\n\nprofile-stderr"instead of"(no output)".a profile in HOME sets no variable and no PATH entry the command sees: receivedtok=[from-profile] leak=[1] probe=[/tmp/pi-bash-home-…/planted-bin/brain-profile-probe] path=[planted]instead oftok=[unset] leak=[unset] probe=[absent] path=[clean]. The host hadGH_TOKEN=host-value, which the allowlist removed; the profile put it back.an operator-admitted BASH_ENV or ENV names no startup file …: receivedstartup-file-ran … nested=[1]instead ofenv=[unset] bash_env=[unset] nested=[unset].Mutations, each restored afterwards:
["bash", "-lc", cmd]back: the two noisy-profile cases and the leak case fail on the same assertions as above; the BASH_ENV case passes (3 fail / 1 pass).BASH_ENV/ENVscrub: only the BASH_ENV case fails (startup-file-ran … nested=[1]) (1 fail / 3 pass).--noprofile --norc(bare-c): stays green, as expected for belt-and-braces flags (see above).The four existing cases (
bash returns output without a command echo): with the hostHOME, 4 pass. With a noisy developerHOME(.bash_profile/.profilesourcing a missing file), 4 pass on this branch. With-lrestored, the same 4 fail with…/.bash_profile: line 2: /nonexistent/env-file: No such file or directory, which is the issue's report reproduced.Suites on head
9dddf96(basec670449d3; each heavy run through the host slot wrapper):bun run test packages/ui-backend-pibun run test packages/ui-backend-claudebun run test packages/ui-sdkbun run test packages/ui-serverbun run test ./tests(fail)lines in its log are the runner harness's deliberately failingpenelope/odysseusfixture children)bun run check:pr --base origin/mainbun scripts/prove-native-host.ts --backend=pi(native lock, scratch worktree)No base comparison was needed: nothing failed.
origin/mainhas since gained #2916 and #2949, which touch ui-server and docs only. The PR still merges cleanly.Checklist
.changeset/pi-bash-no-login-shell.md, patch).--json, MCP or schema change; the tool result shape is unchanged).packages/ui-backend-pi/README.md, exec-wrapper section).Anything left open
HOME's rc and login profile (functions and aliases reach commands; there is a-c -lfallback). Out of scope here per ui-backend-pi: the bash tool runs a login shell, so host profile output reaches the model and may override the allowlisted environment #2960.