Replace bash parser with Python (shlex) for proper quote handling

.github/workflows/ci.yml

@@ -40,7 +40,6 @@ jobs:
               - 'lib/atomic/common.sh'
               - 'extras/pacman-wrapper'
               - 'tests/test_harness.sh'
-              - 'tests/test_config.sh'
               - 'tests/test_validate.sh'
               - 'tests/test_device.sh'
               - 'tests/test_space.sh'
@@ -52,8 +51,10 @@ jobs:
               - 'tests/test_rebuild_uki.sh'
               - '.github/workflows/ci.yml'
             python:
+              - 'lib/atomic/config.py'
               - 'lib/atomic/fstab.py'
               - 'lib/atomic/rootdev.py'
+              - 'tests/test_config.py'
               - 'tests/test_fstab.py'
               - 'tests/test_rootdev.py'
               - '.github/workflows/ci.yml'
@@ -88,8 +89,8 @@ jobs:
     steps:
       - uses: actions/checkout@v6
       - run: pip install --no-cache-dir ruff mypy
-      - run: ruff check lib/atomic/fstab.py lib/atomic/rootdev.py
-      - run: mypy --ignore-missing-imports lib/atomic/fstab.py lib/atomic/rootdev.py
+      - run: ruff check lib/atomic/config.py lib/atomic/fstab.py lib/atomic/rootdev.py
+      - run: mypy --ignore-missing-imports lib/atomic/config.py lib/atomic/fstab.py lib/atomic/rootdev.py
 
   python-test:
     needs: changes
@@ -100,6 +101,15 @@ jobs:
       - run: pip install --no-cache-dir pytest
       - run: python -m pytest tests/ -v
 
+  python-test-root:
+    needs: changes
+    if: needs.changes.outputs.python == 'true'
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v6
+      - run: sudo pip install --no-cache-dir pytest
+      - run: sudo python -m pytest tests/test_config.py::TestParseConfig::test_config_not_owned_by_root -v
+
   bash-test:
     needs: changes
     if: needs.changes.outputs.shell == 'true'
@@ -108,7 +118,6 @@ jobs:
       - uses: actions/checkout@v6
       - run: |
           for t in \
-            tests/test_config.sh \
             tests/test_validate.sh \
             tests/test_device.sh \
             tests/test_space.sh \

Makefile

@@ -1,4 +1,4 @@
-.PHONY: install uninstall reinstall install-conf man clean test
+.PHONY: install uninstall reinstall install-conf man clean test test-root
 
 PREFIX     = /usr
 SYSCONFDIR = /etc
@@ -29,7 +29,6 @@ clean:
 	rm -f $(MANPAGES)
 
 UNIT_TESTS = \
-	tests/test_config.sh \
 	tests/test_validate.sh \
 	tests/test_device.sh \
 	tests/test_space.sh \
@@ -54,6 +53,21 @@ test:
 	bash tests/test_integration.sh
 	python -m pytest tests/test_fstab.py -v
 	python -m pytest tests/test_rootdev.py -v
+	python -m pytest tests/test_config.py -v
+
+ROOT_TESTS = \
+	tests/test_config.py::TestParseConfig::test_config_not_owned_by_root
+
+test-root:
+	@if [ "$$(id -u)" -ne 0 ]; then \
+		echo "SKIP: test-root requires root (run: sudo make test-root)"; \
+	else \
+		for t in $(ROOT_TESTS); do \
+			echo ""; \
+			echo "━━━ $$t ━━━"; \
+			python -m pytest "$$t" -v || exit 1; \
+		done; \
+	fi
 
 install:
 	install -Dm755 bin/atomic-upgrade     $(DESTDIR)$(BINDIR)/atomic-upgrade
@@ -62,6 +76,7 @@ install:
 	install -Dm755 bin/atomic-rebuild-uki $(DESTDIR)$(BINDIR)/atomic-rebuild-uki
 
 	install -Dm644 lib/atomic/common.sh  $(DESTDIR)$(LIBDIR)/common.sh
