Repository navigation
feat(host-tools): shell variables above the bashrc guard, and home-relative ones - #187
Merged
Merged
Conversation
…uard A non-interactive bash, which is what sshd starts for an SSH command, reads ~/.bashrc only as far as the interactive guard that Debian and Ubuntu put near the top of the default file. The managed blocks for github_runner_host_tools_shell_env were appended at the end, below the guard, so the variables were never set for an SSH command on those hosts. A variable is now inserted before the file's first line, and a block an earlier run appended lower down is taken out and written again at the top, so existing hosts converge without anyone touching them. Whether a block leads the file is decided from the file's own order (only blanks, comments and the role's other blocks above it), not from a pattern for the guard, so any way of writing the guard is stepped over. The Homebrew and mise blocks stay at the end, where the interactive setup is. The tests drive the role against a copy of Ubuntu's /etc/skel/.bashrc and read the result with a real bash.
… home directory The play runs with gather_facts false, so a fleet cannot say "the user's home plus .cache/turbo", and github_runner_host_tools_shell_env quotes a literal $HOME so that it never expands. github_runner_host_tools_shell_env_home_relative maps a variable name to a path relative to the user's home directory and writes it as export NAME="$HOME/<path>", so the shell works the home directory out when it reads the startup file. The characters special inside double quotes (a backslash, a dollar sign, a backtick and a double quote) are escaped, so the path reaches the variable as given in sh, bash and zsh. An absolute or empty path, a path with a .. component and a name set in both mappings fail the run before any file is changed. The block is placed, skipped when a startup file already assigns the variable, and checked exactly as for github_runner_host_tools_shell_env.
Mearman
changed the title
…es back The host tools tests run the lines the role writes through a real sh, bash and zsh, and the runner image has no zsh.
Mearman
marked this pull request as ready for review
November 3, 2026 10:00
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
|
🎉 This PR is included in version 2.23.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
This was referenced Nov 3, 2026
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.
github_runner_host_tools_shell_envblocks were appended to the end of~/.bashrc. On Ubuntu that is below the default interactive guard (case $- in *i*) ;; *) return;; esac), and a non-interactive bash, which is what sshd starts forssh host cmd, stops reading the file there.Seen live on the fleet's two Ubuntu 26.04 hosts: the block is at line 131 of
~/.bashrcon one (guard at line 9) and line 124 on the other (guard at line 6), andssh <host> 'echo ${UV_PYTHON_DOWNLOADS-unset}'printsunseton both. On the Macs zsh reads~/.zshenvfor every shell, so it works there.The managed env blocks are now inserted before the first line of the file. A block an earlier run appended lower down is removed and written again at the top (so existing hosts converge on the next run, no duplicate). Whether a block already leads the file is decided from the file's order (only blanks, comments and the role's own blocks above it), so any style of guard is stepped over. The Homebrew and mise blocks stay at the end. zsh follows the same rule, and a
.zshenvlaid out by the previous version (env block after the Homebrew and mise blocks) is already in order and is not touched.Debian's bash reads
~/.bashrcfor a shell whose stdin is a network connection (bash(1), archived); upstream bash does so only if built withSSH_SOURCE_BASHRC.Tests run the role against a byte-for-byte copy of Ubuntu's
/etc/skel/.bashrcand read the result with a real non-interactivebash. Run against the previous tasks, the new bash cases fail (unset != never).The second commit adds
github_runner_host_tools_shell_env_home_relative, a mapping of variable names to paths relative to the user's home, written asexport NAME="$HOME/<path>". The play runs withgather_facts: false, so a fleet cannot name the home directory, andgithub_runner_host_tools_shell_envquotes$HOMEso it stays literal. The path is escaped for double quotes (backslash,$, backtick,") and read back through realsh,bashandzshin the tests. Absolute, empty and..paths, and a name set in both mappings, fail before any file is changed. Placement, the never-override rule and--checkare the same as for the other mapping.