Skip to content

Feature/206 fix issues detected by aquasec scanner #207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
miroslavpojer merged 4 commits into from
Nov 6, 2025
Merged

Feature/206 fix issues detected by aquasec scanner #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c8ae3d5
#206 - Fix issues detected by Aquasec scanner - CWE-295: Improper Cer…
miroslavpojer Nov 6, 2025
62a1cb0
#206 - Fix issues detected by Aquasec scanner - CWE-295: Improper Cer…
miroslavpojer Nov 6, 2025
6ffbe5c
Applied black.
miroslavpojer Nov 6, 2025
e1e2f10
Solution of review comments.
miroslavpojer Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions DEVELOPER.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,13 @@ export INPUT_VERBOSE="true"
export GITHUB_REPOSITORY="< owner >/< repo-name >"
export INPUT_GITHUB_TOKEN=$(printenv <your-env-token-var>)

PROJECT_ROOT="$(pwd)"
export PYTHONPATH="${PYTHONPATH}:${PROJECT_ROOT}"

# Debugging statements
echo "PYTHONPATH: ${PYTHONPATH}"
echo "Current working directory: ${PROJECT_ROOT}"

# Run the Python script
python3 ./<path-to-action-project-root>/main.py
```
Expand Down
4 changes: 3 additions & 1 deletion release_notes_generator/record/factory/default_record_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ def _register_pull_and_its_commits_to_issue(

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

merged_linked_issues: set[str] = self._safe_call(get_issues_for_pr)(pull_number=pull.number) or set()
merged_linked_issues: set[str] = (
self._safe_call(get_issues_for_pr)(pull_number=pull.number, requester=self._github.requester) or set()
)
merged_linked_issues.update(extract_issue_numbers_from_body(pull, pr_repo))
pull_issues: list[str] = list(merged_linked_issues)
attached_any = False
Expand Down
29 changes: 20 additions & 9 deletions release_notes_generator/utils/pull_request_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
import re
from functools import lru_cache

import requests
from github import GithubException
from github.Requester import Requester
from github.PullRequest import PullRequest
from github.Repository import Repository

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


@lru_cache(maxsize=1024)
def get_issues_for_pr(pull_number: int) -> set[str]:
def get_issues_for_pr(pull_number: int, requester: Requester) -> set[str]:
Comment on lines 56 to +57
Copy link

@coderabbitai coderabbitai bot Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: lru_cache incompatible with Requester parameter.

The @lru_cache decorator requires all parameters to be hashable to generate cache keys. The Requester object is likely unhashable, which will cause a TypeError at runtime. Even if it were hashable, caching based on the requester instance doesn't make semantic sense—the same PR number should return the same results regardless of which requester instance is used.

Refactor to exclude the requester from the cache key:

-@lru_cache(maxsize=1024)
-def get_issues_for_pr(pull_number: int, requester: Requester) -> set[str]:
-    """Fetch closing issue numbers for a PR via GitHub GraphQL and return them as a set."""
+def get_issues_for_pr(pull_number: int, requester: Requester) -> set[str]:
+    """Fetch closing issue numbers for a PR via GitHub GraphQL and return them as a set."""
+    return _get_issues_for_pr_cached(pull_number, requester)
+
+
+@lru_cache(maxsize=1024)
+def _get_issues_for_pr_cached(pull_number: int, requester: Requester) -> set[str]:
+    """Internal cached implementation."""
     owner = ActionInputs.get_github_owner()
     name = ActionInputs.get_github_repo_name()

Wait, that doesn't solve the problem. Better approach:

+@lru_cache(maxsize=1024)
+def _fetch_issues_from_graphql(pull_number: int) -> set[str]:
+    """Fetch closing issue numbers for a PR via GitHub GraphQL and return them as a set."""
+    # This function cannot accept requester as it must remain cacheable
+    # Instead, we'll need to refactor to either:
+    # 1. Remove caching entirely if requester varies
+    # 2. Pass requester globally or through a different mechanism
+    # 3. Cache at a higher level where requester is consistent
+    pass
+
 @lru_cache(maxsize=1024)
 def get_issues_for_pr(pull_number: int, requester: Requester) -> set[str]:

Actually, let me provide a cleaner solution that removes the requester from the cache signature:

-@lru_cache(maxsize=1024)
 def get_issues_for_pr(pull_number: int, requester: Requester) -> set[str]:
     """Fetch closing issue numbers for a PR via GitHub GraphQL and return them as a set."""
+    # Delegate to cached implementation that only caches based on pull_number
+    return _get_issues_for_pr_impl(pull_number, requester)
+
+
+# Cache key is based on (pull_number, owner, name) tuple for the query
+_cache = {}
+
+
+def _get_issues_for_pr_impl(pull_number: int, requester: Requester) -> set[str]:
+    """Internal implementation with caching based on pull_number only."""
     owner = ActionInputs.get_github_owner()
     name = ActionInputs.get_github_repo_name()
+    
+    cache_key = (pull_number, owner, name)
+    if cache_key in _cache:
+        return _cache[cache_key]
+    
     query = ISSUES_FOR_PRS.format(
         number=pull_number,
         owner=owner,
         name=name,
         first=LINKED_ISSUES_MAX,
     )
     headers = {
         "Authorization": f"Bearer {ActionInputs.get_github_token()}",
         "Content-Type": "application/json",
     }

     try:
         headers, payload = requester.graphql_query(query, headers)
     except GithubException as e:
         # e.status (int), e.data (dict/str) often contains useful details
         raise RuntimeError(f"GitHub HTTP error {getattr(e, 'status', '?')}: {getattr(e, 'data', e)}") from e

     if not isinstance(payload, dict):
         raise RuntimeError(f"Malformed response payload type: {type(payload)}")

     # GraphQL-level errors come inside a successful HTTP 200
     if "errors" in payload:
         messages = "; ".join(err.get("message", str(err)) for err in payload["errors"])
         raise RuntimeError(f"GitHub GraphQL errors: {messages}")

     if "data" not in payload:
         raise RuntimeError("Malformed GraphQL response: missing 'data'")

-    return {
+    result = {
         f"{owner}/{name}#{node['number']}"
         for node in payload["data"]["repository"]["pullRequest"]["closingIssuesReferences"]["nodes"]
     }
+    
+    _cache[cache_key] = result
+    return result

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In release_notes_generator/utils/pull_request_utils.py around lines 56-57, the
function is decorated with @lru_cache but accepts a Requester object
(unhashable) which will raise TypeError and incorrectly couples cache keys to
the requester; refactor by removing requester from the cached function signature
— create a cached helper that takes only pull_number and returns the issue set,
and have a thin non-cached wrapper that accepts (pull_number, requester) and
calls the cached helper after using the requester to fetch or inject necessary
data; update type hints accordingly and remove @lru_cache from any function that
still accepts Requester.

"""Fetch closing issue numbers for a PR via GitHub GraphQL and return them as a set."""
github_api_url = "https://api.github.com/graphql"
owner = ActionInputs.get_github_owner()
name = ActionInputs.get_github_repo_name()
query = ISSUES_FOR_PRS.format(
Expand All @@ -69,13 +69,24 @@ def get_issues_for_pr(pull_number: int) -> set[str]:
"Content-Type": "application/json",
}

response = requests.post(github_api_url, json={"query": query}, headers=headers, verify=False, timeout=10)
response.raise_for_status() # Raise an error for HTTP issues
data = response.json()
if data.get("errors"):
raise RuntimeError(f"GitHub GraphQL errors: {data['errors']}")
try:
headers, payload = requester.graphql_query(query, headers)
except GithubException as e:
# e.status (int), e.data (dict/str) often contains useful details
raise RuntimeError(f"GitHub HTTP error {getattr(e, 'status', '?')}: {getattr(e, 'data', e)}") from e

if not isinstance(payload, dict):
raise RuntimeError(f"Malformed response payload type: {type(payload)}")

# GraphQL-level errors come inside a successful HTTP 200
if "errors" in payload:
messages = "; ".join(err.get("message", str(err)) for err in payload["errors"])
raise RuntimeError(f"GitHub GraphQL errors: {messages}")

if "data" not in payload:
raise RuntimeError("Malformed GraphQL response: missing 'data'")

return {
f"{owner}/{name}#{node['number']}"
for node in data["data"]["repository"]["pullRequest"]["closingIssuesReferences"]["nodes"]
for node in payload["data"]["repository"]["pullRequest"]["closingIssuesReferences"]["nodes"]
}
3 changes: 2 additions & 1 deletion tests/unit/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from github.PullRequest import PullRequest
from github.Rate import Rate
from github.Repository import Repository
from github.Requester import Requester

from release_notes_generator.model.record.commit_record import CommitRecord
from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord
Expand Down Expand Up @@ -56,7 +57,7 @@ def wrapper(fn):
return wrapper


def mock_get_issues_for_pr(pull_number: int) -> set[int]:
def mock_get_issues_for_pr(pull_number: int, requester: Requester) -> set[int]:
# if pull_number == 150:
# return [451]
return set()
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/release_notes_generator/record/factory/test_default_record_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from github.Commit import Commit
from github.Issue import Issue
from github.PullRequest import PullRequest
from github.Requester import Requester

from release_notes_generator.model.record.commit_record import CommitRecord
from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord
Expand Down Expand Up @@ -337,7 +338,7 @@ def wrapper(fn):
return fn
return wrapper

def mock_get_issues_for_pr_no_issues(pull_number: int) -> list[str]:
def mock_get_issues_for_pr_no_issues(pull_number: int, requester: Requester) -> list[str]:
return []


Expand Down
129 changes: 56 additions & 73 deletions tests/unit/release_notes_generator/utils/test_pull_request_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,17 @@ def test_get_issues_for_pr_success(monkeypatch):
_patch_action_inputs(monkeypatch)
_patch_issues_template(monkeypatch)

captured = {}
class Resp:
def raise_for_status(self): pass
def json(self):
return {
class MockRequester:
def __init__(self):
self.called = False
self.last_query = None
self.last_headers = None
def graphql_query(self, query, headers):
self.called = True
self.last_query = query
self.last_headers = headers
# Return (headers, payload) as expected by the new implementation
return headers, {
"data": {
"repository": {
"pullRequest": {
Expand All @@ -113,34 +119,21 @@ def json(self):
}
}

def fake_post(url, json=None, headers=None, verify=None, timeout=None):
captured["url"] = url
captured["json"] = json
captured["headers"] = headers
captured["verify"] = verify
captured["timeout"] = timeout
return Resp()

monkeypatch.setattr(pru.requests, "post", fake_post)

result = pru.get_issues_for_pr(123)
requester = MockRequester()
result = pru.get_issues_for_pr(123, requester)
assert result == {'OWN/REPO#11', 'OWN/REPO#22'}
assert captured["url"] == "https://api.github.com/graphql"
# Query string correctly formatted
assert captured["json"]["query"] == "Q 123 OWN REPO 10"
# Headers include token
assert captured["headers"]["Authorization"] == "Bearer TOK"
assert captured["verify"] is False
assert captured["timeout"] == 10
assert requester.called
assert requester.last_query == "Q 123 OWN REPO 10"
assert requester.last_headers["Authorization"] == "Bearer TOK"
assert requester.last_headers["Content-Type"] == "application/json"

def test_get_issues_for_pr_empty_nodes(monkeypatch):
_patch_action_inputs(monkeypatch)
_patch_issues_template(monkeypatch)

class Resp:
def raise_for_status(self): pass
def json(self):
return {
class MockRequester:
def graphql_query(self, query, headers):
return headers, {
"data": {
"repository": {
"pullRequest": {
Expand All @@ -150,48 +143,49 @@ def json(self):
}
}

monkeypatch.setattr(pru.requests, "post", lambda *a, **k: Resp())
assert pru.get_issues_for_pr(5) == set()
requester = MockRequester()
assert pru.get_issues_for_pr(5, requester) == set()

def test_get_issues_for_pr_http_error(monkeypatch):
_patch_action_inputs(monkeypatch)
_patch_issues_template(monkeypatch)

class Resp:
def raise_for_status(self):
raise requests.HTTPError("Boom")
def json(self):
return {}
from github import GithubException
class MockGithubException(GithubException):
def __init__(self, status, data):
super().__init__(status, data)
# Do not set self.status or self.data directly

monkeypatch.setattr(pru.requests, "post", lambda *a, **k: Resp())
class MockRequester:
def graphql_query(self, query, headers):
raise MockGithubException(500, "Boom")

with pytest.raises(requests.HTTPError):
pru.get_issues_for_pr(77)
requester = MockRequester()
with pytest.raises(RuntimeError) as excinfo:
pru.get_issues_for_pr(77, requester)
assert "GitHub HTTP error 500" in str(excinfo.value)

def test_get_issues_for_pr_malformed_response(monkeypatch):
_patch_action_inputs(monkeypatch)
_patch_issues_template(monkeypatch)

class Resp:
def raise_for_status(self): pass
def json(self):
# Missing the expected nested keys -> triggers KeyError
return {"data": {}}

monkeypatch.setattr(pru.requests, "post", lambda *a, **k: Resp())
class MockRequester:
def graphql_query(self, query, headers):
return headers, {"data": {}} # Missing nested keys

requester = MockRequester()
with pytest.raises(KeyError):
pru.get_issues_for_pr(42)
pru.get_issues_for_pr(42, requester)

def test_get_issues_for_pr_caching(monkeypatch):
_patch_action_inputs(monkeypatch)
_patch_issues_template(monkeypatch)

calls = {"count": 0}
class Resp:
def raise_for_status(self): pass
def json(self):
return {
class MockRequester:
def graphql_query(self, query, headers):
calls["count"] += 1
return headers, {
"data": {
"repository": {
"pullRequest": {
Expand All @@ -201,14 +195,9 @@ def json(self):
}
}

def fake_post(*a, **k):
calls["count"] += 1
return Resp()

monkeypatch.setattr(pru.requests, "post", fake_post)

first = pru.get_issues_for_pr(900)
second = pru.get_issues_for_pr(900) # should use cache
requester = MockRequester()
first = pru.get_issues_for_pr(900, requester)
second = pru.get_issues_for_pr(900, requester) # should use cache
assert first == {'OWN/REPO#9'} and second == {'OWN/REPO#9'}
assert calls["count"] == 1 # only one network call

Expand All @@ -217,30 +206,24 @@ def test_get_issues_for_pr_different_numbers_not_cached(monkeypatch):
_patch_issues_template(monkeypatch)

calls = {"nums": []}
class Resp:
def __init__(self, n): self.n = n
def raise_for_status(self): pass
def json(self):
return {
class MockRequester:
def graphql_query(self, query, headers):
# Extract pull number back from formatted query tail
pull_num = int(query.split()[1])
calls["nums"].append(pull_num)
return headers, {
"data": {
"repository": {
"pullRequest": {
"closingIssuesReferences": {"nodes": [{"number": self.n}]}
"closingIssuesReferences": {"nodes": [{"number": pull_num}]}
}
}
}
}

def fake_post(url, json=None, **k):
# Extract pull number back from formatted query tail
pull_num = int(json["query"].split()[1])
calls["nums"].append(pull_num)
return Resp(pull_num)

monkeypatch.setattr(pru.requests, "post", fake_post)

r1 = pru.get_issues_for_pr(1)
r2 = pru.get_issues_for_pr(2)
requester = MockRequester()
r1 = pru.get_issues_for_pr(1, requester)
r2 = pru.get_issues_for_pr(2, requester)
assert r1 == {'OWN/REPO#1'}
assert r2 == {'OWN/REPO#2'}
assert calls["nums"] == [1, 2]
Loading