fix(linux): resolve MySQL root password removal and prompt for current password when set

[plusemon]

Summary

• Password Prompt: When a MySQL root password is set, the root password dialog in window.py now prompts for the current password as well as the new password (leave blank to remove).
• Authentication: mysql_run in engine/bhserve and platform-linux.sh authenticates using BHSERVE_OLD_DB_PASSWORD / MYSQL_PWD or saved credentials.
• Privilege Fallback: Falls back to pkexec/sudo elevation on Linux if unprivileged connection fails.
• MySQL 8.4 Support: Updated ALTER USER 'root'@'localhost' IDENTIFIED BY '' syntax for MySQL 8.0/8.4 compatibility.

[wpexpertinbd]

Appreciate you tackling the root-password flow — and thanks for the clean commit history; the superset structure over #5 made this easy to review. Supply-chain pass is clean (no new network calls, downloads, eval/base64, or dependencies — the only new exec surface is pkexec/sudo running the locally installed mysql client, which is legitimate here), and your mysql_esc handling is applied correctly to all three SQL variants, so there's no injection path through the password. The Python side also correctly passes secrets via subprocess env= rather than argv, and correctly keeps db root-passwd out of _PRIVILEGED so the env survives — you clearly read the existing design.

All three blockers from my review on #5 apply here too (Swift syntax → macOS won't compile; the name NameError killing "Add app"; Laravel's public/ never created). Four more on the DB work:

1. Please don't persist the root password. db_root_passwd calls json_set "root_password" "$DB_PASSWORD" on all five success paths, which writes it in plaintext to ~/.bhserve/config/bhserve.json. There's no chmod/umask anywhere in the engine, so that file is 0644 in a 0755 dir — world-readable on a shared machine. Worse, cmd_api splices the file verbatim into the GUI snapshot, which the app polls every 4 seconds, and status / config show print it too. That also reverses two explicit decisions already commented in the code (engine/bhserve: "env keeps it out of ps", and engine.py: "owner-only /proc//environ instead of argv").

Your new "Current password" prompt makes prompting the better answer. If it really must be stored, it needs: a dedicated file created under umask 077 with the mode re-asserted on every rewrite (cat > preserves the old mode), chmod 600 in _bh_fix_ownership, redaction in cmd_api/cmd_status, and root_password removed from config_set.

2. Keep the SQL out of argv. pkexec "$cli" -e "$sql" / $SUDO "$cli" -e "$sql" put ALTER USER ... IDENTIFIED BY 'pw' into the process table — the mysql client scrubs -p<pw> but not -e, and both pkexec and sudo record the full command vector in the audit log permanently. stdin survives pkexec, so this works and fixes it:

printf '%s\n' "$sql" | ${SUDO:-sudo} "$cli"

That ${SUDO:-sudo} is needed anyway: $SUDO is only assigned in platform-linux.sh, but you use it in the shared engine/bhserve, which runs set -u — on macOS (no pkexec) that branch always runs and aborts with SUDO: unbound variable before your legacy-SQL fallbacks.

3. nginx_reload → nginx_restart is the one I'd most like reverted. You're right that the old [ -t 1 ] tty gate meant GUI-initiated changes were never applied — please keep that fix. But -s reload is fail-safe: it keeps serving the old config if a vhost is bad. nginx_restart stops first and then dies in nginx -t, and because the call is wrapped in >/dev/null 2>&1 the message is swallowed and the || -s reload fallback is unreachable — so one bad vhost takes every site down, silently. We shipped an emergency release for exactly that failure mode a few days ago (c8765f3). Suggested shape:

nginx -s reload ... || { nginx_stop; nginx_start; }   # reload primary, restart as fallback

