From 952776c4f9addb4cedf756a17cda7a7f69a6170b Mon Sep 17 00:00:00 2001 From: nevaforget Date: Mon, 4 May 2026 12:17:31 +0200 Subject: [PATCH] batsaver: switch to pkexec helper, drop broken udev permission hack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wheel-write-via-udev approach for charge_control_end_threshold has been broken since 2026-04-08: the audit-remediation commit added ACTION=="add" to the rule, but the threshold attribute doesn't exist yet at the add event on Lenovo, so chmod fails silently and permissions are never set. moonarch-batsaver-toggle has been returning Permission denied since. Replace the udev-rule approach with a pkexec helper: defaults/bin/moonarch-batsaver-apply privileged: validate + write defaults/bin/moonarch-batsaver-toggle user: read sysfs, dispatch via pkexec defaults/bin/moonarch-batsaver-restore boot-time root restore (extracted from inline ExecStart for clarity) Default Standard-pkexec prompt — password cached per session for the ~5min auth window; no polkit no-password rule, no privilege escalation surface from misvalidated input. Same pattern Battery-Health-Charging GNOME extension uses. The boot-time restore service now skips the kernel write when the sysfs value already matches the saved state (Lenovo drivers reject same-value writes with EINVAL). DECISIONS.md documents the failure analysis and trade-offs. CLAUDE.md updated to describe the new flow. moonarch-doctor: udev-effectiveness check removed. --- CLAUDE.md | 4 +-- DECISIONS.md | 7 ++++ defaults/bin/moonarch-batsaver-apply | 35 +++++++++++++++++++ defaults/bin/moonarch-batsaver-restore | 23 ++++++++++++ defaults/bin/moonarch-batsaver-toggle | 11 ++---- .../systemd/system/moonarch-batsaver.service | 6 +--- .../udev/rules.d/90-moonarch-battery.rules | 4 --- scripts/moonarch-doctor | 12 +------ 8 files changed, 71 insertions(+), 31 deletions(-) create mode 100755 defaults/bin/moonarch-batsaver-apply create mode 100755 defaults/bin/moonarch-batsaver-restore delete mode 100644 defaults/etc/udev/rules.d/90-moonarch-battery.rules diff --git a/CLAUDE.md b/CLAUDE.md index b3254dd..d19532e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,8 +14,8 @@ Reproduzierbares Arch-Linux-Setup basierend auf archinstall + Post-Install-Autom Laptops mit `charge_control_end_threshold`-Support (ThinkPad, Framework, etc.) erhalten einen Waybar-Toggle: - Klick auf das Battery-Modul schaltet zwischen 80% und 100% Ladegrenze um - Bei aktiver Conservation erscheint ein ♥-Icon neben der Battery-Anzeige -- Zustand wird in `/var/lib/moonarch/batsaver-threshold` persistiert und beim Boot via systemd-Service wiederhergestellt -- udev-Regel gibt Gruppe `wheel` Schreibzugriff auf den Threshold (kein sudo nötig) +- Zustand wird in `/var/lib/moonarch/batsaver-threshold` persistiert und beim Boot via systemd-Service (`moonarch-batsaver-restore`) wiederhergestellt +- Toggle-Flow: `moonarch-batsaver-toggle` (User-Script) liest sysfs, entscheidet 80↔100, ruft `pkexec /usr/bin/moonarch-batsaver-apply $NEW` für den privilegierten sysfs+state-Schreibschritt. Standard-pkexec-Prompt (Passwort einmal pro Session-Cache) - Auf Desktops ohne Battery-Support versteckt sich das Feature komplett ## Nightlight (Blaufilter) diff --git a/DECISIONS.md b/DECISIONS.md index ea2a55e..d8d529f 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1,5 +1,12 @@ # Decisions +## 2026-05-04 – Battery threshold permissions: udev rule → pkexec helper + +- **Who**: Dominik, ClaudeCode +- **Why**: The wheel-write-via-udev approach for `/sys/class/power_supply/BAT0/charge_control_end_threshold` had been broken since 2026-04-08 (commit `ac2b210`, "audit remediation Q-W3"). That commit added `ACTION=="add"` to `90-moonarch-battery.rules` to "avoid firing on every battery event" — but that filter is precisely what the rule needs not to have. On Lenovo, the threshold attribute does not exist yet at the `add` event (the driver creates it slightly later); the rule fires, `chmod` fails silently because `2>/dev/null` swallows the error, and permissions are never set. The unfiltered original rule worked by accident: `add` failed silently as well, but a subsequent `change` event on the same device caught the now-existing attribute and set permissions. After the audit commit, change events stopped re-firing the rule and the toggle was permanently broken — `moonarch-batsaver-toggle` returned `Permission denied`. Verified via journalctl + manual chmod: rule fires for hidpp_battery_0 (visible exit-1 errors), no trace for BAT0; manual `chmod g+w` on BAT0's threshold succeeds (sysfs accepts the change), so the permission model itself works — only the rule path failed. +- **Tradeoffs**: Three approaches considered. (A) Restore the original unfiltered rule — fixes the symptom by accident, leaves the failure mode intact (silent fail at add, retry hopefully at change). (B) Switch to `tmpfiles.d` — Arch Wiki explicitly warns this can run before driver modules load, undefined for sysfs. (C) pkexec helper with polkit-rule — standard pattern (Battery-Health-Charging GNOME extension uses exactly this). Picked C with default Standard-pkexec prompt rather than no-password polkit rule: minor UX cost (password once per pkauth session, ≈5min cache), eliminates the entire sysfs-permission problem class, no privilege-escalation surface from a misvalidated helper. The wheel-can-write-sysfs design was a moonarch-specific deviation from common Linux practice — bringing it in line with the standard root-orientiert helper pattern. +- **How**: `defaults/bin/moonarch-batsaver-apply` (new): privileged helper invoked via pkexec; strictly validates argument (digits only, range 1-100), writes sysfs (idempotent — skips kernel write when value already matches to avoid Lenovo EINVAL on same-value writes), writes state file. `defaults/bin/moonarch-batsaver-toggle` (rewritten): user-side reads current threshold, picks 80↔100, dispatches `pkexec /usr/bin/moonarch-batsaver-apply $NEW`, then signals waybar. `defaults/etc/udev/rules.d/90-moonarch-battery.rules` deleted (and the now-empty `defaults/etc/udev/` parent removed). PKGBUILD: udev install line removed. `moonarch-doctor`: removed the udev-effectiveness check (no longer relevant). `moonarch-batsaver.service` and `moonarch-batsaver-restore` (also new in this commit, extracted from the old inline ExecStart for readability) keep root-owned boot-time restore — no permission concerns there. `CLAUDE.md` Battery-Conservation-Mode section updated to describe the new flow. + ## 2026-05-04 – Cleanup: remove invented zsh override layer, harden moondoc - **Who**: Dominik, ClaudeCode diff --git a/defaults/bin/moonarch-batsaver-apply b/defaults/bin/moonarch-batsaver-apply new file mode 100755 index 0000000..a91fe56 --- /dev/null +++ b/defaults/bin/moonarch-batsaver-apply @@ -0,0 +1,35 @@ +#!/usr/bin/bash +# ABOUTME: Privileged helper invoked via pkexec from moonarch-batsaver-toggle. +# ABOUTME: Validates the threshold value and writes it to sysfs and the state file. + +set -eu + +VAL="${1:-}" + +case "$VAL" in + ""|*[!0-9]*) + echo "Invalid argument: '$VAL' (expected integer 1-100)" >&2 + exit 1 + ;; +esac + +if [ "$VAL" -lt 1 ] || [ "$VAL" -gt 100 ]; then + echo "Out of range: $VAL (expected 1-100)" >&2 + exit 1 +fi + +THRESHOLD_FILE="/sys/class/power_supply/BAT0/charge_control_end_threshold" +STATE_DIR="/var/lib/moonarch" +STATE_FILE="$STATE_DIR/batsaver-threshold" + +[ -f "$THRESHOLD_FILE" ] || { echo "No battery threshold support" >&2; exit 1; } + +# Skip the kernel write when the value already matches — some Lenovo drivers +# reject same-value writes with EINVAL. +CURRENT=$(cat "$THRESHOLD_FILE") +if [ "$CURRENT" != "$VAL" ]; then + printf %s "$VAL" > "$THRESHOLD_FILE" +fi + +mkdir -p "$STATE_DIR" +printf %s "$VAL" > "$STATE_FILE" diff --git a/defaults/bin/moonarch-batsaver-restore b/defaults/bin/moonarch-batsaver-restore new file mode 100755 index 0000000..09aa7a5 --- /dev/null +++ b/defaults/bin/moonarch-batsaver-restore @@ -0,0 +1,23 @@ +#!/bin/sh +# ABOUTME: Restores the saved battery charge end threshold on boot. +# ABOUTME: Skips silently when the kernel already reports the same value (avoids EINVAL on some Lenovo drivers). + +set -eu + +STATE_FILE="/var/lib/moonarch/batsaver-threshold" +SYS_FILE="/sys/class/power_supply/BAT0/charge_control_end_threshold" + +[ -f "$STATE_FILE" ] || exit 0 +[ -f "$SYS_FILE" ] || exit 0 + +V=$(cat "$STATE_FILE") +case "$V" in + ""|*[!0-9]*) exit 0 ;; +esac +[ "$V" -ge 1 ] && [ "$V" -le 100 ] || exit 0 + +# Some Lenovo drivers reject writing the same value with EINVAL. +C=$(cat "$SYS_FILE") +[ "$C" = "$V" ] && exit 0 + +printf %s "$V" > "$SYS_FILE" diff --git a/defaults/bin/moonarch-batsaver-toggle b/defaults/bin/moonarch-batsaver-toggle index 2d334d1..53054f4 100755 --- a/defaults/bin/moonarch-batsaver-toggle +++ b/defaults/bin/moonarch-batsaver-toggle @@ -1,10 +1,8 @@ #!/usr/bin/bash # ABOUTME: Toggles battery conservation mode between 80% and 100% charge limit. -# ABOUTME: Writes to sysfs (immediate) and state file (persistence across reboots). +# ABOUTME: Reads sysfs as user, dispatches the privileged write via pkexec. THRESHOLD_FILE="/sys/class/power_supply/BAT0/charge_control_end_threshold" -STATE_DIR="/var/lib/moonarch" -STATE_FILE="${STATE_DIR}/batsaver-threshold" CONSERVATION_LIMIT=80 [[ -f "$THRESHOLD_FILE" ]] || exit 1 @@ -18,12 +16,7 @@ else NEW="$CONSERVATION_LIMIT" fi -# Apply immediately -echo "$NEW" > "$THRESHOLD_FILE" || exit 1 - -# Persist for next boot -mkdir -p "$STATE_DIR" -echo "$NEW" > "$STATE_FILE" || exit 1 +pkexec /usr/bin/moonarch-batsaver-apply "$NEW" || exit 1 # Signal Waybar to refresh the batsaver module (SIGRTMIN+9) pkill -RTMIN+9 waybar diff --git a/defaults/etc/systemd/system/moonarch-batsaver.service b/defaults/etc/systemd/system/moonarch-batsaver.service index ca43982..8f8f338 100644 --- a/defaults/etc/systemd/system/moonarch-batsaver.service +++ b/defaults/etc/systemd/system/moonarch-batsaver.service @@ -9,11 +9,7 @@ ConditionPathExists=/var/lib/moonarch/batsaver-threshold [Service] Type=oneshot -# Validate the threshold (integer 1–100) before writing. The state file is -# written by wheel-group users via moonarch-batsaver-toggle; the kernel rejects -# non-numeric values on sysfs, but validating here prevents noise on boot and -# makes the trust boundary explicit. -ExecStart=/bin/sh -c 'V=$(cat /var/lib/moonarch/batsaver-threshold); case "$V" in ""|*[!0-9]*) exit 0;; esac; [ "$V" -ge 1 ] && [ "$V" -le 100 ] && printf %s "$V" > /sys/class/power_supply/BAT0/charge_control_end_threshold' +ExecStart=/usr/bin/moonarch-batsaver-restore NoNewPrivileges=true ProtectHome=true PrivateTmp=true diff --git a/defaults/etc/udev/rules.d/90-moonarch-battery.rules b/defaults/etc/udev/rules.d/90-moonarch-battery.rules deleted file mode 100644 index d424027..0000000 --- a/defaults/etc/udev/rules.d/90-moonarch-battery.rules +++ /dev/null @@ -1,4 +0,0 @@ -# ABOUTME: udev rule granting wheel group write access to battery charge threshold. -# ABOUTME: Enables unprivileged toggling of conservation mode via moonarch-batsaver-toggle. - -SUBSYSTEM=="power_supply", ACTION=="add", ATTR{type}=="Battery", RUN+="/bin/sh -c 'chgrp wheel /sys%p/charge_control_end_threshold 2>/dev/null; chmod g+w /sys%p/charge_control_end_threshold 2>/dev/null'" diff --git a/scripts/moonarch-doctor b/scripts/moonarch-doctor index c291a4c..eb66184 100755 --- a/scripts/moonarch-doctor +++ b/scripts/moonarch-doctor @@ -168,18 +168,8 @@ for svc in NetworkManager bluetooth greetd systemd-timesyncd ufw auto-cpufreq; d done # Battery conservation service (laptop only) -THRESHOLD_FILE="/sys/class/power_supply/BAT0/charge_control_end_threshold" -if [[ -f "$THRESHOLD_FILE" ]]; then +if [[ -f /sys/class/power_supply/BAT0/charge_control_end_threshold ]]; then check_system_service moonarch-batsaver - - # Verify udev rule effectiveness — file must be group=wheel and group-writable - THRESHOLD_GROUP=$(stat -c '%G' "$THRESHOLD_FILE") - THRESHOLD_PERMS=$(stat -c '%a' "$THRESHOLD_FILE") - if [[ "$THRESHOLD_GROUP" == "wheel" ]] && [[ "${THRESHOLD_PERMS:1:1}" -ge 6 ]]; then - pass "battery threshold writable by wheel (udev rule active)" - else - fail "battery threshold not writable by wheel (group=$THRESHOLD_GROUP, mode=$THRESHOLD_PERMS — udev rule not applied)" - fi else pass "moonarch-batsaver (skipped — no battery threshold support)" fi