Skip to content

Commit 32999d8

Browse files
committed
Merge branch 'refactor/python-config-parser'
2 parents 7646e22 + b4c1bb7 commit 32999d8

15 files changed

Lines changed: 490 additions & 425 deletions

‎.github/workflows/ci.yml‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ jobs:
4040
- 'lib/atomic/common.sh'
4141
- 'extras/pacman-wrapper'
4242
- 'tests/test_harness.sh'
43-
- 'tests/test_config.sh'
4443
- 'tests/test_validate.sh'
4544
- 'tests/test_device.sh'
4645
- 'tests/test_space.sh'
@@ -52,8 +51,10 @@ jobs:
5251
- 'tests/test_rebuild_uki.sh'
5352
- '.github/workflows/ci.yml'
5453
python:
54+
- 'lib/atomic/config.py'
5555
- 'lib/atomic/fstab.py'
5656
- 'lib/atomic/rootdev.py'
57+
- 'tests/test_config.py'
5758
- 'tests/test_fstab.py'
5859
- 'tests/test_rootdev.py'
5960
- '.github/workflows/ci.yml'
@@ -88,8 +89,8 @@ jobs:
8889
steps:
8990
- uses: actions/checkout@v6
9091
- run: pip install --no-cache-dir ruff mypy
91-
- run: ruff check lib/atomic/fstab.py lib/atomic/rootdev.py
92-
- run: mypy --ignore-missing-imports lib/atomic/fstab.py lib/atomic/rootdev.py
92+
- run: ruff check lib/atomic/config.py lib/atomic/fstab.py lib/atomic/rootdev.py
93+
- run: mypy --ignore-missing-imports lib/atomic/config.py lib/atomic/fstab.py lib/atomic/rootdev.py
9394

9495
python-test:
9596
needs: changes
@@ -100,6 +101,15 @@ jobs:
100101
- run: pip install --no-cache-dir pytest
101102
- run: python -m pytest tests/ -v
102103

104+
python-test-root:
105+
needs: changes
106+
if: needs.changes.outputs.python == 'true'
107+
runs-on: ubuntu-latest
108+
steps:
109+
- uses: actions/checkout@v6
110+
- run: sudo pip install --no-cache-dir pytest
111+
- run: sudo python -m pytest tests/test_config.py::TestParseConfig::test_config_not_owned_by_root -v
112+
103113
bash-test:
104114
needs: changes
105115
if: needs.changes.outputs.shell == 'true'
@@ -108,7 +118,6 @@ jobs:
108118
- uses: actions/checkout@v6
109119
- run: |
110120
for t in \
111-
tests/test_config.sh \
112121
tests/test_validate.sh \
113122
tests/test_device.sh \
114123
tests/test_space.sh \

‎Makefile‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
.PHONY: install uninstall reinstall install-conf man clean test
1+
.PHONY: install uninstall reinstall install-conf man clean test test-root
22

33
PREFIX = /usr
44
SYSCONFDIR = /etc
@@ -29,7 +29,6 @@ clean:
2929
rm -f $(MANPAGES)
3030

3131
UNIT_TESTS = \
32-
tests/test_config.sh \
3332
tests/test_validate.sh \
3433
tests/test_device.sh \
3534
tests/test_space.sh \
@@ -54,6 +53,21 @@ test:
5453
bash tests/test_integration.sh
5554
python -m pytest tests/test_fstab.py -v
5655
python -m pytest tests/test_rootdev.py -v
56+
python -m pytest tests/test_config.py -v
57+
58+
ROOT_TESTS = \
59+
tests/test_config.py::TestParseConfig::test_config_not_owned_by_root
60+
61+
test-root:
62+
@if [ "$$(id -u)" -ne 0 ]; then \
63+
echo "SKIP: test-root requires root (run: sudo make test-root)"; \
64+
else \
65+
for t in $(ROOT_TESTS); do \
66+
echo ""; \
67+
echo "━━━ $$t ━━━"; \
68+
python -m pytest "$$t" -v || exit 1; \
69+
done; \
70+
fi
5771

5872
install:
5973
install -Dm755 bin/atomic-upgrade $(DESTDIR)$(BINDIR)/atomic-upgrade
@@ -62,6 +76,7 @@ install:
6276
install -Dm755 bin/atomic-rebuild-uki $(DESTDIR)$(BINDIR)/atomic-rebuild-uki
6377

