Skip to content

Commit fdb0bfb

Browse files
refactor(sonar): resolve sonar issues (#7419)
1 parent 84f8405 commit fdb0bfb

3 files changed

Lines changed: 83 additions & 7 deletions

File tree

gh-pages-template/index.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@
4242
</select>
4343
</div>
4444
<div class="col-12 col-md-6 col-lg-5">
45-
<input type="text" class="form-control rounded-0" placeholder="Search" id="search_term">
45+
<input type="text" class="form-control rounded-0" placeholder="Search" id="search_term"
46+
aria-label="Search">
4647
</div>
4748
<button type="button" class="btn btn-warning col-12 col-md-auto rounded-0" onclick="run_search()">
4849
<i class="fa-fw fas fa-search"></i>Search</button>

src/queue_eligibility.py

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,24 +52,71 @@ def normalize_user_id(user_id: object) -> str:
5252
return str(user_id).strip()
5353

5454

55-
def load_auto_approved_user_ids(auto_approved_users_file: Path) -> frozenset[str]:
55+
def resolve_auto_approved_users_file(
56+
auto_approved_users_file: Path,
57+
base_dir: Path | None = None,
58+
) -> Path:
59+
"""
60+
Resolve an auto-approved users file path within the allowed base directory.
61+
62+
Parameters
63+
----------
64+
auto_approved_users_file : pathlib.Path
65+
Candidate JSON file path.
66+
base_dir : pathlib.Path, optional
67+
Directory that may contain the allowlist file.
68+
69+
Returns
70+
-------
71+
pathlib.Path
72+
Canonical allowlist file path.
73+
74+
Raises
75+
------
76+
ValueError
77+
If the resolved path escapes the allowed base directory.
78+
"""
79+
allowed_base_dir = (Path.cwd() if base_dir is None else base_dir).resolve()
80+
candidate_file = auto_approved_users_file
81+
if not candidate_file.is_absolute():
82+
candidate_file = allowed_base_dir / candidate_file
83+
84+
resolved_file = candidate_file.resolve()
85+
try:
86+
resolved_file.relative_to(allowed_base_dir)
87+
except ValueError as error:
88+
raise ValueError('auto-approved users file must be inside the working directory') from error
89+
90+
return resolved_file
91+
92+
93+
def load_auto_approved_user_ids(
94+
auto_approved_users_file: Path,
95+
base_dir: Path | None = None,
96+
) -> frozenset[str]:
5697
"""
5798
Load auto-approved GitHub user ids from a JSON file.
5899
59100
Parameters
60101
----------
61102
auto_approved_users_file : pathlib.Path
62103
JSON file containing auto-approved user objects.
104+
base_dir : pathlib.Path, optional
105+
Directory that may contain the allowlist file.
63106
64107
Returns
65108
-------
66109
frozenset[str]
67110
Normalized GitHub user ids.
68111
"""
69112
try:
70-
with auto_approved_users_file.open(encoding='utf-8') as auto_approved_users_f:
113+
safe_auto_approved_users_file = resolve_auto_approved_users_file(
114+
auto_approved_users_file=auto_approved_users_file,
115+
base_dir=base_dir,
116+
)
117+
with safe_auto_approved_users_file.open(encoding='utf-8') as auto_approved_users_f:
71118
auto_approved_users = json.load(auto_approved_users_f)
72-
except (OSError, json.JSONDecodeError):
119+
except (OSError, ValueError):
73120
return frozenset()
74121

75122
if not isinstance(auto_approved_users, list):

tests/unit/test_queue_eligibility.py

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ def test_load_auto_approved_user_ids_loads_user_ids(tmp_path):
3030

3131
auto_approved_user_ids = queue_eligibility.load_auto_approved_user_ids(
3232
auto_approved_users_file=auto_approved_users_file,
33+
base_dir=tmp_path,
3334
)
3435

3536
assert auto_approved_user_ids == frozenset({'42013603', '88998541'})
@@ -61,6 +62,7 @@ def test_load_auto_approved_user_ids_ignores_malformed_entries(tmp_path):
6162

6263
auto_approved_user_ids = queue_eligibility.load_auto_approved_user_ids(
6364
auto_approved_users_file=auto_approved_users_file,
65+
base_dir=tmp_path,
6466
)
6567

6668
assert auto_approved_user_ids == frozenset({'1234'})
@@ -71,8 +73,14 @@ def test_load_auto_approved_user_ids_fails_closed_for_missing_or_bad_file(tmp_pa
7173
bad_file = tmp_path / 'bad.json'
7274
bad_file.write_text('{', encoding='utf-8')
7375

74-
assert queue_eligibility.load_auto_approved_user_ids(auto_approved_users_file=missing_file) == frozenset()
75-
assert queue_eligibility.load_auto_approved_user_ids(auto_approved_users_file=bad_file) == frozenset()
76+
assert queue_eligibility.load_auto_approved_user_ids(
77+
auto_approved_users_file=missing_file,
78+
base_dir=tmp_path,
79+
) == frozenset()
80+
assert queue_eligibility.load_auto_approved_user_ids(
81+
auto_approved_users_file=bad_file,
82+
base_dir=tmp_path,
83+
) == frozenset()
7684

7785

7886
def test_load_auto_approved_user_ids_fails_closed_for_non_list_json(tmp_path):
@@ -82,6 +90,25 @@ def test_load_auto_approved_user_ids_fails_closed_for_non_list_json(tmp_path):
8290

8391
auto_approved_user_ids = queue_eligibility.load_auto_approved_user_ids(
8492
auto_approved_users_file=auto_approved_users_file,
93+
base_dir=tmp_path,
94+
)
95+
96+
assert auto_approved_user_ids == frozenset()
97+
98+
99+
def test_load_auto_approved_user_ids_rejects_path_outside_base_dir(tmp_path):
100+
base_dir = tmp_path / 'base'
101+
base_dir.mkdir()
102+
outside_file = write_auto_approved_users_file(tmp_path, [
103+
{
104+
'user_id': 42013603,
105+
'username': 'ReenigneArcher',
106+
},
107+
])
108+
109+
auto_approved_user_ids = queue_eligibility.load_auto_approved_user_ids(
110+
auto_approved_users_file=Path('..') / outside_file.name,
111+
base_dir=base_dir,
85112
)
86113

87114
assert auto_approved_user_ids == frozenset()
@@ -213,10 +240,11 @@ def test_main_writes_queue_eligible_result(tmp_path, monkeypatch):
213240
])
214241
output_file = tmp_path / 'github_output'
215242
monkeypatch.setenv('GITHUB_OUTPUT', str(output_file))
243+
monkeypatch.chdir(tmp_path)
216244
monkeypatch.setattr(sys, 'argv', [
217245
'queue_eligibility',
218246
'--auto-approved-users-file',
219-
str(auto_approved_users_file),
247+
auto_approved_users_file.name,
220248
'--user-id',
221249
'42013603',
222250
])

0 commit comments

Comments
 (0)