Skip to content

Commit 948ac56

Browse files
committed
fix(import[common]) Reject --skip-group values containing '/'
why: A value like "bots/subteam" is added to skip_set as-is, but filter_repo splits repo.owner on "/" and compares individual segments, so slash-containing values silently have no effect on filtering. what: - Validate skip_groups_list before ImportOptions construction in _run_import - Return 1 with a clear error for any group value containing "/" - Add test_run_import_rejects_skip_group_with_slash in test_import_repos.py
1 parent a57a2f6 commit 948ac56

2 files changed

Lines changed: 59 additions & 0 deletions

File tree

src/vcspull/cli/import_cmd/_common.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,19 @@ def _run_import(
355355

356356
skip_groups_list: list[str] = skip_groups or []
357357

358+
# Validate skip_groups: each value must be a single path segment.
359+
# A slash inside a value like "bots/subteam" would never match any
360+
# owner segment (filter_repo splits repo.owner on "/" and compares
361+
# individual parts), so it silently has no effect.
362+
for group in skip_groups_list:
363+
if "/" in group:
364+
log.error(
365+
"--skip-group values must be single path segments; "
366+
"'/' is not allowed: %r",
367+
group,
368+
)
369+
return 1
370+
358371
try:
359372
options = ImportOptions(
360373
mode=import_mode,

tests/cli/test_import_repos.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2024,6 +2024,52 @@ def test_codecommit_target_is_optional() -> None:
20242024
assert args.target == "myprefix"
20252025

20262026

2027+
def test_run_import_rejects_skip_group_with_slash(
2028+
tmp_path: pathlib.Path,
2029+
monkeypatch: pytest.MonkeyPatch,
2030+
caplog: pytest.LogCaptureFixture,
2031+
) -> None:
2032+
"""_run_import returns 1 when a --skip-group value contains a slash.
2033+
2034+
A value like 'bots/subteam' can never match any single owner path segment
2035+
(filter_repo splits repo.owner on '/' and compares segments individually),
2036+
so such values silently have no effect. The early validation catches
2037+
this and returns a non-zero exit code.
2038+
"""
2039+
import logging
2040+
2041+
caplog.set_level(logging.ERROR)
2042+
monkeypatch.setenv("HOME", str(tmp_path))
2043+
2044+
workspace = tmp_path / "repos"
2045+
workspace.mkdir()
2046+
2047+
result = _run_import(
2048+
MockImporter(),
2049+
service_name="gitlab",
2050+
target="my-group",
2051+
workspace=str(workspace),
2052+
mode="org",
2053+
language=None,
2054+
topics=None,
2055+
min_stars=0,
2056+
include_archived=False,
2057+
include_forks=False,
2058+
limit=100,
2059+
config_path_str=str(tmp_path / "config.yaml"),
2060+
dry_run=False,
2061+
yes=True,
2062+
output_json=False,
2063+
output_ndjson=False,
2064+
color="never",
2065+
skip_groups=["bots/subteam"],
2066+
)
2067+
2068+
assert result == 1
2069+
assert "bots/subteam" in caplog.text
2070+
assert "'/' is not allowed" in caplog.text
2071+
2072+
20272073
def test_run_import_forwards_with_shared_and_skip_groups(
20282074
tmp_path: pathlib.Path,
20292075
monkeypatch: MonkeyPatch,

0 commit comments

Comments
 (0)