Skip to content

Commit 15623af

Browse files
Feature/206 fix issues detected by aquasec scanner (#207)
* #206 - Fix issues detected by Aquasec scanner - CWE-295: Improper Certificate Validation - Code refactored to different solution.
1 parent dcf920a commit 15623af

File tree

6 files changed

+90
-85
lines changed

6 files changed

+90
-85
lines changed

DEVELOPER.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,13 @@ export INPUT_VERBOSE="true"
176176
export GITHUB_REPOSITORY="< owner >/< repo-name >"
177177
export INPUT_GITHUB_TOKEN=$(printenv <your-env-token-var>)
178178

179+
PROJECT_ROOT="$(pwd)"
180+
export PYTHONPATH="${PYTHONPATH}:${PROJECT_ROOT}"
181+
182+
# Debugging statements
183+
echo "PYTHONPATH: ${PYTHONPATH}"
184+
echo "Current working directory: ${PROJECT_ROOT}"
185+
179186
# Run the Python script
180187
python3 ./<path-to-action-project-root>/main.py
181188
```

release_notes_generator/record/factory/default_record_factory.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,9 @@ def _register_pull_and_its_commits_to_issue(
131131

132132
pr_repo = target_repository if target_repository is not None else data.home_repository
133133

134-
merged_linked_issues: set[str] = self._safe_call(get_issues_for_pr)(pull_number=pull.number) or set()
134+
merged_linked_issues: set[str] = (
135+
self._safe_call(get_issues_for_pr)(pull_number=pull.number, requester=self._github.requester) or set()
136+
)
135137
merged_linked_issues.update(extract_issue_numbers_from_body(pull, pr_repo))
136138
pull_issues: list[str] = list(merged_linked_issues)
137139
attached_any = False

release_notes_generator/utils/pull_request_utils.py

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@
2121
import re
2222
from functools import lru_cache
2323

24-
import requests
24+
from github import GithubException
25+
from github.Requester import Requester
2526
from github.PullRequest import PullRequest
2627
from github.Repository import Repository
2728

@@ -53,9 +54,8 @@ def extract_issue_numbers_from_body(pr: PullRequest, repository: Repository) ->
5354

5455

5556
@lru_cache(maxsize=1024)
56-
def get_issues_for_pr(pull_number: int) -> set[str]:
57+
def get_issues_for_pr(pull_number: int, requester: Requester) -> set[str]:
5758
"""Fetch closing issue numbers for a PR via GitHub GraphQL and return them as a set."""
58-
github_api_url = "https://api.github.com/graphql"
5959
owner = ActionInputs.get_github_owner()
6060
name = ActionInputs.get_github_repo_name()
6161
query = ISSUES_FOR_PRS.format(
@@ -69,13 +69,24 @@ def get_issues_for_pr(pull_number: int) -> set[str]:
6969
"Content-Type": "application/json",
7070
}
7171

72-
response = requests.post(github_api_url, json={"query": query}, headers=headers, verify=False, timeout=10)
73-
response.raise_for_status() # Raise an error for HTTP issues
74-
data = response.json()
75-
if data.get("errors"):
76-
raise RuntimeError(f"GitHub GraphQL errors: {data['errors']}")
72+
try:
73+
headers, payload = requester.graphql_query(query, headers)
74+
except GithubException as e:
75+
# e.status (int), e.data (dict/str) often contains useful details
76+
raise RuntimeError(f"GitHub HTTP error {getattr(e, 'status', '?')}: {getattr(e, 'data', e)}") from e
77+
78+
if not isinstance(payload, dict):
79+
raise RuntimeError(f"Malformed response payload type: {type(payload)}")
80+
81+
# GraphQL-level errors come inside a successful HTTP 200
82+
if "errors" in payload:
83+
messages = "; ".join(err.get("message", str(err)) for err in payload["errors"])
84+
raise RuntimeError(f"GitHub GraphQL errors: {messages}")
85+
86+
if "data" not in payload:
87+
raise RuntimeError("Malformed GraphQL response: missing 'data'")
7788

7889
return {
7990
f"{owner}/{name}#{node['number']}"
80-
for node in data["data"]["repository"]["pullRequest"]["closingIssuesReferences"]["nodes"]
91+
for node in payload["data"]["repository"]["pullRequest"]["closingIssuesReferences"]["nodes"]
8192
}

tests/unit/conftest.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from github.PullRequest import PullRequest
2626
from github.Rate import Rate
2727
from github.Repository import Repository
28+
from github.Requester import Requester
2829

2930
from release_notes_generator.model.record.commit_record import CommitRecord
3031
from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord
@@ -56,7 +57,7 @@ def wrapper(fn):
5657
return wrapper
5758

5859

59-
def mock_get_issues_for_pr(pull_number: int) -> set[int]:
60+
def mock_get_issues_for_pr(pull_number: int, requester: Requester) -> set[int]:
6061
# if pull_number == 150:
6162
# return [451]
6263
return set()

tests/unit/release_notes_generator/record/factory/test_default_record_factory.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from github.Commit import Commit
2222
from github.Issue import Issue
2323
from github.PullRequest import PullRequest
24+
from github.Requester import Requester
2425

2526
from release_notes_generator.model.record.commit_record import CommitRecord
2627
from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord
@@ -337,7 +338,7 @@ def wrapper(fn):
337338
return fn
338339
return wrapper
339340

340-
def mock_get_issues_for_pr_no_issues(pull_number: int) -> list[str]:
341+
def mock_get_issues_for_pr_no_issues(pull_number: int, requester: Requester) -> list[str]:
341342
return []
342343

343344

tests/unit/release_notes_generator/utils/test_pull_request_utils.py

Lines changed: 56 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,17 @@ def test_get_issues_for_pr_success(monkeypatch):
9797
_patch_action_inputs(monkeypatch)
9898
_patch_issues_template(monkeypatch)
9999

100-
captured = {}
101-
class Resp:
102-
def raise_for_status(self): pass
103-
def json(self):
104-
return {
100+
class MockRequester:
101+
def __init__(self):
102+
self.called = False
103+
self.last_query = None
104+
self.last_headers = None
105+
def graphql_query(self, query, headers):
106+
self.called = True
107+
self.last_query = query
108+
self.last_headers = headers
109+
# Return (headers, payload) as expected by the new implementation
110+
return headers, {
105111
"data": {
106112
"repository": {
107113
"pullRequest": {
@@ -113,34 +119,21 @@ def json(self):
113119
}
114120
}
115121

116-
def fake_post(url, json=None, headers=None, verify=None, timeout=None):
117-
captured["url"] = url
118-
captured["json"] = json
119-
captured["headers"] = headers
120-
captured["verify"] = verify
121-
captured["timeout"] = timeout
122-
return Resp()
123-
124-
monkeypatch.setattr(pru.requests, "post", fake_post)
125-
126-
result = pru.get_issues_for_pr(123)
122+
requester = MockRequester()
123+
result = pru.get_issues_for_pr(123, requester)
127124
assert result == {'OWN/REPO#11', 'OWN/REPO#22'}
128-
assert captured["url"] == "https://api.github.com/graphql"
129-
# Query string correctly formatted
130-
assert captured["json"]["query"] == "Q 123 OWN REPO 10"
131-
# Headers include token
132-
assert captured["headers"]["Authorization"] == "Bearer TOK"
133-
assert captured["verify"] is False
134-
assert captured["timeout"] == 10
125+
assert requester.called
126+
assert requester.last_query == "Q 123 OWN REPO 10"
127+
assert requester.last_headers["Authorization"] == "Bearer TOK"
128+
assert requester.last_headers["Content-Type"] == "application/json"
135129

136130
def test_get_issues_for_pr_empty_nodes(monkeypatch):
137131
_patch_action_inputs(monkeypatch)
138132
_patch_issues_template(monkeypatch)
139133

140-
class Resp:
141-
def raise_for_status(self): pass
142-
def json(self):
143-
return {
134+
class MockRequester:
135+
def graphql_query(self, query, headers):
136+
return headers, {
144137
"data": {
145138
"repository": {
146139
"pullRequest": {
@@ -150,48 +143,49 @@ def json(self):
150143
}
151144
}
152145

153-
monkeypatch.setattr(pru.requests, "post", lambda *a, **k: Resp())
154-
assert pru.get_issues_for_pr(5) == set()
146+
requester = MockRequester()
147+
assert pru.get_issues_for_pr(5, requester) == set()
155148

156149
def test_get_issues_for_pr_http_error(monkeypatch):
157150
_patch_action_inputs(monkeypatch)
158151
_patch_issues_template(monkeypatch)
159152

160-
class Resp:
161-
def raise_for_status(self):
162-
raise requests.HTTPError("Boom")
163-
def json(self):
164-
return {}
153+
from github import GithubException
154+
class MockGithubException(GithubException):
155+
def __init__(self, status, data):
156+
super().__init__(status, data)
157+
# Do not set self.status or self.data directly
165158

166-
monkeypatch.setattr(pru.requests, "post", lambda *a, **k: Resp())
159+
class MockRequester:
160+
def graphql_query(self, query, headers):
161+
raise MockGithubException(500, "Boom")
167162

168-
with pytest.raises(requests.HTTPError):
169-
pru.get_issues_for_pr(77)
163+
requester = MockRequester()
164+
with pytest.raises(RuntimeError) as excinfo:
165+
pru.get_issues_for_pr(77, requester)
166+
assert "GitHub HTTP error 500" in str(excinfo.value)
170167

171168
def test_get_issues_for_pr_malformed_response(monkeypatch):
172169
_patch_action_inputs(monkeypatch)
173170
_patch_issues_template(monkeypatch)
174171

175-
class Resp:
176-
def raise_for_status(self): pass
177-
def json(self):
178-
# Missing the expected nested keys -> triggers KeyError
179-
return {"data": {}}
180-
181-
monkeypatch.setattr(pru.requests, "post", lambda *a, **k: Resp())
172+
class MockRequester:
173+
def graphql_query(self, query, headers):
174+
return headers, {"data": {}} # Missing nested keys
182175

176+
requester = MockRequester()
183177
with pytest.raises(KeyError):
184-
pru.get_issues_for_pr(42)
178+
pru.get_issues_for_pr(42, requester)
185179

186180
def test_get_issues_for_pr_caching(monkeypatch):
187181
_patch_action_inputs(monkeypatch)
188182
_patch_issues_template(monkeypatch)
189183

190184
calls = {"count": 0}
191-
class Resp:
192-
def raise_for_status(self): pass
193-
def json(self):
194-
return {
185+
class MockRequester:
186+
def graphql_query(self, query, headers):
187+
calls["count"] += 1
188+
return headers, {
195189
"data": {
196190
"repository": {
197191
"pullRequest": {
@@ -201,14 +195,9 @@ def json(self):
201195
}
202196
}
203197

204-
def fake_post(*a, **k):
205-
calls["count"] += 1
206-
return Resp()
207-
208-
monkeypatch.setattr(pru.requests, "post", fake_post)
209-
210-
first = pru.get_issues_for_pr(900)
211-
second = pru.get_issues_for_pr(900) # should use cache
198+
requester = MockRequester()
199+
first = pru.get_issues_for_pr(900, requester)
200+
second = pru.get_issues_for_pr(900, requester) # should use cache
212201
assert first == {'OWN/REPO#9'} and second == {'OWN/REPO#9'}
213202
assert calls["count"] == 1 # only one network call
214203

@@ -217,30 +206,24 @@ def test_get_issues_for_pr_different_numbers_not_cached(monkeypatch):
217206
_patch_issues_template(monkeypatch)
218207

219208
calls = {"nums": []}
220-
class Resp:
221-
def __init__(self, n): self.n = n
222-
def raise_for_status(self): pass
223-
def json(self):
224-
return {
209+
class MockRequester:
210+
def graphql_query(self, query, headers):
211+
# Extract pull number back from formatted query tail
212+
pull_num = int(query.split()[1])
213+
calls["nums"].append(pull_num)
214+
return headers, {
225215
"data": {
226216
"repository": {
227217
"pullRequest": {
228-
"closingIssuesReferences": {"nodes": [{"number": self.n}]}
218+
"closingIssuesReferences": {"nodes": [{"number": pull_num}]}
229219
}
230220
}
231221
}
232222
}
233223

234-
def fake_post(url, json=None, **k):
235-
# Extract pull number back from formatted query tail
236-
pull_num = int(json["query"].split()[1])
237-
calls["nums"].append(pull_num)
238-
return Resp(pull_num)
239-
240-
monkeypatch.setattr(pru.requests, "post", fake_post)
241-
242-
r1 = pru.get_issues_for_pr(1)
243-
r2 = pru.get_issues_for_pr(2)
224+
requester = MockRequester()
225+
r1 = pru.get_issues_for_pr(1, requester)
226+
r2 = pru.get_issues_for_pr(2, requester)
244227
assert r1 == {'OWN/REPO#1'}
245228
assert r2 == {'OWN/REPO#2'}
246229
assert calls["nums"] == [1, 2]

0 commit comments

Comments
 (0)