From ab2277a6c7a104a0541ea624bdc8f3e64033eb46 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:52:37 -0700 Subject: [PATCH 1/2] add platform-aware cache paths --- CHANGELOG.md | 2 + README.md | 5 +- docs/cache-ownership-and-layout.md | 12 +++++ lib/python/base_cli/history.py | 22 ++++---- lib/python/base_cli/paths.py | 36 ++++++++++++- lib/python/base_cli/profile.py | 16 +----- lib/python/base_cli/testing.py | 3 ++ tests/test_paths.py | 86 ++++++++++++++++++++++++++++++ 8 files changed, 157 insertions(+), 25 deletions(-) create mode 100644 tests/test_paths.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cf0624..9fd11c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and versions are tracked in the repo-root `VERSION` file. ### Changed +- Select platform-aware cache roots (`XDG_CACHE_HOME`, macOS Caches, and + Windows `LOCALAPPDATA`) and normalize home-relative paths across separators. - Make `base_cli.App()` use the consumer-neutral profile by default. - Move manifest discovery, implicit configuration, owner-aware runtime layout, and history persistence out of the generic package. Consumers now provide diff --git a/README.md b/README.md index b75d888..9730f08 100644 --- a/README.md +++ b/README.md @@ -448,7 +448,10 @@ explicitly and return a clear usage error or actionable message. The generic profile uses the configured cache root and an application namespace to create per-run logs, caches, and temporary directories. Pass `cache_root` to `CliProfile.generic()` for deterministic placement in tests or -applications; otherwise the platform cache directory is used. The generic +applications; otherwise the platform cache directory is used. Linux and WSL2 +follow `XDG_CACHE_HOME` or `~/.cache`, macOS uses `~/Library/Caches`, and +Windows uses `%LOCALAPPDATA%` (falling back to `~/AppData/Local`). Set +`BASE_CLI_CACHE_DIR` to override the default on any platform. The generic profile does not prescribe a product-wide cache name or cleanup command. Each invocation is a run bundle containing private (`0600`) `run.json`, diff --git a/docs/cache-ownership-and-layout.md b/docs/cache-ownership-and-layout.md index 3be89fa..9a8e682 100644 --- a/docs/cache-ownership-and-layout.md +++ b/docs/cache-ownership-and-layout.md @@ -4,6 +4,18 @@ Runtime state is rooted at the cache root supplied to `CliProfile.generic()` or the platform cache directory. The generic profile places each application in a sanitized application namespace and does not impose a product-wide cache name. +When no explicit cache root is supplied, the generic profile follows these +platform conventions: + +- Linux and WSL2: `XDG_CACHE_HOME`, or `~/.cache` when it is unset; +- macOS: `~/Library/Caches`; and +- Windows: `%LOCALAPPDATA%`, with `~/AppData/Local` as a fallback. + +`BASE_CLI_CACHE_DIR` overrides these defaults on every platform. WSL2 is +supported when the process runs inside the Linux distribution; Windows-mounted +paths such as `/mnt/c` retain their own filesystem performance and permission +characteristics. + Consumer profiles may choose a different cache root or owner-aware layout when their application needs stronger isolation between projects or checkouts. diff --git a/lib/python/base_cli/history.py b/lib/python/base_cli/history.py index 61b1e1f..e5f502c 100644 --- a/lib/python/base_cli/history.py +++ b/lib/python/base_cli/history.py @@ -315,20 +315,24 @@ def redact_history_text(value: str) -> str: return compact_home_text(redact_text_value(value)) -def compact_optional_path(path: Path | None) -> str | None: +def compact_optional_path(path: Path | None, *, home: Path | str | None = None) -> str | None: if path is None: return None - return compact_path(path) + return compact_path(path, home=home) -def compact_path(path: Path) -> str: - return compact_home_text(str(path.expanduser().resolve(strict=False))) +def compact_path(path: Path, *, home: Path | str | None = None) -> str: + return compact_home_text(str(path.expanduser().resolve(strict=False)), home=home) -def compact_home_text(value: str) -> str: - home = str(Path.home().expanduser().resolve(strict=False)) - if value == home: +def compact_home_text(value: str, *, home: Path | str | None = None) -> str: + home_text = str(home) if home is not None else str(Path.home().expanduser().resolve(strict=False)) + normalized_value = value.replace("\\", "/") + normalized_home = home_text.replace("\\", "/").rstrip("/") + comparison_value = normalized_value.lower() if os.name == "nt" else normalized_value + comparison_home = normalized_home.lower() if os.name == "nt" else normalized_home + if comparison_value == comparison_home: return "~" - if value.startswith(f"{home}/"): - return f"~/{value[len(home) + 1:]}" + if comparison_value.startswith(f"{comparison_home}/"): + return f"~/{normalized_value[len(normalized_home) + 1:]}" return value diff --git a/lib/python/base_cli/paths.py b/lib/python/base_cli/paths.py index c801c8a..9d9967c 100644 --- a/lib/python/base_cli/paths.py +++ b/lib/python/base_cli/paths.py @@ -2,10 +2,12 @@ import contextlib import contextvars +import os import re +import sys import time import uuid -from collections.abc import Iterator +from collections.abc import Iterator, Mapping from pathlib import Path _WORKING_DIRECTORY_OVERRIDE: contextvars.ContextVar[Path | None] = contextvars.ContextVar( @@ -14,6 +16,38 @@ ) +def default_cache_root( + *, + environ: Mapping[str, str] | None = None, + home: Path | None = None, + platform_name: str | None = None, +) -> Path: + """Return the platform-default cache root for the generic profile. + + ``BASE_CLI_CACHE_DIR`` always wins so consumers and tests can provide an + explicit location. Linux follows ``XDG_CACHE_HOME`` when it is set, + macOS uses ``Library/Caches``, and Windows uses ``LOCALAPPDATA``. + """ + + environment = os.environ if environ is None else environ + configured = environment.get("BASE_CLI_CACHE_DIR") + if configured: + return Path(configured).expanduser() + + root = home.expanduser() if home is not None else Path.home() + system = platform_name or sys.platform + if system == "darwin": + return root / "Library" / "Caches" + if system.startswith("win"): + local_app_data = environment.get("LOCALAPPDATA") + return Path(local_app_data).expanduser() if local_app_data else root / "AppData" / "Local" + + xdg_cache_home = environment.get("XDG_CACHE_HOME") + if xdg_cache_home: + return Path(xdg_cache_home).expanduser() + return root / ".cache" + + def current_working_dir() -> Path: return _WORKING_DIRECTORY_OVERRIDE.get() or Path.cwd() diff --git a/lib/python/base_cli/profile.py b/lib/python/base_cli/profile.py index 496a1f3..427e7a0 100644 --- a/lib/python/base_cli/profile.py +++ b/lib/python/base_cli/profile.py @@ -1,7 +1,5 @@ from __future__ import annotations -import os -import sys from collections.abc import Callable from dataclasses import dataclass from datetime import datetime @@ -10,7 +8,7 @@ from ._runtime import RuntimeLayout, runtime_layout from .config import load_yaml_file -from .paths import make_run_id +from .paths import default_cache_root, make_run_id @dataclass(frozen=True) @@ -124,7 +122,7 @@ def _generic_runtime_resolver( application_home: Path | None, ) -> RuntimeResolver: def resolve_runtime(cli_name: str, project: ProjectInfo | None) -> RuntimeBinding: - root = cache_root.expanduser().resolve() if cache_root is not None else _default_cache_root() + root = (cache_root if cache_root is not None else default_cache_root()).expanduser().resolve() run_id = make_run_id() project_root = project.root if project is not None else None project_name = project.name if project is not None else None @@ -147,13 +145,3 @@ def resolve_runtime(cli_name: str, project: ProjectInfo | None) -> RuntimeBindin ) return resolve_runtime - - -def _default_cache_root() -> Path: - configured = os.environ.get("BASE_CLI_CACHE_DIR") - if configured: - return Path(configured).expanduser().resolve() - root = Path.home() - if sys.platform == "darwin": - return root / "Library" / "Caches" - return root / ".cache" diff --git a/lib/python/base_cli/testing.py b/lib/python/base_cli/testing.py index 986eced..2740c14 100644 --- a/lib/python/base_cli/testing.py +++ b/lib/python/base_cli/testing.py @@ -33,6 +33,9 @@ def invoke( invoke_env = dict(env or {}) if home is not None: invoke_env.setdefault("HOME", str(home)) + invoke_env.setdefault("USERPROFILE", str(home)) + invoke_env.setdefault("LOCALAPPDATA", str(home / "AppData" / "Local")) + invoke_env.setdefault("XDG_CACHE_HOME", str(home / ".cache")) invoke_env.setdefault("BASE_CLI_CACHE_DIR", str(home / ".cache")) runner_kwargs = {} if "mix_stderr" in inspect.signature(CliRunner).parameters: diff --git a/tests/test_paths.py b/tests/test_paths.py new file mode 100644 index 0000000..a59a70e --- /dev/null +++ b/tests/test_paths.py @@ -0,0 +1,86 @@ +from __future__ import annotations + +import unittest +from pathlib import Path + +from base_cli.history import compact_home_text +from base_cli.paths import default_cache_root + + +class DefaultCacheRootTests(unittest.TestCase): + def test_explicit_cache_override_wins_on_every_platform(self) -> None: + root = default_cache_root( + environ={ + "BASE_CLI_CACHE_DIR": "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/custom/cache", + "LOCALAPPDATA": "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/local/app-data", + "XDG_CACHE_HOME": "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/xdg/cache", + }, + home=Path("/home/alice"), + platform_name="win32", + ) + + self.assertEqual(root, Path("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/custom/cache")) + + def test_linux_prefers_xdg_cache_home(self) -> None: + root = default_cache_root( + environ={"XDG_CACHE_HOME": "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/xdg/cache"}, + home=Path("/home/alice"), + platform_name="linux", + ) + + self.assertEqual(root, Path("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/xdg/cache")) + + def test_linux_falls_back_to_home_cache(self) -> None: + root = default_cache_root( + environ={}, + home=Path("/home/alice"), + platform_name="linux", + ) + + self.assertEqual(root, Path("/home/alice/.cache")) + + def test_macos_uses_library_caches(self) -> None: + root = default_cache_root( + environ={"XDG_CACHE_HOME": "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/xdg/cache"}, + home=Path("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/Users/alice"), + platform_name="darwin", + ) + + self.assertEqual(root, Path("/Users/alice/Library/Caches")) + + def test_windows_prefers_local_app_data(self) -> None: + root = default_cache_root( + environ={"LOCALAPPDATA": r"C:\Users\alice\AppData\Local"}, + home=Path(r"C:\Users\alice"), + platform_name="win32", + ) + + self.assertEqual(root, Path(r"C:\Users\alice\AppData\Local")) + + def test_windows_falls_back_to_home_local_app_data(self) -> None: + root = default_cache_root( + environ={}, + home=Path(r"C:\Users\alice"), + platform_name="win32", + ) + + self.assertEqual(root, Path(r"C:\Users\alice") / "AppData" / "Local") + + +class HomePathCompactionTests(unittest.TestCase): + def test_compacts_home_paths_with_posix_separators(self) -> None: + self.assertEqual( + compact_home_text("/home/alice/project", home=Path("/home/alice")), + "~/project", + ) + + def test_compacts_home_paths_with_windows_separators(self) -> None: + self.assertEqual( + compact_home_text(r"C:\Users\Alice\project", home=r"C:\Users\Alice"), + "~/project", + ) + + def test_leaves_paths_outside_home_unchanged(self) -> None: + value = r"C:\Users\Bob\project" + + self.assertEqual(compact_home_text(value, home=r"C:\Users\Alice"), value) From 401bbbdaf951c29653cde63768af57e52775ea45 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Sat, 1 Aug 2026 18:42:58 -0700 Subject: [PATCH 2/2] make runtime privacy and locking portable --- CHANGELOG.md | 3 ++ README.md | 8 +++-- docs/cache-ownership-and-layout.md | 8 +++-- lib/python/base_cli/_private_files.py | 19 +++++++++-- lib/python/base_cli/_runtime.py | 7 ++-- lib/python/base_cli/history.py | 5 +-- lib/python/base_cli/logging.py | 5 +-- tests/test_history.py | 49 +++++++++++++++++++++++++++ 8 files changed, 89 insertions(+), 15 deletions(-) create mode 100644 tests/test_history.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fd11c1..e62b76b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and versions are tracked in the repo-root `VERSION` file. ### Changed +- Keep private runtime files and directories owner-only on POSIX, use inherited + user-profile ACLs on Windows, and make history appends binary-safe across + locking backends. - Select platform-aware cache roots (`XDG_CACHE_HOME`, macOS Caches, and Windows `LOCALAPPDATA`) and normalize home-relative paths across separators. - Make `base_cli.App()` use the consumer-neutral profile by default. diff --git a/README.md b/README.md index 9730f08..3f088fd 100644 --- a/README.md +++ b/README.md @@ -454,9 +454,11 @@ Windows uses `%LOCALAPPDATA%` (falling back to `~/AppData/Local`). Set `BASE_CLI_CACHE_DIR` to override the default on any platform. The generic profile does not prescribe a product-wide cache name or cleanup command. -Each invocation is a run bundle containing private (`0600`) `run.json`, -`logs/`, and `tmp/`, while persistent component caches live in the -bundle's cache directory. +Each invocation is a run bundle containing a private `run.json`, `logs/`, and +`tmp/`, while persistent component caches live in the bundle's cache directory. +On POSIX, base-cli enforces owner-only `0600`/`0700` modes. On Windows, the +default user-local cache root relies on inherited user-profile ACLs; consumers +using a custom cache root must provide the appropriate ACL themselves. Use `ctx.on_cleanup()` for cleanup work that should happen even when helper code does not own the main command wrapper: diff --git a/docs/cache-ownership-and-layout.md b/docs/cache-ownership-and-layout.md index 9a8e682..b9c1ebb 100644 --- a/docs/cache-ownership-and-layout.md +++ b/docs/cache-ownership-and-layout.md @@ -26,5 +26,9 @@ Each invocation has a private run bundle containing: - `tmp/` for temporary command data. Persistent component caches live under the owner's `cache/components/` path. -Runtime directories are owner-only (`0700`), and runtime files are owner-only -(`0600`). +On POSIX systems, runtime directories are owner-only (`0700`) and runtime files +are owner-only (`0600`). On Windows, the default `%LOCALAPPDATA%` root relies +on the user-profile ACL inherited by its children; POSIX mode bits cannot +provide the same guarantee there. If `BASE_CLI_CACHE_DIR` points outside the +user profile on Windows, the consumer is responsible for supplying an +appropriately private ACL. diff --git a/lib/python/base_cli/_private_files.py b/lib/python/base_cli/_private_files.py index f7bbacb..44133d5 100644 --- a/lib/python/base_cli/_private_files.py +++ b/lib/python/base_cli/_private_files.py @@ -10,12 +10,25 @@ PRIVATE_FILE_MODE = 0o600 +PRIVATE_DIRECTORY_MODE = 0o700 def restrict_file(path: Path) -> None: - """Ensure an existing runtime file is readable and writable only by its owner.""" + """Apply owner-only POSIX permissions where mode bits are meaningful. - path.chmod(PRIVATE_FILE_MODE) + Windows inherits ACLs from the containing directory instead; the generic + package deliberately does not pretend that ``chmod`` can rewrite them. + """ + + if os.name != "nt": + path.chmod(PRIVATE_FILE_MODE) + + +def restrict_directory(path: Path) -> None: + """Apply owner-only POSIX directory permissions when supported.""" + + if os.name != "nt": + path.chmod(PRIVATE_DIRECTORY_MODE) def write_private_json(path: Path, value: Mapping[str, Any]) -> None: @@ -25,7 +38,7 @@ def write_private_json(path: Path, value: Mapping[str, Any]) -> None: fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, PRIVATE_FILE_MODE) try: fchmod = getattr(os, "fchmod", None) - if fchmod is not None: + if os.name != "nt" and fchmod is not None: fchmod(fd, PRIVATE_FILE_MODE) stream = os.fdopen(fd, "w", encoding="utf-8") fd = -1 diff --git a/lib/python/base_cli/_runtime.py b/lib/python/base_cli/_runtime.py index e1117f7..52f46a4 100644 --- a/lib/python/base_cli/_runtime.py +++ b/lib/python/base_cli/_runtime.py @@ -2,10 +2,11 @@ import json import logging +import os from dataclasses import dataclass from pathlib import Path -from ._private_files import write_private_json +from ._private_files import restrict_directory, write_private_json from .paths import runtime_run_directory_name, runtime_slug @@ -57,9 +58,9 @@ def create_runtime_directory(path: Path, cache_root: Path) -> None: restrict_permissions = _is_within(path, cache_root) try: path.mkdir(parents=True, exist_ok=True) - if restrict_permissions: + if restrict_permissions and os.name != "nt": for directory in [path, *missing]: - directory.chmod(0o700) + restrict_directory(directory) except OSError as exc: raise RuntimeError(_runtime_directory_error(path, cache_root, exc)) from exc diff --git a/lib/python/base_cli/history.py b/lib/python/base_cli/history.py index e5f502c..ac20615 100644 --- a/lib/python/base_cli/history.py +++ b/lib/python/base_cli/history.py @@ -188,13 +188,14 @@ def update_run_metadata(run_root: Path, record: dict[str, Any]) -> None: def append_history_line(path: Path, line: str) -> None: - fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_APPEND, 0o600) + binary_flag = getattr(os, "O_BINARY", 0) + fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_APPEND | binary_flag, 0o600) lock_fd = fd sidecar_fd: int | None = None try: if _fcntl is None and _msvcrt is not None: sidecar_path = path.with_name(f".{path.name}.lock") - sidecar_fd = os.open(sidecar_path, os.O_RDWR | os.O_CREAT, 0o600) + sidecar_fd = os.open(sidecar_path, os.O_RDWR | os.O_CREAT | binary_flag, 0o600) if os.fstat(sidecar_fd).st_size == 0: os.write(sidecar_fd, b"0") restrict_file(sidecar_path) diff --git a/lib/python/base_cli/logging.py b/lib/python/base_cli/logging.py index ece8bdb..89f5617 100644 --- a/lib/python/base_cli/logging.py +++ b/lib/python/base_cli/logging.py @@ -8,6 +8,7 @@ from pathlib import Path from typing import TextIO +from ._private_files import restrict_file from .context import get_current_context from .paths import current_working_dir from .redaction import redact_argv @@ -77,7 +78,7 @@ def _use_color(stream: TextIO) -> bool: def secure_log_file_permissions(log_file: Path) -> None: - log_file.chmod(0o600) + restrict_file(log_file) class SecureLogFileHandler(logging.FileHandler): @@ -85,7 +86,7 @@ def _open(self) -> TextIO: fd = os.open(self.baseFilename, _secure_log_file_open_flags(self.mode), 0o600) try: fchmod = getattr(os, "fchmod", None) - if fchmod is not None: + if os.name != "nt" and fchmod is not None: fchmod(fd, 0o600) return open(fd, self.mode, encoding=self.encoding, errors=self.errors, closefd=True) except BaseException: diff --git a/tests/test_history.py b/tests/test_history.py new file mode 100644 index 0000000..349af3e --- /dev/null +++ b/tests/test_history.py @@ -0,0 +1,49 @@ +from __future__ import annotations + +import json +import tempfile +import unittest +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path +from unittest import mock + +from base_cli import history + + +class _FakeMsvcrt: + LK_LOCK = 1 + LK_UNLCK = 2 + + def __init__(self) -> None: + self.calls: list[tuple[int, int]] = [] + + def locking(self, _fd: int, mode: int, size: int) -> None: + self.calls.append((mode, size)) + + +class HistoryAppendTests(unittest.TestCase): + def test_concurrent_appends_produce_complete_records(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "history.jsonl" + lines = [json.dumps({"run": index}) + "\n" for index in range(24)] + + with ThreadPoolExecutor(max_workers=8) as executor: + list(executor.map(lambda line: history.append_history_line(path, line), lines)) + + records = [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines()] + + self.assertEqual(sorted(record["run"] for record in records), list(range(24))) + + def test_msvcrt_backend_uses_a_private_sidecar_lock(self) -> None: + fake_msvcrt = _FakeMsvcrt() + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "history.jsonl" + with mock.patch.object(history, "_fcntl", None), mock.patch.object( + history, "_msvcrt", fake_msvcrt + ): + history.append_history_line(path, '{"run": 1}\n') + + self.assertEqual(path.read_text(encoding="utf-8"), '{"run": 1}\n') + self.assertTrue(path.with_name(".history.jsonl.lock").is_file()) + + self.assertEqual(fake_msvcrt.calls, [(_FakeMsvcrt.LK_LOCK, 1), (_FakeMsvcrt.LK_UNLCK, 1)])