Skip to content

Commit ff3d3af

Browse files
committed
fix(cli): validate --repo filter on gitt issues list (entrius#1061)
The `--repo <owner/name>` filter added in entrius#910 compares raw user input directly against `repository_full_name`, so: - malformed inputs like `ownerrepo` or `owner//repo` are silently accepted and quietly match nothing (no error to the user); - whitespace-padded valid inputs like `' entrius/gittensor '` fail to match real entries because the comparison string still contains the surrounding spaces. Fix by reusing the existing `validate_repository(verify_exists=False)` helper before any contract read, normalizing `repo_filter` to a clean `owner/repo` string and surfacing `click.BadParameter` through the existing `handle_exception` path so JSON consumers get a structured `bad_parameter` error and human callers exit non-zero. This is the same validator already used by `gitt issues register/cancel` mutating commands, so the read-side `--repo` filter now matches the documented contract enforced everywhere else. ## Tests - Malformed filters (`ownerrepo`, `owner//repo`, `owner/`, `/repo`, `owner repo`) rejected with structured `bad_parameter` error in JSON mode and non-zero exit in human mode, before any contract read. - Whitespace-padded valid filter `' owner/repo '` correctly matches the contract entry and returns the issue. - Mixed-case filter `OWNER/REPO` still matches (preserves existing case-insensitive behavior). - Valid non-matching filter returns empty list rather than all issues. Closes entrius#1061
1 parent 9b620da commit ff3d3af

2 files changed

Lines changed: 121 additions & 0 deletions

File tree

gittensor/cli/issue_commands/view.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
print_network_header,
3333
read_issues_from_contract,
3434
validate_issue_id,
35+
validate_repository,
3536
with_cli_behavior_options,
3637
with_network_contract_options,
3738
)
@@ -88,6 +89,18 @@ def issues_list(
8889
except click.BadParameter as e:
8990
handle_exception(as_json, str(e), 'bad_parameter')
9091

92+
# Normalize and validate the --repo filter before any contract reads so
93+
# malformed input is rejected up-front and whitespace-padded valid input
94+
# still matches `repository_full_name` from the contract. validate_repository
95+
# strips whitespace, enforces owner/name format, and raises
96+
# click.BadParameter on bad input — same contract used by mutating commands.
97+
if repo_filter is not None:
98+
try:
99+
owner, repo_name = validate_repository(repo_filter, verify_exists=False)
100+
repo_filter = f'{owner}/{repo_name}'
101+
except click.BadParameter as e:
102+
handle_exception(as_json, str(e), 'bad_parameter')
103+
91104
contract_addr, ws_endpoint, network_name = _resolve_contract_and_network(
92105
contract,
93106
network,

tests/cli/test_issues_list_json.py

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,111 @@ def test_issues_list_rejects_invalid_id_json(cli_root, runner, bad_id):
7878
assert payload['error']['type'] == 'bad_parameter'
7979
assert 'between 1 and 999999' in payload['error']['message']
8080
mock_read.assert_not_called()
81+
82+
83+
# ---------------------------------------------------------------------------
84+
# --repo filter validation (regression for #1061)
85+
# ---------------------------------------------------------------------------
86+
87+
88+
def _parse_json_payload(output: str) -> dict:
89+
"""Extract and parse the JSON object from CLI output.
90+
91+
Some success paths print a `Network: <name> • Contract: <addr>` header
92+
before the JSON body, which trips json.loads on the raw output. Slice
93+
from the first '{' so the parse is robust regardless of header presence.
94+
"""
95+
start = output.find('{')
96+
if start < 0:
97+
raise AssertionError(f'no JSON object found in output: {output[:200]!r}')
98+
return json.loads(output[start:])
99+
100+
101+
@pytest.mark.parametrize('bad_repo', ['ownerrepo', 'owner//repo', 'owner/', '/repo', 'owner repo'])
102+
def test_issues_list_rejects_malformed_repo_filter_json(cli_root, runner, bad_repo):
103+
"""Malformed --repo input must fail validation up-front before any contract read."""
104+
with patch('gittensor.cli.issue_commands.view.read_issues_from_contract') as mock_read:
105+
result = runner.invoke(
106+
cli_root, ['issues', 'list', '--json', '--repo', bad_repo], catch_exceptions=False
107+
)
108+
109+
assert result.exit_code != 0
110+
payload = json.loads(result.output)
111+
assert payload['success'] is False
112+
assert payload['error']['type'] == 'bad_parameter'
113+
mock_read.assert_not_called()
114+
115+
116+
@pytest.mark.parametrize('bad_repo', ['ownerrepo', 'owner//repo'])
117+
def test_issues_list_rejects_malformed_repo_filter_human(cli_root, runner, bad_repo):
118+
"""Human-mode --repo malformed input must also exit non-zero before contract read."""
119+
with patch('gittensor.cli.issue_commands.view.read_issues_from_contract') as mock_read:
120+
result = runner.invoke(cli_root, ['issues', 'list', '--repo', bad_repo], catch_exceptions=False)
121+
122+
assert result.exit_code != 0
123+
mock_read.assert_not_called()
124+
125+
126+
def test_issues_list_repo_filter_strips_whitespace_json(cli_root, runner):
127+
"""Whitespace-padded valid --repo input must trim and match contract repository_full_name."""
128+
with (
129+
patch(
130+
'gittensor.cli.issue_commands.view._resolve_contract_and_network',
131+
return_value=('5Fakeaddr', 'ws://x', 'test'),
132+
),
133+
patch('gittensor.cli.issue_commands.view.read_issues_from_contract', return_value=FAKE_ISSUES),
134+
):
135+
result = runner.invoke(
136+
cli_root,
137+
['issues', 'list', '--json', '--repo', ' owner/repo '],
138+
catch_exceptions=False,
139+
)
140+
141+
assert result.exit_code == 0
142+
payload = _parse_json_payload(result.output)
143+
assert payload['success'] is True
144+
assert payload['issue_count'] == 1
145+
assert payload['issues'][0]['repository_full_name'] == 'owner/repo'
146+
147+
148+
def test_issues_list_repo_filter_case_insensitive_json(cli_root, runner):
149+
"""--repo filter should match contract entries regardless of case (preserved existing behavior)."""
150+
with (
151+
patch(
152+
'gittensor.cli.issue_commands.view._resolve_contract_and_network',
153+
return_value=('5Fakeaddr', 'ws://x', 'test'),
154+
),
155+
patch('gittensor.cli.issue_commands.view.read_issues_from_contract', return_value=FAKE_ISSUES),
156+
):
157+
result = runner.invoke(
158+
cli_root,
159+
['issues', 'list', '--json', '--repo', 'OWNER/REPO'],
160+
catch_exceptions=False,
161+
)
162+
163+
assert result.exit_code == 0
164+
payload = _parse_json_payload(result.output)
165+
assert payload['success'] is True
166+
assert payload['issue_count'] == 1
167+
168+
169+
def test_issues_list_repo_filter_no_match_returns_empty_json(cli_root, runner):
170+
"""Valid but non-matching --repo input returns empty list, not all issues."""
171+
with (
172+
patch(
173+
'gittensor.cli.issue_commands.view._resolve_contract_and_network',
174+
return_value=('5Fakeaddr', 'ws://x', 'test'),
175+
),
176+
patch('gittensor.cli.issue_commands.view.read_issues_from_contract', return_value=FAKE_ISSUES),
177+
):
178+
result = runner.invoke(
179+
cli_root,
180+
['issues', 'list', '--json', '--repo', 'other/repo'],
181+
catch_exceptions=False,
182+
)
183+
184+
assert result.exit_code == 0
185+
payload = _parse_json_payload(result.output)
186+
assert payload['success'] is True
187+
assert payload['issue_count'] == 0
188+
assert payload['issues'] == []

0 commit comments

Comments
 (0)