Refactor core components and enhance encryption with AES-GCM

[Cyber-Syntax]

This pull request introduces several improvements and refactorings to the autotarcompress codebase, focusing on safer backup handling, improved error management, and code organization. The most significant changes include atomic backup file creation to prevent corruption, enhanced user prompts for overwriting backups, improved configuration parsing, and better handling of edge cases during cleanup and decryption. Imports and utility usage have also been streamlined for clarity and maintainability.

Backup process improvements:

• The backup process now writes to a temporary file and uses atomic rename to avoid race conditions and prevent partial archive corruption. If the backup fails, the temporary file is cleaned up. User confirmation for overwriting existing backups now deletes the old file before starting the backup, ensuring a clean state. (src/autotarcompress/backup_manager.py) [1] [2]
• The prompt for overwriting an existing backup file now shows the exact file to be deleted and only proceeds if the user confirms. (src/autotarcompress/backup_manager.py)

Configuration and parsing fixes:

• The configuration parser now correctly reads dirs_to_backup as newline-delimited entries rather than comma-separated, fixing issues with misparsed backup directories. (src/autotarcompress/config.py)

Cleanup and error handling:

• The cleanup routine now safely skips files with unrecognized names (logging at DEBUG level) instead of crashing, ensuring robust cleanup even with unexpected files present. (src/autotarcompress/cleanup_manager.py)
• The decrypt manager now validates encrypted files for a minimum size before attempting decryption, preventing cryptic errors with truncated or corrupted files. (src/autotarcompress/decrypt_manager.py)

Code organization and import refactoring:

