ZacharyZhang-NY/omarchy
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Harden the polkit migration: exact-layout match, retry on failure, and tests

Browse Source 
Address review feedback on the polkit faillock migration:

- Match only the exact stack the setup commands wrote and replace just the bare
  pam_unix lines, preserving comments and the hardware-auth lines. An
  administrator-authored polkit-1 carrying any other directive is left
  untouched, rather than rebuilt from scratch.

- Also repair the markerless post-removal layout: both remove commands strip
  their own marker lines but leave the bare pam_unix stack behind, so keying on
  a hardware-auth marker skipped those machines permanently.

- Exit non-zero when the backup cannot be created or the rewrite cannot be
  verified (after restoring). omarchy-migrate runs under set -e and records a
  migration complete unconditionally after it returns, so a failed repair must
  fail loudly to be retried instead of silently marked done.

- Add test/shell.d/security-polkit-migration-test.sh covering the fingerprint,
  FIDO2, combined, and markerless layouts, comment preservation, idempotence, an
  untouched administrator stack, and the refused-sudo and unverifiable-write
  failure paths.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
rattatuiandClaude Opus 4.8 committed 2026-09-03 09:18:18 +03:00
1 parent 8a13eac872
commit 21e7975352
2 files changed
+232 -32
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

migrations/1788256455.sh
+59 -32
View File
@@ -10,43 +10,70 @@ polkit=/etc/pam.d/polkit-1
# failures never counted toward the shared tally. The setup commands now defer to
# system-auth; this repairs the files the old ones already wrote.
#
# In scope only when the file exists, no package owns it (Omarchy wrote it), it
# does not already defer to system-auth, and it carries an Omarchy hardware-auth
# marker (the clamshell gate, pam_fprintd, or the FIDO2 authfile). That pins the
# repair to the exact stack the setup commands produced and leaves an
# administrator's own polkit-1 untouched. Idempotent by construction: once
# system-auth is included, this run and every other user's run no-op.
if [[ -f $polkit ]] &&
! pacman -Qo "$polkit" &>/dev/null &&
! grep -qE '^auth[[:space:]]+include[[:space:]]+system-auth' "$polkit" &&
grep -qE 'omarchy-hw-laptop-closed|pam_fprintd\.so|authfile=/etc/fido2/fido2' "$polkit"; then
# Only the exact stack those commands produced is touched. Every non-blank,
# non-comment line must be one of the known hardware-auth auth lines (the
# clamshell gate, pam_fprintd, or the FIDO2 pam_u2f line) or a bare
# `X required pam_unix.so`; all four bare pam_unix lines must be present; and
# system-auth must not already be included. This matches the fingerprint, FIDO2,
# combined, and post-removal (markerless, both remove commands strip only their
# own marker lines) layouts, and refuses any administrator-authored stack that
# carries other directives. The repair replaces only the bare pam_unix lines, so
# comments and the hardware-auth lines are preserved verbatim.
is_omarchy_vulnerable_stack() {
local file=$1 line
local re_gate='^auth[[:space:]]+\[success=1 default=ignore\][[:space:]]+pam_exec\.so quiet /usr/bin/omarchy-hw-laptop-closed[[:space:]]*$'
local re_fprintd='^auth[[:space:]]+sufficient[[:space:]]+pam_fprintd\.so[[:space:]]*$'
local re_u2f='^auth[[:space:]]+sufficient[[:space:]]+pam_u2f\.so cue authfile=/etc/fido2/fido2[[:space:]]*$'
local re_bare='^(auth|account|password|session)[[:space:]]+required[[:space:]]+pam_unix\.so[[:space:]]*$'
local phase
# Already fixed, or partially converted: leave it alone (also makes reruns no-op).
if grep -qE '(auth|account|password|session)[[:space:]]+include[[:space:]]+system-auth' "$file"; then
return 1
fi
# The vulnerable signature: all four phases delegated to a bare pam_unix.
for phase in auth account password session; do
grep -qE "^${phase}[[:space:]]+required[[:space:]]+pam_unix\.so[[:space:]]*\$" "$file" || return 1
done
# Every meaningful line must be one Omarchy itself wrote; anything else means
# an administrator has edited this file, so it is not ours to rewrite.
while IFS= read -r line || [[ -n $line ]]; do
[[ -z ${line//[[:space:]]/} ]] && continue
[[ $line == \#* ]] && continue
[[ $line =~ $re_gate || $line =~ $re_fprintd || $line =~ $re_u2f || $line =~ $re_bare ]] && continue
return 1
done <"$file"
return 0
}
if [[ -f $polkit ]] && is_omarchy_vulnerable_stack "$polkit"; then
echo "Rewriting $polkit to defer to system-auth (restores pam_faillock lockout)..."
# Keep the configured hardware-auth auth lines verbatim (the clamshell gate and
# the pam_fprintd / pam_u2f 'sufficient' lines); only the bare pam_unix stack is
# replaced with the system-auth includes the vendor file and the sudo stack use.
hw_auth=$(grep -E '^auth' "$polkit" | grep -vE 'pam_unix\.so')
rebuilt=$(
[[ -n $hw_auth ]] && printf '%s\n' "$hw_auth"
printf 'auth include system-auth\n'
printf 'account include system-auth\n'
printf 'password include system-auth\n'
printf 'session include system-auth\n'
)
backup="$polkit.omarchy-bak.$(date +%s)"
if sudo cp -a "$polkit" "$backup"; then
printf '%s\n' "$rebuilt" | sudo tee "$polkit" >/dev/null
if ! sudo cp -a "$polkit" "$backup"; then
echo "Could not back up $polkit; leaving it unchanged so the migration retries." >&2
exit 1
fi
if grep -qE '^auth[[:space:]]+include[[:space:]]+system-auth' "$polkit"; then
echo "Restored polkit brute-force protection. Previous file saved at $backup."
else
echo "polkit repair could not be verified; restoring the original file." >&2
sudo cp -a "$backup" "$polkit"
fi
# Replace only the bare pam_unix lines; keep comments, blank lines, and the
# hardware-auth (gate / pam_fprintd / pam_u2f) lines exactly as they are.
rebuilt=$(sed -E 's/^(auth|account|password|session)([[:space:]]+)required[[:space:]]+pam_unix\.so[[:space:]]*$/\1\2include system-auth/' "$polkit")
if ! printf '%s\n' "$rebuilt" | sudo tee "$polkit" >/dev/null; then
echo "Could not write $polkit; restoring the original." >&2
sudo cp -a "$backup" "$polkit" || true
exit 1
fi
if grep -qE '^auth[[:space:]]+include[[:space:]]+system-auth' "$polkit"; then
echo "Restored polkit brute-force protection. Previous file saved at $backup."
else
echo "Administrator privileges are required to repair $polkit. Run omarchy-migrate again from a terminal." >&2
echo "polkit repair could not be verified; restoring the original file." >&2
sudo cp -a "$backup" "$polkit"
exit 1
fi
fi
test/shell.d/security-polkit-migration-test.sh
+173
View File
@@ -0,0 +1,173 @@
#!/bin/bash
set -euo pipefail
source "$(dirname "$0")/base-test.sh"
# Exercises migrations/1788256455.sh, which repairs an Omarchy-created
# /etc/pam.d/polkit-1 that lists pam_unix directly (dropping pam_faillock)
# instead of including system-auth. The migration keeps its production path
# fixed; as in sshd-hardening-migration-test.sh, this test rewrites that one
# assignment in the input fed to bash and stubs sudo so nothing touches the
# host's /etc.
test_dir=$(mktemp -d)
trap 'rm -rf "$test_dir"' EXIT
migration="$ROOT/migrations/1788256455.sh"
stub_bin="$test_dir/bin"
mkdir -p "$stub_bin"
# sudo stub: log, optionally refuse (SUDO_ALLOWED=0), and optionally make a
# `tee` write vanish (WRITE_BREAKS=1) so the verification/restore path is
# exercised. Otherwise run the real command so cp/tee act on the temp file.
cat >"$stub_bin/sudo" <<'STUB'
#!/bin/bash
printf 'sudo %s\n' "$*" >>"${CALL_LOG:?}"
if [[ ${SUDO_ALLOWED:-1} != 1 ]]; then
exit 1
fi
if [[ ${WRITE_BREAKS:-0} == 1 && $1 == tee ]]; then
cat >/dev/null
exit 0
fi
exec "$@"
STUB
chmod +x "$stub_bin"/*
# Run the migration against a polkit-1 file seeded with $1; leaves the result in
# "$test_dir/<scenario>/polkit-1" and records the exit status in migrate_rc.
migrate_rc=0
run_migration() {
local scenario=$1 content=$2
local dir="$test_dir/$scenario"
local polkit="$dir/polkit-1"
mkdir -p "$dir"
printf '%s' "$content" >"$polkit"
: >"$test_dir/$scenario.calls"
migrate_rc=0
sed "s|^polkit=/etc/pam.d/polkit-1\$|polkit=$polkit|" "$migration" |
CALL_LOG="$test_dir/$scenario.calls" PATH="$stub_bin:$PATH" \
SUDO_ALLOWED="${SUDO_ALLOWED:-1}" WRITE_BREAKS="${WRITE_BREAKS:-0}" \
bash -euo pipefail >/dev/null 2>&1 || migrate_rc=$?
}
result() { cat "$test_dir/$1/polkit-1"; }
# The four layouts the old setup / remove commands leave behind.
fingerprint_stack='auth [success=1 default=ignore] pam_exec.so quiet /usr/bin/omarchy-hw-laptop-closed
auth sufficient pam_fprintd.so
auth required pam_unix.so
account required pam_unix.so
password required pam_unix.so
session required pam_unix.so
'
fido2_stack='auth sufficient pam_u2f.so cue authfile=/etc/fido2/fido2
auth required pam_unix.so
account required pam_unix.so
password required pam_unix.so
session required pam_unix.so
'
both_stack='auth [success=1 default=ignore] pam_exec.so quiet /usr/bin/omarchy-hw-laptop-closed
auth sufficient pam_fprintd.so
auth sufficient pam_u2f.so cue authfile=/etc/fido2/fido2
auth required pam_unix.so
account required pam_unix.so
password required pam_unix.so
session required pam_unix.so
'
# What both remove commands leave: their own marker lines stripped, the bare
# pam_unix stack (and no marker) behind.
markerless_stack='auth required pam_unix.so
account required pam_unix.so
password required pam_unix.so
session required pam_unix.so
'
fixed_stack='auth sufficient pam_fprintd.so
auth include system-auth
account include system-auth
password include system-auth
session include system-auth
'
# An administrator's own stack that happens to use pam_fprintd but carries an
# extra directive Omarchy never writes.
admin_stack='auth sufficient pam_fprintd.so
auth required pam_unix.so
auth optional pam_permit.so
account required pam_unix.so
password required pam_unix.so
session required pam_unix.so
'
# Every phase of a repaired stack must defer to system-auth and no bare pam_unix
# may remain in the account/password/session block.
assert_repaired() {
local scenario=$1 phase
for phase in auth account password session; do
grep -qE "^${phase}[[:space:]]+include[[:space:]]+system-auth" <<<"$(result "$scenario")" ||
fail "$scenario: $phase defers to system-auth" "$(result "$scenario")"
done
! grep -qE '^(account|password|session)[[:space:]]+required[[:space:]]+pam_unix' <<<"$(result "$scenario")" ||
fail "$scenario: no bare pam_unix remains" "$(result "$scenario")"
}
run_migration fingerprint "$fingerprint_stack"
(( migrate_rc == 0 )) || fail "fingerprint stack migrates cleanly"
assert_repaired fingerprint
grep -qF 'pam_fprintd.so' <<<"$(result fingerprint)" || fail "fingerprint line is preserved"
grep -qF 'omarchy-hw-laptop-closed' <<<"$(result fingerprint)" || fail "clamshell gate is preserved"
pass "migration repairs the fingerprint stack and keeps its hardware-auth lines"
run_migration fido2 "$fido2_stack"
(( migrate_rc == 0 )) || fail "fido2 stack migrates cleanly"
assert_repaired fido2
grep -qF 'pam_u2f.so cue authfile=/etc/fido2/fido2' <<<"$(result fido2)" || fail "FIDO2 line is preserved"
pass "migration repairs the FIDO2 stack and keeps its hardware-auth line"
run_migration both "$both_stack"
(( migrate_rc == 0 )) || fail "combined stack migrates cleanly"
assert_repaired both
grep -qF 'pam_fprintd.so' <<<"$(result both)" && grep -qF 'pam_u2f.so' <<<"$(result both)" ||
fail "both hardware-auth lines are preserved"
pass "migration repairs a combined fingerprint+FIDO2 stack"
run_migration markerless "$markerless_stack"
(( migrate_rc == 0 )) || fail "markerless stack migrates cleanly"
assert_repaired markerless
pass "migration repairs the markerless post-removal stack"
run_migration comment "# managed by omarchy
$fingerprint_stack"
(( migrate_rc == 0 )) || fail "commented stack migrates cleanly"
assert_repaired comment
grep -qxF '# managed by omarchy' <<<"$(result comment)" || fail "comments are preserved through the rewrite"
pass "migration repairs a commented stack and preserves the comment"
run_migration fixed "$fixed_stack"
[[ "$(result fixed)" == "$(printf '%s' "$fixed_stack")" ]] || fail "an already-fixed stack is left byte-for-byte unchanged"
! grep -q '^sudo ' "$test_dir/fixed.calls" || fail "an already-fixed stack triggers no privileged writes"
pass "migration is idempotent: an already-fixed stack is untouched"
run_migration admin "$admin_stack"
[[ "$(result admin)" == "$(printf '%s' "$admin_stack")" ]] || fail "an administrator-authored stack is left unchanged"
! grep -q '^sudo ' "$test_dir/admin.calls" || fail "an administrator-authored stack triggers no privileged writes"
pass "migration refuses a stack carrying non-Omarchy directives"
# Privilege failure is the retryable case: the migration must exit non-zero so
# omarchy-migrate does not record it complete, and must leave the file unchanged.
SUDO_ALLOWED=0 run_migration no-sudo "$fingerprint_stack"
(( migrate_rc != 0 )) || fail "the migration stays pending when privileges are unavailable"
[[ "$(result no-sudo)" == "$(printf '%s' "$fingerprint_stack")" ]] || fail "a failed repair leaves the original file intact"
pass "migration exits non-zero and preserves the file when sudo is refused"
# A write that does not take effect must be caught by verification, restored,
# and reported as a failure rather than silently marked complete.
WRITE_BREAKS=1 run_migration verify-fail "$fingerprint_stack"
(( migrate_rc != 0 )) || fail "a failed verification exits non-zero"
[[ "$(result verify-fail)" == "$(printf '%s' "$fingerprint_stack")" ]] || fail "a failed verification restores the original file"
pass "migration exits non-zero and restores when the write cannot be verified"