ZacharyZhang-NY/omarchycn
2
2
Fork 0
Code Issues Pull Requests 2 Actions Packages Projects Releases 3 Wiki Activity

Never leave updatedb.conf with two definitions of the same setting

Browse Source 
updatedb refuses to run at all on a config that defines a variable twice
("variable `PRUNEPATHS' was already defined"), so any rewrite that misses
an existing line and appends a second one takes the locate index down
rather than fixing it. Two shapes updatedb accepts got missed: a trailing
comment after the value, and a setting indented by whitespace.

Read the existing paths out of the quoted value and write the whole
setting back canonically instead of splicing into a line of unknown
shape. Quotes are not optional to updatedb ("value in quotes expected
after `='"), so a bare value is already a broken config: rewriting it
quoted repairs the file as a side effect.

The tests now hand every rewritten file to the real parser through
`updatedb --config-file`, which is what caught this.

Read the Snapper config as root when the running user cannot read it.
snapper create-config leaves the config root-only, and a config the user
cannot read was passing for one that wants its timeline snapshots kept.

Report the snapshots the drain could not delete. omarchy-migrate writes
the completion marker whether or not the batches succeeded, so there is
no later run to pick up the remainder, whatever the comment claimed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
David Heinemeier Hansson
2026-07-26 19:19:33 -07:00
co-authored by Claude Opus 5
parent f22e8cd353
commit 9b9d4b39eb
4 changed files with 97 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
install/config/locate.sh
+15 -10
View File
@@ -4,24 +4,29 @@ echo "Configuring locate to skip Btrfs snapshots and index Btrfs subvolumes"
[[ -f $UPDATEDB_CONF_PATH ]] || exit 0
# updatedb refuses to run at all on a config that defines a variable twice, so
# every setting here is rewritten where it already stands and only appended
# when the file has no line for it.
# Btrfs subvolume mounts (like /home) look like bind mounts, so pruning
# bind mounts leaves them out of the index entirely.
if grep -qE '^PRUNE_BIND_MOUNTS[[:space:]]*=' "$UPDATEDB_CONF_PATH"; then
sed -i -E 's|^PRUNE_BIND_MOUNTS[[:space:]]*=.*|PRUNE_BIND_MOUNTS = "no"|' "$UPDATEDB_CONF_PATH"
if grep -qE '^[[:space:]]*PRUNE_BIND_MOUNTS[[:space:]]*=' "$UPDATEDB_CONF_PATH"; then
sed -i -E 's|^[[:space:]]*PRUNE_BIND_MOUNTS[[:space:]]*=.*|PRUNE_BIND_MOUNTS = "no"|' "$UPDATEDB_CONF_PATH"
else
printf '%s\n' 'PRUNE_BIND_MOUNTS = "no"' >>"$UPDATEDB_CONF_PATH"
fi
# Snapper snapshots are nested subvolumes reached by plain directory
# traversal, so without this updatedb indexes the system once per snapshot.
# The quotes are optional in updatedb.conf, so read the paths back out of
# whatever quoting the file uses and write the setting in one canonical form.
pruned=$(sed -nE 's|^PRUNEPATHS[[:space:]]*=[[:space:]]*"?([^"]*)"?[[:space:]]*$|\1|p' "$UPDATEDB_CONF_PATH" | tail -n 1)
if grep -qE '^[[:space:]]*PRUNEPATHS[[:space:]]*=' "$UPDATEDB_CONF_PATH"; then
# updatedb only accepts quoted values and allows a comment after them. Read
# back what the machine already prunes and write the whole setting out again
# rather than splicing into a line of unknown shape.
pruned=$(sed -nE 's|^[[:space:]]*PRUNEPATHS[[:space:]]*=[[:space:]]*"([^"]*)".*|\1|p' "$UPDATEDB_CONF_PATH" | tail -n 1)
if [[ " $pruned " != *" /.snapshots "* ]]; then
if [[ -n $pruned ]]; then
sed -i -E "s|^PRUNEPATHS[[:space:]]*=.*|PRUNEPATHS = \"/.snapshots $pruned\"|" "$UPDATEDB_CONF_PATH"
else
printf '%s\n' 'PRUNEPATHS = "/.snapshots"' >>"$UPDATEDB_CONF_PATH"
if [[ " $pruned " != *" /.snapshots "* ]]; then
sed -i -E "s|^[[:space:]]*PRUNEPATHS[[:space:]]*=.*|PRUNEPATHS = \"/.snapshots${pruned:+ $pruned}\"|" "$UPDATEDB_CONF_PATH"
fi
else
printf '%s\n' 'PRUNEPATHS = "/.snapshots"' >>"$UPDATEDB_CONF_PATH"
fi
migrations/1784809452.sh
+18 -5
View File
@@ -14,8 +14,14 @@ command -v snapper >/dev/null || exit 0
[[ -f $SNAPPER_CONFIG_PATH ]] || exit 0
# Only clean up when timeline snapshotting is off, as Omarchy configures it.
# Anyone who deliberately turned it back on keeps their snapshots.
grep -qFx 'TIMELINE_CREATE="no"' "$SNAPPER_CONFIG_PATH" || exit 0
# Anyone who deliberately turned it back on keeps their snapshots. Snapper's
# own create-config leaves the file readable by root alone, and a config this
# user cannot read must not pass for one that wants its snapshots kept.
if [[ -r $SNAPPER_CONFIG_PATH ]]; then
grep -qFx 'TIMELINE_CREATE="no"' "$SNAPPER_CONFIG_PATH" || exit 0
else
as_root grep -qFx 'TIMELINE_CREATE="no"' "$SNAPPER_CONFIG_PATH" || exit 0
fi
# Earlier installs ran hourly timeline snapshots. Later configs stopped
# creating them but never deleted the existing ones, and number cleanup
@@ -28,16 +34,23 @@ echo "Deleting $(wc -w <<<"$leaked") leaked timeline snapshots (disk space is re
# Delete in small batches; one big delete can die on a DBus timeout partway.
# A failed batch must not take the rest of the migration run down with it, so
# the drain is best effort: whatever survives is picked up by the next run.
# the drain is best effort. omarchy-migrate records the migration either way,
# so say what is left rather than counting on a rerun that will not come.
failed=0
batch=()
for number in $leaked; do
batch+=("$number")
if (( ${#batch[@]} == 20 )); then
as_root snapper -c root delete "${batch[@]}" || true
as_root snapper -c root delete "${batch[@]}" || failed=$((failed + ${#batch[@]}))
batch=()
fi
done
if (( ${#batch[@]} > 0 )); then
as_root snapper -c root delete "${batch[@]}" || true
as_root snapper -c root delete "${batch[@]}" || failed=$((failed + ${#batch[@]}))
fi
if (( failed > 0 )); then
echo "$failed snapshots could not be deleted. Finish with: sudo snapper -c root delete <number>"
fi
test/shell.d/locate-test.sh
+38 -3
View File
@@ -18,6 +18,19 @@ PRUNEPATHS = "/afs /media /mnt /net /sfs /tmp /udev /var/cache /var/lib/pacman/l
CONF
}
# updatedb dies on a config that defines a variable twice, so hand every
# rewritten file to the real parser rather than trusting the greps below.
empty_tree="$test_tmp/empty-tree"
mkdir -p "$empty_tree"
assert_conf_parses() {
command -v updatedb >/dev/null || return 0
local errors
errors=$(updatedb --config-file "$1" -U "$empty_tree" -o "$test_tmp/plocate.db" 2>&1 >/dev/null | grep -F "$1:" || true)
[[ -z $errors ]] || fail "updatedb accepts the rewritten config" "$errors"
}
conf="$test_tmp/updatedb.conf"
stock_conf "$conf"
@@ -25,11 +38,13 @@ OMARCHY_UPDATEDB_CONF_PATH="$conf" bash -euo pipefail "$config_script" >/dev/nul
grep -qFx 'PRUNE_BIND_MOUNTS = "no"' "$conf" || fail "locate config indexes Btrfs subvolume mounts like /home"
grep -qF 'PRUNEPATHS = "/.snapshots /afs' "$conf" || fail "locate config prunes /.snapshots"
assert_conf_parses "$conf"
pass "locate config skips Btrfs snapshots and indexes Btrfs subvolumes"
OMARCHY_UPDATEDB_CONF_PATH="$conf" bash -euo pipefail "$config_script" >/dev/null
[[ $(grep -o '/\.snapshots' "$conf" | wc -l) -eq 1 ]] || fail "locate config is idempotent"
assert_conf_parses "$conf"
pass "locate config leaves an already-configured file alone"
OMARCHY_UPDATEDB_CONF_PATH="$test_tmp/missing.conf" bash -euo pipefail "$config_script" >/dev/null
@@ -44,6 +59,7 @@ OMARCHY_UPDATEDB_CONF_PATH="$conf" bash -euo pipefail "$config_script" >/dev/nul
grep -qFx 'PRUNE_BIND_MOUNTS = "no"' "$conf" || fail "locate config adds a missing PRUNE_BIND_MOUNTS"
grep -qFx 'PRUNEPATHS = "/.snapshots"' "$conf" || fail "locate config adds a missing PRUNEPATHS"
assert_conf_parses "$conf"
pass "locate config adds settings a hand-edited updatedb.conf is missing"
conf="$test_tmp/unspaced-updatedb.conf"
@@ -53,16 +69,34 @@ OMARCHY_UPDATEDB_CONF_PATH="$conf" bash -euo pipefail "$config_script" >/dev/nul
grep -qFx 'PRUNE_BIND_MOUNTS = "no"' "$conf" || fail "locate config rewrites an unspaced PRUNE_BIND_MOUNTS"
grep -qFx 'PRUNEPATHS = "/.snapshots /tmp /var/tmp"' "$conf" || fail "locate config prunes /.snapshots in an unspaced PRUNEPATHS"
[[ $(grep -c '^PRUNEPATHS' "$conf") -eq 1 ]] || fail "locate config keeps a single PRUNEPATHS setting"
[[ $(grep -c 'PRUNEPATHS' "$conf") -eq 1 ]] || fail "locate config keeps a single PRUNEPATHS setting"
assert_conf_parses "$conf"
pass "locate config handles updatedb.conf written without spaces around ="
# updatedb allows a comment after a value and indented settings, and defining
# either setting twice makes it refuse to run at all.
conf="$test_tmp/commented-updatedb.conf"
printf '%s\n' ' PRUNE_BIND_MOUNTS = "yes" # subvolumes look like bind mounts' \
'PRUNEPATHS = "/tmp" # scratch' >"$conf"
OMARCHY_UPDATEDB_CONF_PATH="$conf" bash -euo pipefail "$config_script" >/dev/null
grep -qFx 'PRUNE_BIND_MOUNTS = "no"' "$conf" || fail "locate config rewrites an indented PRUNE_BIND_MOUNTS"
grep -qFx 'PRUNEPATHS = "/.snapshots /tmp"' "$conf" || fail "locate config keeps the paths a commented PRUNEPATHS already prunes"
[[ $(grep -c 'PRUNEPATHS' "$conf") -eq 1 ]] || fail "locate config replaces a commented PRUNEPATHS instead of adding a second one"
assert_conf_parses "$conf"
pass "locate config handles indented settings and trailing comments"
# A hand-edited file may have dropped the quotes updatedb requires, which
# leaves it unparseable until something writes the setting out properly.
conf="$test_tmp/unquoted-updatedb.conf"
printf '%s\n' 'PRUNEPATHS = /tmp' >"$conf"
OMARCHY_UPDATEDB_CONF_PATH="$conf" bash -euo pipefail "$config_script" >/dev/null
grep -qFx 'PRUNEPATHS = "/.snapshots /tmp"' "$conf" || fail "locate config keeps the paths an unquoted PRUNEPATHS already prunes"
[[ $(grep -c '^PRUNEPATHS' "$conf") -eq 1 ]] || fail "locate config replaces an unquoted PRUNEPATHS instead of adding a second one"
grep -qFx 'PRUNEPATHS = "/.snapshots"' "$conf" || fail "locate config repairs an unquoted PRUNEPATHS"
[[ $(grep -c 'PRUNEPATHS' "$conf") -eq 1 ]] || fail "locate config replaces an unquoted PRUNEPATHS instead of adding a second one"
assert_conf_parses "$conf"
pass "locate config handles updatedb.conf written without quotes"
# A path that merely ends in /.snapshots is not the root snapshot directory.
@@ -72,6 +106,7 @@ printf '%s\n' 'PRUNEPATHS = "/var/lib/machines/.snapshots"' >"$conf"
OMARCHY_UPDATEDB_CONF_PATH="$conf" bash -euo pipefail "$config_script" >/dev/null
grep -qFx 'PRUNEPATHS = "/.snapshots /var/lib/machines/.snapshots"' "$conf" || fail "locate config prunes /.snapshots alongside a path that ends in it"
assert_conf_parses "$conf"
pass "locate config tells /.snapshots apart from a path that ends in it"
locate_migration=$(grep -rl 'Configure locate to skip Btrfs snapshots' "$ROOT/migrations" | head -n 1 || true)
test/shell.d/snapper-timeline-leak-test.sh
+26 -4
View File
@@ -15,6 +15,7 @@ mkdir -p "$fake_bin"
cat >"$fake_bin/sudo" <<'STUB'
#!/bin/bash
printf 'sudo %s\n' "$*" >>"$TEST_LOG"
exec "$@"
STUB
chmod +x "$fake_bin/sudo"
@@ -72,14 +73,18 @@ echo "failure: dbus timeout" >&2
exit 1
STUB
TEST_LOG="$test_tmp/calls.log" \
PATH="$fake_bin:$PATH" \
OMARCHY_SNAPPER_CONFIG_PATH="$snapper_config" \
bash -euo pipefail "$leak_migration" >/dev/null 2>&1 ||
output=$(TEST_LOG="$test_tmp/calls.log" \
PATH="$fake_bin:$PATH" \
OMARCHY_SNAPPER_CONFIG_PATH="$snapper_config" \
bash -euo pipefail "$leak_migration" 2>/dev/null) ||
fail "leak migration survives a failed delete batch"
deletes=$(grep -c '^snapper -c root delete ' "$test_tmp/calls.log" || true)
[[ $deletes -eq 3 ]] || fail "leak migration keeps draining after a failed batch" "expected 3 delete calls, got $deletes"
# omarchy-migrate writes the completion marker even when the drain gave up, so
# what is left has to be said out loud rather than left for a rerun.
grep -qF '45 snapshots could not be deleted' <<<"$output" || fail "leak migration reports the snapshots it could not delete" "$output"
pass "leak migration tolerates a batch that fails partway"
: >"$test_tmp/calls.log"
@@ -102,3 +107,20 @@ OMARCHY_SNAPPER_CONFIG_PATH="$test_tmp/missing" \
[[ ! -s $test_tmp/calls.log ]] || fail "leak migration skips systems without a Snapper root config"
pass "leak migration is a no-op without Snapper configured"
# Snapper's create-config writes a root-only config, and a config this user
# cannot read says nothing about whether timeline snapshots are wanted.
: >"$test_tmp/calls.log"
printf '%s\n' 'TIMELINE_CREATE="no"' >"$snapper_config"
chmod 000 "$snapper_config"
TEST_LOG="$test_tmp/calls.log" \
PATH="$fake_bin:$PATH" \
OMARCHY_SNAPPER_CONFIG_PATH="$snapper_config" \
bash -euo pipefail "$leak_migration" >/dev/null 2>&1
chmod 600 "$snapper_config"
grep -qF "sudo grep -qFx TIMELINE_CREATE=\"no\" $snapper_config" "$test_tmp/calls.log" ||
fail "leak migration reads a root-only Snapper config as root" "$(cat "$test_tmp/calls.log")"
pass "leak migration does not mistake an unreadable Snapper config for an intentional one"