• Utility imports have been consolidated for clarity, and several modules have been moved to more logical locations (e.g., progress_bar, size_calculator, get_password). TYPE_CHECKING blocks are used to avoid unnecessary runtime imports in CLI commands. (src/autotarcompress/backup_manager.py, src/autotarcompress/base_manager.py, src/autotarcompress/cli/commands/*, src/autotarcompress/decrypt_manager.py) [1] [2] [3] [4] [5] [6] [7] [8] [9]

These changes collectively make the backup process safer, improve user experience, and enhance the maintainability and reliability of the codebase.

[cubic-dev-ai]

12 issues found across 34 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/autotarcompress/extract_manager.py">

<violation number="1" location="src/autotarcompress/extract_manager.py:111">
P1: The pv path is hardcoded to `/usr/bin/pv`, but `is_pv_available()` uses `shutil.which("pv")` (utils.py:120), which finds pv anywhere on PATH. When pv is installed outside `/usr/bin` (e.g. `/usr/local/bin` or `/bin`), `is_pv_available()` returns True yet `_extract_with_pv` raises FileNotFoundError. That is caught by `except OSError`, which returns False — so the whole extraction fails instead of falling back to `_extract_without_pv`. Same applies to the `/usr/bin/tar` invocation below it on systems where tar is not at that path. Resolve both binaries from PATH (consistent with `is_pv_available`) instead of hardcoding.</violation>

<violation number="2" location="src/autotarcompress/extract_manager.py:115">
P2: `tar_proc` is never read after this assignment, so Ruff's F841 check rejects the file. Call `subprocess.run(...)` directly instead of assigning its result.</violation>

<violation number="3" location="src/autotarcompress/extract_manager.py:160">
P1: When a traversal member appears after a valid member, this loop writes the valid member before detecting the unsafe path and returning `False`. Validate all members before calling `tar.extract` so path validation failure cannot leave partial extracted contents.</violation>

<violation number="4" location="src/autotarcompress/extract_manager.py:164">
P2: `startswith(extract_dir.resolve())` has no path-separator boundary, so a member such as `../extract_secret/file` resolves to a sibling path that still starts with the extract dir string and passes the check. Actual protection now comes only from `tar.extract(..., filter="data")`, which makes this flawed manual check redundant. Prefer relying on `filter="data"` alone (or compare with `is_relative_to`), and remove the misleading manual check.</violation>
</file>

<file name="src/autotarcompress/metadata.py">

<violation number="1" location="src/autotarcompress/metadata.py:128">
P3: New helpers are missing required type hints: `_file_lock` has no return annotation, and `_update_metadata`'s `updater` parameter is untyped. AGENTS.md requires full type annotations and return types on all functions, and mypy strict (`disallow_untyped_defs`) would flag these. Type both with a `Generator`/`Iterator[None, None, None]` return for the contextmanager and a `Callable[[BackupMetadata], None]` for `updater`.</violation>

<violation number="2" location="src/autotarcompress/metadata.py:152">
P1: Concurrent metadata writers can bypass this lock when one process unlinks the lock path after releasing it. Keep a stable `.lock` file instead of deleting it so every process flocks the same inode.</violation>
</file>

<file name="src/autotarcompress/utils.py">

<violation number="1" location="src/autotarcompress/utils.py:104">
P2: When `folder` already names a regular file, this check treats it as an ensured directory and returns it. Reject existing non-directories here so backup setup fails before archive creation.</violation>
</file>

<file name="src/autotarcompress/decrypt_manager.py">

<violation number="1" location="src/autotarcompress/decrypt_manager.py:183">
P1: Existing encrypted backups larger than 64 KiB no longer decrypt because this reader assumes the new chunked format. Add legacy-format detection and fallback, or version the on-disk format before switching formats.</violation>

<violation number="2" location="src/autotarcompress/decrypt_manager.py:184">
P1: When a file ends immediately after a nonce, this branch reports successful decryption of a truncated archive. Treat a missing ciphertext block as corruption, clean up the partial output, and return `False`.</violation>

<violation number="3" location="src/autotarcompress/decrypt_manager.py:188">
P1: Each chunk is authenticated independently without its position or stream completion, so deleting or reordering records can produce altered plaintext with valid tags. Authenticate sequence and end-of-stream metadata, or retain one authenticated record, and fail integrity mismatches.</violation>
</file>

<file name="src/autotarcompress/encrypt_manager.py">

<violation number="1" location="src/autotarcompress/encrypt_manager.py:126">
P1: When an encrypted file loses a complete chunk or has chunks reordered, each remaining GCM tag still verifies because the chunk position is not authenticated, and the decryptor accepts EOF as success. Authenticate the chunk sequence and end-of-file state, then reject missing or out-of-order records.</violation>

<violation number="2" location="src/autotarcompress/encrypt_manager.py:128">
P1: Existing `.enc` backups larger than 64 KiB become unreadable because this format change has no version marker or legacy decoding path. Add an explicit format version and retain support for the previous single-record layout, or provide a migration before switching formats.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: When a traversal member appears after a valid member, this loop writes the valid member before detecting the unsafe path and returning False. Validate all members before calling tar.extract so path validation failure cannot leave partial extracted contents.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/autotarcompress/extract_manager.py, line 160:

<comment>When a traversal member appears after a valid member, this loop writes the valid member before detecting the unsafe path and returning `False`. Validate all members before calling `tar.extract` so path validation failure cannot leave partial extracted contents.</comment>

<file context>
@@ -125,39 +137,48 @@ def _extract_without_pv(
-                for member in tar.getmembers():
+                # single loop does both in order.
+                # This is clearer and avoids iterating the member list twice.
+                for member in members:
+                    # Security: make sure the member won't extract outside
+                    # the target directory (path traversal attack prevention).
</file context>

Suggested change

	for member in members:
	# Security: make sure the member won't extract outside
	# the target directory (path traversal attack prevention).
	target_path = extract_dir / member.name
	if not str(target_path.absolute()).startswith(
	str(extract_dir.absolute())
	if not str(target_path.resolve()).startswith(
	str(extract_dir.resolve())
	):
	self.logger.error(
	"Attempted path traversal: %s", member.name
	"Attempted path traversal detected: %s — "
	"aborting extraction.",
	member.name,
	)
	return False
	# Extract each member with progress tracking
	for member in tar.getmembers():
	tar.extract(member, path=extract_dir)
	# Safe — extract this member immediately.
	tar.extract(member, path=extract_dir, filter="data")
	progress.update(member.size)
	for member in members:
	target_path = extract_dir / member.name
	if not str(target_path.resolve()).startswith(
	str(extract_dir.resolve())
	):
	self.logger.error(
	"Attempted path traversal detected: %s — "
	"aborting extraction.",
	member.name,
	)
	return False
	for member in members:
	tar.extract(member, path=extract_dir, filter="data")
	progress.update(member.size)

[cubic-dev-ai]

P2: startswith(extract_dir.resolve()) has no path-separator boundary, so a member such as ../extract_secret/file resolves to a sibling path that still starts with the extract dir string and passes the check. Actual protection now comes only from tar.extract(..., filter="data"), which makes this flawed manual check redundant. Prefer relying on filter="data" alone (or compare with is_relative_to), and remove the misleading manual check.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/autotarcompress/extract_manager.py, line 164:

<comment>`startswith(extract_dir.resolve())` has no path-separator boundary, so a member such as `../extract_secret/file` resolves to a sibling path that still starts with the extract dir string and passes the check. Actual protection now comes only from `tar.extract(..., filter="data")`, which makes this flawed manual check redundant. Prefer relying on `filter="data"` alone (or compare with `is_relative_to`), and remove the misleading manual check.</comment>

<file context>
@@ -125,39 +137,48 @@ def _extract_without_pv(
                     target_path = extract_dir / member.name
-                    if not str(target_path.absolute()).startswith(
-                        str(extract_dir.absolute())
+                    if not str(target_path.resolve()).startswith(
+                        str(extract_dir.resolve())
                     ):
</file context>

[cubic-dev-ai]

P3: New helpers are missing required type hints: _file_lock has no return annotation, and _update_metadata's updater parameter is untyped. AGENTS.md requires full type annotations and return types on all functions, and mypy strict (disallow_untyped_defs) would flag these. Type both with a Generator/Iterator[None, None, None] return for the contextmanager and a Callable[[BackupMetadata], None] for updater.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/autotarcompress/metadata.py, line 128:

<comment>New helpers are missing required type hints: `_file_lock` has no return annotation, and `_update_metadata`'s `updater` parameter is untyped. AGENTS.md requires full type annotations and return types on all functions, and mypy strict (`disallow_untyped_defs`) would flag these. Type both with a `Generator`/`Iterator[None, None, None]` return for the contextmanager and a `Callable[[BackupMetadata], None]` for `updater`.</comment>

<file context>
@@ -119,55 +124,141 @@ def load_metadata(config_dir: Path) -> BackupMetadata:
 
 
+@contextlib.contextmanager
+def _file_lock(path: Path):
+    """Acquire an exclusive cross-process file lock.
+
</file context>