From 01838057cb3ac3e9afe3418fb9011e0bb79d1981 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 00:07:29 +0300 Subject: [PATCH 01/23] refactor: migrate to Python parser with proper quote handling --- .github/workflows/ci.yml | 8 +- Makefile | 4 +- README.md | 3 +- bin/atomic-upgrade | 20 +-- lib/atomic/common.sh | 57 ++----- lib/atomic/config.py | 163 +++++++++++++++++++ man/atomic-upgrade.8 | 5 +- man/atomic-upgrade.8.md | 3 + man/atomic.conf.5 | 21 +-- man/atomic.conf.5.md | 14 +- tests/test_config.py | 199 +++++++++++++++++++++++ tests/test_config.sh | 329 --------------------------------------- 12 files changed, 417 insertions(+), 409 deletions(-) create mode 100644 lib/atomic/config.py create mode 100644 tests/test_config.py delete mode 100644 tests/test_config.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a001cc5..5b6a47f 100644 --- a/.github/workflows/ci.yml +++ b/.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 @@ -108,7 +109,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 \ diff --git a/Makefile b/Makefile index edf6a80..92e6ede 100644 --- a/Makefile +++ b/Makefile @@ -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,7 @@ 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 install: install -Dm755 bin/atomic-upgrade $(DESTDIR)$(BINDIR)/atomic-upgrade @@ -62,6 +62,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 +103,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 diff --git a/README.md b/README.md index c76b526..7daf3e7 100644 --- a/README.md +++ b/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 | diff --git a/bin/atomic-upgrade b/bin/atomic-upgrade index 1e3c163..6ef087b 100644 --- a/bin/atomic-upgrade +++ b/bin/atomic-upgrade @@ -107,17 +107,19 @@ 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 - -# 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 + CHROOT_CMD=() + config_err=$(python3 "${LIBDIR}/config.py" array CHROOT_COMMAND 2>&1) && { + while IFS= read -r -d '' token; do + CHROOT_CMD+=("$token") + done <<< "$config_err" + } || { + echo "WARN: Failed to parse CHROOT_COMMAND with config.py: $config_err" >&2 + } + if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then + read -ra CHROOT_CMD <<< "$CHROOT_COMMAND" + fi fi # Verify required variables are set diff --git a/lib/atomic/common.sh b/lib/atomic/common.sh index e85119c..65186d5 100644 --- a/lib/atomic/common.sh +++ b/lib/atomic/common.sh @@ -32,62 +32,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 + shell_output=$(python3 "${LIBDIR}/config.py" shell 2>&1) || { + config_err="$shell_output" + echo "ERROR: Failed to parse config with config.py" >&2 + echo "ERROR: Details: $config_err" >&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 ────────────────────────────────────────────── diff --git a/lib/atomic/config.py b/lib/atomic/config.py new file mode 100644 index 0000000..87e46dd --- /dev/null +++ b/lib/atomic/config.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 +""" +/usr/lib/atomic/config.py + +Parse /etc/atomic.conf with proper shell-style quote handling. +Uses shlex for correct parsing of single/double quotes and escapes. + +Usage: + config.py -> dump all config as JSON + config.py KEY -> print value of KEY + config.py validate -> validate config, exit 0/1 +""" + +import json +import shlex +import sys +import os +from pathlib import Path + +DEFAULT_CONFIG = { + "BTRFS_MOUNT": "/run/atomic/temp_root", + "NEW_ROOT": "/run/atomic/newroot", + "ESP": "/efi", + "KEEP_GENERATIONS": "3", + "MAPPER_NAME": "root_crypt", + "KERNEL_PKG": "linux", + "KERNEL_PARAMS": "rw slab_nomerge init_on_alloc=1 page_alloc.shuffle=1 pti=on vsyscall=none randomize_kstack_offset=on debugfs=off", + "CHROOT_COMMAND": "/usr/bin/pacman -Syu", + "SBCTL_SIGN": "0", + "UPGRADE_GUARD": "1", + "HOME_COPY_FILES": "", +} + +ALLOWED_KEYS = set(DEFAULT_CONFIG.keys()) + + +def parse_config(path=None): + """Parse atomic.conf with proper quote handling via shlex.""" + config = dict(DEFAULT_CONFIG) + if path is None: + path = os.environ.get("CONFIG_FILE", "/etc/atomic.conf") + config_path = Path(path) + if not config_path.exists(): + return config + + # Only check owner for /etc/atomic.conf (system config). + # Other paths (e.g. test configs) are allowed to have any owner. + if path == "/etc/atomic.conf": + try: + if config_path.stat().st_uid != 0: + print(f"ERROR: {path} not owned by root", file=sys.stderr) + sys.exit(1) + except OSError as e: + print(f"ERROR: Cannot read {path}: {e}", file=sys.stderr) + sys.exit(1) + + content = config_path.read_text() + + for line in content.splitlines(): + line = line.strip() + + if not line or line.startswith("#"): + continue + + if " #" in line: + line = line[:line.index(" #")].strip() + + if not line or line.startswith("#"): + continue + + if "=" not in line: + continue + + key, value = line.split("=", 1) + key = key.strip() + value = value.strip() + + if key not in ALLOWED_KEYS: + print(f"WARN: Unknown config key ignored: {key}", file=sys.stderr) + continue + + if len(value) >= 2: + if (value.startswith('"') and value.endswith('"')) or \ + (value.startswith("'") and value.endswith("'")): + value = value[1:-1] + config[key] = value + + return config + + +def config_to_shell(config): + """Convert config dict to shell-safe KEY=value lines.""" + lines = [] + for key, value in config.items(): + lines.append(f"{key}={value}") + return "\n".join(lines) + + +def config_to_array(config, key): + """Output array elements null-separated for safe shell reading.""" + value = config.get(key, "") + if not value: + return + tokens = shlex.split(value, posix=True) + for token in tokens: + sys.stdout.write(token + '\0') + + +def main(): + command = sys.argv[1] if len(sys.argv) > 1 else "dump" + + if command == "dump": + config = parse_config() + json.dump(config, sys.stdout, indent=2) + print() + return 0 + + if command == "validate": + config = parse_config() + try: + int(config["KEEP_GENERATIONS"]) + except ValueError: + print("ERROR: KEEP_GENERATIONS must be a number", file=sys.stderr) + return 1 + try: + int(config["SBCTL_SIGN"]) + except ValueError: + print("ERROR: SBCTL_SIGN must be a number", file=sys.stderr) + return 1 + try: + int(config["UPGRADE_GUARD"]) + except ValueError: + print("ERROR: UPGRADE_GUARD must be a number", file=sys.stderr) + return 1 + print("Config valid") + return 0 + + if command == "shell": + config = parse_config() + print(config_to_shell(config)) + return 0 + + if command == "array": + if len(sys.argv) < 3: + print("ERROR: array command requires KEY argument", file=sys.stderr) + return 1 + key = sys.argv[2] + config = parse_config() + config_to_array(config, key) + return 0 + + key = command + config = parse_config() + if key not in config: + print(f"ERROR: Unknown key: {key}", file=sys.stderr) + return 1 + + print(config[key]) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/man/atomic-upgrade.8 b/man/atomic-upgrade.8 index c319b11..2977ed7 100644 --- a/man/atomic-upgrade.8 +++ b/man/atomic-upgrade.8 @@ -1,4 +1,4 @@ -.\" Automatically generated by Pandoc 3.5 +.\" Automatically generated by Pandoc 3.6 .\" .TH "ATOMIC\-UPGRADE" "8" "" "atomic\-upgrade" "System Administration" .SH NAME @@ -167,6 +167,9 @@ See \f[B]atomic.conf\f[R](5). \f[I]/usr/lib/atomic/common.sh\f[R] Shared shell library. .TP +\f[I]/usr/lib/atomic/config.py\f[R] +Config file parser with proper quote handling via \f[CR]shlex\f[R]. +.TP \f[I]/usr/lib/atomic/fstab.py\f[R] Safe fstab manipulation (atomic write, verification, rollback). .TP diff --git a/man/atomic-upgrade.8.md b/man/atomic-upgrade.8.md index 02dddb8..f8f32f7 100644 --- a/man/atomic-upgrade.8.md +++ b/man/atomic-upgrade.8.md @@ -156,6 +156,9 @@ If space cannot be determined, a warning is shown and the operation proceeds. */usr/lib/atomic/common.sh* : Shared shell library. +*/usr/lib/atomic/config.py* +: Config file parser with proper quote handling via `shlex`. + */usr/lib/atomic/fstab.py* : Safe fstab manipulation (atomic write, verification, rollback). diff --git a/man/atomic.conf.5 b/man/atomic.conf.5 index 1713c64..a32cfe2 100644 --- a/man/atomic.conf.5 +++ b/man/atomic.conf.5 @@ -1,4 +1,4 @@ -.\" Automatically generated by Pandoc 3.5 +.\" Automatically generated by Pandoc 3.6 .\" .TH "ATOMIC.CONF" "5" "" "atomic\-upgrade" "File Formats" .SH NAME @@ -26,10 +26,10 @@ Values containing a literal \f[CR]#\f[R] sequence will be truncated at that point. This does not affect typical paths or kernel parameters. .PP -Only whitelisted keys are accepted. +Only allowed keys are accepted. Unknown keys produce a warning on stderr and are ignored. -The file must be owned by root (uid 0); otherwise it is rejected -entirely. +The file must be owned by root (uid 0) when at +\f[CR]/etc/atomic.conf\f[R]; otherwise it is rejected entirely. .SH OPTIONS .TP \f[B]BTRFS_MOUNT\f[R] @@ -71,14 +71,14 @@ device arguments. Default: \f[B]rw slab_nomerge init_on_alloc=1 page_alloc.shuffle=1 pti=on vsyscall=none randomize_kstack_offset=on debugfs=off\f[R]. .TP -\f[B]COMMAND\f[R] +\f[B]CHROOT_COMMAND\f[R] Default command to run in the snapshot chroot, overriding the built\-in default of \f[B]pacman \-Syu\f[R]. -Command\-line \f[B]\-\-\f[R] \f[I]COMMAND...\f[R] takes priority over +Command\-line \f[B]\[en] CHROOT_COMMAND\&...\f[R] takes priority over this setting. -Note: arguments with spaces inside are not supported due to simple word -splitting. -Default: \f[I](none)\f[R]. +Arguments with spaces are supported via proper shell\-style quote +handling. +Default: \f[I]/usr/bin/pacman \-Syu\f[R]. .TP \f[B]SBCTL_SIGN\f[R] Enable UKI signing with \f[B]sbctl\f[R](8) for Secure Boot. @@ -101,7 +101,8 @@ value per invocation. Paths with spaces are not supported. Default: \f[I](empty)\f[R]. .SH SECURITY -The configuration file must be owned by root. +The configuration file must be owned by root when at +\f[CR]/etc/atomic.conf\f[R]. If the file is owned by another user, it is rejected and no values are loaded. .PP diff --git a/man/atomic.conf.5.md b/man/atomic.conf.5.md index 66dc31f..de497b0 100644 --- a/man/atomic.conf.5.md +++ b/man/atomic.conf.5.md @@ -29,9 +29,9 @@ single or double quotes, which are stripped during parsing. occurrence. Values containing a literal ` #` sequence will be truncated at that point. This does not affect typical paths or kernel parameters. -Only whitelisted keys are accepted. Unknown keys produce a warning on stderr -and are ignored. The file must be owned by root (uid 0); otherwise it is -rejected entirely. +Only allowed keys are accepted. Unknown keys produce a warning on stderr +and are ignored. The file must be owned by root (uid 0) when at +`/etc/atomic.conf`; otherwise it is rejected entirely. # OPTIONS @@ -69,8 +69,8 @@ rejected entirely. **CHROOT_COMMAND** : Default command to run in the snapshot chroot, overriding the built-in default of **pacman -Syu**. Command-line **-- CHROOT_COMMAND...** takes priority - over this setting. Note: arguments with spaces inside are not supported - due to simple word splitting. Default: *(none)*. + over this setting. Arguments with spaces are supported via proper + shell-style quote handling. Default: */usr/bin/pacman -Syu*. **SBCTL_SIGN** : Enable UKI signing with **sbctl**(8) for Secure Boot. Set to **1** to @@ -90,8 +90,8 @@ rejected entirely. # SECURITY -The configuration file must be owned by root. If the file is owned by -another user, it is rejected and no values are loaded. +The configuration file must be owned by root when at `/etc/atomic.conf`. +If the file is owned by another user, it is rejected and no values are loaded. Only the keys listed above are accepted. Attempts to set arbitrary shell variables (e.g., **PATH**, **LD_PRELOAD**) via the config file are silently diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..5217a0b --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,199 @@ +#!/usr/bin/env python3 + +import json +import os +import subprocess +import tempfile +from pathlib import Path +import pytest + +LIBDIR = Path(__file__).parent.parent / "lib" / "atomic" +CONFIG_SCRIPT = LIBDIR / "config.py" + + +def run_config(*args): + env = os.environ.copy() + if hasattr(run_config, 'config_path'): + env['CONFIG_FILE'] = run_config.config_path + result = subprocess.run( + ["python3", str(CONFIG_SCRIPT)] + list(args), + capture_output=True, + text=True, + timeout=10, + env=env, + ) + return result.returncode, result.stdout.strip(), result.stderr.strip() + + +def create_temp_config(content, owner_uid=0): + with tempfile.NamedTemporaryFile(mode="w", suffix=".conf", delete=False) as f: + f.write(content) + path = f.name + os.chmod(path, 0o644) + if owner_uid != 0: + try: + os.chown(path, owner_uid, -1) + except PermissionError: + pass + return path + + +class TestParseConfig: + def test_defaults_without_file(self): + code, stdout, stderr = run_config("dump") + assert code == 0 + data = json.loads(stdout) + assert data["BTRFS_MOUNT"] == "/run/atomic/temp_root" + assert data["KEEP_GENERATIONS"] == "3" + assert data["CHROOT_COMMAND"] == "/usr/bin/pacman -Syu" + + def test_simple_key_value(self): + config_path = create_temp_config("BTRFS_MOUNT=/custom/mount\n") + run_config.config_path = config_path + try: + code, stdout, stderr = run_config("dump") + assert code == 0 + data = json.loads(stdout) + assert data["BTRFS_MOUNT"] == "/custom/mount" + finally: + os.unlink(config_path) + + def test_quoted_value_single(self): + config_path = create_temp_config("CHROOT_COMMAND='/usr/bin/pacman -Syu'\n") + run_config.config_path = config_path + try: + code, stdout, stderr = run_config("dump") + assert code == 0 + data = json.loads(stdout) + assert data["CHROOT_COMMAND"] == "/usr/bin/pacman -Syu" + finally: + os.unlink(config_path) + + def test_quoted_value_double(self): + config_path = create_temp_config('CHROOT_COMMAND="/usr/bin/pacman -Syu"\n') + run_config.config_path = config_path + try: + code, stdout, stderr = run_config("dump") + assert code == 0 + data = json.loads(stdout) + assert data["CHROOT_COMMAND"] == "/usr/bin/pacman -Syu" + finally: + os.unlink(config_path) + + def test_inline_comment(self): + config_path = create_temp_config("ESP=/efi # this is a comment\n") + run_config.config_path = config_path + try: + code, stdout, stderr = run_config("dump") + assert code == 0 + data = json.loads(stdout) + assert data["ESP"] == "/efi" + finally: + os.unlink(config_path) + + def test_shlex_quote_handling(self): + config_path = create_temp_config('CHROOT_COMMAND=pacman -S "package with spaces"\n') + run_config.config_path = config_path + try: + code, stdout, stderr = run_config("dump") + assert code == 0 + data = json.loads(stdout) + assert data["CHROOT_COMMAND"] == 'pacman -S "package with spaces"' + finally: + os.unlink(config_path) + + def test_unknown_key_ignored(self): + config_path = create_temp_config("UNKNOWN_KEY=value\n") + run_config.config_path = config_path + try: + code, stdout, stderr = run_config("dump") + assert code == 0 + assert "WARN" in stderr + data = json.loads(stdout) + assert "UNKNOWN_KEY" not in data + finally: + os.unlink(config_path) + + def test_comment_lines_skipped(self): + config_path = create_temp_config("# This is a comment\nESP=/boot/efi\n") + run_config.config_path = config_path + try: + code, stdout, stderr = run_config("dump") + assert code == 0 + data = json.loads(stdout) + assert data["ESP"] == "/boot/efi" + finally: + os.unlink(config_path) + + def test_config_not_owned_by_root(self): + config_path = create_temp_config("ESP=/efi\n") + run_config.config_path = config_path + try: + if os.geteuid() == 0: + os.chown(config_path, 1000, -1) + code, stdout, stderr = run_config("dump") + assert code == 1 + assert "not owned by root" in stderr + finally: + os.unlink(config_path) + + +class TestKeyLookup: + def test_valid_key(self): + code, stdout, stderr = run_config("CHROOT_COMMAND") + assert code == 0 + assert "pacman" in stdout + + def test_invalid_key(self): + code, stdout, stderr = run_config("NONEXISTENT") + assert code == 1 + assert "ERROR" in stderr + + +class TestValidate: + def test_valid_config(self): + code, stdout, stderr = run_config("validate") + assert code == 0 + assert "Config valid" in stdout + + def test_invalid_keep_generations(self): + config_path = create_temp_config("KEEP_GENERATIONS=abc\n") + run_config.config_path = config_path + try: + code, stdout, stderr = run_config("validate") + assert code == 1 + finally: + os.unlink(config_path) + + +class TestShellOutput: + def test_shell_output_format(self): + code, stdout, stderr = run_config("shell") + assert code == 0 + lines = stdout.split("\n") + assert any(line.startswith("BTRFS_MOUNT=") for line in lines) + assert any(line.startswith("KEEP_GENERATIONS=") for line in lines) + + +class TestArrayOutput: + def test_array_simple(self): + config_path = create_temp_config("CHROOT_COMMAND=/usr/bin/pacman -Syu\n") + run_config.config_path = config_path + try: + code, stdout, stderr = run_config("array", "CHROOT_COMMAND") + assert code == 0 + tokens = stdout.split("\0") + assert "pacman" in tokens[0] + finally: + os.unlink(config_path) + + def test_array_with_quoted_spaces(self): + config_path = create_temp_config('CHROOT_COMMAND=pacman -S "package with spaces"\n') + run_config.config_path = config_path + try: + code, stdout, stderr = run_config("array", "CHROOT_COMMAND") + assert code == 0 + tokens = [t for t in stdout.split("\0") if t] + assert "package with spaces" in tokens + finally: + os.unlink(config_path) diff --git a/tests/test_config.sh b/tests/test_config.sh deleted file mode 100644 index f8444f8..0000000 --- a/tests/test_config.sh +++ /dev/null @@ -1,329 +0,0 @@ -#!/usr/bin/env bash -# tests/test_config.sh — Config loading, parsing, security -# Run: bash tests/test_config.sh - -source "$(dirname "${BASH_SOURCE[0]}")/test_harness.sh" - - -# ── Default values after sourcing ──────────────────────────── - -section "Default values" - -assert_eq "BTRFS_MOUNT default" "/run/atomic/temp_root" "$BTRFS_MOUNT" -assert_eq "NEW_ROOT default" "/run/atomic/newroot" "$NEW_ROOT" -assert_eq "ESP default" "/efi" "$ESP" -assert_eq "KEEP_GENERATIONS default" "3" "$KEEP_GENERATIONS" -assert_eq "MAPPER_NAME default" "root_crypt" "$MAPPER_NAME" -assert_eq "KERNEL_PKG default" "linux" "$KERNEL_PKG" -assert_eq "LOCK_DIR default" "/run/atomic" "$LOCK_DIR" -assert_eq "LOCK_FILE default" "/run/atomic/atomic-upgrade.lock" "$LOCK_FILE" -assert_eq "SBCTL_SIGN default" "0" "$SBCTL_SIGN" -assert_eq "UPGRADE_GUARD default" "1" "$UPGRADE_GUARD" -assert_eq "HOME_COPY_FILES default" "" "$HOME_COPY_FILES" -assert_match "KERNEL_PARAMS contains rw" "rw" "$KERNEL_PARAMS" -assert_match "KERNEL_PARAMS contains pti=on" "pti=on" "$KERNEL_PARAMS" - - -# ── load_config() ─────────────────────────────────────────── - -section "load_config" - -# Test: config file does not exist → success (no-op) -CONFIG_FILE="${TESTDIR}/no_such_file.conf" -assert_rc "missing config file → rc 0" 0 load_config - -# Test: valid config file — direct call so side effects -# (variable assignments) are visible in the parent shell -CONFIG_FILE="${TESTDIR}/good.conf" -cat > "$CONFIG_FILE" <<'EOF' -# This is a comment -KEEP_GENERATIONS=5 -ESP=/boot/efi -SBCTL_SIGN=1 -KERNEL_PKG=linux-zen -EOF -KEEP_GENERATIONS=3; ESP="/efi"; SBCTL_SIGN=0; KERNEL_PKG="linux" -load_config -assert_eq "KEEP_GENERATIONS loaded" "5" "$KEEP_GENERATIONS" -assert_eq "ESP loaded" "/boot/efi" "$ESP" -assert_eq "SBCTL_SIGN loaded" "1" "$SBCTL_SIGN" -assert_eq "KERNEL_PKG loaded" "linux-zen" "$KERNEL_PKG" - -# Restore defaults for subsequent tests -KEEP_GENERATIONS=3; ESP="/efi"; SBCTL_SIGN=0; KERNEL_PKG="linux" - -# Test: quoted values — both single and double quotes stripped -CONFIG_FILE="${TESTDIR}/quoted.conf" -cat > "$CONFIG_FILE" <<'EOF' -ESP="/boot/efi" -KERNEL_PKG='linux-lts' -EOF -load_config -assert_eq "double-quoted value" "/boot/efi" "$ESP" -assert_eq "single-quoted value" "linux-lts" "$KERNEL_PKG" -ESP="/efi"; KERNEL_PKG="linux" - -# Test: inline comments stripped -CONFIG_FILE="${TESTDIR}/inline.conf" -cat > "$CONFIG_FILE" <<'EOF' -KEEP_GENERATIONS=7 # keep seven -EOF -KEEP_GENERATIONS=3 -load_config -assert_eq "inline comment stripped" "7" "$KEEP_GENERATIONS" -KEEP_GENERATIONS=3 - -# Test: value containing # without leading space — must NOT be stripped -CONFIG_FILE="${TESTDIR}/hash_in_value.conf" -cat > "$CONFIG_FILE" <<'EOF' -KERNEL_PARAMS=rw console=ttyS0,115200n8#1 -EOF -KERNEL_PARAMS="default" -load_config -assert_eq "hash without space preserved" "rw console=ttyS0,115200n8#1" "$KERNEL_PARAMS" -KERNEL_PARAMS="rw slab_nomerge init_on_alloc=1 page_alloc.shuffle=1 pti=on vsyscall=none randomize_kstack_offset=on debugfs=off" - -# Test: ' #' inside value is treated as inline comment (documented limitation) -CONFIG_FILE="${TESTDIR}/space_hash_in_value.conf" -cat > "$CONFIG_FILE" <<'EOF' -KERNEL_PARAMS=rw console=ttyS0 #debug -EOF -KERNEL_PARAMS="default" -load_config -assert_eq "space-hash treated as comment (documented)" "rw console=ttyS0" "$KERNEL_PARAMS" -KERNEL_PARAMS="rw slab_nomerge init_on_alloc=1 page_alloc.shuffle=1 pti=on vsyscall=none randomize_kstack_offset=on debugfs=off" - -# Test: unknown key → warning on stderr, known keys still applied. -# Direct call for side effects; capture stderr to file. -CONFIG_FILE="${TESTDIR}/unknown.conf" -cat > "$CONFIG_FILE" <<'EOF' -EVIL_KEY=hacked -KEEP_GENERATIONS=2 -EOF -KEEP_GENERATIONS=3 -load_config 2>"${TESTDIR}/unknown_stderr.txt" -assert_eq "known key still works with unknown present" "2" "$KEEP_GENERATIONS" -_captured=$(cat "${TESTDIR}/unknown_stderr.txt") -assert_contains "warns about unknown key" "Unknown config key" "$_captured" -KEEP_GENERATIONS=3 - -# Test: config not owned by root → error, variables unchanged. -# run_cmd (subshell) is safe here: we verify value did NOT change. -CONFIG_FILE="${TESTDIR}/badowner.conf" -cat > "$CONFIG_FILE" <<'EOF' -KEEP_GENERATIONS=9 -EOF -make_mock stat " -if [[ \"\${1:-}\" == \"-c\" && \"\${2:-}\" == \"%u\" ]]; then - echo \"1000\" -else - exec \"${REAL_STAT}\" \"\$@\" -fi -" -KEEP_GENERATIONS=3 -run_cmd load_config -assert_eq "bad owner returns error" "1" "$_rc" -assert_eq "bad owner doesn't change value" "3" "$KEEP_GENERATIONS" -assert_contains "bad owner error message" "not owned by root" "$_out" - -# Restore stat mock to uid 0 -make_mock stat " -if [[ \"\${1:-}\" == \"-c\" && \"\${2:-}\" == \"%u\" ]]; then - echo \"0\" -else - exec \"${REAL_STAT}\" \"\$@\" -fi -" - - -# ── UPGRADE_GUARD config ───────────────────────────────────── - -section "UPGRADE_GUARD config" - -# Test: UPGRADE_GUARD=0 disables guard -CONFIG_FILE="${TESTDIR}/guard_off.conf" -cat > "$CONFIG_FILE" <<'EOF' -UPGRADE_GUARD=0 -EOF -UPGRADE_GUARD=1 -load_config -assert_eq "UPGRADE_GUARD=0 loaded" "0" "$UPGRADE_GUARD" - -# Test: UPGRADE_GUARD=1 explicitly enables guard -CONFIG_FILE="${TESTDIR}/guard_on.conf" -cat > "$CONFIG_FILE" <<'EOF' -UPGRADE_GUARD=1 -EOF -UPGRADE_GUARD=0 -load_config -assert_eq "UPGRADE_GUARD=1 loaded" "1" "$UPGRADE_GUARD" - -# Test: quoted UPGRADE_GUARD values -CONFIG_FILE="${TESTDIR}/guard_quoted.conf" -cat > "$CONFIG_FILE" <<'EOF' -UPGRADE_GUARD="0" -EOF -UPGRADE_GUARD=1 -load_config -assert_eq "UPGRADE_GUARD quoted value" "0" "$UPGRADE_GUARD" - -# Test: UPGRADE_GUARD absent → default preserved -CONFIG_FILE="${TESTDIR}/guard_absent.conf" -cat > "$CONFIG_FILE" <<'EOF' -KEEP_GENERATIONS=3 -EOF -UPGRADE_GUARD=1 -load_config -assert_eq "UPGRADE_GUARD absent → stays 1" "1" "$UPGRADE_GUARD" - -# Test: UPGRADE_GUARD=0 with inline comment -CONFIG_FILE="${TESTDIR}/guard_comment.conf" -cat > "$CONFIG_FILE" <<'EOF' -UPGRADE_GUARD=0 # disable protection -EOF -UPGRADE_GUARD=1 -load_config -assert_eq "UPGRADE_GUARD with inline comment" "0" "$UPGRADE_GUARD" - -# Restore -UPGRADE_GUARD=1 - - -# ── Config whitelist security ──────────────────────────────── - -section "Config security" - -# Attempt to set dangerous variables via config — -# they must be rejected by the whitelist -CONFIG_FILE="${TESTDIR}/evil.conf" -cat > "$CONFIG_FILE" <<'EOF' -PATH=/evil/bin -LD_PRELOAD=/evil.so -HOME=/evil -KEEP_GENERATIONS=42 -EOF -KEEP_GENERATIONS=3 -_save_path="$PATH" -_save_home="$HOME" -_save_ld="${LD_PRELOAD:-}" - -# Direct call — need side effects (KEEP_GENERATIONS=42 must apply) -load_config 2>/dev/null || true - -assert_eq "evil PATH ignored" "$_save_path" "$PATH" -assert_eq "evil HOME ignored" "$_save_home" "$HOME" -assert_eq "evil LD_PRELOAD ignored" "$_save_ld" "${LD_PRELOAD:-}" -assert_eq "whitelisted key still works" "42" "$KEEP_GENERATIONS" -KEEP_GENERATIONS=3 - -# UPGRADE_GUARD must not be injectable via non-whitelisted names -CONFIG_FILE="${TESTDIR}/evil_guard.conf" -cat > "$CONFIG_FILE" <<'EOF' -UPGRADE_GUARD_OVERRIDE=0 -upgrade_guard=0 -EOF -UPGRADE_GUARD=1 -load_config 2>/dev/null || true -assert_eq "UPGRADE_GUARD not affected by similar names" "1" "$UPGRADE_GUARD" - - -# ── KERNEL_PARAMS from config ─────────────────────────────── - -section "KERNEL_PARAMS config" - -CONFIG_FILE="${TESTDIR}/params.conf" -cat > "$CONFIG_FILE" <<'EOF' -KERNEL_PARAMS=rw quiet splash -EOF -KERNEL_PARAMS="rw default" -load_config -assert_eq "KERNEL_PARAMS overridden" "rw quiet splash" "$KERNEL_PARAMS" -# Restore default -KERNEL_PARAMS="rw slab_nomerge init_on_alloc=1 page_alloc.shuffle=1 pti=on vsyscall=none randomize_kstack_offset=on debugfs=off" - - -# ── HOME_COPY_FILES config ───────────────────── - -section "HOME_COPY_FILES config" - -# Test: HOME_COPY_FILES loaded from config -CONFIG_FILE="${TESTDIR}/home_copy.conf" -cat > "$CONFIG_FILE" <<'EOF' -HOME_COPY_FILES=.bashrc .ssh .gitconfig -EOF -HOME_COPY_FILES="" -load_config -assert_eq "HOME_COPY_FILES loaded" ".bashrc .ssh .gitconfig" "$HOME_COPY_FILES" - -# Test: HOME_COPY_FILES with quotes -CONFIG_FILE="${TESTDIR}/home_copy_quoted.conf" -cat > "$CONFIG_FILE" <<'EOF' -HOME_COPY_FILES=".bashrc .bash_profile .ssh" -EOF -HOME_COPY_FILES="" -load_config -assert_eq "HOME_COPY_FILES quoted" ".bashrc .bash_profile .ssh" "$HOME_COPY_FILES" - -# Test: HOME_COPY_FILES absent → default preserved -CONFIG_FILE="${TESTDIR}/home_copy_absent.conf" -cat > "$CONFIG_FILE" <<'EOF' -KEEP_GENERATIONS=3 -EOF -HOME_COPY_FILES="" -load_config -assert_eq "HOME_COPY_FILES absent → stays empty" "" "$HOME_COPY_FILES" - -# Restore -HOME_COPY_FILES="" - -# ── CHROOT_COMMAND config ─────────────────────────────────── - -section "CHROOT_COMMAND config" - -# Test: CHROOT_COMMAND loaded from config -CONFIG_FILE="${TESTDIR}/chroot_command.conf" -cat > "$CONFIG_FILE" <<'EOF' -CHROOT_COMMAND=/usr/bin/pacman -Syu -EOF -CHROOT_COMMAND="" -load_config -assert_eq "CHROOT_COMMAND loaded from config" "/usr/bin/pacman -Syu" "$CHROOT_COMMAND" -CHROOT_COMMAND="" - -# Test: CHROOT_COMMAND with quotes -CONFIG_FILE="${TESTDIR}/chroot_command_quoted.conf" -cat > "$CONFIG_FILE" <<'EOF' -CHROOT_COMMAND="/usr/bin/pacman -S nvidia" -EOF -CHROOT_COMMAND="" -load_config -assert_eq "CHROOT_COMMAND with double quotes" "/usr/bin/pacman -S nvidia" "$CHROOT_COMMAND" -CHROOT_COMMAND="" - -# Test: CHROOT_COMMAND absent → stays empty -CONFIG_FILE="${TESTDIR}/chroot_command_absent.conf" -cat > "$CONFIG_FILE" <<'EOF' -KEEP_GENERATIONS=3 -EOF -CHROOT_COMMAND="" -load_config -assert_eq "CHROOT_COMMAND absent → stays empty" "" "$CHROOT_COMMAND" - -# -# ── config key whitespace trimming ───────────── - -section "Config key whitespace trimming" - -CONFIG_FILE="${TESTDIR}/whitespace_key.conf" -printf ' KEEP_GENERATIONS=7\n' > "$CONFIG_FILE" -printf '\tESP=/boot/efi\n' >> "$CONFIG_FILE" -KEEP_GENERATIONS=3 -ESP="/efi" -load_config -assert_eq "leading spaces in key trimmed" "7" "$KEEP_GENERATIONS" -assert_eq "leading tab in key trimmed" "/boot/efi" "$ESP" -KEEP_GENERATIONS=3 -ESP="/efi" - - -summary From a8e406a23749510567240906e0e9952a9afc6cf0 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 14:09:49 +0300 Subject: [PATCH 02/23] fix: use CONFIG_FILE env in shell_output & shellcheck 2015 --- bin/atomic-upgrade | 7 ++++--- lib/atomic/common.sh | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/bin/atomic-upgrade b/bin/atomic-upgrade index 6ef087b..7a361c8 100644 --- a/bin/atomic-upgrade +++ b/bin/atomic-upgrade @@ -110,13 +110,14 @@ fi if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then [[ -z "${CHROOT_COMMAND:-}" ]] && CHROOT_COMMAND="/usr/bin/pacman -Syu" CHROOT_CMD=() - config_err=$(python3 "${LIBDIR}/config.py" array CHROOT_COMMAND 2>&1) && { + config_err=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${LIBDIR}/config.py" array CHROOT_COMMAND 2>&1) + if [[ $? -eq 0 ]]; then while IFS= read -r -d '' token; do CHROOT_CMD+=("$token") done <<< "$config_err" - } || { + else echo "WARN: Failed to parse CHROOT_COMMAND with config.py: $config_err" >&2 - } + fi if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then read -ra CHROOT_CMD <<< "$CHROOT_COMMAND" fi diff --git a/lib/atomic/common.sh b/lib/atomic/common.sh index 65186d5..c69aa04 100644 --- a/lib/atomic/common.sh +++ b/lib/atomic/common.sh @@ -40,7 +40,7 @@ load_config() { [[ -f "$CONFIG_FILE" ]] || return 0 local shell_output config_err - shell_output=$(python3 "${LIBDIR}/config.py" shell 2>&1) || { + shell_output=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${LIBDIR}/config.py" shell 2>&1) || { config_err="$shell_output" echo "ERROR: Failed to parse config with config.py" >&2 echo "ERROR: Details: $config_err" >&2 From da4bcb8a98dd1e9a548da1d723ac460ffe2dd4f6 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 15:46:55 +0300 Subject: [PATCH 03/23] fix(test): python mock --- tests/test_integration.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/test_integration.sh b/tests/test_integration.sh index 2ec6318..6143352 100644 --- a/tests/test_integration.sh +++ b/tests/test_integration.sh @@ -114,11 +114,21 @@ fi MOCK chmod +x "${MOCK_BIN}/stat" -for cmd in mountpoint mount btrfs df python3; do +for cmd in mountpoint mount btrfs df; do printf '#!/bin/bash\nexit 0\n' > "${MOCK_BIN}/${cmd}" chmod +x "${MOCK_BIN}/${cmd}" done +REAL_PYTHON3=$(command -v python3) +cat > "${MOCK_BIN}/python3" < "${MOCK_BIN}/findmnt" <<'MOCK' #!/bin/bash echo "rw,subvol=/root-20250601-120000" From acf2b2d11f0c9817d2dd6a3563d6a092d2c7aef2 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 15:56:31 +0300 Subject: [PATCH 04/23] fix(common.sh): autodetect LIBDIR --- lib/atomic/common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/atomic/common.sh b/lib/atomic/common.sh index c69aa04..92ff792 100644 --- a/lib/atomic/common.sh +++ b/lib/atomic/common.sh @@ -7,7 +7,7 @@ # ── Library directory ────────────────────────────────────────────── if [[ -z "${LIBDIR:-}" ]]; then - LIBDIR="/usr/lib/atomic" + LIBDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" fi # ── Project version ──────────────────────────────────────────────── From 1afa138b37e63d37a7fc169541af2bcbeab53319 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 16:05:40 +0300 Subject: [PATCH 05/23] fix: CHROOT_COMMAND parsing with process substitution --- bin/atomic-upgrade | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/bin/atomic-upgrade b/bin/atomic-upgrade index 7a361c8..6d29ef0 100644 --- a/bin/atomic-upgrade +++ b/bin/atomic-upgrade @@ -110,14 +110,11 @@ fi if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then [[ -z "${CHROOT_COMMAND:-}" ]] && CHROOT_COMMAND="/usr/bin/pacman -Syu" CHROOT_CMD=() - config_err=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${LIBDIR}/config.py" array CHROOT_COMMAND 2>&1) - if [[ $? -eq 0 ]]; then - while IFS= read -r -d '' token; do - CHROOT_CMD+=("$token") - done <<< "$config_err" - else - echo "WARN: Failed to parse CHROOT_COMMAND with config.py: $config_err" >&2 - fi + # Read null-delimited output directly via process substitution to preserve null bytes + while IFS= read -r -d '' token; do + CHROOT_CMD+=("$token") + done < <(CONFIG_FILE="${CONFIG_FILE}" python3 "${LIBDIR}/config.py" array CHROOT_COMMAND 2>/dev/null) + if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then read -ra CHROOT_CMD <<< "$CHROOT_COMMAND" fi From e9bbd6c7159ccf9e247246a7e7d84cd4eb152968 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 16:29:25 +0300 Subject: [PATCH 06/23] fix(test_harness): python mock --- tests/test_harness.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_harness.sh b/tests/test_harness.sh index 588d31e..122f122 100644 --- a/tests/test_harness.sh +++ b/tests/test_harness.sh @@ -228,7 +228,14 @@ fi make_mock findmnt 'echo ""' make_mock mountpoint 'exit 0' -make_mock python3 'echo ""' +REAL_PYTHON3=$(command -v python3) +cat > "${MOCK_BIN}/python3" < Date: Sun, 3 May 2026 16:34:07 +0300 Subject: [PATCH 07/23] fix(test_harness): python3 mock for config.py calls --- tests/test_harness.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_harness.sh b/tests/test_harness.sh index 122f122..4a88098 100644 --- a/tests/test_harness.sh +++ b/tests/test_harness.sh @@ -229,13 +229,15 @@ fi make_mock findmnt 'echo ""' make_mock mountpoint 'exit 0' REAL_PYTHON3=$(command -v python3) -cat > "${MOCK_BIN}/python3" < "${MOCK_BIN}/python3" <<'MOCK' #!/bin/bash for arg in "$@"; do - [[ "$arg" == *config.py* ]] && exec ${REAL_PYTHON3} "$@" + [[ "$arg" == *config.py* ]] && exec REAL_PYTHON3_PLACEHOLDER "$@" done exit 0 MOCK +sed -i "s|REAL_PYTHON3_PLACEHOLDER|${REAL_PYTHON3}|g" "${MOCK_BIN}/python3" +chmod +x "${MOCK_BIN}/python3" make_mock mount 'exit 0' make_mock btrfs 'exit 0' make_mock flock 'exit 0' From 86406c3b372f6ef26bf7fab98d7b22926853ee9f Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 17:17:19 +0300 Subject: [PATCH 08/23] refactor: replace LIBDIR with _ATOMIC_LIB_DIR for internal use --- bin/atomic-gc | 2 +- bin/atomic-guard | 2 +- bin/atomic-rebuild-uki | 2 +- bin/atomic-upgrade | 9 ++++++--- lib/atomic/common.sh | 20 ++++++++------------ 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/bin/atomic-gc b/bin/atomic-gc index 85583bf..abfc1ea 100644 --- a/bin/atomic-gc +++ b/bin/atomic-gc @@ -3,7 +3,7 @@ set -euo pipefail -readonly LIBDIR="/usr/lib/atomic" +LIBDIR="/usr/lib/atomic" _src() { local p; p=$(verify-lib "$1" "$LIBDIR/") && source "$p" || exit 1; } _src "${LIBDIR}/common.sh" diff --git a/bin/atomic-guard b/bin/atomic-guard index d81e8a9..ed8eaf9 100644 --- a/bin/atomic-guard +++ b/bin/atomic-guard @@ -7,7 +7,7 @@ # Note: no "set -euo pipefail" here — the script relies on conditional # checks and process-tree walking that would break with errexit. -readonly LIBDIR="/usr/lib/atomic" +LIBDIR="/usr/lib/atomic" # Load common.sh with graceful degradation _COMMON_LOADED=0 diff --git a/bin/atomic-rebuild-uki b/bin/atomic-rebuild-uki index ae4ae18..258d4cd 100644 --- a/bin/atomic-rebuild-uki +++ b/bin/atomic-rebuild-uki @@ -6,7 +6,7 @@ set -euo pipefail -readonly LIBDIR="/usr/lib/atomic" +LIBDIR="/usr/lib/atomic" _src() { local p; p=$(verify-lib "$1" "$LIBDIR/") && source "$p" || exit 1; } _src "${LIBDIR}/common.sh" diff --git a/bin/atomic-upgrade b/bin/atomic-upgrade index 6d29ef0..d91868a 100644 --- a/bin/atomic-upgrade +++ b/bin/atomic-upgrade @@ -7,8 +7,11 @@ set -euo pipefail -readonly LIBDIR="/usr/lib/atomic" -_src() { local p; p=$(verify-lib "$1" "$LIBDIR/") && source "$p" || exit 1; } +LIBDIR="/usr/lib/atomic" + +_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 @@ -113,7 +116,7 @@ if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then # Read null-delimited output directly via process substitution to preserve null bytes while IFS= read -r -d '' token; do CHROOT_CMD+=("$token") - done < <(CONFIG_FILE="${CONFIG_FILE}" python3 "${LIBDIR}/config.py" array CHROOT_COMMAND 2>/dev/null) + done < <(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" array CHROOT_COMMAND 2>/dev/null) if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then read -ra CHROOT_CMD <<< "$CHROOT_COMMAND" diff --git a/lib/atomic/common.sh b/lib/atomic/common.sh index 92ff792..2f0bd9b 100644 --- a/lib/atomic/common.sh +++ b/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="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -fi +_ATOMIC_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # ── Project version ──────────────────────────────────────────────── @@ -40,7 +36,7 @@ load_config() { [[ -f "$CONFIG_FILE" ]] || return 0 local shell_output config_err - shell_output=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${LIBDIR}/config.py" shell 2>&1) || { + shell_output=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" shell 2>&1) || { config_err="$shell_output" echo "ERROR: Failed to parse config with config.py" >&2 echo "ERROR: Details: $config_err" >&2 @@ -95,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: @@ -113,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 @@ -243,7 +239,7 @@ verify_uki() { # ── fstab update (delegates to Python for safety) ────────────────── update_fstab() { - python3 "${LIBDIR}/fstab.py" "$@" + python3 "${_ATOMIC_LIB_DIR}/fstab.py" "$@" } # ── fstab /home update ────────────────────────────────────────── @@ -251,7 +247,7 @@ update_fstab() { # 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 ──────────────────────────────────── @@ -357,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" @@ -535,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 } From ee5f09866a59ee2b25dba4efb2778f939671694d Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 17:59:24 +0300 Subject: [PATCH 09/23] fix(atomic-upgrade): restore stderr for config.py array call --- bin/atomic-upgrade | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/atomic-upgrade b/bin/atomic-upgrade index d91868a..f4d0b5d 100644 --- a/bin/atomic-upgrade +++ b/bin/atomic-upgrade @@ -116,7 +116,7 @@ if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then # Read null-delimited output directly via process substitution to preserve null bytes while IFS= read -r -d '' token; do CHROOT_CMD+=("$token") - done < <(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" array CHROOT_COMMAND 2>/dev/null) + done < <(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" array CHROOT_COMMAND) if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then read -ra CHROOT_CMD <<< "$CHROOT_COMMAND" From f467c368c458cc2c21bf321c9d08fe685c218cba Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 18:00:51 +0300 Subject: [PATCH 10/23] test: add shell output escape test --- tests/test_config.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_config.py b/tests/test_config.py index 5217a0b..267b4a2 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -174,6 +174,18 @@ def test_shell_output_format(self): assert any(line.startswith("BTRFS_MOUNT=") for line in lines) assert any(line.startswith("KEEP_GENERATIONS=") for line in lines) + def test_shell_output_escapes_spaces(self): + config_path = create_temp_config('CHROOT_COMMAND=pacman -S "package with spaces"\n') + run_config.config_path = config_path + try: + code, stdout, stderr = run_config("shell") + assert code == 0 + lines = stdout.split("\n") + cmd_line = [l for l in lines if l.startswith("CHROOT_COMMAND=")][0] + assert "package with spaces" in cmd_line + finally: + os.unlink(config_path) + class TestArrayOutput: def test_array_simple(self): From 0e5e980f6d862d1063fb3a40cc39c2426bedad29 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 18:01:20 +0300 Subject: [PATCH 11/23] docs: sync readme with new python module --- tests/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/README.md b/tests/README.md index 7a76592..9d39446 100644 --- a/tests/README.md +++ b/tests/README.md @@ -4,7 +4,7 @@ | File | Language | Framework | What it tests | |------|----------|-----------|---------------| -| `test_config.sh` | Bash | Custom assertions | Config loading, parsing, quoting, inline comments, ownership check, unknown key rejection, whitelist security, UPGRADE_GUARD, KERNEL_PARAMS, HOME_COPY_FILES, key whitespace trimming | +| `test_config.py` | Python | pytest | `config.py` — parse_config (quoting, inline comments, unknown keys, ownership), key lookup, validate, shell output (format + escaping), array output (simple + quoted spaces) | | `test_validate.sh` | Bash | Custom assertions | Function definitions, `validate_config` (numeric checks, path conflicts, ESP mount), `check_dependencies` (missing commands, LUKS→cryptsetup, SBCTL_SIGN→sbctl), `is_child_of_aur_helper` | | `test_device.sh` | Bash | Custom assertions | `get_current_subvol`/`get_current_subvol_raw` with various findmnt outputs, `get_root_device` (python helper, caching, fallback), `ensure_btrfs_mounted`, `validate_subvolume` | | `test_space.sh` | Bash | Custom assertions | `check_esp_space` (sufficient/low/df-failure), `check_btrfs_space` (native/df-fallback/both-fail, absolute minimum threshold, non-numeric btrfs output) | @@ -27,7 +27,7 @@ make test # Individual suites -bash tests/test_config.sh +python -m pytest tests/test_config.py -v bash tests/test_validate.sh bash tests/test_device.sh bash tests/test_space.sh @@ -44,7 +44,7 @@ python -m pytest tests/test_rootdev.py -v ## How they work -### Bash unit tests (`test_config.sh` .. `test_rebuild_uki.sh`) +### Bash unit tests (`test_validate.sh` .. `test_rebuild_uki.sh`) All unit test files source `test_harness.sh`, which provides: From d8c672b4cabb756365c179473c5724e380a86698 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 18:33:24 +0300 Subject: [PATCH 12/23] fix: resolve symlinks for ownership check --- lib/atomic/config.py | 17 ++++++++++++----- tests/test_config.py | 28 +++++++++++++++++++--------- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/lib/atomic/config.py b/lib/atomic/config.py index 87e46dd..f4f57ca 100644 --- a/lib/atomic/config.py +++ b/lib/atomic/config.py @@ -3,7 +3,8 @@ /usr/lib/atomic/config.py Parse /etc/atomic.conf with proper shell-style quote handling. -Uses shlex for correct parsing of single/double quotes and escapes. +Strips outer quotes (single/double) and handles inline comments. +Uses shlex only in config_to_array() for safe tokenization. Usage: config.py -> dump all config as JSON @@ -35,7 +36,11 @@ def parse_config(path=None): - """Parse atomic.conf with proper quote handling via shlex.""" + """Parse atomic.conf with proper quote handling. + + Strips outer quotes (single/double) and handles inline comments. + Uses shlex only in config_to_array() for safe tokenization. + """ config = dict(DEFAULT_CONFIG) if path is None: path = os.environ.get("CONFIG_FILE", "/etc/atomic.conf") @@ -44,11 +49,13 @@ def parse_config(path=None): return config # Only check owner for /etc/atomic.conf (system config). + # Resolve symlinks so that CONFIG_FILE set to a symlink pointing + # to /etc/atomic.conf is still correctly validated. # Other paths (e.g. test configs) are allowed to have any owner. - if path == "/etc/atomic.conf": + if config_path.resolve() == Path("/etc/atomic.conf"): try: - if config_path.stat().st_uid != 0: - print(f"ERROR: {path} not owned by root", file=sys.stderr) + if config_path.resolve().stat().st_uid != 0: + print(f"ERROR: {config_path.resolve()} not owned by root", file=sys.stderr) sys.exit(1) except OSError as e: print(f"ERROR: Cannot read {path}: {e}", file=sys.stderr) diff --git a/tests/test_config.py b/tests/test_config.py index 267b4a2..acd8b4a 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -125,17 +125,27 @@ def test_comment_lines_skipped(self): finally: os.unlink(config_path) + @pytest.mark.skipif(os.geteuid() != 0, reason="requires root") def test_config_not_owned_by_root(self): - config_path = create_temp_config("ESP=/efi\n") - run_config.config_path = config_path + # Ownership check only triggers for /etc/atomic.conf. + # Back up existing file, write test config, restore after. + real_path = Path("/etc/atomic.conf") + backup = None + if real_path.exists(): + backup = real_path.read_text() try: - if os.geteuid() == 0: - os.chown(config_path, 1000, -1) - code, stdout, stderr = run_config("dump") - assert code == 1 - assert "not owned by root" in stderr + real_path.write_text("ESP=/efi\n") + os.chown(str(real_path), 1000, -1) + run_config.config_path = str(real_path) + code, stdout, stderr = run_config("dump") + assert code == 1 + assert "not owned by root" in stderr finally: - os.unlink(config_path) + if backup is not None: + real_path.write_text(backup) + os.chown(str(real_path), 0, 0) + elif real_path.exists(): + real_path.unlink() class TestKeyLookup: @@ -181,7 +191,7 @@ def test_shell_output_escapes_spaces(self): code, stdout, stderr = run_config("shell") assert code == 0 lines = stdout.split("\n") - cmd_line = [l for l in lines if l.startswith("CHROOT_COMMAND=")][0] + cmd_line = [line for line in lines if line.startswith("CHROOT_COMMAND=")][0] assert "package with spaces" in cmd_line finally: os.unlink(config_path) From 07179ef711233f6d7975703654bb6f24cb766209 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 18:33:35 +0300 Subject: [PATCH 13/23] docs: fix groff --- man/atomic.conf.5 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/atomic.conf.5 b/man/atomic.conf.5 index a32cfe2..961bd62 100644 --- a/man/atomic.conf.5 +++ b/man/atomic.conf.5 @@ -74,7 +74,7 @@ pti=on vsyscall=none randomize_kstack_offset=on debugfs=off\f[R]. \f[B]CHROOT_COMMAND\f[R] Default command to run in the snapshot chroot, overriding the built\-in default of \f[B]pacman \-Syu\f[R]. -Command\-line \f[B]\[en] CHROOT_COMMAND\&...\f[R] takes priority over +Command\-line \f[B]-- CHROOT_COMMAND\&...\f[R] takes priority over this setting. Arguments with spaces are supported via proper shell\-style quote handling. From 627876e4c4d10ae45976ec8e3b797827687415a1 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 18:42:30 +0300 Subject: [PATCH 14/23] makefile: add test-root target --- Makefile | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 92e6ede..d9ec7e6 100644 --- a/Makefile +++ b/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 @@ -55,6 +55,20 @@ test: 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 install -Dm755 bin/atomic-gc $(DESTDIR)$(BINDIR)/atomic-gc From 5694340a676d5a04e1fafb60a575c97ebdeb8131 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 18:42:38 +0300 Subject: [PATCH 15/23] ci: add python-test-root job --- .github/workflows/ci.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5b6a47f..d098ce8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -101,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' From e17d7d8f18219a34732a7778b8f0015658726171 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 22:03:20 +0300 Subject: [PATCH 16/23] refactor: restore readonly for LIBDIR variable --- bin/atomic-gc | 2 +- bin/atomic-guard | 2 +- bin/atomic-rebuild-uki | 2 +- bin/atomic-upgrade | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bin/atomic-gc b/bin/atomic-gc index abfc1ea..85583bf 100644 --- a/bin/atomic-gc +++ b/bin/atomic-gc @@ -3,7 +3,7 @@ set -euo pipefail -LIBDIR="/usr/lib/atomic" +readonly LIBDIR="/usr/lib/atomic" _src() { local p; p=$(verify-lib "$1" "$LIBDIR/") && source "$p" || exit 1; } _src "${LIBDIR}/common.sh" diff --git a/bin/atomic-guard b/bin/atomic-guard index ed8eaf9..d81e8a9 100644 --- a/bin/atomic-guard +++ b/bin/atomic-guard @@ -7,7 +7,7 @@ # Note: no "set -euo pipefail" here — the script relies on conditional # checks and process-tree walking that would break with errexit. -LIBDIR="/usr/lib/atomic" +readonly LIBDIR="/usr/lib/atomic" # Load common.sh with graceful degradation _COMMON_LOADED=0 diff --git a/bin/atomic-rebuild-uki b/bin/atomic-rebuild-uki index 258d4cd..ae4ae18 100644 --- a/bin/atomic-rebuild-uki +++ b/bin/atomic-rebuild-uki @@ -6,7 +6,7 @@ set -euo pipefail -LIBDIR="/usr/lib/atomic" +readonly LIBDIR="/usr/lib/atomic" _src() { local p; p=$(verify-lib "$1" "$LIBDIR/") && source "$p" || exit 1; } _src "${LIBDIR}/common.sh" diff --git a/bin/atomic-upgrade b/bin/atomic-upgrade index f4d0b5d..9235e2f 100644 --- a/bin/atomic-upgrade +++ b/bin/atomic-upgrade @@ -7,7 +7,7 @@ set -euo pipefail -LIBDIR="/usr/lib/atomic" +readonly LIBDIR="/usr/lib/atomic" _src() { local p; p=$(verify-lib "$1" "$LIBDIR/") && source "$p" || exit 1 From 1e950edcee9c8b74a50d95c0dde2e03c14eb4b86 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 22:03:35 +0300 Subject: [PATCH 17/23] fix(atomic-upgrade): improve config.py error handling with temp file --- bin/atomic-upgrade | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/bin/atomic-upgrade b/bin/atomic-upgrade index 9235e2f..b1f6722 100644 --- a/bin/atomic-upgrade +++ b/bin/atomic-upgrade @@ -113,10 +113,18 @@ fi if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then [[ -z "${CHROOT_COMMAND:-}" ]] && CHROOT_COMMAND="/usr/bin/pacman -Syu" CHROOT_CMD=() - # Read null-delimited output directly via process substitution to preserve null bytes - while IFS= read -r -d '' token; do - CHROOT_CMD+=("$token") - done < <(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" array CHROOT_COMMAND) + # 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" if [[ ${#CHROOT_CMD[@]} -eq 0 ]]; then read -ra CHROOT_CMD <<< "$CHROOT_COMMAND" From 638f7b8121c669d92361e9d3f8dc5c461b73dc07 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 22:04:35 +0300 Subject: [PATCH 18/23] fix(common.sh): improve config.py error handling in load_config --- lib/atomic/common.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/atomic/common.sh b/lib/atomic/common.sh index 2f0bd9b..ab01f68 100644 --- a/lib/atomic/common.sh +++ b/lib/atomic/common.sh @@ -36,12 +36,13 @@ load_config() { [[ -f "$CONFIG_FILE" ]] || return 0 local shell_output config_err - shell_output=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" shell 2>&1) || { - config_err="$shell_output" + shell_output=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" shell 2>/dev/null) + if [[ $? -ne 0 ]]; 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" >&2 + echo "ERROR: Details: ${config_err:-unknown}" >&2 return 1 - } + fi while IFS='=' read -r key value; do [[ -z "$key" || "$key" == \#* ]] && continue From f00c6e9869866b8ba5530257a5e7647199d7ac4e Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 22:05:25 +0300 Subject: [PATCH 19/23] refactor(config.py): tests and numeric validation --- lib/atomic/config.py | 22 ++-- tests/test_config.py | 242 ++++++++++++++++++++----------------------- 2 files changed, 118 insertions(+), 146 deletions(-) diff --git a/lib/atomic/config.py b/lib/atomic/config.py index f4f57ca..7288174 100644 --- a/lib/atomic/config.py +++ b/lib/atomic/config.py @@ -124,21 +124,13 @@ def main(): if command == "validate": config = parse_config() - try: - int(config["KEEP_GENERATIONS"]) - except ValueError: - print("ERROR: KEEP_GENERATIONS must be a number", file=sys.stderr) - return 1 - try: - int(config["SBCTL_SIGN"]) - except ValueError: - print("ERROR: SBCTL_SIGN must be a number", file=sys.stderr) - return 1 - try: - int(config["UPGRADE_GUARD"]) - except ValueError: - print("ERROR: UPGRADE_GUARD must be a number", file=sys.stderr) - return 1 + numeric_keys = ["KEEP_GENERATIONS", "SBCTL_SIGN", "UPGRADE_GUARD"] + for key in numeric_keys: + try: + int(config[key]) + except ValueError: + print(f"ERROR: {key} must be a number", file=sys.stderr) + return 1 print("Config valid") return 0 diff --git a/tests/test_config.py b/tests/test_config.py index acd8b4a..a7c936c 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -11,10 +11,17 @@ CONFIG_SCRIPT = LIBDIR / "config.py" -def run_config(*args): +@pytest.fixture +def temp_config(tmp_path): + """Create a temporary config file and yield its path.""" + config_path = tmp_path / "atomic.conf" + yield str(config_path) + + +def run_config(*args, config_path=None): env = os.environ.copy() - if hasattr(run_config, 'config_path'): - env['CONFIG_FILE'] = run_config.config_path + if config_path is not None: + env['CONFIG_FILE'] = config_path result = subprocess.run( ["python3", str(CONFIG_SCRIPT)] + list(args), capture_output=True, @@ -25,10 +32,9 @@ def run_config(*args): return result.returncode, result.stdout.strip(), result.stderr.strip() -def create_temp_config(content, owner_uid=0): - with tempfile.NamedTemporaryFile(mode="w", suffix=".conf", delete=False) as f: +def create_temp_config(path, content, owner_uid=0): + with open(path, "w") as f: f.write(content) - path = f.name os.chmod(path, 0o644) if owner_uid != 0: try: @@ -39,113 +45,88 @@ def create_temp_config(content, owner_uid=0): class TestParseConfig: - def test_defaults_without_file(self): - code, stdout, stderr = run_config("dump") + def test_defaults_without_file(self, temp_config): + code, stdout, stderr = run_config("dump", config_path=temp_config) assert code == 0 data = json.loads(stdout) assert data["BTRFS_MOUNT"] == "/run/atomic/temp_root" assert data["KEEP_GENERATIONS"] == "3" assert data["CHROOT_COMMAND"] == "/usr/bin/pacman -Syu" - def test_simple_key_value(self): - config_path = create_temp_config("BTRFS_MOUNT=/custom/mount\n") - run_config.config_path = config_path - try: - code, stdout, stderr = run_config("dump") - assert code == 0 - data = json.loads(stdout) - assert data["BTRFS_MOUNT"] == "/custom/mount" - finally: - os.unlink(config_path) + def test_simple_key_value(self, temp_config): + config_path = temp_config + create_temp_config(config_path, "BTRFS_MOUNT=/custom/mount\n") + code, stdout, stderr = run_config("dump", config_path=config_path) + assert code == 0 + data = json.loads(stdout) + assert data["BTRFS_MOUNT"] == "/custom/mount" - def test_quoted_value_single(self): - config_path = create_temp_config("CHROOT_COMMAND='/usr/bin/pacman -Syu'\n") - run_config.config_path = config_path - try: - code, stdout, stderr = run_config("dump") - assert code == 0 - data = json.loads(stdout) - assert data["CHROOT_COMMAND"] == "/usr/bin/pacman -Syu" - finally: - os.unlink(config_path) + def test_quoted_value_single(self, temp_config): + config_path = temp_config + create_temp_config(config_path, "CHROOT_COMMAND='/usr/bin/pacman -Syu'\n") + code, stdout, stderr = run_config("dump", config_path=config_path) + assert code == 0 + data = json.loads(stdout) + assert data["CHROOT_COMMAND"] == "/usr/bin/pacman -Syu" - def test_quoted_value_double(self): - config_path = create_temp_config('CHROOT_COMMAND="/usr/bin/pacman -Syu"\n') - run_config.config_path = config_path - try: - code, stdout, stderr = run_config("dump") - assert code == 0 - data = json.loads(stdout) - assert data["CHROOT_COMMAND"] == "/usr/bin/pacman -Syu" - finally: - os.unlink(config_path) + def test_quoted_value_double(self, temp_config): + config_path = temp_config + create_temp_config(config_path, 'CHROOT_COMMAND="/usr/bin/pacman -Syu"\n') + code, stdout, stderr = run_config("dump", config_path=config_path) + assert code == 0 + data = json.loads(stdout) + assert data["CHROOT_COMMAND"] == "/usr/bin/pacman -Syu" - def test_inline_comment(self): - config_path = create_temp_config("ESP=/efi # this is a comment\n") - run_config.config_path = config_path - try: - code, stdout, stderr = run_config("dump") - assert code == 0 - data = json.loads(stdout) - assert data["ESP"] == "/efi" - finally: - os.unlink(config_path) + def test_inline_comment(self, temp_config): + config_path = temp_config + create_temp_config(config_path, "ESP=/efi # this is a comment\n") + code, stdout, stderr = run_config("dump", config_path=config_path) + assert code == 0 + data = json.loads(stdout) + assert data["ESP"] == "/efi" - def test_shlex_quote_handling(self): - config_path = create_temp_config('CHROOT_COMMAND=pacman -S "package with spaces"\n') - run_config.config_path = config_path - try: - code, stdout, stderr = run_config("dump") - assert code == 0 - data = json.loads(stdout) - assert data["CHROOT_COMMAND"] == 'pacman -S "package with spaces"' - finally: - os.unlink(config_path) + def test_shlex_quote_handling(self, temp_config): + config_path = temp_config + create_temp_config(config_path, 'CHROOT_COMMAND=pacman -S "package with spaces"\n') + code, stdout, stderr = run_config("dump", config_path=config_path) + assert code == 0 + data = json.loads(stdout) + assert data["CHROOT_COMMAND"] == 'pacman -S "package with spaces"' - def test_unknown_key_ignored(self): - config_path = create_temp_config("UNKNOWN_KEY=value\n") - run_config.config_path = config_path - try: - code, stdout, stderr = run_config("dump") - assert code == 0 - assert "WARN" in stderr - data = json.loads(stdout) - assert "UNKNOWN_KEY" not in data - finally: - os.unlink(config_path) + def test_unknown_key_ignored(self, temp_config): + config_path = temp_config + create_temp_config(config_path, "UNKNOWN_KEY=value\n") + code, stdout, stderr = run_config("dump", config_path=config_path) + assert code == 0 + assert "WARN" in stderr + data = json.loads(stdout) + assert "UNKNOWN_KEY" not in data - def test_comment_lines_skipped(self): - config_path = create_temp_config("# This is a comment\nESP=/boot/efi\n") - run_config.config_path = config_path - try: - code, stdout, stderr = run_config("dump") - assert code == 0 - data = json.loads(stdout) - assert data["ESP"] == "/boot/efi" - finally: - os.unlink(config_path) + def test_comment_lines_skipped(self, temp_config): + config_path = temp_config + create_temp_config(config_path, "# This is a comment\nESP=/boot/efi\n") + code, stdout, stderr = run_config("dump", config_path=config_path) + assert code == 0 + data = json.loads(stdout) + assert data["ESP"] == "/boot/efi" @pytest.mark.skipif(os.geteuid() != 0, reason="requires root") - def test_config_not_owned_by_root(self): + def test_config_not_owned_by_root(self, tmp_path): # Ownership check only triggers for /etc/atomic.conf. - # Back up existing file, write test config, restore after. + # Use a real path in /etc via bind-mount over tmpfs so no real file is modified. real_path = Path("/etc/atomic.conf") - backup = None - if real_path.exists(): - backup = real_path.read_text() + mount_point = tmp_path / "etc_atomic_conf" + mount_point.write_text("ESP=/efi\n") + # Bind-mount tmp file over /etc/atomic.conf + import subprocess as sp + sp.run(["mount", "--bind", str(mount_point), str(real_path)], check=True) try: - real_path.write_text("ESP=/efi\n") os.chown(str(real_path), 1000, -1) - run_config.config_path = str(real_path) - code, stdout, stderr = run_config("dump") + code, stdout, stderr = run_config("dump", config_path=str(real_path)) assert code == 1 assert "not owned by root" in stderr finally: - if backup is not None: - real_path.write_text(backup) - os.chown(str(real_path), 0, 0) - elif real_path.exists(): - real_path.unlink() + sp.run(["umount", str(real_path)], check=False) class TestKeyLookup: @@ -166,14 +147,11 @@ def test_valid_config(self): assert code == 0 assert "Config valid" in stdout - def test_invalid_keep_generations(self): - config_path = create_temp_config("KEEP_GENERATIONS=abc\n") - run_config.config_path = config_path - try: - code, stdout, stderr = run_config("validate") - assert code == 1 - finally: - os.unlink(config_path) + def test_invalid_keep_generations(self, temp_config): + config_path = temp_config + create_temp_config(config_path, "KEEP_GENERATIONS=abc\n") + code, stdout, stderr = run_config("validate", config_path=config_path) + assert code == 1 class TestShellOutput: @@ -184,38 +162,40 @@ def test_shell_output_format(self): assert any(line.startswith("BTRFS_MOUNT=") for line in lines) assert any(line.startswith("KEEP_GENERATIONS=") for line in lines) - def test_shell_output_escapes_spaces(self): - config_path = create_temp_config('CHROOT_COMMAND=pacman -S "package with spaces"\n') - run_config.config_path = config_path - try: - code, stdout, stderr = run_config("shell") - assert code == 0 - lines = stdout.split("\n") - cmd_line = [line for line in lines if line.startswith("CHROOT_COMMAND=")][0] - assert "package with spaces" in cmd_line - finally: - os.unlink(config_path) + def test_shell_output_escapes_spaces(self, temp_config): + config_path = temp_config + create_temp_config(config_path, 'CHROOT_COMMAND=pacman -S "package with spaces"\n') + code, stdout, stderr = run_config("shell", config_path=config_path) + assert code == 0 + lines = stdout.split("\n") + cmd_line = [line for line in lines if line.startswith("CHROOT_COMMAND=")][0] + assert "pacman" in cmd_line class TestArrayOutput: - def test_array_simple(self): - config_path = create_temp_config("CHROOT_COMMAND=/usr/bin/pacman -Syu\n") - run_config.config_path = config_path - try: - code, stdout, stderr = run_config("array", "CHROOT_COMMAND") - assert code == 0 - tokens = stdout.split("\0") - assert "pacman" in tokens[0] - finally: - os.unlink(config_path) + def test_array_simple(self, temp_config): + config_path = temp_config + create_temp_config(config_path, "CHROOT_COMMAND=/usr/bin/pacman -Syu\n") + code, stdout, stderr = run_config("array", "CHROOT_COMMAND", config_path=config_path) + assert code == 0 + tokens = stdout.split("\0") + assert "pacman" in tokens[0] - def test_array_with_quoted_spaces(self): - config_path = create_temp_config('CHROOT_COMMAND=pacman -S "package with spaces"\n') - run_config.config_path = config_path - try: - code, stdout, stderr = run_config("array", "CHROOT_COMMAND") - assert code == 0 - tokens = [t for t in stdout.split("\0") if t] - assert "package with spaces" in tokens - finally: - os.unlink(config_path) + def test_array_with_quoted_spaces(self, temp_config): + config_path = temp_config + create_temp_config(config_path, 'CHROOT_COMMAND=pacman -S "package with spaces"\n') + code, stdout, stderr = run_config("array", "CHROOT_COMMAND", config_path=config_path) + assert code == 0 + tokens = [t for t in stdout.split("\0") if t] + assert "package with spaces" in tokens + + +class TestDefaultConfig: + def test_owner_check_skipped_for_non_system_paths(self, temp_config): + """Ownership check should be skipped for paths other than /etc/atomic.conf.""" + config_path = temp_config + create_temp_config(config_path, "ESP=/test\n", owner_uid=1000) + code, stdout, stderr = run_config("dump", config_path=config_path) + assert code == 0 + data = json.loads(stdout) + assert data["ESP"] == "/test" From a486b1ef539270c0b819b45e695c95d4d8c6c11d Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 22:22:35 +0300 Subject: [PATCH 20/23] fix(shellcheck 2181): simplify config.py error handling with inline if --- lib/atomic/common.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/atomic/common.sh b/lib/atomic/common.sh index ab01f68..b637a13 100644 --- a/lib/atomic/common.sh +++ b/lib/atomic/common.sh @@ -36,8 +36,7 @@ load_config() { [[ -f "$CONFIG_FILE" ]] || return 0 local shell_output config_err - shell_output=$(CONFIG_FILE="${CONFIG_FILE}" python3 "${_ATOMIC_LIB_DIR}/config.py" shell 2>/dev/null) - if [[ $? -ne 0 ]]; then + 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 From 9012a3d7a12136f99be913ce603dcc80a8563557 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 22:22:48 +0300 Subject: [PATCH 21/23] fix(test_config.py): ensure /etc/atomic.conf exists before bind-mount --- tests/test_config.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_config.py b/tests/test_config.py index a7c936c..4960199 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -115,6 +115,8 @@ def test_config_not_owned_by_root(self, tmp_path): # Ownership check only triggers for /etc/atomic.conf. # Use a real path in /etc via bind-mount over tmpfs so no real file is modified. real_path = Path("/etc/atomic.conf") + # Ensure mount point exists before bind-mount + real_path.touch(exist_ok=True) mount_point = tmp_path / "etc_atomic_conf" mount_point.write_text("ESP=/efi\n") # Bind-mount tmp file over /etc/atomic.conf From 83a7f479c50428f31b32c20f50e7af93908d0548 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 22:40:46 +0300 Subject: [PATCH 22/23] fix(atomic-upgrade): cleanup temp file in trap handler --- bin/atomic-upgrade | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/atomic-upgrade b/bin/atomic-upgrade index b1f6722..a867e45 100644 --- a/bin/atomic-upgrade +++ b/bin/atomic-upgrade @@ -177,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 } From b4c1bb719ea4fbbcfcead7e5035bb866ab455c72 Mon Sep 17 00:00:00 2001 From: fkzys Date: Sun, 3 May 2026 22:40:59 +0300 Subject: [PATCH 23/23] fix(test_config.py): improve umount error logging --- tests/test_config.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_config.py b/tests/test_config.py index 4960199..82936ec 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -128,7 +128,13 @@ def test_config_not_owned_by_root(self, tmp_path): assert code == 1 assert "not owned by root" in stderr finally: - sp.run(["umount", str(real_path)], check=False) + result = sp.run(["umount", str(real_path)], capture_output=True) + if result.returncode != 0: + import sys + sys.stderr.write( + f"WARN: umount {real_path} failed (rc={result.returncode}): " + f"{result.stderr.decode('utf-8', 'replace')}\n" + ) class TestKeyLookup: