Skip to content

Fix more vulns #4930

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
lukaszgryglicki merged 4 commits into from
Mar 11, 2026
Merged

Fix more vulns #4930

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
5650844
Fix more vulns
lukaszgryglicki Mar 11, 2026
f6a8866
Address AI feedback
lukaszgryglicki Mar 11, 2026
1fbb4b1
Fix indent
lukaszgryglicki Mar 11, 2026
0f5ce5d
Address AI feedback
lukaszgryglicki Mar 11, 2026
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
59 changes: 30 additions & 29 deletions cla-backend/cla/models/github_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ def clear_caches():
cla.log.info(f"{fn} - cleared github_user_cache")
return {"status": "OK"}
except Exception as e:
cla.log.error(f"{fn} - error clearing caches: {e}")
return {"status": f"Error clearing caches: {e}"}
cla.log.error(f"{fn} - error clearing caches", exc_info=True)
return {"status": "Error clearing caches"}

@dataclass
class CommitLite:
Expand Down Expand Up @@ -213,11 +213,11 @@ def user_from_session(self, request, get_redirect_url):
fn = "github_models.user_from_session"
cla.log.debug(f"{fn} - loading session from request")
session = self._get_request_session(request)
cla.log.debug(f"{fn} - session loaded (keys={list(session.keys())})")
cla.log.debug(f"{fn} - session loaded")

# We can already have token in the session
if "github_oauth2_token" in session:
cla.log.debug(f"{fn} - using existing session GitHub OAuth2 token")
cla.log.debug(f"{fn} - using existing session GitHub OAuth2 authentication")
user = self.get_or_create_user(request)
if user is None:
cla.log.debug(f"{fn} - cannot find user, returning HTTP 404 status")
Expand All @@ -226,7 +226,7 @@ def user_from_session(self, request, get_redirect_url):
return user

