Problem
I ran base-cli's own documented coverage command (--cov=base_cli --cov-report=term-missing, matching pyproject.toml's [tool.coverage] config with branch = true and fail_under = 80). The suite passes overall (86.99% total, above the 80% floor), but two related low-level modules have untested safety-guard branches -- specifically the negative/attack-path branches of code that performs destructive deletes or writes sensitive files:
1. lib/python/base_cli/_cleanup.py (79.5% covered) -- _validated_cleanup_paths's UnsafeCleanupPathError guards are untested.
This function validates a path before _remove_with_directory_handles recursively deletes it. Every one of its safety checks is an untested branch (missing lines 41, 49, 60, 69, 76, 80, 85-86, 89, 91):
- path-traversal rejection (
_contains_parent_reference)
- filesystem-root target rejection (
_is_root_like, checked on both lexical and resolved paths)
- run-ID ownership-marker mismatch rejection
- resolved-vs-lexical path mismatch rejection (the check that catches a symlink swapped in between validation and resolution)
- mounted-directory rejection (
_is_mount_target)
2. lib/python/base_cli/_private_files.py (80.0% covered) -- the symlink-refusal guard when replacing a private file is untested.
Lines 99-101 and 107: if existing is not None and _is_symlink_mode(existing.st_mode): raise OSError(f"refusing to replace symlink '{path}'") (and the non-dir_fd fallback branch of the same check) -- the guard that prevents write_private_json-style helpers from following a symlink onto an owner-only (0o600/0o700) file write. Also untested: the _replace_with_retry exception-retry path (lines 161-163) and a couple of dir_fd fallback branches (64, 71-72, 74, 113-115).
Why this matters
These aren't ordinary missed lines -- they're the specific guard-rail branches that only execute when something is trying to abuse the operation (a symlink swapped into a cleanup or write target, a path that traverses outside the expected root, a mount point). That's exactly the code where a subtle logic error (an inverted condition, a check that doesn't fire for a particular path shape) would silently defeat the protection instead of failing loudly, and it would only be discovered via an actual exploit attempt or a very unlucky user, not a test failure. The sibling Base repo has direct precedent here: #1969 (closed) fixed a real symlink-escape bug in basectl clean's cache-deletion path -- the same class of destructive-delete-path-safety issue this module implements defensively but doesn't yet test.
Proposed change
Add focused tests (tests/ already has fixtures for temp-directory scenarios) that specifically exercise the negative path for each guard:
_validated_cleanup_paths: a cleanup call with a ..-containing path, a path outside run_root, a mismatched run_id, a path that resolves differently than its lexical form (symlink swap), and a mount-point target -- asserting each raises UnsafeCleanupPathError._private_files.py's private-write path: attempt to write over an existing symlink and assert the OSError refusal fires, for both the dir_fd and fallback code paths.
Acceptance criteria
Problem
I ran base-cli's own documented coverage command (
--cov=base_cli --cov-report=term-missing, matchingpyproject.toml's[tool.coverage]config withbranch = trueandfail_under = 80). The suite passes overall (86.99% total, above the 80% floor), but two related low-level modules have untested safety-guard branches -- specifically the negative/attack-path branches of code that performs destructive deletes or writes sensitive files:1.
lib/python/base_cli/_cleanup.py(79.5% covered) --_validated_cleanup_paths'sUnsafeCleanupPathErrorguards are untested.This function validates a path before
_remove_with_directory_handlesrecursively deletes it. Every one of its safety checks is an untested branch (missing lines 41, 49, 60, 69, 76, 80, 85-86, 89, 91):_contains_parent_reference)_is_root_like, checked on both lexical and resolved paths)_is_mount_target)2.
lib/python/base_cli/_private_files.py(80.0% covered) -- the symlink-refusal guard when replacing a private file is untested.Lines 99-101 and 107:
if existing is not None and _is_symlink_mode(existing.st_mode): raise OSError(f"refusing to replace symlink '{path}'")(and the non-dir_fdfallback branch of the same check) -- the guard that preventswrite_private_json-style helpers from following a symlink onto an owner-only (0o600/0o700) file write. Also untested: the_replace_with_retryexception-retry path (lines 161-163) and a couple ofdir_fdfallback branches (64, 71-72, 74, 113-115).Why this matters
These aren't ordinary missed lines -- they're the specific guard-rail branches that only execute when something is trying to abuse the operation (a symlink swapped into a cleanup or write target, a path that traverses outside the expected root, a mount point). That's exactly the code where a subtle logic error (an inverted condition, a check that doesn't fire for a particular path shape) would silently defeat the protection instead of failing loudly, and it would only be discovered via an actual exploit attempt or a very unlucky user, not a test failure. The sibling Base repo has direct precedent here: #1969 (closed) fixed a real symlink-escape bug in
basectl clean's cache-deletion path -- the same class of destructive-delete-path-safety issue this module implements defensively but doesn't yet test.Proposed change
Add focused tests (
tests/already has fixtures for temp-directory scenarios) that specifically exercise the negative path for each guard:_validated_cleanup_paths: a cleanup call with a..-containing path, a path outsiderun_root, a mismatchedrun_id, a path that resolves differently than its lexical form (symlink swap), and a mount-point target -- asserting each raisesUnsafeCleanupPathError._private_files.py's private-write path: attempt to write over an existing symlink and assert theOSErrorrefusal fires, for both thedir_fdand fallback code paths.Acceptance criteria
_cleanup.pyand_private_files.pycoverage moves closer to the repo average (87%).