CLI-1631: handling silent failure of db failure

[kalindi-adhiya]

Motivation
This PR addresses silent failures in database import operations where pipeline commands could fail without being detected, potentially leading to incomplete or corrupted database imports.

Fixes #1631

Proposed changes

• Implemented bash -o pipefail -c "..." to wrap database import pipeline commands, ensuring that if any command in the pipeline (pv, gunzip, or mysql) fails, the entire pipeline fails properly
• Updated importDatabaseDump method in PullCommandBase.php for local database imports:
  • Uses bash -o pipefail -c with double-quoted command string
  • Environment variables ($LOCAL_DUMP_FILEPATH, $MYSQL_PASSWORD, $MYSQL_HOST, $MYSQL_USER, $MYSQL_DATABASE) are passed via the $env array parameter to executeFromCmd()
  • Shell references variables using standard bash syntax ($VAR) for proper substitution from environment
  • Works with both pv and gunzip variants
• Updated createMySqlDumpOnLocal method in CommandBase.php for local database exports:
  • Uses bash -c "set -o pipefail; ..." with double-quoted command string
  • Environment variables ($LOCAL_FILEPATH, $MYSQL_PASSWORD, $MYSQL_HOST, $MYSQL_USER, $MYSQL_DATABASE) passed via $env array
  • Shell references variables using standard bash syntax ($VAR) for proper substitution from environment
  • Ensures pipeline failures are detected during mysqldump operations
• Updated importDatabaseDumpOnRemote method in PushDatabaseCommand.php for remote database imports:
  • Uses bash -o pipefail -c with single-quoted command string to prevent variable expansion
  • Manual escaping (str_replace("'", "'\\''")) properly escapes all database credentials and file paths before string interpolation
  • Complies with codebase standards (no escapeshellarg())
  • Prevents shell injection vulnerabilities from special characters in passwords or other values
  • Added comprehensive test coverage with special characters in all credential fields (password, username, hostname, database name)
  • Added @infection-ignore-all annotation for filepath escaping (system-generated paths under our control)
• Updated corresponding test files to expect the new command format with bash -o pipefail and proper escaping

Impact: Error detection now works correctly!
Before this change, pipeline failures could be silent - if gunzip failed or mysql encountered an error, the command might still appear successful. Now with pipefail enabled, all pipeline failures are properly detected and reported, making database operations much more reliable.

Security improvements:

