Open
Conversation
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces python-jose with PyJWT for JWT handling, redacts sensitive logging across CLA modules, adds unit tests for JWT auth, and updates multiple Python and Node.js dependency pins and package resolution overrides. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates dependency pins/lockfiles across the backend and test harnesses to address reported vulnerabilities, and reduces sensitive information exposure in logs while migrating JWT handling from python-jose to PyJWT in the backend.
Changes:
- Bump/pin a set of Node (Yarn/NPM) and Python dependencies to patched versions (and regenerate relevant lockfiles).
- Replace
python-joseJWT usage withPyJWTfor token parsing/validation in the backend. - Remove or reduce logging of secrets/tokens and session contents in several backend codepaths.
Reviewed changes
Copilot reviewed 11 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rest/yarn.lock | Updates locked transitive Node dependencies for REST tests. |
| tests/rest/requirements.freeze.txt | Bumps pinned Python deps used by REST tests (requests/urllib3/idna/etc.). |
| tests/rest/package.json | Adds Yarn resolutions pins for vulnerable transitive packages. |
| tests/functional/yarn.lock | Updates Yarn lockfile entries for functional tests (partial regen/normalization). |
| tests/functional/package.json | Updates AJV and adds npm overrides for vulnerable transitive packages. |
| tests/functional/package-lock.json | Updates npm lockfile to reflect new overrides/version bumps. |
| cla-backend/yarn.lock | Updates backend Node lockfile (ajv/minimatch/qs/tmp/etc.). |
| cla-backend/requirements.txt | Updates pinned Python deps (boto3/botocore/PyJWT/urllib3/werkzeug/etc.) and removes python-jose-related pins. |
| cla-backend/package.json | Adjusts backend Node dependency constraints and adds extra resolutions pins. |
| cla-backend/cla/utils.py | Reduces debug logging content for OAuth authorization URL/state generation. |
| cla-backend/cla/user_service.py | Removes access-token prefix from debug logging. |
| cla-backend/cla/user.py | Switches bearer-token claim parsing from python-jose to PyJWT. |
| cla-backend/cla/models/github_models.py | Reduces logging of OAuth state/tokens and session contents; tightens bad-state error responses. |
| cla-backend/cla/controllers/github_application.py | Switches JWT import usage and removes private-key logging from initialization. |
| cla-backend/cla/auth.py | Switches JWT verification to PyJWT using RSAAlgorithm.from_jwk and updates exception handling. |
Files not reviewed (1)
- tests/functional/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
tests/functional/package-lock.json:356
- The
node_modules/@cypress/requestentry is missing anintegrityfield after the version bump. To keep installs reproducible and tamper-evident, regeneratepackage-lock.jsonwithnpm installinstead of editing lock entries manually.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
cla-backend/cla/controllers/github_application.py (1)
79-85:⚠️ Potential issue | 🟡 MinorUpdate stale JWT library docstring
The docstring still says this class/method uses
python-jose, but implementation now usesjwtimport for PyJWT. Please align docs to avoid maintenance confusion.📝 Proposed fix
class GithubCLAIntegration(GithubIntegration): """ - Custom GithubIntegration using python-jose instead of pyjwt for token creation. + Custom GithubIntegration using PyJWT for token creation. """ def create_jwt(self): """ - Overloaded to use python-jose instead of pyjwt. - Couldn't get it working with pyjwt. + Overloaded to create the GitHub App JWT using PyJWT. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cla-backend/cla/controllers/github_application.py` around lines 79 - 85, The docstring for the custom GithubIntegration/create_jwt still says it uses python-jose but the implementation imports and uses jwt (PyJWT); update the class and method docstring to reflect that it uses PyJWT (the jwt import) instead of python-jose so docs match the actual implementation (look for the class comment starting "Custom GithubIntegration" and the create_jwt method docstring).cla-backend/cla/user_service.py (1)
209-239:⚠️ Potential issue | 🟠 MajorRemove token fragment logging on token refresh
Line 238 still logs the first 10 characters of the bearer token. Partial token values should not be written to logs.
🔐 Proposed fix
- log.debug(f'{fn} - successfully obtained access_token: {self.access_token[0:10]}...') + log.debug(f'{fn} - successfully obtained access token')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cla-backend/cla/user_service.py` around lines 209 - 239, The code in the access token refresh path (inside the method that uses fn, self.access_token and self.access_token_expires) logs a token fragment via log.debug(f'{fn} - successfully obtained access_token: {self.access_token[0:10]}...'); remove the token fragment from logs and replace it with a non-sensitive success message (e.g. log.debug(f'{fn} - successfully obtained access token') or similar) so that self.access_token or any part of it is never written to logs; update the log statement in the block that handles the requests.post response and ensure no other debug statements output self.access_token or json_data["access_token"].cla-backend/cla/models/github_models.py (1)
247-270:⚠️ Potential issue | 🟠 MajorSensitive session contents are still logged in OAuth flow.
Line 249 logs the full
sessionobject, which can includegithub_oauth2_token. This undermines the redaction hardening introduced in nearby changes (e.g., Line 270).Suggested fix
- cla.log.debug(f"{fn} - Adding github details to session: {session} which is type: {type(session)}...") + session_keys = list(session.keys()) if isinstance(session, dict) else [] + cla.log.debug(f"{fn} - adding github details to session (keys={session_keys}, type={type(session)})")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cla-backend/cla/models/github_models.py` around lines 247 - 270, The code is logging the full session object (session) which can contain sensitive tokens; remove or change the log in the block that currently does cla.log.debug(f"{fn} - Adding github details to session: {session} ...) so it does NOT print session contents — instead log safe metadata only (e.g., type(session) or non-sensitive keys) or explicitly redact/mask sensitive keys like "github_oauth2_token" and "github_oauth2_state"; update any related debug lines that divulge session internals (the statements around session["github_installation_id"], session["github_repository_id"], session["github_change_request_id"], and before calling redirect_to_console / get_authorization_url_and_state) to use the redacted/masked form.cla-backend/package.json (1)
52-77:⚠️ Potential issue | 🟠 Major
resolutionsoverride vulnerability bumps, preventing security upgrades.The resolutions section downgrades packages to older versions than declared in dependencies:
simple-git: dependency specifies^3.32.3but resolution forces^3.16.0qs: dependency specifies^6.14.2but resolution forces6.14.1Update resolutions to match the dependency versions:
Suggested fix
- "qs": "6.14.1", + "qs": "^6.14.2", ... - "simple-git": "^3.16.0", + "simple-git": "^3.32.3",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cla-backend/package.json` around lines 52 - 77, The resolutions block in package.json is forcing older package versions that conflict with dependencies; update the "resolutions" entries so "simple-git" matches the dependency "^3.32.3" and "qs" matches the dependency version (change "qs": "6.14.1" to the same semver used in dependencies, e.g. "^6.14.2"), ensuring the "resolutions" section reflects the declared dependency versions rather than downgrading them; modify the "resolutions" object accordingly and run install to verify no conflicts.
🧹 Nitpick comments (1)
cla-backend/cla/user.py (1)
36-51: Remove unusedcla_userdirectiveThe
cla_userdirective (lines 20–56) disables all JWT verification checks and is dead code—it is never imported or used as a parameter in any endpoint or test. The application correctly usescheck_auth(defined at routes.py:592), which properly validates JWT signatures against Auth0 JWKS and checks token claims. Removecla_userand the associatedCLAUserclass to avoid confusion and prevent accidental future use of unverified JWT handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cla-backend/cla/user.py` around lines 36 - 51, Remove the dead/unsafe cla_user directive and its CLAUser class: locate the CLAUser class and any decorator or factory named cla_user (which currently decodes JWTs with jwt.decode(..., options={...}) and logs errors on parsing via cla.log.error) and delete the entire implementation and any references to it; ensure no imports or registrations of cla_user remain so the app relies solely on the existing check_auth flow (routes.py: check_auth) for JWT validation; run tests to confirm no callers reference CLAUser/cla_user and remove any unused imports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cla-backend/cla/utils.py`:
- Around line 1243-1246: The oauth initialization log currently exposes
sensitive integration metadata by printing client_id, redirect_uri and scope;
update the cla.log.debug calls (the one using f"{fn} - initialized oauth
session..." and the similar call at lines referenced) to remove those values and
instead emit a generic message such as "{fn} - initialized oauth session" or
"{fn} - initialized oauth session for GitHub" using the existing fn identifier;
ensure client_id, redirect_uri and scope variables are not interpolated into
logs anywhere in the oauth session initialization code paths.
In `@cla-backend/requirements.txt`:
- Around line 48-50: The requirements file pins markupsafe to 2.0.1 which
conflicts with Werkzeug 3.1.6; update the markupsafe entry in requirements.txt
(replace "markupsafe==2.0.1") to a compatible spec such as "markupsafe>=2.1.1"
(or a specific 2.1.x pin) so it satisfies Werkzeug's requirement, then
re-resolve/install dependencies (pip-compile/pip install -r requirements.txt)
and run the test suite to ensure no regressions.
In `@tests/functional/package.json`:
- Around line 47-50: The overrides entries using the `parent>child` string
format are invalid and ignored; replace them with nested object overrides so npm
applies them correctly: create an "overrides" object mapping "mochawesome-merge"
and "rimraf" to objects that set "minimatch":"3.1.5" and nest
"brace-expansion":"1.1.12" under "minimatch" for mochawesome-merge, or
alternatively flatten to top-level overrides for "minimatch" and
"brace-expansion" if you want the fixes applied globally.
---
Outside diff comments:
In `@cla-backend/cla/controllers/github_application.py`:
- Around line 79-85: The docstring for the custom GithubIntegration/create_jwt
still says it uses python-jose but the implementation imports and uses jwt
(PyJWT); update the class and method docstring to reflect that it uses PyJWT
(the jwt import) instead of python-jose so docs match the actual implementation
(look for the class comment starting "Custom GithubIntegration" and the
create_jwt method docstring).
In `@cla-backend/cla/models/github_models.py`:
- Around line 247-270: The code is logging the full session object (session)
which can contain sensitive tokens; remove or change the log in the block that
currently does cla.log.debug(f"{fn} - Adding github details to session:
{session} ...) so it does NOT print session contents — instead log safe metadata
only (e.g., type(session) or non-sensitive keys) or explicitly redact/mask
sensitive keys like "github_oauth2_token" and "github_oauth2_state"; update any
related debug lines that divulge session internals (the statements around
session["github_installation_id"], session["github_repository_id"],
session["github_change_request_id"], and before calling redirect_to_console /
get_authorization_url_and_state) to use the redacted/masked form.
In `@cla-backend/cla/user_service.py`:
- Around line 209-239: The code in the access token refresh path (inside the
method that uses fn, self.access_token and self.access_token_expires) logs a
token fragment via log.debug(f'{fn} - successfully obtained access_token:
{self.access_token[0:10]}...'); remove the token fragment from logs and replace
it with a non-sensitive success message (e.g. log.debug(f'{fn} - successfully
obtained access token') or similar) so that self.access_token or any part of it
is never written to logs; update the log statement in the block that handles the
requests.post response and ensure no other debug statements output
self.access_token or json_data["access_token"].
In `@cla-backend/package.json`:
- Around line 52-77: The resolutions block in package.json is forcing older
package versions that conflict with dependencies; update the "resolutions"
entries so "simple-git" matches the dependency "^3.32.3" and "qs" matches the
dependency version (change "qs": "6.14.1" to the same semver used in
dependencies, e.g. "^6.14.2"), ensuring the "resolutions" section reflects the
declared dependency versions rather than downgrading them; modify the
"resolutions" object accordingly and run install to verify no conflicts.
---
Nitpick comments:
In `@cla-backend/cla/user.py`:
- Around line 36-51: Remove the dead/unsafe cla_user directive and its CLAUser
class: locate the CLAUser class and any decorator or factory named cla_user
(which currently decodes JWTs with jwt.decode(..., options={...}) and logs
errors on parsing via cla.log.error) and delete the entire implementation and
any references to it; ensure no imports or registrations of cla_user remain so
the app relies solely on the existing check_auth flow (routes.py: check_auth)
for JWT validation; run tests to confirm no callers reference CLAUser/cla_user
and remove any unused imports.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (4)
cla-backend/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/functional/package-lock.jsonis excluded by!**/package-lock.jsontests/functional/yarn.lockis excluded by!**/yarn.lock,!**/*.locktests/rest/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
cla-backend/cla/auth.pycla-backend/cla/controllers/github_application.pycla-backend/cla/models/github_models.pycla-backend/cla/user.pycla-backend/cla/user_service.pycla-backend/cla/utils.pycla-backend/package.jsoncla-backend/requirements.txttests/functional/package.jsontests/rest/package.jsontests/rest/requirements.freeze.txt
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cla-backend/requirements.txt`:
- Around line 6-7: The requirements.txt pins packages (boto3==1.42.59,
botocore==1.42.59, Werkzeug==3.1.6, urllib3==2.6.3, MarkupSafe==2.1.5) that
require Python ≥3.9 while the repo declares Python 3.6–3.7; either (A) downgrade
those package pins in requirements.txt to versions compatible with Python
3.6/3.7 (choose the latest supported releases for boto3, botocore, Werkzeug,
urllib3, MarkupSafe) and run tests/CI, or (B) update the repository Python
constraints (Pipfile and Dockerfile) to ≥3.9 and adjust any Docker/base images
and CI matrices accordingly; pick one approach, update the respective files
consistently, and ensure CI/installation succeeds.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
tests/functional/package-lock.jsonis excluded by!**/package-lock.jsontests/functional/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
cla-backend/requirements.txttests/functional/package.json
Member
Author
|
@lukaszgryglicki I've opened a new pull request, #4920, to work on those changes. Once the pull request is ready, I'll request review from you. |
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cla-backend/cla/models/github_models.py (1)
286-291:⚠️ Potential issue | 🟠 MajorSession values are still fully logged after string-to-dict conversion.
This log can still expose sensitive fields (e.g., OAuth token) and undercuts the vuln-redaction objective.
💡 Proposed fix
if isinstance(session, str): # convert string to a dict cla.log.debug(f"{fn} - session is type: {type(session)} - converting to dict...") session = json.loads(session) # Reset the session now that we have converted it to a dict request.context["session"] = session - cla.log.debug(f"{fn} - session: {session} which is now type: {type(session)}...") + cla.log.debug(f"{fn} - session converted to dict; keys={list(session.keys())}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cla-backend/cla/models/github_models.py` around lines 286 - 291, The current debug logs in the block that converts the session string to a dict (around variables fn, session and request.context["session"]) still print full session contents and may expose sensitive fields (e.g., OAuth tokens); after json.loads(session) and before assigning back to request.context["session"], sanitize the session dict by removing or masking known sensitive keys (e.g., "access_token", "oauth_token", "token", "refresh_token", "password", "client_secret") and only then log the sanitized session (or log keys/counts rather than values); ensure the sanitized object replaces the original in request.context["session"] so no sensitive values are written to logs.cla-backend/package.json (1)
7-7:⚠️ Potential issue | 🔴 CriticalUpdate Node.js engine requirement or downgrade incompatible dependencies.
The declared engine requirement
"node": ">=16"is incompatible with two pinned dependencies:
- minimatch@10.2.3 requires Node
18 || 20 || >=22- serialize-javascript@7.0.3 requires Node
>=20.0.0Either raise the engine requirement to
"node": ">=20"or pin these packages to Node 16-compatible versions. The remaining dependencies (qs, fast-xml-parser, ajv, validator, tmp) are all compatible with Node >=16.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cla-backend/package.json` at line 7, The package.json currently declares "node": ">=16" but two pinned deps (minimatch@10.2.3 and serialize-javascript@7.0.3) require Node >=18/20; update package.json to either raise the engines.node field to ">=20" or pin those deps to Node-16-compatible versions (e.g., downgrade minimatch to a 9.x release and serialize-javascript to a 6.x release), then regenerate the lockfile (npm/yarn) and run CI to ensure no other dependency breaks; locate and change the engines node field in package.json and the versions for minimatch and serialize-javascript in package.json/package-lock.json (or yarn.lock) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cla-backend/cla/models/github_models.py`:
- Around line 279-282: The debug line calls list(session.keys()) even when
session can be None, causing a crash; update the block around session/request
handling (the code that logs within the function referenced by fn in
github_models.py) to only call session.keys() when session is not None – e.g.,
after the existing "if session is None: cla.log.warning(...)" either return/exit
that branch or change the debug log to use a safe value (like an empty list or
"None") instead of list(session.keys()); ensure the change references the same
variables request, session and fn so logging remains informative.
In `@cla-backend/cla/user.py`:
- Around line 13-14: The jwt.decode call currently disables all validations
allowing forged tokens to set token_params; replace this by restoring signature
and standard claim checks: call jwt.decode with the configured verification
key/secret and explicit algorithms, remove the options that set
verify_signature/verify_exp/etc. to False, and validate required claims (issuer,
audience, subject) before using token_params to build the user principal; keep
the PyJWTError except block but ensure it only handles decode/validation
failures and rejects tokens rather than trusting their claims. Reference: the
jwt.decode call, token_params variable, and the PyJWTError exception handling in
user.py.
---
Outside diff comments:
In `@cla-backend/cla/models/github_models.py`:
- Around line 286-291: The current debug logs in the block that converts the
session string to a dict (around variables fn, session and
request.context["session"]) still print full session contents and may expose
sensitive fields (e.g., OAuth tokens); after json.loads(session) and before
assigning back to request.context["session"], sanitize the session dict by
removing or masking known sensitive keys (e.g., "access_token", "oauth_token",
"token", "refresh_token", "password", "client_secret") and only then log the
sanitized session (or log keys/counts rather than values); ensure the sanitized
object replaces the original in request.context["session"] so no sensitive
values are written to logs.
In `@cla-backend/package.json`:
- Line 7: The package.json currently declares "node": ">=16" but two pinned deps
(minimatch@10.2.3 and serialize-javascript@7.0.3) require Node >=18/20; update
package.json to either raise the engines.node field to ">=20" or pin those deps
to Node-16-compatible versions (e.g., downgrade minimatch to a 9.x release and
serialize-javascript to a 6.x release), then regenerate the lockfile (npm/yarn)
and run CI to ensure no other dependency breaks; locate and change the engines
node field in package.json and the versions for minimatch and
serialize-javascript in package.json/package-lock.json (or yarn.lock)
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c377c791-ebfe-4bda-9d8c-67effaf01819
⛔ Files ignored due to path filters (1)
cla-backend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
cla-backend-go/swagger/.python-versioncla-backend/cla/controllers/github_application.pycla-backend/cla/models/github_models.pycla-backend/cla/user.pycla-backend/package.json
✅ Files skipped from review due to trivial changes (1)
- cla-backend-go/swagger/.python-version
🚧 Files skipped from review as they are similar to previous changes (1)
- cla-backend/cla/controllers/github_application.py
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cla-backend/cla/models/github_models.py (1)
371-391: Minor: Remove extraneous f-string prefixes and consider exception chaining.Lines 381 and 382 use f-strings without any placeholders:
raise falcon.HTTPBadRequest(f"Invalid OAuth2 state", "Invalid OAuth2 state") # Line 381 cla.log.debug(f"handling user-from-session callback") # Line 382Additionally, the exceptions at lines 373 and 391 could benefit from exception chaining to preserve the original error context for debugging:
♻️ Suggested improvements
except (ValueError, json.JSONDecodeError, binascii.Error) as err: cla.log.warning(f"{fn} - failed to decode state, error: {err}") - raise falcon.HTTPBadRequest("Invalid OAuth2 state", "Invalid OAuth2 state") + raise falcon.HTTPBadRequest("Invalid OAuth2 state", "Invalid OAuth2 state") from err state_token = state_data["csrf"] value = state_data["state"] if value != "user-from-session": cla.log.warning(f"{fn} - invalid GitHub OAuth2 state while handling callback") raise falcon.HTTPBadRequest("Invalid OAuth2 state", "Invalid OAuth2 state") if state_token != session_state: cla.log.warning(f"{fn} - invalid GitHub OAuth2 state while handling callback") - raise falcon.HTTPBadRequest(f"Invalid OAuth2 state", "Invalid OAuth2 state") - cla.log.debug(f"handling user-from-session callback") + raise falcon.HTTPBadRequest("Invalid OAuth2 state", "Invalid OAuth2 state") + cla.log.debug("handling user-from-session callback")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") + raise falcon.HTTPBadRequest("OAuth2 code is invalid or expired") from err🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cla-backend/cla/models/github_models.py` around lines 371 - 391, The code uses unnecessary f-string prefixes and doesn't chain exceptions; remove the f-prefix on literal-only strings (e.g., the raise of falcon.HTTPBadRequest("Invalid OAuth2 state", "Invalid OAuth2 state") and cla.log.debug("handling user-from-session callback") ) and add exception chaining so the original errors are preserved (use "raise falcon.HTTPBadRequest(... ) from err" in both the except (ValueError, json.JSONDecodeError, binascii.Error) as err block that parses state_data and in the except Exception as err around the call to self._fetch_token(...)). Ensure you keep the same messages and reference fn/state/session variables as currently used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cla-backend/cla/models/github_models.py`:
- Around line 371-391: The code uses unnecessary f-string prefixes and doesn't
chain exceptions; remove the f-prefix on literal-only strings (e.g., the raise
of falcon.HTTPBadRequest("Invalid OAuth2 state", "Invalid OAuth2 state") and
cla.log.debug("handling user-from-session callback") ) and add exception
chaining so the original errors are preserved (use "raise
falcon.HTTPBadRequest(... ) from err" in both the except (ValueError,
json.JSONDecodeError, binascii.Error) as err block that parses state_data and in
the except Exception as err around the call to self._fetch_token(...)). Ensure
you keep the same messages and reference fn/state/session variables as currently
used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 590fcd5d-182d-4bed-bd8f-ed65c432d10e
📒 Files selected for processing (15)
cla-backend/cla/models/github_models.pycla-backend/cla/tests/unit/test_jwt_auth.pytests/functional/cypress/e2e/v3/organization.cy.tstests/functional/cypress/e2e/v4/company.cy.tstests/functional/cypress/e2e/v4/events.cy.tstests/functional/cypress/e2e/v4/foundation.cy.tstests/functional/cypress/e2e/v4/github-organizations.cy.tstests/functional/cypress/e2e/v4/github-repositories.cy.tstests/functional/cypress/e2e/v4/gitlab-organizations.cy.tstests/functional/cypress/e2e/v4/gitlab-repositories.cy.tstests/functional/cypress/e2e/v4/health.cy.tstests/functional/cypress/e2e/v4/metrics.cy.tstests/functional/cypress/e2e/v4/projects.cy.tstests/functional/cypress/e2e/v4/signatures.cy.tstests/functional/cypress/e2e/v4/version.cy.ts
✅ Files skipped from review due to trivial changes (8)
- tests/functional/cypress/e2e/v4/signatures.cy.ts
- tests/functional/cypress/e2e/v4/metrics.cy.ts
- tests/functional/cypress/e2e/v4/health.cy.ts
- tests/functional/cypress/e2e/v4/foundation.cy.ts
- tests/functional/cypress/e2e/v4/gitlab-organizations.cy.ts
- tests/functional/cypress/e2e/v4/gitlab-repositories.cy.ts
- tests/functional/cypress/e2e/v4/projects.cy.ts
- tests/functional/cypress/e2e/v4/company.cy.ts
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
mlehotskylf
approved these changes
Mar 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixing vulns - very
WIP.cc @mlehotskylf
Signed-off-by: Lukasz Gryglicki lgryglicki@cncf.io
Assisted by OpenAI
Assisted by GitHub Copilot