diff --git a/CHANGES b/CHANGES index b4448941c..756114ddf 100644 --- a/CHANGES +++ b/CHANGES @@ -33,6 +33,14 @@ $ uvx --from 'vcspull' --prerelease allow vcspull _Upcoming changes will be written here._ +### Development + +#### PrivatePath centralizes home-directory redaction (#485) + +- Introduced a dedicated `PrivatePath` helper for all CLI logging and structured + output, ensuring tilde-collapsed paths stay consistent without duplicating the + contraction logic across commands. + ## vcspull v1.46.1 (2025-11-03) ### Bug Fixes diff --git a/docs/api/internals/index.md b/docs/api/internals/index.md index 49382c110..9bbad1982 100644 --- a/docs/api/internals/index.md +++ b/docs/api/internals/index.md @@ -10,4 +10,5 @@ If you need an internal API stabilized please [file an issue](https://github.com ```{toctree} config_reader +private_path ``` diff --git a/docs/api/internals/private_path.md b/docs/api/internals/private_path.md new file mode 100644 index 000000000..d37bc7f92 --- /dev/null +++ b/docs/api/internals/private_path.md @@ -0,0 +1,55 @@ +# PrivatePath – `vcspull._internal.private_path` + +:::{warning} +`PrivatePath` is an internal helper. Its import path and behavior may change +without notice. File an issue if you rely on it downstream so we can discuss a +supported API. +::: + +`PrivatePath` subclasses `pathlib.Path` and normalizes every textual rendering +(`str()`/`repr()`) so the current user’s home directory is collapsed to `~`. +The class behaves exactly like the standard path object for filesystem ops; it +only alters how the path is displayed. This keeps CLI logs, JSON/NDJSON output, +and tests from leaking usernames while preserving full absolute paths for +internal logic. + +```python +from vcspull._internal.private_path import PrivatePath + +home_repo = PrivatePath("~/code/vcspull") +print(home_repo) # -> ~/code/vcspull +print(repr(home_repo)) # -> "PrivatePath('~/code/vcspull')" +``` + +## Usage guidelines + +- Wrap any path destined for user-facing output (logs, console tables, JSON + payloads) in `PrivatePath` before calling `str()`. +- The helper is safe to instantiate with `pathlib.Path` objects or strings; it + does not touch relative paths that lack a home prefix. +- Prefer storing raw `pathlib.Path` objects (or strings) in configuration + models, then convert to `PrivatePath` at the presentation layer. This keeps + serialization and equality checks deterministic while still masking the home + directory when needed. + +## Why not `contract_user_home`? + +The previous `contract_user_home()` helper duplicated the tilde-collapsing logic +in multiple modules and required callers to remember to run it themselves. By +centralizing the behavior in a `pathlib.Path` subclass we get: + +- Built-in protection—`str()` and `repr()` automatically apply the privacy + filter. +- Consistent behavior across every CLI command and test fixture. +- Easier mocking in tests, because `PrivatePath` respects monkeypatched + `Path.home()` implementations. + +If you need alternative redaction behavior, consider composing your own helper +around `PrivatePath` instead of reintroducing ad hoc string munging. + +```{eval-rst} +.. automodule:: vcspull._internal.private_path + :members: + :show-inheritance: + :undoc-members: +``` diff --git a/src/vcspull/_internal/private_path.py b/src/vcspull/_internal/private_path.py new file mode 100644 index 000000000..367d3dfd6 --- /dev/null +++ b/src/vcspull/_internal/private_path.py @@ -0,0 +1,69 @@ +from __future__ import annotations + +import os +import pathlib +import typing as t + +if t.TYPE_CHECKING: + PrivatePathBase = pathlib.Path +else: + PrivatePathBase = type(pathlib.Path()) + + +class PrivatePath(PrivatePathBase): + """Path subclass that hides the user's home directory in textual output. + + The class behaves like :class:`pathlib.Path`, but normalizes string and + representation output to replace the current user's home directory with + ``~``. This is useful when logging or displaying paths that should not leak + potentially sensitive information. + + Examples + -------- + >>> from pathlib import Path + >>> home = Path.home() + >>> PrivatePath(home) + PrivatePath('~') + >>> PrivatePath(home / "projects" / "vcspull") + PrivatePath('~/projects/vcspull') + >>> str(PrivatePath("/tmp/example")) + '/tmp/example' + >>> f'build dir: {PrivatePath(home / "build")}' + 'build dir: ~/build' + >>> '{}'.format(PrivatePath(home / 'notes.txt')) + '~/notes.txt' + """ + + def __new__(cls, *args: t.Any, **kwargs: t.Any) -> PrivatePath: + return super().__new__(cls, *args, **kwargs) + + @classmethod + def _collapse_home(cls, value: str) -> str: + """Collapse the user's home directory to ``~`` in ``value``.""" + if value.startswith("~"): + return value + + home = str(pathlib.Path.home()) + if value == home: + return "~" + + separators = {os.sep} + if os.altsep: + separators.add(os.altsep) + + for sep in separators: + home_with_sep = home + sep + if value.startswith(home_with_sep): + return "~" + value[len(home) :] + + return value + + def __str__(self) -> str: + original = pathlib.Path.__str__(self) + return self._collapse_home(original) + + def __repr__(self) -> str: + return f"{self.__class__.__name__}({str(self)!r})" + + +__all__ = ["PrivatePath"] diff --git a/src/vcspull/cli/add.py b/src/vcspull/cli/add.py index 95f7ff0d4..4a938c2c6 100644 --- a/src/vcspull/cli/add.py +++ b/src/vcspull/cli/add.py @@ -12,6 +12,7 @@ from colorama import Fore, Style from vcspull._internal.config_reader import DuplicateAwareConfigReader +from vcspull._internal.private_path import PrivatePath from vcspull.config import ( canonicalize_workspace_path, expand_dir, @@ -21,7 +22,6 @@ save_config_yaml_with_items, workspace_root_label, ) -from vcspull.util import contract_user_home if t.TYPE_CHECKING: import argparse @@ -220,11 +220,11 @@ def handle_add_command(args: argparse.Namespace) -> None: repo_path = expand_dir(pathlib.Path(repo_input), cwd=cwd) if not repo_path.exists(): - log.error("Repository path %s does not exist.", repo_path) + log.error("Repository path %s does not exist.", PrivatePath(repo_path)) return if not repo_path.is_dir(): - log.error("Repository path %s is not a directory.", repo_path) + log.error("Repository path %s is not a directory.", PrivatePath(repo_path)) return override_name = getattr(args, "override_name", None) @@ -238,7 +238,7 @@ def handle_add_command(args: argparse.Namespace) -> None: display_url, config_url = _normalize_detected_url(detected_remote) if not config_url: - display_url = contract_user_home(repo_path) + display_url = str(PrivatePath(repo_path)) config_url = str(repo_path) log.warning( "Unable to determine git remote for %s; using local path in config.", @@ -262,7 +262,7 @@ def handle_add_command(args: argparse.Namespace) -> None: summary_url = display_url or config_url - display_path = contract_user_home(repo_path) + display_path = str(PrivatePath(repo_path)) log.info("%sFound new repository to import:%s", Fore.GREEN, Style.RESET_ALL) log.info( @@ -319,7 +319,11 @@ def handle_add_command(args: argparse.Namespace) -> None: response = "" proceed = response.strip().lower() in {"y", "yes"} if not proceed: - log.info("Aborted import of '%s' from %s", repo_name, repo_path) + log.info( + "Aborted import of '%s' from %s", + repo_name, + PrivatePath(repo_path), + ) return add_repo( @@ -370,7 +374,7 @@ def add_repo( config_file_path = pathlib.Path.cwd() / ".vcspull.yaml" log.info( "No config specified and no default found, will create at %s", - contract_user_home(config_file_path), + PrivatePath(config_file_path), ) elif len(home_configs) > 1: log.error( @@ -384,7 +388,7 @@ def add_repo( raw_config: dict[str, t.Any] duplicate_root_occurrences: dict[str, list[t.Any]] top_level_items: list[tuple[str, t.Any]] - display_config_path = contract_user_home(config_file_path) + display_config_path = str(PrivatePath(config_file_path)) if config_file_path.exists() and config_file_path.is_file(): try: @@ -400,7 +404,10 @@ def add_repo( ) return except Exception: - log.exception("Error loading YAML from %s. Aborting.", config_file_path) + log.exception( + "Error loading YAML from %s. Aborting.", + PrivatePath(config_file_path), + ) if log.isEnabledFor(logging.DEBUG): traceback.print_exc() return @@ -579,7 +586,10 @@ def _prepare_no_merge_items( Style.RESET_ALL, ) except Exception: - log.exception("Error saving config to %s", config_file_path) + log.exception( + "Error saving config to %s", + PrivatePath(config_file_path), + ) if log.isEnabledFor(logging.DEBUG): traceback.print_exc() elif (duplicate_merge_changes > 0 or config_was_relabelled) and dry_run: @@ -635,7 +645,10 @@ def _prepare_no_merge_items( Style.RESET_ALL, ) except Exception: - log.exception("Error saving config to %s", config_file_path) + log.exception( + "Error saving config to %s", + PrivatePath(config_file_path), + ) if log.isEnabledFor(logging.DEBUG): traceback.print_exc() return @@ -719,7 +732,10 @@ def _prepare_no_merge_items( Style.RESET_ALL, ) except Exception: - log.exception("Error saving config to %s", config_file_path) + log.exception( + "Error saving config to %s", + PrivatePath(config_file_path), + ) if log.isEnabledFor(logging.DEBUG): traceback.print_exc() return @@ -778,6 +794,9 @@ def _prepare_no_merge_items( Style.RESET_ALL, ) except Exception: - log.exception("Error saving config to %s", config_file_path) + log.exception( + "Error saving config to %s", + PrivatePath(config_file_path), + ) if log.isEnabledFor(logging.DEBUG): traceback.print_exc() diff --git a/src/vcspull/cli/discover.py b/src/vcspull/cli/discover.py index bbaf16ea8..f9de85556 100644 --- a/src/vcspull/cli/discover.py +++ b/src/vcspull/cli/discover.py @@ -12,6 +12,7 @@ from colorama import Fore, Style from vcspull._internal.config_reader import DuplicateAwareConfigReader +from vcspull._internal.private_path import PrivatePath from vcspull.config import ( canonicalize_workspace_path, expand_dir, @@ -212,7 +213,10 @@ def discover_repos( ) return except Exception: - log.exception("Error loading YAML from %s. Aborting.", config_file_path) + log.exception( + "Error loading YAML from %s. Aborting.", + PrivatePath(config_file_path), + ) if log.isEnabledFor(logging.DEBUG): traceback.print_exc() return @@ -458,7 +462,10 @@ def discover_repos( Style.RESET_ALL, ) except Exception: - log.exception("Error saving config to %s", config_file_path) + log.exception( + "Error saving config to %s", + PrivatePath(config_file_path), + ) if log.isEnabledFor(logging.DEBUG): traceback.print_exc() return @@ -551,7 +558,10 @@ def discover_repos( Style.RESET_ALL, ) except Exception: - log.exception("Error saving config to %s", config_file_path) + log.exception( + "Error saving config to %s", + PrivatePath(config_file_path), + ) if log.isEnabledFor(logging.DEBUG): traceback.print_exc() return diff --git a/src/vcspull/cli/fmt.py b/src/vcspull/cli/fmt.py index d0dcd8ebb..4967ee788 100644 --- a/src/vcspull/cli/fmt.py +++ b/src/vcspull/cli/fmt.py @@ -11,6 +11,7 @@ from colorama import Fore, Style from vcspull._internal.config_reader import DuplicateAwareConfigReader +from vcspull._internal.private_path import PrivatePath from vcspull.config import ( find_config_files, find_home_config_files, @@ -176,10 +177,16 @@ def format_single_config( DuplicateAwareConfigReader.load_with_duplicates(config_file_path) ) except TypeError: - log.exception("Config file %s is not a mapping", config_file_path) + log.exception( + "Config file %s is not a mapping", + PrivatePath(config_file_path), + ) return False except Exception: - log.exception("Error loading config from %s", config_file_path) + log.exception( + "Error loading config from %s", + PrivatePath(config_file_path), + ) if log.isEnabledFor(logging.DEBUG): traceback.print_exc() return False @@ -359,7 +366,10 @@ def format_single_config( Style.RESET_ALL, ) except Exception: - log.exception("Error saving formatted config to %s", config_file_path) + log.exception( + "Error saving formatted config to %s", + PrivatePath(config_file_path), + ) if log.isEnabledFor(logging.DEBUG): traceback.print_exc() return False diff --git a/src/vcspull/cli/list.py b/src/vcspull/cli/list.py index 5fae15a12..16fdc55bf 100644 --- a/src/vcspull/cli/list.py +++ b/src/vcspull/cli/list.py @@ -5,8 +5,8 @@ import logging import typing as t +from vcspull._internal.private_path import PrivatePath from vcspull.config import filter_repos, find_config_files, load_configs -from vcspull.util import contract_user_home from ._colors import Colors, get_color_mode from ._output import OutputFormatter, get_output_mode @@ -167,7 +167,7 @@ def _output_flat( { "name": repo_name, "url": str(repo_url), - "path": contract_user_home(repo_path), + "path": str(PrivatePath(repo_path)), "workspace_root": str(repo.get("workspace_root", "")), }, ) @@ -175,7 +175,7 @@ def _output_flat( # Human output (contract home directory for privacy/brevity) formatter.emit_text( f"{colors.muted('•')} {colors.info(repo_name)} " - f"{colors.muted('→')} {contract_user_home(repo_path)}", + f"{colors.muted('→')} {PrivatePath(repo_path)}", ) @@ -218,7 +218,7 @@ def _output_tree( { "name": repo_name, "url": str(repo_url), - "path": contract_user_home(repo_path), + "path": str(PrivatePath(repo_path)), "workspace_root": workspace, }, ) @@ -226,5 +226,5 @@ def _output_tree( # Human output: indented repo (contract home directory for privacy/brevity) formatter.emit_text( f" {colors.muted('•')} {colors.info(repo_name)} " - f"{colors.muted('→')} {contract_user_home(repo_path)}", + f"{colors.muted('→')} {PrivatePath(repo_path)}", ) diff --git a/src/vcspull/cli/status.py b/src/vcspull/cli/status.py index b8173b4cf..cfd701563 100644 --- a/src/vcspull/cli/status.py +++ b/src/vcspull/cli/status.py @@ -13,8 +13,8 @@ from dataclasses import dataclass from time import perf_counter +from vcspull._internal.private_path import PrivatePath from vcspull.config import filter_repos, find_config_files, load_configs -from vcspull.util import contract_user_home from ._colors import Colors, get_color_mode from ._output import OutputFormatter, get_output_mode @@ -266,7 +266,7 @@ def check_repo_status(repo: ConfigDict, detailed: bool = False) -> dict[str, t.A status: dict[str, t.Any] = { "name": repo_name, - "path": contract_user_home(repo_path), + "path": str(PrivatePath(repo_path)), "workspace_root": workspace_root, "exists": False, "is_git": False, @@ -546,7 +546,7 @@ def _format_status_line( if detailed: formatter.emit_text( - f" {colors.muted('Path:')} {contract_user_home(status['path'])}", + f" {colors.muted('Path:')} {PrivatePath(status['path'])}", ) branch = status.get("branch") if branch: diff --git a/src/vcspull/cli/sync.py b/src/vcspull/cli/sync.py index b4f1be089..1b512ebfe 100644 --- a/src/vcspull/cli/sync.py +++ b/src/vcspull/cli/sync.py @@ -23,8 +23,8 @@ from libvcs.url import registry as url_tools from vcspull import exc +from vcspull._internal.private_path import PrivatePath from vcspull.config import filter_repos, find_config_files, load_configs -from vcspull.util import contract_user_home from ._colors import Colors, get_color_mode from ._output import ( @@ -264,7 +264,7 @@ def _build_plan_entry( return PlanEntry( name=str(repo.get("name", "unknown")), - path=contract_user_home(repo_path), + path=str(PrivatePath(repo_path)), workspace_root=workspace_root, action=action, detail=detail, @@ -436,7 +436,7 @@ def _render_plan( display_path = entry.path else: # Contract home directory for privacy/brevity in human output - display_path = contract_user_home(display_path) + display_path = str(PrivatePath(display_path)) detail_text = _format_detail_text( entry, @@ -724,7 +724,7 @@ def silent_progress(output: str, timestamp: datetime) -> None: event: dict[str, t.Any] = { "reason": "sync", "name": repo_name, - "path": contract_user_home(repo_path), + "path": str(PrivatePath(repo_path)), "workspace_root": str(workspace_label), } diff --git a/src/vcspull/util.py b/src/vcspull/util.py index 642a9a4c4..744960423 100644 --- a/src/vcspull/util.py +++ b/src/vcspull/util.py @@ -42,40 +42,6 @@ def get_config_dir() -> pathlib.Path: return path -def contract_user_home(path: str | pathlib.Path) -> str: - """Contract user home directory to ~ for display purposes. - - Parameters - ---------- - path : str | pathlib.Path - Path to contract - - Returns - ------- - str - Path with $HOME contracted to ~ - - Examples - -------- - >>> contract_user_home("/home/user/code/repo") - '~/code/repo' - >>> contract_user_home("/opt/project") - '/opt/project' - """ - path_str = str(path) - home_str = str(pathlib.Path.home()) - - # Replace home directory with ~ if path starts with it - if path_str.startswith(home_str): - # Handle both /home/user and /home/user/ cases - relative = path_str[len(home_str) :] - if relative.startswith(os.sep): - relative = relative[1:] - return f"~/{relative}" if relative else "~" - - return path_str - - T = t.TypeVar("T", bound=dict[str, t.Any]) diff --git a/tests/_internal/__init__.py b/tests/_internal/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/_internal/test_private_path.py b/tests/_internal/test_private_path.py new file mode 100644 index 000000000..6b9e1c3ce --- /dev/null +++ b/tests/_internal/test_private_path.py @@ -0,0 +1,42 @@ +from __future__ import annotations + +import os +from pathlib import Path + +from vcspull._internal.private_path import PrivatePath + + +def test_str_collapses_home_directory() -> None: + home = Path.home() + assert str(PrivatePath(home)) == "~" + + project_path = PrivatePath(home / "projects" / "vcspull") + assert str(project_path) == f"~{os.sep}projects{os.sep}vcspull" + + +def test_repr_uses_tilde_for_home_directory() -> None: + home = Path.home() + project_path = PrivatePath(home / "projects") + assert repr(project_path) == f"PrivatePath('~{os.sep}projects')" + + +def test_absolute_paths_outside_home_are_unmodified() -> None: + path = PrivatePath("/tmp/vcspull-example") + assert str(path) == "/tmp/vcspull-example" + assert repr(path) == "PrivatePath('/tmp/vcspull-example')" + + +def test_existing_tilde_prefix_is_preserved() -> None: + path = PrivatePath("~/.config/vcspull") + assert str(path) == "~/.config/vcspull" + + +def test_trailing_slash_collapses_to_directory_label() -> None: + home = Path.home() + path = PrivatePath(home / "projects/") + assert str(path) == f"~{os.sep}projects" + + +def test_relative_path_unmodified() -> None: + path = PrivatePath("relative/path") + assert str(path) == "relative/path" diff --git a/tests/cli/test_add.py b/tests/cli/test_add.py index 2fa5d386f..c09c92718 100644 --- a/tests/cli/test_add.py +++ b/tests/cli/test_add.py @@ -5,6 +5,7 @@ import argparse import logging import os +import pathlib import re import subprocess import textwrap @@ -13,8 +14,8 @@ import pytest from vcspull._internal.config_reader import DuplicateAwareConfigReader +from vcspull._internal.private_path import PrivatePath from vcspull.cli.add import add_repo, create_add_subparser, handle_add_command -from vcspull.util import contract_user_home if t.TYPE_CHECKING: import pathlib @@ -692,6 +693,8 @@ def test_handle_add_command_path_mode( else: repo_arg = str(repo_path) + repo_name = override_name or repo_path.name + args = argparse.Namespace( repo_path=repo_arg, url=explicit_url, @@ -706,16 +709,20 @@ def test_handle_add_command_path_mode( handle_add_command(args) log_output = caplog.text - contracted_path = contract_user_home(repo_path) + contracted_path = str(PrivatePath(repo_path)) assert "Found new repository to import" in log_output assert contracted_path in log_output normalized_log = log_output.replace(str(config_file), "") normalized_log = normalized_log.replace(str(repo_path), "") + normalized_log = normalized_log.replace( + f"Aborted import of '{repo_name}' from {PrivatePath(repo_path)!s}", + f"Aborted import of '{repo_name}' from ", + ) normalized_log = re.sub(r"add\.py:\d+", "add.py:", normalized_log) if preserve_config_path_in_log: - assert contract_user_home(config_file) in log_output + assert str(PrivatePath(config_file)) in log_output snapshot.assert_match( { "test_id": test_id, @@ -734,8 +741,6 @@ def test_handle_add_command_path_mode( if expected_warning is not None: assert expected_warning in log_output - repo_name = override_name or repo_path.name - if expected_written: import yaml @@ -805,6 +810,42 @@ def test_add_repo_dry_run_contracts_config_path( assert "~/.vcspull.yaml" in caplog.text +def test_handle_add_command_abort_uses_private_path( + tmp_path: pathlib.Path, + monkeypatch: MonkeyPatch, + caplog: t.Any, +) -> None: + """Aborted add command logs repo path with home collapsed.""" + caplog.set_level(logging.INFO) + + home = tmp_path / "home" + home.mkdir() + monkeypatch.chdir(home) + # Ensure PrivatePath resolves tilde within the temp home directory. + monkeypatch.setattr(pathlib.Path, "home", lambda: home) + + repo_path = home / "study/python/pytest-docker" + repo_path.mkdir(parents=True) + + args = argparse.Namespace( + repo_path=str(repo_path), + override_name=None, + url=None, + config=str(home / ".vcspull.yaml"), + workspace_root_path=None, + dry_run=False, + assume_yes=False, + merge_duplicates=True, + ) + + monkeypatch.setattr("builtins.input", lambda _: "n") + + handle_add_command(args) + + expected_path = str(PrivatePath(repo_path)) + assert f"Aborted import of 'pytest-docker' from {expected_path}" in caplog.text + + def test_add_parser_rejects_extra_positional() -> None: """Passing both name and URL should raise a parse error in the new parser.""" parser = argparse.ArgumentParser(prog="vcspull") diff --git a/tests/test_utils.py b/tests/test_utils.py index b764ace13..094917ef3 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -5,7 +5,7 @@ import pathlib import typing as t -from vcspull.util import contract_user_home, get_config_dir +from vcspull.util import get_config_dir if t.TYPE_CHECKING: import pytest @@ -37,42 +37,3 @@ def test_vcspull_configdir_no_xdg(monkeypatch: pytest.MonkeyPatch) -> None: """Test retrieving config directory without XDG_CONFIG_HOME set.""" monkeypatch.delenv("XDG_CONFIG_HOME") assert get_config_dir() - - -def test_contract_user_home_contracts_home_path() -> None: - """Test contracting home directory to ~.""" - home = str(pathlib.Path.home()) - - # Test full path - assert contract_user_home(f"{home}/code/repo") == "~/code/repo" - - # Test home directory itself - assert contract_user_home(home) == "~" - - # Test with pathlib.Path - assert contract_user_home(pathlib.Path(home) / "code" / "repo") == "~/code/repo" - - -def test_contract_user_home_preserves_non_home_paths() -> None: - """Test that non-home paths are not contracted.""" - # Test absolute paths outside home - assert contract_user_home("/opt/project") == "/opt/project" - assert contract_user_home("/usr/local/bin") == "/usr/local/bin" - - # Test relative paths - assert contract_user_home("./relative/path") == "./relative/path" - assert contract_user_home("relative/path") == "relative/path" - - -def test_contract_user_home_handles_edge_cases() -> None: - """Test edge cases in path contraction.""" - home = str(pathlib.Path.home()) - - # Test trailing slashes - assert contract_user_home(f"{home}/code/") == "~/code/" - - # Test empty path - assert contract_user_home("") == "" - - # Test path with ~ already in it (should pass through) - assert contract_user_home("~/code/repo") == "~/code/repo"