6478
install -Dm644 lib/atomic/common.sh $(DESTDIR)$(LIBDIR)/common.sh
79+
install -Dm755 lib/atomic/config.py $(DESTDIR)$(LIBDIR)/config.py
6580
install -Dm755 lib/atomic/fstab.py $(DESTDIR)$(LIBDIR)/fstab.py
6681
install -Dm755 lib/atomic/rootdev.py $(DESTDIR)$(LIBDIR)/rootdev.py
6782

@@ -102,6 +117,7 @@ uninstall:
102117
rm -f $(DESTDIR)$(BINDIR)/atomic-gc
103118
rm -f $(DESTDIR)$(BINDIR)/atomic-guard
104119
rm -f $(DESTDIR)$(BINDIR)/atomic-rebuild-uki
120+
rm -f $(DESTDIR)$(LIBDIR)/config.py
105121
rm -rf $(DESTDIR)$(LIBDIR)/
106122
rm -f $(DESTDIR)$(ZSH_COMPDIR)/_atomic-gc
107123
rm -f $(DESTDIR)$(ZSH_COMPDIR)/_atomic-rebuild-uki

‎README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ Edit `/etc/atomic.conf`:
166166
| `MAPPER_NAME` | `root_crypt` | dm-crypt mapper name (fallback if auto-detection fails) |
167167
| `KERNEL_PKG` | `linux` | Kernel package (linux/linux-lts/linux-zen) |
168168
| `KERNEL_PARAMS` | *(security defaults)* | Kernel command line parameters |
169-
| `CHROOT_COMMAND` | *(none)* | Default command in snapshot chroot (overrides built-in `pacman -Syu`) |
169+
| `CHROOT_COMMAND` | `/usr/bin/pacman -Syu` | Default command in snapshot chroot (overrides built-in `pacman -Syu`) |
170170
| `SBCTL_SIGN` | `0` | Sign UKI files with sbctl for Secure Boot (`0`=off, `1`=on) |
171171
| `UPGRADE_GUARD` | `1` | Upgrade guard: block direct `pacman -Syu` (`0`=off, `1`=on) |
172172
| `HOME_COPY_FILES` | *(empty)* | Files to copy into isolated home subvolumes (see [Home isolation](#home-isolation)) |
@@ -336,6 +336,7 @@ If the ESP is not mounted during the orphan sweep phase, it is skipped with a wa
336336
| `atomic-guard` | Pacman hook — blocks direct `-Syu`, allows installs/removes |
337337
| `atomic-rebuild-uki` | Rebuild UKI for existing snapshot |
338338
| `common.sh` | Shared library (config, locking, btrfs, UKI build, GC, home skeleton) |
339+
| `config.py` | Config file parser with proper quote handling via `shlex` |
339340
| `fstab.py` | Safe fstab editing (atomic write + verification + rollback) |
340341
| `rootdev.py` | Auto-detect root device type (LUKS/LVM/plain) and build kernel cmdline |
341342
| `pacman-wrapper` | Optional `/usr/local/bin/pacman` wrapper |

‎bin/atomic-upgrade‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@
88
set -euo pipefail
99

1010
readonly LIBDIR="/usr/lib/atomic"
11-
_src() { local p; p=$(verify-lib "$1" "$LIBDIR/") && source "$p" || exit 1; }
11+
12+
_src() {
13+
local p; p=$(verify-lib "$1" "$LIBDIR/") && source "$p" || exit 1
14+
}
1215

1316
# Source common.sh early for VERSION (skip auto-init for fast --help/--version)
1417
_ATOMIC_NO_INIT=1
@@ -107,17 +110,25 @@ if [[ -n "$CUSTOM_TAG" ]]; then
107110
fi
108111

109112
# Default chroot command: use CHROOT_COMMAND from config (or default in common.sh).
110-
# Note: CHROOT_COMMAND uses simple word splitting; args with spaces inside
111-
# are not supported (same limitation as HOME_COPY_FILES).
112113
if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then
113114
[[ -z "${CHROOT_COMMAND:-}" ]] && CHROOT_COMMAND="/usr/bin/pacman -Syu"
114-
read -ra CHROOT_CMD <<< "$CHROOT_COMMAND"
115-
fi
115+
CHROOT_CMD=()
116+
# Capture null-delimited output via temp file to reliably detect failures.
117+
# Use _ATOMIC_LIB_DIR if available (set by common.sh), else fall back to LIBDIR.
118+
_tmp_array=$(mktemp)
119+
_cfg_py="${_ATOMIC_LIB_DIR:-${LIBDIR}}/config.py"
120+
if CONFIG_FILE="${CONFIG_FILE:-/etc/atomic.conf}" python3 "${_cfg_py}" array CHROOT_COMMAND > "$_tmp_array" 2>/dev/null; then
121+
while IFS= read -r -d '' token; do
122+
CHROOT_CMD+=("$token")
123+
done < "$_tmp_array"
124+
else
125+
echo "WARN: config.py failed, falling back to word-split" >&2
126+
fi
127+
rm -f "$_tmp_array"
116128

117-
# Warn if CHROOT_COMMAND contains quotes (word splitting doesn't process them)
118-
if [[ "$CHROOT_COMMAND" =~ [\'\"] ]]; then
119-
echo "WARN: CHROOT_COMMAND contains quotes, but word splitting doesn't process them." >&2
120-
echo "WARN: Use CLI instead: atomic-upgrade -- pacman -S 'nvidia dkms'" >&2
129+
if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then
130+
read -ra CHROOT_CMD <<< "$CHROOT_COMMAND"
131+
fi
121132
fi
122133

123134
# Verify required variables are set
@@ -166,6 +177,7 @@ cleanup() {
166177

167178
mountpoint -q "$BTRFS_MOUNT" 2>/dev/null && umount "$BTRFS_MOUNT"
168179
[[ -n "${LOCK_FD:-}" ]] && exec {LOCK_FD}>&- 2>/dev/null
180+
[[ -n "${_tmp_array:-}" ]] && rm -f "$_tmp_array" 2>/dev/null
169181

170182
return $exit_code
171183
}

‎lib/atomic/common.sh‎

Lines changed: 16 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,7 @@
44
# Shared functions and configuration for the atomic-upgrade system.
55
# Sourced by: atomic-upgrade, atomic-gc, atomic-rebuild-uki, atomic-guard
66

7-
# ── Library directory ──────────────────────────────────────────────
8-
9-
if [[ -z "${LIBDIR:-}" ]]; then
10-
LIBDIR="/usr/lib/atomic"
11-
fi
7+
_ATOMIC_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
128

139
# ── Project version ────────────────────────────────────────────────
1410

@@ -32,62 +28,25 @@ KERNEL_PARAMS="rw slab_nomerge init_on_alloc=1 page_alloc.shuffle=1 pti=on vsysc
3228
# Default chroot command (overridden by config or CLI -- CHROOT_COMMAND...)
3329
CHROOT_COMMAND="/usr/bin/pacman -Syu"
3430

35-
# ── Config loading (safe parser, no arbitrary code execution) ───────
31+
# ── Config loading (delegates to Python for proper quote handling) ───────
3632

3733
CONFIG_FILE="${CONFIG_FILE:-/etc/atomic.conf}"
3834

3935
load_config() {
4036
[[ -f "$CONFIG_FILE" ]] || return 0
4137

42-
# Refuse to load config not owned by root
43-
local owner
44-
owner=$(stat -c %u "$CONFIG_FILE" 2>/dev/null)
45-
if [[ "$owner" != "0" ]]; then
46-
echo "ERROR: $CONFIG_FILE not owned by root (owner uid: $owner)" >&2
38+
local shell_output config_err
39+
if ! shell_output=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" shell 2>/dev/null); then
40+
config_err=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" shell 2>&1) || true
41+
echo "ERROR: Failed to parse config with config.py" >&2
42+
echo "ERROR: Details: ${config_err:-unknown}" >&2
4743
return 1
4844
fi
4945

50-
# Whitelist of allowed config keys
51-
local -a allowed=(BTRFS_MOUNT NEW_ROOT ESP KEEP_GENERATIONS MAPPER_NAME KERNEL_PARAMS KERNEL_PKG CHROOT_COMMAND SBCTL_SIGN UPGRADE_GUARD HOME_COPY_FILES)
52-
5346
while IFS='=' read -r key value; do
54-
# Strip leading/trailing whitespace from key
55-
key="${key#"${key%%[![:space:]]*}"}"
56-
key="${key%"${key##*[![:space:]]}"}"
57-
# Trim leading/trailing whitespace from value
58-
value="${value#"${value%%[![:space:]]*}"}"
59-
value="${value%"${value##*[![:space:]]}"}"
60-
61-
# Strip inline comments: everything after ' #' (space-hash).
62-
# Limitation: values containing literal ' #' are truncated.
63-
# Quoted values do not protect against this. For kernel params
64-
# and paths used in this config, this is not a practical concern.
65-
value="${value%% #*}"
66-
value="${value%"${value##*[![:space:]]}"}"
67-
68-
# Skip comments and blank lines
69-
[[ "$key" =~ ^#.*$ || -z "$key" ]] && continue
70-
71-
# Check against whitelist
72-
local valid=0
73-
for a in "${allowed[@]}"; do
74-
if [[ "$key" == "$a" ]]; then
75-
valid=1
76-
break
77-
fi
78-
done
79-
80-
if [[ $valid -eq 1 ]]; then
81-
# Strip surrounding quotes (single or double)
82-
value="${value#\"}"
83-
value="${value%\"}"
84-
value="${value#\'}"
85-
value="${value%\'}"
86-
printf -v "$key" '%s' "$value"
87-
else
88-
echo "WARN: Unknown config key ignored: $key" >&2
89-
fi
90-
done < "$CONFIG_FILE"
47+
[[ -z "$key" || "$key" == \#* ]] && continue
48+
printf -v "$key" '%s' "$value"
49+
done <<< "$shell_output"
9150
}
9251

9352
# ── Auto-initialization ──────────────────────────────────────────────
@@ -132,7 +91,7 @@ check_dependencies() {
13291
local root_type
13392
root_type=$(python3 -c "
13493
import json, importlib.util, sys
135-
spec = importlib.util.spec_from_file_location('rootdev', '${LIBDIR}/rootdev.py')
94+
spec = importlib.util.spec_from_file_location('rootdev', '${_ATOMIC_LIB_DIR}/rootdev.py')
13695
mod = importlib.util.module_from_spec(spec)
13796
spec.loader.exec_module(mod)
13897
try:
@@ -150,7 +109,7 @@ except Exception:
150109
return 1
151110
fi
152111
# Verify python helper modules exist
153-
for helper in "${LIBDIR}/fstab.py" "${LIBDIR}/rootdev.py"; do
112+
for helper in "${_ATOMIC_LIB_DIR}/fstab.py" "${_ATOMIC_LIB_DIR}/rootdev.py"; do
154113
[[ -f "$helper" ]] || {
155114
echo "ERROR: Missing helper: $helper" >&2
156115
return 1
@@ -280,15 +239,15 @@ verify_uki() {
280239
# ── fstab update (delegates to Python for safety) ──────────────────
281240

282241
update_fstab() {
283-
python3 "${LIBDIR}/fstab.py" "$@"
242+
python3 "${_ATOMIC_LIB_DIR}/fstab.py" "$@"
284243
}
285244

286245
# ── fstab /home update ──────────────────────────────────────────
287246
# Updates the /home mount entry's subvol= to point to a new subvolume.
288247
# Args: $1 = fstab path, $2 = new home subvolume name
289248

290249
update_fstab_home() {
291-
python3 "${LIBDIR}/fstab.py" home "$@"
250+
python3 "${_ATOMIC_LIB_DIR}/fstab.py" home "$@"
292251
}
293252

294253
# ── Home skeleton population ────────────────────────────────────
@@ -394,7 +353,7 @@ get_root_device() {
394353
fi
395354

396355
local dev
397-
dev=$(python3 "${LIBDIR}/rootdev.py" device 2>/dev/null)
356+
dev=$(python3 "${_ATOMIC_LIB_DIR}/rootdev.py" device 2>/dev/null)
398357
if [[ -n "$dev" && -e "$dev" ]]; then
399358
_ROOT_DEVICE="$dev"
400359
echo "$dev"
@@ -572,7 +531,7 @@ build_uki() {
572531
fi
573532

574533
local root_cmdline
575-
root_cmdline=$(python3 "${LIBDIR}/rootdev.py" cmdline "$new_subvol") || {
534+
root_cmdline=$(python3 "${_ATOMIC_LIB_DIR}/rootdev.py" cmdline "$new_subvol") || {
576535
echo "ERROR: Cannot detect root device for cmdline" >&2
577536
return 1
578537
}

0 commit comments

Comments
 (0)