Merge pull request #4930 from linuxfoundation/unicron-address-codeql-and-dependabot-vulns-2

Fix more vulns

Author: lukaszgryglicki

cla-backend/cla/models/github_models.py

@@ -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:
@@ -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")
@@ -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
@@ -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
@@ -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)
@@ -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
 
@@ -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")
@@ -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")
+            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:
@@ -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)
@@ -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 = []
@@ -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)
@@ -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"
             )
@@ -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
@@ -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"}
         return resp
 
     def process_reopened_pull_request(self, data):
@@ -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:
@@ -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"
@@ -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
@@ -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

cla-backend/cla/utils.py

@@ -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)
 
 
@@ -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:
@@ -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:
@@ -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 []
     return r.json()["list"]
 
 
@@ -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)
@@ -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")
     return pull_request_number