Move GitHub access token from query parameter into header#1261
Move GitHub access token from query parameter into header#1261mhucka merged 10 commits intoquantumlib:mainfrom
Conversation
GitHub deprecated passing the access token in the URL. This change moves it to the Authorization header, which is more secure and compliant with GitHub's current API standards.
There was a problem hiding this comment.
Code Review
This pull request improves security by refactoring the report_status_to_github method to pass the GitHub access token via the Authorization header instead of a URL query parameter. A new security test has been added to verify this implementation. Feedback suggests adding a check for None values in actual_commit_id to prevent malformed API requests and implementing a timeout for the HTTP POST request to avoid potential hanging in CI/CD pipelines.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances the security and robustness of the GitHub status reporting mechanism by moving the access token from URL query parameters to the Authorization header, adding a request timeout, and implementing a guard against null commit IDs. It also introduces a new security test suite to verify these changes. The review feedback suggests transitioning from the legacy token scheme to the Bearer scheme in the Authorization header to align with modern GitHub API standards and ensure compatibility with fine-grained personal access tokens.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request improves security and robustness in the report_status_to_github function by moving the GitHub access token from URL query parameters to an Authorization header. It also adds a 30-second timeout to the POST request and an early return if the commit ID is missing. A new test file, dev_tools/prepared_env_security_test.py, has been introduced to verify that the token is correctly handled. I have no feedback to provide.
GitHub deprecated passing the access token in the URL. This change moves it to the Authorization header, which is more secure and compliant with GitHub's current API standards. It also adds a test file.