fix: keep a master password off the command line

CLAUDE.md

@@ -236,6 +236,16 @@ A few harness details bite when writing an integration test against a real branc
   rather than a bug. `anAccountStampedNow()` compares everything else exactly and takes only
   `passDate` from the actual, after checking it is a timestamp from the last few seconds. Injecting
   `sleep(1)` before the write is how to reproduce it, and how to show a fix works.
+- **An interrupted CLI install test poisons every later run, with an error that looks like a code
+  defect.** `DatabaseUtil::createUser()` grants with `GRANT … IDENTIFIED BY`, and MariaDB 11.8
+  refuses that for an account that **already exists** — *"Can't find any matching row in the user
+  table"* (SQLSTATE 28000, 1133). It creates a new one happily, so the helper works exactly once
+  per user: a run that dies before its teardown leaves `syspass@<ip>`, `syspass@<hostname>` and
+  `sp_*` users behind, and `InstallCommandTest` then fails for everyone afterwards. Confirm it is
+  not yours by running the same test on a stashed tree, then
+  `SELECT user, host FROM mysql.user` and drop the leftovers — everything but `syspass@%`,
+  `root@%`, `root@localhost`, `healthcheck@*` and `mariadb.sys@localhost` is a leftover.
+
 - **Faker's `randomNumber($n)` includes zero**, and forms read a zero id as "not given". A fixture
   drawing a group or profile id that way fails about one run in a hundred, on CI, in whichever pull
   request happened to be open. Use `numberBetween(1, …)`.

src/Infrastructure/Adapter/In/Cli/Commands/CommandBase.php

@@ -30,7 +30,9 @@
 use SP\Application\Config\Services\ConfigFile;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Style\StyleInterface;
 
