diff --git a/CHANGES b/CHANGES index fb4e012f..b9e7faab 100644 --- a/CHANGES +++ b/CHANGES @@ -31,7 +31,33 @@ $ pipx install --suffix=@next 'vcspull' --pip-args '\--pre' --force -_Notes on upcoming releases will be added here_ +### Breaking Changes + +#### JSON/NDJSON paths now use tilde notation + +**All commands** (`list`, `status`, `sync`) now contract home directory paths in JSON/NDJSON output for consistency, privacy, and portability. + +**Before:** +```json +{ + "name": "flask", + "path": "/home/username/code/flask", + "workspace_root": "~/code/" +} +``` + +**After:** +```json +{ + "name": "flask", + "path": "~/code/flask", + "workspace_root": "~/code/" +} +``` + +**Why:** The `workspace_root` field already used tilde notation in JSON output, creating an inconsistency. Full home paths (`/home/username/`) expose usernames and make output less portable between machines. + +**Impact:** Automation consuming JSON/NDJSON output will need to expand `~` to absolute paths if required. Most tools handle tilde expansion automatically. ## vcspull v1.40.0 (2025-10-19) diff --git a/src/vcspull/cli/list.py b/src/vcspull/cli/list.py index d44cc5ee..6eccc2ad 100644 --- a/src/vcspull/cli/list.py +++ b/src/vcspull/cli/list.py @@ -161,12 +161,12 @@ def _output_flat( repo_url = repo.get("url", repo.get("pip_url", "unknown")) repo_path = repo.get("path", "unknown") - # JSON/NDJSON output + # JSON/NDJSON output (contract home for privacy/portability) formatter.emit( { "name": repo_name, "url": str(repo_url), - "path": str(repo_path), + "path": contract_user_home(repo_path), "workspace_root": str(repo.get("workspace_root", "")), } ) @@ -212,12 +212,12 @@ def _output_tree( repo_url = repo.get("url", repo.get("pip_url", "unknown")) repo_path = repo.get("path", "unknown") - # JSON/NDJSON output + # JSON/NDJSON output (contract home for privacy/portability) formatter.emit( { "name": repo_name, "url": str(repo_url), - "path": str(repo_path), + "path": contract_user_home(repo_path), "workspace_root": workspace, } ) diff --git a/src/vcspull/cli/status.py b/src/vcspull/cli/status.py index 0b64ae92..5057d25e 100644 --- a/src/vcspull/cli/status.py +++ b/src/vcspull/cli/status.py @@ -263,7 +263,7 @@ def check_repo_status(repo: ConfigDict, detailed: bool = False) -> dict[str, t.A status: dict[str, t.Any] = { "name": repo_name, - "path": str(repo_path), + "path": contract_user_home(repo_path), "workspace_root": workspace_root, "exists": False, "is_git": False, diff --git a/src/vcspull/cli/sync.py b/src/vcspull/cli/sync.py index d893d12b..2336ef05 100644 --- a/src/vcspull/cli/sync.py +++ b/src/vcspull/cli/sync.py @@ -264,7 +264,7 @@ def _build_plan_entry( return PlanEntry( name=str(repo.get("name", "unknown")), - path=str(repo_path), + path=contract_user_home(repo_path), workspace_root=workspace_root, action=action, detail=detail, @@ -724,7 +724,7 @@ def silent_progress(output: str, timestamp: datetime) -> None: event: dict[str, t.Any] = { "reason": "sync", "name": repo_name, - "path": str(repo_path), + "path": contract_user_home(repo_path), "workspace_root": str(workspace_label), } diff --git a/tests/cli/test_list.py b/tests/cli/test_list.py index 87a1cdc3..30b9bbc0 100644 --- a/tests/cli/test_list.py +++ b/tests/cli/test_list.py @@ -265,3 +265,98 @@ def test_list_repos_pattern_no_match( captured = capsys.readouterr() assert "No repositories found" in captured.out + + +# Tests for path contraction in JSON output + + +class PathContractionFixture(t.NamedTuple): + """Fixture for testing path contraction in JSON/NDJSON output.""" + + test_id: str + output_json: bool + output_ndjson: bool + tree: bool + + +PATH_CONTRACTION_FIXTURES: list[PathContractionFixture] = [ + PathContractionFixture( + test_id="json-output-contracts-paths", + output_json=True, + output_ndjson=False, + tree=False, + ), + PathContractionFixture( + test_id="ndjson-output-contracts-paths", + output_json=False, + output_ndjson=True, + tree=False, + ), + PathContractionFixture( + test_id="json-tree-output-contracts-paths", + output_json=True, + output_ndjson=False, + tree=True, + ), +] + + +@pytest.mark.parametrize( + list(PathContractionFixture._fields), + PATH_CONTRACTION_FIXTURES, + ids=[fixture.test_id for fixture in PATH_CONTRACTION_FIXTURES], +) +def test_list_repos_path_contraction( + test_id: str, + output_json: bool, + output_ndjson: bool, + tree: bool, + tmp_path: pathlib.Path, + monkeypatch: MonkeyPatch, + capsys: t.Any, +) -> None: + """Test that JSON/NDJSON output contracts home directory paths.""" + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.chdir(tmp_path) + + config_file = tmp_path / ".vcspull.yaml" + config_data = { + "~/code/": { + "flask": {"repo": "git+https://github.com/pallets/flask.git"}, + "django": {"repo": "git+https://github.com/django/django.git"}, + }, + } + create_test_config(config_file, config_data) + + list_repos( + repo_patterns=[], + config_path=config_file, + workspace_root=None, + tree=tree, + output_json=output_json, + output_ndjson=output_ndjson, + color="never", + ) + + captured = capsys.readouterr() + + if output_json: + output_data = json.loads(captured.out) + assert isinstance(output_data, list) + for item in output_data: + path = item["path"] + # Path should start with ~/ not /home// + assert path.startswith("~/"), f"Path {path} should be contracted to ~/..." + assert not path.startswith(str(tmp_path)), ( + f"Path {path} should not contain absolute home path" + ) + elif output_ndjson: + lines = [line for line in captured.out.strip().split("\n") if line] + for line in lines: + item = json.loads(line) + path = item["path"] + # Path should start with ~/ not /home// + assert path.startswith("~/"), f"Path {path} should be contracted to ~/..." + assert not path.startswith(str(tmp_path)), ( + f"Path {path} should not contain absolute home path" + ) diff --git a/tests/cli/test_status.py b/tests/cli/test_status.py index 4cf157aa..fa45ebaf 100644 --- a/tests/cli/test_status.py +++ b/tests/cli/test_status.py @@ -843,3 +843,112 @@ def test_status_repos_concurrent_max_concurrent_limit( status_entries = [item for item in output_data if item.get("reason") == "status"] assert len(status_entries) == 5 # All repos should be checked + + +# Tests for path contraction in JSON output + + +class StatusPathContractionFixture(t.NamedTuple): + """Fixture for testing path contraction in status JSON/NDJSON output.""" + + test_id: str + output_json: bool + output_ndjson: bool + detailed: bool + + +STATUS_PATH_CONTRACTION_FIXTURES: list[StatusPathContractionFixture] = [ + StatusPathContractionFixture( + test_id="json-output-contracts-paths", + output_json=True, + output_ndjson=False, + detailed=False, + ), + StatusPathContractionFixture( + test_id="ndjson-output-contracts-paths", + output_json=False, + output_ndjson=True, + detailed=False, + ), + StatusPathContractionFixture( + test_id="json-detailed-contracts-paths", + output_json=True, + output_ndjson=False, + detailed=True, + ), +] + + +@pytest.mark.parametrize( + list(StatusPathContractionFixture._fields), + STATUS_PATH_CONTRACTION_FIXTURES, + ids=[fixture.test_id for fixture in STATUS_PATH_CONTRACTION_FIXTURES], +) +def test_status_repos_path_contraction( + test_id: str, + output_json: bool, + output_ndjson: bool, + detailed: bool, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, + capsys: t.Any, +) -> None: + """Test that status JSON/NDJSON output contracts home directory paths.""" + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.chdir(tmp_path) + + config_file = tmp_path / ".vcspull.yaml" + repo1_path = tmp_path / "code" / "repo1" + repo2_path = tmp_path / "code" / "repo2" + + config_data = { + str(tmp_path / "code") + "/": { + "repo1": {"repo": "git+https://github.com/user/repo1.git"}, + "repo2": {"repo": "git+https://github.com/user/repo2.git"}, + }, + } + create_test_config(config_file, config_data) + + init_git_repo(repo1_path) + init_git_repo(repo2_path) + + status_repos( + repo_patterns=[], + config_path=config_file, + workspace_root=None, + detailed=detailed, + output_json=output_json, + output_ndjson=output_ndjson, + color="never", + concurrent=False, # Use sequential for deterministic testing + max_concurrent=None, + ) + + captured = capsys.readouterr() + + if output_json: + output_data = json.loads(captured.out) + status_entries = [ + item for item in output_data if item.get("reason") == "status" + ] + for entry in status_entries: + path = entry["path"] + # Path should start with ~/ not /home// + assert path.startswith("~/"), f"Path {path} should be contracted to ~/..." + assert not path.startswith(str(tmp_path)), ( + f"Path {path} should not contain absolute home path" + ) + elif output_ndjson: + lines = [line for line in captured.out.strip().split("\n") if line] + status_entries = [ + json.loads(line) + for line in lines + if json.loads(line).get("reason") == "status" + ] + for entry in status_entries: + path = entry["path"] + # Path should start with ~/ not /home// + assert path.startswith("~/"), f"Path {path} should be contracted to ~/..." + assert not path.startswith(str(tmp_path)), ( + f"Path {path} should not contain absolute home path" + )