authorization_url, csrf_token = self.get_authorization_url_and_state(None, None, None, ["user:email"], state='user-from-session')
cla.log.debug(f"{fn} - obtained GitHub OAuth2 state from authorization - storing CSRF token in the session...")
cla.log.debug(f"{fn} - obtained GitHub OAuth2 state from authorization - storing state in the session")
session["github_oauth2_state"] = csrf_token
cla.log.debug(f"{fn} - redirecting user to GitHub OAuth2 authorization URL")
# We must redirect to GitHub OAuth app for authentication, it will return you to /v2/github/installation which will handle returning user data
Expand All @@ -252,7 +252,7 @@ def sign_request(self, installation_id, github_repository_id, change_request_id,
# Not sure if we need a different token for each installation ID...
cla.log.debug(f"{fn} - Loading session from request")
session = self._get_request_session(request)
cla.log.debug(f"{fn} - Adding github details to session: {list(session.keys())} which is type: {type(session)}...")
cla.log.debug(f"{fn} - Adding github details to session")
session["github_installation_id"] = installation_id
session["github_repository_id"] = github_repository_id
session["github_change_request_id"] = change_request_id
Expand All @@ -264,14 +264,14 @@ def sign_request(self, installation_id, github_repository_id, change_request_id,
cla.log.debug(f'{fn} - stored origin url in session')

if "github_oauth2_token" in session:
cla.log.debug(f"{fn} - Using existing session GitHub OAuth2 token")
cla.log.debug(f"{fn} - Using existing session GitHub OAuth2 authentication")
return self.redirect_to_console(installation_id, github_repository_id, change_request_id, origin_url, request)
else:
cla.log.debug(f"{fn} - No existing GitHub OAuth2 token - building authorization url and state")
authorization_url, state = self.get_authorization_url_and_state(
installation_id, github_repository_id, int(change_request_id), ["user:email"]
)
cla.log.debug(f"{fn} - Obtained GitHub OAuth2 state from authorization - storing state in the session...")
cla.log.debug(f"{fn} - Obtained GitHub OAuth2 state from authorization - storing state in the session")
session["github_oauth2_state"] = state
cla.log.debug(f"{fn} - redirecting user to GitHub OAuth2 authorization URL")
raise falcon.HTTPFound(authorization_url)
Expand Down Expand Up @@ -307,7 +307,7 @@ def _get_request_session(self, request) -> dict: # pylint: disable=no-self-use
session = {}
request.context["session"] = session

cla.log.debug(f"{fn} - loaded session (keys={list(session.keys())})")
cla.log.debug(f"{fn} - loaded session")

return session

Expand Down Expand Up @@ -375,7 +375,7 @@ def oauth2_redirect(self, state, code, request): # pylint: disable=too-many-arg
padded_state = state + "=" * (-len(state) % 4)
state_data = json.loads(base64.urlsafe_b64decode(padded_state.encode()).decode())
except (ValueError, json.JSONDecodeError, binascii.Error) as err:
cla.log.warning(f"{fn} - failed to decode state, error: {err}")
cla.log.warning(f"{fn} - failed to decode state, error occurred")
raise falcon.HTTPBadRequest("Invalid OAuth2 state", "Invalid OAuth2 state")

state_token = state_data.get("csrf")
Expand All @@ -398,9 +398,9 @@ def oauth2_redirect(self, state, code, request): # pylint: disable=too-many-arg
try:
token = self._fetch_token(client_id, state, token_url, client_secret, code)
except Exception as err:
cla.log.warning(f"{fn} - GitHub OAuth2 error: {err}. Likely bad or expired code, returning HTTP 404 state.")
raise falcon.HTTPBadRequest("OAuth2 code is invalid or expired")
cla.log.debug(f"{fn} - oauth2 token received - storing token in session")
cla.log.warning(f"{fn} - GitHub OAuth2 error. Likely bad or expired code, returning HTTP 400 status.")
raise falcon.HTTPBadRequest("OAuth2 code is invalid or expired", "OAuth2 code is invalid or expired")
Comment thread
lukaszgryglicki marked this conversation as resolved.
cla.log.debug(f"{fn} - oauth2 authentication received - storing in session")
session["github_oauth2_token"] = token
user = self.get_or_create_user(request)
if user is None:
Expand All @@ -421,7 +421,7 @@ def oauth2_redirect(self, state, code, request): # pylint: disable=too-many-arg
client_secret = os.environ["GH_OAUTH_SECRET"]
cla.log.debug(f"{fn} - fetching oauth2 token from configured GitHub endpoint")
token = self._fetch_token(client_id, state, token_url, client_secret, code)
cla.log.debug(f"{fn} - oauth2 token received - storing token in session")
cla.log.debug(f"{fn} - oauth2 authentication received - storing in session")
session["github_oauth2_token"] = token
cla.log.debug(f"{fn} - redirecting the user back to the contributor console")
return self.redirect_to_console(installation_id, github_repository_id, change_request_id, origin_url, request)
Expand Down Expand Up @@ -1049,7 +1049,7 @@ def update_change_request(self, installation_id, github_repository_id, change_re
pull_request_id=str(change_request_id),
)
except Exception as e:
cla.log.error(f"{fn} - problem saving PR metadata for PR: {pull_request.number}, error: {e}")
cla.log.error(f"{fn} - problem saving PR metadata for PR: {pull_request.number}")

# Find users who have signed and who have not signed.
signed = []
Expand All @@ -1073,7 +1073,7 @@ def update_change_request(self, installation_id, github_repository_id, change_re
try:
future.result()
except Exception as e:
cla.log.error(f"{fn} - Exception in commit author thread for PR: {pull_request.number}, error: {e}")
cla.log.error(f"{fn} - Exception in commit author thread for PR: {pull_request.number}")

# Skip allowlisted bots per org/repo GitHub login/email regexps
missing, allowlisted = self.skip_allowlisted_bots(github_org, repository.get_repository_name(), missing)
Expand Down Expand Up @@ -1431,9 +1431,9 @@ def get_or_create_user(self, request):
# Could not get GitHub user data - maybe user revoked CLA app permissions?
session = self._get_request_session(request)

del session["github_oauth2_state"]
del session["github_oauth2_token"]
cla.log.warning(f"{fn} - Deleted OAuth2 session data - retrying token exchange next time")
session.pop("github_oauth2_state", None)
session.pop("github_oauth2_token", None)
cla.log.warning(f"{fn} - Deleted OAuth2 session data - retrying authentication exchange next time")
raise falcon.HTTPError(
"400 Bad Request", "github_oauth2_token", "Token permissions have been rejected, please try again"
)
Expand Down Expand Up @@ -1520,16 +1520,16 @@ def get_user_data(self, session, client_id): # pylint: disable=no-self-use
fn = "cla.models.github_models.get_user_data"
token = session.get("github_oauth2_token")
if token is None:
cla.log.error(f"{fn} - unable to load github_oauth2_token from session (keys={list(session.keys())})")
cla.log.error(f"{fn} - unable to load github_oauth2_token from session")
return {"error": "could not get user data from session"}

oauth2 = OAuth2Session(client_id, token=token)
request = oauth2.get("https://api.github.com/user")
github_user = request.json()
cla.log.debug(f"{fn} - GitHub user data: %s", github_user)
if "message" in github_user:
cla.log.error(f'{fn} - Could not get user data with OAuth2 token: {github_user["message"]}')
return {"error": "Could not get user data: %s" % github_user["message"]}
cla.log.error(f'{fn} - Could not get user data with OAuth2 authentication')
return {"error": "Could not get user data"}
return github_user

def get_user_emails(self, session: dict, client_id: str) -> Union[List[str], dict]: # pylint: disable=no-self-use
Expand Down Expand Up @@ -1589,13 +1589,14 @@ def _fetch_github_emails(self, session: dict, client_id: str) -> Union[List[dict
# as expected
token = session.get("github_oauth2_token")
if token is None:
cla.log.warning(f"{fn} - unable to load github_oauth2_token from the session - session is empty")
cla.log.warning(f"{fn} - unable to load authentication token from the session - session is empty")
return {"error": "Could not get user emails"}
oauth2 = OAuth2Session(client_id, token=token)
request = oauth2.get("https://api.github.com/user/emails")
resp = request.json()
if "message" in resp:
cla.log.warning(f'{fn} - could not get user emails with OAuth2 token: {resp["message"]}')
return {"error": "Could not get user emails: %s" % resp["message"]}
cla.log.warning(f'{fn} - could not get user emails with OAuth2 authentication')
return {"error": "Could not get user emails"}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return resp

def process_reopened_pull_request(self, data):
Expand Down Expand Up @@ -2187,7 +2188,7 @@ def pygithub_graphql(g, query: str, variables: dict | None = None):
errs = data["errors"]
paths = [e.get("path") for e in errs]
msgs = [e.get("message") for e in errs]
cla.log.error(f"GraphQL errors: {msgs} (paths={paths}, all={errs!r})")
cla.log.error(f"GraphQL errors occurred")
return None
return data.get("data")
except Exception as exc:
Expand Down Expand Up @@ -2535,7 +2536,7 @@ def get_co_author_commits(co_author, commit_sha, pr, installation_id) -> Tuple[U
cla.log.debug(f"{fn} - Detected noreply GitHub email with ID: {id_str}, login: {login_str}")
user = github.get_github_user_by_id(github_id, installation_id)
except Exception as ex:
cla.log.warning(f"{fn} - Error fetching user by ID {id_str}: {ex}")
cla.log.warning(f"{fn} - Error fetching user by ID {id_str}")
user = None

# 2. Check for "username@users.noreply.github.com"
Expand All @@ -2547,7 +2548,7 @@ def get_co_author_commits(co_author, commit_sha, pr, installation_id) -> Tuple[U
cla.log.debug(f"{fn} - Detected noreply GitHub email with login: {login_str}")
user = github.get_github_user_by_login(login_str, installation_id)
except Exception as ex:
cla.log.warning(f"{fn} - Error fetching user by login {login_str}: {ex}")
cla.log.warning(f"{fn} - Error fetching user by login {login_str}")
user = None

# 3. Try to find user by email via GitHub APIs
Expand Down Expand Up @@ -2584,7 +2585,7 @@ def get_co_author_commits(co_author, commit_sha, pr, installation_id) -> Tuple[U
try:
user = github.get_github_user_by_id(github_id, installation_id)
except Exception as ex:
cla.log.warning(f"{fn} - Error fetching user by ID {github_id}: {ex}")
cla.log.warning(f"{fn} - Error fetching user by ID {github_id}")
user = None
except Exception as ex:
# user not found
Expand Down
39 changes: 24 additions & 15 deletions cla-backend/cla/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1277,13 +1277,7 @@ def fetch_token(client_id, state, token_url, client_secret, code, redirect_uri=N
oauth2 = OAuth2Session(client_id, state=state, scope=["user:email"], redirect_uri=redirect_uri)
else:
oauth2 = OAuth2Session(client_id, state=state, scope=["user:email"])
#cla.log.debug(
# f"{fn} - oauth2.fetch_token - "
# f"token_url: {token_url}, "
# f"client_id: {client_id}, "
# f"client_secret: {client_secret}, "
# f"code: {code}"
#)
cla.log.debug(f"{fn} - oauth2.fetch_token called")
return oauth2.fetch_token(token_url, client_secret=client_secret, code=code)


Expand Down Expand Up @@ -1683,7 +1677,7 @@ def lookup_user_github_username(user_github_id: int) -> Optional[str]:

github_user = r.json()
if "message" in github_user:
cla.log.warning(f"Unable to lookup user from id: {user_github_id} " f'- message: {github_user["message"]}')
cla.log.warning(f"Unable to lookup user from id: {user_github_id} - API error occurred")
return None
else:
if "login" in github_user:
Expand Down Expand Up @@ -1716,7 +1710,7 @@ def lookup_user_github_id(user_github_username: str) -> Optional[int]:

github_user = r.json()
if "message" in github_user:
cla.log.warning(f"Unable to lookup user from id: {user_github_username} " f'- message: {github_user["message"]}')
cla.log.warning(f"Unable to lookup user from id: {user_github_username} - API error occurred")
return None
else:
if "id" in github_user:
Expand Down Expand Up @@ -1748,8 +1742,14 @@ def lookup_gitlab_org_members(organization_id):
r = requests.get(f"{cla.config.PLATFORM_GATEWAY_URL}/cla-service/v4/gitlab/group/{organization_id}/members")
r.raise_for_status()
except requests.exceptions.HTTPError as err:
cla.log.warning(f"Could not fetch gitlab org users: {err}")
return {f"error: Could not get user gitlab group id: {organization_id} members: {err}"}
status_code = err.response.status_code if hasattr(err, 'response') and err.response is not None else "unknown"
cla.log.warning(
f"Could not fetch gitlab org users for organization_id={organization_id}: "
f"status_code={status_code}"
)
# Return an empty list so callers that expect an iterable of member dicts
# can safely handle the error case without type errors.
return []
Comment thread
lukaszgryglicki marked this conversation as resolved.
return r.json()["list"]


Expand Down Expand Up @@ -2029,7 +2029,16 @@ def extract_pull_request_number(pull_request_message):
fn = "extract_pull_request_number"
pull_request_number = None
try:
first_line = pull_request_message.splitlines()[0]
if not pull_request_message or not pull_request_message.strip():
cla.log.debug(f"{fn} - empty or whitespace-only message")
return None

lines = pull_request_message.splitlines()
if not lines or not lines[0].strip():
cla.log.debug(f"{fn} - no valid lines in message")
return None

first_line = lines[0]
cla.log.debug(f"{fn} - checking line '{first_line}")
# Case 1: "Merge pull request #N"
matches = re.match(r'^Merge pull request #(\d+)', first_line)
Expand All @@ -2056,7 +2065,7 @@ def extract_pull_request_number(pull_request_message):
cla.log.debug(f"{fn} - extracted PR number {pull_request_number} from merge_queue data: {pull_request_message} by matching first '#N'")
return pull_request_number
else:
cla.log.warning(f"{fn} - error - unable to extract pull request number from message: {pull_request_message}")
except Exception as e:
cla.log.warning(f"{fn} - error - unable to extract pull request number from message: {pull_request_message}, error: {e}")
cla.log.warning(f"{fn} - error - unable to extract pull request number from message")
except (ValueError, AttributeError, IndexError):
cla.log.warning(f"{fn} - error - unable to extract pull request number from message, parse error occurred")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return pull_request_number
Loading
Loading