+use function SP\__;
 use function SP\getFromEnv;
 
 /**
@@ -40,6 +42,8 @@
  */
 abstract class CommandBase extends Command
 {
+    private const REDACTED = '***';
+
     /**
      * @var string[]
      */
@@ -70,6 +74,104 @@ protected static function getEnvVarOrOption(
             ?: $input->getOption($option);
     }
 
+    /**
+     * Tell the operator that a secret they put on the command line was visible, and take it out of
+     * what `ps` shows from here on.
+     *
+     * Called once at the start of a command rather than beside each password, so the value is gone
+     * before the work starts — a master-password rotation re-encrypts every account, and the whole
+     * of that is time another local user could be reading `/proc/<pid>/cmdline`.
+     *
+     * Nothing is said when the value came from the environment or from the prompt: those are the
+     * two ways to give a command a secret that other users on the host cannot read, and they are
+     * what the warning points at.
+     */
+    protected static function warnAboutSecretsOnTheCommandLine(
+        InputInterface $input,
+        StyleInterface $style,
+        string         ...$options
+    ): void {
+        $given = array_values(
+            array_filter($options, static fn(string $option): bool => !empty($input->getOption($option)))
+        );
+
+        if ($given === []) {
+            return;
+        }
+
+        self::hideSecretsFromTheProcessTitle(...$options);
+
+        $style->warning(
+            sprintf(
+                __('A password given as %s is visible to every local user while the command runs. Use the environment variable or let the command ask for it.'),
+                implode(', ', array_map(static fn(string $option): string => '--' . $option, $given))
+            )
+        );
+    }
+
+    /**
+     * Take the named options' values out of what `ps` shows for this process.
+     *
+     * A value passed as `--masterPassword=…` is in `argv`, and `argv` is `/proc/<pid>/cmdline`,
+     * which every local user on the host can read for as long as the command runs — minutes, for a
+     * rotation that re-encrypts every account. That is a different threat from the one the CLI is
+     * otherwise exempt from: `sp:backup` needs no demo guard because whoever can run it already has
+     * `config/config.xml`, but this is about the *other* users on a shared host, who have neither
+     * that file nor any way to read the environment of a process they do not own.
+     *
+     * `cli_set_process_title()` rewrites that memory, so the value is gone from `ps` for the rest
+     * of the run. The title is rebuilt from the real command line with only the secrets replaced,
+     * rather than replaced wholesale, so the process still says what it is.
+     *
+     * This shrinks the window to the moment before the command starts; it cannot close it, and it
+     * does nothing about shell history or a script with the password written into it. The warning
+     * beside each call is the other half.
+     */
+    protected static function hideSecretsFromTheProcessTitle(string ...$options): void
+    {
+        if (!function_exists('cli_set_process_title')) {
+            return;
+        }
+
+        /** @var string[] $argv */
+        $argv = $_SERVER['argv'] ?? [];
+
+        if ($argv === []) {
+            return;
+        }
+
+        $masked = [];
+        $maskNext = false;
+
+        foreach ($argv as $argument) {
+            if ($maskNext) {
+                $masked[] = self::REDACTED;
+                $maskNext = false;
+
+                continue;
+            }
+
+            foreach ($options as $option) {
+                // Both spellings Symfony accepts: --option=value and --option value.
+                if (str_starts_with($argument, sprintf('--%s=', $option))) {
+                    $argument = sprintf('--%s=%s', $option, self::REDACTED);
+
+                    break;
+                }
+
+                if ($argument === sprintf('--%s', $option)) {
+                    $maskNext = true;
+
+                    break;
+                }
+            }
+
+            $masked[] = $argument;
+        }
+
+        @cli_set_process_title(implode(' ', $masked));
+    }
+
     /**
      * @return string|false
      */

src/Infrastructure/Adapter/In/Cli/Commands/Crypt/UpdateMasterPasswordCommand.php

@@ -117,6 +117,8 @@ protected function execute(
     ): int {
         $style = new SymfonyStyle($input, $output);
 
+        self::warnAboutSecretsOnTheCommandLine($input, $style, 'masterPassword', 'currentMasterPassword');
+
         if (!$this->lock()) {
             $style->warning(__('The command is already running in another process'));
 

src/Infrastructure/Adapter/In/Cli/Commands/InstallCommand.php

@@ -162,6 +162,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
     {
         $style = new SymfonyStyle($input, $output);
 
+        self::warnAboutSecretsOnTheCommandLine($input, $style, 'adminPassword', 'masterPassword', 'databasePassword');
+
         try {
             // Throws when sysPass is already installed and --forceInstall was not given;
             // a fresh system installs without the flag

tests/Unit/Infrastructure/Adapter/In/Cli/Commands/SecretsAreNotLeftOnTheCommandLineTest.php

@@ -0,0 +1,142 @@
+<?php
+
+declare(strict_types=1);
+/*
+ * sysPass
+ *
+ * @author nuxsmin
+ * @link https://syspass.org
+ * @copyright 2012-2024, Rubén Domínguez nuxsmin@$syspass.org
+ *
+ * This file is part of sysPass.
+ *
+ * sysPass is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * sysPass is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with sysPass.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+namespace SP\Tests\Unit\Infrastructure\Adapter\In\Cli\Commands;
+
+use PHPUnit\Framework\Attributes\Group;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\TestCase;
+use ReflectionMethod;
+use SP\Infrastructure\Adapter\In\Cli\Commands\CommandBase;
+
+/**
+ * A password given to a command as an option is in `argv`, and `argv` is `/proc/<pid>/cmdline`,
+ * which every other local user on the host can read for as long as the command runs.
+ *
+ * That is a different threat from the one the CLI is otherwise exempt from. `sp:backup` needs no
+ * demo guard because whoever can run it already has `config/config.xml` — but the users this is
+ * about have neither that file nor any way to read the environment of a process they do not own,
+ * and `sp:updateMasterPassword` re-encrypts every account, so the window is minutes rather than an
+ * instant.
+ *
+ * `cli_set_process_title()` rewrites that memory. This asserts it against `/proc` directly, because
+ * the whole point is what another process can see, and because the platform could stop honouring it
+ * without anything else noticing.
+ */
+#[Group('unitary')]
+class SecretsAreNotLeftOnTheCommandLineTest extends TestCase
+{
+    private const SECRET = 'correct-horse-battery-staple';
+    private const OTHER_SECRET = 'a-second-secret-value';
+
+    /** @var string[] */
+    private array $argv = [];
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        if (!function_exists('cli_set_process_title') || !is_readable('/proc/self/cmdline')) {
+            self::markTestSkipped('needs a Linux CLI SAPI with a readable /proc');
+        }
+
+        $this->argv = $_SERVER['argv'] ?? [];
+    }
+
+    protected function tearDown(): void
+    {
+        $_SERVER['argv'] = $this->argv;
+
+        // Put the runner's own title back, so a later `ps` still finds it by name.
+        @cli_set_process_title(implode(' ', $this->argv));
+
+        parent::tearDown();
+    }
+
+    /**
+     * Both spellings the console accepts — `--option=value` and `--option value` — are taken out,
+     * and the command still says what it is.
+     */
+    #[Test]
+    public function aPasswordGivenAsAnOptionIsTakenOutOfTheProcessTitle(): void
+    {
+        $_SERVER['argv'] = [
+            'bin/cli.php',
+            'sp:updateMasterPassword',
+            '--masterPassword=' . self::SECRET,
+            '--currentMasterPassword',
+            self::OTHER_SECRET,
+            '--update',
+        ];
+
+        self::hide('masterPassword', 'currentMasterPassword');
+
+        $cmdline = self::cmdline();
+
+        self::assertStringNotContainsString(self::SECRET, $cmdline);
+        self::assertStringNotContainsString(self::OTHER_SECRET, $cmdline);
+        self::assertStringContainsString('sp:updateMasterPassword', $cmdline);
+        self::assertStringContainsString('--update', $cmdline);
+    }
+
+    /**
+     * An option that carries no secret is left alone — the title is rebuilt from the real command
+     * line rather than replaced, so a process being looked at is still identifiable.
+     */
+    #[Test]
+    public function everythingElseIsLeftAsItWas(): void
+    {
+        $_SERVER['argv'] = ['bin/cli.php', 'sp:backup', '--path=/var/backups/nightly'];
+
+        self::hide('masterPassword');
+
+        self::assertStringContainsString('--path=/var/backups/nightly', self::cmdline());
+    }
+
+    /**
+     * And the check that shows the assertion above is worth something: without the call, the secret
+     * is exactly where it was.
+     */
+    #[Test]
+    public function withoutTheCallTheSecretIsVisible(): void
+    {
+        @cli_set_process_title('bin/cli.php sp:updateMasterPassword --masterPassword=' . self::SECRET);
+
+        self::assertStringContainsString(self::SECRET, self::cmdline());
+    }
+
+    private static function hide(string ...$options): void
+    {
+        (new ReflectionMethod(CommandBase::class, 'hideSecretsFromTheProcessTitle'))->invoke(null, ...$options);
+    }
+
+    private static function cmdline(): string
+    {
+        clearstatcache();
+
+        return str_replace("\0", ' ', (string)file_get_contents('/proc/self/cmdline'));
+    }
+}