Potential fix for code scanning alert no. 13: Information exposure through an exception#9
Draft
venkateshpabbati wants to merge 1 commit intomainfrom
Draft
Potential fix for code scanning alert no. 13: Information exposure through an exception#9venkateshpabbati wants to merge 1 commit intomainfrom
venkateshpabbati wants to merge 1 commit intomainfrom
Conversation
…rough an exception Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
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.
Potential fix for https://github.com/venkateshpabbati/adk-python/security/code-scanning/13
General approach: Do not include raw exception objects or low-level database error text in HTTP responses. Instead, log the full exception on the server and return a generic, user-friendly message. For cases where you still want to differentiate “no record found” vs “unexpected error,” keep explicit, controlled messages (like the current “No booking found…” string) and only sanitize or generalize messages that come from exceptions.
Concrete fix here:
In
hotelbooker_core.py, update theexcept sqlite3.Error as e:block inget_booking_detailsso that:loggingmodule withlogging.exception(...)(which records stack trace)."An internal error occurred while retrieving booking details."instead of f-string including{e}.In
main.py, for consistency and to fix all variants in one go, avoid returningerror_messagefrom internal methods directly to clients when it might contain exception text. Instead:error_messageequals known safe user messages (like the “no booking found…” or “Please provide either a booking ID or a guest name…” already crafted insideHotelBooker), you can return it.However, with the change in
HotelBookerto return an already-generic message, reusingerror_messageinmain.pybecomes safe, because it never contains the raw exception; it’s controlled by our code. So we only need to sanitize in the core layer.Specific changes:
File
contributing/samples/authn-adk-all-in-one/hotel_booker_app/hotelbooker_core.pyget_available_hotels, errors are also usingf"...: {e}". For completeness and to share behavior, it’s better to adjust this too, but CodeQL’s reported path concernsget_booking_details. To keep changes minimal, we will:f"Error getting booking details: {e}"with a generic fixed-string message.loggingis configured somewhere in the app; since we already importloggingat the top ofhotelbooker_core.py, we can safely calllogging.exceptionwithout further imports.File
contributing/samples/authn-adk-all-in-one/hotel_booker_app/main.pyerror_messagevalue will no longer contain sensitive exception details.This keeps all existing control flow and status codes the same, but prevents internal exception details from reaching the client.
Suggested fixes powered by Copilot Autofix. Review carefully before merging.