Skip to content

Prefer /bin/bash and validate SHELL points to bash before using it#81

Open
Copilot wants to merge 5 commits intomainfrom
copilot/fix-shell-binary-detection
Open

Prefer /bin/bash and validate SHELL points to bash before using it#81
Copilot wants to merge 5 commits intomainfrom
copilot/fix-shell-binary-detection

Conversation

Copy link
Contributor

Copilot AI commented Feb 1, 2026 •
edited
Loading

On systems like NixOS, bash is not located at /bin/bash. The wp shell command was hardcoded to this path, causing failures despite the shell being available at $SHELL.

However, directly using $SHELL can break wp shell for users whose login shell is not bash (e.g., zsh, fish), since the command uses bash-specific features like history -r/-s/-w and read -re.

Changes

  • Shell detection priority in REPL.php:

    1. WP_CLI_CUSTOM_SHELL (explicit override)
    2. /bin/bash (preferred when available) ← ensures bash-specific commands work
    3. SHELL (only if it points to bash and /bin/bash doesn't exist) ← new, fixes NixOS
    4. bash (final fallback)

  • Added is_bash_shell() helper: Validates that a shell binary is bash by checking if the basename is exactly bash or starts with bash- (e.g., bash-5.0), preventing false positives.

  • Test coverage: Added scenario verifying that SHELL pointing to non-bash shells (e.g., /bin/sh) is properly ignored and falls back to /bin/bash.

// Before
if ( getenv( 'WP_CLI_CUSTOM_SHELL' ) ) {
    $shell_binary = getenv( 'WP_CLI_CUSTOM_SHELL' );
} else {
    $shell_binary = '/bin/bash';
}

// After
if ( getenv( 'WP_CLI_CUSTOM_SHELL' ) ) {
    $shell_binary = getenv( 'WP_CLI_CUSTOM_SHELL' );
} elseif ( is_file( '/bin/bash' ) && is_readable( '/bin/bash' ) ) {
    // Prefer /bin/bash when available since we use bash-specific commands.
    $shell_binary = '/bin/bash';
} elseif ( getenv( 'SHELL' ) && self::is_bash_shell( getenv( 'SHELL' ) ) ) {
    // Only use SHELL as fallback if it's bash (we use bash-specific commands).
    $shell_binary = getenv( 'SHELL' );
} else {
    // Final fallback for systems without /bin/bash.
    $shell_binary = 'bash';
}

This approach fixes the NixOS issue while preventing breakage for users with non-bash login shells.

Original prompt

This section details on the original issue you should resolve

<issue_title>Shell binary not getting detected correctly?</issue_title>
<issue_description>## Bug Report

The shell binary '/bin/bash' is not valid.

The wp --info command shows the correct Shell, but wp shell does not seem to find it. Since it's not in /bin/bash

[sergio@samara:/var/www/nerdpress]$ wp --info
OS:     Linux 6.6.32 wp-cli/shell-command#1-NixOS SMP PREEMPT_DYNAMIC Sat May 25 14:22:56 UTC 2024 x86_64
Shell:  /run/current-system/sw/bin/bash
PHP binary:     /nix/store/swkmyv7mplz46vlr9jyk7qlp2lxv19z0-php-with-extensions-8.2.19/bin/php
PHP version:    8.2.19
php.ini used:   /nix/store/6g69kphwfkdx4p2qn1fgwm888l15cpz7-wp-cli-2.10.0/etc/php.ini
MySQL binary:   /run/current-system/sw/bin/mysql
MySQL version:  mysql  Ver 15.1 Distrib 10.11.6-MariaDB, for Linux (x86_64) using readline 5.1
SQL modes:      STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION
WP-CLI root dir:        phar://wp-cli.phar/vendor/wp-cli/wp-cli
WP-CLI vendor dir:      phar://wp-cli.phar/vendor
WP_CLI phar path:       /var/www/nerdpress
WP-CLI packages dir:
WP-CLI cache dir:       $XDG_CACHE_HOME/wp-cli
WP-CLI global config:
WP-CLI project config:
WP-CLI version: 2.10.0

[sergio@samara:/var/www/nerdpress]$ wp shell
Error: The shell binary '/bin/bash' is not valid. You can override the shell to be used through the WP_CLI_CUSTOM_SHELL environment variable.

[sergio@samara:/var/www/nerdpress]$ php -a
Interactive shell

php > var_dump( is_file( '/run/current-system/sw/bin/bash' ) );
bool(true)
php > var_dump( is_readable( '/run/current-system/sw/bin/bash' ) );
bool(true)
php > exit

Describe how other contributors can replicate this bug

  • Have get( 'SHELL' ); return something other than /bin/bash
  • Run wp shell

Provide a possible solution

Seems that the wp cli info command gets the shell binary using:
getenv( 'SHELL' )

Where wp shell uses getenv( 'WP_CLI_CUSTOM_SHELL' ) to check for a "custom shell", then assumes /bin/bash is it isn't set.

Why not use $shell_binary = getenv( 'SHELL' ); instead of $shell_binary = '/bin/bash';

</issue_description>

Comments on the Issue (you are @copilot in this section)

@swissspidy `WP_CLI_CUSTOM_SHELL` is an environment variable accessed via `getenv`. It is not a PHP constant. So you would do this instead: 
WP_CLI_CUSTOM_SHELL=/run/current-system/sw/bin/bash wp shell
```</body></comment_new>
<comment_new><author>@swissspidy</author><body>
Good question. Maybe @schlessera or @danielbachhuber have some context about why the current default is a hardcoded `/bin/bash` and not `getenv( 'SHELL' )`. It sounds like a reasonable enhancement at first glance.</body></comment_new>
<comment_new><author>@danielbachhuber</author><body>
I don't recall a specific reason off the top of my head. For a more definitive answer, I'd look at the original pull request(s) to see if there was a discussion around it.</body></comment_new>
</comments>

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@swissspidy
Use SHELL environment variable as fallback before /bin/bash
2071c56 
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix shell binary detection for wp shell command Use SHELL environment variable before falling back to /bin/bash Feb 1, 2026
Copilot AI requested a review from swissspidy February 1, 2026 19:35
@codecov
Copy link

codecov bot commented Feb 1, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 11.11111% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/WP_CLI/Shell/REPL.php 11.11% 8 Missing ⚠️

📢 Thoughts on this report? Let us know!

@swissspidy swissspidy added this to the 2.0.17 milestone Feb 2, 2026
@swissspidy swissspidy requested a review from Copilot February 2, 2026 02:01
@swissspidy

This comment was marked as resolved.

Sign in to view
gemini-code-assist[bot]

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

@swissspidy
Update features/shell.feature
325dbde 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI and others added 2 commits February 4, 2026 02:01
@swissspidy
Prefer /bin/bash and only use SHELL for bash shells
d424ea8 
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
@swissspidy
Improve bash detection and test reliability
8eae8ef 
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Copilot AI changed the title Use SHELL environment variable before falling back to /bin/bash Prefer /bin/bash and validate SHELL points to bash before using it Feb 4, 2026
@swissspidy swissspidy marked this pull request as ready for review February 4, 2026 03:17
@swissspidy swissspidy requested a review from a team as a code owner February 4, 2026 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@swissspidy swissspidy Awaiting requested review from swissspidy

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@swissspidy swissspidy

Copilot code review Copilot

Labels

command:shell scope:distribution state:unconfirmed

Projects

None yet

Milestone

2.0.17

Development

Successfully merging this pull request may close these issues.

Shell binary not getting detected correctly?

2 participants

@swissspidy