Skip to content

Commit f807c5f

Browse files
authored
cli(chore[privacy]): Contract home paths in JSON/NDJSON output (#475)
why: JSON/NDJSON output had inconsistent path representation where workspace_root used tilde notation (~/code/) but path used absolute paths (/home/username/code/repo). This exposed usernames, reduced portability between machines, and created confusing inconsistency in the API. what: - Apply contract_user_home() to path fields in list, status, and sync commands - JSON/NDJSON now consistently uses ~/... for all home directory paths - Add 6 comprehensive parametrized tests using NamedTuple pattern - Document as breaking change in CHANGES with migration guide - Human-readable output unchanged (only JSON/NDJSON affected) refs: breaking change - automation consuming JSON/NDJSON output may need to expand ~ to absolute paths. Most tools handle tilde expansion automatically. See migration guide in CHANGES for details.
2 parents 639c049 + 52da2d7 commit f807c5f

File tree

6 files changed

+238
-8
lines changed

6 files changed

+238
-8
lines changed

CHANGES

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,33 @@ $ pipx install --suffix=@next 'vcspull' --pip-args '\--pre' --force
3131

3232
<!-- Maintainers, insert changes / features for the next release here -->
3333

34-
_Notes on upcoming releases will be added here_
34+
### Breaking Changes
35+
36+
#### JSON/NDJSON paths now use tilde notation
37+
38+
**All commands** (`list`, `status`, `sync`) now contract home directory paths in JSON/NDJSON output for consistency, privacy, and portability.
39+
40+
**Before:**
41+
```json
42+
{
43+
"name": "flask",
44+
"path": "/home/username/code/flask",
45+
"workspace_root": "~/code/"
46+
}
47+
```
48+
49+
**After:**
50+
```json
51+
{
52+
"name": "flask",
53+
"path": "~/code/flask",
54+
"workspace_root": "~/code/"
55+
}
56+
```
57+
58+
**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.
59+
60+
**Impact:** Automation consuming JSON/NDJSON output will need to expand `~` to absolute paths if required. Most tools handle tilde expansion automatically.
3561

3662
## vcspull v1.40.0 (2025-10-19)
3763

src/vcspull/cli/list.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,12 +161,12 @@ def _output_flat(
161161
repo_url = repo.get("url", repo.get("pip_url", "unknown"))
162162
repo_path = repo.get("path", "unknown")
163163

164-
# JSON/NDJSON output
164+
# JSON/NDJSON output (contract home for privacy/portability)
165165
formatter.emit(
166166
{
167167
"name": repo_name,
168168
"url": str(repo_url),
169-
"path": str(repo_path),
169+
"path": contract_user_home(repo_path),
170170
"workspace_root": str(repo.get("workspace_root", "")),
171171
}
172172
)
@@ -212,12 +212,12 @@ def _output_tree(
212212
repo_url = repo.get("url", repo.get("pip_url", "unknown"))
213213
repo_path = repo.get("path", "unknown")
214214

215-
# JSON/NDJSON output
215+
# JSON/NDJSON output (contract home for privacy/portability)
216216
formatter.emit(
217217
{
218218
"name": repo_name,
219219
"url": str(repo_url),
220-
"path": str(repo_path),
220+
"path": contract_user_home(repo_path),
221221
"workspace_root": workspace,
222222
}
223223
)

src/vcspull/cli/status.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ def check_repo_status(repo: ConfigDict, detailed: bool = False) -> dict[str, t.A
263263

264264
status: dict[str, t.Any] = {
265265
"name": repo_name,
266-
"path": str(repo_path),
266+
"path": contract_user_home(repo_path),
267267
"workspace_root": workspace_root,
268268
"exists": False,
269269
"is_git": False,

src/vcspull/cli/sync.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ def _build_plan_entry(
264264

265265
return PlanEntry(
266266
name=str(repo.get("name", "unknown")),
267-
path=str(repo_path),
267+
path=contract_user_home(repo_path),
268268
workspace_root=workspace_root,
269269
action=action,
270270
detail=detail,
@@ -724,7 +724,7 @@ def silent_progress(output: str, timestamp: datetime) -> None:
724724
event: dict[str, t.Any] = {
725725
"reason": "sync",
726726
"name": repo_name,
727-
"path": str(repo_path),
727+
"path": contract_user_home(repo_path),
728728
"workspace_root": str(workspace_label),
729729
}
730730

tests/cli/test_list.py

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,3 +265,98 @@ def test_list_repos_pattern_no_match(
265265

266266
captured = capsys.readouterr()
267267
assert "No repositories found" in captured.out
268+
269+
270+
# Tests for path contraction in JSON output
271+
272+
273+
class PathContractionFixture(t.NamedTuple):
274+
"""Fixture for testing path contraction in JSON/NDJSON output."""
275+
276+
test_id: str
277+
output_json: bool
278+
output_ndjson: bool
279+
tree: bool
280+
281+
282+
PATH_CONTRACTION_FIXTURES: list[PathContractionFixture] = [
283+
PathContractionFixture(
284+
test_id="json-output-contracts-paths",
285+
output_json=True,
286+
output_ndjson=False,
287+
tree=False,
288+
),
289+
PathContractionFixture(
290+
test_id="ndjson-output-contracts-paths",
291+
output_json=False,
292+
output_ndjson=True,
293+
tree=False,
294+
),
295+
PathContractionFixture(
296+
test_id="json-tree-output-contracts-paths",
297+
output_json=True,
298+
output_ndjson=False,
299+
tree=True,
300+
),
301+
]
302+
303+
304+
@pytest.mark.parametrize(
305+
list(PathContractionFixture._fields),
306+
PATH_CONTRACTION_FIXTURES,
307+
ids=[fixture.test_id for fixture in PATH_CONTRACTION_FIXTURES],
308+
)
309+
def test_list_repos_path_contraction(
310+
test_id: str,
311+
output_json: bool,
312+
output_ndjson: bool,
313+
tree: bool,
314+
tmp_path: pathlib.Path,
315+
monkeypatch: MonkeyPatch,
316+
capsys: t.Any,
317+
) -> None:
318+
"""Test that JSON/NDJSON output contracts home directory paths."""
319+
monkeypatch.setenv("HOME", str(tmp_path))
320+
monkeypatch.chdir(tmp_path)
321+
322+
config_file = tmp_path / ".vcspull.yaml"
323+
config_data = {
324+
"~/code/": {
325+
"flask": {"repo": "git+https://github.com/pallets/flask.git"},
326+
"django": {"repo": "git+https://github.com/django/django.git"},
327+
},
328+
}
329+
create_test_config(config_file, config_data)
330+
331+
list_repos(
332+
repo_patterns=[],
333+
config_path=config_file,
334+
workspace_root=None,
335+
tree=tree,
336+
output_json=output_json,
337+
output_ndjson=output_ndjson,
338+
color="never",
339+
)
340+
341+
captured = capsys.readouterr()
342+
343+
if output_json:
344+
output_data = json.loads(captured.out)
345+
assert isinstance(output_data, list)
346+
for item in output_data:
347+
path = item["path"]
348+
# Path should start with ~/ not /home/<user>/
349+
assert path.startswith("~/"), f"Path {path} should be contracted to ~/..."
350+
assert not path.startswith(str(tmp_path)), (
351+
f"Path {path} should not contain absolute home path"
352+
)
353+
elif output_ndjson:
354+
lines = [line for line in captured.out.strip().split("\n") if line]
355+
for line in lines:
356+
item = json.loads(line)
357+
path = item["path"]
358+
# Path should start with ~/ not /home/<user>/
359+
assert path.startswith("~/"), f"Path {path} should be contracted to ~/..."
360+
assert not path.startswith(str(tmp_path)), (
361+
f"Path {path} should not contain absolute home path"
362+
)

tests/cli/test_status.py

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,3 +843,112 @@ def test_status_repos_concurrent_max_concurrent_limit(
843843

844844
status_entries = [item for item in output_data if item.get("reason") == "status"]
845845
assert len(status_entries) == 5 # All repos should be checked
846+
847+
848+
# Tests for path contraction in JSON output
849+
850+
851+
class StatusPathContractionFixture(t.NamedTuple):
852+
"""Fixture for testing path contraction in status JSON/NDJSON output."""
853+
854+
test_id: str
855+
output_json: bool
856+
output_ndjson: bool
857+
detailed: bool
858+
859+
860+
STATUS_PATH_CONTRACTION_FIXTURES: list[StatusPathContractionFixture] = [
861+
StatusPathContractionFixture(
862+
test_id="json-output-contracts-paths",
863+
output_json=True,
864+
output_ndjson=False,
865+
detailed=False,
866+
),
867+
StatusPathContractionFixture(
868+
test_id="ndjson-output-contracts-paths",
869+
output_json=False,
870+
output_ndjson=True,
871+
detailed=False,
872+
),
873+
StatusPathContractionFixture(
874+
test_id="json-detailed-contracts-paths",
875+
output_json=True,
876+
output_ndjson=False,
877+
detailed=True,
878+
),
879+
]
880+
881+
882+
@pytest.mark.parametrize(
883+
list(StatusPathContractionFixture._fields),
884+
STATUS_PATH_CONTRACTION_FIXTURES,
885+
ids=[fixture.test_id for fixture in STATUS_PATH_CONTRACTION_FIXTURES],
886+
)
887+
def test_status_repos_path_contraction(
888+
test_id: str,
889+
output_json: bool,
890+
output_ndjson: bool,
891+
detailed: bool,
892+
tmp_path: pathlib.Path,
893+
monkeypatch: pytest.MonkeyPatch,
894+
capsys: t.Any,
895+
) -> None:
896+
"""Test that status JSON/NDJSON output contracts home directory paths."""
897+
monkeypatch.setenv("HOME", str(tmp_path))
898+
monkeypatch.chdir(tmp_path)
899+
900+
config_file = tmp_path / ".vcspull.yaml"
901+
repo1_path = tmp_path / "code" / "repo1"
902+
repo2_path = tmp_path / "code" / "repo2"
903+
904+
config_data = {
905+
str(tmp_path / "code") + "/": {
906+
"repo1": {"repo": "git+https://github.com/user/repo1.git"},
907+
"repo2": {"repo": "git+https://github.com/user/repo2.git"},
908+
},
909+
}
910+
create_test_config(config_file, config_data)
911+
912+
init_git_repo(repo1_path)
913+
init_git_repo(repo2_path)
914+
915+
status_repos(
916+
repo_patterns=[],
917+
config_path=config_file,
918+
workspace_root=None,
919+
detailed=detailed,
920+
output_json=output_json,
921+
output_ndjson=output_ndjson,
922+
color="never",
923+
concurrent=False, # Use sequential for deterministic testing
924+
max_concurrent=None,
925+
)
926+
927+
captured = capsys.readouterr()
928+
929+
if output_json:
930+
output_data = json.loads(captured.out)
931+
status_entries = [
932+
item for item in output_data if item.get("reason") == "status"
933+
]
934+
for entry in status_entries:
935+
path = entry["path"]
936+
# Path should start with ~/ not /home/<user>/
937+
assert path.startswith("~/"), f"Path {path} should be contracted to ~/..."
938+
assert not path.startswith(str(tmp_path)), (
939+
f"Path {path} should not contain absolute home path"
940+
)
941+
elif output_ndjson:
942+
lines = [line for line in captured.out.strip().split("\n") if line]
943+
status_entries = [
944+
json.loads(line)
945+
for line in lines
946+
if json.loads(line).get("reason") == "status"
947+
]
948+
for entry in status_entries:
949+
path = entry["path"]
950+
# Path should start with ~/ not /home/<user>/
951+
assert path.startswith("~/"), f"Path {path} should be contracted to ~/..."
952+
assert not path.startswith(str(tmp_path)), (
953+
f"Path {path} should not contain absolute home path"
954+
)

0 commit comments

Comments
 (0)