From 60a321e6f4989fd623aadcab546fbd1548cbc3ce Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Tue, 18 Aug 2026 12:11:56 -0400 Subject: [PATCH] Harden Health verification isolation --- .../dot/quickshell/modules/settings/README.md | 17 +- .../2026-08-18-panama-health-recovery.md | 20 ++- tests/quickshell/health-service-contract.sh | 69 +++++++- tests/quickshell/settings-pages-contract.sh | 157 +++++++++++++++--- 4 files changed, 223 insertions(+), 40 deletions(-) diff --git a/config/dot/quickshell/modules/settings/README.md b/config/dot/quickshell/modules/settings/README.md index a2db36c..e037ad1 100644 --- a/config/dot/quickshell/modules/settings/README.md +++ b/config/dot/quickshell/modules/settings/README.md @@ -11,8 +11,10 @@ from the Settings sidebar and its live 54px footer, the degraded-only bar indicator, and Vicinae's **Panama: Check System Health** command. Healthy scans reserve no bar space and produce no notification. -`services/Health.qml` owns the last accepted redacted snapshot and invokes only -`scripts/panama-doctor`. For a concise terminal view, run: +`services/Health.qml` owns the last accepted redacted snapshot. It invokes +`scripts/panama-doctor` for scans and bounded repairs, `wl-copy` only for an +explicit **Copy Report**, and bounded `notify-send` only when an external repair +fails. For a concise terminal view, run: ```bash ~/.config/quickshell/scripts/panama-doctor --summary @@ -108,11 +110,12 @@ slot**, because that is the row's default property, so only the right-hand edge becomes clickable. Use `activatable: true` with `onActivated` for a whole-row target. -**A copy of the Quickshell config shares the live shell's ID.** Quickshell -derives the Shell ID from config *content*, not path, so -`cp -a config/dot/quickshell $tmp && qs -p $tmp kill` kills the running -desktop, and `qs -p $tmp ipc call …` can drive it. Harnesses that point at a -single distinct `.qml` file are safe; copying the whole directory is not. +**A content-identical Quickshell entry can share the live shell's ID.** +Quickshell derives the Shell ID from config *content*, not path. Runtime +harnesses therefore create a distinct semantic entry file, address that exact +file with `qs -p`, and discover its PID from the exact Config path in +`qs list --all`. They terminate only that recorded PID with `kill`; never use +`qs kill` from a copied configuration. ## Where state lives diff --git a/docs/superpowers/plans/2026-08-18-panama-health-recovery.md b/docs/superpowers/plans/2026-08-18-panama-health-recovery.md index 82dbe9b..3417bec 100644 --- a/docs/superpowers/plans/2026-08-18-panama-health-recovery.md +++ b/docs/superpowers/plans/2026-08-18-panama-health-recovery.md @@ -496,7 +496,7 @@ git add config/dot/quickshell/scripts/panama-doctor config/dot/quickshell/servic git commit -m "Add bounded Panama recovery actions" ``` -### Task 7: Full verification, live read-only audit, and documentation +### Task 7: Full verification, controller-deferred live audit, and documentation **Integration note:** `origin/main` added Mouse, Privacy, Region, and Online Accounts destinations while this feature was in review. Merge commit `4ef2f01` @@ -518,7 +518,9 @@ doctor/IPC comparison. Those checks remain for the controller after code review. **Interfaces:** - Consumes: the complete feature and existing regression suite. -- Produces: current user documentation, a redacted live health snapshot, and final verification evidence. +- Produces: current user documentation and final contract evidence. The + redacted live health snapshot and live shell audit are deferred to the + controller after code review. - [x] **Step 1: Document boundaries and entry points** @@ -538,8 +540,8 @@ for test in tests/quickshell/*contract.sh; do "$test"; done for test in tests/hypr/*contract.sh; do "$test"; done ``` -Expected: every command exits 0. After the Settings parity merge, the current -inventory is 65 Quickshell contracts and 2 Hyprland contracts (the original +Expected: every command exits 0. After the latest Settings and installer work, +the current inventory is 66 Quickshell contracts and 2 Hyprland contracts (the original pre-merge estimate was 58). Integration result: syntax and all focused Health/Settings contracts pass. Two @@ -549,7 +551,10 @@ run two failed Focus Session and Health Service. Each failed contract passed immediately when rerun alone. Hyprland contracts passed 2/2. No out-of-scope test or service code was changed to hide this suite-order interference. -- [ ] **Step 3: Run a redacted live read-only comparison** +- [ ] **Step 3: Controller runs a redacted live read-only comparison** + +Deferred to the controller after code review; Task 7 does not produce this +live output. Run: @@ -563,7 +568,10 @@ qs ipc call health status | jq '{status, busy, checks: [.checks[] | {id, status} Expected: helper and direct service states agree. Do not print details from integrations; copied and IPC reports contain only redacted authored observations. -- [ ] **Step 4: Reload and inspect the live shell** +- [ ] **Step 4: Controller reloads and inspects the live shell** + +Deferred to the controller after code review; Task 7 does not reload or inspect +the daily-driver shell. Run: diff --git a/tests/quickshell/health-service-contract.sh b/tests/quickshell/health-service-contract.sh index f19ff73..4482808 100755 --- a/tests/quickshell/health-service-contract.sh +++ b/tests/quickshell/health-service-contract.sh @@ -117,6 +117,8 @@ copy_file="$fixture_dir/copied-report.json" repair_mode_file="$fixture_dir/repair-mode" repair_log="$fixture_dir/repair.log" notification_log="$fixture_dir/notifications.log" +repair_started_file="$fixture_dir/repair-started" +repair_release_file="$fixture_dir/repair-release" printf 'success\n' >"$repair_mode_file" printf '%s\n' \ '#!/usr/bin/env bash' \ @@ -126,8 +128,12 @@ printf '%s\n' \ " printf '%s\\n' '$warning_snapshot'" \ ' exit 0' \ 'fi' \ + 'if [[ "$1" == "--repair" ]]; then' \ + ' repair_start_time="$(awk '\''{ print $22 }'\'' "/proc/$$/stat")"' \ + ' printf "%s|%s\n" "$$" "$repair_start_time" >"$PANAMA_HEALTH_REPAIR_STARTED"' \ + ' while [[ ! -e "$PANAMA_HEALTH_REPAIR_RELEASE" ]]; do sleep 0.02; done' \ + 'fi' \ 'if [[ "$1" == "--repair" && "$2" == "panama.caffeine" && "$3" == "--json" ]]; then' \ - ' sleep 0.25' \ ' case "$(cat "$PANAMA_HEALTH_REPAIR_MODE_FILE")" in' \ ' success) printf "{\"schemaVersion\":1,\"checkId\":\"panama.caffeine\",\"accepted\":true,\"exitCode\":0,\"message\":\"Duplicate inhibitors were released.\"}\\n"; exit 0 ;;' \ ' failed) printf "{\"schemaVersion\":1,\"checkId\":\"panama.caffeine\",\"accepted\":true,\"exitCode\":7,\"message\":\"Duplicate inhibitors could not be released.\"}\\n"; exit 7 ;;' \ @@ -136,7 +142,6 @@ printf '%s\n' \ ' esac' \ 'fi' \ 'if [[ "$1" == "--repair" && "$2" == "desktop.quickshell" && "$3" == "--json" ]]; then' \ - ' sleep 0.25' \ ' printf "{\"schemaVersion\":1,\"checkId\":\"desktop.quickshell\",\"accepted\":true,\"exitCode\":0,\"message\":\"Panama shell restart was requested.\"}\\n"' \ ' exit 0' \ 'fi' \ @@ -154,12 +159,57 @@ chmod +x "$copy_bin/wl-copy" "$copy_bin/notify-send" run() { PATH="$copy_bin:$PATH" PANAMA_HEALTH_HELPER="$helper" PANAMA_HEALTH_COPY_FILE="$copy_file" \ PANAMA_HEALTH_REPAIR_MODE_FILE="$repair_mode_file" PANAMA_HEALTH_REPAIR_LOG="$repair_log" \ - PANAMA_HEALTH_NOTIFICATION_LOG="$notification_log" qs -p "$harness" "$@" + PANAMA_HEALTH_NOTIFICATION_LOG="$notification_log" \ + PANAMA_HEALTH_REPAIR_STARTED="$repair_started_file" PANAMA_HEALTH_REPAIR_RELEASE="$repair_release_file" \ + qs -p "$harness" "$@" } harness_pid="" +harness_start_time="" + +process_identity_matches() { + local pid="$1" expected_start_time="$2" expected_command="${3:-}" current_start_time + + [[ "$pid" =~ ^[0-9]+$ && "$expected_start_time" =~ ^[0-9]+$ ]] || return 1 + [[ -r "/proc/$pid/stat" ]] || return 1 + current_start_time="$(awk '{ print $22 }' "/proc/$pid/stat" 2>/dev/null)" || return 1 + [[ "$current_start_time" == "$expected_start_time" ]] || return 1 + if [[ -n "$expected_command" ]]; then + [[ -r "/proc/$pid/cmdline" ]] || return 1 + tr '\0' '\n' <"/proc/$pid/cmdline" | grep -Fxq "$expected_command" + fi +} cleanup() { - [[ -n "$harness_pid" ]] && kill "$harness_pid" >/dev/null 2>&1 || true + : >"$repair_release_file" + if [[ -f "$repair_started_file" ]]; then + IFS='|' read -r repair_pid repair_start_time <"$repair_started_file" || true + if process_identity_matches "$repair_pid" "$repair_start_time" "$helper"; then + for _ in $(seq 1 40); do + ! process_identity_matches "$repair_pid" "$repair_start_time" "$helper" && break + sleep 0.05 + done + if process_identity_matches "$repair_pid" "$repair_start_time" "$helper"; then + kill "$repair_pid" >/dev/null 2>&1 || true + for _ in $(seq 1 20); do + ! process_identity_matches "$repair_pid" "$repair_start_time" "$helper" && break + sleep 0.05 + done + if process_identity_matches "$repair_pid" "$repair_start_time" "$helper"; then + kill -KILL "$repair_pid" >/dev/null 2>&1 || true + fi + fi + fi + fi + if process_identity_matches "$harness_pid" "$harness_start_time"; then + kill "$harness_pid" >/dev/null 2>&1 || true + for _ in $(seq 1 40); do + ! process_identity_matches "$harness_pid" "$harness_start_time" && break + sleep 0.05 + done + if process_identity_matches "$harness_pid" "$harness_start_time"; then + kill -KILL "$harness_pid" >/dev/null 2>&1 || true + fi + fi rm -rf "$fixture_dir" } trap cleanup EXIT @@ -167,6 +217,7 @@ trap cleanup EXIT PATH="$copy_bin:$PATH" PANAMA_HEALTH_HELPER="$helper" PANAMA_HEALTH_COPY_FILE="$copy_file" \ PANAMA_HEALTH_REPAIR_MODE_FILE="$repair_mode_file" PANAMA_HEALTH_REPAIR_LOG="$repair_log" \ PANAMA_HEALTH_NOTIFICATION_LOG="$notification_log" \ + PANAMA_HEALTH_REPAIR_STARTED="$repair_started_file" PANAMA_HEALTH_REPAIR_RELEASE="$repair_release_file" \ qs -p "$harness" --daemonize >/dev/null for _ in $(seq 1 40); do run ipc show 2>/dev/null | rg -q '^target health-test$' && break @@ -174,6 +225,9 @@ for _ in $(seq 1 40); do done run ipc show 2>/dev/null | rg -q '^target health-test$' || fail 'test IPC target did not start' harness_pid="$(run list | awk '/Process ID:/ { print $3; exit }')" +harness_start_time="$(awk '{ print $22 }' "/proc/$harness_pid/stat" 2>/dev/null || true)" +process_identity_matches "$harness_pid" "$harness_start_time" \ + || fail 'could not capture a stable health harness process identity' [[ "$(run ipc call health-test accept "$warning_snapshot" 0)" == "true" ]] \ || fail 'valid warning snapshot was rejected' @@ -230,14 +284,21 @@ jq -e '.busy == false and .generation == ($before + 2) and .queuedRefresh == fal >/dev/null <<<"$state" || fail "queued refresh did not run exactly once: $state" printf 'success\n' >"$repair_mode_file" +rm -f "$repair_started_file" "$repair_release_file" repair_generation="$(jq -r .generation <<<"$state")" [[ "$(run ipc call health-test repair panama.caffeine)" == "true" ]] \ || fail 'repairable check was refused' +for _ in $(seq 1 100); do + [[ -s "$repair_started_file" ]] && break + sleep 0.05 +done +[[ -s "$repair_started_file" ]] || fail 'repair helper never reached the started marker' run ipc call health-test queue >/dev/null working_state="$(run ipc call health-test status)" jq -e '.repairingId == "panama.caffeine" and .queuedRefresh == true and (.checkStates[] | select(.id == "panama.caffeine") | .status) == "warning"' \ >/dev/null <<<"$working_state" || fail "repair did not retain the degraded row while working: $working_state" +: >"$repair_release_file" for _ in $(seq 1 120); do state="$(run ipc call health-test status)" jq -e '.busy == false and .generation == ($before + 1) and .queuedRefresh == false' --argjson before "$repair_generation" \ diff --git a/tests/quickshell/settings-pages-contract.sh b/tests/quickshell/settings-pages-contract.sh index 77bd238..919605e 100755 --- a/tests/quickshell/settings-pages-contract.sh +++ b/tests/quickshell/settings-pages-contract.sh @@ -85,8 +85,13 @@ fi state_home="$(mktemp -d /tmp/panama-settings-pages-state.XXXXXX)" source_config_path="$repo_dir/config/dot/quickshell" config_path="$state_home/quickshell" +harness="$config_path/settings-pages-harness.qml" test_bin="$state_home/bin" shell_log="$state_home/quickshell.log" +production_config_path="$HOME/.config/quickshell/shell.qml" +harness_pid="" +harness_shell_id="" +production_before="" cleanup_bootstrap() { rm -rf "$state_home" @@ -95,6 +100,21 @@ trap cleanup_bootstrap EXIT mkdir -p "$test_bin" cp -a "$source_config_path" "$config_path" +python3 - "$config_path/shell.qml" "$harness" "$$" <<'PY' +import sys + +source_path, harness_path, identity = sys.argv[1:] +source = open(source_path, encoding="utf-8").read() +needle = "ShellRoot {\n" +replacement = ( + needle + + f' readonly property string settingsPagesHarnessIdentity: "settings-pages-contract-{identity}"\n' +) +if source.count(needle) != 1: + raise SystemExit("shell.qml does not have exactly one ShellRoot") +with open(harness_path, "w", encoding="utf-8") as handle: + handle.write(source.replace(needle, replacement, 1)) +PY cat >"$config_path/scripts/panama-home-assistant" <<'EOF' #!/usr/bin/env bash @@ -138,52 +158,141 @@ EOF chmod +x "$test_bin/flatpak" qs_for_test() { - PATH="$test_bin:$PATH" QS_CONFIG_PATH="$config_path" XDG_STATE_HOME="$state_home" \ - qs -p "$config_path" "$@" + if [[ "${1:-}" == "ipc" && "$harness_pid" =~ ^[0-9]+$ ]]; then + PATH="$test_bin:$PATH" XDG_STATE_HOME="$state_home" \ + qs -p "$harness" ipc --pid "$harness_pid" "${@:2}" + else + PATH="$test_bin:$PATH" XDG_STATE_HOME="$state_home" \ + qs -p "$harness" "$@" + fi } -stop_test_shell() { - qs_for_test kill >/dev/null 2>&1 || true - for _ in $(seq 1 80); do - if ! qs_for_test list 2>/dev/null | rg '^Instance ' >/dev/null \ - && ! qs_for_test ipc show >/dev/null 2>&1; then - return 0 +instances_for_path() { + local expected_path="$1" listing + + listing="$(qs list --all 2>/dev/null)" || return 1 + awk -v expected="$expected_path" ' + /^Instance / { pid = ""; shell_id = "" } + /^[[:space:]]*Process ID:/ { pid = $3 } + /^[[:space:]]*Shell ID:/ { shell_id = $3 } + /^[[:space:]]*Config path:/ { + path = $0 + sub(/^[[:space:]]*Config path: /, "", path) + if (path == expected && pid ~ /^[0-9]+$/ && shell_id != "") + print pid "|" shell_id + } + ' <<<"$listing" +} + +harness_identity_matches() { + local current + + [[ "$harness_pid" =~ ^[0-9]+$ && -n "$harness_shell_id" ]] || return 1 + current="$(instances_for_path "$harness")" || return 1 + grep -Fxq "$harness_pid|$harness_shell_id" <<<"$current" +} + +production_is_preserved() { + local current record pid shell_id + + current="$(instances_for_path "$production_config_path")" || return 1 + while IFS='|' read -r pid shell_id; do + [[ -n "$pid" ]] || continue + kill -0 "$pid" >/dev/null 2>&1 || return 1 + record="$pid|$shell_id" + grep -Fxq "$record" <<<"$current" || return 1 + done <<<"$production_before" +} + +stop_harness() { + local remaining + + if harness_identity_matches; then + kill "$harness_pid" >/dev/null 2>&1 || true + for _ in $(seq 1 80); do + ! kill -0 "$harness_pid" >/dev/null 2>&1 && break + sleep 0.05 + done + if kill -0 "$harness_pid" >/dev/null 2>&1 && harness_identity_matches; then + kill -KILL "$harness_pid" >/dev/null 2>&1 || true + for _ in $(seq 1 20); do + ! kill -0 "$harness_pid" >/dev/null 2>&1 && break + sleep 0.05 + done fi - sleep 0.1 - done - return 1 + fi + remaining="$(instances_for_path "$harness")" || return 1 + harness_pid="" + harness_shell_id="" + [[ -z "$remaining" ]] } cleanup() { - qs_for_test ipc call settings close >/dev/null 2>&1 || true - if stop_test_shell; then + local cleanup_ok=0 + + stop_harness || cleanup_ok=1 + production_is_preserved || cleanup_ok=1 + if (( cleanup_ok == 0 )); then rm -rf "$state_home" else - printf 'settings pages contract: branch shell did not stop; retained %s\n' \ + printf 'settings pages contract: isolated harness cleanup failed; retained %s\n' \ "$state_home" >&2 fi + return "$cleanup_ok" } trap cleanup EXIT start_test_shell() { - stop_test_shell || fail 'pre-existing branch shell did not stop cleanly' + local harness_instances production_pid production_shell_id + + harness_instances="$(instances_for_path "$harness")" \ + || fail 'could not inspect Quickshell instances before starting the runtime harness' + [[ -z "$harness_instances" ]] \ + || fail 'an unexpected process already uses the runtime harness path' for _attempt in 1 2; do qs_for_test --daemonize >"$shell_log" 2>&1 for _ in $(seq 1 80); do - if qs_for_test ipc show 2>/dev/null | rg '^target settings$' >/dev/null; then - return - fi - sleep 0.1 + harness_instances="$(instances_for_path "$harness")" \ + || fail 'could not inspect the runtime harness instance' + [[ -n "$harness_instances" ]] && break + sleep 0.05 done - stop_test_shell || fail 'failed branch-shell attempt did not stop cleanly' + if [[ "$(wc -l <<<"$harness_instances")" == 1 && -n "$harness_instances" ]]; then + IFS='|' read -r harness_pid harness_shell_id <<<"$harness_instances" + [[ "$harness_pid" =~ ^[0-9]+$ ]] \ + || fail 'runtime harness did not expose a numeric PID' + + while IFS='|' read -r production_pid production_shell_id; do + [[ -n "$production_pid" ]] || continue + [[ "$harness_shell_id" != "$production_shell_id" ]] || { + stop_harness + fail 'runtime harness shares a Shell ID with production' + } + done <<<"$production_before" + production_is_preserved || { + stop_harness + fail 'production changed before isolated page routing began' + } + + for _ in $(seq 1 80); do + if qs_for_test ipc show 2>/dev/null | rg '^target settings$' >/dev/null; then + return + fi + sleep 0.1 + done + fi + stop_harness || fail 'failed runtime harness attempt did not stop cleanly' done sed -n '1,200p' "$shell_log" >&2 fail 'isolated branch shell did not start' } +production_before="$(instances_for_path "$production_config_path")" \ + || fail 'could not list Quickshell instances for the production baseline' +production_is_preserved || fail 'could not capture a stable production instance set' start_test_shell qs_for_test ipc call home-assistant fixture ready >/dev/null -shell_pid="$(qs_for_test list | awk '/Process ID:/ { print $3; exit }')" +shell_pid="$harness_pid" [[ "$shell_pid" =~ ^[0-9]+$ ]] || fail 'could not identify the branch shell process' pages=(home appearance displays connectivity home-phone desktop sound notifications screen-intelligence shortcuts services about) @@ -218,6 +327,8 @@ intelligence_desktop_file="$HOME/.local/share/applications/panama-screen-intelli desktop-file-validate "$intelligence_desktop_file" >/dev/null || fail 'Screen Intelligence desktop entry is invalid' trap - EXIT -cleanup +cleanup || fail 'runtime harness did not stop without disturbing production' [[ ! -e "$state_home" ]] || fail 'temporary Settings state was not removed after shell exit' -printf 'settings pages contract: PASS\n' +production_pids="$(cut -d'|' -f1 <<<"$production_before" | paste -sd, -)" +[[ -n "$production_pids" ]] || production_pids="none" +printf 'settings pages contract: PASS (production PIDs preserved: %s)\n' "$production_pids"