• Local commands use executeFromCmd() with environment variable substitution for safe value passing
• Credentials never appear in command strings visible in process listings or logs
• Remote commands manually escape single quotes using the standard bash technique ('\''' pattern) to prevent shell injection
• Properly handles quotes, spaces, and other shell metacharacters in all credential fields
• Complies with codebase standards by avoiding forbidden functions like escapeshellarg()

Technical details:

• Local commands (import/export): Use double-quoted bash strings with escaped inner quotes (\") and standard bash variable syntax ($VAR). Variables are passed via $env array parameter and accessed as environment variables inside bash subprocess.
• Remote commands (SSH import): Use single-quoted bash strings (prevents variable expansion) with manual escaping of credential values using str_replace("'", "'\\''") before string interpolation into command.
• Variable passing: All credentials passed through $env parameter to executeFromCmd() rather than embedded directly in command strings, providing an extra layer of security.

Why standard bash syntax ($VAR) instead of placeholder syntax (${:VAR})?
The ${:VAR} placeholder syntax is designed for direct command execution, but when wrapped inside bash -c "...", the shell tries to interpret it before the substitution can happen, causing "Bad substitution" errors. Using standard bash variable syntax ($VAR) allows the bash subprocess to correctly access variables from its environment.

Alternatives considered

• Using set -o pipefail && directly without bash wrapper - doesn't work in POSIX sh environments, which some systems default to
• Using escapeshellarg() - forbidden by codebase standards (phpcs rules)
• Using ${:VAR} placeholder syntax inside bash -c commands - causes "Bad substitution" errors because shell interprets before substitution
• Using separate commands instead of pipelines - would require temporary files and would be less efficient

Testing steps

1. Follow the contribution guide to set up your development environment or download a pre-built acli.phar for this PR.
2. If running from source, clear the kernel cache to pick up new and changed commands: ./bin/acli ckc
3. Check for regressions: Run existing database pull/push tests to ensure they pass with the new pipeline format

   composer test tests/phpunit/src/Commands/Pull/PullDatabaseCommandTest.php
   composer test tests/phpunit/src/Commands/Push/PushDatabaseCommandTest.php

4. Check new functionality: Verify that database import failures (e.g., corrupted backup file, mysql connection issues) are properly detected and reported rather than silently failing
5. Test with special characters: The new test testPushDatabaseWithSpecialCharsInPassword verifies that database credentials containing single quotes are handled correctly
6. Run mutation tests to verify comprehensive test coverage of escaping logic:

   composer infection -- --filter=src/Command/Push/PushDatabaseCommand.php

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.45%. Comparing base (77b699d) to head (db1ab2e).

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #2007   +/-   ##
=========================================
  Coverage     92.44%   92.45%           
  Complexity     1960     1960           
=========================================
  Files           123      123           
  Lines          7109     7114    +5     
=========================================
+ Hits           6572     6577    +5     
  Misses          537      537           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Try the dev build for this PR: https://acquia-cli.s3.amazonaws.com/build/pr/2007/acli.phar

curl -OL https://acquia-cli.s3.amazonaws.com/build/pr/2007/acli.phar
chmod +x acli.phar

[Copilot]

Pull request overview

This PR aims to prevent “silent success” when importing gzipped MySQL dumps by ensuring pipeline failures (e.g., pv/gunzip) propagate a non-zero exit code during pull/push database imports.

Changes:

• Adds pipefail to the local DB import pipeline used by pull commands.
• Adds pipefail to the remote DB import pipeline used by push:database.
• Updates PHPUnit expectations for the updated shell commands.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
src/Command/Pull/PullCommandBase.php	Updates the local import pipeline command to try to propagate failures from piped commands.
src/Command/Push/PushDatabaseCommand.php	Updates the remote import pipeline command to try to propagate failures from piped commands.
tests/phpunit/src/Commands/Pull/PullCommandTestBase.php	Updates test expectations for the local import command string.
tests/phpunit/src/Commands/Push/PushDatabaseCommandTest.php	Updates test expectations for the remote import command string.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kalindi-adhiya]

Testing Evidance , As exact scenario of mimicing this command failure can not be demonstrated , I have given the proof of regression testing that even with this happy path will not be tempered. Similar solution is also implemented by dane before for other file #1770

[deepakmishra2]

If $database->password (or any other interpolated value) contains a single quote, this will break the bash command or cause unexpected behaviour. The original had this same issue, but wrapping in bash -c '...' makes it more brittle.

[deepakmishra2]

or you can use escapeshellarg() to wrap each value in its own properly-escaped single quotes,

[deepakmishra2]

Also i see proposed changes in the PR differs from what is actually implemented, could you please update the description

[kalindi-adhiya]

Testing result after accommodating @deepakmishra2 suggestion
`kalindi_test_1805:~/project $ ACLI_CLOUD_API_BASE_URI=https://staging.cloud.acquia.com/api ACLI_CLOUD_API_ACCOUNTS_URI=https://staging.accounts.acquia.com/api/auth/oauth/token /home/ide/acli.phar pull:db -vvv

Box Requirements Checker

Using PHP 8.3.30
PHP is using the following php.ini file:
/usr/local/php8.3/etc/cli/php.ini

Checking Box requirements:
✔ This application requires a PHP version matching "^8.2".
✔ The package "zumba/amplitude-php" requires the extension "curl".
✔ The package "league/csv" requires the extension "filter".
✔ The package "symfony/polyfill-mbstring" requires the extension "iconv".
✔ This application requires the extension "json".
✔ The package "guzzlehttp/guzzle" requires the extension "json".
✔ The package "league/oauth2-client" requires the extension "json".
✔ The package "m4tthumphrey/php-gitlab-api" requires the extension "json".
✔ The package "zumba/amplitude-php" requires the extension "json".
✔ The package "composer/ca-bundle" requires the extension "openssl".
✔ The package "composer/ca-bundle" requires the extension "pcre".
✔ The package "vlucas/phpdotenv" requires the extension "pcre".
✔ The package "marc-mabe/php-enum" requires the extension "reflection".
✔ The package "m4tthumphrey/php-gitlab-api" requires the extension "xml".
✔ The package "laminas/laminas-servicemanager" conflicts with the extension "psr".
✔ The package "symfony/dependency-injection" conflicts with the extension "psr".
✔ The package "symfony/service-contracts" conflicts with the extension "psr".

[OK] Your system is ready to run the application.

Acquia CLI version: dev-2007/merge-fb51b124f7755b94b4c0ceef1d5530d2fc4ce0a6
Build date: 2026-06-15T12:01:22Z
PHP runtime: cli
Detected Codebase UUID: 1163d796-8f2b-4305-8c15-85802ec8ee48
Using codebase: devxpipe10nov_donotchangeanythinghereeventheenvironmentscodes

Choose a Cloud Platform environment [Stage, devxpipe10novstage (branch: 2024-10-25.2-multisite)]:
[0] Stage, devxpipe10novstage (branch: 2024-10-25.2-multisite)
[1] Dev29apr, devxpipe10novdev29apr (branch: feature-iflfi-codestudio-build)

1

[debug] Using environment Dev29apr d5aa0229-871d-4fcf-8251-e680f78ed0b9
Connecting to database drupal
[notice] Command: 'which' 'mysql' [Exit: 0]
[notice] Command: 'mysql' '--host' 'localhost' '--user' 'drupal' 'drupal' [Exit: 0]
[debug] Site instance with ID 674073c3-ac4f-4464-b258-c739a4029588.d5aa0229-871d-4fcf-8251-e680f78ed0b9 not found.Not Found
[debug] Site instance with ID 2d7d7a8e-036a-47df-a8b0-100b819976a6.d5aa0229-871d-4fcf-8251-e680f78ed0b9 not found.Not Found
Choose a site instance [apr9meotesting]:
[0] apr9meotesting
[1] meotest11may
[2] devxtest3apr

2

[INFO] Using a database backup that is 3 hours old. Backup #9516774 was created at Mon Jun 15 8:57:13 UTC 2026.

    You can view your backups here: https://cloud.acquia.com/a/environments/d5aa0229-871d-4fcf-8251-e680f78ed0b9/databases
                                                                                                                    
    To generate a new backup, re-run this command with the --on-demand option.                                      
                                                                                                                    

⌛ Downloading 9622b75bc3714d7483dd0b8b58fcfd1e database copy from the Cloud Platform...


⠛ Downloading 9622b75bc3714d7483dd0b8b58fcfd1e database copy from the Cloud Platform
⌛ Importing 9622b75bc3714d7483dd0b8b58fcfd1e database download...

! [NOTE] Cloud IDE only supports importing into the default Drupal database. Acquia CLI will import the NON-DEFAULT
! database 9622b75bc3714d7483dd0b8b58fcfd1e into the DEFAULT database drupal

Dropping tables from database drupal
[notice] Command: 'mysql' '--host' 'localhost' '--user' 'drupal' 'drupal' '--silent' '-e' 'SHOW TABLES;' [Exit: 0]
Importing downloaded file to database drupal
[debug] Importing /tmp/devxpipe10novdev29apr-9622b75bc3714d7483dd0b8b58fcfd1e-db9622b75bc3714d7483dd0b8b58fcfd1e-2026-06-15T08:57:13+00:00.sql.gz to MySQL on local machine
[notice] Command: 'which' 'gunzip' [Exit: 0]
[notice] Command: 'which' 'pv' [Exit: 0]
[notice] Command: bash -o pipefail -c "pv "$LOCAL_DUMP_FILEPATH" --bytes --rate | gunzip | MYSQL_PWD="$MYSQL_PASSWORD" mysql --host="$MYSQL_HOST" --user="$MYSQL_USER" "$MYSQL_DAT

⠛ Importing 9622b75bc3714d7483dd0b8b58fcfd1e database download

[notice] Command: 'which' 'drush' [Exit: 0]
[notice] Command: 'drush' 'status' '--fields=db-status,drush-version' '--format=json' '--no-interaction' [Exit: 0]
[notice] Drush does not have an active database connection. Skipping cache:rebuild
[notice] Drush does not have an active database connection. Skipping sql:sanitize.`