fix: keep a master password off the command line - #906
Merged
blaipr merged 1 commit intoSep 3, 2026
Conversation
sp:updateMasterPassword takes --masterPassword and --currentMasterPassword; sp:install takes --adminPassword, --masterPassword and --databasePassword. A value passed that way 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 — minutes, for a rotation that re-encrypts every account. This is a different threat from the one the CLI is exempt from elsewhere: sp:backup needs no demo guard because whoever runs 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. Both commands already take the same values from an environment variable and fall back to a hidden prompt. CommandBase gains two helpers, called once at the start of each command so the value is gone before the work begins: cli_set_process_title() rewrites the argv memory, and the operator is warned. The title is rebuilt from the real command line with only the named options masked, rather than replaced wholesale, so a process still says what it is — and the test runner does not lose its own name when the CLI tests exercise these commands. It shrinks the window rather than closing it: the shell forks with the full argv before PHP runs, and nothing here touches shell history. The options stay; removing them would break documented automation for no gain the environment variable does not already offer.
blaipr
deleted the
fix/a-master-password-is-not-passed-on-the-command-line
branch
September 3, 2026 02:05
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.
sp:updateMasterPasswordtakes--masterPasswordand--currentMasterPassword;sp:installtakes--adminPassword,--masterPasswordand--databasePassword. A value passed that way is inargv, andargvis/proc/<pid>/cmdline, which every other local user on the host can read foras long as the command runs.
This is a different threat from the one the CLI is otherwise exempt from, and the distinction is
worth being precise about.
sp:backupdeliberately has no demo guard because whoever can run italready has
config/config.xml— the database credentials and the crypto keys — so a guard thereprotects nothing. 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:updateMasterPasswordre-encrypts every account,history row and custom field, so the window is minutes, not an instant.
Both commands already accept the same values through an environment variable, and both fall back to
a hidden prompt. Those are the two ways to hand a command a secret that other local users cannot
read, and neither is any harder to use.
The change
CommandBasegains two helpers, called once at the start of each command:psshows.cli_set_process_title()rewrites that memory, soit is gone for the rest of the run. The title is rebuilt from the real command line with only the
named options replaced by
***, rather than replaced wholesale, so a process being looked atstill says what it is — and so the test runner does not lose its own name when the CLI tests
exercise these commands.
Called at the start rather than beside each password so the secret is gone before the work begins.
What it does not do
It shrinks the window to the moment before the command starts; it cannot close it — the shell forks
with the full
argvbefore PHP runs — and it does nothing about shell history or a script with thepassword written into it. That is what the warning is for. The options are kept: they are
documented, and removing them would break existing automation for no gain the environment variable
does not already offer.
Tests
Asserted against
/proc/self/cmdlinedirectly, because the whole point is what another process cansee, and because the platform could stop honouring
cli_set_process_title()without anything elsenoticing. Both spellings the console accepts —
--option=valueand--option value— are covered,along with a non-secret option being left alone, and a negative control showing the secret is
plainly visible without the call.