diff --git a/docs/superpowers/evidence/phone-focus-adb-common-split-2026-08-17.json b/docs/superpowers/evidence/phone-focus-adb-common-split-2026-08-17.json new file mode 100644 index 00000000..49a7a8d8 --- /dev/null +++ b/docs/superpowers/evidence/phone-focus-adb-common-split-2026-08-17.json @@ -0,0 +1,56 @@ +{ + "intent": "Bring phone_focus_mode/lib/adb_common.sh (346 lines) under the repo-wide 250-line cap without changing the API any consumer gets from one source line.", + "scope": [ + "phone_focus_mode/lib/adb_common.sh — reduced to logging, device selection, root and identity helpers, plus two source lines", + "phone_focus_mode/lib/adb_trusted.sh — new: the enrolled-device record (save, list, forget, match, verify)", + "phone_focus_mode/lib/adb_locking.sh — new: the run lock and per-marker cooldowns", + "Non-goal: the other over-cap phone_focus_mode shell files, whose tests are source-text greps rather than behavioural (see risks)" + ], + "changes": [ + "Split the two coherent, test-covered groups into siblings that adb_common.sh sources at the end of its own body, so every existing consumer keeps getting the complete API from its single existing `source .../lib/adb_common.sh` line. No consumer was edited.", + "Both new files are sourced via $(cd $(dirname ${BASH_SOURCE[0]}) && pwd), so they resolve relative to the library's own directory rather than the caller's — the failure mode the brief documents for install_leechblock.sh, where an entry whose SCRIPT_DIR has no lib/ dies on its first source. Nothing deploys a copy of this library elsewhere, so that blocker does not apply here.", + "Changed _load_trusted_device_values to take the record path as an optional first argument instead of reading a global that _matches_trusted_record shadowed with a `local`. Once the two functions lived in different files, shellcheck could no longer see that link and reported SC2034. Passing the path explicitly removes the cause rather than suppressing the finding, and is what the caller meant anyway; the default keeps the legacy single-device record working.", + "The two `# shellcheck source=` directives are written relative to the script's own directory, matching the repo's source-path=SCRIPTDIR setting in .shellcheckrc." + ], + "verification": [ + { + "command": "bash phone_focus_mode/lib/tests/test_adb_common.sh", + "result": "pass", + "evidence": "Results: 16 passed, 0 failed — identical to the pre-split baseline. This suite sources the library, mocks adb, and asserts real behaviour including an injection payload reaching _sanitize_device_string literally, so it is a behavioural gate rather than a source-text grep." + }, + { + "command": "pre-commit run --files on all three files", + "result": "pass", + "evidence": "Every hook passes, including shellcheck, which this repo runs with NO --severity filter so even info-level findings fail. Zero suppressions were added: the SC2034 the split initially caused was fixed by changing the function signature, and the SC1091s by correcting the source directives." + }, + { + "command": "Sourcing adb_common.sh alone and checking declare -F for functions from all three files", + "result": "pass", + "evidence": "adb_list_serials (kept), adb_save_trusted_device and adb_verify_trusted_identity (moved to adb_trusted.sh), adb_acquire_lock and adb_check_cooldown (moved to adb_locking.sh) are all defined. This is the check that proves the split is transparent to consumers rather than merely syntactically valid." + }, + { + "command": "bash -n on run_phone.sh, phone_backup.sh, distraction_purge.sh", + "result": "pass", + "evidence": "All three real consumers still parse." + }, + { + "command": "Function coverage measurement, run before choosing the seam", + "result": "pass", + "evidence": "13 of 23 functions (56%) in adb_common.sh are named by the test file. Both extracted groups sit inside the covered set — all four locking functions and five of six trusted-device functions are exercised — so the 16 tests genuinely verify the moved code rather than passing around it." + }, + { + "command": "bash ~/utils/scripts/check_file_length.sh --all", + "result": "pass", + "evidence": "adb_common.sh 155, adb_trusted.sh 136, adb_locking.sh 76 — none appears in the over-cap listing, down from a single 346-line file." + } + ], + "risks": [ + "adb_trusted.sh and adb_locking.sh are not independently sourceable: both read globals and call logging helpers that adb_common.sh defines first. Each carries a header comment saying so. Sourcing one directly would fail under set -u rather than misbehave silently.", + "The _load_trusted_device_values signature change is the one behavioural edit in this commit. Its only in-repo caller now passes the path explicitly; the default preserves the previous global-reading behaviour for any caller that does not.", + "The other over-cap files in this package (focus_ctl.sh 1091, deploy.sh 833, config.sh 571, focus_daemon.sh 543, monitor.sh 449) are NOT split here. test_dns_enforcer.sh, test_monitor.sh and test_tether_enforcer.sh contain zero assertions of the form this suite uses, so those files have no behavioural gate — splitting them would be unverifiable in exactly the way the brief warns about." + ], + "rollback": [ + "git revert the commit — one edited file and two added files, with no data, config or schema migration.", + "After rollback, re-run bash phone_focus_mode/lib/tests/test_adb_common.sh; expect 16 passed, 0 failed." + ] +} diff --git a/phone_focus_mode/lib/adb_common.sh b/phone_focus_mode/lib/adb_common.sh index 0a56c4e8..4ec690de 100755 --- a/phone_focus_mode/lib/adb_common.sh +++ b/phone_focus_mode/lib/adb_common.sh @@ -46,8 +46,13 @@ adb_list_serials() { awk 'NR > 1 && $2 ~ /^(device|offline|unauthorized)$/ { print $1 }' } +# Reads one enrolled-device record into TRUSTED_*_LOADED. +# Takes the path explicitly, defaulting to the legacy single-device record. +# Callers that need a specific record pass it rather than shadowing the global, +# which kept the value from leaking into later calls without relying on a +# `local` of a name declared in another file. _load_trusted_device_values() { - local file_path="${TRUSTED_DEVICE_FILE}" + local file_path="${1:-${TRUSTED_DEVICE_FILE}}" local -a loaded_values=() [[ -f "${file_path}" ]] || return 1 @@ -144,203 +149,12 @@ adb_collect_identity() { export DEVICE_MODEL DEVICE_FINGERPRINT DEVICE_SERIAL } -ADB_ENROLLED_DEVICE_FILE="" +# The enrolled-device record. Sourced from this file's own directory so every +# existing consumer keeps getting the whole API from one source line. +# shellcheck source=adb_trusted.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/adb_trusted.sh" -# Path of the per-device record for a serial. Serials are sanitised to -# [A-Za-z0-9._-] by _sanitize_device_string, so this cannot escape the dir. -_trusted_device_path() { - printf '%s/%s.sh' "${TRUSTED_DEVICE_DIR}" "$1" -} - -adb_save_trusted_device() { - local record="" - - adb_collect_identity - mkdir -p "${TRUSTED_DEVICE_DIR}" - chmod 700 "${TRUSTED_DEVICE_DIR}" - record="$(_trusted_device_path "${DEVICE_SERIAL}")" - - { - printf '# Auto-generated trusted device record — do not edit manually.\n' - printf 'TRUSTED_SERIAL=%q\n' "${DEVICE_SERIAL}" - printf 'TRUSTED_MODEL=%q\n' "${DEVICE_MODEL}" - printf 'TRUSTED_FINGERPRINT=%q\n' "${DEVICE_FINGERPRINT}" - } >"${record}" - - chmod 600 "${record}" - ADB_ENROLLED_DEVICE_FILE="${record}" - export ADB_ENROLLED_DEVICE_FILE - _info "Enrolled device: model='${DEVICE_MODEL}' serial='${DEVICE_SERIAL}'" -} - -# Serials with a per-device record, newest first is not meaningful here so -# the order is whatever the glob yields. -adb_list_trusted_serials() { - local record="" - [[ -d "${TRUSTED_DEVICE_DIR}" ]] || return 0 - for record in "${TRUSTED_DEVICE_DIR}"/*.sh; do - [[ -e "${record}" ]] || continue - basename "${record}" .sh - done -} - -adb_forget_trusted_device() { - local serial="${1:?serial required}" - local record="" - record="$(_trusted_device_path "$(_sanitize_device_string "${serial}")")" - if [[ -f "${record}" ]]; then - rm -f "${record}" - _info "Removed trusted device record for '${serial}'" - else - _warn "No trusted device record for '${serial}'" - fi -} - -# Compares the connected device against one record. Returns 0 on a match, -# 1 when the record is for a different device, and 2 when the record names -# this serial but its model or fingerprint has changed -- which is the case -# worth shouting about, since a serial is the easiest field to spoof and a -# changed fingerprint means the device was reflashed. -_matches_trusted_record() { - local record="${1:?record required}" - local saved_serial="" saved_model="" saved_fingerprint="" - # Scoped to this call: _load_trusted_device_values reads the global, and - # leaking a per-record value would change which file later callers use. - local TRUSTED_DEVICE_FILE="${record}" - - if ! _load_trusted_device_values; then - _fatal "Trusted device record exists but could not be read: ${record}" - fi - - saved_serial="${TRUSTED_SERIAL_LOADED:-}" - saved_model="${TRUSTED_MODEL_LOADED:-}" - saved_fingerprint="${TRUSTED_FINGERPRINT_LOADED:-}" - - [[ -n "${saved_serial}" && "${DEVICE_SERIAL}" != "${saved_serial}" ]] && return 1 - - if [[ -n "${saved_model}" && "${DEVICE_MODEL}" != "${saved_model}" ]]; then - _error "Device '${DEVICE_SERIAL}' is enrolled but its model changed: expected '${saved_model}', got '${DEVICE_MODEL}'." - return 2 - fi - - if [[ -n "${saved_fingerprint}" && "${DEVICE_FINGERPRINT}" != "${saved_fingerprint}" ]]; then - _error "Device '${DEVICE_SERIAL}' is enrolled but its build fingerprint changed." - _error " expected: ${saved_fingerprint}" - _error " got: ${DEVICE_FINGERPRINT}" - return 2 - fi - - return 0 -} - -# Fails closed: an unknown device aborts. Any ENROLLED device is accepted, -# so several phones can be managed from one PC without re-enrolling on -# every switch. Only a completely empty store is permissive, and that is -# the first-run case where there is nothing yet to contradict. -adb_verify_trusted_identity() { - local record="" rc=0 - local -a records=() - - if [[ -d "${TRUSTED_DEVICE_DIR}" ]]; then - for record in "${TRUSTED_DEVICE_DIR}"/*.sh; do - [[ -e "${record}" ]] && records+=("${record}") - done - fi - # Legacy single-device record stays authoritative until it is migrated, - # so an existing install keeps its guarantee without any manual step. - [[ -f "${_PHONE_STATE_DIR}/trusted_device.sh" ]] && - records+=("${_PHONE_STATE_DIR}/trusted_device.sh") - - if [[ ${#records[@]} -eq 0 ]]; then - _warn "No trusted device record found. Run 'fresh-phone' to enroll this device." - return 0 - fi - - adb_collect_identity - - for record in "${records[@]}"; do - _matches_trusted_record "${record}" && { - _info "Device identity verified: ${DEVICE_SERIAL} (${DEVICE_MODEL})" - return 0 - } - rc=$? - # A tampered/reflashed match must abort rather than fall through to - # the next record, which would silently downgrade the check. - [[ "${rc}" -eq 2 ]] && _fatal "Refusing to proceed automatically." - done - - _error "Device '${DEVICE_SERIAL}' (${DEVICE_MODEL}) is not enrolled." - _error "Enrolled: $(adb_list_trusted_serials | tr '\n' ' ')" - _fatal "Refusing to proceed automatically. Enroll it first if this is expected." -} - -_adb_release_lock() { - if [[ -n "${LOCK_FILE}" && -d "${LOCK_FILE}" ]]; then - rm -rf "${LOCK_FILE}" - fi -} - -adb_acquire_lock() { - local lock_pid_file="" - local old_pid="" - - mkdir -p "${LOCK_DIR}" - LOCK_FILE="${LOCK_DIR}/run_phone.lock" - lock_pid_file="${LOCK_FILE}/pid" - - if mkdir "${LOCK_FILE}" 2>/dev/null; then - printf '%s\n' "$$" >"${lock_pid_file}" - trap _adb_release_lock EXIT INT TERM - _info "Acquired run lock (PID $$)" - return 0 - fi - - old_pid="$(cat "${lock_pid_file}" 2>/dev/null || printf '')" - if [[ -n "${old_pid}" ]] && kill -0 "${old_pid}" 2>/dev/null; then - _fatal "Another run_phone.sh instance is already running (PID ${old_pid}). Aborting." - fi - - _warn "Stale lock directory found (PID ${old_pid} no longer running). Removing." - rm -rf "${LOCK_FILE}" - - if ! mkdir "${LOCK_FILE}" 2>/dev/null; then - _fatal "Could not acquire run lock at ${LOCK_FILE}. Another process may have raced us." - fi - - printf '%s\n' "$$" >"${lock_pid_file}" - trap _adb_release_lock EXIT INT TERM - _info "Acquired run lock (PID $$)" -} - -adb_check_cooldown() { - local cooldown_secs="${1:-300}" - local elapsed=0 - local last_run=0 - local marker_name="${2:-default}" - local marker="${LAST_RUN_DIR}/${marker_name}" - local now=0 - - mkdir -p "${LAST_RUN_DIR}" - if [[ -f "${marker}" ]]; then - last_run="$(cat "${marker}")" - if [[ "${last_run}" =~ ^[0-9]+$ ]]; then - now="$(date +%s)" - elapsed=$((now - last_run)) - if ((elapsed < cooldown_secs)); then - _info "Cooldown active: last run ${elapsed}s ago, cooldown is ${cooldown_secs}s. Skipping." - return 1 - fi - else - _warn "Ignoring invalid cooldown marker: ${marker}" - fi - fi - - return 0 -} - -adb_mark_last_run() { - local marker_name="${1:-default}" - - mkdir -p "${LAST_RUN_DIR}" - date +%s >"${LAST_RUN_DIR}/${marker_name}" -} +# Run lock and cooldown helpers. Sourced last, from this file's own directory, +# so every existing consumer keeps getting the whole API from one source line. +# shellcheck source=adb_locking.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/adb_locking.sh" diff --git a/phone_focus_mode/lib/adb_locking.sh b/phone_focus_mode/lib/adb_locking.sh new file mode 100755 index 00000000..ccbce6e0 --- /dev/null +++ b/phone_focus_mode/lib/adb_locking.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# lib/adb_locking.sh — single-instance run lock and per-marker cooldowns. +# Sourced by adb_common.sh; do not execute directly, and do not source it +# on its own: it reads LOCK_DIR, LAST_RUN_DIR and LOCK_FILE, and calls the +# _info/_warn/_fatal helpers, all of which adb_common.sh defines first. + +_adb_release_lock() { + if [[ -n "${LOCK_FILE}" && -d "${LOCK_FILE}" ]]; then + rm -rf "${LOCK_FILE}" + fi +} + +adb_acquire_lock() { + local lock_pid_file="" + local old_pid="" + + mkdir -p "${LOCK_DIR}" + LOCK_FILE="${LOCK_DIR}/run_phone.lock" + lock_pid_file="${LOCK_FILE}/pid" + + if mkdir "${LOCK_FILE}" 2>/dev/null; then + printf '%s\n' "$$" >"${lock_pid_file}" + trap _adb_release_lock EXIT INT TERM + _info "Acquired run lock (PID $$)" + return 0 + fi + + old_pid="$(cat "${lock_pid_file}" 2>/dev/null || printf '')" + if [[ -n "${old_pid}" ]] && kill -0 "${old_pid}" 2>/dev/null; then + _fatal "Another run_phone.sh instance is already running (PID ${old_pid}). Aborting." + fi + + _warn "Stale lock directory found (PID ${old_pid} no longer running). Removing." + rm -rf "${LOCK_FILE}" + + if ! mkdir "${LOCK_FILE}" 2>/dev/null; then + _fatal "Could not acquire run lock at ${LOCK_FILE}. Another process may have raced us." + fi + + printf '%s\n' "$$" >"${lock_pid_file}" + trap _adb_release_lock EXIT INT TERM + _info "Acquired run lock (PID $$)" +} + +adb_check_cooldown() { + local cooldown_secs="${1:-300}" + local elapsed=0 + local last_run=0 + local marker_name="${2:-default}" + local marker="${LAST_RUN_DIR}/${marker_name}" + local now=0 + + mkdir -p "${LAST_RUN_DIR}" + if [[ -f "${marker}" ]]; then + last_run="$(cat "${marker}")" + if [[ "${last_run}" =~ ^[0-9]+$ ]]; then + now="$(date +%s)" + elapsed=$((now - last_run)) + if ((elapsed < cooldown_secs)); then + _info "Cooldown active: last run ${elapsed}s ago, cooldown is ${cooldown_secs}s. Skipping." + return 1 + fi + else + _warn "Ignoring invalid cooldown marker: ${marker}" + fi + fi + + return 0 +} + +adb_mark_last_run() { + local marker_name="${1:-default}" + + mkdir -p "${LAST_RUN_DIR}" + date +%s >"${LAST_RUN_DIR}/${marker_name}" +} diff --git a/phone_focus_mode/lib/adb_trusted.sh b/phone_focus_mode/lib/adb_trusted.sh new file mode 100755 index 00000000..a646c0cd --- /dev/null +++ b/phone_focus_mode/lib/adb_trusted.sh @@ -0,0 +1,135 @@ +#!/usr/bin/env bash +# lib/adb_trusted.sh — the enrolled-device record: saving, listing, forgetting, +# and refusing to act on a phone that is not enrolled. +# Sourced by adb_common.sh; do not execute directly, and do not source it on +# its own: it uses TRUSTED_DEVICE_DIR/TRUSTED_DEVICE_FILE, the DEVICE_* vars and +# the _info/_warn/_error/_fatal helpers that adb_common.sh defines first. + +ADB_ENROLLED_DEVICE_FILE="" + +# Path of the per-device record for a serial. Serials are sanitised to +# [A-Za-z0-9._-] by _sanitize_device_string, so this cannot escape the dir. +_trusted_device_path() { + printf '%s/%s.sh' "${TRUSTED_DEVICE_DIR}" "$1" +} + +adb_save_trusted_device() { + local record="" + + adb_collect_identity + mkdir -p "${TRUSTED_DEVICE_DIR}" + chmod 700 "${TRUSTED_DEVICE_DIR}" + record="$(_trusted_device_path "${DEVICE_SERIAL}")" + + { + printf '# Auto-generated trusted device record — do not edit manually.\n' + printf 'TRUSTED_SERIAL=%q\n' "${DEVICE_SERIAL}" + printf 'TRUSTED_MODEL=%q\n' "${DEVICE_MODEL}" + printf 'TRUSTED_FINGERPRINT=%q\n' "${DEVICE_FINGERPRINT}" + } >"${record}" + + chmod 600 "${record}" + ADB_ENROLLED_DEVICE_FILE="${record}" + export ADB_ENROLLED_DEVICE_FILE + _info "Enrolled device: model='${DEVICE_MODEL}' serial='${DEVICE_SERIAL}'" +} + +# Serials with a per-device record, newest first is not meaningful here so +# the order is whatever the glob yields. +adb_list_trusted_serials() { + local record="" + [[ -d "${TRUSTED_DEVICE_DIR}" ]] || return 0 + for record in "${TRUSTED_DEVICE_DIR}"/*.sh; do + [[ -e "${record}" ]] || continue + basename "${record}" .sh + done +} + +adb_forget_trusted_device() { + local serial="${1:?serial required}" + local record="" + record="$(_trusted_device_path "$(_sanitize_device_string "${serial}")")" + if [[ -f "${record}" ]]; then + rm -f "${record}" + _info "Removed trusted device record for '${serial}'" + else + _warn "No trusted device record for '${serial}'" + fi +} + +# Compares the connected device against one record. Returns 0 on a match, +# 1 when the record is for a different device, and 2 when the record names +# this serial but its model or fingerprint has changed -- which is the case +# worth shouting about, since a serial is the easiest field to spoof and a +# changed fingerprint means the device was reflashed. +_matches_trusted_record() { + local record="${1:?record required}" + local saved_serial="" saved_model="" saved_fingerprint="" + + # Passed explicitly rather than shadowing the global, so a per-record value + # cannot change which file later callers read. + if ! _load_trusted_device_values "${record}"; then + _fatal "Trusted device record exists but could not be read: ${record}" + fi + + saved_serial="${TRUSTED_SERIAL_LOADED:-}" + saved_model="${TRUSTED_MODEL_LOADED:-}" + saved_fingerprint="${TRUSTED_FINGERPRINT_LOADED:-}" + + [[ -n "${saved_serial}" && "${DEVICE_SERIAL}" != "${saved_serial}" ]] && return 1 + + if [[ -n "${saved_model}" && "${DEVICE_MODEL}" != "${saved_model}" ]]; then + _error "Device '${DEVICE_SERIAL}' is enrolled but its model changed: expected '${saved_model}', got '${DEVICE_MODEL}'." + return 2 + fi + + if [[ -n "${saved_fingerprint}" && "${DEVICE_FINGERPRINT}" != "${saved_fingerprint}" ]]; then + _error "Device '${DEVICE_SERIAL}' is enrolled but its build fingerprint changed." + _error " expected: ${saved_fingerprint}" + _error " got: ${DEVICE_FINGERPRINT}" + return 2 + fi + + return 0 +} + +# Fails closed: an unknown device aborts. Any ENROLLED device is accepted, +# so several phones can be managed from one PC without re-enrolling on +# every switch. Only a completely empty store is permissive, and that is +# the first-run case where there is nothing yet to contradict. +adb_verify_trusted_identity() { + local record="" rc=0 + local -a records=() + + if [[ -d "${TRUSTED_DEVICE_DIR}" ]]; then + for record in "${TRUSTED_DEVICE_DIR}"/*.sh; do + [[ -e "${record}" ]] && records+=("${record}") + done + fi + # Legacy single-device record stays authoritative until it is migrated, + # so an existing install keeps its guarantee without any manual step. + [[ -f "${_PHONE_STATE_DIR}/trusted_device.sh" ]] && + records+=("${_PHONE_STATE_DIR}/trusted_device.sh") + + if [[ ${#records[@]} -eq 0 ]]; then + _warn "No trusted device record found. Run 'fresh-phone' to enroll this device." + return 0 + fi + + adb_collect_identity + + for record in "${records[@]}"; do + _matches_trusted_record "${record}" && { + _info "Device identity verified: ${DEVICE_SERIAL} (${DEVICE_MODEL})" + return 0 + } + rc=$? + # A tampered/reflashed match must abort rather than fall through to + # the next record, which would silently downgrade the check. + [[ "${rc}" -eq 2 ]] && _fatal "Refusing to proceed automatically." + done + + _error "Device '${DEVICE_SERIAL}' (${DEVICE_MODEL}) is not enrolled." + _error "Enrolled: $(adb_list_trusted_serials | tr '\n' ' ')" + _fatal "Refusing to proceed automatically. Enroll it first if this is expected." +}