+	install -Dm755 lib/atomic/config.py   $(DESTDIR)$(LIBDIR)/config.py
 	install -Dm755 lib/atomic/fstab.py   $(DESTDIR)$(LIBDIR)/fstab.py
 	install -Dm755 lib/atomic/rootdev.py $(DESTDIR)$(LIBDIR)/rootdev.py
 
@@ -102,6 +117,7 @@ uninstall:
 	rm -f  $(DESTDIR)$(BINDIR)/atomic-gc
 	rm -f  $(DESTDIR)$(BINDIR)/atomic-guard
 	rm -f  $(DESTDIR)$(BINDIR)/atomic-rebuild-uki
+	rm -f  $(DESTDIR)$(LIBDIR)/config.py
 	rm -rf $(DESTDIR)$(LIBDIR)/
 	rm -f  $(DESTDIR)$(ZSH_COMPDIR)/_atomic-gc
 	rm -f  $(DESTDIR)$(ZSH_COMPDIR)/_atomic-rebuild-uki

README.md

@@ -166,7 +166,7 @@ Edit `/etc/atomic.conf`:
 | `MAPPER_NAME` | `root_crypt` | dm-crypt mapper name (fallback if auto-detection fails) |
 | `KERNEL_PKG` | `linux` | Kernel package (linux/linux-lts/linux-zen) |
 | `KERNEL_PARAMS` | *(security defaults)* | Kernel command line parameters |
