Merge pull request 'Make save operation transactional' (#3) from transaction_patch into main

Reviewed-on: #3

.gitea/CODEOWNERS

@@ -0,0 +1 @@
+* @mdaleo404

.gitea/workflows/lint-and-security.yml

@@ -0,0 +1,36 @@
+name: Lint & Security
+
+on:
+  pull_request:
+
+jobs:
+  precommit-and-security:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.13"
+
+      - name: Install pre-commit
+        run: pip install pre-commit
+
+      - name: Run pre-commit hooks
+        run: pre-commit run --all-files --color always
+
+      - name: Install Poetry
+        run: |
+          pip install poetry
+          poetry self add poetry-plugin-export
+
+      - name: Install pip-audit
+        run: pip install pip-audit
+
+      - name: Audit dependencies (Poetry lockfile)
+        run: |
+          poetry export -f requirements.txt --without-hashes \
+            | pip-audit -r /dev/stdin

.gitignore

@@ -85,7 +85,7 @@ ipython_config.py
 # pyenv
 #   For a library or package, you might want to ignore these files since the code is
 #   intended to run in multiple environments; otherwise, check them in:
-# .python-version
+.python-version
 
 # pipenv
 #   According to pypa/pipenv#598, it is recommended to include Pipfile.lock in version control.

README.md

@@ -4,6 +4,11 @@
 
 # chguard
 
+<div align="center">
+  <img src="https://git.sysmd.uk/guardutils/chguard/src/branch/main/chguard.png" alt="chguard logo" width="256" />
+</div>
+
+
 **chguard** is a safety-first command-line tool that snapshots and restores
 filesystem ownership and permissions.
 
@@ -193,6 +198,16 @@ Snapshots are stored in a local SQLite database containing:
 
 Usernames and permission strings are resolved only for display.
 
+### TAB completion
+Add this to your `.bashrc`
+```
+eval "$(register-python-argcomplete chguard)"
+```
+And then
+```
+source ~/.bashrc
+```
+
 ## pre-commit
 This project uses [**pre-commit**](https://pre-commit.com/) to run automatic formatting and security checks before each commit (Black, Bandit, and various safety checks).
 

chguard/cli.py

@@ -1,6 +1,8 @@
 from __future__ import annotations
 
 import argparse
+import argcomplete
+import importlib.metadata
 import os
 import sys
 import stat
@@ -27,6 +29,13 @@ from chguard.restore import plan_restore, apply_restore
 from chguard.util import normalize_root
 
 
+def get_version():
+    try:
+        return importlib.metadata.version("chguard")
+    except importlib.metadata.PackageNotFoundError:
+        return "unknown"
+
+
 def _uid_to_name(uid: int) -> str:
     """Return username for uid, or uid as string if unknown."""
     try:
@@ -83,6 +92,19 @@ def _is_root() -> bool:
     return os.geteuid() == 0
 
 
+def complete_state_names(prefix, parsed_args, **kwargs):
+    try:
+        conn = connect(
+            Path(parsed_args.db).expanduser().resolve()
+            if parsed_args.db
+            else None
+        )
+        rows = conn.execute("SELECT name FROM states").fetchall()
+        return [name for (name,) in rows if name.startswith(prefix)]
+    except Exception:
+        return []
+
+
 def main() -> None:
     """
     Entry point for the CLI.
@@ -100,45 +122,99 @@ def main() -> None:
     )
 
     actions = parser.add_mutually_exclusive_group(required=True)
-    actions.add_argument("--save", metavar="PATH", help="Save state for PATH")
+
+    parser.add_argument(
+        "--version",
+        action="version",
+        version=f"chguard {get_version()}",
+    )
+
     actions.add_argument(
-        "--restore", action="store_true", help="Restore a saved state"
-    )
+        "--save",
+        metavar="PATH",
+        help="Save state for PATH",
+    ).completer = argcomplete.FilesCompleter()
+
     actions.add_argument(
-        "--list", action="store_true", help="List saved states"
+        "--restore",
+        action="store_true",
+        help="Restore a saved state",
     )
+
     actions.add_argument(
-        "--delete", metavar="STATE", help="Delete a saved state"
+        "--list",
+        action="store_true",
+        help="List saved states",
+    )
+
+    actions.add_argument(
+        "--delete",
+        metavar="STATE",
+        help="Delete a saved state",
+    ).completer = complete_state_names
+
+    # positional STATE
+    parser.add_argument(
+        "state",
+        nargs="?",
+        help="State name (required with --restore)",
+    ).completer = complete_state_names
+
+    parser.add_argument(
+        "--name",
+        help="State name (required with --save)",
     )
 
     parser.add_argument(
-        "state", nargs="?", help="State name (required with --restore)"
-    )
-    parser.add_argument("--name", help="State name (required with --save)")
-    parser.add_argument(
-        "--overwrite", action="store_true", help="Overwrite existing state"
+        "--overwrite",
+        action="store_true",
+        help="Overwrite existing state",
     )
 
     parser.add_argument(
-        "--permissions", action="store_true", help="Restore MODE only"
-    )
-    parser.add_argument(
-        "--owner", action="store_true", help="Restore OWNER only"
+        "--permissions",
+        action="store_true",
+        help="Restore MODE only",
     )
 
     parser.add_argument(
-        "--dry-run", action="store_true", help="Preview only; do not apply"
-    )
-    parser.add_argument(
-        "--yes", action="store_true", help="Apply without confirmation"
+        "--owner",
+        action="store_true",
+        help="Restore OWNER only",
     )
 
-    parser.add_argument("--root", metavar="PATH", help="Override restore root")
     parser.add_argument(
-        "--exclude", action="append", default=[], help="Exclude path prefix"
+        "--dry-run",
+        action="store_true",
+        help="Preview only; do not apply",
     )
-    parser.add_argument("--db", metavar="PATH", help="Override database path")
 
+    parser.add_argument(
+        "--yes",
+        action="store_true",
+        help="Apply without confirmation",
+    )
+
+    parser.add_argument(
+        "--root",
+        metavar="PATH",
+        help="Override restore root",
+    ).completer = argcomplete.FilesCompleter()
+
+    parser.add_argument(
+        "--exclude",
+        action="append",
+        default=[],
+        help="Exclude path prefix",
+    ).completer = argcomplete.FilesCompleter()
+
+    parser.add_argument(
+        "--db",
+        metavar="PATH",
+        help="Override database path",
+    ).completer = argcomplete.FilesCompleter()
+
+    argcomplete.autocomplete(parser)
     args = parser.parse_args()
     console = Console()
 
@@ -172,42 +248,49 @@ def main() -> None:
 
         root = normalize_root(args.save)
 
-        if state_exists(conn, args.name):
-            if not args.overwrite:
-                raise SystemExit(
-                    f"State '{args.name}' already exists (use --overwrite)"
-                )
-            delete_state(conn, args.name)
+        try:
+            with conn:  # start transaction
+                if state_exists(conn, args.name):
+                    if not args.overwrite:
+                        raise SystemExit(
+                            f"State '{args.name}' already exists (use --overwrite)"
+                        )
+                    # if the new save fails, this delete_state step will also roll back
+                    delete_state(conn, args.name, commit=False)
 
-        state_id = create_state(conn, args.name, str(root), os.getuid())
-
-        # Abort early if root-owned files exist and user is not root.
-        # This prevents creating snapshots that cannot be meaningfully restored.
-        for entry in scan_tree(root, excludes=args.exclude):
-            if entry.uid == 0 and not _is_root():
-                raise SystemExit(
-                    "This path contains root-owned files.\n"
-                    "Saving this state requires sudo."
+                state_id = create_state(
+                    conn, args.name, str(root), os.getuid(), commit=False
                 )
 
-            conn.execute(
-                """
-                INSERT INTO entries (state_id, path, type, mode, uid, gid)
-                VALUES (?, ?, ?, ?, ?, ?)
-                """,
-                (
-                    state_id,
-                    entry.path,
-                    entry.type,
-                    entry.mode,
-                    entry.uid,
-                    entry.gid,
-                ),
-            )
+                # Abort early if root-owned files exist and user is not root.
+                # This prevents creating snapshots that cannot be meaningfully restored.
+                for entry in scan_tree(root, excludes=args.exclude):
+                    if entry.uid == 0 and not _is_root():
+                        raise SystemExit(
+                            "This path contains root-owned files.\n"
+                            "Saving this state requires sudo."
+                        )
 
-        conn.commit()
-        console.print(f"Saved state '{args.name}' for {root}")
-        return
+                    conn.execute(
+                        """
+                        INSERT INTO entries (state_id, path, type, mode, uid, gid)
+                        VALUES (?, ?, ?, ?, ?, ?)
+                        """,
+                        (
+                            state_id,
+                            entry.path,
+                            entry.type,
+                            entry.mode,
+                            entry.uid,
+                            entry.gid,
+                        ),
+                    )
+
+            console.print(f"Saved state '{args.name}' for {root}")
+            return
+
+        except SystemExit:
+            raise
 
     if args.restore:
         if not args.state:

chguard/db.py

@@ -61,19 +61,28 @@ def state_exists(conn: sqlite3.Connection, name: str) -> bool:
 
 
 def create_state(
-    conn: sqlite3.Connection, name: str, root_path: str, created_by_uid: int
+    conn: sqlite3.Connection,
+    name: str,
+    root_path: str,
+    created_by_uid: int,
+    *,
+    commit: bool = True,
 ) -> int:
     cur = conn.execute(
         "INSERT INTO states (name, root_path, created_at, created_by_uid) VALUES (?, ?, ?, ?)",
         (name, root_path, utc_now_iso(), created_by_uid),
     )
-    conn.commit()
+    if commit:
+        conn.commit()
     return int(cur.lastrowid)
 
 
-def delete_state(conn: sqlite3.Connection, name: str) -> int:
+def delete_state(
+    conn: sqlite3.Connection, name: str, commit: bool = True
+) -> int:
     cur = conn.execute("DELETE FROM states WHERE name = ?", (name,))
-    conn.commit()
+    if commit:
+        conn.commit()
     return cur.rowcount
 
 

poetry.lock

@@ -289,4 +289,4 @@ test = ["covdefaults (>=2.3)", "coverage (>=7.2.7)", "coverage-enable-subprocess
 [metadata]
 lock-version = "2.0"
 python-versions = ">=3.10,<4.0"
-content-hash = "49f77d614e46109e49e997fa270cb7093d6f7e7d258e370c4eddd4354c20437f"
+content-hash = "4a5c993fcc16fe3739c43eb00bed750ce0803d45e37c7a786aa0b83bb4930267"

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "chguard"
-version = "0.1.0"
+version = "0.2.2"
 description = "Safety-first tool to snapshot and restore filesystem ownership and permissions."
 authors = ["Marco D'Aleo <marco@marcodaleo.com>"]
 license = "GPL-3.0-or-later"
@@ -12,7 +12,8 @@ repository = "https://git.sysmd.uk/guardutils/chguard"
 python = ">=3.10,<4.0"
 rich = ">=12"
 argcomplete = ">=2"
-platformdirs = "^4.5.1"
+platformdirs = ">=4.5.1"
+filelock = ">=3.20.1"
 
 [tool.poetry.scripts]
 chguard = "chguard.cli:main"