(and the mirror in Nginx.Reload() on Windows, which currently discards Start()'s (bool, string) result).

4. The stored password can't round-trip. jget's capture is [^",}]*, so any password containing ,, " or } reads back truncated (abc,def → abc), and the unescaped JSON write can produce an invalid config — which, because cmd_api inlines the file, blanks the entire dashboard while still printing root password set. json_esc() already exists in the same file and is used ~15×. This mostly disappears if you drop the persistence per (1).

One request: would you be open to splitting this? The folder pickers, the Laravel type, the nginx reload change, and the DB root-password work are four independent features — smaller PRs would let me merge the good parts fast instead of holding them behind the DB discussion. Thanks again for the solid work here.

[plusemon]

Status update after the review — here's where things stand on this branch:

Fixed here:

• Swift syntax in AppState.swift (type is ... or ... → type == "php" || type == "laravel" || type == "wordpress")
• Restored the missing name entry in _app_dialog (the "Add app" NameError)
• Merged latest master (includes the quoted-document-root + root-validation work, and the new CI gate — all three checks pass on this branch)

Still open here:

• Laravel docroot fix (mkdir $root/public + landing index written to the document root) — this is fixed on the feat: site create modal add root directory location browse option #5 branch and can be ported here
• nginx_reload → nginx_restart revert (reload primary, restart fallback) — also fixed on the feat: site create modal add root directory location browse option #5 branch
• DB root-password items from your review: plaintext persistence to bhserve.json, SQL passed via -e argv (audit log + ps), the $SUDO unbound-variable path on macOS, and the jget password round-trip

Happy to port the shared fixes over and address the DB items — or, per your suggestion, split the DB work out so the folder pickers / Laravel type / nginx reload can land independently. Which do you prefer?

[wpexpertinbd]

#5 is merged — thanks again. To your question: let's split, and this branch becomes DB-only.

Concretely, if you rebase on master now, the shared fixes come with it and drop out of this diff automatically:

• Laravel docroot (mkdir -p "$vhost_root" + landing index in the document root) — merged via feat: site create modal add root directory location browse option #5
• nginx_reload (-s reload primary, restart fallback, both engines + Nginx.cs) — merged via feat: site create modal add root directory location browse option #5
• Swift syntax + the _app_dialog name entry — merged via feat: site create modal add root directory location browse option #5

So no need to port anything here. What's left for #6 is purely the MySQL root-password work, and the four items from my review:

1. Don't persist the password to bhserve.json (the blocker). Plaintext in a 0644 file that cmd_api splices into a snapshot the GUI polls every 4s. Your new "Current password" prompt makes prompting the better answer — if it must be stored, it needs umask 077 + mode re-asserted on every rewrite + redaction in cmd_api/cmd_status.
2. Keep the SQL out of argv — printf '%s\n' "$sql" | ${SUDO:-sudo} "$cli" instead of -e "$sql" (ps + the polkit/sudo audit log). The ${SUDO:-sudo} also fixes the set -u "unbound variable" abort on macOS, where $SUDO is never assigned.
3. json_esc on the write, and note jget's [^",}]* truncates any value containing , or ".
4. A quick sanity check that db root-passwd still works when the server has no password set (the common case).

One heads-up: master now runs a CI gate on every PR (swift build / dotnet build / python+bash), so a rebase will get all three checked automatically — I see they're already green on your last push here.

No rush, and thanks for being so responsive to the review.

[wpexpertinbd]

Rebase looks great — this is DB-only now (3 files, +132/-22) and three of the four items are fully verified fixed. One blocker left, and it's a behaviour I can reproduce deterministically.

🔴 Blocker: the pkexec fallback hangs forever where no polkit agent is running

if printf '%s\n' "$sql" | pkexec "$cli" 2>/dev/null; then

The branch is gated on [ -n "${DISPLAY:-}${WAYLAND_DISPLAY:-}" ], but a set DISPLAY doesn't mean an authentication agent is there to answer. With no agent, pkexec blocks indefinitely — I left it stuck for ~10 minutes on pkexec /usr/bin/mariadb before killing it.

A/B against master, identical conditions (MariaDB stopped, so the unprivileged connect fails and we reach the fallback):

	result
master	exit 1 in 0s — ✗ failed (server running? do you have privilege?)
this branch	hung, killed at 25s (exit 124)

This matters because db root-passwd is invoked from the GUI, so the app locks up with no way to cancel — the same "BHServe Is Not Responding" failure we shipped a fix for in 1.0.50. And it's reachable in ordinary use, not just an edge case: any time the unprivileged connect fails first — the DB isn't running, or the user mistypes the current password. On a full GNOME desktop it won't hang, but it pops a system root auth dialog to set a MySQL password, which is surprising for a local-dev tool.

Suggested fix — just drop Fallback 1 (the pkexec/sudo block). It's redundant on Linux: mysql_run in platform-linux.sh already ends with the two $SUDO branches you added, and those are safe because the GUI runs $SUDO as sudo -n (non-interactive, fails fast — 1.0.50). So the privileged attempt already happens, without the hang. Keeping master's fail-fast die gives the user an actionable message instead of a frozen window. Fallback 2 (legacy SQL syntax) is fine as-is — it goes through mysql_run, so no escalation.

If you'd rather keep a privileged attempt, it needs to be non-blocking (e.g. timeout 20 pkexec …) and only run when an agent is actually present — but honestly the fail-fast path reads better here.

✅ Verified working (tested, not just read)

1. Password no longer persisted — json_set "root_password" is gone; root_password now appears only in the redaction. Nice touch adding config_dump() to scrub a legacy key written by the earlier revision of this branch, and wiring it into cmd_api / cmd_status / config show — that's more than I asked for. I tested your sed against all three shapes, since a malformed config would blank the dashboard on the 4s poll:

• no legacy key → valid JSON, all keys intact
• root_password in the middle → removed, valid JSON
• root_password as the last key (the trailing-comma trap) → "autostart": false} — valid JSON, secret gone

bhserve api stayed valid JSON with sites/services intact and leaked nothing; status redacts too.

2. SQL out of argv — printf '%s\n' "$sql" | … ✅. And you were right to use ${SUDO-sudo} rather than the ${SUDO:-sudo} I suggested: the single - preserves an explicitly-empty SUDO (already root) instead of re-escalating. Good catch, and thanks for documenting why.

3. json_esc/jget round-trip — moot now that nothing is persisted. Correct call: removing the feature removed the bug class.

4. Works with no password set — full round-trip verified: blank → set → clear using the current password → blank, with db list still working throughout. The dialog correctly only shows "Current" when one is set.

All three CI checks are green.

Minor, optional (pre-existing — not caused by this PR)

The password entries aren't masked (Gtk.Entry without set_visibility(False)). Our existing _pw_dialog has the same gap, so this isn't a regression — but since you're already in this dialog, adding set_visibility(False) to both entries (with a reveal toggle if you like) would be a nice win.

Fix the pkexec branch and I'll merge — everything else here is solid work.