Fix correctness bugs across the helper scripts

panama-osd read the wrong brightnessctl field, showing the hardware
max instead of a percentage on any backlight device. panama-doctor
called three sibling scripts by bare name with nothing on PATH,
making three health checks permanently and falsely report broken; its
repair actions also reused the short probe timeout, so a slow-but-
successful restart was reported as failed. panama-wifi-qr left the
cleartext passphrase temp file behind on its failure path (the RETURN
trap doesn't fire on exit), and its nmcli parsing broke on connection
names containing a colon or backslash -- verified against a real
NetworkManager profile.

panama-power-profile's set command always returned success regardless
of whether the write actually took. panama-keyring's daemon-origin
check picked whichever gnome-keyring-daemon process happened to
enumerate first in /proc, defeating the exact dual-daemon scenario it
exists to detect; it now resolves the PID that actually owns the
Secret Service D-Bus name. gnf aborted before running a firmware
update whenever the metadata was already current (a non-error exit
under set -e), and its flatpak update lacked the -y its own docs
promise.

Claude-Session: https://claude.ai/code/session_01E6TJUAh41HaP25MVHWkhRZ
This commit is contained in:
Gabriel Brown
2026-08-18 21:23:28 -04:00
parent 719ef2f38e
commit 2a716dac9e
9 changed files with 178 additions and 42 deletions
+33 -4
View File
@@ -61,6 +61,16 @@ class DoctorConfig:
runtime_dir: Path
path: str
timeout: float
# Defaults to this file's own directory, where its sibling helpers
# (panama-action, panama-brightness, calendar-agenda) actually live.
scripts_dir: Path = Path(__file__).resolve().parent
# Repair actions (service restarts, the Quickshell restart-shell action)
# can legitimately run longer than a quick health-check probe -- a
# Quickshell restart alone waits for the old process to exit, the new one
# to start, and settle. Reusing `timeout` here would kill a slow-but-
# successful repair and report it as failed even though a following
# health scan would show everything recovered. See run_repair_command.
repair_timeout: float = 15.0
@property
def command_env(self) -> dict[str, str]:
@@ -193,11 +203,25 @@ def config_from_environment() -> DoctorConfig:
state_home = environment_path("PANAMA_DOCTOR_STATE_HOME", Path(os.environ.get("XDG_STATE_HOME", home / ".local/state")))
runtime_dir = environment_path("PANAMA_DOCTOR_RUNTIME_DIR", Path(os.environ.get("XDG_RUNTIME_DIR", "/run/user/0")))
root = environment_path("PANAMA_DOCTOR_ROOT", Path(__file__).resolve().parents[4])
# Sibling helpers (panama-action, panama-brightness, calendar-agenda) live
# next to this file. PATH is not a reliable way to find them -- nothing in
# this repository puts the Quickshell scripts directory on PATH -- so they
# are invoked by resolved path instead, the same way check_hyprlock already
# resolves panama-lock.
scripts_dir = environment_path("PANAMA_DOCTOR_SCRIPTS_DIR", Path(__file__).resolve().parent)
try:
timeout = float(os.environ.get("PANAMA_DOCTOR_TIMEOUT", "3"))
except ValueError:
timeout = 3.0
return DoctorConfig(root, home, config_home, state_home, runtime_dir, os.environ.get("PANAMA_DOCTOR_PATH", os.environ.get("PATH", "")), max(0.05, min(timeout, 15.0)))
timeout = max(0.05, min(timeout, 15.0))
try:
repair_timeout = float(os.environ.get("PANAMA_DOCTOR_REPAIR_TIMEOUT", "15"))
except ValueError:
repair_timeout = 15.0
# Never shorter than the probe timeout, and bounded so a hung repair still
# gives up rather than blocking the caller indefinitely.
repair_timeout = max(timeout, min(repair_timeout, 30.0))
return DoctorConfig(root, home, config_home, state_home, runtime_dir, os.environ.get("PANAMA_DOCTOR_PATH", os.environ.get("PATH", "")), timeout, scripts_dir, repair_timeout)
def run_command(command: tuple[str, ...], config: DoctorConfig, cwd: Path | None = None) -> CommandResult:
@@ -222,7 +246,7 @@ def run_repair_command(command: tuple[str, ...], config: DoctorConfig, cwd: Path
command,
capture_output=True,
text=True,
timeout=config.timeout,
timeout=config.repair_timeout,
check=False,
env=config.command_env,
cwd=cwd,
@@ -357,7 +381,7 @@ def check_hyprlock(config: DoctorConfig) -> Check:
def check_brightness(config: DoctorConfig) -> Check:
result = run_command(("panama-brightness", "list"), config)
result = run_command((str(config.scripts_dir / "panama-brightness"), "list"), config)
instructions = Action("instructions", "View setup instructions", target="ddc-permissions")
if result.state == "timeout":
return Check("input.brightness", "input-media", "External monitor brightness", "warning", "DDC/CI probe timed out.", instructions)
@@ -418,7 +442,7 @@ def check_home_assistant(config: DoctorConfig) -> Check:
def check_calendar(config: DoctorConfig) -> Check:
result = run_command(("calendar-agenda", "probe"), config)
result = run_command((str(config.scripts_dir / "calendar-agenda"), "probe"), config)
action = Action("open", "Open Date & Time", target="datetime")
if result.state == "missing":
return Check("integration.calendar", "integrations", "Calendar", "unconfigured", "Calendar integration is not installed.")
@@ -583,6 +607,11 @@ def snapshot(config: DoctorConfig) -> dict[str, object]:
def repair_authored_command(check_id: str, config: DoctorConfig) -> RepairResult:
command = REPAIR_COMMANDS[check_id]
if check_id == "desktop.quickshell":
# panama-action is a sibling helper script, not a PATH-resolved
# executable; see check_brightness and check_calendar for the same
# resolution against the same bug.
command = (str(config.scripts_dir / command[0]), *command[1:])
exit_code, _ = run_repair_command(command, config)
message = "Repair completed. A fresh health check will verify recovery." if exit_code == 0 \
else "The authored repair command could not be completed."
+66 -16
View File
@@ -39,13 +39,36 @@ import re
import sys
def daemon_origin():
"""Whether the running secrets daemon came from PAM or from D-Bus activation.
def secrets_name_owner_pid():
"""PID currently owning the org.freedesktop.secrets D-Bus name, if any.
A D-Bus-activated daemon is the signature of the crash-and-replace case
above: it is the one that cannot have the login password. PAM's daemon lives
outside the app slice, so the cgroup tells the two apart.
This is the only reliable way to identify which daemon actually answers
Secret Service calls right now.
"""
try:
import gi
gi.require_version("Gio", "2.0")
from gi.repository import Gio, GLib
bus = Gio.bus_get_sync(Gio.BusType.SESSION, None)
result = bus.call_sync(
"org.freedesktop.DBus",
"/org/freedesktop/DBus",
"org.freedesktop.DBus",
"GetConnectionUnixProcessID",
GLib.Variant("(s)", ("org.freedesktop.secrets",)),
GLib.VariantType("(u)"),
Gio.DBusCallFlags.NONE,
-1,
None,
)
return result.unpack()[0]
except Exception: # noqa: BLE001 - no name owner is a legitimate state
return None
def any_keyring_daemon_running():
try:
for pid in os.listdir("/proc"):
if not pid.isdigit():
@@ -55,19 +78,46 @@ def daemon_origin():
cmdline = handle.read().decode("utf-8", "replace")
except OSError:
continue
if "gnome-keyring-daemon" not in cmdline:
continue
try:
with open(f"/proc/{pid}/cgroup", "r") as handle:
cgroup = handle.read()
except OSError:
return "unknown"
if re.search(r"dbus-.*org\.freedesktop\.secrets", cgroup):
return "dbus"
return "pam"
if "gnome-keyring-daemon" in cmdline:
return True
except OSError:
pass
return "none"
return False
def daemon_origin():
"""Whether the running secrets daemon came from PAM or from D-Bus activation.
A D-Bus-activated daemon is the signature of the crash-and-replace case
above: it is the one that cannot have the login password. PAM's daemon lives
outside the app slice, so the cgroup tells the two apart.
A machine can have two gnome-keyring-daemon processes at once -- a
lingering PAM one alongside its D-Bus-activated replacement -- so which
process this reports on matters: it must be the one that actually owns
org.freedesktop.secrets right now, not merely the first one /proc happens
to enumerate.
"""
owner_pid = secrets_name_owner_pid()
if owner_pid is None:
return "unknown" if any_keyring_daemon_running() else "none"
try:
with open(f"/proc/{owner_pid}/cmdline", "rb") as handle:
cmdline = handle.read().decode("utf-8", "replace")
except OSError:
return "unknown"
if "gnome-keyring-daemon" not in cmdline:
return "unknown"
try:
with open(f"/proc/{owner_pid}/cgroup", "r") as handle:
cgroup = handle.read()
except OSError:
return "unknown"
if re.search(r"dbus-.*org\.freedesktop\.secrets", cgroup):
return "dbus"
return "pam"
def load_service():
+1 -1
View File
@@ -71,7 +71,7 @@ adjust_microphone() {
brightness_percent() {
local output="$1" percent
percent="$(awk -F, 'NR == 1 { value=$5; gsub(/%/, "", value); print value }' <<<"$output")"
percent="$(awk -F, 'NR == 1 { value=$4; gsub(/%/, "", value); print value }' <<<"$output")"
[[ $percent =~ ^[0-9]+$ ]] || return 1
printf '%s\n' "$percent"
}
@@ -67,15 +67,19 @@ cmd_list() {
}
cmd_set() {
local profile="${1:-}"
local profile="${1:-}" status
# Constrained rather than passed through: this reaches a system service.
[[ "$profile" =~ ^[a-z-]+$ ]] || {
printf 'panama-power-profile: refusing a profile name with unexpected characters\n' >&2
return 2
}
# pipefail is set above, so $? here is busctl's real exit status, not
# head's -- a failed write (daemon stopped, profile rejected) must be
# reported rather than always claimed as a success.
busctl set-property "$BUS_NAME" "$OBJECT" "$BUS_NAME" ActiveProfile s "$profile" 2>&1 >/dev/null \
| head -2 >&2
return 0
status=$?
return "$status"
}
case "${1:-list}" in
+35 -10
View File
@@ -26,6 +26,16 @@
set -uo pipefail
# The payload file (created in cmd_qr) is tracked at script scope so it can be
# removed no matter how the script exits -- success, an emit_error exit 0, or a
# signal -- rather than only on a clean function return.
payload_file=""
cleanup_payload() {
[[ -n $payload_file ]] && rm -f -- "$payload_file"
}
trap cleanup_payload EXIT
emit_error() {
printf '{"networks":[],"path":"","error":%s}\n' "$(jq -Rn --arg e "$1" '$e')"
exit 0
@@ -35,22 +45,33 @@ command -v nmcli >/dev/null 2>&1 || emit_error 'NetworkManager is not available'
command -v qrencode >/dev/null 2>&1 || emit_error 'qrencode is not installed, so a Wi-Fi QR code cannot be drawn'
cmd_list() {
local rows=() name ssid psk
local rows=() name type ssid psk
while IFS= read -r name; do
[[ -n "$name" ]] || continue
ssid="$(nmcli -g 802-11-wireless.ssid connection show "$name" 2>/dev/null)"
# nmcli's terse mode backslash-escapes ':' and '\' WITHIN a field so a
# combined NAME,TYPE line stays splittable -- but a plain awk -F:
# doesn't know that, so a name containing either character (e.g.
# "Cafe: Guest") gets split in the wrong place and TYPE no longer
# lines up, silently dropping the connection from this list. Querying
# one field at a time with escaping turned off (-e no) sidesteps the
# problem entirely: there's nothing to split, so each value comes
# back exactly as stored.
type="$(nmcli -e no -g connection.type connection show "$name" 2>/dev/null)"
[[ "$type" == "802-11-wireless" ]] || continue
ssid="$(nmcli -e no -g 802-11-wireless.ssid connection show "$name" 2>/dev/null)"
[[ -n "$ssid" ]] || ssid="$name"
# Only networks whose passphrase this user can actually read are
# shareable. An enterprise network has no passphrase to share at all,
# and a QR code for one would simply not work.
psk="$(nmcli -s -g 802-11-wireless-security.psk connection show "$name" 2>/dev/null)"
psk="$(nmcli -e no -s -g 802-11-wireless-security.psk connection show "$name" 2>/dev/null)"
rows+=("$(jq -cn --arg name "$name" --arg ssid "$ssid" \
--argjson shareable "$([[ -n "$psk" ]] && echo true || echo false)" \
'{name: $name, ssid: $ssid, shareable: $shareable}')")
done < <(nmcli -t -f NAME,TYPE connection show 2>/dev/null \
| awk -F: '$2 == "802-11-wireless" { print $1 }')
done < <(nmcli -e no -t -f NAME connection show 2>/dev/null)
if [[ ${#rows[@]} -eq 0 ]]; then
printf '{"networks":[],"path":"","error":"No saved Wi-Fi networks."}\n'
@@ -77,11 +98,16 @@ cmd_qr() {
local name="${1:-}"
[[ -n "$name" ]] || emit_error 'no network named'
local ssid hidden psk_file payload_file out_dir out_file
ssid="$(nmcli -g 802-11-wireless.ssid connection show "$name" 2>/dev/null)"
local ssid hidden psk_file out_dir out_file
# -e no here too: $name is the literal connection name (list emits it
# un-escaped -- see cmd_list), and nmcli's terse escaping is a one-way
# transform on VALUES, not something connection-show lookups expect on
# their NAME argument. Escaping $ssid/$psk here would feed escape_field
# an already-escaped value below and double-escape it.
ssid="$(nmcli -e no -g 802-11-wireless.ssid connection show "$name" 2>/dev/null)"
[[ -n "$ssid" ]] || emit_error "There is no saved network called \"$name\"."
hidden="$(nmcli -g 802-11-wireless.hidden connection show "$name" 2>/dev/null)"
hidden="$(nmcli -e no -g 802-11-wireless.hidden connection show "$name" 2>/dev/null)"
[[ "$hidden" == "yes" ]] && hidden=true || hidden=false
out_dir="${XDG_RUNTIME_DIR:-/run/user/$(id -u)}/panama"
@@ -96,13 +122,12 @@ cmd_qr() {
# Built in a file rather than a variable that could be echoed, and piped to
# qrencode on stdin so the passphrase never appears in argv.
payload_file="$(mktemp "$out_dir/payload.XXXXXX")" || emit_error 'could not create a temporary file'
trap 'rm -f "$payload_file"' RETURN
{
printf 'WIFI:T:WPA;S:'
printf '%s' "$ssid" | escape_field
printf ';P:'
nmcli -s -g 802-11-wireless-security.psk connection show "$name" 2>/dev/null | escape_field
nmcli -e no -s -g 802-11-wireless-security.psk connection show "$name" 2>/dev/null | escape_field
printf ';H:%s;;' "$hidden"
} >"$payload_file"