-| `CHROOT_COMMAND` | *(none)* | Default command in snapshot chroot (overrides built-in `pacman -Syu`) |
+| `CHROOT_COMMAND` | `/usr/bin/pacman -Syu` | Default command in snapshot chroot (overrides built-in `pacman -Syu`) |
 | `SBCTL_SIGN` | `0` | Sign UKI files with sbctl for Secure Boot (`0`=off, `1`=on) |
 | `UPGRADE_GUARD` | `1` | Upgrade guard: block direct `pacman -Syu` (`0`=off, `1`=on) |
 | `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
 | `atomic-guard` | Pacman hook — blocks direct `-Syu`, allows installs/removes |
 | `atomic-rebuild-uki` | Rebuild UKI for existing snapshot |
 | `common.sh` | Shared library (config, locking, btrfs, UKI build, GC, home skeleton) |
+| `config.py` | Config file parser with proper quote handling via `shlex` |
 | `fstab.py` | Safe fstab editing (atomic write + verification + rollback) |
 | `rootdev.py` | Auto-detect root device type (LUKS/LVM/plain) and build kernel cmdline |
 | `pacman-wrapper` | Optional `/usr/local/bin/pacman` wrapper |

bin/atomic-upgrade

@@ -8,7 +8,10 @@
 set -euo pipefail
 
 readonly LIBDIR="/usr/lib/atomic"
-_src() { local p; p=$(verify-lib "$1" "$LIBDIR/") && source "$p" || exit 1; }
+
+_src() {
+    local p; p=$(verify-lib "$1" "$LIBDIR/") && source "$p" || exit 1
+}
 
 # Source common.sh early for VERSION (skip auto-init for fast --help/--version)
 _ATOMIC_NO_INIT=1
@@ -107,17 +110,25 @@ if [[ -n "$CUSTOM_TAG" ]]; then
 fi
 
 # Default chroot command: use CHROOT_COMMAND from config (or default in common.sh).
-# Note: CHROOT_COMMAND uses simple word splitting; args with spaces inside
-# are not supported (same limitation as HOME_COPY_FILES).
 if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then
     [[ -z "${CHROOT_COMMAND:-}" ]] && CHROOT_COMMAND="/usr/bin/pacman -Syu"
-    read -ra CHROOT_CMD <<< "$CHROOT_COMMAND"
-fi
+    CHROOT_CMD=()
+    # Capture null-delimited output via temp file to reliably detect failures.
+    # Use _ATOMIC_LIB_DIR if available (set by common.sh), else fall back to LIBDIR.
+    _tmp_array=$(mktemp)
+    _cfg_py="${_ATOMIC_LIB_DIR:-${LIBDIR}}/config.py"
+    if CONFIG_FILE="${CONFIG_FILE:-/etc/atomic.conf}" python3 "${_cfg_py}" array CHROOT_COMMAND > "$_tmp_array" 2>/dev/null; then
+        while IFS= read -r -d '' token; do
+            CHROOT_CMD+=("$token")
+        done < "$_tmp_array"
+    else
+        echo "WARN: config.py failed, falling back to word-split" >&2
+    fi
+    rm -f "$_tmp_array"
 
-# Warn if CHROOT_COMMAND contains quotes (word splitting doesn't process them)
-if [[ "$CHROOT_COMMAND" =~ [\'\"] ]]; then
-    echo "WARN: CHROOT_COMMAND contains quotes, but word splitting doesn't process them." >&2
-    echo "WARN: Use CLI instead: atomic-upgrade -- pacman -S 'nvidia dkms'" >&2
+    if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then
+        read -ra CHROOT_CMD <<< "$CHROOT_COMMAND"
+    fi
 fi
 
 # Verify required variables are set
@@ -166,6 +177,7 @@ cleanup() {
 
     mountpoint -q "$BTRFS_MOUNT" 2>/dev/null && umount "$BTRFS_MOUNT"
     [[ -n "${LOCK_FD:-}" ]] && exec {LOCK_FD}>&- 2>/dev/null
+    [[ -n "${_tmp_array:-}" ]] && rm -f "$_tmp_array" 2>/dev/null
 
     return $exit_code
 }

lib/atomic/common.sh

@@ -4,11 +4,7 @@
 # Shared functions and configuration for the atomic-upgrade system.
 # Sourced by: atomic-upgrade, atomic-gc, atomic-rebuild-uki, atomic-guard
 
-# ── Library directory ──────────────────────────────────────────────
-
-if [[ -z "${LIBDIR:-}" ]]; then
-    LIBDIR="/usr/lib/atomic"
-fi
+_ATOMIC_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 
 # ── Project version ────────────────────────────────────────────────
 
@@ -32,62 +28,25 @@ KERNEL_PARAMS="rw slab_nomerge init_on_alloc=1 page_alloc.shuffle=1 pti=on vsysc
 # Default chroot command (overridden by config or CLI -- CHROOT_COMMAND...)
 CHROOT_COMMAND="/usr/bin/pacman -Syu"
 
-# ── Config loading (safe parser, no arbitrary code execution) ───────
+# ── Config loading (delegates to Python for proper quote handling) ───────
 
 CONFIG_FILE="${CONFIG_FILE:-/etc/atomic.conf}"
 
 load_config() {
     [[ -f "$CONFIG_FILE" ]] || return 0
 
-    # Refuse to load config not owned by root
-    local owner
-    owner=$(stat -c %u "$CONFIG_FILE" 2>/dev/null)
-    if [[ "$owner" != "0" ]]; then
-        echo "ERROR: $CONFIG_FILE not owned by root (owner uid: $owner)" >&2
+    local shell_output config_err
+    if ! shell_output=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" shell 2>/dev/null); then
+        config_err=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" shell 2>&1) || true
+        echo "ERROR: Failed to parse config with config.py" >&2
+        echo "ERROR: Details: ${config_err:-unknown}" >&2
         return 1
     fi
 
-    # Whitelist of allowed config keys
-    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)
-
     while IFS='=' read -r key value; do
-        # Strip leading/trailing whitespace from key
-        key="${key#"${key%%[![:space:]]*}"}"
-        key="${key%"${key##*[![:space:]]}"}"
-        # Trim leading/trailing whitespace from value
-        value="${value#"${value%%[![:space:]]*}"}"
-        value="${value%"${value##*[![:space:]]}"}"
-
-        # Strip inline comments: everything after ' #' (space-hash).
-        # Limitation: values containing literal ' #' are truncated.
-        # Quoted values do not protect against this.  For kernel params
-        # and paths used in this config, this is not a practical concern.
-        value="${value%% #*}"
-        value="${value%"${value##*[![:space:]]}"}"
-
-        # Skip comments and blank lines
-        [[ "$key" =~ ^#.*$ || -z "$key" ]] && continue
-
-        # Check against whitelist
-        local valid=0
-        for a in "${allowed[@]}"; do
-            if [[ "$key" == "$a" ]]; then
-                valid=1
-                break
-            fi
-        done
-
-        if [[ $valid -eq 1 ]]; then
-            # Strip surrounding quotes (single or double)
-            value="${value#\"}"
-            value="${value%\"}"
-            value="${value#\'}"
-            value="${value%\'}"
-            printf -v "$key" '%s' "$value"
-        else
-            echo "WARN: Unknown config key ignored: $key" >&2
-        fi
-    done < "$CONFIG_FILE"
+        [[ -z "$key" || "$key" == \#* ]] && continue
+        printf -v "$key" '%s' "$value"
+    done <<< "$shell_output"
 }
 
 # ── Auto-initialization ──────────────────────────────────────────────
@@ -132,7 +91,7 @@ check_dependencies() {
     local root_type
     root_type=$(python3 -c "
 import json, importlib.util, sys
-spec = importlib.util.spec_from_file_location('rootdev', '${LIBDIR}/rootdev.py')
+spec = importlib.util.spec_from_file_location('rootdev', '${_ATOMIC_LIB_DIR}/rootdev.py')
 mod = importlib.util.module_from_spec(spec)
 spec.loader.exec_module(mod)
 try:
@@ -150,7 +109,7 @@ except Exception:
         return 1
     fi
     # Verify python helper modules exist
-    for helper in "${LIBDIR}/fstab.py" "${LIBDIR}/rootdev.py"; do
+    for helper in "${_ATOMIC_LIB_DIR}/fstab.py" "${_ATOMIC_LIB_DIR}/rootdev.py"; do
         [[ -f "$helper" ]] || {
             echo "ERROR: Missing helper: $helper" >&2
             return 1
@@ -280,15 +239,15 @@ verify_uki() {
 # ── fstab update (delegates to Python for safety) ──────────────────
 
 update_fstab() {
-    python3 "${LIBDIR}/fstab.py" "$@"
+    python3 "${_ATOMIC_LIB_DIR}/fstab.py" "$@"
 }
 
 # ── fstab /home update ──────────────────────────────────────────
 # Updates the /home mount entry's subvol= to point to a new subvolume.
 # Args: $1 = fstab path, $2 = new home subvolume name
 
 update_fstab_home() {
-    python3 "${LIBDIR}/fstab.py" home "$@"
+    python3 "${_ATOMIC_LIB_DIR}/fstab.py" home "$@"
 }
 
 # ── Home skeleton population ────────────────────────────────────
@@ -394,7 +353,7 @@ get_root_device() {
     fi
 
     local dev
-    dev=$(python3 "${LIBDIR}/rootdev.py" device 2>/dev/null)
+    dev=$(python3 "${_ATOMIC_LIB_DIR}/rootdev.py" device 2>/dev/null)
     if [[ -n "$dev" && -e "$dev" ]]; then
         _ROOT_DEVICE="$dev"
         echo "$dev"
@@ -572,7 +531,7 @@ build_uki() {
     fi
 
     local root_cmdline
-    root_cmdline=$(python3 "${LIBDIR}/rootdev.py" cmdline "$new_subvol") || {
+    root_cmdline=$(python3 "${_ATOMIC_LIB_DIR}/rootdev.py" cmdline "$new_subvol") || {
         echo "ERROR: Cannot detect root device for cmdline" >&2
         return 1
     }