Skip to content

Potential fix for code scanning alert no. 13: Information exposure through an exception#9

Draft
venkateshpabbati wants to merge 1 commit intomainfrom
alert-autofix-13
Draft

Potential fix for code scanning alert no. 13: Information exposure through an exception#9
venkateshpabbati wants to merge 1 commit intomainfrom
alert-autofix-13

Conversation

@venkateshpabbati
Copy link
Copy Markdown
Owner

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:

  1. In hotelbooker_core.py, update the except sqlite3.Error as e: block in get_booking_details so that:

    • The exception is logged using the standard logging module with logging.exception(...) (which records stack trace).
    • The function returns a generic message like "An internal error occurred while retrieving booking details." instead of f-string including {e}.
  2. In main.py, for consistency and to fix all variants in one go, avoid returning error_message from internal methods directly to clients when it might contain exception text. Instead:

    • If error_message equals known safe user messages (like the “no booking found…” or “Please provide either a booking ID or a guest name…” already crafted inside HotelBooker), you can return it.
    • For unexpected internal errors (currently only coming from the exception path), ignore the internal text and send a generic error string.

    However, with the change in HotelBooker to return an already-generic message, reusing error_message in main.py becomes 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.py

    • In get_available_hotels, errors are also using f"...: {e}". For completeness and to share behavior, it’s better to adjust this too, but CodeQL’s reported path concerns get_booking_details. To keep changes minimal, we will:
      • Log the exception.
      • Replace f"Error getting booking details: {e}" with a generic fixed-string message.
    • Ensure logging is configured somewhere in the app; since we already import logging at the top of hotelbooker_core.py, we can safely call logging.exception without further imports.
  • File contributing/samples/authn-adk-all-in-one/hotel_booker_app/main.py

    • No change is strictly required, because after sanitizing the message in the core, the error_message value will no longer contain sensitive exception details.
    • We will leave it as-is to avoid altering functionality: users still see specific “no booking found” and “missing info” messages.

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.

…rough an exception

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant